refactor(zk_controller): extract dereference and validate steps

Author: maltesander

rust/operator-binary/src/zk_controller.rs

@@ -31,7 +31,7 @@ use stackable_operator::{
     cli::OperatorEnvironmentOptions,
     cluster_resources::{ClusterResourceApplyStrategy, ClusterResources},
     commons::{
-        product_image_selection::{self, ResolvedProductImage},
+        product_image_selection::ResolvedProductImage,
         rbac::build_rbac_resources,
     },
     constants::RESTART_CONTROLLER_ENABLED_LABEL,
@@ -55,7 +55,6 @@ use stackable_operator::{
     },
     kvp::{LabelError, Labels},
     logging::controller::ReconcilerError,
-    product_config_utils::{transform_all_roles_to_config, validate_all_roles_and_groups_config},
     product_logging::{
         self,
         framework::{
@@ -81,7 +80,7 @@ use crate::{
     command::create_init_container_command_args,
     config::jvm::{construct_non_heap_jvm_args, construct_zk_server_heap_env},
     crd::{
-        CONTAINER_IMAGE_BASE_NAME, JMX_METRICS_PORT_NAME, JVM_SECURITY_PROPERTIES_FILE,
+        JMX_METRICS_PORT_NAME, JVM_SECURITY_PROPERTIES_FILE,
         MAX_PREPARE_LOG_FILE_SIZE, MAX_ZK_LOG_FILES_SIZE, METRICS_PROVIDER_HTTP_PORT_NAME,
         STACKABLE_CONFIG_DIR, STACKABLE_DATA_DIR, STACKABLE_LOG_CONFIG_DIR, STACKABLE_LOG_DIR,
         STACKABLE_RW_CONFIG_DIR, ZOOKEEPER_ELECTION_PORT, ZOOKEEPER_ELECTION_PORT_NAME,
@@ -101,6 +100,9 @@ use crate::{
     utils::build_recommended_labels,
 };
 
+mod dereference;
+mod validate;
+
 pub const ZK_CONTROLLER_NAME: &str = "zookeepercluster";
 pub const ZK_FULL_CONTROLLER_NAME: &str = concatcp!(ZK_CONTROLLER_NAME, '.', OPERATOR_NAME);
 pub const LISTENER_VOLUME_NAME: &str = "listener";
@@ -126,12 +128,15 @@ pub enum Error {
         source: error_boundary::InvalidObject,
     },
 
+    #[snafu(display("failed to dereference resources"))]
+    Dereference { source: dereference::Error },
+
+    #[snafu(display("failed to validate cluster"))]
+    ValidateCluster { source: validate::Error },
+
     #[snafu(display("crd validation failure"))]
     CrdValidationFailure { source: crate::crd::Error },
 
-    #[snafu(display("object defines no server role"))]
-    NoServerRole,
-
     #[snafu(display("could not parse role [{role}]"))]
     RoleParseFailure {
         source: strum::ParseError,
@@ -165,16 +170,6 @@ pub enum Error {
         rolegroup: RoleGroupRef<v1alpha1::ZookeeperCluster>,
     },
 
-    #[snafu(display("failed to generate product config"))]
-    GenerateProductConfig {
-        source: stackable_operator::product_config_utils::Error,
-    },
-
-    #[snafu(display("invalid product config"))]
-    InvalidProductConfig {
-        source: stackable_operator::product_config_utils::Error,
-    },
-
     #[snafu(display("failed to serialize [{ZOOKEEPER_PROPERTIES_FILE}] for {}", rolegroup))]
     SerializeZooCfg {
         source: PropertiesWriterError,
@@ -228,9 +223,6 @@ pub enum Error {
         cm_name: String,
     },
 
-    #[snafu(display("failed to initialize security context"))]
-    FailedToInitializeSecurityContext { source: crate::crd::security::Error },
-
     #[snafu(display("failed to resolve and merge config for role and role group"))]
     FailedToResolveConfig { source: crate::crd::Error },
 
@@ -289,11 +281,6 @@ pub enum Error {
         source: stackable_operator::builder::pod::volume::ListenerOperatorVolumeSourceBuilderError,
     },
 
-    #[snafu(display("failed to resolve product image"))]
-    ResolveProductImage {
-        source: product_image_selection::Error,
-    },
-
     #[snafu(display("failed to build service"))]
     BuildService { source: service::Error },
 
@@ -310,16 +297,15 @@ impl ReconcilerError for Error {
         match self {
             Error::MissingSecretLifetime => None,
             Error::InvalidZookeeperCluster { .. } => None,
+            Error::Dereference { .. } => None,
+            Error::ValidateCluster { .. } => None,
             Error::CrdValidationFailure { .. } => None,
-            Error::NoServerRole => None,
             Error::RoleParseFailure { .. } => None,
             Error::InternalOperatorFailure { .. } => None,
             Error::ApplyRoleGroupService { .. } => None,
             Error::BuildRoleGroupConfig { .. } => None,
             Error::ApplyRoleGroupConfig { .. } => None,
             Error::ApplyRoleGroupStatefulSet { .. } => None,
-            Error::GenerateProductConfig { .. } => None,
-            Error::InvalidProductConfig { .. } => None,
             Error::SerializeZooCfg { .. } => None,
             Error::ObjectMissingMetadataForOwnerRef { .. } => None,
             Error::BuildDiscoveryConfig { .. } => None,
@@ -331,7 +317,6 @@ impl ReconcilerError for Error {
             Error::DeleteOrphans { .. } => None,
             Error::VectorAggregatorConfigMapMissing => None,
             Error::InvalidLoggingConfig { .. } => None,
-            Error::FailedToInitializeSecurityContext { .. } => None,
             Error::FailedToResolveConfig { .. } => None,
             Error::FailedToCreatePdb { .. } => None,
             Error::GracefulShutdown { .. } => None,
@@ -346,7 +331,6 @@ impl ReconcilerError for Error {
             Error::ApplyGroupListener { .. } => None,
             Error::BuildListenerPersistentVolume { .. } => None,
             Error::ListenerConfiguration { .. } => None,
-            Error::ResolveProductImage { .. } => None,
             Error::BuildService { .. } => None,
             Error::RetrieveMetricsPortFromConfig { .. } => None,
         }
@@ -364,15 +348,23 @@ pub async fn reconcile_zk(
             .context(InvalidZookeeperClusterSnafu)?;
     let client = &ctx.client;
 
-    let resolved_product_image = zk
-        .spec
-        .image
-        .resolve(
-            CONTAINER_IMAGE_BASE_NAME,
-            &ctx.operator_environment.image_repository,
-            crate::built_info::PKG_VERSION,
-        )
-        .context(ResolveProductImageSnafu)?;
+    // dereference (client required)
+    let dereferenced_objects = dereference::dereference(client, zk)
+        .await
+        .context(DereferenceSnafu)?;
+
+    // validate (no client required)
+    let validate::ValidatedInputs {
+        resolved_product_image,
+        zookeeper_security,
+        validated_role_config,
+    } = validate::validate(
+        zk,
+        &dereferenced_objects,
+        &ctx.operator_environment,
+        &ctx.product_config,
+    )
+    .context(ValidateClusterSnafu)?;
 
     let mut cluster_resources = ClusterResources::new(
         APP_NAME,
@@ -384,39 +376,11 @@ pub async fn reconcile_zk(
     )
     .context(CreateClusterResourcesSnafu)?;
 
-    let validated_config = validate_all_roles_and_groups_config(
-        &resolved_product_image.product_version,
-        &transform_all_roles_to_config(
-            zk,
-            &[(
-                ZookeeperRole::Server.to_string(),
-                (
-                    vec![
-                        PropertyNameKind::Env,
-                        PropertyNameKind::File(ZOOKEEPER_PROPERTIES_FILE.to_string()),
-                        PropertyNameKind::File(JVM_SECURITY_PROPERTIES_FILE.to_string()),
-                    ],
-                    zk.spec.servers.clone().context(NoServerRoleSnafu)?,
-                ),
-            )]
-            .into(),
-        )
-        .context(GenerateProductConfigSnafu)?,
-        &ctx.product_config,
-        false,
-        false,
-    )
-    .context(InvalidProductConfigSnafu)?;
-
-    let role_server_config = validated_config
+    let role_server_config = validated_role_config
         .get(&ZookeeperRole::Server.to_string())
         .map(Cow::Borrowed)
         .unwrap_or_default();
 
-    let zookeeper_security = ZookeeperSecurity::new_from_zookeeper_cluster(client, zk)
-        .await
-        .context(FailedToInitializeSecurityContextSnafu)?;
-
     let (rbac_sa, rbac_rolebinding) = build_rbac_resources(
         zk,
         APP_NAME,
@@ -1072,9 +1036,15 @@ pub fn error_policy(
 
 #[cfg(test)]
 mod tests {
-    use stackable_operator::commons::networking::DomainName;
+    use stackable_operator::{
+        commons::networking::DomainName,
+        product_config_utils::{
+            transform_all_roles_to_config, validate_all_roles_and_groups_config,
+        },
+    };
 
     use super::*;
+    use crate::crd::CONTAINER_IMAGE_BASE_NAME;
 
     #[test]
     fn test_default_config() {

rust/operator-binary/src/zk_controller/dereference.rs

@@ -0,0 +1,44 @@
+//! The dereference step in the ZookeeperCluster controller.
+//!
+//! Fetches all Kubernetes objects referenced by the [`v1alpha1::ZookeeperCluster`] spec and
+//! returns them in [`DereferencedObjects`]. Synchronous validation of the fetched objects
+//! (image resolution, product-config validation, security struct assembly) happens in the
+//! validate step.
+
+use snafu::{ResultExt, Snafu};
+use stackable_operator::client::Client;
+
+use crate::crd::{
+    authentication::{self, ResolvedAuthenticationClasses},
+    v1alpha1,
+};
+
+#[derive(Snafu, Debug)]
+pub enum Error {
+    #[snafu(display("failed to resolve authentication classes"))]
+    ResolveAuthenticationClasses { source: authentication::Error },
+}
+
+type Result<T, E = Error> = std::result::Result<T, E>;
+
+/// Kubernetes objects referenced from the [`v1alpha1::ZookeeperCluster`] spec, already fetched.
+pub struct DereferencedObjects {
+    pub resolved_authentication_classes: ResolvedAuthenticationClasses,
+}
+
+/// Fetches all Kubernetes objects referenced from the [`v1alpha1::ZookeeperCluster`] spec.
+pub async fn dereference(
+    client: &Client,
+    zk: &v1alpha1::ZookeeperCluster,
+) -> Result<DereferencedObjects> {
+    let resolved_authentication_classes = authentication::resolve_authentication_classes(
+        client,
+        &zk.spec.cluster_config.authentication,
+    )
+    .await
+    .context(ResolveAuthenticationClassesSnafu)?;
+
+    Ok(DereferencedObjects {
+        resolved_authentication_classes,
+    })
+}

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

@@ -0,0 +1,121 @@
+//! The validate step in the ZookeeperCluster controller.
+//!
+//! Synchronously validates inputs that don't require a Kubernetes client. Produces
+//! [`ValidatedInputs`], consumed by the rest of `reconcile_zk`.
+
+use product_config::{ProductConfigManager, types::PropertyNameKind};
+use snafu::{OptionExt, ResultExt, Snafu};
+use stackable_operator::{
+    cli::OperatorEnvironmentOptions,
+    commons::product_image_selection::{self, ResolvedProductImage},
+    product_config_utils::{
+        ValidatedRoleConfigByPropertyKind, transform_all_roles_to_config,
+        validate_all_roles_and_groups_config,
+    },
+};
+
+use crate::{
+    crd::{
+        CONTAINER_IMAGE_BASE_NAME, JVM_SECURITY_PROPERTIES_FILE, ZOOKEEPER_PROPERTIES_FILE,
+        ZookeeperRole,
+        security::ZookeeperSecurity,
+        v1alpha1,
+    },
+    zk_controller::dereference::DereferencedObjects,
+};
+
+#[derive(Snafu, Debug)]
+pub enum Error {
+    #[snafu(display("failed to resolve product image"))]
+    ResolveProductImage {
+        source: product_image_selection::Error,
+    },
+
+    #[snafu(display("object defines no server role"))]
+    NoServerRole,
+
+    #[snafu(display("failed to generate product config"))]
+    GenerateProductConfig {
+        source: stackable_operator::product_config_utils::Error,
+    },
+
+    #[snafu(display("invalid product config"))]
+    InvalidProductConfig {
+        source: stackable_operator::product_config_utils::Error,
+    },
+}
+
+type Result<T, E = Error> = std::result::Result<T, E>;
+
+/// Synchronous inputs the rest of `reconcile_zk` needs after dereferencing.
+pub struct ValidatedInputs {
+    pub resolved_product_image: ResolvedProductImage,
+    pub zookeeper_security: ZookeeperSecurity,
+    pub validated_role_config: ValidatedRoleConfigByPropertyKind,
+}
+
+/// Validates the cluster spec and the dereferenced inputs.
+pub fn validate(
+    zk: &v1alpha1::ZookeeperCluster,
+    dereferenced_objects: &DereferencedObjects,
+    operator_environment: &OperatorEnvironmentOptions,
+    product_config: &ProductConfigManager,
+) -> Result<ValidatedInputs> {
+    let resolved_product_image = zk
+        .spec
+        .image
+        .resolve(
+            CONTAINER_IMAGE_BASE_NAME,
+            &operator_environment.image_repository,
+            crate::built_info::PKG_VERSION,
+        )
+        .context(ResolveProductImageSnafu)?;
+
+    let zookeeper_security = ZookeeperSecurity::new(
+        zk,
+        dereferenced_objects.resolved_authentication_classes.clone(),
+    );
+
+    let validated_role_config =
+        validated_product_config(zk, &resolved_product_image.product_version, product_config)?;
+
+    Ok(ValidatedInputs {
+        resolved_product_image,
+        zookeeper_security,
+        validated_role_config,
+    })
+}
+
+fn validated_product_config(
+    zk: &v1alpha1::ZookeeperCluster,
+    product_version: &str,
+    product_config: &ProductConfigManager,
+) -> Result<ValidatedRoleConfigByPropertyKind> {
+    let server_role = zk.spec.servers.clone().context(NoServerRoleSnafu)?;
+
+    let role_config = transform_all_roles_to_config(
+        zk,
+        &[(
+            ZookeeperRole::Server.to_string(),
+            (
+                vec![
+                    PropertyNameKind::Env,
+                    PropertyNameKind::File(ZOOKEEPER_PROPERTIES_FILE.to_string()),
+                    PropertyNameKind::File(JVM_SECURITY_PROPERTIES_FILE.to_string()),
+                ],
+                server_role,
+            ),
+        )]
+        .into(),
+    )
+    .context(GenerateProductConfigSnafu)?;
+
+    validate_all_roles_and_groups_config(
+        product_version,
+        &role_config,
+        product_config,
+        false,
+        false,
+    )
+    .context(InvalidProductConfigSnafu)
+}