Skip to content

Conversation

@jcogilvie
Copy link
Collaborator

@jcogilvie jcogilvie commented Nov 15, 2025

Description of your changes

Problem

When diffing XRs with nested compositions (XRs that create other XRs as composed resources), the tool incorrectly treated nested XRs as entirely new resources. This caused all managed resources owned by nested XRs to appear as removed/re-added, showing hundreds of spurious changes instead of just the actual modifications.

Root Cause: Nested XRs weren't being matched to their existing cluster instances by the crossplane.io/composition-resource-name annotation, causing identity loss during diff calculation.

Solution

  1. Identity Preservation: Nested XRs are now correctly matched to existing cluster instances using their composition-resource-name annotation, preserving their cluster identity across renders
  2. Composite Label Handling: Fixed handling of crossplane.io/composite labels on nested resources - these labels always point to the root XR name (empirically verified behavior), not the immediate parent
  3. Namespace Filtering: Composition client now properly handles namespace filtering for cluster-scoped XRs when fetching by namespace
  4. Two-Phase Diff Calculation: Separated removal detection from non-removal diffs to ensure nested XRs are processed before identifying truly removed resources
  5. Test Normalization: Added normalization to E2E test golden files to strip ANSI codes and replace random generated names with placeholders, reducing test churn while maintaining colorization coverage in integration tests

Testing

  • Added comprehensive E2E test coverage for nested XR scenarios (generateName and explicit names)
  • Added test coverage for Claims with backing XRs

Fixes #144

I have:

  • Read and followed Crossplane's contribution process.
  • Run earthly +reviewable to ensure this PR is ready for review.
  • Added or updated unit tests.
  • Added or updated e2e tests.
    - [ ] Documented this change as needed.
    - [ ] Followed the API promotion workflow if this PR introduces, removes, or promotes an API.

Need help with this checklist? See the cheat sheet.

@jcogilvie jcogilvie force-pushed the fix-unexpected-claim-diff branch 2 times, most recently from c8d0531 to 3f15d31 Compare November 15, 2025 05:44
Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
@jcogilvie jcogilvie force-pushed the fix-unexpected-claim-diff branch from 5b85071 to 4ae6822 Compare November 18, 2025 01:52
@jcogilvie jcogilvie marked this pull request as ready for review November 18, 2025 01:58
Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
@jcogilvie jcogilvie marked this pull request as draft November 18, 2025 04:03
… and not the immediate parent

Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
…mespace filtering on cluster-scoped XRs

Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
@jcogilvie jcogilvie marked this pull request as ready for review November 18, 2025 20:33
Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
…rization in ITs

Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
@jcogilvie jcogilvie changed the title fix: nested XRs should not show add/remove under claims fix: preserve nested XR identity during diff Nov 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nested XR identity not preserved during diff

2 participants