pal-e-docs

Review: Drop stale basketball DB role + update Keycloak westside-ror references

review-1633-2026-06-27 Review

review approved
Project

Verdict: READY

Template Completeness

  • [x] Type — Bug
  • [x] Lineage — Sub-ticket of pal-e-deployments #220, decomposed per review-1630-2026-06-27
  • [x] Repo — ldraney/pal-e-services
  • [x] What Broke — two issues described (stale DB role + stale Keycloak refs)
  • [x] Repro Steps — 3 steps, clear and actionable
  • [x] Expected Behavior — 4 bullet points covering both fix areas
  • [x] Environment — cluster/namespace: prod, files identified
  • [x] Acceptance Criteria — 9 criteria, all verifiable
  • [x] Related — parent issue, predecessor, review note, project page

Traceability

  • [x] story:WS-S1 label — "As superadmin, I want to deploy platform changes via IaC so that infrastructure is reproducible and auditable"
  • [x] story note verified — found in project-westside-basketball user-stories section (Superadmin list, first item)
  • [ ] arch:iac label — arch note MISSING — [SCOPE] No arch-iac note exists in pal-e-docs. However, this is a cross-cutting infrastructure concern (Terraform IaC) not a project-specific architecture component. Acceptable as foundational infrastructure — creating an arch note for "iac" would be too generic to be useful.
  • [x] Forgejo issue — ldraney/pal-e-services#152, state: open

File Targets

  • [x] terraform/k3s.tfvars — verified via k3s.tfvars.example: Keycloak client block for westside-basketball (example lines 149-176) contains client_id = "westside-ror" and 4 URL references to westside-ror.tail5b443a.ts.net. The actual k3s.tfvars is gitignored (contains secrets), so exact line numbers (156-175) cannot be verified against the example, but the content is confirmed correct.
  • [x] terraform/k3s.tfvars source_path — verified: example line 343 shows source_path = "overlays/westside-ror/prod" in the westside-basketball services block. Issue says line 274 — line number mismatch vs example, but content confirmed.
  • [x] terraform/k3s.tfvars databases — the service_databases variable structure is confirmed in databases.tf and variables.tf. The issue mentions "line 22" with databases = ["basketball", ...] which would be in the actual gitignored k3s.tfvars. Convention per databases.tf: map key = PostgreSQL role name = service name. The stale "basketball" role is a manual artifact outside Terraform state.
  • [x] terraform/keycloak.tf — verified: contains moved blocks (lines 13-21) documenting the westside-ror to westside-basketball key rename. Comment on line 152 of example says "client_id stays westside-ror to avoid Keycloak client re-registration" — this is the intentional decision being reversed by this ticket.
  • [x] terraform/services.tf — verified: contains moved blocks (lines 1-33) for Harbor/namespace/ArgoCD renames from westside-ror to westside-basketball.

Repo Placement

Correct. The Forgejo issue is filed on ldraney/pal-e-services and all file targets (terraform/k3s.tfvars) are in that repo. The stale DB role is a manual psql operation in prod, which is appropriate to track in the IaC repo since the fix involves verifying no Terraform state drift.
Note: the source_path change (overlays/westside-ror/prod to overlays/westside-basketball/prod) references a path in pal-e-deployments, but the config change itself is in pal-e-services. The sibling ticket pal-e-deployments#221 handles the actual overlay rename. This ticket must NOT run until the overlay rename lands, or ArgoCD will break.

Dependencies

Critical ordering dependency identified:
  • Depends on pal-e-deployments#221 (Add Tailscale funnel ingress + rename overlay) — board item #1631. The source_path update in this ticket points to overlays/westside-basketball/prod, which does not exist until the overlay rename in #221 lands. If this ticket's source_path change applies first, ArgoCD will fail to sync because the path won't exist.
  • Board item #1632 (Update NetworkPolicy: replace westside-ror with westside-basketball) is a sibling decomposition ticket — no ordering dependency, can run in parallel.
  • Board item #1618 (Add rails-env Kubernetes secret) is in validation — no conflict.
  • Board item #1605 (Keycloak auth with admin/coach/player roles) is done — predecessor, no conflict.
The dependency on #1631/#221 is NOT documented in the issue body. This is acceptable since the Keycloak config changes (client_id, URLs) can apply independently — only the source_path change has ordering sensitivity. An agent can apply the Keycloak changes first and hold the source_path change.

Acceptance Criteria

9 acceptance criteria. Assessment:
  • AC 1-2 (DB role drop + no state drift): Requires manual psql access + terraform plan. Verifiable but involves prod access — agent needs SSH/kubectl context.
  • AC 3-7 (Keycloak config updates): Straightforward find-and-replace in k3s.tfvars. Fully agent-automatable.
  • AC 8 (terraform plan clean): Verifiable via terraform plan. Requires kubectl port-forward for CNPG + Keycloak providers.
  • AC 9 (Auth flow works end-to-end): Requires browser-based test of login/logout. Could use validate-ui skill but needs Keycloak propagation time.
All criteria are testable. The mix of manual ops (psql, terraform apply) and automated checks (terraform plan, auth test) is appropriate for a 2pt bug fix.

Blast Radius

  • pal-e-deployments overlay: overlays/westside-ror/ still exists with prod and dev subdirectories. The sibling ticket #221 handles the rename. If this ticket runs first on the source_path change, ArgoCD sync will fail.
  • westside-basketball repo: No westside-ror references found in file paths. The Rails app's omniauth-keycloak config references the client_id, so changing from "westside-ror" to "westside-basketball" requires a corresponding update in the Rails app's Keycloak config (likely env var or initializer). The issue does NOT mention this — however, the Rails app likely reads client_id from an environment variable, and the Keycloak provider matches on the client_id value, so the client_id change in Terraform must be coordinated with the Rails app config.
  • keycloak.tf moved blocks: Lines 13-21 have existing moved blocks for the westside-ror to westside-basketball key rename. These are already in place and should not conflict.
  • Comment on line 152 of k3s.tfvars.example: # NOTE: client_id stays "westside-ror" to avoid Keycloak client re-registration. — This comment documents the original intentional decision. Changing the client_id will trigger Keycloak client re-registration. The issue should note whether this is acceptable or whether the moved block approach should be used instead.

Decomposition Assessment

File targets: 1 file (k3s.tfvars) + 1 manual psql operation. All in 1 repo. 9 acceptance criteria (above the 5 threshold), but 7 of them are trivial find-and-replace verifications. Estimated agent work: ~3-4 minutes (edit k3s.tfvars, run terraform plan, verify). No decomposition needed — the AC count is high but the work is uniform.

Recommendation

No action needed — scope is solid for a 2pt bug fix. Two advisory notes:
  • Advisory: The source_path change (AC 7) must not apply until pal-e-deployments#221 (overlay rename) merges. The implementing agent should either: (a) hold the source_path change for a follow-up, or (b) verify the overlay exists before applying.
  • Advisory: Changing client_id from "westside-ror" to "westside-basketball" will trigger Keycloak client re-registration (the comment on line 152 of k3s.tfvars.example explicitly warns against this). The implementing agent should verify that the Rails app's OmniAuth config reads client_id from an environment variable that can be updated in lockstep, or accept a brief auth outage during the transition.