diff --git a/.all-contributorsrc b/.all-contributorsrc index 1a7e2eb7bb..f8898e5f97 100644 --- a/.all-contributorsrc +++ b/.all-contributorsrc @@ -1222,6 +1222,17 @@ "contributions": [ "code" ] + }, + { + "login": "hongzexin", + "name": "Jason HONG", + "avatar_url": "https://avatars.githubusercontent.com/u/136784169?v=4", + "profile": "https://github.com/hongzexin", + "contributions": [ + "code", + "test", + "doc" + ] } ], "contributorsPerLine": 7, diff --git a/.env.example b/.env.example index 1e75b20df7..96322c1f04 100644 --- a/.env.example +++ b/.env.example @@ -15,6 +15,8 @@ # Database. SQLite (default) needs nothing. For PostgreSQL: # CODEX_LB_DATABASE_URL=postgresql+asyncpg://codex_lb:codex_lb@127.0.0.1:5432/codex_lb +# Optional shared-DB schema isolation (runtime ",public"; migrations ""): +# CODEX_LB_DATABASE_POSTGRES_SCHEMA=codex_lb_prod # Encryption key file (pin for Docker volumes; must be shared across replicas). # CODEX_LB_ENCRYPTION_KEY_FILE=/var/lib/codex-lb/encryption.key diff --git a/README.md b/README.md index 5fa37d9099..6b0bd131aa 100644 --- a/README.md +++ b/README.md @@ -283,6 +283,9 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/e glopyglerky
glopyglerky

💻 ⚠️ Ahmad Maulana Iqbal
Ahmad Maulana Iqbal

💻 ⚠️ + + Jason HONG
Jason HONG

