fix: cleanup errors, constants, duplication

Author: maltesander

rust/operator-binary/src/container.rs

@@ -131,11 +131,6 @@ pub enum Error {
         source: ListenerOperatorVolumeSourceBuilderError,
     },
 
-    #[snafu(display("missing or invalid labels for the listener volume"))]
-    ListenerVolumeLabels {
-        source: ListenerOperatorVolumeSourceBuilderError,
-    },
-
     #[snafu(display("failed to configure logging"))]
     ConfigureLogging { source: LoggingError },
 

rust/operator-binary/src/controller/build/properties/hadoop_policy.rs

@@ -20,18 +20,11 @@ pub fn build(overrides: KeyValueConfigOverrides) -> String {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::controller::build::properties::test_support::config_overrides;
+    use crate::controller::build::properties::test_support::{EMPTY_HADOOP_XML, config_overrides};
 
     #[test]
     fn empty_overrides_render_empty_configuration() {
-        assert_eq!(
-            build(config_overrides(&[])),
-            concat!(
-                "<?xml version=\"1.0\"?>\n",
-                "<configuration>\n",
-                "</configuration>"
-            )
-        );
+        assert_eq!(build(config_overrides(&[])), EMPTY_HADOOP_XML);
     }
 
     #[test]

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

@@ -7,6 +7,8 @@ use std::collections::BTreeMap;
 
 use stackable_operator::v2::config_overrides::KeyValueConfigOverrides;
 
+use crate::container::{TLS_STORE_DIR, TLS_STORE_PASSWORD};
+
 pub mod core_site;
 pub mod hadoop_policy;
 pub mod hdfs_site;
@@ -32,6 +34,22 @@ fn resolved_overrides(
     defined_entries(overrides.overrides)
 }
 
+/// 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] {
+    [
+        (
+            format!("{prefix}.truststore.location"),
+            format!("{TLS_STORE_DIR}/truststore.p12"),
+        ),
+        (format!("{prefix}.truststore.type"), "pkcs12".to_string()),
+        (
+            format!("{prefix}.truststore.password"),
+            TLS_STORE_PASSWORD.to_string(),
+        ),
+    ]
+}
+
 /// The names of the HDFS config files assembled into the rolegroup `ConfigMap`.
 #[derive(Clone, Copy, Debug, strum::Display)]
 pub enum ConfigFileName {
@@ -79,6 +97,13 @@ pub(crate) mod test_support {
         crd::v1alpha1,
     };
 
+    /// The rendered output of an empty Hadoop-XML configuration (no entries).
+    pub const EMPTY_HADOOP_XML: &str = concat!(
+        "<?xml version=\"1.0\"?>\n",
+        "<configuration>\n",
+        "</configuration>"
+    );
+
     /// Builds a [`KeyValueConfigOverrides`] from `(key, value)` pairs for tests.
     pub fn config_overrides(pairs: &[(&str, &str)]) -> KeyValueConfigOverrides {
         KeyValueConfigOverrides {

rust/operator-binary/src/controller/build/properties/ssl_client.rs

@@ -9,29 +9,13 @@ use stackable_operator::v2::{
     config_file_writer::to_hadoop_xml, config_overrides::KeyValueConfigOverrides,
 };
 
-use crate::{
-    container::{TLS_STORE_DIR, TLS_STORE_PASSWORD},
-    controller::build::properties::resolved_overrides,
-};
+use crate::controller::build::properties::{resolved_overrides, truststore_entries};
 
 /// Renders `ssl-client.xml` for the given HTTPS state and user overrides.
 pub fn build(https_enabled: bool, overrides: KeyValueConfigOverrides) -> String {
     let mut config: BTreeMap<String, String> = BTreeMap::new();
     if https_enabled {
-        config.extend([
-            (
-                "ssl.client.truststore.location".to_string(),
-                format!("{TLS_STORE_DIR}/truststore.p12"),
-            ),
-            (
-                "ssl.client.truststore.type".to_string(),
-                "pkcs12".to_string(),
-            ),
-            (
-                "ssl.client.truststore.password".to_string(),
-                TLS_STORE_PASSWORD.to_string(),
-            ),
-        ]);
+        config.extend(truststore_entries("ssl.client"));
     }
     // Overrides applied last so users win.
     config.extend(resolved_overrides(overrides));
@@ -41,18 +25,14 @@ pub fn build(https_enabled: bool, overrides: KeyValueConfigOverrides) -> String
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::controller::build::properties::test_support::config_overrides;
+    use crate::{
+        container::{TLS_STORE_DIR, TLS_STORE_PASSWORD},
+        controller::build::properties::test_support::{EMPTY_HADOOP_XML, config_overrides},
+    };
 
     #[test]
     fn disabled_https_without_overrides_renders_empty_configuration() {
-        assert_eq!(
-            build(false, config_overrides(&[])),
-            concat!(
-                "<?xml version=\"1.0\"?>\n",
-                "<configuration>\n",
-                "</configuration>"
-            )
-        );
+        assert_eq!(build(false, config_overrides(&[])), EMPTY_HADOOP_XML);
     }
 
     #[test]

rust/operator-binary/src/controller/build/properties/ssl_server.rs

@@ -11,26 +11,15 @@ use stackable_operator::v2::{
 
 use crate::{
     container::{TLS_STORE_DIR, TLS_STORE_PASSWORD},
-    controller::build::properties::resolved_overrides,
+    controller::build::properties::{resolved_overrides, truststore_entries},
 };
 
 /// Renders `ssl-server.xml` for the given HTTPS state and user overrides.
 pub fn build(https_enabled: bool, overrides: KeyValueConfigOverrides) -> String {
     let mut config: BTreeMap<String, String> = BTreeMap::new();
     if https_enabled {
+        config.extend(truststore_entries("ssl.server"));
         config.extend([
-            (
-                "ssl.server.truststore.location".to_string(),
-                format!("{TLS_STORE_DIR}/truststore.p12"),
-            ),
-            (
-                "ssl.server.truststore.type".to_string(),
-                "pkcs12".to_string(),
-            ),
-            (
-                "ssl.server.truststore.password".to_string(),
-                TLS_STORE_PASSWORD.to_string(),
-            ),
             (
                 "ssl.server.keystore.location".to_string(),
                 format!("{TLS_STORE_DIR}/keystore.p12"),
@@ -50,18 +39,11 @@ pub fn build(https_enabled: bool, overrides: KeyValueConfigOverrides) -> String
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::controller::build::properties::test_support::config_overrides;
+    use crate::controller::build::properties::test_support::{EMPTY_HADOOP_XML, config_overrides};
 
     #[test]
     fn disabled_https_without_overrides_renders_empty_configuration() {
-        assert_eq!(
-            build(false, config_overrides(&[])),
-            concat!(
-                "<?xml version=\"1.0\"?>\n",
-                "<configuration>\n",
-                "</configuration>"
-            )
-        );
+        assert_eq!(build(false, config_overrides(&[])), EMPTY_HADOOP_XML);
     }
 
     #[test]

rust/operator-binary/src/controller/validate.rs

@@ -21,10 +21,11 @@ use crate::{
         ValidatedCluster, ValidatedClusterConfig, ValidatedRoleConfig, ValidatedRoleGroupConfig,
     },
     crd::{HdfsNodeRole, v1alpha1},
-    hdfs_controller::CONTAINER_IMAGE_BASE_NAME,
     security::opa::HdfsOpaConfig,
 };
 
+const CONTAINER_IMAGE_BASE_NAME: &str = "hadoop";
+
 #[derive(Snafu, Debug)]
 pub enum Error {
     #[snafu(display("failed to resolve product image"))]

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

@@ -426,7 +426,7 @@ impl v1alpha1::HdfsCluster {
     ///
     /// The `validated_config` is used to extract the ports exposed by the pods.
     ///
-    /// The pod refs returned by `pod_refs` will only be able to able to access HDFS
+    /// The pod refs returned by `pod_refs` will only be able to access HDFS
     /// from inside the Kubernetes cluster. For configuring downstream clients,
     /// consider using [`Self::namenode_listener_refs`] instead.
     pub fn pod_refs(&self, role: &HdfsNodeRole) -> Result<Vec<HdfsPodRef>, Error> {
@@ -678,7 +678,7 @@ impl v1alpha1::HdfsCluster {
         }
     }
 
-    /// Deprecated required JMX metrics port name and metrics port number tuples depending on the role.
+    /// Returns the deprecated JMX metrics port name and port number tuples for the given role.
     pub fn jmx_metrics_ports(&self, role: &HdfsNodeRole) -> Vec<(String, u16)> {
         match role {
             HdfsNodeRole::Name => vec![(

rust/operator-binary/src/hdfs_controller.rs

@@ -1,6 +1,5 @@
 use std::{collections::BTreeMap, sync::Arc};
 
-use const_format::concatcp;
 use snafu::{ResultExt, Snafu};
 use stackable_operator::{
     builder::pod::{PodBuilder, security::PodSecurityContextBuilder},
@@ -59,9 +58,6 @@ use crate::{
 
 pub const RESOURCE_MANAGER_HDFS_CONTROLLER: &str = "hdfs-operator-hdfs-controller";
 pub const HDFS_CONTROLLER_NAME: &str = "hdfs-controller";
-pub const HDFS_FULL_CONTROLLER_NAME: &str = concatcp!(HDFS_CONTROLLER_NAME, '.', OPERATOR_NAME);
-
-pub const CONTAINER_IMAGE_BASE_NAME: &str = "hadoop";
 
 #[derive(Snafu, Debug, EnumDiscriminants)]
 #[strum_discriminants(derive(IntoStaticStr))]
@@ -492,7 +488,7 @@ fn rolegroup_statefulset(
     // Adds all containers and volumes to the pod builder
     // We must use the selector labels ("rolegroup_selector_labels") and not the recommended labels
     // for the ephemeral listener volumes created by this function.
-    // This is because the recommended set contains a "managed-by" label. This labels triggers
+    // This is because the recommended set contains a "managed-by" label. This label triggers
     // the cluster resources to "manage" listeners which is wrong and leads to errors.
     // The listeners are managed by the listener-operator.
     ContainerConfig::add_containers_and_volumes(

rust/operator-binary/src/main.rs

@@ -6,8 +6,9 @@ use std::sync::Arc;
 
 use anyhow::anyhow;
 use clap::Parser;
+use const_format::concatcp;
 use futures::{FutureExt, StreamExt, TryFutureExt};
-use hdfs_controller::HDFS_FULL_CONTROLLER_NAME;
+use hdfs_controller::HDFS_CONTROLLER_NAME;
 use stackable_operator::{
     YamlSchema,
     cli::{Command, RunArguments},
@@ -58,6 +59,7 @@ mod built_info {
 }
 
 pub const OPERATOR_NAME: &str = "hdfs.stackable.tech";
+pub const HDFS_FULL_CONTROLLER_NAME: &str = concatcp!(HDFS_CONTROLLER_NAME, '.', OPERATOR_NAME);
 
 #[derive(clap::Parser)]
 #[clap(about, author)]