Refactor frontend context #327

tobyhede · 2025-09-12T04:57:14Z

Incremental refactoring Part 1 of N

The concept of the plan is to remove all the encryption and other business logic from the frontend and backend handlers.
The context will become the coordinator, responsible for parsing, type-checking, mapping and encrypting data, as well as maintaining session state.

This PR makes context the keeper of all of the data required for message handling:

proxy config,
encrypt config
schema
zerokms

The schema reloading has also changed.
Rather than pass the entire schema service so the reload function can be called, uses tokio channels.
We have not implemented a mechanism to trigger the encrypt config reloading, but will use the same channel mechanism and the command has been implemented.

Context is now the primary interface for handler operations
All handler dependencies now flow through Context

Remaining work:

context is now clearly too big
some functions in frontend still need to be extracted.

Plan is to get the functions out of the frontend and iterate on new abstractions within the context boundary.

Further work will be pushed in separate PRs.

…rontend - Create service traits for encryption and schema operations in services/mod.rs - Refactor Context to use service trait objects instead of direct Proxy dependency - Remove proxy field from Frontend, making it focus solely on PostgreSQL message handling - Context now serves as central coordinator with service delegation methods - Add backward compatibility with new_with_proxy() helper method - Implement service traits on Proxy for seamless migration - Add comprehensive test infrastructure with mock services - Maintain all existing functionality while improving separation of concerns This architectural change makes the frontend more focused, testable, and follows proper dependency injection patterns while preserving backward compatibility.

…per service - Create ColumnMapper service to handle column mapping and encryption configuration - Move get_projection_columns, get_param_columns, get_literal_columns from Frontend - Remove ~150 lines of column processing logic from Frontend - Add Context delegation methods for column processing operations - Remove unused imports (Identifier, TableColumn, Type) from Frontend - Fix test configuration with proper CRN format and required environment variables This refactoring achieves clear separation of concerns: - Frontend focuses solely on PostgreSQL message handling - ColumnMapper handles column mapping and encryption configuration - Context serves as coordinator with clean delegation methods - Maintains all existing functionality while improving code organization

- Extract Statement struct and impl into context/statement.rs (44 lines) - Extract Portal enum and impl into context/portal.rs (68 lines) - Remove ~110 lines from context/mod.rs main file - Add public re-exports for backward compatibility - Clean up unused imports and formatting This modular organization improves: - Code maintainability with focused single-responsibility modules - Context module readability by reducing main file complexity - Developer experience with logical separation of concerns - Architecture consistency following established patterns All existing functionality preserved with proper import handling.

- Move encryption configuration files from proxy/config/ to proxy/encrypt_config/ - Rename EncryptConfig to ColumnEncryptionConfig for clarity - Create new EncryptConfig wrapper struct in manager for better encapsulation - Update module structure and imports throughout codebase

…ation - Add TandemConfig::for_testing() for environment-independent unit tests - Add DatabaseConfig::for_testing() to handle private password field - Update Context::new_for_test() to use new testing constructors - Update Context::new_with_proxy() to extract schema and config from Proxy - Rename Proxy fields: encrypt_config → encrypt_config_manager, schema → schema_manager - Remove environment variable dependency from Context tests - Add integration test for schema reload with mapping disabled

- Replace handler function signature to accept Context<ZeroKms> instead of Proxy + client_id - Add config access methods to Context for database, TLS, and other settings - Convert EncryptionService trait from impl Future to async_trait for cleaner syntax - Update ZeroKms implementation to use async_trait - Modify startup functions to work with Context instead of Proxy - Remove services module as functionality is now integrated into Context - Fix type annotation errors by using concrete Context<ZeroKms> type This refactoring simplifies the architecture by making Context the primary interface for handler operations, avoiding dyn trait objects while maintaining strong typing.

tobyhede · 2025-09-16T00:35:24Z

packages/cipherstash-proxy/src/config/database.rs

    }
