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
2 changes: 2 additions & 0 deletions cmd/crossplane/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
"github.com/crossplane/cli/v2/cmd/crossplane/resource"
"github.com/crossplane/cli/v2/cmd/crossplane/version"
"github.com/crossplane/cli/v2/cmd/crossplane/xpkg"
"github.com/crossplane/cli/v2/cmd/crossplane/xr"
"github.com/crossplane/cli/v2/internal/config"
"github.com/crossplane/cli/v2/internal/maturity"
)
Expand Down Expand Up @@ -69,6 +70,7 @@ type cli struct {
Resource resource.Cmd `cmd:"" help:"Work with Crossplane resources." maturity:"beta"`
Version version.Cmd `cmd:"" help:"Print the client and server version information for the current context."`
XPKG xpkg.Cmd `cmd:"" help:"Work with Crossplane packages."`
XR xr.Cmd `cmd:"" help:"Work with Crossplane Composite Resources (XRs)." maturity:"alpha"`

// Hidden top-level alias for render, since it's GA but has moved.
Render renderxr.Cmd `cmd:"" help:"Render Crossplane compositions locally using functions." hidden:""`
Expand Down
281 changes: 281 additions & 0 deletions cmd/crossplane/xr/generate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,281 @@
/*
Copyright 2026 The Crossplane Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package xr

import (
"github.com/alecthomas/kong"
"github.com/spf13/afero"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/uuid"
"k8s.io/apiserver/pkg/storage/names"
"sigs.k8s.io/yaml"

"github.com/crossplane/crossplane-runtime/v2/pkg/errors"
"github.com/crossplane/crossplane-runtime/v2/pkg/fieldpath"
"github.com/crossplane/crossplane-runtime/v2/pkg/resource/unstructured/composite"

commonIO "github.com/crossplane/cli/v2/cmd/crossplane/convert/io"
)

type generateCmd struct {
// Arguments.
InputFile string `arg:"" default:"-" help:"The Claim YAML file to be converted. If not specified or '-', stdin will be used." optional:"" predictor:"file" type:"path"`

// 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"`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of, or maybe in addition to, this flag, it would be nice to allow an XRD to be specified, which would provide the necessary kind mapping.

Direct bool `help:"Create a direct XR without Claim references and suffix." name:"direct" negatable:""`
GenUID bool `help:"Set a fresh random metadata.uid on the generated XR." name:"gen-uid"`

fs afero.Fs
}

func (c *generateCmd) Help() string {
return `
Generate a Crossplane Composite Resource (XR) from a Claim YAML.

The command reads the Claim from a file (or stdin), produces the equivalent
XR (same spec, derived kind, optional claim reference), and writes the result
to stdout or to a file.

Examples:

# Generate an XR from claim.yaml and print it to stdout (kind = 'X' + Claim's kind).
crossplane xr generate claim.yaml

# Generate an XR from claim.yaml and write it to xr.yaml.
crossplane xr generate claim.yaml -o xr.yaml

# Generate an XR with an explicit name (overrides the default suffix or claim name).
crossplane xr generate claim.yaml --name my-xr

# Generate an XR with a specific kind.
crossplane xr generate claim.yaml --kind MyCompositeResource

# Generate a directly-linked XR (no Claim reference, no name suffix).
crossplane xr generate claim.yaml --direct

# Generate an XR with a fresh random metadata.uid.
crossplane xr generate claim.yaml --gen-uid

# Use in crossplane render
crossplane render <(crossplane xr generate claim.yaml) composition.yaml functions.yaml

# Read the Claim from stdin.
cat claim.yaml | crossplane xr generate -
`
}

// AfterApply implements kong.AfterApply.
func (c *generateCmd) AfterApply() error {
c.fs = afero.NewOsFs()
return nil
}

// Run runs the generate command.
func (c *generateCmd) Run(k *kong.Context) error {
claimData, err := commonIO.Read(c.fs, c.InputFile)
if err != nil {
return err
}

claim := &unstructured.Unstructured{}
if err := yaml.Unmarshal(claimData, claim); err != nil {
return errors.Wrap(err, "Unmarshalling Error")
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Improve error message specificity.

The error message "Unmarshalling Error" doesn't tell the user what went wrong with their Claim YAML. Consider making it more specific and helpful, such as "cannot parse Claim YAML" or "invalid Claim YAML format".

As per coding guidelines, CLI error messages must be especially user-friendly and avoid internal error details.

💬 Suggested improvement
-	if err := yaml.Unmarshal(claimData, claim); err != nil {
-		return errors.Wrap(err, "Unmarshalling Error")
-	}
+	if err := yaml.Unmarshal(claimData, claim); err != nil {
+		return errors.Wrap(err, "cannot parse Claim 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/xr/generate.go` at line 100, Replace the generic
errors.Wrap(err, "Unmarshalling Error") call with a user-friendly, specific CLI
message such as errors.Wrap(err, "cannot parse Claim YAML") or errors.Wrap(err,
"invalid Claim YAML format"); update the string in the existing errors.Wrap(err,
"...") expression so the returned error refers to the Claim YAML parsing problem
(keep the wrapped internal err for diagnostics but avoid exposing raw internal
details in the top-level message).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree a bit with coderabbit on adding a little detail here, but more importantly by convention error messages should start with a lower-case letter.

Suggested change
return errors.Wrap(err, "Unmarshalling Error")
return errors.Wrap(err, "cannot unmarshal claim")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adamwg, great point! The lowercase convention for error messages is indeed a standard Go idiom — the Go wiki's CodeReviewComments specifically states that error strings should not be capitalized. Your suggestion "cannot unmarshal claim" is both idiomatic and descriptive, making it a clear improvement over the original "Unmarshalling Error".


✏️ Learnings added
Learnt from: adamwg
Repo: crossplane/cli PR: 13
File: cmd/crossplane/xr/generate.go:100-100
Timestamp: 2026-05-20T21:52:31.054Z
Learning: In crossplane/cli Go code, error messages passed to errors.Wrap() and errors.New() must start with a lowercase letter, following standard Go conventions (per Go wiki CodeReviewComments). For example, use `errors.Wrap(err, "cannot unmarshal claim")` not `errors.Wrap(err, "Unmarshalling Error")`. This applies to all Go files in the repository.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: adamwg
Repo: crossplane/cli PR: 10
File: internal/project/build.go:361-364
Timestamp: 2026-05-12T22:54:33.936Z
Learning: When reviewing Go code targeting Go 1.26+ (per go.mod and/or CI build settings), do not flag `new(expr)` as a compile error. In Go 1.26, the built-in `new` accepts an expression (not only a type): `new(expr)` allocates a variable of the type of `expr`, initializes it to the value of `expr`, and returns a pointer to it (`*T`). This means usages like `new(someFunc())` or `new(someStringConstant)` are valid syntax and should be allowed under Go 1.26 or later.

Learnt from: adamwg
Repo: crossplane/cli PR: 10
File: internal/project/build.go:361-364
Timestamp: 2026-05-12T22:54:33.936Z
Learning: When reviewing Go code, allow the Go 1.26+ syntax `new(expr)` where `expr` is any expression (not just a type). In Go 1.26, `new(expr)` allocates a value of the type of `expr`, initializes it to the value of `expr`, and returns a `*T` pointer to it (e.g., `new(someFunc())`, `new(someStringConstant)`, `new(pkg.Const)`). If the repository targets Go 1.26 or later (e.g., module `go` directive >= 1.26 and/or build/CI uses Go >= 1.26), do not flag these usages as compilation errors.

Learnt from: adamwg
Repo: crossplane/cli PR: 10
File: cmd/crossplane/dependency/dependency.go:21-25
Timestamp: 2026-05-13T18:59:33.289Z
Learning: In crossplane/cli, experimental/beta CLI command gating is done via the kong command registration struct tag `maturity:"beta"` (or similar) placed on the *parent/top-level* command fields (e.g., top-level `Cmd` fields in `cmd/crossplane/main.go`). Subcommands registered under a parent are already considered gated by that parent maturity. During code review, do not request/introduce programmatic feature flags or add a wrapper function to conditionally register commands; treat the `maturity` tag as the correct and sufficient mechanism. Instead, focus review attention on whether the appropriate maturity tag is set on the relevant parent command.

}

// Convert to XR
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")
}

b, err := yaml.Marshal(xr)
if err != nil {
return errors.Wrap(err, "Unable to marshal back to yaml")
}

data := append([]byte("---\n"), b...)

if c.OutputFile != "" {
if err := afero.WriteFile(c.fs, c.OutputFile, data, 0o644); err != nil {
return errors.Wrapf(err, "cannot write output file %q", c.OutputFile)
}

return nil
}

if _, err := k.Stdout.Write(data); err != nil {
return errors.Wrap(err, "cannot write output")
}

return nil
}

const (
// Error messages.
Comment on lines +136 to +137
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Crossplane style guidelines no longer suggest error message constants, preferring literals in errors.Wrap calls.

errNilInput = "input is nil"
errEmptyClaimYAML = "invalid Claim YAML: parsed object is empty"
errNoAPIVersion = "Claim has no apiVersion"
errParseAPIVersion = "failed to parse Claim APIVersion"
errNoKind = "Claim has no kind section"
errNoSpecSection = "Claim has no spec section"
Comment on lines +136 to +143
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make error messages more user-friendly and actionable.

The error message constants are developer-focused (e.g., "Claim has no apiVersion") rather than user-friendly. They don't provide context about what the user was trying to do or suggest next steps.

Consider messages like:

  • errNoAPIVersion: "The Claim YAML must include an 'apiVersion' field. Please check your Claim definition."
  • errNoKind: "The Claim YAML must include a 'kind' field. Please check your Claim definition."
  • errNoSpecSection: "The Claim YAML must include a 'spec' section. Please check your Claim definition."

As per coding guidelines, error messages must be meaningful to end users, not just developers—avoid technical jargon, include context about what the user was trying to do, and suggest next steps when possible. CLI error messages must be especially user-friendly.

🤖 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/xr/generate.go` around lines 136 - 143, Update the user-facing
error message constants (errNoAPIVersion, errNoKind, errNoSpecSection and
optionally errEmptyClaimYAML and errNilInput) to be user-friendly and
actionable: replace terse developer phrases with full-sentence guidance that
names the missing field and suggests a next step (for example: indicate the
Claim YAML must include an apiVersion/kind/spec and ask the user to check their
Claim definition or validate their YAML). Locate these constants by name in the
file (errNoAPIVersion, errNoKind, errNoSpecSection, errEmptyClaimYAML,
errNilInput) and update their string values accordingly so CLI output gives
clear, helpful instructions to the user.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree with coderabbit here - its suggested error messages are excessive 😆.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adamwg Fair enough! 😄 The concise style keeps the messages clean without being overly prescriptive. I'll note this preference for future reviews.


✏️ Learnings added
Learnt from: adamwg
Repo: crossplane/cli PR: 13
File: cmd/crossplane/xr/generate.go:136-143
Timestamp: 2026-05-20T21:52:28.707Z
Learning: In crossplane/cli, concise error messages (e.g., "Claim has no apiVersion", "Claim has no kind section") are preferred over verbose, instructional messages that spell out suggested next steps. Do not flag these short-form error strings as insufficiently user-friendly in code review.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: adamwg
Repo: crossplane/cli PR: 10
File: internal/project/build.go:361-364
Timestamp: 2026-05-12T22:54:33.936Z
Learning: When reviewing Go code targeting Go 1.26+ (per go.mod and/or CI build settings), do not flag `new(expr)` as a compile error. In Go 1.26, the built-in `new` accepts an expression (not only a type): `new(expr)` allocates a variable of the type of `expr`, initializes it to the value of `expr`, and returns a pointer to it (`*T`). This means usages like `new(someFunc())` or `new(someStringConstant)` are valid syntax and should be allowed under Go 1.26 or later.

Learnt from: adamwg
Repo: crossplane/cli PR: 10
File: internal/project/build.go:361-364
Timestamp: 2026-05-12T22:54:33.936Z
Learning: When reviewing Go code, allow the Go 1.26+ syntax `new(expr)` where `expr` is any expression (not just a type). In Go 1.26, `new(expr)` allocates a value of the type of `expr`, initializes it to the value of `expr`, and returns a `*T` pointer to it (e.g., `new(someFunc())`, `new(someStringConstant)`, `new(pkg.Const)`). If the repository targets Go 1.26 or later (e.g., module `go` directive >= 1.26 and/or build/CI uses Go >= 1.26), do not flag these usages as compilation errors.

Learnt from: adamwg
Repo: crossplane/cli PR: 10
File: cmd/crossplane/dependency/dependency.go:21-25
Timestamp: 2026-05-13T18:59:33.289Z
Learning: In crossplane/cli, experimental/beta CLI command gating is done via the kong command registration struct tag `maturity:"beta"` (or similar) placed on the *parent/top-level* command fields (e.g., top-level `Cmd` fields in `cmd/crossplane/main.go`). Subcommands registered under a parent are already considered gated by that parent maturity. During code review, do not request/introduce programmatic feature flags or add a wrapper function to conditionally register commands; treat the `maturity` tag as the correct and sufficient mechanism. Instead, focus review attention on whether the appropriate maturity tag is set on the relevant parent command.


// Label keys.
labelClaimName = "crossplane.io/claim-name"
labelClaimNamespace = "crossplane.io/claim-namespace"
labelComposite = "crossplane.io/composite"
)

// Options configures ConvertClaimToXR.
type Options struct {
// Name is the XR name. Empty falls back to:
// - claim.Name when Direct is true
// - claim.Name with a random suffix when Direct is false
// A non-empty Name overrides both fallbacks.
Name string

// Kind is the XR kind. Empty defaults to "X" + claim.Kind.
Kind string

// Direct controls XR linkage to the claim:
// - true: no spec.claimRef; no claim-name/claim-namespace labels
// - false: spec.claimRef is set; claim-name/claim-namespace labels added
Direct bool

// GenerateUID, when true, sets metadata.uid to a fresh random UUID.
GenerateUID bool
}

// ConvertClaimToXR converts a Crossplane Claim to a Composite Resource (XR).
func ConvertClaimToXR(claim *unstructured.Unstructured, opts Options) (*composite.Unstructured, error) {
Comment on lines +171 to +172
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mostly duplicates functionality from Crossplane's claim reconciler. Could we factor some code out of the reconciler into crossplane-runtime and consume it here? Similar to how the xcrd package handles XRD->CRD conversion for both the CLI and the XRD reconciler.

Not against having this in the CLI for now - we can get this merged and refactor afterwards.

if claim == nil {
return nil, errors.New(errNilInput)
}

if claim.Object == nil {
return nil, errors.New(errEmptyClaimYAML)
}

// Get Claim's properties
claimName := claim.GetName()

claimKind := claim.GetKind()
if claimKind == "" {
return nil, errors.New(errNoKind)
}

apiVersion := claim.GetAPIVersion()
if apiVersion == "" {
return nil, errors.New(errNoAPIVersion)
}

if _, err := schema.ParseGroupVersion(apiVersion); err != nil {
return nil, errors.Wrap(err, errParseAPIVersion)
}

annotations := claim.GetAnnotations()

labels := claim.GetLabels()
if labels == nil {
labels = make(map[string]string)
}

claimSpec, ok := claim.Object["spec"].(map[string]any)
if !ok || claimSpec == nil {
return nil, errors.New(errNoSpecSection)
}

// Create a new XR and pave it for manipulation
xr := composite.New()

xrPaved, err := fieldpath.PaveObject(xr)
if err != nil {
return nil, errors.Wrap(err, "failed to pave object")
}

if err := xrPaved.SetString("apiVersion", apiVersion); err != nil {
return nil, errors.Wrap(err, "failed to set apiVersion")
}

// Set XR kind - either from opts or by prepending X to Claim's kind
kind := opts.Kind
if kind == "" {
kind = "X" + claimKind
}

if err := xrPaved.SetString("kind", kind); err != nil {
return nil, errors.Wrap(err, "failed to set kind")
}

if len(annotations) > 0 {
if err := xrPaved.SetValue("metadata.annotations", annotations); err != nil {
return nil, errors.Wrap(err, "failed to set annotations")
}
}

if err := xrPaved.SetValue("spec", claimSpec); err != nil {
return nil, errors.Wrap(err, "failed to set spec")
}

xrName := claimName

if !opts.Direct {
xrName = names.SimpleNameGenerator.GenerateName(claimName + "-")
labels[labelClaimName] = claim.GetName()

labels[labelClaimNamespace] = claim.GetNamespace()
if err := xrPaved.SetValue("spec.claimRef", map[string]any{
"apiVersion": apiVersion,
"kind": claimKind,
"name": claimName,
"namespace": claim.GetNamespace(),
}); err != nil {
return nil, errors.Wrap(err, "failed to set claimRef")
}
}
Comment on lines +244 to +257
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Validate namespace in non-Direct mode.

In non-Direct mode, the code uses claim.GetNamespace() (lines 246, 248, 253) without validating that the namespace is non-empty. Claims are namespace-scoped resources, but if a Claim is created without a namespace (or read from a YAML without one), this could result in an XR with empty namespace in spec.claimRef and labels, which would be semantically incorrect.

Should we validate that the namespace is present when not in Direct mode? Or document the expected behavior when namespace is empty?

🛡️ Suggested validation
 	if !opts.Direct {
+		claimNS := claim.GetNamespace()
+		if claimNS == "" {
+			return nil, errors.New("Claim must have a namespace when generating a non-direct XR")
+		}
 		xrName = names.SimpleNameGenerator.GenerateName(claimName + "-")
 		labels[labelClaimName] = claim.GetName()
 
-		labels[labelClaimNamespace] = claim.GetNamespace()
+		labels[labelClaimNamespace] = claimNS
 		if err := xrPaved.SetValue("spec.claimRef", map[string]any{
 			"apiVersion": apiVersion,
 			"kind":       claimKind,
 			"name":       claimName,
-			"namespace":  claim.GetNamespace(),
+			"namespace":  claimNS,
 		}); err != nil {
 			return nil, errors.Wrap(err, "failed to set claimRef")
 		}
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if !opts.Direct {
xrName = names.SimpleNameGenerator.GenerateName(claimName + "-")
labels[labelClaimName] = claim.GetName()
labels[labelClaimNamespace] = claim.GetNamespace()
if err := xrPaved.SetValue("spec.claimRef", map[string]any{
"apiVersion": apiVersion,
"kind": claimKind,
"name": claimName,
"namespace": claim.GetNamespace(),
}); err != nil {
return nil, errors.Wrap(err, "failed to set claimRef")
}
}
if !opts.Direct {
claimNS := claim.GetNamespace()
if claimNS == "" {
return nil, errors.New("Claim must have a namespace when generating a non-direct XR")
}
xrName = names.SimpleNameGenerator.GenerateName(claimName + "-")
labels[labelClaimName] = claim.GetName()
labels[labelClaimNamespace] = claimNS
if err := xrPaved.SetValue("spec.claimRef", map[string]any{
"apiVersion": apiVersion,
"kind": claimKind,
"name": claimName,
"namespace": claimNS,
}); err != nil {
return nil, errors.Wrap(err, "failed to set claimRef")
}
}
🤖 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/xr/generate.go` around lines 244 - 257, When building the XR
in non-Direct mode (the block guarded by if !opts.Direct), validate that
claim.GetNamespace() is non-empty before setting labels[labelClaimNamespace] or
calling xrPaved.SetValue("spec.claimRef", ...); if the namespace is empty return
a wrapped error (e.g. "claim namespace required in non-Direct mode") so we don't
create an XR with an empty claimRef namespace; perform this check early in the
if !opts.Direct block (before using claim.GetNamespace()) and only proceed to
set labels and call xrPaved.SetValue when the namespace is present.


// Explicit Name overrides both Direct's claim-name default and the generated suffix.
if opts.Name != "" {
xrName = opts.Name
}

if err := xrPaved.SetString("metadata.name", xrName); err != nil {
return nil, errors.Wrap(err, "failed to set name")
}

if len(labels) > 0 {
delete(labels, labelComposite)

if err := xrPaved.SetValue("metadata.labels", labels); err != nil {
return nil, errors.Wrap(err, "failed to set labels")
}
}

if opts.GenerateUID {
xr.SetUID(uuid.NewUUID())
}

return xr, nil
}
Loading
Loading