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 4f16ccce878..4f3c27e66cc 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 @@ -196,18 +196,8 @@ func ValidateCustomResourceDefinitionSpec(spec *apiextensions.CustomResourceDefi } allErrs = append(allErrs, ValidateCustomResourceDefinitionNames(&spec.Names, fldPath.Child("names"))...) - - if utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceValidation) { - allErrs = append(allErrs, ValidateCustomResourceDefinitionValidation(spec.Validation, hasAnyStatusEnabled(spec), fldPath.Child("validation"))...) - } else if spec.Validation != nil { - allErrs = append(allErrs, field.Forbidden(fldPath.Child("validation"), "disabled by feature-gate CustomResourceValidation")) - } - - if utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceSubresources) { - allErrs = append(allErrs, ValidateCustomResourceDefinitionSubresources(spec.Subresources, fldPath.Child("subresources"))...) - } else if spec.Subresources != nil { - allErrs = append(allErrs, field.Forbidden(fldPath.Child("subresources"), "disabled by feature-gate CustomResourceSubresources")) - } + allErrs = append(allErrs, ValidateCustomResourceDefinitionValidation(spec.Validation, hasAnyStatusEnabled(spec), fldPath.Child("validation"))...) + allErrs = append(allErrs, ValidateCustomResourceDefinitionSubresources(spec.Subresources, fldPath.Child("subresources"))...) for i := range spec.AdditionalPrinterColumns { if errs := ValidateCustomResourceColumnDefinition(&spec.AdditionalPrinterColumns[i], fldPath.Child("additionalPrinterColumns").Index(i)); len(errs) > 0 { @@ -481,7 +471,7 @@ func ValidateCustomResourceDefinitionValidation(customResourceValidation *apiext if schema := customResourceValidation.OpenAPIV3Schema; schema != nil { // if the status subresource is enabled, only certain fields are allowed inside the root schema. // these fields are chosen such that, if status is extracted as properties["status"], it's validation is not lost. - if utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceSubresources) && statusSubresourceEnabled { + if statusSubresourceEnabled { v := reflect.ValueOf(schema).Elem() for i := 0; i < v.NumField(); i++ { // skip zero values diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/BUILD b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/BUILD index 153b889eacf..c51e800e1b4 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/BUILD +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/BUILD @@ -1,9 +1,6 @@ package(default_visibility = ["//visibility:public"]) -load( - "@io_bazel_rules_go//go:def.bzl", - "go_library", -) +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", @@ -47,3 +44,16 @@ filegroup( srcs = [":package-srcs"], tags = ["automanaged"], ) + +go_test( + name = "go_default_test", + srcs = ["strategy_test.go"], + embed = [":go_default_library"], + deps = [ + "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library", + "//staging/src/k8s.io/apiextensions-apiserver/pkg/features:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature/testing:go_default_library", + ], +) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go index ee3d8eaf042..0fbad281757 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go @@ -54,32 +54,7 @@ func (strategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { crd.Status = apiextensions.CustomResourceDefinitionStatus{} crd.Generation = 1 - // if the feature gate is disabled, drop the feature. - if !utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceValidation) { - crd.Spec.Validation = nil - for i := range crd.Spec.Versions { - crd.Spec.Versions[i].Schema = nil - } - } - if !utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceSubresources) { - crd.Spec.Subresources = nil - for i := range crd.Spec.Versions { - crd.Spec.Versions[i].Subresources = nil - } - } - // On CREATE, if the CustomResourceWebhookConversion feature gate is off, we auto-clear - // the per-version fields. This is to be consistent with the other built-in types, as the - // apiserver drops unknown fields. - if !utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceWebhookConversion) { - for i := range crd.Spec.Versions { - crd.Spec.Versions[i].Schema = nil - crd.Spec.Versions[i].Subresources = nil - crd.Spec.Versions[i].AdditionalPrinterColumns = nil - } - } - if !utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceWebhookConversion) && crd.Spec.Conversion != nil { - crd.Spec.Conversion.WebhookClientConfig = nil - } + dropDisabledFields(&crd.Spec, nil) for _, v := range crd.Spec.Versions { if v.Storage { @@ -109,45 +84,7 @@ func (strategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { newCRD.Generation = oldCRD.Generation + 1 } - if !utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceValidation) { - newCRD.Spec.Validation = nil - oldCRD.Spec.Validation = nil - for i := range newCRD.Spec.Versions { - newCRD.Spec.Versions[i].Schema = nil - } - for i := range oldCRD.Spec.Versions { - oldCRD.Spec.Versions[i].Schema = nil - } - } - if !utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceSubresources) { - newCRD.Spec.Subresources = nil - oldCRD.Spec.Subresources = nil - for i := range newCRD.Spec.Versions { - newCRD.Spec.Versions[i].Subresources = nil - } - for i := range oldCRD.Spec.Versions { - oldCRD.Spec.Versions[i].Subresources = nil - } - } - - // On UPDATE, if the CustomResourceWebhookConversion feature gate is off, we auto-clear - // the per-version fields if the old CRD doesn't use per-version fields already. - // This is to be consistent with the other built-in types, as the apiserver drops unknown - // fields. If the old CRD already uses per-version fields, the CRD is allowed to continue - // use per-version fields. - if !utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceWebhookConversion) && - !hasPerVersionField(oldCRD.Spec.Versions) { - for i := range newCRD.Spec.Versions { - newCRD.Spec.Versions[i].Schema = nil - newCRD.Spec.Versions[i].Subresources = nil - newCRD.Spec.Versions[i].AdditionalPrinterColumns = nil - } - } - if !utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceWebhookConversion) && newCRD.Spec.Conversion != nil { - if oldCRD.Spec.Conversion == nil || newCRD.Spec.Conversion.WebhookClientConfig == nil { - newCRD.Spec.Conversion.WebhookClientConfig = nil - } - } + dropDisabledFields(&newCRD.Spec, &oldCRD.Spec) for _, v := range newCRD.Spec.Versions { if v.Storage { @@ -159,16 +96,6 @@ func (strategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { } } -// hasPerVersionField returns true if a CRD uses per-version schema/subresources/columns fields. -func hasPerVersionField(versions []apiextensions.CustomResourceDefinitionVersion) bool { - for _, v := range versions { - if v.Schema != nil || v.Subresources != nil || len(v.AdditionalPrinterColumns) > 0 { - return true - } - } - return false -} - // Validate validates a new CustomResourceDefinition. func (strategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { return validation.ValidateCustomResourceDefinition(obj.(*apiextensions.CustomResourceDefinition)) @@ -259,3 +186,102 @@ func MatchCustomResourceDefinition(label labels.Selector, field fields.Selector) func CustomResourceDefinitionToSelectableFields(obj *apiextensions.CustomResourceDefinition) fields.Set { return generic.ObjectMetaFieldsSet(&obj.ObjectMeta, true) } + +func dropDisabledFields(crdSpec, oldCrdSpec *apiextensions.CustomResourceDefinitionSpec) { + // if the feature gate is disabled, drop the feature. + if !utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceValidation) && + !validationInUse(oldCrdSpec) { + crdSpec.Validation = nil + for i := range crdSpec.Versions { + crdSpec.Versions[i].Schema = nil + } + } + if !utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceSubresources) && + !subresourceInUse(oldCrdSpec) { + crdSpec.Subresources = nil + for i := range crdSpec.Versions { + crdSpec.Versions[i].Subresources = nil + } + } + + // 1. On CREATE (in which case the old CRD spec is nil), if the CustomResourceWebhookConversion feature gate is off, we auto-clear + // the per-version fields. This is to be consistent with the other built-in types, as the + // apiserver drops unknown fields. + // 2. On UPDATE, if the CustomResourceWebhookConversion feature gate is off, we auto-clear + // the per-version fields if the old CRD doesn't use per-version fields already. + // This is to be consistent with the other built-in types, as the apiserver drops unknown + // fields. If the old CRD already uses per-version fields, the CRD is allowed to continue + // use per-version fields. + if !utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceWebhookConversion) && + !hasPerVersionField(oldCrdSpec) { + for i := range crdSpec.Versions { + crdSpec.Versions[i].Schema = nil + crdSpec.Versions[i].Subresources = nil + crdSpec.Versions[i].AdditionalPrinterColumns = nil + } + } + + if !utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceWebhookConversion) && + !conversionWebhookInUse(oldCrdSpec) { + if crdSpec.Conversion != nil { + crdSpec.Conversion.WebhookClientConfig = nil + } + } + +} + +func validationInUse(crdSpec *apiextensions.CustomResourceDefinitionSpec) bool { + if crdSpec == nil { + return false + } + if crdSpec.Validation != nil { + return true + } + + for i := range crdSpec.Versions { + if crdSpec.Versions[i].Schema != nil { + return true + } + } + return false +} + +func subresourceInUse(crdSpec *apiextensions.CustomResourceDefinitionSpec) bool { + if crdSpec == nil { + return false + } + if crdSpec.Subresources != nil { + return true + } + + for i := range crdSpec.Versions { + if crdSpec.Versions[i].Subresources != nil { + return true + } + } + return false +} + +// hasPerVersionField returns true if a CRD uses per-version schema/subresources/columns fields. +//func hasPerVersionField(versions []apiextensions.CustomResourceDefinitionVersion) bool { +func hasPerVersionField(crdSpec *apiextensions.CustomResourceDefinitionSpec) bool { + if crdSpec == nil { + return false + } + for _, v := range crdSpec.Versions { + if v.Schema != nil || v.Subresources != nil || len(v.AdditionalPrinterColumns) > 0 { + return true + } + } + return false +} + +func conversionWebhookInUse(crdSpec *apiextensions.CustomResourceDefinitionSpec) bool { + if crdSpec == nil { + return false + } + if crdSpec.Conversion == nil { + return false + } + return crdSpec.Conversion.WebhookClientConfig != nil +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy_test.go new file mode 100644 index 00000000000..c67c94721e9 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy_test.go @@ -0,0 +1,472 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package customresourcedefinition + +import ( + "fmt" + "reflect" + "testing" + + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" + apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features" + "k8s.io/apimachinery/pkg/util/diff" + utilfeature "k8s.io/apiserver/pkg/util/feature" + utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing" +) + +func TestDropDisableFieldsCustomResourceDefinition(t *testing.T) { + t.Log("testing unversioned validation..") + for _, validationEnabled := range []bool{true, false} { + crdWithUnversionedValidation := func() *apiextensions.CustomResourceDefinition { + // crd with non-versioned validation + return &apiextensions.CustomResourceDefinition{ + Spec: apiextensions.CustomResourceDefinitionSpec{ + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{}, + }, + }, + } + } + crdWithoutUnversionedValidation := func() *apiextensions.CustomResourceDefinition { + // crd with non-versioned validation + return &apiextensions.CustomResourceDefinition{ + Spec: apiextensions.CustomResourceDefinitionSpec{}, + } + } + crdInfos := []struct { + name string + hasCRValidation bool + crd func() *apiextensions.CustomResourceDefinition + }{ + { + name: "has unversioned validation", + hasCRValidation: true, + crd: crdWithUnversionedValidation, + }, + { + name: "doesn't have unversioned validation", + hasCRValidation: false, + crd: crdWithoutUnversionedValidation, + }, + { + name: "nil", + hasCRValidation: false, + crd: func() *apiextensions.CustomResourceDefinition { return nil }, + }, + } + for _, oldCRDInfo := range crdInfos { + for _, newCRDInfo := range crdInfos { + oldCRDHasValidation, oldCRD := oldCRDInfo.hasCRValidation, oldCRDInfo.crd() + newCRDHasValidation, newCRD := newCRDInfo.hasCRValidation, newCRDInfo.crd() + if newCRD == nil { + continue + } + t.Run(fmt.Sprintf("validation feature enabled=%v, old CRD %v, new CRD %v", validationEnabled, oldCRDInfo.name, newCRDInfo.name), + func(t *testing.T) { + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, apiextensionsfeatures.CustomResourceValidation, validationEnabled)() + var oldCRDSpec *apiextensions.CustomResourceDefinitionSpec + if oldCRD != nil { + oldCRDSpec = &oldCRD.Spec + } + dropDisabledFields(&newCRD.Spec, oldCRDSpec) + // old CRD should never be changed + if !reflect.DeepEqual(oldCRD, oldCRDInfo.crd()) { + t.Errorf("old crd changed: %v", diff.ObjectReflectDiff(oldCRD, oldCRDInfo.crd())) + } + switch { + case validationEnabled || oldCRDHasValidation: + if !reflect.DeepEqual(newCRD, newCRDInfo.crd()) { + t.Errorf("new crd changed: %v", diff.ObjectReflectDiff(newCRD, newCRDInfo.crd())) + } + case newCRDHasValidation: + if reflect.DeepEqual(newCRD, newCRDInfo.crd()) { + t.Errorf("new crd was not changed") + } + if !reflect.DeepEqual(newCRD, crdWithoutUnversionedValidation()) { + t.Errorf("new crd had unversioned validation: %v", diff.ObjectReflectDiff(newCRD, crdWithoutUnversionedValidation())) + } + default: + if !reflect.DeepEqual(newCRD, newCRDInfo.crd()) { + t.Errorf("new crd changed: %v", diff.ObjectReflectDiff(newCRD, newCRDInfo.crd())) + } + } + }, + ) + } + } + } + + t.Log("testing unversioned subresources...") + for _, validationEnabled := range []bool{true, false} { + crdWithUnversionedSubresources := func() *apiextensions.CustomResourceDefinition { + // crd with unversioned subresources + return &apiextensions.CustomResourceDefinition{ + Spec: apiextensions.CustomResourceDefinitionSpec{ + Subresources: &apiextensions.CustomResourceSubresources{}, + }, + } + } + crdWithoutUnversionedSubresources := func() *apiextensions.CustomResourceDefinition { + // crd without unversioned subresources + return &apiextensions.CustomResourceDefinition{ + Spec: apiextensions.CustomResourceDefinitionSpec{}, + } + } + crdInfos := []struct { + name string + hasCRSubresources bool + crd func() *apiextensions.CustomResourceDefinition + }{ + { + name: "has unversioned subresources", + hasCRSubresources: true, + crd: crdWithUnversionedSubresources, + }, + { + name: "doesn't have unversioned subresources", + hasCRSubresources: false, + crd: crdWithoutUnversionedSubresources, + }, + { + name: "nil", + hasCRSubresources: false, + crd: func() *apiextensions.CustomResourceDefinition { return nil }, + }, + } + for _, oldCRDInfo := range crdInfos { + for _, newCRDInfo := range crdInfos { + oldCRDHasSubresources, oldCRD := oldCRDInfo.hasCRSubresources, oldCRDInfo.crd() + newCRDHasSubresources, newCRD := newCRDInfo.hasCRSubresources, newCRDInfo.crd() + if newCRD == nil { + continue + } + t.Run(fmt.Sprintf("subresources feature enabled=%v, old CRD %v, new CRD %v", validationEnabled, oldCRDInfo.name, newCRDInfo.name), + func(t *testing.T) { + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, apiextensionsfeatures.CustomResourceSubresources, validationEnabled)() + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, apiextensionsfeatures.CustomResourceWebhookConversion, true)() + var oldCRDSpec *apiextensions.CustomResourceDefinitionSpec + if oldCRD != nil { + oldCRDSpec = &oldCRD.Spec + } + dropDisabledFields(&newCRD.Spec, oldCRDSpec) + // old CRD should never be changed + if !reflect.DeepEqual(oldCRD, oldCRDInfo.crd()) { + t.Errorf("old crd changed: %v", diff.ObjectReflectDiff(oldCRD, oldCRDInfo.crd())) + } + switch { + case validationEnabled || oldCRDHasSubresources: + if !reflect.DeepEqual(newCRD, newCRDInfo.crd()) { + t.Errorf("new crd changed: %v", diff.ObjectReflectDiff(newCRD, newCRDInfo.crd())) + } + case newCRDHasSubresources: + if reflect.DeepEqual(newCRD, newCRDInfo.crd()) { + t.Errorf("new crd was not changed") + } + if !reflect.DeepEqual(newCRD, crdWithoutUnversionedSubresources()) { + t.Errorf("new crd had unversioned subresources: %v", diff.ObjectReflectDiff(newCRD, crdWithoutUnversionedSubresources())) + } + default: + if !reflect.DeepEqual(newCRD, newCRDInfo.crd()) { + t.Errorf("new crd changed: %v", diff.ObjectReflectDiff(newCRD, newCRDInfo.crd())) + } + } + }, + ) + } + } + } + + t.Log("testing versioned validation..") + for _, conversionEnabled := range []bool{true, false} { + for _, validationEnabled := range []bool{true, false} { + crdWithVersionedValidation := func() *apiextensions.CustomResourceDefinition { + // crd with versioned validation + return &apiextensions.CustomResourceDefinition{ + Spec: apiextensions.CustomResourceDefinitionSpec{ + Versions: []apiextensions.CustomResourceDefinitionVersion{ + { + Name: "v1", + Schema: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{}, + }, + }, + }, + }, + } + } + crdWithoutVersionedValidation := func() *apiextensions.CustomResourceDefinition { + // crd with versioned validation + return &apiextensions.CustomResourceDefinition{ + Spec: apiextensions.CustomResourceDefinitionSpec{ + Versions: []apiextensions.CustomResourceDefinitionVersion{ + { + Name: "v1", + }, + }, + }, + } + } + crdInfos := []struct { + name string + hasCRValidation bool + crd func() *apiextensions.CustomResourceDefinition + }{ + { + name: "has versioned validation", + hasCRValidation: true, + crd: crdWithVersionedValidation, + }, + { + name: "doesn't have versioned validation", + hasCRValidation: false, + crd: crdWithoutVersionedValidation, + }, + { + name: "nil", + hasCRValidation: false, + crd: func() *apiextensions.CustomResourceDefinition { return nil }, + }, + } + for _, oldCRDInfo := range crdInfos { + for _, newCRDInfo := range crdInfos { + oldCRDHasValidation, oldCRD := oldCRDInfo.hasCRValidation, oldCRDInfo.crd() + newCRDHasValidation, newCRD := newCRDInfo.hasCRValidation, newCRDInfo.crd() + if newCRD == nil { + continue + } + t.Run(fmt.Sprintf("validation feature enabled=%v, old CRD %v, new CRD %v", validationEnabled, oldCRDInfo.name, newCRDInfo.name), + func(t *testing.T) { + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, apiextensionsfeatures.CustomResourceValidation, validationEnabled)() + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, apiextensionsfeatures.CustomResourceWebhookConversion, conversionEnabled)() + var oldCRDSpec *apiextensions.CustomResourceDefinitionSpec + if oldCRD != nil { + oldCRDSpec = &oldCRD.Spec + } + dropDisabledFields(&newCRD.Spec, oldCRDSpec) + // old CRD should never be changed + if !reflect.DeepEqual(oldCRD, oldCRDInfo.crd()) { + t.Errorf("old crd changed: %v", diff.ObjectReflectDiff(oldCRD, oldCRDInfo.crd())) + } + switch { + case (conversionEnabled && validationEnabled) || oldCRDHasValidation: + if !reflect.DeepEqual(newCRD, newCRDInfo.crd()) { + t.Errorf("new crd changed: %v", diff.ObjectReflectDiff(newCRD, newCRDInfo.crd())) + } + case !conversionEnabled && !oldCRDHasValidation: + if !reflect.DeepEqual(newCRD, crdWithoutVersionedValidation()) { + t.Errorf("new crd was not changed") + } + case newCRDHasValidation: + if reflect.DeepEqual(newCRD, newCRDInfo.crd()) { + t.Errorf("new crd was not changed") + } + if !reflect.DeepEqual(newCRD, crdWithoutVersionedValidation()) { + t.Errorf("new crd had unversioned validation: %v", diff.ObjectReflectDiff(newCRD, crdWithoutVersionedValidation())) + } + default: + if !reflect.DeepEqual(newCRD, newCRDInfo.crd()) { + t.Errorf("new crd changed: %v", diff.ObjectReflectDiff(newCRD, newCRDInfo.crd())) + } + } + }, + ) + } + } + } + } + + t.Log("testing versioned subresources w/ conversion enabled..") + for _, conversionEnabled := range []bool{true, false} { + for _, validationEnabled := range []bool{true, false} { + crdWithVersionedSubresources := func() *apiextensions.CustomResourceDefinition { + // crd with versioned subresources + return &apiextensions.CustomResourceDefinition{ + Spec: apiextensions.CustomResourceDefinitionSpec{ + Versions: []apiextensions.CustomResourceDefinitionVersion{ + { + Name: "v1", + Subresources: &apiextensions.CustomResourceSubresources{}, + }, + }, + }, + } + } + crdWithoutVersionedSubresources := func() *apiextensions.CustomResourceDefinition { + // crd without versioned subresources + return &apiextensions.CustomResourceDefinition{ + Spec: apiextensions.CustomResourceDefinitionSpec{ + Versions: []apiextensions.CustomResourceDefinitionVersion{ + { + Name: "v1", + }, + }, + }, + } + } + crdInfos := []struct { + name string + hasCRSubresources bool + crd func() *apiextensions.CustomResourceDefinition + }{ + { + name: "has versioned subresources", + hasCRSubresources: true, + crd: crdWithVersionedSubresources, + }, + { + name: "doesn't have versioned subresources", + hasCRSubresources: false, + crd: crdWithoutVersionedSubresources, + }, + { + name: "nil", + hasCRSubresources: false, + crd: func() *apiextensions.CustomResourceDefinition { return nil }, + }, + } + for _, oldCRDInfo := range crdInfos { + for _, newCRDInfo := range crdInfos { + oldCRDHasSubresources, oldCRD := oldCRDInfo.hasCRSubresources, oldCRDInfo.crd() + newCRDHasSubresources, newCRD := newCRDInfo.hasCRSubresources, newCRDInfo.crd() + if newCRD == nil { + continue + } + t.Run(fmt.Sprintf("subresources feature enabled=%v, old CRD %v, new CRD %v", validationEnabled, oldCRDInfo.name, newCRDInfo.name), + func(t *testing.T) { + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, apiextensionsfeatures.CustomResourceSubresources, validationEnabled)() + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, apiextensionsfeatures.CustomResourceWebhookConversion, conversionEnabled)() + var oldCRDSpec *apiextensions.CustomResourceDefinitionSpec + if oldCRD != nil { + oldCRDSpec = &oldCRD.Spec + } + dropDisabledFields(&newCRD.Spec, oldCRDSpec) + // old CRD should never be changed + if !reflect.DeepEqual(oldCRD, oldCRDInfo.crd()) { + t.Errorf("old crd changed: %v", diff.ObjectReflectDiff(oldCRD, oldCRDInfo.crd())) + } + switch { + case (conversionEnabled && validationEnabled) || oldCRDHasSubresources: + if !reflect.DeepEqual(newCRD, newCRDInfo.crd()) { + t.Errorf("new crd changed: %v", diff.ObjectReflectDiff(newCRD, newCRDInfo.crd())) + } + case !conversionEnabled && !oldCRDHasSubresources: + if !reflect.DeepEqual(newCRD, crdWithoutVersionedSubresources()) { + t.Errorf("new crd was not changed") + } + case newCRDHasSubresources: + if reflect.DeepEqual(newCRD, newCRDInfo.crd()) { + t.Errorf("new crd was not changed") + } + if !reflect.DeepEqual(newCRD, crdWithoutVersionedSubresources()) { + t.Errorf("new crd had versioned subresources: %v", diff.ObjectReflectDiff(newCRD, crdWithoutVersionedSubresources())) + } + default: + if !reflect.DeepEqual(newCRD, newCRDInfo.crd()) { + t.Errorf("new crd changed: %v", diff.ObjectReflectDiff(newCRD, newCRDInfo.crd())) + } + } + }, + ) + } + } + } + } + + t.Log("testing conversion webhook..") + for _, validationEnabled := range []bool{true, false} { + crdWithUnversionedConversionWebhook := func() *apiextensions.CustomResourceDefinition { + // crd with conversion webhook + return &apiextensions.CustomResourceDefinition{ + Spec: apiextensions.CustomResourceDefinitionSpec{ + Conversion: &apiextensions.CustomResourceConversion{ + WebhookClientConfig: &apiextensions.WebhookClientConfig{}, + }, + }, + } + } + crdWithoutUnversionedConversionWebhook := func() *apiextensions.CustomResourceDefinition { + // crd with conversion webhook + return &apiextensions.CustomResourceDefinition{ + Spec: apiextensions.CustomResourceDefinitionSpec{ + Conversion: &apiextensions.CustomResourceConversion{}, + }, + } + } + crdInfos := []struct { + name string + hasCRConversionWebhook bool + crd func() *apiextensions.CustomResourceDefinition + }{ + { + name: "has conversion webhook", + hasCRConversionWebhook: true, + crd: crdWithUnversionedConversionWebhook, + }, + { + name: "doesn't have conversion webhook", + hasCRConversionWebhook: false, + crd: crdWithoutUnversionedConversionWebhook, + }, + { + name: "nil", + hasCRConversionWebhook: false, + crd: func() *apiextensions.CustomResourceDefinition { return nil }, + }, + } + for _, oldCRDInfo := range crdInfos { + for _, newCRDInfo := range crdInfos { + oldCRDHasConversionWebhook, oldCRD := oldCRDInfo.hasCRConversionWebhook, oldCRDInfo.crd() + newCRDHasConversionWebhook, newCRD := newCRDInfo.hasCRConversionWebhook, newCRDInfo.crd() + if newCRD == nil { + continue + } + t.Run(fmt.Sprintf("subresources feature enabled=%v, old CRD %v, new CRD %v", validationEnabled, oldCRDInfo.name, newCRDInfo.name), + func(t *testing.T) { + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, apiextensionsfeatures.CustomResourceWebhookConversion, validationEnabled)() + var oldCRDSpec *apiextensions.CustomResourceDefinitionSpec + if oldCRD != nil { + oldCRDSpec = &oldCRD.Spec + } + dropDisabledFields(&newCRD.Spec, oldCRDSpec) + // old CRD should never be changed + if !reflect.DeepEqual(oldCRD, oldCRDInfo.crd()) { + t.Errorf("old crd changed: %v", diff.ObjectReflectDiff(oldCRD, oldCRDInfo.crd())) + } + switch { + case validationEnabled || oldCRDHasConversionWebhook: + if !reflect.DeepEqual(newCRD, newCRDInfo.crd()) { + t.Errorf("new crd changed: %v", diff.ObjectReflectDiff(newCRD, newCRDInfo.crd())) + } + case newCRDHasConversionWebhook: + if reflect.DeepEqual(newCRD, newCRDInfo.crd()) { + t.Errorf("new crd was not changed") + } + if !reflect.DeepEqual(newCRD, crdWithoutUnversionedConversionWebhook()) { + t.Errorf("new crd had webhook conversion: %v", diff.ObjectReflectDiff(newCRD, crdWithoutUnversionedConversionWebhook())) + } + default: + if !reflect.DeepEqual(newCRD, newCRDInfo.crd()) { + t.Errorf("new crd changed: %v", diff.ObjectReflectDiff(newCRD, newCRDInfo.crd())) + } + } + }, + ) + } + } + } + +}