fix: move hardcoded envs to constants, cleanup

maltesander · maltesander · commit 095777974fd6 · 2026-06-19T18:04:31.000+02:00
diff --git a/rust/operator-binary/src/controller/build/container.rs b/rust/operator-binary/src/controller/build/container.rs
@@ -106,6 +106,14 @@ constant!(VECTOR_CONFIG_VOLUME_NAME: VolumeName = "hdfs-config");
 // The volume holding the product logs that Vector tails.
 constant!(VECTOR_LOG_VOLUME_NAME: VolumeName = "log");
 
+// Names of the environment variables set on the HDFS containers. Each is used both as the
+// map key and the `EnvVar.name`, so keep them in one place to avoid the two drifting apart.
+const ENV_TOPOLOGY_LABELS: &str = "TOPOLOGY_LABELS";
+const ENV_HADOOP_OPTS: &str = "HADOOP_OPTS";
+const ENV_KRB5_CONFIG: &str = "KRB5_CONFIG";
+const ENV_KRB5_CLIENT_KTNAME: &str = "KRB5_CLIENT_KTNAME";
+const ENV_CONTAINERDEBUG_LOG_DIRECTORY: &str = "CONTAINERDEBUG_LOG_DIRECTORY";
+
 type Result<T, E = Error> = std::result::Result<T, E>;
 
 #[derive(Snafu, Debug, EnumDiscriminants)]
