From 065bf2004d27e5e3f1be3c0f128347d4060d8954 Mon Sep 17 00:00:00 2001
From: Marek Siarkowicz <siarkowicz@google.com>
Date: Fri, 31 Jan 2025 11:49:28 +0100
Subject: [PATCH] Deprecate WatchFromStorageWithoutResourceVersion

Around the 1.31 release, we discovered that a change introduced in 1.27 allowead
clients to open WATCH requests directly to etcd. This had detrimental consequences,
enabling abusive clients to bypass caching and overwhelm etcd.
Unlike the API server, etcd lacks protection against such behavior.

To mitigate this, we redirected all WATCH requests to be served from the cache.
The WatchFromStorageWithoutResourceVersion feature gate was retained as an escape hatch.
However, since we have no plans to allow direct WATCH requests to etcd again,
this flag is now obsolete.

Direct WATCH requests to etcd offer no advantage, as they don't provide stronger
consistency guarantees. WATCH operations are inherently inconsistent; unlike LIST
operations, they do not confirm the resource version with a quorum. While Kubernetes
uses the WithRequireLeader option on WATCH requests to prevent maintaining connections
to isolated etcd members, the API server provides the same level of guarantee through
its health checks, which fail if it cannot connect to etcd member.  Therefore,
the WatchFromStorageWithoutResourceVersion feature gate can be deprecated and removed.
---
 pkg/features/versioned_kube_features.go       |  1 +
 .../apiserver/pkg/features/kube_features.go   |  1 +
 .../storage/cacher/cacher_whitebox_test.go    | 22 +------------------
 .../test_data/versioned_feature_list.yaml     |  4 ++++
 4 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/pkg/features/versioned_kube_features.go b/pkg/features/versioned_kube_features.go
index 5f5236f9b94..e56aef1b752 100644
--- a/pkg/features/versioned_kube_features.go
+++ b/pkg/features/versioned_kube_features.go
@@ -368,6 +368,7 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate
 
 	genericfeatures.WatchFromStorageWithoutResourceVersion: {
 		{Version: version.MustParse("1.27"), Default: false, PreRelease: featuregate.Beta},
+		{Version: version.MustParse("1.33"), Default: false, PreRelease: featuregate.Deprecated, LockToDefault: true},
 	},
 
 	genericfeatures.WatchList: {
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 becfa026da0..7bede74c034 100644
--- a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go
+++ b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go
@@ -407,6 +407,7 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate
 
 	WatchFromStorageWithoutResourceVersion: {
 		{Version: version.MustParse("1.27"), Default: false, PreRelease: featuregate.Beta},
+		{Version: version.MustParse("1.33"), Default: false, PreRelease: featuregate.Deprecated, LockToDefault: true},
 	},
 
 	WatchList: {
diff --git a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_whitebox_test.go b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_whitebox_test.go
index 5378560a6f4..d267503cd73 100644
--- a/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_whitebox_test.go
+++ b/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher_whitebox_test.go
@@ -594,27 +594,7 @@ func TestWatchCacheBypass(t *testing.T) {
 		Predicate:       storage.Everything,
 	})
 	if err != nil {
-		t.Errorf("Watch with RV=0 should be served from cache: %v", err)
-	}
-
-	featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.WatchFromStorageWithoutResourceVersion, false)
-	_, err = proxy.Watch(context.TODO(), "pod/ns", storage.ListOptions{
-		ResourceVersion: "",
-		Predicate:       storage.Everything,
-	})
-	if err != nil {
-		t.Errorf("With WatchFromStorageWithoutResourceVersion disabled, watch with unset RV should be served from cache: %v", err)
-	}
-
-	// Inject error to underlying layer and check if cacher is not bypassed.
-	backingStorage.injectError(errDummy)
-	featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.WatchFromStorageWithoutResourceVersion, true)
-	_, err = proxy.Watch(context.TODO(), "pod/ns", storage.ListOptions{
-		ResourceVersion: "",
-		Predicate:       storage.Everything,
-	})
-	if !errors.Is(err, errDummy) {
-		t.Errorf("With WatchFromStorageWithoutResourceVersion enabled, watch with unset RV should be served from storage: %v", err)
+		t.Errorf("Watch without RV=0 should be served from cache: %v", err)
 	}
 }
 
diff --git a/test/featuregates_linter/test_data/versioned_feature_list.yaml b/test/featuregates_linter/test_data/versioned_feature_list.yaml
index 6a3d8df9bf6..e41145f4a04 100644
--- a/test/featuregates_linter/test_data/versioned_feature_list.yaml
+++ b/test/featuregates_linter/test_data/versioned_feature_list.yaml
@@ -1478,6 +1478,10 @@
     lockToDefault: false
     preRelease: Beta
     version: "1.27"
+  - default: false
+    lockToDefault: true
+    preRelease: Deprecated
+    version: "1.33"
 - name: WatchList
   versionedSpecs:
   - default: false