test: add unit tests for conditional settings

Author: maltesander

rust/operator-binary/src/crd/security.rs

@@ -61,20 +61,19 @@ impl ZookeeperSecurity {
     // ports
     pub const CLIENT_PORT: Port = Port(2181);
     pub const CLIENT_PORT_NAME: &'static str = "clientPort";
+    // TLS store file names (relative to the respective TLS dir)
+    pub const KEYSTORE_FILE: &'static str = "keystore.p12";
     // directories
     pub const QUORUM_TLS_DIR: &'static str = "/stackable/quorum_tls";
     pub const QUORUM_TLS_MOUNT_DIR: &'static str = "/stackable/quorum_tls_mount";
+    pub const QUORUM_TLS_VOLUME_NAME: &'static str = "quorum-tls";
     pub const SECURE_CLIENT_PORT: Port = Port(2282);
     pub const SECURE_CLIENT_PORT_NAME: &'static str = "secureClientPort";
     pub const SERVER_CNXN_FACTORY: &'static str = "serverCnxnFactory";
     pub const SERVER_TLS_DIR: &'static str = "/stackable/server_tls";
     pub const SERVER_TLS_MOUNT_DIR: &'static str = "/stackable/server_tls_mount";
     // TLS volume names (the mount name must match the volume name)
     pub const SERVER_TLS_VOLUME_NAME: &'static str = "server-tls";
-    pub const QUORUM_TLS_VOLUME_NAME: &'static str = "quorum-tls";
-    // TLS store file names (relative to the respective TLS dir)
-    pub const KEYSTORE_FILE: &'static str = "keystore.p12";
-    pub const TRUSTSTORE_FILE: &'static str = "truststore.p12";
     // Common TLS
     pub const SSL_AUTH_PROVIDER_X509: &'static str = "authProvider.x509";
     // Client TLS
@@ -95,6 +94,7 @@ impl ZookeeperSecurity {
     // Mis
     pub const STORE_PASSWORD_ENV: &'static str = "STORE_PASSWORD";
     pub const SYSTEM_TRUST_STORE_DIR: &'static str = "/etc/pki/java/cacerts";
+    pub const TRUSTSTORE_FILE: &'static str = "truststore.p12";
 
     /// Build a `ZookeeperSecurity` from a [`v1alpha1::ZookeeperCluster`] and already-resolved
     /// [`DereferencedAuthenticationClasses`]. Synchronous; intended to be called from the validate

rust/operator-binary/src/zk_controller.rs

@@ -434,16 +434,23 @@ pub(crate) mod test_support {
         }
     }
 
-    /// Runs the real validate step against a minimal (auth-free) fixture.
-    pub fn validated_cluster(zk: &v1alpha1::ZookeeperCluster) -> ValidatedCluster {
+    /// Runs the real validate step against a minimal (auth-free) fixture, returning the result so
+    /// tests can assert on validation errors.
+    pub fn try_validate(
+        zk: &v1alpha1::ZookeeperCluster,
+    ) -> Result<ValidatedCluster, super::validate::Error> {
         validate(
             zk,
             &DereferencedObjects {
                 authentication_classes: DereferencedAuthenticationClasses::new_for_tests(),
             },
             &operator_environment(),
         )
-        .expect("validate should succeed for the test fixture")
+    }
+
+    /// Runs the real validate step against a minimal (auth-free) fixture.
+    pub fn validated_cluster(zk: &v1alpha1::ZookeeperCluster) -> ValidatedCluster {
+        try_validate(zk).expect("validate should succeed for the test fixture")
     }
 }
 
@@ -599,6 +606,131 @@ mod tests {
         assert!(zoo_cfg.contains("syncLimit=2"), "{zoo_cfg}");
     }
 
+    #[test]
+    fn test_non_tls_uses_insecure_client_port() {
+        // With server TLS disabled (`serverSecretClass: null`) the insecure client port is used and
+        // none of the server-TLS settings (keystore, port unification) are emitted. This exercises
+        // the non-TLS branch of `ZookeeperSecurity::{client_port, config_settings}`.
+        let zookeeper_yaml = r#"
+        apiVersion: zookeeper.stackable.tech/v1alpha1
+        kind: ZookeeperCluster
+        metadata:
+          name: simple-zookeeper
+        spec:
+          image:
+            productVersion: "3.9.5"
+          clusterConfig:
+            tls:
+              serverSecretClass: null
+          servers:
+            roleGroups:
+              default:
+                replicas: 3
+        "#;
+        let cm = build_config_map(zookeeper_yaml).data.unwrap();
+        let zoo_cfg = cm.get("zoo.cfg").unwrap();
+        assert!(zoo_cfg.contains("clientPort=2181"), "{zoo_cfg}");
+        assert!(!zoo_cfg.contains("client.portUnification"), "{zoo_cfg}");
+        // The server-TLS keystore line (distinct from the always-present quorum keystore).
+        assert!(!zoo_cfg.contains("ssl.keyStore.location"), "{zoo_cfg}");
+    }
+
+    #[test]
+    fn test_config_override_wins_over_typed_config() {
+        // The typed `config` sets `syncLimit`, but a `configOverride` for the same key must win
+        // (configOverrides are layered last in `zoo.cfg` precedence).
+        let zookeeper_yaml = r#"
+        apiVersion: zookeeper.stackable.tech/v1alpha1
+        kind: ZookeeperCluster
+        metadata:
+          name: simple-zookeeper
+        spec:
+          image:
+            productVersion: "3.9.5"
+          servers:
+            roleGroups:
+              default:
+                replicas: 3
+                config:
+                  syncLimit: 9
+                configOverrides:
+                  zoo.cfg:
+                    syncLimit: "15"
+        "#;
+        let cm = build_config_map(zookeeper_yaml).data.unwrap();
+        let zoo_cfg = cm.get("zoo.cfg").unwrap();
+        assert!(zoo_cfg.contains("syncLimit=15"), "{zoo_cfg}");
+    }
+
+    #[test]
+    fn test_custom_log_config_omits_logback() {
+        // Automatic logging renders `logback.xml` into the ConfigMap; a custom log ConfigMap
+        // suppresses it (the `Custom` arm of `build_logback_config`).
+        let automatic_yaml = r#"
+        apiVersion: zookeeper.stackable.tech/v1alpha1
+        kind: ZookeeperCluster
+        metadata:
+          name: simple-zookeeper
+        spec:
+          image:
+            productVersion: "3.9.5"
+          servers:
+            roleGroups:
+              default:
+                replicas: 3
+        "#;
+        let automatic = build_config_map(automatic_yaml).data.unwrap();
+        assert!(automatic.contains_key("logback.xml"));
+
+        let custom_yaml = r#"
+        apiVersion: zookeeper.stackable.tech/v1alpha1
+        kind: ZookeeperCluster
+        metadata:
+          name: simple-zookeeper
+        spec:
+          image:
+            productVersion: "3.9.5"
+          servers:
+            roleGroups:
+              default:
+                replicas: 3
+                config:
+                  logging:
+                    containers:
+                      zookeeper:
+                        custom:
+                          configMap: my-log-config
+        "#;
+        let custom = build_config_map(custom_yaml).data.unwrap();
+        assert!(!custom.contains_key("logback.xml"));
+    }
+
+    #[test]
+    fn test_vector_agent_adds_vector_config() {
+        // Enabling the Vector agent (with the required aggregator discovery ConfigMap) adds
+        // `vector.yaml` to the rolegroup ConfigMap.
+        let zookeeper_yaml = r#"
+        apiVersion: zookeeper.stackable.tech/v1alpha1
+        kind: ZookeeperCluster
+        metadata:
+          name: simple-zookeeper
+        spec:
+          image:
+            productVersion: "3.9.5"
+          clusterConfig:
+            vectorAggregatorConfigMapName: vector-aggregator-discovery
+          servers:
+            roleGroups:
+              default:
+                replicas: 3
+                config:
+                  logging:
+                    enableVectorAgent: true
+        "#;
+        let cm = build_config_map(zookeeper_yaml).data.unwrap();
+        assert!(cm.contains_key("vector.yaml"));
+    }
+
     fn build_config_map(zookeeper_yaml: &str) -> ConfigMap {
         let zookeeper = minimal_zk(zookeeper_yaml);
         let validated_cluster = validated_cluster(&zookeeper);

rust/operator-binary/src/zk_controller/build/properties/zoo_cfg.rs

@@ -177,9 +177,190 @@ impl ValidatedCluster {
 
 #[cfg(test)]
 mod tests {
+    use std::str::FromStr;
+
+    use stackable_operator::v2::types::operator::RoleGroupName;
+
     use super::*;
     use crate::zk_controller::test_support::{cluster_info, minimal_zk, validated_cluster};
 
+    /// Validates `yaml` into a [`ValidatedCluster`] and returns its `server` `default` role group.
+    fn validated_with_default_rg(yaml: &str) -> (ValidatedCluster, ZookeeperRoleGroupConfig) {
+        let validated = validated_cluster(&minimal_zk(yaml));
+        let rg_name = RoleGroupName::from_str("default").expect("valid role group name");
+        let rg = validated.role_group_configs[&ZookeeperRole::Server][&rg_name].clone();
+        (validated, rg)
+    }
+
+    #[test]
+    fn server_addresses_append_secure_client_port_when_tls_enabled() {
+        // TLS is enabled by default, so the quorum entries advertise the secure client port.
+        let (validated, _) = validated_with_default_rg(
+            r#"
+            apiVersion: zookeeper.stackable.tech/v1alpha1
+            kind: ZookeeperCluster
+            metadata:
+              name: test-zk
+            spec:
+              image:
+                productVersion: "3.9.5"
+              servers:
+                roleGroups:
+                  default:
+                    replicas: 1
+            "#,
+        );
+        let entry = server_addresses(&validated, &cluster_info())
+            .remove("server.1")
+            .expect("missing server.1");
+        assert!(entry.ends_with(";2282"), "unexpected entry: {entry}");
+    }
+
+    #[test]
+    fn server_addresses_append_insecure_client_port_when_tls_disabled() {
+        // With server TLS disabled the quorum entries advertise the insecure client port.
+        let (validated, _) = validated_with_default_rg(
+            r#"
+            apiVersion: zookeeper.stackable.tech/v1alpha1
+            kind: ZookeeperCluster
+            metadata:
+              name: test-zk
+            spec:
+              image:
+                productVersion: "3.9.5"
+              clusterConfig:
+                tls:
+                  serverSecretClass: null
+              servers:
+                roleGroups:
+                  default:
+                    replicas: 1
+            "#,
+        );
+        let entry = server_addresses(&validated, &cluster_info())
+            .remove("server.1")
+            .expect("missing server.1");
+        assert!(entry.ends_with(";2181"), "unexpected entry: {entry}");
+    }
+
+    #[test]
+    fn server_addresses_unset_replicas_predicts_single_entry() {
+        // An unset replica count (HPA-managed) predicts a single-server quorum entry.
+        let validated = validated_cluster(&minimal_zk(
+            r#"
+            apiVersion: zookeeper.stackable.tech/v1alpha1
+            kind: ZookeeperCluster
+            metadata:
+              name: test-zk
+            spec:
+              image:
+                productVersion: "3.9.5"
+              servers:
+                roleGroups:
+                  default: {}
+            "#,
+        ));
+        let addresses = server_addresses(&validated, &cluster_info());
+        assert_eq!(addresses.len(), 1);
+        assert!(addresses.contains_key("server.1"), "{addresses:?}");
+    }
+
+    #[test]
+    fn server_addresses_honor_myid_offset_across_role_groups() {
+        // The quorum keys are `server.<i + myidOffset>`, aggregated across all server role groups.
+        let validated = validated_cluster(&minimal_zk(
+            r#"
+            apiVersion: zookeeper.stackable.tech/v1alpha1
+            kind: ZookeeperCluster
+            metadata:
+              name: test-zk
+            spec:
+              image:
+                productVersion: "3.9.5"
+              servers:
+                roleGroups:
+                  primary:
+                    replicas: 2
+                  secondary:
+                    replicas: 2
+                    config:
+                      myidOffset: 10
+            "#,
+        ));
+        let addresses = server_addresses(&validated, &cluster_info());
+        // primary: offset 1 -> server.1, server.2; secondary: offset 10 -> server.10, server.11.
+        for expected in ["server.1", "server.2", "server.10", "server.11"] {
+            assert!(
+                addresses.contains_key(expected),
+                "missing {expected} in {addresses:?}"
+            );
+        }
+        assert_eq!(addresses.len(), 4);
+    }
+
+    #[test]
+    fn metrics_http_port_defaults_and_honors_override() {
+        // Default: the seeded `METRICS_PROVIDER_HTTP_PORT`.
+        let (validated, rg) = validated_with_default_rg(
+            r#"
+            apiVersion: zookeeper.stackable.tech/v1alpha1
+            kind: ZookeeperCluster
+            metadata:
+              name: test-zk
+            spec:
+              image:
+                productVersion: "3.9.5"
+              servers:
+                roleGroups:
+                  default:
+                    replicas: 1
+            "#,
+        );
+        assert_eq!(validated.metrics_http_port(&rg), METRICS_PROVIDER_HTTP_PORT);
+
+        // A numeric `configOverride` wins.
+        let (validated, rg) = validated_with_default_rg(
+            r#"
+            apiVersion: zookeeper.stackable.tech/v1alpha1
+            kind: ZookeeperCluster
+            metadata:
+              name: test-zk
+            spec:
+              image:
+                productVersion: "3.9.5"
+              servers:
+                roleGroups:
+                  default:
+                    replicas: 1
+                    configOverrides:
+                      zoo.cfg:
+                        metricsProvider.httpPort: "9999"
+            "#,
+        );
+        assert_eq!(validated.metrics_http_port(&rg), Port::from(9999));
+
+        // A non-numeric `configOverride` falls back to the default.
+        let (validated, rg) = validated_with_default_rg(
+            r#"
+            apiVersion: zookeeper.stackable.tech/v1alpha1
+            kind: ZookeeperCluster
+            metadata:
+              name: test-zk
+            spec:
+              image:
+                productVersion: "3.9.5"
+              servers:
+                roleGroups:
+                  default:
+                    replicas: 1
+                    configOverrides:
+                      zoo.cfg:
+                        metricsProvider.httpPort: "not-a-port"
+            "#,
+        );
+        assert_eq!(validated.metrics_http_port(&rg), METRICS_PROVIDER_HTTP_PORT);
+    }
+
     #[test]
     fn server_addresses_predicts_one_entry_per_replica() {
         let zk = minimal_zk(