Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

apiserver proxy removes leading slash from path #52022

Closed
VasylSamoilov opened this issue Sep 6, 2017 · 8 comments
Closed

apiserver proxy removes leading slash from path #52022

VasylSamoilov opened this issue Sep 6, 2017 · 8 comments
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.

Comments

@VasylSamoilov
Copy link

Is this a BUG REPORT or FEATURE REQUEST?:

/kind bug

What happened:

Apiserver proxy removes leading slash from path that is send to destination service or pod, effectively breaking applications.

Example tshark output:

Hypertext Transfer Protocol
GET test HTTP/1.1\r\n
[Expert Info (Chat/Sequence): GET test HTTP/1.1\r\n]
[GET test HTTP/1.1\r\n]
[Severity level: Chat]
[Group: Sequence]
Request Method: GET
Request URI: test
Request Version: HTTP/1.1
Host: localhost:8080\r\n
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.101 Safari/537.36\r\n
Accept-Encoding: gzip, deflate, br\r\n
Accept-Language: en-US,en;q=0.8,uk;q=0.6,ru;q=0.4\r\n
Cache-Control: no-cache\r\n
Connection: Upgrade\r\n
Origin: http://localhost:8080\r\n
Pragma: no-cache\r\n
Sec-Websocket-Extensions: permessage-deflate; client_max_window_bits\r\n
Sec-Websocket-Key: rtBOKvTHtRAbTpz9vhyAng==\r\n
Sec-Websocket-Version: 13\r\n
Upgrade: websocket\r\n
X-Forwarded-For: 127.0.0.1\r\n
\r\n
[Full request URI: http://localhost:8080test]
[HTTP request 1/1]

What you expected to happen:

Proxied URLs should begin with slash.
Example:
GET /test HTTP/1.1\r\n

How to reproduce it (as minimally and precisely as possible):

Use this guideline to construct proxy URL: https://kubernetes.io/docs/tasks/access-application-cluster/access-cluster/#discovering-builtin-services

This request will not work (404 not found):
curl http://localhost:8080/api/v1/namespaces/default/services/testservice:http/proxy/test
backend result (wrong): GET test HTTP/1.1\r\n

Request with double slash will work:
curl http://localhost:8080/api/v1/namespaces/default/services/testservice:http/proxy//test
backend result (correct): GET /test HTTP/1.1\r\n

Also old-style proxy URL format works too:
curl http://localhost:8080/api/v1/proxy/namespaces/default/services/testservice:http/test
backend result (correct): GET /test HTTP/1.1\r\n

Anything else we need to know?:
First observed with 1.7 kubernetes release

Environment:

  • Kubernetes version (use kubectl version):
    Client Version: version.Info{Major:"1", Minor:"8+", GitVersion:"v1.8.0-alpha.3", GitCommit:"6a4203eb4b5f59ac7a602e0c83023d15d991fd58", GitTreeState:"clean", BuildDate:"2017-08-23T22:58:52Z", GoVersion:"go1.8.3", Compiler:"gc", Platform:"linux/amd64"}
    Server Version: version.Info{Major:"1", Minor:"7+", GitVersion:"v1.7.5-dirty", GitCommit:"17d7182a7ccbb167074be7a87f0a68bd00d58d97", GitTreeState:"dirty", BuildDate:"2017-09-04T14:44:32Z", GoVersion:"go1.8.3", Compiler:"gc", Platform:"linux/amd64"}

  • Cloud provider or hardware configuration**:
    Bare metal

  • OS (e.g. from /etc/os-release):
    CoreOS 1409.6.0

  • Kernel (e.g. uname -a):
    4.11.9-coreos

  • Install tools:

  • Others:

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Sep 6, 2017
@k8s-github-robot
Copy link

@VasylSamoilov
There are no sig labels on this issue. Please add a sig label by:

  1. mentioning a sig: @kubernetes/sig-<group-name>-<group-suffix>
    e.g., @kubernetes/sig-contributor-experience-<group-suffix> to notify the contributor experience sig, OR

  2. specifying the label manually: /sig <label>
    e.g., /sig scalability to apply the sig/scalability label

Note: Method 1 will trigger an email to the group. You can find the group list here and label list here.
The <group-suffix> in the method 1 has to be replaced with one of these: bugs, feature-requests, pr-reviews, test-failures, proposals

@k8s-github-robot k8s-github-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Sep 6, 2017
@VasylSamoilov
Copy link
Author

@kubernetes/sig-api-machinery-bugs

@k8s-ci-robot k8s-ci-robot added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. kind/bug Categorizes issue or PR as related to a bug. labels Sep 6, 2017
@k8s-ci-robot
Copy link
Contributor

@VasylSamoilov: Reiterating the mentions to trigger a notification:
@kubernetes/sig-api-machinery-bugs.

In response to this:

@kubernetes/sig-api-machinery-bugs

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-github-robot k8s-github-robot removed the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Sep 6, 2017
@liggitt liggitt self-assigned this Sep 6, 2017
@liggitt
Copy link
Member

liggitt commented Sep 6, 2017

bisected to 6875e95

the stringification of the proxied location looks like it was normalizing the path and prepending a slash

that commit switched to copying the incoming request (which was being manually constructed from the parsed subresource path) as-is, which left the path missing a leading /

@liggitt
Copy link
Member

liggitt commented Sep 7, 2017

also, this appears to be limited to upgrade requests

@liggitt
Copy link
Member

liggitt commented Sep 7, 2017

and actually, the upgrade issue was introduced in 715d5d9#diff-1fbb2e342b0473af45e93e3e26031e80L226

@VasylSamoilov
Copy link
Author

I can confirm that my initial report was incorrect, only upgrade requests are affected.

@VasylSamoilov
Copy link
Author

VasylSamoilov commented Sep 7, 2017

Correct curl to reproduce the issue is to make websocket upgrade request:
curl -H "Upgrade: websocket" -H "Sec-Websocket-Version: 13" -H "Sec-Websocket-Extensions: permessage-deflate; client_max_window_bits" -H "Sec-Websocket-Key: rtBOKvTHtRAbTpz9vhyAng==" http://localhost:8080/api/v1/namespaces/default/services/testservice:http/proxy/test

k8s-github-robot pushed a commit that referenced this issue Sep 7, 2017
Automatic merge from submit-queue

Fix proxied request-uri to be valid HTTP requests

Fixes #52022, introduced in 1.7. Stringifying/re-parsing the URL masked that the path was not constructed with a leading `/` in the first place.

This makes upgrade requests proxied to pods/services via the API server proxy subresources be valid HTTP requests

```release-note
Fixes an issue with upgrade requests made via pod/service/node proxy subresources sending a non-absolute HTTP request-uri to backends
```
k8s-github-robot pushed a commit that referenced this issue Sep 8, 2017
…5-upstream-release-1.7-1504806739

Automatic merge from submit-queue

Automated cherry pick of #52065

Cherry pick of #52065 on release-1.7.

Fixes #52022, introduced in 1.7. Stringifying/re-parsing the URL masked that the path was not constructed with a leading `/` in the first place.

This makes upgrade requests proxied to pods/services via the API server proxy subresources be valid HTTP requests

```release-note
Fixes an issue with upgrade requests made via pod/service/node proxy subresources sending a non-absolute HTTP request-uri to backends
```
k8s-github-robot pushed a commit that referenced this issue Sep 23, 2017
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 #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
```
sttts pushed a commit to sttts/apimachinery that referenced this issue Sep 24, 2017
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 kubernetes/kubernetes#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
sttts pushed a commit to sttts/apiserver that referenced this issue Sep 24, 2017
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 kubernetes/kubernetes#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
sttts pushed a commit to sttts/apimachinery that referenced this issue Oct 13, 2017
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 kubernetes/kubernetes#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
sttts pushed a commit to sttts/apiserver that referenced this issue Oct 13, 2017
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 kubernetes/kubernetes#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
sttts pushed a commit to sttts/apimachinery that referenced this issue Oct 14, 2017
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 kubernetes/kubernetes#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
sttts pushed a commit to sttts/apiserver that referenced this issue Oct 14, 2017
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 kubernetes/kubernetes#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
sttts pushed a commit to sttts/apimachinery that referenced this issue Oct 16, 2017
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 kubernetes/kubernetes#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
sttts pushed a commit to sttts/apiserver that referenced this issue Oct 16, 2017
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 kubernetes/kubernetes#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
akhilerm pushed a commit to akhilerm/apimachinery that referenced this issue Sep 20, 2022
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 kubernetes/kubernetes#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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Projects
None yet
Development

No branches or pull requests

4 participants