http local_ratelimit: add request_headers_to_add option #16076
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: William Fu <wfu@pinterest.com>
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
…oxy#16073) * Fix handling of timestamps in zipkin annotations / logs. The intended replacement from string to int was not happening as expected. this causes ingestion issues with non-lenient collectors, such as jaeger Signed-off-by: Douglas Reid <douglas-reid@users.noreply.github.com>
Signed-off-by: Yan Avlasov <yavlasov@google.com>
|
/assign-from @envoyproxy/api-shepherds |
|
@envoyproxy/api-shepherds assignee is @markdroth |
Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: Ryan Northey <ryan@synca.io> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: William Fu <wfu@pinterest.com>
rgs1
reviewed
Apr 20, 2021
There was a problem hiding this comment.
LGTM, just one nit.
Over to the @mattklein123 for final review & merging.
| // have been rate limited and are forwarded upstream. This can only occur when the | ||
| // filter is enabled but not enforced. | ||
| repeated config.core.v3.HeaderValueOption request_headers_to_add = 10 | ||
| [(validate.rules).repeated = {max_items: 10}]; |
There was a problem hiding this comment.
Probably move this next to response_headers_to_add?
There was a problem hiding this comment.
I put this last since this is the 10th proto field - keeping the binary encoding tags in sequential order
There was a problem hiding this comment.
I would agree with @rgs1 here. There is no requirement to keep fields in the order of their ids. But putting related fields together makes proto easier to read.
Remove deprecated runtime override envoy.reloadable_features.overload_manager_disable_keepalive_drain_http2. Multiplexing HTTP connections (HTTP/2, HTTP/3) will always be sent a GOAWAY when the overload action for disabling connection keepalive triggers. Signed-off-by: Alex Konradi <akonradi@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: William Fu <wfu@pinterest.com>
Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com>
Signed-off-by: Kevin Baichoo <kbaichoo@google.com>
…5859) Creates mechanisms to account for allocated bytes in buffers. This will be followed up in a PR wiring this through the H2 codec. Related envoyproxy#15791 Signed-off-by: Kevin Baichoo <kbaichoo@google.com>
Signed-off-by: Nitin <nigoyal@microsoft.com>
…y#16085) Signed-off-by: Teju Nareddy <nareddyt@google.com>
Signed-off-by: Ryan Northey <ryan@synca.io>
|
/wait-any |
adisuissa
reviewed
Apr 21, 2021
| @@ -91,4 +91,10 @@ message LocalRateLimit { | |||
| // | |||
| // The filter supports a range of 0 - 10 inclusively for stage numbers. | |||
| uint32 stage = 9 [(validate.rules).uint32 = {lte: 10}]; | |||
|
|
|||
| // Specifies a list of HTTP headers that should be added to each requests that | |||
There was a problem hiding this comment.
nit: s/requests/request/
in the resent version of rules_go, the issue bazel-contrib/rules_go#2771 was fixed. It should address the bazel build issue on some Linux or MacOS (bazelbuild/bazel#12986) Signed-off-by: izemlyanskiy <izemlyanskiy@pulsepoint.com>
…xy#16038) * Bring parity to the docker images in the install docs pages Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com>
…15958) * Fix the default sampling rate for AWS X-Ray tracer extension to 5% and test for the same if no localized sampling rule is applied. Update the release notes with the bug fix information Signed-off-by: Sunil Narasimhamurthy <sunnrs@amazon.com>
* Remove HCM stream error runtime override. Remove envoy.reloadable_features.hcm_stream_error_on_invalid_message now deprecated Signed-off-by: Alex Konradi <akonradi@google.com>
another breakout from envoyproxy#15833 Signed-off-by: Ryan Northey <ryan@synca.io>
This PR updates our QUICHE dependency to commit 88c8d5903d851744410ea9840201b6507feae981. Signed-off-by: David Schinazi <dschinazi@google.com>
Signed-off-by: Peter Jausovec <peter.jausovec@gmail.com>
Signed-off-by: Asra Ali <asraa@google.com> Commit Message: Temporary CI fix before envoyproxy#16074 Risk Level: Low Testing: Docs Changes: Release Notes: Platform Specific Features:
envoyproxy#16109) This change will not change the functionality, it just changes the internal implementation detail. JwKsCache holds both config and cache. Currently, the whole JwksCache object is in the thread local slot, but actually, only the cache data needs to be in in the thread local. This change will move the thread local data inside JwksDataImpl, only stores its {jwks, and expire} into thread local. Move JwksCache object out of thread local. This change is in preparation to support async fetch of remote_jwks proposed in envoyproxy#14556 (comment). Detail changes: * Created `tls` for {jwks, expire} inside JwksDataImpl * Removed `tls` for JwksCache in FilterConfigImpl * Removed `enable_shared_from_this` from FilterConfigImpl Risk Level: Low Testing: Unit-tested. Docs Changes: None Release Notes: None Signed-off-by: Wayne Zhang <qiwzhang@google.com>
…stream duration (envoyproxy#15585)" (envoyproxy#16133) This reverts commit ffa1680. Signed-off-by: Snow Pettersen <snowp@lyft.com>
aggregate dependabot updates and use pip-compile to resolve dependency hashes Signed-off-by: Ryan Northey <ryan@synca.io> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Dan Zhang <danzh@google.com>
* api: Update xds_protocol doc to remove v2 and cleanup Signed-off-by: Ryan Northey <ryan@synca.io>
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>
…/envoy into local-ratelimit-req-headers Signed-off-by: William Fu <wfu@pinterest.com>
williamsfu99
requested review from
alyssawilk,
antoniovicente,
cpakulski,
dio,
lambdai and
lizan
as code owners
repokitteh-read-only
bot
added
the
deps
|
Oops - tried to rebase - going to reopen a separate PR to avoid any forcepush |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: William Fu wfu@pinterest.com
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: