2026-05-15 — Universe Onboarding PR1

Resumed /plans/universe-onboarding.md at PR1 (Broker config foundations: WorldConfig.PublicURL).

Changes

• tools/demarkus-broker/internal/broker/config.go — added WorldConfig.PublicURL string with a doc comment explaining the field belongs on the broker (single source of truth for the client-facing address), is optional, and that a blank value means "skip in /me/install."
• deploy/helm/demarkus-broker/values.yaml — documented publicURL in the commented worlds[] example with the rationale for setting it.
• deploy/helm/demarkus-broker/templates/secret-config.yaml — render publicURL: "<value-or-empty>" for every world. Always emitted so the YAML shape is stable across set/unset, and operators can grep for the field.
• tools/demarkus-broker/internal/broker/config_test.go — two new subtests: empty default round-trip + explicit mark:// value round-trip.
• deploy/helm/demarkus-broker/tests/secret-config_test.yaml — two new cases: blank renders as publicURL: "", operator-supplied value renders verbatim.

Verification

• go test ./internal/broker/ -run TestLoadConfig → 29/29 pass (2 new).
• helm unittest -f tests/secret-config_test.yaml . → 13/13 pass (2 new).
• bash pre-commit.sh → format, vet, lint clean across protocol/server/client/tools.

Scope discipline

