From 6567bbee7b3d374d7e92d289e5a8cca072232809 Mon Sep 17 00:00:00 2001 From: Haowei Cai Date: Fri, 15 Sep 2017 09:54:30 -0700 Subject: [PATCH 1/4] Add redirection test for service using request host. Transport has no scheme and host. --- .../apimachinery/pkg/util/proxy/transport_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/staging/src/k8s.io/apimachinery/pkg/util/proxy/transport_test.go b/staging/src/k8s.io/apimachinery/pkg/util/proxy/transport_test.go index f32bcf69edc..e5450078403 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/proxy/transport_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/proxy/transport_test.go @@ -53,6 +53,9 @@ func TestProxyTransport(t *testing.T) { Host: "foo.com", PathPrepend: "/proxy/node/node1:10250", } + emptyHostAndSchemeTransport := &Transport{ + PathPrepend: "/proxy/node/node1:10250", + } type Item struct { input string sourceURL string @@ -62,6 +65,7 @@ func TestProxyTransport(t *testing.T) { forwardedURI string redirect string redirectWant string + reqHost string } table := map[string]Item{ @@ -158,6 +162,14 @@ func TestProxyTransport(t *testing.T) { redirectWant: "http://example.com/redirected/target/", forwardedURI: "/proxy/node/node1:10250/redirect", }, + "redirect abs use reqHost no host no scheme": { + sourceURL: "http://mynode.com/redirect", + transport: emptyHostAndSchemeTransport, + redirect: "http://10.0.0.1:8001/redirected/target/", + redirectWant: "http://10.0.0.1:8001/proxy/node/node1:10250/redirected/target/", + forwardedURI: "/proxy/node/node1:10250/redirect", + reqHost: "10.0.0.1:8001", + }, "source contains the redirect already": { input: `
kubelet.loggoogle.log
`, sourceURL: "http://foo.com/logs/log.log", @@ -233,6 +245,9 @@ func TestProxyTransport(t *testing.T) { t.Errorf("%v: Unexpected error: %v", name, err) return } + if item.reqHost != "" { + req.Host = item.reqHost + } resp, err := item.transport.RoundTrip(req) if err != nil { t.Errorf("%v: Unexpected error: %v", name, err) From 3102f37e8a1914d3bcce648eba0893b67850e6af Mon Sep 17 00:00:00 2001 From: Haowei Cai Date: Fri, 15 Sep 2017 09:57:14 -0700 Subject: [PATCH 2/4] Handles redirection when service returns absolute path with request's host. --- .../apimachinery/pkg/util/proxy/transport.go | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/util/proxy/transport.go b/staging/src/k8s.io/apimachinery/pkg/util/proxy/transport.go index 5bf22969741..cb89c406761 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/proxy/transport.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/proxy/transport.go @@ -109,7 +109,7 @@ func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { } if redirect := resp.Header.Get("Location"); redirect != "" { - resp.Header.Set("Location", t.rewriteURL(redirect, req.URL)) + resp.Header.Set("Location", t.rewriteURL(redirect, req.URL, req.Host)) return resp, nil } @@ -131,21 +131,27 @@ func (rt *Transport) WrappedRoundTripper() http.RoundTripper { // rewriteURL rewrites a single URL to go through the proxy, if the URL refers // to the same host as sourceURL, which is the page on which the target URL -// occurred. If any error occurs (e.g. parsing), it returns targetURL. -func (t *Transport) rewriteURL(targetURL string, sourceURL *url.URL) string { +// occurred, or if the URL matches the sourceHost. If any error occurs (e.g. +// parsing), it returns targetURL. +func (t *Transport) rewriteURL(targetURL string, sourceURL *url.URL, sourceHost string) string { url, err := url.Parse(targetURL) if err != nil { return targetURL } - isDifferentHost := url.Host != "" && url.Host != sourceURL.Host + isDifferentHost := url.Host != "" && url.Host != sourceURL.Host && url.Host != sourceHost isRelative := !strings.HasPrefix(url.Path, "/") if isDifferentHost || isRelative { return targetURL } - url.Scheme = t.Scheme - url.Host = t.Host + // Do not rewrite scheme and host if the Transport has empty scheme and host + // when targetURL already contains the sourceHost + if !(url.Host == sourceHost && t.Scheme == "" && t.Host == "") { + url.Scheme = t.Scheme + url.Host = t.Host + } + origPath := url.Path // Do not rewrite URL if the sourceURL already contains the necessary prefix. if strings.HasPrefix(url.Path, t.PathPrepend) { @@ -223,7 +229,7 @@ func (t *Transport) rewriteResponse(req *http.Request, resp *http.Response) (*ht } urlRewriter := func(targetUrl string) string { - return t.rewriteURL(targetUrl, req.URL) + return t.rewriteURL(targetUrl, req.URL, req.Host) } err := rewriteHTML(reader, writer, urlRewriter) if err != nil { From eff813644256a9f61f24d008361e06d8206b5b7f Mon Sep 17 00:00:00 2001 From: Haowei Cai Date: Thu, 12 Oct 2017 14:26:05 -0700 Subject: [PATCH 3/4] Change name from sourceHost to sourceRequestHost. --- .../k8s.io/apimachinery/pkg/util/proxy/transport.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/util/proxy/transport.go b/staging/src/k8s.io/apimachinery/pkg/util/proxy/transport.go index cb89c406761..6a03ab169eb 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/proxy/transport.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/proxy/transport.go @@ -131,23 +131,23 @@ func (rt *Transport) WrappedRoundTripper() http.RoundTripper { // rewriteURL rewrites a single URL to go through the proxy, if the URL refers // to the same host as sourceURL, which is the page on which the target URL -// occurred, or if the URL matches the sourceHost. If any error occurs (e.g. +// occurred, or if the URL matches the sourceRequestHost. If any error occurs (e.g. // parsing), it returns targetURL. -func (t *Transport) rewriteURL(targetURL string, sourceURL *url.URL, sourceHost string) string { +func (t *Transport) rewriteURL(targetURL string, sourceURL *url.URL, sourceRequestHost string) string { url, err := url.Parse(targetURL) if err != nil { return targetURL } - isDifferentHost := url.Host != "" && url.Host != sourceURL.Host && url.Host != sourceHost + isDifferentHost := url.Host != "" && url.Host != sourceURL.Host && url.Host != sourceRequestHost isRelative := !strings.HasPrefix(url.Path, "/") if isDifferentHost || isRelative { return targetURL } // Do not rewrite scheme and host if the Transport has empty scheme and host - // when targetURL already contains the sourceHost - if !(url.Host == sourceHost && t.Scheme == "" && t.Host == "") { + // when targetURL already contains the sourceRequestHost + if !(url.Host == sourceRequestHost && t.Scheme == "" && t.Host == "") { url.Scheme = t.Scheme url.Host = t.Host } From 4a5b9dd70ee9467b514c5317d8921e73e6756995 Mon Sep 17 00:00:00 2001 From: Haowei Cai Date: Mon, 23 Oct 2017 15:48:11 -0700 Subject: [PATCH 4/4] Add comments to explain difference between req.URL.Host and req.Host. --- .../k8s.io/apimachinery/pkg/util/proxy/transport.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/staging/src/k8s.io/apimachinery/pkg/util/proxy/transport.go b/staging/src/k8s.io/apimachinery/pkg/util/proxy/transport.go index 6a03ab169eb..6c34ab5241d 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/proxy/transport.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/proxy/transport.go @@ -139,6 +139,18 @@ func (t *Transport) rewriteURL(targetURL string, sourceURL *url.URL, sourceReque return targetURL } + // Example: + // When API server processes a proxy request to a service (e.g. /api/v1/namespace/foo/service/bar/proxy/), + // the sourceURL.Host (i.e. req.URL.Host) is the endpoint IP address of the service. The + // sourceRequestHost (i.e. req.Host) is the Host header that specifies the host on which the + // URL is sought, which can be different from sourceURL.Host. For example, if user sends the + // request through "kubectl proxy" locally (i.e. localhost:8001/api/v1/namespace/foo/service/bar/proxy/), + // sourceRequestHost is "localhost:8001". + // + // If the service's response URL contains non-empty host, and url.Host is equal to either sourceURL.Host + // or sourceRequestHost, we should not consider the returned URL to be a completely different host. + // It's the API server's responsibility to rewrite a same-host-and-absolute-path URL and append the + // necessary URL prefix (i.e. /api/v1/namespace/foo/service/bar/proxy/). isDifferentHost := url.Host != "" && url.Host != sourceURL.Host && url.Host != sourceRequestHost isRelative := !strings.HasPrefix(url.Path, "/") if isDifferentHost || isRelative {