clean up

Author: sweb

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 {

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.<myid>` 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(

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()
     }
 

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