Skip to content

feat(chart): add trustBundle, and exercise real auth in the e2e sandbox job - #77

Merged
dcmcand merged 3 commits into
mainfrom
feat/chart-trust-bundle-real-auth-e2e
Aug 21, 2026
Merged

feat(chart): add trustBundle, and exercise real auth in the e2e sandbox job#77
dcmcand merged 3 commits into
mainfrom
feat/chart-trust-bundle-real-auth-e2e

Conversation

@dcmcand

@dcmcand dcmcand commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #76. Makes the e2e job test the OIDC path it has always skipped.

Why the job ran in dev mode

/readyz only turns 200 once the pod completes OIDC discovery against https://keycloak.nebari.local, and the sandbox gateway's certificate is signed by a self-signed CA the pod has no reason to trust. #76 fixed the other half of that blocker - the sandbox now resolves *.nebari.local from inside the cluster - leaving CA trust as the only thing standing in the way.

Why this goes in the chart

Trusting a private CA is a real deployment need, not a CI-only one: any Nebari behind a private CA or a TLS-terminating proxy has it. So rather than work around it in the workflow, the chart gains a values surface for it:

trustBundle:
  configMapName: ""
  key: ca-certificates.crt

When configMapName is set, the chart mounts that key read-only at /etc/ssl/nebari and points SSL_CERT_FILE at it. No Go change is needed - oidc.NewProvider uses Go's default pool, which honors that variable. Only the configured key is projected, so an unrelated entry in the ConfigMap cannot show up beside the bundle.

The ConfigMap must hold a complete bundle - public roots plus the private CA - because SSL_CERT_FILE replaces Go's default pool rather than extending it. NIC already produces exactly that when trust_bundle is configured: a nebari-trust-bundle ConfigMap, public roots included, projected into every namespace via trust-manager. Both values.yaml and the chart README say so, because a CA-only file is the easy mistake and it breaks TLS to anything publicly signed.

One targeted refactor came with it: three features now contribute volumes, and the gate was spelled out twice (once for volumeMounts, once for volumes). A third term repeated in both places is where that stops being readable, so the condition moved into a nebari-frames.hasVolumes helper.

Guards against a test that tests nothing

Two, both learned from #76:

  • The chart job now asserts on rendered content rather than only that a render succeeded. A silently-empty render would previously still pass kubeconform. It also asserts the feature stays opt-in - no SSL_CERT_FILE without trustBundle.configMapName.
  • The e2e job asserts the pod is really in real-auth mode: FRAMES_DEV_MODE absent, OIDC_ISSUER_URL present, SSL_CERT_FILE pointing at the mount. Dev mode also reports ready, so a quiet fallback to it would otherwise look completely green. That is precisely how v3's gateway-ip test only regex-checks the address, never connects to it action-nebari-sandbox#103 sat unnoticed for months.

Otherwise no new wait logic: Synced/Healthy already gates on the readiness probe, so it is itself the assertion that in-cluster DNS, CA trust, and the operator's client provisioning all work.

Scope

Deliberately readiness only. An authenticated API call is a natural next step but a separate piece of work: the operator sets DirectAccessGrantsEnabled: false on both clients it provisions, and frames validates an ID token whose aud must be the SPA client, so there is no simple grant to reach for. Getting a token means either scripting the authorization-code + PKCE flow or mutating Keycloak first - a real tradeoff that deserves its own design pass rather than being smuggled in here.

Known gap: the sandbox ConfigMap holds the gateway CA alone, not public-roots-plus-CA, so SSL_CERT_FILE narrows the pod's trust to that one CA. Harmless here - frames only talks to in-cluster Keycloak - but it means this job does not exercise the complete-bundle contract the chart documents.

Verification

Locally: helm lint, all four volume-gate combinations (including the no-volumes case the new helper has to preserve), the exact e2e value set rendering with OIDC_ISSUER_URL and SSL_CERT_FILE present and FRAMES_DEV_MODE absent, schema rejecting both an empty key and unknown properties, kubeconform on all three renders, plus make test and make lint (unchanged - no Go code touched).

The real verification is the e2e job going green with auth on, which is what CI here decides.

Also corrects AGENTS.md, which still described the sandbox as k3d after #76.

Records nebari-dev/nebari-infrastructure-core#612 next to the pool-pinning
step so whoever revisits this knows the workaround has an expiry: once NIC
fixes the InfraSettings/Deploy ordering, the step and the nic-config can
both go and the action's own config template can be used again.
The e2e job ran with auth.devMode=true, so the OIDC path was never
tested. The blocker was CA trust: the pod must complete OIDC discovery
against https://keycloak.nebari.local before /readyz turns 200, and the
sandbox gateway's certificate is signed by a self-signed CA the pod has
no reason to trust. The sandbox resolving *.nebari.local in-cluster
removed the other half of that blocker.

