diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1a8c9e8..a782592 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,6 +39,20 @@ jobs: - name: Test run: make test + images: + name: build + verify role images + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v5 + + # Reading a Dockerfile cannot tell you an image builds, that its entrypoint + # serves, or that sqlalchemy is genuinely absent from the engine rather than + # merely unmentioned. tests/test_deploy_invariants.py does the cheap textual + # half in the unit suite; this job does the half that needs a Docker daemon, + # so the ADR 0002 boundary is re-proved against the artefact on every PR (#81). + - name: Build both images and assert their deployment invariants + run: make images-verify + secret-scan: name: secret scan (gitleaks) runs-on: ubuntu-latest diff --git a/Makefile b/Makefile index d1c806c..bbb5a04 100644 --- a/Makefile +++ b/Makefile @@ -2,7 +2,8 @@ COMPOSE ?= docker compose -f deploy/compose/docker-compose.yml --env-file .env # Engine replica count for `make scale`. N ?= 2 -.PHONY: help sync hooks lint format type test check up down destroy migrate seed logs ps scale init-env secrets +.PHONY: help sync hooks lint format type test check images images-verify up down destroy \ + migrate seed logs ps scale init-env secrets help: ## List the available targets @grep -hE '^[a-zA-Z_-]+:.*?## ' $(MAKEFILE_LIST) \ @@ -37,6 +38,18 @@ test: ## pytest across all workspace members check: lint type test ## Everything CI runs +# ─── Images ─────────────────────────────────────────────────────────────────── + +images: ## Build both role images + docker build -f deploy/docker/api.Dockerfile -t icebergsst/api:verify . + docker build -f deploy/docker/engine.Dockerfile -t icebergsst/engine:verify . + +# Properties of the built artefact rather than of the Dockerfile text: both +# entrypoints serve, both run non-root, and no database package is importable in +# the engine image (ADR 0002). CI runs this on every PR (#81). +images-verify: ## Build both images and assert their deployment invariants + ./deploy/docker/verify-images.sh + # ─── Local stack ────────────────────────────────────────────────────────────── # `up` waits for every healthcheck before migrating, so what it hands back is a # stack that is ready rather than one that is merely started. @@ -52,7 +65,7 @@ destroy: ## Stop the stack and delete its data volume $(COMPOSE) down --volumes migrate: ## Apply migrations (api role only — it owns the schema) - $(COMPOSE) run --rm api alembic -c apps/api/alembic.ini upgrade head + $(COMPOSE) run --rm api python -m iceberg_api migrate seed: ## Load development fixtures (refuses to run in prod) $(COMPOSE) run --rm api python -m iceberg_api.seed diff --git a/README.md b/README.md index 86a5993..9145fae 100644 --- a/README.md +++ b/README.md @@ -118,7 +118,9 @@ The console is then at and the OpenAPI docs at `/docs`. OIDC configured in `.env`; the first person to sign in lands as a viewer unless `ICEBERG_BOOTSTRAP_ADMIN_SUBJECT` names them. -`make check` runs what CI runs: `ruff`, `mypy`, and `pytest`. `make help` lists every target. +`make check` runs what CI runs: `ruff`, `mypy`, and `pytest`. `make images-verify` builds both role +images and asserts the properties they must have — both entrypoints serve, both run non-root, and +no database package is importable in the engine image (ADR 0002). `make help` lists every target. ## Repository layout diff --git a/apps/api/pyproject.toml b/apps/api/pyproject.toml index c307dcd..dae2f3f 100644 --- a/apps/api/pyproject.toml +++ b/apps/api/pyproject.toml @@ -8,8 +8,15 @@ dependencies = [ "alembic>=1.18.5", # Cron expression parsing for the schedule model and scheduler tick. "croniter>=6.0.0", + # iceberg_api.dispatch enqueues task ids on the broker (ADR 0009). Declared + # here because the api image installs this package alone: leaning on the + # workspace having iceberg-engine installed builds an image that imports + # dramatiq and does not have it. + "dramatiq[redis]>=2.2.0", "fastapi>=0.139.2", - "iceberg-core", + # [db] pulls in the ORM: the api role is the only one that owns the schema, + # so it is the only one that declares a dependency on it (ADR 0002). + "iceberg-core[db]", # The M3 web UI's template engine. Imported directly by iceberg_api.web — # declared rather than leaned on transitively through fastapi's extras. "jinja2>=3.1.6", diff --git a/apps/api/alembic.ini b/apps/api/src/iceberg_api/alembic.ini similarity index 52% rename from apps/api/alembic.ini rename to apps/api/src/iceberg_api/alembic.ini index be5b361..0a4934d 100644 --- a/apps/api/alembic.ini +++ b/apps/api/src/iceberg_api/alembic.ini @@ -1,15 +1,28 @@ # Alembic configuration for the api role — the only role that owns the schema -# (docs/deployment.md § Migrations). Run from the repo root: +# (docs/deployment.md § Migrations). Normally reached through the operator CLI, +# which finds this file inside the installed package: # -# uv run alembic -c apps/api/alembic.ini upgrade head +# uv run python -m iceberg_api migrate # dev +# python -m iceberg_api migrate # in the api container +# +# It lives *inside* the package rather than beside pyproject.toml so that it +# travels in the wheel with the migrations it points at. The runtime image +# installs the package and copies no source tree (deploy/docker/api.Dockerfile), +# so a config file outside src/ would simply not be there. %(here)s resolves +# against this file, which makes the path correct in a source checkout and in +# site-packages alike. +# +# Authoring a revision still goes through alembic directly, since that reads the +# source tree by nature: +# +# uv run alembic -c apps/api/src/iceberg_api/alembic.ini revision --autogenerate -m "add x" # # The database URL is not set here: env.py takes it from ICEBERG_DATABASE_URL via # ApiSettings, so migrations and the running API can never disagree about which # database they mean (and a URL with a password stays out of version control). [alembic] -script_location = %(here)s/src/iceberg_api/migrations -prepend_sys_path = . +script_location = %(here)s/migrations path_separator = os timezone = UTC diff --git a/apps/api/src/iceberg_api/cli.py b/apps/api/src/iceberg_api/cli.py index 0068dbd..4495e87 100644 --- a/apps/api/src/iceberg_api/cli.py +++ b/apps/api/src/iceberg_api/cli.py @@ -16,8 +16,11 @@ import uuid from collections.abc import Sequence from datetime import UTC, datetime +from pathlib import Path import structlog +from alembic import command +from alembic.config import Config from iceberg_core.config import get_api_settings from iceberg_core.db import session_scope from iceberg_core.logging import configure_logging @@ -55,9 +58,37 @@ def _build_parser() -> argparse.ArgumentParser: commands.add_parser("reclaim", help="return expired-lease tasks to the queue") commands.add_parser("scheduler-tick", help="run one scheduler round now") + + migrate_parser = commands.add_parser("migrate", help="apply migrations up to a revision") + migrate_parser.add_argument( + "--revision", + default="head", + help="target revision (default: head; accepts e.g. -1 to step back)", + ) return parser +#: The packaged alembic config, resolved against this module rather than the +#: working directory. The api image installs the package and copies no source +#: tree, so a repo-relative path would be correct in a checkout and wrong in +#: every container. +ALEMBIC_INI = Path(__file__).resolve().parent / "alembic.ini" + + +def alembic_config() -> Config: + """The alembic config as every role should reach it. + + One loader for the compose `make migrate`, the Helm pre-upgrade Job, and the + migration tests, so none of them can drift onto a different script location. + """ + return Config(str(ALEMBIC_INI)) + + +def migrate(revision: str = "head") -> None: + """Apply migrations. The api role owns the schema; nothing else runs this.""" + command.upgrade(alembic_config(), revision) + + def mint_engine_token(name: str, version: str | None) -> tuple[uuid.UUID, str]: """Register or rotate ``name``; return its id and new token. @@ -127,6 +158,9 @@ def main(argv: Sequence[str] | None = None) -> int: f"skipped={len(result.skipped)}", file=sys.stderr, ) + case "migrate": + migrate(args.revision) + print(f"migrated to {args.revision}", file=sys.stderr) return 0 diff --git a/apps/api/tests/test_migrations.py b/apps/api/tests/test_migrations.py index 0b85eb1..b7061e8 100644 --- a/apps/api/tests/test_migrations.py +++ b/apps/api/tests/test_migrations.py @@ -12,20 +12,22 @@ import pytest from alembic import command from alembic.config import Config +from iceberg_api.cli import alembic_config from iceberg_core.enums import ScanStatus, ScanTrigger, SourceType from iceberg_core.models import Scan, Source, metadata from sqlalchemy import Engine, create_engine, inspect from sqlalchemy.exc import IntegrityError from sqlmodel import Session, select -ALEMBIC_INI = Path(__file__).resolve().parents[1] / "alembic.ini" - @pytest.fixture(name="migrated_engine") def migrated_engine_fixture(tmp_path: Path) -> Iterator[tuple[Engine, Config]]: """A file-backed SQLite database with migrations applied to head.""" url = f"sqlite:///{tmp_path / 'migrations.db'}" - config = Config(str(ALEMBIC_INI)) + # Deliberately the same loader the operator CLI and the Helm Job use: these + # tests are only evidence about production migrations if they run the same + # config, from the same place. + config = alembic_config() config.attributes["sqlalchemy.url"] = url # The suite configures its own logging; let Alembic keep its hands off it. config.attributes["skip_logging_config"] = True diff --git a/deploy/docker/api.Dockerfile b/deploy/docker/api.Dockerfile index 8e52288..3c3dadf 100644 --- a/deploy/docker/api.Dockerfile +++ b/deploy/docker/api.Dockerfile @@ -16,6 +16,10 @@ ENV UV_COMPILE_BYTECODE=1 \ UV_LINK_MODE=copy \ UV_PYTHON_DOWNLOADS=never +# The builder works at the same path the runtime stage will use. A venv is not +# relocatable: uv writes `#!/app/.venv/bin/python` into every console script, so +# building at /src and copying to /app leaves `uvicorn` with a shebang pointing at +# a directory that does not exist — "exec: no such file or directory" at start. WORKDIR /app # Dependencies first, from the lock file and manifests alone: editing application @@ -27,11 +31,15 @@ COPY apps/engine/pyproject.toml apps/engine/ COPY packages/core/pyproject.toml packages/core/ COPY packages/detect/pyproject.toml packages/detect/ COPY packages/connectors/pyproject.toml packages/connectors/ -RUN uv sync --locked --no-dev --no-install-workspace --package iceberg-api +RUN uv sync --locked --no-dev --no-editable --no-install-workspace --package iceberg-api COPY apps/ apps/ COPY packages/ packages/ -RUN uv sync --locked --no-dev --package iceberg-api +# --no-editable installs the workspace packages into the venv proper rather than +# linking back at /src, so the venv is self-contained and the runtime stage can +# take it alone. Templates, static assets, migrations and alembic.ini all live +# under src/iceberg_api/ and travel in the wheel with it. +RUN uv sync --locked --no-dev --no-editable --package iceberg-api FROM python:3.14-slim AS runtime @@ -44,7 +52,9 @@ ENV PYTHONUNBUFFERED=1 \ RUN useradd --create-home --uid 10001 iceberg WORKDIR /app -COPY --from=builder --chown=iceberg:iceberg /app /app +# The venv and nothing else: no source tree, no uv, no build dependencies. What +# is not in the image cannot be imported by accident or read by an attacker. +COPY --from=builder --chown=iceberg:iceberg /app/.venv /app/.venv USER iceberg EXPOSE 8000 diff --git a/deploy/docker/engine.Dockerfile b/deploy/docker/engine.Dockerfile index c892817..f9bd9e8 100644 --- a/deploy/docker/engine.Dockerfile +++ b/deploy/docker/engine.Dockerfile @@ -17,21 +17,24 @@ ENV UV_COMPILE_BYTECODE=1 \ UV_LINK_MODE=copy \ UV_PYTHON_DOWNLOADS=never +# Same path as the runtime stage — a venv is not relocatable. See api.Dockerfile. WORKDIR /app # Dependency layer first — see api.Dockerfile for why. --package iceberg-engine -# means the api's dependencies (and its Postgres driver) are not installed here. +# means the api's dependencies are not installed here: no Postgres driver, no +# alembic, and no ORM, because iceberg-core carries sqlmodel in its `db` extra +# and only apps/api asks for it. COPY pyproject.toml uv.lock ./ COPY apps/api/pyproject.toml apps/api/ COPY apps/engine/pyproject.toml apps/engine/ COPY packages/core/pyproject.toml packages/core/ COPY packages/detect/pyproject.toml packages/detect/ COPY packages/connectors/pyproject.toml packages/connectors/ -RUN uv sync --locked --no-dev --no-install-workspace --package iceberg-engine +RUN uv sync --locked --no-dev --no-editable --no-install-workspace --package iceberg-engine COPY apps/ apps/ COPY packages/ packages/ -RUN uv sync --locked --no-dev --package iceberg-engine +RUN uv sync --locked --no-dev --no-editable --package iceberg-engine FROM python:3.14-slim AS runtime @@ -44,7 +47,10 @@ ENV PYTHONUNBUFFERED=1 \ RUN useradd --create-home --uid 10001 iceberg WORKDIR /app -COPY --from=builder --chown=iceberg:iceberg /app /app +# The venv alone — see api.Dockerfile. Here it also carries the ADR 0002 +# boundary: deploy/docker/verify-images.sh proves no database package is +# importable in this image, which is only true because nothing copies one in. +COPY --from=builder --chown=iceberg:iceberg /app/.venv /app/.venv USER iceberg EXPOSE 9191 diff --git a/deploy/docker/verify-images.sh b/deploy/docker/verify-images.sh new file mode 100755 index 0000000..b0b37c6 --- /dev/null +++ b/deploy/docker/verify-images.sh @@ -0,0 +1,153 @@ +#!/usr/bin/env bash +# +# Build both role images and assert the properties they are supposed to have. +# +# make images-verify # build, then check +# ./deploy/docker/verify-images.sh --no-build # check images already built +# +# This exists because the alternative is reading the Dockerfiles and believing +# them. tests/test_deploy_invariants.py does read them — it catches a DATABASE_URL +# added to the engine service — but it cannot tell you that the image builds, that +# the entrypoint serves, or that sqlalchemy is genuinely absent rather than merely +# unmentioned. Those are properties of the built artefact, so they are checked +# against the built artefact (#81). +# +# The engine's database isolation (ADR 0002) is the check that matters most: an +# engine that can import an ORM is one refactor away from holding a connection, +# and the invariant is easier to keep when breaking it fails the build. + +set -euo pipefail + +API_IMAGE="${API_IMAGE:-icebergsst/api:verify}" +ENGINE_IMAGE="${ENGINE_IMAGE:-icebergsst/engine:verify}" +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" + +build=1 +[[ "${1:-}" == "--no-build" ]] && build=0 + +failures=0 +pass() { printf ' \033[32mok\033[0m %s\n' "$1"; } +fail() { printf ' \033[31mFAIL\033[0m %s\n' "$1"; failures=$((failures + 1)); } +group() { printf '\n\033[1m%s\033[0m\n' "$1"; } + +# check +check() { + if [[ "$2" == "$3" ]]; then pass "$1"; else fail "$1 (expected '$2', got '$3')"; fi +} + +if (( build )); then + group "Building images" + docker build -q -f "$REPO_ROOT/deploy/docker/api.Dockerfile" -t "$API_IMAGE" "$REPO_ROOT" >/dev/null + pass "api image builds" + docker build -q -f "$REPO_ROOT/deploy/docker/engine.Dockerfile" -t "$ENGINE_IMAGE" "$REPO_ROOT" >/dev/null + pass "engine image builds" +fi + +# ── Both roles run unprivileged ─────────────────────────────────────────────── +# A container that runs as root turns any RCE in a connector or a template into +# root in the container, and root in the container is the first half of most +# escapes. +group "Non-root runtime" +check "api runs as uid 10001" "10001" "$(docker run --rm "$API_IMAGE" id -u)" +check "engine runs as uid 10001" "10001" "$(docker run --rm "$ENGINE_IMAGE" id -u)" + +# ── ADR 0002: the engine cannot reach the database ──────────────────────────── +group "Engine database isolation (ADR 0002)" +# Deliberately an import, not importlib.util.find_spec. iceberg_core ships db.py +# and models/ to both roles — they are the same package — so find_spec locates +# those files in the engine image and reports them present. What actually matters +# is whether they can be *used*, and they cannot: their ORM lives in the +# iceberg-core[db] extra that only apps/api depends on, so importing them raises +# ModuleNotFoundError. Importing is the check that distinguishes the two. +db_probe=' +import importlib +usable = [] +for name in ("sqlalchemy", "sqlmodel", "psycopg", "asyncpg", "alembic", + "iceberg_core.db", "iceberg_core.models"): + try: + importlib.import_module(name) + except ImportError: + continue + usable.append(name) +print(",".join(usable)) +' +usable="$(docker run --rm "$ENGINE_IMAGE" python -c "$db_probe")" +check "no database package is importable in the engine image" "" "$usable" + +# The other half of the same boundary: no configuration that would let it try. +engine_env="$(docker inspect --format '{{range .Config.Env}}{{println .}}{{end}}' "$ENGINE_IMAGE" \ + | grep -Ei 'DATABASE|POSTGRES|MASTER_KEY' || true)" +check "no database or master-key variable is baked into the engine image" "" "$engine_env" + +# ── Entrypoints actually serve ──────────────────────────────────────────────── +# The image's HEALTHCHECK is the contract compose and Kubernetes rely on, so the +# check here is the same request they make. +group "Entrypoints serve" + +serves() { + local name="$1" image="$2" port="$3" path="$4" description="$5" + shift 5 + docker rm -f "$name" >/dev/null 2>&1 || true + docker run -d --name "$name" "$@" "$image" >/dev/null + local ok=0 + for _ in $(seq 1 40); do + if docker exec "$name" python -c " +import sys, urllib.request +try: + urllib.request.urlopen('http://127.0.0.1:$port$path', timeout=2).read() +except Exception: + sys.exit(1) +" >/dev/null 2>&1; then + ok=1 + break + fi + sleep 1 + done + if (( ok )); then + pass "$description" + else + fail "$description" + docker logs "$name" 2>&1 | tail -20 | sed 's/^/ /' + fi + docker rm -f "$name" >/dev/null 2>&1 || true +} + +# The api needs enough configuration to construct its settings; it does not need +# a reachable database to answer /healthz, which is the point of the endpoint. +serves iceberg-verify-api "$API_IMAGE" 8000 /healthz "api serves /healthz" \ + -e ICEBERG_DATABASE_URL="postgresql+psycopg://iceberg:unused@127.0.0.1:5432/iceberg" \ + -e ICEBERG_REDIS_URL="redis://127.0.0.1:6379/0" \ + -e ICEBERG_MASTER_KEY="$(docker run --rm "$API_IMAGE" python -m iceberg_core.secrets generate-master-key)" \ + -e ICEBERG_SESSION_SECRET="verification-only-session-secret-not-a-real-one" \ + -e ICEBERG_FINGERPRINT_PEPPER_REF="env:ICEBERG_VERIFY_PEPPER" + +serves iceberg-verify-engine "$ENGINE_IMAGE" 9191 /metrics "engine serves /metrics" \ + -e ICEBERG_REDIS_URL="redis://127.0.0.1:6379/0" \ + -e ICEBERG_API_BASE_URL="http://127.0.0.1:8000" + +# ── The api can find its migrations ─────────────────────────────────────────── +# The runtime stage copies only the venv, so alembic.ini and the revision scripts +# have to be inside the installed package. `heads` reads the script directory +# without touching a database, which is exactly the part that could be missing. +group "Migrations are packaged" +if docker run --rm \ + -e ICEBERG_DATABASE_URL="postgresql+psycopg://iceberg:unused@127.0.0.1:5432/iceberg" \ + -e ICEBERG_REDIS_URL="redis://127.0.0.1:6379/0" \ + -e ICEBERG_MASTER_KEY="unused" \ + -e ICEBERG_SESSION_SECRET="unused" \ + "$API_IMAGE" python -c " +from alembic import command +from iceberg_api.cli import alembic_config +command.heads(alembic_config()) +" >/dev/null 2>&1; then + pass "api image resolves its alembic config and revision scripts" +else + fail "api image resolves its alembic config and revision scripts" +fi + +group "Result" +if (( failures )); then + printf '\033[31m%d check(s) failed\033[0m\n' "$failures" + exit 1 +fi +printf '\033[32mall checks passed\033[0m\n' diff --git a/docs/data-model.md b/docs/data-model.md index 17970d5..f28597a 100644 --- a/docs/data-model.md +++ b/docs/data-model.md @@ -129,15 +129,25 @@ Registered worker. ## Migrations -Alembic lives with the api role (`apps/api/alembic.ini`, migrations under -`apps/api/src/iceberg_api/migrations/`), because only the api role owns the schema. Autogenerate -targets the shared SQLModel metadata, and the database URL comes from `ICEBERG_DATABASE_URL` via -`ApiSettings` rather than the ini, so migrations cannot point somewhere the API does not. +Alembic lives with the api role, *inside* the package: `apps/api/src/iceberg_api/alembic.ini` +alongside the migrations under `apps/api/src/iceberg_api/migrations/`. Only the api role owns the +schema. Keeping the config in the package means it travels in the wheel, so the api image — which +installs the package and copies no source tree — can find it; `%(here)s` resolves correctly in a +checkout and in site-packages alike. Autogenerate targets the shared SQLModel metadata, and the +database URL comes from `ICEBERG_DATABASE_URL` via `ApiSettings` rather than the ini, so migrations +cannot point somewhere the API does not. + +Applying migrations goes through the operator CLI, which loads the packaged config. One entry +point for the compose stack, the Helm pre-upgrade Job and the tests means none of them can drift +onto a different script location: ``` -make migrate # in the compose stack -uv run alembic -c apps/api/alembic.ini upgrade head # against ICEBERG_DATABASE_URL -uv run alembic -c apps/api/alembic.ini revision --autogenerate -m "add x" +make migrate # in the compose stack +uv run python -m iceberg_api migrate # against ICEBERG_DATABASE_URL +uv run python -m iceberg_api migrate --revision -1 # step back one + +# Authoring a revision reads the source tree by nature, so it calls alembic directly: +uv run alembic -c apps/api/src/iceberg_api/alembic.ini revision --autogenerate -m "add x" ``` A new entity must be re-exported from `iceberg_core.models` or autogenerate will not see it. diff --git a/docs/deployment.md b/docs/deployment.md index 24e832c..dfddbe1 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -48,9 +48,13 @@ is a shared trust surface (`docs/security.md`). secret-store backend selection (env-key vs Vault). ## Migrations -Alembic, configured at `apps/api/alembic.ini`. Only the **api** role runs migrations (it owns the -schema); engines never touch the DB. In the compose stack: `make migrate`. Directly: -`uv run alembic -c apps/api/alembic.ini upgrade head`, which reads `ICEBERG_DATABASE_URL`. +Alembic, configured at `apps/api/src/iceberg_api/alembic.ini` — inside the package, so it ships in +the wheel with the revisions it points at and the api image can find it without a source tree. +Only the **api** role runs migrations (it owns the schema); engines never touch the DB. + +`python -m iceberg_api migrate` is the entry point everywhere: `make migrate` in the compose +stack, the pre-upgrade `Job` in Helm, and `uv run python -m iceberg_api migrate` locally. All of +them read `ICEBERG_DATABASE_URL`. ## Scaling model - Throughput scales by adding **engine** replicas — more Dramatiq consumers pulling scan tasks diff --git a/packages/core/pyproject.toml b/packages/core/pyproject.toml index 27cc3a0..bf9bbc9 100644 --- a/packages/core/pyproject.toml +++ b/packages/core/pyproject.toml @@ -3,17 +3,26 @@ name = "iceberg-core" version = "0.1.0" description = "Shared models, config, secret-store, fingerprinting, and redaction utilities." requires-python = ">=3.14" +# Deliberately no ORM here. Everything in this list is needed by both roles — +# config, logging, secret store, redaction, fingerprinting. `iceberg_core.db` and +# `iceberg_core.models` are api-role modules and their dependency lives in the +# `db` extra below, so an engine image cannot even import an ORM (ADR 0002). dependencies = [ "cryptography>=49.0.0", "prometheus-client>=0.25.0", "pydantic-settings>=2.13.0", - "sqlmodel>=0.0.39", "structlog>=26.1.0", ] [project.optional-dependencies] -# Only needed to load the pytest plugin below; production images skip this. -testing = ["pytest>=9.1.1"] +# The api role's half of this package: iceberg_core.db and iceberg_core.models. +# apps/api depends on iceberg-core[db]; apps/engine depends on plain iceberg-core, +# which is what keeps sqlalchemy out of the engine image entirely rather than +# merely unused — deploy/docker/verify-images.sh probes for it. +db = ["sqlmodel>=0.0.39"] +# Only needed to load the pytest plugin below; production images skip this. The +# shared fixtures are database fixtures, so this implies the db extra. +testing = ["iceberg-core[db]", "pytest>=9.1.1"] [project.entry-points.pytest11] # Publishes iceberg_core.testing so every workspace member gets the shared diff --git a/packages/detect/pyproject.toml b/packages/detect/pyproject.toml index 8068f74..c3c059d 100644 --- a/packages/detect/pyproject.toml +++ b/packages/detect/pyproject.toml @@ -7,6 +7,9 @@ dependencies = [ "iceberg-core", # Rule packs are YAML shipped inside the engine image (ADR 0008). "pyyaml>=6.0.3", + # Imported directly by the engine module — declared rather than leaned on + # transitively through iceberg-core, same as everywhere else here. + "structlog>=26.1.0", ] [build-system] diff --git a/tests/test_deploy_invariants.py b/tests/test_deploy_invariants.py index 5870bd9..0f7751d 100644 --- a/tests/test_deploy_invariants.py +++ b/tests/test_deploy_invariants.py @@ -1,9 +1,16 @@ """Deployment-level invariants (ADR 0002). -`make up` and `docker build` are not runnable in CI, so the properties the -containers must have are asserted against the files that define them. These are -cheap checks for expensive mistakes: an engine handed a database URL is a -credential-isolation failure that no unit test would notice. +These read the files that define the containers rather than the containers: cheap +checks for expensive mistakes, running in the ordinary unit-test suite where an +engine handed a database URL fails in seconds. + +They are the fast half of the story. The slow half is +``deploy/docker/verify-images.sh`` (``make images-verify``), which builds both +images and probes the artefacts — no database package importable in the engine, +both entrypoints serving, both running non-root. Reading a Dockerfile cannot tell +you an image builds, and a text check cannot tell you sqlalchemy is genuinely +absent rather than merely unmentioned (#81). Keep both: this file catches the +mistake in the edit, that script catches it in the result. """ import importlib @@ -38,6 +45,20 @@ def _configured_variables(dockerfile: Path) -> list[str]: return [match.group(1) for match in CONFIG_DIRECTIVE_RE.finditer(text)] +def _instructions(dockerfile: Path) -> str: + """The Dockerfile with comments stripped. + + These files explain themselves at length, and several comments name the very + packages the checks below forbid — "no Postgres driver, no alembic" is the + engine Dockerfile stating the invariant, not breaking it. Substring checks + over the raw text would read prose as instruction and fail on a comment that + got the rule right. + """ + return "\n".join( + line for line in dockerfile.read_text().splitlines() if not line.lstrip().startswith("#") + ) + + def test_both_role_images_exist() -> None: assert API_DOCKERFILE.is_file() assert ENGINE_DOCKERFILE.is_file() @@ -53,14 +74,54 @@ def test_the_engine_image_carries_no_database_configuration() -> None: def test_the_engine_image_installs_no_database_driver() -> None: - """``uv sync --package iceberg-engine`` is what keeps psycopg out of the image.""" - contents = ENGINE_DOCKERFILE.read_text() + """``uv sync --package iceberg-engine`` is what keeps psycopg out of the image. + + The stronger form of this — proving nothing database-shaped is importable in + the built image — is in ``deploy/docker/verify-images.sh``. + """ + contents = _instructions(ENGINE_DOCKERFILE) assert "--package iceberg-engine" in contents assert "psycopg" not in contents assert "alembic" not in contents +def test_the_engine_never_asks_for_the_orm_extra() -> None: + """``iceberg-core[db]`` carries sqlmodel; only the api role may depend on it. + + This is what makes the image probe pass: the engine's dependency closure has + no ORM in it, so none is installed, so none can be imported (ADR 0002). + """ + engine_manifest = (REPO_ROOT / "apps" / "engine" / "pyproject.toml").read_text() + api_manifest = (REPO_ROOT / "apps" / "api" / "pyproject.toml").read_text() + + assert "iceberg-core[db]" not in engine_manifest + assert "iceberg-core[db]" in api_manifest + + core_manifest = (REPO_ROOT / "packages" / "core" / "pyproject.toml").read_text() + base, _, extras = core_manifest.partition("[project.optional-dependencies]") + assert "sqlmodel" not in base, "sqlmodel must stay in the db extra, not core's base deps" + assert "sqlmodel" in extras + + +def test_the_image_verification_runs_in_ci() -> None: + """The artefact-level checks are only worth having if they actually run. + + ``verify-images.sh`` is the thing that proves the images build and that the + engine cannot import an ORM. A green PR should mean both were re-checked, so + the CI job that does it is asserted here rather than trusted (#81). + """ + script = DOCKER_DIR / "verify-images.sh" + assert script.is_file() + assert script.stat().st_mode & 0o111, "verify-images.sh must be executable" + + workflow = (REPO_ROOT / ".github" / "workflows" / "ci.yml").read_text() + assert "make images-verify" in workflow + + makefile = (REPO_ROOT / "Makefile").read_text() + assert "images-verify:" in makefile + + def test_neither_image_runs_as_root() -> None: for dockerfile in (API_DOCKERFILE, ENGINE_DOCKERFILE): assert "USER iceberg" in dockerfile.read_text(), dockerfile.name @@ -90,8 +151,8 @@ def test_the_build_context_excludes_secrets_and_history() -> None: def test_only_the_api_image_owns_migrations() -> None: """Schema ownership is the api role's alone (docs/deployment.md § Migrations).""" - assert "alembic" not in ENGINE_DOCKERFILE.read_text() - assert "--package iceberg-api" in API_DOCKERFILE.read_text() + assert "alembic" not in _instructions(ENGINE_DOCKERFILE) + assert "--package iceberg-api" in _instructions(API_DOCKERFILE) @pytest.fixture(name="compose", scope="module") diff --git a/uv.lock b/uv.lock index 45b9b60..7649a2c 100644 --- a/uv.lock +++ b/uv.lock @@ -405,9 +405,10 @@ source = { editable = "apps/api" } dependencies = [ { name = "alembic" }, { name = "croniter" }, + { name = "dramatiq", extra = ["redis"] }, { name = "fastapi" }, { name = "httpx2" }, - { name = "iceberg-core" }, + { name = "iceberg-core", extra = ["db"] }, { name = "jinja2" }, { name = "prometheus-client" }, { name = "psycopg", extra = ["binary"] }, @@ -421,9 +422,10 @@ dependencies = [ requires-dist = [ { name = "alembic", specifier = ">=1.18.5" }, { name = "croniter", specifier = ">=6.0.0" }, + { name = "dramatiq", extras = ["redis"], specifier = ">=2.2.0" }, { name = "fastapi", specifier = ">=0.139.2" }, { name = "httpx2", specifier = ">=2.7.0" }, - { name = "iceberg-core", editable = "packages/core" }, + { name = "iceberg-core", extras = ["db"], editable = "packages/core" }, { name = "jinja2", specifier = ">=3.1.6" }, { name = "prometheus-client", specifier = ">=0.25.0" }, { name = "psycopg", extras = ["binary"], specifier = ">=3.2.12" }, @@ -460,25 +462,29 @@ dependencies = [ { name = "cryptography" }, { name = "prometheus-client" }, { name = "pydantic-settings" }, - { name = "sqlmodel" }, { name = "structlog" }, ] [package.optional-dependencies] +db = [ + { name = "sqlmodel" }, +] testing = [ { name = "pytest" }, + { name = "sqlmodel" }, ] [package.metadata] requires-dist = [ { name = "cryptography", specifier = ">=49.0.0" }, + { name = "iceberg-core", extras = ["db"], marker = "extra == 'testing'", editable = "packages/core" }, { name = "prometheus-client", specifier = ">=0.25.0" }, { name = "pydantic-settings", specifier = ">=2.13.0" }, { name = "pytest", marker = "extra == 'testing'", specifier = ">=9.1.1" }, - { name = "sqlmodel", specifier = ">=0.0.39" }, + { name = "sqlmodel", marker = "extra == 'db'", specifier = ">=0.0.39" }, { name = "structlog", specifier = ">=26.1.0" }, ] -provides-extras = ["testing"] +provides-extras = ["db", "testing"] [[package]] name = "iceberg-detect" @@ -487,12 +493,14 @@ source = { editable = "packages/detect" } dependencies = [ { name = "iceberg-core" }, { name = "pyyaml" }, + { name = "structlog" }, ] [package.metadata] requires-dist = [ { name = "iceberg-core", editable = "packages/core" }, { name = "pyyaml", specifier = ">=6.0.3" }, + { name = "structlog", specifier = ">=26.1.0" }, ] [[package]]