feat: comprehensive test infrastructure improvements
- Fix unit test imports: add env setup in conftest.py before module imports - Add 24 new auth router tests (RBAC, preferences, password validation) - Add 16 new tests for litellm and chat_summary routers - Apply black formatting and ruff linting across codebase - Add pre-commit hooks configuration (black, ruff, file checks) - Increase CI coverage threshold from 40% to 50% Test Results: - 206 tests passing (91 unit + 115 integration) - Coverage: 58.79% on core modules - auth.py: 57% → 85%, litellm.py: 23% → 87%, chat_summary.py: 41% → 100% - auth_service: 96.51%, config: 100%, rbac: 93.48%
This commit is contained in:
+116
-63
@@ -1,30 +1,49 @@
|
||||
from fastapi import FastAPI, Depends, Request
|
||||
from fastapi.staticfiles import StaticFiles
|
||||
import asyncio
|
||||
import logging
|
||||
import time
|
||||
import traceback as tb
|
||||
from contextlib import asynccontextmanager
|
||||
from datetime import datetime, timedelta, timezone
|
||||
|
||||
import docker.errors
|
||||
import httpx
|
||||
from fastapi import Depends, FastAPI, Request
|
||||
from fastapi.exceptions import RequestValidationError
|
||||
from fastapi.middleware.cors import CORSMiddleware
|
||||
from fastapi.middleware.gzip import GZipMiddleware
|
||||
from fastapi.responses import JSONResponse
|
||||
from fastapi.exceptions import RequestValidationError
|
||||
from fastapi.staticfiles import StaticFiles
|
||||
from slowapi import Limiter
|
||||
from slowapi.util import get_remote_address
|
||||
from slowapi.errors import RateLimitExceeded
|
||||
from routers import docker_router, gitea, files, terminal, system, chat_summary, security, passkey, totp, litellm, cc_connect, info_engine, opc_tasks, opc_agents, opc_ws, opc_projects, auth as auth_router
|
||||
from slowapi.util import get_remote_address
|
||||
|
||||
import auth_service as auth_module
|
||||
import config
|
||||
from config import CORS_ORIGINS, TELEGRAM_BOT_TOKEN, TELEGRAM_CHAT_ID, VOLUME_ROOT, get_client_ip
|
||||
from rbac import require_page
|
||||
from contextlib import asynccontextmanager
|
||||
|
||||
import asyncio
|
||||
import httpx
|
||||
import logging
|
||||
import time
|
||||
import jwt
|
||||
import docker.errors
|
||||
import traceback as tb
|
||||
from datetime import datetime, timezone, timedelta
|
||||
from config import TELEGRAM_BOT_TOKEN, TELEGRAM_CHAT_ID, CORS_ORIGINS, SECRET_KEY, ALGORITHM, VOLUME_ROOT, get_client_ip
|
||||
from routers import auth as auth_router
|
||||
from routers import (
|
||||
cc_connect,
|
||||
chat_summary,
|
||||
docker_router,
|
||||
files,
|
||||
gitea,
|
||||
info_engine,
|
||||
litellm,
|
||||
opc_agents,
|
||||
opc_projects,
|
||||
opc_tasks,
|
||||
opc_ws,
|
||||
passkey,
|
||||
security,
|
||||
system,
|
||||
terminal,
|
||||
totp,
|
||||
)
|
||||
|
||||
_tz_cst = timezone(timedelta(hours=8))
|
||||
|
||||
|
||||
@asynccontextmanager
|
||||
async def lifespan(app: FastAPI):
|
||||
# Startup
|
||||
@@ -32,16 +51,19 @@ async def lifespan(app: FastAPI):
|
||||
|
||||
# Initialize OPC database
|
||||
from db import opc_db
|
||||
|
||||
try:
|
||||
await opc_db.init_db()
|
||||
print("OPC database initialized")
|
||||
except Exception as e:
|
||||
print(f"Failed to initialize OPC database: {e}")
|
||||
import traceback
|
||||
|
||||
traceback.print_exc()
|
||||
|
||||
# Start agent executor service
|
||||
from services import agent_executor
|
||||
|
||||
executor_task = None
|
||||
try:
|
||||
executor_task = asyncio.create_task(agent_executor.start_executor())
|
||||
@@ -49,6 +71,7 @@ async def lifespan(app: FastAPI):
|
||||
except Exception as e:
|
||||
print(f"Failed to start agent executor: {e}")
|
||||
import traceback
|
||||
|
||||
traceback.print_exc()
|
||||
|
||||
monitor_task = asyncio.create_task(monitor_containers())
|
||||
@@ -61,6 +84,7 @@ async def lifespan(app: FastAPI):
|
||||
executor_task.cancel()
|
||||
monitor_task.cancel()
|
||||
|
||||
|
||||
limiter = Limiter(key_func=get_remote_address)
|
||||
app = FastAPI(title="NAS Dashboard", lifespan=lifespan)
|
||||
app.state.limiter = limiter
|
||||
@@ -76,6 +100,7 @@ app.add_middleware(
|
||||
# Compress responses (static files and API responses)
|
||||
app.add_middleware(GZipMiddleware, minimum_size=1000)
|
||||
|
||||
|
||||
@app.exception_handler(RateLimitExceeded)
|
||||
async def rate_limit_handler(request: Request, exc: RateLimitExceeded):
|
||||
return JSONResponse(status_code=429, content={"detail": "Too many requests, try again later"})
|
||||
@@ -85,30 +110,21 @@ async def rate_limit_handler(request: Request, exc: RateLimitExceeded):
|
||||
async def validation_exception_handler(request: Request, exc: RequestValidationError):
|
||||
"""Handle request validation errors with detailed information."""
|
||||
logging.error(f"Validation error on {request.method} {request.url.path}: {exc.errors()}")
|
||||
return JSONResponse(
|
||||
status_code=422,
|
||||
content={"detail": exc.errors()}
|
||||
)
|
||||
return JSONResponse(status_code=422, content={"detail": exc.errors()})
|
||||
|
||||
|
||||
@app.exception_handler(docker.errors.NotFound)
|
||||
async def docker_not_found_handler(request: Request, exc: docker.errors.NotFound):
|
||||
"""Handle Docker container/image not found errors."""
|
||||
logging.warning(f"Docker resource not found: {exc}")
|
||||
return JSONResponse(
|
||||
status_code=404,
|
||||
content={"detail": "Docker resource not found"}
|
||||
)
|
||||
return JSONResponse(status_code=404, content={"detail": "Docker resource not found"})
|
||||
|
||||
|
||||
@app.exception_handler(docker.errors.APIError)
|
||||
async def docker_api_error_handler(request: Request, exc: docker.errors.APIError):
|
||||
"""Handle Docker API errors."""
|
||||
logging.error(f"Docker API error: {exc}")
|
||||
return JSONResponse(
|
||||
status_code=503,
|
||||
content={"detail": "Docker service unavailable"}
|
||||
)
|
||||
return JSONResponse(status_code=503, content={"detail": "Docker service unavailable"})
|
||||
|
||||
|
||||
@app.exception_handler(Exception)
|
||||
@@ -116,10 +132,8 @@ async def generic_exception_handler(request: Request, exc: Exception):
|
||||
"""Handle all unhandled exceptions."""
|
||||
logging.error(f"Unhandled exception on {request.method} {request.url.path}: {exc}")
|
||||
logging.error(tb.format_exc())
|
||||
return JSONResponse(
|
||||
status_code=500,
|
||||
content={"detail": "Internal server error"}
|
||||
)
|
||||
return JSONResponse(status_code=500, content={"detail": "Internal server error"})
|
||||
|
||||
|
||||
@app.middleware("http")
|
||||
async def security_headers(request: Request, call_next):
|
||||
@@ -128,7 +142,9 @@ async def security_headers(request: Request, call_next):
|
||||
response.headers["X-Frame-Options"] = "DENY"
|
||||
response.headers["X-XSS-Protection"] = "1; mode=block"
|
||||
response.headers["Referrer-Policy"] = "strict-origin-when-cross-origin"
|
||||
response.headers["Content-Security-Policy"] = "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; connect-src 'self' wss: ws:; frame-ancestors 'none'"
|
||||
response.headers["Content-Security-Policy"] = (
|
||||
"default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; connect-src 'self' wss: ws:; frame-ancestors 'none'"
|
||||
)
|
||||
|
||||
# Cache static assets with hashed filenames (Vite generates these)
|
||||
if request.url.path.startswith("/assets/"):
|
||||
@@ -136,8 +152,10 @@ async def security_headers(request: Request, call_next):
|
||||
|
||||
return response
|
||||
|
||||
|
||||
# Audit logger
|
||||
import os
|
||||
|
||||
_audit_log_path = os.path.join(VOLUME_ROOT, "docker/nas-dashboard/audit.log")
|
||||
os.makedirs(os.path.dirname(_audit_log_path), exist_ok=True)
|
||||
_audit_logger = logging.getLogger("audit")
|
||||
@@ -147,6 +165,7 @@ _audit_handler.setFormatter(logging.Formatter("%(message)s"))
|
||||
_audit_logger.addHandler(_audit_handler)
|
||||
_audit_logger.propagate = False
|
||||
|
||||
|
||||
@app.middleware("http")
|
||||
async def audit_log(request: Request, call_next):
|
||||
start = time.time()
|
||||
@@ -156,19 +175,27 @@ async def audit_log(request: Request, call_next):
|
||||
username = user.username if user else "-"
|
||||
_audit_logger.info(
|
||||
"%s %s %s %s %s %d %dms",
|
||||
datetime.now(_tz_cst).strftime("%Y-%m-%dT%H:%M:%S"), get_client_ip(request), username,
|
||||
request.method, request.url.path, response.status_code, duration,
|
||||
datetime.now(_tz_cst).strftime("%Y-%m-%dT%H:%M:%S"),
|
||||
get_client_ip(request),
|
||||
username,
|
||||
request.method,
|
||||
request.url.path,
|
||||
response.status_code,
|
||||
duration,
|
||||
)
|
||||
return response
|
||||
|
||||
|
||||
async def monitor_containers():
|
||||
import docker
|
||||
|
||||
from config import DOCKER_HOST
|
||||
|
||||
client = docker.DockerClient(base_url=DOCKER_HOST)
|
||||
|
||||
|
||||
# Store initial states
|
||||
previous_states = {}
|
||||
|
||||
|
||||
while True:
|
||||
try:
|
||||
containers = await asyncio.to_thread(client.containers.list, all=True)
|
||||
@@ -190,13 +217,15 @@ async def monitor_containers():
|
||||
previous_states[c.id] = current_status
|
||||
except Exception as e:
|
||||
logging.getLogger(__name__).error("Error in container monitor: %s", e)
|
||||
|
||||
|
||||
await asyncio.sleep(60)
|
||||
|
||||
|
||||
@app.get("/api/health")
|
||||
async def health():
|
||||
return {"status": "ok"}
|
||||
|
||||
|
||||
@app.get("/api/client-ip")
|
||||
async def client_ip(request: Request):
|
||||
ip = get_client_ip(request)
|
||||
@@ -215,8 +244,10 @@ async def client_ip(request: Request):
|
||||
|
||||
return {"lan": lan}
|
||||
|
||||
|
||||
def _router_reachable():
|
||||
import socket
|
||||
|
||||
try:
|
||||
s = socket.socket()
|
||||
s.settimeout(1)
|
||||
@@ -226,43 +257,63 @@ def _router_reachable():
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
|
||||
# Dependency to store user in request.state for rbac dependencies
|
||||
async def _inject_user(request: Request, user = Depends(auth_module.get_current_user)):
|
||||
async def _inject_user(request: Request, user=Depends(auth_module.get_current_user)):
|
||||
request.state.user = user
|
||||
return user
|
||||
|
||||
|
||||
# Public auth endpoints
|
||||
app.include_router(auth_router.router, prefix="/api/auth", tags=["auth"])
|
||||
app.include_router(passkey.router, prefix="/api/auth", tags=["passkey"])
|
||||
app.include_router(totp.router, prefix="/api/auth", tags=["totp"])
|
||||
|
||||
# Protected endpoints with page-level RBAC
|
||||
app.include_router(docker_router.router, prefix="/api/docker",
|
||||
dependencies=[Depends(_inject_user), Depends(require_page("docker"))])
|
||||
app.include_router(gitea.router, prefix="/api/gitea",
|
||||
dependencies=[Depends(_inject_user), Depends(require_page("gitea"))])
|
||||
app.include_router(files.router, prefix="/api/files",
|
||||
dependencies=[Depends(_inject_user), Depends(require_page("files"))])
|
||||
app.include_router(system.router, prefix="/api/system",
|
||||
dependencies=[Depends(_inject_user), Depends(require_page("dashboard"))])
|
||||
app.include_router(litellm.router, prefix="/api/litellm",
|
||||
dependencies=[Depends(_inject_user), Depends(require_page("dashboard"))])
|
||||
app.include_router(cc_connect.router, prefix="/api/cc-connect",
|
||||
dependencies=[Depends(_inject_user), Depends(require_page("dashboard"))])
|
||||
app.include_router(chat_summary.router, prefix="/api/chat-summary",
|
||||
dependencies=[Depends(_inject_user), Depends(require_page("chat-digest"))])
|
||||
app.include_router(info_engine.router, prefix="/api/info-engine",
|
||||
dependencies=[Depends(_inject_user), Depends(require_page("dashboard"))])
|
||||
app.include_router(security.router, prefix="/api/security",
|
||||
dependencies=[Depends(_inject_user), Depends(require_page("security"))])
|
||||
app.include_router(
|
||||
docker_router.router, prefix="/api/docker", dependencies=[Depends(_inject_user), Depends(require_page("docker"))]
|
||||
)
|
||||
app.include_router(
|
||||
gitea.router, prefix="/api/gitea", dependencies=[Depends(_inject_user), Depends(require_page("gitea"))]
|
||||
)
|
||||
app.include_router(
|
||||
files.router, prefix="/api/files", dependencies=[Depends(_inject_user), Depends(require_page("files"))]
|
||||
)
|
||||
app.include_router(
|
||||
system.router, prefix="/api/system", dependencies=[Depends(_inject_user), Depends(require_page("dashboard"))]
|
||||
)
|
||||
app.include_router(
|
||||
litellm.router, prefix="/api/litellm", dependencies=[Depends(_inject_user), Depends(require_page("dashboard"))]
|
||||
)
|
||||
app.include_router(
|
||||
cc_connect.router,
|
||||
prefix="/api/cc-connect",
|
||||
dependencies=[Depends(_inject_user), Depends(require_page("dashboard"))],
|
||||
)
|
||||
app.include_router(
|
||||
chat_summary.router,
|
||||
prefix="/api/chat-summary",
|
||||
dependencies=[Depends(_inject_user), Depends(require_page("chat-digest"))],
|
||||
)
|
||||
app.include_router(
|
||||
info_engine.router,
|
||||
prefix="/api/info-engine",
|
||||
dependencies=[Depends(_inject_user), Depends(require_page("dashboard"))],
|
||||
)
|
||||
app.include_router(
|
||||
security.router, prefix="/api/security", dependencies=[Depends(_inject_user), Depends(require_page("security"))]
|
||||
)
|
||||
|
||||
# OPC endpoints
|
||||
app.include_router(opc_tasks.router, prefix="/api/opc",
|
||||
dependencies=[Depends(_inject_user), Depends(require_page("opc"))])
|
||||
app.include_router(opc_agents.router, prefix="/api/opc",
|
||||
dependencies=[Depends(_inject_user), Depends(require_page("opc"))])
|
||||
app.include_router(opc_projects.router, prefix="/api/opc",
|
||||
dependencies=[Depends(_inject_user), Depends(require_page("opc"))])
|
||||
app.include_router(
|
||||
opc_tasks.router, prefix="/api/opc", dependencies=[Depends(_inject_user), Depends(require_page("opc"))]
|
||||
)
|
||||
app.include_router(
|
||||
opc_agents.router, prefix="/api/opc", dependencies=[Depends(_inject_user), Depends(require_page("opc"))]
|
||||
)
|
||||
app.include_router(
|
||||
opc_projects.router, prefix="/api/opc", dependencies=[Depends(_inject_user), Depends(require_page("opc"))]
|
||||
)
|
||||
|
||||
# WebSocket endpoint (auth handled inside handler via auth cookie; query token fallback temporary)
|
||||
app.add_api_websocket_route("/ws/terminal", terminal.ws_endpoint)
|
||||
@@ -270,11 +321,13 @@ app.add_api_websocket_route("/ws/opc", opc_ws.websocket_endpoint)
|
||||
|
||||
# Mount static files (skip if directory doesn't exist, e.g., in test environments)
|
||||
import os
|
||||
|
||||
if os.path.isdir("/app/static"):
|
||||
app.mount("/", StaticFiles(directory="/app/static", html=True), name="static")
|
||||
else:
|
||||
# In test environment, create a minimal fallback
|
||||
import tempfile
|
||||
|
||||
_test_static = tempfile.mkdtemp()
|
||||
with open(os.path.join(_test_static, "index.html"), "w") as f:
|
||||
f.write("<html><body>Test Environment</body></html>")
|
||||
|
||||
Reference in New Issue
Block a user