From d014591a11198e3eec905cb3d64141ead83ae359 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Fri, 10 May 2019 19:48:55 +0200 Subject: [PATCH] apiextensions: forbid false x-kubernetes-preserve-unknown-fields --- .../pkg/apis/apiextensions/deepcopy.go | 10 ++++++ .../apis/apiextensions/types_jsonschema.go | 3 +- .../apis/apiextensions/v1beta1/deepcopy.go | 10 ++++++ .../apiextensions/v1beta1/generated.proto | 1 + .../apiextensions/v1beta1/types_jsonschema.go | 3 +- .../pkg/apis/apiextensions/validation/BUILD | 1 + .../apiextensions/validation/validation.go | 4 +++ .../validation/validation_test.go | 36 +++++++++++++++++++ .../pkg/apiserver/schema/convert.go | 18 +++++++--- .../pkg/apiserver/schema/structural.go | 2 ++ .../pkg/apiserver/validation/validation.go | 4 +-- 11 files changed, 83 insertions(+), 9 deletions(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/deepcopy.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/deepcopy.go index 37b4d1df9fe..51fb72df3cf 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/deepcopy.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/deepcopy.go @@ -258,5 +258,15 @@ func (in *JSONSchemaProps) DeepCopy() *JSONSchemaProps { } } + if in.XPreserveUnknownFields != nil { + in, out := &in.XPreserveUnknownFields, &out.XPreserveUnknownFields + if *in == nil { + *out = nil + } else { + *out = new(bool) + **out = **in + } + } + return out } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types_jsonschema.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types_jsonschema.go index e0cba964731..78223934628 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types_jsonschema.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types_jsonschema.go @@ -61,7 +61,8 @@ type JSONSchemaProps struct { // in the validation schema. This affects fields recursively, // but switches back to normal pruning behaviour if nested // properties or additionalProperties are specified in the schema. - XPreserveUnknownFields bool + // This can either be true or undefined. False is forbidden. + XPreserveUnknownFields *bool // x-kubernetes-embedded-resource defines that the value is an // embedded Kubernetes runtime.Object, with TypeMeta and diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/deepcopy.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/deepcopy.go index f6a114e2b39..f67f4418125 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/deepcopy.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/deepcopy.go @@ -234,5 +234,15 @@ func (in *JSONSchemaProps) DeepCopy() *JSONSchemaProps { } } + if in.XPreserveUnknownFields != nil { + in, out := &in.XPreserveUnknownFields, &out.XPreserveUnknownFields + if *in == nil { + *out = nil + } else { + *out = new(bool) + **out = **in + } + } + return out } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/generated.proto b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/generated.proto index a0c23a44f56..8961120d4cb 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/generated.proto +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/generated.proto @@ -449,6 +449,7 @@ message JSONSchemaProps { // in the validation schema. This affects fields recursively, // but switches back to normal pruning behaviour if nested // properties or additionalProperties are specified in the schema. + // This can either be true or undefined. False is forbidden. optional bool xKubernetesPreserveUnknownFields = 38; // x-kubernetes-embedded-resource defines that the value is an diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types_jsonschema.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types_jsonschema.go index 84f26e600af..da5e857f110 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types_jsonschema.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types_jsonschema.go @@ -61,7 +61,8 @@ type JSONSchemaProps struct { // in the validation schema. This affects fields recursively, // but switches back to normal pruning behaviour if nested // properties or additionalProperties are specified in the schema. - XPreserveUnknownFields bool `json:"x-kubernetes-preserve-unknown-fields,omitempty" protobuf:"bytes,38,opt,name=xKubernetesPreserveUnknownFields"` + // This can either be true or undefined. False is forbidden. + XPreserveUnknownFields *bool `json:"x-kubernetes-preserve-unknown-fields,omitempty" protobuf:"bytes,38,opt,name=xKubernetesPreserveUnknownFields"` // x-kubernetes-embedded-resource defines that the value is an // embedded Kubernetes runtime.Object, with TypeMeta and diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/BUILD b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/BUILD index 5f974a41f5e..319042d44d5 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/BUILD +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/BUILD @@ -34,6 +34,7 @@ go_test( "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", + "//vendor/k8s.io/utils/pointer:go_default_library", ], ) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go index e91de0eaa88..2950ffaae27 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go @@ -673,6 +673,10 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch } } + if schema.XPreserveUnknownFields != nil && *schema.XPreserveUnknownFields == false { + allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-preserve-unknown-fields"), *schema.XPreserveUnknownFields, "must be true or undefined")) + } + return allErrs } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go index 33387c6af3a..babd1c309bc 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go @@ -22,6 +22,7 @@ import ( "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/utils/pointer" ) type validationMatch struct { @@ -935,6 +936,41 @@ func TestValidateCustomResourceDefinition(t *testing.T) { invalid("spec", "versions"), }, }, + { + name: "x-kubernetes-preserve-unknown-field: false", + resource: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Version: "version", + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + XPreserveUnknownFields: pointer.BoolPtr(false), + }, + }, + Versions: []apiextensions.CustomResourceDefinitionVersion{ + { + Name: "version", + Served: true, + Storage: true, + }, + }, + Scope: apiextensions.NamespaceScoped, + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "Plural", + ListKind: "PluralList", + }, + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{ + invalid("spec", "validation", "openAPIV3Schema", "x-kubernetes-preserve-unknown-fields"), + }, + }, } for _, tc := range tests { diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/convert.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/convert.go index 2ed71b2618c..388fb0a47e1 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/convert.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/convert.go @@ -241,11 +241,19 @@ func newExtensions(s *apiextensions.JSONSchemaProps) (*Extensions, error) { return nil, nil } - return &Extensions{ - XPreserveUnknownFields: s.XPreserveUnknownFields, - XEmbeddedResource: s.XEmbeddedResource, - XIntOrString: s.XIntOrString, - }, nil + ret := &Extensions{ + XEmbeddedResource: s.XEmbeddedResource, + XIntOrString: s.XIntOrString, + } + + if s.XPreserveUnknownFields != nil { + if !*s.XPreserveUnknownFields { + return nil, fmt.Errorf("'x-kubernetes-preserve-unknown-fields' must be true or undefined") + } + ret.XPreserveUnknownFields = true + } + + return ret, nil } // validateUnsupportedFields checks that those fields rejected by validation are actually unset. diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/structural.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/structural.go index 996336c7dc7..a060644a7cd 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/structural.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/structural.go @@ -66,6 +66,8 @@ type Extensions struct { // in the validation schema. This affects fields recursively, // but switches back to normal pruning behaviour if nested // properties or additionalProperties are specified in the schema. + // False means that the pruning behaviour is inherited from the parent. + // False does not mean to activate pruning. XPreserveUnknownFields bool // x-kubernetes-embedded-resource defines that the value is an 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 00a8ac93c1b..039c37b1246 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 @@ -195,8 +195,8 @@ func ConvertJSONSchemaPropsWithPostProcess(in *apiextensions.JSONSchemaProps, ou } } - if in.XPreserveUnknownFields { - out.VendorExtensible.AddExtension("x-kubernetes-preserve-unknown-fields", true) + if in.XPreserveUnknownFields != nil { + out.VendorExtensible.AddExtension("x-kubernetes-preserve-unknown-fields", *in.XPreserveUnknownFields) } if in.XEmbeddedResource { out.VendorExtensible.AddExtension("x-kubernetes-embedded-resource", true)