fix(archive): resolve CompositionDefinition via definition-ref labels + unique-kind fallback

internal/tools/archive/getter.go

@@ -315,20 +315,73 @@ func (g *dynamicGetter) searchCompositionDefinition(gvr schema.GroupVersionResou
 
 	compositionDefinition := &all.Items[0]
 	if tot > 1 {
+		instanceLabels := mg.GetLabels()
 		found := false
-		for _, el := range all.Items {
-			version, kind, err := getChartVersionKind(&el)
-			if err != nil {
-				g.logger.Debug("Failed to get chart version and kind", "error", err.Error(), "compositionDefinitionName", el.GetName(), "compositionDefinitionNamespace", el.GetNamespace(), "gvr", gvr.String())
-				continue
+
+		// 1. Authoritative: the definition-ref labels stamped on the composition instance.
+		// These identify the owning CompositionDefinition by name+namespace and survive
+		// chart-version bumps, unlike the composition-version label which is only migrated
+		// by a successful reconcile.
+		refName := instanceLabels[compositionMeta.CompositionDefinitionNameLabel]
+		refNamespace := instanceLabels[compositionMeta.CompositionDefinitionNamespaceLabel]
+		if refName != "" && refNamespace != "" {
+			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
+					break
+				}
+			}
+			if !found {
+				// 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())
+			}
+		}
+
+		// 2. Exact match on chart version + kind (previous behavior).
+		if !found {
+			for i := range all.Items {
+				el := &all.Items[i]
+				version, kind, err := getChartVersionKind(el)
+				if err != nil {
+					g.logger.Debug("Failed to get chart version and kind", "error", err.Error(), "compositionDefinitionName", el.GetName(), "compositionDefinitionNamespace", el.GetNamespace(), "gvr", gvr.String())
+					continue
+				}
+				if version == instanceLabels[compositionMeta.CompositionVersionLabel] && kind == mg.GetKind() {
+					compositionDefinition = el
+					g.logger.Debug("Found matching composition definition", "compositionDefinitionName", el.GetName(), "compositionDefinitionNamespace", el.GetNamespace(), "gvr", gvr.String())
+					found = true
+					break
+				}
+			}
+		}
+
+		// 3. Last resort: a single definition serving this kind. During a chart-version bump
+		// the definition's status version moves ahead of the instance's composition-version
+		// label (which only a successful reconcile would migrate), so an exact version match
+		// can never succeed and reconciliation wedges. Tolerate the skew when the owner is
+		// unambiguous.
+		if !found {
+			var sameKind []*unstructured.Unstructured
+			for i := range all.Items {
+				el := &all.Items[i]
+				_, kind, err := getChartVersionKind(el)
+				if err != nil {
+					continue
+				}
+				if kind == mg.GetKind() {
+					sameKind = append(sameKind, el)
+				}
 			}
-			if version == mg.GetLabels()[compositionMeta.CompositionVersionLabel] && kind == mg.GetKind() {
-				compositionDefinition = &el
-				g.logger.Debug("Found matching composition definition", "compositionDefinitionName", el.GetName(), "compositionDefinitionNamespace", el.GetNamespace(), "gvr", gvr.String())
+			if len(sameKind) == 1 {
+				compositionDefinition = sameKind[0]
+				g.logger.Warn("Resolved composition definition by unique kind, tolerating composition-version label skew", "compositionDefinitionName", compositionDefinition.GetName(), "compositionDefinitionNamespace", compositionDefinition.GetNamespace(), "expectedVersion", instanceLabels[compositionMeta.CompositionVersionLabel], "kind", mg.GetKind(), "gvr", gvr.String())
 				found = true
-				break
 			}
 		}
+
 		if !found {
 			return nil,
 				fmt.Errorf("too many definitions [%d] found for '%v' in namespace: %s", tot, gvr.String(), mg.GetNamespace())

internal/tools/archive/getter_search_test.go

@@ -0,0 +1,174 @@
+package archive
+
+import (
+	"strings"
+	"testing"
+
+	compositionMeta "github.com/krateoplatformops/composition-dynamic-controller/pkg/meta"
+	"github.com/krateoplatformops/unstructured-runtime/pkg/logging"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	dynamicfake "k8s.io/client-go/dynamic/fake"
+)
+
+// searchTestCDGVR is the GVR searchCompositionDefinition lists.
+var searchTestCDGVR = schema.GroupVersionResource{
+	Group:    "core.krateo.io",
+	Version:  "v1alpha1",
+	Resource: "compositiondefinitions",
+}
+
+// newSearchTestCD builds a CompositionDefinition with the status fields the matcher reads
+// (status.apiVersion carries the chart-version suffix, status.kind the served kind).
+func newSearchTestCD(name, namespace, statusVersion, statusKind string) *unstructured.Unstructured {
+	cd := &unstructured.Unstructured{}
+	cd.SetGroupVersionKind(schema.GroupVersionKind{Group: "core.krateo.io", Version: "v1alpha1", Kind: "CompositionDefinition"})
+	cd.SetNamespace(namespace)
+	cd.SetName(name)
+	_ = unstructured.SetNestedField(cd.Object, "composition.krateo.io/"+statusVersion, "status", "apiVersion")
+	_ = unstructured.SetNestedField(cd.Object, statusKind, "status", "kind")
+	return cd
+}
+
+// newSearchTestComposition builds a composition instance with the given kind and
+// composition-version label; extraLabels lets cases add the definition-ref labels.
+func newSearchTestComposition(kind, versionLabel string, extraLabels map[string]string) *unstructured.Unstructured {
+	lbl := map[string]string{
+		compositionMeta.CompositionVersionLabel: versionLabel,
+	}
+	for k, v := range extraLabels {
+		lbl[k] = v
+	}
+	mg := &unstructured.Unstructured{}
+	mg.SetAPIVersion("composition.krateo.io/" + versionLabel)
+	mg.SetKind(kind)
+	mg.SetNamespace("krateo-system")
+	mg.SetName("my-instance")
+	mg.SetLabels(lbl)
+	return mg
+}
+
+func definitionRefLabels(name, namespace string) map[string]string {
+	return map[string]string{
+		compositionMeta.CompositionDefinitionNameLabel:      name,
+		compositionMeta.CompositionDefinitionNamespaceLabel: namespace,
+	}
+}
+
+func TestSearchCompositionDefinition(t *testing.T) {
+	gvr := schema.GroupVersionResource{Group: "composition.krateo.io", Version: "v1-5-11", Resource: "fireworksapps"}
+
+	cases := []struct {
+		name        string
+		definitions []*unstructured.Unstructured
+		instance    *unstructured.Unstructured
+		wantName    string // expected resolved CompositionDefinition name; "" => expect error
+		wantErr     string // substring the error must contain when wantName == ""
+	}{
+		{
+			// (a) The instance's composition-version label lags the CD (chart bump in flight)
+			// AND two CDs serve the same kind, so neither exact-version nor unique-kind can
+			// resolve: only the definition-ref labels identify the owner.
+			name: "definition-ref labels win despite version skew and same-kind ambiguity",
+			definitions: []*unstructured.Unstructured{
+				newSearchTestCD("portal", "krateo-system", "v1-5-12", "FireworksApp"),
+				newSearchTestCD("portal-old", "krateo-system", "v1-5-10", "FireworksApp"),
+			},
+			instance: newSearchTestComposition("FireworksApp", "v1-5-11",
+				definitionRefLabels("portal", "krateo-system")),
+			wantName: "portal",
+		},
+		{
+			// (b) Regression: the pre-existing exact (version, kind) match still resolves
+			// when no definition-ref labels are present.
+			name: "exact version+kind match still works without ref labels",
+			definitions: []*unstructured.Unstructured{
+				newSearchTestCD("portal", "krateo-system", "v1-5-11", "FireworksApp"),
+				newSearchTestCD("other", "krateo-system", "v2-0-0", "OtherApp"),
+			},
+			instance: newSearchTestComposition("FireworksApp", "v1-5-11", nil),
+			wantName: "portal",
+		},
+		{
+			// (b bis) Stale definition-ref labels (owner renamed/deleted) must not wedge:
+			// fall through to the exact version+kind match.
+			name: "stale ref labels fall through to exact version match",
+			definitions: []*unstructured.Unstructured{
+				newSearchTestCD("portal", "krateo-system", "v1-5-11", "FireworksApp"),
+				newSearchTestCD("other", "krateo-system", "v2-0-0", "OtherApp"),
+			},
+			instance: newSearchTestComposition("FireworksApp", "v1-5-11",
+				definitionRefLabels("gone", "krateo-system")),
+			wantName: "portal",
+		},
+		{
+			// (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.
+			name: "unique-kind fallback tolerates version skew without ref labels",
+			definitions: []*unstructured.Unstructured{
+				newSearchTestCD("portal", "krateo-system", "v1-5-12", "FireworksApp"),
+				newSearchTestCD("other", "krateo-system", "v2-0-0", "OtherApp"),
+			},
+			instance: newSearchTestComposition("FireworksApp", "v1-5-11", nil),
+			wantName: "portal",
+		},
+		{
+			// (d) Two CDs serve the same kind, version skew, no ref labels: still ambiguous,
+			// keep the existing error rather than guessing.
+			name: "ambiguous same-kind definitions with version skew still error",
+			definitions: []*unstructured.Unstructured{
+				newSearchTestCD("portal", "krateo-system", "v1-5-12", "FireworksApp"),
+				newSearchTestCD("portal-old", "krateo-system", "v1-5-10", "FireworksApp"),
+			},
+			instance: newSearchTestComposition("FireworksApp", "v1-5-11", nil),
+			wantErr:  "too many definitions",
+		},
+		{
+			// tot == 1 fast path unchanged: the single CD is used even with version skew.
+			name: "single definition fast path unchanged",
+			definitions: []*unstructured.Unstructured{
+				newSearchTestCD("portal", "krateo-system", "v1-5-12", "FireworksApp"),
+			},
+			instance: newSearchTestComposition("FireworksApp", "v1-5-11", nil),
+			wantName: "portal",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			scheme := runtime.NewScheme()
+			gvrToListKind := map[schema.GroupVersionResource]string{searchTestCDGVR: "CompositionDefinitionList"}
+
+			objs := make([]runtime.Object, 0, len(tc.definitions))
+			for _, cd := range tc.definitions {
+				objs = append(objs, cd)
+			}
+			dyn := dynamicfake.NewSimpleDynamicClientWithCustomListKinds(scheme, gvrToListKind, objs...)
+
+			g := &dynamicGetter{
+				dynamicClient: dyn,
+				logger:        logging.NewNopLogger(),
+			}
+
+			got, err := g.searchCompositionDefinition(gvr, tc.instance)
+
+			if tc.wantName == "" {
+				if err == nil {
+					t.Fatalf("expected error containing %q, got definition %q", tc.wantErr, got.GetName())
+				}
+				if !strings.Contains(err.Error(), tc.wantErr) {
+					t.Fatalf("expected error containing %q, got: %v", tc.wantErr, err)
+				}
+				return
+			}
+
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if got.GetName() != tc.wantName {
+				t.Errorf("resolved definition = %q, want %q", got.GetName(), tc.wantName)
+			}
+		})
+	}
+}