refactor: switch to EnvVarSet, introduce role_utils

Author: maltesander

rust/operator-binary/src/controller.rs

@@ -504,7 +504,7 @@ fn build_rolegroup_statefulset(
     druid_auth_config: &Option<DruidAuthenticationConfig>,
     service_account: &ServiceAccount,
 ) -> Result<StatefulSet> {
-    let merged_rolegroup_config = &rg.merged_config;
+    let merged_rolegroup_config = &rg.config;
     // prepare container builder
     let prepare_container_name = Container::Prepare.to_string();
     let mut cb_prepare = ContainerBuilder::new(&prepare_container_name).context(
@@ -625,7 +625,7 @@ fn build_rolegroup_statefulset(
     metadata_database_connection_details.add_to_container(&mut cb_druid);
 
     // rest of env: the validated env overrides, rendered in sorted-by-name order.
-    let mut rest_env: Vec<EnvVar> = rg.env.clone().into();
+    let mut rest_env: Vec<EnvVar> = rg.env_overrides.clone().into();
 
     if let Some(auth_config) = druid_auth_config {
         rest_env.extend(auth_config.get_env_var_mounts(druid, role))
@@ -757,11 +757,8 @@ fn build_rolegroup_statefulset(
     }
 
     let mut pod_template = pb.build_template();
-    pod_template.merge_from(druid.pod_overrides_for_role(role).clone());
-    if let Some(pod_overrides) = druid.pod_overrides_for_role_group(role, &rolegroup_ref.role_group)
-    {
-        pod_template.merge_from(pod_overrides.clone());
-    }
+    // The role and rolegroup pod overrides were already merged (rolegroup wins) during validation.
+    pod_template.merge_from(rg.pod_overrides.clone());
 
     Ok(StatefulSet {
         metadata: ObjectMetaBuilder::new()
@@ -931,23 +928,17 @@ mod test {
     use rstest::*;
     use stackable_operator::{
         database_connections::drivers::jdbc::JdbcDatabaseConnection,
-        v2::{
-            builder::pod::container::EnvVarSet,
-            types::{
-                kubernetes::{NamespaceName, Uid},
-                operator::ClusterName,
-            },
+        v2::types::{
+            kubernetes::{NamespaceName, Uid},
+            operator::ClusterName,
         },
     };
 
     use super::*;
     use crate::{
         controller::{
-            build::{
-                config_map::build_rolegroup_config_map,
-                properties::{ConfigFileName, runtime_properties},
-            },
-            validate::{DruidRoleGroupConfig, ValidatedCluster, ValidatedClusterConfig},
+            build::{config_map::build_rolegroup_config_map, properties::ConfigFileName},
+            validate::{ValidatedCluster, ValidatedClusterConfig},
         },
         crd::{PROP_SEGMENT_CACHE_LOCATIONS, authentication::AuthenticationClassesResolved},
         extensions::get_extension_list,
@@ -993,22 +984,22 @@ mod test {
             Some("tls".to_string()),
         );
 
-        let merged = druid.merged_config().expect("merged config");
-        let merged_config = merged
-            .common_config(&DruidRole::Historical, tested_rolegroup_name)
-            .expect("common config for tested rolegroup");
-
         // The segment cache property is injected dynamically by the config_map builder from the
-        // merged resources, independent of the precomputed runtime_config. We still populate the
-        // runtime_config with the static role defaults to mirror the production path.
-        let rg = DruidRoleGroupConfig {
-            merged_config,
-            runtime_config: runtime_properties::defaults(&DruidRole::Historical),
-            security_config: BTreeMap::new(),
-            env: EnvVarSet::new(),
-            // The test only asserts on runtime.properties, so the rendered jvm.config is irrelevant.
-            jvm_config: String::new(),
-        };
+        // merged resources of the validated role group config.
+        let rg = druid
+            .merged_role(&DruidRole::Historical)
+            .expect("merged historical role")
+            .get(tested_rolegroup_name)
+            .expect("tested rolegroup")
+            .clone();
+
+        // The build step renders jvm.config from the erased role; populate the one role the test
+        // exercises.
+        let mut roles = BTreeMap::new();
+        roles.insert(
+            DruidRole::Historical,
+            druid.get_role(&DruidRole::Historical),
+        );
 
         let extensions = get_extension_list(&druid, &druid_tls_security, &None);
         let metadata_storage_type = druid
@@ -1039,8 +1030,10 @@ mod test {
                 extensions,
                 metadata_storage_type,
                 metadata_db_connection,
+                deep_storage: druid.spec.cluster_config.deep_storage.clone(),
             },
             BTreeMap::new(),
+            roles,
         );
 
         let rolegroup_ref = RoleGroupRef {

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

@@ -1,15 +1,15 @@
 //! Builds the rolegroup [`ConfigMap`] from a [`ValidatedCluster`].
 //!
-//! The per-file "validated config" maps (runtime.properties / security.properties) are taken
-//! from the [`DruidRoleGroupConfig`] precomputed in the validate step; product-config is no
-//! longer involved.
+//! The per-file configs (runtime.properties / security.properties / jvm.config) are rendered here
+//! from the merged [`DruidRoleGroupConfig`] (config plus the merged config overrides); the
+//! recommended cluster-level runtime properties and the erased roles needed for `jvm.config` are
+//! carried on `ValidatedCluster`. Product-config is no longer involved.
 //!
 //! Metadata, owner reference and recommended labels are derived entirely from `ValidatedCluster`
 //! (which carries the validated name/namespace/uid and implements `Resource`).
 //!
-//! The builder no longer reads the raw [`v1alpha1::DruidCluster`] at all: the extensions load
-//! list, the metadata-database connection / storage type and the rendered `jvm.config` are all
-//! precomputed on `ValidatedCluster` during the validate step.
+//! The builder does not read the raw [`v1alpha1::DruidCluster`] at all: everything it needs is
+//! carried on `ValidatedCluster` (resolved during the validate step).
 
 use std::collections::BTreeMap;
 
@@ -28,15 +28,19 @@ use stackable_operator::{
 use crate::{
     controller::{
         DRUID_CONTROLLER_NAME,
-        build::properties::{
-            ConfigFileName,
-            logging::{build_log4j2_config, build_vector_config},
+        build::{
+            jvm::construct_jvm_args,
+            properties::{
+                ConfigFileName,
+                logging::{build_log4j2_config, build_vector_config},
+                runtime_properties, security_properties,
+            },
         },
         validate::{DruidRoleGroupConfig, ValidatedCluster},
     },
     crd::{
-        DruidRole, build_recommended_labels, build_string_list, env_var_reference, file_reference,
-        v1alpha1,
+        DruidConfigOverrides, DruidRole, STACKABLE_TRUST_STORE, STACKABLE_TRUST_STORE_PASSWORD,
+        build_recommended_labels, build_string_list, env_var_reference, file_reference, v1alpha1,
     },
 };
 
@@ -87,10 +91,56 @@ pub enum Error {
     GenerateAuthenticationRuntimeSettings {
         source: crate::authentication::Error,
     },
+
+    #[snafu(display("failed to derive Druid memory settings from resources"))]
+    DeriveMemorySettings { source: crate::crd::resource::Error },
+
+    #[snafu(display("failed to construct the jvm.config"))]
+    GetJvmConfig {
+        source: crate::controller::build::jvm::Error,
+    },
 }
 
 type Result<T, E = Error> = std::result::Result<T, E>;
 
+const INDEXER_JAVA_OPTS: &str = "druid.indexer.runner.javaOptsArray";
+
+/// The `druid.indexer.runner.javaOptsArray` entry that must be present in *every* rendered file
+/// (runtime.properties and security.properties) for MiddleManagers.
+fn middlemanager_indexer_java_opts() -> (String, String) {
+    (
+        INDEXER_JAVA_OPTS.to_string(),
+        build_string_list(&[
+            format!("-Djavax.net.ssl.trustStore={STACKABLE_TRUST_STORE}"),
+            format!("-Djavax.net.ssl.trustStorePassword={STACKABLE_TRUST_STORE_PASSWORD}"),
+            "-Djavax.net.ssl.trustStoreType=pkcs12".to_owned(),
+        ]),
+    )
+}
+
+/// Returns the user-supplied key/value overrides for the given config file from a
+/// [`DruidConfigOverrides`].
+///
+/// The CRD override map allows a value-less key (`someKey:` / null in YAML), modelled as
+/// `Option<String>`. We flatten `None` to an empty string here, matching how the Java properties
+/// writer rendered a missing value (`key=`), so the rest of the pipeline can work with plain
+/// `String` values.
+fn key_value_overrides(
+    overrides: &DruidConfigOverrides,
+    file: ConfigFileName,
+) -> BTreeMap<String, String> {
+    let raw = match file {
+        ConfigFileName::RuntimeProperties => overrides.runtime_properties.overrides.clone(),
+        ConfigFileName::SecurityProperties => overrides.security_properties.overrides.clone(),
+        // log4j2.properties is rendered by the logging framework, and jvm.config is rendered from
+        // JVM argument overrides; neither is assembled from key/value overrides here.
+        ConfigFileName::Log4j2Properties | ConfigFileName::JvmConfig => BTreeMap::new(),
+    };
+    raw.into_iter()
+        .map(|(k, v)| (k, v.unwrap_or_default()))
+        .collect()
+}
+
 /// The rolegroup [`ConfigMap`] configures the rolegroup based on the configuration given by the administrator
 pub fn build_rolegroup_config_map(
     cluster: &ValidatedCluster,
@@ -117,7 +167,7 @@ pub fn build_rolegroup_config_map(
         // This has to be done here since there is no other suitable place for it.
         // Previously such properties were added in the compute_files() function,
         // but that code path is now incompatible with the design of fragment merging.
-        rg.merged_config
+        rg.config
             .resources
             .update_druid_config_file(&mut conf)
             .context(UpdateDruidConfigFromResourcesSnafu)?;
@@ -217,8 +267,22 @@ pub fn build_rolegroup_config_map(
             );
         };
 
-        // extend the config to respect the precomputed defaults and overrides
-        conf.extend(rg.runtime_config.clone());
+        // Role/rolegroup runtime.properties: the recommended cluster-config-derived properties,
+        // the MiddleManager indexer opts, the per-role defaults and finally the user overrides
+        // (each layer wins over the previous, and over the cluster-level properties above).
+        conf.extend(runtime_properties::cluster_runtime_properties(
+            &cluster_config.deep_storage,
+            cluster_config.opa_connection_string.is_some(),
+        ));
+        if *role == DruidRole::MiddleManager {
+            let (k, v) = middlemanager_indexer_java_opts();
+            conf.insert(k, v);
+        }
+        conf.extend(runtime_properties::defaults(role));
+        conf.extend(key_value_overrides(
+            &rg.config_overrides,
+            ConfigFileName::RuntimeProperties,
+        ));
 
         let runtime_properties =
             to_java_properties_string(conf.iter()).context(SerializeRuntimePropertiesSnafu)?;
@@ -229,14 +293,37 @@ pub fn build_rolegroup_config_map(
     }
 
     // ----- jvm.config -----
-    // Precomputed during validation; see `DruidRoleGroupConfig::jvm_config`.
-    cm_conf_data.insert(ConfigFileName::JvmConfig.to_string(), rg.jvm_config.clone());
+    {
+        let (heap, direct) = rg
+            .config
+            .resources
+            .get_memory_sizes(role)
+            .context(DeriveMemorySettingsSnafu)?;
+        let jvm_config = construct_jvm_args(
+            role,
+            cluster.get_role(role),
+            &rolegroup.role_group,
+            heap,
+            direct,
+        )
+        .context(GetJvmConfigSnafu)?;
+        cm_conf_data.insert(ConfigFileName::JvmConfig.to_string(), jvm_config);
+    }
 
     // ----- security.properties -----
     {
+        let mut security_config: BTreeMap<String, String> = BTreeMap::new();
+        if *role == DruidRole::MiddleManager {
+            let (k, v) = middlemanager_indexer_java_opts();
+            security_config.insert(k, v);
+        }
+        let security_overrides =
+            key_value_overrides(&rg.config_overrides, ConfigFileName::SecurityProperties);
+        security_config.extend(security_properties::build(&security_overrides));
+
         cm_conf_data.insert(
             ConfigFileName::SecurityProperties.to_string(),
-            to_java_properties_string(rg.security_config.iter()).with_context(|_| {
+            to_java_properties_string(security_config.iter()).with_context(|_| {
                 JvmSecurityPropertiesSnafu {
                     rolegroup: rolegroup.role_group.clone(),
                 }
@@ -265,11 +352,11 @@ pub fn build_rolegroup_config_map(
         config_map_builder.add_data(filename, file_content);
     }
 
-    if let Some(log4j2_config) = build_log4j2_config(&rg.merged_config.logging) {
+    if let Some(log4j2_config) = build_log4j2_config(&rg.config.logging) {
         config_map_builder.add_data(ConfigFileName::Log4j2Properties.to_string(), log4j2_config);
     }
 
-    if let Some(vector_config) = build_vector_config(rolegroup, &rg.merged_config.logging) {
+    if let Some(vector_config) = build_vector_config(rolegroup, &rg.config.logging) {
         config_map_builder.add_data(VECTOR_CONFIG_FILE, vector_config);
     }
 

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

@@ -294,10 +294,11 @@ mod tests {
             serde_yaml::with::singleton_map_recursive::deserialize(deserializer).unwrap();
 
         let role = druid.get_role(druid_role);
-        let merged_config = druid.merged_config().unwrap();
-        let (heap, direct) = merged_config
-            .common_config(druid_role, "default")
+        let merged_role = druid.merged_role(druid_role).unwrap();
+        let (heap, direct) = merged_role
+            .get("default")
             .unwrap()
+            .config
             .resources
             .get_memory_sizes(druid_role)
             .unwrap();

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

@@ -9,7 +9,63 @@
 
 use std::collections::BTreeMap;
 
-use crate::crd::DruidRole;
+use crate::crd::{DeepStorageSpec, DruidRole, METRICS_PORT};
+
+// deep storage
+const DS_TYPE: &str = "druid.storage.type";
+const DS_DIRECTORY: &str = "druid.storage.storageDirectory";
+const DS_BASE_KEY: &str = "druid.storage.baseKey";
+// OPA
+const AUTH_AUTHORIZERS: &str = "druid.auth.authorizers";
+const AUTH_AUTHORIZERS_VALUE: &str = "[\"OpaAuthorizer\"]";
+const AUTH_AUTHORIZER_OPA_TYPE: &str = "druid.auth.authorizer.OpaAuthorizer.type";
+const AUTH_AUTHORIZER_OPA_TYPE_VALUE: &str = "opa";
+// metrics
+const PROMETHEUS_PORT: &str = "druid.emitter.prometheus.port";
+
+/// The recommended cluster-level `runtime.properties` derived from the cluster config (deep
+/// storage, OPA authorization and metrics). These are independent of role and role group.
+///
+/// `opa_authorization_enabled` mirrors `authorization.opa` being configured (equivalently, the
+/// OPA connection string having been resolved during dereferencing).
+pub fn cluster_runtime_properties(
+    deep_storage: &DeepStorageSpec,
+    opa_authorization_enabled: bool,
+) -> BTreeMap<String, String> {
+    let mut result = BTreeMap::new();
+
+    // OPA
+    if opa_authorization_enabled {
+        result.insert(
+            AUTH_AUTHORIZERS.to_string(),
+            AUTH_AUTHORIZERS_VALUE.to_string(),
+        );
+        result.insert(
+            AUTH_AUTHORIZER_OPA_TYPE.to_string(),
+            AUTH_AUTHORIZER_OPA_TYPE_VALUE.to_string(),
+        );
+        // The opaUri still needs to be set, but that requires a discovery config map and is
+        // handled in the controller.
+    }
+
+    // deep storage
+    result.insert(DS_TYPE.to_string(), deep_storage.to_string());
+    match deep_storage {
+        DeepStorageSpec::Hdfs(hdfs) => {
+            result.insert(DS_DIRECTORY.to_string(), hdfs.directory.clone());
+        }
+        DeepStorageSpec::S3(s3_spec) => {
+            if let Some(key) = &s3_spec.base_key {
+                result.insert(DS_BASE_KEY.to_string(), key.to_string());
+            }
+        }
+    }
+
+    // metrics
+    result.insert(PROMETHEUS_PORT.to_string(), METRICS_PORT.to_string());
+
+    result
+}
 
 /// Defaults rendered for every role.
 const ALL_ROLES: &[(&str, &str)] = &[