ext_authz: add method_override to HttpService#44564
Conversation
Adds a method_override field to the ext_authz HttpService config that, when set, replaces the HTTP method on the outgoing authorization request. Without this, the auth server must accept every method the upstream receives (GET, DELETE, PATCH, etc.) rather than exposing a single fixed endpoint like POST /auth. Fixes envoyproxy#5357 Signed-off-by: Derek Argueta <derek.argueta@airbnb.com>
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
|
CI failed due to a 504 gateway timeout when downloading |
|
/retest |
adisuissa
left a comment
adisuissa
left a comment
There was a problem hiding this comment.
Thanks.
Please explain how does this fix #5357 (which AFIACT discusses other things other than overriding method).
After reviewing this PR I think I'm missing something fundamental. I've re-read the PR description, but I'm confused why gRPC is mentioned.
| under the filter's configured name with a ``.shadow`` suffix | ||
| (``envoy.filters.http.ext_authz.shadow`` by default) so that a subsequent filter can read | ||
| and optionally enforce it. | ||
| - area: lua |
There was a problem hiding this comment.
Seems that this file includes many changes that don't need to be here. Can you please clean this?
| // Only one of ``path_prefix`` or ``path_override`` may be set. | ||
| string path_override = 10; | ||
|
|
||
| // Overrides the HTTP method of the authorization request sent to the authorization service. |
There was a problem hiding this comment.
high-level question:
not sure I fully understand why the method needs an override or why it is dependent on the incoming request... Shouldn't it just be decided by the contents of server_uri?
| } else { | ||
| headers->addCopy(key, header.raw_value()); | ||
| } | ||
| } else if (key == Http::Headers::get().Method && !config_->methodOverride().empty()) { |
There was a problem hiding this comment.
Why is the else if needed here? if there's a :path header doesn't the :method need to be overridden?
3 participants
Fixes #5357.
The HTTP ext_authz filter forwards the original request's method to the authorization server verbatim. This means the auth server has to accept every HTTP method the upstream receives (GET, POST, DELETE, PATCH, etc.) rather than being able to expose a simple fixed endpoint like
POST /auth. The gRPC ext_authz implementation doesn't have this problem since it always calls a single RPC.This adds a
method_overridefield toHttpServicethat, when set, substitutes the specified method on the outgoing authorization request. It pairs naturally with the existingpath_overridefield — together they let you target a single fixed auth endpoint regardless of the original request.method_override = 11onHttpService,next-free-fieldbumped to 12validateMethodOverride()rejects values containing whitespace; applied in bothencode_raw_headersand legacy header paths inRawHttpClientImpl::check()HttpServiceconstructorRisk Level: low
Testing: unit & integration tests added
Docs Changes: done
Release Notes: done
Platform Specific Features: no