refactor: move gitsync components to ValidatedRoleGroupConfig

Author: maltesander

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

@@ -13,7 +13,6 @@ use crate::{
     controller::{
         ValidatedCluster,
         build::{
-            git_sync,
             properties::{
                 ConfigFileName, authorizers, bootstrap_conf, login_identity_providers,
                 nifi_properties, product_logging, security_properties, state_management_xml,
@@ -58,9 +57,6 @@ pub enum Error {
 
     #[snafu(display("the cluster has no rolegroup [{role_group}] in role [{role}]"))]
     MissingRoleGroup { role: String, role_group: String },
-
-    #[snafu(display("failed to build git-sync resources"))]
-    BuildGitSyncResources { source: git_sync::Error },
 }
 
 type Result<T, E = Error> = std::result::Result<T, E>;
@@ -86,8 +82,6 @@ pub fn build_rolegroup_config_map(
         })?;
 
     let proxy_hosts = proxy_hosts::compute_proxy_hosts(cluster, cluster_info);
-    let git_sync_resources =
-        git_sync::build_git_sync_resources(cluster, rg).context(BuildGitSyncResourcesSnafu)?;
 
     let mut cm_builder = ConfigMapBuilder::new();
 
@@ -112,11 +106,11 @@ pub fn build_rolegroup_config_map(
         )
         .add_data(
             ConfigFileName::NifiProperties.to_string(),
-            nifi_properties::build(cluster, rg, &proxy_hosts, &git_sync_resources).with_context(
-                |_| BuildNifiPropertiesSnafu {
+            nifi_properties::build(cluster, rg, &proxy_hosts).with_context(|_| {
+                BuildNifiPropertiesSnafu {
                     rolegroup: role_group_name.clone(),
-                },
-            )?,
+                }
+            })?,
         )
         .add_data(
             ConfigFileName::StateManagementXml.to_string(),

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

@@ -1,13 +1,14 @@
 //! Builds the git-sync resources (volumes, mounts, containers) for a NiFi Node rolegroup.
 
 use snafu::{ResultExt, Snafu};
-use stackable_operator::{crd::git_sync, k8s_openapi::api::core::v1::EnvVar};
+use stackable_operator::{
+    commons::product_image_selection::ResolvedProductImage, crd::git_sync,
+    k8s_openapi::api::core::v1::EnvVar, v2::builder::pod::container::EnvVarSet,
+};
 
 use crate::{
-    controller::{
-        ValidatedCluster, ValidatedRoleGroupConfig, build::resource::statefulset::LOG_VOLUME_NAME,
-    },
-    crd::Container,
+    controller::build::resource::statefulset::LOG_VOLUME_NAME,
+    crd::{Container, NifiConfig},
 };
 
 #[derive(Snafu, Debug)]
@@ -21,18 +22,24 @@ type Result<T, E = Error> = std::result::Result<T, E>;
 /// Builds the [`git_sync::v1alpha2::GitSyncResources`] for a single Node rolegroup. The env vars
 /// and logging configuration differ per rolegroup, so the resources are computed per rolegroup
 /// rather than once for the whole cluster.
+///
+/// Called from the [`validate`](crate::controller::validate) step; the result is stored on the
+/// [`ValidatedRoleGroupConfig`](crate::controller::ValidatedRoleGroupConfig) and consumed by the
+/// downstream builders.
 pub fn build_git_sync_resources(
-    cluster: &ValidatedCluster,
-    rg: &ValidatedRoleGroupConfig,
+    custom_components_git_sync: &[git_sync::v1alpha2::GitSync],
+    image: &ResolvedProductImage,
+    config: &NifiConfig,
+    env_overrides: &EnvVarSet,
 ) -> Result<git_sync::v1alpha2::GitSyncResources> {
-    let env_vars: Vec<EnvVar> = rg.env_overrides.clone().into();
+    let env_vars: Vec<EnvVar> = env_overrides.clone().into();
     git_sync::v1alpha2::GitSyncResources::new(
-        &cluster.cluster_config.custom_components_git_sync,
-        &cluster.image,
+        custom_components_git_sync,
+        image,
         &env_vars,
         &[],
         LOG_VOLUME_NAME,
-        &rg.config.logging.for_container(&Container::GitSync),
+        &config.logging.for_container(&Container::GitSync),
     )
     .context(InvalidGitSyncSpecSnafu)
 }

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

@@ -130,9 +130,6 @@ pub(crate) mod test_support {
         let nifi: v1alpha1::NifiCluster =
             serde_yaml::from_str(MINIMAL_NIFI_YAML).expect("invalid test YAML");
 
-        let role_group_configs = build_role_group_configs(&nifi, &None)
-            .expect("role group configs should merge for minimal fixture");
-
         let image = ResolvedProductImage {
             product_version: "2.9.0".to_string(),
             app_version_label_value: "2.9.0".parse::<LabelValue>().unwrap(),
@@ -141,6 +138,9 @@ pub(crate) mod test_support {
             pull_secrets: None,
         };
 
+        let role_group_configs = build_role_group_configs(&nifi, &image, &None)
+            .expect("role group configs should merge for minimal fixture");
+
         let name = ClusterName::from_str("simple-nifi").expect("valid cluster name");
         let namespace = NamespaceName::from_str("default").expect("valid namespace");
         let uid = Uid::from_str("e6ac237d-a6d4-43a1-8135-f36506110912").expect("valid uid");
@@ -180,11 +180,6 @@ pub(crate) mod test_support {
                     .pod_overrides
                     .clone(),
                 host_header_check: nifi.spec.cluster_config.host_header_check.clone(),
-                custom_components_git_sync: nifi
-                    .spec
-                    .cluster_config
-                    .custom_components_git_sync
-                    .clone(),
             },
         )
     }
@@ -199,10 +194,4 @@ pub(crate) mod test_support {
             })
             .expect("minimal_validated_cluster must contain a 'default' role group")
     }
-
-    /// Build an empty [`GitSyncResources`] (no git-sync configured).
-    pub fn empty_git_sync_resources()
-    -> stackable_operator::crd::git_sync::v1alpha2::GitSyncResources {
-        stackable_operator::crd::git_sync::v1alpha2::GitSyncResources::default()
-    }
 }

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

@@ -55,7 +55,7 @@ mod tests {
 
     use super::*;
     use crate::{
-        controller::validate::build_role_group_configs,
+        controller::validate::{build_role_group_configs, test_resolved_product_image},
         crd::{NifiRole, v1alpha1},
     };
 
@@ -64,7 +64,8 @@ mod tests {
             serde_yaml::from_str(nifi_cluster).expect("illegal test input");
 
         let role_group_configs =
-            build_role_group_configs(&nifi, &None).expect("failed to build role group configs");
+            build_role_group_configs(&nifi, &test_resolved_product_image(), &None)
+                .expect("failed to build role group configs");
         let rg = role_group_configs
             .get(&NifiRole::Node)
             .and_then(|groups| {

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

@@ -3,7 +3,7 @@
 use std::collections::BTreeMap;
 
 use snafu::{ResultExt, ensure};
-use stackable_operator::{crd::git_sync, memory::MemoryQuantity};
+use stackable_operator::memory::MemoryQuantity;
 
 use super::format_properties;
 use crate::{
@@ -32,8 +32,8 @@ pub fn build(
     cluster: &ValidatedCluster,
     rg: &ValidatedRoleGroupConfig,
     proxy_hosts: &str,
-    git_sync_resources: &git_sync::v1alpha2::GitSyncResources,
 ) -> Result<String, Error> {
+    let git_sync_resources = &rg.git_sync_resources;
     let product_version = &cluster.image.product_version;
     let auth_config = &cluster.cluster_config.authentication;
     let resource_config = &rg.config.resources;
@@ -590,9 +590,7 @@ fn storage_quantity_to_nifi(quantity: MemoryQuantity) -> String {
 mod tests {
     use super::*;
     use crate::{
-        controller::build::properties::test_support::{
-            default_rg, empty_git_sync_resources, minimal_validated_cluster,
-        },
+        controller::build::properties::test_support::{default_rg, minimal_validated_cluster},
         crd::HTTPS_PORT,
     };
 
@@ -602,9 +600,8 @@ mod tests {
     fn test_stable_keys_present() {
         let cluster = minimal_validated_cluster();
         let rg = default_rg(&cluster);
-        let git_sync = empty_git_sync_resources();
 
-        let props = build(&cluster, rg, "*", &git_sync).expect("build should succeed");
+        let props = build(&cluster, rg, "*").expect("build should succeed");
 
         // HTTPS port
         assert!(
@@ -645,7 +642,7 @@ mod tests {
         use stackable_operator::v2::types::operator::RoleGroupName;
 
         use crate::{
-            controller::validate::build_role_group_configs,
+            controller::validate::{build_role_group_configs, test_resolved_product_image},
             crd::{NifiRole, v1alpha1},
         };
 
@@ -674,7 +671,8 @@ mod tests {
         "#;
         let nifi: v1alpha1::NifiCluster = serde_yaml::from_str(yaml).expect("invalid test YAML");
         let mut role_group_configs =
-            build_role_group_configs(&nifi, &None).expect("failed to build role group configs");
+            build_role_group_configs(&nifi, &test_resolved_product_image(), &None)
+                .expect("failed to build role group configs");
         let default_rg_name = "default"
             .parse::<RoleGroupName>()
             .expect("valid role-group name");
@@ -683,17 +681,21 @@ mod tests {
             .and_then(|groups| groups.remove(&default_rg_name))
             .expect("default role group must exist");
 
-        // Build a cluster with this rg substituted in
+        // Build a cluster with this rg substituted in, then borrow it back for the build call
+        // (`ValidatedRoleGroupConfig` is not `Clone`).
         let mut cluster = minimal_validated_cluster();
         cluster
             .role_group_configs
             .get_mut(&NifiRole::Node)
             .unwrap()
-            .insert(default_rg_name, rg.clone());
+            .insert(default_rg_name.clone(), rg);
+        let rg = cluster
+            .role_group_configs
+            .get(&NifiRole::Node)
+            .and_then(|groups| groups.get(&default_rg_name))
+            .expect("default role group must exist");
 
-        let git_sync = empty_git_sync_resources();
-        let props =
-            build(&cluster, &rg, "*", &git_sync).expect("build with override should succeed");
+        let props = build(&cluster, rg, "*").expect("build with override should succeed");
 
         assert!(
             props.contains("some.custom.key=some-custom-value"),

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

@@ -49,6 +49,7 @@ mod tests {
             pod_overrides: Default::default(),
             product_specific_common_config: JavaCommonConfig::default(),
             vector_container: None,
+            git_sync_resources: Default::default(),
         }
     }
 

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

@@ -14,7 +14,7 @@ use stackable_operator::{
         },
     },
     constants::RESTART_CONTROLLER_ENABLED_LABEL,
-    crd::{authentication::oidc::v1alpha1::AuthenticationProvider, git_sync},
+    crd::authentication::oidc::v1alpha1::AuthenticationProvider,
     k8s_openapi::{
         DeepMerge,
         api::{
@@ -171,7 +171,6 @@ pub(crate) async fn build_node_rolegroup_statefulset(
     rolling_update_supported: bool,
     replicas: Option<i32>,
     service_account_name: &str,
-    git_sync_resources: &git_sync::v1alpha2::GitSyncResources,
 ) -> Result<StatefulSet> {
     tracing::debug!("Building statefulset");
 
@@ -180,6 +179,7 @@ pub(crate) async fn build_node_rolegroup_statefulset(
     let resolved_product_image = &cluster.image;
     let authentication_config = &cluster.cluster_config.authentication;
     let authorization_config = &cluster.cluster_config.authorization;
+    let git_sync_resources = &rg.git_sync_resources;
 
     // Type-safe names for this role group's resources (StatefulSet, ConfigMap, headless Service).
     let resource_names = cluster.resource_names(role_group_name);

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

@@ -52,7 +52,6 @@ pub(crate) mod validate;
 /// Produced from the result of
 /// [`with_validated_config`](stackable_operator::v2::role_utils::with_validated_config) in the
 /// [`validate`] step; downstream builders consume this rather than the raw `NifiCluster`.
-#[derive(Clone, Debug)]
 pub struct ValidatedRoleGroupConfig {
     /// The desired number of replicas (defaulted to 1 during validation).
     ///
@@ -75,6 +74,11 @@ pub struct ValidatedRoleGroupConfig {
     /// ConfigMap name), validated up-front in the [`validate`] step. `None` when the Vector agent
     /// is disabled for this role group.
     pub vector_container: Option<VectorContainerLogConfig>,
+    /// The git-sync resources (containers, volumes, mounts) for this role group, resolved from the
+    /// cluster's `customComponentsGitSync` specs up-front in the [`validate`] step. The env vars and
+    /// logging config differ per role group, so these are computed per role group. Consumed by both
+    /// the StatefulSet builder and the `nifi.properties` builder.
+    pub git_sync_resources: git_sync::v1alpha2::GitSyncResources,
 }
 
 /// The validated NifiCluster: everything `reconcile_nifi` needs after dereferencing,
@@ -107,8 +111,6 @@ pub struct ValidatedClusterConfig {
     pub authentication: NifiAuthenticationConfig,
     /// The cluster authorization settings.
     pub authorization: ResolvedNifiAuthorizationConfig,
-    /// The git-sync specs, resolved into git-sync resources at build time.
-    pub custom_components_git_sync: Vec<git_sync::v1alpha2::GitSync>,
     /// The clustering backend (ZooKeeper or Kubernetes), copied from the spec.
     pub clustering_backend: v1alpha1::NifiClusteringBackend,
     /// The host-header-check config, resolved into the proxy hosts allow-list at build time.