Merge pull request #52933 from liggitt/proxy-subpath-slash

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>..

Preserve leading and trailing slashes on proxy subpaths

subresource parsing was not populating path parameters correctly (leading and trailing slashes were being stripped)

this caused bad locations to be sent to the proxy, causing https://github.com/kubernetes/kubernetes/issues/52022. the first attempt to fix that (#52065) unconditionally prefixed '/', which broke the redirect case (#52813 #52729)

fixes #52813, fixes #52729

needs to be picked to 1.7 and 1.8

```release-note
Restores redirect behavior for proxy subresources
```

Kubernetes-commit: e0f75338b5720fbce0aa02b0cf79965950089dbc
This commit is contained in:
Kubernetes Publisher 2017-09-23 12:34:41 -07:00
commit 57baa7511f
2 changed files with 30 additions and 7 deletions

View File

@ -2135,15 +2135,25 @@ func TestGetWithOptions(t *testing.T) {
requestURL: "/namespaces/default/simple/id?param1=test1&param2=test2",
expectedPath: "",
},
{
name: "with root slash",
requestURL: "/namespaces/default/simple/id/?param1=test1&param2=test2",
expectedPath: "/",
},
{
name: "with path",
requestURL: "/namespaces/default/simple/id/a/different/path?param1=test1&param2=test2",
expectedPath: "a/different/path",
expectedPath: "/a/different/path",
},
{
name: "with path with trailing slash",
requestURL: "/namespaces/default/simple/id/a/different/path/?param1=test1&param2=test2",
expectedPath: "/a/different/path/",
},
{
name: "as subresource",
requestURL: "/namespaces/default/simple/id/subresource/another/different/path?param1=test1&param2=test2",
expectedPath: "another/different/path",
expectedPath: "/another/different/path",
},
{
name: "cluster-scoped basic",
@ -2155,13 +2165,13 @@ func TestGetWithOptions(t *testing.T) {
name: "cluster-scoped basic with path",
rootScoped: true,
requestURL: "/simple/id/a/cluster/path?param1=test1&param2=test2",
expectedPath: "a/cluster/path",
expectedPath: "/a/cluster/path",
},
{
name: "cluster-scoped basic as subresource",
rootScoped: true,
requestURL: "/simple/id/subresource/another/cluster/path?param1=test1&param2=test2",
expectedPath: "another/cluster/path",
expectedPath: "/another/cluster/path",
},
}
@ -2556,7 +2566,7 @@ func TestConnectWithOptions(t *testing.T) {
func TestConnectWithOptionsAndPath(t *testing.T) {
responseText := "Hello World"
itemID := "theID"
testPath := "a/b/c/def"
testPath := "/a/b/c/def"
connectStorage := &ConnecterRESTStorage{
connectHandler: &OutputConnect{
response: responseText,
@ -2572,7 +2582,7 @@ func TestConnectWithOptionsAndPath(t *testing.T) {
server := httptest.NewServer(handler)
defer server.Close()
resp, err := http.Get(server.URL + "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/default/simple/" + itemID + "/connect/" + testPath + "?param1=value1&param2=value2")
resp, err := http.Get(server.URL + "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/default/simple/" + itemID + "/connect" + testPath + "?param1=value1&param2=value2")
if err != nil {
t.Errorf("unexpected error: %v", err)

View File

@ -212,7 +212,20 @@ func getRequestOptions(req *http.Request, scope RequestScope, into runtime.Objec
if isSubresource {
startingIndex = 3
}
newQuery[subpathKey] = []string{strings.Join(requestInfo.Parts[startingIndex:], "/")}
p := strings.Join(requestInfo.Parts[startingIndex:], "/")
// ensure non-empty subpaths correctly reflect a leading slash
if len(p) > 0 && !strings.HasPrefix(p, "/") {
p = "/" + p
}
// ensure subpaths correctly reflect the presence of a trailing slash on the original request
if strings.HasSuffix(requestInfo.Path, "/") && !strings.HasSuffix(p, "/") {
p += "/"
}
newQuery[subpathKey] = []string{p}
query = newQuery
}
return scope.ParameterCodec.DecodeParameters(query, scope.Kind.GroupVersion(), into)