diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f47139a..377dd07a 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. They are emitted only while they do not exist yet, so their + 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]). - NiFi startup and readiness probes now use the local management server's `/health` and `/health/cluster` endpoints instead of a bare TCP check ([#976]). @@ -26,6 +33,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 [#976]: https://github.com/stackabletech/nifi-operator/pull/976 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 d0a2d7e6..fab731cb 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, }, @@ -62,6 +64,9 @@ pub const MANAGEMENT_SERVER_PORT: u16 = 52020; // 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 { @@ -83,7 +88,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![]; @@ -132,31 +137,34 @@ pub fn build(cluster: &ValidatedCluster) -> Result { services, listeners, config_maps, + secrets: build_secrets(cluster), 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 } @@ -197,6 +205,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..b932dc25 100644 --- a/rust/operator-binary/src/controller/build/properties.rs +++ b/rust/operator-binary/src/controller/build/properties.rs @@ -81,9 +81,15 @@ pub(crate) mod test_support { use std::str::FromStr as _; use stackable_operator::{ - commons::{networking::DomainName, product_image_selection::ResolvedProductImage}, - crd::authentication::r#static::v1alpha1::{ - AuthenticationProvider as StaticAuthProvider, UserCredentialsSecretRef, + commons::{ + networking::DomainName, product_image_selection::ResolvedProductImage, + tls_verification::TlsClientDetails, + }, + crd::authentication::{ + oidc, + r#static::v1alpha1::{ + AuthenticationProvider as StaticAuthProvider, UserCredentialsSecretRef, + }, }, kvp::LabelValue, v2::types::{ @@ -95,7 +101,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 +186,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 +196,7 @@ pub(crate) mod test_support { uid, image, product_version, + deployed_product_version, role_config, role_group_configs, ValidatedClusterConfig { @@ -213,9 +223,39 @@ 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 { + sensitive_key: None, + oidc_admin_password: None, + }, ) } + /// The OIDC authentication config, for tests that need an authentication method other than the + /// fixture's `SingleUser`, the admin password Secret is only used with this one. + pub fn oidc_authentication_config(cluster_name: &ClusterName) -> NifiAuthenticationConfig { + NifiAuthenticationConfig::Oidc { + provider: oidc::v1alpha1::AuthenticationProvider::new( + "keycloak.mycorp.org" + .to_owned() + .try_into() + .expect("valid hostname"), + 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_name.clone(), + } + } + /// Return the "default" role-group config from a [`ValidatedCluster`]. pub fn default_rg(cluster: &ValidatedCluster) -> &NifiRoleGroupConfig { cluster 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 a475d2e9..2feba98c 100644 --- a/rust/operator-binary/src/controller/build/resource/mod.rs +++ b/rust/operator-binary/src/controller/build/resource/mod.rs @@ -7,5 +7,6 @@ pub mod listener; pub mod pdb; pub mod probes; 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..54d21387 --- /dev/null +++ b/rust/operator-binary/src/controller/build/resource/secret.rs @@ -0,0 +1,314 @@ +//! Builds the Secrets whose contents this operator generates: the sensitive-properties key and, +//! for OIDC authentication, the admin password. +//! +//! Both are emitted only while they do not exist yet, as determined by the dereference step. Their +//! contents are randomly generated, so an identical Secret can never be *rebuilt*, and rewriting an +//! existing one would rotate contents that have to stay stable — a fresh sensitive-properties key +//! cannot decrypt the sensitive values of the persisted flow. +//! +//! Emitting them only once is enough because, unlike the operator-generated Secrets of the sibling +//! operators, these are deliberately not owned by the NifiCluster (see [`secret_meta`]) and hence +//! never orphan-deleted. So there is nothing to re-emit them for. +//! +//! Everything that would leave an existing Secret unusable is rejected by the validate step. + +use std::collections::BTreeMap; + +use rand::{RngExt, distr::Alphanumeric}; +use stackable_operator::{ + builder::meta::ObjectMetaBuilder, + k8s_openapi::{api::core::v1::Secret, apimachinery::pkg::apis::meta::v1::ObjectMeta}, + 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; + +/// Builds every Secret of this cluster: the sensitive-properties key and, for OIDC +/// authentication, the admin password. +/// +/// Infallible: everything that could make a Secret unusable is rejected by the validate step, +/// which checks that an existing Secret carries its expected key and that a missing one may be +/// generated. +pub fn build_secrets(cluster: &ValidatedCluster) -> Vec { + 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 and the Secret does not exist yet. Without +/// `autoGenerate` the Secret is provided and owned by the user, so this operator must not write to +/// it at all. The validate step merely requires it to exist. +fn build_sensitive_key_secret(cluster: &ValidatedCluster) -> Option { + let sensitive_properties = &cluster.cluster_config.sensitive_properties; + if !sensitive_properties.auto_generate || cluster.existing_secrets.sensitive_key.is_some() { + return None; + } + + let name = sensitive_properties.key_secret.to_string(); + tracing::info!( + secret.name = name, + "No existing sensitive properties key found, generating new one" + ); + Some(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 — the only authentication method that uses it — and only +/// while the Secret does not exist yet. +fn build_oidc_admin_password_secret(cluster: &ValidatedCluster) -> Option { + if !matches!( + cluster.cluster_config.authentication, + NifiAuthenticationConfig::Oidc { .. } + ) || cluster.existing_secrets.oidc_admin_password.is_some() + { + return None; + } + + let name = build_oidc_admin_password_secret_name(&cluster.name); + tracing::info!( + secret.name = name, + "No existing oidc admin password secret found, generating new one" + ); + Some(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() + } +} + +/// Metadata of a generated Secret. +/// +/// 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::kube::ResourceExt as _; + + use super::*; + use crate::controller::build::properties::test_support::{ + app_version_label, minimal_validated_cluster, oidc_authentication_config, + }; + + /// The Secret the fixture asks for in `spec.clusterConfig.sensitiveProperties.keySecret`. + const SENSITIVE_KEY_SECRET: &str = "simple-nifi-sensitive-property-key"; + + /// The admin password Secret, whose name this operator derives from the fixture's cluster name. + const OIDC_ADMIN_PASSWORD_SECRET: &str = "simple-nifi-oidc-admin-password"; + + /// Everything the tests below assume about the shared fixture. A change to it fails here, + /// instead of as a puzzling failure in one of them. + #[test] + fn fixture_preconditions() { + let cluster = minimal_validated_cluster(); + + assert!(cluster.cluster_config.sensitive_properties.auto_generate); + assert!(!matches!( + cluster.cluster_config.authentication, + NifiAuthenticationConfig::Oidc { .. } + )); + // As on the first reconcile run: neither Secret exists yet. + assert!(cluster.existing_secrets.sensitive_key.is_none()); + assert!(cluster.existing_secrets.oidc_admin_password.is_none()); + assert_eq!( + cluster + .cluster_config + .sensitive_properties + .key_secret + .to_string(), + SENSITIVE_KEY_SECRET + ); + assert_eq!( + build_oidc_admin_password_secret_name(&cluster.name), + OIDC_ADMIN_PASSWORD_SECRET + ); + } + + /// The fixture switched over to OIDC authentication, which additionally needs the admin + /// password Secret. + fn oidc_cluster() -> ValidatedCluster { + let mut cluster = minimal_validated_cluster(); + let authentication = oidc_authentication_config(&cluster.name); + cluster.cluster_config.authentication = authentication; + cluster + } + + /// An existing Secret, as the dereference step hands it over. Only its presence matters to the + /// build step, whether its contents are usable is the validate step's business — so it + /// deliberately carries none. + fn existing_secret(name: &str) -> Secret { + Secret { + metadata: ObjectMeta { + name: Some(name.to_owned()), + namespace: Some("default".to_owned()), + ..ObjectMeta::default() + }, + ..Secret::default() + } + } + + /// The built Secrets keyed by name: which order [`build_secrets`] returns them in is not part + /// of its contract. + fn to_secret_map(secrets: Vec) -> BTreeMap { + secrets + .into_iter() + .map(|secret| (secret.name_any(), secret)) + .collect() + } + + #[test] + fn generates_the_sensitive_key_secret_when_it_is_missing() { + let secrets = to_secret_map(build_secrets(&minimal_validated_cluster())); + + assert_eq!( + secrets + .get(SENSITIVE_KEY_SECRET) + .expect("should be emitted") + .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 left alone: it is not owned by the cluster, so nothing deletes it, + /// and rewriting it would rotate a key that has to keep decrypting the persisted flow. + #[test] + fn does_not_emit_an_existing_sensitive_key_secret() { + let mut cluster = minimal_validated_cluster(); + cluster.existing_secrets.sensitive_key = Some(existing_secret(SENSITIVE_KEY_SECRET)); + + let secrets = build_secrets(&cluster); + + assert!(secrets.is_empty()); + } + + /// Without `autoGenerate` the Secret belongs to the user, so this operator never writes it — + /// not even while it is missing, a case the validate step rejects before the build step runs. + #[test] + fn never_generates_a_user_provided_sensitive_key_secret() { + let mut cluster = minimal_validated_cluster(); + cluster.cluster_config.sensitive_properties.auto_generate = false; + + let secrets = build_secrets(&cluster); + + assert!(secrets.is_empty()); + } + + #[test] + fn generates_the_oidc_admin_password_secret_when_it_is_missing() { + let secrets = to_secret_map(build_secrets(&oidc_cluster())); + + assert!( + secrets + .get(OIDC_ADMIN_PASSWORD_SECRET) + .expect("should be emitted") + .string_data + .as_ref() + .expect("a generated Secret carries its contents in string_data") + .contains_key(STACKABLE_ADMIN_USERNAME) + ); + } + + #[test] + fn does_not_emit_an_existing_oidc_admin_password_secret() { + let mut cluster = oidc_cluster(); + cluster.existing_secrets.oidc_admin_password = + Some(existing_secret(OIDC_ADMIN_PASSWORD_SECRET)); + + let secrets = to_secret_map(build_secrets(&cluster)); + + assert!(!secrets.contains_key(OIDC_ADMIN_PASSWORD_SECRET)); + assert!( + secrets.contains_key(SENSITIVE_KEY_SECRET), + "the still missing sensitive key Secret is unaffected" + ); + } + + #[test] + fn omits_the_oidc_admin_password_secret_for_other_authentication_methods() { + // `fixture_preconditions` locks that the fixture does not use OIDC authentication. + let secrets = to_secret_map(build_secrets(&minimal_validated_cluster())); + + assert!(!secrets.contains_key(OIDC_ADMIN_PASSWORD_SECRET)); + } + + /// 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 = to_secret_map(build_secrets(&oidc_cluster())); + + assert!( + secrets.contains_key(SENSITIVE_KEY_SECRET) + && secrets.contains_key(OIDC_ADMIN_PASSWORD_SECRET), + "both Secrets must be checked, not an empty list" + ); + for secret in secrets.values() { + 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 af0c3cc4..446f2128 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, @@ -76,9 +77,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, + }, }, }; @@ -94,8 +96,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 }, @@ -600,7 +602,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) @@ -651,7 +653,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..74b2b148 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 validate step can check an existing Secret for its expected key, and +/// the build step can generate only the ones that are still missing. See +/// [`build::resource::secret`](crate::controller::build::resource::secret). +#[derive(Clone, Debug)] +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..a6993a3b 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 only generates the ones that are still missing, + /// instead of rotating their contents 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..6fe4993a 100644 --- a/rust/operator-binary/src/controller/validate.rs +++ b/rust/operator-binary/src/controller/validate.rs @@ -5,12 +5,13 @@ use std::{collections::BTreeMap, str::FromStr as _}; -use snafu::{OptionExt, ResultExt, Snafu}; +use snafu::{OptionExt, ResultExt, Snafu, ensure}; use stackable_operator::{ cli::OperatorEnvironmentOptions, commons::product_image_selection, config::fragment, - kube::ResourceExt as _, + k8s_openapi::api::core::v1::Secret, + kube::{ResourceExt as _, runtime::reflector::ObjectRef}, product_logging::spec::Logging, role_utils::CommonConfiguration, v2::{ @@ -21,7 +22,7 @@ use stackable_operator::{ }, role_utils::with_validated_config, types::{ - kubernetes::ConfigMapName, + kubernetes::{ConfigMapName, NamespaceName}, operator::{ProductVersion, RoleGroupName}, }, }, @@ -33,10 +34,15 @@ use super::{ ValidatedNifiConfig, ValidatedRoleConfig, ValidatedSensitiveProperties, }; use crate::{ - controller::{build::git_sync::build_git_sync_resources, dereference::DereferencedObjects}, - crd::{Container, NifiConfig, NifiRole, v1alpha1}, + controller::{ + build::{ + git_sync::build_git_sync_resources, resource::secret::SENSITIVE_PROPERTY_KEY_NAME, + }, + dereference::{DereferencedObjects, ExistingSecrets}, + }, + crd::{Container, NifiConfig, NifiRole, sensitive_properties, v1alpha1}, security::{ - authentication::{self, NifiAuthenticationConfig}, + authentication::{self, NifiAuthenticationConfig, STACKABLE_ADMIN_USERNAME}, authorization::ResolvedNifiAuthorizationConfig, }, }; @@ -91,6 +97,29 @@ 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, + }, + + #[snafu(display( + "sensitive key secret [{namespace}/{name}] is missing, but auto generation is disabled", + ))] + SensitiveKeySecretMissing { name: String, namespace: String }, + + #[snafu(display( + "the existing sensitive key secret {secret} does not contain the key \ + {SENSITIVE_PROPERTY_KEY_NAME}", + ))] + SensitiveKeySecretIncomplete { secret: ObjectRef }, + + #[snafu(display( + "the existing admin password secret {secret} does not contain the key \ + {STACKABLE_ADMIN_USERNAME}", + ))] + AdminPasswordSecretIncomplete { secret: ObjectRef }, } type Result = std::result::Result; @@ -122,6 +151,13 @@ pub fn validate( &dereferenced_objects.authorization, ); + validate_existing_secrets( + &dereferenced_objects.existing_secrets, + &nifi.spec.cluster_config.sensitive_properties, + &authentication_config, + &dereferenced_objects.namespace, + )?; + let sensitive_properties_algorithm = nifi .spec .cluster_config @@ -159,6 +195,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 +212,7 @@ pub fn validate( uid, image, product_version, + deployed_product_version, role_config, role_group_configs, ValidatedClusterConfig { @@ -186,9 +233,72 @@ 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(), )) } +/// Checks the preconditions on the Secrets whose contents this operator generates. +/// +/// The build step only ever *creates* a missing Secret, it never rewrites an existing one (see the +/// [`secret`](crate::controller::build::resource::secret) module docs). So a Secret that is present +/// but does not carry its expected key is rejected here, rather than surfacing later as NiFi Pods +/// that cannot start. +/// +/// Rejecting rather than filling in the missing key is deliberate: a regenerated +/// sensitive-properties key cannot decrypt the sensitive values of an already persisted flow, so +/// silently writing a fresh one would destroy data. The admin password Secret follows the same rule +/// for consistency; it is not this operator's to overwrite either. +fn validate_existing_secrets( + existing_secrets: &ExistingSecrets, + sensitive_properties: &sensitive_properties::NifiSensitivePropertiesConfig, + authentication: &NifiAuthenticationConfig, + namespace: &NamespaceName, +) -> Result<()> { + match &existing_secrets.sensitive_key { + Some(secret) => ensure!( + secret_contains_key(secret, SENSITIVE_PROPERTY_KEY_NAME), + SensitiveKeySecretIncompleteSnafu { + secret: ObjectRef::from_obj(secret), + } + ), + // Without `autoGenerate` the Secret is provided and owned by the user, so this operator + // never creates it and it is merely required to exist. + None => ensure!( + sensitive_properties.auto_generate, + SensitiveKeySecretMissingSnafu { + name: sensitive_properties.key_secret.to_string(), + namespace: namespace.to_string(), + } + ), + } + + // The admin password Secret is only mounted for OIDC authentication; a leftover from a + // previous authentication method must not fail the cluster. It is named by this operator, so + // unlike the sensitive key Secret it is never required to exist up-front. + if matches!(authentication, NifiAuthenticationConfig::Oidc { .. }) + && let Some(secret) = &existing_secrets.oidc_admin_password + { + ensure!( + secret_contains_key(secret, STACKABLE_ADMIN_USERNAME), + AdminPasswordSecretIncompleteSnafu { + secret: ObjectRef::from_obj(secret), + } + ); + } + + Ok(()) +} + +/// Whether the given fetched Secret carries `key`. Only `data` is inspected: the API server always +/// returns the contents there, `string_data` is write-only. +fn secret_contains_key(secret: &Secret, key: &str) -> bool { + secret + .data + .iter() + .flat_map(BTreeMap::keys) + .any(|existing| existing == key) +} + pub(crate) fn build_role_group_configs( nifi: &v1alpha1::NifiCluster, image: &product_image_selection::ResolvedProductImage, @@ -312,19 +422,187 @@ pub(crate) fn test_resolved_product_image() -> product_image_selection::Resolved mod tests { use pretty_assertions::assert_eq; use stackable_operator::{ - commons::networking::DomainName, crd::authentication::core as auth_core, - v2::types::kubernetes::ConfigMapName, + commons::networking::DomainName, + crd::authentication::core as auth_core, + k8s_openapi::{ByteString, apimachinery::pkg::apis::meta::v1::ObjectMeta}, + v2::types::{kubernetes::ConfigMapName, operator::ClusterName}, }; use super::*; use crate::{ - controller::build::properties::test_support::app_version_label, + controller::{ + build::properties::test_support::{app_version_label, oidc_authentication_config}, + dereference::ExistingSecrets, + }, security::{ authentication::DereferencedAuthenticationClasses, authorization::DereferencedAuthorization, }, }; + /// The name of the sensitive-properties key Secret in every fixture below. + const SENSITIVE_KEY_SECRET_NAME: &str = "simple-nifi-sensitive-property-key"; + + /// `spec.clusterConfig.sensitiveProperties` with the given `autoGenerate` setting. + fn sensitive_properties( + auto_generate: bool, + ) -> sensitive_properties::NifiSensitivePropertiesConfig { + serde_yaml::from_str(&format!( + "keySecret: {SENSITIVE_KEY_SECRET_NAME}\nautoGenerate: {auto_generate}" + )) + .expect("valid sensitiveProperties") + } + + /// A Secret as the API server returns it: contents in `data`, under the given `keys`. Only the + /// key names matter here, so the values are a fixed placeholder. + fn fetched_secret(name: &str, keys: &[&str]) -> Secret { + Secret { + metadata: ObjectMeta { + name: Some(name.to_owned()), + namespace: Some("default".to_owned()), + ..ObjectMeta::default() + }, + data: Some( + keys.iter() + .map(|key| ((*key).to_owned(), ByteString(b"irrelevant".to_vec()))) + .collect(), + ), + ..Secret::default() + } + } + + fn single_user_authentication() -> NifiAuthenticationConfig { + NifiAuthenticationConfig::SingleUser { + provider: serde_yaml::from_str( + "userCredentialsSecret:\n name: nifi-admin-credentials-simple", + ) + .expect("valid static provider"), + } + } + + fn test_namespace() -> NamespaceName { + NamespaceName::from_str("default").expect("valid namespace") + } + + /// With `autoGenerate` the build step creates the Secret, so it may be absent. + #[test] + fn accepts_a_missing_sensitive_key_secret_when_auto_generation_is_enabled() { + let existing_secrets = ExistingSecrets { + sensitive_key: None, + oidc_admin_password: None, + }; + + validate_existing_secrets( + &existing_secrets, + &sensitive_properties(true), + &single_user_authentication(), + &test_namespace(), + ) + .expect("a missing Secret is generated by the build step"); + } + + /// Without `autoGenerate` the Secret is the user's to provide, so it has to be there already. + #[test] + fn rejects_a_missing_sensitive_key_secret_when_auto_generation_is_disabled() { + let existing_secrets = ExistingSecrets { + sensitive_key: None, + oidc_admin_password: None, + }; + + let error = validate_existing_secrets( + &existing_secrets, + &sensitive_properties(false), + &single_user_authentication(), + &test_namespace(), + ) + .expect_err("the missing Secret must be reported"); + + assert!( + matches!(error, Error::SensitiveKeySecretMissing { .. }), + "unexpected error: {error:?}" + ); + } + + /// An existing Secret is never rewritten, so one without the expected key would leave the NiFi + /// Pods with an unusable mount — regardless of `autoGenerate`. + #[test] + fn rejects_an_existing_sensitive_key_secret_without_its_key() { + for auto_generate in [true, false] { + let existing_secrets = ExistingSecrets { + sensitive_key: Some(fetched_secret( + SENSITIVE_KEY_SECRET_NAME, + &["some-other-key"], + )), + oidc_admin_password: None, + }; + + let error = validate_existing_secrets( + &existing_secrets, + &sensitive_properties(auto_generate), + &single_user_authentication(), + &test_namespace(), + ) + .expect_err("the incomplete Secret must be reported"); + + assert!( + matches!(error, Error::SensitiveKeySecretIncomplete { .. }), + "unexpected error for autoGenerate={auto_generate}: {error:?}" + ); + } + } + + #[test] + fn rejects_an_existing_admin_password_secret_without_its_key() { + let cluster_name = ClusterName::from_str("simple-nifi").expect("valid cluster name"); + let existing_secrets = ExistingSecrets { + sensitive_key: Some(fetched_secret( + SENSITIVE_KEY_SECRET_NAME, + &[SENSITIVE_PROPERTY_KEY_NAME], + )), + oidc_admin_password: Some(fetched_secret( + "simple-nifi-oidc-admin-password", + &["some-other-user"], + )), + }; + + let error = validate_existing_secrets( + &existing_secrets, + &sensitive_properties(true), + &oidc_authentication_config(&cluster_name), + &test_namespace(), + ) + .expect_err("the incomplete Secret must be reported"); + + assert!( + matches!(error, Error::AdminPasswordSecretIncomplete { .. }), + "unexpected error: {error:?}" + ); + } + + /// The admin password Secret is only mounted for OIDC, so a leftover from a previous + /// authentication method must not fail the cluster. + #[test] + fn ignores_the_admin_password_secret_for_other_authentication_methods() { + let existing_secrets = ExistingSecrets { + sensitive_key: Some(fetched_secret( + SENSITIVE_KEY_SECRET_NAME, + &[SENSITIVE_PROPERTY_KEY_NAME], + )), + oidc_admin_password: Some(fetched_secret( + "simple-nifi-oidc-admin-password", + &["some-other-user"], + )), + }; + + validate_existing_secrets( + &existing_secrets, + &sensitive_properties(true), + &single_user_authentication(), + &test_namespace(), + ) + .expect("the unused Secret must be ignored"); + } + /// Locks every value the validate step itself derives from the minimal fixture — so a /// validation regression fails here, with a validate-shaped message, instead of surfacing as /// a confusing build-test failure downstream. @@ -379,6 +657,11 @@ mod tests { auth_entry, auth_class, )]), authorization: DereferencedAuthorization::without_opa(), + // As on the first reconcile run: neither Secret exists yet. + existing_secrets: ExistingSecrets { + sensitive_key: None, + oidc_admin_password: None, + }, }; let operator_environment = OperatorEnvironmentOptions { operator_namespace: "stackable-operators".to_owned(), @@ -401,10 +684,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) - } - } -}