diff --git a/cmd/crossplane/composition/integration_test.go b/cmd/crossplane/composition/integration_test.go index 6b7368fa..881cd4e0 100644 --- a/cmd/crossplane/composition/integration_test.go +++ b/cmd/crossplane/composition/integration_test.go @@ -2001,6 +2001,75 @@ Summary: 1 modified`, ExpectedExitCode: dp.ExitCodeDiffDetected, NoColor: true, }, + "NestedXRUsesRunComposition": { + Reason: "Validates that a nested XR whose Composition is in the run renders with the run's, not the cluster's", + SetupFiles: []string{ + "../../../internal/testutils/integration/testdata/comp/resources/xrd.yaml", + "../../../internal/testutils/integration/testdata/comp/resources/original-composition.yaml", + "../../../internal/testutils/integration/testdata/comp/resources/functions.yaml", + "testdata/comp/resources/nested-xr/parent-xrd.yaml", + "testdata/comp/resources/nested-xr/parent-composition.yaml", + }, + CrossplaneManagedResources: []integration.HierarchicalOwnershipRelation{ + { + OwnerFile: "testdata/comp/resources/nested-xr/existing-parent-xr.yaml", + OwnedFiles: map[string]*integration.HierarchicalOwnershipRelation{ + "testdata/comp/resources/nested-xr/existing-parent-direct-downstream.yaml": nil, + "testdata/comp/resources/nested-xr/existing-nested-child-xr.yaml": { + OwnedFiles: map[string]*integration.HierarchicalOwnershipRelation{ + "testdata/comp/resources/nested-xr/existing-child-downstream.yaml": nil, + }, + }, + }, + }, + }, + // Both the parent's and the child's Compositions change. + InputFiles: []string{"testdata/comp/updated-parent-composition.yaml", "../../../internal/testutils/integration/testdata/comp/updated-composition.yaml"}, + Namespace: "default", + // Under the parent, the nested XR's downstream is rendered with the run's child Composition, so + // it shows the same change as under the nested XR's own entry. + ExpectedOutput: `=== Impact Analysis === + +~~~ XDownstreamResource/test-parent-direct + apiVersion: ns.nop.example.org/v1alpha1 + kind: XDownstreamResource + metadata: + annotations: + crossplane.io/composition-resource-name: direct-resource + labels: + crossplane.io/composite: test-parent + name: test-parent-direct + namespace: default + spec: + forProvider: + configData: parent-parent-value +- resourceTier: parent-tier ++ resourceTier: premium-tier + +--- +~~~ XDownstreamResource/test-parent-nested + apiVersion: ns.nop.example.org/v1alpha1 + kind: XDownstreamResource + metadata: + annotations: + crossplane.io/composition-resource-name: nop-resource + labels: + crossplane.io/composite: test-parent + name: test-parent-nested + namespace: default + spec: + forProvider: +- configData: child-config-value +- resourceTier: basic ++ configData: updated-child-config-value ++ resourceTier: premium + +--- + +Summary: 2 modified`, + ExpectedExitCode: dp.ExitCodeDiffDetected, + NoColor: true, + }, "CrossNamespaceResourceCollision": { Reason: "Validates that resources with the same name in different namespaces are correctly distinguished", Skip: true, diff --git a/internal/diff/client/crossplane/revision_selector.go b/internal/diff/client/crossplane/revision_selector.go index c02a0735..3c083f29 100644 --- a/internal/diff/client/crossplane/revision_selector.go +++ b/internal/diff/client/crossplane/revision_selector.go @@ -166,6 +166,14 @@ func XRCompositionRevisionRef(obj map[string]any, apiVersion string) (string, er return name, err } +// XRCompositionRef reads the name in an XR/Claim's compositionRef, checking the paths XRUpdatePolicy +// does. Empty when the ref is unset. A non-nil error means the field is present but malformed (not a +// string). +func XRCompositionRef(obj map[string]any, apiVersion string) (string, error) { + name, _, err := nestedCrossplaneString(obj, apiVersion, "compositionRef", "name") + return name, err +} + // labelSelectorFromUnstructuredMap converts an unstructured compositionRevisionSelector map (as // stored on an XR spec) into a typed metav1.LabelSelector via runtime conversion. func labelSelectorFromUnstructuredMap(m map[string]any) (*metav1.LabelSelector, error) { diff --git a/internal/diff/diffprocessor/comp_processor.go b/internal/diff/diffprocessor/comp_processor.go index 53c2970f..dfc8b572 100644 --- a/internal/diff/diffprocessor/comp_processor.go +++ b/internal/diff/diffprocessor/comp_processor.go @@ -192,6 +192,8 @@ func (p *DefaultCompDiffProcessor) DiffComposition(ctx context.Context, composit compositionErrors int // composites are the successfully rendered composites of every composition, for the overlap check. composites []renderedComposite + // run holds every Composition of the run, so a nested XR of another type renders with its own. + run = p.runCompositions(ctx, compositions) ) // Process each composition, filtering out non-Composition objects @@ -247,7 +249,7 @@ func (p *DefaultCompDiffProcessor) DiffComposition(ctx context.Context, composit ) if err == nil { - compResult, rendered, err = p.processSingleComposition(ctx, comp, affectedXRs, scoped) + compResult, rendered, err = p.processSingleComposition(ctx, comp, affectedXRs, scoped, run) } composites = append(composites, rendered...) @@ -405,10 +407,11 @@ func (p *DefaultCompDiffProcessor) preflightResourceRefs(ctx context.Context, co // `affectedXRs` is the pre-resolved set of XRs to evaluate (caller decides via DiffComposition's // switch whether this comes from the --resource preflight or default-discovery via FindComposites). // `scoped` records that the set came from --resource rather than discovery (see CompositionDiff.Scoped). +// `run` holds the run's Compositions by name, for nested XRs of other types (see runCompositions). // // It also returns each composite it rendered without error, with its group of diffs and unchanged // renderings (which the impact analysis leaves out), for DiffComposition's render-overlap check. -func (p *DefaultCompDiffProcessor) processSingleComposition(ctx context.Context, newComp *un.Unstructured, affectedXRs []*un.Unstructured, scoped bool) (*dt.CompositionDiff, []renderedComposite, error) { +func (p *DefaultCompDiffProcessor) processSingleComposition(ctx context.Context, newComp *un.Unstructured, affectedXRs []*un.Unstructured, scoped bool, run map[string]runComposition) (*dt.CompositionDiff, []renderedComposite, error) { result := &dt.CompositionDiff{ Name: newComp.GetName(), Scoped: scoped, @@ -422,24 +425,22 @@ func (p *DefaultCompDiffProcessor) processSingleComposition(ctx context.Context, }, } - // First, calculate the composition diff itself - comparison, err := p.calculateCompositionDiff(ctx, newComp) - if err != nil { - return nil, nil, errors.Wrap(err, "cannot calculate composition diff") + // The composition diff itself, the typed composition and the identity of the revision it would + // produce come from the run (see runCompositions), so a nested XR of this composition's type renders + // under another composition's parent with the same revision this composition's own XRs get. + rc, ok := run[newComp.GetName()] + if !ok { + return nil, nil, errors.Errorf("composition %q is not in the run", newComp.GetName()) } - result.CompositionDiff = comparison.diff - result.MaskedChangesOnly = comparison.diff == nil && comparison.changed() - - cliComp := &apiextensionsv1.Composition{} - if err := runtime.DefaultUnstructuredConverter.FromUnstructured(newComp.Object, cliComp); err != nil { - return nil, nil, errors.Wrapf(err, "cannot convert composition %q to typed", newComp.GetName()) + if rc.err != nil { + return nil, nil, rc.err } - // Predict the identity of the revision this composition would produce. Needed twice below: to - // evaluate compositionRevisionSelectors against the label set the revision would carry, and to seed - // the composites so a template observing the revision name renders the value it would really get. - pred := predictRevision(cliComp) + comparison, cliComp, pred := rc.comparison, rc.comp, rc.pred + + result.CompositionDiff = comparison.diff + result.MaskedChangesOnly = comparison.diff == nil && comparison.changed() // Report what applying this composition does to CompositionRevisions regardless of whether the // composites are evaluated below. This is the mutative consequence of the apply, and it is @@ -451,9 +452,7 @@ func (p *DefaultCompDiffProcessor) processSingleComposition(ctx context.Context, CreatesRevision: comparison.changed(), } - if comparison.revisionNamePredictable { - result.RevisionImpact.PredictedRevisionName = pred.name - } + result.RevisionImpact.PredictedRevisionName = rc.revisionName() // Partition XRs by whether they would adopt the diffed composition's resulting revision. This is // local (no renders), and every path below needs it: the composites that would adopt the new @@ -581,12 +580,9 @@ func (p *DefaultCompDiffProcessor) processSingleComposition(ctx context.Context, // When the name is not predictable we render with the refs the composites have — the pre-#474 // behaviour — and say so, rather than guess one. Rendering with a name we invented would manufacture // a downstream diff on a converged cluster for exactly the compositions this feature exists to serve. - revisionName := "" + revisionName := rc.revisionName() - switch { - case comparison.revisionNamePredictable: - revisionName = pred.name - case comparison.changed(): + if revisionName == "" && comparison.changed() { p.config.Advisories.Raise(dt.OutputWarning{ Message: "Could not predict the name of the CompositionRevision this composition would create, because it differs from the cluster's only by the annotation a client-side `kubectl apply` writes — whose post-apply value depends on how you apply, not on the file. Composites were rendered with their existing compositionRevisionRef, so if any of your composition templates read the revision name, a resulting change was not detected. Apply with `--server-side` (or via Argo/Flux) to make it predictable.", Context: map[string]string{"composition": newComp.GetName()}, @@ -596,7 +592,7 @@ func (p *DefaultCompDiffProcessor) processSingleComposition(ctx context.Context, // Process kept XRs and collect diffs to determine which ones have changes p.config.Logger.Debug("Processing XRs to collect diff information", "count", len(keptXRs)) - xrResults := p.collectXRDiffs(ctx, keptXRs, cliComp, revisionName, predictedRevisionLabels(newComp, pred), pinnedXRs) + xrResults := p.collectXRDiffs(ctx, keptXRs, cliComp, revisionName, rc.revisionLabels, pinnedXRs, run) composites := make([]renderedComposite, 0, len(keptXRs)) for _, xr := range keptXRs { @@ -678,12 +674,17 @@ func withoutClaimedXRs(kept []*un.Unstructured, dropped []filteredXR, xrKind str } // overlapGroups returns the groups of composites for the overlap check, without each composite another -// one in the run manages (see managedBy): a nested XR, controlled by its parent, is still rendered and -// reported, but the parent's rendering of what they share wins. A kept claim's backing XR is dropped (see withoutClaimedXRs), so it cannot -// exclude an XR nested under it here. Such a nested XR carries the crossplane.io/claim-name and -// claim-namespace labels Crossplane copies onto composed resources, so withoutClaimedXRs drops it as bound -// to the claim (see boundToClaim) before it is rendered; one lacking those labels reaches this check, -// controlled by no rendered composite. +// one in the run manages (see managedBy). A nested XR, controlled by its parent, is still rendered and +// reported, but the parent's rendering of what they share wins: it renders the nested XR as the parent +// would write it, with the nested XR's Composition from the run when the run has it (see +// runCompositionFor), where the nested XR's own render starts from the spec it has now. +// +// A kept claim's backing XR is dropped (see withoutClaimedXRs), so it cannot exclude an XR nested under +// it here. Such a nested XR carries the crossplane.io/claim-name and claim-namespace labels Crossplane +// copies onto composed resources, so it is bound to the claim (see boundToClaim): withoutClaimedXRs +// drops one of the backing XR's own type before it is rendered, and one of another type, rendered for +// its own Composition, is left out here. One lacking those labels reaches this check, controlled by no +// rendered composite. func overlapGroups(composites []renderedComposite) []dt.XRDiffGroup { groups := make([]dt.XRDiffGroup, 0, len(composites)) @@ -708,7 +709,10 @@ func overlapGroups(composites []renderedComposite) []dt.XRDiffGroup { // // pinned, keyed by dt.MakeDiffKeyFromResource, holds the composites rendered against their own pinned // revision instead of cliComp; 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 { +// +// run holds the run's Compositions by name; a nested XR of another type is rendered with its own from +// there when the run has it (see runCompositionFor). +func (p *DefaultCompDiffProcessor) collectXRDiffs(ctx context.Context, xrs []*un.Unstructured, cliComp *apiextensionsv1.Composition, revisionName string, revisionLabels map[string]string, pinned map[string]pinnedResolution, run map[string]runComposition) map[string]*XRDiffResult { // Extract the target GVK from the CLI composition's compositeTypeRef cliCompTargetAPIVersion := cliComp.Spec.CompositeTypeRef.APIVersion cliCompTargetKind := cliComp.Spec.CompositeTypeRef.Kind @@ -733,7 +737,8 @@ func (p *DefaultCompDiffProcessor) collectXRDiffs(ctx context.Context, xrs []*un // 1. Root-level resources (XRs and Claims that use this composition) // 2. XRs whose type matches the CLI composition's compositeTypeRef // - // For nested XRs with different types, looks up from the cluster. + // A nested XR of another type is rendered with its Composition from the run when the run has it, + // and otherwise with the cluster's (see runCompositionFor). // // The provider is handed the effective composite (see CompositeResolver), so a root claim reaches it // as its backing XR, whose key is not in rootResourceKeys: Check 2 matches it by type instead. @@ -772,15 +777,15 @@ func (p *DefaultCompDiffProcessor) collectXRDiffs(ctx context.Context, xrs []*un return p.cliCompositionRevision(res, cliComp, revisionName, revisionLabels) } - // This is a nested XR with a different type - look up its composition from the cluster - p.config.Logger.Debug("Resource does not match CLI composition type, looking up from cluster", + // This is a nested XR with a different type: its Composition comes from the run, or the cluster. + p.config.Logger.Debug("Resource does not match CLI composition type, looking up its own composition", "resource", resourceID, "resourceAPIVersion", resAPIVersion, "resourceKind", resKind, "cliCompTargetAPIVersion", cliCompTargetAPIVersion, "cliCompTargetKind", cliCompTargetKind) - return p.compositionClient.FindMatchingComposition(ctx, res) + return p.runCompositionFor(ctx, res, run) } results := make(map[string]*XRDiffResult) @@ -928,6 +933,128 @@ func (p *DefaultCompDiffProcessor) cliCompositionRevision(res *un.Unstructured, return resolved, nil } +// runComposition is one Composition of the run: its comparison with the cluster's, its typed form and +// the predicted identity and labels of the revision it would produce, or the error that kept it from +// being read. It is the one source of these for the Composition's own processing (see +// processSingleComposition) and for a nested XR of its type under another Composition's parent (see +// runCompositionFor). +type runComposition struct { + comparison compositionComparison + comp *apiextensionsv1.Composition + pred predictedRevision + revisionLabels map[string]string + err error +} + +// revisionName is the predicted name of the revision rc would produce, or empty when it cannot be +// predicted (see compositionComparison.revisionNamePredictable). +func (rc runComposition) revisionName() string { + if !rc.comparison.revisionNamePredictable { + return "" + } + + return rc.pred.name +} + +// runCompositions returns the run's Compositions by name. Names are unique within a run (see +// simulate.RejectDuplicateNames). A Composition that cannot be read carries its error, which its own +// processing reports. +func (p *DefaultCompDiffProcessor) runCompositions(ctx context.Context, compositions []*un.Unstructured) map[string]runComposition { + run := make(map[string]runComposition, len(compositions)) + + for _, u := range compositions { + if u.GetKind() != "Composition" { + continue + } + + // The comparison strips cluster metadata from u, so it runs before the conversion. + comparison, err := p.calculateCompositionDiff(ctx, u) + if err != nil { + run[u.GetName()] = runComposition{err: errors.Wrap(err, "cannot calculate composition diff")} + continue + } + + comp := &apiextensionsv1.Composition{} + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(u.Object, comp); err != nil { + run[u.GetName()] = runComposition{err: errors.Wrapf(err, "cannot convert composition %q to typed", u.GetName())} + continue + } + + // The identity of the revision this composition would produce, used to evaluate + // compositionRevisionSelectors against the label set the revision would carry, and to seed the + // composites so a template observing the revision name renders the value it would really get. + pred := predictRevision(comp) + run[u.GetName()] = runComposition{comparison: comparison, comp: comp, pred: pred, revisionLabels: predictedRevisionLabels(u, pred)} + } + + return run +} + +// runCompositionFor resolves the composition of res, a nested XR of a type other than the diffed +// composition's. The run's Composition is used when it is the one res names in compositionRef, or, when +// res names none, the one the cluster selects for it (by compositionSelector or type), and res would +// adopt the revision it produces (see classifyXR). res then renders with that revision's predicted name, +// as a composite of the diffed composition does (see cliCompositionRevision). Otherwise res gets the +// cluster's composition and revision: a Manual res stays on the revision it is pinned to, as Crossplane +// would keep it. +// +// TODO(jcogilvie): selection by compositionSelector or type runs over the cluster's Compositions, so a run +// Composition the cluster does not have yet, or whose labels the run changes, is not selected for a +// nested XR without a compositionRef. Crossplane writes compositionRef on every XR it reconciles, so +// only a nested XR the run would create is affected; overlay the run onto the cluster's candidates if +// that case matters. +func (p *DefaultCompDiffProcessor) runCompositionFor(ctx context.Context, res *un.Unstructured, run map[string]runComposition) (dtypes.ResolvedComposition, error) { + name, err := xp.XRCompositionRef(res.Object, res.GetAPIVersion()) + if err != nil { + return dtypes.ResolvedComposition{}, errors.Wrapf(err, "cannot read compositionRef for %s/%s", res.GetKind(), res.GetName()) + } + + // cluster is the cluster's resolution, looked up only when res names no composition or the run's + // does not apply to it. + var cluster dtypes.ResolvedComposition + + if name == "" { + if cluster, err = p.compositionClient.FindMatchingComposition(ctx, res); err != nil { + return dtypes.ResolvedComposition{}, err + } + + name = cluster.Composition.GetName() + } + + rc, inRun := run[name] + if inRun { + if rc.err != nil { + return dtypes.ResolvedComposition{}, errors.Wrapf(rc.err, "cannot use composition %q from the run", name) + } + + if ref := rc.comp.Spec.CompositeTypeRef; ref.APIVersion != res.GetAPIVersion() || ref.Kind != res.GetKind() { + return dtypes.ResolvedComposition{}, errors.Errorf("composition %q in the run composes %s %s, but nested XR %q is a %s %s", rc.comp.GetName(), ref.APIVersion, ref.Kind, res.GetName(), res.GetAPIVersion(), res.GetKind()) + } + + disposition, err := p.classifyXR(res, rc.revisionLabels, rc.comp.GetLabels()) + if err != nil { + return dtypes.ResolvedComposition{}, err + } + + inRun = disposition.filtered == nil && !disposition.pinnedToRevision + } + + if !inRun { + if cluster.Composition != nil { + return cluster, nil + } + + return p.compositionClient.FindMatchingComposition(ctx, res) + } + + p.config.Logger.Debug("Nested XR's composition is in the run, using it", + "resource", fmt.Sprintf("%s/%s", res.GetKind(), res.GetName()), + "composition", rc.comp.GetName()) + + // classifyXR kept res, so it adopts the revision, as cliCompositionRevision would find. + return dtypes.ResolvedComposition{Composition: rc.comp, RevisionName: rc.revisionName()}, nil +} + // compositionComparison is the result of comparing a proposed composition against its in-cluster // version. // diff --git a/internal/diff/diffprocessor/comp_processor_test.go b/internal/diff/diffprocessor/comp_processor_test.go index ce35cf26..c3e95ec5 100644 --- a/internal/diff/diffprocessor/comp_processor_test.go +++ b/internal/diff/diffprocessor/comp_processor_test.go @@ -1308,7 +1308,7 @@ func TestDefaultCompDiffProcessor_collectXRDiffs_RevisionName(t *testing.T) { } comp := tu.NewComposition("comp").WithCompositeTypeRef("example.org/v1", "XResource").WithPipelineMode().Build() - results := processor.collectXRDiffs(t.Context(), []*un.Unstructured{tt.xr}, comp, tt.revisionName, revisionLabels, nil) + results := processor.collectXRDiffs(t.Context(), []*un.Unstructured{tt.xr}, comp, tt.revisionName, revisionLabels, nil, nil) if err := results[dt.MakeDiffKeyFromResource(tt.xr)].Error; err != nil { t.Fatalf("%s\ncollectXRDiffs(): unexpected error: %v", tt.reason, err) @@ -1365,7 +1365,7 @@ func TestDefaultCompDiffProcessor_collectXRDiffs_PerXR(t *testing.T) { } comp := tu.NewComposition("comp").WithCompositeTypeRef("example.org/v1", "XResource").WithPipelineMode().Build() - results := processor.collectXRDiffs(t.Context(), []*un.Unstructured{slow, fast, failing}, comp, "", nil, nil) + results := processor.collectXRDiffs(t.Context(), []*un.Unstructured{slow, fast, failing}, comp, "", nil, nil, nil) got := map[string]string{} for _, xr := range []*un.Unstructured{slow, fast, failing} { @@ -1688,7 +1688,7 @@ func TestDefaultCompDiffProcessor_collectXRDiffs_NestedXRCompositionLookup(t *te }, } - _ = processor.collectXRDiffs(ctx, tt.xrs, tt.cliComposition, "", nil, nil) + _ = processor.collectXRDiffs(ctx, tt.xrs, tt.cliComposition, "", nil, nil, nil) // Verify the composition requests if len(*compositionRequests) < 2 { @@ -1714,6 +1714,155 @@ func TestDefaultCompDiffProcessor_collectXRDiffs_NestedXRCompositionLookup(t *te } } +// TestDefaultCompDiffProcessor_collectXRDiffs_NestedXRRunComposition covers a nested XR of another type +// than the diffed composition's: it renders with its own Composition from the run when the run has it +// and it would adopt that Composition's new revision, and with the cluster's otherwise (#515). +func TestDefaultCompDiffProcessor_collectXRDiffs_NestedXRRunComposition(t *testing.T) { + parentComp := tu.NewComposition("parent").WithCompositeTypeRef("parent.example.org/v1", "XParent").WithPipelineMode().Build() + // The run's child composition; the cluster's has no labels, so the run changes it. + runChild := tu.NewComposition("child").WithCompositeTypeRef("child.example.org/v1", "XChild").WithPipelineMode(). + WithLabels(map[string]string{"version": "2"}) + clusterChild := tu.NewComposition("child").WithCompositeTypeRef("child.example.org/v1", "XChild").WithPipelineMode().Build() + otherChild := tu.NewComposition("other-child").WithCompositeTypeRef("child.example.org/v1", "XChild").WithPipelineMode().Build() + + root := tu.NewResource("parent.example.org/v1", "XParent", "root").Build() + nested := func(ref string) *tu.ResourceBuilder { + b := tu.NewResource("child.example.org/v1", "XChild", "nested") + if ref != "" { + b = b.WithNestedField(map[string]any{"name": ref}, "spec", "crossplane", "compositionRef") + } + + return b + } + + childRevision := predictRevision(runChild.Build()).name + + type args struct { + // nested is the nested XR the root's render hands the provider. + nested *un.Unstructured + // cluster is the composition the cluster resolves nested to. + cluster types.ResolvedComposition + // runComps are the run's Compositions; runChild when unset. + runComps []*un.Unstructured + // run, when set, replaces the run built from runComps. + run map[string]runComposition + } + + // want is the composition the nested XR renders with: its name, labels (the run's copy carries + // version 2) and revision, or the error resolving it. + type want struct { + Composition string + Labels map[string]string + Revision string + Err string + } + + cases := map[string]struct { + reason string + args args + want want + }{ + "RefToRunCompositionGetsIt": { + reason: "A nested XR naming a Composition in the run renders with the run's copy and its predicted revision.", + args: args{nested: nested("child").Build(), cluster: types.ResolvedComposition{Composition: clusterChild, RevisionName: "child-0ld0001"}}, + want: want{Composition: "child", Labels: runChild.Build().GetLabels(), Revision: childRevision}, + }, + "ClusterSelectedRunCompositionGetsIt": { + reason: "A nested XR naming no Composition renders with the run's copy of the one the cluster selects for it.", + args: args{nested: nested("").Build(), cluster: types.ResolvedComposition{Composition: clusterChild}}, + want: want{Composition: "child", Labels: runChild.Build().GetLabels(), Revision: childRevision}, + }, + "CompositionNotInRunGetsClusters": { + reason: "A nested XR whose Composition is not in the run renders with the cluster's.", + args: args{nested: nested("other-child").Build(), cluster: types.ResolvedComposition{Composition: otherChild, RevisionName: "other-child-0ld0001"}}, + want: want{Composition: "other-child", Revision: "other-child-0ld0001"}, + }, + "ManualNestedXRStaysPinned": { + reason: "A Manual nested XR does not adopt the run's revision, so it renders with the cluster's pinned one.", + args: args{ + nested: nested("child").WithNestedField("Manual", "spec", "crossplane", "compositionUpdatePolicy"). + WithNestedField(map[string]any{"name": "child-0ld0001"}, "spec", "crossplane", "compositionRevisionRef").Build(), + cluster: types.ResolvedComposition{Composition: clusterChild, RevisionName: "child-0ld0001"}, + }, + want: want{Composition: "child", Revision: "child-0ld0001"}, + }, + "SelectorRejectingRunRevisionGetsClusters": { + reason: "An Automatic nested XR whose compositionRevisionSelector rejects the run's revision labels does not adopt it, so it renders with the cluster's.", + args: args{ + nested: nested("child").WithCompositionRevisionSelector(xp.CrossplaneAPIExtGroupV2, map[string]string{"version": "3"}, nil).Build(), + cluster: types.ResolvedComposition{Composition: clusterChild, RevisionName: "child-0ld0001"}, + }, + want: want{Composition: "child", Revision: "child-0ld0001"}, + }, + "RunCompositionErrorIsReturned": { + reason: "A nested XR naming a run Composition that could not be read fails, rather than silently rendering with the cluster's.", + args: args{ + nested: nested("child").Build(), + cluster: types.ResolvedComposition{Composition: clusterChild, RevisionName: "child-0ld0001"}, + run: map[string]runComposition{"child": {err: errors.New("boom")}}, + }, + want: want{Err: `cannot use composition "child" from the run: boom`}, + }, + "IncompatibleRunCompositionIsAnError": { + reason: "A nested XR naming a run Composition of another composite type fails.", + args: args{ + nested: nested("child").Build(), + cluster: types.ResolvedComposition{Composition: clusterChild, RevisionName: "child-0ld0001"}, + runComps: []*un.Unstructured{tu.NewComposition("child").WithCompositeTypeRef("child.example.org/v1", "XOther").WithPipelineMode().BuildAsUnstructured()}, + }, + want: want{Err: `composition "child" in the run composes child.example.org/v1 XOther, but nested XR "nested" is a child.example.org/v1 XChild`}, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + var ( + got types.ResolvedComposition + gotErr error + ) + + processor := &DefaultCompDiffProcessor{ + compositionClient: tu.NewMockCompositionClient(). + WithSuccessfulCompositionFetches([]*apiextensionsv1.Composition{parentComp, clusterChild}). + WithFindMatchingComposition(func(context.Context, *un.Unstructured) (types.ResolvedComposition, error) { + return tc.args.cluster, nil + }). + Build(), + xrProc: &tu.MockDiffProcessor{ + DiffSingleResourceFn: func(ctx context.Context, _ *un.Unstructured, provider types.CompositionProvider) (map[string]*dt.ResourceDiff, map[string]*dt.ResourceDiff, error) { + got, gotErr = provider(ctx, tc.args.nested) + + return map[string]*dt.ResourceDiff{}, nil, gotErr + }, + }, + config: ProcessorConfig{Logger: tu.TestLogger(t, false)}, + } + + runComps := tc.args.runComps + if runComps == nil { + runComps = []*un.Unstructured{runChild.BuildAsUnstructured()} + } + + run := tc.args.run + if run == nil { + run = processor.runCompositions(t.Context(), runComps) + } + + _ = processor.collectXRDiffs(t.Context(), []*un.Unstructured{root}, parentComp, "", nil, nil, run) + + gotWant := want{} + if gotErr != nil { + gotWant.Err = gotErr.Error() + } else { + gotWant = want{Composition: got.Composition.GetName(), Labels: got.Composition.GetLabels(), Revision: got.RevisionName} + } + if diff := gcmp.Diff(tc.want, gotWant); diff != "" { + t.Errorf("%s\ncollectXRDiffs() nested XR's composition (-want +got):\n%s", tc.reason, diff) + } + }) + } +} + // TestDefaultCompDiffProcessor_DiffComposition_ReportErrorOutput verifies that when XR processing // fails, the report carries the detailed error, attributed to the failed XR, for structured output, // and the returned error names the failing XR and carries the detail: the command prints it once. @@ -2036,7 +2185,7 @@ func TestDefaultCompDiffProcessor_DiffComposition_ResourceMode(t *testing.T) { proc := newCompProcessorForTest(t, client, false /* IncludeManual */) - got, _, err := proc.processSingleComposition(t.Context(), comp, []*un.Unstructured{manualXR, pinnedManualXR}, tt.scoped) + got, _, err := proc.processSingleComposition(t.Context(), comp, []*un.Unstructured{manualXR, pinnedManualXR}, tt.scoped, proc.runCompositions(t.Context(), []*un.Unstructured{comp})) if err != nil { t.Fatalf("processSingleComposition: %v", err) } @@ -2144,7 +2293,7 @@ func TestDefaultCompDiffProcessor_DiffComposition_ResourceMode(t *testing.T) { proc := newCompProcessorForTest(t, client, tt.includeManual) proc.config.AnalyzeOn = tt.analyzeOn - got, _, err := proc.processSingleComposition(t.Context(), metaOnlyComp, []*un.Unstructured{xr1, manualXR}, tt.scoped) + got, _, err := proc.processSingleComposition(t.Context(), metaOnlyComp, []*un.Unstructured{xr1, manualXR}, tt.scoped, proc.runCompositions(t.Context(), []*un.Unstructured{metaOnlyComp})) if err != nil { t.Fatalf("processSingleComposition: %v", err) } @@ -2204,7 +2353,7 @@ func TestDefaultCompDiffProcessor_DiffComposition_ResourceMode(t *testing.T) { proc := newCompProcessorForTest(t, client, false /* IncludeManual */) - got, _, err := proc.processSingleComposition(t.Context(), labeledComp, []*un.Unstructured{selectorMismatchXR}, true /* scoped */) + got, _, err := proc.processSingleComposition(t.Context(), labeledComp, []*un.Unstructured{selectorMismatchXR}, true /* scoped */, proc.runCompositions(t.Context(), []*un.Unstructured{labeledComp})) if err != nil { t.Fatalf("processSingleComposition: %v", err) } @@ -2320,7 +2469,8 @@ func TestDefaultCompDiffProcessor_processSingleComposition_ChangedFunctions(t *t proc.config.AnalyzeOn = tt.analyzeOn WithChangedFunctions(tt.changed...)(&proc.config) - got, _, err := proc.processSingleComposition(t.Context(), builder().BuildAsUnstructured(), []*un.Unstructured{xr}, false) + comp := builder().BuildAsUnstructured() + got, _, err := proc.processSingleComposition(t.Context(), comp, []*un.Unstructured{xr}, false, proc.runCompositions(t.Context(), []*un.Unstructured{comp})) if err != nil { t.Fatalf("processSingleComposition: %v", err) } @@ -2459,7 +2609,8 @@ func TestDefaultCompDiffProcessor_processSingleComposition_PinnedXRWithChangedFu }, } - got, _, err := proc.processSingleComposition(t.Context(), cliComp.BuildAsUnstructured(), []*un.Unstructured{pinnedXR("pinned")}, false) + comp := cliComp.BuildAsUnstructured() + got, _, err := proc.processSingleComposition(t.Context(), comp, []*un.Unstructured{pinnedXR("pinned")}, false, proc.runCompositions(t.Context(), []*un.Unstructured{comp})) if err != nil { t.Fatalf("processSingleComposition: %v", err) } @@ -2536,7 +2687,8 @@ func TestDefaultCompDiffProcessor_processSingleComposition_PinnedBackingXROfFilt }, } - got, _, err := proc.processSingleComposition(t.Context(), cliComp.BuildAsUnstructured(), []*un.Unstructured{claim, backing}, false) + comp := cliComp.BuildAsUnstructured() + got, _, err := proc.processSingleComposition(t.Context(), comp, []*un.Unstructured{claim, backing}, false, proc.runCompositions(t.Context(), []*un.Unstructured{comp})) if err != nil { t.Fatalf("processSingleComposition: %v", err) }