From 69b0699610099f239296c2659a5f936a6573bf59 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Wed, 28 Oct 2020 18:54:26 +0100 Subject: [PATCH] apiextensions: switch validation to kube-openapi --- .../pkg/apiserver/customresource_handler.go | 15 +- .../apiserver/schema/defaulting/validation.go | 7 +- .../pkg/apiserver/schema/kubeopenapi.go | 151 ++++++++++++++++++ .../pkg/apiserver/schema/kubeopenapi_test.go | 102 ++++++++++++ .../pkg/apiserver/validation/formats.go | 2 +- .../pkg/apiserver/validation/formats_test.go | 2 +- .../pkg/apiserver/validation/validation.go | 8 +- .../apiserver/validation/validation_test.go | 5 +- .../pkg/registry/customresource/strategy.go | 3 +- .../pkg/registry/customresource/validator.go | 3 +- test/e2e/apimachinery/crd_publish_openapi.go | 10 +- 11 files changed, 282 insertions(+), 26 deletions(-) create mode 100644 staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/kubeopenapi.go create mode 100644 staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/kubeopenapi_test.go diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go index fe8c3c293d8..2391639a140 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go @@ -26,9 +26,7 @@ import ( "sync/atomic" "time" - "github.com/go-openapi/spec" - "github.com/go-openapi/strfmt" - "github.com/go-openapi/validate" + goopenapispec "github.com/go-openapi/spec" apiextensionshelpers "k8s.io/apiextensions-apiserver/pkg/apihelpers" apiextensionsinternal "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" @@ -86,6 +84,9 @@ import ( "k8s.io/client-go/tools/cache" "k8s.io/klog/v2" "k8s.io/kube-openapi/pkg/util/proto" + "k8s.io/kube-openapi/pkg/validation/spec" + "k8s.io/kube-openapi/pkg/validation/strfmt" + "k8s.io/kube-openapi/pkg/validation/validate" ) // crdHandler serves the `/apis` endpoint. @@ -128,7 +129,7 @@ type crdHandler struct { // staticOpenAPISpec is used as a base for the schema of CR's for the // purpose of managing fields, it is how CR handlers get the structure // of TypeMeta and ObjectMeta - staticOpenAPISpec *spec.Swagger + staticOpenAPISpec *goopenapispec.Swagger // The limit on the request size that would be accepted and decoded in a write request // 0 means no limit. @@ -183,7 +184,7 @@ func NewCustomResourceDefinitionHandler( authorizer authorizer.Authorizer, requestTimeout time.Duration, minRequestTimeout time.Duration, - staticOpenAPISpec *spec.Swagger, + staticOpenAPISpec *goopenapispec.Swagger, maxRequestBodyBytes int64) (*crdHandler, error) { ret := &crdHandler{ versionDiscoveryHandler: versionDiscoveryHandler, @@ -1309,7 +1310,7 @@ func serverStartingError() error { // buildOpenAPIModelsForApply constructs openapi models from any validation schemas specified in the custom resource, // and merges it with the models defined in the static OpenAPI spec. // Returns nil models if the ServerSideApply feature is disabled, or the static spec is nil, or an error is encountered. -func buildOpenAPIModelsForApply(staticOpenAPISpec *spec.Swagger, crd *apiextensionsv1.CustomResourceDefinition) (proto.Models, error) { +func buildOpenAPIModelsForApply(staticOpenAPISpec *goopenapispec.Swagger, crd *apiextensionsv1.CustomResourceDefinition) (proto.Models, error) { if !utilfeature.DefaultFeatureGate.Enabled(features.ServerSideApply) { return nil, nil } @@ -1317,7 +1318,7 @@ func buildOpenAPIModelsForApply(staticOpenAPISpec *spec.Swagger, crd *apiextensi return nil, nil } - specs := []*spec.Swagger{} + specs := []*goopenapispec.Swagger{} for _, v := range crd.Spec.Versions { s, err := builder.BuildSwagger(crd, v.Name, builder.Options{V2: false, StripDefaults: true, StripValueValidation: true, StripNullable: true, AllowNonStructural: true}) if err != nil { diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting/validation.go index 254724dd824..7700a6ced9d 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting/validation.go @@ -20,9 +20,6 @@ import ( "fmt" "reflect" - "github.com/go-openapi/strfmt" - goopenapivalidate "github.com/go-openapi/validate" - structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" schemaobjectmeta "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta" "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning" @@ -30,6 +27,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/kube-openapi/pkg/validation/strfmt" + kubeopenapivalidate "k8s.io/kube-openapi/pkg/validation/validate" ) // ValidateDefaults checks that default values validate and are properly pruned. @@ -67,7 +66,7 @@ func validate(pth *field.Path, s *structuralschema.Structural, rootSchema *struc allErrs := field.ErrorList{} if s.Default.Object != nil { - validator := goopenapivalidate.NewSchemaValidator(s.ToGoOpenAPI(), nil, "", strfmt.Default) + validator := kubeopenapivalidate.NewSchemaValidator(s.ToKubeOpenAPI(), nil, "", strfmt.Default) if insideMeta { obj, _, err := f(runtime.DeepCopyJSONValue(s.Default.Object)) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/kubeopenapi.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/kubeopenapi.go new file mode 100644 index 00000000000..f389ed4e05b --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/kubeopenapi.go @@ -0,0 +1,151 @@ +/* +Copyright 2020 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 ( + "k8s.io/kube-openapi/pkg/validation/spec" +) + +// ToKubeOpenAPI converts a structural schema to go-openapi schema. It is faithful and roundtrippable. +func (s *Structural) ToKubeOpenAPI() *spec.Schema { + if s == nil { + return nil + } + + ret := &spec.Schema{} + + if s.Items != nil { + ret.Items = &spec.SchemaOrArray{Schema: s.Items.ToKubeOpenAPI()} + } + if s.Properties != nil { + ret.Properties = make(map[string]spec.Schema, len(s.Properties)) + for k, v := range s.Properties { + ret.Properties[k] = *v.ToKubeOpenAPI() + } + } + s.Generic.toKubeOpenAPI(ret) + s.Extensions.toKubeOpenAPI(ret) + s.ValueValidation.toKubeOpenAPI(ret) + + return ret +} + +func (g *Generic) toKubeOpenAPI(ret *spec.Schema) { + if g == nil { + return + } + + if len(g.Type) != 0 { + ret.Type = spec.StringOrArray{g.Type} + } + ret.Nullable = g.Nullable + if g.AdditionalProperties != nil { + ret.AdditionalProperties = &spec.SchemaOrBool{ + Allows: g.AdditionalProperties.Bool, + Schema: g.AdditionalProperties.Structural.ToKubeOpenAPI(), + } + } + ret.Description = g.Description + ret.Title = g.Title + ret.Default = g.Default.Object +} + +func (x *Extensions) toKubeOpenAPI(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) + } + if len(x.XListMapKeys) > 0 { + ret.VendorExtensible.AddExtension("x-kubernetes-list-map-keys", x.XListMapKeys) + } + if x.XListType != nil { + ret.VendorExtensible.AddExtension("x-kubernetes-list-type", *x.XListType) + } + if x.XMapType != nil { + ret.VendorExtensible.AddExtension("x-kubernetes-map-type", *x.XMapType) + } +} + +func (v *ValueValidation) toKubeOpenAPI(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].toKubeOpenAPI()) + } + for i := range v.AnyOf { + ret.AnyOf = append(ret.AnyOf, *v.AnyOf[i].toKubeOpenAPI()) + } + for i := range v.OneOf { + ret.OneOf = append(ret.OneOf, *v.OneOf[i].toKubeOpenAPI()) + } + ret.Not = v.Not.toKubeOpenAPI() +} + +func (vv *NestedValueValidation) toKubeOpenAPI() *spec.Schema { + if vv == nil { + return nil + } + + ret := &spec.Schema{} + + vv.ValueValidation.toKubeOpenAPI(ret) + if vv.Items != nil { + ret.Items = &spec.SchemaOrArray{Schema: vv.Items.toKubeOpenAPI()} + } + if vv.Properties != nil { + ret.Properties = make(map[string]spec.Schema, len(vv.Properties)) + for k, v := range vv.Properties { + ret.Properties[k] = *v.toKubeOpenAPI() + } + } + vv.ForbiddenGenerics.toKubeOpenAPI(ret) // normally empty. Exception: int-or-string + vv.ForbiddenExtensions.toKubeOpenAPI(ret) // shouldn't do anything + + return ret +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/kubeopenapi_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/kubeopenapi_test.go new file mode 100644 index 00000000000..84266bdc314 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/kubeopenapi_test.go @@ -0,0 +1,102 @@ +/* +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 TestStructuralKubeOpenAPIRoundtrip(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(7) { + case 0: + s.Object = float64(42.2) + case 1: + s.Object = map[string]interface{}{"foo": "bar"} + case 2: + s.Object = "" + case 3: + s.Object = []interface{}{} + case 4: + s.Object = map[string]interface{}{} + case 5: + s.Object = nil + case 6: + s.Object = int64(42) + } + }, + ) + 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) + + kubeOpenAPI := orig.ToKubeOpenAPI() + bs, err := json.Marshal(kubeOpenAPI) + if err != nil { + t.Fatal(err) + } + v1beta1Schema := &apiextensionsv1beta1.JSONSchemaProps{} + err = json.Unmarshal(bs, 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.ObjectGoPrintDiff(orig, s)) + } + } +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/formats.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/formats.go index f0211576f71..f67c1c58e31 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/formats.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/formats.go @@ -19,8 +19,8 @@ package validation import ( "strings" - "github.com/go-openapi/spec" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/kube-openapi/pkg/validation/spec" ) var supportedFormats = sets.NewString( diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/formats_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/formats_test.go index b97410efb82..17c13ac6ada 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/formats_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/formats_test.go @@ -19,7 +19,7 @@ package validation import ( "testing" - "github.com/go-openapi/strfmt" + "k8s.io/kube-openapi/pkg/validation/strfmt" ) func TestRegistryFormats(t *testing.T) { 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 fd661f92640..3a29d9f7573 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 @@ -20,10 +20,10 @@ import ( "encoding/json" "strings" - openapierrors "github.com/go-openapi/errors" - "github.com/go-openapi/spec" - "github.com/go-openapi/strfmt" - "github.com/go-openapi/validate" + openapierrors "k8s.io/kube-openapi/pkg/validation/errors" + "k8s.io/kube-openapi/pkg/validation/spec" + "k8s.io/kube-openapi/pkg/validation/strfmt" + "k8s.io/kube-openapi/pkg/validation/validate" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" "k8s.io/apimachinery/pkg/util/validation/field" diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go index 2250c32c4e8..d8b859993c6 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go @@ -20,8 +20,6 @@ import ( "math/rand" "testing" - "github.com/go-openapi/spec" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" apiextensionsfuzzer "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -31,6 +29,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apimachinery/pkg/util/json" "k8s.io/apimachinery/pkg/util/sets" + kubeopenapispec "k8s.io/kube-openapi/pkg/validation/spec" ) // TestRoundTrip checks the conversion to go-openapi types. @@ -58,7 +57,7 @@ func TestRoundTrip(t *testing.T) { f.Fuzz(internal) // internal -> go-openapi - openAPITypes := &spec.Schema{} + openAPITypes := &kubeopenapispec.Schema{} if err := ConvertJSONSchemaProps(internal, openAPITypes); err != nil { t.Fatal(err) } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go index 551176ec60f..a65595dc141 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go @@ -19,8 +19,6 @@ package customresource import ( "context" - "github.com/go-openapi/validate" - apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -32,6 +30,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" apiserverstorage "k8s.io/apiserver/pkg/storage" "k8s.io/apiserver/pkg/storage/names" + "k8s.io/kube-openapi/pkg/validation/validate" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go index 6d89e1bbe24..c3e31021d9d 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go @@ -22,14 +22,13 @@ import ( "math" "strings" - "github.com/go-openapi/validate" - "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/api/validation" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/kube-openapi/pkg/validation/validate" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" apiservervalidation "k8s.io/apiextensions-apiserver/pkg/apiserver/validation" diff --git a/test/e2e/apimachinery/crd_publish_openapi.go b/test/e2e/apimachinery/crd_publish_openapi.go index 1431e5388ef..d4e141bd47e 100644 --- a/test/e2e/apimachinery/crd_publish_openapi.go +++ b/test/e2e/apimachinery/crd_publish_openapi.go @@ -29,6 +29,7 @@ import ( "github.com/go-openapi/spec" "github.com/onsi/ginkgo" openapiutil "k8s.io/kube-openapi/pkg/util" + kubeopenapispec "k8s.io/kube-openapi/pkg/validation/spec" "k8s.io/utils/pointer" "sigs.k8s.io/yaml" @@ -683,10 +684,15 @@ func convertJSONSchemaProps(in []byte, out *spec.Schema) error { if err := apiextensionsv1.Convert_v1_JSONSchemaProps_To_apiextensions_JSONSchemaProps(&external, &internal, nil); err != nil { return err } - if err := validation.ConvertJSONSchemaPropsWithPostProcess(&internal, out, validation.StripUnsupportedFormatsPostProcess); err != nil { + kubeOut := kubeopenapispec.Schema{} + if err := validation.ConvertJSONSchemaPropsWithPostProcess(&internal, &kubeOut, validation.StripUnsupportedFormatsPostProcess); err != nil { return err } - return nil + bs, err := json.Marshal(kubeOut) + if err != nil { + return err + } + return json.Unmarshal(bs, out) } // dropDefaults drops properties and extension that we added to a schema