CRD v1: disallow spec.preserveUnknownFields=true

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/defaults.go

@@ -51,9 +51,6 @@ func SetDefaults_CustomResourceDefinitionSpec(obj *CustomResourceDefinitionSpec)
 			Strategy: NoneConverter,
 		}
 	}
-	if obj.PreserveUnknownFields == nil {
-		obj.PreserveUnknownFields = utilpointer.BoolPtr(true)
-	}
 }
 
 // SetDefaults_ServiceReference sets defaults for Webhook's ServiceReference

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/defaults_test.go

@@ -39,8 +39,7 @@ func TestDefaults(t *testing.T) {
 			original: &CustomResourceDefinition{},
 			expected: &CustomResourceDefinition{
 				Spec: CustomResourceDefinitionSpec{
-					Conversion:            &CustomResourceConversion{Strategy: NoneConverter},
+					Conversion: &CustomResourceConversion{Strategy: NoneConverter},
-					PreserveUnknownFields: utilpointer.BoolPtr(true),
 				},
 			},
 		},
@@ -57,7 +56,6 @@ func TestDefaults(t *testing.T) {
 							},
 						},
 					},
-					PreserveUnknownFields: utilpointer.BoolPtr(true),
 				},
 			},
 			expected: &CustomResourceDefinition{
@@ -71,7 +69,6 @@ func TestDefaults(t *testing.T) {
 							},
 						},
 					},
-					PreserveUnknownFields: utilpointer.BoolPtr(true),
 				},
 			},
 		},
