api: Support suppressing the 'x-envoy-' headers added by the HTTP router

[liorokman]

What type of PR is this?
This PR enhances the ClientTrafficPolicy by adding the following option:

   # Suppress the "x-envoy-" headers from being added by the HTTP router.
   suppressEnvoyHeaders: true

What this PR does / why we need it:

This PR adds the ability to suppress the "x-envoy-" headers added by the Envoy HTTP router.

The following headers are affected:

• Upstream:
  • x-envoy-attempt-count
  • x-envoy-is-timeout-retry
  • x-envoy-expected-rq-timeout-ms
  • x-envoy-original-path
  • x-envoy-upstream-stream-duration-ms
• Downstream:
  • x-envoy-upstream-service-time
  • x-envoy-overloaded

Which issue(s) this PR fixes:
Support for this flag is mentioned in #1846

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (243c42c) 64.80% compared to head (736f187) 64.73%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2358      +/-   ##
==========================================
- Coverage   64.80%   64.73%   -0.08%     
==========================================
  Files         113      113              
  Lines       16567    16567              
==========================================
- Hits        10737    10725      -12     
- Misses       5159     5168       +9     
- Partials      671      674       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[zhaohuabing]

This will change both the downstream and upstream behavior. Would it be appropriate to put this in ClientTraffciPolicy?

[liorokman]

The relevant Envoy Proxy configuration is this: https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/http/router/v3/router.proto#extensions-filters-http-router-v3-router .

It affects both upstream and downstream. It's not possible to affect just the upstream or just the downstream - Envoy Proxy doesn't give that option.

I think that both ClientTrafficPolicy and BackendTrafficPolicy are equally unfit, but that ClientTrafficPolicy is still the better fit. I think that users that look for something like this are looking for a way to prevent their downstream clients from receiving these headers.

Do you think there's a better place in the Envoy Gateway configuration to specify this?

[Xunzhuo]

/retest

[zhaohuabing]

I think that both ClientTrafficPolicy and BackendTrafficPolicy are equally unfit, but that ClientTrafficPolicy is still the better fit. I think that users that look for something like this are looking for a way to prevent their downstream clients from receiving these headers.

Looks like this option only suppresses the headers generated by the router.

x-envoy-attempt-count
x-envoy-is-timeout-retry
x-envoy-expected-rq-timeout-ms
x-envoy-original-path
x-envoy-upstream-stream-duration-ms

And there are many more "x-envoy-xxx" headers generated by other filters, here is the full list: https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_conn_man/headers

So I wonder whether this will work for the use case you mentioned or not.

Do you think there's a better place in the Envoy Gateway configuration to specify this?
Unfortunately, I don't have one either. EG doesn't have a home for such an option that affects both the downstream and upstream. I agree that we can settle with one of them.

[liorokman]

So I wonder whether this will work for the use case you mentioned or not.

Looking at the list of headers mentioned in the HTTP connection manager documentation:

• x-envoy-downstream-service-cluster is only added if the add_user_agent option is set to true
• x-envoy-downstream-service-node is only added if the add_user_agent is set to true
• x-envoy-force-trace is relevant if it's already set in the incoming request
• x-envoy-internal was only sent upstream in my tests, and removed from the response, so no issues here.
• x-envoy-original-dst-host needs to be enabled explicitly with the use_http_header configuration.

This only leaves the x-envoy-external-address header.

[zhaohuabing]

LGTM. Defer to other @envoyproxy/gateway-maintainers

[zirain]

LGTM

[liorokman]

@zirain @zhaohuabing Can I submit a PR that implements this?

[zirain]

This's a small changes, you can do the implement here, or you can send another one later.