-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ratelimit: be able to disable x-envoy-ratelimited response header sent #13270
ratelimit: be able to disable x-envoy-ratelimited response header sent #13270
Conversation
…t in case of ratelimited request Signed-off-by: András Czigány <andras.czigany@strivacity.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API shepherd review
api/envoy/extensions/filters/http/ratelimit/v3/rate_limit.proto
Outdated
Show resolved
Hide resolved
api/envoy/extensions/filters/http/ratelimit/v3/rate_limit.proto
Outdated
Show resolved
Hide resolved
…der sent in case of ratelimited request Signed-off-by: András Czigány <andras.czigany@strivacity.com>
@andrascz do you mind merging master, the test failure should go away if you do so. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mostly lgtm, just a docs question
/wait |
@andrascz do you mind if I ask what your use case is for this change? |
…nse header sent in case of ratelimited request Signed-off-by: András Czigány <andras.czigany@strivacity.com>
…nvoy-ratelimited-header Signed-off-by: András Czigány <andras.czigany@strivacity.com>
@numerodix Our use case is that currently we are rate limiting on our Istio ingressgateway which handles our public production traffic and we do not want to send this header outside of the cluster. As the 429 response is a local reply and ResponseMapper does not have any options to remove a header, it seems the easiest way to achieve this. |
…d response header sent in case of ratelimited request Signed-off-by: András Czigány <andras.czigany@strivacity.com>
@numerodix any additional comments here? Were you asking out of curiosity? /wait-any |
@junr03 Was asking out of curiosity, yes. :) |
…nvoy-ratelimited-header Signed-off-by: András Czigány <andras.czigany@strivacity.com>
Signed-off-by: András Czigány <andras.czigany@strivacity.com>
Signed-off-by: András Czigány <andras.czigany@strivacity.com>
…nvoy-ratelimited-header Signed-off-by: András Czigány <andras.czigany@strivacity.com>
Signed-off-by: András Czigány <andras.czigany@strivacity.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API LGTM
Oops needs a main merge. /wait |
…nvoy-ratelimited-header Signed-off-by: András Czigány <andras.czigany@strivacity.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
* master: ci: use multiple stage (envoyproxy#13557) tls: update BoringSSL to 2192bbc8 (4240). (envoyproxy#13567) fix macos v8 build (envoyproxy#13572) Fixed Health Check Fuzz corpus syntax (envoyproxy#13576) ci: Remove shellcheck diff (envoyproxy#13560) ci: Increate brew retry interval (envoyproxy#13565) dependencies: fix some of the fallout from Wasm merge. (envoyproxy#13569) hds: add support for delta updates in specifier (envoyproxy#13067) ci: workaround for actions/runner-images#1811 (envoyproxy#13577) ratelimit: be able to disable x-envoy-ratelimited response header sent (envoyproxy#13270) Update opencensus library (envoyproxy#13549) ci: use azp for api and go-control-plane sync (envoyproxy#13550) docs: Remove/make generic lyft references in docs (envoyproxy#13559) check_format: adding 2 more release note checks (envoyproxy#13444) [Wasm] Add cluster metadata fallback and upstream host metadata (envoyproxy#13477) [fuzz] Added validation for secrets (envoyproxy#13543) Add Platform Specific Feature guidance to PR template (envoyproxy#13547) Signed-off-by: Michael Puncel <mpuncel@squareup.com>
Commit Message: make emitting of the X-Envoy-RateLimited header configurable
Signed-off-by: András Czigány andras.czigany@strivacity.com
Additional Description:
Risk Level: Low
Testing: unit and integration tests
Docs Changes: proto and current changelog
Release Notes: N/A see docs above