test: improve unit tes coverage

Author: maltesander

rust/operator-binary/src/authorization/opa.rs

@@ -150,3 +150,70 @@ impl TrinoOpaConfig {
         })
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn minimal_opa() -> TrinoOpaConfig {
+        TrinoOpaConfig {
+            non_batched_connection_string: "http://opa/allow".to_string(),
+            batched_connection_string: "http://opa/batch".to_string(),
+            row_filters_connection_string: None,
+            batched_column_masking_connection_string: None,
+            allow_permission_management_operations: false,
+            tls_secret_class: None,
+        }
+    }
+
+    #[test]
+    fn as_config_renders_only_required_keys_when_optionals_are_unset() {
+        let config = minimal_opa().as_config();
+
+        assert_eq!(
+            config.get("access-control.name").map(String::as_str),
+            Some("opa")
+        );
+        assert_eq!(
+            config.get("opa.policy.uri").map(String::as_str),
+            Some("http://opa/allow")
+        );
+        assert_eq!(
+            config.get("opa.policy.batched-uri").map(String::as_str),
+            Some("http://opa/batch")
+        );
+        assert!(!config.contains_key("opa.policy.row-filters-uri"));
+        assert!(!config.contains_key("opa.policy.batch-column-masking-uri"));
+        assert!(!config.contains_key("opa.allow-permission-management-operations"));
+    }
+
+    #[test]
+    fn as_config_renders_optional_keys_when_set() {
+        let config = TrinoOpaConfig {
+            row_filters_connection_string: Some("http://opa/rowFilters".to_string()),
+            batched_column_masking_connection_string: Some(
+                "http://opa/batchColumnMasks".to_string(),
+            ),
+            allow_permission_management_operations: true,
+            ..minimal_opa()
+        }
+        .as_config();
+
+        assert_eq!(
+            config.get("opa.policy.row-filters-uri").map(String::as_str),
+            Some("http://opa/rowFilters")
+        );
+        assert_eq!(
+            config
+                .get("opa.policy.batch-column-masking-uri")
+                .map(String::as_str),
+            Some("http://opa/batchColumnMasks")
+        );
+        assert_eq!(
+            config
+                .get("opa.allow-permission-management-operations")
+                .map(String::as_str),
+            Some("true")
+        );
+    }
+}

rust/operator-binary/src/config/s3.rs

@@ -130,3 +130,47 @@ impl ResolvedS3Config {
         Ok(resolved_config)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use stackable_operator::commons::tls_verification::Tls;
+
+    use super::*;
+
+    fn tls_details(verification: Option<TlsVerification>) -> TlsClientDetails {
+        TlsClientDetails {
+            tls: verification.map(|verification| Tls { verification }),
+        }
+    }
+
+    #[test]
+    fn no_tls_yields_no_truststore_commands() {
+        assert!(
+            s3_tls_truststore_commands(&tls_details(None))
+                .unwrap()
+                .is_empty()
+        );
+    }
+
+    #[test]
+    fn webpki_verification_yields_no_truststore_commands() {
+        let details = tls_details(Some(TlsVerification::Server(TlsServerVerification {
+            ca_cert: CaCert::WebPki {},
+        })));
+        assert!(s3_tls_truststore_commands(&details).unwrap().is_empty());
+    }
+
+    #[test]
+    fn secret_class_verification_yields_truststore_commands() {
+        let details = tls_details(Some(TlsVerification::Server(TlsServerVerification {
+            ca_cert: CaCert::SecretClass("tls".to_string()),
+        })));
+        assert!(!s3_tls_truststore_commands(&details).unwrap().is_empty());
+    }
+
+    #[test]
+    fn disabled_verification_is_rejected() {
+        let details = tls_details(Some(TlsVerification::None {}));
+        assert!(s3_tls_truststore_commands(&details).is_err());
+    }
+}

rust/operator-binary/src/controller/build/graceful_shutdown.rs

@@ -145,7 +145,13 @@ mod tests {
     use stackable_operator::shared::time::Duration;
 
     use super::*;
-    use crate::controller::build::properties::test_support::validated_cluster_from_yaml;
+    use crate::{
+        config::fault_tolerant_execution::ResolvedFaultTolerantExecutionConfig,
+        controller::build::properties::test_support::{
+            MINIMAL_TRINO_YAML, empty_derefs, validated_cluster_from_yaml,
+            validated_cluster_from_yaml_with_derefs,
+        },
+    };
 
     /// A worker role group without an explicit `gracefulShutdownTimeout` falls back to the
     /// product default.
@@ -255,4 +261,107 @@ mod tests {
             Duration::from_minutes_unchecked(5)
         );
     }