Trusting a private CA is a real deployment need, not a CI-only one - any
Nebari behind a private CA or a TLS-terminating proxy has it - so this
adds it to the chart rather than hacking around it in the workflow:

    trustBundle:
      configMapName: ""
      key: ca-certificates.crt

When configMapName is set the chart mounts that key read-only at
/etc/ssl/nebari and points SSL_CERT_FILE at it. No Go change is needed:
oidc.NewProvider uses the default pool, which honors that variable. Only
the configured key is projected, so an unrelated entry in the ConfigMap
cannot appear beside the bundle.

The ConfigMap must hold a COMPLETE bundle - public roots plus the private
CA - because SSL_CERT_FILE replaces Go's default pool rather than
extending it. NIC already produces exactly that on a cluster with
trust_bundle configured: a nebari-trust-bundle ConfigMap projected into
every namespace, public roots included. values.yaml and the chart README
both say so, since a CA-only file is the easy mistake here.

Three features now contribute volumes, and the gate was spelled out twice
(volumeMounts and volumes). A third term repeated in both places is where
that stops being readable, so the condition moves into a
nebari-frames.hasVolumes helper.

In the e2e job the sandbox CA is staged into the frames namespace from
the action's ca-cert-path output, which avoids copying it out of
kube-public (a ConfigMap cannot be mounted across namespaces) and lets
the namespace be created up front carrying the operator's opt-in label.
No new wait logic: Synced/Healthy already gates on the readiness probe,
so it is itself the assertion that DNS, CA trust, and the operator's
client provisioning all work.

Two deliberate guards against a test that passes without testing
anything. The chart job now asserts on rendered content rather than only
that a render succeeded, and the e2e job asserts the pod is really in
real-auth mode - FRAMES_DEV_MODE absent, OIDC_ISSUER_URL present,
SSL_CERT_FILE pointing at the mount. Dev mode also reports ready, so a
quiet fallback to it would otherwise look green. This is the failure mode
that let action-nebari-sandbox#103 sit unnoticed.

Note the sandbox ConfigMap holds the gateway CA alone, so the job does
not cover the complete-bundle case a production cluster uses.

Also corrects AGENTS.md, which still described the sandbox as k3d.
Real auth failed at DNS, before TLS was ever attempted:

  auth: OIDC discovery not ready; retrying
  error: ... lookup keycloak.nebari.local on 10.96.0.10:53: server misbehaving

The sandbox action's in-cluster DNS step is a no-op on kind. It writes a
`coredns-custom` ConfigMap, which is a k3s convention - k3s CoreDNS mounts
it and does `import /etc/coredns/custom/*.server`. kind's CoreDNS comes
from kubeadm with a plain `.:53` Corefile and no such import, so the
ConfigMap is created, CoreDNS is restarted, and the file is never read.
The step still logs "In-cluster DNS ready", which is what makes it hard
to spot. Filed as nebari-dev/action-nebari-sandbox#104.

Keycloak advertises the external issuer in its discovery document, so the
frames pod must resolve keycloak.nebari.local to complete discovery. Add
the zone to the Corefile kind actually reads, via a merge patch on just
that key so nothing else in the ConfigMap is disturbed, skipping when the
zone is already present so a re-run is not additive.

Then prove it, with a throwaway pod that resolves the name and must see
the gateway IP. Without that the failure surfaces fifteen minutes later
as an unexplained unhealthy Application - which is precisely how the
upstream no-op went unnoticed, since nothing in the action's suite ever
resolves a hostname from inside a pod.

