api: support rate limit action based on cidr match #42845
Conversation
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
agrawroh
left a comment
agrawroh
left a comment
There was a problem hiding this comment.
Just curios, why can't we use the existing Dynamic Metadata for this? It should be trivial to write Dynamic Metadata for Remote Address and use that instead right?
|
@agrawroh the challenge here is not to apply rate limits to remote address ,in fact we already have the RemoteAddress rate limit action for this and can also use GenericKey with CEL without any dynamic metadata. But the usecase here is to apply different rate limits to remote addresses matching on some CIDRs. See the referred issue #36442 and also the related envoyproxy/gateway#4385 issue. |
Signed-off-by: Rudrakh Panigrahi <[email protected]>
rudrakhp
force-pushed
the
api_cidr_matcher
branch
from
75d5207 to
42e82a7
Compare
wbpcode
left a comment
wbpcode
left a comment
There was a problem hiding this comment.
Thanks for the contribution and some comments are added.
| // [#not-implemented-hide:] | ||
| // Specifies an address matcher that controls whether the rate limit action is applied. | ||
| // The matcher checks the remote address (trusted address from | ||
| // :ref:`x-forwarded-for <config_http_conn_man_headers_x-forwarded-for>`) | ||
| // against the specified CIDR ranges. The rate limit action will be applied if | ||
| // the remote address matches any of the CIDR ranges (or does not match any if | ||
| // ``invert_match`` is set to true in the address matcher). If this field is not | ||
| // specified, the rate limit action will be applied to all remote addresses. | ||
| type.matcher.v3.AddressMatcher address_matcher = 3; |
There was a problem hiding this comment.
@wbpcode While the original use case can be handled by RemoteAddressMatcher:
I have usecase that I should add ratelimiting to all addresses (0.0.0.0/0) EXCEPT few "whitelisted ips".
It cannot handle a slightly modified usecase where the rate limit bucket needs to be shared among IPs with common prefix.
There was a problem hiding this comment.
I will suggest to use the RemoteAddressMatch to cover this case and it should be possible?
There was a problem hiding this comment.
@wbpcode here is a more concrete usecase that I don't see how to solve using RemoteAddressMatcher, please do correct me if I am wrong:
To apply rate limiting based on a masked network prefix while exempting specific CIDR ranges from being aggregated into that limit.
E.g.
- [This is not possible in
RemoteAddressMatcher] When processing traffic from remote addresses10.1.1.2,10.1.1.15or10.1.10.23, it must generate the same descriptor so as to consume from the same10.1.0.0/16rate limit bucket. - But it must also have a whitelist so this descriptor is not created for
10.1.2.0/24and bypasses rate limit. So if remote address is10.1.2.58, although it is in the10.1.0.0/16range, rate limit descriptor will be skipped.
| // The following descriptor entry is appended to the descriptor: | ||
| // | ||
| // .. code-block:: cpp | ||
| // | ||
| // ("remote_address_match", "<descriptor_value>") | ||
| // [#not-implemented-hide:] | ||
| message RemoteAddressMatch { | ||
| // Descriptor value of entry. | ||
| // | ||
| // The same :ref:`format specifier <config_access_log_format>` as used for | ||
| // :ref:`HTTP access logging <config_access_log>` applies here, however | ||
| // unknown specifier values are replaced with the empty string instead of ``-``. | ||
| // | ||
| // .. note:: | ||
| // | ||
| // The format string can contain multiple valid substitution fields. If multiple substitution | ||
| // fields are present, their results will be concatenated to form the final descriptor value. | ||
| // If it contains no substitution fields, the value will be used as is. | ||
| // All substitution fields will be evaluated and their results concatenated. | ||
| // If the final concatenated result is empty and ``default_value`` is set, the ``default_value`` will be used. | ||
| // If ``default_value`` is not set and the result is empty, this descriptor will be skipped | ||
| // and not included in the rate limit call. | ||
| // | ||
| // For example, ``static_value`` will be used as is since there are no substitution fields. | ||
| // ``%REQ(:method)%`` will be replaced with the HTTP method, and | ||
| // ``%REQ(:method)%%REQ(:path)%`` will be replaced with the concatenation of the HTTP method and path. | ||
| // ``%CEL(request.headers['user-id'])%`` will use CEL to extract the user ID from request headers. | ||
| // | ||
| string descriptor_value = 1 [(validate.rules).string = {min_len: 1}]; | ||
|
|
||
| // The key to use in the descriptor entry. | ||
| // | ||
| // Defaults to ``remote_address_match``. | ||
| string descriptor_key = 2; | ||
|
|
||
| // An optional value to use if the final concatenated ``descriptor_value`` result is empty. | ||
| string default_value = 3; | ||
|
|
||
| // Specifies an address matcher that controls whether the rate limit action is applied. | ||
| // The matcher checks the remote address (trusted address from | ||
| // :ref:`x-forwarded-for <config_http_conn_man_headers_x-forwarded-for>`) | ||
| // against the specified CIDR ranges. The rate limit action will be applied if | ||
| // the remote address matches any of the CIDR ranges (or does not match any if | ||
| // ``invert_match`` is set to true in the address matcher). | ||
| type.matcher.v3.AddressMatcher address_matcher = 4 | ||
| [(validate.rules).message = {required: true}]; | ||
| } |
4 participants
Commit Message: [api] support rate limit action based on cidr match
Additional Description: Proposed API changes to support rate limit actions on remote address match. This includes additions of a new rate limit action
RemoteAddressMatchas well as inclusion of a CIDR matcher in theMaskedRemoteAddressaction.Risk Level: Low (doesn't break/affect existing usecases)
Testing: N/A (API changes only)
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A
[Optional Runtime guard:]
Related #36442
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
Optional API Considerations:
RemoteAddressMatchrate limit action will cater to most usecases, including for request's remote address using%DOWNSTREAM_REMOTE_ADDRESS%.MaskedRemoteAddressrate limit action as this usecases can't be directly supported inRemoteAddressMatch.type.matcher.v3.AddressMatcher. Might need to implementinvert_matchin existing usage atFilterStateIpRangeMatcher::match.