refactor: introduce ValidatedLoggingConfig

Author: maltesander

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

@@ -2,11 +2,8 @@
 
 use stackable_operator::{
     memory::BinaryMultiple,
-    product_logging::{
-        self,
-        spec::{ContainerLogConfig, ContainerLogConfigChoice, Logging},
-    },
-    v2::product_logging::framework::STACKABLE_LOG_DIR,
+    product_logging,
+    v2::product_logging::framework::{STACKABLE_LOG_DIR, ValidatedContainerLogConfigChoice},
 };
 
 use crate::crd::{Container, MAX_NIFI_LOG_FILES_SIZE};
@@ -49,12 +46,10 @@ const ADDITIONAL_LOGBACK_CONFIG: &str = r#"  <appender name="PASSTHROUGH" class=
 ///
 /// Returns `None` when the container uses a custom log ConfigMap instead of the operator's
 /// automatic logging configuration.
-pub fn build_logback_config(logging: &Logging<Container>) -> Option<String> {
-    let ContainerLogConfig {
-        choice: Some(ContainerLogConfigChoice::Automatic(log_config)),
-    } = logging.containers.get(&Container::Nifi)?
-    else {
-        return None;
+pub fn build_logback_config(nifi_container: &ValidatedContainerLogConfigChoice) -> Option<String> {
+    let log_config = match nifi_container {
+        ValidatedContainerLogConfigChoice::Automatic(log_config) => log_config,
+        ValidatedContainerLogConfigChoice::Custom(_) => return None,
     };
 
     Some(product_logging::framework::create_logback_config(

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

@@ -30,14 +30,15 @@ mod tests {
 
     use super::*;
     use crate::{
-        controller::ValidatedRoleGroupConfig,
+        controller::{ValidatedLogging, ValidatedRoleGroupConfig},
         crd::{NifiConfig, v1alpha1::NifiConfigOverrides},
     };
 
     fn make_rg(overrides: Option<BTreeMap<String, String>>) -> ValidatedRoleGroupConfig {
         use std::str::FromStr as _;
 
         use stackable_operator::v2::{
+            product_logging::framework::ValidatedContainerLogConfigChoice,
             role_utils::JavaCommonConfig, types::operator::RoleGroupName,
         };
         ValidatedRoleGroupConfig {
@@ -53,7 +54,11 @@ mod tests {
             env_overrides: EnvVarSet::new(),
             pod_overrides: Default::default(),
             product_specific_common_config: JavaCommonConfig::default(),
-            vector_container: None,
+            logging: ValidatedLogging {
+                nifi_container: ValidatedContainerLogConfigChoice::Automatic(Default::default()),
+                vector_container: None,
+                enable_vector_agent: false,
+            },
             git_sync_resources: Default::default(),
         }
     }

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

@@ -113,11 +113,12 @@ pub fn build_rolegroup_config_map(
             })?,
         );
 
-    if let Some(logback_config) = product_logging::build_logback_config(&rg.config.logging) {
+    if let Some(logback_config) = product_logging::build_logback_config(&rg.logging.nifi_container)
+    {
         cm_builder.add_data(ConfigFileName::Logback.to_string(), logback_config);
     }
 
-    if rg.config.logging.enable_vector_agent {
+    if rg.logging.enable_vector_agent {
         cm_builder.add_data(
             VECTOR_CONFIG_FILE,
             product_logging::vector_config_file_content(),

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

@@ -31,15 +31,14 @@ use stackable_operator::{
     product_logging::{
         self,
         framework::{create_vector_shutdown_file_command, remove_vector_shutdown_file_command},
-        spec::{
-            ConfigMapLogConfig, ContainerLogConfig, ContainerLogConfigChoice,
-            CustomContainerLogConfig,
-        },
+        spec::{ContainerLogConfig, ContainerLogConfigChoice},
     },
     utils::{COMMON_BASH_TRAP_FUNCTIONS, cluster_info::KubernetesClusterInfo},
     v2::{
         builder::pod::container::{EnvVarSet, new_container_builder},
-        product_logging::framework::{STACKABLE_LOG_DIR, vector_container},
+        product_logging::framework::{
+            STACKABLE_LOG_DIR, ValidatedContainerLogConfigChoice, vector_container,
+        },
         types::kubernetes::{ContainerName, VolumeName},
     },
 };
@@ -502,41 +501,30 @@ pub(crate) async fn build_node_rolegroup_statefulset(
         .add_volumes(git_sync_resources.git_ca_cert_volumes.to_owned())
         .context(AddVolumeSnafu)?;
 
-    if let Some(ContainerLogConfig {
-        choice:
-            Some(ContainerLogConfigChoice::Custom(CustomContainerLogConfig {
-                custom: ConfigMapLogConfig { config_map },
-            })),
-    }) = merged_config.logging.containers.get(&Container::Nifi)
-    {
-        pod_builder
-            .add_volume(Volume {
-                name: LOG_CONFIG_VOLUME_NAME.to_string(),
-                config_map: Some(ConfigMapVolumeSource {
-                    name: config_map.clone(),
-                    ..ConfigMapVolumeSource::default()
-                }),
-                ..Volume::default()
-            })
-            .context(AddVolumeSnafu)?;
-    } else {
-        pod_builder
-            .add_volume(Volume {
-                name: LOG_CONFIG_VOLUME_NAME.to_string(),
-                config_map: Some(ConfigMapVolumeSource {
-                    name: resource_names.role_group_config_map().to_string(),
-                    ..ConfigMapVolumeSource::default()
-                }),
-                ..Volume::default()
-            })
-            .context(AddVolumeSnafu)?;
-    }
+    // The NiFi `log-config` volume sources from the custom log ConfigMap when one is configured,
+    // otherwise from this rolegroup's ConfigMap (which carries the operator-generated `logback.xml`).
+    let log_config_map_name = match &rg.logging.nifi_container {
+        ValidatedContainerLogConfigChoice::Custom(config_map) => config_map.to_string(),
+        ValidatedContainerLogConfigChoice::Automatic(_) => {
+            resource_names.role_group_config_map().to_string()
+        }
+    };
+    pod_builder
+        .add_volume(Volume {
+            name: LOG_CONFIG_VOLUME_NAME.to_string(),
+            config_map: Some(ConfigMapVolumeSource {
+                name: log_config_map_name,
+                ..ConfigMapVolumeSource::default()
+            }),
+            ..Volume::default()
+        })
+        .context(AddVolumeSnafu)?;
 
     // The Vector logging config was validated up-front in the `validate` step. The static
     // `vector.yaml` is shipped in the rolegroup `ConfigMap`; the per-rolegroup values (namespace,
     // cluster/role/role-group, aggregator address, log levels) are injected as environment
     // variables here and substituted by Vector at runtime.
-    if let Some(vector_log_config) = &rg.vector_container {
+    if let Some(vector_log_config) = &rg.logging.vector_container {
         pod_builder.add_container(vector_container(
             &VECTOR_CONTAINER_NAME,
             resolved_product_image,

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

@@ -17,7 +17,7 @@ use stackable_operator::{
         HasName, HasUid, NameIsValidLabelValue,
         builder::pod::container::EnvVarSet,
         kvp::label::{recommended_labels, role_group_selector, role_selector},
-        product_logging::framework::VectorContainerLogConfig,
+        product_logging::framework::{ValidatedContainerLogConfigChoice, VectorContainerLogConfig},
         role_group_utils::ResourceNames,
         role_utils::JavaCommonConfig,
         types::{
@@ -74,17 +74,33 @@ pub struct ValidatedRoleGroupConfig {
     /// The merged (role <- role group) JVM argument overrides, applied on top of the
     /// operator-generated JVM arguments when building `bootstrap.conf`.
     pub product_specific_common_config: JavaCommonConfig,
-    /// The validated Vector container logging config (log config choice + aggregator discovery
-    /// 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 validated logging configuration (NiFi and optional Vector container), validated up-front
+    /// in the [`validate`] step (mirroring hive/opensearch).
+    pub logging: ValidatedLogging,
     /// 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,
 }
 
+/// Validated logging configuration for the NiFi and (optional) Vector container.
+///
+/// Produced up-front by the [`validate`] step (mirroring hive/opensearch) so that an invalid custom
+/// log ConfigMap name, or a missing Vector aggregator discovery ConfigMap name, fails reconciliation
+/// during validation rather than at resource-build time.
+#[derive(Clone, Debug)]
+pub struct ValidatedLogging {
+    /// The NiFi container log config choice (automatic logging vs a custom log ConfigMap). Consumed
+    /// by the `logback.xml` builder and the StatefulSet's `log-config` volume.
+    pub nifi_container: ValidatedContainerLogConfigChoice,
+    /// The Vector container log config (log config choice + aggregator discovery ConfigMap name).
+    /// `None` when the Vector agent is disabled for this role group.
+    pub vector_container: Option<VectorContainerLogConfig>,
+    /// Whether the Vector log agent is enabled for this role group.
+    pub enable_vector_agent: bool,
+}
+
 /// The validated NifiCluster: everything `reconcile_nifi` needs after dereferencing,
 /// in fail-safe / resolved form. This is the single resolved representation of the cluster;
 /// downstream builders should source everything from here and never touch the raw `NifiCluster`.

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

@@ -11,6 +11,7 @@ use stackable_operator::{
     commons::product_image_selection,
     config::fragment,
     kube::ResourceExt as _,
+    product_logging::spec::Logging,
     role_utils::CommonConfiguration,
     v2::{
         builder::pod::container::{self, EnvVarName, EnvVarSet},
@@ -28,7 +29,8 @@ use stackable_operator::{
 use strum::{EnumDiscriminants, IntoStaticStr};
 
 use super::{
-    ValidatedCluster, ValidatedClusterConfig, ValidatedRoleConfig, ValidatedRoleGroupConfig,
+    ValidatedCluster, ValidatedClusterConfig, ValidatedLogging, ValidatedRoleConfig,
+    ValidatedRoleGroupConfig,
 };
 use crate::{
     controller::{build::git_sync::build_git_sync_resources, dereference::DereferencedObjects},
@@ -236,24 +238,11 @@ pub(crate) fn build_role_group_configs(
             );
         }
 
-        // Validate the Vector container logging config up-front (mirroring the opensearch- and
-        // hive-operators) so an invalid log ConfigMap name, or a missing aggregator discovery
-        // ConfigMap name, fails before any resources are built.
-        let vector_container = if config.logging.enable_vector_agent {
-            let vector_aggregator_config_map_name = vector_aggregator_config_map_name
-                .clone()
-                .context(MissingVectorAggregatorConfigMapNameSnafu)?;
-            Some(VectorContainerLogConfig {
-                log_config: validate_logging_configuration_for_container(
-                    &config.logging,
-                    &Container::Vector,
-                )
-                .context(ValidateLoggingConfigSnafu)?,
-                vector_aggregator_config_map_name,
-            })
-        } else {
-            None
-        };
+        // Validate the logging config (NiFi + optional Vector container) up-front (mirroring the
+        // opensearch- and hive-operators) so an invalid custom log ConfigMap name, or a missing
+        // Vector aggregator discovery ConfigMap name, fails during validation rather than at
+        // resource-build time.
+        let logging = validate_logging(&config.logging, vector_aggregator_config_map_name)?;
 
         // The git-sync resources depend on this role group's env-var overrides and logging config,
         // so they are resolved (and validated) per role group up-front rather than at build time.
@@ -275,7 +264,7 @@ pub(crate) fn build_role_group_configs(
                 env_overrides: env_overrides_set,
                 pod_overrides,
                 product_specific_common_config,
-                vector_container,
+                logging,
                 git_sync_resources,
             },
         );
@@ -286,6 +275,37 @@ pub(crate) fn build_role_group_configs(
     Ok(role_group_configs)
 }
 
+/// Validates the logging configuration for the NiFi (and optional Vector) container.
+///
+/// `vector_aggregator_config_map_name` is the discovery ConfigMap name of the Vector aggregator;
+/// it is required (and validated) only when the Vector agent is enabled.
+fn validate_logging(
+    logging: &Logging<Container>,
+    vector_aggregator_config_map_name: &Option<ConfigMapName>,
+) -> Result<ValidatedLogging> {
+    let nifi_container = validate_logging_configuration_for_container(logging, &Container::Nifi)
+        .context(ValidateLoggingConfigSnafu)?;
+
+    let vector_container = if logging.enable_vector_agent {
+        let vector_aggregator_config_map_name = vector_aggregator_config_map_name
+            .clone()
+            .context(MissingVectorAggregatorConfigMapNameSnafu)?;
+        Some(VectorContainerLogConfig {
+            log_config: validate_logging_configuration_for_container(logging, &Container::Vector)
+                .context(ValidateLoggingConfigSnafu)?,
+            vector_aggregator_config_map_name,
+        })
+    } else {
+        None
+    };
+
+    Ok(ValidatedLogging {
+        nifi_container,
+        vector_container,
+        enable_vector_agent: logging.enable_vector_agent,
+    })
+}
+
 /// A minimal resolved product image (NiFi 2.9.0) for tests that need to build role-group configs.
 #[cfg(test)]
 pub(crate) fn test_resolved_product_image() -> product_image_selection::ResolvedProductImage {
@@ -370,6 +390,7 @@ mod tests {
             .expect("role group configs should validate");
 
         let vector = default_rg(&configs)
+            .logging
             .vector_container
             .as_ref()
             .expect("the Vector container config should be present when the agent is enabled");
@@ -402,6 +423,6 @@ mod tests {
         let configs = build_role_group_configs(&nifi, &test_resolved_product_image(), &None)
             .expect("role group configs should validate");
 
-        assert!(default_rg(&configs).vector_container.is_none());
+        assert!(default_rg(&configs).logging.vector_container.is_none());
     }
 }