fix: error ranaming, shared test import, extract validated_role_group_config helper

Author: maltesander

rust/operator-binary/src/controller.rs

@@ -698,3 +698,27 @@ pub fn error_policy(
         _ => Action::requeue(*Duration::from_secs(5)),
     }
 }
+
+#[cfg(test)]
+pub(crate) mod test_support {
+    use crate::{
+        controller::dereference::DereferencedObjects,
+        crd::authentication::{
+            self, SupersetClientAuthenticationDetailsResolved, v1alpha1::FlaskRolesSyncMoment,
+        },
+    };
+
+    /// A [`DereferencedObjects`] with no authentication classes and no OPA config, for tests that
+    /// build a `ValidatedCluster` without exercising the dereference step.
+    pub(crate) fn default_dereferenced() -> DereferencedObjects {
+        DereferencedObjects {
+            authentication_config: SupersetClientAuthenticationDetailsResolved {
+                authentication_classes_resolved: vec![],
+                user_registration: true,
+                user_registration_role: authentication::DEFAULT_USER_REGISTRATION_ROLE.to_string(),
+                sync_roles_at: FlaskRolesSyncMoment::default(),
+            },
+            opa_config: None,
+        }
+    }
+}

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

@@ -80,27 +80,10 @@ mod tests {
 
     use super::*;
     use crate::{
-        controller::{dereference::DereferencedObjects, validate::validate_cluster},
-        crd::{
-            authentication::{
-                self, SupersetClientAuthenticationDetailsResolved, v1alpha1::FlaskRolesSyncMoment,
-            },
-            v1alpha1,
-        },
+        controller::{test_support::default_dereferenced, validate::validate_cluster},
+        crd::v1alpha1,
     };
 
-    fn default_dereferenced() -> DereferencedObjects {
-        DereferencedObjects {
-            authentication_config: SupersetClientAuthenticationDetailsResolved {
-                authentication_classes_resolved: vec![],
-                user_registration: true,
-                user_registration_role: authentication::DEFAULT_USER_REGISTRATION_ROLE.to_string(),
-                sync_roles_at: FlaskRolesSyncMoment::default(),
-            },
-            opa_config: None,
-        }
-    }
-
     /// The rolegroup ConfigMap carries `superset_config.py` and (for automatic logging)
     /// `log_config.py`, and omits `vector.yaml` while the Vector agent is disabled (the default).
     #[test]

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

@@ -43,11 +43,6 @@ pub enum Error {
         source: stackable_operator::builder::pod::Error,
     },
 
-    #[snafu(display("failed to build Metadata"))]
-    MetadataBuild {
-        source: stackable_operator::builder::meta::Error,
-    },
-
     #[snafu(display("failed to add needed volume"))]
     AddVolume {
         source: stackable_operator::builder::pod::Error,
@@ -71,7 +66,6 @@ pub fn build_rolegroup_deployment(
     rolegroup_config: &SupersetRoleGroupConfig,
     sa_name: &str,
 ) -> Result<Deployment> {
-    let resolved_product_image = &validated.image;
     let merged_config = &rolegroup_config.config;
 
     let resource_names = validated.resource_names(superset_role, role_group_name);
@@ -101,7 +95,7 @@ pub fn build_rolegroup_deployment(
 
     let mut pb = PodBuilder::new();
     pb.metadata(metadata)
-        .image_pull_secrets_from_product_image(resolved_product_image)
+        .image_pull_secrets_from_product_image(&validated.image)
         .security_context(
             PodSecurityContextBuilder::new()
                 .fs_group(super::SECRET_OPERATOR_FS_GROUP) // Needed for secret-operator
@@ -151,7 +145,7 @@ pub fn build_rolegroup_deployment(
         &rolegroup_config.config.logging.superset_container,
     ))
     .context(AddVolumeSnafu)?;
-    pb.add_container(super::build_metrics_container(resolved_product_image));
+    pb.add_container(super::build_metrics_container(&validated.image));
 
     if let Some(vector_container) =
         super::build_vector_container(validated, superset_role, role_group_name, rolegroup_config)

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

@@ -64,11 +64,6 @@ pub enum Error {
         source: stackable_operator::builder::pod::Error,
     },
 
-    #[snafu(display("failed to build Metadata"))]
-    MetadataBuild {
-        source: stackable_operator::builder::meta::Error,
-    },
-
     #[snafu(display("failed to add LDAP Volumes and VolumeMounts"))]
     AddLdapVolumesAndVolumeMounts {
         source: stackable_operator::crd::authentication::ldap::v1alpha1::Error,

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

@@ -32,7 +32,7 @@ use crate::{
         dereference::DereferencedObjects,
     },
     crd::{
-        SupersetRole,
+        SupersetRole, SupersetRoleGroupType, SupersetRoleType,
         v1alpha1::{
             Container, SupersetCluster, SupersetConfig, SupersetConfigFragment,
             SupersetConfigOverrides, SupersetRoleConfig,
@@ -62,15 +62,16 @@ pub enum Error {
         source: stackable_operator::v2::controller_utils::Error,
     },
 
-    #[snafu(display("failed to resolve and merge config for role group {role_group}"))]
-    FailedToResolveConfig {
+    #[snafu(display("failed to validate the config for role group {role_group}"))]
+    ValidateConfig {
         source: fragment::ValidationError,
-        role_group: String,
+        role_group: RoleGroupName,
     },
 
-    #[snafu(display("failed to parse environment variable name"))]
+    #[snafu(display("invalid environment variable override name in role group {role_group}"))]
     ParseEnvVarName {
         source: stackable_operator::v2::macros::attributed_string_type::Error,
+        role_group: RoleGroupName,
     },
 
     #[snafu(display("invalid role group name {role_group}"))]
@@ -174,53 +175,19 @@ pub fn validate_cluster(
 
         let mut group_configs = BTreeMap::new();
         for (rolegroup_name, rolegroup) in &resolved_role.role_groups {
-            let validated_rg = with_validated_config::<
-                SupersetConfig,
-                GenericCommonConfig,
-                SupersetConfigFragment,
-                SupersetRoleConfig,
-                SupersetConfigOverrides,
-            >(rolegroup, resolved_role, &default_config)
-            .with_context(|_| FailedToResolveConfigSnafu {
-                role_group: rolegroup_name.clone(),
-            })?;
-
-            let mut env_overrides = EnvVarSet::new();
-            for (name, value) in validated_rg.config.env_overrides {
-                env_overrides = env_overrides.with_value(
-                    &EnvVarName::from_str(&name).context(ParseEnvVarNameSnafu)?,
-                    value,
-                );
-            }
-
             let role_group_name = RoleGroupName::from_str(rolegroup_name).with_context(|_| {
                 ParseRoleGroupNameSnafu {
                     role_group: rolegroup_name.clone(),
                 }
             })?;
-
-            let logging = validate_logging(
-                &validated_rg.config.config.logging,
+            let validated_rg = validate_role_group_config(
+                &role_group_name,
+                rolegroup,
+                resolved_role,
+                &default_config,
                 &vector_aggregator_config_map_name,
             )?;
-
-            group_configs.insert(
-                role_group_name,
-                SupersetRoleGroupConfig {
-                    replicas: validated_rg.replicas,
-                    config: ValidatedSupersetConfig::from_merged(
-                        validated_rg.config.config,
-                        logging,
-                    ),
-                    config_overrides: validated_rg.config.config_overrides,
-                    env_overrides,
-                    cli_overrides: validated_rg.config.cli_overrides,
-                    pod_overrides: validated_rg.config.pod_overrides,
-                    product_specific_common_config: validated_rg
-                        .config
-                        .product_specific_common_config,
-                },
-            );
+            group_configs.insert(role_group_name, validated_rg);
         }
 
         role_groups.insert(role, group_configs);
@@ -252,6 +219,51 @@ pub fn validate_cluster(
     ))
 }
 
+/// Merges and validates one role group into a [`SupersetRoleGroupConfig`].
+fn validate_role_group_config(
+    role_group_name: &RoleGroupName,
+    role_group: &SupersetRoleGroupType,
+    role: &SupersetRoleType,
+    default_config: &SupersetConfigFragment,
+    vector_aggregator_config_map_name: &Option<ConfigMapName>,
+) -> Result<SupersetRoleGroupConfig, Error> {
+    let merged = with_validated_config::<
+        SupersetConfig,
+        GenericCommonConfig,
+        SupersetConfigFragment,
+        SupersetRoleConfig,
+        SupersetConfigOverrides,
+    >(role_group, role, default_config)
+    .with_context(|_| ValidateConfigSnafu {
+        role_group: role_group_name.clone(),
+    })?;
+
+    let mut env_overrides = EnvVarSet::new();
+    for (env_var_name, env_var_value) in merged.config.env_overrides {
+        env_overrides = env_overrides.with_value(
+            &EnvVarName::from_str(&env_var_name).with_context(|_| ParseEnvVarNameSnafu {
+                role_group: role_group_name.clone(),
+            })?,
+            env_var_value,
+        );
+    }
+
+    let logging = validate_logging(
+        &merged.config.config.logging,
+        vector_aggregator_config_map_name,
+    )?;
+
+    Ok(SupersetRoleGroupConfig {
+        replicas: merged.replicas,
+        config: ValidatedSupersetConfig::from_merged(merged.config.config, logging),
+        config_overrides: merged.config.config_overrides,
+        env_overrides,
+        cli_overrides: merged.config.cli_overrides,
+        pod_overrides: merged.config.pod_overrides,
+        product_specific_common_config: merged.config.product_specific_common_config,
+    })
+}
+
 #[cfg(test)]
 mod tests {
 
@@ -267,28 +279,10 @@ mod tests {
 
     use super::{Error, validate_cluster, validate_logging};
     use crate::{
-        controller::dereference::DereferencedObjects,
-        crd::{
-            SupersetRole,
-            authentication::{
-                self, SupersetClientAuthenticationDetailsResolved, v1alpha1::FlaskRolesSyncMoment,
-            },
-            v1alpha1,
-        },
+        controller::test_support::default_dereferenced,
+        crd::{SupersetRole, v1alpha1},
     };
 
-    fn default_dereferenced() -> DereferencedObjects {
-        DereferencedObjects {
-            authentication_config: SupersetClientAuthenticationDetailsResolved {
-                authentication_classes_resolved: vec![],
-                user_registration: true,
-                user_registration_role: authentication::DEFAULT_USER_REGISTRATION_ROLE.to_string(),
-                sync_roles_at: FlaskRolesSyncMoment::default(),
-            },
-            opa_config: None,
-        }
-    }
-
     /// Builds a [`Logging`] with automatic log configuration for the Superset and Vector containers.
     fn automatic_logging(enable_vector_agent: bool) -> Logging<v1alpha1::Container> {
         let automatic = || ContainerLogConfig {

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

@@ -19,7 +19,7 @@ use stackable_operator::{
     kube::{CustomResource, ResourceExt},
     memory::{BinaryMultiple, MemoryQuantity},
     product_logging::{self, spec::Logging},
-    role_utils::{GenericRoleConfig, Role},
+    role_utils::{GenericRoleConfig, Role, RoleGroup},
     schemars::{self, JsonSchema},
     shared::time::Duration,
     status::condition::{ClusterCondition, HasStatusCondition},
@@ -90,6 +90,12 @@ pub type SupersetRoleType = Role<
     GenericCommonConfig,
 >;
 
+pub type SupersetRoleGroupType = RoleGroup<
+    v1alpha1::SupersetConfigFragment,
+    GenericCommonConfig,
+    v1alpha1::SupersetConfigOverrides,
+>;
+
 #[derive(Display, EnumIter, EnumString)]
 #[strum(serialize_all = "SCREAMING_SNAKE_CASE")]
 pub enum SupersetConfigOptions {