diff --git a/CHANGELOG.md b/CHANGELOG.md index 75b88a5c..5d04c514 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +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]). +- 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 @@ -24,6 +31,7 @@ All notable changes to this project will be documented in this file. [#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 [#975]: https://github.com/stackabletech/nifi-operator/pull/975 ## [26.7.0] - 2026-07-21 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/extra/crds.yaml b/extra/crds.yaml index 9af32095..5190da3a 100644 --- a/extra/crds.yaml +++ b/extra/crds.yaml @@ -315,8 +315,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/controller/apply.rs b/rust/operator-binary/src/controller/apply.rs new file mode 100644 index 00000000..065af25d --- /dev/null +++ b/rust/operator-binary/src/controller/apply.rs @@ -0,0 +1,139 @@ +//! 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, +}; + +#[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, + }, +} + +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, + secrets, + 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 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?; + + // 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, + secrets, + 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) + } +} diff --git a/rust/operator-binary/src/controller/build.rs b/rust/operator-binary/src/controller/build.rs index 6376f9df..29a1b15b 100644 --- a/rust/operator-binary/src/controller/build.rs +++ b/rust/operator-binary/src/controller/build.rs @@ -2,11 +2,12 @@ //! //! [`ValidatedCluster`]: crate::controller::ValidatedCluster -use std::str::FromStr; +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}, @@ -15,12 +16,13 @@ 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}, 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, }, @@ -49,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 { @@ -63,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. @@ -70,7 +78,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![]; @@ -119,31 +127,34 @@ 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)], + status: PhantomData, }) } -/// 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 } @@ -184,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 760fe651..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::{ @@ -179,6 +180,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 +190,7 @@ pub(crate) mod test_support { uid, image, product_version, + deployed_product_version, role_config, role_group_configs, ValidatedClusterConfig { @@ -213,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/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/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/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 39346f9e..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) @@ -662,7 +664,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(), 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 dc8a01c5..99f0cc4e 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::{ @@ -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, @@ -52,22 +53,47 @@ 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. 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; + +/// 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. -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, 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, + pub status: PhantomData, } /// A validated, merged (default <- role <- role-group) NiFi rolegroup config. @@ -171,8 +197,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, @@ -180,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`. @@ -228,9 +266,11 @@ impl ValidatedCluster { uid: Uid, image: ResolvedProductImage, product_version: ProductVersion, + deployed_product_version: ProductVersion, role_config: ValidatedRoleConfig, role_group_configs: BTreeMap>, cluster_config: ValidatedClusterConfig, + existing_secrets: ExistingSecrets, ) -> Self { let metadata = ObjectMeta { name: Some(name.to_string()), @@ -247,9 +287,11 @@ impl ValidatedCluster { uid, image, product_version, + deployed_product_version, role_config, role_group_configs, cluster_config, + existing_secrets, } } @@ -288,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/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/controller/validate.rs b/rust/operator-binary/src/controller/validate.rs index 498db02e..3e11319e 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 { @@ -186,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(), )) } @@ -318,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, @@ -379,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(), @@ -401,10 +423,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/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). diff --git a/rust/operator-binary/src/nifi_controller.rs b/rust/operator-binary/src/nifi_controller.rs index 88c59a0d..34235c12 100644 --- a/rust/operator-binary/src/nifi_controller.rs +++ b/rust/operator-binary/src/nifi_controller.rs @@ -1,6 +1,11 @@ //! 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::{str::FromStr, sync::Arc}; +use std::sync::Arc; use const_format::concatcp; use snafu::{ResultExt, Snafu}; @@ -14,22 +19,18 @@ use stackable_operator::{ }, logging::controller::ReconcilerError, shared::time::Duration, - status::condition::{ - compute_conditions, operations::ClusterOperationsConditionBuilder, - statefulset::StatefulSetConditionBuilder, - }, - v2::{cluster_resources::cluster_resources_new, types::operator::ProductVersion}, }; 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}, + build, dereference, + update_status::{self, update_status}, + validate, }, + crd::v1alpha1, }; pub const NIFI_CONTROLLER_NAME: &str = "nificluster"; @@ -42,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 { @@ -55,26 +55,14 @@ 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, - }, - #[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("failed to apply the Kubernetes resources"))] + ApplyResources { source: apply::Error }, - #[snafu(display("security failure"))] - Security { source: crate::security::Error }, + #[snafu(display("failed to update the cluster status"))] + UpdateStatus { source: update_status::Error }, } type Result = std::result::Result; @@ -108,118 +96,24 @@ 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; + // 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) + 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)?; - - 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 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); - - 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"), - ), - conditions, - }; + ) + .apply(resources) + .await + .context(ApplyResourcesSnafu)?; - 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()) } 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) - } - } -}