This is the third workaround in this job for a v3 regression, all from the
same k3d-to-kind migration: the MetalLB pool (#612) and now DNS, plus the
gateway-ip test gap (#103) that let both ship green. Each carries the
issue reference and comes out when upstream lands.
@dcmcand

dcmcand commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

The first e2e run failed, and it turned out not to be the trust bundle - the pod never reached TLS at all:

auth: OIDC discovery not ready; retrying
issuer: https://keycloak.nebari.local/realms/nebari
error: ... dial tcp: lookup keycloak.nebari.local on 10.96.0.10:53: server misbehaving

DNS, not certificates. The sandbox action's in-cluster DNS step is a no-op on kind: it writes a coredns-custom ConfigMap, which is a k3s convention (k3s CoreDNS mounts it and does import /etc/coredns/custom/*.server). kind's CoreDNS comes from kubeadm with a plain .:53 Corefile and no such import, so the ConfigMap is created, CoreDNS is restarted, and the file is never read. The step still logs In-cluster DNS ready, ~10 minutes before the pod started getting SERVFAIL in the same run.

Filed upstream as nebari-dev/action-nebari-sandbox#104. Same blind spot as #103: both are features whose entire purpose is reachability from inside the cluster, and neither has a test that resolves or connects to anything from a pod.

Worked around here by adding the zone to the Corefile kind actually reads - a merge patch on just that key, skipped when the zone is already present - and then proving it with a throwaway pod that resolves the name and must see the gateway IP. Without that assertion the failure shows up fifteen minutes later as an unexplained unhealthy Application, which is exactly how the upstream no-op stayed invisible.

That makes three workarounds in this job for v3 regressions, all from the same k3d-to-kind migration: the MetalLB pool (nebari-dev/nebari-infrastructure-core#612), now DNS, plus the gateway-ip test gap that let both ship green. Each carries its issue reference in a comment and comes out when upstream lands.

Verified locally by extracting the rendered step out of the workflow YAML and executing it against a stubbed kubectl: the Corefile patch produces the right block with {{ .Name }} intact, is idempotent on a second application, and both failure paths (probe cannot resolve, gateway-ip empty) exit non-zero with a clear message rather than passing quietly.

Note the trust bundle is still unproven either way - the pod has never yet got far enough to attempt TLS. This run is the first real test of it.

@dcmcand
dcmcand merged commit 1c2f47d into main Aug 21, 2026
5 checks passed
@dcmcand
dcmcand deleted the feat/chart-trust-bundle-real-auth-e2e branch August 21, 2026 22:21
jbouder added a commit that referenced this pull request Aug 24, 2026
…truncation

Rebased onto main, which brought #72, #73, #76, and #77. The rebase produced no
git conflicts but two packages that did not compile, plus a migration collision
that stopped the server booting. Fixes those, the codec data-loss bug, and the
smaller items from the same review.

Blockers:

- Port the MCP write surface to the free-form body. `write.go` declared ten
  slot-named input fields against a `frames.Doc` that no longer has them. It now
  carries `body` and `template`: `body` because that is what a Frame's content
  is, and `template` because the reflective guard in `resources_test.go` asked
  for a decision and the answer is yes — without it `create_frame` cannot make a
  template at all, and an omitted-means-keep pointer stops `update_frame`
  de-listing one by accident. The guard now walks `frames.Doc` alone, since
  `SlotTable` is gone.

- Renumber `005_frame_is_template.sql` to 006. main's `005_canonical_membership_
  email.sql` claimed the same version, and goose rejects duplicates at provider
  construction, so no migration ran at all. Adds the missing `-- +goose Down`.
  `migrate_legacy_test.go` builds a schema as of 004 and did not carry the frames
  table, so 006's ALTER failed there as "no such table"; the fixture now carries
  every table a later migration touches, and asserts a pre-006 frame comes
  forward as not-a-template.

- Match the frontmatter delimiter only at column 0. `TrimSpace(line) == "---"`
  let an indented `---` inside a YAML block scalar close the frontmatter, which
  truncated the document and dropped every field after it — including documents
  the exporter itself produced, where the error named a field the author never
  touched. Table-driven cases cover the block scalar in both positions, a `---`
  in the body, and a full round trip through the codec's own output. The
  unqualified "lossless" claims are replaced with the two normalizations that
  actually happen.

Should-fix:

- `frame-yaml.ts` claimed to mirror `legacy.go` and did not: no two-space
  continuation indent, and `.trim()` where Go trims newlines only. That reaches
  storage, since restoring a legacy version re-serializes the TypeScript render
  as canonical content — one rule with a nested list became three flat rules,
  permanently. The port is now faithful, and both sides are pinned to one shared
  fixture, `testdata/legacy-slots/`, compared whole rather than by substring.
  Substring assertions over single-line values are what let the drift through.

Also:

- Pin the legacy-vs-`body:` precedence, which was silent and untested, and stop
  `Parse` naming the unexported `frames.docYAML` in unknown-key errors that reach
  API clients unwrapped. The error now lists the recognized keys.
- Cover `is_template` at the sqlite level: four SELECT lists and four Scan calls,
  none of them exercised by the service tests, which use `store.NewMemory()`.
  Asserts both the true and false case and the columns either side, so a
  scan-order mismatch fails.
- Guard the empty version in `compose.go`'s `Inherits from:` line, matching
  `MarshalMarkdown`.
- Give the one page-level heading that cannot use `PageHeader` the classes
  `PageHeader` exists to hold steady.
- Rewrite the parts of AGENTS.md and the three design docs that described the
  deleted slot design as current. The MCP doc's §3.4 gets the struck-through
  supersede treatment #73 set the precedent for on that same file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant