From 684dcd430717f5d184544c821682ba37eb1747da Mon Sep 17 00:00:00 2001 From: Cesar Wong Date: Wed, 27 May 2015 21:05:07 +0000 Subject: [PATCH] Fix proxying of URLs that end in "/" in the pod proxy subresource Also handles proxying of URLs that have an empty path and don't end in a slash "/" by redirecting to the same location with a slash appended. --- pkg/registry/generic/rest/proxy.go | 31 ++++++++- pkg/registry/generic/rest/proxy_test.go | 85 ++++++++++++++++++++++--- pkg/util/proxy/transport.go | 6 +- 3 files changed, 110 insertions(+), 12 deletions(-) diff --git a/pkg/registry/generic/rest/proxy.go b/pkg/registry/generic/rest/proxy.go index cd037f3e008..2d5e31338ee 100644 --- a/pkg/registry/generic/rest/proxy.go +++ b/pkg/registry/generic/rest/proxy.go @@ -76,12 +76,33 @@ func (h *UpgradeAwareProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Re return } + loc := *h.Location + loc.RawQuery = req.URL.RawQuery + + // If original request URL ended in '/', append a '/' at the end of the + // of the proxy URL + if !strings.HasSuffix(loc.Path, "/") && strings.HasSuffix(req.URL.Path, "/") { + loc.Path += "/" + } + + // From pkg/apiserver/proxy.go#ServeHTTP: + // Redirect requests with an empty path to a location that ends with a '/' + // This is essentially a hack for https://github.com/GoogleCloudPlatform/kubernetes/issues/4958. + // Note: Keep this code after tryUpgrade to not break that flow. + if len(loc.Path) == 0 { + var queryPart string + if len(req.URL.RawQuery) > 0 { + queryPart = "?" + req.URL.RawQuery + } + w.Header().Set("Location", req.URL.Path+"/"+queryPart) + w.WriteHeader(http.StatusMovedPermanently) + return + } + if h.Transport == nil { h.Transport = h.defaultProxyTransport(req.URL) } - loc := *h.Location - loc.RawQuery = req.URL.RawQuery newReq, err := http.NewRequest(req.Method, loc.String(), req.Body) if err != nil { h.err = err @@ -188,7 +209,11 @@ func (h *UpgradeAwareProxyHandler) dialURL() (net.Conn, error) { func (h *UpgradeAwareProxyHandler) defaultProxyTransport(url *url.URL) http.RoundTripper { scheme := url.Scheme host := url.Host - pathPrepend := strings.TrimRight(url.Path, h.Location.Path) + suffix := h.Location.Path + if strings.HasSuffix(url.Path, "/") && !strings.HasSuffix(suffix, "/") { + suffix += "/" + } + pathPrepend := strings.TrimSuffix(url.Path, suffix) return &proxy.Transport{ Scheme: scheme, Host: host, diff --git a/pkg/registry/generic/rest/proxy_test.go b/pkg/registry/generic/rest/proxy_test.go index 780b0a9e534..7b4eca07cdf 100644 --- a/pkg/registry/generic/rest/proxy_test.go +++ b/pkg/registry/generic/rest/proxy_test.go @@ -26,6 +26,8 @@ import ( "testing" "golang.org/x/net/websocket" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/util/proxy" ) type SimpleBackendHandler struct { @@ -88,24 +90,28 @@ func TestServeHTTP(t *testing.T) { name string method string requestPath string + expectedPath string requestBody string requestParams map[string]string requestHeader map[string]string }{ { - name: "root path, simple get", - method: "GET", - requestPath: "/", + name: "root path, simple get", + method: "GET", + requestPath: "/", + expectedPath: "/", }, { - name: "simple path, get", - method: "GET", - requestPath: "/path/to/test", + name: "simple path, get", + method: "GET", + requestPath: "/path/to/test", + expectedPath: "/path/to/test", }, { name: "request params", method: "POST", - requestPath: "/some/path", + requestPath: "/some/path/", + expectedPath: "/some/path/", requestParams: map[string]string{"param1": "value/1", "param2": "value%2"}, requestBody: "test request body", }, @@ -113,8 +119,15 @@ func TestServeHTTP(t *testing.T) { name: "request headers", method: "PUT", requestPath: "/some/path", + expectedPath: "/some/path", requestHeader: map[string]string{"Header1": "value1", "Header2": "value2"}, }, + { + name: "empty path - slash should be added", + method: "GET", + requestPath: "", + expectedPath: "/", + }, } for _, test := range tests { @@ -176,7 +189,7 @@ func TestServeHTTP(t *testing.T) { } // Path - if backendHandler.requestURL.Path != test.requestPath { + if backendHandler.requestURL.Path != test.expectedPath { t.Errorf("Unexpected request path: %s", backendHandler.requestURL.Path) } // Parameters @@ -240,3 +253,59 @@ func TestProxyUpgrade(t *testing.T) { t.Fatalf("expected '%#v', got '%#v'", e, a) } } + +func TestDefaultProxyTransport(t *testing.T) { + tests := []struct { + name, + url, + location, + expectedScheme, + expectedHost, + expectedPathPrepend string + }{ + + { + name: "simple path", + url: "http://test.server:8080/a/test/location", + location: "http://localhost/location", + expectedScheme: "http", + expectedHost: "test.server:8080", + expectedPathPrepend: "/a/test", + }, + { + name: "empty path", + url: "http://test.server:8080/a/test/", + location: "http://localhost", + expectedScheme: "http", + expectedHost: "test.server:8080", + expectedPathPrepend: "/a/test", + }, + { + name: "location ending in slash", + url: "http://test.server:8080/a/test/", + location: "http://localhost/", + expectedScheme: "http", + expectedHost: "test.server:8080", + expectedPathPrepend: "/a/test", + }, + } + + for _, test := range tests { + locURL, _ := url.Parse(test.location) + URL, _ := url.Parse(test.url) + h := UpgradeAwareProxyHandler{ + Location: locURL, + } + result := h.defaultProxyTransport(URL) + transport := result.(*proxy.Transport) + if transport.Scheme != test.expectedScheme { + t.Errorf("%s: unexpected scheme. Actual: %s, Expected: %s", test.name, transport.Scheme, test.expectedScheme) + } + if transport.Host != test.expectedHost { + t.Errorf("%s: unexpected host. Actual: %s, Expected: %s", test.name, transport.Host, test.expectedHost) + } + if transport.PathPrepend != test.expectedPathPrepend { + t.Errorf("%s: unexpected path prepend. Actual: %s, Expected: %s", test.name, transport.PathPrepend, test.expectedPathPrepend) + } + } +} diff --git a/pkg/util/proxy/transport.go b/pkg/util/proxy/transport.go index e925e316404..f49ceed0ea7 100644 --- a/pkg/util/proxy/transport.go +++ b/pkg/util/proxy/transport.go @@ -77,7 +77,11 @@ type Transport struct { // RoundTrip implements the http.RoundTripper interface func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { // Add reverse proxy headers. - req.Header.Set("X-Forwarded-Uri", t.PathPrepend+req.URL.Path) + forwardedURI := path.Join(t.PathPrepend, req.URL.Path) + if strings.HasSuffix(req.URL.Path, "/") { + forwardedURI = forwardedURI + "/" + } + req.Header.Set("X-Forwarded-Uri", forwardedURI) req.Header.Set("X-Forwarded-Host", t.Host) req.Header.Set("X-Forwarded-Proto", t.Scheme)