-
Notifications
You must be signed in to change notification settings - Fork 503
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
GEP: Add Path Redirects and Rewrites #726
Comments
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
/lifecycle frozen |
How can I go about requesting a change to the gateway-api to support regex/capture group Path rewrites? Istio won't merge the PR that would support this until it gets added to the gateway api.... and many people ( including me ;D ) need regex path and capture group rewrites. (see: istio/istio#22290 & istio/api#1854) The feature is discussed in #731 but only mentions it as a "Future extension" Thanks for helping me get some movement on this. |
@dacloutier, thanks for the feedback. I understand this feature is important to you and a lot of people, and the reason we've left it for a future extension in #731 is that it's actually pretty complicated to write a standard way for it to work, that will work across multiple proxies/Gateway implementations, and still allow you to migrate your config between Gateway implementations (which is a primary goal of this project). We definitely plan to get to this, but need to make sure that whatever will do will be usable to a broad base of people. Just as a quick example, it's very common for different implementations to use different regex flavors, but for the Gateway API, we need to be able to do something that will work across as many as possible. Also, what we've found up until this point is that some things that you would think are pretty basic functionality can vary wildly between implementations and make it very difficult for there to be a single, standard way to implement them. Timeouts for HTTP connections are a great example of this - every implementation has there own set of timeouts that mean subtly different things, and we haven't been able to build a good common set to be able to make a HTTPRoute TimeoutPolicy. Given that regex is already pretty complicated, and there are different flavors, and different implementations, we need to be a very careful with the design. I know "we're going to get to it" is a very unsatisfying answer, but unfortunately, that's all we have at the moment. I certainly think that your enthusiasm here does a great job of pushing it up my priority list. 😄 I should also say that, as always, contributions are welcomed - often the hardest part is getting the discussion started! |
Hi @youngnick, Just wanted to drop this here as well. (And I am sure you already looked at this stuff yourself but I guess it does not harm) even though it is not very popular as it is rather limited it is often a subset of many Regex flavors. And for the basic Regex Rewrite this standard would mostly suffice as a solution here. (Just my honest opinion does not need to be the case though)
this might be a very good first step that can be extended later. |
Thanks @rufreakde, you make a good point about using POSIX regex as a probably-compatible flavor, we'll make sure we keep that in mind. |
In case anyone is interested in contributing, a thorough document that ensures that POSIX regexes (or even a subset) is supported and has the same semantics across PCRE, RE2, < other possible regex engines found in implementations >. |
Looks like Github ate some of your comment @hbagdi ? |
Looks like Github ate some of your comment @hbagdi ? 🤦🏾♂️ Words interpreted incorrectly as Markdown ( ). Fixed now. |
@hbagdi you mean some documentation for later use? PCRE: RE2: |
@robscott as per our discussions at Kubecon, it would seem we want to move this into standard for |
As much as I'd love to fit this in, we don't have conformance test coverage for this yet. Since we're already running behind on the v0.6.0 release, I'm going to bump this to the v0.7.0 release. |
Hey everyone, we're hoping to graduate this feature to standard release channel as part of v0.7.0. Here's where I think we are with graduation criteria:
Update: It looks like we have enough implementations in place to graduate to standard channel. So far I know the following have support:
And the following do not yet:
@youngnick, @skriss, @pleshakov or anyone else, are there any additional implementations supporting this feature? |
@robscott Contour has largely implemented both, the only field I know we don't yet have implemented is redirect path rewrites which is marked experimental. |
@robscott Envoy Gateway has implemented all of them now. |
I think my initial comment was a bit confusing, this GEP covers:
Both are currently marked as experimental and we're trying to find implementations that support them so we can graduate to beta. @skriss does your comment above mean that Contour supports 2 but not 1? |
@robscott thanks for the clarification - Contour now has support for both, we got path redirects merged last week. |
What's next for us here? Does anyone specifically want to champion this one and push it forward? |
I believe the next steps are to confirm that we have at least 2 implementations that pass conformance tests for this feature. It sounds like we probably do based on the comments above. If that's the case, we should look into graduating this to standard in v0.7.0. |
For Envoy Gateway, the HTTPRouteRedirectPath conformance test was disabled due to envoyproxy/gateway#993. |
thanks for raising this @danehans |
Why is Substitution not found here? |
@Bryce-huang I think you want #2177 |
What would you like to be added:
This GEP attempts to cover 2 related but different features:
Once this GEP is complete, HTTPRoute should include filters that enable basic path and hostname redirects and rewrites. Although there is definitely variation among implementations as far as what is possible, there appears to be enough common ground for a portable API.
Why is this needed:
This is a common feature request, as noted by #200 and #678.
The text was updated successfully, but these errors were encountered: