search

envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
24.27k stars 4.69k forks source link

Need 3XX Location URL rewrite feature #12591

Open shivanshu21 opened 3 years ago

shivanshu21 commented 3 years ago

Title:

Description: We use an encode filter to rewrite location URL today in eBay traffic engineering, but this looks like a feature that should be supported from upstream Envoy side.

We are using Envoy 1.13. Is this feature already available in master? Is it required upstream?

What we are looking for is:

  1. Upstream sends some location header and 302.
  2. Envoy has an option for the route where it can rewrite the location header to a different value.
shivanshu21 commented 3 years ago

https://github.com/envoyproxy/envoy/issues/10259

shivanshu21 commented 3 years ago

Tried posting this in Slack too. No response.

How about we introduce a new route action called 3xx_location_rewrite here: https://github.com/envoyproxy/envoy/blob/master/api/envoy/config/route/v3/route_components.proto#L837

And implement this over here: Filter::onUpstreamHeaders() https://github.com/envoyproxy/envoy/blob/master/source/common/router/router.cc#L1295 Based on whether response_code is 3xx (or more specifically 301/302).

@mattklein123 @htuch ?

mattklein123 commented 3 years ago

cc @alyssawilk any thoughts on ^?

alyssawilk commented 3 years ago

I think it's reasonable, especially if we can use the same API and libraries that we use for rewrites on the request URL.

Is there anything which might vary based for location headers based on incoming requests such that we'd want to perform the rewrite for cache responses as well? I'd hope not, but ynk.

shivanshu21 commented 3 years ago

@mattklein123 @alyssawilk Should we convert this from question to an issue? I will raise a PR this week for adding this.

alyssawilk commented 3 years ago

SGTM, thanks. I'll assign it your way!

ccravens commented 3 years ago

@shivanshu21 this is great work! I'm concerned that if it's only specific to location headers, could this not be more generalized for any HTTP header by just specifying 1) the header to modify (ie - Location) 2) Regex specifying what needs to be replaced and with what.

What are your thoughts? thanks!

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

ToniCipriani commented 3 years ago

Any updates on whether this is coming?

shivanshu21 commented 3 years ago

The PR was halted for Unified match predicates to be implemented.

This issue got expanded into a generalized header rewrite use case. Let me check if the unified match predicate work is done then I can rebase.

https://github.com/envoyproxy/envoy/pull/12938

snuggie12 commented 2 years ago

Is that https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-headermatcher-string-match ??