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
7 changes: 7 additions & 0 deletions cmd/crossplane/config/help/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,10 @@ Enable alpha commands:
```shell
crossplane config set features.enableAlpha true
```

Generate GetX/SetX accessor methods on generated Go models (off by default), so
generated resources can be used through interfaces and generics:

```shell
crossplane config set features.generateGoModelAccessors true
```
5 changes: 3 additions & 2 deletions cmd/crossplane/config/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ type boolSetter func(c *config.Config, v bool)
//
//nolint:gochecknoglobals // This is a constant.
var boolKeys = map[string]boolSetter{
"features.enableAlpha": func(c *config.Config, v bool) { c.Features.EnableAlpha = v },
"features.disableBeta": func(c *config.Config, v bool) { c.Features.DisableBeta = v },
"features.enableAlpha": func(c *config.Config, v bool) { c.Features.EnableAlpha = v },
"features.disableBeta": func(c *config.Config, v bool) { c.Features.DisableBeta = v },
"features.generateGoModelAccessors": func(c *config.Config, v bool) { c.Features.GenerateGoModelAccessors = v },
}

func (c *setCmd) AfterApply() error {
Expand Down
5 changes: 3 additions & 2 deletions cmd/crossplane/function/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
"github.com/crossplane/crossplane-runtime/v2/pkg/xpkg"

v1alpha1 "github.com/crossplane/cli/v2/apis/dev/v1alpha1"
"github.com/crossplane/cli/v2/internal/config"
"github.com/crossplane/cli/v2/internal/filesystem"
"github.com/crossplane/cli/v2/internal/kcl"
"github.com/crossplane/cli/v2/internal/project/projectfile"
Expand Down Expand Up @@ -144,7 +145,7 @@ func functionSchemaLanguage(functionLang string) string {
}

// Run generates a function scaffold.
func (c *generateCmd) Run(sp terminal.SpinnerPrinter) error {
func (c *generateCmd) Run(sp terminal.SpinnerPrinter, cfg *config.Config) error {
if err := c.validatePaths(); err != nil {
return err
}
Expand All @@ -156,7 +157,7 @@ func (c *generateCmd) Run(sp terminal.SpinnerPrinter) error {
}
schemaMgr := manager.New(
c.schemasFS,
generator.Filter(generator.AllLanguages(), c.proj.Spec.Schemas.GetLanguages()),
generator.Filter(generator.AllLanguages(generator.WithGoModelAccessors(cfg.Features.GenerateGoModelAccessors)), c.proj.Spec.Schemas.GetLanguages()),
runner.NewRealSchemaRunner(runner.WithImageConfig(c.proj.Spec.ImageConfigs)),
)

Expand Down
3 changes: 2 additions & 1 deletion cmd/crossplane/function/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
apiextv1 "github.com/crossplane/crossplane/apis/v2/apiextensions/v1"

v1alpha1 "github.com/crossplane/cli/v2/apis/dev/v1alpha1"
"github.com/crossplane/cli/v2/internal/config"
"github.com/crossplane/cli/v2/internal/terminal"
)

Expand Down Expand Up @@ -324,7 +325,7 @@ func TestRunErrors(t *testing.T) {
case "afterApply":
err = c.AfterApply()
case "run":
err = c.Run(terminal.NewSpinnerPrinter(io.Discard, false))
err = c.Run(terminal.NewSpinnerPrinter(io.Discard, false), &config.Config{})
}
if err == nil {
t.Fatalf("expected error containing %q, got nil", tc.wantErrSubstring)
Expand Down
2 changes: 2 additions & 0 deletions cmd/crossplane/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ func main() {
// at runtime.
kong.BindTo(logger, (*logging.Logger)(nil)),
kong.BindTo(configcmd.ConfigPath(cfgPath), (*configcmd.ConfigPath)(nil)),
// Bind the loaded config so commands can read feature flags at runtime.
kong.Bind(cfg),
Comment on lines +129 to +130

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all Run methods that take *config.Config and any direct (non-kong) callers in tests.
rg -nP --type=go '\)\s*Run\([^)]*\*config\.Config' -C1
echo '--- direct Run callers in tests ---'
rg -nP --type=go '\.Run\(' -g '**/*_test.go' -C2

Repository: crossplane/cli

Length of output: 152


Binding cfg is correct, but adding a nil guard prevents future panics

The search confirms build, run, and generate rely on the kong.Bind(cfg) at line 130, and no direct .Run() calls were found in tests, so the current path is safe.

However, to make these commands robust against future programmatic invocation or test changes where Kong might not inject cfg:

  1. Add a simple nil check at the start of Run methods in cmd/crossplane/project/build.go, cmd/crossplane/project/run.go, and cmd/crossplane/function/generate.go.
  2. Return an error if cfg is nil to avoid a panic on cfg.Features access.

This defensive pattern ensures the command fails gracefully with a clear message rather than crashing silently if called outside the Kong pipeline.

🤖 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/main.go` around lines 129 - 130, The `build`, `run`, and
`generate` command entrypoints currently assume `cfg` is always injected by
Kong, which could panic if they are invoked programmatically or in tests; add a
nil check at the start of each `Run` method in
`cmd/crossplane/project/build.go`, `cmd/crossplane/project/run.go`, and
`cmd/crossplane/function/generate.go`, and return a clear error before any
`cfg.Features` access when `cfg` is nil.

kong.Help(helpPrinter),
kong.UsageOnError())

Expand Down
5 changes: 3 additions & 2 deletions cmd/crossplane/project/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (

devv1alpha1 "github.com/crossplane/cli/v2/apis/dev/v1alpha1"
"github.com/crossplane/cli/v2/internal/async"
"github.com/crossplane/cli/v2/internal/config"
"github.com/crossplane/cli/v2/internal/dependency"
"github.com/crossplane/cli/v2/internal/project"
"github.com/crossplane/cli/v2/internal/project/functions"
Expand Down Expand Up @@ -83,7 +84,7 @@ func (c *buildCmd) AfterApply() error {
}

// Run executes the build command.
func (c *buildCmd) Run(logger logging.Logger, sp terminal.SpinnerPrinter) error {
func (c *buildCmd) Run(logger logging.Logger, sp terminal.SpinnerPrinter, cfg *config.Config) error {
ctx := context.Background()

if c.Repository != "" {
Expand All @@ -97,7 +98,7 @@ func (c *buildCmd) Run(logger logging.Logger, sp terminal.SpinnerPrinter) error
concurrency := max(1, c.MaxConcurrency)

schemasFS := afero.NewBasePathFs(c.projFS, c.proj.Spec.Paths.Schemas)
generators := generator.Filter(generator.AllLanguages(), c.proj.Spec.Schemas.GetLanguages())
generators := generator.Filter(generator.AllLanguages(generator.WithGoModelAccessors(cfg.Features.GenerateGoModelAccessors)), c.proj.Spec.Schemas.GetLanguages())
schemaRunner := runner.NewRealSchemaRunner(runner.WithImageConfig(c.proj.Spec.ImageConfigs))
schemaMgr := manager.New(schemasFS, generators, schemaRunner)
cacheDir := c.CacheDir
Expand Down
5 changes: 3 additions & 2 deletions cmd/crossplane/project/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
devv1alpha1 "github.com/crossplane/cli/v2/apis/dev/v1alpha1"
"github.com/crossplane/cli/v2/cmd/crossplane/render"
"github.com/crossplane/cli/v2/internal/async"
"github.com/crossplane/cli/v2/internal/config"
"github.com/crossplane/cli/v2/internal/dependency"
"github.com/crossplane/cli/v2/internal/project"
"github.com/crossplane/cli/v2/internal/project/controlplane"
Expand Down Expand Up @@ -132,7 +133,7 @@ func (c *runCmd) AfterApply() error {
}

// Run executes the run command.
func (c *runCmd) Run(logger logging.Logger, sp terminal.SpinnerPrinter) error { //nolint:gocyclo // Main command orchestration.
func (c *runCmd) Run(logger logging.Logger, sp terminal.SpinnerPrinter, cfg *config.Config) error { //nolint:gocyclo // Main command orchestration.
ctx := context.Background()

if c.Repository != "" {
Expand All @@ -150,7 +151,7 @@ func (c *runCmd) Run(logger logging.Logger, sp terminal.SpinnerPrinter) error {
concurrency := max(1, c.MaxConcurrency)

schemasFS := afero.NewBasePathFs(c.projFS, c.proj.Spec.Paths.Schemas)
generators := generator.Filter(generator.AllLanguages(), c.proj.Spec.Schemas.GetLanguages())
generators := generator.Filter(generator.AllLanguages(generator.WithGoModelAccessors(cfg.Features.GenerateGoModelAccessors)), c.proj.Spec.Schemas.GetLanguages())
schemaRunner := runner.NewRealSchemaRunner(runner.WithImageConfig(c.proj.Spec.ImageConfigs))
schemaMgr := manager.New(schemasFS, generators, schemaRunner)
cacheDir := c.CacheDir
Expand Down
5 changes: 5 additions & 0 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ type Config struct {
type Features struct {
EnableAlpha bool `json:"enableAlpha,omitempty"`
DisableBeta bool `json:"disableBeta,omitempty"`

// GenerateGoModelAccessors enables generation of GetX/SetX accessor methods
// on generated Go models. Disabled by default; opt in to use generics and
// interfaces over generated resources.
GenerateGoModelAccessors bool `json:"generateGoModelAccessors,omitempty"`
}

// Load reads a Config from path. A missing file is not an error; the zero
Expand Down
161 changes: 161 additions & 0 deletions internal/schemas/generator/accessors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
/*
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 generator

import (
"go/ast"
"go/format"
"go/parser"
"go/token"
"strings"

"github.com/crossplane/crossplane-runtime/v2/pkg/errors"
)

// accessorReceiver is the receiver variable name used by generated accessor
// methods. A single letter cannot collide with any generated package import
// alias, which are all multi-letter.
const accessorReceiver = "o"

// addAccessors generates GetX/SetX accessor methods for every field of every
// struct type declared in the given Go source. Getters return the field's
// (pointer) type as-is and setters take the same type, so the generated methods
// reference only types already present in the file and never require new
// imports. Type aliases are skipped: their Type is not a struct literal, so they
// share the underlying struct's method set for free.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
func addAccessors(code string) (string, error) {
fset := token.NewFileSet()
f, err := parser.ParseFile(fset, "", code, parser.ParseComments)
if err != nil {
return "", errors.Wrap(err, "failed to parse Go code for accessors")
}

// Collect the methods that already exist on each type, so we never emit a
// GetX/SetX that collides with a method oapi-codegen already generated
// (e.g. GetAdditionalProperties, or union As/From/Merge helpers). A
// duplicate method would make the package fail to compile.
existing := collectExistingMethods(f)

var b strings.Builder
// Walk declarations in source order so the generated output is stable.
for _, decl := range f.Decls {
gen, ok := decl.(*ast.GenDecl)
if !ok || gen.Tok != token.TYPE {
continue
}
for _, spec := range gen.Specs {
ts, ok := spec.(*ast.TypeSpec)
if !ok {
continue
}
// Skip type aliases (`type Foo = Bar`); only generate accessors for
// struct type definitions.
if ts.Assign.IsValid() {
continue
}
st, ok := ts.Type.(*ast.StructType)
if !ok || st.Fields == nil {
continue
}
writeStructAccessors(&b, fset, ts.Name.Name, st, existing[ts.Name.Name])
}
}

if b.Len() == 0 {
return code, nil
}

combined := code + "\n" + b.String()
formatted, err := format.Source([]byte(combined))
if err != nil {
return "", errors.Wrap(err, "failed to format generated accessors")
}
return string(formatted), nil
}

// collectExistingMethods returns, per receiver type name, the set of method
// names already declared in the file.
func collectExistingMethods(f *ast.File) map[string]map[string]bool {
existing := map[string]map[string]bool{}
for _, decl := range f.Decls {
fn, ok := decl.(*ast.FuncDecl)
if !ok || fn.Recv == nil || len(fn.Recv.List) != 1 {
continue
}
recv := receiverTypeName(fn.Recv.List[0].Type)
if recv == "" {
continue
}
if existing[recv] == nil {
existing[recv] = map[string]bool{}
}
existing[recv][fn.Name.Name] = true
}
return existing
}

// receiverTypeName returns the bare type name of a method receiver, stripping a
// leading pointer if present (e.g. `*Foo` -> `Foo`).
func receiverTypeName(e ast.Expr) string {
if star, ok := e.(*ast.StarExpr); ok {
e = star.X
}
if id, ok := e.(*ast.Ident); ok {
return id.Name
}
return ""
}

// writeStructAccessors appends a getter and setter for each named field of the
// given struct to b. Any accessor whose name already exists in skip is omitted
// to avoid colliding with methods oapi-codegen already generated.
func writeStructAccessors(b *strings.Builder, fset *token.FileSet, typeName string, st *ast.StructType, skip map[string]bool) {
for _, field := range st.Fields.List {
// Skip embedded/anonymous fields; generated models don't use them.
if len(field.Names) == 0 {
continue
}

var typ strings.Builder
if err := format.Node(&typ, fset, field.Type); err != nil {
// format.Node only fails on malformed nodes, which cannot occur for
// a node we just parsed; skip defensively rather than panic.
continue
}
fieldType := typ.String()

for _, name := range field.Names {
fieldName := name.Name

// Getter.
if !skip["Get"+fieldName] {
b.WriteString("\n// Get" + fieldName + " returns the " + fieldName + " field.\n")
b.WriteString("func (" + accessorReceiver + " *" + typeName + ") Get" + fieldName + "() " + fieldType + " {\n")
b.WriteString("\treturn " + accessorReceiver + "." + fieldName + "\n")
b.WriteString("}\n")
}

// Setter.
if !skip["Set"+fieldName] {
b.WriteString("\n// Set" + fieldName + " sets the " + fieldName + " field.\n")
b.WriteString("func (" + accessorReceiver + " *" + typeName + ") Set" + fieldName + "(v " + fieldType + ") {\n")
b.WriteString("\t" + accessorReceiver + "." + fieldName + " = v\n")
b.WriteString("}\n")
}
}
}
}
Loading