pal-e-docs

Review v2: k8s Job provision admin_app Postgres user + grants

review-1097-2026-04-25-v2 Doc

review ready
Project

Verdict: READY (with one [SCOPE] carryover)

Re-review of board item #1097 — Forgejo forgejo_admin/pal-e-platform#302. Issue body fully rewritten since review-1097-2026-04-25: 3-option fork removed, Path A (idempotent k8s Job in pal-e-deployments) selected. Type: Task.

Changes since v1

  • Title rewritten: "k8s Job: provision admin_app Postgres user + grants on basketball db" (was generic "create admin_app user")
  • 3-option fork removed; Path A explicitly chosen with rationale block citing v1 review findings
  • SQL spelled out in body (CREATE ROLE DO-block, GRANT, ALTER DEFAULT PRIVILEGES)
  • Three concrete deliverables enumerated (password gen, Job manifest + kustomization, DATABASE_URL secret mirror)
  • 10 AC including idempotency + blocked-write test + forward-grant test
  • Constraints section explicitly forbids cyrilgdn provider, forbids modifying basketball-api Deployment, forbids CREATE/ALTER/DROP grants
  • Label drift: arch:postgres-grants → arch:postgres

Template Completeness

  • [x] Type — Task
  • [x] Lineage (with scope-revision lineage block)
  • [x] Repo
  • [x] Scope (3 deliverables, SQL inline)
  • [x] Acceptance Criteria — 10 items
  • [x] Test Expectations
  • [x] Constraints
  • [x] Checklist
  • [x] Related (links v1 review for provenance)

Traceability

  • [x] story:admin-row-crud label — present
  • [x] story note verified — story-westside-admin-admin-row-crud exists in project-westside-admin user-stories table
  • [x] arch:postgres label — present (changed from arch:postgres-grants)
  • [ ] arch note MISSING — neither arch-postgres nor arch-deployment-westside-admin exist as backing notes. [SCOPE] Carryover from v1: create arch-deployment-westside-admin and a stub arch-postgres (or fold grants into the parent). Should not block ticket execution.
  • [x] Forgejo issue — open, valid (#302)
  • [x] type:infra label — present

SQL Idempotency Audit

  • CREATE ROLE — wrapped in DO $$ ... IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname='admin_app') ... CREATE ROLE ... END $$. Idempotent for creation. Caveat: if the role already exists, the Job will NOT update the password — there's no ALTER ROLE branch. For initial provisioning + re-apply with same password, this is correct (no-op). For password rotation, the Job won't propagate a new password unless ALTER ROLE is added. Acceptable for v1 since rotation is deferred to the companion SOP.
  • GRANT USAGE / GRANT SELECT,INSERT,UPDATE,DELETE / GRANT USAGE ON SEQUENCES — Postgres GRANT is idempotent by design (re-grant is a no-op). Correct.
  • ALTER DEFAULT PRIVILEGES — idempotent. Correct pattern for "future tables get same grants automatically." This is the right tool for the AC "all future tables" requirement.
  • Conclusion: idempotency holds for the scope as written. Re-running the Job is safe.

Grants Minimality

  • USAGE on schema public — required for any table access. ✓
  • SELECT, INSERT, UPDATE, DELETE on tables — exactly the row-CRUD scope. ✓
  • USAGE on sequences — needed for nextval (SERIAL/identity inserts). ✓
  • NO CREATE, ALTER, DROP, TRUNCATE, REFERENCES, TRIGGER. ✓
  • NO superuser, NO CREATEDB, NO CREATEROLE. ✓ (default for non-superuser CREATE ROLE)
  • Minor: if the app ever needs currval() or lastval(), sequences also need SELECT. USAGE alone covers nextval() which is what Drizzle uses for inserts. Not a blocker — flag for later if reads of sequence state are added.
  • Blocked-write AC (CREATE TABLE → permission denied) is the correct fail-test signal that minimality holds.

