diff --git a/CHANGELOG.md b/CHANGELOG.md index 851019e7..67a2863a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,13 @@ - Internal operator refactoring: introduce a build() step in the reconciler that assembles all relevant Kubernetes resources before anything is applied ([#756]). - Bump stackable-operator to 0.114.0 ([#765]). +- The RBAC ServiceAccount and RoleBinding are now built with the operator-rs `v2::rbac` + functions and carry the full set of recommended labels ([#761]). +- BREAKING: The `nodes` role is now required by the CRD; a SupersetCluster without it was + previously accepted by the API server but reconciled to no `nodes` resources ([#761]). [#756]: https://github.com/stackabletech/superset-operator/pull/756 +[#761]: https://github.com/stackabletech/superset-operator/pull/761 [#765]: https://github.com/stackabletech/superset-operator/pull/765 ## [26.7.0] - 2026-07-21 diff --git a/extra/crds.yaml b/extra/crds.yaml index 4ef981fc..a2acc28f 100644 --- a/extra/crds.yaml +++ b/extra/crds.yaml @@ -1183,7 +1183,6 @@ spec: at role level, the `roleConfig`. You can learn more about this in the [Roles and role group concept documentation](https://docs.stackable.tech/home/nightly/concepts/roles-and-role-groups). - nullable: true properties: cliOverrides: additionalProperties: @@ -2669,6 +2668,7 @@ spec: required: - clusterConfig - image + - nodes type: object status: nullable: true diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index f9db48a2..17011681 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -15,17 +15,17 @@ use stackable_operator::{ affinity::StackableAffinity, product_image_selection::ResolvedProductImage, random_secret_creation::{self, create_random_secret_if_not_exists}, - rbac::build_rbac_resources, resources::{NoRuntimeLimits, Resources}, }, crd::listener, k8s_openapi::api::{ apps::v1::{Deployment, StatefulSet}, - core::v1::{ConfigMap, Secret, Service}, + core::v1::{ConfigMap, Secret, Service, ServiceAccount}, policy::v1::PodDisruptionBudget, + rbac::v1::RoleBinding, }, kube::{ - Resource, ResourceExt, + Resource, api::ObjectMeta, core::{DeserializeGuard, error_boundary}, runtime::controller::Action, @@ -39,12 +39,11 @@ use stackable_operator::{ }, v2::{ HasName, HasUid, NameIsValidLabelValue, - builder::meta::ownerreference_from_resource, cluster_resources::cluster_resources_new, kvp::label::{recommended_labels, role_group_selector}, product_logging::framework::{ValidatedContainerLogConfigChoice, VectorContainerLogConfig}, role_group_utils::ResourceNames, - role_utils::{GenericCommonConfig, RoleGroupConfig}, + role_utils::{self, GenericCommonConfig, RoleGroupConfig}, types::{ kubernetes::{ListenerClassName, ListenerName, NamespaceName, Uid}, operator::{ @@ -94,6 +93,8 @@ pub struct KubernetesResources { pub listeners: Vec, pub config_maps: Vec, pub pod_disruption_budgets: Vec, + pub service_accounts: Vec, + pub role_bindings: Vec, } /// Per-role configuration extracted during validation. @@ -226,24 +227,33 @@ impl ValidatedCluster { } } - pub fn resource_names( + pub fn role_group_resource_names( &self, role: &SupersetRole, role_group_name: &RoleGroupName, ) -> ResourceNames { ResourceNames { cluster_name: self.name.clone(), - role_name: role.role_name(), + role_name: role.into(), role_group_name: role_group_name.clone(), } } + /// 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(), + } + } + pub fn recommended_labels( &self, role: &SupersetRole, role_group_name: &RoleGroupName, ) -> Labels { - self.recommended_labels_for(&role.role_name(), role_group_name) + self.recommended_labels_for(&role.into(), role_group_name) } pub fn recommended_labels_for( @@ -263,7 +273,7 @@ impl ValidatedCluster { ) -> Labels { self.recommended_labels_with( &build::UNVERSIONED_PRODUCT_VERSION, - &role.role_name(), + &role.into(), role_group_name, ) } @@ -290,28 +300,7 @@ impl ValidatedCluster { role: &SupersetRole, role_group_name: &RoleGroupName, ) -> Labels { - role_group_selector(self, &product_name(), &role.role_name(), role_group_name) - } - - /// Returns an [`ObjectMetaBuilder`] pre-filled with the namespace, an owner reference back to - /// this cluster, and the recommended labels for a resource named `name` in `role`/ - /// `role_group_name`. - /// - /// Consolidates the metadata chain repeated by the role-group child-resource builders. Call - /// sites that need extra labels/annotations chain them onto the returned builder. - pub(crate) fn object_meta( - &self, - name: impl Into, - role: &SupersetRole, - role_group_name: &RoleGroupName, - ) -> ObjectMetaBuilder { - let mut builder = ObjectMetaBuilder::new(); - builder - .name_and_namespace(self) - .name(name) - .ownerreference(ownerreference_from_resource(self, None, Some(true))) - .with_labels(self.recommended_labels(role, role_group_name)); - builder + role_group_selector(self, &product_name(), &role.into(), role_group_name) } } @@ -409,27 +398,6 @@ pub enum Error { source: stackable_operator::client::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 build RBAC objects"))] - BuildRBACObjects { - source: stackable_operator::commons::rbac::Error, - }, - - #[snafu(display("failed to get required Labels"))] - GetRequiredLabels { - source: - stackable_operator::kvp::KeyValuePairError, - }, - #[snafu(display("SupersetCluster object is invalid"))] InvalidSupersetCluster { source: error_boundary::InvalidObject, @@ -504,24 +472,6 @@ pub async fn reconcile_superset( &superset.spec.object_overrides, ); - let (rbac_sa, rbac_rolebinding) = build_rbac_resources( - superset, - APP_NAME, - cluster_resources - .get_required_labels() - .context(GetRequiredLabelsSnafu)?, - ) - .context(BuildRBACObjectsSnafu)?; - - let rbac_sa = cluster_resources - .add(client, rbac_sa) - .await - .context(ApplyServiceAccountSnafu)?; - cluster_resources - .add(client, rbac_rolebinding) - .await - .context(ApplyRoleBindingSnafu)?; - // TODO: Can be removed after SDP 26.7 is released (it's only a migration from 26.3 - 26.7) // (don't forget about the snafu Error variants). // Removal is tracked in https://github.com/stackabletech/superset-operator/issues/755 @@ -536,7 +486,7 @@ pub async fn reconcile_superset( .await .context(CreateSecretKeySecretSnafu)?; - let resources = build::build(&validated, &rbac_sa.name_any()).context(BuildResourcesSnafu)?; + let resources = build::build(&validated).context(BuildResourcesSnafu)?; let mut statefulset_cond_builder = StatefulSetConditionBuilder::default(); let mut deployment_cond_builder = DeploymentConditionBuilder::default(); @@ -544,6 +494,18 @@ pub async fn reconcile_superset( // The StatefulSets/Deployments are applied last, so every ConfigMap and Secret they mount // already exists — otherwise a changed mount would restart the Pods. // 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) @@ -687,6 +649,18 @@ pub fn error_policy( #[cfg(test)] pub(crate) mod test_support { + /// 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 + ) + } + use crate::{ controller::dereference::DereferencedObjects, crd::authentication::{ diff --git a/rust/operator-binary/src/controller/build/mod.rs b/rust/operator-binary/src/controller/build/mod.rs index 0b2a1a0e..72ea8676 100644 --- a/rust/operator-binary/src/controller/build/mod.rs +++ b/rust/operator-binary/src/controller/build/mod.rs @@ -3,7 +3,13 @@ use std::str::FromStr; use snafu::{ResultExt, Snafu}; -use stackable_operator::v2::types::operator::{ProductVersion, RoleGroupName}; +use stackable_operator::{ + builder::meta::ObjectMetaBuilder, + v2::{ + builder::meta::ownerreference_from_resource, + types::operator::{ProductVersion, RoleGroupName}, + }, +}; use crate::{ controller::{ @@ -13,6 +19,7 @@ use crate::{ deployment::build_rolegroup_deployment, listener::build_group_listener, pdb::build_pdb, + rbac::{build_role_binding, build_service_account}, service::{build_rolegroup_headless_service, build_rolegroup_metrics_service}, statefulset::build_node_rolegroup_statefulset, }, @@ -26,7 +33,7 @@ pub mod resource; // Placeholder role-group name used for the recommended labels of the role-level `Listener` // (which is not tied to a single role group). -stackable_operator::constant!(pub(crate) PLACEHOLDER_LISTENER_ROLE_GROUP: RoleGroupName = "none"); +stackable_operator::constant!(pub(crate) NONE_ROLE_GROUP_NAME: RoleGroupName = "none"); // Product version used for the recommended labels of PVC templates, which cannot be modified after // deployment. A constant `none` keeps those labels stable across version upgrades. @@ -54,13 +61,7 @@ pub enum Error { } /// Builds every Kubernetes resource for the given validated cluster. -/// -/// `service_account_name` is the name of the RBAC `ServiceAccount` the role-group Pods run under. -/// The RBAC resources themselves are built and applied separately in the reconcile step. -pub fn build( - cluster: &ValidatedCluster, - service_account_name: &str, -) -> Result { +pub fn build(cluster: &ValidatedCluster) -> Result { let mut stateful_sets = vec![]; let mut deployments = vec![]; let mut services = vec![]; @@ -110,7 +111,6 @@ pub fn build( superset_role, role_group_name, rolegroup_config, - service_account_name, ) .context(StatefulSetSnafu { role_group: role_group_name.clone(), @@ -124,7 +124,6 @@ pub fn build( superset_role, role_group_name, rolegroup_config, - service_account_name, ) .context(DeploymentSnafu { role_group: role_group_name.clone(), @@ -162,14 +161,36 @@ pub fn build( listeners, config_maps, pod_disruption_budgets, + service_accounts: vec![build_service_account(cluster)], + role_bindings: vec![build_role_binding(cluster)], }) } +/// Returns an [`ObjectMetaBuilder`] pre-filled with the namespace, an owner reference back to +/// the cluster, and the recommended labels for a resource named `name` in `role`/ +/// `role_group_name`. +/// +/// Consolidates the metadata chain repeated by the role-group child-resource builders. Call +/// sites that need extra labels/annotations chain them onto the returned builder. +pub(crate) fn object_meta( + validated: &ValidatedCluster, + name: impl Into, + role: &SupersetRole, + role_group_name: &RoleGroupName, +) -> ObjectMetaBuilder { + let mut builder = ObjectMetaBuilder::new(); + builder + .name_and_namespace(validated) + .name(name) + .ownerreference(ownerreference_from_resource(validated, None, Some(true))) + .with_labels(validated.recommended_labels(role, role_group_name)); + builder +} + #[cfg(test)] -mod tests { - use stackable_operator::{kube::Resource, utils::yaml_from_str_singleton_map}; +pub(crate) mod test_support { + use stackable_operator::utils::yaml_from_str_singleton_map; - use super::build; use crate::{ controller::{ ValidatedCluster, test_support::default_dereferenced, validate::validate_cluster, @@ -177,8 +198,13 @@ mod tests { crd::v1alpha1, }; - /// A validated cluster with a `node`, `worker` and `beat` role (one `default` role group each). - fn validated_cluster() -> ValidatedCluster { + /// A validated cluster with a `node`, `worker` and `beat` role (one `default` role group + /// each). + /// + /// The cluster name (`simple-superset`) deliberately differs from the product name + /// (`superset`), so tests asserting recommended labels catch swapped `name`/`instance` + /// values. + pub fn validated_cluster() -> ValidatedCluster { let input = r#" apiVersion: superset.stackable.tech/v1alpha1 kind: SupersetCluster @@ -213,6 +239,13 @@ mod tests { yaml_from_str_singleton_map(input).expect("illegal test input"); validate_cluster(&superset, default_dereferenced(), "test-repo").expect("validated") } +} + +#[cfg(test)] +mod tests { + use stackable_operator::kube::Resource; + + use super::{build, test_support::validated_cluster}; fn sorted_names(resources: &[impl Resource]) -> Vec<&str> { let mut names: Vec<&str> = resources @@ -230,7 +263,7 @@ mod tests { #[test] fn build_produces_expected_resource_names() { let cluster = validated_cluster(); - let resources = build(&cluster, "simple-superset-serviceaccount").expect("build succeeds"); + let resources = build(&cluster).expect("build succeeds"); assert_eq!( sorted_names(&resources.stateful_sets), @@ -262,5 +295,14 @@ mod tests { "simple-superset-worker" ] ); + // The cluster-shared RBAC pair. + assert_eq!( + sorted_names(&resources.service_accounts), + ["simple-superset-serviceaccount"] + ); + assert_eq!( + sorted_names(&resources.role_bindings), + ["simple-superset-rolebinding"] + ); } } 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 77683dfb..e2343f6e 100644 --- a/rust/operator-binary/src/controller/build/resource/config_map.rs +++ b/rust/operator-binary/src/controller/build/resource/config_map.rs @@ -7,7 +7,10 @@ use stackable_operator::{ use crate::{ controller::{ ValidatedCluster, ValidatedSupersetConfig, - build::properties::{ConfigFileName, product_logging, superset_config}, + build::{ + object_meta, + properties::{ConfigFileName, product_logging, superset_config}, + }, }, crd::{SupersetRole, v1alpha1::SupersetConfigOverrides}, }; @@ -46,16 +49,16 @@ pub fn build_rolegroup_config_map( cm_builder .metadata( - validated - .object_meta( - validated - .resource_names(role, role_group_name) - .role_group_config_map() - .to_string(), - role, - role_group_name, - ) - .build(), + object_meta( + validated, + validated + .role_group_resource_names(role, role_group_name) + .role_group_config_map() + .to_string(), + role, + role_group_name, + ) + .build(), ) .add_data(ConfigFileName::SupersetConfig.to_string(), config_file); diff --git a/rust/operator-binary/src/controller/build/resource/deployment.rs b/rust/operator-binary/src/controller/build/resource/deployment.rs index a857c602..3d2d4200 100644 --- a/rust/operator-binary/src/controller/build/resource/deployment.rs +++ b/rust/operator-binary/src/controller/build/resource/deployment.rs @@ -22,7 +22,10 @@ use stackable_operator::{ }; use crate::{ - controller::{SupersetRoleGroupConfig, ValidatedCluster, build::properties::ConfigFileName}, + controller::{ + SupersetRoleGroupConfig, ValidatedCluster, + build::{object_meta, properties::ConfigFileName}, + }, crd::{PYTHONPATH, STACKABLE_CONFIG_DIR, STACKABLE_LOG_CONFIG_DIR, SupersetRole}, }; @@ -64,11 +67,10 @@ pub fn build_rolegroup_deployment( superset_role: &SupersetRole, role_group_name: &RoleGroupName, rolegroup_config: &SupersetRoleGroupConfig, - sa_name: &str, ) -> Result { let merged_config = &rolegroup_config.config; - let resource_names = validated.resource_names(superset_role, role_group_name); + let resource_names = validated.role_group_resource_names(superset_role, role_group_name); let recommended_object_labels = validated.recommended_labels(superset_role, role_group_name); // The Celery process command, liveness probe and replica policy are the only differences @@ -102,7 +104,12 @@ pub fn build_rolegroup_deployment( .build(), ) .affinity(&merged_config.affinity) - .service_account_name(sa_name); + .service_account_name( + validated + .cluster_resource_names() + .service_account_name() + .to_string(), + ); let mut superset_cb = super::build_superset_container_builder(validated, rolegroup_config) .context(BuildContainerSnafu)?; @@ -157,14 +164,14 @@ pub fn build_rolegroup_deployment( pod_template.merge_from(rolegroup_config.pod_overrides.clone()); Ok(Deployment { - metadata: validated - .object_meta( - resource_names.deployment_name().to_string(), - superset_role, - role_group_name, - ) - .with_label(RESTART_CONTROLLER_ENABLED_LABEL.to_owned()) - .build(), + metadata: object_meta( + validated, + resource_names.deployment_name().to_string(), + superset_role, + role_group_name, + ) + .with_label(RESTART_CONTROLLER_ENABLED_LABEL.to_owned()) + .build(), spec: Some(DeploymentSpec { replicas: replicas.map(i32::from), selector: LabelSelector { diff --git a/rust/operator-binary/src/controller/build/resource/listener.rs b/rust/operator-binary/src/controller/build/resource/listener.rs index 1319a235..f5d493be 100644 --- a/rust/operator-binary/src/controller/build/resource/listener.rs +++ b/rust/operator-binary/src/controller/build/resource/listener.rs @@ -1,7 +1,10 @@ use stackable_operator::{crd::listener, v2::types::kubernetes::ListenerClassName}; use crate::{ - controller::{ValidatedCluster, build::PLACEHOLDER_LISTENER_ROLE_GROUP}, + controller::{ + ValidatedCluster, + build::{NONE_ROLE_GROUP_NAME, object_meta}, + }, crd::{APP_PORT, APP_PORT_NAME, SupersetRole}, }; @@ -15,9 +18,7 @@ pub fn build_group_listener( ) -> listener::v1alpha1::Listener { // The group listener is a role-level object, so the constant `none` placeholder role-group is // used for the recommended labels. - let metadata = validated - .object_meta(listener_group_name, role, &PLACEHOLDER_LISTENER_ROLE_GROUP) - .build(); + let metadata = object_meta(validated, listener_group_name, role, &NONE_ROLE_GROUP_NAME).build(); let spec = listener::v1alpha1::ListenerSpec { class_name: Some(listener_class.to_string()), diff --git a/rust/operator-binary/src/controller/build/resource/mod.rs b/rust/operator-binary/src/controller/build/resource/mod.rs index 9c9bd98b..05f3b529 100644 --- a/rust/operator-binary/src/controller/build/resource/mod.rs +++ b/rust/operator-binary/src/controller/build/resource/mod.rs @@ -49,6 +49,7 @@ pub mod config_map; pub mod deployment; pub mod listener; pub mod pdb; +pub mod rbac; pub mod service; pub mod statefulset; @@ -252,7 +253,7 @@ pub(crate) fn build_vector_container( &Container::Vector.to_container_name(), &validated.image, vector_log_config, - &validated.resource_names(superset_role, role_group_name), + &validated.role_group_resource_names(superset_role, role_group_name), &CONFIG_VOLUME_NAME, &LOG_VOLUME_NAME, EnvVarSet::new(), diff --git a/rust/operator-binary/src/controller/build/resource/pdb.rs b/rust/operator-binary/src/controller/build/resource/pdb.rs index 002e73ca..7aece9a3 100644 --- a/rust/operator-binary/src/controller/build/resource/pdb.rs +++ b/rust/operator-binary/src/controller/build/resource/pdb.rs @@ -25,7 +25,7 @@ pub fn build_pdb( let pdb = pod_disruption_budget_builder_with_role( validated, &product_name(), - &role.role_name(), + &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..ad478f43 --- /dev/null +++ b/rust/operator-binary/src/controller/build/resource/rbac.rs @@ -0,0 +1,137 @@ +//! 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) +} + +#[cfg(test)] +mod tests { + use serde_json::json; + + use super::*; + use crate::controller::{ + build::test_support::validated_cluster, test_support::app_version_label, + }; + + // `simple-superset` vs `superset`: see the swap-guard note on `validated_cluster`. + + #[test] + fn test_service_account() { + let service_account = build_service_account(&validated_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": "simple-superset", + "app.kubernetes.io/managed-by": "superset.stackable.tech_supersetcluster", + "app.kubernetes.io/name": "superset", + "app.kubernetes.io/role-group": "none", + "app.kubernetes.io/version": app_version_label("4.1.4"), + "stackable.tech/vendor": "Stackable" + }, + "name": "simple-superset-serviceaccount", + "namespace": "default", + "ownerReferences": [ + { + "apiVersion": "superset.stackable.tech/v1alpha1", + "controller": true, + "kind": "SupersetCluster", + "name": "simple-superset", + "uid": "01234567-89ab-cdef-0123-456789abcdef" + } + ] + } + }), + serde_json::to_value(service_account).expect("must be serializable") + ); + } + + #[test] + fn test_role_binding() { + let role_binding = build_role_binding(&validated_cluster()); + + assert_eq!( + json!({ + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "RoleBinding", + "metadata": { + "labels": { + "app.kubernetes.io/component": "none", + "app.kubernetes.io/instance": "simple-superset", + "app.kubernetes.io/managed-by": "superset.stackable.tech_supersetcluster", + "app.kubernetes.io/name": "superset", + "app.kubernetes.io/role-group": "none", + "app.kubernetes.io/version": app_version_label("4.1.4"), + "stackable.tech/vendor": "Stackable" + }, + "name": "simple-superset-rolebinding", + "namespace": "default", + "ownerReferences": [ + { + "apiVersion": "superset.stackable.tech/v1alpha1", + "controller": true, + "kind": "SupersetCluster", + "name": "simple-superset", + "uid": "01234567-89ab-cdef-0123-456789abcdef" + } + ] + }, + "roleRef": { + "apiGroup": "rbac.authorization.k8s.io", + "kind": "ClusterRole", + "name": "superset-clusterrole" + }, + "subjects": [ + { + "kind": "ServiceAccount", + "name": "simple-superset-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 ab350ba0..9b682381 100644 --- a/rust/operator-binary/src/controller/build/resource/service.rs +++ b/rust/operator-binary/src/controller/build/resource/service.rs @@ -7,7 +7,7 @@ use stackable_operator::{ }; use crate::{ - controller::ValidatedCluster, + controller::{ValidatedCluster, build::object_meta}, crd::{APP_PORT, APP_PORT_NAME, METRICS_PORT, METRICS_PORT_NAME, SupersetRole}, }; @@ -25,16 +25,16 @@ pub fn build_rolegroup_headless_service( role_group_name: &RoleGroupName, ) -> Service { Service { - metadata: validated - .object_meta( - validated - .resource_names(role, role_group_name) - .headless_service_name() - .to_string(), - role, - role_group_name, - ) - .build(), + metadata: object_meta( + validated, + validated + .role_group_resource_names(role, role_group_name) + .headless_service_name() + .to_string(), + role, + role_group_name, + ) + .build(), spec: Some(ServiceSpec { // Internal communication does not need to be exposed type_: Some(SERVICE_TYPE_CLUSTER_IP.to_owned()), @@ -54,22 +54,22 @@ pub fn build_rolegroup_metrics_service( role: &SupersetRole, role_group_name: &RoleGroupName, ) -> Service { - let resource_names = validated.resource_names(role, role_group_name); + let resource_names = validated.role_group_resource_names(role, role_group_name); Service { - metadata: validated - .object_meta( - resource_names.metrics_service_name().to_string(), - role, - role_group_name, - ) - .with_labels(prometheus_labels(&Scraping::Enabled)) - .with_annotations(prometheus_annotations( - &Scraping::Enabled, - &Scheme::Http, - "/metrics", - &METRICS_PORT, - )) - .build(), + metadata: object_meta( + validated, + resource_names.metrics_service_name().to_string(), + role, + role_group_name, + ) + .with_labels(prometheus_labels(&Scraping::Enabled)) + .with_annotations(prometheus_annotations( + &Scraping::Enabled, + &Scheme::Http, + "/metrics", + &METRICS_PORT, + )) + .build(), spec: Some(ServiceSpec { // Internal communication does not need to be exposed type_: Some(SERVICE_TYPE_CLUSTER_IP.to_owned()), @@ -100,3 +100,81 @@ fn service_ports() -> Vec { ..ServicePort::default() }] } + +#[cfg(test)] +mod tests { + use serde_json::json; + use stackable_operator::v2::types::operator::RoleGroupName; + + use super::*; + use crate::controller::{ + build::test_support::validated_cluster, test_support::app_version_label, + }; + + /// Every metrics Service must carry the Prometheus scrape label and the + /// `prometheus.io/path|port|scheme|scrape` annotations, or Prometheus stops discovering the + /// endpoints. + #[test] + fn test_rolegroup_metrics_service() { + let validated = validated_cluster(); + let role_group_name: RoleGroupName = "default".parse().expect("valid role group name"); + + let service = + build_rolegroup_metrics_service(&validated, &SupersetRole::Node, &role_group_name); + + assert_eq!( + json!({ + "apiVersion": "v1", + "kind": "Service", + "metadata": { + "annotations": { + "prometheus.io/path": "/metrics", + "prometheus.io/port": "9102", + "prometheus.io/scheme": "http", + "prometheus.io/scrape": "true" + }, + "labels": { + "app.kubernetes.io/component": "node", + "app.kubernetes.io/instance": "simple-superset", + "app.kubernetes.io/managed-by": "superset.stackable.tech_supersetcluster", + "app.kubernetes.io/name": "superset", + "app.kubernetes.io/role-group": "default", + "app.kubernetes.io/version": app_version_label("4.1.4"), + "prometheus.io/scrape": "true", + "stackable.tech/vendor": "Stackable" + }, + "name": "simple-superset-node-default-metrics", + "namespace": "default", + "ownerReferences": [ + { + "apiVersion": "superset.stackable.tech/v1alpha1", + "controller": true, + "kind": "SupersetCluster", + "name": "simple-superset", + "uid": "01234567-89ab-cdef-0123-456789abcdef" + } + ] + }, + "spec": { + "clusterIP": "None", + "ports": [ + { + "name": "metrics", + "port": 9102, + "protocol": "TCP" + } + ], + "publishNotReadyAddresses": true, + "selector": { + "app.kubernetes.io/component": "node", + "app.kubernetes.io/instance": "simple-superset", + "app.kubernetes.io/name": "superset", + "app.kubernetes.io/role-group": "default" + }, + "type": "ClusterIP" + } + }), + serde_json::to_value(service).expect("must be serializable") + ); + } +} diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs index 0ec8b888..11a6be38 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -38,6 +38,7 @@ use crate::{ SupersetRoleGroupConfig, ValidatedCluster, build::{ command::add_cert_to_python_certifi_command, + object_meta, properties::{ConfigFileName, superset_config}, resource::listener::LISTENER_VOLUME_DIR, }, @@ -94,11 +95,10 @@ pub fn build_node_rolegroup_statefulset( superset_role: &SupersetRole, role_group_name: &RoleGroupName, rolegroup_config: &SupersetRoleGroupConfig, - sa_name: &str, ) -> Result { let merged_config = &rolegroup_config.config; - let resource_names = validated.resource_names(superset_role, role_group_name); + let resource_names = validated.role_group_resource_names(superset_role, role_group_name); let recommended_object_labels = validated.recommended_labels(superset_role, role_group_name); // Used for PVC templates that cannot be modified once they are deployed (a constant "none" // version keeps the labels stable across version upgrades). @@ -118,7 +118,12 @@ pub fn build_node_rolegroup_statefulset( .build(), ) .affinity(&merged_config.affinity) - .service_account_name(sa_name); + .service_account_name( + validated + .cluster_resource_names() + .service_account_name() + .to_string(), + ); let mut superset_cb = super::build_superset_container_builder(validated, rolegroup_config) .context(BuildContainerSnafu)?; @@ -234,14 +239,14 @@ pub fn build_node_rolegroup_statefulset( pod_template.merge_from(rolegroup_config.pod_overrides.clone()); Ok(StatefulSet { - metadata: validated - .object_meta( - resource_names.stateful_set_name().to_string(), - superset_role, - role_group_name, - ) - .with_label(RESTART_CONTROLLER_ENABLED_LABEL.to_owned()) - .build(), + metadata: object_meta( + validated, + resource_names.stateful_set_name().to_string(), + superset_role, + role_group_name, + ) + .with_label(RESTART_CONTROLLER_ENABLED_LABEL.to_owned()) + .build(), spec: Some(StatefulSetSpec { pod_management_policy: Some(POD_MANAGEMENT_POLICY_ORDERED_READY.to_string()), replicas: rolegroup_config.replicas.map(i32::from), diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 0da57a77..56c14146 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, ContainerName, ListenerClassName}, + operator::RoleName, }, }, versioned::versioned, @@ -174,8 +175,7 @@ pub mod versioned { pub object_overrides: ObjectOverrides, // no doc - docs in the struct. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub nodes: Option, + pub nodes: SupersetRoleType, // no doc - docs in the struct. #[serde(default, skip_serializing_if = "Option::is_none")] @@ -409,19 +409,27 @@ impl SupersetRole { superset: &v1alpha1::SupersetCluster, ) -> Option { match self { - Self::Node => superset - .spec - .nodes - .as_ref() - .map(|node| node.role_config.listener_class.clone()), + Self::Node => Some(superset.spec.nodes.role_config.listener_class.clone()), Self::Worker | Self::Beat => None, } } +} - pub fn role_name(&self) -> stackable_operator::v2::types::operator::RoleName { - self.to_string() +impl From for RoleName { + fn from(value: SupersetRole) -> Self { + value + .to_string() .parse() - .expect("a Superset serialises to a valid RoleName") + .expect("a SupersetRole serialises to a valid RoleName") + } +} + +impl From<&SupersetRole> for RoleName { + fn from(value: &SupersetRole) -> Self { + value + .to_string() + .parse() + .expect("a SupersetRole serialises to a valid RoleName") } } @@ -586,7 +594,9 @@ impl v1alpha1::SupersetCluster { pub fn get_role(&self, role: &SupersetRole) -> Option<&SupersetRoleType> { match role { - SupersetRole::Node => self.spec.nodes.as_ref(), + // The `nodes` role is required by the CRD; `Option` is kept for the signature shared + // with the genuinely optional Celery roles. + SupersetRole::Node => Some(&self.spec.nodes), SupersetRole::Worker => self.spec.workers.as_ref(), SupersetRole::Beat => self.spec.beat.as_ref(), } @@ -603,9 +613,22 @@ impl v1alpha1::SupersetCluster { #[cfg(test)] mod tests { - use stackable_operator::versioned::test_utils::RoundtripTestData; - - use super::v1alpha1; + use stackable_operator::{ + v2::types::operator::RoleName, versioned::test_utils::RoundtripTestData, + }; + use strum::IntoEnumIterator; + + use super::{SupersetRole, v1alpha1}; + + /// Locks the invariant behind the `expect` in the `From for RoleName` impls: + /// every `SupersetRole` variant (present and future) must serialise to a valid `RoleName`. + #[test] + fn every_superset_role_serialises_to_a_valid_role_name() { + for role in SupersetRole::iter() { + let _: RoleName = (&role).into(); + let _: RoleName = role.into(); + } + } impl RoundtripTestData for v1alpha1::SupersetClusterSpec { fn roundtrip_test_data() -> Vec {