Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 3 additions & 1 deletion Earthfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
29 changes: 20 additions & 9 deletions cmd/diff/client/crossplane/composition_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}

Expand All @@ -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
}

Expand All @@ -707,13 +715,15 @@ func (c *DefaultCompositionClient) FindCompositesUsingComposition(ctx context.Co
c.logger.Debug("Error extracting claim type from XRD",
"xrd", xrd.GetName(),
"error", err)

return matchingResources, nil
}

// If XRD doesn't define claims, just return the XRs
if claimGVK.Empty() {
c.logger.Debug("XRD does not define claims",
"xrd", xrd.GetName())

return matchingResources, nil
}

Expand All @@ -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
}

Expand Down
2 changes: 2 additions & 0 deletions cmd/diff/client/crossplane/composition_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
48 changes: 20 additions & 28 deletions cmd/diff/diff_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -2770,7 +2774,7 @@ Summary: 2 modified
spec:
compositeTypeRef:
apiVersion: diff.example.org/v1alpha1
kind: XNopClaim
kind: XNop
mode: Pipeline
pipeline:
- functionRef:
Expand Down Expand Up @@ -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
Expand All @@ -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




---

Expand Down
Loading
Loading