refactor: add hdfssite to ConfigFileName, clean up constants, share object_meta builder

Author: maltesander

rust/operator-binary/src/controller.rs

@@ -289,6 +289,31 @@ impl ValidatedCluster {
     pub fn role_listener_name(&self, hive_role: &HiveRole) -> String {
         format!("{name}-{role}", name = self.name, role = hive_role)
     }
+
+    /// Returns an [`ObjectMetaBuilder`] pre-filled with the namespace, an owner reference back to
+    /// this cluster, and the recommended labels for a resource named `name` in `role_group_name`.
+    ///
+    /// Consolidates the metadata chain repeated by the child-resource builders. Call sites that
+    /// need extra labels/annotations chain them onto the returned builder.
+    pub(crate) fn object_meta(
+        &self,
+        name: impl Into<String>,
+        role_group_name: &RoleGroupName,
+    ) -> stackable_operator::builder::meta::ObjectMetaBuilder {
+        let mut builder = stackable_operator::builder::meta::ObjectMetaBuilder::new();
+        builder
+            .name_and_namespace(self)
+            .name(name)
+            .ownerreference(
+                stackable_operator::v2::builder::meta::ownerreference_from_resource(
+                    self,
+                    None,
+                    Some(true),
+                ),
+            )
+            .with_labels(self.recommended_labels(role_group_name));
+        builder
+    }
 }
 
 /// Lets [`ValidatedCluster`] stand in for the raw [`v1alpha1::HiveCluster`] when building owner
@@ -623,3 +648,43 @@ pub(crate) mod test_support {
         .expect("validate should succeed for the test fixture")
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use std::str::FromStr;
+
+    use super::{RoleGroupName, test_support::*};
+
+    const DERBY_YAML: &str = r#"
+        apiVersion: hive.stackable.tech/v1alpha1
+        kind: HiveCluster
+        metadata:
+          name: simple-hive
+          namespace: default
+          uid: 12345678-1234-1234-1234-123456789012
+        spec:
+          image:
+            productVersion: 4.2.0
+          clusterConfig:
+            metadataDatabase:
+              derby: {}
+          metastore:
+            roleGroups:
+              default:
+                replicas: 1
+        "#;
+
+    #[test]
+    fn object_meta_sets_namespace_owner_and_recommended_labels() {
+        let hive = minimal_hive(DERBY_YAML);
+        let cluster = validated_cluster(&hive);
+        let role_group_name = RoleGroupName::from_str("default").expect("valid role group name");
+
+        let meta = cluster.object_meta("test-name", &role_group_name).build();
+
+        assert_eq!(meta.name.as_deref(), Some("test-name"));
+        assert_eq!(meta.namespace.as_deref(), Some(cluster.namespace.as_ref()));
+        assert!(meta.owner_references.is_some());
+        assert!(meta.labels.is_some());
+    }
+}

rust/operator-binary/src/controller/build/command.rs

@@ -1,6 +1,6 @@
 use stackable_operator::crd::s3;
 
