pal-e-docs

Review: Provision k8s secrets for notion-mcp-remote

review-1047-2026-04-21 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Board item #1047 / Forgejo issue forgejo_admin/notion-mcp-remote#7 — feat: provision app secrets in notion-mcp-remote namespace (pre-sync). Type: Feature. Points: 2. Labels: type:feature,story:ops-deploy-gitops,arch:k8s-secret.

Template Completeness

Checked against template-issue-feature expected sections:
  • [x] Type — Feature
  • [x] Lineage — standalone, cites project + story + SOP row
  • [x] Repo — forgejo_admin/notion-mcp-remote
  • [x] User Story — role / want / so-that present
  • [x] Context — references SOP, envFrom pattern, dependencies called out
  • [x] File Targets — correctly marked "No repo file changes. This is a kubectl operation."
  • [x] Acceptance Criteria — 5 items, each verifiable
  • [x] Test Expectations — present, with runnable kubectl command
  • [x] Constraints — three guardrails (no git-stored manifests, generated entropy, namespace prereq)
  • [x] Checklist — 4 items
  • [x] Related — links to project, story, SOP, arch note

Traceability

  • [x] story:ops-deploy-gitops label — GitOps Deploy via pal-e Platform
  • [x] story note verified — story-notion-mcp-remote-ops-deploy-gitops exists; listed in project-notion-mcp-remote user-stories section; AC row "Secrets provisioned via kubectl create secret generic before first ArgoCD sync" is the exact line this ticket fulfills
  • [~] arch:k8s-secret label — no dedicated arch-k8s-secret note exists. The relevant architecture coverage lives in arch-deployment-notion-mcp-remote (key decision: "Secrets created via kubectl before first sync") and the platform-wide arch-secrets-pipeline. The label is unusual for this project (other tickets on this board use service-scoped arch labels like arch:notion-oauth, arch:tailscale-funnel).
  • [x] Forgejo issue — URL valid, issue #7 is open, body is well-formed

File Targets

No repo file targets (Feature ticket is a pure operator/kubectl action). However, to verify that the ticket's secret shape matches what the code actually consumes, I spot-checked the deployment manifest and server envvars.
  • [x] server.py — confirmed reads NOTION_OAUTH_CLIENT_ID, NOTION_OAUTH_CLIENT_SECRET, SESSION_SECRET, BASE_URL, ONBOARD_SECRET (lines 19–25). No Fernet/token-encryption key is read beyond SESSION_SECRET which TokenStore uses for encryption. Env-var names in the ticket match the code.
  • [!] k8s/deployment.yaml — MISMATCH. Currently references Secret name notion-mcp-secrets (not notion-mcp-remote as the ticket AC specifies), via valueFrom: secretKeyRef (not envFrom), with keys in kebab-case: base-url, oauth-client-id, oauth-client-secret, session-secret, onboard-secret. If an operator follows the ticket AC literally (kubectl create secret generic notion-mcp-remote with UPPER_SNAKE keys), the pod will CreateContainerConfigError because the secret the Deployment references does not exist and the keys won't line up.
This is the primary refinement blocker. See Recommendations.

Repo Placement

OK. Issue lives on forgejo_admin/notion-mcp-remote; the kustomize overlay that would reference the secret lives on forgejo_admin/pal-e-deployments (tracked by board item #1044); the namespace lives on forgejo_admin/pal-e-platform (issue #296, board item #1045). Cross-repo linkage is correct — this ticket correctly scopes only the kubectl provisioning step.

Dependencies

  • Blocks on issue #6 (board #1046) — "feat: register public Notion OAuth integration for remote proxy" — produces the real CLIENT_ID / CLIENT_SECRET values. Currently in backlog. Dependency is explicitly documented in the Context section. OK.
  • Blocks on pal-e-platform issue #296 (board #1045) — "tofu apply" that creates the notion-mcp-remote namespace. Currently in backlog. Dependency is explicitly documented in Constraints. OK.
  • Blocks pal-e-deployments #132 (board #1044) kustomize overlay and pal-e-services #57 (board #1043) ArgoCD Application — both must not sync until secrets exist, or they will be replaced with placeholders per SOP. Not explicitly documented as downstream blockees in the issue; the SOP reference implies it.
  • All dependencies are documented adequately for an operator to sequence the work.

Acceptance Criteria

Criteria are testable and each has a runnable verification command. The final AC "Disaster-recovery copy stored in ~/secrets/notion-mcp-remote/" aligns with sop-secrets-management Layer 2 (plaintext .env host backup). The issue picks the legacy manual kubectl path (documented in sop-secrets-management → "Adding a new app secret (manual path — legacy)"), which is still explicitly blessed by the pre-deploy-validation checklist row for "Application secrets."
The SOP does note that SOPS+Age is the preferred path for new services. The ticket chose manual kubectl, which is a valid strategic choice for a pre-sync bootstrap (SOPS still requires ArgoCD to run decryption, so manual is the only option strictly before the first sync). No refinement needed on this front — calling it out for visibility.

Blast Radius

  • Other remote MCP services on the platform (e.g. gmail-mcp-remote pattern) likely use the same pre-sync manual path. No collateral risk.
  • Regenerating SESSION_SECRET invalidates any existing encrypted tokens.json — since this is the first-time bootstrap (no prior tokens), no data loss. Worth a callout in the issue body for future rotations.
  • If the Secret name / keys are fixed (see refinement below), no code change is needed; if envFrom is adopted instead, the Deployment manifest changes and a new image need not be rebuilt but the Deployment patch will churn.

Decomposition Assessment

1 repo, 0 file targets, 5 AC items, single kubectl command plus backup copy. Estimated operator time once dependencies are met: <5 minutes. No decomposition needed.

Recommendation

  • [BODY] Reconcile the secret name and key shape with k8s/deployment.yaml. The deployment currently references Secret notion-mcp-secrets with kebab-case keys via valueFrom: secretKeyRef. Pick one of two paths and update the AC accordingly:
  • [LABEL] Consider replacing arch:k8s-secret with a more specific, existing arch note reference. arch:deployment-notion-mcp-remote would map to the existing arch note that already documents the "secrets before first sync" decision; arch:secrets-pipeline would map to the platform-wide note. Optional — the current label is interpretable but inconsistent with the rest of this board.
  • [BODY] (nit) The Context paragraph references envFrom as the reference mechanism, which contradicts both the current deployment (valueFrom) and Option A above. Update whichever language survives the refinement.
  • [BODY] (nit) Consider adding a post-provision callout to the issue body: "If SESSION_SECRET is ever rotated, any persisted encrypted tokens in the PVC become unreadable and users must re-OAuth." This is first-time bootstrap so it is not a blocker, but future readers benefit.

Summary

Scope, traceability, dependencies, and SOP alignment are all solid. One concrete, fixable mismatch between the AC's secret shape and what the deployment manifest actually reads is the reason this ticket is NEEDS_REFINEMENT rather than READY. Once the AC is reconciled with k8s/deployment.yaml (Option A is the least churn), the ticket can proceed to todo.