Review: Deployment Secret + env wiring for Keycloak (child of #7)

review-962-2026-04-11 Review

review needs-refinement

Verdict: NEEDS_REFINEMENT

Board item #962 — forgejo_admin/westside-streamlit#12. Child C of #7 Keycloak decomposition.

Template Completeness

  • [x] Type = Feature
  • [x] Lineage / User Story / Context / Repo / File Targets / Acceptance Criteria / Test Expectations / Constraints / Checklist / Related — all present

Traceability

  • [x] story:reachable label — "Marcus can open the dashboard on his phone with Keycloak SSO"
  • [x] story note verified — story-westside-streamlit-reachable listed in project-westside-streamlit user-stories
  • [x] arch:keycloak-oidc label
  • [ ] arch note MISSING — [SCOPE] Create architecture note arch-keycloak-oidc (search_notes returns nothing). Affects all three #7 children.
  • [x] Forgejo issue — https://forgejo.tail5b443a.ts.net/forgejo_admin/westside-streamlit/issues/12 open
  • [x] parent:7 label — umbrella linkage documented

File Targets

  • [ ] overlays/westside-streamlit/secret-keycloak.sops.yaml — ISSUE: repo SOPS convention is *.enc.yaml, not *.sops.yaml. .sops.yaml creation_rules regex is \.enc\.yaml$. A .sops.yaml file will NOT be auto-encrypted by SOPS and will commit plaintext. Rename to secret-keycloak.enc.yaml. Precedent: overlays/basketball-api/prod/harbor-creds.enc.yaml.
  • [ ] overlays/westside-streamlit/deployment.yaml — ISSUE: existing overlays don't ship a literal deployment.yaml. Pattern is bases/standard + a deployment-patch.yaml (see overlays/basketball-api/prod/). Env additions should go in a deployment-patch patching the base Deployment, not a new deployment.yaml.
  • [ ] Overlay directory name — ISSUE: existing streamlit overlay is overlays/pal-e-streamlit/ and overlays follow {service}/{dev,prod}/ layout. Ticket writes overlays/westside-streamlit/ with no env subdir. Needs alignment with #4's actual directory structure before this ticket runs.
  • [x] kustomization.yaml — correct file to register new Secret resource

Repo Placement

Correct repo: forgejo_admin/pal-e-deployments. Single-repo change.

Dependencies

Sequencing documented and correct: #4 (creates overlay Deployment/patch) → #10 Child A (creates Keycloak client, produces secret value) → this ticket (Child C) → lands alongside #11 Child B (consumes env vars). Hard-blocks #6 Tailscale funnel. Board state: #4, #10, #11 all in backlog; #7 in next_up. This ticket cannot enter in_progress until #4 and #10 are merged.

Acceptance Criteria

7 ACs, all testable: SOPS round-trip (sops -d), AC for single-key-only content, env wiring verified via kubectl exec ... env | grep KC_, rollout status, kustomize build. Real commands. Only gap: AC does not assert the git status plaintext-gate — it's in Constraints but not an AC. Minor.

Blast Radius

Purely additive to an overlay that doesn't exist yet — near-zero blast radius for other services. The KC_CLIENT_SECRET rotation story inherits from Child A. ArgoCD sync will pick up the new Secret + Deployment patch on merge. Watch: basketball-api uses the same age recipient; any SOPS tooling mistake here could theoretically affect that service's round-trip if decryption config is shared.

Decomposition Assessment

3 files, 7 ACs, single repo, purely additive. Estimated <5 min agent work. No decomposition needed.

Recommendation

  • [BODY] Rename secret-keycloak.sops.yamlsecret-keycloak.enc.yaml in File Targets, Acceptance Criteria, and Test Expectations. Match the repo's existing SOPS creation_rules regex (\.enc\.yaml$). Cite precedent overlays/basketball-api/prod/harbor-creds.enc.yaml.
  • [BODY] Replace overlays/westside-streamlit/deployment.yaml with overlays/{overlay-name}/{env}/deployment-patch.yaml (additive patch to the base Deployment, not a literal deployment.yaml). Match the basketball-api overlay pattern.
  • [BODY] Align overlay directory name + env subdir with #4 output. Either overlays/pal-e-streamlit/{dev,prod}/ or confirm #4 creates overlays/westside-streamlit/{dev,prod}/. Add env subdir to all three file paths.
  • [BODY] Promote the "git status before commit — no plaintext" gate from Constraints into an Acceptance Criterion so QA explicitly verifies.
  • [SCOPE] Create architecture note arch-keycloak-oidc covering realm, client, redirect URIs, and the env var contract. Blocks arch traceability for #7, #10, #11, and #12.