diff --git a/CLAUDE.md b/CLAUDE.md index fbd3e11..3f1b2fe 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -167,10 +167,45 @@ cmd/diff/ - Scope validation: Namespaced XRs cannot own cluster-scoped resources (except Claims) - Namespace propagation: XR namespace propagates to managed resources in Crossplane v2 +**Claim Label Behavior (Empirically Verified)** +Crossplane ALWAYS uses the XR name for the `crossplane.io/composite` label on composed resources, even when rendering from a Claim. + +Evidence from empirical testing (2025-11-18): +```yaml +# Claim: my-test-claim (namespace: claim-test-ns) +# XR: my-test-claim-mjwln (cluster-scoped, auto-generated suffix) +# Composed NopResource labels: +labels: + crossplane.io/claim-name: my-test-claim # Points to Claim + crossplane.io/claim-namespace: claim-test-ns # Claim namespace + crossplane.io/composite: my-test-claim-mjwln # Points to XR, NOT Claim! +``` + +Key implications: +- When diffing Claims, the `crossplane.io/composite` label should NOT change between renders +- Crossplane templates use `{{ .observed.composite.resource.metadata.name }}` which is the XR name +- The XR is the actual composite owner; the Claim just references it via `spec.resourceRef` +- Test expectations must reflect this: NO label changes when modifying existing Claims +- This behavior is consistent across Crossplane versions + **Diff Calculation** - Compares rendered resources against cluster state via server-side dry-run - Detects additions, modifications, and removals - Handles `generateName` by matching via labels/annotations (`crossplane.io/composition-resource-name`) +- Uses two-phase approach to correctly handle nested XRs: + 1. **Phase 1 - Non-removal diffs**: `CalculateNonRemovalDiffs` computes diffs for all rendered resources + 2. **Phase 2 - Removal detection**: `CalculateRemovedResourceDiffs` identifies resources to be removed + - This separation is critical because nested XRs must be processed before detecting removals + - Nested XRs may render additional composed resources that shouldn't be marked as removals + +**Resource Management** +- `ResourceManager` handles all resource fetching and cluster state operations +- Key responsibilities: + - `FetchCurrentObject`: Retrieves existing resource from cluster (for identity preservation) + - `FetchObservedResources`: Fetches resource tree to find all composed resources (including nested) + - `UpdateOwnerReferences`: Updates owner references with dry-run annotations +- Separation of concerns: `DiffCalculator` focuses on diff logic, `ResourceManager` handles cluster I/O +- Identity preservation: Fetches existing nested XRs to maintain their cluster identity across renders ## Design Principles diff --git a/Earthfile b/Earthfile index aaa01d6..53ef4f6 100644 --- a/Earthfile +++ b/Earthfile @@ -124,6 +124,7 @@ e2e-internal: ARG GOOS=${TARGETOS} ARG FLAGS="-test-suite=base -labels=crossplane-version=${CROSSPLANE_IMAGE_TAG}" ARG TEST_FORMAT=testname + ARG E2E_DUMP_EXPECTED # Using earthly image to allow compatibility with different development environments e.g. WSL FROM earthly/dind:alpine-3.20-docker-26.1.5-r0 RUN wget https://dl.google.com/go/go${GO_VERSION}.${GOOS}-${GOARCH}.tar.gz @@ -151,10 +152,11 @@ e2e-internal: WITH DOCKER --pull crossplane/crossplane:${CROSSPLANE_IMAGE_TAG} # TODO(negz:) Set GITHUB_ACTIONS=true and use RUN --raw-output when # https://github.com/earthly/earthly/issues/4143 is fixed. - RUN gotestsum --no-color=false --format ${TEST_FORMAT} --junitfile e2e-tests.xml --raw-command go tool test2json -t -p E2E ./e2e -test.v -crossplane-image=crossplane/crossplane:${CROSSPLANE_IMAGE_TAG} ${FLAGS} + RUN E2E_DUMP_EXPECTED=${E2E_DUMP_EXPECTED} gotestsum --no-color=false --format ${TEST_FORMAT} --junitfile e2e-tests.xml --raw-command go tool test2json -t -p E2E ./e2e -test.v -crossplane-image=crossplane/crossplane:${CROSSPLANE_IMAGE_TAG} ${FLAGS} END FINALLY SAVE ARTIFACT --if-exists e2e-tests.xml AS LOCAL _output/tests/e2e-tests.xml + SAVE ARTIFACT --if-exists test/e2e/manifests/beta/diff AS LOCAL test/e2e/manifests/beta/diff END # go-modules downloads Crossplane's go modules. It's the base target of most Go diff --git a/cmd/diff/client/crossplane/composition_client.go b/cmd/diff/client/crossplane/composition_client.go index 6f92bb1..0c483bb 100644 --- a/cmd/diff/client/crossplane/composition_client.go +++ b/cmd/diff/client/crossplane/composition_client.go @@ -670,19 +670,26 @@ func (c *DefaultCompositionClient) FindCompositesUsingComposition(ctx context.Co "gvk", xrGVK.String()) // List all resources of this XR type in the specified namespace + // Note: If namespace is specified and XRs are cluster-scoped, this will fail gracefully + // and we'll continue to search for Claims xrs, err := c.resourceClient.ListResources(ctx, xrGVK, namespace) - if err != nil { - return nil, errors.Wrapf(err, "cannot list XRs of type %s in namespace %s", xrGVK.String(), namespace) - } - - c.logger.Debug("Found XRs of target type", "count", len(xrs)) - // Filter XRs that use this specific composition var matchingResources []*un.Unstructured - for _, xr := range xrs { - if c.resourceUsesComposition(xr, compositionName) { - matchingResources = append(matchingResources, xr) + if err != nil { + // Log the error but don't fail - we'll try to find Claims instead + c.logger.Debug("Cannot list XRs (will search for claims if XRD defines them)", + "xrGVK", xrGVK.String(), + "namespace", namespace, + "error", err) + } else { + c.logger.Debug("Found XRs of target type", "count", len(xrs)) + + // Filter XRs that use this specific composition + for _, xr := range xrs { + if c.resourceUsesComposition(xr, compositionName) { + matchingResources = append(matchingResources, xr) + } } } @@ -697,6 +704,7 @@ func (c *DefaultCompositionClient) FindCompositesUsingComposition(ctx context.Co c.logger.Debug("Cannot get XRD for XR type (will not search for claims)", "xrGVK", xrGVK.String(), "error", err) + return matchingResources, nil } @@ -707,6 +715,7 @@ func (c *DefaultCompositionClient) FindCompositesUsingComposition(ctx context.Co c.logger.Debug("Error extracting claim type from XRD", "xrd", xrd.GetName(), "error", err) + return matchingResources, nil } @@ -714,6 +723,7 @@ func (c *DefaultCompositionClient) FindCompositesUsingComposition(ctx context.Co if claimGVK.Empty() { c.logger.Debug("XRD does not define claims", "xrd", xrd.GetName()) + return matchingResources, nil } @@ -727,6 +737,7 @@ func (c *DefaultCompositionClient) FindCompositesUsingComposition(ctx context.Co c.logger.Debug("Cannot list claims of type (will only return XRs)", "claimGVK", claimGVK.String(), "error", err) + return matchingResources, nil } diff --git a/cmd/diff/client/crossplane/composition_client_test.go b/cmd/diff/client/crossplane/composition_client_test.go index e43bb26..4f8d9c7 100644 --- a/cmd/diff/client/crossplane/composition_client_test.go +++ b/cmd/diff/client/crossplane/composition_client_test.go @@ -1600,9 +1600,11 @@ func TestDefaultCompositionClient_getClaimTypeFromXRD(t *testing.T) { t.Errorf("\n%s\ngetClaimTypeFromXRD(): expected error containing %q but got none", tt.reason, tt.want.errMsg) return } + if !strings.Contains(err.Error(), tt.want.errMsg) { t.Errorf("\n%s\ngetClaimTypeFromXRD(): expected error containing %q, got %q", tt.reason, tt.want.errMsg, err.Error()) } + return } diff --git a/cmd/diff/diff_integration_test.go b/cmd/diff/diff_integration_test.go index 66e5e6c..4cc282e 100644 --- a/cmd/diff/diff_integration_test.go +++ b/cmd/diff/diff_integration_test.go @@ -2744,23 +2744,27 @@ Summary: 2 modified }, "CompositionChangeImpactsClaims": { reason: "Validates composition change impacts existing Claims (issue #120)", - // Set up existing Claims that use the composition + // Set up existing Claims and their XRs that use the original composition setupFiles: []string{ + // XRD, composition, and functions "testdata/comp/resources/claim-xrd.yaml", "testdata/comp/resources/claim-composition.yaml", "testdata/comp/resources/functions.yaml", - "testdata/comp/resources/existing-namespace.yaml", - // Add existing Claims that use the composition + // Test namespace + "testdata/comp/resources/test-namespace.yaml", + // Existing Claims and their corresponding XRs "testdata/comp/resources/existing-claim-1.yaml", - "testdata/comp/resources/existing-claim-2.yaml", - // Add the downstream resources created by the Claims' XRs + "testdata/comp/resources/existing-claim-1-xr.yaml", "testdata/comp/resources/existing-claim-downstream-1.yaml", + "testdata/comp/resources/existing-claim-2.yaml", + "testdata/comp/resources/existing-claim-2-xr.yaml", "testdata/comp/resources/existing-claim-downstream-2.yaml", }, - // New composition file that will be diffed + // Updated composition that will be diffed inputFiles: []string{"testdata/comp/updated-claim-composition.yaml"}, namespace: "test-namespace", - expectedOutput: `=== Composition Changes === + expectedOutput: ` +=== Composition Changes === ~~~ Composition/nopclaims.diff.example.org apiVersion: apiextensions.crossplane.io/v1 @@ -2770,7 +2774,7 @@ Summary: 2 modified spec: compositeTypeRef: apiVersion: diff.example.org/v1alpha1 - kind: XNopClaim + kind: XNop mode: Pipeline pipeline: - functionRef: @@ -2821,21 +2825,15 @@ Summary: 2 resources with changes labels: crossplane.io/claim-name: test-claim-1 crossplane.io/claim-namespace: test-namespace -- crossplane.io/composite: test-claim-1-xr + crossplane.io/composite: test-claim-1-xr - name: test-claim-1-xr -- spec: -- forProvider: ++ name: test-claim-1 + spec: + forProvider: - configData: claim-value-1 - resourceTier: basic -+ crossplane.io/composite: test-claim-1 -+ name: test-claim-1 -+ spec: -+ forProvider: + configData: updated-claim-value-1 + resourceTier: premium - - - --- ~~~ XDownstreamResource/test-claim-2-xr @@ -2848,21 +2846,15 @@ Summary: 2 resources with changes labels: crossplane.io/claim-name: test-claim-2 crossplane.io/claim-namespace: test-namespace -- crossplane.io/composite: test-claim-2-xr + crossplane.io/composite: test-claim-2-xr - name: test-claim-2-xr -- spec: -- forProvider: ++ name: test-claim-2 + spec: + forProvider: - configData: claim-value-2 - resourceTier: basic -+ crossplane.io/composite: test-claim-2 -+ name: test-claim-2 -+ spec: -+ forProvider: + configData: updated-claim-value-2 + resourceTier: premium - - - --- diff --git a/cmd/diff/diffprocessor/diff_calculator.go b/cmd/diff/diffprocessor/diff_calculator.go index 33c170e..9738299 100644 --- a/cmd/diff/diffprocessor/diff_calculator.go +++ b/cmd/diff/diffprocessor/diff_calculator.go @@ -13,7 +13,6 @@ import ( "github.com/crossplane/crossplane-runtime/v2/pkg/errors" "github.com/crossplane/crossplane-runtime/v2/pkg/logging" - cpd "github.com/crossplane/crossplane-runtime/v2/pkg/resource/unstructured/composed" cmp "github.com/crossplane/crossplane-runtime/v2/pkg/resource/unstructured/composite" "github.com/crossplane/crossplane/v2/cmd/crank/common/resource" @@ -25,15 +24,18 @@ type DiffCalculator interface { // CalculateDiff computes the diff for a single resource CalculateDiff(ctx context.Context, composite *un.Unstructured, desired *un.Unstructured) (*dt.ResourceDiff, error) - // CalculateDiffs computes all diffs for the rendered resources and identifies resources to be removed + // CalculateDiffs computes all diffs including removals for the rendered resources. + // This is the primary method that most code should use. CalculateDiffs(ctx context.Context, xr *cmp.Unstructured, desired render.Outputs) (map[string]*dt.ResourceDiff, error) - // CalculateRemovedResourceDiffs identifies resources that would be removed and calculates their diffs - CalculateRemovedResourceDiffs(ctx context.Context, xr *un.Unstructured, renderedResources map[string]bool) (map[string]*dt.ResourceDiff, error) + // CalculateNonRemovalDiffs computes diffs for modified/added resources and returns + // the set of rendered resource keys. This is used by nested XR processing. + // Returns: (diffs map, rendered resource keys, error) + CalculateNonRemovalDiffs(ctx context.Context, xr *cmp.Unstructured, desired render.Outputs) (map[string]*dt.ResourceDiff, map[string]bool, error) - // FetchObservedResources fetches the observed composed resources for the given XR. - // Returns a flat slice of composed resources suitable for render.Inputs.ObservedResources. - FetchObservedResources(ctx context.Context, xr *cmp.Unstructured) ([]cpd.Unstructured, error) + // CalculateRemovedResourceDiffs identifies resources that exist in the cluster but are not + // in the rendered set. This is called after nested XR processing is complete. + CalculateRemovedResourceDiffs(ctx context.Context, xr *un.Unstructured, renderedResources map[string]bool) (map[string]*dt.ResourceDiff, error) } // DefaultDiffCalculator implements the DiffCalculator interface. @@ -102,6 +104,11 @@ func (c *DefaultDiffCalculator) CalculateDiff(ctx context.Context, composite *un // Preserve existing resource identity for resources with generateName desired = c.preserveExistingResourceIdentity(current, desired, resourceID, name) + // Preserve the composite label for ALL existing resources + // This is critical because in Crossplane, all resources in a tree point to the ROOT composite, + // not their immediate parent. We must never change this label. + desired = c.preserveCompositeLabel(current, desired, resourceID) + // Update owner references if needed (done after preserving existing labels) // IMPORTANT: For composed resources, the owner should be the XR, not a Claim. // When composite is the current XR from the cluster, we use it as the owner. @@ -144,8 +151,46 @@ func (c *DefaultDiffCalculator) CalculateDiff(ctx context.Context, composite *un return diff, nil } -// CalculateDiffs collects all diffs for the desired resources and identifies resources to be removed. -func (c *DefaultDiffCalculator) CalculateDiffs(ctx context.Context, xr *cmp.Unstructured, desired render.Outputs) (map[string]*dt.ResourceDiff, error) { +// CalculateNonRemovalDiffs computes diffs for modified/added resources and returns +// the set of rendered resource keys for removal detection. +// +// TWO-PHASE DIFF ALGORITHM: +// This method implements Phase 1 of our two-phase diff calculation. The two-phase +// approach is necessary to correctly handle nested XRs (Composite Resources that +// themselves create other Composite Resources). +// +// WHY TWO PHASES? +// When processing nested XRs, we must: +// 1. Phase 1 (this method): Calculate diffs for all rendered resources (adds/modifications) +// and build a set of "rendered resource keys" that tracks what was generated +// 2. Phase 2 (CalculateRemovedResourceDiffs): Compare cluster state against rendered +// resources to identify removals +// +// The separation is critical because: +// - Nested XRs are processed recursively BETWEEN these phases +// - Nested XRs generate additional composed resources that must be added to the +// "rendered resources" set before removal detection +// - If we detected removals too early, we'd falsely identify nested XR resources +// as "to be removed" before they've been processed +// +// EXAMPLE SCENARIO: +// +// Parent XR renders: [Resource-A, NestedXR-B] +// NestedXR-B renders: [Resource-C, Resource-D] +// +// Without two phases: +// - We'd see cluster has [Resource-A, Resource-C, Resource-D] from prior render +// - We'd see new render has [Resource-A, NestedXR-B] +// - We'd INCORRECTLY mark Resource-C and Resource-D as removed +// +// With two phases: +// Phase 1: Calculate diffs for [Resource-A, NestedXR-B], track as rendered +// Process nested: Recurse into NestedXR-B, add Resource-C and Resource-D to rendered set +// Phase 2: Now see [Resource-A, NestedXR-B, Resource-C, Resource-D] as rendered +// No false removal detection! +// +// Returns: (diffs map, rendered resource keys, error). +func (c *DefaultDiffCalculator) CalculateNonRemovalDiffs(ctx context.Context, xr *cmp.Unstructured, desired render.Outputs) (map[string]*dt.ResourceDiff, map[string]bool, error) { xrName := xr.GetName() c.logger.Debug("Calculating diffs", "xr", xrName, @@ -161,7 +206,7 @@ func (c *DefaultDiffCalculator) CalculateDiffs(ctx context.Context, xr *cmp.Unst // First, calculate diff for the XR itself xrDiff, err := c.CalculateDiff(ctx, nil, xr.GetUnstructured()) if err != nil || xrDiff == nil { - return nil, errors.Wrap(err, "cannot calculate diff for XR") + return nil, nil, errors.Wrap(err, "cannot calculate diff for XR") } else if xrDiff.DiffType != dt.DiffTypeEqual { key := xrDiff.GetDiffKey() diffs[key] = xrDiff @@ -207,31 +252,44 @@ func (c *DefaultDiffCalculator) CalculateDiffs(ctx context.Context, xr *cmp.Unst } renderedResources[diffKey] = true - } - - if xrDiff.Current != nil { - // it only makes sense to calculate removal of things if we have a current XR. - c.logger.Debug("Finding resources to be removed", "xr", xrName) - - removedDiffs, err := c.CalculateRemovedResourceDiffs(ctx, xrDiff.Current, renderedResources) - if err != nil { - c.logger.Debug("Error calculating removed resources (continuing)", "error", err) - } else if len(removedDiffs) > 0 { - // Add removed resources to the diffs map - maps.Copy(diffs, removedDiffs) - } + c.logger.Debug("Added resource to renderedResources", + "xr", xrName, + "diffKey", diffKey, + "diffType", diff.DiffType) } // Log a summary c.logger.Debug("Diff calculation complete", "totalDiffs", len(diffs), + "renderedResourcesCount", len(renderedResources), "errors", len(errs), "xr", xrName) if len(errs) > 0 { - return diffs, errors.Join(errs...) + return diffs, renderedResources, errors.Join(errs...) + } + + return diffs, renderedResources, nil +} + +// CalculateDiffs computes all diffs including removals for the rendered resources. +// This is the primary method that most code should use. +func (c *DefaultDiffCalculator) CalculateDiffs(ctx context.Context, xr *cmp.Unstructured, desired render.Outputs) (map[string]*dt.ResourceDiff, error) { + // First calculate diffs for modified/added resources + diffs, renderedResources, err := c.CalculateNonRemovalDiffs(ctx, xr, desired) + if err != nil { + return nil, err } + // Then detect removed resources + removedDiffs, err := c.CalculateRemovedResourceDiffs(ctx, xr.GetUnstructured(), renderedResources) + if err != nil { + return nil, err + } + + // Merge removed diffs into the main diffs map + maps.Copy(diffs, removedDiffs) + return diffs, nil } @@ -302,67 +360,6 @@ func (c *DefaultDiffCalculator) CalculateRemovedResourceDiffs(ctx context.Contex return removedDiffs, nil } -// FetchObservedResources fetches the observed composed resources for the given XR. -// This returns a flat slice of all composed resources found in the resource tree, -// which can be directly used for render.Inputs.ObservedResources. -func (c *DefaultDiffCalculator) FetchObservedResources(ctx context.Context, xr *cmp.Unstructured) ([]cpd.Unstructured, error) { - c.logger.Debug("Fetching observed resources for XR", - "xr_kind", xr.GetKind(), - "xr_name", xr.GetName()) - - // Get the resource tree from the cluster - tree, err := c.treeClient.GetResourceTree(ctx, &xr.Unstructured) - if err != nil { - c.logger.Debug("Failed to get resource tree for XR", - "xr", xr.GetName(), - "error", err) - - return nil, errors.Wrap(err, "cannot get resource tree") - } - - // Extract composed resources from the tree - observed := extractComposedResources(tree) - - c.logger.Debug("Fetched observed composed resources", - "xr", xr.GetName(), - "count", len(observed)) - - return observed, nil -} - -// extractComposedResources recursively extracts all composed resources from a resource tree. -// It returns a flat slice of composed resources, suitable for render.Inputs.ObservedResources. -// Only includes resources with the crossplane.io/composition-resource-name annotation. -func extractComposedResources(tree *resource.Resource) []cpd.Unstructured { - var resources []cpd.Unstructured - - // Recursively collect composed resources from the tree - var collectResources func(node *resource.Resource) - - collectResources = func(node *resource.Resource) { - // Only include resources that have the composition-resource-name annotation - // (this filters out the root XR and non-composed resources) - if _, hasAnno := node.Unstructured.GetAnnotations()["crossplane.io/composition-resource-name"]; hasAnno { - // Convert to cpd.Unstructured (composed resource) - resources = append(resources, cpd.Unstructured{ - Unstructured: node.Unstructured, - }) - } - - // Recursively process children - for _, child := range node.Children { - collectResources(child) - } - } - - // Start from root's children to avoid including the XR itself - for _, child := range tree.Children { - collectResources(child) - } - - return resources -} - // preserveExistingResourceIdentity preserves the identity (name, generateName, labels) from an existing // resource with generateName to ensure dry-run apply works on the correct resource identity. // This is critical for claim scenarios where the rendered name differs from the generated name. @@ -409,3 +406,41 @@ func (c *DefaultDiffCalculator) preserveExistingResourceIdentity(current, desire return desiredCopy } + +// preserveCompositeLabel preserves the crossplane.io/composite label from an existing resource. +// This is critical because in Crossplane, all resources in a tree point to the ROOT composite, +// not their immediate parent. We must never change this label for existing resources. +func (c *DefaultDiffCalculator) preserveCompositeLabel(current, desired *un.Unstructured, resourceID string) *un.Unstructured { + // If there's no current resource, nothing to preserve + if current == nil { + return desired + } + + // Get the composite label from the current resource + currentLabels := current.GetLabels() + if currentLabels == nil { + return desired + } + + compositeLabel, exists := currentLabels["crossplane.io/composite"] + if !exists { + return desired + } + + // Preserve the composite label in the desired resource + desiredCopy := desired.DeepCopy() + + desiredLabels := desiredCopy.GetLabels() + if desiredLabels == nil { + desiredLabels = make(map[string]string) + } + + desiredLabels["crossplane.io/composite"] = compositeLabel + desiredCopy.SetLabels(desiredLabels) + + c.logger.Debug("Preserved composite label from existing resource", + "resource", resourceID, + "compositeLabel", compositeLabel) + + return desiredCopy +} diff --git a/cmd/diff/diffprocessor/diff_calculator_test.go b/cmd/diff/diffprocessor/diff_calculator_test.go index 531c899..960dbf6 100644 --- a/cmd/diff/diffprocessor/diff_calculator_test.go +++ b/cmd/diff/diffprocessor/diff_calculator_test.go @@ -85,7 +85,7 @@ func TestDefaultDiffCalculator_CalculateDiff(t *testing.T) { Build() // Create resource manager - resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false)) + resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.NewMockResourceTreeClient().Build(), tu.TestLogger(t, false)) return applyClient, resourceTreeClient, resourceManager }, @@ -115,7 +115,7 @@ func TestDefaultDiffCalculator_CalculateDiff(t *testing.T) { Build() // Create resource manager - resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false)) + resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.NewMockResourceTreeClient().Build(), tu.TestLogger(t, false)) return applyClient, resourceTreeClient, resourceManager }, @@ -146,7 +146,7 @@ func TestDefaultDiffCalculator_CalculateDiff(t *testing.T) { Build() // Create resource manager - resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false)) + resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.NewMockResourceTreeClient().Build(), tu.TestLogger(t, false)) return applyClient, resourceTreeClient, resourceManager }, @@ -184,7 +184,7 @@ func TestDefaultDiffCalculator_CalculateDiff(t *testing.T) { Build() // Create resource manager - resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false)) + resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.NewMockResourceTreeClient().Build(), tu.TestLogger(t, false)) return applyClient, resourceTreeClient, resourceManager }, @@ -214,7 +214,7 @@ func TestDefaultDiffCalculator_CalculateDiff(t *testing.T) { Build() // Create resource manager - resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false)) + resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.NewMockResourceTreeClient().Build(), tu.TestLogger(t, false)) return applyClient, resourceTreeClient, resourceManager }, @@ -240,7 +240,7 @@ func TestDefaultDiffCalculator_CalculateDiff(t *testing.T) { Build() // Create resource manager - resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false)) + resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.NewMockResourceTreeClient().Build(), tu.TestLogger(t, false)) return applyClient, resourceTreeClient, resourceManager }, @@ -313,7 +313,7 @@ func TestDefaultDiffCalculator_CalculateDiff(t *testing.T) { Build() // Create resource manager - resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false)) + resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.NewMockResourceTreeClient().Build(), tu.TestLogger(t, false)) return applyClient, resourceTreeClient, resourceManager }, @@ -462,7 +462,7 @@ func TestDefaultDiffCalculator_CalculateDiffs(t *testing.T) { Build() // Create resource manager - resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false)) + resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.NewMockResourceTreeClient().Build(), tu.TestLogger(t, false)) return applyClient, resourceTreeClient, resourceManager }, @@ -498,7 +498,7 @@ func TestDefaultDiffCalculator_CalculateDiffs(t *testing.T) { Build() // Create resource manager - resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false)) + resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.NewMockResourceTreeClient().Build(), tu.TestLogger(t, false)) return applyClient, resourceTreeClient, resourceManager }, @@ -537,7 +537,7 @@ func TestDefaultDiffCalculator_CalculateDiffs(t *testing.T) { Build() // Create resource manager - resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false)) + resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.NewMockResourceTreeClient().Build(), tu.TestLogger(t, false)) return applyClient, resourceTreeClient, resourceManager }, @@ -580,7 +580,7 @@ func TestDefaultDiffCalculator_CalculateDiffs(t *testing.T) { Build() // Create resource manager - resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false)) + resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.NewMockResourceTreeClient().Build(), tu.TestLogger(t, false)) return applyClient, resourceTreeClient, resourceManager }, @@ -637,7 +637,7 @@ func TestDefaultDiffCalculator_CalculateDiffs(t *testing.T) { Build() // Create resource manager - resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false)) + resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.NewMockResourceTreeClient().Build(), tu.TestLogger(t, false)) return applyClient, resourceTreeClient, resourceManager }, @@ -677,6 +677,7 @@ func TestDefaultDiffCalculator_CalculateDiffs(t *testing.T) { ) // Call the function under test + // Use CalculateDiffs which includes removal detection diffs, err := calculator.CalculateDiffs(ctx, tt.inputXR, tt.renderedOut) // Check error condition @@ -771,7 +772,7 @@ func TestDefaultDiffCalculator_CalculateRemovedResourceDiffs(t *testing.T) { // Create a resource manager (not directly used in this test) resourceClient := tu.NewMockResourceClient().Build() - resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false)) + resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.NewMockResourceTreeClient().Build(), tu.TestLogger(t, false)) return applyClient, resourceTreeClient, resourceManager }, @@ -800,7 +801,7 @@ func TestDefaultDiffCalculator_CalculateRemovedResourceDiffs(t *testing.T) { // Create a resource manager (not directly used in this test) resourceClient := tu.NewMockResourceClient().Build() - resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false)) + resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.NewMockResourceTreeClient().Build(), tu.TestLogger(t, false)) return applyClient, resourceTreeClient, resourceManager }, @@ -826,7 +827,7 @@ func TestDefaultDiffCalculator_CalculateRemovedResourceDiffs(t *testing.T) { // Create a resource manager (not directly used in this test) resourceClient := tu.NewMockResourceClient().Build() - resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.TestLogger(t, false)) + resourceManager := NewResourceManager(resourceClient, tu.NewMockDefinitionClient().Build(), tu.NewMockResourceTreeClient().Build(), tu.TestLogger(t, false)) return applyClient, resourceTreeClient, resourceManager }, @@ -899,3 +900,254 @@ func TestDefaultDiffCalculator_CalculateRemovedResourceDiffs(t *testing.T) { }) } } + +func TestDefaultDiffCalculator_preserveCompositeLabel(t *testing.T) { + tests := []struct { + name string + current *un.Unstructured + desired *un.Unstructured + expectedLabel string + expectLabelExists bool + }{ + { + name: "PreservesCompositeLabelFromExistingResourceWithFullName", + current: &un.Unstructured{ + Object: map[string]any{ + "apiVersion": "nop.crossplane.io/v1alpha1", + "kind": "NopResource", + "metadata": map[string]any{ + "name": "existing-resource", + "labels": map[string]any{ + "crossplane.io/composite": "root-xr-name", + }, + }, + }, + }, + desired: &un.Unstructured{ + Object: map[string]any{ + "apiVersion": "nop.crossplane.io/v1alpha1", + "kind": "NopResource", + "metadata": map[string]any{ + "name": "existing-resource", + "labels": map[string]any{ + "crossplane.io/composite": "child-xr-name", + }, + }, + }, + }, + expectedLabel: "root-xr-name", + expectLabelExists: true, + }, + { + name: "PreservesCompositeLabelFromExistingResourceWithGenerateName", + current: &un.Unstructured{ + Object: map[string]any{ + "apiVersion": "nop.crossplane.io/v1alpha1", + "kind": "NopResource", + "metadata": map[string]any{ + "generateName": "existing-resource-", + "name": "existing-resource-abc123", + "labels": map[string]any{ + "crossplane.io/composite": "root-xr-name", + }, + }, + }, + }, + desired: &un.Unstructured{ + Object: map[string]any{ + "apiVersion": "nop.crossplane.io/v1alpha1", + "kind": "NopResource", + "metadata": map[string]any{ + "generateName": "existing-resource-", + "labels": map[string]any{ + "crossplane.io/composite": "child-xr-name", + }, + }, + }, + }, + expectedLabel: "root-xr-name", + expectLabelExists: true, + }, + { + name: "NoPreservationWhenCurrentIsNil", + current: nil, + desired: &un.Unstructured{ + Object: map[string]any{ + "apiVersion": "nop.crossplane.io/v1alpha1", + "kind": "NopResource", + "metadata": map[string]any{ + "name": "new-resource", + "labels": map[string]any{ + "crossplane.io/composite": "child-xr-name", + }, + }, + }, + }, + expectedLabel: "child-xr-name", + expectLabelExists: true, + }, + { + name: "NoPreservationWhenCurrentHasNoCompositeLabel", + current: &un.Unstructured{ + Object: map[string]any{ + "apiVersion": "nop.crossplane.io/v1alpha1", + "kind": "NopResource", + "metadata": map[string]any{ + "name": "existing-resource", + "labels": map[string]any{ + "some-other-label": "value", + }, + }, + }, + }, + desired: &un.Unstructured{ + Object: map[string]any{ + "apiVersion": "nop.crossplane.io/v1alpha1", + "kind": "NopResource", + "metadata": map[string]any{ + "name": "existing-resource", + "labels": map[string]any{ + "crossplane.io/composite": "child-xr-name", + }, + }, + }, + }, + expectedLabel: "child-xr-name", + expectLabelExists: true, + }, + { + name: "NoPreservationWhenCurrentHasNoLabels", + current: &un.Unstructured{ + Object: map[string]any{ + "apiVersion": "nop.crossplane.io/v1alpha1", + "kind": "NopResource", + "metadata": map[string]any{ + "name": "existing-resource", + }, + }, + }, + desired: &un.Unstructured{ + Object: map[string]any{ + "apiVersion": "nop.crossplane.io/v1alpha1", + "kind": "NopResource", + "metadata": map[string]any{ + "name": "existing-resource", + "labels": map[string]any{ + "crossplane.io/composite": "child-xr-name", + }, + }, + }, + }, + expectedLabel: "child-xr-name", + expectLabelExists: true, + }, + { + name: "CreatesLabelsMapWhenDesiredHasNoLabels", + current: &un.Unstructured{ + Object: map[string]any{ + "apiVersion": "nop.crossplane.io/v1alpha1", + "kind": "NopResource", + "metadata": map[string]any{ + "name": "existing-resource", + "labels": map[string]any{ + "crossplane.io/composite": "root-xr-name", + }, + }, + }, + }, + desired: &un.Unstructured{ + Object: map[string]any{ + "apiVersion": "nop.crossplane.io/v1alpha1", + "kind": "NopResource", + "metadata": map[string]any{ + "name": "existing-resource", + }, + }, + }, + expectedLabel: "root-xr-name", + expectLabelExists: true, + }, + { + name: "PreservesOtherLabelsOnDesiredResource", + current: &un.Unstructured{ + Object: map[string]any{ + "apiVersion": "nop.crossplane.io/v1alpha1", + "kind": "NopResource", + "metadata": map[string]any{ + "name": "existing-resource", + "labels": map[string]any{ + "crossplane.io/composite": "root-xr-name", + }, + }, + }, + }, + desired: &un.Unstructured{ + Object: map[string]any{ + "apiVersion": "nop.crossplane.io/v1alpha1", + "kind": "NopResource", + "metadata": map[string]any{ + "name": "existing-resource", + "labels": map[string]any{ + "crossplane.io/composite": "child-xr-name", + "custom-label": "custom-value", + "another-label": "another-value", + }, + }, + }, + }, + expectedLabel: "root-xr-name", + expectLabelExists: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a DiffCalculator instance + logger := tu.TestLogger(t, false) + calc := &DefaultDiffCalculator{ + logger: logger, + } + + // Call the function + result := calc.preserveCompositeLabel(tt.current, tt.desired, "test-resource") + + // Verify the result + labels := result.GetLabels() + if tt.expectLabelExists { + if labels == nil { + t.Fatal("Expected labels map to exist, but it was nil") + } + + actualLabel, exists := labels["crossplane.io/composite"] + if !exists { + t.Fatal("Expected crossplane.io/composite label to exist, but it did not") + } + + if actualLabel != tt.expectedLabel { + t.Errorf("Expected composite label to be %q, got %q", tt.expectedLabel, actualLabel) + } + + // Verify that result is a deep copy only when we actually preserved a label + // (i.e., when current had a composite label to preserve) + if tt.current != nil && tt.current.GetLabels() != nil { + if _, hasCompositeLabel := tt.current.GetLabels()["crossplane.io/composite"]; hasCompositeLabel { + if result == tt.desired { + t.Error("Expected result to be a deep copy when preserving label, but got the same pointer") + } + } + } + + // Verify other labels are preserved + if tt.name == "PreservesOtherLabelsOnDesiredResource" { + if labels["custom-label"] != "custom-value" { + t.Errorf("Expected custom-label to be preserved as 'custom-value', got %q", labels["custom-label"]) + } + + if labels["another-label"] != "another-value" { + t.Errorf("Expected another-label to be preserved as 'another-value', got %q", labels["another-label"]) + } + } + } + }) + } +} diff --git a/cmd/diff/diffprocessor/diff_processor.go b/cmd/diff/diffprocessor/diff_processor.go index 965bcf4..a79ff68 100644 --- a/cmd/diff/diffprocessor/diff_processor.go +++ b/cmd/diff/diffprocessor/diff_processor.go @@ -49,6 +49,7 @@ type DefaultDiffProcessor struct { compClient xp.CompositionClient defClient xp.DefinitionClient schemaClient k8.SchemaClient + resourceManager ResourceManager config ProcessorConfig functionProvider FunctionProvider schemaValidator SchemaValidator @@ -85,7 +86,7 @@ func NewDiffProcessor(k8cs k8.Clients, xpcs xp.Clients, opts ...ProcessorOption) diffOpts := config.GetDiffOptions() // Create components using factories - resourceManager := config.Factories.ResourceManager(k8cs.Resource, xpcs.Definition, config.Logger) + resourceManager := config.Factories.ResourceManager(k8cs.Resource, xpcs.Definition, xpcs.ResourceTree, config.Logger) schemaValidator := config.Factories.SchemaValidator(k8cs.Schema, xpcs.Definition, config.Logger) requirementsProvider := config.Factories.RequirementsProvider(k8cs.Resource, xpcs.Environment, config.RenderFunc, config.Logger) diffCalculator := config.Factories.DiffCalculator(k8cs.Apply, xpcs.ResourceTree, resourceManager, config.Logger, diffOpts) @@ -96,6 +97,7 @@ func NewDiffProcessor(k8cs k8.Clients, xpcs xp.Clients, opts ...ProcessorOption) compClient: xpcs.Composition, defClient: xpcs.Definition, schemaClient: k8cs.Schema, + resourceManager: resourceManager, config: config, functionProvider: functionProvider, schemaValidator: schemaValidator, @@ -201,20 +203,28 @@ func (p *DefaultDiffProcessor) PerformDiff(ctx context.Context, stdout io.Writer // DiffSingleResource handles one resource at a time and returns its diffs. // The compositionProvider function is called to obtain the composition to use for rendering. +// This is the public method for top-level XR diffing, which enables removal detection. func (p *DefaultDiffProcessor) DiffSingleResource(ctx context.Context, res *un.Unstructured, compositionProvider types.CompositionProvider) (map[string]*dt.ResourceDiff, error) { + diffs, _, err := p.diffSingleResourceInternal(ctx, res, compositionProvider, true) + return diffs, err +} + +// diffSingleResourceInternal is the internal implementation that allows control over removal detection. +// detectRemovals should be true for top-level XRs and false for nested XRs (which don't own their composed resources). +func (p *DefaultDiffProcessor) diffSingleResourceInternal(ctx context.Context, res *un.Unstructured, compositionProvider types.CompositionProvider, detectRemovals bool) (map[string]*dt.ResourceDiff, map[string]bool, error) { resourceID := fmt.Sprintf("%s/%s", res.GetKind(), res.GetName()) p.config.Logger.Debug("Processing resource", "resource", resourceID) xr, done, err := p.SanitizeXR(res, resourceID) if done { - return nil, err + return nil, nil, err } // Get the composition using the provided function comp, err := compositionProvider(ctx, res) if err != nil { p.config.Logger.Debug("Failed to get composition", "resource", resourceID, "error", err) - return nil, errors.Wrap(err, "cannot get composition") + return nil, nil, errors.Wrap(err, "cannot get composition") } p.config.Logger.Debug("Resource setup complete", "resource", resourceID, "composition", comp.GetName()) @@ -223,7 +233,7 @@ func (p *DefaultDiffProcessor) DiffSingleResource(ctx context.Context, res *un.U fns, err := p.functionProvider.GetFunctionsForComposition(comp) if err != nil { p.config.Logger.Debug("Failed to get functions", "resource", resourceID, "error", err) - return nil, errors.Wrap(err, "cannot get functions for composition") + return nil, nil, errors.Wrap(err, "cannot get functions for composition") } // Note: Serialization mutex prevents concurrent Docker operations. @@ -233,12 +243,33 @@ func (p *DefaultDiffProcessor) DiffSingleResource(ctx context.Context, res *un.U err = p.applyXRDDefaults(ctx, xr, resourceID) if err != nil { p.config.Logger.Debug("Failed to apply XRD defaults", "resource", resourceID, "error", err) - return nil, errors.Wrap(err, "cannot apply XRD defaults") + return nil, nil, errors.Wrap(err, "cannot apply XRD defaults") + } + + // Fetch the existing XR from the cluster to populate UID and other cluster-specific fields. + // This ensures that when composition functions set owner references on nested resources, + // they use the correct UID from the cluster, preventing duplicate owner reference errors. + existingXRFromCluster, isNew, err := p.resourceManager.FetchCurrentObject(ctx, nil, xr.GetUnstructured()) + switch { + case err == nil && !isNew && existingXRFromCluster != nil: + // Preserve cluster-specific fields from the existing XR + xr.SetUID(existingXRFromCluster.GetUID()) + xr.SetResourceVersion(existingXRFromCluster.GetResourceVersion()) + p.config.Logger.Debug("Populated XR with cluster UID before rendering", + "resource", resourceID, + "uid", existingXRFromCluster.GetUID()) + case isNew: + p.config.Logger.Debug("XR is new (will render without UID)", "resource", resourceID) + default: + // Error fetching + p.config.Logger.Debug("Error fetching XR from cluster (will render without UID)", + "resource", resourceID, + "error", err) } // Fetch observed resources for use in rendering (needed for getComposedResource template function) // For new XRs that don't exist in the cluster yet, this will return an empty list - observedResources, err := p.diffCalculator.FetchObservedResources(ctx, xr) + observedResources, err := p.resourceManager.FetchObservedResources(ctx, xr) if err != nil { // Log the error but continue with empty observed resources // This handles the case where the XR doesn't exist in the cluster yet @@ -253,7 +284,7 @@ func (p *DefaultDiffProcessor) DiffSingleResource(ctx context.Context, res *un.U desired, err := p.RenderWithRequirements(ctx, xr, comp, fns, resourceID, observedResources) if err != nil { p.config.Logger.Debug("Resource rendering failed", "resource", resourceID, "error", err) - return nil, errors.Wrap(err, "cannot render resources with requirements") + return nil, nil, errors.Wrap(err, "cannot render resources with requirements") } // Merge the result of the render together with the input XR @@ -267,7 +298,7 @@ func (p *DefaultDiffProcessor) DiffSingleResource(ctx context.Context, res *un.U ) if err != nil { p.config.Logger.Debug("Failed to merge XR", "resource", resourceID, "error", err) - return nil, errors.Wrap(err, "cannot merge input XR with result of rendered XR") + return nil, nil, errors.Wrap(err, "cannot merge input XR with result of rendered XR") } // Clean up namespaces from cluster-scoped resources @@ -277,16 +308,16 @@ func (p *DefaultDiffProcessor) DiffSingleResource(ctx context.Context, res *un.U // for details on the upstream fix needed. if err := p.removeNamespacesFromClusterScopedResources(ctx, desired.ComposedResources); err != nil { p.config.Logger.Debug("Failed to clean up namespaces from cluster-scoped resources", "resource", resourceID, "error", err) - return nil, errors.Wrap(err, "cannot clean up namespaces from cluster-scoped resources") + return nil, nil, errors.Wrap(err, "cannot clean up namespaces from cluster-scoped resources") } // Validate the resources if err := p.schemaValidator.ValidateResources(ctx, xrUnstructured, desired.ComposedResources); err != nil { p.config.Logger.Debug("Resource validation failed", "resource", resourceID, "error", err) - return nil, errors.Wrap(err, "cannot validate resources") + return nil, nil, errors.Wrap(err, "cannot validate resources") } - // Calculate diffs + // Calculate diffs (without removal detection) p.config.Logger.Debug("Calculating diffs", "resource", resourceID) // Clean the XR for diff calculation - remove managed fields that can cause apply issues @@ -294,7 +325,7 @@ func (p *DefaultDiffProcessor) DiffSingleResource(ctx context.Context, res *un.U cleanXR.SetManagedFields(nil) cleanXR.SetResourceVersion("") - diffs, err := p.diffCalculator.CalculateDiffs(ctx, cleanXR, desired) + diffs, renderedResources, err := p.diffCalculator.CalculateNonRemovalDiffs(ctx, cleanXR, desired) if err != nil { // We don't fail completely if some diffs couldn't be calculated p.config.Logger.Debug("Partial error calculating diffs", "resource", resourceID, "error", err) @@ -303,41 +334,147 @@ func (p *DefaultDiffProcessor) DiffSingleResource(ctx context.Context, res *un.U // Check for nested XRs in the composed resources and process them recursively p.config.Logger.Debug("Checking for nested XRs", "resource", resourceID, "composedCount", len(desired.ComposedResources)) - nestedDiffs, err := p.ProcessNestedXRs(ctx, desired.ComposedResources, compositionProvider, resourceID, 1) + // Extract the existing XR from the cluster (if it exists) to pass to ProcessNestedXRs + // This is needed because ProcessNestedXRs fetches observed resources using the parent XR's UID, + // which is only available on the existing cluster XR, not the modified XR from the input file. + var existingXR *cmp.Unstructured + + xrDiffKey := dt.MakeDiffKey(xr.GetAPIVersion(), xr.GetKind(), xr.GetName()) + if xrDiff, ok := diffs[xrDiffKey]; ok && xrDiff.Current != nil { + // Convert from unstructured.Unstructured to composite.Unstructured + existingXR = cmp.New() + + err := runtime.DefaultUnstructuredConverter.FromUnstructured(xrDiff.Current.Object, existingXR) + if err != nil { + p.config.Logger.Debug("Failed to convert existing XR to composite unstructured", + "resource", resourceID, + "error", err) + // Continue with nil existingXR - ProcessNestedXRs will handle this case + } + } + + nestedDiffs, nestedRenderedResources, err := p.ProcessNestedXRs(ctx, desired.ComposedResources, compositionProvider, resourceID, existingXR, 1) if err != nil { p.config.Logger.Debug("Error processing nested XRs", "resource", resourceID, "error", err) - return nil, errors.Wrap(err, "cannot process nested XRs") + return nil, nil, errors.Wrap(err, "cannot process nested XRs") } + p.config.Logger.Debug("Before merging nested resources", + "resource", resourceID, + "renderedResourcesCount", len(renderedResources), + "nestedRenderedResourcesCount", len(nestedRenderedResources)) + // Merge nested diffs into our result maps.Copy(diffs, nestedDiffs) + // Merge nested rendered resources into our tracking map + // This ensures that resources from nested XRs (including unchanged ones) are not flagged as removed + maps.Copy(renderedResources, nestedRenderedResources) + + p.config.Logger.Debug("After merging nested resources", + "resource", resourceID, + "renderedResourcesCount", len(renderedResources)) + + // Now detect removals if requested (only for top-level XRs) + // This must happen after nested XR processing to avoid false positives + if detectRemovals && existingXR != nil { + p.config.Logger.Debug("Detecting removed resources", "resource", resourceID, "renderedCount", len(renderedResources)) + + removedDiffs, err := p.diffCalculator.CalculateRemovedResourceDiffs(ctx, existingXR.GetUnstructured(), renderedResources) + if err != nil { + p.config.Logger.Debug("Error detecting removed resources (continuing)", "resource", resourceID, "error", err) + } else if len(removedDiffs) > 0 { + maps.Copy(diffs, removedDiffs) + p.config.Logger.Debug("Found removed resources", "resource", resourceID, "removedCount", len(removedDiffs)) + } + } + p.config.Logger.Debug("Resource processing complete", "resource", resourceID, "diffCount", len(diffs), "nestedDiffCount", len(nestedDiffs), "hasErrors", err != nil) - return diffs, err + return diffs, renderedResources, err +} + +// findExistingNestedXR locates an existing nested XR in the observed resources by matching +// the composition-resource-name annotation and kind. +func findExistingNestedXR(nestedXR *un.Unstructured, observedResources []cpd.Unstructured) *un.Unstructured { + compositionResourceName := nestedXR.GetAnnotations()["crossplane.io/composition-resource-name"] + if compositionResourceName == "" { + return nil + } + + for _, obs := range observedResources { + obsUnstructured := &un.Unstructured{Object: obs.UnstructuredContent()} + obsCompResName := obsUnstructured.GetAnnotations()["crossplane.io/composition-resource-name"] + + // Match by composition-resource-name annotation and kind + if obsCompResName == compositionResourceName && obsUnstructured.GetKind() == nestedXR.GetKind() { + return obsUnstructured + } + } + + return nil +} + +// preserveNestedXRIdentity updates the nested XR to preserve the identity of an existing XR +// by copying its name, generateName, UID, and composite label. +func preserveNestedXRIdentity(nestedXR, existingNestedXR *un.Unstructured) { + // Preserve the actual cluster name and UID + nestedXR.SetName(existingNestedXR.GetName()) + nestedXR.SetGenerateName(existingNestedXR.GetGenerateName()) + nestedXR.SetUID(existingNestedXR.GetUID()) + + // Preserve the composite label so child resources get matched correctly + if labels := existingNestedXR.GetLabels(); labels != nil { + if compositeLabel, exists := labels["crossplane.io/composite"]; exists { + nestedXRLabels := nestedXR.GetLabels() + if nestedXRLabels == nil { + nestedXRLabels = make(map[string]string) + } + + nestedXRLabels["crossplane.io/composite"] = compositeLabel + nestedXR.SetLabels(nestedXRLabels) + } + } +} + +// mergeNestedXRSpecs merges the parent-produced nested XR spec with the existing cluster XR. +// The existing cluster XR provides the base (with all fields populated by its own composition), +// and the parent-produced spec provides overrides for fields the parent wants to change. +func mergeNestedXRSpecs(existingNestedXR, parentProducedXR *un.Unstructured) *un.Unstructured { + // Start with a deep copy of the existing cluster XR + merged := existingNestedXR.DeepCopy() + + // Merge the parent-produced spec into the existing XR's spec + // mergo.WithOverride means parent fields take precedence + // Deep merge ensures nested maps are merged recursively + _ = mergo.Merge(&merged.Object, parentProducedXR.Object, mergo.WithOverride) + + return merged } // ProcessNestedXRs recursively processes composed resources that are themselves XRs. // It checks each composed resource to see if it's an XR, and if so, processes it through -// its own composition pipeline to get the full tree of diffs. +// its own composition pipeline to get the full tree of diffs. It preserves the identity +// of existing nested XRs to ensure accurate diff calculation. func (p *DefaultDiffProcessor) ProcessNestedXRs( ctx context.Context, composedResources []cpd.Unstructured, compositionProvider types.CompositionProvider, parentResourceID string, + parentXR *cmp.Unstructured, depth int, -) (map[string]*dt.ResourceDiff, error) { +) (map[string]*dt.ResourceDiff, map[string]bool, error) { if depth > p.config.MaxNestedDepth { p.config.Logger.Debug("Maximum nesting depth exceeded", "parentResource", parentResourceID, "depth", depth, "maxDepth", p.config.MaxNestedDepth) - return nil, errors.New("maximum nesting depth exceeded") + return nil, nil, errors.New("maximum nesting depth exceeded") } p.config.Logger.Debug("Processing nested XRs", @@ -345,27 +482,71 @@ func (p *DefaultDiffProcessor) ProcessNestedXRs( "composedResourceCount", len(composedResources), "depth", depth) + // Fetch observed resources from parent XR to find existing nested XRs + // This allows us to preserve the identity of nested XRs that already exist in the cluster + var observedResources []cpd.Unstructured + + if parentXR != nil { + obs, err := p.resourceManager.FetchObservedResources(ctx, parentXR) + if err != nil { + // Log but continue - nested XRs without existing cluster state will show as new (with "(generated)") + p.config.Logger.Debug("Could not fetch observed resources for parent XR (continuing)", + "parentResource", parentResourceID, + "error", err) + } else { + observedResources = obs + } + } + allDiffs := make(map[string]*dt.ResourceDiff) + allRenderedResources := make(map[string]bool) for _, composed := range composedResources { - un := &un.Unstructured{Object: composed.UnstructuredContent()} + nestedXR := &un.Unstructured{Object: composed.UnstructuredContent()} // Check if this composed resource is itself an XR - isXR, _ := p.getCompositeResourceXRD(ctx, un) + isXR, _ := p.getCompositeResourceXRD(ctx, nestedXR) if !isXR { // Skip non-XR resources continue } - nestedResourceID := fmt.Sprintf("%s/%s (nested depth %d)", un.GetKind(), un.GetName(), depth) + nestedResourceID := fmt.Sprintf("%s/%s (nested depth %d)", nestedXR.GetKind(), nestedXR.GetName(), depth) p.config.Logger.Debug("Found nested XR, processing recursively", "nestedXR", nestedResourceID, "parentXR", parentResourceID, "depth", depth) + // Find the matching existing nested XR in observed resources (if it exists) + // Match by composition-resource-name annotation to find the correct existing resource + existingNestedXR := findExistingNestedXR(nestedXR, observedResources) + + // Determine which XR to use for rendering + xrToRender := nestedXR + if existingNestedXR != nil { + p.config.Logger.Debug("Found existing nested XR in cluster", + "nestedXR", nestedResourceID, + "existingName", existingNestedXR.GetName(), + "compositionResourceName", nestedXR.GetAnnotations()["crossplane.io/composition-resource-name"]) + + // For nested XRs, we should render based on the spec that merges: + // 1. The existing cluster XR's spec (for fields managed by the nested XR's own composition) + // 2. The parent-produced spec (for fields the parent wants to change) + // + // We start with the existing cluster XR to preserve its identity and state, + // then overlay the parent's changes on top. + xrToRender = mergeNestedXRSpecs(existingNestedXR, nestedXR) + + p.config.Logger.Debug("Merged existing cluster XR spec with parent-produced changes", + "nestedXR", nestedResourceID, + "existingName", existingNestedXR.GetName()) + } + // Recursively process this nested XR - nestedDiffs, err := p.DiffSingleResource(ctx, un, compositionProvider) + // Use detectRemovals=false for nested XRs since they don't own their composed resources + // (resources are owned by the top-level parent XR in Crossplane's ownership model) + nestedDiffs, nestedRenderedResources, err := p.diffSingleResourceInternal(ctx, xrToRender, compositionProvider, false) if err != nil { // Check if the error is due to missing composition // Note: It's valid to have an XRD in Crossplane without a composition attached to it. @@ -376,7 +557,7 @@ func (p *DefaultDiffProcessor) ProcessNestedXRs( p.config.Logger.Info("Skipping nested XR processing due to missing composition", "nestedXR", nestedResourceID, "parentXR", parentResourceID, - "gvk", un.GroupVersionKind().String()) + "gvk", nestedXR.GroupVersionKind().String()) // Continue processing other nested XRs continue } @@ -387,23 +568,28 @@ func (p *DefaultDiffProcessor) ProcessNestedXRs( "parentXR", parentResourceID, "error", err) - return nil, errors.Wrapf(err, "cannot process nested XR %s", nestedResourceID) + return nil, nil, errors.Wrapf(err, "cannot process nested XR %s", nestedResourceID) } // Merge diffs from nested XR maps.Copy(allDiffs, nestedDiffs) + // Merge rendered resources from nested XR + maps.Copy(allRenderedResources, nestedRenderedResources) p.config.Logger.Debug("Nested XR processed successfully", "nestedXR", nestedResourceID, - "diffCount", len(nestedDiffs)) + "diffCount", len(nestedDiffs), + "nestedRenderedResourcesCount", len(nestedRenderedResources), + "allRenderedResourcesCount", len(allRenderedResources)) } p.config.Logger.Debug("Finished processing nested XRs", "parentResource", parentResourceID, "totalNestedDiffs", len(allDiffs), + "totalRenderedResourcesCount", len(allRenderedResources), "depth", depth) - return allDiffs, nil + return allDiffs, allRenderedResources, nil } // SanitizeXR makes an XR into a valid unstructured object that we can use in a dry-run apply. diff --git a/cmd/diff/diffprocessor/diff_processor_test.go b/cmd/diff/diffprocessor/diff_processor_test.go index 244f2da..20d4138 100644 --- a/cmd/diff/diffprocessor/diff_processor_test.go +++ b/cmd/diff/diffprocessor/diff_processor_test.go @@ -415,8 +415,9 @@ func TestDefaultDiffProcessor_PerformDiff(t *testing.T) { // Override the diff calculator factory to return actual diffs WithDiffCalculatorFactory(func(k8.ApplyClient, xp.ResourceTreeClient, ResourceManager, logging.Logger, renderer.DiffOptions) DiffCalculator { return &tu.MockDiffCalculator{ - CalculateDiffsFn: func(context.Context, *cmp.Unstructured, render.Outputs) (map[string]*dt.ResourceDiff, error) { + CalculateNonRemovalDiffsFn: func(context.Context, *cmp.Unstructured, render.Outputs) (map[string]*dt.ResourceDiff, map[string]bool, error) { diffs := make(map[string]*dt.ResourceDiff) + rendered := make(map[string]bool) // Add a modified diff (not just equal) lineDiffs := []diffmatchpatch.Diff{ @@ -424,7 +425,8 @@ func TestDefaultDiffProcessor_PerformDiff(t *testing.T) { {Type: diffmatchpatch.DiffInsert, Text: " field: new-value"}, } - diffs["example.org/v1/XR1/test-xr"] = &dt.ResourceDiff{ + diffKey1 := "example.org/v1/XR1/test-xr" + diffs[diffKey1] = &dt.ResourceDiff{ Gvk: schema.GroupVersionKind{Group: "example.org", Version: "v1", Kind: "XR1"}, ResourceName: "test-xr", DiffType: dt.DiffTypeModified, @@ -432,9 +434,11 @@ func TestDefaultDiffProcessor_PerformDiff(t *testing.T) { Current: resource1, // Set current for completeness Desired: resource1, // Set desired for completeness } + rendered[diffKey1] = true // Add a composed resource diff that's also modified - diffs["example.org/v1/ComposedResource/resource-a"] = &dt.ResourceDiff{ + diffKey2 := "example.org/v1/ComposedResource/resource-a" + diffs[diffKey2] = &dt.ResourceDiff{ Gvk: schema.GroupVersionKind{Group: "example.org", Version: "v1", Kind: "ComposedResource"}, ResourceName: "resource-a", DiffType: dt.DiffTypeModified, @@ -442,8 +446,9 @@ func TestDefaultDiffProcessor_PerformDiff(t *testing.T) { Current: composedResource, Desired: composedResource, } + rendered[diffKey2] = true - return diffs, nil + return diffs, rendered, nil }, } }), @@ -1548,6 +1553,7 @@ func TestDefaultDiffProcessor_ProcessNestedXRs(t *testing.T) { Environment: tu.NewMockEnvironmentClient(). WithNoEnvironmentConfigs(). Build(), + ResourceTree: tu.NewMockResourceTreeClient().Build(), } // Create a child CRD with proper schema for childField @@ -1643,6 +1649,7 @@ func TestDefaultDiffProcessor_ProcessNestedXRs(t *testing.T) { Environment: tu.NewMockEnvironmentClient(). WithNoEnvironmentConfigs(). Build(), + ResourceTree: tu.NewMockResourceTreeClient().Build(), } // Create a child CRD with proper schema for childField @@ -1685,6 +1692,115 @@ func TestDefaultDiffProcessor_ProcessNestedXRs(t *testing.T) { wantDiffCount: 1, // Only the child XR should be processed wantErr: false, }, + "NestedXRWithExistingResourcesPreservesIdentity": { + setupMocks: func() (xp.Clients, k8.Clients) { + // This test reproduces the bug where existing nested XR identity is not preserved + // resulting in all managed resources showing as removed/added instead of modified + + // Create an EXISTING nested XR with actual cluster name (not generateName) + existingChildXR := tu.NewResource("nested.example.org/v1alpha1", "XChildResource", "parent-xr-child-abc123"). + WithGenerateName("parent-xr-"). + WithSpecField("childField", "existing-value"). + WithCompositionResourceName("child-xr"). + WithLabels(map[string]string{ + "crossplane.io/composite": "parent-xr-abc", // Existing composite label + }). + Build() + + // Create an existing managed resource owned by the nested XR + existingManagedResource := tu.NewResource("nop.example.org/v1alpha1", "NopResource", "parent-xr-child-abc123-managed-xyz"). + WithGenerateName("parent-xr-child-abc123-"). + WithSpecField("forProvider", map[string]any{ + "configData": "existing-data", + }). + WithCompositionResourceName("managed-resource"). + WithLabels(map[string]string{ + "crossplane.io/composite": "parent-xr-child-abc123", // Points to existing nested XR + }). + Build() + + // Create a parent XR that owns the nested XR + parentXR := tu.NewResource("parent.example.org/v1alpha1", "XParentResource", "parent-xr-abc"). + WithGenerateName("parent-xr-"). + Build() + + // Create functions + functions := []pkgv1.Function{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "function-go-templating", + }, + Spec: pkgv1.FunctionSpec{ + PackageSpec: pkgv1.PackageSpec{ + Package: "xpkg.crossplane.io/crossplane-contrib/function-go-templating:v0.11.0", + }, + }, + }, + } + + xpClients := xp.Clients{ + Definition: tu.NewMockDefinitionClient(). + WithXRD(childXRD). + Build(), + Composition: tu.NewMockCompositionClient(). + WithComposition(childComposition). + Build(), + Function: tu.NewMockFunctionClient(). + WithSuccessfulFunctionsFetch(functions). + Build(), + Environment: tu.NewMockEnvironmentClient(). + WithNoEnvironmentConfigs(). + Build(), + // Mock resource tree to return existing nested XR and its managed resources + ResourceTree: tu.NewMockResourceTreeClient(). + WithResourceTreeFromXRAndComposed( + parentXR, + []*un.Unstructured{existingChildXR, existingManagedResource}, + ). + Build(), + } + + // Create CRDs + childCRD := tu.NewCRD("xchildresources.nested.example.org", "nested.example.org", "XChildResource"). + WithListKind("XChildResourceList"). + WithPlural("xchildresources"). + WithSingular("xchildresource"). + WithVersion("v1alpha1", true, true). + WithStandardSchema("childField"). + Build() + + nopCRD := tu.NewCRD("nopresources.nop.example.org", "nop.example.org", "NopResource"). + WithListKind("NopResourceList"). + WithPlural("nopresources"). + WithSingular("nopresource"). + WithVersion("v1alpha1", true, true). + WithStandardSchema("configData"). + Build() + + k8sClients := k8.Clients{ + Apply: tu.NewMockApplyClient().Build(), + Resource: tu.NewMockResourceClient().Build(), + Schema: tu.NewMockSchemaClient(). + WithFoundCRD("nested.example.org", "XChildResource", childCRD). + WithFoundCRD("nop.example.org", "NopResource", nopCRD). + WithSuccessfulCRDByNameFetch("xchildresources.nested.example.org", childCRD). + Build(), + Type: tu.NewMockTypeConverter().Build(), + } + + return xpClients, k8sClients + }, + composedResources: []cpd.Unstructured{ + // The RENDERED nested XR (from parent composition) with generateName but no name + {Unstructured: *childXR}, + }, + parentResourceID: "XParentResource/parent-xr-abc", + depth: 1, + // With identity preservation fix, nested XR maintains its cluster identity + // so managed resources show as modified rather than removed/added + wantDiffCount: 1, // Just the nested XR diff, not its managed resources as separate remove/add + wantErr: false, + }, } for name, tt := range tests { @@ -1704,9 +1820,10 @@ func TestDefaultDiffProcessor_ProcessNestedXRs(t *testing.T) { }), WithDiffCalculatorFactory(func(k8.ApplyClient, xp.ResourceTreeClient, ResourceManager, logging.Logger, renderer.DiffOptions) DiffCalculator { return &tu.MockDiffCalculator{ - CalculateDiffsFn: func(_ context.Context, xr *cmp.Unstructured, _ render.Outputs) (map[string]*dt.ResourceDiff, error) { + CalculateNonRemovalDiffsFn: func(_ context.Context, xr *cmp.Unstructured, _ render.Outputs) (map[string]*dt.ResourceDiff, map[string]bool, error) { // Return a simple diff for the XR to make the test pass diffs := make(map[string]*dt.ResourceDiff) + rendered := make(map[string]bool) gvk := xr.GroupVersionKind() resourceID := gvk.Kind + "/" + xr.GetName() diffs[resourceID] = &dt.ResourceDiff{ @@ -1714,8 +1831,9 @@ func TestDefaultDiffProcessor_ProcessNestedXRs(t *testing.T) { ResourceName: xr.GetName(), DiffType: dt.DiffTypeAdded, } + rendered[resourceID] = true - return diffs, nil + return diffs, rendered, nil }, } }), @@ -1730,8 +1848,11 @@ func TestDefaultDiffProcessor_ProcessNestedXRs(t *testing.T) { return xpClients.Composition.FindMatchingComposition(ctx, res) } + // Create a mock parent XR (nil is acceptable for tests that don't need observed resources) + var parentXR *cmp.Unstructured + // Call the method under test - diffs, err := processor.ProcessNestedXRs(ctx, tt.composedResources, compositionProvider, tt.parentResourceID, tt.depth) + diffs, _, err := processor.ProcessNestedXRs(ctx, tt.composedResources, compositionProvider, tt.parentResourceID, parentXR, tt.depth) // Check error if (err != nil) != tt.wantErr { diff --git a/cmd/diff/diffprocessor/processor_config.go b/cmd/diff/diffprocessor/processor_config.go index a234705..f541334 100644 --- a/cmd/diff/diffprocessor/processor_config.go +++ b/cmd/diff/diffprocessor/processor_config.go @@ -46,7 +46,7 @@ type ProcessorConfig struct { // ComponentFactories contains factory functions for creating processor components. type ComponentFactories struct { // ResourceManager creates a ResourceManager - ResourceManager func(client k8.ResourceClient, defClient xp.DefinitionClient, logger logging.Logger) ResourceManager + ResourceManager func(client k8.ResourceClient, defClient xp.DefinitionClient, treeClient xp.ResourceTreeClient, logger logging.Logger) ResourceManager // SchemaValidator creates a SchemaValidator SchemaValidator func(schema k8.SchemaClient, def xp.DefinitionClient, logger logging.Logger) SchemaValidator @@ -131,7 +131,7 @@ func WithRenderMutex(mu *sync.Mutex) ProcessorOption { } // WithResourceManagerFactory sets the ResourceManager factory function. -func WithResourceManagerFactory(factory func(k8.ResourceClient, xp.DefinitionClient, logging.Logger) ResourceManager) ProcessorOption { +func WithResourceManagerFactory(factory func(k8.ResourceClient, xp.DefinitionClient, xp.ResourceTreeClient, logging.Logger) ResourceManager) ProcessorOption { return func(config *ProcessorConfig) { config.Factories.ResourceManager = factory } diff --git a/cmd/diff/diffprocessor/requirements_provider.go b/cmd/diff/diffprocessor/requirements_provider.go index 746c429..1c4fb41 100644 --- a/cmd/diff/diffprocessor/requirements_provider.go +++ b/cmd/diff/diffprocessor/requirements_provider.go @@ -2,12 +2,12 @@ package diffprocessor import ( "context" - "fmt" "strings" "sync" xp "github.com/crossplane-contrib/crossplane-diff/cmd/diff/client/crossplane" k8 "github.com/crossplane-contrib/crossplane-diff/cmd/diff/client/kubernetes" + dt "github.com/crossplane-contrib/crossplane-diff/cmd/diff/renderer/types" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" un "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" @@ -76,7 +76,7 @@ func (p *RequirementsProvider) cacheResources(resources []*un.Unstructured) { defer p.cacheMutex.Unlock() for _, res := range resources { - key := fmt.Sprintf("%s/%s/%s", res.GetAPIVersion(), res.GetKind(), res.GetName()) + key := dt.MakeDiffKey(res.GetAPIVersion(), res.GetKind(), res.GetName()) p.resourceCache[key] = res } } @@ -86,7 +86,7 @@ func (p *RequirementsProvider) getCachedResource(apiVersion, kind, name string) p.cacheMutex.RLock() defer p.cacheMutex.RUnlock() - key := fmt.Sprintf("%s/%s/%s", apiVersion, kind, name) + key := dt.MakeDiffKey(apiVersion, kind, name) return p.resourceCache[key] } diff --git a/cmd/diff/diffprocessor/resource_manager.go b/cmd/diff/diffprocessor/resource_manager.go index 83e991e..163aabb 100644 --- a/cmd/diff/diffprocessor/resource_manager.go +++ b/cmd/diff/diffprocessor/resource_manager.go @@ -16,6 +16,10 @@ import ( "github.com/crossplane/crossplane-runtime/v2/pkg/errors" "github.com/crossplane/crossplane-runtime/v2/pkg/logging" + cpd "github.com/crossplane/crossplane-runtime/v2/pkg/resource/unstructured/composed" + cmp "github.com/crossplane/crossplane-runtime/v2/pkg/resource/unstructured/composite" + + "github.com/crossplane/crossplane/v2/cmd/crank/common/resource" ) // ResourceManager handles resource-related operations like fetching, updating owner refs, @@ -26,21 +30,26 @@ type ResourceManager interface { // UpdateOwnerRefs ensures all OwnerReferences have valid UIDs UpdateOwnerRefs(ctx context.Context, parent *un.Unstructured, child *un.Unstructured) + + // FetchObservedResources fetches the observed composed resources for the given XR + FetchObservedResources(ctx context.Context, xr *cmp.Unstructured) ([]cpd.Unstructured, error) } // DefaultResourceManager implements ResourceManager interface. type DefaultResourceManager struct { - client k8.ResourceClient - defClient xp.DefinitionClient - logger logging.Logger + client k8.ResourceClient + defClient xp.DefinitionClient + treeClient xp.ResourceTreeClient + logger logging.Logger } // NewResourceManager creates a new DefaultResourceManager. -func NewResourceManager(client k8.ResourceClient, defClient xp.DefinitionClient, logger logging.Logger) ResourceManager { +func NewResourceManager(client k8.ResourceClient, defClient xp.DefinitionClient, treeClient xp.ResourceTreeClient, logger logging.Logger) ResourceManager { return &DefaultResourceManager{ - client: client, - defClient: defClient, - logger: logger, + client: client, + defClient: defClient, + treeClient: treeClient, + logger: logger, } } @@ -509,12 +518,83 @@ func (m *DefaultResourceManager) updateCompositeOwnerLabel(ctx context.Context, "child", child.GetName()) } default: - // For XRs, set the composite label - labels["crossplane.io/composite"] = parentName - m.logger.Debug("Updated composite owner label", - "composite", parentName, - "child", child.GetName()) + // For XRs, only set the composite label if it doesn't already exist + // If it exists, preserve it (e.g., for managed resources that already have correct ownership) + existingComposite, hasComposite := labels["crossplane.io/composite"] + if !hasComposite { + // No existing composite label, set it to the parent XR name + labels["crossplane.io/composite"] = parentName + m.logger.Debug("Set composite label for new XR resource", + "xrName", parentName, + "child", child.GetName()) + } else { + // Preserve existing composite label + m.logger.Debug("Preserved existing composite label for XR resource", + "xrName", parentName, + "existingComposite", existingComposite, + "child", child.GetName()) + } } child.SetLabels(labels) } + +// FetchObservedResources fetches the observed composed resources for the given XR. +// Returns a flat slice of composed resources suitable for render.Inputs.ObservedResources. +func (m *DefaultResourceManager) FetchObservedResources(ctx context.Context, xr *cmp.Unstructured) ([]cpd.Unstructured, error) { + m.logger.Debug("Fetching observed resources for XR", + "xr_kind", xr.GetKind(), + "xr_name", xr.GetName()) + + // Get the resource tree from the cluster + tree, err := m.treeClient.GetResourceTree(ctx, &xr.Unstructured) + if err != nil { + m.logger.Debug("Failed to get resource tree for XR", + "xr", xr.GetName(), + "error", err) + + return nil, errors.Wrap(err, "cannot get resource tree") + } + + // Extract composed resources from the tree + observed := extractComposedResourcesFromTree(tree) + + m.logger.Debug("Fetched observed composed resources", + "xr", xr.GetName(), + "count", len(observed)) + + return observed, nil +} + +// extractComposedResourcesFromTree recursively extracts all composed resources from a resource tree. +// It returns a flat slice of composed resources, suitable for render.Inputs.ObservedResources. +// Only includes resources with the crossplane.io/composition-resource-name annotation. +func extractComposedResourcesFromTree(tree *resource.Resource) []cpd.Unstructured { + var resources []cpd.Unstructured + + // Recursively collect composed resources from the tree + var collectResources func(node *resource.Resource) + + collectResources = func(node *resource.Resource) { + // Only include resources that have the composition-resource-name annotation + // (this filters out the root XR and non-composed resources) + if _, hasAnno := node.Unstructured.GetAnnotations()["crossplane.io/composition-resource-name"]; hasAnno { + // Convert to cpd.Unstructured (composed resource) + resources = append(resources, cpd.Unstructured{ + Unstructured: node.Unstructured, + }) + } + + // Recursively process children + for _, child := range node.Children { + collectResources(child) + } + } + + // Start from root's children to avoid including the XR itself + for _, child := range tree.Children { + collectResources(child) + } + + return resources +} diff --git a/cmd/diff/diffprocessor/resource_manager_test.go b/cmd/diff/diffprocessor/resource_manager_test.go index 28fb6e2..f0cc564 100644 --- a/cmd/diff/diffprocessor/resource_manager_test.go +++ b/cmd/diff/diffprocessor/resource_manager_test.go @@ -2,10 +2,12 @@ package diffprocessor import ( "context" + "sort" "strings" "testing" tu "github.com/crossplane-contrib/crossplane-diff/cmd/diff/testutils" + gcmp "github.com/google/go-cmp/cmp" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" un "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -13,6 +15,9 @@ import ( "k8s.io/utils/ptr" "github.com/crossplane/crossplane-runtime/v2/pkg/errors" + cmp "github.com/crossplane/crossplane-runtime/v2/pkg/resource/unstructured/composite" + + "github.com/crossplane/crossplane/v2/cmd/crank/common/resource" ) const ( @@ -334,7 +339,7 @@ func TestDefaultResourceManager_FetchCurrentObject(t *testing.T) { // Create the resource manager resourceClient := tt.setupResourceClient() - rm := NewResourceManager(resourceClient, tt.defClient, tu.TestLogger(t, false)) + rm := NewResourceManager(resourceClient, tt.defClient, tu.NewMockResourceTreeClient().Build(), tu.TestLogger(t, false)) // Call the method under test current, isNew, err := rm.FetchCurrentObject(ctx, tt.composite, tt.desired) @@ -721,7 +726,7 @@ func TestDefaultResourceManager_UpdateOwnerRefs(t *testing.T) { for name, tt := range tests { t.Run(name, func(t *testing.T) { // Create the resource manager - rm := NewResourceManager(tu.NewMockResourceClient().Build(), tt.defClient, tu.TestLogger(t, false)) + rm := NewResourceManager(tu.NewMockResourceClient().Build(), tt.defClient, tu.NewMockResourceTreeClient().Build(), tu.TestLogger(t, false)) // Need to create a copy of the child to avoid modifying test data child := tt.child.DeepCopy() @@ -1062,3 +1067,271 @@ func TestDefaultResourceManager_updateCompositeOwnerLabel_EdgeCases(t *testing.T }) } } + +func TestDefaultResourceManager_FetchObservedResources(t *testing.T) { + ctx := t.Context() + + // Create test XR + testXR := tu.NewResource("example.org/v1", "XR", "test-xr"). + WithSpecField("field", "value"). + Build() + + // Create composed resources with composition-resource-name annotation + composedResource1 := tu.NewResource("example.org/v1", "ManagedResource", "resource-1"). + WithSpecField("field", "value1"). + WithAnnotations(map[string]string{ + "crossplane.io/composition-resource-name": "db-instance", + }). + Build() + + composedResource2 := tu.NewResource("example.org/v1", "ManagedResource", "resource-2"). + WithSpecField("field", "value2"). + WithAnnotations(map[string]string{ + "crossplane.io/composition-resource-name": "db-subnet", + }). + Build() + + // Create a nested XR (child XR with composition-resource-name annotation) + nestedXR := tu.NewResource("example.org/v1", "ChildXR", "nested-xr"). + WithSpecField("nested", "value"). + WithAnnotations(map[string]string{ + "crossplane.io/composition-resource-name": "child-xr", + }). + Build() + + // Create composed resources under the nested XR + nestedComposedResource := tu.NewResource("example.org/v1", "NestedResource", "nested-resource-1"). + WithSpecField("field", "nested-value"). + WithAnnotations(map[string]string{ + "crossplane.io/composition-resource-name": "nested-db", + }). + Build() + + // Create a resource without the composition-resource-name annotation (should be filtered out) + resourceWithoutAnnotation := tu.NewResource("example.org/v1", "OtherResource", "other-resource"). + WithSpecField("field", "other"). + Build() + + tests := map[string]struct { + setupTreeClient func() *tu.MockResourceTreeClient + xr *un.Unstructured + wantCount int + wantResourceIDs []string // Names of resources we expect to find + wantErr bool + }{ + "SuccessfullyFetchesFlatComposedResources": { + setupTreeClient: func() *tu.MockResourceTreeClient { + return tu.NewMockResourceTreeClient(). + WithResourceTreeFromXRAndComposed(testXR, []*un.Unstructured{ + composedResource1, + composedResource2, + }). + Build() + }, + xr: testXR, + wantCount: 2, + wantResourceIDs: []string{"resource-1", "resource-2"}, + wantErr: false, + }, + "SuccessfullyFetchesNestedResources": { + setupTreeClient: func() *tu.MockResourceTreeClient { + // Build a tree with nested structure: + // XR + // -> composedResource1 + // -> nestedXR + // -> nestedComposedResource + return tu.NewMockResourceTreeClient(). + WithGetResourceTree(func(_ context.Context, _ *un.Unstructured) (*resource.Resource, error) { + return &resource.Resource{ + Unstructured: *testXR.DeepCopy(), + Children: []*resource.Resource{ + { + Unstructured: *composedResource1.DeepCopy(), + Children: []*resource.Resource{}, + }, + { + Unstructured: *nestedXR.DeepCopy(), + Children: []*resource.Resource{ + { + Unstructured: *nestedComposedResource.DeepCopy(), + Children: []*resource.Resource{}, + }, + }, + }, + }, + }, nil + }). + Build() + }, + xr: testXR, + wantCount: 3, // composedResource1, nestedXR, nestedComposedResource + wantResourceIDs: []string{"resource-1", "nested-xr", "nested-resource-1"}, + wantErr: false, + }, + "FiltersOutResourcesWithoutAnnotation": { + setupTreeClient: func() *tu.MockResourceTreeClient { + return tu.NewMockResourceTreeClient(). + WithGetResourceTree(func(_ context.Context, _ *un.Unstructured) (*resource.Resource, error) { + return &resource.Resource{ + Unstructured: *testXR.DeepCopy(), + Children: []*resource.Resource{ + { + Unstructured: *composedResource1.DeepCopy(), + Children: []*resource.Resource{}, + }, + { + // This resource lacks the annotation and should be filtered out + Unstructured: *resourceWithoutAnnotation.DeepCopy(), + Children: []*resource.Resource{}, + }, + { + Unstructured: *composedResource2.DeepCopy(), + Children: []*resource.Resource{}, + }, + }, + }, nil + }). + Build() + }, + xr: testXR, + wantCount: 2, // Only composedResource1 and composedResource2 + wantResourceIDs: []string{"resource-1", "resource-2"}, + wantErr: false, + }, + "ReturnsEmptySliceWhenNoComposedResources": { + setupTreeClient: func() *tu.MockResourceTreeClient { + return tu.NewMockResourceTreeClient(). + WithEmptyResourceTree(). + Build() + }, + xr: testXR, + wantCount: 0, + wantResourceIDs: []string{}, + wantErr: false, + }, + "ReturnsEmptySliceWhenOnlyRootXRInTree": { + setupTreeClient: func() *tu.MockResourceTreeClient { + return tu.NewMockResourceTreeClient(). + WithGetResourceTree(func(_ context.Context, _ *un.Unstructured) (*resource.Resource, error) { + // Tree with only root (XR itself has no composition-resource-name) + return &resource.Resource{ + Unstructured: *testXR.DeepCopy(), + Children: []*resource.Resource{}, + }, nil + }). + Build() + }, + xr: testXR, + wantCount: 0, + wantResourceIDs: []string{}, + wantErr: false, + }, + "ReturnsErrorWhenTreeClientFails": { + setupTreeClient: func() *tu.MockResourceTreeClient { + return tu.NewMockResourceTreeClient(). + WithFailedResourceTreeFetch("failed to get tree"). + Build() + }, + xr: testXR, + wantErr: true, + }, + "HandlesDeepNestedStructure": { + setupTreeClient: func() *tu.MockResourceTreeClient { + // Build a deeply nested tree: + // XR + // -> composedResource1 + // -> nestedXR + // -> nestedComposedResource + // -> composedResource2 + return tu.NewMockResourceTreeClient(). + WithGetResourceTree(func(_ context.Context, _ *un.Unstructured) (*resource.Resource, error) { + return &resource.Resource{ + Unstructured: *testXR.DeepCopy(), + Children: []*resource.Resource{ + { + Unstructured: *composedResource1.DeepCopy(), + Children: []*resource.Resource{ + { + Unstructured: *nestedXR.DeepCopy(), + Children: []*resource.Resource{ + { + Unstructured: *nestedComposedResource.DeepCopy(), + Children: []*resource.Resource{ + { + Unstructured: *composedResource2.DeepCopy(), + Children: []*resource.Resource{}, + }, + }, + }, + }, + }, + }, + }, + }, + }, nil + }). + Build() + }, + xr: testXR, + wantCount: 4, // All 4 resources have the annotation + wantResourceIDs: []string{"resource-1", "nested-xr", "nested-resource-1", "resource-2"}, + wantErr: false, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + // Use the constructor and interface type to test via the public API + rm := NewResourceManager( + nil, // client not used by FetchObservedResources + nil, // defClient not used by FetchObservedResources + tt.setupTreeClient(), + tu.TestLogger(t, false), + ) + + observed, err := rm.FetchObservedResources(ctx, &cmp.Unstructured{Unstructured: *tt.xr}) + + if tt.wantErr { + if err == nil { + t.Errorf("FetchObservedResources() expected error, got nil") + } + + return + } + + if err != nil { + t.Errorf("FetchObservedResources() unexpected error: %v", err) + return + } + + if len(observed) != tt.wantCount { + t.Errorf("FetchObservedResources() got %d resources, want %d", len(observed), tt.wantCount) + } + + // Verify we got the expected resources + if len(tt.wantResourceIDs) > 0 { + // Extract resource IDs from observed resources + var gotResourceIDs []string + for _, res := range observed { + gotResourceIDs = append(gotResourceIDs, res.GetName()) + } + + sort.Strings(gotResourceIDs) + + wantResourceIDs := append([]string{}, tt.wantResourceIDs...) + sort.Strings(wantResourceIDs) + + if diff := gcmp.Diff(wantResourceIDs, gotResourceIDs); diff != "" { + t.Errorf("FetchObservedResources() resource IDs mismatch (-want +got):\n%s", diff) + } + + // Verify all resources have the composition-resource-name annotation + for _, res := range observed { + if _, hasAnno := res.GetAnnotations()["crossplane.io/composition-resource-name"]; !hasAnno { + t.Errorf("FetchObservedResources() returned resource %s without composition-resource-name annotation", res.GetName()) + } + } + } + }) + } +} diff --git a/cmd/diff/testdata/comp/crds/nopclaim-crd.yaml b/cmd/diff/testdata/comp/crds/nopclaim-crd.yaml index 2b0dec7..72a5ca9 100644 --- a/cmd/diff/testdata/comp/crds/nopclaim-crd.yaml +++ b/cmd/diff/testdata/comp/crds/nopclaim-crd.yaml @@ -11,81 +11,55 @@ spec: singular: nopclaim scope: Namespaced versions: - - name: v1alpha1 - served: true - storage: true - schema: - openAPIV3Schema: - type: object - properties: - spec: - type: object - properties: - coolField: - type: string - compositionRef: - type: object - properties: - name: - type: string - compositeDeletePolicy: - type: string - enum: - - Background - - Foreground - default: "Background" - compositionUpdatePolicy: - type: string - enum: - - Automatic - - Manual - default: "Automatic" - compositionSelector: - type: object - properties: - matchLabels: - type: object - additionalProperties: - type: string - publishConnectionDetailsTo: - type: object - properties: - name: - type: string - configRef: - type: object - properties: - name: - type: string - metadata: - type: object - properties: - labels: - type: object - additionalProperties: - type: string - annotations: - type: object - additionalProperties: - type: string - writeConnectionSecretsToNamespace: - type: string - status: - type: object - properties: - conditions: - type: array - items: + - name: v1alpha1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + apiVersion: + type: string + kind: + type: string + metadata: + type: object + spec: + type: object + properties: + coolField: + type: string + compositionRef: + type: object + properties: + name: + type: string + compositionSelector: + type: object + properties: + matchLabels: type: object - properties: - lastTransitionTime: - type: string - format: date-time - message: - type: string - reason: - type: string - status: - type: string - type: - type: string \ No newline at end of file + additionalProperties: + type: string + compositionUpdatePolicy: + type: string + enum: + - Automatic + - Manual + compositeDeletePolicy: + type: string + enum: + - Background + - Foreground + resourceRef: + type: object + properties: + apiVersion: + type: string + kind: + type: string + name: + type: string + status: + type: object + x-kubernetes-preserve-unknown-fields: true diff --git a/cmd/diff/testdata/comp/crds/xnopclaim-crd.yaml b/cmd/diff/testdata/comp/crds/xnop-crd.yaml similarity index 89% rename from cmd/diff/testdata/comp/crds/xnopclaim-crd.yaml rename to cmd/diff/testdata/comp/crds/xnop-crd.yaml index ea6e8ba..bf7ef0b 100644 --- a/cmd/diff/testdata/comp/crds/xnopclaim-crd.yaml +++ b/cmd/diff/testdata/comp/crds/xnop-crd.yaml @@ -1,15 +1,15 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: - name: xnopclaims.diff.example.org + name: xnops.diff.example.org spec: group: diff.example.org names: - kind: XNopClaim - listKind: XNopClaimList - plural: xnopclaims - singular: xnopclaim - scope: Namespaced + kind: XNop + listKind: XNopList + plural: xnops + singular: xnop + scope: Cluster versions: - name: v1alpha1 served: true diff --git a/cmd/diff/testdata/comp/resources/claim-composition.yaml b/cmd/diff/testdata/comp/resources/claim-composition.yaml index 1ebce9c..58219d5 100644 --- a/cmd/diff/testdata/comp/resources/claim-composition.yaml +++ b/cmd/diff/testdata/comp/resources/claim-composition.yaml @@ -5,7 +5,7 @@ metadata: spec: compositeTypeRef: apiVersion: diff.example.org/v1alpha1 - kind: XNopClaim + kind: XNop mode: Pipeline pipeline: - step: generate-resources diff --git a/cmd/diff/testdata/comp/resources/claim-xrd.yaml b/cmd/diff/testdata/comp/resources/claim-xrd.yaml index f6d7bfc..c09eda0 100644 --- a/cmd/diff/testdata/comp/resources/claim-xrd.yaml +++ b/cmd/diff/testdata/comp/resources/claim-xrd.yaml @@ -1,12 +1,12 @@ apiVersion: apiextensions.crossplane.io/v1 kind: CompositeResourceDefinition metadata: - name: xnopclaims.diff.example.org + name: xnops.diff.example.org spec: group: diff.example.org names: - kind: XNopClaim - plural: xnopclaims + kind: XNop + plural: xnops claimNames: kind: NopClaim plural: nopclaims diff --git a/cmd/diff/testdata/comp/resources/existing-claim-1-xr.yaml b/cmd/diff/testdata/comp/resources/existing-claim-1-xr.yaml new file mode 100644 index 0000000..272bdba --- /dev/null +++ b/cmd/diff/testdata/comp/resources/existing-claim-1-xr.yaml @@ -0,0 +1,21 @@ +apiVersion: diff.example.org/v1alpha1 +kind: XNop +metadata: + name: test-claim-1-xr + labels: + crossplane.io/claim-name: test-claim-1 + crossplane.io/claim-namespace: test-namespace +spec: + claimRef: + apiVersion: diff.example.org/v1alpha1 + kind: NopClaim + name: test-claim-1 + namespace: test-namespace + coolField: claim-value-1 + compositeDeletePolicy: Background + compositionRef: + name: nopclaims.diff.example.org + resourceRefs: + - apiVersion: nop.example.org/v1alpha1 + kind: XDownstreamResource + name: test-claim-1-xr diff --git a/cmd/diff/testdata/comp/resources/existing-claim-1.yaml b/cmd/diff/testdata/comp/resources/existing-claim-1.yaml index c1fd078..3d06c37 100644 --- a/cmd/diff/testdata/comp/resources/existing-claim-1.yaml +++ b/cmd/diff/testdata/comp/resources/existing-claim-1.yaml @@ -8,3 +8,7 @@ spec: compositeDeletePolicy: Background compositionRef: name: nopclaims.diff.example.org + resourceRef: + apiVersion: diff.example.org/v1alpha1 + kind: XNop + name: test-claim-1-xr diff --git a/cmd/diff/testdata/comp/resources/existing-claim-2-xr.yaml b/cmd/diff/testdata/comp/resources/existing-claim-2-xr.yaml new file mode 100644 index 0000000..91abea9 --- /dev/null +++ b/cmd/diff/testdata/comp/resources/existing-claim-2-xr.yaml @@ -0,0 +1,21 @@ +apiVersion: diff.example.org/v1alpha1 +kind: XNop +metadata: + name: test-claim-2-xr + labels: + crossplane.io/claim-name: test-claim-2 + crossplane.io/claim-namespace: test-namespace +spec: + claimRef: + apiVersion: diff.example.org/v1alpha1 + kind: NopClaim + name: test-claim-2 + namespace: test-namespace + coolField: claim-value-2 + compositeDeletePolicy: Background + compositionRef: + name: nopclaims.diff.example.org + resourceRefs: + - apiVersion: nop.example.org/v1alpha1 + kind: XDownstreamResource + name: test-claim-2-xr diff --git a/cmd/diff/testdata/comp/resources/existing-claim-2.yaml b/cmd/diff/testdata/comp/resources/existing-claim-2.yaml index 16eab1d..1dedf02 100644 --- a/cmd/diff/testdata/comp/resources/existing-claim-2.yaml +++ b/cmd/diff/testdata/comp/resources/existing-claim-2.yaml @@ -8,3 +8,7 @@ spec: compositeDeletePolicy: Background compositionRef: name: nopclaims.diff.example.org + resourceRef: + apiVersion: diff.example.org/v1alpha1 + kind: XNop + name: test-claim-2-xr diff --git a/cmd/diff/testdata/comp/resources/existing-xr-from-claim-1.yaml b/cmd/diff/testdata/comp/resources/existing-xr-from-claim-1.yaml index 75c388a..190e216 100644 --- a/cmd/diff/testdata/comp/resources/existing-xr-from-claim-1.yaml +++ b/cmd/diff/testdata/comp/resources/existing-xr-from-claim-1.yaml @@ -1,5 +1,5 @@ apiVersion: diff.example.org/v1alpha1 -kind: XNopClaim +kind: XNop metadata: name: test-claim-1-xr namespace: test-namespace diff --git a/cmd/diff/testdata/comp/resources/existing-xr-from-claim-2.yaml b/cmd/diff/testdata/comp/resources/existing-xr-from-claim-2.yaml index d2dd523..34cc171 100644 --- a/cmd/diff/testdata/comp/resources/existing-xr-from-claim-2.yaml +++ b/cmd/diff/testdata/comp/resources/existing-xr-from-claim-2.yaml @@ -1,5 +1,5 @@ apiVersion: diff.example.org/v1alpha1 -kind: XNopClaim +kind: XNop metadata: name: test-claim-2-xr namespace: test-namespace diff --git a/cmd/diff/testdata/comp/resources/test-namespace.yaml b/cmd/diff/testdata/comp/resources/test-namespace.yaml new file mode 100644 index 0000000..f8db044 --- /dev/null +++ b/cmd/diff/testdata/comp/resources/test-namespace.yaml @@ -0,0 +1,4 @@ +apiVersion: v1 +kind: Namespace +metadata: + name: test-namespace diff --git a/cmd/diff/testdata/comp/updated-claim-composition.yaml b/cmd/diff/testdata/comp/updated-claim-composition.yaml index 0754324..f619a36 100644 --- a/cmd/diff/testdata/comp/updated-claim-composition.yaml +++ b/cmd/diff/testdata/comp/updated-claim-composition.yaml @@ -5,7 +5,7 @@ metadata: spec: compositeTypeRef: apiVersion: diff.example.org/v1alpha1 - kind: XNopClaim + kind: XNop mode: Pipeline pipeline: - step: generate-resources diff --git a/cmd/diff/testutils/mocks.go b/cmd/diff/testutils/mocks.go index f7f53da..9e03c3b 100644 --- a/cmd/diff/testutils/mocks.go +++ b/cmd/diff/testutils/mocks.go @@ -544,10 +544,10 @@ func (m *MockTypeConverter) GetResourceNameForGVK(ctx context.Context, gvk schem // MockCompositionClient implements the crossplane.CompositionClient interface. type MockCompositionClient struct { - InitializeFn func(ctx context.Context) error - FindMatchingCompositionFn func(ctx context.Context, res *un.Unstructured) (*xpextv1.Composition, error) - ListCompositionsFn func(ctx context.Context) ([]*xpextv1.Composition, error) - GetCompositionFn func(ctx context.Context, name string) (*xpextv1.Composition, error) + InitializeFn func(ctx context.Context) error + FindMatchingCompositionFn func(ctx context.Context, res *un.Unstructured) (*xpextv1.Composition, error) + ListCompositionsFn func(ctx context.Context) ([]*xpextv1.Composition, error) + GetCompositionFn func(ctx context.Context, name string) (*xpextv1.Composition, error) FindCompositesUsingCompositionFn func(ctx context.Context, compositionName string, namespace string) ([]*un.Unstructured, error) } @@ -748,8 +748,8 @@ func (m *MockResourceTreeClient) GetResourceTree(ctx context.Context, root *un.U type MockDiffCalculator struct { CalculateDiffFn func(context.Context, *un.Unstructured, *un.Unstructured) (*dt.ResourceDiff, error) CalculateDiffsFn func(context.Context, *cmp.Unstructured, render.Outputs) (map[string]*dt.ResourceDiff, error) + CalculateNonRemovalDiffsFn func(context.Context, *cmp.Unstructured, render.Outputs) (map[string]*dt.ResourceDiff, map[string]bool, error) CalculateRemovedResourceDiffsFn func(context.Context, *un.Unstructured, map[string]bool) (map[string]*dt.ResourceDiff, error) - FetchObservedResourcesFn func(context.Context, *cmp.Unstructured) ([]cpd.Unstructured, error) } // CalculateDiff implements DiffCalculator. @@ -770,19 +770,19 @@ func (m *MockDiffCalculator) CalculateDiffs(ctx context.Context, xr *cmp.Unstruc return nil, nil } -// CalculateRemovedResourceDiffs implements DiffCalculator. -func (m *MockDiffCalculator) CalculateRemovedResourceDiffs(ctx context.Context, xr *un.Unstructured, renderedResources map[string]bool) (map[string]*dt.ResourceDiff, error) { - if m.CalculateRemovedResourceDiffsFn != nil { - return m.CalculateRemovedResourceDiffsFn(ctx, xr, renderedResources) +// CalculateNonRemovalDiffs implements DiffCalculator. +func (m *MockDiffCalculator) CalculateNonRemovalDiffs(ctx context.Context, xr *cmp.Unstructured, desired render.Outputs) (map[string]*dt.ResourceDiff, map[string]bool, error) { + if m.CalculateNonRemovalDiffsFn != nil { + return m.CalculateNonRemovalDiffsFn(ctx, xr, desired) } - return nil, nil + return nil, nil, nil } -// FetchObservedResources implements DiffCalculator. -func (m *MockDiffCalculator) FetchObservedResources(ctx context.Context, xr *cmp.Unstructured) ([]cpd.Unstructured, error) { - if m.FetchObservedResourcesFn != nil { - return m.FetchObservedResourcesFn(ctx, xr) +// CalculateRemovedResourceDiffs implements DiffCalculator. +func (m *MockDiffCalculator) CalculateRemovedResourceDiffs(ctx context.Context, xr *un.Unstructured, renderedResources map[string]bool) (map[string]*dt.ResourceDiff, error) { + if m.CalculateRemovedResourceDiffsFn != nil { + return m.CalculateRemovedResourceDiffsFn(ctx, xr, renderedResources) } return nil, nil diff --git a/test/e2e/claim_test.go b/test/e2e/claim_test.go index dc215f1..c5e56ad 100644 --- a/test/e2e/claim_test.go +++ b/test/e2e/claim_test.go @@ -148,3 +148,122 @@ func TestDiffExistingClaim(t *testing.T) { Feature(), ) } + +// TestDiffNewClaimWithNestedXRs tests the crossplane diff command against new claims that create nested XRs. +// This covers the case where a claim creates an XR that itself creates child XRs (nested composition). +func TestDiffNewClaimWithNestedXRs(t *testing.T) { + imageTag := strings.Split(environment.GetCrossplaneImage(), ":")[1] + manifests := filepath.Join("test/e2e/manifests/beta/diff", imageTag, "v1-claim-nested") + setupPath := filepath.Join(manifests, "setup") + + environment.Test(t, + features.New("DiffNewClaimWithNestedXRs"). + WithLabel(e2e.LabelArea, LabelAreaDiff). + WithLabel(e2e.LabelSize, e2e.LabelSizeSmall). + WithLabel(config.LabelTestSuite, config.TestSuiteDefault). + WithLabel(LabelCrossplaneVersion, CrossplaneVersionRelease120). + WithLabel(LabelCrossplaneVersion, CrossplaneVersionMain). + WithSetup("CreatePrerequisites", funcs.AllOf( + funcs.ApplyResources(e2e.FieldManager, setupPath, "*.yaml"), + funcs.ResourcesCreatedWithin(30*time.Second, setupPath, "*.yaml"), + )). + WithSetup("PrerequisitesAreReady", funcs.AllOf( + funcs.ResourcesHaveConditionWithin(1*time.Minute, setupPath, "parent-definition.yaml", apiextensionsv1.WatchingComposite()), + funcs.ResourcesHaveConditionWithin(1*time.Minute, setupPath, "child-definition.yaml", apiextensionsv1.WatchingComposite()), + )). + Assess("CanDiffNewClaimWithNestedXRs", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { + t.Helper() + + output, log, err := RunXRDiff(t, c, "./crossplane-diff", filepath.Join(manifests, "new-claim.yaml")) + if err != nil { + t.Fatalf("Error running diff command: %v\nLog output:\n%s", err, log) + } + + expectPath := filepath.Join(manifests, "expect") + assertDiffMatchesFile(t, output, filepath.Join(expectPath, "new-claim.ansi"), log) + + return ctx + }). + WithTeardown("DeletePrerequisites", funcs.AllOf( + func(ctx context.Context, t *testing.T, e *envconf.Config) context.Context { + t.Helper() + // default to `main` variant + nopList := clusterNopList + + // we should only ever be running with one version label + if slices.Contains(e.Labels()[LabelCrossplaneVersion], CrossplaneVersionRelease120) { + nopList = v1NopList + } + + funcs.ResourcesDeletedAfterListedAreGone(3*time.Minute, setupPath, "*.yaml", nopList)(ctx, t, e) + + return ctx + }, + )). + Feature(), + ) +} + +// TestDiffExistingClaimWithNestedXRs tests the crossplane diff command against existing claims that create nested XRs. +// This test verifies that nested XR identity is preserved when diffing modified claims. +func TestDiffExistingClaimWithNestedXRs(t *testing.T) { + imageTag := strings.Split(environment.GetCrossplaneImage(), ":")[1] + manifests := filepath.Join("test/e2e/manifests/beta/diff", imageTag, "v1-claim-nested") + setupPath := filepath.Join(manifests, "setup") + + environment.Test(t, + features.New("DiffExistingClaimWithNestedXRs"). + WithLabel(e2e.LabelArea, LabelAreaDiff). + WithLabel(e2e.LabelSize, e2e.LabelSizeSmall). + WithLabel(config.LabelTestSuite, config.TestSuiteDefault). + WithLabel(LabelCrossplaneVersion, CrossplaneVersionRelease120). + WithLabel(LabelCrossplaneVersion, CrossplaneVersionMain). + WithSetup("CreatePrerequisites", funcs.AllOf( + funcs.ApplyResources(e2e.FieldManager, setupPath, "*.yaml"), + funcs.ResourcesCreatedWithin(30*time.Second, setupPath, "*.yaml"), + )). + WithSetup("PrerequisitesAreReady", funcs.AllOf( + funcs.ResourcesHaveConditionWithin(1*time.Minute, setupPath, "parent-definition.yaml", apiextensionsv1.WatchingComposite()), + funcs.ResourcesHaveConditionWithin(1*time.Minute, setupPath, "child-definition.yaml", apiextensionsv1.WatchingComposite()), + )). + WithSetup("CreateClaim", funcs.AllOf( + funcs.ApplyResources(e2e.FieldManager, manifests, "existing-claim.yaml"), + funcs.ResourcesCreatedWithin(1*time.Minute, manifests, "existing-claim.yaml"), + funcs.ResourcesHaveConditionWithin(2*time.Minute, manifests, "existing-claim.yaml", xpv1.Available()), + )). + Assess("CanDiffExistingClaimWithNestedXRs", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { + t.Helper() + + output, log, err := RunXRDiff(t, c, "./crossplane-diff", filepath.Join(manifests, "modified-claim.yaml")) + if err != nil { + t.Fatalf("Error running diff command: %v\nLog output:\n%s", err, log) + } + + expectPath := filepath.Join(manifests, "expect") + assertDiffMatchesFile(t, output, filepath.Join(expectPath, "existing-claim.ansi"), log) + + return ctx + }). + WithTeardown("DeleteClaim", funcs.AllOf( + funcs.DeleteResources(manifests, "existing-claim.yaml"), + funcs.ResourcesDeletedWithin(2*time.Minute, manifests, "existing-claim.yaml"), + )). + WithTeardown("DeletePrerequisites", funcs.AllOf( + func(ctx context.Context, t *testing.T, e *envconf.Config) context.Context { + t.Helper() + // default to `main` variant + nopList := clusterNopList + + // we should only ever be running with one version label + if slices.Contains(e.Labels()[LabelCrossplaneVersion], CrossplaneVersionRelease120) { + nopList = v1NopList + } + + funcs.ResourcesDeletedAfterListedAreGone(3*time.Minute, setupPath, "*.yaml", nopList)(ctx, t, e) + + return ctx + }, + )). + Feature(), + ) +} diff --git a/test/e2e/helpers_test.go b/test/e2e/helpers_test.go index 2e55474..0910a20 100644 --- a/test/e2e/helpers_test.go +++ b/test/e2e/helpers_test.go @@ -22,6 +22,7 @@ import ( "fmt" "os" "os/exec" + "path/filepath" "regexp" "strings" "testing" @@ -66,16 +67,18 @@ var clusterNopList = composed.NewList(composed.FromReferenceToList(corev1.Object // Regular expressions to match the dynamic parts. var ( - resourceNameRegex = regexp.MustCompile(`(existing-resource)-[a-z0-9]{5,}(?:-nop-resource)?`) - compResourceNameRegex = regexp.MustCompile(`(test-comp-resource)-[a-z0-9]{5,}`) - fanoutResourceNameRegex = regexp.MustCompile(`(test-fanout-resource-\d{2})-[a-z0-9]{5,}`) - claimNameRegex = regexp.MustCompile(`(test-claim)-[a-z0-9]{5,}(?:-[a-z0-9]{5,})?`) - compClaimNameRegex = regexp.MustCompile(`(test-comp-claim)-[a-z0-9]{5,}`) - claimCompositionRevisionRegex = regexp.MustCompile(`(xnopclaims\.claim\.diff\.example\.org)-[a-z0-9]{7,}`) - compositionRevisionRegex = regexp.MustCompile(`(xnopresources\.(cluster\.|legacy\.)?diff\.example\.org)-[a-z0-9]{7,}`) - nestedCompositionRevisionRegex = regexp.MustCompile(`(child-nop-composition|parent-nop-composition)-[a-z0-9]{7,}`) + resourceNameRegex = regexp.MustCompile(`(existing-resource)-[a-z0-9]{5,}(?:-nop-resource)?`) + compResourceNameRegex = regexp.MustCompile(`(test-comp-resource)-[a-z0-9]{5,}`) + fanoutResourceNameRegex = regexp.MustCompile(`(test-fanout-resource-\d{2})-[a-z0-9]{5,}`) + claimNameRegex = regexp.MustCompile(`(test-claim)-[a-z0-9]{5,}(?:-[a-z0-9]{5,})?`) + compClaimNameRegex = regexp.MustCompile(`(test-comp-claim)-[a-z0-9]{5,}`) + nestedGenerateNameRegex = regexp.MustCompile(`(test-parent-generatename-child)-[a-z0-9]{12,16}`) + nestedClaimGenerateNameRegex = regexp.MustCompile(`(existing-parent-claim)-[a-z0-9]{5,}(?:-[a-z0-9]{12,16})?`) + claimCompositionRevisionRegex = regexp.MustCompile(`(xnopclaims\.claim\.diff\.example\.org)-[a-z0-9]{7,}`) + compositionRevisionRegex = regexp.MustCompile(`(xnopresources\.(cluster\.|legacy\.)?diff\.example\.org)-[a-z0-9]{7,}`) + nestedCompositionRevisionRegex = regexp.MustCompile(`(child-nop-composition|parent-nop-composition)-[a-z0-9]{7,}`) compClaimCompositionRevisionRegex = regexp.MustCompile(`(xnopclaimdiffresources\.claimdiff\.example\.org)-[a-z0-9]{7,}`) - ansiEscapeRegex = regexp.MustCompile(`\x1b\[[0-9;]*m`) + ansiEscapeRegex = regexp.MustCompile(`\x1b\[[0-9;]*m`) ) // runCrossplaneDiff runs the crossplane diff command with the specified subcommand on the provided resources. @@ -128,6 +131,8 @@ func normalizeLine(line string) string { line = fanoutResourceNameRegex.ReplaceAllString(line, "${1}-XXXXX") line = claimNameRegex.ReplaceAllString(line, "${1}-XXXXX") line = compClaimNameRegex.ReplaceAllString(line, "${1}-XXXXX") + line = nestedGenerateNameRegex.ReplaceAllString(line, "${1}-XXXXX") + line = nestedClaimGenerateNameRegex.ReplaceAllString(line, "${1}-XXXXX") // Replace composition revision refs with random hash line = compositionRevisionRegex.ReplaceAllString(line, "${1}-XXXXXXX") @@ -162,6 +167,32 @@ func parseStringContent(content string) ([]string, []string) { func assertDiffMatchesFile(t *testing.T, actual, expectedSource, log string) { t.Helper() + // If E2E_DUMP_EXPECTED is set, write the actual output to the expected file + if os.Getenv("E2E_DUMP_EXPECTED") != "" { + // Ensure the directory exists + if err := os.MkdirAll(filepath.Dir(expectedSource), 0o755); err != nil { + t.Fatalf("Failed to create directory for expected file: %v", err) + } + + // Normalize the output before writing to reduce churn from random generated names + _, normalizedLines := parseStringContent(actual) + + normalizedOutput := strings.Join(normalizedLines, "\n") + if strings.HasSuffix(actual, "\n") { + // Add trailing newline if original had one + normalizedOutput += "\n" + } + + // Write the normalized output to the expected file + if err := os.WriteFile(expectedSource, []byte(normalizedOutput), 0o644); err != nil { + t.Fatalf("Failed to write expected file: %v", err) + } + + t.Logf("Wrote normalized expected output to %s", expectedSource) + + return + } + expected, err := os.ReadFile(expectedSource) if err != nil { t.Fatalf("Failed to read expected file: %v", err) diff --git a/test/e2e/manifests/beta/diff/main/comp-claim/expect/existing-claim.ansi b/test/e2e/manifests/beta/diff/main/comp-claim/expect/existing-claim.ansi index cb16a48..9b4fa5c 100644 --- a/test/e2e/manifests/beta/diff/main/comp-claim/expect/existing-claim.ansi +++ b/test/e2e/manifests/beta/diff/main/comp-claim/expect/existing-claim.ansi @@ -30,10 +30,10 @@ conditionStatus: "True" time: 0s fields: -- configData: {{ .observed.composite.resource.spec.coolField }} -- resourceTier: basic -+ configData: updated-{{ .observed.composite.resource.spec.coolField }} -+ resourceTier: premium +- configData: {{ .observed.composite.resource.spec.coolField }} +- resourceTier: basic ++ configData: updated-{{ .observed.composite.resource.spec.coolField }} ++ resourceTier: premium kind: GoTemplate source: Inline step: generate-resources @@ -66,8 +66,7 @@ Summary: 2 resources with changes labels: crossplane.io/claim-name: test-comp-claim crossplane.io/claim-namespace: default -- crossplane.io/composite: test-comp-claim-XXXXX -+ crossplane.io/composite: test-comp-claim + crossplane.io/composite: test-comp-claim-XXXXX name: test-comp-claim-XXXXX spec: deletionPolicy: Delete @@ -77,10 +76,10 @@ Summary: 2 resources with changes conditionType: Ready time: 0s fields: -- configData: claim-value-1 -- resourceTier: basic -+ configData: updated-claim-value-1 -+ resourceTier: premium +- configData: claim-value-1 +- resourceTier: basic ++ configData: updated-claim-value-1 ++ resourceTier: premium managementPolicies: - '*' providerConfigRef: @@ -101,7 +100,7 @@ Summary: 2 resources with changes name: xnopclaimdiffresources.claimdiff.example.org compositionRevisionRef: name: xnopclaimdiffresources.claimdiff.example.org-XXXXXXX -+ compositionUpdatePolicy: Automatic ++ compositionUpdatePolicy: Automatic coolField: claim-value-1 crossplane: compositionRef: diff --git a/test/e2e/manifests/beta/diff/main/comp-fanout/expect/existing-xrs.ansi b/test/e2e/manifests/beta/diff/main/comp-fanout/expect/existing-xrs.ansi index 440516b..58a8364 100644 --- a/test/e2e/manifests/beta/diff/main/comp-fanout/expect/existing-xrs.ansi +++ b/test/e2e/manifests/beta/diff/main/comp-fanout/expect/existing-xrs.ansi @@ -30,18 +30,18 @@ patches: - fromFieldPath: spec.coolField toFieldPath: metadata.annotations[config-data] -+ transforms: -+ - string: -+ fmt: updated-%s -+ type: Format -+ type: string ++ transforms: ++ - string: ++ fmt: updated-%s ++ type: Format ++ type: string type: FromCompositeFieldPath - fromFieldPath: spec.coolField toFieldPath: metadata.annotations[resource-tier] transforms: - string: -- fmt: basic-%s -+ fmt: premium-%s +- fmt: basic-%s ++ fmt: premium-%s type: Format type: string type: FromCompositeFieldPath @@ -56,35 +56,35 @@ Summary: 1 modified === Affected Composite Resources === - ⚠ XCompDiffFanoutResource/test-fanout-resource-01 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-02 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-03 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-04 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-05 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-06 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-07 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-08 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-09 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-10 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-11 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-12 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-13 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-14 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-16 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-17 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-18 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-19 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-20 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-21 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-22 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-23 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-24 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-25 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-26 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-27 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-28 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-29 (cluster-scoped) - ⚠ XCompDiffFanoutResource/test-fanout-resource-30 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-01 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-02 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-03 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-04 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-05 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-06 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-07 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-08 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-09 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-10 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-11 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-12 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-13 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-14 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-16 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-17 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-18 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-19 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-20 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-21 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-22 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-23 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-24 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-25 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-26 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-27 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-28 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-29 (cluster-scoped) + ⚠ XCompDiffFanoutResource/test-fanout-resource-30 (cluster-scoped) Summary: 29 resources with changes @@ -95,12 +95,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-01 -+ config-data: updated-value-01 +- config-data: value-01 ++ config-data: updated-value-01 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-01-XXXXX -- resource-tier: basic-value-01 -+ resource-tier: premium-value-01 +- resource-tier: basic-value-01 ++ resource-tier: premium-value-01 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-01- @@ -125,12 +125,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-02 -+ config-data: updated-value-02 +- config-data: value-02 ++ config-data: updated-value-02 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-02-XXXXX -- resource-tier: basic-value-02 -+ resource-tier: premium-value-02 +- resource-tier: basic-value-02 ++ resource-tier: premium-value-02 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-02- @@ -155,12 +155,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-03 -+ config-data: updated-value-03 +- config-data: value-03 ++ config-data: updated-value-03 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-03-XXXXX -- resource-tier: basic-value-03 -+ resource-tier: premium-value-03 +- resource-tier: basic-value-03 ++ resource-tier: premium-value-03 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-03- @@ -185,12 +185,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-04 -+ config-data: updated-value-04 +- config-data: value-04 ++ config-data: updated-value-04 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-04-XXXXX -- resource-tier: basic-value-04 -+ resource-tier: premium-value-04 +- resource-tier: basic-value-04 ++ resource-tier: premium-value-04 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-04- @@ -215,12 +215,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-05 -+ config-data: updated-value-05 +- config-data: value-05 ++ config-data: updated-value-05 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-05-XXXXX -- resource-tier: basic-value-05 -+ resource-tier: premium-value-05 +- resource-tier: basic-value-05 ++ resource-tier: premium-value-05 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-05- @@ -245,12 +245,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-06 -+ config-data: updated-value-06 +- config-data: value-06 ++ config-data: updated-value-06 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-06-XXXXX -- resource-tier: basic-value-06 -+ resource-tier: premium-value-06 +- resource-tier: basic-value-06 ++ resource-tier: premium-value-06 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-06- @@ -275,12 +275,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-07 -+ config-data: updated-value-07 +- config-data: value-07 ++ config-data: updated-value-07 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-07-XXXXX -- resource-tier: basic-value-07 -+ resource-tier: premium-value-07 +- resource-tier: basic-value-07 ++ resource-tier: premium-value-07 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-07- @@ -305,12 +305,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-08 -+ config-data: updated-value-08 +- config-data: value-08 ++ config-data: updated-value-08 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-08-XXXXX -- resource-tier: basic-value-08 -+ resource-tier: premium-value-08 +- resource-tier: basic-value-08 ++ resource-tier: premium-value-08 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-08- @@ -335,12 +335,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-09 -+ config-data: updated-value-09 +- config-data: value-09 ++ config-data: updated-value-09 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-09-XXXXX -- resource-tier: basic-value-09 -+ resource-tier: premium-value-09 +- resource-tier: basic-value-09 ++ resource-tier: premium-value-09 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-09- @@ -365,12 +365,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-10 -+ config-data: updated-value-10 +- config-data: value-10 ++ config-data: updated-value-10 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-10-XXXXX -- resource-tier: basic-value-10 -+ resource-tier: premium-value-10 +- resource-tier: basic-value-10 ++ resource-tier: premium-value-10 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-10- @@ -395,12 +395,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-11 -+ config-data: updated-value-11 +- config-data: value-11 ++ config-data: updated-value-11 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-11-XXXXX -- resource-tier: basic-value-11 -+ resource-tier: premium-value-11 +- resource-tier: basic-value-11 ++ resource-tier: premium-value-11 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-11- @@ -425,12 +425,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-12 -+ config-data: updated-value-12 +- config-data: value-12 ++ config-data: updated-value-12 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-12-XXXXX -- resource-tier: basic-value-12 -+ resource-tier: premium-value-12 +- resource-tier: basic-value-12 ++ resource-tier: premium-value-12 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-12- @@ -455,12 +455,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-13 -+ config-data: updated-value-13 +- config-data: value-13 ++ config-data: updated-value-13 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-13-XXXXX -- resource-tier: basic-value-13 -+ resource-tier: premium-value-13 +- resource-tier: basic-value-13 ++ resource-tier: premium-value-13 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-13- @@ -485,12 +485,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-14 -+ config-data: updated-value-14 +- config-data: value-14 ++ config-data: updated-value-14 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-14-XXXXX -- resource-tier: basic-value-14 -+ resource-tier: premium-value-14 +- resource-tier: basic-value-14 ++ resource-tier: premium-value-14 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-14- @@ -515,12 +515,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-16 -+ config-data: updated-value-16 +- config-data: value-16 ++ config-data: updated-value-16 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-16-XXXXX -- resource-tier: basic-value-16 -+ resource-tier: premium-value-16 +- resource-tier: basic-value-16 ++ resource-tier: premium-value-16 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-16- @@ -545,12 +545,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-17 -+ config-data: updated-value-17 +- config-data: value-17 ++ config-data: updated-value-17 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-17-XXXXX -- resource-tier: basic-value-17 -+ resource-tier: premium-value-17 +- resource-tier: basic-value-17 ++ resource-tier: premium-value-17 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-17- @@ -575,12 +575,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-18 -+ config-data: updated-value-18 +- config-data: value-18 ++ config-data: updated-value-18 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-18-XXXXX -- resource-tier: basic-value-18 -+ resource-tier: premium-value-18 +- resource-tier: basic-value-18 ++ resource-tier: premium-value-18 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-18- @@ -605,12 +605,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-19 -+ config-data: updated-value-19 +- config-data: value-19 ++ config-data: updated-value-19 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-19-XXXXX -- resource-tier: basic-value-19 -+ resource-tier: premium-value-19 +- resource-tier: basic-value-19 ++ resource-tier: premium-value-19 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-19- @@ -635,12 +635,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-20 -+ config-data: updated-value-20 +- config-data: value-20 ++ config-data: updated-value-20 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-20-XXXXX -- resource-tier: basic-value-20 -+ resource-tier: premium-value-20 +- resource-tier: basic-value-20 ++ resource-tier: premium-value-20 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-20- @@ -665,12 +665,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-21 -+ config-data: updated-value-21 +- config-data: value-21 ++ config-data: updated-value-21 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-21-XXXXX -- resource-tier: basic-value-21 -+ resource-tier: premium-value-21 +- resource-tier: basic-value-21 ++ resource-tier: premium-value-21 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-21- @@ -695,12 +695,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-22 -+ config-data: updated-value-22 +- config-data: value-22 ++ config-data: updated-value-22 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-22-XXXXX -- resource-tier: basic-value-22 -+ resource-tier: premium-value-22 +- resource-tier: basic-value-22 ++ resource-tier: premium-value-22 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-22- @@ -725,12 +725,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-23 -+ config-data: updated-value-23 +- config-data: value-23 ++ config-data: updated-value-23 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-23-XXXXX -- resource-tier: basic-value-23 -+ resource-tier: premium-value-23 +- resource-tier: basic-value-23 ++ resource-tier: premium-value-23 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-23- @@ -755,12 +755,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-24 -+ config-data: updated-value-24 +- config-data: value-24 ++ config-data: updated-value-24 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-24-XXXXX -- resource-tier: basic-value-24 -+ resource-tier: premium-value-24 +- resource-tier: basic-value-24 ++ resource-tier: premium-value-24 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-24- @@ -785,12 +785,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-25 -+ config-data: updated-value-25 +- config-data: value-25 ++ config-data: updated-value-25 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-25-XXXXX -- resource-tier: basic-value-25 -+ resource-tier: premium-value-25 +- resource-tier: basic-value-25 ++ resource-tier: premium-value-25 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-25- @@ -815,12 +815,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-26 -+ config-data: updated-value-26 +- config-data: value-26 ++ config-data: updated-value-26 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-26-XXXXX -- resource-tier: basic-value-26 -+ resource-tier: premium-value-26 +- resource-tier: basic-value-26 ++ resource-tier: premium-value-26 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-26- @@ -845,12 +845,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-27 -+ config-data: updated-value-27 +- config-data: value-27 ++ config-data: updated-value-27 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-27-XXXXX -- resource-tier: basic-value-27 -+ resource-tier: premium-value-27 +- resource-tier: basic-value-27 ++ resource-tier: premium-value-27 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-27- @@ -875,12 +875,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-28 -+ config-data: updated-value-28 +- config-data: value-28 ++ config-data: updated-value-28 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-28-XXXXX -- resource-tier: basic-value-28 -+ resource-tier: premium-value-28 +- resource-tier: basic-value-28 ++ resource-tier: premium-value-28 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-28- @@ -905,12 +905,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-29 -+ config-data: updated-value-29 +- config-data: value-29 ++ config-data: updated-value-29 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-29-XXXXX -- resource-tier: basic-value-29 -+ resource-tier: premium-value-29 +- resource-tier: basic-value-29 ++ resource-tier: premium-value-29 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-29- @@ -935,12 +935,12 @@ Summary: 29 resources with changes kind: ClusterNopResource metadata: annotations: -- config-data: value-30 -+ config-data: updated-value-30 +- config-data: value-30 ++ config-data: updated-value-30 crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-fanout-resource-30-XXXXX -- resource-tier: basic-value-30 -+ resource-tier: premium-value-30 +- resource-tier: basic-value-30 ++ resource-tier: premium-value-30 finalizers: - finalizer.managedresource.crossplane.io generateName: test-fanout-resource-30- diff --git a/test/e2e/manifests/beta/diff/main/comp-getcomposed/expect/existing-xr.ansi b/test/e2e/manifests/beta/diff/main/comp-getcomposed/expect/existing-xr.ansi index 35a7429..1e532a6 100644 --- a/test/e2e/manifests/beta/diff/main/comp-getcomposed/expect/existing-xr.ansi +++ b/test/e2e/manifests/beta/diff/main/comp-getcomposed/expect/existing-xr.ansi @@ -47,22 +47,22 @@ type: FromCompositeFieldPath step: generate-resources - functionRef: -+ name: function-go-templating -+ input: -+ apiVersion: gotemplating.fn.crossplane.io/v1beta1 -+ inline: -+ template: | -+ {{ $bucket := getComposedResource . "test-bucket" }} -+ --- -+ apiVersion: getcomposed.example.org/v1alpha1 -+ kind: XGetComposedResource -+ status: -+ bucketRef: -+ name: {{ $bucket.metadata.name }} -+ kind: GoTemplate -+ source: Inline -+ step: use-getcomposed -+ - functionRef: ++ name: function-go-templating ++ input: ++ apiVersion: gotemplating.fn.crossplane.io/v1beta1 ++ inline: ++ template: | ++ {{ $bucket := getComposedResource . "test-bucket" }} ++ --- ++ apiVersion: getcomposed.example.org/v1alpha1 ++ kind: XGetComposedResource ++ status: ++ bucketRef: ++ name: {{ $bucket.metadata.name }} ++ kind: GoTemplate ++ source: Inline ++ step: use-getcomposed ++ - functionRef: name: function-auto-ready step: detect-ready-resources @@ -72,7 +72,7 @@ Summary: 1 modified === Affected Composite Resources === - ✓ XGetComposedResource/test-getcomposed-resource (cluster-scoped) + ✓ XGetComposedResource/test-getcomposed-resource (cluster-scoped) Summary: 1 resource unchanged diff --git a/test/e2e/manifests/beta/diff/main/comp/expect/existing-xr.ansi b/test/e2e/manifests/beta/diff/main/comp/expect/existing-xr.ansi index f3bcb0b..983616d 100644 --- a/test/e2e/manifests/beta/diff/main/comp/expect/existing-xr.ansi +++ b/test/e2e/manifests/beta/diff/main/comp/expect/existing-xr.ansi @@ -30,18 +30,18 @@ patches: - fromFieldPath: spec.coolField toFieldPath: metadata.annotations[config-data] -+ transforms: -+ - string: -+ fmt: updated-%s -+ type: Format -+ type: string ++ transforms: ++ - string: ++ fmt: updated-%s ++ type: Format ++ type: string type: FromCompositeFieldPath - fromFieldPath: spec.coolField toFieldPath: metadata.annotations[resource-tier] transforms: - string: -- fmt: basic-%s -+ fmt: premium-%s +- fmt: basic-%s ++ fmt: premium-%s type: Format type: string type: FromCompositeFieldPath @@ -56,7 +56,7 @@ Summary: 1 modified === Affected Composite Resources === - ⚠ XCompDiffResource/test-comp-resource (cluster-scoped) + ⚠ XCompDiffResource/test-comp-resource (cluster-scoped) Summary: 1 resource with changes @@ -67,12 +67,12 @@ Summary: 1 resource with changes kind: ClusterNopResource metadata: annotations: -- config-data: existing-value -+ config-data: updated-existing-value +- config-data: existing-value ++ config-data: updated-existing-value crossplane.io/composition-resource-name: nop-resource crossplane.io/external-name: test-comp-resource-XXXXX -- resource-tier: basic-existing-value -+ resource-tier: premium-existing-value +- resource-tier: basic-existing-value ++ resource-tier: premium-existing-value finalizers: - finalizer.managedresource.crossplane.io generateName: test-comp-resource- diff --git a/test/e2e/manifests/beta/diff/main/v1-claim-nested/existing-claim.yaml b/test/e2e/manifests/beta/diff/main/v1-claim-nested/existing-claim.yaml new file mode 100644 index 0000000..7e4f42e --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/v1-claim-nested/existing-claim.yaml @@ -0,0 +1,10 @@ +apiVersion: claimnested.diff.example.org/v1alpha1 +kind: ParentNopClaim +metadata: + name: existing-parent-claim + namespace: default +spec: + parentField: existing-parent-value + compositeDeletePolicy: Background + compositionRef: + name: parent-nop-claim-composition diff --git a/test/e2e/manifests/beta/diff/main/v1-claim-nested/expect/existing-claim.ansi b/test/e2e/manifests/beta/diff/main/v1-claim-nested/expect/existing-claim.ansi new file mode 100644 index 0000000..a73ccc7 --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/v1-claim-nested/expect/existing-claim.ansi @@ -0,0 +1,81 @@ +~~~ ClusterNopResource/existing-parent-claim-XXXXX + apiVersion: nop.crossplane.io/v1alpha1 + kind: ClusterNopResource + metadata: + annotations: +- child-field: existing-parent-value ++ child-field: modified-parent-value + crossplane.io/composition-resource-name: nop-resource + crossplane.io/external-name: existing-parent-claim-XXXXX + finalizers: + - finalizer.managedresource.crossplane.io + generateName: existing-parent-claim-XXXXX- + labels: + crossplane.io/claim-name: existing-parent-claim + crossplane.io/claim-namespace: default + crossplane.io/composite: existing-parent-claim-XXXXX + name: existing-parent-claim-XXXXX + spec: + deletionPolicy: Delete + forProvider: + conditionAfter: + - conditionStatus: "True" + conditionType: Ready + time: 0s + managementPolicies: + - '*' + providerConfigRef: + name: default + +--- +~~~ ParentNopClaim/existing-parent-claim + apiVersion: claimnested.diff.example.org/v1alpha1 + kind: ParentNopClaim + metadata: + finalizers: + - finalizer.apiextensions.crossplane.io ++ labels: ++ new-label: added-value + name: existing-parent-claim + namespace: default + spec: + compositeDeletePolicy: Background + compositionRef: + name: parent-nop-claim-composition + compositionRevisionRef: + name: parent-nop-claim-composition-7c02fc0 +- parentField: existing-parent-value ++ compositionUpdatePolicy: Automatic ++ parentField: modified-parent-value + resourceRef: + apiVersion: claimnested.diff.example.org/v1alpha1 + kind: XParentNopClaim + name: existing-parent-claim-XXXXX + +--- +~~~ XChildNopClaim/existing-parent-claim-XXXXX + apiVersion: claimnested.diff.example.org/v1alpha1 + kind: XChildNopClaim + metadata: + annotations: + crossplane.io/composition-resource-name: child-xr + finalizers: + - composite.apiextensions.crossplane.io + generateName: existing-parent-claim-XXXXX- + labels: + crossplane.io/claim-name: existing-parent-claim + crossplane.io/claim-namespace: default + crossplane.io/composite: existing-parent-claim-XXXXX + name: existing-parent-claim-XXXXX + spec: +- childField: existing-parent-value ++ childField: modified-parent-value + compositionRef: + name: child-nop-claim-composition + compositionRevisionRef: + name: child-nop-claim-composition-d23139b + compositionUpdatePolicy: Automatic + +--- + +Summary: 3 modified diff --git a/test/e2e/manifests/beta/diff/main/v1-claim-nested/expect/new-claim.ansi b/test/e2e/manifests/beta/diff/main/v1-claim-nested/expect/new-claim.ansi new file mode 100644 index 0000000..ff251ad --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/v1-claim-nested/expect/new-claim.ansi @@ -0,0 +1,53 @@ ++++ ClusterNopResource/test-parent-claim-(generated)-(generated) ++ apiVersion: nop.crossplane.io/v1alpha1 ++ kind: ClusterNopResource ++ metadata: ++ annotations: ++ child-field: new-parent-value ++ crossplane.io/composition-resource-name: nop-resource ++ generateName: test-parent-claim-(generated)- ++ labels: ++ crossplane.io/composite: test-parent-claim-(generated) ++ spec: ++ deletionPolicy: Delete ++ forProvider: ++ conditionAfter: ++ - conditionStatus: "True" ++ conditionType: Ready ++ time: 0s ++ managementPolicies: ++ - '*' ++ providerConfigRef: ++ name: default + +--- ++++ ParentNopClaim/test-parent-claim ++ apiVersion: claimnested.diff.example.org/v1alpha1 ++ kind: ParentNopClaim ++ metadata: ++ name: test-parent-claim ++ namespace: default ++ spec: ++ compositeDeletePolicy: Background ++ compositionRef: ++ name: parent-nop-claim-composition ++ compositionUpdatePolicy: Automatic ++ parentField: new-parent-value + +--- ++++ XChildNopClaim/test-parent-claim-(generated) ++ apiVersion: claimnested.diff.example.org/v1alpha1 ++ kind: XChildNopClaim ++ metadata: ++ annotations: ++ crossplane.io/composition-resource-name: child-xr ++ generateName: test-parent-claim- ++ labels: ++ crossplane.io/composite: test-parent-claim ++ spec: ++ childField: new-parent-value ++ compositionUpdatePolicy: Automatic + +--- + +Summary: 3 added diff --git a/test/e2e/manifests/beta/diff/main/v1-claim-nested/modified-claim.yaml b/test/e2e/manifests/beta/diff/main/v1-claim-nested/modified-claim.yaml new file mode 100644 index 0000000..67844ef --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/v1-claim-nested/modified-claim.yaml @@ -0,0 +1,12 @@ +apiVersion: claimnested.diff.example.org/v1alpha1 +kind: ParentNopClaim +metadata: + name: existing-parent-claim + namespace: default + labels: + new-label: added-value +spec: + parentField: modified-parent-value + compositeDeletePolicy: Background + compositionRef: + name: parent-nop-claim-composition diff --git a/test/e2e/manifests/beta/diff/main/v1-claim-nested/new-claim.yaml b/test/e2e/manifests/beta/diff/main/v1-claim-nested/new-claim.yaml new file mode 100644 index 0000000..eb4409e --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/v1-claim-nested/new-claim.yaml @@ -0,0 +1,10 @@ +apiVersion: claimnested.diff.example.org/v1alpha1 +kind: ParentNopClaim +metadata: + name: test-parent-claim + namespace: default +spec: + parentField: new-parent-value + compositeDeletePolicy: Background + compositionRef: + name: parent-nop-claim-composition diff --git a/test/e2e/manifests/beta/diff/main/v1-claim-nested/setup/child-composition.yaml b/test/e2e/manifests/beta/diff/main/v1-claim-nested/setup/child-composition.yaml new file mode 100644 index 0000000..aa98e66 --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/v1-claim-nested/setup/child-composition.yaml @@ -0,0 +1,34 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: child-nop-claim-composition +spec: + compositeTypeRef: + apiVersion: claimnested.diff.example.org/v1alpha1 + kind: XChildNopClaim + mode: Pipeline + pipeline: + - step: create-nop-resource + functionRef: + name: function-patch-and-transform + input: + apiVersion: pt.fn.crossplane.io/v1beta1 + kind: Resources + resources: + - name: nop-resource + base: + apiVersion: nop.crossplane.io/v1alpha1 + kind: ClusterNopResource + spec: + forProvider: + conditionAfter: + - conditionType: Ready + conditionStatus: "True" + time: 0s + patches: + - type: FromCompositeFieldPath + fromFieldPath: spec.childField + toFieldPath: metadata.annotations[child-field] + - step: auto-ready + functionRef: + name: function-auto-ready diff --git a/test/e2e/manifests/beta/diff/main/v1-claim-nested/setup/child-definition.yaml b/test/e2e/manifests/beta/diff/main/v1-claim-nested/setup/child-definition.yaml new file mode 100644 index 0000000..bbc9009 --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/v1-claim-nested/setup/child-definition.yaml @@ -0,0 +1,29 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: CompositeResourceDefinition +metadata: + name: xchildnopclaims.claimnested.diff.example.org +spec: + group: claimnested.diff.example.org + names: + kind: XChildNopClaim + plural: xchildnopclaims + versions: + - name: v1alpha1 + served: true + referenceable: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + childField: + type: string + required: + - childField + status: + type: object + properties: + message: + type: string diff --git a/test/e2e/manifests/beta/diff/main/v1-claim-nested/setup/parent-composition.yaml b/test/e2e/manifests/beta/diff/main/v1-claim-nested/setup/parent-composition.yaml new file mode 100644 index 0000000..fc8f73a --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/v1-claim-nested/setup/parent-composition.yaml @@ -0,0 +1,31 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: parent-nop-claim-composition +spec: + compositeTypeRef: + apiVersion: claimnested.diff.example.org/v1alpha1 + kind: XParentNopClaim + mode: Pipeline + pipeline: + - step: create-child-xr + functionRef: + name: function-go-templating + input: + apiVersion: gotemplating.fn.crossplane.io/v1beta1 + kind: GoTemplate + source: Inline + inline: + template: | + apiVersion: claimnested.diff.example.org/v1alpha1 + kind: XChildNopClaim + metadata: + generateName: {{ .observed.composite.resource.metadata.name }}- + namespace: {{ .observed.composite.resource.metadata.namespace }} + annotations: + gotemplating.fn.crossplane.io/composition-resource-name: child-xr + spec: + childField: {{ .observed.composite.resource.spec.parentField }} + - step: auto-ready + functionRef: + name: function-auto-ready diff --git a/test/e2e/manifests/beta/diff/main/v1-claim-nested/setup/parent-definition.yaml b/test/e2e/manifests/beta/diff/main/v1-claim-nested/setup/parent-definition.yaml new file mode 100644 index 0000000..ff963b3 --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/v1-claim-nested/setup/parent-definition.yaml @@ -0,0 +1,32 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: CompositeResourceDefinition +metadata: + name: xparentnopclaims.claimnested.diff.example.org +spec: + group: claimnested.diff.example.org + names: + kind: XParentNopClaim + plural: xparentnopclaims + claimNames: + kind: ParentNopClaim + plural: parentnopclaims + versions: + - name: v1alpha1 + served: true + referenceable: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + parentField: + type: string + required: + - parentField + status: + type: object + properties: + message: + type: string diff --git a/test/e2e/manifests/beta/diff/main/v1-claim/expect/existing-claim.ansi b/test/e2e/manifests/beta/diff/main/v1-claim/expect/existing-claim.ansi index b0f53a6..b1957b1 100644 --- a/test/e2e/manifests/beta/diff/main/v1-claim/expect/existing-claim.ansi +++ b/test/e2e/manifests/beta/diff/main/v1-claim/expect/existing-claim.ansi @@ -1,21 +1,21 @@ -~~~ ClusterNopResource/test-claim-xxxxx +~~~ ClusterNopResource/test-claim-XXXXX apiVersion: nop.crossplane.io/v1alpha1 kind: ClusterNopResource metadata: annotations: -- cool-field: existing-value -+ cool-field: modified-value +- cool-field: existing-value ++ cool-field: modified-value crossplane.io/composition-resource-name: nop-resource - crossplane.io/external-name: test-claim-xxxxx-xxxxx -+ setting: enabled + crossplane.io/external-name: test-claim-XXXXX ++ setting: enabled finalizers: - finalizer.managedresource.crossplane.io - generateName: test-claim-xxxxx- + generateName: test-claim-XXXXX- labels: crossplane.io/claim-name: test-claim crossplane.io/claim-namespace: default - crossplane.io/composite: test-claim-xxxxx-xxxxx - name: test-claim-xxxxx-xxxxx + crossplane.io/composite: test-claim-XXXXX + name: test-claim-XXXXX spec: deletionPolicy: Delete forProvider: @@ -42,18 +42,18 @@ compositionRef: name: xnopclaims.claim.diff.example.org compositionRevisionRef: - name: xnopclaims.claim.diff.example.org-xxxxxxx -- coolField: existing-value -+ compositionUpdatePolicy: Automatic -+ coolField: modified-value -+ parameters: -+ config: -+ setting1: enabled + name: xnopclaims.claim.diff.example.org-XXXXXXX +- coolField: existing-value ++ compositionUpdatePolicy: Automatic ++ coolField: modified-value ++ parameters: ++ config: ++ setting1: enabled resourceRef: apiVersion: claim.diff.example.org/v1alpha1 kind: XNopClaim - name: test-claim-xxxxx + name: test-claim-XXXXX --- -Summary: 2 modified \ No newline at end of file +Summary: 2 modified diff --git a/test/e2e/manifests/beta/diff/main/v1-claim/expect/new-claim.ansi b/test/e2e/manifests/beta/diff/main/v1-claim/expect/new-claim.ansi index c456cb0..346f439 100644 --- a/test/e2e/manifests/beta/diff/main/v1-claim/expect/new-claim.ansi +++ b/test/e2e/manifests/beta/diff/main/v1-claim/expect/new-claim.ansi @@ -1,39 +1,39 @@ +++ ClusterNopResource/test-claim-(generated) -+ apiVersion: nop.crossplane.io/v1alpha1 -+ kind: ClusterNopResource -+ metadata: -+ annotations: -+ cool-field: new-value -+ crossplane.io/composition-resource-name: nop-resource -+ generateName: test-claim- -+ labels: -+ crossplane.io/composite: test-claim -+ spec: -+ deletionPolicy: Delete -+ forProvider: -+ conditionAfter: -+ - conditionStatus: "True" -+ conditionType: Ready -+ time: 0s -+ managementPolicies: -+ - '*' -+ providerConfigRef: -+ name: default ++ apiVersion: nop.crossplane.io/v1alpha1 ++ kind: ClusterNopResource ++ metadata: ++ annotations: ++ cool-field: new-value ++ crossplane.io/composition-resource-name: nop-resource ++ generateName: test-claim- ++ labels: ++ crossplane.io/composite: test-claim ++ spec: ++ deletionPolicy: Delete ++ forProvider: ++ conditionAfter: ++ - conditionStatus: "True" ++ conditionType: Ready ++ time: 0s ++ managementPolicies: ++ - '*' ++ providerConfigRef: ++ name: default --- +++ NopClaim/test-claim -+ apiVersion: claim.diff.example.org/v1alpha1 -+ kind: NopClaim -+ metadata: -+ name: test-claim -+ namespace: default -+ spec: -+ compositeDeletePolicy: Background -+ compositionRef: -+ name: xnopclaims.claim.diff.example.org -+ compositionUpdatePolicy: Automatic -+ coolField: new-value ++ apiVersion: claim.diff.example.org/v1alpha1 ++ kind: NopClaim ++ metadata: ++ name: test-claim ++ namespace: default ++ spec: ++ compositeDeletePolicy: Background ++ compositionRef: ++ name: xnopclaims.claim.diff.example.org ++ compositionUpdatePolicy: Automatic ++ coolField: new-value --- -Summary: 2 added \ No newline at end of file +Summary: 2 added diff --git a/test/e2e/manifests/beta/diff/main/v1/expect/existing-xr.ansi b/test/e2e/manifests/beta/diff/main/v1/expect/existing-xr.ansi index a25c0af..891c7d2 100644 --- a/test/e2e/manifests/beta/diff/main/v1/expect/existing-xr.ansi +++ b/test/e2e/manifests/beta/diff/main/v1/expect/existing-xr.ansi @@ -1,20 +1,20 @@ -~~~ ClusterNopResource/existing-resource-czpjg +~~~ ClusterNopResource/existing-resource-XXXXX apiVersion: nop.crossplane.io/v1alpha1 kind: ClusterNopResource metadata: annotations: -- cool-field: I'm existing! -+ cool-field: I'm modified! +- cool-field: I'm existing! ++ cool-field: I'm modified! crossplane.io/composition-resource-name: nop-resource - crossplane.io/external-name: existing-resource-czpjg -- setting: original -+ setting: changed + crossplane.io/external-name: existing-resource-XXXXX +- setting: original ++ setting: changed finalizers: - finalizer.managedresource.crossplane.io generateName: existing-resource- labels: crossplane.io/composite: existing-resource - name: existing-resource-czpjg + name: existing-resource-XXXXX spec: deletionPolicy: Delete forProvider: @@ -41,17 +41,17 @@ compositionRef: name: xnopresources.legacy.diff.example.org compositionRevisionRef: - name: xnopresources.legacy.diff.example.org-acd5cd1 + name: xnopresources.legacy.diff.example.org-XXXXXXX compositionUpdatePolicy: Automatic -- coolField: I'm existing! -+ coolField: I'm modified! +- coolField: I'm existing! ++ coolField: I'm modified! parameters: config: -- setting1: original -- setting2: original -+ setting1: changed -+ setting2: original -+ setting3: new-value +- setting1: original +- setting2: original ++ setting1: changed ++ setting2: original ++ setting3: new-value diff --git a/test/e2e/manifests/beta/diff/main/v1/expect/new-xr.ansi b/test/e2e/manifests/beta/diff/main/v1/expect/new-xr.ansi index 5f26c7e..f5faa14 100644 --- a/test/e2e/manifests/beta/diff/main/v1/expect/new-xr.ansi +++ b/test/e2e/manifests/beta/diff/main/v1/expect/new-xr.ansi @@ -1,40 +1,40 @@ +++ ClusterNopResource/new-resource-(generated) -+ apiVersion: nop.crossplane.io/v1alpha1 -+ kind: ClusterNopResource -+ metadata: -+ annotations: -+ cool-field: I'm new! -+ crossplane.io/composition-resource-name: nop-resource -+ setting: value1 -+ generateName: new-resource- -+ labels: -+ crossplane.io/composite: new-resource -+ spec: -+ deletionPolicy: Delete -+ forProvider: -+ conditionAfter: -+ - conditionStatus: "True" -+ conditionType: Ready -+ time: 0s -+ managementPolicies: -+ - '*' -+ providerConfigRef: -+ name: default ++ apiVersion: nop.crossplane.io/v1alpha1 ++ kind: ClusterNopResource ++ metadata: ++ annotations: ++ cool-field: I'm new! ++ crossplane.io/composition-resource-name: nop-resource ++ setting: value1 ++ generateName: new-resource- ++ labels: ++ crossplane.io/composite: new-resource ++ spec: ++ deletionPolicy: Delete ++ forProvider: ++ conditionAfter: ++ - conditionStatus: "True" ++ conditionType: Ready ++ time: 0s ++ managementPolicies: ++ - '*' ++ providerConfigRef: ++ name: default --- +++ XNopResource/new-resource -+ apiVersion: legacy.diff.example.org/v1alpha1 -+ kind: XNopResource -+ metadata: -+ name: new-resource -+ spec: -+ compositionUpdatePolicy: Automatic -+ coolField: I'm new! -+ parameters: -+ config: -+ setting1: value1 -+ setting2: value2 ++ apiVersion: legacy.diff.example.org/v1alpha1 ++ kind: XNopResource ++ metadata: ++ name: new-resource ++ spec: ++ compositionUpdatePolicy: Automatic ++ coolField: I'm new! ++ parameters: ++ config: ++ setting1: value1 ++ setting2: value2 --- -Summary: 2 added \ No newline at end of file +Summary: 2 added diff --git a/test/e2e/manifests/beta/diff/main/v2-cluster/expect/existing-xr.ansi b/test/e2e/manifests/beta/diff/main/v2-cluster/expect/existing-xr.ansi index 77db05a..e5c3c89 100644 --- a/test/e2e/manifests/beta/diff/main/v2-cluster/expect/existing-xr.ansi +++ b/test/e2e/manifests/beta/diff/main/v2-cluster/expect/existing-xr.ansi @@ -1,20 +1,20 @@ -~~~ ClusterNopResource/existing-resource-czpjg +~~~ ClusterNopResource/existing-resource-XXXXX apiVersion: nop.crossplane.io/v1alpha1 kind: ClusterNopResource metadata: annotations: -- cool-field: I'm existing! -+ cool-field: I'm modified! +- cool-field: I'm existing! ++ cool-field: I'm modified! crossplane.io/composition-resource-name: nop-resource - crossplane.io/external-name: existing-resource-czpjg -- setting: original -+ setting: changed + crossplane.io/external-name: existing-resource-XXXXX +- setting: original ++ setting: changed finalizers: - finalizer.managedresource.crossplane.io generateName: existing-resource- labels: crossplane.io/composite: existing-resource - name: existing-resource-czpjg + name: existing-resource-XXXXX spec: deletionPolicy: Delete forProvider: @@ -38,21 +38,21 @@ crossplane.io/composite: existing-resource name: existing-resource spec: -- coolField: I'm existing! -+ coolField: I'm modified! +- coolField: I'm existing! ++ coolField: I'm modified! crossplane: compositionRef: name: xnopresources.cluster.diff.example.org compositionRevisionRef: - name: xnopresources.cluster.diff.example.org-acd5cd1 + name: xnopresources.cluster.diff.example.org-XXXXXXX compositionUpdatePolicy: Automatic parameters: config: -- setting1: original -- setting2: original -+ setting1: changed -+ setting2: original -+ setting3: new-value +- setting1: original +- setting2: original ++ setting1: changed ++ setting2: original ++ setting3: new-value diff --git a/test/e2e/manifests/beta/diff/main/v2-cluster/expect/new-xr.ansi b/test/e2e/manifests/beta/diff/main/v2-cluster/expect/new-xr.ansi index 625a04e..13fb999 100644 --- a/test/e2e/manifests/beta/diff/main/v2-cluster/expect/new-xr.ansi +++ b/test/e2e/manifests/beta/diff/main/v2-cluster/expect/new-xr.ansi @@ -1,39 +1,39 @@ +++ ClusterNopResource/new-resource-(generated) -+ apiVersion: nop.crossplane.io/v1alpha1 -+ kind: ClusterNopResource -+ metadata: -+ annotations: -+ cool-field: I'm new! -+ crossplane.io/composition-resource-name: nop-resource -+ setting: value1 -+ generateName: new-resource- -+ labels: -+ crossplane.io/composite: new-resource -+ spec: -+ deletionPolicy: Delete -+ forProvider: -+ conditionAfter: -+ - conditionStatus: "True" -+ conditionType: Ready -+ time: 0s -+ managementPolicies: -+ - '*' -+ providerConfigRef: -+ name: default ++ apiVersion: nop.crossplane.io/v1alpha1 ++ kind: ClusterNopResource ++ metadata: ++ annotations: ++ cool-field: I'm new! ++ crossplane.io/composition-resource-name: nop-resource ++ setting: value1 ++ generateName: new-resource- ++ labels: ++ crossplane.io/composite: new-resource ++ spec: ++ deletionPolicy: Delete ++ forProvider: ++ conditionAfter: ++ - conditionStatus: "True" ++ conditionType: Ready ++ time: 0s ++ managementPolicies: ++ - '*' ++ providerConfigRef: ++ name: default --- +++ XNopResource/new-resource -+ apiVersion: cluster.diff.example.org/v1alpha1 -+ kind: XNopResource -+ metadata: -+ name: new-resource -+ spec: -+ coolField: I'm new! -+ parameters: -+ config: -+ setting1: value1 -+ setting2: value2 ++ apiVersion: cluster.diff.example.org/v1alpha1 ++ kind: XNopResource ++ metadata: ++ name: new-resource ++ spec: ++ coolField: I'm new! ++ parameters: ++ config: ++ setting1: value1 ++ setting2: value2 --- -Summary: 2 added \ No newline at end of file +Summary: 2 added diff --git a/test/e2e/manifests/beta/diff/main/v2-namespaced/expect/existing-xr.ansi b/test/e2e/manifests/beta/diff/main/v2-namespaced/expect/existing-xr.ansi index 0198471..6b484e3 100644 --- a/test/e2e/manifests/beta/diff/main/v2-namespaced/expect/existing-xr.ansi +++ b/test/e2e/manifests/beta/diff/main/v2-namespaced/expect/existing-xr.ansi @@ -1,20 +1,20 @@ -~~~ NopResource/existing-resource-czpjg +~~~ NopResource/existing-resource-XXXXX apiVersion: nop.crossplane.io/v1alpha1 kind: NopResource metadata: annotations: -- cool-field: I'm existing! -+ cool-field: I'm modified! +- cool-field: I'm existing! ++ cool-field: I'm modified! crossplane.io/composition-resource-name: nop-resource - crossplane.io/external-name: existing-resource-czpjg -- setting: original -+ setting: changed + crossplane.io/external-name: existing-resource-XXXXX +- setting: original ++ setting: changed finalizers: - finalizer.managedresource.crossplane.io generateName: existing-resource- labels: crossplane.io/composite: existing-resource - name: existing-resource-czpjg + name: existing-resource-XXXXX namespace: default spec: deletionPolicy: Delete @@ -40,21 +40,21 @@ name: existing-resource namespace: default spec: -- coolField: I'm existing! -+ coolField: I'm modified! +- coolField: I'm existing! ++ coolField: I'm modified! crossplane: compositionRef: name: xnopresources.diff.example.org compositionRevisionRef: - name: xnopresources.diff.example.org-acd5cd1 + name: xnopresources.diff.example.org-XXXXXXX compositionUpdatePolicy: Automatic parameters: config: -- setting1: original -- setting2: original -+ setting1: changed -+ setting2: original -+ setting3: new-value +- setting1: original +- setting2: original ++ setting1: changed ++ setting2: original ++ setting3: new-value diff --git a/test/e2e/manifests/beta/diff/main/v2-namespaced/expect/new-xr.ansi b/test/e2e/manifests/beta/diff/main/v2-namespaced/expect/new-xr.ansi index ca1e3c0..86baa79 100644 --- a/test/e2e/manifests/beta/diff/main/v2-namespaced/expect/new-xr.ansi +++ b/test/e2e/manifests/beta/diff/main/v2-namespaced/expect/new-xr.ansi @@ -1,41 +1,41 @@ +++ NopResource/new-resource-(generated) -+ apiVersion: nop.crossplane.io/v1alpha1 -+ kind: NopResource -+ metadata: -+ annotations: -+ cool-field: I'm new! -+ crossplane.io/composition-resource-name: nop-resource -+ setting: value1 -+ generateName: new-resource- -+ labels: -+ crossplane.io/composite: new-resource -+ namespace: default -+ spec: -+ deletionPolicy: Delete -+ forProvider: -+ conditionAfter: -+ - conditionStatus: "True" -+ conditionType: Ready -+ time: 0s -+ managementPolicies: -+ - '*' -+ providerConfigRef: -+ name: default ++ apiVersion: nop.crossplane.io/v1alpha1 ++ kind: NopResource ++ metadata: ++ annotations: ++ cool-field: I'm new! ++ crossplane.io/composition-resource-name: nop-resource ++ setting: value1 ++ generateName: new-resource- ++ labels: ++ crossplane.io/composite: new-resource ++ namespace: default ++ spec: ++ deletionPolicy: Delete ++ forProvider: ++ conditionAfter: ++ - conditionStatus: "True" ++ conditionType: Ready ++ time: 0s ++ managementPolicies: ++ - '*' ++ providerConfigRef: ++ name: default --- +++ XNopResource/new-resource -+ apiVersion: diff.example.org/v1alpha1 -+ kind: XNopResource -+ metadata: -+ name: new-resource -+ namespace: default -+ spec: -+ coolField: I'm new! -+ parameters: -+ config: -+ setting1: value1 -+ setting2: value2 ++ apiVersion: diff.example.org/v1alpha1 ++ kind: XNopResource ++ metadata: ++ name: new-resource ++ namespace: default ++ spec: ++ coolField: I'm new! ++ parameters: ++ config: ++ setting1: value1 ++ setting2: value2 --- -Summary: 2 added \ No newline at end of file +Summary: 2 added diff --git a/test/e2e/manifests/beta/diff/main/v2-nested-generatename/existing-parent-xr.yaml b/test/e2e/manifests/beta/diff/main/v2-nested-generatename/existing-parent-xr.yaml new file mode 100644 index 0000000..f995eb4 --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/v2-nested-generatename/existing-parent-xr.yaml @@ -0,0 +1,10 @@ +apiVersion: nested.diff.example.org/v1alpha1 +kind: XParentNop +metadata: + name: test-parent-generatename + namespace: default +spec: + crossplane: + compositionRef: + name: parent-nop-composition-generatename + parentField: "existing-value" diff --git a/test/e2e/manifests/beta/diff/main/v2-nested-generatename/expect/existing-parent-xr.ansi b/test/e2e/manifests/beta/diff/main/v2-nested-generatename/expect/existing-parent-xr.ansi new file mode 100644 index 0000000..a6df8e8 --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/v2-nested-generatename/expect/existing-parent-xr.ansi @@ -0,0 +1,47 @@ +~~~ XChildNop/test-parent-generatename-child-XXXXX + apiVersion: nested.diff.example.org/v1alpha1 + kind: XChildNop + metadata: + annotations: + crossplane.io/composition-resource-name: child-xr + finalizers: + - composite.apiextensions.crossplane.io + generateName: test-parent-generatename-child- + labels: + crossplane.io/composite: test-parent-generatename + name: test-parent-generatename-child-XXXXX + namespace: default + spec: +- childField: existing-value ++ childField: modified-value + crossplane: + compositionRef: + name: child-nop-composition + compositionRevisionRef: + name: child-nop-composition-XXXXXXX + compositionUpdatePolicy: Automatic + +--- +~~~ XParentNop/test-parent-generatename + apiVersion: nested.diff.example.org/v1alpha1 + kind: XParentNop + metadata: + finalizers: + - composite.apiextensions.crossplane.io + labels: + crossplane.io/composite: test-parent-generatename + name: test-parent-generatename + namespace: default + spec: + crossplane: + compositionRef: + name: parent-nop-composition-XXXXXXX + compositionRevisionRef: + name: parent-nop-composition-XXXXXXX-caacbdf + compositionUpdatePolicy: Automatic +- parentField: existing-value ++ parentField: modified-value + +--- + +Summary: 2 modified diff --git a/test/e2e/manifests/beta/diff/main/v2-nested-generatename/modified-parent-xr.yaml b/test/e2e/manifests/beta/diff/main/v2-nested-generatename/modified-parent-xr.yaml new file mode 100644 index 0000000..32e4572 --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/v2-nested-generatename/modified-parent-xr.yaml @@ -0,0 +1,10 @@ +apiVersion: nested.diff.example.org/v1alpha1 +kind: XParentNop +metadata: + name: test-parent-generatename + namespace: default +spec: + crossplane: + compositionRef: + name: parent-nop-composition-generatename + parentField: "modified-value" diff --git a/test/e2e/manifests/beta/diff/main/v2-nested-generatename/setup/child-composition.yaml b/test/e2e/manifests/beta/diff/main/v2-nested-generatename/setup/child-composition.yaml new file mode 100644 index 0000000..7f9239a --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/v2-nested-generatename/setup/child-composition.yaml @@ -0,0 +1,34 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: child-nop-composition +spec: + compositeTypeRef: + apiVersion: nested.diff.example.org/v1alpha1 + kind: XChildNop + mode: Pipeline + pipeline: + - step: create-nop-resource + functionRef: + name: function-go-templating + input: + apiVersion: gotemplating.fn.crossplane.io/v1beta1 + kind: GoTemplate + source: Inline + inline: + template: | + apiVersion: nop.crossplane.io/v1alpha1 + kind: NopResource + metadata: + name: {{ .observed.composite.resource.metadata.name }}-nop + annotations: + gotemplating.fn.crossplane.io/composition-resource-name: nop-resource + spec: + forProvider: + conditionAfter: + - conditionType: Ready + conditionStatus: "True" + time: 0s + - step: auto-ready + functionRef: + name: function-auto-ready diff --git a/test/e2e/manifests/beta/diff/main/v2-nested-generatename/setup/child-definition.yaml b/test/e2e/manifests/beta/diff/main/v2-nested-generatename/setup/child-definition.yaml new file mode 100644 index 0000000..64e9bfc --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/v2-nested-generatename/setup/child-definition.yaml @@ -0,0 +1,30 @@ +apiVersion: apiextensions.crossplane.io/v2 +kind: CompositeResourceDefinition +metadata: + name: xchildnops.nested.diff.example.org +spec: + group: nested.diff.example.org + names: + kind: XChildNop + plural: xchildnops + scope: Namespaced + versions: + - name: v1alpha1 + served: true + referenceable: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + childField: + type: string + required: + - childField + status: + type: object + properties: + message: + type: string diff --git a/test/e2e/manifests/beta/diff/main/v2-nested-generatename/setup/parent-composition.yaml b/test/e2e/manifests/beta/diff/main/v2-nested-generatename/setup/parent-composition.yaml new file mode 100644 index 0000000..a52f5b0 --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/v2-nested-generatename/setup/parent-composition.yaml @@ -0,0 +1,32 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: parent-nop-composition-generatename +spec: + compositeTypeRef: + apiVersion: nested.diff.example.org/v1alpha1 + kind: XParentNop + mode: Pipeline + pipeline: + - step: create-child-xr + functionRef: + name: function-go-templating + input: + apiVersion: gotemplating.fn.crossplane.io/v1beta1 + kind: GoTemplate + source: Inline + inline: + template: | + apiVersion: nested.diff.example.org/v1alpha1 + kind: XChildNop + metadata: + # Use generateName instead of explicit name to reproduce the bug + generateName: {{ .observed.composite.resource.metadata.name }}-child- + namespace: {{ .observed.composite.resource.metadata.namespace }} + annotations: + gotemplating.fn.crossplane.io/composition-resource-name: child-xr + spec: + childField: {{ .observed.composite.resource.spec.parentField }} + - step: auto-ready + functionRef: + name: function-auto-ready diff --git a/test/e2e/manifests/beta/diff/main/v2-nested-generatename/setup/parent-definition.yaml b/test/e2e/manifests/beta/diff/main/v2-nested-generatename/setup/parent-definition.yaml new file mode 100644 index 0000000..6a28d9f --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/v2-nested-generatename/setup/parent-definition.yaml @@ -0,0 +1,30 @@ +apiVersion: apiextensions.crossplane.io/v2 +kind: CompositeResourceDefinition +metadata: + name: xparentnops.nested.diff.example.org +spec: + group: nested.diff.example.org + names: + kind: XParentNop + plural: xparentnops + scope: Namespaced + versions: + - name: v1alpha1 + served: true + referenceable: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + parentField: + type: string + required: + - parentField + status: + type: object + properties: + message: + type: string diff --git a/test/e2e/manifests/beta/diff/main/v2-nested/expect/existing-parent-xr.ansi b/test/e2e/manifests/beta/diff/main/v2-nested/expect/existing-parent-xr.ansi index fb12aac..8c0e0c0 100644 --- a/test/e2e/manifests/beta/diff/main/v2-nested/expect/existing-parent-xr.ansi +++ b/test/e2e/manifests/beta/diff/main/v2-nested/expect/existing-parent-xr.ansi @@ -1,30 +1,3 @@ -~~~ NopResource/test-parent-existing-child-nop - apiVersion: nop.crossplane.io/v1alpha1 - kind: NopResource - metadata: - annotations: - crossplane.io/composition-resource-name: nop-resource - crossplane.io/external-name: test-parent-existing-child-nop - finalizers: - - finalizer.managedresource.crossplane.io - labels: -- crossplane.io/composite: test-parent-existing -+ crossplane.io/composite: test-parent-existing-child - name: test-parent-existing-child-nop - namespace: default - spec: - deletionPolicy: Delete - forProvider: - conditionAfter: - - conditionStatus: "True" - conditionType: Ready - time: 0s - managementPolicies: - - '*' - providerConfigRef: - name: default - ---- ~~~ XChildNop/test-parent-existing-child apiVersion: nested.diff.example.org/v1alpha1 kind: XChildNop @@ -70,4 +43,4 @@ --- -Summary: 3 modified +Summary: 2 modified diff --git a/test/e2e/manifests/beta/diff/release-1.20/v1-claim-nested/existing-claim.yaml b/test/e2e/manifests/beta/diff/release-1.20/v1-claim-nested/existing-claim.yaml new file mode 100644 index 0000000..7e4f42e --- /dev/null +++ b/test/e2e/manifests/beta/diff/release-1.20/v1-claim-nested/existing-claim.yaml @@ -0,0 +1,10 @@ +apiVersion: claimnested.diff.example.org/v1alpha1 +kind: ParentNopClaim +metadata: + name: existing-parent-claim + namespace: default +spec: + parentField: existing-parent-value + compositeDeletePolicy: Background + compositionRef: + name: parent-nop-claim-composition diff --git a/test/e2e/manifests/beta/diff/release-1.20/v1-claim-nested/modified-claim.yaml b/test/e2e/manifests/beta/diff/release-1.20/v1-claim-nested/modified-claim.yaml new file mode 100644 index 0000000..67844ef --- /dev/null +++ b/test/e2e/manifests/beta/diff/release-1.20/v1-claim-nested/modified-claim.yaml @@ -0,0 +1,12 @@ +apiVersion: claimnested.diff.example.org/v1alpha1 +kind: ParentNopClaim +metadata: + name: existing-parent-claim + namespace: default + labels: + new-label: added-value +spec: + parentField: modified-parent-value + compositeDeletePolicy: Background + compositionRef: + name: parent-nop-claim-composition diff --git a/test/e2e/manifests/beta/diff/release-1.20/v1-claim-nested/new-claim.yaml b/test/e2e/manifests/beta/diff/release-1.20/v1-claim-nested/new-claim.yaml new file mode 100644 index 0000000..eb4409e --- /dev/null +++ b/test/e2e/manifests/beta/diff/release-1.20/v1-claim-nested/new-claim.yaml @@ -0,0 +1,10 @@ +apiVersion: claimnested.diff.example.org/v1alpha1 +kind: ParentNopClaim +metadata: + name: test-parent-claim + namespace: default +spec: + parentField: new-parent-value + compositeDeletePolicy: Background + compositionRef: + name: parent-nop-claim-composition diff --git a/test/e2e/manifests/beta/diff/release-1.20/v1-claim-nested/setup/child-composition.yaml b/test/e2e/manifests/beta/diff/release-1.20/v1-claim-nested/setup/child-composition.yaml new file mode 100644 index 0000000..b2d18bd --- /dev/null +++ b/test/e2e/manifests/beta/diff/release-1.20/v1-claim-nested/setup/child-composition.yaml @@ -0,0 +1,34 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: child-nop-claim-composition +spec: + compositeTypeRef: + apiVersion: claimnested.diff.example.org/v1alpha1 + kind: XChildNopClaim + mode: Pipeline + pipeline: + - step: create-nop-resource + functionRef: + name: function-patch-and-transform + input: + apiVersion: pt.fn.crossplane.io/v1beta1 + kind: Resources + resources: + - name: nop-resource + base: + apiVersion: nop.crossplane.io/v1alpha1 + kind: NopResource + spec: + forProvider: + conditionAfter: + - conditionType: Ready + conditionStatus: "True" + time: 0s + patches: + - type: FromCompositeFieldPath + fromFieldPath: spec.childField + toFieldPath: metadata.annotations[child-field] + - step: auto-ready + functionRef: + name: function-auto-ready diff --git a/test/e2e/manifests/beta/diff/release-1.20/v1-claim-nested/setup/child-definition.yaml b/test/e2e/manifests/beta/diff/release-1.20/v1-claim-nested/setup/child-definition.yaml new file mode 100644 index 0000000..bbc9009 --- /dev/null +++ b/test/e2e/manifests/beta/diff/release-1.20/v1-claim-nested/setup/child-definition.yaml @@ -0,0 +1,29 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: CompositeResourceDefinition +metadata: + name: xchildnopclaims.claimnested.diff.example.org +spec: + group: claimnested.diff.example.org + names: + kind: XChildNopClaim + plural: xchildnopclaims + versions: + - name: v1alpha1 + served: true + referenceable: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + childField: + type: string + required: + - childField + status: + type: object + properties: + message: + type: string diff --git a/test/e2e/manifests/beta/diff/release-1.20/v1-claim-nested/setup/parent-composition.yaml b/test/e2e/manifests/beta/diff/release-1.20/v1-claim-nested/setup/parent-composition.yaml new file mode 100644 index 0000000..fc8f73a --- /dev/null +++ b/test/e2e/manifests/beta/diff/release-1.20/v1-claim-nested/setup/parent-composition.yaml @@ -0,0 +1,31 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: parent-nop-claim-composition +spec: + compositeTypeRef: + apiVersion: claimnested.diff.example.org/v1alpha1 + kind: XParentNopClaim + mode: Pipeline + pipeline: + - step: create-child-xr + functionRef: + name: function-go-templating + input: + apiVersion: gotemplating.fn.crossplane.io/v1beta1 + kind: GoTemplate + source: Inline + inline: + template: | + apiVersion: claimnested.diff.example.org/v1alpha1 + kind: XChildNopClaim + metadata: + generateName: {{ .observed.composite.resource.metadata.name }}- + namespace: {{ .observed.composite.resource.metadata.namespace }} + annotations: + gotemplating.fn.crossplane.io/composition-resource-name: child-xr + spec: + childField: {{ .observed.composite.resource.spec.parentField }} + - step: auto-ready + functionRef: + name: function-auto-ready diff --git a/test/e2e/manifests/beta/diff/release-1.20/v1-claim-nested/setup/parent-definition.yaml b/test/e2e/manifests/beta/diff/release-1.20/v1-claim-nested/setup/parent-definition.yaml new file mode 100644 index 0000000..ff963b3 --- /dev/null +++ b/test/e2e/manifests/beta/diff/release-1.20/v1-claim-nested/setup/parent-definition.yaml @@ -0,0 +1,32 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: CompositeResourceDefinition +metadata: + name: xparentnopclaims.claimnested.diff.example.org +spec: + group: claimnested.diff.example.org + names: + kind: XParentNopClaim + plural: xparentnopclaims + claimNames: + kind: ParentNopClaim + plural: parentnopclaims + versions: + - name: v1alpha1 + served: true + referenceable: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + parentField: + type: string + required: + - parentField + status: + type: object + properties: + message: + type: string diff --git a/test/e2e/xr_advanced_test.go b/test/e2e/xr_advanced_test.go index 194180d..3b771e4 100644 --- a/test/e2e/xr_advanced_test.go +++ b/test/e2e/xr_advanced_test.go @@ -187,3 +187,70 @@ func TestDiffExistingNestedResourceV2(t *testing.T) { Feature(), ) } + +// TestDiffExistingNestedResourceV2WithGenerateName tests the crossplane diff command +// against existing nested XR resources where the child XR uses generateName instead of an explicit name. +// +// This is a minimal E2E reproduction of the nested XR identity preservation bug. +// The bug manifests when: +// 1. A parent XR creates a child XR using generateName (not explicit name) +// 2. The parent XR is modified (changing spec fields) +// 3. Without identity preservation, the child XR gets a new random suffix on each render +// 4. This causes all managed resources owned by the child XR to appear as removed/added +// +// The existing TestDiffExistingNestedResourceV2 test doesn't catch this bug because +// it uses an explicit name template: `name: {{ .observed.composite.resource.metadata.name }}-child` +// This produces deterministic naming (always "test-parent-existing-child"), masking the bug. +// +// This test uses `generateName: {{ .observed.composite.resource.metadata.name }}-child-` +// which produces non-deterministic names (e.g., "test-parent-generatename-child-abc123"). +// Without identity preservation, each render would get a new random suffix. +func TestDiffExistingNestedResourceV2WithGenerateName(t *testing.T) { + imageTag := strings.Split(environment.GetCrossplaneImage(), ":")[1] + manifests := filepath.Join("test/e2e/manifests/beta/diff", imageTag, "v2-nested-generatename") + setupPath := filepath.Join(manifests, "setup") + + environment.Test(t, + features.New("DiffExistingNestedResourceV2WithGenerateName"). + WithLabel(e2e.LabelArea, LabelAreaDiff). + WithLabel(e2e.LabelSize, e2e.LabelSizeSmall). + WithLabel(config.LabelTestSuite, config.TestSuiteDefault). + WithLabel(LabelCrossplaneVersion, CrossplaneVersionMain). + WithSetup("CreatePrerequisites", funcs.AllOf( + funcs.ApplyResources(e2e.FieldManager, setupPath, "*.yaml"), + funcs.ResourcesCreatedWithin(30*time.Second, setupPath, "*.yaml"), + )). + WithSetup("PrerequisitesAreReady", funcs.AllOf( + funcs.ResourcesHaveConditionWithin(1*time.Minute, setupPath, "parent-definition.yaml", apiextensionsv1.WatchingComposite()), + funcs.ResourcesHaveConditionWithin(1*time.Minute, setupPath, "child-definition.yaml", apiextensionsv1.WatchingComposite()), + )). + WithSetup("CreateExistingXR", funcs.AllOf( + funcs.ApplyResources(e2e.FieldManager, manifests, "existing-parent-xr.yaml"), + funcs.ResourcesCreatedWithin(1*time.Minute, manifests, "existing-parent-xr.yaml"), + )). + WithSetup("ExistingXRIsReady", funcs.AllOf( + funcs.ResourcesHaveConditionWithin(2*time.Minute, manifests, "existing-parent-xr.yaml", xpv1.Available()), + )). + Assess("CanDiffExistingNestedResourceWithGenerateName", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { + t.Helper() + + output, log, err := RunXRDiff(t, c, "./crossplane-diff", filepath.Join(manifests, "modified-parent-xr.yaml")) + if err != nil { + t.Fatalf("Error running diff command: %v\nLog output:\n%s", err, log) + } + + expectPath := filepath.Join(manifests, "expect") + assertDiffMatchesFile(t, output, filepath.Join(expectPath, "existing-parent-xr.ansi"), log) + + return ctx + }). + WithTeardown("DeleteResources", funcs.AllOf( + funcs.DeleteResources(manifests, "existing-parent-xr.yaml"), + funcs.ResourcesDeletedWithin(2*time.Minute, manifests, "existing-parent-xr.yaml"), + )). + WithTeardown("DeletePrerequisites", funcs.AllOf( + funcs.ResourcesDeletedAfterListedAreGone(3*time.Minute, setupPath, "*.yaml", nsNopList), + )). + Feature(), + ) +}