envoyproxy
Ratelimiting doesn't work with both clientSelectors (i.e. headers and sourceCIDR) enabled
Description:
What issue is being seen? Describe what should be happening instead of the bug, for example: Envoy should not crash, the expected value isn't returned, etc. Ratelimiting doesn't work with both clientSelectors (i.e. headers and sourceCIDR) enabled. I would expect it to work if the headers match and the request is from the IP in the sourceCIDR range.
Repro steps:
Include sample requests, environment, etc. All data and inputs required to reproduce the bug. Followed all steps from quickstart. This is my ratelimit config:
rateLimit:
global:
rules:
- clientSelectors:
- headers:
- name: x-user-id
value: one
sourceCIDR:
type: Exact
value: 0.0.0.0/0
limit:
requests: 3
unit: Hour
type: Global
Note: If there are privacy concerns, sanitize the data prior to sharing.
Environment:
Include the environment like gateway version, envoy version and so on.
Logs:
Include the access logs and the Envoy logs.
These are the envoy-ratelimit logs, note this line descriptor does not match any limit, no limits applied.
time="2024-09-27T00:56:04Z" level=debug msg="CertProvider: waiting for runtime update"
time="2024-09-27T00:56:04Z" level=info msg="Starting xDS client connection for rate limit configurations"
time="2024-09-27T00:56:04Z" level=info msg="Dialing xDS Management Server: 'envoy-gateway:18001'"
time="2024-09-27T00:56:04Z" level=warning msg="connecting to redis on redis.redis-system.svc.cluster.local:6379 with pool size 10"
time="2024-09-27T00:56:04Z" level=debug msg="Implicit pipelining enabled: false"
time="2024-09-27T00:56:04Z" level=warning msg="Listening for debug on '0.0.0.0:6070'"
time="2024-09-27T00:56:04Z" level=warning msg="Listening for gRPC on '0.0.0.0:8081'"
time="2024-09-27T00:56:04Z" level=debug msg="Waiting for config update event"
time="2024-09-27T00:56:04Z" level=warning msg="Listening for HTTP on '0.0.0.0:8080'"
time="2024-09-27T00:56:04Z" level=info msg="Connection to xDS Management Server is successful"
time="2024-09-27T00:56:04Z" level=debug msg="loading domain: default/eg/http"
time="2024-09-27T00:56:04Z" level=debug msg="loading descriptor: key=default/eg/http.httproute/default/http-ratelimit/rule/0/match/0/ratelimit_example_httproute/default/http-ratelimit/rule/0/match/0/ratelimit_example"
time="2024-09-27T00:56:04Z" level=debug msg="Creating stats for key: 'default/eg/http.httproute/default/http-ratelimit/rule/0/match/0/ratelimit_example_httproute/default/http-ratelimit/rule/0/match/0/ratelimit_example.masked_remote_address_0.0.0.0/0'"
time="2024-09-27T00:56:04Z" level=debug msg="loading descriptor: key=default/eg/http.httproute/default/http-ratelimit/rule/0/match/0/ratelimit_example_httproute/default/http-ratelimit/rule/0/match/0/ratelimit_example.masked_remote_address_0.0.0.0/0 ratelimit={requests_per_unit=3, unit=HOUR, unlimited=false, shadow_mode=false}"
time="2024-09-27T00:56:04Z" level=debug msg="Setting config retrieved from config provider"
time="2024-09-27T00:56:04Z" level=info msg="Successfully loaded new configuration"
time="2024-09-27T00:56:04Z" level=debug msg="Waiting for config update event"
time="2024-09-27T00:56:39Z" level=debug msg="got descriptor: (httproute/default/http-ratelimit/rule/0/match/0/ratelimit_example=httproute/default/http-ratelimit/rule/0/match/0/ratelimit_example),(rule-0-match-0=rule-0-match-0),(masked_remote_address=0.0.0.0/0)"
time="2024-09-27T00:56:39Z" level=debug msg="starting get limit lookup"
time="2024-09-27T00:56:39Z" level=debug msg="looking up key: httproute/default/http-ratelimit/rule/0/match/0/ratelimit_example_httproute/default/http-ratelimit/rule/0/match/0/ratelimit_example"
time="2024-09-27T00:56:39Z" level=debug msg="iterating to next level"
time="2024-09-27T00:56:39Z" level=debug msg="looking up key: rule-0-match-0_rule-0-match-0"
time="2024-09-27T00:56:39Z" level=debug msg="looking up key: rule-0-match-0"
time="2024-09-27T00:56:39Z" level=debug msg="descriptor does not match any limit, no limits applied"
time="2024-09-27T00:56:39Z" level=debug msg="starting cache lookup"
time="2024-09-27T00:56:39Z" level=debug msg="returning normal response"
I do see that the config is reflected in xds-ir (in gateway logs):
2024-09-27T00:54:27.139Z INFO gateway-api runner/runner.go:186 {
"accessLog": {
"text": [
{
"path": "/dev/stdout"
}
]
},
"http": [
{
"name": "default/eg/http",
"address": "0.0.0.0",
"port": 10080,
"metadata": {
"kind": "Gateway",
"name": "eg",
"namespace": "default",
"sectionName": "http"
},
"hostnames": [
"*"
],
"routes": [
{
"name": "httproute/default/backend/rule/0/match/0/www_example_com",
"hostname": "www.example.com",
"isHTTP2": false,
"pathMatch": {
"name": "",
"prefix": "/",
"distinct": false
},
"destination": {
"name": "httproute/default/backend/rule/0",
"settings": [
{
"weight": 1,
"protocol": "HTTP",
"endpoints": [
{
"host": "10.244.0.10",
"port": 3000
},
{
"host": "10.244.0.10",
"port": 3000
}
],
"addressType": "IP"
}
]
},
"metadata": {
"kind": "HTTPRoute",
"name": "backend",
"namespace": "default"
}
},
{
"name": "httproute/default/http-ratelimit/rule/0/match/0/ratelimit_example",
"hostname": "ratelimit.example",
"isHTTP2": false,
"pathMatch": {
"name": "",
"prefix": "/",
"distinct": false
},
"destination": {
"name": "httproute/default/http-ratelimit/rule/0",
"settings": [
{
"weight": 1,
"protocol": "HTTP",
"endpoints": [
{
"host": "10.244.0.10",
"port": 3000
},
{
"host": "10.244.0.10",
"port": 3000
}
],
"addressType": "IP"
}
]
},
"traffic": {
"rateLimit": {
"global": {
"rules": [
{
"headerMatches": [
{
"name": "x-user-id",
"exact": "one",
"distinct": false
}
],
"cidrMatch": {
"cidr": "0.0.0.0/0",
"ip": "0.0.0.0",
"maskLen": 0,
"isIPv6": false,
"distinct": false
},
"limit": {
"requests": 3,
"unit": "Hour"
}
}
]
}
}
},
"metadata": {
"kind": "HTTPRoute",
"name": "http-ratelimit",
"namespace": "default"
}
}
],
"isHTTP2": false,
"path": {
"mergeSlashes": true,
"escapedSlashesAction": "UnescapeAndRedirect"
}
}
]
} {"runner": "gateway-api", "xds-ir": "default/eg"}
Thanks !
Looks weird, can you share the output of egctl config rl -n envoy-gateway-system ?
Looks weird, can you share the output of egctl config rl -n envoy-gateway-system ?
@shawnh2 Sure, here's the output:
default/eg/http.httproute/default/http-ratelimit/rule/0/match/0/ratelimit_example_httproute/default/http-ratelimit/rule/0/match/0/ratelimit_example.masked_remote_address_0.0.0.0/0: unit=HOUR requests_per_unit=3, shadow_mode: false
I think there is a bug in this method: buildRateLimitServiceDescriptors
https://github.com/envoyproxy/gateway/blob/cbd92e2a423de036d5c8b9db530f0a66d5ce47f9/internal/xds/translator/ratelimit.go#L329
It does not consider the case where both the clientSelectors are present.
I made this change to make it work for the time being:
if rule.CIDRMatch != nil {
// MaskedRemoteAddress case
pbDesc := new(rlsconfv3.RateLimitDescriptor)
pbDesc.Key = "masked_remote_address"
pbDesc.Value = rule.CIDRMatch.CIDR
rateLimit := rlsconfv3.RateLimitPolicy{
RequestsPerUnit: uint32(rule.Limit.Requests),
Unit: rlsconfv3.RateLimitUnit(rlsconfv3.RateLimitUnit_value[strings.ToUpper(string(rule.Limit.Unit))]),
}
if rule.CIDRMatch.Distinct {
pbDesc.Descriptors = []*rlsconfv3.RateLimitDescriptor{
{
Key: "remote_address",
RateLimit: &rateLimit,
},
}
} else {
pbDesc.RateLimit = &rateLimit
}
if len(rule.HeaderMatches) != 0 {
cur.Descriptors = []*rlsconfv3.RateLimitDescriptor{pbDesc}
} else {
head = pbDesc
cur = head
}
}
for mIdx, match := range rule.HeaderMatches {
pbDesc := new(rlsconfv3.RateLimitDescriptor)
// Case for distinct match
if match.Distinct {
// RequestHeader case
pbDesc.Key = getRouteRuleDescriptor(rIdx, mIdx)
} else {
// HeaderValueMatch case
pbDesc.Key = getRouteRuleDescriptor(rIdx, mIdx)
pbDesc.Value = getRouteRuleDescriptor(rIdx, mIdx)
}
// Add the ratelimit values to the last descriptor
if mIdx == len(rule.HeaderMatches)-1 && rule.CIDRMatch == nil {
rateLimit := rlsconfv3.RateLimitPolicy{
RequestsPerUnit: uint32(rule.Limit.Requests),
Unit: rlsconfv3.RateLimitUnit(rlsconfv3.RateLimitUnit_value[strings.ToUpper(string(rule.Limit.Unit))]),
}
pbDesc.RateLimit = &rateLimit
}
if mIdx == 0 {
head = pbDesc
} else {
cur.Descriptors = []*rlsconfv3.RateLimitDescriptor{pbDesc}
}
cur = pbDesc
}
Thanks for reporting this, it is indeed a bug. Though we have created RL actions for both Headers & CIDR correctly,
https://github.com/envoyproxy/gateway/blob/cbd92e2a423de036d5c8b9db530f0a66d5ce47f9/internal/xds/translator/ratelimit.go#L142
but doing wrong when building RL Descriptors.