From 207f0c70f329c9813339a43a7a04ff1a2ac2e2a2 Mon Sep 17 00:00:00 2001 From: Jefftree Date: Fri, 25 Mar 2022 10:53:36 -0700 Subject: [PATCH] Wrap Refs with AllOf --- pkg/builder3/openapi.go | 30 +++++++++++++ pkg/builder3/openapi_test.go | 54 ++++++++++++++++++++++++ test/integration/testdata/golden.v3.json | 34 ++++++++++++--- 3 files changed, 111 insertions(+), 7 deletions(-) diff --git a/pkg/builder3/openapi.go b/pkg/builder3/openapi.go index 6f929c3a2..d15229d92 100644 --- a/pkg/builder3/openapi.go +++ b/pkg/builder3/openapi.go @@ -20,12 +20,14 @@ import ( "encoding/json" "fmt" "net/http" + "reflect" "strings" restful "github.com/emicklei/go-restful" "k8s.io/kube-openapi/pkg/common" "k8s.io/kube-openapi/pkg/common/restfuladapter" + "k8s.io/kube-openapi/pkg/schemamutation" "k8s.io/kube-openapi/pkg/spec3" "k8s.io/kube-openapi/pkg/util" "k8s.io/kube-openapi/pkg/validation/spec" @@ -396,6 +398,7 @@ func (o *openAPI) buildDefinitionRecursively(name string) error { } // delete the embedded v2 schema if exists, otherwise no-op delete(schema.VendorExtensible.Extensions, common.ExtensionV2Schema) + schema = wrapRefs(schema) o.spec.Components.Schemas[uniqueName] = schema for _, v := range item.Dependencies { if err := o.buildDefinitionRecursively(v); err != nil { @@ -436,3 +439,30 @@ func (o *openAPI) toSchema(name string) (_ *spec.Schema, err error) { }, nil } } + +// wrapRefs wraps OpenAPI V3 Schema refs that contain sibling elements. +// AllOf is used to wrap the Ref to prevent references from having sibling elements +// Please see https://github.com/kubernetes/kubernetes/issues/106387#issuecomment-967640388 +func wrapRefs(schema *spec.Schema) *spec.Schema { + walker := schemamutation.Walker{ + SchemaCallback: func(schema *spec.Schema) *spec.Schema { + orig := schema + clone := func() { + if orig == schema { + schema = new(spec.Schema) + *schema = *orig + } + } + if schema.Ref.String() != "" && !reflect.DeepEqual(*schema, spec.Schema{SchemaProps: spec.SchemaProps{Ref: schema.Ref}}) { + clone() + refSchema := new(spec.Schema) + refSchema.Ref = schema.Ref + schema.Ref = spec.Ref{} + schema.AllOf = []spec.Schema{*refSchema} + } + return schema + }, + RefCallback: schemamutation.RefCallbackNoop, + } + return walker.WalkSchema(schema) +} diff --git a/pkg/builder3/openapi_test.go b/pkg/builder3/openapi_test.go index 12c57a26d..58a0369f7 100644 --- a/pkg/builder3/openapi_test.go +++ b/pkg/builder3/openapi_test.go @@ -89,6 +89,28 @@ func (_ TestInput) OpenAPIDefinition() *openapi.OpenAPIDefinition { }, }, }, + "reference-extension": { + VendorExtensible: spec.VendorExtensible{ + Extensions: map[string]interface{}{"extension": "value"}, + }, + SchemaProps: spec.SchemaProps{ + Ref: spec.MustCreateRef("/components/schemas/builder3.TestOutput"), + }, + }, + "reference-nullable": { + SchemaProps: spec.SchemaProps{ + Ref: spec.MustCreateRef("/components/schemas/builder3.TestOutput"), + Nullable: true, + }, + }, + "reference-default": { + SchemaProps: spec.SchemaProps{ + Ref: spec.MustCreateRef("/components/schemas/builder3.TestOutput"), + Default: map[string]interface{}{}, + }, + }, + + } schema.Extensions = spec.Extensions{"x-test": "test"} def := openapi.EmbedOpenAPIDefinitionIntoV2Extension(openapi.OpenAPIDefinition{ @@ -338,6 +360,38 @@ func getTestInputDefinition() *spec.Schema { }, }, }, + "reference-extension": { + VendorExtensible: spec.VendorExtensible{ + Extensions: map[string]interface{}{"extension": "value"}, + }, + SchemaProps: spec.SchemaProps{ + AllOf: []spec.Schema{{ + SchemaProps: spec.SchemaProps{ + Ref: spec.MustCreateRef("/components/schemas/builder3.TestOutput"), + }, + }}, + }, + }, + "reference-nullable": { + SchemaProps: spec.SchemaProps{ + Nullable: true, + AllOf: []spec.Schema{{ + SchemaProps: spec.SchemaProps{ + Ref: spec.MustCreateRef("/components/schemas/builder3.TestOutput"), + }, + }}, + }, + }, + "reference-default": { + SchemaProps: spec.SchemaProps{ + AllOf: []spec.Schema{{ + SchemaProps: spec.SchemaProps{ + Ref: spec.MustCreateRef("/components/schemas/builder3.TestOutput"), + }, + }}, + Default: map[string]interface{}{}, + }, + }, }, }, VendorExtensible: spec.VendorExtensible{ diff --git a/test/integration/testdata/golden.v3.json b/test/integration/testdata/golden.v3.json index 9c44a68f3..8ead9f6ae 100644 --- a/test/integration/testdata/golden.v3.json +++ b/test/integration/testdata/golden.v3.json @@ -514,14 +514,22 @@ }, "OtherSub": { "default": {}, - "$ref": "#/components/schemas/defaults.SubStruct" + "allOf": [ + { + "$ref": "#/components/schemas/defaults.SubStruct" + } + ] }, "Sub": { "default": { "i": 5, "s": "foo" }, - "$ref": "#/components/schemas/defaults.SubStruct" + "allOf": [ + { + "$ref": "#/components/schemas/defaults.SubStruct" + } + ] } } }, @@ -688,7 +696,11 @@ "type": "array", "items": { "default": {}, - "$ref": "#/components/schemas/listtype.Item" + "allOf": [ + { + "$ref": "#/components/schemas/listtype.Item" + } + ] }, "x-kubernetes-list-map-keys": [ "port" @@ -754,8 +766,12 @@ "properties": { "Field": { "default": {}, - "x-kubernetes-map-type": "atomic", - "$ref": "#/components/schemas/structtype.ContainedStruct" + "allOf": [ + { + "$ref": "#/components/schemas/structtype.ContainedStruct" + } + ], + "x-kubernetes-map-type": "atomic" }, "OtherField": { "type": "integer", @@ -790,8 +806,12 @@ "properties": { "Field": { "default": {}, - "x-kubernetes-map-type": "granular", - "$ref": "#/components/schemas/structtype.ContainedStruct" + "allOf": [ + { + "$ref": "#/components/schemas/structtype.ContainedStruct" + } + ], + "x-kubernetes-map-type": "granular" }, "OtherField": { "type": "integer",