From a1bfb76458a219577884d312ceb93eb1bb3f7e39 Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Fri, 26 Aug 2022 11:33:56 +0200 Subject: [PATCH] remove DryRun feature gate checks --- pkg/features/kube_features.go | 2 +- .../apiserver/pkg/endpoints/apiserver_test.go | 61 ------------------- .../pkg/endpoints/handlers/create.go | 7 --- .../pkg/endpoints/handlers/delete.go | 12 ---- .../apiserver/pkg/endpoints/handlers/patch.go | 7 --- .../pkg/endpoints/handlers/update.go | 7 --- .../apiserver/pkg/features/kube_features.go | 2 +- 7 files changed, 2 insertions(+), 96 deletions(-) diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index d9da0304dd4ea..38bb3868d4602 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -1125,7 +1125,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS genericfeatures.CustomResourceValidationExpressions: {Default: true, PreRelease: featuregate.Beta}, - genericfeatures.DryRun: {Default: true, PreRelease: featuregate.GA}, + genericfeatures.DryRun: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.28 genericfeatures.OpenAPIEnums: {Default: true, PreRelease: featuregate.Beta}, diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go index c744e876e7872..7d37fa558ce36 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go @@ -4001,16 +4001,6 @@ func runRequest(t testing.TB, path, verb string, data []byte, contentType string return response } -// encodeOrFatal is used by TestDryRun to parse an object and stop right -// away if it fails. -func encodeOrFatal(t *testing.T, obj runtime.Object) []byte { - data, err := runtime.Encode(testCodec, obj) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - return data -} - type SimpleRESTStorageWithDeleteCollection struct { SimpleRESTStorage } @@ -4272,57 +4262,6 @@ other: bar`) } } -func TestDryRunDisabled(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DryRun, false)() - - tests := []struct { - path string - verb string - data []byte - contentType string - }{ - {path: "/namespaces/default/simples", verb: "POST", data: encodeOrFatal(t, &genericapitesting.Simple{Other: "bar"})}, - {path: "/namespaces/default/simples/id", verb: "PUT", data: encodeOrFatal(t, &genericapitesting.Simple{ObjectMeta: metav1.ObjectMeta{Name: "id"}, Other: "bar"})}, - {path: "/namespaces/default/simples/id", verb: "PATCH", data: []byte(`{"labels":{"foo":"bar"}}`), contentType: "application/merge-patch+json; charset=UTF-8"}, - {path: "/namespaces/default/simples/id", verb: "DELETE"}, - {path: "/namespaces/default/simples", verb: "DELETE"}, - {path: "/namespaces/default/simples/id/subsimple", verb: "DELETE"}, - } - - server := httptest.NewServer(handle(map[string]rest.Storage{ - "simples": &SimpleRESTStorageWithDeleteCollection{ - SimpleRESTStorage{ - item: genericapitesting.Simple{ - ObjectMeta: metav1.ObjectMeta{ - Name: "id", - Namespace: "", - UID: "uid", - }, - Other: "bar", - }, - }, - }, - "simples/subsimple": &SimpleXGSubresourceRESTStorage{ - item: genericapitesting.SimpleXGSubresource{ - SubresourceInfo: "foo", - }, - itemGVK: testGroup2Version.WithKind("SimpleXGSubresource"), - }, - })) - defer server.Close() - for _, test := range tests { - baseURL := server.URL + "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version - response := runRequest(t, baseURL+test.path, test.verb, test.data, test.contentType) - if response.StatusCode == http.StatusBadRequest { - t.Fatalf("unexpected BadRequest: %#v", response) - } - response = runRequest(t, baseURL+test.path+"?dryRun", test.verb, test.data, test.contentType) - if response.StatusCode != http.StatusBadRequest { - t.Fatalf("unexpected non BadRequest: %#v", response) - } - } -} - type SimpleXGSubresourceRESTStorage struct { item genericapitesting.SimpleXGSubresource itemGVK schema.GroupVersionKind diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go index 1f3ce809448d0..d38cbb0c3cdc5 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go @@ -39,10 +39,8 @@ import ( "k8s.io/apiserver/pkg/endpoints/handlers/finisher" "k8s.io/apiserver/pkg/endpoints/handlers/negotiation" "k8s.io/apiserver/pkg/endpoints/request" - "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/registry/rest" "k8s.io/apiserver/pkg/util/dryrun" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/klog/v2" utiltrace "k8s.io/utils/trace" ) @@ -55,11 +53,6 @@ func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Int trace := utiltrace.New("Create", traceFields(req)...) defer trace.LogIfLong(500 * time.Millisecond) - if isDryRun(req.URL) && !utilfeature.DefaultFeatureGate.Enabled(features.DryRun) { - scope.err(errors.NewBadRequest("the dryRun feature is disabled"), w, req) - return - } - namespace, name, err := scope.Namer.Name(req) if err != nil { if includeName { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go index a7712f115fa6f..dab250b082793 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go @@ -35,10 +35,8 @@ import ( "k8s.io/apiserver/pkg/endpoints/handlers/finisher" "k8s.io/apiserver/pkg/endpoints/handlers/negotiation" "k8s.io/apiserver/pkg/endpoints/request" - "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/registry/rest" "k8s.io/apiserver/pkg/util/dryrun" - utilfeature "k8s.io/apiserver/pkg/util/feature" utiltrace "k8s.io/utils/trace" ) @@ -50,11 +48,6 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope *RequestSc trace := utiltrace.New("Delete", traceFields(req)...) defer trace.LogIfLong(500 * time.Millisecond) - if isDryRun(req.URL) && !utilfeature.DefaultFeatureGate.Enabled(features.DryRun) { - scope.err(errors.NewBadRequest("the dryRun feature is disabled"), w, req) - return - } - namespace, name, err := scope.Namer.Name(req) if err != nil { scope.err(err, w, req) @@ -172,11 +165,6 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope *RequestSc trace := utiltrace.New("Delete", traceFields(req)...) defer trace.LogIfLong(500 * time.Millisecond) - if isDryRun(req.URL) && !utilfeature.DefaultFeatureGate.Enabled(features.DryRun) { - scope.err(errors.NewBadRequest("the dryRun feature is disabled"), w, req) - return - } - namespace, err := scope.Namer.Namespace(req) if err != nil { scope.err(err, w, req) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go index a0a8fb6ca7765..98909533edbcf 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go @@ -47,10 +47,8 @@ import ( "k8s.io/apiserver/pkg/endpoints/handlers/finisher" "k8s.io/apiserver/pkg/endpoints/handlers/negotiation" "k8s.io/apiserver/pkg/endpoints/request" - "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/registry/rest" "k8s.io/apiserver/pkg/util/dryrun" - utilfeature "k8s.io/apiserver/pkg/util/feature" utiltrace "k8s.io/utils/trace" ) @@ -66,11 +64,6 @@ func PatchResource(r rest.Patcher, scope *RequestScope, admit admission.Interfac trace := utiltrace.New("Patch", traceFields(req)...) defer trace.LogIfLong(500 * time.Millisecond) - if isDryRun(req.URL) && !utilfeature.DefaultFeatureGate.Enabled(features.DryRun) { - scope.err(errors.NewBadRequest("the dryRun feature is disabled"), w, req) - return - } - // Do this first, otherwise name extraction can fail for unrecognized content types // TODO: handle this in negotiation contentType := req.Header.Get("Content-Type") diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go index cb0ba5d7de8df..afc95947fe9b8 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go @@ -37,10 +37,8 @@ import ( "k8s.io/apiserver/pkg/endpoints/handlers/finisher" "k8s.io/apiserver/pkg/endpoints/handlers/negotiation" "k8s.io/apiserver/pkg/endpoints/request" - "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/registry/rest" "k8s.io/apiserver/pkg/util/dryrun" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/klog/v2" utiltrace "k8s.io/utils/trace" ) @@ -52,11 +50,6 @@ func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interfa trace := utiltrace.New("Update", traceFields(req)...) defer trace.LogIfLong(500 * time.Millisecond) - if isDryRun(req.URL) && !utilfeature.DefaultFeatureGate.Enabled(features.DryRun) { - scope.err(errors.NewBadRequest("the dryRun feature is disabled"), w, req) - return - } - namespace, name, err := scope.Namer.Name(req) if err != nil { scope.err(err, w, req) diff --git a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go index 4debb7bedb893..a74e23da1a0b4 100644 --- a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go +++ b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go @@ -208,7 +208,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS CustomResourceValidationExpressions: {Default: true, PreRelease: featuregate.Beta}, - DryRun: {Default: true, PreRelease: featuregate.GA}, + DryRun: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.28 EfficientWatchResumption: {Default: true, PreRelease: featuregate.GA, LockToDefault: true},