expose configuration for envoy's RateLimitedAsResourceExhausted by vroldanbet · Pull Request #4971 · projectcontour/contour

vroldanbet · 2023-01-10T18:30:52Z

The Rate Limit filter in Envoy translates a 429 HTTP response code to UNAVAILABLE as specified in the gRPC mapping document. Alternatively, Envoy allows translating it to a less ambiguous RESOURCE_EXHAUSTED.

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.

This PR is largely inspired in the work done at #3457

FYI @sunjayBhatia as we discussed about this in #4901

Closes #4901.

github-actions · 2023-01-10T18:32:27Z

Hi @vroldanbet! Welcome to our community and thank you for opening your first Pull Request. Someone will review it soon. Thank you for committing to making Contour better. You can also join us on our mailing list and in our channel in the Kubernetes Slack Workspace

skriss · 2023-01-11T19:34:24Z

Thanks for the PR @vroldanbet! Please run make generate and commit the results to get the codegen CI check to pass.

skriss · 2023-01-11T19:40:19Z

Please also address https://github.com/projectcontour/contour/actions/runs/3895937483/jobs/6651861120, the output has instructions to follow. Thanks!

vroldanbet · 2023-01-11T20:16:58Z

@skriss done! Ready for another CI run!

examples/contour/01-crds.yaml

skriss · 2023-01-12T15:53:19Z

