Merge pull request kubernetes#77750 from sttts/sttts-structural-schema-false-XPreserveUnknownFields

k8s-ci-robot · web-flow · commit 44301940eae0 · 2019-05-13T21:49:22.000-07:00
apiextensions: forbid false x-kubernetes-preserve-unknown-fields
diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json
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
@@ -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
@@ -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
@@ -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.pb.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/generated.pb.go
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
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
@@ -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/v1beta1/zz_generated.conversion.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/zz_generated.conversion.go
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)

Original file line number	Diff line number	Diff line change
`@@ -258,5 +258,15 @@ func (in JSONSchemaProps) DeepCopy() JSONSchemaProps {`
`258`	`258`	`}`
`259`	`259`	`}`
`260`	`260`
	`261`	`+ if in.XPreserveUnknownFields != nil {`
	`262`	`+ in, out := &in.XPreserveUnknownFields, &out.XPreserveUnknownFields`
	`263`	`+ if *in == nil {`
	`264`	`+ *out = nil`
	`265`	`+ } else {`
	`266`	`+ *out = new(bool)`
	`267`	`+ out = in`
	`268`	`+ }`
	`269`	`+ }`
	`270`	`+`
`261`	`271`	`return out`
`262`	`272`	`}`
Original file line number	Diff line number	Diff line change
`@@ -234,5 +234,15 @@ func (in JSONSchemaProps) DeepCopy() JSONSchemaProps {`
`234`	`234`	`}`
`235`	`235`	`}`
`236`	`236`
	`237`	`+ if in.XPreserveUnknownFields != nil {`
	`238`	`+ in, out := &in.XPreserveUnknownFields, &out.XPreserveUnknownFields`
	`239`	`+ if *in == nil {`
	`240`	`+ *out = nil`
	`241`	`+ } else {`
	`242`	`+ *out = new(bool)`
	`243`	`+ out = in`
	`244`	`+ }`
	`245`	`+ }`
	`246`	`+`
`237`	`247`	`return out`
`238`	`248`	`}`
Original file line number	Diff line number	Diff line change
`@@ -34,6 +34,7 @@ go_test(`
`34`	`34`	`"//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library",`
`35`	`35`	`"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",`
`36`	`36`	`"//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",`
	`37`	`+ "//vendor/k8s.io/utils/pointer:go_default_library",`
`37`	`38`	`],`
`38`	`39`	`)`
`39`	`40`
Original file line number	Diff line number	Diff line change
`@@ -673,6 +673,10 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch`
`673`	`673`	`}`
`674`	`674`	`}`
`675`	`675`
	`676`	`+ if schema.XPreserveUnknownFields != nil && *schema.XPreserveUnknownFields == false {`
	`677`	`+ allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-preserve-unknown-fields"), *schema.XPreserveUnknownFields, "must be true or undefined"))`
	`678`	`+ }`
	`679`	`+`
`676`	`680`	`return allErrs`
`677`	`681`	`}`
`678`	`682`