Extend proxy test to test all URL rewriting cases.

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()
 		}
 	}
 }

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)
+		}
 	}
 }