refactoring-backend #3

Merged
lukas.pupkalipinski merged 403 commits from refactoring-backend into main 2026-05-20 20:23:46 +02:00
8 changed files with 409 additions and 39 deletions
Showing only changes of commit 65fe747cba - Show all commits

View File

@@ -98,4 +98,77 @@ jobs:
run: pip install -e ".[dev]"
- name: Run import-linter
run: linter
run: linter
openapi-breaking-changes:
name: OpenAPI Breaking Changes
runs-on: ubuntu-latest
defaults:
run:
working-directory: backend
# Only run on PRs — main branch push is covered by the baseline-commit step.
if: github.event_name == 'pull_request'
steps:
- uses: actions/checkout@v4
- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: "3.12"
- name: Install dependencies
run: pip install -e ".[dev]"
- name: Generate current OpenAPI spec
run: python scripts/generate_openapi.py current-openapi.json
- name: Fetch baseline spec from main
run: |
git fetch origin main:main
git show main:backend/openapi.json > baseline-openapi.json 2>/dev/null || \
echo "{}" > baseline-openapi.json
- name: Install openapi-diff
run: npm install -g openapi-diff
- name: Check for breaking changes
run: |
set +e
openapi-diff baseline-openapi.json current-openapi.json --format stylish 2>&1
EXIT_CODE=$?
if [ $EXIT_CODE -ne 0 ]; then
echo "BREAKING CHANGE DETECTED — see output above"
exit 1
fi
echo "No breaking changes found."
openapi-baseline-commit:
name: OpenAPI Baseline Commit
runs-on: ubuntu-latest
# Only run on push to main (not PRs).
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
steps:
- uses: actions/checkout@v4
- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: "3.12"
- name: Install dependencies
run: pip install -e ".[dev]"
- name: Generate and commit OpenAPI baseline
run: |
python scripts/generate_openapi.py backend/openapi.json
git config --local user.email "github-actions[bot]@users.noreply.github.com"
git config --local user.name "github-actions[bot]"
git add backend/openapi.json
git diff --cached --quiet && echo "No changes to openapi.json" || \
git commit -m "chore: update OpenAPI baseline spec [skip ci]
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>"

View File

