Merge pull request #106830 from pacoxu/StreamingProxyRedirects-remove

remove ValidateProxyRedirects and StreamingProxyRedirects

pkg/features/kube_features.go

@@ -950,8 +950,6 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 
 	// inherited features from generic apiserver, relisted here to get a conflict if it is changed
 	// unintentionally on either side:
-	genericfeatures.StreamingProxyRedirects:             {Default: false, PreRelease: featuregate.Deprecated}, // remove in 1.24
-	genericfeatures.ValidateProxyRedirects:              {Default: true, PreRelease: featuregate.Deprecated},
 	genericfeatures.AdvancedAuditing:                    {Default: true, PreRelease: featuregate.GA},
 	genericfeatures.APIResponseCompression:              {Default: true, PreRelease: featuregate.Beta},
 	genericfeatures.APIListChunking:                     {Default: true, PreRelease: featuregate.Beta},

pkg/registry/core/pod/rest/subresources.go

@@ -25,10 +25,8 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/net"
 	"k8s.io/apimachinery/pkg/util/proxy"
-	genericfeatures "k8s.io/apiserver/pkg/features"
 	genericregistry "k8s.io/apiserver/pkg/registry/generic/registry"
 	"k8s.io/apiserver/pkg/registry/rest"
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/capabilities"
 	"k8s.io/kubernetes/pkg/kubelet/client"
@@ -73,7 +71,7 @@ func (r *ProxyREST) Connect(ctx context.Context, id string, opts runtime.Object,
 	}
 	location.Path = net.JoinPreservingTrailingSlash(location.Path, proxyOpts.Path)
 	// Return a proxy handler that uses the desired transport, wrapped with additional proxy handling (to get URL rewriting, X-Forwarded-* headers, etc)
-	return newThrottledUpgradeAwareProxyHandler(location, transport, true, false, false, responder), nil
+	return newThrottledUpgradeAwareProxyHandler(location, transport, true, false, responder), nil
 }
 
 // Support both GET and POST methods. We must support GET for browsers that want to use WebSockets.
@@ -103,7 +101,7 @@ func (r *AttachREST) Connect(ctx context.Context, name string, opts runtime.Obje
 	if err != nil {
 		return nil, err
 	}
-	return newThrottledUpgradeAwareProxyHandler(location, transport, false, true, true, responder), nil
+	return newThrottledUpgradeAwareProxyHandler(location, transport, false, true, responder), nil
 }
 
 // NewConnectOptions returns the versioned object that represents exec parameters
@@ -140,7 +138,7 @@ func (r *ExecREST) Connect(ctx context.Context, name string, opts runtime.Object
 	if err != nil {
 		return nil, err
 	}
-	return newThrottledUpgradeAwareProxyHandler(location, transport, false, true, true, responder), nil
+	return newThrottledUpgradeAwareProxyHandler(location, transport, false, true, responder), nil
 }
 
 // NewConnectOptions returns the versioned object that represents exec parameters
@@ -188,13 +186,11 @@ func (r *PortForwardREST) Connect(ctx context.Context, name string, opts runtime
 	if err != nil {
 		return nil, err
 	}
-	return newThrottledUpgradeAwareProxyHandler(location, transport, false, true, true, responder), nil
+	return newThrottledUpgradeAwareProxyHandler(location, transport, false, true, responder), nil
 }
 
