Review: Provision infrastructure in pal-e-services

review-593-2026-03-28 Doc

review needs-refinement

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Standalone, scoped from design spec
  • [x] Repo — forgejo_admin/pal-e-services
  • [x] User Story — Marcus as admin, infra provisioning
  • [x] Context — sufficient background
  • [x] File Targets — present with should/should-not-touch
  • [x] Acceptance Criteria — 4 criteria listed
  • [x] Test Expectations — tofu validate + plan command
  • [x] Constraints — present
  • [x] Checklist — present
  • [x] Related — present

Traceability

  • [x] story:read-ops — Admin Read Operations via GroupMe
  • [x] story:write-ops — Admin Write Operations via GroupMe
  • [x] arch:A4 — Deployment architecture component
  • [x] Forgejo issue — forgejo_admin/westside-ai-assistant#1, open

File Targets

  • [x] terraform/k3s.tfvars — verified: file exists, contains both services map (line 104) and keycloak_clients map (line 40). basketball-api reference entry confirmed at line 113. westside-app client reference confirmed at line 41.
  • [ ] terraform/variables.tf — ISSUE: not listed as a file target, but MUST be modified. The keycloak_clients variable type (line 102-125) has no service_accounts_enabled attribute. Adding a service-account client requires adding this attribute to the variable type object.
  • [ ] terraform/keycloak.tf — ISSUE: not listed as a file target, but MUST be modified. The keycloak_openid_client resource (line 91-118) does not set service_accounts_enabled. Additionally, there is no keycloak_openid_client_service_account_role resource anywhere in the codebase — this is needed to map the admin role to the service account.

Repo Placement

Correct. Issue is filed on westside-ai-assistant but specifies work in forgejo_admin/pal-e-services, which is the correct repo for infrastructure provisioning. Single-repo change.

Dependencies

  • [ ] Issue #2 (Create kustomize overlay in pal-e-deployments) — board item #594, backlog. The services entry's source_path = "overlays/westside-ai-assistant/prod" does not exist yet in pal-e-deployments. ArgoCD will fail to sync until the overlay is created. Merge order: #1 first, #2 second, apply after both merged. Not documented in issue scope.
  • [x] Issue #3 (NetworkPolicy update) — board item #595, backlog. Post-deploy dependency, not blocking for this ticket.

Acceptance Criteria

  • [ ] AC1: services entry — testable via tofu plan, but missing required fields. The AC says port: 8000, funnel: true, source_repo, source_path but omits forgejo_repo and image_repo, which are REQUIRED fields per the variable definition in variables.tf (lines 130-131).
  • [ ] AC2: "Keycloak client westside-ai-bot is confidential, service account enabled" — NOT achievable with current variable type. The keycloak_clients variable has no service_accounts_enabled attribute. The keycloak_openid_client resource does not set it. Variable + resource must both be extended.
  • [ ] AC3: "Client has realm role admin mapped" — NOT achievable with current codebase. No keycloak_openid_client_service_account_role resource exists. A new resource block must be added to keycloak.tf, plus a new variable attribute to drive it (e.g., service_account_realm_roles).
  • [x] AC4: tofu plan shows expected resources — testable, but will only show correct resources once AC2/AC3 infrastructure is added.

Blast Radius

Low for the services entry — existing for_each pattern handles new entries automatically. Medium for the keycloak changes — modifying the variable type and resource affects ALL existing keycloak clients. The new attributes must use optional() with safe defaults (false / []) to avoid breaking existing client entries. No other service currently uses service accounts, so this is a net-new capability addition to the platform. Rollback is straightforward: revert the tfvars entry.

Decomposition Assessment

The ticket touches 3 files (k3s.tfvars, variables.tf, keycloak.tf) in 1 repo with 4 AC. Once the file targets and AC are corrected, this is still achievable in a single agent pass (<5 min). The three files are tightly coupled (variable definition, resource usage, variable values) and cannot be parallelized. No decomposition needed.

Recommendation

  • [BODY] Add file target: terraform/variables.tf — add service_accounts_enabled = optional(bool, false) and service_account_realm_roles = optional(list(string), []) to the keycloak_clients variable type object.
  • [BODY] Add file target: terraform/keycloak.tf — add service_accounts_enabled = each.value.service_accounts_enabled to the keycloak_openid_client resource block, and add a new keycloak_openid_client_service_account_realm_role resource for service account role mapping.
  • [BODY] Fix AC1: add forgejo_repo: "forgejo_admin/westside-ai-assistant" and image_repo: "westside-ai-assistant/api" to the expected services entry fields.
  • [BODY] Add dependency note to Constraints section: "Depends on issue #2 (kustomize overlay) before tofu apply. Merge order: #1 first, #2 second, apply after both merged."
  • [BODY] Clarify in File Targets "should NOT touch" section: remove any implication that variables.tf and keycloak.tf are off-limits — they must be modified to support service accounts.