Review: pal-e-platform#306 — Add admin_app_db_password to Salt pillar

review-306-2026-04-25 Doc

active review ready

Verdict: READY (with three low-severity body patches recommended)

Ticket is dispatch-ready in substance — file target is real, ACs are testable, lineage to PR #304 is correct, and the constraint set matches the existing pillar/Makefile pipeline. Three low-severity body inaccuracies are flagged for Ava to patch when promoting backlog → todo: Repo field, sibling-pattern reference, and missing arch:salt trace.

Scope

Ticket #306 (Type: Chore) is the operator-step companion to PR #304. It adds admin_app_db_password to the GPG-encrypted Salt pillar at salt/pillar/secrets/platform.sls so make tofu-apply can render secrets.auto.tfvars with the new variable PR #304 declares in terraform/variables.tf and terraform/modules/database/variables.tf. Without this entry, make tofu-validate-secrets exits non-zero (the Makefile in PR #304 already lists admin_app_db_password in TF_SECRET_VARS), blocking apply.

Template Completeness

  • [x] Type — Chore
  • [x] Lineage — Track D / PR #304
  • [~] Repo — present but inaccurate ("salt repo (or wherever ... — verify)"); see Finding 1
  • [x] User Story — clear, names operator + outcome
  • [x] Context — explains why the apply fails without the entry
  • [x] File Targets — salt/pillar/secrets/platform.sls
  • [x] Acceptance Criteria — three concrete, testable items
  • [x] Test Expectations — apply runs end-to-end without prompt
  • [x] Constraints — GPG required, no plaintext, ordering note
  • [x] Checklist — five discrete steps
  • [x] Related — blocks PR #304, triggered by Track D, memory ref

Traceability

  • [x] story:admin-row-crud — present in user story prefix; matches PR #304 label
  • [ ] arch:salt — not named in body; see Finding 3
  • [x] Forgejo issue — open at forgejo_admin/pal-e-platform#306

File Targets

  • [x] salt/pillar/secrets/platform.sls — verified exists at /home/ldraney/pal-e-platform/salt/pillar/secrets/platform.sls. Uses #!yaml|gpg renderer. Schema is secrets.platform.<key>: | followed by a multi-line PGP MESSAGE block. Closest sibling pattern: paledocs_db_password (lines 281–298).

Targets are specific enough — single file, single key insertion, well-established pattern in the same file.

Repo Placement

Work belongs in forgejo_admin/pal-e-platform (this repo). The ticket's "Repo" field hedges with "salt repo (or wherever ... — verify)" — verified during review: there is no separate salt repo; the pillar lives in pal-e-platform. Single-repo scope, no cross-repo coordination.

Dependencies

  • [~] PR #304 — currently open, not yet merged. Per the ticket's own constraint, the pillar entry must land before make tofu-apply runs after #304 merges. Order can be: pillar PR opened/merged in parallel with #304, but apply must wait for both. Not blocking dispatch — dev agent can land #306 independently.
  • [x] GPG keyring — assumed present on the operator host (already used to seal 20+ existing pillar entries).
  • [x] Salt pillar pipeline — operational (verified by reading tofu-secrets Makefile target).

Acceptance Criteria

  • AC1: "admin_app_db_password exists in Salt pillar, GPG-encrypted, with strong generated value." — Testable: grep -A1 admin_app_db_password salt/pillar/secrets/platform.sls shows PGP MESSAGE block.
  • AC2: "make tofu-apply runs without prompting for the variable." — Testable: operator command, exit code 0 from tofu-validate-secrets.
  • AC3: "Password generation procedure documented (length, charset, source)." — Testable but underspecified; PR #304's example file already names openssl rand -hex 32, so the dev agent can mirror that. Suggest including the exact generation command inline so the AC is unambiguous.

Blast Radius

  • Files touched: 1 (salt/pillar/secrets/platform.sls).
  • Services impacted: none directly until make tofu-apply runs; then PR #304's Job + Secret in basketball-api namespace get provisioned.
  • Failure modes if buggy: malformed YAML breaks salt-call pillar.get (highstate fails for all keys, not just this one); wrong GPG recipient fails to decrypt at render time. Both fail loud at apply time, not silently.
  • Rollback: revert the pillar commit. Trivial.

Decomposition Assessment

  • Discrete changes: 1 (single-key pillar edit). Well under three-thing limit.
  • Estimated agent time: under 5 minutes. Generate password → encrypt → insert → commit → verify tofu-validate-secrets passes.
  • No subtasks to parallelize.

Findings

  1. [low] Repo field is wrong / ambiguous. Body says "salt repo (or wherever salt/pillar/secrets/platform.sls lives — verify)". Verified: pillar lives in this repo, forgejo_admin/pal-e-platform. There is no separate "salt repo." Patch the Repo line so the dev agent does not go hunting.
  2. [low] Sibling-pattern reference is incorrect. Constraints say "matches how postgres_admin_password and other DB credentials live." There is no postgres_admin_password key in the pillar. The actual sibling DB-password is paledocs_db_password. Replace the reference so the dev agent has a concrete model to copy.
  3. [low] Missing arch: trace. User story prefix story:admin-row-crud is present, but no arch trace is named. Per the traceability triangle, this work is arch:salt (pillar/secrets pipeline) — separate from PR #304's arch:postgres. Add arch:salt as the board-item label and trace before promoting.
  4. [info] Multi-pillar verification — answer is "no others." Ticket asks the agent to verify whether sibling pillar files need the key. Verified at review: salt/pillar/secrets/platform.sls is the single source consumed by make tofu-secrets (which calls pillar.get secrets:platform). No environment overlays exist. Cheap to leave the verification step in.
  5. [info] GPG encryption command is implicit. Ticket mandates GPG encryption but doesn't pin the exact command. Existing pillar entries imply gpg --armor --encrypt --recipient <keyid> piped from openssl rand -hex 32. Dev agent can derive from the existing file shape; not blocking.
  6. [info] Coordination ordering with PR #304. Constraint says "pillar entry must exist before #304 merges and apply runs." Practically: pillar PR can land in parallel; the gating event is the operator running make tofu-apply, not the merges themselves. Worth confirming with Lucas before dev dispatch.

Recommendation

Promote backlog → todo with three low-severity body patches:

  1. Repo: forgejo_admin/pal-e-platform
  2. Constraints: replace postgres_admin_password reference with paledocs_db_password (lines 281–298 of the pillar are the model)
  3. Add arch:salt label and trace alongside story:admin-row-crud

Optional polish: pin the generation command (openssl rand -hex 32) directly in AC3 so it is unambiguously testable.

Once Ava patches those, the ticket is dispatch-ready — single-file change, well-modeled by an existing sibling, fast for a dev agent, low blast radius, trivial rollback.