refactor: Add ValidatedCluster methods and use RoleGroupName.

Author: maltesander

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

@@ -2,11 +2,10 @@
 
 use snafu::{OptionExt, 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,
-    role_utils::RoleGroupRef,
-    v2::{builder::meta::ownerreference_from_resource, config_file_writer::PropertiesWriterError},
+    v2::{config_file_writer::PropertiesWriterError, types::operator::RoleGroupName},
 };
 
 use crate::{
@@ -17,8 +16,7 @@ use crate::{
             ssl_server,
         },
     },
-    crd::{HbaseRole, v1alpha1},
-    hbase_controller::build_recommended_labels,
+    crd::HbaseRole,
 };
 
 #[derive(Snafu, Debug)]
@@ -35,11 +33,6 @@ pub enum Error {
         role_group: String,
     },
 
-    #[snafu(display("failed to build object meta data"))]
-    ObjectMeta {
-        source: stackable_operator::builder::meta::Error,
-    },
-
     #[snafu(display("cannot build config map for role {role:?} and role group {role_group:?}"))]
     Assemble {
         source: stackable_operator::builder::configmap::Error,
@@ -53,17 +46,20 @@ type Result<T, E = Error> = std::result::Result<T, E>;
 pub fn build_rolegroup_config_map(
     cluster: &ValidatedCluster,
     role: &HbaseRole,
-    rolegroup_ref: &RoleGroupRef<v1alpha1::HbaseCluster>,
+    role_group_name: &RoleGroupName,
 ) -> Result<ConfigMap> {
-    tracing::info!("Setting up ConfigMap for {:?}", rolegroup_ref);
+    tracing::info!(
+        "Setting up ConfigMap for {role}/{role_group_name}",
+        role = role.to_string()
+    );
 
     let rg = cluster
         .role_group_configs
         .get(role)
-        .and_then(|groups| groups.get(&rolegroup_ref.role_group))
+        .and_then(|groups| groups.get(role_group_name))
         .with_context(|| MissingRoleGroupSnafu {
-            role: rolegroup_ref.role.clone(),
-            role_group: rolegroup_ref.role_group.clone(),
+            role: role.to_string(),
+            role_group: role_group_name.to_string(),
         })?;
 
     let cluster_config = &cluster.cluster_config;
@@ -101,21 +97,19 @@ pub fn build_rolegroup_config_map(
     let security_properties =
         security_properties::build(role, overrides.security_properties.clone()).with_context(
             |_| JvmSecurityPropertiesSnafu {
-                role_group: rolegroup_ref.role_group.clone(),
+                role_group: role_group_name.to_string(),
             },
         )?;
 
-    let cm_metadata = ObjectMetaBuilder::new()
-        .name_and_namespace(cluster)
-        .name(rolegroup_ref.object_name())
-        .ownerreference(ownerreference_from_resource(cluster, None, Some(true)))
-        .with_recommended_labels(&build_recommended_labels(
-            cluster,
-            &cluster.image.app_version_label_value,
-            &rolegroup_ref.role,
-            &rolegroup_ref.role_group,
-        ))
-        .context(ObjectMetaSnafu)?
+    let cm_metadata = cluster
+        .object_meta(
+            cluster
+                .resource_names(role, role_group_name)
+                .role_group_config_map()
+                .to_string(),
+            role,
+            role_group_name,
+        )
         .build();
 
     let mut builder = ConfigMapBuilder::new();
@@ -137,12 +131,14 @@ pub fn build_rolegroup_config_map(
     if let Some(log4j2_properties) = logging::build_log4j2(rg.config.logging()) {
         builder.add_data(ConfigFileName::Log4j2.to_string(), log4j2_properties);
     }
-    if let Some(vector_config) = logging::build_vector_config(rolegroup_ref, rg.config.logging()) {
+    if let Some(vector_config) =
+        logging::build_vector_config(cluster, role, role_group_name, rg.config.logging())
+    {
         builder.add_data(VECTOR_CONFIG_FILE, vector_config);
     }
 
     builder.build().with_context(|_| AssembleSnafu {
-        role: rolegroup_ref.role.clone(),
-        role_group: rolegroup_ref.role_group.clone(),
+        role: role.to_string(),
+        role_group: role_group_name.to_string(),
     })
 }

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

@@ -1,4 +1,5 @@
 use stackable_operator::{
+    kube::runtime::reflector::ObjectRef,
     memory::{BinaryMultiple, MemoryQuantity},
     product_logging::{
         self,
@@ -7,9 +8,13 @@ use stackable_operator::{
         },
     },
     role_utils::RoleGroupRef,
+    v2::types::operator::RoleGroupName,
 };
 
-use crate::crd::{Container, v1alpha1};
+use crate::{
+    controller::ValidatedCluster,
+    crd::{Container, HbaseRole},
+};
 
 pub const STACKABLE_LOG_DIR: &str = "/stackable/log";
 pub const MAX_HBASE_LOG_FILES_SIZE: MemoryQuantity = MemoryQuantity {
@@ -38,7 +43,9 @@ pub fn build_log4j2(logging: &Logging<Container>) -> Option<String> {
 ///
 /// Returns `None` when the Vector agent is disabled for this role group.
 pub fn build_vector_config(
-    rolegroup: &RoleGroupRef<v1alpha1::HbaseCluster>,
+    cluster: &ValidatedCluster,
+    role: &HbaseRole,
+    role_group_name: &RoleGroupName,
     logging: &Logging<Container>,
 ) -> Option<String> {
     if !logging.enable_vector_agent {
@@ -52,8 +59,14 @@ pub fn build_vector_config(
         _ => None,
     };
 
+    let rolegroup = RoleGroupRef {
+        cluster: ObjectRef::from_obj(cluster),
+        role: role.to_string(),
+        role_group: role_group_name.to_string(),
+    };
+
     Some(product_logging::framework::create_vector_config(
-        rolegroup,
+        &rolegroup,
         vector_log_config,
     ))
 }

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

@@ -60,6 +60,8 @@ mod tests {
 
 #[cfg(test)]
 pub(crate) mod test_support {
+    use std::str::FromStr;
+
     use stackable_operator::{
         commons::networking::DomainName, utils::cluster_info::KubernetesClusterInfo,
     };
@@ -133,6 +135,9 @@ spec:
         validated_cluster: &'a ValidatedCluster,
         role: &HbaseRole,
     ) -> &'a AnyServiceConfig {
-        &validated_cluster.role_group_configs[role]["default"].config
+        let default_role_group =
+            stackable_operator::v2::types::operator::RoleGroupName::from_str("default")
+                .expect("'default' is a valid role group name");
+        &validated_cluster.role_group_configs[role][&default_role_group].config
     }
 }

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

@@ -2,28 +2,52 @@ pub mod build;
 pub mod dereference;
 pub mod validate;
 
-use std::collections::BTreeMap;
+use std::{collections::BTreeMap, str::FromStr};
 
 use stackable_operator::{
+    builder::meta::ObjectMetaBuilder,
     commons::product_image_selection::ResolvedProductImage,
     k8s_openapi::{api::core::v1::PodTemplateSpec, apimachinery::pkg::apis::meta::v1::ObjectMeta},
     kube::Resource,
+    kvp::Labels,
     v2::{
         HasName, HasUid, NameIsValidLabelValue,
-        builder::pod::container::EnvVarSet,
+        builder::{meta::ownerreference_from_resource, pod::container::EnvVarSet},
+        kvp::label::{recommended_labels, role_group_selector},
+        role_group_utils::ResourceNames,
         types::{
             kubernetes::{NamespaceName, Uid},
-            operator::ClusterName,
+            operator::{
+                ClusterName, ControllerName, OperatorName, ProductName, ProductVersion,
+                RoleGroupName, RoleName,
+            },
         },
     },
 };
 
 use crate::{
-    crd::{AnyServiceConfig, HbaseRole, v1alpha1},
+    crd::{APP_NAME, AnyServiceConfig, HbaseRole, OPERATOR_NAME, v1alpha1},
+    hbase_controller::HBASE_CONTROLLER_NAME,
     security::opa::HbaseOpaConfig,
     zookeeper::ZookeeperConnectionInformation,
 };
 
+/// The product name (`hbase`) as a type-safe label value.
+pub(crate) fn product_name() -> ProductName {
+    ProductName::from_str(APP_NAME).expect("'hbase' is a valid product name")
+}
+
+/// The operator name as a type-safe label value.
+pub(crate) fn operator_name() -> OperatorName {
+    OperatorName::from_str(OPERATOR_NAME).expect("the operator name is a valid label value")
+}
+
+/// The controller name as a type-safe label value.
+pub(crate) fn controller_name() -> ControllerName {
+    ControllerName::from_str(HBASE_CONTROLLER_NAME)
+        .expect("the controller name is a valid label value")
+}
+
 /// The validated cluster: proves that config merging and validation succeeded for
 /// every role and role group before any resources are created.
 #[derive(Clone, Debug)]
@@ -37,8 +61,12 @@ pub struct ValidatedCluster {
     /// The UID of the `HbaseCluster` object, used to build owner references.
     pub uid: Uid,
     pub image: ResolvedProductImage,
+    /// The product version as a valid label value, used for the recommended
+    /// `app.kubernetes.io/version` label. Derived from the resolved image's app version label
+    /// value.
+    pub product_version: ProductVersion,
     pub cluster_config: ValidatedClusterConfig,
-    pub role_group_configs: BTreeMap<HbaseRole, BTreeMap<String, ValidatedRoleGroupConfig>>,
+    pub role_group_configs: BTreeMap<HbaseRole, BTreeMap<RoleGroupName, ValidatedRoleGroupConfig>>,
     pub role_configs: BTreeMap<HbaseRole, ValidatedRoleConfig>,
 }
 
@@ -50,9 +78,13 @@ impl ValidatedCluster {
         uid: Uid,
         image: ResolvedProductImage,
         cluster_config: ValidatedClusterConfig,
-        role_group_configs: BTreeMap<HbaseRole, BTreeMap<String, ValidatedRoleGroupConfig>>,
+        role_group_configs: BTreeMap<HbaseRole, BTreeMap<RoleGroupName, ValidatedRoleGroupConfig>>,
         role_configs: BTreeMap<HbaseRole, ValidatedRoleConfig>,
     ) -> Self {
+        // `app_version_label_value` is constructed to be a valid label value, so it is also a
+        // valid `ProductVersion`.
+        let product_version = ProductVersion::from_str(&image.app_version_label_value)
+            .expect("the app version label value is a valid product version");
         Self {
             metadata: ObjectMeta {
                 name: Some(name.to_string()),
@@ -63,11 +95,90 @@ impl ValidatedCluster {
             name,
             uid,
             image,
+            product_version,
             cluster_config,
             role_group_configs,
             role_configs,
         }
     }
+
+    /// The Kubernetes role name for an [`HbaseRole`] (e.g. `master`, `regionserver`,
+    /// `restserver`).
+    pub fn role_name(hbase_role: &HbaseRole) -> RoleName {
+        RoleName::from_str(&hbase_role.to_string()).expect("an HbaseRole name is a valid role name")
+    }
+
+    /// Type-safe names for the resources of a given role group.
+    pub(crate) fn resource_names(
+        &self,
+        hbase_role: &HbaseRole,
+        role_group_name: &RoleGroupName,
+    ) -> ResourceNames {
+        ResourceNames {
+            cluster_name: self.name.clone(),
+            role_name: Self::role_name(hbase_role),
+            role_group_name: role_group_name.clone(),
+        }
+    }
+
+    /// Recommended labels for a role-group resource.
+    pub fn recommended_labels(
+        &self,
+        hbase_role: &HbaseRole,
+        role_group_name: &RoleGroupName,
+    ) -> Labels {
+        recommended_labels(
+            self,
+            &product_name(),
+            &self.product_version,
+            &operator_name(),
+            &controller_name(),
+            &Self::role_name(hbase_role),
+            role_group_name,
+        )
+    }
+
+    /// Selector labels matching the pods of a role group.
+    pub fn role_group_selector(
+        &self,
+        hbase_role: &HbaseRole,
+        role_group_name: &RoleGroupName,
+    ) -> Labels {
+        role_group_selector(
+            self,
+            &product_name(),
+            &Self::role_name(hbase_role),
+            role_group_name,
+        )
+    }
+
+    /// 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>,
+        hbase_role: &HbaseRole,
+        role_group_name: &RoleGroupName,
+    ) -> ObjectMetaBuilder {
+        let mut builder = ObjectMetaBuilder::new();
+        builder
+            .name_and_namespace(self)
+            .name(name)
+            .ownerreference(ownerreference_from_resource(self, None, Some(true)))
+            .with_labels(self.recommended_labels(hbase_role, role_group_name));
+        builder
+    }
+
+    /// Whether Kerberos is enabled for this cluster.
+    ///
+    /// Mirrors [`v1alpha1::HbaseCluster::has_kerberos_enabled`], derived here from the validated
+    /// config so build steps don't have to re-read the raw cluster.
+    pub fn has_kerberos_enabled(&self) -> bool {
+        self.cluster_config.kerberos_enabled
+    }
 }
 
 impl Resource for ValidatedCluster {

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

@@ -14,6 +14,7 @@ use stackable_operator::{
         builder::pod::container::{self, EnvVarName, EnvVarSet},
         controller_utils::{get_cluster_name, get_namespace, get_uid},
         role_utils::{JavaCommonConfig, with_validated_config},
+        types::operator::RoleGroupName,
     },
 };
 use strum::IntoEnumIterator;
@@ -53,6 +54,12 @@ pub enum Error {
     #[snafu(display("invalid environment variable override name"))]
     ParseEnvVarName { source: container::Error },
 
+    #[snafu(display("invalid role group name {role_group}"))]
+    ParseRoleGroupName {
+        source: stackable_operator::v2::macros::attributed_string_type::Error,
+        role_group: String,
+    },
+
     #[snafu(display("failed to resolve kerberos config"))]
     AddKerberosConfig { source: kerberos::Error },
 }
@@ -187,7 +194,7 @@ fn validate_role_group_configs<Config, ValidatedConfig>(
     >,
     default_config: Config,
     wrap: fn(ValidatedConfig) -> AnyServiceConfig,
-) -> Result<BTreeMap<String, ValidatedRoleGroupConfig>, Error>
+) -> Result<BTreeMap<RoleGroupName, ValidatedRoleGroupConfig>, Error>
 where
     Config: Clone + Merge,
     ValidatedConfig: FromFragment<Fragment = Config>,
@@ -199,6 +206,11 @@ where
     role.role_groups
         .iter()
         .map(|(role_group_name, role_group)| {
+            let role_group_name = RoleGroupName::from_str(role_group_name).with_context(|_| {
+                ParseRoleGroupNameSnafu {
+                    role_group: role_group_name.clone(),
+                }
+            })?;
             let validated = with_validated_config::<
                 ValidatedConfig,
                 JavaCommonConfig,