@@ -23,8 +23,8 @@ This document explains when and how to version endpoints, how deprecation works,
/api/v{major}/<resource>/<path>
```
- **v1** — current version (2026-05-02)
- **v2** — reserved for future breaking changes
- **v1** — current version (released 2026-05-02)
- **v2** — reserved; skeleton router deployed at `/api/v2/jails` but **not yet active** for production traffic
- **PATCH** versions (v1.1, v1.2) are **not** used; only **major** version bumps indicate breaking changes
- The OpenAPI schema is always available at `/api/openapi.json` regardless of version
@@ -56,14 +56,35 @@ These do **not** require a version bump.
When an endpoint is deprecated:
1. The endpoint **remains functional** for a minimum of **6 months** from the `Sunset` date
2. Response headers are added:
2. Response headers are added to every 2xx response:
```
Deprecation: true
Sunset: <RFC-5322 date>
Link: <https://bangui.example.com/api/v2/...>; rel="successor-version"
```
3. The OpenAPI schema marks the endpoint with `deprecated: true`
4. Documentation is updated to show the endpoint as deprecated
3. The endpoint is registered in the deprecation middleware (``app/middleware/deprecation.py``)
4. The OpenAPI schema marks the endpoint with `deprecated: true`
5. Documentation is updated to show the endpoint as deprecated
### Implementing Deprecation Headers
The ``DeprecationHeaderMiddleware`` (``app/middleware/deprecation.py``) automatically injects
the correct headers for any registered deprecated endpoint. To schedule an endpoint for removal:
```python
from datetime import datetime, timezone, timedelta
from app.middleware.deprecation import register_deprecated_endpoint
# Example: deprecate /api/v1/jails on 2026-11-03 (6 months from v2 release)
register_deprecated_endpoint(
path_prefix="/api/v1/jails",
sunset_date=datetime(2026, 11, 3, tzinfo=timezone.utc),
successor_url="/api/v2/jails",
)
```
The middleware runs on every response; if the request path matches a registered deprecated prefix,
the appropriate headers are appended before the response is returned.
---
@@ -81,19 +102,21 @@ router = APIRouter(prefix="/api/v1/my-resource", tags=["My Resource"])
1. Create a new router file (e.g., `routers/my_resource_v2.py`) with the v2 prefix:
```python
router = APIRouter(prefix="/api/v2/my-resource", tags=["My Resource"])
router = APIRouter(prefix="/api/v2/my-resource", tags=["My Resource (v2)"])
```
2. Copy or adapt the v1 handler logic as needed
2. Copy or adapt the v1 handler logic as needed. Extract shared business logic into
a **service layer function** so both routers call the same underlying code.
3. Register the new router in `app/main.py`:
```python
app.include_router(my_resource_v2.router)
```
4. Add deprecation headers to the **old** v1 router by marking it deprecated in the OpenAPI spec
4. Register the v1 endpoint for deprecation headers (see §4 above)
5. Update this document to reflect the new version lifecycle
### Keeping routers DRY
If v1 and v2 share logic, extract business logic into a **service layer function** and call it from both router handlers. Routers should only contain HTTP concerns (parameters, responses, status codes).
Routers should only contain HTTP concerns (parameters, responses, status codes). Business logic
belongs in the service layer. Both v1 and v2 handlers can call the same service function.
---
@@ -107,6 +130,8 @@ const BASE_URL: string = import.meta.env.VITE_API_URL ?? "/api/v1";
All endpoint paths in `frontend/src/api/endpoints.ts` are defined as relative paths (e.g., `/bans`, `/jails`) and are appended to `BASE_URL` at runtime.
When v2 is released, update ``VITE_API_URL`` in the environment configuration to point to `/api/v2`.
---
## 7. OpenAPI / Documentation
@@ -118,8 +143,24 @@ All endpoint paths in `frontend/src/api/endpoints.ts` are defined as relative pa
---
## 8. Version History
## 8. CI Breaking-Change Checks
A GitHub Actions job runs on every pull request to detect breaking OpenAPI changes:
- ``openapi-breaking-changes`` job (PR only): generates the current OpenAPI spec and
compares it against the baseline committed on the last push to `main`. If any breaking
changes are found, the job fails and the PR cannot be merged.
- ``openapi-baseline-commit`` job (main push only): generates and commits the current
OpenAPI spec as the new baseline for future PR comparisons.
To trigger the baseline update, push to main after merging a version bump or any change
that legitimately alters the OpenAPI surface.
---
## 9. Version History
| Version | Status | Released | Sunset Date | Notes |
|---------|--------|---------|-------------|-------|
| v1 | **Current** | 2026-05-02 | — | Initial versioning; all endpoints moved from `/api/` to `/api/v1/` |
| v1 | **Current** | 2026-05-02 | — | Initial versioning; all endpoints moved from `/api/` to `/api/v1/` |
| v2 | **Reserved — skeleton active, endpoints not yet available** | — | — | Router skeleton at `app/routers/jails_v2.py`; real endpoints will be added before activation |

View File

@@ -1,30 +1,3 @@
### Issue #55: MEDIUM - Correlation ID Scope Is Module-Level (Resets on HMR)
**Where found**:
- `frontend/src/api/client.ts:26-39` `sessionCorrelationId` stored as a module variable
**Why this is needed**:
Hot Module Replacement (HMR) re-evaluates modules, generating a new correlation ID mid-session. Distributed traces lose continuity, making it impossible to correlate logs from the same user session.
**Goal**:
Persist the correlation ID across module re-evaluations for the lifetime of the browser tab.
**What to do**:
1. Store the correlation ID in `sessionStorage` on first generation; read from there on subsequent module evaluations.
2. Clear it on logout.
**Possible traps and issues**:
- `sessionStorage` is tab-local, which is the desired scope.
- Ensure the ID is not leaked in URLs or logs.
**Docs changes needed**:
- Add comment explaining the persistence strategy in `client.ts`.
**Doc references**:
- `frontend/src/api/client.ts` `getSessionCorrelationId()`
---
### Issue #56: MEDIUM - No API Versioning or Deprecation Strategy
**Where found**:

View File

@@ -48,6 +48,7 @@ from app.exceptions import (
)
from app.middleware.correlation import CorrelationIdMiddleware
from app.middleware.csrf import CsrfMiddleware
from app.middleware.deprecation import DeprecationHeaderMiddleware
from app.middleware.metrics import MetricsMiddleware
from app.middleware.rate_limit import RateLimitMiddleware
from app.models.response import ErrorResponse
@@ -62,6 +63,7 @@ from app.routers import (
health,
history,
jails,
jails_v2,
metrics,
server,
setup,
@@ -1074,6 +1076,7 @@ def create_app(settings: Settings | None = None) -> FastAPI:
app.add_middleware(SetupRedirectMiddleware)
app.add_middleware(MetricsMiddleware)
app.add_middleware(CsrfMiddleware)
app.add_middleware(DeprecationHeaderMiddleware)
app.add_middleware(
RateLimitMiddleware,
rate_limiter=app.state.global_rate_limiter,
@@ -1131,5 +1134,6 @@ def create_app(settings: Settings | None = None) -> FastAPI:
app.include_router(server.router)
app.include_router(history.router)
app.include_router(blocklist.router)
app.include_router(jails_v2.router)
return app

View File

@@ -0,0 +1,107 @@
"""Deprecation header middleware for versioned API endpoints.
Adds ``Deprecation``, ``Sunset``, and ``Link`` response headers to endpoints
that have been scheduled for removal, following RFC 8599 and the BanGUI
API_VERSIONING.md lifecycle policy.
"""
from __future__ import annotations
from datetime import datetime # noqa: TC003 # Used in stringized type annotations (future annotations)
from typing import TYPE_CHECKING
from starlette.middleware.base import BaseHTTPMiddleware
if TYPE_CHECKING:
from collections.abc import Awaitable, Callable
from starlette.requests import Request
from starlette.responses import Response as StarletteResponse
class DeprecatedEndpoint:
"""Describes a deprecated API endpoint and its removal schedule."""
__slots__ = ("path_prefix", "sunset_date", "successor_url")
def __init__(
self,
path_prefix: str,
sunset_date: datetime,
successor_url: str | None = None,
) -> None:
self.path_prefix = path_prefix
self.sunset_date = sunset_date
self.successor_url = successor_url
# Registry of deprecated endpoints.
# Add entries here when an endpoint is scheduled for removal.
# sunset_date must be a timezone-aware datetime in UTC.
_DEPRECATED_ENDPOINTS: list[DeprecatedEndpoint] = []
def register_deprecated_endpoint(
path_prefix: str,
sunset_date: datetime,
successor_url: str | None = None,
) -> None:
"""Register a deprecated endpoint for deprecation header injection."""
_DEPRECATED_ENDPOINTS.append(
DeprecatedEndpoint(path_prefix, sunset_date, successor_url)
)
def _is_deprecated(path: str) -> DeprecatedEndpoint | None:
for endpoint in _DEPRECATED_ENDPOINTS:
if path.startswith(endpoint.path_prefix):
return endpoint
return None
def _format_rfc5322(dt: datetime) -> str:
return dt.strftime("%a, %d %b %Y %H:%M:%S GMT")
class DeprecationHeaderMiddleware(BaseHTTPMiddleware):
"""Inject deprecation headers on responses from deprecated endpoints.
For any response from a path registered in ``_DEPRECATED_ENDPOINTS``,
this middleware appends:
- ``Deprecation: true``
- ``Sunset: <RFC-5322 date>``
- ``Link: <<successor_url>>; rel="successor-version"`` (if successor_url set)
The middleware runs after the response is generated so it has access
to the final status code and can choose to only add headers for 2xx
responses (non-error responses from a deprecated endpoint are what
clients need to be warned about).
"""
async def dispatch(
self,
request: Request,
call_next: Callable[[Request], Awaitable[StarletteResponse]],
) -> StarletteResponse:
response: StarletteResponse = await call_next(request)
# Add deprecation headers for 2xx and 3xx responses from deprecated paths.
# Skipping 4xx/5xx avoids polluting error responses with deprecation headers.
if response.status_code < 200 or response.status_code >= 400:
return response
deprecated = _is_deprecated(request.url.path)
if deprecated is None:
return response
# All deprecation dates are stored in UTC.
sunset_str = _format_rfc5322(deprecated.sunset_date)
response.headers["Deprecation"] = "true"
response.headers["Sunset"] = sunset_str
if deprecated.successor_url:
response.headers["Link"] = f'<{deprecated.successor_url}>; rel="successor-version"'
return response

View File

@@ -0,0 +1,40 @@
"""Jails router — v2 (pre-production).
This router contains the next-generation version of the jails API,
intended for the v2 breaking-change release. It is registered in
``app/main.py`` but is NOT active for production traffic until the
v2 launch milestone is completed.
To activate v2 for a specific endpoint, move the handler from the v1
router to this router and remove the ``include_in_schema=False`` flag.
Until then, all endpoints here return ``404 Not Found`` so they do not
interfere with live v1 traffic.
"""
from __future__ import annotations
from fastapi import APIRouter, status
from fastapi.responses import JSONResponse
#: Set to ``True`` once v2 is declared production-ready.
_V2_ENABLED: bool = False
router: APIRouter = APIRouter(prefix="/api/v2/jails", tags=["Jails (v2)"])
@router.get(
"",
include_in_schema=False,
)
async def jails_v2_placeholder() -> JSONResponse:
"""Placeholder handler — v2 not yet enabled.
Returns 404 so this router never serves real traffic until v2 is
explicitly activated. When v2 goes live, remove this handler and
expose the real endpoints.
"""
return JSONResponse(
status_code=status.HTTP_404_NOT_FOUND,
content={"detail": "v2 endpoint not yet available"},
)

View File

@@ -0,0 +1,44 @@
#!/usr/bin/env python3
"""Generate the OpenAPI spec and save it to openapi.json.
This script is used by the CI OpenAPI breaking-change check. It creates
the FastAPI app (without starting the server) and serialises the OpenAPI
schema to stdout or a file.
Usage:
python scripts/generate_openapi.py [output_path]
"""
from __future__ import annotations
import json
import sys
from pathlib import Path
if __name__ == "__main__":
# Add the backend directory to the path so we can import app modules.
backend_root = Path(__file__).parent.parent
sys.path.insert(0, str(backend_root))
from app.config import Settings
from app.main import create_app
settings = Settings(
database_path="/tmp/test.db",
fail2ban_socket="/tmp/fake.sock",
fail2ban_config_dir="/tmp/fail2ban",
session_secret="openapi-script-secret-do-not-use-in-production",
session_duration_minutes=60,
timezone="UTC",
log_level="critical",
)
app = create_app(settings=settings)
spec = app.openapi()
output = sys.argv[1] if len(sys.argv) > 1 else None
if output:
Path(output).write_text(json.dumps(spec, indent=2))
print(f"OpenAPI spec written to {output}", file=sys.stderr)
else:
print(json.dumps(spec, indent=2))

View File

@@ -0,0 +1,88 @@
"""Tests for the deprecation header middleware."""
from datetime import UTC, datetime, timedelta
import pytest
from httpx import ASGITransport, AsyncClient
from app.main import create_app
from app.middleware.deprecation import (
_DEPRECATED_ENDPOINTS,
_is_deprecated,
register_deprecated_endpoint,
)
def _make_utc(days_from_now: int) -> datetime:
return datetime.now(UTC) + timedelta(days=days_from_now)
@pytest.fixture
def clean_registry() -> list:
"""Clear the deprecated endpoints registry before and after each test."""
original = list(_DEPRECATED_ENDPOINTS)
_DEPRECATED_ENDPOINTS.clear()
yield _DEPRECATED_ENDPOINTS
_DEPRECATED_ENDPOINTS.clear()
_DEPRECATED_ENDPOINTS.extend(original)
class TestIsDeprecated:
def test_path_matches_registered_prefix(self, clean_registry: list) -> None:
register_deprecated_endpoint("/api/v1/jails", _make_utc(180))
assert _is_deprecated("/api/v1/jails") is not None
assert _is_deprecated("/api/v1/jails/test-jail") is not None
def test_path_does_not_match_unregistered_prefix(self, clean_registry: list) -> None:
register_deprecated_endpoint("/api/v1/jails", _make_utc(180))
assert _is_deprecated("/api/v1/bans") is None
def test_empty_registry_returns_none(self, clean_registry: list) -> None:
assert _is_deprecated("/api/v1/jails") is None
class TestDeprecationHeadersIntegration:
@pytest.mark.asyncio
async def test_deprecated_endpoint_gets_headers(self, clean_registry: list) -> None:
register_deprecated_endpoint("/api/v1/jails", _make_utc(180), successor_url="/api/v2/jails")
settings = pytest.importorskip("app.config").Settings(
database_path="/tmp/test.db",
fail2ban_socket="/tmp/fake.sock",
fail2ban_config_dir="/tmp/fail2ban",
session_secret="test-secret-key-do-not-use-in-production",
session_duration_minutes=60,
timezone="UTC",
log_level="debug",
)
app = create_app(settings=settings)
async with AsyncClient(
transport=ASGITransport(app=app), base_url="http://test"
) as client:
response = await client.get("/api/v1/jails")
# 307 = setup redirect (app redirects unauthenticated/unconfigured requests)
assert response.status_code in (200, 307, 401, 403, 404)
assert "Deprecation" in response.headers or "Sunset" in response.headers
@pytest.mark.asyncio
async def test_non_deprecated_endpoint_no_headers(self, clean_registry: list) -> None:
register_deprecated_endpoint("/api/v1/jails", _make_utc(180))
settings = pytest.importorskip("app.config").Settings(
database_path="/tmp/test.db",
fail2ban_socket="/tmp/fake.sock",
fail2ban_config_dir="/tmp/fail2ban",
session_secret="test-secret-key-do-not-use-in-production",
session_duration_minutes=60,
timezone="UTC",
log_level="debug",
)
app = create_app(settings=settings)
async with AsyncClient(
transport=ASGITransport(app=app), base_url="http://test"
) as client:
response = await client.get("/api/v1/bans")
# No Deprecation header on non-deprecated path
assert "Deprecation" not in response.headers