Skip to content
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

http local_ratelimit: add request_headers_to_add option #16178

Merged
merged 5 commits into from
Apr 29, 2021

Conversation

williamsfu99
Copy link
Contributor

Commit Message:
Introduce the ability to add request headers to forwarded upstream requests when the local HTTP rate limit filter determines (through its configured ruleset) that a request should be rate limited, but is not hard enforcing the incoming traffic.

Additional Description:
This is useful when a backend service should take customized action when a particular traffic volume is reached, rather than forcing request shedding by the proxy.

Risk Level: Low, adding a new feature
Testing: Modified unit tests
Docs Changes: Updated associated doc
Release Notes:
Platform Specific Features:

William Fu added 4 commits April 26, 2021 10:37
Signed-off-by: William Fu <wfu@pinterest.com>
Signed-off-by: William Fu <wfu@pinterest.com>
Signed-off-by: William Fu <wfu@pinterest.com>
Signed-off-by: William Fu <wfu@pinterest.com>
@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
envoyproxy/api-shepherds assignee is @htuch
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #16178 was opened by williamsfu99.

see: more, trace.

// Specifies a list of HTTP headers that should be added to each request that
// has been rate limited and is also forwarded upstream. This can only occur when the
// filter is enabled but not enforced.
repeated config.core.v3.HeaderValueOption request_headers_to_add = 10
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably needs a different name, maybe request_headers_to_add_when_not_enforced?

…orced

Signed-off-by: William Fu <wfu@pinterest.com>
@yanavlasov
Copy link
Contributor

Code LGTM. Waiting for API approval.

Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm api

@htuch htuch merged commit ce1cb0e into envoyproxy:main Apr 29, 2021
gokulnair pushed a commit to gokulnair/envoy that referenced this pull request May 6, 2021
…6178)

Introduce the ability to add request headers to forwarded upstream requests when the local HTTP rate limit filter determines (through its configured ruleset) that a request should be rate limited, but is not hard enforcing the incoming traffic.

Additional Description:
This is useful when a backend service should take customized action when a particular traffic volume is reached, rather than forcing request shedding by the proxy.

Risk Level: Low, adding a new feature
Testing: Modified unit tests
Docs Changes: Updated associated doc

Signed-off-by: William Fu <wfu@pinterest.com>
Signed-off-by: Gokul Nair <gnair@twitter.com>
gokulnair pushed a commit to gokulnair/envoy that referenced this pull request May 6, 2021
…6178)

Introduce the ability to add request headers to forwarded upstream requests when the local HTTP rate limit filter determines (through its configured ruleset) that a request should be rate limited, but is not hard enforcing the incoming traffic.

Additional Description:
This is useful when a backend service should take customized action when a particular traffic volume is reached, rather than forcing request shedding by the proxy.

Risk Level: Low, adding a new feature
Testing: Modified unit tests
Docs Changes: Updated associated doc

Signed-off-by: William Fu <wfu@pinterest.com>
Signed-off-by: Gokul Nair <gnair@twitter.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants