pal-e-docs

Review: S1 — pal-e-services: add westside-streamlit to var.services

review-1069-2026-04-22 Doc

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Board item #1069 · Forgejo issue forgejo_admin/pal-e-services#60 · Reviewer: Dottie · Date: 2026-04-22

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — explicit (audit-discovered, links siblings + supersession)
  • [x] Repo — forgejo_admin/pal-e-services
  • [x] User Story — well-formed As/I want/So that
  • [x] Context — substrate audit summary, schema verified, hostname declared
  • [x] File Targets — files to modify and NOT to touch both listed
  • [x] Acceptance Criteria — 7 items
  • [x] Test Expectations — fmt/validate/plan with explicit run commands
  • [x] Constraints — present
  • [x] Checklist — present
  • [x] Related — present

Traceability

  • [x] story:reachable label — Dashboard Reachable from Phone
  • [x] story note verified — story-westside-streamlit-reachable exists at id 1401
  • [x] arch:services-tf label present
  • [ ] arch note MISSING — no arch-services-tf note in pal-e-docs (search returned no results). The project-westside-streamlit Architecture section lists arch-domain-westside-streamlit, arch-dataflow-westside-streamlit, arch-deployment-westside-streamlit, arch-ci-pipeline — but nothing for the services-tf component. [SCOPE] Create architecture note arch-services-tf documenting the var.services fan-out pattern (Harbor project + robot accounts + Harbor creds + namespace + ArgoCD app + optional funnel). This pattern serves the entire platform, not just westside-streamlit, so the note belongs at platform scope, not project scope.
  • [x] Forgejo issue — #60 open, body well-structured

File Targets

  • [x] ~/pal-e-services/terraform/k3s.tfvars — verified, exists, contains the services map block. Confirmed westside-streamlit key is NOT yet present (no false-positive), so the change is correctly scoped as additive.
  • [x] ~/pal-e-services/terraform/variables.tf — verified, schema (lines 165-177) supports optional source_repo + source_path for kustomize overlays. Required fields: forgejo_repo, image_repo, port, funnel. Optional: target_revision, source_repo, source_path. Issue's "no schema extension needed" claim is correct.
  • [x] ~/pal-e-services/terraform/services.tf — verified, fans out via for_each = var.services across harbor_project.service, harbor_robot_account.service_ci, harbor_robot_account.service_pull, kubernetes_namespace_v1.service, kubernetes_secret_v1.harbor_creds, argocd_application.service. AC plan-output checks match the resource names exactly.
  • [x] Overlay path — ~/pal-e-deployments/overlays/westside-streamlit/prod/ verified (6 files: namespace, kustomization, ingress, secret.enc, harbor-creds.enc, deployment-patch). Namespace name in namespace.yaml = westside-streamlit ✓ matches AC.
  • [!] Issue body says: "Reference the shape used by an existing kustomize-overlay service (e.g. westside-contracts, pal-e-dora-exporter)." — BOTH OF THESE REFERENCE SERVICES DO NOT EXIST in ~/pal-e-services/terraform/k3s.tfvars. grep westside-contracts ~/pal-e-services/terraform/k3s.tfvars returns zero hits; same for pal-e-dora-exporter. A dev grepping for the suggested examples will waste time. Existing kustomize-overlay services in k3s.tfvars: platform-validation, basketball-api, pal-e-docs, westsidekingsandqueens, pal-e-app, gcal-scheduler, mcd-tracker, mcd-tracker-app, pal-e-mail, westside-ai-assistant, playme2k. [BODY] Replace the example list with services that actually exist — recommend basketball-api (close shape: same file lives in pal-e-deployments overlay, image_repo prefix matches service key) AND pal-e-mail (matches the funnel = false pattern this ticket needs, see next finding).

Repo Placement

OK. Issue is correctly filed on forgejo_admin/pal-e-services. Single-repo change, no cross-repo concerns.

Dependencies

Documented:
  • Lineage block calls out sibling pal-e-services#61 (Keycloak client) and notes that westside-streamlit#3/#6 are blocked by this ticket.
  • Lineage notes that westside-streamlit#10 was closed-but-incomplete and is superseded by sibling #61. Verified by reading #61 body.
Board state cross-check (board-westside-streamlit):
  • #1069 (this ticket, S1) and #1070 (S2/Keycloak) both in backlog.
  • #1071 (C1 Monthly), #1073 (C3 Tournament, blocked by C2), #1074 (O1 Marcus onboarding), #1072 (C2 migration 048) all in backlog.
  • QA column has #960 (Keycloak client #10), #961 (streamlit-keycloak in-app), #933 (CI pipeline #2), #936 (Postgres role #5), #935 (k8s deploy #4) — none of these are blocking this ticket.
  • #938 (Tailscale funnel #7) and #962 (#12 secret/env) in next_up — the ticket does not block on these.