DATABASE_URL Handoff to #1095/#133

  • Path: pal-e-deployments/overlays/westside-admin/prod/westside-admin-secrets.enc.yaml — explicit.
  • Key: DATABASE_URL — explicit.
  • Connection string: postgresql://admin_app:<password>@postgres.basketball-api.svc.cluster.local:5432/basketball — verified against postgres.yaml: Service name is postgres in namespace basketball-api, port 5432, DB name basketball. Correct.
  • Cross-namespace DNS works in k3s — overlay-side NetworkPolicy must allow westside-admin → basketball-api:5432 (separate concern, likely covered by #133 or a follow-up).
  • Companion ticket #1095 (pal-e-deployments#133) is the consumer; contract is unambiguous.
  • Verdict: handoff is well-defined.

File Targets / Path Verification

  • [x] pal-e-deployments/overlays/basketball-api/prod/ — exists, contains kustomization.yaml + postgres.yaml. New admin-app-user-job.yaml goes here.
  • [x] pal-e-deployments/overlays/basketball-api/prod/postgres.yaml — verified: plain postgres:16-alpine Deployment, Service named postgres, namespace basketball-api, DB basketball, user basketball, password from Secret basketball-api-secrets key postgres-password.
  • [ ] pal-e-deployments/overlays/westside-admin/prod/westside-admin-secrets.enc.yaml — directory does NOT exist yet. Created by companion #1095/#133. Acceptable: ordering allows this Job (#1097) to land first; secret mirror coordinates with the consumer overlay.
  • Minor [BODY] nit (non-blocking): body says "Mounts the basketball-api postgres superuser credentials (existing Secret) for connecting as superuser." The role basketball is the DB owner (created via POSTGRES_USER env), not formally SUPERUSER. As DB owner it CAN create roles in its database and grant on objects it owns, so the SQL works — but the wording "superuser" is technically inaccurate. Consider "owner credentials" instead.

Repo Placement

OK. Tracking issue lives on pal-e-platform (correct — bootstrap repo). Manifest deliverable lives in pal-e-deployments (correct — overlay home). Secret deliverable also in pal-e-deployments via #133. Three repos involved, two PRs expected (one per repo as needed); body acknowledges this.

Dependencies

Foundational. Blocks: forgejo_admin/westside-admin#1 (Drizzle integration), forgejo_admin/pal-e-deployments#133 (overlay secret consumer = board item #1095). No upstream blockers. Can start immediately.

Acceptance Criteria

10 AC, all testable: role exists check, grants check, negative-grant check, password storage check, secret mirror check, idempotency check, three psql connection tests (read/write/blocked), SOP note creation. Each is verifiable by an agent with explicit psql commands. The blocked-write test (CREATE TABLE foo → permission denied) is the strongest minimality signal. Test Expectations adds a forward-grant test (create table as superuser, verify admin_app immediately has CRUD) that proves ALTER DEFAULT PRIVILEGES works. AC are excellent.

Blast Radius

Low. Plain-Postgres Deployment pattern is also used by westsidekingsandqueens and westside-streamlit — same admin_app pattern would extend cleanly later if needed, but only basketball is in scope here. Job touches only the basketball DB. Do NOT reuse basketball-api-secrets for the new credential — keep it in a fresh Secret to keep blast radius isolated. Body should call this out explicitly but it's implied by "Mirror the password Secret" pointing to a separate file.

Decomposition Assessment

3 deliverables, ~3-4 files (Job manifest + kustomization edit + secret mirror + SOP note), 10 AC but all run in seconds via psql. Estimated agent work: ~10 min. Fits a single agent pass at the upper edge of the 5-minute rule. No decomposition needed, but agent should be briefed tightly to avoid scope drift.

Recommendation

READY to advance backlog → todo. v1 [BODY] recommendations all addressed (concrete approach chosen, secret handoff explicit, cyrilgdn provider option removed). Two non-blocking carryovers:
  • [SCOPE] Carryover from v1: create arch-deployment-westside-admin and stub arch-postgres backing notes. Foundational refinement, does not block ticket execution but story:arch traceability is incomplete without it. Recommend Ava queue this as a separate ticket on board-westside-admin.
  • [BODY] Optional polish: replace "superuser credentials" with "owner credentials" (the role basketball is DB owner, not formally SUPERUSER). Non-blocking; SQL semantics are correct as written.
  • [BODY] Optional polish: add a one-line note that the Job's Secret holding admin_app password should be a NEW Secret (not basketball-api-secrets) for blast-radius isolation. Implied but worth being explicit.
None of the optional polish items block READY. The arch-note carryover is a separate scoping ticket, not a body fix.