feat: source rate limit value from dynamic metadata - #9216
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26302b18c5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
26302b1 to
89af148
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9216 +/- ##
==========================================
- Coverage 75.08% 75.04% -0.04%
==========================================
Files 252 252
Lines 40819 40838 +19
==========================================
- Hits 30649 30647 -2
- Misses 8082 8100 +18
- Partials 2088 2091 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
31e87d5 to
f0c9427
Compare
|
@codex review |
There was a problem hiding this comment.
Pull request overview
This PR adds support for sourcing a Global rate limit rule’s per-request limit value from dynamic metadata, via a new optional limit.fromMetadata field on BackendTrafficPolicy rate limit rules. This enables multi-tenant scenarios where an upstream filter (e.g. ext_proc) writes tenant-specific limit structs into dynamic metadata, while the existing requests/unit remain as defaults.
Changes:
- API: Introduces
RateLimitValue.FromMetadataandRateLimitValueMetadata{namespace,key}, and restricts its usage on Local rate limits via CEL validation. - GatewayAPI → IR → xDS: Propagates
fromMetadatainto IR and translates it into Envoy’s route-levelRateLimit.limitdynamic-metadata override. - Tests/docs: Updates CEL validation tests, golden testdata outputs, generated CRDs, API docs, and release notes.
Reviewed changes
Copilot reviewed 10 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
api/v1alpha1/ratelimit_types.go |
Adds RateLimitValue.FromMetadata and the RateLimitValueMetadata type; adds CEL validation blocking fromMetadata for Local rate limits. |
api/v1alpha1/zz_generated.deepcopy.go |
Regenerates deepcopy logic to correctly deep-copy the new pointer field. |
internal/gatewayapi/backendtrafficpolicy.go |
Translates API limit.fromMetadata into IR RateLimitValue.FromMetadata. |
internal/ir/xds.go |
Extends IR rate limit value to carry FromMetadata and defines RateLimitValueMetadata. |
internal/ir/zz_generated.deepcopy.go |
Regenerates deepcopy logic for IR types to deep-copy RateLimitValue (now contains pointers). |
internal/xds/translator/ratelimit.go |
Implements Envoy route-level RateLimit.limit override using dynamic metadata; keeps request/response rule overrides consistent. |
internal/xds/translator/testdata/in/xds-ir/ratelimit.yaml |
Adds IR input coverage for a limit-from-metadata route/rule. |
internal/xds/translator/testdata/out/xds-ir/ratelimit.routes.yaml |
Updates golden output to include the dynamic-metadata limit override on the route rate limit action. |
internal/xds/translator/testdata/out/xds-ir/ratelimit.clusters.yaml |
Updates golden output for new destination cluster associated with added test route. |
internal/xds/translator/testdata/out/xds-ir/ratelimit.endpoints.yaml |
Updates golden output for new destination endpoints associated with added test route. |
internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit.in.yaml |
Adds limit.fromMetadata to BackendTrafficPolicy test input. |
internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit.out.yaml |
Updates golden output to reflect fromMetadata propagation into rendered resources/IR. |
test/cel-validation/backendtrafficpolicy_test.go |
Adds CEL validation coverage ensuring fromMetadata is rejected for Local rate limits. |
charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml |
Regenerates CRD schema/docs and includes the new CEL validation + schema for fromMetadata. |
charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml |
Same CRD regeneration for the alternate helm chart path. |
site/content/en/latest/api/extension_types.md |
Updates generated API reference docs for fromMetadata and RateLimitValueMetadata. |
release-notes/current.yaml |
Adds a release note entry for the new fromMetadata capability for global rate limits. |
Files not reviewed (2)
- api/v1alpha1/zz_generated.deepcopy.go: Generated file
- internal/ir/zz_generated.deepcopy.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25b20096cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Namespace is the namespace of the dynamic metadata. | ||
| // | ||
| // +kubebuilder:validation:Required | ||
| Namespace string `json:"namespace"` |
There was a problem hiding this comment.
Reject empty metadata namespace/key
When fromMetadata is configured with an empty namespace (and similarly an empty key), Kubernetes admission still accepts it because Required only checks presence for strings; buildRateLimitOverride then copies the value directly into Envoy's MetadataKey, whose key and path segment keys must be non-empty, causing Envoy to reject/NACK the generated route instead of applying the policy. Add MinLength=1 validation to both fields so invalid metadata references are rejected at the API boundary.
Useful? React with 👍 / 👎.
Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
|
/retest |
1 similar comment
|
/retest |
|
LGTM, thanks! |
* feat: source rate limit value from dynamic metadata Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com> * chore: regenerate helm CRD test snapshots for fromMetadata field Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com> * codex review: add minlen:1 validations Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com> --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
* feat: source rate limit value from dynamic metadata Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com> * chore: regenerate helm CRD test snapshots for fromMetadata field Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com> * codex review: add minlen:1 validations Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com> --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
* feat: source rate limit value from dynamic metadata Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com> * chore: regenerate helm CRD test snapshots for fromMetadata field Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com> * codex review: add minlen:1 validations Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com> --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
…imit.fromMetadata The migrated test drives the per-tenant limit through BackendTrafficPolicy limit.fromMetadata, which only exists on the Envoy Gateway development build (v0.0.0-latest, from envoyproxy/gateway#9216). On tagged releases the field is pruned, the static default applies, and the assertions can never observe a 429. Gate the test on the installed Envoy Gateway version via new e2elib helpers (EnvoyGatewayVersion / EnvoyGatewaySupportsLimitFromMetadata) so it runs for real on v0.0.0-latest and skips on releases such as v1.8.1. Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
* feat: source rate limit value from dynamic metadata Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com> * chore: regenerate helm CRD test snapshots for fromMetadata field Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com> * codex review: add minlen:1 validations Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com> --------- Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com> Signed-off-by: Andrea Abellonio <andrea.abellonio@servescale.ai>
…imit.fromMetadata The migrated test drives the per-tenant limit through BackendTrafficPolicy limit.fromMetadata, which only exists on the Envoy Gateway development build (v0.0.0-latest, from envoyproxy/gateway#9216). On tagged releases the field is pruned, the static default applies, and the assertions can never observe a 429. Gate the test on the installed Envoy Gateway version via new e2elib helpers (EnvoyGatewayVersion / EnvoyGatewaySupportsLimitFromMetadata) so it runs for real on v0.0.0-latest and skips on releases such as v1.8.1. Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
**Description**
Give each tenant a different rate limit while every tenant shares one
policy and one set of routes. The tenant's limit travels with the
request as dynamic metadata, so there are no per-tenant config objects.
This is documentation and an e2e. There is no API change: Envoy
Gateway's `limit.fromMetadata` can read the limit from any metadata
namespace, so whichever filter knows the tenant can write it and the
policy reads it directly. AI Gateway's part is the other half — turning
the LLM's token usage into the cost charged against that limit, which
already exists as `globalLLMRequestCosts`.
**How it works**
1. `ext_authz` resolves the tenant and returns its budget as dynamic
metadata.
2. One `BackendTrafficPolicy` reads that budget with
`limit.fromMetadata`, bucketed per tenant with `Distinct`.
3. AI Gateway emits the request's token usage, and the policy charges it
against the budget.
**Usage**
The auth server returns the budget in its Check response. Envoy files it
under `envoy.filters.http.ext_authz`. The value must be the struct
Envoy's rate limit override reads:
```json
{
"dynamic_metadata": {
"fields": {
"total_limit": {
"struct_value": {
"fields": {
"requests_per_unit": { "number_value": 100000 },
"unit": { "string_value": "HOUR" }
}
}
}
}
}
}
```
`unit` is one of `SECOND`, `MINUTE`, `HOUR`, `DAY`, `MONTH`, `YEAR`.
Dynamic metadata is used rather than a header because only filters can
write it, so a tenant cannot raise its own limit.
`GatewayConfig` declares the cost to charge:
```yaml
apiVersion: aigateway.envoyproxy.io/v1beta1
kind: GatewayConfig
metadata:
name: envoy-ai-gateway
namespace: default
spec:
globalLLMRequestCosts:
- metadataKey: llm_total_token
type: TotalToken
```
One policy then serves every tenant:
```yaml
apiVersion: gateway.envoyproxy.io/v1alpha1
kind: BackendTrafficPolicy
metadata:
name: tenant-token-budget
namespace: default
spec:
targetRefs:
- group: gateway.networking.k8s.io
kind: Gateway
name: envoy-ai-gateway
rateLimit:
type: Global
global:
rules:
- clientSelectors:
- headers:
- name: x-tenant-id
type: Distinct
limit:
requests: 1000 # default when ext_authz says nothing
unit: Hour
fromMetadata:
namespace: envoy.filters.http.ext_authz # where ext_authz writes
key: total_limit
cost:
request: { from: Number, number: 0 } # charge tokens, not requests
response:
from: Metadata
metadata:
namespace: io.envoy.ai_gateway
key: llm_total_token
```
For separate input, output and total budgets, declare one cost key per
token kind (`InputToken`, `OutputToken`, `TotalToken`), have the auth
server return one budget per kind, and add one rule per kind. A tenant
stops at whichever budget it exhausts first.
**Behavior**
Request-time cost is 0 and tokens are charged from the response, so a
budget of N against a cost of C allows N/C + 1 calls.
| Metadata value | Effect |
| ------------------------------------- |
-------------------------------------------------- |
| valid `{ requests_per_unit, unit }` | becomes the limit for that
request |
| absent | the static `requests`/`unit` on the rule applies |
| `requests_per_unit: 0` | a real limit of 0, which suspends the tenant
|
Two things to plan for:
- Missing metadata fails open to the static default, so keep that
default a conservative ceiling rather than a placeholder.
- `0` is a real limit and does not fall back. A source with no opinion
about a request must omit the field, not send `0` — an uninitialized
field or a failed lookup will otherwise suspend the tenant.
**Why no API change**
An earlier revision of this PR added
`GatewayConfig.spec.globalRateLimits`, which read the limit from one
namespace and re-emitted it into `io.envoy.ai_gateway`. That was written
when the source was a request **header** (#2223), which Envoy's override
cannot read, so something had to bridge it. Review moved the source to
dynamic metadata so clients cannot forge it — and that removed the need
for the bridge, because the filter writing the metadata can write the
final struct itself.
What remained was reshaping `"100000/HOUR"` into a struct. It cost a
`metadata` request attribute on the router ext_proc filter, a change to
how every request is classified router vs upstream, and a
rolling-upgrade break needing its own workaround. Not worth it for a
string reshape on the path of every request.
A source that cannot produce the struct — a `jwt_authn` claim, or a
third-party auth sidecar returning a plain string — still needs
something to transcode. The docs point at a Lua filter. If that case
turns out to be common, an API is worth revisiting on its own merits.
**Testing**
`Test_DynamicMetadataRateLimit` runs the configuration above and asserts
each tier is throttled at its own number:
| tier | budget from ext_authz | calls allowed |
| ------------ | --------------------------- | ------------- |
| `premium` | 6 total tokens/HOUR | 4 |
| `basic` | 2 total tokens/HOUR | 2 |
| `unknown` | none — static `requests: 4` | 3 |
| `suspended` | 0 total tokens/HOUR | 1, then 429 |
| `tri-input` | 6 input, others generous | 3 |
| `tri-output` | 3 output, others generous | 4 |
| `tri-total` | 12 total, others generous | 4 |
The triplet cases use a lopsided reply (3 in, 1 out, 4 total) so the
three budgets produce different counts. With an even reply, input and
output would be indistinguishable and a mix-up between them would go
unnoticed.
The test skips unless Envoy Gateway has `limit.fromMetadata`
(envoyproxy/gateway#9216), which is on main but not yet in a tagged
release. There is a TODO on the version check to relax it once a release
includes it.
**Related Issues/PRs**
Closes #2223, with
documentation instead of the API that issue proposed — see above.
Consumes envoyproxy/gateway#9216 (`limit.fromMetadata` on
`BackendTrafficPolicy`).
Signed-off-by: Anurag Aggarwal <kanurag94@gmail.com>
Co-authored-by: Ignasi Barrera <ignasi@tetrate.io>
Fixes #9212
What
Adds an optional
fromMetadatafield to a global rate limit rule'slimit, letting the limit value be read from per-request dynamic metadata instead of being a fixed number.Why
In a multi-tenant gateway, each tenant often needs a different limit while sharing a single policy/route. Today that forces a separate policy or route per tenant. With this, an upstream ext_proc can write each tenant's limit into dynamic metadata and one rule applies it per request.
requests/unitstay required and act as the default when the metadata isn't present.This mirrors how request cost is already sourced from metadata (
cost.metadata→HitsAddend).How
Wires the value through to Envoy's route-level
RateLimit.limitoverride (RateLimit.Override.DynamicMetadata), building aMetadataKeyfrom the namespace/key. Note this differs from cost: the override resolves to a struct ({requests_per_unit, unit}), not a plain number — the ext_proc is responsible for writing that struct.RateLimitValue.FromMetadata+RateLimitValueMetadata{namespace, key}Testing
fromMetadataon Local rate limits (verified against envtest)