internal/concierge/impersonator: use http/2.0 as much as we can

Signed-off-by: Monis Khan <mok@vmware.com>

internal/concierge/impersonator/impersonator.go

@@ -217,15 +217,14 @@ func newImpersonationReverseProxyFunc(restConfig *rest.Config) (func(*genericapi
 		return nil, fmt.Errorf("could not parse host URL from in-cluster config: %w", err)
 	}
 
-	kubeTransportConfig, err := restConfig.TransportConfig()
+	http1RoundTripper, err := getTransportForProtocol(restConfig, "http/1.1")
 	if err != nil {
-		return nil, fmt.Errorf("could not get in-cluster transport config: %w", err)
+		return nil, fmt.Errorf("could not get http/1.1 round tripper: %w", err)
 	}
-	kubeTransportConfig.TLS.NextProtos = []string{"http/1.1"} // TODO huh?
 
-	kubeRoundTripper, err := transport.New(kubeTransportConfig)
+	http2RoundTripper, err := getTransportForProtocol(restConfig, "h2")
 	if err != nil {
-		return nil, fmt.Errorf("could not get in-cluster transport: %w", err)
+		return nil, fmt.Errorf("could not get http/2.0 round tripper: %w", err)
 	}
 
 	return func(c *genericapiserver.Config) http.Handler {
@@ -259,7 +258,26 @@ func newImpersonationReverseProxyFunc(restConfig *rest.Config) (func(*genericapi
 				return
 			}
 
-			rt, err := getTransportForUser(userInfo, kubeRoundTripper)
+			reqInfo, ok := request.RequestInfoFrom(r.Context())
+			if !ok {
+				plog.Warning("aggregated API server logic did not set request info but it is always supposed to do so",
+					"url", r.URL.String(),
+					"method", r.Method,
+				)
+				newInternalErrResponse(w, r, c.Serializer, "invalid request info")
+				return
+			}
+
+			// when we are running regular requests (e.g., CRUD) we should always be able to use HTTP/2.0
+			// since KAS always supports that and it goes through proxies just fine. for long running
+			// requests (e.g., proxy, watch), we know they use http/1.1 with an upgrade to
+			// websockets/SPDY (this upgrade is NEVER to HTTP/2.0 as the KAS does not support that).
+			baseRT := http2RoundTripper
+			if c.LongRunningFunc(r, reqInfo) {
+				baseRT = http1RoundTripper
+			}
+
+			rt, err := getTransportForUser(userInfo, baseRT)
 			if err != nil {
 				plog.WarningErr("rejecting request as we cannot act as the current user", err,
 					"url", r.URL.String(),
@@ -335,3 +353,13 @@ func newStatusErrResponse(w http.ResponseWriter, r *http.Request, s runtime.Nego
 	gv := schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion}
 	responsewriters.ErrorNegotiated(err, s, gv, w, r)
 }
+
+func getTransportForProtocol(restConfig *rest.Config, protocol string) (http.RoundTripper, error) {
+	transportConfig, err := restConfig.TransportConfig()
+	if err != nil {
+		return nil, fmt.Errorf("could not get in-cluster transport config: %w", err)
+	}
+	transportConfig.TLS.NextProtos = []string{protocol}
+
+	return transport.New(transportConfig)
+}

internal/concierge/impersonator/impersonator_test.go

@@ -5,6 +5,7 @@ package impersonator
 
 import (
 	"context"
+	"math/rand"
 	"net"
 	"net/http"
 	"net/http/httptest"
@@ -359,7 +360,7 @@ func TestImpersonatorHTTPHandler(t *testing.T) {
 				ExecProvider: &api.ExecConfig{},
 				AuthProvider: &api.AuthProviderConfig{},
 			},
-			wantCreationErr: "could not get in-cluster transport config: execProvider and authProvider cannot be used in combination",
+			wantCreationErr: "could not get http/1.1 round tripper: could not get in-cluster transport config: execProvider and authProvider cannot be used in combination",
 		},
 		{
 			name: "fail to get transport from config",
@@ -369,7 +370,7 @@ func TestImpersonatorHTTPHandler(t *testing.T) {
 				Transport:       http.DefaultTransport,
 				TLSClientConfig: rest.TLSClientConfig{Insecure: true},
 			},
-			wantCreationErr: "could not get in-cluster transport: using a custom transport with TLS certificate options or the insecure flag is not allowed",
+			wantCreationErr: "could not get http/1.1 round tripper: using a custom transport with TLS certificate options or the insecure flag is not allowed",
 		},
 		{
 			name:           "Impersonate-User header already in request",
@@ -513,6 +514,10 @@ func TestImpersonatorHTTPHandler(t *testing.T) {
 			metav1.AddToGroupVersion(scheme, metav1.Unversioned)
 			codecs := serializer.NewCodecFactory(scheme)
 			serverConfig := genericapiserver.NewRecommendedConfig(codecs)
+			serverConfig.Config.LongRunningFunc = func(_ *http.Request, _ *request.RequestInfo) bool {
+				// take the HTTP/2.0 vs HTTP/1.1 branch randomly to make sure we exercise both branches
+				return rand.Int()%2 == 0 //nolint:gosec // we don't care whether this is cryptographically secure or not
+			}
 
 			w := httptest.NewRecorder()
 			requestBeforeServe := tt.request.Clone(tt.request.Context())