Skip to content

Commit 5e5d619

Browse files
committed
chore: log when object overrides are not applied
1 parent 7b9f9ac commit 5e5d619

4 files changed

Lines changed: 156 additions & 14 deletions

File tree

crates/stackable-operator/CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,14 @@ All notable changes to this project will be documented in this file.
44

55
## [Unreleased]
66

7+
### Changed
8+
9+
- `ClusterResources` now warns about `objectOverrides` entries that did not match any of the objects it created ([#1264]).
10+
- BREAKING: To enable this, `apply_deep_merge` now returns whether the merge matched the base object and `ObjectOverrides::apply_to`
11+
returns the indices of the entries that matched.
12+
13+
[#1264]: https://github.com/stackabletech/operator-rs/pull/1264
14+
715
## [0.116.0] - 2026-08-14
816

917
### Added

crates/stackable-operator/src/cluster_resources.rs

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,11 @@ pub struct ClusterResources<'a> {
445445

446446
/// Arbitrary Kubernetes object overrides specified by the user via the CRD.
447447
object_overrides: &'a ObjectOverrides,
448+
449+
/// The indices of the [`ObjectOverrides`] entries that matched at least one of the added
450+
/// resources. Entries that never matched anything are warned about in
451+
/// [`ClusterResources::delete_orphaned_resources`].
452+
matched_object_overrides: HashSet<usize>,
448453
}
449454

450455
impl<'a> ClusterResources<'a> {
@@ -499,6 +504,7 @@ impl<'a> ClusterResources<'a> {
499504
resource_ids: HashSet::default(),
500505
apply_strategy,
501506
object_overrides,
507+
matched_object_overrides: HashSet::default(),
502508
})
503509
}
504510

@@ -570,10 +576,12 @@ impl<'a> ClusterResources<'a> {
570576

571577
let mut mutated = resource.maybe_mutate(&self.apply_strategy);
572578

573-
// We apply the object overrides of the user at the very end to offer maximum flexibility.
574-
self.object_overrides
579+
let matched_object_overrides = self
580+
.object_overrides
575581
.apply_to(&mut mutated)
576582
.context(ApplyObjectOverridesSnafu)?;
583+
self.matched_object_overrides
584+
.extend(matched_object_overrides);
577585

578586
let patched_resource = self
579587
.apply_strategy
@@ -657,6 +665,10 @@ impl<'a> ClusterResources<'a> {
657665
///
658666
/// * `client` - The client which is used to access Kubernetes
659667
pub async fn delete_orphaned_resources(self, client: &Client) -> Result<()> {
668+
// All resources of this cluster have been added at this point, so we now know which object
669+
// overrides did not match anything.
670+
self.warn_about_unmatched_object_overrides();
671+
660672
// We can only delete Listeners in case the "crds" feature is enabled, otherwise it's a NOP.
661673
#[cfg(feature = "crds")]
662674
let delete_listeners = self
@@ -681,6 +693,40 @@ impl<'a> ClusterResources<'a> {
681693
Ok(())
682694
}
683695

696+
/// Warns about every object override that did not match any of the added resources.
697+
fn warn_about_unmatched_object_overrides(&self) {
698+
for (index, object_override) in self
699+
.object_overrides
700+
.unmatched(&self.matched_object_overrides)
701+
{
702+
let (api_version, kind) = object_override
703+
.types
704+
.as_ref()
705+
.map_or(("<not set>", "<not set>"), |types| {
706+
(types.api_version.as_str(), types.kind.as_str())
707+
});
708+
let name = object_override
709+
.metadata
710+
.name
711+
.as_deref()
712+
.unwrap_or("<not set>");
713+
let namespace = object_override
714+
.metadata
715+
.namespace
716+
.as_deref()
717+
.unwrap_or("<not set>");
718+
719+
warn!(
720+
"The objectOverride at index {index} (apiVersion: {api_version:?}, kind: \
721+
{kind:?}, metadata.name: {name:?}, metadata.namespace: {namespace:?}) did not \
722+
match any object created for this cluster and therefore had no effect. Please \
723+
check that apiVersion, kind and metadata.name are correct and that \
724+
metadata.namespace is set to {cluster_namespace:?}.",
725+
cluster_namespace = self.namespace,
726+
);
727+
}
728+
}
729+
684730
/// Deletes all deployed resources of the given kind which are labelled as if they belong to
685731
/// this cluster instance but are not contained in the given list.
686732
///

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

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::collections::HashSet;
2+
13
use k8s_openapi::DeepMerge;
24
use kube::api::DynamicObject;
35
use schemars::JsonSchema;
@@ -27,13 +29,37 @@ impl ObjectOverrides {
2729
///
2830
/// Merges are only applied to objects that have the same apiVersion, kind, name
2931
/// and namespace.
30-
pub fn apply_to<R>(&self, base: &mut R) -> Result<(), super::Error>
32+
///
33+
/// Returns the indices of the entries that matched `base` and were therefore merged into it.
34+
/// Callers that apply the overrides can collect these indices and pass them to
35+
/// [`ObjectOverrides::unmatched`] afterwards, to warn about entries that never matched anything.
36+
pub fn apply_to<R>(&self, base: &mut R) -> Result<Vec<usize>, super::Error>
3137
where
3238
R: kube::Resource<DynamicType = ()> + DeepMerge + DeserializeOwned,
3339
{
34-
for object_override in &self.0 {
35-
apply_deep_merge(base, object_override)?;
40+
let mut matched_indices = Vec::new();
41+
42+
for (index, object_override) in self.0.iter().enumerate() {
43+
if apply_deep_merge(base, object_override)? {
44+
matched_indices.push(index);
45+
}
3646
}
37-
Ok(())
47+
48+
Ok(matched_indices)
49+
}
50+
51+
/// Returns all entries (and their index) that are not contained in `matched_indices`.
52+
///
53+
/// These entries did not match any of the objects they were applied to and therefore had no
54+
/// effect at all. Common causes are a missing or wrong `metadata.namespace`, a typo in
55+
/// `metadata.name` or a wrong `apiVersion` or `kind`.
56+
pub fn unmatched<'a>(
57+
&'a self,
58+
matched_indices: &'a HashSet<usize>,
59+
) -> impl Iterator<Item = (usize, &'a DynamicObject)> {
60+
self.0
61+
.iter()
62+
.enumerate()
63+
.filter(move |(index, _)| !matched_indices.contains(index))
3864
}
3965
}

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

Lines changed: 70 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,32 +23,34 @@ pub enum Error {
2323
/// Merges are only applied to objects that have the same apiVersion, kind, name
2424
/// and namespace.
2525
///
26+
/// Returns whether the merge matched the base object and was therefore applied.
27+
///
2628
/// In case the merge matches the base object, it will get cloned prior to merging.
2729
/// We modeled it this way, as most of the time it won't match, so we don't need to proactively
2830
/// clone.
29-
pub fn apply_deep_merge<R>(base: &mut R, merge: &DynamicObject) -> Result<(), Error>
31+
pub fn apply_deep_merge<R>(base: &mut R, merge: &DynamicObject) -> Result<bool, Error>
3032
where
3133
R: kube::Resource<DynamicType = ()> + DeepMerge + DeserializeOwned,
3234
{
3335
let Some(merge_type) = &merge.types else {
34-
return Ok(());
36+
return Ok(false);
3537
};
3638
if merge_type.api_version != R::api_version(&()) || merge_type.kind != R::kind(&()) {
37-
return Ok(());
39+
return Ok(false);
3840
}
3941
let Some(merge_name) = &merge.metadata.name else {
40-
return Ok(());
42+
return Ok(false);
4143
};
4244

4345
// The name always needs to match
4446
if &base.name_any() != merge_name {
45-
return Ok(());
47+
return Ok(false);
4648
}
4749

4850
// If there is a namespace on the base object, it needs to match as well
4951
// Note that it is not set for cluster-scoped objects.
5052
if base.namespace() != merge.metadata.namespace {
51-
return Ok(());
53+
return Ok(false);
5254
}
5355

5456
let deserialized_merge = merge
@@ -61,12 +63,15 @@ where
6163
})?;
6264
base.merge_from(deserialized_merge);
6365

64-
Ok(())
66+
Ok(true)
6567
}
6668

6769
#[cfg(test)]
6870
mod tests {
69-
use std::{collections::BTreeMap, vec};
71+
use std::{
72+
collections::{BTreeMap, HashSet},
73+
vec,
74+
};
7075

7176
use indoc::indoc;
7277
use k8s_openapi::{
@@ -230,6 +235,63 @@ mod tests {
230235
assert_eq!(sa, original, "The merge shouldn't have changed anything");
231236
}
232237

238+
#[test]
239+
fn service_account_not_merged_as_namespace_missing() {
240+
let mut sa = generate_service_account();
241+
let object_overrides: ObjectOverrides = serde_yaml::from_str(indoc! {"
242+
- apiVersion: v1
243+
kind: ServiceAccount
244+
metadata:
245+
name: trino-serviceaccount
246+
# namespace omitted, so it does not match the namespaced base object
247+
labels:
248+
app.kubernetes.io/name: overwritten
249+
foo: bar
250+
"})
251+
.expect("test YAML is valid");
252+
253+
let original = sa.clone();
254+
let matched_indices = object_overrides
255+
.apply_to(&mut sa)
256+
.expect("merging onto test object works");
257+
assert_eq!(sa, original, "The merge shouldn't have changed anything");
258+
assert_eq!(matched_indices, Vec::<usize>::new());
259+
}
260+
261+
#[test]
262+
fn unmatched_overrides_are_reported() {
263+
let mut sa = generate_service_account();
264+
let object_overrides: ObjectOverrides = serde_yaml::from_str(indoc! {"
265+
- apiVersion: v1
266+
kind: ServiceAccount
267+
metadata:
268+
name: trino-serviceaccount
269+
namespace: default
270+
labels:
271+
foo: bar
272+
- apiVersion: v1
273+
kind: ServiceAccount
274+
metadata:
275+
name: trino-serviceaccount-typo # name mismatch
276+
namespace: default
277+
"})
278+
.expect("test YAML is valid");
279+
280+
let matched_indices = object_overrides
281+
.apply_to(&mut sa)
282+
.expect("merging onto test object works");
283+
assert_eq!(matched_indices, vec![0]);
284+
285+
let unmatched = object_overrides
286+
.unmatched(&HashSet::from_iter(matched_indices))
287+
.map(|(index, object_override)| (index, object_override.metadata.name.clone()))
288+
.collect::<Vec<_>>();
289+
assert_eq!(
290+
unmatched,
291+
vec![(1, Some("trino-serviceaccount-typo".to_owned()))]
292+
);
293+
}
294+
233295
#[test]
234296
fn service_account_not_merged_as_different_api_version() {
235297
let mut sa = generate_service_account();

0 commit comments

Comments
 (0)