refactor: introduce proper framework module split

Author: maltesander

rust/operator-binary/src/container.rs

@@ -53,6 +53,7 @@ use stackable_operator::{
     },
     role_utils::RoleGroupRef,
     utils::{COMMON_BASH_TRAP_FUNCTIONS, cluster_info::KubernetesClusterInfo},
+    v2::builder::pod::container::EnvVarSet,
 };
 use strum::{Display, EnumDiscriminants, IntoStaticStr};
 
@@ -218,7 +219,7 @@ impl ContainerConfig {
         rolegroup_ref: &RoleGroupRef<v1alpha1::HdfsCluster>,
         resolved_product_image: &ResolvedProductImage,
         merged_config: &AnyNodeConfig,
-        env_overrides: Option<&BTreeMap<String, String>>,
+        env_overrides: Option<&EnvVarSet>,
         zk_config_map_name: &str,
         namenode_podrefs: &[HdfsPodRef],
         labels: &Labels,
@@ -472,7 +473,7 @@ impl ContainerConfig {
         rolegroup_ref: &RoleGroupRef<v1alpha1::HdfsCluster>,
         resolved_product_image: &ResolvedProductImage,
         zookeeper_config_map_name: &str,
-        env_overrides: Option<&BTreeMap<String, String>>,
+        env_overrides: Option<&EnvVarSet>,
         merged_config: &AnyNodeConfig,
         labels: &Labels,
     ) -> Result<Container, Error> {
@@ -533,7 +534,7 @@ impl ContainerConfig {
         role_group: &str,
         resolved_product_image: &ResolvedProductImage,
         zookeeper_config_map_name: &str,
-        env_overrides: Option<&BTreeMap<String, String>>,
+        env_overrides: Option<&EnvVarSet>,
         namenode_podrefs: &[HdfsPodRef],
         merged_config: &AnyNodeConfig,
         labels: &Labels,
@@ -871,7 +872,7 @@ impl ContainerConfig {
         hdfs: &v1alpha1::HdfsCluster,
         role_group: &str,
         zookeeper_config_map_name: &str,
-        env_overrides: Option<&BTreeMap<String, String>>,
+        env_overrides: Option<&EnvVarSet>,
         resources: Option<&ResourceRequirements>,
     ) -> Result<Vec<EnvVar>, Error> {
         // Maps env var name to env var object. This allows env_overrides to work
@@ -963,11 +964,12 @@ impl ContainerConfig {
         );
 
         // Overrides need to come last
-        let mut env_override_vars: BTreeMap<String, EnvVar> =
-            Self::transform_env_overrides_to_env_vars(env_overrides)
-                .into_iter()
-                .map(|env_var| (env_var.name.clone(), env_var))
-                .collect();
+        let mut env_override_vars: BTreeMap<String, EnvVar> = env_overrides
+            .cloned()
+            .unwrap_or_default()
+            .into_iter()
+            .map(|env_var| (env_var.name.clone(), env_var))
+            .collect();
 
         env.append(&mut env_override_vars);
 
@@ -1303,22 +1305,6 @@ impl ContainerConfig {
         }
     }
 
-    /// Transform the ProductConfig map structure to a Vector of env vars.
-    fn transform_env_overrides_to_env_vars(
-        env_overrides: Option<&BTreeMap<String, String>>,
-    ) -> Vec<EnvVar> {
-        env_overrides
-            .cloned()
-            .unwrap_or_default()
-            .into_iter()
-            .map(|(k, v)| EnvVar {
-                name: k,
-                value: Some(v),
-                ..EnvVar::default()
-            })
-            .collect()
-    }
-
     /// Common shared or required container env variables.
     fn shared_env_vars(hadoop_conf_dir: &str, zk_config_map_name: &str) -> Vec<EnvVar> {
         vec![

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

@@ -121,7 +121,13 @@ spec:
     }
 
     pub fn validated_cluster() -> ValidatedCluster {
-        validate_cluster(&minimal_hdfs(), "oci.example.org", None)
-            .expect("validate should succeed for the minimal fixture")
+        validate_cluster(
+            &minimal_hdfs(),
+            "oci.example.org",
+            crate::controller::dereference::DereferencedObjects {
+                hdfs_opa_config: None,
+            },
+        )
+        .expect("validate should succeed for the minimal fixture")
     }
 }

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

@@ -205,13 +205,18 @@ pub struct ValidatedRoleConfig {
 }
 
 /// Per-rolegroup configuration: the merged CRD config plus the merged
-/// (role <- role group) `configOverrides` and `envOverrides`.
-#[derive(Clone, Debug)]
-pub struct ValidatedRoleGroupConfig {
-    /// The number of replicas (pods) for this role group, used to derive the
-    /// per-pod [`HdfsPodRef`]s via [`ValidatedCluster::pod_refs`].
-    pub replicas: u16,
-    pub config: AnyNodeConfig,
-    pub config_overrides: v1alpha1::HdfsConfigOverrides,
-    pub env_overrides: BTreeMap<String, String>,
-}
+/// (role <- role group) `configOverrides`, `envOverrides`, `cliOverrides` and
+/// `podOverrides`.
+///
+/// This is the local-`framework` [`RoleGroupConfig`](crate::framework::role_utils::RoleGroupConfig)
+/// specialised for HDFS: the validated config is the per-role [`AnyNodeConfig`],
+/// the product-specific common config is [`JavaCommonConfig`] (whose JVM-argument
+/// merge is fallible, hence the vendored framework variant), and the config
+/// overrides are [`v1alpha1::HdfsConfigOverrides`]. The `replicas` field is used
+/// to derive the per-pod [`HdfsPodRef`]s via [`ValidatedCluster::pod_refs`] and
+/// `env_overrides` is the typed [`EnvVarSet`](stackable_operator::v2::builder::pod::container::EnvVarSet).
+pub type ValidatedRoleGroupConfig = crate::framework::role_utils::RoleGroupConfig<
+    AnyNodeConfig,
+    stackable_operator::role_utils::JavaCommonConfig,
+    v1alpha1::HdfsConfigOverrides,
+>;

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

@@ -1,27 +1,33 @@
 //! The validate step in the HdfsCluster controller.
 //!
 //! Synchronously merges and validates the cluster spec into the typed [`ValidatedCluster`]
-//! consumed by `controller::build::*`. Config fragments are merged and validated via
-//! [`HdfsNodeRole::merged_config`], and the per-file `configOverrides` / `envOverrides`
-//! are merged here (role group wins).
+//! consumed by `controller::build::*`. Each role group is merged and validated via the
+//! local-`framework` [`with_validated_config`], which folds the config fragment
+//! (default <- role <- role group) together with the `configOverrides`, `envOverrides`,
+//! `cliOverrides` and `podOverrides` (role group wins) into a single
+//! [`RoleGroupConfig`](crate::framework::role_utils::RoleGroupConfig).
 
 use std::collections::BTreeMap;
 
 use snafu::{ResultExt, Snafu};
 use stackable_operator::{
     commons::product_image_selection,
-    config::merge::Merge,
-    role_utils::{GenericRoleConfig, JavaCommonConfig, Role, RoleGroup},
+    config::{fragment::FromFragment, merge::Merge},
+    role_utils::{GenericRoleConfig, JavaCommonConfig, Role},
     v2::controller_utils::{get_cluster_name, get_namespace, get_uid},
 };
 use strum::IntoEnumIterator;
 
 use crate::{
     controller::{
         ValidatedCluster, ValidatedClusterConfig, ValidatedRoleConfig, ValidatedRoleGroupConfig,
+        dereference::DereferencedObjects,
     },
-    crd::{HdfsNodeRole, v1alpha1},
-    security::opa::HdfsOpaConfig,
+    crd::{
+        AnyNodeConfig, DataNodeConfigFragment, HdfsNodeRole, JournalNodeConfigFragment,
+        NameNodeConfigFragment, v1alpha1,
+    },
+    framework::role_utils::with_validated_config,
 };
 
 const CONTAINER_IMAGE_BASE_NAME: &str = "hadoop";
@@ -48,16 +54,18 @@ pub enum Error {
         source: stackable_operator::v2::controller_utils::Error,
     },
 
-    #[snafu(display("failed to resolve and merge config for role and role group"))]
-    FailedToResolveConfig { source: crate::crd::Error },
+    #[snafu(display("failed to merge and validate the role group config"))]
+    ValidateRoleGroupConfig {
+        source: crate::framework::role_utils::Error,
+    },
 }
 
 pub fn validate_cluster(
     hdfs: &v1alpha1::HdfsCluster,
     image_repository: &str,
-    hdfs_opa_config: Option<HdfsOpaConfig>,
+    dereferenced_objects: DereferencedObjects,
 ) -> Result<ValidatedCluster, Error> {
-    let resolved_product_image = hdfs
+    let image: product_image_selection::ResolvedProductImage = hdfs
         .spec
         .image
         .resolve(
@@ -69,6 +77,7 @@ pub fn validate_cluster(
 
     let mut role_groups = BTreeMap::new();
     let mut role_configs = BTreeMap::new();
+    let cluster_name = get_cluster_name(hdfs).context(GetClusterNameSnafu)?;
 
     for hdfs_role in HdfsNodeRole::iter() {
         if let Some(GenericRoleConfig {
@@ -79,83 +88,87 @@ pub fn validate_cluster(
         }
 
         let group_configs = match hdfs_role {
-            HdfsNodeRole::Name => {
-                validate_role_group_configs(hdfs, hdfs_role, hdfs.spec.name_nodes.as_ref())?
-            }
-            HdfsNodeRole::Data => {
-                validate_role_group_configs(hdfs, hdfs_role, hdfs.spec.data_nodes.as_ref())?
-            }
-            HdfsNodeRole::Journal => {
-                validate_role_group_configs(hdfs, hdfs_role, hdfs.spec.journal_nodes.as_ref())?
-            }
+            HdfsNodeRole::Name => validate_role_group_configs(
+                hdfs.spec.name_nodes.as_ref(),
+                NameNodeConfigFragment::default_config(cluster_name.as_ref(), &hdfs_role),
+                AnyNodeConfig::Name,
+            )?,
+            HdfsNodeRole::Data => validate_role_group_configs(
+                hdfs.spec.data_nodes.as_ref(),
+                DataNodeConfigFragment::default_config(cluster_name.as_ref(), &hdfs_role),
+                AnyNodeConfig::Data,
+            )?,
+            HdfsNodeRole::Journal => validate_role_group_configs(
+                hdfs.spec.journal_nodes.as_ref(),
+                JournalNodeConfigFragment::default_config(cluster_name.as_ref(), &hdfs_role),
+                AnyNodeConfig::Journal,
+            )?,
         };
 
         role_groups.insert(hdfs_role, group_configs);
     }
 
-    let cluster_name = get_cluster_name(hdfs).context(GetClusterNameSnafu)?;
     let namespace = get_namespace(hdfs).context(GetClusterNamespaceSnafu)?;
     let uid = get_uid(hdfs).context(GetClusterUidSnafu)?;
 
     Ok(ValidatedCluster::new(
         cluster_name,
         namespace,
         uid,
-        resolved_product_image,
-        ValidatedClusterConfig::resolve(hdfs, hdfs_opa_config),
+        image,
+        ValidatedClusterConfig::resolve(hdfs, dereferenced_objects.hdfs_opa_config),
         role_groups,
         role_configs,
     ))
 }
 
 /// Validates every role group of a role into a map keyed by role group name.
 ///
+/// Each role group is merged and validated via the local-`framework`
+/// [`with_validated_config`], which folds the CRD config fragment (default <-
+/// role <- role group) plus the `configOverrides`, `envOverrides`, `cliOverrides`
+/// and `podOverrides` (role group wins) into a single
+/// [`RoleGroupConfig`](crate::framework::role_utils::RoleGroupConfig). The
+/// concrete per-role validated config is wrapped into [`AnyNodeConfig`] via `wrap`.
+///
 /// Returns an empty map if the role is not configured.
-fn validate_role_group_configs<C>(
-    hdfs: &v1alpha1::HdfsCluster,
-    hdfs_role: HdfsNodeRole,
-    role: Option<&Role<C, v1alpha1::HdfsConfigOverrides, GenericRoleConfig, JavaCommonConfig>>,
-) -> Result<BTreeMap<String, ValidatedRoleGroupConfig>, Error> {
+fn validate_role_group_configs<Config, ValidatedConfig>(
+    role: Option<&Role<Config, v1alpha1::HdfsConfigOverrides, GenericRoleConfig, JavaCommonConfig>>,
+    default_config: Config,
+    wrap: fn(ValidatedConfig) -> AnyNodeConfig,
+) -> Result<BTreeMap<String, ValidatedRoleGroupConfig>, Error>
+where
+    Config: Clone + Merge,
+    ValidatedConfig: FromFragment<Fragment = Config>,
+{
     let Some(role) = role else {
         return Ok(BTreeMap::new());
     };
 
     role.role_groups
         .iter()
         .map(|(role_group_name, role_group)| {
-            let validated =
-                validate_role_group_config(hdfs, hdfs_role, role, role_group_name, role_group)?;
+            let validated = with_validated_config::<
+                ValidatedConfig,
+                JavaCommonConfig,
+                Config,
+                GenericRoleConfig,
+                v1alpha1::HdfsConfigOverrides,
+            >(role_group, role, &default_config)
+            .context(ValidateRoleGroupConfigSnafu)?;
+
+            // Re-wrap the per-role validated config into the role-agnostic
+            // `AnyNodeConfig`; the merged overrides carry over unchanged.
+            let validated = ValidatedRoleGroupConfig {
+                replicas: validated.replicas,
+                config: wrap(validated.config),
+                config_overrides: validated.config_overrides,
+                env_overrides: validated.env_overrides,
+                cli_overrides: validated.cli_overrides,
+                pod_overrides: validated.pod_overrides,
+                product_specific_common_config: validated.product_specific_common_config,
+            };
             Ok((role_group_name.clone(), validated))
         })
         .collect()
 }
-
-/// Validates a single role group into a [`ValidatedRoleGroupConfig`]: merges and
-/// validates the CRD config via [`HdfsNodeRole::merged_config`] and merges the
-/// role-level and role-group-level `configOverrides` and `envOverrides` (the role
-/// group wins).
-fn validate_role_group_config<C>(
-    hdfs: &v1alpha1::HdfsCluster,
-    hdfs_role: HdfsNodeRole,
-    role: &Role<C, v1alpha1::HdfsConfigOverrides, GenericRoleConfig, JavaCommonConfig>,
-    role_group_name: &str,
-    role_group: &RoleGroup<C, JavaCommonConfig, v1alpha1::HdfsConfigOverrides>,
-) -> Result<ValidatedRoleGroupConfig, Error> {
-    let config = hdfs_role
-        .merged_config(hdfs, role_group_name)
-        .context(FailedToResolveConfigSnafu)?;
-
-    let mut config_overrides = role_group.config.config_overrides.clone();
-    config_overrides.merge(&role.config.config_overrides);
-
-    let mut env_overrides = BTreeMap::new();
-    env_overrides.extend(role.config.env_overrides.clone());
-    env_overrides.extend(role_group.config.env_overrides.clone());
-
-    Ok(ValidatedRoleGroupConfig {
-        replicas: role_group.replicas.unwrap_or_default(),
-        config,
-        config_overrides,
-        env_overrides,
-    })
-}

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

@@ -25,10 +25,7 @@ use stackable_operator::{
     },
     crd::listener,
     deep_merger::ObjectOverrides,
-    k8s_openapi::{
-        api::core::v1::{Pod, PodTemplateSpec},
-        apimachinery::pkg::api::resource::Quantity,
-    },
+    k8s_openapi::{api::core::v1::Pod, apimachinery::pkg::api::resource::Quantity},
     kube::{CustomResource, ResourceExt, runtime::reflector::ObjectRef},
     kvp::{LabelError, Labels},
     product_logging::{
@@ -372,44 +369,6 @@ impl v1alpha1::HdfsCluster {
         .context(MergeJvmArgumentOverridesSnafu)
     }
 
-    pub fn pod_overrides_for_role(&self, role: &HdfsNodeRole) -> Option<&PodTemplateSpec> {
-        match role {
-            HdfsNodeRole::Name => self
-                .spec
-                .name_nodes
-                .as_ref()
-                .map(|n| &n.config.pod_overrides),
-            HdfsNodeRole::Data => self
-                .spec
-                .data_nodes
-                .as_ref()
-                .map(|n| &n.config.pod_overrides),
-            HdfsNodeRole::Journal => self
-                .spec
-                .journal_nodes
-                .as_ref()
-                .map(|n| &n.config.pod_overrides),
-        }
-    }
-
-    pub fn pod_overrides_for_role_group(
-        &self,
-        role: &HdfsNodeRole,
-        role_group: &str,
-    ) -> Option<&PodTemplateSpec> {
-        match role {
-            HdfsNodeRole::Name => self
-                .namenode_rolegroup(role_group)
-                .map(|r| &r.config.pod_overrides),
-            HdfsNodeRole::Data => self
-                .datanode_rolegroup(role_group)
-                .map(|r| &r.config.pod_overrides),
-            HdfsNodeRole::Journal => self
-                .journalnode_rolegroup(role_group)
-                .map(|r| &r.config.pod_overrides),
-        }
-    }
-
     pub fn rolegroup_ref(
         &self,
         role_name: impl Into<String>,