Implement source IP RateLimit#1129
Conversation
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #1129 +/- ##
==========================================
+ Coverage 62.55% 62.57% +0.01%
==========================================
Files 72 72
Lines 9596 9646 +50
==========================================
+ Hits 6003 6036 +33
- Misses 3179 3192 +13
- Partials 414 418 +4
... and 2 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
internal/xds/translator/ratelimit.go
Outdated
There was a problem hiding this comment.
from the spec it looks like the masked remote address is being sent to RLS https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#config-route-v3-ratelimit-action-maskedremoteaddress
how are individual IPs getting rate limited in a unique/distinct way ?
There was a problem hiding this comment.
this boils down to a top level API question, when a user specifies 192.168.1.0/24and a limit of 2 requests/hour, does this mean 192.168.1.1 and 192.168.1.2 are treated as the same client so limit is reached when 192.168.1.1 sends 1 request and 192.168.1.2 sends another request within the same hour ?
Or
is 192.168.1.1 and 192.168.1.2 allowed 2 requests each ?
There was a problem hiding this comment.
in that case you should use 192.168.1.1/32 and 192.168.1.1/32
There was a problem hiding this comment.
thanks for clarifying , we should clarify this in the API
gateway/api/v1alpha1/ratelimitfilter_types.go
Line 101 in 27d14f8
All IP Addresses within the specified CIDR are treated as a single client selector and share the rate limit value
In the future, we could add a Distinct match type so each IP is treated in a unique way
There was a problem hiding this comment.
MaskedRemoteAddress is usefull for both Global and Local ratelimit.
In istio, we want to use local ratelimit between pod and pod, pod ip is short live, marked_remote_address will be very useful in this scenario.
b1ce0e8 to
c491704
Compare
Signed-off-by: hejianpeng <hejianpeng2@huawei.com>
Signed-off-by: hejianpeng <hejianpeng2@huawei.com>
c491704 to
4a1c512
Compare
|
hey @zirain has this been tested end to end ? would be great to track it with a GH issue so we track docs completion for this feature |
Signed-off-by: hejianpeng <hejianpeng2@huawei.com>
internal/xds/translator/testdata/in/ratelimit-config/masked-remote-address-match.yaml
Outdated
Show resolved
Hide resolved
|
open #1144 |
arkodg
left a comment
There was a problem hiding this comment.
LGTM, thanks for building out this useful feature !
Xunzhuo
left a comment
There was a problem hiding this comment.
LGTM, thanks! Can you update the ratelimit doc to describe this?
fixes: #852