-use super::{opa::HiveOpaConfig, properties::ConfigFileName};
+use super::{opa::HiveOpaConfig, properties::ConfigFileName, statefulset::HDFS_CONFIG_MOUNT_DIR};
 use crate::crd::{
     STACKABLE_CONFIG_DIR, STACKABLE_CONFIG_MOUNT_DIR, STACKABLE_LOG_CONFIG_MOUNT_DIR,
     STACKABLE_TRUST_STORE, STACKABLE_TRUST_STORE_PASSWORD, v1alpha1,
@@ -13,6 +13,7 @@ pub fn build_container_command_args(
     hive_opa_config: Option<&HiveOpaConfig>,
 ) -> Vec<String> {
     let log4j2_properties = ConfigFileName::Log4j2;
+    let core_site = ConfigFileName::CoreSite;
     let mut args = vec![
         // copy config files to a writeable empty folder in order to set s3 access and secret keys
         format!("echo copying {STACKABLE_CONFIG_MOUNT_DIR} to {STACKABLE_CONFIG_DIR}"),
@@ -26,7 +27,7 @@ pub fn build_container_command_args(
         ),
         // Template config files
         format!(
-            "if test -f {STACKABLE_CONFIG_DIR}/core-site.xml; then config-utils template {STACKABLE_CONFIG_DIR}/core-site.xml; fi"
+            "if test -f {STACKABLE_CONFIG_DIR}/{core_site}; then config-utils template {STACKABLE_CONFIG_DIR}/{core_site}; fi"
         ),
         format!(
             "if test -f {STACKABLE_CONFIG_DIR}/hive-site.xml; then config-utils template {STACKABLE_CONFIG_DIR}/hive-site.xml; fi"
@@ -39,8 +40,8 @@ pub fn build_container_command_args(
 
     if hive.spec.cluster_config.hdfs.is_some() {
         args.extend([
-            format!("echo copying /stackable/mount/hdfs-config to {STACKABLE_CONFIG_DIR}"),
-            format!("cp -RL /stackable/mount/hdfs-config/* {STACKABLE_CONFIG_DIR}"),
+            format!("echo copying {HDFS_CONFIG_MOUNT_DIR} to {STACKABLE_CONFIG_DIR}"),
+            format!("cp -RL {HDFS_CONFIG_MOUNT_DIR}/* {STACKABLE_CONFIG_DIR}"),
         ]);
     }
 

rust/operator-binary/src/controller/build/config_map.rs

@@ -2,13 +2,10 @@
 
 use snafu::{ResultExt, Snafu};
 use stackable_operator::{
-    builder::{configmap::ConfigMapBuilder, meta::ObjectMetaBuilder},
+    builder::configmap::ConfigMapBuilder,
     k8s_openapi::api::core::v1::ConfigMap,
     product_logging::framework::VECTOR_CONFIG_FILE,
-    v2::{
-        builder::meta::ownerreference_from_resource,
-        config_file_writer::{PropertiesWriterError, to_hadoop_xml, to_java_properties_string},
-    },
+    v2::config_file_writer::{PropertiesWriterError, to_hadoop_xml, to_java_properties_string},
 };
 
 use crate::controller::{
@@ -63,16 +60,14 @@ pub fn build_metastore_rolegroup_config_map(
     let mut cm_builder = ConfigMapBuilder::new();
     cm_builder
         .metadata(
-            ObjectMetaBuilder::new()
-                .name_and_namespace(cluster)
-                .name(
+            cluster
+                .object_meta(
                     cluster
                         .resource_names(role_group_name)
                         .role_group_config_map()
                         .to_string(),
+                    role_group_name,
                 )
-                .ownerreference(ownerreference_from_resource(cluster, None, Some(true)))
-                .with_labels(cluster.recommended_labels(role_group_name))
                 .build(),
         )
         .add_data(

rust/operator-binary/src/controller/build/discovery.rs

@@ -1,17 +1,13 @@
-use std::str::FromStr;
-
 use snafu::{ResultExt, Snafu};
 use stackable_operator::{
-    builder::{configmap::ConfigMapBuilder, meta::ObjectMetaBuilder},
-    crd::listener::v1alpha1::Listener,
-    k8s_openapi::api::core::v1::ConfigMap,
-    kube::runtime::reflector::ObjectRef,
-    v2::builder::meta::ownerreference_from_resource,
+    builder::configmap::ConfigMapBuilder, crd::listener::v1alpha1::Listener,
+    k8s_openapi::api::core::v1::ConfigMap, kube::runtime::reflector::ObjectRef,
 };
 
 use crate::{
     controller::{
-        RoleGroupName, ValidatedCluster, build::listener::build_listener_connection_string,
+        ValidatedCluster,
+        build::{PLACEHOLDER_DISCOVERY_ROLE_GROUP, listener::build_listener_connection_string},
     },
     crd::{HiveRole, v1alpha1},
 };
@@ -64,17 +60,11 @@ fn build_discovery_configmap(
     let mut discovery_configmap = ConfigMapBuilder::new();
 
     discovery_configmap.metadata(
-        ObjectMetaBuilder::new()
-            .name_and_namespace(cluster)
-            .ownerreference(ownerreference_from_resource(cluster, None, Some(true)))
-            // Discovery is a role-level object; "discovery" is used as a placeholder role-group
-            // name for the recommended labels.
-            .with_labels(
-                cluster.recommended_labels(
-                    &RoleGroupName::from_str("discovery")
-                        .expect("'discovery' is a valid role group name"),
-                ),
-            )
+        cluster
+            // Discovery is a role-level object; the cluster name is used as the resource name
+            // (matching `name_and_namespace`) and "discovery" as a placeholder role-group name
+            // for the recommended labels.
+            .object_meta(cluster.name.to_string(), &PLACEHOLDER_DISCOVERY_ROLE_GROUP)
             .build(),
     );
 

rust/operator-binary/src/controller/build/jvm.rs

@@ -1,7 +1,7 @@
 use snafu::{OptionExt, ResultExt, Snafu};
 use stackable_operator::memory::{BinaryMultiple, MemoryQuantity};
 
-use super::properties::ConfigFileName;
+use super::{kerberos::STACKABLE_KERBEROS_DIR, properties::ConfigFileName};
 use crate::{
     controller::HiveRoleGroupConfig,
     crd::{
@@ -37,7 +37,9 @@ fn construct_jvm_args(hive: &HiveCluster, rg: &HiveRoleGroupConfig) -> Vec<Strin
     ];
 
     if hive.has_kerberos_enabled() {
-        jvm_args.push("-Djava.security.krb5.conf=/stackable/kerberos/krb5.conf".to_owned());
+        jvm_args.push(format!(
+            "-Djava.security.krb5.conf={STACKABLE_KERBEROS_DIR}/krb5.conf"
+        ));
     }
 
     // Apply the already-merged (role + role group) JVM argument overrides on top of the
@@ -123,11 +125,13 @@ mod tests {
 
         assert_eq!(
             non_heap_jvm_args,
-            "-Djava.security.properties=/stackable/config/security.properties \
-            -javaagent:/stackable/jmx/jmx_prometheus_javaagent.jar=9084:/stackable/jmx/jmx_hive_config.yaml \
-            -Djavax.net.ssl.trustStore=/stackable/truststore.p12 \
-            -Djavax.net.ssl.trustStorePassword=changeit \
+            format!(
+                "-Djava.security.properties={STACKABLE_CONFIG_DIR}/security.properties \
+            -javaagent:/stackable/jmx/jmx_prometheus_javaagent.jar={METRICS_PORT}:/stackable/jmx/jmx_hive_config.yaml \
+            -Djavax.net.ssl.trustStore={STACKABLE_TRUST_STORE} \
+            -Djavax.net.ssl.trustStorePassword={STACKABLE_TRUST_STORE_PASSWORD} \
             -Djavax.net.ssl.trustStoreType=pkcs12"
+            )
         );
         assert_eq!(hadoop_heapsize_env, "614");
     }
@@ -176,14 +180,16 @@ mod tests {
 
         assert_eq!(
             non_heap_jvm_args,
-            "-Djava.security.properties=/stackable/config/security.properties \
-            -javaagent:/stackable/jmx/jmx_prometheus_javaagent.jar=9084:/stackable/jmx/jmx_hive_config.yaml \
-            -Djavax.net.ssl.trustStore=/stackable/truststore.p12 \
-            -Djavax.net.ssl.trustStorePassword=changeit \
+            format!(
+                "-Djava.security.properties={STACKABLE_CONFIG_DIR}/security.properties \
+            -javaagent:/stackable/jmx/jmx_prometheus_javaagent.jar={METRICS_PORT}:/stackable/jmx/jmx_hive_config.yaml \
+            -Djavax.net.ssl.trustStore={STACKABLE_TRUST_STORE} \
+            -Djavax.net.ssl.trustStorePassword={STACKABLE_TRUST_STORE_PASSWORD} \
             -Djavax.net.ssl.trustStoreType=pkcs12 \
             -Dhttps.proxyHost=proxy.my.corp \
             -Djava.net.preferIPv4Stack=true \
             -Dhttps.proxyPort=1234"
+            )
         );
         assert_eq!(hadoop_heapsize_env, "34406");
     }

rust/operator-binary/src/controller/build/kerberos.rs

@@ -1,4 +1,4 @@
-use std::collections::BTreeMap;
+use std::{collections::BTreeMap, str::FromStr};
 
 use indoc::formatdoc;
 use snafu::{ResultExt, Snafu};
@@ -17,11 +17,20 @@ use stackable_operator::{
     commons::secret_class::SecretClassVolumeProvisionParts,
     kube::ResourceExt,
     utils::cluster_info::KubernetesClusterInfo,
+    v2::types::kubernetes::VolumeName,
 };
 
 use super::properties::ConfigFileName;
 use crate::crd::{HiveRole, STACKABLE_CONFIG_DIR, v1alpha1};
 
+// Typed name for the Kerberos secret-operator volume, reusing the existing `"kerberos"` string
+// value so the produced volume/mount name is unchanged.
+stackable_operator::constant!(pub(crate) KERBEROS_VOLUME_NAME: VolumeName = "kerberos");
+
+/// The directory the Kerberos secret-operator volume is mounted at. `krb5.conf` and `keytab`
+/// sub-paths are derived from this.
+pub(crate) const STACKABLE_KERBEROS_DIR: &str = "/stackable/kerberos";
+
 #[derive(Snafu, Debug)]
 #[allow(clippy::enum_variant_names)] // all variants have the same prefix: `Add`
 pub enum Error {
@@ -57,16 +66,16 @@ pub fn add_kerberos_pod_config(
         .build()
         .context(AddKerberosSecretVolumeSnafu)?;
         pb.add_volume(
-            VolumeBuilder::new("kerberos")
+            VolumeBuilder::new(&*KERBEROS_VOLUME_NAME)
                 .ephemeral(kerberos_secret_operator_volume)
                 .build(),
         )
         .context(AddVolumeSnafu)?;
-        cb.add_volume_mount("kerberos", "/stackable/kerberos")
+        cb.add_volume_mount(&*KERBEROS_VOLUME_NAME, STACKABLE_KERBEROS_DIR)
             .context(AddVolumeMountSnafu)?;
 
         // Needed env vars
-        cb.add_env_var("KRB5_CONFIG", "/stackable/kerberos/krb5.conf");
+        cb.add_env_var("KRB5_CONFIG", format!("{STACKABLE_KERBEROS_DIR}/krb5.conf"));
     }
 
     Ok(())
@@ -97,7 +106,7 @@ pub fn kerberos_config_properties(
         ),
         (
             "hive.metastore.kerberos.keytab.file".to_string(),
-            "/stackable/kerberos/keytab".to_string(),
+            format!("{STACKABLE_KERBEROS_DIR}/keytab"),
         ),
         (
             "hive.metastore.sasl.enabled".to_string(),
@@ -113,15 +122,17 @@ pub fn kerberos_container_start_commands(hive: &v1alpha1::HiveCluster) -> String
 
     let hive_site_xml = ConfigFileName::HiveSite;
     let mut args = vec![formatdoc! {"
-        export KERBEROS_REALM=$(grep -oP 'default_realm = \\K.*' /stackable/kerberos/krb5.conf)
+        export KERBEROS_REALM=$(grep -oP 'default_realm = \\K.*' {STACKABLE_KERBEROS_DIR}/krb5.conf)
         sed -i -e 's/${{env.KERBEROS_REALM}}/'\"$KERBEROS_REALM/g\" {STACKABLE_CONFIG_DIR}/{hive_site_xml}",
     }];
 
     if hive.spec.cluster_config.hdfs.is_some() {
+        let core_site_xml = ConfigFileName::CoreSite;
+        let hdfs_site_xml = ConfigFileName::HdfsSite;
         args.extend([
             formatdoc! {"
-                sed -i -e 's/${{env.KERBEROS_REALM}}/'\"$KERBEROS_REALM/g\" {STACKABLE_CONFIG_DIR}/core-site.xml
-                sed -i -e 's/${{env.KERBEROS_REALM}}/'\"$KERBEROS_REALM/g\" {STACKABLE_CONFIG_DIR}/hdfs-site.xml",
+                sed -i -e 's/${{env.KERBEROS_REALM}}/'\"$KERBEROS_REALM/g\" {STACKABLE_CONFIG_DIR}/{core_site_xml}
+                sed -i -e 's/${{env.KERBEROS_REALM}}/'\"$KERBEROS_REALM/g\" {STACKABLE_CONFIG_DIR}/{hdfs_site_xml}",
             }
         ]);
     }

rust/operator-binary/src/controller/build/listener.rs

@@ -1,14 +1,8 @@
-use std::str::FromStr;
-
 use snafu::{OptionExt, Snafu};
-use stackable_operator::{
-    builder::meta::ObjectMetaBuilder,
-    crd::listener::v1alpha1::{Listener, ListenerPort, ListenerSpec},
-    v2::builder::meta::ownerreference_from_resource,
-};
+use stackable_operator::crd::listener::v1alpha1::{Listener, ListenerPort, ListenerSpec};
 
 use crate::{
-    controller::{RoleGroupName, ValidatedCluster},
+    controller::{ValidatedCluster, build::PLACEHOLDER_LISTENER_ROLE_GROUP},
     crd::{HIVE_PORT, HIVE_PORT_NAME, HiveRole},
 };
 
@@ -63,13 +57,11 @@ pub fn build_role_listener(
 ) -> Listener {
     // The role listener is a role-level (not role-group-level) object, so there is no real
     // role-group name; "none" is used as a placeholder for the recommended labels.
-    let metadata = ObjectMetaBuilder::new()
-        .name_and_namespace(cluster)
-        .name(cluster.role_listener_name(hive_role))
-        .ownerreference(ownerreference_from_resource(cluster, None, Some(true)))
-        .with_labels(cluster.recommended_labels(
-            &RoleGroupName::from_str("none").expect("'none' is a valid role group name"),
-        ))
+    let metadata = cluster
+        .object_meta(
+            cluster.role_listener_name(hive_role),
+            &PLACEHOLDER_LISTENER_ROLE_GROUP,
+        )
         .build();
 
     let spec = ListenerSpec {

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

@@ -1,5 +1,21 @@
 //! Builders that turn a `ValidatedCluster` into Kubernetes resources.
 
+use std::str::FromStr;
+
+use stackable_operator::v2::types::operator::{ProductVersion, RoleGroupName};
+
+// Placeholder role-group name used for the recommended labels of the role-level discovery
+// `ConfigMap` (which is not tied to a single role group).
+stackable_operator::constant!(pub(crate) PLACEHOLDER_DISCOVERY_ROLE_GROUP: RoleGroupName = "discovery");
+
+// Placeholder role-group name used for the recommended labels of the role-level `Listener`
+// (which is not tied to a single role group).
+stackable_operator::constant!(pub(crate) PLACEHOLDER_LISTENER_ROLE_GROUP: RoleGroupName = "none");
+
+// Placeholder product version used for labels on PVC templates, which cannot be modified once
+// deployed. A constant value keeps the labels stable across version upgrades.
+stackable_operator::constant!(pub(crate) UNVERSIONED_PRODUCT_VERSION: ProductVersion = "none");
+
 pub mod command;
 pub mod config_map;
 pub mod discovery;