From fe10d9b1425b50512b9e376b14f86f4a59e4df1e Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Tue, 29 Aug 2023 09:44:44 -0700 Subject: [PATCH 1/4] update gengo --- go.mod | 2 +- go.sum | 4 ++-- test/integration/go.mod | 2 +- test/integration/go.sum | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 0061b576e..f083109ae 100644 --- a/go.mod +++ b/go.mod @@ -21,7 +21,7 @@ require ( google.golang.org/protobuf v1.27.1 gopkg.in/yaml.v2 v2.4.0 gopkg.in/yaml.v3 v3.0.1 - k8s.io/gengo v0.0.0-20210813121822-485abfe95c7c + k8s.io/gengo v0.0.0-20230829151522-9cce18d56c01 k8s.io/klog/v2 v2.2.0 k8s.io/utils v0.0.0-20210802155522-efc7438f0176 sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd diff --git a/go.sum b/go.sum index 2be67da49..861021bd9 100644 --- a/go.sum +++ b/go.sum @@ -110,8 +110,8 @@ gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -k8s.io/gengo v0.0.0-20210813121822-485abfe95c7c h1:GohjlNKauSai7gN4wsJkeZ3WAJx4Sh+oT/b5IYn5suA= -k8s.io/gengo v0.0.0-20210813121822-485abfe95c7c/go.mod h1:FiNAH4ZV3gBg2Kwh89tzAEV2be7d5xI0vBa/VySYy3E= +k8s.io/gengo v0.0.0-20230829151522-9cce18d56c01 h1:pWEwq4Asjm4vjW7vcsmijwBhOr1/shsbSYiWXmNGlks= +k8s.io/gengo v0.0.0-20230829151522-9cce18d56c01/go.mod h1:FiNAH4ZV3gBg2Kwh89tzAEV2be7d5xI0vBa/VySYy3E= k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE= k8s.io/klog/v2 v2.2.0 h1:XRvcwJozkgZ1UQJmfMGpvRthQHOvihEhYtDfAaxMz/A= k8s.io/klog/v2 v2.2.0/go.mod h1:Od+F08eJP+W3HUb4pSrPpgp9DGU4GzlpG/TmITuYh/Y= diff --git a/test/integration/go.mod b/test/integration/go.mod index 6bd5c19ce..d0c0774bd 100644 --- a/test/integration/go.mod +++ b/test/integration/go.mod @@ -34,7 +34,7 @@ require ( google.golang.org/protobuf v1.27.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - k8s.io/gengo v0.0.0-20210813121822-485abfe95c7c // indirect + k8s.io/gengo v0.0.0-20230829151522-9cce18d56c01 // indirect k8s.io/klog/v2 v2.2.0 // indirect ) diff --git a/test/integration/go.sum b/test/integration/go.sum index f466b5a6e..c65fdb224 100644 --- a/test/integration/go.sum +++ b/test/integration/go.sum @@ -113,8 +113,8 @@ gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -k8s.io/gengo v0.0.0-20210813121822-485abfe95c7c h1:GohjlNKauSai7gN4wsJkeZ3WAJx4Sh+oT/b5IYn5suA= -k8s.io/gengo v0.0.0-20210813121822-485abfe95c7c/go.mod h1:FiNAH4ZV3gBg2Kwh89tzAEV2be7d5xI0vBa/VySYy3E= +k8s.io/gengo v0.0.0-20230829151522-9cce18d56c01 h1:pWEwq4Asjm4vjW7vcsmijwBhOr1/shsbSYiWXmNGlks= +k8s.io/gengo v0.0.0-20230829151522-9cce18d56c01/go.mod h1:FiNAH4ZV3gBg2Kwh89tzAEV2be7d5xI0vBa/VySYy3E= k8s.io/klog/v2 v2.2.0 h1:XRvcwJozkgZ1UQJmfMGpvRthQHOvihEhYtDfAaxMz/A= k8s.io/klog/v2 v2.2.0/go.mod h1:Od+F08eJP+W3HUb4pSrPpgp9DGU4GzlpG/TmITuYh/Y= sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo= From 3b85353e19c4e9c90e78c49e021b36b57828836c Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Tue, 29 Aug 2023 10:15:03 -0700 Subject: [PATCH 2/4] add support for symbol references in defaults refactor openapi test to return import lines add symbol reference test --- pkg/generators/openapi.go | 33 ++++--- pkg/generators/openapi_test.go | 155 ++++++++++++++++++++++++++------- 2 files changed, 146 insertions(+), 42 deletions(-) diff --git a/pkg/generators/openapi.go b/pkg/generators/openapi.go index 4654bbe9c..21b5ec321 100644 --- a/pkg/generators/openapi.go +++ b/pkg/generators/openapi.go @@ -26,6 +26,7 @@ import ( "sort" "strings" + defaultergen "k8s.io/gengo/examples/defaulter-gen/generators" "k8s.io/gengo/generator" "k8s.io/gengo/namer" "k8s.io/gengo/types" @@ -120,7 +121,7 @@ func newOpenAPIGen(sanitizedName string, targetPackage string) generator.Generat DefaultGen: generator.DefaultGen{ OptionalName: sanitizedName, }, - imports: generator.NewImportTracker(), + imports: generator.NewImportTrackerForPackage(targetPackage), targetPackage: targetPackage, } } @@ -553,19 +554,26 @@ func (g openAPITypeWriter) validatePatchTags(m *types.Member, parent *types.Type return nil } -func defaultFromComments(comments []string) (interface{}, error) { +func defaultFromComments(comments []string, commentPath string) (interface{}, *types.Name, error) { tag, err := getSingleTagsValue(comments, tagDefault) if tag == "" { - return nil, err + return nil, nil, err } var i interface{} - if err := json.Unmarshal([]byte(tag), &i); err != nil { - return nil, fmt.Errorf("failed to unmarshal default: %v", err) + if id, ok := defaultergen.ParseSymbolReference(tag, commentPath); ok { + return nil, &id, nil + } else if err := json.Unmarshal([]byte(tag), &i); err != nil { + return nil, nil, fmt.Errorf("failed to unmarshal default: %v", err) } - return i, nil + return i, nil, nil } func mustEnforceDefault(t *types.Type, omitEmpty bool) (interface{}, error) { + // If t implements custom JSON marshalling, all of this logic is likely wrong + if _, isUnmarshaller := t.Methods["UnmarshalJSON"]; isUnmarshaller { + return nil, nil + } + switch t.Kind { case types.Pointer, types.Map, types.Slice, types.Array, types.Interface: return nil, nil @@ -585,9 +593,10 @@ func mustEnforceDefault(t *types.Type, omitEmpty bool) (interface{}, error) { } } -func (g openAPITypeWriter) generateDefault(comments []string, t *types.Type, omitEmpty bool) error { +func (g openAPITypeWriter) generateDefault(comments []string, t *types.Type, omitEmpty bool, commentOwningType *types.Type) error { t = resolveAliasAndEmbeddedType(t) - def, err := defaultFromComments(comments) + + def, ref, err := defaultFromComments(comments, commentOwningType.Name.Package) if err != nil { return err } @@ -603,6 +612,8 @@ func (g openAPITypeWriter) generateDefault(comments []string, t *types.Type, omi } if def != nil { g.Do("Default: $.$,\n", fmt.Sprintf("%#v", def)) + } else if ref != nil { + g.Do("Default: $.|raw$,\n", &types.Type{Name: *ref}) } return nil } @@ -676,7 +687,7 @@ func (g openAPITypeWriter) generateProperty(m *types.Member, parent *types.Type) return nil } omitEmpty := strings.Contains(reflect.StructTag(m.Tags).Get("json"), "omitempty") - if err := g.generateDefault(m.CommentLines, m.Type, omitEmpty); err != nil { + if err := g.generateDefault(m.CommentLines, m.Type, omitEmpty, parent); err != nil { return fmt.Errorf("failed to generate default in %v: %v: %v", parent, m.Name, err) } t := resolveAliasAndPtrType(m.Type) @@ -762,7 +773,7 @@ func (g openAPITypeWriter) generateMapProperty(t *types.Type) error { g.Do("Type: []string{\"object\"},\n", nil) g.Do("AdditionalProperties: &spec.SchemaOrBool{\nAllows: true,\nSchema: &spec.Schema{\nSchemaProps: spec.SchemaProps{\n", nil) - if err := g.generateDefault(t.Elem.CommentLines, t.Elem, false); err != nil { + if err := g.generateDefault(t.Elem.CommentLines, t.Elem, false, t.Elem); err != nil { return err } typeString, format := openapi.OpenAPITypeFormat(elemType.String()) @@ -795,7 +806,7 @@ func (g openAPITypeWriter) generateSliceProperty(t *types.Type) error { elemType := resolveAliasAndPtrType(t.Elem) g.Do("Type: []string{\"array\"},\n", nil) g.Do("Items: &spec.SchemaOrArray{\nSchema: &spec.Schema{\nSchemaProps: spec.SchemaProps{\n", nil) - if err := g.generateDefault(t.Elem.CommentLines, t.Elem, false); err != nil { + if err := g.generateDefault(t.Elem.CommentLines, t.Elem, false, t.Elem); err != nil { return err } typeString, format := openapi.OpenAPITypeFormat(elemType.String()) diff --git a/pkg/generators/openapi_test.go b/pkg/generators/openapi_test.go index c5d424d83..1a48bff20 100644 --- a/pkg/generators/openapi_test.go +++ b/pkg/generators/openapi_test.go @@ -23,8 +23,9 @@ import ( "strings" "testing" - "github.com/stretchr/testify/assert" + "go/format" + "github.com/stretchr/testify/assert" "k8s.io/gengo/generator" "k8s.io/gengo/namer" "k8s.io/gengo/parser" @@ -47,14 +48,16 @@ func construct(t *testing.T, files map[string]string, testNamer namer.Namer) (*p return b, u, o } -func testOpenAPITypeWriter(t *testing.T, code string) (error, error, *assert.Assertions, *bytes.Buffer, *bytes.Buffer) { +func testOpenAPITypeWriter(t *testing.T, code string) (error, error, *assert.Assertions, *bytes.Buffer, *bytes.Buffer, []string) { assert := assert.New(t) var testFiles = map[string]string{ "base/foo/bar.go": code, } - rawNamer := namer.NewRawNamer("o", nil) + outputPackage := "base/output" + imports := generator.NewImportTrackerForPackage(outputPackage) + rawNamer := namer.NewRawNamer(outputPackage, imports) namers := namer.NameSystems{ - "raw": namer.NewRawNamer("", nil), + "raw": rawNamer, "private": &namer.NameStrategy{ Join: func(pre string, in []string, post string) string { return strings.Join(in, "_") @@ -77,11 +80,11 @@ func testOpenAPITypeWriter(t *testing.T, code string) (error, error, *assert.Ass funcSW := generator.NewSnippetWriter(funcBuffer, context, "$", "$") funcError := newOpenAPITypeWriter(funcSW, context).generate(blahT) - return callError, funcError, assert, callBuffer, funcBuffer + return callError, funcError, assert, callBuffer, funcBuffer, imports.ImportLines() } func TestSimple(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo // Blah is a test. @@ -381,7 +384,7 @@ Extensions: spec.Extensions{ } func TestEmptyProperties(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo // Blah demonstrate a struct without fields. @@ -411,7 +414,7 @@ Type: []string{"object"}, } func TestNestedStruct(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo // Nested is used as struct field @@ -461,7 +464,7 @@ Dependencies: []string{ } func TestNestedStructPointer(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo // Nested is used as struct pointer field @@ -510,7 +513,7 @@ Dependencies: []string{ } func TestEmbeddedStruct(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo // Nested is used as embedded struct field @@ -560,7 +563,7 @@ Dependencies: []string{ } func TestSingleEmbeddedStruct(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo import "time" @@ -612,7 +615,7 @@ Dependencies: []string{ } func TestEmbeddedInlineStruct(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo // Nested is used as embedded inline struct field @@ -661,7 +664,7 @@ Required: []string{"String"}, } func TestEmbeddedInlineStructPointer(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo // Nested is used as embedded inline struct pointer field. @@ -710,7 +713,7 @@ Required: []string{"String"}, } func TestNestedMapString(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo // Map sample tests openAPIGen.generateMapProperty method. @@ -769,7 +772,7 @@ Required: []string{"StringToArray"}, } func TestNestedMapInt(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo // Map sample tests openAPIGen.generateMapProperty method. @@ -828,7 +831,7 @@ Required: []string{"StringToArray"}, } func TestNestedMapBoolean(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo // Map sample tests openAPIGen.generateMapProperty method. @@ -887,7 +890,7 @@ Required: []string{"StringToArray"}, } func TestFailingSample1(t *testing.T) { - _, funcErr, assert, _, _ := testOpenAPITypeWriter(t, ` + _, funcErr, assert, _, _, _ := testOpenAPITypeWriter(t, ` package foo // Map sample tests openAPIGen.generateMapProperty method. @@ -902,7 +905,7 @@ type Blah struct { } func TestFailingSample2(t *testing.T) { - _, funcErr, assert, _, _ := testOpenAPITypeWriter(t, ` + _, funcErr, assert, _, _, _ := testOpenAPITypeWriter(t, ` package foo // Map sample tests openAPIGen.generateMapProperty method. @@ -972,7 +975,7 @@ type Item string `, for i, test := range tests { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { - _, funcErr, assert, _, _ := testOpenAPITypeWriter(t, test.definition) + _, funcErr, assert, _, _, _ := testOpenAPITypeWriter(t, test.definition) if assert.Error(funcErr, "An error was expected") { assert.Equal(funcErr, test.expectedError) } @@ -981,7 +984,7 @@ type Item string `, } func TestCustomDef(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo import openapi "k8s.io/kube-openapi/pkg/common" @@ -1012,7 +1015,7 @@ func (_ Blah) OpenAPIDefinition() openapi.OpenAPIDefinition { } func TestCustomDefV3(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo import openapi "k8s.io/kube-openapi/pkg/common" @@ -1043,7 +1046,7 @@ func (_ Blah) OpenAPIV3Definition() openapi.OpenAPIDefinition { } func TestCustomDefV2AndV3(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo import openapi "k8s.io/kube-openapi/pkg/common" @@ -1085,7 +1088,7 @@ func (_ Blah) OpenAPIDefinition() openapi.OpenAPIDefinition { } func TestCustomDefs(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo // Blah is a custom type @@ -1119,7 +1122,7 @@ Format:foo.Blah{}.OpenAPISchemaFormat(), } func TestCustomDefsV3(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo import openapi "k8s.io/kube-openapi/pkg/common" @@ -1166,7 +1169,7 @@ Format:foo.Blah{}.OpenAPISchemaFormat(), } func TestV3OneOfTypes(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo // Blah is a custom type @@ -1210,7 +1213,7 @@ Format:foo.Blah{}.OpenAPISchemaFormat(), } func TestPointer(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo // PointerSample demonstrate pointer's properties @@ -1297,7 +1300,7 @@ Dependencies: []string{ } func TestNestedLists(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo // Blah is a test. @@ -1361,7 +1364,7 @@ Extensions: spec.Extensions{ } func TestNestListOfMaps(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo // Blah is a test. @@ -1433,7 +1436,7 @@ Extensions: spec.Extensions{ } func TestExtensions(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo // Blah is a test. @@ -1530,7 +1533,7 @@ Extensions: spec.Extensions{ } func TestUnion(t *testing.T) { - callErr, funcErr, assert, callBuffer, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo // Blah is a test. @@ -1616,7 +1619,7 @@ map[string]interface{}{ } func TestEnum(t *testing.T) { - callErr, funcErr, assert, _, funcBuffer := testOpenAPITypeWriter(t, ` + callErr, funcErr, assert, _, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo // EnumType is the enumType. @@ -1692,3 +1695,93 @@ Extensions: spec.Extensions{ `, funcBuffer.String()) } + +func TestSymbolReference(t *testing.T) { + callErr, funcErr, assert, _, funcBuffer, imports := testOpenAPITypeWriter(t, ` +package foo + +// +k8s:openapi-gen=true +type Blah struct { + // +default="A Default Value" + // +optional + Value *string + + // User constant local to the output package fully qualified + // +default=ref(base/output.MyConst) + // +optional + FullyQualifiedOutputValue *string + + // Local to types but not to output + // +default=ref(MyConst) + // +optional + LocalValue *string + + // +default=ref(base/foo.MyConst) + // +optional + FullyQualifiedLocalValue *string + + // +default=ref(k8s.io/api/v1.TerminationPathDefault) + // +optional + FullyQualifiedExternalValue *string +} + `) + assert.NoError(funcErr) + assert.NoError(callErr) + assert.ElementsMatch(imports, []string{`v1 "k8s.io/api/v1"`, `foo "base/foo"`, `common "k8s.io/kube-openapi/pkg/common"`, `spec "k8s.io/kube-openapi/pkg/validation/spec"`}) + + if formatted, err := format.Source(funcBuffer.Bytes()); err != nil { + t.Fatal(err) + } else { + assert.Equal(string(formatted), `func schema_base_foo_Blah(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "Value": { + SchemaProps: spec.SchemaProps{ + Default: "A Default Value", + Type: []string{"string"}, + Format: "", + }, + }, + "FullyQualifiedOutputValue": { + SchemaProps: spec.SchemaProps{ + Description: "User constant local to the output package fully qualified", + Default: MyConst, + Type: []string{"string"}, + Format: "", + }, + }, + "LocalValue": { + SchemaProps: spec.SchemaProps{ + Description: "Local to types but not to output", + Default: foo.MyConst, + Type: []string{"string"}, + Format: "", + }, + }, + "FullyQualifiedLocalValue": { + SchemaProps: spec.SchemaProps{ + Default: foo.MyConst, + Type: []string{"string"}, + Format: "", + }, + }, + "FullyQualifiedExternalValue": { + SchemaProps: spec.SchemaProps{ + Default: v1.TerminationPathDefault, + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, + } +} + +`) + } + +} From 0f86ea7185b7b41b01d05c33c7d6d0bc8e2e21b8 Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Thu, 31 Aug 2023 14:02:46 -0700 Subject: [PATCH 3/4] add support for getting nested default --- pkg/generators/openapi.go | 43 ++++++++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/pkg/generators/openapi.go b/pkg/generators/openapi.go index 21b5ec321..6b5c6997b 100644 --- a/pkg/generators/openapi.go +++ b/pkg/generators/openapi.go @@ -554,13 +554,39 @@ func (g openAPITypeWriter) validatePatchTags(m *types.Member, parent *types.Type return nil } -func defaultFromComments(comments []string, commentPath string) (interface{}, *types.Name, error) { - tag, err := getSingleTagsValue(comments, tagDefault) +func defaultFromComments(comments []string, commentPath string, t *types.Type) (interface{}, *types.Name, error) { + var tag string + + for { + var err error + tag, err = getSingleTagsValue(comments, tagDefault) + if err != nil { + return nil, nil, err + } + + if t == nil || len(tag) > 0 { + break + } + + comments = t.CommentLines + commentPath = t.Name.Package + switch t.Kind { + case types.Pointer: + t = t.Elem + case types.Alias: + t = t.Underlying + default: + t = nil + } + } + if tag == "" { - return nil, nil, err + return nil, nil, nil } + var i interface{} if id, ok := defaultergen.ParseSymbolReference(tag, commentPath); ok { + klog.Errorf("%v, %v", id, commentPath) return nil, &id, nil } else if err := json.Unmarshal([]byte(tag), &i); err != nil { return nil, nil, fmt.Errorf("failed to unmarshal default: %v", err) @@ -569,11 +595,6 @@ func defaultFromComments(comments []string, commentPath string) (interface{}, *t } func mustEnforceDefault(t *types.Type, omitEmpty bool) (interface{}, error) { - // If t implements custom JSON marshalling, all of this logic is likely wrong - if _, isUnmarshaller := t.Methods["UnmarshalJSON"]; isUnmarshaller { - return nil, nil - } - switch t.Kind { case types.Pointer, types.Map, types.Slice, types.Array, types.Interface: return nil, nil @@ -594,13 +615,11 @@ func mustEnforceDefault(t *types.Type, omitEmpty bool) (interface{}, error) { } func (g openAPITypeWriter) generateDefault(comments []string, t *types.Type, omitEmpty bool, commentOwningType *types.Type) error { - t = resolveAliasAndEmbeddedType(t) - - def, ref, err := defaultFromComments(comments, commentOwningType.Name.Package) + def, ref, err := defaultFromComments(comments, commentOwningType.Name.Package, t) if err != nil { return err } - if enforced, err := mustEnforceDefault(t, omitEmpty); err != nil { + if enforced, err := mustEnforceDefault(resolveAliasAndEmbeddedType(t), omitEmpty); err != nil { return err } else if enforced != nil { if def == nil { From 29ed3ba61eb8e7a153fed938288292b42f8b4271 Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Thu, 31 Aug 2023 14:02:50 -0700 Subject: [PATCH 4/4] add integration test --- .../pkg/generated/openapi_generated.go | 42 ++++++++++++++++++- test/integration/testdata/defaults/default.go | 15 +++++++ test/integration/testdata/golden.v2.json | 20 +++++++++ test/integration/testdata/golden.v3.json | 20 +++++++++ 4 files changed, 95 insertions(+), 2 deletions(-) diff --git a/test/integration/pkg/generated/openapi_generated.go b/test/integration/pkg/generated/openapi_generated.go index d9dc25cbf..436da767e 100644 --- a/test/integration/pkg/generated/openapi_generated.go +++ b/test/integration/pkg/generated/openapi_generated.go @@ -17,7 +17,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -// Code generated by openapi-gen.go. DO NOT EDIT. +// Code generated by openapi-gen. DO NOT EDIT. // This file was autogenerated by openapi-gen. Do not edit it manually! @@ -27,6 +27,8 @@ import ( common "k8s.io/kube-openapi/pkg/common" spec "k8s.io/kube-openapi/pkg/validation/spec" custom "k8s.io/kube-openapi/test/integration/testdata/custom" + defaults "k8s.io/kube-openapi/test/integration/testdata/defaults" + enumtype "k8s.io/kube-openapi/test/integration/testdata/enumtype" ) func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenAPIDefinition { @@ -157,6 +159,41 @@ func schema_test_integration_testdata_defaults_Defaulted(ref common.ReferenceCal }, }, }, + "localSymbolReference": { + SchemaProps: spec.SchemaProps{ + Default: defaults.ConstantValue, + Type: []string{"string"}, + Format: "", + }, + }, + "fullyQualifiedSymbolReference": { + SchemaProps: spec.SchemaProps{ + Default: defaults.ConstantValue, + Type: []string{"string"}, + Format: "", + }, + }, + "externalSymbolReference": { + SchemaProps: spec.SchemaProps{ + Default: enumtype.FruitApple, + Type: []string{"string"}, + Format: "", + }, + }, + "pointerConversionSymbolReference": { + SchemaProps: spec.SchemaProps{ + Default: enumtype.FruitApple, + Type: []string{"string"}, + Format: "", + }, + }, + "defaultedAliasSymbolReference": { + SchemaProps: spec.SchemaProps{ + Default: defaults.ConstantValue, + Type: []string{"string"}, + Format: "", + }, + }, }, Required: []string{"OtherField", "List", "Sub", "OtherSub", "Map"}, }, @@ -341,7 +378,8 @@ func schema_test_integration_testdata_enumtype_FruitsBasket(ref common.Reference Default: "", Type: []string{"string"}, Format: "", - Enum: []interface{}{"apple", "banana", "onigiri"}}, + Enum: []interface{}{"apple", "banana", "onigiri"}, + }, }, "count": { SchemaProps: spec.SchemaProps{ diff --git a/test/integration/testdata/defaults/default.go b/test/integration/testdata/defaults/default.go index abc08099c..61e503e67 100644 --- a/test/integration/testdata/defaults/default.go +++ b/test/integration/testdata/defaults/default.go @@ -15,8 +15,23 @@ type Defaulted struct { // +default={"foo": "bar"} Map map[string]Item + + // +default=ref(ConstantValue) + LocalSymbolReference string `json:"localSymbolReference,omitempty"` + // +default=ref(k8s.io/kube-openapi/test/integration/testdata/defaults.ConstantValue) + FullyQualifiedSymbolReference string `json:"fullyQualifiedSymbolReference,omitempty"` + // +default=ref(k8s.io/kube-openapi/test/integration/testdata/enumtype.FruitApple) + ExternalSymbolReference string `json:"externalSymbolReference,omitempty"` + // +default=ref(k8s.io/kube-openapi/test/integration/testdata/enumtype.FruitApple) + PointerConversionSymbolReference *DefaultedItem `json:"pointerConversionSymbolReference,omitempty"` + DefaultedAliasSymbolReference DefaultedItem `json:"defaultedAliasSymbolReference,omitempty"` } +const ConstantValue string = "SymbolConstant" + +// +default=ref(ConstantValue) +type DefaultedItem string + // +k8s:openapi-gen=true type Item string diff --git a/test/integration/testdata/golden.v2.json b/test/integration/testdata/golden.v2.json index 526b5a128..ce35021cb 100644 --- a/test/integration/testdata/golden.v2.json +++ b/test/integration/testdata/golden.v2.json @@ -670,6 +670,26 @@ "s": "foo" }, "$ref": "#/definitions/defaults.SubStruct" + }, + "defaultedAliasSymbolReference": { + "type": "string", + "default": "SymbolConstant" + }, + "externalSymbolReference": { + "type": "string", + "default": "apple" + }, + "fullyQualifiedSymbolReference": { + "type": "string", + "default": "SymbolConstant" + }, + "localSymbolReference": { + "type": "string", + "default": "SymbolConstant" + }, + "pointerConversionSymbolReference": { + "type": "string", + "default": "apple" } } }, diff --git a/test/integration/testdata/golden.v3.json b/test/integration/testdata/golden.v3.json index 456f5a81c..cbbc55267 100644 --- a/test/integration/testdata/golden.v3.json +++ b/test/integration/testdata/golden.v3.json @@ -628,6 +628,26 @@ "$ref": "#/components/schemas/defaults.SubStruct" } ] + }, + "defaultedAliasSymbolReference": { + "type": "string", + "default": "SymbolConstant" + }, + "externalSymbolReference": { + "type": "string", + "default": "apple" + }, + "fullyQualifiedSymbolReference": { + "type": "string", + "default": "SymbolConstant" + }, + "localSymbolReference": { + "type": "string", + "default": "SymbolConstant" + }, + "pointerConversionSymbolReference": { + "type": "string", + "default": "apple" } } },