Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion rust/info-fetcher-commons/src/utils/secret.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
97 changes: 70 additions & 27 deletions rust/operator-binary/src/controller/build/resource/daemonset/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -198,14 +193,6 @@ pub enum Error {

type Result<T, E = Error> = std::result::Result<T, E>;

/// The typed [`ContainerName`] for a [`Container`]. The enum's `Display` values are all valid
/// container names, so this conversion is infallible.
fn container_name(container: &Container) -> ContainerName {
ContainerName::from_str(&container.to_string())
.expect("Container enum variants are valid container names")
}

/// The CPU and memory requests/limits shared by the bundle-builder and user-info-fetcher sidecars.
/// A [`VolumeMount`] the container may only read from.
///
/// Used for the config and credential volumes of the info-fetcher sidecars: they hold data the
Expand All @@ -221,6 +208,7 @@ fn read_only_mount(name: &str, mount_path: &str) -> VolumeMount {
}
}

/// The CPU and memory requests/limits shared by the bundle-builder and user-info-fetcher sidecars.
fn sidecar_resource_requirements() -> ResourceRequirements {
ResourceRequirementsBuilder::new()
.with_cpu_request("100m")
Expand Down Expand Up @@ -295,14 +283,14 @@ pub fn build_server_rolegroup_daemonset(

let mut pb = PodBuilder::new();

let prepare_container_name = container_name(&Container::Prepare);
let mut cb_prepare = new_container_builder(&prepare_container_name);
let prepare_container_name: &ContainerName = &Container::Prepare;
let mut cb_prepare = new_container_builder(prepare_container_name);

let bundle_builder_container_name = container_name(&Container::BundleBuilder);
let mut cb_bundle_builder = new_container_builder(&bundle_builder_container_name);
let bundle_builder_container_name: &ContainerName = &Container::BundleBuilder;
let mut cb_bundle_builder = new_container_builder(bundle_builder_container_name);

let opa_container_name = container_name(&Container::Opa);
let mut cb_opa = new_container_builder(&opa_container_name);
let opa_container_name: &ContainerName = &Container::Opa;
let mut cb_opa = new_container_builder(opa_container_name);

cb_prepare
.image_from_product_image(resolved_product_image)
Expand All @@ -312,9 +300,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
Expand All @@ -337,9 +325,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,
Expand Down Expand Up @@ -383,16 +371,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() {
Expand Down Expand Up @@ -516,7 +504,7 @@ pub fn build_server_rolegroup_daemonset(
// the Vector agent is enabled and the aggregator discovery ConfigMap name is valid.
if let Some(vector_log_config) = &merged_config.logging.vector_container {
pb.add_container(vector_container(
&container_name(&Container::Vector),
&Container::Vector,
resolved_product_image,
vector_log_config,
&cluster.role_group_resource_names(role_group_name),
Expand Down Expand Up @@ -853,6 +841,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;
Expand Down Expand Up @@ -1264,6 +1253,60 @@ mod tests {
);
}

/// The Entra backend projects its client credentials Secret like the Keycloak backend does. Its
/// TLS CA volume is named `<secret-class>-ca-cert` after the user's SecretClass, so this also
/// checks that a SecretClass-derived name coexists with the statically named credentials
/// volumes of both info-fetchers in one pod.
#[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!({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ use crate::controller::{
self,
resource::daemonset::{
CONFIG_DIR, CONFIG_VOLUME_NAME, LOG_VOLUME_NAME, RESOURCE_INFO_FETCHER_CREDENTIALS_DIR,
RESOURCE_INFO_FETCHER_CREDENTIALS_VOLUME_NAME, STACKABLE_LOG_DIR, container_name,
read_only_mount, sidecar_container_log_level, sidecar_resource_requirements,
RESOURCE_INFO_FETCHER_CREDENTIALS_VOLUME_NAME, STACKABLE_LOG_DIR, read_only_mount,
sidecar_container_log_level, sidecar_resource_requirements,
stackable_rust_cli_env_vars,
},
},
Expand All @@ -48,6 +48,7 @@ pub enum Error {

type Result<T, E = Error> = std::result::Result<T, E>;

/// Adds the Resource Info Fetcher sidecar container to the given [`PodBuilder`].
pub fn add_resource_info_fetcher_sidecar(
pb: &mut PodBuilder,
cluster: &ValidatedCluster,
Expand All @@ -56,8 +57,7 @@ pub fn add_resource_info_fetcher_sidecar(
cluster_info: &KubernetesClusterInfo,
) -> Result<()> {
if let Some(resource_info) = &cluster.cluster_config.resource_info {
let rif_container_name = container_name(&Container::ResourceInfoFetcher);
let mut cb_rif = new_container_builder(&rif_container_name);
let mut cb_rif = new_container_builder(&Container::ResourceInfoFetcher);

// All operator-set environment variables of the resource-info-fetcher container, collected
// into an `EnvVarSet` so that every name occurs only once.
Expand Down Expand Up @@ -87,7 +87,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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use crate::controller::{
resource::daemonset::{
CONFIG_DIR, CONFIG_VOLUME_NAME, LOG_VOLUME_NAME, STACKABLE_LOG_DIR,
USER_INFO_FETCHER_CREDENTIALS_DIR, USER_INFO_FETCHER_CREDENTIALS_VOLUME_NAME,
USER_INFO_FETCHER_KERBEROS_DIR, USER_INFO_FETCHER_KERBEROS_VOLUME_NAME, container_name,
USER_INFO_FETCHER_KERBEROS_DIR, USER_INFO_FETCHER_KERBEROS_VOLUME_NAME,
read_only_mount, sidecar_container_log_level, sidecar_resource_requirements,
stackable_rust_cli_env_vars,
},
Expand Down Expand Up @@ -78,6 +78,7 @@ pub enum Error {

type Result<T, E = Error> = std::result::Result<T, E>;

/// Adds the User Info Fetcher sidecar container to the given [`PodBuilder`].
pub fn add_user_info_fetcher_sidecar(
pb: &mut PodBuilder,
cluster: &ValidatedCluster,
Expand All @@ -86,8 +87,7 @@ pub fn add_user_info_fetcher_sidecar(
cluster_info: &KubernetesClusterInfo,
) -> Result<()> {
if let Some(user_info) = &cluster.cluster_config.user_info {
let user_info_fetcher_container_name = container_name(&Container::UserInfoFetcher);
let mut cb_user_info_fetcher = new_container_builder(&user_info_fetcher_container_name);
let mut cb_user_info_fetcher = new_container_builder(&Container::UserInfoFetcher);

// All operator-set environment variables of the user-info-fetcher container, collected
// into an `EnvVarSet` so that every name occurs only once. The backend match below may
Expand Down Expand Up @@ -117,7 +117,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 {
Expand Down
53 changes: 50 additions & 3 deletions rust/operator-binary/src/crd/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use stackable_operator::{
config_overrides::JsonConfigOverrides,
role_utils::{GenericCommonConfig, Role},
types::{
kubernetes::{ConfigMapName, SecretClassName},
kubernetes::{ConfigMapName, ContainerName, SecretClassName},
operator::RoleName,
},
},
Expand Down Expand Up @@ -219,6 +219,30 @@ pub enum Container {
ResourceInfoFetcher,
}

// Typed container names. They must match the strum `Display` (kebab-case) of the variants above,
// which is pinned by a unit test.
constant!(PREPARE_CONTAINER_NAME: ContainerName = "prepare");
constant!(VECTOR_CONTAINER_NAME: ContainerName = "vector");
constant!(BUNDLE_BUILDER_CONTAINER_NAME: ContainerName = "bundle-builder");
constant!(OPA_CONTAINER_NAME: ContainerName = "opa");
constant!(USER_INFO_FETCHER_CONTAINER_NAME: ContainerName = "user-info-fetcher");
constant!(RESOURCE_INFO_FETCHER_CONTAINER_NAME: ContainerName = "resource-info-fetcher");

impl Deref for Container {
type Target = ContainerName;

fn deref(&self) -> &Self::Target {
match self {
Container::Prepare => &PREPARE_CONTAINER_NAME,
Container::Vector => &VECTOR_CONTAINER_NAME,
Container::BundleBuilder => &BUNDLE_BUILDER_CONTAINER_NAME,
Container::Opa => &OPA_CONTAINER_NAME,
Container::UserInfoFetcher => &USER_INFO_FETCHER_CONTAINER_NAME,
Container::ResourceInfoFetcher => &RESOURCE_INFO_FETCHER_CONTAINER_NAME,
}
}
}

// NOTE (@Techassi): This struct can currently NOT be versioned because it is used via Role which
// makes it incredible hard to implement the From trait for conversions.
#[derive(Clone, Debug, Default, Fragment, JsonSchema, PartialEq)]
Expand Down Expand Up @@ -331,14 +355,37 @@ impl HasStatusCondition for v1alpha2::OpaCluster {
#[cfg(test)]
mod tests {
use indoc::formatdoc;
use stackable_operator::versioned::test_utils::RoundtripTestData;
use stackable_operator::{
v2::types::kubernetes::ContainerName, versioned::test_utils::RoundtripTestData,
};
use strum::IntoEnumIterator;

use super::{SERVER_ROLE_NAME, v1alpha1, v1alpha2};
use super::{
BUNDLE_BUILDER_CONTAINER_NAME, Container, OPA_CONTAINER_NAME, PREPARE_CONTAINER_NAME,
RESOURCE_INFO_FETCHER_CONTAINER_NAME, SERVER_ROLE_NAME, USER_INFO_FETCHER_CONTAINER_NAME,
VECTOR_CONTAINER_NAME, v1alpha1, v1alpha2,
};

#[test]
fn test_constants() {
// Test that dereferencing the constants does not panic.
let _ = *SERVER_ROLE_NAME;
let _ = *PREPARE_CONTAINER_NAME;
let _ = *VECTOR_CONTAINER_NAME;
let _ = *BUNDLE_BUILDER_CONTAINER_NAME;
let _ = *OPA_CONTAINER_NAME;
let _ = *USER_INFO_FETCHER_CONTAINER_NAME;
let _ = *RESOURCE_INFO_FETCHER_CONTAINER_NAME;
}

/// The typed container names behind `Container`'s `Deref` must agree with its strum
/// `Display`, which the logging configuration still uses as the per-container key.
#[test]
fn container_names_match_display() {
for container in Container::iter() {
let container_name: &ContainerName = &container;
assert_eq!(container_name.to_string(), container.to_string());
}
}

impl RoundtripTestData for v1alpha1::OpaClusterSpec {
Expand Down
20 changes: 18 additions & 2 deletions rust/operator-binary/src/crd/user_info_fetcher/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<Tls> {
Expand All @@ -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;
}
}
Loading