prole/docs/plans/onboarding-tdd-phase-a.md
chrisfu 00f0ebec07 Merge claude/crazy-bose-fec256 into main
Bringing the long-running session-feature branch back into main in one
deliberate sweep. The branch carried the cluster work that's been live for
weeks (cross-cluster CNPG metrics, Grafana w/ Google OAuth, supabase
oauth2-proxy, cluster recovery, pg.0.knoe.dev + per-engineer onboarding,
GCS-backed CNPG backups via Workload Identity, the env-contamination
guard, the Junie brief queue, the cnpg-grafana CSRF + memory-request
fixes from today), while main accumulated Junie's parallel knoe-auth
Phase 2 OIDC work (full provider surface: discovery, authorize, token,
userinfo, JWKS, RS256 signing, code exchange, session services).

Key decision: the two branches did COMPETING rebrands off the same
starting point (5ba9b63, 2026-04-27):

  - claude branch (commit b355855, earlier): org.prole.authority.* →
                                              dev.knoe.auth.*
                                              (artifact renamed to
                                              knoe-auth.jar)
  - main (commit 9daa94b, recent): org.prole.authority.* →
                                    dev.knoe.authority.*
                                    (kept "authority" artifact name)

dev.knoe.auth wins: cluster runs from this name, the Maven artifact is
already knoe-auth.jar, and the broader rename is the documented
namespace direction (per ~/.claude/projects/-Users-chrisfu-dev-knoe-db/
memory/MEMORY.md). All of main's recent Phase 2 OIDC content was ported
from authority/src/.../dev/knoe/authority/ into
authority/src/.../dev/knoe/auth/ with package declarations rewritten.

== File-level resolution summary ==

Textual conflicts (4):

  authority/pom.xml
    - Took our artifactId="auth"
    - Took our branch's removal of spring-security-kerberos-client
      (verified: Junie's Phase 2 OIDC code does not import it; the dep
      was already-dead config)

  docs/pipeline-phases.md
    - Took our branch's "Phase 1 not started" status. Main had a
      misplaced "✅ Complete" with a knoe-auth-Phase-1 commit ref
      in the autobuild Phase 1 section — different domain.

  docs/plans/knoe-auth-round-1.md
    - Took our branch's dev.knoe.auth file table (vs main's
      dev.knoe.authority listing). Pure rename mismatch.

  supabase/helm/knoe-supabase/templates/kong/config.yaml
    - Took our branch's onboard route + plain dashboard wiring.
      Main had an oauth2proxy.enabled toggle that put oauth2-proxy as
      a Kong upstream — but the deployed architecture (commit 25f1b2e)
      has oauth2-proxy in FRONT of Kong, not behind. Main's wrapper
      reflected an architecture that was never deployed.
    - Took our branch's removal of basic-auth from dashboard route
      (queue #15 brief still tracks the matching values.yaml /
      kong/deployment.yaml cleanup).

Java tree reconciliation (44 file-pairs):

  20 dual-path source files + 2 dual-path tests
    Body-identical between main's authority/ and our branch's auth/
    after stripping package decls — main's commit 9daa94b was a pure
    rebrand. Took our branch's auth/ version for all 22.

  8 main-only source files (Phase 2 OIDC), ported into auth/:
    web/JwksController.java
    web/OidcAuthorizeController.java
    web/OidcDiscoveryController.java
    web/OidcTokenController.java
    web/OidcUserInfoController.java
    session/OidcCodeService.java
    session/OidcTokenService.java
    session/SessionService.java

  12 main-only test files, ported into auth/:
    HealthControllerTest.java
    enroll/EnrollValueTypesTest.java
    enroll/EnrollmentControllerTest.java
    enroll/TotpServiceTest.java
    kerberos/KadminClientTest.java
    kerberos/KerberosSpnegoResultTest.java
    web/LoginControllerTest.java
    admin/AdminControllerTest.java
    user/PrincipalNormalizerTest.java
    regression/IdentityRegressionTest.java
    session/OidcCodeServiceTest.java
    session/SessionServiceTest.java

  Port mechanics: read main:authority/...<file> via git show, then sed
  rewrite `package dev.knoe.authority` → `package dev.knoe.auth` and
  `import dev.knoe.authority` → `import dev.knoe.auth`. Body content
  unchanged.

  authority/src/main/java/dev/knoe/authority/ — DELETED (duplicate)
  authority/src/test/java/dev/knoe/authority/  — DELETED (duplicate)

