Merge pull request #1220 from lsst-sqre/tickets/DM-48495

DM-48495: Add rate limiting integration with nginx

docs/user-guide/headers.rst

@@ -0,0 +1,39 @@
+################
+Response headers
+################
+
+The following headers may be added to the response from a service protected by Gafaelfawr.
+They will only be added for services behind an authenticated ``GafaelfawrIngress`` resource, not an anonymous one.
+
+Rate limit headers
+==================
+
+``Retry-After``
+    Only sent on 429 responses once the rate limit has been exceeded.
+    Specifies the time at which the rate limit will reset and the user will be able to make requests again.
+    The value is an HTTP date.
+
+``X-RateLimit-Limit``
+    This request is subject to a rate limit.
+    The value of this header is the total number of requests permitted in each time window, which currently is always fifteen minutes.
+    See ``X-RateLimit-Remaining`` for the number of requests left in that interval.
+
+``X-RateLimit-Remaining``
+    The number of requests to this service remaining in the user's quota.
+    The quota will reset at the time given by ``X-RateLimit-Reset``.
+
+``X-RateLimit-Reset``
+    The time at which the rate limit quota will reset, in seconds since epoch.
+    At this time, the number of requests seen will be reset to zero, and the user will receive another full allotment of their quota.
+
+``X-RateLimit-Resource``
+    The name of the resource being rate-limited.
+    This will match the ``service`` setting of the ``GafaelfawrIngress`` Kubernetes resource.
+    Clients can use this header to understand what requests are subject to a given quota.
+
+``X-RateLimit-Used``
+    The number of requests to this resource seen within the rate limit period.
+    This will be ``X-RateLimit-Limit`` minus ``X-RateLimit-Remaining``.
+
+The ``X-RateLimit`` headers are sent for successful responses, error responses from the underlying service, and 429 responses.
+The ``Retry-After`` header is only sent as part of a 429 response.

docs/user-guide/index.rst

@@ -27,6 +27,7 @@ Also see the `Phalanx Gafaelfawr application documentation <https://phalanx.lsst
 .. toctree::
    :caption: Reference
 
+   headers
    logging
    cli
    metrics

src/gafaelfawr/constants.py

@@ -106,9 +106,21 @@
 """
 
 NGINX_SNIPPET = """\
+auth_request_set $auth_error_body $upstream_http_x_error_body;
+auth_request_set $auth_ratelimit_limit $upstream_http_x_ratelimit_limit;
+auth_request_set $auth_ratelimit_remaining\
+ $upstream_http_x_ratelimit_remaining;
+auth_request_set $auth_ratelimit_reset $upstream_http_x_ratelimit_reset;
+auth_request_set $auth_ratelimit_resource $upstream_http_x_ratelimit_resource;
+auth_request_set $auth_ratelimit_used $upstream_http_x_ratelimit_used;
+auth_request_set $auth_retry_after $upstream_http_retry_after;
 auth_request_set $auth_www_authenticate $upstream_http_www_authenticate;
 auth_request_set $auth_status $upstream_http_x_error_status;
-auth_request_set $auth_error_body $upstream_http_x_error_body;
+more_set_headers "X-RateLimit-Limit: $auth_ratelimit_limit";
+more_set_headers "X-RateLimit-Remaining: $auth_ratelimit_remaining";
+more_set_headers "X-RateLimit-Reset: $auth_ratelimit_reset";
+more_set_headers "X-RateLimit-Resource: $auth_ratelimit_resource";
+more_set_headers "X-RateLimit-Used: $auth_ratelimit_used";
 error_page 403 = @autherror;
 """
 """Code snippet to put into NGINX configuration for each ingress."""

src/gafaelfawr/handlers/ingress.py

@@ -8,6 +8,7 @@
 
 from __future__ import annotations
 
+import json
 from dataclasses import dataclass
 from datetime import UTC, datetime, timedelta
 from email.utils import format_datetime
@@ -582,12 +583,17 @@ async def check_rate_limit(
     if allowed:
         return status
     else:
+        # Return a 403 error with the actual status code and body in the
+        # headers, where they will be parsed by the ingress-nginx integration.
         msg = f"Rate limit ({quota}/15m) exceeded"
+        detail = [{"msg": msg, "type": "rate_limited"}]
         raise HTTPException(
-            detail=[{"msg": msg, "type": "rate_limited"}],
-            status_code=429,
+            detail=detail,
+            status_code=403,
             headers={
                 "Cache-Control": "no-cache, no-store",
+                "X-Error-Body": json.dumps({"detail": detail}),
+                "X-Error-Status": "429",
                 "Retry-After": format_datetime(retry_after, usegmt=True),
                 **status.to_http_headers(),
             },

tests/handlers/ingress_rate_test.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import json
 from datetime import UTC, datetime, timedelta
 from email.utils import parsedate_to_datetime
 
@@ -58,10 +59,13 @@ async def test_rate_limit(client: AsyncClient, factory: Factory) -> None:
         params={"scope": "read:all", "service": "test"},
         headers=headers,
     )
-    assert r.status_code == 429
+    assert r.status_code == 403
     retry_after = parsedate_to_datetime(r.headers["Retry-After"])
     assert expected <= retry_after
     assert retry_after <= expected + timedelta(seconds=5)
+    body = json.loads(r.headers["X-Error-Body"])
+    assert body["detail"][0]["type"] == "rate_limited"
+    assert r.headers["X-Error-Status"] == "429"
     assert r.headers["X-RateLimit-Limit"] == "2"
     assert r.headers["X-RateLimit-Remaining"] == "0"
     assert r.headers["X-RateLimit-Used"] == "2"