fix(conftest): tuple-based prod DSN identity check #26

Merged
kaykayyali merged 1 commits from fix/conftest-test-db-isolation into main 2026-06-26 15:49:55 +00:00
Owner

Why this follow-up

PR #25 was merged, but a Kay review caught a hole: the prod-safety guard used a host-or-port check. Port 5432 in the compose network is also the in-container test DB port (db-test:5432). A worker running pytest inside the orchestrator container pointing at the in-container test DB would have been wrongly blocked by the old port check.

This replaces the host/port check with a full (host, port, user, dbname) tuple check. The four canonical prod tuples are explicitly listed; any other combination is treated as not-prod.

What changes

  • tests/conftest.py: replace _PROD_HOSTS / _PROD_PORTS with _PROD_DSNS (set of tuples). _prod_safety_guard() now builds the full DSN tuple from DB_CONFIG and checks membership.
  • tests/test_conftest_safety.py: 13 tests (was 8). New tests cover:
    • In-container test DSN (db-test:5432/damascus_test/damascus_test) is treated as not-prod
    • Wrong user (damascuswrong_user) is treated as not-prod
    • Wrong dbname is treated as not-prod
    • _PROD_DSNS contains exactly the four canonical prod tuples
    • _PROD_DSNS does NOT contain any test tuple

Verification

  • pytest tests/test_conftest_safety.py -v → 13 passed
  • pytest tests/contract/ tests/unit/ -v → 65 passed (no regressions)
  • Combined → 78 passed, 0 failed

Deployment order (same as PR #25)

  1. PR #23 (db-test compose) already merged
  2. PR #25 (conftest guard) already merged
  3. This PR (tuple-based identity check)
  4. Force-recreate the orchestrator container: docker compose up -d --force-recreate --no-deps orchestrator orchestrator-scheduler

After #4, any worker pytest run hits db-test (port 5433 host / 5432 container) and prod work_items survives — both via the host-loopback port and the in-container compose network.

## Why this follow-up PR #25 was merged, but a Kay review caught a hole: the prod-safety guard used a host-or-port check. Port 5432 in the compose network is also the in-container test DB port (db-test:5432). A worker running pytest inside the orchestrator container pointing at the in-container test DB would have been wrongly blocked by the old port check. This replaces the host/port check with a full (host, port, user, dbname) tuple check. The four canonical prod tuples are explicitly listed; any other combination is treated as not-prod. ## What changes - `tests/conftest.py`: replace `_PROD_HOSTS` / `_PROD_PORTS` with `_PROD_DSNS` (set of tuples). `_prod_safety_guard()` now builds the full DSN tuple from DB_CONFIG and checks membership. - `tests/test_conftest_safety.py`: 13 tests (was 8). New tests cover: - In-container test DSN (`db-test:5432/damascus_test/damascus_test`) is treated as not-prod - Wrong user (`damascus` → `wrong_user`) is treated as not-prod - Wrong dbname is treated as not-prod - `_PROD_DSNS` contains exactly the four canonical prod tuples - `_PROD_DSNS` does NOT contain any test tuple ## Verification - `pytest tests/test_conftest_safety.py -v` → 13 passed - `pytest tests/contract/ tests/unit/ -v` → 65 passed (no regressions) - Combined → 78 passed, 0 failed ## Deployment order (same as PR #25) 1. PR #23 (db-test compose) ✅ already merged 2. PR #25 (conftest guard) ✅ already merged 3. **This PR** (tuple-based identity check) 4. Force-recreate the orchestrator container: `docker compose up -d --force-recreate --no-deps orchestrator orchestrator-scheduler` After #4, any worker pytest run hits `db-test` (port 5433 host / 5432 container) and prod work_items survives — both via the host-loopback port and the in-container compose network.
kaykayyali added 1 commit 2026-06-26 15:49:49 +00:00
fix(conftest): tuple-based prod DSN identity check
Some checks failed
test / contract-and-unit (pull_request) Failing after 14s
3aded098d7
The previous prod-safety guard used a host-or-port check, which had
a hole: port 5432 in the compose network is also the in-container
test DB port (db-test:5432). A worker running pytest inside the
orchestrator container could legitimately point at db-test:5432,
but the old guard would have skipped the TRUNCATE (treating port
5432 as prod), leaving the test suite with stale state.

Switch the identity check to a full (host, port, user, dbname)
tuple. The four canonical prod tuples are:
  (127.0.0.1,                          5432, 'damascus', 'damascus')
  (localhost,                          5432, 'damascus', 'damascus')
  (db,                                 5432, 'damascus', 'damascus')
  (damascus-orchestrator-db-1,         5432, 'damascus', 'damascus')

Any other combination is treated as not-prod and the test runs.
This includes the in-container test DSN
  (db-test, 5432, 'damascus_test', 'damascus_test')
which the previous port-only check would have wrongly flagged.

Test additions (tests/test_conftest_safety.py, 13 tests total):
- test_db_config_defaults_to_test_db
- test_db_config_explicit_overrides
- test_prod_safety_guard_skips_host_loopback_prod
- test_prod_safety_guard_skips_in_container_via_db_host
- test_prod_safety_guard_skips_localhost
- test_prod_safety_guard_skips_container_name
- test_prod_safety_guard_treats_in_container_test_as_safe  (NEW)
- test_prod_safety_guard_treats_wrong_user_as_safe        (NEW)
- test_prod_safety_guard_treats_wrong_dbname_as_safe      (NEW)
- test_prod_safety_guard_opt_in
- test_prod_dsn_constant_includes_all_four_prod_tuples    (NEW)
- test_prod_dsn_excludes_test_tuples                     (NEW)
- test_module_invariants

Verification:
- pytest tests/test_conftest_safety.py -v  → 13 passed
- pytest tests/contract/ tests/unit/ -v   → 65 passed (no regressions)
- combined                                  → 78 passed, 0 failed

Follow-up to PR #25.
kaykayyali merged commit 4d65e47558 into main 2026-06-26 15:49:55 +00:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kaykayyali/damascus-orchestrator#26