diff --git a/CHANGELOG.md b/CHANGELOG.md index 984ee009..3814242e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,13 @@ 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 ([#961]). - Bump stackable-operator to 0.114.0 ([#970]) +- The RBAC ServiceAccount and RoleBinding are now built with the operator-rs `v2::rbac` + functions and carry the full set of recommended labels ([#966]). +- BREAKING: The `nodes` role is now required by the CRD; a NifiCluster without it was + previously accepted by the API server but failed reconciliation ([#966]). [#961]: https://github.com/stackabletech/nifi-operator/pull/961 +[#966]: https://github.com/stackabletech/nifi-operator/pull/966 [#970]: https://github.com/stackabletech/nifi-operator/pull/970 ## [26.7.0] - 2026-07-21 diff --git a/extra/crds.yaml b/extra/crds.yaml index 73720bae..8d9b8e7e 100644 --- a/extra/crds.yaml +++ b/extra/crds.yaml @@ -489,7 +489,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: @@ -2209,6 +2208,7 @@ spec: required: - clusterConfig - image + - nodes type: object status: nullable: true diff --git a/rust/operator-binary/src/controller/build.rs b/rust/operator-binary/src/controller/build.rs index 7ba9d7c9..6376f9df 100644 --- a/rust/operator-binary/src/controller/build.rs +++ b/rust/operator-binary/src/controller/build.rs @@ -4,8 +4,14 @@ use std::str::FromStr; -use snafu::{OptionExt, ResultExt, Snafu}; -use stackable_operator::v2::types::{common::Port, operator::RoleGroupName}; +use snafu::{ResultExt, Snafu}; +use stackable_operator::{ + builder::meta::ObjectMetaBuilder, + v2::{ + builder::meta::ownerreference_from_resource, + types::{common::Port, operator::RoleGroupName}, + }, +}; use crate::{ controller::{ @@ -14,6 +20,7 @@ use crate::{ config_map::build_rolegroup_config_map, listener::{build_group_listener, group_listener_name}, pdb::build_pdb, + rbac::{build_role_binding, build_service_account}, service::{build_rolegroup_headless_service, build_rolegroup_metrics_service}, statefulset::build_node_rolegroup_statefulset, }, @@ -45,9 +52,6 @@ pub const NIFI_PYTHON_WORKING_DIRECTORY: &str = "/nifi-python-working-directory" #[derive(Snafu, Debug)] pub enum Error { - #[snafu(display("NifiCluster has no nodes role defined"))] - NoNodesDefined, - #[snafu(display("failed to build ConfigMap for role group {role_group}"))] ConfigMap { source: resource::config_map::Error, @@ -66,13 +70,7 @@ pub enum Error { /// Does not need a Kubernetes client: every reference to another Kubernetes resource is already /// dereferenced and validated by this point, so the errors returned here are resource-assembly /// failures only. -/// -/// `service_account_name` is the name of the RBAC `ServiceAccount` the role-group Pods run under -/// (RBAC resources are built and applied separately, in the reconcile step). -pub fn build( - cluster: &ValidatedCluster, - service_account_name: &str, -) -> Result { +pub fn build(cluster: &ValidatedCluster) -> Result { let mut stateful_sets = vec![]; let mut services = vec![]; let mut listeners = vec![]; @@ -84,7 +82,7 @@ pub fn build( let node_role_group_configs = cluster .role_group_configs .get(&nifi_role) - .context(NoNodesDefinedSnafu)?; + .expect("the nodes role is required by the CRD and validate always inserts it"); // Role-level resources (one per role): the PodDisruptionBudget and the group Listener. let role_config = &cluster.role_config; @@ -109,16 +107,10 @@ pub fn build( let effective_replicas = rg.replicas.map(i32::from); stateful_sets.push( - build_node_rolegroup_statefulset( - cluster, - role_group_name, - rg, - effective_replicas, - service_account_name, - ) - .context(StatefulSetSnafu { - role_group: role_group_name.clone(), - })?, + build_node_rolegroup_statefulset(cluster, role_group_name, rg, effective_replicas) + .context(StatefulSetSnafu { + role_group: role_group_name.clone(), + })?, ); } @@ -128,9 +120,33 @@ 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_group_name`. +/// +/// Consolidates the metadata chain repeated by the child-resource builders. Call sites that +/// need extra labels/annotations chain them onto the returned builder. Role-level resources +/// (e.g. the per-role [`Listener`](stackable_operator::crd::listener::v1alpha1::Listener)) pass +/// the placeholder role-group `none`, preserving the historical +/// `app.kubernetes.io/role-group: none` label. +pub(crate) fn object_meta( + cluster: &ValidatedCluster, + name: impl Into, + role_group_name: &RoleGroupName, +) -> ObjectMetaBuilder { + let mut builder = ObjectMetaBuilder::new(); + builder + .name_and_namespace(cluster) + .name(name) + .ownerreference(ownerreference_from_resource(cluster, None, Some(true))) + .with_labels(cluster.recommended_labels(role_group_name)); + builder +} + #[cfg(test)] mod tests { use stackable_operator::kube::Resource; @@ -149,7 +165,7 @@ mod tests { #[test] fn build_produces_expected_resources() { let cluster = minimal_validated_cluster(); - let resources = build(&cluster, "simple-nifi-serviceaccount").expect("build succeeds"); + let resources = build(&cluster).expect("build succeeds"); // The minimal fixture has a single `default` role group for the `node` role. assert_eq!( @@ -168,5 +184,14 @@ mod tests { sorted_names(&resources.pod_disruption_budgets), ["simple-nifi-node"] ); + // The cluster-shared RBAC pair. + assert_eq!( + sorted_names(&resources.service_accounts), + ["simple-nifi-serviceaccount"] + ); + assert_eq!( + sorted_names(&resources.role_bindings), + ["simple-nifi-rolebinding"] + ); } } diff --git a/rust/operator-binary/src/controller/build/properties.rs b/rust/operator-binary/src/controller/build/properties.rs index 58c4ba0c..760fe651 100644 --- a/rust/operator-binary/src/controller/build/properties.rs +++ b/rust/operator-binary/src/controller/build/properties.rs @@ -104,8 +104,23 @@ 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 + ) + } + /// A minimal NiFi cluster YAML. Mirrors the fixture used by bootstrap_conf tests, /// stripped down to the mandatory fields only (Kubernetes clustering backend, SingleUser auth). + /// + /// The cluster name (`simple-nifi`) deliberately differs from the product name (`nifi`), so + /// tests asserting recommended labels catch swapped `name`/`instance` values. pub const MINIMAL_NIFI_YAML: &str = r#" apiVersion: nifi.stackable.tech/v1alpha1 kind: NifiCluster @@ -140,10 +155,12 @@ pub(crate) mod test_support { let nifi: v1alpha1::NifiCluster = serde_yaml::from_str(MINIMAL_NIFI_YAML).expect("invalid test YAML"); + // Mirrors what `image.resolve()` produces in production, so label-asserting tests see + // realistic values. let image = ResolvedProductImage { product_version: "2.9.0".to_string(), - app_version_label_value: "2.9.0".parse::().unwrap(), - image: "oci.stackable.tech/sdp/nifi:2.9.0-stackable0.0.0-dev".to_string(), + app_version_label_value: app_version_label("2.9.0").parse::().unwrap(), + image: format!("oci.stackable.tech/sdp/nifi:{}", app_version_label("2.9.0")), image_pull_policy: "IfNotPresent".to_string(), pull_secrets: None, }; @@ -151,13 +168,11 @@ pub(crate) mod test_support { let role_group_configs = build_role_group_configs(&nifi, &image, &None) .expect("role group configs should merge for minimal fixture"); - let role_config = nifi - .role_config(&NifiRole::Node) - .map(|role_config| ValidatedRoleConfig { - pdb: role_config.common.pod_disruption_budget.clone(), - listener_class: role_config.listener_class.clone(), - }) - .expect("the minimal fixture defines the nodes role"); + let node_role_config = nifi.role_config(&NifiRole::Node); + let role_config = ValidatedRoleConfig { + pdb: node_role_config.common.pod_disruption_budget.clone(), + listener_class: node_role_config.listener_class.clone(), + }; let name = ClusterName::from_str("simple-nifi").expect("valid cluster name"); let namespace = NamespaceName::from_str("default").expect("valid namespace"); 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 d402b96a..7f4cc896 100644 --- a/rust/operator-binary/src/controller/build/resource/config_map.rs +++ b/rust/operator-binary/src/controller/build/resource/config_map.rs @@ -9,6 +9,7 @@ use stackable_operator::{ use crate::controller::{ NifiRoleGroupConfig, ValidatedCluster, build::{ + object_meta, properties::{ ConfigFileName, authorizers, bootstrap_conf, login_identity_providers, nifi_properties, product_logging, security_properties, state_management_xml, @@ -66,15 +67,15 @@ pub fn build_rolegroup_config_map( cm_builder .metadata( - cluster - .object_meta( - cluster - .resource_names(role_group_name) - .role_group_config_map() - .to_string(), - role_group_name, - ) - .build(), + object_meta( + cluster, + cluster + .role_group_resource_names(role_group_name) + .role_group_config_map() + .to_string(), + role_group_name, + ) + .build(), ) .add_data( ConfigFileName::BootstrapConf.to_string(), diff --git a/rust/operator-binary/src/controller/build/resource/listener.rs b/rust/operator-binary/src/controller/build/resource/listener.rs index fe979328..11151b7c 100644 --- a/rust/operator-binary/src/controller/build/resource/listener.rs +++ b/rust/operator-binary/src/controller/build/resource/listener.rs @@ -14,7 +14,7 @@ use stackable_operator::{ use crate::controller::{ ValidatedCluster, - build::{HTTPS_PORT, HTTPS_PORT_NAME, PLACEHOLDER_LISTENER_ROLE_GROUP}, + build::{HTTPS_PORT, HTTPS_PORT_NAME, PLACEHOLDER_LISTENER_ROLE_GROUP, object_meta}, }; pub const LISTENER_VOLUME_NAME: &str = "listener"; @@ -29,12 +29,12 @@ pub fn build_group_listener( listener_group_name: ListenerName, ) -> Listener { Listener { - metadata: cluster - .object_meta( - listener_group_name.to_string(), - &PLACEHOLDER_LISTENER_ROLE_GROUP, - ) - .build(), + metadata: object_meta( + cluster, + listener_group_name.to_string(), + &PLACEHOLDER_LISTENER_ROLE_GROUP, + ) + .build(), spec: ListenerSpec { class_name: Some(listener_class.to_string()), ports: Some(vec![ListenerPort { diff --git a/rust/operator-binary/src/controller/build/resource/mod.rs b/rust/operator-binary/src/controller/build/resource/mod.rs index d040825b..9598a324 100644 --- a/rust/operator-binary/src/controller/build/resource/mod.rs +++ b/rust/operator-binary/src/controller/build/resource/mod.rs @@ -5,5 +5,6 @@ pub mod config_map; pub mod listener; pub mod pdb; +pub mod rbac; pub mod service; pub mod statefulset; diff --git a/rust/operator-binary/src/controller/build/resource/pdb.rs b/rust/operator-binary/src/controller/build/resource/pdb.rs index 9c8be14a..ec44cd3c 100644 --- a/rust/operator-binary/src/controller/build/resource/pdb.rs +++ b/rust/operator-binary/src/controller/build/resource/pdb.rs @@ -23,7 +23,7 @@ pub fn build_pdb( let pdb = pod_disruption_budget_builder_with_role( cluster, &product_name(), - &ValidatedCluster::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..acb29d8d --- /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::properties::test_support::{ + app_version_label, minimal_validated_cluster, + }; + + // `simple-nifi` vs `nifi`: see the swap-guard note on `MINIMAL_NIFI_YAML`. + + #[test] + fn test_service_account() { + let service_account = build_service_account(&minimal_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-nifi", + "app.kubernetes.io/managed-by": "nifi.stackable.tech_nificluster", + "app.kubernetes.io/name": "nifi", + "app.kubernetes.io/role-group": "none", + "app.kubernetes.io/version": app_version_label("2.9.0"), + "stackable.tech/vendor": "Stackable" + }, + "name": "simple-nifi-serviceaccount", + "namespace": "default", + "ownerReferences": [ + { + "apiVersion": "nifi.stackable.tech/v1alpha1", + "controller": true, + "kind": "NifiCluster", + "name": "simple-nifi", + "uid": "e6ac237d-a6d4-43a1-8135-f36506110912" + } + ] + } + }), + serde_json::to_value(service_account).expect("must be serializable") + ); + } + + #[test] + fn test_role_binding() { + let role_binding = build_role_binding(&minimal_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-nifi", + "app.kubernetes.io/managed-by": "nifi.stackable.tech_nificluster", + "app.kubernetes.io/name": "nifi", + "app.kubernetes.io/role-group": "none", + "app.kubernetes.io/version": app_version_label("2.9.0"), + "stackable.tech/vendor": "Stackable" + }, + "name": "simple-nifi-rolebinding", + "namespace": "default", + "ownerReferences": [ + { + "apiVersion": "nifi.stackable.tech/v1alpha1", + "controller": true, + "kind": "NifiCluster", + "name": "simple-nifi", + "uid": "e6ac237d-a6d4-43a1-8135-f36506110912" + } + ] + }, + "roleRef": { + "apiGroup": "rbac.authorization.k8s.io", + "kind": "ClusterRole", + "name": "nifi-clusterrole" + }, + "subjects": [ + { + "kind": "ServiceAccount", + "name": "simple-nifi-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 e2c6947e..0aeab6d9 100644 --- a/rust/operator-binary/src/controller/build/resource/service.rs +++ b/rust/operator-binary/src/controller/build/resource/service.rs @@ -9,7 +9,7 @@ use stackable_operator::{ use crate::controller::{ ValidatedCluster, - build::{HTTPS_PORT, HTTPS_PORT_NAME}, + build::{HTTPS_PORT, HTTPS_PORT_NAME, object_meta}, }; /// The rolegroup headless [`Service`] is a service that allows direct access to the instances of a certain rolegroup @@ -19,15 +19,15 @@ pub fn build_rolegroup_headless_service( role_group_name: &RoleGroupName, ) -> Service { Service { - metadata: cluster - .object_meta( - cluster - .resource_names(role_group_name) - .headless_service_name() - .to_string(), - role_group_name, - ) - .build(), + metadata: object_meta( + cluster, + cluster + .role_group_resource_names(role_group_name) + .headless_service_name() + .to_string(), + role_group_name, + ) + .build(), spec: Some(ServiceSpec { // Internal communication does not need to be exposed type_: Some("ClusterIP".to_string()), @@ -47,17 +47,17 @@ pub fn build_rolegroup_metrics_service( role_group_name: &RoleGroupName, ) -> Service { Service { - metadata: cluster - .object_meta( - cluster - .resource_names(role_group_name) - .metrics_service_name() - .to_string(), - role_group_name, - ) - .with_labels(service::prometheus_labels(&Scraping::Enabled)) - .with_annotations(prometheus_annotations()) - .build(), + metadata: object_meta( + cluster, + cluster + .role_group_resource_names(role_group_name) + .metrics_service_name() + .to_string(), + role_group_name, + ) + .with_labels(service::prometheus_labels(&Scraping::Enabled)) + .with_annotations(prometheus_annotations()) + .build(), spec: Some(ServiceSpec { // Internal communication does not need to be exposed type_: Some("ClusterIP".to_string()), @@ -103,9 +103,12 @@ mod tests { use std::str::FromStr as _; use pretty_assertions::assert_eq; + use serde_json::json; use super::*; - use crate::controller::build::properties::test_support::minimal_validated_cluster; + use crate::controller::build::properties::test_support::{ + app_version_label, minimal_validated_cluster, + }; #[test] fn headless_service_is_cluster_ip_none_with_https_port() { @@ -124,4 +127,70 @@ mod tests { assert_eq!(Some(HTTPS_PORT_NAME.to_string()), ports[0].name); assert_eq!(i32::from(HTTPS_PORT), ports[0].port); } + + /// 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 cluster = minimal_validated_cluster(); + let role_group_name = RoleGroupName::from_str("default").expect("valid role-group name"); + + let service = build_rolegroup_metrics_service(&cluster, &role_group_name); + + assert_eq!( + json!({ + "apiVersion": "v1", + "kind": "Service", + "metadata": { + "annotations": { + "prometheus.io/path": "/nifi-api/flow/metrics/prometheus", + "prometheus.io/port": "8443", + "prometheus.io/scheme": "https", + "prometheus.io/scrape": "true" + }, + "labels": { + "app.kubernetes.io/component": "node", + "app.kubernetes.io/instance": "simple-nifi", + "app.kubernetes.io/managed-by": "nifi.stackable.tech_nificluster", + "app.kubernetes.io/name": "nifi", + "app.kubernetes.io/role-group": "default", + "app.kubernetes.io/version": app_version_label("2.9.0"), + "prometheus.io/scrape": "true", + "stackable.tech/vendor": "Stackable" + }, + "name": "simple-nifi-node-default-metrics", + "namespace": "default", + "ownerReferences": [ + { + "apiVersion": "nifi.stackable.tech/v1alpha1", + "controller": true, + "kind": "NifiCluster", + "name": "simple-nifi", + "uid": "e6ac237d-a6d4-43a1-8135-f36506110912" + } + ] + }, + "spec": { + "clusterIP": "None", + "ports": [ + { + "name": "https", + "port": 8443, + "protocol": "TCP" + } + ], + "publishNotReadyAddresses": true, + "selector": { + "app.kubernetes.io/component": "node", + "app.kubernetes.io/instance": "simple-nifi", + "app.kubernetes.io/name": "nifi", + "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 8c01c35f..98df6735 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -52,6 +52,7 @@ use crate::{ BALANCE_PORT, BALANCE_PORT_NAME, HTTPS_PORT, HTTPS_PORT_NAME, NIFI_CONFIG_DIRECTORY, NIFI_PYTHON_WORKING_DIRECTORY, PROTOCOL_PORT, PROTOCOL_PORT_NAME, graceful_shutdown::add_graceful_shutdown_config, + object_meta, properties::ConfigFileName, resource::listener::{ LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME, build_group_listener_pvc, @@ -169,7 +170,6 @@ pub(crate) fn build_node_rolegroup_statefulset( role_group_name: &RoleGroupName, rg: &NifiRoleGroupConfig, effective_replicas: Option, - service_account_name: &str, ) -> Result { tracing::debug!("Building statefulset"); @@ -181,7 +181,7 @@ pub(crate) fn build_node_rolegroup_statefulset( let git_sync_resources = &rg.config.git_sync_resources; // Type-safe names for this role group's resources (StatefulSet, ConfigMap, headless Service). - let resource_names = cluster.resource_names(role_group_name); + let resource_names = cluster.role_group_resource_names(role_group_name); // The validated, merged `NifiConfig` is the single source of truth; the ConfigMap builder // sources the same `rg.config`. @@ -604,7 +604,7 @@ pub(crate) fn build_node_rolegroup_statefulset( &cluster.cluster_config.server_tls_secret_class, &KEYSTORE_VOLUME_NAME, [cluster - .resource_names(role_group_name) + .role_group_resource_names(role_group_name) .metrics_service_name() .to_string()], SecretFormat::TlsPkcs12, @@ -642,7 +642,12 @@ pub(crate) fn build_node_rolegroup_statefulset( ..Volume::default() }) .context(AddVolumeSnafu)? - .service_account_name(service_account_name) + .service_account_name( + cluster + .cluster_resource_names() + .service_account_name() + .to_string(), + ) .security_context(PodSecurityContextBuilder::new().fs_group(1000).build()); let mut pod_template = pod_builder.build_template(); @@ -650,13 +655,13 @@ pub(crate) fn build_node_rolegroup_statefulset( pod_template.merge_from(rg.pod_overrides.clone()); Ok(StatefulSet { - metadata: cluster - .object_meta( - resource_names.stateful_set_name().to_string(), - role_group_name, - ) - .with_label(RESTART_CONTROLLER_ENABLED_LABEL.to_owned()) - .build(), + metadata: object_meta( + cluster, + resource_names.stateful_set_name().to_string(), + role_group_name, + ) + .with_label(RESTART_CONTROLLER_ENABLED_LABEL.to_owned()) + .build(), spec: Some(StatefulSetSpec { pod_management_policy: Some("Parallel".to_string()), replicas: effective_replicas, @@ -709,7 +714,7 @@ fn get_volume_claim_templates( // Used for PVC templates that cannot be modified once they are deployed, so the version label // is set to the placeholder `none` to keep the labels stable across version upgrades. - let unversioned_recommended_labels = cluster.recommended_labels_unversioned(role_group_name); + let unversioned_recommended_labels = cluster.unversioned_recommended_labels(role_group_name); // listener endpoints will use persistent volumes // so that load balancers can hard-code the target addresses and diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index 4098ad17..dc8a01c5 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -15,8 +15,9 @@ use stackable_operator::{ k8s_openapi::{ api::{ apps::v1::StatefulSet, - core::v1::{ConfigMap, Service, Volume}, + core::v1::{ConfigMap, Service, ServiceAccount, Volume}, policy::v1::PodDisruptionBudget, + rbac::v1::RoleBinding, }, apimachinery::pkg::apis::meta::v1::ObjectMeta, }, @@ -28,7 +29,7 @@ use stackable_operator::{ kvp::label::{recommended_labels, role_group_selector}, product_logging::framework::{ValidatedContainerLogConfigChoice, VectorContainerLogConfig}, role_group_utils::ResourceNames, - role_utils::{JavaCommonConfig, RoleGroupConfig}, + role_utils::{self, JavaCommonConfig, RoleGroupConfig}, types::{ kubernetes::{ListenerClassName, NamespaceName, SecretClassName, SecretName, Uid}, operator::{ @@ -55,6 +56,9 @@ pub(crate) mod build; pub(crate) mod dereference; pub(crate) mod validate; +// Placeholder version label value for resources whose labels must not change after deployment. +stackable_operator::constant!(UNVERSIONED_PRODUCT_VERSION: ProductVersion = "none"); + /// Every Kubernetes resource produced by the [`build`] step. pub struct KubernetesResources { pub stateful_sets: Vec, @@ -62,6 +66,8 @@ pub struct KubernetesResources { pub listeners: Vec, pub config_maps: Vec, pub pod_disruption_budgets: Vec, + pub service_accounts: Vec, + pub role_bindings: Vec, } /// A validated, merged (default <- role <- role-group) NiFi rolegroup config. @@ -168,7 +174,7 @@ pub struct ValidatedCluster { /// label on built resources. pub product_version: ProductVersion, /// Per-role configuration (PodDisruptionBudget and listener class). The `nodes` role is - /// mandatory, so this is always present. + /// required by the CRD, so this is always present. pub role_config: ValidatedRoleConfig, /// Cluster wide settings. pub cluster_config: ValidatedClusterConfig, @@ -247,25 +253,55 @@ impl ValidatedCluster { } } - /// The single NiFi role name (`node`). - pub fn role_name() -> RoleName { - RoleName::from_str(&NifiRole::Node.to_string()) - .expect("the node role name is a valid role name") + /// Type-safe names for the per-cluster RBAC resources: the ServiceAccount shared by all + /// Pods, its (namespaced) RoleBinding, and the operator-deployed ClusterRole it binds. + pub fn cluster_resource_names(&self) -> role_utils::ResourceNames { + role_utils::ResourceNames { + cluster_name: self.name.clone(), + product_name: product_name(), + } } /// Type-safe names for the resources of a given role group. - pub(crate) fn resource_names(&self, role_group_name: &RoleGroupName) -> ResourceNames { + pub(crate) fn role_group_resource_names( + &self, + role_group_name: &RoleGroupName, + ) -> ResourceNames { ResourceNames { cluster_name: self.name.clone(), - role_name: Self::role_name(), + role_name: NifiRole::Node.into(), role_group_name: role_group_name.clone(), } } - /// Recommended labels for a role-group resource, using the given product version. - fn recommended_labels_for( + /// Recommended labels for a role-group resource. + pub fn recommended_labels(&self, role_group_name: &RoleGroupName) -> Labels { + self.recommended_labels_for(&NifiRole::Node.into(), role_group_name) + } + + /// Recommended labels for a resource that is not tied to a concrete role, 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) + } + + /// Recommended labels with the constant [`UNVERSIONED_PRODUCT_VERSION`], for PVC templates + /// that cannot be modified after deployment (keeps the labels stable across version upgrades). + pub fn unversioned_recommended_labels(&self, role_group_name: &RoleGroupName) -> Labels { + self.recommended_labels_with( + &UNVERSIONED_PRODUCT_VERSION, + &NifiRole::Node.into(), + role_group_name, + ) + } + + fn recommended_labels_with( &self, product_version: &ProductVersion, + role_name: &RoleName, role_group_name: &RoleGroupName, ) -> Labels { recommended_labels( @@ -274,57 +310,19 @@ impl ValidatedCluster { product_version, &operator_name(), &controller_name(), - &Self::role_name(), + role_name, role_group_name, ) } - /// Recommended labels for a role-group resource. - pub fn recommended_labels(&self, role_group_name: &RoleGroupName) -> Labels { - self.recommended_labels_for(&self.product_version, role_group_name) - } - - /// Recommended labels for resources whose labels must stay stable across version upgrades - /// (e.g. PVC templates, which are immutable once created), using the placeholder version - /// `none` for `app.kubernetes.io/version`. - pub fn recommended_labels_unversioned(&self, role_group_name: &RoleGroupName) -> Labels { - let unversioned = ProductVersion::from_str("none") - .expect("'none' is a valid product version label value"); - self.recommended_labels_for(&unversioned, role_group_name) - } - /// Selector labels matching the pods of a role group. pub fn role_group_selector(&self, role_group_name: &RoleGroupName) -> Labels { - role_group_selector(self, &product_name(), &Self::role_name(), role_group_name) - } - - /// Returns an [`ObjectMetaBuilder`](stackable_operator::builder::meta::ObjectMetaBuilder) - /// pre-filled with the namespace, an owner reference back to this cluster, and the recommended - /// labels for a resource named `name` in `role_group_name`. - /// - /// Consolidates the metadata chain repeated by the child-resource builders. Call sites that - /// need extra labels/annotations chain them onto the returned builder. Role-level resources - /// (e.g. the per-role [`Listener`](stackable_operator::crd::listener::v1alpha1::Listener)) pass - /// the placeholder role-group `none`, preserving the historical - /// `app.kubernetes.io/role-group: none` label. - pub(crate) fn object_meta( - &self, - name: impl Into, - role_group_name: &RoleGroupName, - ) -> stackable_operator::builder::meta::ObjectMetaBuilder { - let mut builder = stackable_operator::builder::meta::ObjectMetaBuilder::new(); - builder - .name_and_namespace(self) - .name(name) - .ownerreference( - stackable_operator::v2::builder::meta::ownerreference_from_resource( - self, - None, - Some(true), - ), - ) - .with_labels(self.recommended_labels(role_group_name)); - builder + role_group_selector( + self, + &product_name(), + &NifiRole::Node.into(), + role_group_name, + ) } } @@ -390,3 +388,21 @@ impl Resource for ValidatedCluster { &mut self.metadata } } + +#[cfg(test)] +mod tests { + use stackable_operator::v2::types::operator::RoleName; + use strum::IntoEnumIterator; + + use crate::crd::NifiRole; + + /// Locks the invariant behind the `expect` in the `From for RoleName` impls: + /// every `NifiRole` variant (present and future) must serialise to a valid `RoleName`. + #[test] + fn every_nifi_role_serialises_to_a_valid_role_name() { + for role in NifiRole::iter() { + let _: RoleName = (&role).into(); + let _: RoleName = role.into(); + } + } +} diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs index c5d7950a..498db02e 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -53,9 +53,6 @@ pub enum Error { source: product_image_selection::Error, }, - #[snafu(display("object has no nodes defined"))] - NoNodesDefined, - #[snafu(display("failed to get the cluster name"))] GetClusterName { source: controller_utils::Error }, @@ -145,15 +142,13 @@ pub fn validate( build_role_group_configs(nifi, &image, &vector_aggregator_config_map_name)?; // Per-role config (PDB + listener class), extracted here so downstream builders source it from - // the `ValidatedCluster` rather than the raw `NifiCluster`. The `nodes` role is mandatory - // (already enforced by `build_role_group_configs` above), so this is always present. - let role_config = nifi - .role_config(&NifiRole::Node) - .map(|role_config| ValidatedRoleConfig { - pdb: role_config.common.pod_disruption_budget.clone(), - listener_class: role_config.listener_class.clone(), - }) - .context(NoNodesDefinedSnafu)?; + // the `ValidatedCluster` rather than the raw `NifiCluster`. The `nodes` role is required by + // the CRD, so this is always present. + let node_role_config = nifi.role_config(&NifiRole::Node); + let role_config = ValidatedRoleConfig { + pdb: node_role_config.common.pod_disruption_budget.clone(), + listener_class: node_role_config.listener_class.clone(), + }; let namespace = dereferenced_objects.namespace.clone(); let cluster_domain = dereferenced_objects.cluster_domain.clone(); @@ -199,7 +194,7 @@ pub(crate) fn build_role_group_configs( image: &product_image_selection::ResolvedProductImage, vector_aggregator_config_map_name: &Option, ) -> Result>> { - let role = nifi.spec.nodes.as_ref().context(NoNodesDefinedSnafu)?; + let role = &nifi.spec.nodes; let default_config = NifiConfig::default_config(&nifi.name_any(), &NifiRole::Node); let mut groups: BTreeMap = BTreeMap::new(); @@ -316,9 +311,146 @@ pub(crate) fn test_resolved_product_image() -> product_image_selection::Resolved #[cfg(test)] mod tests { use pretty_assertions::assert_eq; - use stackable_operator::v2::types::kubernetes::ConfigMapName; + use stackable_operator::{ + commons::networking::DomainName, crd::authentication::core as auth_core, + v2::types::kubernetes::ConfigMapName, + }; use super::*; + use crate::{ + controller::build::properties::test_support::app_version_label, + security::{ + authentication::DereferencedAuthenticationClasses, + authorization::DereferencedAuthorization, + }, + }; + + /// Locks every value the validate step itself derives from the minimal fixture — so a + /// validation regression fails here, with a validate-shaped message, instead of surfacing as + /// a confusing build-test failure downstream. + /// + /// The merged per-role-group config (resources, affinity, logging defaults, …) is produced by + /// `with_validated_config` and the config defaults, whose contracts are tested in operator-rs; + /// only the values this module derives on top are re-asserted here. + #[test] + fn validate_ok_derives_expected_values() { + let yaml = r#" + apiVersion: nifi.stackable.tech/v1alpha1 + kind: NifiCluster + metadata: + name: simple-nifi + namespace: default + uid: e6ac237d-a6d4-43a1-8135-f36506110912 + spec: + image: + productVersion: 2.9.0 + clusterConfig: + authentication: + - authenticationClass: nifi-admin-credentials-simple + sensitiveProperties: + keySecret: simple-nifi-sensitive-property-key + autoGenerate: true + nodes: + roleGroups: + default: + replicas: 1 + "#; + let nifi: v1alpha1::NifiCluster = serde_yaml::from_str(yaml).expect("valid test YAML"); + + let auth_class: auth_core::v1alpha1::AuthenticationClass = serde_yaml::from_str( + r#" + metadata: + name: nifi-admin-credentials-simple + spec: + provider: !static + userCredentialsSecret: + name: nifi-admin-credentials-simple + "#, + ) + .expect("valid static AuthenticationClass"); + let auth_entry: auth_core::v1alpha1::ClientAuthenticationDetails = + serde_yaml::from_str("authenticationClass: nifi-admin-credentials-simple") + .expect("valid authentication entry"); + + let dereferenced_objects = DereferencedObjects { + namespace: "default".parse().expect("valid namespace"), + cluster_domain: DomainName::from_str("cluster.local").expect("valid cluster domain"), + authentication_classes: DereferencedAuthenticationClasses::from_entries(vec![( + auth_entry, auth_class, + )]), + authorization: DereferencedAuthorization::without_opa(), + }; + let operator_environment = OperatorEnvironmentOptions { + operator_namespace: "stackable-operators".to_owned(), + operator_service_name: "nifi-operator".to_owned(), + image_repository: "oci.example.org".to_owned(), + }; + + let cluster = validate(&nifi, &dereferenced_objects, &operator_environment) + .expect("the minimal fixture validates"); + + assert_eq!(cluster.name.to_string(), "simple-nifi"); + assert_eq!(cluster.namespace.to_string(), "default"); + assert_eq!( + cluster.uid.to_string(), + "e6ac237d-a6d4-43a1-8135-f36506110912" + ); + assert_eq!(cluster.cluster_domain.to_string(), "cluster.local"); + assert_eq!( + cluster.image.image, + format!("oci.example.org/nifi:{}", app_version_label("2.9.0")) + ); + assert_eq!(cluster.image.product_version, "2.9.0"); + assert_eq!( + cluster.product_version.to_string(), + app_version_label("2.9.0") + ); + + // The role config falls back to its defaults: PDBs enabled, cluster-internal listener. + assert!(cluster.role_config.pdb.enabled); + assert_eq!(cluster.role_config.pdb.max_unavailable, None); + assert_eq!( + cluster.role_config.listener_class.to_string(), + "cluster-internal" + ); + + // SingleUser authentication and authorization, the default (Kubernetes) clustering + // backend, and the default `tls` server SecretClass. + let cluster_config = &cluster.cluster_config; + assert!(matches!( + &cluster_config.authentication, + NifiAuthenticationConfig::SingleUser { provider } + if provider.user_credentials_secret.name == "nifi-admin-credentials-simple" + )); + assert!(matches!( + cluster_config.authorization, + ResolvedNifiAuthorizationConfig::SingleUser + )); + assert!(matches!( + cluster_config.clustering_backend, + v1alpha1::NifiClusteringBackend::Kubernetes {} + )); + assert_eq!(cluster_config.server_tls_secret_class.to_string(), "tls"); + assert_eq!( + cluster_config.sensitive_properties.key_secret.to_string(), + "simple-nifi-sensitive-property-key" + ); + assert!(cluster_config.sensitive_properties.auto_generate); + assert!(cluster_config.extra_volumes.is_empty()); + + // A single `node` role with the single `default` role group; the Vector agent is off. + assert_eq!(cluster.role_group_configs.len(), 1); + let role_groups = &cluster.role_group_configs[&NifiRole::Node]; + let role_group_names: Vec = role_groups.keys().map(ToString::to_string).collect(); + assert_eq!(role_group_names, ["default"]); + let role_group = role_groups + .values() + .next() + .expect("the default role group exists"); + assert_eq!(role_group.replicas, Some(1)); + assert!(!role_group.config.logging.enable_vector_agent); + assert_eq!(role_group.config.logging.vector_container, None); + } /// A NiFi cluster with the Vector agent enabled at the Node role level. const NIFI_VECTOR_ENABLED_YAML: &str = r#" diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index 1d1acf67..e573d8a8 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -36,7 +36,7 @@ use stackable_operator::{ role_utils::JavaCommonConfig, types::{ kubernetes::{ConfigMapName, ListenerClassName, SecretClassName}, - operator::ProductVersion, + operator::{ProductVersion, RoleName}, }, }, versioned::versioned, @@ -79,8 +79,7 @@ pub mod versioned { pub cluster_config: v1alpha1::NifiClusterConfig, // no doc - docs in Role struct. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub nodes: Option, + pub nodes: NifiRoleType, // no doc - docs in ProductImage struct. pub image: ProductImage, @@ -186,9 +185,9 @@ impl HasStatusCondition for v1alpha1::NifiCluster { } impl v1alpha1::NifiCluster { - pub fn role_config(&self, role: &NifiRole) -> Option<&NifiNodeRoleConfig> { + pub fn role_config(&self, role: &NifiRole) -> &NifiNodeRoleConfig { match role { - NifiRole::Node => self.spec.nodes.as_ref().map(|n| &n.role_config), + NifiRole::Node => &self.spec.nodes.role_config, } } @@ -224,7 +223,17 @@ pub fn default_allow_all() -> bool { } #[derive( - Clone, Debug, Deserialize, Eq, JsonSchema, Ord, PartialEq, PartialOrd, Serialize, strum::Display, + Clone, + Debug, + Deserialize, + Eq, + JsonSchema, + Ord, + PartialEq, + PartialOrd, + Serialize, + strum::Display, + strum::EnumIter, )] #[serde(rename_all = "camelCase")] #[strum(serialize_all = "camelCase")] @@ -233,6 +242,18 @@ pub enum NifiRole { Node, } +impl From for RoleName { + fn from(value: NifiRole) -> Self { + RoleName::from_str(&value.to_string()).expect("a NifiRole is a valid role name") + } +} + +impl From<&NifiRole> for RoleName { + fn from(value: &NifiRole) -> Self { + RoleName::from_str(&value.to_string()).expect("a NifiRole is a valid role name") + } +} + #[derive(Clone, Debug, Default, Deserialize, JsonSchema, Serialize)] pub struct NifiStatus { pub deployed_version: Option, diff --git a/rust/operator-binary/src/nifi_controller.rs b/rust/operator-binary/src/nifi_controller.rs index 0cee34c5..88c59a0d 100644 --- a/rust/operator-binary/src/nifi_controller.rs +++ b/rust/operator-binary/src/nifi_controller.rs @@ -8,9 +8,7 @@ use stackable_operator::{ cli::OperatorEnvironmentOptions, client::Client, cluster_resources::ClusterResourceApplyStrategy, - commons::rbac::build_rbac_resources, kube::{ - ResourceExt, core::{DeserializeGuard, error_boundary}, runtime::controller::Action, }, @@ -27,7 +25,7 @@ use strum::{EnumDiscriminants, IntoStaticStr}; use crate::{ OPERATOR_NAME, controller::{build, controller_name, dereference, operator_name, product_name, validate}, - crd::{APP_NAME, NifiStatus, v1alpha1}, + crd::{NifiStatus, v1alpha1}, security::{ authentication::NifiAuthenticationConfig, check_or_generate_oidc_admin_password, check_or_generate_sensitive_key, @@ -75,27 +73,6 @@ pub enum Error { source: stackable_operator::cluster_resources::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 resources"))] - BuildRbacResources { - source: stackable_operator::commons::rbac::Error, - }, - - #[snafu(display("failed to get required labels"))] - GetRequiredLabels { - source: - stackable_operator::kvp::KeyValuePairError, - }, - #[snafu(display("security failure"))] Security { source: crate::security::Error }, } @@ -164,33 +141,25 @@ pub async fn reconcile_nifi( .context(SecuritySnafu)?; } - let (rbac_sa, rbac_rolebinding) = build_rbac_resources( - nifi, - APP_NAME, - cluster_resources - .get_required_labels() - .context(GetRequiredLabelsSnafu)?, - ) - .context(BuildRbacResourcesSnafu)?; - - let rbac_sa = cluster_resources - .add(client, rbac_sa) - .await - .context(ApplyServiceAccountSnafu)?; - - cluster_resources - .add(client, rbac_rolebinding) - .await - .context(ApplyRoleBindingSnafu)?; - - let resources = - build::build(&validated_cluster, &rbac_sa.name_any()).context(BuildResourcesSnafu)?; + let resources = build::build(&validated_cluster).context(BuildResourcesSnafu)?; let mut ss_cond_builder = StatefulSetConditionBuilder::default(); // Apply order: everything before StatefulSets, StatefulSets last. A StatefulSet must be applied // after all ConfigMaps and Secrets it mounts, otherwise the Pods restart unnecessarily. // See https://github.com/stackabletech/commons-operator/issues/111 for details. + for service_account in resources.service_accounts { + cluster_resources + .add(client, service_account) + .await + .context(ApplyResourceSnafu)?; + } + for role_binding in resources.role_bindings { + cluster_resources + .add(client, role_binding) + .await + .context(ApplyResourceSnafu)?; + } for service in resources.services { cluster_resources .add(client, service) diff --git a/rust/operator-binary/src/security/authentication.rs b/rust/operator-binary/src/security/authentication.rs index bc9eeb18..0d561c03 100644 --- a/rust/operator-binary/src/security/authentication.rs +++ b/rust/operator-binary/src/security/authentication.rs @@ -114,6 +114,18 @@ impl DereferencedAuthenticationClasses { Ok(DereferencedAuthenticationClasses { entries }) } + + /// Builds the struct directly from already-fetched entries, for tests that cannot use the + /// client-based [`Self::dereference`]. + #[cfg(test)] + pub(crate) fn from_entries( + entries: Vec<( + auth_core::v1alpha1::ClientAuthenticationDetails, + auth_core::v1alpha1::AuthenticationClass, + )>, + ) -> Self { + Self { entries } + } } #[derive(Clone)] diff --git a/rust/operator-binary/src/security/authorization.rs b/rust/operator-binary/src/security/authorization.rs index 14c6e679..4789a95a 100644 --- a/rust/operator-binary/src/security/authorization.rs +++ b/rust/operator-binary/src/security/authorization.rs @@ -60,6 +60,15 @@ pub struct DereferencedAuthorization { } impl DereferencedAuthorization { + /// A dereferenced authorization with no OPA ConfigMap, for tests that cannot use the + /// client-based [`Self::dereference`]. + #[cfg(test)] + pub(crate) fn without_opa() -> Self { + Self { + opa_config_map: None, + } + } + /// Fetch the OPA ConfigMap referenced by the authorization spec, if applicable. pub async fn dereference( nifi_authorization: &NifiAuthorization,