Merge pull request #77667 from sttts/sttts-structural-schema-go-openapi

apiextensions: roundtrip structural schema through go-openapi, JSON and our API

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

@@ -5,8 +5,10 @@ go_library(
     srcs = [
         "complete.go",
         "convert.go",
+        "goopenapi.go",
         "structural.go",
         "validation.go",
+        "visitor.go",
         "zz_generated.deepcopy.go",
     ],
     importmap = "k8s.io/kubernetes/vendor/k8s.io/apiextensions-apiserver/pkg/apiserver/schema",
@@ -16,6 +18,7 @@ go_library(
         "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
+        "//vendor/github.com/go-openapi/spec:go_default_library",
     ],
 )
 
@@ -35,9 +38,17 @@ filegroup(
 
 go_test(
     name = "go_default_test",
-    srcs = ["validation_test.go"],
+    srcs = [
+        "convert_test.go",
+        "goopenapi_test.go",
+        "validation_test.go",
+    ],
     embed = [":go_default_library"],
     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/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/rand:go_default_library",
         "//vendor/github.com/google/gofuzz:go_default_library",
     ],

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

@@ -0,0 +1,112 @@
+/*
+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
+
+import (
+	"math/rand"
+	"reflect"
+	"testing"
+	"time"
+
+	fuzz "github.com/google/gofuzz"
+
+	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
+	apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
+	"k8s.io/apimachinery/pkg/util/diff"
+	"k8s.io/apimachinery/pkg/util/json"
+)
+
+func TestStructuralRoundtripOrError(t *testing.T) {
+	f := fuzz.New()
+	seed := time.Now().UnixNano()
+	t.Logf("seed = %v", seed)
+	//seed = int64(1549012506261785182)
+	f.RandSource(rand.New(rand.NewSource(seed)))
+	f.Funcs(
+		func(s *apiextensions.JSON, c fuzz.Continue) {
+			*s = apiextensions.JSON(map[string]interface{}{"foo": float64(42.2)})
+		},
+		func(s *apiextensions.JSONSchemaPropsOrArray, c fuzz.Continue) {
+			c.FuzzNoCustom(s)
+			if s.Schema != nil {
+				s.JSONSchemas = nil
+			} else if s.JSONSchemas == nil {
+				s.Schema = &apiextensions.JSONSchemaProps{}
+			}
+		},
+		func(s *apiextensions.JSONSchemaPropsOrBool, c fuzz.Continue) {
+			c.FuzzNoCustom(s)
+			if s.Schema != nil {
+				s.Allows = false
+			}
+		},
+		func(s **string, c fuzz.Continue) {
+			c.FuzzNoCustom(s)
+			if *s != nil && **s == "" {
+				*s = nil
+			}
+		},
+	)
+
+	f.MaxDepth(2)
+	f.NilChance(0.5)
+
+	for i := 0; i < 10000; i++ {
+		// fuzz a random field in JSONSchemaProps
+		origSchema := &apiextensions.JSONSchemaProps{}
+		x := reflect.ValueOf(origSchema).Elem()
+		n := rand.Intn(x.NumField())
+		if name := x.Type().Field(n).Name; name == "Example" || name == "ExternalDocs" {
+			// we drop these intentionally
+			continue
+		}
+		f.Fuzz(x.Field(n).Addr().Interface())
+		if origSchema.Nullable {
+			// non-empty type for nullable. nullable:true with empty type does not roundtrip because
+			// go-openapi does not allow to encode that (we use type slices otherwise).
+			origSchema.Type = "string"
+		}
+
+		// it roundtrips or NewStructural errors out. We should never drop anything
+		orig, err := NewStructural(origSchema)
+		if err != nil {
+			continue
+		}
+
+		// roundtrip through go-openapi, JSON, v1beta1 JSONSchemaProp, internal JSONSchemaProp
+		goOpenAPI := orig.ToGoOpenAPI()
+		bs, err := json.Marshal(goOpenAPI)
+		if err != nil {
+			t.Fatal(err)
+		}
+		str := nullTypeRE.ReplaceAllString(string(bs), `"type":"$1","nullable":true`) // unfold nullable type:[<type>,"null"] -> type:<type>,nullable:true
+		v1beta1Schema := &apiextensionsv1beta1.JSONSchemaProps{}
+		err = json.Unmarshal([]byte(str), v1beta1Schema)
+		if err != nil {
+			t.Fatal(err)
+		}
+		internalSchema := &apiextensions.JSONSchemaProps{}
+		err = apiextensionsv1beta1.Convert_v1beta1_JSONSchemaProps_To_apiextensions_JSONSchemaProps(v1beta1Schema, internalSchema, nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if !reflect.DeepEqual(origSchema, internalSchema) {
+			t.Fatalf("original and result differ: %v", diff.ObjectDiff(origSchema, internalSchema))
+		}
+	}
+}

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

@@ -0,0 +1,154 @@
+/*
+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
+
+import (
+	"github.com/go-openapi/spec"
+)
+
+// ToGoOpenAPI converts a structural schema to go-openapi schema. It is faithful and roundtrippable
+// with the exception of `nullable:true` for empty type (`type:""`).
+//
+// WARNING: Do not use the returned schema to perform CRD validation until this restriction is solved.
+//
+// Nullable:true is mapped to `type:[<structural-type>,"null"]`
+// if the structural type is non-empty, and nullable is dropped if the structural type is empty.
+func (s *Structural) ToGoOpenAPI() *spec.Schema {
+	if s == nil {
+		return nil
+	}
+
+	ret := &spec.Schema{}
+
+	if s.Items != nil {
+		ret.Items = &spec.SchemaOrArray{Schema: s.Items.ToGoOpenAPI()}
+	}
+	if s.Properties != nil {
+		ret.Properties = make(map[string]spec.Schema, len(s.Properties))
+		for k, v := range s.Properties {
+			ret.Properties[k] = *v.ToGoOpenAPI()
+		}
+	}
+	s.Generic.toGoOpenAPI(ret)
+	s.Extensions.toGoOpenAPI(ret)
+	s.ValueValidation.toGoOpenAPI(ret)
+
+	return ret
+}
+
+func (g *Generic) toGoOpenAPI(ret *spec.Schema) {
+	if g == nil {
+		return
+	}
+
+	if len(g.Type) != 0 {
+		ret.Type = spec.StringOrArray{g.Type}
+		if g.Nullable {
+			// go-openapi does not support nullable, but multiple type values.
+			// Only when type is already non-empty, adding null to the types is correct though.
+			// If you add null as only type, you enforce null, in contrast to nullable being
+			// ineffective if no type is provided in a schema.
+			ret.Type = append(ret.Type, "null")
+		}
+	}
+	if g.AdditionalProperties != nil {
+		ret.AdditionalProperties = &spec.SchemaOrBool{
+			Allows: g.AdditionalProperties.Bool,
+			Schema: g.AdditionalProperties.Structural.ToGoOpenAPI(),
+		}
+	}
+	ret.Description = g.Description
+	ret.Title = g.Title
+	ret.Default = g.Default.Object
+}
+
+func (x *Extensions) toGoOpenAPI(ret *spec.Schema) {
+	if x == nil {
+		return
+	}
+
+	if x.XPreserveUnknownFields {
+		ret.VendorExtensible.AddExtension("x-kubernetes-preserve-unknown-fields", true)
+	}
+	if x.XEmbeddedResource {
+		ret.VendorExtensible.AddExtension("x-kubernetes-embedded-resource", true)
+	}
+	if x.XIntOrString {
+		ret.VendorExtensible.AddExtension("x-kubernetes-int-or-string", true)
+	}
+}
+
+func (v *ValueValidation) toGoOpenAPI(ret *spec.Schema) {
+	if v == nil {
+		return
+	}
+
+	ret.Format = v.Format
+	ret.Maximum = v.Maximum
+	ret.ExclusiveMaximum = v.ExclusiveMaximum
+	ret.Minimum = v.Minimum
+	ret.ExclusiveMinimum = v.ExclusiveMinimum
+	ret.MaxLength = v.MaxLength
+	ret.MinLength = v.MinLength
+	ret.Pattern = v.Pattern
+	ret.MaxItems = v.MaxItems
+	ret.MinItems = v.MinItems
+	ret.UniqueItems = v.UniqueItems
+	ret.MultipleOf = v.MultipleOf
+	if v.Enum != nil {
+		ret.Enum = make([]interface{}, 0, len(v.Enum))
+		for i := range v.Enum {
+			ret.Enum = append(ret.Enum, v.Enum[i].Object)
+		}
+	}
+	ret.MaxProperties = v.MaxProperties
+	ret.MinProperties = v.MinProperties
+	ret.Required = v.Required
+	for i := range v.AllOf {
+		ret.AllOf = append(ret.AllOf, *v.AllOf[i].toGoOpenAPI())
+	}
+	for i := range v.AnyOf {
+		ret.AnyOf = append(ret.AnyOf, *v.AnyOf[i].toGoOpenAPI())
+	}
+	for i := range v.OneOf {
+		ret.OneOf = append(ret.OneOf, *v.OneOf[i].toGoOpenAPI())
+	}
+	ret.Not = v.Not.toGoOpenAPI()
+}
+
+func (vv *NestedValueValidation) toGoOpenAPI() *spec.Schema {
+	if vv == nil {
+		return nil
+	}
+
+	ret := &spec.Schema{}
+
+	vv.ValueValidation.toGoOpenAPI(ret)
+	if vv.Items != nil {
+		ret.Items = &spec.SchemaOrArray{Schema: vv.Items.toGoOpenAPI()}
+	}
+	if vv.Properties != nil {
+		ret.Properties = make(map[string]spec.Schema, len(vv.Properties))
+		for k, v := range vv.Properties {
+			ret.Properties[k] = *v.toGoOpenAPI()
+		}
+	}
+	vv.ForbiddenGenerics.toGoOpenAPI(ret)   // normally empty. Exception: int-or-string
+	vv.ForbiddenExtensions.toGoOpenAPI(ret) // shouldn't do anything
+
+	return ret
+}

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

@@ -0,0 +1,116 @@
+/*
+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
+
+import (
+	"math/rand"
+	"reflect"
+	"regexp"
+	"testing"
+	"time"
+
+	fuzz "github.com/google/gofuzz"
+
+	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
+	apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
+	"k8s.io/apimachinery/pkg/util/diff"
+	"k8s.io/apimachinery/pkg/util/json"
+)
+
+var nullTypeRE = regexp.MustCompile(`"type":\["([^"]*)","null"]`)
+
+func TestStructuralRoundtrip(t *testing.T) {
+	f := fuzz.New()
+	seed := time.Now().UnixNano()
+	t.Logf("seed = %v", seed)
+	//seed = int64(1549012506261785182)
+	f.RandSource(rand.New(rand.NewSource(seed)))
+	f.Funcs(
+		func(s *JSON, c fuzz.Continue) {
+			switch c.Intn(6) {
+			case 0:
+				s.Object = float64(42.0)
+			case 1:
+				s.Object = map[string]interface{}{"foo": "bar"}
+			case 2:
+				s.Object = ""
+			case 3:
+				s.Object = []string{}
+			case 4:
+				s.Object = map[string]interface{}{}
+			case 5:
+				s.Object = nil
+			}
+		},
+		func(g *Generic, c fuzz.Continue) {
+			c.FuzzNoCustom(g)
+
+			// TODO: make nullable in case of empty type survive go-openapi JSON -> API schema roundtrip
+			// go-openapi does not support nullable. Adding it to a type slice produces OpenAPI v3
+			// incompatible JSON which we cannot unmarshal (without string-replace magic to transform
+			// null types back into nullable). If type is empty, nullable:true is not preserved
+			// at all.
+			if len(g.Type) == 0 {
+				g.Nullable = false
+			}
+		},
+	)
+	f.MaxDepth(3)
+	f.NilChance(0.5)
+
+	for i := 0; i < 10000; i++ {
+		orig := &Structural{}
+		f.Fuzz(orig)
+
+		// normalize Structural.ValueValidation to zero values if it was nil before
+		normalizer := Visitor{
+			Structural: func(s *Structural) bool {
+				if s.ValueValidation == nil {
+					s.ValueValidation = &ValueValidation{}
+					return true
+				}
+				return false
+			},
+		}
+		normalizer.Visit(orig)
+
+		goOpenAPI := orig.ToGoOpenAPI()
+		bs, err := json.Marshal(goOpenAPI)
+		if err != nil {
+			t.Fatal(err)
+		}
+		str := nullTypeRE.ReplaceAllString(string(bs), `"type":"$1","nullable":true`) // unfold nullable type:[<type>,"null"] -> type:<type>,nullable:true
+		v1beta1Schema := &apiextensionsv1beta1.JSONSchemaProps{}
+		err = json.Unmarshal([]byte(str), v1beta1Schema)
+		if err != nil {
+			t.Fatal(err)
+		}
+		internalSchema := &apiextensions.JSONSchemaProps{}
+		err = apiextensionsv1beta1.Convert_v1beta1_JSONSchemaProps_To_apiextensions_JSONSchemaProps(v1beta1Schema, internalSchema, nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		s, err := NewStructural(internalSchema)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if !reflect.DeepEqual(orig, s) {
+			t.Fatalf("original and result differ: %v", diff.ObjectDiff(orig, s))
+		}
+	}
+}

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

@@ -0,0 +1,106 @@
+/*
+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
+
+// Visitor recursively walks through a structural schema.
+type Visitor struct {
+	// Structural is called on each Structural node in the schema, before recursing into
+	// the subtrees. It is allowed to mutate s. Return true if something has been changed.
+	// +optional
+	Structural func(s *Structural) bool
+	// NestedValueValidation is called on each NestedValueValidation node in the schema,
+	// before recursing into subtrees. It is allowed to mutate vv. Return true if something
+	// has been changed.
+	// +optional
+	NestedValueValidation func(vv *NestedValueValidation) bool
+}
+
+// Visit recursively walks through the structural schema and calls the given callbacks
+// at each node of those types.
+func (m *Visitor) Visit(s *Structural) {
+	m.visitStructural(s)
+}
+
+func (m *Visitor) visitStructural(s *Structural) bool {
+	ret := false
+	if m.Structural != nil {
+		ret = m.Structural(s)
+	}
+
+	if s.Items != nil {
+		m.visitStructural(s.Items)
+	}
+	for k, v := range s.Properties {
+		if changed := m.visitStructural(&v); changed {
+			ret = true
+			s.Properties[k] = v
+		}
+	}
+	if s.Generic.AdditionalProperties != nil && s.Generic.AdditionalProperties.Structural != nil {
+		m.visitStructural(s.Generic.AdditionalProperties.Structural)
+	}
+	if s.ValueValidation != nil {
+		for i := range s.ValueValidation.AllOf {
+			m.visitNestedValueValidation(&s.ValueValidation.AllOf[i])
+		}
+		for i := range s.ValueValidation.AnyOf {
+			m.visitNestedValueValidation(&s.ValueValidation.AnyOf[i])
+		}
+		for i := range s.ValueValidation.OneOf {
+			m.visitNestedValueValidation(&s.ValueValidation.OneOf[i])
+		}
+		if s.ValueValidation.Not != nil {
+			m.visitNestedValueValidation(s.ValueValidation.Not)
+		}
+	}
+
+	return ret
+}
+
+func (m *Visitor) visitNestedValueValidation(vv *NestedValueValidation) bool {
+	ret := false
+	if m.NestedValueValidation != nil {
+		ret = m.NestedValueValidation(vv)
+	}
+
+	if vv.Items != nil {
+		m.visitNestedValueValidation(vv.Items)
+	}
+	for k, v := range vv.Properties {
+		if changed := m.visitNestedValueValidation(&v); changed {
+			ret = true
+			vv.Properties[k] = v
+		}
+	}
+	if vv.ForbiddenGenerics.AdditionalProperties != nil && vv.ForbiddenGenerics.AdditionalProperties.Structural != nil {
+		m.visitStructural(vv.ForbiddenGenerics.AdditionalProperties.Structural)
+	}
+	for i := range vv.ValueValidation.AllOf {
+		m.visitNestedValueValidation(&vv.ValueValidation.AllOf[i])
+	}
+	for i := range vv.ValueValidation.AnyOf {
+		m.visitNestedValueValidation(&vv.ValueValidation.AnyOf[i])
+	}
+	for i := range vv.ValueValidation.OneOf {
+		m.visitNestedValueValidation(&vv.ValueValidation.OneOf[i])
+	}
+	if vv.ValueValidation.Not != nil {
+		m.visitNestedValueValidation(vv.ValueValidation.Not)
+	}
+
+	return ret
+}

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

@@ -29,6 +29,7 @@ func NewSchemaValidator(customResourceValidation *apiextensions.CustomResourceVa
 	// Convert CRD schema to openapi schema
 	openapiSchema := &spec.Schema{}
 	if customResourceValidation != nil {
+		// WARNING: do not replace this with Structural.ToGoOpenAPI until it supports nullable.
 		if err := ConvertJSONSchemaProps(customResourceValidation.OpenAPIV3Schema, openapiSchema); err != nil {
 			return nil, nil, err
 		}