refactor: introduce ValidatedMetastoreConfig; caluclate vector toml in build_config_map

Author: maltesander

rust/operator-binary/src/controller.rs

@@ -13,7 +13,12 @@ pub use stackable_operator::v2::types::operator::RoleGroupName;
 use stackable_operator::{
     cli::OperatorEnvironmentOptions,
     cluster_resources::ClusterResourceApplyStrategy,
-    commons::{product_image_selection::ResolvedProductImage, rbac::build_rbac_resources},
+    commons::{
+        affinity::StackableAffinity,
+        product_image_selection::ResolvedProductImage,
+        rbac::build_rbac_resources,
+        resources::{NoRuntimeLimits, Resources},
+    },
     crd::{listener::v1alpha1::Listener, s3},
     database_connections::drivers::jdbc::JdbcDatabaseConnectionDetails,
     kube::{
@@ -169,22 +174,58 @@ impl ReconcilerError for Error {
 }
 
 /// A validated, merged Hive metastore role-group config.
-///
-/// Built by [`validate::validate_cluster`] from the upstream
-/// [`stackable_operator::v2::role_utils::with_validated_config`] result. Holds only the
-/// fields the build steps consume; `cli_overrides` and the product-specific common config
-/// are intentionally not carried (hive does not use them).
+pub type HiveRoleGroupConfig = stackable_operator::v2::role_utils::RoleGroupConfig<
+    ValidatedMetaStoreConfig,
+    stackable_operator::v2::role_utils::JavaCommonConfig,
+    v1alpha1::HiveConfigOverrides,
+>;
+
+/// A validated Hive metastore config: the merged [`MetaStoreConfig`] with its raw `logging`
+/// replaced by the up-front [`ValidatedLogging`](validate::ValidatedLogging) (so an invalid
+/// custom log ConfigMap name or a missing Vector aggregator name fails reconciliation during
+/// validation).
 #[derive(Clone, Debug, PartialEq)]
-pub struct HiveRoleGroupConfig {
-    pub replicas: u16,
-    pub config: MetaStoreConfig,
-    pub config_overrides: v1alpha1::HiveConfigOverrides,
-    pub env_overrides: stackable_operator::v2::builder::pod::container::EnvVarSet,
-    pub pod_overrides: stackable_operator::k8s_openapi::api::core::v1::PodTemplateSpec,
-    pub jvm_argument_overrides:
-        stackable_operator::v2::jvm_argument_overrides::JvmArgumentOverrides,
-    /// Validated logging configuration (derived from `config.logging` during validation).
-    pub logging: crate::controller::validate::ValidatedLogging,
+pub struct ValidatedMetaStoreConfig {
+    pub affinity: StackableAffinity,
+    pub graceful_shutdown_timeout: Option<Duration>,
+    pub logging: validate::ValidatedLogging,
+    pub resources: Resources<crate::crd::MetastoreStorageConfig, NoRuntimeLimits>,
+    pub warehouse_dir: Option<String>,
+}
+
+impl ValidatedMetaStoreConfig {
+    /// Builds the validated config from the merged [`MetaStoreConfig`], swapping in the
+    /// already-validated logging.
+    fn from_merged(merged: MetaStoreConfig, logging: validate::ValidatedLogging) -> Self {
+        Self {
+            warehouse_dir: merged.warehouse_dir,
+            resources: merged.resources,
+            logging,
+            affinity: merged.affinity,
+            graceful_shutdown_timeout: merged.graceful_shutdown_timeout,
+        }
+    }
+
+    /// Wraps a merged [`MetaStoreConfig`] with trivial automatic logging, for builder tests
+    /// that only exercise the non-logging parts of the config.
+    #[cfg(test)]
+    pub(crate) fn from_merged_for_test(merged: MetaStoreConfig) -> Self {
+        use stackable_operator::{
+            product_logging::spec::AutomaticContainerLogConfig,
+            v2::product_logging::framework::ValidatedContainerLogConfigChoice,
+        };
+
+        Self::from_merged(
+            merged,
+            validate::ValidatedLogging {
+                hive_container: ValidatedContainerLogConfigChoice::Automatic(
+                    AutomaticContainerLogConfig::default(),
+                ),
+                vector_container: None,
+                enable_vector_agent: false,
+            },
+        )
+    }
 }
 
 /// The validated cluster: the typed, merged result of the validate step. Subsequent
@@ -480,14 +521,6 @@ pub async fn reconcile_hive(
 
     for (hive_role, role_group_configs) in &validated_cluster.role_group_configs {
         for (role_group_name, rg) in role_group_configs {
-            // The Vector agent config (`vector.yaml`) is a static, env-var-parameterized file
-            // (mirroring the opensearch-operator). It is only added to the ConfigMap when the
-            // Vector agent is enabled for this role group.
-            let vector_config = rg
-                .logging
-                .enable_vector_agent
-                .then(build::properties::product_logging::vector_config_file_content);
-
             let rg_metrics_service =
                 build_rolegroup_metrics_service(&validated_cluster, role_group_name);
 
@@ -498,7 +531,6 @@ pub async fn reconcile_hive(
                 &validated_cluster,
                 role_group_name,
                 rg,
-                vector_config,
             )
             .with_context(|_| BuildRoleGroupConfigMapSnafu {
                 role_group: role_group_name.clone(),

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

@@ -1,7 +1,7 @@
 use snafu::{ResultExt, Snafu};
 use stackable_operator::builder::pod::PodBuilder;
 
-use crate::crd::MetaStoreConfig;
+use crate::controller::ValidatedMetaStoreConfig;
 
 #[derive(Debug, Snafu)]
 pub enum Error {
@@ -12,7 +12,7 @@ pub enum Error {
 }
 
 pub fn add_graceful_shutdown_config(
-    merged_config: &MetaStoreConfig,
+    merged_config: &ValidatedMetaStoreConfig,
     pod_builder: &mut PodBuilder,
 ) -> Result<(), Error> {
     // This must be always set by the merge mechanism, as we provide a default value,

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

@@ -3,10 +3,9 @@ use stackable_operator::memory::{BinaryMultiple, MemoryQuantity};
 
 use super::{kerberos::STACKABLE_KERBEROS_DIR, properties::ConfigFileName};
 use crate::{
-    controller::{HiveRoleGroupConfig, ValidatedCluster},
+    controller::{HiveRoleGroupConfig, ValidatedCluster, ValidatedMetaStoreConfig},
     crd::{
-        METRICS_PORT, MetaStoreConfig, STACKABLE_CONFIG_DIR, STACKABLE_TRUST_STORE,
-        STACKABLE_TRUST_STORE_PASSWORD,
+        METRICS_PORT, STACKABLE_CONFIG_DIR, STACKABLE_TRUST_STORE, STACKABLE_TRUST_STORE_PASSWORD,
     },
 };
 
@@ -44,7 +43,9 @@ fn construct_jvm_args(cluster: &ValidatedCluster, rg: &HiveRoleGroupConfig) -> V
 
     // Apply the already-merged (role + role group) JVM argument overrides on top of the
     // operator-generated base arguments.
-    rg.jvm_argument_overrides.apply_to(jvm_args)
+    rg.product_specific_common_config
+        .jvm_argument_overrides
+        .apply_to(jvm_args)
 }
 
 /// Arguments that go into `HADOOP_OPTS`, so *not* the heap settings (which you can get using
@@ -58,7 +59,9 @@ pub fn construct_non_heap_jvm_args(cluster: &ValidatedCluster, rg: &HiveRoleGrou
 
 /// This will be put into `HADOOP_HEAPSIZE`, which is just the heap size in megabytes (*without* the `m`
 /// unit prepended).
-pub fn construct_hadoop_heapsize_env(merged_config: &MetaStoreConfig) -> Result<String, Error> {
+pub fn construct_hadoop_heapsize_env(
+    merged_config: &ValidatedMetaStoreConfig,
+) -> Result<String, Error> {
     let heap_size_in_mb = (MemoryQuantity::try_from(
         merged_config
             .resources

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

@@ -15,7 +15,7 @@ use std::collections::BTreeMap;
 use snafu::{ResultExt, Snafu};
 use stackable_operator::{crd::s3, k8s_openapi::api::core::v1::EnvVar};
 
-use crate::{controller::ValidatedClusterConfig, crd::MetaStoreConfig};
+use crate::controller::{ValidatedClusterConfig, ValidatedMetaStoreConfig};
 
 const DEFAULT_WAREHOUSE_DIR: &str = "/stackable/warehouse";
 const HIVE_METASTORE_PORT: &str = "hive.metastore.port";
@@ -51,7 +51,7 @@ type Result<T, E = Error> = std::result::Result<T, E>;
 pub fn build(
     cluster_config: &ValidatedClusterConfig,
     product_version: &str,
-    merged_config: &MetaStoreConfig,
+    merged_config: &ValidatedMetaStoreConfig,
     overrides: BTreeMap<String, String>,
 ) -> Result<BTreeMap<String, String>> {
     let database_connection_details = &cluster_config.metadata_database_connection_details;
@@ -135,12 +135,14 @@ pub fn build(
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::controller::build::properties::test_support::derby_cluster_config;
+    use crate::{
+        controller::build::properties::test_support::derby_cluster_config, crd::MetaStoreConfig,
+    };
 
     #[test]
     fn defaults_present_for_minimal_derby_cluster() {
         let cluster_config = derby_cluster_config();
-        let merged = MetaStoreConfig::default();
+        let merged = ValidatedMetaStoreConfig::from_merged_for_test(MetaStoreConfig::default());
 
         let data =
             build(&cluster_config, "4.0.0", &merged, BTreeMap::new()).expect("build hive-site");
@@ -162,10 +164,10 @@ mod tests {
     #[test]
     fn warehouse_dir_spec_overrides_default() {
         let cluster_config = derby_cluster_config();
-        let merged = MetaStoreConfig {
+        let merged = ValidatedMetaStoreConfig::from_merged_for_test(MetaStoreConfig {
             warehouse_dir: Some("/custom/warehouse".to_string()),
             ..MetaStoreConfig::default()
-        };
+        });
 
         let data =
             build(&cluster_config, "4.0.0", &merged, BTreeMap::new()).expect("build hive-site");
@@ -179,7 +181,7 @@ mod tests {
     #[test]
     fn user_override_wins_over_everything() {
         let cluster_config = derby_cluster_config();
-        let merged = MetaStoreConfig::default();
+        let merged = ValidatedMetaStoreConfig::from_merged_for_test(MetaStoreConfig::default());
         let overrides = [("hive.metastore.port".to_string(), "1234".to_string())]
             .into_iter()
             .collect();

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

@@ -3,13 +3,8 @@
 
 use stackable_operator::{
     memory::{BinaryMultiple, MemoryQuantity},
-    product_logging::{
-        self,
-        spec::{
-            AutomaticContainerLogConfig, ContainerLogConfig, ContainerLogConfigChoice, Logging,
-        },
-    },
-    v2::product_logging::framework::STACKABLE_LOG_DIR,
+    product_logging::{self, spec::AutomaticContainerLogConfig},
+    v2::product_logging::framework::{STACKABLE_LOG_DIR, ValidatedContainerLogConfigChoice},
 };
 
 use crate::crd::Container;
@@ -33,14 +28,12 @@ pub fn vector_config_file_content() -> String {
 /// Renders `log4j2.properties` for the Hive metastore container.
 ///
 /// Returns `None` when the Hive container does not use the operator's automatic logging
-/// configuration (e.g. a custom log ConfigMap is referenced instead), in which case no
+/// configuration (i.e. a custom log ConfigMap is referenced instead), in which case no
 /// `log4j2.properties` should be added to the rolegroup `ConfigMap`.
-pub fn build_log4j2(logging: &Logging<Container>) -> Option<String> {
-    match logging.containers.get(&Container::Hive) {
-        Some(ContainerLogConfig {
-            choice: Some(ContainerLogConfigChoice::Automatic(log_config)),
-        }) => Some(log4j2_config(log_config)),
-        _ => None,
+pub fn build_log4j2(hive_container: &ValidatedContainerLogConfigChoice) -> Option<String> {
+    match hive_container {
+        ValidatedContainerLogConfigChoice::Automatic(log_config) => Some(log4j2_config(log_config)),
+        ValidatedContainerLogConfigChoice::Custom(_) => None,
     }
 }
 

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

@@ -34,14 +34,10 @@ type Result<T, E = Error> = std::result::Result<T, E>;
 
 /// The rolegroup [`ConfigMap`] configures the rolegroup based on the configuration given by the
 /// administrator.
-///
-/// `vector_config` is the Vector agent config (`vector.yaml`) built by the caller; it is `None`
-/// when the Vector agent is disabled.
 pub fn build_metastore_rolegroup_config_map(
     cluster: &ValidatedCluster,
     role_group_name: &RoleGroupName,
     rg: &HiveRoleGroupConfig,
-    vector_config: Option<String>,
 ) -> Result<ConfigMap> {
     // hive-site.xml
     let hive_site_overrides = rg.config_overrides.hive_site_xml.overrides.clone();
@@ -88,11 +84,16 @@ pub fn build_metastore_rolegroup_config_map(
         );
     }
 
-    if let Some(log4j2_properties) = product_logging::build_log4j2(&rg.config.logging) {
+    if let Some(log4j2_properties) =
+        product_logging::build_log4j2(&rg.config.logging.hive_container)
+    {
         cm_builder.add_data(ConfigFileName::Log4j2.to_string(), log4j2_properties);
     }
-    if let Some(vector_config) = vector_config {
-        cm_builder.add_data(VECTOR_CONFIG_FILE, vector_config);
+    if rg.config.logging.enable_vector_agent {
+        cm_builder.add_data(
+            VECTOR_CONFIG_FILE,
+            product_logging::vector_config_file_content(),
+        );
     }
 
     cm_builder.build().with_context(|_| AssembleSnafu {

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

@@ -383,7 +383,7 @@ pub(crate) fn build_metastore_rolegroup_statefulset(
     // The Hive container's log config ConfigMap: either the operator-generated one (the rolegroup
     // ConfigMap, which carries the automatic `log4j2.properties`) or a user-provided custom
     // ConfigMap. This branches on the *validated* logging choice (see `ValidatedLogging`).
-    let log_config_volume_config_map = match &rg.logging.hive_container {
+    let log_config_volume_config_map = match &rg.config.logging.hive_container {
         ValidatedContainerLogConfigChoice::Custom(config_map_name) => config_map_name.to_string(),
         ValidatedContainerLogConfigChoice::Automatic(_) => {
             resource_names.role_group_config_map().to_string()
@@ -412,7 +412,7 @@ pub(crate) fn build_metastore_rolegroup_statefulset(
 
     // N.B. the vector container should *follow* the hive container so that the hive one is the
     // default, is started first and can provide any dependencies that vector expects
-    if let Some(vector_log_config) = &rg.logging.vector_container {
+    if let Some(vector_log_config) = &rg.config.logging.vector_container {
         pod_builder.add_container(vector_container(
             &VECTOR_CONTAINER_NAME,
             resolved_product_image,

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

@@ -22,7 +22,7 @@ use stackable_operator::{
 use crate::{
     controller::{
         HiveRoleGroupConfig, RoleGroupName, ValidatedCluster, ValidatedClusterConfig,
-        ValidatedRoleConfig, build::kerberos::kerberos_config_properties,
+        ValidatedMetaStoreConfig, ValidatedRoleConfig, build::kerberos::kerberos_config_properties,
         dereference::DereferencedObjects,
     },
     crd::{
@@ -273,12 +273,6 @@ pub fn validate_cluster(
 }
 
 /// Merges and validates one role group into a [`HiveRoleGroupConfig`].
-///
-/// Uses the upstream [`with_validated_config`] (which merges the config fragment, the
-/// `configOverrides`, the `envOverrides`, the `podOverrides` and the product-specific
-/// [`JavaCommonConfig`] — including its `jvmArgumentOverrides`). The merged `envOverrides`
-/// (`HashMap`) are converted into an [`EnvVarSet`] here so invalid names fail validation
-/// early (the opensearch-operator pattern).
 fn validate_role_group_config(
     role_group_name: &RoleGroupName,
     role_group: &crate::crd::HiveRoleGroupType,
@@ -314,15 +308,13 @@ fn validate_role_group_config(
 
     Ok(HiveRoleGroupConfig {
         replicas: merged.replicas.unwrap_or(1),
-        config: merged.config.config,
+        config: ValidatedMetaStoreConfig::from_merged(merged.config.config, logging),
         config_overrides: merged.config.config_overrides,
         env_overrides,
+        // Hive does not use CLI overrides; the field is carried (and merged upstream) but unused.
+        cli_overrides: merged.config.cli_overrides,
         pod_overrides: merged.config.pod_overrides,
-        jvm_argument_overrides: merged
-            .config
-            .product_specific_common_config
-            .jvm_argument_overrides,
-        logging,
+        product_specific_common_config: merged.config.product_specific_common_config,
     })
 }