Nguyen Minh Phuc
66eb6cb0ee
The codebase was written by several agents and had the same concept done more
than one way. This makes it read as one voice, with no behaviour change.
Dedup, each to a single canonical form:
- INSTANCE_COLUMNS: the 13-column instances SELECT list existed as _COLUMNS in
instances.py and reconcile.py (byte-identical) and inlined a third time in
the worker. One exported constant now.
- Settings.runtime_dsn: the three entrypoints each chose between str(pg_dsn)
and pg_dsn.unicode_string(). One property.
- yaml_tempfile: helm._ValuesFile and k8s._ManifestFile were the same
write-yaml-to-a-temp-dir context manager. One helper in adapters/tempyaml.py.
- services/_runtime.py: sleep_or_stop and install_stop_signals were copied
between the worker and reconciler loops. One module, so shutdown behaviour
cannot drift between them.
- k8s.ensure_namespace used MANAGED_BY_LABEL/VALUE from helm.py instead of a
hardcoded literal, so the managed-by label has one definition.
- SvcforgeError is now the root of every svcforge exception (CatalogError,
IllegalTransition, BadWindow, HandlerError), keeping each stdlib base in the
MRO, so `except SvcforgeError` means what errors.py says it does.
- ERROR_MAX_CHARS replaces the repeated `[-2000:]` truncation feeding the same
error columns.
- the reconciler reads settings.metrics_port like the worker, dropping its
duplicate DEFAULT_METRICS_PORT and redundant --metrics-port option; the
SVCFORGE_METRICS_PORT env override still applies through pydantic.
Two smaller correctness/consistency fixes:
- RateLimitResult.retry_after_s computed its delta against datetime.now(UTC)
while the limiter runs on an injectable clock, so it was meaningless under a
FakeClock and drifted by request latency in production. It now carries a
checked_at from the same clock as reset_at.
- handle_provision's notifier.send is wrapped like the reconciler's: a flaky
webhook after the READY CAS would fail the task, and the retry would hit the
READY early-return and drop the notification, turning a good provision into a
failed one.
172 lines
8.0 KiB
Python
172 lines
8.0 KiB
Python
"""The app factory and its lifespan.
|
|
|
|
`create_app(settings)` is a factory, not a module-level `app = FastAPI()`, for one reason:
|
|
a test needs an app pointed at a throwaway Postgres, and an import-time app reads the real
|
|
environment at import time — before any fixture can say otherwise.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import asyncio
|
|
from collections.abc import AsyncIterator
|
|
from contextlib import asynccontextmanager
|
|
|
|
from fastapi import FastAPI, Request
|
|
from fastapi.exceptions import RequestValidationError
|
|
from fastapi.responses import JSONResponse
|
|
from jwt import PyJWKClient
|
|
from starlette.exceptions import HTTPException
|
|
|
|
from services.api.models import ErrorBody
|
|
from services.api.routes import health, instances
|
|
from svcforge_core import obs
|
|
from svcforge_core.adapters.redis import RateLimiter, make_redis
|
|
from svcforge_core.domain.catalog import load_catalog
|
|
from svcforge_core.repo.db import make_pool
|
|
from svcforge_core.settings import Settings, load_settings
|
|
|
|
log = obs.get_logger("svcforge.api")
|
|
|
|
|
|
@asynccontextmanager
|
|
async def lifespan(app: FastAPI) -> AsyncIterator[None]:
|
|
"""Open the pool, yield, close the pool.
|
|
|
|
A lifespan context, not the deprecated startup/shutdown event decorators: those cannot
|
|
express "this resource lives for exactly as long as the app", and give you no place to
|
|
put the teardown next to the setup. Closing the pool matters — an unclosed pool means
|
|
connections linger server-side after SIGTERM, and on a pooled Postgres with a small
|
|
connection budget a few rolling deploys exhaust it.
|
|
|
|
(The old decorator's name is spelled nowhere in this package on purpose: CI greps for
|
|
the literal string, and a comment quoting it fails the gate just as loudly as a call.)
|
|
"""
|
|
settings: Settings = app.state.settings
|
|
|
|
app.state.catalog = load_catalog(settings.catalog_path)
|
|
|
|
# Redis is optional by construction. `make_redis` returns None when no DSN is set, and
|
|
# every consumer treats None as "skip" — so a deployment without Redis loses rate
|
|
# limiting and keeps everything else. Built here rather than per request because a
|
|
# connection pool per request is a connection pool per request.
|
|
redis = make_redis(settings)
|
|
app.state.redis = redis
|
|
app.state.rate_limiter = (
|
|
RateLimiter(redis, limit=settings.rate_limit_per_minute, window_s=60) if redis is not None else None
|
|
)
|
|
|
|
pool = make_pool(settings.runtime_dsn, settings.pool_min_size, settings.pool_max_size)
|
|
# wait=True fails NOW, loudly, if the DSN is wrong — instead of at the first request,
|
|
# as a PoolTimeout, in front of a user.
|
|
await pool.open(wait=True)
|
|
app.state.pool = pool
|
|
|
|
# The pool is open from here on, so everything below is inside the try: an exception
|
|
# in JWKS setup must still close it, or a crash-looping pod leaks a connection per
|
|
# restart until the database refuses new ones.
|
|
try:
|
|
if settings.jwks_url and not settings.auth_disabled:
|
|
client = PyJWKClient(settings.jwks_url, cache_keys=True, lifespan=300)
|
|
app.state.jwks_client = client
|
|
# Warm the cache off the loop so the first authenticated request does not pay
|
|
# a blocking urlopen. Best-effort: a slow identity provider must not stop the
|
|
# pod from starting — a cache miss later just costs one to_thread hop.
|
|
try:
|
|
await asyncio.to_thread(client.get_signing_keys)
|
|
except Exception: # deliberate catch-all: startup must not hinge on the IdP being up
|
|
log.warning("JWKS warm-up failed; keys will be fetched on first use", exc_info=True)
|
|
else:
|
|
app.state.jwks_client = None
|
|
|
|
yield
|
|
finally:
|
|
await pool.close()
|
|
if redis is not None:
|
|
await redis.aclose()
|
|
|
|
|
|
async def _http_exception_handler(request: Request, exc: Exception) -> JSONResponse:
|
|
"""Render HTTPException bodies as ErrorBody, so every error has one shape.
|
|
|
|
Handlers raise `detail={"code": ..., "message": ...}`; FastAPI's default would nest
|
|
that under `{"detail": {...}}`. Plain-string details (raised by FastAPI itself, e.g.
|
|
a 405) are wrapped so clients never have to branch on the body's type.
|
|
|
|
Registered on starlette's HTTPException, not fastapi's. fastapi.HTTPException is a
|
|
subclass, and Starlette matches handlers by walking type(exc).__mro__, so a handler
|
|
keyed on the subclass never fires for a framework-raised 404 or 405 — which are
|
|
starlette.HTTPException instances. Keying on the parent catches both: app handlers
|
|
raise the FastAPI subclass with a dict detail, the framework raises the parent with a
|
|
str detail, and the branch below renders each into ErrorBody.
|
|
"""
|
|
assert isinstance(exc, HTTPException) # noqa: S101 - registered only for HTTPException
|
|
# Widened to object deliberately. Starlette types `detail` as str, but FastAPI passes
|
|
# through whatever a handler raised — our handlers raise dicts. Narrowing off the
|
|
# declared type would make mypy call the dict branch unreachable and delete it.
|
|
detail: object = exc.detail
|
|
if isinstance(detail, dict) and "code" in detail and "message" in detail:
|
|
body = ErrorBody(code=str(detail["code"]), message=str(detail["message"]))
|
|
else:
|
|
body = ErrorBody(code=f"http_{exc.status_code}", message=str(detail))
|
|
return JSONResponse(status_code=exc.status_code, content=body.model_dump(), headers=exc.headers)
|
|
|
|
|
|
async def _validation_exception_handler(request: Request, exc: Exception) -> JSONResponse:
|
|
"""Render request-validation failures as ErrorBody too.
|
|
|
|
A body that fails validation (a forbidden extra field, a bad type, an out-of-range
|
|
ttl_days) raises RequestValidationError, which the HTTPException handler above never
|
|
sees. Without this it returns FastAPI's default `{"detail": [...]}` — a second 422 shape
|
|
alongside the ErrorBody 422s the handlers raise. This gives every 422 one shape.
|
|
"""
|
|
assert isinstance(exc, RequestValidationError) # noqa: S101 - registered only for this
|
|
return JSONResponse(
|
|
status_code=422,
|
|
content=ErrorBody(code="validation_error", message=str(exc.errors())).model_dump(),
|
|
)
|
|
|
|
|
|
def create_app(settings: Settings | None = None) -> FastAPI:
|
|
"""App factory: lifespan, routers, exception handler, /metrics."""
|
|
settings = settings or load_settings()
|
|
|
|
# FIRST, before any router is built and before any logger is bound. Without this the
|
|
# API is the one service of three that never configures structlog: its lines go out
|
|
# through logging.lastResort as bare text on stderr with no service, no trace_id and
|
|
# no JSON envelope — a parse failure in the collector, and unattributable in Loki.
|
|
# `settings.log_json` was silently inert here for the same reason.
|
|
obs.setup("svcforge-api", settings)
|
|
|
|
# Refuse the dev escape hatches when SVCFORGE_ENVIRONMENT says this is not a laptop.
|
|
# Called unconditionally and early: a check that only runs from a branch someone
|
|
# remembered to write is a check that does not run.
|
|
settings.check_production()
|
|
|
|
app = FastAPI(
|
|
title="svcforge",
|
|
version="0.1.0",
|
|
summary="X-as-a-Service control plane",
|
|
lifespan=lifespan,
|
|
)
|
|
app.state.settings = settings
|
|
|
|
# /metrics is a normal route on health.router, not an app.mount — see health.metrics
|
|
# for why the mount does not actually serve a bare /metrics.
|
|
app.include_router(health.router)
|
|
app.include_router(instances.router)
|
|
|
|
app.add_exception_handler(HTTPException, _http_exception_handler)
|
|
app.add_exception_handler(RequestValidationError, _validation_exception_handler)
|
|
return app
|
|
|
|
|
|
def app() -> FastAPI:
|
|
"""Entry point for `uvicorn services.api.main:app --factory`."""
|
|
return create_app()
|
|
|
|
|
|
# There is deliberately no `if __name__ == "__main__"` here. `services/api/__main__.py` is
|
|
# the single entrypoint, and the image's ENTRYPOINT uses it. A second one in this module
|
|
# drifted from it — different log_level, different access_log — so `python -m services.api`
|
|
# and `python services/api/main.py` started the same app two different ways.
|