From 46e622216d917b04f225dca6e0f2152039910b34 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Wed, 22 Jan 2020 17:37:14 -0500 Subject: [PATCH] Remove use of CustomResourceDefaulting feature gate --- .../pkg/apis/apiextensions/validation/BUILD | 4 -- .../validation/validation_test.go | 69 +++++-------------- .../test/integration/BUILD | 1 - .../test/integration/conversion/BUILD | 3 - .../integration/conversion/conversion_test.go | 24 +------ .../test/integration/defaulting_test.go | 7 -- .../test/integration/objectmeta_test.go | 5 -- 7 files changed, 19 insertions(+), 94 deletions(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/BUILD b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/BUILD index d7768676c53..77522255dff 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/BUILD +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/BUILD @@ -40,7 +40,6 @@ go_test( "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1:go_default_library", - "//staging/src/k8s.io/apiextensions-apiserver/pkg/features:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/apitesting/fuzzer:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", @@ -48,9 +47,6 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/json:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", - "//staging/src/k8s.io/component-base/featuregate:go_default_library", - "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library", "//vendor/k8s.io/utils/pointer:go_default_library", ], ) 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 9186e0b8378..0a6dee6c7ca 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 @@ -25,7 +25,6 @@ import ( 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" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -33,9 +32,6 @@ import ( "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apimachinery/pkg/util/json" "k8s.io/apimachinery/pkg/util/validation/field" - utilfeature "k8s.io/apiserver/pkg/util/feature" - "k8s.io/component-base/featuregate" - featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/utils/pointer" ) @@ -78,12 +74,10 @@ func TestValidateCustomResourceDefinition(t *testing.T) { }, } tests := []struct { - name string - resource *apiextensions.CustomResourceDefinition - requestGV schema.GroupVersion - errors []validationMatch - enabledFeatures []featuregate.Feature - disabledFeatures []featuregate.Feature + name string + resource *apiextensions.CustomResourceDefinition + requestGV schema.GroupVersion + errors []validationMatch }{ { name: "invalid types allowed via v1beta1", @@ -1645,8 +1639,7 @@ func TestValidateCustomResourceDefinition(t *testing.T) { StoredVersions: []string{"version"}, }, }, - enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, - requestGV: apiextensionsv1beta1.SchemeGroupVersion, + requestGV: apiextensionsv1beta1.SchemeGroupVersion, errors: []validationMatch{ forbidden("spec", "validation", "openAPIV3Schema", "properties[a]", "default"), // disallowed via v1beta1 }, @@ -1683,8 +1676,7 @@ func TestValidateCustomResourceDefinition(t *testing.T) { StoredVersions: []string{"version"}, }, }, - enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, - requestGV: apiextensionsv1.SchemeGroupVersion, + requestGV: apiextensionsv1.SchemeGroupVersion, }, { name: "x-kubernetes-int-or-string without structural", @@ -1937,7 +1929,6 @@ func TestValidateCustomResourceDefinition(t *testing.T) { required("spec", "validation", "openAPIV3Schema", "properties[a]", "type"), required("spec", "validation", "openAPIV3Schema", "type"), }, - enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, }, { name: "defaults with enabled feature gate, structural schema", @@ -1971,8 +1962,7 @@ func TestValidateCustomResourceDefinition(t *testing.T) { StoredVersions: []string{"version"}, }, }, - errors: []validationMatch{}, - enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, + errors: []validationMatch{}, }, { name: "defaults in value validation with enabled feature gate, structural schema", @@ -2029,7 +2019,6 @@ func TestValidateCustomResourceDefinition(t *testing.T) { forbidden("spec", "validation", "openAPIV3Schema", "properties[a]", "anyOf[0]", "default"), forbidden("spec", "validation", "openAPIV3Schema", "properties[a]", "oneOf[0]", "default"), }, - enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, }, { name: "invalid defaults with enabled feature gate, structural schema", @@ -2203,8 +2192,6 @@ func TestValidateCustomResourceDefinition(t *testing.T) { invalid("spec", "validation", "openAPIV3Schema", "properties[e]", "properties[preserveUnknownFields]", "default"), invalid("spec", "validation", "openAPIV3Schema", "properties[e]", "properties[nestedProperties]", "default"), }, - - enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, }, { name: "additionalProperties at resource root", @@ -2254,7 +2241,6 @@ func TestValidateCustomResourceDefinition(t *testing.T) { required("spec", "validation", "openAPIV3Schema", "properties[embedded1]", "properties"), forbidden("spec", "validation", "openAPIV3Schema", "properties[embedded2]", "additionalProperties"), }, - enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, }, { // TODO: remove in a follow-up. This blocks is here for easy review. @@ -2448,7 +2434,6 @@ func TestValidateCustomResourceDefinition(t *testing.T) { // Invalid value: wrongly typed invalid("spec", "versions[3]", "schema", "openAPIV3Schema", "properties[embedded]", "properties[metadata]", "properties[name]", "default"), }, - enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, }, { name: "default inside additionalSchema", @@ -2509,7 +2494,6 @@ func TestValidateCustomResourceDefinition(t *testing.T) { // Forbidden: must not be set inside additionalProperties applying to object metadata forbidden("spec", "validation", "openAPIV3Schema", "properties[embedded]", "properties[metadata]", "properties[annotations]", "additionalProperties", "default"), }, - enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, }, { name: "top-level metadata default", @@ -2555,7 +2539,6 @@ func TestValidateCustomResourceDefinition(t *testing.T) { errors: []validationMatch{ forbidden("spec", "validation", "openAPIV3Schema", "properties[metadata]", "default"), }, - enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, }, { name: "embedded metadata defaults", @@ -3937,7 +3920,6 @@ func TestValidateCustomResourceDefinition(t *testing.T) { required("spec", "validation", "openAPIV3Schema", "properties[spanning-defaults-with-missing-typemeta]", "default", "embedded", "apiVersion"), required("spec", "validation", "openAPIV3Schema", "properties[spanning-defaults-with-missing-typemeta]", "default", "embedded", "kind"), }, - enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, }, { name: "contradicting meta field types", @@ -4086,18 +4068,11 @@ func TestValidateCustomResourceDefinition(t *testing.T) { invalid("spec", "validation", "openAPIV3Schema", "properties[nested]", "properties[invalid]", "properties[kind]", "type"), invalid("spec", "validation", "openAPIV3Schema", "properties[nested]", "properties[invalid]", "properties[metadata]", "type"), }, - enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - for _, gate := range tc.enabledFeatures { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, gate, true)() - } - for _, gate := range tc.disabledFeatures { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, gate, false)() - } // duplicate defaulting behaviour if tc.resource.Spec.Conversion != nil && tc.resource.Spec.Conversion.Strategy == apiextensions.WebhookConverter && len(tc.resource.Spec.Conversion.ConversionReviewVersions) == 0 { @@ -4132,13 +4107,11 @@ func TestValidateCustomResourceDefinition(t *testing.T) { func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { tests := []struct { - name string - old *apiextensions.CustomResourceDefinition - resource *apiextensions.CustomResourceDefinition - requestGV schema.GroupVersion - errors []validationMatch - enabledFeatures []featuregate.Feature - disabledFeatures []featuregate.Feature + name string + old *apiextensions.CustomResourceDefinition + resource *apiextensions.CustomResourceDefinition + requestGV schema.GroupVersion + errors []validationMatch }{ { name: "invalid type updates allowed via v1beta1", @@ -5880,9 +5853,8 @@ 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}, + requestGV: apiextensionsv1beta1.SchemeGroupVersion, + errors: []validationMatch{forbidden("spec.validation.openAPIV3Schema.properties[a].default")}, }, { name: "setting defaults with enabled feature gate via v1", @@ -5963,9 +5935,8 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { StoredVersions: []string{"version"}, }, }, - requestGV: apiextensionsv1.SchemeGroupVersion, - errors: []validationMatch{}, - enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, + requestGV: apiextensionsv1.SchemeGroupVersion, + errors: []validationMatch{}, }, { name: "add default with enabled feature gate, structural schema, without pruning", @@ -6032,19 +6003,11 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { errors: []validationMatch{ invalid("spec", "preserveUnknownFields"), }, - enabledFeatures: []featuregate.Feature{features.CustomResourceDefaulting}, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - for _, gate := range tc.enabledFeatures { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, gate, true)() - } - for _, gate := range tc.disabledFeatures { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, gate, false)() - } - errs := ValidateCustomResourceDefinitionUpdate(tc.resource, tc.old, tc.requestGV) seenErrs := make([]bool, len(errs)) diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/BUILD b/staging/src/k8s.io/apiextensions-apiserver/test/integration/BUILD index e7e888d7a7f..100480ae76a 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/BUILD +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/BUILD @@ -35,7 +35,6 @@ go_test( "//staging/src/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/scheme:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/cmd/server/options:go_default_library", - "//staging/src/k8s.io/apiextensions-apiserver/pkg/features:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/test/integration/fixtures:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/test/integration/storage:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/BUILD b/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/BUILD index 75dd7342153..764ddc6bc01 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/BUILD +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/BUILD @@ -12,7 +12,6 @@ go_test( "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/cmd/server/options:go_default_library", - "//staging/src/k8s.io/apiextensions-apiserver/pkg/features:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/test/integration/fixtures:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/test/integration/storage:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", @@ -25,9 +24,7 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/util/uuid:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage/etcd3:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/dynamic:go_default_library", - "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library", "//vendor/github.com/google/go-cmp/cmp:go_default_library", ], ) diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/conversion_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/conversion_test.go index 8955921a5f7..5c18683507b 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/conversion_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/conversion_test.go @@ -38,15 +38,12 @@ import ( "k8s.io/apimachinery/pkg/util/uuid" "k8s.io/apimachinery/pkg/util/wait" etcd3watcher "k8s.io/apiserver/pkg/storage/etcd3" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/dynamic" - featuregatetesting "k8s.io/component-base/featuregate/testing" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" serveroptions "k8s.io/apiextensions-apiserver/pkg/cmd/server/options" - apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features" "k8s.io/apiextensions-apiserver/test/integration/fixtures" "k8s.io/apiextensions-apiserver/test/integration/storage" ) @@ -58,17 +55,13 @@ func checks(checkers ...Checker) []Checker { } func TestWebhookConverterWithWatchCache(t *testing.T) { - testWebhookConverter(t, false, true) + testWebhookConverter(t, true) } func TestWebhookConverterWithoutWatchCache(t *testing.T) { - testWebhookConverter(t, false, false) + testWebhookConverter(t, false) } -func TestWebhookConverterWithDefaulting(t *testing.T) { - testWebhookConverter(t, true, true) -} - -func testWebhookConverter(t *testing.T, defaulting, watchCache bool) { +func testWebhookConverter(t *testing.T, watchCache bool) { tests := []struct { group string handler http.Handler @@ -167,11 +160,6 @@ func testWebhookConverter(t *testing.T, defaulting, watchCache bool) { etcd3watcher.TestOnlySetFatalOnDecodeError(false) defer etcd3watcher.TestOnlySetFatalOnDecodeError(true) - // enable necessary features - if defaulting { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, apiextensionsfeatures.CustomResourceDefaulting, true)() - } - tearDown, config, options, err := fixtures.StartDefaultServer(t, fmt.Sprintf("--watch-cache=%v", watchCache)) if err != nil { t.Fatal(err) @@ -192,12 +180,6 @@ func testWebhookConverter(t *testing.T, defaulting, watchCache bool) { crd := multiVersionFixture.DeepCopy() - if !defaulting { - for i := range crd.Spec.Versions { - delete(crd.Spec.Versions[i].Schema.OpenAPIV3Schema.Properties, "defaults") - } - } - RESTOptionsGetter := serveroptions.NewCRDRESTOptionsGetter(*options.RecommendedOptions.Etcd) restOptions, err := RESTOptionsGetter.GetRESTOptions(schema.GroupResource{Group: crd.Spec.Group, Resource: crd.Spec.Names.Plural}) if err != nil { diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/defaulting_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/defaulting_test.go index 1c9688f47c3..3505b4097bd 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/defaulting_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/defaulting_test.go @@ -32,14 +32,11 @@ import ( "k8s.io/apimachinery/pkg/util/json" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apimachinery/pkg/watch" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/dynamic" - utilfeaturetesting "k8s.io/component-base/featuregate/testing" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" serveroptions "k8s.io/apiextensions-apiserver/pkg/cmd/server/options" - "k8s.io/apiextensions-apiserver/pkg/features" "k8s.io/apiextensions-apiserver/test/integration/fixtures" "k8s.io/apiextensions-apiserver/test/integration/storage" ) @@ -167,8 +164,6 @@ func TestCustomResourceDefaultingWithoutWatchCache(t *testing.T) { } func testDefaulting(t *testing.T, watchCache bool) { - defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CustomResourceDefaulting, true)() - tearDownFn, apiExtensionClient, dynamicClient, err := fixtures.StartDefaultServerWithClients(t, fmt.Sprintf("--watch-cache=%v", watchCache)) if err != nil { t.Fatal(err) @@ -572,8 +567,6 @@ metadata: ` func TestCustomResourceDefaultingOfMetaFields(t *testing.T) { - defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CustomResourceDefaulting, true)() - tearDown, config, options, err := fixtures.StartDefaultServer(t) if err != nil { t.Fatal(err) diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/objectmeta_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/objectmeta_test.go index 6a75f60b1f1..fa1fb0828b8 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/objectmeta_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/objectmeta_test.go @@ -32,7 +32,6 @@ import ( apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" serveroptions "k8s.io/apiextensions-apiserver/pkg/cmd/server/options" - "k8s.io/apiextensions-apiserver/pkg/features" "k8s.io/apiextensions-apiserver/test/integration/fixtures" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -41,9 +40,7 @@ import ( "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/json" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/dynamic" - featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/utils/pointer" ) @@ -431,8 +428,6 @@ invalidDefaults: {} ) func TestEmbeddedResources(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CustomResourceDefaulting, true)() - tearDownFn, apiExtensionClient, dynamicClient, err := fixtures.StartDefaultServerWithClients(t) if err != nil { t.Fatal(err)