Recommendation: dependencies are correctly mapped; no missing blockers.

Acceptance Criteria

7 criteria, all individually verifiable. Notes:
  • AC1 (key = westside-streamlit) — verifiable via grep
  • AC2 (image_repo prefix) — verifiable via diff and Harbor project name post-apply
  • AC3 (source_path = "overlays/westside-streamlit/prod") — pre-verified, directory exists
  • AC4 (5 specific resource creates) — exact match to services.tf resource names; agent can grep plan output
  • AC5 (ArgoCD targets namespace westside-streamlit) — guaranteed by destination.namespace = each.key in services.tf line 155
  • AC6 (namespace creates cleanly or no-ops) — handles the pre-existing-namespace edge case correctly
  • AC7 (no tofu apply) — explicit Lucas-applies-after-merge gate
Missing AC — see Blast Radius / Decomposition for required additions.

Blast Radius

Critical risk: funnel collision. The kustomize overlay at ~/pal-e-deployments/overlays/westside-streamlit/prod/ingress.yaml already declares its own Tailscale ingress:
If the new tfvars entry sets funnel = true, kubernetes_ingress_v1.service_funnel["westside-streamlit"] in services.tf (line 177) will create a SECOND westside-streamlit-funnel ingress in the same namespace. Both will register with Tailscale; only one will win the funnel hostname; the loser becomes a dead address. The comment block right above kubernetes_ingress_v1.service_funnel (services.tf:171-175) explicitly warns about this pattern — it's the exact lesson learned from westside-landing-funnel.
The issue's File Targets section says "Copy the shape from an existing kustomize-overlay service entry verbatim" — but most kustomize-overlay services in k3s.tfvars use funnel = true (e.g. basketball-api, pal-e-docs, pal-e-app, gcal-scheduler, mcd-tracker, mcd-tracker-app, westside-ai-assistant, playme2k, platform-validation). Only westsidekingsandqueens and pal-e-mail use funnel = false. A literal "copy verbatim" instruction will likely select the wrong template. [BODY] Add an explicit AC: "funnel = false — the kustomize overlay at overlays/westside-streamlit/prod/ingress.yaml already manages the Tailscale ingress; a Terraform-managed funnel would create a duplicate ingress and cause the same dead-address class of bug warned about in services.tf:171-175."
Secondary risk: forgejo_repo field omission. The schema requires forgejo_repo (line 168 of variables.tf, no default). The issue's AC list does not name it. A dev who copies westsidekingsandqueens verbatim will end up with forgejo_repo = "forgejo_admin/westside-app" — wrong. The correct value is forgejo_admin/westside-streamlit. [BODY] Add to AC: "forgejo_repo = "forgejo_admin/westside-streamlit" (the source repo for ArgoCD Image Updater write-back; distinct from source_repo which points to pal-e-deployments)."
Tertiary risk: port field. The funnel block uses each.value.port as the upstream port. With funnel = false this is harmless (the funnel resource is gated if v.funnel), so any sane number works. Recommend documenting port = 80 to match the overlay's Service port (overlay maps port: 80 → targetPort: 8501 in kustomization.yaml lines 47-51). Not a hard blocker since funnel = false short-circuits the field, but it pre-empts a future toggle bug.
Pattern-similar bugs to consider: Per feedback_harbor_project_naming, the image_repo prefix and service key MUST match exactly to avoid the 36-hour-outage class of bug. AC2 enforces this. ✓

Decomposition Assessment

5-minute rule: 1 file, 1 repo, ~10 lines added to k3s.tfvars, 7 ACs. Well within a single agent pass. No decomposition needed.

Recommendation

Three findings; two are body-fix, one is scope. None are BLOCK-class.
  • [BODY] Replace the non-existent reference services in File Targets (westside-contracts, pal-e-dora-exporter) with services that actually exist in k3s.tfvars. Suggested: basketball-api for the kustomize-overlay shape and pal-e-mail for the funnel = false precedent. Without this, a dev grepping for the suggested examples wastes time and may copy a wrong-shape template.
  • [BODY] Add explicit AC: funnel = false with a one-line reason citing the existing overlay-managed ingress and the services.tf:171-175 warning. This is the highest-impact fix — without it the dev is one mis-copy away from a dead-ingress incident.
  • [BODY] Add explicit AC: forgejo_repo = "forgejo_admin/westside-streamlit" so a verbatim copy from westsidekingsandqueens doesn't carry the wrong source repo through.
  • [SCOPE] Create architecture note arch-services-tf at platform scope documenting the var.services fan-out pattern (label-traceability requirement; the label arch:services-tf currently points to nothing). Lower urgency than the body fixes — the ticket can advance once the body is fixed; the arch note can be filed as a sibling story-tracking ticket.
After the three [BODY] fixes land via skill-refine-ticket, this ticket is APPROVED for advance to todo.