refactor: use v2 VolumeName

Author: maltesander

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

@@ -1,4 +1,4 @@
-use std::collections::BTreeMap;
+use std::{collections::BTreeMap, str::FromStr};
 
 use snafu::{ResultExt, Snafu};
 use stackable_operator::{
@@ -13,6 +13,7 @@ use stackable_operator::{
     commons::secret_class::SecretClassVolumeProvisionParts,
     shared::time::Duration,
     utils::cluster_info::KubernetesClusterInfo,
+    v2::types::kubernetes::VolumeName,
 };
 
 use crate::{
@@ -25,8 +26,8 @@ pub const STACKABLE_KERBEROS_DIR: &str = "/stackable/kerberos";
 /// Path of the `krb5.conf` rendered into the Kerberos secret volume. Referenced both here (the
 /// `KRB5_CONFIG` env var) and by the JVM args builder.
 pub const KRB5_CONFIG_PATH: &str = const_format::concatcp!(STACKABLE_KERBEROS_DIR, "/krb5.conf");
-/// Name of the Kerberos secret volume.
-const KERBEROS_VOLUME_NAME: &str = "kerberos";
+// Name of the Kerberos secret volume.
+stackable_operator::constant!(KERBEROS_VOLUME_NAME: VolumeName = "kerberos");
 /// The RPC/data-transfer quality-of-protection level used when Kerberos is enabled.
 const PROTECTION_PRIVACY: &str = "privacy";
 
@@ -242,12 +243,12 @@ pub fn add_kerberos_pod_config(
         .build()
         .context(BuildKerberosSecretVolumeSnafu)?;
         pb.add_volume(
-            VolumeBuilder::new(KERBEROS_VOLUME_NAME)
+            VolumeBuilder::new(&*KERBEROS_VOLUME_NAME)
                 .ephemeral(kerberos_secret_operator_volume)
                 .build(),
         )
         .context(AddVolumeSnafu)?;
-        cb.add_volume_mount(KERBEROS_VOLUME_NAME, STACKABLE_KERBEROS_DIR)
+        cb.add_volume_mount(&*KERBEROS_VOLUME_NAME, STACKABLE_KERBEROS_DIR)
             .context(AddVolumeMountSnafu)?;
 
         // Needed env vars
@@ -257,7 +258,7 @@ pub fn add_kerberos_pod_config(
     if let Some(https_secret_class) = cluster.cluster_config.https_secret_class.clone() {
         // Mount TLS keystore
         pb.add_volume(
-            VolumeBuilder::new(TLS_STORE_VOLUME_NAME)
+            VolumeBuilder::new(&*TLS_STORE_VOLUME_NAME)
                 .ephemeral(
                     SecretOperatorVolumeSourceBuilder::new(
                         https_secret_class,
@@ -278,7 +279,7 @@ pub fn add_kerberos_pod_config(
                 .build(),
         )
         .context(AddVolumeSnafu)?;
-        cb.add_volume_mount(TLS_STORE_VOLUME_NAME, TLS_STORE_DIR)
+        cb.add_volume_mount(&*TLS_STORE_VOLUME_NAME, TLS_STORE_DIR)
             .context(AddVolumeMountSnafu)?;
     }
     Ok(())

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

@@ -1,6 +1,6 @@
 //! Build the listener `Volume`/`PersistentVolumeClaim` exposing a rolegroup.
 
-use std::str::FromStr;
+use std::{str::FromStr, sync::LazyLock};
 
 use snafu::{ResultExt, Snafu};
 use stackable_operator::{
@@ -21,6 +21,13 @@ use stackable_operator::{
 
 use crate::crd::{AnyServiceConfig, HbaseRole, LISTENER_VOLUME_NAME};
 
+/// The rest servers' listener `PersistentVolumeClaim` reuses the listener volume name
+/// ([`LISTENER_VOLUME_NAME`]); the claim and the volume must share a name.
+static LISTENER_PVC_NAME: LazyLock<PersistentVolumeClaimName> = LazyLock::new(|| {
+    PersistentVolumeClaimName::from_str(LISTENER_VOLUME_NAME)
+        .expect("LISTENER_VOLUME_NAME is a valid PersistentVolumeClaim name")
+});
+
 #[derive(Snafu, Debug)]
 pub enum Error {
     #[snafu(display("failed to build listener volume"))]
@@ -72,8 +79,7 @@ pub fn build_listener_pvc(
         HbaseRole::RestServer => Some(vec![listener_operator_volume_source_builder_build_pvc(
             &TypedListenerReference::ListenerClass(merged_config.listener_class()),
             recommended_labels,
-            &PersistentVolumeClaimName::from_str(LISTENER_VOLUME_NAME)
-                .expect("LISTENER_VOLUME_NAME is a valid PersistentVolumeClaim name"),
+            &LISTENER_PVC_NAME,
         )]),
     }
 }

rust/operator-binary/src/controller/build/resource/statefulset.rs

@@ -57,6 +57,14 @@ stackable_operator::constant!(HDFS_DISCOVERY_VOLUME_NAME: VolumeName = "hdfs-dis
 stackable_operator::constant!(LOG_CONFIG_VOLUME_NAME: VolumeName = "log-config");
 stackable_operator::constant!(LOG_VOLUME_NAME: VolumeName = "log");
 
+// Environment variable names set on the HBase container. Declared as typed constants (instead of
+// `EnvVarName::from_str_unsafe` at the use site) and validated by `env_var_names_are_valid`.
+stackable_operator::constant!(HBASE_CONF_DIR_ENV: EnvVarName = "HBASE_CONF_DIR");
+stackable_operator::constant!(HADOOP_CONF_DIR_ENV: EnvVarName = "HADOOP_CONF_DIR");
+stackable_operator::constant!(REGION_MOVER_OPTS_ENV: EnvVarName = "REGION_MOVER_OPTS");
+stackable_operator::constant!(RUN_REGION_MOVER_ENV: EnvVarName = "RUN_REGION_MOVER");
+stackable_operator::constant!(STACKABLE_LOG_DIR_ENV: EnvVarName = "STACKABLE_LOG_DIR");
+
 pub static CONTAINERDEBUG_LOG_DIRECTORY: std::sync::LazyLock<String> =
     std::sync::LazyLock::new(|| format!("{STACKABLE_LOG_DIR}/containerdebug"));
 
@@ -148,29 +156,17 @@ pub fn build_rolegroup_statefulset(
     };
 
     let merged_env = EnvVarSet::new()
-        .with_value(
-            &EnvVarName::from_str_unsafe("HBASE_CONF_DIR"),
-            CONFIG_DIR_NAME,
-        )
+        .with_value(&HBASE_CONF_DIR_ENV, CONFIG_DIR_NAME)
         // required by phoenix (for cases where Kerberos is enabled): see https://issues.apache.org/jira/browse/PHOENIX-2369
-        .with_value(
-            &EnvVarName::from_str_unsafe("HADOOP_CONF_DIR"),
-            CONFIG_DIR_NAME,
-        )
+        .with_value(&HADOOP_CONF_DIR_ENV, CONFIG_DIR_NAME)
         .merge(validated_rg_config.env_overrides.clone())
         // These env vars are set for all roles to avoid bash's "unbound variable" errors.
+        .with_value(&REGION_MOVER_OPTS_ENV, merged_config.region_mover_args())
         .with_value(
-            &EnvVarName::from_str_unsafe("REGION_MOVER_OPTS"),
-            merged_config.region_mover_args(),
-        )
-        .with_value(
-            &EnvVarName::from_str_unsafe("RUN_REGION_MOVER"),
+            &RUN_REGION_MOVER_ENV,
             merged_config.run_region_mover().to_string(),
         )
-        .with_value(
-            &EnvVarName::from_str_unsafe("STACKABLE_LOG_DIR"),
-            STACKABLE_LOG_DIR,
-        );
+        .with_value(&STACKABLE_LOG_DIR_ENV, STACKABLE_LOG_DIR);
 
     let role_name = hbase_role.cli_role_name();
     let mut hbase_container = new_container_builder(&HBASE_CONTAINER_NAME);
@@ -353,3 +349,20 @@ fn command() -> Vec<String> {
         "-c".to_string(),
     ]
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// The env-var-name constants are built with `EnvVarName::from_str`, which panics on an invalid
+    /// name. This test forces every constant to be evaluated so a typo is caught at test time rather
+    /// than during reconciliation.
+    #[test]
+    fn env_var_names_are_valid() {
+        assert_eq!(HBASE_CONF_DIR_ENV.to_string(), "HBASE_CONF_DIR");
+        assert_eq!(HADOOP_CONF_DIR_ENV.to_string(), "HADOOP_CONF_DIR");
+        assert_eq!(REGION_MOVER_OPTS_ENV.to_string(), "REGION_MOVER_OPTS");
+        assert_eq!(RUN_REGION_MOVER_ENV.to_string(), "RUN_REGION_MOVER");
+        assert_eq!(STACKABLE_LOG_DIR_ENV.to_string(), "STACKABLE_LOG_DIR");
+    }
+}

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

@@ -29,7 +29,7 @@ use stackable_operator::{
         role_utils::JavaCommonConfig,
         types::{
             common::Port,
-            kubernetes::{ConfigMapName, ListenerClassName, SecretClassName},
+            kubernetes::{ConfigMapName, ListenerClassName, SecretClassName, VolumeName},
         },
     },
     versioned::versioned,
@@ -50,7 +50,7 @@ pub const OPERATOR_NAME: &str = "hbase.stackable.com";
 pub const CONFIG_DIR_NAME: &str = "/stackable/conf";
 
 pub const TLS_STORE_DIR: &str = "/stackable/tls";
-pub const TLS_STORE_VOLUME_NAME: &str = "tls";
+stackable_operator::constant!(pub TLS_STORE_VOLUME_NAME: VolumeName = "tls");
 pub const TLS_STORE_PASSWORD: &str = "changeit";
 /// The key- and truststore type used for all HBase TLS stores.
 pub const TLS_STORE_TYPE: &str = "pkcs12";