From e2b645ec15d77aae363eddfea6dfb391d0511364 Mon Sep 17 00:00:00 2001
From: Daniel Smith <dbsmith@google.com>
Date: Wed, 27 Aug 2014 16:10:44 -0700
Subject: [PATCH 1/3] Add a generic proxier

To proxy traffic to anything that implements ResourceLocation.
Currently, this is only services. This is easily extensible to minions
(would supercede existing mechanism) and pods.
---
 pkg/apiserver/apiserver.go           |   2 +
 pkg/apiserver/apiserver_test.go      |   5 +-
 pkg/apiserver/minionproxy.go         |   1 +
 pkg/apiserver/proxy.go               | 188 +++++++++++++++++++++++++++
 pkg/apiserver/proxy_test.go          | 137 +++++++++++++++++++
 pkg/apiserver/redirect_test.go       |   1 +
 pkg/registry/service/storage.go      |   2 +-
 pkg/registry/service/storage_test.go |   2 +-
 8 files changed, 334 insertions(+), 4 deletions(-)
 create mode 100644 pkg/apiserver/proxy.go
 create mode 100644 pkg/apiserver/proxy_test.go

diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go
index d08f5dbe006..1e56cfd810f 100644
--- a/pkg/apiserver/apiserver.go
+++ b/pkg/apiserver/apiserver.go
@@ -99,8 +99,10 @@ func (g *APIGroup) InstallREST(mux mux, paths ...string) {
 
 	for _, prefix := range paths {
 		prefix = strings.TrimRight(prefix, "/")
+		proxyHandler := &ProxyHandler{prefix + "/proxy/", g.handler.storage, g.handler.codec}
 		mux.Handle(prefix+"/", http.StripPrefix(prefix, restHandler))
 		mux.Handle(prefix+"/watch/", http.StripPrefix(prefix+"/watch/", watchHandler))
+		mux.Handle(prefix+"/proxy/", http.StripPrefix(prefix+"/proxy/", proxyHandler))
 		mux.Handle(prefix+"/redirect/", http.StripPrefix(prefix+"/redirect/", redirectHandler))
 		mux.Handle(prefix+"/operations", http.StripPrefix(prefix+"/operations", opHandler))
 		mux.Handle(prefix+"/operations/", http.StripPrefix(prefix+"/operations/", opHandler))
diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go
index 75bbbab51ab..8e461437d0f 100644
--- a/pkg/apiserver/apiserver_test.go
+++ b/pkg/apiserver/apiserver_test.go
@@ -73,8 +73,9 @@ type SimpleRESTStorage struct {
 	requestedFieldSelector   labels.Selector
 	requestedResourceVersion uint64
 
-	// The location
+	// The id requested, and location to return for ResourceLocation
 	requestedResourceLocationID string
+	resourceLocation            string
 
 	// If non-nil, called inside the WorkFunc when answering update, delete, create.
 	// obj receives the original input to the update, delete, or create call.
@@ -153,7 +154,7 @@ func (storage *SimpleRESTStorage) ResourceLocation(id string) (string, error) {
 	if err := storage.errors["resourceLocation"]; err != nil {
 		return "", err
 	}
-	return id, nil
+	return storage.resourceLocation, nil
 }
 
 func extractBody(response *http.Response, object interface{}) (string, error) {
diff --git a/pkg/apiserver/minionproxy.go b/pkg/apiserver/minionproxy.go
index b85e0ccc8ef..23ac515a367 100644
--- a/pkg/apiserver/minionproxy.go
+++ b/pkg/apiserver/minionproxy.go
@@ -31,6 +31,7 @@ import (
 	"github.com/golang/glog"
 )
 
+// TODO: replace with proxy handler on minions
 func handleProxyMinion(w http.ResponseWriter, req *http.Request) {
 	path := strings.TrimLeft(req.URL.Path, "/")
 	rawQuery := req.URL.RawQuery
diff --git a/pkg/apiserver/proxy.go b/pkg/apiserver/proxy.go
new file mode 100644
index 00000000000..2152c15667c
--- /dev/null
+++ b/pkg/apiserver/proxy.go
@@ -0,0 +1,188 @@
+/*
+Copyright 2014 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package apiserver
+
+import (
+	"bytes"
+	"fmt"
+	"io/ioutil"
+	"net/http"
+	"net/http/httputil"
+	"net/url"
+	"path"
+	"strings"
+
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/httplog"
+
+	"code.google.com/p/go.net/html"
+	"code.google.com/p/go.net/html/atom"
+	"github.com/golang/glog"
+)
+
+type ProxyHandler struct {
+	prefix  string
+	storage map[string]RESTStorage
+	codec   Codec
+}
+
+func (r *ProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
+	parts := strings.SplitN(req.URL.Path, "/", 3)
+	if len(parts) < 2 {
+		notFound(w, req)
+		return
+	}
+	resourceName := parts[0]
+	id := parts[1]
+	rest := ""
+	if len(parts) == 3 {
+		rest = parts[2]
+	}
+	storage, ok := r.storage[resourceName]
+	if !ok {
+		httplog.LogOf(w).Addf("'%v' has no storage object", resourceName)
+		notFound(w, req)
+		return
+	}
+
+	redirector, ok := storage.(Redirector)
+	if !ok {
+		httplog.LogOf(w).Addf("'%v' is not a redirector", resourceName)
+		notFound(w, req)
+		return
+	}
+
+	location, err := redirector.ResourceLocation(id)
+	if err != nil {
+		status := errToAPIStatus(err)
+		writeJSON(status.Code, r.codec, status, w)
+		return
+	}
+
+	destURL, err := url.Parse(location)
+	if err != nil {
+		status := errToAPIStatus(err)
+		writeJSON(status.Code, r.codec, status, w)
+		return
+	}
+	destURL.Path = rest
+	destURL.RawQuery = req.URL.RawQuery
+	newReq, err := http.NewRequest(req.Method, destURL.String(), req.Body)
+	if err != nil {
+		glog.Errorf("Failed to create request: %s", err)
+	}
+	newReq.Header = req.Header
+
+	proxy := httputil.NewSingleHostReverseProxy(&url.URL{Scheme: "http", Host: destURL.Host})
+	proxy.Transport = &proxyTransport{
+		proxyScheme:      req.URL.Scheme,
+		proxyHost:        req.URL.Host,
+		proxyPathPrepend: path.Join(r.prefix, resourceName, id),
+	}
+	proxy.ServeHTTP(w, newReq)
+}
+
+type proxyTransport struct {
+	proxyScheme      string
+	proxyHost        string
+	proxyPathPrepend string
+}
+
+func (t *proxyTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	resp, err := http.DefaultTransport.RoundTrip(req)
+
+	if err != nil {
+		message := err.Error() + "\n" + req.URL.String()
+		if strings.Contains(err.Error(), "connection refused") {
+			message = fmt.Sprintf("Failed to connect to %s (%s)", req.URL.Host, err)
+		}
+		resp = &http.Response{
+			StatusCode: http.StatusServiceUnavailable,
+			Body:       ioutil.NopCloser(strings.NewReader(message)),
+		}
+		return resp, nil
+	}
+
+	if resp.Header.Get("Content-Type") != "text/html" {
+		// Do nothing, simply pass through
+		return resp, err
+	}
+
+	resp, err = t.fixLinks(req, resp)
+	return resp, err
+}
+
+// fixLinks modifies links in an HTML file such that they will be redirected through the proxy if needed.
+func (t *proxyTransport) fixLinks(req *http.Request, resp *http.Response) (*http.Response, error) {
+	body, err := ioutil.ReadAll(resp.Body)
+	resp.Body.Close()
+	if err != nil {
+		// copying the response body did not work
+		return nil, err
+	}
+
+	bodyNode := &html.Node{
+		Type:     html.ElementNode,
+		Data:     "body",
+		DataAtom: atom.Body,
+	}
+	nodes, err := html.ParseFragment(bytes.NewBuffer(body), bodyNode)
+	if err != nil {
+		glog.Errorf("Failed to found <body> node: %v", err)
+		return resp, err
+	}
+
+	// Define the method to traverse the doc tree and update href node to
+	// point to correct minion
+	var updateHRef func(*html.Node)
+	updateHRef = func(n *html.Node) {
+		if n.Type == html.ElementNode && n.Data == "a" {
+			for i, attr := range n.Attr {
+				if attr.Key == "href" {
+					url, err := url.Parse(attr.Val)
+					if err != nil {
+						continue
+					}
+					url.Scheme = t.proxyScheme
+					url.Host = t.proxyHost
+					url.Path = path.Join(t.proxyPathPrepend, path.Dir(req.URL.Path), url.Path, "/")
+					n.Attr[i].Val = url.String()
+					break
+				}
+			}
+		}
+		for c := n.FirstChild; c != nil; c = c.NextSibling {
+			updateHRef(c)
+		}
+	}
+
+	newContent := &bytes.Buffer{}
+	for _, n := range nodes {
+		updateHRef(n)
+		err = html.Render(newContent, n)
+		if err != nil {
+			glog.Errorf("Failed to render: %v", err)
+		}
+	}
+
+	resp.Body = ioutil.NopCloser(newContent)
+	// Update header node with new content-length
+	// TODO: Remove any hash/signature headers here?
+	resp.Header.Del("Content-Length")
+	resp.ContentLength = int64(newContent.Len())
+
+	return resp, err
+}
diff --git a/pkg/apiserver/proxy_test.go b/pkg/apiserver/proxy_test.go
new file mode 100644
index 00000000000..aea841344e1
--- /dev/null
+++ b/pkg/apiserver/proxy_test.go
@@ -0,0 +1,137 @@
+/*
+Copyright 2014 Google Inc. All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package apiserver
+
+import (
+	"fmt"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strings"
+	"testing"
+)
+
+func TestProxyTransport_fixLinks(t *testing.T) {
+	content := string(`<pre><a href="kubelet.log">kubelet.log</a><a href="/google.log">google.log</a></pre>`)
+	transport := &proxyTransport{
+		proxyScheme:      "http",
+		proxyHost:        "foo.com",
+		proxyPathPrepend: "/proxy/minion/minion1:10250/",
+	}
+
+	// Test /logs/
+	request := &http.Request{
+		Method: "GET",
+		URL: &url.URL{
+			Path: "/logs/",
+		},
+	}
+	response := &http.Response{
+		Status:     "200 OK",
+		StatusCode: http.StatusOK,
+		Body:       ioutil.NopCloser(strings.NewReader(content)),
+		Close:      true,
+	}
+	updated_resp, _ := transport.fixLinks(request, response)
+	body, _ := ioutil.ReadAll(updated_resp.Body)
+	expected := string(`<pre><a href="http://foo.com/proxy/minion/minion1:10250/logs/kubelet.log">kubelet.log</a><a href="http://foo.com/proxy/minion/minion1:10250/logs/google.log">google.log</a></pre>`)
+	if !strings.Contains(string(body), expected) {
+		t.Errorf("Received wrong content: %s", string(body))
+	}
+
+	// Test subdir under /logs/
+	request = &http.Request{
+		Method: "GET",
+		URL: &url.URL{
+			Path: "/whatever/apt/somelog.log",
+		},
+	}
+	transport.proxyScheme = "https"
+	transport.proxyPathPrepend = "/proxy/minion/minion1:8080/"
+	response = &http.Response{
+		Status:     "200 OK",
+		StatusCode: http.StatusOK,
+		Body:       ioutil.NopCloser(strings.NewReader(content)),
+		Close:      true,
+	}
+	updated_resp, _ = transport.fixLinks(request, response)
+	body, _ = ioutil.ReadAll(updated_resp.Body)
+	expected = string(`<pre><a href="https://foo.com/proxy/minion/minion1:8080/whatever/apt/kubelet.log">kubelet.log</a><a href="https://foo.com/proxy/minion/minion1:8080/whatever/apt/google.log">google.log</a></pre>`)
+	if !strings.Contains(string(body), expected) {
+		t.Errorf("Received wrong content: %s", string(body))
+	}
+}
+
+func TestProxy(t *testing.T) {
+	table := []struct {
+		method   string
+		path     string
+		reqBody  string
+		respBody string
+	}{
+		{"GET", "/some/dir", "", "answer"},
+		{"POST", "/some/other/dir", "question", "answer"},
+		{"PUT", "/some/dir/id", "different question", "answer"},
+		{"DELETE", "/some/dir/id", "", "ok"},
+	}
+
+	for _, item := range table {
+		proxyServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+			gotBody, err := ioutil.ReadAll(req.Body)
+			if err != nil {
+				t.Errorf("%v - unexpected error %v", item.method, err)
+			}
+			if e, a := item.reqBody, string(gotBody); e != a {
+				t.Errorf("%v - expected %v, got %v", item.method, e, a)
+			}
+			fmt.Fprint(w, item.respBody)
+		}))
+
+		simpleStorage := &SimpleRESTStorage{
+			errors:           map[string]error{},
+			resourceLocation: proxyServer.URL,
+		}
+		handler := Handle(map[string]RESTStorage{
+			"foo": simpleStorage,
+		}, codec, "/prefix/version")
+		server := httptest.NewServer(handler)
+
+		req, err := http.NewRequest(
+			item.method,
+			server.URL+"/prefix/version/proxy/foo/id"+item.path,
+			strings.NewReader(item.reqBody),
+		)
+		if err != nil {
+			t.Errorf("%v - unexpected error %v", item.method, err)
+			continue
+		}
+		resp, err := http.DefaultClient.Do(req)
+		if err != nil {
+			t.Errorf("%v - unexpected error %v", item.method, err)
+			continue
+		}
+		gotResp, err := ioutil.ReadAll(resp.Body)
+		if err != nil {
+			t.Errorf("%v - unexpected error %v", item.method, err)
+		}
+		resp.Body.Close()
+		if e, a := item.respBody, string(gotResp); e != a {
+			t.Errorf("%v - expected %v, got %v", item.method, e, a)
+		}
+	}
+}
diff --git a/pkg/apiserver/redirect_test.go b/pkg/apiserver/redirect_test.go
index aa780e6e60c..6425f920d8c 100644
--- a/pkg/apiserver/redirect_test.go
+++ b/pkg/apiserver/redirect_test.go
@@ -51,6 +51,7 @@ func TestRedirect(t *testing.T) {
 
 	for _, item := range table {
 		simpleStorage.errors["resourceLocation"] = item.err
+		simpleStorage.resourceLocation = item.id
 		resp, err := client.Get(server.URL + "/prefix/version/redirect/foo/" + item.id)
 		if resp == nil {
 			t.Fatalf("Unexpected nil resp")
diff --git a/pkg/registry/service/storage.go b/pkg/registry/service/storage.go
index 44da947b49c..6ea612b9b3b 100644
--- a/pkg/registry/service/storage.go
+++ b/pkg/registry/service/storage.go
@@ -179,7 +179,7 @@ func (rs *RegistryStorage) ResourceLocation(id string) (string, error) {
 	if len(e.Endpoints) == 0 {
 		return "", fmt.Errorf("no endpoints available for %v", id)
 	}
-	return e.Endpoints[rand.Intn(len(e.Endpoints))], nil
+	return "http://" + e.Endpoints[rand.Intn(len(e.Endpoints))], nil
 }
 
 func (rs *RegistryStorage) deleteExternalLoadBalancer(service *api.Service) error {
diff --git a/pkg/registry/service/storage_test.go b/pkg/registry/service/storage_test.go
index bddfc782cda..49e27978c0c 100644
--- a/pkg/registry/service/storage_test.go
+++ b/pkg/registry/service/storage_test.go
@@ -291,7 +291,7 @@ func TestServiceRegistryResourceLocation(t *testing.T) {
 	if err != nil {
 		t.Errorf("Unexpected error: %v", err)
 	}
-	if e, a := "foo:80", location; e != a {
+	if e, a := "http://foo:80", location; e != a {
 		t.Errorf("Expected %v, but got %v", e, a)
 	}
 	if e, a := "foo", registry.GottenID; e != a {

From 6554ee96f7a317c4e66a88fd87a9c18db18cc263 Mon Sep 17 00:00:00 2001
From: Daniel Smith <dbsmith@google.com>
Date: Wed, 3 Sep 2014 17:14:22 -0700
Subject: [PATCH 2/3] Rewrite proxy code for clarity and correctness.

---
 pkg/apiserver/proxy.go | 135 +++++++++++++++++++++++++----------------
 1 file changed, 84 insertions(+), 51 deletions(-)

diff --git a/pkg/apiserver/proxy.go b/pkg/apiserver/proxy.go
index 2152c15667c..6ee6d250f75 100644
--- a/pkg/apiserver/proxy.go
+++ b/pkg/apiserver/proxy.go
@@ -27,12 +27,47 @@ import (
 	"strings"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/httplog"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 
 	"code.google.com/p/go.net/html"
-	"code.google.com/p/go.net/html/atom"
 	"github.com/golang/glog"
 )
 
+// tagsToAttrs states which attributes of which tags require URL substitution.
+// Sources: http://www.w3.org/TR/REC-html40/index/attributes.html
+//          http://www.w3.org/html/wg/drafts/html/master/index.html#attributes-1
+var tagsToAttrs = map[string]util.StringSet{
+	"a":          util.NewStringSet("href"),
+	"applet":     util.NewStringSet("codebase"),
+	"area":       util.NewStringSet("href"),
+	"audio":      util.NewStringSet("src"),
+	"base":       util.NewStringSet("href"),
+	"blockquote": util.NewStringSet("cite"),
+	"body":       util.NewStringSet("background"),
+	"button":     util.NewStringSet("formaction"),
+	"command":    util.NewStringSet("icon"),
+	"del":        util.NewStringSet("cite"),
+	"embed":      util.NewStringSet("src"),
+	"form":       util.NewStringSet("action"),
+	"frame":      util.NewStringSet("longdesc", "src"),
+	"head":       util.NewStringSet("profile"),
+	"html":       util.NewStringSet("manifest"),
+	"iframe":     util.NewStringSet("longdesc", "src"),
+	"img":        util.NewStringSet("longdesc", "src", "usemap"),
+	"input":      util.NewStringSet("src", "usemap", "formaction"),
+	"ins":        util.NewStringSet("cite"),
+	"link":       util.NewStringSet("href"),
+	"object":     util.NewStringSet("classid", "codebase", "data", "usemap"),
+	"q":          util.NewStringSet("cite"),
+	"script":     util.NewStringSet("src"),
+	"source":     util.NewStringSet("src"),
+	"video":      util.NewStringSet("poster", "src"),
+
+	// TODO: css URLs hidden in style elements.
+}
+
+// ProxyHandler provides a http.Handler which will proxy traffic to locations
+// specified by items implementing Redirector.
 type ProxyHandler struct {
 	prefix  string
 	storage map[string]RESTStorage
@@ -105,10 +140,7 @@ func (t *proxyTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 	resp, err := http.DefaultTransport.RoundTrip(req)
 
 	if err != nil {
-		message := err.Error() + "\n" + req.URL.String()
-		if strings.Contains(err.Error(), "connection refused") {
-			message = fmt.Sprintf("Failed to connect to %s (%s)", req.URL.Host, err)
-		}
+		message := fmt.Sprintf("Error: '%s'\nTrying to reach: '%v'", err.Error(), req.URL.String())
 		resp = &http.Response{
 			StatusCode: http.StatusServiceUnavailable,
 			Body:       ioutil.NopCloser(strings.NewReader(message)),
@@ -118,64 +150,65 @@ func (t *proxyTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 
 	if resp.Header.Get("Content-Type") != "text/html" {
 		// Do nothing, simply pass through
-		return resp, err
+		return resp, nil
 	}
 
-	resp, err = t.fixLinks(req, resp)
-	return resp, err
+	return t.fixLinks(req, resp)
+}
+
+// updateURLs checks and updates any of n's attributes that are listed in tagsToAttrs.
+// Any URLs found are, if they're relative, updated with the necessary changes to make
+// a visit to that URL also go through the proxy.
+// sourceURL is the URL of the page which we're currently on; it's required to make
+// relative links work.
+func (t *proxyTransport) updateURLs(n *html.Node, sourceURL *url.URL) {
+	if n.Type != html.ElementNode {
+		return
+	}
+	attrs, ok := tagsToAttrs[n.Data]
+	if !ok {
+		return
+	}
+	for i, attr := range n.Attr {
+		if !attrs.Has(attr.Key) {
+			continue
+		}
+		url, err := url.Parse(attr.Val)
+		if err != nil {
+			continue
+		}
+		// Is this URL relative?
+		if url.Host == "" || url.Host == sourceURL.Host {
+			url.Scheme = t.proxyScheme
+			url.Host = t.proxyHost
+			url.Path = path.Join(t.proxyPathPrepend, path.Dir(sourceURL.Path), url.Path, "/")
+			n.Attr[i].Val = url.String()
+		}
+	}
+}
+
+// scan recursively calls f for every n and every subnode of n.
+func (t *proxyTransport) scan(n *html.Node, f func(*html.Node)) {
+	f(n)
+	for c := n.FirstChild; c != nil; c = c.NextSibling {
+		t.scan(c, f)
+	}
 }
 
 // fixLinks modifies links in an HTML file such that they will be redirected through the proxy if needed.
 func (t *proxyTransport) fixLinks(req *http.Request, resp *http.Response) (*http.Response, error) {
-	body, err := ioutil.ReadAll(resp.Body)
-	resp.Body.Close()
-	if err != nil {
-		// copying the response body did not work
-		return nil, err
-	}
+	defer resp.Body.Close()
 
-	bodyNode := &html.Node{
-		Type:     html.ElementNode,
-		Data:     "body",
-		DataAtom: atom.Body,
-	}
-	nodes, err := html.ParseFragment(bytes.NewBuffer(body), bodyNode)
+	doc, err := html.Parse(resp.Body)
 	if err != nil {
-		glog.Errorf("Failed to found <body> node: %v", err)
+		glog.Errorf("Parse failed: %v", err)
 		return resp, err
 	}
 
-	// Define the method to traverse the doc tree and update href node to
-	// point to correct minion
-	var updateHRef func(*html.Node)
-	updateHRef = func(n *html.Node) {
-		if n.Type == html.ElementNode && n.Data == "a" {
-			for i, attr := range n.Attr {
-				if attr.Key == "href" {
-					url, err := url.Parse(attr.Val)
-					if err != nil {
-						continue
-					}
-					url.Scheme = t.proxyScheme
-					url.Host = t.proxyHost
-					url.Path = path.Join(t.proxyPathPrepend, path.Dir(req.URL.Path), url.Path, "/")
-					n.Attr[i].Val = url.String()
-					break
-				}
-			}
-		}
-		for c := n.FirstChild; c != nil; c = c.NextSibling {
-			updateHRef(c)
-		}
-	}
-
 	newContent := &bytes.Buffer{}
-	for _, n := range nodes {
-		updateHRef(n)
-		err = html.Render(newContent, n)
-		if err != nil {
-			glog.Errorf("Failed to render: %v", err)
-		}
+	t.scan(doc, func(n *html.Node) { t.updateURLs(n, req.URL) })
+	if err := html.Render(newContent, doc); err != nil {
+		glog.Errorf("Failed to render: %v", err)
 	}
 
 	resp.Body = ioutil.NopCloser(newContent)

From b6f1f84875344475d477a83f068833ef7a8a83d2 Mon Sep 17 00:00:00 2001
From: Daniel Smith <dbsmith@google.com>
Date: Thu, 4 Sep 2014 12:28:47 -0700
Subject: [PATCH 3/3] Extend proxy test to test all URL rewriting cases.

---
 pkg/apiserver/proxy.go      |   7 +-
 pkg/apiserver/proxy_test.go | 131 +++++++++++++++++++++++++-----------
 2 files changed, 98 insertions(+), 40 deletions(-)

diff --git a/pkg/apiserver/proxy.go b/pkg/apiserver/proxy.go
index 6ee6d250f75..02ebf9c39d4 100644
--- a/pkg/apiserver/proxy.go
+++ b/pkg/apiserver/proxy.go
@@ -178,11 +178,16 @@ func (t *proxyTransport) updateURLs(n *html.Node, sourceURL *url.URL) {
 			continue
 		}
 		// Is this URL relative?
-		if url.Host == "" || url.Host == sourceURL.Host {
+		if url.Host == "" {
 			url.Scheme = t.proxyScheme
 			url.Host = t.proxyHost
 			url.Path = path.Join(t.proxyPathPrepend, path.Dir(sourceURL.Path), url.Path, "/")
 			n.Attr[i].Val = url.String()
+		} else if url.Host == sourceURL.Host {
+			url.Scheme = t.proxyScheme
+			url.Host = t.proxyHost
+			url.Path = path.Join(t.proxyPathPrepend, url.Path)
+			n.Attr[i].Val = url.String()
 		}
 	}
 }
diff --git a/pkg/apiserver/proxy_test.go b/pkg/apiserver/proxy_test.go
index aea841344e1..b50eb67264d 100644
--- a/pkg/apiserver/proxy_test.go
+++ b/pkg/apiserver/proxy_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package apiserver
 
 import (
+	"bytes"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -24,56 +25,108 @@ import (
 	"net/url"
 	"strings"
 	"testing"
+
+	"code.google.com/p/go.net/html"
 )
 
+func parseURLOrDie(inURL string) *url.URL {
+	parsed, err := url.Parse(inURL)
+	if err != nil {
+		panic(err)
+	}
+	return parsed
+}
+
+// fmtHTML parses and re-emits 'in', effectively canonicalizing it.
+func fmtHTML(in string) string {
+	doc, err := html.Parse(strings.NewReader(in))
+	if err != nil {
+		panic(err)
+	}
+	out := &bytes.Buffer{}
+	if err := html.Render(out, doc); err != nil {
+		panic(err)
+	}
+	return string(out.Bytes())
+}
+
 func TestProxyTransport_fixLinks(t *testing.T) {
-	content := string(`<pre><a href="kubelet.log">kubelet.log</a><a href="/google.log">google.log</a></pre>`)
-	transport := &proxyTransport{
+	testTransport := &proxyTransport{
 		proxyScheme:      "http",
 		proxyHost:        "foo.com",
 		proxyPathPrepend: "/proxy/minion/minion1:10250/",
 	}
-
-	// Test /logs/
-	request := &http.Request{
-		Method: "GET",
-		URL: &url.URL{
-			Path: "/logs/",
-		},
-	}
-	response := &http.Response{
-		Status:     "200 OK",
-		StatusCode: http.StatusOK,
-		Body:       ioutil.NopCloser(strings.NewReader(content)),
-		Close:      true,
-	}
-	updated_resp, _ := transport.fixLinks(request, response)
-	body, _ := ioutil.ReadAll(updated_resp.Body)
-	expected := string(`<pre><a href="http://foo.com/proxy/minion/minion1:10250/logs/kubelet.log">kubelet.log</a><a href="http://foo.com/proxy/minion/minion1:10250/logs/google.log">google.log</a></pre>`)
-	if !strings.Contains(string(body), expected) {
-		t.Errorf("Received wrong content: %s", string(body))
+	testTransport2 := &proxyTransport{
+		proxyScheme:      "https",
+		proxyHost:        "foo.com",
+		proxyPathPrepend: "/proxy/minion/minion1:8080/",
 	}
 
-	// Test subdir under /logs/
-	request = &http.Request{
-		Method: "GET",
-		URL: &url.URL{
-			Path: "/whatever/apt/somelog.log",
+	table := map[string]struct {
+		input     string
+		sourceURL string
+		transport *proxyTransport
+		output    string
+	}{
+		"normal": {
+			input:     `<pre><a href="kubelet.log">kubelet.log</a><a href="/google.log">google.log</a></pre>`,
+			sourceURL: "http://myminion.com/logs/log.log",
+			transport: testTransport,
+			output:    `<pre><a href="http://foo.com/proxy/minion/minion1:10250/logs/kubelet.log">kubelet.log</a><a href="http://foo.com/proxy/minion/minion1:10250/logs/google.log">google.log</a></pre>`,
+		},
+		"subdir": {
+			input:     `<a href="kubelet.log">kubelet.log</a><a href="/google.log">google.log</a>`,
+			sourceURL: "http://myminion.com/whatever/apt/somelog.log",
+			transport: testTransport2,
+			output:    `<a href="https://foo.com/proxy/minion/minion1:8080/whatever/apt/kubelet.log">kubelet.log</a><a href="https://foo.com/proxy/minion/minion1:8080/whatever/apt/google.log">google.log</a>`,
+		},
+		"image": {
+			input:     `<pre><img src="kubernetes.jpg"/></pre>`,
+			sourceURL: "http://myminion.com/",
+			transport: testTransport,
+			output:    `<pre><img src="http://foo.com/proxy/minion/minion1:10250/kubernetes.jpg"/></pre>`,
+		},
+		"abs": {
+			input:     `<script src="http://google.com/kubernetes.js"/>`,
+			sourceURL: "http://myminion.com/any/path/",
+			transport: testTransport,
+			output:    `<script src="http://google.com/kubernetes.js"/>`,
+		},
+		"abs but same host": {
+			input:     `<script src="http://myminion.com/kubernetes.js"/>`,
+			sourceURL: "http://myminion.com/any/path/",
+			transport: testTransport,
+			output:    `<script src="http://foo.com/proxy/minion/minion1:10250/kubernetes.js"/>`,
 		},
 	}
-	transport.proxyScheme = "https"
-	transport.proxyPathPrepend = "/proxy/minion/minion1:8080/"
-	response = &http.Response{
-		Status:     "200 OK",
-		StatusCode: http.StatusOK,
-		Body:       ioutil.NopCloser(strings.NewReader(content)),
-		Close:      true,
-	}
-	updated_resp, _ = transport.fixLinks(request, response)
-	body, _ = ioutil.ReadAll(updated_resp.Body)
-	expected = string(`<pre><a href="https://foo.com/proxy/minion/minion1:8080/whatever/apt/kubelet.log">kubelet.log</a><a href="https://foo.com/proxy/minion/minion1:8080/whatever/apt/google.log">google.log</a></pre>`)
-	if !strings.Contains(string(body), expected) {
-		t.Errorf("Received wrong content: %s", string(body))
+
+	for name, item := range table {
+		// Canonicalize the html so we can diff.
+		item.input = fmtHTML(item.input)
+		item.output = fmtHTML(item.output)
+		req := &http.Request{
+			Method: "GET",
+			URL:    parseURLOrDie(item.sourceURL),
+		}
+		resp := &http.Response{
+			Status:     "200 OK",
+			StatusCode: http.StatusOK,
+			Body:       ioutil.NopCloser(strings.NewReader(item.input)),
+			Close:      true,
+		}
+		updatedResp, err := item.transport.fixLinks(req, resp)
+		if err != nil {
+			t.Errorf("%v: Unexpected error: %v", name, err)
+			continue
+		}
+		body, err := ioutil.ReadAll(updatedResp.Body)
+		if err != nil {
+			t.Errorf("%v: Unexpected error: %v", name, err)
+			continue
+		}
+		if e, a := item.output, string(body); e != a {
+			t.Errorf("%v: expected %v, but got %v", name, e, a)
+		}
 	}
 }