refactor(claim): consume cli's ConvertClaimToXR for new-claim synthesis#350
Open
jcogilvie wants to merge 2 commits into
Open
refactor(claim): consume cli's ConvertClaimToXR for new-claim synthesis#350jcogilvie wants to merge 2 commits into
jcogilvie wants to merge 2 commits into
Conversation
Replaces our hand-rolled paving of the synthetic backing XR with a call to the upstream `ConvertClaimToXR` helper added to crossplane/cli. We still look up the XRD to pass the authoritative XR kind (XRDs aren't required to follow the "X"+claimKind convention) and pin Name to the claim's name to preserve clean diff output. The upstream helper additionally copies the claim's annotations and sets crossplane.io/claim-name / crossplane.io/claim-namespace labels on the synthesized XR, matching what real Crossplane produces. Existing e2e fixtures only render the claim and composed resources (not the dummy XR), so no fixture churn was needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
Picks up the auto-fixable issues that were already present in the tree plus the gci ordering caused by the new import in the prior commit: - gci: re-sort imports in comp.go, diff_processor.go, ref.go, ref_test.go - staticcheck (QF1002): switch to tagged switch in comp_processor.go and ref.go where the conditions were comparing a single value No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Jonathan Ogilvie <jonathan.ogilvie@sumologic.com>
jcogilvie
force-pushed
the
convert-claim-to-xr
branch
from
d7bc2fc to
79389e3
Compare
Contributor
There was a problem hiding this comment.
✅ Ready to approve
The refactor centralizes dummy XR synthesis on a tested upstream helper while preserving required behavior (authoritative XR kind + deterministic naming) and is exercised by existing unit/integration tests.
Note: this review does not count toward required approvals for merging.
Pull request overview
This PR refactors how crossplane-diff synthesizes a dummy backing XR for net-new Claims by delegating conversion to the upstream Crossplane CLI helper (ConvertClaimToXR), while still consulting the XRD to ensure the synthesized XR uses the authoritative kind.
Changes:
- Replace hand-rolled dummy XR creation in
synthesizeDummyBackingXRForNewClaimwithcrossplane/cli’sConvertClaimToXR, pinning the XR name to the Claim name and using the XRD-derived XR Kind. - Minor code tidy-ups (switch simplifications, import organization, formatting) in ref parsing/formatting and composition processor logic.
- Minor test formatting adjustments.
File summaries
| File | Description |
|---|---|
| cmd/diff/ref/ref.go | Minor refactor of Format and import organization in ref parsing/formatting utilities. |
| cmd/diff/ref/ref_test.go | Formatting-only adjustments to test table struct field alignment. |
| cmd/diff/diffprocessor/diff_processor.go | Refactors dummy backing XR synthesis to use upstream CLI ConvertClaimToXR with XRD-derived Kind and deterministic naming. |
| cmd/diff/diffprocessor/comp_processor.go | Minor switch-statement simplification in update policy partitioning. |
| cmd/diff/comp.go | Import organization tweak (no functional change). |
Copilot's findings
- Files reviewed: 4/5 changed files
- Comments generated: 0
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of your changes
Replaces our hand-rolled paving inside
synthesizeDummyBackingXRForNewClaimwith a call to the upstreamConvertClaimToXRhelper recently added tocrossplane/cli(cmd/crossplane/xr/generate.go).We still look up the XRD so we can pass the authoritative XR
Kind— XRDs may use names that don't follow the"X"+claimKindconvention. We also pinOptions.Nameto the claim's name so the synthesized XR keeps clean diff output (vs. upstream's default random suffix).Behavior change to call out: the upstream helper additionally copies the claim's annotations to the synthesized XR and sets
crossplane.io/claim-name/crossplane.io/claim-namespacelabels on it. Both match what real Crossplane produces. The existing new-claim e2e fixtures only render the claim and composed resources (not the dummy XR itself), so no golden-file churn was required.Coverage: the existing
TestDefaultDiffProcessor_synthesizeDummyBackingXRForNewClaimunit test and theNewClaimShowsDiff/NewClaimWithClaimRefCompositionenvtest integration tests exercise this path and continue to pass.Fixes #
I have:
earthly -P +reviewableto ensure this PR is ready for review.Added or updated e2e tests.Documented this change as needed.Followed the API promotion workflow if this PR introduces, removes, or promotes an API.🤖 Generated with Claude Code