@@ -79,9 +76,8 @@ func TestDefaults(t *testing.T) {
 			name: "storage status defaults",
 			original: &CustomResourceDefinition{
 				Spec: CustomResourceDefinitionSpec{
-					Scope:                 NamespaceScoped,
+					Scope:      NamespaceScoped,
-					Conversion:            &CustomResourceConversion{Strategy: NoneConverter},
+					Conversion: &CustomResourceConversion{Strategy: NoneConverter},
-					PreserveUnknownFields: utilpointer.BoolPtr(true),
 					Versions: []CustomResourceDefinitionVersion{
 						{Name: "v1", Storage: false, Served: true},
 						{Name: "v2", Storage: true, Served: true},
@@ -91,9 +87,8 @@ func TestDefaults(t *testing.T) {
 			},
 			expected: &CustomResourceDefinition{
 				Spec: CustomResourceDefinitionSpec{
-					Scope:                 NamespaceScoped,
+					Scope:      NamespaceScoped,
-					Conversion:            &CustomResourceConversion{Strategy: NoneConverter},
+					Conversion: &CustomResourceConversion{Strategy: NoneConverter},
-					PreserveUnknownFields: utilpointer.BoolPtr(true),
 					Versions: []CustomResourceDefinitionVersion{
 						{Name: "v1", Storage: false, Served: true},
 						{Name: "v2", Storage: true, Served: true},
@@ -109,9 +104,8 @@ func TestDefaults(t *testing.T) {
 			name: "version defaults",
 			original: &CustomResourceDefinition{
 				Spec: CustomResourceDefinitionSpec{
-					Scope:                 NamespaceScoped,
+					Scope:      NamespaceScoped,
-					Conversion:            &CustomResourceConversion{Strategy: NoneConverter},
+					Conversion: &CustomResourceConversion{Strategy: NoneConverter},
-					PreserveUnknownFields: utilpointer.BoolPtr(true),
 					Versions: []CustomResourceDefinitionVersion{
 						{Name: "v1", Storage: true},
 					},
@@ -119,9 +113,8 @@ func TestDefaults(t *testing.T) {
 			},
 			expected: &CustomResourceDefinition{
 				Spec: CustomResourceDefinitionSpec{
-					Scope:                 NamespaceScoped,
+					Scope:      NamespaceScoped,
-					Conversion:            &CustomResourceConversion{Strategy: NoneConverter},
+					Conversion: &CustomResourceConversion{Strategy: NoneConverter},
-					PreserveUnknownFields: utilpointer.BoolPtr(true),
 					Versions: []CustomResourceDefinitionVersion{
 						{Name: "v1", Storage: true},
 					},

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/types.go

@@ -57,8 +57,9 @@ type CustomResourceDefinitionSpec struct {
 	// preserveUnknownFields disables pruning of object fields which are not
 	// specified in the OpenAPI schema. apiVersion, kind, metadata and known
 	// fields inside metadata are always preserved.
-	// Defaults to true in v1beta and will default to false in v1.
+	// This field is deprecated in favor of setting `x-preserve-unknown-fields` to true in `spec.versions[*].schema.openAPIV3Schema`.
-	PreserveUnknownFields *bool `json:"preserveUnknownFields,omitempty" protobuf:"varint,10,opt,name=preserveUnknownFields"`
+	// +optional
+	PreserveUnknownFields bool `json:"preserveUnknownFields,omitempty" protobuf:"varint,10,opt,name=preserveUnknownFields"`
 }
 
 // CustomResourceConversion describes how to convert different versions of a CR.

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

@@ -72,6 +72,7 @@ func ValidateCustomResourceDefinition(obj *apiextensions.CustomResourceDefinitio
 	allErrs = append(allErrs, ValidateCustomResourceDefinitionStatus(&obj.Status, field.NewPath("status"))...)
 	allErrs = append(allErrs, ValidateCustomResourceDefinitionStoredVersions(obj.Status.StoredVersions, obj.Spec.Versions, field.NewPath("status").Child("storedVersions"))...)
 	allErrs = append(allErrs, validateAPIApproval(obj, nil, requestGV)...)
+	allErrs = append(allErrs, validatePreserveUnknownFields(obj, nil, requestGV)...)
 	return allErrs
 }
 
@@ -98,6 +99,7 @@ func ValidateCustomResourceDefinitionUpdate(obj, oldObj *apiextensions.CustomRes
 	allErrs = append(allErrs, ValidateCustomResourceDefinitionStatus(&obj.Status, field.NewPath("status"))...)
 	allErrs = append(allErrs, ValidateCustomResourceDefinitionStoredVersions(obj.Status.StoredVersions, obj.Spec.Versions, field.NewPath("status").Child("storedVersions"))...)
 	allErrs = append(allErrs, validateAPIApproval(obj, oldObj, requestGV)...)
+	allErrs = append(allErrs, validatePreserveUnknownFields(obj, oldObj, requestGV)...)
 	return allErrs
 }
 
@@ -1159,6 +1161,25 @@ func validateAPIApproval(newCRD, oldCRD *apiextensions.CustomResourceDefinition,
 	}
 }
 
+func validatePreserveUnknownFields(crd, oldCRD *apiextensions.CustomResourceDefinition, requestGV schema.GroupVersion) field.ErrorList {
+	if requestGV == v1beta1.SchemeGroupVersion {
+		// no-op for compatibility with v1beta1
+		return nil
+	}
+
+	if oldCRD != nil && oldCRD.Spec.PreserveUnknownFields != nil && *oldCRD.Spec.PreserveUnknownFields {
+		// no-op for compatibility with existing data
+		return nil
+	}
+
+	var errs field.ErrorList
+	if crd != nil && crd.Spec.PreserveUnknownFields != nil && *crd.Spec.PreserveUnknownFields {
+		// disallow changing spec.preserveUnknownFields=false to spec.preserveUnknownFields=true
+		errs = append(errs, field.Invalid(field.NewPath("spec").Child("preserveUnknownFields"), crd.Spec.PreserveUnknownFields, "cannot set to true, set x-preserve-unknown-fields to true in spec.versions[*].schema instead"))
+	}
+	return errs
+}
+
 // SchemaHasInvalidTypes returns true if it contains invalid offending openapi-v3 specification.
 func SchemaHasInvalidTypes(s *apiextensions.JSONSchemaProps) bool {
 	if s == nil {

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

@@ -23,6 +23,7 @@ import (
 
 	"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"
 	apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	"k8s.io/apiextensions-apiserver/pkg/features"
 	"k8s.io/apimachinery/pkg/api/apitesting/fuzzer"
@@ -1364,6 +1365,24 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
 				required("spec", "versions[1]", "schema", "openAPIV3Schema"),
 			},
 		},
+		{
+			name: "preserveUnknownFields: true via v1",
+			resource: &apiextensions.CustomResourceDefinition{
+				ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"},
+				Spec: apiextensions.CustomResourceDefinitionSpec{
+					Group:                 "group.com",
+					Version:               "version",
+					Versions:              []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}},
+					Validation:            &apiextensions.CustomResourceValidation{OpenAPIV3Schema: &apiextensions.JSONSchemaProps{Type: "object"}},
+					Scope:                 apiextensions.NamespaceScoped,
+					Names:                 apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"},
+					PreserveUnknownFields: pointer.BoolPtr(true),
+				},
+				Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}},
+			},
+			requestGV: apiextensionsv1.SchemeGroupVersion,
+			errors:    []validationMatch{invalid("spec.preserveUnknownFields")},
+		},
 		{
 			name: "labelSelectorPath outside of .spec and .status",
 			resource: &apiextensions.CustomResourceDefinition{
@@ -1988,6 +2007,7 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
 					StoredVersions: []string{"version"},
 				},
 			},
+			requestGV: apiextensionsv1beta1.SchemeGroupVersion,
 			errors: []validationMatch{
 				invalid("spec", "preserveUnknownFields"),
 			},
@@ -3376,6 +3396,99 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) {
 			requestGV: apiextensionsv1beta1.SchemeGroupVersion,
 			errors:    []validationMatch{},
 		},
+		{
+			name: "switch to preserveUnknownFields: true is allowed via v1beta1",
+			old: &apiextensions.CustomResourceDefinition{
+				ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"},
+				Spec: apiextensions.CustomResourceDefinitionSpec{
+					Group:                 "group.com",
+					Version:               "version",
+					Versions:              []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}},
+					Validation:            &apiextensions.CustomResourceValidation{OpenAPIV3Schema: &apiextensions.JSONSchemaProps{Type: "object"}},
+					Scope:                 apiextensions.NamespaceScoped,
+					Names:                 apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"},
+					PreserveUnknownFields: pointer.BoolPtr(false),
+				},
+				Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}},
+			},
+			resource: &apiextensions.CustomResourceDefinition{
+				ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"},
+				Spec: apiextensions.CustomResourceDefinitionSpec{
+					Group:                 "group.com",
+					Version:               "version",
+					Versions:              []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}},
+					Validation:            &apiextensions.CustomResourceValidation{OpenAPIV3Schema: &apiextensions.JSONSchemaProps{Type: "object"}},
+					Scope:                 apiextensions.NamespaceScoped,
+					Names:                 apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"},
+					PreserveUnknownFields: pointer.BoolPtr(true),
+				},
+				Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}},
+			},
+			requestGV: apiextensionsv1beta1.SchemeGroupVersion,
+			errors:    []validationMatch{},
+		},
+		{
+			name: "switch to preserveUnknownFields: true is forbidden via v1",
+			old: &apiextensions.CustomResourceDefinition{
+				ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"},
+				Spec: apiextensions.CustomResourceDefinitionSpec{
+					Group:                 "group.com",
+					Version:               "version",
+					Versions:              []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}},
+					Validation:            &apiextensions.CustomResourceValidation{OpenAPIV3Schema: &apiextensions.JSONSchemaProps{Type: "object"}},
+					Scope:                 apiextensions.NamespaceScoped,
+					Names:                 apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"},
+					PreserveUnknownFields: pointer.BoolPtr(false),
+				},
+				Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}},
+			},
+			resource: &apiextensions.CustomResourceDefinition{
+				ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"},
+				Spec: apiextensions.CustomResourceDefinitionSpec{
+					Group:                 "group.com",
+					Version:               "version",
+					Versions:              []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}},
+					Validation:            &apiextensions.CustomResourceValidation{OpenAPIV3Schema: &apiextensions.JSONSchemaProps{Type: "object"}},
+					Scope:                 apiextensions.NamespaceScoped,
+					Names:                 apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"},
+					PreserveUnknownFields: pointer.BoolPtr(true),
+				},
+				Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}},
+			},
+			requestGV: apiextensionsv1.SchemeGroupVersion,
+			errors:    []validationMatch{invalid("spec.preserveUnknownFields")},
+		},
+		{
+			name: "keep preserveUnknownFields: true is allowed via v1",
+			old: &apiextensions.CustomResourceDefinition{
+				ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"},
+				Spec: apiextensions.CustomResourceDefinitionSpec{
+					Group:                 "group.com",
+					Version:               "version",
+					Versions:              []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}},
+					Validation:            &apiextensions.CustomResourceValidation{OpenAPIV3Schema: &apiextensions.JSONSchemaProps{Type: "object"}},
+					Scope:                 apiextensions.NamespaceScoped,
+					Names:                 apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"},
+					PreserveUnknownFields: pointer.BoolPtr(true),
+				},
+				Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}},
+			},
+			resource: &apiextensions.CustomResourceDefinition{
+				ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "1"},
+				Spec: apiextensions.CustomResourceDefinitionSpec{
+					Group:                 "group.com",
+					Version:               "version",
+					Versions:              []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}},
+					Validation:            &apiextensions.CustomResourceValidation{OpenAPIV3Schema: &apiextensions.JSONSchemaProps{Type: "object"}},
+					Scope:                 apiextensions.NamespaceScoped,
+					Names:                 apiextensions.CustomResourceDefinitionNames{Plural: "plural", Singular: "singular", Kind: "Plural", ListKind: "PluralList"},
+					PreserveUnknownFields: pointer.BoolPtr(true),
+				},
+				Status: apiextensions.CustomResourceDefinitionStatus{StoredVersions: []string{"version"}},
+			},
+			requestGV: apiextensionsv1.SchemeGroupVersion,
+			errors:    []validationMatch{},
+		},
 		{
 			name: "setting defaults with enabled feature gate",
 			old: &apiextensions.CustomResourceDefinition{