feat: Adds a warning when a defined object override was not applied

crates/stackable-operator/CHANGELOG.md

@@ -4,6 +4,15 @@ 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.
+- `metadata.namespace` on an `objectOverrides` entry is now optional and defaults to the namespace of the object it is merged into ([#1264]).
+
+[#1264]: https://github.com/stackabletech/operator-rs/pull/1264
+
 ## [0.116.0] - 2026-08-14
 
 ### Added

crates/stackable-operator/crds/DummyCluster.yaml

@@ -1776,6 +1776,9 @@ spec:
                   creates.
 
                   List entries are arbitrary YAML objects, which need to be valid Kubernetes objects.
+                  An entry is merged into every object with the same apiVersion, kind and name. The
+                  `metadata.namespace` field is optional, it defaults to the namespace of the object the
+                  entry is merged into.
 
                   Read the [Object overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#object-overrides)
                   for more information.

crates/stackable-operator/crds/Listener.yaml

@@ -45,6 +45,9 @@ spec:
                   creates.
 
                   List entries are arbitrary YAML objects, which need to be valid Kubernetes objects.
+                  An entry is merged into every object with the same apiVersion, kind and name. The
+                  `metadata.namespace` field is optional, it defaults to the namespace of the object the
+                  entry is merged into.
 
                   Read the [Object overrides documentation](https://docs.stackable.tech/home/nightly/concepts/overrides#object-overrides)
                   for more information.

crates/stackable-operator/src/cluster_resources.rs

@@ -445,6 +445,11 @@ pub struct ClusterResources<'a> {
 
     /// Arbitrary Kubernetes object overrides specified by the user via the CRD.
     object_overrides: &'a ObjectOverrides,
+
+    /// The indices of the [`ObjectOverrides`] entries that matched at least one of the added
+    /// resources. Entries that never matched anything are warned about in
+    /// [`ClusterResources::delete_orphaned_resources`].
+    matched_object_overrides: HashSet<usize>,
 }
 
 impl<'a> ClusterResources<'a> {
@@ -499,6 +504,7 @@ impl<'a> ClusterResources<'a> {
             resource_ids: HashSet::default(),
             apply_strategy,
             object_overrides,
+            matched_object_overrides: HashSet::default(),
         })
     }
 
@@ -570,10 +576,27 @@ 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
+        // Every object is expected to be created in the namespace of the cluster. Object overrides
+        // without a `metadata.namespace` as well as the deletion of orphaned resources rely on this.
+        if mutated.namespace().as_deref() != Some(self.namespace.as_str()) {
+            warn!(
+                "The {kind} {name:?} is created in namespace {object_namespace:?} instead of the \
+                namespace of the cluster ({cluster_namespace:?}). This is a bug in the operator: \
+                objectOverrides without a metadata.namespace can match it unintentionally and it \
+                is never deleted once it becomes orphaned.",
+                kind = T::kind(&()),
+                name = mutated.name_any(),
+                object_namespace = mutated.namespace(),
+                cluster_namespace = self.namespace,
+            );
+        }
+
+        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 +680,10 @@ impl<'a> ClusterResources<'a> {
     ///
     /// * `client` - The client which is used to access Kubernetes
     pub async fn delete_orphaned_resources(self, client: &Client) -> Result<()> {
+        // All resources of this cluster have been added at this point, so we now know which object
+        // overrides did not match anything.
+        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 +708,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(("<not set>", "<not set>"), |types| {
+                    (types.api_version.as_str(), types.kind.as_str())
+                });
+            let name = object_override
+                .metadata
+                .name
+                .as_deref()
+                .unwrap_or("<not set>");
+            let namespace = object_override
+                .metadata
+                .namespace
+                .as_deref()
+                .unwrap_or("<not set>");
+
+            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 either not set or 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.
     ///

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;
@@ -12,6 +14,9 @@ pub struct ObjectOverrides(
     /// creates.
     ///
     /// List entries are arbitrary YAML objects, which need to be valid Kubernetes objects.
+    /// An entry is merged into every object with the same apiVersion, kind and name. The
+    /// `metadata.namespace` field is optional, it defaults to the namespace of the object the
+    /// entry is merged into.
     ///
     /// Read the [Object overrides documentation](DOCS_BASE_URL_PLACEHOLDER/concepts/overrides#object-overrides)
     /// for more information.
@@ -25,15 +30,36 @@ impl ObjectOverrides {
     /// Takes an arbitrary Kubernetes object (`base`) and applies the configured list of deep merges
     /// to it.
     ///
-    /// Merges are only applied to objects that have the same apiVersion, kind, name
-    /// and namespace.
-    pub fn apply_to<R>(&self, base: &mut R) -> Result<(), super::Error>
+    /// Merges are only applied to objects that have the same apiVersion, kind and name. A merge
+    /// with a namespace additionally needs to match the namespace of `base`, an omitted namespace
+    /// matches any namespace.
+    ///
+    /// Returns the indices of the entries that matched `base` and were therefore merged into it.
+    /// Callers that apply the overrides can collect these indices and pass them to
+    /// [`ObjectOverrides::unmatched`] afterwards, to warn about entries that never matched anything.
+    pub fn apply_to<R>(&self, base: &mut R) -> Result<Vec<usize>, super::Error>
     where
         R: kube::Resource<DynamicType = ()> + 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<usize>,
+    ) -> impl Iterator<Item = (usize, &'a DynamicObject)> {
+        self.0
+            .iter()
+            .enumerate()
+            .filter(move |(index, _)| !matched_indices.contains(index))
     }
 }

crates/stackable-operator/src/deep_merger/mod.rs

@@ -20,35 +20,41 @@ pub enum Error {
 
 /// Takes an arbitrary Kubernetes object (`base`) and applies the deep merge.
 ///
-/// Merges are only applied to objects that have the same apiVersion, kind, name
-/// and namespace.
+/// Merges are only applied to objects that have the same apiVersion, kind and name. A merge with a
+/// namespace additionally needs to match the namespace of `base`, an omitted namespace matches any
+/// 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<R>(base: &mut R, merge: &DynamicObject) -> Result<(), Error>
+pub fn apply_deep_merge<R>(base: &mut R, merge: &DynamicObject) -> Result<bool, Error>
 where
     R: kube::Resource<DynamicType = ()> + 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(());
+    // If the merge has a namespace, it needs to match as well. An omitted namespace matches any
+    // namespace, this function does not know which namespace to expect.
+    //
+    // Note that the base namespace is not set for cluster-scoped objects, in which case a merge
+    // with a namespace never matches.
+    if merge.metadata.namespace.is_some() && base.namespace() != merge.metadata.namespace {
+        return Ok(false);
     }
 
     let deserialized_merge = merge
@@ -61,12 +67,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 +239,102 @@ mod tests {
         assert_eq!(sa, original, "The merge shouldn't have changed anything");
     }
 
+    #[test]
+    fn service_account_merged_as_namespace_defaulted() {
+        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 defaults to the namespace of the base object
+                labels:
+                  app.kubernetes.io/name: overwritten
+                  foo: bar
+        "})
+        .expect("test YAML is valid");
+
+        assert_has_label(&sa, "app.kubernetes.io/name", "trino");
+        let matched_indices = object_overrides
+            .apply_to(&mut sa)
+            .expect("merging onto test object works");
+        assert_has_label(&sa, "app.kubernetes.io/name", "overwritten");
+        assert_eq!(matched_indices, vec![0]);
+        assert_eq!(
+            sa.metadata.namespace.as_deref(),
+            Some("default"),
+            "The namespace of the base object shouldn't have been removed"
+        );
+    }
+
+    #[test]
+    fn cluster_scoped_object_not_merged_as_namespace_set() {
+        let mut storage_class: StorageClass = serde_yaml::from_str(indoc! {"
+            apiVersion: storage.k8s.io/v1
+            kind: StorageClass
+            metadata:
+              name: low-latency
+              labels:
+                foo: original
+            provisioner: csi-driver.example-vendor.example
+        "})
+        .expect("test YAML is valid");
+        let object_overrides: ObjectOverrides = serde_yaml::from_str(indoc! {"
+            - apiVersion: storage.k8s.io/v1
+              kind: StorageClass
+              metadata:
+                name: low-latency
+                namespace: default # the base object is cluster-scoped, so this never matches
+                labels:
+                  foo: overwritten
+        "})
+        .expect("test YAML is valid");
+
+        let original = storage_class.clone();
+        let matched_indices = object_overrides
+            .apply_to(&mut storage_class)
+            .expect("merging onto test object works");
+        assert_eq!(
+            storage_class, original,
+            "The merge shouldn't have changed anything"
+        );
+        assert_eq!(matched_indices, Vec::<usize>::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::<Vec<_>>();
+        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();