Merge pull request kubernetes#6175 from nikhiljindal/proxy

Updating proxy to return 301 to add a "/" at the end for kubernetes#4958 (attempt 2)
nikhiljindal · Mar 30, 2015 · 83d5c20 · 83d5c20
2 parents 1dc895e + 4c4a084
commit 83d5c20
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 0 deletions.
diff --git a/pkg/apiserver/proxy.go b/pkg/apiserver/proxy.go
@@ -191,6 +191,15 @@ func (r *ProxyHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		return
 	}
 
+	// Redirect requests of the form "/{resource}/{name}" to "/{resource}/{name}/"
+	// 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(parts) == 2 && !strings.HasSuffix(req.URL.Path, "/") {
+		w.Header().Set("Location", req.URL.Path+"/")
+		w.WriteHeader(http.StatusMovedPermanently)
+		return
+	}
+
 	proxy := httputil.NewSingleHostReverseProxy(&url.URL{Scheme: location.Scheme, Host: location.Host})
 	if transport == nil {
 		prepend := path.Join(r.prefix, resource, id)

diff --git a/pkg/apiserver/proxy_test.go b/pkg/apiserver/proxy_test.go
@@ -367,3 +367,59 @@ func TestProxyUpgrade(t *testing.T) {
 		t.Fatalf("expected '%#v', got '%#v'", e, a)
 	}
 }
+
+func TestRedirectOnMissingTrailingSlash(t *testing.T) {
+	table := []struct {
+		// The requested path
+		path string
+		// The path requested on the proxy server.
+		proxyServerPath string
+	}{
+		{"/trailing/slash/", "/trailing/slash/"},
+		{"/", "/"},
+		// "/" should be added at the end.
+		{"", "/"},
+		// "/" should not be added at a non-root path.
+		{"/some/path", "/some/path"},
+	}
+
+	for _, item := range table {
+		proxyServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+			if req.URL.Path != item.proxyServerPath {
+				t.Errorf("Unexpected request on path: %s, expected path: %s, item: %v", req.URL.Path, item.proxyServerPath, item)
+			}
+		}))
+		defer proxyServer.Close()
+
+		serverURL, _ := url.Parse(proxyServer.URL)
+		simpleStorage := &SimpleRESTStorage{
+			errors:                    map[string]error{},
+			resourceLocation:          serverURL,
+			expectedResourceNamespace: "ns",
+		}
+
+		handler := handleNamespaced(map[string]rest.Storage{"foo": simpleStorage})
+		server := httptest.NewServer(handler)
+		defer server.Close()
+
+		proxyTestPattern := "/api/version2/proxy/namespaces/ns/foo/id" + item.path
+		req, err := http.NewRequest(
+			"GET",
+			server.URL+proxyTestPattern,
+			strings.NewReader(""),
+		)
+		if err != nil {
+			t.Errorf("unexpected error %v", err)
+			continue
+		}
+		// Note: We are using a default client here, that follows redirects.
+		resp, err := http.DefaultClient.Do(req)
+		if err != nil {
+			t.Errorf("unexpected error %v", err)
+			continue
+		}
+		if resp.StatusCode != http.StatusOK {
+			t.Errorf("Unexpected errorCode: %v, expected: 200. Response: %v, item: %v", resp.StatusCode, resp, item)
+		}
+	}
+}