refactor: Non-optional config overrides + ConfigFileName for file names

Mirror trino-operator and hdfs-operator:

- Switch ZookeeperConfigOverrides to stackable_operator::v2::config_overrides::
  KeyValueConfigOverrides with non-optional fields and a derived Merge impl,
  replacing the manual Merge impl and the KeyValueOverridesProvider/
  get_key_value_overrides indirection. The zoo.cfg/security.properties builders
  now read rg.config_overrides.<field> directly and resolve via resolved_overrides
  (None values are dropped).
- Drop the hard-coded ZOOKEEPER_PROPERTIES_FILE / JVM_SECURITY_PROPERTIES_FILE
  crd constants. The ConfigMap file names now come solely from the ConfigFileName
  enum; jvm.rs keeps a local security.properties const for the JVM system property
  (as trino does).

CRD regenerated: the configOverrides fields become non-nullable objects
(default {}) with nullable string values, matching trino/hdfs. Backwards-
compatible for existing string-valued overrides. All 16 tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: maltesander

extra/crds.yaml

@@ -653,23 +653,25 @@ spec:
                     properties:
                       security.properties:
                         additionalProperties:
+                          nullable: true
                           type: string
+                        default: {}
                         description: |-
                           Flat key-value overrides for `*.properties`, Hadoop XML, etc.
 
                           This is backwards-compatible with the existing flat key-value YAML format
                           used by `HashMap<String, String>`.
-                        nullable: true
                         type: object
                       zoo.cfg:
                         additionalProperties:
+                          nullable: true
                           type: string
+                        default: {}
                         description: |-
                           Flat key-value overrides for `*.properties`, Hadoop XML, etc.
 
                           This is backwards-compatible with the existing flat key-value YAML format
                           used by `HashMap<String, String>`.
-                        nullable: true
                         type: object
                     type: object
                   envOverrides:
@@ -1208,23 +1210,25 @@ spec:
                           properties:
                             security.properties:
                               additionalProperties:
+                                nullable: true
                                 type: string
+                              default: {}
                               description: |-
                                 Flat key-value overrides for `*.properties`, Hadoop XML, etc.
 
                                 This is backwards-compatible with the existing flat key-value YAML format
                                 used by `HashMap<String, String>`.
-                              nullable: true
                               type: object
                             zoo.cfg:
                               additionalProperties:
+                                nullable: true
                                 type: string
+                              default: {}
                               description: |-
                                 Flat key-value overrides for `*.properties`, Hadoop XML, etc.
 
                                 This is backwards-compatible with the existing flat key-value YAML format
                                 used by `HashMap<String, String>`.
-                              nullable: true
                               type: object
                           type: object
                         envOverrides:

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

@@ -5,13 +5,17 @@ use stackable_operator::{
 };
 
 use crate::crd::{
-    JMX_METRICS_PORT, JVM_SECURITY_PROPERTIES_FILE, LOG4J_CONFIG_FILE, LOGBACK_CONFIG_FILE,
-    LoggingFramework, STACKABLE_CONFIG_DIR, STACKABLE_LOG_CONFIG_DIR, ZookeeperServerRoleType,
+    JMX_METRICS_PORT, LOG4J_CONFIG_FILE, LOGBACK_CONFIG_FILE, LoggingFramework,
+    STACKABLE_CONFIG_DIR, STACKABLE_LOG_CONFIG_DIR, ZookeeperServerRoleType,
     v1alpha1::{ZookeeperCluster, ZookeeperConfig},
 };
 
 const JAVA_HEAP_FACTOR: f32 = 0.8;
 
