rate-limit: make 429 response mapping configurable #4879

venilnoronha · 2018-10-27T03:34:25Z

Description: This PR enables the configuration of the mapping that translates 429 response code to a gRPC status code. By default, the Rate Limit filter in Envoy translates a 429 HTTP response code to UNAVAILABLE as specified in the gRPC mapping document. Google, however, recommends translating a 429 response to RESOURCE_EXHAUSTED. This PR provides a flag named
rate_limited_as_resource_exhausted in the RateLimit config which allows users to explicitly specify whether they want 429 responses to be mapped to RESOURCE_EXHAUSTED, while UNAVAILABLE remains the default.

References:

Risk Level: Low
Testing: Added 2 new tests
Docs Changes: N/A
Release Notes: Added a note about the configuration
Fixes #4735

/cc @ramaraochavali @lizan

This commit enables the configuration of the mapping that translates 429 response code to a gRPC status code. By default, the Rate Limit filter in Envoy translates a 429 HTTP response code to UNAVAILABLE as specified in the gRPC mapping document. Google, however, recommends translating a 429 response to RESOURCE_EXHAUSTED. This commit provides a flag named rate_limited_as_resource_exhausted in the RateLimit config which allows users to explicitly specify whether they want 429 responses to be mapped to RESOURCE_EXHAUSTED, while UNAVAILABLE remains the default. References: * https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md * https://cloud.google.com/apis/design/errors#generating_errors Signed-off-by: Venil Noronha <[email protected]>

include/envoy/http/filter.h

Signed-off-by: Venil Noronha <[email protected]>

include/envoy/grpc/status.h

source/common/http/utility.cc

Signed-off-by: Venil Noronha <[email protected]>

stale · 2018-11-06T18:03:04Z

This pull request has been automatically marked as stale because it has not had activity in the last 7 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!

venilnoronha · 2018-11-06T18:11:54Z

not stale.

mattklein123 · 2018-11-06T18:17:32Z

@lizan @junr03 ping on review.

lizan

LGTM, sorry for the delay.

junr03

thanks, just a few comments

api/envoy/config/filter/http/rate_limit/v2/rate_limit.proto

junr03 · 2018-11-06T19:16:09Z

include/envoy/http/filter.h

   */
  virtual void sendLocalReply(Code response_code, const std::string& body_text,
-                              std::function<void(HeaderMap& headers)> modify_headers) PURE;
+                              std::function<void(HeaderMap& headers)> modify_headers,
+                              const absl::optional<Grpc::Status::GrpcStatus> grpc_status) PURE;


I have been thinking more and more about our changes to the filter API. Every time we change this API we are making every person that has written thirdparty filters update. I don't know if there are better solutions to make argument extension in these functions easier/less painful for consumers. I am curious what the rest of the @envoyproxy/maintainers thoughts are about this.

We are tracking this concern here: #3390. Right now we decided that we are OK with API changes so that we can continue to move relatively fast. This will likely not always be the case.

#3390 is for this, I think now we are OK but need release notes for extension impacting API changes.

test/common/http/utility_test.cc

docs/root/intro/version_history.rst

* Add release note on HTTP filter API change * Update rate_limit proto comment * Update test to use utility function instead of hard-coded string Signed-off-by: Venil Noronha <[email protected]>

The Rate Limit filter in Envoy translates a 429 HTTP response code to UNAVAILABLE as specified in the gRPC mapping document, but Google recommends translating it to RESOURCE_EXHAUSTED (see https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md) This commit introduces a new setting to allow contour to forward the same parameter introduced in envoyproxy/envoy#4879 The default value is disabled to retain the original behaviour of returning UNAVAILABLE, as changing it would be a breaking change.

The Rate Limit filter in Envoy translates a 429 HTTP response code to UNAVAILABLE as specified in the gRPC mapping document, but Google recommends translating it to RESOURCE_EXHAUSTED (see https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md) This commit introduces a new setting to allow contour to forward the same parameter introduced in envoyproxy/envoy#4879 The default value is disabled to retain the original behaviour of returning UNAVAILABLE, as changing it would be a breaking change. Signed-off-by: Victor Roldan Betancort <[email protected]>

The Rate Limit filter in Envoy translates a 429 HTTP response code to UNAVAILABLE as specified in the gRPC mapping document, but Google recommends translating it to RESOURCE_EXHAUSTED (see https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md) This commit introduces a new setting to allow contour to forward the same parameter introduced in envoyproxy/envoy#4879 The default value is disabled to retain the original behaviour of returning UNAVAILABLE, as changing it would be a breaking change. Signed-off-by: Víctor Roldán Betancort <[email protected]>

