gateway-api icon indicating copy to clipboard operation
gateway-api copied to clipboard

GEP: Add Path Redirects and Rewrites

Open robscott opened this issue 4 years ago • 10 comments

What would you like to be added: This GEP attempts to cover 2 related but different features:

  • Path Redirects
  • Rewrites

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 https://github.com/kubernetes-sigs/gateway-api/issues/200 and https://github.com/kubernetes-sigs/gateway-api/issues/678.

robscott avatar Jul 19 '21 21:07 robscott

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:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot avatar Oct 17 '21 21:10 k8s-triage-robot

/lifecycle frozen

jpeach avatar Oct 17 '21 21:10 jpeach

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: https://github.com/istio/istio/issues/22290 & https://github.com/istio/api/pull/1854)

The feature is discussed in https://github.com/kubernetes-sigs/gateway-api/pull/731 but only mentions it as a "Future extension"

Thanks for helping me get some movement on this.

dacloutier avatar Dec 02 '21 09:12 dacloutier

@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!

youngnick avatar Dec 03 '21 05:12 youngnick

Hi @youngnick, thanks for the update and the clarification that it is on the Radar here.

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)

The POSIX basic and extended

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) Of course I would love to see a more elaborate version but in terms of transportability

we need to be able to do something that will work across as many as possible

this might be a very good first step that can be extended later.

rufreakde avatar Dec 04 '21 17:12 rufreakde

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.

youngnick avatar Dec 06 '21 01:12 youngnick

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

hbagdi avatar Dec 06 '21 18:12 hbagdi

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

Looks like Github ate some of your comment @hbagdi ?

youngnick avatar Dec 09 '21 23:12 youngnick

Looks like Github ate some of your comment @hbagdi ?

🤦🏾‍♂️ Words interpreted incorrectly as Markdown ( ). Fixed now.

hbagdi avatar Dec 10 '21 17:12 hbagdi

@hbagdi you mean some documentation for later use? Several Different Regex Engines: Overview

PCRE: Documentation

RE2: Documentation Github Issue talking about compatibility and the RE2:POSIX option

rufreakde avatar Dec 10 '21 17:12 rufreakde

@robscott as per our discussions at Kubecon, it would seem we want to move this into standard for v0.6.0, or maybe v0.6.1 is that right? Is this even the best issue to track this with?

shaneutt avatar Nov 02 '22 17:11 shaneutt

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.

robscott avatar Nov 07 '22 22:11 robscott

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:

  • [x] Implemented by several implementations.
  • [x] Conformance test framework is in place, with some coverage of basic functionality.
  • [x] Validation is well thought out.
  • [x] Most of the API surface is being exercised by users.
  • [ ] Approval from subproject owners + KEP reviewers.

~~I think we've accomplished most of the graduation criteria, but I'm looking for some help determining which implementations have support for this to see if we can check that box.~~

Update: It looks like we have enough implementations in place to graduate to standard channel.

So far I know the following have support:

  • Envoy Gateway
  • Istio
  • Contour

And the following do not yet:

  • GKE
  • Kong

@youngnick, @skriss, @pleshakov or anyone else, are there any additional implementations supporting this feature?

robscott avatar Feb 06 '23 22:02 robscott

@robscott at NGINX Kubernetes Gateway we have implemented request redirect filter but without support for the path field yet link

pleshakov avatar Feb 07 '23 00:02 pleshakov

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

skriss avatar Feb 07 '23 15:02 skriss

@robscott Envoy Gateway has implemented all of them now.

Xunzhuo avatar Feb 07 '23 15:02 Xunzhuo

I think my initial comment was a bit confusing, this GEP covers:

  1. Path Redirects
  2. Entirety of Rewrite filter

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 avatar Feb 13 '23 20:02 robscott

@robscott thanks for the clarification - Contour now has support for both, we got path redirects merged last week.

skriss avatar Feb 14 '23 01:02 skriss

What's next for us here? Does anyone specifically want to champion this one and push it forward?

shaneutt avatar Feb 21 '23 19:02 shaneutt

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.

robscott avatar Feb 22 '23 23:02 robscott

For Envoy Gateway, the HTTPRouteRedirectPath conformance test was disabled due to https://github.com/envoyproxy/gateway/issues/993.

danehans avatar Mar 14 '23 16:03 danehans

For Envoy Gateway, the HTTPRouteRedirectPath conformance test was disabled due to envoyproxy/gateway#993.

thanks for raising this @danehans We anticipate that once https://github.com/kubernetes-sigs/gateway-api/issues/1805 is addressed, Envoy Gateway should be able to support HTTPRouteRedirectPath.

arkodg avatar Mar 14 '23 18:03 arkodg

Why is Substitution not found here?

Bryce-huang avatar Aug 31 '23 10:08 Bryce-huang

@Bryce-huang I think you want #2177

robscott avatar Aug 31 '23 23:08 robscott