+/// The JVM security properties file the operator writes into the rolegroup
+/// ConfigMap (see `zk_controller::build::properties::ConfigFileName`).
+const JVM_SECURITY_PROPERTIES_FILE: &str = "security.properties";
+
 #[derive(Snafu, Debug)]
 pub enum Error {
     #[snafu(display("invalid memory resource configuration - missing default or value in crd?"))]

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

@@ -16,7 +16,6 @@ use stackable_operator::{
         fragment::{self, Fragment, ValidationError},
         merge::Merge,
     },
-    config_overrides::{KeyValueConfigOverrides, KeyValueOverridesProvider},
     crd::ClusterRef,
     deep_merger::ObjectOverrides,
     k8s_openapi::{
@@ -31,6 +30,7 @@ use stackable_operator::{
     shared::time::Duration,
     status::condition::{ClusterCondition, HasStatusCondition},
     utils::cluster_info::KubernetesClusterInfo,
+    v2::config_overrides::KeyValueConfigOverrides,
     versioned::versioned,
 };
 use strum::{Display, EnumIter, EnumString, IntoEnumIterator};
@@ -49,8 +49,6 @@ pub const APP_NAME: &str = "zookeeper";
 pub const OPERATOR_NAME: &str = "zookeeper.stackable.tech";
 pub const FIELD_MANAGER: &str = "zookeeper-operator";
 
-pub const ZOOKEEPER_PROPERTIES_FILE: &str = "zoo.cfg";
-pub const JVM_SECURITY_PROPERTIES_FILE: &str = "security.properties";
 
 pub const ZOOKEEPER_SERVER_PORT_NAME: &str = "zk";
 pub const ZOOKEEPER_LEADER_PORT_NAME: &str = "zk-leader";
@@ -281,17 +279,13 @@ pub mod versioned {
         Zookeeper,
     }
 
-    #[derive(Clone, Debug, Default, Deserialize, JsonSchema, PartialEq, Serialize)]
+    #[derive(Clone, Debug, Default, Deserialize, Merge, JsonSchema, PartialEq, Serialize)]
     pub struct ZookeeperConfigOverrides {
-        #[serde(default, rename = "zoo.cfg", skip_serializing_if = "Option::is_none")]
-        pub zoo_cfg: Option<KeyValueConfigOverrides>,
+        #[serde(default, rename = "zoo.cfg")]
+        pub zoo_cfg: KeyValueConfigOverrides,
 
-        #[serde(
-            default,
-            rename = "security.properties",
-            skip_serializing_if = "Option::is_none"
-        )]
-        pub security_properties: Option<KeyValueConfigOverrides>,
+        #[serde(default, rename = "security.properties")]
+        pub security_properties: KeyValueConfigOverrides,
     }
 
     #[derive(Clone, Default, Debug, Deserialize, Eq, JsonSchema, PartialEq, Serialize)]
@@ -345,47 +339,6 @@ pub mod versioned {
     }
 }
 
-impl Merge for v1alpha1::ZookeeperConfigOverrides {
-    /// Merges `defaults` (role level) into `self` (role-group level). Keys present
-    /// in `self` win; keys only in `defaults` are added. Mirrors how the previous
-    /// product-config pipeline merged role and role-group `configOverrides`.
-    fn merge(&mut self, defaults: &Self) {
-        merge_key_value_overrides(&mut self.zoo_cfg, &defaults.zoo_cfg);
-        merge_key_value_overrides(&mut self.security_properties, &defaults.security_properties);
-    }
-}
-
-/// Merges two optional [`KeyValueConfigOverrides`], with `this` taking precedence.
-fn merge_key_value_overrides(
-    this: &mut Option<KeyValueConfigOverrides>,
-    defaults: &Option<KeyValueConfigOverrides>,
-) {
-    match (this.as_mut(), defaults) {
-        (Some(this), Some(defaults)) => {
-            for (key, value) in &defaults.overrides {
-                this.overrides
-                    .entry(key.clone())
-                    .or_insert_with(|| value.clone());
-            }
-        }
-        (None, Some(defaults)) => *this = Some(defaults.clone()),
-        (Some(_), None) | (None, None) => {}
-    }
-}
-
-impl KeyValueOverridesProvider for v1alpha1::ZookeeperConfigOverrides {
-    fn get_key_value_overrides(&self, file: &str) -> BTreeMap<String, Option<String>> {
-        let field = match file {
-            ZOOKEEPER_PROPERTIES_FILE => self.zoo_cfg.as_ref(),
-            JVM_SECURITY_PROPERTIES_FILE => self.security_properties.as_ref(),
-            _ => None,
-        };
-        field
-            .map(KeyValueConfigOverrides::as_product_config_overrides)
-            .unwrap_or_default()
-    }
-}
-
 #[derive(
     Clone,
     Debug,

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