No validation in PR1 — plan calls for "optional; validation only requires it when used." Shape validation (mark:// URL, port present, etc.) lives with the /me/install consumer in PR5.

KnownFields(true) means older broker binaries would reject the new chart's rendered config because of the unknown publicURL line. Chart appVersion gates broker upgrade as usual; not new for this PR.

Next

PR2 — broker discovery doc (GET /.well-known/openid-configuration proxying the IdP discovery with device-endpoint overrides). Resume at tools/demarkus-broker/internal/broker/discovery.go (new file) + route registration in server.go.

Pending review: should PR1 ship as a standalone PR or be folded into PR2? Plan calls for independent reviewability per PR, so default is standalone.

PR2 — Broker discovery doc

PR1 (#135) merged. Continuing through plan PR2: broker-hosted /.well-known/openid-configuration that proxies the IdP's discovery doc with broker-hosted endpoint overrides.

Changes

• tools/demarkus-broker/internal/broker/discovery.go (new) — Discovery type. Fetches <idpIssuer>/.well-known/openid-configuration once at NewDiscovery, parses into map[string]any so unknown fields proxy through unchanged, rewrites issuer + device_authorization_endpoint + token_endpoint to broker URLs, caches the rendered body with a 5-minute TTL. Lazy refresh on TTL expiry, stale-while-error on refresh failure. Constructor blocks on initial fetch so a misconfigured/unreachable IdP fails the broker at boot rather than at first /soul-join.
• tools/demarkus-broker/internal/broker/discovery_test.go (new) — 13 test cases via httptest fake-IdP: constructor validation (broker URL + issuer required), upstream-failure failure paths, override correctness, proxy of jwks_uri + userinfo_endpoint + authorization_endpoint + arrays, response headers, cache hit count within TTL, refresh after TTL, stale-while-error, recovery from transient failure, trailing-slash trim, malformed JSON rejection.
• tools/demarkus-broker/internal/broker/server.go — Server gains a discovery *Discovery field and NewServer takes it as a constructor arg. Routes() registers GET /.well-known/openid-configuration only when discovery != nil, so existing tests pass nil and skip the route cleanly.
• tools/demarkus-broker/internal/broker/server_test.go — TestWellKnownDiscoveryRouteRegistered (wires real Discovery into NewServer, asserts override applied through the mounted route) + TestWellKnownDiscoveryRouteSkippedWhenNil (404 when constructed nil).
• tools/demarkus-broker/main.go — NewDiscovery called between NewVerifier and NewServer; passes cfg.Server.PublicURL + cfg.OIDC.Issuer. Failure is fatal at startup, same shape as NewVerifier.
• tools/demarkus-broker/internal/broker/config.go — new ServerConfig.PublicURL field, required at load, trailing slash stripped once at load so every downstream consumer sees the canonical form. Doc comment notes it intentionally diverges from OIDC.RedirectURL (which is purpose-specific to the IdP redirect).
• tools/demarkus-broker/internal/broker/config_test.go — 2 new subtests: required-rejection + trailing-slash trim.
• deploy/helm/demarkus-broker/values.yaml — server.publicURL: "" with full operator-facing comment.
• deploy/helm/demarkus-broker/templates/secret-config.yaml — renders publicURL: {{ required ... }} so chart-render fails clearly when omitted (same shape as oidc.redirectURL).
• deploy/helm/demarkus-broker/tests/secret-config_test.yaml — 2 new cases (renders value, fail-render when blank) + added server.publicURL to the suite-wide set: block.
• deploy/helm/demarkus-broker/tests/deployment_test.yaml — same suite-wide set: addition (this suite also templates secret-config).
• deploy/kind/values-broker.yaml, deploy/kind/values-broker-argo.yaml — set server.publicURL: "http://localhost:8080" to mirror the existing redirectURL shape. Stage 2/4 harnesses don't exercise the well-known route but the broker's LoadConfig now requires the field.

Verification

• go test ./internal/broker/ -count=1 → all green (4.3s). 13 new Discovery tests + 2 new server route tests + 2 new config tests.
• helm unittest . → 55/55 pass across all 7 suites (was 52/52 before; 3 new).
• bash pre-commit.sh → format + vet + lint clean.

Design notes (worth remembering)

• Override shape is intentionally lossy at the seam. Override issuer to broker URL but leave jwks_uri pointed at the IdP. ID tokens during PR2/PR3 are still IdP-signed and carry IdP's iss. Strict client validation against this discovery doc would fail; the plugin's device-flow client (PR6) only consumes device_authorization_endpoint + token_endpoint from this doc so the mismatch is invisible to it. PR4 closes the gap by minting broker-signed id_tokens with a broker-hosted jwks_uri. Doc comment in discovery.go calls this out explicitly so a future reader doesn't trip over it.
• map[string]any proxy beats a typed struct. Tried a typed idpDiscovery struct first; killed it because real IdPs ship 20+ fields each (Google's code_challenge_methods_supported, Auth0's mfa_challenge_endpoint, Okta's request_object_signing_alg_values_supported, etc.) and the broker has no business knowing or filtering them. Decode → mutate three keys → re-encode is the right shape; tests assert both the overridden fields AND a couple proxy fields so regressions surface.
• Lazy refresh, not background goroutine. Plan §Risks talks about "5-minute TTL on the discovery cache." Two implementation shapes: background ticker, or in-handler lazy. Picked lazy because (a) one fewer goroutine lifecycle to wire into the broker shutdown path, (b) the load is human-scale — even with N replicas behind a Service, the per-instance refresh rate caps at 1/5min, (c) thundering herd on simultaneous expiry is negligible at this rate (two replicas might both refresh once; the IdP doesn't notice). Stale-while-error keeps the well-known endpoint available during transient upstream blips.
• Constructor signature pattern. NewDiscovery takes a DiscoveryConfig struct, not a long positional arg list — per /guidelines.md "more than 4 parameters is a smell." Same pattern as the existing Issuer/Verifier constructors take typed configs.
• Test isolation gotcha. Existing oidc_test.go already had fakeIdP + newFakeIdP. Renamed mine to fakeDiscoveryIdP + newFakeDiscoveryIdP rather than touch the existing symbol — both are test-package-scope so the rename is purely local. Worth keeping in mind if a third test file ever needs a fake IdP: extract to a shared testhelpers_test.go helper instead of accumulating per-suite copies.
• http.NoBody, not nil, on GET requests. gocritic flagged the nil body on http.NewRequestWithContext; switched to http.NoBody (the idiomatic singleton for empty request bodies). Same fix in tests using httptest.NewRequest.

Scope discipline

PR2 spec called for ~150 lines + tests. Final tally: ~210 lines of production code (discovery.go) + ~310 lines of tests + ~70 lines of config/chart plumbing + ~50 lines of dev-values updates. Over the 150-line estimate but every piece is load-bearing for PR3 (verification_uri uses brokerURL) and PR4 (broker-signed tokens use brokerURL as issuer).

Next

PR3 — broker device flow (RFC 8628). New file device.go with /device/authorize + /device (HTML form + /auth/login redirect glue) + /device/token polling. ~600 lines + tests, ~2 days. Will exercise the route registration path that PR2 sets up; will also be the first time the broker holds short-lived per-flow state in-memory (device_code map with janitor goroutine).

Open question to resolve before PR3

mock-oauth2-server device-code support — plan §Open Questions item 1. Need to check navikt/mock-oauth2-server v2.x source for /device_authorization endpoint before designing the kind Stage 5 harness in PR7. Not blocking PR3 itself (broker-side tests use a fake verifier), but if mock-oauth2-server doesn't support it, the kind end-to-end story diverges.

End of session — PR2 merged, PR3 plan written

• PR2 (#136, broker discovery doc) merged after two rounds of CodeRabbit feedback. Final state included Server.PublicURL config field (required, trimmed, absolute-URL validated), Discovery type with 5-min TTL caching + refresh-mutex stampede protection + hard fetch deadline, route registration on /.well-known/openid-configuration, 16 broker tests + 5 helm-unittest cases.
• Coderabbit lessons worth saving:
  • t.Fatalf inside spawned goroutines panics that goroutine without releasing pending channel sends. Concurrency tests need an error-returning helper that surfaces failures through the channel; the main goroutine asserts after <-. Wrote serveAndDecodeErr as the canonical pattern for this kind of test.
  • When a constructor accepts an injected *http.Client via config struct, that client may not have a Timeout set. Wrap the fetch context with context.WithTimeout inside the operation so the deadline holds regardless of what the caller injected. Belt-and-suspenders on top of the default client's own timeout.
  • helm install regression tests in CI (test-upgrade-wipe.sh invocation) need explicit --set for every newly-required chart value. Adding a required field to values.yaml without updating CI passes helm-unittest but fails the actual install regression. Worth grepping .github/workflows/ for helm install calls when adding a required field to any chart.
  • README install examples count as a test surface — coderabbit flagged the missing --set server.publicURL in the dev-install README block. Update README install commands alongside the chart change.
• PR3 plan published to /plans/universe-onboarding-pr3.md with full architecture, sub-task breakdown, open design questions, and tomorrow-morning resume steps. Index updated to surface it.
• Two design questions deferred to tomorrow's session:
  1. Verifier.Exchange signature change (lean: change it, three call sites, clean refactor) vs. parallel ExchangeWithTokens method. Latter is messier and we'd refactor anyway in PR4.
  2. RFC 8628 slow_down interval-increment policy. Lean: not for PR3 (RFC says MAY, not MUST); revisit if a customer reports IdP abuse.
• Tomorrow's first move: mark_fetch /plans/universe-onboarding-pr3.md, confirm the two open questions, branch from main, start at Step 1 (Verifier refactor) as its own commit.