From 913c9b5d67497d1854022b581bad28e3d5504a89 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 3 Nov 2026 23:56:13 +0400 Subject: [PATCH 01/25] Add the simulation persistence library with its pooled engine Add libs/policyengine-simulation-persistence, the package through which the simulation service (the Simulation Entrypoint) connects to the Public API's Postgres database with its own role. This commit holds only the connection: the runtime engine, built from the canonical postgresql:// URL the deployment stores, with the psycopg 3 driver, a small pool, pre-ping, a connect timeout and no server-side prepared statements, so the same URL works through Supabase's poolers. A malformed URL raises ValueError without repeating the URL or the parser's message, since the URL holds a password. The package is unit-tested in the PR test matrix and by make test, type-checked by the Pyright job under sim.quality.pyright-libraries-and-entry, and scanned by the database access guard. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/pr.yml | 2 + Makefile | 1 + docs/engineering/skills/type-checking.md | 6 +- docs/generated/rules.md | 8 +- .../README.md | 25 ++ .../pyproject.toml | 39 +++ .../__init__.py | 11 + .../engine.py | 60 +++++ .../tests/test_engine.py | 63 +++++ .../uv.lock | 231 ++++++++++++++++++ .../tests/test_database_access_boundaries.py | 1 + rules/registry.toml | 4 +- 12 files changed, 443 insertions(+), 8 deletions(-) create mode 100644 libs/policyengine-simulation-persistence/README.md create mode 100644 libs/policyengine-simulation-persistence/pyproject.toml create mode 100644 libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/__init__.py create mode 100644 libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/engine.py create mode 100644 libs/policyengine-simulation-persistence/tests/test_engine.py create mode 100644 libs/policyengine-simulation-persistence/uv.lock diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 548fa8063..3a2fc61a7 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -51,6 +51,7 @@ jobs: - projects/policyengine-simulation-gateway - libs/policyengine-simulation-contract - libs/policyengine-simulation-observability + - libs/policyengine-simulation-persistence - libs/policyengine-stage12-persistence - libs/policyengine-fastapi @@ -191,6 +192,7 @@ jobs: package: - libs/policyengine-simulation-contract - libs/policyengine-simulation-observability + - libs/policyengine-simulation-persistence - libs/policyengine-stage12-persistence - libs/policyengine-fastapi - projects/policyengine-simulation-entry diff --git a/Makefile b/Makefile index 4c79846e1..48d8e083f 100644 --- a/Makefile +++ b/Makefile @@ -91,6 +91,7 @@ test: projects/policyengine-simulation-gateway \ libs/policyengine-simulation-contract \ libs/policyengine-simulation-observability \ + libs/policyengine-simulation-persistence \ libs/policyengine-stage12-persistence \ libs/policyengine-fastapi; do \ echo "Testing $$proj..."; \ diff --git a/docs/engineering/skills/type-checking.md b/docs/engineering/skills/type-checking.md index 56a444b58..b1b8abbd2 100644 --- a/docs/engineering/skills/type-checking.md +++ b/docs/engineering/skills/type-checking.md @@ -11,9 +11,9 @@ Every uv package is type-checked with Pyright on every pull request, by the - `sim.quality.pyright-libraries-and-entry`, `sim.quality.pyright-gateway`, `sim.quality.pyright-executor` and `sim.quality.pyright-apis-integ`: the - contract, observability, Stage 12 persistence and fastapi libraries, the - simulation entry, the gateway, the executor and the generated-client - integration tests have no Pyright errors. + contract, observability, simulation persistence, Stage 12 persistence and + fastapi libraries, the simulation entry, the gateway, the executor and the + generated-client integration tests have no Pyright errors. - `sim.quality.every-package-type-checked`: a Pyright rule lists every uv package under `libs/` and `projects/`. When you add a package, give it a `[tool.pyright] include`, add it to a Pyright rule's `enforcer.projects`, and diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 53cdb5b83..0810123d7 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -318,7 +318,7 @@ flowchart LR | `sim.quality.pyright-apis-integ` | The generated-client integration-test project, apis-integ, has no Pyright errors, checked with the pinned Pyright version in the project's own locked environment. | mechanical | enforced | yes | pyright in `projects/policyengine-apis-integ` | `pr.yml:pyright` | yes | | `sim.quality.pyright-executor` | The simulation executor has no Pyright errors, checked with the pinned Pyright version in the executor's own locked environment. | mechanical | enforced | yes | pyright in `projects/policyengine-simulation-executor` | `pr.yml:pyright` | yes | | `sim.quality.pyright-gateway` | The simulation gateway has no Pyright errors, checked with the pinned Pyright version in the gateway's own locked environment. | mechanical | enforced | yes | pyright in `projects/policyengine-simulation-gateway` | `pr.yml:pyright` | yes | -| `sim.quality.pyright-libraries-and-entry` | The contract, observability, Stage 12 persistence and fastapi libraries and the simulation entry have no Pyright errors, checked with the pinned Pyright version in each package's own locked environment. | mechanical | enforced | yes | pyright in `libs/policyengine-simulation-contract`, `libs/policyengine-simulation-observability`, `libs/policyengine-stage12-persistence`, `libs/policyengine-fastapi`, `projects/policyengine-simulation-entry` | `pr.yml:pyright` | yes | +| `sim.quality.pyright-libraries-and-entry` | The contract, observability, simulation persistence, Stage 12 persistence and fastapi libraries and the simulation entry have no Pyright errors, checked with the pinned Pyright version in each package's own locked environment. | mechanical | enforced | yes | pyright in `libs/policyengine-simulation-contract`, `libs/policyengine-simulation-observability`, `libs/policyengine-simulation-persistence`, `libs/policyengine-stage12-persistence`, `libs/policyengine-fastapi`, `projects/policyengine-simulation-entry` | `pr.yml:pyright` | yes | | `sim.quality.python-file-length` | No tracked Python file has more than 800 lines, or more than 1,500 lines if it is test code: a file under a tests or fixtures directory, or a conftest.py. | metric | enforced | yes | quality guard `file-length` | `pr.yml:quality-guards` | yes | | `sim.quality.ruff-format-src` | Source code under projects/*/src and libs/*/src is formatted with ruff format. | mechanical | enforced | yes | ruff format | `pr.yml:lint` | — | | `sim.quality.ruff-format-tests` | Test code is formatted with ruff format too; CI currently checks only src directories. | mechanical | doc-only | no | nothing | — | — | @@ -351,7 +351,7 @@ flowchart LR r_sim_quality_pyright_gateway("sim.quality.pyright-gateway (enforced)") e_pyright_in_projects_policyengine_simulation_gateway["pyright in projects/policyengine-simulation-gateway"] r_sim_quality_pyright_libraries_and_entry("sim.quality.pyright-libraries-and-entry (enforced)") - e_pyright_in_libs_policyengine_simulation_contract__libs_policyengine_simulation_observability__libs_policyengine_stage12_persistence__libs_policyengine_fastapi__projects_policyengine_simulation_entry["pyright in libs/policyengine-simulation-contract, libs/policyengine-simulation-observability, libs/policyengine-stage12-persistence, libs/policyengine-fastapi, projects/policyengine-simulation-entry"] + e_pyright_in_libs_policyengine_simulation_contract__libs_policyengine_simulation_observability__libs_policyengine_simulation_persistence__libs_policyengine_stage12_persistence__libs_policyengine_fastapi__projects_policyengine_simulation_entry["pyright in libs/policyengine-simulation-contract, libs/policyengine-simulation-observability, libs/policyengine-simulation-persistence, libs/policyengine-stage12-persistence, libs/policyengine-fastapi, projects/policyengine-simulation-entry"] r_sim_quality_python_file_length("sim.quality.python-file-length (enforced)") e_quality_guard_file_length["quality guard file-length"] r_sim_quality_ruff_format_src("sim.quality.ruff-format-src (enforced)") @@ -385,8 +385,8 @@ flowchart LR e_pyright_in_projects_policyengine_simulation_executor --> j_CI_pr_yml_pyright r_sim_quality_pyright_gateway --> e_pyright_in_projects_policyengine_simulation_gateway e_pyright_in_projects_policyengine_simulation_gateway --> j_CI_pr_yml_pyright - r_sim_quality_pyright_libraries_and_entry --> e_pyright_in_libs_policyengine_simulation_contract__libs_policyengine_simulation_observability__libs_policyengine_stage12_persistence__libs_policyengine_fastapi__projects_policyengine_simulation_entry - e_pyright_in_libs_policyengine_simulation_contract__libs_policyengine_simulation_observability__libs_policyengine_stage12_persistence__libs_policyengine_fastapi__projects_policyengine_simulation_entry --> j_CI_pr_yml_pyright + r_sim_quality_pyright_libraries_and_entry --> e_pyright_in_libs_policyengine_simulation_contract__libs_policyengine_simulation_observability__libs_policyengine_simulation_persistence__libs_policyengine_stage12_persistence__libs_policyengine_fastapi__projects_policyengine_simulation_entry + e_pyright_in_libs_policyengine_simulation_contract__libs_policyengine_simulation_observability__libs_policyengine_simulation_persistence__libs_policyengine_stage12_persistence__libs_policyengine_fastapi__projects_policyengine_simulation_entry --> j_CI_pr_yml_pyright r_sim_quality_python_file_length --> e_quality_guard_file_length e_quality_guard_file_length --> j_CI_pr_yml_quality_guards r_sim_quality_ruff_format_src --> e_ruff_format diff --git a/libs/policyengine-simulation-persistence/README.md b/libs/policyengine-simulation-persistence/README.md new file mode 100644 index 000000000..30ab8eed7 --- /dev/null +++ b/libs/policyengine-simulation-persistence/README.md @@ -0,0 +1,25 @@ +# PolicyEngine simulation persistence + +Database connections for the simulation service (the Simulation Entrypoint on +Cloud Run). The service connects to the Public API's Postgres database with +its own role, and writes rows in tables the Public API's migration chain +creates. This repository has no schema or migration chain of its own. + +The temporary Stage 12 comparison tables are not part of this library: +`libs/policyengine-stage12-persistence` writes them with its own connection. + +## Connections + +`create_simulation_engine` builds the runtime engine from the canonical +`postgresql://` URL the deployment stores, selecting the psycopg 3 driver. It +keeps a small pool, checks connections before use, and never prepares +server-side statements, so the same URL works through Supabase's session and +transaction poolers. A malformed URL raises `ValueError` without repeating the +URL, which holds a password. + +## Tests + +```bash +uv sync --extra test +uv run pytest tests/ +``` diff --git a/libs/policyengine-simulation-persistence/pyproject.toml b/libs/policyengine-simulation-persistence/pyproject.toml new file mode 100644 index 000000000..96ddf7231 --- /dev/null +++ b/libs/policyengine-simulation-persistence/pyproject.toml @@ -0,0 +1,39 @@ +[project] +name = "policyengine-simulation-persistence" +version = "0.1.0" +readme = "README.md" +authors = [ + {name = "PolicyEngine", email = "hello@policyengine.org"}, +] +license = {file = "../../LICENSE"} +requires-python = ">=3.13,<3.14" +dependencies = [ + "psycopg[binary]>=3.2,<4", + "sqlalchemy>=2,<3", +] + +[project.optional-dependencies] +test = [ + "pytest>=8.3.4", + "pytest-cov>=6.1.1", +] +build = [ + "pyright>=1.1.401", +] + +[build-system] +requires = ["hatchling>=1.26"] +build-backend = "hatchling.build" + +[tool.hatch.build.targets.wheel] +sources = ["src"] +only-include = ["src/policyengine_simulation_persistence"] +packages = ["src/policyengine_simulation_persistence"] + +[tool.pytest.ini_options] +pythonpath = ["src"] +testpaths = ["tests"] + +[tool.pyright] +# The pr.yml pyright job checks these paths: python -m scripts.rules pyright. +include = ["src"] diff --git a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/__init__.py b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/__init__.py new file mode 100644 index 000000000..bab08ce04 --- /dev/null +++ b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/__init__.py @@ -0,0 +1,11 @@ +"""Connections to the Public API's database for the simulation service.""" + +from policyengine_simulation_persistence.engine import ( + create_simulation_engine, + normalized_database_url, +) + +__all__ = [ + "create_simulation_engine", + "normalized_database_url", +] diff --git a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/engine.py b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/engine.py new file mode 100644 index 000000000..6d5bd1b15 --- /dev/null +++ b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/engine.py @@ -0,0 +1,60 @@ +"""Database connections for the simulation service's own role.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from sqlalchemy import create_engine +from sqlalchemy.engine import URL, make_url +from sqlalchemy.exc import ArgumentError + +if TYPE_CHECKING: + from sqlalchemy import Engine + +CONNECT_TIMEOUT_SECONDS = 5 +# Each Cloud Run instance serves up to 80 concurrent requests. Each database +# operation holds a connection for one short transaction, so a small pool is +# enough. +POOL_SIZE = 2 +MAX_OVERFLOW = 3 + + +def normalized_database_url(database_url: str) -> URL: + """Return ``database_url`` with the psycopg 3 driver selected. + + The stored secrets use the canonical ``postgresql://`` scheme. Raise + ValueError, without echoing the value, when the URL is missing, malformed + or incomplete. + """ + if not database_url or database_url != database_url.strip(): + msg = "Simulation database URL is missing or malformed" + raise ValueError(msg) + try: + parsed = make_url(database_url) + except (ArgumentError, ValueError): + # Raised without the parser's message, which may quote the URL. + msg = "Simulation database URL is malformed" + raise ValueError(msg) from None + if parsed.drivername != "postgresql": + msg = "Simulation database URL must use postgresql://" + raise ValueError(msg) + if not parsed.username or not parsed.host or not parsed.database: + msg = "Simulation database URL is incomplete" + raise ValueError(msg) + return parsed.set(drivername="postgresql+psycopg") + + +def create_simulation_engine(database_url: str) -> Engine: + """Build the engine the simulation service uses; it creates no objects.""" + return create_engine( + normalized_database_url(database_url), + pool_size=POOL_SIZE, + max_overflow=MAX_OVERFLOW, + pool_pre_ping=True, + connect_args={ + "connect_timeout": CONNECT_TIMEOUT_SECONDS, + # Never use server-side prepared statements, so the same engine + # works through Supabase's session and transaction poolers. + "prepare_threshold": None, + }, + ) diff --git a/libs/policyengine-simulation-persistence/tests/test_engine.py b/libs/policyengine-simulation-persistence/tests/test_engine.py new file mode 100644 index 000000000..bb7f87028 --- /dev/null +++ b/libs/policyengine-simulation-persistence/tests/test_engine.py @@ -0,0 +1,63 @@ +"""Connection settings for the simulation service's database, without a database.""" + +from __future__ import annotations + +import pytest +from policyengine_simulation_persistence import ( + create_simulation_engine, + normalized_database_url, +) + + +def test_database_url_selects_psycopg_without_changing_the_secret_format() -> None: + url = normalized_database_url( + "postgresql://simulation_service:secret@db.example:5432/postgres" + ) + + assert url.drivername == "postgresql+psycopg" + assert url.username == "simulation_service" + assert url.host == "db.example" + assert url.database == "postgres" + + +@pytest.mark.parametrize( + "database_url", + [ + "", + " postgresql://user:secret@db.example/postgres", + "postgres://user:secret@db.example/postgres", + "postgresql+psycopg://user:secret@db.example/postgres", + "postgresql://db.example/postgres", + "postgresql://user:secret@db.example", + "not a url", + ], +) +def test_malformed_database_urls_are_rejected_without_echoing_them( + database_url, +) -> None: + with pytest.raises(ValueError, match="Simulation database URL") as error: + normalized_database_url(database_url) + + assert "secret" not in str(error.value) + assert error.value.__cause__ is None or "secret" not in str(error.value.__cause__) + + +def test_runtime_engine_never_prepares_server_side_statements(monkeypatch) -> None: + captured_options = {} + + def capture_engine(url, **options): + captured_options.update(options, url=url) + return object() + + monkeypatch.setattr( + "policyengine_simulation_persistence.engine.create_engine", + capture_engine, + ) + + create_simulation_engine("postgresql://simulation_service:secret@db.example/postgres") + + assert captured_options["url"].drivername == "postgresql+psycopg" + assert captured_options["connect_args"]["prepare_threshold"] is None + assert captured_options["connect_args"]["connect_timeout"] > 0 + assert captured_options["pool_pre_ping"] is True + assert captured_options["pool_size"] >= 1 diff --git a/libs/policyengine-simulation-persistence/uv.lock b/libs/policyengine-simulation-persistence/uv.lock new file mode 100644 index 000000000..90f5cbc6e --- /dev/null +++ b/libs/policyengine-simulation-persistence/uv.lock @@ -0,0 +1,231 @@ +version = 1 +revision = 5 +requires-python = "==3.13.*" + +[[package]] +name = "colorama" +version = "0.4.6" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/d8/53/6f443c9a4a8358a93a6792e2acffb9d9d5cb0a5cfd8802644b7b1c9a02e4/colorama-0.4.6.tar.gz", hash = "sha256:08695f5cb7ed6e0531a20572697297273c47b8cae5a63ffc6d6ed5c201be6e44", size = 27697, upload-time = "2022-10-25T02:36:22.414Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/d1/d6/3965ed04c63042e047cb6a3e6ed1a63a35087b6a609aa3a15ed8ac56c221/colorama-0.4.6-py2.py3-none-any.whl", hash = "sha256:4f1d9991f5acc0ca119f9d443620b77f9d6b33703e51011c16baf57afb285fc6", size = 25335, upload-time = "2022-10-25T02:36:20.889Z" }, +] + +[[package]] +name = "coverage" +version = "7.17.1" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/02/9b/639ff6f5b3158999fab6f03cb7597526ad57e15cad0c6cfa9b64bac874dc/coverage-7.17.1.tar.gz", hash = "sha256:5c5c77f95a0e83e6b79eff948d251c9911cfd61d67f5059159fc25b7bc4aec27", size = 995565, upload-time = "2026-11-03T12:38:47.608Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/04/e0/ff26f5a74cfcfe131bebe09649a9c9dc84a8fe8fed5cbed0117c57f7efeb/coverage-7.17.1-cp313-cp313-android_24_arm64_v8a.whl", hash = "sha256:9b8d68b2330cda90e1972e78c93c6fd572453c5b4e29594aa98550694efb0466", size = 228775, upload-time = "2026-11-03T12:36:47.054Z" }, + { url = "https://files.pythonhosted.org/packages/df/af/b83c4a7d7ed81458298c137aeefbfb7a088d36fff6fb60a0f8375e731990/coverage-7.17.1-cp313-cp313-android_24_x86_64.whl", hash = "sha256:b975d0bd867eefeff37ba28aee6ee08a0987ca3ce6bfb5e0d3ddc1b40cf3e52e", size = 228845, upload-time = "2026-11-03T12:36:48.362Z" }, + { url = "https://files.pythonhosted.org/packages/52/07/3156d72bb538d05aa4cf51d90a86cc705e9cb570d8a0eeef80880cb21d09/coverage-7.17.1-cp313-cp313-ios_13_0_arm64_iphoneos.whl", hash = "sha256:60002a0ea7f5e5f64006aba9f37aa29a4b1e52756bc0fbbc2471188b2732e4ed", size = 227091, upload-time = "2026-11-03T12:36:49.687Z" }, + { url = "https://files.pythonhosted.org/packages/cb/50/03128c382b303e02a48b1f9658bda6412288fa1d83b478696a08e410c0f6/coverage-7.17.1-cp313-cp313-ios_13_0_arm64_iphonesimulator.whl", hash = "sha256:80b613bd68caa146d090023a18dfc504eb958ff525e31e6f2368b363502bb1ea", size = 227396, upload-time = "2026-11-03T12:36:51.162Z" }, + { url = "https://files.pythonhosted.org/packages/90/83/b617213e11609d6d5d64afc701e40384bbfb39c9999531ef747b6d1259cb/coverage-7.17.1-cp313-cp313-macosx_10_13_x86_64.whl", hash = "sha256:8c2e67f95c8de089ba0ff11c7b7ee83ec0890d418e8d0185e863367f5250a087", size = 227516, upload-time = "2026-11-03T12:36:52.897Z" }, + { url = "https://files.pythonhosted.org/packages/44/98/71e36bd7bc121a33c5c78a1f687587802b2806299feb4b9f2fcf21616007/coverage-7.17.1-cp313-cp313-macosx_11_0_arm64.whl", hash = "sha256:56f81ec13a8790a0f2ea30a14d2f39dec2374115eb88cb25cc2c0226f72f8b0e", size = 227781, upload-time = "2026-11-03T12:36:54.144Z" }, + { url = "https://files.pythonhosted.org/packages/04/e8/28f4c350e124242a757b86cd19d71d807eda8b202126c39fb57eb1495f7b/coverage-7.17.1-cp313-cp313-manylinux1_i686.manylinux_2_28_i686.manylinux_2_5_i686.whl", hash = "sha256:b7fb16b5c4fa1a37d69204ec5f7b8b187d2cfc54a2c084e2750ceee443748ac0", size = 258947, upload-time = "2026-11-03T12:36:55.461Z" }, + { url = "https://files.pythonhosted.org/packages/64/94/f99d4cbd1b187ac1029cadf293a934f8a2300908d8055b5c9de11577a875/coverage-7.17.1-cp313-cp313-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl", hash = "sha256:b05c06318fd46706c9055a1acc6f4aa0d3a63b211bf3b6114d3c1feab76e37f2", size = 261858, upload-time = "2026-11-03T12:36:56.804Z" }, + { url = "https://files.pythonhosted.org/packages/58/9c/b91da5d4790ac926e6bede81a1bac0b8e216d2574f5c332c7494d23b88de/coverage-7.17.1-cp313-cp313-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:8f3bbb5f9c074e97f7082987ead736036aa522cb39b996b8ed9c5aa8eee008be", size = 262791, upload-time = "2026-11-03T12:36:58.072Z" }, + { url = "https://files.pythonhosted.org/packages/4b/15/2527c7e46bcc7f69e792b7a0eba1e70d9effb5ae29f29d704487954975ba/coverage-7.17.1-cp313-cp313-manylinux2014_ppc64le.manylinux_2_17_ppc64le.manylinux_2_28_ppc64le.whl", hash = "sha256:6eb807f5eb02f64eb19f39b63015de046b312b847e3919b639a0ca19ae9a10eb", size = 264857, upload-time = "2026-11-03T12:36:59.79Z" }, + { url = "https://files.pythonhosted.org/packages/b8/83/6cf7a9cc1a4188227dc74e9293dd006e6e76b97e24747452f3b17b300f30/coverage-7.17.1-cp313-cp313-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:0bf74f2960a3d0f39067bddd658db1331a17828c93102922864699401df82dc7", size = 258994, upload-time = "2026-11-03T12:37:01.07Z" }, + { url = "https://files.pythonhosted.org/packages/98/0d/1f027bbe75df8b9553e87ad49889a288409e25bad00032391678b612c5ab/coverage-7.17.1-cp313-cp313-musllinux_1_2_aarch64.whl", hash = "sha256:3da4fe4cd099239c10691ff6deb9fd291e5438ad77c2583cf235a5b9aa9250ae", size = 260950, upload-time = "2026-11-03T12:37:02.553Z" }, + { url = "https://files.pythonhosted.org/packages/a4/25/5f02f89f262a126a90fda3061ea5beb0721202d9b8469b4a50adb85b2e2a/coverage-7.17.1-cp313-cp313-musllinux_1_2_i686.whl", hash = "sha256:d2c2973dcbcd883ec94305f513b7c05be6359679f25ad7134adf0cc512cf405e", size = 258871, upload-time = "2026-11-03T12:37:03.896Z" }, + { url = "https://files.pythonhosted.org/packages/50/2e/61f046a3ecddb2fc6e44350de857840e01fffbe83246302aa2382e8b9ac5/coverage-7.17.1-cp313-cp313-musllinux_1_2_ppc64le.whl", hash = "sha256:deea769f5199ea3e8053d5274e737e84f4a9cfbb1e69330066101825b098aac6", size = 262967, upload-time = "2026-11-03T12:37:05.191Z" }, + { url = "https://files.pythonhosted.org/packages/33/41/a45075de5bb61a646882912b7fc21cd594a83fe9be0ee6c77ffafa3a2ba7/coverage-7.17.1-cp313-cp313-musllinux_1_2_riscv64.whl", hash = "sha256:7fdea302c956e1e85526a294ce64495b0029edc451b4655e2a9a85af6029fa24", size = 258721, upload-time = "2026-11-03T12:37:06.44Z" }, + { url = "https://files.pythonhosted.org/packages/89/ac/a58424f93baf37f1f36a9ad60d622d84482e26b017157e101ad87e965e28/coverage-7.17.1-cp313-cp313-musllinux_1_2_x86_64.whl", hash = "sha256:32141eb9f9d228da33d45baf789736bc3c00a31ea9150aa7f48bbc08cfe2849f", size = 260799, upload-time = "2026-11-03T12:37:08.058Z" }, + { url = "https://files.pythonhosted.org/packages/27/10/9c424a67004ebeff99a03030115c4f8562eefabf0fa2ae889289efa85aa9/coverage-7.17.1-cp313-cp313-win32.whl", hash = "sha256:fc7b1ee12d7376ea85f6d09a272da3011c879e605d2f354d4a1c21c890cb571c", size = 229400, upload-time = "2026-11-03T12:37:09.337Z" }, + { url = "https://files.pythonhosted.org/packages/9e/0c/1b92a40cf24fa09e1bc2e7f9120a312e0b4a792b5f907852cbf64b75ab98/coverage-7.17.1-cp313-cp313-win_amd64.whl", hash = "sha256:26d0fa82cd76c888058c182b8ebb6326578f89f01cc1e576dde76bba8e446261", size = 230128, upload-time = "2026-11-03T12:37:10.69Z" }, + { url = "https://files.pythonhosted.org/packages/c3/af/67fa4bc27bdd15514810ac43c1f1285ca46e9b7af29859655948574f419a/coverage-7.17.1-cp313-cp313-win_arm64.whl", hash = "sha256:ddaca2c745275aede7b7511618aba9e1e17728f772edd1d2016529def73df01d", size = 229853, upload-time = "2026-11-03T12:37:12.173Z" }, + { url = "https://files.pythonhosted.org/packages/18/61/7ad3e5f92557c73044d3dfaf88cc65fb7b1ad589c744e8629af8e6370224/coverage-7.17.1-py3-none-any.whl", hash = "sha256:fda1023828e4a9f4a40ef06783e005f3630f805f839edc679c87f597f33e041f", size = 219074, upload-time = "2026-11-03T12:38:46.229Z" }, +] + +[[package]] +name = "iniconfig" +version = "2.3.1" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/01/e1/2069291243c926a2ff1cd706c7f3eeb9b62144bf60f77c9fb9ff2fb26bd3/iniconfig-2.3.1.tar.gz", hash = "sha256:67f4b9c50da0dedf52af349e7749a80a9057a5031199791b906c3bb3ae878960", size = 21209, upload-time = "2026-10-06T22:48:38.076Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/56/43/4ca9e49d27a1fcf6bece6f6aec0ea46bb9112489b93d4b688fb415457bdb/iniconfig-2.3.1-py3-none-any.whl", hash = "sha256:9121e2c1fdb355232495be3194c8dfe87ccc2d5dee45947b78e68f499790d7a7", size = 7552, upload-time = "2026-10-06T22:48:36.959Z" }, +] + +[[package]] +name = "nodeenv" +version = "1.12.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/e6/99/d27cfc9518f49e4cbc25c683ec7cf8704a9d31311341c291c0e78ddf791b/nodeenv-1.12.0.tar.gz", hash = "sha256:4ea7f88339310afebc3253f2c7a91b94bacf97d73a36a5c108962abdfaca501e", size = 131292, upload-time = "2026-10-31T19:03:18.847Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/2d/6a/8a9ac7d5e2f7cf53c459bfef90ebc17a2f8cd54b665af96799d4c534840e/nodeenv-1.12.0-py3-none-any.whl", hash = "sha256:80644d65c10cc2be12e3e4b54b3367ff288c7ad24ae75c2dcc65454fc7449d4d", size = 39442, upload-time = "2026-10-31T19:03:17.427Z" }, +] + +[[package]] +name = "packaging" +version = "26.3" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/7d/fa/3944b40b07da9ce895c0e6303a5ab7d53da063554f534556b134a54d6093/packaging-26.3.tar.gz", hash = "sha256:94edc256424af38762eb31306eed28beb9f0efc50a8837492c9d6fd6004aed79", size = 313412, upload-time = "2026-08-04T18:15:28.737Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/63/34/ba1c580383c9eada3711951fef0795c80b829a078d72188184bcab9dd527/packaging-26.3-py3-none-any.whl", hash = "sha256:d7193f7c8e4e93f444fde0262bf90af30e16fa0ad0ad44cb553c87339b23cd1c", size = 129956, upload-time = "2026-08-04T18:15:27.159Z" }, +] + +[[package]] +name = "pluggy" +version = "1.7.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/bf/db/7fc19e6f2dc92a966727031389fc2e08b558f0f25eb7403c1119ad4713cd/pluggy-1.7.0.tar.gz", hash = "sha256:d1eaa46ebb595891b860ab086b4d09c8588af65ebd4361b8e8f4bb8920b90ba8", size = 123304, upload-time = "2026-10-15T09:50:58.343Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/40/9e/2b38731e0fc536806f16490e1a12d7f0dc2a1235aa8cc07bcc75416a7daa/pluggy-1.7.0-py3-none-any.whl", hash = "sha256:7dd7b0d8832ba3cb632c306926ded123429211b83641b35dc5c41ad2d34f9bec", size = 27082, upload-time = "2026-10-15T09:50:56.808Z" }, +] + +[[package]] +name = "policyengine-simulation-persistence" +version = "0.1.0" +source = { editable = "." } +dependencies = [ + { name = "psycopg", extra = ["binary"] }, + { name = "sqlalchemy" }, +] + +[package.optional-dependencies] +build = [ + { name = "pyright" }, +] +test = [ + { name = "pytest" }, + { name = "pytest-cov" }, +] + +[package.metadata] +requires-dist = [ + { name = "psycopg", extras = ["binary"], specifier = ">=3.2,<4" }, + { name = "pyright", marker = "extra == 'build'", specifier = ">=1.1.401" }, + { name = "pytest", marker = "extra == 'test'", specifier = ">=8.3.4" }, + { name = "pytest-cov", marker = "extra == 'test'", specifier = ">=6.1.1" }, + { name = "sqlalchemy", specifier = ">=2,<3" }, +] +provides-extras = ["test", "build"] + +[[package]] +name = "psycopg" +version = "3.3.7" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "tzdata", marker = "sys_platform == 'win32'" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/77/a3/55c9fb649391fddec8d7ba3005162cd76e8cb3b2eea3dc922684aa0013e4/psycopg-3.3.7.tar.gz", hash = "sha256:6574f23fd79bf129ee0616b49f6e358faf7ae1bf3a362d2ad599cf8f9c3d4935", size = 168912, upload-time = "2026-11-03T03:58:07.876Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/b0/2a/2e05cc01b2817b8baff419823774bb928efd0a36de12da78f19482cfa329/psycopg-3.3.7-py3-none-any.whl", hash = "sha256:d592e88a077e01ff57f22d59f16e828b92f2b2d9f439b6679d582b48d74cf306", size = 216207, upload-time = "2026-11-03T03:51:21.09Z" }, +] + +[package.optional-dependencies] +binary = [ + { name = "psycopg-binary", marker = "implementation_name != 'pypy'" }, +] + +[[package]] +name = "psycopg-binary" +version = "3.3.7" +source = { registry = "https://pypi.org/simple" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/ce/ff/936c13638c8b9bd4519d3ad7996e7600b066909156cb1cd6f55b73d2203e/psycopg_binary-3.3.7-cp313-cp313-macosx_10_13_x86_64.whl", hash = "sha256:e0639d06c50c684bb9867fb4c0593b7327525275142c0b620053c55134572773", size = 4715405, upload-time = "2026-11-03T03:54:45.749Z" }, + { url = "https://files.pythonhosted.org/packages/7a/19/98a3d8dd5b61478ce242f017b70d68241d151826409ab3f88066ed2ec327/psycopg_binary-3.3.7-cp313-cp313-macosx_11_0_arm64.whl", hash = "sha256:89b508e50ccfe93a440ce9450c33ff78a3e2462dab369e66ee63e725562421cb", size = 4775846, upload-time = "2026-11-03T03:54:51.138Z" }, + { url = "https://files.pythonhosted.org/packages/9d/74/c855489f8be5ee3d8bfd4cee523e036a49b3aa1bfdc6eb5b72c34828fdb2/psycopg_binary-3.3.7-cp313-cp313-manylinux2014_ppc64le.manylinux_2_17_ppc64le.whl", hash = "sha256:a0aeb1b680b2ee414a0196a9dd94b17edaa75c1c04f7cddadc9aae95d0dd940c", size = 5575585, upload-time = "2026-11-03T03:54:57.816Z" }, + { url = "https://files.pythonhosted.org/packages/ff/c9/860db102775b7fcf1f08fe06b0d845f926fe5ab89599d23007693354331d/psycopg_binary-3.3.7-cp313-cp313-manylinux2014_x86_64.manylinux_2_17_x86_64.whl", hash = "sha256:e3cc435fd83ff3fe627370bb8208bbcfd79631be2ac3b49b7a5c7edb528011c9", size = 5301826, upload-time = "2026-11-03T03:55:05.309Z" }, + { url = "https://files.pythonhosted.org/packages/a7/37/5f09fb2d1ad05f439329350097c702c52c1355156c176e3515d12496f723/psycopg_binary-3.3.7-cp313-cp313-manylinux_2_27_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:0b311c834ece61e9be0df48c6078ca8949ef50d43a884f4c05628f77a8b31442", size = 6852614, upload-time = "2026-11-03T03:55:14.88Z" }, + { url = "https://files.pythonhosted.org/packages/6e/62/befa167c2ea5bfdeb65ce0f799aa35d78eb693250b650963ec7cba1fa17d/psycopg_binary-3.3.7-cp313-cp313-manylinux_2_38_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:15873c72c6db035b7bbe9d443dedc571753c713b7d10223aea6438310c0f468e", size = 5076255, upload-time = "2026-11-03T03:55:23.593Z" }, + { url = "https://files.pythonhosted.org/packages/eb/97/8e622891df044606e0819cd17b589d4b47834eddd3f941848edab207b7e8/psycopg_binary-3.3.7-cp313-cp313-musllinux_1_2_aarch64.whl", hash = "sha256:191dc5f2056368e0556359be7782a3be201399c7f1dfc3fe0ebffde37446e1f2", size = 4619359, upload-time = "2026-11-03T03:55:31.516Z" }, + { url = "https://files.pythonhosted.org/packages/40/84/857ec7d5ab5e63ec05e98d22d0e8c3b85df5a49cdfb8b3fc84879424040a/psycopg_binary-3.3.7-cp313-cp313-musllinux_1_2_ppc64le.whl", hash = "sha256:21dccda9c04e52240b9d6d934c5b710c198fdd7d0c60a358d78927988bca154e", size = 4301370, upload-time = "2026-11-03T03:55:35.691Z" }, + { url = "https://files.pythonhosted.org/packages/d9/f3/9a21844b395d49dbe7e6fd53c7cddfc7e54766e7d9957c89a8fec6c75c2a/psycopg_binary-3.3.7-cp313-cp313-musllinux_1_2_riscv64.whl", hash = "sha256:9ae3e685302569c34bf159927e9667d3afd5dd34e68a92afabe05e2022dafc68", size = 4021521, upload-time = "2026-11-03T03:55:39.78Z" }, + { url = "https://files.pythonhosted.org/packages/08/d7/8eda1d662b7049a6729a6b5f30601561e5204bb7ff3ae15796640afdba53/psycopg_binary-3.3.7-cp313-cp313-musllinux_1_2_x86_64.whl", hash = "sha256:4c8ab53d4a7c9f68d4b2cf878d1e56880bb87a1008435a5c3770422ac1e1b4ef", size = 4334750, upload-time = "2026-11-03T03:55:45.384Z" }, + { url = "https://files.pythonhosted.org/packages/dd/92/2a1ad37faa52270594e92c483a28084d75a3c696b95ad8f6fc18faf39ead/psycopg_binary-3.3.7-cp313-cp313-win_amd64.whl", hash = "sha256:63fc0a5a226e63db37a3f038f2581ea8b8189112daa86bd8ae52c3897ca105da", size = 3618325, upload-time = "2026-11-03T03:55:49.757Z" }, +] + +[[package]] +name = "pygments" +version = "2.21.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/49/2e/ced460408999b33da6b31b0021b0f37d329e202d4169aeb164493778f25b/pygments-2.21.0.tar.gz", hash = "sha256:610ca751c9bc2492b38eb9a38a7fbc93edbbb2d7182edaf34e66ae493dee5c8c", size = 5005329, upload-time = "2026-08-17T08:02:48.824Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/71/46/17f022dd3e953bf20a04a028a21ec746d942f8d2af30fa0f124fa0e6a684/pygments-2.21.0-py3-none-any.whl", hash = "sha256:2363c69b61c4a97c838da3b130dcd6468f4848992b21a82f2a63ec34377137d9", size = 1250147, upload-time = "2026-08-17T08:02:44.912Z" }, +] + +[[package]] +name = "pyright" +version = "1.1.414" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "nodeenv" }, + { name = "typing-extensions" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/e1/1b/244c7b710031ada80f27e579ec20d28a2285dfc318fed0339866b1047f12/pyright-1.1.414.tar.gz", hash = "sha256:523c0a97c60da6333234955c277730c9cf4f5bd6d5399e7b7d2b0fc5d3599524", size = 4154638, upload-time = "2026-09-10T12:26:53.181Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/d7/ba/18b6e682ead424ad24bcc134339ae5d1b931cd9ae260540592a058a91279/pyright-1.1.414-py3-none-any.whl", hash = "sha256:2a6b4b3298c9eec174c5ed83bd338de6eee82df2992f3e1930e6199d381be36f", size = 6225049, upload-time = "2026-09-10T12:26:51.427Z" }, +] + +[[package]] +name = "pytest" +version = "9.2.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "colorama", marker = "sys_platform == 'win32'" }, + { name = "iniconfig" }, + { name = "packaging" }, + { name = "pluggy" }, + { name = "pygments" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/79/42/bb4641652e58dedeac2ec51abc1f8cae4fee4ec63899a0999016229a0f2a/pytest-9.2.0.tar.gz", hash = "sha256:97c4f3b1f3c7414aa36465e09b8220f1d03627993bd51b1282e193753c80557b", size = 1797555, upload-time = "2026-10-27T21:30:20.728Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/51/39/1f5d8c8877a0fa327db959c19341d4be61660de1ab6b7391f9eb805bcd67/pytest-9.2.0-py3-none-any.whl", hash = "sha256:07acdcf88ec174f907dc01af70bf52d49b014bbab03faeeade31c96b18c2e588", size = 411776, upload-time = "2026-10-27T21:30:19.131Z" }, +] + +[[package]] +name = "pytest-cov" +version = "7.1.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "coverage" }, + { name = "pluggy" }, + { name = "pytest" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/b1/51/a849f96e117386044471c8ec2bd6cfebacda285da9525c9106aeb28da671/pytest_cov-7.1.0.tar.gz", hash = "sha256:30674f2b5f6351aa09702a9c8c364f6a01c27aae0c1366ae8016160d1efc56b2", size = 55592, upload-time = "2026-03-21T20:11:16.284Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/9d/7a/d968e294073affff457b041c2be9868a40c1c71f4a35fcc1e45e5493067b/pytest_cov-7.1.0-py3-none-any.whl", hash = "sha256:a0461110b7865f9a271aa1b51e516c9a95de9d696734a2f71e3e78f46e1d4678", size = 22876, upload-time = "2026-03-21T20:11:14.438Z" }, +] + +[[package]] +name = "sqlalchemy" +version = "2.1.6" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "typing-extensions" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/36/dc/9addbf490110b5727380059ee542ad8353bdcee45a37ba6a5e0976966ae0/sqlalchemy-2.1.6.tar.gz", hash = "sha256:499eae5c9051efe96cb9a73ec2fbde8ffa4186f102b9094ebab8fcc9d3b99f52", size = 10589700, upload-time = "2026-10-21T16:25:06.497Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/d7/db/ecf1cf1f5c8cb7661081dcb0561633c174213d7af5fccc17ab48cbe9812a/sqlalchemy-2.1.6-cp313-cp313-macosx_11_0_arm64.whl", hash = "sha256:0534d3f41295a74d9fe2e0838e06ce6329a0855b2965df5f5d1c4d3f6f1406d0", size = 2468626, upload-time = "2026-10-21T18:49:33.602Z" }, + { url = "https://files.pythonhosted.org/packages/9b/98/1cccefb4fc94f4efd621604063acbbea4e3d530c27151fd7249d68adabf2/sqlalchemy-2.1.6-cp313-cp313-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:650f97996a4c529f4930969f3ba11bcdb57ff160ed1cb87ea0174393bf4f5f73", size = 4602545, upload-time = "2026-10-21T18:54:18.766Z" }, + { url = "https://files.pythonhosted.org/packages/d3/b0/1817d7406ed92c8647a1d1db965a1bc2dd5a3f80d18e4d28ad62daf9bd72/sqlalchemy-2.1.6-cp313-cp313-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:0bada6bd29ae58a0581bf6977c0a43dfd67388fd09129f1475c37eca370cccd8", size = 4655491, upload-time = "2026-10-21T19:05:20.434Z" }, + { url = "https://files.pythonhosted.org/packages/41/5c/0bdaa1676fb182b93e3c87c09cafd08a734ef29820aac6312b87a88f8537/sqlalchemy-2.1.6-cp313-cp313-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:96f5fcdfb7e89b6d895e2ddc165f67b3126a585dfdf2a95b0c1f3fcf3380d7d4", size = 4319667, upload-time = "2026-10-21T19:23:16.637Z" }, + { url = "https://files.pythonhosted.org/packages/c1/d7/319e5a049b6f5235d1229858972abb05b8eae2fdd6639dc7bdbbc9cdb68d/sqlalchemy-2.1.6-cp313-cp313-musllinux_1_2_aarch64.whl", hash = "sha256:015bd7b0687b5feaecb9988bc2f36ecb39daca83bd806b094b43b19ea503b0a9", size = 4525975, upload-time = "2026-10-21T18:54:21.553Z" }, + { url = "https://files.pythonhosted.org/packages/19/3b/6e8e5c14a3bcfae59a081a27f33e4f225df8140daab8e90fc3c42afd0d0e/sqlalchemy-2.1.6-cp313-cp313-musllinux_1_2_riscv64.whl", hash = "sha256:bf883ae772971bc6155cb64cdfb9e31c50a2099f3b7c8b51b0401bf608a9fe8e", size = 4320900, upload-time = "2026-10-21T19:23:18.835Z" }, + { url = "https://files.pythonhosted.org/packages/b8/a7/7ad220d5ea6308a9a32395785f4ce8beff4d00b0ae56ab9f2dafcca28743/sqlalchemy-2.1.6-cp313-cp313-musllinux_1_2_x86_64.whl", hash = "sha256:1ff897d13de613768123dee68474956a51fe641c42bba7af6cb7776716a8dbcb", size = 4616756, upload-time = "2026-10-21T19:05:22.265Z" }, + { url = "https://files.pythonhosted.org/packages/08/81/53a198f5a29f247adc68c0c6da69bd2c7bdd89d3ffc58f9e626ee3160f18/sqlalchemy-2.1.6-cp313-cp313-win32.whl", hash = "sha256:9db8bacb2a69b4b5388bc1439463170af28f4cc710933085b81d1658d931d2c2", size = 2382042, upload-time = "2026-10-21T17:11:33.968Z" }, + { url = "https://files.pythonhosted.org/packages/b4/87/69e86a23cd9ccef0d654c09dc69dfea6d62b1311d2f13ca3daab1bcd922e/sqlalchemy-2.1.6-cp313-cp313-win_amd64.whl", hash = "sha256:b9f0b92b4a88b630f1e67df816b43cb017bb583479917dd100541d4f490a9517", size = 2432597, upload-time = "2026-10-21T17:11:35.514Z" }, + { url = "https://files.pythonhosted.org/packages/98/90/a7f81395135cc5da724403ece4340f7e04c1ce752da15d0ad337591ec9c9/sqlalchemy-2.1.6-cp313-cp313-win_arm64.whl", hash = "sha256:bc495d00f235c3df6f376187a4ef48a56e162fdcc0a113ee34518332aa1aa2df", size = 2393224, upload-time = "2026-10-21T18:54:18.405Z" }, + { url = "https://files.pythonhosted.org/packages/c1/69/c75534e336b06b3fd3f76de73d89736510800e60c77c58d4e4ce18389018/sqlalchemy-2.1.6-py3-none-any.whl", hash = "sha256:106878cf3795cd3d357e2e858609f9e44f20f19b2d8e48a3532bef3f90b03fbe", size = 2063055, upload-time = "2026-10-21T18:42:10.234Z" }, +] + +[[package]] +name = "typing-extensions" +version = "4.16.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/f6/cc/6253133b5bb138fc3306cebfbda2c520f545d36b5be2c7255cc528bb45d6/typing_extensions-4.16.0.tar.gz", hash = "sha256:dc983d19a509c94dba722ee6abd33940f7c05a89e243c47e907eb4db6f1a43e5", size = 113555, upload-time = "2026-07-02T08:40:05.92Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/49/d3/b8441a820a491ddfc024b0b0cf0393375b75ea13866d9c66727e54c2fc80/typing_extensions-4.16.0-py3-none-any.whl", hash = "sha256:481caa481374e813c1b176ada14e97f1f67a4539ce9cfeb3f350d78d6370c2e8", size = 45571, upload-time = "2026-07-02T08:40:04.659Z" }, +] + +[[package]] +name = "tzdata" +version = "2026.6" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/7a/29/6a1391d31cd00ff9e927215a224a880e1e84e826b64ba80a499e51c074d8/tzdata-2026.6.tar.gz", hash = "sha256:85d2cd9c09f81f0bb29db27759fc83af4f1e620efb814f8c5bf8b4cc4feece6d", size = 201017, upload-time = "2026-10-28T09:57:09.137Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/0a/66/9a06c2227fedfd4919f0eaaa8b94377d9730882a7e4f95823c9a331ab5e1/tzdata-2026.6-py2.py3-none-any.whl", hash = "sha256:0a10eec6e13ef16b627fabed9a5f2ba61a62b1c67f327305f4e7e412a70b4aec", size = 348210, upload-time = "2026-10-28T09:57:07.276Z" }, +] diff --git a/projects/policyengine-simulation-entry/tests/test_database_access_boundaries.py b/projects/policyengine-simulation-entry/tests/test_database_access_boundaries.py index 81d7ad590..18293a918 100644 --- a/projects/policyengine-simulation-entry/tests/test_database_access_boundaries.py +++ b/projects/policyengine-simulation-entry/tests/test_database_access_boundaries.py @@ -34,6 +34,7 @@ "libs/policyengine-fastapi/src", "libs/policyengine-simulation-contract/src", "libs/policyengine-simulation-observability/src", + "libs/policyengine-simulation-persistence/src", "libs/policyengine-stage12-persistence/src", ".github/scripts", ) diff --git a/rules/registry.toml b/rules/registry.toml index 1b314011d..289f51b83 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -1125,11 +1125,12 @@ invocation = "unittest discover -s scripts/rules/tests" [[rule]] id = "sim.quality.pyright-libraries-and-entry" category = "quality" -statement = "The contract, observability, Stage 12 persistence and fastapi libraries and the simulation entry have no Pyright errors, checked with the pinned Pyright version in each package's own locked environment." +statement = "The contract, observability, simulation persistence, Stage 12 persistence and fastapi libraries and the simulation entry have no Pyright errors, checked with the pinned Pyright version in each package's own locked environment." rationale = "docs/engineering/skills/type-checking.md#rules" scope = [ "libs/policyengine-simulation-contract/src/**/*.py", "libs/policyengine-simulation-observability/src/**/*.py", + "libs/policyengine-simulation-persistence/src/**/*.py", "libs/policyengine-stage12-persistence/src/**/*.py", "libs/policyengine-fastapi/src/**/*.py", "projects/policyengine-simulation-entry/src/**/*.py", @@ -1142,6 +1143,7 @@ kind = "pyright" projects = [ "libs/policyengine-simulation-contract", "libs/policyengine-simulation-observability", + "libs/policyengine-simulation-persistence", "libs/policyengine-stage12-persistence", "libs/policyengine-fastapi", "projects/policyengine-simulation-entry", From 5f4b6fe3f3b4e6991b2fbb6fdbfc9eecc46a5474 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 3 Nov 2026 23:59:46 +0400 Subject: [PATCH 02/25] Report in the readiness check whether the simulation service reached its database The Simulation Entrypoint reads SIMULATION_DATABASE_URL, its own role's URL for the database that holds the Public API's tables. When it is set, the app builds a pooled engine (no connection opens until first use), disposes of it at shutdown, and GET /ready reports whether a pooled connection answered a trivial query: {"status": "ready", "simulation_database": {"reachable": true}} The database does not change the readiness status, because no old-gateway or Stage 12 route uses it yet: an unreachable or slow database (bounded at three seconds) is reported as unreachable, logged with its error type only, and /ready keeps following the old gateway. Without the setting, /ready keeps its old shape. A URL that does not use the canonical postgresql:// scheme fails validation without repeating the URL. The entrypoint depends on the persistence library, so its image's build context now includes it; a new test checks that every local library the entrypoint installs is in the build context. Co-Authored-By: Claude Opus 5.5 --- docs/generated/rules.md | 2 +- .../Dockerfile.dockerignore | 2 + .../pyproject.toml | 2 + .../src/policyengine_simulation_entry/app.py | 57 +++++- .../policyengine_simulation_entry/config.py | 14 ++ .../entry_runtime.py | 17 ++ .../policyengine_simulation_entry/schemas.py | 22 +++ .../simulation_database.py | 40 ++++ .../tests/test_config.py | 38 ++++ .../tests/test_deployment_assets_traffic.py | 23 +++ .../tests/test_openapi.py | 16 +- .../tests/test_simulation_database.py | 186 ++++++++++++++++++ .../policyengine-simulation-entry/uv.lock | 21 ++ rules/registry.toml | 5 +- 14 files changed, 434 insertions(+), 11 deletions(-) create mode 100644 projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py create mode 100644 projects/policyengine-simulation-entry/tests/test_simulation_database.py diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 0810123d7..f1e902e4a 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -142,7 +142,7 @@ flowchart LR | Rule | Statement | Tier | Status | Blocking | Enforced by | CI | Proof | | --- | --- | --- | --- | --- | --- | --- | --- | -| `sim.deploy.container-context-minimal` | Container builds exclude local environments and unrelated projects, and the entrypoint container builds for the requested target platform. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py`, `projects/policyengine-simulation-entry/tests/test_deployment_assets_traffic.py` | `pr.yml:test` | **unproven** | +| `sim.deploy.container-context-minimal` | Container builds exclude local environments and unrelated projects but include every local library the entrypoint installs, and the entrypoint container builds for the requested target platform. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py`, `projects/policyengine-simulation-entry/tests/test_deployment_assets_traffic.py` | `pr.yml:test` | **unproven** | | `sim.deploy.docker-images-build` | The simulation entry and executor Docker images build. | mechanical | enforced | yes | CI job `docker-build` | `pr.yml:docker-build` | — | | `sim.deploy.gcloud-without-terraform` | Deployment uses the gcloud-based workflow, not Terraform. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets.py` | `pr.yml:test` | **unproven** | | `sim.deploy.image-smoke-covers-every-image-lib` | The image smoke path filter includes every library mounted into a Modal image, so a change to any of them runs the smoke. | mechanical | doc-only | no | nothing | — | — | diff --git a/projects/policyengine-simulation-entry/Dockerfile.dockerignore b/projects/policyengine-simulation-entry/Dockerfile.dockerignore index 9379db178..a54f9060f 100644 --- a/projects/policyengine-simulation-entry/Dockerfile.dockerignore +++ b/projects/policyengine-simulation-entry/Dockerfile.dockerignore @@ -13,6 +13,8 @@ libs/* !libs/policyengine-simulation-contract/** !libs/policyengine-simulation-observability/ !libs/policyengine-simulation-observability/** +!libs/policyengine-simulation-persistence/ +!libs/policyengine-simulation-persistence/** !libs/policyengine-stage12-persistence/ !libs/policyengine-stage12-persistence/** libs/**/tests diff --git a/projects/policyengine-simulation-entry/pyproject.toml b/projects/policyengine-simulation-entry/pyproject.toml index 1b43b4f1d..11f569523 100644 --- a/projects/policyengine-simulation-entry/pyproject.toml +++ b/projects/policyengine-simulation-entry/pyproject.toml @@ -19,6 +19,7 @@ dependencies = [ "policyengine-observability[fastapi,google,httpx,otlp-grpc]>=3.0.4,<4", "policyengine-simulation-contract", "policyengine-simulation-observability", + "policyengine-simulation-persistence", "policyengine-stage12-persistence", "pyjwt>=2.10.1,<3", "uvicorn>=0.35.0,<1", @@ -30,6 +31,7 @@ packages = ["src/policyengine_simulation_entry"] [tool.uv.sources] policyengine-simulation-contract = { path = "../../libs/policyengine-simulation-contract", editable = true } policyengine-simulation-observability = { path = "../../libs/policyengine-simulation-observability", editable = true } +policyengine-simulation-persistence = { path = "../../libs/policyengine-simulation-persistence", editable = true } policyengine-stage12-persistence = { path = "../../libs/policyengine-stage12-persistence", editable = true } [project.optional-dependencies] diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py index 759610fc6..b9a2ac66c 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py @@ -25,7 +25,6 @@ JobSubmitResponse, PingRequest, PingResponse, - ReadinessResponse, SimulationErrorResponse, SimulationRequest, VersionMap, @@ -59,15 +58,19 @@ ComparisonBackend, EntryRuntime, ResponseIdentifier, + SimulationDatabaseConnection, model_json, request_identifiers, route_template, set_observability_header, ) from policyengine_simulation_entry.schemas import ( + SimulationDatabaseReadiness, + SimulationEntrypointReadinessResponse, TemporaryStage12ReportResponse, TemporaryStage12SubmissionResponse, ) +from policyengine_simulation_entry.simulation_database import SimulationDatabase from policyengine_simulation_entry.stage12_backend import ( Stage12ComparisonBackend, TemporaryStage12DispatchFailed, @@ -84,11 +87,13 @@ "STAGE12_MODAL_SUBMISSION_TIMEOUT_SECONDS", "ComparisonBackend", "ResponseIdentifier", + "SimulationDatabaseConnection", "app", "create_app", ] logger = logging.getLogger(__name__) +SIMULATION_DATABASE_READINESS_TIMEOUT_SECONDS = 3.0 def _temporary_submission_payload( @@ -462,6 +467,26 @@ async def get_budget_window_job( ) +async def _simulation_database_readiness( + database: SimulationDatabaseConnection, +) -> SimulationDatabaseReadiness: + """Report whether the simulation service reached its database.""" + try: + await asyncio.wait_for( + database.check_connection(), + timeout=SIMULATION_DATABASE_READINESS_TIMEOUT_SECONDS, + ) + # Any failure to reach the database is reported, never raised: the + # old-gateway routes this check covers do not use it. + except Exception as error: # noqa: BLE001 # reported as unreachable + logger.warning( + "simulation_database_unreachable", + extra={"error_type": type(error).__name__}, + ) + return SimulationDatabaseReadiness(reachable=False) + return SimulationDatabaseReadiness(reachable=True) + + def _register_service_routes(app: FastAPI, entry: EntryRuntime) -> None: """Add the public version, health, readiness and ping routes.""" @@ -494,14 +519,23 @@ async def get_country_versions(kind: str, request: Request) -> Response: async def health() -> HealthResponse: return HealthResponse() - @app.get("/ready", response_model=ReadinessResponse) + @app.get("/ready", response_model=SimulationEntrypointReadinessResponse) async def ready() -> Response: - if await entry.backend.ready(): - return JSONResponse(ReadinessResponse(status="ready").model_dump()) - return JSONResponse( - ReadinessResponse(status="not_ready").model_dump(), - status_code=503, + backend_ready = await entry.backend.ready() + status = "ready" if backend_ready else "not_ready" + status_code = 200 if backend_ready else 503 + if entry.database is None: + # Without a configured database the response keeps its old shape. + readiness = SimulationEntrypointReadinessResponse(status=status) + return JSONResponse( + readiness.model_dump(mode="json", exclude={"simulation_database"}), + status_code=status_code, + ) + readiness = SimulationEntrypointReadinessResponse( + status=status, + simulation_database=await _simulation_database_readiness(entry.database), ) + return JSONResponse(readiness.model_dump(mode="json"), status_code=status_code) @app.post( "/ping", @@ -521,9 +555,15 @@ def create_app( backend: SimulationBackend | None = None, auth_dependency: AuthenticationDependency | None = None, comparison_backend: ComparisonBackend | None = None, + simulation_database: SimulationDatabaseConnection | None = None, ) -> FastAPI: """Build the app with injectable auth/backend seams for hermetic tests.""" runtime_settings = settings or Settings.from_env() + runtime_database = simulation_database + if runtime_database is None and runtime_settings.simulation_database_url: + runtime_database = SimulationDatabase.from_url( + runtime_settings.simulation_database_url + ) @asynccontextmanager async def lifespan(_: FastAPI) -> AsyncIterator[None]: @@ -533,6 +573,8 @@ async def lifespan(_: FastAPI) -> AsyncIterator[None]: yield finally: await runtime_backend.close() + if runtime_database is not None: + runtime_database.dispose() runtime.shutdown() app = FastAPI( @@ -571,6 +613,7 @@ async def lifespan(_: FastAPI) -> AsyncIterator[None]: runtime=runtime, backend=runtime_backend, comparison=runtime_comparison, + database=runtime_database, ) _install_request_context(app, runtime_settings) diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/config.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/config.py index 8385097e9..e9c7af833 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/config.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/config.py @@ -92,6 +92,10 @@ class Settings: stage12_database_url: str = "" stage12_artifact_bucket: str = "" stage12_retention_days: int = REQUIRED_STAGE12_RETENTION_DAYS + # The simulation service role's URL for the database that holds the Public + # API's tables. Empty outside deployed environments; nothing on the + # old-gateway or Stage 12 paths reads it. + simulation_database_url: str = "" @classmethod def from_env(cls) -> Settings: @@ -142,6 +146,7 @@ def from_env(cls) -> Settings: stage12_database_url=os.getenv("STAGE12_DATABASE_URL", ""), stage12_artifact_bucket=os.getenv("STAGE12_ARTIFACT_BUCKET", ""), stage12_retention_days=int(os.getenv("STAGE12_RETENTION_DAYS", "30")), + simulation_database_url=os.getenv("SIMULATION_DATABASE_URL", ""), ) @property @@ -170,6 +175,7 @@ def validate(self) -> None: self._validate_caller_auth() self._validate_old_gateway() self._validate_stage12() + self._validate_simulation_database() def _validate_caller_auth(self) -> None: """Require caller authentication in production, with an HTTPS issuer.""" @@ -267,3 +273,11 @@ def _validate_stage12(self) -> None: ): msg = "STAGE12_DATABASE_URL must use the canonical postgresql:// scheme." raise ConfigurationError(msg) + + def _validate_simulation_database(self) -> None: + """Require the canonical scheme for the service role's URL, when set.""" + if self.simulation_database_url and not self.simulation_database_url.startswith( + "postgresql://" + ): + msg = "SIMULATION_DATABASE_URL must use the canonical postgresql:// scheme." + raise ConfigurationError(msg) diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/entry_runtime.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/entry_runtime.py index 279700705..4b6d5fcc6 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/entry_runtime.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/entry_runtime.py @@ -210,6 +210,21 @@ def _add_response_telemetry( attributes[response_identifier_key] = response_identifier +class SimulationDatabaseConnection(Protocol): + """The simulation service's database, as the routes use it. + + The real implementation is ``SimulationDatabase``; tests inject fakes. + """ + + async def check_connection(self) -> None: + """Raise unless a pooled connection answers a trivial query.""" + ... + + def dispose(self) -> None: + """Close every pooled connection.""" + ... + + class EntryRuntime: """The settings, telemetry runtime and backends the entry routes share.""" @@ -220,12 +235,14 @@ def __init__( runtime: ObservabilityRuntime, backend: SimulationBackend, comparison: ComparisonBackend | None, + database: SimulationDatabaseConnection | None, ) -> None: """Keep the dependencies that ``create_app`` resolved.""" self.settings = settings self.runtime = runtime self.backend = backend self.comparison = comparison + self.database = database def bind_observability_id(self, request: Request, value: object) -> str | None: """Bind a normalized observability ID to the request and telemetry context.""" diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/schemas.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/schemas.py index d387526df..00876581c 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/schemas.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/schemas.py @@ -115,3 +115,25 @@ class TemporaryStage12ReportResponse(BaseModel): report: ComparisonReportRecord simulations: tuple[ComparisonSimulationRecord, ...] + + +class SimulationDatabaseReadiness(BaseModel): + """The simulation service's database, as the readiness check found it. + + ``reachable`` says whether a pooled connection answered within the check's + time limit. It doesn't change ``status``: no old-gateway or Stage 12 route + uses this database. + """ + + model_config = ConfigDict(extra="forbid", frozen=True) + + reachable: bool + + +class SimulationEntrypointReadinessResponse(BaseModel): + """Old-gateway readiness, plus the database check when one is configured.""" + + model_config = ConfigDict(extra="forbid", frozen=True) + + status: Literal["ready", "not_ready"] + simulation_database: SimulationDatabaseReadiness | None = None diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py new file mode 100644 index 000000000..1fe2047eb --- /dev/null +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py @@ -0,0 +1,40 @@ +"""The simulation service's connection to the Public API's database.""" + +from __future__ import annotations + +import asyncio +from typing import TYPE_CHECKING + +from policyengine_simulation_persistence import create_simulation_engine +from sqlalchemy import select + +if TYPE_CHECKING: + from sqlalchemy import Engine + + +class SimulationDatabase: + """Run short, blocking database operations off the event loop.""" + + def __init__(self, engine: Engine) -> None: + """Use ``engine``, which connects as the simulation service's role.""" + self.engine = engine + + @classmethod + def from_url(cls, database_url: str) -> SimulationDatabase: + """Build the runtime engine; no connection opens until first use. + + Raise ValueError, without echoing the URL, when it is malformed. + """ + return cls(create_simulation_engine(database_url)) + + def _check_connection(self) -> None: + with self.engine.connect() as connection: + connection.execute(select(1)) + + async def check_connection(self) -> None: + """Raise unless a pooled connection answers a trivial query.""" + await asyncio.to_thread(self._check_connection) + + def dispose(self) -> None: + """Close every pooled connection.""" + self.engine.dispose() diff --git a/projects/policyengine-simulation-entry/tests/test_config.py b/projects/policyengine-simulation-entry/tests/test_config.py index 8dbf6809f..171b00ebc 100644 --- a/projects/policyengine-simulation-entry/tests/test_config.py +++ b/projects/policyengine-simulation-entry/tests/test_config.py @@ -297,3 +297,41 @@ def test_validation_reports_the_first_failed_check(overrides, message): ) def test_consistent_configuration_validates(overrides): make_settings(**overrides).validate() + + +def test_the_simulation_database_is_optional(monkeypatch): + from policyengine_simulation_entry.config import Settings + + monkeypatch.delenv("SIMULATION_DATABASE_URL", raising=False) + settings = Settings.from_env() + + assert settings.simulation_database_url == "" + make_settings(simulation_database_url="").validate() + + +def test_the_simulation_database_url_is_read_from_the_environment(monkeypatch): + from policyengine_simulation_entry.config import Settings + + monkeypatch.setenv( + "SIMULATION_DATABASE_URL", + "postgresql://simulation_service:secret@db.example:5432/postgres", + ) + + assert Settings.from_env().simulation_database_url == ( + "postgresql://simulation_service:secret@db.example:5432/postgres" + ) + + +@pytest.mark.parametrize( + "database_url", + [ + "postgres://simulation_service:secret@db.example/postgres", + "postgresql+psycopg://simulation_service:secret@db.example/postgres", + "mysql://simulation_service:secret@db.example/postgres", + ], +) +def test_the_simulation_database_url_must_be_canonical(database_url): + with pytest.raises(ConfigurationError, match="SIMULATION_DATABASE_URL") as error: + make_settings(simulation_database_url=database_url).validate() + + assert "secret" not in str(error.value) diff --git a/projects/policyengine-simulation-entry/tests/test_deployment_assets_traffic.py b/projects/policyengine-simulation-entry/tests/test_deployment_assets_traffic.py index cda498290..68508c896 100644 --- a/projects/policyengine-simulation-entry/tests/test_deployment_assets_traffic.py +++ b/projects/policyengine-simulation-entry/tests/test_deployment_assets_traffic.py @@ -4,6 +4,7 @@ import os import textwrap +from pathlib import Path import tomllib from test_deployment_assets_support import ( @@ -172,6 +173,28 @@ def test_container_context_excludes_local_environments_and_unrelated_projects(): ) +def test_container_context_includes_every_local_library_the_entrypoint_installs(): + ignore_rules = DOCKERIGNORE.read_text(encoding="utf-8").splitlines() + pyproject = tomllib.loads( + ( + REPOSITORY_ROOT + / "projects" + / "policyengine-simulation-entry" + / "pyproject.toml" + ).read_text(encoding="utf-8") + ) + library_paths = [ + Path(source["path"]).name + for source in pyproject["tool"]["uv"]["sources"].values() + if "path" in source + ] + + assert library_paths + for library in library_paths: + assert f"!libs/{library}/" in ignore_rules, library + assert f"!libs/{library}/**" in ignore_rules, library + + def test_production_lock_contains_the_direct_stage12_database_runtime(): lockfile = tomllib.loads( ( diff --git a/projects/policyengine-simulation-entry/tests/test_openapi.py b/projects/policyengine-simulation-entry/tests/test_openapi.py index b31f97ada..a01f546d3 100644 --- a/projects/policyengine-simulation-entry/tests/test_openapi.py +++ b/projects/policyengine-simulation-entry/tests/test_openapi.py @@ -136,6 +136,18 @@ def test_local_area_breakdowns_may_list_the_withheld_areas_of_each_level(): assert field not in breakdown["required"] +def test_readiness_documents_whether_the_simulation_database_was_reached(): + spec = create_app().openapi() + readiness = spec["paths"]["/ready"]["get"]["responses"]["200"] + schema = readiness["content"]["application/json"]["schema"] + database = spec["components"]["schemas"]["SimulationDatabaseReadiness"] + + assert schema == { + "$ref": "#/components/schemas/SimulationEntrypointReadinessResponse" + } + assert set(database["properties"]) == {"reachable"} + + def test_route_table_is_frozen(): assert methods(cast("OpenAPIDocument", create_app().openapi())) == EXPECTED_ROUTES @@ -175,7 +187,9 @@ def test_normalized_contract_matches_old_gateway(): cloud_run_spec ) == normalized_compatibility_paths(gateway_spec) cloud_run_schemas = deepcopy(cloud_run_spec["components"]["schemas"]) - cloud_run_schemas.pop("ReadinessResponse") + # The entrypoint's readiness response adds its own database's revision. + cloud_run_schemas.pop("SimulationEntrypointReadinessResponse") + cloud_run_schemas.pop("SimulationDatabaseReadiness") cloud_run_schemas.pop("SimulationErrorResponse") assert cloud_run_schemas == gateway_spec["components"]["schemas"] gateway_operation_ids = operation_ids(gateway_spec) diff --git a/projects/policyengine-simulation-entry/tests/test_simulation_database.py b/projects/policyengine-simulation-entry/tests/test_simulation_database.py new file mode 100644 index 000000000..191ebaf76 --- /dev/null +++ b/projects/policyengine-simulation-entry/tests/test_simulation_database.py @@ -0,0 +1,186 @@ +"""The simulation service's database: readiness reporting and lifetime.""" + +from __future__ import annotations + +import asyncio +import threading + +import pytest +from conftest import FakeBackend, make_settings +from fastapi.testclient import TestClient +from policyengine_simulation_entry import app as app_module +from policyengine_simulation_entry.app import create_app +from policyengine_simulation_entry.simulation_database import SimulationDatabase +from sqlalchemy.exc import OperationalError + +DATABASE_URL = "postgresql://simulation_service:secret@db.example:5432/postgres" + + +class FakeSimulationDatabase: + """Answer the connection check, or fail the way an unreachable database does.""" + + def __init__(self, *, failure=None, delay_seconds=0): + self.failure = failure + self.delay_seconds = delay_seconds + self.checks = 0 + self.disposed = False + + async def check_connection(self): + self.checks += 1 + if self.delay_seconds: + await asyncio.sleep(self.delay_seconds) + if self.failure is not None: + raise self.failure + + def dispose(self): + self.disposed = True + + +def _client(backend, database): + return TestClient( + create_app( + settings=make_settings(simulation_database_url=DATABASE_URL), + backend=backend, + auth_dependency=lambda: None, + simulation_database=database, + ) + ) + + +def test_readiness_reports_that_the_database_was_reached(): + database = FakeSimulationDatabase() + with _client(FakeBackend(), database) as client: + result = client.get("/ready") + + assert result.status_code == 200 + assert result.json() == { + "status": "ready", + "simulation_database": {"reachable": True}, + } + assert database.checks == 1 + + +def test_an_unreachable_database_does_not_change_old_gateway_readiness(caplog): + failure = OperationalError("SELECT 1", {}, Exception("password=secret")) + with _client(FakeBackend(), FakeSimulationDatabase(failure=failure)) as client: + result = client.get("/ready") + + assert result.status_code == 200 + assert result.json() == { + "status": "ready", + "simulation_database": {"reachable": False}, + } + assert "simulation_database_unreachable" in caplog.text + assert "secret" not in caplog.text + + +def test_readiness_waits_only_briefly_for_the_database(monkeypatch): + monkeypatch.setattr( + app_module, "SIMULATION_DATABASE_READINESS_TIMEOUT_SECONDS", 0.01 + ) + slow_database = FakeSimulationDatabase(delay_seconds=1) + with _client(FakeBackend(), slow_database) as client: + database = client.get("/ready").json()["simulation_database"] + + assert database == {"reachable": False} + + +def test_readiness_still_follows_the_old_gateway(): + backend = FakeBackend() + backend.is_ready = False + with _client(backend, FakeSimulationDatabase()) as client: + result = client.get("/ready") + + assert result.status_code == 503 + assert result.json() == { + "status": "not_ready", + "simulation_database": {"reachable": True}, + } + + +def test_readiness_keeps_its_old_shape_without_a_database(): + with TestClient( + create_app( + settings=make_settings(), + backend=FakeBackend(), + auth_dependency=lambda: None, + ) + ) as client: + result = client.get("/ready") + + assert result.status_code == 200 + assert result.json() == {"status": "ready"} + + +def test_the_database_is_released_at_shutdown(): + database = FakeSimulationDatabase() + with _client(FakeBackend(), database): + assert database.disposed is False + + assert database.disposed is True + + +def test_settings_build_the_database_only_when_it_is_configured(monkeypatch): + built_urls = [] + + def build(database_url): + built_urls.append(database_url) + return FakeSimulationDatabase() + + monkeypatch.setattr(SimulationDatabase, "from_url", staticmethod(build)) + + create_app(settings=make_settings(), backend=FakeBackend()) + assert built_urls == [] + + create_app( + settings=make_settings(simulation_database_url=DATABASE_URL), + backend=FakeBackend(), + ) + assert built_urls == [DATABASE_URL] + + +def test_the_database_checks_its_connection_off_the_event_loop(): + statements = [] + statement_threads = [] + + class FakeConnection: + def __enter__(self): + return self + + def __exit__(self, *_): + return False + + def execute(self, statement): + statements.append(str(statement)) + statement_threads.append(threading.get_ident()) + + class FakeEngine: + disposed = False + + def connect(self): + return FakeConnection() + + def dispose(self): + self.disposed = True + + engine = FakeEngine() + database = SimulationDatabase(engine) + + async def check_from_the_event_loop(): + await database.check_connection() + return threading.get_ident() + + event_loop_thread = asyncio.run(check_from_the_event_loop()) + database.dispose() + + assert statements == ["SELECT 1"] + assert statement_threads != [event_loop_thread] + assert engine.disposed is True + + +@pytest.mark.parametrize("database_url", ["", "postgres://user:secret@db/postgres"]) +def test_a_malformed_database_url_never_builds_an_engine(database_url): + with pytest.raises(ValueError, match="Simulation database URL") as error: + SimulationDatabase.from_url(database_url) + + assert "secret" not in str(error.value) diff --git a/projects/policyengine-simulation-entry/uv.lock b/projects/policyengine-simulation-entry/uv.lock index e9a587c24..afff6f7a7 100644 --- a/projects/policyengine-simulation-entry/uv.lock +++ b/projects/policyengine-simulation-entry/uv.lock @@ -901,6 +901,7 @@ dependencies = [ { name = "policyengine-observability", extra = ["fastapi", "google", "httpx", "otlp-grpc"] }, { name = "policyengine-simulation-contract" }, { name = "policyengine-simulation-observability" }, + { name = "policyengine-simulation-persistence" }, { name = "policyengine-stage12-persistence" }, { name = "pyjwt" }, { name = "uvicorn" }, @@ -928,6 +929,7 @@ requires-dist = [ { name = "policyengine-observability", extras = ["fastapi", "google", "httpx", "otlp-grpc"], specifier = ">=3.0.4,<4" }, { name = "policyengine-simulation-contract", editable = "../../libs/policyengine-simulation-contract" }, { name = "policyengine-simulation-observability", editable = "../../libs/policyengine-simulation-observability" }, + { name = "policyengine-simulation-persistence", editable = "../../libs/policyengine-simulation-persistence" }, { name = "policyengine-stage12-persistence", editable = "../../libs/policyengine-stage12-persistence" }, { name = "pyjwt", specifier = ">=2.10.1,<3" }, { name = "pyright", marker = "extra == 'build'", specifier = ">=1.1.401" }, @@ -963,6 +965,25 @@ requires-dist = [ ] provides-extras = ["test", "build"] +[[package]] +name = "policyengine-simulation-persistence" +version = "0.1.0" +source = { editable = "../../libs/policyengine-simulation-persistence" } +dependencies = [ + { name = "psycopg", extra = ["binary"] }, + { name = "sqlalchemy" }, +] + +[package.metadata] +requires-dist = [ + { name = "psycopg", extras = ["binary"], specifier = ">=3.2,<4" }, + { name = "pyright", marker = "extra == 'build'", specifier = ">=1.1.401" }, + { name = "pytest", marker = "extra == 'test'", specifier = ">=8.3.4" }, + { name = "pytest-cov", marker = "extra == 'test'", specifier = ">=6.1.1" }, + { name = "sqlalchemy", specifier = ">=2,<3" }, +] +provides-extras = ["test", "build"] + [[package]] name = "policyengine-stage12-persistence" version = "0.1.0" diff --git a/rules/registry.toml b/rules/registry.toml index 289f51b83..b280fd7c3 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -591,9 +591,9 @@ test = "tests/test_release_workflow.py::test_rule_proof_release_commit_deploys_o [[rule]] id = "sim.deploy.container-context-minimal" category = "deployment" -statement = "Container builds exclude local environments and unrelated projects, and the entrypoint container builds for the requested target platform." +statement = "Container builds exclude local environments and unrelated projects but include every local library the entrypoint installs, and the entrypoint container builds for the requested target platform." rationale = "docs/operations" -scope = ["projects/policyengine-simulation-entry/Dockerfile"] +scope = ["projects/policyengine-simulation-entry/Dockerfile", "projects/policyengine-simulation-entry/Dockerfile.dockerignore"] tier = "mechanical" status = "enforced" autofix = false @@ -602,6 +602,7 @@ kind = "pytest" project = "projects/policyengine-simulation-entry" nodes = [ "tests/test_deployment_assets_traffic.py::test_container_context_excludes_local_environments_and_unrelated_projects", + "tests/test_deployment_assets_traffic.py::test_container_context_includes_every_local_library_the_entrypoint_installs", "tests/test_deployment_assets_cloud_run.py::test_entrypoint_container_uses_requested_target_platform", ] [[rule.ci]] From 158bdc2c1a743a851b0b5fd2a9efa9edbf5f526f Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:01:22 +0400 Subject: [PATCH 03/25] Stop the entrypoint deploy when any runtime secret cannot be resolved The deploy script built its Cloud Run secret list in array assignments with one command substitution per secret. Bash gives such an assignment the exit status of its last substitution only, so in the Stage 12 block a failure to resolve either Modal token secret (an invalid name, a failed version listing, or no enabled version) did not stop the script, and it deployed with an empty secret reference. Each secret is now resolved in its own assignment, so set -e stops the script at the first failure, before gcloud run deploy runs: the main path's secrets through add_runtime_secret, and the Stage 12 comparison path's through add_stage12_runtime_secret, its own copy with the same behaviour, so it shares no deploy helper with the main path and is removed with it. A test covers every runtime secret (both Modal token secrets deployed before this change), and another checks that each block uses only its own helper. Both are registered under sim.deploy.workflow-shell-safety. Co-Authored-By: Claude Opus 5.5 --- .../deploy-cloud-run-simulation-entry.sh | 36 ++++++++++----- docs/generated/rules.md | 2 +- .../tests/test_deployment_assets_cloud_run.py | 44 +++++++++++++++++++ rules/registry.toml | 6 ++- 4 files changed, 75 insertions(+), 13 deletions(-) diff --git a/.github/scripts/deploy-cloud-run-simulation-entry.sh b/.github/scripts/deploy-cloud-run-simulation-entry.sh index 4320514ff..29d1ab9af 100755 --- a/.github/scripts/deploy-cloud-run-simulation-entry.sh +++ b/.github/scripts/deploy-cloud-run-simulation-entry.sh @@ -58,6 +58,27 @@ resolve_secret_reference() { printf '%s:%s' "${reference}" "${highest_enabled_version}" } +runtime_secrets=() + +# Resolve one secret per assignment: an array assignment takes its exit status +# from its last command substitution only, so resolving several secrets in +# one would let an earlier failure pass unnoticed. +add_runtime_secret() { + local variable_name="$1" + local resolved_reference + resolved_reference="$(resolve_secret_reference "$2")" + runtime_secrets+=("${variable_name}=${resolved_reference}") +} + +# The Stage 12 comparison path's own copy, so it shares no deploy helper with +# the main path and is removed with it. +add_stage12_runtime_secret() { + local variable_name="$1" + local resolved_reference + resolved_reference="$(resolve_secret_reference "$2")" + runtime_secrets+=("${variable_name}=${resolved_reference}") +} + required_variables=( PROJECT_ID REGION @@ -115,11 +136,8 @@ export SIMULATION_ENTRYPOINT_ENFORCE_PERMISSIONS_VALUE="${SIMULATION_ENTRYPOINT_ [[ "${SIMULATION_ENTRYPOINT_ENFORCE_PERMISSIONS_VALUE}" == "0" || "${SIMULATION_ENTRYPOINT_ENFORCE_PERMISSIONS_VALUE}" == "1" ]] \ || fail "SIMULATION_ENTRYPOINT_ENFORCE_PERMISSIONS_VALUE must be 0 or 1" -runtime_secrets=( - "OLD_GATEWAY_AUTH_CLIENT_SECRET=$( - resolve_secret_reference "${OLD_GATEWAY_AUTH_CLIENT_SECRET_SECRET_NAME}" - )" -) +add_runtime_secret OLD_GATEWAY_AUTH_CLIENT_SECRET \ + "${OLD_GATEWAY_AUTH_CLIENT_SECRET_SECRET_NAME}" if [[ "${DEPLOY_STAGE12_V2}" == "true" ]]; then stage12_variables=( @@ -138,11 +156,9 @@ if [[ "${DEPLOY_STAGE12_V2}" == "true" ]]; then || fail "STAGE12_ARTIFACT_BUCKET_VALUE is invalid" validate_secret_reference "${STAGE12_DATABASE_URL_SECRET_NAME}" - runtime_secrets+=( - "MODAL_TOKEN_ID=$(resolve_secret_reference "${MODAL_TOKEN_ID_SECRET_NAME}")" - "MODAL_TOKEN_SECRET=$(resolve_secret_reference "${MODAL_TOKEN_SECRET_SECRET_NAME}")" - "STAGE12_DATABASE_URL=$(resolve_secret_reference "${STAGE12_DATABASE_URL_SECRET_NAME}")" - ) + add_stage12_runtime_secret MODAL_TOKEN_ID "${MODAL_TOKEN_ID_SECRET_NAME}" + add_stage12_runtime_secret MODAL_TOKEN_SECRET "${MODAL_TOKEN_SECRET_SECRET_NAME}" + add_stage12_runtime_secret STAGE12_DATABASE_URL "${STAGE12_DATABASE_URL_SECRET_NAME}" fi runtime_environment_file="${RUNNER_TEMP}/simulation-entry-runtime-environment.yaml" diff --git a/docs/generated/rules.md b/docs/generated/rules.md index f1e902e4a..d97cf45a2 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -158,7 +158,7 @@ flowchart LR | `sim.deploy.smoke-tests-strict` | Candidate and stable smoke tests reject a revision mismatch without retrying, and fail if the previous revision does not converge. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets.py` | `pr.yml:test` | **unproven** | | `sim.deploy.staging-promotes-tested-revision` | Every staging deployment passes promote_entrypoint: ${{ github.ref == 'refs/heads/main' }}, so a run from main moves the staging service's stable Cloud Run traffic to the entrypoint revision that passed the staging tests, and an accidental manual run from another branch neither moves that traffic nor deletes older revisions. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets_promotion.py` | `pr.yml:test` | yes | | `sim.deploy.traffic-changes-exact` | Traffic changes target an exact, validated revision, are reversible, and refuse to run over an intervening promotion. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets_traffic.py` | `pr.yml:test` | **unproven** | -| `sim.deploy.workflow-shell-safety` | Cloud Run deployment never interpolates GitHub expressions into shell, escapes environment values, and pins secret versions. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py` | `pr.yml:test` | **unproven** | +| `sim.deploy.workflow-shell-safety` | Cloud Run deployment never interpolates GitHub expressions into shell, escapes environment values, pins secret versions, stops before deploying when any runtime secret cannot be resolved, and resolves the Stage 12 secrets with the Stage 12 block's own helper. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py` | `pr.yml:test` | **unproven** | ```mermaid flowchart LR diff --git a/projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py b/projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py index 1052bfab5..66b171162 100644 --- a/projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py +++ b/projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py @@ -159,6 +159,50 @@ def test_cloud_run_deployment_escapes_environment_and_pins_secret_versions(tmp_p assert ":latest" not in secrets_argument +@pytest.mark.parametrize( + "secret_name_variable", + [ + "OLD_GATEWAY_AUTH_CLIENT_SECRET_SECRET_NAME", + "MODAL_TOKEN_ID_SECRET_NAME", + "MODAL_TOKEN_SECRET_SECRET_NAME", + "STAGE12_DATABASE_URL_SECRET_NAME", + ], +) +def test_cloud_run_deployment_stops_when_a_runtime_secret_cannot_be_resolved( + tmp_path, secret_name_variable +): + command_environment = _cloud_run_deploy_environment(tmp_path) | { + secret_name_variable: "not a secret name" + } + + result = subprocess.run( + [CLOUD_RUN_DEPLOY_SCRIPT], + check=False, + capture_output=True, + text=True, + env=command_environment, + ) + + assert result.returncode != 0 + assert "invalid Secret Manager reference" in result.stderr + assert not Path(command_environment["GCLOUD_ARGUMENTS_CAPTURE"]).exists() + + +def test_stage12_secrets_are_resolved_by_the_stage12_block_alone(): + """The comparison path shares no deploy helper with the main path.""" + script = Path(CLOUD_RUN_DEPLOY_SCRIPT).read_text(encoding="utf-8") + stage12_block = script.split('if [[ "${DEPLOY_STAGE12_V2}" == "true" ]]; then', 1)[ + 1 + ].split("\nfi\n", 1)[0] + main_path = script.replace(stage12_block, "") + + assert "add_stage12_runtime_secret STAGE12_DATABASE_URL" in stage12_block + assert "add_runtime_secret " not in stage12_block + assert "add_stage12_runtime_secret " not in main_path.replace( + "add_stage12_runtime_secret() {", "" + ) + + @pytest.mark.parametrize( ("configured", "expected"), [(None, "0"), ("", "0"), ("0", "0"), ("1", "1")], diff --git a/rules/registry.toml b/rules/registry.toml index b280fd7c3..0952b49a5 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -317,9 +317,9 @@ invocation = "uv run --frozen pytest tests/" [[rule]] id = "sim.deploy.workflow-shell-safety" category = "deployment" -statement = "Cloud Run deployment never interpolates GitHub expressions into shell, escapes environment values, and pins secret versions." +statement = "Cloud Run deployment never interpolates GitHub expressions into shell, escapes environment values, pins secret versions, stops before deploying when any runtime secret cannot be resolved, and resolves the Stage 12 secrets with the Stage 12 block's own helper." rationale = "docs/operations" -scope = [".github/workflows/*.yml"] +scope = [".github/workflows/*.yml", ".github/scripts/deploy-cloud-run-simulation-entry.sh"] tier = "mechanical" status = "enforced" autofix = false @@ -329,6 +329,8 @@ project = "projects/policyengine-simulation-entry" nodes = [ "tests/test_deployment_assets_cloud_run.py::test_cloud_run_deployment_does_not_interpolate_github_values_in_shell", "tests/test_deployment_assets_cloud_run.py::test_cloud_run_deployment_escapes_environment_and_pins_secret_versions", + "tests/test_deployment_assets_cloud_run.py::test_cloud_run_deployment_stops_when_a_runtime_secret_cannot_be_resolved", + "tests/test_deployment_assets_cloud_run.py::test_stage12_secrets_are_resolved_by_the_stage12_block_alone", ] [[rule.ci]] workflow = ".github/workflows/pr.yml" From 62326b5d55c0a903ecd4d31bbe2be9f8d8ae6bab Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:02:01 +0400 Subject: [PATCH 04/25] Prove the workflow-shell-safety and container-context-minimal rules This branch tightens both rules, which had no proof: - sim.deploy.workflow-shell-safety now also requires every runtime secret to be resolved in its own assignment. combined_secret_resolutions finds a Bash array assignment that resolves more than one secret, where an earlier failure would pass unnoticed. The deploy script passes it (a new test, registered under the rule); main's script before this branch fails it, and so does the proof's bad fixture. - sim.deploy.container-context-minimal now also requires every local library the entrypoint installs to be in its build context. libraries_missing_from_context compares a project's path sources with its .dockerignore; the existing test uses it, and the proof's bad fixture leaves one library out. Co-Authored-By: Claude Opus 5.5 --- docs/generated/rules.md | 6 +- .../tests/deployment_rule_checks.py | 55 +++++++++++++++++++ .../tests/test_deployment_assets_cloud_run.py | 26 +++++++++ .../tests/test_deployment_assets_traffic.py | 50 ++++++++++------- .../bad.dockerignore | 4 ++ .../good.dockerignore | 6 ++ .../pyproject.toml | 9 +++ .../bad_deploy_script.sh | 9 +++ .../good_deploy_script.sh | 16 ++++++ rules/registry.toml | 11 ++++ 10 files changed, 169 insertions(+), 23 deletions(-) create mode 100644 projects/policyengine-simulation-entry/tests/deployment_rule_checks.py create mode 100644 rules/fixtures/sim.deploy.container-context-minimal/bad.dockerignore create mode 100644 rules/fixtures/sim.deploy.container-context-minimal/good.dockerignore create mode 100644 rules/fixtures/sim.deploy.container-context-minimal/pyproject.toml create mode 100644 rules/fixtures/sim.deploy.workflow-shell-safety/bad_deploy_script.sh create mode 100644 rules/fixtures/sim.deploy.workflow-shell-safety/good_deploy_script.sh diff --git a/docs/generated/rules.md b/docs/generated/rules.md index d97cf45a2..ffa05c21c 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -42,7 +42,6 @@ Every rule that governs `policyengine-sim-api` is listed here with how it is enf - `sim.contracts.clients-publish-deployed-commit` - `sim.contracts.full-integration-beta-only` - `sim.contracts.gateway-openapi-golden` -- `sim.deploy.container-context-minimal` - `sim.deploy.gcloud-without-terraform` - `sim.deploy.numerical-sentinel-gate` - `sim.deploy.permission-enforcement-wired` @@ -53,7 +52,6 @@ Every rule that governs `policyengine-sim-api` is listed here with how it is enf - `sim.deploy.shell-scripts-valid` - `sim.deploy.smoke-tests-strict` - `sim.deploy.traffic-changes-exact` -- `sim.deploy.workflow-shell-safety` - `sim.process.rules-registry` - `sim.stage12.deployment-scope` - `sim.stage12.deployment-uses-api-schema` @@ -142,7 +140,7 @@ flowchart LR | Rule | Statement | Tier | Status | Blocking | Enforced by | CI | Proof | | --- | --- | --- | --- | --- | --- | --- | --- | -| `sim.deploy.container-context-minimal` | Container builds exclude local environments and unrelated projects but include every local library the entrypoint installs, and the entrypoint container builds for the requested target platform. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py`, `projects/policyengine-simulation-entry/tests/test_deployment_assets_traffic.py` | `pr.yml:test` | **unproven** | +| `sim.deploy.container-context-minimal` | Container builds exclude local environments and unrelated projects but include every local library the entrypoint installs, and the entrypoint container builds for the requested target platform. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py`, `projects/policyengine-simulation-entry/tests/test_deployment_assets_traffic.py` | `pr.yml:test` | yes | | `sim.deploy.docker-images-build` | The simulation entry and executor Docker images build. | mechanical | enforced | yes | CI job `docker-build` | `pr.yml:docker-build` | — | | `sim.deploy.gcloud-without-terraform` | Deployment uses the gcloud-based workflow, not Terraform. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets.py` | `pr.yml:test` | **unproven** | | `sim.deploy.image-smoke-covers-every-image-lib` | The image smoke path filter includes every library mounted into a Modal image, so a change to any of them runs the smoke. | mechanical | doc-only | no | nothing | — | — | @@ -158,7 +156,7 @@ flowchart LR | `sim.deploy.smoke-tests-strict` | Candidate and stable smoke tests reject a revision mismatch without retrying, and fail if the previous revision does not converge. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets.py` | `pr.yml:test` | **unproven** | | `sim.deploy.staging-promotes-tested-revision` | Every staging deployment passes promote_entrypoint: ${{ github.ref == 'refs/heads/main' }}, so a run from main moves the staging service's stable Cloud Run traffic to the entrypoint revision that passed the staging tests, and an accidental manual run from another branch neither moves that traffic nor deletes older revisions. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets_promotion.py` | `pr.yml:test` | yes | | `sim.deploy.traffic-changes-exact` | Traffic changes target an exact, validated revision, are reversible, and refuse to run over an intervening promotion. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets_traffic.py` | `pr.yml:test` | **unproven** | -| `sim.deploy.workflow-shell-safety` | Cloud Run deployment never interpolates GitHub expressions into shell, escapes environment values, pins secret versions, stops before deploying when any runtime secret cannot be resolved, and resolves the Stage 12 secrets with the Stage 12 block's own helper. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py` | `pr.yml:test` | **unproven** | +| `sim.deploy.workflow-shell-safety` | Cloud Run deployment never interpolates GitHub expressions into shell, escapes environment values, pins secret versions, stops before deploying when any runtime secret cannot be resolved, and resolves the Stage 12 secrets with the Stage 12 block's own helper. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py` | `pr.yml:test` | yes | ```mermaid flowchart LR diff --git a/projects/policyengine-simulation-entry/tests/deployment_rule_checks.py b/projects/policyengine-simulation-entry/tests/deployment_rule_checks.py new file mode 100644 index 000000000..fa06bed25 --- /dev/null +++ b/projects/policyengine-simulation-entry/tests/deployment_rule_checks.py @@ -0,0 +1,55 @@ +"""Checks behind two deployment rules, runnable on the repository or a fixture. + +- ``combined_secret_resolutions`` (sim.deploy.workflow-shell-safety): Bash + gives an array assignment the exit status of its last command substitution + only, so an assignment that resolves several secrets lets an earlier + failure pass unnoticed. +- ``libraries_missing_from_context`` (sim.deploy.container-context-minimal): + every local library the entrypoint installs must be in its build context. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import tomllib + +ARRAY_ASSIGNMENT = re.compile(r"^\s*\w+\+?=\(", re.MULTILINE) +# The line that closes a multi-line array assignment: only its parenthesis. +ARRAY_END = re.compile(r"\n[ \t]*\)") +SECRET_RESOLUTION = "$(resolve_secret_reference" + + +def combined_secret_resolutions(script_text: str) -> list[str]: + """Return array assignments that resolve more than one secret.""" + violations = [] + for match in ARRAY_ASSIGNMENT.finditer(script_text): + line_end = script_text.find("\n", match.end()) + first_line = script_text[match.start() : line_end if line_end >= 0 else None] + if ")" in first_line: + assignment = first_line + else: + closing = ARRAY_END.search(script_text, match.end()) + assignment = script_text[ + match.start() : closing.start() if closing is not None else None + ] + if assignment.count(SECRET_RESOLUTION) > 1: + violations.append(assignment.strip().splitlines()[0]) + return violations + + +def libraries_missing_from_context(pyproject_text: str, ignore_text: str) -> list[str]: + """Return the local libraries the project installs but its context leaves out.""" + rules = ignore_text.splitlines() + sources = ( + tomllib.loads(pyproject_text).get("tool", {}).get("uv", {}).get("sources", {}) + ) + libraries = [ + Path(source["path"]).name for source in sources.values() if "path" in source + ] + return [ + library + for library in libraries + if f"!libs/{library}/" not in rules or f"!libs/{library}/**" not in rules + ] diff --git a/projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py b/projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py index 66b171162..c3b557af1 100644 --- a/projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py +++ b/projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py @@ -10,10 +10,12 @@ from pathlib import Path import pytest +from deployment_rule_checks import SECRET_RESOLUTION, combined_secret_resolutions from test_deployment_assets_support import ( CLOUD_RUN_DEPLOY_SCRIPT, DEPLOY_WORKFLOW, DOCKERFILE, + REPOSITORY_ROOT, REUSABLE_DEPLOY_WORKFLOW, STAGE12_DEPLOY_WORKFLOW, ) @@ -188,6 +190,30 @@ def test_cloud_run_deployment_stops_when_a_runtime_secret_cannot_be_resolved( assert not Path(command_environment["GCLOUD_ARGUMENTS_CAPTURE"]).exists() +def test_cloud_run_deployment_resolves_each_secret_in_its_own_assignment(): + script = Path(CLOUD_RUN_DEPLOY_SCRIPT).read_text(encoding="utf-8") + + assert SECRET_RESOLUTION in script + assert combined_secret_resolutions(script) == [] + + +def test_rule_proof_workflow_shell_safety(): + """Proof for rule sim.deploy.workflow-shell-safety (rules/registry.toml).""" + fixtures = ( + REPOSITORY_ROOT / "rules" / "fixtures" / ("sim.deploy.workflow-shell-safety") + ) + + assert combined_secret_resolutions( + (fixtures / "bad_deploy_script.sh").read_text(encoding="utf-8") + ) == ["runtime_secrets=("] + assert ( + combined_secret_resolutions( + (fixtures / "good_deploy_script.sh").read_text(encoding="utf-8") + ) + == [] + ) + + def test_stage12_secrets_are_resolved_by_the_stage12_block_alone(): """The comparison path shares no deploy helper with the main path.""" script = Path(CLOUD_RUN_DEPLOY_SCRIPT).read_text(encoding="utf-8") diff --git a/projects/policyengine-simulation-entry/tests/test_deployment_assets_traffic.py b/projects/policyengine-simulation-entry/tests/test_deployment_assets_traffic.py index 68508c896..df2e4e669 100644 --- a/projects/policyengine-simulation-entry/tests/test_deployment_assets_traffic.py +++ b/projects/policyengine-simulation-entry/tests/test_deployment_assets_traffic.py @@ -4,9 +4,9 @@ import os import textwrap -from pathlib import Path import tomllib +from deployment_rule_checks import libraries_missing_from_context from test_deployment_assets_support import ( DOCKERIGNORE, GITIGNORE, @@ -174,25 +174,37 @@ def test_container_context_excludes_local_environments_and_unrelated_projects(): def test_container_context_includes_every_local_library_the_entrypoint_installs(): - ignore_rules = DOCKERIGNORE.read_text(encoding="utf-8").splitlines() - pyproject = tomllib.loads( - ( - REPOSITORY_ROOT - / "projects" - / "policyengine-simulation-entry" - / "pyproject.toml" - ).read_text(encoding="utf-8") + entry_project = REPOSITORY_ROOT / "projects" / "policyengine-simulation-entry" + pyproject = (entry_project / "pyproject.toml").read_text(encoding="utf-8") + + assert "path = " in pyproject + assert ( + libraries_missing_from_context( + pyproject, DOCKERIGNORE.read_text(encoding="utf-8") + ) + == [] + ) + + +def test_rule_proof_container_context_minimal(): + """Proof for rule sim.deploy.container-context-minimal (rules/registry.toml).""" + fixtures = ( + REPOSITORY_ROOT + / "rules" + / "fixtures" + / ("sim.deploy.container-context-minimal") + ) + pyproject = (fixtures / "pyproject.toml").read_text(encoding="utf-8") + + assert libraries_missing_from_context( + pyproject, (fixtures / "bad.dockerignore").read_text(encoding="utf-8") + ) == ["fixture-persistence"] + assert ( + libraries_missing_from_context( + pyproject, (fixtures / "good.dockerignore").read_text(encoding="utf-8") + ) + == [] ) - library_paths = [ - Path(source["path"]).name - for source in pyproject["tool"]["uv"]["sources"].values() - if "path" in source - ] - - assert library_paths - for library in library_paths: - assert f"!libs/{library}/" in ignore_rules, library - assert f"!libs/{library}/**" in ignore_rules, library def test_production_lock_contains_the_direct_stage12_database_runtime(): diff --git a/rules/fixtures/sim.deploy.container-context-minimal/bad.dockerignore b/rules/fixtures/sim.deploy.container-context-minimal/bad.dockerignore new file mode 100644 index 000000000..d8ed8cfbb --- /dev/null +++ b/rules/fixtures/sim.deploy.container-context-minimal/bad.dockerignore @@ -0,0 +1,4 @@ +# Proof fixture: the context leaves out one library the project installs. +libs/* +!libs/fixture-contract/ +!libs/fixture-contract/** diff --git a/rules/fixtures/sim.deploy.container-context-minimal/good.dockerignore b/rules/fixtures/sim.deploy.container-context-minimal/good.dockerignore new file mode 100644 index 000000000..d80779a02 --- /dev/null +++ b/rules/fixtures/sim.deploy.container-context-minimal/good.dockerignore @@ -0,0 +1,6 @@ +# Proof fixture: the context includes every library the project installs. +libs/* +!libs/fixture-contract/ +!libs/fixture-contract/** +!libs/fixture-persistence/ +!libs/fixture-persistence/** diff --git a/rules/fixtures/sim.deploy.container-context-minimal/pyproject.toml b/rules/fixtures/sim.deploy.container-context-minimal/pyproject.toml new file mode 100644 index 000000000..aa2c53cc7 --- /dev/null +++ b/rules/fixtures/sim.deploy.container-context-minimal/pyproject.toml @@ -0,0 +1,9 @@ +# Proof fixture: a project that installs two local libraries by path. +[project] +name = "fixture-entrypoint" +version = "0.1.0" +dependencies = ["fixture-contract", "fixture-persistence"] + +[tool.uv.sources] +fixture-contract = { path = "../../libs/fixture-contract", editable = true } +fixture-persistence = { path = "../../libs/fixture-persistence", editable = true } diff --git a/rules/fixtures/sim.deploy.workflow-shell-safety/bad_deploy_script.sh b/rules/fixtures/sim.deploy.workflow-shell-safety/bad_deploy_script.sh new file mode 100644 index 000000000..73d926095 --- /dev/null +++ b/rules/fixtures/sim.deploy.workflow-shell-safety/bad_deploy_script.sh @@ -0,0 +1,9 @@ +#!/usr/bin/env bash +# Proof fixture: one array assignment resolves two secrets, so a failure to +# resolve the first is ignored and the deploy goes on with an empty reference. +set -euo pipefail + +runtime_secrets=( + "OLD_GATEWAY_AUTH_CLIENT_SECRET=$(resolve_secret_reference "${OLD_GATEWAY_AUTH_CLIENT_SECRET_SECRET_NAME}")" + "SIMULATION_DATABASE_URL=$(resolve_secret_reference "${SIMULATION_DATABASE_URL_SECRET_NAME}")" +) diff --git a/rules/fixtures/sim.deploy.workflow-shell-safety/good_deploy_script.sh b/rules/fixtures/sim.deploy.workflow-shell-safety/good_deploy_script.sh new file mode 100644 index 000000000..6edd73590 --- /dev/null +++ b/rules/fixtures/sim.deploy.workflow-shell-safety/good_deploy_script.sh @@ -0,0 +1,16 @@ +#!/usr/bin/env bash +# Proof fixture: each secret is resolved in its own assignment, so set -e stops +# the script at the first failure. +set -euo pipefail + +runtime_secrets=() + +add_runtime_secret() { + local variable_name="$1" + local resolved_reference + resolved_reference="$(resolve_secret_reference "$2")" + runtime_secrets+=("${variable_name}=${resolved_reference}") +} + +add_runtime_secret OLD_GATEWAY_AUTH_CLIENT_SECRET "${OLD_GATEWAY_AUTH_CLIENT_SECRET_SECRET_NAME}" +add_runtime_secret SIMULATION_DATABASE_URL "${SIMULATION_DATABASE_URL_SECRET_NAME}" diff --git a/rules/registry.toml b/rules/registry.toml index 0952b49a5..1823474c8 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -330,12 +330,18 @@ nodes = [ "tests/test_deployment_assets_cloud_run.py::test_cloud_run_deployment_does_not_interpolate_github_values_in_shell", "tests/test_deployment_assets_cloud_run.py::test_cloud_run_deployment_escapes_environment_and_pins_secret_versions", "tests/test_deployment_assets_cloud_run.py::test_cloud_run_deployment_stops_when_a_runtime_secret_cannot_be_resolved", + "tests/test_deployment_assets_cloud_run.py::test_cloud_run_deployment_resolves_each_secret_in_its_own_assignment", "tests/test_deployment_assets_cloud_run.py::test_stage12_secrets_are_resolved_by_the_stage12_block_alone", ] [[rule.ci]] workflow = ".github/workflows/pr.yml" job = "test" invocation = "uv run --frozen pytest tests/" +[rule.proof] +bad_fixture = "rules/fixtures/sim.deploy.workflow-shell-safety/bad_deploy_script.sh" +good_fixture = "rules/fixtures/sim.deploy.workflow-shell-safety/good_deploy_script.sh" +project = "projects/policyengine-simulation-entry" +test = "tests/test_deployment_assets_cloud_run.py::test_rule_proof_workflow_shell_safety" [[rule]] id = "sim.deploy.revision-tags-valid" @@ -611,6 +617,11 @@ nodes = [ workflow = ".github/workflows/pr.yml" job = "test" invocation = "uv run --frozen pytest tests/" +[rule.proof] +bad_fixture = "rules/fixtures/sim.deploy.container-context-minimal/bad.dockerignore" +good_fixture = "rules/fixtures/sim.deploy.container-context-minimal/good.dockerignore" +project = "projects/policyengine-simulation-entry" +test = "tests/test_deployment_assets_traffic.py::test_rule_proof_container_context_minimal" [[rule]] id = "sim.deploy.docker-images-build" From 9d174398cb5fea6bd5a34da13dd9293fa0d54da7 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:04:07 +0400 Subject: [PATCH 05/25] Mount the simulation service's database URL on every entrypoint deploy The deploy script now requires SIMULATION_DATABASE_URL_SECRET_NAME (a GitHub environment variable in beta and prod, passed in the deploy step's env) and mounts that Secret Manager secret's newest enabled version as SIMULATION_DATABASE_URL on every deploy, whatever the Stage 12 setting. The secret holds the simulation service role's URL for the database that holds the Public API's tables. It is resolved like the other runtime secrets, in its own assignment, so a missing or invalid name, or a secret with no enabled version, stops the deploy before gcloud run deploy runs. The deployment document describes the wiring. Co-Authored-By: Claude Opus 5.5 --- .../deploy-cloud-run-simulation-entry.sh | 4 + .../workflows/simulation-deploy.reusable.yml | 1 + .../DEPLOYMENT.md | 17 ++++ .../tests/test_deployment_assets_cloud_run.py | 15 ++-- .../test_simulation_database_deployment.py | 88 +++++++++++++++++++ 5 files changed, 120 insertions(+), 5 deletions(-) create mode 100644 projects/policyengine-simulation-entry/tests/test_simulation_database_deployment.py diff --git a/.github/scripts/deploy-cloud-run-simulation-entry.sh b/.github/scripts/deploy-cloud-run-simulation-entry.sh index 29d1ab9af..57f8a80ab 100755 --- a/.github/scripts/deploy-cloud-run-simulation-entry.sh +++ b/.github/scripts/deploy-cloud-run-simulation-entry.sh @@ -98,6 +98,7 @@ required_variables=( OLD_GATEWAY_AUTH_AUDIENCE_VALUE OLD_GATEWAY_AUTH_CLIENT_ID_VALUE OLD_GATEWAY_AUTH_CLIENT_SECRET_SECRET_NAME + SIMULATION_DATABASE_URL_SECRET_NAME OBSERVABILITY_SERVICE_NAMESPACE OBSERVABILITY_TRACE_PROJECT_ID OTEL_EXPORTER_OTLP_ENDPOINT @@ -138,6 +139,9 @@ export SIMULATION_ENTRYPOINT_ENFORCE_PERMISSIONS_VALUE="${SIMULATION_ENTRYPOINT_ add_runtime_secret OLD_GATEWAY_AUTH_CLIENT_SECRET \ "${OLD_GATEWAY_AUTH_CLIENT_SECRET_SECRET_NAME}" +# The simulation service role's URL for the database that holds the Public +# API's tables, on every deploy whatever the Stage 12 setting. +add_runtime_secret SIMULATION_DATABASE_URL "${SIMULATION_DATABASE_URL_SECRET_NAME}" if [[ "${DEPLOY_STAGE12_V2}" == "true" ]]; then stage12_variables=( diff --git a/.github/workflows/simulation-deploy.reusable.yml b/.github/workflows/simulation-deploy.reusable.yml index 667b222db..7427ecfdc 100644 --- a/.github/workflows/simulation-deploy.reusable.yml +++ b/.github/workflows/simulation-deploy.reusable.yml @@ -241,6 +241,7 @@ jobs: OLD_GATEWAY_AUTH_AUDIENCE_VALUE: ${{ secrets.OLD_GATEWAY_AUTH_AUDIENCE }} OLD_GATEWAY_AUTH_CLIENT_ID_VALUE: ${{ secrets.OLD_GATEWAY_AUTH_CLIENT_ID }} OLD_GATEWAY_AUTH_CLIENT_SECRET_SECRET_NAME: ${{ vars.OLD_GATEWAY_AUTH_CLIENT_SECRET_SECRET_NAME }} + SIMULATION_DATABASE_URL_SECRET_NAME: ${{ vars.SIMULATION_DATABASE_URL_SECRET_NAME }} STAGE12_ENABLED_VALUE: ${{ vars.STAGE12_ENABLED }} STAGE12_ARTIFACT_BUCKET_VALUE: ${{ vars.STAGE12_ARTIFACT_BUCKET }} STAGE12_DATABASE_URL_SECRET_NAME: ${{ vars.STAGE12_DATABASE_URL_SECRET_NAME }} diff --git a/projects/policyengine-simulation-entry/DEPLOYMENT.md b/projects/policyengine-simulation-entry/DEPLOYMENT.md index 76038ce31..a064d8902 100644 --- a/projects/policyengine-simulation-entry/DEPLOYMENT.md +++ b/projects/policyengine-simulation-entry/DEPLOYMENT.md @@ -22,6 +22,23 @@ Credential values belong in the cloud secret store. They must not be committed to the repository or stored as plain GitHub variables. The deployment workflow receives only the configuration needed to reference those protected resources. +## Simulation database + +The simulation service connects to the Postgres database that holds the +Public API's tables, with its own login role. This repository has no schema +or migration chain of its own: the Public API's migration chain creates and +changes those tables, and the Public API's privilege manifest grants the +role what it may do on them. + +The deploy step resolves the Secret Manager secret named by the +`SIMULATION_DATABASE_URL_SECRET_NAME` environment variable to its newest +enabled version and mounts that version as `SIMULATION_DATABASE_URL` on every +deploy, whatever the Stage 12 setting. The deploy service account must be +able to list the secret's versions (`gcloud secrets versions list`), as for +the other runtime secrets, and only the entrypoint's runtime service account +may read it. A missing or invalid name, or a secret with no enabled version, +stops the deploy before `gcloud run deploy` runs. + ## Candidate qualification For each eligible revision, the workflow first deploys `beta` diff --git a/projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py b/projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py index c3b557af1..db1513b71 100644 --- a/projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py +++ b/projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py @@ -41,7 +41,7 @@ def test_cloud_run_deployment_does_not_interpolate_github_values_in_shell(): assert ":latest" not in deployment_script -def _cloud_run_deploy_environment(tmp_path: Path) -> dict[str, str]: +def cloud_run_deploy_environment(tmp_path: Path) -> dict[str, str]: """Return a complete deployment environment backed by a fake gcloud.""" fake_bin = tmp_path / "bin" fake_bin.mkdir() @@ -116,11 +116,14 @@ def _cloud_run_deploy_environment(tmp_path: Path) -> dict[str, str]: "STAGE12_DATABASE_URL_SECRET_NAME": "stage12-staging-database-url", "MODAL_TOKEN_ID_SECRET_NAME": "modal-token-id", "MODAL_TOKEN_SECRET_SECRET_NAME": "modal-token-secret", + "SIMULATION_DATABASE_URL_SECRET_NAME": ( + "simulation-staging-service-database-url" + ), } def test_cloud_run_deployment_escapes_environment_and_pins_secret_versions(tmp_path): - command_environment = _cloud_run_deploy_environment(tmp_path) + command_environment = cloud_run_deploy_environment(tmp_path) arguments_capture = Path(command_environment["GCLOUD_ARGUMENTS_CAPTURE"]) env_capture = Path(command_environment["GCLOUD_ENV_FILE_CAPTURE"]) @@ -154,6 +157,7 @@ def test_cloud_run_deployment_escapes_environment_and_pins_secret_versions(tmp_p secrets_argument = arguments[arguments.index("--set-secrets") + 1] assert secrets_argument == ( "OLD_GATEWAY_AUTH_CLIENT_SECRET=old-client-secret:3," + "SIMULATION_DATABASE_URL=simulation-staging-service-database-url:3," "MODAL_TOKEN_ID=modal-token-id:3," "MODAL_TOKEN_SECRET=modal-token-secret:3," "STAGE12_DATABASE_URL=stage12-staging-database-url:3" @@ -165,6 +169,7 @@ def test_cloud_run_deployment_escapes_environment_and_pins_secret_versions(tmp_p "secret_name_variable", [ "OLD_GATEWAY_AUTH_CLIENT_SECRET_SECRET_NAME", + "SIMULATION_DATABASE_URL_SECRET_NAME", "MODAL_TOKEN_ID_SECRET_NAME", "MODAL_TOKEN_SECRET_SECRET_NAME", "STAGE12_DATABASE_URL_SECRET_NAME", @@ -173,7 +178,7 @@ def test_cloud_run_deployment_escapes_environment_and_pins_secret_versions(tmp_p def test_cloud_run_deployment_stops_when_a_runtime_secret_cannot_be_resolved( tmp_path, secret_name_variable ): - command_environment = _cloud_run_deploy_environment(tmp_path) | { + command_environment = cloud_run_deploy_environment(tmp_path) | { secret_name_variable: "not a secret name" } @@ -239,7 +244,7 @@ def test_cloud_run_deployment_defaults_permission_enforcement_to_log_only( configured, expected, ): - command_environment = _cloud_run_deploy_environment(tmp_path) + command_environment = cloud_run_deploy_environment(tmp_path) if configured is not None: command_environment["SIMULATION_ENTRYPOINT_ENFORCE_PERMISSIONS_VALUE"] = ( configured @@ -262,7 +267,7 @@ def test_cloud_run_deployment_rejects_invalid_permission_enforcement( tmp_path, configured, ): - command_environment = _cloud_run_deploy_environment(tmp_path) + command_environment = cloud_run_deploy_environment(tmp_path) command_environment["SIMULATION_ENTRYPOINT_ENFORCE_PERMISSIONS_VALUE"] = configured result = subprocess.run( diff --git a/projects/policyengine-simulation-entry/tests/test_simulation_database_deployment.py b/projects/policyengine-simulation-entry/tests/test_simulation_database_deployment.py new file mode 100644 index 000000000..a6156f16c --- /dev/null +++ b/projects/policyengine-simulation-entry/tests/test_simulation_database_deployment.py @@ -0,0 +1,88 @@ +"""Deployment wiring for the simulation service's database. + +The entrypoint receives its role's URL as a pinned Cloud Run secret on every +deploy, whatever the Stage 12 setting. +""" + +from __future__ import annotations + +import json +import subprocess +from pathlib import Path + +import pytest +from test_deployment_assets_cloud_run import cloud_run_deploy_environment +from test_deployment_assets_support import ( + CLOUD_RUN_DEPLOY_SCRIPT, + REUSABLE_DEPLOY_WORKFLOW, + workflow_jobs, +) + + +def _run_deploy(tmp_path, **overrides): + environment = cloud_run_deploy_environment(tmp_path) | overrides + result = subprocess.run( + [CLOUD_RUN_DEPLOY_SCRIPT], + check=False, + capture_output=True, + text=True, + env=environment, + ) + return result, environment + + +def _secrets_argument(environment) -> str: + arguments = ( + Path(environment["GCLOUD_ARGUMENTS_CAPTURE"]) + .read_text(encoding="utf-8") + .splitlines() + ) + return arguments[arguments.index("--set-secrets") + 1] + + +@pytest.mark.parametrize("deploy_stage12", ["true", "false"]) +def test_the_entrypoint_receives_the_database_url_as_a_pinned_secret( + tmp_path, deploy_stage12 +): + result, environment = _run_deploy(tmp_path, DEPLOY_STAGE12_V2=deploy_stage12) + + assert result.returncode == 0, result.stderr + assert ( + "SIMULATION_DATABASE_URL=simulation-staging-service-database-url:3" + in _secrets_argument(environment).split(",") + ) + runtime_environment = json.loads( + Path(environment["GCLOUD_ENV_FILE_CAPTURE"]).read_text(encoding="utf-8") + ) + assert "SIMULATION_DATABASE_URL" not in runtime_environment + + +@pytest.mark.parametrize( + "secret_name", + ["", "not a secret name", "projects/x/secrets/name/versions/1"], +) +def test_the_entrypoint_deploy_requires_a_valid_database_url_secret( + tmp_path, secret_name +): + result, environment = _run_deploy( + tmp_path, SIMULATION_DATABASE_URL_SECRET_NAME=secret_name + ) + + assert result.returncode != 0 + assert "SIMULATION_DATABASE_URL_SECRET_NAME" in result.stderr or ( + "Secret Manager reference" in result.stderr + ) + assert not Path(environment["GCLOUD_ARGUMENTS_CAPTURE"]).exists() + + +def test_the_deploy_step_passes_the_database_url_secret_name(): + deploy_entrypoint = workflow_jobs(REUSABLE_DEPLOY_WORKFLOW)["deploy_entrypoint"] + deploy_step = next( + step + for step in deploy_entrypoint["steps"] + if step.get("name") == "Deploy Cloud Run entrypoint candidate" + ) + + assert deploy_step["env"]["SIMULATION_DATABASE_URL_SECRET_NAME"] == ( + "${{ vars.SIMULATION_DATABASE_URL_SECRET_NAME }}" + ) From 41e0989d088924f47d9d04e9c17fd94b9cb1b048 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:09:47 +0400 Subject: [PATCH 06/25] Name the table privileges the simulation service's role lacks in the readiness check The persistence library now lists the table privileges the simulation service's role needs on the Public API's tables (SERVICE_TABLE_PRIVILEGES): SELECT and INSERT on the tables it creates rows in (simulations, reports, simulation_runs, report_runs, report_run_simulation_runs and household_jobs), and SELECT on the policy, household and catalog tables a run reads. missing_table_privileges asks PostgreSQL, in one query, which of them the connected role lacks; a table missing from the public schema lacks all of its privileges, and a table of the same name in another schema on the search path doesn't count. GET /ready reports the result next to whether the database was reached: {"status": "ready", "simulation_database": {"reachable": true, "missing_privileges": []}} missing_privileges is null when the database was not reached. Neither field changes the readiness status. Those tests need PostgreSQL, which the Test job doesn't provide, so a new Simulation database job runs the library's postgres-marked tests on postgres:17 and fails rather than skips without the server. CI complete needs it. The new rule sim.persistence.missing-privileges-reported, in a new persistence category, records the check, with a proof. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/pr.yml | 52 ++++- AGENTS.md | 4 + docs/generated/rules.md | 19 +- .../README.md | 19 ++ .../pyproject.toml | 4 + .../__init__.py | 6 + .../access.py | 78 +++++++ .../tests/conftest.py | 87 ++++++++ .../tests/test_access.py | 210 ++++++++++++++++++ .../tests/test_database_job.py | 44 ++++ .../uv.lock | 20 ++ .../DEPLOYMENT.md | 6 + .../src/policyengine_simulation_entry/app.py | 16 +- .../entry_runtime.py | 4 +- .../policyengine_simulation_entry/schemas.py | 7 +- .../simulation_database.py | 16 +- .../tests/test_openapi.py | 4 +- .../tests/test_simulation_database.py | 66 ++++-- .../policyengine-simulation-entry/uv.lock | 1 + .../bad_grants.txt | 21 ++ .../good_grants.txt | 21 ++ rules/registry.toml | 34 +++ 22 files changed, 695 insertions(+), 44 deletions(-) create mode 100644 libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/access.py create mode 100644 libs/policyengine-simulation-persistence/tests/conftest.py create mode 100644 libs/policyengine-simulation-persistence/tests/test_access.py create mode 100644 libs/policyengine-simulation-persistence/tests/test_database_job.py create mode 100644 rules/fixtures/sim.persistence.missing-privileges-reported/bad_grants.txt create mode 100644 rules/fixtures/sim.persistence.missing-privileges-reported/good_grants.txt diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 3a2fc61a7..f828253a2 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -16,7 +16,7 @@ jobs: # every job passed. See docs/engineering/skills/github-prs.md. name: CI complete if: ${{ always() }} - needs: [test, lint, pyright, quality-guards, check-changelog, docker-build, test-integration] + needs: [test, simulation-database, lint, pyright, quality-guards, check-changelog, docker-build, test-integration] runs-on: ubuntu-latest permissions: contents: read @@ -77,6 +77,56 @@ jobs: working-directory: ${{ matrix.project }} run: uv run --frozen pytest tests/ -v + simulation-database: + name: Simulation database + # Runs the persistence tests that need PostgreSQL, which the Test job + # skips, on a fresh server. Full CI runs only on the lowest unmerged pull + # request in a stack. + if: ${{ !github.event.pull_request.stack || github.event.pull_request.stack.base.ref == github.event.pull_request.base.ref }} + runs-on: ubuntu-latest + permissions: + contents: read + services: + postgres: + image: postgres:17 + env: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: simulation_ci + POSTGRES_DB: postgres + ports: + - 5432:5432 + options: >- + --health-cmd="pg_isready -U postgres" + --health-interval=5s + --health-timeout=5s + --health-retries=20 + env: + SIMULATION_TEST_DATABASE_URL: postgresql://postgres:simulation_ci@127.0.0.1:5432/postgres + # A missing server fails the tests instead of skipping them. + SIMULATION_REQUIRE_DATABASE_TESTS: "1" + + steps: + - uses: actions/checkout@v6 + + - name: Set up Python + uses: actions/setup-python@v6 + with: + python-version: '3.13' + + - name: Install uv + uses: astral-sh/setup-uv@v8.1.0 + with: + version: "0.13.6" + enable-cache: true + + - name: Install dependencies + working-directory: libs/policyengine-simulation-persistence + run: uv sync --extra test --frozen + + - name: Run the persistence tests on PostgreSQL + working-directory: libs/policyengine-simulation-persistence + run: uv run --frozen pytest tests/ -v -m postgres + lint: name: Lint # Full CI runs only on the lowest unmerged pull request in a stack. diff --git a/AGENTS.md b/AGENTS.md index 40cf004b1..959f144ec 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -130,6 +130,10 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`. - Checked: `sim.stage12.deployment-scope`, `sim.stage12.deployment-uses-api-schema`, `sim.stage12.no-migration-chain`, `sim.stage12.no-production-user-associations`, `sim.stage12.no-schema-ddl`, `sim.stage12.persistence-typed-dml-only`, `sim.stage12.production-lock-has-db-runtime` +### Simulation service database + +- Checked: `sim.persistence.missing-privileges-reported` + ### Deployment pipeline - Checked: `sim.deploy.container-context-minimal`, `sim.deploy.docker-images-build`, `sim.deploy.gcloud-without-terraform`, `sim.deploy.modal-image-smoke`, `sim.deploy.numerical-sentinel-gate`, `sim.deploy.permission-enforcement-wired`, `sim.deploy.promotion-order-explicit`, `sim.deploy.release-commit-deploys-once`, `sim.deploy.release-identity-smoke`, `sim.deploy.revision-tags-valid`, `sim.deploy.routing-publication-journaled`, `sim.deploy.shell-scripts-valid`, `sim.deploy.smoke-tests-strict`, `sim.deploy.staging-promotes-tested-revision`, `sim.deploy.traffic-changes-exact`, `sim.deploy.workflow-shell-safety` diff --git a/docs/generated/rules.md b/docs/generated/rules.md index ffa05c21c..9c6628021 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -8,11 +8,11 @@ Every rule that governs `policyengine-sim-api` is listed here with how it is enf | Tier | enforced | ratcheted | advisory | doc-only | Total | | --- | ---: | ---: | ---: | ---: | ---: | -| mechanical | 57 | 2 | 0 | 14 | 73 | +| mechanical | 58 | 2 | 0 | 14 | 74 | | metric | 1 | 0 | 0 | 0 | 1 | | judgment | 0 | 0 | 0 | 0 | 0 | -**Blocking:** 60 of 74 rules. +**Blocking:** 61 of 75 rules. ## Gaps @@ -136,6 +136,21 @@ flowchart LR e_pytest_projects_policyengine_simulation_entry_tests_test_deployment_assets_traffic_py --> j_CI_pr_yml_test ``` +## Simulation service database + +| Rule | Statement | Tier | Status | Blocking | Enforced by | CI | Proof | +| --- | --- | --- | --- | --- | --- | --- | --- | +| `sim.persistence.missing-privileges-reported` | The simulation service's readiness check names every table privilege its role needs on the Public API's tables and lacks, as PostgreSQL reports it, and counts a table missing from the public schema as lacking all of them. | mechanical | enforced | yes | pytest `libs/policyengine-simulation-persistence/tests/test_access.py` | `pr.yml:simulation-database` | yes | + +```mermaid +flowchart LR + r_sim_persistence_missing_privileges_reported("sim.persistence.missing-privileges-reported (enforced)") + e_pytest_libs_policyengine_simulation_persistence_tests_test_access_py["pytest libs/policyengine-simulation-persistence/tests/test_access.py"] + j_CI_pr_yml_simulation_database["CI pr.yml:simulation-database"] + r_sim_persistence_missing_privileges_reported --> e_pytest_libs_policyengine_simulation_persistence_tests_test_access_py + e_pytest_libs_policyengine_simulation_persistence_tests_test_access_py --> j_CI_pr_yml_simulation_database +``` + ## Deployment pipeline | Rule | Statement | Tier | Status | Blocking | Enforced by | CI | Proof | diff --git a/libs/policyengine-simulation-persistence/README.md b/libs/policyengine-simulation-persistence/README.md index 30ab8eed7..3e29cb093 100644 --- a/libs/policyengine-simulation-persistence/README.md +++ b/libs/policyengine-simulation-persistence/README.md @@ -17,9 +17,28 @@ server-side statements, so the same URL works through Supabase's session and transaction poolers. A malformed URL raises `ValueError` without repeating the URL, which holds a password. +## The role's privileges + +`SERVICE_TABLE_PRIVILEGES` lists the table privileges the service's role +needs: `SELECT` and `INSERT` on the tables it creates rows in +(`simulations`, `reports`, `simulation_runs`, `report_runs`, +`report_run_simulation_runs` and `household_jobs`), and `SELECT` on the +policy, household and catalog tables a run reads. The Public API's privilege +manifest grants them. `missing_table_privileges` returns each one the +connected role lacks, as `"INSERT on reports"`; a table missing from the +`public` schema lacks all of its privileges. The Simulation Entrypoint's +readiness check reports the result. + ## Tests ```bash uv sync --extra test uv run pytest tests/ ``` + +Tests marked `postgres` need a PostgreSQL server: set +`SIMULATION_TEST_DATABASE_URL` to its superuser URL, such as +`postgresql://postgres:password@127.0.0.1:5432/postgres`. Each test creates and +drops its own database. Without the variable they skip; the PR workflow's +`Simulation database` job sets `SIMULATION_REQUIRE_DATABASE_TESTS=1`, so a +missing server fails them there. diff --git a/libs/policyengine-simulation-persistence/pyproject.toml b/libs/policyengine-simulation-persistence/pyproject.toml index 96ddf7231..ed64ed0f9 100644 --- a/libs/policyengine-simulation-persistence/pyproject.toml +++ b/libs/policyengine-simulation-persistence/pyproject.toml @@ -16,6 +16,7 @@ dependencies = [ test = [ "pytest>=8.3.4", "pytest-cov>=6.1.1", + "pyyaml>=6.0.2", ] build = [ "pyright>=1.1.401", @@ -33,6 +34,9 @@ packages = ["src/policyengine_simulation_persistence"] [tool.pytest.ini_options] pythonpath = ["src"] testpaths = ["tests"] +markers = [ + "postgres: needs a disposable PostgreSQL server named by SIMULATION_TEST_DATABASE_URL", +] [tool.pyright] # The pr.yml pyright job checks these paths: python -m scripts.rules pyright. diff --git a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/__init__.py b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/__init__.py index bab08ce04..9d338640b 100644 --- a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/__init__.py +++ b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/__init__.py @@ -1,11 +1,17 @@ """Connections to the Public API's database for the simulation service.""" +from policyengine_simulation_persistence.access import ( + SERVICE_TABLE_PRIVILEGES, + missing_table_privileges, +) from policyengine_simulation_persistence.engine import ( create_simulation_engine, normalized_database_url, ) __all__ = [ + "SERVICE_TABLE_PRIVILEGES", "create_simulation_engine", + "missing_table_privileges", "normalized_database_url", ] diff --git a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/access.py b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/access.py new file mode 100644 index 000000000..ac179f444 --- /dev/null +++ b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/access.py @@ -0,0 +1,78 @@ +"""The privileges the simulation service's role needs on the Public API's tables. + +The Public API's privilege manifest grants the role these privileges, and +its migration chain creates the tables. The readiness check reports any the +connected role lacks, which shows that the deployment's database URL and the +role's grants both reached the running service. +""" + +from __future__ import annotations + +from types import MappingProxyType +from typing import TYPE_CHECKING, Final + +from sqlalchemy import String, column, false, func, select, values + +if TYPE_CHECKING: + from collections.abc import Mapping + + from sqlalchemy import Connection + +TABLE_SCHEMA: Final = "public" +_CREATE: Final = frozenset({"SELECT", "INSERT"}) +_READ: Final = frozenset({"SELECT"}) + +# The service creates simulations, reports, their runs and household jobs, and +# reads the policy, household and catalog rows a run needs. +SERVICE_TABLE_PRIVILEGES: Final[Mapping[str, frozenset[str]]] = MappingProxyType( + { + "simulations": _CREATE, + "reports": _CREATE, + "simulation_runs": _CREATE, + "report_runs": _CREATE, + "report_run_simulation_runs": _CREATE, + "household_jobs": _CREATE, + "policies": _READ, + "households": _READ, + "tax_benefit_models": _READ, + "tax_benefit_model_versions": _READ, + "regions": _READ, + "datasets": _READ, + "parameters": _READ, + "parameter_values": _READ, + } +) + + +def missing_table_privileges(connection: Connection) -> list[str]: + """Return each required privilege the connected role lacks, ordered by table. + + Each is named as ``" on "``. A table missing from the + ``public`` schema lacks every privilege the service needs on it. + """ + required = values( + column("table_name", String), + column("privilege", String), + name="required_privileges", + ).data( + [ + (table_name, privilege) + for table_name, privileges in SERVICE_TABLE_PRIVILEGES.items() + for privilege in sorted(privileges) + ] + ) + # to_regclass returns null for a missing table, and has_table_privilege + # returns null for a null table, so a missing table counts as not granted. + granted = func.coalesce( + func.has_table_privilege( + func.to_regclass(func.concat(f"{TABLE_SCHEMA}.", required.c.table_name)), + required.c.privilege, + ), + false(), + ) + rows = connection.execute( + select(required.c.table_name, required.c.privilege) + .where(~granted) + .order_by(required.c.table_name, required.c.privilege) + ) + return [f"{privilege} on {table_name}" for table_name, privilege in rows] diff --git a/libs/policyengine-simulation-persistence/tests/conftest.py b/libs/policyengine-simulation-persistence/tests/conftest.py new file mode 100644 index 000000000..bac706e37 --- /dev/null +++ b/libs/policyengine-simulation-persistence/tests/conftest.py @@ -0,0 +1,87 @@ +"""Disposable PostgreSQL databases for the tests that need one. + +Tests marked ``postgres`` need a PostgreSQL server whose superuser URL is in +``SIMULATION_TEST_DATABASE_URL``. Each test gets a new, empty database and a +login role that holds no privilege on it until the test grants one. Without +the variable those tests skip, unless ``SIMULATION_REQUIRE_DATABASE_TESTS=1`` +(set in CI), where they fail instead. +""" + +from __future__ import annotations + +import os +from dataclasses import dataclass +from typing import TYPE_CHECKING +from uuid import uuid4 + +import pytest +from sqlalchemy import create_engine, text +from sqlalchemy.engine import URL, make_url + +if TYPE_CHECKING: + from collections.abc import Iterator + +DATABASE_URL_VARIABLE = "SIMULATION_TEST_DATABASE_URL" +REQUIRE_DATABASE_VARIABLE = "SIMULATION_REQUIRE_DATABASE_TESTS" +# A login role standing in for the simulation service's role. +SERVICE_TEST_ROLE = "simulation_service_test" +SERVICE_TEST_ROLE_PASSWORD = "simulation_service_test" + + +@dataclass(frozen=True) +class DisposableDatabase: + """One empty database and the URLs of the roles that use it.""" + + name: str + superuser_url: str + service_url: str + + +def _canonical(url: URL) -> str: + return url.set(drivername="postgresql").render_as_string(hide_password=False) + + +@pytest.fixture(scope="session") +def postgres_server_url() -> URL: + """Return the superuser URL of the test server, or skip without one.""" + configured = os.environ.get(DATABASE_URL_VARIABLE, "") + if not configured: + message = f"{DATABASE_URL_VARIABLE} names no PostgreSQL server" + if os.environ.get(REQUIRE_DATABASE_VARIABLE) == "1": + pytest.fail(message) + pytest.skip(message) + return make_url(configured).set(drivername="postgresql+psycopg") + + +@pytest.fixture +def disposable_database(postgres_server_url: URL) -> Iterator[DisposableDatabase]: + """Create an empty database and the service's test role; drop the database after.""" + database_name = f"simulation_test_{uuid4().hex[:12]}" + server_engine = create_engine(postgres_server_url, isolation_level="AUTOCOMMIT") + try: + with server_engine.connect() as connection: + connection.execute( + text( + "DO $$ BEGIN IF NOT EXISTS (SELECT FROM pg_roles WHERE rolname = " + f"'{SERVICE_TEST_ROLE}') THEN CREATE ROLE {SERVICE_TEST_ROLE} " + f"LOGIN PASSWORD '{SERVICE_TEST_ROLE_PASSWORD}'; END IF; END $$" + ) + ) + connection.execute(text(f'CREATE DATABASE "{database_name}"')) + yield DisposableDatabase( + name=database_name, + superuser_url=_canonical(postgres_server_url.set(database=database_name)), + service_url=_canonical( + postgres_server_url.set( + username=SERVICE_TEST_ROLE, + password=SERVICE_TEST_ROLE_PASSWORD, + database=database_name, + ) + ), + ) + finally: + with server_engine.connect() as connection: + connection.execute( + text(f'DROP DATABASE IF EXISTS "{database_name}" WITH (FORCE)') + ) + server_engine.dispose() diff --git a/libs/policyengine-simulation-persistence/tests/test_access.py b/libs/policyengine-simulation-persistence/tests/test_access.py new file mode 100644 index 000000000..62054a1a3 --- /dev/null +++ b/libs/policyengine-simulation-persistence/tests/test_access.py @@ -0,0 +1,210 @@ +"""The privileges the simulation service's role needs on the Public API's tables.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +from conftest import SERVICE_TEST_ROLE +from policyengine_simulation_persistence import ( + SERVICE_TABLE_PRIVILEGES, + create_simulation_engine, + missing_table_privileges, +) +from sqlalchemy import create_engine, text +from sqlalchemy.engine import make_url + +PROOF_FIXTURES = ( + Path(__file__).resolve().parents[3] + / "rules" + / "fixtures" + / "sim.persistence.missing-privileges-reported" +) +WRITTEN_TABLES = ( + "simulations", + "reports", + "simulation_runs", + "report_runs", + "report_run_simulation_runs", + "household_jobs", +) +READ_TABLES = ( + "policies", + "households", + "tax_benefit_models", + "tax_benefit_model_versions", + "regions", + "datasets", + "parameters", + "parameter_values", +) + + +def test_the_service_may_create_and_read_rows_in_every_table_it_writes() -> None: + for table_name in WRITTEN_TABLES: + assert SERVICE_TABLE_PRIVILEGES[table_name] == frozenset({"SELECT", "INSERT"}) + + +def test_the_service_only_reads_the_rows_a_run_needs() -> None: + for table_name in READ_TABLES: + assert SERVICE_TABLE_PRIVILEGES[table_name] == frozenset({"SELECT"}) + assert set(SERVICE_TABLE_PRIVILEGES) == {*WRITTEN_TABLES, *READ_TABLES} + + +def _superuser_engine(database): + return create_engine( + make_url(database.superuser_url).set(drivername="postgresql+psycopg"), + isolation_level="AUTOCOMMIT", + ) + + +def _create_tables(database, table_names, *, schema="public"): + engine = _superuser_engine(database) + try: + with engine.connect() as connection: + for table_name in table_names: + connection.execute( + text(f"CREATE TABLE {schema}.{table_name} (id uuid PRIMARY KEY)") + ) + finally: + engine.dispose() + + +def _grant(database, statements): + engine = _superuser_engine(database) + try: + with engine.connect() as connection: + for statement in statements: + connection.execute(text(statement)) + finally: + engine.dispose() + + +def _grant_every_required_privilege(database, *, except_for=()): + _grant( + database, + [ + f"GRANT {privilege} ON public.{table_name} TO {SERVICE_TEST_ROLE}" + for table_name, privileges in SERVICE_TABLE_PRIVILEGES.items() + for privilege in sorted(privileges) + if (privilege, table_name) not in except_for + ], + ) + + +def _missing_privileges_as_the_service(database): + engine = create_simulation_engine(database.service_url) + try: + with engine.connect() as connection: + return missing_table_privileges(connection) + finally: + engine.dispose() + + +@pytest.mark.postgres +def test_a_role_with_every_required_privilege_misses_none(disposable_database): + _create_tables(disposable_database, SERVICE_TABLE_PRIVILEGES) + _grant_every_required_privilege(disposable_database) + + assert _missing_privileges_as_the_service(disposable_database) == [] + + +@pytest.mark.postgres +def test_each_missing_privilege_is_named(disposable_database): + _create_tables(disposable_database, SERVICE_TABLE_PRIVILEGES) + _grant_every_required_privilege( + disposable_database, + except_for={("INSERT", "simulation_runs"), ("SELECT", "policies")}, + ) + + assert _missing_privileges_as_the_service(disposable_database) == [ + "SELECT on policies", + "INSERT on simulation_runs", + ] + + +@pytest.mark.postgres +def test_a_missing_table_lacks_every_privilege(disposable_database): + present_tables = [ + table_name + for table_name in SERVICE_TABLE_PRIVILEGES + if table_name != "household_jobs" + ] + _create_tables(disposable_database, present_tables) + _grant( + disposable_database, + [ + f"GRANT {privilege} ON public.{table_name} TO {SERVICE_TEST_ROLE}" + for table_name in present_tables + for privilege in sorted(SERVICE_TABLE_PRIVILEGES[table_name]) + ], + ) + + assert _missing_privileges_as_the_service(disposable_database) == [ + "INSERT on household_jobs", + "SELECT on household_jobs", + ] + + +@pytest.mark.postgres +def test_only_the_public_schema_counts(disposable_database): + _grant( + disposable_database, + [ + "CREATE SCHEMA decoy", + f"GRANT USAGE ON SCHEMA decoy TO {SERVICE_TEST_ROLE}", + f"ALTER ROLE {SERVICE_TEST_ROLE} IN DATABASE " + f'"{disposable_database.name}" SET search_path = decoy, public', + ], + ) + _create_tables(disposable_database, SERVICE_TABLE_PRIVILEGES) + _create_tables(disposable_database, ["reports"], schema="decoy") + _grant_every_required_privilege( + disposable_database, except_for={("INSERT", "reports")} + ) + _grant( + disposable_database, + [f"GRANT SELECT, INSERT ON decoy.reports TO {SERVICE_TEST_ROLE}"], + ) + + assert _missing_privileges_as_the_service(disposable_database) == [ + "INSERT on reports", + ] + + +def _granted_privileges(fixture: Path) -> list[tuple[str, str]]: + """Read ``
`` lines, skipping comments.""" + lines = fixture.read_text(encoding="utf-8").splitlines() + return [ + (privilege, table_name) + for privilege, table_name in ( + line.split() for line in lines if line and not line.startswith("#") + ) + ] + + +@pytest.mark.postgres +@pytest.mark.parametrize( + ("fixture_name", "expected_missing"), + [("bad_grants.txt", ["INSERT on simulation_runs"]), ("good_grants.txt", [])], +) +def test_rule_proof_missing_privileges_reported( + disposable_database, fixture_name, expected_missing +): + """Proof for rule sim.persistence.missing-privileges-reported. + + The bad grants leave out ``INSERT`` on ``simulation_runs``; the good ones + hold every privilege the service needs. + """ + _create_tables(disposable_database, SERVICE_TABLE_PRIVILEGES) + _grant( + disposable_database, + [ + f"GRANT {privilege} ON public.{table_name} TO {SERVICE_TEST_ROLE}" + for privilege, table_name in _granted_privileges( + PROOF_FIXTURES / fixture_name + ) + ], + ) + + assert _missing_privileges_as_the_service(disposable_database) == expected_missing diff --git a/libs/policyengine-simulation-persistence/tests/test_database_job.py b/libs/policyengine-simulation-persistence/tests/test_database_job.py new file mode 100644 index 000000000..e1436f30f --- /dev/null +++ b/libs/policyengine-simulation-persistence/tests/test_database_job.py @@ -0,0 +1,44 @@ +"""The PR job that runs the persistence tests needing PostgreSQL. + +Those tests skip without a server, so the ``Test`` job never runs them; the +``Simulation database`` job gives them one and makes a missing server fail. +""" + +from __future__ import annotations + +from pathlib import Path + +import yaml + +PR_WORKFLOW = Path(__file__).resolve().parents[3] / ".github" / "workflows" / "pr.yml" +JOB_ID = "simulation-database" +STACK_CONDITION = ( + "${{ !github.event.pull_request.stack || " + "github.event.pull_request.stack.base.ref == github.event.pull_request.base.ref }}" +) + + +def _jobs() -> dict: + return yaml.safe_load(PR_WORKFLOW.read_text(encoding="utf-8"))["jobs"] + + +def test_the_database_tests_run_on_postgres_and_may_not_skip() -> None: + job = _jobs()[JOB_ID] + server = job["services"]["postgres"] + + assert server["image"] == "postgres:17" + assert job["env"]["SIMULATION_TEST_DATABASE_URL"] == ( + f"postgresql://postgres:{server['env']['POSTGRES_PASSWORD']}" + "@127.0.0.1:5432/postgres" + ) + assert job["env"]["SIMULATION_REQUIRE_DATABASE_TESTS"] == "1" + test_step = job["steps"][-1] + assert test_step["working-directory"] == "libs/policyengine-simulation-persistence" + assert test_step["run"] == "uv run --frozen pytest tests/ -v -m postgres" + + +def test_ci_complete_needs_the_database_job_at_the_bottom_of_a_stack() -> None: + jobs = _jobs() + + assert JOB_ID in jobs["ci-complete"]["needs"] + assert jobs[JOB_ID]["if"] == STACK_CONDITION diff --git a/libs/policyengine-simulation-persistence/uv.lock b/libs/policyengine-simulation-persistence/uv.lock index 90f5cbc6e..257ad6e76 100644 --- a/libs/policyengine-simulation-persistence/uv.lock +++ b/libs/policyengine-simulation-persistence/uv.lock @@ -91,6 +91,7 @@ build = [ test = [ { name = "pytest" }, { name = "pytest-cov" }, + { name = "pyyaml" }, ] [package.metadata] @@ -99,6 +100,7 @@ requires-dist = [ { name = "pyright", marker = "extra == 'build'", specifier = ">=1.1.401" }, { name = "pytest", marker = "extra == 'test'", specifier = ">=8.3.4" }, { name = "pytest-cov", marker = "extra == 'test'", specifier = ">=6.1.1" }, + { name = "pyyaml", marker = "extra == 'test'", specifier = ">=6.0.2" }, { name = "sqlalchemy", specifier = ">=2,<3" }, ] provides-extras = ["test", "build"] @@ -190,6 +192,24 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/9d/7a/d968e294073affff457b041c2be9868a40c1c71f4a35fcc1e45e5493067b/pytest_cov-7.1.0-py3-none-any.whl", hash = "sha256:a0461110b7865f9a271aa1b51e516c9a95de9d696734a2f71e3e78f46e1d4678", size = 22876, upload-time = "2026-03-21T20:11:14.438Z" }, ] +[[package]] +name = "pyyaml" +version = "6.0.3" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/05/8e/961c0007c59b8dd7729d542c61a4d537767a59645b82a0b521206e1e25c2/pyyaml-6.0.3.tar.gz", hash = "sha256:d76623373421df22fb4cf8817020cbb7ef15c725b9d5e45f17e189bfc384190f", size = 130960, upload-time = "2025-09-25T21:33:16.546Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/d1/11/0fd08f8192109f7169db964b5707a2f1e8b745d4e239b784a5a1dd80d1db/pyyaml-6.0.3-cp313-cp313-macosx_10_13_x86_64.whl", hash = "sha256:8da9669d359f02c0b91ccc01cac4a67f16afec0dac22c2ad09f46bee0697eba8", size = 181669, upload-time = "2025-09-25T21:32:23.673Z" }, + { url = "https://files.pythonhosted.org/packages/b1/16/95309993f1d3748cd644e02e38b75d50cbc0d9561d21f390a76242ce073f/pyyaml-6.0.3-cp313-cp313-macosx_11_0_arm64.whl", hash = "sha256:2283a07e2c21a2aa78d9c4442724ec1eb15f5e42a723b99cb3d822d48f5f7ad1", size = 173252, upload-time = "2025-09-25T21:32:25.149Z" }, + { url = "https://files.pythonhosted.org/packages/50/31/b20f376d3f810b9b2371e72ef5adb33879b25edb7a6d072cb7ca0c486398/pyyaml-6.0.3-cp313-cp313-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:ee2922902c45ae8ccada2c5b501ab86c36525b883eff4255313a253a3160861c", size = 767081, upload-time = "2025-09-25T21:32:26.575Z" }, + { url = "https://files.pythonhosted.org/packages/49/1e/a55ca81e949270d5d4432fbbd19dfea5321eda7c41a849d443dc92fd1ff7/pyyaml-6.0.3-cp313-cp313-manylinux2014_s390x.manylinux_2_17_s390x.manylinux_2_28_s390x.whl", hash = "sha256:a33284e20b78bd4a18c8c2282d549d10bc8408a2a7ff57653c0cf0b9be0afce5", size = 841159, upload-time = "2025-09-25T21:32:27.727Z" }, + { url = "https://files.pythonhosted.org/packages/74/27/e5b8f34d02d9995b80abcef563ea1f8b56d20134d8f4e5e81733b1feceb2/pyyaml-6.0.3-cp313-cp313-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:0f29edc409a6392443abf94b9cf89ce99889a1dd5376d94316ae5145dfedd5d6", size = 801626, upload-time = "2025-09-25T21:32:28.878Z" }, + { url = "https://files.pythonhosted.org/packages/f9/11/ba845c23988798f40e52ba45f34849aa8a1f2d4af4b798588010792ebad6/pyyaml-6.0.3-cp313-cp313-musllinux_1_2_aarch64.whl", hash = "sha256:f7057c9a337546edc7973c0d3ba84ddcdf0daa14533c2065749c9075001090e6", size = 753613, upload-time = "2025-09-25T21:32:30.178Z" }, + { url = "https://files.pythonhosted.org/packages/3d/e0/7966e1a7bfc0a45bf0a7fb6b98ea03fc9b8d84fa7f2229e9659680b69ee3/pyyaml-6.0.3-cp313-cp313-musllinux_1_2_x86_64.whl", hash = "sha256:eda16858a3cab07b80edaf74336ece1f986ba330fdb8ee0d6c0d68fe82bc96be", size = 794115, upload-time = "2025-09-25T21:32:31.353Z" }, + { url = "https://files.pythonhosted.org/packages/de/94/980b50a6531b3019e45ddeada0626d45fa85cbe22300844a7983285bed3b/pyyaml-6.0.3-cp313-cp313-win32.whl", hash = "sha256:d0eae10f8159e8fdad514efdc92d74fd8d682c933a6dd088030f3834bc8e6b26", size = 137427, upload-time = "2025-09-25T21:32:32.58Z" }, + { url = "https://files.pythonhosted.org/packages/97/c9/39d5b874e8b28845e4ec2202b5da735d0199dbe5b8fb85f91398814a9a46/pyyaml-6.0.3-cp313-cp313-win_amd64.whl", hash = "sha256:79005a0d97d5ddabfeeea4cf676af11e647e41d81c9a7722a193022accdb6b7c", size = 154090, upload-time = "2025-09-25T21:32:33.659Z" }, + { url = "https://files.pythonhosted.org/packages/73/e8/2bdf3ca2090f68bb3d75b44da7bbc71843b19c9f2b9cb9b0f4ab7a5a4329/pyyaml-6.0.3-cp313-cp313-win_arm64.whl", hash = "sha256:5498cd1645aa724a7c71c8f378eb29ebe23da2fc0d7a08071d89469bf1d2defb", size = 140246, upload-time = "2025-09-25T21:32:34.663Z" }, +] + [[package]] name = "sqlalchemy" version = "2.1.6" diff --git a/projects/policyengine-simulation-entry/DEPLOYMENT.md b/projects/policyengine-simulation-entry/DEPLOYMENT.md index a064d8902..77b75028a 100644 --- a/projects/policyengine-simulation-entry/DEPLOYMENT.md +++ b/projects/policyengine-simulation-entry/DEPLOYMENT.md @@ -39,6 +39,12 @@ the other runtime secrets, and only the entrypoint's runtime service account may read it. A missing or invalid name, or a secret with no enabled version, stops the deploy before `gcloud run deploy` runs. +`GET /ready` reports, under `simulation_database`, whether the entrypoint +reached the database and which table privileges its role lacks +(`missing_privileges`, such as `"INSERT on reports"`; the list is in +`libs/policyengine-simulation-persistence`). The database does not change the +readiness status, because no old-gateway or Stage 12 route uses it. + ## Candidate qualification For each eligible revision, the workflow first deploys `beta` diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py index b9a2ac66c..e8c38678e 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py @@ -470,21 +470,23 @@ async def get_budget_window_job( async def _simulation_database_readiness( database: SimulationDatabaseConnection, ) -> SimulationDatabaseReadiness: - """Report whether the simulation service reached its database.""" + """Report whether the service reached its database, and what its role lacks.""" try: - await asyncio.wait_for( - database.check_connection(), + missing_privileges = await asyncio.wait_for( + database.missing_privileges(), timeout=SIMULATION_DATABASE_READINESS_TIMEOUT_SECONDS, ) - # Any failure to reach the database is reported, never raised: the - # old-gateway routes this check covers do not use it. + # Any failure to reach or read the database is reported, never raised: + # the old-gateway routes this check covers do not use it. except Exception as error: # noqa: BLE001 # reported as unreachable logger.warning( "simulation_database_unreachable", extra={"error_type": type(error).__name__}, ) - return SimulationDatabaseReadiness(reachable=False) - return SimulationDatabaseReadiness(reachable=True) + return SimulationDatabaseReadiness(reachable=False, missing_privileges=None) + return SimulationDatabaseReadiness( + reachable=True, missing_privileges=tuple(missing_privileges) + ) def _register_service_routes(app: FastAPI, entry: EntryRuntime) -> None: diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/entry_runtime.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/entry_runtime.py index 4b6d5fcc6..6712422d3 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/entry_runtime.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/entry_runtime.py @@ -216,8 +216,8 @@ class SimulationDatabaseConnection(Protocol): The real implementation is ``SimulationDatabase``; tests inject fakes. """ - async def check_connection(self) -> None: - """Raise unless a pooled connection answers a trivial query.""" + async def missing_privileges(self) -> list[str]: + """Return each table privilege the service needs and its role lacks.""" ... def dispose(self) -> None: diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/schemas.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/schemas.py index 00876581c..9c8e1d574 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/schemas.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/schemas.py @@ -121,13 +121,16 @@ class SimulationDatabaseReadiness(BaseModel): """The simulation service's database, as the readiness check found it. ``reachable`` says whether a pooled connection answered within the check's - time limit. It doesn't change ``status``: no old-gateway or Stage 12 route - uses this database. + time limit. ``missing_privileges`` names each table privilege the service + needs and its role lacks, such as ``"INSERT on reports"``, or is null when + the database was not reached. Neither changes ``status``: no old-gateway or + Stage 12 route uses this database. """ model_config = ConfigDict(extra="forbid", frozen=True) reachable: bool + missing_privileges: tuple[str, ...] | None class SimulationEntrypointReadinessResponse(BaseModel): diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py index 1fe2047eb..8c5b19c23 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py @@ -5,8 +5,10 @@ import asyncio from typing import TYPE_CHECKING -from policyengine_simulation_persistence import create_simulation_engine -from sqlalchemy import select +from policyengine_simulation_persistence import ( + create_simulation_engine, + missing_table_privileges, +) if TYPE_CHECKING: from sqlalchemy import Engine @@ -27,13 +29,13 @@ def from_url(cls, database_url: str) -> SimulationDatabase: """ return cls(create_simulation_engine(database_url)) - def _check_connection(self) -> None: + def _missing_privileges(self) -> list[str]: with self.engine.connect() as connection: - connection.execute(select(1)) + return missing_table_privileges(connection) - async def check_connection(self) -> None: - """Raise unless a pooled connection answers a trivial query.""" - await asyncio.to_thread(self._check_connection) + async def missing_privileges(self) -> list[str]: + """Return each table privilege the service needs and its role lacks.""" + return await asyncio.to_thread(self._missing_privileges) def dispose(self) -> None: """Close every pooled connection.""" diff --git a/projects/policyengine-simulation-entry/tests/test_openapi.py b/projects/policyengine-simulation-entry/tests/test_openapi.py index a01f546d3..f95b5e3a5 100644 --- a/projects/policyengine-simulation-entry/tests/test_openapi.py +++ b/projects/policyengine-simulation-entry/tests/test_openapi.py @@ -136,7 +136,7 @@ def test_local_area_breakdowns_may_list_the_withheld_areas_of_each_level(): assert field not in breakdown["required"] -def test_readiness_documents_whether_the_simulation_database_was_reached(): +def test_readiness_documents_the_simulation_database_check(): spec = create_app().openapi() readiness = spec["paths"]["/ready"]["get"]["responses"]["200"] schema = readiness["content"]["application/json"]["schema"] @@ -145,7 +145,7 @@ def test_readiness_documents_whether_the_simulation_database_was_reached(): assert schema == { "$ref": "#/components/schemas/SimulationEntrypointReadinessResponse" } - assert set(database["properties"]) == {"reachable"} + assert set(database["properties"]) == {"reachable", "missing_privileges"} def test_route_table_is_frozen(): diff --git a/projects/policyengine-simulation-entry/tests/test_simulation_database.py b/projects/policyengine-simulation-entry/tests/test_simulation_database.py index 191ebaf76..864eb010f 100644 --- a/projects/policyengine-simulation-entry/tests/test_simulation_database.py +++ b/projects/policyengine-simulation-entry/tests/test_simulation_database.py @@ -9,6 +9,7 @@ from conftest import FakeBackend, make_settings from fastapi.testclient import TestClient from policyengine_simulation_entry import app as app_module +from policyengine_simulation_entry import simulation_database as database_module from policyengine_simulation_entry.app import create_app from policyengine_simulation_entry.simulation_database import SimulationDatabase from sqlalchemy.exc import OperationalError @@ -17,20 +18,22 @@ class FakeSimulationDatabase: - """Answer the connection check, or fail the way an unreachable database does.""" + """Name the privileges the role lacks, or fail as an unreachable database does.""" - def __init__(self, *, failure=None, delay_seconds=0): + def __init__(self, *, missing_privileges=(), failure=None, delay_seconds=0): + self.missing = list(missing_privileges) self.failure = failure self.delay_seconds = delay_seconds self.checks = 0 self.disposed = False - async def check_connection(self): + async def missing_privileges(self): self.checks += 1 if self.delay_seconds: await asyncio.sleep(self.delay_seconds) if self.failure is not None: raise self.failure + return self.missing def dispose(self): self.disposed = True @@ -47,7 +50,7 @@ def _client(backend, database): ) -def test_readiness_reports_that_the_database_was_reached(): +def test_readiness_reports_a_reached_database_with_every_privilege(): database = FakeSimulationDatabase() with _client(FakeBackend(), database) as client: result = client.get("/ready") @@ -55,11 +58,28 @@ def test_readiness_reports_that_the_database_was_reached(): assert result.status_code == 200 assert result.json() == { "status": "ready", - "simulation_database": {"reachable": True}, + "simulation_database": {"reachable": True, "missing_privileges": []}, } assert database.checks == 1 +def test_readiness_names_the_privileges_the_role_lacks(): + database = FakeSimulationDatabase( + missing_privileges=["INSERT on reports", "SELECT on policies"] + ) + with _client(FakeBackend(), database) as client: + result = client.get("/ready") + + assert result.status_code == 200 + assert result.json() == { + "status": "ready", + "simulation_database": { + "reachable": True, + "missing_privileges": ["INSERT on reports", "SELECT on policies"], + }, + } + + def test_an_unreachable_database_does_not_change_old_gateway_readiness(caplog): failure = OperationalError("SELECT 1", {}, Exception("password=secret")) with _client(FakeBackend(), FakeSimulationDatabase(failure=failure)) as client: @@ -68,7 +88,7 @@ def test_an_unreachable_database_does_not_change_old_gateway_readiness(caplog): assert result.status_code == 200 assert result.json() == { "status": "ready", - "simulation_database": {"reachable": False}, + "simulation_database": {"reachable": False, "missing_privileges": None}, } assert "simulation_database_unreachable" in caplog.text assert "secret" not in caplog.text @@ -82,7 +102,7 @@ def test_readiness_waits_only_briefly_for_the_database(monkeypatch): with _client(FakeBackend(), slow_database) as client: database = client.get("/ready").json()["simulation_database"] - assert database == {"reachable": False} + assert database == {"reachable": False, "missing_privileges": None} def test_readiness_still_follows_the_old_gateway(): @@ -94,7 +114,7 @@ def test_readiness_still_follows_the_old_gateway(): assert result.status_code == 503 assert result.json() == { "status": "not_ready", - "simulation_database": {"reachable": True}, + "simulation_database": {"reachable": True, "missing_privileges": []}, } @@ -139,21 +159,18 @@ def build(database_url): assert built_urls == [DATABASE_URL] -def test_the_database_checks_its_connection_off_the_event_loop(): - statements = [] - statement_threads = [] +def test_the_database_reads_the_missing_privileges_off_the_event_loop(monkeypatch): + connections = [] + query_threads = [] class FakeConnection: def __enter__(self): + connections.append(self) return self def __exit__(self, *_): return False - def execute(self, statement): - statements.append(str(statement)) - statement_threads.append(threading.get_ident()) - class FakeEngine: disposed = False @@ -163,18 +180,25 @@ def connect(self): def dispose(self): self.disposed = True + def missing_table_privileges(connection): + assert connection in connections + query_threads.append(threading.get_ident()) + return ["INSERT on reports"] + + monkeypatch.setattr( + database_module, "missing_table_privileges", missing_table_privileges + ) engine = FakeEngine() database = SimulationDatabase(engine) - async def check_from_the_event_loop(): - await database.check_connection() - return threading.get_ident() + async def read_from_the_event_loop(): + return await database.missing_privileges(), threading.get_ident() - event_loop_thread = asyncio.run(check_from_the_event_loop()) + missing, event_loop_thread = asyncio.run(read_from_the_event_loop()) database.dispose() - assert statements == ["SELECT 1"] - assert statement_threads != [event_loop_thread] + assert missing == ["INSERT on reports"] + assert query_threads != [event_loop_thread] assert engine.disposed is True diff --git a/projects/policyengine-simulation-entry/uv.lock b/projects/policyengine-simulation-entry/uv.lock index afff6f7a7..d3b36ad8d 100644 --- a/projects/policyengine-simulation-entry/uv.lock +++ b/projects/policyengine-simulation-entry/uv.lock @@ -980,6 +980,7 @@ requires-dist = [ { name = "pyright", marker = "extra == 'build'", specifier = ">=1.1.401" }, { name = "pytest", marker = "extra == 'test'", specifier = ">=8.3.4" }, { name = "pytest-cov", marker = "extra == 'test'", specifier = ">=6.1.1" }, + { name = "pyyaml", marker = "extra == 'test'", specifier = ">=6.0.2" }, { name = "sqlalchemy", specifier = ">=2,<3" }, ] provides-extras = ["test", "build"] diff --git a/rules/fixtures/sim.persistence.missing-privileges-reported/bad_grants.txt b/rules/fixtures/sim.persistence.missing-privileges-reported/bad_grants.txt new file mode 100644 index 000000000..40a090eb9 --- /dev/null +++ b/rules/fixtures/sim.persistence.missing-privileges-reported/bad_grants.txt @@ -0,0 +1,21 @@ +# Proof fixture: the grants with INSERT on simulation_runs left out, as when +# a role was set up before the table existed. +INSERT simulations +SELECT simulations +INSERT reports +SELECT reports +SELECT simulation_runs +INSERT report_runs +SELECT report_runs +INSERT report_run_simulation_runs +SELECT report_run_simulation_runs +INSERT household_jobs +SELECT household_jobs +SELECT policies +SELECT households +SELECT tax_benefit_models +SELECT tax_benefit_model_versions +SELECT regions +SELECT datasets +SELECT parameters +SELECT parameter_values diff --git a/rules/fixtures/sim.persistence.missing-privileges-reported/good_grants.txt b/rules/fixtures/sim.persistence.missing-privileges-reported/good_grants.txt new file mode 100644 index 000000000..f5817ac8e --- /dev/null +++ b/rules/fixtures/sim.persistence.missing-privileges-reported/good_grants.txt @@ -0,0 +1,21 @@ +# Proof fixture: every privilege the simulation service needs, one per line. +INSERT simulations +SELECT simulations +INSERT reports +SELECT reports +INSERT simulation_runs +SELECT simulation_runs +INSERT report_runs +SELECT report_runs +INSERT report_run_simulation_runs +SELECT report_run_simulation_runs +INSERT household_jobs +SELECT household_jobs +SELECT policies +SELECT households +SELECT tax_benefit_models +SELECT tax_benefit_model_versions +SELECT regions +SELECT datasets +SELECT parameters +SELECT parameter_values diff --git a/rules/registry.toml b/rules/registry.toml index 1823474c8..0180a9066 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -21,6 +21,10 @@ title = "Service and package boundaries" id = "stage12" title = "Stage 12 persistence" +[[category]] +id = "persistence" +title = "Simulation service database" + [[category]] id = "deployment" title = "Deployment pipeline" @@ -133,6 +137,36 @@ good_fixture = "rules/fixtures/sim.boundaries.sqlalchemy-and-alembic-only/good_d project = "projects/policyengine-simulation-entry" test = "tests/test_database_access_boundaries.py::test_rule_proof_sqlalchemy_and_alembic_only" +# --- Simulation service database ------------------------------------------ + +[[rule]] +id = "sim.persistence.missing-privileges-reported" +category = "persistence" +statement = "The simulation service's readiness check names every table privilege its role needs on the Public API's tables and lacks, as PostgreSQL reports it, and counts a table missing from the public schema as lacking all of them." +rationale = "libs/policyengine-simulation-persistence/README.md#the-roles-privileges" +scope = ["libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/access.py"] +tier = "mechanical" +status = "enforced" +autofix = false +[rule.enforcer] +kind = "pytest" +project = "libs/policyengine-simulation-persistence" +nodes = [ + "tests/test_access.py::test_a_role_with_every_required_privilege_misses_none", + "tests/test_access.py::test_each_missing_privilege_is_named", + "tests/test_access.py::test_a_missing_table_lacks_every_privilege", + "tests/test_access.py::test_only_the_public_schema_counts", +] +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "simulation-database" +invocation = "uv run --frozen pytest tests/ -v -m postgres" +[rule.proof] +bad_fixture = "rules/fixtures/sim.persistence.missing-privileges-reported/bad_grants.txt" +good_fixture = "rules/fixtures/sim.persistence.missing-privileges-reported/good_grants.txt" +project = "libs/policyengine-simulation-persistence" +test = "tests/test_access.py::test_rule_proof_missing_privileges_reported" + # --- Stage 12 persistence -------------------------------------------------- [[rule]] From 6a52dd2312d73814d62002d5b4a8070fc39e0a9c Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:11:46 +0400 Subject: [PATCH 07/25] Require the candidate to reach its database with every privilege its role needs The candidate smoke check now sets REQUIRE_SIMULATION_DATABASE_ACCESS=true and then requires /ready to report the simulation service's database reached with no table privilege missing, which shows that the role's URL reached the revision and that the role holds its grants. A failure names what /ready reported, such as "INSERT on simulation_runs". /ready waits at most three seconds for the database, while a new connection may take up to five, so a cold candidate whose first connection is slow is asked again while it reports the database unreachable, up to SIMULATION_DATABASE_READY_MAX_ATTEMPTS requests in all (six by default), with the usual retry delay. A reached database with a missing privilege fails at once. The stable and rollback checks don't set the requirement, because an older image may not use the database. The fake curl in the deployment tests can now serve a sequence of /ready bodies, and the new tests are registered under sim.deploy.smoke-tests-strict. Co-Authored-By: Claude Opus 5.5 --- .../cloud-run-simulation-entry-smoke.sh | 45 +++++- .../workflows/simulation-deploy.reusable.yml | 2 + docs/generated/rules.md | 6 +- .../DEPLOYMENT.md | 9 +- .../tests/test_deployment_assets_support.py | 17 +- .../test_simulation_database_deployment.py | 145 +++++++++++++++++- rules/registry.toml | 5 +- 7 files changed, 221 insertions(+), 8 deletions(-) diff --git a/.github/scripts/cloud-run-simulation-entry-smoke.sh b/.github/scripts/cloud-run-simulation-entry-smoke.sh index 84c0726da..b41afeb15 100755 --- a/.github/scripts/cloud-run-simulation-entry-smoke.sh +++ b/.github/scripts/cloud-run-simulation-entry-smoke.sh @@ -25,6 +25,12 @@ previous_revision="${3:-}" # and model. # This is the identity the gateway reports from the routing registry; the # numerical release sentinel checks what the worker computes. +# REQUIRE_SIMULATION_DATABASE_ACCESS=true (candidate checks only): /ready must +# report the simulation service's database reached, with no table privilege +# missing, which shows that the role's URL reached the revision and the role +# holds its grants. Stable and rollback checks leave it unset, because an +# older image may not use the database. +require_database_access="${REQUIRE_SIMULATION_DATABASE_ACCESS:-}" expected_identity="${EXPECTED_IDENTITY:-none}" expected_policyengine_version="${EXPECTED_POLICYENGINE_VERSION:-}" expected_app_name="${EXPECTED_APP_NAME:-}" @@ -39,6 +45,14 @@ case "${expected_identity}" in exit 2 ;; esac +case "${require_database_access}" in + "" | true) ;; + *) + printf 'REQUIRE_SIMULATION_DATABASE_ACCESS must be true or empty, not %s\n' \ + "${require_database_access}" >&2 + exit 2 + ;; +esac case "${expected_latest}" in "" | true) ;; *) @@ -63,6 +77,10 @@ if [ "${expected_identity}" = "required" ] || fi routing_max_attempts="${SIMULATION_ENTRYPOINT_ROUTING_MAX_ATTEMPTS:-24}" routing_retry_delay_seconds="${SIMULATION_ENTRYPOINT_ROUTING_RETRY_DELAY_SECONDS:-5}" +# A cold candidate may not reach its database within /ready's three-second +# wait; an unreachable database is asked about again, up to this many +# requests in all. +database_ready_max_attempts="${SIMULATION_DATABASE_READY_MAX_ATTEMPTS:-6}" if ! [[ "${routing_max_attempts}" =~ ^[1-9][0-9]*$ ]]; then printf 'SIMULATION_ENTRYPOINT_ROUTING_MAX_ATTEMPTS must be a positive integer\n' >&2 @@ -72,6 +90,10 @@ if ! [[ "${routing_retry_delay_seconds}" =~ ^[0-9]+$ ]]; then printf 'SIMULATION_ENTRYPOINT_ROUTING_RETRY_DELAY_SECONDS must be a non-negative integer\n' >&2 exit 2 fi +if ! [[ "${database_ready_max_attempts}" =~ ^[1-9][0-9]*$ ]]; then + printf 'SIMULATION_DATABASE_READY_MAX_ATTEMPTS must be a positive integer\n' >&2 + exit 2 +fi health_headers="$(mktemp)" health_body="$(mktemp)" @@ -131,8 +153,27 @@ while true; do attempt=$((attempt + 1)) done -curl --fail --silent --show-error "${base_url}/ready" | - jq -e '.status == "ready"' >/dev/null +ready_body="$(curl --fail --silent --show-error "${base_url}/ready")" +jq -e '.status == "ready"' <<<"${ready_body}" >/dev/null +if [ "${require_database_access}" = "true" ]; then + database_attempt=1 + while [ "${database_attempt}" -lt "${database_ready_max_attempts}" ] && + jq -e '.simulation_database.reachable == false' <<<"${ready_body}" >/dev/null; do + printf '%s: the simulation database was not reached; asking again (%s/%s)\n' \ + "${base_url}" "${database_attempt}" "${database_ready_max_attempts}" >&2 + sleep "${routing_retry_delay_seconds}" + ready_body="$(curl --fail --silent --show-error "${base_url}/ready")" + database_attempt=$((database_attempt + 1)) + done + if ! jq -e ' + .simulation_database.reachable == true + and .simulation_database.missing_privileges == [] + ' <<<"${ready_body}" >/dev/null; then + printf '%s: the simulation service cannot use the simulation database: %s\n' \ + "${base_url}" "$(jq -c '.simulation_database' <<<"${ready_body}")" >&2 + exit 1 + fi +fi curl --fail --silent --show-error "${base_url}/versions" --output "${versions_body}" jq -e 'type == "object"' "${versions_body}" >/dev/null diff --git a/.github/workflows/simulation-deploy.reusable.yml b/.github/workflows/simulation-deploy.reusable.yml index 7427ecfdc..d181564dc 100644 --- a/.github/workflows/simulation-deploy.reusable.yml +++ b/.github/workflows/simulation-deploy.reusable.yml @@ -765,6 +765,8 @@ jobs: EXPECTED_UK_DATASET_SHA256: ${{ inputs.deploy_existing_stack && needs.prepare.outputs.uk_dataset_sha256 || '' }} # Latest must route to this release only when it should advance it. EXPECTED_LATEST: ${{ inputs.deploy_existing_stack && needs.update_routing.outputs.advances_latest == 'true' && 'true' || '' }} + # The candidate reaches its database, and its role holds its grants. + REQUIRE_SIMULATION_DATABASE_ACCESS: "true" run: .github/scripts/cloud-run-simulation-entry-smoke.sh "${{ needs.deploy_entrypoint.outputs.candidate_url }}" "${{ needs.deploy_entrypoint.outputs.revision }}" - name: Download the numerical release sentinel diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 9c6628021..880bcf468 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -168,7 +168,7 @@ flowchart LR | `sim.deploy.revision-tags-valid` | Cloud Run revision tags are valid on a workflow's first run and fit the existing service names. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py` | `pr.yml:test` | **unproven** | | `sim.deploy.routing-publication-journaled` | Each routing publication is named by its run, attempt and environment and journaled before it is applied; an always-running finalizer resolves it from the journal with the release-gate outcome, a release reports success only after that finalizer completes, and routing jobs never tolerate a failed command. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets_release_gates.py`, `projects/policyengine-simulation-entry/tests/test_deployment_assets_routing.py` | `pr.yml:test` | **unproven** | | `sim.deploy.shell-scripts-valid` | Deployment shell scripts have valid shell syntax. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets.py` | `pr.yml:test` | **unproven** | -| `sim.deploy.smoke-tests-strict` | Candidate and stable smoke tests reject a revision mismatch without retrying, and fail if the previous revision does not converge. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets.py` | `pr.yml:test` | **unproven** | +| `sim.deploy.smoke-tests-strict` | Candidate and stable smoke tests reject a revision mismatch without retrying, and fail if the previous revision does not converge; the candidate smoke also requires the simulation service to reach its database with every table privilege its role needs. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets.py`, `projects/policyengine-simulation-entry/tests/test_simulation_database_deployment.py` | `pr.yml:test` | **unproven** | | `sim.deploy.staging-promotes-tested-revision` | Every staging deployment passes promote_entrypoint: ${{ github.ref == 'refs/heads/main' }}, so a run from main moves the staging service's stable Cloud Run traffic to the entrypoint revision that passed the staging tests, and an accidental manual run from another branch neither moves that traffic nor deletes older revisions. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets_promotion.py` | `pr.yml:test` | yes | | `sim.deploy.traffic-changes-exact` | Traffic changes target an exact, validated revision, are reversible, and refuse to run over an intervening promotion. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets_traffic.py` | `pr.yml:test` | **unproven** | | `sim.deploy.workflow-shell-safety` | Cloud Run deployment never interpolates GitHub expressions into shell, escapes environment values, pins secret versions, stops before deploying when any runtime secret cannot be resolved, and resolves the Stage 12 secrets with the Stage 12 block's own helper. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py` | `pr.yml:test` | yes | @@ -203,6 +203,7 @@ flowchart LR e_pytest_projects_policyengine_simulation_entry_tests_test_deployment_assets_release_gates_py__projects_policyengine_simulation_entry_tests_test_deployment_assets_routing_py["pytest projects/policyengine-simulation-entry/tests/test_deployment_assets_release_gates.py, projects/policyengine-simulation-entry/tests/test_deployment_assets_routing.py"] r_sim_deploy_shell_scripts_valid("sim.deploy.shell-scripts-valid (enforced)") r_sim_deploy_smoke_tests_strict("sim.deploy.smoke-tests-strict (enforced)") + e_pytest_projects_policyengine_simulation_entry_tests_test_deployment_assets_py__projects_policyengine_simulation_entry_tests_test_simulation_database_deployment_py["pytest projects/policyengine-simulation-entry/tests/test_deployment_assets.py, projects/policyengine-simulation-entry/tests/test_simulation_database_deployment.py"] r_sim_deploy_staging_promotes_tested_revision("sim.deploy.staging-promotes-tested-revision (enforced)") r_sim_deploy_traffic_changes_exact("sim.deploy.traffic-changes-exact (enforced)") e_pytest_projects_policyengine_simulation_entry_tests_test_deployment_assets_traffic_py["pytest projects/policyengine-simulation-entry/tests/test_deployment_assets_traffic.py"] @@ -230,7 +231,8 @@ flowchart LR r_sim_deploy_routing_publication_journaled --> e_pytest_projects_policyengine_simulation_entry_tests_test_deployment_assets_release_gates_py__projects_policyengine_simulation_entry_tests_test_deployment_assets_routing_py e_pytest_projects_policyengine_simulation_entry_tests_test_deployment_assets_release_gates_py__projects_policyengine_simulation_entry_tests_test_deployment_assets_routing_py --> j_CI_pr_yml_test r_sim_deploy_shell_scripts_valid --> e_pytest_projects_policyengine_simulation_entry_tests_test_deployment_assets_py - r_sim_deploy_smoke_tests_strict --> e_pytest_projects_policyengine_simulation_entry_tests_test_deployment_assets_py + r_sim_deploy_smoke_tests_strict --> e_pytest_projects_policyengine_simulation_entry_tests_test_deployment_assets_py__projects_policyengine_simulation_entry_tests_test_simulation_database_deployment_py + e_pytest_projects_policyengine_simulation_entry_tests_test_deployment_assets_py__projects_policyengine_simulation_entry_tests_test_simulation_database_deployment_py --> j_CI_pr_yml_test r_sim_deploy_staging_promotes_tested_revision --> e_pytest_projects_policyengine_simulation_entry_tests_test_deployment_assets_promotion_py r_sim_deploy_traffic_changes_exact --> e_pytest_projects_policyengine_simulation_entry_tests_test_deployment_assets_traffic_py e_pytest_projects_policyengine_simulation_entry_tests_test_deployment_assets_traffic_py --> j_CI_pr_yml_test diff --git a/projects/policyengine-simulation-entry/DEPLOYMENT.md b/projects/policyengine-simulation-entry/DEPLOYMENT.md index 77b75028a..53f979754 100644 --- a/projects/policyengine-simulation-entry/DEPLOYMENT.md +++ b/projects/policyengine-simulation-entry/DEPLOYMENT.md @@ -43,7 +43,14 @@ stops the deploy before `gcloud run deploy` runs. reached the database and which table privileges its role lacks (`missing_privileges`, such as `"INSERT on reports"`; the list is in `libs/policyengine-simulation-persistence`). The database does not change the -readiness status, because no old-gateway or Stage 12 route uses it. +readiness status, because no old-gateway or Stage 12 route uses it. The +candidate smoke check (`REQUIRE_SIMULATION_DATABASE_ACCESS=true`) requires the +database reached with no privilege missing, which shows that the role's URL +reached the revision and that the role holds its grants. While a cold +candidate has not reached its database yet, it asks again, up to six requests +in all; a reached database with a missing privilege fails at once. The stable +and rollback checks don't require it, because an older image may not use the +database. ## Candidate qualification diff --git a/projects/policyengine-simulation-entry/tests/test_deployment_assets_support.py b/projects/policyengine-simulation-entry/tests/test_deployment_assets_support.py index 70ae911b0..2c24724d3 100644 --- a/projects/policyengine-simulation-entry/tests/test_deployment_assets_support.py +++ b/projects/policyengine-simulation-entry/tests/test_deployment_assets_support.py @@ -142,7 +142,22 @@ def write_fake_smoke_curl(tmp_path: Path, revisions: list[str]) -> tuple[Path, P printf '{"status":"healthy"}' > "${body_file}" ;; */ready) - printf '{"status":"ready"}' + ready_body='{"status":"ready"}' + if [ -n "${FAKE_CURL_READY_BODIES:-}" ]; then + # One body per line, served in order; the last one repeats. + ready_count=0 + if [ -f "${FAKE_CURL_READY_BODIES}.count" ]; then + ready_count="$(cat "${FAKE_CURL_READY_BODIES}.count")" + fi + ready_count=$((ready_count + 1)) + printf '%s' "${ready_count}" > "${FAKE_CURL_READY_BODIES}.count" + line_count="$(wc -l < "${FAKE_CURL_READY_BODIES}")" + if [ "${ready_count}" -gt "${line_count}" ]; then + ready_count="${line_count}" + fi + ready_body="$(sed -n "${ready_count}p" "${FAKE_CURL_READY_BODIES}")" + fi + printf '%s' "${FAKE_CURL_READY:-${ready_body}}" ;; */versions) versions='{}' diff --git a/projects/policyengine-simulation-entry/tests/test_simulation_database_deployment.py b/projects/policyengine-simulation-entry/tests/test_simulation_database_deployment.py index a6156f16c..8931d7b05 100644 --- a/projects/policyengine-simulation-entry/tests/test_simulation_database_deployment.py +++ b/projects/policyengine-simulation-entry/tests/test_simulation_database_deployment.py @@ -1,7 +1,8 @@ """Deployment wiring for the simulation service's database. The entrypoint receives its role's URL as a pinned Cloud Run secret on every -deploy, whatever the Stage 12 setting. +deploy, whatever the Stage 12 setting, and the candidate smoke requires it to +reach the database with every table privilege its role needs. """ from __future__ import annotations @@ -15,7 +16,10 @@ from test_deployment_assets_support import ( CLOUD_RUN_DEPLOY_SCRIPT, REUSABLE_DEPLOY_WORKFLOW, + SMOKE_SCRIPT, + smoke_test_env, workflow_jobs, + write_fake_smoke_curl, ) @@ -86,3 +90,142 @@ def test_the_deploy_step_passes_the_database_url_secret_name(): assert deploy_step["env"]["SIMULATION_DATABASE_URL_SECRET_NAME"] == ( "${{ vars.SIMULATION_DATABASE_URL_SECRET_NAME }}" ) + + +# --- The candidate smoke -------------------------------------------------- + + +def test_the_candidate_smoke_requires_access_to_the_database(): + integration = workflow_jobs(REUSABLE_DEPLOY_WORKFLOW)["integration"] + smoke = next( + step + for step in integration["steps"] + if step.get("name") == "Verify the complete deployed request path" + ) + + assert smoke["env"]["REQUIRE_SIMULATION_DATABASE_ACCESS"] == "true" + + +def _ready_body(*, reachable=True, missing_privileges=()): + return json.dumps( + { + "status": "ready", + "simulation_database": { + "reachable": reachable, + "missing_privileges": ( + list(missing_privileges) if reachable else None + ), + }, + } + ) + + +def _run_smoke(tmp_path, ready_body, *, require): + state_file, revisions_file = write_fake_smoke_curl(tmp_path, ["revision-1"]) + environment = smoke_test_env(tmp_path, state_file, revisions_file) | { + "FAKE_CURL_READY": ready_body, + } + if require is not None: + environment["REQUIRE_SIMULATION_DATABASE_ACCESS"] = require + return subprocess.run( + ["bash", SMOKE_SCRIPT, "https://candidate.example.test", "revision-1"], + check=False, + capture_output=True, + text=True, + env=environment, + ) + + +def test_the_smoke_accepts_a_reached_database_with_every_privilege(tmp_path): + result = _run_smoke(tmp_path, _ready_body(), require="true") + + assert result.returncode == 0, result.stderr + + +@pytest.mark.parametrize( + "ready_body", + [ + _ready_body(reachable=False), + _ready_body(missing_privileges=["INSERT on simulation_runs"]), + json.dumps({"status": "ready"}), + ], + ids=["unreachable", "missing-privilege", "unconfigured"], +) +def test_the_smoke_rejects_a_database_the_service_cannot_use(tmp_path, ready_body): + result = _run_smoke(tmp_path, ready_body, require="true") + + assert result.returncode == 1 + assert "simulation database" in result.stderr + + +def test_the_smoke_names_the_missing_privileges(tmp_path): + result = _run_smoke( + tmp_path, + _ready_body(missing_privileges=["INSERT on simulation_runs"]), + require="true", + ) + + assert "INSERT on simulation_runs" in result.stderr + + +def _run_smoke_with_ready_bodies(tmp_path, bodies): + bodies_file = tmp_path / "ready-bodies" + bodies_file.write_text("\n".join(bodies) + "\n", encoding="utf-8") + state_file, revisions_file = write_fake_smoke_curl(tmp_path, ["revision-1"]) + environment = smoke_test_env(tmp_path, state_file, revisions_file) | { + "FAKE_CURL_READY_BODIES": str(bodies_file), + "REQUIRE_SIMULATION_DATABASE_ACCESS": "true", + "SIMULATION_DATABASE_READY_MAX_ATTEMPTS": "3", + } + result = subprocess.run( + ["bash", SMOKE_SCRIPT, "https://candidate.example.test", "revision-1"], + check=False, + capture_output=True, + text=True, + env=environment, + ) + ready_requests = int(Path(f"{bodies_file}.count").read_text(encoding="utf-8")) + return result, ready_requests + + +def test_the_smoke_waits_for_a_database_a_cold_instance_has_not_reached(tmp_path): + result, ready_requests = _run_smoke_with_ready_bodies( + tmp_path, + [_ready_body(reachable=False), _ready_body(reachable=False), _ready_body()], + ) + + assert result.returncode == 0, result.stderr + assert ready_requests == 3 + + +def test_the_smoke_stops_waiting_for_an_unreachable_database(tmp_path): + result, ready_requests = _run_smoke_with_ready_bodies( + tmp_path, [_ready_body(reachable=False)] + ) + + assert result.returncode == 1 + assert ready_requests == 3 + assert "simulation database" in result.stderr + + +def test_the_smoke_does_not_wait_for_privileges_the_role_lacks(tmp_path): + result, ready_requests = _run_smoke_with_ready_bodies( + tmp_path, + [_ready_body(missing_privileges=["SELECT on policies"]), _ready_body()], + ) + + assert result.returncode == 1 + assert ready_requests == 1 + + +def test_stable_and_rollback_smokes_ignore_the_database(tmp_path): + result = _run_smoke(tmp_path, _ready_body(reachable=False), require=None) + + assert result.returncode == 0, result.stderr + + +def test_the_smoke_refuses_an_unknown_database_requirement(tmp_path): + result = _run_smoke(tmp_path, _ready_body(), require="yes") + + assert result.returncode == 2 + assert "REQUIRE_SIMULATION_DATABASE_ACCESS" in result.stderr diff --git a/rules/registry.toml b/rules/registry.toml index 0180a9066..b82db4f23 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -447,7 +447,7 @@ test = "tests/test_deployment_assets_promotion.py::test_rule_proof_staging_promo [[rule]] id = "sim.deploy.smoke-tests-strict" category = "deployment" -statement = "Candidate and stable smoke tests reject a revision mismatch without retrying, and fail if the previous revision does not converge." +statement = "Candidate and stable smoke tests reject a revision mismatch without retrying, and fail if the previous revision does not converge; the candidate smoke also requires the simulation service to reach its database with every table privilege its role needs." rationale = "docs/operations" scope = [".github/scripts/*.sh"] tier = "mechanical" @@ -461,6 +461,9 @@ nodes = [ "tests/test_deployment_assets.py::test_stable_smoke_rejects_an_unexpected_revision_without_retrying", "tests/test_deployment_assets.py::test_stable_smoke_retries_the_previous_revision_until_target_is_served", "tests/test_deployment_assets.py::test_stable_smoke_fails_when_the_previous_revision_does_not_converge", + "tests/test_simulation_database_deployment.py::test_the_candidate_smoke_requires_access_to_the_database", + "tests/test_simulation_database_deployment.py::test_the_smoke_rejects_a_database_the_service_cannot_use", + "tests/test_simulation_database_deployment.py::test_the_smoke_does_not_wait_for_privileges_the_role_lacks", ] [[rule.ci]] workflow = ".github/workflows/pr.yml" From 6b65b46bba95f67de32f5e5e4fdba9a7385daeac Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:17:12 +0400 Subject: [PATCH 08/25] Format the new database test modules Ruff's formatter, at the pinned version, rewraps two calls in the test modules this branch added. No behaviour changes. Co-Authored-By: Claude Opus 5.5 --- libs/policyengine-simulation-persistence/tests/test_engine.py | 4 +++- .../tests/test_simulation_database_deployment.py | 4 +--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libs/policyengine-simulation-persistence/tests/test_engine.py b/libs/policyengine-simulation-persistence/tests/test_engine.py index bb7f87028..daf2d61d1 100644 --- a/libs/policyengine-simulation-persistence/tests/test_engine.py +++ b/libs/policyengine-simulation-persistence/tests/test_engine.py @@ -54,7 +54,9 @@ def capture_engine(url, **options): capture_engine, ) - create_simulation_engine("postgresql://simulation_service:secret@db.example/postgres") + create_simulation_engine( + "postgresql://simulation_service:secret@db.example/postgres" + ) assert captured_options["url"].drivername == "postgresql+psycopg" assert captured_options["connect_args"]["prepare_threshold"] is None diff --git a/projects/policyengine-simulation-entry/tests/test_simulation_database_deployment.py b/projects/policyengine-simulation-entry/tests/test_simulation_database_deployment.py index 8931d7b05..f5743c932 100644 --- a/projects/policyengine-simulation-entry/tests/test_simulation_database_deployment.py +++ b/projects/policyengine-simulation-entry/tests/test_simulation_database_deployment.py @@ -112,9 +112,7 @@ def _ready_body(*, reachable=True, missing_privileges=()): "status": "ready", "simulation_database": { "reachable": reachable, - "missing_privileges": ( - list(missing_privileges) if reachable else None - ), + "missing_privileges": (list(missing_privileges) if reachable else None), }, } ) From 787a6fcb567f740261bdc6fddcc73ddf25c82803 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:26:55 +0400 Subject: [PATCH 09/25] Model the six tables the simulation service writes in the Public API's database The persistence library now describes, with SQLAlchemy Core in service_metadata, the tables the simulation service creates rows in: simulations, reports, simulation_runs, report_runs, report_run_simulation_runs and household_jobs. Each model matches the table the Public API's v2 migration chain builds, column for column (type, nullability, server default), with every constraint and index under the chain's name. Foreign keys stay between these tables, because another service writes the policy, household and catalog rows they name. Status, trigger, role and type columns use StrEnum classes and store each member's value. The models only describe the tables: runtime code never creates or changes them. Tests marked public_api_schema run as the service's role (policyengine_v2_simulation_service) in a database the Public API's chain built with its declared grants, named by PUBLIC_API_SCHEMA_DATABASE_URL: the role lacks none of the privileges the service needs, and creates and reads back a row in every modelled table, in a transaction rolled back afterwards. Without the database they skip. The database-access skill now says that the Public API's migrations own every table this repository uses. Co-Authored-By: Claude Opus 5.5 --- docs/engineering/skills/database-access.md | 26 +- .../README.md | 19 + .../pyproject.toml | 1 + .../__init__.py | 28 ++ .../tables.py | 382 ++++++++++++++++++ .../tests/conftest.py | 49 ++- .../tests/test_service_writes.py | 135 +++++++ .../tests/test_tables.py | 32 ++ 8 files changed, 660 insertions(+), 12 deletions(-) create mode 100644 libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py create mode 100644 libs/policyengine-simulation-persistence/tests/test_service_writes.py create mode 100644 libs/policyengine-simulation-persistence/tests/test_tables.py diff --git a/docs/engineering/skills/database-access.md b/docs/engineering/skills/database-access.md index 1ed83a886..89fc3e13f 100644 --- a/docs/engineering/skills/database-access.md +++ b/docs/engineering/skills/database-access.md @@ -29,17 +29,21 @@ It builds on the Stage 12 rules: ## Who owns the schema -- **Today**, policyengine-api's Alembic migrations own the only tables this - repository uses, the Stage 12 comparison tables - (`sim.stage12.no-migration-chain`). This repository's code reads and - writes their rows, and deployment validation compares their live schema - with its SQLAlchemy mappings; nothing here changes them. -- **Once #847 merges**, this repository's own Alembic chain at `migrations/` - owns the `simulation` schema, and the Stage 12 tables stay with - policyengine-api. - -The rule holds either way: whichever chain owns a table, only that chain's -migrations change it. +policyengine-api's Alembic migrations own every table this repository uses, +and this repository has no migration chain of its own +(`sim.stage12.no-migration-chain`): + +- the Stage 12 comparison tables, whose live schema deployment validation + compares with the Stage 12 library's SQLAlchemy mappings; +- the tables the simulation service writes (`simulations`, `reports`, + `simulation_runs`, `report_runs`, `report_run_simulation_runs` and + `household_jobs`), modelled in `libs/policyengine-simulation-persistence`. + The service connects as its own role, which policyengine-api's privilege + manifest grants. + +This repository's code reads and writes their rows; nothing here changes +them. A change to one of those tables starts in policyengine-api, and the +models here follow it. ## What the guard checks diff --git a/libs/policyengine-simulation-persistence/README.md b/libs/policyengine-simulation-persistence/README.md index 3e29cb093..9dcdcd079 100644 --- a/libs/policyengine-simulation-persistence/README.md +++ b/libs/policyengine-simulation-persistence/README.md @@ -17,6 +17,19 @@ server-side statements, so the same URL works through Supabase's session and transaction poolers. A malformed URL raises `ValueError` without repeating the URL, which holds a password. +## Tables + +`tables.py` models, with SQLAlchemy Core, the six tables the service creates +rows in: `simulations`, `reports`, `simulation_runs`, `report_runs`, +`report_run_simulation_runs` and `household_jobs`, all in `service_metadata`. +Each model matches the table the Public API's v2 migration chain builds: +every column with its type, nullability and server default, and every +constraint and index under the chain's name. The models only describe the +tables; runtime code never creates or changes them. Their foreign keys stay +between these tables, because another service writes the policy, household +and catalog rows they name. Status and other enum columns use the `StrEnum` +classes beside them and store each member's value. + ## The role's privileges `SERVICE_TABLE_PRIVILEGES` lists the table privileges the service's role @@ -36,6 +49,12 @@ uv sync --extra test uv run pytest tests/ ``` +Tests marked `public_api_schema` run as the service's role in a database +the Public API's own v2 migration chain built, with the privileges its +manifest declares granted: set `PUBLIC_API_SCHEMA_DATABASE_URL` to that +database's superuser URL. They create a row in every modelled table and roll +it back. Without the variable they skip. + Tests marked `postgres` need a PostgreSQL server: set `SIMULATION_TEST_DATABASE_URL` to its superuser URL, such as `postgresql://postgres:password@127.0.0.1:5432/postgres`. Each test creates and diff --git a/libs/policyengine-simulation-persistence/pyproject.toml b/libs/policyengine-simulation-persistence/pyproject.toml index ed64ed0f9..ae0ebed92 100644 --- a/libs/policyengine-simulation-persistence/pyproject.toml +++ b/libs/policyengine-simulation-persistence/pyproject.toml @@ -36,6 +36,7 @@ pythonpath = ["src"] testpaths = ["tests"] markers = [ "postgres: needs a disposable PostgreSQL server named by SIMULATION_TEST_DATABASE_URL", + "public_api_schema: needs a database the Public API's migration chain built, named by PUBLIC_API_SCHEMA_DATABASE_URL", ] [tool.pyright] diff --git a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/__init__.py b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/__init__.py index 9d338640b..dfc896fe0 100644 --- a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/__init__.py +++ b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/__init__.py @@ -8,10 +8,38 @@ create_simulation_engine, normalized_database_url, ) +from policyengine_simulation_persistence.tables import ( + HouseholdJobStatus, + ReportRunStatus, + ReportRunTrigger, + SimulationRole, + SimulationStatus, + SimulationType, + household_jobs, + report_run_simulation_runs, + report_runs, + reports, + service_metadata, + simulation_runs, + simulations, +) __all__ = [ "SERVICE_TABLE_PRIVILEGES", + "HouseholdJobStatus", + "ReportRunStatus", + "ReportRunTrigger", + "SimulationRole", + "SimulationStatus", + "SimulationType", "create_simulation_engine", + "household_jobs", "missing_table_privileges", "normalized_database_url", + "report_run_simulation_runs", + "report_runs", + "reports", + "service_metadata", + "simulation_runs", + "simulations", ] diff --git a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py new file mode 100644 index 000000000..4921305a5 --- /dev/null +++ b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py @@ -0,0 +1,382 @@ +"""Models of the Public API's tables the simulation service writes. + +The Public API's v2 migration chain creates and changes these tables; this +module only describes them, so the service can write typed rows. Each table +matches what the chain builds, column for column, with its constraints and +indexes under the same names. Foreign keys stay between these tables: the +policy, household and catalog rows they name are written by another service, +so the simulation service checks them itself. +""" + +from __future__ import annotations + +from enum import StrEnum +from typing import TYPE_CHECKING, Final +from uuid import uuid4 + +from sqlalchemy import ( + JSON, + CheckConstraint, + Column, + DateTime, + Enum, + ForeignKeyConstraint, + Index, + Integer, + MetaData, + PrimaryKeyConstraint, + String, + Table, + Text, + UniqueConstraint, + Uuid, + text, +) + +if TYPE_CHECKING: + from datetime import datetime + from uuid import UUID + +VERSION_LENGTH: Final = 128 +MODAL_CALL_ID_LENGTH: Final = 255 +OUTPUT_URI_LENGTH: Final = 2048 +SHA256_LENGTH: Final = 64 + + +class SimulationType(StrEnum): + """Whether a simulation runs on one household or on a population dataset.""" + + HOUSEHOLD = "household" + ECONOMY = "economy" + + +class SimulationStatus(StrEnum): + """Lifecycle state of a simulation run.""" + + PENDING = "pending" + RUNNING = "running" + SUCCEEDED = "succeeded" + FAILED = "failed" + + +class ReportRunStatus(StrEnum): + """Lifecycle state of a report run.""" + + PENDING = "pending" + RUNNING = "running" + SUCCEEDED = "succeeded" + FAILED = "failed" + + +class ReportRunTrigger(StrEnum): + """What started a report run.""" + + INITIAL = "initial" + MANUAL = "manual" + SYSTEM = "system" + + +class SimulationRole(StrEnum): + """The part a simulation run plays in a report run.""" + + BASELINE = "baseline" + REFORM = "reform" + + +class HouseholdJobStatus(StrEnum): + """Lifecycle state of a household job.""" + + PENDING = "pending" + RUNNING = "running" + SUCCEEDED = "succeeded" + FAILED = "failed" + + +def _enum_type(values: type[StrEnum], name: str) -> Enum: + """Store each member's value, under the Postgres type the chain names.""" + return Enum( + values, + name=name, + values_callable=lambda members: [member.value for member in members], + ) + + +def _identifier() -> Column[UUID]: + return Column("id", Uuid, nullable=False, default=uuid4) + + +def _timestamp(name: str, *, server_now: bool = False) -> Column[datetime]: + """Return a timezone-aware timestamp column, defaulting to now() if asked.""" + if server_now: + return Column( + name, + DateTime(timezone=True), + nullable=False, + server_default=text("now()"), + ) + return Column(name, DateTime(timezone=True), nullable=True) + + +def _terminal_completion(table_name: str) -> CheckConstraint: + """Require a succeeded or failed row to record when it completed.""" + return CheckConstraint( + "status NOT IN ('succeeded', 'failed') OR completed_at IS NOT NULL", + name=f"ck_{table_name}_terminal_completion", + ) + + +def _year_between(table_name: str, *, nullable: bool) -> CheckConstraint: + condition = "year BETWEEN 1900 AND 2200" + if nullable: + condition = f"year IS NULL OR {condition}" + return CheckConstraint(condition, name=f"ck_{table_name}_year") + + +service_metadata = MetaData() + +simulations = Table( + "simulations", + service_metadata, + _identifier(), + _timestamp("created_at", server_now=True), + _timestamp("updated_at", server_now=True), + Column( + "simulation_type", + _enum_type(SimulationType, "v2_simulation_type"), + nullable=False, + ), + Column("dataset_id", Uuid, nullable=True), + Column("household_id", Uuid, nullable=True), + Column("policy_id", Uuid, nullable=True), + Column("dynamic_id", Uuid, nullable=True), + Column("tax_benefit_model_version_id", Uuid, nullable=False), + Column("region_id", Uuid, nullable=True), + Column("filter_field", String(128), nullable=True), + Column("filter_value", String(255), nullable=True), + Column("filter_strategy", String(64), nullable=True), + Column("year", Integer, nullable=True), + PrimaryKeyConstraint("id", name="pk_simulations"), + CheckConstraint( + "(filter_field IS NULL) = (filter_value IS NULL)", + name="ck_simulations_filter_pair", + ), + CheckConstraint( + "(simulation_type = 'household' AND household_id IS NOT NULL " + "AND dataset_id IS NULL) OR " + "(simulation_type = 'economy' AND dataset_id IS NOT NULL " + "AND household_id IS NULL)", + name="ck_simulations_type_input", + ), + _year_between("simulations", nullable=True), + Index( + "ix_simulations_tax_benefit_model_version_id", + "tax_benefit_model_version_id", + ), +) + +reports = Table( + "reports", + service_metadata, + _identifier(), + _timestamp("created_at", server_now=True), + _timestamp("updated_at", server_now=True), + Column("label", String(255), nullable=False), + Column("description", String, nullable=True), + Column("country", String(16), nullable=False), + Column("type", String(128), nullable=True), + Column("user_id", Uuid, nullable=True), + Column("tax_benefit_model_id", Uuid, nullable=False), + Column("policy_id", Uuid, nullable=True), + Column("baseline_simulation_id", Uuid, nullable=True), + Column("reform_simulation_id", Uuid, nullable=True), + Column("household_id", Uuid, nullable=True), + Column("dataset_id", Uuid, nullable=True), + Column("region_id", Uuid, nullable=True), + Column("year", Integer, nullable=True), + Column("inputs", JSON, nullable=False), + PrimaryKeyConstraint("id", name="pk_reports"), + ForeignKeyConstraint( + ["baseline_simulation_id"], + ["simulations.id"], + name="fk_reports_baseline_simulation_id_simulations", + ondelete="SET NULL", + ), + ForeignKeyConstraint( + ["reform_simulation_id"], + ["simulations.id"], + name="fk_reports_reform_simulation_id_simulations", + ondelete="SET NULL", + ), + _year_between("reports", nullable=True), + Index("ix_reports_country_type_created_at", "country", "type", "created_at"), + Index("ix_reports_tax_benefit_model_id", "tax_benefit_model_id"), +) + +simulation_runs = Table( + "simulation_runs", + service_metadata, + _identifier(), + _timestamp("created_at", server_now=True), + _timestamp("updated_at", server_now=True), + Column("simulation_id", Uuid, nullable=False), + Column("tax_benefit_model_version_id", Uuid, nullable=False), + Column("country_package_version", String(VERSION_LENGTH), nullable=False), + Column("policyengine_version", String(VERSION_LENGTH), nullable=False), + Column("dataset_id", Uuid, nullable=True), + Column("region_id", Uuid, nullable=True), + Column( + "status", + _enum_type(SimulationStatus, "v2_simulation_status"), + nullable=False, + ), + Column("modal_call_id", String(MODAL_CALL_ID_LENGTH), nullable=True), + Column("error_message", String, nullable=True), + _timestamp("started_at"), + _timestamp("completed_at"), + Column("household_result", JSON, nullable=True), + Column("output_uri", String(OUTPUT_URI_LENGTH), nullable=True), + Column("output_sha256", String(SHA256_LENGTH), nullable=True), + PrimaryKeyConstraint("id", name="pk_simulation_runs"), + ForeignKeyConstraint( + ["simulation_id"], + ["simulations.id"], + name="fk_simulation_runs_simulation_id_simulations", + ondelete="CASCADE", + ), + CheckConstraint( + "(output_uri IS NULL) = (output_sha256 IS NULL)", + name="ck_simulation_runs_output_pair", + ), + CheckConstraint( + f"output_sha256 IS NULL OR length(output_sha256) = {SHA256_LENGTH}", + name="ck_simulation_runs_output_sha256_length", + ), + _terminal_completion("simulation_runs"), + Index( + "ix_simulation_runs_current_output", + "simulation_id", + "status", + "country_package_version", + "policyengine_version", + "completed_at", + "id", + ), + Index("ix_simulation_runs_status_started_at", "status", "started_at"), +) + +report_runs = Table( + "report_runs", + service_metadata, + _identifier(), + _timestamp("created_at", server_now=True), + _timestamp("updated_at", server_now=True), + Column("report_id", Uuid, nullable=False), + Column("country_package_version", String(VERSION_LENGTH), nullable=False), + Column("policyengine_version", String(VERSION_LENGTH), nullable=False), + Column( + "status", + _enum_type(ReportRunStatus, "v2_report_run_status"), + nullable=False, + ), + Column( + "trigger", + _enum_type(ReportRunTrigger, "v2_report_run_trigger"), + nullable=False, + ), + Column("idempotency_key", Uuid, nullable=True), + _timestamp("started_at"), + _timestamp("completed_at"), + Column("error_message", String, nullable=True), + Column("markdown", Text, nullable=True), + Column("modal_call_id", String(MODAL_CALL_ID_LENGTH), nullable=True), + Column("labor_supply_result", JSON, nullable=True), + Column("cliff_impact_result", JSON, nullable=True), + PrimaryKeyConstraint("id", name="pk_report_runs"), + ForeignKeyConstraint( + ["report_id"], + ["reports.id"], + name="fk_report_runs_report_id_reports", + ondelete="CASCADE", + ), + UniqueConstraint( + "report_id", + "idempotency_key", + name="uq_report_runs_report_idempotency_key", + ), + _terminal_completion("report_runs"), + Index( + "ix_report_runs_current_output", + "report_id", + "status", + "country_package_version", + "policyengine_version", + "completed_at", + "id", + ), + Index("ix_report_runs_status_started_at", "status", "started_at"), +) + +report_run_simulation_runs = Table( + "report_run_simulation_runs", + service_metadata, + _identifier(), + _timestamp("created_at", server_now=True), + Column("report_run_id", Uuid, nullable=False), + Column("simulation_run_id", Uuid, nullable=False), + Column("role", _enum_type(SimulationRole, "v2_simulation_role"), nullable=False), + PrimaryKeyConstraint("id", name="pk_report_run_simulation_runs"), + ForeignKeyConstraint( + ["report_run_id"], + ["report_runs.id"], + name="fk_report_run_simulation_runs_report_run_id_report_runs", + ondelete="CASCADE", + ), + ForeignKeyConstraint( + ["simulation_run_id"], + ["simulation_runs.id"], + name="fk_report_run_simulation_runs_simulation_run_id_simulation_runs", + ondelete="RESTRICT", + ), + UniqueConstraint( + "report_run_id", + "role", + "simulation_run_id", + name="uq_report_run_simulation_runs_member", + ), + Index( + "ix_report_run_simulation_runs_simulation_run_id", + "simulation_run_id", + ), +) + +household_jobs = Table( + "household_jobs", + service_metadata, + _identifier(), + _timestamp("created_at", server_now=True), + Column("country", String(16), nullable=False), + Column("request_data", JSON, nullable=False), + Column("policy_id", Uuid, nullable=True), + Column("dynamic_id", Uuid, nullable=True), + Column( + "status", + _enum_type(HouseholdJobStatus, "v2_household_job_status"), + nullable=False, + ), + Column("error_message", String, nullable=True), + Column("result", JSON, nullable=True), + _timestamp("started_at"), + _timestamp("completed_at"), + _timestamp("updated_at", server_now=True), + Column("household_id", Uuid, nullable=False), + Column("year", Integer, nullable=False), + Column("tax_benefit_model_version_id", Uuid, nullable=False), + Column("country_package_version", String(VERSION_LENGTH), nullable=False), + Column("policyengine_version", String(VERSION_LENGTH), nullable=False), + Column("modal_call_id", String(MODAL_CALL_ID_LENGTH), nullable=True), + PrimaryKeyConstraint("id", name="pk_household_jobs"), + _terminal_completion("household_jobs"), + _year_between("household_jobs", nullable=False), + Index("ix_household_jobs_household_id", "household_id"), + Index("ix_household_jobs_status_created_at", "status", "created_at"), +) diff --git a/libs/policyengine-simulation-persistence/tests/conftest.py b/libs/policyengine-simulation-persistence/tests/conftest.py index bac706e37..7412a6dc4 100644 --- a/libs/policyengine-simulation-persistence/tests/conftest.py +++ b/libs/policyengine-simulation-persistence/tests/conftest.py @@ -1,31 +1,46 @@ -"""Disposable PostgreSQL databases for the tests that need one. +"""PostgreSQL databases for the tests that need one. Tests marked ``postgres`` need a PostgreSQL server whose superuser URL is in ``SIMULATION_TEST_DATABASE_URL``. Each test gets a new, empty database and a login role that holds no privilege on it until the test grants one. Without the variable those tests skip, unless ``SIMULATION_REQUIRE_DATABASE_TESTS=1`` (set in CI), where they fail instead. + +Tests marked ``public_api_schema`` need a database the Public API's own v2 +migration chain built, with the privileges its manifest declares granted +(``.github/scripts/public-api-schema-reference.sh`` builds one). Its +superuser URL is in ``PUBLIC_API_SCHEMA_DATABASE_URL``. Without it those +tests skip, unless ``SIMULATION_REQUIRE_PUBLIC_API_SCHEMA=1``. """ from __future__ import annotations import os +from contextlib import contextmanager from dataclasses import dataclass from typing import TYPE_CHECKING from uuid import uuid4 import pytest +from policyengine_simulation_persistence import create_simulation_engine from sqlalchemy import create_engine, text from sqlalchemy.engine import URL, make_url if TYPE_CHECKING: from collections.abc import Iterator + from sqlalchemy import Connection + DATABASE_URL_VARIABLE = "SIMULATION_TEST_DATABASE_URL" REQUIRE_DATABASE_VARIABLE = "SIMULATION_REQUIRE_DATABASE_TESTS" # A login role standing in for the simulation service's role. SERVICE_TEST_ROLE = "simulation_service_test" SERVICE_TEST_ROLE_PASSWORD = "simulation_service_test" +PUBLIC_API_DATABASE_VARIABLE = "PUBLIC_API_SCHEMA_DATABASE_URL" +REQUIRE_PUBLIC_API_VARIABLE = "SIMULATION_REQUIRE_PUBLIC_API_SCHEMA" +# The role the Public API's privilege manifest manages for the simulation +# service; the deployed SIMULATION_DATABASE_URL logs in as it. +SERVICE_ROLE = "policyengine_v2_simulation_service" @dataclass(frozen=True) @@ -85,3 +100,35 @@ def disposable_database(postgres_server_url: URL) -> Iterator[DisposableDatabase text(f'DROP DATABASE IF EXISTS "{database_name}" WITH (FORCE)') ) server_engine.dispose() + + +@pytest.fixture +def public_api_database_url() -> str: + """Return the URL of the database the Public API's chain built, or skip.""" + configured = os.environ.get(PUBLIC_API_DATABASE_VARIABLE, "") + if not configured: + message = f"{PUBLIC_API_DATABASE_VARIABLE} names no Public API database" + if os.environ.get(REQUIRE_PUBLIC_API_VARIABLE) == "1": + pytest.fail(message) + pytest.skip(message) + return configured + + +@contextmanager +def connection_as_the_service(database_url: str) -> Iterator[Connection]: + """Yield a connection acting as the service's role, rolled back afterwards. + + The manifest's roles may have no login in a test database, so the + superuser connects and takes the role for the transaction alone. + """ + engine = create_simulation_engine(database_url) + try: + with engine.connect() as connection: + transaction = connection.begin() + try: + connection.execute(text(f"SET LOCAL ROLE {SERVICE_ROLE}")) + yield connection + finally: + transaction.rollback() + finally: + engine.dispose() diff --git a/libs/policyengine-simulation-persistence/tests/test_service_writes.py b/libs/policyengine-simulation-persistence/tests/test_service_writes.py new file mode 100644 index 000000000..783edf17d --- /dev/null +++ b/libs/policyengine-simulation-persistence/tests/test_service_writes.py @@ -0,0 +1,135 @@ +"""The simulation service's role writes its tables in the Public API's database. + +These tests run against a database the Public API's own v2 migration chain +built, with the privileges its manifest declares granted +(``PUBLIC_API_SCHEMA_DATABASE_URL``; see ``conftest``). Each test works in +one transaction as the service's role and rolls it back, so the database +stays as the chain built it. +""" + +from __future__ import annotations + +from uuid import uuid4 + +import pytest +from conftest import connection_as_the_service +from policyengine_simulation_persistence import ( + HouseholdJobStatus, + ReportRunStatus, + ReportRunTrigger, + SimulationRole, + SimulationStatus, + SimulationType, + household_jobs, + missing_table_privileges, + report_run_simulation_runs, + report_runs, + reports, + simulation_runs, + simulations, +) +from sqlalchemy import func, insert, select + +pytestmark = pytest.mark.public_api_schema + +COUNTRY_PACKAGE_VERSION = "1.402.0" +POLICYENGINE_VERSION = "3.4.0" + + +def test_the_service_role_holds_every_privilege_the_service_needs( + public_api_database_url, +) -> None: + with connection_as_the_service(public_api_database_url) as connection: + assert missing_table_privileges(connection) == [] + + +def test_the_service_role_creates_and_reads_a_row_in_every_table_it_writes( + public_api_database_url, +) -> None: + model_version_id = uuid4() + with connection_as_the_service(public_api_database_url) as connection: + simulation_id = connection.execute( + insert(simulations) + .values( + simulation_type=SimulationType.ECONOMY, + dataset_id=uuid4(), + tax_benefit_model_version_id=model_version_id, + year=2026, + ) + .returning(simulations.c.id) + ).scalar_one() + report_id = connection.execute( + insert(reports) + .values( + label="Test report", + country="us", + tax_benefit_model_id=uuid4(), + baseline_simulation_id=simulation_id, + year=2026, + inputs={}, + ) + .returning(reports.c.id) + ).scalar_one() + simulation_run_id = connection.execute( + insert(simulation_runs) + .values( + simulation_id=simulation_id, + tax_benefit_model_version_id=model_version_id, + country_package_version=COUNTRY_PACKAGE_VERSION, + policyengine_version=POLICYENGINE_VERSION, + status=SimulationStatus.PENDING, + ) + .returning(simulation_runs.c.id) + ).scalar_one() + report_run_id = connection.execute( + insert(report_runs) + .values( + report_id=report_id, + country_package_version=COUNTRY_PACKAGE_VERSION, + policyengine_version=POLICYENGINE_VERSION, + status=ReportRunStatus.PENDING, + trigger=ReportRunTrigger.INITIAL, + ) + .returning(report_runs.c.id) + ).scalar_one() + connection.execute( + insert(report_run_simulation_runs).values( + report_run_id=report_run_id, + simulation_run_id=simulation_run_id, + role=SimulationRole.BASELINE, + ) + ) + connection.execute( + insert(household_jobs).values( + country="us", + request_data={}, + status=HouseholdJobStatus.PENDING, + household_id=uuid4(), + year=2026, + tax_benefit_model_version_id=model_version_id, + country_package_version=COUNTRY_PACKAGE_VERSION, + policyengine_version=POLICYENGINE_VERSION, + ) + ) + + assert ( + connection.execute( + select(simulation_runs.c.status).where( + simulation_runs.c.id == simulation_run_id + ) + ).scalar_one() + == SimulationStatus.PENDING + ) + assert ( + connection.execute( + select(report_run_simulation_runs.c.role).where( + report_run_simulation_runs.c.report_run_id == report_run_id + ) + ).scalar_one() + == SimulationRole.BASELINE + ) + for table in (simulations, reports, report_runs, household_jobs): + assert ( + connection.execute(select(func.count()).select_from(table)).scalar_one() + >= 1 + ), table.name diff --git a/libs/policyengine-simulation-persistence/tests/test_tables.py b/libs/policyengine-simulation-persistence/tests/test_tables.py new file mode 100644 index 000000000..2c3bb5235 --- /dev/null +++ b/libs/policyengine-simulation-persistence/tests/test_tables.py @@ -0,0 +1,32 @@ +"""Sim's models of the Public API's tables the simulation service writes.""" + +from __future__ import annotations + +from policyengine_simulation_persistence import ( + SERVICE_TABLE_PRIVILEGES, + service_metadata, +) + + +def test_the_models_describe_every_table_the_service_creates_rows_in() -> None: + created_tables = { + table_name + for table_name, privileges in SERVICE_TABLE_PRIVILEGES.items() + if "INSERT" in privileges + } + + assert set(service_metadata.tables) == created_tables + + +def test_foreign_keys_stay_between_the_services_own_tables() -> None: + """Another service writes the policy, household and catalog rows.""" + for table in service_metadata.tables.values(): + for foreign_key in table.foreign_keys: + assert foreign_key.column.table.name in service_metadata.tables, ( + f"{table.name}.{foreign_key.parent.name}" + ) + + +def test_every_table_lives_in_the_public_schema() -> None: + for table in service_metadata.tables.values(): + assert table.schema is None, table.name From d4690fb76a3a9834212f3c3f9dfdf4fa20f9a5af Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:27:05 +0400 Subject: [PATCH 10/25] Add a changelog fragment for the simulation service's database connection Co-Authored-By: Claude Opus 5.5 --- changelog.d/897.added.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/897.added.md diff --git a/changelog.d/897.added.md b/changelog.d/897.added.md new file mode 100644 index 000000000..4048c3ab2 --- /dev/null +++ b/changelog.d/897.added.md @@ -0,0 +1 @@ +The Simulation Entrypoint connects to the Public API's database with its own role, mounted as `SIMULATION_DATABASE_URL` on every deploy, and models the tables it writes there (simulations, reports, their runs and household jobs). `GET /ready` reports whether it reached the database and which table privileges its role lacks, and the candidate check requires none missing. Every runtime secret is now resolved in its own assignment, so one that cannot be resolved stops the deploy. From 9d69106700372b8afd4cae678eaeb91e082fa910 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:46:13 +0400 Subject: [PATCH 11/25] Name a missing USAGE privilege on the public schema instead of reporting the database unreachable missing_table_privileges found each table with to_regclass, which raises "permission denied for schema" for a role without USAGE on public. The readiness check caught that as a failure to reach the database, so a reached database with a grant missing was reported as unreachable, and the candidate check asked again six times before failing with a message about connectivity. Tables are now found through PostgreSQL's catalog (pg_class and pg_namespace), which any role may read, and a missing USAGE privilege on the schema is named first, as "USAGE on schema public". The rule's statement says so, and the new test is one of its enforcers. Co-Authored-By: Claude Opus 5.5 --- docs/generated/rules.md | 2 +- .../README.md | 7 ++- .../access.py | 52 +++++++++++++++---- .../tests/test_access.py | 12 +++++ rules/registry.toml | 3 +- 5 files changed, 63 insertions(+), 13 deletions(-) diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 880bcf468..73bfe7829 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -140,7 +140,7 @@ flowchart LR | Rule | Statement | Tier | Status | Blocking | Enforced by | CI | Proof | | --- | --- | --- | --- | --- | --- | --- | --- | -| `sim.persistence.missing-privileges-reported` | The simulation service's readiness check names every table privilege its role needs on the Public API's tables and lacks, as PostgreSQL reports it, and counts a table missing from the public schema as lacking all of them. | mechanical | enforced | yes | pytest `libs/policyengine-simulation-persistence/tests/test_access.py` | `pr.yml:simulation-database` | yes | +| `sim.persistence.missing-privileges-reported` | The simulation service's privilege check, which its readiness check reports, names every table privilege its role needs on the Public API's tables and lacks, as PostgreSQL reports it, counts a table missing from the public schema as lacking all of them, and names a missing USAGE privilege on that schema. | mechanical | enforced | yes | pytest `libs/policyengine-simulation-persistence/tests/test_access.py` | `pr.yml:simulation-database` | yes | ```mermaid flowchart LR diff --git a/libs/policyengine-simulation-persistence/README.md b/libs/policyengine-simulation-persistence/README.md index 9dcdcd079..cd636e060 100644 --- a/libs/policyengine-simulation-persistence/README.md +++ b/libs/policyengine-simulation-persistence/README.md @@ -39,8 +39,11 @@ needs: `SELECT` and `INSERT` on the tables it creates rows in policy, household and catalog tables a run reads. The Public API's privilege manifest grants them. `missing_table_privileges` returns each one the connected role lacks, as `"INSERT on reports"`; a table missing from the -`public` schema lacks all of its privileges. The Simulation Entrypoint's -readiness check reports the result. +`public` schema lacks all of its privileges, and a role without the `USAGE` +privilege on that schema is told so first (`"USAGE on schema public"`), since +it can use none of its table grants. Tables are found through PostgreSQL's +catalog, which needs no privilege. The Simulation Entrypoint's readiness +check reports the result. ## Tests diff --git a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/access.py b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/access.py index ac179f444..2a9ddef9c 100644 --- a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/access.py +++ b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/access.py @@ -11,7 +11,7 @@ from types import MappingProxyType from typing import TYPE_CHECKING, Final -from sqlalchemy import String, column, false, func, select, values +from sqlalchemy import String, and_, column, false, func, select, table, values if TYPE_CHECKING: from collections.abc import Mapping @@ -19,6 +19,19 @@ from sqlalchemy import Connection TABLE_SCHEMA: Final = "public" +# PostgreSQL's catalog, which any role may read: a table is found by name +# without the USAGE privilege on its schema that to_regclass would need. +_pg_class = table( + "pg_class", + column("oid"), + column("relname"), + column("relnamespace"), + column("relkind"), + schema="pg_catalog", +) +_pg_namespace = table( + "pg_namespace", column("oid"), column("nspname"), schema="pg_catalog" +) _CREATE: Final = frozenset({"SELECT", "INSERT"}) _READ: Final = frozenset({"SELECT"}) @@ -48,7 +61,9 @@ def missing_table_privileges(connection: Connection) -> list[str]: """Return each required privilege the connected role lacks, ordered by table. Each is named as ``" on
"``. A table missing from the - ``public`` schema lacks every privilege the service needs on it. + ``public`` schema lacks every privilege the service needs on it. A role + without the USAGE privilege on that schema can't use any table grant, so + ``"USAGE on schema public"`` comes first. """ required = values( column("table_name", String), @@ -61,18 +76,37 @@ def missing_table_privileges(connection: Connection) -> list[str]: for privilege in sorted(privileges) ] ) - # to_regclass returns null for a missing table, and has_table_privilege - # returns null for a null table, so a missing table counts as not granted. + public_tables = ( + select(_pg_class.c.oid, _pg_class.c.relname) + .join(_pg_namespace, _pg_namespace.c.oid == _pg_class.c.relnamespace) + .where( + and_( + _pg_namespace.c.nspname == TABLE_SCHEMA, + _pg_class.c.relkind.in_(["r", "p"]), + ) + ) + .subquery("public_tables") + ) + # A missing table joins to a null oid, and has_table_privilege returns + # null for a null table, so a missing table counts as not granted. granted = func.coalesce( - func.has_table_privilege( - func.to_regclass(func.concat(f"{TABLE_SCHEMA}.", required.c.table_name)), - required.c.privilege, - ), + func.has_table_privilege(public_tables.c.oid, required.c.privilege), false(), ) rows = connection.execute( select(required.c.table_name, required.c.privilege) + .select_from( + required.outerjoin( + public_tables, public_tables.c.relname == required.c.table_name + ) + ) .where(~granted) .order_by(required.c.table_name, required.c.privilege) ) - return [f"{privilege} on {table_name}" for table_name, privilege in rows] + missing = [f"{privilege} on {table_name}" for table_name, privilege in rows] + schema_usage = connection.execute( + select(func.has_schema_privilege(TABLE_SCHEMA, "USAGE")) + ).scalar_one() + if not schema_usage: + missing.insert(0, f"USAGE on schema {TABLE_SCHEMA}") + return missing diff --git a/libs/policyengine-simulation-persistence/tests/test_access.py b/libs/policyengine-simulation-persistence/tests/test_access.py index 62054a1a3..aca0997f7 100644 --- a/libs/policyengine-simulation-persistence/tests/test_access.py +++ b/libs/policyengine-simulation-persistence/tests/test_access.py @@ -208,3 +208,15 @@ def test_rule_proof_missing_privileges_reported( ) assert _missing_privileges_as_the_service(disposable_database) == expected_missing + + +@pytest.mark.postgres +def test_a_role_without_usage_on_the_schema_is_told_so(disposable_database): + """Without USAGE on ``public``, the role can't use its table grants.""" + _create_tables(disposable_database, SERVICE_TABLE_PRIVILEGES) + _grant_every_required_privilege(disposable_database) + _grant(disposable_database, ["REVOKE USAGE ON SCHEMA public FROM PUBLIC"]) + + assert _missing_privileges_as_the_service(disposable_database) == [ + "USAGE on schema public", + ] diff --git a/rules/registry.toml b/rules/registry.toml index b82db4f23..df88f12d2 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -142,7 +142,7 @@ test = "tests/test_database_access_boundaries.py::test_rule_proof_sqlalchemy_and [[rule]] id = "sim.persistence.missing-privileges-reported" category = "persistence" -statement = "The simulation service's readiness check names every table privilege its role needs on the Public API's tables and lacks, as PostgreSQL reports it, and counts a table missing from the public schema as lacking all of them." +statement = "The simulation service's privilege check, which its readiness check reports, names every table privilege its role needs on the Public API's tables and lacks, as PostgreSQL reports it, counts a table missing from the public schema as lacking all of them, and names a missing USAGE privilege on that schema." rationale = "libs/policyengine-simulation-persistence/README.md#the-roles-privileges" scope = ["libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/access.py"] tier = "mechanical" @@ -156,6 +156,7 @@ nodes = [ "tests/test_access.py::test_each_missing_privilege_is_named", "tests/test_access.py::test_a_missing_table_lacks_every_privilege", "tests/test_access.py::test_only_the_public_schema_counts", + "tests/test_access.py::test_a_role_without_usage_on_the_schema_is_told_so", ] [[rule.ci]] workflow = ".github/workflows/pr.yml" From cd47675fcd1cd11bafc5e93ff3cafae6f4a2bb37 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:47:06 +0400 Subject: [PATCH 12/25] Validate the simulation database URL with the persistence library's own check Settings._validate_simulation_database checked only the URL's scheme, while the engine also refuses a URL without a user name, host or database, or with surrounding spaces. The settings now ask normalized_database_url, so they accept exactly the URLs the engine does, and say why one is invalid without repeating it. Co-Authored-By: Claude Opus 5.5 --- .../policyengine_simulation_entry/config.py | 19 +++++++++++++------ .../tests/test_config.py | 4 ++++ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/config.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/config.py index e9c7af833..bf12af7d8 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/config.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/config.py @@ -11,6 +11,7 @@ V2_VERSION_MANIFEST_NAME, assert_separate_manifest_names, ) +from policyengine_simulation_persistence import normalized_database_url PRODUCTION_ENVIRONMENTS = frozenset({"main", "prod", "production"}) MODAL_GATEWAY_HOST_SUFFIX = ".modal.run" @@ -275,9 +276,15 @@ def _validate_stage12(self) -> None: raise ConfigurationError(msg) def _validate_simulation_database(self) -> None: - """Require the canonical scheme for the service role's URL, when set.""" - if self.simulation_database_url and not self.simulation_database_url.startswith( - "postgresql://" - ): - msg = "SIMULATION_DATABASE_URL must use the canonical postgresql:// scheme." - raise ConfigurationError(msg) + """Require a complete canonical URL for the service role, when set. + + The persistence library's own check decides, so the settings accept + exactly the URLs the engine does; its message never repeats the URL. + """ + if not self.simulation_database_url: + return + try: + normalized_database_url(self.simulation_database_url) + except ValueError as error: + msg = f"SIMULATION_DATABASE_URL is invalid: {error}" + raise ConfigurationError(msg) from None diff --git a/projects/policyengine-simulation-entry/tests/test_config.py b/projects/policyengine-simulation-entry/tests/test_config.py index 171b00ebc..768462633 100644 --- a/projects/policyengine-simulation-entry/tests/test_config.py +++ b/projects/policyengine-simulation-entry/tests/test_config.py @@ -328,6 +328,10 @@ def test_the_simulation_database_url_is_read_from_the_environment(monkeypatch): "postgres://simulation_service:secret@db.example/postgres", "postgresql+psycopg://simulation_service:secret@db.example/postgres", "mysql://simulation_service:secret@db.example/postgres", + # The engine refuses these too, so the settings must. + "postgresql://db.example/postgres", + "postgresql://simulation_service:secret@db.example", + " postgresql://simulation_service:secret@db.example/postgres", ], ) def test_the_simulation_database_url_must_be_canonical(database_url): From c8633651cee73ec0bb29e4156fe97a77c0a2bc74 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:47:48 +0400 Subject: [PATCH 13/25] Keep the Cloud Run deploy environment helper in the deployment test support module The database deployment tests imported cloud_run_deploy_environment from another test module. It now lives in test_deployment_assets_support with the other shared deployment helpers, and both modules import it from there. No test changes what it checks. Co-Authored-By: Claude Opus 5.5 --- .../tests/test_deployment_assets_cloud_run.py | 84 +------------------ .../tests/test_deployment_assets_support.py | 81 ++++++++++++++++++ .../test_simulation_database_deployment.py | 2 +- 3 files changed, 83 insertions(+), 84 deletions(-) diff --git a/projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py b/projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py index db1513b71..8ce6dccb4 100644 --- a/projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py +++ b/projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py @@ -3,10 +3,8 @@ from __future__ import annotations import json -import os import re import subprocess -import textwrap from pathlib import Path import pytest @@ -18,6 +16,7 @@ REPOSITORY_ROOT, REUSABLE_DEPLOY_WORKFLOW, STAGE12_DEPLOY_WORKFLOW, + cloud_run_deploy_environment, ) @@ -41,87 +40,6 @@ def test_cloud_run_deployment_does_not_interpolate_github_values_in_shell(): assert ":latest" not in deployment_script -def cloud_run_deploy_environment(tmp_path: Path) -> dict[str, str]: - """Return a complete deployment environment backed by a fake gcloud.""" - fake_bin = tmp_path / "bin" - fake_bin.mkdir() - fake_gcloud = fake_bin / "gcloud" - fake_gcloud.write_text( - textwrap.dedent( - """\ - #!/usr/bin/env bash - set -euo pipefail - - if [[ "$1 $2 $3" == "secrets versions list" ]]; then - printf '%s\n' 1 3 2 - exit 0 - fi - - if [[ "$1 $2" == "run deploy" ]]; then - printf '%s\n' "$@" > "${GCLOUD_ARGUMENTS_CAPTURE}" - while (($#)); do - if [[ "$1" == "--env-vars-file" ]]; then - cp "$2" "${GCLOUD_ENV_FILE_CAPTURE}" - exit 0 - fi - shift - done - fi - - printf 'unexpected gcloud invocation\n' >&2 - exit 1 - """ - ), - encoding="utf-8", - ) - fake_gcloud.chmod(0o755) - environment = { - name: value - for name, value in os.environ.items() - if name != "SIMULATION_ENTRYPOINT_ENFORCE_PERMISSIONS_VALUE" - } - return environment | { - "PATH": f"{fake_bin}:{os.environ['PATH']}", - "RUNNER_TEMP": str(tmp_path), - "GCLOUD_ARGUMENTS_CAPTURE": str(tmp_path / "gcloud-arguments.txt"), - "GCLOUD_ENV_FILE_CAPTURE": str(tmp_path / "runtime-environment.json"), - "PROJECT_ID": "policyengine-staging", - "REGION": "us-central1", - "IMAGE": "us-central1-docker.pkg.dev/project/repository/image:sha", - "TAG": "s-123", - "DEPLOY_STAGE12_V2": "true", - "APP_ENVIRONMENT": "staging", - "MODAL_ENVIRONMENT": "staging", - "ENTRYPOINT_SERVICE": "policyengine-simulation-entry-staging", - "ENTRYPOINT_RUNTIME_SERVICE_ACCOUNT": ( - "simulation-entry@policyengine-staging.iam.gserviceaccount.com" - ), - "ENTRYPOINT_MIN_INSTANCES": "0", - "ENTRYPOINT_MAX_INSTANCES": "10", - "SIMULATION_ENTRYPOINT_AUTH_ISSUER_VALUE": ( - "https://issuer.example/path?literal=$(not-a-command)" - ), - "SIMULATION_ENTRYPOINT_AUTH_AUDIENCE_VALUE": "simulation-api", - "OLD_GATEWAY_URL_VALUE": "https://legacy.example", - "OLD_GATEWAY_AUTH_ISSUER_VALUE": "https://issuer.example", - "OLD_GATEWAY_AUTH_AUDIENCE_VALUE": "legacy-api", - "OLD_GATEWAY_AUTH_CLIENT_ID_VALUE": "entrypoint-client", - "OLD_GATEWAY_AUTH_CLIENT_SECRET_SECRET_NAME": "old-client-secret", - "OBSERVABILITY_SERVICE_NAMESPACE": "policyengine.simulation-entry", - "OBSERVABILITY_TRACE_PROJECT_ID": "policyengine-observability", - "OTEL_EXPORTER_OTLP_ENDPOINT": "https://collector.example", - "POLICYENGINE_OTEL_GOOGLE_AUDIENCE": "https://collector.example", - "STAGE12_ENABLED_VALUE": "0", - "STAGE12_ARTIFACT_BUCKET_VALUE": "policyengine-stage12-staging", - "STAGE12_DATABASE_URL_SECRET_NAME": "stage12-staging-database-url", - "MODAL_TOKEN_ID_SECRET_NAME": "modal-token-id", - "MODAL_TOKEN_SECRET_SECRET_NAME": "modal-token-secret", - "SIMULATION_DATABASE_URL_SECRET_NAME": ( - "simulation-staging-service-database-url" - ), - } - - def test_cloud_run_deployment_escapes_environment_and_pins_secret_versions(tmp_path): command_environment = cloud_run_deploy_environment(tmp_path) arguments_capture = Path(command_environment["GCLOUD_ARGUMENTS_CAPTURE"]) diff --git a/projects/policyengine-simulation-entry/tests/test_deployment_assets_support.py b/projects/policyengine-simulation-entry/tests/test_deployment_assets_support.py index 2c24724d3..7996ca62e 100644 --- a/projects/policyengine-simulation-entry/tests/test_deployment_assets_support.py +++ b/projects/policyengine-simulation-entry/tests/test_deployment_assets_support.py @@ -438,3 +438,84 @@ def workflow_jobs(path: Path) -> dict: import yaml return yaml.safe_load(path.read_text(encoding="utf-8"))["jobs"] + + +def cloud_run_deploy_environment(tmp_path: Path) -> dict[str, str]: + """Return a complete deployment environment backed by a fake gcloud.""" + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + fake_gcloud = fake_bin / "gcloud" + fake_gcloud.write_text( + textwrap.dedent( + """\ + #!/usr/bin/env bash + set -euo pipefail + + if [[ "$1 $2 $3" == "secrets versions list" ]]; then + printf '%s\n' 1 3 2 + exit 0 + fi + + if [[ "$1 $2" == "run deploy" ]]; then + printf '%s\n' "$@" > "${GCLOUD_ARGUMENTS_CAPTURE}" + while (($#)); do + if [[ "$1" == "--env-vars-file" ]]; then + cp "$2" "${GCLOUD_ENV_FILE_CAPTURE}" + exit 0 + fi + shift + done + fi + + printf 'unexpected gcloud invocation\n' >&2 + exit 1 + """ + ), + encoding="utf-8", + ) + fake_gcloud.chmod(0o755) + environment = { + name: value + for name, value in os.environ.items() + if name != "SIMULATION_ENTRYPOINT_ENFORCE_PERMISSIONS_VALUE" + } + return environment | { + "PATH": f"{fake_bin}:{os.environ['PATH']}", + "RUNNER_TEMP": str(tmp_path), + "GCLOUD_ARGUMENTS_CAPTURE": str(tmp_path / "gcloud-arguments.txt"), + "GCLOUD_ENV_FILE_CAPTURE": str(tmp_path / "runtime-environment.json"), + "PROJECT_ID": "policyengine-staging", + "REGION": "us-central1", + "IMAGE": "us-central1-docker.pkg.dev/project/repository/image:sha", + "TAG": "s-123", + "DEPLOY_STAGE12_V2": "true", + "APP_ENVIRONMENT": "staging", + "MODAL_ENVIRONMENT": "staging", + "ENTRYPOINT_SERVICE": "policyengine-simulation-entry-staging", + "ENTRYPOINT_RUNTIME_SERVICE_ACCOUNT": ( + "simulation-entry@policyengine-staging.iam.gserviceaccount.com" + ), + "ENTRYPOINT_MIN_INSTANCES": "0", + "ENTRYPOINT_MAX_INSTANCES": "10", + "SIMULATION_ENTRYPOINT_AUTH_ISSUER_VALUE": ( + "https://issuer.example/path?literal=$(not-a-command)" + ), + "SIMULATION_ENTRYPOINT_AUTH_AUDIENCE_VALUE": "simulation-api", + "OLD_GATEWAY_URL_VALUE": "https://legacy.example", + "OLD_GATEWAY_AUTH_ISSUER_VALUE": "https://issuer.example", + "OLD_GATEWAY_AUTH_AUDIENCE_VALUE": "legacy-api", + "OLD_GATEWAY_AUTH_CLIENT_ID_VALUE": "entrypoint-client", + "OLD_GATEWAY_AUTH_CLIENT_SECRET_SECRET_NAME": "old-client-secret", + "OBSERVABILITY_SERVICE_NAMESPACE": "policyengine.simulation-entry", + "OBSERVABILITY_TRACE_PROJECT_ID": "policyengine-observability", + "OTEL_EXPORTER_OTLP_ENDPOINT": "https://collector.example", + "POLICYENGINE_OTEL_GOOGLE_AUDIENCE": "https://collector.example", + "STAGE12_ENABLED_VALUE": "0", + "STAGE12_ARTIFACT_BUCKET_VALUE": "policyengine-stage12-staging", + "STAGE12_DATABASE_URL_SECRET_NAME": "stage12-staging-database-url", + "MODAL_TOKEN_ID_SECRET_NAME": "modal-token-id", + "MODAL_TOKEN_SECRET_SECRET_NAME": "modal-token-secret", + "SIMULATION_DATABASE_URL_SECRET_NAME": ( + "simulation-staging-service-database-url" + ), + } diff --git a/projects/policyengine-simulation-entry/tests/test_simulation_database_deployment.py b/projects/policyengine-simulation-entry/tests/test_simulation_database_deployment.py index f5743c932..476ad0bd2 100644 --- a/projects/policyengine-simulation-entry/tests/test_simulation_database_deployment.py +++ b/projects/policyengine-simulation-entry/tests/test_simulation_database_deployment.py @@ -12,11 +12,11 @@ from pathlib import Path import pytest -from test_deployment_assets_cloud_run import cloud_run_deploy_environment from test_deployment_assets_support import ( CLOUD_RUN_DEPLOY_SCRIPT, REUSABLE_DEPLOY_WORKFLOW, SMOKE_SCRIPT, + cloud_run_deploy_environment, smoke_test_env, workflow_jobs, write_fake_smoke_curl, From 1f7d5eb82ef21f18d33de5d5630014c24bc99706 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:48:02 +0400 Subject: [PATCH 14/25] Correct documentation the reworked database connection left behind The OpenAPI test's comment still spoke of the database's revision, the persistence tests' conftest named a script this change does not add, and AGENTS.md's summary of PR CI did not mention the PostgreSQL tests. The deployment document now says what each environment needs, and in which order, before the first deploy that carries the connection. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 7 ++++--- .../tests/conftest.py | 7 ++++--- projects/policyengine-simulation-entry/DEPLOYMENT.md | 9 +++++++++ .../policyengine-simulation-entry/tests/test_openapi.py | 2 +- 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 959f144ec..cc4a77446 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -91,9 +91,10 @@ same-repository draft PR. - The simulation service lives in `projects/policyengine-simulation-executor`. - API integration tests live in `projects/policyengine-apis-integ`. -- PR CI runs simulation unit tests, Ruff format and lint checks, Pyright for - every package, quality guards (including the rules registry), the changelog - fragment check, Docker build, and local integration tests. A separate +- PR CI runs simulation unit tests, the persistence tests that need + PostgreSQL, Ruff format and lint checks, Pyright for every package, quality + guards (including the rules registry), the changelog fragment check, Docker + build, and local integration tests. A separate `PR description` job checks that every open PR's description starts with `Fixes #ISSUE_NUMBER` for an open issue in this repository. - Every PR adds a changelog fragment, `changelog.d/..md`, and never diff --git a/libs/policyengine-simulation-persistence/tests/conftest.py b/libs/policyengine-simulation-persistence/tests/conftest.py index 7412a6dc4..b1aa8306e 100644 --- a/libs/policyengine-simulation-persistence/tests/conftest.py +++ b/libs/policyengine-simulation-persistence/tests/conftest.py @@ -8,9 +8,10 @@ Tests marked ``public_api_schema`` need a database the Public API's own v2 migration chain built, with the privileges its manifest declares granted -(``.github/scripts/public-api-schema-reference.sh`` builds one). Its -superuser URL is in ``PUBLIC_API_SCHEMA_DATABASE_URL``. Without it those -tests skip, unless ``SIMULATION_REQUIRE_PUBLIC_API_SCHEMA=1``. +(run its ``alembic -c alembic-v2.ini upgrade head`` and +``scripts/reconcile_v2_privileges.py apply --create-missing-roles`` against +it). Its superuser URL is in ``PUBLIC_API_SCHEMA_DATABASE_URL``. Without it +those tests skip, unless ``SIMULATION_REQUIRE_PUBLIC_API_SCHEMA=1``. """ from __future__ import annotations diff --git a/projects/policyengine-simulation-entry/DEPLOYMENT.md b/projects/policyengine-simulation-entry/DEPLOYMENT.md index 53f979754..6e76be588 100644 --- a/projects/policyengine-simulation-entry/DEPLOYMENT.md +++ b/projects/policyengine-simulation-entry/DEPLOYMENT.md @@ -39,6 +39,15 @@ the other runtime secrets, and only the entrypoint's runtime service account may read it. A missing or invalid name, or a secret with no enabled version, stops the deploy before `gcloud run deploy` runs. +So before the first deploy that carries this connection, each environment +needs, in order: the Public API's tables the service writes, migrated in that +environment's database; the service's login role, with the grants the Public +API's privilege manifest declares; the role's URL stored in Secret Manager, +readable by the runtime service account; and the `beta` or `prod` +`SIMULATION_DATABASE_URL_SECRET_NAME` variable naming it. Without the +variable the deploy stops at once; without the tables, the role or its grants +the candidate check below fails. + `GET /ready` reports, under `simulation_database`, whether the entrypoint reached the database and which table privileges its role lacks (`missing_privileges`, such as `"INSERT on reports"`; the list is in diff --git a/projects/policyengine-simulation-entry/tests/test_openapi.py b/projects/policyengine-simulation-entry/tests/test_openapi.py index f95b5e3a5..325fcbea5 100644 --- a/projects/policyengine-simulation-entry/tests/test_openapi.py +++ b/projects/policyengine-simulation-entry/tests/test_openapi.py @@ -187,7 +187,7 @@ def test_normalized_contract_matches_old_gateway(): cloud_run_spec ) == normalized_compatibility_paths(gateway_spec) cloud_run_schemas = deepcopy(cloud_run_spec["components"]["schemas"]) - # The entrypoint's readiness response adds its own database's revision. + # The entrypoint's readiness response adds the simulation database check. cloud_run_schemas.pop("SimulationEntrypointReadinessResponse") cloud_run_schemas.pop("SimulationDatabaseReadiness") cloud_run_schemas.pop("SimulationErrorResponse") From 2878f274a4fd9363fd2ba185da1383f9c6220378 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:49:24 +0400 Subject: [PATCH 15/25] Model the two report_runs columns the Public API's latest revision adds The Public API's revision that stores the economy output fields with no column adds report_runs.spm_provenance (JSON) and data_version (varchar(128)), both nullable. The models now have them, so they again match the tables the Public API's chain builds. Co-Authored-By: Claude Opus 5.5 --- .../src/policyengine_simulation_persistence/tables.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py index 4921305a5..37189dab3 100644 --- a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py +++ b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py @@ -291,6 +291,8 @@ def _year_between(table_name: str, *, nullable: bool) -> CheckConstraint: Column("modal_call_id", String(MODAL_CALL_ID_LENGTH), nullable=True), Column("labor_supply_result", JSON, nullable=True), Column("cliff_impact_result", JSON, nullable=True), + Column("spm_provenance", JSON, nullable=True), + Column("data_version", String(VERSION_LENGTH), nullable=True), PrimaryKeyConstraint("id", name="pk_report_runs"), ForeignKeyConstraint( ["report_id"], From fb324eca5f8f385d7f615830b34f84ccb4f264f9 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 01:08:53 +0400 Subject: [PATCH 16/25] End the readiness query in the database before the readiness check stops waiting /ready waits at most three seconds for the database, but asyncio.wait_for can't stop the thread the query runs on, so a database that accepted the connection and then stalled held a worker of the default thread pool for as long as it stalled, once per request to the public route. The query now runs in a transaction whose statement_timeout, set with set_config local to that transaction, is two seconds, so the database ends it first; it works through Supabase's transaction pooler and never outlives the check. Co-Authored-By: Claude Opus 5.5 --- .../simulation_database.py | 18 ++++- .../tests/test_simulation_database.py | 66 +++++++++++++++++++ 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py index 8c5b19c23..7d77dbc85 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py @@ -9,10 +9,15 @@ create_simulation_engine, missing_table_privileges, ) +from sqlalchemy import func, select if TYPE_CHECKING: from sqlalchemy import Engine +# The readiness check stops waiting after three seconds, but it can't stop the +# thread its query runs on, so the database ends the query sooner. +READINESS_STATEMENT_TIMEOUT_MILLISECONDS = 2000 + class SimulationDatabase: """Run short, blocking database operations off the event loop.""" @@ -30,7 +35,18 @@ def from_url(cls, database_url: str) -> SimulationDatabase: return cls(create_simulation_engine(database_url)) def _missing_privileges(self) -> list[str]: - with self.engine.connect() as connection: + with self.engine.connect() as connection, connection.begin(): + # Local to the transaction, so it works through Supabase's + # transaction pooler and never outlives this check. + connection.execute( + select( + func.set_config( + "statement_timeout", + str(READINESS_STATEMENT_TIMEOUT_MILLISECONDS), + True, # noqa: FBT003 # set_config's is_local argument + ) + ) + ) return missing_table_privileges(connection) async def missing_privileges(self) -> list[str]: diff --git a/projects/policyengine-simulation-entry/tests/test_simulation_database.py b/projects/policyengine-simulation-entry/tests/test_simulation_database.py index 864eb010f..ce6393e59 100644 --- a/projects/policyengine-simulation-entry/tests/test_simulation_database.py +++ b/projects/policyengine-simulation-entry/tests/test_simulation_database.py @@ -3,6 +3,7 @@ from __future__ import annotations import asyncio +import contextlib import threading import pytest @@ -171,6 +172,12 @@ def __enter__(self): def __exit__(self, *_): return False + def begin(self): + return contextlib.nullcontext() + + def execute(self, statement): + return None + class FakeEngine: disposed = False @@ -208,3 +215,62 @@ def test_a_malformed_database_url_never_builds_an_engine(database_url): SimulationDatabase.from_url(database_url) assert "secret" not in str(error.value) + + +def test_the_readiness_query_is_bounded_on_the_server(monkeypatch): + """A stalled database can't hold a worker thread past the statement limit.""" + executed = [] + + class FakeTransaction: + def __enter__(self): + executed.append("BEGIN") + return self + + def __exit__(self, *_): + executed.append("END") + return False + + class FakeConnection: + def __enter__(self): + return self + + def __exit__(self, *_): + return False + + def begin(self): + return FakeTransaction() + + def execute(self, statement): + compiled = statement.compile() + executed.append((str(compiled), dict(compiled.params))) + + class FakeEngine: + def connect(self): + return FakeConnection() + + def missing_table_privileges(connection): + executed.append("privileges") + return [] + + monkeypatch.setattr( + database_module, "missing_table_privileges", missing_table_privileges + ) + + asyncio.run(SimulationDatabase(FakeEngine()).missing_privileges()) + + assert executed[0] == "BEGIN" + statement, parameters = executed[1] + assert "set_config" in statement + assert sorted(parameters.values(), key=str) == sorted( + [ + "statement_timeout", + str(database_module.READINESS_STATEMENT_TIMEOUT_MILLISECONDS), + True, + ], + key=str, + ) + assert executed[2:] == ["privileges", "END"] + assert ( + database_module.READINESS_STATEMENT_TIMEOUT_MILLISECONDS + < app_module.SIMULATION_DATABASE_READINESS_TIMEOUT_SECONDS * 1000 + ) From 48225db56c79da7233d1e54d10b936568c38b564 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 01:09:32 +0400 Subject: [PATCH 17/25] Name the public schema on the table models The privilege check looks only at the public schema, but the models left their schema unnamed, so a write through them followed the role's search_path and would reach a table of the same name in a schema earlier on it. service_metadata now names public, as the Public API's tables live there, and the foreign keys name it too. The privilege check reuses the same constant. Co-Authored-By: Claude Opus 5.5 --- .../README.md | 4 +++- .../access.py | 3 ++- .../tables.py | 17 ++++++++++------- .../tests/test_tables.py | 11 +++++++---- 4 files changed, 22 insertions(+), 13 deletions(-) diff --git a/libs/policyengine-simulation-persistence/README.md b/libs/policyengine-simulation-persistence/README.md index cd636e060..8ea1dc3be 100644 --- a/libs/policyengine-simulation-persistence/README.md +++ b/libs/policyengine-simulation-persistence/README.md @@ -21,7 +21,9 @@ URL, which holds a password. `tables.py` models, with SQLAlchemy Core, the six tables the service creates rows in: `simulations`, `reports`, `simulation_runs`, `report_runs`, -`report_run_simulation_runs` and `household_jobs`, all in `service_metadata`. +`report_run_simulation_runs` and `household_jobs`, all in `service_metadata`, +which names the `public` schema so writes reach those tables whatever the +role's `search_path`. Each model matches the table the Public API's v2 migration chain builds: every column with its type, nullability and server default, and every constraint and index under the chain's name. The models only describe the diff --git a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/access.py b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/access.py index 2a9ddef9c..4ee79b71d 100644 --- a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/access.py +++ b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/access.py @@ -13,12 +13,13 @@ from sqlalchemy import String, and_, column, false, func, select, table, values +from policyengine_simulation_persistence.tables import TABLE_SCHEMA + if TYPE_CHECKING: from collections.abc import Mapping from sqlalchemy import Connection -TABLE_SCHEMA: Final = "public" # PostgreSQL's catalog, which any role may read: a table is found by name # without the USAGE privilege on its schema that to_regclass would need. _pg_class = table( diff --git a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py index 37189dab3..04bdb995b 100644 --- a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py +++ b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py @@ -132,7 +132,10 @@ def _year_between(table_name: str, *, nullable: bool) -> CheckConstraint: return CheckConstraint(condition, name=f"ck_{table_name}_year") -service_metadata = MetaData() +# Named explicitly, so writes reach the Public API's tables whatever the +# role's search_path. +TABLE_SCHEMA: Final = "public" +service_metadata = MetaData(schema=TABLE_SCHEMA) simulations = Table( "simulations", @@ -197,13 +200,13 @@ def _year_between(table_name: str, *, nullable: bool) -> CheckConstraint: PrimaryKeyConstraint("id", name="pk_reports"), ForeignKeyConstraint( ["baseline_simulation_id"], - ["simulations.id"], + [f"{TABLE_SCHEMA}.simulations.id"], name="fk_reports_baseline_simulation_id_simulations", ondelete="SET NULL", ), ForeignKeyConstraint( ["reform_simulation_id"], - ["simulations.id"], + [f"{TABLE_SCHEMA}.simulations.id"], name="fk_reports_reform_simulation_id_simulations", ondelete="SET NULL", ), @@ -239,7 +242,7 @@ def _year_between(table_name: str, *, nullable: bool) -> CheckConstraint: PrimaryKeyConstraint("id", name="pk_simulation_runs"), ForeignKeyConstraint( ["simulation_id"], - ["simulations.id"], + [f"{TABLE_SCHEMA}.simulations.id"], name="fk_simulation_runs_simulation_id_simulations", ondelete="CASCADE", ), @@ -296,7 +299,7 @@ def _year_between(table_name: str, *, nullable: bool) -> CheckConstraint: PrimaryKeyConstraint("id", name="pk_report_runs"), ForeignKeyConstraint( ["report_id"], - ["reports.id"], + [f"{TABLE_SCHEMA}.reports.id"], name="fk_report_runs_report_id_reports", ondelete="CASCADE", ), @@ -329,13 +332,13 @@ def _year_between(table_name: str, *, nullable: bool) -> CheckConstraint: PrimaryKeyConstraint("id", name="pk_report_run_simulation_runs"), ForeignKeyConstraint( ["report_run_id"], - ["report_runs.id"], + [f"{TABLE_SCHEMA}.report_runs.id"], name="fk_report_run_simulation_runs_report_run_id_report_runs", ondelete="CASCADE", ), ForeignKeyConstraint( ["simulation_run_id"], - ["simulation_runs.id"], + [f"{TABLE_SCHEMA}.simulation_runs.id"], name="fk_report_run_simulation_runs_simulation_run_id_simulation_runs", ondelete="RESTRICT", ), diff --git a/libs/policyengine-simulation-persistence/tests/test_tables.py b/libs/policyengine-simulation-persistence/tests/test_tables.py index 2c3bb5235..e3a542cad 100644 --- a/libs/policyengine-simulation-persistence/tests/test_tables.py +++ b/libs/policyengine-simulation-persistence/tests/test_tables.py @@ -15,18 +15,21 @@ def test_the_models_describe_every_table_the_service_creates_rows_in() -> None: if "INSERT" in privileges } - assert set(service_metadata.tables) == created_tables + assert {table.name for table in service_metadata.tables.values()} == ( + created_tables + ) def test_foreign_keys_stay_between_the_services_own_tables() -> None: """Another service writes the policy, household and catalog rows.""" for table in service_metadata.tables.values(): for foreign_key in table.foreign_keys: - assert foreign_key.column.table.name in service_metadata.tables, ( + assert foreign_key.column.table in service_metadata.tables.values(), ( f"{table.name}.{foreign_key.parent.name}" ) -def test_every_table_lives_in_the_public_schema() -> None: +def test_every_table_names_the_public_schema() -> None: + """Writes go to the Public API's tables whatever the role's search_path.""" for table in service_metadata.tables.values(): - assert table.schema is None, table.name + assert table.schema == "public", table.name From db7dc5572547757f6d100055c0b41dc0cb67c578 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 01:10:03 +0400 Subject: [PATCH 18/25] Check the simulation database URL before building its engine create_app built the engine from SIMULATION_DATABASE_URL before the lifespan ran Settings.validate(), so a malformed URL stopped the import with the library's ValueError and the settings' own check only ever ran in tests. create_app now runs that check first, so a malformed URL is a ConfigurationError naming SIMULATION_DATABASE_URL, without repeating it, at startup like every other setting. Co-Authored-By: Claude Opus 5.5 --- .../src/policyengine_simulation_entry/app.py | 1 + .../src/policyengine_simulation_entry/config.py | 6 ++++-- .../tests/test_simulation_database.py | 15 +++++++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py index e8c38678e..2c8f5bc04 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py @@ -563,6 +563,7 @@ def create_app( runtime_settings = settings or Settings.from_env() runtime_database = simulation_database if runtime_database is None and runtime_settings.simulation_database_url: + runtime_settings.validate_simulation_database() runtime_database = SimulationDatabase.from_url( runtime_settings.simulation_database_url ) diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/config.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/config.py index bf12af7d8..4e53d060d 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/config.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/config.py @@ -176,7 +176,7 @@ def validate(self) -> None: self._validate_caller_auth() self._validate_old_gateway() self._validate_stage12() - self._validate_simulation_database() + self.validate_simulation_database() def _validate_caller_auth(self) -> None: """Require caller authentication in production, with an HTTPS issuer.""" @@ -275,11 +275,13 @@ def _validate_stage12(self) -> None: msg = "STAGE12_DATABASE_URL must use the canonical postgresql:// scheme." raise ConfigurationError(msg) - def _validate_simulation_database(self) -> None: + def validate_simulation_database(self) -> None: """Require a complete canonical URL for the service role, when set. The persistence library's own check decides, so the settings accept exactly the URLs the engine does; its message never repeats the URL. + ``create_app`` calls it before building the engine, so a malformed URL + is a ConfigurationError at startup. """ if not self.simulation_database_url: return diff --git a/projects/policyengine-simulation-entry/tests/test_simulation_database.py b/projects/policyengine-simulation-entry/tests/test_simulation_database.py index ce6393e59..ee6b5dbdb 100644 --- a/projects/policyengine-simulation-entry/tests/test_simulation_database.py +++ b/projects/policyengine-simulation-entry/tests/test_simulation_database.py @@ -274,3 +274,18 @@ def missing_table_privileges(connection): database_module.READINESS_STATEMENT_TIMEOUT_MILLISECONDS < app_module.SIMULATION_DATABASE_READINESS_TIMEOUT_SECONDS * 1000 ) + + +def test_a_malformed_database_url_stops_the_app_with_a_configuration_error(): + """The settings' check runs before the engine is built, as at startup.""" + from policyengine_simulation_entry.config import ConfigurationError + + with pytest.raises(ConfigurationError, match="SIMULATION_DATABASE_URL") as error: + create_app( + settings=make_settings( + simulation_database_url="postgresql://db.example/postgres?password=secret" + ), + backend=FakeBackend(), + ) + + assert "secret" not in str(error.value) From d8e77d2a64c64cf3c96d16ae2686a9ce10e46562 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 01:10:04 +0400 Subject: [PATCH 19/25] Say exactly what the Stage 12 secret wrapper keeps apart The deploy script's comment said the Stage 12 block's wrapper shares no deploy helper with the main path, but both wrappers call resolve_secret_reference. The comment and the rule's statement now say what is separate: the wrapper itself. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/deploy-cloud-run-simulation-entry.sh | 5 +++-- docs/generated/rules.md | 2 +- rules/registry.toml | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/scripts/deploy-cloud-run-simulation-entry.sh b/.github/scripts/deploy-cloud-run-simulation-entry.sh index 57f8a80ab..307376595 100755 --- a/.github/scripts/deploy-cloud-run-simulation-entry.sh +++ b/.github/scripts/deploy-cloud-run-simulation-entry.sh @@ -70,8 +70,9 @@ add_runtime_secret() { runtime_secrets+=("${variable_name}=${resolved_reference}") } -# The Stage 12 comparison path's own copy, so it shares no deploy helper with -# the main path and is removed with it. +# The Stage 12 comparison path's own copy of add_runtime_secret, so the +# Stage 12 block resolves its secrets through a wrapper of its own and the +# wrapper is removed with it. Both still call resolve_secret_reference. add_stage12_runtime_secret() { local variable_name="$1" local resolved_reference diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 73bfe7829..fbc62d4ed 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -171,7 +171,7 @@ flowchart LR | `sim.deploy.smoke-tests-strict` | Candidate and stable smoke tests reject a revision mismatch without retrying, and fail if the previous revision does not converge; the candidate smoke also requires the simulation service to reach its database with every table privilege its role needs. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets.py`, `projects/policyengine-simulation-entry/tests/test_simulation_database_deployment.py` | `pr.yml:test` | **unproven** | | `sim.deploy.staging-promotes-tested-revision` | Every staging deployment passes promote_entrypoint: ${{ github.ref == 'refs/heads/main' }}, so a run from main moves the staging service's stable Cloud Run traffic to the entrypoint revision that passed the staging tests, and an accidental manual run from another branch neither moves that traffic nor deletes older revisions. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets_promotion.py` | `pr.yml:test` | yes | | `sim.deploy.traffic-changes-exact` | Traffic changes target an exact, validated revision, are reversible, and refuse to run over an intervening promotion. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets_traffic.py` | `pr.yml:test` | **unproven** | -| `sim.deploy.workflow-shell-safety` | Cloud Run deployment never interpolates GitHub expressions into shell, escapes environment values, pins secret versions, stops before deploying when any runtime secret cannot be resolved, and resolves the Stage 12 secrets with the Stage 12 block's own helper. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py` | `pr.yml:test` | yes | +| `sim.deploy.workflow-shell-safety` | Cloud Run deployment never interpolates GitHub expressions into shell, escapes environment values, pins secret versions, stops before deploying when any runtime secret cannot be resolved, and resolves the Stage 12 secrets through the Stage 12 block's own wrapper. | mechanical | enforced | yes | pytest `projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py` | `pr.yml:test` | yes | ```mermaid flowchart LR diff --git a/rules/registry.toml b/rules/registry.toml index df88f12d2..84f99548a 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -352,7 +352,7 @@ invocation = "uv run --frozen pytest tests/" [[rule]] id = "sim.deploy.workflow-shell-safety" category = "deployment" -statement = "Cloud Run deployment never interpolates GitHub expressions into shell, escapes environment values, pins secret versions, stops before deploying when any runtime secret cannot be resolved, and resolves the Stage 12 secrets with the Stage 12 block's own helper." +statement = "Cloud Run deployment never interpolates GitHub expressions into shell, escapes environment values, pins secret versions, stops before deploying when any runtime secret cannot be resolved, and resolves the Stage 12 secrets through the Stage 12 block's own wrapper." rationale = "docs/operations" scope = [".github/workflows/*.yml", ".github/scripts/deploy-cloud-run-simulation-entry.sh"] tier = "mechanical" From 456388c2172d78c7b95fb68c645c288d3519e43c Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 01:32:20 +0400 Subject: [PATCH 20/25] Give the readiness check a thread of its own, and bound its transaction in the library The database's statement_timeout bounds only statements, not the wait for a pooled connection, connecting or a dead socket, so a stalled database still let each request to the public /ready route hold a thread of the default executor, which the Stage 12 backend's calls and the old-gateway client's name lookups also use. SimulationDatabase now runs the check on a ThreadPoolExecutor of one thread of its own, so a stalled database holds at most that thread and never one of the default executor; checks that time out while it is busy are cancelled before they start. dispose stops it. The bounded transaction moves to the persistence library as read_missing_table_privileges, where PostgreSQL tests show that it reads the privileges, that the database ends a statement that runs too long, and that no statement_timeout stays on the pooled connection either way. The readiness documents and the smoke message say that "reachable": false also covers a database that was reached but didn't answer in time, and the models' docstring no longer claims a check the service doesn't make yet. Co-Authored-By: Claude Opus 5.5 --- .../cloud-run-simulation-entry-smoke.sh | 2 +- .../README.md | 7 +- .../__init__.py | 2 + .../access.py | 25 ++- .../tables.py | 2 +- .../tests/test_access.py | 58 +++++- .../DEPLOYMENT.md | 5 +- .../policyengine_simulation_entry/schemas.py | 5 +- .../simulation_database.py | 44 ++-- .../tests/test_simulation_database.py | 195 ++++++++---------- 10 files changed, 210 insertions(+), 135 deletions(-) diff --git a/.github/scripts/cloud-run-simulation-entry-smoke.sh b/.github/scripts/cloud-run-simulation-entry-smoke.sh index b41afeb15..ea97b3650 100755 --- a/.github/scripts/cloud-run-simulation-entry-smoke.sh +++ b/.github/scripts/cloud-run-simulation-entry-smoke.sh @@ -159,7 +159,7 @@ if [ "${require_database_access}" = "true" ]; then database_attempt=1 while [ "${database_attempt}" -lt "${database_ready_max_attempts}" ] && jq -e '.simulation_database.reachable == false' <<<"${ready_body}" >/dev/null; do - printf '%s: the simulation database was not reached; asking again (%s/%s)\n' \ + printf '%s: the simulation database did not answer in time; asking again (%s/%s)\n' \ "${base_url}" "${database_attempt}" "${database_ready_max_attempts}" >&2 sleep "${routing_retry_delay_seconds}" ready_body="$(curl --fail --silent --show-error "${base_url}/ready")" diff --git a/libs/policyengine-simulation-persistence/README.md b/libs/policyengine-simulation-persistence/README.md index 8ea1dc3be..ec5634919 100644 --- a/libs/policyengine-simulation-persistence/README.md +++ b/libs/policyengine-simulation-persistence/README.md @@ -44,8 +44,11 @@ connected role lacks, as `"INSERT on reports"`; a table missing from the `public` schema lacks all of its privileges, and a role without the `USAGE` privilege on that schema is told so first (`"USAGE on schema public"`), since it can use none of its table grants. Tables are found through PostgreSQL's -catalog, which needs no privilege. The Simulation Entrypoint's readiness -check reports the result. +catalog, which needs no privilege. `read_missing_table_privileges` runs that +check in a transaction whose `statement_timeout` it sets with `set_config`, +local to the transaction, so the database ends a check that runs too long; +the Simulation Entrypoint's readiness check uses it, on a thread of its own, +and reports the result. ## Tests diff --git a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/__init__.py b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/__init__.py index dfc896fe0..4e14aab9b 100644 --- a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/__init__.py +++ b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/__init__.py @@ -3,6 +3,7 @@ from policyengine_simulation_persistence.access import ( SERVICE_TABLE_PRIVILEGES, missing_table_privileges, + read_missing_table_privileges, ) from policyengine_simulation_persistence.engine import ( create_simulation_engine, @@ -36,6 +37,7 @@ "household_jobs", "missing_table_privileges", "normalized_database_url", + "read_missing_table_privileges", "report_run_simulation_runs", "report_runs", "reports", diff --git a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/access.py b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/access.py index 4ee79b71d..2af0999cd 100644 --- a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/access.py +++ b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/access.py @@ -18,7 +18,7 @@ if TYPE_CHECKING: from collections.abc import Mapping - from sqlalchemy import Connection + from sqlalchemy import Connection, Engine # PostgreSQL's catalog, which any role may read: a table is found by name # without the USAGE privilege on its schema that to_regclass would need. @@ -111,3 +111,26 @@ def missing_table_privileges(connection: Connection) -> list[str]: if not schema_usage: missing.insert(0, f"USAGE on schema {TABLE_SCHEMA}") return missing + + +def read_missing_table_privileges( + engine: Engine, *, statement_timeout_milliseconds: int +) -> list[str]: + """Run ``missing_table_privileges`` in a transaction the database bounds. + + ``statement_timeout`` is set with ``set_config`` local to the transaction, + so it works through Supabase's transaction pooler and never stays on the + pooled connection; a statement that runs longer raises the database's + cancellation as an OperationalError. + """ + with engine.connect() as connection, connection.begin(): + connection.execute( + select( + func.set_config( + "statement_timeout", + str(statement_timeout_milliseconds), + True, # noqa: FBT003 # set_config's is_local argument + ) + ) + ) + return missing_table_privileges(connection) diff --git a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py index 04bdb995b..0d89ba1b9 100644 --- a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py +++ b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py @@ -5,7 +5,7 @@ matches what the chain builds, column for column, with its constraints and indexes under the same names. Foreign keys stay between these tables: the policy, household and catalog rows they name are written by another service, -so the simulation service checks them itself. +so no foreign key checks them, and the code that writes these rows must. """ from __future__ import annotations diff --git a/libs/policyengine-simulation-persistence/tests/test_access.py b/libs/policyengine-simulation-persistence/tests/test_access.py index aca0997f7..9e688c5ad 100644 --- a/libs/policyengine-simulation-persistence/tests/test_access.py +++ b/libs/policyengine-simulation-persistence/tests/test_access.py @@ -10,9 +10,12 @@ SERVICE_TABLE_PRIVILEGES, create_simulation_engine, missing_table_privileges, + read_missing_table_privileges, ) -from sqlalchemy import create_engine, text +from policyengine_simulation_persistence import access as access_module +from sqlalchemy import create_engine, func, select, text from sqlalchemy.engine import make_url +from sqlalchemy.exc import OperationalError PROOF_FIXTURES = ( Path(__file__).resolve().parents[3] @@ -220,3 +223,56 @@ def test_a_role_without_usage_on_the_schema_is_told_so(disposable_database): assert _missing_privileges_as_the_service(disposable_database) == [ "USAGE on schema public", ] + + +def _service_engine_with_one_connection(database): + """Return an engine whose next connection is always the one it pooled.""" + return create_engine( + make_url(database.service_url).set(drivername="postgresql+psycopg"), + pool_size=1, + max_overflow=0, + ) + + +def _statement_timeout(engine) -> str: + with engine.connect() as connection: + return connection.execute(text("SHOW statement_timeout")).scalar_one() + + +@pytest.mark.postgres +def test_the_bounded_check_reads_the_privileges_and_leaves_no_timeout_behind( + disposable_database, +): + _create_tables(disposable_database, SERVICE_TABLE_PRIVILEGES) + _grant_every_required_privilege(disposable_database) + engine = _service_engine_with_one_connection(disposable_database) + try: + missing = read_missing_table_privileges( + engine, statement_timeout_milliseconds=2000 + ) + timeout_afterwards = _statement_timeout(engine) + finally: + engine.dispose() + + assert missing == [] + assert timeout_afterwards == "0" + + +@pytest.mark.postgres +def test_the_database_ends_a_bounded_check_that_runs_too_long( + disposable_database, monkeypatch +): + def sleep_instead(connection): + connection.execute(select(func.pg_sleep(2))) + return [] + + monkeypatch.setattr(access_module, "missing_table_privileges", sleep_instead) + engine = _service_engine_with_one_connection(disposable_database) + try: + with pytest.raises(OperationalError, match="statement timeout"): + read_missing_table_privileges(engine, statement_timeout_milliseconds=100) + timeout_afterwards = _statement_timeout(engine) + finally: + engine.dispose() + + assert timeout_afterwards == "0" diff --git a/projects/policyengine-simulation-entry/DEPLOYMENT.md b/projects/policyengine-simulation-entry/DEPLOYMENT.md index 6e76be588..b94b4dfc1 100644 --- a/projects/policyengine-simulation-entry/DEPLOYMENT.md +++ b/projects/policyengine-simulation-entry/DEPLOYMENT.md @@ -48,8 +48,9 @@ readable by the runtime service account; and the `beta` or `prod` variable the deploy stops at once; without the tables, the role or its grants the candidate check below fails. -`GET /ready` reports, under `simulation_database`, whether the entrypoint -reached the database and which table privileges its role lacks +`GET /ready` reports, under `simulation_database`, whether the database +answered within three seconds (`reachable`; false also when it was reached +but didn't answer in time) and which table privileges its role lacks (`missing_privileges`, such as `"INSERT on reports"`; the list is in `libs/policyengine-simulation-persistence`). The database does not change the readiness status, because no old-gateway or Stage 12 route uses it. The diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/schemas.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/schemas.py index 9c8e1d574..1b6578e11 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/schemas.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/schemas.py @@ -120,8 +120,9 @@ class TemporaryStage12ReportResponse(BaseModel): class SimulationDatabaseReadiness(BaseModel): """The simulation service's database, as the readiness check found it. - ``reachable`` says whether a pooled connection answered within the check's - time limit. ``missing_privileges`` names each table privilege the service + ``reachable`` says whether the database answered the check within its + time limit: false when it wasn't reached, or was reached but didn't + answer in time. ``missing_privileges`` names each table privilege the service needs and its role lacks, such as ``"INSERT on reports"``, or is null when the database was not reached. Neither changes ``status``: no old-gateway or Stage 12 route uses this database. diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py index 7d77dbc85..4e0ad3e68 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py @@ -3,20 +3,22 @@ from __future__ import annotations import asyncio +from concurrent.futures import ThreadPoolExecutor +from functools import partial from typing import TYPE_CHECKING from policyengine_simulation_persistence import ( create_simulation_engine, - missing_table_privileges, + read_missing_table_privileges, ) -from sqlalchemy import func, select if TYPE_CHECKING: from sqlalchemy import Engine -# The readiness check stops waiting after three seconds, but it can't stop the -# thread its query runs on, so the database ends the query sooner. +# The readiness check stops waiting after three seconds; the database ends the +# query sooner, once the connection answers. READINESS_STATEMENT_TIMEOUT_MILLISECONDS = 2000 +READINESS_THREAD_NAME_PREFIX = "simulation-database-readiness" class SimulationDatabase: @@ -25,6 +27,13 @@ class SimulationDatabase: def __init__(self, engine: Engine) -> None: """Use ``engine``, which connects as the simulation service's role.""" self.engine = engine + # One thread of its own: waiting for a pooled connection, connecting + # and a dead socket aren't bounded by the database's time limit, so a + # stalled database may hold this thread, but never more than this one, + # and never one of the default executor the other routes use. + self.executor = ThreadPoolExecutor( + max_workers=1, thread_name_prefix=READINESS_THREAD_NAME_PREFIX + ) @classmethod def from_url(cls, database_url: str) -> SimulationDatabase: @@ -34,25 +43,18 @@ def from_url(cls, database_url: str) -> SimulationDatabase: """ return cls(create_simulation_engine(database_url)) - def _missing_privileges(self) -> list[str]: - with self.engine.connect() as connection, connection.begin(): - # Local to the transaction, so it works through Supabase's - # transaction pooler and never outlives this check. - connection.execute( - select( - func.set_config( - "statement_timeout", - str(READINESS_STATEMENT_TIMEOUT_MILLISECONDS), - True, # noqa: FBT003 # set_config's is_local argument - ) - ) - ) - return missing_table_privileges(connection) - async def missing_privileges(self) -> list[str]: """Return each table privilege the service needs and its role lacks.""" - return await asyncio.to_thread(self._missing_privileges) + return await asyncio.get_running_loop().run_in_executor( + self.executor, + partial( + read_missing_table_privileges, + self.engine, + statement_timeout_milliseconds=READINESS_STATEMENT_TIMEOUT_MILLISECONDS, + ), + ) def dispose(self) -> None: - """Close every pooled connection.""" + """Stop the check's thread and close every pooled connection.""" + self.executor.shutdown(wait=False, cancel_futures=True) self.engine.dispose() diff --git a/projects/policyengine-simulation-entry/tests/test_simulation_database.py b/projects/policyengine-simulation-entry/tests/test_simulation_database.py index ee6b5dbdb..8dd37539d 100644 --- a/projects/policyengine-simulation-entry/tests/test_simulation_database.py +++ b/projects/policyengine-simulation-entry/tests/test_simulation_database.py @@ -3,7 +3,6 @@ from __future__ import annotations import asyncio -import contextlib import threading import pytest @@ -160,132 +159,120 @@ def build(database_url): assert built_urls == [DATABASE_URL] -def test_the_database_reads_the_missing_privileges_off_the_event_loop(monkeypatch): - connections = [] - query_threads = [] +@pytest.mark.parametrize("database_url", ["", "postgres://user:secret@db/postgres"]) +def test_a_malformed_database_url_never_builds_an_engine(database_url): + with pytest.raises(ValueError, match="Simulation database URL") as error: + SimulationDatabase.from_url(database_url) - class FakeConnection: - def __enter__(self): - connections.append(self) - return self + assert "secret" not in str(error.value) - def __exit__(self, *_): - return False - def begin(self): - return contextlib.nullcontext() +def test_a_malformed_database_url_stops_the_app_with_a_configuration_error(): + """The settings' check runs before the engine is built, as at startup.""" + from policyengine_simulation_entry.config import ConfigurationError - def execute(self, statement): - return None + with pytest.raises(ConfigurationError, match="SIMULATION_DATABASE_URL") as error: + create_app( + settings=make_settings( + simulation_database_url="postgresql://db.example/postgres?password=secret" + ), + backend=FakeBackend(), + ) - class FakeEngine: - disposed = False + assert "secret" not in str(error.value) - def connect(self): - return FakeConnection() - def dispose(self): - self.disposed = True +def test_the_check_runs_on_its_own_thread_within_the_database_time_limit( + monkeypatch, +): + calls = [] - def missing_table_privileges(connection): - assert connection in connections - query_threads.append(threading.get_ident()) + def read_missing_table_privileges(engine, *, statement_timeout_milliseconds): + calls.append( + (engine, statement_timeout_milliseconds, threading.current_thread().name) + ) return ["INSERT on reports"] monkeypatch.setattr( - database_module, "missing_table_privileges", missing_table_privileges + database_module, "read_missing_table_privileges", read_missing_table_privileges ) - engine = FakeEngine() + engine = object() database = SimulationDatabase(engine) - async def read_from_the_event_loop(): - return await database.missing_privileges(), threading.get_ident() - - missing, event_loop_thread = asyncio.run(read_from_the_event_loop()) - database.dispose() + try: + missing = asyncio.run(database.missing_privileges()) + finally: + database.executor.shutdown(wait=True) assert missing == ["INSERT on reports"] - assert query_threads != [event_loop_thread] - assert engine.disposed is True - - -@pytest.mark.parametrize("database_url", ["", "postgres://user:secret@db/postgres"]) -def test_a_malformed_database_url_never_builds_an_engine(database_url): - with pytest.raises(ValueError, match="Simulation database URL") as error: - SimulationDatabase.from_url(database_url) - - assert "secret" not in str(error.value) - - -def test_the_readiness_query_is_bounded_on_the_server(monkeypatch): - """A stalled database can't hold a worker thread past the statement limit.""" - executed = [] - - class FakeTransaction: - def __enter__(self): - executed.append("BEGIN") - return self + [(called_engine, timeout_milliseconds, thread_name)] = calls + assert called_engine is engine + expected_timeout = database_module.READINESS_STATEMENT_TIMEOUT_MILLISECONDS + assert timeout_milliseconds == expected_timeout + assert timeout_milliseconds < ( + app_module.SIMULATION_DATABASE_READINESS_TIMEOUT_SECONDS * 1000 + ) + assert thread_name.startswith(database_module.READINESS_THREAD_NAME_PREFIX) + + +def test_a_stalled_database_holds_one_thread_and_no_thread_of_the_default_pool( + monkeypatch, +): + """Checks that time out don't pile up threads other routes need.""" + release = threading.Event() + running = [] + most_running_at_once = [] + lock = threading.Lock() + + def stall(engine, *, statement_timeout_milliseconds): + with lock: + running.append(threading.current_thread().name) + most_running_at_once.append(len(running)) + release.wait(timeout=5) + with lock: + running.remove(threading.current_thread().name) + return [] - def __exit__(self, *_): - executed.append("END") - return False + monkeypatch.setattr(database_module, "read_missing_table_privileges", stall) + database = SimulationDatabase(object()) - class FakeConnection: - def __enter__(self): - return self + async def three_checks_that_time_out(): + results = await asyncio.gather( + *( + asyncio.wait_for(database.missing_privileges(), timeout=0.05) + for _ in range(3) + ), + return_exceptions=True, + ) + default_pool_threads = [ + thread.name + for thread in threading.enumerate() + if thread.name.startswith("asyncio_") + ] + return results, default_pool_threads - def __exit__(self, *_): - return False + try: + results, default_pool_threads = asyncio.run(three_checks_that_time_out()) + finally: + release.set() + database.executor.shutdown(wait=True) - def begin(self): - return FakeTransaction() + assert all(isinstance(result, TimeoutError) for result in results) + assert max(most_running_at_once) == 1 + assert default_pool_threads == [] - def execute(self, statement): - compiled = statement.compile() - executed.append((str(compiled), dict(compiled.params))) +def test_disposing_the_database_also_stops_its_thread(): class FakeEngine: - def connect(self): - return FakeConnection() - - def missing_table_privileges(connection): - executed.append("privileges") - return [] - - monkeypatch.setattr( - database_module, "missing_table_privileges", missing_table_privileges - ) - - asyncio.run(SimulationDatabase(FakeEngine()).missing_privileges()) - - assert executed[0] == "BEGIN" - statement, parameters = executed[1] - assert "set_config" in statement - assert sorted(parameters.values(), key=str) == sorted( - [ - "statement_timeout", - str(database_module.READINESS_STATEMENT_TIMEOUT_MILLISECONDS), - True, - ], - key=str, - ) - assert executed[2:] == ["privileges", "END"] - assert ( - database_module.READINESS_STATEMENT_TIMEOUT_MILLISECONDS - < app_module.SIMULATION_DATABASE_READINESS_TIMEOUT_SECONDS * 1000 - ) - + disposed = False -def test_a_malformed_database_url_stops_the_app_with_a_configuration_error(): - """The settings' check runs before the engine is built, as at startup.""" - from policyengine_simulation_entry.config import ConfigurationError + def dispose(self): + self.disposed = True - with pytest.raises(ConfigurationError, match="SIMULATION_DATABASE_URL") as error: - create_app( - settings=make_settings( - simulation_database_url="postgresql://db.example/postgres?password=secret" - ), - backend=FakeBackend(), - ) + engine = FakeEngine() + database = SimulationDatabase(engine) + database.dispose() - assert "secret" not in str(error.value) + assert engine.disposed is True + with pytest.raises(RuntimeError): + database.executor.submit(print) From 7778af6ede9466a4001e0674c912ee7686ce01d1 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 01:34:41 +0400 Subject: [PATCH 21/25] Name the public schema on the models' enum types SQLAlchemy 2.1, which the persistence library's own lock resolves, gives an enum the metadata's schema, but 2.0.54, which the entrypoint's image installs, doesn't, so there the six enum types followed the role's search_path. Each now names public itself. The test fails under 2.0.54 without the change. Co-Authored-By: Claude Opus 5.5 --- .../policyengine_simulation_persistence/tables.py | 7 ++++++- .../tests/test_tables.py | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py index 0d89ba1b9..8225dd31b 100644 --- a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py +++ b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py @@ -93,10 +93,15 @@ class HouseholdJobStatus(StrEnum): def _enum_type(values: type[StrEnum], name: str) -> Enum: - """Store each member's value, under the Postgres type the chain names.""" + """Store each member's value, under the Postgres type the chain names. + + The type names its schema itself: SQLAlchemy 2.0 doesn't give an enum the + metadata's schema, so it would otherwise follow the role's search_path. + """ return Enum( values, name=name, + schema=TABLE_SCHEMA, values_callable=lambda members: [member.value for member in members], ) diff --git a/libs/policyengine-simulation-persistence/tests/test_tables.py b/libs/policyengine-simulation-persistence/tests/test_tables.py index e3a542cad..c6a451b7e 100644 --- a/libs/policyengine-simulation-persistence/tests/test_tables.py +++ b/libs/policyengine-simulation-persistence/tests/test_tables.py @@ -6,6 +6,7 @@ SERVICE_TABLE_PRIVILEGES, service_metadata, ) +from sqlalchemy import Enum def test_the_models_describe_every_table_the_service_creates_rows_in() -> None: @@ -33,3 +34,17 @@ def test_every_table_names_the_public_schema() -> None: """Writes go to the Public API's tables whatever the role's search_path.""" for table in service_metadata.tables.values(): assert table.schema == "public", table.name + + +def test_every_enum_type_names_the_public_schema() -> None: + """Enum types resolve to the Public API's whatever the SQLAlchemy release.""" + enum_columns = [ + column + for table in service_metadata.tables.values() + for column in table.columns + if isinstance(column.type, Enum) + ] + + assert len(enum_columns) == 6 + for column in enum_columns: + assert column.type.schema == "public", f"{column.table.name}.{column.name}" From 4ecd7cd9c66cea6a117161bbd487578e6189278d Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 01:35:10 +0400 Subject: [PATCH 22/25] Find a combined secret resolution that opens on the assignment's first line combined_secret_resolutions took a ) anywhere on an array assignment's first line as its end, so an assignment whose first element resolved a secret on that line, with a second on the next, was read as one line and passed. The assignment now ends on its first line only when that line's parentheses balance. The new test is one of sim.deploy.workflow-shell-safety's enforcers. Co-Authored-By: Claude Opus 5.5 --- .../tests/deployment_rule_checks.py | 4 +++- .../tests/test_deployment_assets_cloud_run.py | 15 +++++++++++++++ rules/registry.toml | 1 + 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/projects/policyengine-simulation-entry/tests/deployment_rule_checks.py b/projects/policyengine-simulation-entry/tests/deployment_rule_checks.py index fa06bed25..9d6a94898 100644 --- a/projects/policyengine-simulation-entry/tests/deployment_rule_checks.py +++ b/projects/policyengine-simulation-entry/tests/deployment_rule_checks.py @@ -27,7 +27,9 @@ def combined_secret_resolutions(script_text: str) -> list[str]: for match in ARRAY_ASSIGNMENT.finditer(script_text): line_end = script_text.find("\n", match.end()) first_line = script_text[match.start() : line_end if line_end >= 0 else None] - if ")" in first_line: + # A command substitution on the first line closes its own parenthesis, + # so the assignment ends on that line only if they all balance. + if first_line.count("(") == first_line.count(")"): assignment = first_line else: closing = ARRAY_END.search(script_text, match.end()) diff --git a/projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py b/projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py index 8ce6dccb4..e714759bb 100644 --- a/projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py +++ b/projects/policyengine-simulation-entry/tests/test_deployment_assets_cloud_run.py @@ -5,6 +5,7 @@ import json import re import subprocess +import textwrap from pathlib import Path import pytest @@ -113,6 +114,20 @@ def test_cloud_run_deployment_stops_when_a_runtime_secret_cannot_be_resolved( assert not Path(command_environment["GCLOUD_ARGUMENTS_CAPTURE"]).exists() +def test_an_assignment_that_opens_with_a_resolved_secret_is_still_found(): + script = textwrap.dedent( + """\ + runtime_secrets=("A=$(resolve_secret_reference "${A_NAME}")" + "B=$(resolve_secret_reference "${B_NAME}")" + ) + """ + ) + + assert combined_secret_resolutions(script) == [ + 'runtime_secrets=("A=$(resolve_secret_reference "${A_NAME}")"' + ] + + def test_cloud_run_deployment_resolves_each_secret_in_its_own_assignment(): script = Path(CLOUD_RUN_DEPLOY_SCRIPT).read_text(encoding="utf-8") diff --git a/rules/registry.toml b/rules/registry.toml index 84f99548a..66cbc9ddf 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -366,6 +366,7 @@ nodes = [ "tests/test_deployment_assets_cloud_run.py::test_cloud_run_deployment_escapes_environment_and_pins_secret_versions", "tests/test_deployment_assets_cloud_run.py::test_cloud_run_deployment_stops_when_a_runtime_secret_cannot_be_resolved", "tests/test_deployment_assets_cloud_run.py::test_cloud_run_deployment_resolves_each_secret_in_its_own_assignment", + "tests/test_deployment_assets_cloud_run.py::test_an_assignment_that_opens_with_a_resolved_secret_is_still_found", "tests/test_deployment_assets_cloud_run.py::test_stage12_secrets_are_resolved_by_the_stage12_block_alone", ] [[rule.ci]] From e87311d5083d479f67226e0fdb001e0051b68e11 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 01:48:43 +0400 Subject: [PATCH 23/25] Stop modelling the four report_runs columns the Public API moved to report_run_comparisons The Public API's revision dce0caa329cc moves labor_supply_result, cliff_impact_result, spm_provenance and data_version from report_runs to a new report_run_comparisons table, one row per report run and compared pair, which the Modal workers write and the simulation service doesn't. The report_runs model drops them, so it again matches the Public API's table. Co-Authored-By: Claude Opus 5.5 --- .../src/policyengine_simulation_persistence/tables.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py index 8225dd31b..e14f998c3 100644 --- a/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py +++ b/libs/policyengine-simulation-persistence/src/policyengine_simulation_persistence/tables.py @@ -297,10 +297,6 @@ def _year_between(table_name: str, *, nullable: bool) -> CheckConstraint: Column("error_message", String, nullable=True), Column("markdown", Text, nullable=True), Column("modal_call_id", String(MODAL_CALL_ID_LENGTH), nullable=True), - Column("labor_supply_result", JSON, nullable=True), - Column("cliff_impact_result", JSON, nullable=True), - Column("spm_provenance", JSON, nullable=True), - Column("data_version", String(VERSION_LENGTH), nullable=True), PrimaryKeyConstraint("id", name="pk_report_runs"), ForeignKeyConstraint( ["report_id"], From 269e7f8425626dc567c3e1e36985c06537776dfd Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 01:50:30 +0400 Subject: [PATCH 24/25] Make the readiness tests prove what they claim, and describe a failed check fully The enum test now also checks the type the helper builds before a table adopts it, so it fails under the persistence library's own SQLAlchemy 2.1 when the helper stops naming public, not only under the image's 2.0. The stalled-database test checks that only the first of three checks ever ran, and the dispose test that a started worker exits. "reachable": false is described as any failed check (not reached, login refused, no answer in time), the smoke script's retry message says so, the persistence readme says statement_timeout ends each statement of the check, and SimulationDatabase.dispose says the database is single-use afterwards. Co-Authored-By: Claude Opus 5.5 --- .../cloud-run-simulation-entry-smoke.sh | 2 +- .../README.md | 3 ++- .../tests/test_tables.py | 12 +++++++++- .../DEPLOYMENT.md | 7 +++--- .../policyengine_simulation_entry/schemas.py | 6 ++--- .../simulation_database.py | 6 ++++- .../tests/test_simulation_database.py | 22 +++++++++++++++---- 7 files changed, 44 insertions(+), 14 deletions(-) diff --git a/.github/scripts/cloud-run-simulation-entry-smoke.sh b/.github/scripts/cloud-run-simulation-entry-smoke.sh index ea97b3650..63db83f32 100755 --- a/.github/scripts/cloud-run-simulation-entry-smoke.sh +++ b/.github/scripts/cloud-run-simulation-entry-smoke.sh @@ -159,7 +159,7 @@ if [ "${require_database_access}" = "true" ]; then database_attempt=1 while [ "${database_attempt}" -lt "${database_ready_max_attempts}" ] && jq -e '.simulation_database.reachable == false' <<<"${ready_body}" >/dev/null; do - printf '%s: the simulation database did not answer in time; asking again (%s/%s)\n' \ + printf '%s: the simulation database check failed; asking again (%s/%s)\n' \ "${base_url}" "${database_attempt}" "${database_ready_max_attempts}" >&2 sleep "${routing_retry_delay_seconds}" ready_body="$(curl --fail --silent --show-error "${base_url}/ready")" diff --git a/libs/policyengine-simulation-persistence/README.md b/libs/policyengine-simulation-persistence/README.md index ec5634919..934246c69 100644 --- a/libs/policyengine-simulation-persistence/README.md +++ b/libs/policyengine-simulation-persistence/README.md @@ -46,7 +46,8 @@ privilege on that schema is told so first (`"USAGE on schema public"`), since it can use none of its table grants. Tables are found through PostgreSQL's catalog, which needs no privilege. `read_missing_table_privileges` runs that check in a transaction whose `statement_timeout` it sets with `set_config`, -local to the transaction, so the database ends a check that runs too long; +local to the transaction, so the database ends any statement of the check +that runs too long; the Simulation Entrypoint's readiness check uses it, on a thread of its own, and reports the result. diff --git a/libs/policyengine-simulation-persistence/tests/test_tables.py b/libs/policyengine-simulation-persistence/tests/test_tables.py index c6a451b7e..8d5a4a4e6 100644 --- a/libs/policyengine-simulation-persistence/tests/test_tables.py +++ b/libs/policyengine-simulation-persistence/tests/test_tables.py @@ -4,8 +4,10 @@ from policyengine_simulation_persistence import ( SERVICE_TABLE_PRIVILEGES, + SimulationStatus, service_metadata, ) +from policyengine_simulation_persistence import tables as tables_module from sqlalchemy import Enum @@ -37,7 +39,12 @@ def test_every_table_names_the_public_schema() -> None: def test_every_enum_type_names_the_public_schema() -> None: - """Enum types resolve to the Public API's whatever the SQLAlchemy release.""" + """Enum types resolve to the Public API's whatever the SQLAlchemy release. + + SQLAlchemy 2.1 gives an enum attached to a table the metadata's schema, + 2.0 doesn't, so the types the tables use are checked, and so is the type + the helper builds before any table adopts it. + """ enum_columns = [ column for table in service_metadata.tables.values() @@ -48,3 +55,6 @@ def test_every_enum_type_names_the_public_schema() -> None: assert len(enum_columns) == 6 for column in enum_columns: assert column.type.schema == "public", f"{column.table.name}.{column.name}" + # The helper every enum column uses, before any table adopts its type. + helper_type = tables_module._enum_type(SimulationStatus, "v2_example") + assert helper_type.schema == "public" diff --git a/projects/policyengine-simulation-entry/DEPLOYMENT.md b/projects/policyengine-simulation-entry/DEPLOYMENT.md index b94b4dfc1..0c2ca55ea 100644 --- a/projects/policyengine-simulation-entry/DEPLOYMENT.md +++ b/projects/policyengine-simulation-entry/DEPLOYMENT.md @@ -48,9 +48,10 @@ readable by the runtime service account; and the `beta` or `prod` variable the deploy stops at once; without the tables, the role or its grants the candidate check below fails. -`GET /ready` reports, under `simulation_database`, whether the database -answered within three seconds (`reachable`; false also when it was reached -but didn't answer in time) and which table privileges its role lacks +`GET /ready` reports, under `simulation_database`, whether the check of the +database succeeded within three seconds (`reachable`; false when the +database wasn't reached, refused the role's login, or didn't answer in time) +and which table privileges its role lacks (`missing_privileges`, such as `"INSERT on reports"`; the list is in `libs/policyengine-simulation-persistence`). The database does not change the readiness status, because no old-gateway or Stage 12 route uses it. The diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/schemas.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/schemas.py index 1b6578e11..ef26a3ee4 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/schemas.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/schemas.py @@ -120,9 +120,9 @@ class TemporaryStage12ReportResponse(BaseModel): class SimulationDatabaseReadiness(BaseModel): """The simulation service's database, as the readiness check found it. - ``reachable`` says whether the database answered the check within its - time limit: false when it wasn't reached, or was reached but didn't - answer in time. ``missing_privileges`` names each table privilege the service + ``reachable`` says whether the check succeeded within its time limit: + false when the database wasn't reached, refused the role's login, or + didn't answer in time. ``missing_privileges`` names each table privilege the service needs and its role lacks, such as ``"INSERT on reports"``, or is null when the database was not reached. Neither changes ``status``: no old-gateway or Stage 12 route uses this database. diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py index 4e0ad3e68..eefff521c 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py @@ -55,6 +55,10 @@ async def missing_privileges(self) -> list[str]: ) def dispose(self) -> None: - """Stop the check's thread and close every pooled connection.""" + """Stop the check's thread and close every pooled connection. + + The thread doesn't restart, so a disposed database reports every later + check as failed; the app disposes of it once, at shutdown. + """ self.executor.shutdown(wait=False, cancel_futures=True) self.engine.dispose() diff --git a/projects/policyengine-simulation-entry/tests/test_simulation_database.py b/projects/policyengine-simulation-entry/tests/test_simulation_database.py index 8dd37539d..73c31b51b 100644 --- a/projects/policyengine-simulation-entry/tests/test_simulation_database.py +++ b/projects/policyengine-simulation-entry/tests/test_simulation_database.py @@ -258,21 +258,35 @@ async def three_checks_that_time_out(): database.executor.shutdown(wait=True) assert all(isinstance(result, TimeoutError) for result in results) - assert max(most_running_at_once) == 1 + # Only the first check ever ran: the two that timed out while it held the + # thread were cancelled before they started, and never ran afterwards. + assert most_running_at_once == [1] assert default_pool_threads == [] -def test_disposing_the_database_also_stops_its_thread(): +def test_disposing_the_database_also_stops_its_thread(monkeypatch): class FakeEngine: disposed = False def dispose(self): self.disposed = True + monkeypatch.setattr( + database_module, + "read_missing_table_privileges", + lambda engine, *, statement_timeout_milliseconds: [], + ) engine = FakeEngine() database = SimulationDatabase(engine) + asyncio.run(database.missing_privileges()) + [worker] = [ + thread + for thread in threading.enumerate() + if thread.name.startswith(database_module.READINESS_THREAD_NAME_PREFIX) + ] + database.dispose() + worker.join(timeout=5) assert engine.disposed is True - with pytest.raises(RuntimeError): - database.executor.submit(print) + assert not worker.is_alive() From 21760a9a8f34f40227e9dcc4cb18191cc189e311 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Wed, 4 Nov 2026 02:02:03 +0400 Subject: [PATCH 25/25] Test that a disposed database fails every later check, and finish the readiness wording A test now shows that /ready reports a disposed database's check as failed, and it fails if dispose leaves the check's thread running. The readiness docstrings, the smoke script's comment and the deployment document no longer speak of a database that was not reached where any failed check is meant, and the time limit is described per statement. Co-Authored-By: Claude Opus 5.5 --- .../cloud-run-simulation-entry-smoke.sh | 2 +- .../DEPLOYMENT.md | 7 +++--- .../src/policyengine_simulation_entry/app.py | 2 +- .../policyengine_simulation_entry/schemas.py | 8 +++---- .../simulation_database.py | 4 ++-- .../tests/test_simulation_database.py | 22 +++++++++++++++++++ 6 files changed, 34 insertions(+), 11 deletions(-) diff --git a/.github/scripts/cloud-run-simulation-entry-smoke.sh b/.github/scripts/cloud-run-simulation-entry-smoke.sh index 63db83f32..d51fb80c1 100755 --- a/.github/scripts/cloud-run-simulation-entry-smoke.sh +++ b/.github/scripts/cloud-run-simulation-entry-smoke.sh @@ -78,7 +78,7 @@ fi routing_max_attempts="${SIMULATION_ENTRYPOINT_ROUTING_MAX_ATTEMPTS:-24}" routing_retry_delay_seconds="${SIMULATION_ENTRYPOINT_ROUTING_RETRY_DELAY_SECONDS:-5}" # A cold candidate may not reach its database within /ready's three-second -# wait; an unreachable database is asked about again, up to this many +# wait; a failed database check is asked about again, up to this many # requests in all. database_ready_max_attempts="${SIMULATION_DATABASE_READY_MAX_ATTEMPTS:-6}" diff --git a/projects/policyengine-simulation-entry/DEPLOYMENT.md b/projects/policyengine-simulation-entry/DEPLOYMENT.md index 0c2ca55ea..51d24837a 100644 --- a/projects/policyengine-simulation-entry/DEPLOYMENT.md +++ b/projects/policyengine-simulation-entry/DEPLOYMENT.md @@ -57,9 +57,10 @@ and which table privileges its role lacks readiness status, because no old-gateway or Stage 12 route uses it. The candidate smoke check (`REQUIRE_SIMULATION_DATABASE_ACCESS=true`) requires the database reached with no privilege missing, which shows that the role's URL -reached the revision and that the role holds its grants. While a cold -candidate has not reached its database yet, it asks again, up to six requests -in all; a reached database with a missing privilege fails at once. The stable +reached the revision and that the role holds its grants. While the check +fails, as it does for a cold candidate that hasn't reached its database yet, +it asks again, up to six requests in all; a database that answers with a +privilege missing fails at once. The stable and rollback checks don't require it, because an older image may not use the database. diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py index 2c8f5bc04..143d7ac40 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/app.py @@ -470,7 +470,7 @@ async def get_budget_window_job( async def _simulation_database_readiness( database: SimulationDatabaseConnection, ) -> SimulationDatabaseReadiness: - """Report whether the service reached its database, and what its role lacks.""" + """Report whether the database check succeeded, and what the role lacks.""" try: missing_privileges = await asyncio.wait_for( database.missing_privileges(), diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/schemas.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/schemas.py index ef26a3ee4..dbfd3e037 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/schemas.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/schemas.py @@ -122,10 +122,10 @@ class SimulationDatabaseReadiness(BaseModel): ``reachable`` says whether the check succeeded within its time limit: false when the database wasn't reached, refused the role's login, or - didn't answer in time. ``missing_privileges`` names each table privilege the service - needs and its role lacks, such as ``"INSERT on reports"``, or is null when - the database was not reached. Neither changes ``status``: no old-gateway or - Stage 12 route uses this database. + didn't answer in time. ``missing_privileges`` names each table privilege + the service needs and its role lacks, such as ``"INSERT on reports"``, or + is null when the check failed. Neither changes ``status``: no old-gateway + or Stage 12 route uses this database. """ model_config = ConfigDict(extra="forbid", frozen=True) diff --git a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py index eefff521c..63a95feda 100644 --- a/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py +++ b/projects/policyengine-simulation-entry/src/policyengine_simulation_entry/simulation_database.py @@ -15,8 +15,8 @@ if TYPE_CHECKING: from sqlalchemy import Engine -# The readiness check stops waiting after three seconds; the database ends the -# query sooner, once the connection answers. +# The readiness check stops waiting after three seconds; the database ends +# each statement of the check that runs longer than this, once connected. READINESS_STATEMENT_TIMEOUT_MILLISECONDS = 2000 READINESS_THREAD_NAME_PREFIX = "simulation-database-readiness" diff --git a/projects/policyengine-simulation-entry/tests/test_simulation_database.py b/projects/policyengine-simulation-entry/tests/test_simulation_database.py index 73c31b51b..65762e8ca 100644 --- a/projects/policyengine-simulation-entry/tests/test_simulation_database.py +++ b/projects/policyengine-simulation-entry/tests/test_simulation_database.py @@ -290,3 +290,25 @@ def dispose(self): assert engine.disposed is True assert not worker.is_alive() + + +def test_a_disposed_database_reports_every_later_check_as_failed(monkeypatch): + class FakeEngine: + def dispose(self): + return None + + monkeypatch.setattr( + database_module, + "read_missing_table_privileges", + lambda engine, *, statement_timeout_milliseconds: [], + ) + database = SimulationDatabase(FakeEngine()) + database.dispose() + + with _client(FakeBackend(), database) as client: + result = client.get("/ready") + + assert result.json()["simulation_database"] == { + "reachable": False, + "missing_privileges": None, + }