Merge pull request #52556 from roycaihw/51790

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Apiserver proxy rewrites URL when service returns absolute path with request's host

**What this PR does / why we need it**:
When a service responses with an URL using an absolute path and the request's host (e.g. in redirection location), current transport recognizes the URL as a different host and doesn't rewrite the absolute path.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #51790

**Special notes for your reviewer**:

**Release note**:

```release-note
Apiserver proxy rewrites URL when service returns absolute path with request's host.
```

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,39 @@ 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.
+// occurred, or if the URL matches the sourceRequestHost. If any error occurs (e.g.
-func (t *Transport) rewriteURL(targetURL string, sourceURL *url.URL) string {
+// parsing), it returns targetURL.
+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
+	// 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 {
 		return targetURL
 	}
 
-	url.Scheme = t.Scheme
+	// Do not rewrite scheme and host if the Transport has empty scheme and host
-	url.Host = t.Host
+	// when targetURL already contains the sourceRequestHost
+	if !(url.Host == sourceRequestHost && 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 +241,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 {

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:        `<pre><a href="kubelet.log">kubelet.log</a><a href="http://foo.com/proxy/node/node1:10250/google.log">google.log</a></pre>`,
 			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)