Merge pull request #38724 from deads2k/fed-12-fix-exec

Automatic merge from submit-queue

fix connection upgrades through kuberentes-discovery

The initial upgrade through the proxy doesn't use the passed transport to handle the communication to the remote side.  Since we need auth proxy headers, this broke the upgrade for exec.

This sets those headers once if its an upgrade request (the transport stomps them if called anyway, so it won't shadow.).

@sttts I think this is the last required piece.  Then we start wiring in for e2e.

cmd/kubernetes-discovery/main.go

@@ -17,6 +17,7 @@ limitations under the License.
 package main
 
 import (
+	"flag"
 	"os"
 	"runtime"
 
@@ -43,6 +44,7 @@ func main() {
 	}
 
 	cmd := server.NewCommandStartDiscoveryServer(os.Stdout, os.Stderr)
+	cmd.Flags().AddGoFlagSet(flag.CommandLine)
 	if err := cmd.Execute(); err != nil {
 		cmdutil.CheckErr(err)
 	}

cmd/kubernetes-discovery/pkg/apiserver/handler_proxy.go

@@ -98,8 +98,20 @@ func (r *proxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 
 	upgrade := false
 	// we need to wrap the roundtripper in another roundtripper which will apply the front proxy headers
-	proxyRoundTripper = transport.NewAuthProxyRoundTripper(user.GetName(), user.GetGroups(), user.GetExtra(), proxyRoundTripper)
 	proxyRoundTripper, upgrade, err = r.maybeWrapForConnectionUpgrades(proxyRoundTripper, req)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+	proxyRoundTripper = transport.NewAuthProxyRoundTripper(user.GetName(), user.GetGroups(), user.GetExtra(), proxyRoundTripper)
+
+	// if we are upgrading, then the upgrade path tries to use this request with the TLS config we provide, but it does
+	// NOT use the roundtripper.  Its a direct call that bypasses the round tripper.  This means that we have to
+	// attach the "correct" user headers to the request ahead of time.  After the initial upgrade, we'll be back
+	// at the roundtripper flow, so we only have to muck with this request, but we do have to do it.
+	if upgrade {
+		transport.SetAuthProxyHeaders(newReq, user.GetName(), user.GetGroups(), user.GetExtra())
+	}
 
 	handler := genericrest.NewUpgradeAwareProxyHandler(location, proxyRoundTripper, true, upgrade, &responder{w: w})
 	handler.ServeHTTP(w, newReq)

pkg/client/transport/round_trippers.go

@@ -106,6 +106,13 @@ func NewAuthProxyRoundTripper(username string, groups []string, extra map[string
 
 func (rt *authProxyRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
 	req = cloneRequest(req)
+	SetAuthProxyHeaders(req, rt.username, rt.groups, rt.extra)
+
+	return rt.rt.RoundTrip(req)
+}
+
+// SetAuthProxyHeaders stomps the auth proxy header fields.  It mutates its argument.
+func SetAuthProxyHeaders(req *http.Request, username string, groups []string, extra map[string][]string) {
 	req.Header.Del("X-Remote-User")
 	req.Header.Del("X-Remote-Group")
 	for key := range req.Header {
@@ -114,17 +121,15 @@ func (rt *authProxyRoundTripper) RoundTrip(req *http.Request) (*http.Response, e
 		}
 	}
 
-	req.Header.Set("X-Remote-User", rt.username)
-	for _, group := range rt.groups {
+	req.Header.Set("X-Remote-User", username)
+	for _, group := range groups {
 		req.Header.Add("X-Remote-Group", group)
 	}
-	for key, values := range rt.extra {
+	for key, values := range extra {
 		for _, value := range values {
 			req.Header.Add("X-Remote-Extra-"+key, value)
 		}
 	}
-
-	return rt.rt.RoundTrip(req)
 }
 
 func (rt *authProxyRoundTripper) CancelRequest(req *http.Request) {