perf: scale workers + per-tablet rate limiting for 20 concurrent users
The default 2-worker gunicorn could only serve 2 concurrent tablet requests, queueing the rest, and the rate limiter saw every tablet as the same Nginx container IP, so 20 users would have collectively burned through the 100 req/min general bucket. - gunicorn: 5 workers x 4 gthread, --forwarded-allow-ips=*, access log - uvicorn: 4 workers, --proxy-headers, --forwarded-allow-ips=* - RateLimitMiddleware: resolve real client IP from X-Forwarded-For -> X-Real-IP -> request.client.host - Bump rate_limit_general 100 -> 300 req/min/IP (per tablet now) - Flask: ProxyFix(x_for=1, x_proto=1, x_host=1) so request.remote_addr is the tablet IP, not the Nginx IP - APIClient: forward X-Forwarded-For + X-Real-IP to FastAPI for both JSON and multipart/files calls; safe no-op outside request context - 12 new tests (7 server + 5 client) covering header precedence, forwarding behavior and ProxyFix install Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
+9
-1
@@ -22,4 +22,12 @@ RUN pybabel compile -d translations
|
|||||||
|
|
||||||
EXPOSE 5000
|
EXPOSE 5000
|
||||||
|
|
||||||
CMD ["gunicorn", "--workers", "2", "--bind", "0.0.0.0:5000", "app:create_app()"]
|
CMD ["gunicorn", \
|
||||||
|
"--workers", "5", \
|
||||||
|
"--threads", "4", \
|
||||||
|
"--worker-class", "gthread", \
|
||||||
|
"--timeout", "60", \
|
||||||
|
"--bind", "0.0.0.0:5000", \
|
||||||
|
"--access-logfile", "-", \
|
||||||
|
"--forwarded-allow-ips", "*", \
|
||||||
|
"app:create_app()"]
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ from flask import Flask, redirect, url_for, session, request
|
|||||||
from flask_babel import Babel
|
from flask_babel import Babel
|
||||||
from flask_wtf.csrf import CSRFProtect
|
from flask_wtf.csrf import CSRFProtect
|
||||||
from markupsafe import Markup
|
from markupsafe import Markup
|
||||||
|
from werkzeug.middleware.proxy_fix import ProxyFix
|
||||||
|
|
||||||
from config import Config
|
from config import Config
|
||||||
|
|
||||||
@@ -26,6 +27,11 @@ def create_app() -> Flask:
|
|||||||
app = Flask(__name__)
|
app = Flask(__name__)
|
||||||
app.config.from_object(Config)
|
app.config.from_object(Config)
|
||||||
|
|
||||||
|
# Trust one reverse-proxy hop (Nginx in dev, Traefik in prod) so that
|
||||||
|
# request.remote_addr returns the real tablet IP rather than the proxy IP.
|
||||||
|
# The APIClient forwards that IP to FastAPI for accurate rate limiting.
|
||||||
|
app.wsgi_app = ProxyFix(app.wsgi_app, x_for=1, x_proto=1, x_host=1)
|
||||||
|
|
||||||
# Initialize CSRF protection
|
# Initialize CSRF protection
|
||||||
csrf = CSRFProtect(app)
|
csrf = CSRFProtect(app)
|
||||||
|
|
||||||
|
|||||||
@@ -2,7 +2,7 @@
|
|||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
import requests
|
import requests
|
||||||
from flask import session
|
from flask import has_request_context, request, session
|
||||||
|
|
||||||
from config import Config
|
from config import Config
|
||||||
|
|
||||||
@@ -14,13 +14,29 @@ class APIClient:
|
|||||||
self.base_url = Config.API_SERVER_URL.rstrip("/")
|
self.base_url = Config.API_SERVER_URL.rstrip("/")
|
||||||
self.timeout = 30
|
self.timeout = 30
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _real_client_ip() -> str | None:
|
||||||
|
"""Best-effort tablet IP for downstream rate limiting.
|
||||||
|
|
||||||
|
Behind Nginx + ProxyFix, ``request.remote_addr`` already resolves to
|
||||||
|
the tablet IP. Outside a request context (background tasks, tests),
|
||||||
|
return None so we don't forge a bogus header.
|
||||||
|
"""
|
||||||
|
if not has_request_context():
|
||||||
|
return None
|
||||||
|
return request.remote_addr
|
||||||
|
|
||||||
@property
|
@property
|
||||||
def _headers(self) -> dict[str, str]:
|
def _headers(self) -> dict[str, str]:
|
||||||
"""Build request headers with API key from session."""
|
"""Build request headers with API key + real client IP from session."""
|
||||||
headers = {"Content-Type": "application/json"}
|
headers = {"Content-Type": "application/json"}
|
||||||
api_key = session.get("api_key")
|
api_key = session.get("api_key")
|
||||||
if api_key:
|
if api_key:
|
||||||
headers["X-API-Key"] = api_key
|
headers["X-API-Key"] = api_key
|
||||||
|
client_ip = self._real_client_ip()
|
||||||
|
if client_ip:
|
||||||
|
headers["X-Forwarded-For"] = client_ip
|
||||||
|
headers["X-Real-IP"] = client_ip
|
||||||
return headers
|
return headers
|
||||||
|
|
||||||
def _handle_response(self, response: requests.Response) -> dict[str, Any]:
|
def _handle_response(self, response: requests.Response) -> dict[str, Any]:
|
||||||
@@ -76,6 +92,10 @@ class APIClient:
|
|||||||
try:
|
try:
|
||||||
if files:
|
if files:
|
||||||
headers = {"X-API-Key": session.get("api_key", "")}
|
headers = {"X-API-Key": session.get("api_key", "")}
|
||||||
|
client_ip = self._real_client_ip()
|
||||||
|
if client_ip:
|
||||||
|
headers["X-Forwarded-For"] = client_ip
|
||||||
|
headers["X-Real-IP"] = client_ip
|
||||||
response = requests.post(
|
response = requests.post(
|
||||||
f"{self.base_url}{endpoint}",
|
f"{self.base_url}{endpoint}",
|
||||||
headers=headers,
|
headers=headers,
|
||||||
|
|||||||
@@ -0,0 +1,70 @@
|
|||||||
|
"""Tests that APIClient forwards the real tablet IP via X-Forwarded-For."""
|
||||||
|
from unittest.mock import patch
|
||||||
|
|
||||||
|
from services.api_client import api_client
|
||||||
|
|
||||||
|
|
||||||
|
def _last_call_headers(mock):
|
||||||
|
"""Return the headers kwarg from the most recent requests.* call."""
|
||||||
|
return mock.call_args.kwargs["headers"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_get_forwards_real_client_ip(flask_app):
|
||||||
|
with flask_app.test_request_context(
|
||||||
|
"/", environ_base={"REMOTE_ADDR": "203.0.113.10"}
|
||||||
|
):
|
||||||
|
with patch("services.api_client.requests.get") as mock_get:
|
||||||
|
mock_get.return_value.ok = True
|
||||||
|
mock_get.return_value.status_code = 200
|
||||||
|
mock_get.return_value.json.return_value = {}
|
||||||
|
api_client.get("/api/health")
|
||||||
|
|
||||||
|
headers = _last_call_headers(mock_get)
|
||||||
|
assert headers["X-Forwarded-For"] == "203.0.113.10"
|
||||||
|
assert headers["X-Real-IP"] == "203.0.113.10"
|
||||||
|
|
||||||
|
|
||||||
|
def test_post_json_forwards_real_client_ip(flask_app):
|
||||||
|
with flask_app.test_request_context(
|
||||||
|
"/", environ_base={"REMOTE_ADDR": "198.51.100.42"}
|
||||||
|
):
|
||||||
|
with patch("services.api_client.requests.post") as mock_post:
|
||||||
|
mock_post.return_value.ok = True
|
||||||
|
mock_post.return_value.status_code = 200
|
||||||
|
mock_post.return_value.json.return_value = {}
|
||||||
|
api_client.post("/api/foo", data={"x": 1})
|
||||||
|
|
||||||
|
headers = _last_call_headers(mock_post)
|
||||||
|
assert headers["X-Forwarded-For"] == "198.51.100.42"
|
||||||
|
|
||||||
|
|
||||||
|
def test_post_with_files_forwards_real_client_ip(flask_app):
|
||||||
|
with flask_app.test_request_context(
|
||||||
|
"/", environ_base={"REMOTE_ADDR": "198.51.100.77"}
|
||||||
|
):
|
||||||
|
with patch("services.api_client.requests.post") as mock_post:
|
||||||
|
mock_post.return_value.ok = True
|
||||||
|
mock_post.return_value.status_code = 200
|
||||||
|
mock_post.return_value.json.return_value = {}
|
||||||
|
api_client.post(
|
||||||
|
"/api/files/upload", data={}, files={"file": ("x.txt", b"data")}
|
||||||
|
)
|
||||||
|
|
||||||
|
headers = _last_call_headers(mock_post)
|
||||||
|
assert headers["X-Forwarded-For"] == "198.51.100.77"
|
||||||
|
assert headers["X-Real-IP"] == "198.51.100.77"
|
||||||
|
|
||||||
|
|
||||||
|
def test_real_client_ip_returns_none_outside_request_context(flask_app):
|
||||||
|
"""Background workers without a Flask request must NOT inject a fake IP."""
|
||||||
|
from services.api_client import APIClient
|
||||||
|
|
||||||
|
with flask_app.app_context():
|
||||||
|
assert APIClient._real_client_ip() is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_proxy_fix_is_installed(flask_app):
|
||||||
|
"""ProxyFix must wrap wsgi_app so REMOTE_ADDR is rewritten in production."""
|
||||||
|
from werkzeug.middleware.proxy_fix import ProxyFix
|
||||||
|
|
||||||
|
assert isinstance(flask_app.wsgi_app, ProxyFix)
|
||||||
+1
-1
@@ -23,4 +23,4 @@ RUN mkdir -p uploads/images uploads/pdfs uploads/logos uploads/reports
|
|||||||
EXPOSE 8000
|
EXPOSE 8000
|
||||||
|
|
||||||
# Entry point: Alembic upgrade + Uvicorn
|
# Entry point: Alembic upgrade + Uvicorn
|
||||||
CMD ["sh", "-c", "alembic -c migrations/alembic.ini upgrade head && uvicorn main:app --host 0.0.0.0 --port 8000 --workers 2"]
|
CMD ["sh", "-c", "alembic -c migrations/alembic.ini upgrade head && uvicorn main:app --host 0.0.0.0 --port 8000 --workers 4 --proxy-headers --forwarded-allow-ips='*'"]
|
||||||
|
|||||||
+2
-2
@@ -23,9 +23,9 @@ class Settings(BaseSettings):
|
|||||||
upload_dir: str = "uploads"
|
upload_dir: str = "uploads"
|
||||||
max_upload_size_mb: int = 50
|
max_upload_size_mb: int = 50
|
||||||
|
|
||||||
# Rate Limiting (requests per minute)
|
# Rate Limiting (requests per minute, per real client IP)
|
||||||
rate_limit_login: int = 5
|
rate_limit_login: int = 5
|
||||||
rate_limit_general: int = 100
|
rate_limit_general: int = 300
|
||||||
|
|
||||||
# SSL (Production)
|
# SSL (Production)
|
||||||
ssl_certfile: str | None = None
|
ssl_certfile: str | None = None
|
||||||
|
|||||||
@@ -32,6 +32,25 @@ class RateLimitMiddleware(BaseHTTPMiddleware):
|
|||||||
self._general_requests: dict[str, list[float]] = defaultdict(list)
|
self._general_requests: dict[str, list[float]] = defaultdict(list)
|
||||||
self._request_count = 0 # Counter for triggering eviction
|
self._request_count = 0 # Counter for triggering eviction
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _client_ip(request: Request) -> str:
|
||||||
|
"""Resolve the originating client IP, honoring proxy headers.
|
||||||
|
|
||||||
|
Order of precedence: ``X-Forwarded-For`` (first hop), ``X-Real-IP``,
|
||||||
|
``request.client.host``. Required because Nginx and the Flask client
|
||||||
|
sit between the tablet and the API; without parsing these headers
|
||||||
|
every tablet shares one bucket.
|
||||||
|
"""
|
||||||
|
xff = request.headers.get("x-forwarded-for")
|
||||||
|
if xff:
|
||||||
|
first = xff.split(",")[0].strip()
|
||||||
|
if first:
|
||||||
|
return first
|
||||||
|
real = request.headers.get("x-real-ip")
|
||||||
|
if real:
|
||||||
|
return real.strip()
|
||||||
|
return request.client.host if request.client else "unknown"
|
||||||
|
|
||||||
def _clean_window(self, timestamps: list[float], now: float) -> list[float]:
|
def _clean_window(self, timestamps: list[float], now: float) -> list[float]:
|
||||||
"""Remove timestamps outside the current sliding window."""
|
"""Remove timestamps outside the current sliding window."""
|
||||||
cutoff = now - self.WINDOW_SECONDS
|
cutoff = now - self.WINDOW_SECONDS
|
||||||
@@ -68,7 +87,7 @@ class RateLimitMiddleware(BaseHTTPMiddleware):
|
|||||||
return True, 0
|
return True, 0
|
||||||
|
|
||||||
async def dispatch(self, request: Request, call_next: Callable) -> Response:
|
async def dispatch(self, request: Request, call_next: Callable) -> Response:
|
||||||
client_ip = request.client.host if request.client else "unknown"
|
client_ip = self._client_ip(request)
|
||||||
now = time.time()
|
now = time.time()
|
||||||
path = request.url.path
|
path = request.url.path
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,65 @@
|
|||||||
|
"""Tests for RateLimitMiddleware honoring proxy headers (X-Forwarded-For)."""
|
||||||
|
from middleware.rate_limit import RateLimitMiddleware
|
||||||
|
|
||||||
|
|
||||||
|
class _FakeRequest:
|
||||||
|
"""Minimal stand-in for starlette.requests.Request."""
|
||||||
|
|
||||||
|
def __init__(self, headers: dict[str, str], host: str | None = "10.0.0.1"):
|
||||||
|
self.headers = headers
|
||||||
|
|
||||||
|
class _Client:
|
||||||
|
pass
|
||||||
|
|
||||||
|
if host is None:
|
||||||
|
self.client = None
|
||||||
|
else:
|
||||||
|
self.client = _Client()
|
||||||
|
self.client.host = host
|
||||||
|
|
||||||
|
|
||||||
|
def test_client_ip_uses_x_forwarded_for_first_hop():
|
||||||
|
req = _FakeRequest(
|
||||||
|
headers={"x-forwarded-for": "203.0.113.5, 10.0.0.2"},
|
||||||
|
host="10.0.0.1",
|
||||||
|
)
|
||||||
|
assert RateLimitMiddleware._client_ip(req) == "203.0.113.5"
|
||||||
|
|
||||||
|
|
||||||
|
def test_client_ip_strips_whitespace():
|
||||||
|
req = _FakeRequest(headers={"x-forwarded-for": " 198.51.100.7 "})
|
||||||
|
assert RateLimitMiddleware._client_ip(req) == "198.51.100.7"
|
||||||
|
|
||||||
|
|
||||||
|
def test_client_ip_falls_back_to_x_real_ip():
|
||||||
|
req = _FakeRequest(headers={"x-real-ip": "203.0.113.99"}, host="10.0.0.1")
|
||||||
|
assert RateLimitMiddleware._client_ip(req) == "203.0.113.99"
|
||||||
|
|
||||||
|
|
||||||
|
def test_client_ip_falls_back_to_request_client_host():
|
||||||
|
req = _FakeRequest(headers={}, host="172.18.0.5")
|
||||||
|
assert RateLimitMiddleware._client_ip(req) == "172.18.0.5"
|
||||||
|
|
||||||
|
|
||||||
|
def test_client_ip_returns_unknown_without_client():
|
||||||
|
req = _FakeRequest(headers={}, host=None)
|
||||||
|
assert RateLimitMiddleware._client_ip(req) == "unknown"
|
||||||
|
|
||||||
|
|
||||||
|
def test_x_forwarded_for_overrides_x_real_ip():
|
||||||
|
req = _FakeRequest(
|
||||||
|
headers={
|
||||||
|
"x-forwarded-for": "203.0.113.5",
|
||||||
|
"x-real-ip": "10.0.0.2",
|
||||||
|
},
|
||||||
|
host="10.0.0.1",
|
||||||
|
)
|
||||||
|
assert RateLimitMiddleware._client_ip(req) == "203.0.113.5"
|
||||||
|
|
||||||
|
|
||||||
|
def test_empty_x_forwarded_for_falls_through():
|
||||||
|
req = _FakeRequest(
|
||||||
|
headers={"x-forwarded-for": "", "x-real-ip": "203.0.113.42"},
|
||||||
|
host="10.0.0.1",
|
||||||
|
)
|
||||||
|
assert RateLimitMiddleware._client_ip(req) == "203.0.113.42"
|
||||||
Reference in New Issue
Block a user