router: Support request header addition/removal and host header rewrite for mirroring #41178
Conversation
Signed-off-by: Ben Plotnick <[email protected]>
Signed-off-by: Ben Plotnick <[email protected]>
Signed-off-by: Ben Plotnick <[email protected]>
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
Signed-off-by: Ben Plotnick <[email protected]>
Signed-off-by: Ben Plotnick <[email protected]>
Signed-off-by: Ben Plotnick <[email protected]>
| // Specifies a list of HTTP headers that should be added to each mirrored request. | ||
| // Headers specified here take precedence over any headers with the same name | ||
| // from the original request. For more information, including details on header value syntax, see | ||
| // the documentation on :ref:`custom request headers | ||
| // <config_http_conn_man_headers_custom_request_headers>`. | ||
| repeated core.v3.HeaderValueOption request_headers_to_add = 7 | ||
| [(validate.rules).repeated = {max_items: 1000}]; | ||
|
|
||
| // Specifies a list of HTTP headers that should be removed from each request that is sent to the | ||
| // mirror cluster. | ||
| repeated string request_headers_to_remove = 8 [(validate.rules).repeated = { | ||
| items {string {well_known_regex: HTTP_HEADER_NAME strict: false}} | ||
| }]; | ||
|
|
||
| // Indicates that during mirroring, the host header will be swapped with this value. | ||
| // :ref:`disable_shadow_host_suffix_append | ||
| // <envoy_v3_api_field_config.route.v3.RouteAction.RequestMirrorPolicy.disable_shadow_host_suffix_append>` | ||
| // is implicitly enabled if this field is set. | ||
| string host_rewrite_literal = 9 | ||
| [(validate.rules).string = {well_known_regex: HTTP_HEADER_VALUE strict: false}]; |
There was a problem hiding this comment.
Could upstream http filter be another choice? Or could we to reuse our message HeaderMutation then we can control the order of header addition/removal and also could support remove_on_match?
There was a problem hiding this comment.
Good idea. Moved to using HeaderMutation
wrt upstream filters, they are equally applied to regular and shadowed requests. so I think the only way we could do this would be if we 1) applied separate filter chains for shadow or 2) had a special upstream filter (or perhaps extended the existing header mutation filter to understand whether the request was shadowed?)
There was a problem hiding this comment.
wrt upstream filters, they are equally applied to regular and shadowed requests. so I think the only way we could do this would be if we 1) applied separate filter chains for shadow or 2) had a special upstream filter (or perhaps extended the existing header mutation filter to understand whether the request was shadowed?)
The upstream filters could configured at cluster, I think you only need a specific cluster for shadow traffic (and obviously we will have a shadow cluster, right?)
If possible, I will prefer to use the existing mechanism to address your requirements :)
There was a problem hiding this comment.
I'm only seeing upstream network filters at the cluster level, not http filters.
But it doesn't matter anyway. For both use-cases i need this for, we are mirroring to an existing cluster that has other uses. Imagine doing dual writes, but with two routes where one routes primary and mirrors to follower, and the other routes to follower and mirrors to primary.
Signed-off-by: Ben Plotnick <[email protected]>
Signed-off-by: Ben Plotnick <[email protected]>
Signed-off-by: Ben Plotnick <[email protected]>
Signed-off-by: Ben Plotnick <[email protected]>
Signed-off-by: Ben Plotnick <[email protected]>
| // Specifies a list of header mutations that should be applied to each mirrored request. | ||
| // Header mutations are applied in the order they are specified. For more information, including | ||
| // details on header value syntax, see the documentation on :ref:`custom request headers | ||
| // <config_http_conn_man_headers_custom_request_headers>`. | ||
| repeated common.mutation_rules.v3.HeaderMutation request_mutations = 7 |
There was a problem hiding this comment.
| // Specifies a list of header mutations that should be applied to each mirrored request. | |
| // Header mutations are applied in the order they are specified. For more information, including | |
| // details on header value syntax, see the documentation on :ref:`custom request headers | |
| // <config_http_conn_man_headers_custom_request_headers>`. | |
| repeated common.mutation_rules.v3.HeaderMutation request_mutations = 7 | |
| // Specifies a list of header mutations that should be applied to each mirrored request. | |
| // Header mutations are applied in the order they are specified. For more information, including | |
| // details on header value syntax, see the documentation on :ref:`custom request headers | |
| // <config_http_conn_man_headers_custom_request_headers>`. | |
| repeated common.mutation_rules.v3.HeaderMutation request_headers_mutations = 7 |
In case possible trailer change in the future. :)
| router_->onDestroy(); | ||
| } | ||
|
|
||
| TEST_P(RouterShadowingTest, ShadowWithAdvancedHeaderMutations) { |
There was a problem hiding this comment.
I think this test is unnecessary because it should be covered by HeaderEvaluator self's test. We needn't to test these logic again and again at every position where the HeaderMutation is used.
Commit Message: router: Allow request header mutations and host header rewrite for mirroring
Additional Description:
Risk Level: Low
Testing: unit/integ tests
Docs Changes:
Release Notes: Added support for
request_mutationsinrequest_mirror_policiesto enable header manipulation for mirror requests. Added support forhost_rewrite_literalinrequest_mirror_policiesto enable host header rewrite for mirror requests.Platform Specific Features: