From d07316668931e047075e893217d772e65fcdddbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCller?= Date: Fri, 14 Aug 2026 09:30:00 +0200 Subject: [PATCH] chore: log when object overrides are not applied --- crates/stackable-operator/CHANGELOG.md | 8 ++ .../src/cluster_resources.rs | 47 ++++++++++- .../stackable-operator/src/deep_merger/crd.rs | 28 ++++++- .../stackable-operator/src/deep_merger/mod.rs | 78 +++++++++++++++++-- 4 files changed, 147 insertions(+), 14 deletions(-) diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index c306526c1..06ad7e83b 100644 --- a/crates/stackable-operator/CHANGELOG.md +++ b/crates/stackable-operator/CHANGELOG.md @@ -4,6 +4,14 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Changed + +- `ClusterResources` now warns about `objectOverrides` entries that did not match any of the objects it created ([#1264]). +- BREAKING: To enable this, `apply_deep_merge` now returns whether the merge matched the base object and `ObjectOverrides::apply_to` + returns the indices of the entries that matched. + +[#1264]: https://github.com/stackabletech/operator-rs/pull/1264 + ## [0.116.0] - 2026-08-14 ### Added diff --git a/crates/stackable-operator/src/cluster_resources.rs b/crates/stackable-operator/src/cluster_resources.rs index ef457bbfc..f4da2e6a6 100644 --- a/crates/stackable-operator/src/cluster_resources.rs +++ b/crates/stackable-operator/src/cluster_resources.rs @@ -445,6 +445,10 @@ pub struct ClusterResources<'a> { /// Arbitrary Kubernetes object overrides specified by the user via the CRD. object_overrides: &'a ObjectOverrides, + + /// The indices of the object_overrides entries that matched at least one of + /// the added resources. + matched_object_overrides: HashSet, } impl<'a> ClusterResources<'a> { @@ -499,6 +503,7 @@ impl<'a> ClusterResources<'a> { resource_ids: HashSet::default(), apply_strategy, object_overrides, + matched_object_overrides: HashSet::default(), }) } @@ -570,10 +575,12 @@ impl<'a> ClusterResources<'a> { let mut mutated = resource.maybe_mutate(&self.apply_strategy); - // We apply the object overrides of the user at the very end to offer maximum flexibility. - self.object_overrides + let matched_object_overrides = self + .object_overrides .apply_to(&mut mutated) .context(ApplyObjectOverridesSnafu)?; + self.matched_object_overrides + .extend(matched_object_overrides); let patched_resource = self .apply_strategy @@ -657,6 +664,8 @@ impl<'a> ClusterResources<'a> { /// /// * `client` - The client which is used to access Kubernetes pub async fn delete_orphaned_resources(self, client: &Client) -> Result<()> { + self.warn_about_unmatched_object_overrides(); + // We can only delete Listeners in case the "crds" feature is enabled, otherwise it's a NOP. #[cfg(feature = "crds")] let delete_listeners = self @@ -681,6 +690,40 @@ impl<'a> ClusterResources<'a> { Ok(()) } + /// Warns about every object override that did not match any of the added resources. + fn warn_about_unmatched_object_overrides(&self) { + for (index, object_override) in self + .object_overrides + .unmatched(&self.matched_object_overrides) + { + let (api_version, kind) = object_override + .types + .as_ref() + .map_or(("", ""), |types| { + (types.api_version.as_str(), types.kind.as_str()) + }); + let name = object_override + .metadata + .name + .as_deref() + .unwrap_or(""); + let namespace = object_override + .metadata + .namespace + .as_deref() + .unwrap_or(""); + + warn!( + "The objectOverride at index {index} (apiVersion: {api_version:?}, kind: \ + {kind:?}, metadata.name: {name:?}, metadata.namespace: {namespace:?}) did not \ + match any object created for this cluster and therefore had no effect. Please \ + check that apiVersion, kind and metadata.name are correct and that \ + metadata.namespace is set to {cluster_namespace:?}.", + cluster_namespace = self.namespace, + ); + } + } + /// Deletes all deployed resources of the given kind which are labelled as if they belong to /// this cluster instance but are not contained in the given list. /// diff --git a/crates/stackable-operator/src/deep_merger/crd.rs b/crates/stackable-operator/src/deep_merger/crd.rs index d0099d835..9bfb2e81b 100644 --- a/crates/stackable-operator/src/deep_merger/crd.rs +++ b/crates/stackable-operator/src/deep_merger/crd.rs @@ -1,3 +1,5 @@ +use std::collections::HashSet; + use k8s_openapi::DeepMerge; use kube::api::DynamicObject; use schemars::JsonSchema; @@ -27,13 +29,31 @@ impl ObjectOverrides { /// /// Merges are only applied to objects that have the same apiVersion, kind, name /// and namespace. - pub fn apply_to(&self, base: &mut R) -> Result<(), super::Error> + /// + /// Returns the indices of the entries that matched `base` and were therefore merged into it. + pub fn apply_to(&self, base: &mut R) -> Result, super::Error> where R: kube::Resource + DeepMerge + DeserializeOwned, { - for object_override in &self.0 { - apply_deep_merge(base, object_override)?; + let mut matched_indices = Vec::new(); + + for (index, object_override) in self.0.iter().enumerate() { + if apply_deep_merge(base, object_override)? { + matched_indices.push(index); + } } - Ok(()) + + Ok(matched_indices) + } + + /// Returns all entries (and their index) that are not contained in `matched_indices`. + pub fn unmatched<'a>( + &'a self, + matched_indices: &'a HashSet, + ) -> impl Iterator { + self.0 + .iter() + .enumerate() + .filter(move |(index, _)| !matched_indices.contains(index)) } } diff --git a/crates/stackable-operator/src/deep_merger/mod.rs b/crates/stackable-operator/src/deep_merger/mod.rs index f167f3a4e..3863bcb02 100644 --- a/crates/stackable-operator/src/deep_merger/mod.rs +++ b/crates/stackable-operator/src/deep_merger/mod.rs @@ -23,32 +23,34 @@ pub enum Error { /// Merges are only applied to objects that have the same apiVersion, kind, name /// and namespace. /// +/// Returns whether the merge matched the base object and was therefore applied. +/// /// In case the merge matches the base object, it will get cloned prior to merging. /// We modeled it this way, as most of the time it won't match, so we don't need to proactively /// clone. -pub fn apply_deep_merge(base: &mut R, merge: &DynamicObject) -> Result<(), Error> +pub fn apply_deep_merge(base: &mut R, merge: &DynamicObject) -> Result where R: kube::Resource + DeepMerge + DeserializeOwned, { let Some(merge_type) = &merge.types else { - return Ok(()); + return Ok(false); }; if merge_type.api_version != R::api_version(&()) || merge_type.kind != R::kind(&()) { - return Ok(()); + return Ok(false); } let Some(merge_name) = &merge.metadata.name else { - return Ok(()); + return Ok(false); }; // The name always needs to match if &base.name_any() != merge_name { - return Ok(()); + return Ok(false); } // If there is a namespace on the base object, it needs to match as well // Note that it is not set for cluster-scoped objects. if base.namespace() != merge.metadata.namespace { - return Ok(()); + return Ok(false); } let deserialized_merge = merge @@ -61,12 +63,15 @@ where })?; base.merge_from(deserialized_merge); - Ok(()) + Ok(true) } #[cfg(test)] mod tests { - use std::{collections::BTreeMap, vec}; + use std::{ + collections::{BTreeMap, HashSet}, + vec, + }; use indoc::indoc; use k8s_openapi::{ @@ -230,6 +235,63 @@ mod tests { assert_eq!(sa, original, "The merge shouldn't have changed anything"); } + #[test] + fn service_account_not_merged_as_namespace_missing() { + let mut sa = generate_service_account(); + let object_overrides: ObjectOverrides = serde_yaml::from_str(indoc! {" + - apiVersion: v1 + kind: ServiceAccount + metadata: + name: trino-serviceaccount + # namespace omitted, so it does not match the namespaced base object + labels: + app.kubernetes.io/name: overwritten + foo: bar + "}) + .expect("test YAML is valid"); + + let original = sa.clone(); + let matched_indices = object_overrides + .apply_to(&mut sa) + .expect("merging onto test object works"); + assert_eq!(sa, original, "The merge shouldn't have changed anything"); + assert_eq!(matched_indices, Vec::::new()); + } + + #[test] + fn unmatched_overrides_are_reported() { + let mut sa = generate_service_account(); + let object_overrides: ObjectOverrides = serde_yaml::from_str(indoc! {" + - apiVersion: v1 + kind: ServiceAccount + metadata: + name: trino-serviceaccount + namespace: default + labels: + foo: bar + - apiVersion: v1 + kind: ServiceAccount + metadata: + name: trino-serviceaccount-typo # name mismatch + namespace: default + "}) + .expect("test YAML is valid"); + + let matched_indices = object_overrides + .apply_to(&mut sa) + .expect("merging onto test object works"); + assert_eq!(matched_indices, vec![0]); + + let unmatched = object_overrides + .unmatched(&HashSet::from_iter(matched_indices)) + .map(|(index, object_override)| (index, object_override.metadata.name.clone())) + .collect::>(); + assert_eq!( + unmatched, + vec![(1, Some("trino-serviceaccount-typo".to_owned()))] + ); + } + #[test] fn service_account_not_merged_as_different_api_version() { let mut sa = generate_service_account();