make the broker role required in the CRD

Author: adwk67

CHANGELOG.md

@@ -11,6 +11,8 @@ All notable changes to this project will be documented in this file.
 - Bump stackable-operator to 0.114.0 ([#994]).
 - The RBAC ServiceAccount and RoleBinding are now built with the operator-rs `v2::rbac`
   functions and carry the full set of recommended labels ([#990]).
+- BREAKING: The `brokers` role is now required by the CRD; a KafkaCluster without it was
+  previously accepted by the API server but failed reconciliation ([#990]).
 
 [#985]: https://github.com/stackabletech/kafka-operator/pull/985
 [#990]: https://github.com/stackabletech/kafka-operator/pull/990

extra/crds.yaml

@@ -35,7 +35,6 @@ spec:
                   at role level, the `roleConfig`.
                   You can learn more about this in the
                   [Roles and role group concept documentation](https://docs.stackable.tech/home/nightly/concepts/roles-and-role-groups).
-                nullable: true
                 properties:
                   cliOverrides:
                     additionalProperties:
@@ -2260,6 +2259,7 @@ spec:
                   x-kubernetes-preserve-unknown-fields: true
                 type: array
             required:
+            - brokers
             - image
             type: object
           status:

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

@@ -35,7 +35,7 @@ use crate::{
         security::{self, ValidatedKafkaSecurity},
     },
     crd::{
-        self, CONTAINER_IMAGE_BASE_NAME,
+        CONTAINER_IMAGE_BASE_NAME,
         authentication::{self},
         role::{
             AnyConfig, AnyConfigOverrides, KafkaRole,
@@ -62,9 +62,6 @@ pub enum Error {
     #[snafu(display("failed to validate authentication method"))]
     FailedToValidateAuthenticationMethod { source: security::Error },
 
-    #[snafu(display("cluster object defines no '{role}' role"))]
-    MissingKafkaRole { source: crd::Error, role: KafkaRole },
-
     #[snafu(display("failed to merge and validate the role group config"))]
     ValidateRoleGroupConfig {
         source: stackable_operator::config::fragment::ValidationError,
@@ -249,10 +246,8 @@ pub fn validate(
         BTreeMap<RoleGroupName, ValidatedRoleGroupConfig>,
     > = BTreeMap::new();
 
-    // Brokers always exist.
-    let broker_role = kafka.broker_role().context(MissingKafkaRoleSnafu {
-        role: KafkaRole::Broker,
-    })?;
+    // The broker role is required by the CRD.
+    let broker_role = &kafka.spec.brokers;
     let broker_groups = validate_role_group_configs(
         broker_role,
         BrokerConfig::default_config(&kafka.name_any(), &KafkaRole::Broker.to_string()),

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

@@ -7,7 +7,7 @@ pub mod tls;
 
 use authentication::KafkaAuthentication;
 use serde::{Deserialize, Serialize};
-use snafu::{OptionExt, Snafu};
+use snafu::Snafu;
 use stackable_operator::{
     commons::{
         cluster_operation::ClusterOperation, networking::DomainName,
@@ -73,9 +73,6 @@ pub enum Error {
     ))]
     Kafka4RequiresKraftMetadataManager,
 
-    #[snafu(display("The Kafka role [{role}] is missing from spec"))]
-    MissingRole { role: String },
-
     #[snafu(display(
         "Kafka version 4 and higher requires a Kraft controller (configured via `spec.controller`)"
     ))]
@@ -130,7 +127,7 @@ pub mod versioned {
         pub image: ProductImage,
 
         // no doc - docs in Role struct.
-        pub brokers: Option<BrokerRole>,
+        pub brokers: BrokerRole,
 
         // no doc - docs in Role struct.
         pub controllers: Option<ControllerRole>,
@@ -320,12 +317,6 @@ impl v1alpha1::KafkaCluster {
                 _ => None,
             })
     }
-
-    pub fn broker_role(&self) -> Result<&BrokerRole, Error> {
-        self.spec.brokers.as_ref().context(MissingRoleSnafu {
-            role: KafkaRole::Broker.to_string(),
-        })
-    }
 }
 
 /// Reference to a single `Pod` that is a component of a [`KafkaCluster`]
@@ -444,6 +435,10 @@ mod tests {
         spec:
           image:
             productVersion: 3.9.2
+          brokers:
+            roleGroups:
+              default:
+                replicas: 1
           clusterConfig:
             zookeeperConfigMapName: xyz
         "#;
@@ -463,6 +458,10 @@ mod tests {
         spec:
           image:
             productVersion: 3.9.2
+          brokers:
+            roleGroups:
+              default:
+                replicas: 1
           clusterConfig:
             tls:
               serverSecretClass: simple-kafka-server-tls
@@ -488,6 +487,10 @@ mod tests {
         spec:
           image:
             productVersion: 3.9.2
+          brokers:
+            roleGroups:
+              default:
+                replicas: 1
           clusterConfig:
             tls:
               serverSecretClass: null
@@ -509,6 +512,10 @@ mod tests {
         spec:
           image:
             productVersion: 3.9.2
+          brokers:
+            roleGroups:
+              default:
+                replicas: 1
           zookeeperConfigMapName: xyz
           clusterConfig:
             tls:
@@ -534,6 +541,10 @@ mod tests {
         spec:
           image:
             productVersion: 3.9.2
+          brokers:
+            roleGroups:
+              default:
+                replicas: 1
           clusterConfig:
             zookeeperConfigMapName: xyz
         "#;
@@ -553,6 +564,10 @@ mod tests {
         spec:
           image:
             productVersion: 3.9.2
+          brokers:
+            roleGroups:
+              default:
+                replicas: 1
           clusterConfig:
             tls:
               internalSecretClass: simple-kafka-internal-tls
@@ -574,6 +589,10 @@ mod tests {
         spec:
           image:
             productVersion: 3.9.2
+          brokers:
+            roleGroups:
+              default:
+                replicas: 1
           clusterConfig:
             tls:
               serverSecretClass: simple-kafka-server-tls