Merge pull request #77554 from sttts/sttts-structural-publishing

apiextensions: publish (only) structural OpenAPI schemas

staging/src/k8s.io/apiextensions-apiserver/go.mod

@@ -16,6 +16,7 @@ require (
 	github.com/go-openapi/strfmt v0.17.0
 	github.com/go-openapi/validate v0.18.0
 	github.com/gogo/protobuf v0.0.0-20171007142547-342cbe0a0415
+	github.com/google/go-cmp v0.3.0
 	github.com/google/gofuzz v0.0.0-20170612174753-24818f796faf
 	github.com/googleapis/gnostic v0.0.0-20170729233727-0c5108395e2d
 	github.com/inconshreveable/mousetrap v1.0.0 // indirect

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/BUILD

@@ -7,6 +7,7 @@ go_library(
         "convert.go",
         "goopenapi.go",
         "structural.go",
+        "unfold.go",
         "validation.go",
         "visitor.go",
         "zz_generated.deepcopy.go",

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/unfold.go

@@ -0,0 +1,63 @@
+/*
+Copyright 2019 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package schema
+
+// Unfold expands vendor extensions of a structural schema.
+// It mutates the receiver.
+func (s *Structural) Unfold() *Structural {
+	if s == nil {
+		return nil
+	}
+
+	mapper := Visitor{
+		Structural: func(s *Structural) bool {
+			if !s.XIntOrString {
+				return false
+			}
+
+			skipAnyOf := isIntOrStringAnyOfPattern(s)
+			skipFirstAllOfAnyOf := isIntOrStringAllOfPattern(s)
+			if skipAnyOf || skipFirstAllOfAnyOf {
+				return false
+			}
+
+			if s.AnyOf == nil {
+				s.AnyOf = []NestedValueValidation{
+					{ForbiddenGenerics: Generic{Type: "integer"}},
+					{ForbiddenGenerics: Generic{Type: "string"}},
+				}
+			} else {
+				s.AllOf = append([]NestedValueValidation{
+					{
+						ValueValidation: ValueValidation{
+							AnyOf: []NestedValueValidation{
+								{ForbiddenGenerics: Generic{Type: "integer"}},
+								{ForbiddenGenerics: Generic{Type: "string"}},
+							},
+						},
+					},
+				}, s.AllOf...)
+			}
+
+			return true
+		},
+		NestedValueValidation: nil, // x-kubernetes-int-or-string cannot be set in nested value validation
+	}
+	mapper.Visit(s)
+
+	return s
+}

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/validation.go

@@ -97,15 +97,8 @@ func validateStructuralInvariants(s *Structural, lvl level, fldPath *field.Path)
 	//      - type: integer
 	//      - type: string
 	//    - ... zero or more
-	skipAnyOf := false
+	skipAnyOf := isIntOrStringAnyOfPattern(s)
-	skipFirstAllOfAnyOf := false
+	skipFirstAllOfAnyOf := isIntOrStringAllOfPattern(s)
-	if s.XIntOrString && s.ValueValidation != nil {
-		if len(s.ValueValidation.AnyOf) == 2 && reflect.DeepEqual(s.ValueValidation.AnyOf, intOrStringAnyOf) {
-			skipAnyOf = true
-		} else if len(s.ValueValidation.AllOf) >= 1 && len(s.ValueValidation.AllOf[0].AnyOf) == 2 && reflect.DeepEqual(s.ValueValidation.AllOf[0].AnyOf, intOrStringAnyOf) {
-			skipFirstAllOfAnyOf = true
-		}
-	}
 
 	allErrs = append(allErrs, validateValueValidation(s.ValueValidation, skipAnyOf, skipFirstAllOfAnyOf, lvl, fldPath)...)
 
@@ -157,6 +150,20 @@ func validateStructuralInvariants(s *Structural, lvl level, fldPath *field.Path)
 	return allErrs
 }
 
+func isIntOrStringAnyOfPattern(s *Structural) bool {
+	if s == nil || s.ValueValidation == nil {
+		return false
+	}
+	return len(s.ValueValidation.AnyOf) == 2 && reflect.DeepEqual(s.ValueValidation.AnyOf, intOrStringAnyOf)
+}
+
+func isIntOrStringAllOfPattern(s *Structural) bool {
+	if s == nil || s.ValueValidation == nil {
+		return false
+	}
+	return len(s.ValueValidation.AllOf) >= 1 && len(s.ValueValidation.AllOf[0].AnyOf) == 2 && reflect.DeepEqual(s.ValueValidation.AllOf[0].AnyOf, intOrStringAnyOf)
+}
+
 // validateGeneric checks the generic fields of a structural schema.
 func validateGeneric(g *Generic, lvl level, fldPath *field.Path) field.ErrorList {
 	if g == nil {

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

@@ -14,7 +14,7 @@ go_library(
     deps = [
         "//staging/src/k8s.io/api/autoscaling/v1:go_default_library",
         "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library",
-        "//staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation:go_default_library",
+        "//staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema:go_default_library",
         "//staging/src/k8s.io/apiextensions-apiserver/pkg/client/informers/internalversion/apiextensions/internalversion:go_default_library",
         "//staging/src/k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/internalversion:go_default_library",
         "//staging/src/k8s.io/apiextensions-apiserver/pkg/generated/openapi:go_default_library",
@@ -49,10 +49,12 @@ go_test(
     deps = [
         "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library",
         "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1:go_default_library",
+        "//staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/json:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//vendor/github.com/go-openapi/spec:go_default_library",
+        "//vendor/github.com/google/go-cmp/cmp:go_default_library",
         "//vendor/github.com/google/gofuzz:go_default_library",
         "//vendor/github.com/googleapis/gnostic/OpenAPIv2:go_default_library",
         "//vendor/github.com/googleapis/gnostic/compiler:go_default_library",

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 {
-			WithDescription(swaggerPartialObjectMetadataDescriptions["metadata"]))
+			schema = ToStructuralOpenAPIV2(schema)
-		addTypeMetaProperties(schema)
 		}
-	schema.AddExtension(endpoints.ROUTE_META_GVK, []interface{}{
+		ret = schema.ToGoOpenAPI()
+		ret.SetProperty("metadata", *spec.RefSchema(objectMetaSchemaRef).
+			WithDescription(swaggerPartialObjectMetadataDescriptions["metadata"]))
+		addTypeMetaProperties(ret)
+	}
+	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.
+			for _, metaField := range []string{"kind", "apiVersion", "metadata"} {
 				if _, found := got.schema.Properties["kind"]; found {
-				got.schema.Properties["kind"] = spec.Schema{}
+					prop := got.schema.Properties[metaField]
+					prop.Description = ""
+					got.schema.Properties[metaField] = prop
 				}
-			if _, found := got.schema.Properties["apiVersion"]; found {
-				got.schema.Properties["apiVersion"] = spec.Schema{}
-			}
-			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
+			if s.Nullable {
-		case len(p.Type) == 1:
+				s.Type = ""
-			switch p.Type[0] {
+				s.Nullable = false
-			case "null":
+
-				// https://github.com/kubernetes/kube-openapi/pull/143/files#diff-ce77fea74b9dd098045004410023e0c3R219
+				// untyped values break if items or properties are set in kubectl
-				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
+				s.Items = nil
-				if p.Items == nil || (p.Items.Schema == nil && len(p.Items.Schemas) != 1) {
+				s.Properties = nil
-					p.Type = nil
+
-					p.Items = nil
+				changed = true
-				}
-			}
-		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
+			return changed
-		if p.Items != nil && len(p.Items.Schemas) == 1 {
+		},
-			p.Items = &spec.SchemaOrArray{Schema: &p.Items.Schemas[0]}
+		// we drop all junctors above, and hence, never reach nested value validations
+		NestedValueValidation: nil,
 	}
+	mapper.Visit(out)
 
-		// general fixups not supported by gnostic
+	return out
-		p.ID = ""
-		p.Schema = ""
-		p.Definitions = 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
-	})
-
-	// restore root level type in input, and remove it in output if we had added it
-	// TODO: remove with Kubernetes 1.15
-	in.Type = oldRootType
-	if len(oldRootType) == 0 {
-		out.Type = nil
-	}
-
-	return out, nil
 }

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)
@@ -118,38 +123,29 @@ func Test_ConvertJSONSchemaPropsToOpenAPIv2SchemaFuzzing(t *testing.T) {
 		name        string
 		in          *apiextensions.JSONSchemaProps
 		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,32 +584,44 @@ 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 !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")
 				}
-			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))
 			}
 		})
 	}
 }
 
+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()
 		seed := time.Now().UnixNano()
 		randSource := rand.New(rand.NewSource(seed))
 		f.RandSource(randSource)
-			t.Logf("seed = %d", seed)
+		t.Logf("iteration %d with seed %d", i, seed)
 
 		fuzzFuncs(f, func(ref *spec.Ref, c fuzz.Continue, visible bool) {
 			var url string
@@ -678,10 +660,11 @@ func TestKubeOpenapiRejectionFiltering(t *testing.T) {
 		}
 
 		// apply the filter
-			filtered, err := ConvertJSONSchemaPropsToOpenAPIv2Schema(internalSchema)
+		ss, err := structuralschema.NewStructural(internalSchema)
 		if err != nil {
-				t.Fatalf("failed to filter: %v", err)
+			t.Fatal(err)
 		}
+		filtered := ToStructuralOpenAPIV2(ss).ToGoOpenAPI()
 
 		// create a doc out of it
 		filteredSwagger := &spec.Swagger{
@@ -722,7 +705,6 @@ func TestKubeOpenapiRejectionFiltering(t *testing.T) {
 		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 := apiextensionsv1beta1.JSONSchemaProps{}
+				s.Type = nil
-				if reflect.TypeOf(s.Type).String() != "string" {
-					panic(fmt.Errorf("this simplifaction is outdated: apiextensions/v1beta1 types not a single string anymore, but %T", s.Type))
 			}
+
+			s.ID = ""
+			s.Ref = spec.Ref{}
+			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"},
 					},