From 662c53971c67425583b5e14363b0f5511015d240 Mon Sep 17 00:00:00 2001 From: Haowei Cai Date: Thu, 28 Feb 2019 18:43:29 -0800 Subject: [PATCH 1/4] openapi v3 to v2 conversion k/k#71137: - apiextensions: prune {any,one}Of + Not recursively on OpenAPI v2 conversion roycaihw/kubernetes#6: - apiextensions: filter CRD schema to not break (too) strict kube-openapi - model validator; - SQUASH: fix root level filtering to not drop properties; - SQUASH: fix incomplete test specs which degenerate during kubectl <= 1.13 filtering Co-authored-by: Dr. Stefan Schimanski --- .../pkg/apiserver/validation/validation.go | 83 +- .../pkg/controller/openapi/conversion.go | 118 +++ .../pkg/controller/openapi/conversion_test.go | 841 ++++++++++++++++++ 3 files changed, 1010 insertions(+), 32 deletions(-) create mode 100644 staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/conversion.go create mode 100644 staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/conversion_test.go diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation.go index 558ddb1fcd..99ee921165 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation.go @@ -50,8 +50,17 @@ func ValidateCustomResource(customResource interface{}, validator *validate.Sche return nil } -// ConvertJSONSchemaProps converts the schema from apiextensions.JSONSchemaPropos to go-openapi/spec.Schema +// ConvertJSONSchemaProps converts the schema from apiextensions.JSONSchemaPropos to go-openapi/spec.Schema. func ConvertJSONSchemaProps(in *apiextensions.JSONSchemaProps, out *spec.Schema) error { + return ConvertJSONSchemaPropsWithPostProcess(in, out, nil) +} + +// PostProcessFunc post-processes one node of a spec.Schema. +type PostProcessFunc func(*spec.Schema) error + +// ConvertJSONSchemaPropsWithPostProcess converts the schema from apiextensions.JSONSchemaPropos to go-openapi/spec.Schema +// and run a post process step on each JSONSchemaProps node. +func ConvertJSONSchemaPropsWithPostProcess(in *apiextensions.JSONSchemaProps, out *spec.Schema, postProcess PostProcessFunc) error { if in == nil { return nil } @@ -86,41 +95,43 @@ func ConvertJSONSchemaProps(in *apiextensions.JSONSchemaProps, out *spec.Schema) out.Example = *(in.Example) } - out.Enum = make([]interface{}, len(in.Enum)) - for k, v := range in.Enum { - out.Enum[k] = v + if in.Enum != nil { + out.Enum = make([]interface{}, len(in.Enum)) + for k, v := range in.Enum { + out.Enum[k] = v + } } - if err := convertSliceOfJSONSchemaProps(&in.AllOf, &out.AllOf); err != nil { + if err := convertSliceOfJSONSchemaProps(&in.AllOf, &out.AllOf, postProcess); err != nil { return err } - if err := convertSliceOfJSONSchemaProps(&in.OneOf, &out.OneOf); err != nil { + if err := convertSliceOfJSONSchemaProps(&in.OneOf, &out.OneOf, postProcess); err != nil { return err } - if err := convertSliceOfJSONSchemaProps(&in.AnyOf, &out.AnyOf); err != nil { + if err := convertSliceOfJSONSchemaProps(&in.AnyOf, &out.AnyOf, postProcess); err != nil { return err } if in.Not != nil { in, out := &in.Not, &out.Not *out = new(spec.Schema) - if err := ConvertJSONSchemaProps(*in, *out); err != nil { + if err := ConvertJSONSchemaPropsWithPostProcess(*in, *out, postProcess); err != nil { return err } } var err error - out.Properties, err = convertMapOfJSONSchemaProps(in.Properties) + out.Properties, err = convertMapOfJSONSchemaProps(in.Properties, postProcess) if err != nil { return err } - out.PatternProperties, err = convertMapOfJSONSchemaProps(in.PatternProperties) + out.PatternProperties, err = convertMapOfJSONSchemaProps(in.PatternProperties, postProcess) if err != nil { return err } - out.Definitions, err = convertMapOfJSONSchemaProps(in.Definitions) + out.Definitions, err = convertMapOfJSONSchemaProps(in.Definitions, postProcess) if err != nil { return err } @@ -135,7 +146,7 @@ func ConvertJSONSchemaProps(in *apiextensions.JSONSchemaProps, out *spec.Schema) if in.AdditionalProperties != nil { in, out := &in.AdditionalProperties, &out.AdditionalProperties *out = new(spec.SchemaOrBool) - if err := convertJSONSchemaPropsorBool(*in, *out); err != nil { + if err := convertJSONSchemaPropsorBool(*in, *out, postProcess); err != nil { return err } } @@ -143,7 +154,7 @@ func ConvertJSONSchemaProps(in *apiextensions.JSONSchemaProps, out *spec.Schema) if in.AdditionalItems != nil { in, out := &in.AdditionalItems, &out.AdditionalItems *out = new(spec.SchemaOrBool) - if err := convertJSONSchemaPropsorBool(*in, *out); err != nil { + if err := convertJSONSchemaPropsorBool(*in, *out, postProcess); err != nil { return err } } @@ -151,7 +162,7 @@ func ConvertJSONSchemaProps(in *apiextensions.JSONSchemaProps, out *spec.Schema) if in.Items != nil { in, out := &in.Items, &out.Items *out = new(spec.SchemaOrArray) - if err := convertJSONSchemaPropsOrArray(*in, *out); err != nil { + if err := convertJSONSchemaPropsOrArray(*in, *out, postProcess); err != nil { return err } } @@ -161,7 +172,7 @@ func ConvertJSONSchemaProps(in *apiextensions.JSONSchemaProps, out *spec.Schema) *out = make(spec.Dependencies, len(*in)) for key, val := range *in { newVal := new(spec.SchemaOrStringArray) - if err := convertJSONSchemaPropsOrStringArray(&val, newVal); err != nil { + if err := convertJSONSchemaPropsOrStringArray(&val, newVal, postProcess); err != nil { return err } (*out)[key] = *newVal @@ -174,14 +185,20 @@ func ConvertJSONSchemaProps(in *apiextensions.JSONSchemaProps, out *spec.Schema) out.ExternalDocs.URL = in.ExternalDocs.URL } + if postProcess != nil { + if err := postProcess(out); err != nil { + return err + } + } + return nil } -func convertSliceOfJSONSchemaProps(in *[]apiextensions.JSONSchemaProps, out *[]spec.Schema) error { +func convertSliceOfJSONSchemaProps(in *[]apiextensions.JSONSchemaProps, out *[]spec.Schema, postProcess PostProcessFunc) error { if in != nil { for _, jsonSchemaProps := range *in { schema := spec.Schema{} - if err := ConvertJSONSchemaProps(&jsonSchemaProps, &schema); err != nil { + if err := ConvertJSONSchemaPropsWithPostProcess(&jsonSchemaProps, &schema, postProcess); err != nil { return err } *out = append(*out, schema) @@ -190,25 +207,27 @@ func convertSliceOfJSONSchemaProps(in *[]apiextensions.JSONSchemaProps, out *[]s return nil } -func convertMapOfJSONSchemaProps(in map[string]apiextensions.JSONSchemaProps) (map[string]spec.Schema, error) { +func convertMapOfJSONSchemaProps(in map[string]apiextensions.JSONSchemaProps, postProcess PostProcessFunc) (map[string]spec.Schema, error) { + if in == nil { + return nil, nil + } + out := make(map[string]spec.Schema) - if len(in) != 0 { - for k, jsonSchemaProps := range in { - schema := spec.Schema{} - if err := ConvertJSONSchemaProps(&jsonSchemaProps, &schema); err != nil { - return nil, err - } - out[k] = schema + for k, jsonSchemaProps := range in { + schema := spec.Schema{} + if err := ConvertJSONSchemaPropsWithPostProcess(&jsonSchemaProps, &schema, postProcess); err != nil { + return nil, err } + out[k] = schema } return out, nil } -func convertJSONSchemaPropsOrArray(in *apiextensions.JSONSchemaPropsOrArray, out *spec.SchemaOrArray) error { +func convertJSONSchemaPropsOrArray(in *apiextensions.JSONSchemaPropsOrArray, out *spec.SchemaOrArray, postProcess PostProcessFunc) error { if in.Schema != nil { in, out := &in.Schema, &out.Schema *out = new(spec.Schema) - if err := ConvertJSONSchemaProps(*in, *out); err != nil { + if err := ConvertJSONSchemaPropsWithPostProcess(*in, *out, postProcess); err != nil { return err } } @@ -216,7 +235,7 @@ func convertJSONSchemaPropsOrArray(in *apiextensions.JSONSchemaPropsOrArray, out in, out := &in.JSONSchemas, &out.Schemas *out = make([]spec.Schema, len(*in)) for i := range *in { - if err := ConvertJSONSchemaProps(&(*in)[i], &(*out)[i]); err != nil { + if err := ConvertJSONSchemaPropsWithPostProcess(&(*in)[i], &(*out)[i], postProcess); err != nil { return err } } @@ -224,24 +243,24 @@ func convertJSONSchemaPropsOrArray(in *apiextensions.JSONSchemaPropsOrArray, out return nil } -func convertJSONSchemaPropsorBool(in *apiextensions.JSONSchemaPropsOrBool, out *spec.SchemaOrBool) error { +func convertJSONSchemaPropsorBool(in *apiextensions.JSONSchemaPropsOrBool, out *spec.SchemaOrBool, postProcess PostProcessFunc) error { out.Allows = in.Allows if in.Schema != nil { in, out := &in.Schema, &out.Schema *out = new(spec.Schema) - if err := ConvertJSONSchemaProps(*in, *out); err != nil { + if err := ConvertJSONSchemaPropsWithPostProcess(*in, *out, postProcess); err != nil { return err } } return nil } -func convertJSONSchemaPropsOrStringArray(in *apiextensions.JSONSchemaPropsOrStringArray, out *spec.SchemaOrStringArray) error { +func convertJSONSchemaPropsOrStringArray(in *apiextensions.JSONSchemaPropsOrStringArray, out *spec.SchemaOrStringArray, postProcess PostProcessFunc) error { out.Property = in.Property if in.Schema != nil { in, out := &in.Schema, &out.Schema *out = new(spec.Schema) - if err := ConvertJSONSchemaProps(*in, *out); err != nil { + if err := ConvertJSONSchemaPropsWithPostProcess(*in, *out, postProcess); err != nil { return err } } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/conversion.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/conversion.go new file mode 100644 index 0000000000..173060f7b1 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/conversion.go @@ -0,0 +1,118 @@ +/* +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 openapi + +import ( + "strings" + + "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 +// (*apiextensions.JSONSchemaProps) to an OpenAPI v2 schema (*spec.Schema). +func ConvertJSONSchemaPropsToOpenAPIv2Schema(in *apiextensions.JSONSchemaProps) (*spec.Schema, error) { + if in == nil { + return nil, nil + } + + // dirty hack to temporarily set the type at the root. See continuation at the func bottom. + // 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) + validation.ConvertJSONSchemaPropsWithPostProcess(in, out, func(p *spec.Schema) error { + p.OneOf = nil + // TODO(roycaihw): preserve cases where we only have one subtree in AnyOf, same for OneOf + p.AnyOf = nil + p.Not = nil + + // 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 p.Ref.String() != "" { + // https://github.com/kubernetes/kube-openapi/pull/143/files#diff-62afddb578e9db18fb32ffb6b7802d92R95 + p.Properties = nil + + // https://github.com/kubernetes/kube-openapi/pull/143/files#diff-62afddb578e9db18fb32ffb6b7802d92R99 + p.Type = nil + + // https://github.com/kubernetes/kube-openapi/pull/143/files#diff-62afddb578e9db18fb32ffb6b7802d92R104 + if !strings.HasPrefix(p.Ref.String(), "#/definitions/") { + p.Ref = spec.Ref{} + } + } + + if 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 + } else if 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 + } + } + } else { + // https://github.com/kubernetes/kube-openapi/pull/143/files#diff-62afddb578e9db18fb32ffb6b7802d92R248 + p.Properties = nil + } + + // normalize items + if p.Items != nil && len(p.Items.Schemas) == 1 { + p.Items = &spec.SchemaOrArray{Schema: &p.Items.Schemas[0]} + } + + // general fixups not supported by gnostic + 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 +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/conversion_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/conversion_test.go new file mode 100644 index 0000000000..571bd51bd6 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/conversion_test.go @@ -0,0 +1,841 @@ +/* +Copyright 2018 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 openapi + +import ( + "encoding/json" + "fmt" + "math" + "math/rand" + "reflect" + "testing" + "time" + + "github.com/go-openapi/spec" + 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" + "k8s.io/kube-openapi/pkg/util/proto" +) + +func Test_ConvertJSONSchemaPropsToOpenAPIv2Schema(t *testing.T) { + var spec = []byte(`description: Foo CRD for Testing +type: object +properties: + spec: + description: Specification of Foo + type: object + properties: + bars: + description: List of Bars and their specs. + type: array + items: + type: object + required: + - name + properties: + name: + description: Name of Bar. + type: string + age: + description: Age of Bar. + type: string + bazs: + description: List of Bazs. + items: + type: string + type: array + status: + description: Status of Foo + type: object + properties: + bars: + description: List of Bars and their statuses. + type: array + items: + type: object + properties: + name: + description: Name of Bar. + type: string + available: + description: Whether the Bar is installed. + type: boolean + quxType: + description: Indicates to external qux type. + pattern: in-tree|out-of-tree + type: string`) + + specV1beta1 := apiextensionsv1beta1.JSONSchemaProps{} + if err := yaml.Unmarshal(spec, &specV1beta1); err != nil { + t.Fatal(err) + } + + specInternal := apiextensions.JSONSchemaProps{} + if err := apiextensionsv1beta1.Convert_v1beta1_JSONSchemaProps_To_apiextensions_JSONSchemaProps(&specV1beta1, &specInternal, nil); err != nil { + t.Fatal(err) + } + + schema, err := ConvertJSONSchemaPropsToOpenAPIv2Schema(&specInternal) + if err != nil { + t.Fatal(err) + } + + if _, found := schema.Properties["spec"]; !found { + t.Errorf("spec not found") + } + if _, found := schema.Properties["status"]; !found { + t.Errorf("status not found") + } +} + +func Test_ConvertJSONSchemaPropsToOpenAPIv2SchemaFuzzing(t *testing.T) { + testStr := "test" + testStr2 := "test2" + testFloat64 := float64(6.4) + testInt64 := int64(64) + testApiextensionsJSON := apiextensions.JSON(testStr) + + tests := []struct { + name string + in *apiextensions.JSONSchemaProps + expected *spec.Schema + }{ + { + name: "id", + in: &apiextensions.JSONSchemaProps{ + ID: testStr, + }, + expected: new(spec.Schema), + // not supported by gnostic + // expected: new(spec.Schema). + // WithID(testStr), + }, + { + name: "$schema", + in: &apiextensions.JSONSchemaProps{ + Schema: "test", + }, + expected: new(spec.Schema), + // not supported by gnostic + // expected: &spec.Schema{ + // SchemaProps: spec.SchemaProps{ + // Schema: "test", + // }, + // }, + }, + { + name: "$ref", + in: &apiextensions.JSONSchemaProps{ + Ref: &testStr, + }, + expected: new(spec.Schema), + // https://github.com/kubernetes/kube-openapi/pull/143/files#diff-62afddb578e9db18fb32ffb6b7802d92R104 + // expected: spec.RefSchema(testStr), + }, + { + name: "description", + in: &apiextensions.JSONSchemaProps{ + Description: testStr, + }, + expected: new(spec.Schema). + WithDescription(testStr), + }, + { + name: "type and format", + in: &apiextensions.JSONSchemaProps{ + Type: testStr, + Format: testStr2, + }, + expected: new(spec.Schema). + Typed(testStr, testStr2), + }, + { + name: "title", + in: &apiextensions.JSONSchemaProps{ + Title: testStr, + }, + expected: new(spec.Schema). + WithTitle(testStr), + }, + { + name: "default", + in: &apiextensions.JSONSchemaProps{ + Default: &testApiextensionsJSON, + }, + expected: new(spec.Schema). + WithDefault(testStr), + }, + { + name: "maximum and exclusiveMaximum", + in: &apiextensions.JSONSchemaProps{ + Maximum: &testFloat64, + ExclusiveMaximum: true, + }, + expected: new(spec.Schema). + WithMaximum(testFloat64, true), + }, + { + name: "minimum and exclusiveMinimum", + in: &apiextensions.JSONSchemaProps{ + Minimum: &testFloat64, + ExclusiveMinimum: true, + }, + expected: new(spec.Schema). + WithMinimum(testFloat64, true), + }, + { + name: "maxLength", + in: &apiextensions.JSONSchemaProps{ + MaxLength: &testInt64, + }, + expected: new(spec.Schema). + WithMaxLength(testInt64), + }, + { + name: "minLength", + in: &apiextensions.JSONSchemaProps{ + MinLength: &testInt64, + }, + expected: new(spec.Schema). + WithMinLength(testInt64), + }, + { + name: "pattern", + in: &apiextensions.JSONSchemaProps{ + Pattern: testStr, + }, + expected: new(spec.Schema). + WithPattern(testStr), + }, + { + name: "maxItems", + in: &apiextensions.JSONSchemaProps{ + MaxItems: &testInt64, + }, + expected: new(spec.Schema). + WithMaxItems(testInt64), + }, + { + name: "minItems", + in: &apiextensions.JSONSchemaProps{ + MinItems: &testInt64, + }, + expected: new(spec.Schema). + WithMinItems(testInt64), + }, + { + name: "uniqueItems", + in: &apiextensions.JSONSchemaProps{ + UniqueItems: true, + }, + expected: new(spec.Schema). + UniqueValues(), + }, + { + name: "multipleOf", + in: &apiextensions.JSONSchemaProps{ + MultipleOf: &testFloat64, + }, + expected: new(spec.Schema). + WithMultipleOf(testFloat64), + }, + { + name: "enum", + in: &apiextensions.JSONSchemaProps{ + Enum: []apiextensions.JSON{apiextensions.JSON(testStr), apiextensions.JSON(testStr2)}, + }, + expected: new(spec.Schema). + WithEnum(testStr, testStr2), + }, + { + name: "maxProperties", + in: &apiextensions.JSONSchemaProps{ + MaxProperties: &testInt64, + }, + expected: new(spec.Schema). + WithMaxProperties(testInt64), + }, + { + name: "minProperties", + in: &apiextensions.JSONSchemaProps{ + MinProperties: &testInt64, + }, + expected: new(spec.Schema). + WithMinProperties(testInt64), + }, + { + name: "required", + in: &apiextensions.JSONSchemaProps{ + Required: []string{testStr, testStr2}, + }, + expected: new(spec.Schema). + WithRequired(testStr, testStr2), + }, + { + name: "items single props", + in: &apiextensions.JSONSchemaProps{ + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "boolean", + }, + }, + }, + expected: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Items: &spec.SchemaOrArray{ + Schema: spec.BooleanProperty(), + }, + }, + }, + }, + { + name: "items array props", + in: &apiextensions.JSONSchemaProps{ + Items: &apiextensions.JSONSchemaPropsOrArray{ + JSONSchemas: []apiextensions.JSONSchemaProps{ + {Type: "boolean"}, + {Type: "string"}, + }, + }, + }, + expected: new(spec.Schema), + // 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", + in: &apiextensions.JSONSchemaProps{ + AllOf: []apiextensions.JSONSchemaProps{ + {Type: "boolean"}, + {Type: "string"}, + }, + }, + expected: new(spec.Schema). + WithAllOf(*spec.BooleanProperty(), *spec.StringProperty()), + }, + { + name: "oneOf", + in: &apiextensions.JSONSchemaProps{ + OneOf: []apiextensions.JSONSchemaProps{ + {Type: "boolean"}, + {Type: "string"}, + }, + }, + expected: new(spec.Schema), + // not supported by openapi v2 + // expected: &spec.Schema{ + // SchemaProps: spec.SchemaProps{ + // OneOf: []spec.Schema{ + // *spec.BooleanProperty(), + // *spec.StringProperty(), + // }, + // }, + // }, + }, + { + name: "anyOf", + in: &apiextensions.JSONSchemaProps{ + AnyOf: []apiextensions.JSONSchemaProps{ + {Type: "boolean"}, + {Type: "string"}, + }, + }, + expected: new(spec.Schema), + // not supported by openapi v2 + // expected: &spec.Schema{ + // SchemaProps: spec.SchemaProps{ + // AnyOf: []spec.Schema{ + // *spec.BooleanProperty(), + // *spec.StringProperty(), + // }, + // }, + // }, + }, + { + name: "not", + in: &apiextensions.JSONSchemaProps{ + Not: &apiextensions.JSONSchemaProps{ + Type: "boolean", + }, + }, + expected: new(spec.Schema), + // not supported by openapi v2 + // expected: &spec.Schema{ + // SchemaProps: spec.SchemaProps{ + // Not: spec.BooleanProperty(), + // }, + // }, + }, + { + name: "nested logic", + in: &apiextensions.JSONSchemaProps{ + AllOf: []apiextensions.JSONSchemaProps{ + { + Not: &apiextensions.JSONSchemaProps{ + Type: "boolean", + }, + }, + { + AnyOf: []apiextensions.JSONSchemaProps{ + {Type: "boolean"}, + {Type: "string"}, + }, + }, + { + OneOf: []apiextensions.JSONSchemaProps{ + {Type: "boolean"}, + {Type: "string"}, + }, + }, + {Type: "string"}, + }, + AnyOf: []apiextensions.JSONSchemaProps{ + { + Not: &apiextensions.JSONSchemaProps{ + Type: "boolean", + }, + }, + { + AnyOf: []apiextensions.JSONSchemaProps{ + {Type: "boolean"}, + {Type: "string"}, + }, + }, + { + OneOf: []apiextensions.JSONSchemaProps{ + {Type: "boolean"}, + {Type: "string"}, + }, + }, + {Type: "string"}, + }, + OneOf: []apiextensions.JSONSchemaProps{ + { + Not: &apiextensions.JSONSchemaProps{ + Type: "boolean", + }, + }, + { + AnyOf: []apiextensions.JSONSchemaProps{ + {Type: "boolean"}, + {Type: "string"}, + }, + }, + { + OneOf: []apiextensions.JSONSchemaProps{ + {Type: "boolean"}, + {Type: "string"}, + }, + }, + {Type: "string"}, + }, + Not: &apiextensions.JSONSchemaProps{ + Not: &apiextensions.JSONSchemaProps{ + Type: "boolean", + }, + AnyOf: []apiextensions.JSONSchemaProps{ + {Type: "boolean"}, + {Type: "string"}, + }, + OneOf: []apiextensions.JSONSchemaProps{ + {Type: "boolean"}, + {Type: "string"}, + }, + }, + }, + expected: new(spec.Schema). + WithAllOf(spec.Schema{}, spec.Schema{}, spec.Schema{}, *spec.StringProperty()), + }, + { + name: "properties", + in: &apiextensions.JSONSchemaProps{ + Properties: map[string]apiextensions.JSONSchemaProps{ + testStr: {Type: "boolean"}, + }, + }, + expected: new(spec.Schema). + SetProperty(testStr, *spec.BooleanProperty()), + }, + { + name: "additionalProperties", + in: &apiextensions.JSONSchemaProps{ + AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{ + Allows: true, + Schema: &apiextensions.JSONSchemaProps{Type: "boolean"}, + }, + }, + expected: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + AdditionalProperties: &spec.SchemaOrBool{ + Allows: true, + Schema: spec.BooleanProperty(), + }, + }, + }, + }, + { + name: "patternProperties", + in: &apiextensions.JSONSchemaProps{ + PatternProperties: map[string]apiextensions.JSONSchemaProps{ + testStr: {Type: "boolean"}, + }, + }, + expected: new(spec.Schema), + // not supported by gnostic + // expected: &spec.Schema{ + // SchemaProps: spec.SchemaProps{ + // PatternProperties: map[string]spec.Schema{ + // testStr: *spec.BooleanProperty(), + // }, + // }, + // }, + }, + { + name: "dependencies schema", + in: &apiextensions.JSONSchemaProps{ + Dependencies: apiextensions.JSONSchemaDependencies{ + testStr: apiextensions.JSONSchemaPropsOrStringArray{ + Schema: &apiextensions.JSONSchemaProps{Type: "boolean"}, + }, + }, + }, + expected: new(spec.Schema), + // not supported by gnostic + // expected: &spec.Schema{ + // SchemaProps: spec.SchemaProps{ + // Dependencies: spec.Dependencies{ + // testStr: spec.SchemaOrStringArray{ + // Schema: spec.BooleanProperty(), + // }, + // }, + // }, + // }, + }, + { + name: "dependencies string array", + in: &apiextensions.JSONSchemaProps{ + Dependencies: apiextensions.JSONSchemaDependencies{ + testStr: apiextensions.JSONSchemaPropsOrStringArray{ + Property: []string{testStr2}, + }, + }, + }, + expected: new(spec.Schema), + // not supported by gnostic + // expected: &spec.Schema{ + // SchemaProps: spec.SchemaProps{ + // Dependencies: spec.Dependencies{ + // testStr: spec.SchemaOrStringArray{ + // Property: []string{testStr2}, + // }, + // }, + // }, + // }, + }, + { + name: "additionalItems", + in: &apiextensions.JSONSchemaProps{ + AdditionalItems: &apiextensions.JSONSchemaPropsOrBool{ + Allows: true, + Schema: &apiextensions.JSONSchemaProps{Type: "boolean"}, + }, + }, + expected: new(spec.Schema), + // not supported by gnostic + // expected: &spec.Schema{ + // SchemaProps: spec.SchemaProps{ + // AdditionalItems: &spec.SchemaOrBool{ + // Allows: true, + // Schema: spec.BooleanProperty(), + // }, + // }, + // }, + }, + { + name: "definitions", + in: &apiextensions.JSONSchemaProps{ + Definitions: apiextensions.JSONSchemaDefinitions{ + testStr: apiextensions.JSONSchemaProps{Type: "boolean"}, + }, + }, + expected: new(spec.Schema), + // not supported by gnostic + // expected: &spec.Schema{ + // SchemaProps: spec.SchemaProps{ + // Definitions: spec.Definitions{ + // testStr: *spec.BooleanProperty(), + // }, + // }, + // }, + }, + { + name: "externalDocs", + in: &apiextensions.JSONSchemaProps{ + ExternalDocs: &apiextensions.ExternalDocumentation{ + Description: testStr, + URL: testStr2, + }, + }, + expected: new(spec.Schema). + WithExternalDocs(testStr, testStr2), + }, + { + name: "example", + in: &apiextensions.JSONSchemaProps{ + Example: &testApiextensionsJSON, + }, + expected: new(spec.Schema). + WithExample(testStr), + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + out, err := ConvertJSONSchemaPropsToOpenAPIv2Schema(test.in) + if err != nil { + t.Fatalf("unexpected error in converting openapi schema: %v", err) + } + 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)) + } + }) + } +} + +// 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) + + fuzzFuncs(f, func(ref *spec.Ref, c fuzz.Continue, visible bool) { + var url string + if c.RandBool() { + url = fmt.Sprintf("http://%d", c.Intn(100000)) + } else { + url = "#/definitions/test" + } + 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.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) + } + }) + } +} + +// fuzzFuncs is copied from kube-openapi/pkg/aggregator. It fuzzes go-openapi/spec schemata. +func fuzzFuncs(f *fuzz.Fuzzer, refFunc func(ref *spec.Ref, c fuzz.Continue, visible bool)) { + invisible := 0 // == 0 means visible, > 0 means invisible + depth := 0 + maxDepth := 3 + nilChance := func(depth int) float64 { + return math.Pow(0.9, math.Max(0.0, float64(maxDepth-depth))) + } + updateFuzzer := func(depth int) { + f.NilChance(nilChance(depth)) + f.NumElements(0, max(0, maxDepth-depth)) + } + updateFuzzer(depth) + enter := func(o interface{}, recursive bool, c fuzz.Continue) { + if recursive { + depth++ + updateFuzzer(depth) + } + + invisible++ + c.FuzzNoCustom(o) + invisible-- + } + leave := func(recursive bool) { + if recursive { + depth-- + updateFuzzer(depth) + } + } + f.Funcs( + func(ref *spec.Ref, c fuzz.Continue) { + refFunc(ref, c, invisible == 0) + }, + func(sa *spec.SchemaOrStringArray, c fuzz.Continue) { + *sa = spec.SchemaOrStringArray{} + if c.RandBool() { + c.Fuzz(&sa.Schema) + } else { + c.Fuzz(&sa.Property) + } + if sa.Schema == nil && len(sa.Property) == 0 { + *sa = spec.SchemaOrStringArray{Schema: &spec.Schema{}} + } + }, + func(url *spec.SchemaURL, c fuzz.Continue) { + *url = spec.SchemaURL("http://url") + }, + func(s *spec.Dependencies, c fuzz.Continue) { + enter(s, false, c) + defer leave(false) + + // and nothing with invisible==false + }, + func(p *spec.SimpleSchema, c fuzz.Continue) { + // gofuzz is broken and calls this even for *SimpleSchema fields, ignoring NilChance, leading to infinite recursion + if c.Float64() > nilChance(depth) { + return + } + + enter(p, true, c) + defer leave(true) + + c.FuzzNoCustom(p) + + // reset JSON fields to some correct JSON + if p.Default != nil { + p.Default = "42" + } + if p.Example != nil { + p.Example = "42" + } + }, + func(s *spec.SchemaProps, c fuzz.Continue) { + // gofuzz is broken and calls this even for *SchemaProps fields, ignoring NilChance, leading to infinite recursion + if c.Float64() > nilChance(depth) { + return + } + + enter(s, true, c) + defer leave(true) + + c.FuzzNoCustom(s) + + // we don't support multi-type schema props yet in apiextensions/v1beta1 + if len(s.Type) > 1 { + s.Type = s.Type[:1] + + s := apiextensionsv1beta1.JSONSchemaProps{} + 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)) + } + } + + // reset JSON fields to some correct JSON + if s.Default != nil { + s.Default = "42" + } + for i := range s.Enum { + s.Enum[i] = "42" + } + }, + func(i *interface{}, c fuzz.Continue) { + // do nothing for examples and defaults. These are free form JSON fields. + }, + ) +} + +func max(i, j int) int { + if i > j { + return i + } + return j +} From 19ccaf54085fc561e26f67934468b0d7729f7e70 Mon Sep 17 00:00:00 2001 From: Haowei Cai Date: Thu, 28 Feb 2019 19:33:11 -0800 Subject: [PATCH 2/4] allow importing kube-openapi --- staging/publishing/import-restrictions.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/staging/publishing/import-restrictions.yaml b/staging/publishing/import-restrictions.yaml index 05eef5ca12..e6f511ddd5 100644 --- a/staging/publishing/import-restrictions.yaml +++ b/staging/publishing/import-restrictions.yaml @@ -137,6 +137,7 @@ - k8s.io/client-go - k8s.io/component-base - k8s.io/klog + - k8s.io/kube-openapi - baseImportPath: "./vendor/k8s.io/kube-openapi/" allowedImports: From c2dd76263a84fe46414c8eea28b392bef6f79234 Mon Sep 17 00:00:00 2001 From: Haowei Cai Date: Thu, 28 Feb 2019 19:33:25 -0800 Subject: [PATCH 3/4] generated --- .../src/k8s.io/apiextensions-apiserver/BUILD | 1 + .../pkg/controller/openapi/BUILD | 45 +++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/BUILD diff --git a/staging/src/k8s.io/apiextensions-apiserver/BUILD b/staging/src/k8s.io/apiextensions-apiserver/BUILD index 44528361fe..911ac1469a 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/BUILD +++ b/staging/src/k8s.io/apiextensions-apiserver/BUILD @@ -47,6 +47,7 @@ filegroup( "//staging/src/k8s.io/apiextensions-apiserver/pkg/cmd/server:all-srcs", "//staging/src/k8s.io/apiextensions-apiserver/pkg/controller/establish:all-srcs", "//staging/src/k8s.io/apiextensions-apiserver/pkg/controller/finalizer:all-srcs", + "//staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi:all-srcs", "//staging/src/k8s.io/apiextensions-apiserver/pkg/controller/status:all-srcs", "//staging/src/k8s.io/apiextensions-apiserver/pkg/crdserverscheme:all-srcs", "//staging/src/k8s.io/apiextensions-apiserver/pkg/features:all-srcs", diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/BUILD b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/BUILD new file mode 100644 index 0000000000..557398c071 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/BUILD @@ -0,0 +1,45 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = ["conversion.go"], + importmap = "k8s.io/kubernetes/vendor/k8s.io/apiextensions-apiserver/pkg/controller/openapi", + importpath = "k8s.io/apiextensions-apiserver/pkg/controller/openapi", + visibility = ["//visibility:public"], + deps = [ + "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library", + "//staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation:go_default_library", + "//vendor/github.com/go-openapi/spec:go_default_library", + ], +) + +go_test( + name = "go_default_test", + srcs = ["conversion_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", + "//vendor/github.com/go-openapi/spec: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", + "//vendor/gopkg.in/yaml.v2:go_default_library", + "//vendor/k8s.io/kube-openapi/pkg/util/proto:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) From 43843dad0d87f2efcbf06d66b9b052ef67701dec Mon Sep 17 00:00:00 2001 From: Haowei Cai Date: Thu, 28 Feb 2019 22:11:49 -0800 Subject: [PATCH 4/4] clean up import --- .../pkg/controller/openapi/conversion_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/conversion_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/conversion_test.go index 571bd51bd6..a6c6fd9454 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/conversion_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/conversion_test.go @@ -26,8 +26,8 @@ import ( "time" "github.com/go-openapi/spec" - fuzz "github.com/google/gofuzz" - openapi_v2 "github.com/googleapis/gnostic/OpenAPIv2" + "github.com/google/gofuzz" + "github.com/googleapis/gnostic/OpenAPIv2" "github.com/googleapis/gnostic/compiler" "gopkg.in/yaml.v2" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"