Skip to content

Commit 6f6de7b

Browse files
jopemachineclaude
andcommitted
feat(BA-7365): key the manager rate limiter by user id
`execute_rate_limit_logic()` and `get_rolling_count()` now take a `UserID` and build the counter key themselves, so the manager and the web server share one per-user allowance instead of the manager counting per access key. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 0d8b70d commit 6f6de7b

8 files changed

Lines changed: 53 additions & 37 deletions

File tree

changes/13771.feature.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Add per-user rate limiting to the webserver that rejects over-limit requests with HTTP 429 before they reach the manager, enforcing the existing keypair rate limit against a single per-user allowance.
1+
Add per-user rate limiting to the webserver that rejects over-limit requests with HTTP 429 before they reach the manager, and key the manager-side rate limiter by the authenticated user as well, so holding multiple keypairs no longer multiplies a user's allowance.

src/ai/backend/common/clients/valkey_client/valkey_rate_limit/client.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
create_valkey_client,
1111
)
1212
from ai.backend.common.exception import BackendAIError
13+
from ai.backend.common.identifier.user import UserID
1314
from ai.backend.common.metrics.metric import DomainType, LayerType
1415
from ai.backend.common.resilience import (
1516
BackoffStrategy,
@@ -110,15 +111,14 @@ async def close(self) -> None:
110111
@valkey_rate_limit_resilience.apply()
111112
async def execute_rate_limit_logic(
112113
self,
113-
key: str,
114+
user_id: UserID,
114115
window: int = _DEFAULT_RATE_LIMIT_EXPIRATION,
115116
) -> int:
116117
"""
117118
Execute the rate limiting logic for rolling counter.
118119
This replicates the Lua script logic using individual commands.
119120
120-
:param key: The counter key to rate limit, e.g. an access key or a
121-
namespaced user key.
121+
:param user_id: The user the rolling counter is keyed by.
122122
:param window: The time window for rate limiting in seconds.
123123
:return: The current count.
124124
"""
@@ -128,23 +128,23 @@ async def execute_rate_limit_logic(
128128
async with self._client.client() as conn:
129129
result = await conn.invoke_script(
130130
Script(_RATE_LIMIT_SCRIPT),
131-
keys=[key],
131+
keys=[f"user.{user_id}"],
132132
args=[str(now_float), str(window)],
133133
)
134134

135135
# The last result is the count
136136
return cast(int, result)
137137

138138
@valkey_rate_limit_resilience.apply()
139-
async def get_rolling_count(self, key: str) -> int:
139+
async def get_rolling_count(self, user_id: UserID) -> int:
140140
"""
141-
Get the current rolling count for a counter key.
141+
Get the current rolling count of a user.
142142
143-
:param key: The counter key to get the count for.
143+
:param user_id: The user the rolling counter is keyed by.
144144
:return: The current count.
145145
"""
146146
async with self._client.client() as conn:
147-
return await conn.zcard(key)
147+
return await conn.zcard(f"user.{user_id}")
148148

149149
@valkey_rate_limit_resilience.apply()
150150
async def set_rate_limit_config(

src/ai/backend/manager/api/gql_legacy/keypair.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
from ai.backend.common.clients.valkey_client.valkey_rate_limit.client import ValkeyRateLimitClient
1515
from ai.backend.common.defs import REDIS_RATE_LIMIT_DB, RedisRole
16+
from ai.backend.common.identifier.user import UserID
1617
from ai.backend.common.types import AccessKey
1718
from ai.backend.manager.data.kernel.types import KernelStatus
1819
from ai.backend.manager.data.keypair.types import KeyPairCreator, KeyPairData
@@ -196,7 +197,7 @@ async def resolve_rolling_count(self, info: graphene.ResolveInfo) -> int:
196197
human_readable_name="ratelimit",
197198
)
198199
try:
199-
return await valkey_client.get_rolling_count(self.access_key)
200+
return await valkey_client.get_rolling_count(UserID(self.user))
200201
finally:
201202
await valkey_client.close()
202203

src/ai/backend/manager/api/rest/ratelimit/handler.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@
33
This module provides the ``rlim_middleware`` function which is installed
44
as a global aiohttp middleware. There are no route handlers — rate
55
limiting is applied transparently to all authorized requests.
6+
7+
The rolling counter is keyed by the authenticated user, the same counter the
8+
web server's limiter uses. This is the fallback guard for clients that reach
9+
the manager directly; requests proxied by the web server are already limited
10+
there.
611
"""
712

813
from __future__ import annotations
@@ -38,9 +43,8 @@ async def rlim_middleware(
3843
"""Global middleware implementing a rolling-counter rate limiter."""
3944
if request["is_authorized"]:
4045
rate_limit = request["keypair"]["rate_limit"]
41-
access_key = request["keypair"]["access_key"]
4246
rolling_count = await valkey_client.execute_rate_limit_logic(
43-
key=access_key,
47+
user_id=request["user"]["uuid"],
4448
window=_rlim_window,
4549
)
4650
if rate_limit is not None and rolling_count > rate_limit:

src/ai/backend/web/ratelimit.py

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,25 @@
11
"""Per-user rate limiting for requests proxied to the manager.
22
3-
The same rolling-counter mechanism and limit value as the manager-side rate
4-
limiter (``manager/api/rest/ratelimit``): the limit is the keypair
5-
``rate_limit`` delivered at login (``None`` means unlimited), but the counter
6-
is keyed by the login user instead of the access key so that holding multiple
7-
keypairs does not multiply the allowance. Over-limit requests are rejected
8-
with HTTP 429 at the web server, before they reach the manager.
3+
The same rolling counter and limit value as the manager-side rate limiter
4+
(``manager/api/rest/ratelimit``): the limit is the keypair ``rate_limit``
5+
delivered at login (``None`` means unlimited), and the counter is keyed by the
6+
login user, so holding multiple keypairs does not multiply the allowance.
7+
Over-limit requests are rejected with HTTP 429 at the web server, before they
8+
reach the manager.
99
"""
1010

1111
from __future__ import annotations
1212

1313
import json
1414
import logging
15+
import uuid
1516
from collections.abc import Awaitable, Callable
1617
from typing import Final
1718

1819
from aiohttp import web
1920

2021
from ai.backend.common.clients.valkey_client.valkey_rate_limit.client import ValkeyRateLimitClient
22+
from ai.backend.common.identifier.user import UserID
2123
from ai.backend.common.web.session import get_session
2224
from ai.backend.logging import BraceStyleAdapter
2325

@@ -38,15 +40,15 @@ async def rate_limit_middleware(
3840
if not session.get("authenticated", False):
3941
return await handler(request)
4042
token = session.get("token") or {}
41-
user_id = token.get("user_id")
42-
if user_id is None:
43+
raw_user_id = token.get("user_id")
44+
if raw_user_id is None:
4345
# Session created against a manager that does not send the user ID yet.
4446
return await handler(request)
4547
rate_limit = token.get("rate_limit")
4648

4749
valkey_client: ValkeyRateLimitClient = request.app["valkey_rate_limit"]
4850
rolling_count = await valkey_client.execute_rate_limit_logic(
49-
f"user.{user_id}",
51+
user_id=UserID(uuid.UUID(raw_user_id)),
5052
window=_rlim_window,
5153
)
5254
remaining = max(rate_limit - rolling_count, 0) if rate_limit is not None else rolling_count
Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,33 @@
11
from __future__ import annotations
22

3-
import random
3+
import uuid
44

55
from ai.backend.common.clients.valkey_client.valkey_rate_limit.client import (
66
ValkeyRateLimitClient,
77
)
8+
from ai.backend.common.identifier.user import UserID
89

910

1011
async def test_valkey_rate_limit_logic_execution(
1112
test_valkey_rate_limit: ValkeyRateLimitClient,
1213
) -> None:
1314
"""Test rate limiting logic execution."""
14-
access_key = f"test-logic-{random.randint(1000, 9999)}"
15+
user_id = UserID(uuid.uuid4())
1516

1617
# Execute the rate limiting logic
1718
result = await test_valkey_rate_limit.execute_rate_limit_logic(
18-
key=access_key,
19+
user_id=user_id,
1920
window=60,
2021
)
2122

2223
assert result == 1 # First request should return 1
2324

2425
# Execute again
2526
result2 = await test_valkey_rate_limit.execute_rate_limit_logic(
26-
key=access_key,
27+
user_id=user_id,
2728
window=60,
2829
)
2930

3031
assert result2 == 2 # Second request should return 2
32+
33+
assert await test_valkey_rate_limit.get_rolling_count(user_id) == 2

tests/unit/manager/api/test_ratelimit.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from __future__ import annotations
22

3+
import uuid
34
from dataclasses import dataclass
45
from typing import Any
56
from unittest.mock import AsyncMock, MagicMock
@@ -8,9 +9,12 @@
89
from aiohttp import web
910

1011
from ai.backend.common.clients.valkey_client.valkey_rate_limit.client import ValkeyRateLimitClient
12+
from ai.backend.common.identifier.user import UserID
1113
from ai.backend.manager.api.rest.ratelimit.handler import _rlim_window, make_rlim_middleware
1214
from ai.backend.manager.errors.api import RateLimitExceeded
1315

16+
_USER_ID = UserID(uuid.UUID("12345678-1234-5678-1234-567812345678"))
17+
1418

1519
@dataclass
1620
class RateLimitSuccessCase:
@@ -65,16 +69,16 @@ def mock_request_anonymous(self) -> web.Request:
6569
def mock_request_authorized(self) -> web.Request:
6670
"""Mock request for authorized user."""
6771
request = MagicMock(spec=web.Request)
68-
keypair_data = {
69-
"rate_limit": 30000,
70-
"access_key": "AKIAIOSFODNN7EXAMPLE",
71-
}
72+
keypair_data = {"rate_limit": 30000}
73+
user_data = {"uuid": _USER_ID}
7274

7375
def getitem(key: Any) -> Any:
7476
if key == "is_authorized":
7577
return True
7678
if key == "keypair":
7779
return keypair_data
80+
if key == "user":
81+
return user_data
7882
return None
7983

8084
request.__getitem__ = MagicMock(side_effect=getitem)
@@ -162,7 +166,7 @@ async def test_authorized_query_within_rate_limit(
162166

163167
# Valkey should be called for authorized requests
164168
mock_valkey_client.execute_rate_limit_logic.assert_called_once_with(
165-
key="AKIAIOSFODNN7EXAMPLE",
169+
user_id=_USER_ID,
166170
window=_rlim_window,
167171
)
168172

@@ -211,6 +215,6 @@ async def test_authorized_query_exceeds_rate_limit(
211215

212216
# Valkey should still be called
213217
mock_valkey_client.execute_rate_limit_logic.assert_called_once_with(
214-
key="AKIAIOSFODNN7EXAMPLE",
218+
user_id=_USER_ID,
215219
window=_rlim_window,
216220
)

tests/unit/webserver/test_ratelimit.py

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from __future__ import annotations
22

3+
import uuid
34
from dataclasses import dataclass
45
from typing import Any
56
from unittest.mock import AsyncMock
@@ -10,10 +11,11 @@
1011
from pytest_mock import MockerFixture
1112

1213
from ai.backend.common.clients.valkey_client.valkey_rate_limit.client import ValkeyRateLimitClient
14+
from ai.backend.common.identifier.user import UserID
1315
from ai.backend.web import ratelimit
1416
from ai.backend.web.ratelimit import rate_limit_middleware
1517

16-
_USER_ID = "12345678-1234-5678-1234-567812345678"
18+
_USER_ID = UserID(uuid.UUID("12345678-1234-5678-1234-567812345678"))
1719
_RATE_LIMIT = 1000
1820

1921

@@ -49,7 +51,7 @@ class _PassThroughCase:
4951
path="/server/login",
5052
session={
5153
"authenticated": True,
52-
"token": {"user_id": _USER_ID, "rate_limit": _RATE_LIMIT},
54+
"token": {"user_id": str(_USER_ID), "rate_limit": _RATE_LIMIT},
5355
},
5456
),
5557
_PassThroughCase(
@@ -93,7 +95,7 @@ async def test_counts_request_per_user_and_sets_headers(
9395
) -> None:
9496
session = {
9597
"authenticated": True,
96-
"token": {"user_id": _USER_ID, "rate_limit": _RATE_LIMIT},
98+
"token": {"user_id": str(_USER_ID), "rate_limit": _RATE_LIMIT},
9799
}
98100
mocker.patch.object(ratelimit, "get_session", AsyncMock(return_value=session))
99101
request = make_mocked_request(
@@ -104,7 +106,7 @@ async def test_counts_request_per_user_and_sets_headers(
104106

105107
assert response is handler_response
106108
mock_valkey_rate_limit_client.execute_rate_limit_logic.assert_awaited_once_with(
107-
f"user.{_USER_ID}",
109+
user_id=_USER_ID,
108110
window=ratelimit._rlim_window,
109111
)
110112
assert response.headers["X-RateLimit-Limit"] == str(_RATE_LIMIT)
@@ -119,7 +121,7 @@ async def test_rejects_over_limit_request_with_429(
119121
) -> None:
120122
session = {
121123
"authenticated": True,
122-
"token": {"user_id": _USER_ID, "rate_limit": _RATE_LIMIT},
124+
"token": {"user_id": str(_USER_ID), "rate_limit": _RATE_LIMIT},
123125
}
124126
mocker.patch.object(ratelimit, "get_session", AsyncMock(return_value=session))
125127
mock_valkey_rate_limit_client.execute_rate_limit_logic.return_value = _RATE_LIMIT + 1
@@ -144,7 +146,7 @@ async def test_null_rate_limit_counts_but_never_rejects(
144146
) -> None:
145147
session = {
146148
"authenticated": True,
147-
"token": {"user_id": _USER_ID, "rate_limit": None},
149+
"token": {"user_id": str(_USER_ID), "rate_limit": None},
148150
}
149151
mocker.patch.object(ratelimit, "get_session", AsyncMock(return_value=session))
150152
mock_valkey_rate_limit_client.execute_rate_limit_logic.return_value = 10_000_000

0 commit comments

Comments
 (0)