Repository navigation
feat: add opt-in per-caller request rate limiting - #458
Conversation
Move the request-size resolution out of main.rs into server_settings.rs, unchanged. main.rs is at its file-size exception, and the next [server] setting needs room next to this one. Signed-off-by: Zheng Lu <Lz429671594@gmail.com>
A shared gateway had only global concurrency bounds, so one caller could use up the upstream's capacity and paid tool quotas for everyone. `--rate-limit-requests-per-minute`, `AGENTIC_RATE_LIMIT_REQUESTS_PER_MINUTE` or `[server] rate_limit_requests_per_minute` now turns on a per-caller limit, with an optional burst. It is off by default. - Callers are keyed by OIDC issuer and subject, otherwise by a keyed hash of the bearer token or `x-api-key`, otherwise by one anonymous bucket. Raw credentials are never stored. - The limiter uses GCRA over a bounded map. Idle callers are pruned, and callers beyond the cap share an overflow bucket. The lock is never held across an await. - The middleware runs inside OIDC authentication on every protected route. A limited request gets 429 in OpenAI or Anthropic format with retry-after, retry-after-ms and x-ratelimit-* headers, and never reaches the upstream. - A WebSocket is charged for its upgrade and for each client `response.create`, from the upgrade request's own credentials. A limited create gets an error frame, and the connection stays open. Steering, injection and gateway-started continuations are not charged. Closes vllm-project#457 Signed-off-by: Zheng Lu <Lz429671594@gmail.com>
Signed-off-by: Zheng Lu <Lz429671594@gmail.com>
Once the map held MAX_KEYS active callers, every request from a new key scanned the whole map under the shared lock. A client rotating bearer values could make each of its requests an O(100,000) critical section for every caller. Sweeps of a full map now run at most once per SWEEP_INTERVAL (1 s). New callers in between share the overflow bucket without scanning. In release builds, 2,000 new-key requests against a full 100,000-caller map take 0.66 ms instead of 349 ms. Signed-off-by: Zheng Lu <Lz429671594@gmail.com>
retry_after_seconds and retry_after_millis truncated to milliseconds and microseconds before rounding up, so a wait of 1 s + 1 ns was advertised as `retry-after: 1`. That is shorter than the limiter requires, while `Retry-After` is the minimum wait. Both now round up from nanoseconds. Signed-off-by: Zheng Lu <Lz429671594@gmail.com>
One untracked caller keeps sending while the key map stays full. The overflow bucket rejects all but one request, and none of them rescans the map. The test fails on the code before the sweep interval, where each rejected request scanned every caller under the lock. Signed-off-by: Zheng Lu <Lz429671594@gmail.com>
The rate limiter repeated auth's choice between the OpenAI and Anthropic envelopes and its request-ID handling, and both built their bodies with json!. error_response.rs now holds one GatewayError builder with typed envelopes and a dynamic message. Each caller adds its own headers: WWW-Authenticate for authentication, retry hints for rate limits. auth.rs drops from 791 to 727 production lines, and its size baseline is lowered to match. Signed-off-by: Zheng Lu <Lz429671594@gmail.com>
Embedders configure rate limiting through the public RateLimitConfig; the limiter itself is used only inside the crate. Drop its unused config() accessor, since the middleware reads the field directly. Signed-off-by: Zheng Lu <Lz429671594@gmail.com>
|
|
||
| The gateway keeps only a keyed hash of a token or API key, never the credential. Without OIDC, the gateway does not | ||
| verify these credentials (the upstream does), so they identify callers for rate limiting only. A WebSocket is charged | ||
| to the caller of its upgrade request, never to the server's `OPENAI_API_KEY`. |
There was a problem hiding this comment.
Without OIDC these keys are whatever the client sends, so the limit only binds callers that reuse a credential. In a scratch test at 1 request per minute, 20 requests that each carried a new made-up bearer all returned 200 and reached the upstream. A limited client without credentials also got through again just by adding any bearer, and x-api-key behaves the same. Could the docs say this plainly, e.g. "without OIDC, a client can get a fresh bucket by changing its credential; enable OIDC to enforce per-caller limits against untrusted clients"?
There was a problem hiding this comment.
Fixed in 1b77d39. The API docs now say that without OIDC the limit only binds clients that reuse a credential: a client gets a fresh bucket by sending a different bearer token or x-api-key, and a client without credentials leaves the shared bucket by adding any. They also recommend enabling OIDC to enforce per-caller limits against untrusted clients. The README and the Kubernetes guide say the same.
The tests already cover both escapes: a_caller_over_its_limit_gets_an_openai_429_with_retry_headers shows another bearer gets its own bucket, and callers_without_credentials_share_one_bucket shows adding a bearer leaves the anonymous bucket. With OIDC, rate_limits_key_oidc_callers_by_subject_not_token shows a new token for the same subject does not reset the bucket.
There was a problem hiding this comment.
A fresh x-api-key escapes the limit while the gateway pays with its server key
When the gateway runs the request itself (store: true, the default), a client can send a new made-up x-api-key (or a lowercase bearer token) on each request. Each value gets its own bucket, and the gateway never forwards it: it calls the upstream with its own server key, so nobody ever checks the credential. With the limit set to 1 request per minute, five such requests all reached the upstream as Bearer server-key; it still reproduces on the new head. This is why the 200s in @Mohammad-nassar10's thread happen, and why the doc's "the upstream does" verification doesn't hold on this path.
Suggested fix: don't key the bucket on a credential the gateway doesn't verify. Use the verified identity when OIDC is on; without it, key on something the client can't mint for free (for example the peer address), and document that per-caller limits need OIDC against untrusted clients.
There was a problem hiding this comment.
Fixed in 3d32f92, as you suggested:
- A request is charged to the verified OIDC principal, otherwise to the connection's peer address.
- An IPv6 address counts as its /64, since one host commonly holds a whole /64, and an IPv4-mapped address counts as its IPv4 caller.
- A bearer token or
x-api-keynever chooses the bucket. - The server now records each connection's peer address.
gateway_listenerreturns the concrete tapped listener, so axum can provideConnectInfo.
Regression test, changing_the_credential_does_not_reset_the_bucket:
- It mirrors your reproduction: 1 request per minute, a
store: trueexecutor request with a server key configured, then a newx-api-key, a newBearer, a lowercasebearer, and no credential. - All four are now limited, and the upstream sees one request.
- It failed before the fix: the fresh
x-api-keywas processed.
separate_peer_addresses_have_separate_buckets covers the new keying through the router with injected peer addresses: the same address on a new port is the same caller, another address has its own bucket, and one IPv6 /64 is one caller.
On the real binary, from 127.0.0.1 the first request is admitted, and rotating x-api-key, using a lowercase bearer, or dropping the credential then gets 429. From 127.0.0.2 a request is admitted.
At one request per minute, the advertised wait is 60 s less the time since the bucket's first request. The integration tests asserted exactly 60 s, so a run slower than one second (as under parallel load) failed on a correct `59`. They now check each hint against the measured elapsed time, and check the hints against each other exactly: `retry-after` is `retry-after-ms` rounded up, the reset is that wait plus one interval per extra burst request, and the message quotes `retry-after`. The paused-time unit tests still pin the exact rounding. Signed-off-by: Zheng Lu <Lz429671594@gmail.com>
Without OIDC, the gateway keys callers by credentials it does not verify, so a client gets a fresh bucket by changing its bearer token or `x-api-key`, and a client without credentials leaves the shared bucket by adding any. Say so, and recommend OIDC for untrusted clients. Signed-off-by: Zheng Lu <Lz429671594@gmail.com>
A bearer token or `x-api-key` chose the bucket even though the gateway does not verify either. On executor routes the gateway may call the upstream with its own key, so nothing ever checked them. A client could send a fresh made-up `x-api-key`, or a lowercase `bearer` token, with every request and never be limited. Requests are now charged to the verified OIDC principal, otherwise to the connection's peer address. An IPv6 address counts as its /64, since one host commonly holds a whole /64. The server records each connection's peer address (`gateway_listener` now returns the concrete tapped listener so axum can provide `ConnectInfo`). A request with no known peer, from a router served without connection info, shares one bucket. Signed-off-by: Zheng Lu <Lz429671594@gmail.com>
Callers are told apart by OIDC identity, otherwise by peer address, never by a credential the gateway does not verify. Behind a proxy or ingress every request shares the proxy's address, so per-caller limits need OIDC. Forwarded-for headers are not trusted. Signed-off-by: Zheng Lu <Lz429671594@gmail.com>
There was a problem hiding this comment.
@Zheng-Lu LGTM.
could you please update ARCHITECTURE.md to describe rate_limit.rs, error_response.rs, server_settings.rs, and authentication before rate-limit admission.
Brings in vllm-project#458 (opt-in per-caller request rate limiting). Merged without conflicts. Signed-off-by: haoshan98 <haoshanw@gmail.com>
…#480) ## Summary Follow-up to #458, as requested in @maralbahari's approval: #458 (review). Documentation only. `ARCHITECTURE.md` now describes: - **Request admission** (new section). Protected routes run OIDC (`require_oidc`), then rate-limit admission (`enforce_rate_limit`), then the retrieval-only API-key check, then the handler. Authentication comes first because the limiter keys a bucket by the verified principal whenever there is one. The section also explains why the rate-limit `route_layer` is added before the OIDC one, and which routes sit outside both layers. - **`rate_limit.rs`** (new section): GCRA buckets, keys (OIDC principal, otherwise the peer address with IPv6 grouped by /64; never unverified credentials or forwarded-for headers), the `MAX_KEYS` and overflow bound, the retry hints, and the per-frame WebSocket charge through `RateLimitCharge`. - **`error_response.rs`** (new section): `GatewayError` and the OpenAI or Anthropic envelope chosen by `ErrorFormat::for_path`, used for authentication failures and rate limits. - **`server_settings.rs`**: the `[server]` admission settings and their CLI > environment > file > default resolution. - **Smaller updates:** - `rate_limit` is added to the library exports and the gateway's module table; - `server.rs` serves with connect info; - `auth.rs` mentions `require_api_key`; - a new "Change request admission" row in the quick reference. ## Test Plan - Checked each statement against `main` at a7dc7b5: - the layer order in `build_router_with_auth`; - `RateLimitKey::for_caller`; - `LimiterState`; - the WebSocket `RateLimitCharge::check` path, which sends an error event and keeps the connection open; - the OIDC 401 and 503 responses; - `server_settings::resolve_rate_limit_values`. - `pre-commit run --files ARCHITECTURE.md` passes. Signed-off-by: Zheng Lu <Lz429671594@gmail.com> Co-authored-by: Maral <maralbahari.98@gmail.com>

