refactor: switch to EnvVarSet

Author: maltesander

rust/operator-binary/src/controller.rs

@@ -196,9 +196,6 @@ pub enum Error {
         container_name: String,
     },
 
-    #[snafu(display("failed to resolve and merge config for role and role group"))]
-    FailedToResolveConfig { source: crate::crd::Error },
-
     #[snafu(display("failed to build git-sync resources"))]
     BuildGitSyncResources { source: build::git_sync::Error },
 
@@ -406,11 +403,6 @@ pub async fn reconcile_nifi(
 
             tracing::debug!("Processing rolegroup {}", rolegroup);
 
-            let merged_config = nifi
-                .merged_config(&nifi_role, rolegroup_name)
-                .context(FailedToResolveConfigSnafu)?;
-
-            // Computed here for the StatefulSet; the ConfigMap builder computes its own copy.
             let git_sync_resources =
                 build::git_sync::build_git_sync_resources(&validated_cluster, rg)
                     .context(BuildGitSyncResourcesSnafu)?;
@@ -462,7 +454,6 @@ pub async fn reconcile_nifi(
                 &rolegroup,
                 role,
                 rg,
-                &merged_config,
                 authentication_config,
                 authorization_config,
                 rolling_upgrade_supported,
@@ -617,18 +608,6 @@ pub async fn reconcile_nifi(
 
 const USERDATA_MOUNTPOINT: &str = "/stackable/userdata";
 
-/// Build a `Vec<EnvVar>` from a plain `BTreeMap<String, String>` of env overrides.
-fn env_vars_from_overrides(env_overrides: &BTreeMap<String, String>) -> Vec<EnvVar> {
-    env_overrides
-        .iter()
-        .map(|(name, value)| EnvVar {
-            name: name.clone(),
-            value: Some(value.clone()),
-            ..EnvVar::default()
-        })
-        .collect()
-}
-
 /// The rolegroup [`StatefulSet`] runs the rolegroup, as configured by the administrator.
 ///
 /// The [`Pod`](`stackable_operator::k8s_openapi::api::core::v1::Pod`)s are accessible through the
@@ -642,7 +621,6 @@ async fn build_node_rolegroup_statefulset(
     rolegroup_ref: &RoleGroupRef<v1alpha1::NifiCluster>,
     role: &NifiRoleType,
     rg: &NifiRoleGroupConfig,
-    merged_config: &NifiConfig,
     authentication_config: &NifiAuthenticationConfig,
     authorization_config: &ResolvedNifiAuthorizationConfig,
     rolling_update_supported: bool,
@@ -651,9 +629,12 @@ async fn build_node_rolegroup_statefulset(
     git_sync_resources: &git_sync::v1alpha2::GitSyncResources,
 ) -> Result<StatefulSet> {
     tracing::debug!("Building statefulset");
-    let role_group = role.role_groups.get(&rolegroup_ref.role_group);
 
-    let mut env_vars: Vec<EnvVar> = env_vars_from_overrides(&rg.env_overrides);
+    // The validated, merged `NifiConfig` is the single source of truth; the ConfigMap builder
+    // sources the same `rg.config`.
+    let merged_config = &rg.config;
+
+    let mut env_vars: Vec<EnvVar> = rg.env_overrides.clone().into();
 
     // we need the POD_NAME env var to overwrite `nifi.cluster.node.address` later
     env_vars.push(EnvVar {
@@ -1153,10 +1134,8 @@ async fn build_node_rolegroup_statefulset(
     );
 
     let mut pod_template = pod_builder.build_template();
-    pod_template.merge_from(role.config.pod_overrides.clone());
-    if let Some(role_group) = role_group {
-        pod_template.merge_from(role_group.config.pod_overrides.clone());
-    }
+    // `rg.pod_overrides` is already the role <- rolegroup merge produced by the framework.
+    pod_template.merge_from(rg.pod_overrides.clone());
 
     Ok(StatefulSet {
         metadata: ObjectMetaBuilder::new()

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

@@ -1,11 +1,11 @@
 //! Builds the git-sync resources (volumes, mounts, containers) for a NiFi Node rolegroup.
 
 use snafu::{ResultExt, Snafu};
-use stackable_operator::crd::git_sync;
+use stackable_operator::{crd::git_sync, k8s_openapi::api::core::v1::EnvVar};
 
 use crate::{
     controller::{
-        LOG_VOLUME_NAME, env_vars_from_overrides,
+        LOG_VOLUME_NAME,
         validate::{NifiRoleGroupConfig, ValidatedCluster},
     },
     crd::Container,
@@ -26,10 +26,11 @@ pub fn build_git_sync_resources(
     cluster: &ValidatedCluster,
     rg: &NifiRoleGroupConfig,
 ) -> Result<git_sync::v1alpha2::GitSyncResources> {
+    let env_vars: Vec<EnvVar> = rg.env_overrides.clone().into();
     git_sync::v1alpha2::GitSyncResources::new(
         &cluster.cluster_config.custom_components_git_sync,
         &cluster.image,
-        &env_vars_from_overrides(&rg.env_overrides),
+        &env_vars,
         &[],
         LOG_VOLUME_NAME,
         &rg.config.logging.for_container(&Container::GitSync),

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

@@ -27,7 +27,9 @@ pub fn build(rg: &NifiRoleGroupConfig) -> Result<String, PropertiesWriterError>
 mod tests {
     use std::collections::BTreeMap;
 
-    use stackable_operator::v2::config_overrides::KeyValueConfigOverrides;
+    use stackable_operator::v2::{
+        builder::pod::container::EnvVarSet, config_overrides::KeyValueConfigOverrides,
+    };
 
     use super::*;
     use crate::{
@@ -50,7 +52,7 @@ mod tests {
                 },
                 ..Default::default()
             },
-            env_overrides: BTreeMap::new(),
+            env_overrides: EnvVarSet::new(),
             cli_overrides: BTreeMap::new(),
             pod_overrides: Default::default(),
             product_specific_common_config: JavaCommonConfig::default(),

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

@@ -58,9 +58,9 @@ pub enum Error {
     #[snafu(display("invalid NiFi authentication configuration"))]
     InvalidAuthenticationConfig { source: authentication::Error },
 
-    #[snafu(display("failed to validate config fragment for a rolegroup"))]
-    InvalidConfigFragment {
-        source: stackable_operator::config::fragment::ValidationError,
+    #[snafu(display("failed to build the config for a rolegroup"))]
+    BuildRoleGroupConfig {
+        source: crate::framework::role_utils::Error,
     },
 
     #[snafu(display("invalid sensitive properties algorithm"))]
@@ -263,7 +263,7 @@ fn build_role_group_configs(
     for (rg_name, rg) in &role.role_groups {
         let validated_rg =
             with_validated_config::<NifiConfig, _, _, _, _>(rg, role, &default_config)
-                .context(InvalidConfigFragmentSnafu)?;
+                .context(BuildRoleGroupConfigSnafu)?;
         groups.insert(rg_name.clone(), validated_rg);
     }
 

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

@@ -32,7 +32,10 @@ mod tests {
     };
 
     use super::*;
-    use crate::crd::v1alpha1;
+    use crate::{
+        crd::{NifiConfig, v1alpha1},
+        framework::role_utils::with_validated_config,
+    };
 
     #[test]
     fn test_affinity_defaults() {
@@ -58,7 +61,14 @@ mod tests {
         let deserializer = serde_yaml::Deserializer::from_str(input);
         let nifi: v1alpha1::NifiCluster =
             serde_yaml::with::singleton_map_recursive::deserialize(deserializer).unwrap();
-        let merged_config = nifi.merged_config(&NifiRole::Node, "default").unwrap();
+
+        let role = nifi.spec.nodes.as_ref().unwrap();
+        let default_config = NifiConfig::default_config("simple-nifi", &NifiRole::Node);
+        let role_group = role.role_groups.get("default").unwrap();
+        let merged_config =
+            with_validated_config::<NifiConfig, _, _, _, _>(role_group, role, &default_config)
+                .unwrap()
+                .config;
 
         assert_eq!(
             merged_config.affinity,

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

@@ -7,7 +7,6 @@ use affinity::get_affinity;
 use authorization::NifiAuthorization;
 use sensitive_properties::NifiSensitivePropertiesConfig;
 use serde::{Deserialize, Serialize};
-use snafu::{OptionExt, ResultExt, Snafu};
 use stackable_operator::{
     commons::{
         affinity::StackableAffinity,
@@ -18,17 +17,14 @@ use stackable_operator::{
             PvcConfig, PvcConfigFragment, Resources, ResourcesFragment,
         },
     },
-    config::{
-        fragment::{self, Fragment, ValidationError},
-        merge::Merge,
-    },
+    config::{fragment::Fragment, merge::Merge},
     crd::{authentication::core as auth_core, git_sync},
     deep_merger::ObjectOverrides,
     k8s_openapi::{
         api::core::v1::{PodTemplateSpec, Volume},
         apimachinery::pkg::api::resource::Quantity,
     },
-    kube::{CustomResource, ResourceExt, runtime::reflector::ObjectRef},
+    kube::{CustomResource, runtime::reflector::ObjectRef},
     memory::MemoryQuantity,
     product_logging::{self, spec::Logging},
     role_utils::{GenericRoleConfig, JavaCommonConfig, Role, RoleGroupRef},
@@ -62,15 +58,6 @@ const DEFAULT_NODE_GRACEFUL_SHUTDOWN_TIMEOUT: Duration = Duration::from_minutes_
 pub type NifiRoleType =
     Role<NifiConfigFragment, v1alpha1::NifiConfigOverrides, NifiNodeRoleConfig, JavaCommonConfig>;
 
-#[derive(Snafu, Debug)]
-pub enum Error {
-    #[snafu(display("the NiFi role [{role}] is missing from spec"))]
-    MissingNifiRole { role: String },
-
-    #[snafu(display("fragment validation failure"))]
-    FragmentValidationFailure { source: ValidationError },
-}
-
 #[versioned(
     version(name = "v1alpha1"),
     crates(
@@ -230,37 +217,6 @@ impl v1alpha1::NifiCluster {
     pub fn server_tls_secret_class(&self) -> &str {
         &self.spec.cluster_config.tls.server_secret_class
     }
-
-    /// Retrieve and merge resource configs for role and role groups
-    pub fn merged_config(&self, role: &NifiRole, role_group: &str) -> Result<NifiConfig, Error> {
-        // Initialize the result with all default values as baseline
-        let conf_defaults = NifiConfig::default_config(&self.name_any(), role);
-
-        let role = self.spec.nodes.as_ref().context(MissingNifiRoleSnafu {
-            role: role.to_string(),
-        })?;
-
-        // Retrieve role resource config
-        let mut conf_role = role.config.config.to_owned();
-
-        // Retrieve rolegroup specific resource config
-        let mut conf_rolegroup = role
-            .role_groups
-            .get(role_group)
-            .map(|rg| rg.config.config.clone())
-            .unwrap_or_default();
-
-        // Merge more specific configs into default config
-        // Hierarchy is:
-        // 1. RoleGroup
-        // 2. Role
-        // 3. Default
-        conf_role.merge(&conf_defaults);
-        conf_rolegroup.merge(&conf_role);
-
-        tracing::debug!("Merged config: {:?}", conf_rolegroup);
-        fragment::validate(conf_rolegroup).context(FragmentValidationFailureSnafu)
-    }
 }
 
 #[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)]