Looks like one UT still needs to be updated for the new field (https://github.com/projectcontour/contour/blob/main/cmd/contour/servecontext_test.go#L623-L645), otherwise this looks good to me

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 <vroldanbet@authzed.com>

vroldanbet · 2023-01-12T19:33:37Z

@skriss just pushed the fix, thanks for your patience and prompt feedback 🙏🏻

codecov · 2023-01-12T19:50:27Z

Codecov Report

Merging #4971 (5ecbdb7) into main (63dd999) will increase coverage by 0.00%.
The diff coverage is 65.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4971   +/-   ##
=======================================
  Coverage   77.61%   77.61%           
=======================================
  Files         140      140           
  Lines       16871    16875    +4     
=======================================
+ Hits        13094    13098    +4     
  Misses       3520     3520           
  Partials      257      257

Impacted Files	Coverage Δ
cmd/contour/serve.go	`22.31% <0.00%> (-0.04%)`	⬇️
pkg/config/parameters.go	`86.61% <ø> (ø)`
cmd/contour/servecontext.go	`80.91% <100.00%> (+0.05%)`	⬆️
internal/envoy/v3/ratelimit.go	`100.00% <100.00%> (ø)`
internal/xdscache/v3/listener.go	`91.33% <100.00%> (+0.02%)`	⬆️
internal/sorter/sorter.go	`98.97% <0.00%> (+0.51%)`	⬆️

skriss

LGTM, thanks @vroldanbet!

…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 <vroldanbet@authzed.com> Signed-off-by: yy <yang.yang@daocloud.io>

…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 <vroldanbet@authzed.com>

Signed-off-by: yy <yang.yang@daocloud.io> add some unit test Signed-off-by: yy <yang.yang@daocloud.io> git rebase Signed-off-by: yy <yang.yang@daocloud.io> 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 <vroldanbet@authzed.com> Signed-off-by: yy <yang.yang@daocloud.io> rebase Signed-off-by: yy <yang.yang@daocloud.io> update tracing config validate Signed-off-by: yy <yang.yang@daocloud.io> make generate Signed-off-by: yy <yang.yang@daocloud.io> add chengelog Signed-off-by: yy <yang.yang@daocloud.io> update make general Signed-off-by: yy <yang.yang@daocloud.io> goimport Signed-off-by: yy <yang.yang@daocloud.io> update tracing Signed-off-by: yy <yang.yang@daocloud.io> fix golint Signed-off-by: yy <yang.yang@daocloud.io> update test Signed-off-by: yy <yang.yang@daocloud.io> delete unused code Signed-off-by: yy <yang.yang@daocloud.io> delete error file Signed-off-by: yy <yang.yang@daocloud.io> update changelog Signed-off-by: yy <yang.yang@daocloud.io> fix some mistake Signed-off-by: yy <yang.yang@daocloud.io>

Signed-off-by: yy <yang.yang@daocloud.io> add some unit test Signed-off-by: yy <yang.yang@daocloud.io> git rebase Signed-off-by: yy <yang.yang@daocloud.io> 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 <vroldanbet@authzed.com> Signed-off-by: yy <yang.yang@daocloud.io> rebase Signed-off-by: yy <yang.yang@daocloud.io> update tracing config validate Signed-off-by: yy <yang.yang@daocloud.io> make generate Signed-off-by: yy <yang.yang@daocloud.io> add chengelog Signed-off-by: yy <yang.yang@daocloud.io> update make general Signed-off-by: yy <yang.yang@daocloud.io> goimport Signed-off-by: yy <yang.yang@daocloud.io> update tracing Signed-off-by: yy <yang.yang@daocloud.io> fix golint Signed-off-by: yy <yang.yang@daocloud.io> update test Signed-off-by: yy <yang.yang@daocloud.io> delete unused code Signed-off-by: yy <yang.yang@daocloud.io> delete error file Signed-off-by: yy <yang.yang@daocloud.io> update changelog Signed-off-by: yy <yang.yang@daocloud.io> fix some mistake Signed-off-by: yy <yang.yang@daocloud.io> 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 <claytonivorgonsalves@gmail.com> Signed-off-by: yy <yang.yang@daocloud.io> refactor DAG and DAG consumers to support >2 Listeners (projectcontour#5128) Updates projectcontour#4960. Signed-off-by: Steve Kriss <krisss@vmware.com> Signed-off-by: yy <yang.yang@daocloud.io> resolve conflict Signed-off-by: yy <yang.yang@daocloud.io> fix Signed-off-by: yy <yang.yang@daocloud.io>

vroldanbet requested a review from a team as a code owner January 10, 2023 18:30

vroldanbet requested review from stevesloka and tsaarni and removed request for a team January 10, 2023 18:30

vroldanbet force-pushed the add-ratelimit-grpc-error-code-setting branch from d0d279d to bcc63fa Compare January 10, 2023 18:32

vroldanbet force-pushed the add-ratelimit-grpc-error-code-setting branch from bcc63fa to 0a3c88d Compare January 10, 2023 18:35

vroldanbet mentioned this pull request Jan 10, 2023

Question: Returning custom gRPC header or status in rate limiting #4901

Closed

skriss requested review from skriss and sunjayBhatia January 11, 2023 19:13

skriss added the release-note/small A small change that needs one line of explanation in the release notes. label Jan 11, 2023

vroldanbet force-pushed the add-ratelimit-grpc-error-code-setting branch 2 times, most recently from 6c32bfb to 147084d Compare January 11, 2023 20:15

skriss requested changes Jan 11, 2023

View reviewed changes

examples/contour/01-crds.yaml Outdated Show resolved Hide resolved

vroldanbet force-pushed the add-ratelimit-grpc-error-code-setting branch 2 times, most recently from 9a9b5e9 to 939e866 Compare January 12, 2023 12:09

vroldanbet force-pushed the add-ratelimit-grpc-error-code-setting branch from 939e866 to 5ecbdb7 Compare January 12, 2023 19:33

sunjayBhatia approved these changes Jan 12, 2023

View reviewed changes

vroldanbet requested review from skriss and removed request for stevesloka and tsaarni January 12, 2023 20:19

skriss approved these changes Jan 12, 2023

View reviewed changes

skriss merged commit c06623a into projectcontour:main Jan 12, 2023

vroldanbet deleted the add-ratelimit-grpc-error-code-setting branch January 12, 2023 22:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

expose configuration for envoy's RateLimitedAsResourceExhausted#4971

expose configuration for envoy's RateLimitedAsResourceExhausted#4971
skriss merged 1 commit intoprojectcontour:mainfrom
vroldanbet:add-ratelimit-grpc-error-code-setting

vroldanbet commented Jan 10, 2023 •

edited by skriss

Loading

Uh oh!

github-actions bot commented Jan 10, 2023

Uh oh!

skriss commented Jan 11, 2023

Uh oh!

skriss commented Jan 11, 2023

Uh oh!

vroldanbet commented Jan 11, 2023

Uh oh!

Uh oh!

skriss commented Jan 12, 2023

Uh oh!

vroldanbet commented Jan 12, 2023

Uh oh!

codecov bot commented Jan 12, 2023

Uh oh!

skriss left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

vroldanbet commented Jan 10, 2023 • edited by skriss Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 10, 2023

Uh oh!

skriss commented Jan 11, 2023

Uh oh!

skriss commented Jan 11, 2023

Uh oh!

vroldanbet commented Jan 11, 2023

Uh oh!

Uh oh!

skriss commented Jan 12, 2023

Uh oh!

vroldanbet commented Jan 12, 2023

Uh oh!

codecov bot commented Jan 12, 2023

Codecov Report

Uh oh!

skriss left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vroldanbet commented Jan 10, 2023 •

edited by skriss

Loading