fix: split fetch and validate auth class steps (#1036)

* fix: split fetch and validate auth class steps

* fix: rename resolved_authentication_classes to authentication_classes in dereference step.

* fix: rename ResolvedAuth... to DereferencedAuthenticationClass.

* fix: rename missing resolved leftovers.

Author: maltesander

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

@@ -51,32 +51,64 @@ pub mod versioned {
 }
 
 #[derive(Clone, Debug)]
-/// Helper struct that contains resolved AuthenticationClasses to reduce network API calls.
-pub struct ResolvedAuthenticationClasses {
-    resolved_authentication_classes: Vec<core::v1alpha1::AuthenticationClass>,
+/// Helper struct that contains dereferenced AuthenticationClasses to reduce network API calls.
+pub struct DereferencedAuthenticationClasses {
+    dereferenced_authentication_classes: Vec<core::v1alpha1::AuthenticationClass>,
 }
 
-impl ResolvedAuthenticationClasses {
+impl DereferencedAuthenticationClasses {
+    /// Fetch the referenced AuthenticationClasses from the Kubernetes API without validating them.
+    ///
+    /// Call [`Self::validate`] on the result to enforce the constraints documented there.
+    pub async fn fetch_references(
+        client: &Client,
+        auth_classes: &Vec<v1alpha1::ZookeeperAuthentication>,
+    ) -> Result<DereferencedAuthenticationClasses, Error> {
+        let mut dereferenced_authentication_classes: Vec<core::v1alpha1::AuthenticationClass> =
+            vec![];
+
+        for auth_class in auth_classes {
+            dereferenced_authentication_classes.push(
+                core::v1alpha1::AuthenticationClass::resolve(
+                    client,
+                    &auth_class.authentication_class,
+                )
+                .await
+                .context(AuthenticationClassRetrievalSnafu {
+                    authentication_class: ObjectRef::<core::v1alpha1::AuthenticationClass>::new(
+                        &auth_class.authentication_class,
+                    ),
+                })?,
+            );
+        }
+
+        Ok(DereferencedAuthenticationClasses {
+            dereferenced_authentication_classes,
+        })
+    }
+
     /// Return the (first) TLS `AuthenticationClass` if available
     pub fn get_tls_authentication_class(&self) -> Option<&core::v1alpha1::AuthenticationClass> {
-        self.resolved_authentication_classes.iter().find(|auth| {
-            matches!(
-                auth.spec.provider,
-                core::v1alpha1::AuthenticationClassProvider::Tls(_)
-            )
-        })
+        self.dereferenced_authentication_classes
+            .iter()
+            .find(|auth| {
+                matches!(
+                    auth.spec.provider,
+                    core::v1alpha1::AuthenticationClassProvider::Tls(_)
+                )
+            })
     }
 
-    /// Validates the resolved AuthenticationClasses.
+    /// Validates the dereferenced AuthenticationClasses.
     /// Currently errors out if:
     /// - More than one AuthenticationClass was provided
     /// - AuthenticationClass mechanism was not supported
     pub fn validate(&self) -> Result<Self, Error> {
-        if self.resolved_authentication_classes.len() > 1 {
+        if self.dereferenced_authentication_classes.len() > 1 {
             return Err(Error::MultipleAuthenticationClassesProvided);
         }
 
-        for auth_class in &self.resolved_authentication_classes {
+        for auth_class in &self.dereferenced_authentication_classes {
             match &auth_class.spec.provider {
                 core::v1alpha1::AuthenticationClassProvider::Tls(_) => {}
                 core::v1alpha1::AuthenticationClassProvider::Ldap(_)
@@ -96,36 +128,8 @@ impl ResolvedAuthenticationClasses {
 
     /// USE ONLY IN TESTS! We can not put it behind `#[cfg(test)]` because of <https://github.com/rust-lang/cargo/issues/8379>
     pub fn new_for_tests() -> Self {
-        ResolvedAuthenticationClasses {
-            resolved_authentication_classes: vec![],
+        DereferencedAuthenticationClasses {
+            dereferenced_authentication_classes: vec![],
         }
     }
 }
-
-/// Resolve provided AuthenticationClasses via API calls and validate the contents.
-/// Currently errors out if:
-/// - AuthenticationClass could not be resolved
-/// - Validation failed
-pub async fn resolve_authentication_classes(
-    client: &Client,
-    auth_classes: &Vec<v1alpha1::ZookeeperAuthentication>,
-) -> Result<ResolvedAuthenticationClasses, Error> {
-    let mut resolved_authentication_classes: Vec<core::v1alpha1::AuthenticationClass> = vec![];
-
-    for auth_class in auth_classes {
-        resolved_authentication_classes.push(
-            core::v1alpha1::AuthenticationClass::resolve(client, &auth_class.authentication_class)
-                .await
-                .context(AuthenticationClassRetrievalSnafu {
-                    authentication_class: ObjectRef::<core::v1alpha1::AuthenticationClass>::new(
-                        &auth_class.authentication_class,
-                    ),
-                })?,
-        );
-    }
-
-    ResolvedAuthenticationClasses {
-        resolved_authentication_classes,
-    }
-    .validate()
-}

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

@@ -26,7 +26,7 @@ use stackable_operator::{
 };
 
 use crate::{
-    crd::{authentication::ResolvedAuthenticationClasses, tls, v1alpha1},
+    crd::{authentication::DereferencedAuthenticationClasses, tls, v1alpha1},
     zk_controller::LISTENER_VOLUME_NAME,
 };
 
@@ -51,7 +51,7 @@ pub enum Error {
 
 /// Helper struct combining TLS settings for server and quorum with the resolved AuthenticationClasses
 pub struct ZookeeperSecurity {
-    resolved_authentication_classes: ResolvedAuthenticationClasses,
+    resolved_authentication_classes: DereferencedAuthenticationClasses,
     server_secret_class: Option<String>,
     quorum_secret_class: String,
 }
@@ -90,11 +90,11 @@ impl ZookeeperSecurity {
     pub const SYSTEM_TRUST_STORE_DIR: &'static str = "/etc/pki/java/cacerts";
 
     /// Build a `ZookeeperSecurity` from a [`v1alpha1::ZookeeperCluster`] and already-resolved
-    /// [`ResolvedAuthenticationClasses`]. Synchronous; intended to be called from the validate
+    /// [`DereferencedAuthenticationClasses`]. Synchronous; intended to be called from the validate
     /// step of the controllers.
     pub fn new(
         zk: &v1alpha1::ZookeeperCluster,
-        resolved_authentication_classes: ResolvedAuthenticationClasses,
+        resolved_authentication_classes: DereferencedAuthenticationClasses,
     ) -> Self {
         ZookeeperSecurity {
             resolved_authentication_classes,
@@ -351,7 +351,7 @@ impl ZookeeperSecurity {
     /// USE ONLY IN TESTS! We can not put it behind `#[cfg(test)]` because of <https://github.com/rust-lang/cargo/issues/8379>
     pub fn new_for_tests() -> Self {
         ZookeeperSecurity {
-            resolved_authentication_classes: ResolvedAuthenticationClasses::new_for_tests(),
+            resolved_authentication_classes: DereferencedAuthenticationClasses::new_for_tests(),
             server_secret_class: Some("tls".to_owned()),
             quorum_secret_class: "tls".to_string(),
         }

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

@@ -9,36 +9,37 @@ use snafu::{ResultExt, Snafu};
 use stackable_operator::client::Client;
 
 use crate::crd::{
-    authentication::{self, ResolvedAuthenticationClasses},
+    authentication::{self, DereferencedAuthenticationClasses},
     v1alpha1,
 };
 
 #[derive(Snafu, Debug)]
 pub enum Error {
-    #[snafu(display("failed to resolve authentication classes"))]
-    ResolveAuthenticationClasses { source: authentication::Error },
+    #[snafu(display("failed to fetch authentication classes"))]
+    FetchAuthenticationClasses { source: authentication::Error },
 }
 
 type Result<T, E = Error> = std::result::Result<T, E>;
 
-/// Kubernetes objects referenced from the [`v1alpha1::ZookeeperCluster`] spec, already fetched.
+/// Kubernetes objects referenced from the [`v1alpha1::ZookeeperCluster`] spec, already fetched but
+/// not yet validated.
 pub struct DereferencedObjects {
-    pub resolved_authentication_classes: ResolvedAuthenticationClasses,
+    pub authentication_classes: DereferencedAuthenticationClasses,
 }
 
 /// 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(
+    let authentication_classes = DereferencedAuthenticationClasses::fetch_references(
         client,
         &zk.spec.cluster_config.authentication,
     )
     .await
-    .context(ResolveAuthenticationClassesSnafu)?;
+    .context(FetchAuthenticationClassesSnafu)?;
 
     Ok(DereferencedObjects {
-        resolved_authentication_classes,
+        authentication_classes,
     })
 }

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

@@ -17,7 +17,7 @@ use stackable_operator::{
 use crate::{
     crd::{
         CONTAINER_IMAGE_BASE_NAME, JVM_SECURITY_PROPERTIES_FILE, ZOOKEEPER_PROPERTIES_FILE,
-        ZookeeperRole, security::ZookeeperSecurity, v1alpha1,
+        ZookeeperRole, authentication, security::ZookeeperSecurity, v1alpha1,
     },
     zk_controller::dereference::DereferencedObjects,
 };
@@ -29,6 +29,9 @@ pub enum Error {
         source: product_image_selection::Error,
     },
 
+    #[snafu(display("failed to validate authentication classes"))]
+    InvalidAuthenticationClassConfiguration { source: authentication::Error },
+
     #[snafu(display("object defines no server role"))]
     NoServerRole,
 
@@ -69,10 +72,12 @@ pub fn validate(
         )
         .context(ResolveProductImageSnafu)?;
 
-    let zookeeper_security = ZookeeperSecurity::new(
-        zk,
-        dereferenced_objects.resolved_authentication_classes.clone(),
-    );
+    let resolved_authentication_classes = dereferenced_objects
+        .authentication_classes
+        .validate()
+        .context(InvalidAuthenticationClassConfigurationSnafu)?;
+
+    let zookeeper_security = ZookeeperSecurity::new(zk, resolved_authentication_classes);
 
     let validated_role_config =
         validated_product_config(zk, &resolved_product_image.product_version, product_config)?;

rust/operator-binary/src/znode_controller.rs

@@ -267,7 +267,7 @@ pub async fn reconcile_znode(
                     // block finalizer removal.
                     let zookeeper_security = ZookeeperSecurity::new(
                         &dereferenced.zk,
-                        dereferenced.resolved_authentication_classes.clone(),
+                        dereferenced.authentication_classes.clone(),
                     );
                     reconcile_cleanup(client, dereferenced.zk, &zookeeper_security, &znode_path)
                         .await

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

@@ -1,8 +1,9 @@
 //! The dereference step in the ZookeeperZnode controller.
 //!
 //! Fetches the parent [`v1alpha1::ZookeeperCluster`] referenced by the znode's
-//! `spec.clusterRef`, plus the [`ResolvedAuthenticationClasses`] of that cluster. Both Apply
-//! and Cleanup paths in `reconcile_znode` share this output.
+//! `spec.clusterRef`, plus the [`DereferencedAuthenticationClasses`] of that cluster. Both Apply
+//! and Cleanup paths in `reconcile_znode` share this output. Synchronous validation of the
+//! fetched objects happens in the validate step.
 
 use snafu::{ResultExt, Snafu};
 use stackable_operator::{
@@ -11,7 +12,7 @@ use stackable_operator::{
 };
 
 use crate::crd::{
-    authentication::{self, ResolvedAuthenticationClasses},
+    authentication::{self, DereferencedAuthenticationClasses},
     v1alpha1,
 };
 
@@ -32,16 +33,16 @@ pub enum Error {
         zk: ObjectRef<v1alpha1::ZookeeperCluster>,
     },
 
-    #[snafu(display("failed to resolve authentication classes"))]
-    ResolveAuthenticationClasses { source: authentication::Error },
+    #[snafu(display("failed to fetch authentication classes"))]
+    FetchAuthenticationClasses { source: authentication::Error },
 }
 
 type Result<T, E = Error> = std::result::Result<T, E>;
 
 /// Kubernetes objects referenced from the [`v1alpha1::ZookeeperZnode`] spec, already fetched.
 pub struct DereferencedObjects {
     pub zk: v1alpha1::ZookeeperCluster,
-    pub resolved_authentication_classes: ResolvedAuthenticationClasses,
+    pub authentication_classes: DereferencedAuthenticationClasses,
 }
 
 /// Fetches all Kubernetes objects referenced from the [`v1alpha1::ZookeeperZnode`] spec.
@@ -51,16 +52,16 @@ pub async fn dereference(
 ) -> Result<DereferencedObjects> {
     let zk = find_zk_of_znode(client, znode).await?;
 
-    let resolved_authentication_classes = authentication::resolve_authentication_classes(
+    let authentication_classes = DereferencedAuthenticationClasses::fetch_references(
         client,
         &zk.spec.cluster_config.authentication,
     )
     .await
-    .context(ResolveAuthenticationClassesSnafu)?;
+    .context(FetchAuthenticationClassesSnafu)?;
 
     Ok(DereferencedObjects {
         zk,
-        resolved_authentication_classes,
+        authentication_classes,
     })
 }
 

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

@@ -10,7 +10,7 @@ use stackable_operator::{
 };
 
 use crate::{
-    crd::{CONTAINER_IMAGE_BASE_NAME, security::ZookeeperSecurity, v1alpha1},
+    crd::{CONTAINER_IMAGE_BASE_NAME, authentication, security::ZookeeperSecurity, v1alpha1},
     znode_controller::dereference::DereferencedObjects,
 };
 
@@ -20,6 +20,9 @@ pub enum Error {
     ResolveProductImage {
         source: product_image_selection::Error,
     },
+
+    #[snafu(display("failed to validate authentication classes"))]
+    InvalidAuthenticationClassConfiguration { source: authentication::Error },
 }
 
 type Result<T, E = Error> = std::result::Result<T, E>;
@@ -47,10 +50,13 @@ pub fn validate(
         )
         .context(ResolveProductImageSnafu)?;
 
-    let zookeeper_security = ZookeeperSecurity::new(
-        &dereferenced_objects.zk,
-        dereferenced_objects.resolved_authentication_classes.clone(),
-    );
+    let resolved_authentication_classes = dereferenced_objects
+        .authentication_classes
+        .validate()
+        .context(InvalidAuthenticationClassConfigurationSnafu)?;
+
+    let zookeeper_security =
+        ZookeeperSecurity::new(&dereferenced_objects.zk, resolved_authentication_classes);
 
     Ok(ValidatedInputs {
         resolved_product_image,