conn_manager: add overload header if request is dropped due to OM
Commit Message: Add x-envoy-overloaded flag to local reply sent by connection manager in case Overload Manager led to the need of shedding some load. Added an API field in connection manager to allow the suppression of the x-envoy-overload flag.
Additional Description:
This is different from the handling in the http filter where x-envoy-overloaded flag added when maintenance mode is enabled or when the upstream circuit breaking is triggered.
The connection manager does not initialize the filters when "envoy overload" is detected thus requiring special handling.
N.B: For the time being I used the existing x-envoy-overloaded header, if you prefer I can introduce a different header exclusive to the case where the envoy itself is overloaded (e.g: x-envoy-local-overloaded).
To prevent malicious actors from gaining information about Envoy's status, a flag has been added in the API to suppress the header.
Fixes #15106
Risk Level: Low Testing: Added unit-tests and integration test for verifying the header being added Docs Changes: Done Release Notes: Done Platform Specific Features: -
Hi @leosarra, welcome and thank you for your contribution.
We will try to review your Pull Request as quickly as possible.
In the meantime, please take a look at the contribution guidelines if you have not done so already.
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @adisuissa
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).
Thank you everyone of the feedback :). I will soon have some bandwidth and I will start addressing all the comments
Based on the feedback the following changes were performed:
- The header that is added in case envoy itself is overloaded (based on OM) is now "x-envoy-local-overloaded", I do not reuse anymore the "x-envoy-overloaded" header which remains exclusive to upstream circuit breaking usecases
- The "x-envoy-local-overloaded" is no longer added by default, it is controlled by a skip_local_overload_append flag in connection manager (default: true)
- doc changed accordingly
- test that the header is not added by default. I'm a bit lost on how to add a test to verify that the header is added when the flag is set, I'm having difficulty in mocking connectionManagerConfig for conn_manager_impl_test_2.cc :(
I'm still implementing the integration tests
I have rebased and made the requested changes. Test coverage via integration tests has been added as well. Let me know if it looks good to you
Thank you all of you for the feedback :)
/assign-from @envoyproxy/senior-maintainers