fix: add tests & cleanup

Author: maltesander

rust/operator-binary/src/controller/build/resource/pdb.rs

@@ -36,3 +36,54 @@ pub fn build_pdb(
 fn max_unavailable_nodes() -> u16 {
     1
 }
+
+#[cfg(test)]
+mod tests {
+    use pretty_assertions::assert_eq;
+    use stackable_operator::{
+        commons::pdb::PdbConfig, k8s_openapi::apimachinery::pkg::util::intstr::IntOrString,
+    };
+
+    use super::*;
+    use crate::controller::build::properties::test_support::minimal_validated_cluster;
+
+    #[test]
+    fn build_pdb_returns_none_when_disabled() {
+        let cluster = minimal_validated_cluster();
+        let pdb = PdbConfig {
+            enabled: false,
+            max_unavailable: None,
+        };
+        assert!(build_pdb(&pdb, &cluster, &NifiRole::Node).is_none());
+    }
+
+    #[test]
+    fn build_pdb_uses_explicit_max_unavailable() {
+        let cluster = minimal_validated_cluster();
+        let pdb = PdbConfig {
+            enabled: true,
+            max_unavailable: Some(2),
+        };
+
+        let spec = build_pdb(&pdb, &cluster, &NifiRole::Node)
+            .expect("an enabled PDB must be built")
+            .spec
+            .expect("the PDB must have a spec");
+        assert_eq!(Some(IntOrString::Int(2)), spec.max_unavailable);
+    }
+
+    #[test]
+    fn build_pdb_defaults_max_unavailable_to_one() {
+        let cluster = minimal_validated_cluster();
+        let pdb = PdbConfig {
+            enabled: true,
+            max_unavailable: None,
+        };
+
+        let spec = build_pdb(&pdb, &cluster, &NifiRole::Node)
+            .expect("an enabled PDB must be built")
+            .spec
+            .expect("the PDB must have a spec");
+        assert_eq!(Some(IntOrString::Int(1)), spec.max_unavailable);
+    }
+}

rust/operator-binary/src/controller/build/resource/service.rs

@@ -139,3 +139,47 @@ fn prometheus_annotations(product_version: &str) -> Annotations {
     ])
     .expect("should be valid annotations")
 }
+
+#[cfg(test)]
+mod tests {
+    use std::str::FromStr as _;
+
+    use pretty_assertions::assert_eq;
+    use rstest::rstest;
+
+    use super::*;
+    use crate::controller::build::properties::test_support::minimal_validated_cluster;
+
+    #[rstest]
+    // NiFi 1.x exposes metrics on a dedicated JMX-exporter port ...
+    #[case("1.28.1", METRICS_PORT_NAME, METRICS_PORT)]
+    // ... while NiFi 2.x serves them on the HTTPS port.
+    #[case("2.9.0", HTTPS_PORT_NAME, HTTPS_PORT)]
+    fn metrics_service_port_depends_on_version(
+        #[case] product_version: &str,
+        #[case] expected_name: &str,
+        #[case] expected_port: u16,
+    ) {
+        let port = metrics_service_port(product_version);
+        assert_eq!(Some(expected_name.to_string()), port.name);
+        assert_eq!(i32::from(expected_port), port.port);
+    }
+
+    #[test]
+    fn headless_service_is_cluster_ip_none_with_https_port() {
+        let cluster = minimal_validated_cluster();
+        let rg = RoleGroupName::from_str("default").expect("valid role-group name");
+
+        let spec = build_rolegroup_headless_service(&cluster, &rg)
+            .spec
+            .expect("headless service must have a spec");
+
+        assert_eq!(Some("ClusterIP".to_string()), spec.type_);
+        assert_eq!(Some("None".to_string()), spec.cluster_ip);
+
+        let ports = spec.ports.expect("headless service must expose ports");
+        assert_eq!(1, ports.len());
+        assert_eq!(Some(HTTPS_PORT_NAME.to_string()), ports[0].name);
+        assert_eq!(i32::from(HTTPS_PORT), ports[0].port);
+    }
+}

