diff --git a/CHANGELOG.md b/CHANGELOG.md index b7d5f594..8e9d9e2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,7 @@ All notable changes to this project will be documented in this file. deletion is required ([#880]). - The operator now watches all resources that it creates and early-exits the reconcile action when the cluster is marked for deletion ([#882]). +- Make operations infallible where dependent on static inputs ([#886]). ### Fixed @@ -81,6 +82,7 @@ All notable changes to this project will be documented in this file. [#872]: https://github.com/stackabletech/opa-operator/pull/872 [#880]: https://github.com/stackabletech/opa-operator/pull/880 [#882]: https://github.com/stackabletech/opa-operator/pull/882 +[#886]: https://github.com/stackabletech/opa-operator/pull/886 ## [26.7.0] - 2026-07-21 diff --git a/rust/info-fetcher-commons/src/utils/secret.rs b/rust/info-fetcher-commons/src/utils/secret.rs index 70b1526d..f60f4725 100644 --- a/rust/info-fetcher-commons/src/utils/secret.rs +++ b/rust/info-fetcher-commons/src/utils/secret.rs @@ -13,7 +13,7 @@ const REDACTED: &str = "[redacted]"; /// `?token` or `#[instrument]` away from writing that token to the log file the Vector agent ships /// off the node. Wrapping the value means the leak has to be an explicit decision ([`Secret::expose`]) /// rather than an accident: the type has no [`Display`](fmt::Display), and its -/// [`Debug`](fmt::Debug) renders [`REDACTED`], so every struct that holds one can keep deriving +/// [`Debug`](fmt::Debug) renders a default value, so every struct that holds one can keep deriving /// `Debug` safely. #[derive(Clone, PartialEq, Eq, Deserialize)] #[serde(transparent)] diff --git a/rust/operator-binary/src/controller/build/resource/daemonset/mod.rs b/rust/operator-binary/src/controller/build/resource/daemonset/mod.rs index 36d93b63..29f77c29 100644 --- a/rust/operator-binary/src/controller/build/resource/daemonset/mod.rs +++ b/rust/operator-binary/src/controller/build/resource/daemonset/mod.rs @@ -177,11 +177,6 @@ pub enum Error { #[snafu(display("failed to add needed volume"))] AddVolume { source: builder::pod::Error }, - #[snafu(display("failed to add needed volumeMount"))] - AddVolumeMount { - source: builder::pod::container::Error, - }, - #[snafu(display("failed to build TLS volume"))] TlsVolumeBuild { source: builder::pod::volume::SecretOperatorVolumeSourceBuilderError, @@ -312,9 +307,9 @@ pub fn build_server_rolegroup_daemonset( .join(" && "), ]) .add_volume_mount(BUNDLES_VOLUME_NAME.as_ref(), BUNDLES_DIR) - .context(AddVolumeMountSnafu)? + .expect("The mount paths are statically defined and there should be no duplicates.") .add_volume_mount(LOG_VOLUME_NAME.as_ref(), STACKABLE_LOG_DIR) - .context(AddVolumeMountSnafu)? + .expect("The mount paths are statically defined and there should be no duplicates.") .resources(merged_config.resources.to_owned().into()); // All operator-set environment variables of the bundle-builder container, collected into an @@ -337,9 +332,9 @@ pub fn build_server_rolegroup_daemonset( )]) .add_env_vars(bundle_builder_env_vars) .add_volume_mount(BUNDLES_VOLUME_NAME.as_ref(), BUNDLES_DIR) - .context(AddVolumeMountSnafu)? + .expect("The mount paths are statically defined and there should be no duplicates.") .add_volume_mount(LOG_VOLUME_NAME.as_ref(), STACKABLE_LOG_DIR) - .context(AddVolumeMountSnafu)? + .expect("The mount paths are statically defined and there should be no duplicates.") .resources(sidecar_resource_requirements()) .readiness_probe(http_readiness_probe( BUNDLE_BUILDER_PROBE_PATH, @@ -383,16 +378,16 @@ pub fn build_server_rolegroup_daemonset( cb_opa.add_container_port(service::APP_TLS_PORT_NAME, service::APP_TLS_PORT.into()); cb_opa .add_volume_mount(TLS_VOLUME_NAME.as_ref(), TLS_STORE_DIR) - .context(AddVolumeMountSnafu)?; + .expect("The mount paths are statically defined and there should be no duplicates."); } else { cb_opa.add_container_port(APP_PORT_NAME, APP_PORT.into()); } cb_opa .add_volume_mount(CONFIG_VOLUME_NAME.as_ref(), CONFIG_DIR) - .context(AddVolumeMountSnafu)? + .expect("The mount paths are statically defined and there should be no duplicates.") .add_volume_mount(LOG_VOLUME_NAME.as_ref(), STACKABLE_LOG_DIR) - .context(AddVolumeMountSnafu)? + .expect("The mount paths are statically defined and there should be no duplicates.") .resources(merged_config.resources.to_owned().into()); let (probe_port_name, probe_scheme) = if cluster.is_tls_enabled() { @@ -853,6 +848,7 @@ mod tests { let _ = *BUNDLES_VOLUME_NAME; let _ = *USER_INFO_FETCHER_CREDENTIALS_VOLUME_NAME; let _ = *USER_INFO_FETCHER_KERBEROS_VOLUME_NAME; + let _ = *RESOURCE_INFO_FETCHER_CREDENTIALS_VOLUME_NAME; let _ = *TLS_VOLUME_NAME; let _ = *CONTAINERDEBUG_LOG_DIRECTORY; let _ = *WATCH_NAMESPACE; @@ -1264,6 +1260,60 @@ mod tests { ); } + /// The Entra backend projects its client credentials Secret like the Keycloak backend does. Its + /// TLS CA volume is named after the user's SecretClass and is added to the pod *before* the + /// resource-info-fetcher's statically named credentials volume, so this also pins that the two + /// do not collide (the derived name ends in `-ca-cert`). + #[test] + fn user_info_fetcher_entra_backend_mounts_client_credentials_next_to_resource_info_fetcher() { + let ds = build(&validated_cluster_from_spec(json!({ + "image": { "productVersion": "1.2.3" }, + "clusterConfig": { + "userInfo": { + "backend": { + "entra": { + "tenantId": "my-tenant", + "clientCredentialsSecret": "entra-credentials", + "tls": { + "verification": { + "server": { "caCert": { "secretClass": "my-ca" } } + } + }, + } + } + }, + "resourceInfo": { + "backend": { + "dataHub": { + "hostname": "datahub-gms.default.svc.cluster.local", + "credentialsSecretName": "datahub-credentials", + } + } + }, + }, + "servers": { "roleGroups": { "default": {} } }, + }))); + + let volumes = volume_names(&ds); + for expected in [ + "user-info-fetcher-credentials", + "my-ca-ca-cert", + "resource-info-fetcher-credentials", + ] { + assert!( + volumes.contains(&expected.to_owned()), + "missing volume {expected}" + ); + } + + let uif = uif_container(&ds); + assert_eq!( + mount_path(&uif, "user-info-fetcher-credentials"), + "/stackable/credentials" + ); + assert_eq!(read_only(&uif, "user-info-fetcher-credentials"), Some(true)); + } + /// A cluster running both info-fetcher sidecars, so their shared wiring can be asserted in one go. fn cluster_with_both_info_fetchers() -> ValidatedCluster { validated_cluster_from_spec(json!({ diff --git a/rust/operator-binary/src/controller/build/resource/daemonset/resource_info_fetcher.rs b/rust/operator-binary/src/controller/build/resource/daemonset/resource_info_fetcher.rs index 21efc533..94bafe3f 100644 --- a/rust/operator-binary/src/controller/build/resource/daemonset/resource_info_fetcher.rs +++ b/rust/operator-binary/src/controller/build/resource/daemonset/resource_info_fetcher.rs @@ -48,6 +48,7 @@ pub enum Error { type Result = std::result::Result; +/// Adds the Resource Info Fetcher sidecar container to the given [`PodBuilder`]. pub fn add_resource_info_fetcher_sidecar( pb: &mut PodBuilder, cluster: &ValidatedCluster, @@ -87,7 +88,7 @@ pub fn add_resource_info_fetcher_sidecar( // `stackable_rust_cli_env_vars`). They have to land on the shared log volume, // because that is the only place the Vector agent collects them from. .add_volume_mount(LOG_VOLUME_NAME.as_ref(), STACKABLE_LOG_DIR) - .context(AddVolumeMountSnafu)? + .expect("The mount paths are statically defined and there should be no duplicates.") .resources(sidecar_resource_requirements()); match &resource_info.backend { diff --git a/rust/operator-binary/src/controller/build/resource/daemonset/user_info_fetcher.rs b/rust/operator-binary/src/controller/build/resource/daemonset/user_info_fetcher.rs index ba2348c9..491e178b 100644 --- a/rust/operator-binary/src/controller/build/resource/daemonset/user_info_fetcher.rs +++ b/rust/operator-binary/src/controller/build/resource/daemonset/user_info_fetcher.rs @@ -78,6 +78,7 @@ pub enum Error { type Result = std::result::Result; +/// Adds the User Info Fetcher sidecar container to the given [`PodBuilder`]. pub fn add_user_info_fetcher_sidecar( pb: &mut PodBuilder, cluster: &ValidatedCluster, @@ -117,7 +118,7 @@ pub fn add_user_info_fetcher_sidecar( // `stackable_rust_cli_env_vars`). They have to land on the shared log volume, // because that is the only place the Vector agent collects them from. .add_volume_mount(LOG_VOLUME_NAME.as_ref(), STACKABLE_LOG_DIR) - .context(AddVolumeMountSnafu)? + .expect("The mount paths are statically defined and there should be no duplicates.") .resources(sidecar_resource_requirements()); match &user_info.backend { diff --git a/rust/operator-binary/src/crd/user_info_fetcher/mod.rs b/rust/operator-binary/src/crd/user_info_fetcher/mod.rs index 3de2d163..022a61bb 100644 --- a/rust/operator-binary/src/crd/user_info_fetcher/mod.rs +++ b/rust/operator-binary/src/crd/user_info_fetcher/mod.rs @@ -7,6 +7,7 @@ use stackable_operator::{ secret_class::SecretClassVolume, tls_verification::{CaCert, Tls, TlsClientDetails, TlsServerVerification, TlsVerification}, }, + constant, schemars::{self, JsonSchema}, v2::types::kubernetes::{SecretClassName, SecretName}, versioned::versioned, @@ -215,12 +216,15 @@ fn default_root_path() -> String { "/".to_string() } +constant!(ENTRA_DEFAULT_TOKEN_HOSTNAME: HostName = "login.microsoft.com"); +constant!(ENTRA_DEFAULT_USER_INFO_HOSTNAME: HostName = "graph.microsoft.com"); + fn entra_default_token_hostname() -> HostName { - HostName::from_str("login.microsoft.com").unwrap() + ENTRA_DEFAULT_TOKEN_HOSTNAME.clone() } fn entra_default_user_info_hostname() -> HostName { - HostName::from_str("graph.microsoft.com").unwrap() + ENTRA_DEFAULT_USER_INFO_HOSTNAME.clone() } fn default_tls_web_pki() -> Option { @@ -246,3 +250,15 @@ fn openldap_default_user_name_attribute() -> String { fn openldap_default_group_member_attribute() -> String { "member".to_string() } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_constants() { + // Test that dereferencing the constants does not panic. + let _ = *ENTRA_DEFAULT_TOKEN_HOSTNAME; + let _ = *ENTRA_DEFAULT_USER_INFO_HOSTNAME; + } +}