From 1c5b9cd001badb6b816b14da3f95f569c0b10561 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Mon, 20 Jul 2026 12:51:07 +0200 Subject: [PATCH 01/10] factor: add infallible rbac functions --- Cargo.lock | 11 +- Cargo.toml | 2 +- .../src/controller/build/mod.rs | 160 ++++++++++++------ .../controller/build/resource/discovery.rs | 46 ++--- .../src/controller/build/resource/mod.rs | 1 + .../src/controller/build/resource/rbac.rs | 42 +++++ .../src/controller/build/resource/service.rs | 74 ++------ .../controller/build/resource/statefulset.rs | 14 +- rust/operator-binary/src/controller/mod.rs | 104 +++++++++++- rust/operator-binary/src/hdfs_controller.rs | 80 +++------ rust/operator-binary/src/main.rs | 22 +-- 11 files changed, 327 insertions(+), 229 deletions(-) create mode 100644 rust/operator-binary/src/controller/build/resource/rbac.rs diff --git a/Cargo.lock b/Cargo.lock index 901a9590..c9aeca22 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1607,7 +1607,6 @@ dependencies = [ [[package]] name = "k8s-version" version = "0.1.3" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.113.3#013bbf43f7006a4ddfc08a147f68441ed88b462b" dependencies = [ "darling", "regex", @@ -3015,7 +3014,6 @@ checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" [[package]] name = "stackable-certs" version = "0.4.1" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.113.3#013bbf43f7006a4ddfc08a147f68441ed88b462b" dependencies = [ "const-oid", "ecdsa", @@ -3060,8 +3058,7 @@ dependencies = [ [[package]] name = "stackable-operator" -version = "0.113.3" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.113.3#013bbf43f7006a4ddfc08a147f68441ed88b462b" +version = "0.113.4" dependencies = [ "base64", "clap", @@ -3106,7 +3103,6 @@ dependencies = [ [[package]] name = "stackable-operator-derive" version = "0.3.1" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.113.3#013bbf43f7006a4ddfc08a147f68441ed88b462b" dependencies = [ "darling", "proc-macro2", @@ -3117,7 +3113,6 @@ dependencies = [ [[package]] name = "stackable-shared" version = "0.1.2" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.113.3#013bbf43f7006a4ddfc08a147f68441ed88b462b" dependencies = [ "jiff", "k8s-openapi", @@ -3134,7 +3129,6 @@ dependencies = [ [[package]] name = "stackable-telemetry" version = "0.6.5" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.113.3#013bbf43f7006a4ddfc08a147f68441ed88b462b" dependencies = [ "axum", "clap", @@ -3158,7 +3152,6 @@ dependencies = [ [[package]] name = "stackable-versioned" version = "0.11.1" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.113.3#013bbf43f7006a4ddfc08a147f68441ed88b462b" dependencies = [ "kube", "schemars", @@ -3172,7 +3165,6 @@ dependencies = [ [[package]] name = "stackable-versioned-macros" version = "0.11.1" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.113.3#013bbf43f7006a4ddfc08a147f68441ed88b462b" dependencies = [ "convert_case", "convert_case_extras", @@ -3190,7 +3182,6 @@ dependencies = [ [[package]] name = "stackable-webhook" version = "0.9.2" -source = "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.113.3#013bbf43f7006a4ddfc08a147f68441ed88b462b" dependencies = [ "arc-swap", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index 56b2161f..a14ccc4d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,4 +31,4 @@ tracing-futures = { version = "0.2", features = ["futures-03"] } [patch."https://github.com/stackabletech/operator-rs.git"] #stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "main" } -#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/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index 9b18e795..a62f2d0c 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -1,19 +1,18 @@ -use std::{collections::HashMap, str::FromStr}; +use std::collections::HashMap; use snafu::{ResultExt, Snafu}; use stackable_operator::{ builder::meta::ObjectMetaBuilder, kvp::{LabelError, Labels}, utils::cluster_info::KubernetesClusterInfo, - v2::{ - builder::meta::ownerreference_from_resource, - types::{common::Port, kubernetes::ServiceName, operator::RoleGroupName}, - }, + v2::types::{common::Port, operator::RoleGroupName}, }; use crate::{ - build_recommended_labels, - controller::{KubernetesResources, ValidatedCluster}, + controller::{ + KubernetesResources, ValidatedCluster, + build::resource::rbac::{build_role_binding, build_service_account}, + }, crd::{ HdfsNodeRole, HdfsPodRef, constants::{ @@ -32,7 +31,6 @@ use crate::{ SERVICE_PORT_NAME_RPC, }, }, - hdfs_controller::RESOURCE_MANAGER_HDFS_CONTROLLER, }; pub mod container; @@ -74,9 +72,6 @@ pub enum Error { /// `cluster_info` carries static cluster information resolved at operator startup (e.g. the /// cluster domain used to build Kerberos principals), not a live client. /// -/// `service_account_name` is the name of the RBAC `ServiceAccount` the role-group Pods run under. -/// The RBAC resources are built and applied separately in the reconcile step. -/// /// The resources are returned as flat, unordered collections. The reconcile step re-groups the /// StatefulSets by role to preserve HDFS's ordered, rollout-gated deployment during upgrades. The /// discovery `ConfigMap` is deliberately not built here: it needs a live client to resolve @@ -84,7 +79,6 @@ pub enum Error { pub fn build( cluster: &ValidatedCluster, cluster_info: &KubernetesClusterInfo, - service_account_name: &str, ) -> Result { let mut services = vec![]; let mut config_maps = vec![]; @@ -126,7 +120,6 @@ pub fn build( role, role_group_name, rg_config, - service_account_name, ) .context(StatefulSetSnafu { role: *role, @@ -145,6 +138,8 @@ pub fn build( config_maps, pod_disruption_budgets, stateful_sets, + service_accounts: vec![build_service_account(cluster)], + role_bindings: vec![build_role_binding(cluster)], }) } @@ -168,15 +163,7 @@ pub(crate) fn pod_refs(cluster: &ValidatedCluster, role: &HdfsNodeRole) -> Vec ObjectMetaBuilder { - let role_name = role.to_string(); - let mut metadata = ObjectMetaBuilder::new(); - metadata - .name_and_namespace(cluster) - .name( - cluster - .resource_names(role, role_group_name) - .qualified_role_group_name(), - ) - .ownerreference(ownerreference_from_resource(cluster, None, Some(true))) - .with_recommended_labels(&build_recommended_labels( - cluster, - RESOURCE_MANAGER_HDFS_CONTROLLER, - &cluster.image.app_version_label_value, - &role_name, - role_group_name.as_ref(), - )) - .expect( - "the recommended labels are valid because the ValidatedCluster uses \ - fail-safe typed values", - ); - metadata + cluster.object_meta( + cluster + .resource_names(role, role_group_name) + .qualified_role_group_name() + .to_string(), + cluster.recommended_labels(role, role_group_name), + ) } /// The rolegroup selector labels (also used as `Service`/`StatefulSet` selectors) for @@ -373,10 +345,15 @@ fn role_data_ports(role: &HdfsNodeRole, https_enabled: bool) -> Vec<(String, Por #[cfg(test)] mod tests { + use std::collections::BTreeMap; + use stackable_operator::kube::Resource; use super::build; - use crate::controller::build::properties::test_support::{cluster_info, validated_cluster}; + use crate::{ + controller::build::properties::test_support::{self, cluster_info, validated_cluster}, + test_support::deserialize_and_validate_cluster, + }; /// The sorted `metadata.name`s of a resource collection. fn sorted_names(resources: &[impl Resource]) -> Vec { @@ -394,8 +371,7 @@ mod tests { #[test] fn build_produces_expected_resource_names() { let cluster = validated_cluster(); - let resources = - build(&cluster, &cluster_info(), "hdfs-serviceaccount").expect("build succeeds"); + let resources = build(&cluster, &cluster_info()).expect("build succeeds"); assert_eq!( sorted_names(&resources.stateful_sets), @@ -405,8 +381,19 @@ mod tests { "hdfs-namenode-default", ] ); - // One headless and one metrics Service per role group. - assert_eq!(resources.services.len(), 6); + // One headless (un-suffixed, see `ValidatedCluster::governing_service_name`) and one + // metrics Service per role group. + assert_eq!( + sorted_names(&resources.services), + [ + "hdfs-datanode-default", + "hdfs-datanode-default-metrics", + "hdfs-journalnode-default", + "hdfs-journalnode-default-metrics", + "hdfs-namenode-default", + "hdfs-namenode-default-metrics", + ] + ); assert_eq!( sorted_names(&resources.config_maps), [ @@ -421,4 +408,79 @@ mod tests { ["hdfs-datanode", "hdfs-journalnode", "hdfs-namenode"] ); } + + /// Every StatefulSet's (immutable) `serviceName` must reference a headless Service that the + /// build step actually produces — the pods' DNS names depend on the pair agreeing. Guards the + /// coupling that `ValidatedCluster::governing_service_name` centralises. + #[test] + fn statefulset_service_name_references_built_service() { + let cluster = validated_cluster(); + let resources = build(&cluster, &cluster_info()).expect("build succeeds"); + + let service_names = sorted_names(&resources.services); + for stateful_set in &resources.stateful_sets { + let service_name = stateful_set + .spec + .as_ref() + .and_then(|spec| spec.service_name.as_deref()) + .expect("every StatefulSet sets serviceName"); + assert!( + service_names.iter().any(|name| name == service_name), + "StatefulSet references headless Service {service_name:?}, which is not built \ + (built Services: {service_names:?})" + ); + } + } + + /// 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 + /// `hdfs`, which would mask exactly that swap). + #[test] + fn build_produces_rbac() { + let cluster = deserialize_and_validate_cluster( + &test_support::MINIMAL_HDFS_YAML.replace("name: hdfs", "name: my-hdfs"), + ); + let resources = build(&cluster, &cluster_info()).expect("build succeeds"); + + assert_eq!( + sorted_names(&resources.service_accounts), + ["my-hdfs-serviceaccount"] + ); + assert_eq!( + sorted_names(&resources.role_bindings), + ["my-hdfs-rolebinding"] + ); + + let expected_labels = BTreeMap::from( + [ + ("app.kubernetes.io/component", "none"), + ("app.kubernetes.io/instance", "my-hdfs"), + ( + "app.kubernetes.io/managed-by", + "hdfs.stackable.tech_hdfs-operator-hdfs-controller", + ), + ("app.kubernetes.io/name", "hdfs"), + ("app.kubernetes.io/role-group", "none"), + ("app.kubernetes.io/version", "3.4.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, "hdfs-clusterrole"); + } } diff --git a/rust/operator-binary/src/controller/build/resource/discovery.rs b/rust/operator-binary/src/controller/build/resource/discovery.rs index 5db023af..253aa447 100644 --- a/rust/operator-binary/src/controller/build/resource/discovery.rs +++ b/rust/operator-binary/src/controller/build/resource/discovery.rs @@ -1,15 +1,19 @@ //! Build the discovery `ConfigMap` for the HdfsCluster. +use std::str::FromStr; + use snafu::{ResultExt, Snafu}; use stackable_operator::{ - builder::{configmap::ConfigMapBuilder, meta::ObjectMetaBuilder}, + builder::configmap::ConfigMapBuilder, k8s_openapi::api::core::v1::ConfigMap, utils::cluster_info::KubernetesClusterInfo, - v2::builder::meta::ownerreference_from_resource, + v2::{ + kvp::label::recommended_labels, + types::operator::{ControllerName, RoleGroupName}, + }, }; use crate::{ - build_recommended_labels, controller::{ ValidatedCluster, build::{ @@ -18,6 +22,7 @@ use crate::{ ConfigFileName, core_site::CoreSiteConfigBuilder, hdfs_site::HdfsSiteConfigBuilder, }, }, + operator_name, product_name, }, crd::{HdfsNodeRole, HdfsPodRef}, hdfs_controller::HDFS_CONTROLLER_NAME, @@ -25,6 +30,8 @@ use crate::{ type Result = std::result::Result; +stackable_operator::constant!(DISCOVERY_ROLE_GROUP: RoleGroupName = "discovery"); + #[derive(Snafu, Debug)] #[allow(clippy::enum_variant_names)] pub enum Error { @@ -32,11 +39,6 @@ pub enum Error { BuildConfigMap { source: stackable_operator::builder::configmap::Error, }, - - #[snafu(display("failed to build object meta data"))] - ObjectMeta { - source: stackable_operator::builder::meta::Error, - }, } /// Creates a discovery config map containing the `hdfs-site.xml` and `core-site.xml` @@ -50,18 +52,22 @@ pub fn build_discovery_config_map( cluster_info: &KubernetesClusterInfo, namenode_podrefs: &[HdfsPodRef], ) -> Result { - let metadata = ObjectMetaBuilder::new() - .name_and_namespace(cluster) - .ownerreference(ownerreference_from_resource(cluster, None, Some(true))) - .with_recommended_labels(&build_recommended_labels( - cluster, - HDFS_CONTROLLER_NAME, - &cluster.image.app_version_label_value, - &HdfsNodeRole::Name.to_string(), - "discovery", - )) - .context(ObjectMetaSnafu)? - .build(); + // The discovery ConfigMap deliberately deviates from the standard resource identity: it is + // labelled with the `hdfs-controller` controller name (NOT `controller_name()`, i.e. + // `hdfs-operator-hdfs-controller` like the role-group resources), which keeps it outside their + // `ClusterResources` orphan-matching, and with the namenode role plus a `discovery` role-group. + let labels = recommended_labels( + cluster, + &product_name(), + &cluster.product_version, + &operator_name(), + &ControllerName::from_str(HDFS_CONTROLLER_NAME) + .expect("the hdfs controller name is a valid label value"), + &ValidatedCluster::role_name(&HdfsNodeRole::Name), + &DISCOVERY_ROLE_GROUP, + ); + + let metadata = cluster.object_meta(cluster.name.clone(), labels).build(); ConfigMapBuilder::new() .metadata(metadata) diff --git a/rust/operator-binary/src/controller/build/resource/mod.rs b/rust/operator-binary/src/controller/build/resource/mod.rs index 9c3b1b5a..9788477b 100644 --- a/rust/operator-binary/src/controller/build/resource/mod.rs +++ b/rust/operator-binary/src/controller/build/resource/mod.rs @@ -1,5 +1,6 @@ pub mod config_map; pub mod discovery; 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 new file mode 100644 index 00000000..8d52854b --- /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.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/service.rs b/rust/operator-binary/src/controller/build/resource/service.rs index 64662cc8..e5313e6f 100644 --- a/rust/operator-binary/src/controller/build/resource/service.rs +++ b/rust/operator-binary/src/controller/build/resource/service.rs @@ -3,23 +3,14 @@ //! Prometheus scrape labels/annotations. use snafu::{ResultExt, Snafu}; use stackable_operator::{ - builder::meta::ObjectMetaBuilder, k8s_openapi::api::core::v1::{Service, ServicePort, ServiceSpec}, kvp::LabelError, - v2::{ - builder::{ - meta::ownerreference_from_resource, - service::{Scheme, Scraping, prometheus_annotations, prometheus_labels}, - }, - types::operator::RoleGroupName, - }, + v2::types::operator::RoleGroupName, }; use crate::{ - build_recommended_labels, controller::{ValidatedCluster, build}, crd::HdfsNodeRole, - hdfs_controller::RESOURCE_MANAGER_HDFS_CONTROLLER, }; #[derive(Snafu, Debug)] @@ -42,26 +33,6 @@ pub(crate) fn rolegroup_headless_service( ) -> Result { tracing::info!("Setting up headless Service for role {role} role group {role_group_name}"); - let resource_names = cluster.resource_names(role, role_group_name); - let role_name = role.to_string(); - // TODO: The v2 `ResourceNames::headless_service_name()` would add a `-headless` suffix, but - // we deliberately keep the un-suffixed name here so the StatefulSet's (immutable) `serviceName` - // and the pod DNS names stay unchanged for existing clusters. A decision is needed on whether - // to adopt the suffixed name (requires StatefulSet recreation on upgrade). - let mut metadata_builder = ObjectMetaBuilder::new(); - metadata_builder - .name_and_namespace(cluster) - .name(resource_names.qualified_role_group_name()) - .ownerreference(ownerreference_from_resource(cluster, None, Some(true))) - .with_recommended_labels(&build_recommended_labels( - cluster, - RESOURCE_MANAGER_HDFS_CONTROLLER, - &cluster.image.app_version_label_value, - &role_name, - role_group_name.as_ref(), - )) - .context(ObjectMetaSnafu)?; - let service_spec = ServiceSpec { // Internal communication does not need to be exposed type_: Some("ClusterIP".to_string()), @@ -87,7 +58,14 @@ pub(crate) fn rolegroup_headless_service( }; Ok(Service { - metadata: metadata_builder.build(), + metadata: cluster + .object_meta( + cluster + .governing_service_name(role, role_group_name) + .to_string(), + cluster.recommended_labels(role, role_group_name), + ) + .build(), spec: Some(service_spec), status: None, }) @@ -100,9 +78,6 @@ pub(crate) fn rolegroup_metrics_service( ) -> Result { tracing::info!("Setting up metrics Service for role {role} role group {role_group_name}"); - let resource_names = cluster.resource_names(role, role_group_name); - let role_name = role.to_string(); - let service_spec = ServiceSpec { // Internal communication does not need to be exposed type_: Some("ClusterIP".to_string()), @@ -128,29 +103,14 @@ pub(crate) fn rolegroup_metrics_service( }; Ok(Service { - metadata: ObjectMetaBuilder::new() - .name_and_namespace(cluster) - .name(resource_names.metrics_service_name()) - .ownerreference(ownerreference_from_resource(cluster, None, Some(true))) - .with_recommended_labels(&build_recommended_labels( - cluster, - RESOURCE_MANAGER_HDFS_CONTROLLER, - &cluster.image.app_version_label_value, - &role_name, - role_group_name.as_ref(), - )) - .context(ObjectMetaSnafu)? - .with_labels(prometheus_labels(&Scraping::Enabled)) - .with_annotations(prometheus_annotations( - &Scraping::Enabled, - if cluster.has_https_enabled() { - &Scheme::Https - } else { - &Scheme::Http - }, - "/prom", - &build::native_metrics_port(cluster, role), - )) + metadata: cluster + .object_meta( + cluster + .resource_names(role, role_group_name) + .metrics_service_name() + .to_string(), + cluster.recommended_labels(role, role_group_name), + ) .build(), spec: Some(service_spec), status: None, diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs index 6b7b32f2..a13592e7 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -47,7 +47,6 @@ pub(crate) fn build_rolegroup_statefulset( role: &HdfsNodeRole, role_group_name: &RoleGroupName, rolegroup_config: &ValidatedRoleGroupConfig, - service_account_name: &str, ) -> Result { tracing::info!("Setting up StatefulSet for role {role} role group {role_group_name}"); @@ -69,7 +68,12 @@ pub(crate) fn build_rolegroup_statefulset( pb.metadata(pb_metadata) .image_pull_secrets_from_product_image(image) .affinity(&merged_config.affinity) - .service_account_name(service_account_name) + .service_account_name( + validated + .rbac_resource_names() + .service_account_name() + .to_string(), + ) .security_context(PodSecurityContextBuilder::new().fs_group(1000).build()); // Adds all containers and volumes to the pod builder @@ -107,13 +111,9 @@ pub(crate) fn build_rolegroup_statefulset( match_labels: Some(rolegroup_selector_labels.into()), ..LabelSelector::default() }, - // Must match the headless Service name so the StatefulSet's pods get stable DNS names. - // See the TODO in `build::resource::service::rolegroup_headless_service` about the - // un-suffixed name. service_name: Some( validated - .resource_names(role, role_group_name) - .qualified_role_group_name() + .governing_service_name(role, role_group_name) .to_string(), ), template: pod_template, diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index 800d6157..5e4301d4 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -1,21 +1,27 @@ use std::{collections::BTreeMap, str::FromStr}; use stackable_operator::{ + builder::meta::ObjectMetaBuilder, commons::product_image_selection::ResolvedProductImage, 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, v2::{ HasName, HasUid, NameIsValidLabelValue, + builder::meta::ownerreference_from_resource, + kvp::label::recommended_labels, role_group_utils::ResourceNames, - role_utils::RoleGroupConfig, + role_utils::{self, RoleGroupConfig}, types::{ - kubernetes::{ConfigMapName, NamespaceName, Uid}, + kubernetes::{ConfigMapName, NamespaceName, ServiceName, Uid}, operator::{ - ClusterName, ControllerName, OperatorName, ProductName, RoleGroupName, RoleName, + ClusterName, ControllerName, OperatorName, ProductName, ProductVersion, + RoleGroupName, RoleName, }, }, }, @@ -47,6 +53,8 @@ pub struct KubernetesResources { pub config_maps: Vec, pub pod_disruption_budgets: Vec, pub stateful_sets: Vec, + pub service_accounts: Vec, + pub role_bindings: Vec, } /// The [`RoleGroupConfig`] specialised for HDFS: the validated config is the @@ -73,6 +81,7 @@ pub struct ValidatedCluster { pub namespace: NamespaceName, /// The cluster's Kubernetes UID, used to build owner references. pub uid: Uid, + pub product_version: ProductVersion, pub image: ResolvedProductImage, pub cluster_config: ValidatedClusterConfig, pub role_groups: BTreeMap>, @@ -94,6 +103,10 @@ impl ValidatedCluster { role_configs: BTreeMap, status: ValidatedClusterStatus, ) -> Self { + // `app_version_label_value` is constructed to be a valid label value, so it is also a valid + // `ProductVersion`. + let product_version = ProductVersion::from_str(&image.app_version_label_value) + .expect("the app version label value is a valid product version"); Self { metadata: ObjectMeta { name: Some(name.to_string()), @@ -107,6 +120,7 @@ impl ValidatedCluster { namespace, uid, image, + product_version, cluster_config, role_groups, role_configs, @@ -139,6 +153,15 @@ impl ValidatedCluster { RoleName::from_str(&role.to_string()).expect("a HdfsNodeRole 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 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 the given role group. pub(crate) fn resource_names( &self, @@ -151,6 +174,76 @@ impl ValidatedCluster { role_group_name: role_group_name.clone(), } } + + /// Recommended labels for a resource that is not tied to a concrete [`HdfsNodeRole`], using a free-form role/role-group label value. + pub fn recommended_labels_for( + &self, + 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(), + product_version, + &operator_name(), + &controller_name(), + role_name, + role_group_name, + ) + } + + /// Recommended labels for a role-group resource. + pub fn recommended_labels( + &self, + role: &HdfsNodeRole, + role_group_name: &RoleGroupName, + ) -> Labels { + self.recommended_labels_for(&Self::role_name(role), role_group_name) + } + + /// Returns an [`ObjectMetaBuilder`] pre-filled with the namespace, the resource `name`, an owner + /// reference back to this cluster, and the given recommended `labels`. + pub(crate) fn object_meta(&self, name: impl Into, labels: Labels) -> ObjectMetaBuilder { + let mut builder = ObjectMetaBuilder::new(); + builder + .name_and_namespace(self) + .name(name) + .ownerreference(ownerreference_from_resource(self, None, Some(true))) + .with_labels(labels); + builder + } + + /// The name of a role group's governing headless Service. + /// + /// Used as the headless Service's own name, as the StatefulSet's (immutable) `serviceName`, + /// and for the pod DNS references derived from them; the three reference each other, so they + /// must be derived here and nowhere else. + // + // TODO: The v2 `ResourceNames::headless_service_name()` would add a `-headless` suffix, but we + // deliberately keep the un-suffixed name so the StatefulSet's (immutable) `serviceName` and the + // pod DNS names stay unchanged for existing clusters. A decision is needed on whether to adopt + // the suffixed name (requires StatefulSet recreation on upgrade). + pub(crate) fn governing_service_name( + &self, + role: &HdfsNodeRole, + role_group_name: &RoleGroupName, + ) -> ServiceName { + ServiceName::from_str( + self.resource_names(role, role_group_name) + .qualified_role_group_name() + .as_ref(), + ) + .expect("a qualified role group name is a valid Service name") + } } /// The product name (`hdfs`) as a type-safe label value. @@ -169,8 +262,7 @@ pub(crate) fn controller_name() -> ControllerName { .expect("the controller name is a valid label value") } -/// Lets [`ValidatedCluster`] be used as the owner [`Resource`] (e.g. in -/// [`ObjectMetaBuilder::ownerreference_from_resource`]). The kind/group/version/plural +/// Lets [`ValidatedCluster`] be used as the owner [`Resource`]. The kind/group/version/plural /// are delegated to [`v1alpha1::HdfsCluster`] so the generated owner references are /// identical to the ones built from the raw cluster object. impl Resource for ValidatedCluster { diff --git a/rust/operator-binary/src/hdfs_controller.rs b/rust/operator-binary/src/hdfs_controller.rs index 2df9ac2e..5b94a220 100644 --- a/rust/operator-binary/src/hdfs_controller.rs +++ b/rust/operator-binary/src/hdfs_controller.rs @@ -5,7 +5,6 @@ use stackable_operator::{ cli::OperatorEnvironmentOptions, client::Client, cluster_resources::ClusterResourceApplyStrategy, - commons::rbac::build_rbac_resources, iter::reverse_if, kube::{ Resource, ResourceExt, @@ -45,6 +44,11 @@ pub const HDFS_CONTROLLER_NAME: &str = "hdfs-controller"; #[derive(Snafu, Debug, EnumDiscriminants)] #[strum_discriminants(derive(IntoStaticStr))] pub enum Error { + #[snafu(display("failed to apply Kubernetes resource"))] + ApplyResource { + source: stackable_operator::cluster_resources::Error, + }, + #[snafu(display("failed to dereference cluster resources"))] Dereference { source: crate::controller::dereference::Error, @@ -55,18 +59,6 @@ pub enum Error { source: crate::controller::validate::Error, }, - #[snafu(display("cannot create rolegroup service {name:?}"))] - ApplyRoleGroupService { - source: stackable_operator::cluster_resources::Error, - name: String, - }, - - #[snafu(display("cannot create role group config map {name:?}"))] - ApplyRoleGroupConfigMap { - source: stackable_operator::cluster_resources::Error, - name: String, - }, - #[snafu(display("cannot create role group stateful set {name:?}"))] ApplyRoleGroupStatefulSet { source: stackable_operator::cluster_resources::Error, @@ -90,16 +82,6 @@ pub enum Error { #[snafu(display("cannot build config discovery config map"))] BuildDiscoveryConfigMap { source: discovery::Error }, - #[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 delete orphaned resources"))] DeleteOrphanedResources { source: stackable_operator::cluster_resources::Error, @@ -118,11 +100,6 @@ pub enum Error { source: stackable_operator::client::Error, }, - #[snafu(display("failed to build RBAC resources"))] - BuildRbacResources { - source: stackable_operator::commons::rbac::Error, - }, - #[snafu(display("failed to build cluster resources label"))] BuildClusterResourcesLabel { source: LabelError }, @@ -181,57 +158,44 @@ pub async fn reconcile_hdfs( &hdfs.spec.object_overrides, ); - // The service account and rolebinding will be created per cluster - let (rbac_sa, rbac_rolebinding) = build_rbac_resources( - hdfs, - APP_NAME, - cluster_resources - .get_required_labels() - .context(BuildClusterResourcesLabelSnafu)?, - ) - .context(BuildRbacResourcesSnafu)?; - - cluster_resources - .add(client, rbac_sa.clone()) - .await - .context(ApplyServiceAccountSnafu)?; - cluster_resources - .add(client, rbac_rolebinding) - .await - .context(ApplyRoleBindingSnafu)?; - // Build every (non-discovery) Kubernetes resource up front. This step needs no client: all // external references are already dereferenced and validated. The ServiceAccount name is // deterministic on the built RBAC object, so the build does not depend on the applied one. - let resources = build::build( - &validated_cluster, - &client.kubernetes_cluster_info, - &rbac_sa.name_any(), - ) - .context(BuildResourcesSnafu)?; + let resources = build::build(&validated_cluster, &client.kubernetes_cluster_info) + .context(BuildResourcesSnafu)?; // Apply Services, ConfigMaps and PodDisruptionBudgets first. The StatefulSets are applied // afterwards so that every ConfigMap a Pod mounts already exists, which prevents unnecessary // Pod restarts. 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 { - let name = service.name_any(); cluster_resources .add(client, service) .await - .with_context(|_| ApplyRoleGroupServiceSnafu { name })?; + .context(ApplyResourceSnafu)?; } for config_map in resources.config_maps { - let name = config_map.name_any(); cluster_resources .add(client, config_map) .await - .with_context(|_| ApplyRoleGroupConfigMapSnafu { name })?; + .context(ApplyResourceSnafu)?; } for pdb in resources.pod_disruption_budgets { cluster_resources .add(client, pdb) .await - .context(ApplyPdbSnafu)?; + .context(ApplyResourceSnafu)?; } let upgrade_state = validated_cluster.status.upgrade_state; diff --git a/rust/operator-binary/src/main.rs b/rust/operator-binary/src/main.rs index 55206a05..00666519 100644 --- a/rust/operator-binary/src/main.rs +++ b/rust/operator-binary/src/main.rs @@ -28,7 +28,6 @@ use stackable_operator::{ reflector, watcher, }, }, - kvp::ObjectLabels, logging::controller::report_controller_reconciled, shared::yaml::SerializeOptions, telemetry::Tracing, @@ -38,7 +37,7 @@ use tracing::info_span; use tracing_futures::Instrument; use crate::{ - crd::{HdfsCluster, HdfsClusterVersion, constants::APP_NAME, v1alpha1}, + crd::{HdfsCluster, HdfsClusterVersion, v1alpha1}, webhooks::conversion::create_webhook_server, }; @@ -226,25 +225,6 @@ async fn main() -> anyhow::Result<()> { Ok(()) } -/// Creates recommended `ObjectLabels` to be used in deployed resources -pub fn build_recommended_labels<'a, T>( - owner: &'a T, - controller_name: &'a str, - app_version: &'a str, - role: &'a str, - role_group: &'a str, -) -> ObjectLabels<'a, T> { - ObjectLabels { - owner, - app_name: APP_NAME, - app_version, - operator_name: OPERATOR_NAME, - controller_name, - role, - role_group, - } -} - fn references_config_map( hdfs: &DeserializeGuard, config_map: &DeserializeGuard, From 827580e2de85d532cf6f00511fb7e7b5b9f5504d Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Wed, 22 Jul 2026 10:52:55 +0200 Subject: [PATCH 02/10] regenerate nix --- Cargo.nix | 65 ++++++++--------------------------------------- crate-hashes.json | 9 ------- 2 files changed, 10 insertions(+), 64 deletions(-) diff --git a/Cargo.nix b/Cargo.nix index 98af8089..232bc703 100644 --- a/Cargo.nix +++ b/Cargo.nix @@ -5105,12 +5105,7 @@ rec { crateName = "k8s-version"; version = "0.1.3"; edition = "2024"; - workspace_member = null; - src = pkgs.fetchgit { - url = "https://github.com/stackabletech/operator-rs.git"; - rev = "013bbf43f7006a4ddfc08a147f68441ed88b462b"; - sha256 = "054p2mcinq3x4iykmc72nhga7kapc8ihkx9mnzqq3qdm2bk2qcps"; - }; + src = lib.cleanSourceWith { filter = sourceFilter; src = ../operator-rs/crates/k8s-version; }; libName = "k8s_version"; authors = [ "Stackable GmbH " @@ -9889,12 +9884,7 @@ rec { crateName = "stackable-certs"; version = "0.4.1"; edition = "2024"; - workspace_member = null; - src = pkgs.fetchgit { - url = "https://github.com/stackabletech/operator-rs.git"; - rev = "013bbf43f7006a4ddfc08a147f68441ed88b462b"; - sha256 = "054p2mcinq3x4iykmc72nhga7kapc8ihkx9mnzqq3qdm2bk2qcps"; - }; + src = lib.cleanSourceWith { filter = sourceFilter; src = ../operator-rs/crates/stackable-certs; }; libName = "stackable_certs"; authors = [ "Stackable GmbH " @@ -10084,14 +10074,9 @@ rec { }; "stackable-operator" = rec { crateName = "stackable-operator"; - version = "0.113.3"; + version = "0.113.4"; edition = "2024"; - workspace_member = null; - src = pkgs.fetchgit { - url = "https://github.com/stackabletech/operator-rs.git"; - rev = "013bbf43f7006a4ddfc08a147f68441ed88b462b"; - sha256 = "054p2mcinq3x4iykmc72nhga7kapc8ihkx9mnzqq3qdm2bk2qcps"; - }; + src = lib.cleanSourceWith { filter = sourceFilter; src = ../operator-rs/crates/stackable-operator; }; libName = "stackable_operator"; authors = [ "Stackable GmbH " @@ -10285,12 +10270,7 @@ rec { crateName = "stackable-operator-derive"; version = "0.3.1"; edition = "2024"; - workspace_member = null; - src = pkgs.fetchgit { - url = "https://github.com/stackabletech/operator-rs.git"; - rev = "013bbf43f7006a4ddfc08a147f68441ed88b462b"; - sha256 = "054p2mcinq3x4iykmc72nhga7kapc8ihkx9mnzqq3qdm2bk2qcps"; - }; + src = lib.cleanSourceWith { filter = sourceFilter; src = ../operator-rs/crates/stackable-operator-derive; }; procMacro = true; libName = "stackable_operator_derive"; authors = [ @@ -10320,12 +10300,7 @@ rec { crateName = "stackable-shared"; version = "0.1.2"; edition = "2024"; - workspace_member = null; - src = pkgs.fetchgit { - url = "https://github.com/stackabletech/operator-rs.git"; - rev = "013bbf43f7006a4ddfc08a147f68441ed88b462b"; - sha256 = "054p2mcinq3x4iykmc72nhga7kapc8ihkx9mnzqq3qdm2bk2qcps"; - }; + src = lib.cleanSourceWith { filter = sourceFilter; src = ../operator-rs/crates/stackable-shared; }; libName = "stackable_shared"; authors = [ "Stackable GmbH " @@ -10401,12 +10376,7 @@ rec { crateName = "stackable-telemetry"; version = "0.6.5"; edition = "2024"; - workspace_member = null; - src = pkgs.fetchgit { - url = "https://github.com/stackabletech/operator-rs.git"; - rev = "013bbf43f7006a4ddfc08a147f68441ed88b462b"; - sha256 = "054p2mcinq3x4iykmc72nhga7kapc8ihkx9mnzqq3qdm2bk2qcps"; - }; + src = lib.cleanSourceWith { filter = sourceFilter; src = ../operator-rs/crates/stackable-telemetry; }; libName = "stackable_telemetry"; authors = [ "Stackable GmbH " @@ -10511,12 +10481,7 @@ rec { crateName = "stackable-versioned"; version = "0.11.1"; edition = "2024"; - workspace_member = null; - src = pkgs.fetchgit { - url = "https://github.com/stackabletech/operator-rs.git"; - rev = "013bbf43f7006a4ddfc08a147f68441ed88b462b"; - sha256 = "054p2mcinq3x4iykmc72nhga7kapc8ihkx9mnzqq3qdm2bk2qcps"; - }; + src = lib.cleanSourceWith { filter = sourceFilter; src = ../operator-rs/crates/stackable-versioned; }; libName = "stackable_versioned"; authors = [ "Stackable GmbH " @@ -10561,12 +10526,7 @@ rec { crateName = "stackable-versioned-macros"; version = "0.11.1"; edition = "2024"; - workspace_member = null; - src = pkgs.fetchgit { - url = "https://github.com/stackabletech/operator-rs.git"; - rev = "013bbf43f7006a4ddfc08a147f68441ed88b462b"; - sha256 = "054p2mcinq3x4iykmc72nhga7kapc8ihkx9mnzqq3qdm2bk2qcps"; - }; + src = lib.cleanSourceWith { filter = sourceFilter; src = ../operator-rs/crates/stackable-versioned-macros; }; procMacro = true; libName = "stackable_versioned_macros"; authors = [ @@ -10629,12 +10589,7 @@ rec { crateName = "stackable-webhook"; version = "0.9.2"; edition = "2024"; - workspace_member = null; - src = pkgs.fetchgit { - url = "https://github.com/stackabletech/operator-rs.git"; - rev = "013bbf43f7006a4ddfc08a147f68441ed88b462b"; - sha256 = "054p2mcinq3x4iykmc72nhga7kapc8ihkx9mnzqq3qdm2bk2qcps"; - }; + src = lib.cleanSourceWith { filter = sourceFilter; src = ../operator-rs/crates/stackable-webhook; }; libName = "stackable_webhook"; authors = [ "Stackable GmbH " diff --git a/crate-hashes.json b/crate-hashes.json index cd3917c3..ceeb77ce 100644 --- a/crate-hashes.json +++ b/crate-hashes.json @@ -1,12 +1,3 @@ { - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.113.3#k8s-version@0.1.3": "054p2mcinq3x4iykmc72nhga7kapc8ihkx9mnzqq3qdm2bk2qcps", - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.113.3#stackable-certs@0.4.1": "054p2mcinq3x4iykmc72nhga7kapc8ihkx9mnzqq3qdm2bk2qcps", - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.113.3#stackable-operator-derive@0.3.1": "054p2mcinq3x4iykmc72nhga7kapc8ihkx9mnzqq3qdm2bk2qcps", - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.113.3#stackable-operator@0.113.3": "054p2mcinq3x4iykmc72nhga7kapc8ihkx9mnzqq3qdm2bk2qcps", - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.113.3#stackable-shared@0.1.2": "054p2mcinq3x4iykmc72nhga7kapc8ihkx9mnzqq3qdm2bk2qcps", - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.113.3#stackable-telemetry@0.6.5": "054p2mcinq3x4iykmc72nhga7kapc8ihkx9mnzqq3qdm2bk2qcps", - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.113.3#stackable-versioned-macros@0.11.1": "054p2mcinq3x4iykmc72nhga7kapc8ihkx9mnzqq3qdm2bk2qcps", - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.113.3#stackable-versioned@0.11.1": "054p2mcinq3x4iykmc72nhga7kapc8ihkx9mnzqq3qdm2bk2qcps", - "git+https://github.com/stackabletech/operator-rs.git?tag=stackable-operator-0.113.3#stackable-webhook@0.9.2": "054p2mcinq3x4iykmc72nhga7kapc8ihkx9mnzqq3qdm2bk2qcps", "git+https://github.com/stackabletech/product-config.git?tag=0.8.0#product-config@0.8.0": "1dz70kapm2wdqcr7ndyjji0lhsl98bsq95gnb2lw487wf6yr7987" } \ No newline at end of file From 26f2390fdcb2482e62b081a870c8b5d0eef4ae41 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Wed, 22 Jul 2026 10:53:28 +0200 Subject: [PATCH 03/10] changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d84fb15..c8245055 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,8 +8,11 @@ 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 ([#801]). +- The RBAC ServiceAccount and RoleBinding are now built with the operator-rs `v2::rbac` + functions and carry the full set of recommended labels ([#806]). [#801]: https://github.com/stackabletech/hdfs-operator/pull/801 +[#806]: https://github.com/stackabletech/hdfs-operator/pull/806 ## [26.7.0] - 2026-07-21 From 0efc5490e1db058eaf2caf0afdd8c7018901636f Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Wed, 22 Jul 2026 11:04:11 +0200 Subject: [PATCH 04/10] switch op-rs to main branch --- Cargo.lock | 9 +++++++++ Cargo.toml | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c9aeca22..8b894792 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1607,6 +1607,7 @@ dependencies = [ [[package]] name = "k8s-version" version = "0.1.3" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=main#0e5442574027eff876b6763df930dc6924d7bfdd" dependencies = [ "darling", "regex", @@ -3014,6 +3015,7 @@ checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" [[package]] name = "stackable-certs" version = "0.4.1" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=main#0e5442574027eff876b6763df930dc6924d7bfdd" dependencies = [ "const-oid", "ecdsa", @@ -3059,6 +3061,7 @@ dependencies = [ [[package]] name = "stackable-operator" version = "0.113.4" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=main#0e5442574027eff876b6763df930dc6924d7bfdd" dependencies = [ "base64", "clap", @@ -3103,6 +3106,7 @@ dependencies = [ [[package]] name = "stackable-operator-derive" version = "0.3.1" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=main#0e5442574027eff876b6763df930dc6924d7bfdd" dependencies = [ "darling", "proc-macro2", @@ -3113,6 +3117,7 @@ dependencies = [ [[package]] name = "stackable-shared" version = "0.1.2" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=main#0e5442574027eff876b6763df930dc6924d7bfdd" dependencies = [ "jiff", "k8s-openapi", @@ -3129,6 +3134,7 @@ dependencies = [ [[package]] name = "stackable-telemetry" version = "0.6.5" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=main#0e5442574027eff876b6763df930dc6924d7bfdd" dependencies = [ "axum", "clap", @@ -3152,6 +3158,7 @@ dependencies = [ [[package]] name = "stackable-versioned" version = "0.11.1" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=main#0e5442574027eff876b6763df930dc6924d7bfdd" dependencies = [ "kube", "schemars", @@ -3165,6 +3172,7 @@ dependencies = [ [[package]] name = "stackable-versioned-macros" version = "0.11.1" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=main#0e5442574027eff876b6763df930dc6924d7bfdd" dependencies = [ "convert_case", "convert_case_extras", @@ -3182,6 +3190,7 @@ dependencies = [ [[package]] name = "stackable-webhook" version = "0.9.2" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=main#0e5442574027eff876b6763df930dc6924d7bfdd" dependencies = [ "arc-swap", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index a14ccc4d..fb62d613 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,5 +30,5 @@ tracing = "0.1" tracing-futures = { version = "0.2", features = ["futures-03"] } [patch."https://github.com/stackabletech/operator-rs.git"] -#stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "main" } -stackable-operator = { path = "../operator-rs/crates/stackable-operator" } +stackable-operator = { git = "https://github.com/stackabletech//operator-rs.git", branch = "main" } +# stackable-operator = { path = "../operator-rs/crates/stackable-operator" } From c97d2dae127516831de5ad152bc655115365b019 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Wed, 22 Jul 2026 17:51:41 +0200 Subject: [PATCH 05/10] rename resource names functions, add role parse test --- .../src/controller/build/container.rs | 4 ++-- .../src/controller/build/mod.rs | 2 +- .../src/controller/build/resource/rbac.rs | 4 ++-- .../src/controller/build/resource/service.rs | 2 +- .../controller/build/resource/statefulset.rs | 2 +- rust/operator-binary/src/controller/mod.rs | 23 ++++++++++++++++--- 6 files changed, 27 insertions(+), 10 deletions(-) diff --git a/rust/operator-binary/src/controller/build/container.rs b/rust/operator-binary/src/controller/build/container.rs index 14bf10a6..be4ca4ad 100644 --- a/rust/operator-binary/src/controller/build/container.rs +++ b/rust/operator-binary/src/controller/build/container.rs @@ -237,7 +237,7 @@ impl ContainerConfig { // HDFS main container let main_container_config = Self::from(*role); - let resource_names = cluster.resource_names(role, role_group_name); + let resource_names = cluster.role_group_resource_names(role, role_group_name); let object_name = resource_names.qualified_role_group_name().to_string(); let merged_config = &rolegroup_config.config; @@ -285,7 +285,7 @@ impl ContainerConfig { log_config, vector_aggregator_config_map_name, }, - &cluster.resource_names(role, role_group_name), + &cluster.role_group_resource_names(role, role_group_name), &VECTOR_CONFIG_VOLUME_NAME, &VECTOR_LOG_VOLUME_NAME, EnvVarSet::new(), diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index a62f2d0c..31034ca8 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -192,7 +192,7 @@ pub(crate) fn rolegroup_metadata( ) -> ObjectMetaBuilder { cluster.object_meta( cluster - .resource_names(role, role_group_name) + .role_group_resource_names(role, role_group_name) .qualified_role_group_name() .to_string(), cluster.recommended_labels(role, role_group_name), diff --git a/rust/operator-binary/src/controller/build/resource/rbac.rs b/rust/operator-binary/src/controller/build/resource/rbac.rs index 8d52854b..e59a8818 100644 --- a/rust/operator-binary/src/controller/build/resource/rbac.rs +++ b/rust/operator-binary/src/controller/build/resource/rbac.rs @@ -20,7 +20,7 @@ stackable_operator::constant!(NONE_ROLE_GROUP_NAME: RoleGroupName = "none"); pub fn build_service_account(cluster: &ValidatedCluster) -> ServiceAccount { rbac::build_service_account( cluster, - &cluster.rbac_resource_names(), + &cluster.cluster_resource_names(), rbac_labels(cluster), ) } @@ -30,7 +30,7 @@ pub fn build_service_account(cluster: &ValidatedCluster) -> ServiceAccount { pub fn build_role_binding(cluster: &ValidatedCluster) -> RoleBinding { rbac::build_role_binding( cluster, - &cluster.rbac_resource_names(), + &cluster.cluster_resource_names(), rbac_labels(cluster), ) } diff --git a/rust/operator-binary/src/controller/build/resource/service.rs b/rust/operator-binary/src/controller/build/resource/service.rs index e5313e6f..a1acdd49 100644 --- a/rust/operator-binary/src/controller/build/resource/service.rs +++ b/rust/operator-binary/src/controller/build/resource/service.rs @@ -106,7 +106,7 @@ pub(crate) fn rolegroup_metrics_service( metadata: cluster .object_meta( cluster - .resource_names(role, role_group_name) + .role_group_resource_names(role, role_group_name) .metrics_service_name() .to_string(), cluster.recommended_labels(role, 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 index a13592e7..590b5b7a 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -70,7 +70,7 @@ pub(crate) fn build_rolegroup_statefulset( .affinity(&merged_config.affinity) .service_account_name( validated - .rbac_resource_names() + .cluster_resource_names() .service_account_name() .to_string(), ) diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index 5e4301d4..4008d71c 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -155,7 +155,7 @@ impl ValidatedCluster { /// 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 { + pub fn cluster_resource_names(&self) -> role_utils::ResourceNames { role_utils::ResourceNames { cluster_name: self.name.clone(), product_name: product_name(), @@ -163,7 +163,7 @@ impl ValidatedCluster { } /// Type-safe names for the resources of the given role group. - pub(crate) fn resource_names( + pub(crate) fn role_group_resource_names( &self, role: &HdfsNodeRole, role_group_name: &RoleGroupName, @@ -238,7 +238,7 @@ impl ValidatedCluster { role_group_name: &RoleGroupName, ) -> ServiceName { ServiceName::from_str( - self.resource_names(role, role_group_name) + self.role_group_resource_names(role, role_group_name) .qualified_role_group_name() .as_ref(), ) @@ -379,3 +379,20 @@ impl ValidatedClusterConfig { pub struct ValidatedRoleConfig { pub pdb: stackable_operator::commons::pdb::PdbConfig, } + +#[cfg(test)] +mod tests { + use strum::IntoEnumIterator; + + use super::ValidatedCluster; + use crate::crd::HdfsNodeRole; + + /// Locks the invariant behind the `expect` in [`ValidatedCluster::role_name`]: every + /// `HdfsNodeRole` variant (present and future) must serialise to a valid `RoleName`. + #[test] + fn every_hdfs_node_role_serialises_to_a_valid_role_name() { + for role in HdfsNodeRole::iter() { + ValidatedCluster::role_name(&role); + } + } +} From f5b2e167fbfd1973db47af002a1608e29e422da3 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Thu, 23 Jul 2026 18:40:15 +0200 Subject: [PATCH 06/10] replace role_name with From impls, add missing labels --- .../src/controller/build/mod.rs | 55 +++++++++++++++++++ .../controller/build/resource/discovery.rs | 2 +- .../src/controller/build/resource/service.rs | 16 +++++- rust/operator-binary/src/controller/mod.rs | 17 +++--- rust/operator-binary/src/crd/mod.rs | 13 +++++ 5 files changed, 91 insertions(+), 12 deletions(-) diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index 31034ca8..c39b346b 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -365,6 +365,61 @@ mod tests { names } + /// Every metrics Service must carry the Prometheus scrape label and the + /// `prometheus.io/path|port|scheme|scrape` annotations, or Prometheus stops discovering the + /// endpoints (caught by the HDFS smoke test 2026-07-23 after the labels migration dropped + /// them). + #[test] + fn metrics_services_carry_prometheus_label_and_annotations() { + let cluster = validated_cluster(); + let resources = build(&cluster, &cluster_info()).expect("build succeeds"); + + let metrics_services: Vec<_> = resources + .services + .iter() + .filter(|service| { + service + .metadata + .name + .as_deref() + .is_some_and(|name| name.ends_with("-metrics")) + }) + .collect(); + assert!(!metrics_services.is_empty(), "no metrics Services built"); + + for service in metrics_services { + let name = service.metadata.name.as_deref().unwrap_or_default(); + let labels = service.metadata.labels.as_ref().expect("labels are set"); + assert_eq!( + labels.get("prometheus.io/scrape").map(String::as_str), + Some("true"), + "{name} lacks the scrape label" + ); + + // The native metrics port of the role, as asserted by the smoke test. + let expected_port = match name { + n if n.contains("-namenode-") => "9870", + n if n.contains("-datanode-") => "9864", + n if n.contains("-journalnode-") => "8480", + other => panic!("unexpected metrics Service {other}"), + }; + let expected_annotations = BTreeMap::from( + [ + ("prometheus.io/path", "/prom"), + ("prometheus.io/port", expected_port), + ("prometheus.io/scheme", "http"), + ("prometheus.io/scrape", "true"), + ] + .map(|(key, value)| (key.to_string(), value.to_string())), + ); + assert_eq!( + service.metadata.annotations.as_ref(), + Some(&expected_annotations), + "{name} annotations mismatch" + ); + } + } + /// The aggregator emits, for the minimal three-role cluster (one `default` role group each): /// one StatefulSet and one ConfigMap per role group, one headless plus one metrics Service per /// role group, and one default PDB per role. diff --git a/rust/operator-binary/src/controller/build/resource/discovery.rs b/rust/operator-binary/src/controller/build/resource/discovery.rs index 253aa447..8cf7a331 100644 --- a/rust/operator-binary/src/controller/build/resource/discovery.rs +++ b/rust/operator-binary/src/controller/build/resource/discovery.rs @@ -63,7 +63,7 @@ pub fn build_discovery_config_map( &operator_name(), &ControllerName::from_str(HDFS_CONTROLLER_NAME) .expect("the hdfs controller name is a valid label value"), - &ValidatedCluster::role_name(&HdfsNodeRole::Name), + &HdfsNodeRole::Name.into(), &DISCOVERY_ROLE_GROUP, ); diff --git a/rust/operator-binary/src/controller/build/resource/service.rs b/rust/operator-binary/src/controller/build/resource/service.rs index a1acdd49..68a10510 100644 --- a/rust/operator-binary/src/controller/build/resource/service.rs +++ b/rust/operator-binary/src/controller/build/resource/service.rs @@ -5,7 +5,10 @@ use snafu::{ResultExt, Snafu}; use stackable_operator::{ k8s_openapi::api::core::v1::{Service, ServicePort, ServiceSpec}, kvp::LabelError, - v2::types::operator::RoleGroupName, + v2::{ + builder::service::{Scheme, Scraping, prometheus_annotations, prometheus_labels}, + types::operator::RoleGroupName, + }, }; use crate::{ @@ -111,6 +114,17 @@ pub(crate) fn rolegroup_metrics_service( .to_string(), cluster.recommended_labels(role, role_group_name), ) + .with_labels(prometheus_labels(&Scraping::Enabled)) + .with_annotations(prometheus_annotations( + &Scraping::Enabled, + if cluster.has_https_enabled() { + &Scheme::Https + } else { + &Scheme::Http + }, + "/prom", + &build::native_metrics_port(cluster, role), + )) .build(), spec: Some(service_spec), status: None, diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index 4008d71c..4955be6f 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -149,10 +149,6 @@ impl ValidatedCluster { } /// The type-safe role name for an HDFS role (`namenode`/`datanode`/`journalnode`). - pub(crate) fn role_name(role: &HdfsNodeRole) -> RoleName { - RoleName::from_str(&role.to_string()).expect("a HdfsNodeRole 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 { @@ -170,7 +166,7 @@ impl ValidatedCluster { ) -> ResourceNames { ResourceNames { cluster_name: self.name.clone(), - role_name: Self::role_name(role), + role_name: role.into(), role_group_name: role_group_name.clone(), } } @@ -207,7 +203,7 @@ impl ValidatedCluster { role: &HdfsNodeRole, role_group_name: &RoleGroupName, ) -> Labels { - self.recommended_labels_for(&Self::role_name(role), role_group_name) + self.recommended_labels_for(&role.into(), role_group_name) } /// Returns an [`ObjectMetaBuilder`] pre-filled with the namespace, the resource `name`, an owner @@ -382,17 +378,18 @@ pub struct ValidatedRoleConfig { #[cfg(test)] mod tests { + use stackable_operator::v2::types::operator::RoleName; use strum::IntoEnumIterator; - use super::ValidatedCluster; use crate::crd::HdfsNodeRole; - /// Locks the invariant behind the `expect` in [`ValidatedCluster::role_name`]: every - /// `HdfsNodeRole` variant (present and future) must serialise to a valid `RoleName`. + /// Locks the invariant behind the `expect` in the `From for RoleName` impls: + /// every `HdfsNodeRole` variant (present and future) must serialise to a valid `RoleName`. #[test] fn every_hdfs_node_role_serialises_to_a_valid_role_name() { for role in HdfsNodeRole::iter() { - ValidatedCluster::role_name(&role); + 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 5747df6e..75fe5c84 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -43,6 +43,7 @@ use stackable_operator::{ types::{ common::Port, kubernetes::{ConfigMapName, ListenerClassName, NamespaceName, ServiceName}, + operator::RoleName, }, }, versioned::versioned, @@ -497,6 +498,18 @@ pub enum HdfsNodeRole { Data, } +impl From for RoleName { + fn from(value: HdfsNodeRole) -> Self { + RoleName::from_str(&value.to_string()).expect("a HdfsNodeRole is a valid role name") + } +} + +impl From<&HdfsNodeRole> for RoleName { + fn from(value: &HdfsNodeRole) -> Self { + RoleName::from_str(&value.to_string()).expect("a HdfsNodeRole is a valid role name") + } +} + impl HdfsNodeRole { pub fn min_replicas(&self) -> u16 { match self { From 06c9f7e972627466fb3c280a2b93b93a0203957c Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 24 Jul 2026 16:58:51 +0200 Subject: [PATCH 07/10] added missed role/From instance --- rust/operator-binary/src/controller/build/resource/pdb.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/rust/operator-binary/src/controller/build/resource/pdb.rs b/rust/operator-binary/src/controller/build/resource/pdb.rs index de088c61..1251c729 100644 --- a/rust/operator-binary/src/controller/build/resource/pdb.rs +++ b/rust/operator-binary/src/controller/build/resource/pdb.rs @@ -1,7 +1,4 @@ -use std::{ - cmp::{max, min}, - str::FromStr, -}; +use std::cmp::{max, min}; use stackable_operator::{ k8s_openapi::api::policy::v1::PodDisruptionBudget, @@ -28,8 +25,7 @@ pub fn build_pdb(cluster: &ValidatedCluster, role: &HdfsNodeRole) -> Option max_unavailable_journal_nodes(), }); - let role_name = - RoleName::from_str(&role.to_string()).expect("a HdfsNodeRole is a valid role name"); + let role_name: RoleName = role.into(); let pdb = pod_disruption_budget_builder_with_role( cluster, &product_name(), From b45ed81d5bd29bc5d74e56030ec54adc9c4b5e64 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 24 Jul 2026 17:31:44 +0200 Subject: [PATCH 08/10] improve tests: placement and thoroughness --- .../src/controller/build/mod.rs | 120 +----------------- .../src/controller/build/resource/rbac.rs | 100 +++++++++++++++ .../src/controller/build/resource/service.rs | 86 +++++++++++++ 3 files changed, 193 insertions(+), 113 deletions(-) diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index c39b346b..c4591ed9 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -345,15 +345,10 @@ fn role_data_ports(role: &HdfsNodeRole, https_enabled: bool) -> Vec<(String, Por #[cfg(test)] mod tests { - use std::collections::BTreeMap; - use stackable_operator::kube::Resource; use super::build; - use crate::{ - controller::build::properties::test_support::{self, cluster_info, validated_cluster}, - test_support::deserialize_and_validate_cluster, - }; + use crate::controller::build::properties::test_support::{cluster_info, validated_cluster}; /// The sorted `metadata.name`s of a resource collection. fn sorted_names(resources: &[impl Resource]) -> Vec { @@ -365,61 +360,6 @@ mod tests { names } - /// Every metrics Service must carry the Prometheus scrape label and the - /// `prometheus.io/path|port|scheme|scrape` annotations, or Prometheus stops discovering the - /// endpoints (caught by the HDFS smoke test 2026-07-23 after the labels migration dropped - /// them). - #[test] - fn metrics_services_carry_prometheus_label_and_annotations() { - let cluster = validated_cluster(); - let resources = build(&cluster, &cluster_info()).expect("build succeeds"); - - let metrics_services: Vec<_> = resources - .services - .iter() - .filter(|service| { - service - .metadata - .name - .as_deref() - .is_some_and(|name| name.ends_with("-metrics")) - }) - .collect(); - assert!(!metrics_services.is_empty(), "no metrics Services built"); - - for service in metrics_services { - let name = service.metadata.name.as_deref().unwrap_or_default(); - let labels = service.metadata.labels.as_ref().expect("labels are set"); - assert_eq!( - labels.get("prometheus.io/scrape").map(String::as_str), - Some("true"), - "{name} lacks the scrape label" - ); - - // The native metrics port of the role, as asserted by the smoke test. - let expected_port = match name { - n if n.contains("-namenode-") => "9870", - n if n.contains("-datanode-") => "9864", - n if n.contains("-journalnode-") => "8480", - other => panic!("unexpected metrics Service {other}"), - }; - let expected_annotations = BTreeMap::from( - [ - ("prometheus.io/path", "/prom"), - ("prometheus.io/port", expected_port), - ("prometheus.io/scheme", "http"), - ("prometheus.io/scrape", "true"), - ] - .map(|(key, value)| (key.to_string(), value.to_string())), - ); - assert_eq!( - service.metadata.annotations.as_ref(), - Some(&expected_annotations), - "{name} annotations mismatch" - ); - } - } - /// The aggregator emits, for the minimal three-role cluster (one `default` role group each): /// one StatefulSet and one ConfigMap per role group, one headless plus one metrics Service per /// role group, and one default PDB per role. @@ -462,6 +402,12 @@ mod tests { sorted_names(&resources.pod_disruption_budgets), ["hdfs-datanode", "hdfs-journalnode", "hdfs-namenode"] ); + // The cluster-shared RBAC pair. + assert_eq!( + sorted_names(&resources.service_accounts), + ["hdfs-serviceaccount"] + ); + assert_eq!(sorted_names(&resources.role_bindings), ["hdfs-rolebinding"]); } /// Every StatefulSet's (immutable) `serviceName` must reference a headless Service that the @@ -486,56 +432,4 @@ mod tests { ); } } - - /// 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 - /// `hdfs`, which would mask exactly that swap). - #[test] - fn build_produces_rbac() { - let cluster = deserialize_and_validate_cluster( - &test_support::MINIMAL_HDFS_YAML.replace("name: hdfs", "name: my-hdfs"), - ); - let resources = build(&cluster, &cluster_info()).expect("build succeeds"); - - assert_eq!( - sorted_names(&resources.service_accounts), - ["my-hdfs-serviceaccount"] - ); - assert_eq!( - sorted_names(&resources.role_bindings), - ["my-hdfs-rolebinding"] - ); - - let expected_labels = BTreeMap::from( - [ - ("app.kubernetes.io/component", "none"), - ("app.kubernetes.io/instance", "my-hdfs"), - ( - "app.kubernetes.io/managed-by", - "hdfs.stackable.tech_hdfs-operator-hdfs-controller", - ), - ("app.kubernetes.io/name", "hdfs"), - ("app.kubernetes.io/role-group", "none"), - ("app.kubernetes.io/version", "3.4.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, "hdfs-clusterrole"); - } } diff --git a/rust/operator-binary/src/controller/build/resource/rbac.rs b/rust/operator-binary/src/controller/build/resource/rbac.rs index e59a8818..4dabc778 100644 --- a/rust/operator-binary/src/controller/build/resource/rbac.rs +++ b/rust/operator-binary/src/controller/build/resource/rbac.rs @@ -40,3 +40,103 @@ pub fn build_role_binding(cluster: &ValidatedCluster) -> RoleBinding { fn rbac_labels(cluster: &ValidatedCluster) -> Labels { cluster.recommended_labels_for(&NONE_ROLE_NAME, &NONE_ROLE_GROUP_NAME) } + +#[cfg(test)] +mod tests { + use serde_json::json; + + use super::*; + use crate::{ + controller::build::properties::test_support::MINIMAL_HDFS_YAML, + test_support::deserialize_and_validate_cluster, + }; + + /// The cluster name is deliberately different from the product name (`hdfs`) so that + /// swapped `name`/`instance` label values cannot pass unnoticed. + fn swap_guard_cluster() -> crate::controller::ValidatedCluster { + deserialize_and_validate_cluster(&MINIMAL_HDFS_YAML.replace("name: hdfs", "name: my-hdfs")) + } + + #[test] + fn test_service_account() { + let service_account = build_service_account(&swap_guard_cluster()); + + assert_eq!( + json!({ + "apiVersion": "v1", + "kind": "ServiceAccount", + "metadata": { + // The RBAC resources are cluster-shared, so role and role group are `none`. + "labels": { + "app.kubernetes.io/component": "none", + "app.kubernetes.io/instance": "my-hdfs", + "app.kubernetes.io/managed-by": "hdfs.stackable.tech_hdfs-operator-hdfs-controller", + "app.kubernetes.io/name": "hdfs", + "app.kubernetes.io/role-group": "none", + "app.kubernetes.io/version": "3.4.0-stackable0.0.0-dev", + "stackable.tech/vendor": "Stackable" + }, + "name": "my-hdfs-serviceaccount", + "namespace": "default", + "ownerReferences": [ + { + "apiVersion": "hdfs.stackable.tech/v1alpha1", + "controller": true, + "kind": "HdfsCluster", + "name": "my-hdfs", + "uid": "c2c8c5c0-0b5a-4b1e-9f3e-1a2b3c4d5e6f" + } + ] + } + }), + serde_json::to_value(service_account).expect("must be serializable") + ); + } + + #[test] + fn test_role_binding() { + let role_binding = build_role_binding(&swap_guard_cluster()); + + assert_eq!( + json!({ + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "RoleBinding", + "metadata": { + "labels": { + "app.kubernetes.io/component": "none", + "app.kubernetes.io/instance": "my-hdfs", + "app.kubernetes.io/managed-by": "hdfs.stackable.tech_hdfs-operator-hdfs-controller", + "app.kubernetes.io/name": "hdfs", + "app.kubernetes.io/role-group": "none", + "app.kubernetes.io/version": "3.4.0-stackable0.0.0-dev", + "stackable.tech/vendor": "Stackable" + }, + "name": "my-hdfs-rolebinding", + "namespace": "default", + "ownerReferences": [ + { + "apiVersion": "hdfs.stackable.tech/v1alpha1", + "controller": true, + "kind": "HdfsCluster", + "name": "my-hdfs", + "uid": "c2c8c5c0-0b5a-4b1e-9f3e-1a2b3c4d5e6f" + } + ] + }, + "roleRef": { + "apiGroup": "rbac.authorization.k8s.io", + "kind": "ClusterRole", + "name": "hdfs-clusterrole" + }, + "subjects": [ + { + "kind": "ServiceAccount", + "name": "my-hdfs-serviceaccount", + "namespace": "default" + } + ] + }), + serde_json::to_value(role_binding).expect("must be serializable") + ); + } +} diff --git a/rust/operator-binary/src/controller/build/resource/service.rs b/rust/operator-binary/src/controller/build/resource/service.rs index 68a10510..57e73390 100644 --- a/rust/operator-binary/src/controller/build/resource/service.rs +++ b/rust/operator-binary/src/controller/build/resource/service.rs @@ -130,3 +130,89 @@ pub(crate) fn rolegroup_metrics_service( status: None, }) } + +#[cfg(test)] +mod tests { + use serde_json::json; + use stackable_operator::v2::types::operator::RoleGroupName; + + use super::*; + use crate::{ + controller::build::properties::test_support::validated_cluster, crd::HdfsNodeRole, + }; + + #[test] + fn test_rolegroup_metrics_service() { + let cluster = validated_cluster(); + let role = &HdfsNodeRole::Name; + let role_group_name: RoleGroupName = "default".parse().expect("valid role group name"); + + let service = + rolegroup_metrics_service(&cluster, role, &role_group_name).expect("should not fail"); + + assert_eq!( + json!({ + "apiVersion": "v1", + "kind": "Service", + "metadata": { + // Every metrics Service must carry the Prometheus scrape label and the + // `prometheus.io/path|port|scheme|scrape` annotations, or Prometheus stops + // discovering the endpoints. + "annotations": { + "prometheus.io/path": "/prom", + "prometheus.io/port": "9870", + "prometheus.io/scheme": "http", + "prometheus.io/scrape": "true" + }, + "labels": { + "app.kubernetes.io/component": "namenode", + "app.kubernetes.io/instance": "hdfs", + "app.kubernetes.io/managed-by": "hdfs.stackable.tech_hdfs-operator-hdfs-controller", + "app.kubernetes.io/name": "hdfs", + "app.kubernetes.io/role-group": "default", + "app.kubernetes.io/version": "3.4.0-stackable0.0.0-dev", + "prometheus.io/scrape": "true", + "stackable.tech/vendor": "Stackable" + }, + "name": "hdfs-namenode-default-metrics", + "namespace": "default", + "ownerReferences": [ + { + "apiVersion": "hdfs.stackable.tech/v1alpha1", + "controller": true, + "kind": "HdfsCluster", + "name": "hdfs", + "uid": "c2c8c5c0-0b5a-4b1e-9f3e-1a2b3c4d5e6f" + } + ] + }, + "spec": { + "clusterIP": "None", + "ports": [ + { + "name": "metrics", + "port": 9870, + "protocol": "TCP" + }, + { + "name": "jmx-metrics", + "port": 8183, + "protocol": "TCP" + } + ], + "publishNotReadyAddresses": true, + "selector": { + "app.kubernetes.io/component": "namenode", + "app.kubernetes.io/instance": "hdfs", + "app.kubernetes.io/name": "hdfs", + "app.kubernetes.io/role-group": "default", + "group": "default", + "role": "namenode" + }, + "type": "ClusterIP" + } + }), + serde_json::to_value(service).expect("must be serializable") + ); + } +} From 43207ec4f23e872f8dba61cbeabcbc469de8bbbd Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 24 Jul 2026 17:45:48 +0200 Subject: [PATCH 09/10] move object_meta to build step --- .../src/controller/build/mod.rs | 24 +++++++- .../controller/build/resource/discovery.rs | 3 +- .../src/controller/build/resource/service.rs | 56 +++++++++---------- rust/operator-binary/src/controller/mod.rs | 14 ----- 4 files changed, 52 insertions(+), 45 deletions(-) diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index c4591ed9..a19c50d0 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -5,7 +5,10 @@ use stackable_operator::{ builder::meta::ObjectMetaBuilder, kvp::{LabelError, Labels}, utils::cluster_info::KubernetesClusterInfo, - v2::types::{common::Port, operator::RoleGroupName}, + v2::{ + builder::meta::ownerreference_from_resource, + types::{common::Port, operator::RoleGroupName}, + }, }; use crate::{ @@ -178,6 +181,22 @@ pub(crate) fn pod_refs(cluster: &ValidatedCluster, role: &HdfsNodeRole) -> Vec, + labels: Labels, +) -> ObjectMetaBuilder { + let mut builder = ObjectMetaBuilder::new(); + builder + .name_and_namespace(cluster) + .name(name) + .ownerreference(ownerreference_from_resource(cluster, None, Some(true))) + .with_labels(labels); + builder +} + /// Builds the common [`ObjectMetaBuilder`] shared by a role group's owned resources /// (the ConfigMap and the StatefulSet): name, namespace, owner reference and the /// recommended labels, all derived from the validated cluster. @@ -190,7 +209,8 @@ pub(crate) fn rolegroup_metadata( role: &HdfsNodeRole, role_group_name: &RoleGroupName, ) -> ObjectMetaBuilder { - cluster.object_meta( + object_meta( + cluster, cluster .role_group_resource_names(role, role_group_name) .qualified_role_group_name() diff --git a/rust/operator-binary/src/controller/build/resource/discovery.rs b/rust/operator-binary/src/controller/build/resource/discovery.rs index 8cf7a331..44b52cb8 100644 --- a/rust/operator-binary/src/controller/build/resource/discovery.rs +++ b/rust/operator-binary/src/controller/build/resource/discovery.rs @@ -18,6 +18,7 @@ use crate::{ ValidatedCluster, build::{ kerberos::KerberosConfig, + object_meta, properties::{ ConfigFileName, core_site::CoreSiteConfigBuilder, hdfs_site::HdfsSiteConfigBuilder, }, @@ -67,7 +68,7 @@ pub fn build_discovery_config_map( &DISCOVERY_ROLE_GROUP, ); - let metadata = cluster.object_meta(cluster.name.clone(), labels).build(); + let metadata = object_meta(cluster, cluster.name.clone(), labels).build(); ConfigMapBuilder::new() .metadata(metadata) diff --git a/rust/operator-binary/src/controller/build/resource/service.rs b/rust/operator-binary/src/controller/build/resource/service.rs index 57e73390..900348a3 100644 --- a/rust/operator-binary/src/controller/build/resource/service.rs +++ b/rust/operator-binary/src/controller/build/resource/service.rs @@ -61,14 +61,14 @@ pub(crate) fn rolegroup_headless_service( }; Ok(Service { - metadata: cluster - .object_meta( - cluster - .governing_service_name(role, role_group_name) - .to_string(), - cluster.recommended_labels(role, role_group_name), - ) - .build(), + metadata: build::object_meta( + cluster, + cluster + .governing_service_name(role, role_group_name) + .to_string(), + cluster.recommended_labels(role, role_group_name), + ) + .build(), spec: Some(service_spec), status: None, }) @@ -106,26 +106,26 @@ pub(crate) fn rolegroup_metrics_service( }; Ok(Service { - metadata: cluster - .object_meta( - cluster - .role_group_resource_names(role, role_group_name) - .metrics_service_name() - .to_string(), - cluster.recommended_labels(role, role_group_name), - ) - .with_labels(prometheus_labels(&Scraping::Enabled)) - .with_annotations(prometheus_annotations( - &Scraping::Enabled, - if cluster.has_https_enabled() { - &Scheme::Https - } else { - &Scheme::Http - }, - "/prom", - &build::native_metrics_port(cluster, role), - )) - .build(), + metadata: build::object_meta( + cluster, + cluster + .role_group_resource_names(role, role_group_name) + .metrics_service_name() + .to_string(), + cluster.recommended_labels(role, role_group_name), + ) + .with_labels(prometheus_labels(&Scraping::Enabled)) + .with_annotations(prometheus_annotations( + &Scraping::Enabled, + if cluster.has_https_enabled() { + &Scheme::Https + } else { + &Scheme::Http + }, + "/prom", + &build::native_metrics_port(cluster, role), + )) + .build(), spec: Some(service_spec), status: None, }) diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index 4955be6f..2f351d7a 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -1,7 +1,6 @@ use std::{collections::BTreeMap, str::FromStr}; use stackable_operator::{ - builder::meta::ObjectMetaBuilder, commons::product_image_selection::ResolvedProductImage, k8s_openapi::api::{ apps::v1::StatefulSet, @@ -13,7 +12,6 @@ use stackable_operator::{ kvp::Labels, v2::{ HasName, HasUid, NameIsValidLabelValue, - builder::meta::ownerreference_from_resource, kvp::label::recommended_labels, role_group_utils::ResourceNames, role_utils::{self, RoleGroupConfig}, @@ -206,18 +204,6 @@ impl ValidatedCluster { self.recommended_labels_for(&role.into(), role_group_name) } - /// Returns an [`ObjectMetaBuilder`] pre-filled with the namespace, the resource `name`, an owner - /// reference back to this cluster, and the given recommended `labels`. - pub(crate) fn object_meta(&self, name: impl Into, labels: Labels) -> ObjectMetaBuilder { - let mut builder = ObjectMetaBuilder::new(); - builder - .name_and_namespace(self) - .name(name) - .ownerreference(ownerreference_from_resource(self, None, Some(true))) - .with_labels(labels); - builder - } - /// The name of a role group's governing headless Service. /// /// Used as the headless Service's own name, as the StatefulSet's (immutable) `serviceName`, From 96d773134ef4d9659593e121665b9597afe27b28 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Mon, 27 Jul 2026 10:54:26 +0200 Subject: [PATCH 10/10] derive the version label in tests from the operator version --- .../src/controller/build/resource/rbac.rs | 6 +++--- .../src/controller/build/resource/service.rs | 3 ++- rust/operator-binary/src/test_support.rs | 12 ++++++++++++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/rust/operator-binary/src/controller/build/resource/rbac.rs b/rust/operator-binary/src/controller/build/resource/rbac.rs index 4dabc778..2e05dc7e 100644 --- a/rust/operator-binary/src/controller/build/resource/rbac.rs +++ b/rust/operator-binary/src/controller/build/resource/rbac.rs @@ -48,7 +48,7 @@ mod tests { use super::*; use crate::{ controller::build::properties::test_support::MINIMAL_HDFS_YAML, - test_support::deserialize_and_validate_cluster, + test_support::{app_version_label, deserialize_and_validate_cluster}, }; /// The cluster name is deliberately different from the product name (`hdfs`) so that @@ -73,7 +73,7 @@ mod tests { "app.kubernetes.io/managed-by": "hdfs.stackable.tech_hdfs-operator-hdfs-controller", "app.kubernetes.io/name": "hdfs", "app.kubernetes.io/role-group": "none", - "app.kubernetes.io/version": "3.4.0-stackable0.0.0-dev", + "app.kubernetes.io/version": app_version_label("3.4.0"), "stackable.tech/vendor": "Stackable" }, "name": "my-hdfs-serviceaccount", @@ -108,7 +108,7 @@ mod tests { "app.kubernetes.io/managed-by": "hdfs.stackable.tech_hdfs-operator-hdfs-controller", "app.kubernetes.io/name": "hdfs", "app.kubernetes.io/role-group": "none", - "app.kubernetes.io/version": "3.4.0-stackable0.0.0-dev", + "app.kubernetes.io/version": app_version_label("3.4.0"), "stackable.tech/vendor": "Stackable" }, "name": "my-hdfs-rolebinding", diff --git a/rust/operator-binary/src/controller/build/resource/service.rs b/rust/operator-binary/src/controller/build/resource/service.rs index 900348a3..61a5863d 100644 --- a/rust/operator-binary/src/controller/build/resource/service.rs +++ b/rust/operator-binary/src/controller/build/resource/service.rs @@ -139,6 +139,7 @@ mod tests { use super::*; use crate::{ controller::build::properties::test_support::validated_cluster, crd::HdfsNodeRole, + test_support::app_version_label, }; #[test] @@ -170,7 +171,7 @@ mod tests { "app.kubernetes.io/managed-by": "hdfs.stackable.tech_hdfs-operator-hdfs-controller", "app.kubernetes.io/name": "hdfs", "app.kubernetes.io/role-group": "default", - "app.kubernetes.io/version": "3.4.0-stackable0.0.0-dev", + "app.kubernetes.io/version": app_version_label("3.4.0"), "prometheus.io/scrape": "true", "stackable.tech/vendor": "Stackable" }, diff --git a/rust/operator-binary/src/test_support.rs b/rust/operator-binary/src/test_support.rs index 61a02307..4f47b3b9 100644 --- a/rust/operator-binary/src/test_support.rs +++ b/rust/operator-binary/src/test_support.rs @@ -7,6 +7,18 @@ use crate::{ crd::{AnyNodeConfig, DataNodeConfig, HdfsNodeRole, v1alpha1}, }; +/// 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 + ) +} + pub fn deserialize_cluster(spec: &str) -> v1alpha1::HdfsCluster { let deserializer = serde_yaml::Deserializer::from_str(spec); serde_yaml::with::singleton_map_recursive::deserialize(deserializer).expect("")