feat(claimtoxr): Add new subcommand resource convert claim-to-xr#8
Closed
tampakrap wants to merge 27 commits into
Closed
feat(claimtoxr): Add new subcommand resource convert claim-to-xr#8tampakrap wants to merge 27 commits into
resource convert claim-to-xr#8tampakrap wants to merge 27 commits into
Conversation
This commit adds build infrastructure for the standalone CLI repository, with a stub cmd/crossplane. We'll add the existing crank code from crossplane in a subsequent commit. Signed-off-by: Adam Wolfe Gordon <awg@upbound.io>
Signed-off-by: Adam Wolfe Gordon <awg@upbound.io>
This is all copied from crossplane/crossplane and updated to remove the parts we don't need (e.g., pushing container images). Signed-off-by: Adam Wolfe Gordon <awg@upbound.io>
This commit contains the current `cmd/crank` from c/c and the supporting `internal/docker` package, with imports updated as necessary. Signed-off-by: Adam Wolfe Gordon <awg@upbound.io>
The CLI's version will not be guaranteed to match a Crossplane version going forward, so we can't use the CLI's version number as the default image tag for render. Use the `stable` tag instead, so that we always use the latest stable Crossplane. Signed-off-by: Adam Wolfe Gordon <awg@upbound.io>
We'll fill out more content in the README later, but bootstrap it for now. Signed-off-by: Adam Wolfe Gordon <awg@upbound.io>
Signed-off-by: Adam Wolfe Gordon <awg@upbound.io>
Signed-off-by: Adam Wolfe Gordon <awg@upbound.io>
We don't actually have any fuzz tests yet, and this job relies on some configuration existing in the google oss-fuzz repository. Signed-off-by: Adam Wolfe Gordon <awg@upbound.io>
ci: Remove the fuzz test job
Copy Crossplane's coderabbit config as a starting point and remove irrelevant parts. Signed-off-by: Adam Wolfe Gordon <awg@upbound.io>
Historically, we've introduced new commands in the `crossplane alpha` or `crossplane beta` trees and moved them as they matured. This makes it awkward to introduce new commands in existing trees, since the tree gets split across maturity levels, and also breaks users when commands mature since their invocation changes. Move all the existing alpha and beta commands to the top level (or into the right tree, in the case of `crossplane render op`). Start indicating maturity using kong tags. Alpha and beta features are hidden from help by default but can be enabled by configuration and have a maturity indicator added to their help automatically. Make `crossplane render xr` the default subcommand for `crossplane render` so that existing render users are not broken by this change. Fixes crossplane/crossplane#7309 Signed-off-by: Adam Wolfe Gordon <awg@upbound.io>
Now that we have a config file, we need commands to manage it. Add basic `crossplane config view` and `crossplane config set` commands so we can view and update the config. Signed-off-by: Adam Wolfe Gordon <awg@upbound.io>
This repository has additional maintainers beyond the @crossplane/crossplane-maintainers group. Signed-off-by: Adam Wolfe Gordon <awg@upbound.io>
The protos in this repository are vendored from crossplane/crossplane, so no need to lint them or push them to the Buf schema registry. This avoids the need for a `BUF_TOKEN` secret in this repo. Signed-off-by: Adam Wolfe Gordon <awg@upbound.io>
Add coderabbit config and clean up configuration
As proposed in the DevEx design doc, reorganize our command tree so that it's noun-centric. Leave `crossplane render` as an alias for `crossplane xr render` since that command is GA and we don't want to break any users/scripts. Allow alpha and beta commands to move without aliases. Signed-off-by: Adam Wolfe Gordon <awg@upbound.io>
Show only the top-level nouns in the help. The user can use the `--help` flag on any of these commands to see the subcommands. This will help keep our help tidy and readable as we expand functionality. Signed-off-by: Adam Wolfe Gordon <awg@upbound.io>
In keeping with how Crossplane handles pre-GA features, enable beta features by default (and have a config option to disable them), but leave alpha features disabled by default (with a config option to enable). Signed-off-by: Adam Wolfe Gordon <awg@upbound.io>
Signed-off-by: Adam Wolfe Gordon <awg@upbound.io>
Reorganize the command tree and introduce config
Export the entire nixpkgs-unstable rather than just its go package so that our nix setup can use arbitrary packages from unstable as needed. Matches the change in crossplane/crossplane commit 744190edf. Signed-off-by: Adam Wolfe Gordon <awg@upbound.io>
Upcoming developer experience changes require golang 1.26. Do the update separately and resolve new lint issues so that we don't mix the changes into the bigger DevEx PR. Signed-off-by: Adam Wolfe Gordon <awg@upbound.io>
We use the function protobufs in the function we implement for injecting and extracting context during render. When this code lived in crossplane/crossplane it made sense to use the protobufs from there, but now that we're separate it would be better to use the versions in function-sdk-go like a normal function. This removes our dependency on crossplane/crossplane, which is nice. Signed-off-by: Adam Wolfe Gordon <awg@upbound.io>
Update go to 1.26
Use function protobufs from function-sdk-go
Introduce a Claim to XR converter: `crossplane resource convert claim-to-xr`. Similarly to other converter functions/tools, it takes a Claim either on stdin or as file argument and generates an equivalent XR YAML. The parent `crossplane resource convert` is marked as maturity "alpha", which applies to the subtree as well. This new tool can be used together with the render command as shown in the example below: ```bash crossplane render <(crossplane resource convert claim-to-xr claim.yaml) composition.yaml functions.yaml ``` The library function ConvertClaimToXR can be used by downstream tools (eg crossplane-diff) that rely on Claim to XR conversion. The tool (and as a result the converter function, via the Options struct) is configurable and provides the following flags: - `--direct`: omit spec.claimRef and the equivalent labels. - `--name`: specify an explicit XR name, skipping either the random suffix in the non-direct mode or the Claim name fallback in direct mode. - `--kind`: override the derived "X<Kind>" default. - `--gen-uid`: populate metadata.uid with a fresh random UUID Signed-off-by: Theo Chatzimichos <tampakrap@gmail.com>
📝 WalkthroughWalkthroughThis PR adds a new ChangesClaim-to-XR Conversion Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
cmd/crossplane/resource/convert/claimtoxr/cmd.go (1)
111-113: ⚡ Quick winConsider adding more context to the conversion error.
While this error message is functional, it could be more helpful by including what kind of resource conversion was attempted. This helps users understand the context when troubleshooting.
💬 Suggested improvement
xr, err := ConvertClaimToXR(claim, Options{ Name: c.Name, Kind: c.Kind, Direct: c.Direct, GenerateUID: c.GenUID, }) if err != nil { - return errors.Wrap(err, "failed to convert Claim to XR") + return errors.Wrapf(err, "failed to convert Claim %s/%s to Composite Resource", claim.GetNamespace(), claim.GetName()) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/crossplane/resource/convert/claimtoxr/cmd.go` around lines 111 - 113, Update the error wrapping around errors.Wrap(err, "failed to convert Claim to XR") to include concrete resource context (e.g., the Claim's kind, name and namespace or the target XR kind) so logs show what conversion failed; locate the error call (errors.Wrap) in the Claim-to-XR conversion function and augment the message to include identifiers (for example using claim.GetName(), claim.GetNamespace(), claim.GetKind() or the target XR type variable) so the returned wrapped error reads something like "failed to convert Claim <kind>/<namespace>/<name> to XR <targetKind>".cmd/crossplane/resource/convert/claimtoxr/converter.go (1)
136-156: 💤 Low valueConsider validating name length constraints.
Based on the external context, Crossplane has constraints on XR names (claim names cannot be longer than 57 characters to allow for the suffix). Should the conversion function validate this to provide a better error message, or is it acceptable to let Kubernetes validation catch this later?
This is more of a UX question - failing early with a clear message vs. letting it fail during apply. What's the team's preference?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/crossplane/resource/convert/claimtoxr/converter.go` around lines 136 - 156, Validate XR name length before generation and before accepting an explicit name: when not opts.Direct (i.e., you will call names.SimpleNameGenerator.GenerateName using claimName+"-") check that len(claimName) <= 57 and return a clear error from the converter function if it’s too long; also validate opts.Name when provided (the explicit override) to ensure it meets Kubernetes resource name limits (e.g., <= 63 chars) and return a descriptive error if it exceeds the limit. Update the logic around xrName, opts.Direct, and opts.Name (and any callers that set labels[labelClaimName]/labelClaimNamespace or call xrPaved.SetValue) to perform these checks before generating or assigning xrName so the function fails early with a helpful message.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/crossplane/resource/convert/claimtoxr/cmd.go`:
- Around line 100-102: The YAML unmarshal error uses a technical message
("Unmarshalling Error"); update the error returned when calling
yaml.Unmarshal(claimData, claim) to a user-facing message such as "failed to
parse Claim YAML: the input file does not contain valid YAML; please check the
file format and syntax" and wrap the original error for internal debugging
(e.g., annotate it) so callers get actionable guidance while preserving the
underlying error details from yaml.Unmarshal for logs/troubleshooting.
---
Nitpick comments:
In `@cmd/crossplane/resource/convert/claimtoxr/cmd.go`:
- Around line 111-113: Update the error wrapping around errors.Wrap(err, "failed
to convert Claim to XR") to include concrete resource context (e.g., the Claim's
kind, name and namespace or the target XR kind) so logs show what conversion
failed; locate the error call (errors.Wrap) in the Claim-to-XR conversion
function and augment the message to include identifiers (for example using
claim.GetName(), claim.GetNamespace(), claim.GetKind() or the target XR type
variable) so the returned wrapped error reads something like "failed to convert
Claim <kind>/<namespace>/<name> to XR <targetKind>".
In `@cmd/crossplane/resource/convert/claimtoxr/converter.go`:
- Around line 136-156: Validate XR name length before generation and before
accepting an explicit name: when not opts.Direct (i.e., you will call
names.SimpleNameGenerator.GenerateName using claimName+"-") check that
len(claimName) <= 57 and return a clear error from the converter function if
it’s too long; also validate opts.Name when provided (the explicit override) to
ensure it meets Kubernetes resource name limits (e.g., <= 63 chars) and return a
descriptive error if it exceeds the limit. Update the logic around xrName,
opts.Direct, and opts.Name (and any callers that set
labels[labelClaimName]/labelClaimNamespace or call xrPaved.SetValue) to perform
these checks before generating or assigning xrName so the function fails early
with a helpful message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2363730b-0695-45f3-8c36-c05a53e9a7cd
📒 Files selected for processing (5)
cmd/crossplane/resource/convert/claimtoxr/cmd.gocmd/crossplane/resource/convert/claimtoxr/converter.gocmd/crossplane/resource/convert/claimtoxr/converter_test.gocmd/crossplane/resource/convert/convert.gocmd/crossplane/resource/resource.go
Comment on lines
+100
to
+102
| if err := yaml.Unmarshal(claimData, claim); err != nil { | ||
| return errors.Wrap(err, "Unmarshalling Error") | ||
| } |
There was a problem hiding this comment.
Improve error message to be more user-friendly.
The error message "Unmarshalling Error" uses technical jargon that may not be meaningful to end users. CLI error messages should explain what went wrong from the user's perspective and ideally suggest next steps. As per coding guidelines, CLI error messages must avoid internal error details and provide actionable guidance.
Consider a message like:
"failed to parse Claim YAML: the input file does not contain valid YAML. Please check the file format and syntax"
💬 Suggested improvement
claim := &unstructured.Unstructured{}
if err := yaml.Unmarshal(claimData, claim); err != nil {
- return errors.Wrap(err, "Unmarshalling Error")
+ return errors.Wrap(err, "failed to parse Claim YAML: the input file does not contain valid YAML")
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/crossplane/resource/convert/claimtoxr/cmd.go` around lines 100 - 102, The
YAML unmarshal error uses a technical message ("Unmarshalling Error"); update
the error returned when calling yaml.Unmarshal(claimData, claim) to a
user-facing message such as "failed to parse Claim YAML: the input file does not
contain valid YAML; please check the file format and syntax" and wrap the
original error for internal debugging (e.g., annotate it) so callers get
actionable guidance while preserving the underlying error details from
yaml.Unmarshal for logs/troubleshooting.
Collaborator
Author
|
I have created a branch for (I am using |
Collaborator
Author
|
I chose |
haarchri
reviewed
May 11, 2026
| // Flags. | ||
| OutputFile string `help:"The file to write the generated XR YAML to. If not specified, stdout will be used." placeholder:"PATH" predictor:"file" short:"o" type:"path"` | ||
| Name string `help:"The name to use for the XR. If empty, defaults to the Claim's name (direct mode) or the Claim's name with a random suffix (non-direct)." placeholder:"NAME" type:"string"` | ||
| Kind string `help:"The kind to use for the XR. If not specified, 'X' will be prepended to the Claim's kind (e.g. Infra -> XInfra)." placeholder:"KIND" type:"string"` |
Member
There was a problem hiding this comment.
for me it feels safer to have the corresponding XRD as input - so i don't need to guess the Kind
| output = f | ||
| } | ||
|
|
||
| outputW := bufio.NewWriter(output) |
| } | ||
|
|
||
| // ConvertClaimToXR converts a Crossplane Claim to a Composite Resource (XR). | ||
| func ConvertClaimToXR(claim *unstructured.Unstructured, opts Options) (*composite.Unstructured, error) { |
Member
There was a problem hiding this comment.
do we need this to be exported ?, yes if we would switch it to internal folder - and thats what i would do
Collaborator
There was a problem hiding this comment.
I think we want this exposed as an API so downstream folks (e.g. diff) can use it w/o having to invoke the CLI. (See the example)
|
|
||
| // Cmd contains commands for working with Crossplane resources. | ||
| type Cmd struct { | ||
| Convert convert.Cmd `cmd:"" help:"Convert a Crossplane resource to a different kind." maturity:"alpha"` |
Member
There was a problem hiding this comment.
Suggested change
| Convert convert.Cmd `cmd:"" help:"Convert a Crossplane resource to a different kind." maturity:"alpha"` | |
| Convert convert.Cmd `cmd:"" help:"Convert a Crossplane resource to a different kind (e.g. Claim to XR)." maturity:"alpha"` |
| * Claim -> Composite Resource (XR) | ||
|
|
||
| Examples: | ||
| # Convert a Claim YAML file to an XR YAML file. |
Member
There was a problem hiding this comment.
can we add a real example for what this command is helpful?
# Render a Composition using a converted Claim as the XR input.
crossplane render <(crossplane resource convert claim-to-xr claim.yaml) composition.yaml functions.yaml
| // Set XR kind - either from opts or by prepending X to Claim's kind | ||
| kind := opts.Kind | ||
| if kind == "" { | ||
| kind = "X" + claimKind |
Member
There was a problem hiding this comment.
lets check my comment above regarding XRD input
Member
|
@tampakrap regarding your comment:
i would lean toward grouping these under xr as the noun:
Wdyt? |
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
4 participants
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
Introduce a Claim to XR converter:
crossplane resource convert claim-to-xr. Similarly to other converter functions/tools, it takes a Claim either on stdin or as file argument and generates an equivalent XR YAML.The parent
crossplane resource convertis marked as maturity "alpha", which applies to the subtree as well.This new tool can be used together with the render command as shown in the example below:
crossplane render <(crossplane resource convert claim-to-xr claim.yaml) composition.yaml functions.yamlThe library function ConvertClaimToXR can be used by downstream tools (eg crossplane-diff) that rely on Claim to XR conversion.
The tool (and as a result the converter function, via the Options struct) is configurable and provides the following flags:
--direct: omit spec.claimRef and the equivalent labels.--name: specify an explicit XR name, skipping either the random suffix in the non-direct mode or the Claim name fallback in direct mode.--kind: override the derived "X" default.--gen-uid: populate metadata.uid with a fresh random UUIDI didn't create new docs issue/PR, we can use crossplane/docs#1088
I have:
./nix.sh flake checkto ensure this PR is ready for review.Linked a PR or a docs tracking issue to document this change.Addedbackport release-x.ylabels to auto-backport this PR.Need help with this checklist? See the cheat sheet.