Files
Chris CoutinhoandClaude Opus 4.7 292cbb3292 feat(storage): pluggable database backend via DATABASE_URL (ADR-026)
Adds a `DATABASE_URL` setting that lets `RefreshTokenStorage` run against
any SQLAlchemy async backend, primarily `postgresql+asyncpg://...` for
HA k8s deployments. Default behavior is unchanged: when `DATABASE_URL` is
unset the server falls back to the existing `TOKEN_STORAGE_DB` path /
ephemeral SQLite tempfile.

Why
---
Today every MCP pod needs its own PVC to hold the SQLite file, which
pins the Deployment to one replica and blocks horizontal scaling. With
this change, operators can point all replicas at a shared Postgres
(CNPG, RDS, etc.) and the pods become stateless. Encryption stays in
Python (Fernet); the database only sees ciphertext.

What changed
------------
- `config.get_database_url()` resolves DATABASE_URL → TOKEN_STORAGE_DB →
  ephemeral tempfile in that priority order.
- `RefreshTokenStorage` builds a process-shared `AsyncEngine` in
  `initialize()`. SQLite gets NullPool; Postgres gets pool_size=10,
  max_overflow=20, pool_pre_ping=True. 30 aiosqlite call sites adapted
  via a thin `_DBConn` / `_Cursor` / `_Row` / `_ExecuteCtx` shim so
  existing method bodies need no churn beyond the connection
  context-manager swap.
- 7 `INSERT OR REPLACE` statements rewritten as portable
  `INSERT ... ON CONFLICT (...) DO UPDATE` (SQLite ≥ 3.24, Postgres ≥ 9.5).
- `sqlite_master` legacy-detection lookup replaced with SQLAlchemy
  inspector so the path works against either backend.
- File-permission hardening + parent-dir creation gated on
  `is_sqlite_url(...)` — centralized backends manage their own filesystem.
- Alembic migrations 001/002/003/005 converted from raw `op.execute(SQL)`
  to portable `op.create_table()` / `op.create_index()` with SQLAlchemy
  types. All timestamp columns are `sa.BigInteger` so Postgres allocates
  BIGINT (unix epochs don't fit in INT4). SQLite treats BIGINT as
  INTEGER, so existing deployments at revision 006 see no schema drift.
- `migrations.py` + CLI take URLs; `db {upgrade,downgrade,current,history}`
  gain `--database-url / -u` alongside the legacy `--database-path / -d`.
  `get_current_revision()` uses SQLAlchemy inspector instead of raw
  sqlite3, so the CLI works against Postgres too.
- `docker-compose.yml` adds a `postgres-test` service under the
  `postgres` profile (pinned `postgres:16-alpine` digest) for
  integration testing.
- Unit storage tests parametrized over backends via shared
  `tests/fixtures/storage_backend.py` — every test in
  `test_app_password_storage.py` and `test_webhook_storage.py` runs
  once per backend that is available. Postgres is opted in by
  `TEST_DATABASE_URL`.
- New `tests/integration/test_storage_postgres.py` (5 tests, marked
  `postgres` + `integration`) covers refresh-token, app-password,
  OAuth-session, webhook, and audit-log paths end-to-end on Postgres.
- New `docs/ADR-026-pluggable-database-backend.md` records the decision;
  `docs/configuration.md` documents `DATABASE_URL` with examples.

Out of scope
------------
- No SQLite → Postgres data migration tool (clean cutover; tokens reissue
  on next login, webhooks re-register on next sync tick).
- This repo does not provision Postgres. The matching helm chart change
  lives in cbcoutinho/helm-charts (database.url / existingSecret values).

Verification
------------
- `uv run pytest tests/unit/` — 1012 passed, SQLite path unchanged.
- `docker compose --profile postgres up -d postgres-test`
- `TEST_DATABASE_URL=... uv run pytest tests/integration/test_storage_postgres.py -m postgres -v`
  — 5 passed.
- `TEST_DATABASE_URL=... uv run pytest tests/unit/test_app_password_storage.py
  tests/unit/test_webhook_storage.py` — 50 passed (25 per backend).
- `uv run ruff check && uv run ruff format --check && uv run ty check -- nextcloud_mcp_server` — clean.

Tracked on Astrolabe Cloud POC board, card #99.

---

_This PR was generated with the help of AI, and reviewed by a Human_

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-16 18:06:42 +02:00

98 lines
3.3 KiB
Python

"""Shared pytest fixture for parametrizing storage tests over backends.
Tests for ``RefreshTokenStorage`` are exercised against every backend that is
available in the current environment:
- ``sqlite`` — always available; uses a per-test tempfile.
- ``postgres`` — opt-in. Bring up the test instance with::
docker compose --profile postgres up -d postgres-test
and export the URL so the fixture picks it up::
export TEST_DATABASE_URL=postgresql+asyncpg://mcp:mcp@localhost:5433/mcp
When ``TEST_DATABASE_URL`` is unset (or the host is unreachable), the
Postgres parametrization is skipped automatically so the suite still runs
cleanly without Docker.
Each Postgres test runs against an isolated schema that is dropped and
recreated between tests, mirroring the per-tempfile isolation that the
SQLite path gets for free.
"""
from __future__ import annotations
import os
from typing import Any
import pytest
def _postgres_url() -> str | None:
"""Resolve the Postgres URL for tests, or ``None`` when opted out."""
return os.environ.get("TEST_DATABASE_URL") or None
def _postgres_reachable(url: str) -> bool:
"""Return ``True`` if the configured Postgres accepts TCP connections.
A lightweight socket probe is used rather than a full DB handshake so
we don't have to ship a sync Postgres driver (psycopg2) just for the
test gate — asyncpg only works inside an event loop.
"""
import socket
from urllib.parse import urlparse
try:
parsed = urlparse(url)
host = parsed.hostname or "localhost"
port = parsed.port or 5432
with socket.create_connection((host, port), timeout=1.0):
return True
except OSError:
return False
def _backend_params() -> list[Any]:
"""Build the pytest parametrize list, gating Postgres on availability."""
params: list[Any] = [pytest.param("sqlite", id="sqlite")]
url = _postgres_url()
if url and _postgres_reachable(url):
params.append(pytest.param(url, id="postgres"))
return params
@pytest.fixture(params=_backend_params())
def storage_backend(request):
"""Yield ``{"kind": ..., "url": ..., "reset": <async>}`` per backend.
For ``sqlite`` the test fixture builds its own tempfile path; only the
``kind`` discriminator is used. For ``postgres`` the URL is forwarded
and a ``reset()`` coroutine is provided so test fixtures can wipe the
schema between parametrized runs.
"""
if request.param == "sqlite":
yield {"kind": "sqlite"}
return
url = request.param
async def reset() -> None:
# Use a fresh async engine so we don't fight an async connection
# the test might still be holding open at teardown time. asyncpg
# is the only driver we ship for Postgres, so the reset path stays
# event-loop-only (no psycopg2 dependency required).
from sqlalchemy import text
from sqlalchemy.ext.asyncio import create_async_engine
engine = create_async_engine(url, future=True)
try:
async with engine.begin() as conn:
await conn.execute(text("DROP SCHEMA public CASCADE"))
await conn.execute(text("CREATE SCHEMA public"))
finally:
await engine.dispose()
yield {"kind": "postgres", "url": url, "reset": reset}