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{},