-
Notifications
You must be signed in to change notification settings - Fork 518
Ingress proposal: path rewriting #287
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
Closed
Closed
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
f7697c1
Added a networking proposal about path rewriting
l0rd badca6b
networking/path-rewriting: fixing examples
l0rd c7a4a29
Updated Routes Path Rewriting Support title
l0rd a78b4fd
moved path-rewrite.md to network/ingress folder
l0rd 1361e62
Move path-rewriting.md in enhancements/ingress
l0rd 0ceed35
feat(annotation): Provides more help on this annotation
benoitf e9416f9
Merge pull request #1 from benoitf/add-doc
l0rd 98ef927
fix(doc): Provides a link to the PR providing the documentation
benoitf 7ded79a
Merge pull request #2 from benoitf/path-rewrite-fix
l0rd 4c40aa0
Added route rewrite-path test plan
l0rd File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,115 @@ | ||
--- | ||
title: path-rewriting-support | ||
authors: | ||
- "@l0rd" | ||
reviewers: | ||
- TBD | ||
approvers: | ||
- TBD | ||
creation-date: 2020-04-17 | ||
last-updated: 2020-04-17 | ||
status: provisional | ||
--- | ||
|
||
# Routes Path Rewriting Support | ||
|
||
## Release Signoff Checklist | ||
|
||
- [ ] Enhancement is `implementable` | ||
- [ ] Design details are appropriately documented from clear requirements | ||
- [ ] Test plan is defined | ||
- [ ] Graduation criteria for dev preview, tech preview, GA | ||
- [ ] User-facing documentation is created in [openshift-docs](https://github.com/openshift/openshift-docs/) | ||
|
||
## Summary | ||
|
||
Some Ingress controller implementations support doing path rewriting. This is | ||
used to modify the path in the HTTP request as it gets proxied: | ||
|
||
`http://example.com/path` ---> `http://example.com/` | ||
|
||
This is a proposal to support path rewriting for Routes as well. | ||
|
||
## Motivation | ||
|
||
To use non wildcard SSL certificates we want to direct traffic based on the path | ||
rather then hostname (e.g. `http://example.com/app` instead of | ||
`http://app.example.com/`). | ||
|
||
We want to remove the path from the HTTP requests (`http://example.com/path` --> | ||
`http://example.com/`) because the upstream services are serving on /. | ||
|
||
There is an open issue about that: <https://github.com/openshift/origin/issues/20474> | ||
|
||
## Proposal | ||
|
||
### User Stories | ||
|
||
[CodeReady Workspaces](https://developers.redhat.com/products/codeready-workspaces/overview) | ||
is a controller that dynamically creates developers workspaces (web IDEs). A | ||
workspace is a web applications with N microservices exposing N routes (N | ||
depends on the workspace but is usually ~5). | ||
|
||
When a developer starts his workspace, N routes are created. When he stops it | ||
the N routes are deleted. | ||
|
||
**Wildcard certificates problems** | ||
|
||
By default workspaces routes have URLs with random subdomains like | ||
https://<random-part>.example.com. We create a lots of these routes and | ||
that's not an issue if the customer is allowed to use a wildcard TLS | ||
certificate (https://*.example.com). | ||
|
||
But not all customers can use wildcard certs. For that reason we now support | ||
fixed domain scenario using URLs with a fixed hostname and a variable path: | ||
https://example.com/<random-part>/. We have been able to implement this | ||
single-host option because upstream Ingress controllers support path rewriting. | ||
For instance with the traefik and nginx controllers adding the following annotations does | ||
the trick: | ||
|
||
`traefik.ingress.kubernetes.io/rewrite-target: /` | ||
`nginx.ingress.kubernetes.io/rewrite-target: /$1` | ||
|
||
We would like to support these annotations for Routes as well. | ||
We don't need support of regex path matching as `Route.spec.path` is not using regex as well. | ||
|
||
**Rewrite Target documentation** | ||
Documentation proposal at https://github.com/openshift/openshift-docs/pull/22021 | ||
|
||
**Annotation example** | ||
|
||
``` | ||
kind: Route | ||
metadata: | ||
name: example | ||
annotations: | ||
haproxy.router.openshift.io/rewrite-target: / | ||
``` | ||
|
||
## Design Details | ||
|
||
### Test Plan | ||
|
||
Considered the multiple supported examples of rewrite-path we are going | ||
to add some unit test to verify them. We are NOT going to include any particular | ||
e2e test. | ||
|
||
## Implementation History | ||
|
||
- 2020-05-12 Draft PRs submitted on https://github.com/openshift/router/pull/129 and | ||
https://github.com/openshift/openshift-docs#22021 | ||
|
||
## Alternatives | ||
|
||
We can fix this on the CRW side: we can deploy a reverse proxy per every CRW | ||
installation (https://crw.example.com) and use it to route requests coming from | ||
users browsers. | ||
|
||
However that means shifting a responsibility that is an OpenShift responsibility | ||
(creating endpoints) to CRW (and that's not its purpose). That also means missing | ||
an opportunity to help those OpenShift customers that, as we do, are requesting | ||
path rewriting. | ||
|
||
I think that supporting path rewriting on OpenShift Route controller is a better | ||
option. If OpenShift teams do not have the bandwidth to work on it we (CRW team) | ||
can work on the proposal and implementation. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need capture? The
Route
API'sspec.path
value is not a regexp.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @Miciah I've added a paragraph about that we don't need capture.
But it was an example with nginx annotation. I've added example for the openshift annotation and added as well an example with Traefik