Skip to content

Commit 9429438

Browse files
jopemachineclaude
andcommitted
feat(BA-7365): read the rate limit the manager publishes
The limit is no longer carried in the session token, so the middleware reads the value the manager publishes for the user. A user with no published limit passes through: the manager republishes on every authorized request, so the value is missing only before a user's first proxied request or after a window of inactivity, and the manager-side limiter covers both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 28f4079 commit 9429438

2 files changed

Lines changed: 33 additions & 50 deletions

File tree

src/ai/backend/web/ratelimit.py

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
"""Per-user rate limiting for requests proxied to the manager.
22
33
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.
4+
(``manager/api/rest/ratelimit``): the counter is keyed by the login user, so
5+
holding multiple keypairs does not multiply the allowance, and the limit is
6+
the value the manager publishes for that user. Over-limit requests are
7+
rejected with HTTP 429 at the web server, before they reach the manager.
8+
9+
A user with no published limit is not limited here. The manager republishes
10+
on every authorized request, so the value is missing only before a user's
11+
first proxied request or after a window of inactivity, and the manager-side
12+
limiter covers both.
913
"""
1014

1115
from __future__ import annotations
@@ -42,22 +46,25 @@ async def rate_limit_middleware(
4246
token = session.get("token") or {}
4347
raw_user_id = token.get("user_id")
4448
if raw_user_id is None:
45-
# Session created against a manager that does not send the user ID yet.
49+
# A session stored before the login handler started keeping the user id.
4650
return await handler(request)
47-
rate_limit = token.get("rate_limit")
51+
user_id = UserID(uuid.UUID(raw_user_id))
4852

4953
valkey_client: ValkeyRateLimitClient = request.app["valkey_rate_limit"]
54+
rate_limit = await valkey_client.get_user_rate_limit(user_id)
55+
if rate_limit is None:
56+
return await handler(request)
57+
5058
rolling_count = await valkey_client.execute_rate_limit_logic(
51-
user_id=UserID(uuid.UUID(raw_user_id)),
59+
user_id=user_id,
5260
window=_rlim_window,
5361
)
54-
remaining = max(rate_limit - rolling_count, 0) if rate_limit is not None else rolling_count
5562
rlim_headers = {
5663
"X-RateLimit-Limit": str(rate_limit),
57-
"X-RateLimit-Remaining": str(remaining),
64+
"X-RateLimit-Remaining": str(max(rate_limit - rolling_count, 0)),
5865
"X-RateLimit-Window": str(_rlim_window),
5966
}
60-
if rate_limit is not None and rolling_count > rate_limit:
67+
if rolling_count > rate_limit:
6168
return web.HTTPTooManyRequests(
6269
text=json.dumps({
6370
"type": "https://api.backend.ai/probs/rate-limit-exceeded",

tests/unit/webserver/test_ratelimit.py

Lines changed: 15 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,13 @@
1717

1818
_USER_ID = UserID(uuid.UUID("12345678-1234-5678-1234-567812345678"))
1919
_RATE_LIMIT = 1000
20+
_AUTHENTICATED_SESSION = {"authenticated": True, "token": {"user_id": str(_USER_ID)}}
2021

2122

2223
@pytest.fixture
2324
def mock_valkey_rate_limit_client() -> AsyncMock:
2425
client = AsyncMock(spec=ValkeyRateLimitClient)
26+
client.get_user_rate_limit.return_value = _RATE_LIMIT
2527
client.execute_rate_limit_logic.return_value = 1
2628
return client
2729

@@ -41,6 +43,7 @@ class _PassThroughCase:
4143
id: str
4244
path: str
4345
session: dict[str, Any]
46+
published_rate_limit: int | None = _RATE_LIMIT
4447

4548

4649
@pytest.mark.parametrize(
@@ -49,21 +52,24 @@ class _PassThroughCase:
4952
_PassThroughCase(
5053
id="non-proxied-path",
5154
path="/server/login",
52-
session={
53-
"authenticated": True,
54-
"token": {"user_id": str(_USER_ID), "rate_limit": _RATE_LIMIT},
55-
},
55+
session=_AUTHENTICATED_SESSION,
5656
),
5757
_PassThroughCase(
5858
id="unauthenticated",
5959
path="/func/session",
6060
session={},
6161
),
6262
_PassThroughCase(
63-
id="no-user-id-in-token",
63+
id="session-stored-before-user-id",
6464
path="/func/session",
6565
session={"authenticated": True, "token": {"access_key": "AKTEST"}},
6666
),
67+
_PassThroughCase(
68+
id="no-published-rate-limit",
69+
path="/func/session",
70+
session=_AUTHENTICATED_SESSION,
71+
published_rate_limit=None,
72+
),
6773
],
6874
ids=lambda case: case.id,
6975
)
@@ -75,6 +81,7 @@ async def test_pass_through_without_rate_limiting(
7581
handler_response: web.Response,
7682
) -> None:
7783
mocker.patch.object(ratelimit, "get_session", AsyncMock(return_value=case.session))
84+
mock_valkey_rate_limit_client.get_user_rate_limit.return_value = case.published_rate_limit
7885
request = make_mocked_request(
7986
"GET", case.path, app={"valkey_rate_limit": mock_valkey_rate_limit_client}
8087
)
@@ -93,18 +100,15 @@ async def test_counts_request_per_user_and_sets_headers(
93100
handler: AsyncMock,
94101
handler_response: web.Response,
95102
) -> None:
96-
session = {
97-
"authenticated": True,
98-
"token": {"user_id": str(_USER_ID), "rate_limit": _RATE_LIMIT},
99-
}
100-
mocker.patch.object(ratelimit, "get_session", AsyncMock(return_value=session))
103+
mocker.patch.object(ratelimit, "get_session", AsyncMock(return_value=_AUTHENTICATED_SESSION))
101104
request = make_mocked_request(
102105
"GET", "/func/session", app={"valkey_rate_limit": mock_valkey_rate_limit_client}
103106
)
104107

105108
response = await rate_limit_middleware(request, handler)
106109

107110
assert response is handler_response
111+
mock_valkey_rate_limit_client.get_user_rate_limit.assert_awaited_once_with(_USER_ID)
108112
mock_valkey_rate_limit_client.execute_rate_limit_logic.assert_awaited_once_with(
109113
user_id=_USER_ID,
110114
window=ratelimit._rlim_window,
@@ -119,11 +123,7 @@ async def test_rejects_over_limit_request_with_429(
119123
mock_valkey_rate_limit_client: AsyncMock,
120124
handler: AsyncMock,
121125
) -> None:
122-
session = {
123-
"authenticated": True,
124-
"token": {"user_id": str(_USER_ID), "rate_limit": _RATE_LIMIT},
125-
}
126-
mocker.patch.object(ratelimit, "get_session", AsyncMock(return_value=session))
126+
mocker.patch.object(ratelimit, "get_session", AsyncMock(return_value=_AUTHENTICATED_SESSION))
127127
mock_valkey_rate_limit_client.execute_rate_limit_logic.return_value = _RATE_LIMIT + 1
128128
request = make_mocked_request(
129129
"GET", "/func/session", app={"valkey_rate_limit": mock_valkey_rate_limit_client}
@@ -136,27 +136,3 @@ async def test_rejects_over_limit_request_with_429(
136136
assert response.content_type == "application/problem+json"
137137
assert response.headers["X-RateLimit-Limit"] == str(_RATE_LIMIT)
138138
assert response.headers["X-RateLimit-Remaining"] == "0"
139-
140-
141-
async def test_null_rate_limit_counts_but_never_rejects(
142-
mocker: MockerFixture,
143-
mock_valkey_rate_limit_client: AsyncMock,
144-
handler: AsyncMock,
145-
handler_response: web.Response,
146-
) -> None:
147-
session = {
148-
"authenticated": True,
149-
"token": {"user_id": str(_USER_ID), "rate_limit": None},
150-
}
151-
mocker.patch.object(ratelimit, "get_session", AsyncMock(return_value=session))
152-
mock_valkey_rate_limit_client.execute_rate_limit_logic.return_value = 10_000_000
153-
request = make_mocked_request(
154-
"GET", "/func/session", app={"valkey_rate_limit": mock_valkey_rate_limit_client}
155-
)
156-
157-
response = await rate_limit_middleware(request, handler)
158-
159-
assert response is handler_response
160-
handler.assert_awaited_once_with(request)
161-
mock_valkey_rate_limit_client.execute_rate_limit_logic.assert_awaited_once()
162-
assert response.headers["X-RateLimit-Remaining"] == "10000000"

0 commit comments

Comments
 (0)