From f6c3c021836a27ef1ad93a51e96015df6b556a96 Mon Sep 17 00:00:00 2001 From: Lukas Date: Sat, 2 May 2026 21:57:00 +0200 Subject: [PATCH] Refactor response handling and health check endpoints - Enhance response model with additional fields and validation - Update health and server router implementations - Improve frontend type definitions and API integration - Clean up documentation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Docs/Tasks.md | 43 ----------------------------- backend/app/models/response.py | 50 +++++++++++++++++++++++++++++++++- backend/app/routers/health.py | 12 ++++---- backend/app/routers/server.py | 8 ++++-- frontend/src/api/health.ts | 2 +- frontend/src/types/response.ts | 20 ++++++++++++++ frontend/src/types/server.ts | 12 ++++---- 7 files changed, 88 insertions(+), 59 deletions(-) diff --git a/Docs/Tasks.md b/Docs/Tasks.md index c797d7a..4398fc5 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -1,46 +1,3 @@ -### Issue #5: HIGH - API Has No Versioning Strategy - -**Where found**: -- All routers use `/api/` prefix with no version indicator -- `backend/app/main.py` - FastAPI app initialization -- Frontend type generation expects schema at `/api/openapi.json` -- No deprecation warnings in responses - -**Why this is needed**: -Without versioning, any breaking change in API responses immediately breaks all deployed frontend clients. There's no way to support multiple client versions. The current architecture has no migration path for API evolution. - -**Goal**: -Implement API versioning that allows: -- Supporting multiple client versions simultaneously -- Deprecating endpoints safely -- Rolling out breaking changes without downtime - -**What to do**: -1. Introduce versioning: `/api/v1/`, `/api/v2/`, etc. -2. Move all current endpoints to `/api/v1/` -3. Keep v1 stable and backward compatible -4. When breaking changes needed, create v2 endpoints -5. Add deprecation headers to old endpoints: `Deprecation: true`, `Sunset: date` -6. Update frontend to use versioned API URLs -7. Add CI check to prevent accidental breaking changes - -**Possible traps and issues**: -- Frontend needs to be updated to use versioned URLs -- Existing deployments must continue working (can't remove v1) -- Developers might accidentally break backward compatibility in v1 -- Multiple versions mean duplicate code (need proper code sharing) -- Route registering becomes more complex - -**Docs changes needed**: -- Create `Docs/API_VERSIONING.md` explaining strategy and when to bump versions -- Update API documentation to show which version each endpoint is in -- Add to `Docs/Backend-Development.md` - "Creating versioned endpoints" - -**Doc references**: -- DATABASE_API_DEPLOYMENT_ISSUES.md - Issue "2.2 No API Versioning Strategy" - ---- - ### Issue #6: HIGH - Inconsistent API Response Format Between Endpoints **Where found**: diff --git a/backend/app/models/response.py b/backend/app/models/response.py index 09f7f5a..17bd868 100644 --- a/backend/app/models/response.py +++ b/backend/app/models/response.py @@ -94,7 +94,7 @@ Note on field naming: - All responses with multiple items include a "total" field. """ -from typing import Generic, TypeVar +from typing import Generic, Literal, TypeVar from pydantic import BaseModel, ConfigDict, Field @@ -326,3 +326,51 @@ class ErrorResponse(BanGuiBaseModel): default=None, description="Unique ID for correlating this error with request logs on both frontend and backend.", ) + + +class HealthResponse(BanGuiBaseModel): + """Standardized response for the health check endpoint. + + Fields: + status: Application health status — 'ok' when healthy, 'unavailable' otherwise. + fail2ban: fail2ban daemon status — 'online' or 'offline'. + + Example: + ```python + # Healthy (HTTP 200) + { + "status": "ok", + "fail2ban": "online" + } + + # Unhealthy (HTTP 503) + { + "status": "unavailable", + "fail2ban": "offline" + } + ``` + """ + + status: Literal["ok", "unavailable"] = Field( + ..., + description="Application health status: 'ok' when healthy, 'unavailable' otherwise.", + ) + fail2ban: Literal["online", "offline"] = Field( + ..., + description="fail2ban daemon status: 'online' when reachable, 'offline' otherwise.", + ) + + +class FlushLogsResponse(BanGuiBaseModel): + """Standardized response for the flush-logs command endpoint. + + Fields: + message: Human-readable result message from fail2ban. + + Example: + ```python + {"message": "Success: fail2ban log files were flushed."} + ``` + """ + + message: str = Field(..., description="Human-readable result message from fail2ban.") diff --git a/backend/app/routers/health.py b/backend/app/routers/health.py index e63ab82..722cddb 100644 --- a/backend/app/routers/health.py +++ b/backend/app/routers/health.py @@ -10,11 +10,12 @@ from fastapi import APIRouter, status from fastapi.responses import JSONResponse from app.dependencies import ServerStatusDep +from app.models.response import HealthResponse router: APIRouter = APIRouter(prefix="/api/v1/health", tags=["Health"]) -@router.get("", summary="Application health check") +@router.get("", summary="Application health check", response_model=HealthResponse) async def health_check(server_status: ServerStatusDep) -> JSONResponse: """Return application and fail2ban status. @@ -27,17 +28,16 @@ async def health_check(server_status: ServerStatusDep) -> JSONResponse: server_status: Injected cached server status snapshot. Returns: - HTTP 200 with ``{"status": "ok", "fail2ban": "online"}`` if healthy, - or HTTP 503 with ``{"status": "unavailable", "fail2ban": "offline"}`` - if fail2ban is unreachable. + HTTP 200 with :class:`~app.models.response.HealthResponse` when healthy, + HTTP 503 with :class:`~app.models.response.HealthResponse` when fail2ban is offline. """ if not server_status.online: return JSONResponse( status_code=status.HTTP_503_SERVICE_UNAVAILABLE, - content={"status": "unavailable", "fail2ban": "offline"}, + content=HealthResponse(status="unavailable", fail2ban="offline").model_dump(), ) return JSONResponse( status_code=status.HTTP_200_OK, - content={"status": "ok", "fail2ban": "online"}, + content=HealthResponse(status="ok", fail2ban="online").model_dump(), ) diff --git a/backend/app/routers/server.py b/backend/app/routers/server.py index 4b31c6f..25de9d4 100644 --- a/backend/app/routers/server.py +++ b/backend/app/routers/server.py @@ -14,6 +14,7 @@ from fastapi import APIRouter, Request, status from app.dependencies import AuthDep, Fail2BanSocketDep from app.mappers import server_mappers +from app.models.response import FlushLogsResponse from app.models.server import ServerSettingsResponse, ServerSettingsUpdate from app.services import server_service @@ -86,12 +87,13 @@ async def update_server_settings( "/flush-logs", status_code=status.HTTP_200_OK, summary="Flush and re-open fail2ban log files", + response_model=FlushLogsResponse, ) async def flush_logs( request: Request, _auth: AuthDep, socket_path: Fail2BanSocketDep, -) -> dict[str, str]: +) -> FlushLogsResponse: """Flush and re-open fail2ban log files. Useful after log rotation so the daemon writes to the newly created @@ -102,11 +104,11 @@ async def flush_logs( _auth: Validated session. Returns: - ``{"message": ""}`` + :class:`~app.models.response.FlushLogsResponse` with the result from fail2ban. Raises: HTTPException: 400 when the command is rejected. HTTPException: 502 when fail2ban is unreachable. """ result = await server_service.flush_logs(socket_path) - return {"message": result} + return FlushLogsResponse(message=result) diff --git a/frontend/src/api/health.ts b/frontend/src/api/health.ts index bf28a23..e3ed6da 100644 --- a/frontend/src/api/health.ts +++ b/frontend/src/api/health.ts @@ -1,6 +1,6 @@ import { get } from "./client"; import { ENDPOINTS } from "./endpoints"; -import type { HealthResponse } from "../types/server"; +import type { HealthResponse } from "../types/response"; export async function fetchHealth(): Promise { return get(ENDPOINTS.health); diff --git a/frontend/src/types/response.ts b/frontend/src/types/response.ts index 26c6cc2..f0ae9a1 100644 --- a/frontend/src/types/response.ts +++ b/frontend/src/types/response.ts @@ -36,3 +36,23 @@ export interface ErrorResponse { /** Unique ID for correlating this error with request logs on both frontend and backend. */ correlation_id?: string; } + +/** + * Standardized health check response. + * Mirrors `backend/app/models/response.py:HealthResponse`. + */ +export interface HealthResponse { + /** Application health status: 'ok' when healthy, 'unavailable' otherwise. */ + status: "ok" | "unavailable"; + /** fail2ban daemon status: 'online' when reachable, 'offline' otherwise. */ + fail2ban: "online" | "offline"; +} + +/** + * Standardized flush-logs command response. + * Mirrors `backend/app/models/response.py:FlushLogsResponse`. + */ +export interface FlushLogsResponse { + /** Human-readable result message from fail2ban. */ + message: string; +} diff --git a/frontend/src/types/server.ts b/frontend/src/types/server.ts index 3ff076a..e4a34b8 100644 --- a/frontend/src/types/server.ts +++ b/frontend/src/types/server.ts @@ -4,6 +4,8 @@ * `backend/app/models/server.py` */ +import type { HealthResponse as HealthResponseBase } from "./response"; + /** Cached fail2ban server health snapshot. */ export interface ServerStatus { /** Whether fail2ban is reachable via its socket. */ @@ -23,8 +25,8 @@ export interface ServerStatusResponse { status: ServerStatus; } -/** Response shape for ``GET /api/health``. */ -export interface HealthResponse { - status: "ok"; - fail2ban: "online" | "offline"; -} +/** + * Response shape for ``GET /api/health``. + * Re-exports the canonical type from `types/response.ts`. + */ +export type HealthResponse = HealthResponseBase;