-func newThrottledUpgradeAwareProxyHandler(location *url.URL, transport http.RoundTripper, wrapTransport, upgradeRequired, interceptRedirects bool, responder rest.Responder) *proxy.UpgradeAwareHandler {
+func newThrottledUpgradeAwareProxyHandler(location *url.URL, transport http.RoundTripper, wrapTransport, upgradeRequired bool, responder rest.Responder) *proxy.UpgradeAwareHandler {
 	handler := proxy.NewUpgradeAwareHandler(location, transport, wrapTransport, upgradeRequired, proxy.NewErrorResponder(responder))
-	handler.InterceptRedirects = interceptRedirects && utilfeature.DefaultFeatureGate.Enabled(genericfeatures.StreamingProxyRedirects)
-	handler.RequireSameHostRedirects = utilfeature.DefaultFeatureGate.Enabled(genericfeatures.ValidateProxyRedirects)
 	handler.MaxBytesPerSec = capabilities.Get().PerConnectionBandwidthLimitBytesPerSec
 	return handler
 }

staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware.go

@@ -69,11 +69,6 @@ type UpgradeAwareHandler struct {
 	UpgradeTransport UpgradeRequestRoundTripper
 	// WrapTransport indicates whether the provided Transport should be wrapped with default proxy transport behavior (URL rewriting, X-Forwarded-* header setting)
 	WrapTransport bool
-	// InterceptRedirects determines whether the proxy should sniff backend responses for redirects,
-	// following them as necessary.
-	InterceptRedirects bool
-	// RequireSameHostRedirects only allows redirects to the same host. It is only used if InterceptRedirects=true.
-	RequireSameHostRedirects bool
 	// UseRequestLocation will use the incoming request URL when talking to the backend server.
 	UseRequestLocation bool
 	// UseLocationHost overrides the HTTP host header in requests to the backend server to use the Host from Location.
@@ -310,17 +305,12 @@ func (h *UpgradeAwareHandler) tryUpgrade(w http.ResponseWriter, req *http.Reques
 	// Only append X-Forwarded-For in the upgrade path, since httputil.NewSingleHostReverseProxy
 	// handles this in the non-upgrade path.
 	utilnet.AppendForwardedForHeader(clone)
-	if h.InterceptRedirects {
-		klog.V(6).Infof("Connecting to backend proxy (intercepting redirects) %s\n  Headers: %v", &location, clone.Header)
-		backendConn, rawResponse, err = utilnet.ConnectWithRedirects(req.Method, &location, clone.Header, req.Body, utilnet.DialerFunc(h.DialForUpgrade), h.RequireSameHostRedirects)
-	} else {
 	klog.V(6).Infof("Connecting to backend proxy (direct dial) %s\n  Headers: %v", &location, clone.Header)
 	if h.UseLocationHost {
 		clone.Host = h.Location.Host
 	}
 	clone.URL = &location
 	backendConn, err = h.DialForUpgrade(clone)
-	}
 	if err != nil {
 		klog.V(6).Infof("Proxy connection error: %v", err)
 		h.Responder.Error(w, req, err)

staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware_test.go

@@ -501,13 +501,8 @@ func TestProxyUpgrade(t *testing.T) {
 	}
 
 	for k, tc := range testcases {
-		for _, redirect := range []bool{false, true} {
 		tcName := k
 		backendPath := "/hello"
-			if redirect {
-				tcName += " with redirect"
-				backendPath = "/redirect"
-			}
 		func() { // Cleanup after each test case.
 			backend := http.NewServeMux()
 			backend.Handle("/hello", websocket.Handler(func(ws *websocket.Conn) {
@@ -532,7 +527,6 @@ func TestProxyUpgrade(t *testing.T) {
 			serverURL.Path = backendPath
 			proxyHandler := NewUpgradeAwareHandler(serverURL, tc.ProxyTransport, false, false, &noErrorsAllowed{t: t})
 			proxyHandler.UpgradeTransport = tc.UpgradeTransport
-				proxyHandler.InterceptRedirects = redirect
 			proxy := httptest.NewServer(proxyHandler)
 			defer proxy.Close()
 
@@ -556,7 +550,6 @@ func TestProxyUpgrade(t *testing.T) {
 			}
 		}()
 	}
-	}
 }
 
 type noErrorsAllowed struct {
@@ -614,9 +607,8 @@ func TestProxyUpgradeConnectionErrorResponse(t *testing.T) {
 }
 
 func TestProxyUpgradeErrorResponseTerminates(t *testing.T) {
-	for _, intercept := range []bool{true, false} {
 	for _, code := range []int{400, 500} {
-			t.Run(fmt.Sprintf("intercept=%v,code=%v", intercept, code), func(t *testing.T) {
+		t.Run(fmt.Sprintf("code=%v", code), func(t *testing.T) {
 			// Set up a backend server
 			backend := http.NewServeMux()
 			backend.Handle("/hello", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -633,7 +625,6 @@ func TestProxyUpgradeErrorResponseTerminates(t *testing.T) {
 
 			// Set up a proxy pointing to a specific path on the backend
 			proxyHandler := NewUpgradeAwareHandler(backendServerURL, nil, false, false, &noErrorsAllowed{t: t})
-				proxyHandler.InterceptRedirects = intercept
 			proxy := httptest.NewServer(proxyHandler)
 			defer proxy.Close()
 			proxyURL, _ := url.Parse(proxy.URL)
@@ -672,13 +663,11 @@ func TestProxyUpgradeErrorResponseTerminates(t *testing.T) {
 			conn.Close()
 		})
 	}
-	}
 }
 
 func TestProxyUpgradeErrorResponse(t *testing.T) {
-	for _, intercept := range []bool{true, false} {
 	for _, code := range []int{200, 300, 302, 307} {
-			t.Run(fmt.Sprintf("intercept=%v,code=%v", intercept, code), func(t *testing.T) {
+		t.Run(fmt.Sprintf("code=%v", code), func(t *testing.T) {
 			// Set up a backend server
 			backend := http.NewServeMux()
 			backend.Handle("/hello", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -691,8 +680,6 @@ func TestProxyUpgradeErrorResponse(t *testing.T) {
 
 			// Set up a proxy pointing to a specific path on the backend
 			proxyHandler := NewUpgradeAwareHandler(backendServerURL, nil, false, false, &fakeResponder{t: t})
-				proxyHandler.InterceptRedirects = intercept
-				proxyHandler.RequireSameHostRedirects = true
 			proxy := httptest.NewServer(proxyHandler)
 			defer proxy.Close()
 			proxyURL, _ := url.Parse(proxy.URL)
@@ -715,7 +702,6 @@ func TestProxyUpgradeErrorResponse(t *testing.T) {
 			conn.Close()
 		})
 	}
-	}
 }
 
 func TestDefaultProxyTransport(t *testing.T) {

staging/src/k8s.io/apiserver/pkg/features/kube_features.go

@@ -30,26 +30,6 @@ const (
 	// // alpha: v1.4
 	// MyFeature() bool
 
-	// owner: @tallclair
-	// alpha: v1.5
-	// beta: v1.6
-	// deprecated: v1.18
-	//
-	// StreamingProxyRedirects controls whether the apiserver should intercept (and follow)
-	// redirects from the backend (Kubelet) for streaming requests (exec/attach/port-forward).
-	//
-	// This feature is deprecated, and will be removed in v1.24.
-	StreamingProxyRedirects featuregate.Feature = "StreamingProxyRedirects"
-
-	// owner: @tallclair
-	// alpha: v1.12
-	// beta: v1.14
-	// deprecated: v1.22
-	//
-	// ValidateProxyRedirects controls whether the apiserver should validate that redirects are only
-	// followed to the same host. Only used if StreamingProxyRedirects is enabled.
-	ValidateProxyRedirects featuregate.Feature = "ValidateProxyRedirects"
-
 	// owner: @tallclair
 	// alpha: v1.7
 	// beta: v1.8
@@ -213,8 +193,6 @@ func init() {
 // To add a new feature, define a key for it above and add it here. The features will be
 // available throughout Kubernetes binaries.
 var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
-	StreamingProxyRedirects:             {Default: false, PreRelease: featuregate.Deprecated},
-	ValidateProxyRedirects:              {Default: true, PreRelease: featuregate.Deprecated},
 	AdvancedAuditing:                    {Default: true, PreRelease: featuregate.GA},
 	APIResponseCompression:              {Default: true, PreRelease: featuregate.Beta},
 	APIListChunking:                     {Default: true, PreRelease: featuregate.Beta},

staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_proxy.go

@@ -32,9 +32,7 @@ import (
 	"k8s.io/apiserver/pkg/endpoints/handlers/responsewriters"
 	endpointmetrics "k8s.io/apiserver/pkg/endpoints/metrics"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
-	genericfeatures "k8s.io/apiserver/pkg/features"
 	"k8s.io/apiserver/pkg/server/egressselector"
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	utilflowcontrol "k8s.io/apiserver/pkg/util/flowcontrol"
 	"k8s.io/apiserver/pkg/util/x509metrics"
 	restclient "k8s.io/client-go/rest"
@@ -174,8 +172,6 @@ func (r *proxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	}
 
 	handler := proxy.NewUpgradeAwareHandler(location, proxyRoundTripper, true, upgrade, &responder{w: w})
-	handler.InterceptRedirects = utilfeature.DefaultFeatureGate.Enabled(genericfeatures.StreamingProxyRedirects)
-	handler.RequireSameHostRedirects = utilfeature.DefaultFeatureGate.Enabled(genericfeatures.ValidateProxyRedirects)
 	utilflowcontrol.RequestDelegated(req.Context())
 	handler.ServeHTTP(w, newReq)
 }