drop crd api fields

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"))
-	}
 
 	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

staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/BUILD

@@ -1,9 +1,6 @@
 package(default_visibility = ["//visibility:public"])
 
-load(
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
-    "@io_bazel_rules_go//go:def.bzl",
-    "go_library",
-)
 
 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",
+    ],
+)

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.
+	dropDisabledFields(&crd.Spec, nil)
-	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
-	}
 
 	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) {
+	dropDisabledFields(&newCRD.Spec, &oldCRD.Spec)
-		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
-		}
-	}
 
 	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
+}

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()))
+							}
+						}
+					},
+				)
+			}
+		}
+	}
+
+}