@@ -900,36 +908,36 @@ impl ContainerConfig {
             && let Some(rack_awareness) = cluster.rackawareness_config()
         {
             env.insert(
-                "TOPOLOGY_LABELS".to_string(),
+                ENV_TOPOLOGY_LABELS.to_string(),
                 EnvVar {
-                    name: "TOPOLOGY_LABELS".to_string(),
+                    name: ENV_TOPOLOGY_LABELS.to_string(),
                     value: Some(rack_awareness),
                     ..EnvVar::default()
                 },
             );
         }
 
         env.insert(
-            "HADOOP_OPTS".to_string(),
+            ENV_HADOOP_OPTS.to_string(),
             EnvVar {
-                name: "HADOOP_OPTS".to_string(),
+                name: ENV_HADOOP_OPTS.to_string(),
                 value: Some(construct_global_jvm_args(cluster.has_kerberos_enabled())),
                 ..EnvVar::default()
             },
         );
         if cluster.has_kerberos_enabled() {
             env.insert(
-                "KRB5_CONFIG".to_string(),
+                ENV_KRB5_CONFIG.to_string(),
                 EnvVar {
-                    name: "KRB5_CONFIG".to_string(),
+                    name: ENV_KRB5_CONFIG.to_string(),
                     value: Some(format!("{KERBEROS_CONTAINER_PATH}/krb5.conf")),
                     ..EnvVar::default()
                 },
             );
             env.insert(
-                "KRB5_CLIENT_KTNAME".to_string(),
+                ENV_KRB5_CLIENT_KTNAME.to_string(),
                 EnvVar {
-                    name: "KRB5_CLIENT_KTNAME".to_string(),
+                    name: ENV_KRB5_CLIENT_KTNAME.to_string(),
                     value: Some(format!("{KERBEROS_CONTAINER_PATH}/keytab")),
                     ..EnvVar::default()
                 },
@@ -938,9 +946,9 @@ impl ContainerConfig {
 
         // Needed for the `containerdebug` process to log it's tracing information to.
         env.insert(
-            "CONTAINERDEBUG_LOG_DIRECTORY".to_string(),
+            ENV_CONTAINERDEBUG_LOG_DIRECTORY.to_string(),
             EnvVar {
-                name: "CONTAINERDEBUG_LOG_DIRECTORY".to_string(),
+                name: ENV_CONTAINERDEBUG_LOG_DIRECTORY.to_string(),
                 value: Some(format!("{STACKABLE_LOG_DIR}/containerdebug")),
                 value_from: None,
             },
diff --git a/rust/operator-binary/src/controller/build/kerberos.rs b/rust/operator-binary/src/controller/build/kerberos.rs
@@ -6,6 +6,10 @@ use crate::controller::build::properties::{
 
 pub const KERBEROS_CONTAINER_PATH: &str = "/stackable/kerberos";
 
+/// Hadoop wire-encryption protection level requiring authentication, integrity and
+/// confidentiality. Used for both `dfs.data.transfer.protection` and `hadoop.rpc.protection`.
+const WIRE_ENCRYPTION_PRIVACY: &str = "privacy";
+
 /// The cluster-wide security settings the `security_config` builders need,
 /// resolved from the `HdfsCluster` so the builders don't depend on the raw CRD.
 pub struct KerberosConfig<'a> {
@@ -48,7 +52,7 @@ impl HdfsSiteConfigBuilder {
     }
 
     fn add_wire_encryption_settings(&mut self) -> &mut Self {
-        self.add("dfs.data.transfer.protection", "privacy");
+        self.add("dfs.data.transfer.protection", WIRE_ENCRYPTION_PRIVACY);
         self.add("dfs.encrypt.data.transfer", "true");
         self.add(
             "dfs.encrypt.data.transfer.cipher.suite",
@@ -163,7 +167,7 @@ impl CoreSiteConfigBuilder {
     }
 
     fn add_wire_encryption_settings(&mut self) -> &mut Self {
-        self.add("hadoop.rpc.protection", "privacy");
+        self.add("hadoop.rpc.protection", WIRE_ENCRYPTION_PRIVACY);
         self
     }
 }
diff --git a/rust/operator-binary/src/controller/build/properties/hdfs_site.rs b/rust/operator-binary/src/controller/build/properties/hdfs_site.rs
@@ -231,7 +231,8 @@ impl HdfsSiteConfigBuilder {
                         jnid.fqdn(cluster_info),
                         jnid.ports
                             .get(&String::from(DFS_JOURNALNODE_RPC_ADDRESS))
-                            .map_or(DEFAULT_JOURNAL_NODE_RPC_PORT, |p| p.clone())
+                            .cloned()
+                            .unwrap_or(DEFAULT_JOURNAL_NODE_RPC_PORT)
                     ))
                     .collect::<Vec<_>>()
                     .join(";"),
@@ -316,7 +317,8 @@ impl HdfsSiteConfigBuilder {
                     port = nn
                         .ports
                         .get(port_name)
-                        .map_or(default_port.clone(), |p| p.clone())
+                        .cloned()
+                        .unwrap_or_else(|| default_port.clone())
                 ),
             );
         }
diff --git a/rust/operator-binary/src/controller/build/properties/mod.rs b/rust/operator-binary/src/controller/build/properties/mod.rs
@@ -13,6 +13,9 @@ pub mod security_properties;
 pub mod ssl_client;
 pub mod ssl_server;
 
+/// The keystore/truststore type used for the TLS stores provisioned by secret-operator.
+pub(crate) const KEYSTORE_TYPE_PKCS12: &str = "pkcs12";
+
 /// The `<prefix>.truststore.*` entries (location, type, password) injected into
 /// `ssl-client.xml` / `ssl-server.xml` when HTTPS is enabled.
 fn truststore_entries(prefix: &str) -> [(String, String); 3] {
@@ -21,7 +24,10 @@ fn truststore_entries(prefix: &str) -> [(String, String); 3] {
             format!("{prefix}.truststore.location"),
             format!("{TLS_STORE_DIR}/truststore.p12"),
         ),
-        (format!("{prefix}.truststore.type"), "pkcs12".to_string()),
+        (
+            format!("{prefix}.truststore.type"),
+            KEYSTORE_TYPE_PKCS12.to_string(),
+        ),
         (
             format!("{prefix}.truststore.password"),
             TLS_STORE_PASSWORD.to_string(),
diff --git a/rust/operator-binary/src/controller/build/properties/ssl_server.rs b/rust/operator-binary/src/controller/build/properties/ssl_server.rs
@@ -11,7 +11,7 @@ use stackable_operator::v2::{
 
 use crate::controller::build::{
     container::{TLS_STORE_DIR, TLS_STORE_PASSWORD},
-    properties::truststore_entries,
+    properties::{KEYSTORE_TYPE_PKCS12, truststore_entries},
 };
 
 /// Renders `ssl-server.xml` for the given HTTPS state and user overrides.
@@ -24,7 +24,10 @@ pub fn build(https_enabled: bool, overrides: KeyValueConfigOverrides) -> String
                 "ssl.server.keystore.location".to_string(),
                 format!("{TLS_STORE_DIR}/keystore.p12"),
             ),
-            ("ssl.server.keystore.type".to_string(), "pkcs12".to_string()),
+            (
+                "ssl.server.keystore.type".to_string(),
+                KEYSTORE_TYPE_PKCS12.to_string(),
+            ),
             (
                 "ssl.server.keystore.password".to_string(),
                 TLS_STORE_PASSWORD.to_string(),
diff --git a/rust/operator-binary/src/controller/build/resource/service.rs b/rust/operator-binary/src/controller/build/resource/service.rs
@@ -1,3 +1,6 @@
+//! Builds the per-rolegroup `Service`s for the HdfsCluster: a headless `Service`
+//! backing the `StatefulSet` pod DNS, and a metrics `Service` carrying the
+//! Prometheus scrape labels/annotations.
 use snafu::{ResultExt, Snafu};
 use stackable_operator::{
     builder::meta::ObjectMetaBuilder,
@@ -30,11 +33,13 @@ pub enum Error {
     RoleGroupSelectorLabels { source: LabelError },
 }
 
+type Result<T, E = Error> = std::result::Result<T, E>;
+
 pub(crate) fn rolegroup_headless_service(
     cluster: &ValidatedCluster,
     role: &HdfsNodeRole,
     role_group_name: &RoleGroupName,
-) -> Result<Service, Error> {
+) -> Result<Service> {
     tracing::info!("Setting up headless Service for role {role} role group {role_group_name}");
 
     let resource_names = cluster.resource_names(role, role_group_name);
@@ -92,7 +97,7 @@ pub(crate) fn rolegroup_metrics_service(
     cluster: &ValidatedCluster,
     role: &HdfsNodeRole,
     role_group_name: &RoleGroupName,
-) -> Result<Service, Error> {
+) -> Result<Service> {
     tracing::info!("Setting up metrics Service for role {role} role group {role_group_name}");
 
     let resource_names = cluster.resource_names(role, role_group_name);
diff --git a/rust/operator-binary/src/hdfs_controller.rs b/rust/operator-binary/src/hdfs_controller.rs
@@ -139,7 +139,7 @@ pub enum Error {
         source: error_boundary::InvalidObject,
     },
 
-    #[snafu(display("failed to builds service"))]
+    #[snafu(display("failed to build service"))]
     BuildService { source: service::Error },
 }
 

Original file line number	Diff line number	Diff line change
`@@ -6,6 +6,10 @@ use crate::controller::build::properties::{`
`6`	`6`
`7`	`7`	`pub const KERBEROS_CONTAINER_PATH: &str = "/stackable/kerberos";`
`8`	`8`
	`9`	`+/// Hadoop wire-encryption protection level requiring authentication, integrity and`
	`10`	+/// confidentiality. Used for both `dfs.data.transfer.protection` and `hadoop.rpc.protection`.
	`11`	`+const WIRE_ENCRYPTION_PRIVACY: &str = "privacy";`
	`12`	`+`
`9`	`13`	/// The cluster-wide security settings the `security_config` builders need,
`10`	`14`	/// resolved from the `HdfsCluster` so the builders don't depend on the raw CRD.
`11`	`15`	`pub struct KerberosConfig<'a> {`
`@@ -48,7 +52,7 @@ impl HdfsSiteConfigBuilder {`
`48`	`52`	`}`
`49`	`53`
`50`	`54`	`fn add_wire_encryption_settings(&mut self) -> &mut Self {`
`51`		`- self.add("dfs.data.transfer.protection", "privacy");`
	`55`	`+ self.add("dfs.data.transfer.protection", WIRE_ENCRYPTION_PRIVACY);`
`52`	`56`	`self.add("dfs.encrypt.data.transfer", "true");`
`53`	`57`	`self.add(`
`54`	`58`	`"dfs.encrypt.data.transfer.cipher.suite",`
`@@ -163,7 +167,7 @@ impl CoreSiteConfigBuilder {`
`163`	`167`	`}`
`164`	`168`
`165`	`169`	`fn add_wire_encryption_settings(&mut self) -> &mut Self {`
`166`		`- self.add("hadoop.rpc.protection", "privacy");`
	`170`	`+ self.add("hadoop.rpc.protection", WIRE_ENCRYPTION_PRIVACY);`
`167`	`171`	`self`
`168`	`172`	`}`
`169`	`173`	`}`
Original file line number	Diff line number	Diff line change
`@@ -139,7 +139,7 @@ pub enum Error {`
`139`	`139`	`source: error_boundary::InvalidObject,`
`140`	`140`	`},`
`141`	`141`
`142`		`- #[snafu(display("failed to builds service"))]`
	`142`	`+ #[snafu(display("failed to build service"))]`
`143`	`143`	`BuildService { source: service::Error },`
`144`	`144`	`}`
`145`	`145`