Merge pull request #298 from Jefftree/allOf

Wrap Refs with AllOf
kubernetes · Mar 26, 2022 · e69a86c · e69a86c
2 parents 9f9c01d + 207f0c7
commit e69a86c
Show file tree

Hide file tree

Showing 3 changed files with 111 additions and 7 deletions.
diff --git 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
@@ -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
@@ -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",