Uh oh!
There was an error while loading. Please reload this page.
test(seal-check): run the egress-enforcement probe live in k3d CI (#1184) - #541
Conversation
…184) RFC-0003 D12: the chart's enforcement probes ship as `helm.sh/hook: test` Jobs, but `helm test` ran nowhere in CI — SEAL-CHECK §8.4 recorded the k3s NetworkPolicy substrate as verified (#504) while the full-chart egress probe run stayed "pending". This closes that gap. - scripts/tests/e2e-seal-check.sh: install the local chart on a real k3d cluster with public images + the egress lockdown engaged (allowExternalHttps=false), then `helm test --filter` the egress-enforcement seal-check. Requires BOTH a zero exit AND the probe's `OK egress lockdown verified` marker in the logs — guarding the helm-test-`--filter`-matches-nothing silent-pass trap. - .github/workflows/helm-ci.yaml: a `seal-check-e2e` job mirroring upgrade-e2e (stock ubuntu runner, zero secrets — public curl vs 1.1.1.1). - client/tests/egress_enforcement_check_test.yaml: pin the probe Job's metadata.name so the e2e --filter can never silently drift off it. Local: shellcheck clean · helm template renders the probe Job · helm-unittest 27 suites / 320 tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Aug 3, 2026
bugbot run |
… 127) The sourced libs define install_kubectl/install_k3d/install_helm but do not call them; create_cluster + helm need the binaries on PATH first, and a stock runner has none preinstalled. Mirror e2e-auto-upgrade.sh's prerequisite block + the post-create node-ready wait. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Aug 3, 2026
bugbot run |
The egress-enforcement probe is a Job-type test hook with hook-delete-policy
hook-succeeded. `helm test --logs` looks up the pod by the Job's bare name,
but a Job's pod has a generated suffix ("pods not found"), and Helm deletes
the Job on success anyway — so --logs errored even though the probe passed.
Drive off the exit code instead (the probe exits 0 only when egress is
verified blocked), guard the --filter-silent-pass by asserting the hook is in
`helm get hooks` first, and dump the pod log via kubectl only on failure
(the Job persists when it fails).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>LukasWodka
commented
Aug 3, 2026
bugbot run |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Three Bugbot findings on the new e2e seal-check:
- Medium: set TRACEBLOC_NO_AUTOSTART=1 before create_cluster (like the sibling
e2e-*.sh) so it never mutates the host's Docker restart policy / runs
`systemctl enable docker`.
- Medium: add scripts/tests/e2e-seal-check.sh to helm-ci.yaml on.push/
on.pull_request paths so script-only edits re-trigger the k3d job.
- Low: enumerate the script in the installer-tests ShellCheck gate (both the
error and warning passes), matching the other e2e entrypoints.
The High finding ("Job logs never reach marker check") was already resolved in
the prior commit — the script drives off `helm test`'s exit code and no longer
greps --logs output.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>LukasWodka
commented
Aug 3, 2026
bugbot run |
…bot) `helm get hooks | grep -q` lets grep close the pipe on first match, SIGPIPE-ing helm mid-write; under set -o pipefail that false-fails the guard even when the hook exists. Capture helm output to a var, then grep a here-string — no pipe. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Aug 3, 2026
bugbot run |
Uh oh!
There was an error while loading. Please reload this page.
The script's own bounds (create_cluster up to 15m + helm test 360s + tool install + helm install) can exceed a 20m GHA cap on a slow cluster bring-up, false-failing even while each component is inside its own timeout. Match the sibling k3d job (upgrade-e2e = 30m). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Aug 3, 2026
bugbot run |
saqlainsyed007
left a comment
There was a problem hiding this comment.
Code review — the mechanism is well thought through (the helm test --filter silent-pass trap is genuinely closed by the hook-existence guard + the name-pin unittest, and driving off exit code rather than --logs is the right call for a Job hook). A few things worth addressing before this becomes a trusted gate, the first being a real soundness gap.
Also cleared while reviewing: the missing export USER vs. the sibling script is safe (every $USER ref in the sourced libs is guarded), and the new script correctly does not need a scripts/manifest.sha256 entry (test scripts aren't in gen-manifest.sh's FILES list).
One doc nit: the PR description says the script requires both a zero exit and the OK egress lockdown verified log marker — but the implemented script drives off the exit code only (its comments correctly explain why). The code is internally consistent; the description overstates it.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…aqlain)
- Positive control (blocking): before trusting a BLOCKED probe, prove the
cluster can REACH the host from a non-policied pod (in `default`, ungoverned
by the namespace-scoped training-egress policy). Without it, egress failing
for an unrelated reason (runner firewall / target outage / rate-limit) would
make the probe print OK and pass green while the NetworkPolicy did nothing.
A reachable positive + a blocked training pod = the block is attributable to
the policy. Positive-control failure now fails the seal-check as inconclusive.
- Bump enforcementProbeTimeoutSeconds 60s->240s (blocking): on a cold GHA
runner k3s can take >60s to program the pod iptables while the chart installs;
the probe is single-shot (backoffLimit 0), so 60s false-fails. 240s is well
inside the 360s helm-test budget.
- Add scripts/lib/** to helm-ci paths (blocking): the script sources
scripts/lib/{common,setup-linux,cluster,preflight}.sh, so a lib-only edit must
re-trigger seal-check-e2e + upgrade-e2e (both depend on it).
- Derive NS from CLUSTER_NAME so a CLUSTER_NAME override isolates a run under
one name instead of desyncing cluster vs release/namespace.
shellcheck clean; workflow parses.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>LukasWodka
commented
Aug 3, 2026
All five review points addressed in the latest push (positive control + 240s probe timeout + scripts/lib/** paths + NS derivation; prelude-dedup deferred to a follow-up), and the PR description corrected to match the exit-code-driven implementation. bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 08f169c. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…t (Bugbot) - Add --tlsv1.2 to the positive-control curl — the curl-tls house rule (the required quality/house-rules gate) rejects a curl that could negotiate a downgraded TLS version. - Wait for the default ServiceAccount before the positive-control kubectl run: a fast runner can schedule the pod before the SA exists, aborting under set -e with 'serviceaccount default not found' before the attribution message. - Add e2e-seal-check.sh to the standard-checks.yml Lint shellcheck list (the REQUIRED branch-protection gate) — installer-tests had it, the required Lint did not, so a shellcheck regression in the new script could miss the gate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Aug 3, 2026
Also fixed the |
saqlainsyed007
left a comment
There was a problem hiding this comment.
Approving — round-2 asks are all resolved and the seal-check is now sound.
I re-verified the mechanism end-to-end against the chart, not just the diff:
- The probe fails loudly when egress is NOT sealed.
templates/egress-enforcement-check.yamlexits 0 only on curl exit 7/28 (TCP connect refused/timed out), retries up to the timeout, and exits 1 with a loud banner if the connect ever succeeds (rc=0 or any TLS-layer code 35/52/56/60, all latched viareached=1) or if every attempt is DNS-only (exit 6). No swallowed timeout, no|| true, no grep-matches-nothing pass. The verdict is keyed on curl's exit code, not HTTP status. - No false-green path. For the gate to pass green, the positive control must Succeed AND
helm testmust exit 0; the probe Job exits 0 only when egress is blocked. An open egress makes the probe exit 1. Image-pull / timeout / DNS failures all fail safe (inconclusive), never green. - The
helm test --filtersilent-pass trap is genuinely closed by thehelm get hooksexistence guard (now a here-string, no SIGPIPE under pipefail) plus the name-pin unittest — belt and braces. - Netpol scoping confirms attribution.
network-policy-training.yamlis namespace-scoped to the release ns and selectstracebloc.io/workload=training; withallowExternalHttps=falserule 2 (0.0.0.0/0:443) is dropped, so the training pod's 1.1.1.1:443 is denied. The positive control runs indefault(ungoverned) — a genuine, non-tautological control that can fail if the runner/target blocks egress, exactly ruling out the environmental false-block the prior review flagged. clusterCidrs defaults (10/8, 172.16/12, 192.168/16) cover k3d's 10.42/10.43 CIDRs, so thefailguard won't trip on the credential-free install.
Round-2 items — all landed: positive control (added), enforcementProbeTimeoutSeconds=240 inner deadline, scripts/lib/** trigger paths, NS derived from CLUSTER_NAME, TRACEBLOC_NO_AUTOSTART=1, SA-wait race guard, 30m job timeout, required standard-checks Lint + installer-tests shellcheck lists. Prelude duplication was correctly deferred as an out-of-scope follow-up.
One non-blocking nit inline. shellcheck is reported clean; bash -n passes here.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Saqlain flagged (PR #541, thread on e2e-seal-check.sh:36) that the bring-up prelude is copy-pasted near-verbatim across scripts/tests/e2e-*.sh, and multiple Bugbot rounds have had to edit every copy in lockstep. Extract the two truly-identical, drift-prone blocks into scripts/tests/lib/e2e-common.sh: - e2e_isolate_env <name> — USER + CLUSTER_NAME default + TRACEBLOC_NO_AUTOSTART - e2e_install_prereqs — has docker + umask + install_{kubectl,k3d,helm} e2e-cluster / e2e-proxy / e2e-journey / e2e-auto-upgrade now source the lib and call these; each keeps its own CLUSTER_NAME default (passed as the arg) and its distinct logic. auto-upgrade keeps its extra `has jq` guard before the call. Deliberately NOT unified (would change behavior): the sub-lib `source` set (proxy/journey source 3 libs, not preflight — a pre-existing inconsistency, flagged not fixed), the cleanup/trap bodies (each reaps its own squid/work dirs), and CHART_DIR (only the chart-installing scripts). e2e-seal-check.sh (open on #541) adopts the lib as a fast-follow once both land — kept non-stacked. Added the lib to both shellcheck gates (installer-tests + the required standard-checks Lint). shellcheck --severity=error/warning clean; bash -n ok. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…550) Saqlain flagged (PR #541, thread on e2e-seal-check.sh:36) that the bring-up prelude is copy-pasted near-verbatim across scripts/tests/e2e-*.sh, and multiple Bugbot rounds have had to edit every copy in lockstep. Extract the two truly-identical, drift-prone blocks into scripts/tests/lib/e2e-common.sh: - e2e_isolate_env <name> — USER + CLUSTER_NAME default + TRACEBLOC_NO_AUTOSTART - e2e_install_prereqs — has docker + umask + install_{kubectl,k3d,helm} e2e-cluster / e2e-proxy / e2e-journey / e2e-auto-upgrade now source the lib and call these; each keeps its own CLUSTER_NAME default (passed as the arg) and its distinct logic. auto-upgrade keeps its extra `has jq` guard before the call. Deliberately NOT unified (would change behavior): the sub-lib `source` set (proxy/journey source 3 libs, not preflight — a pre-existing inconsistency, flagged not fixed), the cleanup/trap bodies (each reaps its own squid/work dirs), and CHART_DIR (only the chart-installing scripts). e2e-seal-check.sh (open on #541) adopts the lib as a fast-follow once both land — kept non-stacked. Added the lib to both shellcheck gates (installer-tests + the required standard-checks Lint). shellcheck --severity=error/warning clean; bash -n ok. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…ollow) (#566) Promised on the #541 review threads, now that #550 (the shared lib) and #541 (the seal-check script) are both on develop: - e2e-seal-check.sh now sources scripts/tests/lib/e2e-common.sh and uses e2e_isolate_env / e2e_install_prereqs, matching the other e2e-*.sh (drops the inlined isolation-env + install block). Keeps its own NS=$CLUSTER_NAME and local fail() for its assertions. - Harden the positive control (Saqlain nit): pin networkPolicy.training. enforcementProbeHost to a single $HOST var the install passes AND the positive control targets, so the probe and the control can never drift onto different hosts (was: HOST hardcoded while the probe used the chart default). shellcheck --severity=error/warning clean; helm template renders the probe Job with HOST=1.1.1.1 pinned. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Aug 3, 2026
/fr-pass |
…ow) (#598) * test(e2e): full seal suite vs the dev backend — backend#1184 deferred fast-follow New e2e-full-seal.sh installs the working-tree chart on real k3d as the dedicated dev e2e-test-agent (real credentials, CLIENT_ENV=dev), waits for every release PVC to Bind and jobs-manager to hold a real backend session, then runs helm test UNFILTERED — egress-enforcement + backend-reachability + storage-assertions in one release, hook-presence-guarded so a regated check can't vanish silently. New helm-ci job full-seal-e2e runs it on push/dispatch only and skips green with a notice until the e2e-test-agent secrets (TB_E2E_CLIENT_ID / TB_E2E_CLIENT_PASSWORD) are provisioned. The egress positive control moves verbatim into e2e-common.sh (one copy, shared with e2e-seal-check.sh). SEAL-CHECK.md gains the CI-coverage map and drops three follow-ups that shipped since (#541 live probe, cli#393 verdict, cli#449 matrix). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(e2e): address Bugbot findings on the full-seal harness - credentials travel in a mode-0600 temp values file, never on argv (process-list exposure on a shared runner + helm --set comma/brace mangling); removed on every exit path — installer parity - the PVC wait does one guarded fetch per iteration, so a transient kubectl failure retries until the deadline instead of aborting under set -euo pipefail - full-seal-e2e job timeout 30m -> 45m: the script stacks a 300s PVC wait, two 300s rollouts and a 600s unfiltered helm test on create_cluster's 15m bound — GHA must not kill a slow-but-healthy run Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(e2e): local-path provisions RWO only — pin pvcAccessMode on the full-seal install (Bugbot) The chart's PVC default is ReadWriteMany; rancher.io/local-path never provisions it, so every claim sat Pending and both the Bound pre-wait and storage-assertions were guaranteed to fail once the job activates. The installer writes pvcAccessMode: ReadWriteOnce for this exact storage path — the harness now installs with the same value. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

What & why — RFC-0003 D12 seal check (backend#1184)
The chart already ships its enforcement probes as
helm.sh/hook: testJobs (egress-enforcement, backend-reachability, storage-assertions), buthelm testruns nowhere in CI — so the probes have never actually executed in a pipeline. SEAL-CHECK §8.4 records the k3s NetworkPolicy substrate as verified (#504, a manual throwaway-cluster check) while the full-chart egress-enforcement probe run is still marked pending.This PR makes that probe run for real, on every CI push.
Changes
scripts/tests/e2e-seal-check.sh— installs the local working-tree chart on a real k3d cluster (reusing the installer'screate_clusterpath) with public images and the egress lockdown engaged (allowExternalHttps=false), then runs only the egress-enforcement seal-check viahelm test --filter. A training-labelled pod's direct TCP egress to1.1.1.1:443must be blocked by k3s's CNI..github/workflows/helm-ci.yaml— aseal-check-e2ejob mirroringupgrade-e2e: stockubuntu-latest, zero secrets (publiccurlimage, no backend, no private images).client/tests/egress_enforcement_check_test.yaml— pins the probe Job'smetadata.nameso the e2e--filtercan never silently drift off it.The subtleties worth a reviewer's eye
helm test --filterexits 0 when the filter matches nothing — a silent false-pass. The script guards this by asserting the probe hook is inhelm get hooksbefore testing, then drives the verdict offhelm test's exit code (the probe Job exits 0 only when egress is verified blocked). It does not grep--logs(unreliable for Job hooks). The name-pin unittest is belt-and-braces on the filter name.Why only egress-enforcement
It's the single seal-check that needs no credentials.
backend-reachabilityneeds a real backend + proxy, and a bound-PVCstorage-assertionspass needs the dev harness — both belong to the dev-environment e2e run (the remaining #1184 subtask), not a stock runner.Verified locally
shellcheckclean ·helm templaterenders the probe Job as<release>-egress-enforcement-check·helm unittest27 suites / 320 tests green. The live k3d run is exercised by the new CI job itself (like the other e2e scripts, it isn't runnable on a dev laptop without docker+k3d).Notes
develop.🤖 Generated with Claude Code
Note
Low Risk
Changes are CI, test automation, and a helm-unittest assertion; they do not alter runtime chart behavior or production install paths.
Overview
Adds live CI coverage for the chart’s egress-enforcement
helm testhook, closing the SEAL-CHECK gap where probes existed buthelm testnever ran in the pipeline.A new
scripts/tests/e2e-seal-check.shbrings up an isolated k3d cluster, installs the local chart withallowExternalHttps=false(and a longer enforcement probe timeout for slow CNI reconcile on GHA), runs a positive control curl from an unpolicy’d pod to1.1.1.1, then asserts the probe hook is present and runshelm test --filteron<release>-egress-enforcement-check, failing on exit code (not vacuous filter matches or unreliable--logs)..github/workflows/helm-ci.yamlgains aseal-check-e2ejob (30m timeout, no secrets). Path filters and shellcheck lists in helm-ci, installer-tests, and standard-checks include the new script (andscripts/lib/**for helm-ci triggers).client/tests/egress_enforcement_check_test.yamlpins the probe Jobmetadata.nametoRELEASE-NAME-egress-enforcement-checkso the e2e filter cannot silently match nothing and pass.Reviewed by Cursor Bugbot for commit 068bd9f. Bugbot is set up for automated code reviews on this repo. Configure here.