mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 12:31:29 +00:00
shankar0123
def4be9b38
The v2.1.0 release-gate Phase-9 docker compose smoke run against a
fresh Postgres surfaced two real defects in the migration files that
testcontainers schema-per-test never exercised. Both reproduce by
running 'docker compose down -v && docker compose up --build'
against the current master tree.
Bug A — migration 000045_users_deactivated_at.up.sql is malformed.
The 000029 schema defines:
permissions (id TEXT PRIMARY KEY, name TEXT NOT NULL UNIQUE,
namespace TEXT NOT NULL)
role_permissions (..., permission_id TEXT NOT NULL REFERENCES ..., ...)
But 000045 was written as:
INSERT INTO permissions (name) VALUES ... -- missing id + namespace
INSERT INTO role_permissions (role_id, permission, ...) VALUES ...
^^ wrong column name
On a cold-DB run this fails immediately with:
pq: null value in column "id" of relation "permissions"
violates not-null constraint
Fix: provide id + namespace columns, use permission_id (the actual
column name), ON CONFLICT (id) DO NOTHING. The new permission ids
follow the existing 'p-auth-*' prefix convention (p-auth-user-read +
p-auth-user-deactivate) used by 000029.
Bug B — migration 000029_rbac.up.sql is not idempotent post-000043.
000029 originally created actor_roles with:
UNIQUE (actor_id, actor_type, role_id, tenant_id)
Audit 2026-05-10 HIGH-10 closure / migration 000043 drops that
constraint and re-creates it WITH scope columns:
UNIQUE (actor_id, actor_type, role_id, scope_type, scope_id, tenant_id)
The migration runner (internal/repository/postgres/db.go::RunMigrations)
is naive — no tracker table — and re-runs every *.up.sql file on
every server boot. On the second-and-later boots, 000029's seed
INSERT for actor-demo-anon-admin still references the
pre-000043 constraint name in its ON CONFLICT clause:
ON CONFLICT (actor_id, actor_type, role_id, tenant_id) DO NOTHING
Postgres errors out with:
pq: there is no unique or exclusion constraint matching the
ON CONFLICT specification
Fix: pin the conflict target to the row's primary key 'id' column
(always present, never altered). The seed row's deterministic id
'ar-demo-anon-admin' makes ON CONFLICT (id) work under both pre-
and post-000043 schemas.
Why testcontainers schema-per-test missed these:
Each test in internal/repository/postgres/*_test.go spins up a
fresh schema and applies every .up.sql in order ONCE. The full
'000029 -> 000043 -> retry 000029' cascade never happens because
migrations don't re-run within a test. Phase-9 docker compose
smoke is the only test path that exercises the server-restart-
on-error retry, which is exactly the missing coverage.
Verify (sandbox): go test ./internal/repository/postgres/ PASS.
Workstation re-runs 'docker compose down -v && docker compose up'
to confirm both bugs are closed.
46 lines
2.0 KiB
PL/PgSQL
46 lines
2.0 KiB
PL/PgSQL
-- 000045_users_deactivated_at.up.sql
|
|
-- Audit 2026-05-10 MED-11 closure: federated-user admin surface.
|
|
--
|
|
-- Adds the deactivated_at column to users so the admin DELETE-by-id
|
|
-- path can soft-delete a federated identity without destroying the
|
|
-- row (the row is the OIDC binding — destroying it would re-mint a
|
|
-- fresh user on the next IdP login under the same subject, losing
|
|
-- the audit trail). Also seeds two new catalogue permissions:
|
|
--
|
|
-- auth.user.read — list / get a user. Seeded into r-admin,
|
|
-- r-operator, r-auditor.
|
|
-- auth.user.deactivate — set deactivated_at + cascade-revoke
|
|
-- sessions. Seeded into r-admin ONLY.
|
|
--
|
|
-- Idempotent. Single transaction.
|
|
|
|
BEGIN;
|
|
|
|
ALTER TABLE users
|
|
ADD COLUMN IF NOT EXISTS deactivated_at TIMESTAMPTZ;
|
|
|
|
-- v2.1.0 Phase-9 cold-DB-smoke fix: the original commit set just
|
|
-- (name) and used a `permission` column in role_permissions. The
|
|
-- 000029 schema actually defines `permissions (id PK, name UNIQUE,
|
|
-- namespace NOT NULL)` and `role_permissions (..., permission_id, ...)`.
|
|
-- testcontainers schema-per-test never exercised this path; the cold
|
|
-- compose-up smoke caught it on a fresh Postgres.
|
|
INSERT INTO permissions (id, name, namespace) VALUES
|
|
('p-auth-user-read', 'auth.user.read', 'auth.user'),
|
|
('p-auth-user-deactivate', 'auth.user.deactivate', 'auth.user')
|
|
ON CONFLICT (id) DO NOTHING;
|
|
|
|
-- Read is broad (admin / operator / auditor).
|
|
INSERT INTO role_permissions (role_id, permission_id, scope_type, scope_id) VALUES
|
|
('r-admin', 'p-auth-user-read', 'global', NULL),
|
|
('r-operator', 'p-auth-user-read', 'global', NULL),
|
|
('r-auditor', 'p-auth-user-read', 'global', NULL)
|
|
ON CONFLICT (role_id, permission_id, scope_type, scope_id) DO NOTHING;
|
|
|
|
-- Deactivate is admin-only.
|
|
INSERT INTO role_permissions (role_id, permission_id, scope_type, scope_id) VALUES
|
|
('r-admin', 'p-auth-user-deactivate', 'global', NULL)
|
|
ON CONFLICT (role_id, permission_id, scope_type, scope_id) DO NOTHING;
|
|
|
|
COMMIT;
|