internal redirects: support passing headers from response to request #30793
Conversation
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
|
I'm not particularly excited about the field name, so I'm happy to take suggestions for a better name if desired. Also, I feel like I don't understand the lifetimes for the header maps very well, so I defaulted to using SetCopy(). I'm happy to change those if that helps at all. |
|
At least one of the failed checks looks to be a flake in the test infrastructure, btw. |
|
/lgtm api |
docs/root/intro/arch_overview/http/http_connection_management.rst
Outdated
Show resolved
Hide resolved
|
/wait on CI and comments |
pugmajere
force-pushed
the
pugmajere-internal-redirects-copy-headers-to-request
branch
from
77ee6a9
to
ff92313
Compare
|
I've been fighting a bit with the pre-push checks because they don't work on M2 Macs, which is my primary development environment for this work. I think things will pass now, at least. |
This adds a new (repeated) field in the internal redirect policy, "response_headers_to_preserve". When set, the headers named there will be copied from the response that triggers an internal redirect into the request that follows. This allows some limited information passing through the internal redirect system. The current system is faithful to the idea that internal redirects are purely a latency optimization, and should behave similarly to if the redirect had been passed to the downstream user-agent. This does violate that idea. Other proxies, such as Nginx, have a much more flexible way of handling internal redirects that allows a fair bit of information passing like this. This should allow implementations to adopt Envoy that are using this kind of information passing, with reduced needs to rearchitect. Fixes: envoyproxy#30441 Fixes: envoyproxy#16777 Signed-off-by: Ryan Anderson <ryan.anderson@snowflake.com> Signed-off-by: Ryan Anderson <ryan@michonline.com>
Signed-off-by: Ryan Anderson <ryan@michonline.com>
Signed-off-by: Ryan Anderson <ryan@michonline.com>
Signed-off-by: Ryan Anderson <ryan@michonline.com>
Signed-off-by: Ryan Anderson <ryan@michonline.com>
Signed-off-by: Ryan Anderson <ryan@michonline.com>
Signed-off-by: Ryan Anderson <ryan@michonline.com>
Signed-off-by: Ryan Anderson <ryan@michonline.com>
Signed-off-by: Ryan Anderson <ryan@michonline.com>
Signed-off-by: Ryan Anderson <ryan@michonline.com>
Signed-off-by: Ryan Anderson <ryan@michonline.com>
Signed-off-by: Ryan Anderson <ryan@michonline.com>
Signed-off-by: Ryan Anderson <ryan@michonline.com>
Signed-off-by: Ryan Anderson <ryan@michonline.com>
Signed-off-by: Ryan Anderson <ryan@michonline.com>
|
LGTM, though it looks like this needs a main merge. |
Signed-off-by: Ryan Anderson <ryan@michonline.com>
|
/wait |
…-request Signed-off-by: Ryan Anderson <ryan@michonline.com>
|
Fixed the merge conflicts in the changelog since I think this is hopefully done now. |
…-request Signed-off-by: Ryan Anderson <ryan@michonline.com>
|
I don't think the latest changes caused any of these test failures, so if someone can retrigger the tests, that would be great. (The tests failed after I did the conflict-merge for the changelog, which ... seems unlikely to cause the failures seen, lol.) |
|
/retry-azp (Not sure if that will work, but worth a shot) |
|
/retest :-) |
pugmajere
deleted the
pugmajere-internal-redirects-copy-headers-to-request
branch
Commit Message:
This adds a new (repeated) field in the internal redirect policy, "response_headers_to_copy". When set, the headers named there will be copied from the response that triggers an internal redirect into the request that follows.
This allows some limited information passing through the internal redirect system.
The current system is faithful to the idea that internal redirects are purely a latency optimization, and should behave similarly to if the redirect had been passed to the downstream user-agent. This does violate that idea.
Other proxies, such as Nginx, have a much more flexible way of handling internal redirects that allows a fair bit of information passing like this. This should allow implementations to adopt Envoy that are using this kind of information passing, with reduced needs to rearchitect.
Risk Level: Low
Testing: Various tests added. Manual test done as well.
Docs Changes: Docs updated to reflect the additional features.
Release Notes: Release notes updated.
Fixes: #30441
Fixes: #16777
API Considerations: Requires configuration, so default is no behavior change. Otherwise, I believe everything else is covered or not relevant.