From ad4917ca9412bc8372af185c35b73f69064e7bc2 Mon Sep 17 00:00:00 2001 From: Jonathan Ogilvie Date: Sun, 8 Nov 2026 18:02:26 -0500 Subject: [PATCH 1/2] fix(simulate): render claims pinned to a revision that calls a changed function A Manual XR pinned to a revision that calls a changed function is rendered against that revision rather than deferred. Claims were missed: the pin check compared the revision's compositeTypeRef with the claim's kind, and the composition provider looked the pin up under the claim's key, while the render hands the provider the claim's backing XR. So a Manual claim of a legacy cluster-scoped XR stayed deferred. Add DiffProcessor.ResolveComposite, which returns the defaulted effective composite DiffSingleResource hands its composition provider (for a claim, its backing XR with the claim synced in), sharing one helper with the render so the two agree. pinnedFunctionChangeXRs resolves the revision for that composite and checks it against the composite's type, keeping the claim, and records the composite's key so the provider finds the pin. Fixes #516 Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: Jonathan Ogilvie --- internal/diff/diffprocessor/comp_processor.go | 62 +++++++++++---- .../diff/diffprocessor/comp_processor_test.go | 78 ++++++++++++++++--- internal/diff/diffprocessor/diff_processor.go | 63 +++++++++++---- internal/diff/testutils/mocks.go | 11 +++ 4 files changed, 170 insertions(+), 44 deletions(-) diff --git a/internal/diff/diffprocessor/comp_processor.go b/internal/diff/diffprocessor/comp_processor.go index 53c2970f..2cd08c07 100644 --- a/internal/diff/diffprocessor/comp_processor.go +++ b/internal/diff/diffprocessor/comp_processor.go @@ -501,7 +501,8 @@ func (p *DefaultCompDiffProcessor) processSingleComposition(ctx context.Context, // A dropped backing XR is reported under its claim, so it does not re-point on its own. Nor does its // pin apply to the claim's render, which is handed the backing XR (see collectXRDiffs): the claim's - // spec, policy included, is what Crossplane syncs into that XR. + // spec, policy included, is what Crossplane syncs into that XR, so only the claim's own pin, resolved + // for that synced XR, does. maps.DeleteFunc(repointingXRs, func(key string, _ bool) bool { return !slices.ContainsFunc(keptXRs, isKey(key)) }) maps.DeleteFunc(pinnedXRs, func(key string, _ pinnedResolution) bool { return !slices.ContainsFunc(keptXRs, isKey(key)) }) @@ -655,8 +656,8 @@ func objectRef(res *un.Unstructured) corev1.ObjectReference { // another kind) in scope, read from the binding Crossplane records on either side (see managedBy), so the // change is reported once, under the claim. A filtered XR is removed whenever its claim is in scope. A // kept XR is removed only when its claim is kept too: one whose claim is filtered, such as a Manual claim -// whose backing XR is pinned to a revision calling a changed function, is still rendered, since nothing -// else reports it. +// naming a revision that calls no changed function while its backing XR is still pinned to one that +// does, is still rendered, since nothing else reports it. func withoutClaimedXRs(kept []*un.Unstructured, dropped []filteredXR, xrKind string) ([]*un.Unstructured, []filteredXR) { boundIn := func(claims []*un.Unstructured) func(*un.Unstructured) bool { return func(xr *un.Unstructured) bool { @@ -707,7 +708,8 @@ func overlapGroups(composites []renderedComposite) []dt.XRDiffGroup { // cliCompositionRevision for which composites are rendered with that name. // // pinned, keyed by dt.MakeDiffKeyFromResource, holds the composites rendered against their own pinned -// revision instead of cliComp; see pinnedFunctionChangeXRs. +// revision instead of cliComp, each with the key of the composite its render resolves the composition +// for; see pinnedFunctionChangeXRs. func (p *DefaultCompDiffProcessor) collectXRDiffs(ctx context.Context, xrs []*un.Unstructured, cliComp *apiextensionsv1.Composition, revisionName string, revisionLabels map[string]string, pinned map[string]pinnedResolution) map[string]*XRDiffResult { // Extract the target GVK from the CLI composition's compositeTypeRef cliCompTargetAPIVersion := cliComp.Spec.CompositeTypeRef.APIVersion @@ -729,6 +731,15 @@ func (p *DefaultCompDiffProcessor) collectXRDiffs(ctx context.Context, xrs []*un rootResourceKeys[key] = true } + // The provider is handed the composite the pin was resolved for, which for a claim is its backing + // XR, so it looks pins up under that composite's key. + pinnedComposites := make(map[string]dtypes.ResolvedComposition, len(pinned)) + for _, pin := range pinned { + if pin.err == nil { + pinnedComposites[pin.composite] = pin.resolved + } + } + // Composition provider that returns CLI composition for: // 1. Root-level resources (XRs and Claims that use this composition) // 2. XRs whose type matches the CLI composition's compositeTypeRef @@ -746,12 +757,12 @@ func (p *DefaultCompDiffProcessor) collectXRDiffs(ctx context.Context, xrs []*un // Check 1: Is this a root-level resource (XR or Claim supplied as affectedXRs to this composition)? // Root-level resources always use the CLI composition, even claims whose GVK differs from the XR type. key := dt.MakeDiffKeyFromResource(res) - if pin, ok := pinned[key]; ok { + if pin, ok := pinnedComposites[key]; ok { p.config.Logger.Debug("Resource is pinned to a revision calling a changed function, using that revision", "resource", resourceID, - "revision", pin.resolved.RevisionName) + "revision", pin.RevisionName) - return pin.resolved, nil + return pin, nil } if rootResourceKeys[key] { @@ -825,6 +836,10 @@ func (p *DefaultCompDiffProcessor) collectXRDiffs(ctx context.Context, xrs []*un type pinnedResolution struct { resolved dtypes.ResolvedComposition err error + + // composite is the dt.MakeDiffKeyFromResource of the composite the render resolves the composition + // for: the XR itself or, for a claim, its backing XR (see DiffProcessor.ResolveComposite). + composite string } // anyResolved reports whether any pinned lookup succeeded. @@ -844,6 +859,9 @@ func anyResolved(m map[string]pinnedResolution) bool { // revision instead of the diffed composition. It returns those XRs' resolutions (keyed by // dt.MakeDiffKeyFromResource), kept with the moved XRs appended, and dropped without them. // +// The revision is looked up for the composite the render resolves the composition for: the XR itself +// or, for a Manual claim, its backing XR with the claim synced in, whose pin is the claim's. +// // It does nothing when no function changed or under IncludeManual, which keeps Manual XRs and renders // them against the diffed composition. A Manual XR whose revision calls no changed function stays dropped. // A failed lookup is that XR's own error (a kept XR whose result is the error), not the run's. So with @@ -865,8 +883,14 @@ func (p *DefaultCompDiffProcessor) pinnedFunctionChangeXRs(ctx context.Context, continue } - resolved, err := p.compositionClient.FindMatchingComposition(ctx, d.xr) - if err == nil && !pinnedRevisionCallsChangedFunction(d.xr, resolved, p.config.ChangedFunctions) { + var resolved dtypes.ResolvedComposition + + composite, err := p.xrProc.ResolveComposite(ctx, d.xr) + if err == nil { + resolved, err = p.compositionClient.FindMatchingComposition(ctx, composite) + } + + if err == nil && !pinnedRevisionCallsChangedFunction(composite, resolved, p.config.ChangedFunctions) { remaining = append(remaining, d) continue } @@ -875,19 +899,23 @@ func (p *DefaultCompDiffProcessor) pinnedFunctionChangeXRs(ctx context.Context, pinned = make(map[string]pinnedResolution) } - pinned[dt.MakeDiffKeyFromResource(d.xr)] = pinnedResolution{resolved: resolved, err: errors.Wrapf(err, "cannot resolve the revision %s is pinned to", d.xr.GetName())} + pin := pinnedResolution{resolved: resolved, err: errors.Wrapf(err, "cannot resolve the revision %s is pinned to", d.xr.GetName())} + if err == nil { + pin.composite = dt.MakeDiffKeyFromResource(composite) + } + + pinned[dt.MakeDiffKeyFromResource(d.xr)] = pin kept = append(kept, d.xr) } return pinned, kept, remaining } -// pinnedRevisionCallsChangedFunction reports whether resolved is the pinned revision of xr and calls a -// changed function. A resolution with no revision name (the composition itself) or for another type -// (a claim, whose backing XR the render looks up under a different key) is not. -// TODO(jcogilvie): a pinned claim stays deferred, so a legacy cluster-scoped XR's claim pinned to a -// revision that calls a changed function is missed; route the backing XR's key to support it. -func pinnedRevisionCallsChangedFunction(xr *un.Unstructured, resolved dtypes.ResolvedComposition, changed sets.Set[string]) bool { +// pinnedRevisionCallsChangedFunction reports whether resolved is the pinned revision of composite and +// calls a changed function. composite is the one the render resolves the composition for, so for a +// claim it is the backing XR, whose type the revision's compositeTypeRef names. A resolution with no +// revision name (the composition itself) or for another type is not. +func pinnedRevisionCallsChangedFunction(composite *un.Unstructured, resolved dtypes.ResolvedComposition, changed sets.Set[string]) bool { if resolved.Composition == nil || resolved.RevisionName == "" { return false } @@ -896,7 +924,7 @@ func pinnedRevisionCallsChangedFunction(xr *un.Unstructured, resolved dtypes.Res ref := resolved.Composition.Spec.CompositeTypeRef refGV, err := schema.ParseGroupVersion(ref.APIVersion) - if err != nil || refGV.Group != xr.GroupVersionKind().Group || ref.Kind != xr.GetKind() { + if err != nil || refGV.Group != composite.GroupVersionKind().Group || ref.Kind != composite.GetKind() { return false } diff --git a/internal/diff/diffprocessor/comp_processor_test.go b/internal/diff/diffprocessor/comp_processor_test.go index ce35cf26..ef87f1b5 100644 --- a/internal/diff/diffprocessor/comp_processor_test.go +++ b/internal/diff/diffprocessor/comp_processor_test.go @@ -2355,6 +2355,22 @@ func TestDefaultCompDiffProcessor_processSingleComposition_PinnedXRWithChangedFu Build() } + // A Manual claim, whose render resolves the composition for its backing XR (see + // DiffProcessor.ResolveComposite): the revision's compositeTypeRef names the XR's kind, not the claim's. + claim := tu.NewResource("example.org/v1", "Claim", "pinned").InNamespace("ns"). + WithSpecField("crossplane", map[string]any{ + "compositionUpdatePolicy": "Manual", + "compositionRevisionRef": map[string]any{"name": "test-comp-rev1"}, + }). + Build() + resolveComposite := func(_ context.Context, res *un.Unstructured) (*un.Unstructured, error) { + if res.GetKind() == "Claim" { + return pinnedXR("pinned-xyz"), nil + } + + return res, nil + } + // The pinned revision's pipeline has a step the CLI composition lacks, so a render that saw the // CLI composition instead is detectable. revisionWith := func(fn string) *apiextensionsv1.Composition { @@ -2380,6 +2396,7 @@ func TestDefaultCompDiffProcessor_processSingleComposition_PinnedXRWithChangedFu lookupErr error includeManual bool unrelated bool + claim bool want want }{ "PinnedXRWithChangedFunctionRendersPinnedRevision": { @@ -2390,6 +2407,18 @@ func TestDefaultCompDiffProcessor_processSingleComposition_PinnedXRWithChangedFu revision: revisionWith("function-other"), want: want{Status: dt.XRStatusFiltered, FilterReason: dt.FilterReasonManualPolicy}, }, + // Issue #516: the revision is checked against the backing XR's type, and the render, handed the + // backing XR, finds the pin under its key. + "PinnedClaimWithChangedFunctionRendersPinnedRevision": { + revision: revisionWith("function-a"), + claim: true, + want: want{Status: dt.XRStatusUnchanged, Pinned: true, RenderedRevision: "test-comp-rev1", Pipeline: []string{"function-a"}}, + }, + "PinnedClaimWithUnrelatedFunctionChangeStaysDeferred": { + revision: revisionWith("function-other"), + claim: true, + want: want{Status: dt.XRStatusFiltered, FilterReason: dt.FilterReasonManualPolicy}, + }, "PinnedXRWithChangedFunctionRendersWhenCompositionCallsNone": { revision: revisionWith("function-a"), unrelated: true, @@ -2444,8 +2473,15 @@ func TestDefaultCompDiffProcessor_processSingleComposition_PinnedXRWithChangedFu ) proc.xrProc = &tu.MockDiffProcessor{ + ResolveCompositeFn: resolveComposite, + // Like the real one, the provider is handed the effective composite. DiffSingleResourceFn: func(ctx context.Context, xr *un.Unstructured, provider types.CompositionProvider) (map[string]*dt.ResourceDiff, map[string]*dt.ResourceDiff, error) { - resolved, err := provider(ctx, xr) + composite, err := resolveComposite(ctx, xr) + if err != nil { + return nil, nil, err + } + + resolved, err := provider(ctx, composite) if err != nil { return nil, nil, err } @@ -2459,7 +2495,12 @@ func TestDefaultCompDiffProcessor_processSingleComposition_PinnedXRWithChangedFu }, } - got, _, err := proc.processSingleComposition(t.Context(), cliComp.BuildAsUnstructured(), []*un.Unstructured{pinnedXR("pinned")}, false) + input := pinnedXR("pinned") + if tt.claim { + input = claim + } + + got, _, err := proc.processSingleComposition(t.Context(), cliComp.BuildAsUnstructured(), []*un.Unstructured{input}, false) if err != nil { t.Fatalf("processSingleComposition: %v", err) } @@ -2491,9 +2532,9 @@ func TestDefaultCompDiffProcessor_processSingleComposition_PinnedXRWithChangedFu } // TestDefaultCompDiffProcessor_processSingleComposition_PinnedBackingXROfFilteredClaim pins that the -// backing-XR drop runs after the pinned step: a Manual claim stays filtered (a pinned claim is -// deferred), so its backing XR, pinned to a revision calling a changed function, is the only entry that -// reports that change and must still be rendered against its pinned revision. +// backing-XR drop runs after the pinned step: a Manual claim naming a revision that calls no changed +// function stays filtered, so its backing XR, still pinned in the cluster to a revision calling a changed +// function, is the only entry that reports that change and must still be rendered against that revision. func TestDefaultCompDiffProcessor_processSingleComposition_PinnedBackingXROfFilteredClaim(t *testing.T) { cliComp := tu.NewComposition("test-comp"). WithCompositeTypeRef("example.org/v1", "XR"). @@ -2502,20 +2543,26 @@ func TestDefaultCompDiffProcessor_processSingleComposition_PinnedBackingXROfFilt manual := map[string]any{"compositionUpdatePolicy": "Manual", "compositionRevisionRef": map[string]any{"name": "test-comp-rev1"}} claim := tu.NewResource("example.org/v1", "Claim", "my-claim").InNamespace("ns"). - WithSpecField("crossplane", manual).Build() + WithSpecField("crossplane", map[string]any{"compositionUpdatePolicy": "Manual", "compositionRevisionRef": map[string]any{"name": "test-comp-rev0"}}).Build() backing := tu.NewResource("example.org/v1", "XR", "my-claim-xyz"). WithSpecField("compositionRef", map[string]any{"name": "test-comp"}). WithSpecField("crossplane", manual). WithSpecField("claimRef", map[string]any{"apiVersion": "example.org/v1", "kind": "Claim", "name": "my-claim", "namespace": "ns"}). Build() - revision := tu.NewComposition("test-comp").WithCompositeTypeRef("example.org/v1", "XR").WithPipelineMode(). - WithPipelineStep("step-old", "function-a", nil).Build() + // Revision 1 calls the changed function; revision 0, which the claim names, does not. + revisions := map[string]*apiextensionsv1.Composition{ + "test-comp-rev0": tu.NewComposition("test-comp").WithCompositeTypeRef("example.org/v1", "XR").WithPipelineMode(). + WithPipelineStep("step-old", "function-other", nil).Build(), + "test-comp-rev1": tu.NewComposition("test-comp").WithCompositeTypeRef("example.org/v1", "XR").WithPipelineMode(). + WithPipelineStep("step-old", "function-a", nil).Build(), + } client := tu.NewMockCompositionClient(). WithSuccessfulCompositionFetch(cliComp.Build()). - WithFindMatchingComposition(func(context.Context, *un.Unstructured) (types.ResolvedComposition, error) { - return types.ResolvedComposition{Composition: revision, RevisionName: "test-comp-rev1"}, nil + WithFindMatchingComposition(func(_ context.Context, res *un.Unstructured) (types.ResolvedComposition, error) { + name, _, _ := un.NestedString(res.Object, "spec", "crossplane", "compositionRevisionRef", "name") + return types.ResolvedComposition{Composition: revisions[name], RevisionName: name}, nil }). Build() @@ -2524,6 +2571,17 @@ func TestDefaultCompDiffProcessor_processSingleComposition_PinnedBackingXROfFilt rendered := map[string]string{} proc.xrProc = &tu.MockDiffProcessor{ + // The claim's effective composite is its backing XR with the claim's revision synced in. + ResolveCompositeFn: func(_ context.Context, res *un.Unstructured) (*un.Unstructured, error) { + if res.GetKind() != "Claim" { + return res, nil + } + + synced := backing.DeepCopy() + _ = un.SetNestedField(synced.Object, "test-comp-rev0", "spec", "crossplane", "compositionRevisionRef", "name") + + return synced, nil + }, DiffSingleResourceFn: func(ctx context.Context, xr *un.Unstructured, provider types.CompositionProvider) (map[string]*dt.ResourceDiff, map[string]*dt.ResourceDiff, error) { resolved, err := provider(ctx, xr) if err != nil { diff --git a/internal/diff/diffprocessor/diff_processor.go b/internal/diff/diffprocessor/diff_processor.go index 5ff00b9e..f52482bc 100644 --- a/internal/diff/diffprocessor/diff_processor.go +++ b/internal/diff/diffprocessor/diff_processor.go @@ -56,6 +56,10 @@ type DiffProcessor interface { // which only the render-overlap check consumes. DiffSingleResource(ctx context.Context, res *un.Unstructured, compositionProvider types.CompositionProvider) (map[string]*dt.ResourceDiff, map[string]*dt.ResourceDiff, error) + // ResolveComposite returns the composite DiffSingleResource hands its composition provider for res: + // the defaulted effective composite (see CompositeResolver), which for a claim is its backing XR. + ResolveComposite(ctx context.Context, res *un.Unstructured) (*un.Unstructured, error) + // Initialize prepares the processor's providers before the first diff. Initialize(ctx context.Context) error @@ -365,6 +369,46 @@ func (p *DefaultDiffProcessor) DiffSingleResource(ctx context.Context, res *un.U return diffs, rendered.Unchanged(), err } +// ResolveComposite returns the defaulted effective composite of res; see DiffProcessor. +func (p *DefaultDiffProcessor) ResolveComposite(ctx context.Context, res *un.Unstructured) (*un.Unstructured, error) { + resourceID := fmt.Sprintf("%s/%s", res.GetKind(), res.GetName()) + + authoredXR, _, err := p.SanitizeXR(res, resourceID) + if err != nil { + return nil, err + } + + _, defaultedXR, err := p.resolveComposite(ctx, authoredXR, nil, resourceID) + + return defaultedXR, err +} + +// resolveComposite returns the effective composite of authoredXR and its defaulted effective XR, from +// which the composition is resolved and which render consumes. existing is as for Resolve. +func (p *DefaultDiffProcessor) resolveComposite(ctx context.Context, authoredXR *cmp.Unstructured, existing *un.Unstructured, resourceID string) (EffectiveComposite, *un.Unstructured, error) { + // What Crossplane would actually reconcile: the input over its cluster copy or, for a claim, its + // backing XR with the claim synced in. Computed before anything reads the composite's composition, + // revision or update policy, so that resolving the composition and rendering agree on them. + composite, err := p.compositeResolver.Resolve(ctx, authoredXR, existing) + if err != nil { + p.config.Logger.Debug("Failed to resolve the effective composite", "resource", resourceID, "error", err) + return EffectiveComposite{}, nil, errors.Wrap(err, "cannot resolve the effective composite") + } + + // Default the effective XR the way the apiserver would, so that composition resolution and render + // see the spec Crossplane would see. This follows inheriting the cluster's fields, so that a default + // never overrides a value the cluster already holds (a Manual policy, say). The policy is strict: + // rendering without those defaults could produce a wrong diff. The storage version it reports is + // only for describing a predicted addition (see predictLocally), so it is not wanted here. + defaultedXR, _, err := p.renderDefaulter.Default(ctx, composite.Effective) + if err != nil { + p.config.Logger.Debug("Failed to apply XRD defaults", "resource", resourceID, "error", err) + return EffectiveComposite{}, nil, errors.Wrap(err, "cannot apply XRD defaults") + } + + return composite, defaultedXR, nil +} + // diffSingleResourceInternal is the internal implementation that allows control over removal detection. // parentXR should be nil for root XRs, and the parent XR for nested XRs. // existing is res's cluster copy when the caller already has it (a nested XR found among its parent's @@ -386,28 +430,13 @@ func (p *DefaultDiffProcessor) diffSingleResourceInternal(ctx context.Context, r return nil, types.RenderedResources{}, err } - // What Crossplane would actually reconcile: the input over its cluster copy or, for a claim, its - // backing XR with the claim synced in. Computed before anything reads the composite's composition, - // revision or update policy, so that resolving the composition and rendering agree on them. - composite, err := p.compositeResolver.Resolve(ctx, authoredXR, existing) + composite, defaultedXR, err := p.resolveComposite(ctx, authoredXR, existing, resourceID) if err != nil { - p.config.Logger.Debug("Failed to resolve the effective composite", "resource", resourceID, "error", err) - return nil, types.RenderedResources{}, errors.Wrap(err, "cannot resolve the effective composite") + return nil, types.RenderedResources{}, err } p.warnIfDeleting(composite.Cluster, parentXR, resourceID) - // Default the effective XR the way the apiserver would, so that composition resolution and render - // see the spec Crossplane would see. This follows inheriting the cluster's fields, so that a default - // never overrides a value the cluster already holds (a Manual policy, say). The policy is strict: - // rendering without those defaults could produce a wrong diff. The storage version it reports is - // only for describing a predicted addition (see predictLocally), so it is not wanted here. - defaultedXR, _, err := p.renderDefaulter.Default(ctx, composite.Effective) - if err != nil { - p.config.Logger.Debug("Failed to apply XRD defaults", "resource", resourceID, "error", err) - return nil, types.RenderedResources{}, errors.Wrap(err, "cannot apply XRD defaults") - } - // Get the composition using the provided function. It is resolved from the defaulted effective XR, // so that it sees the revision a composite is pinned to and the update policy an XRD defaults. resolved, err := compositionProvider(ctx, defaultedXR) diff --git a/internal/diff/testutils/mocks.go b/internal/diff/testutils/mocks.go index 761055d5..6bcd364b 100644 --- a/internal/diff/testutils/mocks.go +++ b/internal/diff/testutils/mocks.go @@ -259,9 +259,20 @@ type MockDiffProcessor struct { InitializeFn func(ctx context.Context) error PerformDiffFn func(ctx context.Context, resources []*un.Unstructured, compositionProvider types.CompositionProvider) (*dt.XRReport, error) DiffSingleResourceFn func(ctx context.Context, res *un.Unstructured, compositionProvider types.CompositionProvider) (map[string]*dt.ResourceDiff, map[string]*dt.ResourceDiff, error) + ResolveCompositeFn func(ctx context.Context, res *un.Unstructured) (*un.Unstructured, error) CleanupFn func(ctx context.Context) error } +// ResolveComposite implements the DiffProcessor.ResolveComposite method. Unset, res is its own +// effective composite. +func (m *MockDiffProcessor) ResolveComposite(ctx context.Context, res *un.Unstructured) (*un.Unstructured, error) { + if m.ResolveCompositeFn != nil { + return m.ResolveCompositeFn(ctx, res) + } + + return res, nil +} + // Initialize implements the DiffProcessor interface. func (m *MockDiffProcessor) Initialize(ctx context.Context) error { if m.InitializeFn != nil { From 8dda7c28cdfc8f87fd1e692029c90d04639776fb Mon Sep 17 00:00:00 2001 From: Jonathan Ogilvie Date: Tue, 10 Nov 2026 17:26:31 -0500 Subject: [PATCH 2/2] test(simulate): cover a pinned claim calling a changed function Add TestSimulatePinnedClaimChangedFunction: a Manual claim of a legacy cluster-scoped XR whose backing XR is pinned to a revision calling a changed embedded function is rendered against that revision; one whose revision does not call it stays deferred. Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: Jonathan Ogilvie --- .../project/integration_plan_output_test.go | 112 ++++++++++++++++++ .../project/testdata/pinned-claim/claim.yaml | 15 +++ .../testdata/pinned-claim/downstream.yaml | 10 ++ .../pinned-claim/unchanged-composition.yaml | 32 +++++ .../project/testdata/pinned-claim/xr.yaml | 15 +++ .../project/testdata/pinned-claim/xrd.yaml | 26 ++++ .../embedded-claim/apis/composition.yaml | 16 +++ .../embedded-claim/cluster-function.yaml | 9 ++ .../embedded-claim/crossplane-project.yaml | 6 + .../functions/render/template.gotmpl | 9 ++ 10 files changed, 250 insertions(+) create mode 100644 cmd/crossplane/project/testdata/pinned-claim/claim.yaml create mode 100644 cmd/crossplane/project/testdata/pinned-claim/downstream.yaml create mode 100644 cmd/crossplane/project/testdata/pinned-claim/unchanged-composition.yaml create mode 100644 cmd/crossplane/project/testdata/pinned-claim/xr.yaml create mode 100644 cmd/crossplane/project/testdata/pinned-claim/xrd.yaml create mode 100644 cmd/crossplane/project/testdata/project/embedded-claim/apis/composition.yaml create mode 100644 cmd/crossplane/project/testdata/project/embedded-claim/cluster-function.yaml create mode 100644 cmd/crossplane/project/testdata/project/embedded-claim/crossplane-project.yaml create mode 100644 cmd/crossplane/project/testdata/project/embedded-claim/functions/render/template.gotmpl diff --git a/cmd/crossplane/project/integration_plan_output_test.go b/cmd/crossplane/project/integration_plan_output_test.go index dbed8e77..2f7de251 100644 --- a/cmd/crossplane/project/integration_plan_output_test.go +++ b/cmd/crossplane/project/integration_plan_output_test.go @@ -458,6 +458,118 @@ func TestSimulatePinnedChangedFunction(t *testing.T) { } } +// TestSimulatePinnedClaimChangedFunction is TestSimulatePinnedChangedFunction +// for a Manual claim of a legacy cluster-scoped XR: the pin lives on the +// claim's backing XR, and the claim is rendered against it (or left deferred) +// just as that XR would be. +func TestSimulatePinnedClaimChangedFunction(t *testing.T) { + t.Parallel() + + const pinned = "testdata/pinned-claim/" + + cases := map[string]struct { + reason string + // composition is the cluster's copy of the project's Composition, + // whose current revision the backing XR is pinned to. + composition string + // wantDeferred is whether the claim stays deferred; otherwise it is + // rendered against its backing XR's pinned revision. + wantDeferred bool + }{ + "PinnedClaimWithChangedFunctionRendersPinnedRevision": { + reason: "A Manual claim whose backing XR's pinned revision calls a changed function is rendered against that revision, not deferred.", + composition: "testdata/project/embedded-claim/apis/composition.yaml", + }, + "PinnedClaimWithUnchangedFunctionDeferred": { + reason: "A Manual claim whose backing XR's pinned revision calls no changed function stays deferred.", + composition: pinned + "unchanged-composition.yaml", + wantDeferred: true, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + + if reason := integrationEnvGap(); reason != "" { + t.Skip(reason) + } + + cluster := integration.Start(t, integration.Setup{ + CRDDirs: []string{"../../../internal/testutils/integration/testdata/comp/crds"}, + SetupFiles: []string{ + pinned + "xrd.yaml", + tc.composition, + "../../../internal/testutils/integration/testdata/comp/resources/functions.yaml", + "testdata/project/embedded-claim/cluster-function.yaml", + pinned + "claim.yaml", + }, + CrossplaneManagedResources: []integration.HierarchicalOwnershipRelation{ + integration.Composes(pinned+"xr.yaml", pinned+"downstream.yaml"), + }, + XRDAPIVersion: integration.XRDV1, // the XRD is v1, so Crossplane writes the XR's resourceRefs at spec.resourceRefs + }) + cfg := cluster.Config + + // The harness pins the backing XR to its Composition's current revision. + xr := &un.Unstructured{} + xr.SetAPIVersion("pinned.diff.example.org/v1alpha1") + xr.SetKind("XPinned") + if err := cluster.Client.Get(cluster.Context, client.ObjectKey{Name: "manual-claim-x1y2z"}, xr); err != nil { + t.Fatalf("cannot get the pinned backing XR: %v", err) + } + rev, _, _ := un.NestedString(xr.Object, "spec", "compositionRevisionRef", "name") + + base := simharness.WithRenderBinary([]string{ + "project", "simulate", "--timeout=" + simharness.DefaultTimeout.String(), + "-f", copyProjectFixture(t, "embedded-claim"), + "--cache-dir=" + t.TempDir(), "--skip-plan", "-n", "default", + }, simharness.LocalCrossplaneBinary(t)) + + var stdout, stderr bytes.Buffer + code, err := simharness.RunSimulateArgvWith(t, cfg, append(slices.Clip(base), "-o", "json"), &simulate.Hooks{}, &stdout, &stderr) + if err != nil { + t.Fatalf("\n%s\nsimulate: unexpected error: %v\nstderr:\n%s", tc.reason, err, stderr.String()) + } + var res simulate.Result + if err := json.Unmarshal(stdout.Bytes(), &res); err != nil { + t.Fatalf("\n%s\nsimulate -o json: output is not a simulate.Result: %v\n%s", tc.reason, err, stdout.String()) + } + if len(res.Composites) != 1 { + t.Fatalf("\n%s\nsimulate -o json: want 1 composite, got %+v", tc.reason, res.Composites) + } + c := res.Composites[0] + + if tc.wantDeferred { + if c.Status != simulate.CompositeDeferred || strings.HasPrefix(c.Detail, "rendered against") { + t.Errorf("\n%s\nsimulate -o json: want a deferred pinned claim, got %+v", tc.reason, c) + } + return + } + + if code != dp.ExitCodeDiffDetected { + t.Errorf("\n%s\nsimulate: want exit code %d, got %d", tc.reason, dp.ExitCodeDiffDetected, code) + } + if c.Status != simulate.CompositeChanged || !c.Pinned || c.Detail != "rendered against pinned revision "+rev+" with the changed functions" { + t.Errorf("\n%s\nsimulate -o json: want a changed pinned claim rendered against %s, got %+v", tc.reason, rev, c) + } + // The plan is rendered by the project's build of the function the + // pinned revision calls, which prefixes configData with "embedded-". + var found bool + for _, p := range res.Plans { + for _, ch := range p.Changes { + if ch.Path == "spec.forProvider.configData" && ch.Planned != nil && ch.Planned.Value == "embedded-existing-value" { + found = true + } + } + } + if !found { + t.Errorf("\n%s\nsimulate -o json: want a configData change to the embedded function's output, got plans %+v", tc.reason, res.Plans) + } + }) + } +} + // TestSimulateNameRejected runs the --name scopes simulate refuses: one // naming a composite no supplied Composition reaches, and a namespace/name // combined with --namespace. diff --git a/cmd/crossplane/project/testdata/pinned-claim/claim.yaml b/cmd/crossplane/project/testdata/pinned-claim/claim.yaml new file mode 100644 index 00000000..15b0a59c --- /dev/null +++ b/cmd/crossplane/project/testdata/pinned-claim/claim.yaml @@ -0,0 +1,15 @@ +# A Manual claim bound to xr.yaml. The pin lives on the backing XR alone. +apiVersion: pinned.diff.example.org/v1alpha1 +kind: Pinned +metadata: + name: manual-claim + namespace: default +spec: + coolField: existing-value + compositionUpdatePolicy: Manual + compositionRef: + name: xpinneds.pinned.diff.example.org + resourceRef: + apiVersion: pinned.diff.example.org/v1alpha1 + kind: XPinned + name: manual-claim-x1y2z diff --git a/cmd/crossplane/project/testdata/pinned-claim/downstream.yaml b/cmd/crossplane/project/testdata/pinned-claim/downstream.yaml new file mode 100644 index 00000000..3dd73eea --- /dev/null +++ b/cmd/crossplane/project/testdata/pinned-claim/downstream.yaml @@ -0,0 +1,10 @@ +# The composed resource of xr.yaml. +apiVersion: nop.example.org/v1alpha1 +kind: XDownstreamResource +metadata: + name: manual-claim-x1y2z + annotations: + crossplane.io/composition-resource-name: nop-resource +spec: + forProvider: + configData: existing-value diff --git a/cmd/crossplane/project/testdata/pinned-claim/unchanged-composition.yaml b/cmd/crossplane/project/testdata/pinned-claim/unchanged-composition.yaml new file mode 100644 index 00000000..3903a6fd --- /dev/null +++ b/cmd/crossplane/project/testdata/pinned-claim/unchanged-composition.yaml @@ -0,0 +1,32 @@ +# The cluster's Composition when its pipeline calls no function the project changed. +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: xpinneds.pinned.diff.example.org +spec: + compositeTypeRef: + apiVersion: pinned.diff.example.org/v1alpha1 + kind: XPinned + mode: Pipeline + pipeline: + - step: generate-resources + functionRef: + name: function-go-templating + input: + apiVersion: template.fn.crossplane.io/v1beta1 + kind: GoTemplate + source: Inline + inline: + template: | + apiVersion: nop.example.org/v1alpha1 + kind: XDownstreamResource + metadata: + name: {{ .observed.composite.resource.metadata.name }} + annotations: + gotemplating.fn.crossplane.io/composition-resource-name: nop-resource + spec: + forProvider: + configData: {{ .observed.composite.resource.spec.coolField }} + - step: automatically-detect-ready-composed-resources + functionRef: + name: function-auto-ready diff --git a/cmd/crossplane/project/testdata/pinned-claim/xr.yaml b/cmd/crossplane/project/testdata/pinned-claim/xr.yaml new file mode 100644 index 00000000..3cac95ea --- /dev/null +++ b/cmd/crossplane/project/testdata/pinned-claim/xr.yaml @@ -0,0 +1,15 @@ +# The backing XR of claim.yaml. The harness pins it to the Composition's current revision. +apiVersion: pinned.diff.example.org/v1alpha1 +kind: XPinned +metadata: + name: manual-claim-x1y2z +spec: + coolField: existing-value + compositionUpdatePolicy: Manual + compositionRef: + name: xpinneds.pinned.diff.example.org + claimRef: + apiVersion: pinned.diff.example.org/v1alpha1 + kind: Pinned + name: manual-claim + namespace: default diff --git a/cmd/crossplane/project/testdata/pinned-claim/xrd.yaml b/cmd/crossplane/project/testdata/pinned-claim/xrd.yaml new file mode 100644 index 00000000..57de4e01 --- /dev/null +++ b/cmd/crossplane/project/testdata/pinned-claim/xrd.yaml @@ -0,0 +1,26 @@ +# A legacy cluster-scoped XRD (v1, with claimNames) for the pinned-claim project simulate cases. +apiVersion: apiextensions.crossplane.io/v1 +kind: CompositeResourceDefinition +metadata: + name: xpinneds.pinned.diff.example.org +spec: + group: pinned.diff.example.org + names: + kind: XPinned + plural: xpinneds + claimNames: + kind: Pinned + plural: pinneds + versions: + - name: v1alpha1 + served: true + referenceable: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + coolField: + type: string diff --git a/cmd/crossplane/project/testdata/project/embedded-claim/apis/composition.yaml b/cmd/crossplane/project/testdata/project/embedded-claim/apis/composition.yaml new file mode 100644 index 00000000..4c24f7ca --- /dev/null +++ b/cmd/crossplane/project/testdata/project/embedded-claim/apis/composition.yaml @@ -0,0 +1,16 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: xpinneds.pinned.diff.example.org +spec: + compositeTypeRef: + apiVersion: pinned.diff.example.org/v1alpha1 + kind: XPinned + mode: Pipeline + pipeline: + - step: generate-resources + functionRef: + name: example-itprojectrender + - step: automatically-detect-ready-composed-resources + functionRef: + name: function-auto-ready diff --git a/cmd/crossplane/project/testdata/project/embedded-claim/cluster-function.yaml b/cmd/crossplane/project/testdata/project/embedded-claim/cluster-function.yaml new file mode 100644 index 00000000..42b38287 --- /dev/null +++ b/cmd/crossplane/project/testdata/project/embedded-claim/cluster-function.yaml @@ -0,0 +1,9 @@ +--- +# A cluster Function with the same name as the project's embedded function. The +# project's function must win; this one has no step input, so go-templating errors if it is used. +apiVersion: pkg.crossplane.io/v1beta1 +kind: Function +metadata: + name: example-itprojectrender +spec: + package: xpkg.crossplane.io/crossplane-contrib/function-go-templating:v0.11.0 diff --git a/cmd/crossplane/project/testdata/project/embedded-claim/crossplane-project.yaml b/cmd/crossplane/project/testdata/project/embedded-claim/crossplane-project.yaml new file mode 100644 index 00000000..b227feb7 --- /dev/null +++ b/cmd/crossplane/project/testdata/project/embedded-claim/crossplane-project.yaml @@ -0,0 +1,6 @@ +apiVersion: dev.crossplane.io/v1alpha1 +kind: Project +metadata: + name: it-project +spec: + repository: xpkg.crossplane.io/example/itproject diff --git a/cmd/crossplane/project/testdata/project/embedded-claim/functions/render/template.gotmpl b/cmd/crossplane/project/testdata/project/embedded-claim/functions/render/template.gotmpl new file mode 100644 index 00000000..81120499 --- /dev/null +++ b/cmd/crossplane/project/testdata/project/embedded-claim/functions/render/template.gotmpl @@ -0,0 +1,9 @@ +apiVersion: nop.example.org/v1alpha1 +kind: XDownstreamResource +metadata: + name: {{ .observed.composite.resource.metadata.name }} + annotations: + gotemplating.fn.crossplane.io/composition-resource-name: nop-resource +spec: + forProvider: + configData: embedded-{{ .observed.composite.resource.spec.coolField }}