refactor: Build RBAC with recommended labels (#846)

* factor: add infallible rbac functions

* reference op-rs remote branch not local one

* bump op-rs to 0.114.0

* rename resource names functions

* review feedback

* remove inaccurate statement from comment

Author: adwk67

CHANGELOG.md

@@ -8,9 +8,12 @@ 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 ([#841]).
+- The RBAC ServiceAccount and RoleBinding are now built with the operator-rs `v2::rbac`
+  functions and carry the full set of recommended labels ([#846]).
 - Bump stackable-operator to 0.114.0 ([#855]).
 
 [#841]: https://github.com/stackabletech/druid-operator/pull/841
+[#846]: https://github.com/stackabletech/druid-operator/pull/846
 [#855]: https://github.com/stackabletech/druid-operator/pull/855
 
 ## [26.7.0] - 2026-07-21

rust/operator-binary/src/controller.rs

@@ -8,19 +8,17 @@ use snafu::{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::{
-        ResourceExt,
         core::{DeserializeGuard, error_boundary},
         runtime::controller::Action,
     },
-    kvp::{KeyValuePairError, LabelValueError},
     logging::controller::ReconcilerError,
     shared::time::Duration,
     status::condition::{
@@ -83,6 +81,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>,
 }
 
 #[derive(Snafu, Debug, EnumDiscriminants)]
@@ -122,26 +122,6 @@ pub enum Error {
         source: crate::internal_secret::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 get required labels"))]
-    GetRequiredLabels {
-        source: KeyValuePairError<LabelValueError>,
-    },
-
     #[snafu(display("DruidCluster object is invalid"))]
     InvalidDruidCluster {
         source: error_boundary::InvalidObject,
@@ -196,39 +176,30 @@ pub async fn reconcile_druid(
         &druid.spec.object_overrides,
     );
 
-    let (rbac_sa, rbac_rolebinding) = build_rbac_resources(
-        druid,
-        APP_NAME,
-        cluster_resources
-            .get_required_labels()
-            .context(GetRequiredLabelsSnafu)?,
-    )
-    .context(BuildRbacResourcesSnafu)?;
-
-    // The ServiceAccount name is deterministic on the built object, so the StatefulSet builder only
-    // needs the name and does not depend on the applied ServiceAccount.
-    let service_account_name = rbac_sa.name_any();
-
-    cluster_resources
-        .add(client, rbac_sa)
-        .await
-        .context(ApplyServiceAccountSnafu)?;
-    cluster_resources
-        .add(client, rbac_rolebinding)
-        .await
-        .context(ApplyRoleBindingSnafu)?;
-
     // The internal secret is shared across all roles and role groups, so it only needs to be
     // created once per reconcile rather than inside the role loop below.
     create_shared_internal_secret(&validated_cluster, client, DRUID_CONTROLLER_NAME)
         .await
         .context(FailedInternalSecretCreationSnafu)?;
 
-    let resources =
-        build::build(&validated_cluster, &service_account_name).context(BuildResourcesSnafu)?;
+    let resources = build::build(&validated_cluster).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)?;
+    }
+
     // Apply order: everything a Pod mounts (ConfigMaps) must exist before the StatefulSets, so the
     // StatefulSets are applied last to prevent unnecessary Pod restarts.
     // See https://github.com/stackabletech/commons-operator/issues/111 for details.

rust/operator-binary/src/controller/build/mod.rs

@@ -3,7 +3,7 @@
 use std::str::FromStr;
 
 use snafu::{ResultExt, Snafu};
-use stackable_operator::v2::types::operator::{ProductVersion, RoleGroupName};
+use stackable_operator::v2::types::operator::RoleGroupName;
 
 use crate::{
     controller::{
@@ -12,6 +12,7 @@ use crate::{
             config_map::build_rolegroup_config_map,
             listener::{build_group_listener, group_listener_name},
             pdb::build_pdb,
+            rbac::{build_role_binding, build_service_account},
             service::{build_rolegroup_headless_service, build_rolegroup_metrics_service},
             statefulset::build_rolegroup_statefulset,
         },
@@ -26,11 +27,7 @@ stackable_operator::constant!(pub(crate) PLACEHOLDER_DISCOVERY_ROLE_GROUP: RoleG
 
 // Placeholder role-group name used for the recommended labels of the role-level `Listener`
 // (which is not tied to a single role group).
-stackable_operator::constant!(pub(crate) PLACEHOLDER_LISTENER_ROLE_GROUP: RoleGroupName = "none");
-
-// Placeholder product version used for labels on PVC templates, which cannot be modified once
-// deployed. A constant value keeps the labels stable across version upgrades.
-stackable_operator::constant!(pub(crate) UNVERSIONED_PRODUCT_VERSION: ProductVersion = "none");
+stackable_operator::constant!(pub(crate) NONE_ROLE_GROUP_NAME: RoleGroupName = "none");
 
 pub mod authentication;
 pub mod graceful_shutdown;
@@ -57,19 +54,13 @@ pub enum Error {
 /// Builds the Kubernetes resources for the given validated cluster.
 ///
 /// Does not need a Kubernetes client: every reference to another Kubernetes resource is already
-/// dereferenced and validated by this point. The remaining errors are resource-assembly failures
-/// only.
-///
-/// `service_account_name` is the name of the RBAC `ServiceAccount` the role-group Pods run under
-/// (RBAC resources are built and applied separately, in the reconcile step).
+/// dereferenced and validated by this point.
+/// The remaining errors are resource-assembly failures only.
 ///
 /// The Router group `Listener` and the discovery `ConfigMap`s are not built here: the discovery
 /// `ConfigMap` derives from the *applied* Router listener's ingress address, so both are built and
 /// applied in the reconcile step instead.
-pub fn build(
-    cluster: &ValidatedCluster,
-    service_account_name: &str,
-) -> Result<KubernetesResources, Error> {
+pub fn build(cluster: &ValidatedCluster) -> Result<KubernetesResources, Error> {
     let mut stateful_sets = vec![];
     let mut services = vec![];
     let mut listeners = vec![];
@@ -116,16 +107,11 @@ pub fn build(
                 )?,
             );
             stateful_sets.push(
-                build_rolegroup_statefulset(
-                    cluster,
-                    druid_role,
-                    role_group_name,
-                    rg,
-                    service_account_name,
-                )
-                .context(StatefulSetSnafu {
-                    role_group: role_group_name.clone(),
-                })?,
+                build_rolegroup_statefulset(cluster, druid_role, role_group_name, rg).context(
+                    StatefulSetSnafu {
+                        role_group: role_group_name.clone(),
+                    },
+                )?,
             );
         }
     }
@@ -136,11 +122,15 @@ pub fn build(
         listeners,
         config_maps,
         pod_disruption_budgets,
+        service_accounts: vec![build_service_account(cluster)],
+        role_bindings: vec![build_role_binding(cluster)],
     })
 }
 
 #[cfg(test)]
 mod tests {
+    use std::collections::BTreeMap;
+
     use stackable_operator::kube::Resource;
 
     use super::build;
@@ -161,7 +151,7 @@ mod tests {
     fn build_produces_expected_resource_names() {
         let druid = druid_from_yaml(MINIMAL_DRUID_YAML);
         let cluster = validated_cluster(&druid);
-        let resources = build(&cluster, "simple-druid-serviceaccount").expect("build succeeds");
+        let resources = build(&cluster).expect("build succeeds");
 
         // One StatefulSet and one ConfigMap per role group (one role group per role).
         let expected_role_group_names = [
@@ -188,4 +178,54 @@ mod tests {
             ["simple-druid-broker", "simple-druid-coordinator"]
         );
     }
+
+    /// Locks the RBAC resource names, the roleRef, and the recommended label set against
+    /// accidental drift. The fixture's cluster name deliberately differs from the product name so
+    /// that swapped `name`/`instance` label values cannot pass unnoticed.
+    #[test]
+    fn build_produces_rbac() {
+        let druid = druid_from_yaml(MINIMAL_DRUID_YAML);
+        let cluster = validated_cluster(&druid);
+        let resources = build(&cluster).expect("build succeeds");
+
+        assert_eq!(
+            sorted_names(&resources.service_accounts),
+            ["simple-druid-serviceaccount"]
+        );
+        assert_eq!(
+            sorted_names(&resources.role_bindings),
+            ["simple-druid-rolebinding"]
+        );
+
+        let expected_labels = BTreeMap::from(
+            [
+                ("app.kubernetes.io/component", "none"),
+                ("app.kubernetes.io/instance", "simple-druid"),
+                (
+                    "app.kubernetes.io/managed-by",
+                    "druid.stackable.tech_druidcluster",
+                ),
+                ("app.kubernetes.io/name", "druid"),
+                ("app.kubernetes.io/role-group", "none"),
+                ("app.kubernetes.io/version", "30.0.0-stackable0.0.0-dev"),
+                ("stackable.tech/vendor", "Stackable"),
+            ]
+            .map(|(key, value)| (key.to_string(), value.to_string())),
+        );
+        let service_account = resources
+            .service_accounts
+            .first()
+            .expect("a ServiceAccount is built");
+        assert_eq!(
+            service_account.metadata.labels,
+            Some(expected_labels.clone())
+        );
+
+        let role_binding = resources
+            .role_bindings
+            .first()
+            .expect("a RoleBinding is built");
+        assert_eq!(role_binding.metadata.labels, Some(expected_labels));
+        assert_eq!(role_binding.role_ref.name, "druid-clusterrole");
+    }
 }

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

@@ -151,7 +151,7 @@ pub fn build_rolegroup_config_map(
     role_group_name: &RoleGroupName,
     rg: &DruidRoleGroupConfig,
 ) -> Result<ConfigMap> {
-    let resource_names = cluster.resource_names(role, role_group_name);
+    let resource_names = cluster.role_group_resource_names(role, role_group_name);
     let cluster_config = &cluster.cluster_config;
     let druid_tls_security = &cluster_config.druid_tls_security;
     let druid_auth_config = &cluster_config.druid_auth_config;

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

@@ -15,7 +15,7 @@ use stackable_operator::{
 
 use crate::{
     controller::{
-        build::{PLACEHOLDER_LISTENER_ROLE_GROUP, security::listener_ports},
+        build::{NONE_ROLE_GROUP_NAME, security::listener_ports},
         validate::ValidatedCluster,
     },
     crd::{
@@ -52,7 +52,7 @@ pub fn build_group_listener(
             .object_meta(
                 listener_group_name.to_string(),
                 druid_role,
-                &PLACEHOLDER_LISTENER_ROLE_GROUP,
+                &NONE_ROLE_GROUP_NAME,
             )
             .build(),
         spec: listener::v1alpha1::ListenerSpec {

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

@@ -4,5 +4,6 @@ pub mod config_map;
 pub mod discovery;
 pub mod listener;
 pub mod pdb;
+pub mod rbac;
 pub mod service;
 pub mod statefulset;

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

@@ -27,7 +27,7 @@ pub fn build_pdb(
     let pdb = pod_disruption_budget_builder_with_role(
         cluster,
         &product_name(),
-        &role.to_role_name(),
+        &role.into(),
         &operator_name(),
         &controller_name(),
     )

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

@@ -0,0 +1,42 @@
+//! Builds the RBAC resources (ServiceAccount + RoleBinding) shared by all role groups.
+
+use std::str::FromStr;
+
+use stackable_operator::{
+    k8s_openapi::api::{core::v1::ServiceAccount, rbac::v1::RoleBinding},
+    kvp::Labels,
+    v2::{
+        rbac,
+        types::operator::{RoleGroupName, RoleName},
+    },
+};
+
+use crate::controller::validate::ValidatedCluster;
+
+stackable_operator::constant!(NONE_ROLE_NAME: RoleName = "none");
+stackable_operator::constant!(NONE_ROLE_GROUP_NAME: RoleGroupName = "none");
+
+/// Builds the [`ServiceAccount`] that all role-group Pods run under.
+pub fn build_service_account(cluster: &ValidatedCluster) -> ServiceAccount {
+    rbac::build_service_account(
+        cluster,
+        &cluster.cluster_resource_names(),
+        rbac_labels(cluster),
+    )
+}
+
+/// Builds the [`RoleBinding`] that binds the [`ServiceAccount`] from [`build_service_account`] to
+/// the operator-deployed ClusterRole.
+pub fn build_role_binding(cluster: &ValidatedCluster) -> RoleBinding {
+    rbac::build_role_binding(
+        cluster,
+        &cluster.cluster_resource_names(),
+        rbac_labels(cluster),
+    )
+}
+
+/// Both resources are shared by the whole cluster rather than tied to a role or role group, so
+/// the recommended labels carry `none` for both values.
+fn rbac_labels(cluster: &ValidatedCluster) -> Labels {
+    cluster.recommended_labels_for(&NONE_ROLE_NAME, &NONE_ROLE_GROUP_NAME)
+}

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

@@ -22,7 +22,7 @@ pub fn build_rolegroup_headless_service(
         metadata: cluster
             .object_meta(
                 cluster
-                    .resource_names(druid_role, role_group_name)
+                    .role_group_resource_names(druid_role, role_group_name)
                     .headless_service_name()
                     .to_string(),
                 druid_role,
@@ -59,7 +59,7 @@ pub fn build_rolegroup_metrics_service(
         metadata: cluster
             .object_meta(
                 cluster
-                    .resource_names(druid_role, role_group_name)
+                    .role_group_resource_names(druid_role, role_group_name)
                     .metrics_service_name()
                     .to_string(),
                 druid_role,