Summary
Closes #457
The gateway had only global concurrency bounds, so one caller could use up the upstream's capacity and shared web-search or code-interpreter quotas for everyone. This PR adds an opt-in, per-caller request rate limit. It is off by default: no middleware is installed and no headers change.
Configuration (CLI > environment >
[server]file setting, followingmax_request_body_size_bytes):[server])--rate-limit-requests-per-minuteAGENTIC_RATE_LIMIT_REQUESTS_PER_MINUTErate_limit_requests_per_minute--rate-limit-burstAGENTIC_RATE_LIMIT_BURSTrate_limit_burst(default: the per-minute rate)Caller key (first match):
x-api-key;Raw credentials are never stored. A WebSocket is keyed by its upgrade request's own credentials, never by the server's
OPENAI_API_KEYfallback.Algorithm: GCRA (equivalent to a token bucket). Each caller holds one theoretical arrival time in a
std::sync::Mutex<HashMap>, usingtokio::time::Instantso tests can pause time. No lock is held across an await. Memory is bounded at 100,000 callers: idle callers are pruned first, and new callers share an overflow bucket while every tracked caller is active, with awarnat most once a minute. No new dependencies.HTTP:
route_layeron the protected routes, layered before OIDC so that it runs after authentication and sees the principal./health,/readyand Swagger are exempt.429in OpenAI format (rate_limit_error/rate_limit_exceeded), or in Anthropic format withrequest-idon/v1/messagesandcount_tokens.retry-after,retry-after-ms,x-ratelimit-limit-requests,x-ratelimit-remaining-requests: 0, andx-ratelimit-reset-requests. The last is the time until the full burst returns, formatted like1.5sor2m3s.WebSocket:
response.createis charged before any lane or byte capacity is reserved. A limited create gets anerrorframe withstatus: 429, and the connection stays open.response.steer,response.injectand gateway-started continuations are not charged. The check sits where client messages arrive, not inWsMultiplexer::schedule, which also runs automatic continuations.Structure:
rate_limit.rsholds the limiter, middleware,429builder and retry formatting.server_settings.rsresolves the[server]table. Its first commit is a pure move of the existing body-size resolution out ofmain.rs, which was at its 644-line exception (now 616).ServerConfiggainsrate_limit, soAppStateand its 16 test literals are unchanged.auth.rsis at its line baseline. Its only change is visibility (AuthErrorFormatandfor_path, reused for the 429 format), andbearer_tokeninauth/api_key.rsis also now crate-visible.Scope:
agenticlauncher flags. The launcher's child process inherits the environment variables, and the configuration file is shared.Conflicts: a trial merge with #436, #440 and #420 is clean.
Test Plan
Unit tests:
rate_limit::tests(paused time):retry_afterandreset;Debug; bearer andx-api-keyshare a key;Basicis anonymous);x-ratelimit-reset-requestsformatting;server_settings::tests:[server]table.config_filetests: the new keys parse,0is rejected, and an unset burst is not written.tests/rate_limit_test.rs(one request per minute, so the outcomes are deterministic):a_caller_over_its_limit_gets_an_openai_429_with_retry_headers: exact body and all five headers; the limited request never reaches the upstream; another bearer is unaffected.callers_without_credentials_share_one_buckethealth_endpoints_are_never_limitedmessages_routes_answer_in_the_anthropic_format_and_key_by_api_key: exact body; therequest-idheader matches the body.rate_limiting_is_off_by_default: 10 requests and no rate-limit headers.websocket_creates_are_charged_and_a_limited_create_keeps_the_connection: the upgrade and two creates use a burst of 3; the next create gets the exact429frame; thenresponse.steeris answered on the same connection.http_and_websocket_share_a_bucket_and_the_upgrade_is_charged: a limited upgrade gets HTTP429before switching protocols.a_websocket_without_credentials_is_not_charged_to_the_server_keyoidc_auth_test::rate_limits_key_oidc_callers_by_subject_not_token: two tokens for one subject share a bucket, and another subject has its own.Negative controls (each applied to the finished code):
Binary smoke test at 1 request per minute:
--helplists both flags.AGENTIC_RATE_LIMIT_BURSTwithout a rate fails at startup:AGENTIC_RATE_LIMIT_BURST requires AGENTIC_RATE_LIMIT_REQUESTS_PER_MINUTE./v1/modelsrequest reaches the upstream; the second gets429with all headers;/healthstill returns200.Gate:
cargo fmt -- --checkcargo clippy --workspace --all-targets -- -D warnings--features embedded-code-interpreter) for both cratespre-commit run --all-filesmkdocs build --strictcargo test --workspace --no-fail-fast: 2530 passed, 1 failed. The failure istool::web_fetch::http::tests::the_deadline_bounds_a_slow_origin, which also fails on unmodifiedmainwhen the whole lib suite runs in parallel.