diff --git a/CHANGELOG.md b/CHANGELOG.md index fb37d6ad..7c2398e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,9 +6,12 @@ - Internal operator refactoring: introduce a build() step in the reconciler that assembles all relevant Kubernetes resources before anything is applied ([#776]). +- The RBAC ServiceAccount and RoleBinding are now built with the operator-rs `v2::rbac` + functions and carry the full set of recommended labels ([#782]). - Bump stackable-operator to 0.114.0 ([#786]). [#776]: https://github.com/stackabletech/hbase-operator/pull/776 +[#782]: https://github.com/stackabletech/hbase-operator/pull/782 [#786]: https://github.com/stackabletech/hbase-operator/pull/786 ## [26.7.0] - 2026-07-21 diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index 1c5dac7a..dd8fea79 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -1,4 +1,4 @@ -//! Builders that turn a [`ValidatedCluster`](crate::controller::ValidatedCluster) into +//! Builders that turn a [`ValidatedCluster`] into //! Kubernetes resources. use std::str::FromStr; @@ -15,6 +15,7 @@ use crate::{ config_map::{self, build_rolegroup_config_map}, discovery::{self, build_discovery_config_map}, pdb::build_pdb, + rbac::{build_role_binding, build_service_account}, service::{build_rolegroup_metrics_service, build_rolegroup_service}, statefulset::{self, build_rolegroup_statefulset}, }, @@ -50,13 +51,10 @@ 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. `cluster_info` is static cluster metadata (not a client call), and -/// `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). +/// failures only. `cluster_info` is static cluster metadata (not a client call). pub fn build( cluster: &ValidatedCluster, cluster_info: &KubernetesClusterInfo, - service_account_name: &str, ) -> Result { let mut stateful_sets = vec![]; let mut services = vec![]; @@ -83,17 +81,11 @@ pub fn build( })?, ); stateful_sets.push( - build_rolegroup_statefulset( - cluster, - hbase_role, - role_group_name, - rg_config, - service_account_name, - ) - .with_context(|_| StatefulSetSnafu { - hbase_role: hbase_role.clone(), - role_group: role_group_name.clone(), - })?, + build_rolegroup_statefulset(cluster, hbase_role, role_group_name, rg_config) + .with_context(|_| StatefulSetSnafu { + hbase_role: hbase_role.clone(), + role_group: role_group_name.clone(), + })?, ); } @@ -113,6 +105,8 @@ pub fn build( services, config_maps, pod_disruption_budgets, + service_accounts: vec![build_service_account(cluster)], + role_bindings: vec![build_role_binding(cluster)], }) } @@ -127,6 +121,8 @@ pub mod role; #[cfg(test)] mod tests { + use std::collections::BTreeMap; + use stackable_operator::kube::Resource; use super::build; @@ -146,8 +142,7 @@ mod tests { fn build_produces_expected_resource_names() { let cluster = test_utils::validated_cluster(); let cluster_info = test_utils::cluster_info(); - let resources = - build(&cluster, &cluster_info, "hbase-serviceaccount").expect("build succeeds"); + let resources = build(&cluster, &cluster_info).expect("build succeeds"); // One StatefulSet per role group (one `default` group for each of the three roles). assert_eq!( @@ -186,4 +181,58 @@ mod tests { ["hbase-master", "hbase-regionserver", "hbase-restserver"] ); } + + /// Locks the RBAC resource names, the roleRef, and the recommended label set against + /// accidental drift. The cluster name deliberately differs from the product name so that + /// swapped `name`/`instance` label values cannot pass unnoticed (the shared fixture is named + /// `hbase`, which would mask exactly that swap). + #[test] + fn build_produces_rbac() { + let hbase = test_utils::hbase_from_yaml( + &test_utils::MINIMAL_HBASE_YAML.replace("name: hbase", "name: my-hbase"), + ); + let cluster = test_utils::validated_cluster_from(&hbase); + let cluster_info = test_utils::cluster_info(); + let resources = build(&cluster, &cluster_info).expect("build succeeds"); + + assert_eq!( + sorted_names(&resources.service_accounts), + ["my-hbase-serviceaccount"] + ); + assert_eq!( + sorted_names(&resources.role_bindings), + ["my-hbase-rolebinding"] + ); + + let expected_labels = BTreeMap::from( + [ + ("app.kubernetes.io/component", "none"), + ("app.kubernetes.io/instance", "my-hbase"), + ( + "app.kubernetes.io/managed-by", + "hbase.stackable.com_hbasecluster", + ), + ("app.kubernetes.io/name", "hbase"), + ("app.kubernetes.io/role-group", "none"), + ("app.kubernetes.io/version", "2.6.3-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, "hbase-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 index 8e5037e1..8c3f1030 100644 --- a/rust/operator-binary/src/controller/build/resource/config_map.rs +++ b/rust/operator-binary/src/controller/build/resource/config_map.rs @@ -111,7 +111,7 @@ pub fn build_rolegroup_config_map( let cm_metadata = cluster .object_meta( cluster - .resource_names(role, role_group_name) + .role_group_resource_names(role, role_group_name) .role_group_config_map() .to_string(), role, diff --git a/rust/operator-binary/src/controller/build/resource/mod.rs b/rust/operator-binary/src/controller/build/resource/mod.rs index f9b822cd..6bfb9adb 100644 --- a/rust/operator-binary/src/controller/build/resource/mod.rs +++ b/rust/operator-binary/src/controller/build/resource/mod.rs @@ -1,9 +1,9 @@ -//! Builders that turn a [`ValidatedCluster`](crate::controller::ValidatedCluster) into -//! Kubernetes resources. +//! Builders for individual Kubernetes resources (one module per resource type). pub mod config_map; pub mod discovery; 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/pdb.rs b/rust/operator-binary/src/controller/build/resource/pdb.rs index e1a57531..81fdf339 100644 --- a/rust/operator-binary/src/controller/build/resource/pdb.rs +++ b/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(), - &ValidatedCluster::role_name(role), + &role.into(), &operator_name(), &controller_name(), ) diff --git a/rust/operator-binary/src/controller/build/resource/rbac.rs b/rust/operator-binary/src/controller/build/resource/rbac.rs new file mode 100644 index 00000000..e59a8818 --- /dev/null +++ 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 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) +} diff --git a/rust/operator-binary/src/controller/build/resource/service.rs b/rust/operator-binary/src/controller/build/resource/service.rs index 103b1b67..f12025b8 100644 --- a/rust/operator-binary/src/controller/build/resource/service.rs +++ b/rust/operator-binary/src/controller/build/resource/service.rs @@ -35,7 +35,7 @@ pub fn build_rolegroup_service( metadata: cluster .object_meta( cluster - .resource_names(hbase_role, role_group_name) + .role_group_resource_names(hbase_role, role_group_name) .headless_service_name() .to_string(), hbase_role, @@ -77,7 +77,7 @@ pub fn build_rolegroup_metrics_service( metadata: cluster .object_meta( cluster - .resource_names(hbase_role, role_group_name) + .role_group_resource_names(hbase_role, role_group_name) .metrics_service_name() .to_string(), hbase_role, diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs index 1ae98a9c..8c4b722e 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -106,12 +106,11 @@ pub fn build_rolegroup_statefulset( hbase_role: &HbaseRole, role_group_name: &RoleGroupName, validated_rg_config: &HbaseRoleGroupConfig, - service_account_name: &str, ) -> Result { let resolved_product_image = &cluster.image; let merged_config = &validated_rg_config.config.config; let logging = &validated_rg_config.config.logging; - let resource_names = cluster.resource_names(hbase_role, role_group_name); + let resource_names = cluster.role_group_resource_names(hbase_role, role_group_name); let https_enabled = cluster.has_https_enabled(); let ports = hbase_role @@ -239,7 +238,12 @@ pub fn build_rolegroup_statefulset( )), ) .context(AddVolumeSnafu)? - .service_account_name(service_account_name) + .service_account_name( + cluster + .cluster_resource_names() + .service_account_name() + .to_string(), + ) .security_context(PodSecurityContextBuilder::new().fs_group(1000).build()); // The HBase container's log config ConfigMap: either the operator-generated one (the diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index 2cb2c9e6..3c5c28a5 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -13,8 +13,9 @@ use stackable_operator::{ k8s_openapi::{ api::{ apps::v1::StatefulSet, - core::v1::{ConfigMap, Service}, + core::v1::{ConfigMap, Service, ServiceAccount}, policy::v1::PodDisruptionBudget, + rbac::v1::RoleBinding, }, apimachinery::pkg::apis::meta::v1::ObjectMeta, }, @@ -25,6 +26,7 @@ use stackable_operator::{ builder::meta::ownerreference_from_resource, kvp::label::{recommended_labels, role_group_selector}, role_group_utils::ResourceNames, + role_utils, types::{ kubernetes::{ConfigMapName, NamespaceName, SecretClassName, Uid}, operator::{ @@ -67,6 +69,8 @@ pub struct KubernetesResources { pub services: Vec, pub config_maps: Vec, pub pod_disruption_budgets: Vec, + pub service_accounts: Vec, + pub role_bindings: Vec, } /// The validated cluster: proves that config merging and validation succeeded for @@ -126,38 +130,56 @@ impl ValidatedCluster { } } - /// The Kubernetes role name for an [`HbaseRole`] (e.g. `master`, `regionserver`, - /// `restserver`). - pub fn role_name(hbase_role: &HbaseRole) -> RoleName { - RoleName::from_str(&hbase_role.to_string()).expect("an HbaseRole name is a valid role name") + /// 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 cluster_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( + pub(crate) fn role_group_resource_names( &self, hbase_role: &HbaseRole, role_group_name: &RoleGroupName, ) -> ResourceNames { ResourceNames { cluster_name: self.name.clone(), - role_name: Self::role_name(hbase_role), + role_name: hbase_role.into(), role_group_name: role_group_name.clone(), } } /// Recommended labels for a role-group resource. - pub fn recommended_labels( + pub fn recommended_labels(&self, role: &HbaseRole, role_group_name: &RoleGroupName) -> Labels { + self.recommended_labels_for(&role.into(), role_group_name) + } + + /// Recommended labels for a resource that is not tied to a concrete [`HbaseRole`] (e.g. the + /// Kubernetes executor pod template), using a free-form role/role-group label value. + pub fn recommended_labels_for( &self, - hbase_role: &HbaseRole, + role_name: &RoleName, + role_group_name: &RoleGroupName, + ) -> Labels { + self.recommended_labels_with(&self.product_version, role_name, role_group_name) + } + + fn recommended_labels_with( + &self, + product_version: &ProductVersion, + role_name: &RoleName, role_group_name: &RoleGroupName, ) -> Labels { recommended_labels( self, &product_name(), - &self.product_version, + product_version, &operator_name(), &controller_name(), - &Self::role_name(hbase_role), + role_name, role_group_name, ) } @@ -168,12 +190,7 @@ impl ValidatedCluster { hbase_role: &HbaseRole, role_group_name: &RoleGroupName, ) -> Labels { - role_group_selector( - self, - &product_name(), - &Self::role_name(hbase_role), - role_group_name, - ) + role_group_selector(self, &product_name(), &hbase_role.into(), role_group_name) } /// Returns an [`ObjectMetaBuilder`] pre-filled with the namespace, an owner reference back to @@ -295,3 +312,21 @@ pub type HbaseRoleGroupConfig = stackable_operator::v2::role_utils::RoleGroupCon stackable_operator::v2::role_utils::JavaCommonConfig, v1alpha1::HbaseConfigOverrides, >; + +#[cfg(test)] +mod tests { + use stackable_operator::v2::types::operator::RoleName; + use strum::IntoEnumIterator; + + use crate::crd::HbaseRole; + + /// Locks the invariant behind the `expect` in the `From for RoleName` impls: + /// every `HbaseRole` variant (present and future) must serialise to a valid `RoleName`. + #[test] + fn every_hbase_role_serialises_to_a_valid_role_name() { + for role in HbaseRole::iter() { + let _: RoleName = (&role).into(); + let _: RoleName = role.into(); + } + } +} diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index e372ea2e..b398c860 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -30,6 +30,7 @@ use stackable_operator::{ types::{ common::Port, kubernetes::{ConfigMapName, ListenerClassName, SecretClassName, VolumeName}, + operator::RoleName, }, }, versioned::versioned, @@ -262,6 +263,18 @@ pub enum HbaseRole { RestServer, } +impl From for RoleName { + fn from(value: HbaseRole) -> Self { + RoleName::from_str(&value.to_string()).expect("an HbaseRole name is a valid role name") + } +} + +impl From<&HbaseRole> for RoleName { + fn from(value: &HbaseRole) -> Self { + RoleName::from_str(&value.to_string()).expect("an HbaseRole name is a valid role name") + } +} + impl HbaseRole { const DEFAULT_MASTER_GRACEFUL_SHUTDOWN_TIMEOUT: Duration = Duration::from_minutes_unchecked(20); // Auto TLS certificate lifetime diff --git a/rust/operator-binary/src/hbase_controller.rs b/rust/operator-binary/src/hbase_controller.rs index 61d6ba46..f2861c39 100644 --- a/rust/operator-binary/src/hbase_controller.rs +++ b/rust/operator-binary/src/hbase_controller.rs @@ -11,13 +11,10 @@ use snafu::{ResultExt, Snafu}; use stackable_operator::{ cli::OperatorEnvironmentOptions, cluster_resources::ClusterResourceApplyStrategy, - commons::rbac::build_rbac_resources, kube::{ - ResourceExt, core::{DeserializeGuard, error_boundary}, runtime::controller::Action, }, - kvp::LabelError, logging::controller::ReconcilerError, shared::time::Duration, status::condition::{ @@ -30,7 +27,7 @@ use strum::{EnumDiscriminants, IntoStaticStr}; use crate::{ controller::{build, controller_name, operator_name, product_name}, - crd::{APP_NAME, HbaseClusterStatus, OPERATOR_NAME, v1alpha1}, + crd::{HbaseClusterStatus, OPERATOR_NAME, v1alpha1}, }; pub struct Ctx { @@ -46,24 +43,6 @@ pub enum Error { source: stackable_operator::cluster_resources::Error, }, - #[snafu(display("failed to build RBAC resources"))] - BuildRbacResources { - source: stackable_operator::commons::rbac::Error, - }, - - #[snafu(display("failed to build label"))] - BuildLabel { source: LabelError }, - - #[snafu(display("failed to patch service account"))] - ApplyServiceAccount { - source: stackable_operator::cluster_resources::Error, - }, - - #[snafu(display("failed to patch role binding"))] - ApplyRoleBinding { - source: stackable_operator::cluster_resources::Error, - }, - #[snafu(display("failed to build cluster resources"))] BuildResources { source: build::Error }, @@ -137,39 +116,26 @@ pub async fn reconcile_hbase( &hbase.spec.object_overrides, ); - let (rbac_sa, rbac_rolebinding) = build_rbac_resources( - hbase, - APP_NAME, - cluster_resources - .get_required_labels() - .context(BuildLabelSnafu)?, - ) - .context(BuildRbacResourcesSnafu)?; - cluster_resources - .add(client, rbac_sa.clone()) - .await - .context(ApplyServiceAccountSnafu)?; - cluster_resources - .add(client, rbac_rolebinding) - .await - .context(ApplyRoleBindingSnafu)?; - - // The ServiceAccount name is deterministic on the built object, so the build step does not - // depend on the applied ServiceAccount. - let service_account_name = rbac_sa.name_any(); - - let resources = build::build( - &validated_cluster, - &client.kubernetes_cluster_info, - &service_account_name, - ) - .context(BuildResourcesSnafu)?; + let resources = build::build(&validated_cluster, &client.kubernetes_cluster_info) + .context(BuildResourcesSnafu)?; let mut ss_cond_builder = StatefulSetConditionBuilder::default(); // Apply order: everything before the StatefulSets, StatefulSets last. A changed ConfigMap or // Secret a Pod mounts must exist before the Pod restarts, otherwise the Pod restarts again // unnecessarily. See https://github.com/stackabletech/commons-operator/issues/111 for details. + 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)