Survive credential rotation: re-read the lease for every connection
The deployment ran for one lease window and then sat unready for eight hours. Platform credentials are 30-minute leases, not passwords: the service read the mounted URL once at start-up, so External Secrets kept the file current while the engine held the URL it booted with, and every reconnection after the first expiry used a credential the database had already revoked. make_engine now takes an optional refresh callable, invoked by a do_connect hook each time the pool opens a connection, and pool_recycle is 900s so a pooled connection is retired well inside the lease. Only username and password are taken from the refreshed URL — host, port and database come from the engine, so a malformed refresh cannot silently redirect the service somewhere else. Two things behaved correctly and are worth keeping. /readyz reported the real cause, "database unreachable: OperationalError", rather than a generic failure. And liveness stayed independent of the database, so the pod was never restart-looped: it was alive, unable to serve, and said so. Pointing liveness at a database-dependent path would have masked this as a crash loop. Service tests 47 -> 49, including one asserting pool_recycle stays inside the shortest lease the platform issues. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bjefh8NUiEiahN4JLwoSKM Assistant: claude-code Assistant-Model: opus Assistant-Process: 388925@bnt-lap001 Assistant-Session: 3507023f-e0fd-4a1e-9d90-a0d4217d1502
This commit is contained in:
parent
8a1a2426d5
commit
f6e20b5e0c
5 changed files with 76 additions and 9 deletions
|
|
@ -4,7 +4,7 @@ build-backend = "hatchling.build"
|
|||
|
||||
[project]
|
||||
name = "canned-prompts-service"
|
||||
version = "0.1.4"
|
||||
version = "0.1.5"
|
||||
description = "Hosted registry and index service for Canned Prompt Format packages"
|
||||
requires-python = ">=3.12"
|
||||
dependencies = [
|
||||
|
|
|
|||
|
|
@ -8,4 +8,4 @@ boundary holds, so rendering stays deterministic and a `derive` default remains
|
|||
a declaration the service does not satisfy.
|
||||
"""
|
||||
|
||||
__version__ = "0.1.4"
|
||||
__version__ = "0.1.5"
|
||||
|
|
|
|||
|
|
@ -24,7 +24,12 @@ from .settings import Settings, get_settings
|
|||
def create_app(settings: Settings | None = None, engine: Engine | None = None) -> FastAPI:
|
||||
settings = settings or get_settings()
|
||||
if engine is None and settings.configured:
|
||||
engine = make_engine(settings.resolved_database_url)
|
||||
# Pass the resolver, not just the value: the credential is a lease and
|
||||
# the mounted file is refreshed under us.
|
||||
engine = make_engine(
|
||||
settings.resolved_database_url,
|
||||
refresh=lambda: settings.resolved_database_url,
|
||||
)
|
||||
|
||||
app = FastAPI(title="canned-prompts registry", version=__version__)
|
||||
app.state.settings = settings
|
||||
|
|
|
|||
|
|
@ -11,8 +11,10 @@ from dataclasses import dataclass
|
|||
import os
|
||||
import re
|
||||
|
||||
from sqlalchemy import create_engine, text
|
||||
from sqlalchemy.engine import Engine
|
||||
from typing import Callable
|
||||
|
||||
from sqlalchemy import create_engine, event, text
|
||||
from sqlalchemy.engine import Engine, make_url
|
||||
from sqlalchemy.orm import DeclarativeBase, Session, sessionmaker
|
||||
|
||||
|
||||
|
|
@ -20,10 +22,44 @@ class Base(DeclarativeBase):
|
|||
pass
|
||||
|
||||
|
||||
def make_engine(database_url: str) -> Engine:
|
||||
# future=True is the 2.0 default; pool_pre_ping keeps a recycled connection
|
||||
# from surfacing as a request error after a database restart.
|
||||
return create_engine(database_url, pool_pre_ping=True, future=True)
|
||||
# Well under the shortest credential lease the platform issues (30 minutes for
|
||||
# the runtime role), so a pooled connection is retired before its credential
|
||||
# expires rather than failing mid-request.
|
||||
POOL_RECYCLE_SECONDS = 900
|
||||
|
||||
|
||||
def make_engine(database_url: str, refresh: "Callable[[], str] | None" = None) -> Engine:
|
||||
"""Build the engine, re-reading credentials for every new connection.
|
||||
|
||||
`refresh`, when given, is called each time the pool opens a DBAPI
|
||||
connection and returns the *current* URL.
|
||||
|
||||
This matters because platform credentials are short leases, not passwords.
|
||||
Reading the mounted file once at start-up worked for exactly one lease
|
||||
window and then failed permanently: External Secrets kept the file current,
|
||||
the engine kept the URL it booted with, and every reconnection used a
|
||||
credential the database had already revoked.
|
||||
"""
|
||||
engine = create_engine(database_url, pool_pre_ping=True, future=True,
|
||||
pool_recycle=POOL_RECYCLE_SECONDS)
|
||||
if refresh is None:
|
||||
return engine
|
||||
|
||||
@event.listens_for(engine, "do_connect")
|
||||
def _use_current_credentials(dialect, conn_rec, cargs, cparams): # noqa: ANN001
|
||||
current = refresh()
|
||||
if not current:
|
||||
return None
|
||||
url = make_url(current)
|
||||
# Only the parts a lease rotates. Host, port and database come from the
|
||||
# engine so a malformed refresh cannot silently redirect the service.
|
||||
if url.username:
|
||||
cparams["user"] = url.username
|
||||
if url.password:
|
||||
cparams["password"] = url.password
|
||||
return None
|
||||
|
||||
return engine
|
||||
|
||||
|
||||
def make_session_factory(engine: Engine) -> sessionmaker[Session]:
|
||||
|
|
|
|||
|
|
@ -138,3 +138,29 @@ def test_absent_database_file_still_fails_loudly(tmp_path: Path) -> None:
|
|||
settings = Settings(database_url_file=str(tmp_path / "absent"))
|
||||
with pytest.raises(RuntimeError, match="cannot read secret file"):
|
||||
_ = settings.resolved_database_url
|
||||
|
||||
|
||||
def test_engine_rereads_credentials_on_each_connection(tmp_path: Path) -> None:
|
||||
"""Platform credentials are 30-minute leases, not passwords. Reading the
|
||||
file once at start-up worked for one lease window and then failed
|
||||
permanently — External Secrets kept the file current while the engine kept
|
||||
the URL it booted with."""
|
||||
secret = tmp_path / "url"
|
||||
secret.write_text(f"sqlite:///{tmp_path / 'a.db'}", encoding="utf-8")
|
||||
settings = Settings(database_url_file=str(secret))
|
||||
|
||||
seen: list[str] = []
|
||||
engine = make_engine(
|
||||
settings.resolved_database_url,
|
||||
refresh=lambda: (seen.append(settings.resolved_database_url) or settings.resolved_database_url),
|
||||
)
|
||||
with engine.connect():
|
||||
pass
|
||||
assert seen, "the refresh hook must run when the pool opens a connection"
|
||||
|
||||
|
||||
def test_pool_recycle_is_shorter_than_the_shortest_lease() -> None:
|
||||
"""30-minute runtime lease; a pooled connection must be retired first."""
|
||||
from canned_prompts_service.db import POOL_RECYCLE_SECONDS
|
||||
|
||||
assert POOL_RECYCLE_SECONDS < 30 * 60
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue