Use KeyValueConfigOverrides with BTreeMap<String, String>

Author: siegfriedweber

Cargo.lock

@@ -30,6 +30,6 @@ tracing = "0.1"
 tracing-futures = { version = "0.2", features = ["futures-03"] }
 
 [patch."https://github.com/stackabletech/operator-rs.git"]
-stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "smooth-operator" }
+stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "feat/improve-keyvalueconfigoverrides" }
 # stackable-operator = { path = "../operator-rs/crates/stackable-operator" }
 # stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "main" }

Cargo.nix

@@ -40,8 +40,8 @@ impl HdfsSiteConfigBuilder {
         self
     }
 
-    pub fn extend(&mut self, properties: &BTreeMap<String, String>) -> &mut Self {
-        self.config.extend(properties.clone());
+    pub fn extend(&mut self, properties: impl IntoIterator<Item = (String, String)>) -> &mut Self {
+        self.config.extend(properties);
         self
     }
 
@@ -238,8 +238,8 @@ impl CoreSiteConfigBuilder {
         self
     }
 
-    pub fn extend(&mut self, properties: &BTreeMap<String, String>) -> &mut Self {
-        self.config.extend(properties.clone());
+    pub fn extend(&mut self, properties: impl IntoIterator<Item = (String, String)>) -> &mut Self {
+        self.config.extend(properties);
         self
     }
 

Cargo.toml

@@ -1,14 +1,12 @@
 //! Builds the `core-site.xml` config file.
 
-use std::collections::BTreeMap;
-
 use stackable_operator::{
     utils::cluster_info::KubernetesClusterInfo, v2::config_overrides::KeyValueConfigOverrides,
 };
 
 use crate::{
-    config::CoreSiteConfigBuilder, controller::build::properties::resolved_overrides,
-    crd::HdfsNodeRole, hdfs_controller::ValidatedCluster, security::kerberos::KerberosConfig,
+    config::CoreSiteConfigBuilder, crd::HdfsNodeRole, hdfs_controller::ValidatedCluster,
+    security::kerberos::KerberosConfig,
 };
 
 /// Renders `core-site.xml`: operator defaults + kerberos/OPA security config,
@@ -48,24 +46,21 @@ pub fn build(
         opa_config.add_core_site_config(&mut core_site);
     }
     // the extend with config must come last in order to have overrides working!!!
-    let overrides: BTreeMap<String, String> = resolved_overrides(overrides).collect();
-    core_site.extend(&overrides).build_as_xml()
+    core_site.extend(overrides).build_as_xml()
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::controller::build::properties::test_support::{
-        cluster_info, config_overrides, validated_cluster,
-    };
+    use crate::controller::build::properties::test_support::{cluster_info, validated_cluster};
 
     #[test]
     fn renders_operator_defaults() {
         let xml = build(
             &validated_cluster(),
             HdfsNodeRole::Name,
             &cluster_info(),
-            config_overrides(&[]),
+            KeyValueConfigOverrides::default(),
         );
         assert!(
             xml.contains("<name>fs.defaultFS</name>\n    <value>hdfs://hdfs/</value>"),
@@ -89,7 +84,7 @@ mod tests {
             &validated_cluster(),
             HdfsNodeRole::Name,
             &cluster_info(),
-            config_overrides(&[("io.file.buffer.size", "65536")]),
+            [("io.file.buffer.size", "65536")].into(),
         );
         assert!(
             xml.contains("<name>io.file.buffer.size</name>\n    <value>65536</value>"),

crate-hashes.json

@@ -3,29 +3,23 @@
 //! The operator sets no defaults here; the file exists purely so users can
 //! supply `configOverrides`.
 
-use std::collections::BTreeMap;
-
 use stackable_operator::v2::{
     config_file_writer::to_hadoop_xml, config_overrides::KeyValueConfigOverrides,
 };
 
-use crate::controller::build::properties::resolved_overrides;
-
 /// Renders `hadoop-policy.xml` from the user-provided overrides only.
 pub fn build(overrides: KeyValueConfigOverrides) -> String {
-    let config: BTreeMap<String, String> = resolved_overrides(overrides).collect();
-    to_hadoop_xml(config.iter())
+    to_hadoop_xml(overrides.iter())
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::controller::build::properties::test_support::config_overrides;
 
     #[test]
     fn empty_overrides_render_empty_configuration() {
         assert_eq!(
-            build(config_overrides(&[])),
+            build(KeyValueConfigOverrides::default()),
             concat!(
                 "<?xml version=\"1.0\"?>\n",
                 "<configuration>\n",
@@ -37,7 +31,7 @@ mod tests {
     #[test]
     fn overrides_are_rendered_as_properties() {
         assert_eq!(
-            build(config_overrides(&[("security.client.protocol.acl", "*")])),
+            build([("security.client.protocol.acl", "*")].into()),
             concat!(
                 "<?xml version=\"1.0\"?>\n",
                 "<configuration>\n",

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

@@ -1,14 +1,11 @@
 //! Builds the `hdfs-site.xml` config file.
 
-use std::collections::BTreeMap;
-
 use stackable_operator::{
     utils::cluster_info::KubernetesClusterInfo, v2::config_overrides::KeyValueConfigOverrides,
 };
 
 use crate::{
     config::HdfsSiteConfigBuilder,
-    controller::build::properties::resolved_overrides,
     crd::{AnyNodeConfig, HdfsNodeRole},
     hdfs_controller::ValidatedCluster,
 };
@@ -110,16 +107,15 @@ pub fn build(
         opa_config.add_hdfs_site_config(&mut hdfs_site);
     }
     // the extend with config must come last in order to have overrides working!!!
-    let overrides: BTreeMap<String, String> = resolved_overrides(overrides).collect();
-    hdfs_site.extend(&overrides).build_as_xml()
+    hdfs_site.extend(overrides).build_as_xml()
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
     use crate::{
         controller::build::properties::test_support::{
-            cluster_info, config_overrides, minimal_hdfs, validated_cluster,
+            cluster_info, minimal_hdfs, validated_cluster,
         },
         crd::{HdfsNodeRole, v1alpha1},
     };
@@ -137,7 +133,7 @@ mod tests {
             &validated_cluster(),
             &cluster_info(),
             &merged,
-            config_overrides(&[]),
+            KeyValueConfigOverrides::default(),
         );
         assert!(
             xml.contains("<name>dfs.replication</name>\n    <value>3</value>"),
@@ -156,7 +152,7 @@ mod tests {
             &validated_cluster(),
             &cluster_info(),
             &merged,
-            config_overrides(&[("dfs.replication", "5")]),
+            [("dfs.replication", "5")].into(),
         );
         assert!(
             xml.contains("<name>dfs.replication</name>\n    <value>5</value>"),