From 58c0f8706e5ac8b553ab55b07d561a1fe9184b22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCller?= Date: Tue, 14 Jul 2026 13:44:29 +0200 Subject: [PATCH 01/12] refactor: cover smoke 14-assert via config map unit tests for tls scenarios --- .../operator-binary/src/crd/authentication.rs | 24 ++ rust/operator-binary/src/zk_controller.rs | 310 +++++++++++++++++- 2 files changed, 324 insertions(+), 10 deletions(-) diff --git a/rust/operator-binary/src/crd/authentication.rs b/rust/operator-binary/src/crd/authentication.rs index 93b0a31c..8a2c0f10 100644 --- a/rust/operator-binary/src/crd/authentication.rs +++ b/rust/operator-binary/src/crd/authentication.rs @@ -131,4 +131,28 @@ impl DereferencedAuthenticationClasses { dereferenced_authentication_classes: vec![], } } + + /// USE ONLY IN TESTS! Builds a [`DereferencedAuthenticationClasses`] holding a single TLS + /// `AuthenticationClass`, mirroring the `use-client-auth-tls` kuttl scenario. This exercises the + /// client-mTLS branches of [`crate::crd::security::ZookeeperSecurity`] (secure client port and + /// `ssl.clientAuth=need`), including the case where server TLS is otherwise disabled and the + /// auth class alone turns TLS on. + #[cfg(test)] + pub fn new_for_tests_with_tls_client_auth() -> Self { + use stackable_operator::crd::authentication::tls; + + let auth_class = core::v1alpha1::AuthenticationClass::new( + "zk-client-auth-tls", + core::v1alpha1::AuthenticationClassSpec { + provider: core::v1alpha1::AuthenticationClassProvider::Tls( + tls::v1alpha1::AuthenticationProvider { + client_cert_secret_class: Some("zk-client-auth-secret".to_owned()), + }, + ), + }, + ); + DereferencedAuthenticationClasses { + dereferenced_authentication_classes: vec![auth_class], + } + } } diff --git a/rust/operator-binary/src/zk_controller.rs b/rust/operator-binary/src/zk_controller.rs index 67100563..98ed51b3 100644 --- a/rust/operator-binary/src/zk_controller.rs +++ b/rust/operator-binary/src/zk_controller.rs @@ -438,11 +438,20 @@ pub(crate) mod test_support { /// tests can assert on validation errors. pub fn try_validate( zk: &v1alpha1::ZookeeperCluster, + ) -> Result { + try_validate_with_auth(zk, DereferencedAuthenticationClasses::new_for_tests()) + } + + /// Runs the real validate step with caller-supplied (dereferenced) AuthenticationClasses, so + /// tests can exercise the client-mTLS matrix. + pub fn try_validate_with_auth( + zk: &v1alpha1::ZookeeperCluster, + authentication_classes: DereferencedAuthenticationClasses, ) -> Result { validate( zk, &DereferencedObjects { - authentication_classes: DereferencedAuthenticationClasses::new_for_tests(), + authentication_classes, }, &operator_environment(), ) @@ -452,18 +461,33 @@ pub(crate) mod test_support { pub fn validated_cluster(zk: &v1alpha1::ZookeeperCluster) -> ValidatedCluster { try_validate(zk).expect("validate should succeed for the test fixture") } + + /// Runs the real validate step with a single TLS client-auth `AuthenticationClass`, mirroring + /// the `use-client-auth-tls` kuttl scenario. + pub fn validated_cluster_with_client_auth(zk: &v1alpha1::ZookeeperCluster) -> ValidatedCluster { + try_validate_with_auth( + zk, + DereferencedAuthenticationClasses::new_for_tests_with_tls_client_auth(), + ) + .expect("validate should succeed for the test fixture") + } } #[cfg(test)] mod tests { - use std::str::FromStr; + use std::{collections::BTreeSet, str::FromStr}; use stackable_operator::{ k8s_openapi::api::core::v1::ConfigMap, v2::types::operator::RoleGroupName, }; use super::*; - use crate::zk_controller::test_support::{cluster_info, minimal_zk, validated_cluster}; + use crate::zk_controller::{ + test_support::{ + cluster_info, minimal_zk, validated_cluster, validated_cluster_with_client_auth, + }, + validate::ValidatedCluster, + }; #[test] fn test_default_config() { @@ -536,9 +560,10 @@ mod tests { #[test] fn test_seeded_operator_defaults() { - // These values are seeded directly by the ConfigMap builder and must stay - // byte-identical (pinned by the kuttl snapshot - // `tests/templates/kuttl/smoke/14-assert.yaml.j2`). + // These values are seeded directly by the ConfigMap builder and must stay byte-identical. + // This test (together with the TLS x client-auth matrix tests below) is the source of truth + // for the rendered `zoo.cfg` / `security.properties`; the kuttl smoke keeps only the + // live-cluster readiness/status checks. let zookeeper_yaml = r#" apiVersion: zookeeper.stackable.tech/v1alpha1 kind: ZookeeperCluster @@ -731,15 +756,280 @@ mod tests { assert!(cm.contains_key("vector.yaml")); } + // --------------------------------------------------------------------------------------------- + // TLS x client-auth matrix (candidate #1 in unit-tests.md). + // + // These four tests own what the kuttl smoke `14-assert` `zoo.cfg` heredoc used to assert across + // the `use-server-tls` x `use-client-auth-tls` scenario matrix: the exact set of `zoo.cfg` + // properties and the ports/`ssl.*` lines that vary with TLS and client mTLS. Asserting the full + // key set (not just `contains`) catches accidentally added *or* removed properties. Client mTLS + // is injected via `validated_cluster_with_client_auth`, which the previous `new_for_tests()` + // path could not reach. + // --------------------------------------------------------------------------------------------- + + /// The `zoo.cfg` keys that are always present regardless of the TLS/client-auth matrix + /// (operator-injected defaults + quorum TLS, which is always on). + fn base_zoo_cfg_keys() -> BTreeSet { + [ + "admin.serverPort", + "authProvider.x509", + "clientPort", + "dataDir", + "initLimit", + "metricsProvider.className", + "metricsProvider.httpPort", + "serverCnxnFactory", + "ssl.quorum.clientAuth", + "ssl.quorum.hostnameVerification", + "ssl.quorum.keyStore.location", + "ssl.quorum.trustStore.location", + "sslQuorum", + "syncLimit", + "tickTime", + ] + .into_iter() + .map(str::to_owned) + .collect() + } + + /// The `zoo.cfg` property keys, excluding the replica-dependent `server.` quorum entries + /// (those are asserted separately in `test_server_lines_use_myid_offset_across_rolegroups`). + fn zoo_cfg_keys(cm: &ConfigMap) -> BTreeSet { + cm.data + .as_ref() + .unwrap() + .get("zoo.cfg") + .unwrap() + .lines() + .filter_map(|line| line.split_once('=').map(|(k, _)| k.to_owned())) + .filter(|k| !k.starts_with("server.")) + .collect() + } + + /// TLS off, client-auth off: insecure client port, no server-`ssl.*` lines, no + /// `client.portUnification`, no `ssl.clientAuth`. + #[test] + fn test_matrix_no_tls_no_client_auth() { + let cm = build_config_map( + 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 zoo_cfg = cm.data.as_ref().unwrap().get("zoo.cfg").unwrap(); + + assert_eq!(zoo_cfg_keys(&cm), base_zoo_cfg_keys()); + assert!(zoo_cfg.contains("clientPort=2181"), "{zoo_cfg}"); + assert!(!zoo_cfg.contains("client.portUnification"), "{zoo_cfg}"); + assert!(!zoo_cfg.contains("ssl.clientAuth"), "{zoo_cfg}"); + assert!(!zoo_cfg.contains("ssl.keyStore.location"), "{zoo_cfg}"); + } + + /// TLS on (default), client-auth off: secure client port, server-`ssl.*` lines and + /// `client.portUnification`, but no `ssl.clientAuth`. + #[test] + fn test_matrix_server_tls_no_client_auth() { + let cm = build_config_map( + r#" + apiVersion: zookeeper.stackable.tech/v1alpha1 + kind: ZookeeperCluster + metadata: + name: simple-zookeeper + spec: + image: + productVersion: "3.9.5" + servers: + roleGroups: + default: + replicas: 3 + "#, + ); + let zoo_cfg = cm.data.as_ref().unwrap().get("zoo.cfg").unwrap(); + + let mut expected = base_zoo_cfg_keys(); + expected.extend( + [ + "client.portUnification", + "ssl.hostnameVerification", + "ssl.keyStore.location", + "ssl.trustStore.location", + ] + .into_iter() + .map(str::to_owned), + ); + assert_eq!(zoo_cfg_keys(&cm), expected); + assert!(zoo_cfg.contains("clientPort=2282"), "{zoo_cfg}"); + assert!(zoo_cfg.contains("client.portUnification=true"), "{zoo_cfg}"); + assert!( + zoo_cfg.contains("ssl.keyStore.location=/stackable/server_tls/keystore.p12"), + "{zoo_cfg}" + ); + assert!(!zoo_cfg.contains("ssl.clientAuth"), "{zoo_cfg}"); + } + + /// TLS on, client-auth on: as above plus `ssl.clientAuth=need`. + #[test] + fn test_matrix_server_tls_and_client_auth() { + let zk = minimal_zk( + r#" + apiVersion: zookeeper.stackable.tech/v1alpha1 + kind: ZookeeperCluster + metadata: + name: simple-zookeeper + spec: + image: + productVersion: "3.9.5" + servers: + roleGroups: + default: + replicas: 3 + "#, + ); + let cm = config_map_for(&validated_cluster_with_client_auth(&zk), "default"); + let zoo_cfg = cm.data.as_ref().unwrap().get("zoo.cfg").unwrap(); + + let mut expected = base_zoo_cfg_keys(); + expected.extend( + [ + "client.portUnification", + "ssl.clientAuth", + "ssl.hostnameVerification", + "ssl.keyStore.location", + "ssl.trustStore.location", + ] + .into_iter() + .map(str::to_owned), + ); + assert_eq!(zoo_cfg_keys(&cm), expected); + assert!(zoo_cfg.contains("clientPort=2282"), "{zoo_cfg}"); + assert!(zoo_cfg.contains("ssl.clientAuth=need"), "{zoo_cfg}"); + } + + /// Client-auth on while server TLS is explicitly disabled: the client `AuthenticationClass` + /// alone turns TLS on (secure port, `ssl.*` lines, `client.portUnification`) and adds + /// `ssl.clientAuth=need`. This cross term was previously unreachable in unit tests. + #[test] + fn test_matrix_client_auth_forces_tls_without_server_secret_class() { + let zk = minimal_zk( + 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 = config_map_for(&validated_cluster_with_client_auth(&zk), "default"); + let zoo_cfg = cm.data.as_ref().unwrap().get("zoo.cfg").unwrap(); + + let mut expected = base_zoo_cfg_keys(); + expected.extend( + [ + "client.portUnification", + "ssl.clientAuth", + "ssl.hostnameVerification", + "ssl.keyStore.location", + "ssl.trustStore.location", + ] + .into_iter() + .map(str::to_owned), + ); + assert_eq!(zoo_cfg_keys(&cm), expected); + assert!(zoo_cfg.contains("clientPort=2282"), "{zoo_cfg}"); + assert!(zoo_cfg.contains("client.portUnification=true"), "{zoo_cfg}"); + assert!(zoo_cfg.contains("ssl.clientAuth=need"), "{zoo_cfg}"); + } + + /// `server.` quorum lines are rendered into the per-rolegroup `zoo.cfg` with + /// `myidOffset` applied and colons escaped, aggregated across *all* server role groups. Mirrors + /// the kuttl smoke `14-assert` `server.N` lines (primary offset 10, secondary offset 20). + #[test] + fn test_server_lines_use_myid_offset_across_rolegroups() { + let zk = minimal_zk( + r#" + apiVersion: zookeeper.stackable.tech/v1alpha1 + kind: ZookeeperCluster + metadata: + name: test-zk + spec: + image: + productVersion: "3.9.5" + clusterConfig: + tls: + serverSecretClass: null + servers: + roleGroups: + primary: + replicas: 2 + config: + myidOffset: 10 + secondary: + replicas: 1 + config: + myidOffset: 20 + "#, + ); + let validated = validated_cluster(&zk); + // The `server.N` lines are identical in every rolegroup's `zoo.cfg`; inspect the primary's. + let cm = config_map_for(&validated, "primary"); + let zoo_cfg = cm.data.as_ref().unwrap().get("zoo.cfg").unwrap(); + + // Colons are escaped by the Java-properties writer (`\:`); non-TLS client port 2181. + assert!( + zoo_cfg.contains( + "server.10=test-zk-server-primary-0.test-zk-server-primary-headless.default.svc.cluster.local\\:2888\\:3888;2181" + ), + "{zoo_cfg}" + ); + assert!( + zoo_cfg.contains("server.11=test-zk-server-primary-1."), + "{zoo_cfg}" + ); + assert!( + zoo_cfg.contains("server.20=test-zk-server-secondary-0."), + "{zoo_cfg}" + ); + // Exactly the three expected quorum entries, no more. + assert_eq!( + zoo_cfg.lines().filter(|l| l.starts_with("server.")).count(), + 3, + "{zoo_cfg}" + ); + } + fn build_config_map(zookeeper_yaml: &str) -> ConfigMap { - let zookeeper = minimal_zk(zookeeper_yaml); - let validated_cluster = validated_cluster(&zookeeper); - let role_group_name = RoleGroupName::from_str("default").expect("valid role group name"); + config_map_for(&validated_cluster(&minimal_zk(zookeeper_yaml)), "default") + } + + /// Builds the rolegroup `ConfigMap` for the named server role group of a validated cluster. + fn config_map_for(validated_cluster: &ValidatedCluster, role_group: &str) -> ConfigMap { + let role_group_name = RoleGroupName::from_str(role_group).expect("valid role group name"); let rolegroup_config = &validated_cluster.role_group_configs[&ZookeeperRole::Server][&role_group_name]; config_map::build_server_rolegroup_config_map( - &validated_cluster, + validated_cluster, &cluster_info(), &role_group_name, rolegroup_config, From 6b040e0406bfd98ff54d22adaa26554e06e228ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCller?= Date: Thu, 16 Jul 2026 08:28:00 +0200 Subject: [PATCH 02/12] refactor: cover shape checks from 13-assert --- .../src/zk_controller/build/resource/pdb.rs | 77 ++++++ .../zk_controller/build/resource/service.rs | 122 +++++++++ .../build/resource/statefulset.rs | 253 ++++++++++++++++++ 3 files changed, 452 insertions(+) diff --git a/rust/operator-binary/src/zk_controller/build/resource/pdb.rs b/rust/operator-binary/src/zk_controller/build/resource/pdb.rs index 6c8099e9..57949d38 100644 --- a/rust/operator-binary/src/zk_controller/build/resource/pdb.rs +++ b/rust/operator-binary/src/zk_controller/build/resource/pdb.rs @@ -37,3 +37,80 @@ pub fn build_pdb( fn max_unavailable_servers() -> u16 { 1 } + +#[cfg(test)] +mod tests { + use stackable_operator::k8s_openapi::apimachinery::pkg::util::intstr::IntOrString; + + use super::*; + use crate::zk_controller::test_support::{minimal_zk, validated_cluster}; + + /// By default PDBs are enabled and default to `maxUnavailable: 1` for the server role. + #[test] + fn pdb_enabled_by_default_with_max_unavailable_one() { + let validated = validated_cluster(&minimal_zk( + r#" + apiVersion: zookeeper.stackable.tech/v1alpha1 + kind: ZookeeperCluster + metadata: + name: simple-zookeeper + spec: + image: + productVersion: "3.9.5" + servers: + roleGroups: + default: + replicas: 3 + "#, + )); + let role_config = validated.role_config.as_ref().expect("role config present"); + + let pdb = build_pdb(&role_config.pdb, &validated, &ZookeeperRole::Server) + .expect("PDB enabled by default"); + let spec = pdb.spec.as_ref().unwrap(); + assert_eq!(spec.max_unavailable, Some(IntOrString::Int(1))); + + let match_labels = spec + .selector + .as_ref() + .unwrap() + .match_labels + .as_ref() + .unwrap(); + assert_eq!( + match_labels + .get("app.kubernetes.io/component") + .map(String::as_str), + Some("server") + ); + } + + /// Disabling the PDB via `roleConfig` produces no `PodDisruptionBudget`. + #[test] + fn pdb_disabled_returns_none() { + let validated = validated_cluster(&minimal_zk( + r#" + apiVersion: zookeeper.stackable.tech/v1alpha1 + kind: ZookeeperCluster + metadata: + name: simple-zookeeper + spec: + image: + productVersion: "3.9.5" + servers: + roleConfig: + podDisruptionBudget: + enabled: false + roleGroups: + default: + replicas: 3 + "#, + )); + let role_config = validated.role_config.as_ref().expect("role config present"); + + assert!( + build_pdb(&role_config.pdb, &validated, &ZookeeperRole::Server).is_none(), + "PDB should be None when disabled" + ); + } +} diff --git a/rust/operator-binary/src/zk_controller/build/resource/service.rs b/rust/operator-binary/src/zk_controller/build/resource/service.rs index 81ac8b92..4670794b 100644 --- a/rust/operator-binary/src/zk_controller/build/resource/service.rs +++ b/rust/operator-binary/src/zk_controller/build/resource/service.rs @@ -115,3 +115,125 @@ pub(crate) fn build_server_rolegroup_metrics_service( status: None, } } + +#[cfg(test)] +mod tests { + use std::str::FromStr; + + use super::*; + use crate::{ + crd::ZookeeperRole, + zk_controller::test_support::{minimal_zk, validated_cluster}, + }; + + const DEFAULT_ZK: &str = r#" + apiVersion: zookeeper.stackable.tech/v1alpha1 + kind: ZookeeperCluster + metadata: + name: simple-zookeeper + spec: + image: + productVersion: "3.9.5" + servers: + roleGroups: + default: + replicas: 3 + "#; + + fn port(service: &Service, name: &str) -> i32 { + service + .spec + .as_ref() + .unwrap() + .ports + .as_ref() + .unwrap() + .iter() + .find(|p| p.name.as_deref() == Some(name)) + .unwrap_or_else(|| panic!("missing service port {name}")) + .port + } + + #[test] + fn headless_service_shape() { + let validated = validated_cluster(&minimal_zk(DEFAULT_ZK)); + let rg = RoleGroupName::from_str("default").expect("valid role group name"); + let service = build_server_rolegroup_headless_service(&validated, &rg); + + assert_eq!( + service.metadata.name.as_deref(), + Some("simple-zookeeper-server-default-headless") + ); + + let spec = service.spec.as_ref().unwrap(); + assert_eq!(spec.type_.as_deref(), Some("ClusterIP")); + assert_eq!(spec.cluster_ip.as_deref(), Some("None")); + assert_eq!(spec.publish_not_ready_addresses, Some(true)); + + // Only the leader/election ports (independent of the client TLS matrix). + assert_eq!(port(&service, ZOOKEEPER_LEADER_PORT_NAME), 2888); + assert_eq!(port(&service, ZOOKEEPER_ELECTION_PORT_NAME), 3888); + assert_eq!(spec.ports.as_ref().unwrap().len(), 2); + + let selector = spec.selector.as_ref().unwrap(); + assert_eq!( + selector + .get("app.kubernetes.io/component") + .map(String::as_str), + Some("server") + ); + assert_eq!( + selector + .get("app.kubernetes.io/role-group") + .map(String::as_str), + Some("default") + ); + } + + #[test] + fn metrics_service_shape_and_prometheus_annotations() { + let validated = validated_cluster(&minimal_zk(DEFAULT_ZK)); + let rg = RoleGroupName::from_str("default").expect("valid role group name"); + let rg_config = validated.role_group_configs[&ZookeeperRole::Server][&rg].clone(); + let service = build_server_rolegroup_metrics_service(&validated, &rg, &rg_config); + + assert_eq!( + service.metadata.name.as_deref(), + Some("simple-zookeeper-server-default-metrics") + ); + + // Prometheus scrape annotations. + let annotations = service.metadata.annotations.as_ref().unwrap(); + assert_eq!( + annotations.get("prometheus.io/path").map(String::as_str), + Some("/metrics") + ); + assert_eq!( + annotations.get("prometheus.io/port").map(String::as_str), + Some("7000") + ); + assert_eq!( + annotations.get("prometheus.io/scheme").map(String::as_str), + Some("http") + ); + assert_eq!( + annotations.get("prometheus.io/scrape").map(String::as_str), + Some("true") + ); + // ... and the scrape label. + assert_eq!( + service + .metadata + .labels + .as_ref() + .unwrap() + .get("prometheus.io/scrape") + .map(String::as_str), + Some("true") + ); + + // Legacy JMX port plus the Prometheus http port. + assert_eq!(port(&service, JMX_METRICS_PORT_NAME), 9505); + assert_eq!(port(&service, METRICS_PROVIDER_HTTP_PORT_NAME), 7000); + } +} diff --git a/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs b/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs index 21c104ef..98561ee2 100644 --- a/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs @@ -527,4 +527,257 @@ mod tests { assert_ne!(name, "my-log-config"); assert!(name.contains("simple-zookeeper"), "{name}"); } + + // --------------------------------------------------------------------------------------------- + // StatefulSet resource shapes (candidate #2 in unit-tests.md). + // + // These own what the kuttl smoke `13-assert` asserted on the StatefulSet spec/metadata: + // replicas, `podManagementPolicy`, `serviceName`, graceful-shutdown period, container resources + // (including the `podOverrides` merge), the `data`/`listener` PVC templates and the heap env var + // (which also covers candidate #7's `test_heap.sh`). The `status:` stanza (`readyReplicas`) is a + // runtime signal and stays in kuttl. + // --------------------------------------------------------------------------------------------- + + fn build_sts(yaml: &str, role_group: &str) -> StatefulSet { + let validated = validated_cluster(&minimal_zk(yaml)); + let rg_name = RoleGroupName::from_str(role_group).expect("valid role group name"); + let rg = validated.role_group_configs[&ZookeeperRole::Server][&rg_name].clone(); + build_server_rolegroup_statefulset(&validated, &rg_name, &rg).expect("statefulset builds") + } + + fn zookeeper_container( + sts: &StatefulSet, + ) -> &stackable_operator::k8s_openapi::api::core::v1::Container { + sts.spec + .as_ref() + .unwrap() + .template + .spec + .as_ref() + .unwrap() + .containers + .iter() + .find(|c| c.name == APP_NAME) + .expect("zookeeper container") + } + + #[test] + fn statefulset_shape_defaults() { + let sts = build_sts( + r#" + apiVersion: zookeeper.stackable.tech/v1alpha1 + kind: ZookeeperCluster + metadata: + name: simple-zookeeper + spec: + image: + productVersion: "3.9.5" + servers: + roleGroups: + default: + replicas: 2 + "#, + "default", + ); + + let spec = sts.spec.as_ref().unwrap(); + assert_eq!(spec.pod_management_policy.as_deref(), Some("Parallel")); + assert_eq!(spec.replicas, Some(2)); + assert_eq!( + spec.service_name.as_deref(), + Some("simple-zookeeper-server-default-headless") + ); + + let pod_spec = spec.template.spec.as_ref().unwrap(); + // Default graceful-shutdown timeout -> terminationGracePeriodSeconds. + assert_eq!(pod_spec.termination_grace_period_seconds, Some(120)); + assert_eq!( + pod_spec.service_account_name.as_deref(), + Some("simple-zookeeper-serviceaccount") + ); + + let labels = sts.metadata.labels.as_ref().unwrap(); + assert_eq!( + labels + .get("app.kubernetes.io/component") + .map(String::as_str), + Some("server") + ); + assert_eq!( + labels + .get("app.kubernetes.io/role-group") + .map(String::as_str), + Some("default") + ); + // The restarter label lets the restart controller pick up config changes. + assert_eq!( + labels + .get("restarter.stackable.tech/enabled") + .map(String::as_str), + Some("true") + ); + + let owner = &sts.metadata.owner_references.as_ref().unwrap()[0]; + assert_eq!(owner.kind, "ZookeeperCluster"); + assert_eq!(owner.name, "simple-zookeeper"); + assert_eq!(owner.controller, Some(true)); + } + + #[test] + fn statefulset_unset_replicas_leaves_spec_replicas_none() { + // An unset replica count leaves `.spec.replicas` unset so an HPA can manage it. + let sts = build_sts( + r#" + apiVersion: zookeeper.stackable.tech/v1alpha1 + kind: ZookeeperCluster + metadata: + name: simple-zookeeper + spec: + image: + productVersion: "3.9.5" + servers: + roleGroups: + default: {} + "#, + "default", + ); + assert_eq!(sts.spec.as_ref().unwrap().replicas, None); + } + + #[test] + fn pod_overrides_merge_into_container_resources() { + // The role sets the base resources; a rolegroup `podOverride` overrides only cpu, and the + // k8s deep-merge must keep the un-overridden memory. Mirrors the kuttl `secondary` group. + let sts = build_sts( + r#" + apiVersion: zookeeper.stackable.tech/v1alpha1 + kind: ZookeeperCluster + metadata: + name: simple-zookeeper + spec: + image: + productVersion: "3.9.5" + servers: + config: + resources: + cpu: + min: 250m + max: 500m + memory: + limit: 512Mi + roleGroups: + default: + replicas: 1 + podOverrides: + spec: + containers: + - name: zookeeper + resources: + requests: + cpu: 300m + limits: + cpu: 600m + "#, + "default", + ); + + let resources = zookeeper_container(&sts).resources.as_ref().unwrap(); + let requests = resources.requests.as_ref().unwrap(); + let limits = resources.limits.as_ref().unwrap(); + // cpu comes from the podOverride ... + assert_eq!(requests.get("cpu").unwrap().0, "300m"); + assert_eq!(limits.get("cpu").unwrap().0, "600m"); + // ... memory is untouched by the merge (Stackable sets memory request == limit). + assert_eq!(requests.get("memory").unwrap().0, "512Mi"); + assert_eq!(limits.get("memory").unwrap().0, "512Mi"); + } + + #[test] + fn volume_claim_templates_carry_data_and_listener_pvcs() { + let sts = build_sts( + r#" + apiVersion: zookeeper.stackable.tech/v1alpha1 + kind: ZookeeperCluster + metadata: + name: simple-zookeeper + spec: + image: + productVersion: "3.9.5" + servers: + roleGroups: + default: + replicas: 1 + config: + resources: + storage: + data: + capacity: 2Gi + "#, + "default", + ); + + let pvcs = sts + .spec + .as_ref() + .unwrap() + .volume_claim_templates + .as_ref() + .unwrap(); + + let data = pvcs + .iter() + .find(|p| p.metadata.name.as_deref() == Some("data")) + .expect("data PVC template"); + let storage = data + .spec + .as_ref() + .unwrap() + .resources + .as_ref() + .unwrap() + .requests + .as_ref() + .unwrap() + .get("storage") + .unwrap(); + assert_eq!(storage.0, "2Gi"); + + // The listener PVC template is always appended alongside the data PVC. + assert!( + pvcs.iter() + .any(|p| p.metadata.name.as_deref() == Some(LISTENER_VOLUME_NAME)), + "missing listener PVC template" + ); + } + + #[test] + fn statefulset_sets_zk_server_heap_env() { + // Candidate #7: the heap value computed in jvm.rs lands on the STS as `ZK_SERVER_HEAP`. + // The 512Mi default memory limit x 0.8 -> 409 MiB (pinned by the jvm.rs unit tests). + let sts = build_sts( + r#" + apiVersion: zookeeper.stackable.tech/v1alpha1 + kind: ZookeeperCluster + metadata: + name: simple-zookeeper + spec: + image: + productVersion: "3.9.5" + servers: + roleGroups: + default: + replicas: 1 + "#, + "default", + ); + + let heap = zookeeper_container(&sts) + .env + .as_ref() + .unwrap() + .iter() + .find(|e| e.name == "ZK_SERVER_HEAP") + .expect("ZK_SERVER_HEAP env var"); + assert_eq!(heap.value.as_deref(), Some("409")); + } } From 431752b91e076c58eba387c0d6772ed1bdeaa8bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCller?= Date: Thu, 16 Jul 2026 08:32:38 +0200 Subject: [PATCH 03/12] refactor: cover override from 11-assert and delete assert --- .../build/resource/statefulset.rs | 45 +++++++++++++++++++ tests/templates/kuttl/smoke/11-assert.yaml | 18 -------- 2 files changed, 45 insertions(+), 18 deletions(-) delete mode 100644 tests/templates/kuttl/smoke/11-assert.yaml diff --git a/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs b/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs index 98561ee2..01da11b9 100644 --- a/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs @@ -780,4 +780,49 @@ mod tests { .expect("ZK_SERVER_HEAP env var"); assert_eq!(heap.value.as_deref(), Some("409")); } + + #[test] + fn env_overrides_apply_with_role_group_precedence() { + // Candidate #4: `envOverrides` land on the zookeeper container, with the rolegroup value + // winning over the role value on conflict. Mirrors the kuttl smoke `11-assert` setup: + // COMMON_VAR is set at both levels (group wins), ROLE_VAR only at the role, GROUP_VAR only + // at the group. + let sts = build_sts( + r#" + apiVersion: zookeeper.stackable.tech/v1alpha1 + kind: ZookeeperCluster + metadata: + name: simple-zookeeper + spec: + image: + productVersion: "3.9.5" + servers: + envOverrides: + COMMON_VAR: role-value + ROLE_VAR: role-value + roleGroups: + primary: + replicas: 1 + envOverrides: + COMMON_VAR: group-value + GROUP_VAR: group-value + "#, + "primary", + ); + + let env = zookeeper_container(&sts).env.as_ref().unwrap(); + let env_value = |name: &str| { + env.iter() + .find(|e| e.name == name) + .unwrap_or_else(|| panic!("missing env var {name}")) + .value + .as_deref() + }; + + // Rolegroup value overrides the role value on conflict. + assert_eq!(env_value("COMMON_VAR"), Some("group-value")); + // Role-only and group-only overrides are both present. + assert_eq!(env_value("ROLE_VAR"), Some("role-value")); + assert_eq!(env_value("GROUP_VAR"), Some("group-value")); + } } diff --git a/tests/templates/kuttl/smoke/11-assert.yaml b/tests/templates/kuttl/smoke/11-assert.yaml deleted file mode 100644 index 1de89bcc..00000000 --- a/tests/templates/kuttl/smoke/11-assert.yaml +++ /dev/null @@ -1,18 +0,0 @@ ---- -apiVersion: kuttl.dev/v1beta1 -kind: TestAssert -timeout: 60 -commands: - # - # Test envOverrides - # - # configOverrides are covered by the ConfigMap data snapshot in 14-assert.yaml.j2; - # env overrides are not, because the operator emits MYID_OFFSET into the - # StatefulSet env array (kuttl matches arrays positionally), so a shape-based - # assertion in 13-assert.yaml.j2 wouldn't be stable. These targeted yq checks - # stay here. - # - - script: | - kubectl -n $NAMESPACE get sts test-zk-server-primary -o yaml | yq -e '.spec.template.spec.containers[] | select (.name == "zookeeper") | .env[] | select (.name == "COMMON_VAR" and .value == "group-value")' - kubectl -n $NAMESPACE get sts test-zk-server-primary -o yaml | yq -e '.spec.template.spec.containers[] | select (.name == "zookeeper") | .env[] | select (.name == "GROUP_VAR" and .value == "group-value")' - kubectl -n $NAMESPACE get sts test-zk-server-primary -o yaml | yq -e '.spec.template.spec.containers[] | select (.name == "zookeeper") | .env[] | select (.name == "ROLE_VAR" and .value == "role-value")' From b2dc4a47055729e6cd74b8de1c875d8f0eb99a78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCller?= Date: Thu, 16 Jul 2026 08:44:39 +0200 Subject: [PATCH 04/12] refactor: reduce tls scenarios in kuttl testing --- .../kuttl/smoke/10-install-zookeeper.yaml.j2 | 12 ++++------ tests/templates/kuttl/smoke/13-assert.yaml.j2 | 5 ++-- tests/templates/kuttl/smoke/14-assert.yaml.j2 | 5 ++-- tests/templates/kuttl/smoke/21-assert.yaml.j2 | 2 +- .../smoke/21-prepare-test-zookeeper.yaml.j2 | 2 +- tests/templates/kuttl/smoke/test_tls.sh.j2 | 2 +- tests/test-definition.yaml | 23 ++++++++----------- 7 files changed, 22 insertions(+), 29 deletions(-) diff --git a/tests/templates/kuttl/smoke/10-install-zookeeper.yaml.j2 b/tests/templates/kuttl/smoke/10-install-zookeeper.yaml.j2 index 8244aa7a..94bbbb5b 100644 --- a/tests/templates/kuttl/smoke/10-install-zookeeper.yaml.j2 +++ b/tests/templates/kuttl/smoke/10-install-zookeeper.yaml.j2 @@ -22,17 +22,15 @@ spec: {% endif %} pullPolicy: IfNotPresent clusterConfig: -{% if test_scenario['values']['use-server-tls'] == 'true' %} +{% if test_scenario['values']['use-tls'] == 'true' %} tls: serverSecretClass: zk-client-secret + authentication: + - authenticationClass: zk-client-auth-tls {% else %} tls: serverSecretClass: null {% endif %} -{% if test_scenario['values']['use-client-auth-tls'] == 'true' %} - authentication: - - authenticationClass: zk-client-auth-tls -{% endif %} {% if lookup('env', 'VECTOR_AGGREGATOR') %} vectorAggregatorConfigMapName: vector-aggregator-discovery {% endif %} @@ -85,7 +83,7 @@ spec: cpu: 300m limits: cpu: 600m -{% if test_scenario['values']['use-client-auth-tls'] == 'true' %} +{% if test_scenario['values']['use-tls'] == 'true' %} --- apiVersion: authentication.stackable.tech/v1alpha1 kind: AuthenticationClass @@ -109,7 +107,7 @@ spec: namespace: default autoGenerate: true {% endif %} -{% if test_scenario['values']['use-server-tls'] == 'true' %} +{% if test_scenario['values']['use-tls'] == 'true' %} --- apiVersion: secrets.stackable.tech/v1alpha1 kind: SecretClass diff --git a/tests/templates/kuttl/smoke/13-assert.yaml.j2 b/tests/templates/kuttl/smoke/13-assert.yaml.j2 index 1aa0c0c9..eaf4cbe8 100644 --- a/tests/templates/kuttl/smoke/13-assert.yaml.j2 +++ b/tests/templates/kuttl/smoke/13-assert.yaml.j2 @@ -1,7 +1,6 @@ {# Templating flags used throughout this file. #} -{% set use_client_tls = test_scenario['values']['use-server-tls'] == 'true' - or test_scenario['values']['use-client-auth-tls'] == 'true' %} -{% set use_client_auth = test_scenario['values']['use-client-auth-tls'] == 'true' %} +{% set use_client_tls = test_scenario['values']['use-tls'] == 'true' %} +{% set use_client_auth = test_scenario['values']['use-tls'] == 'true' %} {% set vector_enabled = lookup('env', 'VECTOR_AGGREGATOR') | length > 0 %} {% set zk_client_port = 2282 if use_client_tls else 2181 %} --- diff --git a/tests/templates/kuttl/smoke/14-assert.yaml.j2 b/tests/templates/kuttl/smoke/14-assert.yaml.j2 index 804d6a41..68a30f2e 100644 --- a/tests/templates/kuttl/smoke/14-assert.yaml.j2 +++ b/tests/templates/kuttl/smoke/14-assert.yaml.j2 @@ -1,7 +1,6 @@ {# Templating flags used throughout this file. #} -{% set use_client_tls = test_scenario['values']['use-server-tls'] == 'true' - or test_scenario['values']['use-client-auth-tls'] == 'true' %} -{% set use_client_auth = test_scenario['values']['use-client-auth-tls'] == 'true' %} +{% set use_client_tls = test_scenario['values']['use-tls'] == 'true' %} +{% set use_client_auth = test_scenario['values']['use-tls'] == 'true' %} {% set zk_client_port = 2282 if use_client_tls else 2181 %} --- # Snapshot the full `.data` of each operator-managed ConfigMap. diff --git a/tests/templates/kuttl/smoke/21-assert.yaml.j2 b/tests/templates/kuttl/smoke/21-assert.yaml.j2 index 5e2d0108..055fa5ac 100644 --- a/tests/templates/kuttl/smoke/21-assert.yaml.j2 +++ b/tests/templates/kuttl/smoke/21-assert.yaml.j2 @@ -6,6 +6,6 @@ metadata: commands: - script: kubectl exec -n $NAMESPACE zk-test-helper-0 -- python /tmp/test_zookeeper.py -n $NAMESPACE - script: kubectl exec -n $NAMESPACE test-zk-server-primary-0 --container='zookeeper' -- /tmp/test_heap.sh -{% if test_scenario['values']['use-client-auth-tls'] == 'true' or test_scenario['values']['use-server-tls'] == 'true' %} +{% if test_scenario['values']['use-tls'] == 'true' %} - script: kubectl exec -n $NAMESPACE test-zk-server-primary-0 --container='zookeeper' -- /tmp/test_tls.sh $NAMESPACE {% endif %} diff --git a/tests/templates/kuttl/smoke/21-prepare-test-zookeeper.yaml.j2 b/tests/templates/kuttl/smoke/21-prepare-test-zookeeper.yaml.j2 index 967d259e..1278085e 100644 --- a/tests/templates/kuttl/smoke/21-prepare-test-zookeeper.yaml.j2 +++ b/tests/templates/kuttl/smoke/21-prepare-test-zookeeper.yaml.j2 @@ -4,6 +4,6 @@ kind: TestStep commands: - script: kubectl cp -n $NAMESPACE ./test_zookeeper.py zk-test-helper-0:/tmp - script: kubectl cp -n $NAMESPACE ./test_heap.sh test-zk-server-primary-0:/tmp --container='zookeeper' -{% if test_scenario['values']['use-client-auth-tls'] == 'true' or test_scenario['values']['use-server-tls'] == 'true' %} +{% if test_scenario['values']['use-tls'] == 'true' %} - script: kubectl cp -n $NAMESPACE ./test_tls.sh test-zk-server-primary-0:/tmp --container='zookeeper' {% endif %} diff --git a/tests/templates/kuttl/smoke/test_tls.sh.j2 b/tests/templates/kuttl/smoke/test_tls.sh.j2 index cc7be7f8..eff0a862 100755 --- a/tests/templates/kuttl/smoke/test_tls.sh.j2 +++ b/tests/templates/kuttl/smoke/test_tls.sh.j2 @@ -3,7 +3,7 @@ NAMESPACE=$1 -{% if test_scenario['values']['use-client-auth-tls'] == 'true' or test_scenario['values']['use-server-tls'] == 'true' %} +{% if test_scenario['values']['use-tls'] == 'true' %} SERVER="test-zk-server.${NAMESPACE}.svc.cluster.local:2282" {% else %} SERVER="test-zk-server.${NAMESPACE}.svc.cluster.local:2181" diff --git a/tests/test-definition.yaml b/tests/test-definition.yaml index 2bb193a5..22d15874 100644 --- a/tests/test-definition.yaml +++ b/tests/test-definition.yaml @@ -11,11 +11,13 @@ dimensions: - 3.9.5 # To use a custom image, add a comma and the full name after the product version # - x.x.x,oci.stackable.tech/sdp/zookeeper:x.x.x-stackable0.0.0-dev - - name: use-server-tls - values: - - "true" - - "false" - - name: use-client-auth-tls + # A single toggle for the runtime smoke test: "true" enables server TLS *and* + # client-auth (mutual) TLS together, "false" disables TLS entirely. The full + # use-server-tls x use-client-auth-tls config-text matrix (server-only TLS, the + # auth-forces-TLS cross term, port/ssl.* lines) is covered exhaustively by the + # Rust unit tests in zk_controller.rs, so the runtime smoke only needs one TLS + # run (exercising test_tls.sh's mutual handshake) and one non-TLS run. + - name: use-tls values: - "true" - "false" @@ -26,8 +28,7 @@ tests: - name: smoke dimensions: - zookeeper - - use-server-tls - - use-client-auth-tls + - use-tls - openshift - name: delete-rolegroup dimensions: @@ -51,9 +52,7 @@ suites: - dimensions: - name: zookeeper expr: last - - name: use-server-tls - expr: "true" - - name: use-client-auth-tls + - name: use-tls expr: "true" - name: smoke-latest select: @@ -68,9 +67,7 @@ suites: - dimensions: - name: zookeeper expr: last - - name: use-server-tls - expr: "true" - - name: use-client-auth-tls + - name: use-tls expr: "true" - name: openshift expr: "true" From 590a4b25c36e591d8f0a960933ef0415db095146 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCller?= Date: Thu, 16 Jul 2026 08:54:27 +0200 Subject: [PATCH 05/12] refactor: reduce 14-assert and 13-assert --- .../build/resource/statefulset.rs | 94 +++++ tests/templates/kuttl/smoke/13-assert.yaml.j2 | 329 +----------------- tests/templates/kuttl/smoke/14-assert.yaml.j2 | 30 +- 3 files changed, 116 insertions(+), 337 deletions(-) diff --git a/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs b/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs index 01da11b9..87123255 100644 --- a/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs @@ -781,6 +781,100 @@ mod tests { assert_eq!(heap.value.as_deref(), Some("409")); } + #[test] + fn vector_agent_adds_vector_container_to_statefulset() { + // Enabling the Vector agent wires a `vector` sidecar onto the StatefulSet, mounting the + // `config` (for `vector.yaml`) and `log` volumes. The env vars the sidecar carries are set + // by the upstream `vector_container` helper, so this only pins the wiring seam this operator + // owns: that the sidecar is added at all when the agent is enabled. Without the agent, no + // such container exists. + let sts = build_sts( + 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: 1 + config: + logging: + enableVectorAgent: true + "#, + "default", + ); + + let vector = sts + .spec + .as_ref() + .unwrap() + .template + .spec + .as_ref() + .unwrap() + .containers + .iter() + .find(|c| c.name == VECTOR_CONTAINER_NAME.as_ref()) + .expect("vector container"); + + let mount_names: Vec<&str> = vector + .volume_mounts + .as_ref() + .unwrap() + .iter() + .map(|m| m.name.as_str()) + .collect(); + assert!( + mount_names.contains(&CONFIG_VOLUME_NAME.as_ref()), + "vector container missing `config` volume mount: {mount_names:?}" + ); + assert!( + mount_names.contains(&LOG_VOLUME_NAME.as_ref()), + "vector container missing `log` volume mount: {mount_names:?}" + ); + } + + #[test] + fn no_vector_container_without_agent() { + // Sanity counterpart: with the agent disabled (the default), the StatefulSet carries no + // `vector` sidecar. + let sts = build_sts( + r#" + apiVersion: zookeeper.stackable.tech/v1alpha1 + kind: ZookeeperCluster + metadata: + name: simple-zookeeper + spec: + image: + productVersion: "3.9.5" + servers: + roleGroups: + default: + replicas: 1 + "#, + "default", + ); + + let has_vector = sts + .spec + .as_ref() + .unwrap() + .template + .spec + .as_ref() + .unwrap() + .containers + .iter() + .any(|c| c.name == VECTOR_CONTAINER_NAME.as_ref()); + assert!(!has_vector, "unexpected vector container without the agent"); + } + #[test] fn env_overrides_apply_with_role_group_precedence() { // Candidate #4: `envOverrides` land on the zookeeper container, with the rolegroup value diff --git a/tests/templates/kuttl/smoke/13-assert.yaml.j2 b/tests/templates/kuttl/smoke/13-assert.yaml.j2 index eaf4cbe8..408a5811 100644 --- a/tests/templates/kuttl/smoke/13-assert.yaml.j2 +++ b/tests/templates/kuttl/smoke/13-assert.yaml.j2 @@ -1,16 +1,18 @@ -{# Templating flags used throughout this file. #} -{% set use_client_tls = test_scenario['values']['use-tls'] == 'true' %} -{% set use_client_auth = test_scenario['values']['use-tls'] == 'true' %} -{% set vector_enabled = lookup('env', 'VECTOR_AGGREGATOR') | length > 0 %} -{% set zk_client_port = 2282 if use_client_tls else 2181 %} --- -# Declarative shape assertions for every operator-managed resource in the smoke -# test except ConfigMap *.data* (covered in 14-assert.yaml.j2). +# Runtime-only assertions for the smoke cluster. The full spec/metadata *shapes* +# of the operator-built resources (StatefulSets, headless/metrics Services, PDB, +# PVC templates) are pinned by pure-builder unit tests in +# rust/operator-binary/src/zk_controller/build/resource/{statefulset,service,pdb}.rs, +# so they are not re-diffed here. What stays is what a unit test cannot prove: # -# kuttl performs subset matching: any field omitted here is not checked, and -# the live object may carry additional keys/labels. We therefore omit fields -# that are random per install (uids, resourceVersion, clusterIP, and the -# `listener.stackable.tech/mnt.` selector on the cluster-internal Service). +# * the StatefulSets actually reconciled to a running state (status.readyReplicas) +# * the data PVCs were provisioned and bound (status.phase: Bound) +# * the PDB this operator builds is actually applied (bare existence) +# * the resources this operator does NOT build itself exist at runtime: +# - the cluster-internal `test-zk-server` Service (created by the +# listener-operator from the Listener object) +# - the ServiceAccount + RoleBinding (created by operator-rs +# `build_rbac_resources`, not this repo's build code) apiVersion: kuttl.dev/v1beta1 kind: TestAssert timeout: 60 @@ -19,48 +21,6 @@ apiVersion: apps/v1 kind: StatefulSet metadata: name: test-zk-server-primary - labels: - app.kubernetes.io/component: server - app.kubernetes.io/instance: test-zk - app.kubernetes.io/managed-by: zookeeper.stackable.tech_zookeepercluster - app.kubernetes.io/name: zookeeper - app.kubernetes.io/role-group: primary - restarter.stackable.tech/enabled: "true" - stackable.tech/vendor: Stackable - ownerReferences: - - apiVersion: zookeeper.stackable.tech/v1alpha1 - controller: true - kind: ZookeeperCluster - name: test-zk -spec: - podManagementPolicy: Parallel - replicas: 2 - serviceName: test-zk-server-primary-headless - template: - metadata: - labels: - app.kubernetes.io/component: server - app.kubernetes.io/instance: test-zk - app.kubernetes.io/managed-by: zookeeper.stackable.tech_zookeepercluster - app.kubernetes.io/name: zookeeper - app.kubernetes.io/role-group: primary - stackable.tech/vendor: Stackable - spec: - serviceAccount: test-zk-serviceaccount - serviceAccountName: test-zk-serviceaccount - terminationGracePeriodSeconds: 120 - containers: - - name: zookeeper - resources: - limits: - cpu: 500m - memory: 512Mi - requests: - cpu: 250m - memory: 512Mi -{% if vector_enabled %} - - name: vector -{% endif %} status: readyReplicas: 2 replicas: 2 @@ -69,281 +29,38 @@ apiVersion: apps/v1 kind: StatefulSet metadata: name: test-zk-server-secondary - labels: - app.kubernetes.io/component: server - app.kubernetes.io/instance: test-zk - app.kubernetes.io/managed-by: zookeeper.stackable.tech_zookeepercluster - app.kubernetes.io/name: zookeeper - app.kubernetes.io/role-group: secondary - restarter.stackable.tech/enabled: "true" - stackable.tech/vendor: Stackable - ownerReferences: - - apiVersion: zookeeper.stackable.tech/v1alpha1 - controller: true - kind: ZookeeperCluster - name: test-zk -spec: - podManagementPolicy: Parallel - replicas: 1 - serviceName: test-zk-server-secondary-headless - template: - spec: - serviceAccount: test-zk-serviceaccount - serviceAccountName: test-zk-serviceaccount - terminationGracePeriodSeconds: 120 - containers: - - name: zookeeper - resources: - limits: - cpu: 600m # From podOverrides - memory: 512Mi - requests: - cpu: 300m # From podOverrides - memory: 512Mi -{% if vector_enabled %} - - name: vector -{% endif %} status: readyReplicas: 1 replicas: 1 --- -# Cluster-internal listener Service. `selector` is intentionally not asserted: -# it carries a per-install `listener.stackable.tech/mnt.` key. -apiVersion: v1 -kind: Service +# Built by this operator; its shape is unit-tested in pdb.rs. Kept here as a bare +# existence check so the smoke test still proves the PDB is actually applied. +apiVersion: policy/v1 +kind: PodDisruptionBudget metadata: name: test-zk-server - labels: - app.kubernetes.io/component: server - app.kubernetes.io/instance: test-zk-server - app.kubernetes.io/managed-by: listeners.stackable.tech_listener - app.kubernetes.io/name: listener - stackable.tech/vendor: Stackable - ownerReferences: - - apiVersion: listeners.stackable.tech/v1alpha1 - controller: true - kind: Listener - name: test-zk-server -spec: - type: ClusterIP - ports: - - name: zk - port: {{ zk_client_port }} - protocol: TCP - targetPort: {{ zk_client_port }} --- +# Created at runtime by the listener-operator, not by this operator's build code. apiVersion: v1 kind: Service -metadata: - name: test-zk-server-primary-headless - labels: - app.kubernetes.io/component: server - app.kubernetes.io/instance: test-zk - app.kubernetes.io/managed-by: zookeeper.stackable.tech_zookeepercluster - app.kubernetes.io/name: zookeeper - app.kubernetes.io/role-group: primary - stackable.tech/vendor: Stackable - ownerReferences: - - apiVersion: zookeeper.stackable.tech/v1alpha1 - controller: true - kind: ZookeeperCluster - name: test-zk -spec: - clusterIP: None - publishNotReadyAddresses: true - selector: - app.kubernetes.io/component: server - app.kubernetes.io/instance: test-zk - app.kubernetes.io/name: zookeeper - app.kubernetes.io/role-group: primary - ports: - - name: zk-leader - port: 2888 - protocol: TCP - targetPort: 2888 - - name: zk-election - port: 3888 - protocol: TCP - targetPort: 3888 ---- -apiVersion: v1 -kind: Service -metadata: - name: test-zk-server-primary-metrics - annotations: - prometheus.io/path: /metrics - prometheus.io/port: "7000" - prometheus.io/scheme: http - prometheus.io/scrape: "true" - labels: - app.kubernetes.io/component: server - app.kubernetes.io/instance: test-zk - app.kubernetes.io/managed-by: zookeeper.stackable.tech_zookeepercluster - app.kubernetes.io/name: zookeeper - app.kubernetes.io/role-group: primary - prometheus.io/scrape: "true" - stackable.tech/vendor: Stackable - ownerReferences: - - apiVersion: zookeeper.stackable.tech/v1alpha1 - controller: true - kind: ZookeeperCluster - name: test-zk -spec: - clusterIP: None - publishNotReadyAddresses: true - selector: - app.kubernetes.io/component: server - app.kubernetes.io/instance: test-zk - app.kubernetes.io/name: zookeeper - app.kubernetes.io/role-group: primary - ports: - - name: jmx-metrics - port: 9505 - protocol: TCP - targetPort: 9505 - - name: metrics - port: 7000 - protocol: TCP - targetPort: 7000 ---- -apiVersion: v1 -kind: Service -metadata: - name: test-zk-server-secondary-headless - labels: - app.kubernetes.io/component: server - app.kubernetes.io/instance: test-zk - app.kubernetes.io/managed-by: zookeeper.stackable.tech_zookeepercluster - app.kubernetes.io/name: zookeeper - app.kubernetes.io/role-group: secondary - stackable.tech/vendor: Stackable - ownerReferences: - - apiVersion: zookeeper.stackable.tech/v1alpha1 - controller: true - kind: ZookeeperCluster - name: test-zk -spec: - clusterIP: None - publishNotReadyAddresses: true - selector: - app.kubernetes.io/component: server - app.kubernetes.io/instance: test-zk - app.kubernetes.io/name: zookeeper - app.kubernetes.io/role-group: secondary - ports: - - name: zk-leader - port: 2888 - protocol: TCP - targetPort: 2888 - - name: zk-election - port: 3888 - protocol: TCP - targetPort: 3888 ---- -apiVersion: v1 -kind: Service -metadata: - name: test-zk-server-secondary-metrics - annotations: - prometheus.io/path: /metrics - prometheus.io/port: "7000" - prometheus.io/scheme: http - prometheus.io/scrape: "true" - labels: - app.kubernetes.io/component: server - app.kubernetes.io/instance: test-zk - app.kubernetes.io/managed-by: zookeeper.stackable.tech_zookeepercluster - app.kubernetes.io/name: zookeeper - app.kubernetes.io/role-group: secondary - prometheus.io/scrape: "true" - stackable.tech/vendor: Stackable - ownerReferences: - - apiVersion: zookeeper.stackable.tech/v1alpha1 - controller: true - kind: ZookeeperCluster - name: test-zk -spec: - clusterIP: None - publishNotReadyAddresses: true - selector: - app.kubernetes.io/component: server - app.kubernetes.io/instance: test-zk - app.kubernetes.io/name: zookeeper - app.kubernetes.io/role-group: secondary - ports: - - name: jmx-metrics - port: 9505 - protocol: TCP - targetPort: 9505 - - name: metrics - port: 7000 - protocol: TCP - targetPort: 7000 ---- -apiVersion: policy/v1 -kind: PodDisruptionBudget metadata: name: test-zk-server - labels: - app.kubernetes.io/component: server - app.kubernetes.io/instance: test-zk - app.kubernetes.io/managed-by: zookeeper.stackable.tech_zookeepercluster - app.kubernetes.io/name: zookeeper - ownerReferences: - - apiVersion: zookeeper.stackable.tech/v1alpha1 - controller: true - kind: ZookeeperCluster - name: test-zk -spec: - maxUnavailable: 1 - selector: - matchLabels: - app.kubernetes.io/component: server - app.kubernetes.io/instance: test-zk - app.kubernetes.io/name: zookeeper -status: - currentHealthy: 3 - disruptionsAllowed: 1 - expectedPods: 3 --- +# Created by operator-rs `build_rbac_resources`, not by this repo. apiVersion: v1 kind: ServiceAccount metadata: name: test-zk-serviceaccount - labels: - app.kubernetes.io/instance: test-zk - app.kubernetes.io/managed-by: zookeeper.stackable.tech_zookeepercluster - app.kubernetes.io/name: zookeeper - ownerReferences: - - apiVersion: zookeeper.stackable.tech/v1alpha1 - controller: true - kind: ZookeeperCluster - name: test-zk --- apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding metadata: name: test-zk-rolebinding - labels: - app.kubernetes.io/instance: test-zk - app.kubernetes.io/managed-by: zookeeper.stackable.tech_zookeepercluster - app.kubernetes.io/name: zookeeper -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: ClusterRole - name: zookeeper-clusterrole -subjects: - - kind: ServiceAccount - name: test-zk-serviceaccount --- apiVersion: v1 kind: PersistentVolumeClaim metadata: name: data-test-zk-server-primary-0 -spec: - resources: - requests: - storage: 1Gi status: phase: Bound --- @@ -351,10 +68,6 @@ apiVersion: v1 kind: PersistentVolumeClaim metadata: name: data-test-zk-server-primary-1 -spec: - resources: - requests: - storage: 1Gi status: phase: Bound --- @@ -362,9 +75,5 @@ apiVersion: v1 kind: PersistentVolumeClaim metadata: name: data-test-zk-server-secondary-0 -spec: - resources: - requests: - storage: 2Gi status: phase: Bound diff --git a/tests/templates/kuttl/smoke/14-assert.yaml.j2 b/tests/templates/kuttl/smoke/14-assert.yaml.j2 index 68a30f2e..a534f986 100644 --- a/tests/templates/kuttl/smoke/14-assert.yaml.j2 +++ b/tests/templates/kuttl/smoke/14-assert.yaml.j2 @@ -21,33 +21,9 @@ apiVersion: kuttl.dev/v1beta1 kind: TestAssert timeout: 60 commands: -{% if lookup('env', 'VECTOR_AGGREGATOR') %} - # Verify the Vector agent container's environment is wired up by the v2 `vector_container`: - # cluster/role/role-group identity, log/data dirs, the config path and the aggregator address. - - script: | - check_vector_env() { - sts="$1" - role_group="$2" - manifest=$(kubectl -n "$NAMESPACE" get sts "$sts" -o yaml) - check() { - printf '%s\n' "$manifest" \ - | yq -e ".spec.template.spec.containers[] | select(.name == \"vector\") | .env[] | select(.name == \"$1\") | $2" >/dev/null \ - || { echo "ERROR: $sts vector container env $1 did not match expectation"; exit 1; } - } - check CLUSTER_NAME '.value == "test-zk"' - check ROLE_NAME '.value == "server"' - check ROLE_GROUP_NAME ".value == \"$role_group\"" - check LOG_DIR '.value == "/stackable/log"' - check DATA_DIR '.value == "/stackable/log/_vector-state"' - check VECTOR_CONFIG_YAML '.value == "/stackable/config/vector.yaml"' - check VECTOR_FILE_LOG_LEVEL '.value == "info"' - check VECTOR_LOG '.value == "info"' - check NAMESPACE '.valueFrom.fieldRef.fieldPath == "metadata.namespace"' - check VECTOR_AGGREGATOR_ADDRESS '.valueFrom.configMapKeyRef | select(.name == "vector-aggregator-discovery" and .key == "ADDRESS")' - } - check_vector_env test-zk-server-primary primary - check_vector_env test-zk-server-secondary secondary -{% endif %} + # The Vector sidecar's env wiring is set by the upstream `vector_container` helper and its + # presence on the StatefulSet is unit-tested (statefulset.rs + # `vector_agent_adds_vector_container_to_statefulset`), so no runtime env assertion is needed here. - script: | expected=$(cat <<'YAMLEOF' | sed "s|__NAMESPACE__|$NAMESPACE|g" | yq -o=json ZOOKEEPER: test-zk-server.__NAMESPACE__.svc.cluster.local:{{ zk_client_port }} From 0a68d92cee9aa4c27e548983017848931a4a4827 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCller?= Date: Thu, 16 Jul 2026 09:02:19 +0200 Subject: [PATCH 06/12] refactor: replace 14-assert cm discovery checks --- .../zk_controller/build/resource/discovery.rs | 158 +++++++++++++++++- .../src/znode_controller/validate.rs | 55 ++++++ tests/templates/kuttl/smoke/14-assert.yaml.j2 | 64 +++---- 3 files changed, 237 insertions(+), 40 deletions(-) diff --git a/rust/operator-binary/src/zk_controller/build/resource/discovery.rs b/rust/operator-binary/src/zk_controller/build/resource/discovery.rs index 550bbf10..fc9fab6b 100644 --- a/rust/operator-binary/src/zk_controller/build/resource/discovery.rs +++ b/rust/operator-binary/src/zk_controller/build/resource/discovery.rs @@ -225,6 +225,61 @@ mod tests { }; use super::*; + use crate::{ + zk_controller::{ + ZK_CONTROLLER_NAME, + test_support::{minimal_zk, validated_cluster}, + }, + znode_controller::{ + ZNODE_CONTROLLER_NAME, + validate::test_support::{minimal_znode, validated_znode}, + }, + }; + + /// A `ZookeeperCluster` fixture. `test_support`'s auth-free `new_for_tests()` still enables + /// server TLS, so this resolves to the secure client port (2282). + const TLS_ZK: &str = r#" + apiVersion: zookeeper.stackable.tech/v1alpha1 + kind: ZookeeperCluster + metadata: + name: test-zk + spec: + image: + productVersion: "3.9.5" + servers: + roleGroups: + default: + replicas: 3 + "#; + + /// Same as [`TLS_ZK`] but with `serverSecretClass: null`, disabling server TLS so the insecure + /// client port (2181) is used. + const NON_TLS_ZK: &str = 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: 3 + "#; + + const ZNODE: &str = r#" + apiVersion: zookeeper.stackable.tech/v1alpha1 + kind: ZookeeperZnode + metadata: + name: test-znode + spec: + clusterRef: + name: test-zk + "#; fn listener(ingress_addresses: Option>) -> Listener { Listener { @@ -241,14 +296,26 @@ mod tests { } } - fn ingress(port: i32) -> ListenerIngress { + fn ingress_at(address: &str, port: i32) -> ListenerIngress { ListenerIngress { - address: "node-0".to_owned(), + address: address.to_owned(), address_type: AddressType::Hostname, ports: BTreeMap::from([(ZOOKEEPER_SERVER_PORT_NAME.to_owned(), port)]), } } + fn ingress(port: i32) -> ListenerIngress { + ingress_at("node-0", port) + } + + /// Pulls a `.data` key out of a built ConfigMap. + fn data<'a>(cm: &'a ConfigMap, key: &str) -> &'a str { + cm.data + .as_ref() + .and_then(|d| d.get(key)) + .unwrap_or_else(|| panic!("discovery ConfigMap missing key {key}")) + } + #[test] fn listener_addresses_returns_host_port_pairs() { let listener = listener(Some(vec![ingress(2181)])); @@ -285,4 +352,91 @@ mod tests { Err(Error::InvalidPort { .. }) )); } + + #[test] + fn cluster_discovery_cm_data_tls() { + // TLS cluster -> secure client port 2282. The listener exposes the same port, as it does at + // runtime. No chroot, so `ZOOKEEPER` equals `ZOOKEEPER_HOSTS` and `ZOOKEEPER_CHROOT` is `/`. + let validated = validated_cluster(&minimal_zk(TLS_ZK)); + let cm = build_discovery_configmap( + &validated, + ZK_CONTROLLER_NAME, + listener(Some(vec![ingress(2282)])), + ) + .expect("discovery CM builds"); + + assert_eq!(data(&cm, "ZOOKEEPER"), "node-0:2282"); + assert_eq!(data(&cm, "ZOOKEEPER_HOSTS"), "node-0:2282"); + assert_eq!(data(&cm, "ZOOKEEPER_CLIENT_PORT"), "2282"); + assert_eq!(data(&cm, "ZOOKEEPER_CHROOT"), "/"); + } + + #[test] + fn cluster_discovery_cm_data_non_tls() { + // Server TLS disabled -> insecure client port 2181. + let validated = validated_cluster(&minimal_zk(NON_TLS_ZK)); + let cm = build_discovery_configmap( + &validated, + ZK_CONTROLLER_NAME, + listener(Some(vec![ingress(2181)])), + ) + .expect("discovery CM builds"); + + assert_eq!(data(&cm, "ZOOKEEPER"), "node-0:2181"); + assert_eq!(data(&cm, "ZOOKEEPER_HOSTS"), "node-0:2181"); + assert_eq!(data(&cm, "ZOOKEEPER_CLIENT_PORT"), "2181"); + assert_eq!(data(&cm, "ZOOKEEPER_CHROOT"), "/"); + } + + #[test] + fn cluster_discovery_cm_joins_multiple_hosts_sorted() { + // Multiple listener ingress addresses are joined into one comma-separated connection string, + // ordered by the `BTreeSet` in `listener_addresses` (i.e. sorted). + let validated = validated_cluster(&minimal_zk(TLS_ZK)); + let cm = build_discovery_configmap( + &validated, + ZK_CONTROLLER_NAME, + listener(Some(vec![ + ingress_at("node-1", 2282), + ingress_at("node-0", 2282), + ])), + ) + .expect("discovery CM builds"); + + assert_eq!(data(&cm, "ZOOKEEPER"), "node-0:2282,node-1:2282"); + assert_eq!(data(&cm, "ZOOKEEPER_HOSTS"), "node-0:2282,node-1:2282"); + } + + #[test] + fn znode_discovery_cm_appends_chroot_to_connection_string_only() { + // The chroot is appended to `ZOOKEEPER` (the merged Java client string) but NOT to + // `ZOOKEEPER_HOSTS` (kept separate for clients that don't understand the merged format). + let validated = validated_znode(&minimal_znode(ZNODE), TLS_ZK); + let cm = build_znode_discovery_configmap( + &validated, + ZNODE_CONTROLLER_NAME, + listener(Some(vec![ingress(2282)])), + "/znode-abc", + ) + .expect("znode discovery CM builds"); + + assert_eq!(data(&cm, "ZOOKEEPER"), "node-0:2282/znode-abc"); + assert_eq!(data(&cm, "ZOOKEEPER_HOSTS"), "node-0:2282"); + assert_eq!(data(&cm, "ZOOKEEPER_CLIENT_PORT"), "2282"); + assert_eq!(data(&cm, "ZOOKEEPER_CHROOT"), "/znode-abc"); + } + + #[test] + fn znode_discovery_cm_rejects_relative_chroot() { + let validated = validated_znode(&minimal_znode(ZNODE), TLS_ZK); + assert!(matches!( + build_znode_discovery_configmap( + &validated, + ZNODE_CONTROLLER_NAME, + listener(Some(vec![ingress(2282)])), + "znode-abc", + ), + Err(Error::RelativeChroot { .. }) + )); + } } diff --git a/rust/operator-binary/src/znode_controller/validate.rs b/rust/operator-binary/src/znode_controller/validate.rs index 899b6cdb..06a7b5d1 100644 --- a/rust/operator-binary/src/znode_controller/validate.rs +++ b/rust/operator-binary/src/znode_controller/validate.rs @@ -200,3 +200,58 @@ pub fn validate( object_overrides: znode.spec.object_overrides.clone(), }) } + +/// Shared helpers for building validated test znodes from minimal YAML fixtures. +/// +/// Mirrors `zk_controller::test_support`: rather than hand-constructing a [`ValidatedZnode`] +/// (whose `metadata` is private), tests run the real [`validate`] step so the wiring under test +/// (product version, `zookeeper_security`, identity) matches production. +#[cfg(test)] +pub(crate) mod test_support { + use stackable_operator::cli::OperatorEnvironmentOptions; + + use super::{ValidatedZnode, validate}; + use crate::{ + crd::{authentication::DereferencedAuthenticationClasses, v1alpha1}, + zk_controller::test_support::minimal_zk, + znode_controller::dereference::DereferencedObjects, + }; + + /// Parses a minimal `ZookeeperZnode` test fixture, defaulting `namespace`/`uid` so the + /// validate step can build a [`ValidatedZnode`]. + pub fn minimal_znode(yaml: &str) -> v1alpha1::ZookeeperZnode { + let mut znode: v1alpha1::ZookeeperZnode = + serde_yaml::from_str(yaml).expect("invalid test ZookeeperZnode YAML"); + znode + .metadata + .namespace + .get_or_insert_with(|| "default".to_owned()); + znode + .metadata + .uid + .get_or_insert_with(|| "5f1d9a2e-3c4b-4a1e-9b7d-2e6f8c0a1b3c".to_owned()); + znode + } + + fn operator_environment() -> OperatorEnvironmentOptions { + OperatorEnvironmentOptions { + operator_namespace: "stackable-operators".to_owned(), + operator_service_name: "zookeeper-operator".to_owned(), + image_repository: "oci.example.org".to_owned(), + } + } + + /// Runs the real znode validate step against a minimal znode fixture and the given + /// `ZookeeperCluster` YAML (which controls product version and `zookeeper_security`). + pub fn validated_znode(znode: &v1alpha1::ZookeeperZnode, zk_yaml: &str) -> ValidatedZnode { + validate( + znode, + &DereferencedObjects { + zk: minimal_zk(zk_yaml), + authentication_classes: DereferencedAuthenticationClasses::new_for_tests(), + }, + &operator_environment(), + ) + .expect("znode validate should succeed for the test fixture") + } +} diff --git a/tests/templates/kuttl/smoke/14-assert.yaml.j2 b/tests/templates/kuttl/smoke/14-assert.yaml.j2 index a534f986..51df9033 100644 --- a/tests/templates/kuttl/smoke/14-assert.yaml.j2 +++ b/tests/templates/kuttl/smoke/14-assert.yaml.j2 @@ -24,45 +24,33 @@ commands: # The Vector sidecar's env wiring is set by the upstream `vector_container` helper and its # presence on the StatefulSet is unit-tested (statefulset.rs # `vector_agent_adds_vector_container_to_statefulset`), so no runtime env assertion is needed here. + # + # Discovery ConfigMap content (`ZOOKEEPER`/`ZOOKEEPER_HOSTS`/`ZOOKEEPER_CHROOT`/ + # `ZOOKEEPER_CLIENT_PORT`, incl. the znode chroot suffix and the secure-vs-insecure client port) + # is unit-tested by `build_(znode_)discovery_configmap` (discovery.rs). The checks below only guard + # what a unit test cannot: that the CMs actually reconciled with the real service-name/namespace + # wiring and carry non-empty values. - script: | - expected=$(cat <<'YAMLEOF' | sed "s|__NAMESPACE__|$NAMESPACE|g" | yq -o=json - ZOOKEEPER: test-zk-server.__NAMESPACE__.svc.cluster.local:{{ zk_client_port }} - ZOOKEEPER_CHROOT: / - ZOOKEEPER_CLIENT_PORT: "{{ zk_client_port }}" - ZOOKEEPER_HOSTS: test-zk-server.__NAMESPACE__.svc.cluster.local:{{ zk_client_port }} - YAMLEOF - ) - actual=$(kubectl -n $NAMESPACE get cm test-zk -o yaml | yq -o=json '.data') - expected_file=$(mktemp) && actual_file=$(mktemp) - printf '%s\n' "$expected" > "$expected_file" - printf '%s\n' "$actual" > "$actual_file" - if ! diff_out=$(diff -u "$expected_file" "$actual_file"); then - echo "ERROR: ConfigMap test-zk data drifted from snapshot." - printf '%s\n' "$diff_out" - rm -f "$expected_file" "$actual_file" - exit 1 - fi - rm -f "$expected_file" "$actual_file" - - script: | - ZNODE_UID=$(kubectl -n $NAMESPACE get zookeeperznode test-znode -o jsonpath='{.metadata.uid}') - expected=$(cat <<'YAMLEOF' | sed -e "s|__NAMESPACE__|$NAMESPACE|g" -e "s|__ZNODE_UID__|$ZNODE_UID|g" | yq -o=json - ZOOKEEPER: test-zk-server.__NAMESPACE__.svc.cluster.local:{{ zk_client_port }}/znode-__ZNODE_UID__ - ZOOKEEPER_CHROOT: /znode-__ZNODE_UID__ - ZOOKEEPER_CLIENT_PORT: "{{ zk_client_port }}" - ZOOKEEPER_HOSTS: test-zk-server.__NAMESPACE__.svc.cluster.local:{{ zk_client_port }} - YAMLEOF - ) - actual=$(kubectl -n $NAMESPACE get cm test-znode -o yaml | yq -o=json '.data') - expected_file=$(mktemp) && actual_file=$(mktemp) - printf '%s\n' "$expected" > "$expected_file" - printf '%s\n' "$actual" > "$actual_file" - if ! diff_out=$(diff -u "$expected_file" "$actual_file"); then - echo "ERROR: ConfigMap test-znode data drifted from snapshot." - printf '%s\n' "$diff_out" - rm -f "$expected_file" "$actual_file" - exit 1 - fi - rm -f "$expected_file" "$actual_file" + for cm in test-zk test-znode; do + data=$(kubectl -n $NAMESPACE get cm "$cm" -o yaml | yq -o=json '.data') + for key in ZOOKEEPER ZOOKEEPER_HOSTS ZOOKEEPER_CLIENT_PORT ZOOKEEPER_CHROOT; do + value=$(printf '%s' "$data" | yq -r ".$key // \"\"") + if [ -z "$value" ]; then + echo "ERROR: discovery ConfigMap $cm is missing/empty key $key." + printf '%s\n' "$data" + exit 1 + fi + done + # The connection string must point at the cluster-internal Service in this namespace. + host=$(printf '%s' "$data" | yq -r '.ZOOKEEPER_HOSTS') + case "$host" in + test-zk-server.$NAMESPACE.svc.cluster.local:*) ;; + *) + echo "ERROR: discovery ConfigMap $cm ZOOKEEPER_HOSTS=$host does not target the expected Service." + exit 1 + ;; + esac + done - script: | expected=$(cat <<'YAMLEOF' | sed "s|__NAMESPACE__|$NAMESPACE|g" | yq -o=json logback.xml: | From 4c89124b8e01eb45d64dba4963642c3bbce24080 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCller?= Date: Thu, 16 Jul 2026 09:31:36 +0200 Subject: [PATCH 07/12] cleaning up --- rust/operator-binary/src/zk_controller.rs | 153 +++++ tests/templates/kuttl/smoke/14-assert.yaml.j2 | 598 +----------------- tests/templates/kuttl/smoke/21-assert.yaml.j2 | 1 - .../smoke/21-prepare-test-zookeeper.yaml.j2 | 1 - tests/templates/kuttl/smoke/test_heap.sh | 16 - 5 files changed, 169 insertions(+), 600 deletions(-) delete mode 100755 tests/templates/kuttl/smoke/test_heap.sh diff --git a/rust/operator-binary/src/zk_controller.rs b/rust/operator-binary/src/zk_controller.rs index 98ed51b3..3136491d 100644 --- a/rust/operator-binary/src/zk_controller.rs +++ b/rust/operator-binary/src/zk_controller.rs @@ -756,6 +756,159 @@ mod tests { assert!(cm.contains_key("vector.yaml")); } + #[test] + fn test_vector_config_absent_when_agent_disabled() { + // Default logging has the Vector agent disabled, so no `vector.yaml` is added to the + // ConfigMap. Pins the negative branch alongside `test_vector_agent_adds_vector_config`. + 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 + "#; + let cm = build_config_map(zookeeper_yaml).data.unwrap(); + assert!(!cm.contains_key("vector.yaml")); + } + + #[test] + fn test_logback_default_structure() { + // Pins the structural parts of the rendered `logback.xml` that this operator controls (log + // dir + file name, console conversion pattern, max file size, appender wiring). This is the + // source of truth that used to live in the kuttl smoke `14-assert` heredoc; the levels are + // covered separately by `test_logback_renders_zookeeper_container_log_levels`. + 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 + "#; + let cm = build_config_map(zookeeper_yaml).data.unwrap(); + let logback = cm.get("logback.xml").unwrap(); + + // Console appender + the operator's conversion pattern. + assert!( + logback.contains( + r#""# + ), + "{logback}" + ); + assert!( + logback.contains( + "%d{ISO8601} [myid:%X{myid}] - %-5p [%t:%C{1}@%L] - %m%n" + ), + "{logback}" + ); + + // Rolling file appender writing to the operator's log dir + file name. + assert!( + logback.contains( + r#""# + ), + "{logback}" + ); + assert!( + logback.contains("/stackable/log/zookeeper/zookeeper.log4j.xml"), + "{logback}" + ); + assert!( + logback.contains( + "/stackable/log/zookeeper/zookeeper.log4j.xml.%i" + ), + "{logback}" + ); + // 10 MiB total across 2 files -> 5MB per file (derived from MAX_ZK_LOG_FILES_SIZE). + assert!( + logback.contains("5MB"), + "{logback}" + ); + + // Root wires both appenders and defaults to INFO. + assert!(logback.contains(r#""#), "{logback}"); + assert!( + logback.contains(r#""#) + && logback.contains(r#""#), + "{logback}" + ); + } + + #[test] + fn test_logback_renders_zookeeper_container_log_levels() { + // The zookeeper container's automatic log config drives `logback.xml`: the console and file + // appender ThresholdFilter levels, per-logger levels, and the root level. (The `prepare` and + // `vector` container levels do NOT affect `logback.xml` — they flow into those containers' + // own config via upstream behavior — so only the zookeeper container is set here.) + 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: 1 + config: + logging: + containers: + zookeeper: + console: + level: DEBUG + file: + level: WARN + loggers: + ROOT: + level: ERROR + org.apache.zookeeper: + level: TRACE + "#; + let cm = build_config_map(zookeeper_yaml).data.unwrap(); + let logback = cm.get("logback.xml").unwrap(); + + // The console and file appenders share the same ThresholdFilter element, so split on the + // FILE appender to tie each level to its appender: console -> DEBUG, file -> WARN. + let (console_part, file_part) = logback + .split_once(r#"name="FILE""#) + .unwrap_or_else(|| panic!("FILE appender present: {logback}")); + assert!( + console_part.contains("DEBUG"), + "console appender level should be DEBUG: {logback}" + ); + assert!( + !console_part.contains("WARN"), + "console appender level should not be WARN: {logback}" + ); + assert!( + file_part.contains("WARN"), + "file appender level should be WARN: {logback}" + ); + + // The ROOT logger level maps to the `` element; a named logger renders its own entry. + assert!( + logback.contains(r#""#), + "root level should be ERROR: {logback}" + ); + assert!( + logback.contains(r#""#), + "named logger should render at TRACE: {logback}" + ); + } + // --------------------------------------------------------------------------------------------- // TLS x client-auth matrix (candidate #1 in unit-tests.md). // diff --git a/tests/templates/kuttl/smoke/14-assert.yaml.j2 b/tests/templates/kuttl/smoke/14-assert.yaml.j2 index 51df9033..c13c8608 100644 --- a/tests/templates/kuttl/smoke/14-assert.yaml.j2 +++ b/tests/templates/kuttl/smoke/14-assert.yaml.j2 @@ -1,35 +1,26 @@ -{# Templating flags used throughout this file. #} -{% set use_client_tls = test_scenario['values']['use-tls'] == 'true' %} -{% set use_client_auth = test_scenario['values']['use-tls'] == 'true' %} -{% set zk_client_port = 2282 if use_client_tls else 2181 %} --- -# Snapshot the full `.data` of each operator-managed ConfigMap. -# Any code change that alters rendered config values will fail these diffs. +# Discovery ConfigMap sanity check. # -# Runs as its own step (after 13) so kuttl does not re-evaluate the heavy -# heredocs on every 1-second readiness retry of the install step. By this -# point the cluster is in steady state, so each script runs once. +# Runs as its own step (after 13) so kuttl does not re-evaluate it on every 1-second readiness +# retry of the install step. By this point the cluster is in steady state. # -# The heredoc is quoted (`<<'YAMLEOF'`) so shell substitution is disabled and -# property-file escapes like `\:` survive verbatim. `__NAMESPACE__` is -# substituted afterwards via `sed`, and for the test-znode CM also -# `__ZNODE_UID__`, looked up at runtime from the ZookeeperZnode resource. -# Both sides are normalized to canonical JSON via `yq -o=json`; keys are -# already alphabetical on both sides (operator stores BTreeMap; kubectl -# serializes maps sorted; the heredoc is hand-sorted). +# The full `.data` of every operator-managed ConfigMap (zoo.cfg / security.properties / +# logback.xml / vector.yaml for the server rolegroups, and the discovery ZOOKEEPER* values) used +# to be snapshotted here byte-for-byte. That is now owned by unit tests, which reproduce the same +# output without a live cluster: +# - zoo.cfg (TLS x client-auth matrix, configOverride precedence, server.N + myidOffset): +# zk_controller.rs `test_matrix_*`, `test_config_overrides`, `test_server_lines_*` +# - security.properties (byte-for-byte): zk_controller.rs `test_seeded_operator_defaults` +# - logback.xml (structure + per-container levels): zk_controller.rs `test_logback_*` +# - vector.yaml presence: zk_controller.rs `test_vector_agent_adds_vector_config` +# - discovery ZOOKEEPER* values: discovery.rs `*_discovery_cm_*` +# +# The check below only guards what a unit test cannot: that the discovery CMs actually reconciled +# with the real service-name/namespace wiring and carry non-empty values. apiVersion: kuttl.dev/v1beta1 kind: TestAssert timeout: 60 commands: - # The Vector sidecar's env wiring is set by the upstream `vector_container` helper and its - # presence on the StatefulSet is unit-tested (statefulset.rs - # `vector_agent_adds_vector_container_to_statefulset`), so no runtime env assertion is needed here. - # - # Discovery ConfigMap content (`ZOOKEEPER`/`ZOOKEEPER_HOSTS`/`ZOOKEEPER_CHROOT`/ - # `ZOOKEEPER_CLIENT_PORT`, incl. the znode chroot suffix and the secure-vs-insecure client port) - # is unit-tested by `build_(znode_)discovery_configmap` (discovery.rs). The checks below only guard - # what a unit test cannot: that the CMs actually reconciled with the real service-name/namespace - # wiring and carry non-empty values. - script: | for cm in test-zk test-znode; do data=$(kubectl -n $NAMESPACE get cm "$cm" -o yaml | yq -o=json '.data') @@ -51,560 +42,3 @@ commands: ;; esac done - - script: | - expected=$(cat <<'YAMLEOF' | sed "s|__NAMESPACE__|$NAMESPACE|g" | yq -o=json - logback.xml: | - - - - %d{ISO8601} [myid:%X{myid}] - %-5p [%t:%C{1}@%L] - %m%n - - - INFO - - - - - /stackable/log/zookeeper/zookeeper.log4j.xml - - - - - INFO - - - 1 - 1 - /stackable/log/zookeeper/zookeeper.log4j.xml.%i - - - 5MB - - 5000 - - - - - - - - - - - security.properties: | - networkaddress.cache.negative.ttl=0 - networkaddress.cache.ttl=5 -{% if lookup('env', 'VECTOR_AGGREGATOR') %} - vector.yaml: | - --- - data_dir: ${DATA_DIR} - - log_schema: - host_key: pod - - sources: - # Reads the internal Vector logs - vector: - type: internal_logs - - files_stdout: - type: file - include: - - ${LOG_DIR}/*/*.stdout.log - - files_stderr: - type: file - include: - - ${LOG_DIR}/*/*.stderr.log - - files_log4j: - type: file - include: - - ${LOG_DIR}/*/*.log4j.xml - line_delimiter: "\r\n" - multiline: - mode: halt_before - start_pattern: ^" + raw_message + "" - parsed_event, err = parse_xml(wrapped_xml_event) - if err != null { - error = "XML not parsable: " + err - .errors = push(.errors, error) - log(error, level: "warn") - .message = raw_message - } else { - root = object!(parsed_event.root) - if !is_object(root.event) { - error = "Parsed event contains no \"event\" tag." - .errors = push(.errors, error) - log(error, level: "warn") - .message = raw_message - } else { - if keys(root) != ["event"] { - .errors = push(.errors, "Parsed event contains multiple tags: " + join!(keys(root), ", ")) - } - event = object!(root.event) - - epoch_milliseconds, err = to_int(event.@timestamp) - if err == null && epoch_milliseconds != 0 { - converted_timestamp, err = from_unix_timestamp(epoch_milliseconds, "milliseconds") - if err == null { - .timestamp = converted_timestamp - } else { - .errors = push(.errors, "Time not parsable, using current time instead: " + err) - } - } else { - .errors = push(.errors, "Timestamp not found, using current time instead.") - } - - .logger, err = string(event.@logger) - if err != null || is_empty(.logger) { - .errors = push(.errors, "Logger not found.") - } - - level, err = string(event.@level) - if err != null { - .errors = push(.errors, "Level not found, using \"" + .level + "\" instead.") - } else if !includes(["TRACE", "DEBUG", "INFO", "WARN", "ERROR", "FATAL"], level) { - .errors = push(.errors, "Level \"" + level + "\" unknown, using \"" + .level + "\" instead.") - } else { - .level = level - } - - message, err = string(event.message) - if err != null || is_empty(message) { - .errors = push(.errors, "Message not found.") - } - throwable = string(event.throwable) ?? "" - .message = join!(compact([message, throwable]), "\n") - } - } - - # Extends the processed files with the fields "container" and "file" - extended_logs_files: - inputs: - - processed_files_* - type: remap - source: | - del(.source_type) - if .errors == [] { - del(.errors) - } - . |= parse_regex!(.file, r'^${LOG_DIR}/(?P.*?)/(?P.*?)$') - - # Filters the logs of the Vector agent according to the defined log level - filtered_logs_vector: - inputs: - - vector - type: filter - condition: > - (.metadata.level == "TRACE" && "${VECTOR_FILE_LOG_LEVEL}" == "trace") || - (.metadata.level == "DEBUG" && includes(["trace", "debug"], "${VECTOR_FILE_LOG_LEVEL}")) || - (.metadata.level == "INFO" && includes(["trace", "debug", "info"], "${VECTOR_FILE_LOG_LEVEL}")) || - (.metadata.level == "WARN" && includes(["trace", "debug", "info", "warn"], "${VECTOR_FILE_LOG_LEVEL}")) || - (.metadata.level == "ERROR" && includes(["trace", "debug", "info", "warn", "error"], "${VECTOR_FILE_LOG_LEVEL}")) - - # Aligns the logs of the Vector agent with the common format - extended_logs_vector: - inputs: - - filtered_logs_vector - type: remap - source: | - .container = "vector" - .level = .metadata.level - .logger = .metadata.module_path - if exists(.file) { .processed_file = del(.file) } - del(.metadata) - del(.pid) - del(.source_type) - - # Add the fields "namespace", "cluster", "role" and "roleGroup" to all logs - extended_logs: - inputs: - - extended_logs_* - type: remap - source: | - .namespace = "${NAMESPACE}" - .cluster = "${CLUSTER_NAME}" - .role = "${ROLE_NAME}" - .roleGroup = "${ROLE_GROUP_NAME}" - - sinks: - # Forward the logs to the Vector aggregator - aggregator: - inputs: - - extended_logs - type: vector - address: ${VECTOR_AGGREGATOR_ADDRESS} -{% endif %} - zoo.cfg: | - admin.serverPort=8080 - authProvider.x509=org.apache.zookeeper.server.auth.X509AuthenticationProvider -{% if use_client_tls %} - client.portUnification=true -{% endif %} - clientPort={{ zk_client_port }} - dataDir=/stackable/data - initLimit=5 - metricsProvider.className=org.apache.zookeeper.metrics.prometheus.PrometheusMetricsProvider - metricsProvider.httpPort=7000 - prop.common=group - prop.group=group - prop.role=role - server.10=test-zk-server-primary-0.test-zk-server-primary-headless.__NAMESPACE__.svc.cluster.local\:2888\:3888;{{ zk_client_port }} - server.11=test-zk-server-primary-1.test-zk-server-primary-headless.__NAMESPACE__.svc.cluster.local\:2888\:3888;{{ zk_client_port }} - server.20=test-zk-server-secondary-0.test-zk-server-secondary-headless.__NAMESPACE__.svc.cluster.local\:2888\:3888;{{ zk_client_port }} - serverCnxnFactory=org.apache.zookeeper.server.NettyServerCnxnFactory -{% if use_client_auth %} - ssl.clientAuth=need -{% endif %} -{% if use_client_tls %} - ssl.hostnameVerification=true - ssl.keyStore.location=/stackable/server_tls/keystore.p12 -{% endif %} - ssl.quorum.clientAuth=need - ssl.quorum.hostnameVerification=true - ssl.quorum.keyStore.location=/stackable/quorum_tls/keystore.p12 - ssl.quorum.trustStore.location=/stackable/quorum_tls/truststore.p12 -{% if use_client_tls %} - ssl.trustStore.location=/stackable/server_tls/truststore.p12 -{% endif %} - sslQuorum=true - syncLimit=2 - tickTime=3000 - YAMLEOF - ) - actual=$(kubectl -n $NAMESPACE get cm test-zk-server-primary -o yaml | yq -o=json '.data') - expected_file=$(mktemp) && actual_file=$(mktemp) - printf '%s\n' "$expected" > "$expected_file" - printf '%s\n' "$actual" > "$actual_file" - if ! diff_out=$(diff -u "$expected_file" "$actual_file"); then - echo "ERROR: ConfigMap test-zk-server-primary data drifted from snapshot." - printf '%s\n' "$diff_out" - rm -f "$expected_file" "$actual_file" - exit 1 - fi - rm -f "$expected_file" "$actual_file" - - script: | - expected=$(cat <<'YAMLEOF' | sed "s|__NAMESPACE__|$NAMESPACE|g" | yq -o=json - logback.xml: | - - - - %d{ISO8601} [myid:%X{myid}] - %-5p [%t:%C{1}@%L] - %m%n - - - INFO - - - - - /stackable/log/zookeeper/zookeeper.log4j.xml - - - - - INFO - - - 1 - 1 - /stackable/log/zookeeper/zookeeper.log4j.xml.%i - - - 5MB - - 5000 - - - - - - - - - - - security.properties: | - networkaddress.cache.negative.ttl=0 - networkaddress.cache.ttl=5 -{% if lookup('env', 'VECTOR_AGGREGATOR') %} - vector.yaml: | - --- - data_dir: ${DATA_DIR} - - log_schema: - host_key: pod - - sources: - # Reads the internal Vector logs - vector: - type: internal_logs - - files_stdout: - type: file - include: - - ${LOG_DIR}/*/*.stdout.log - - files_stderr: - type: file - include: - - ${LOG_DIR}/*/*.stderr.log - - files_log4j: - type: file - include: - - ${LOG_DIR}/*/*.log4j.xml - line_delimiter: "\r\n" - multiline: - mode: halt_before - start_pattern: ^" + raw_message + "" - parsed_event, err = parse_xml(wrapped_xml_event) - if err != null { - error = "XML not parsable: " + err - .errors = push(.errors, error) - log(error, level: "warn") - .message = raw_message - } else { - root = object!(parsed_event.root) - if !is_object(root.event) { - error = "Parsed event contains no \"event\" tag." - .errors = push(.errors, error) - log(error, level: "warn") - .message = raw_message - } else { - if keys(root) != ["event"] { - .errors = push(.errors, "Parsed event contains multiple tags: " + join!(keys(root), ", ")) - } - event = object!(root.event) - - epoch_milliseconds, err = to_int(event.@timestamp) - if err == null && epoch_milliseconds != 0 { - converted_timestamp, err = from_unix_timestamp(epoch_milliseconds, "milliseconds") - if err == null { - .timestamp = converted_timestamp - } else { - .errors = push(.errors, "Time not parsable, using current time instead: " + err) - } - } else { - .errors = push(.errors, "Timestamp not found, using current time instead.") - } - - .logger, err = string(event.@logger) - if err != null || is_empty(.logger) { - .errors = push(.errors, "Logger not found.") - } - - level, err = string(event.@level) - if err != null { - .errors = push(.errors, "Level not found, using \"" + .level + "\" instead.") - } else if !includes(["TRACE", "DEBUG", "INFO", "WARN", "ERROR", "FATAL"], level) { - .errors = push(.errors, "Level \"" + level + "\" unknown, using \"" + .level + "\" instead.") - } else { - .level = level - } - - message, err = string(event.message) - if err != null || is_empty(message) { - .errors = push(.errors, "Message not found.") - } - throwable = string(event.throwable) ?? "" - .message = join!(compact([message, throwable]), "\n") - } - } - - # Extends the processed files with the fields "container" and "file" - extended_logs_files: - inputs: - - processed_files_* - type: remap - source: | - del(.source_type) - if .errors == [] { - del(.errors) - } - . |= parse_regex!(.file, r'^${LOG_DIR}/(?P.*?)/(?P.*?)$') - - # Filters the logs of the Vector agent according to the defined log level - filtered_logs_vector: - inputs: - - vector - type: filter - condition: > - (.metadata.level == "TRACE" && "${VECTOR_FILE_LOG_LEVEL}" == "trace") || - (.metadata.level == "DEBUG" && includes(["trace", "debug"], "${VECTOR_FILE_LOG_LEVEL}")) || - (.metadata.level == "INFO" && includes(["trace", "debug", "info"], "${VECTOR_FILE_LOG_LEVEL}")) || - (.metadata.level == "WARN" && includes(["trace", "debug", "info", "warn"], "${VECTOR_FILE_LOG_LEVEL}")) || - (.metadata.level == "ERROR" && includes(["trace", "debug", "info", "warn", "error"], "${VECTOR_FILE_LOG_LEVEL}")) - - # Aligns the logs of the Vector agent with the common format - extended_logs_vector: - inputs: - - filtered_logs_vector - type: remap - source: | - .container = "vector" - .level = .metadata.level - .logger = .metadata.module_path - if exists(.file) { .processed_file = del(.file) } - del(.metadata) - del(.pid) - del(.source_type) - - # Add the fields "namespace", "cluster", "role" and "roleGroup" to all logs - extended_logs: - inputs: - - extended_logs_* - type: remap - source: | - .namespace = "${NAMESPACE}" - .cluster = "${CLUSTER_NAME}" - .role = "${ROLE_NAME}" - .roleGroup = "${ROLE_GROUP_NAME}" - - sinks: - # Forward the logs to the Vector aggregator - aggregator: - inputs: - - extended_logs - type: vector - address: ${VECTOR_AGGREGATOR_ADDRESS} -{% endif %} - zoo.cfg: | - admin.serverPort=8080 - authProvider.x509=org.apache.zookeeper.server.auth.X509AuthenticationProvider -{% if use_client_tls %} - client.portUnification=true -{% endif %} - clientPort={{ zk_client_port }} - dataDir=/stackable/data - initLimit=5 - metricsProvider.className=org.apache.zookeeper.metrics.prometheus.PrometheusMetricsProvider - metricsProvider.httpPort=7000 - prop.common=role - prop.role=role - server.10=test-zk-server-primary-0.test-zk-server-primary-headless.__NAMESPACE__.svc.cluster.local\:2888\:3888;{{ zk_client_port }} - server.11=test-zk-server-primary-1.test-zk-server-primary-headless.__NAMESPACE__.svc.cluster.local\:2888\:3888;{{ zk_client_port }} - server.20=test-zk-server-secondary-0.test-zk-server-secondary-headless.__NAMESPACE__.svc.cluster.local\:2888\:3888;{{ zk_client_port }} - serverCnxnFactory=org.apache.zookeeper.server.NettyServerCnxnFactory -{% if use_client_auth %} - ssl.clientAuth=need -{% endif %} -{% if use_client_tls %} - ssl.hostnameVerification=true - ssl.keyStore.location=/stackable/server_tls/keystore.p12 -{% endif %} - ssl.quorum.clientAuth=need - ssl.quorum.hostnameVerification=true - ssl.quorum.keyStore.location=/stackable/quorum_tls/keystore.p12 - ssl.quorum.trustStore.location=/stackable/quorum_tls/truststore.p12 -{% if use_client_tls %} - ssl.trustStore.location=/stackable/server_tls/truststore.p12 -{% endif %} - sslQuorum=true - syncLimit=2 - tickTime=3000 - YAMLEOF - ) - actual=$(kubectl -n $NAMESPACE get cm test-zk-server-secondary -o yaml | yq -o=json '.data') - expected_file=$(mktemp) && actual_file=$(mktemp) - printf '%s\n' "$expected" > "$expected_file" - printf '%s\n' "$actual" > "$actual_file" - if ! diff_out=$(diff -u "$expected_file" "$actual_file"); then - echo "ERROR: ConfigMap test-zk-server-secondary data drifted from snapshot." - printf '%s\n' "$diff_out" - rm -f "$expected_file" "$actual_file" - exit 1 - fi - rm -f "$expected_file" "$actual_file" diff --git a/tests/templates/kuttl/smoke/21-assert.yaml.j2 b/tests/templates/kuttl/smoke/21-assert.yaml.j2 index 055fa5ac..421b00d9 100644 --- a/tests/templates/kuttl/smoke/21-assert.yaml.j2 +++ b/tests/templates/kuttl/smoke/21-assert.yaml.j2 @@ -5,7 +5,6 @@ metadata: name: test-regorule commands: - script: kubectl exec -n $NAMESPACE zk-test-helper-0 -- python /tmp/test_zookeeper.py -n $NAMESPACE - - script: kubectl exec -n $NAMESPACE test-zk-server-primary-0 --container='zookeeper' -- /tmp/test_heap.sh {% if test_scenario['values']['use-tls'] == 'true' %} - script: kubectl exec -n $NAMESPACE test-zk-server-primary-0 --container='zookeeper' -- /tmp/test_tls.sh $NAMESPACE {% endif %} diff --git a/tests/templates/kuttl/smoke/21-prepare-test-zookeeper.yaml.j2 b/tests/templates/kuttl/smoke/21-prepare-test-zookeeper.yaml.j2 index 1278085e..f90b7fdb 100644 --- a/tests/templates/kuttl/smoke/21-prepare-test-zookeeper.yaml.j2 +++ b/tests/templates/kuttl/smoke/21-prepare-test-zookeeper.yaml.j2 @@ -3,7 +3,6 @@ apiVersion: kuttl.dev/v1beta1 kind: TestStep commands: - script: kubectl cp -n $NAMESPACE ./test_zookeeper.py zk-test-helper-0:/tmp - - script: kubectl cp -n $NAMESPACE ./test_heap.sh test-zk-server-primary-0:/tmp --container='zookeeper' {% if test_scenario['values']['use-tls'] == 'true' %} - script: kubectl cp -n $NAMESPACE ./test_tls.sh test-zk-server-primary-0:/tmp --container='zookeeper' {% endif %} diff --git a/tests/templates/kuttl/smoke/test_heap.sh b/tests/templates/kuttl/smoke/test_heap.sh deleted file mode 100755 index b515b0ae..00000000 --- a/tests/templates/kuttl/smoke/test_heap.sh +++ /dev/null @@ -1,16 +0,0 @@ -#!/usr/bin/env bash -# Usage: test_heap.sh - -# 0.5Gi * 1024 -> 512Mebi * 0.8 -> 409 -EXPECTED_HEAP=409 - -# Check if ZK_SERVER_HEAP is set to the correct calculated value -if [[ $ZK_SERVER_HEAP == "$EXPECTED_HEAP" ]] -then - echo "[SUCCESS] ZK_SERVER_HEAP set to $EXPECTED_HEAP" -else - echo "[ERROR] ZK_SERVER_HEAP not set or set with wrong value: $ZK_SERVER_HEAP" - exit 1 -fi - -echo "[SUCCESS] All heap settings tests successful!" From 177e0189d5aaf1512afad43186859dd139eeceab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCller?= Date: Thu, 16 Jul 2026 09:55:14 +0200 Subject: [PATCH 08/12] refactor checks to move away from string matches --- rust/operator-binary/src/zk_controller.rs | 326 +++++++++++----------- 1 file changed, 158 insertions(+), 168 deletions(-) diff --git a/rust/operator-binary/src/zk_controller.rs b/rust/operator-binary/src/zk_controller.rs index 3136491d..4e45df40 100644 --- a/rust/operator-binary/src/zk_controller.rs +++ b/rust/operator-binary/src/zk_controller.rs @@ -475,7 +475,10 @@ pub(crate) mod test_support { #[cfg(test)] mod tests { - use std::{collections::BTreeSet, str::FromStr}; + use std::{ + collections::{BTreeMap, BTreeSet}, + str::FromStr, + }; use stackable_operator::{ k8s_openapi::api::core::v1::ConfigMap, v2::types::operator::RoleGroupName, @@ -483,6 +486,7 @@ mod tests { use super::*; use crate::zk_controller::{ + build::properties::zoo_cfg, test_support::{ cluster_info, minimal_zk, validated_cluster, validated_cluster_with_client_auth, }, @@ -914,16 +918,82 @@ mod tests { // // These four tests own what the kuttl smoke `14-assert` `zoo.cfg` heredoc used to assert across // the `use-server-tls` x `use-client-auth-tls` scenario matrix: the exact set of `zoo.cfg` - // properties and the ports/`ssl.*` lines that vary with TLS and client mTLS. Asserting the full - // key set (not just `contains`) catches accidentally added *or* removed properties. Client mTLS - // is injected via `validated_cluster_with_client_auth`, which the previous `new_for_tests()` - // path could not reach. + // properties and the ports/`ssl.*` lines that vary with TLS and client mTLS. They assert on the + // structured key/value map produced by the pure `zoo_cfg::build` seam (via `zoo_cfg_map`) rather + // than re-parsing rendered text, so key sets and values are compared exactly. Asserting the full + // key set catches accidentally added *or* removed properties, and makes the negative cases + // (`ssl.clientAuth` absent, etc.) fall out of the set equality instead of needing brittle + // `!contains` checks. Only two fixtures are needed: server TLS on vs. off. Client mTLS is layered + // on via `validated_cluster_with_client_auth`, which the previous `new_for_tests()` path could + // not reach. // --------------------------------------------------------------------------------------------- + /// Server role group with default TLS (server `AuthenticationClass` off, server TLS on). + const SERVER_TLS_YAML: &str = r#" + apiVersion: zookeeper.stackable.tech/v1alpha1 + kind: ZookeeperCluster + metadata: + name: simple-zookeeper + spec: + image: + productVersion: "3.9.5" + servers: + roleGroups: + default: + replicas: 3 + "#; + + /// As [`SERVER_TLS_YAML`] but with server TLS explicitly disabled (`serverSecretClass: null`). + const NO_SERVER_TLS_YAML: &str = 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 + "#; + + /// Builds the structured `zoo.cfg` key/value map for a server role group via the pure + /// `zoo_cfg::build` seam (before Java-properties serialization). Asserting on this map gives + /// exact key-set and value comparisons without a hand-rolled parser; the serialization itself is + /// pinned separately (`test_server_lines_use_myid_offset_across_rolegroups`, + /// `test_seeded_operator_defaults`). + fn zoo_cfg_map(validated: &ValidatedCluster, role_group: &str) -> BTreeMap { + let role_group_name = RoleGroupName::from_str(role_group).expect("valid role group name"); + let rolegroup_config = + &validated.role_group_configs[&ZookeeperRole::Server][&role_group_name]; + let server_addresses = zoo_cfg::server_addresses(validated, &cluster_info()); + zoo_cfg::build(validated, rolegroup_config, &server_addresses) + } + + /// Builds a `BTreeSet` from string slices, for comparing against `zoo.cfg` key sets. + fn key_set<'a>(keys: impl IntoIterator) -> BTreeSet { + keys.into_iter().map(str::to_owned).collect() + } + + /// The non-`server.` property keys of a built `zoo.cfg` map. The quorum `server.` + /// entries are replica-dependent and asserted separately in + /// `test_server_lines_use_myid_offset_across_rolegroups`. + fn property_keys(zoo_cfg: &BTreeMap) -> BTreeSet { + zoo_cfg + .keys() + .filter(|k| !k.starts_with("server.")) + .cloned() + .collect() + } + /// The `zoo.cfg` keys that are always present regardless of the TLS/client-auth matrix /// (operator-injected defaults + quorum TLS, which is always on). fn base_zoo_cfg_keys() -> BTreeSet { - [ + key_set([ "admin.serverPort", "authProvider.x509", "clientPort", @@ -939,135 +1009,68 @@ mod tests { "sslQuorum", "syncLimit", "tickTime", - ] - .into_iter() - .map(str::to_owned) - .collect() - } - - /// The `zoo.cfg` property keys, excluding the replica-dependent `server.` quorum entries - /// (those are asserted separately in `test_server_lines_use_myid_offset_across_rolegroups`). - fn zoo_cfg_keys(cm: &ConfigMap) -> BTreeSet { - cm.data - .as_ref() - .unwrap() - .get("zoo.cfg") - .unwrap() - .lines() - .filter_map(|line| line.split_once('=').map(|(k, _)| k.to_owned())) - .filter(|k| !k.starts_with("server.")) - .collect() + ]) } /// TLS off, client-auth off: insecure client port, no server-`ssl.*` lines, no /// `client.portUnification`, no `ssl.clientAuth`. #[test] fn test_matrix_no_tls_no_client_auth() { - let cm = build_config_map( - 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 zoo_cfg = zoo_cfg_map( + &validated_cluster(&minimal_zk(NO_SERVER_TLS_YAML)), + "default", ); - let zoo_cfg = cm.data.as_ref().unwrap().get("zoo.cfg").unwrap(); - assert_eq!(zoo_cfg_keys(&cm), base_zoo_cfg_keys()); - assert!(zoo_cfg.contains("clientPort=2181"), "{zoo_cfg}"); - assert!(!zoo_cfg.contains("client.portUnification"), "{zoo_cfg}"); - assert!(!zoo_cfg.contains("ssl.clientAuth"), "{zoo_cfg}"); - assert!(!zoo_cfg.contains("ssl.keyStore.location"), "{zoo_cfg}"); + // The exact base key set proves the absence of every server-TLS / client-auth property. + assert_eq!(property_keys(&zoo_cfg), base_zoo_cfg_keys()); + assert_eq!(zoo_cfg["clientPort"], "2181"); } /// TLS on (default), client-auth off: secure client port, server-`ssl.*` lines and - /// `client.portUnification`, but no `ssl.clientAuth`. + /// `client.portUnification`, but no `ssl.clientAuth` (proven by the key set). #[test] fn test_matrix_server_tls_no_client_auth() { - let cm = build_config_map( - r#" - apiVersion: zookeeper.stackable.tech/v1alpha1 - kind: ZookeeperCluster - metadata: - name: simple-zookeeper - spec: - image: - productVersion: "3.9.5" - servers: - roleGroups: - default: - replicas: 3 - "#, - ); - let zoo_cfg = cm.data.as_ref().unwrap().get("zoo.cfg").unwrap(); - - let mut expected = base_zoo_cfg_keys(); - expected.extend( - [ - "client.portUnification", - "ssl.hostnameVerification", - "ssl.keyStore.location", - "ssl.trustStore.location", - ] - .into_iter() - .map(str::to_owned), + let zoo_cfg = zoo_cfg_map(&validated_cluster(&minimal_zk(SERVER_TLS_YAML)), "default"); + + assert_eq!( + property_keys(&zoo_cfg), + &base_zoo_cfg_keys() + | &key_set([ + "client.portUnification", + "ssl.hostnameVerification", + "ssl.keyStore.location", + "ssl.trustStore.location", + ]), ); - assert_eq!(zoo_cfg_keys(&cm), expected); - assert!(zoo_cfg.contains("clientPort=2282"), "{zoo_cfg}"); - assert!(zoo_cfg.contains("client.portUnification=true"), "{zoo_cfg}"); - assert!( - zoo_cfg.contains("ssl.keyStore.location=/stackable/server_tls/keystore.p12"), - "{zoo_cfg}" + assert_eq!(zoo_cfg["clientPort"], "2282"); + assert_eq!(zoo_cfg["client.portUnification"], "true"); + assert_eq!( + zoo_cfg["ssl.keyStore.location"], + "/stackable/server_tls/keystore.p12" ); - assert!(!zoo_cfg.contains("ssl.clientAuth"), "{zoo_cfg}"); } /// TLS on, client-auth on: as above plus `ssl.clientAuth=need`. #[test] fn test_matrix_server_tls_and_client_auth() { - let zk = minimal_zk( - r#" - apiVersion: zookeeper.stackable.tech/v1alpha1 - kind: ZookeeperCluster - metadata: - name: simple-zookeeper - spec: - image: - productVersion: "3.9.5" - servers: - roleGroups: - default: - replicas: 3 - "#, + let zoo_cfg = zoo_cfg_map( + &validated_cluster_with_client_auth(&minimal_zk(SERVER_TLS_YAML)), + "default", ); - let cm = config_map_for(&validated_cluster_with_client_auth(&zk), "default"); - let zoo_cfg = cm.data.as_ref().unwrap().get("zoo.cfg").unwrap(); - - let mut expected = base_zoo_cfg_keys(); - expected.extend( - [ - "client.portUnification", - "ssl.clientAuth", - "ssl.hostnameVerification", - "ssl.keyStore.location", - "ssl.trustStore.location", - ] - .into_iter() - .map(str::to_owned), + + assert_eq!( + property_keys(&zoo_cfg), + &base_zoo_cfg_keys() + | &key_set([ + "client.portUnification", + "ssl.clientAuth", + "ssl.hostnameVerification", + "ssl.keyStore.location", + "ssl.trustStore.location", + ]), ); - assert_eq!(zoo_cfg_keys(&cm), expected); - assert!(zoo_cfg.contains("clientPort=2282"), "{zoo_cfg}"); - assert!(zoo_cfg.contains("ssl.clientAuth=need"), "{zoo_cfg}"); + assert_eq!(zoo_cfg["clientPort"], "2282"); + assert_eq!(zoo_cfg["ssl.clientAuth"], "need"); } /// Client-auth on while server TLS is explicitly disabled: the client `AuthenticationClass` @@ -1075,43 +1078,25 @@ mod tests { /// `ssl.clientAuth=need`. This cross term was previously unreachable in unit tests. #[test] fn test_matrix_client_auth_forces_tls_without_server_secret_class() { - let zk = minimal_zk( - 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 zoo_cfg = zoo_cfg_map( + &validated_cluster_with_client_auth(&minimal_zk(NO_SERVER_TLS_YAML)), + "default", ); - let cm = config_map_for(&validated_cluster_with_client_auth(&zk), "default"); - let zoo_cfg = cm.data.as_ref().unwrap().get("zoo.cfg").unwrap(); - - let mut expected = base_zoo_cfg_keys(); - expected.extend( - [ - "client.portUnification", - "ssl.clientAuth", - "ssl.hostnameVerification", - "ssl.keyStore.location", - "ssl.trustStore.location", - ] - .into_iter() - .map(str::to_owned), + + assert_eq!( + property_keys(&zoo_cfg), + &base_zoo_cfg_keys() + | &key_set([ + "client.portUnification", + "ssl.clientAuth", + "ssl.hostnameVerification", + "ssl.keyStore.location", + "ssl.trustStore.location", + ]), ); - assert_eq!(zoo_cfg_keys(&cm), expected); - assert!(zoo_cfg.contains("clientPort=2282"), "{zoo_cfg}"); - assert!(zoo_cfg.contains("client.portUnification=true"), "{zoo_cfg}"); - assert!(zoo_cfg.contains("ssl.clientAuth=need"), "{zoo_cfg}"); + assert_eq!(zoo_cfg["clientPort"], "2282"); + assert_eq!(zoo_cfg["client.portUnification"], "true"); + assert_eq!(zoo_cfg["ssl.clientAuth"], "need"); } /// `server.` quorum lines are rendered into the per-rolegroup `zoo.cfg` with @@ -1144,30 +1129,35 @@ mod tests { "#, ); let validated = validated_cluster(&zk); - // The `server.N` lines are identical in every rolegroup's `zoo.cfg`; inspect the primary's. - let cm = config_map_for(&validated, "primary"); - let zoo_cfg = cm.data.as_ref().unwrap().get("zoo.cfg").unwrap(); - // Colons are escaped by the Java-properties writer (`\:`); non-TLS client port 2181. - assert!( - zoo_cfg.contains( - "server.10=test-zk-server-primary-0.test-zk-server-primary-headless.default.svc.cluster.local\\:2888\\:3888;2181" - ), - "{zoo_cfg}" + // The `server.N` entries are identical in every rolegroup's `zoo.cfg`; inspect the primary's. + let zoo_cfg = zoo_cfg_map(&validated, "primary"); + let server_keys: BTreeSet = zoo_cfg + .keys() + .filter(|k| k.starts_with("server.")) + .cloned() + .collect(); + // Exactly the offset-shifted ids and nothing else: primary (offset 10) -> 10, 11; + // secondary (offset 20) -> 20. + assert_eq!( + server_keys, + key_set(["server.10", "server.11", "server.20"]) ); - assert!( - zoo_cfg.contains("server.11=test-zk-server-primary-1."), - "{zoo_cfg}" + // Composed value (pre-serialization, colons unescaped): FQDN + leader/election ports + + // non-TLS client port 2181. + assert_eq!( + zoo_cfg["server.10"], + "test-zk-server-primary-0.test-zk-server-primary-headless.default.svc.cluster.local:2888:3888;2181" ); + + // The Java-properties serializer escapes the colons (`\:`); pin that on the rendered file. + let rendered = config_map_for(&validated, "primary"); + let zoo_cfg_file = rendered.data.as_ref().unwrap().get("zoo.cfg").unwrap(); assert!( - zoo_cfg.contains("server.20=test-zk-server-secondary-0."), - "{zoo_cfg}" - ); - // Exactly the three expected quorum entries, no more. - assert_eq!( - zoo_cfg.lines().filter(|l| l.starts_with("server.")).count(), - 3, - "{zoo_cfg}" + zoo_cfg_file.contains( + "server.10=test-zk-server-primary-0.test-zk-server-primary-headless.default.svc.cluster.local\\:2888\\:3888;2181" + ), + "{zoo_cfg_file}" ); } From 68248a5f70248146bde3d90fab46d1651de4e7e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCller?= Date: Thu, 16 Jul 2026 10:32:31 +0200 Subject: [PATCH 09/12] refactor resource access --- rust/operator-binary/src/crd/affinity.rs | 14 +++--- rust/operator-binary/src/zk_controller.rs | 45 ++++++++++++------- .../zk_controller/build/properties/zoo_cfg.rs | 11 ++--- .../zk_controller/build/resource/service.rs | 14 +++--- .../build/resource/statefulset.rs | 14 +++--- .../src/zk_controller/validate.rs | 18 +++----- 6 files changed, 57 insertions(+), 59 deletions(-) diff --git a/rust/operator-binary/src/crd/affinity.rs b/rust/operator-binary/src/crd/affinity.rs index f65fcb5b..5b3cbe4a 100644 --- a/rust/operator-binary/src/crd/affinity.rs +++ b/rust/operator-binary/src/crd/affinity.rs @@ -25,7 +25,7 @@ pub fn get_affinity(cluster_name: &str, role: &ZookeeperRole) -> StackableAffini #[cfg(test)] mod tests { - use std::{collections::BTreeMap, str::FromStr}; + use std::collections::BTreeMap; use stackable_operator::{ commons::affinity::StackableAffinity, @@ -33,12 +33,10 @@ mod tests { api::core::v1::{PodAffinityTerm, PodAntiAffinity, WeightedPodAffinityTerm}, apimachinery::pkg::apis::meta::v1::LabelSelector, }, - v2::types::operator::RoleGroupName, }; - use crate::{ - crd::affinity::ZookeeperRole, - zk_controller::test_support::{minimal_zk, validated_cluster}, + use crate::zk_controller::test_support::{ + minimal_zk, server_rolegroup_config, validated_cluster, }; #[test] @@ -99,9 +97,9 @@ mod tests { node_selector: None, }; - let default_group = RoleGroupName::from_str("default").expect("valid role group name"); - let affinity = validated_cluster(&zk).role_group_configs[&ZookeeperRole::Server] - [&default_group] + let validated = validated_cluster(&zk); + let affinity = server_rolegroup_config(&validated, "default") + .1 .config .affinity .clone(); diff --git a/rust/operator-binary/src/zk_controller.rs b/rust/operator-binary/src/zk_controller.rs index 4e45df40..498ff47c 100644 --- a/rust/operator-binary/src/zk_controller.rs +++ b/rust/operator-binary/src/zk_controller.rs @@ -393,16 +393,18 @@ pub fn error_policy( /// Shared helpers for building validated test clusters from minimal YAML fixtures. #[cfg(test)] pub(crate) mod test_support { + use std::str::FromStr; + use stackable_operator::{ cli::OperatorEnvironmentOptions, commons::networking::DomainName, - utils::cluster_info::KubernetesClusterInfo, + utils::cluster_info::KubernetesClusterInfo, v2::types::operator::RoleGroupName, }; use crate::{ - crd::{authentication::DereferencedAuthenticationClasses, v1alpha1}, + crd::{ZookeeperRole, authentication::DereferencedAuthenticationClasses, v1alpha1}, zk_controller::{ dereference::DereferencedObjects, - validate::{ValidatedCluster, validate}, + validate::{ValidatedCluster, ZookeeperRoleGroupConfig, validate}, }, }; @@ -471,24 +473,36 @@ pub(crate) mod test_support { ) .expect("validate should succeed for the test fixture") } + + /// Looks up the validated, merged config of the named `server` role group together with its + /// parsed [`RoleGroupName`] — the standard `(name, config)` inputs to the + /// `build_server_rolegroup_*` functions. Panics if the group does not exist. + pub fn server_rolegroup_config<'a>( + validated: &'a ValidatedCluster, + role_group: &str, + ) -> (RoleGroupName, &'a ZookeeperRoleGroupConfig) { + let role_group_name = RoleGroupName::from_str(role_group).expect("valid role group name"); + let config = validated + .role_group_configs + .get(&ZookeeperRole::Server) + .and_then(|groups| groups.get(&role_group_name)) + .unwrap_or_else(|| panic!("server role group {role_group:?} should exist")); + (role_group_name, config) + } } #[cfg(test)] mod tests { - use std::{ - collections::{BTreeMap, BTreeSet}, - str::FromStr, - }; + use std::collections::{BTreeMap, BTreeSet}; - use stackable_operator::{ - k8s_openapi::api::core::v1::ConfigMap, v2::types::operator::RoleGroupName, - }; + use stackable_operator::k8s_openapi::api::core::v1::ConfigMap; use super::*; use crate::zk_controller::{ build::properties::zoo_cfg, test_support::{ - cluster_info, minimal_zk, validated_cluster, validated_cluster_with_client_auth, + cluster_info, minimal_zk, server_rolegroup_config, validated_cluster, + validated_cluster_with_client_auth, }, validate::ValidatedCluster, }; @@ -967,9 +981,7 @@ mod tests { /// pinned separately (`test_server_lines_use_myid_offset_across_rolegroups`, /// `test_seeded_operator_defaults`). fn zoo_cfg_map(validated: &ValidatedCluster, role_group: &str) -> BTreeMap { - let role_group_name = RoleGroupName::from_str(role_group).expect("valid role group name"); - let rolegroup_config = - &validated.role_group_configs[&ZookeeperRole::Server][&role_group_name]; + let (_, rolegroup_config) = server_rolegroup_config(validated, role_group); let server_addresses = zoo_cfg::server_addresses(validated, &cluster_info()); zoo_cfg::build(validated, rolegroup_config, &server_addresses) } @@ -1167,9 +1179,8 @@ mod tests { /// Builds the rolegroup `ConfigMap` for the named server role group of a validated cluster. fn config_map_for(validated_cluster: &ValidatedCluster, role_group: &str) -> ConfigMap { - let role_group_name = RoleGroupName::from_str(role_group).expect("valid role group name"); - let rolegroup_config = - &validated_cluster.role_group_configs[&ZookeeperRole::Server][&role_group_name]; + let (role_group_name, rolegroup_config) = + server_rolegroup_config(validated_cluster, role_group); config_map::build_server_rolegroup_config_map( validated_cluster, diff --git a/rust/operator-binary/src/zk_controller/build/properties/zoo_cfg.rs b/rust/operator-binary/src/zk_controller/build/properties/zoo_cfg.rs index a679f178..b221aa13 100644 --- a/rust/operator-binary/src/zk_controller/build/properties/zoo_cfg.rs +++ b/rust/operator-binary/src/zk_controller/build/properties/zoo_cfg.rs @@ -177,18 +177,15 @@ 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}; + use crate::zk_controller::test_support::{ + cluster_info, minimal_zk, server_rolegroup_config, 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(); + let rg = server_rolegroup_config(&validated, "default").1.clone(); (validated, rg) } diff --git a/rust/operator-binary/src/zk_controller/build/resource/service.rs b/rust/operator-binary/src/zk_controller/build/resource/service.rs index 4670794b..e2701a02 100644 --- a/rust/operator-binary/src/zk_controller/build/resource/service.rs +++ b/rust/operator-binary/src/zk_controller/build/resource/service.rs @@ -118,12 +118,9 @@ pub(crate) fn build_server_rolegroup_metrics_service( #[cfg(test)] mod tests { - use std::str::FromStr; - use super::*; - use crate::{ - crd::ZookeeperRole, - zk_controller::test_support::{minimal_zk, validated_cluster}, + use crate::zk_controller::test_support::{ + minimal_zk, server_rolegroup_config, validated_cluster, }; const DEFAULT_ZK: &str = r#" @@ -157,7 +154,7 @@ mod tests { #[test] fn headless_service_shape() { let validated = validated_cluster(&minimal_zk(DEFAULT_ZK)); - let rg = RoleGroupName::from_str("default").expect("valid role group name"); + let (rg, _) = server_rolegroup_config(&validated, "default"); let service = build_server_rolegroup_headless_service(&validated, &rg); assert_eq!( @@ -193,9 +190,8 @@ mod tests { #[test] fn metrics_service_shape_and_prometheus_annotations() { let validated = validated_cluster(&minimal_zk(DEFAULT_ZK)); - let rg = RoleGroupName::from_str("default").expect("valid role group name"); - let rg_config = validated.role_group_configs[&ZookeeperRole::Server][&rg].clone(); - let service = build_server_rolegroup_metrics_service(&validated, &rg, &rg_config); + let (rg, rg_config) = server_rolegroup_config(&validated, "default"); + let service = build_server_rolegroup_metrics_service(&validated, &rg, rg_config); assert_eq!( service.metadata.name.as_deref(), diff --git a/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs b/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs index 87123255..72fb78ab 100644 --- a/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs @@ -457,15 +457,16 @@ pub fn build_server_rolegroup_statefulset( #[cfg(test)] mod tests { use super::*; - use crate::zk_controller::test_support::{minimal_zk, validated_cluster}; + use crate::zk_controller::test_support::{ + minimal_zk, server_rolegroup_config, validated_cluster, + }; /// Builds the `default` server StatefulSet for `yaml` and returns the ConfigMap name mounted by /// its `log-config` volume. fn log_config_map_name(yaml: &str) -> String { 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(); - build_server_rolegroup_statefulset(&validated, &rg_name, &rg) + let (rg_name, rg) = server_rolegroup_config(&validated, "default"); + build_server_rolegroup_statefulset(&validated, &rg_name, rg) .expect("statefulset builds") .spec .and_then(|spec| spec.template.spec) @@ -540,9 +541,8 @@ mod tests { fn build_sts(yaml: &str, role_group: &str) -> StatefulSet { let validated = validated_cluster(&minimal_zk(yaml)); - let rg_name = RoleGroupName::from_str(role_group).expect("valid role group name"); - let rg = validated.role_group_configs[&ZookeeperRole::Server][&rg_name].clone(); - build_server_rolegroup_statefulset(&validated, &rg_name, &rg).expect("statefulset builds") + let (rg_name, rg) = server_rolegroup_config(&validated, role_group); + build_server_rolegroup_statefulset(&validated, &rg_name, rg).expect("statefulset builds") } fn zookeeper_container( diff --git a/rust/operator-binary/src/zk_controller/validate.rs b/rust/operator-binary/src/zk_controller/validate.rs index 37bac4ee..28811da3 100644 --- a/rust/operator-binary/src/zk_controller/validate.rs +++ b/rust/operator-binary/src/zk_controller/validate.rs @@ -599,7 +599,9 @@ mod tests { use stackable_operator::k8s_openapi::apimachinery::pkg::api::resource::Quantity; use super::*; - use crate::zk_controller::test_support::{minimal_zk, try_validate, validated_cluster}; + use crate::zk_controller::test_support::{ + minimal_zk, server_rolegroup_config, try_validate, validated_cluster, + }; #[test] fn enabling_vector_without_aggregator_name_fails_validation() { @@ -658,17 +660,11 @@ mod tests { } /// Looks up the validated, merged config of a single server role group by name. - fn server_role_group( - validated: &ValidatedCluster, + fn server_role_group<'a>( + validated: &'a ValidatedCluster, role_group: &str, - ) -> ZookeeperRoleGroupConfig { - let role_group_name = RoleGroupName::from_str(role_group).expect("valid role group name"); - validated - .role_group_configs - .get(&ZookeeperRole::Server) - .and_then(|groups| groups.get(&role_group_name)) - .unwrap_or_else(|| panic!("server role group {role_group:?} should exist")) - .clone() + ) -> &'a ZookeeperRoleGroupConfig { + server_rolegroup_config(validated, role_group).1 } /// Mirrors the `resources` integration test (which can no longer use >16 character role-group From 29f71f89764a6401c11de34a362dff556adfe680 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCller?= Date: Thu, 16 Jul 2026 11:31:40 +0200 Subject: [PATCH 10/12] clean up --- .../operator-binary/src/crd/authentication.rs | 7 +-- rust/operator-binary/src/zk_controller.rs | 60 +++++-------------- .../src/zk_controller/build/jvm.rs | 16 ++--- .../build/resource/statefulset.rs | 29 ++++----- 4 files changed, 34 insertions(+), 78 deletions(-) diff --git a/rust/operator-binary/src/crd/authentication.rs b/rust/operator-binary/src/crd/authentication.rs index 8a2c0f10..24ac67a7 100644 --- a/rust/operator-binary/src/crd/authentication.rs +++ b/rust/operator-binary/src/crd/authentication.rs @@ -132,10 +132,9 @@ impl DereferencedAuthenticationClasses { } } - /// USE ONLY IN TESTS! Builds a [`DereferencedAuthenticationClasses`] holding a single TLS - /// `AuthenticationClass`, mirroring the `use-client-auth-tls` kuttl scenario. This exercises the - /// client-mTLS branches of [`crate::crd::security::ZookeeperSecurity`] (secure client port and - /// `ssl.clientAuth=need`), including the case where server TLS is otherwise disabled and the + /// Builds a [`DereferencedAuthenticationClasses`] holding a single TLS `AuthenticationClass`. + /// Exercises the client-mTLS branches of [`crate::crd::security::ZookeeperSecurity`] (secure + /// client port and `ssl.clientAuth=need`), including the case where server TLS is off and the /// auth class alone turns TLS on. #[cfg(test)] pub fn new_for_tests_with_tls_client_auth() -> Self { diff --git a/rust/operator-binary/src/zk_controller.rs b/rust/operator-binary/src/zk_controller.rs index 498ff47c..5f25ab5d 100644 --- a/rust/operator-binary/src/zk_controller.rs +++ b/rust/operator-binary/src/zk_controller.rs @@ -464,8 +464,7 @@ pub(crate) mod test_support { try_validate(zk).expect("validate should succeed for the test fixture") } - /// Runs the real validate step with a single TLS client-auth `AuthenticationClass`, mirroring - /// the `use-client-auth-tls` kuttl scenario. + /// Runs the real validate step with a single TLS client-auth `AuthenticationClass`. pub fn validated_cluster_with_client_auth(zk: &v1alpha1::ZookeeperCluster) -> ValidatedCluster { try_validate_with_auth( zk, @@ -778,43 +777,16 @@ mod tests { fn test_vector_config_absent_when_agent_disabled() { // Default logging has the Vector agent disabled, so no `vector.yaml` is added to the // ConfigMap. Pins the negative branch alongside `test_vector_agent_adds_vector_config`. - 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 - "#; - let cm = build_config_map(zookeeper_yaml).data.unwrap(); + let cm = build_config_map(SERVER_TLS_YAML).data.unwrap(); assert!(!cm.contains_key("vector.yaml")); } #[test] fn test_logback_default_structure() { // Pins the structural parts of the rendered `logback.xml` that this operator controls (log - // dir + file name, console conversion pattern, max file size, appender wiring). This is the - // source of truth that used to live in the kuttl smoke `14-assert` heredoc; the levels are - // covered separately by `test_logback_renders_zookeeper_container_log_levels`. - 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 - "#; - let cm = build_config_map(zookeeper_yaml).data.unwrap(); + // dir + file name, console conversion pattern, max file size, appender wiring). The levels + // are covered separately by `test_logback_renders_zookeeper_container_log_levels`. + let cm = build_config_map(SERVER_TLS_YAML).data.unwrap(); let logback = cm.get("logback.xml").unwrap(); // Console appender + the operator's conversion pattern. @@ -928,18 +900,14 @@ mod tests { } // --------------------------------------------------------------------------------------------- - // TLS x client-auth matrix (candidate #1 in unit-tests.md). + // TLS x client-auth matrix. // - // These four tests own what the kuttl smoke `14-assert` `zoo.cfg` heredoc used to assert across - // the `use-server-tls` x `use-client-auth-tls` scenario matrix: the exact set of `zoo.cfg` - // properties and the ports/`ssl.*` lines that vary with TLS and client mTLS. They assert on the - // structured key/value map produced by the pure `zoo_cfg::build` seam (via `zoo_cfg_map`) rather - // than re-parsing rendered text, so key sets and values are compared exactly. Asserting the full - // key set catches accidentally added *or* removed properties, and makes the negative cases - // (`ssl.clientAuth` absent, etc.) fall out of the set equality instead of needing brittle - // `!contains` checks. Only two fixtures are needed: server TLS on vs. off. Client mTLS is layered - // on via `validated_cluster_with_client_auth`, which the previous `new_for_tests()` path could - // not reach. + // These four tests own the `zoo.cfg` TLS/client-mTLS matrix the kuttl smoke `14-assert` heredoc + // used to check: the exact property key set plus the ports/`ssl.*` lines that vary with server + // TLS and client mTLS. They assert on the structured map from the pure `zoo_cfg::build` seam + // (via `zoo_cfg_map`), so the full key set compares exactly: a missing or extra property fails + // set equality, with no brittle `!contains` checks. Server TLS is toggled by the two fixtures; + // client mTLS is layered on via `validated_cluster_with_client_auth`. // --------------------------------------------------------------------------------------------- /// Server role group with default TLS (server `AuthenticationClass` off, server TLS on). @@ -1112,8 +1080,8 @@ mod tests { } /// `server.` quorum lines are rendered into the per-rolegroup `zoo.cfg` with - /// `myidOffset` applied and colons escaped, aggregated across *all* server role groups. Mirrors - /// the kuttl smoke `14-assert` `server.N` lines (primary offset 10, secondary offset 20). + /// `myidOffset` applied and colons escaped, aggregated across *all* server role groups + /// (primary offset 10, secondary offset 20). #[test] fn test_server_lines_use_myid_offset_across_rolegroups() { let zk = minimal_zk( diff --git a/rust/operator-binary/src/zk_controller/build/jvm.rs b/rust/operator-binary/src/zk_controller/build/jvm.rs index a42dea95..e37fb674 100644 --- a/rust/operator-binary/src/zk_controller/build/jvm.rs +++ b/rust/operator-binary/src/zk_controller/build/jvm.rs @@ -81,24 +81,16 @@ fn is_heap_jvm_argument(jvm_argument: &str) -> bool { #[cfg(test)] mod tests { - use std::str::FromStr; - - use stackable_operator::v2::types::operator::RoleGroupName; - use super::*; use crate::{ - crd::{ZookeeperRole, v1alpha1::ZookeeperCluster}, - zk_controller::test_support::{minimal_zk, validated_cluster}, + crd::v1alpha1::ZookeeperCluster, + zk_controller::test_support::{minimal_zk, server_rolegroup_config, validated_cluster}, }; /// The validated, merged config for the `default` server role group. fn server_default(zk: &ZookeeperCluster) -> ZookeeperRoleGroupConfig { - let default_group = RoleGroupName::from_str("default").expect("valid role group name"); - validated_cluster(zk) - .role_group_configs - .get(&ZookeeperRole::Server) - .and_then(|groups| groups.get(&default_group)) - .expect("server default role group should exist") + server_rolegroup_config(&validated_cluster(zk), "default") + .1 .clone() } diff --git a/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs b/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs index 72fb78ab..f3846eed 100644 --- a/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs @@ -530,13 +530,12 @@ mod tests { } // --------------------------------------------------------------------------------------------- - // StatefulSet resource shapes (candidate #2 in unit-tests.md). + // StatefulSet resource shapes. // - // These own what the kuttl smoke `13-assert` asserted on the StatefulSet spec/metadata: - // replicas, `podManagementPolicy`, `serviceName`, graceful-shutdown period, container resources - // (including the `podOverrides` merge), the `data`/`listener` PVC templates and the heap env var - // (which also covers candidate #7's `test_heap.sh`). The `status:` stanza (`readyReplicas`) is a - // runtime signal and stays in kuttl. + // These own what the kuttl smoke `13-assert` checked on the StatefulSet spec/metadata: replicas, + // `podManagementPolicy`, `serviceName`, graceful-shutdown period, container resources (incl. the + // `podOverrides` merge), the `data`/`listener` PVC templates and the heap env var. The `status:` + // stanza (`readyReplicas`) is a runtime signal and stays in kuttl. // --------------------------------------------------------------------------------------------- fn build_sts(yaml: &str, role_group: &str) -> StatefulSet { @@ -752,8 +751,8 @@ mod tests { #[test] fn statefulset_sets_zk_server_heap_env() { - // Candidate #7: the heap value computed in jvm.rs lands on the STS as `ZK_SERVER_HEAP`. - // The 512Mi default memory limit x 0.8 -> 409 MiB (pinned by the jvm.rs unit tests). + // The heap value computed in jvm.rs lands on the STS as `ZK_SERVER_HEAP`: + // 512Mi default memory limit x 0.8 -> 409 MiB (calculation pinned by the jvm.rs tests). let sts = build_sts( r#" apiVersion: zookeeper.stackable.tech/v1alpha1 @@ -784,10 +783,9 @@ mod tests { #[test] fn vector_agent_adds_vector_container_to_statefulset() { // Enabling the Vector agent wires a `vector` sidecar onto the StatefulSet, mounting the - // `config` (for `vector.yaml`) and `log` volumes. The env vars the sidecar carries are set - // by the upstream `vector_container` helper, so this only pins the wiring seam this operator - // owns: that the sidecar is added at all when the agent is enabled. Without the agent, no - // such container exists. + // `config` (for `vector.yaml`) and `log` volumes. Its env vars come from the upstream + // `vector_container` helper, so this pins only the seam this operator owns: that the sidecar + // is added when the agent is enabled. let sts = build_sts( r#" apiVersion: zookeeper.stackable.tech/v1alpha1 @@ -877,10 +875,9 @@ mod tests { #[test] fn env_overrides_apply_with_role_group_precedence() { - // Candidate #4: `envOverrides` land on the zookeeper container, with the rolegroup value - // winning over the role value on conflict. Mirrors the kuttl smoke `11-assert` setup: - // COMMON_VAR is set at both levels (group wins), ROLE_VAR only at the role, GROUP_VAR only - // at the group. + // `envOverrides` land on the zookeeper container, with the rolegroup value winning over the + // role value on conflict: COMMON_VAR is set at both levels (group wins), ROLE_VAR only at + // the role, GROUP_VAR only at the group. let sts = build_sts( r#" apiVersion: zookeeper.stackable.tech/v1alpha1 From 596a46a9ceb9d01289eee5518c6ec0be63615a7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCller?= Date: Thu, 16 Jul 2026 13:43:29 +0200 Subject: [PATCH 11/12] refactor: dry cluster definitions and move helper code in own file --- rust/operator-binary/src/crd/affinity.rs | 4 +- rust/operator-binary/src/main.rs | 2 + rust/operator-binary/src/test_support.rs | 128 +++++++++++++++ rust/operator-binary/src/zk_controller.rs | 154 +----------------- .../src/zk_controller/build/jvm.rs | 19 +-- .../zk_controller/build/properties/zoo_cfg.rs | 2 +- .../zk_controller/build/resource/discovery.rs | 6 +- .../src/zk_controller/build/resource/pdb.rs | 18 +- .../zk_controller/build/resource/service.rs | 22 +-- .../build/resource/statefulset.rs | 54 +----- .../src/zk_controller/validate.rs | 2 +- .../src/znode_controller/validate.rs | 4 +- 12 files changed, 158 insertions(+), 257 deletions(-) create mode 100644 rust/operator-binary/src/test_support.rs diff --git a/rust/operator-binary/src/crd/affinity.rs b/rust/operator-binary/src/crd/affinity.rs index 5b3cbe4a..1be4d3d6 100644 --- a/rust/operator-binary/src/crd/affinity.rs +++ b/rust/operator-binary/src/crd/affinity.rs @@ -35,9 +35,7 @@ mod tests { }, }; - use crate::zk_controller::test_support::{ - minimal_zk, server_rolegroup_config, validated_cluster, - }; + use crate::test_support::{minimal_zk, server_rolegroup_config, validated_cluster}; #[test] fn test_affinity_defaults() { diff --git a/rust/operator-binary/src/main.rs b/rust/operator-binary/src/main.rs index 873e73d6..5cf3c8a7 100644 --- a/rust/operator-binary/src/main.rs +++ b/rust/operator-binary/src/main.rs @@ -40,6 +40,8 @@ use crate::{ }; pub mod crd; +#[cfg(test)] +mod test_support; mod webhooks; mod zk_controller; mod znode_controller; diff --git a/rust/operator-binary/src/test_support.rs b/rust/operator-binary/src/test_support.rs new file mode 100644 index 00000000..8082b68d --- /dev/null +++ b/rust/operator-binary/src/test_support.rs @@ -0,0 +1,128 @@ +//! Shared helpers for building validated test clusters from minimal YAML fixtures. +//! +//! Lives at the crate root because it is used across `zk_controller`, `znode_controller` and +//! `crd` test modules, not just one of them. + +use std::str::FromStr; + +use stackable_operator::{ + cli::OperatorEnvironmentOptions, commons::networking::DomainName, + utils::cluster_info::KubernetesClusterInfo, v2::types::operator::RoleGroupName, +}; + +use crate::{ + crd::{ZookeeperRole, authentication::DereferencedAuthenticationClasses, v1alpha1}, + zk_controller::{ + dereference::DereferencedObjects, + validate::{ValidatedCluster, ZookeeperRoleGroupConfig, validate}, + }, +}; + +/// Parses a minimal `ZookeeperCluster` test fixture, defaulting `namespace`/`uid` so the +/// validate step can build a [`ValidatedCluster`]. +pub fn minimal_zk(yaml: &str) -> v1alpha1::ZookeeperCluster { + let mut zk: v1alpha1::ZookeeperCluster = + serde_yaml::from_str(yaml).expect("invalid test ZookeeperCluster YAML"); + zk.metadata + .namespace + .get_or_insert_with(|| "default".to_owned()); + zk.metadata + .uid + .get_or_insert_with(|| "c27b3971-ca72-42c1-80a4-abdfc1db0ddd".to_owned()); + zk +} + +/// Minimal valid `ZookeeperCluster` YAML: a single `default` server role group with `replicas` +/// servers and nothing else. +/// +/// Use this (or [`minimal_zk_default`]) for tests that just need *a* valid cluster and don't +/// care about the spec. Keep an inline fixture when the spec detail is the thing under test. +pub fn minimal_zk_yaml(replicas: u16) -> String { + format!( + r#" + apiVersion: zookeeper.stackable.tech/v1alpha1 + kind: ZookeeperCluster + metadata: + name: simple-zookeeper + spec: + image: + productVersion: "3.9.5" + servers: + roleGroups: + default: + replicas: {replicas} + "# + ) +} + +/// Parsed counterpart of [`minimal_zk_yaml`]. +pub fn minimal_zk_default(replicas: u16) -> v1alpha1::ZookeeperCluster { + minimal_zk(&minimal_zk_yaml(replicas)) +} + +pub fn cluster_info() -> KubernetesClusterInfo { + KubernetesClusterInfo { + cluster_domain: DomainName::try_from("cluster.local").expect("valid domain"), + } +} + +fn operator_environment() -> OperatorEnvironmentOptions { + OperatorEnvironmentOptions { + operator_namespace: "stackable-operators".to_owned(), + operator_service_name: "zookeeper-operator".to_owned(), + image_repository: "oci.example.org".to_owned(), + } +} + +/// 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 { + try_validate_with_auth(zk, DereferencedAuthenticationClasses::new_for_tests()) +} + +/// Runs the real validate step with caller-supplied (dereferenced) AuthenticationClasses, so +/// tests can exercise the client-mTLS matrix. +pub fn try_validate_with_auth( + zk: &v1alpha1::ZookeeperCluster, + authentication_classes: DereferencedAuthenticationClasses, +) -> Result { + validate( + zk, + &DereferencedObjects { + authentication_classes, + }, + &operator_environment(), + ) +} + +/// 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") +} + +/// Runs the real validate step with a single TLS client-auth `AuthenticationClass`. +pub fn validated_cluster_with_client_auth(zk: &v1alpha1::ZookeeperCluster) -> ValidatedCluster { + try_validate_with_auth( + zk, + DereferencedAuthenticationClasses::new_for_tests_with_tls_client_auth(), + ) + .expect("validate should succeed for the test fixture") +} + +/// Looks up the validated, merged config of the named `server` role group together with its +/// parsed [`RoleGroupName`] — the standard `(name, config)` inputs to the +/// `build_server_rolegroup_*` functions. Panics if the group does not exist. +pub fn server_rolegroup_config<'a>( + validated: &'a ValidatedCluster, + role_group: &str, +) -> (RoleGroupName, &'a ZookeeperRoleGroupConfig) { + let role_group_name = RoleGroupName::from_str(role_group).expect("valid role group name"); + let config = validated + .role_group_configs + .get(&ZookeeperRole::Server) + .and_then(|groups| groups.get(&role_group_name)) + .unwrap_or_else(|| panic!("server role group {role_group:?} should exist")); + (role_group_name, config) +} diff --git a/rust/operator-binary/src/zk_controller.rs b/rust/operator-binary/src/zk_controller.rs index 5f25ab5d..c35b3f38 100644 --- a/rust/operator-binary/src/zk_controller.rs +++ b/rust/operator-binary/src/zk_controller.rs @@ -45,7 +45,7 @@ use crate::{ }; pub(crate) mod build; -mod dereference; +pub(crate) mod dereference; pub(crate) mod validate; pub const ZK_CONTROLLER_NAME: &str = "zookeepercluster"; @@ -390,106 +390,6 @@ pub fn error_policy( } } -/// Shared helpers for building validated test clusters from minimal YAML fixtures. -#[cfg(test)] -pub(crate) mod test_support { - use std::str::FromStr; - - use stackable_operator::{ - cli::OperatorEnvironmentOptions, commons::networking::DomainName, - utils::cluster_info::KubernetesClusterInfo, v2::types::operator::RoleGroupName, - }; - - use crate::{ - crd::{ZookeeperRole, authentication::DereferencedAuthenticationClasses, v1alpha1}, - zk_controller::{ - dereference::DereferencedObjects, - validate::{ValidatedCluster, ZookeeperRoleGroupConfig, validate}, - }, - }; - - /// Parses a minimal `ZookeeperCluster` test fixture, defaulting `namespace`/`uid` so the - /// validate step can build a [`ValidatedCluster`]. - pub fn minimal_zk(yaml: &str) -> v1alpha1::ZookeeperCluster { - let mut zk: v1alpha1::ZookeeperCluster = - serde_yaml::from_str(yaml).expect("invalid test ZookeeperCluster YAML"); - zk.metadata - .namespace - .get_or_insert_with(|| "default".to_owned()); - zk.metadata - .uid - .get_or_insert_with(|| "c27b3971-ca72-42c1-80a4-abdfc1db0ddd".to_owned()); - zk - } - - pub fn cluster_info() -> KubernetesClusterInfo { - KubernetesClusterInfo { - cluster_domain: DomainName::try_from("cluster.local").expect("valid domain"), - } - } - - fn operator_environment() -> OperatorEnvironmentOptions { - OperatorEnvironmentOptions { - operator_namespace: "stackable-operators".to_owned(), - operator_service_name: "zookeeper-operator".to_owned(), - image_repository: "oci.example.org".to_owned(), - } - } - - /// 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 { - try_validate_with_auth(zk, DereferencedAuthenticationClasses::new_for_tests()) - } - - /// Runs the real validate step with caller-supplied (dereferenced) AuthenticationClasses, so - /// tests can exercise the client-mTLS matrix. - pub fn try_validate_with_auth( - zk: &v1alpha1::ZookeeperCluster, - authentication_classes: DereferencedAuthenticationClasses, - ) -> Result { - validate( - zk, - &DereferencedObjects { - authentication_classes, - }, - &operator_environment(), - ) - } - - /// 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") - } - - /// Runs the real validate step with a single TLS client-auth `AuthenticationClass`. - pub fn validated_cluster_with_client_auth(zk: &v1alpha1::ZookeeperCluster) -> ValidatedCluster { - try_validate_with_auth( - zk, - DereferencedAuthenticationClasses::new_for_tests_with_tls_client_auth(), - ) - .expect("validate should succeed for the test fixture") - } - - /// Looks up the validated, merged config of the named `server` role group together with its - /// parsed [`RoleGroupName`] — the standard `(name, config)` inputs to the - /// `build_server_rolegroup_*` functions. Panics if the group does not exist. - pub fn server_rolegroup_config<'a>( - validated: &'a ValidatedCluster, - role_group: &str, - ) -> (RoleGroupName, &'a ZookeeperRoleGroupConfig) { - let role_group_name = RoleGroupName::from_str(role_group).expect("valid role group name"); - let config = validated - .role_group_configs - .get(&ZookeeperRole::Server) - .and_then(|groups| groups.get(&role_group_name)) - .unwrap_or_else(|| panic!("server role group {role_group:?} should exist")); - (role_group_name, config) - } -} - #[cfg(test)] mod tests { use std::collections::{BTreeMap, BTreeSet}; @@ -497,31 +397,17 @@ mod tests { use stackable_operator::k8s_openapi::api::core::v1::ConfigMap; use super::*; - use crate::zk_controller::{ - build::properties::zoo_cfg, + use crate::{ test_support::{ - cluster_info, minimal_zk, server_rolegroup_config, validated_cluster, + cluster_info, minimal_zk, minimal_zk_yaml, server_rolegroup_config, validated_cluster, validated_cluster_with_client_auth, }, - validate::ValidatedCluster, + zk_controller::{build::properties::zoo_cfg, validate::ValidatedCluster}, }; #[test] fn test_default_config() { - 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 - "#; - let cm = build_config_map(zookeeper_yaml).data.unwrap(); + let cm = build_config_map(&minimal_zk_yaml(3)).data.unwrap(); let config = cm.get("zoo.cfg").unwrap(); assert!(config.contains( "authProvider.x509=org.apache.zookeeper.server.auth.X509AuthenticationProvider" @@ -581,20 +467,7 @@ mod tests { // This test (together with the TLS x client-auth matrix tests below) is the source of truth // for the rendered `zoo.cfg` / `security.properties`; the kuttl smoke keeps only the // live-cluster readiness/status checks. - 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 - "#; - let cm = build_config_map(zookeeper_yaml).data.unwrap(); + let cm = build_config_map(&minimal_zk_yaml(3)).data.unwrap(); // `security.properties` is fully operator-injected; assert it byte-for-byte. assert_eq!( @@ -708,20 +581,7 @@ mod tests { 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(); + let automatic = build_config_map(&minimal_zk_yaml(3)).data.unwrap(); assert!(automatic.contains_key("logback.xml")); let custom_yaml = r#" diff --git a/rust/operator-binary/src/zk_controller/build/jvm.rs b/rust/operator-binary/src/zk_controller/build/jvm.rs index e37fb674..d4e15ea1 100644 --- a/rust/operator-binary/src/zk_controller/build/jvm.rs +++ b/rust/operator-binary/src/zk_controller/build/jvm.rs @@ -84,7 +84,9 @@ mod tests { use super::*; use crate::{ crd::v1alpha1::ZookeeperCluster, - zk_controller::test_support::{minimal_zk, server_rolegroup_config, validated_cluster}, + test_support::{ + minimal_zk, minimal_zk_default, server_rolegroup_config, validated_cluster, + }, }; /// The validated, merged config for the `default` server role group. @@ -96,20 +98,7 @@ mod tests { #[test] fn test_construct_jvm_arguments_defaults() { - let input = r#" - apiVersion: zookeeper.stackable.tech/v1alpha1 - kind: ZookeeperCluster - metadata: - name: simple-zookeeper - spec: - image: - productVersion: "3.9.5" - servers: - roleGroups: - default: - replicas: 1 - "#; - let zk = minimal_zk(input); + let zk = minimal_zk_default(1); let rg = server_default(&zk); let non_heap_jvm_args = construct_non_heap_jvm_args(&rg); let zk_server_heap_env = diff --git a/rust/operator-binary/src/zk_controller/build/properties/zoo_cfg.rs b/rust/operator-binary/src/zk_controller/build/properties/zoo_cfg.rs index b221aa13..71a3eecf 100644 --- a/rust/operator-binary/src/zk_controller/build/properties/zoo_cfg.rs +++ b/rust/operator-binary/src/zk_controller/build/properties/zoo_cfg.rs @@ -178,7 +178,7 @@ impl ValidatedCluster { #[cfg(test)] mod tests { use super::*; - use crate::zk_controller::test_support::{ + use crate::test_support::{ cluster_info, minimal_zk, server_rolegroup_config, validated_cluster, }; diff --git a/rust/operator-binary/src/zk_controller/build/resource/discovery.rs b/rust/operator-binary/src/zk_controller/build/resource/discovery.rs index fc9fab6b..efc00580 100644 --- a/rust/operator-binary/src/zk_controller/build/resource/discovery.rs +++ b/rust/operator-binary/src/zk_controller/build/resource/discovery.rs @@ -226,10 +226,8 @@ mod tests { use super::*; use crate::{ - zk_controller::{ - ZK_CONTROLLER_NAME, - test_support::{minimal_zk, validated_cluster}, - }, + test_support::{minimal_zk, validated_cluster}, + zk_controller::ZK_CONTROLLER_NAME, znode_controller::{ ZNODE_CONTROLLER_NAME, validate::test_support::{minimal_znode, validated_znode}, diff --git a/rust/operator-binary/src/zk_controller/build/resource/pdb.rs b/rust/operator-binary/src/zk_controller/build/resource/pdb.rs index 57949d38..4ac773dc 100644 --- a/rust/operator-binary/src/zk_controller/build/resource/pdb.rs +++ b/rust/operator-binary/src/zk_controller/build/resource/pdb.rs @@ -43,26 +43,12 @@ mod tests { use stackable_operator::k8s_openapi::apimachinery::pkg::util::intstr::IntOrString; use super::*; - use crate::zk_controller::test_support::{minimal_zk, validated_cluster}; + use crate::test_support::{minimal_zk, minimal_zk_default, validated_cluster}; /// By default PDBs are enabled and default to `maxUnavailable: 1` for the server role. #[test] fn pdb_enabled_by_default_with_max_unavailable_one() { - let validated = validated_cluster(&minimal_zk( - r#" - apiVersion: zookeeper.stackable.tech/v1alpha1 - kind: ZookeeperCluster - metadata: - name: simple-zookeeper - spec: - image: - productVersion: "3.9.5" - servers: - roleGroups: - default: - replicas: 3 - "#, - )); + let validated = validated_cluster(&minimal_zk_default(3)); let role_config = validated.role_config.as_ref().expect("role config present"); let pdb = build_pdb(&role_config.pdb, &validated, &ZookeeperRole::Server) diff --git a/rust/operator-binary/src/zk_controller/build/resource/service.rs b/rust/operator-binary/src/zk_controller/build/resource/service.rs index e2701a02..1173235f 100644 --- a/rust/operator-binary/src/zk_controller/build/resource/service.rs +++ b/rust/operator-binary/src/zk_controller/build/resource/service.rs @@ -119,23 +119,7 @@ pub(crate) fn build_server_rolegroup_metrics_service( #[cfg(test)] mod tests { use super::*; - use crate::zk_controller::test_support::{ - minimal_zk, server_rolegroup_config, validated_cluster, - }; - - const DEFAULT_ZK: &str = r#" - apiVersion: zookeeper.stackable.tech/v1alpha1 - kind: ZookeeperCluster - metadata: - name: simple-zookeeper - spec: - image: - productVersion: "3.9.5" - servers: - roleGroups: - default: - replicas: 3 - "#; + use crate::test_support::{minimal_zk_default, server_rolegroup_config, validated_cluster}; fn port(service: &Service, name: &str) -> i32 { service @@ -153,7 +137,7 @@ mod tests { #[test] fn headless_service_shape() { - let validated = validated_cluster(&minimal_zk(DEFAULT_ZK)); + let validated = validated_cluster(&minimal_zk_default(3)); let (rg, _) = server_rolegroup_config(&validated, "default"); let service = build_server_rolegroup_headless_service(&validated, &rg); @@ -189,7 +173,7 @@ mod tests { #[test] fn metrics_service_shape_and_prometheus_annotations() { - let validated = validated_cluster(&minimal_zk(DEFAULT_ZK)); + let validated = validated_cluster(&minimal_zk_default(3)); let (rg, rg_config) = server_rolegroup_config(&validated, "default"); let service = build_server_rolegroup_metrics_service(&validated, &rg, rg_config); diff --git a/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs b/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs index f3846eed..8d3514ec 100644 --- a/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs @@ -457,8 +457,8 @@ pub fn build_server_rolegroup_statefulset( #[cfg(test)] mod tests { use super::*; - use crate::zk_controller::test_support::{ - minimal_zk, server_rolegroup_config, validated_cluster, + use crate::test_support::{ + minimal_zk, minimal_zk_yaml, server_rolegroup_config, validated_cluster, }; /// Builds the `default` server StatefulSet for `yaml` and returns the ConfigMap name mounted by @@ -510,21 +510,7 @@ mod tests { #[test] fn automatic_log_config_mounts_rolegroup_config_map() { // Automatic logging mounts the role group's own ConfigMap, not a user-provided one. - let name = log_config_map_name( - r#" - apiVersion: zookeeper.stackable.tech/v1alpha1 - kind: ZookeeperCluster - metadata: - name: simple-zookeeper - spec: - image: - productVersion: "3.9.5" - servers: - roleGroups: - default: - replicas: 1 - "#, - ); + let name = log_config_map_name(&minimal_zk_yaml(1)); assert_ne!(name, "my-log-config"); assert!(name.contains("simple-zookeeper"), "{name}"); } @@ -753,22 +739,7 @@ mod tests { fn statefulset_sets_zk_server_heap_env() { // The heap value computed in jvm.rs lands on the STS as `ZK_SERVER_HEAP`: // 512Mi default memory limit x 0.8 -> 409 MiB (calculation pinned by the jvm.rs tests). - let sts = build_sts( - r#" - apiVersion: zookeeper.stackable.tech/v1alpha1 - kind: ZookeeperCluster - metadata: - name: simple-zookeeper - spec: - image: - productVersion: "3.9.5" - servers: - roleGroups: - default: - replicas: 1 - "#, - "default", - ); + let sts = build_sts(&minimal_zk_yaml(1), "default"); let heap = zookeeper_container(&sts) .env @@ -842,22 +813,7 @@ mod tests { fn no_vector_container_without_agent() { // Sanity counterpart: with the agent disabled (the default), the StatefulSet carries no // `vector` sidecar. - let sts = build_sts( - r#" - apiVersion: zookeeper.stackable.tech/v1alpha1 - kind: ZookeeperCluster - metadata: - name: simple-zookeeper - spec: - image: - productVersion: "3.9.5" - servers: - roleGroups: - default: - replicas: 1 - "#, - "default", - ); + let sts = build_sts(&minimal_zk_yaml(1), "default"); let has_vector = sts .spec diff --git a/rust/operator-binary/src/zk_controller/validate.rs b/rust/operator-binary/src/zk_controller/validate.rs index 28811da3..27a000e1 100644 --- a/rust/operator-binary/src/zk_controller/validate.rs +++ b/rust/operator-binary/src/zk_controller/validate.rs @@ -599,7 +599,7 @@ mod tests { use stackable_operator::k8s_openapi::apimachinery::pkg::api::resource::Quantity; use super::*; - use crate::zk_controller::test_support::{ + use crate::test_support::{ minimal_zk, server_rolegroup_config, try_validate, validated_cluster, }; diff --git a/rust/operator-binary/src/znode_controller/validate.rs b/rust/operator-binary/src/znode_controller/validate.rs index 06a7b5d1..9dea93d9 100644 --- a/rust/operator-binary/src/znode_controller/validate.rs +++ b/rust/operator-binary/src/znode_controller/validate.rs @@ -203,7 +203,7 @@ pub fn validate( /// Shared helpers for building validated test znodes from minimal YAML fixtures. /// -/// Mirrors `zk_controller::test_support`: rather than hand-constructing a [`ValidatedZnode`] +/// Mirrors the crate-level `test_support`: rather than hand-constructing a [`ValidatedZnode`] /// (whose `metadata` is private), tests run the real [`validate`] step so the wiring under test /// (product version, `zookeeper_security`, identity) matches production. #[cfg(test)] @@ -213,7 +213,7 @@ pub(crate) mod test_support { use super::{ValidatedZnode, validate}; use crate::{ crd::{authentication::DereferencedAuthenticationClasses, v1alpha1}, - zk_controller::test_support::minimal_zk, + test_support::minimal_zk, znode_controller::dereference::DereferencedObjects, }; From 97337fc39cff0b1bce2a21ebc88b28ce3c06a91c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCller?= Date: Fri, 17 Jul 2026 10:08:21 +0200 Subject: [PATCH 12/12] refactor: additional kuttl checks and fixture consolidation --- rust/operator-binary/src/test_support.rs | 2 +- .../build/resource/statefulset.rs | 32 +++++++++++++++++-- .../src/znode_controller/validate.rs | 12 +------ tests/templates/kuttl/smoke/13-assert.yaml.j2 | 24 ++++++++++++-- 4 files changed, 52 insertions(+), 18 deletions(-) diff --git a/rust/operator-binary/src/test_support.rs b/rust/operator-binary/src/test_support.rs index 8082b68d..ee6662d6 100644 --- a/rust/operator-binary/src/test_support.rs +++ b/rust/operator-binary/src/test_support.rs @@ -66,7 +66,7 @@ pub fn cluster_info() -> KubernetesClusterInfo { } } -fn operator_environment() -> OperatorEnvironmentOptions { +pub(crate) fn operator_environment() -> OperatorEnvironmentOptions { OperatorEnvironmentOptions { operator_namespace: "stackable-operators".to_owned(), operator_service_name: "zookeeper-operator".to_owned(), diff --git a/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs b/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs index 8d3514ec..f637c080 100644 --- a/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/zk_controller/build/resource/statefulset.rs @@ -754,9 +754,10 @@ mod tests { #[test] fn vector_agent_adds_vector_container_to_statefulset() { // Enabling the Vector agent wires a `vector` sidecar onto the StatefulSet, mounting the - // `config` (for `vector.yaml`) and `log` volumes. Its env vars come from the upstream - // `vector_container` helper, so this pins only the seam this operator owns: that the sidecar - // is added when the agent is enabled. + // `config` (for `vector.yaml`) and `log` volumes. The env var format comes from the + // upstream `vector_container` helper, but the identity values (cluster/role/role-group) and + // the aggregator ConfigMap reference are wiring this operator supplies, so those are pinned + // here (they were previously only checked by the kuttl smoke `14-assert` heredoc). let sts = build_sts( r#" apiVersion: zookeeper.stackable.tech/v1alpha1 @@ -807,6 +808,31 @@ mod tests { mount_names.contains(&LOG_VOLUME_NAME.as_ref()), "vector container missing `log` volume mount: {mount_names:?}" ); + + // Identity wiring this operator passes into the upstream `vector_container` helper. + let env = vector.env.as_ref().expect("vector container env"); + let env_value = |name: &str| { + env.iter() + .find(|e| e.name == name) + .unwrap_or_else(|| panic!("vector env {name} missing")) + .value + .as_deref() + }; + assert_eq!(env_value("CLUSTER_NAME"), Some("simple-zookeeper")); + assert_eq!(env_value("ROLE_NAME"), Some("server")); + assert_eq!(env_value("ROLE_GROUP_NAME"), Some("default")); + + // The aggregator address resolves from the discovery ConfigMap named in `clusterConfig`. + let aggregator = env + .iter() + .find(|e| e.name == "VECTOR_AGGREGATOR_ADDRESS") + .expect("VECTOR_AGGREGATOR_ADDRESS env var") + .value_from + .as_ref() + .and_then(|source| source.config_map_key_ref.as_ref()) + .expect("VECTOR_AGGREGATOR_ADDRESS resolved from a ConfigMap key"); + assert_eq!(aggregator.name, "vector-aggregator-discovery"); + assert_eq!(aggregator.key, "ADDRESS"); } #[test] diff --git a/rust/operator-binary/src/znode_controller/validate.rs b/rust/operator-binary/src/znode_controller/validate.rs index 9dea93d9..2b6d1186 100644 --- a/rust/operator-binary/src/znode_controller/validate.rs +++ b/rust/operator-binary/src/znode_controller/validate.rs @@ -208,12 +208,10 @@ pub fn validate( /// (product version, `zookeeper_security`, identity) matches production. #[cfg(test)] pub(crate) mod test_support { - use stackable_operator::cli::OperatorEnvironmentOptions; - use super::{ValidatedZnode, validate}; use crate::{ crd::{authentication::DereferencedAuthenticationClasses, v1alpha1}, - test_support::minimal_zk, + test_support::{minimal_zk, operator_environment}, znode_controller::dereference::DereferencedObjects, }; @@ -233,14 +231,6 @@ pub(crate) mod test_support { znode } - fn operator_environment() -> OperatorEnvironmentOptions { - OperatorEnvironmentOptions { - operator_namespace: "stackable-operators".to_owned(), - operator_service_name: "zookeeper-operator".to_owned(), - image_repository: "oci.example.org".to_owned(), - } - } - /// Runs the real znode validate step against a minimal znode fixture and the given /// `ZookeeperCluster` YAML (which controls product version and `zookeeper_security`). pub fn validated_znode(znode: &v1alpha1::ZookeeperZnode, zk_yaml: &str) -> ValidatedZnode { diff --git a/tests/templates/kuttl/smoke/13-assert.yaml.j2 b/tests/templates/kuttl/smoke/13-assert.yaml.j2 index 408a5811..029492ac 100644 --- a/tests/templates/kuttl/smoke/13-assert.yaml.j2 +++ b/tests/templates/kuttl/smoke/13-assert.yaml.j2 @@ -7,7 +7,11 @@ # # * the StatefulSets actually reconciled to a running state (status.readyReplicas) # * the data PVCs were provisioned and bound (status.phase: Bound) -# * the PDB this operator builds is actually applied (bare existence) +# * the PDB this operator builds is applied and matched the running server pods +# (status.expectedPods) +# * the metrics Services this operator builds actually reconciled (bare existence +# per role group): readiness does not depend on them, so a unit test alone would +# not catch a reconcile that stopped emitting them # * the resources this operator does NOT build itself exist at runtime: # - the cluster-internal `test-zk-server` Service (created by the # listener-operator from the Listener object) @@ -33,12 +37,26 @@ status: readyReplicas: 1 replicas: 1 --- -# Built by this operator; its shape is unit-tested in pdb.rs. Kept here as a bare -# existence check so the smoke test still proves the PDB is actually applied. +# Built by this operator; its shape is unit-tested in pdb.rs. Kept here to prove the +# PDB is applied and that k8s matched it against the 3 running server pods. apiVersion: policy/v1 kind: PodDisruptionBudget metadata: name: test-zk-server +status: + expectedPods: 3 +--- +# Metrics Services built by this operator (shape unit-tested in service.rs); one bare +# existence check per role group so a reconcile that stopped emitting them fails here. +apiVersion: v1 +kind: Service +metadata: + name: test-zk-server-primary-metrics +--- +apiVersion: v1 +kind: Service +metadata: + name: test-zk-server-secondary-metrics --- # Created at runtime by the listener-operator, not by this operator's build code. apiVersion: v1