router: Make direct response body size configurable#13487
router: Make direct response body size configurable#13487dio wants to merge 3 commits intoenvoyproxy:masterfrom dio:max-body-size
Conversation
This patch adds max_body_size_bytes to set the maximum bytes of the direct response body size. Signed-off-by: Dhi Aurrahman <dio@tetrate.io>
zuercher
left a comment
There was a problem hiding this comment.
One minor nit.
More of an API question, but I wonder if this should be done as a runtime flag rather than an API field?
|
I have no strong preference, I can change it to get the value from a specified |
I don't have a strong opinion on whether we make this a full config field or a runtime field. Do we think people will use this limit on a per-route basis or just want a global override? @alyssawilk any thoughts? |
|
If this is just OOM avoidance, I think a single value is probably good enough. |
|
@alyssawilk the use-case presented in #13422 is to make it larger than the current limit. |
|
Fair, same goes for OOM avoidance avoidance :-P Unless someone asks to configure it per-route, I think we can have one value. |
|
One value is fine to start with. I'd vote to make it part of the API as it makes it possible to reason about limits consistently, e.g. when looking at a config like https://www.envoyproxy.io/docs/envoy/latest/configuration/best_practices/edge#best-practices-edge |
|
Config sounds good. We could put it at the top level of the route config, or possible on the HCM config and wire it through to the route. |
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
@zuercher I would also like this change to be merged, is there anything I can do to help? |
|
@dio can you open a fresh PR and let's get this merged? |
|
Yes, sure. Sorry this was slipped under my radar :(. |
This patch adds max_direct_response_body_size_bytes to set the maximum bytes of the direct response body size (in bytes). The config is added as a field in RouteConfiguration. Reviving #13487 with a slightly different approach (add the config to RouteConfiguration instead of directly per direct response config entry). Risk Level: Low, since the default behavior is preserved. Testing: Updated to test the newly introduced config. Docs Changes: Updated. Release Notes: Added. Fixes #13422 Signed-off-by: Dhi Aurrahman <dio@rockybars.com>
Commit Message: This patch adds
max_body_size_bytesto set the maximum bytes of the direct response body size.Risk Level: Low, since the default behavior is preserved.
Testing: Updated to test the newly introduced config.
Docs Changes: Updated.
Release Notes: Added.
Fixes #13422
Signed-off-by: Dhi Aurrahman dio@tetrate.io