== Verification ==

- grep -rln '<<<<<<<' across .java/.md/.yaml/.yml/.sh/.xml/.tpl: clean
- find authority/src -path '*/dev/knoe/authority*': empty (subtree gone)
- grep 'package dev.knoe.authority' across repo: clean
- bash -n install.sh deploy.sh etc/preflight_kubecontext.sh: clean
- git ls-files -u | wc -l: 0 unmerged paths
- helm lint supabase/helm/knoe-supabase: pre-existing failure on
  studioIngress.enabled undefined in values.yaml (introduced by Junie
  on main; unrelated to this merge — flagging as follow-up).

== Followups (carried into TODO ranked queue or noted here) ==

  - helm lint failure: studioIngress block in values.yaml is missing
    enable flag; templates/studio/{ingress,oauth2proxy-deployment,
    oauth2proxy-service}.yaml all reference studioIngress.enabled with
    no default. Pre-existing on main; not introduced by this merge.
  - The five Junie briefs filed on this branch are now reachable from
    main at docs/plans/junie/{02,06,07,13,15}-*.md. Junie can pick them
    up in any order.
  - knoe-auth Phase 2 OIDC source (now at dev.knoe.auth.*) is not yet
    deployed to the cluster. Deployment is its own task.
  - The branch claude/crazy-bose-fec256 stays in place (worktree at
    .claude/worktrees/crazy-bose-fec256 may have ongoing context for
    Claude Code sessions). Safe to delete once next session starts
    cleanly from main.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-01 16:39:10 -07:00

15 KiB

Phase A — TDD design brief for the onboarding system

For Junie in Sonnet mode. This is the design phase. You produce a design doc (docs/plans/onboarding-tdd.md) plus one hard test fixture. Phase B (Codex) consumes the design and writes the bulk of the test bodies. Do not write production-quality test files in Phase A — write the spec they'll be written from.


1. Why we're slowing down

Five commits landed in rapid succession over the last sitting, none with automated test coverage:

Commit Surface
f805404 oauth2-proxy --skip-auth-route for Supabase API surfaces (unifies db.0.knoe.dev for Studio + SDK)
79a3f76 GRANT anon, authenticated, service_role TO supabase_storage_admin; oauth2-proxy --pass/set-authorization-header removed; /support 302→mailto stop-gap
8c2956b pg_stat_statements moved from public to extensions schema (durable across all 4 deploy modes)
317b20b TCP LB pg.0.knoe.dev (34.106.156.196), CNPG cert SAN, knoe_developer role, chrisfu + ron engineer roles, pg_hba tightened
313ee88 Reusable engineer onboarding: etc/onboard_engineer.sh, knoe-onboard nginx pod, /onboard.html Kong route, membership-based pg_hba (+knoe_developer), committed CA cert

About to onboard the second human engineer (ron) against this stack. Standing decision was: don't add a user, even of one, before we have tests covering the surfaces a user touches. This brief is that decision being acted on.


2. What you produce in Phase A

