From 3046476ea2e256dd46f00708428d172d4e84fff3 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Fri, 7 Aug 2026 14:02:18 +0200 Subject: [PATCH 1/8] refactor: Add Prepared type-state marker to KubernetesResources Make KubernetesResources generic over a marker that records whether the resources have only been built or have already been applied, and have build() return KubernetesResources. This prepares the extraction of the apply and update_status steps, where the Applied counterpart lets the type system prove that the cluster status is derived from the resource specifications returned by the API server rather than from the merely built ones. The Applied marker itself is added together with the apply step, since a marker struct that nothing constructs yet fails the dead_code lint. --- rust/operator-binary/src/controller/build.rs | 7 ++++--- rust/operator-binary/src/controller/mod.rs | 13 +++++++++++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/rust/operator-binary/src/controller/build.rs b/rust/operator-binary/src/controller/build.rs index 6376f9df..808d8d1d 100644 --- a/rust/operator-binary/src/controller/build.rs +++ b/rust/operator-binary/src/controller/build.rs @@ -2,7 +2,7 @@ //! //! [`ValidatedCluster`]: crate::controller::ValidatedCluster -use std::str::FromStr; +use std::{marker::PhantomData, str::FromStr}; use snafu::{ResultExt, Snafu}; use stackable_operator::{ @@ -15,7 +15,7 @@ use stackable_operator::{ use crate::{ controller::{ - KubernetesResources, ValidatedCluster, + KubernetesResources, Prepared, ValidatedCluster, build::resource::{ config_map::build_rolegroup_config_map, listener::{build_group_listener, group_listener_name}, @@ -70,7 +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. -pub fn build(cluster: &ValidatedCluster) -> Result { +pub fn build(cluster: &ValidatedCluster) -> Result, Error> { let mut stateful_sets = vec![]; let mut services = vec![]; let mut listeners = vec![]; @@ -122,6 +122,7 @@ pub fn build(cluster: &ValidatedCluster) -> Result { pod_disruption_budgets, service_accounts: vec![build_service_account(cluster)], role_bindings: vec![build_role_binding(cluster)], + status: PhantomData, }) } diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index dc8a01c5..e304c1b4 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -2,7 +2,7 @@ //! [`validate`] step and consumed by the [`build`] steps, plus the //! `dereference` / `validate` / `build` sub-modules. -use std::{collections::BTreeMap, str::FromStr as _}; +use std::{collections::BTreeMap, marker::PhantomData, str::FromStr as _}; use stackable_operator::{ commons::{ @@ -59,8 +59,16 @@ 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"); +/// Marker for prepared Kubernetes resources which are not applied yet. +pub struct Prepared; + /// Every Kubernetes resource produced by the [`build`] step. -pub struct KubernetesResources { +/// +/// `T` is a marker that indicates whether these resources are only [`Prepared`] or already +/// applied. It lets the type system prove that the cluster status is derived from the applied +/// resources (which carry the API server's view, e.g. the StatefulSet status) rather than from the +/// merely built ones. +pub struct KubernetesResources { pub stateful_sets: Vec, pub services: Vec, pub listeners: Vec, @@ -68,6 +76,7 @@ pub struct KubernetesResources { pub pod_disruption_budgets: Vec, pub service_accounts: Vec, pub role_bindings: Vec, + pub status: PhantomData, } /// A validated, merged (default <- role <- role-group) NiFi rolegroup config. From a5ace31efd8d999e011f5bfcb86a9485f19a6777 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Fri, 7 Aug 2026 14:30:03 +0200 Subject: [PATCH 2/8] refactor: Parse the deployed product version in the validate step Add deployed_product_version to ValidatedCluster, holding the bare NiFi version (for example 2.9.0) that is reported as status.deployedVersion. It is deliberately separate from product_version, which carries the full image app version label value (for example 2.9.0-stackable0.0.0-dev) used for the app.kubernetes.io/version label. This also fixes a latent panic. The reconciler parsed the bare product version with expect(), assuming it to be a valid label value. That holds for app_version_label_value, which resolve() truncates to the label value length limit, but not for product_version, which is copied verbatim from user input. Parsing now happens in the validate step and returns a ParseProductVersion error instead of panicking the reconcile task. --- .../src/controller/build/properties.rs | 3 +++ rust/operator-binary/src/controller/mod.rs | 12 ++++++++++- .../src/controller/validate.rs | 20 +++++++++++++++++++ rust/operator-binary/src/nifi_controller.rs | 10 +++------- 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/rust/operator-binary/src/controller/build/properties.rs b/rust/operator-binary/src/controller/build/properties.rs index 760fe651..9cf8179a 100644 --- a/rust/operator-binary/src/controller/build/properties.rs +++ b/rust/operator-binary/src/controller/build/properties.rs @@ -179,6 +179,8 @@ pub(crate) mod test_support { let uid = Uid::from_str("e6ac237d-a6d4-43a1-8135-f36506110912").expect("valid uid"); let product_version = ProductVersion::from_str(&image.app_version_label_value) .expect("valid product version"); + let deployed_product_version = + ProductVersion::from_str(&image.product_version).expect("valid product version"); ValidatedCluster::new( name, @@ -187,6 +189,7 @@ pub(crate) mod test_support { uid, image, product_version, + deployed_product_version, role_config, role_group_configs, ValidatedClusterConfig { diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index e304c1b4..c425f478 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -180,8 +180,16 @@ pub struct ValidatedCluster { /// The product image. pub image: ResolvedProductImage, /// The product version as a type-safe label value, used for the `app.kubernetes.io/version` - /// label on built resources. + /// label on built resources. This is the full image app version (for example + /// `2.9.0-stackable0.0.0-dev`), not the bare NiFi version. pub product_version: ProductVersion, + /// The bare NiFi version (for example `2.9.0`), reported as `status.deployedVersion`. + /// + /// Deliberately separate from [`Self::product_version`]: that one carries the image app + /// version label value, whereas this is the product version the user asked for. The status + /// field is user facing and is asserted bare by the `upgrade` integration test, so the two + /// must not be conflated. + pub deployed_product_version: ProductVersion, /// Per-role configuration (PodDisruptionBudget and listener class). The `nodes` role is /// required by the CRD, so this is always present. pub role_config: ValidatedRoleConfig, @@ -237,6 +245,7 @@ impl ValidatedCluster { uid: Uid, image: ResolvedProductImage, product_version: ProductVersion, + deployed_product_version: ProductVersion, role_config: ValidatedRoleConfig, role_group_configs: BTreeMap>, cluster_config: ValidatedClusterConfig, @@ -256,6 +265,7 @@ impl ValidatedCluster { uid, image, product_version, + deployed_product_version, role_config, role_group_configs, cluster_config, diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs index 498db02e..485c712f 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -91,6 +91,12 @@ pub enum Error { ValidateLoggingConfig { source: stackable_operator::v2::product_logging::framework::Error, }, + + #[snafu(display("the product version {product_version:?} is invalid"))] + ParseProductVersion { + source: stackable_operator::v2::macros::attributed_string_type::Error, + product_version: String, + }, } type Result = std::result::Result; @@ -159,6 +165,16 @@ pub fn validate( let product_version = ProductVersion::from_str(&image.app_version_label_value) .expect("the app version label value is a valid product version"); + // The bare product version, reported as `status.deployedVersion`. Unlike + // `app_version_label_value` this is the user's input copied verbatim (it is never truncated to + // the label value length limit), so it has to be parsed fallibly. + let deployed_product_version = + ProductVersion::from_str(&image.product_version).with_context(|_| { + ParseProductVersionSnafu { + product_version: image.product_version.clone(), + } + })?; + Ok(ValidatedCluster::new( name, namespace, @@ -166,6 +182,7 @@ pub fn validate( uid, image, product_version, + deployed_product_version, role_config, role_group_configs, ValidatedClusterConfig { @@ -401,10 +418,13 @@ mod tests { format!("oci.example.org/nifi:{}", app_version_label("2.9.0")) ); assert_eq!(cluster.image.product_version, "2.9.0"); + // The label value carries the `-stackable` suffix, the version reported + // in `status.deployedVersion` does not. assert_eq!( cluster.product_version.to_string(), app_version_label("2.9.0") ); + assert_eq!(cluster.deployed_product_version.to_string(), "2.9.0"); // The role config falls back to its defaults: PDBs enabled, cluster-internal listener. assert!(cluster.role_config.pdb.enabled); diff --git a/rust/operator-binary/src/nifi_controller.rs b/rust/operator-binary/src/nifi_controller.rs index 88c59a0d..33a8336f 100644 --- a/rust/operator-binary/src/nifi_controller.rs +++ b/rust/operator-binary/src/nifi_controller.rs @@ -1,6 +1,6 @@ //! Ensures that `Pod`s are configured and running for each [`v1alpha1::NifiCluster`]. -use std::{str::FromStr, sync::Arc}; +use std::sync::Arc; use const_format::concatcp; use snafu::{ResultExt, Snafu}; @@ -18,7 +18,7 @@ use stackable_operator::{ compute_conditions, operations::ClusterOperationsConditionBuilder, statefulset::StatefulSetConditionBuilder, }, - v2::{cluster_resources::cluster_resources_new, types::operator::ProductVersion}, + v2::cluster_resources::cluster_resources_new, }; use strum::{EnumDiscriminants, IntoStaticStr}; @@ -108,7 +108,6 @@ pub async fn reconcile_nifi( validate::validate(nifi, &dereferenced_objects, &ctx.operator_environment) .context(ValidateClusterSnafu)?; - let resolved_product_image = &validated_cluster.image; let authentication_config = &validated_cluster.cluster_config.authentication; tracing::info!("Checking for sensitive key configuration"); @@ -209,10 +208,7 @@ pub async fn reconcile_nifi( let conditions = compute_conditions(nifi, &[&ss_cond_builder, &cluster_operation_cond_builder]); let status = NifiStatus { - deployed_version: Some( - ProductVersion::from_str(&resolved_product_image.product_version) - .expect("the resolved product version is a valid product version label value"), - ), + deployed_version: Some(validated_cluster.deployed_product_version.clone()), conditions, }; From 0285d436663321002db180cc38abc4144e2aea48 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Fri, 7 Aug 2026 14:33:47 +0200 Subject: [PATCH 3/8] refactor: Extract the apply step into an Applier Move the resource application out of reconcile_nifi into a dedicated controller/apply.rs, mirroring the airflow and hbase operators. The Applier owns the ClusterResources, applies each resource kind through a single generic helper and returns KubernetesResources, so the seven near identical apply loops collapse into one line each. Deleting orphaned resources moves along with it. Also move the two Secret side effects (the sensitive properties key and the OIDC admin password) into ensure_secrets in the same module. These are read-or-create client operations and are deliberately not tracked in ClusterResources, so they survive orphan deletion and an existing Secret is never overwritten. The reconciler's error enum collapses three variants into a single ApplyResources variant delegating to apply::Error. --- rust/operator-binary/src/controller/apply.rs | 171 +++++++++++++++++++ rust/operator-binary/src/controller/mod.rs | 7 +- rust/operator-binary/src/nifi_controller.rs | 125 +++----------- 3 files changed, 198 insertions(+), 105 deletions(-) create mode 100644 rust/operator-binary/src/controller/apply.rs diff --git a/rust/operator-binary/src/controller/apply.rs b/rust/operator-binary/src/controller/apply.rs new file mode 100644 index 00000000..7f1c962b --- /dev/null +++ b/rust/operator-binary/src/controller/apply.rs @@ -0,0 +1,171 @@ +//! The apply step in the NifiCluster controller. + +use std::marker::PhantomData; + +use snafu::{ResultExt, Snafu}; +use stackable_operator::{ + client::Client, + cluster_resources::{ClusterResource, ClusterResourceApplyStrategy, ClusterResources}, + deep_merger::ObjectOverrides, + v2::cluster_resources::cluster_resources_new, +}; +use strum::{EnumDiscriminants, IntoStaticStr}; + +use crate::{ + controller::{ + Applied, KubernetesResources, Prepared, ValidatedCluster, controller_name, operator_name, + product_name, + }, + security::{ + authentication::NifiAuthenticationConfig, check_or_generate_oidc_admin_password, + check_or_generate_sensitive_key, + }, +}; + +#[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 delete orphaned resources"))] + DeleteOrphanedResources { + source: stackable_operator::cluster_resources::Error, + }, + + #[snafu(display("security failure"))] + Security { source: crate::security::Error }, +} + +type Result = std::result::Result; + +/// Applier for the Kubernetes resource specifications produced by this controller. +/// +/// The implementation is not tied to this controller and could theoretically be moved to +/// stackable_operator if [`KubernetesResources`] would contain all possible resource types. +pub struct Applier<'a> { + client: &'a Client, + cluster_resources: ClusterResources<'a>, +} + +impl<'a> Applier<'a> { + pub fn new( + client: &'a Client, + cluster: &ValidatedCluster, + apply_strategy: ClusterResourceApplyStrategy, + object_overrides: &'a ObjectOverrides, + ) -> Applier<'a> { + let cluster_resources = cluster_resources_new( + &product_name(), + &operator_name(), + &controller_name(), + &cluster.name, + &cluster.namespace, + &cluster.uid, + apply_strategy, + object_overrides, + ); + + Applier { + client, + cluster_resources, + } + } + + /// Applies the given Kubernetes resources and marks them as applied. + pub async fn apply( + mut self, + resources: KubernetesResources, + ) -> Result> { + // Destructured without `..`, so adding a field to [`KubernetesResources`] fails to + // compile here instead of silently never being applied. + let KubernetesResources { + stateful_sets, + services, + listeners, + config_maps, + pod_disruption_budgets, + service_accounts, + role_bindings, + status: _, + } = resources; + + // Apply order is: StatefulSets last (a changed mounted ConfigMap or Secret must exist + // first, else the Pods restart unnecessarily, see commons-operator#111). The ServiceAccount + // comes first because the Pods reference it at creation time. + let service_accounts = self.add_resources(service_accounts).await?; + let role_bindings = self.add_resources(role_bindings).await?; + let services = self.add_resources(services).await?; + let listeners = self.add_resources(listeners).await?; + let config_maps = self.add_resources(config_maps).await?; + let pod_disruption_budgets = self.add_resources(pod_disruption_budgets).await?; + let stateful_sets = self.add_resources(stateful_sets).await?; + + // Remove any orphaned resources that still exist in Kubernetes, but have not been added to + // the cluster resources during this reconciliation. + // TODO: this doesn't cater for a graceful cluster shrink, for that we'd need to predict + // the resources that will be removed and run a disconnect/offload job for those + // see https://github.com/stackabletech/nifi-operator/issues/314 + self.cluster_resources + .delete_orphaned_resources(self.client) + .await + .context(DeleteOrphanedResourcesSnafu)?; + + Ok(KubernetesResources { + stateful_sets, + services, + listeners, + config_maps, + pod_disruption_budgets, + service_accounts, + role_bindings, + status: PhantomData, + }) + } + + async fn add_resources( + &mut self, + resources: Vec, + ) -> Result> { + let mut applied_resources = vec![]; + + for resource in resources { + let applied_resource = self + .cluster_resources + .add(self.client, resource) + .await + .context(ApplyResourceSnafu)?; + applied_resources.push(applied_resource); + } + + Ok(applied_resources) + } +} + +/// Ensures the Secrets that the NiFi Pods mount but that the operator does not own exist, creating +/// any that are missing: the sensitive properties key and (for OIDC authentication) the admin +/// password. +/// +/// These are read-or-create client operations, so they cannot be part of the client-free `build()` +/// step. They are also deliberately not tracked in [`ClusterResources`], so they survive orphan +/// deletion and an existing Secret is never overwritten. +pub async fn ensure_secrets(client: &Client, cluster: &ValidatedCluster) -> Result<()> { + tracing::info!("Checking for sensitive key configuration"); + check_or_generate_sensitive_key( + client, + &cluster.cluster_config.sensitive_properties, + &cluster.namespace, + ) + .await + .context(SecuritySnafu)?; + + if let NifiAuthenticationConfig::Oidc { .. } = cluster.cluster_config.authentication { + check_or_generate_oidc_admin_password(client, &cluster.name, &cluster.namespace) + .await + .context(SecuritySnafu)?; + } + + Ok(()) +} diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index c425f478..263f8111 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -52,6 +52,7 @@ use crate::{ }, }; +pub(crate) mod apply; pub(crate) mod build; pub(crate) mod dereference; pub(crate) mod validate; @@ -62,10 +63,14 @@ stackable_operator::constant!(UNVERSIONED_PRODUCT_VERSION: ProductVersion = "non /// Marker for prepared Kubernetes resources which are not applied yet. pub struct Prepared; +/// Marker for Kubernetes resources which have been applied, i.e. the specifications as returned by +/// the Kubernetes API server. +pub struct Applied; + /// Every Kubernetes resource produced by the [`build`] step. /// /// `T` is a marker that indicates whether these resources are only [`Prepared`] or already -/// applied. It lets the type system prove that the cluster status is derived from the applied +/// [`Applied`]. It lets the type system prove that the cluster status is derived from the applied /// resources (which carry the API server's view, e.g. the StatefulSet status) rather than from the /// merely built ones. pub struct KubernetesResources { diff --git a/rust/operator-binary/src/nifi_controller.rs b/rust/operator-binary/src/nifi_controller.rs index 33a8336f..1d3ac9bf 100644 --- a/rust/operator-binary/src/nifi_controller.rs +++ b/rust/operator-binary/src/nifi_controller.rs @@ -18,18 +18,16 @@ use stackable_operator::{ compute_conditions, operations::ClusterOperationsConditionBuilder, statefulset::StatefulSetConditionBuilder, }, - v2::cluster_resources::cluster_resources_new, }; use strum::{EnumDiscriminants, IntoStaticStr}; use crate::{ OPERATOR_NAME, - controller::{build, controller_name, dereference, operator_name, product_name, validate}, - crd::{NifiStatus, v1alpha1}, - security::{ - authentication::NifiAuthenticationConfig, check_or_generate_oidc_admin_password, - check_or_generate_sensitive_key, + controller::{ + apply::{self, Applier, ensure_secrets}, + build, dereference, validate, }, + crd::{NifiStatus, v1alpha1}, }; pub const NIFI_CONTROLLER_NAME: &str = "nificluster"; @@ -55,11 +53,6 @@ pub enum Error { #[snafu(display("failed to validate cluster"))] ValidateCluster { source: validate::Error }, - #[snafu(display("failed to delete orphaned resources"))] - DeleteOrphanedResources { - source: stackable_operator::cluster_resources::Error, - }, - #[snafu(display("failed to update status"))] StatusUpdate { source: stackable_operator::client::Error, @@ -68,13 +61,8 @@ pub enum Error { #[snafu(display("failed to build the Kubernetes resources"))] BuildResources { source: build::Error }, - #[snafu(display("failed to apply Kubernetes resource"))] - ApplyResource { - source: stackable_operator::cluster_resources::Error, - }, - - #[snafu(display("security failure"))] - Security { source: crate::security::Error }, + #[snafu(display("failed to apply the Kubernetes resources"))] + ApplyResources { source: apply::Error }, } type Result = std::result::Result; @@ -108,99 +96,28 @@ pub async fn reconcile_nifi( validate::validate(nifi, &dereferenced_objects, &ctx.operator_environment) .context(ValidateClusterSnafu)?; - let authentication_config = &validated_cluster.cluster_config.authentication; + // build (no Kubernetes API calls required) + let resources = build::build(&validated_cluster).context(BuildResourcesSnafu)?; - tracing::info!("Checking for sensitive key configuration"); - check_or_generate_sensitive_key( + // apply (client required) + ensure_secrets(client, &validated_cluster) + .await + .context(ApplyResourcesSnafu)?; + + let applied = Applier::new( client, - &validated_cluster.cluster_config.sensitive_properties, - &validated_cluster.namespace, - ) - .await - .context(SecuritySnafu)?; - - let mut cluster_resources = cluster_resources_new( - &product_name(), - &operator_name(), - &controller_name(), - &validated_cluster.name, - &validated_cluster.namespace, - &validated_cluster.uid, + &validated_cluster, ClusterResourceApplyStrategy::from(&nifi.spec.cluster_operation), &nifi.spec.object_overrides, - ); - - if let NifiAuthenticationConfig::Oidc { .. } = authentication_config { - check_or_generate_oidc_admin_password( - client, - &validated_cluster.name, - &validated_cluster.namespace, - ) - .await - .context(SecuritySnafu)?; - } - - let resources = build::build(&validated_cluster).context(BuildResourcesSnafu)?; + ) + .apply(resources) + .await + .context(ApplyResourcesSnafu)?; 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) - .await - .context(ApplyResourceSnafu)?; + for stateful_set in &applied.stateful_sets { + ss_cond_builder.add(stateful_set.clone()); } - for listener in resources.listeners { - cluster_resources - .add(client, listener) - .await - .context(ApplyResourceSnafu)?; - } - for config_map in resources.config_maps { - cluster_resources - .add(client, config_map) - .await - .context(ApplyResourceSnafu)?; - } - for pdb in resources.pod_disruption_budgets { - cluster_resources - .add(client, pdb) - .await - .context(ApplyResourceSnafu)?; - } - for stateful_set in resources.stateful_sets { - ss_cond_builder.add( - cluster_resources - .add(client, stateful_set) - .await - .context(ApplyResourceSnafu)?, - ); - } - - // Remove any orphaned resources that still exist in k8s, but have not been added to - // the cluster resources during the reconciliation - // TODO: this doesn't cater for a graceful cluster shrink, for that we'd need to predict - // the resources that will be removed and run a disconnect/offload job for those - // see https://github.com/stackabletech/nifi-operator/issues/314 - cluster_resources - .delete_orphaned_resources(client) - .await - .context(DeleteOrphanedResourcesSnafu)?; let cluster_operation_cond_builder = ClusterOperationsConditionBuilder::new(&nifi.spec.cluster_operation); From 2791cd89e28ca5efd89a356b5567806eb5dec997 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Fri, 7 Aug 2026 14:37:19 +0200 Subject: [PATCH 4/8] refactor: Extract the update_status step Move the cluster status handling out of reconcile_nifi into a dedicated controller/update_status.rs, mirroring the airflow and hbase operators. The StatefulSet and cluster operation condition builders, computing the conditions and patching the status all live there now. update_status takes KubernetesResources, so the type system proves the conditions derive from the resource specifications returned by the API server rather than from the merely built ones. Unlike the sibling operators it also takes the ValidatedCluster, because nifi additionally reports the deployed product version. reconcile_nifi is now a flat dereference -> validate -> build -> apply -> update_status pipeline, and its error enum delegates to the per step errors throughout. --- rust/operator-binary/src/controller/mod.rs | 1 + .../src/controller/update_status.rs | 61 +++++++++++++++++++ rust/operator-binary/src/nifi_controller.rs | 45 +++++--------- 3 files changed, 77 insertions(+), 30 deletions(-) create mode 100644 rust/operator-binary/src/controller/update_status.rs diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index 263f8111..bcf73c8e 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -55,6 +55,7 @@ use crate::{ pub(crate) mod apply; pub(crate) mod build; pub(crate) mod dereference; +pub(crate) mod update_status; pub(crate) mod validate; // Placeholder version label value for resources whose labels must not change after deployment. diff --git a/rust/operator-binary/src/controller/update_status.rs b/rust/operator-binary/src/controller/update_status.rs new file mode 100644 index 00000000..01243da3 --- /dev/null +++ b/rust/operator-binary/src/controller/update_status.rs @@ -0,0 +1,61 @@ +//! The update_status step in the NifiCluster controller. + +use snafu::{ResultExt, Snafu}; +use stackable_operator::{ + client::Client, + status::condition::{ + compute_conditions, operations::ClusterOperationsConditionBuilder, + statefulset::StatefulSetConditionBuilder, + }, +}; +use strum::{EnumDiscriminants, IntoStaticStr}; + +use crate::{ + OPERATOR_NAME, + controller::{Applied, KubernetesResources, ValidatedCluster}, + crd::{NifiStatus, v1alpha1}, +}; + +#[derive(Snafu, Debug, EnumDiscriminants)] +#[strum_discriminants(derive(IntoStaticStr))] +pub enum Error { + #[snafu(display("failed to update status"))] + ApplyStatus { + source: stackable_operator::client::Error, + }, +} + +type Result = std::result::Result; + +/// Computes the cluster status from the applied resources and patches it onto the +/// [`v1alpha1::NifiCluster`]. Takes [`KubernetesResources`] so the type system proves the +/// status derives from applied resources, not merely built ones. +/// +/// Unlike the sibling operators this also reports the deployed product version, which is why it +/// takes the [`ValidatedCluster`] as well. +pub async fn update_status( + client: &Client, + nifi: &v1alpha1::NifiCluster, + cluster: &ValidatedCluster, + applied: &KubernetesResources, +) -> Result<()> { + let mut ss_cond_builder = StatefulSetConditionBuilder::default(); + for stateful_set in &applied.stateful_sets { + ss_cond_builder.add(stateful_set.clone()); + } + + let cluster_operation_cond_builder = + ClusterOperationsConditionBuilder::new(&nifi.spec.cluster_operation); + + let status = NifiStatus { + deployed_version: Some(cluster.deployed_product_version.clone()), + conditions: compute_conditions(nifi, &[&ss_cond_builder, &cluster_operation_cond_builder]), + }; + + client + .apply_patch_status(OPERATOR_NAME, nifi, &status) + .await + .context(ApplyStatusSnafu)?; + + Ok(()) +} diff --git a/rust/operator-binary/src/nifi_controller.rs b/rust/operator-binary/src/nifi_controller.rs index 1d3ac9bf..f5c3a8c9 100644 --- a/rust/operator-binary/src/nifi_controller.rs +++ b/rust/operator-binary/src/nifi_controller.rs @@ -1,4 +1,9 @@ //! Ensures that `Pod`s are configured and running for each [`v1alpha1::NifiCluster`]. +//! +//! This is the controller driver: it runs the +//! `dereference -> validate -> build -> apply -> update_status` pipeline. The validated cluster +//! type and the resource builders live under the [`crate::controller`] module tree; this file is +//! kept next to `main.rs` for consistency with the other Stackable operators. use std::sync::Arc; @@ -14,10 +19,6 @@ use stackable_operator::{ }, logging::controller::ReconcilerError, shared::time::Duration, - status::condition::{ - compute_conditions, operations::ClusterOperationsConditionBuilder, - statefulset::StatefulSetConditionBuilder, - }, }; use strum::{EnumDiscriminants, IntoStaticStr}; @@ -25,9 +26,11 @@ use crate::{ OPERATOR_NAME, controller::{ apply::{self, Applier, ensure_secrets}, - build, dereference, validate, + build, dereference, + update_status::{self, update_status}, + validate, }, - crd::{NifiStatus, v1alpha1}, + crd::v1alpha1, }; pub const NIFI_CONTROLLER_NAME: &str = "nificluster"; @@ -40,7 +43,6 @@ pub struct Ctx { #[derive(Snafu, Debug, EnumDiscriminants)] #[strum_discriminants(derive(IntoStaticStr))] -#[allow(clippy::enum_variant_names)] pub enum Error { #[snafu(display("NifiCluster object is invalid"))] InvalidNifiCluster { @@ -53,16 +55,14 @@ pub enum Error { #[snafu(display("failed to validate cluster"))] ValidateCluster { source: validate::Error }, - #[snafu(display("failed to update status"))] - StatusUpdate { - source: stackable_operator::client::Error, - }, - #[snafu(display("failed to build the Kubernetes resources"))] BuildResources { source: build::Error }, #[snafu(display("failed to apply the Kubernetes resources"))] ApplyResources { source: apply::Error }, + + #[snafu(display("failed to update the cluster status"))] + UpdateStatus { source: update_status::Error }, } type Result = std::result::Result; @@ -114,25 +114,10 @@ pub async fn reconcile_nifi( .await .context(ApplyResourcesSnafu)?; - let mut ss_cond_builder = StatefulSetConditionBuilder::default(); - for stateful_set in &applied.stateful_sets { - ss_cond_builder.add(stateful_set.clone()); - } - - let cluster_operation_cond_builder = - ClusterOperationsConditionBuilder::new(&nifi.spec.cluster_operation); - - let conditions = compute_conditions(nifi, &[&ss_cond_builder, &cluster_operation_cond_builder]); - - let status = NifiStatus { - deployed_version: Some(validated_cluster.deployed_product_version.clone()), - conditions, - }; - - client - .apply_patch_status(OPERATOR_NAME, nifi, &status) + // update status (client required) + update_status(client, nifi, &validated_cluster, &applied) .await - .context(StatusUpdateSnafu)?; + .context(UpdateStatusSnafu)?; Ok(Action::await_change()) } From f5c1e91c7c0eb9116e75e4fab03928baa9f1a468 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Fri, 7 Aug 2026 14:44:17 +0200 Subject: [PATCH 5/8] refactor: Pass recommended labels into object_meta Change object_meta to take the recommended Labels instead of a RoleGroupName it derives them from, matching the hbase operator. The call sites now pick the label variant they need, so resources that are not tied to a role group (or that must keep stable labels across version upgrades) can use the same helper instead of assembling their own metadata chain. No functional change, the labels are identical. --- rust/operator-binary/src/controller/build.rs | 18 ++++++++++-------- .../controller/build/resource/config_map.rs | 2 +- .../src/controller/build/resource/listener.rs | 2 +- .../src/controller/build/resource/service.rs | 4 ++-- .../controller/build/resource/statefulset.rs | 2 +- 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/rust/operator-binary/src/controller/build.rs b/rust/operator-binary/src/controller/build.rs index 808d8d1d..fa129a37 100644 --- a/rust/operator-binary/src/controller/build.rs +++ b/rust/operator-binary/src/controller/build.rs @@ -7,6 +7,7 @@ use std::{marker::PhantomData, str::FromStr}; use snafu::{ResultExt, Snafu}; use stackable_operator::{ builder::meta::ObjectMetaBuilder, + kvp::Labels, v2::{ builder::meta::ownerreference_from_resource, types::{common::Port, operator::RoleGroupName}, @@ -126,25 +127,26 @@ pub fn build(cluster: &ValidatedCluster) -> Result }) } -/// 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`. +/// Returns an [`ObjectMetaBuilder`] pre-filled with the cluster's namespace, an owner reference +/// back to the cluster, the resource `name` and the given `recommended_labels`. /// /// 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. +/// need extra labels/annotations chain them onto the returned builder. The labels are passed in +/// rather than derived here, so callers can pick the variant they need: role-level resources +/// (e.g. the per-role [`Listener`](stackable_operator::crd::listener::v1alpha1::Listener)) use the +/// placeholder role group `none`, and resources that must not change after deployment use the +/// unversioned labels. pub(crate) fn object_meta( cluster: &ValidatedCluster, name: impl Into, - role_group_name: &RoleGroupName, + recommended_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(cluster.recommended_labels(role_group_name)); + .with_labels(recommended_labels); builder } 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 7f4cc896..86d96da9 100644 --- a/rust/operator-binary/src/controller/build/resource/config_map.rs +++ b/rust/operator-binary/src/controller/build/resource/config_map.rs @@ -73,7 +73,7 @@ pub fn build_rolegroup_config_map( .role_group_resource_names(role_group_name) .role_group_config_map() .to_string(), - role_group_name, + cluster.recommended_labels(role_group_name), ) .build(), ) diff --git a/rust/operator-binary/src/controller/build/resource/listener.rs b/rust/operator-binary/src/controller/build/resource/listener.rs index 11151b7c..38d91ad2 100644 --- a/rust/operator-binary/src/controller/build/resource/listener.rs +++ b/rust/operator-binary/src/controller/build/resource/listener.rs @@ -32,7 +32,7 @@ pub fn build_group_listener( metadata: object_meta( cluster, listener_group_name.to_string(), - &PLACEHOLDER_LISTENER_ROLE_GROUP, + cluster.recommended_labels(&PLACEHOLDER_LISTENER_ROLE_GROUP), ) .build(), spec: ListenerSpec { diff --git a/rust/operator-binary/src/controller/build/resource/service.rs b/rust/operator-binary/src/controller/build/resource/service.rs index 0aeab6d9..f65a23f8 100644 --- a/rust/operator-binary/src/controller/build/resource/service.rs +++ b/rust/operator-binary/src/controller/build/resource/service.rs @@ -25,7 +25,7 @@ pub fn build_rolegroup_headless_service( .role_group_resource_names(role_group_name) .headless_service_name() .to_string(), - role_group_name, + cluster.recommended_labels(role_group_name), ) .build(), spec: Some(ServiceSpec { @@ -53,7 +53,7 @@ pub fn build_rolegroup_metrics_service( .role_group_resource_names(role_group_name) .metrics_service_name() .to_string(), - role_group_name, + cluster.recommended_labels(role_group_name), ) .with_labels(service::prometheus_labels(&Scraping::Enabled)) .with_annotations(prometheus_annotations()) diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs index 98df6735..87f9c9f0 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -658,7 +658,7 @@ pub(crate) fn build_node_rolegroup_statefulset( metadata: object_meta( cluster, resource_names.stateful_set_name().to_string(), - role_group_name, + cluster.recommended_labels(role_group_name), ) .with_label(RESTART_CONTROLLER_ENABLED_LABEL.to_owned()) .build(), From 8972dc0f849edcd8c4791edaf82b5b3ac4623f62 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Fri, 7 Aug 2026 15:02:44 +0200 Subject: [PATCH 6/8] docs: Correct the default sensitive properties algorithm The CRD documentation claimed nifiPbkdf2AesGcm256 is the default, but the Default derive on NifiSensitiveKeyAlgorithm selects nifiArgon2AesGcm256, which is what validate falls back to when the field is left out. The security usage guide already documents Argon2 as the deployed default, so only the CRD field description was wrong. Regenerated extra/crds.yaml. --- extra/crds.yaml | 4 ++-- rust/operator-binary/src/crd/sensitive_properties.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/extra/crds.yaml b/extra/crds.yaml index 8d9b8e7e..fd5cd7d8 100644 --- a/extra/crds.yaml +++ b/extra/crds.yaml @@ -317,8 +317,8 @@ spec: This setting configures the encryption algorithm to use to encrypt sensitive properties. Valid values are: - `nifiPbkdf2AesGcm256` (the default value), - `nifiArgon2AesGcm256`, + `nifiArgon2AesGcm256` (the default value), + `nifiPbkdf2AesGcm256`, Learn more about the specifics of the algorithm parameters in the [NiFi documentation](https://nifi.apache.org/docs/nifi-docs/html/administration-guide.html#property-encryption-algorithms). diff --git a/rust/operator-binary/src/crd/sensitive_properties.rs b/rust/operator-binary/src/crd/sensitive_properties.rs index f72aed77..6e43bcc7 100644 --- a/rust/operator-binary/src/crd/sensitive_properties.rs +++ b/rust/operator-binary/src/crd/sensitive_properties.rs @@ -25,8 +25,8 @@ pub struct NifiSensitivePropertiesConfig { /// This setting configures the encryption algorithm to use to encrypt sensitive properties. /// Valid values are: /// - /// `nifiPbkdf2AesGcm256` (the default value), - /// `nifiArgon2AesGcm256`, + /// `nifiArgon2AesGcm256` (the default value), + /// `nifiPbkdf2AesGcm256`, /// /// Learn more about the specifics of the algorithm parameters in the /// [NiFi documentation](https://nifi.apache.org/docs/nifi-docs/html/administration-guide.html#property-encryption-algorithms). From 2b1f587737c7db88990efbbf60333582f4080237 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Fri, 7 Aug 2026 15:35:09 +0200 Subject: [PATCH 7/8] chore: adapt changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3814242e..da5d9d9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,10 +13,13 @@ All notable changes to this project will be documented in this file. 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]). +- The reconciler now applies resources and derives the cluster status in discrete + apply and update_status steps for the `nifi_controller` ([#974]). [#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 +[#974]: https://github.com/stackabletech/nifi-operator/pull/974 ## [26.7.0] - 2026-07-21 From 8547ce59ab2b0050f0ca29d1ded2c1ec4d1d3239 Mon Sep 17 00:00:00 2001 From: Malte Sander Date: Tue, 11 Aug 2026 16:23:06 +0200 Subject: [PATCH 8/8] refactor: fold the operator-generated Secrets into the reconciliation pipeline The sensitive-properties key and the OIDC admin password Secret were created by a read-or-create side effect (ensure_secrets) that ran before the apply step and was deliberately untracked, so it needed special-casing around orphan deletion. Both are now dereferenced, built and applied like every other cluster resource. Their contents are randomly generated and can therefore not be rebuilt, so an existing Secret is re-emitted with its fetched contents unchanged, which makes applying it a no-op and never rotates the key. The Secrets deliberately keep carrying no owner reference: the sensitive-properties key still decrypts the persisted flow after the cluster is recreated, and it may have been created by the user. For the same reason a user-provided key Secret (autoGenerate unset) is only required to exist and is never emitted, so the operator never writes to it. This empties out three modules, which are retired along the way: - security/sensitive_key.rs held nothing but two constants, which move to the builders that use them (the Secret key name to the Secret builder, the mount path to the shared build paths). - security/oidc.rs is split by concern: the nifi.properties fragment moves into the nifi.properties builder, the admin password Secret name next to the builder that emits that Secret. - security/mod.rs loses the build_tls_volume pass-through and the error-wrapping enum it existed for; the StatefulSet builder now calls tls::build_tls_volume directly and reports the actual failure. The operator now needs the patch permission on secrets. --- CHANGELOG.md | 5 + .../templates/clusterrole-operator.yaml | 5 +- rust/operator-binary/src/controller/apply.rs | 44 +- rust/operator-binary/src/controller/build.rs | 14 + .../src/controller/build/properties.rs | 5 +- .../build/properties/nifi_properties.rs | 142 +++++- .../src/controller/build/resource/mod.rs | 1 + .../src/controller/build/resource/rbac.rs | 24 +- .../src/controller/build/resource/secret.rs | 418 ++++++++++++++++++ .../controller/build/resource/statefulset.rs | 14 +- .../src/controller/dereference.rs | 64 ++- rust/operator-binary/src/controller/mod.rs | 26 +- .../src/controller/validate.rs | 7 +- rust/operator-binary/src/nifi_controller.rs | 6 +- .../src/security/authentication.rs | 4 +- rust/operator-binary/src/security/mod.rs | 72 +-- rust/operator-binary/src/security/oidc.rs | 220 --------- .../src/security/sensitive_key.rs | 78 ---- 18 files changed, 697 insertions(+), 452 deletions(-) create mode 100644 rust/operator-binary/src/controller/build/resource/secret.rs delete mode 100644 rust/operator-binary/src/security/oidc.rs delete mode 100644 rust/operator-binary/src/security/sensitive_key.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 4cb7018f..5d04c514 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,11 @@ All notable changes to this project will be documented in this file. previously accepted by the API server but failed reconciliation ([#966]). - The reconciler now applies resources and derives the cluster status in discrete apply and update_status steps for the `nifi_controller` ([#974]). +- The sensitive properties key Secret and (for OIDC authentication) the admin password Secret are + now dereferenced, built and applied like every other resource, instead of being created + out-of-band before the apply step. An existing Secret is re-emitted with its contents unchanged, + so applying it is a no-op and the contents are never rotated. The operator therefore now needs + the `patch` permission on `secrets` ([#974]). - All product containers now run with `securityContext.runAsNonRoot` set to `true` to improve security ([#975]). ### Fixed diff --git a/deploy/helm/nifi-operator/templates/clusterrole-operator.yaml b/deploy/helm/nifi-operator/templates/clusterrole-operator.yaml index 845b4ed5..fa4777b2 100644 --- a/deploy/helm/nifi-operator/templates/clusterrole-operator.yaml +++ b/deploy/helm/nifi-operator/templates/clusterrole-operator.yaml @@ -39,7 +39,9 @@ rules: - get - list - patch - # Sensitive properties key and (when OIDC) admin password secret. + # Sensitive properties key and (when OIDC) admin password Secret. Applied via SSA like every + # other resource, but deliberately not owned by the NifiCluster, so they are never orphan-deleted + # (which is also why no `list` is needed here). - apiGroups: - "" resources: @@ -47,6 +49,7 @@ rules: verbs: - get - create + - patch # RoleBinding created per NifiCluster to bind the product ClusterRole to the workload # ServiceAccount. Applied via SSA and tracked for orphan cleanup. - apiGroups: diff --git a/rust/operator-binary/src/controller/apply.rs b/rust/operator-binary/src/controller/apply.rs index 7f1c962b..065af25d 100644 --- a/rust/operator-binary/src/controller/apply.rs +++ b/rust/operator-binary/src/controller/apply.rs @@ -11,15 +11,9 @@ use stackable_operator::{ }; use strum::{EnumDiscriminants, IntoStaticStr}; -use crate::{ - controller::{ - Applied, KubernetesResources, Prepared, ValidatedCluster, controller_name, operator_name, - product_name, - }, - security::{ - authentication::NifiAuthenticationConfig, check_or_generate_oidc_admin_password, - check_or_generate_sensitive_key, - }, +use crate::controller::{ + Applied, KubernetesResources, Prepared, ValidatedCluster, controller_name, operator_name, + product_name, }; #[derive(Snafu, Debug, EnumDiscriminants)] @@ -34,9 +28,6 @@ pub enum Error { DeleteOrphanedResources { source: stackable_operator::cluster_resources::Error, }, - - #[snafu(display("security failure"))] - Security { source: crate::security::Error }, } type Result = std::result::Result; @@ -86,6 +77,7 @@ impl<'a> Applier<'a> { services, listeners, config_maps, + secrets, pod_disruption_budgets, service_accounts, role_bindings, @@ -100,6 +92,7 @@ impl<'a> Applier<'a> { let services = self.add_resources(services).await?; let listeners = self.add_resources(listeners).await?; let config_maps = self.add_resources(config_maps).await?; + let secrets = self.add_resources(secrets).await?; let pod_disruption_budgets = self.add_resources(pod_disruption_budgets).await?; let stateful_sets = self.add_resources(stateful_sets).await?; @@ -118,6 +111,7 @@ impl<'a> Applier<'a> { services, listeners, config_maps, + secrets, pod_disruption_budgets, service_accounts, role_bindings, @@ -143,29 +137,3 @@ impl<'a> Applier<'a> { Ok(applied_resources) } } - -/// Ensures the Secrets that the NiFi Pods mount but that the operator does not own exist, creating -/// any that are missing: the sensitive properties key and (for OIDC authentication) the admin -/// password. -/// -/// These are read-or-create client operations, so they cannot be part of the client-free `build()` -/// step. They are also deliberately not tracked in [`ClusterResources`], so they survive orphan -/// deletion and an existing Secret is never overwritten. -pub async fn ensure_secrets(client: &Client, cluster: &ValidatedCluster) -> Result<()> { - tracing::info!("Checking for sensitive key configuration"); - check_or_generate_sensitive_key( - client, - &cluster.cluster_config.sensitive_properties, - &cluster.namespace, - ) - .await - .context(SecuritySnafu)?; - - if let NifiAuthenticationConfig::Oidc { .. } = cluster.cluster_config.authentication { - check_or_generate_oidc_admin_password(client, &cluster.name, &cluster.namespace) - .await - .context(SecuritySnafu)?; - } - - Ok(()) -} diff --git a/rust/operator-binary/src/controller/build.rs b/rust/operator-binary/src/controller/build.rs index fa129a37..29a1b15b 100644 --- a/rust/operator-binary/src/controller/build.rs +++ b/rust/operator-binary/src/controller/build.rs @@ -22,6 +22,7 @@ use crate::{ listener::{build_group_listener, group_listener_name}, pdb::build_pdb, rbac::{build_role_binding, build_service_account}, + secret::build_secrets, service::{build_rolegroup_headless_service, build_rolegroup_metrics_service}, statefulset::build_node_rolegroup_statefulset, }, @@ -50,6 +51,9 @@ pub const BALANCE_PORT: Port = Port(6243); // Filesystem paths shared by multiple builders. Single-consumer paths live in their builder. pub const NIFI_CONFIG_DIRECTORY: &str = "/stackable/nifi/conf"; pub const NIFI_PYTHON_WORKING_DIRECTORY: &str = "/nifi-python-working-directory"; +/// Mount path of the sensitive-properties key Secret, whose contents are keyed by +/// [`SENSITIVE_PROPERTY_KEY_NAME`](resource::secret::SENSITIVE_PROPERTY_KEY_NAME). +pub const SENSITIVE_PROPERTY_VOLUME_MOUNT: &str = "/stackable/sensitiveproperty"; #[derive(Snafu, Debug)] pub enum Error { @@ -64,6 +68,9 @@ pub enum Error { source: resource::statefulset::Error, role_group: RoleGroupName, }, + + #[snafu(display("failed to build the Secrets"))] + Secrets { source: resource::secret::Error }, } /// Builds every Kubernetes resource for the given validated cluster. @@ -120,6 +127,7 @@ pub fn build(cluster: &ValidatedCluster) -> Result services, listeners, config_maps, + secrets: build_secrets(cluster).context(SecretsSnafu)?, pod_disruption_budgets, service_accounts: vec![build_service_account(cluster)], role_bindings: vec![build_role_binding(cluster)], @@ -187,6 +195,12 @@ mod tests { sorted_names(&resources.pod_disruption_budgets), ["simple-nifi-node"] ); + // The sensitive-properties key Secret, generated because the fixture has none yet. The + // OIDC admin password Secret is absent because the fixture uses SingleUser authentication. + assert_eq!( + sorted_names(&resources.secrets), + ["simple-nifi-sensitive-property-key"] + ); // The cluster-shared RBAC pair. assert_eq!( sorted_names(&resources.service_accounts), diff --git a/rust/operator-binary/src/controller/build/properties.rs b/rust/operator-binary/src/controller/build/properties.rs index 9cf8179a..0156e149 100644 --- a/rust/operator-binary/src/controller/build/properties.rs +++ b/rust/operator-binary/src/controller/build/properties.rs @@ -95,7 +95,8 @@ pub(crate) mod test_support { use crate::{ controller::{ NifiRoleGroupConfig, ValidatedCluster, ValidatedClusterConfig, ValidatedRoleConfig, - ValidatedSensitiveProperties, validate::build_role_group_configs, + ValidatedSensitiveProperties, dereference::ExistingSecrets, + validate::build_role_group_configs, }, crd::{NifiRole, v1alpha1}, security::{ @@ -216,6 +217,8 @@ pub(crate) mod test_support { extra_volumes: nifi.spec.cluster_config.extra_volumes.clone(), host_header_check: nifi.spec.cluster_config.host_header_check.clone(), }, + // As on the first reconcile run: neither Secret exists yet. + ExistingSecrets::default(), ) } diff --git a/rust/operator-binary/src/controller/build/properties/nifi_properties.rs b/rust/operator-binary/src/controller/build/properties/nifi_properties.rs index 283d6f10..32b9cabc 100644 --- a/rust/operator-binary/src/controller/build/properties/nifi_properties.rs +++ b/rust/operator-binary/src/controller/build/properties/nifi_properties.rs @@ -4,6 +4,8 @@ use std::collections::BTreeMap; use snafu::{ResultExt, Snafu}; use stackable_operator::{ + commons::tls_verification::{CaCert, TlsServerVerification, TlsVerification}, + crd::authentication::oidc, memory::MemoryQuantity, role_utils::{ZeroReplicasCounting, fixed_replica_count}, }; @@ -19,18 +21,18 @@ use crate::{ NifiRoleGroupConfig, ValidatedCluster, build::{ HTTPS_PORT, NIFI_CONFIG_DIRECTORY, NIFI_PYTHON_WORKING_DIRECTORY, PROTOCOL_PORT, - resource::statefulset::{ - NODE_ADDRESS_ENV, STACKLET_NAME_ENV, ZOOKEEPER_CHROOT_ENV, ZOOKEEPER_HOSTS_ENV, + SENSITIVE_PROPERTY_VOLUME_MOUNT, + resource::{ + secret::SENSITIVE_PROPERTY_KEY_NAME, + statefulset::{ + NODE_ADDRESS_ENV, STACKLET_NAME_ENV, ZOOKEEPER_CHROOT_ENV, ZOOKEEPER_HOSTS_ENV, + }, }, }, }, crd::{NifiRole, storage::NifiRepository, v1alpha1}, - security::{ - authentication::{ - NifiAuthenticationConfig, STACKABLE_SERVER_TLS_DIR, STACKABLE_TLS_STORE_PASSWORD, - }, - oidc::add_oidc_config_to_properties, - sensitive_key::{SENSITIVE_PROPERTY_KEY_NAME, SENSITIVE_PROPERTY_VOLUME_MOUNT}, + security::authentication::{ + NifiAuthenticationConfig, STACKABLE_SERVER_TLS_DIR, STACKABLE_TLS_STORE_PASSWORD, }, }; @@ -47,10 +49,13 @@ pub enum Error { repo: NifiRepository, }, - #[snafu(display("failed to generate OIDC config"))] - GenerateOidcConfig { - source: crate::security::oidc::Error, + #[snafu(display("invalid well-known OIDC configuration URL"))] + InvalidWellKnownConfigUrl { + source: stackable_operator::crd::authentication::oidc::v1alpha1::Error, }, + + #[snafu(display("Nifi doesn't support skipping the OIDC TLS verification"))] + SkippingTlsVerificationNotSupported {}, } /// NiFi Python (`nipy`) extension directories, mounted only by the `nifi.properties` builder. @@ -488,8 +493,7 @@ pub fn build( ); if let NifiAuthenticationConfig::Oidc { provider, oidc, .. } = auth_config { - add_oidc_config_to_properties(provider, oidc, &mut properties) - .context(GenerateOidcConfigSnafu)?; + add_oidc_config_to_properties(provider, oidc, &mut properties)?; }; // cluster node properties (only configure for cluster nodes) @@ -610,6 +614,61 @@ pub fn build( Ok(format_properties(properties)) } +/// Adds all the required configuration properties to enable OIDC authentication. +fn add_oidc_config_to_properties( + provider: &oidc::v1alpha1::AuthenticationProvider, + client_auth_options: &oidc::v1alpha1::ClientAuthenticationOptions, + properties: &mut BTreeMap, +) -> Result<(), Error> { + let well_known_url = provider + .well_known_config_url() + .context(InvalidWellKnownConfigUrlSnafu)?; + + properties.insert( + "nifi.security.user.oidc.discovery.url".to_string(), + well_known_url.to_string(), + ); + let (oidc_client_id_env, oidc_client_secret_env) = + oidc::v1alpha1::AuthenticationProvider::client_credentials_env_names( + &client_auth_options.client_credentials_secret_ref, + ); + properties.insert( + "nifi.security.user.oidc.client.id".to_string(), + format!("${{env:{oidc_client_id_env}}}").to_string(), + ); + properties.insert( + "nifi.security.user.oidc.client.secret".to_string(), + format!("${{env:{oidc_client_secret_env}}}").to_string(), + ); + let scopes = provider.scopes.join(","); + properties.insert( + "nifi.security.user.oidc.additional.scopes".to_string(), + scopes.to_string(), + ); + properties.insert( + "nifi.security.user.oidc.claim.identifying.user".to_string(), + provider.principal_claim.to_string(), + ); + + if let Some(tls) = &provider.tls.tls { + let truststore_strategy = match tls.verification { + TlsVerification::None {} => SkippingTlsVerificationNotSupportedSnafu.fail()?, + TlsVerification::Server(TlsServerVerification { + ca_cert: CaCert::SecretClass(_), + }) => "NIFI", // The cert get's added to the stackable truststore + TlsVerification::Server(TlsServerVerification { + ca_cert: CaCert::WebPki {}, + }) => "JDK", // The cert needs to be in the system truststore + }; + properties.insert( + "nifi.security.user.oidc.truststore.strategy".to_owned(), + truststore_strategy.to_owned(), + ); + } + + Ok(()) +} + fn storage_quantity_to_nifi(quantity: MemoryQuantity) -> String { format!( "{}MB", @@ -621,12 +680,69 @@ fn storage_quantity_to_nifi(quantity: MemoryQuantity) -> String { #[cfg(test)] mod tests { + use rstest::rstest; + use stackable_operator::commons::tls_verification::{Tls, TlsClientDetails}; + use super::*; use crate::controller::build::{ HTTPS_PORT, properties::test_support::{default_rg, minimal_validated_cluster}, }; + #[rstest] + #[case("/realms/sdp")] + #[case("/realms/sdp/")] + #[case("/realms/sdp/////")] + fn test_add_oidc_config(#[case] root_path: String) { + let mut properties = BTreeMap::new(); + let provider = oidc::v1alpha1::AuthenticationProvider::new( + "keycloak.mycorp.org".to_owned().try_into().unwrap(), + Some(443), + root_path, + TlsClientDetails { + tls: Some(Tls { + verification: TlsVerification::Server(TlsServerVerification { + ca_cert: CaCert::WebPki {}, + }), + }), + }, + "preferred_username".to_owned(), + vec!["openid".to_owned()], + None, + ); + let oidc = oidc::v1alpha1::ClientAuthenticationOptions { + client_credentials_secret_ref: "nifi-keycloak-client".to_owned(), + extra_scopes: vec![], + product_specific_fields: (), + }; + + add_oidc_config_to_properties(&provider, &oidc, &mut properties) + .expect("OIDC config adding failed"); + + assert_eq!( + properties.get("nifi.security.user.oidc.additional.scopes"), + Some(&"openid".to_owned()) + ); + assert_eq!( + properties.get("nifi.security.user.oidc.claim.identifying.user"), + Some(&"preferred_username".to_owned()) + ); + assert_eq!( + properties.get("nifi.security.user.oidc.discovery.url"), + Some( + &"https://keycloak.mycorp.org/realms/sdp/.well-known/openid-configuration" + .to_owned() + ) + ); + assert_eq!( + properties.get("nifi.security.user.oidc.truststore.strategy"), + Some(&"JDK".to_owned()) + ); + + assert!(properties.contains_key("nifi.security.user.oidc.client.id")); + assert!(properties.contains_key("nifi.security.user.oidc.client.secret")); + } + /// Verify that core stable keys are present in the rendered nifi.properties with their /// expected values. Assertions are on substrings — they do NOT assert the full file. #[test] diff --git a/rust/operator-binary/src/controller/build/resource/mod.rs b/rust/operator-binary/src/controller/build/resource/mod.rs index 9598a324..5643e4c9 100644 --- a/rust/operator-binary/src/controller/build/resource/mod.rs +++ b/rust/operator-binary/src/controller/build/resource/mod.rs @@ -6,5 +6,6 @@ pub mod config_map; pub mod listener; pub mod pdb; pub mod rbac; +pub mod secret; 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 index acb29d8d..10ccdc02 100644 --- a/rust/operator-binary/src/controller/build/resource/rbac.rs +++ b/rust/operator-binary/src/controller/build/resource/rbac.rs @@ -1,27 +1,21 @@ //! 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}, - }, + v2::rbac, }; 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. +/// +/// Both RBAC resources are shared by the whole cluster rather than tied to a role or role group, +/// hence the cluster-shared recommended labels. pub fn build_service_account(cluster: &ValidatedCluster) -> ServiceAccount { rbac::build_service_account( cluster, &cluster.cluster_resource_names(), - rbac_labels(cluster), + cluster.cluster_shared_recommended_labels(), ) } @@ -31,16 +25,10 @@ pub fn build_role_binding(cluster: &ValidatedCluster) -> RoleBinding { rbac::build_role_binding( cluster, &cluster.cluster_resource_names(), - rbac_labels(cluster), + cluster.cluster_shared_recommended_labels(), ) } -/// 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; diff --git a/rust/operator-binary/src/controller/build/resource/secret.rs b/rust/operator-binary/src/controller/build/resource/secret.rs new file mode 100644 index 00000000..47546894 --- /dev/null +++ b/rust/operator-binary/src/controller/build/resource/secret.rs @@ -0,0 +1,418 @@ +//! Builds the Secrets whose contents this operator generates: the sensitive-properties key and, +//! for OIDC authentication, the admin password. +//! +//! Their contents are randomly generated, so an identical Secret can never be *rebuilt*. Instead +//! the contents fetched in the dereference step are re-emitted unchanged, which makes applying +//! them a no-op. That way each Secret is emitted on *every* reconcile run and can be applied and +//! tracked like every other resource, rather than being a read-or-create side effect outside the +//! regular pipeline. + +use std::collections::BTreeMap; + +use rand::{RngExt, distr::Alphanumeric}; +use snafu::{Snafu, ensure}; +use stackable_operator::{ + builder::meta::ObjectMetaBuilder, + k8s_openapi::{api::core::v1::Secret, apimachinery::pkg::apis::meta::v1::ObjectMeta}, + kube::runtime::reflector::ObjectRef, + v2::types::operator::ClusterName, +}; + +use crate::{ + controller::ValidatedCluster, + security::authentication::{NifiAuthenticationConfig, STACKABLE_ADMIN_USERNAME}, +}; + +/// The key under which the sensitive-properties key is stored in its Secret. The `nifi.properties` +/// builder references the mounted file by this same name, so the two must agree. +pub const SENSITIVE_PROPERTY_KEY_NAME: &str = "nifiSensitivePropsKey"; + +/// The length of the passwords generated here. +const GENERATED_PASSWORD_LENGTH: usize = 15; + +#[derive(Snafu, Debug)] +pub enum Error { + #[snafu(display( + "sensitive key secret [{namespace}/{name}] is missing, but auto generation is disabled", + ))] + SensitiveKeySecretMissing { name: String, namespace: String }, + + #[snafu(display( + "found existing admin password secret {secret}, but the key {STACKABLE_ADMIN_USERNAME} is missing", + ))] + MissingAdminPasswordKey { secret: ObjectRef }, +} + +type Result = std::result::Result; + +/// Builds every Secret of this cluster: the sensitive-properties key and, for OIDC +/// authentication, the admin password. +pub fn build_secrets(cluster: &ValidatedCluster) -> Result> { + Ok(build_sensitive_key_secret(cluster)? + .into_iter() + .chain(build_oidc_admin_password_secret(cluster)?) + .collect()) +} + +/// The Secret holding the key with which NiFi encrypts the sensitive properties of its +/// processors, mounted by the NiFi Pods. +/// +/// Only emitted when `autoGenerate` is set. Without it the Secret is provided and owned by the +/// user, so this operator must not write to it at all — it is merely required to exist. +/// +/// A Secret that is already present is re-emitted unchanged (see the module docs); regenerating +/// it would render the sensitive properties of the persisted flow undecryptable. +fn build_sensitive_key_secret(cluster: &ValidatedCluster) -> Result> { + let sensitive_properties = &cluster.cluster_config.sensitive_properties; + let name = sensitive_properties.key_secret.to_string(); + let existing = cluster.existing_secrets.sensitive_key.as_ref(); + + if !sensitive_properties.auto_generate { + ensure!( + existing.is_some(), + SensitiveKeySecretMissingSnafu { + name, + namespace: cluster.namespace.to_string(), + } + ); + return Ok(None); + } + + Ok(Some(match existing { + Some(existing) => reemit_secret(cluster, &name, existing), + None => { + tracing::info!( + secret.name = name, + "No existing sensitive properties key found, generating new one" + ); + generate_secret(cluster, &name, SENSITIVE_PROPERTY_KEY_NAME) + } + })) +} + +/// The name of the Secret built by [`build_oidc_admin_password_secret`], which the StatefulSet +/// builder mounts and the dereference step looks up. +pub fn build_oidc_admin_password_secret_name(cluster_name: &ClusterName) -> String { + format!("{cluster_name}-oidc-admin-password") +} + +/// The Secret holding the password of the admin user that can access the API, mounted by the NiFi +/// Pods. This admin user is the same as for SingleUser authentication. +/// +/// Only emitted for OIDC authentication, which is the only authentication method that uses it. +fn build_oidc_admin_password_secret(cluster: &ValidatedCluster) -> Result> { + if !matches!( + cluster.cluster_config.authentication, + NifiAuthenticationConfig::Oidc { .. } + ) { + return Ok(None); + } + + let name = build_oidc_admin_password_secret_name(&cluster.name); + + Ok(Some(match &cluster.existing_secrets.oidc_admin_password { + Some(existing) => { + // An existing Secret without the admin password is not replaced: it was not + // created by this operator, so overwriting it would clobber whatever it holds. + let admin_password_present = existing + .data + .iter() + .flat_map(|data| data.keys()) + .any(|key| key == STACKABLE_ADMIN_USERNAME); + ensure!( + admin_password_present, + MissingAdminPasswordKeySnafu { + secret: ObjectRef::from_obj(existing), + } + ); + + reemit_secret(cluster, &name, existing) + } + None => { + tracing::info!( + secret.name = name, + "No existing oidc admin password secret found, generating new one" + ); + generate_secret(cluster, &name, STACKABLE_ADMIN_USERNAME) + } + })) +} + +/// A Secret holding a freshly generated random password under the given `key`. +fn generate_secret(cluster: &ValidatedCluster, name: &str, key: &str) -> Secret { + let password: String = rand::rng() + .sample_iter(&Alphanumeric) + .take(GENERATED_PASSWORD_LENGTH) + .map(char::from) + .collect(); + + Secret { + metadata: secret_meta(cluster, name), + string_data: Some(BTreeMap::from([(key.to_string(), password)])), + ..Secret::default() + } +} + +/// Re-emits an existing Secret, carrying its fetched `data` over unchanged: the contents are +/// randomly generated at creation and cannot be rebuilt, so echoing them back is the only way to +/// emit the Secret on every run without rotating its contents. Applying identical contents +/// changes nothing on the server (no watch event, no propagation into the Pods, no restart). +/// +/// The metadata is built fresh rather than echoed, because a fetched object carries +/// server-populated fields (`resourceVersion`, `uid`, `managedFields`) that must not appear in an +/// apply patch. +fn reemit_secret(cluster: &ValidatedCluster, name: &str, existing: &Secret) -> Secret { + Secret { + metadata: secret_meta(cluster, name), + data: existing.data.clone(), + ..Secret::default() + } +} + +/// Metadata shared by the freshly generated and the re-emitted Secret, so that the two are +/// identical apart from their contents. +/// +/// Deliberately carries no owner reference, unlike every other resource built by this operator: +/// both Secrets have to outlive the NifiCluster. The sensitive-properties key still decrypts the +/// persisted flow after the cluster is recreated, and it may even have been created by the user +/// rather than by this operator. Not being owned by the cluster also keeps them out of +/// `ClusterResources`' orphan listing, which only considers directly owned resources. +fn secret_meta(cluster: &ValidatedCluster, name: &str) -> ObjectMeta { + ObjectMetaBuilder::new() + .name_and_namespace(cluster) + .name(name) + .with_labels(cluster.cluster_shared_recommended_labels()) + .build() +} + +#[cfg(test)] +mod tests { + use stackable_operator::{ + commons::tls_verification::TlsClientDetails, crd::authentication::oidc, + k8s_openapi::ByteString, kube::ResourceExt as _, + }; + + use super::*; + use crate::controller::build::properties::test_support::{ + app_version_label, minimal_validated_cluster, + }; + + /// A Secret as it comes back from the API server: contents in `data`, plus the server-owned + /// metadata that must not be echoed into an apply patch. + fn fetched_secret(name: &str, key: &str) -> Secret { + serde_json::from_value(serde_json::json!({ + "apiVersion": "v1", + "kind": "Secret", + "metadata": { + "name": name, + "namespace": "default", + "resourceVersion": "12345", + "uid": "0a3b1f0e-1111-2222-3333-444455556666", + }, + "data": { key: "b2xkLXNlY3JldA==" }, + })) + .expect("valid Secret") + } + + fn oidc_cluster() -> ValidatedCluster { + let mut cluster = minimal_validated_cluster(); + let cluster_name = cluster.name.clone(); + cluster.cluster_config.authentication = NifiAuthenticationConfig::Oidc { + provider: oidc::v1alpha1::AuthenticationProvider::new( + "keycloak.mycorp.org".to_owned().try_into().unwrap(), + Some(443), + "/realms/sdp".to_owned(), + TlsClientDetails { tls: None }, + "preferred_username".to_owned(), + vec!["openid".to_owned()], + None, + ), + oidc: oidc::v1alpha1::ClientAuthenticationOptions { + client_credentials_secret_ref: "nifi-keycloak-client".to_owned(), + extra_scopes: vec![], + product_specific_fields: (), + }, + cluster_name, + }; + cluster + } + + #[test] + fn generates_the_sensitive_key_secret_when_it_is_missing() { + let secrets = + build_secrets(&minimal_validated_cluster()).expect("the Secrets must be buildable"); + + let [secret] = secrets.as_slice() else { + panic!("SingleUser authentication only needs the sensitive key Secret"); + }; + assert_eq!(secret.name_any(), "simple-nifi-sensitive-property-key"); + assert_eq!( + secret + .string_data + .as_ref() + .expect("a generated Secret carries its contents in string_data") + .get(SENSITIVE_PROPERTY_KEY_NAME) + .map(String::len), + Some(GENERATED_PASSWORD_LENGTH) + ); + } + + /// An existing Secret is re-emitted with its fetched contents unchanged, so that applying it + /// is a no-op instead of rotating the key on every reconcile run. + #[test] + fn reemits_the_existing_sensitive_key_secret_unchanged() { + let mut cluster = minimal_validated_cluster(); + cluster.existing_secrets.sensitive_key = Some(fetched_secret( + "simple-nifi-sensitive-property-key", + SENSITIVE_PROPERTY_KEY_NAME, + )); + + let secrets = build_secrets(&cluster).expect("the Secrets must be buildable"); + + let [secret] = secrets.as_slice() else { + panic!("SingleUser authentication only needs the sensitive key Secret"); + }; + assert_eq!( + secret.data.as_ref().and_then(|data| data + .get(SENSITIVE_PROPERTY_KEY_NAME) + .map(|ByteString(value)| value.clone())), + Some(b"old-secret".to_vec()), + "the fetched contents must be carried over unchanged" + ); + assert!( + secret.string_data.is_none(), + "nothing may be regenerated for an existing Secret" + ); + // The server-populated metadata of the fetched Secret must not end up in the apply patch. + assert_eq!(secret.metadata.resource_version, None); + assert_eq!(secret.metadata.uid, None); + } + + /// Without `autoGenerate` the Secret belongs to the user, so it is only required to exist and + /// is never emitted (and hence never written to). + #[test] + fn never_emits_a_user_provided_sensitive_key_secret() { + let mut cluster = minimal_validated_cluster(); + cluster.cluster_config.sensitive_properties.auto_generate = false; + cluster.existing_secrets.sensitive_key = Some(fetched_secret( + "simple-nifi-sensitive-property-key", + SENSITIVE_PROPERTY_KEY_NAME, + )); + + let secrets = build_secrets(&cluster).expect("the Secrets must be buildable"); + + assert!(secrets.is_empty()); + } + + #[test] + fn fails_when_the_sensitive_key_secret_is_missing_and_auto_generation_is_disabled() { + let mut cluster = minimal_validated_cluster(); + cluster.cluster_config.sensitive_properties.auto_generate = false; + + let error = build_secrets(&cluster).expect_err("the missing Secret must be reported"); + + assert!( + matches!(error, Error::SensitiveKeySecretMissing { .. }), + "unexpected error: {error:?}" + ); + } + + #[test] + fn generates_the_oidc_admin_password_secret_when_it_is_missing() { + let secrets = build_secrets(&oidc_cluster()).expect("the Secrets must be buildable"); + + let [_sensitive_key, admin_password] = secrets.as_slice() else { + panic!("OIDC authentication needs both Secrets"); + }; + assert_eq!(admin_password.name_any(), "simple-nifi-oidc-admin-password"); + assert!( + admin_password + .string_data + .as_ref() + .expect("a generated Secret carries its contents in string_data") + .contains_key(STACKABLE_ADMIN_USERNAME) + ); + } + + #[test] + fn reemits_the_existing_oidc_admin_password_secret_unchanged() { + let mut cluster = oidc_cluster(); + cluster.existing_secrets.oidc_admin_password = Some(fetched_secret( + "simple-nifi-oidc-admin-password", + STACKABLE_ADMIN_USERNAME, + )); + + let secrets = build_secrets(&cluster).expect("the Secrets must be buildable"); + + let [_sensitive_key, admin_password] = secrets.as_slice() else { + panic!("OIDC authentication needs both Secrets"); + }; + assert_eq!( + admin_password.data, + fetched_secret("simple-nifi-oidc-admin-password", STACKABLE_ADMIN_USERNAME).data, + "the fetched contents must be carried over unchanged" + ); + assert!( + admin_password.string_data.is_none(), + "nothing may be regenerated for an existing Secret" + ); + } + + #[test] + fn fails_when_the_existing_oidc_admin_password_secret_has_no_admin_password() { + let mut cluster = oidc_cluster(); + cluster.existing_secrets.oidc_admin_password = Some(fetched_secret( + "simple-nifi-oidc-admin-password", + "some-other-user", + )); + + let error = build_secrets(&cluster).expect_err("the incomplete Secret must be reported"); + + assert!( + matches!(error, Error::MissingAdminPasswordKey { .. }), + "unexpected error: {error:?}" + ); + } + + #[test] + fn omits_the_oidc_admin_password_secret_for_other_authentication_methods() { + // The minimal fixture uses SingleUser authentication. + let secrets = + build_secrets(&minimal_validated_cluster()).expect("the Secrets must be buildable"); + + assert!( + !secrets + .iter() + .any(|secret| secret.name_any() == "simple-nifi-oidc-admin-password") + ); + } + + /// Locks the metadata both Secrets carry: the labels `ClusterResources::add` requires (without + /// them the apply step rejects the resource) and the deliberately absent owner reference. + /// + /// [`ClusterResources::add`]: stackable_operator::cluster_resources::ClusterResources::add + #[test] + fn secret_metadata_is_labelled_but_not_owned_by_the_cluster() { + let secrets = build_secrets(&oidc_cluster()).expect("the Secrets must be buildable"); + + for secret in &secrets { + assert_eq!( + serde_json::to_value(&secret.metadata).expect("must be serializable"), + serde_json::json!({ + // The Secrets 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": secret.name_any(), + "namespace": "default", + }), + ); + } + } +} diff --git a/rust/operator-binary/src/controller/build/resource/statefulset.rs b/rust/operator-binary/src/controller/build/resource/statefulset.rs index 0f72379f..39f3aff6 100644 --- a/rust/operator-binary/src/controller/build/resource/statefulset.rs +++ b/rust/operator-binary/src/controller/build/resource/statefulset.rs @@ -51,6 +51,7 @@ use crate::{ build::{ BALANCE_PORT, BALANCE_PORT_NAME, HTTPS_PORT, HTTPS_PORT_NAME, NIFI_CONFIG_DIRECTORY, NIFI_PYTHON_WORKING_DIRECTORY, PROTOCOL_PORT, PROTOCOL_PORT_NAME, + SENSITIVE_PROPERTY_VOLUME_MOUNT, graceful_shutdown::add_graceful_shutdown_config, object_meta, properties::ConfigFileName, @@ -71,9 +72,10 @@ use crate::{ NifiAuthenticationConfig, STACKABLE_SERVER_TLS_DIR, STACKABLE_TLS_STORE_PASSWORD, }, authorization::{self, OPA_TLS_MOUNT_PATH, ResolvedNifiAuthorizationConfig}, - build_tls_volume, - sensitive_key::SENSITIVE_PROPERTY_VOLUME_MOUNT, - tls::{KEYSTORE_NIFI_CONTAINER_MOUNT, KEYSTORE_VOLUME_NAME, TRUSTSTORE_VOLUME_NAME}, + tls::{ + self, KEYSTORE_NIFI_CONTAINER_MOUNT, KEYSTORE_VOLUME_NAME, TRUSTSTORE_VOLUME_NAME, + build_tls_volume, + }, }, }; @@ -89,8 +91,8 @@ pub enum Error { source: crate::security::authentication::Error, }, - #[snafu(display("security failure"))] - Security { source: crate::security::Error }, + #[snafu(display("failed to build the TLS certificate Volume"))] + BuildTlsVolume { source: tls::Error }, #[snafu(display("failed to add needed volume"))] AddVolume { source: builder::pod::Error }, @@ -611,7 +613,7 @@ pub(crate) fn build_node_rolegroup_statefulset( &requested_secret_lifetime, Some(LISTENER_VOLUME_NAME), ) - .context(SecuritySnafu)?, + .context(BuildTlsVolumeSnafu)?, ) .context(AddVolumeSnafu)? .add_empty_dir_volume(TRUSTSTORE_VOLUME_NAME.to_string(), None) diff --git a/rust/operator-binary/src/controller/dereference.rs b/rust/operator-binary/src/controller/dereference.rs index 5eadb6ed..e896f7b7 100644 --- a/rust/operator-binary/src/controller/dereference.rs +++ b/rust/operator-binary/src/controller/dereference.rs @@ -7,13 +7,15 @@ use snafu::{ResultExt, Snafu}; use stackable_operator::{ client::Client, commons::networking::DomainName, + k8s_openapi::api::core::v1::Secret, v2::{ - controller_utils::{self, get_namespace}, + controller_utils::{self, get_cluster_name, get_namespace}, types::kubernetes::NamespaceName, }, }; use crate::{ + controller::build::resource::secret::build_oidc_admin_password_secret_name, crd::v1alpha1, security::{ authentication::{self, DereferencedAuthenticationClasses}, @@ -26,11 +28,20 @@ pub enum Error { #[snafu(display("failed to get the namespace"))] GetNamespace { source: controller_utils::Error }, + #[snafu(display("failed to get the cluster name"))] + GetClusterName { source: controller_utils::Error }, + #[snafu(display("failed to dereference NiFi authentication classes"))] DereferenceAuthenticationClasses { source: authentication::Error }, #[snafu(display("failed to dereference NiFi authorization config"))] DereferenceAuthorization { source: authorization_mod::Error }, + + #[snafu(display("failed to get the Secret {secret_name:?}"))] + GetSecret { + source: stackable_operator::client::Error, + secret_name: String, + }, } type Result = std::result::Result; @@ -44,6 +55,27 @@ pub struct DereferencedObjects { pub cluster_domain: DomainName, pub authentication_classes: DereferencedAuthenticationClasses, pub authorization: DereferencedAuthorization, + /// The Secrets whose contents this operator generates, as currently stored in Kubernetes. + pub existing_secrets: ExistingSecrets, +} + +/// The Secrets whose contents this operator generates, as currently stored in Kubernetes. +/// +/// Their contents are randomly generated at creation and can therefore not be rebuilt. They are +/// fetched here so that the build step can re-emit the existing contents unchanged, and only +/// generate fresh ones when a Secret is missing. See +/// [`build::resource::secret`](crate::controller::build::resource::secret). +#[derive(Clone, Debug, Default)] +pub struct ExistingSecrets { + /// The sensitive-properties key Secret named by + /// `spec.clusterConfig.sensitiveProperties.keySecret`, mounted by the NiFi Pods. + pub sensitive_key: Option, + + /// The admin password Secret for OIDC authentication, which this operator names itself. + /// + /// Fetched unconditionally because the authentication type is only resolved in the validate + /// step; the build step emits it for OIDC authentication only. + pub oidc_admin_password: Option, } /// Fetches all Kubernetes objects referenced from the [`v1alpha1::NifiCluster`] spec. @@ -65,10 +97,40 @@ pub async fn dereference( .await .context(DereferenceAuthorizationSnafu)?; + let cluster_name = get_cluster_name(nifi).context(GetClusterNameSnafu)?; + let existing_secrets = ExistingSecrets { + sensitive_key: get_secret_opt( + client, + &nifi.spec.cluster_config.sensitive_properties.key_secret, + &namespace, + ) + .await?, + oidc_admin_password: get_secret_opt( + client, + &build_oidc_admin_password_secret_name(&cluster_name), + &namespace, + ) + .await?, + }; + Ok(DereferencedObjects { namespace, cluster_domain: client.kubernetes_cluster_info.cluster_domain.clone(), authentication_classes, authorization, + existing_secrets, }) } + +/// Fetches the Secret with the given name, returning `None` if it does not exist. +async fn get_secret_opt( + client: &Client, + secret_name: &impl AsRef, + namespace: &NamespaceName, +) -> Result> { + let secret_name = secret_name.as_ref(); + client + .get_opt::(secret_name, namespace.as_ref()) + .await + .context(GetSecretSnafu { secret_name }) +} diff --git a/rust/operator-binary/src/controller/mod.rs b/rust/operator-binary/src/controller/mod.rs index bcf73c8e..99f0cc4e 100644 --- a/rust/operator-binary/src/controller/mod.rs +++ b/rust/operator-binary/src/controller/mod.rs @@ -15,7 +15,7 @@ use stackable_operator::{ k8s_openapi::{ api::{ apps::v1::StatefulSet, - core::v1::{ConfigMap, Service, ServiceAccount, Volume}, + core::v1::{ConfigMap, Secret, Service, ServiceAccount, Volume}, policy::v1::PodDisruptionBudget, rbac::v1::RoleBinding, }, @@ -42,6 +42,7 @@ use stackable_operator::{ use crate::{ OPERATOR_NAME, + controller::dereference::ExistingSecrets, crd::{ APP_NAME, HostHeaderCheckConfig, NifiConfig, NifiRole, NifiStorageConfig, sensitive_properties::NifiSensitiveKeyAlgorithm, v1alpha1, @@ -61,6 +62,12 @@ 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"); +// Placeholder role and role-group label values for resources that are shared by the whole cluster +// and therefore not tied to a role or role group (see +// [`ValidatedCluster::cluster_shared_recommended_labels`]). +stackable_operator::constant!(NONE_ROLE_NAME: RoleName = "none"); +stackable_operator::constant!(NONE_ROLE_GROUP_NAME: RoleGroupName = "none"); + /// Marker for prepared Kubernetes resources which are not applied yet. pub struct Prepared; @@ -79,6 +86,10 @@ pub struct KubernetesResources { pub services: Vec, pub listeners: Vec, pub config_maps: Vec, + /// The Secrets whose contents this operator generates: the sensitive-properties key and, for + /// OIDC authentication, the admin password. See + /// [`build::resource::secret`](crate::controller::build::resource::secret). + pub secrets: Vec, pub pod_disruption_budgets: Vec, pub service_accounts: Vec, pub role_bindings: Vec, @@ -203,6 +214,10 @@ pub struct ValidatedCluster { pub cluster_config: ValidatedClusterConfig, /// Collected configuration per rolegroup. pub role_group_configs: BTreeMap>, + /// The Secrets whose contents this operator generates, as currently stored in Kubernetes. + /// Carried through so the [`build`] step can re-emit their contents unchanged instead of + /// rotating them on every run. + pub existing_secrets: ExistingSecrets, } /// The resolved `spec.clusterConfig`. @@ -255,6 +270,7 @@ impl ValidatedCluster { role_config: ValidatedRoleConfig, role_group_configs: BTreeMap>, cluster_config: ValidatedClusterConfig, + existing_secrets: ExistingSecrets, ) -> Self { let metadata = ObjectMeta { name: Some(name.to_string()), @@ -275,6 +291,7 @@ impl ValidatedCluster { role_config, role_group_configs, cluster_config, + existing_secrets, } } @@ -313,6 +330,13 @@ impl ValidatedCluster { self.recommended_labels_with(&self.product_version, role_name, role_group_name) } + /// Recommended labels for a resource that is shared by the whole cluster rather than tied to a + /// role or role group (the RBAC pair, the Secrets built by this operator), which is expressed + /// by carrying `none` for both label values. + pub fn cluster_shared_recommended_labels(&self) -> Labels { + self.recommended_labels_for(&NONE_ROLE_NAME, &NONE_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 { diff --git a/rust/operator-binary/src/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs index 485c712f..3e11319e 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -203,6 +203,7 @@ pub fn validate( extra_volumes: nifi.spec.cluster_config.extra_volumes.clone(), host_header_check: nifi.spec.cluster_config.host_header_check.clone(), }, + dereferenced_objects.existing_secrets.clone(), )) } @@ -335,7 +336,9 @@ mod tests { use super::*; use crate::{ - controller::build::properties::test_support::app_version_label, + controller::{ + build::properties::test_support::app_version_label, dereference::ExistingSecrets, + }, security::{ authentication::DereferencedAuthenticationClasses, authorization::DereferencedAuthorization, @@ -396,6 +399,8 @@ mod tests { auth_entry, auth_class, )]), authorization: DereferencedAuthorization::without_opa(), + // As on the first reconcile run: neither Secret exists yet. + existing_secrets: ExistingSecrets::default(), }; let operator_environment = OperatorEnvironmentOptions { operator_namespace: "stackable-operators".to_owned(), diff --git a/rust/operator-binary/src/nifi_controller.rs b/rust/operator-binary/src/nifi_controller.rs index f5c3a8c9..34235c12 100644 --- a/rust/operator-binary/src/nifi_controller.rs +++ b/rust/operator-binary/src/nifi_controller.rs @@ -25,7 +25,7 @@ use strum::{EnumDiscriminants, IntoStaticStr}; use crate::{ OPERATOR_NAME, controller::{ - apply::{self, Applier, ensure_secrets}, + apply::{self, Applier}, build, dereference, update_status::{self, update_status}, validate, @@ -100,10 +100,6 @@ pub async fn reconcile_nifi( let resources = build::build(&validated_cluster).context(BuildResourcesSnafu)?; // apply (client required) - ensure_secrets(client, &validated_cluster) - .await - .context(ApplyResourcesSnafu)?; - let applied = Applier::new( client, &validated_cluster, diff --git a/rust/operator-binary/src/security/authentication.rs b/rust/operator-binary/src/security/authentication.rs index 0d561c03..61395d94 100644 --- a/rust/operator-binary/src/security/authentication.rs +++ b/rust/operator-binary/src/security/authentication.rs @@ -12,7 +12,9 @@ use stackable_operator::{ v2::types::operator::ClusterName, }; -use crate::{crd::v1alpha1, security::oidc::build_oidc_admin_password_secret_name}; +use crate::{ + controller::build::resource::secret::build_oidc_admin_password_secret_name, crd::v1alpha1, +}; pub const STACKABLE_ADMIN_USERNAME: &str = "admin"; diff --git a/rust/operator-binary/src/security/mod.rs b/rust/operator-binary/src/security/mod.rs index 2697d801..c50c1872 100644 --- a/rust/operator-binary/src/security/mod.rs +++ b/rust/operator-binary/src/security/mod.rs @@ -1,72 +1,8 @@ -use snafu::{ResultExt, Snafu}; -use stackable_operator::{ - builder::pod::volume::SecretFormat, - client::Client, - k8s_openapi::api::core::v1::Volume, - shared::time::Duration, - v2::types::{ - kubernetes::{NamespaceName, SecretClassName, VolumeName}, - operator::ClusterName, - }, -}; - -use crate::controller::ValidatedSensitiveProperties; +//! The security-related inputs of a NifiCluster: authentication, authorization and TLS. +//! +//! These modules resolve and validate what the spec asks for; the Kubernetes resources derived +//! from them are assembled by [`crate::controller::build`]. pub mod authentication; pub mod authorization; -pub mod oidc; -pub mod sensitive_key; pub mod tls; - -type Result = std::result::Result; - -#[derive(Snafu, Debug)] -pub enum Error { - #[snafu(display("tls failure"))] - Tls { source: tls::Error }, - - #[snafu(display("sensitive key failure"))] - SensitiveKey { source: sensitive_key::Error }, - - #[snafu(display("failed to ensure OIDC admin password exists"))] - OidcAdminPassword { source: oidc::Error }, -} - -pub async fn check_or_generate_sensitive_key( - client: &Client, - sensitive_properties: &ValidatedSensitiveProperties, - namespace: &NamespaceName, -) -> Result { - sensitive_key::check_or_generate_sensitive_key(client, sensitive_properties, namespace) - .await - .context(SensitiveKeySnafu) -} - -pub async fn check_or_generate_oidc_admin_password( - client: &Client, - cluster_name: &ClusterName, - namespace: &NamespaceName, -) -> Result { - oidc::check_or_generate_oidc_admin_password(client, cluster_name, namespace) - .await - .context(OidcAdminPasswordSnafu) -} - -pub fn build_tls_volume( - server_tls_secret_class: &SecretClassName, - volume_name: &VolumeName, - service_scopes: impl IntoIterator>, - secret_format: SecretFormat, - requested_secret_lifetime: &Duration, - listener_scope: Option<&str>, -) -> Result { - tls::build_tls_volume( - server_tls_secret_class, - volume_name, - service_scopes, - secret_format, - requested_secret_lifetime, - listener_scope, - ) - .context(TlsSnafu) -} diff --git a/rust/operator-binary/src/security/oidc.rs b/rust/operator-binary/src/security/oidc.rs deleted file mode 100644 index 4d1db3fb..00000000 --- a/rust/operator-binary/src/security/oidc.rs +++ /dev/null @@ -1,220 +0,0 @@ -use std::collections::BTreeMap; - -use rand::{RngExt, distr::Alphanumeric}; -use snafu::{ResultExt, Snafu}; -use stackable_operator::{ - builder::meta::ObjectMetaBuilder, - client::Client, - commons::tls_verification::{CaCert, TlsServerVerification, TlsVerification}, - crd::authentication::oidc, - k8s_openapi::api::core::v1::Secret, - kube::runtime::reflector::ObjectRef, - v2::types::{kubernetes::NamespaceName, operator::ClusterName}, -}; - -use crate::security::authentication::STACKABLE_ADMIN_USERNAME; - -type Result = std::result::Result; - -#[derive(Snafu, Debug)] -pub enum Error { - #[snafu(display("failed to fetch or create OIDC admin password secret"))] - OidcAdminPasswordSecret { - source: stackable_operator::client::Error, - }, - - #[snafu(display( - "found existing admin password secret {secret:?}, but the key {STACKABLE_ADMIN_USERNAME} is missing", - ))] - MissingAdminPasswordKey { secret: ObjectRef }, - - #[snafu(display("invalid well-known OIDC configuration URL"))] - InvalidWellKnownConfigUrl { - source: stackable_operator::crd::authentication::oidc::v1alpha1::Error, - }, - - #[snafu(display("Nifi doesn't support skipping the OIDC TLS verification"))] - SkippingTlsVerificationNotSupported {}, -} - -/// Generate a secret containing the password for the admin user that can access the API. -/// -/// This admin user is the same as for SingleUser authentication. -pub(crate) async fn check_or_generate_oidc_admin_password( - client: &Client, - cluster_name: &ClusterName, - namespace: &NamespaceName, -) -> Result { - tracing::debug!("Checking for OIDC admin password configuration"); - match client - .get_opt::( - &build_oidc_admin_password_secret_name(cluster_name), - namespace.as_ref(), - ) - .await - .context(OidcAdminPasswordSecretSnafu)? - { - Some(secret) => { - let admin_password_present = secret - .data - .iter() - .flat_map(|data| data.keys()) - .any(|key| key == STACKABLE_ADMIN_USERNAME); - - if admin_password_present { - Ok(false) - } else { - MissingAdminPasswordKeySnafu { - secret: ObjectRef::from_obj(&secret), - } - .fail()? - } - } - None => { - tracing::info!("No existing oidc admin password secret found, generating new one"); - let password: String = rand::rng() - .sample_iter(&Alphanumeric) - .take(15) - .map(char::from) - .collect(); - - let mut secret_data = BTreeMap::new(); - secret_data.insert(STACKABLE_ADMIN_USERNAME.to_string(), password); - - let new_secret = Secret { - metadata: ObjectMetaBuilder::new() - .namespace(namespace) - .name(build_oidc_admin_password_secret_name(cluster_name)) - .build(), - string_data: Some(secret_data), - ..Secret::default() - }; - client - .create(&new_secret) - .await - .context(OidcAdminPasswordSecretSnafu)?; - Ok(true) - } - } -} - -pub fn build_oidc_admin_password_secret_name(cluster_name: &ClusterName) -> String { - format!("{cluster_name}-oidc-admin-password") -} - -/// Adds all the required configuration properties to enable OIDC authentication. -pub fn add_oidc_config_to_properties( - provider: &oidc::v1alpha1::AuthenticationProvider, - client_auth_options: &oidc::v1alpha1::ClientAuthenticationOptions, - properties: &mut BTreeMap, -) -> Result<(), Error> { - let well_known_url = provider - .well_known_config_url() - .context(InvalidWellKnownConfigUrlSnafu)?; - - properties.insert( - "nifi.security.user.oidc.discovery.url".to_string(), - well_known_url.to_string(), - ); - let (oidc_client_id_env, oidc_client_secret_env) = - oidc::v1alpha1::AuthenticationProvider::client_credentials_env_names( - &client_auth_options.client_credentials_secret_ref, - ); - properties.insert( - "nifi.security.user.oidc.client.id".to_string(), - format!("${{env:{oidc_client_id_env}}}").to_string(), - ); - properties.insert( - "nifi.security.user.oidc.client.secret".to_string(), - format!("${{env:{oidc_client_secret_env}}}").to_string(), - ); - let scopes = provider.scopes.join(","); - properties.insert( - "nifi.security.user.oidc.additional.scopes".to_string(), - scopes.to_string(), - ); - properties.insert( - "nifi.security.user.oidc.claim.identifying.user".to_string(), - provider.principal_claim.to_string(), - ); - - if let Some(tls) = &provider.tls.tls { - let truststore_strategy = match tls.verification { - TlsVerification::None {} => SkippingTlsVerificationNotSupportedSnafu.fail()?, - TlsVerification::Server(TlsServerVerification { - ca_cert: CaCert::SecretClass(_), - }) => "NIFI", // The cert get's added to the stackable truststore - TlsVerification::Server(TlsServerVerification { - ca_cert: CaCert::WebPki {}, - }) => "JDK", // The cert needs to be in the system truststore - }; - properties.insert( - "nifi.security.user.oidc.truststore.strategy".to_owned(), - truststore_strategy.to_owned(), - ); - } - - Ok(()) -} - -#[cfg(test)] -mod tests { - use rstest::rstest; - use stackable_operator::commons::tls_verification::{Tls, TlsClientDetails}; - - use super::*; - - #[rstest] - #[case("/realms/sdp")] - #[case("/realms/sdp/")] - #[case("/realms/sdp/////")] - fn test_add_oidc_config(#[case] root_path: String) { - let mut properties = BTreeMap::new(); - let provider = oidc::v1alpha1::AuthenticationProvider::new( - "keycloak.mycorp.org".to_owned().try_into().unwrap(), - Some(443), - root_path, - TlsClientDetails { - tls: Some(Tls { - verification: TlsVerification::Server(TlsServerVerification { - ca_cert: CaCert::WebPki {}, - }), - }), - }, - "preferred_username".to_owned(), - vec!["openid".to_owned()], - None, - ); - let oidc = oidc::v1alpha1::ClientAuthenticationOptions { - client_credentials_secret_ref: "nifi-keycloak-client".to_owned(), - extra_scopes: vec![], - product_specific_fields: (), - }; - - add_oidc_config_to_properties(&provider, &oidc, &mut properties) - .expect("OIDC config adding failed"); - - assert_eq!( - properties.get("nifi.security.user.oidc.additional.scopes"), - Some(&"openid".to_owned()) - ); - assert_eq!( - properties.get("nifi.security.user.oidc.claim.identifying.user"), - Some(&"preferred_username".to_owned()) - ); - assert_eq!( - properties.get("nifi.security.user.oidc.discovery.url"), - Some( - &"https://keycloak.mycorp.org/realms/sdp/.well-known/openid-configuration" - .to_owned() - ) - ); - assert_eq!( - properties.get("nifi.security.user.oidc.truststore.strategy"), - Some(&"JDK".to_owned()) - ); - - assert!(properties.contains_key("nifi.security.user.oidc.client.id")); - assert!(properties.contains_key("nifi.security.user.oidc.client.secret")); - } -} diff --git a/rust/operator-binary/src/security/sensitive_key.rs b/rust/operator-binary/src/security/sensitive_key.rs deleted file mode 100644 index 0fefdbc2..00000000 --- a/rust/operator-binary/src/security/sensitive_key.rs +++ /dev/null @@ -1,78 +0,0 @@ -use std::collections::BTreeMap; - -use rand::{RngExt, distr::Alphanumeric}; -use snafu::{ResultExt, Snafu}; -use stackable_operator::{ - builder::meta::ObjectMetaBuilder, client::Client, k8s_openapi::api::core::v1::Secret, - v2::types::kubernetes::NamespaceName, -}; - -use crate::controller::ValidatedSensitiveProperties; - -/// The key under which the generated sensitive-properties key is stored in the Secret. The -/// `nifi.properties` builder references the mounted file by this same name, so the two must agree. -pub const SENSITIVE_PROPERTY_KEY_NAME: &str = "nifiSensitivePropsKey"; - -/// Mount path of the sensitive-properties key Secret -pub const SENSITIVE_PROPERTY_VOLUME_MOUNT: &str = "/stackable/sensitiveproperty"; - -type Result = std::result::Result; - -#[derive(Snafu, Debug)] -pub enum Error { - #[snafu(display("failed to check sensitive property key secret"))] - SensitiveKeySecret { - source: stackable_operator::client::Error, - }, - - #[snafu(display( - "sensitive key secret [{namespace}/{name}] is missing, but auto generation is disabled", - ))] - SensitiveKeySecretMissing { name: String, namespace: String }, -} - -pub(crate) async fn check_or_generate_sensitive_key( - client: &Client, - sensitive_properties: &ValidatedSensitiveProperties, - namespace: &NamespaceName, -) -> Result { - let key_secret = &sensitive_properties.key_secret; - match client - .get_opt::(key_secret.as_ref(), namespace.as_ref()) - .await - .context(SensitiveKeySecretSnafu)? - { - Some(_) => Ok(false), - None => { - if !sensitive_properties.auto_generate { - return Err(Error::SensitiveKeySecretMissing { - name: key_secret.to_string(), - namespace: namespace.to_string(), - }); - } - tracing::info!("No existing sensitive properties key found, generating new one"); - let password: String = rand::rng() - .sample_iter(&Alphanumeric) - .take(15) - .map(char::from) - .collect(); - - let mut secret_data = BTreeMap::new(); - secret_data.insert(SENSITIVE_PROPERTY_KEY_NAME.to_string(), password); - - let new_secret = Secret { - metadata: ObjectMetaBuilder::new() - .namespace(namespace) - .name(key_secret.to_string()) - .build(), - string_data: Some(secret_data), - ..Secret::default() - }; - client - .create(&new_secret) - .await - .context(SensitiveKeySecretSnafu)?; - Ok(true) - } - } -}