+
+    #[cfg(test)]
+    pub fn for_testing() -> Self {


Started with impl Default but a default implies that there is a sensible default, but there is not.

Went withfor_testing instead, but open to ideas.

I'm ok with this

tobyhede · 2025-09-16T00:36:36Z

packages/cipherstash-proxy/src/main.rs

-
                    client_id += 1;

+                    let context = proxy.context(client_id);


Proxy is the core orchestrator. We create a new context from the proxy for a particular client.

tobyhede · 2025-09-16T00:38:23Z

packages/cipherstash-proxy/src/postgresql/column_mapper.rs

+    encrypt_config: Arc<EncryptConfig>,
+}
+
+impl ColumnMapper {


Functions responsible for mapping a column to configuration lifted straight from frontend.

I expect this to evolve with further refactoring.

tobyhede · 2025-09-16T00:39:31Z

packages/cipherstash-proxy/src/postgresql/context/portal.rs

+use std::sync::Arc;
+
+#[derive(Clone, Debug)]
+pub enum Portal {


Pulled from context.rs to reduce file size.

tobyhede · 2025-09-16T00:39:45Z

packages/cipherstash-proxy/src/postgresql/context/statement.rs

+/// Type Analysed parameters and projection
+///
+#[derive(Debug, Clone, PartialEq)]
+pub struct Statement {


Pulled from context.rs to reduce file size.

tobyhede · 2025-09-16T00:40:36Z

packages/cipherstash-proxy/src/proxy/encrypt_config/config.rs

-use tracing::debug;
+
+#[derive(Debug, Deserialize, Serialize, Clone, Default)]
+pub struct ColumnEncryptionConfig {


Too many config ... just trying to disambiguate.

tobyhede · 2025-09-16T00:43:25Z

packages/cipherstash-proxy/src/proxy/mod.rs

+        Ok(version)
+    }
+
+    pub fn receive(


Responsible for receiving ReloadCommand messages.

The ReloadCommand holds a responder that lets the calling context know that reload is complete.

Copilot

Pull Request Overview

This PR implements incremental refactoring to remove encryption and business logic from frontend/backend handlers by making Context the central coordinator for data handling. It restructures the proxy architecture to use Context as the primary interface for handler operations.

Refactors Context to include all handler dependencies (config, encryption service, schema, etc.)
Introduces new EncryptionService trait and moves encryption logic to ZeroKms implementation
Replaces direct schema service passing with tokio channel-based reload mechanism

Reviewed Changes

Copilot reviewed 27 out of 28 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
packages/cipherstash-proxy/src/proxy/mod.rs	Major refactor to create Context factory method and implement channel-based reload system
packages/cipherstash-proxy/src/postgresql/context/mod.rs	Transforms Context into generic service coordinator with encryption, config and schema access
packages/cipherstash-proxy/src/postgresql/frontend.rs	Removes direct proxy dependency, delegates operations through Context
packages/cipherstash-proxy/src/postgresql/backend.rs	Removes direct proxy dependency, uses Context for all operations
packages/cipherstash-proxy/src/postgresql/handler.rs	Updates to use Context instead of Proxy for client handling
packages/cipherstash-proxy/src/proxy/zerokms/zerokms.rs	Implements EncryptionService trait and removes default_keyset_id parameter
packages/cipherstash-proxy/src/postgresql/column_mapper.rs	New service for processing SQL statement columns and mapping to encryption configs

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

packages/showcase/src/schema.rs

packages/showcase/src/data.rs

packages/cipherstash-proxy/src/postgresql/context/statement.rs

packages/cipherstash-proxy/src/postgresql/column_mapper.rs

Copilot · 2025-09-17T02:18:45Z

packages/cipherstash-proxy/src/main.rs

-                (listener, proxy) = reload_config(listener, &args, proxy).await;
+                (listener, proxy) = match reload_config(listener, &args).await {
+                    Ok((listener, proxy)) => (listener, proxy),
+                    Err(_) => todo!(),


The todo!() macro will panic at runtime if this branch is executed. Replace with proper error handling that logs the error and either returns a default value or exits gracefully.

Suggested change

Err(_) => todo!(),

Err(err) => {

error!(

msg = "Failed to reload configuration",

error = err.to_string()

);

std::process::exit(exitcode::CONFIG);

},

packages/cipherstash-proxy/src/main.rs

packages/cipherstash-proxy/src/postgresql/context/mod.rs

freshtonic

Factoring^after > Factoring^before → approved

Added a few comments.

freshtonic · 2025-09-17T02:15:33Z

packages/cipherstash-proxy/src/config/database.rs

    }
+
+    #[cfg(test)]
+    pub fn for_testing() -> Self {


I'm ok with this

freshtonic · 2025-09-17T02:21:17Z

packages/cipherstash-proxy/src/proxy/mod.rs

+        tokio::task::spawn(async move {
+            while let Some(command) = reload_receiver.recv().await {
+                debug!(msg = "ReloadCommand received", ?command);
+                match command {
+                    ReloadCommand::DatabaseSchema(responder) => {
+                        schema_manager.reload().await;
+                        encrypt_config_manager.reload().await;
+                        let _ = responder.send(());
+                    }
+                    ReloadCommand::EncryptSchema(responder) => {
+                        encrypt_config_manager.reload().await;
+                        let _ = responder.send(());
+                    }
+                }
+            }
+        });
+    }


This is cool, reminds me of Erlang/Elixir gen servers.

Instead of sending () to the responder, would it an ergonomic improvement to send the reloaded database schema or encrypt schema? Of course, only of the responders care about the content of the schemas and not just that they're up to date.

The schema is already in an Arc<ArcSwap<T> so no need to resend to share the data.
I did have it sending true, but the empty responder is idiomatic and appears in the tokio guide.

freshtonic · 2025-09-17T02:22:02Z

packages/showcase/src/data.rs

 };

 pub async fn insert_test_data() {
+    println!("Insert test data");


Debugging leftover?

tobyhede added 5 commits September 9, 2025 15:05

tobyhede force-pushed the refactor-frontend-context branch 2 times, most recently from 58e1d62 to ef25adb Compare September 12, 2025 05:48

tobyhede force-pushed the refactor-frontend-context branch from ef25adb to a35b9f0 Compare September 12, 2025 05:50

wip

bc1aa6b

tobyhede force-pushed the refactor-frontend-context branch from dc7eed3 to bc1aa6b Compare September 15, 2025 02:31

tobyhede commented Sep 16, 2025

View reviewed changes

freshtonic self-requested a review September 17, 2025 02:15

coderdan requested a review from Copilot September 17, 2025 02:17

Copilot AI reviewed Sep 17, 2025

View reviewed changes

freshtonic approved these changes Sep 17, 2025

View reviewed changes

tobyhede added 2 commits September 17, 2025 14:29

fix: lint and cleanup

d44599f

chore: streamline config reload

9c05593

tobyhede merged commit 83fe635 into main Oct 20, 2025
2 checks passed

tobyhede deleted the refactor-frontend-context branch October 20, 2025 23:09

-                    Err(_) => todo!(),
+                    Err(err) => {
+                        error!(
+                            msg = "Failed to reload configuration",
+                            error = err.to_string()
+                        );
+                        std::process::exit(exitcode::CONFIG);
+                    },

Refactor frontend context #327

Refactor frontend context #327

Uh oh!

Conversation

tobyhede commented Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

freshtonic left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tobyhede commented Sep 12, 2025 •

edited

Loading

freshtonic left a comment •

edited

Loading