From e38b155520e340f7018d849553bb449bfb755279 Mon Sep 17 00:00:00 2001 From: Erik Miller Date: Fri, 26 Jun 2026 10:02:35 -0700 Subject: [PATCH 1/6] generator: add opt-in GetX/SetX accessors to generated Go models Generate pointer-in/pointer-out accessor methods for every field of every struct in the generated Go models, so consumers can abstract over resources with interfaces and generics. Folded into generateGo as a single AST post-process step so it applies uniformly across all generation paths. Gated behind a new features.generateGoModelAccessors config flag (off by default), threaded from config through the project build/run and function generate commands into the Go generator. Skips any GetX/SetX whose name already exists on a type, so the accessors never collide with methods oapi-codegen already emits (e.g. union or additionalProperties helpers). Verified by a compile gate that builds the generated module with a consumer that uses an accessor through an interface; the gate skips gracefully when module dependencies cannot be resolved offline. Signed-off-by: Erik Miller --- cmd/crossplane/config/help/config.md | 7 + cmd/crossplane/config/set.go | 5 +- cmd/crossplane/function/generate.go | 5 +- cmd/crossplane/function/generate_test.go | 3 +- cmd/crossplane/main.go | 2 + cmd/crossplane/project/build.go | 5 +- cmd/crossplane/project/run.go | 5 +- internal/config/config.go | 5 + internal/schemas/generator/accessors.go | 161 ++++++++ internal/schemas/generator/accessors_test.go | 393 +++++++++++++++++++ internal/schemas/generator/go.go | 46 ++- internal/schemas/generator/interface.go | 23 +- 12 files changed, 633 insertions(+), 27 deletions(-) create mode 100644 internal/schemas/generator/accessors.go create mode 100644 internal/schemas/generator/accessors_test.go diff --git a/cmd/crossplane/config/help/config.md b/cmd/crossplane/config/help/config.md index 8803afb0..cb1ff0b4 100644 --- a/cmd/crossplane/config/help/config.md +++ b/cmd/crossplane/config/help/config.md @@ -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 +``` diff --git a/cmd/crossplane/config/set.go b/cmd/crossplane/config/set.go index b85fc5ce..b73b5798 100644 --- a/cmd/crossplane/config/set.go +++ b/cmd/crossplane/config/set.go @@ -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 { diff --git a/cmd/crossplane/function/generate.go b/cmd/crossplane/function/generate.go index 52c57734..1984e7ce 100644 --- a/cmd/crossplane/function/generate.go +++ b/cmd/crossplane/function/generate.go @@ -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" @@ -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 } @@ -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)), ) diff --git a/cmd/crossplane/function/generate_test.go b/cmd/crossplane/function/generate_test.go index d23dade4..e7da277b 100644 --- a/cmd/crossplane/function/generate_test.go +++ b/cmd/crossplane/function/generate_test.go @@ -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" ) @@ -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) diff --git a/cmd/crossplane/main.go b/cmd/crossplane/main.go index ce415f1b..1184cdec 100644 --- a/cmd/crossplane/main.go +++ b/cmd/crossplane/main.go @@ -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), kong.Help(helpPrinter), kong.UsageOnError()) diff --git a/cmd/crossplane/project/build.go b/cmd/crossplane/project/build.go index 825283d6..6ee9e3a8 100644 --- a/cmd/crossplane/project/build.go +++ b/cmd/crossplane/project/build.go @@ -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" @@ -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 != "" { @@ -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 diff --git a/cmd/crossplane/project/run.go b/cmd/crossplane/project/run.go index 8395e137..bd3bc6f4 100644 --- a/cmd/crossplane/project/run.go +++ b/cmd/crossplane/project/run.go @@ -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" @@ -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 != "" { @@ -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 diff --git a/internal/config/config.go b/internal/config/config.go index a3a5b238..de1ab2d5 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -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 diff --git a/internal/schemas/generator/accessors.go b/internal/schemas/generator/accessors.go new file mode 100644 index 00000000..518a338e --- /dev/null +++ b/internal/schemas/generator/accessors.go @@ -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. +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") + } + } + } +} diff --git a/internal/schemas/generator/accessors_test.go b/internal/schemas/generator/accessors_test.go new file mode 100644 index 00000000..ca68b0a9 --- /dev/null +++ b/internal/schemas/generator/accessors_test.go @@ -0,0 +1,393 @@ +/* +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/parser" + "go/token" + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/spf13/afero" +) + +// collectMethods parses Go source and returns a set of the methods it declares, +// keyed by "recv.name", with the rendered type of the single param or result. +func collectMethods(t *testing.T, src string) map[string]string { + t.Helper() + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, "", src, parser.ParseComments) + if err != nil { + t.Fatalf("failed to parse generated source: %v\n%s", err, src) + } + + out := map[string]string{} + for _, decl := range f.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Recv == nil || len(fn.Recv.List) != 1 { + continue + } + recv := renderRecv(fn.Recv.List[0].Type) + var typ string + switch { + case fn.Type.Results != nil && len(fn.Type.Results.List) == 1: + typ = renderExpr(t, fn.Type.Results.List[0].Type) + case fn.Type.Params != nil && len(fn.Type.Params.List) == 1: + typ = renderExpr(t, fn.Type.Params.List[0].Type) + } + out[recv+"."+fn.Name.Name] = typ + } + return out +} + +func renderRecv(e ast.Expr) string { + if star, ok := e.(*ast.StarExpr); ok { + if id, ok := star.X.(*ast.Ident); ok { + return id.Name + } + } + if id, ok := e.(*ast.Ident); ok { + return id.Name + } + return "" +} + +func renderExpr(t *testing.T, e ast.Expr) string { + t.Helper() + switch x := e.(type) { + case *ast.StarExpr: + return "*" + renderExpr(t, x.X) + case *ast.Ident: + return x.Name + case *ast.ArrayType: + return "[]" + renderExpr(t, x.Elt) + case *ast.MapType: + return "map[" + renderExpr(t, x.Key) + "]" + renderExpr(t, x.Value) + case *ast.SelectorExpr: + return renderExpr(t, x.X) + "." + x.Sel.Name + default: + return "" + } +} + +// hasMethod reports whether src declares a method recv.name (recv without the +// leading *). +func hasMethod(t *testing.T, src []byte, recv, name string) bool { + t.Helper() + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, "", src, parser.ParseComments) + if err != nil { + t.Fatalf("failed to parse source: %v", err) + } + for _, decl := range f.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Recv == nil || len(fn.Recv.List) != 1 { + continue + } + if renderRecv(fn.Recv.List[0].Type) == recv && fn.Name.Name == name { + return true + } + } + return false +} + +// TestGenerateFromCRDNoAccessorsByDefault verifies the feature gate: with the +// default (disabled) generator, no accessor methods are emitted. +func TestGenerateFromCRDNoAccessorsByDefault(t *testing.T) { + inputFS := afero.NewBasePathFs(afero.FromIOFS{FS: testdataFS}, "testdata") + schemaFS, err := goGenerator{}.GenerateFromCRD(t.Context(), inputFS, nil) + if err != nil { + t.Fatal(err) + } + + crd, err := afero.ReadFile(schemaFS, "models/co/acme/platform/v1alpha1/xaccountscaffold.go") + if err != nil { + t.Fatal(err) + } + if hasMethod(t, crd, "XAccountScaffold", "GetSpec") { + t.Error("accessors must not be generated when the feature is disabled") + } +} + +func TestGenerateFromCRDIncludesAccessors(t *testing.T) { + inputFS := afero.NewBasePathFs(afero.FromIOFS{FS: testdataFS}, "testdata") + schemaFS, err := goGenerator{accessors: true}.GenerateFromCRD(t.Context(), inputFS, nil) + if err != nil { + t.Fatal(err) + } + + cases := []struct { + name string + file string + recv string + method string + reason string + }{ + { + name: "TopLevelResourceGetter", + file: "models/co/acme/platform/v1alpha1/xaccountscaffold.go", + recv: "XAccountScaffold", + method: "GetSpec", + reason: "top-level resource structs get getters", + }, + { + name: "TopLevelResourceSetter", + file: "models/co/acme/platform/v1alpha1/xaccountscaffold.go", + recv: "XAccountScaffold", + method: "SetSpec", + reason: "top-level resource structs get setters", + }, + { + name: "TopLevelK8sFieldAccessor", + file: "models/co/acme/platform/v1alpha1/xaccountscaffold.go", + recv: "XAccountScaffold", + method: "GetMetadata", + reason: "fields of imported k8s types are still accessible", + }, + { + name: "NestedStructGetter", + file: "models/co/acme/platform/v1alpha1/xaccountscaffold.go", + recv: "XAccountScaffoldSpec", + method: "GetParameters", + reason: "nested structs get accessors, not just the top-level resource", + }, + { + name: "NestedStructSetter", + file: "models/co/acme/platform/v1alpha1/xaccountscaffold.go", + recv: "XAccountScaffoldSpec", + method: "SetParameters", + reason: "nested structs get setters too", + }, + { + name: "SharedK8sBuildingBlock", + file: "models/io/k8s/meta/v1/meta.go", + recv: "ObjectMeta", + method: "GetName", + reason: "accessors everywhere includes the shared k8s building-block types", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + src, err := afero.ReadFile(schemaFS, tc.file) + if err != nil { + t.Fatal(err) + } + if !hasMethod(t, src, tc.recv, tc.method) { + t.Errorf("expected %s to declare %s.%s (%s)", tc.file, tc.recv, tc.method, tc.reason) + } + }) + } +} + +// materialize writes every file in the afero filesystem out to dir on disk. +func materialize(t *testing.T, fs afero.Fs, dir string) { + t.Helper() + err := afero.Walk(fs, "", func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info.IsDir() { + return nil + } + bs, err := afero.ReadFile(fs, path) + if err != nil { + return err + } + dst := filepath.Join(dir, path) + if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil { + return err + } + return os.WriteFile(dst, bs, 0o644) + }) + if err != nil { + t.Fatalf("failed to materialize generated FS: %v", err) + } +} + +// TestGeneratedModelsCompileWithAccessors is the real verification: it writes +// the generated models to disk, adds a consumer that exercises an accessor +// *through an interface*, and compiles the whole module. Parsing alone would +// pass on output that fails to compile (malformed signatures, name collisions); +// this catches that, and the interface usage proves the actual goal. +func TestGeneratedModelsCompileWithAccessors(t *testing.T) { + if _, err := exec.LookPath("go"); err != nil { + t.Skip("go toolchain not available; skipping compile gate") + } + + inputFS := afero.NewBasePathFs(afero.FromIOFS{FS: testdataFS}, "testdata") + schemaFS, err := goGenerator{accessors: true}.GenerateFromCRD(t.Context(), inputFS, nil) + if err != nil { + t.Fatal(err) + } + + dir := t.TempDir() + materialize(t, schemaFS, dir) + + // A consumer that abstracts over the generated resource via an interface, + // satisfied structurally by the generated accessors. + consumer := `package consumer + +import v1alpha1 "dev.crossplane.io/models/co/acme/platform/v1alpha1" + +type specAccessor interface { + GetSpec() *v1alpha1.XAccountScaffoldSpec + SetSpec(*v1alpha1.XAccountScaffoldSpec) +} + +var _ specAccessor = &v1alpha1.XAccountScaffold{} + +// useAccessors round-trips a value through the interface to prove the methods +// are usable, not just present. +func useAccessors(a specAccessor) *v1alpha1.XAccountScaffoldSpec { + a.SetSpec(a.GetSpec()) + return a.GetSpec() +} + +var _ = useAccessors +` + consumerDir := filepath.Join(dir, "models", "consumer") + if err := os.MkdirAll(consumerDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(consumerDir, "consumer.go"), []byte(consumer), 0o644); err != nil { + t.Fatal(err) + } + + modelsDir := filepath.Join(dir, "models") + + // The generated module depends on github.com/oapi-codegen/runtime, which is + // not a dependency of this repository, so building it requires resolving + // that module from the proxy or module cache. Probe with `go mod download` + // first: if modules can't be resolved (e.g. an offline runner with + // GOPROXY=off and a cold cache), skip rather than report a false failure. + // Internal dev.crossplane.io/models/... imports resolve within the module + // itself, so no replace directive is needed for them. + probe := exec.CommandContext(t.Context(), "go", "mod", "download") + probe.Dir = modelsDir + if out, err := probe.CombinedOutput(); err != nil { + t.Skipf("cannot resolve generated module dependencies (offline?); skipping compile gate: %v\n%s", err, out) + } + + cmd := exec.CommandContext(t.Context(), "go", "build", "./...") + cmd.Dir = modelsDir + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("generated models failed to compile: %v\n%s", err, out) + } +} + +func TestAddAccessors(t *testing.T) { + input := `package v1alpha1 + +type Foo struct { + Name *string ` + "`json:\"name,omitempty\"`" + ` + Items *[]string ` + "`json:\"items,omitempty\"`" + ` + Labels *map[string]string ` + "`json:\"labels,omitempty\"`" + ` + Bar *Bar ` + "`json:\"bar,omitempty\"`" + ` +} + +type Bar struct { + Count *int64 ` + "`json:\"count,omitempty\"`" + ` +} + +// FooAlias is a type alias and must NOT receive its own accessors. +type FooAlias = Foo +` + + got, err := addAccessors(input) + if err != nil { + t.Fatalf("addAccessors returned error: %v", err) + } + + // collectMethods returns every method in the output, so an exact diff + // against want verifies both that the expected accessors exist with the + // right pointer signatures and that nothing extra was generated — in + // particular, that the FooAlias type alias did not get its own methods. + want := map[string]string{ + "Foo.GetName": "*string", + "Foo.SetName": "*string", + "Foo.GetItems": "*[]string", + "Foo.SetItems": "*[]string", + "Foo.GetLabels": "*map[string]string", + "Foo.SetLabels": "*map[string]string", + "Foo.GetBar": "*Bar", + "Foo.SetBar": "*Bar", + "Bar.GetCount": "*int64", + "Bar.SetCount": "*int64", + } + + if diff := cmp.Diff(want, collectMethods(t, got)); diff != "" { + t.Errorf("generated accessors (-want +got):\n%s", diff) + } +} + +// countMethods returns how many times recv.name is declared in src. +func countMethods(t *testing.T, src string, recv, name string) int { + t.Helper() + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, "", src, parser.ParseComments) + if err != nil { + t.Fatalf("failed to parse source: %v\n%s", err, src) + } + n := 0 + for _, decl := range f.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Recv == nil || len(fn.Recv.List) != 1 { + continue + } + if renderRecv(fn.Recv.List[0].Type) == recv && fn.Name.Name == name { + n++ + } + } + return n +} + +// TestAddAccessorsSkipsExistingMethods guards against colliding with methods +// oapi-codegen already emits (e.g. GetAdditionalProperties, union As/From +// helpers). A GetX/SetX whose name already exists on the type must not be +// re-emitted, or the package would fail to compile with a duplicate method. +func TestAddAccessorsSkipsExistingMethods(t *testing.T) { + input := `package v1alpha1 + +type Foo struct { + AdditionalProperties *map[string]string ` + "`json:\"-\"`" + ` +} + +// Pre-existing accessor, as oapi-codegen would generate for additionalProperties. +func (o *Foo) GetAdditionalProperties() *map[string]string { + return o.AdditionalProperties +} +` + + got, err := addAccessors(input) + if err != nil { + t.Fatalf("addAccessors returned error: %v", err) + } + + // The pre-existing getter must remain the only GetAdditionalProperties. + if n := countMethods(t, got, "Foo", "GetAdditionalProperties"); n != 1 { + t.Errorf("expected exactly 1 GetAdditionalProperties (no duplicate), got %d", n) + } + // The setter doesn't exist yet, so it should still be generated. + if n := countMethods(t, got, "Foo", "SetAdditionalProperties"); n != 1 { + t.Errorf("expected SetAdditionalProperties to be generated, got %d", n) + } +} diff --git a/internal/schemas/generator/go.go b/internal/schemas/generator/go.go index 129f76ea..58fe2689 100644 --- a/internal/schemas/generator/go.go +++ b/internal/schemas/generator/go.go @@ -138,14 +138,18 @@ var ( ) ` -type goGenerator struct{} +// goGenerator generates Go models. accessors controls whether GetX/SetX +// accessor methods are emitted for the generated structs. +type goGenerator struct { + accessors bool +} func (goGenerator) Language() string { return devv1alpha1.SchemaLanguageGo } // GenerateFromCRD generates Go schemas for the CRDs in the given filesystem. -func (goGenerator) GenerateFromCRD(_ context.Context, fromFS afero.Fs, _ runner.SchemaRunner) (afero.Fs, error) { +func (g goGenerator) GenerateFromCRD(_ context.Context, fromFS afero.Fs, _ runner.SchemaRunner) (afero.Fs, error) { openAPIs, err := goCollectOpenAPIs(fromFS) if err != nil { return nil, err @@ -217,7 +221,7 @@ func (goGenerator) GenerateFromCRD(_ context.Context, fromFS afero.Fs, _ runner. refMutator = goReferenceK8sTypesForCRDs } - code, err := generateGo(pkgSpec, version, + code, err := generateGo(pkgSpec, version, g.accessors, goRenameTypes, goRenameEnums, goReplaceNumberWithInt, @@ -247,7 +251,7 @@ func (goGenerator) GenerateFromCRD(_ context.Context, fromFS afero.Fs, _ runner. // Generate models for the non-k8s schemas. for _, oapi := range openAPIs { - code, err := generateGo(oapi.spec, oapi.version, + code, err := generateGo(oapi.spec, oapi.version, g.accessors, goRenameTypes, goRenameEnums, goReplaceNumberWithInt, @@ -376,7 +380,7 @@ func goCollectOpenAPIs(fromFS afero.Fs) ([]goOpenAPI, error) { //nolint:gocognit // `generateGo`, since `codegen.Generate` is not concurrency safe. var generateGoMutex sync.Mutex //nolint:gochecknoglobals // Must be global. -func generateGo(s *spec3.OpenAPI, version string, mutators ...func(*spec3.OpenAPI)) (string, error) { +func generateGo(s *spec3.OpenAPI, version string, accessors bool, mutators ...func(*spec3.OpenAPI)) (string, error) { // codegen.Generate sets some global state that's used by the utility // functions we call from our mutators. That has two implications for us: // @@ -438,6 +442,16 @@ func generateGo(s *spec3.OpenAPI, version string, mutators ...func(*spec3.OpenAP return "", errors.Wrap(err, "failed to fix missing imports") } + // Generate GetX/SetX accessor methods for every struct field, so consumers + // can abstract over the models with interfaces and generics. Gated behind + // the features.generateGoModelAccessors flag. + if accessors { + goCode, err = addAccessors(goCode) + if err != nil { + return "", errors.Wrap(err, "failed to add accessors") + } + } + goCodeBytes, err := format.Source([]byte(goCode)) if err != nil { return "", errors.Wrap(err, "failed to format go code") @@ -1088,7 +1102,7 @@ func fixK8sTypeNames(code string) (string, error) { } // GenerateFromOpenAPI generates Go schemas for the OpenAPI docs in the given filesystem. -func (goGenerator) GenerateFromOpenAPI(_ context.Context, fromFS afero.Fs, _ runner.SchemaRunner) (afero.Fs, error) { +func (g goGenerator) GenerateFromOpenAPI(_ context.Context, fromFS afero.Fs, _ runner.SchemaRunner) (afero.Fs, error) { // Walk through filesystem to collect OpenAPI specs openAPISpecs, err := collectOpenAPISpecs(fromFS) if err != nil { @@ -1107,12 +1121,12 @@ func (goGenerator) GenerateFromOpenAPI(_ context.Context, fromFS afero.Fs, _ run } // Generate K8s shared schemas - if err := generateK8sSharedSchemas(openAPISpecs, schemaFS); err != nil { + if err := generateK8sSharedSchemas(openAPISpecs, schemaFS, g.accessors); err != nil { return nil, err } // Generate models for the rest - if err := generateModelsWithGVK(openAPISpecs, schemaFS); err != nil { + if err := generateModelsWithGVK(openAPISpecs, schemaFS, g.accessors); err != nil { return nil, err } @@ -1189,7 +1203,7 @@ func initializeSchemaFS() (afero.Fs, error) { } // generateK8sSharedSchemas extracts and generates shared K8s schemas. -func generateK8sSharedSchemas(openAPISpecs []*spec3.OpenAPI, schemaFS afero.Fs) error { +func generateK8sSharedSchemas(openAPISpecs []*spec3.OpenAPI, schemaFS afero.Fs, accessors bool) error { k8sSchemasByPackage := make(map[string]map[string]*spec.Schema) // Collect all K8s schemas from all OpenAPI specs, grouped by package @@ -1209,7 +1223,7 @@ func generateK8sSharedSchemas(openAPISpecs []*spec3.OpenAPI, schemaFS afero.Fs) continue } - if err := generateK8sPackageCode(pkg, schemas, schemaFS); err != nil { + if err := generateK8sPackageCode(pkg, schemas, schemaFS, accessors); err != nil { return err } } @@ -1218,7 +1232,7 @@ func generateK8sSharedSchemas(openAPISpecs []*spec3.OpenAPI, schemaFS afero.Fs) } // generateK8sPackageCode generates code for a single K8s package. -func generateK8sPackageCode(pkg string, schemas map[string]*spec.Schema, schemaFS afero.Fs) error { +func generateK8sPackageCode(pkg string, schemas map[string]*spec.Schema, schemaFS afero.Fs, accessors bool) error { // Create a spec for this package pkgSpec := &spec3.OpenAPI{ Version: "3.0.0", @@ -1230,7 +1244,7 @@ func generateK8sPackageCode(pkg string, schemas map[string]*spec.Schema, schemaF // Determine the group, kind, and version from the package name group, kind, version := getK8sPackageInfo(pkg) - code, err := generateGo(pkgSpec, version, + code, err := generateGo(pkgSpec, version, accessors, goRenameTypes, goRenameEnums, goReplaceNumberWithInt, @@ -1277,12 +1291,12 @@ func getK8sPackageInfo(pkg string) (group, kind, version string) { } // generateModelsWithGVK generates models for schemas with GVK information. -func generateModelsWithGVK(openAPISpecs []*spec3.OpenAPI, schemaFS afero.Fs) error { +func generateModelsWithGVK(openAPISpecs []*spec3.OpenAPI, schemaFS afero.Fs, accessors bool) error { for _, openAPISpec := range openAPISpecs { gvkGroups := groupSchemasByGVK(openAPISpec) for gvkKey, schemas := range gvkGroups { - if err := generateGVKGroupCode(gvkKey, schemas, openAPISpec, schemaFS); err != nil { + if err := generateGVKGroupCode(gvkKey, schemas, openAPISpec, schemaFS, accessors); err != nil { return err } } @@ -1345,7 +1359,7 @@ func extractGVKKey(schema *spec.Schema) string { } // generateGVKGroupCode generates code for a GVK group. -func generateGVKGroupCode(gvkKey string, schemas map[string]*spec.Schema, openAPISpec *spec3.OpenAPI, schemaFS afero.Fs) error { +func generateGVKGroupCode(gvkKey string, schemas map[string]*spec.Schema, openAPISpec *spec3.OpenAPI, schemaFS afero.Fs, accessors bool) error { parts := strings.Split(gvkKey, "/") group, version := parts[0], parts[1] @@ -1372,7 +1386,7 @@ func generateGVKGroupCode(gvkKey string, schemas map[string]*spec.Schema, openAP // Add schemas that don't have GVK extensions (supporting types) maps.Copy(groupSpec.Components.Schemas, openAPISpec.Components.Schemas) - code, err := generateGo(groupSpec, version, + code, err := generateGo(groupSpec, version, accessors, goRenameTypes, goRenameEnums, goReplaceNumberWithInt, diff --git a/internal/schemas/generator/interface.go b/internal/schemas/generator/interface.go index d41300b9..4bace4dd 100644 --- a/internal/schemas/generator/interface.go +++ b/internal/schemas/generator/interface.go @@ -34,12 +34,31 @@ type Interface interface { GenerateFromOpenAPI(ctx context.Context, fs afero.Fs, runner runner.SchemaRunner) (afero.Fs, error) } +// options holds configurable behavior shared across generators. +type options struct { + goModelAccessors bool +} + +// Option configures the generators returned by AllLanguages. +type Option func(*options) + +// WithGoModelAccessors enables generation of GetX/SetX accessor methods on the +// generated Go models. It is disabled by default and gated behind the +// features.generateGoModelAccessors config flag. +func WithGoModelAccessors(enabled bool) Option { + return func(o *options) { o.goModelAccessors = enabled } +} + // AllLanguages returns generators for all supported languages. The set of // supported language identifiers is defined by // devv1alpha1.SupportedSchemaLanguages. -func AllLanguages() []Interface { +func AllLanguages(opts ...Option) []Interface { + o := &options{} + for _, opt := range opts { + opt(o) + } return []Interface{ - &goGenerator{}, + &goGenerator{accessors: o.goModelAccessors}, &jsonGenerator{}, &kclGenerator{}, &pythonGenerator{}, From 07a44e723228d6f3b6785c91716296165d79ca27 Mon Sep 17 00:00:00 2001 From: Erik Miller Date: Tue, 21 Jul 2026 08:31:52 -0700 Subject: [PATCH 2/6] generator: address review feedback on Go model accessors Apply accessor generation after Go post-processing at the call sites via a new applyAccessors helper, rather than folding it into generateGo. This mirrors the runtime.Object generation approach in #162 so both features use the same "post-process the generated code after the fact" pattern, and makes generateGo's signature identical across both, keeping the two changes easy to reconcile. Generating accessors after fixK8sTypeNames/removeSelfImports also means they reference the final type names. Skip unexported struct fields when emitting accessors: generated models don't currently have any, but an accessor for one would be useless to external consumers and could produce oddly-cased method names. Reuse receiverTypeName from accessors.go in the tests instead of redefining an equivalent renderRecv helper. Signed-off-by: Erik Miller --- internal/schemas/generator/accessors.go | 17 ++++++++ internal/schemas/generator/accessors_test.go | 18 ++------ internal/schemas/generator/go.go | 44 +++++++++++++------- 3 files changed, 49 insertions(+), 30 deletions(-) diff --git a/internal/schemas/generator/accessors.go b/internal/schemas/generator/accessors.go index 518a338e..658abd15 100644 --- a/internal/schemas/generator/accessors.go +++ b/internal/schemas/generator/accessors.go @@ -31,6 +31,16 @@ import ( // alias, which are all multi-letter. const accessorReceiver = "o" +// applyAccessors runs addAccessors when enabled. It is called by the generation +// loops after all other Go post-processing, so the accessors reference the +// final, fixed-up type names. +func applyAccessors(code string, enabled bool) (string, error) { + if !enabled { + return code, nil + } + return addAccessors(code) +} + // 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 @@ -139,6 +149,13 @@ func writeStructAccessors(b *strings.Builder, fset *token.FileSet, typeName stri fieldType := typ.String() for _, name := range field.Names { + // Skip unexported fields: an accessor for them would be useless to + // external consumers and could produce oddly-cased method names. + // Generated models don't currently have any, but guard defensively. + if !name.IsExported() { + continue + } + fieldName := name.Name // Getter. diff --git a/internal/schemas/generator/accessors_test.go b/internal/schemas/generator/accessors_test.go index ca68b0a9..d9f98412 100644 --- a/internal/schemas/generator/accessors_test.go +++ b/internal/schemas/generator/accessors_test.go @@ -45,7 +45,7 @@ func collectMethods(t *testing.T, src string) map[string]string { if !ok || fn.Recv == nil || len(fn.Recv.List) != 1 { continue } - recv := renderRecv(fn.Recv.List[0].Type) + recv := receiverTypeName(fn.Recv.List[0].Type) var typ string switch { case fn.Type.Results != nil && len(fn.Type.Results.List) == 1: @@ -58,18 +58,6 @@ func collectMethods(t *testing.T, src string) map[string]string { return out } -func renderRecv(e ast.Expr) string { - if star, ok := e.(*ast.StarExpr); ok { - if id, ok := star.X.(*ast.Ident); ok { - return id.Name - } - } - if id, ok := e.(*ast.Ident); ok { - return id.Name - } - return "" -} - func renderExpr(t *testing.T, e ast.Expr) string { t.Helper() switch x := e.(type) { @@ -102,7 +90,7 @@ func hasMethod(t *testing.T, src []byte, recv, name string) bool { if !ok || fn.Recv == nil || len(fn.Recv.List) != 1 { continue } - if renderRecv(fn.Recv.List[0].Type) == recv && fn.Name.Name == name { + if receiverTypeName(fn.Recv.List[0].Type) == recv && fn.Name.Name == name { return true } } @@ -353,7 +341,7 @@ func countMethods(t *testing.T, src string, recv, name string) int { if !ok || fn.Recv == nil || len(fn.Recv.List) != 1 { continue } - if renderRecv(fn.Recv.List[0].Type) == recv && fn.Name.Name == name { + if receiverTypeName(fn.Recv.List[0].Type) == recv && fn.Name.Name == name { n++ } } diff --git a/internal/schemas/generator/go.go b/internal/schemas/generator/go.go index 58fe2689..2d54cdce 100644 --- a/internal/schemas/generator/go.go +++ b/internal/schemas/generator/go.go @@ -221,7 +221,7 @@ func (g goGenerator) GenerateFromCRD(_ context.Context, fromFS afero.Fs, _ runne refMutator = goReferenceK8sTypesForCRDs } - code, err := generateGo(pkgSpec, version, g.accessors, + code, err := generateGo(pkgSpec, version, goRenameTypes, goRenameEnums, goReplaceNumberWithInt, @@ -244,6 +244,12 @@ func (g goGenerator) GenerateFromCRD(_ context.Context, fromFS afero.Fs, _ runne return nil, err } + // Add GetX/SetX accessors last, so they see the final type names. + code, err = applyAccessors(code, g.accessors) + if err != nil { + return nil, err + } + if err := writeGoCode(schemaFS, group, kind, version, code); err != nil { return nil, err } @@ -251,7 +257,7 @@ func (g goGenerator) GenerateFromCRD(_ context.Context, fromFS afero.Fs, _ runne // Generate models for the non-k8s schemas. for _, oapi := range openAPIs { - code, err := generateGo(oapi.spec, oapi.version, g.accessors, + code, err := generateGo(oapi.spec, oapi.version, goRenameTypes, goRenameEnums, goReplaceNumberWithInt, @@ -264,6 +270,12 @@ func (g goGenerator) GenerateFromCRD(_ context.Context, fromFS afero.Fs, _ runne return nil, err } + // Add GetX/SetX accessors last, so they see the final type names. + code, err = applyAccessors(code, g.accessors) + if err != nil { + return nil, err + } + if err := writeGoCode(schemaFS, oapi.crd.Spec.Group, oapi.crd.Spec.Names.Kind, oapi.version, code); err != nil { return nil, err } @@ -380,7 +392,7 @@ func goCollectOpenAPIs(fromFS afero.Fs) ([]goOpenAPI, error) { //nolint:gocognit // `generateGo`, since `codegen.Generate` is not concurrency safe. var generateGoMutex sync.Mutex //nolint:gochecknoglobals // Must be global. -func generateGo(s *spec3.OpenAPI, version string, accessors bool, mutators ...func(*spec3.OpenAPI)) (string, error) { +func generateGo(s *spec3.OpenAPI, version string, mutators ...func(*spec3.OpenAPI)) (string, error) { // codegen.Generate sets some global state that's used by the utility // functions we call from our mutators. That has two implications for us: // @@ -442,16 +454,6 @@ func generateGo(s *spec3.OpenAPI, version string, accessors bool, mutators ...fu return "", errors.Wrap(err, "failed to fix missing imports") } - // Generate GetX/SetX accessor methods for every struct field, so consumers - // can abstract over the models with interfaces and generics. Gated behind - // the features.generateGoModelAccessors flag. - if accessors { - goCode, err = addAccessors(goCode) - if err != nil { - return "", errors.Wrap(err, "failed to add accessors") - } - } - goCodeBytes, err := format.Source([]byte(goCode)) if err != nil { return "", errors.Wrap(err, "failed to format go code") @@ -1244,7 +1246,7 @@ func generateK8sPackageCode(pkg string, schemas map[string]*spec.Schema, schemaF // Determine the group, kind, and version from the package name group, kind, version := getK8sPackageInfo(pkg) - code, err := generateGo(pkgSpec, version, accessors, + code, err := generateGo(pkgSpec, version, goRenameTypes, goRenameEnums, goReplaceNumberWithInt, @@ -1267,6 +1269,12 @@ func generateK8sPackageCode(pkg string, schemas map[string]*spec.Schema, schemaF return errors.Wrap(err, "failed to remove self imports") } + // Add GetX/SetX accessors last, so they see the final type names. + code, err = applyAccessors(code, accessors) + if err != nil { + return errors.Wrap(err, "failed to add accessors") + } + return writeGoCode(schemaFS, group, kind, version, code) } @@ -1386,7 +1394,7 @@ func generateGVKGroupCode(gvkKey string, schemas map[string]*spec.Schema, openAP // Add schemas that don't have GVK extensions (supporting types) maps.Copy(groupSpec.Components.Schemas, openAPISpec.Components.Schemas) - code, err := generateGo(groupSpec, version, accessors, + code, err := generateGo(groupSpec, version, goRenameTypes, goRenameEnums, goReplaceNumberWithInt, @@ -1400,5 +1408,11 @@ func generateGVKGroupCode(gvkKey string, schemas map[string]*spec.Schema, openAP return err } + // Add GetX/SetX accessors last, so they see the final type names. + code, err = applyAccessors(code, accessors) + if err != nil { + return errors.Wrap(err, "failed to add accessors") + } + return writeGoCode(schemaFS, group, kind, version, code) } From 8671b3951740ea2bfbce286c8f29c4f2d379d392 Mon Sep 17 00:00:00 2001 From: Erik Miller Date: Tue, 21 Jul 2026 08:46:18 -0700 Subject: [PATCH 3/6] generator: keep GenerateFromCRD under the complexity limit Applying accessors in each generation loop pushed GenerateFromCRD past the gocognit limit. Extract the shared-K8s-package loop body into a generateSharedK8sPackage helper to bring it back under. Signed-off-by: Erik Miller --- internal/schemas/generator/go.go | 138 ++++++++++++++++--------------- 1 file changed, 72 insertions(+), 66 deletions(-) diff --git a/internal/schemas/generator/go.go b/internal/schemas/generator/go.go index 2d54cdce..d998510c 100644 --- a/internal/schemas/generator/go.go +++ b/internal/schemas/generator/go.go @@ -185,72 +185,7 @@ func (g goGenerator) GenerateFromCRD(_ context.Context, fromFS afero.Fs, _ runne // Generate separate files for each K8s package for pkg, schemas := range k8sSchemasByPackage { - if len(schemas) == 0 { - continue - } - - // Create a spec for this package - pkgSpec := &spec3.OpenAPI{ - Version: "3.0.0", - Components: &spec3.Components{ - Schemas: schemas, - }, - } - - // Determine the group, kind, and version from the package name - var group, kind, version string - switch pkg { - case k8sPkgMetaV1: - group = "meta.k8s.io" - kind = "meta" - version = "v1" - case k8sPkgAutoscalingV1: - group = k8sPkgNameAutoscaling - kind = k8sPkgNameAutoscaling - version = "v1" - } - - // For K8s packages that reference meta.v1, we need to use the correct - // meta import path. The meta.v1 package uses goReferenceK8sTypes (core - // path) because self-references get stripped. Other packages like - // autoscaling use goReferenceK8sTypesForCRDs (non-core path) to - // reference the CRD meta.v1 package at - // dev.crossplane.io/models/io/k8s/meta/v1. - refMutator := goReferenceK8sTypes - if pkg != k8sPkgMetaV1 { - refMutator = goReferenceK8sTypesForCRDs - } - - code, err := generateGo(pkgSpec, version, - goRenameTypes, - goRenameEnums, - goReplaceNumberWithInt, - goRemoveRequired, - refMutator, - ) - if err != nil { - return nil, err - } - - // shorten the auto‑generated K8s type names - code, err = fixK8sTypeNames(code) - if err != nil { - return nil, err - } - - // remove the self‑import (e.g. meta/v1 importing itself) - code, err = removeSelfImports(code, pkg) - if err != nil { - return nil, err - } - - // Add GetX/SetX accessors last, so they see the final type names. - code, err = applyAccessors(code, g.accessors) - if err != nil { - return nil, err - } - - if err := writeGoCode(schemaFS, group, kind, version, code); err != nil { + if err := g.generateSharedK8sPackage(schemaFS, pkg, schemas); err != nil { return nil, err } } @@ -284,6 +219,77 @@ func (g goGenerator) GenerateFromCRD(_ context.Context, fromFS afero.Fs, _ runne return schemaFS, nil } +// generateSharedK8sPackage generates the Go model file for a single shared K8s +// package (e.g. meta/v1) into schemaFS. +func (g goGenerator) generateSharedK8sPackage(schemaFS afero.Fs, pkg string, schemas map[string]*spec.Schema) error { + if len(schemas) == 0 { + return nil + } + + // Create a spec for this package + pkgSpec := &spec3.OpenAPI{ + Version: "3.0.0", + Components: &spec3.Components{ + Schemas: schemas, + }, + } + + // Determine the group, kind, and version from the package name + var group, kind, version string + switch pkg { + case k8sPkgMetaV1: + group = "meta.k8s.io" + kind = "meta" + version = "v1" + case k8sPkgAutoscalingV1: + group = k8sPkgNameAutoscaling + kind = k8sPkgNameAutoscaling + version = "v1" + } + + // For K8s packages that reference meta.v1, we need to use the correct + // meta import path. The meta.v1 package uses goReferenceK8sTypes (core + // path) because self-references get stripped. Other packages like + // autoscaling use goReferenceK8sTypesForCRDs (non-core path) to + // reference the CRD meta.v1 package at + // dev.crossplane.io/models/io/k8s/meta/v1. + refMutator := goReferenceK8sTypes + if pkg != k8sPkgMetaV1 { + refMutator = goReferenceK8sTypesForCRDs + } + + code, err := generateGo(pkgSpec, version, + goRenameTypes, + goRenameEnums, + goReplaceNumberWithInt, + goRemoveRequired, + refMutator, + ) + if err != nil { + return err + } + + // shorten the auto‑generated K8s type names + code, err = fixK8sTypeNames(code) + if err != nil { + return err + } + + // remove the self‑import (e.g. meta/v1 importing itself) + code, err = removeSelfImports(code, pkg) + if err != nil { + return err + } + + // Add GetX/SetX accessors last, so they see the final type names. + code, err = applyAccessors(code, g.accessors) + if err != nil { + return err + } + + return writeGoCode(schemaFS, group, kind, version, code) +} + type goOpenAPI struct { crd *extv1.CustomResourceDefinition version string From 231e1a84997e847f9efa46d6551f2662e2efc1c0 Mon Sep 17 00:00:00 2001 From: Erik Miller Date: Tue, 21 Jul 2026 09:39:53 -0700 Subject: [PATCH 4/6] generator: clarify accessor generation error message Reword the wrapped error at both accessor call sites from the generic "failed to add accessors" to "failed to generate Go model accessors". Signed-off-by: Erik Miller --- internal/schemas/generator/go.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/schemas/generator/go.go b/internal/schemas/generator/go.go index d998510c..2a7d3b8f 100644 --- a/internal/schemas/generator/go.go +++ b/internal/schemas/generator/go.go @@ -1278,7 +1278,7 @@ func generateK8sPackageCode(pkg string, schemas map[string]*spec.Schema, schemaF // Add GetX/SetX accessors last, so they see the final type names. code, err = applyAccessors(code, accessors) if err != nil { - return errors.Wrap(err, "failed to add accessors") + return errors.Wrap(err, "failed to generate Go model accessors") } return writeGoCode(schemaFS, group, kind, version, code) @@ -1417,7 +1417,7 @@ func generateGVKGroupCode(gvkKey string, schemas map[string]*spec.Schema, openAP // Add GetX/SetX accessors last, so they see the final type names. code, err = applyAccessors(code, accessors) if err != nil { - return errors.Wrap(err, "failed to add accessors") + return errors.Wrap(err, "failed to generate Go model accessors") } return writeGoCode(schemaFS, group, kind, version, code) From 474c701ecb8597608f861f5622a16503f113c011 Mon Sep 17 00:00:00 2001 From: Erik Miller Date: Tue, 28 Jul 2026 11:10:06 -0700 Subject: [PATCH 5/6] config: use active voice in the accessors help text Upstream main dropped the `level: error` input from the Vale step, so reviewdog now runs at `-level=info` and Vale warnings fail validate-docs. "so generated resources can be used through interfaces and generics" trips write-good.Passive. Verified with Vale 3.14.2 against the crossplane/docs config at the alert level the action uses: 0 errors, 0 warnings. Signed-off-by: Erik Miller --- cmd/crossplane/config/help/config.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/crossplane/config/help/config.md b/cmd/crossplane/config/help/config.md index cb1ff0b4..ce3560ee 100644 --- a/cmd/crossplane/config/help/config.md +++ b/cmd/crossplane/config/help/config.md @@ -20,7 +20,7 @@ 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: +you can reach generated resources through interfaces and generics: ```shell crossplane config set features.generateGoModelAccessors true From f3e994d8785c8c633eae314f32b5cc6adb008a7e Mon Sep 17 00:00:00 2001 From: Erik Miller Date: Thu, 30 Jul 2026 06:46:19 -0700 Subject: [PATCH 6/6] generator: thread the accessors flag through the remaining commands project build, project run and function generate already built their generators from the config, but every other command that generates schemas fell back to the flag-off default: - crossplane dependency add - crossplane dependency update-cache - crossplane composition generate - crossplane composition render / crossplane render - crossplane operation render The render commands built the dependency manager without WithSchemaGenerators, so dependency schemas came from the default even once the project's own schemas honored the flag. Build the generators before the manager and hand the same set to both. crossplane composition generate builds its dependency manager in AfterApply, so the config binding goes there rather than on Run. Note on the one remaining default: NewManager still defaults to generator.AllLanguages(), which is correct for a caller that only reads or cleans the cache (crossplane dependency clean-cache). Documented on NewManager so the next flag doesn't repeat this. Verified each command resolves its kong binding and reaches its own logic rather than failing on a missing binding. Signed-off-by: Erik Miller --- cmd/crossplane/composition/generate.go | 8 +++++++- cmd/crossplane/dependency/add.go | 8 +++++++- cmd/crossplane/dependency/cache.go | 8 ++++++-- cmd/crossplane/render/op/cmd.go | 13 +++++++++---- cmd/crossplane/render/op/cmd_test.go | 3 ++- cmd/crossplane/render/xr/cmd.go | 13 +++++++++---- cmd/crossplane/render/xr/cmd_test.go | 3 ++- internal/dependency/manager.go | 5 ++++- 8 files changed, 46 insertions(+), 15 deletions(-) diff --git a/cmd/crossplane/composition/generate.go b/cmd/crossplane/composition/generate.go index 7d5cc73a..a7748183 100644 --- a/cmd/crossplane/composition/generate.go +++ b/cmd/crossplane/composition/generate.go @@ -36,8 +36,10 @@ import ( pkgv1 "github.com/crossplane/crossplane/apis/v2/pkg/v1" "github.com/crossplane/cli/v2/apis/dev/v1alpha1" + "github.com/crossplane/cli/v2/internal/config" "github.com/crossplane/cli/v2/internal/dependency" "github.com/crossplane/cli/v2/internal/project/projectfile" + "github.com/crossplane/cli/v2/internal/schemas/generator" "github.com/crossplane/cli/v2/internal/terminal" clixpkg "github.com/crossplane/cli/v2/internal/xpkg" @@ -71,7 +73,7 @@ func (c *generateCmd) Help() string { } // AfterApply sets up the project filesystem. -func (c *generateCmd) AfterApply() error { +func (c *generateCmd) AfterApply(cfg *config.Config) error { projFilePath, err := filepath.Abs(c.ProjectFile) if err != nil { return err @@ -103,6 +105,10 @@ func (c *generateCmd) AfterApply() error { c.depManager = dependency.NewManager(proj, c.projFS, dependency.WithProjectFile(filepath.Base(c.ProjectFile)), + dependency.WithSchemaGenerators(generator.Filter( + generator.AllLanguages(generator.WithGoModelAccessors(cfg.Features.GenerateGoModelAccessors)), + proj.Spec.Schemas.GetLanguages(), + )), dependency.WithXpkgClient(client), dependency.WithResolver(resolver), ) diff --git a/cmd/crossplane/dependency/add.go b/cmd/crossplane/dependency/add.go index 13bb8bdb..80a49e08 100644 --- a/cmd/crossplane/dependency/add.go +++ b/cmd/crossplane/dependency/add.go @@ -28,8 +28,10 @@ import ( "github.com/crossplane/crossplane-runtime/v2/pkg/logging" "github.com/crossplane/cli/v2/apis/dev/v1alpha1" + "github.com/crossplane/cli/v2/internal/config" "github.com/crossplane/cli/v2/internal/dependency" "github.com/crossplane/cli/v2/internal/project/projectfile" + "github.com/crossplane/cli/v2/internal/schemas/generator" "github.com/crossplane/cli/v2/internal/terminal" clixpkg "github.com/crossplane/cli/v2/internal/xpkg" @@ -56,7 +58,7 @@ func (c *addCmd) Help() string { } // Run executes the add command. -func (c *addCmd) Run(logger logging.Logger, sp terminal.SpinnerPrinter) error { +func (c *addCmd) Run(logger logging.Logger, sp terminal.SpinnerPrinter, cfg *config.Config) error { ctx := context.Background() projFilePath, err := filepath.Abs(c.ProjectFile) @@ -88,6 +90,10 @@ func (c *addCmd) Run(logger logging.Logger, sp terminal.SpinnerPrinter) error { m := dependency.NewManager(proj, projFS, dependency.WithProjectFile(c.ProjectFile), + dependency.WithSchemaGenerators(generator.Filter( + generator.AllLanguages(generator.WithGoModelAccessors(cfg.Features.GenerateGoModelAccessors)), + proj.Spec.Schemas.GetLanguages(), + )), dependency.WithXpkgClient(client), dependency.WithResolver(resolver), ) diff --git a/cmd/crossplane/dependency/cache.go b/cmd/crossplane/dependency/cache.go index 16a2bd43..6ac9b577 100644 --- a/cmd/crossplane/dependency/cache.go +++ b/cmd/crossplane/dependency/cache.go @@ -27,6 +27,7 @@ import ( "github.com/crossplane/crossplane-runtime/v2/pkg/logging" "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/projectfile" "github.com/crossplane/cli/v2/internal/schemas/generator" @@ -52,7 +53,7 @@ func (c *updateCacheCmd) Help() string { } // Run executes the update-cache command. -func (c *updateCacheCmd) Run(logger logging.Logger, sp terminal.SpinnerPrinter) error { +func (c *updateCacheCmd) Run(logger logging.Logger, sp terminal.SpinnerPrinter, cfg *config.Config) error { ctx := context.Background() projFilePath, err := filepath.Abs(c.ProjectFile) @@ -84,7 +85,10 @@ func (c *updateCacheCmd) Run(logger logging.Logger, sp terminal.SpinnerPrinter) opts := []dependency.ManagerOption{ dependency.WithProjectFile(c.ProjectFile), - dependency.WithSchemaGenerators(generator.Filter(generator.AllLanguages(), proj.Spec.Schemas.GetLanguages())), + dependency.WithSchemaGenerators(generator.Filter( + generator.AllLanguages(generator.WithGoModelAccessors(cfg.Features.GenerateGoModelAccessors)), + proj.Spec.Schemas.GetLanguages(), + )), dependency.WithXpkgClient(client), dependency.WithResolver(resolver), } diff --git a/cmd/crossplane/render/op/cmd.go b/cmd/crossplane/render/op/cmd.go index d6dec277..f7f782c5 100644 --- a/cmd/crossplane/render/op/cmd.go +++ b/cmd/crossplane/render/op/cmd.go @@ -40,6 +40,7 @@ import ( "github.com/crossplane/cli/v2/cmd/crossplane/render" "github.com/crossplane/cli/v2/cmd/crossplane/render/contextfn" "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" @@ -101,7 +102,7 @@ func (c *Cmd) AfterApply() error { } // Run alpha render op. -func (c *Cmd) Run(k *kong.Context, log logging.Logger, sp terminal.SpinnerPrinter) error { //nolint:gocognit // Orchestration is inherently complex. +func (c *Cmd) Run(k *kong.Context, log logging.Logger, sp terminal.SpinnerPrinter, cfg *config.Config) error { //nolint:gocognit // Orchestration is inherently complex. ctx, cancel := context.WithTimeout(context.Background(), c.Timeout) defer cancel() @@ -149,7 +150,7 @@ func (c *Cmd) Run(k *kong.Context, log logging.Logger, sp terminal.SpinnerPrinte } // Load functions - fns, err := c.loadFunctions(ctx, log, sp) + fns, err := c.loadFunctions(ctx, log, sp, cfg) if err != nil { return err } @@ -317,7 +318,7 @@ func (c *Cmd) Run(k *kong.Context, log logging.Logger, sp terminal.SpinnerPrinte return nil } -func (c *Cmd) loadFunctions(ctx context.Context, log logging.Logger, sp terminal.SpinnerPrinter) ([]pkgv1.Function, error) { +func (c *Cmd) loadFunctions(ctx context.Context, log logging.Logger, sp terminal.SpinnerPrinter, cfg *config.Config) ([]pkgv1.Function, error) { if c.Functions != "" { fns, err := render.LoadFunctions(c.fs, c.Functions) if err != nil { @@ -359,8 +360,13 @@ func (c *Cmd) loadFunctions(ctx context.Context, log logging.Logger, sp terminal } resolver := clixpkg.NewResolver(xpkgClient) + // Built here rather than alongside the schema manager below so the + // dependency manager generates dependency schemas the same way. + generators := generator.AllLanguages(generator.WithGoModelAccessors(cfg.Features.GenerateGoModelAccessors)) + depMgr := dependency.NewManager(proj, projFS, dependency.WithProjectFile(filepath.Base(projFilePath)), + dependency.WithSchemaGenerators(generators), dependency.WithXpkgClient(xpkgClient), dependency.WithResolver(resolver), ) @@ -376,7 +382,6 @@ func (c *Cmd) loadFunctions(ctx context.Context, log logging.Logger, sp terminal if err := sp.WrapAsyncWithSuccessSpinners(func(ch async.EventChannel) error { schemasFS := afero.NewBasePathFs(projFS, proj.Spec.Paths.Schemas) - generators := generator.AllLanguages() schemaRunner := runner.NewRealSchemaRunner(runner.WithImageConfig(proj.Spec.ImageConfigs)) schemaMgr := manager.New(schemasFS, generators, schemaRunner) diff --git a/cmd/crossplane/render/op/cmd_test.go b/cmd/crossplane/render/op/cmd_test.go index 460e5bf3..ef8564a7 100644 --- a/cmd/crossplane/render/op/cmd_test.go +++ b/cmd/crossplane/render/op/cmd_test.go @@ -36,6 +36,7 @@ import ( pkgv1 "github.com/crossplane/crossplane/apis/v2/pkg/v1" "github.com/crossplane/cli/v2/cmd/crossplane/render" + "github.com/crossplane/cli/v2/internal/config" "github.com/crossplane/cli/v2/internal/terminal" renderv1alpha1 "github.com/crossplane/cli/v2/proto/render/v1alpha1" @@ -425,7 +426,7 @@ func TestCmdRun(t *testing.T) { buf := &bytes.Buffer{} kctx := &kong.Context{Kong: &kong.Kong{Stdout: buf, Stderr: io.Discard}} - err := tc.args.cmd.Run(kctx, logging.NewNopLogger(), terminal.NewSpinnerPrinter(io.Discard, false)) + err := tc.args.cmd.Run(kctx, logging.NewNopLogger(), terminal.NewSpinnerPrinter(io.Discard, false), &config.Config{}) if diff := cmp.Diff(tc.want.err, err, cmpopts.EquateErrors()); diff != "" { t.Errorf("\n%s\nRun(...): -want error, +got error:\n%s", tc.reason, diff) } diff --git a/cmd/crossplane/render/xr/cmd.go b/cmd/crossplane/render/xr/cmd.go index a7902230..b0aee54a 100644 --- a/cmd/crossplane/render/xr/cmd.go +++ b/cmd/crossplane/render/xr/cmd.go @@ -46,6 +46,7 @@ import ( "github.com/crossplane/cli/v2/cmd/crossplane/render" "github.com/crossplane/cli/v2/cmd/crossplane/render/contextfn" "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" @@ -111,7 +112,7 @@ func (c *Cmd) AfterApply() error { } // Run render. -func (c *Cmd) Run(k *kong.Context, log logging.Logger, sp terminal.SpinnerPrinter) error { //nolint:gocognit // Orchestration is inherently complex. +func (c *Cmd) Run(k *kong.Context, log logging.Logger, sp terminal.SpinnerPrinter, cfg *config.Config) error { //nolint:gocognit // Orchestration is inherently complex. ctx, cancel := context.WithTimeout(context.Background(), c.Timeout) defer cancel() @@ -157,7 +158,7 @@ func (c *Cmd) Run(k *kong.Context, log logging.Logger, sp terminal.SpinnerPrinte return errors.Errorf("render only supports Composition Function pipelines: Composition %q must use spec.mode: Pipeline", comp.GetName()) } - fns, err := c.loadFunctions(ctx, log, sp) + fns, err := c.loadFunctions(ctx, log, sp, cfg) if err != nil { return err } @@ -393,7 +394,7 @@ func (c *Cmd) Run(k *kong.Context, log logging.Logger, sp terminal.SpinnerPrinte return nil } -func (c *Cmd) loadFunctions(ctx context.Context, log logging.Logger, sp terminal.SpinnerPrinter) ([]pkgv1.Function, error) { +func (c *Cmd) loadFunctions(ctx context.Context, log logging.Logger, sp terminal.SpinnerPrinter, cfg *config.Config) ([]pkgv1.Function, error) { if c.Functions != "" { fns, err := render.LoadFunctions(c.fs, c.Functions) if err != nil { @@ -435,8 +436,13 @@ func (c *Cmd) loadFunctions(ctx context.Context, log logging.Logger, sp terminal } resolver := clixpkg.NewResolver(xpkgClient) + // Built here rather than alongside the schema manager below so the + // dependency manager generates dependency schemas the same way. + generators := generator.AllLanguages(generator.WithGoModelAccessors(cfg.Features.GenerateGoModelAccessors)) + depMgr := dependency.NewManager(proj, projFS, dependency.WithProjectFile(filepath.Base(projFilePath)), + dependency.WithSchemaGenerators(generators), dependency.WithXpkgClient(xpkgClient), dependency.WithResolver(resolver), ) @@ -452,7 +458,6 @@ func (c *Cmd) loadFunctions(ctx context.Context, log logging.Logger, sp terminal if err := sp.WrapAsyncWithSuccessSpinners(func(ch async.EventChannel) error { schemasFS := afero.NewBasePathFs(projFS, proj.Spec.Paths.Schemas) - generators := generator.AllLanguages() schemaRunner := runner.NewRealSchemaRunner(runner.WithImageConfig(proj.Spec.ImageConfigs)) schemaMgr := manager.New(schemasFS, generators, schemaRunner) diff --git a/cmd/crossplane/render/xr/cmd_test.go b/cmd/crossplane/render/xr/cmd_test.go index 74c0fd0e..9a98bae7 100644 --- a/cmd/crossplane/render/xr/cmd_test.go +++ b/cmd/crossplane/render/xr/cmd_test.go @@ -36,6 +36,7 @@ import ( pkgv1 "github.com/crossplane/crossplane/apis/v2/pkg/v1" "github.com/crossplane/cli/v2/cmd/crossplane/render" + "github.com/crossplane/cli/v2/internal/config" "github.com/crossplane/cli/v2/internal/terminal" renderv1alpha1 "github.com/crossplane/cli/v2/proto/render/v1alpha1" @@ -606,7 +607,7 @@ func TestCmdRun(t *testing.T) { buf := &bytes.Buffer{} kctx := &kong.Context{Kong: &kong.Kong{Stdout: buf, Stderr: io.Discard}} - err := tc.args.cmd.Run(kctx, logging.NewNopLogger(), terminal.NewSpinnerPrinter(io.Discard, false)) + err := tc.args.cmd.Run(kctx, logging.NewNopLogger(), terminal.NewSpinnerPrinter(io.Discard, false), &config.Config{}) if diff := cmp.Diff(tc.want.err, err, cmpopts.EquateErrors()); diff != "" { t.Errorf("\n%s\nRun(...): -want error, +got error:\n%s", tc.reason, diff) } diff --git a/internal/dependency/manager.go b/internal/dependency/manager.go index 79c946be..baa52261 100644 --- a/internal/dependency/manager.go +++ b/internal/dependency/manager.go @@ -125,7 +125,10 @@ func WithResolver(r *clixpkg.Resolver) ManagerOption { } } -// NewManager returns an initialized dependency manager. +// NewManager returns an initialized dependency manager. The default schema +// generators are configured with every generator option off, so a caller that +// generates schemas must pass WithSchemaGenerators to honor the feature flags +// in the user's config. func NewManager(proj *v1alpha1.Project, projFS afero.Fs, opts ...ManagerOption) *Manager { options := &managerOptions{ projFile: "crossplane-project.yaml",