envoyproxy / envoyproxy/gateway

Rate limit: Only the last `sourceCIDR` in `clientSelectors` takes effect

Open
#10,025 0 comments 0 reactions 0 assignees View on GitHub
triage
Dominant language
Go
Stars
3k
Forks
864
Avg merge
2d 2h
Merged PRs (30d)
140

Description

> Addresses below are placeholders rather than real values: `x.x.x.x/32` is a single address
we wanted exempt, `y.y.y.y/24` and `z.z.z.z/24` are two ranges we wanted exempt, and
`a.a.a.a`, `b.b.b.b`, `c.c.c.c` are three distinct clients sending traffic. Each placeholder
is consistent throughout.

*Description*:

When a global rate limit rule has more than one `sourceCIDR` in its `clientSelectors`, only
the last one seems to have any effect. The earlier ones are dropped without any error or
status condition, and the policy still reports `Accepted=True`.

This may cause issue in 2 ways.

The first is if you put `type: Distinct` on one selector and `invert: true` on another,
which seemed like the natural way to write "rate limit every client individually, except
this CIDR":

```yaml
- clientSelectors:
- sourceCIDR:
value: 0.0.0.0/0
type: Distinct
- sourceCIDR:
value: x.x.x.x/32
invert: true
limit:
requests: 5
unit: Minute
```

The `Distinct` selector disappears, so instead of one bucket per client IP you get a single
bucket shared by every non-exempt client. That's a fairly big behavioural difference from
what the config reads like. The CRD docs for `clientSelectors` say "All individual select
conditions must hold True", which is what led me to write it this way.

The second is if you want more than one CIDR exempt:

```yaml
- clientSelectors:
- sourceCIDR:
value: y.y.y.y/24
type: Distinct
invert: true
- sourceCIDR:
value: z.z.z.z/24
type: Distinct
invert: true
limit:
requests: 5
unit: Minute
```

Here `Distinct` survives because it's on the last selector, so per-IP bucketing keeps
working and everything looks healthy. But only `z.z.z.z/24` is actually exempt.
`y.y.y.y/24` is still rate limited. That one took a while to notice.

Putting both `type: Distinct` and `invert: true` on a single selector does work, and is what
we ended up using. It's just limited to one CIDR.

*Additional context*:

I had a look at the code to try to understand it, though I'm not familiar with the codebase
so I may be misreading. In `internal/gatewayapi/backendtrafficpolicy.go` the loop over
`ClientSelectors` assigns to `irRule.CIDRMatch` rather than appending:

```go
for _, match := range rule.ClientSelectors {
if match.SourceCIDR != nil {
...
irRule.CIDRMatch = cidrMatch
}
}
```

whereas `HeaderMatches` and `MethodMatches` a few lines up use `append`. And `CIDRMatch` on
`RateLimitRule` in `internal/ir/xds.go` is a single pointer rather than a slice, so I don't
think it could hold more than one anyway. If that's right then the last selector would win,
which matches what we see, but I could easily be missing something. `PathMatch` is assigned
in the same loop and is also a single pointer, so two selectors each carrying a `path` might
behave the same way — I haven't tested that.

Header selectors don't have this problem. `Distinct` on one header selector plus `invert` on
another works fine, which is what made the CIDR behaviour surprising. There's an existing
e2e test doing exactly that pattern for headers
(`ratelimit-header-invert-match-global.yaml`). The two CIDR invert tests added in
[#8407](https://github.com/envoyproxy/gateway/pull/8407) each use a single non-`Distinct`
selector, and `ratelimit-cidr-match.yaml` uses `Distinct` without invert, so as far as I can
tell neither the combination nor the multi-selector case is covered for CIDRs.

Expected behaviour: the selectors combine the way the CRD description suggests.

Related: [#4385](https://github.com/envoyproxy/gateway/issues/4385) asked for exempting "few
whitelisted ips" and was closed by #8407, which as far as I can tell supports one CIDR, so
the plural part of that request may still be open. There's a comment in that thread from
2024-10-28 using five `sourceCIDR` selectors and asking why it wasn't working, which I
suspect was the same thing.

*Repro steps*:

Apply a rate limit with two `sourceCIDR` selectors, both `Distinct`, both `invert`:

```yaml
apiVersion: gateway.envoyproxy.io/v1alpha1
kind: BackendTrafficPolicy
metadata:
name: example-ratelimit
namespace: default
spec:
targetRefs:
- group: gateway.networking.k8s.io
kind: HTTPRoute
name: example
rateLimit:
type: Global
global:
rules:
- clientSelectors:
- sourceCIDR:
value: y.y.y.y/24
type: Distinct
invert: true
- sourceCIDR:
value: z.z.z.z/24
type: Distinct
invert: true
limit:
requests: 5
unit: Minute
shadowMode: true
```

Then dump the route config:

```
egctl config envoy-proxy route -n envoy-gateway-system -o yaml
```

Only `z.z.z.z/24` shows up. `y.y.y.y/24` isn't referenced anywhere.

We confirmed the behaviour by starting from a working single-selector config and just adding
a second selector, changing nothing else. Before adding it, requests from a client in
`y.y.y.y/24` produced no rate limit counter at all, which is correct since it was
exempt. After adding the second selector, the same client's requests were counted. So adding
a selector removed an exemption that was previously working.

The policy reports `Accepted=True` in both cases and there's no warning anywhere that we
could find.

*Environment*:

- Envoy Gateway v1.9.1 (chart `gateway-helm` 1.9.1)
- Envoy 1.39.1 (`envoyproxy/envoy:distroless-v1.39.1`)
- Global rate limiting with Redis/Valkey backend
- AKS 1.31, Gateway API v1
- Also checked `main` (as of 2026-09-14) and the assignment in `backendtrafficpolicy.go`
looks unchanged there, though we haven't tested against it

*Logs*:

No relevant errors in the Envoy Gateway controller logs or the rate limit service logs in
either case.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with internal/gatewayapi/backendtrafficpolicy.go and the RateLimitRule definition in internal/ir/xds.go, then compare CIDR handling with HeaderMatches. Review ratelimit-header-invert-match-global.yaml and ratelimit-cidr-match.yaml before adding coverage for multiple inverted CIDR selectors. Done means all selectors remain effective, including distinct bucketing and multiple exemptions, with the relevant tests passing.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, kubernetes, redis
Domain
api, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.