From 4b343cd7e5faf8b01c13f28cf441fd46ccfa0afc Mon Sep 17 00:00:00 2001 From: Bham06 Date: Sat, 8 Aug 2026 10:37:11 +0100 Subject: [PATCH] render: Ignore the observed copy of the XR in --observed-resources A render run emits the composite resource as its first output document, so an --observed-resources file produced by a previous run usually contains a UID-less copy of the XR. Keyed by GVK, namespace and name, that copy overwrites the real XR in the engine's store and blanks its UID, which makes the ownership check drop every composed resource. The XR is already supplied as the positional argument, so drop the redundant copy when building the render request. Fixes #47 Signed-off-by: Bham06 --- cmd/crossplane/render/convert.go | 23 ++++- cmd/crossplane/render/convert_test.go | 120 ++++++++++++++++++++++++++ 2 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 cmd/crossplane/render/convert_test.go diff --git a/cmd/crossplane/render/convert.go b/cmd/crossplane/render/convert.go index f445cdd4..4a433126 100644 --- a/cmd/crossplane/render/convert.go +++ b/cmd/crossplane/render/convert.go @@ -57,7 +57,7 @@ func BuildCompositeRequest(in CompositionInputs) (*renderv1alpha1.RenderRequest, }) } - observedStructs, err := composedToStructs(in.ObservedResources) + observedStructs, err := composedToStructs(filterObservedXR(in.ObservedResources, in.CompositeResource)) if err != nil { return nil, errors.Wrap(err, "cannot convert observed resources to protobuf") } @@ -278,6 +278,27 @@ func asStructFromTyped(o runtime.Object) (*structpb.Struct, error) { return resource.AsStruct(u) } +// filterObservedXR drops any observed resource that is really the composite +// resource itself. A render run emits the XR as its first output document, so +// an --observed-resources file produced by a previous run usually contains a +// UID-less copy of the XR. Keyed by GVK+namespace+name, that copy would +// overwrite the real XR in the engine's store and blank its UID, causing the +// ownership check to drop every composed resource. The XR is already supplied +// as the positional argument, so the copy is redundant. See issue #47. +func filterObservedXR(observed []composed.Unstructured, xr *ucomposite.Unstructured) []composed.Unstructured { + out := make([]composed.Unstructured, 0, len(observed)) + for i := range observed { + o := observed[i] + if o.GroupVersionKind() == xr.GroupVersionKind() && + o.GetNamespace() == xr.GetNamespace() && + o.GetName() == xr.GetName() { + continue + } + out = append(out, o) + } + return out +} + func composedToStructs(resources []composed.Unstructured) ([]*structpb.Struct, error) { out := make([]*structpb.Struct, 0, len(resources)) for i := range resources { diff --git a/cmd/crossplane/render/convert_test.go b/cmd/crossplane/render/convert_test.go new file mode 100644 index 00000000..b5b1165f --- /dev/null +++ b/cmd/crossplane/render/convert_test.go @@ -0,0 +1,120 @@ +/* +Copyright 2026 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package render + +import ( + "fmt" + "testing" + + "github.com/google/go-cmp/cmp" + + "github.com/crossplane/crossplane-runtime/v2/pkg/resource/unstructured/composed" + ucomposite "github.com/crossplane/crossplane-runtime/v2/pkg/resource/unstructured/composite" +) + +func TestFilterObservedXR(t *testing.T) { + // composite builds a composite resource (XR) with the given identity. + composite := func(apiVersion, kind, namespace, name string) *ucomposite.Unstructured { + xr := ucomposite.New() + xr.SetAPIVersion(apiVersion) + xr.SetKind(kind) + xr.SetNamespace(namespace) + xr.SetName(name) + return xr + } + + // observed builds an observed composed resource with the given identity. + observed := func(apiVersion, kind, namespace, name string) composed.Unstructured { + o := composed.New() + o.SetAPIVersion(apiVersion) + o.SetKind(kind) + o.SetNamespace(namespace) + o.SetName(name) + return *o + } + + // keys projects resources to comparable "apiVersion/kind/namespace/name" + // strings so we compare identity rather than the underlying maps. + keys := func(rs []composed.Unstructured) []string { + out := make([]string, len(rs)) + for i := range rs { + out[i] = fmt.Sprintf("%s/%s/%s/%s", rs[i].GetAPIVersion(), rs[i].GetKind(), rs[i].GetNamespace(), rs[i].GetName()) + } + return out + } + + cases := map[string]struct { + xr *ucomposite.Unstructured + observed []composed.Unstructured + want []string + }{ + "DropsObservedCopyOfXR": { + // The observed file contains a copy of the XR (as `crossplane + // render` emits it) alongside a real composed resource. Only the + // composed resource should survive. + xr: composite("example.org/v1", "XR", "default", "my-xr"), + observed: []composed.Unstructured{ + observed("example.org/v1", "XR", "default", "my-xr"), + observed("example.org/v1", "Composed", "default", "cd-a"), + }, + want: []string{"example.org/v1/Composed/default/cd-a"}, + }, + "KeepsAllWhenNoXRCopy": { + xr: composite("example.org/v1", "XR", "default", "my-xr"), + observed: []composed.Unstructured{ + observed("example.org/v1", "Composed", "default", "cd-a"), + observed("example.org/v1", "Composed", "default", "cd-b"), + }, + want: []string{ + "example.org/v1/Composed/default/cd-a", + "example.org/v1/Composed/default/cd-b", + }, + }, + "DropsClusterScopedXRCopy": { + // Cluster-scoped XR: namespace is empty on both the XR and its copy. + xr: composite("example.org/v1", "XR", "", "my-xr"), + observed: []composed.Unstructured{ + observed("example.org/v1", "XR", "", "my-xr"), + observed("example.org/v1", "Composed", "", "cd-a"), + }, + want: []string{"example.org/v1/Composed//cd-a"}, + }, + "KeepsSameNameDifferentKind": { + // A resource sharing the XR's name and namespace but a different + // kind is not the XR and must be kept. + xr: composite("example.org/v1", "XR", "default", "my-xr"), + observed: []composed.Unstructured{ + observed("example.org/v1", "Other", "default", "my-xr"), + }, + want: []string{"example.org/v1/Other/default/my-xr"}, + }, + "EmptyObserved": { + xr: composite("example.org/v1", "XR", "default", "my-xr"), + observed: nil, + want: []string{}, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + got := filterObservedXR(tc.observed, tc.xr) + if diff := cmp.Diff(tc.want, keys(got)); diff != "" { + t.Errorf("filterObservedXR(): -want +got:\n%s", diff) + } + }) + } +}