refactor: Flatten config maps and introduce RoleBuilder (#887)

* refactor: Flatten role-group config maps and introduce RoleBuilder

* changelog

* reorganise imports

* reorganise imports

* provide more robust/readable yaml for the test

* remove entries from CL that do nto belong there

* move drain_pre_stop_hook to middle_manager.rs, removing graceful_shutdown.rs

* build role group resources directly instead of via build()

* only resolve graceful validated logging, shutdown and secret lifetime once

Author: adwk67

CHANGELOG.md

@@ -36,6 +36,9 @@ All notable changes to this project will be documented in this file.
   operator upgrade, delete each broker, coordinator and router StatefulSet so that the operator
   immediately recreates it with the new labels ([#865]).
 - Make operations infallible where dependent on static inputs ([#869], [#874]).
+- Internal operator refactoring: build each role group through per-role modules (brokers,
+  coordinators, historicals, middle managers, routers) and a shared role group builder, with
+  each role's configuration held in typed fields of the validated cluster ([#887]).
 - Support Vector 0.58.0 ([#885], [#889]).
 - Replace the unmaintained `serde_yaml` with `yaml_serde` ([#888]).
 - All pods now set `securityContext.seccompProfile.type` to `RuntimeDefault` ([#888]).
@@ -66,6 +69,7 @@ All notable changes to this project will be documented in this file.
 [#879]: https://github.com/stackabletech/druid-operator/pull/879
 [#880]: https://github.com/stackabletech/druid-operator/pull/880
 [#885]: https://github.com/stackabletech/druid-operator/pull/885
+[#887]: https://github.com/stackabletech/druid-operator/pull/887
 [#888]: https://github.com/stackabletech/druid-operator/pull/888
 [#889]: https://github.com/stackabletech/druid-operator/pull/889
 [#891]: https://github.com/stackabletech/druid-operator/pull/891

rust/operator-binary/src/controller.rs

@@ -178,22 +178,14 @@ pub fn error_policy(
 mod test {
     use std::str::FromStr;
 
-    use rstest::*;
     use stackable_operator::{
         client::Client,
         commons::networking::DomainName,
         kube::{Client as KubeClient, Config, runtime::controller::Action},
         utils::cluster_info::KubernetesClusterInfo,
-        v2::types::operator::RoleGroupName,
     };
 
     use super::{CONTROLLER_NAME, OPERATOR_NAME, PRODUCT_NAME, *};
-    use crate::{
-        controller::build::{
-            properties::ConfigFileName, resource::config_map::build_rolegroup_config_map,
-        },
-        crd::{DruidRole, PROP_SEGMENT_CACHE_LOCATIONS},
-    };
 
     #[test]
     fn test_constants() {
@@ -203,70 +195,6 @@ mod test {
         let _ = *CONTROLLER_NAME;
     }
 
-    #[rstest]
-    #[case(
-        "segment_cache.yaml",
-        "default",
-        "[{\"path\":\"/stackable/var/druid/segment-cache\",\"maxSize\":\"1G\",\"freeSpacePercent\":\"5\"}]"
-    )]
-    #[case(
-        "segment_cache.yaml",
-        "secondary",
-        "[{\"path\":\"/stackable/var/druid/segment-cache\",\"maxSize\":\"5G\",\"freeSpacePercent\":\"2\"}]"
-    )]
-    fn segment_cache_location_property(
-        #[case] druid_manifest: &str,
-        #[case] tested_rolegroup_name: &str,
-        #[case] expected_druid_segment_cache_property: &str,
-    ) {
-        let yaml =
-            std::fs::read_to_string(format!("test/resources/druid_controller/{druid_manifest}"))
-                .unwrap();
-        let druid = crate::controller::validate::test_support::druid_from_yaml(&yaml);
-
-        let cluster = crate::controller::validate::test_support::validated_cluster(&druid);
-
-        // The segment cache property is injected dynamically by the config_map builder from the
-        // merged resources of the validated role group config.
-        let rg = cluster
-            .role_group_configs
-            .get(&DruidRole::Historical)
-            .expect("historical role groups")
-            .get(&RoleGroupName::from_str(tested_rolegroup_name).unwrap())
-            .expect("tested rolegroup")
-            .clone();
-
-        let rg_configmap = build_rolegroup_config_map(
-            &cluster,
-            &DruidRole::Historical,
-            &RoleGroupName::from_str(tested_rolegroup_name).unwrap(),
-            &rg,
-        )
-        .expect("build rolegroup config map");
-
-        let druid_segment_cache_property = rg_configmap
-            .data
-            .unwrap()
-            .get(&ConfigFileName::RuntimeProperties.to_string())
-            .unwrap()
-            .to_string();
-
-        let escaped_segment_cache_property =
-            stackable_operator::v2::config_file_writer::to_java_properties_string(
-                vec![(
-                    &PROP_SEGMENT_CACHE_LOCATIONS.to_string(),
-                    &expected_druid_segment_cache_property.to_string(),
-                )]
-                .into_iter(),
-            )
-            .unwrap();
-
-        assert!(
-            druid_segment_cache_property.contains(&escaped_segment_cache_property),
-            "role group {tested_rolegroup_name}"
-        );
-    }
-
     /// The client points at a closed port, so any API call would fail the reconciliation: an `Ok`
     /// proves that a cluster being deleted returns before the reconciler touches the Kubernetes
     /// API, and because the spec is invalid, before the [`DeserializeGuard`] is unwrapped.

rust/operator-binary/src/controller/build/authentication/ldap.rs

@@ -2,12 +2,12 @@ use std::collections::BTreeMap;
 
 use snafu::ResultExt;
 use stackable_operator::{
-    builder::pod::{PodBuilder, container::ContainerBuilder},
     crd::authentication::ldap,
+    k8s_openapi::api::core::v1::{Volume, VolumeMount},
 };
 
 use super::{
-    AddLdapVolumesSnafu, ConstructLdapEndpointUrlSnafu, Error, MissingLdapBindCredentialsSnafu,
+    ConstructLdapEndpointUrlSnafu, Error, LdapVolumesSnafu, MissingLdapBindCredentialsSnafu,
 };
 use crate::crd::{
     STACKABLE_TRUST_STORE_PASSWORD, file_reference,
@@ -97,20 +97,17 @@ pub(super) fn prepare_container_commands(
     }
 }
 
-pub(super) fn add_volumes_and_mounts(
+/// The volumes of the LDAP bind credentials and the provider's TLS CA certificate, and their
+/// mounts.
+pub(super) fn volumes_and_mounts(
     provider: &ldap::v1alpha1::AuthenticationProvider,
-    pb: &mut PodBuilder,
-    cb_druid: &mut ContainerBuilder,
-    cb_prepare: &mut ContainerBuilder,
-) -> Result<(), Error> {
+) -> Result<(Vec<Volume>, Vec<VolumeMount>), Error> {
     // TODO: Connecting to an LDAP server without bind credentials does not seem to be configurable in Druid at the moment
     // see https://github.com/stackabletech/druid-operator/issues/383 for future work.
     // Expect bind credentials to be provided for now, and throw return a useful error if there are none.
     if provider.bind_credentials_mount_paths().is_none() {
         return MissingLdapBindCredentialsSnafu.fail();
     }
 
-    provider
-        .add_volumes_and_mounts(pb, vec![cb_druid, cb_prepare])
-        .context(AddLdapVolumesSnafu)
+    provider.volumes_and_mounts().context(LdapVolumesSnafu)
 }

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

@@ -9,15 +9,15 @@ use std::collections::BTreeMap;
 
 use snafu::Snafu;
 use stackable_operator::{
-    builder::pod::{PodBuilder, container::ContainerBuilder},
+    k8s_openapi::api::core::v1::{Volume, VolumeMount},
     v2::builder::pod::container::EnvVarSet,
 };
 
 use crate::{
     authentication::DruidAuthenticationConfig,
     controller::validate::ValidatedCluster,
     crd::{
-        DruidRole, env_var_reference,
+        env_var_reference,
         security::{
             INTERNAL_INITIAL_CLIENT_PASSWORD_ENV, INTERNAL_INITIAL_CLIENT_PASSWORD_SECRET_KEY,
         },
@@ -51,13 +51,13 @@ pub enum Error {
         source: stackable_operator::crd::authentication::oidc::v1alpha1::Error,
     },
 
-    #[snafu(display("failed to add LDAP Volumes and VolumeMounts to the Pod and containers"))]
-    AddLdapVolumes {
+    #[snafu(display("failed to build the LDAP volumes and volume mounts"))]
+    LdapVolumes {
         source: stackable_operator::crd::authentication::ldap::v1alpha1::Error,
     },
 
-    #[snafu(display("failed to add OIDC Volumes and VolumeMounts to the Pod and containers"))]
-    AddOidcVolumes {
+    #[snafu(display("failed to build the OIDC volumes and volume mounts"))]
+    OidcVolumes {
         source: stackable_operator::commons::tls_verification::TlsClientDetailsError,
     },
 
@@ -98,9 +98,12 @@ fn set_authenticator_chain(
 
 /// Creates the authentication and authorization parts of the runtime.properties config file.
 /// Configuration related to TLS authentication is added in `controller::build::security`.
+///
+/// `oidc_enabled` says whether the role authenticates its users with OIDC when the cluster uses
+/// OIDC; a role that does not gets the system authenticator only.
 pub fn generate_runtime_properties_config(
     auth: &DruidAuthenticationConfig,
-    role: &DruidRole,
+    oidc_enabled: bool,
 ) -> Result<BTreeMap<String, String>> {
     let mut config: BTreeMap<String, String> = BTreeMap::new();
 
@@ -111,7 +114,7 @@ pub fn generate_runtime_properties_config(
         }
         DruidAuthenticationConfig::Oidc { provider, oidc } => {
             generate_common_runtime_properties_config(&mut config);
-            oidc::generate_runtime_properties_config(provider, oidc, role, &mut config)?
+            oidc::generate_runtime_properties_config(provider, oidc, oidc_enabled, &mut config)?
         }
         DruidAuthenticationConfig::Tls {} => (),
     }
@@ -145,10 +148,12 @@ pub fn prepare_container_commands(auth: &DruidAuthenticationConfig) -> Vec<Strin
     command
 }
 
+/// The environment variables of the `druid` container that carry the authentication secrets.
+/// `oidc_enabled` is as for [`generate_runtime_properties_config`].
 pub fn get_env_var_mounts(
     auth: &DruidAuthenticationConfig,
     cluster: &ValidatedCluster,
-    role: &DruidRole,
+    oidc_enabled: bool,
 ) -> EnvVarSet {
     let internal_secret_name = build_shared_internal_secret_name(&cluster.name);
     let mut env_vars = EnvVarSet::new().with_secret_key_ref(
@@ -158,25 +163,24 @@ pub fn get_env_var_mounts(
     );
 
     if let DruidAuthenticationConfig::Oidc { oidc, .. } = auth {
-        env_vars = env_vars.merge(oidc::get_env_var_mounts(role, oidc, &internal_secret_name));
+        env_vars = env_vars.merge(oidc::get_env_var_mounts(
+            oidc_enabled,
+            oidc,
+            &internal_secret_name,
+        ));
     }
     env_vars
 }
 
-pub fn add_volumes_and_mounts(
+/// The pod volumes the authentication provider needs and their mounts on the `druid` and
+/// `prepare` containers.
+pub fn volumes_and_mounts(
     auth: &DruidAuthenticationConfig,
-    pb: &mut PodBuilder,
-    cb_druid: &mut ContainerBuilder,
-    cb_prepare: &mut ContainerBuilder,
-) -> Result<()> {
+) -> Result<(Vec<Volume>, Vec<VolumeMount>)> {
     match auth {
-        DruidAuthenticationConfig::Ldap { provider } => {
-            ldap::add_volumes_and_mounts(provider, pb, cb_druid, cb_prepare)
-        }
-        DruidAuthenticationConfig::Oidc { provider, .. } => {
-            oidc::add_volumes_and_mounts(provider, pb, cb_druid, cb_prepare)
-        }
-        DruidAuthenticationConfig::Tls {} => Ok(()),
+        DruidAuthenticationConfig::Ldap { provider } => ldap::volumes_and_mounts(provider),
+        DruidAuthenticationConfig::Oidc { provider, .. } => oidc::volumes_and_mounts(provider),
+        DruidAuthenticationConfig::Tls {} => Ok((Vec::new(), Vec::new())),
     }
 }
 
@@ -258,8 +262,7 @@ mod test {
 
     #[test]
     fn test_ldap_config_is_added() {
-        let got = generate_runtime_properties_config(&ldap_auth_config(), &DruidRole::Coordinator)
-            .unwrap();
+        let got = generate_runtime_properties_config(&ldap_auth_config(), true).unwrap();
 
         assert!(got.contains_key("druid.auth.authenticator.Ldap.type"));
     }
@@ -268,8 +271,7 @@ mod test {
     /// alongside the provider-specific (LDAP/OIDC) config.
     #[test]
     fn common_system_auth_config_is_added() {
-        let got = generate_runtime_properties_config(&ldap_auth_config(), &DruidRole::Coordinator)
-            .unwrap();
+        let got = generate_runtime_properties_config(&ldap_auth_config(), true).unwrap();
 
         assert_eq!(
             got.get("druid.auth.authenticator.DruidSystemAuthenticator.type"),
@@ -290,11 +292,8 @@ mod test {
     /// contributes nothing to runtime.properties.
     #[test]
     fn tls_only_produces_no_runtime_properties() {
-        let got = generate_runtime_properties_config(
-            &DruidAuthenticationConfig::Tls {},
-            &DruidRole::Broker,
-        )
-        .unwrap();
+        let got =
+            generate_runtime_properties_config(&DruidAuthenticationConfig::Tls {}, true).unwrap();
 
         assert!(got.is_empty());
     }