+
+    fn fte_derefs() -> crate::controller::dereference::DereferencedObjects {
+        let mut derefs = empty_derefs();
+        derefs.resolved_fte_config = Some(ResolvedFaultTolerantExecutionConfig {
+            config_properties: BTreeMap::new(),
+            exchange_manager_properties: BTreeMap::new(),
+            volumes: Vec::new(),
+            volume_mounts: Vec::new(),
+            init_container_extra_start_commands: Vec::new(),
+        });
+        derefs
+    }
+
+    #[test]
+    fn coordinator_props_set_query_max_execution_time_without_fte() {
+        let cluster = validated_cluster_from_yaml(MINIMAL_TRINO_YAML);
+        let props = graceful_shutdown_config_properties(&cluster, TrinoRole::Coordinator);
+        // The default worker graceful-shutdown timeout is 60 minutes (3600s).
+        assert_eq!(
+            props.get("query.max-execution-time").map(String::as_str),
+            Some("3600s")
+        );
+    }
+
+    #[test]
+    fn coordinator_props_empty_with_fault_tolerant_execution() {
+        let cluster = validated_cluster_from_yaml_with_derefs(MINIMAL_TRINO_YAML, fte_derefs());
+        let props = graceful_shutdown_config_properties(&cluster, TrinoRole::Coordinator);
+        // With fault-tolerant execution, queries may be retried, so no max-execution-time is set.
+        assert!(props.is_empty());
+    }
+
+    #[test]
+    fn worker_props_set_shutdown_grace_period() {
+        let cluster = validated_cluster_from_yaml(MINIMAL_TRINO_YAML);
+        let props = graceful_shutdown_config_properties(&cluster, TrinoRole::Worker);
+        assert_eq!(
+            props.get("shutdown.grace-period").map(String::as_str),
+            Some("30s")
+        );
+    }
+
+    #[test]
+    fn worker_termination_grace_period_adds_overhead_and_sets_pre_stop() {
+        let cluster = validated_cluster_from_yaml(MINIMAL_TRINO_YAML);
+        let merged = &cluster.role_group_configs[&TrinoRole::Worker]
+            .values()
+            .next()
+            .unwrap()
+            .config;
+        let mut pod_builder = PodBuilder::new();
+        let mut trino_builder = ContainerBuilder::new("trino").unwrap();
+        add_graceful_shutdown_config(
+            &cluster,
+            &TrinoRole::Worker,
+            merged,
+            &mut pod_builder,
+            &mut trino_builder,
+        )
+        .unwrap();
+
+        // Default worker timeout 3600s + 2 * 30s grace + 10s safety = 3670s.
+        let spec = pod_builder.build_template().spec.unwrap();
+        assert_eq!(spec.termination_grace_period_seconds, Some(3670));
+
+        let command = trino_builder
+            .build()
+            .lifecycle
+            .unwrap()
+            .pre_stop
+            .unwrap()
+            .exec
+            .unwrap()
+            .command
+            .unwrap();
+        assert!(command.iter().any(|arg| arg.contains("sleep 3670")));
+    }
+
+    #[test]
+    fn coordinator_termination_grace_period_has_no_overhead_or_pre_stop() {
+        let cluster = validated_cluster_from_yaml(MINIMAL_TRINO_YAML);
+        let merged = &cluster.role_group_configs[&TrinoRole::Coordinator]
+            .values()
+            .next()
+            .unwrap()
+            .config;
+        let mut pod_builder = PodBuilder::new();
+        let mut trino_builder = ContainerBuilder::new("trino").unwrap();
+        add_graceful_shutdown_config(
+            &cluster,
+            &TrinoRole::Coordinator,
+            merged,
+            &mut pod_builder,
+            &mut trino_builder,
+        )
+        .unwrap();
+
+        // The coordinator default timeout (900s) is used verbatim, with no overhead.
+        let spec = pod_builder.build_template().spec.unwrap();
+        assert_eq!(spec.termination_grace_period_seconds, Some(900));
+        // Coordinators do not get a graceful-shutdown pre-stop hook.
+        assert!(trino_builder.build().lifecycle.is_none());
+    }
 }

rust/operator-binary/src/controller/build/properties/mod.rs

@@ -42,25 +42,40 @@ pub(crate) mod test_support {
         crd::{authentication::ResolvedAuthenticationClassRef, v1alpha1},
     };
 