The Rate Limit filter in Envoy translates a 429 HTTP response code to UNAVAILABLE as specified in the gRPC mapping document, but Google recommends translating it to RESOURCE_EXHAUSTED (see https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md) This commit introduces a new setting to allow contour to forward the same parameter introduced in envoyproxy/envoy#4879 The default value is disabled to retain the original behaviour of returning UNAVAILABLE, as changing it would be a breaking change. Closes #4901. Signed-off-by: Víctor Roldán Betancort <[email protected]>

…ectcontour#4971) The Rate Limit filter in Envoy translates a 429 HTTP response code to UNAVAILABLE as specified in the gRPC mapping document, but Google recommends translating it to RESOURCE_EXHAUSTED (see https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md) This commit introduces a new setting to allow contour to forward the same parameter introduced in envoyproxy/envoy#4879 The default value is disabled to retain the original behaviour of returning UNAVAILABLE, as changing it would be a breaking change. Closes projectcontour#4901. Signed-off-by: Víctor Roldán Betancort <[email protected]> Signed-off-by: yy <[email protected]>

…ectcontour#4971) The Rate Limit filter in Envoy translates a 429 HTTP response code to UNAVAILABLE as specified in the gRPC mapping document, but Google recommends translating it to RESOURCE_EXHAUSTED (see https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md) This commit introduces a new setting to allow contour to forward the same parameter introduced in envoyproxy/envoy#4879 The default value is disabled to retain the original behaviour of returning UNAVAILABLE, as changing it would be a breaking change. Closes projectcontour#4901. Signed-off-by: Víctor Roldán Betancort <[email protected]>

Signed-off-by: yy <[email protected]> add some unit test Signed-off-by: yy <[email protected]> git rebase Signed-off-by: yy <[email protected]> expose configuration for envoy's RateLimitedAsResourceExhausted (projectcontour#4971) The Rate Limit filter in Envoy translates a 429 HTTP response code to UNAVAILABLE as specified in the gRPC mapping document, but Google recommends translating it to RESOURCE_EXHAUSTED (see https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md) This commit introduces a new setting to allow contour to forward the same parameter introduced in envoyproxy/envoy#4879 The default value is disabled to retain the original behaviour of returning UNAVAILABLE, as changing it would be a breaking change. Closes projectcontour#4901. Signed-off-by: Víctor Roldán Betancort <[email protected]> Signed-off-by: yy <[email protected]> rebase Signed-off-by: yy <[email protected]> update tracing config validate Signed-off-by: yy <[email protected]> make generate Signed-off-by: yy <[email protected]> add chengelog Signed-off-by: yy <[email protected]> update make general Signed-off-by: yy <[email protected]> goimport Signed-off-by: yy <[email protected]> update tracing Signed-off-by: yy <[email protected]> fix golint Signed-off-by: yy <[email protected]> update test Signed-off-by: yy <[email protected]> delete unused code Signed-off-by: yy <[email protected]> delete error file Signed-off-by: yy <[email protected]> update changelog Signed-off-by: yy <[email protected]> fix some mistake Signed-off-by: yy <[email protected]>

Signed-off-by: yy <[email protected]> add some unit test Signed-off-by: yy <[email protected]> git rebase Signed-off-by: yy <[email protected]> expose configuration for envoy's RateLimitedAsResourceExhausted (projectcontour#4971) The Rate Limit filter in Envoy translates a 429 HTTP response code to UNAVAILABLE as specified in the gRPC mapping document, but Google recommends translating it to RESOURCE_EXHAUSTED (see https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md) This commit introduces a new setting to allow contour to forward the same parameter introduced in envoyproxy/envoy#4879 The default value is disabled to retain the original behaviour of returning UNAVAILABLE, as changing it would be a breaking change. Closes projectcontour#4901. Signed-off-by: Víctor Roldán Betancort <[email protected]> Signed-off-by: yy <[email protected]> rebase Signed-off-by: yy <[email protected]> update tracing config validate Signed-off-by: yy <[email protected]> make generate Signed-off-by: yy <[email protected]> add chengelog Signed-off-by: yy <[email protected]> update make general Signed-off-by: yy <[email protected]> goimport Signed-off-by: yy <[email protected]> update tracing Signed-off-by: yy <[email protected]> fix golint Signed-off-by: yy <[email protected]> update test Signed-off-by: yy <[email protected]> delete unused code Signed-off-by: yy <[email protected]> delete error file Signed-off-by: yy <[email protected]> update changelog Signed-off-by: yy <[email protected]> fix some mistake Signed-off-by: yy <[email protected]> feat: Add HTTP support for External Auth (projectcontour#4994) Support globally configuring an external auth server which is enabled by default for all vhosts, both HTTP and HTTPS. Closes projectcontour#4954. Signed-off-by: claytonig <[email protected]> Signed-off-by: yy <[email protected]> refactor DAG and DAG consumers to support >2 Listeners (projectcontour#5128) Updates projectcontour#4960. Signed-off-by: Steve Kriss <[email protected]> Signed-off-by: yy <[email protected]> resolve conflict Signed-off-by: yy <[email protected]> fix Signed-off-by: yy <[email protected]>

