refactor crd to use spec.clusterConfig.clientProtocol.spooling

razvan · razvan · commit e9167100c645 · 2025-09-05T10:56:23.000+02:00
diff --git a/deploy/helm/trino-operator/crds/crds.yaml b/deploy/helm/trino-operator/crds/crds.yaml
diff --git a/docs/modules/trino/pages/usage-guide/client-spooling-protocol.adoc b/docs/modules/trino/pages/usage-guide/client-spooling-protocol.adoc
@@ -16,12 +16,13 @@ To enable it, you need to set the `spec.clusterConfig.clientSpoolingProtocol` co
 ----
 spec:
   clusterConfig:
-    clientSpoolingProtocol:
-      location: "s3://spooling-bucket/trino/" # <1>
-      filesystem:
-        s3: # <2>
-          connection:
-            reference: "minio"
+    clientProtocol:
+      spooling:
+        location: "s3://spooling-bucket/trino/" # <1>
+        filesystem:
+          s3: # <2>
+            connection:
+              reference: "minio"
 ----
 <1> Specifies the location where spooled data will be stored. This example uses an S3 bucket.
 <2> Configures the filesystem type for spooling. Only S3 is supported currently via the custom resource definition.
diff --git a/rust/operator-binary/src/command.rs b/rust/operator-binary/src/command.rs
@@ -24,7 +24,7 @@ pub fn container_prepare_args(
     catalogs: &[CatalogConfig],
     merged_config: &v1alpha1::TrinoConfig,
     resolved_fte_config: &Option<ResolvedFaultTolerantExecutionConfig>,
-    resolved_spooling_config: &Option<client_protocol::ResolvedSpoolingProtocolConfig>,
+    resolved_spooling_config: &Option<client_protocol::ResolvedClientProtocolConfig>,
 ) -> Vec<String> {
     let mut args = vec![];
 
@@ -98,7 +98,7 @@ pub fn container_trino_args(
     authentication_config: &TrinoAuthenticationConfig,
     catalogs: &[CatalogConfig],
     resolved_fte_config: &Option<ResolvedFaultTolerantExecutionConfig>,
-    resolved_spooling_config: &Option<client_protocol::ResolvedSpoolingProtocolConfig>,
+    resolved_spooling_config: &Option<client_protocol::ResolvedClientProtocolConfig>,
 ) -> Vec<String> {
     let mut args = vec![
         // copy config files to a writeable empty folder
diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs
@@ -464,10 +464,9 @@ pub async fn reconcile_trino(
     };
 
     // Resolve client spooling protocol configuration with S3 connections if needed
-    let resolved_spooling_config = match trino.spec.cluster_config.client_spooling_protocol.as_ref()
-    {
+    let resolved_client_protocol_config = match trino.spec.cluster_config.client_protocol.as_ref() {
         Some(spooling_config) => Some(
-            client_protocol::ResolvedSpoolingProtocolConfig::from_config(
+            client_protocol::ResolvedClientProtocolConfig::from_config(
                 spooling_config,
                 Some(client),
                 &namespace,
@@ -599,7 +598,7 @@ pub async fn reconcile_trino(
                 &trino_opa_config,
                 &client.kubernetes_cluster_info,
                 &resolved_fte_config,
-                &resolved_spooling_config,
+                &resolved_client_protocol_config,
             )?;
             let rg_catalog_configmap = build_rolegroup_catalog_config_map(
                 trino,
@@ -618,7 +617,7 @@ pub async fn reconcile_trino(
                 &catalogs,
                 &rbac_sa.name_any(),
                 &resolved_fte_config,
-                &resolved_spooling_config,
+                &resolved_client_protocol_config,
             )?;
 
             cluster_resources
@@ -728,7 +727,7 @@ fn build_rolegroup_config_map(
     trino_opa_config: &Option<TrinoOpaConfig>,
     cluster_info: &KubernetesClusterInfo,
     resolved_fte_config: &Option<ResolvedFaultTolerantExecutionConfig>,
-    resolved_spooling_config: &Option<client_protocol::ResolvedSpoolingProtocolConfig>,
+    resolved_spooling_config: &Option<client_protocol::ResolvedClientProtocolConfig>,
 ) -> Result<ConfigMap> {
     let mut cm_conf_data = BTreeMap::new();
 
@@ -1013,7 +1012,7 @@ fn build_rolegroup_statefulset(
     catalogs: &[CatalogConfig],
     sa_name: &str,
     resolved_fte_config: &Option<ResolvedFaultTolerantExecutionConfig>,
-    resolved_spooling_config: &Option<client_protocol::ResolvedSpoolingProtocolConfig>,
+    resolved_spooling_config: &Option<client_protocol::ResolvedClientProtocolConfig>,
 ) -> Result<StatefulSet> {
     let role = trino
         .role(trino_role)
@@ -1689,7 +1688,7 @@ fn tls_volume_mounts(
     catalogs: &[CatalogConfig],
     requested_secret_lifetime: &Duration,
     resolved_fte_config: &Option<ResolvedFaultTolerantExecutionConfig>,
-    resolved_spooling_config: &Option<client_protocol::ResolvedSpoolingProtocolConfig>,
+    resolved_spooling_config: &Option<client_protocol::ResolvedClientProtocolConfig>,
 ) -> Result<()> {
     if let Some(server_tls) = trino.get_server_tls() {
         cb_prepare
diff --git a/rust/operator-binary/src/crd/client_protocol.rs b/rust/operator-binary/src/crd/client_protocol.rs
@@ -23,6 +23,11 @@ use crate::{
 const SPOOLING_S3_AWS_ACCESS_KEY: &str = "SPOOLING_S3_AWS_ACCESS_KEY";
 const SPOOLING_S3_AWS_SECRET_KEY: &str = "SPOOLING_S3_AWS_SECRET_KEY";
 
+#[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)]
+#[serde(rename_all = "camelCase")]
+pub enum ClientProtocolConfig {
+    Spooling(ClientSpoolingProtocolConfig),
+}
 #[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)]
 #[serde(rename_all = "camelCase")]
 pub struct ClientSpoolingProtocolConfig {
@@ -70,7 +75,7 @@ pub struct S3SpoolingConfig {
     pub upload_part_size: Option<Quantity>,
 }
 
-pub struct ResolvedSpoolingProtocolConfig {
+pub struct ResolvedClientProtocolConfig {
     /// Properties to add to config.properties
     pub config_properties: BTreeMap<String, String>,
 
@@ -92,11 +97,11 @@ pub struct ResolvedSpoolingProtocolConfig {
     pub init_container_extra_start_commands: Vec<String>,
 }
 
-impl ResolvedSpoolingProtocolConfig {
+impl ResolvedClientProtocolConfig {
     /// Resolve S3 connection properties from Kubernetes resources
     /// and prepare spooling filesystem configuration.
     pub async fn from_config(
-        config: &ClientSpoolingProtocolConfig,
+        config: &ClientProtocolConfig,
         client: Option<&Client>,
         namespace: &str,
     ) -> Result<Self, Error> {
@@ -109,39 +114,43 @@ impl ResolvedSpoolingProtocolConfig {
             init_container_extra_start_commands: Vec::new(),
         };
 
-        // Resolve external resources if Kubernetes client is available
-        // This should always be the case, except for when this function is called during unit tests
-        if let Some(client) = client {
-            match &config.filesystem {
-                SpoolingFileSystemConfig::S3(s3_config) => {
-                    resolved_config
-                        .resolve_s3_backend(s3_config, client, namespace)
-                        .await?;
+        match config {
+            ClientProtocolConfig::Spooling(spooling_config) => {
+                // Resolve external resources if Kubernetes client is available
+                // This should always be the case, except for when this function is called during unit tests
+                if let Some(client) = client {
+                    match &spooling_config.filesystem {
+                        SpoolingFileSystemConfig::S3(s3_config) => {
+                            resolved_config
+                                .resolve_s3_backend(s3_config, client, namespace)
+                                .await?;
+                        }
+                    }
                 }
-            }
-        }
 
-        resolved_config.spooling_manager_properties.extend([
-            ("fs.location".to_string(), config.location.clone()),
-            (
-                "spooling-manager.name".to_string(),
-                "filesystem".to_string(),
-            ),
-        ]);
-
-        // Enable spooling protocol
-        resolved_config.config_properties.extend([
-            ("protocol.spooling.enabled".to_string(), "true".to_string()),
-            (
-                "protocol.spooling.shared-secret-key".to_string(),
-                format!("${{ENV:{secret}}}", secret = ENV_SPOOLING_SECRET),
-            ),
-        ]);
+                resolved_config.spooling_manager_properties.extend([
+                    ("fs.location".to_string(), spooling_config.location.clone()),
+                    (
+                        "spooling-manager.name".to_string(),
+                        "filesystem".to_string(),
+                    ),
+                ]);
+
+                // Enable spooling protocol
+                resolved_config.config_properties.extend([
+                    ("protocol.spooling.enabled".to_string(), "true".to_string()),
+                    (
+                        "protocol.spooling.shared-secret-key".to_string(),
+                        format!("${{ENV:{secret}}}", secret = ENV_SPOOLING_SECRET),
+                    ),
+                ]);
 
-        // Finally, extend the spooling manager properties with any user configuration
-        resolved_config
-            .spooling_manager_properties
-            .extend(config.config_overrides.clone());
+                // Finally, extend the spooling manager properties with any user configuration
+                resolved_config
+                    .spooling_manager_properties
+                    .extend(spooling_config.config_overrides.clone());
+            }
+        }
 
         Ok(resolved_config)
     }
@@ -252,7 +261,7 @@ mod tests {
 
     #[tokio::test]
     async fn test_spooling_config() {
-        let config = ClientSpoolingProtocolConfig {
+        let config = ClientProtocolConfig::Spooling(ClientSpoolingProtocolConfig {
             location: "s3://my-bucket/spooling".to_string(),
             filesystem: SpoolingFileSystemConfig::S3(S3SpoolingConfig {
                 connection:
@@ -265,9 +274,9 @@ mod tests {
                 upload_part_size: None,
             }),
             config_overrides: HashMap::new(),
-        };
+        });
 
-        let resolved_spooling_config = ResolvedSpoolingProtocolConfig::from_config(
+        let resolved_spooling_config = ResolvedClientProtocolConfig::from_config(
             &config, None, // No client, so no external resolution
             "default",
         )
@@ -292,7 +301,7 @@ mod tests {
 
     #[tokio::test]
     async fn test_spooling_config_overrides() {
-        let config = ClientSpoolingProtocolConfig {
+        let config = ClientProtocolConfig::Spooling(ClientSpoolingProtocolConfig {
             location: "s3://my-bucket/spooling".to_string(),
             filesystem: SpoolingFileSystemConfig::S3(S3SpoolingConfig {
                 connection:
@@ -308,9 +317,9 @@ mod tests {
                 "protocol.spooling.retrieval-mode".to_string(),
                 "STORAGE".to_string(),
             )]),
-        };
+        });
 
-        let resolved_spooling_config = ResolvedSpoolingProtocolConfig::from_config(
+        let resolved_spooling_config = ResolvedClientProtocolConfig::from_config(
             &config, None, // No client, so no external resolution
             "default",
         )
diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs
@@ -305,7 +305,7 @@ pub mod versioned {
 
         /// Client spooling protocol configuration.
         #[serde(skip_serializing_if = "Option::is_none")]
-        pub client_spooling_protocol: Option<client_protocol::ClientSpoolingProtocolConfig>,
+        pub client_protocol: Option<client_protocol::ClientProtocolConfig>,
 
         /// Name of the Vector aggregator [discovery ConfigMap](DOCS_BASE_URL_PLACEHOLDER/concepts/service_discovery).
         /// It must contain the key `ADDRESS` with the address of the Vector aggregator.
diff --git a/tests/templates/kuttl/client-spooling/02-install-trino.yaml.j2 b/tests/templates/kuttl/client-spooling/02-install-trino.yaml.j2
@@ -16,13 +16,13 @@ spec:
     catalogLabelSelector:
       matchLabels:
         trino: trino
-    clientSpoolingProtocol:
-      enabled: true
-      location: "s3://spooling-bucket/trino/"
-      filesystem:
-        s3:
-          connection:
-            reference: "minio"
+    clientProtocol:
+      spooling:
+        location: "s3://spooling-bucket/trino/"
+        filesystem:
+          s3:
+            connection:
+              reference: "minio"
 {% if lookup('env', 'VECTOR_AGGREGATOR') %}
     vectorAggregatorConfigMapName: vector-aggregator-discovery
 {% endif %}
