api: support rate limit action based on cidr match

[rudrakhp]

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 RemoteAddressMatch as well as inclusion of a CIDR matcher in the MaskedRemoteAddress action.
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:

• RemoteAddressMatch rate limit action will cater to usecases that need to match on remote address.
• Need to enhance substitution formatter to support %MASKED_REMOTE_ADDRESS(prefix_len)%.
• Reusing existing type.matcher.v3.AddressMatcher. Need to implement invert_match in existing usage at FilterStateIpRangeMatcher::match.

[repokitteh-read-only]

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @adisuissa
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #42845 was opened by rudrakhp.

see: more, trace.

[agrawroh]

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?

[rudrakhp]

@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.

[wbpcode]

Thanks for the contribution and some comments are added.

[rudrakhp]

/retest

[adisuissa]

Thanks!
I think that matching is a good feature to have, I just wonder whether it could be more generic.

[adisuissa]

I think this new field is confusing because there are already remote_address and remote_address_match. I think calrifying the reasoning behind this feature is the first step. I'm not an expert in this domain, but I would suggest one of the following:

1. consider adding options to RemoteAddress and MaskedRemoteAddress that enables the matching (the reasoning behind this is that I think that matching is more generic and orthogonal to whether the matched object is a remote address, a request header, etc).
2. See if this should be a typed-extension (using the extension field).

cc @wbpcode due to the comment in #36442

[rudrakhp]

@adisuissa thanks for the comment. This is supposed to handle all the remote address matching usecases in a generic way by leveraging CEL and substitution formatting in descriptor_value. For example:

• Using %DOWNSTREAM_REMOTE_ADDRESS% here will handle the existing RemoteAddress usecase with CIDR matching.
• We might need to introduce formatter like %MASKED_DOWNSTREAM_REMOTE_ADDRESS(len)% to handle MaskedRemoteAddress usecase with CIDR matching. See feat: add formatters for masked IP addresses #42969 .

@wbpcode is aligned with the RemoteAddressMatch approach (see comments above), if the name is confusing I will think of some alternatives.

[rudrakhp]

^ bump @adisuissa @wbpcode
How does remote_address_range_match / remote_address_value_match sound instead of remote_address_match?

[adisuissa]

@wbpcode is the expert on substitution formatters. I'm just presenting my thoughts on design and combination of features.

[rudrakhp]

Sure, thanks for your inputs!