-
Notifications
You must be signed in to change notification settings - Fork 39.8k
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
Fix proxied request-uri to be valid HTTP requests #52065
Conversation
Test? |
789ca69
to
4bd692e
Compare
Non-upgrade requests are tested in the e2e proxy tests already. I tested upgrade request proxying manually. Adding upgrade proxy tests would require a websocket test image... can open a follow-up issue for that |
4bd692e
to
e9d8976
Compare
/lgtm |
@liggitt here's the unit test error:
|
e9d8976
to
9648f1c
Compare
fixed unit tests to go through the constructor |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: liggitt, ncdc Associated issue: 52022 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
/test all [submit-queue is verifying that this PR is safe to merge] |
Automatic merge from submit-queue |
…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 ```
Commit found in the "release-1.7" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked. |
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 ```
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