From ddbe4c914f588bfa34f9eba572d35ae84ccee8f4 Mon Sep 17 00:00:00 2001
From: Daniel Smith <dbsmith@google.com>
Date: Thu, 18 Jun 2015 17:52:36 -0700
Subject: [PATCH] Fix proxy rewriting

---
 pkg/apiserver/proxy.go           | 40 ++++++++++++++++++++++++--------
 pkg/kubectl/proxy_server_test.go |  8 +++----
 pkg/util/proxy/transport.go      |  8 ++++++-
 3 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/pkg/apiserver/proxy.go b/pkg/apiserver/proxy.go
index e04b2a6478a..a7a538c4abf 100644
--- a/pkg/apiserver/proxy.go
+++ b/pkg/apiserver/proxy.go
@@ -20,6 +20,7 @@ import (
 	"crypto/tls"
 	"fmt"
 	"io"
+	"math/rand"
 	"net"
 	"net/http"
 	"net/http/httputil"
@@ -55,6 +56,8 @@ type ProxyHandler struct {
 }
 
 func (r *ProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
+	proxyHandlerTraceID := rand.Int63()
+
 	var verb string
 	var apiResource string
 	var httpCode int
@@ -108,7 +111,7 @@ func (r *ProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		return
 	}
 
-	location, transport, err := redirector.ResourceLocation(ctx, id)
+	location, roundTripper, err := redirector.ResourceLocation(ctx, id)
 	if err != nil {
 		httplog.LogOf(req, w).Addf("Error getting ResourceLocation: %v", err)
 		status := errToAPIStatus(err)
@@ -123,8 +126,11 @@ func (r *ProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		return
 	}
 	// If we have a custom dialer, and no pre-existing transport, initialize it to use the dialer.
-	if transport == nil && r.dial != nil {
-		transport = &http.Transport{Dial: r.dial}
+	if roundTripper == nil && r.dial != nil {
+		glog.V(5).Infof("[%x: %v] making a dial-only transport...", proxyHandlerTraceID, req.URL)
+		roundTripper = &http.Transport{Dial: r.dial}
+	} else if roundTripper != nil {
+		glog.V(5).Infof("[%x: %v] using transport %T...", proxyHandlerTraceID, req.URL, roundTripper)
 	}
 
 	// Default to http
@@ -158,7 +164,7 @@ func (r *ProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	// TODO convert this entire proxy to an UpgradeAwareProxy similar to
 	// https://github.com/openshift/origin/blob/master/pkg/util/httpproxy/upgradeawareproxy.go.
 	// That proxy needs to be modified to support multiple backends, not just 1.
-	if r.tryUpgrade(w, req, newReq, location, transport) {
+	if r.tryUpgrade(w, req, newReq, location, roundTripper) {
 		return
 	}
 
@@ -175,19 +181,33 @@ func (r *ProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		return
 	}
 
+	start := time.Now()
+	glog.V(4).Infof("[%x] Beginning proxy %s...", proxyHandlerTraceID, req.URL)
+	defer func() {
+		glog.V(4).Infof("[%x] Proxy %v finished %v.", proxyHandlerTraceID, req.URL, time.Now().Sub(start))
+	}()
+
 	proxy := httputil.NewSingleHostReverseProxy(&url.URL{Scheme: location.Scheme, Host: location.Host})
-	if transport == nil {
+	alreadyRewriting := false
+	if roundTripper != nil {
+		_, alreadyRewriting = roundTripper.(*proxyutil.Transport)
+		glog.V(5).Infof("[%x] Not making a reriting transport for proxy %s...", proxyHandlerTraceID, req.URL)
+	}
+	if !alreadyRewriting {
+		glog.V(5).Infof("[%x] making a transport for proxy %s...", proxyHandlerTraceID, req.URL)
 		prepend := path.Join(r.prefix, resource, id)
 		if len(namespace) > 0 {
 			prepend = path.Join(r.prefix, "namespaces", namespace, resource, id)
 		}
-		transport = &proxyutil.Transport{
-			Scheme:      req.URL.Scheme,
-			Host:        req.URL.Host,
-			PathPrepend: prepend,
+		pTransport := &proxyutil.Transport{
+			Scheme:       req.URL.Scheme,
+			Host:         req.URL.Host,
+			PathPrepend:  prepend,
+			RoundTripper: roundTripper,
 		}
+		roundTripper = pTransport
 	}
-	proxy.Transport = transport
+	proxy.Transport = roundTripper
 	proxy.FlushInterval = 200 * time.Millisecond
 	proxy.ServeHTTP(w, newReq)
 }
diff --git a/pkg/kubectl/proxy_server_test.go b/pkg/kubectl/proxy_server_test.go
index 8ad4fe65f61..10eba1133bb 100644
--- a/pkg/kubectl/proxy_server_test.go
+++ b/pkg/kubectl/proxy_server_test.go
@@ -98,10 +98,10 @@ func TestAccept(t *testing.T) {
 			acceptPaths:  DefaultPathAcceptRE,
 			rejectPaths:  DefaultPathRejectRE,
 			acceptHosts:  DefaultHostAcceptRE,
-			path:         "/foo/v1/pods",
+			path:         "/ui",
 			host:         "localhost",
 			method:       "GET",
-			expectAccept: false,
+			expectAccept: true,
 		},
 		{
 			acceptPaths:  DefaultPathAcceptRE,
@@ -230,7 +230,7 @@ func TestAPIRequests(t *testing.T) {
 
 	// httptest.NewServer should always generate a valid URL.
 	target, _ := url.Parse(ts.URL)
-	proxy := newProxyServer(target)
+	proxy := newProxy(target)
 
 	tests := []struct{ method, body string }{
 		{"GET", ""},
@@ -291,7 +291,7 @@ func TestPathHandling(t *testing.T) {
 			if err != nil {
 				t.Fatalf("%#v: %v", item, err)
 			}
-			pts := httptest.NewServer(p.mux)
+			pts := httptest.NewServer(p.handler)
 			defer pts.Close()
 
 			r, err := http.Get(pts.URL + item.reqPath)
diff --git a/pkg/util/proxy/transport.go b/pkg/util/proxy/transport.go
index c0544dfa8e6..aa9d3cff682 100644
--- a/pkg/util/proxy/transport.go
+++ b/pkg/util/proxy/transport.go
@@ -73,6 +73,8 @@ type Transport struct {
 	Scheme      string
 	Host        string
 	PathPrepend string
+
+	http.RoundTripper
 }
 
 // RoundTrip implements the http.RoundTripper interface
@@ -86,7 +88,11 @@ func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) {
 	req.Header.Set("X-Forwarded-Host", t.Host)
 	req.Header.Set("X-Forwarded-Proto", t.Scheme)
 
-	resp, err := http.DefaultTransport.RoundTrip(req)
+	rt := t.RoundTripper
+	if rt == nil {
+		rt = http.DefaultTransport
+	}
+	resp, err := rt.RoundTrip(req)
 
 	if err != nil {
 		message := fmt.Sprintf("Error: '%s'\nTrying to reach: '%v'", err.Error(), req.URL.String())