pal-e-docs

Review: B: westside-admin dev overlay (hostPath + tailnet)

review-1143-2026-05-03 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Reviewer: Dottie. Board item #1143 on board-westside-admin. Forgejo issue: forgejo_admin/pal-e-deployments#148. The scope is well-shaped (clear file list, real reference overlay, correct security posture rationale), but five concrete, blocking issues will cause the dev pod to fail to come up if shipped as written. Fix in the issue body before advancing to todo.

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — links pal-e-services#73 and notes blocking dependency
  • [x] Repo — pal-e-deployments
  • [x] User Story — Marcus / dev hot-reload / iteration
  • [x] Context — pattern reference, auth model, env vars, port rationale, ingress posture
  • [x] File Targets — five files enumerated with content sketch
  • [x] Acceptance Criteria — 7 items, all observable
  • [x] Test Expectations — kustomize lint + smoke curl
  • [x] Constraints — mirror reference, reuse Secret, no package.json edits, FF main pre-branch
  • [x] Checklist — present
  • [x] Related — pal-e-services#73 cross-reference

Traceability

  • [x] type:feature label — present on board item
  • [x] arch:dev-overlay label — present on board item
  • [ ] arch:dev-overlay backing note MISSING — search_notes("arch-dev-overlay") returns empty. [SCOPE] Create arch-dev-overlay note (or reuse arch-deployment-westside-admin with a Dev section appended).
  • [x] story:marcus-admin-dev-access label — present on board item
  • [ ] story:marcus-admin-dev-access backing entry MISSING — project-westside-admin#user-stories contains only admin-row-crud. [SCOPE] Add marcus-admin-dev-access row to project-westside-admin User Stories table (Key, Story Note, Role=Marcus, Success Metric).
  • [x] Forgejo issue valid and open — pal-e-deployments#148.
  • [x] Sibling issue valid — pal-e-services#73 (also open, same backing story label).

