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 1/5] 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(); From 9a2adee270b91d580cde02631dbcf7b4206dba0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCller?= Date: Tue, 18 Aug 2026 19:13:33 +0200 Subject: [PATCH 2/5] chore: add comment on late warning --- crates/stackable-operator/src/cluster_resources.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/stackable-operator/src/cluster_resources.rs b/crates/stackable-operator/src/cluster_resources.rs index f4da2e6a6..d5f0cde7f 100644 --- a/crates/stackable-operator/src/cluster_resources.rs +++ b/crates/stackable-operator/src/cluster_resources.rs @@ -664,6 +664,14 @@ impl<'a> ClusterResources<'a> { /// /// * `client` - The client which is used to access Kubernetes pub async fn delete_orphaned_resources(self, client: &Client) -> Result<()> { + // We warn late about unmatched object overrides, as every override is matched against + // each object individually (by apiVersion, kind, name and namespace). An override that + // e.g. targets the discovery ConfigMap will therefore not match any of the rolegroup + // ConfigMaps, so whether an override matched nothing at all can only be determined once + // all objects have been added. + // As this function consumes `self` and finalizes the cluster creation, it is the last + // point at which we can do so without requiring an extra call in every operator. + // The downside is that the warnings are lost in case reconciliation fails earlier. self.warn_about_unmatched_object_overrides(); // We can only delete Listeners in case the "crds" feature is enabled, otherwise it's a NOP. From c48736ce4943110fc03b78a25271e111c4eafd68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCller?= Date: Fri, 21 Aug 2026 08:03:53 +0200 Subject: [PATCH 3/5] fix: improve warning output --- crates/stackable-operator/src/cluster_resources.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/crates/stackable-operator/src/cluster_resources.rs b/crates/stackable-operator/src/cluster_resources.rs index d5f0cde7f..56d2cf92b 100644 --- a/crates/stackable-operator/src/cluster_resources.rs +++ b/crates/stackable-operator/src/cluster_resources.rs @@ -722,12 +722,15 @@ impl<'a> ClusterResources<'a> { .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:?}.", + index, + api_version, + kind, + metadata.name = name, + metadata.namespace = namespace, cluster_namespace = self.namespace, + "objectOverride 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 matches the cluster namespace." ); } } From 6033b8771af748e43f8c9ee878fc708847a25b8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCller?= Date: Fri, 21 Aug 2026 08:21:04 +0200 Subject: [PATCH 4/5] fix: consolidate changelog entries --- crates/stackable-operator/CHANGELOG.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/stackable-operator/CHANGELOG.md b/crates/stackable-operator/CHANGELOG.md index 06ad7e83b..0d47fab3c 100644 --- a/crates/stackable-operator/CHANGELOG.md +++ b/crates/stackable-operator/CHANGELOG.md @@ -6,9 +6,7 @@ All notable changes to this project will be documented in this file. ### 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. +- BREAKING: `ClusterResources` now warns about `objectOverrides` entries that did not match any of the objects it created. To enable this, the signatures of `apply_deep_merge` and `ObjectOverrides::apply_to` needed to be adjusted ([#1264]). [#1264]: https://github.com/stackabletech/operator-rs/pull/1264 From 1e3efa00bae50db57d3ad9ed7aa83563d12c52b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCller?= Date: Fri, 21 Aug 2026 08:26:43 +0200 Subject: [PATCH 5/5] chore: appease cargo-deny --- Cargo.lock | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a51eaa14d..1bbbcdd63 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1082,7 +1082,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -1383,9 +1383,9 @@ dependencies = [ [[package]] name = "h2" -version = "0.4.15" +version = "0.4.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6cb093c84e8bd9b188d4c4a8cb6579fc016968d14c99882163cd3ff402a4f155" +checksum = "839c0e8a181239723652be9062bb56ca5bf5f64011f73b623f6f4fc59086a228" dependencies = [ "atomic-waker", "bytes", @@ -3259,7 +3259,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -3316,7 +3316,7 @@ dependencies = [ "security-framework", "security-framework-sys", "webpki-root-certs", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -4100,7 +4100,7 @@ dependencies = [ "getrandom 0.4.3", "once_cell", "rustix", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] @@ -4876,7 +4876,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]]