Review: Service onboarding: Keycloak + Twitch IdP + CNPG + funnel

review-786-2026-04-03-final Doc

review ready

Verdict: APPROVED

Third review pass. All four items from previous NEEDS_REFINEMENT verdict have been resolved. Scope is tight, traceability complete, file targets verified.

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Standalone
  • [x] Repo — forgejo_admin/pal-e-services
  • [x] User Story — Twitch viewer sign-in via Keycloak
  • [x] Context — thorough, includes key decisions (service key, CNPG strategy)
  • [x] File Targets — 4 files to modify, 2 files explicitly excluded
  • [x] Acceptance Criteria — 7 items, all verifiable
  • [x] Test Expectations — 4 items with concrete commands
  • [x] Constraints — 4 items including tofu plan -lock=false
  • [x] Checklist — present
  • [x] Related — project, arch notes, sibling ticket

Traceability

  • [x] story:challenger-auth label — "Challenger Twitch Login"
  • [x] story note verified — story-twitch-2k-wager-challenger-auth (id 1157) exists, listed in project-twitch-2k-wager user-stories table
  • [x] arch:dataflow label — Data Flow architecture
  • [x] arch note verified — arch-dataflow-twitch-2k-wager (id 1155) exists, active
  • [x] arch:deployment label — Deployment architecture
  • [x] arch note verified — arch-deployment-twitch-2k-wager (id 1156) exists, active
  • [x] Forgejo issue — forgejo_admin/twitch-2k-wager#1, open

File Targets

  • [x] k3s.tfvars — verified: contains keycloak_realms, keycloak_clients, and services maps to extend. Existing patterns (westside-basketball realm, westside-landing client, 8 service entries) provide clear templates.
  • [x] keycloak.tf — verified: contains realm, client, role resources via for_each. No existing keycloak_oidc_identity_provider resource — confirms this is a NEW resource type as ticket states.
  • [x] variables.tf — verified: contains keycloak_realms and keycloak_clients type definitions. No identity_provider variable type exists yet — needs adding as ticket states.
  • [x] cnpg.tf — verified: shared pal-e-postgres cluster in postgres namespace. Comment explicitly documents that new databases require SQL, not manifest changes. Ticket correctly targets this file for database + role provisioning.
  • [x] services.tf — correctly listed as "NOT to touch" — generic for_each over var.services handles everything.

Repo Placement

OK. Issue body correctly identifies pal-e-services as the target repo. Forgejo issue is filed on twitch-2k-wager (project repo), which is the correct organizational home. Single repo scope — no cross-repo concerns.

Dependencies

  • [x] Board item #788 (Kustomize overlay, forgejo_admin/twitch-2k-wager#3) — depends on this ticket, documented in Related section. Currently in todo column.
  • [x] No upstream blockers. This ticket is foundational infra with no prerequisites.

Acceptance Criteria

All 7 criteria are agent-verifiable:
  • AC 1-3 (Keycloak realm, Twitch IdP, OIDC client): verifiable via tofu plan output showing expected resources
  • AC 4 (service key playme2k): verifiable via tofu plan showing Harbor project, namespace, ArgoCD app, funnel
  • AC 5 (database + role): verifiable via tofu plan or kubectl exec psql check
  • AC 6 (Twitch credentials as variables): verifiable by checking variables.tf definitions
  • AC 7 (clean tofu plan): directly executable command
Test expectations are concrete: tofu plan, kubectl get ns, Keycloak admin console, HTTPS resolution check.

Blast Radius

Low. Net-new resources only — no modifications to existing infrastructure. The keycloak_oidc_identity_provider is a new resource type not yet in the codebase, so no existing patterns to break. The services for_each is additive. CNPG database creation is additive to the shared cluster. Rollback is straightforward: tofu destroy targeting the new resources.

Decomposition Assessment

4 file targets, 1 repo, 7 AC. The AC count is slightly above the 5 threshold, but all resources are tightly coupled — a single tofu apply provisions the full stack (realm, IdP, client, service entry, database). Decomposing would create artificial boundaries between interdependent Terraform resources that must be applied together. No independent subtasks exist that could be parallelized. No decomposition needed.

Previous Review Resolution

  • [x] Namespace vs hostname — RESOLVED. Service key = playme2k, namespace = playme2k, hostname = playme2k.tail5b443a.ts.net. Explicitly stated in issue body.
  • [x] CNPG strategy — RESOLVED. Shared cluster, new database. Explicitly stated: "Use the existing pal-e-postgres CNPG cluster in the postgres namespace."
  • [x] Phantom SOP reference — RESOLVED. Removed from issue body.
  • [x] arch:twitch-oauth label — RESOLVED. Changed to arch:dataflow,arch:deployment. Both backing notes exist.

Recommendation

No action needed.