envoy
envoy copied to clipboard
envoyproxy
allow configuring max request buffer size without buffering it per route
Commit Message: allow configuring max request buffer size without buffering it per route Additional Description: With this feature, we can increase the buffer size for some routes which has filters that buffers the request optionally, for example, a WAF filter. Risk Level: Low Testing: Unit & Integration Docs Changes: Release Notes: Platform Specific Features: [Optional Runtime guard:] Fixes #32205 [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional API Considerations:]
As a reminder, PRs marked as draft will not be automatically assigned reviewers, or be handled by maintainer-oncall triage.
Please mark your PR as ready when you want it to be reviewed!
:cat:
Caused by: https://github.com/envoyproxy/envoy/pull/33412 was opened by spacewander.
![repokitteh-read-only[bot] avatar](https://avatars.githubusercontent.com/in/16729?v=4 "Published by a pub.dev verified publisher"){kind=small}
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @markdroth
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).
:cat:
Caused by: https://github.com/envoyproxy/envoy/pull/33412 was opened by spacewander.
The discussion in #32205 seems to have landed on adding this to the buffer filter instead of adding a knob to the route config. I think that approach would be better than adding a new top-level field like this.
/wait
The discussion in #32205 seems to have landed on adding this to the buffer filter instead of adding a knob to the route config. I think that approach would be better than adding a new top-level field like this.
/wait
There is a later discussion happened in https://github.com/envoyproxy/envoy/pull/32571. Sorry for not sync it to the original discussion issue.
@alyssawilk explained a new idea about adding it to the route part: https://github.com/envoyproxy/envoy/pull/32571#pullrequestreview-1917411023
I think it's reasonable so I create a new PR here.
The discussion in #32205 seems to have landed on adding this to the buffer filter instead of adding a knob to the route config. I think that approach would be better than adding a new top-level field like this. /wait
There is a later discussion happened in #32571. Sorry for not sync it to the original discussion issue.
@alyssawilk explained a new idea about adding it to the route part: #32571 (review)
I think it's reasonable so I create a new PR here.
May I ask why you can't set the buffer limit in your WAF filter?
The discussion in #32205 seems to have landed on adding this to the buffer filter instead of adding a knob to the route config. I think that approach would be better than adding a new top-level field like this. /wait
There is a later discussion happened in #32571. Sorry for not sync it to the original discussion issue. @alyssawilk explained a new idea about adding it to the route part: #32571 (review) I think it's reasonable so I create a new PR here.
May I ask why you can't set the buffer limit in your WAF filter?
@soulxu Two reasons:
- The WAF filter is written in WASM and there is no way to set limit in Wasm code.
- There can be a general solution, not just apply to WAF filter, but also to any filter which buffering the data optionally.
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!
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
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!
@soulxu Would you take a look at this again?
sorry, I missed your message, I will try to give a review tomorrow
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!