rust/operator-binary/src/controller/build/resource/statefulset.rs

@@ -57,6 +57,7 @@ use crate::{
         ValidatedCluster, ValidatedRoleGroupConfig,
         build::{
             graceful_shutdown::add_graceful_shutdown_config,
+            properties::ConfigFileName,
             resource::{
                 listener::{
                     LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME, build_group_listener_pvc,
@@ -291,8 +292,11 @@ pub(crate) async fn build_node_rolegroup_statefulset(
         format!("keytool -importkeystore -srckeystore {KEYSTORE_NIFI_CONTAINER_MOUNT}/truststore.p12 -destkeystore {STACKABLE_SERVER_TLS_DIR}/truststore.p12 -srcstorepass {STACKABLE_TLS_STORE_PASSWORD} -deststorepass {STACKABLE_TLS_STORE_PASSWORD}"),
 
         "echo Replacing config directory".to_string(),
-        "cp /conf/* /stackable/nifi/conf".to_string(),
-        "test -L /stackable/nifi/conf/logback.xml || ln -sf /stackable/log_config/logback.xml /stackable/nifi/conf/logback.xml".to_string(),
+        format!("cp {CONFIG_VOLUME_MOUNT}/* {NIFI_CONFIG_DIRECTORY}"),
+        format!(
+            "test -L {NIFI_CONFIG_DIRECTORY}/{logback} || ln -sf {STACKABLE_LOG_CONFIG_DIR}/{logback} {NIFI_CONFIG_DIRECTORY}/{logback}",
+            logback = ConfigFileName::Logback
+        ),
         format!(r#"export NODE_ADDRESS="{node_address}""#),
     ]);
 
@@ -311,23 +315,27 @@ pub(crate) async fn build_node_rolegroup_statefulset(
         ]);
     }
 
-    prepare_args.extend(vec![
-        "export LISTENER_DEFAULT_ADDRESS=$(cat /stackable/listener/default-address/address)"
-            .to_string(),
-    ]);
-    prepare_args.extend(vec![
-        "export LISTENER_DEFAULT_PORT_HTTPS=$(cat /stackable/listener/default-address/ports/https)"
-            .to_string(),
-    ]);
-
-    prepare_args.extend(vec![
-        "echo Templating config files".to_string(),
-        "config-utils template /stackable/nifi/conf/nifi.properties".to_string(),
-        "config-utils template /stackable/nifi/conf/state-management.xml".to_string(),
-        "config-utils template /stackable/nifi/conf/login-identity-providers.xml".to_string(),
-        "config-utils template /stackable/nifi/conf/authorizers.xml".to_string(),
-        "config-utils template /stackable/nifi/conf/security.properties".to_string(),
-    ]);
+    prepare_args.push(format!(
+        "export LISTENER_DEFAULT_ADDRESS=$(cat {LISTENER_VOLUME_DIR}/default-address/address)"
+    ));
+    prepare_args.push(format!(
+        "export LISTENER_DEFAULT_PORT_HTTPS=$(cat {LISTENER_VOLUME_DIR}/default-address/ports/https)"
+    ));
+
+    // Template the config files that contain `${env:...}`/`${file:...}` placeholders, in a fixed
+    // order. Sourced from the `ConfigFileName` enum so the file names stay in sync with the
+    // ConfigMap builder; `bootstrap.conf` and `logback.xml` are intentionally not templated.
+    prepare_args.push("echo Templating config files".to_string());
+    prepare_args.extend(
+        [
+            ConfigFileName::NifiProperties,
+            ConfigFileName::StateManagementXml,
+            ConfigFileName::LoginIdentityProviders,
+            ConfigFileName::Authorizers,
+            ConfigFileName::SecurityProperties,
+        ]
+        .map(|file| format!("config-utils template {NIFI_CONFIG_DIRECTORY}/{file}")),
+    );
 
     let mut container_prepare = new_container_builder(&PREPARE_CONTAINER_NAME);
 

rust/operator-binary/src/controller/validate.rs

@@ -246,3 +246,107 @@ pub(crate) fn build_role_group_configs(
     role_group_configs.insert(NifiRole::Node, groups);
     Ok(role_group_configs)
 }
+
+#[cfg(test)]
+mod tests {
+    use pretty_assertions::assert_eq;
+    use stackable_operator::v2::types::kubernetes::ConfigMapName;
+
+    use super::*;
+
+    /// A NiFi cluster with the Vector agent enabled at the Node role level.
+    const NIFI_VECTOR_ENABLED_YAML: &str = r#"
+        apiVersion: nifi.stackable.tech/v1alpha1
+        kind: NifiCluster
+        metadata:
+          name: simple-nifi
+          namespace: default
+        spec:
+          image:
+            productVersion: 2.9.0
+          clusterConfig:
+            authentication:
+              - authenticationClass: nifi-admin-credentials-simple
+            sensitiveProperties:
+              keySecret: simple-nifi-sensitive-property-key
+              autoGenerate: true
+          nodes:
+            config:
+              logging:
+                enableVectorAgent: true
+            roleGroups:
+              default:
+                replicas: 1
+    "#;
+
+    /// A minimal NiFi cluster with the Vector agent disabled (the default).
+    const NIFI_VECTOR_DISABLED_YAML: &str = r#"
+        apiVersion: nifi.stackable.tech/v1alpha1
+        kind: NifiCluster
+        metadata:
+          name: simple-nifi
+          namespace: default
+        spec:
+          image:
+            productVersion: 2.9.0
+          clusterConfig:
+            authentication:
+              - authenticationClass: nifi-admin-credentials-simple
+            sensitiveProperties:
+              keySecret: simple-nifi-sensitive-property-key
+              autoGenerate: true
+          nodes:
+            roleGroups:
+              default:
+                replicas: 1
+    "#;
+
+    fn default_rg(
+        configs: &BTreeMap<NifiRole, BTreeMap<RoleGroupName, ValidatedRoleGroupConfig>>,
+    ) -> &ValidatedRoleGroupConfig {
+        configs[&NifiRole::Node]
+            .get(&RoleGroupName::from_str("default").expect("valid role-group name"))
+            .expect("the 'default' role group must exist")
+    }
+
+    #[test]
+    fn vector_container_is_validated_when_agent_enabled() {
+        let nifi: v1alpha1::NifiCluster =
+            serde_yaml::from_str(NIFI_VECTOR_ENABLED_YAML).expect("invalid test YAML");
+        let aggregator = Some(ConfigMapName::from_str("nifi-vector-aggregator-discovery").unwrap());
+
+        let configs = build_role_group_configs(&nifi, &aggregator)
+            .expect("role group configs should validate");
+
+        let vector = default_rg(&configs)
+            .vector_container
+            .as_ref()
+            .expect("the Vector container config should be present when the agent is enabled");
+        assert_eq!(
+            "nifi-vector-aggregator-discovery",
+            vector.vector_aggregator_config_map_name.to_string()
+        );
+    }
+
+    #[test]
+    fn vector_agent_enabled_without_aggregator_name_fails() {
+        let nifi: v1alpha1::NifiCluster =
+            serde_yaml::from_str(NIFI_VECTOR_ENABLED_YAML).expect("invalid test YAML");
+
+        let error = build_role_group_configs(&nifi, &None)
+            .expect_err("a missing aggregator ConfigMap name must fail when Vector is enabled");
+        assert!(matches!(error, Error::MissingVectorAggregatorConfigMapName));
+    }
+
+    #[test]
+    fn no_vector_container_when_agent_disabled() {
+        let nifi: v1alpha1::NifiCluster =
+            serde_yaml::from_str(NIFI_VECTOR_DISABLED_YAML).expect("invalid test YAML");
+
+        // The aggregator name is not required when the Vector agent is disabled.
+        let configs =
+            build_role_group_configs(&nifi, &None).expect("role group configs should validate");
+
+        assert!(default_rg(&configs).vector_container.is_none());
+    }
+}