File Targets

  • [x] overlays/westside-admin/prod/* — verified to exist on origin/main (5 files, including `westside-admin-secrets.enc.yaml` and a `deployment-patch.yaml` that targets the standard base name `app`).
  • [x] overlays/pal-e-dictionary/dev/ reference — verified to exist locally on archbox at /home/ldraney/pal-e-deployments/overlays/pal-e-dictionary/dev/. NOTE: it does not exist on origin/main (untracked in cwd). Reference is informational only; the dev agent must read the local files to mirror them.
  • [x] ~/westside-admin/src/hooks.server.ts — verified. Per-request gate present (REQUIRED_ROLE = 'admin', isAuthExcluded excludes /auth/* + /health, role check at line 264). Auth model claim is accurate: dev pod inherits the gate automatically.
  • [x] Local ~/westside-admin is 3 commits behind origin/main at review time — Constraints section already calls this out (git pull origin main).
  • [x] Local ~/pal-e-deployments is on stale branch 75-rename-pal-e-production — Constraints already require FF main first.
  • [x] westside-admin-secrets Secret — verified to exist in westside-admin namespace. Contains 6 keys: COOKIE_SIGNING_KEY, DATABASE_URL, KEYCLOAK_CLIENT_ID, KEYCLOAK_CLIENT_SECRET, KEYCLOAK_REALM, KEYCLOAK_URL. The issue says DATABASE_URL is "NOT in this Secret" — that is wrong. See Issue 4 below.

Repo Placement

OK — issue correctly filed on pal-e-deployments. The redirect-URI sibling on pal-e-services is a separate issue (#73), correctly split.

Dependencies

  • Blocking: pal-e-services#73 — Keycloak client must list https://westside-admin-dev.tail5b443a.ts.net/auth/callback as a valid redirect URI. Without it, OIDC callback returns 400. Issue body documents this clearly. Board item A on board-westside-admin#1142 tracks it.
  • Implicit dependency NOT documented: ArgoCD Application registration. The existing westside-admin ArgoCD Application is sourced at overlays/westside-admin/prod — NOT the parent directory. Adding overlays/westside-admin/dev/ will be invisible to ArgoCD until a new Application resource (e.g., westside-admin-dev) is created in the cluster pointing at the dev overlay path. The issue's AC #2 ("After ArgoCD syncs ... pod 1/1 Running") will silently never satisfy because Argo isn't watching the new path. [BODY] Add an explicit Constraints/File-Targets bullet: "Register a new ArgoCD Application westside-admin-dev pointing at overlays/westside-admin/dev (mirror the pattern used for pal-e-dictionary-dev) — OR — explicitly state that this overlay is applied via kubectl apply -k by Lucas and lives outside ArgoCD." Without that decision, the dev agent has no path to satisfy AC #2.

Acceptance Criteria

Seven AC, all observable. Mostly testable. Two concerns:
  • AC #2 is not satisfiable as written until the ArgoCD-vs-manual-apply question is resolved (see Dependencies).
  • AC #6 (nslookup westside-admin-dev.tail5b443a.ts.net from outside the tailnet does not resolve to a public IP) is testable but flaky — Tailscale magicdns sometimes returns nothing, sometimes returns the tailnet IP, depending on resolver. A more robust assertion: "from a non-tailnet host, an HTTPS request to the hostname connection-refuses or DNS-NXDOMAINs; from a tailnet host, the TLS cert is issued and the SvelteKit dev server responds." Optional polish, not blocking.

Blast Radius

Five hard blockers found. Listing in priority order.
  • [BLOCKER 1 — Port 5177 is taken] The issue claims "Next free: 5177" based on observing dictionary=5175, kanban=5176. That observation is stale. Live cluster check at review time: pal-e-hub/pal-e-hub-dev stood up 6 min ago with containerPort=5177. Other in-cluster ports in the 5170–5200 range: mcd-tracker-dev=5173, westside-dev=5174, pal-e-dictionary-dev=5175, pal-e-kanban-dev=5176, pal-e-hub-dev=5177, playme2k-dev=5175 (own ns, no conflict). Next actually free: 5178. [BODY] Replace every 5177 in the issue body (Context, deployment.yaml sketch, service.yaml sketch) with 5178. Also update the kustomization.yaml header comment to "Vite port 5178".
  • [BLOCKER 2 — PSA restricted will reject the dev pod] The westside-admin namespace has pod-security.kubernetes.io/enforce: restricted applied (verified live + present in overlays/westside-admin/prod/namespace.yaml on origin/main). The reference overlays/pal-e-dictionary/dev/deployment.yaml has no securityContext — and the pal-e-dictionary namespace has no PSA enforcement, which is why the reference works there. Mirroring "exactly" into westside-admin will fail admission with `seccompProfile not set / runAsNonRoot not set`. [BODY] Add to deployment.yaml sketch: pod-level securityContext with runAsNonRoot: true, runAsUser: 1000, runAsGroup: 1000, fsGroup: 1000, seccompProfile.type: RuntimeDefault; and container-level securityContext with allowPrivilegeEscalation: false, readOnlyRootFilesystem: false (Vite needs writes), capabilities.drop: ["ALL"]. Also note: node:22 default user is root — image will need an explicit non-root user. Two viable options: (a) set image: node:22-bookworm-slim and pre-create a writable workdir under /tmp/app, or (b) use the existing precedent: prod's deployment-patch.yaml sets runAsUser: 1000 on the standard base — replicate that. The simpler path (b): copy the prod patch's securityContext stanza verbatim into the new dev deployment.
  • [BLOCKER 3 — Reference overlay contradicts "tailnet-only" requirement] Constraints say "Mirror overlays/pal-e-dictionary/dev/ structure exactly. Same filenames, same yaml shape, same idioms." The reference ingress.yaml has tailscale.com/funnel: "true". The Context section says "Do not add tailscale.com/funnel: \"true\"". These two instructions directly conflict — a literal mirror produces a public funnel, which the issue explicitly forbids per feedback_funnel_requires_auth. [BODY] Reword the Constraint to "Mirror overlays/pal-e-dictionary/dev/ structure (filenames + general yaml shape), with two intentional deviations: (a) port 5178 not 5175; (b) ingress.yaml omits tailscale.com/funnel: \"true\" — tailnet-only ingress only." Also note the prod ingress IS funneled — the dev pod is the deviation, and the reasoning (admin-grade data) is sound.
  • [BLOCKER 4 — DATABASE_URL claim is wrong] Issue says: "DATABASE_URL is NOT needed for dev — it lives in a separate Secret (admin-app-db-url) used by prod only." Live check: westside-admin-secrets contains 6 keys including DATABASE_URL. Both Secrets contain it (the prod patch sources DATABASE_URL from admin-app-db-url, but the legacy mirrored copy in westside-admin-secrets still exists). Practical effect of envFrom: [secretRef: westside-admin-secrets]: the dev pod WILL receive a real DATABASE_URL. That is probably fine for Marcus's read paths — but the issue's claim is factually wrong, and a future reader will be misled. [BODY] Reword to: "DATABASE_URL is present in westside-admin-secrets (legacy mirror). The dev pod will inherit it via envFrom; this is acceptable for Marcus's read-only views but the dev pod must NOT be used to exercise write paths until a separate dev DB role is wired (out of scope for this ticket — file follow-up if write testing is needed)."
  • [BLOCKER 5 — Selector collision risk] Issue says "deployment named distinctly from prod (suggest westside-admin-dev)". Good. But the prod deployment selector is app: westside-admin (verified live + in prod kustomization patch). The dev deployment must use a distinct label key/value across all three of: spec.selector.matchLabels, spec.template.metadata.labels, and the Service's spec.selector. The reference pal-e-dictionary overlay uses app: pal-e-dictionary-dev (distinct from a non-existent prod) — the dev agent could accidentally just use app: westside-admin if they copy too literally. [BODY] Add an explicit Constraint: "All three label fields (deployment selector, deployment pod template, service selector) must be app: westside-admin-dev — NOT app: westside-admin — to avoid selector collision with the prod deployment in the same namespace. Verify with kubectl -n westside-admin get pods -l app=westside-admin-dev returning only the dev pod and kubectl -n westside-admin get pods -l app=westside-admin returning only the prod pod after kubectl apply." Without this, an over-broad selector returns both pods and the dev Service load-balances 50% of dev traffic to prod (or vice versa).

Decomposition Assessment

Five files in one repo, one agent, ~5 ACs to verify post-merge. Estimated dev-agent time: 25–40 min including kustomize lint + PR + describing apply path. Below the 5-minute rule's per-AC ceiling but well within a single ticket. No decomposition needed. Keep as one ticket.

Recommendation

Verdict NEEDS_REFINEMENT. Before advancing to todo, apply these to the Forgejo issue body (the single source of truth dev agents read):
  • [BODY] Replace 5177 with 5178 everywhere in the issue (Context port table, deployment containerPort, service targetPort, kustomization comment).
  • [BODY] Add securityContext requirement (pod + container level) per Blocker 2; reference prod's deployment-patch.yaml securityContext as the template to copy.
  • [BODY] Reword the "mirror exactly" Constraint to call out the two intentional deviations (port + no funnel annotation).
  • [BODY] Fix the DATABASE_URL Context paragraph to reflect that the Secret DOES contain it; note dev is read-only-acceptable but flag for follow-up if write testing becomes needed.
  • [BODY] Add explicit Constraint requiring app: westside-admin-dev across deployment selector, pod template labels, and service selector to avoid selector collision with prod in the same namespace.
  • [BODY] Resolve the ArgoCD-vs-manual-apply question: either add a Constraint/File-Target requiring a new ArgoCD westside-admin-dev Application resource pointing at the new path, OR explicitly state the dev overlay is hand-applied with kubectl apply -k and lives outside ArgoCD. Reword AC #2 accordingly.
  • [SCOPE] Add marcus-admin-dev-access row to project-westside-admin#user-stories table. (Dottie can do this on Ava's instruction once Ava confirms wording.)
  • [SCOPE] Decide whether arch:dev-overlay deserves a standalone arch-dev-overlay note or appends a Dev section to arch-deployment-westside-admin.
Once the six [BODY] items land in the issue, the ticket is ready for re-review and APPROVED → todo. The two [SCOPE] items are doc-side and don't block dev work but should be tracked so traceability is complete by the time the PR merges.