Three artifacts, in order:

  1. The f-string fix at knoe/core/ops/cloudnative_pg.py:1372 committed first. 30 test-collection errors block the Phase 1 pipeline; until that lands, no test we write has anywhere to run. See top of docs/TODO.md. Trivial: extract field.replace('.', '\\.') to a variable before the f-string. One commit, message starts fix(test):.

  2. docs/plans/onboarding-tdd.md — the design doc. This is the main deliverable. It must be detailed enough that Codex (Sonnet's successor in Phase B, no shared memory) can write every test body from it without needing to make architectural calls. Sections required:

    • Test taxonomy (Tier 1 / Tier 2 / Tier 3) with one-line acceptance per test — not bash code, but a precise spec of what's being asserted.
    • Fixture architecture decision (k3d-per-run vs. shared persistent vs. hybrid) with the tradeoff analysis recorded.
    • Browser-test framework decision (Playwright vs. Puppeteer vs. ...) with why.
    • Directory layout under tests/ and how it slots into the existing pytest structure (read tests/test_render_supabase_hostname.py and tests/test_repair_update_and_supabase_flags.py for the conventions to match).
    • CI integration — these tests slot into Phase 1 of docs/pipeline-phases.md. Document the new run config(s), and whether the integration tier needs its own marker (e.g. pytest -m integration) so unit tests stay fast for local dev.
    • Definition of done per tier — what passes count as the boundary covered vs. work-in-progress.
  3. The first hard fixture, working. Specifically: a deterministic way to simulate "external client connecting to the cluster" inside CI. This is the single hardest design call in Phase A; everything in Tier 1 hangs off it. Get it functional and verified — even if it's just a 30-line conftest.py helper — so Codex can rely on it from day one. One commit, message starts test(infra):.

You do not write test bodies in Phase A. You write the spec and the hardest fixture. Resist the temptation to land a few "easy" tests — Codex gets a much cleaner working environment if Phase A's output is purely design + fixture.


3. Test taxonomy (the spec to flesh out)

This is what the conversation that produced this brief settled on. Use it as the skeleton for docs/plans/onboarding-tdd.md Section "Test taxonomy", but you're expected to refine it: add edge cases, kill duplicates, surface gaps.

Tier 1 — Security boundaries (must-have before more onboards)

The wall protecting the database from the public internet. No test = no proof. Hard-gating on the team's deploy pipeline.

  1. pg_hba rejection matrix. For each axis cross-product:

    • Source: external IP, internal cluster IP (10.0.0.0/8 simulated)
    • Encryption: TLS, plaintext
    • Role: chrisfu, ron, a fresh-onboarded knoe_developer member, postgres, supabase_admin, anon, a non-member of knoe_developer with valid LOGIN + password
    • The matrix has 36 cells minimum. Acceptance: each cell asserts the expected connect / reject / auth-fail outcome.
  2. +knoe_developer membership semantics is load-bearing. A role granted knoe_developer is reachable externally; revoking the grant immediately removes external reachability. A role with LOGIN + password but NOT in the group cannot connect externally even with valid creds. (This is the reusability lynchpin from commit 313ee88; a regression here means "everyone with a postgres password is reachable from the public internet".)

  3. oauth2-proxy --skip-auth-route regex tightness. For each declared route (/auth/v1/.*, /rest/v1/.*, /realtime/v1/.*, /storage/v1/.*, /functions/v1/.*, /graphql/v1/.*, /pg/.*, /onboard.html, /support):

    • The exact path skips Google sign-in.
    • Variant paths that should be gated (e.g. /api/profile, /Auth/v1/foo, double-slash, URL-encoded traversal probes) do get redirected to Google.
    • Each declared route is exercised with at least one negative test that would fail if the regex is loosened by accident.
  4. hostnossl reject precedence. A property test: any pg_hba rule order that places a permissive rule (e.g. host all all 10.0.0.0/8) before a hostnossl ... reject line, and would let an external plaintext connection match the permissive rule, must FAIL the test. This is the regression detector for the externalTrafficPolicy: Cluster -> Local bug we hit in Phase 1 — preserve the lesson.

Tier 2 — Onboarding flow correctness

CI-gating (block merges) but not deploy-gating.

  1. onboard_engineer.sh is idempotent. Run twice in a row → one role exists with the second run's password. --rotate on a non-existent user → clear error, no partial state. --revoke on a connected session → role dropped, sessions terminated cleanly.

  2. onboard_engineer.sh input validation. Username regex; email is *@knoey.com; rejection of shell-injection / SQL-injection probes; clear error messages.

  3. Reveal page renders correctly. Headless-browser tests:

    • onboard.html#user=test&pw=Zm9v&exp=2099-01-01T00:00:00Z → password decoded and shown.
    • exp in the past → expired-state UI.
    • Missing required fields → error UI.
    • Copy buttons populate clipboard.
    • On render, history.replaceState strips the fragment from the URL.
    • The pw is base64-decoded correctly even with URL-safe -_ chars.
  4. CA cert in repo matches live CA. SHA256 fingerprint of etc/knoe-db-ca.crt matches kubectl get secret knoe-db-ca. When CNPG rotates, this test fails loudly instead of engineers silently 502'ing.

Tier 3 — Adjacent surfaces we touched

Regression coverage. CI-gating.

  1. Studio internal paths still gate via Google. /, /api/profile, /api/database/* → 302 to accounts.google.com.
  2. db.0.knoe.dev/storage/v1/bucket reachable as service-role. The GRANT we landed in 79a3f76 makes this work; pin it.
  3. pg_stat_statements lives in extensions post-bootstrap. PG18 docker fixture for k3d/k3s/min; live-cluster check for gke. Database advisor's public.pg_stat_statements warning stays cleared.
  4. api.0.knoe.dev alias still works until docs/TODO.md follow-up #14 explicitly retires it.

4. Decisions you must make in Phase A

These are the architectural calls Codex needs nailed down before writing test bodies. Sonnet's reasoning bandwidth is concentrated here.

4a. Fixture architecture for "external client"

Tier 1 needs to assert "an external client cannot connect with X". Options:

Option Pros Cons
k3d cluster + sidecar pod with externalTrafficPolicy: Local LB Closest to prod (real LB IP, real pg_hba) Slow to spin up; LB IP allocation in k3d is non-trivial
k3d cluster + iptables rule simulating "external" via a labeled namespace's pod IP space Faster; deterministic Diverges from prod; risk that test passes but real LB doesn't
Live cluster (gke knoe-dev-cnpg-0) + ephemeral test roles Highest fidelity Pollutes prod with test roles; risk of leaving them around; can't run in CI without GCP creds
Hybrid: k3d for most assertions; one nightly job against live cluster for the fidelity-sensitive few Best of both Two test rigs to maintain

You decide. Document the tradeoff. Note that CLAUDE.md establishes k3d as the canonical local mode, so the bias should lean k3d unless you find a real reason otherwise.

4b. Browser-test framework

Tier 2.7 needs headless-browser support. Candidates: Playwright (Node), Puppeteer (Node), Selenium (multi-language), Cypress (Node, e2e-focused).

Read tests/ — current Python pytest-only. Adding a Node toolchain just for browser tests is a real cost. Consider:

  • Are there ~5 browser-tested behaviors total? Then a dedicated framework may be overkill — a pytest test that uses selenium (Python) + a headless Chrome may be enough.
  • Are there ~25? Dedicated framework worth the cost.

4c. k3d-per-run vs. shared persistent local cluster

Per-run Shared persistent
Speed Slow (60-90s spinup) Fast (~0s)
Determinism High Low (tests can pollute each other)
Local dev UX Friction Zero friction
CI Required for parallel runs Doesn't work for parallel

You decide. Docs/pipeline-phases.md would benefit from your call being recorded explicitly, since Phase 2 of that pipeline will face the same question.

4d. Acceptance criteria per tier

What counts as "Tier 1 done"? Specifically:

  • Coverage threshold (e.g. all 36 pg_hba cells; all 9+ skip-auth-route cases)?
  • Mutation testing pass rate?
  • Just "the spec items are all green"?

Document the answer per tier. Tier 1's bar should be the highest — it's the security wall.

4e. Retroactive coverage policy

The five commits (f805404 through 313ee88) landed without tests. We can:

  • Cover them all in this round. The Tier 1 + 2 + 3 spec already covers most of the surface area, but not all (e.g. pg_stat_statements schema move's behavior on a NEW deploy isn't directly tested by the spec; the storage GRANT's effect on bucket listing isn't directly tested).
  • Cover only the onboarding system in this round; queue the rest for follow-up.

I lean toward "cover them all" — the test debt only grows otherwise — but the call is yours given you can see the actual scope better than I drafted.


5. Files to read first

In order:

  1. docs/plans/README.md — the team's plan-doc conventions.
  2. docs/pipeline-phases.md — where these tests slot into the autobuild pipeline.
  3. docs/TODO.md — current queue and the f-string blocker you'll fix first.
  4. tests/test_render_supabase_hostname.py and tests/test_repair_update_and_supabase_flags.py — existing test patterns to follow.
  5. docs/db-access.md and docs/onboarding.md — what the onboarding system actually does.
  6. etc/onboard_engineer.sh — the script under test.
  7. deploy/gcp/gke/knoe-onboard.yaml — the static page (HTML embedded in the ConfigMap).
  8. deploy/gcp/gke/knoe-db.yaml — pg_hba block (the surface most needing test coverage).
  9. deploy/gcp/gke/oauth2-proxy-deployment.yaml — --skip-auth-route args.
  10. CLAUDE.md §"install.sh / deploy.sh pre-flight" — env-contamination warning is a category-mate of the boundary tests you're designing.

6. Definition of done for Phase A

When all four are true:

  • knoe/core/ops/cloudnative_pg.py:1372 f-string fix committed; pytest collection succeeds repo-wide (pytest --collect-only returns clean).
  • docs/plans/onboarding-tdd.md exists, addresses every section in Sec. 2.2 above, and is reviewable by chrisfu.
  • The first hard fixture (Sec. 4a's choice, implemented) is committed with at least one Tier 1 test that uses it green.
  • You have not committed bodies for any other tests. Codex's job.

7. Out of scope for Phase A (Codex picks up)

  • Writing test bodies for Tiers 1, 2, 3 (except the one Tier 1 test that validates the fixture from Sec. 4a is functional).
  • CI wiring beyond what's needed for the f-string fix.
  • Mutation-testing setup, if your acceptance criteria call for it.
  • Infrastructure for live-cluster integration tests (only relevant if your Sec. 4a decision calls for one).
  • Updating engineer-facing docs to mention the test suite. Codex updates docs/onboarding.md and docs/db-access.md after Phase B lands.

8. Handoff format to Phase B

Codex's brief (separate doc, will be drafted by chrisfu after this lands) will reference:

  • docs/plans/onboarding-tdd.md — the spec Codex implements.
  • The fixture you committed in Sec. 4a — Codex's tests use it.
  • Your acceptance criteria per tier — Codex knows when each tier is done.
  • The directory layout you defined — Codex's PRs land in the right places.

So docs/plans/onboarding-tdd.md needs to be prescriptive, not "considerations" or "options". Codex isn't going to make architectural calls; it'll execute against what you wrote. Land the design.


9. Working notes

  • You can ask chrisfu questions. If you hit a decision that genuinely needs human input (e.g. "should we let CI provision GCP credentials" — policy call, not technical), surface it rather than guessing. But the questions in Sec. 4 are yours to answer; don't punt them upward.
  • The plan in this brief is also wrong somewhere. The taxonomy is my best draft from a conversation; you may find that some Tier 1 items are redundant with Tier 2, or that there's a Tier 0 ("does the cluster even start?") missing. Calling that out IN the design doc is part of the deliverable.
  • Slow is smooth. This brief deliberately scopes Phase A to design + one fixture so the design isn't rushed. Resist scope creep into Phase B.

You're starting fresh in a Sonnet context. Read the files in Sec. 5, then the existing plans in docs/plans/, then come back here. Good design lands.