refactor: use ConfigMapName, SecretName, ListenerName in CRD

Author: maltesander

extra/crds.yaml

@@ -491,6 +491,9 @@ spec:
                     properties:
                       listenerClass:
                         default: cluster-internal
+                        maxLength: 253
+                        minLength: 1
+                        pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
                         type: string
                       podDisruptionBudget:
                         default:
@@ -1091,6 +1094,9 @@ spec:
                               The [discovery ConfigMap](https://docs.stackable.tech/home/nightly/concepts/service_discovery)
                               for the HDFS instance. When running an HDFS cluster with the Stackable operator, the operator
                               will create this ConfigMap for you. It has the same name as your HDFSCluster resource.
+                            maxLength: 253
+                            minLength: 1
+                            pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
                             type: string
                           directory:
                             description: The directory inside of HDFS where Druid should store its data.
@@ -1574,7 +1580,10 @@ spec:
                           - If TLS encryption is used at all
                           - Which cert the servers should use to authenticate themselves against the clients
                           - Which cert the servers should use to authenticate themselves among each other
+                        maxLength: 253
+                        minLength: 1
                         nullable: true
+                        pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
                         type: string
                     type: object
                   vectorAggregatorConfigMapName:
@@ -1583,14 +1592,20 @@ spec:
                       It must contain the key `ADDRESS` with the address of the Vector aggregator.
                       Follow the [logging tutorial](https://docs.stackable.tech/home/nightly/tutorials/logging-vector-aggregator)
                       to learn how to configure log aggregation with Vector.
+                    maxLength: 253
+                    minLength: 1
                     nullable: true
+                    pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
                     type: string
                   zookeeperConfigMapName:
                     description: |-
                       Druid requires a ZooKeeper cluster connection to run.
                       Provide the name of the ZooKeeper [discovery ConfigMap](https://docs.stackable.tech/home/nightly/concepts/service_discovery)
                       here. When using the [Stackable operator for Apache ZooKeeper](https://docs.stackable.tech/home/nightly/zookeeper/)
                       to deploy a ZooKeeper cluster, this will simply be the name of your ZookeeperCluster resource.
+                    maxLength: 253
+                    minLength: 1
+                    pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
                     type: string
                 required:
                 - deepStorage
@@ -2090,6 +2105,9 @@ spec:
                     properties:
                       listenerClass:
                         default: cluster-internal
+                        maxLength: 253
+                        minLength: 1
+                        pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
                         type: string
                       podDisruptionBudget:
                         default:
@@ -5118,6 +5136,9 @@ spec:
                     properties:
                       listenerClass:
                         default: cluster-internal
+                        maxLength: 253
+                        minLength: 1
+                        pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
                         type: string
                       podDisruptionBudget:
                         default:

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

@@ -404,7 +404,7 @@ fn add_hdfs_cm_volume_and_volume_mounts(
             .context(AddVolumeMountSnafu)?;
         pb.add_volume(
             VolumeBuilder::new(HDFS_CONFIG_VOLUME_NAME)
-                .with_config_map(&hdfs.config_map_name)
+                .with_config_map(hdfs.config_map_name.to_string())
                 .build(),
         )
         .context(AddVolumeSnafu)?;

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

@@ -84,7 +84,11 @@ pub async fn dereference(
         .as_deref()
         .context(ObjectHasNoNamespaceSnafu)?;
 
-    let zk_confmap = druid.spec.cluster_config.zookeeper_config_map_name.clone();
+    let zk_confmap = druid
+        .spec
+        .cluster_config
+        .zookeeper_config_map_name
+        .to_string();
     let zookeeper_connection_string = client
         .get::<ConfigMap>(&zk_confmap, namespace)
         .await

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

@@ -39,7 +39,7 @@ pub fn get_affinity(
             {
                 vec![affinity_between_role_pods(
                     "hdfs",
-                    hdfs_discovery_cm_name, // The discovery cm has the same name as the HdfsCluster itself
+                    hdfs_discovery_cm_name.as_ref(), // The discovery cm has the same name as the HdfsCluster itself
                     "datanode",
                     50,
                 )]

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

@@ -128,10 +128,11 @@ impl AuthenticationClassesResolved {
                 .context(AuthenticationClassRetrievalFailedSnafu)?;
 
             let auth_class_name = auth_class.name_any();
-            let server_and_internal_secret_class = cluster_config
-                .tls
-                .as_ref()
-                .and_then(|tls| tls.server_and_internal_secret_class.to_owned());
+            let server_and_internal_secret_class = cluster_config.tls.as_ref().and_then(|tls| {
+                tls.server_and_internal_secret_class
+                    .as_ref()
+                    .map(|secret_class| secret_class.to_string())
+            });
 
             match &auth_class.spec.provider {
                 core::v1alpha1::AuthenticationClassProvider::Tls(provider) => {

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

@@ -45,7 +45,7 @@ use stackable_operator::{
         },
         role_utils::{JavaCommonConfig, RoleGroupConfig, with_validated_config},
         types::{
-            kubernetes::ConfigMapName,
+            kubernetes::{ConfigMapName, ListenerClassName},
             operator::{RoleGroupName, RoleName},
         },
     },
@@ -199,11 +199,6 @@ pub enum Error {
         "the Vector agent is enabled but the Vector aggregator ConfigMap name is missing"
     ))]
     MissingVectorAggregatorConfigMapName,
-
-    #[snafu(display("invalid Vector aggregator ConfigMap name"))]
-    ParseVectorAggregatorConfigMapName {
-        source: stackable_operator::v2::macros::attributed_string_type::Error,
-    },
 }
 
 #[versioned(
@@ -328,14 +323,14 @@ pub mod versioned {
         /// Provide the name of the ZooKeeper [discovery ConfigMap](DOCS_BASE_URL_PLACEHOLDER/concepts/service_discovery)
         /// here. When using the [Stackable operator for Apache ZooKeeper](DOCS_BASE_URL_PLACEHOLDER/zookeeper/)
         /// to deploy a ZooKeeper cluster, this will simply be the name of your ZookeeperCluster resource.
-        pub zookeeper_config_map_name: String,
+        pub zookeeper_config_map_name: ConfigMapName,
 
         /// Name of the Vector aggregator [discovery ConfigMap](DOCS_BASE_URL_PLACEHOLDER/concepts/service_discovery).
         /// It must contain the key `ADDRESS` with the address of the Vector aggregator.
         /// Follow the [logging tutorial](DOCS_BASE_URL_PLACEHOLDER/tutorials/logging-vector-aggregator)
         /// to learn how to configure log aggregation with Vector.
         #[serde(skip_serializing_if = "Option::is_none")]
-        pub vector_aggregator_config_map_name: Option<String>,
+        pub vector_aggregator_config_map_name: Option<ConfigMapName>,
 
         /// Extra volumes similar to `.spec.volumes` on a Pod to mount into every container, this can be useful to for
         /// example make client certificates, keytabs or similar things available to processors. These volumes will be
@@ -351,7 +346,7 @@ pub mod versioned {
         pub generic: GenericRoleConfig,
 
         #[serde(default = "druid_default_listener_class")]
-        pub listener_class: String,
+        pub listener_class: ListenerClassName,
     }
 }
 
@@ -468,16 +463,14 @@ impl v1alpha1::DruidCluster {
         let deep_storage = &self.spec.cluster_config.deep_storage;
         let name = self.name_any();
 
-        // The Vector aggregator discovery ConfigMap name (validated here so an invalid name fails
-        // up-front). It is only required when the Vector agent is enabled for a role group.
+        // The Vector aggregator discovery ConfigMap name (a typed `ConfigMapName`, so an invalid
+        // name is already rejected at deserialization). It is only required when the Vector agent
+        // is enabled for a role group.
         let vector_aggregator_config_map_name = self
             .spec
             .cluster_config
             .vector_aggregator_config_map_name
-            .as_deref()
-            .map(ConfigMapName::from_str)
-            .transpose()
-            .context(ParseVectorAggregatorConfigMapNameSnafu)?;
+            .clone();
 
         // All roles erase to an identical `ValidatedDruidConfig`; only the typed config, the role
         // config type and the `RoleResource` variant (historicals carry typed storage) differ.
@@ -705,8 +698,8 @@ impl Default for v1alpha1::DruidRoleConfig {
     }
 }
 
-fn druid_default_listener_class() -> String {
-    "cluster-internal".to_string()
+fn druid_default_listener_class() -> ListenerClassName {
+    ListenerClassName::from_str("cluster-internal").expect("a valid listener class name")
 }
 
 #[derive(
@@ -851,7 +844,7 @@ impl DruidRole {
         }
     }
 
-    pub fn listener_class_name(&self, druid: &v1alpha1::DruidCluster) -> Option<String> {
+    pub fn listener_class_name(&self, druid: &v1alpha1::DruidCluster) -> Option<ListenerClassName> {
         match self {
             Self::Broker => Some(druid.spec.brokers.role_config.listener_class.to_owned()),
             Self::Coordinator => Some(
@@ -893,7 +886,7 @@ pub struct HdfsDeepStorageSpec {
     /// The [discovery ConfigMap](DOCS_BASE_URL_PLACEHOLDER/concepts/service_discovery)
     /// for the HDFS instance. When running an HDFS cluster with the Stackable operator, the operator
     /// will create this ConfigMap for you. It has the same name as your HDFSCluster resource.
-    pub config_map_name: String,
+    pub config_map_name: ConfigMapName,
     /// The directory inside of HDFS where Druid should store its data.
     pub directory: String,
 }

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

@@ -119,12 +119,13 @@ impl DruidTlsSecurity {
     ) -> Self {
         DruidTlsSecurity {
             auth_classes: auth_classes.clone(),
-            server_and_internal_secret_class: druid
-                .spec
-                .cluster_config
-                .tls
-                .as_ref()
-                .and_then(|tls| tls.server_and_internal_secret_class.clone()),
+            server_and_internal_secret_class: druid.spec.cluster_config.tls.as_ref().and_then(
+                |tls| {
+                    tls.server_and_internal_secret_class
+                        .as_ref()
+                        .map(|secret_class| secret_class.to_string())
+                },
+            ),
         }
     }
 

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

@@ -1,5 +1,10 @@
+use std::str::FromStr;
+
 use serde::{Deserialize, Serialize};
-use stackable_operator::schemars::{self, JsonSchema};
+use stackable_operator::{
+    schemars::{self, JsonSchema},
+    v2::types::kubernetes::SecretClassName,
+};
 
 const TLS_DEFAULT_SECRET_CLASS: &str = "tls";
 
@@ -14,7 +19,7 @@ pub struct DruidTls {
     // happens via the HTTPS port. Even if both HTTPS and HTTP port are enabled, Druid clients
     // will default to using TLS.
     #[serde(default = "tls_default", skip_serializing_if = "Option::is_none")]
-    pub server_and_internal_secret_class: Option<String>,
+    pub server_and_internal_secret_class: Option<SecretClassName>,
 }
 
 /// Default TLS settings. Internal and server communication default to "tls" secret class.
@@ -25,14 +30,18 @@ pub fn default_druid_tls() -> Option<DruidTls> {
 }
 
 /// Helper methods to provide defaults in the CRDs and tests
-pub fn tls_default() -> Option<String> {
-    Some(TLS_DEFAULT_SECRET_CLASS.to_string())
+pub fn tls_default() -> Option<SecretClassName> {
+    Some(SecretClassName::from_str(TLS_DEFAULT_SECRET_CLASS).expect("a valid secret class name"))
 }
 
 #[cfg(test)]
 mod tests {
+    use std::str::FromStr;
+
     use indoc::formatdoc;
-    use stackable_operator::utils::yaml_from_str_singleton_map;
+    use stackable_operator::{
+        utils::yaml_from_str_singleton_map, v2::types::kubernetes::SecretClassName,
+    };
 
     use crate::crd::{tls::DruidTls, v1alpha1::DruidClusterConfig};
 
@@ -54,7 +63,9 @@ zookeeperConfigMapName: zk-config-map
         assert_eq!(
             druid_cluster_config.tls,
             Some(DruidTls {
-                server_and_internal_secret_class: Some("tls".to_string())
+                server_and_internal_secret_class: Some(
+                    SecretClassName::from_str("tls").expect("a valid secret class name")
+                )
             }),
         );
         assert_eq!(druid_cluster_config.authentication, vec![]);
@@ -73,7 +84,10 @@ zookeeperConfigMapName: zk-config-map
         assert_eq!(
             druid_cluster_config.tls,
             Some(DruidTls {
-                server_and_internal_secret_class: Some("druid-secret-class".to_string())
+                server_and_internal_secret_class: Some(
+                    SecretClassName::from_str("druid-secret-class")
+                        .expect("a valid secret class name")
+                )
             }),
         );
         assert_eq!(druid_cluster_config.authentication, vec![]);
@@ -126,7 +140,10 @@ zookeeperConfigMapName: zk-config-map
         assert_eq!(
             druid_cluster_config.tls,
             Some(DruidTls {
-                server_and_internal_secret_class: Some("druid-secret-class".to_string())
+                server_and_internal_secret_class: Some(
+                    SecretClassName::from_str("druid-secret-class")
+                        .expect("a valid secret class name")
+                )
             }),
         );
         /*

rust/operator-binary/src/main.rs

@@ -197,7 +197,12 @@ fn references_config_map(
         return false;
     };
 
-    druid.spec.cluster_config.zookeeper_config_map_name == config_map.name_any()
+    druid
+        .spec
+        .cluster_config
+        .zookeeper_config_map_name
+        .to_string()
+        == config_map.name_any()
         || match &druid.spec.cluster_config.authorization {
             Some(druid_authorization) => {
                 druid_authorization.opa.config_map_name == config_map.name_any()
@@ -206,7 +211,7 @@ fn references_config_map(
         }
         || match &druid.spec.cluster_config.deep_storage {
             crd::DeepStorageSpec::Hdfs(hdfs_spec) => {
-                hdfs_spec.config_map_name == config_map.name_any()
+                hdfs_spec.config_map_name.to_string() == config_map.name_any()
             }
             _ => false,
         }