venilnoronha force-pushed the config-grpc-status branch 2 times, most recently from ab2877a to 7f58656 Compare October 27, 2018 03:42

venilnoronha force-pushed the config-grpc-status branch from 7f58656 to 44967f0 Compare October 27, 2018 03:44

lizan self-assigned this Oct 27, 2018

lizan suggested changes Oct 27, 2018

View reviewed changes

include/envoy/http/filter.h Show resolved Hide resolved

mattklein123 assigned junr03 Oct 27, 2018

filters: take optional status map input

161ba4d

Signed-off-by: Venil Noronha <[email protected]>

venilnoronha force-pushed the config-grpc-status branch from b7cd9e7 to 161ba4d Compare October 29, 2018 22:28

Replace Grpc::StatusMap with Status::GrpcStatus

bd1201e

Signed-off-by: Venil Noronha <[email protected]>

ramaraochavali reviewed Oct 30, 2018

View reviewed changes

include/envoy/grpc/status.h Outdated Show resolved Hide resolved

ramaraochavali reviewed Oct 30, 2018

View reviewed changes

source/common/http/utility.cc Outdated Show resolved Hide resolved

Revert httpToGrpcStatus API changes

d6a69ba

Signed-off-by: Venil Noronha <[email protected]>

mattklein123 mentioned this pull request Nov 5, 2018

router: support for rate limit retry policy #4946

Closed

stale bot added the stale stalebot believes this issue/PR has not been touched recently label Nov 6, 2018

stale bot removed the stale stalebot believes this issue/PR has not been touched recently label Nov 6, 2018

lizan approved these changes Nov 6, 2018

View reviewed changes

junr03 reviewed Nov 6, 2018

View reviewed changes

lizan suggested changes Nov 6, 2018

View reviewed changes

docs/root/intro/version_history.rst Show resolved Hide resolved

Address review comments

104bc64

* Add release note on HTTP filter API change * Update rate_limit proto comment * Update test to use utility function instead of hard-coded string Signed-off-by: Venil Noronha <[email protected]>

lizan approved these changes Nov 6, 2018

View reviewed changes

mattklein123 approved these changes Nov 7, 2018

View reviewed changes

mattklein123 merged commit f71a883 into envoyproxy:master Nov 7, 2018

pwen mentioned this pull request Oct 12, 2020

Expose rate_limited_as_resource_exhausted in Ambassador configuration emissary-ingress/emissary#3016

Closed

BrendanGraham14 mentioned this pull request May 26, 2022

Support rate_limited_as_resource_exhausted for local rate limit events #21461

Closed

vroldanbet mentioned this pull request Jan 10, 2023

expose configuration for envoy's RateLimitedAsResourceExhausted projectcontour/contour#4971

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rate-limit: make 429 response mapping configurable #4879

rate-limit: make 429 response mapping configurable #4879

venilnoronha commented Oct 27, 2018 •

edited

Loading

stale bot commented Nov 6, 2018

venilnoronha commented Nov 6, 2018

mattklein123 commented Nov 6, 2018

lizan left a comment

junr03 left a comment

junr03 Nov 6, 2018

mattklein123 Nov 6, 2018

lizan Nov 6, 2018

rate-limit: make 429 response mapping configurable #4879

rate-limit: make 429 response mapping configurable #4879

Conversation

venilnoronha commented Oct 27, 2018 • edited Loading

stale bot commented Nov 6, 2018

venilnoronha commented Nov 6, 2018

mattklein123 commented Nov 6, 2018

lizan left a comment

Choose a reason for hiding this comment

junr03 left a comment

Choose a reason for hiding this comment

junr03 Nov 6, 2018

Choose a reason for hiding this comment

mattklein123 Nov 6, 2018

Choose a reason for hiding this comment

lizan Nov 6, 2018

Choose a reason for hiding this comment

venilnoronha commented Oct 27, 2018 •

edited

Loading