+    /// Dereferenced objects with no external resources resolved.
+    pub fn empty_derefs() -> DereferencedObjects {
+        DereferencedObjects {
+            resolved_authentication_classes: Vec::new(),
+            catalog_definitions: Vec::new(),
+            catalogs: Vec::new(),
+            trino_opa_config: None,
+            resolved_fte_config: None,
+            resolved_client_protocol_config: None,
+        }
+    }
+
     pub fn validated_cluster_from_yaml(yaml: &str) -> ValidatedCluster {
-        validated_cluster_from_yaml_with_auth(yaml, Vec::new())
+        validated_cluster_from_yaml_with_derefs(yaml, empty_derefs())
     }
 
     /// Like [`validated_cluster_from_yaml`], but injects already-resolved AuthenticationClasses so
     /// tests can exercise authentication-dependent branches.
     pub fn validated_cluster_from_yaml_with_auth(
         yaml: &str,
         resolved_authentication_classes: Vec<ResolvedAuthenticationClassRef>,
+    ) -> ValidatedCluster {
+        let mut derefs = empty_derefs();
+        derefs.resolved_authentication_classes = resolved_authentication_classes;
+        validated_cluster_from_yaml_with_derefs(yaml, derefs)
+    }
+
+    /// Validates `yaml` against the given (test-supplied) dereferenced objects, letting tests
+    /// exercise branches that depend on resolved inputs (e.g. fault-tolerant execution).
+    pub fn validated_cluster_from_yaml_with_derefs(
+        yaml: &str,
+        derefs: DereferencedObjects,
     ) -> ValidatedCluster {
         let trino: v1alpha1::TrinoCluster = serde_yaml::from_str(yaml).expect("invalid test YAML");
-        let derefs = DereferencedObjects {
-            resolved_authentication_classes,
-            catalog_definitions: Vec::new(),
-            catalogs: Vec::new(),
-            trino_opa_config: None,
-            resolved_fte_config: None,
-            resolved_client_protocol_config: None,
-        };
         let operator_env = OperatorEnvironmentOptions {
             operator_namespace: "stackable-operators".to_string(),
             operator_service_name: "trino-operator".to_string(),

rust/operator-binary/src/controller/build/properties/security_properties.rs

@@ -44,15 +44,20 @@ mod tests {
         crd::TrinoRole,
     };
 
-    #[test]
-    fn default_renders_networkaddress_cache_settings() {
-        let cluster = validated_cluster_from_yaml(MINIMAL_TRINO_YAML);
-        let rg = cluster.role_group_configs[&TrinoRole::Coordinator]
+    fn coordinator_rg(
+        cluster: &crate::controller::ValidatedCluster,
+    ) -> crate::controller::TrinoRoleGroupConfig {
+        cluster.role_group_configs[&TrinoRole::Coordinator]
             .values()
             .next()
             .unwrap()
-            .clone();
-        let props = build(&rg);
+            .clone()
+    }
+
+    #[test]
+    fn default_renders_networkaddress_cache_settings() {
+        let cluster = validated_cluster_from_yaml(MINIMAL_TRINO_YAML);
+        let props = build(&coordinator_rg(&cluster));
         assert_eq!(
             props.get("networkaddress.cache.ttl").map(String::as_str),
             Some("30")
@@ -64,4 +69,54 @@ mod tests {
             Some("0")
         );
     }
+
+    #[test]
+    fn user_override_wins_and_extra_key_is_added() {
+        let cluster = validated_cluster_from_yaml(
+            r#"
+            apiVersion: trino.stackable.tech/v1alpha1
+            kind: TrinoCluster
+            metadata:
+              name: simple-trino
+              namespace: default
+              uid: "e6ac237d-a6d4-43a1-8135-f36506110912"
+            spec:
+              image:
+                productVersion: "479"
+              clusterConfig:
+                catalogLabelSelector: {}
+              coordinators:
+                roleGroups:
+                  default:
+                    replicas: 1
+                    configOverrides:
+                      security.properties:
+                        networkaddress.cache.ttl: "99"
+                        custom.extra.key: "myvalue"
+              workers:
+                roleGroups:
+                  default:
+                    replicas: 1
+            "#,
+        );
+        let props = build(&coordinator_rg(&cluster));
+
+        // User override wins over the default.
+        assert_eq!(
+            props.get("networkaddress.cache.ttl").map(String::as_str),
+            Some("99")
+        );
+        // Extra (non-default) override key is added.
+        assert_eq!(
+            props.get("custom.extra.key").map(String::as_str),
+            Some("myvalue")
+        );
+        // Untouched default remains.
+        assert_eq!(
+            props
+                .get("networkaddress.cache.negative.ttl")
+                .map(String::as_str),
+            Some("0")
+        );
+    }
 }