💻 ⚠️ 📖 + diff --git a/app/core/config/settings.py b/app/core/config/settings.py index e38723400d..b05907572f 100644 --- a/app/core/config/settings.py +++ b/app/core/config/settings.py @@ -249,6 +249,7 @@ class Settings(BaseSettings): database_sqlite_pre_migrate_backup_enabled: bool = True database_sqlite_pre_migrate_backup_max_files: int = Field(default=5, ge=1) database_sqlite_startup_check_mode: Literal["quick", "full", "off"] = "quick" + database_postgres_schema: str | None = None database_alembic_auto_remap_enabled: bool = True database_migration_lock_timeout_seconds: float = Field(default=300.0, gt=0) upstream_base_url: str = "https://chatgpt.com/backend-api" @@ -491,6 +492,20 @@ def _expand_database_url(cls, value: str) -> str: return f"{prefix}{Path(path).expanduser()}" return value + @field_validator("database_postgres_schema", mode="before") + @classmethod + def _normalize_database_postgres_schema(cls, value: OptionalStringInput) -> str | None: + if value is None: + return None + if isinstance(value, str): + stripped = value.strip() + if not stripped: + return None + if len(stripped.encode("utf-8")) > 63: + raise ValueError("database_postgres_schema must be at most 63 UTF-8 bytes") + return stripped + raise TypeError("database_postgres_schema must be a string") + @field_validator("encryption_key_file", mode="before") @classmethod def _expand_encryption_key_file(cls, value: str | Path) -> Path: diff --git a/app/db/alembic/env.py b/app/db/alembic/env.py index ab5d485c7b..2d3fce6627 100644 --- a/app/db/alembic/env.py +++ b/app/db/alembic/env.py @@ -3,10 +3,16 @@ from logging.config import fileConfig from alembic import context -from sqlalchemy import engine_from_config, pool +from sqlalchemy import engine_from_config, make_url, pool from app.core.config.settings import get_settings -from app.db.migration_url import to_sync_database_url +from app.db.migration_url import ( + apply_postgres_migration_search_path, + ensure_postgres_schema_exists, + normalize_postgres_schema, + postgres_migration_search_path, + to_sync_database_url, +) from app.db.models import Base config = context.config @@ -27,6 +33,9 @@ def _sync_database_url() -> str: def run_migrations_offline() -> None: url = _sync_database_url() config.set_main_option("sqlalchemy.url", url) + schema = None + if make_url(url).get_backend_name() == "postgresql": + schema = normalize_postgres_schema(get_settings().database_postgres_schema) context.configure( url=url, @@ -35,8 +44,13 @@ def run_migrations_offline() -> None: dialect_opts={"paramstyle": "named"}, compare_type=True, render_as_batch=url.startswith("sqlite"), + version_table_schema=schema, ) + search_path = postgres_migration_search_path(schema) + if search_path is not None: + context.execute(f"SET search_path TO {search_path}") + with context.begin_transaction(): context.run_migrations() @@ -53,11 +67,19 @@ def run_migrations_online() -> None: ) with connectable.connect() as connection: + schema = normalize_postgres_schema(get_settings().database_postgres_schema) + ensure_postgres_schema_exists(connection, schema) + apply_postgres_migration_search_path(connection, schema) + if connection.dialect.name == "postgresql" and connection.in_transaction(): + # Schema creation and set_config autobegin a transaction. Commit it + # before Alembic owns transaction boundaries and autocommit blocks. + connection.commit() context.configure( connection=connection, target_metadata=target_metadata, compare_type=True, render_as_batch=connection.dialect.name == "sqlite", + version_table_schema=schema if connection.dialect.name == "postgresql" else None, ) with context.begin_transaction(): diff --git a/app/db/alembic/versions/20260310_000000_fix_postgresql_enum_value_casing.py b/app/db/alembic/versions/20260310_000000_fix_postgresql_enum_value_casing.py index e5b34ca550..95a32f61b6 100644 --- a/app/db/alembic/versions/20260310_000000_fix_postgresql_enum_value_casing.py +++ b/app/db/alembic/versions/20260310_000000_fix_postgresql_enum_value_casing.py @@ -43,7 +43,8 @@ def _enum_value_exists(bind: sa.engine.Connection, enum_type_name: str, enum_val sa.text( "SELECT 1 FROM pg_enum e " "JOIN pg_type t ON e.enumtypid = t.oid " - "WHERE t.typname = :type_name AND e.enumlabel = :value" + "WHERE pg_type_is_visible(t.oid) " + "AND t.typname = :type_name AND e.enumlabel = :value" ), {"type_name": enum_type_name, "value": enum_value}, ) diff --git a/app/db/alembic/versions/20260604_000000_add_reauth_required_account_status.py b/app/db/alembic/versions/20260604_000000_add_reauth_required_account_status.py index 4b6d686890..04b9c1e5b3 100644 --- a/app/db/alembic/versions/20260604_000000_add_reauth_required_account_status.py +++ b/app/db/alembic/versions/20260604_000000_add_reauth_required_account_status.py @@ -73,7 +73,8 @@ def _enum_value_exists(enum_type_name: str, enum_value: str) -> bool: sa.text( "SELECT 1 FROM pg_enum e " "JOIN pg_type t ON e.enumtypid = t.oid " - "WHERE t.typname = :type_name AND e.enumlabel = :value" + "WHERE pg_type_is_visible(t.oid) " + "AND t.typname = :type_name AND e.enumlabel = :value" ), {"type_name": enum_type_name, "value": enum_value}, ) diff --git a/app/db/migrate.py b/app/db/migrate.py index a75e404255..35b41a05d1 100644 --- a/app/db/migrate.py +++ b/app/db/migrate.py @@ -22,7 +22,13 @@ from app.core.config.settings import get_settings from app.db.alembic.revision_ids import LEGACY_MIGRATION_TO_NEW_REVISION, OLD_TO_NEW_REVISION_MAP, REVISION_ID_PATTERN from app.db.migration_lock import migration_lock -from app.db.migration_url import to_sync_database_url +from app.db.migration_url import ( + apply_postgres_migration_search_path, + ensure_postgres_schema_exists, + normalize_postgres_schema, + postgres_qualified_name, + to_sync_database_url, +) from app.db.models import Base logger = logging.getLogger(__name__) @@ -178,6 +184,8 @@ def _sync_connection(sync_database_url: str) -> Iterator[Connection]: engine = create_engine(sync_database_url, future=True) try: with engine.connect() as connection: + ensure_postgres_schema_exists(connection, get_settings().database_postgres_schema) + apply_postgres_migration_search_path(connection, get_settings().database_postgres_schema) yield connection finally: engine.dispose() @@ -188,6 +196,8 @@ def _sync_transaction(sync_database_url: str) -> Iterator[Connection]: engine = create_engine(sync_database_url, future=True) try: with engine.begin() as connection: + ensure_postgres_schema_exists(connection, get_settings().database_postgres_schema) + apply_postgres_migration_search_path(connection, get_settings().database_postgres_schema) yield connection finally: engine.dispose() @@ -195,18 +205,31 @@ def _sync_transaction(sync_database_url: str) -> Iterator[Connection]: def _read_table_names(connection: Connection) -> set[str]: inspector = inspect(connection) - return set(inspector.get_table_names()) + schema = _configured_postgres_schema(connection) + return set(inspector.get_table_names(schema=schema)) + + +def _configured_postgres_schema(connection: Connection) -> str | None: + if connection.dialect.name != "postgresql": + return None + return normalize_postgres_schema(get_settings().database_postgres_schema) + + +def _migration_table_name(connection: Connection, table_name: str) -> str: + return postgres_qualified_name(table_name, _configured_postgres_schema(connection)) def _read_legacy_migration_names(connection: Connection) -> set[str]: - result = connection.execute(text(f"SELECT name FROM {_LEGACY_MIGRATIONS_TABLE}")) + table_name = _migration_table_name(connection, _LEGACY_MIGRATIONS_TABLE) + result = connection.execute(text(f"SELECT name FROM {table_name}")) names = {str(row[0]) for row in result.fetchall() if row and row[0] is not None} return names def _read_current_revisions_from_connection(connection: Connection) -> tuple[str, ...]: + table_name = _migration_table_name(connection, _ALEMBIC_VERSION_TABLE) try: - rows = connection.execute(text(f"SELECT {_ALEMBIC_VERSION_COLUMN} FROM {_ALEMBIC_VERSION_TABLE}")).fetchall() + rows = connection.execute(text(f"SELECT {_ALEMBIC_VERSION_COLUMN} FROM {table_name}")).fetchall() except (sa_exc.ProgrammingError, sa_exc.OperationalError) as exc: # PostgreSQL can still raise UndefinedTable here on a fresh database if # the alembic_version table is absent when startup migration state is @@ -355,7 +378,9 @@ def _ensure_alembic_version_table_capacity_for_connection(connection: Connection return inspector = inspect(connection) - if not inspector.has_table(_ALEMBIC_VERSION_TABLE): + schema = _configured_postgres_schema(connection) + table_name = postgres_qualified_name(_ALEMBIC_VERSION_TABLE, schema) + if not inspector.has_table(_ALEMBIC_VERSION_TABLE, schema=schema): # IF NOT EXISTS is defense-in-depth against concurrent out-of-band # `alembic upgrade` invocations that bypass run_upgrade's migration # lock; the product paths are already serialized by migration_lock. @@ -363,7 +388,7 @@ def _ensure_alembic_version_table_capacity_for_connection(connection: Connection text( " ".join( ( - f"CREATE TABLE IF NOT EXISTS {_ALEMBIC_VERSION_TABLE} (", + f"CREATE TABLE IF NOT EXISTS {table_name} (", f"{_ALEMBIC_VERSION_COLUMN} VARCHAR({required_length}) NOT NULL,", f"PRIMARY KEY ({_ALEMBIC_VERSION_COLUMN})", ")", @@ -373,7 +398,7 @@ def _ensure_alembic_version_table_capacity_for_connection(connection: Connection ) return - columns = inspector.get_columns(_ALEMBIC_VERSION_TABLE) + columns = inspector.get_columns(_ALEMBIC_VERSION_TABLE, schema=schema) version_num_column = next((column for column in columns if column.get("name") == _ALEMBIC_VERSION_COLUMN), None) if version_num_column is None: raise MigrationBootstrapError( @@ -385,10 +410,7 @@ def _ensure_alembic_version_table_capacity_for_connection(connection: Connection return connection.execute( - text( - f"ALTER TABLE {_ALEMBIC_VERSION_TABLE} " - f"ALTER COLUMN {_ALEMBIC_VERSION_COLUMN} TYPE VARCHAR({required_length})" - ) + text(f"ALTER TABLE {table_name} ALTER COLUMN {_ALEMBIC_VERSION_COLUMN} TYPE VARCHAR({required_length})") ) @@ -478,13 +500,11 @@ def _remap_legacy_alembic_revisions(config: Config) -> tuple[str, ...]: if remapped == current_revisions: return () - connection.execute(text(f"DELETE FROM {_ALEMBIC_VERSION_TABLE}")) + table_name = _migration_table_name(connection, _ALEMBIC_VERSION_TABLE) + connection.execute(text(f"DELETE FROM {table_name}")) for revision in remapped: connection.execute( - text( - f"INSERT INTO {_ALEMBIC_VERSION_TABLE} ({_ALEMBIC_VERSION_COLUMN}) " - f"VALUES (:{_ALEMBIC_VERSION_COLUMN})" - ), + text(f"INSERT INTO {table_name} ({_ALEMBIC_VERSION_COLUMN}) VALUES (:{_ALEMBIC_VERSION_COLUMN})"), {_ALEMBIC_VERSION_COLUMN: revision}, ) diff --git a/app/db/migration_url.py b/app/db/migration_url.py index f9dadbf5f9..3145ae0eaf 100644 --- a/app/db/migration_url.py +++ b/app/db/migration_url.py @@ -1,6 +1,8 @@ from __future__ import annotations -from sqlalchemy.engine import make_url +from sqlalchemy import text +from sqlalchemy.engine import Connection, make_url +from sqlalchemy.schema import CreateSchema from app.db.sqlite_utils import normalize_sqlite_url @@ -16,3 +18,78 @@ def to_sync_database_url(database_url: str) -> str: parsed = parsed.set(drivername="postgresql+psycopg") return parsed.render_as_string(hide_password=False) + + +def normalize_postgres_schema(schema: str | None) -> str | None: + if schema is None: + return None + normalized = schema.strip() + if not normalized: + return None + return normalized + + +def quote_postgres_identifier(identifier: str) -> str: + return f'"{identifier.replace(chr(34), chr(34) * 2)}"' + + +def postgres_search_path(schema: str | None) -> str | None: + normalized = normalize_postgres_schema(schema) + if normalized is None: + return None + quoted = quote_postgres_identifier(normalized) + if normalized == "public": + return quoted + return f"{quoted},public" + + +def postgres_migration_search_path(schema: str | None) -> str | None: + normalized = normalize_postgres_schema(schema) + if normalized is None: + return None + return quote_postgres_identifier(normalized) + + +def postgres_qualified_name(identifier: str, schema: str | None) -> str: + normalized = normalize_postgres_schema(schema) + quoted_identifier = quote_postgres_identifier(identifier) + if normalized is None: + return quoted_identifier + return f"{quote_postgres_identifier(normalized)}.{quoted_identifier}" + + +def ensure_postgres_schema_exists(connection: Connection, schema: str | None) -> None: + if schema is None or connection.dialect.name != "postgresql": + return + + normalized = normalize_postgres_schema(schema) + if normalized is None: + return + if normalized == "public": + return + + connection.execute(CreateSchema(normalized, if_not_exists=True)) + + +def apply_postgres_search_path(connection: Connection, schema: str | None) -> None: + search_path = postgres_search_path(schema) + if search_path is None or connection.dialect.name != "postgresql": + return + connection.execute( + text("SELECT set_config('search_path', :search_path, false)"), + {"search_path": search_path}, + ) + + +def apply_postgres_migration_search_path(connection: Connection, schema: str | None) -> None: + normalized = normalize_postgres_schema(schema) + search_path = postgres_migration_search_path(normalized) + if search_path is None or connection.dialect.name != "postgresql": + return + connection.execute( + text("SELECT set_config('search_path', :search_path, false)"), + {"search_path": search_path}, + ) + # Reflection caches PostgreSQL's default schema before this deployment- + # specific search path is applied. Keep it aligned with unqualified DDL. + connection.dialect.default_schema_name = normalized diff --git a/app/db/session.py b/app/db/session.py index cd8a45a729..7ab8bf3591 100644 --- a/app/db/session.py +++ b/app/db/session.py @@ -17,6 +17,7 @@ from sqlalchemy.pool import NullPool from app.core.config.settings import get_settings +from app.db.migration_url import postgres_search_path from app.db.sqlite_utils import ( SqliteIntegrityCheckMode, check_sqlite_integrity, @@ -76,7 +77,11 @@ def _postgres_async_connect_args(url: str) -> dict[str, object] | None: # bridge-session cleanup stop running, and account/stream lease expiry is # mis-evaluated. Forcing UTC keeps stored timestamps correct regardless of # the container time zone. - connect_args: dict[str, object] = {"server_settings": {"timezone": "UTC"}} + server_settings: dict[str, str] = {"timezone": "UTC"} + search_path = postgres_search_path(_settings.database_postgres_schema) + if search_path is not None: + server_settings["search_path"] = search_path + connect_args: dict[str, object] = {"server_settings": server_settings} if os.environ.get("CODEX_LB_TEST_DATABASE_URL"): connect_args["prepared_statement_cache_size"] = 0 return connect_args diff --git a/app/modules/proxy/durable_bridge_repository.py b/app/modules/proxy/durable_bridge_repository.py index bcea92b62b..4b73e2993c 100644 --- a/app/modules/proxy/durable_bridge_repository.py +++ b/app/modules/proxy/durable_bridge_repository.py @@ -1786,7 +1786,7 @@ async def missing_durable_bridge_tables(session: AsyncSession) -> tuple[str, ... result = await session.execute( text( "SELECT table_name FROM information_schema.tables " - "WHERE table_schema = 'public' " + "WHERE table_schema = ANY (current_schemas(false)) " "AND table_name IN (" "'http_bridge_sessions', 'http_bridge_session_aliases', 'http_bridge_retry_circuits', " "'http_bridge_recovery_attempts'" diff --git a/deploy/helm/codex-lb/README.md b/deploy/helm/codex-lb/README.md index b661342553..b39326bec0 100644 --- a/deploy/helm/codex-lb/README.md +++ b/deploy/helm/codex-lb/README.md @@ -216,6 +216,7 @@ This chart intentionally keeps migration behavior explicit by install mode. - Application pods use a schema gate initContainer when `migration.enabled=true`, `config.databaseMigrateOnStartup=false`, and `migration.schemaGate.enabled=true`. - That initContainer runs `python -m app.db.migrate wait-for-head` and blocks the app container until the database is at Alembic head. - In bundled mode, `values-bundled.yaml` enables startup migration instead of the schema gate so fresh self-contained installs do not deadlock on `helm install --wait`. +- Shared PostgreSQL installs can isolate codex-lb in an application schema; see the [Kubernetes deployment guide](https://soju06.github.io/codex-lb/deployment/kubernetes/#shared-postgresql-databases). This means: diff --git a/deploy/helm/codex-lb/templates/configmap.yaml b/deploy/helm/codex-lb/templates/configmap.yaml index f4833af865..3bed31053f 100644 --- a/deploy/helm/codex-lb/templates/configmap.yaml +++ b/deploy/helm/codex-lb/templates/configmap.yaml @@ -14,6 +14,9 @@ data: CODEX_LB_DATABASE_MIGRATE_ON_STARTUP: {{ .Values.config.databaseMigrateOnStartup | toString | quote }} CODEX_LB_DATABASE_POOL_SIZE: {{ .Values.config.databasePoolSize | toString | quote }} CODEX_LB_DATABASE_MAX_OVERFLOW: {{ .Values.config.databaseMaxOverflow | toString | quote }} + {{- if .Values.config.databasePostgresSchema }} + CODEX_LB_DATABASE_POSTGRES_SCHEMA: {{ .Values.config.databasePostgresSchema | quote }} + {{- end }} # Upstream {{- if .Values.config.upstreamBaseUrl }} CODEX_LB_UPSTREAM_BASE_URL: {{ .Values.config.upstreamBaseUrl | quote }} diff --git a/deploy/helm/codex-lb/templates/deployment.yaml b/deploy/helm/codex-lb/templates/deployment.yaml index b2e39df3b6..e271f13e79 100644 --- a/deploy/helm/codex-lb/templates/deployment.yaml +++ b/deploy/helm/codex-lb/templates/deployment.yaml @@ -209,6 +209,10 @@ spec: secretKeyRef: name: {{ include "codex-lb.databaseUrlSecretName" . }} key: {{ .Values.auth.secretKeys.databaseUrl }} + {{- if .Values.config.databasePostgresSchema }} + - name: CODEX_LB_DATABASE_POSTGRES_SCHEMA + value: {{ .Values.config.databasePostgresSchema | quote }} + {{- end }} - name: CODEX_LB_ENCRYPTION_KEY_FILE value: /var/lib/codex-lb/encryption.key volumeMounts: @@ -240,6 +244,10 @@ spec: secretKeyRef: name: {{ include "codex-lb.databaseUrlSecretName" . }} key: {{ .Values.auth.secretKeys.databaseUrl }} + {{- if .Values.config.databasePostgresSchema }} + - name: CODEX_LB_DATABASE_POSTGRES_SCHEMA + value: {{ .Values.config.databasePostgresSchema | quote }} + {{- end }} - name: CODEX_LB_ENCRYPTION_KEY_FILE value: /var/lib/codex-lb/encryption.key volumeMounts: diff --git a/deploy/helm/codex-lb/templates/hooks/migration-job.yaml b/deploy/helm/codex-lb/templates/hooks/migration-job.yaml index 0d0bb9993c..ba0f5fb1f0 100644 --- a/deploy/helm/codex-lb/templates/hooks/migration-job.yaml +++ b/deploy/helm/codex-lb/templates/hooks/migration-job.yaml @@ -95,6 +95,10 @@ spec: - name: CODEX_LB_DATABASE_URL value: {{ include "codex-lb.databaseUrl" . | quote }} {{- end }} + {{- if .Values.config.databasePostgresSchema }} + - name: CODEX_LB_DATABASE_POSTGRES_SCHEMA + value: {{ .Values.config.databasePostgresSchema | quote }} + {{- end }} {{- with .Values.extraEnv }} {{- toYaml . | nindent 12 }} {{- end }} diff --git a/deploy/helm/codex-lb/values-external-db.yaml b/deploy/helm/codex-lb/values-external-db.yaml index 82a19c65ac..353fea18fa 100644 --- a/deploy/helm/codex-lb/values-external-db.yaml +++ b/deploy/helm/codex-lb/values-external-db.yaml @@ -12,3 +12,6 @@ migration: # - externalDatabase.host/user/database (+ optional port) # - externalDatabase.existingSecret # - auth.existingSecret +# +# Optional for shared PostgreSQL databases: +# config.databasePostgresSchema: codex_lb_prod diff --git a/deploy/helm/codex-lb/values.schema.json b/deploy/helm/codex-lb/values.schema.json index 292d550eb0..4d380f921d 100644 --- a/deploy/helm/codex-lb/values.schema.json +++ b/deploy/helm/codex-lb/values.schema.json @@ -207,6 +207,9 @@ "config": { "type": "object", "properties": { + "databasePostgresSchema": { + "type": "string" + }, "shutdownDrainTimeoutSeconds": { "type": "integer", "minimum": 1 diff --git a/deploy/helm/codex-lb/values.yaml b/deploy/helm/codex-lb/values.yaml index c1b67733b6..035278f5cb 100644 --- a/deploy/helm/codex-lb/values.yaml +++ b/deploy/helm/codex-lb/values.yaml @@ -123,6 +123,8 @@ serviceAccount: config: # @param config.databaseMigrateOnStartup Disable auto-migration (migration Job handles it) databaseMigrateOnStartup: false + # @param config.databasePostgresSchema Optional PostgreSQL schema search_path prefix for shared databases + databasePostgresSchema: "" # @param config.databasePoolSize SQLAlchemy connection pool size # Each supported replica has one worker with two pools (request + background). # Reserve 20 of PostgreSQL's default 100 slots for PG internals, migrations, and operations. diff --git a/docs/deployment/kubernetes.md b/docs/deployment/kubernetes.md index 6031e1978f..1cb2545532 100644 --- a/docs/deployment/kubernetes.md +++ b/docs/deployment/kubernetes.md @@ -12,6 +12,39 @@ kubectl port-forward svc/codex-lb 2455:2455 Open [localhost:2455](http://localhost:2455) → Add account → Done. +## Shared PostgreSQL databases + +For an external PostgreSQL database shared by multiple applications, assign +codex-lb its own schema with `config.databasePostgresSchema`: + +```yaml +postgresql: + enabled: false +externalDatabase: + existingSecret: codex-lb-app-secret +config: + databasePostgresSchema: codex_lb_prod +``` + +The runtime searches `codex_lb_prod` first and retains `public` as a read +fallback. Migration jobs and schema-gate init containers operate only in +`codex_lb_prod`, so another application's `public.alembic_version` cannot make +an uninitialized codex-lb schema appear current. + +On the first migration run, codex-lb executes `CREATE SCHEMA IF NOT EXISTS` +before inspecting or advancing Alembic state. If the deployment user cannot +create schemas, create `codex_lb_prod` once as a database administrator. Keep +the same `config.databasePostgresSchema` value for the workload, migration job, +and schema gate; the chart wires it to all three paths. + +The setting is optional. Omitting it preserves PostgreSQL's existing default +search-path behavior. See the owning +[database-backends](https://github.com/Soju06/codex-lb/tree/main/openspec/specs/database-backends), +[database-migrations](https://github.com/Soju06/codex-lb/tree/main/openspec/specs/database-migrations), +and +[deployment-installation](https://github.com/Soju06/codex-lb/tree/main/openspec/specs/deployment-installation) +contracts. + ## Multi-replica behavior The Helm chart auto-configures HTTP `/responses` owner handoff for multi-replica installs using a headless-service DNS name per pod. The default cluster domain is `cluster.local`; set Helm `clusterDomain` if your cluster uses a different suffix. Override `config.sessionBridgeAdvertiseBaseUrl` only if pods must be reached through a different internal address. diff --git a/docs/reference/settings.md b/docs/reference/settings.md index e7486b2321..2551bac015 100644 --- a/docs/reference/settings.md +++ b/docs/reference/settings.md @@ -7,7 +7,7 @@ Regenerate with `uv run python scripts/generate_settings_reference.py`; `tests/unit/test_settings_reference.py` fails when this page drifts from `app/core/config/settings.py`. -codex-lb currently exposes 118 settings. Every setting is an environment +codex-lb currently exposes 119 settings. Every setting is an environment variable with the `CODEX_LB_` prefix (process environment or `.env` / `.env.local` next to the process). All defaults work with zero configuration — start from [Configuration](../configuration.md) for the handful that matter, @@ -37,6 +37,7 @@ the host side of the compose `ports` mapping instead. | `CODEX_LB_DATABASE_MIGRATION_LOCK_TIMEOUT_SECONDS` | `float` | `300.0` | | `CODEX_LB_DATABASE_MIGRATIONS_FAIL_FAST` | `bool` | `True` | | `CODEX_LB_DATABASE_POOL_SIZE` | `int` | `15` | +| `CODEX_LB_DATABASE_POSTGRES_SCHEMA` | `str \| None` | `None` | | `CODEX_LB_DATABASE_SQLITE_PRE_MIGRATE_BACKUP_ENABLED` | `bool` | `True` | | `CODEX_LB_DATABASE_SQLITE_PRE_MIGRATE_BACKUP_MAX_FILES` | `int` | `5` | | `CODEX_LB_DATABASE_SQLITE_STARTUP_CHECK_MODE` | `'quick' \| 'full' \| 'off'` | `'quick'` | diff --git a/openspec/changes/support-postgres-schema-overrides/.openspec.yaml b/openspec/changes/support-postgres-schema-overrides/.openspec.yaml new file mode 100644 index 0000000000..5081c98763 --- /dev/null +++ b/openspec/changes/support-postgres-schema-overrides/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-12 diff --git a/openspec/changes/support-postgres-schema-overrides/proposal.md b/openspec/changes/support-postgres-schema-overrides/proposal.md new file mode 100644 index 0000000000..fb98d139b6 --- /dev/null +++ b/openspec/changes/support-postgres-schema-overrides/proposal.md @@ -0,0 +1,40 @@ +# Change: support-postgres-schema-overrides + +## Why + +CHEK's production deployment reuses an existing PostgreSQL instance instead of +provisioning a dedicated database per application. `codex-lb` currently assumes +the PostgreSQL session operates in `public`, so runtime queries, migration +checks, and durable-bridge table guards all target the default schema. In a +shared database that can leak into the wrong schema, report missing tables even +after successful migrations, or require operators to create one database per +deployment just to isolate objects. + +## What Changes + +- Add one explicit `database_postgres_schema` setting that, when configured, + pins PostgreSQL runtime sessions to `,public` and migration paths to + the configured schema only. +- Make Alembic, startup drift checks, migration waits, and durable-bridge table + detection honor the configured active PostgreSQL schemas instead of assuming + `public`. +- Ensure migration/bootstrap paths create the configured PostgreSQL schema on + first use when the database user is allowed to do so, so shared-database + installs do not accidentally fall back to `public` Alembic state. +- Scope migration search paths and Alembic's version table to the configured + schema only, while retaining the documented `public` fallback for runtime + application queries. +- Surface the setting through the Helm chart and environment examples so shared + database installs can keep the app, migration job, and readiness guards on + the same schema contract. +- Document the shared-database installation path and cover the new wiring with + focused unit tests. + +## Impact + +- Affected specs: `database-backends`, `database-migrations`, + `deployment-installation` +- Affected code: settings, database session wiring, migration helpers, + durable-bridge schema detection, Helm values/templates, and documentation +- Existing dedicated-database installs remain unchanged because the default is + unset and preserves PostgreSQL's normal search path diff --git a/openspec/changes/support-postgres-schema-overrides/specs/database-backends/spec.md b/openspec/changes/support-postgres-schema-overrides/specs/database-backends/spec.md new file mode 100644 index 0000000000..2ede91e3ee --- /dev/null +++ b/openspec/changes/support-postgres-schema-overrides/specs/database-backends/spec.md @@ -0,0 +1,44 @@ +## ADDED Requirements + +### Requirement: PostgreSQL runtime sessions honor an optional application schema + +When `database_url` resolves to PostgreSQL and `database_postgres_schema` is a +non-empty string, the application MUST configure every runtime SQLAlchemy +asyncpg connection with a PostgreSQL search path of `,public`. +This requirement applies to the request-path engine, the optional background +engine, and any other runtime PostgreSQL async engine created through the +shared engine helper. When the setting is omitted or empty, PostgreSQL runtime +behavior MUST remain unchanged. + +#### Scenario: Runtime asyncpg connections search the configured schema first + +- **GIVEN** `database_url` uses `postgresql+asyncpg://` +- **AND** `database_postgres_schema = "codex_lb_prod"` +- **WHEN** the application opens a new runtime PostgreSQL connection +- **THEN** the connection uses `search_path = "codex_lb_prod",public` +- **AND** unqualified table reads and writes resolve to `codex_lb_prod` before + `public` + +#### Scenario: Omitted schema preserves existing runtime behavior + +- **GIVEN** `database_url` uses `postgresql+asyncpg://` +- **AND** `database_postgres_schema` is omitted or empty +- **WHEN** the application opens a new runtime PostgreSQL connection +- **THEN** no search-path override is configured + +### Requirement: Durable bridge table guards honor the active PostgreSQL schemas + +When durable-bridge readiness or cleanup code checks whether the bridge tables +exist on PostgreSQL, it MUST resolve table presence from the connection's +currently active non-temporary schemas instead of assuming `public`. This keeps +shared-database installs compatible with a schema-specific search path while +preserving existing SQLite behavior. + +#### Scenario: Non-public schema tables satisfy the durable-bridge guard + +- **GIVEN** the database backend is PostgreSQL +- **AND** the active search path includes `codex_lb_prod,public` +- **AND** the required durable-bridge tables exist in `codex_lb_prod` +- **WHEN** the durable-bridge table guard runs +- **THEN** it reports that the tables are present +- **AND** it does not require duplicate copies in `public` diff --git a/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md b/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md new file mode 100644 index 0000000000..0b45ab941c --- /dev/null +++ b/openspec/changes/support-postgres-schema-overrides/specs/database-migrations/spec.md @@ -0,0 +1,87 @@ +## ADDED Requirements + +### Requirement: Migration paths honor an optional PostgreSQL application schema + +When `database_url` resolves to PostgreSQL and `database_postgres_schema` is a +non-empty string, every sync migration path MUST operate with a PostgreSQL +search path containing only ``. This requirement applies to the +Alembic environment, startup migration upgrade path, startup drift check, +`wait-for-head`, and `wait-for-connection` / schema-inspection helpers that use +the shared sync connection factories. When the setting is omitted or empty, +existing migration behavior MUST remain unchanged. + +Configured PostgreSQL schema names MUST NOT exceed PostgreSQL's 63-byte UTF-8 +identifier limit. Alembic offline SQL generation MUST emit a safely quoted, +schema-only search path before migration operations. + +#### Scenario: Startup migrations run inside the configured schema + +- **GIVEN** `database_url` resolves to PostgreSQL +- **AND** `database_postgres_schema = "codex_lb_prod"` +- **WHEN** startup migration or drift-check code opens a sync PostgreSQL + connection +- **THEN** the connection uses a migration-only `search_path = "codex_lb_prod"` +- **AND** Alembic upgrades and ORM drift checks resolve unqualified schema + objects inside `codex_lb_prod` + +#### Scenario: Shared-schema installs do not require public duplicates + +- **GIVEN** PostgreSQL objects exist only in `codex_lb_prod` +- **AND** `database_postgres_schema = "codex_lb_prod"` +- **WHEN** `wait-for-head` or another schema-state check runs after migrations +- **THEN** the check succeeds against `codex_lb_prod` +- **AND** it does not report missing tables solely because `public` is empty + +#### Scenario: Offline migration SQL targets only the configured schema + +- **GIVEN** `database_postgres_schema = "codex_lb_prod"` +- **WHEN** Alembic generates an offline upgrade script +- **THEN** the script sets `search_path` to the safely quoted configured schema +- **AND** migration operations do not fall back to `public` + +#### Scenario: Overlong PostgreSQL schema names fail during configuration + +- **GIVEN** `database_postgres_schema` exceeds 63 bytes when UTF-8 encoded +- **WHEN** application settings are loaded +- **THEN** validation fails before PostgreSQL can truncate the identifier + +### Requirement: Migration bootstrap creates the configured PostgreSQL schema before reading schema state + +When `database_url` resolves to PostgreSQL and `database_postgres_schema` is a +non-empty string other than `public`, migration and schema-inspection paths +MUST ensure that schema exists before they read Alembic state or run upgrades. +This requirement applies to startup migration entrypoints, Alembic online +migrations, drift checks, and migration wait helpers that use the shared sync +connection factories. If the database user cannot create schemas, the failure +MUST surface directly instead of silently falling back to `public`. + +#### Scenario: First shared-schema install does not reuse public Alembic state + +- **GIVEN** `database_url` resolves to PostgreSQL +- **AND** `database_postgres_schema = "codex_lb_prod"` +- **AND** `codex_lb_prod` does not exist yet +- **WHEN** startup migration or Alembic online migration begins +- **THEN** codex-lb creates `codex_lb_prod` before setting `search_path` +- **AND** subsequent Alembic state reads and upgrades resolve inside + `codex_lb_prod` + +#### Scenario: Public migration state cannot mask an uninitialized application schema + +- **GIVEN** `public.alembic_version` is already at the current head +- **AND** `database_postgres_schema = "codex_lb_prod"` +- **AND** `codex_lb_prod` has no Alembic version table +- **WHEN** codex-lb inspects or upgrades the configured schema +- **THEN** it treats `codex_lb_prod` as uninitialized +- **AND** it creates and migrates tables in `codex_lb_prod` instead of reusing + the migration state from `public` +- **AND** PostgreSQL catalog probes use the enum type visible through the active + migration search path + +#### Scenario: Existing default search paths retain visible public enum probes + +- **GIVEN** no schema override is configured +- **AND** a user schema precedes `public` on the default search path +- **AND** the visible codex-lb enum type exists in `public` +- **WHEN** a historical migration probes that enum's labels +- **THEN** it resolves the visible `public` type rather than assuming the first + schema owns the type diff --git a/openspec/changes/support-postgres-schema-overrides/specs/deployment-installation/spec.md b/openspec/changes/support-postgres-schema-overrides/specs/deployment-installation/spec.md new file mode 100644 index 0000000000..4e6292a507 --- /dev/null +++ b/openspec/changes/support-postgres-schema-overrides/specs/deployment-installation/spec.md @@ -0,0 +1,24 @@ +## ADDED Requirements + +### Requirement: Helm external PostgreSQL installs support an optional schema override + +The Helm chart MUST expose one optional `config.databasePostgresSchema` value +for PostgreSQL installs that reuse a shared database. When set, the rendered +runtime workload, migration job, and startup database guard init containers +MUST all receive `CODEX_LB_DATABASE_POSTGRES_SCHEMA` with the same value. When +unset, the chart MUST preserve the current no-override behavior. + +#### Scenario: Shared PostgreSQL schema is wired consistently + +- **WHEN** the chart renders with `postgresql.enabled=false` +- **AND** `config.databasePostgresSchema=codex_lb_prod` +- **THEN** the main workload receives + `CODEX_LB_DATABASE_POSTGRES_SCHEMA=codex_lb_prod` +- **AND** the migration job receives the same environment variable +- **AND** startup database init containers receive the same environment variable + +#### Scenario: Default installs remain unchanged + +- **WHEN** the chart renders without `config.databasePostgresSchema` +- **THEN** `CODEX_LB_DATABASE_POSTGRES_SCHEMA` is not emitted +- **AND** existing dedicated-database installs remain unchanged diff --git a/openspec/changes/support-postgres-schema-overrides/tasks.md b/openspec/changes/support-postgres-schema-overrides/tasks.md new file mode 100644 index 0000000000..409e618d75 --- /dev/null +++ b/openspec/changes/support-postgres-schema-overrides/tasks.md @@ -0,0 +1,35 @@ +# Tasks: support-postgres-schema-overrides + +## 1. Runtime and migration wiring + +- [x] 1.1 Add one normalized `database_postgres_schema` setting for shared + PostgreSQL database installs +- [x] 1.2 Configure asyncpg runtime engines to apply `,public` when the + setting is present +- [x] 1.3 Apply a schema-only PostgreSQL search path to sync migration, Alembic, + and startup schema-check connections +- [x] 1.3.1 Create the configured PostgreSQL schema before migration/bootstrap + state reads so first installs do not fall back to `public` +- [x] 1.3.2 Scope migration state and Alembic's version table to the configured + schema without the runtime `public` fallback +- [x] 1.3.3 Commit PostgreSQL schema setup before Alembic owns transaction + boundaries and scope historical enum catalog probes to the visible type +- [x] 1.3.4 Reject PostgreSQL schema names beyond the 63-byte identifier limit + and emit the schema-only search path in offline Alembic SQL +- [x] 1.4 Make durable-bridge table guards resolve against the active + PostgreSQL schemas instead of hard-coding `public` + +## 2. Install contract and docs + +- [x] 2.1 Expose the schema setting through Helm values, templates, and example + overlays +- [x] 2.2 Document shared PostgreSQL installs in the Kubernetes guide and + `.env.example` +- [x] 2.3 Regenerate the checked-in settings reference page + +## 3. Verification + +- [x] 3.1 Add focused unit coverage for runtime engine kwargs, migration search + path helpers, Helm rendering, and durable-bridge schema detection +- [x] 3.2 Run focused unit tests for the new schema wiring +- [x] 3.3 Run `openspec validate --specs` diff --git a/tests/integration/test_migrations.py b/tests/integration/test_migrations.py index b20c38dbb9..f39609e68f 100644 --- a/tests/integration/test_migrations.py +++ b/tests/integration/test_migrations.py @@ -1,10 +1,12 @@ from __future__ import annotations +import importlib from collections.abc import Callable +from uuid import uuid4 import pytest from anyio import to_thread -from sqlalchemy import text +from sqlalchemy import create_engine, inspect, text from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine from app.core.auth import DEFAULT_PLAN @@ -34,6 +36,7 @@ run_startup_migrations, run_upgrade, ) +from app.db.migration_url import to_sync_database_url from app.db.models import Account, AccountStatus from app.db.session import SessionLocal from app.modules.accounts.repository import AccountsRepository @@ -55,6 +58,82 @@ def _is_postgresql_database_url(url: str) -> bool: return url.startswith("postgresql+") +@pytest.mark.skipif(not _is_postgresql_database_url(_DATABASE_URL), reason="PostgreSQL-only schema isolation") +def test_configured_schema_does_not_reuse_public_alembic_state(monkeypatch: pytest.MonkeyPatch) -> None: + schema = f"codex_lb_test_{uuid4().hex}" + sync_url = to_sync_database_url(_DATABASE_URL) + + monkeypatch.delenv("CODEX_LB_DATABASE_POSTGRES_SCHEMA", raising=False) + get_settings.cache_clear() + public_result = run_upgrade(_DATABASE_URL, "head", bootstrap_legacy=False) + + monkeypatch.setenv("CODEX_LB_DATABASE_POSTGRES_SCHEMA", schema) + get_settings.cache_clear() + try: + state_before = inspect_migration_state(_DATABASE_URL) + assert state_before.current_revision is None + assert state_before.has_alembic_version_table is False + assert state_before.needs_upgrade is True + + schema_result = run_upgrade(_DATABASE_URL, "head", bootstrap_legacy=False) + assert schema_result.current_revision == public_result.current_revision + + engine = create_engine(sync_url, future=True) + try: + with engine.connect() as connection: + inspector = inspect(connection) + assert inspector.has_table("alembic_version", schema=schema) + assert inspector.has_table("accounts", schema=schema) + enum_values = connection.execute( + text( + "SELECT e.enumlabel FROM pg_enum e " + "JOIN pg_type t ON e.enumtypid = t.oid " + "JOIN pg_namespace n ON t.typnamespace = n.oid " + "WHERE n.nspname = :schema AND t.typname = 'account_status'" + ), + {"schema": schema}, + ).scalars() + assert "reauth_required" in set(enum_values) + finally: + engine.dispose() + finally: + engine = create_engine(sync_url, future=True) + try: + with engine.begin() as connection: + connection.execute(text(f'DROP SCHEMA IF EXISTS "{schema}" CASCADE')) + finally: + engine.dispose() + monkeypatch.delenv("CODEX_LB_DATABASE_POSTGRES_SCHEMA", raising=False) + get_settings.cache_clear() + + +@pytest.mark.skipif(not _is_postgresql_database_url(_DATABASE_URL), reason="PostgreSQL-only enum visibility") +def test_enum_probe_follows_visible_public_type_without_schema_override(monkeypatch: pytest.MonkeyPatch) -> None: + schema = f"codex_lb_path_{uuid4().hex}" + sync_url = to_sync_database_url(_DATABASE_URL) + monkeypatch.delenv("CODEX_LB_DATABASE_POSTGRES_SCHEMA", raising=False) + get_settings.cache_clear() + run_upgrade(_DATABASE_URL, "head", bootstrap_legacy=False) + + engine = create_engine(sync_url, future=True) + try: + with engine.begin() as connection: + connection.execute(text(f'CREATE SCHEMA "{schema}"')) + connection.execute( + text("SELECT set_config('search_path', :search_path, false)"), + {"search_path": f'"{schema}",public'}, + ) + migration = importlib.import_module( + "app.db.alembic.versions.20260310_000000_fix_postgresql_enum_value_casing" + ) + + assert migration._enum_value_exists(connection, "account_status", "active") is True + finally: + with engine.begin() as connection: + connection.execute(text(f'DROP SCHEMA IF EXISTS "{schema}" CASCADE')) + engine.dispose() + + def _make_account(account_id: str, email: str, plan_type: str) -> Account: encryptor = TokenEncryptor() return Account( diff --git a/tests/unit/test_db_migrate.py b/tests/unit/test_db_migrate.py index 92ab87f8ed..9b1589eb93 100644 --- a/tests/unit/test_db_migrate.py +++ b/tests/unit/test_db_migrate.py @@ -5,6 +5,7 @@ import sqlite3 import sys from datetime import datetime, timedelta, timezone +from io import StringIO from pathlib import Path from types import SimpleNamespace from typing import cast @@ -15,7 +16,9 @@ from alembic.util.exc import CommandError from sqlalchemy import create_engine, inspect, text from sqlalchemy import exc as sa_exc +from sqlalchemy.dialects import postgresql from sqlalchemy.engine import Connection +from sqlalchemy.schema import CreateSchema import app.db.migrate as migrate_module from app.db.alembic.revision_ids import OLD_TO_NEW_REVISION_MAP @@ -34,7 +37,14 @@ wait_for_connection, wait_for_head, ) -from app.db.migration_url import to_sync_database_url +from app.db.migration_url import ( + apply_postgres_migration_search_path, + apply_postgres_search_path, + ensure_postgres_schema_exists, + postgres_migration_search_path, + postgres_search_path, + to_sync_database_url, +) from app.db.models import Base from app.modules.usage.additional_quota_keys import clear_additional_quota_registry_cache @@ -213,6 +223,184 @@ def test_wait_for_connection_times_out_when_database_stays_unreachable(monkeypat wait_for_connection("sqlite+aiosqlite:///tmp/test.db", timeout_seconds=2.0, interval_seconds=1.0) +def test_postgres_search_path_helper_normalizes_schema() -> None: + assert postgres_search_path(None) is None + assert postgres_search_path("") is None + assert postgres_search_path(" ") is None + assert postgres_search_path("public") == '"public"' + assert postgres_search_path("codex_lb_prod") == '"codex_lb_prod",public' + assert postgres_search_path('tenant"blue') == '"tenant""blue",public' + assert postgres_migration_search_path("codex_lb_prod") == '"codex_lb_prod"' + + +def test_postgres_offline_migration_sets_quoted_schema_only_search_path( + monkeypatch: pytest.MonkeyPatch, +) -> None: + output = StringIO() + config = _build_alembic_config("postgresql+asyncpg://user:pass@db.example.com:5432/codex_lb") + config.output_buffer = output + monkeypatch.setenv("CODEX_LB_DATABASE_POSTGRES_SCHEMA", 'tenant"blue') + from app.core.config.settings import get_settings + + get_settings.cache_clear() + try: + # An empty head:head range still executes env.py without asking older + # data-aware revisions to reflect from Alembic's offline mock bind. + command.upgrade(config, "head:head", sql=True) + finally: + get_settings.cache_clear() + + generated_sql = output.getvalue() + assert 'SET search_path TO "tenant""blue";' in generated_sql + assert 'SET search_path TO "tenant""blue",public;' not in generated_sql + + +def test_apply_postgres_search_path_is_noop_for_non_postgres() -> None: + class _Connection: + dialect = SimpleNamespace(name="sqlite") + + def execute(self, *_args, **_kwargs) -> None: + raise AssertionError("execute should not be called for non-PostgreSQL backends") + + apply_postgres_search_path(cast(Connection, _Connection()), "codex_lb_prod") + + +def test_apply_postgres_search_path_sets_transaction_scope_path_for_postgres() -> None: + calls: list[tuple[object, object]] = [] + + class _Connection: + dialect = SimpleNamespace(name="postgresql") + + def execute(self, statement: object, params: object) -> None: + calls.append((statement, params)) + + connection = cast(Connection, _Connection()) + + apply_postgres_search_path(connection, "codex_lb_prod") + + assert len(calls) == 1 + _statement, params = calls[0] + assert params == {"search_path": '"codex_lb_prod",public'} + + +def test_apply_postgres_migration_search_path_excludes_public_fallback() -> None: + calls: list[tuple[object, object]] = [] + + class _Connection: + dialect = SimpleNamespace(name="postgresql") + + def execute(self, statement: object, params: object) -> None: + calls.append((statement, params)) + + connection = cast(Connection, _Connection()) + apply_postgres_migration_search_path(connection, "codex_lb_prod") + + assert len(calls) == 1 + _statement, params = calls[0] + assert params == {"search_path": '"codex_lb_prod"'} + assert connection.dialect.default_schema_name == "codex_lb_prod" + + +def test_ensure_postgres_schema_exists_is_noop_for_non_postgres() -> None: + class _Connection: + dialect = SimpleNamespace(name="sqlite") + + def execute(self, *_args, **_kwargs) -> None: + raise AssertionError("execute should not be called for non-PostgreSQL backends") + + ensure_postgres_schema_exists(cast(Connection, _Connection()), "codex_lb_prod") + + +def test_ensure_postgres_schema_exists_skips_public_schema() -> None: + class _Connection: + dialect = SimpleNamespace(name="postgresql") + + def execute(self, *_args, **_kwargs) -> None: + raise AssertionError("public schema should not trigger CREATE SCHEMA") + + ensure_postgres_schema_exists(cast(Connection, _Connection()), "public") + + +def test_ensure_postgres_schema_exists_creates_missing_postgres_schema() -> None: + calls: list[CreateSchema] = [] + + class _Connection: + dialect = SimpleNamespace(name="postgresql") + + def execute(self, statement: CreateSchema) -> None: + calls.append(statement) + + ensure_postgres_schema_exists(cast(Connection, _Connection()), "codex_lb_prod") + + assert len(calls) == 1 + statement = calls[0] + compiled = str(statement.compile(dialect=postgresql.dialect())) + assert compiled == "CREATE SCHEMA IF NOT EXISTS codex_lb_prod" + + +def test_sync_connection_creates_schema_before_search_path(monkeypatch: pytest.MonkeyPatch) -> None: + calls: list[tuple[str, str | None]] = [] + + class _ConnectionContext: + def __enter__(self) -> Connection: + return cast(Connection, object()) + + def __exit__(self, exc_type, exc, tb) -> bool: + return False + + class _Engine: + def connect(self) -> _ConnectionContext: + return _ConnectionContext() + + def dispose(self) -> None: + return None + + monkeypatch.setattr(migrate_module, "create_engine", lambda *_args, **_kwargs: _Engine()) + monkeypatch.setattr( + migrate_module, + "get_settings", + lambda: SimpleNamespace(database_postgres_schema="codex_lb_prod"), + ) + monkeypatch.setattr( + migrate_module, + "ensure_postgres_schema_exists", + lambda _connection, schema: calls.append(("ensure", schema)), + ) + monkeypatch.setattr( + migrate_module, + "apply_postgres_migration_search_path", + lambda _connection, schema: calls.append(("search_path", schema)), + ) + + with migrate_module._sync_connection("postgresql+psycopg://codex_lb:codex_lb@127.0.0.1:5432/codex_lb"): + pass + + assert calls == [ + ("ensure", "codex_lb_prod"), + ("search_path", "codex_lb_prod"), + ] + + +def test_read_table_names_scopes_postgres_inspection_to_configured_schema(monkeypatch) -> None: + calls: list[str | None] = [] + + class _Inspector: + def get_table_names(self, *, schema: str | None) -> list[str]: + calls.append(schema) + return ["alembic_version"] + + connection = cast(Connection, SimpleNamespace(dialect=SimpleNamespace(name="postgresql"))) + monkeypatch.setattr(migrate_module, "inspect", lambda _connection: _Inspector()) + monkeypatch.setattr( + migrate_module, + "get_settings", + lambda: SimpleNamespace(database_postgres_schema="codex_lb_prod"), + ) + + assert migrate_module._read_table_names(connection) == {"alembic_version"} + assert calls == ["codex_lb_prod"] + + def test_schema_migration_contract_matches_after_upgrade(tmp_path: Path) -> None: """Prisma-style contract: migrated schema must match ORM metadata and policy.""" db_path = tmp_path / "contract.db" @@ -1983,6 +2171,8 @@ def execute(self, statement: object) -> None: class _MissingAlembicVersionConnection: + dialect = SimpleNamespace(name="postgresql") + def execute(self, statement: object) -> None: raise sa_exc.ProgrammingError( str(statement), @@ -1992,6 +2182,8 @@ def execute(self, statement: object) -> None: class _MissingAlembicVersionSQLiteConnection: + dialect = SimpleNamespace(name="sqlite") + def execute(self, statement: object) -> None: raise sa_exc.OperationalError( str(statement), @@ -2005,12 +2197,14 @@ def __init__(self, *, has_table: bool, version_num_length: int | None = None) -> self._has_table = has_table self._version_num_length = version_num_length - def has_table(self, table_name: str) -> bool: + def has_table(self, table_name: str, *, schema: str | None = None) -> bool: assert table_name == "alembic_version" + assert schema is None return self._has_table - def get_columns(self, table_name: str) -> list[dict[str, object]]: + def get_columns(self, table_name: str, *, schema: str | None = None) -> list[dict[str, object]]: assert table_name == "alembic_version" + assert schema is None return [ { "name": "version_num", @@ -2027,7 +2221,7 @@ def test_ensure_alembic_version_table_capacity_creates_table_when_missing(monkey _ensure_alembic_version_table_capacity_for_connection(cast(Connection, connection), required_length=64) assert connection.executed_sql == [ - "CREATE TABLE IF NOT EXISTS alembic_version ( version_num VARCHAR(64) NOT NULL, PRIMARY KEY (version_num) )" + 'CREATE TABLE IF NOT EXISTS "alembic_version" ( version_num VARCHAR(64) NOT NULL, PRIMARY KEY (version_num) )' ] @@ -2038,7 +2232,7 @@ def test_ensure_alembic_version_table_capacity_alters_short_column(monkeypatch) _ensure_alembic_version_table_capacity_for_connection(cast(Connection, connection), required_length=64) - assert connection.executed_sql == ["ALTER TABLE alembic_version ALTER COLUMN version_num TYPE VARCHAR(64)"] + assert connection.executed_sql == ['ALTER TABLE "alembic_version" ALTER COLUMN version_num TYPE VARCHAR(64)'] def test_read_current_revisions_returns_empty_when_alembic_version_table_is_missing() -> None: diff --git a/tests/unit/test_db_session.py b/tests/unit/test_db_session.py index 7cddca872a..3792f1cdf0 100644 --- a/tests/unit/test_db_session.py +++ b/tests/unit/test_db_session.py @@ -29,6 +29,7 @@ class _FakeSettings: database_sqlite_pre_migrate_backup_enabled: bool = False database_sqlite_pre_migrate_backup_max_files: int = 5 database_sqlite_startup_check_mode: str = "quick" + database_postgres_schema: str | None = None database_migrations_fail_fast: bool = False @@ -328,6 +329,27 @@ def test_postgres_connect_args_pin_session_timezone_to_utc(monkeypatch) -> None: assert connect_args == {"server_settings": {"timezone": "UTC"}} +def test_postgres_connect_args_include_search_path_when_schema_is_configured(monkeypatch) -> None: + monkeypatch.delenv("CODEX_LB_TEST_DATABASE_URL", raising=False) + monkeypatch.setattr( + session_module, + "_settings", + _FakeSettings( + database_url="postgresql+asyncpg://u:p@h/db", + database_postgres_schema="codex_lb_prod", + ), + ) + + connect_args = session_module._postgres_async_connect_args("postgresql+asyncpg://u:p@h/db") + + assert connect_args == { + "server_settings": { + "timezone": "UTC", + "search_path": '"codex_lb_prod",public', + } + } + + def test_postgres_connect_args_pin_utc_and_keep_test_db_url_tuning(monkeypatch) -> None: monkeypatch.setenv("CODEX_LB_TEST_DATABASE_URL", "1") diff --git a/tests/unit/test_durable_bridge_sessions.py b/tests/unit/test_durable_bridge_sessions.py index dd154d75c1..69201a089a 100644 --- a/tests/unit/test_durable_bridge_sessions.py +++ b/tests/unit/test_durable_bridge_sessions.py @@ -32,6 +32,7 @@ from app.modules.proxy.durable_bridge_repository import ( DurableBridgeAliasRegistration, DurableBridgeRepository, + missing_durable_bridge_tables, ) pytestmark = pytest.mark.unit @@ -139,6 +140,25 @@ async def test_durable_bridge_lookup_prefers_turn_state_then_previous_response_t assert by_session.canonical_key == "sid-123" +@pytest.mark.asyncio +async def test_missing_durable_bridge_tables_checks_current_postgres_schemas() -> None: + captured_sql: list[str] = [] + + class _PostgresSession: + def get_bind(self) -> SimpleNamespace: + return SimpleNamespace(dialect=SimpleNamespace(name="postgresql")) + + async def execute(self, statement: object) -> SimpleNamespace: + captured_sql.append(str(statement)) + return SimpleNamespace(fetchall=lambda: [("http_bridge_sessions",)]) + + missing = await missing_durable_bridge_tables(cast(AsyncSession, _PostgresSession())) + + assert "http_bridge_session_aliases" in missing + assert any("current_schemas(false)" in sql for sql in captured_sql) + assert all("table_schema = 'public'" not in sql for sql in captured_sql) + + @pytest.mark.asyncio async def test_reversible_recovery_turn_state_registration_restores_previous_owner( coordinator: DurableBridgeSessionCoordinator, diff --git a/tests/unit/test_helm_external_secrets.py b/tests/unit/test_helm_external_secrets.py index 379e691e32..30cba23d8d 100644 --- a/tests/unit/test_helm_external_secrets.py +++ b/tests/unit/test_helm_external_secrets.py @@ -1,8 +1,10 @@ from __future__ import annotations +import os import re import shutil import subprocess +import tempfile from pathlib import Path import pytest @@ -13,6 +15,24 @@ _REPO_ROOT = Path(__file__).resolve().parents[2] _CHART_DIR = _REPO_ROOT / "deploy" / "helm" / "codex-lb" _DEPENDENCY_BUILD_COMPLETE = False +_HELM_HOME = Path(tempfile.mkdtemp(prefix="codex-lb-helm-")) +_HELM_ENV = { + "HELM_CONFIG_HOME": str(_HELM_HOME / "config"), + "HELM_CACHE_HOME": str(_HELM_HOME / "cache"), + "HELM_DATA_HOME": str(_HELM_HOME / "data"), + "HELM_REGISTRY_CONFIG": str(_HELM_HOME / "config" / "registry.json"), + "HELM_REPOSITORY_CONFIG": str(_HELM_HOME / "config" / "repositories.yaml"), + "HELM_REPOSITORY_CACHE": str(_HELM_HOME / "repository"), +} + +for path in ( + _HELM_HOME / "config", + _HELM_HOME / "cache", + _HELM_HOME / "data", + _HELM_HOME / "repository", +): + path.mkdir(parents=True, exist_ok=True) +(_HELM_HOME / "config" / "repositories.yaml").touch() def _ensure_chart_dependencies() -> None: @@ -29,6 +49,7 @@ def _ensure_chart_dependencies() -> None: check=True, capture_output=True, text=True, + env={**os.environ, **_HELM_ENV}, ) _DEPENDENCY_BUILD_COMPLETE = True @@ -43,6 +64,7 @@ def _helm_template(*args: str) -> str: check=True, capture_output=True, text=True, + env={**os.environ, **_HELM_ENV}, ) return completed.stdout @@ -58,6 +80,7 @@ def _helm_template_failure(*args: str) -> subprocess.CalledProcessError: check=True, capture_output=True, text=True, + env={**os.environ, **_HELM_ENV}, ) return exc_info.value @@ -927,6 +950,20 @@ def test_external_database_url_is_rendered_into_chart_managed_secret_when_postgr assert 'database-url: "postgresql+asyncpg://user:pass@db.example.com:5432/codexlb"' in rendered +def test_database_postgres_schema_is_rendered_for_runtime_and_migration_paths() -> None: + rendered = _helm_template( + "--set", + "postgresql.enabled=false", + "--set", + "externalDatabase.url=postgresql+asyncpg://user:pass@db.example.com:5432/codexlb", + "--set", + "config.databasePostgresSchema=codex_lb_prod", + ) + + assert 'CODEX_LB_DATABASE_POSTGRES_SCHEMA: "codex_lb_prod"' in rendered + assert rendered.count("name: CODEX_LB_DATABASE_POSTGRES_SCHEMA") == 2 + + def test_network_policy_does_not_allow_http_ingress_from_all_namespaces_by_default() -> None: rendered = _helm_template( "-f", diff --git a/tests/unit/test_settings_postgres.py b/tests/unit/test_settings_postgres.py new file mode 100644 index 0000000000..507ac297b8 --- /dev/null +++ b/tests/unit/test_settings_postgres.py @@ -0,0 +1,20 @@ +from __future__ import annotations + +import pytest +from pydantic import ValidationError + +from app.core.config.settings import Settings + + +def test_database_postgres_schema_accepts_postgres_identifier_byte_limit() -> None: + schema = "a" * 63 + + settings = Settings(_env_file=None, database_postgres_schema=schema) + + assert settings.database_postgres_schema == schema + + +@pytest.mark.parametrize("schema", ["a" * 64, "数" * 22]) +def test_database_postgres_schema_rejects_more_than_63_utf8_bytes(schema: str) -> None: + with pytest.raises(ValidationError, match="at most 63 UTF-8 bytes"): + Settings(_env_file=None, database_postgres_schema=schema) diff --git a/tests/unit/test_settings_reference.py b/tests/unit/test_settings_reference.py index 428b5057b1..d4c129e892 100644 --- a/tests/unit/test_settings_reference.py +++ b/tests/unit/test_settings_reference.py @@ -54,7 +54,11 @@ def _isolated_settings(**overrides: Any) -> Settings: # 117 -> 118: http_responses_session_bridge_anchor_poison_failure_threshold # (bridge restart anchor poisoning). Not hardcoded because operators need a # bounded deployment-specific poison threshold while recovery telemetry matures. -MAX_SETTINGS_FIELDS = 118 +# 118 -> 119: database_postgres_schema. Shared-database deployments need one +# explicit schema knob so runtime engines, migration jobs, and schema gates use +# the same non-public search_path; there is no safe fixed default when a single +# PostgreSQL database is reused by multiple applications. +MAX_SETTINGS_FIELDS = 119 def test_generated_settings_reference_matches_code() -> None: