factor: add infallible rbac functions

adwk67 · adwk67 · commit abd02d11079d · 2026-07-23T13:09:07.000+02:00
diff --git a/Cargo.toml b/Cargo.toml
@@ -30,4 +30,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 = { path = "../operator-rs/crates/stackable-operator" }
+stackable-operator = { path = "../operator-rs/crates/stackable-operator" }
diff --git a/rust/operator-binary/src/controller/build.rs b/rust/operator-binary/src/controller/build.rs
@@ -13,6 +13,7 @@ use crate::controller::{
         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,
             headless_service_ports,
@@ -52,13 +53,9 @@ pub enum Error {
 /// Does not need a Kubernetes client: every reference to another Kubernetes resource is already
 /// dereferenced and validated by this point, so the errors returned here 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).
 pub fn build(
     cluster: &ValidatedCluster,
     cluster_info: &KubernetesClusterInfo,
-    service_account_name: &str,
 ) -> Result<KubernetesResources, Error> {
     let mut stateful_sets = vec![];
     let mut services = vec![];
@@ -115,7 +112,6 @@ pub fn build(
                     role,
                     role_group_name,
                     role_group_config,
-                    service_account_name,
                 )
                 .context(StatefulSetSnafu {
                     role_group: role_group_name.clone(),
@@ -147,11 +143,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::{
         commons::networking::DomainName, kube::Resource, utils::cluster_info::KubernetesClusterInfo,
     };
@@ -177,8 +177,7 @@ mod tests {
                 .expect("cluster.local is a valid domain name"),
         };
 
-        let resources =
-            build(&cluster, &cluster_info, "simple-trino-serviceaccount").expect("build succeeds");
+        let resources = build(&cluster, &cluster_info).expect("build succeeds");
 
         // One StatefulSet per role group.
         assert_eq!(
@@ -219,4 +218,58 @@ mod tests {
             ["simple-trino-coordinator", "simple-trino-worker"]
         );
     }
+
+    /// 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 cluster = validated_cluster();
+        let cluster_info = KubernetesClusterInfo {
+            cluster_domain: DomainName::try_from("cluster.local")
+                .expect("cluster.local is a valid domain name"),
+        };
+
+        let resources = build(&cluster, &cluster_info).expect("build succeeds");
+
+        assert_eq!(
+            sorted_names(&resources.service_accounts),
+            ["simple-trino-serviceaccount"]
+        );
+        assert_eq!(
+            sorted_names(&resources.role_bindings),
+            ["simple-trino-rolebinding"]
+        );
+
+        let expected_labels = BTreeMap::from(
+            [
+                ("app.kubernetes.io/component", "none"),
+                ("app.kubernetes.io/instance", "simple-trino"),
+                (
+                    "app.kubernetes.io/managed-by",
+                    "trino.stackable.tech_trinocluster",
+                ),
+                ("app.kubernetes.io/name", "trino"),
+                ("app.kubernetes.io/role-group", "none"),
+                ("app.kubernetes.io/version", "481-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, "trino-clusterrole");
+    }
 }
diff --git a/rust/operator-binary/src/controller/build/resource/config_map.rs b/rust/operator-binary/src/controller/build/resource/config_map.rs
@@ -164,7 +164,7 @@ pub fn build_rolegroup_config_map(
     // 8. jvm.config. The role + role-group `jvmArgumentOverrides` were already merged in the
     // validate step and are carried by `product_specific_common_config`.
     let jvm_config = jvm::jvm_config(
-        cluster.product_version,
+        cluster.numeric_product_version,
         &rg.config,
         &rg.product_specific_common_config.jvm_argument_overrides,
     )
diff --git a/rust/operator-binary/src/controller/build/resource/mod.rs b/rust/operator-binary/src/controller/build/resource/mod.rs
@@ -7,5 +7,6 @@
 pub mod config_map;
 pub mod listener;
 pub mod pdb;
+pub mod rbac;
 pub mod service;
 pub mod statefulset;
diff --git a/rust/operator-binary/src/controller/build/resource/rbac.rs b/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::ValidatedCluster;
+
+stackable_operator::constant!(NONE_ROLE_NAME: RoleName = "none");
+stackable_operator::constant!(NONE_ROLE_GROUP_NAME: RoleGroupName = "none");
+
+/// Builds the [`ServiceAccount`] that the coordinator and worker Pods run under.
+pub fn build_service_account(cluster: &ValidatedCluster) -> ServiceAccount {
+    rbac::build_service_account(
+        cluster,
+        &cluster.rbac_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.rbac_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)
+}
diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs
@@ -41,9 +41,9 @@ use crate::{
     authorization::opa::OPA_TLS_VOLUME_NAME,
     controller::{
         MAX_PREPARE_LOG_FILE_SIZE, RoleGroupName, STACKABLE_LOG_CONFIG_DIR, STACKABLE_LOG_DIR,
-        TrinoRoleGroupConfig, ValidatedCluster, build,
+        TrinoRoleGroupConfig, ValidatedCluster,
         build::{
-            command,
+            self, command,
             resource::listener::{
                 LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME, build_group_listener_pvc,
                 group_listener_name, secret_volume_listener_scope,
@@ -137,10 +137,9 @@ pub fn build_rolegroup_statefulset(
     trino_role: &TrinoRole,
     role_group_name: &RoleGroupName,
     role_group_config: &TrinoRoleGroupConfig,
-    sa_name: &str,
 ) -> Result<StatefulSet> {
     // Everything below is derived from the validated cluster and the validated role-group config,
-    // so the caller only needs to pass those (plus the applied ServiceAccount name).
+    // so the caller only needs to pass those.
     let resolved_product_image = &cluster.image;
     let trino_authentication_config = &cluster.cluster_config.authentication;
     let catalogs = &cluster.cluster_config.catalogs;
@@ -422,7 +421,12 @@ pub fn build_rolegroup_statefulset(
             )),
         )
         .context(AddVolumeSnafu)?
-        .service_account_name(sa_name)
+        .service_account_name(
+            cluster
+                .rbac_resource_names()
+                .service_account_name()
+                .to_string(),
+        )
         .security_context(PodSecurityContextBuilder::new().fs_group(1000).build());
 
     let mut pod_template = pod_builder.build_template();
diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs
@@ -10,8 +10,9 @@ use stackable_operator::{
     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::{Resource, api::ObjectMeta},
     kvp::Labels,
@@ -22,6 +23,7 @@ use stackable_operator::{
         builder::meta::ownerreference_from_resource,
         kvp::label::{recommended_labels, role_group_selector},
         role_group_utils::ResourceNames,
+        role_utils,
         types::{
             kubernetes::{ListenerClassName, NamespaceName, SecretClassName, Uid},
             operator::{
@@ -64,13 +66,18 @@ pub(crate) fn shared_spooling_secret_name(cluster_name: &ClusterName) -> String
     format!("{cluster_name}-spooling-secret")
 }
 
+// Placeholder version label value for resources whose labels must not change after deployment.
+stackable_operator::constant!(UNVERSIONED_PRODUCT_VERSION: ProductVersion = "none");
+
 /// Every Kubernetes resource produced by the client-free [`build()`](build::build) step.
 pub struct KubernetesResources {
     pub stateful_sets: Vec<StatefulSet>,
     pub services: Vec<Service>,
     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(Clone, Debug)]
@@ -153,7 +160,11 @@ pub struct ValidatedCluster {
     pub namespace: NamespaceName,
     pub uid: Uid,
     pub image: ResolvedProductImage,
-    pub product_version: u16,
+    /// The numeric Trino version (e.g. `481`), used for version-dependent configuration.
+    pub numeric_product_version: u16,
+    /// The version label value (`app.kubernetes.io/version`) as a type-safe [`ProductVersion`],
+    /// parsed once from the resolved image's app version label value.
+    pub product_version: ProductVersion,
     pub cluster_config: ValidatedClusterConfig,
     pub role_configs: BTreeMap<TrinoRole, ValidatedRoleConfig>,
     pub role_group_configs: BTreeMap<TrinoRole, BTreeMap<RoleGroupName, TrinoRoleGroupConfig>>,
@@ -166,7 +177,7 @@ impl ValidatedCluster {
         namespace: NamespaceName,
         uid: Uid,
         image: ResolvedProductImage,
-        product_version: u16,
+        numeric_product_version: u16,
         cluster_config: ValidatedClusterConfig,
         role_configs: BTreeMap<TrinoRole, ValidatedRoleConfig>,
         role_group_configs: BTreeMap<TrinoRole, BTreeMap<RoleGroupName, TrinoRoleGroupConfig>>,
@@ -181,8 +192,10 @@ impl ValidatedCluster {
             name,
             namespace,
             uid,
+            product_version: ProductVersion::from_str(&image.app_version_label_value)
+                .expect("the app version label value is a valid product version"),
             image,
-            product_version,
+            numeric_product_version,
             cluster_config,
             role_configs,
             role_group_configs,
@@ -219,6 +232,15 @@ impl ValidatedCluster {
         self.cluster_config.authentication_enabled() || self.server_tls_enabled()
     }
 
+    /// Type-safe names for the per-cluster RBAC resources: the ServiceAccount shared by all
+    /// Pods, its (namespaced) RoleBinding, and the operator-deployed ClusterRole it binds.
+    pub fn rbac_resource_names(&self) -> role_utils::ResourceNames {
+        role_utils::ResourceNames {
+            cluster_name: self.name.clone(),
+            product_name: product_name(),
+        }
+    }
+
     /// Type-safe names for the resources of a given role group.
     pub(crate) fn resource_names(
         &self,
@@ -270,16 +292,10 @@ impl ValidatedCluster {
         RoleName::from_str(&role.to_string()).expect("a TrinoRole is a valid RFC 1123 role name")
     }
 
-    /// The version label value (`app.kubernetes.io/version`) as a type-safe [`ProductVersion`].
-    fn version_label(&self) -> ProductVersion {
-        ProductVersion::from_str(&self.image.app_version_label_value)
-            .expect("the app version label value is a valid product version")
-    }
-
-    fn recommended_labels_with_version(
+    fn recommended_labels_with(
         &self,
         version: &ProductVersion,
-        role: &TrinoRole,
+        role_name: &RoleName,
         role_group_name: &RoleGroupName,
     ) -> Labels {
         recommended_labels(
@@ -288,38 +304,42 @@ impl ValidatedCluster {
             version,
             &operator_name(),
             &controller_name(),
-            &Self::role_name(role),
+            role_name,
             role_group_name,
         )
     }
 
     /// Recommended labels for a role-group resource (using the resolved product version).
-    pub(crate) fn recommended_labels(
+    pub fn recommended_labels(&self, role: &TrinoRole, role_group_name: &RoleGroupName) -> Labels {
+        self.recommended_labels_for(&Self::role_name(role), role_group_name)
+    }
+
+    /// Recommended labels for a resource that is not tied to a concrete [`TrinoRole`] (e.g. the
+    /// cluster-shared RBAC resources), using a free-form role/role-group label value.
+    pub fn recommended_labels_for(
         &self,
-        role: &TrinoRole,
+        role_name: &RoleName,
         role_group_name: &RoleGroupName,
     ) -> Labels {
-        self.recommended_labels_with_version(&self.version_label(), role, role_group_name)
+        self.recommended_labels_with(&self.product_version, role_name, role_group_name)
     }
 
-    /// Recommended labels using a fixed `"none"` version, for resources whose labels must not
-    /// change after creation (e.g. listener PVC templates).
-    pub(crate) fn unversioned_recommended_labels(
+    /// Recommended labels with the constant [`UNVERSIONED_PRODUCT_VERSION`], for resources whose
+    /// labels must not change after creation (e.g. listener PVC templates).
+    pub fn unversioned_recommended_labels(
         &self,
         role: &TrinoRole,
         role_group_name: &RoleGroupName,
     ) -> Labels {
-        let none = ProductVersion::from_str("none")
-            .expect("\"none\" is a valid product version label value");
-        self.recommended_labels_with_version(&none, role, role_group_name)
+        self.recommended_labels_with(
+            &UNVERSIONED_PRODUCT_VERSION,
+            &Self::role_name(role),
+            role_group_name,
+        )
     }
 
     /// Selector labels matching the pods of a role group.
-    pub(crate) fn role_group_selector(
-        &self,
-        role: &TrinoRole,
-        role_group_name: &RoleGroupName,
-    ) -> Labels {
+    pub fn role_group_selector(&self, role: &TrinoRole, role_group_name: &RoleGroupName) -> Labels {
         role_group_selector(
             self,
             &product_name(),
diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs
diff --git a/rust/operator-binary/src/trino_controller.rs b/rust/operator-binary/src/trino_controller.rs

Original file line number	Diff line number	Diff line change
`@@ -164,7 +164,7 @@ pub fn build_rolegroup_config_map(`
`164`	`164`	// 8. jvm.config. The role + role-group `jvmArgumentOverrides` were already merged in the
`165`	`165`	// validate step and are carried by `product_specific_common_config`.
`166`	`166`	`let jvm_config = jvm::jvm_config(`
`167`		`- cluster.product_version,`
	`167`	`+ cluster.numeric_product_version,`
`168`	`168`	`&rg.config,`
`169`	`169`	`&rg.product_specific_common_config.jvm_argument_overrides,`
`170`	`170`	`)`