diff --git a/pkg/generators/config.go b/pkg/generators/config.go index 8335ed82..1bcf2a52 100644 --- a/pkg/generators/config.go +++ b/pkg/generators/config.go @@ -69,7 +69,6 @@ func GetOpenAPITargets(context *generator.Context, args *args.Args, boilerplate newOpenAPIGen( args.OutputFile, args.OutputPkg, - len(args.OutputModelNameFile) > 0, ), newAPIViolationGen(), } diff --git a/pkg/generators/openapi.go b/pkg/generators/openapi.go index 621cc91d..5d58754a 100644 --- a/pkg/generators/openapi.go +++ b/pkg/generators/openapi.go @@ -127,19 +127,17 @@ const ( type openAPIGen struct { generator.GoGenerator // TargetPackage is the package that will get GetOpenAPIDefinitions function returns all open API definitions. - targetPackage string - imports namer.ImportTracker - useGeneratedModelNames bool + targetPackage string + imports namer.ImportTracker } -func newOpenAPIGen(outputFilename string, targetPackage string, useGeneratedModelNames bool) generator.Generator { +func newOpenAPIGen(outputFilename string, targetPackage string) generator.Generator { return &openAPIGen{ GoGenerator: generator.GoGenerator{ OutputFilename: outputFilename, }, - imports: generator.NewImportTrackerForPackage(targetPackage), - targetPackage: targetPackage, - useGeneratedModelNames: useGeneratedModelNames, + imports: generator.NewImportTrackerForPackage(targetPackage), + targetPackage: targetPackage, } } @@ -181,7 +179,7 @@ func (g *openAPIGen) Init(c *generator.Context, w io.Writer) error { sw.Do("return map[string]$.OpenAPIDefinition|raw${\n", argsFromType(nil)) for _, t := range c.Order { - err := newOpenAPITypeWriter(sw, c, g.useGeneratedModelNames).generateCall(t) + err := newOpenAPITypeWriter(sw, c).generateCall(t) if err != nil { return err } @@ -196,7 +194,7 @@ func (g *openAPIGen) Init(c *generator.Context, w io.Writer) error { func (g *openAPIGen) GenerateType(c *generator.Context, t *types.Type, w io.Writer) error { klog.V(5).Infof("generating for type %v", t) sw := generator.NewSnippetWriter(w, c, "$", "$") - err := newOpenAPITypeWriter(sw, c, g.useGeneratedModelNames).generate(t) + err := newOpenAPITypeWriter(sw, c).generate(t) if err != nil { return err } @@ -235,16 +233,14 @@ type openAPITypeWriter struct { refTypes map[string]*types.Type enumContext *enumContext GetDefinitionInterface *types.Type - useGeneratedModelNames bool } -func newOpenAPITypeWriter(sw *generator.SnippetWriter, c *generator.Context, useGeneratedModelNames bool) openAPITypeWriter { +func newOpenAPITypeWriter(sw *generator.SnippetWriter, c *generator.Context) openAPITypeWriter { return openAPITypeWriter{ - SnippetWriter: sw, - context: c, - refTypes: map[string]*types.Type{}, - enumContext: newEnumContext(c), - useGeneratedModelNames: useGeneratedModelNames, + SnippetWriter: sw, + context: c, + refTypes: map[string]*types.Type{}, + enumContext: newEnumContext(c), } } @@ -299,6 +295,40 @@ func hasOpenAPIV3OneOfMethod(t *types.Type) bool { return false } +func hasOpenAPIModelName(t *types.Type) bool { + for mn, mt := range t.Methods { + if mn != "OpenAPIModelName" { + continue + } + return methodReturnsValue(mt, "", "string") + } + return false +} + +func (g openAPITypeWriter) shouldUseOpenAPIModelName(t *types.Type) bool { + // Finds non-generated OpenAPIModelName() functions. + // Generated OpenAPIModelName() are ignored due to the 'ignore_autogenerated' build tag + // but are handled below by checking for use of the +k8s:openapi-model-package. + // This approach allows code generators to be called in any order. + if hasOpenAPIModelName(t) { + return true + } + + value, err := extractOpenAPISchemaNamePackage(t.CommentLines) + if err != nil { + klog.Fatalf("Type %v: invalid %s:%v", t, tagModelPackage, err) + } + if value != "" { + return true + } + pkg := g.context.Universe.Package(t.Name.Package) + value, err = extractOpenAPISchemaNamePackage(pkg.Comments) + if err != nil { + klog.Fatalf("Package %v: invalid %s:%v", pkg, tagModelPackage, err) + } + return value != "" +} + // typeShortName returns short package name (e.g. the name x appears in package x definition) dot type name. func typeShortName(t *types.Type) string { // `path` vs. `filepath` because packages use '/' @@ -349,7 +379,7 @@ func (g openAPITypeWriter) generateCall(t *types.Type) error { args := argsFromType(t) - if g.useGeneratedModelNames { + if g.shouldUseOpenAPIModelName(t) { g.Do("$.|raw${}.OpenAPIModelName(): ", t) } else { // Legacy case: use the "canonical type name" @@ -685,7 +715,7 @@ func (g openAPITypeWriter) generate(t *types.Type) error { g.Do("Dependencies: []string{\n", args) for _, k := range deps { t := g.refTypes[k] - if g.useGeneratedModelNames { + if g.shouldUseOpenAPIModelName(t) { g.Do("$.|raw${}.OpenAPIModelName(),", t) } else { g.Do("\"$.$\",", k) @@ -1051,7 +1081,7 @@ func (g openAPITypeWriter) generateSimpleProperty(typeString, format string) { func (g openAPITypeWriter) generateReferenceProperty(t *types.Type) { g.refTypes[t.Name.String()] = t - if g.useGeneratedModelNames { + if g.shouldUseOpenAPIModelName(t) { g.Do("Ref: ref($.|raw${}.OpenAPIModelName()),\n", t) } else { g.Do("Ref: ref(\"$.$\"),\n", t.Name.String()) diff --git a/pkg/generators/openapi_test.go b/pkg/generators/openapi_test.go index 92da8ad1..55be4c55 100644 --- a/pkg/generators/openapi_test.go +++ b/pkg/generators/openapi_test.go @@ -68,11 +68,11 @@ func testOpenAPITypeWriter(t *testing.T, cfg *packages.Config) (error, error, *b callBuffer := &bytes.Buffer{} callSW := generator.NewSnippetWriter(callBuffer, context, "$", "$") - callError := newOpenAPITypeWriter(callSW, context, false).generateCall(blahT) + callError := newOpenAPITypeWriter(callSW, context).generateCall(blahT) funcBuffer := &bytes.Buffer{} funcSW := generator.NewSnippetWriter(funcBuffer, context, "$", "$") - funcError := newOpenAPITypeWriter(funcSW, context, false).generate(blahT) + funcError := newOpenAPITypeWriter(funcSW, context).generate(blahT) return callError, funcError, callBuffer, funcBuffer, imports.ImportLines() } diff --git a/test/integration/naming_test.go b/test/integration/naming_test.go new file mode 100644 index 00000000..2a45d2de --- /dev/null +++ b/test/integration/naming_test.go @@ -0,0 +1,53 @@ +/* +Copyright 2024 The Kubernetes 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 integration + +import ( + "fmt" + "testing" + + "k8s.io/kube-openapi/pkg/util" + "k8s.io/kube-openapi/pkg/validation/spec" + generated "k8s.io/kube-openapi/test/integration/pkg/generated/namedmodels" + "k8s.io/kube-openapi/test/integration/testdata/namedmodels" +) + +func TestCanonicalTypeNames(t *testing.T) { + defs := generated.GetOpenAPIDefinitions(func(path string) spec.Ref { + return spec.MustCreateRef(path) + }) + + // Define the types we want to check. + typeTestCases := []any{namedmodels.Struct{}, namedmodels.ContainedStruct{}, namedmodels.AtomicStruct{}} + + for _, v := range typeTestCases { + t.Run(fmt.Sprintf("%T", v), func(t *testing.T) { + // Get the canonical name using the generator's logic. + canonicalName := util.GetCanonicalTypeName(v) + + // Check if the canonical name exists as a key in the generated map. + if _, ok := defs[canonicalName]; !ok { + t.Errorf("canonical type name %q for type %q not found in GetOpenAPIDefinitions", canonicalName, v) + } + }) + } + + // Additionally, verify that the number of generated definitions matches our expectation. + if len(defs) != len(typeTestCases) { + t.Errorf("expected %d definitions, but got %d", len(typeTestCases), len(defs)) + } +} diff --git a/test/integration/pkg/generated/namedmodels/openapi_generated.go b/test/integration/pkg/generated/namedmodels/openapi_generated.go index ee514531..ab238ac2 100644 --- a/test/integration/pkg/generated/namedmodels/openapi_generated.go +++ b/test/integration/pkg/generated/namedmodels/openapi_generated.go @@ -29,9 +29,9 @@ import ( func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenAPIDefinition { return map[string]common.OpenAPIDefinition{ - testdatanamedmodels.AtomicStruct{}.OpenAPIModelName(): schema_test_integration_testdata_namedmodels_AtomicStruct(ref), - testdatanamedmodels.ContainedStruct{}.OpenAPIModelName(): schema_test_integration_testdata_namedmodels_ContainedStruct(ref), - testdatanamedmodels.Struct{}.OpenAPIModelName(): schema_test_integration_testdata_namedmodels_Struct(ref), + testdatanamedmodels.AtomicStruct{}.OpenAPIModelName(): schema_test_integration_testdata_namedmodels_AtomicStruct(ref), + "k8s.io/kube-openapi/test/integration/testdata/namedmodels.ContainedStruct": schema_test_integration_testdata_namedmodels_ContainedStruct(ref), + testdatanamedmodels.Struct{}.OpenAPIModelName(): schema_test_integration_testdata_namedmodels_Struct(ref), } } @@ -43,9 +43,10 @@ func schema_test_integration_testdata_namedmodels_AtomicStruct(ref common.Refere Properties: map[string]spec.Schema{ "Field": { SchemaProps: spec.SchemaProps{ - Default: 0, - Type: []string{"integer"}, - Format: "int32", + Description: "The generator should respect a manually declared OpenAPIModelName.", + Default: 0, + Type: []string{"integer"}, + Format: "int32", }, }, }, @@ -73,8 +74,9 @@ func schema_test_integration_testdata_namedmodels_Struct(ref common.ReferenceCal Properties: map[string]spec.Schema{ "Field": { SchemaProps: spec.SchemaProps{ - Default: map[string]interface{}{}, - Ref: ref(testdatanamedmodels.ContainedStruct{}.OpenAPIModelName()), + Description: "The generator should see the +k8s:openapi-model-package and assume that a OpenAPIModelName is (or will be) generated.", + Default: map[string]interface{}{}, + Ref: ref("k8s.io/kube-openapi/test/integration/testdata/namedmodels.ContainedStruct"), }, }, "OtherField": { @@ -89,6 +91,6 @@ func schema_test_integration_testdata_namedmodels_Struct(ref common.ReferenceCal }, }, Dependencies: []string{ - testdatanamedmodels.ContainedStruct{}.OpenAPIModelName()}, + "k8s.io/kube-openapi/test/integration/testdata/namedmodels.ContainedStruct"}, } } diff --git a/test/integration/testdata/namedmodels/doc.go b/test/integration/testdata/namedmodels/doc.go index afc56e58..4cbfb1d5 100644 --- a/test/integration/testdata/namedmodels/doc.go +++ b/test/integration/testdata/namedmodels/doc.go @@ -1,3 +1,2 @@ // +k8s:openapi-gen=true -// +k8s:openapi-model-package=io.k8s.kube-openapi.test.integration.testdata.namedmodels package namedmodels diff --git a/test/integration/testdata/namedmodels/struct.go b/test/integration/testdata/namedmodels/struct.go index 7a136a10..94fab054 100644 --- a/test/integration/testdata/namedmodels/struct.go +++ b/test/integration/testdata/namedmodels/struct.go @@ -1,12 +1,18 @@ package namedmodels -type Struct struct { +// +k8s:openapi-model-package=io.k8s.kube-openapi.test.integration.testdata.namedmodels +type Struct struct { // The generator should see the +k8s:openapi-model-package and assume that a OpenAPIModelName is (or will be) generated. Field ContainedStruct OtherField int } -type ContainedStruct struct{} +type ContainedStruct struct{} // The generator should use the go package name since there is no OpenAPIModelName declared. -type AtomicStruct struct { +type AtomicStruct struct { // The generator should respect a manually declared OpenAPIModelName. Field int } + +// OpenAPIModelName returns the OpenAPI model name for this type. +func (in AtomicStruct) OpenAPIModelName() string { + return "io.k8s.kube-openapi.test.integration.testdata.namedmodels.AtomicStruct" +} diff --git a/test/integration/testdata/namedmodels/zz_generated_model_name.go b/test/integration/testdata/namedmodels/zz_generated_model_name.go index 0c2dcbd1..9c010cad 100644 --- a/test/integration/testdata/namedmodels/zz_generated_model_name.go +++ b/test/integration/testdata/namedmodels/zz_generated_model_name.go @@ -21,16 +21,6 @@ limitations under the License. package namedmodels -// OpenAPIModelName returns the OpenAPI model name for this type. -func (in AtomicStruct) OpenAPIModelName() string { - return "io.k8s.kube-openapi.test.integration.testdata.namedmodels.AtomicStruct" -} - -// OpenAPIModelName returns the OpenAPI model name for this type. -func (in ContainedStruct) OpenAPIModelName() string { - return "io.k8s.kube-openapi.test.integration.testdata.namedmodels.ContainedStruct" -} - // OpenAPIModelName returns the OpenAPI model name for this type. func (in Struct) OpenAPIModelName() string { return "io.k8s.kube-openapi.test.integration.testdata.namedmodels.Struct"