xds: Add ext_authz response handling - #12896
Conversation
Part 2 of the client-side ext_authz filter. Sits on top of #12493. Adds CheckResponseHandler, which interprets the CheckResponse from the authorization service. It evaluates OkHttpResponse vs DeniedHttpResponse, maps HTTP status codes to gRPC statuses, applies failure_mode_allow semantics when the authz server is unreachable, and validates decoder header mutations against the configured HeaderMutationRulesConfig. AuthzResponse is the resulting value object carrying the allow/deny decision, the gRPC status for denied calls, and any header/trailer mutations to apply.
| public class CheckResponseHandler { | ||
| private final HeaderMutationFilter headerMutationFilter; | ||
|
|
||
| public CheckResponseHandler(HeaderMutationFilter headerMutationFilter) { |
There was a problem hiding this comment.
Make the class and the constructor package-private.
| return AuthzResponse.allow( | ||
| HeaderMutations.create(ImmutableList.of(), ImmutableList.of())).build(); | ||
| } | ||
| OkHttpResponse okResponse = response.getOkResponse(); |
There was a problem hiding this comment.
nit: Can just inline the expression without using a local variable.
| return AuthzResponse.allow(allowedMutations.requestMutations()) | ||
| .setResponseHeaderMutations(allowedMutations.responseMutations()).build(); |
There was a problem hiding this comment.
Looking at this usage, the response mutations can also be an argument to AuthzResponse.allow instead of making it set EMPTY_MUTATIONS for response.
| } | ||
| DeniedHttpResponse deniedResponse = response.getDeniedResponse(); | ||
| CheckResponseMutations allowedMutations = | ||
| buildHeaderMutationsFromDeniedResponse(deniedResponse); |
There was a problem hiding this comment.
Since only response mutations is used for the denied case, can we either avoid calling this method that creates mutations for both request and response, or add a boolean argument to indicate skipping request mutations?
| } | ||
|
|
||
| @Test | ||
| public void handleResponse_ok_edgeCaseHeaders() { |
There was a problem hiding this comment.
nit: Can we use a more indicative name that tells what edge case it is?
|
|
||
| @Test | ||
| public void handleResponse_okWithMutations() { | ||
| HeaderValueOption option = |
There was a problem hiding this comment.
Can we also add envoy HeaderValueOptions for ADD_IF_ABSENT, OVERWRITE_IF_EXISTS_OR_ADD and OVERWRITE_IF_EXISTS and verify their translation in the same test?
2 participants
Part 2 of the client-side ext_authz filter. Sits on top of #12493.
Adds CheckResponseHandler, which interprets the CheckResponse from the authorization service. It evaluates OkHttpResponse vs DeniedHttpResponse, maps HTTP status codes to gRPC statuses, applies failure_mode_allow semantics when the authz server is unreachable, and validates decoder header mutations against the configured HeaderMutationRulesConfig.
AuthzResponse is the resulting value object carrying the allow/deny decision, the gRPC status for denied calls, and any header/trailer mutations to apply.