@@ -7,6 +7,8 @@
 
 use std::collections::BTreeMap;
 
+use stackable_operator::v2::config_overrides::KeyValueConfigOverrides;
+
 pub mod logging;
 pub mod security_properties;
 pub mod zoo_cfg;
@@ -20,22 +22,16 @@ pub enum ConfigFileName {
     SecurityProperties,
 }
 
-/// Applies user-provided key-value overrides to a property map: `Some(value)`
-/// sets the key, `None` removes it.
-pub(crate) fn apply_overrides(
-    target: &mut BTreeMap<String, String>,
-    overrides: BTreeMap<String, Option<String>>,
-) {
-    for (key, value) in overrides {
-        match value {
-            Some(value) => {
-                target.insert(key, value);
-            }
-            None => {
-                target.remove(&key);
-            }
-        }
-    }
+/// Resolves user-provided [`KeyValueConfigOverrides`] into the `(key, value)`
+/// pairs to merge into a config file, dropping entries whose value is unset
+/// (`None`).
+pub(crate) fn resolved_overrides(
+    overrides: KeyValueConfigOverrides,
+) -> impl Iterator<Item = (String, String)> {
+    overrides
+        .overrides
+        .into_iter()
+        .filter_map(|(key, value)| value.map(|value| (key, value)))
 }
 
 /// Converts a `key -> value` map into the `key -> Some(value)` shape expected by

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

@@ -2,11 +2,9 @@
 
 use std::collections::BTreeMap;
 
-use stackable_operator::config_overrides::KeyValueOverridesProvider;
+use crate::zk_controller::validate::ZookeeperRoleGroupConfig;
 
-use crate::{crd::JVM_SECURITY_PROPERTIES_FILE, zk_controller::validate::ZookeeperRoleGroupConfig};
-
-use super::apply_overrides;
+use super::resolved_overrides;
 
 const NETWORKADDRESS_CACHE_TTL: &str = "networkaddress.cache.ttl";
 const NETWORKADDRESS_CACHE_NEGATIVE_TTL: &str = "networkaddress.cache.negative.ttl";
@@ -32,12 +30,9 @@ pub fn build(rolegroup_config: &ZookeeperRoleGroupConfig) -> BTreeMap<String, St
     );
 
     // configOverrides go last so they win.
-    apply_overrides(
-        &mut security_properties,
-        rolegroup_config
-            .config_overrides
-            .get_key_value_overrides(JVM_SECURITY_PROPERTIES_FILE),
-    );
+    security_properties.extend(resolved_overrides(
+        rolegroup_config.config_overrides.security_properties.clone(),
+    ));
 
     security_properties
 }

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

@@ -7,17 +7,15 @@
 
 use std::collections::BTreeMap;
 
-use stackable_operator::config_overrides::KeyValueOverridesProvider;
-
 use crate::{
     crd::{
         METRICS_PROVIDER_HTTP_PORT, METRICS_PROVIDER_HTTP_PORT_KEY, STACKABLE_DATA_DIR,
-        ZOOKEEPER_PROPERTIES_FILE, security::ZookeeperSecurity, v1alpha1::ZookeeperConfig,
+        security::ZookeeperSecurity, v1alpha1::ZookeeperConfig,
     },
     zk_controller::validate::{ValidatedCluster, ZookeeperRoleGroupConfig},
 };
 
-use super::apply_overrides;
+use super::resolved_overrides;
 
 const ADMIN_SERVER_PORT_KEY: &str = "admin.serverPort";
 const DEFAULT_ADMIN_SERVER_PORT: &str = "8080";
@@ -102,12 +100,9 @@ pub fn build(
     }
 
     // 5. configOverrides go last so they win.
-    apply_overrides(
-        &mut zoo_cfg,
-        rolegroup_config
-            .config_overrides
-            .get_key_value_overrides(ZOOKEEPER_PROPERTIES_FILE),
-    );
+    zoo_cfg.extend(resolved_overrides(
+        rolegroup_config.config_overrides.zoo_cfg.clone(),
+    ));
 
     zoo_cfg
 }