diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 612421e..20aa935 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -98,4 +98,77 @@ jobs: run: pip install -e ".[dev]" - name: Run import-linter - run: linter \ No newline at end of file + 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>" \ No newline at end of file diff --git a/Docs/API_VERSIONING.md b/Docs/API_VERSIONING.md index 2d5f17a..c00d8bc 100644 --- a/Docs/API_VERSIONING.md +++ b/Docs/API_VERSIONING.md @@ -23,8 +23,8 @@ This document explains when and how to version endpoints, how deprecation works, /api/v{major}// ``` -- **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: Link: ; 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/` | \ No newline at end of file +| 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 | \ No newline at end of file diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 27f6670..3a7fbc9 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -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**: diff --git a/backend/app/main.py b/backend/app/main.py index 965bc82..6358ab0 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -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 diff --git a/backend/app/middleware/deprecation.py b/backend/app/middleware/deprecation.py new file mode 100644 index 0000000..69a728f --- /dev/null +++ b/backend/app/middleware/deprecation.py @@ -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: `` + - ``Link: <>; 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 diff --git a/backend/app/routers/jails_v2.py b/backend/app/routers/jails_v2.py new file mode 100644 index 0000000..e9b1bef --- /dev/null +++ b/backend/app/routers/jails_v2.py @@ -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"}, + ) diff --git a/backend/scripts/generate_openapi.py b/backend/scripts/generate_openapi.py new file mode 100644 index 0000000..822f4c2 --- /dev/null +++ b/backend/scripts/generate_openapi.py @@ -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)) diff --git a/backend/tests/test_deprecation_middleware.py b/backend/tests/test_deprecation_middleware.py new file mode 100644 index 0000000..40262f0 --- /dev/null +++ b/backend/tests/test_deprecation_middleware.py @@ -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