apiextensions: switch OpenAPI pubilshing to structural schema

staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder.go

@@ -26,6 +26,7 @@ import (
 	"github.com/go-openapi/spec"
 
 	v1 "k8s.io/api/autoscaling/v1"
+	structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	metav1beta1 "k8s.io/apimachinery/pkg/apis/meta/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -58,22 +59,24 @@ var namer *openapi.DefinitionNamer
 
 // BuildSwagger builds swagger for the given crd in the given version
 func BuildSwagger(crd *apiextensions.CustomResourceDefinition, version string) (*spec.Swagger, error) {
-	var schema *spec.Schema
+	var schema *structuralschema.Structural
 	s, err := apiextensions.GetSchemaForVersion(crd, version)
 	if err != nil {
 		return nil, err
 	}
 	if s != nil && s.OpenAPIV3Schema != nil {
-		schema, err = ConvertJSONSchemaPropsToOpenAPIv2Schema(s.OpenAPIV3Schema)
+		ss, err := structuralschema.NewStructural(s.OpenAPIV3Schema)
-		if err != nil {
+		if err == nil && len(structuralschema.ValidateStructural(ss, nil)) == 0 {
-			return nil, err
+			// skip non-structural schemas
+			schema = ss.Unfold()
 		}
 	}
+
 	// TODO(roycaihw): remove the WebService templating below. The following logic
 	// comes from function registerResourceHandlers() in k8s.io/apiserver.
 	// Alternatives are either (ideally) refactoring registerResourceHandlers() to
 	// reuse the code, or faking an APIInstaller for CR to feed to registerResourceHandlers().
-	b := newBuilder(crd, version, schema)
+	b := newBuilder(crd, version, schema, true)
 
 	// Sample response types for building web service
 	sample := &CRDCanonicalTypeNamer{
@@ -288,23 +291,27 @@ func (b *builder) buildRoute(root, path, action, verb string, sample interface{}
 
 // buildKubeNative builds input schema with Kubernetes' native object meta, type meta and
 // extensions
-func (b *builder) buildKubeNative(schema *spec.Schema) *spec.Schema {
+func (b *builder) buildKubeNative(schema *structuralschema.Structural, v2 bool) (ret *spec.Schema) {
 	// only add properties if we have a schema. Otherwise, kubectl would (wrongly) assume additionalProperties=false
 	// and forbid anything outside of apiVersion, kind and metadata. We have to fix kubectl to stop doing this, e.g. by
 	// adding additionalProperties=true support to explicitly allow additional fields.
 	// TODO: fix kubectl to understand additionalProperties=true
 	if schema == nil {
-		schema = &spec.Schema{
+		ret = &spec.Schema{
 			SchemaProps: spec.SchemaProps{Type: []string{"object"}},
 		}
 		// no, we cannot add more properties here, not even TypeMeta/ObjectMeta because kubectl will complain about
 		// unknown fields for anything else.
 	} else {
-		schema.SetProperty("metadata", *spec.RefSchema(objectMetaSchemaRef).
+		if v2 {
+			schema = ToStructuralOpenAPIV2(schema)
+		}
+		ret = schema.ToGoOpenAPI()
+		ret.SetProperty("metadata", *spec.RefSchema(objectMetaSchemaRef).
 			WithDescription(swaggerPartialObjectMetadataDescriptions["metadata"]))
-		addTypeMetaProperties(schema)
+		addTypeMetaProperties(ret)
 	}
-	schema.AddExtension(endpoints.ROUTE_META_GVK, []interface{}{
+	ret.AddExtension(endpoints.ROUTE_META_GVK, []interface{}{
 		map[string]interface{}{
 			"group":   b.group,
 			"version": b.version,
@@ -312,7 +319,7 @@ func (b *builder) buildKubeNative(schema *spec.Schema) *spec.Schema {
 		},
 	})
 
-	return schema
+	return ret
 }
 
 // getDefinition gets definition for given Kubernetes type. This function is extracted from
@@ -391,7 +398,7 @@ func (b *builder) getOpenAPIConfig() *common.Config {
 	}
 }
 
-func newBuilder(crd *apiextensions.CustomResourceDefinition, version string, schema *spec.Schema) *builder {
+func newBuilder(crd *apiextensions.CustomResourceDefinition, version string, schema *structuralschema.Structural, v2 bool) *builder {
 	b := &builder{
 		schema: &spec.Schema{
 			SchemaProps: spec.SchemaProps{Type: []string{"object"}},
@@ -410,7 +417,7 @@ func newBuilder(crd *apiextensions.CustomResourceDefinition, version string, sch
 	}
 
 	// Pre-build schema with Kubernetes native properties
-	b.schema = b.buildKubeNative(schema)
+	b.schema = b.buildKubeNative(schema, v2)
 	b.listSchema = b.buildListSchema()
 
 	return b

staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder_test.go

@@ -22,14 +22,14 @@ import (
 
 	"github.com/go-openapi/spec"
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
+	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
+	structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
 	"k8s.io/apimachinery/pkg/util/diff"
 	"k8s.io/apimachinery/pkg/util/json"
 	"k8s.io/apimachinery/pkg/util/sets"
 )
 
 func TestNewBuilder(t *testing.T) {
-	type args struct {
-	}
 	tests := []struct {
 		name string
 
@@ -37,41 +37,302 @@ func TestNewBuilder(t *testing.T) {
 
 		wantedSchema      string
 		wantedItemsSchema string
+
+		v2 bool // produce OpenAPIv2
 	}{
 		{
 			"nil",
 			"",
 			`{"type":"object","x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`, `{"$ref":"#/definitions/io.k8s.bar.v1.Foo"}`,
+			true,
 		},
-		{"empty",
+		{"with properties",
-			"{}",
+			`{"type":"object","properties":{"spec":{"type":"object"},"status":{"type":"object"}}}`,
-			`{"properties":{"apiVersion":{},"kind":{},"metadata":{}},"x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
+			`{"type":"object","properties":{"apiVersion":{"type":"string"},"kind":{"type":"string"},"metadata":{"$ref":"#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"},"spec":{"type":"object"},"status":{"type":"object"}},"x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
 			`{"$ref":"#/definitions/io.k8s.bar.v1.Foo"}`,
+			true,
 		},
-		{"empty properties",
+		{"type only",
-			`{"properties":{"spec":{},"status":{}}}`,
-			`{"properties":{"apiVersion":{},"kind":{},"metadata":{},"spec":{},"status":{}},"x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
-			`{"$ref":"#/definitions/io.k8s.bar.v1.Foo"}`,
-		},
-		{"filled properties",
-			`{"properties":{"spec":{"type":"object"},"status":{"type":"object"}}}`,
-			`{"properties":{"apiVersion":{},"kind":{},"metadata":{},"spec":{"type":"object"},"status":{"type":"object"}},"x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
-			`{"$ref":"#/definitions/io.k8s.bar.v1.Foo"}`,
-		},
-		{"type",
 			`{"type":"object"}`,
-			`{"properties":{"apiVersion":{},"kind":{},"metadata":{}},"type":"object","x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
+			`{"type":"object","properties":{"apiVersion":{"type":"string"},"kind":{"type":"string"},"metadata":{"$ref":"#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"}},"x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]}`,
 			`{"$ref":"#/definitions/io.k8s.bar.v1.Foo"}`,
+			true,
+		},
+		{"with extensions",
+			`
+{
+  "type":"object", 
+  "properties": {
+    "int-or-string-1": {
+      "x-kubernetes-int-or-string": true,
+      "anyOf": [
+        {"type":"integer"},
+        {"type":"string"}
+      ]
+    },
+    "int-or-string-2": {
+      "x-kubernetes-int-or-string": true,
+      "allOf": [{
+        "anyOf": [
+          {"type":"integer"},
+          {"type":"string"}
+        ]
+      }, {
+        "anyOf": [
+          {"minimum": 42.0}
+        ]
+      }]
+    },
+    "int-or-string-3": {
+      "x-kubernetes-int-or-string": true,
+      "anyOf": [
+        {"type":"integer"},
+        {"type":"string"}
+      ],
+      "allOf": [{
+        "anyOf": [
+          {"minimum": 42.0}
+        ]
+      }]
+    },
+    "int-or-string-4": {
+      "x-kubernetes-int-or-string": true,
+      "anyOf": [
+        {"minimum": 42.0}
+      ]
+    },
+    "int-or-string-5": {
+      "x-kubernetes-int-or-string": true,
+      "anyOf": [
+        {"minimum": 42.0}
+      ],
+      "allOf": [
+        {"minimum": 42.0}
+      ]
+    },
+    "int-or-string-6": {
+      "x-kubernetes-int-or-string": true
+    },
+    "preserve-unknown-fields": {
+      "x-kubernetes-preserve-unknown-fields": true
+    },
+    "embedded-object": {
+      "x-kubernetes-embedded-resource": true,
+      "x-kubernetes-preserve-unknown-fields": true,
+      "type": "object"
+    }
+  }
+}`,
+			`
+{
+  "type":"object",
+  "properties": {
+    "apiVersion": {"type":"string"},
+    "kind": {"type":"string"},
+    "metadata": {"$ref":"#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"},
+    "int-or-string-1": {
+      "x-kubernetes-int-or-string": true
+    },
+    "int-or-string-2": {
+      "x-kubernetes-int-or-string": true
+    },
+    "int-or-string-3": {
+      "x-kubernetes-int-or-string": true
+    },
+    "int-or-string-4": {
+      "x-kubernetes-int-or-string": true
+    },
+    "int-or-string-5": {
+      "x-kubernetes-int-or-string": true
+    },
+    "int-or-string-6": {
+      "x-kubernetes-int-or-string": true
+    },
+    "preserve-unknown-fields": {
+      "x-kubernetes-preserve-unknown-fields": true
+    },
+    "embedded-object": {
+      "x-kubernetes-embedded-resource": true,
+      "x-kubernetes-preserve-unknown-fields": true,
+      "type": "object"
+    }
+  },
+  "x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]
+}`,
+			`{"$ref":"#/definitions/io.k8s.bar.v1.Foo"}`,
+			true,
+		},
+		{"with extensions as v3 schema",
+			`
+{
+  "type":"object", 
+  "properties": {
+    "int-or-string-1": {
+      "x-kubernetes-int-or-string": true,
+      "anyOf": [
+        {"type":"integer"},
+        {"type":"string"}
+      ]
+    },
+    "int-or-string-2": {
+      "x-kubernetes-int-or-string": true,
+      "allOf": [{
+        "anyOf": [
+          {"type":"integer"},
+          {"type":"string"}
+        ]
+      }, {
+        "anyOf": [
+          {"minimum": 42.0}
+        ]
+      }]
+    },
+    "int-or-string-3": {
+      "x-kubernetes-int-or-string": true,
+      "anyOf": [
+        {"type":"integer"},
+        {"type":"string"}
+      ],
+      "allOf": [{
+        "anyOf": [
+          {"minimum": 42.0}
+        ]
+      }]
+    },
+    "int-or-string-4": {
+      "x-kubernetes-int-or-string": true,
+      "anyOf": [
+        {"minimum": 42.0}
+      ]
+    },
+    "int-or-string-5": {
+      "x-kubernetes-int-or-string": true,
+      "anyOf": [
+        {"minimum": 42.0}
+      ],
+      "allOf": [
+        {"minimum": 42.0}
+      ]
+    },
+    "int-or-string-6": {
+      "x-kubernetes-int-or-string": true
+    },
+    "preserve-unknown-fields": {
+      "x-kubernetes-preserve-unknown-fields": true
+    },
+    "embedded-object": {
+      "x-kubernetes-embedded-resource": true,
+      "x-kubernetes-preserve-unknown-fields": true,
+      "type": "object"
+    }
+  }
+}`,
+			`
+{
+  "type":"object",
+  "properties": {
+    "apiVersion": {"type":"string"},
+    "kind": {"type":"string"},
+    "metadata": {"$ref":"#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"},
+    "int-or-string-1": {
+      "x-kubernetes-int-or-string": true,
+      "anyOf": [
+        {"type":"integer"},
+        {"type":"string"}
+      ]
+    },
+    "int-or-string-2": {
+      "x-kubernetes-int-or-string": true,
+      "allOf": [{
+        "anyOf": [
+          {"type":"integer"},
+          {"type":"string"}
+        ]
+      }, {
+        "anyOf": [
+          {"minimum": 42.0}
+        ]
+      }]
+    },
+    "int-or-string-3": {
+      "x-kubernetes-int-or-string": true,
+      "anyOf": [
+        {"type":"integer"},
+        {"type":"string"}
+      ],
+      "allOf": [{
+        "anyOf": [
+          {"minimum": 42.0}
+        ]
+      }]
+    },
+    "int-or-string-4": {
+      "x-kubernetes-int-or-string": true,
+      "allOf": [{
+        "anyOf": [
+          {"type":"integer"},
+          {"type":"string"}
+        ]
+      }],
+      "anyOf": [
+        {"minimum": 42.0}
+      ]
+    },
+    "int-or-string-5": {
+      "x-kubernetes-int-or-string": true,
+      "anyOf": [
+        {"minimum": 42.0}
+      ],
+      "allOf": [{
+        "anyOf": [
+          {"type":"integer"},
+          {"type":"string"}
+        ]
+      }, {
+        "minimum": 42.0
+      }]
+    },
+    "int-or-string-6": {
+      "x-kubernetes-int-or-string": true,
+      "anyOf": [
+        {"type":"integer"},
+        {"type":"string"}
+      ]
+    },
+    "preserve-unknown-fields": {
+      "x-kubernetes-preserve-unknown-fields": true
+    },
+    "embedded-object": {
+      "x-kubernetes-embedded-resource": true,
+      "x-kubernetes-preserve-unknown-fields": true,
+      "type": "object"
+    }
+  },
+  "x-kubernetes-group-version-kind":[{"group":"bar.k8s.io","kind":"Foo","version":"v1"}]
+}`,
+			`{"$ref":"#/definitions/io.k8s.bar.v1.Foo"}`,
+			false,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			var schema *spec.Schema
+			var schema *structuralschema.Structural
 			if len(tt.schema) > 0 {
-				schema = &spec.Schema{}
+				v1beta1Schema := &v1beta1.JSONSchemaProps{}
-				if err := json.Unmarshal([]byte(tt.schema), schema); err != nil {
+				if err := json.Unmarshal([]byte(tt.schema), &v1beta1Schema); err != nil {
 					t.Fatal(err)
 				}
+				internalSchema := &apiextensions.JSONSchemaProps{}
+				v1beta1.Convert_v1beta1_JSONSchemaProps_To_apiextensions_JSONSchemaProps(v1beta1Schema, internalSchema, nil)
+				var err error
+				schema, err = structuralschema.NewStructural(internalSchema)
+				if err != nil {
+					t.Fatalf("structural schema error: %v", err)
+				}
+				if errs := structuralschema.ValidateStructural(schema, nil); len(errs) > 0 {
+					t.Fatalf("structural schema validation error: %v", errs.ToAggregate())
+				}
+				schema = schema.Unfold()
 			}
 
 			got := newBuilder(&apiextensions.CustomResourceDefinition{
@@ -86,7 +347,7 @@ func TestNewBuilder(t *testing.T) {
 					},
 					Scope: apiextensions.NamespaceScoped,
 				},
-			}, "v1", schema)
+			}, "v1", schema, tt.v2)
 
 			var wantedSchema, wantedItemsSchema spec.Schema
 			if err := json.Unmarshal([]byte(tt.wantedSchema), &wantedSchema); err != nil {
@@ -103,14 +364,12 @@ func TestNewBuilder(t *testing.T) {
 			}
 
 			// wipe out TypeMeta/ObjectMeta content, with those many lines of descriptions. We trust that they match here.
-			if _, found := got.schema.Properties["kind"]; found {
+			for _, metaField := range []string{"kind", "apiVersion", "metadata"} {
-				got.schema.Properties["kind"] = spec.Schema{}
+				if _, found := got.schema.Properties["kind"]; found {
-			}
+					prop := got.schema.Properties[metaField]
-			if _, found := got.schema.Properties["apiVersion"]; found {
+					prop.Description = ""
-				got.schema.Properties["apiVersion"] = spec.Schema{}
+					got.schema.Properties[metaField] = prop
-			}
+				}
-			if _, found := got.schema.Properties["metadata"]; found {
-				got.schema.Properties["metadata"] = spec.Schema{}
 			}
 
 			if !reflect.DeepEqual(&wantedSchema, got.schema) {

staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/conversion.go

@@ -17,106 +17,60 @@ limitations under the License.
 package openapi
 
 import (
-	"strings"
+	structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
-
-	"github.com/go-openapi/spec"
-
-	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
-	"k8s.io/apiextensions-apiserver/pkg/apiserver/validation"
 )
 
-// ConvertJSONSchemaPropsToOpenAPIv2Schema converts our internal OpenAPI v3 schema
+// ToStructuralOpenAPIV2 converts our internal OpenAPI v3 structural schema to
-// (*apiextensions.JSONSchemaProps) to an OpenAPI v2 schema (*spec.Schema).
+// to a v2 compatible schema.
-func ConvertJSONSchemaPropsToOpenAPIv2Schema(in *apiextensions.JSONSchemaProps) (*spec.Schema, error) {
+func ToStructuralOpenAPIV2(in *structuralschema.Structural) *structuralschema.Structural {
 	if in == nil {
-		return nil, nil
+		return nil
 	}
 
-	// dirty hack to temporarily set the type at the root. See continuation at the func bottom.
+	out := in.DeepCopy()
-	// TODO: remove for Kubernetes 1.15
-	oldRootType := in.Type
-	if len(in.Type) == 0 {
-		in.Type = "object"
-	}
 
 	// Remove unsupported fields in OpenAPI v2 recursively
-	out := new(spec.Schema)
+	mapper := structuralschema.Visitor{
-	validation.ConvertJSONSchemaPropsWithPostProcess(in, out, func(p *spec.Schema) error {
+		Structural: func(s *structuralschema.Structural) bool {
-		p.OneOf = nil
+			changed := false
-		// TODO(roycaihw): preserve cases where we only have one subtree in AnyOf, same for OneOf
+			if s.ValueValidation != nil {
-		p.AnyOf = nil
+				if s.ValueValidation.AllOf != nil {
-		p.Not = nil
+					s.ValueValidation.AllOf = nil
-
+					changed = true
-		// TODO: drop everything below in 1.15 when we have passed one version skew towards kube-openapi in <1.14, which rejects valid openapi schemata
+				}
-
+				if s.ValueValidation.OneOf != nil {
-		if p.Ref.String() != "" {
+					s.ValueValidation.OneOf = nil
-			// https://github.com/kubernetes/kube-openapi/pull/143/files#diff-62afddb578e9db18fb32ffb6b7802d92R95
+					changed = true
-			p.Properties = nil
+				}
-
+				if s.ValueValidation.AnyOf != nil {
-			// https://github.com/kubernetes/kube-openapi/pull/143/files#diff-62afddb578e9db18fb32ffb6b7802d92R99
+					s.ValueValidation.AnyOf = nil
-			p.Type = nil
+					changed = true
-
+				}
-			// https://github.com/kubernetes/kube-openapi/pull/143/files#diff-62afddb578e9db18fb32ffb6b7802d92R104
+				if s.ValueValidation.Not != nil {
-			if !strings.HasPrefix(p.Ref.String(), "#/definitions/") {
+					s.ValueValidation.Not = nil
-				p.Ref = spec.Ref{}
+					changed = true
-			}
-		}
-
-		switch {
-		case len(p.Type) == 2 && (p.Type[0] == "null" || p.Type[1] == "null"):
-			// https://github.com/kubernetes/kube-openapi/pull/143/files#diff-ce77fea74b9dd098045004410023e0c3R219
-			p.Type = nil
-		case len(p.Type) == 1:
-			switch p.Type[0] {
-			case "null":
-				// https://github.com/kubernetes/kube-openapi/pull/143/files#diff-ce77fea74b9dd098045004410023e0c3R219
-				p.Type = nil
-			case "array":
-				// https://github.com/kubernetes/kube-openapi/pull/143/files#diff-62afddb578e9db18fb32ffb6b7802d92R183
-				// https://github.com/kubernetes/kube-openapi/pull/143/files#diff-62afddb578e9db18fb32ffb6b7802d92R184
-				if p.Items == nil || (p.Items.Schema == nil && len(p.Items.Schemas) != 1) {
-					p.Type = nil
-					p.Items = nil
 				}
 			}
-		case len(p.Type) > 1:
-			// https://github.com/kubernetes/kube-openapi/pull/143/files#diff-62afddb578e9db18fb32ffb6b7802d92R272
-			// We also set Properties to null to enforce parseArbitrary at https://github.com/kubernetes/kube-openapi/blob/814a8073653e40e0e324205d093770d4e7bb811f/pkg/util/proto/document.go#L247
-			p.Type = nil
-			p.Properties = nil
-		default:
-			// https://github.com/kubernetes/kube-openapi/pull/143/files#diff-62afddb578e9db18fb32ffb6b7802d92R248
-			p.Properties = nil
-		}
 
-		// normalize items
+			// https://github.com/kubernetes/kube-openapi/pull/143/files#diff-ce77fea74b9dd098045004410023e0c3R219
-		if p.Items != nil && len(p.Items.Schemas) == 1 {
+			if s.Nullable {
-			p.Items = &spec.SchemaOrArray{Schema: &p.Items.Schemas[0]}
+				s.Type = ""
-		}
+				s.Nullable = false
 
-		// general fixups not supported by gnostic
+				// untyped values break if items or properties are set in kubectl
-		p.ID = ""
+				// https://github.com/kubernetes/kube-openapi/pull/143/files#diff-62afddb578e9db18fb32ffb6b7802d92R183
-		p.Schema = ""
+				s.Items = nil
-		p.Definitions = nil
+				s.Properties = nil
-		p.AdditionalItems = nil
-		p.Dependencies = nil
-		p.PatternProperties = nil
-		if p.ExternalDocs != nil && len(p.ExternalDocs.URL) == 0 {
-			p.ExternalDocs = nil
-		}
-		if p.Items != nil && p.Items.Schemas != nil {
-			p.Items = nil
-		}
 
-		return nil
+				changed = true
-	})
+			}
 
-	// restore root level type in input, and remove it in output if we had added it
+			return changed
-	// TODO: remove with Kubernetes 1.15
+		},
-	in.Type = oldRootType
+		// we drop all junctors above, and hence, never reach nested value validations
-	if len(oldRootType) == 0 {
+		NestedValueValidation: nil,
-		out.Type = nil
 	}
+	mapper.Visit(out)
 
-	return out, nil
+	return out
 }

staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/conversion_test.go

@@ -26,13 +26,15 @@ import (
 	"time"
 
 	"github.com/go-openapi/spec"
-	"github.com/google/gofuzz"
+	"github.com/google/go-cmp/cmp"
-	"github.com/googleapis/gnostic/OpenAPIv2"
+	fuzz "github.com/google/gofuzz"
+	openapi_v2 "github.com/googleapis/gnostic/OpenAPIv2"
 	"github.com/googleapis/gnostic/compiler"
 	"gopkg.in/yaml.v2"
+
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
 	apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
-	"k8s.io/apimachinery/pkg/util/diff"
+	structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
 	"k8s.io/kube-openapi/pkg/util/proto"
 )
 
@@ -94,11 +96,14 @@ properties:
 		t.Fatal(err)
 	}
 
-	schema, err := ConvertJSONSchemaPropsToOpenAPIv2Schema(&specInternal)
+	ss, err := structuralschema.NewStructural(&specInternal)
 	if err != nil {
 		t.Fatal(err)
 	}
 
+	ssV2 := ToStructuralOpenAPIV2(ss)
+	schema := ssV2.ToGoOpenAPI()
+
 	if _, found := schema.Properties["spec"]; !found {
 		t.Errorf("spec not found")
 	}
@@ -107,7 +112,7 @@ properties:
 	}
 }
 
-func Test_ConvertJSONSchemaPropsToOpenAPIv2SchemaFuzzing(t *testing.T) {
+func Test_ConvertJSONSchemaPropsToOpenAPIv2SchemaByType(t *testing.T) {
 	testStr := "test"
 	testStr2 := "test2"
 	testFloat64 := float64(6.4)
@@ -115,41 +120,32 @@ func Test_ConvertJSONSchemaPropsToOpenAPIv2SchemaFuzzing(t *testing.T) {
 	testApiextensionsJSON := apiextensions.JSON(testStr)
 
 	tests := []struct {
-		name     string
+		name        string
-		in       *apiextensions.JSONSchemaProps
+		in          *apiextensions.JSONSchemaProps
-		expected *spec.Schema
+		expected    *spec.Schema
+		expectError bool
+		expectDiff  bool
 	}{
 		{
 			name: "id",
 			in: &apiextensions.JSONSchemaProps{
 				ID: testStr,
 			},
-			expected: new(spec.Schema),
+			expectError: true, // rejected by kube validation and NewStructural
-			// not supported by gnostic
-			// expected: new(spec.Schema).
-			// 	WithID(testStr),
 		},
 		{
 			name: "$schema",
 			in: &apiextensions.JSONSchemaProps{
 				Schema: "test",
 			},
-			expected: new(spec.Schema),
+			expectError: true, // rejected by kube validation and NewStructural
-			// not supported by gnostic
-			// expected: &spec.Schema{
-			// 	SchemaProps: spec.SchemaProps{
-			// 		Schema: "test",
-			// 	},
-			// },
 		},
 		{
 			name: "$ref",
 			in: &apiextensions.JSONSchemaProps{
 				Ref: &testStr,
 			},
-			expected: new(spec.Schema),
+			expectError: true, // rejected by kube validation and NewStructural
-			// https://github.com/kubernetes/kube-openapi/pull/143/files#diff-62afddb578e9db18fb32ffb6b7802d92R104
-			// expected: spec.RefSchema(testStr),
 		},
 		{
 			name: "description",
@@ -168,6 +164,14 @@ func Test_ConvertJSONSchemaPropsToOpenAPIv2SchemaFuzzing(t *testing.T) {
 			expected: new(spec.Schema).
 				Typed(testStr, testStr2),
 		},
+		{
+			name: "nullable",
+			in: &apiextensions.JSONSchemaProps{
+				Type:     "object",
+				Nullable: true,
+			},
+			expected: new(spec.Schema),
+		},
 		{
 			name: "title",
 			in: &apiextensions.JSONSchemaProps{
@@ -317,18 +321,7 @@ func Test_ConvertJSONSchemaPropsToOpenAPIv2SchemaFuzzing(t *testing.T) {
 					},
 				},
 			},
-			expected: new(spec.Schema),
+			expectError: true, // rejected by kube validation and NewStructural
-			// https://github.com/kubernetes/kube-openapi/pull/143/files#diff-62afddb578e9db18fb32ffb6b7802d92R272
-			// expected: &spec.Schema{
-			// 	SchemaProps: spec.SchemaProps{
-			// 		Items: &spec.SchemaOrArray{
-			// 			Schemas: []spec.Schema{
-			// 				*spec.BooleanProperty(),
-			// 				*spec.StringProperty(),
-			// 			},
-			// 		},
-			// 	},
-			// },
 		},
 		{
 			name: "allOf",
@@ -338,8 +331,10 @@ func Test_ConvertJSONSchemaPropsToOpenAPIv2SchemaFuzzing(t *testing.T) {
 					{Type: "string"},
 				},
 			},
-			expected: new(spec.Schema).
+			expected: new(spec.Schema),
-				WithAllOf(*spec.BooleanProperty(), *spec.StringProperty()),
+			// intentionally not exported in v2
+			// expected: new(spec.Schema).
+			//   WithAllOf(*spec.BooleanProperty(), *spec.StringProperty()),
 		},
 		{
 			name: "oneOf",
@@ -471,8 +466,10 @@ func Test_ConvertJSONSchemaPropsToOpenAPIv2SchemaFuzzing(t *testing.T) {
 					},
 				},
 			},
-			expected: new(spec.Schema).
+			expected: new(spec.Schema),
-				WithAllOf(spec.Schema{}, spec.Schema{}, spec.Schema{}, *spec.StringProperty()),
+			// not supported by OpenAPI v2 + allOf intentionally not exported
+			// expected: new(spec.Schema).
+			//	WithAllOf(spec.Schema{}, spec.Schema{}, spec.Schema{}, *spec.StringProperty()),
 		},
 		{
 			name: "properties",
@@ -485,22 +482,39 @@ func Test_ConvertJSONSchemaPropsToOpenAPIv2SchemaFuzzing(t *testing.T) {
 				SetProperty(testStr, *spec.BooleanProperty()),
 		},
 		{
-			name: "additionalProperties",
+			name: "additionalProperties schema",
 			in: &apiextensions.JSONSchemaProps{
 				AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{
-					Allows: true,
+					Allows: false,
 					Schema: &apiextensions.JSONSchemaProps{Type: "boolean"},
 				},
 			},
 			expected: &spec.Schema{
 				SchemaProps: spec.SchemaProps{
 					AdditionalProperties: &spec.SchemaOrBool{
-						Allows: true,
+						Allows: false,
 						Schema: spec.BooleanProperty(),
 					},
 				},
 			},
 		},
+		{
+			name: "additionalProperties bool",
+			in: &apiextensions.JSONSchemaProps{
+				AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{
+					Allows: true,
+					Schema: nil,
+				},
+			},
+			expected: &spec.Schema{
+				SchemaProps: spec.SchemaProps{
+					AdditionalProperties: &spec.SchemaOrBool{
+						Allows: true,
+						Schema: nil,
+					},
+				},
+			},
+		},
 		{
 			name: "patternProperties",
 			in: &apiextensions.JSONSchemaProps{
@@ -508,15 +522,7 @@ func Test_ConvertJSONSchemaPropsToOpenAPIv2SchemaFuzzing(t *testing.T) {
 					testStr: {Type: "boolean"},
 				},
 			},
-			expected: new(spec.Schema),
+			expectError: true, // rejected by kube validation and NewStructural
-			// not supported by gnostic
-			// expected: &spec.Schema{
-			// 	SchemaProps: spec.SchemaProps{
-			// 		PatternProperties: map[string]spec.Schema{
-			// 			testStr: *spec.BooleanProperty(),
-			// 		},
-			// 	},
-			// },
 		},
 		{
 			name: "dependencies schema",
@@ -527,17 +533,7 @@ func Test_ConvertJSONSchemaPropsToOpenAPIv2SchemaFuzzing(t *testing.T) {
 					},
 				},
 			},
-			expected: new(spec.Schema),
+			expectError: true, // rejected by kube validation and NewStructural
-			// not supported by gnostic
-			// expected: &spec.Schema{
-			// 	SchemaProps: spec.SchemaProps{
-			// 		Dependencies: spec.Dependencies{
-			// 			testStr: spec.SchemaOrStringArray{
-			// 				Schema: spec.BooleanProperty(),
-			// 			},
-			// 		},
-			// 	},
-			// },
 		},
 		{
 			name: "dependencies string array",
@@ -548,17 +544,7 @@ func Test_ConvertJSONSchemaPropsToOpenAPIv2SchemaFuzzing(t *testing.T) {
 					},
 				},
 			},
-			expected: new(spec.Schema),
+			expectError: true, // rejected by kube validation and NewStructural
-			// not supported by gnostic
-			// expected: &spec.Schema{
-			// 	SchemaProps: spec.SchemaProps{
-			// 		Dependencies: spec.Dependencies{
-			// 			testStr: spec.SchemaOrStringArray{
-			// 				Property: []string{testStr2},
-			// 			},
-			// 		},
-			// 	},
-			// },
 		},
 		{
 			name: "additionalItems",
@@ -568,16 +554,7 @@ func Test_ConvertJSONSchemaPropsToOpenAPIv2SchemaFuzzing(t *testing.T) {
 					Schema: &apiextensions.JSONSchemaProps{Type: "boolean"},
 				},
 			},
-			expected: new(spec.Schema),
+			expectError: true, // rejected by kube validation and NewStructural
-			// not supported by gnostic
-			// expected: &spec.Schema{
-			// 	SchemaProps: spec.SchemaProps{
-			// 		AdditionalItems: &spec.SchemaOrBool{
-			// 			Allows: true,
-			// 			Schema: spec.BooleanProperty(),
-			// 		},
-			// 	},
-			// },
 		},
 		{
 			name: "definitions",
@@ -586,15 +563,7 @@ func Test_ConvertJSONSchemaPropsToOpenAPIv2SchemaFuzzing(t *testing.T) {
 					testStr: apiextensions.JSONSchemaProps{Type: "boolean"},
 				},
 			},
-			expected: new(spec.Schema),
+			expectError: true, // rejected by kube validation and NewStructural
-			// not supported by gnostic
-			// expected: &spec.Schema{
-			// 	SchemaProps: spec.SchemaProps{
-			// 		Definitions: spec.Definitions{
-			// 			testStr: *spec.BooleanProperty(),
-			// 		},
-			// 	},
-			// },
 		},
 		{
 			name: "externalDocs",
@@ -606,6 +575,7 @@ func Test_ConvertJSONSchemaPropsToOpenAPIv2SchemaFuzzing(t *testing.T) {
 			},
 			expected: new(spec.Schema).
 				WithExternalDocs(testStr, testStr2),
+			expectDiff: true,
 		},
 		{
 			name: "example",
@@ -614,115 +584,127 @@ func Test_ConvertJSONSchemaPropsToOpenAPIv2SchemaFuzzing(t *testing.T) {
 			},
 			expected: new(spec.Schema).
 				WithExample(testStr),
+			expectDiff: true,
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			out, err := ConvertJSONSchemaPropsToOpenAPIv2Schema(test.in)
+			ss, err := structuralschema.NewStructural(test.in)
-			if err != nil {
+			if err != nil && !test.expectError {
-				t.Fatalf("unexpected error in converting openapi schema: %v", err)
+				t.Fatalf("structural schema error: %v", err)
+			} else if err == nil && test.expectError {
+				t.Fatalf("expected NewStructural error, but didn't get any")
 			}
-			if !reflect.DeepEqual(*out, *test.expected) {
+
-				t.Errorf("unexpected result:\n  want=%v\n   got=%v\n\n%s", *test.expected, *out, diff.ObjectDiff(*test.expected, *out))
+			if !test.expectError {
+				out := ToStructuralOpenAPIV2(ss).ToGoOpenAPI()
+				if equal := reflect.DeepEqual(*out, *test.expected); !equal && !test.expectDiff {
+					t.Errorf("unexpected result:\n  want=%v\n   got=%v\n\n%s", *test.expected, *out, cmp.Diff(*test.expected, *out, cmp.Comparer(refEqual)))
+				} else if equal && test.expectDiff {
+					t.Errorf("expected diff, but didn't get any")
+				}
 			}
 		})
 	}
 }
 
+func refEqual(x spec.Ref, y spec.Ref) bool {
+	return x.String() == y.String()
+}
+
 // TestKubeOpenapiRejectionFiltering tests that the CRD openapi schema filtering leads to a spec that the
 // kube-openapi/pkg/util/proto model code support in version used in Kubernetes 1.13.
 func TestKubeOpenapiRejectionFiltering(t *testing.T) {
 	for i := 0; i < 10000; i++ {
-		t.Run(fmt.Sprintf("iteration %d", i), func(t *testing.T) {
+		f := fuzz.New()
-			f := fuzz.New()
+		seed := time.Now().UnixNano()
-			seed := time.Now().UnixNano()
+		randSource := rand.New(rand.NewSource(seed))
-			randSource := rand.New(rand.NewSource(seed))
+		f.RandSource(randSource)
-			f.RandSource(randSource)
+		t.Logf("iteration %d with seed %d", i, seed)
-			t.Logf("seed = %d", seed)
 
-			fuzzFuncs(f, func(ref *spec.Ref, c fuzz.Continue, visible bool) {
+		fuzzFuncs(f, func(ref *spec.Ref, c fuzz.Continue, visible bool) {
-				var url string
+			var url string
-				if c.RandBool() {
+			if c.RandBool() {
-					url = fmt.Sprintf("http://%d", c.Intn(100000))
+				url = fmt.Sprintf("http://%d", c.Intn(100000))
-				} else {
+			} else {
-					url = "#/definitions/test"
+				url = "#/definitions/test"
-				}
+			}
-				r, err := spec.NewRef(url)
+			r, err := spec.NewRef(url)
-				if err != nil {
-					t.Fatalf("failed to fuzz ref: %v", err)
-				}
-				*ref = r
-			})
-
-			// create go-openapi object and fuzz it (we start here because we have the powerful fuzzer already
-			s := &spec.Schema{}
-			f.Fuzz(s)
-
-			// convert to apiextensions v1beta1
-			bs, err := json.Marshal(s)
 			if err != nil {
-				t.Fatal(err)
+				t.Fatalf("failed to fuzz ref: %v", err)
-			}
-			t.Log(string(bs))
-
-			var schema *apiextensionsv1beta1.JSONSchemaProps
-			if err := json.Unmarshal(bs, &schema); err != nil {
-				t.Fatalf("failed to unmarshal JSON into apiextensions/v1beta1: %v", err)
-			}
-
-			// convert to internal
-			internalSchema := &apiextensions.JSONSchemaProps{}
-			if err := apiextensionsv1beta1.Convert_v1beta1_JSONSchemaProps_To_apiextensions_JSONSchemaProps(schema, internalSchema, nil); err != nil {
-				t.Fatalf("failed to convert from apiextensions/v1beta1 to internal: %v", err)
-			}
-
-			// apply the filter
-			filtered, err := ConvertJSONSchemaPropsToOpenAPIv2Schema(internalSchema)
-			if err != nil {
-				t.Fatalf("failed to filter: %v", err)
-			}
-
-			// create a doc out of it
-			filteredSwagger := &spec.Swagger{
-				SwaggerProps: spec.SwaggerProps{
-					Definitions: spec.Definitions{
-						"test": *filtered,
-					},
-					Info: &spec.Info{
-						InfoProps: spec.InfoProps{
-							Description: "test",
-							Version:     "test",
-							Title:       "test",
-						},
-					},
-					Swagger: "2.0",
-				},
-			}
-
-			// convert to JSON
-			bs, err = json.Marshal(filteredSwagger)
-			if err != nil {
-				t.Fatalf("failed to encode filtered to JSON: %v", err)
-			}
-
-			// unmarshal as yaml
-			var yml yaml.MapSlice
-			if err := yaml.Unmarshal(bs, &yml); err != nil {
-				t.Fatalf("failed to decode filtered JSON by into memory: %v", err)
-			}
-
-			// create gnostic doc
-			doc, err := openapi_v2.NewDocument(yml, compiler.NewContext("$root", nil))
-			if err != nil {
-				t.Fatalf("failed to create gnostic doc: %v", err)
-			}
-
-			// load with kube-openapi/pkg/util/proto
-			if _, err := proto.NewOpenAPIData(doc); err != nil {
-				t.Fatalf("failed to convert to kube-openapi/pkg/util/proto model: %v", err)
 			}
+			*ref = r
 		})
+
+		// create go-openapi object and fuzz it (we start here because we have the powerful fuzzer already
+		s := &spec.Schema{}
+		f.Fuzz(s)
+
+		// convert to apiextensions v1beta1
+		bs, err := json.Marshal(s)
+		if err != nil {
+			t.Fatal(err)
+		}
+		t.Log(string(bs))
+
+		var schema *apiextensionsv1beta1.JSONSchemaProps
+		if err := json.Unmarshal(bs, &schema); err != nil {
+			t.Fatalf("failed to unmarshal JSON into apiextensions/v1beta1: %v", err)
+		}
+
+		// convert to internal
+		internalSchema := &apiextensions.JSONSchemaProps{}
+		if err := apiextensionsv1beta1.Convert_v1beta1_JSONSchemaProps_To_apiextensions_JSONSchemaProps(schema, internalSchema, nil); err != nil {
+			t.Fatalf("failed to convert from apiextensions/v1beta1 to internal: %v", err)
+		}
+
+		// apply the filter
+		ss, err := structuralschema.NewStructural(internalSchema)
+		if err != nil {
+			t.Fatal(err)
+		}
+		filtered := ToStructuralOpenAPIV2(ss).ToGoOpenAPI()
+
+		// create a doc out of it
+		filteredSwagger := &spec.Swagger{
+			SwaggerProps: spec.SwaggerProps{
+				Definitions: spec.Definitions{
+					"test": *filtered,
+				},
+				Info: &spec.Info{
+					InfoProps: spec.InfoProps{
+						Description: "test",
+						Version:     "test",
+						Title:       "test",
+					},
+				},
+				Swagger: "2.0",
+			},
+		}
+
+		// convert to JSON
+		bs, err = json.Marshal(filteredSwagger)
+		if err != nil {
+			t.Fatalf("failed to encode filtered to JSON: %v", err)
+		}
+
+		// unmarshal as yaml
+		var yml yaml.MapSlice
+		if err := yaml.Unmarshal(bs, &yml); err != nil {
+			t.Fatalf("failed to decode filtered JSON by into memory: %v", err)
+		}
+
+		// create gnostic doc
+		doc, err := openapi_v2.NewDocument(yml, compiler.NewContext("$root", nil))
+		if err != nil {
+			t.Fatalf("failed to create gnostic doc: %v", err)
+		}
+
+		// load with kube-openapi/pkg/util/proto
+		if _, err := proto.NewOpenAPIData(doc); err != nil {
+			t.Fatalf("failed to convert to kube-openapi/pkg/util/proto model: %v", err)
+		}
 	}
 }
 
@@ -794,9 +776,11 @@ func fuzzFuncs(f *fuzz.Fuzzer, refFunc func(ref *spec.Ref, c fuzz.Continue, visi
 			if p.Default != nil {
 				p.Default = "42"
 			}
-			if p.Example != nil {
+			p.Example = nil
-				p.Example = "42"
+		},
-			}
+		func(s *spec.SwaggerSchemaProps, c fuzz.Continue) {
+			// nothing allowed
+			*s = spec.SwaggerSchemaProps{}
 		},
 		func(s *spec.SchemaProps, c fuzz.Continue) {
 			// gofuzz is broken and calls this even for *SchemaProps fields, ignoring NilChance, leading to infinite recursion
@@ -809,14 +793,26 @@ func fuzzFuncs(f *fuzz.Fuzzer, refFunc func(ref *spec.Ref, c fuzz.Continue, visi
 
 			c.FuzzNoCustom(s)
 
-			// we don't support multi-type schema props yet in apiextensions/v1beta1
+			if c.RandBool() {
-			if len(s.Type) > 1 {
+				types := []string{"object", "array", "boolean", "string", "integer", "number"}
-				s.Type = s.Type[:1]
+				s.Type = []string{types[c.Intn(len(types))]}
+			} else {
+				s.Type = nil
+			}
 
-				s := apiextensionsv1beta1.JSONSchemaProps{}
+			s.ID = ""
-				if reflect.TypeOf(s.Type).String() != "string" {
+			s.Ref = spec.Ref{}
-					panic(fmt.Errorf("this simplifaction is outdated: apiextensions/v1beta1 types not a single string anymore, but %T", s.Type))
+			s.AdditionalItems = nil
-				}
+			s.Dependencies = nil
+			s.Schema = ""
+			s.PatternProperties = nil
+			s.Definitions = nil
+
+			if len(s.Type) == 1 && s.Type[0] == "array" {
+				s.Items = &spec.SchemaOrArray{Schema: &spec.Schema{}}
+				c.Fuzz(s.Items.Schema)
+			} else {
+				s.Items = nil
 			}
 
 			// reset JSON fields to some correct JSON

test/integration/master/crd_test.go

@@ -160,6 +160,7 @@ func TestCRDOpenAPI(t *testing.T) {
 			},
 			Validation: &apiextensionsv1beta1.CustomResourceValidation{
 				OpenAPIV3Schema: &apiextensionsv1beta1.JSONSchemaProps{
+					Type: "object",
 					Properties: map[string]apiextensionsv1beta1.JSONSchemaProps{
 						"foo": {Type: "string"},
 					},