diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/defaults.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/defaults.go index 1b7ea2ffe0b..5cebec927bd 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/defaults.go +++ b/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 diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/defaults_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/defaults_test.go index ba29b4bf995..7640c10c5c4 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/defaults_test.go +++ b/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}, - PreserveUnknownFields: utilpointer.BoolPtr(true), + Conversion: &CustomResourceConversion{Strategy: NoneConverter}, }, }, }, @@ -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, - Conversion: &CustomResourceConversion{Strategy: NoneConverter}, - PreserveUnknownFields: utilpointer.BoolPtr(true), + Scope: NamespaceScoped, + Conversion: &CustomResourceConversion{Strategy: NoneConverter}, 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, - Conversion: &CustomResourceConversion{Strategy: NoneConverter}, - PreserveUnknownFields: utilpointer.BoolPtr(true), + Scope: NamespaceScoped, + Conversion: &CustomResourceConversion{Strategy: NoneConverter}, 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, - Conversion: &CustomResourceConversion{Strategy: NoneConverter}, - PreserveUnknownFields: utilpointer.BoolPtr(true), + Scope: NamespaceScoped, + Conversion: &CustomResourceConversion{Strategy: NoneConverter}, Versions: []CustomResourceDefinitionVersion{ {Name: "v1", Storage: true}, }, @@ -119,9 +113,8 @@ func TestDefaults(t *testing.T) { }, expected: &CustomResourceDefinition{ Spec: CustomResourceDefinitionSpec{ - Scope: NamespaceScoped, - Conversion: &CustomResourceConversion{Strategy: NoneConverter}, - PreserveUnknownFields: utilpointer.BoolPtr(true), + Scope: NamespaceScoped, + Conversion: &CustomResourceConversion{Strategy: NoneConverter}, Versions: []CustomResourceDefinitionVersion{ {Name: "v1", Storage: true}, }, diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/types.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/types.go index 3c819624048..da5a80e1660 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/types.go +++ b/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. - PreserveUnknownFields *bool `json:"preserveUnknownFields,omitempty" protobuf:"varint,10,opt,name=preserveUnknownFields"` + // This field is deprecated in favor of setting `x-preserve-unknown-fields` to true in `spec.versions[*].schema.openAPIV3Schema`. + // +optional + PreserveUnknownFields bool `json:"preserveUnknownFields,omitempty" protobuf:"varint,10,opt,name=preserveUnknownFields"` } // CustomResourceConversion describes how to convert different versions of a CR. 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 fb57ccf1eeb..e2606024b90 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 @@ -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 { 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 67806c34d90..23957f473cd 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 @@ -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{