From e59472577fb8c7903f53f2a67cb7079469a59c8e Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Wed, 29 May 2019 23:17:43 +0200 Subject: [PATCH] apiextensions: remove ratcheting logic to allow conversion with preserveUnknownField --- .../apiextensions/validation/validation.go | 12 +-- .../validation/validation_test.go | 76 ------------------- 2 files changed, 4 insertions(+), 84 deletions(-) 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 2a0f663bdf7..ba49f192b2f 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 @@ -121,10 +121,10 @@ func ValidateCustomResourceDefinitionVersion(version *apiextensions.CustomResour // ValidateCustomResourceDefinitionSpec statically validates func ValidateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefinitionSpec, fldPath *field.Path) field.ErrorList { allowDefaults := utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceDefaulting) - return validateCustomResourceDefinitionSpec(spec, true, allowDefaults, false, fldPath) + return validateCustomResourceDefinitionSpec(spec, true, allowDefaults, fldPath) } -func validateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefinitionSpec, requireRecognizedVersion, allowDefaults, allowConversionWithPreserveUnknownFields bool, fldPath *field.Path) field.ErrorList { +func validateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefinitionSpec, requireRecognizedVersion, allowDefaults bool, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if len(spec.Group) == 0 { @@ -237,7 +237,7 @@ func validateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefi } } - if !allowConversionWithPreserveUnknownFields && conversionAndPreserveUnknownFields(spec) { + if (spec.Conversion != nil && spec.Conversion.Strategy != apiextensions.NoneConverter) && (spec.PreserveUnknownFields == nil || *spec.PreserveUnknownFields) { allErrs = append(allErrs, field.Invalid(fldPath.Child("conversion").Child("strategy"), spec.Conversion.Strategy, "must be None if spec.preserveUnknownFields is true")) } allErrs = append(allErrs, validateCustomResourceConversion(spec.Conversion, requireRecognizedVersion, fldPath.Child("conversion"))...) @@ -245,10 +245,6 @@ func validateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefi return allErrs } -func conversionAndPreserveUnknownFields(spec *apiextensions.CustomResourceDefinitionSpec) bool { - return (spec.Conversion != nil && spec.Conversion.Strategy != apiextensions.NoneConverter) && (spec.PreserveUnknownFields == nil || *spec.PreserveUnknownFields) -} - func validateEnumStrings(fldPath *field.Path, value string, accepted []string, required bool) field.ErrorList { if value == "" { if required { @@ -366,7 +362,7 @@ func ValidateCustomResourceDefinitionSpecUpdate(spec, oldSpec *apiextensions.Cus // find out whether any schema had default before. Then we keep allowing it. allowDefaults := utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceDefaulting) || specHasDefaults(oldSpec) - allErrs := validateCustomResourceDefinitionSpec(spec, requireRecognizedVersion, allowDefaults, conversionAndPreserveUnknownFields(oldSpec), fldPath) + allErrs := validateCustomResourceDefinitionSpec(spec, requireRecognizedVersion, allowDefaults, fldPath) if established { // these effect the storage and cannot be changed therefore 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 1d174e34ead..f7df70171ba 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 @@ -2044,82 +2044,6 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { invalid("spec", "conversion", "conversionReviewVersions"), }, }, - { - name: "webhookconfig: should accept preserveUnknownFields=true if set before", - resource: &apiextensions.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "42"}, - Spec: apiextensions.CustomResourceDefinitionSpec{ - Group: "group.com", - Scope: apiextensions.ResourceScope("Cluster"), - Names: apiextensions.CustomResourceDefinitionNames{ - Plural: "plural", - Singular: "singular", - Kind: "Plural", - ListKind: "PluralList", - }, - Versions: []apiextensions.CustomResourceDefinitionVersion{ - { - Name: "version", - Served: true, - Storage: true, - }, - { - Name: "version2", - Served: true, - Storage: false, - }, - }, - Conversion: &apiextensions.CustomResourceConversion{ - Strategy: apiextensions.ConversionStrategyType("Webhook"), - WebhookClientConfig: &apiextensions.WebhookClientConfig{ - URL: strPtr("https://example.com/webhook"), - }, - ConversionReviewVersions: []string{"v1beta1"}, - }, - PreserveUnknownFields: pointer.BoolPtr(true), - }, - Status: apiextensions.CustomResourceDefinitionStatus{ - StoredVersions: []string{"version"}, - }, - }, - old: &apiextensions.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com", ResourceVersion: "42"}, - Spec: apiextensions.CustomResourceDefinitionSpec{ - Group: "group.com", - Scope: apiextensions.ResourceScope("Cluster"), - Names: apiextensions.CustomResourceDefinitionNames{ - Plural: "plural", - Singular: "singular", - Kind: "Plural", - ListKind: "PluralList", - }, - Versions: []apiextensions.CustomResourceDefinitionVersion{ - { - Name: "version", - Served: true, - Storage: true, - }, - { - Name: "version2", - Served: true, - Storage: false, - }, - }, - Conversion: &apiextensions.CustomResourceConversion{ - Strategy: apiextensions.ConversionStrategyType("Webhook"), - WebhookClientConfig: &apiextensions.WebhookClientConfig{ - URL: strPtr("https://example.com/webhook"), - }, - ConversionReviewVersions: []string{"v1beta1"}, - }, - PreserveUnknownFields: pointer.BoolPtr(true), - }, - Status: apiextensions.CustomResourceDefinitionStatus{ - StoredVersions: []string{"version"}, - }, - }, - errors: []validationMatch{}, - }, { name: "unchanged", old: &apiextensions.CustomResourceDefinition{