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 812526c8c96..06a70cd1134 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 @@ -62,7 +62,7 @@ func ValidateCustomResourceDefinition(obj *apiextensions.CustomResourceDefinitio } opts := validationOptions{ - allowDefaults: allowDefaults(requestGV), + allowDefaults: allowDefaults(requestGV, nil), requireRecognizedConversionReviewVersion: true, requireImmutableNames: false, requireOpenAPISchema: requireOpenAPISchema(requestGV, nil), @@ -92,7 +92,7 @@ type validationOptions struct { // ValidateCustomResourceDefinitionUpdate statically validates func ValidateCustomResourceDefinitionUpdate(obj, oldObj *apiextensions.CustomResourceDefinition, requestGV schema.GroupVersion) field.ErrorList { opts := validationOptions{ - allowDefaults: allowDefaults(requestGV) || specHasDefaults(&oldObj.Spec), + allowDefaults: allowDefaults(requestGV, &oldObj.Spec), requireRecognizedConversionReviewVersion: oldObj.Spec.Conversion == nil || hasValidConversionReviewVersionOrEmpty(oldObj.Spec.Conversion.ConversionReviewVersions), requireImmutableNames: apiextensions.IsCRDConditionTrue(oldObj, apiextensions.Established), requireOpenAPISchema: requireOpenAPISchema(requestGV, &oldObj.Spec), @@ -988,10 +988,17 @@ func allVersionsSpecifyOpenAPISchema(spec *apiextensions.CustomResourceDefinitio } // allowDefaults returns true if the defaulting feature is enabled and the request group version allows adding defaults -func allowDefaults(requestGV schema.GroupVersion) bool { +func allowDefaults(requestGV schema.GroupVersion, oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool { + if oldCRDSpec != nil && specHasDefaults(oldCRDSpec) { + // don't tighten validation on existing persisted data + return true + } if !utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceDefaulting) { return false } + if requestGV == v1beta1.SchemeGroupVersion { + return false + } return true } 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 41d5f567a3c..ee85292bd40 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 @@ -1542,6 +1542,79 @@ func TestValidateCustomResourceDefinition(t *testing.T) { forbidden("spec", "validation", "openAPIV3Schema", "properties[a]", "default"), // disabled feature-gate }, }, + { + name: "defaults with enabled feature gate via v1beta1", + resource: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Version: "version", + Versions: singleVersionList, + Scope: apiextensions.NamespaceScoped, + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "Plural", + ListKind: "PluralList", + }, + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "a": { + Type: "number", + Default: jsonPtr(42.0), + }, + }, + }, + }, + PreserveUnknownFields: pointer.BoolPtr(true), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, + requestGV: apiextensionsv1beta1.SchemeGroupVersion, + errors: []validationMatch{ + forbidden("spec", "validation", "openAPIV3Schema", "properties[a]", "default"), // disallowed via v1beta1 + }, + }, + { + name: "defaults with enabled feature gate via v1", + resource: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Version: "version", + Versions: singleVersionList, + Scope: apiextensions.NamespaceScoped, + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "Plural", + ListKind: "PluralList", + }, + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "a": { + Type: "number", + Default: jsonPtr(42.0), + }, + }, + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, + requestGV: apiextensionsv1.SchemeGroupVersion, + }, { name: "x-kubernetes-int-or-string without structural", resource: &apiextensions.CustomResourceDefinition{ @@ -2019,44 +2092,6 @@ func TestValidateCustomResourceDefinition(t *testing.T) { enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, }, - { - name: "defaults with enabled feature gate, structural schema, without pruning", - resource: &apiextensions.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, - Spec: apiextensions.CustomResourceDefinitionSpec{ - Group: "group.com", - Version: "version", - Versions: singleVersionList, - Scope: apiextensions.NamespaceScoped, - Names: apiextensions.CustomResourceDefinitionNames{ - Plural: "plural", - Singular: "singular", - Kind: "Plural", - ListKind: "PluralList", - }, - Validation: &apiextensions.CustomResourceValidation{ - OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ - Type: "object", - Properties: map[string]apiextensions.JSONSchemaProps{ - "a": { - Type: "number", - Default: jsonPtr(42.0), - }, - }, - }, - }, - PreserveUnknownFields: pointer.BoolPtr(true), - }, - Status: apiextensions.CustomResourceDefinitionStatus{ - StoredVersions: []string{"version"}, - }, - }, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, - errors: []validationMatch{ - invalid("spec", "preserveUnknownFields"), - }, - enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, - }, { name: "additionalProperties at resource root", resource: &apiextensions.CustomResourceDefinition{ @@ -3697,7 +3732,7 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { }, }, { - name: "setting defaults with enabled feature gate", + name: "setting defaults with enabled feature gate via v1beta1", old: &apiextensions.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{ Name: "plural.group.com", @@ -3775,11 +3810,95 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { StoredVersions: []string{"version"}, }, }, + requestGV: apiextensionsv1beta1.SchemeGroupVersion, + errors: []validationMatch{forbidden("spec.validation.openAPIV3Schema.properties[a].default")}, + enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, + }, + { + name: "setting defaults with enabled feature gate via v1", + old: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "plural.group.com", + ResourceVersion: "42", + }, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Version: "version", + Versions: []apiextensions.CustomResourceDefinitionVersion{ + { + Name: "version", + Served: true, + Storage: true, + }, + }, + Scope: apiextensions.NamespaceScoped, + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "Plural", + ListKind: "PluralList", + }, + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "a": { + Type: "number", + }, + }, + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + resource: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "plural.group.com", + ResourceVersion: "42", + }, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Version: "version", + Versions: []apiextensions.CustomResourceDefinitionVersion{ + { + Name: "version", + Served: true, + Storage: true, + }, + }, + Scope: apiextensions.NamespaceScoped, + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "Plural", + ListKind: "PluralList", + }, + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "a": { + Type: "number", + Default: jsonPtr(42.0), + }, + }, + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + requestGV: apiextensionsv1.SchemeGroupVersion, errors: []validationMatch{}, enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, }, { - name: "ratcheting validation of defaults with disabled feature gate", + name: "ratcheting validation of defaults with disabled feature gate via v1beta1", old: &apiextensions.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{ Name: "plural.group.com", @@ -3862,7 +3981,162 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { StoredVersions: []string{"version"}, }, }, - errors: []validationMatch{}, + requestGV: apiextensionsv1beta1.SchemeGroupVersion, + errors: []validationMatch{}, + }, + { + name: "ratcheting validation of defaults with disabled feature gate via v1", + old: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "plural.group.com", + ResourceVersion: "42", + }, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Version: "version", + Versions: []apiextensions.CustomResourceDefinitionVersion{ + { + Name: "version", + Served: true, + Storage: true, + }, + }, + Scope: apiextensions.NamespaceScoped, + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "Plural", + ListKind: "PluralList", + }, + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "a": { + Type: "number", + Default: jsonPtr(42.0), + }, + }, + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + resource: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "plural.group.com", + ResourceVersion: "42", + }, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Version: "version", + Versions: []apiextensions.CustomResourceDefinitionVersion{ + { + Name: "version", + Served: true, + Storage: true, + }, + }, + Scope: apiextensions.NamespaceScoped, + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "Plural", + ListKind: "PluralList", + }, + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "a": { + Type: "number", + Default: jsonPtr(42.0), + }, + "b": { + Type: "number", + Default: jsonPtr(43.0), + }, + }, + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + requestGV: apiextensionsv1.SchemeGroupVersion, + errors: []validationMatch{}, + }, + { + name: "add default with enabled feature gate, structural schema, without pruning", + old: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "42"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Version: "version", + Versions: []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}}, + Scope: apiextensions.NamespaceScoped, + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "Plural", + ListKind: "PluralList", + }, + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "a": { + Type: "number", + //Default: jsonPtr(42.0), + }, + }, + }, + }, + PreserveUnknownFields: pointer.BoolPtr(true), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + resource: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "42"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Version: "version", + Versions: []apiextensions.CustomResourceDefinitionVersion{{Name: "version", Served: true, Storage: true}}, + Scope: apiextensions.NamespaceScoped, + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "Plural", + ListKind: "PluralList", + }, + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "a": { + Type: "number", + Default: jsonPtr(42.0), + }, + }, + }, + }, + PreserveUnknownFields: pointer.BoolPtr(true), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{ + invalid("spec", "preserveUnknownFields"), + }, + enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, }, }