refactor: remove calulcated properties from Validated* structs

Author: maltesander

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

@@ -129,7 +129,7 @@ mod tests {
         let region_server = &validated_cluster.role_group_configs[&HbaseRole::RegionServer]
             [&test_utils::role_group_name("default")];
 
-        let global_jvm_args = construct_global_jvm_args(hbase.has_kerberos_enabled());
+        let global_jvm_args = construct_global_jvm_args(validated_cluster.has_kerberos_enabled());
         let hbase_heapsize_env =
             construct_hbase_heapsize_env(&region_server.config.config).unwrap();
 
@@ -189,7 +189,7 @@ mod tests {
         let region_server = &validated_cluster.role_group_configs[&HbaseRole::RegionServer]
             [&test_utils::role_group_name("default")];
 
-        let global_jvm_args = construct_global_jvm_args(hbase.has_kerberos_enabled());
+        let global_jvm_args = construct_global_jvm_args(validated_cluster.has_kerberos_enabled());
         let hbase_heapsize_env =
             construct_hbase_heapsize_env(&region_server.config.config).unwrap();
 

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

@@ -51,6 +51,58 @@ pub enum Error {
     },
 }
 
+/// The `hbase-site.xml` Kerberos properties for `cluster`, gated on Kerberos being enabled
+/// (empty when disabled). Derived in the build step from the validated cluster.
+pub fn hbase_site_kerberos_config(
+    cluster: &ValidatedCluster,
+    cluster_info: &KubernetesClusterInfo,
+) -> BTreeMap<String, String> {
+    if cluster.has_kerberos_enabled() {
+        kerberos_config_properties(
+            cluster.name.as_ref(),
+            cluster.namespace.as_ref(),
+            cluster_info,
+        )
+    } else {
+        BTreeMap::new()
+    }
+}
+
+/// The Kerberos properties for the discovery `hbase-site.xml` exposed to clients, gated on
+/// Kerberos being enabled (empty when disabled).
+pub fn discovery_kerberos_config(
+    cluster: &ValidatedCluster,
+    cluster_info: &KubernetesClusterInfo,
+) -> BTreeMap<String, String> {
+    if cluster.has_kerberos_enabled() {
+        kerberos_discovery_config_properties(
+            cluster.name.as_ref(),
+            cluster.namespace.as_ref(),
+            cluster_info,
+        )
+    } else {
+        BTreeMap::new()
+    }
+}
+
+/// The `ssl-server.xml` settings for `cluster`, gated on HTTPS being enabled (empty when disabled).
+pub fn ssl_server_settings(cluster: &ValidatedCluster) -> BTreeMap<String, String> {
+    if cluster.has_https_enabled() {
+        kerberos_ssl_server_settings()
+    } else {
+        BTreeMap::new()
+    }
+}
+
+/// The `ssl-client.xml` settings for `cluster`, gated on HTTPS being enabled (empty when disabled).
+pub fn ssl_client_settings(cluster: &ValidatedCluster) -> BTreeMap<String, String> {
+    if cluster.has_https_enabled() {
+        kerberos_ssl_client_settings()
+    } else {
+        BTreeMap::new()
+    }
+}
+
 pub fn kerberos_config_properties(
     hbase_name: &str,
     hbase_namespace: &str,

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

@@ -5,6 +5,7 @@ use stackable_operator::{
     builder::configmap::ConfigMapBuilder,
     k8s_openapi::api::core::v1::ConfigMap,
     product_logging::framework::VECTOR_CONFIG_FILE,
+    utils::cluster_info::KubernetesClusterInfo,
     v2::{config_file_writer::PropertiesWriterError, types::operator::RoleGroupName},
 };
 
@@ -13,6 +14,7 @@ use crate::{
         ValidatedCluster,
         build::{
             jvm::construct_role_specific_non_heap_jvm_args,
+            kerberos,
             properties::{
                 ConfigFileName, hbase_env, hbase_site, logging, security_properties, ssl_client,
                 ssl_server,
@@ -48,6 +50,7 @@ type Result<T, E = Error> = std::result::Result<T, E>;
 
 pub fn build_rolegroup_config_map(
     cluster: &ValidatedCluster,
+    cluster_info: &KubernetesClusterInfo,
     role: &HbaseRole,
     role_group_name: &RoleGroupName,
 ) -> Result<ConfigMap> {
@@ -75,7 +78,7 @@ pub fn build_rolegroup_config_map(
         cluster_config
             .zookeeper_connection_information
             .as_hbase_settings(),
-        cluster_config.hbase_site_kerberos_config.clone(),
+        kerberos::hbase_site_kerberos_config(cluster, cluster_info),
         cluster_config.hbase_opa_config.as_ref(),
         overrides.hbase_site_xml.clone(),
     );
@@ -90,11 +93,11 @@ pub fn build_rolegroup_config_map(
     .context(BuildHbaseEnvSnafu)?;
 
     let ssl_server_xml = ssl_server::build(
-        cluster_config.ssl_server_settings.clone(),
+        kerberos::ssl_server_settings(cluster),
         overrides.ssl_server_xml.clone(),
     );
     let ssl_client_xml = ssl_client::build(
-        cluster_config.ssl_client_settings.clone(),
+        kerberos::ssl_client_settings(cluster),
         overrides.ssl_client_xml.clone(),
     );
 

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

@@ -6,11 +6,15 @@ use stackable_operator::{
     k8s_openapi::api::core::v1::ConfigMap,
     kube::Resource,
     kvp::ObjectLabels,
+    utils::cluster_info::KubernetesClusterInfo,
     v2::{builder::meta::ownerreference_from_resource, config_file_writer::to_hadoop_xml},
 };
 
 use crate::{
-    controller::{ValidatedCluster, build::properties::ConfigFileName},
+    controller::{
+        ValidatedCluster,
+        build::{kerberos, properties::ConfigFileName},
+    },
     crd::{APP_NAME, HbaseRole, OPERATOR_NAME},
 };
 
@@ -34,13 +38,16 @@ pub enum Error {
 }
 
 /// Creates a discovery config map containing the `hbase-site.xml` for clients.
-pub fn build_discovery_config_map(cluster: &ValidatedCluster) -> Result<ConfigMap> {
+pub fn build_discovery_config_map(
+    cluster: &ValidatedCluster,
+    cluster_info: &KubernetesClusterInfo,
+) -> Result<ConfigMap> {
     let cluster_config = &cluster.cluster_config;
 
     let mut hbase_site = cluster_config
         .zookeeper_connection_information
         .as_hbase_settings();
-    hbase_site.extend(cluster_config.discovery_kerberos_config.clone());
+    hbase_site.extend(kerberos::discovery_kerberos_config(cluster, cluster_info));
 
     ConfigMapBuilder::new()
         .metadata(

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

@@ -179,9 +179,6 @@ impl ValidatedCluster {
     }
 
     /// 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_secret_class.is_some()
     }
@@ -252,15 +249,6 @@ pub struct ValidatedClusterConfig {
     pub https_secret_class: Option<SecretClassName>,
     /// The HDFS discovery ConfigMap name the cluster connects to.
     pub hdfs_config_map_name: ConfigMapName,
-    // Pre-resolved kerberos properties for hbase-site.xml (empty when kerberos is disabled).
-    pub hbase_site_kerberos_config: BTreeMap<String, String>,
-    // Pre-resolved kerberos properties for the discovery `hbase-site.xml` exposed to clients
-    // (empty when kerberos is disabled).
-    pub discovery_kerberos_config: BTreeMap<String, String>,
-    // Pre-resolved ssl-server.xml settings (empty when HTTPS is disabled).
-    pub ssl_server_settings: BTreeMap<String, String>,
-    // Pre-resolved ssl-client.xml settings (empty when HTTPS is disabled).
-    pub ssl_client_settings: BTreeMap<String, String>,
     // Pre-resolved zookeeper connection settings.
     pub zookeeper_connection_information: ZookeeperConnectionInformation,
 }

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

@@ -10,7 +10,6 @@ use stackable_operator::{
     kube::ResourceExt,
     product_logging::spec::Logging,
     role_utils::{CommonConfiguration, GenericRoleConfig, Role},
-    utils::cluster_info::KubernetesClusterInfo,
     v2::{
         builder::pod::container::{self, EnvVarName, EnvVarSet},
         controller_utils::{get_cluster_name, get_namespace, get_uid},
@@ -27,12 +26,7 @@ use strum::IntoEnumIterator;
 use crate::{
     controller::{
         HbaseRoleGroupConfig, ValidatedCluster, ValidatedClusterConfig, ValidatedHbaseConfig,
-        ValidatedRoleConfig,
-        build::kerberos::{
-            kerberos_config_properties, kerberos_discovery_config_properties,
-            kerberos_ssl_client_settings, kerberos_ssl_server_settings,
-        },
-        dereference::DereferencedObjects,
+        ValidatedRoleConfig, dereference::DereferencedObjects,
     },
     crd::{
         AnyServiceConfig, Container, HbaseConfigFragment, HbaseRole, RegionServerConfigFragment,
@@ -122,7 +116,6 @@ fn validate_logging(
 pub fn validate_cluster(
     hbase: &v1alpha1::HbaseCluster,
     image_repository: &str,
-    cluster_info: &KubernetesClusterInfo,
     dereferenced_objects: DereferencedObjects,
 ) -> Result<ValidatedCluster, Error> {
     let resolved_product_image = hbase
@@ -211,32 +204,6 @@ pub fn validate_cluster(
     let namespace = get_namespace(hbase).context(GetClusterIdentitySnafu)?;
     let uid = get_uid(hbase).context(GetClusterIdentitySnafu)?;
 
-    let kerberos_enabled = hbase.has_kerberos_enabled();
-    let https_enabled = hbase.has_https_enabled();
-
-    // Kerberos- and TLS-related properties, pre-resolved here so the build step stays a pure
-    // function of `ValidatedCluster` (empty when the respective feature is disabled).
-    let hbase_site_kerberos_config = if kerberos_enabled {
-        kerberos_config_properties(name.as_ref(), namespace.as_ref(), cluster_info)
-    } else {
-        BTreeMap::new()
-    };
-    let discovery_kerberos_config = if kerberos_enabled {
-        kerberos_discovery_config_properties(name.as_ref(), namespace.as_ref(), cluster_info)
-    } else {
-        BTreeMap::new()
-    };
-    let ssl_server_settings = if https_enabled {
-        kerberos_ssl_server_settings()
-    } else {
-        BTreeMap::new()
-    };
-    let ssl_client_settings = if https_enabled {
-        kerberos_ssl_client_settings()
-    } else {
-        BTreeMap::new()
-    };
-
     Ok(ValidatedCluster::new(
         name,
         namespace,
@@ -248,10 +215,6 @@ pub fn validate_cluster(
             kerberos_secret_class: hbase.kerberos_secret_class(),
             https_secret_class: hbase.https_secret_class(),
             hdfs_config_map_name: hbase.spec.cluster_config.hdfs_config_map_name.clone(),
-            hbase_site_kerberos_config,
-            discovery_kerberos_config,
-            ssl_server_settings,
-            ssl_client_settings,
         },
         role_groups,
         role_configs,

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

@@ -214,10 +214,6 @@ impl v1alpha1::HbaseCluster {
         }
     }
 
-    pub fn has_kerberos_enabled(&self) -> bool {
-        self.kerberos_secret_class().is_some()
-    }
-
     pub fn kerberos_secret_class(&self) -> Option<SecretClassName> {
         self.spec
             .cluster_config
@@ -227,10 +223,6 @@ impl v1alpha1::HbaseCluster {
             .map(|k| k.secret_class.clone())
     }
 
-    pub fn has_https_enabled(&self) -> bool {
-        self.https_secret_class().is_some()
-    }
-
     pub fn https_secret_class(&self) -> Option<SecretClassName> {
         self.spec
             .cluster_config

rust/operator-binary/src/hbase_controller.rs

@@ -167,7 +167,6 @@ pub async fn reconcile_hbase(
     let validated_cluster = crate::controller::validate::validate_cluster(
         hbase,
         &ctx.operator_environment.image_repository,
-        &client.kubernetes_cluster_info,
         dereferenced_objects,
     )
     .context(ValidateSnafu)?;
@@ -210,9 +209,13 @@ pub async fn reconcile_hbase(
             let rg_metrics_service =
                 build_rolegroup_metrics_service(&validated_cluster, hbase_role, role_group_name);
 
-            let rg_configmap =
-                build_rolegroup_config_map(&validated_cluster, hbase_role, role_group_name)
-                    .context(BuildRolegroupConfigMapSnafu)?;
+            let rg_configmap = build_rolegroup_config_map(
+                &validated_cluster,
+                &client.kubernetes_cluster_info,
+                hbase_role,
+                role_group_name,
+            )
+            .context(BuildRolegroupConfigMapSnafu)?;
             let rg_statefulset = build_rolegroup_statefulset(
                 &validated_cluster,
                 hbase_role,
@@ -268,7 +271,8 @@ pub async fn reconcile_hbase(
     // Discovery CM will fail to build until the rest of the cluster has been deployed, so do it last
     // so that failure won't inhibit the rest of the cluster from booting up.
     let discovery_cm =
-        build_discovery_config_map(&validated_cluster).context(BuildDiscoveryConfigMapSnafu)?;
+        build_discovery_config_map(&validated_cluster, &client.kubernetes_cluster_info)
+            .context(BuildDiscoveryConfigMapSnafu)?;
     cluster_resources
         .add(client, discovery_cm)
         .await

rust/operator-binary/src/test_utils.rs

@@ -7,10 +7,7 @@
 
 use std::str::FromStr;
 
-use stackable_operator::{
-    commons::networking::DomainName, utils::cluster_info::KubernetesClusterInfo,
-    v2::types::operator::RoleGroupName,
-};
+use stackable_operator::v2::types::operator::RoleGroupName;
 
 use crate::{
     controller::{
@@ -59,19 +56,12 @@ pub fn minimal_hbase() -> v1alpha1::HbaseCluster {
     hbase_from_yaml(MINIMAL_HBASE_YAML)
 }
 
-pub fn cluster_info() -> KubernetesClusterInfo {
-    KubernetesClusterInfo {
-        cluster_domain: DomainName::try_from("cluster.local").expect("valid domain"),
-    }
-}
-
 /// Runs the real validation pipeline over `hbase`, with a fixed dereferenced ZooKeeper connection
 /// and no OPA.
 pub fn validated_cluster_from(hbase: &v1alpha1::HbaseCluster) -> ValidatedCluster {
     validate_cluster(
         hbase,
         "oci.example.org",
-        &cluster_info(),
         DereferencedObjects {
             zookeeper_connection_information: ZookeeperConnectionInformation::for_tests(),
             hbase_opa_config: None,