From 0a03d8cd3979bfb135e49636c207fcf4b3493622 Mon Sep 17 00:00:00 2001 From: Diego Braga Date: Mon, 13 Jul 2026 22:59:04 +0200 Subject: [PATCH] fix(archive): kind cross-check on definition-ref label resolution Step 1 of searchCompositionDefinition trusted the definition-ref labels blindly: if they ever pointed at a CompositionDefinition of a different kind (mislabeled instance, name reuse after delete/recreate) it would fetch the wrong chart. Verify status.kind against the instance kind via getChartVersionKind before using the referenced definition; on mismatch (or unreadable status kind) log a Warn and fall through to the exact version+kind match instead of using it or erroring. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/tools/archive/getter.go | 22 +++++++++++++++----- internal/tools/archive/getter_search_test.go | 14 +++++++++++++ 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/internal/tools/archive/getter.go b/internal/tools/archive/getter.go index 2801188..cba36fb 100644 --- a/internal/tools/archive/getter.go +++ b/internal/tools/archive/getter.go @@ -325,16 +325,28 @@ func (g *dynamicGetter) searchCompositionDefinition(gvr schema.GroupVersionResou refName := instanceLabels[compositionMeta.CompositionDefinitionNameLabel] refNamespace := instanceLabels[compositionMeta.CompositionDefinitionNamespaceLabel] if refName != "" && refNamespace != "" { + refMatched := false for i := range all.Items { el := &all.Items[i] - if el.GetName() == refName && el.GetNamespace() == refNamespace { - compositionDefinition = el - g.logger.Debug("Resolved composition definition via definition-ref labels", "compositionDefinitionName", refName, "compositionDefinitionNamespace", refNamespace, "gvr", gvr.String()) - found = true + if el.GetName() != refName || el.GetNamespace() != refNamespace { + continue + } + refMatched = true + // The ref labels identify the owner by name+namespace only: guard against + // them pointing at a definition serving a DIFFERENT kind (mislabeled + // instance, name reuse after delete/recreate), which would fetch the + // wrong chart. An unreadable status kind is treated as a mismatch. + _, kind, kindErr := getChartVersionKind(el) + if kindErr != nil || kind != mg.GetKind() { + g.logger.Warn("Definition-ref labels point at a composition definition of a different kind, falling back to version/kind matching", "compositionDefinitionName", refName, "compositionDefinitionNamespace", refNamespace, "expectedKind", mg.GetKind(), "foundKind", kind, "gvr", gvr.String()) break } + compositionDefinition = el + g.logger.Debug("Resolved composition definition via definition-ref labels", "compositionDefinitionName", refName, "compositionDefinitionNamespace", refNamespace, "gvr", gvr.String()) + found = true + break } - if !found { + if !refMatched { // Stale labels are possible: fall through to version/kind matching. g.logger.Debug("Definition-ref labels did not match any composition definition, falling back to version/kind matching", "compositionDefinitionName", refName, "compositionDefinitionNamespace", refNamespace, "gvr", gvr.String()) } diff --git a/internal/tools/archive/getter_search_test.go b/internal/tools/archive/getter_search_test.go index 382ddcc..64e3def 100644 --- a/internal/tools/archive/getter_search_test.go +++ b/internal/tools/archive/getter_search_test.go @@ -102,6 +102,20 @@ func TestSearchCompositionDefinition(t *testing.T) { definitionRefLabels("gone", "krateo-system")), wantName: "portal", }, + { + // (b ter) Kind guard: the ref labels point at an EXISTING CD that serves a + // DIFFERENT kind (mislabeled instance, name reuse after delete/recreate). + // Trusting the labels would fetch the wrong chart: fall through to the exact + // version+kind match on the true owner instead. + name: "ref labels pointing at a different-kind definition fall through to exact match", + definitions: []*unstructured.Unstructured{ + newSearchTestCD("portal", "krateo-system", "v1-5-11", "OtherApp"), + newSearchTestCD("true-owner", "krateo-system", "v1-5-11", "FireworksApp"), + }, + instance: newSearchTestComposition("FireworksApp", "v1-5-11", + definitionRefLabels("portal", "krateo-system")), + wantName: "true-owner", + }, { // (c) Version-bump wedge: label says v1-5-11, CD moved to v1-5-12, no ref labels. // Exactly one CD serves the kind -> unique-kind fallback unwedges the migration.