From 23431b481b203036cb9ad2ac4d253e374d765abe Mon Sep 17 00:00:00 2001 From: Maria Ntalla Date: Wed, 6 Nov 2019 10:18:30 +0000 Subject: [PATCH 1/7] Support +mapType, +structType markers and convert both of them to openapi extension: x-kubernetes-map-type. Also adding minor changes to the integration tests README to ensure that it's up to date after recent changes that add v2 support (PR #166). wip --- pkg/generators/extension.go | 10 ++++ pkg/generators/extension_test.go | 58 +++++++++++++++++++ pkg/idl/doc.go | 2 +- test/integration/README.md | 10 ++-- test/integration/integration_suite_test.go | 2 + test/integration/testdata/golden.v2.report | 6 ++ .../testdata/maptype/atomic-map.go | 7 +++ .../testdata/maptype/granular-map.go | 7 +++ .../testdata/structtype/atomic-struct.go | 8 +++ .../testdata/structtype/granular-struct.go | 8 +++ 10 files changed, 112 insertions(+), 6 deletions(-) create mode 100644 test/integration/testdata/maptype/atomic-map.go create mode 100644 test/integration/testdata/maptype/granular-map.go create mode 100644 test/integration/testdata/structtype/atomic-struct.go create mode 100644 test/integration/testdata/structtype/granular-struct.go diff --git a/pkg/generators/extension.go b/pkg/generators/extension.go index 14eab18f6..9c3b5c4fd 100644 --- a/pkg/generators/extension.go +++ b/pkg/generators/extension.go @@ -54,6 +54,16 @@ var tagToExtension = map[string]extensionAttributes{ kind: types.Slice, allowedValues: sets.NewString("atomic", "set", "map"), }, + "mapType": { + xName: "x-kubernetes-map-type", + kind: types.Map, + allowedValues: sets.NewString("atomic", "granular"), + }, + "structType": { + xName: "x-kubernetes-map-type", + kind: types.Struct, + allowedValues: sets.NewString("atomic", "granular"), + }, } // Extension encapsulates information necessary to generate an OpenAPI extension. diff --git a/pkg/generators/extension_test.go b/pkg/generators/extension_test.go index d1214bd9a..7c2c0dff1 100644 --- a/pkg/generators/extension_test.go +++ b/pkg/generators/extension_test.go @@ -58,6 +58,12 @@ func TestSingleTagExtension(t *testing.T) { extensionName: "x-kubernetes-list-map-keys", extensionValues: []string{"port"}, }, + { + comments: []string{"+mapType=granular"}, + extensionTag: "mapType", + extensionName: "x-kubernetes-map-type", + extensionValues: []string{"granular"}, + }, { comments: []string{"+k8s:openapi-gen=x-kubernetes-member-tag:member_test"}, extensionTag: "k8s:openapi-gen", @@ -205,6 +211,18 @@ func TestExtensionAllowedValues(t *testing.T) { }, allowedValues: nil, }, + { + e: extension{ + idlTag: "mapType", + }, + allowedValues: sets.NewString("atomic", "granular"), + }, + { + e: extension{ + idlTag: "structType", + }, + allowedValues: sets.NewString("atomic", "granular"), + }, { e: extension{ idlTag: "k8s:openapi-gen", @@ -259,6 +277,20 @@ func TestExtensionAllowedValues(t *testing.T) { values: []string{"atomic"}, }, }, + { + e: extension{ + idlTag: "mapType", + xName: "x-kubernetes-map-type", + values: []string{"atomic"}, + }, + }, + { + e: extension{ + idlTag: "structType", + xName: "x-kubernetes-map-type", + values: []string{"granular"}, + }, + }, } for _, test := range successTests { actualErr := test.e.validateAllowedValues() @@ -292,6 +324,20 @@ func TestExtensionAllowedValues(t *testing.T) { values: []string{"not-allowed"}, }, }, + { + e: extension{ + idlTag: "mapType", + xName: "x-kubernetes-map-type", + values: []string{"something-pretty-wrong"}, + }, + }, + { + e: extension{ + idlTag: "structType", + xName: "x-kubernetes-map-type", + values: []string{"not-quite-right"}, + }, + }, } for _, test := range failureTests { actualErr := test.e.validateAllowedValues() @@ -326,6 +372,18 @@ func TestExtensionKind(t *testing.T) { }, kind: types.Slice, }, + { + e: extension{ + idlTag: "mapType", + }, + kind: types.Map, + }, + { + e: extension{ + idlTag: "structType", + }, + kind: types.Struct, + }, { e: extension{ idlTag: "listMapKey", diff --git a/pkg/idl/doc.go b/pkg/idl/doc.go index 9d59aaf0e..ef1ed9dca 100644 --- a/pkg/idl/doc.go +++ b/pkg/idl/doc.go @@ -133,7 +133,7 @@ type PatchStrategy string // used on any struct. // // Using this tag will generate the following OpenAPI extension: -// "x-kubernetes-struct-type": "atomic" +// "x-kubernetes-map-type": "atomic" type StructType string // Union is TBD. diff --git a/test/integration/README.md b/test/integration/README.md index 0f72f96d7..bb672a0d3 100644 --- a/test/integration/README.md +++ b/test/integration/README.md @@ -13,26 +13,26 @@ $ go test -v . First, run the generator to create `openapi_generated.go` file which specifies the `OpenAPIDefinition` for each type, and generate the golden API rule violation report file . Note that if you do not pass a report -filename (`./testdata/golden.report` in the command below) to let the generator +filename (`./testdata/golden.v2.report` in the command below) to let the generator to print API rule violations to the file, the generator will return error to stderr on API rule violations. ```bash $ go run ../../cmd/openapi-gen/openapi-gen.go \ - -i "k8s.io/kube-openapi/test/integration/testdata/listtype,k8s.io/kube-openapi/test/integration/testdata/dummytype,k8s.io/kube-openapi/test/integration/testdata/uniontype" \ + -i "k8s.io/kube-openapi/test/integration/testdata/listtype,k8s.io/kube-openapi/test/integration/testdata/maptype,k8s.io/kube-openapi/test/integration/testdata/structtype,k8s.io/kube-openapi/test/integration/testdata/dummytype,k8s.io/kube-openapi/test/integration/testdata/uniontype" \ -o pkg \ -p generated \ -O openapi_generated \ - -r ./testdata/golden.report + -r ./testdata/golden.v2.report ``` The generated file `pkg/generated/openapi_generated.go` should have been created. Next, run the OpenAPI builder to create the Swagger file which includes -the definitions. The output file named `golden.json` will be output in +the definitions. The output file named `golden.v2.json` will be output in the current directory. ```bash -$ go run builder/main.go testdata/golden.json +$ go run builder/main.go testdata/golden.v2.json ``` After the golden spec is generated, please clean up the generated file diff --git a/test/integration/integration_suite_test.go b/test/integration/integration_suite_test.go index 5b6a8f72f..cc1f0afd3 100644 --- a/test/integration/integration_suite_test.go +++ b/test/integration/integration_suite_test.go @@ -32,6 +32,8 @@ const ( testdataDir = "./testdata" testPkgDir = "k8s.io/kube-openapi/test/integration/testdata" inputDir = testPkgDir + "/listtype" + + "," + testPkgDir + "/maptype" + + "," + testPkgDir + "/structtype" + "," + testPkgDir + "/dummytype" + "," + testPkgDir + "/uniontype" + "," + testPkgDir + "/custom" diff --git a/test/integration/testdata/golden.v2.report b/test/integration/testdata/golden.v2.report index 85735aa23..cfed82e35 100644 --- a/test/integration/testdata/golden.v2.report +++ b/test/integration/testdata/golden.v2.report @@ -13,4 +13,10 @@ API rule violation: names_match,k8s.io/kube-openapi/test/integration/testdata/li API rule violation: names_match,k8s.io/kube-openapi/test/integration/testdata/listtype,MapList,Field API rule violation: names_match,k8s.io/kube-openapi/test/integration/testdata/listtype,SetList,Field API rule violation: names_match,k8s.io/kube-openapi/test/integration/testdata/listtype,UntypedList,Field +API rule violation: names_match,k8s.io/kube-openapi/test/integration/testdata/maptype,AtomicMap,KeyValue +API rule violation: names_match,k8s.io/kube-openapi/test/integration/testdata/maptype,GranularMap,KeyValue +API rule violation: names_match,k8s.io/kube-openapi/test/integration/testdata/structtype,AtomicStruct,Field +API rule violation: names_match,k8s.io/kube-openapi/test/integration/testdata/structtype,AtomicStruct,OtherField +API rule violation: names_match,k8s.io/kube-openapi/test/integration/testdata/structtype,GranularStruct,Field +API rule violation: names_match,k8s.io/kube-openapi/test/integration/testdata/structtype,GranularStruct,OtherField API rule violation: omitempty_match_case,k8s.io/kube-openapi/test/integration/testdata/listtype,Item,C diff --git a/test/integration/testdata/maptype/atomic-map.go b/test/integration/testdata/maptype/atomic-map.go new file mode 100644 index 000000000..7447eb2ba --- /dev/null +++ b/test/integration/testdata/maptype/atomic-map.go @@ -0,0 +1,7 @@ +package maptype + +// +k8s:openapi-gen=true +type AtomicMap struct { + // +mapType=atomic + KeyValue map[string]string +} diff --git a/test/integration/testdata/maptype/granular-map.go b/test/integration/testdata/maptype/granular-map.go new file mode 100644 index 000000000..422d95a2c --- /dev/null +++ b/test/integration/testdata/maptype/granular-map.go @@ -0,0 +1,7 @@ +package maptype + +// +k8s:openapi-gen=true +type GranularMap struct { + // +mapType=granular + KeyValue map[string]string +} diff --git a/test/integration/testdata/structtype/atomic-struct.go b/test/integration/testdata/structtype/atomic-struct.go new file mode 100644 index 000000000..92dfd77b4 --- /dev/null +++ b/test/integration/testdata/structtype/atomic-struct.go @@ -0,0 +1,8 @@ +package structtype + +// +k8s:openapi-gen=true +type AtomicStruct struct { + // +structType=atomic + Field string + OtherField int +} diff --git a/test/integration/testdata/structtype/granular-struct.go b/test/integration/testdata/structtype/granular-struct.go new file mode 100644 index 000000000..0eff03a94 --- /dev/null +++ b/test/integration/testdata/structtype/granular-struct.go @@ -0,0 +1,8 @@ +package structtype + +// +k8s:openapi-gen=true +type GranularStruct struct { + // +structType=granular + Field string + OtherField int +} From 7c6c06ecc2f3642d997470ce14e2eeb85a8dabe8 Mon Sep 17 00:00:00 2001 From: Maria Ntalla Date: Mon, 16 Dec 2019 14:09:55 +0000 Subject: [PATCH 2/7] Correct valid values for mapType, structType It's atomic/granular, not only atomic. --- pkg/idl/doc.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/pkg/idl/doc.go b/pkg/idl/doc.go index ef1ed9dca..8146af874 100644 --- a/pkg/idl/doc.go +++ b/pkg/idl/doc.go @@ -79,13 +79,15 @@ type ListType string type ListMapKey string // MapType annotates a map to further describe its topology. It may -// have only one value: "atomic". Atomic means that the entire map is -// considered as a whole, rather than as distinct values. +// have one of two values: `atomic` or `granular`. `atomic` means that the entire map is +// considered as a whole; actors that wish to update the map can only +// entirely replace it. `granular` means that specific values in the map can be +// updated separately from other fields. // // By default, a map will be considered as a set of distinct values that -// can be updated individually. This default WILL NOT generate any -// openapi extension, as this will also be interpreted as the default -// behavior in the openapi definition. +// can be updated individually (i.e. the equivalent of `granular`). +// This default WILL NOT generate any openapi extension, as this will also +// be interpreted as the default behavior in the openapi definition. // // This tag MUST only be used on maps, or the generation step will fail. // @@ -114,11 +116,13 @@ type PatchMergeKey string type PatchStrategy string // StructType annotates a struct to further describe its topology. It may -// have only one value: "atomic". Atomic means that the entire struct is -// considered as a whole, rather than as distinct values. +// have one of two values: `atomic` or `granular`. `atomic` means that the entire struct is +// considered as a whole; actors that wish to update the struct can only +// entirely replace it. `granular` means that specific fields in the struct can be +// updated separately from other fields. // // By default, a struct will be considered as a set of distinct values that -// can be updated individually. This default WILL NOT generate any +// can be updated individually (`granular`). This default WILL NOT generate any // openapi extension, as this will also be interpreted as the default // behavior in the openapi definition. // From a0384dd483d95391e59ea2171ff8c76ac4fb6c69 Mon Sep 17 00:00:00 2001 From: Maria Ntalla Date: Tue, 17 Dec 2019 13:56:31 +0000 Subject: [PATCH 3/7] Add routes for structType and mapType in integration tests --- test/integration/builder/main.go | 4 + .../pkg/generated/openapi_generated.go | 170 +++++++++++++++--- test/integration/testdata/golden.v2.json | 140 +++++++++++++++ 3 files changed, 285 insertions(+), 29 deletions(-) diff --git a/test/integration/builder/main.go b/test/integration/builder/main.go index 355c88590..7c531ff9a 100644 --- a/test/integration/builder/main.go +++ b/test/integration/builder/main.go @@ -115,6 +115,10 @@ func createWebServices() []*restful.WebService { w.Route(buildRouteForType(w, "custom", "Bak")) w.Route(buildRouteForType(w, "custom", "Bac")) w.Route(buildRouteForType(w, "custom", "Bah")) + w.Route(buildRouteForType(w, "maptype", "GranularMap")) + w.Route(buildRouteForType(w, "maptype", "AtomicMap")) + w.Route(buildRouteForType(w, "structtype", "GranularStruct")) + w.Route(buildRouteForType(w, "structtype", "AtomicStruct")) return []*restful.WebService{w} } diff --git a/test/integration/pkg/generated/openapi_generated.go b/test/integration/pkg/generated/openapi_generated.go index b59894f9b..65370068c 100644 --- a/test/integration/pkg/generated/openapi_generated.go +++ b/test/integration/pkg/generated/openapi_generated.go @@ -25,42 +25,30 @@ package generated import ( spec "github.com/go-openapi/spec" common "k8s.io/kube-openapi/pkg/common" - custom "k8s.io/kube-openapi/test/integration/testdata/custom" ) func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenAPIDefinition { return map[string]common.OpenAPIDefinition{ - "k8s.io/kube-openapi/test/integration/testdata/custom.Bac": common.EmbedOpenAPIDefinitionIntoV2Extension(custom.Bac{}.OpenAPIV3Definition(), custom.Bac{}.OpenAPIDefinition()), - "k8s.io/kube-openapi/test/integration/testdata/custom.Bah": schema_test_integration_testdata_custom_Bah(ref), - "k8s.io/kube-openapi/test/integration/testdata/custom.Bak": custom.Bak{}.OpenAPIDefinition(), - "k8s.io/kube-openapi/test/integration/testdata/custom.Bal": custom.Bal{}.OpenAPIV3Definition(), - "k8s.io/kube-openapi/test/integration/testdata/dummytype.Bar": schema_test_integration_testdata_dummytype_Bar(ref), - "k8s.io/kube-openapi/test/integration/testdata/dummytype.Baz": schema_test_integration_testdata_dummytype_Baz(ref), - "k8s.io/kube-openapi/test/integration/testdata/dummytype.Foo": schema_test_integration_testdata_dummytype_Foo(ref), - "k8s.io/kube-openapi/test/integration/testdata/dummytype.Waldo": schema_test_integration_testdata_dummytype_Waldo(ref), - "k8s.io/kube-openapi/test/integration/testdata/listtype.AtomicList": schema_test_integration_testdata_listtype_AtomicList(ref), - "k8s.io/kube-openapi/test/integration/testdata/listtype.Item": schema_test_integration_testdata_listtype_Item(ref), - "k8s.io/kube-openapi/test/integration/testdata/listtype.MapList": schema_test_integration_testdata_listtype_MapList(ref), - "k8s.io/kube-openapi/test/integration/testdata/listtype.SetList": schema_test_integration_testdata_listtype_SetList(ref), - "k8s.io/kube-openapi/test/integration/testdata/listtype.UntypedList": schema_test_integration_testdata_listtype_UntypedList(ref), - "k8s.io/kube-openapi/test/integration/testdata/uniontype.InlinedUnion": schema_test_integration_testdata_uniontype_InlinedUnion(ref), - "k8s.io/kube-openapi/test/integration/testdata/uniontype.TopLevelUnion": schema_test_integration_testdata_uniontype_TopLevelUnion(ref), - "k8s.io/kube-openapi/test/integration/testdata/uniontype.Union": schema_test_integration_testdata_uniontype_Union(ref), - "k8s.io/kube-openapi/test/integration/testdata/uniontype.Union2": schema_test_integration_testdata_uniontype_Union2(ref), + "k8s.io/kube-openapi/test/integration/testdata/dummytype.Bar": schema_test_integration_testdata_dummytype_Bar(ref), + "k8s.io/kube-openapi/test/integration/testdata/dummytype.Baz": schema_test_integration_testdata_dummytype_Baz(ref), + "k8s.io/kube-openapi/test/integration/testdata/dummytype.Foo": schema_test_integration_testdata_dummytype_Foo(ref), + "k8s.io/kube-openapi/test/integration/testdata/dummytype.Waldo": schema_test_integration_testdata_dummytype_Waldo(ref), + "k8s.io/kube-openapi/test/integration/testdata/listtype.AtomicList": schema_test_integration_testdata_listtype_AtomicList(ref), + "k8s.io/kube-openapi/test/integration/testdata/listtype.Item": schema_test_integration_testdata_listtype_Item(ref), + "k8s.io/kube-openapi/test/integration/testdata/listtype.MapList": schema_test_integration_testdata_listtype_MapList(ref), + "k8s.io/kube-openapi/test/integration/testdata/listtype.SetList": schema_test_integration_testdata_listtype_SetList(ref), + "k8s.io/kube-openapi/test/integration/testdata/listtype.UntypedList": schema_test_integration_testdata_listtype_UntypedList(ref), + "k8s.io/kube-openapi/test/integration/testdata/maptype.AtomicMap": schema_test_integration_testdata_maptype_AtomicMap(ref), + "k8s.io/kube-openapi/test/integration/testdata/maptype.GranularMap": schema_test_integration_testdata_maptype_GranularMap(ref), + "k8s.io/kube-openapi/test/integration/testdata/structtype.AtomicStruct": schema_test_integration_testdata_structtype_AtomicStruct(ref), + "k8s.io/kube-openapi/test/integration/testdata/structtype.GranularStruct": schema_test_integration_testdata_structtype_GranularStruct(ref), + "k8s.io/kube-openapi/test/integration/testdata/uniontype.InlinedUnion": schema_test_integration_testdata_uniontype_InlinedUnion(ref), + "k8s.io/kube-openapi/test/integration/testdata/uniontype.TopLevelUnion": schema_test_integration_testdata_uniontype_TopLevelUnion(ref), + "k8s.io/kube-openapi/test/integration/testdata/uniontype.Union": schema_test_integration_testdata_uniontype_Union(ref), + "k8s.io/kube-openapi/test/integration/testdata/uniontype.Union2": schema_test_integration_testdata_uniontype_Union2(ref), } } -func schema_test_integration_testdata_custom_Bah(ref common.ReferenceCallback) common.OpenAPIDefinition { - return common.EmbedOpenAPIDefinitionIntoV2Extension(custom.Bah{}.OpenAPIV3Definition(), common.OpenAPIDefinition{ - Schema: spec.Schema{ - SchemaProps: spec.SchemaProps{ - Type: custom.Bah{}.OpenAPISchemaType(), - Format: custom.Bah{}.OpenAPISchemaFormat(), - }, - }, - }) -} - func schema_test_integration_testdata_dummytype_Bar(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ @@ -325,6 +313,130 @@ func schema_test_integration_testdata_listtype_UntypedList(ref common.ReferenceC } } +func schema_test_integration_testdata_maptype_AtomicMap(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "KeyValue": { + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-map-type": "atomic", + }, + }, + SchemaProps: spec.SchemaProps{ + Type: []string{"object"}, + AdditionalProperties: &spec.SchemaOrBool{ + Allows: true, + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, + }, + Required: []string{"KeyValue"}, + }, + }, + } +} + +func schema_test_integration_testdata_maptype_GranularMap(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "KeyValue": { + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-map-type": "granular", + }, + }, + SchemaProps: spec.SchemaProps{ + Type: []string{"object"}, + AdditionalProperties: &spec.SchemaOrBool{ + Allows: true, + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, + }, + Required: []string{"KeyValue"}, + }, + }, + } +} + +func schema_test_integration_testdata_structtype_AtomicStruct(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "Field": { + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-map-type": "atomic", + }, + }, + SchemaProps: spec.SchemaProps{ + Type: []string{"string"}, + Format: "", + }, + }, + "OtherField": { + SchemaProps: spec.SchemaProps{ + Type: []string{"integer"}, + Format: "int32", + }, + }, + }, + Required: []string{"Field", "OtherField"}, + }, + }, + } +} + +func schema_test_integration_testdata_structtype_GranularStruct(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "Field": { + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-map-type": "granular", + }, + }, + SchemaProps: spec.SchemaProps{ + Type: []string{"string"}, + Format: "", + }, + }, + "OtherField": { + SchemaProps: spec.SchemaProps{ + Type: []string{"integer"}, + Format: "int32", + }, + }, + }, + Required: []string{"Field", "OtherField"}, + }, + }, + } +} + func schema_test_integration_testdata_uniontype_InlinedUnion(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ diff --git a/test/integration/testdata/golden.v2.json b/test/integration/testdata/golden.v2.json index c541c6f97..6e7f94d36 100644 --- a/test/integration/testdata/golden.v2.json +++ b/test/integration/testdata/golden.v2.json @@ -214,6 +214,82 @@ } } }, + "/test/maptype/atomicmap": { + "get": { + "schemes": [ + "https" + ], + "operationId": "func15", + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/maptype.AtomicMap" + } + }, + "404": { + "$ref": "#/responses/NotFound" + } + } + } + }, + "/test/maptype/granularmap": { + "get": { + "schemes": [ + "https" + ], + "operationId": "func14", + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/maptype.GranularMap" + } + }, + "404": { + "$ref": "#/responses/NotFound" + } + } + } + }, + "/test/structtype/atomicstruct": { + "get": { + "schemes": [ + "https" + ], + "operationId": "func17", + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/structtype.AtomicStruct" + } + }, + "404": { + "$ref": "#/responses/NotFound" + } + } + } + }, + "/test/structtype/granularstruct": { + "get": { + "schemes": [ + "https" + ], + "operationId": "func16", + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/structtype.GranularStruct" + } + }, + "404": { + "$ref": "#/responses/NotFound" + } + } + } + }, "/test/uniontype/inlinedunion": { "get": { "schemes": [ @@ -403,6 +479,70 @@ } } }, + "maptype.AtomicMap": { + "type": "object", + "required": [ + "KeyValue" + ], + "properties": { + "KeyValue": { + "type": "object", + "additionalProperties": { + "type": "string" + }, + "x-kubernetes-map-type": "atomic" + } + } + }, + "maptype.GranularMap": { + "type": "object", + "required": [ + "KeyValue" + ], + "properties": { + "KeyValue": { + "type": "object", + "additionalProperties": { + "type": "string" + }, + "x-kubernetes-map-type": "granular" + } + } + }, + "structtype.AtomicStruct": { + "type": "object", + "required": [ + "Field", + "OtherField" + ], + "properties": { + "Field": { + "type": "string", + "x-kubernetes-map-type": "atomic" + }, + "OtherField": { + "type": "integer", + "format": "int32" + } + } + }, + "structtype.GranularStruct": { + "type": "object", + "required": [ + "Field", + "OtherField" + ], + "properties": { + "Field": { + "type": "string", + "x-kubernetes-map-type": "granular" + }, + "OtherField": { + "type": "integer", + "format": "int32" + } + } + }, "uniontype.InlinedUnion": { "type": "object", "required": [ From e01d666bc8dcd34e95f1d9d9f7686ba311175fd8 Mon Sep 17 00:00:00 2001 From: Maria Ntalla Date: Tue, 21 Jan 2020 15:39:39 +0000 Subject: [PATCH 4/7] Apply structType on struct --- test/integration/testdata/golden.v2.json | 11 +++++++---- test/integration/testdata/structtype/atomic-struct.go | 5 ++++- .../testdata/structtype/granular-struct.go | 2 +- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/test/integration/testdata/golden.v2.json b/test/integration/testdata/golden.v2.json index 6e7f94d36..ac53978a2 100644 --- a/test/integration/testdata/golden.v2.json +++ b/test/integration/testdata/golden.v2.json @@ -517,8 +517,8 @@ ], "properties": { "Field": { - "type": "string", - "x-kubernetes-map-type": "atomic" + "x-kubernetes-map-type": "atomic", + "$ref": "#/definitions/structtype.ContainedStruct" }, "OtherField": { "type": "integer", @@ -526,6 +526,9 @@ } } }, + "structtype.ContainedStruct": { + "type": "object" + }, "structtype.GranularStruct": { "type": "object", "required": [ @@ -534,8 +537,8 @@ ], "properties": { "Field": { - "type": "string", - "x-kubernetes-map-type": "granular" + "x-kubernetes-map-type": "granular", + "$ref": "#/definitions/structtype.ContainedStruct" }, "OtherField": { "type": "integer", diff --git a/test/integration/testdata/structtype/atomic-struct.go b/test/integration/testdata/structtype/atomic-struct.go index 92dfd77b4..41ebe388e 100644 --- a/test/integration/testdata/structtype/atomic-struct.go +++ b/test/integration/testdata/structtype/atomic-struct.go @@ -3,6 +3,9 @@ package structtype // +k8s:openapi-gen=true type AtomicStruct struct { // +structType=atomic - Field string + Field ContainedStruct OtherField int } + +// +k8s:openapi-gen=true +type ContainedStruct struct{} diff --git a/test/integration/testdata/structtype/granular-struct.go b/test/integration/testdata/structtype/granular-struct.go index 0eff03a94..c31b325de 100644 --- a/test/integration/testdata/structtype/granular-struct.go +++ b/test/integration/testdata/structtype/granular-struct.go @@ -3,6 +3,6 @@ package structtype // +k8s:openapi-gen=true type GranularStruct struct { // +structType=granular - Field string + Field ContainedStruct OtherField int } From 8230c670f9e3f3509b4dcd98d9eb33fa3e8945df Mon Sep 17 00:00:00 2001 From: Maria Ntalla Date: Thu, 23 Jan 2020 16:51:14 +0000 Subject: [PATCH 5/7] Document that a defaulted mapType generates openapi extension --- pkg/idl/doc.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/idl/doc.go b/pkg/idl/doc.go index 8146af874..f870405be 100644 --- a/pkg/idl/doc.go +++ b/pkg/idl/doc.go @@ -86,8 +86,7 @@ type ListMapKey string // // By default, a map will be considered as a set of distinct values that // can be updated individually (i.e. the equivalent of `granular`). -// This default WILL NOT generate any openapi extension, as this will also -// be interpreted as the default behavior in the openapi definition. +// This default will still generate an OpenAPI extension with key: "x-kubernetes-map-type". // // This tag MUST only be used on maps, or the generation step will fail. // @@ -122,9 +121,8 @@ type PatchStrategy string // updated separately from other fields. // // By default, a struct will be considered as a set of distinct values that -// can be updated individually (`granular`). This default WILL NOT generate any -// openapi extension, as this will also be interpreted as the default -// behavior in the openapi definition. +// can be updated individually (`granular`). +// This default will still generate an OpenAPI extension with key: "x-kubernetes-map-type". // // This tag MUST only be used on structs, or the generation step will fail. // From d17e71df89a7d1fc4857206269ab0e7d826d48ca Mon Sep 17 00:00:00 2001 From: Maria Ntalla Date: Wed, 29 Jan 2020 16:27:23 +0000 Subject: [PATCH 6/7] Don't commit openapi_generated.go --- .../pkg/generated/openapi_generated.go | 170 +++--------------- 1 file changed, 29 insertions(+), 141 deletions(-) diff --git a/test/integration/pkg/generated/openapi_generated.go b/test/integration/pkg/generated/openapi_generated.go index 65370068c..b59894f9b 100644 --- a/test/integration/pkg/generated/openapi_generated.go +++ b/test/integration/pkg/generated/openapi_generated.go @@ -25,30 +25,42 @@ package generated import ( spec "github.com/go-openapi/spec" common "k8s.io/kube-openapi/pkg/common" + custom "k8s.io/kube-openapi/test/integration/testdata/custom" ) func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenAPIDefinition { return map[string]common.OpenAPIDefinition{ - "k8s.io/kube-openapi/test/integration/testdata/dummytype.Bar": schema_test_integration_testdata_dummytype_Bar(ref), - "k8s.io/kube-openapi/test/integration/testdata/dummytype.Baz": schema_test_integration_testdata_dummytype_Baz(ref), - "k8s.io/kube-openapi/test/integration/testdata/dummytype.Foo": schema_test_integration_testdata_dummytype_Foo(ref), - "k8s.io/kube-openapi/test/integration/testdata/dummytype.Waldo": schema_test_integration_testdata_dummytype_Waldo(ref), - "k8s.io/kube-openapi/test/integration/testdata/listtype.AtomicList": schema_test_integration_testdata_listtype_AtomicList(ref), - "k8s.io/kube-openapi/test/integration/testdata/listtype.Item": schema_test_integration_testdata_listtype_Item(ref), - "k8s.io/kube-openapi/test/integration/testdata/listtype.MapList": schema_test_integration_testdata_listtype_MapList(ref), - "k8s.io/kube-openapi/test/integration/testdata/listtype.SetList": schema_test_integration_testdata_listtype_SetList(ref), - "k8s.io/kube-openapi/test/integration/testdata/listtype.UntypedList": schema_test_integration_testdata_listtype_UntypedList(ref), - "k8s.io/kube-openapi/test/integration/testdata/maptype.AtomicMap": schema_test_integration_testdata_maptype_AtomicMap(ref), - "k8s.io/kube-openapi/test/integration/testdata/maptype.GranularMap": schema_test_integration_testdata_maptype_GranularMap(ref), - "k8s.io/kube-openapi/test/integration/testdata/structtype.AtomicStruct": schema_test_integration_testdata_structtype_AtomicStruct(ref), - "k8s.io/kube-openapi/test/integration/testdata/structtype.GranularStruct": schema_test_integration_testdata_structtype_GranularStruct(ref), - "k8s.io/kube-openapi/test/integration/testdata/uniontype.InlinedUnion": schema_test_integration_testdata_uniontype_InlinedUnion(ref), - "k8s.io/kube-openapi/test/integration/testdata/uniontype.TopLevelUnion": schema_test_integration_testdata_uniontype_TopLevelUnion(ref), - "k8s.io/kube-openapi/test/integration/testdata/uniontype.Union": schema_test_integration_testdata_uniontype_Union(ref), - "k8s.io/kube-openapi/test/integration/testdata/uniontype.Union2": schema_test_integration_testdata_uniontype_Union2(ref), + "k8s.io/kube-openapi/test/integration/testdata/custom.Bac": common.EmbedOpenAPIDefinitionIntoV2Extension(custom.Bac{}.OpenAPIV3Definition(), custom.Bac{}.OpenAPIDefinition()), + "k8s.io/kube-openapi/test/integration/testdata/custom.Bah": schema_test_integration_testdata_custom_Bah(ref), + "k8s.io/kube-openapi/test/integration/testdata/custom.Bak": custom.Bak{}.OpenAPIDefinition(), + "k8s.io/kube-openapi/test/integration/testdata/custom.Bal": custom.Bal{}.OpenAPIV3Definition(), + "k8s.io/kube-openapi/test/integration/testdata/dummytype.Bar": schema_test_integration_testdata_dummytype_Bar(ref), + "k8s.io/kube-openapi/test/integration/testdata/dummytype.Baz": schema_test_integration_testdata_dummytype_Baz(ref), + "k8s.io/kube-openapi/test/integration/testdata/dummytype.Foo": schema_test_integration_testdata_dummytype_Foo(ref), + "k8s.io/kube-openapi/test/integration/testdata/dummytype.Waldo": schema_test_integration_testdata_dummytype_Waldo(ref), + "k8s.io/kube-openapi/test/integration/testdata/listtype.AtomicList": schema_test_integration_testdata_listtype_AtomicList(ref), + "k8s.io/kube-openapi/test/integration/testdata/listtype.Item": schema_test_integration_testdata_listtype_Item(ref), + "k8s.io/kube-openapi/test/integration/testdata/listtype.MapList": schema_test_integration_testdata_listtype_MapList(ref), + "k8s.io/kube-openapi/test/integration/testdata/listtype.SetList": schema_test_integration_testdata_listtype_SetList(ref), + "k8s.io/kube-openapi/test/integration/testdata/listtype.UntypedList": schema_test_integration_testdata_listtype_UntypedList(ref), + "k8s.io/kube-openapi/test/integration/testdata/uniontype.InlinedUnion": schema_test_integration_testdata_uniontype_InlinedUnion(ref), + "k8s.io/kube-openapi/test/integration/testdata/uniontype.TopLevelUnion": schema_test_integration_testdata_uniontype_TopLevelUnion(ref), + "k8s.io/kube-openapi/test/integration/testdata/uniontype.Union": schema_test_integration_testdata_uniontype_Union(ref), + "k8s.io/kube-openapi/test/integration/testdata/uniontype.Union2": schema_test_integration_testdata_uniontype_Union2(ref), } } +func schema_test_integration_testdata_custom_Bah(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.EmbedOpenAPIDefinitionIntoV2Extension(custom.Bah{}.OpenAPIV3Definition(), common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: custom.Bah{}.OpenAPISchemaType(), + Format: custom.Bah{}.OpenAPISchemaFormat(), + }, + }, + }) +} + func schema_test_integration_testdata_dummytype_Bar(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ @@ -313,130 +325,6 @@ func schema_test_integration_testdata_listtype_UntypedList(ref common.ReferenceC } } -func schema_test_integration_testdata_maptype_AtomicMap(ref common.ReferenceCallback) common.OpenAPIDefinition { - return common.OpenAPIDefinition{ - Schema: spec.Schema{ - SchemaProps: spec.SchemaProps{ - Type: []string{"object"}, - Properties: map[string]spec.Schema{ - "KeyValue": { - VendorExtensible: spec.VendorExtensible{ - Extensions: spec.Extensions{ - "x-kubernetes-map-type": "atomic", - }, - }, - SchemaProps: spec.SchemaProps{ - Type: []string{"object"}, - AdditionalProperties: &spec.SchemaOrBool{ - Allows: true, - Schema: &spec.Schema{ - SchemaProps: spec.SchemaProps{ - Type: []string{"string"}, - Format: "", - }, - }, - }, - }, - }, - }, - Required: []string{"KeyValue"}, - }, - }, - } -} - -func schema_test_integration_testdata_maptype_GranularMap(ref common.ReferenceCallback) common.OpenAPIDefinition { - return common.OpenAPIDefinition{ - Schema: spec.Schema{ - SchemaProps: spec.SchemaProps{ - Type: []string{"object"}, - Properties: map[string]spec.Schema{ - "KeyValue": { - VendorExtensible: spec.VendorExtensible{ - Extensions: spec.Extensions{ - "x-kubernetes-map-type": "granular", - }, - }, - SchemaProps: spec.SchemaProps{ - Type: []string{"object"}, - AdditionalProperties: &spec.SchemaOrBool{ - Allows: true, - Schema: &spec.Schema{ - SchemaProps: spec.SchemaProps{ - Type: []string{"string"}, - Format: "", - }, - }, - }, - }, - }, - }, - Required: []string{"KeyValue"}, - }, - }, - } -} - -func schema_test_integration_testdata_structtype_AtomicStruct(ref common.ReferenceCallback) common.OpenAPIDefinition { - return common.OpenAPIDefinition{ - Schema: spec.Schema{ - SchemaProps: spec.SchemaProps{ - Type: []string{"object"}, - Properties: map[string]spec.Schema{ - "Field": { - VendorExtensible: spec.VendorExtensible{ - Extensions: spec.Extensions{ - "x-kubernetes-map-type": "atomic", - }, - }, - SchemaProps: spec.SchemaProps{ - Type: []string{"string"}, - Format: "", - }, - }, - "OtherField": { - SchemaProps: spec.SchemaProps{ - Type: []string{"integer"}, - Format: "int32", - }, - }, - }, - Required: []string{"Field", "OtherField"}, - }, - }, - } -} - -func schema_test_integration_testdata_structtype_GranularStruct(ref common.ReferenceCallback) common.OpenAPIDefinition { - return common.OpenAPIDefinition{ - Schema: spec.Schema{ - SchemaProps: spec.SchemaProps{ - Type: []string{"object"}, - Properties: map[string]spec.Schema{ - "Field": { - VendorExtensible: spec.VendorExtensible{ - Extensions: spec.Extensions{ - "x-kubernetes-map-type": "granular", - }, - }, - SchemaProps: spec.SchemaProps{ - Type: []string{"string"}, - Format: "", - }, - }, - "OtherField": { - SchemaProps: spec.SchemaProps{ - Type: []string{"integer"}, - Format: "int32", - }, - }, - }, - Required: []string{"Field", "OtherField"}, - }, - }, - } -} - func schema_test_integration_testdata_uniontype_InlinedUnion(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ From b567921ba62c9e27501426def55a0a18ee879d15 Mon Sep 17 00:00:00 2001 From: Maria Ntalla Date: Thu, 30 Jan 2020 15:44:54 +0000 Subject: [PATCH 7/7] Add testdata/custom to the API rule violations check --- test/integration/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/README.md b/test/integration/README.md index bb672a0d3..d2d1fe90f 100644 --- a/test/integration/README.md +++ b/test/integration/README.md @@ -12,14 +12,14 @@ $ go test -v . First, run the generator to create `openapi_generated.go` file which specifies the `OpenAPIDefinition` for each type, and generate the golden API rule -violation report file . Note that if you do not pass a report +violation report file. Note that if you do not pass a report filename (`./testdata/golden.v2.report` in the command below) to let the generator to print API rule violations to the file, the generator will return error to stderr on API rule violations. ```bash $ go run ../../cmd/openapi-gen/openapi-gen.go \ - -i "k8s.io/kube-openapi/test/integration/testdata/listtype,k8s.io/kube-openapi/test/integration/testdata/maptype,k8s.io/kube-openapi/test/integration/testdata/structtype,k8s.io/kube-openapi/test/integration/testdata/dummytype,k8s.io/kube-openapi/test/integration/testdata/uniontype" \ + -i "k8s.io/kube-openapi/test/integration/testdata/custom,k8s.io/kube-openapi/test/integration/testdata/listtype,k8s.io/kube-openapi/test/integration/testdata/maptype,k8s.io/kube-openapi/test/integration/testdata/structtype,k8s.io/kube-openapi/test/integration/testdata/dummytype,k8s.io/kube-openapi/test/integration/testdata/uniontype" \ -o pkg \ -p generated \ -O openapi_generated \