diff --git a/migrations/env.py b/migrations/env.py index 40e4f85..32d39d9 100644 --- a/migrations/env.py +++ b/migrations/env.py @@ -6,7 +6,7 @@ from alembic import context from sqlalchemy import engine_from_config, pool from sbom_nexus.config import database_target, migration_role -from sbom_nexus.database import database_url, metadata +from sbom_nexus.database import assume_migration_role, database_url, metadata config = context.config @@ -45,7 +45,7 @@ def run_migrations_online() -> None: if role and connection.dialect.name != "postgresql": raise RuntimeError("SBOM_NEXUS_MIGRATION_ROLE requires PostgreSQL") if role: - connection.exec_driver_sql(f'SET ROLE "{role}"') + assume_migration_role(connection, role) context.configure( connection=connection, target_metadata=target_metadata, diff --git a/src/sbom_nexus/database.py b/src/sbom_nexus/database.py index a5cd60e..2d36a42 100644 --- a/src/sbom_nexus/database.py +++ b/src/sbom_nexus/database.py @@ -21,7 +21,7 @@ from sqlalchemy import ( create_engine, event, ) -from sqlalchemy.engine import Engine +from sqlalchemy.engine import Connection, Engine NAMING_CONVENTION = { "ix": "ix_%(table_name)s_%(column_0_name)s", @@ -130,6 +130,15 @@ def database_url(value: str | Path) -> str: return text +def assume_migration_role(connection: Connection, role: str) -> None: + """Assume the durable owner without leaving Alembic inside an outer txn.""" + connection.exec_driver_sql(f'SET ROLE "{role}"') + # SQLAlchemy 2 autobegins on SET ROLE. Commit that transaction before + # Alembic opens its own so the migration transaction is not rolled back + # when the connection closes. SET ROLE remains active for the session. + connection.commit() + + def _dynamic_connection_factory(url_file: Path): def connect(): try: diff --git a/tests/test_database_rotation.py b/tests/test_database_rotation.py index 742e874..9281ea8 100644 --- a/tests/test_database_rotation.py +++ b/tests/test_database_rotation.py @@ -5,6 +5,21 @@ from pathlib import Path from sbom_nexus import database +def test_migration_role_commits_autobegin_before_alembic_transaction() -> None: + events: list[str] = [] + + class Connection: + def exec_driver_sql(self, statement: str) -> None: + events.append(statement) + + def commit(self) -> None: + events.append("COMMIT") + + database.assume_migration_role(Connection(), "sbom_nexus_owner") + + assert events == ['SET ROLE "sbom_nexus_owner"', "COMMIT"] + + def test_dynamic_connection_factory_rereads_mounted_url(monkeypatch, tmp_path: Path) -> None: secret = tmp_path / "url" observed: list[str] = [] diff --git a/workplans/SBOM-WP-0004-database-lease-rotation.md b/workplans/SBOM-WP-0004-database-lease-rotation.md index 52daeae..f06671f 100644 --- a/workplans/SBOM-WP-0004-database-lease-rotation.md +++ b/workplans/SBOM-WP-0004-database-lease-rotation.md @@ -37,6 +37,11 @@ replaced with the current mounted credential. Completed with tests proving two connection attempts observe two file values and that the engine URL contains neither username nor password. +The first live promotion also exposed an Alembic `SET ROLE` autobegin defect: +the migration logged success but its outer transaction rolled back on close. +The migration role helper now commits only the role-assumption transaction +before Alembic starts its own, with an ordering regression test. + ## Separate liveness from database readiness ```task