refactor: Build RBAC with recommended labels

CHANGELOG.md

@@ -9,8 +9,13 @@ All notable changes to this project will be documented in this file.
 - Internal operator refactoring: introduce a build() step in the reconciler that
   assembles all relevant Kubernetes resources before anything is applied ([#1053]).
 - Bump `stackable-operator` to 0.114.0 ([#1063]).
+- The RBAC ServiceAccount and RoleBinding are now built with the operator-rs `v2::rbac`
+  functions and carry the full set of recommended labels ([#1060]).
+- BREAKING: The `servers` role is now required by the CRD.
+  Previously a ZookeeperCluster without it was accepted by the API server but failed reconciliation ([#1060]).
 
 [#1053]: https://github.com/stackabletech/zookeeper-operator/pull/1053
+[#1060]: https://github.com/stackabletech/zookeeper-operator/pull/1060
 [#1063]: https://github.com/stackabletech/zookeeper-operator/pull/1063
 
 ## [26.7.0] - 2026-07-21

Cargo.toml

@@ -31,7 +31,7 @@ tokio-zookeeper = "0.4"
 tracing = "0.1"
 
 [patch."https://github.com/stackabletech/operator-rs.git"]
-# stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "smooth-operator" }
+# stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "main" }
 # stackable-operator = { path = "../operator-rs/crates/stackable-operator" }
 
 [patch.crates-io]

extra/crds.yaml

@@ -223,7 +223,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:
@@ -1303,6 +1302,7 @@ spec:
                 type: object
             required:
             - image
+            - servers
             type: object
           status:
             nullable: true

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

@@ -1,7 +1,6 @@
 use std::str::FromStr;
 
 use serde::{Deserialize, Serialize};
-use snafu::{OptionExt, Snafu};
 use stackable_operator::{
     commons::{
         affinity::StackableAffinity,
@@ -31,6 +30,7 @@ use stackable_operator::{
             kubernetes::{
                 ConfigMapName, ListenerClassName, ListenerName, NamespaceName, ServiceName,
             },
+            operator::RoleName,
         },
     },
     versioned::versioned,
@@ -81,12 +81,6 @@ pub const CONTAINER_IMAGE_BASE_NAME: &str = "zookeeper";
 const DEFAULT_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT: Duration = Duration::from_minutes_unchecked(2);
 pub const DEFAULT_LISTENER_CLASS: &str = "cluster-internal";
 
-#[derive(Snafu, Debug)]
-pub enum Error {
-    #[snafu(display("the role {role} is not defined"))]
-    CannotRetrieveZookeeperRole { role: String },
-}
-
 pub type ZookeeperServerRoleType = Role<
     v1alpha1::ZookeeperConfigFragment,
     v1alpha1::ZookeeperConfigOverrides,
@@ -136,8 +130,7 @@ pub mod versioned {
         pub image: ProductImage,
 
         // no doc - it's in the struct.
-        #[serde(skip_serializing_if = "Option::is_none")]
-        pub servers: Option<ZookeeperServerRoleType>,
+        pub servers: ZookeeperServerRoleType,
     }
 
     #[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)]
@@ -335,6 +328,18 @@ pub enum ZookeeperRole {
     Server,
 }
 
+impl From<ZookeeperRole> for RoleName {
+    fn from(value: ZookeeperRole) -> Self {
+        RoleName::from_str(&value.to_string()).expect("a ZookeeperRole is a valid role name")
+    }
+}
+
+impl From<&ZookeeperRole> for RoleName {
+    fn from(value: &ZookeeperRole) -> Self {
+        RoleName::from_str(&value.to_string()).expect("a ZookeeperRole is a valid role name")
+    }
+}
+
 /// Reference to a single `Pod` that is a component of a [`v1alpha1::ZookeeperCluster`]
 ///
 /// Used for service discovery.
@@ -446,19 +451,16 @@ impl v1alpha1::ZookeeperCluster {
         ))
     }
 
-    /// Returns a reference to the role. Raises an error if the role is not defined.
-    pub fn role(&self, role_variant: &ZookeeperRole) -> Result<&ZookeeperServerRoleType, Error> {
+    /// Returns the given role (the `servers` role is required by the CRD).
+    pub fn role(&self, role_variant: &ZookeeperRole) -> &ZookeeperServerRoleType {
         match role_variant {
-            ZookeeperRole::Server => self.spec.servers.as_ref(),
+            ZookeeperRole::Server => &self.spec.servers,
         }
-        .with_context(|| CannotRetrieveZookeeperRoleSnafu {
-            role: role_variant.to_string(),
-        })
     }
 
-    pub fn role_config(&self, role: &ZookeeperRole) -> Option<&ZookeeperServerRoleConfig> {
+    pub fn role_config(&self, role: &ZookeeperRole) -> &ZookeeperServerRoleConfig {
         match role {
-            ZookeeperRole::Server => self.spec.servers.as_ref().map(|s| &s.role_config),
+            ZookeeperRole::Server => &self.spec.servers.role_config,
         }
     }
 }
@@ -498,6 +500,10 @@ mod tests {
         spec:
           image:
             productVersion: "3.9.5"
+          servers:
+            roleGroups:
+              default:
+                replicas: 1
         "#;
         let zookeeper: v1alpha1::ZookeeperCluster =
             serde_yaml::from_str(input).expect("illegal test input");
@@ -518,6 +524,10 @@ mod tests {
         spec:
           image:
             productVersion: "3.9.5"
+          servers:
+            roleGroups:
+              default:
+                replicas: 1
           clusterConfig:
             tls:
               serverSecretClass: simple-zookeeper-client-tls
@@ -542,6 +552,10 @@ mod tests {
         spec:
           image:
             productVersion: "3.9.5"
+          servers:
+            roleGroups:
+              default:
+                replicas: 1
           clusterConfig:
             tls:
               serverSecretClass: null
@@ -562,6 +576,10 @@ mod tests {
         spec:
           image:
             productVersion: "3.9.5"
+          servers:
+            roleGroups:
+              default:
+                replicas: 1
           clusterConfig:
             tls:
               quorumSecretClass: simple-zookeeper-quorum-tls
@@ -588,6 +606,10 @@ mod tests {
         spec:
           image:
             productVersion: "3.9.5"
+          servers:
+            roleGroups:
+              default:
+                replicas: 1
         "#;
         let zookeeper: v1alpha1::ZookeeperCluster =
             serde_yaml::from_str(input).expect("illegal test input");
@@ -609,6 +631,10 @@ mod tests {
         spec:
           image:
             productVersion: "3.9.5"
+          servers:
+            roleGroups:
+              default:
+                replicas: 1
           clusterConfig:
             tls:
               quorumSecretClass: simple-zookeeper-quorum-tls
@@ -632,6 +658,10 @@ mod tests {
         spec:
           image:
             productVersion: "3.9.5"
+          servers:
+            roleGroups:
+              default:
+                replicas: 1
           clusterConfig:
             tls:
               serverSecretClass: simple-zookeeper-server-tls

rust/operator-binary/src/zk_controller.rs

@@ -7,19 +7,18 @@ use snafu::{OptionExt, ResultExt, Snafu};
 use stackable_operator::{
     cli::OperatorEnvironmentOptions,
     cluster_resources::ClusterResourceApplyStrategy,
-    commons::rbac::build_rbac_resources,
     crd::listener::v1alpha1::Listener,
     k8s_openapi::api::{
         apps::v1::StatefulSet,
-        core::v1::{ConfigMap, Service},
+        core::v1::{ConfigMap, Service, ServiceAccount},
         policy::v1::PodDisruptionBudget,
+        rbac::v1::RoleBinding,
     },
     kube::{
         api::DynamicObject,
         core::{DeserializeGuard, error_boundary},
         runtime::controller,
     },
-    kvp::LabelError,
     logging::controller::ReconcilerError,
     shared::time::Duration,
     status::condition::{
@@ -31,7 +30,7 @@ use stackable_operator::{
 use strum::{EnumDiscriminants, IntoStaticStr};
 
 use crate::{
-    APP_NAME, OPERATOR_NAME, ObjectRef,
+    OPERATOR_NAME, ObjectRef,
     crd::v1alpha1,
     zk_controller::{
         build::resource::discovery,
@@ -69,12 +68,6 @@ pub enum Error {
     #[snafu(display("failed to validate cluster"))]
     ValidateCluster { source: validate::Error },
 
-    #[snafu(display("crd validation failure"))]
-    CrdValidationFailure { source: crate::crd::Error },
-
-    #[snafu(display("internal operator failure"))]
-    InternalOperatorFailure { source: crate::crd::Error },
-
     #[snafu(display("failed to build the Kubernetes resources"))]
     BuildResources { source: build::Error },
 
@@ -106,29 +99,11 @@ pub enum Error {
         source: stackable_operator::client::Error,
     },
 
-    #[snafu(display("failed to create RBAC service account"))]
-    ApplyServiceAccount {
-        source: stackable_operator::cluster_resources::Error,
-    },
-
-    #[snafu(display("failed to create RBAC role binding"))]
-    ApplyRoleBinding {
-        source: stackable_operator::cluster_resources::Error,
-    },
-
-    #[snafu(display("failed to build RBAC resources"))]
-    BuildRbacResources {
-        source: stackable_operator::commons::rbac::Error,
-    },
-
     #[snafu(display("failed to delete orphaned resources"))]
     DeleteOrphans {
         source: stackable_operator::cluster_resources::Error,
     },
 
-    #[snafu(display("failed to build label"))]
-    BuildLabel { source: LabelError },
-
     #[snafu(display("failed to build object meta data"))]
     ObjectMeta {
         source: stackable_operator::builder::meta::Error,
@@ -145,20 +120,14 @@ impl ReconcilerError for Error {
             Error::InvalidZookeeperCluster { .. } => None,
             Error::Dereference { .. } => None,
             Error::ValidateCluster { .. } => None,
-            Error::CrdValidationFailure { .. } => None,
-            Error::InternalOperatorFailure { .. } => None,
             Error::BuildResources { .. } => None,
             Error::ApplyResource { .. } => None,
             Error::ObjectMissingMetadataForOwnerRef { .. } => None,
             Error::NoRoleListener => None,
             Error::BuildDiscoveryConfig { .. } => None,
             Error::ApplyDiscoveryConfig { .. } => None,
             Error::ApplyStatus { .. } => None,
-            Error::ApplyServiceAccount { .. } => None,
-            Error::ApplyRoleBinding { .. } => None,
-            Error::BuildRbacResources { .. } => None,
             Error::DeleteOrphans { .. } => None,
-            Error::BuildLabel { .. } => None,
             Error::ObjectMeta { .. } => None,
         }
     }
@@ -173,6 +142,8 @@ pub struct KubernetesResources {
     pub listeners: Vec<Listener>,
     pub config_maps: Vec<ConfigMap>,
     pub pod_disruption_budgets: Vec<PodDisruptionBudget>,
+    pub service_accounts: Vec<ServiceAccount>,
+    pub role_bindings: Vec<RoleBinding>,
 }
 
 pub async fn reconcile_zk(
@@ -209,30 +180,24 @@ pub async fn reconcile_zk(
         &validated_cluster.object_overrides,
     );
 
-    let (rbac_sa, rbac_rolebinding) = build_rbac_resources(
-        zk,
-        APP_NAME,
-        cluster_resources
-            .get_required_labels()
-            .context(BuildLabelSnafu)?,
-    )
-    .context(BuildRbacResourcesSnafu)?;
-
-    cluster_resources
-        .add(client, rbac_sa)
-        .await
-        .context(ApplyServiceAccountSnafu)?;
-
-    cluster_resources
-        .add(client, rbac_rolebinding)
-        .await
-        .context(ApplyRoleBindingSnafu)?;
-
     let resources = build::build(&validated_cluster, &client.kubernetes_cluster_info)
         .context(BuildResourcesSnafu)?;
 
     let mut ss_cond_builder = StatefulSetConditionBuilder::default();
 
+    for service_account in resources.service_accounts {
+        cluster_resources
+            .add(client, service_account)
+            .await
+            .context(ApplyResourceSnafu)?;
+    }
+    for role_binding in resources.role_bindings {
+        cluster_resources
+            .add(client, role_binding)
+            .await
+            .context(ApplyResourceSnafu)?;
+    }
+
     for service in resources.services {
         cluster_resources
             .add(client, service)
@@ -345,8 +310,22 @@ pub(crate) mod test_support {
         },
     };
 
+    /// The expected `app.kubernetes.io/version` label value for the given product version.
+    ///
+    /// The `-stackable` suffix carries the operator's own version, which is `0.0.0-dev` on main
+    /// but rewritten by the release process — so tests must derive it rather than hardcode it,
+    /// or they fail on release branches.
+    pub fn app_version_label(product_version: &str) -> String {
+        format!(
+            "{product_version}-stackable{}",
+            crate::built_info::PKG_VERSION
+        )
+    }
+
     /// Parses a minimal `ZookeeperCluster` test fixture, defaulting `namespace`/`uid` so the
-    /// validate step can build a [`ValidatedCluster`].
+    /// validate step can build a [`ValidatedCluster`]. Test fixtures name their clusters
+    /// `simple-zookeeper` — deliberately different from the product name (`zookeeper`), so tests
+    /// asserting recommended labels catch swapped `name`/`instance` values.
     pub fn minimal_zk(yaml: &str) -> v1alpha1::ZookeeperCluster {
         let mut zk: v1alpha1::ZookeeperCluster =
             serde_yaml::from_str(yaml).expect("invalid test ZookeeperCluster YAML");