refactor: Build RBAC with recommended labels

CHANGELOG.md

@@ -10,8 +10,11 @@ All notable changes to this project will be documented in this file.
   server reconcilers that assembles all relevant Kubernetes resources before anything
   is applied ([#721]).
 - Bump stackable-operator to 0.114.0 ([#732]).
+- The RBAC ServiceAccounts and RoleBindings of the history and connect servers are now
+  built with the operator-rs `v2::rbac` functions and carry the recommended labels ([#727]).
 
 [#721]: https://github.com/stackabletech/spark-k8s-operator/pull/721
+[#727]: https://github.com/stackabletech/spark-k8s-operator/pull/727
 [#732]: https://github.com/stackabletech/spark-k8s-operator/pull/732
 
 ## [26.7.0] - 2026-07-21

Cargo.toml

@@ -31,5 +31,5 @@ indoc = "2"
 regex = "1.12"
 
 [patch."https://github.com/stackabletech/operator-rs.git"]
-# stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch="main" }
+# stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch="feat/smooth-operator/build-rbac" }
 # stackable-operator = { path = "../operator-rs/crates/stackable-operator" }

rust/operator-binary/src/connect/common.rs

@@ -1,11 +1,12 @@
-use std::collections::BTreeMap;
+use std::{collections::BTreeMap, str::FromStr};
 
 use snafu::{ResultExt, Snafu};
 use stackable_operator::v2::{
     config_file_writer::{PropertiesWriterError, to_java_properties_string},
     role_utils::JavaCommonConfig,
+    types::operator::RoleName,
 };
-use strum::Display;
+use strum::{Display, EnumIter};
 
 use super::crd::CONNECT_EXECUTOR_ROLE_NAME;
 use crate::{
@@ -29,13 +30,30 @@ pub enum Error {
     MetricsProperties { source: PropertiesWriterError },
 }
 
-#[derive(Clone, Debug, Display)]
+#[derive(Clone, Debug, Display, EnumIter)]
 #[strum(serialize_all = "lowercase")]
 pub(crate) enum SparkConnectRole {
     Server,
     Executor,
 }
 
+impl From<SparkConnectRole> for RoleName {
+    fn from(value: SparkConnectRole) -> Self {
+        (&value).into()
+    }
+}
+
+impl From<&SparkConnectRole> for RoleName {
+    fn from(value: &SparkConnectRole) -> Self {
+        match value {
+            SparkConnectRole::Server => RoleName::from_str(CONNECT_SERVER_ROLE_NAME)
+                .expect("CONNECT_SERVER_ROLE_NAME is a valid role name"),
+            SparkConnectRole::Executor => RoleName::from_str(CONNECT_EXECUTOR_ROLE_NAME)
+                .expect("CONNECT_EXECUTOR_ROLE_NAME is a valid role name"),
+        }
+    }
+}
+
 pub(crate) fn object_name(stacklet_name: &str, role: SparkConnectRole) -> String {
     match role {
         SparkConnectRole::Server => format!("{}-{}", stacklet_name, CONNECT_SERVER_ROLE_NAME),
@@ -110,3 +128,21 @@ pub(crate) fn metrics_properties(
 
     to_java_properties_string(result.iter()).context(MetricsPropertiesSnafu)
 }
+
+#[cfg(test)]
+mod tests {
+    use stackable_operator::v2::types::operator::RoleName;
+    use strum::IntoEnumIterator;
+
+    use super::SparkConnectRole;
+
+    /// Locks the invariant behind the `expect` in the `From<SparkConnectRole> for RoleName`
+    /// impls: every variant (present and future) must map to a valid `RoleName`.
+    #[test]
+    fn every_spark_connect_role_maps_to_a_valid_role_name() {
+        for role in SparkConnectRole::iter() {
+            let _: RoleName = (&role).into();
+            let _: RoleName = role.into();
+        }
+    }
+}

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

@@ -3,7 +3,6 @@ use std::sync::Arc;
 use snafu::{ResultExt, Snafu};
 use stackable_operator::{
     cluster_resources::ClusterResourceApplyStrategy,
-    commons::rbac::build_rbac_resources,
     crd::listener,
     k8s_openapi::api::{
         apps::v1::StatefulSet,
@@ -25,7 +24,7 @@ use stackable_operator::{
 };
 use strum::{EnumDiscriminants, IntoStaticStr};
 
-use super::crd::{CONNECT_APP_NAME, v1alpha1};
+use super::crd::v1alpha1;
 use crate::{Ctx, connect::crd::SparkConnectServerStatus, crd::constants::OPERATOR_NAME};
 
 pub mod build;
@@ -44,16 +43,6 @@ pub enum Error {
         source: stackable_operator::cluster_resources::Error,
     },
 
-    #[snafu(display("failed to apply role ServiceAccount"))]
-    ApplyServiceAccount {
-        source: stackable_operator::cluster_resources::Error,
-    },
-
-    #[snafu(display("failed to apply global RoleBinding"))]
-    ApplyRoleBinding {
-        source: stackable_operator::cluster_resources::Error,
-    },
-
     #[snafu(display("failed to update status of spark connect server {name}"))]
     ApplyStatus {
         source: stackable_operator::client::Error,
@@ -65,22 +54,11 @@ pub enum Error {
         source: stackable_operator::cluster_resources::Error,
     },
 
-    #[snafu(display("failed to get required Labels"))]
-    GetRequiredLabels {
-        source:
-            stackable_operator::kvp::KeyValuePairError<stackable_operator::kvp::LabelValueError>,
-    },
-
     #[snafu(display("SparkConnectServer object is invalid"))]
     InvalidSparkConnectServer {
         source: error_boundary::InvalidObject,
     },
 
-    #[snafu(display("failed to build RBAC resources"))]
-    BuildRbacResources {
-        source: stackable_operator::commons::rbac::Error,
-    },
-
     #[snafu(display("failed to dereference SparkConnectServer"))]
     DereferenceSparkConnectServer { source: dereference::Error },
 
@@ -150,32 +128,19 @@ pub async fn reconcile(
         &scs.spec.object_overrides,
     );
 
-    // Use a dedicated service account for connect server pods. Building the RBAC resources needs
-    // the cluster-resource labels, so it stays in the reconcile step; the built objects (whose
-    // names are deterministic) are handed to the client-free build step.
-    let (service_account, role_binding) = build_rbac_resources(
-        scs,
-        CONNECT_APP_NAME,
-        cluster_resources
-            .get_required_labels()
-            .context(GetRequiredLabelsSnafu)?,
-    )
-    .context(BuildRbacResourcesSnafu)?;
-
-    let resources = build::build(&validated, service_account, role_binding, &scs.spec.args)
-        .context(BuildResourcesSnafu)?;
+    let resources = build::build(&validated, &scs.spec.args).context(BuildResourcesSnafu)?;
 
     // Apply order: ServiceAccount and RoleBinding first, then the Services, ConfigMaps and
     // Listener, and finally the StatefulSet (it mounts the ConfigMaps and runs under the SA, so
     // they must exist first).
     cluster_resources
         .add(client, resources.service_account)
         .await
-        .context(ApplyServiceAccountSnafu)?;
+        .context(ApplyResourceSnafu)?;
     cluster_resources
         .add(client, resources.role_binding)
         .await
-        .context(ApplyRoleBindingSnafu)?;
+        .context(ApplyResourceSnafu)?;
     for service in resources.services {
         cluster_resources
             .add(client, service)

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

@@ -6,18 +6,20 @@
 //! together in one module is clearer than scattering them across per-kind modules.
 
 pub(crate) mod executor;
+pub(crate) mod rbac;
 pub(crate) mod server;
 pub(crate) mod service;
 
 use snafu::{ResultExt, Snafu};
-use stackable_operator::{
-    k8s_openapi::api::{core::v1::ServiceAccount, rbac::v1::RoleBinding},
-    kube::ResourceExt,
-};
+use stackable_operator::kube::ResourceExt;
 
 use crate::connect::{
     common,
-    controller::{SparkConnectResources, validate::ValidatedSparkConnectServer},
+    controller::{
+        SparkConnectResources,
+        build::rbac::{build_role_binding, build_service_account},
+        validate::ValidatedSparkConnectServer,
+    },
 };
 
 #[derive(Snafu, Debug)]
@@ -56,8 +58,6 @@ pub enum Error {
 /// Builds every Kubernetes resource for the given validated SparkConnectServer.
 pub(crate) fn build(
     validated: &ValidatedSparkConnectServer,
-    service_account: ServiceAccount,
-    role_binding: RoleBinding,
     user_args: &[String],
 ) -> Result<SparkConnectResources, Error> {
     let resolved_s3 = &validated.cluster_config.resolved_s3;
@@ -70,8 +70,7 @@ pub(crate) fn build(
         resolved_s3
             .spark_properties()
             .context(S3SparkPropertiesSnafu)?,
-        server::server_properties(validated, &headless_service, &service_account)
-            .context(ServerPropertiesSnafu)?,
+        server::server_properties(validated, &headless_service).context(ServerPropertiesSnafu)?,
         executor::executor_properties(validated).context(ExecutorPropertiesSnafu)?,
     ])
     .context(SerializePropertiesSnafu)?;
@@ -97,21 +96,111 @@ pub(crate) fn build(
     let listener = server::build_listener(validated);
 
     let args = server::command_args(user_args);
-    let stateful_set = server::build_stateful_set(
-        validated,
-        &service_account,
-        &server_config_map,
-        &listener.name_any(),
-        args,
-    )
-    .context(BuildServerStatefulSetSnafu)?;
+    let stateful_set =
+        server::build_stateful_set(validated, &server_config_map, &listener.name_any(), args)
+            .context(BuildServerStatefulSetSnafu)?;
 
     Ok(SparkConnectResources {
-        service_account,
-        role_binding,
+        service_account: build_service_account(validated),
+        role_binding: build_role_binding(validated),
         services: vec![headless_service, metrics_service],
         config_maps: vec![executor_config_map, server_config_map],
         listener,
         stateful_set,
     })
 }
+
+#[cfg(test)]
+mod tests {
+    use std::collections::BTreeMap;
+
+    use indoc::indoc;
+    use stackable_operator::{cli::OperatorEnvironmentOptions, utils::yaml_from_str_singleton_map};
+
+    use super::build;
+    use crate::connect::{
+        controller::{
+            dereference::DereferencedSparkConnectServer,
+            validate::{ValidatedSparkConnectServer, validate},
+        },
+        crd::v1alpha1,
+        s3::ResolvedS3,
+    };
+
+    /// Minimal S3-free `SparkConnectServer` fixture, keeping the dereference step client-free;
+    /// the `uid` allows owner references to be derived from it.
+    const CONNECT_YAML: &str = indoc! {r#"
+        apiVersion: spark.stackable.tech/v1alpha1
+        kind: SparkConnectServer
+        metadata:
+          name: my-connect
+          namespace: default
+          uid: 12345678-1234-1234-1234-123456789012
+        spec:
+          image:
+            productVersion: 4.1.2
+        "#};
+
+    /// Runs the real validate step against the minimal fixture.
+    fn minimal_validated_cluster() -> ValidatedSparkConnectServer {
+        let scs: v1alpha1::SparkConnectServer = yaml_from_str_singleton_map(CONNECT_YAML)
+            .expect("invalid test SparkConnectServer YAML");
+        validate(
+            &scs,
+            DereferencedSparkConnectServer {
+                resolved_s3: ResolvedS3::none(),
+            },
+            &OperatorEnvironmentOptions {
+                operator_namespace: "stackable-operators".to_string(),
+                operator_service_name: "spark-k8s-operator".to_string(),
+                image_repository: "oci.example.org/sdp".to_string(),
+            },
+        )
+        .expect("validate should succeed for the test fixture")
+    }
+
+    /// 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 resources = build(&minimal_validated_cluster(), &[]).expect("build succeeds");
+
+        assert_eq!(
+            resources.service_account.metadata.name.as_deref(),
+            Some("my-connect-serviceaccount")
+        );
+        assert_eq!(
+            resources.role_binding.metadata.name.as_deref(),
+            Some("my-connect-rolebinding")
+        );
+
+        let expected_labels = BTreeMap::from(
+            [
+                ("app.kubernetes.io/component", "none"),
+                ("app.kubernetes.io/instance", "my-connect"),
+                (
+                    "app.kubernetes.io/managed-by",
+                    "spark.stackable.tech_connect",
+                ),
+                ("app.kubernetes.io/name", "spark-connect"),
+                ("app.kubernetes.io/role-group", "none"),
+                ("app.kubernetes.io/version", "4.1.2-stackable0.0.0-dev"),
+                ("stackable.tech/vendor", "Stackable"),
+            ]
+            .map(|(key, value)| (key.to_string(), value.to_string())),
+        );
+        assert_eq!(
+            resources.service_account.metadata.labels,
+            Some(expected_labels.clone())
+        );
+        assert_eq!(
+            resources.role_binding.metadata.labels,
+            Some(expected_labels)
+        );
+        assert_eq!(
+            resources.role_binding.role_ref.name,
+            "spark-connect-clusterrole"
+        );
+    }
+}

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

@@ -0,0 +1,37 @@
+//! 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::connect::controller::validate::ValidatedSparkConnectServer;
+
+stackable_operator::constant!(NONE_ROLE_NAME: RoleName = "none");
+stackable_operator::constant!(NONE_ROLE_GROUP_NAME: RoleGroupName = "none");
+
+pub fn build_service_account(server: &ValidatedSparkConnectServer) -> ServiceAccount {
+    rbac::build_service_account(
+        server,
+        &server.cluster_resource_names(),
+        rbac_labels(server),
+    )
+}
+
+pub fn build_role_binding(server: &ValidatedSparkConnectServer) -> RoleBinding {
+    rbac::build_role_binding(
+        server,
+        &server.cluster_resource_names(),
+        rbac_labels(server),
+    )
+}
+
+fn rbac_labels(server: &ValidatedSparkConnectServer) -> Labels {
+    server.recommended_labels_for(&NONE_ROLE_NAME, &NONE_ROLE_GROUP_NAME)
+}