pal-e-docs

Review: Add paldocs Keycloak OIDC client via Terraform

review-1387-2026-07-03 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Re-review after previous NEEDS_REVISION finding (missing NetworkPolicy dependency). That dependency is now filed as pal-e-platform #491 (board item #1708). Core scope is solid -- remaining findings are traceability gaps and a redirect URI scope concern.

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- Child of paldocs#23
  • [x] Repo -- ldraney/pal-e-services
  • [x] User Story
  • [x] Context
  • [x] File Targets
  • [x] Feature Flag -- none
  • [x] Acceptance Criteria -- 4 items
  • [x] Test Expectations -- tofu validate + plan
  • [x] Constraints
  • [x] Checklist
  • [x] Related

Traceability

  • [x] story:auth label -- present on board item #1387
  • [x] story note verified -- story-paldocs-auth found in project-paldocs user-stories section
  • [x] arch:keycloak label -- present on board item #1387
  • [ ] arch note MISSING -- search for "arch-keycloak" returned no results. [SCOPE] Create architecture note arch-keycloak for Keycloak component.
  • [x] Forgejo issue -- ldraney/paldocs#26, open

File Targets

  • [x] terraform/k3s.tfvars -- verified: exists (symlink to ~/secrets/pal-e-services/k3s.tfvars). keycloak_clients map at line 92. No paldocs entry exists yet. pal-enterprises precedent confirmed at line 116 (public_client=false, PKCE S256, realm roles mapper, use_refresh_tokens).
  • [x] terraform/keycloak.tf -- verified: exists. Uses for_each on var.keycloak_clients (line 148-149). No changes needed.
  • [x] terraform/variables.tf -- verified: exists. Type supports public_client (line 173), pkce_code_challenge_method (line 176), include_realm_roles_mapper (line 185), use_refresh_tokens (line 194). No changes needed.

Repo Placement

Mismatch: issue filed on ldraney/paldocs but ### Repo says ldraney/pal-e-services. File targets are in pal-e-services/terraform/. Other cross-repo board items follow the convention of filing in the work repo: pal-e-platform #490, pal-e-deployments #224, pal-e-platform #491 are all filed where the work happens. This issue breaks that convention. Advisory -- the ### Repo field disambiguates for agents, so not blocking.

Dependencies

  • Upstream (resolved): pal-e-platform #491 / board #1708 (NetworkPolicy: paldocs namespace to Keycloak) -- filed on board, sprint:3. This was the previous review finding, now addressed.
  • Downstream: #1704 (KEYCLOAK_* env vars in paldocs deployment), #1389 (Rails OIDC integration), #1390 (visibility filtering). These depend on the Keycloak client existing.
  • Not documented: NetworkPolicy dependency (#1708) is not mentioned in the issue body's Lineage or Related sections.

Acceptance Criteria

4 criteria, all agent-verifiable. Test expectations include real commands (tofu validate, tofu plan -var-file=k3s.tfvars). No missing criteria for the Terraform scope.

Blast Radius

Low. The for_each pattern in keycloak.tf isolates each client as a separate resource. Adding paldocs to keycloak_clients cannot affect existing clients. The pal-e-docs realm is shared with pal-e-app but no realm-level changes are needed (admin/user roles already exist).
Redirect URI concern: The issue says "Follow the existing pal-enterprises client pattern exactly," but pal-enterprises only uses Tailscale hostnames. The landscaping-assistant client (line 139-166 in k3s.tfvars) is the better precedent -- it includes BOTH Tailscale hostname AND public domain URIs (landscaping-assistant.app). Since the user owns paldocs.app and DNS/Caddy work is on the board (#1703), the paldocs client should include paldocs.app URIs alongside paldocs.tail5b443a.ts.net. Following pal-enterprises literally would require a follow-up change when the public domain goes live.

Decomposition Assessment

1 file target, 4 AC, estimated agent work under 5 minutes. No decomposition needed.

Recommendations

  • [SCOPE] Create architecture note arch-keycloak for Keycloak component in pal-e-docs.
  • [BODY] Update Constraints: change precedent from pal-enterprises to landscaping-assistant. Include both paldocs.tail5b443a.ts.net and paldocs.app in redirect URIs, web origins, and post-logout URIs. DNS/Caddy work (#1703) is on the board -- wiring both domains now avoids a follow-up client update.
  • [BODY] Add pal-e-platform #491 (NetworkPolicy) to Lineage or Related section as a dependency.
  • [BODY] Repo placement advisory: consider refiling as pal-e-services issue to match convention (other cross-repo items on paldocs board are filed where work happens). Not blocking -- ### Repo field disambiguates.