release-train: develop -> staging - #542
Merged
Merged
Conversation
The caller's header described a "FR on dev" -> "Ready for staging" transition that does not exist. RFC-BACKEND-1405 D6 retired the dev-side review -- "On dev" is set automatically when the train merges to develop -- so /fr-pass applies at exactly one gate: FR on staging -> Ready for prod. The reusable workflow it calls has only ever implemented the staging hop and documents that correctly at the top of the file, so this was the comment drifting away from the code, not a behaviour change. Comment-only; no logic touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…lling
The template job installed kubeconform with:
curl -sSL .../releases/latest/download/kubeconform-linux-amd64.tar.gz \
| tar xz -C /usr/local/bin
Two independent problems. `releases/latest` is a mutable pointer, so the
binary this job executed changed whenever upstream cut a release — we
were not pinned to anything. And piping curl into tar extracts the bytes
as they arrive, so there was no moment at which a digest could have been
checked even if we had one; the archive was unpacked onto PATH first and
inspected never.
What could previously execute unreviewed: whatever those bytes happened
to be. A replaced release asset, a compromised upstream account, or a
MITM on the download would land an executable in /usr/local/bin and the
next step ran it. This job runs on the matrix for all four platforms, so
it happened four times per CI run.
What now cannot: the download is pinned to an explicit version, written
to a temp file, verified against a digest pinned in the workflow, and
only installed once it matches. Bytes that do not match the digest never
become an executable on PATH.
Follows the pattern already used in tracebloc/.github's actionlint.yml
(version + SHA-256 + `sha256sum -c`).
kubeconform 0.8.0, kubeconform-linux-amd64.tar.gz
sha256 9bc2bffbf71f261128533edaf912153948b7ff238f9a531ae6d34466ec287883
source: the release's own CHECKSUMS asset (not computed from a download)
Version pin is behaviour-preserving: `releases/latest` currently
redirects to v0.8.0.
The digest is asserted to be 64 hex characters before use. `sha256sum -c`
treats a malformed line as "no properly formatted checksum lines found",
and whether that exits non-zero depends on the coreutils build — so an
empty or truncated variable could otherwise verify nothing while the step
still went green. Every failure mode was tested on ubuntu:22.04 (GNU
coreutils 8.32): digest mismatch, empty digest, truncated digest,
uppercase digest, HTTP 404, and a real substitution (pinned v0.8.0 digest
against the v0.7.0 asset). All exit non-zero and leave no binary on PATH.
Refs tracebloc/backend#1426
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…RFC D3/D4) (#528) * feat(#435): single-source cross-OS installer facts + CI parity gate (RFC D3/D4) The costliest drift class of the installer sweep was FACTS diverging between the three OS implementations — the #410 incident (k3d/helm pins bumped in bash #382 but not PowerShell #410) failed a real customer install. Copy already had the byte-exact catalog; behavior facts get the same treatment here. - scripts/spec/facts.env: the single source of truth for cross-OS facts. Tool version pins (K3D_VERSION / HELM_VERSION / K8S_VERSION) + the READY_TIMEOUT budget. - scripts/check-facts.sh: --write stamps the spec into every consumer (bash common.sh + summary.sh, PowerShell install-k8s.ps1); --check is the CI gate (mirrors gen-manifest's write/check split). Nothing is sourced at runtime — consumers carry literal values, so the single-file verified bootstrap (R8) is untouched. - CI: installer-tests.yaml runs `check-facts.sh --check`, failing the PR if any consumer drifted from the spec — so the #410 incident (a pin in one OS path but not the other) can no longer ship. install-k8s.ps1's lockstep comment updated to point at the spec. Tests: scripts/tests/check-facts.bats — the #410 incident reproduced as a red check (both directions), --write round-trips for versions + the timeout, fail-closed on a missing pattern, bad-mode rejection. Scope note: hosts are already single-sourced + drift-checked (check-drift Checks 1 & 5); memory floors (bash-only today) and the behavior-parity matrix are follow-ups on this same mechanism — a new fact is one row in facts.env + check-facts.sh. Closes #435 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#435): enforce K8S_VERSION in PowerShell too + shellcheck check-facts.sh (Bugbot) Two Bugbot findings: 1. (High) install-k8s.ps1 ALSO pins K8S_VERSION ($K8S_VERSION default, passed to k3d as --image rancher/k3s:$K8S_VERSION), but check-facts treated the pin as bash-only — so bumping the spec + --write updated only common.sh, Windows stayed stale, and --check stayed green: the exact #410 hole for this pin. Added install-k8s.ps1 as a K8S_VERSION consumer (extract + rewrite), updated the facts.env note. 2. (Low) scripts/check-facts.sh was wired for --check but not in the explicit shellcheck --severity=error file list (unlike gen-manifest.sh) — a regression in the facts gate wouldn't fail static CI. Added it to both shellcheck lines. Tests: K8S_VERSION drift in PowerShell -> RED; a K8S bump stamps BOTH consumers. 11/11. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…unner (RFC D5) (#540) * feat(#436): real Windows installer e2e on a self-hosted nested-virt runner (RFC D5) GitHub's windows-latest runners can't nest virtualization, so install-k8s.ps1 had only mocked Pester while the bash installer gets a 9-distro prereq matrix + real-k3d e2e on every push. This adds the missing Windows e2e leg (RFC-CLIENT-0003 D5), on a self-hosted runner that DOES support nested virt. - .github/workflows/windows-e2e.yaml: schedule-only (nightly) + workflow_dispatch, NOT per-PR; runs-on [self-hosted, windows, nested-virt]; concurrency-guarded; 45-min cap; uploads the install log as an artifact on failure; tears the cluster down every run. - scripts/tests/e2e-windows.ps1: the credential-free driver, mirroring e2e-journey.sh — dot-sources install-k8s.ps1 with TB_PESTER=1 (main() does not run), verifies Docker/WSL are up (runner prereqs; it does NOT reinstall Docker Desktop per run), installs the tools, runs the installer's real New-K3dCluster, applies a credential-free stub the CLI discovery keys off (labels + the ingestor SA), asserts the discovery-shaped state, and checks the installer's cluster-create copy landed in the log. Stops before Invoke-ProvisionClient (Steps 5-6 mint a real backend credential — out of scope), exactly as e2e-journey stops before the CLI connects. - docs/WINDOWS-E2E.md: one-time runner setup (labels, WSL2 + Docker Desktop + nested virt, runner account rights) + what green/red means. - installer-tests.yaml: lint e2e-windows.ps1 with PSScriptAnalyzer on every push so a syntax/verb regression fails fast, not at the nightly run. Chose the self-hosted-runner route (Azure isn't available to us). Validated here: YAML parses, PSScriptAnalyzer clean (0 errors; the Write-Host/empty-catch/unicode warnings match install-k8s.ps1's own tolerated set), check-style clean. It CANNOT be validated off a nested-virt Windows host — the first scheduled run on the registered runner shakes it out, which is exactly the issue's acceptance (a broken installer commit turns it red with the install log attached). Closes #436 * fix(#436): admin assert + non-vacuous copy check + bounded kubectl in the e2e driver (Bugbot) Three Bugbot findings on the Windows e2e driver: 1. (High) TB_PESTER=1 skips the installer's self-elevation gate, but Initialize-ToolDir creates %ProgramFiles%\tracebloc\bin and writes the Machine PATH — admin-only. The docs wrongly said a non-admin account suffices. Assert Administrator up front (fail fast with a pointer) and correct docs/WINDOWS-E2E.md: the runner MUST run elevated (the Windows installer is inherently admin). 2. (Medium) The install-log copy assertion was SKIPPED when LOG_FILE was missing, yet the script still printed PASS — a vacuous seal. Now a missing log fails (never report PASS on an unverified check). 3. (Medium) Several kubectl calls (wait/create/apply/get) had no --request-timeout, so a wedged API server would hang to the 45-min job cap. Added --request-timeout=30s to each (the PowerShell analog of the bash journey's watchdog). PSScriptAnalyzer: 0 errors (the remaining warnings match install-k8s.ps1's tolerated set); YAML valid; check-style clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#436): bound docker info + teardown, and assert the FULL discovery selector (Bugbot r2) Two more Bugbot findings on the e2e driver: 1. (High) `docker info` and the finally `k3d cluster delete` had no deadline, so a wedged Docker/WSL or a stuck delete would hang the shared runner to the 45-min job cap — undercutting the --request-timeout hardening already on every kubectl call. Added an Invoke-Bounded helper (Start-Process + the installer's killing Wait-ProcessWithDeadline) and ran both through it (30s / 120s). 2. (Medium) The discovery assertion only filtered app.kubernetes.io/name=client, so a stub missing managed-by=Helm or a non -jobs-manager name still reached PASS. Now it matches the FULL selector DiscoverParentRelease uses: name=client AND managed-by=Helm, AND the Deployment name ends in -jobs-manager. PSScriptAnalyzer 0 errors; warnings unchanged (match install-k8s.ps1's tolerated set). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#436): fresh-cluster pre-clean, log survives timeout-cancel, bounded safety-net (Bugbot r3) Three findings from the persistent-runner reality: 1. (High) A leftover tbe2ewin cluster sends New-K3dCluster down its reuse path (which still logs 'Creating k3d cluster'), so the copy check + PASS could succeed WITHOUT a real create. Pre-clean any stale cluster (bounded) before New-K3dCluster so every run genuinely creates one. 2. (High) The install-log upload was gated on failure() only, but a timeout-minutes hit CANCELS the job (failure() is false), and the always() teardown then wiped the log the acceptance criteria need. Upload on !success() (failed OR cancelled/timed out), before the teardown. 3. (Medium) The workflow safety-net k3d delete was unbounded. Bound it with Start-Process + Wait-Process -Timeout 120 (+ force-kill), matching the driver's Invoke-Bounded. YAML valid; PSScriptAnalyzer 0 errors; check-style clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#436): put the installer tool dir on GITHUB_PATH so teardown's k3d resolves (Bugbot r4) (High) The always-on safety-net teardown runs bare `k3d` in a fresh step process, but Initialize-ToolDir writes the MACHINE PATH — which a subsequent step doesn't pick up mid-job — so k3d could be unresolved there and, when the driver was killed before its finally, the tbe2ewin cluster would leak on the shared runner. Prepend %ProgramFiles%\tracebloc\bin to GITHUB_PATH in the setup step so every later step resolves k3d (the driver's own calls already resolve via RefreshPath in-process). YAML valid. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
docs(ci): /fr-pass advances staging → prod only (backend#1405 D6)
security(ci): verify kubeconform against a pinned digest before installing
…#199-1) (#537) Test-only, no behavior change — the regression net for the egress-lockdown default flip (#199-2). The egress mechanism is fully shipped (client-runtime#102 / #378); #199 is just flipping allowExternalHttps to false by default, which drops the rule-2 external-HTTPS hole and shifts the remaining egress indices. - Pin allowExternalHttps: true in the three cases that assert the rule-2 hole or the MySQL rule at a rule-2-dependent index (external-443, MySQL, OpenShift), so they keep passing once the default flips. - Add two locked-down cases (allowExternalHttps: false): the 0.0.0.0/0 rule is gone (MySQL shifts to egress[1]), the UNCONDITIONAL requests-proxy egress (8888) survives so pods can still POST results/FLOPs, and the egress-proxy allowlist rule (3128) renders when egressProxy.enabled. 13 network-policy tests (was 11), 322 helm suite green. Lands first so #199-2 is a green-to-green flip. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Author
|
bugbot run |
tracebloc-release-train
Bot
added
gate-nudge
…PASS pre-clean (Bugbot #542) (#544) check-facts.sh: _spec_get and _extract piped `sed -n … | head -1` under `set -o pipefail`. On a second match large enough to fill the pipe buffer, head closes after line 1, sed takes SIGPIPE, and the pipeline exits 141. In _extract the pipeline is the function's terminal command, so that 141 aborts the facts gate before any drift message prints — a crash / fail-open on duplicate input. Both helpers now capture the whole output and take the first line with `${all%%$'\n'*}` (the repo's gpu-nvidia.sh idiom), leaving no pipe to break. Empty (no-match) results still yield empty, so callers are unchanged. e2e-windows.ps1: the pre-clean `k3d cluster delete` ran via `Invoke-Bounded … | Out-Null`, discarding its exit code. On the persistent Windows runner a timed-out (124) or failed delete leaves the tbe2ewin cluster in place; New-K3dCluster then takes its REUSE path, which still logs "Creating k3d cluster", so the copy check + PASS succeed WITHOUT a real create — the exact false-green the pre-clean was added to prevent. The pre-clean now gates on the exit code (k3d delete is idempotent → 0 when absent) and Stop-E2e's the run on non-zero, so a stale cluster can never fall through. Tests: two bats cases in check-facts.bats feed duplicate matches past the pipe buffer and assert --check still passes (the _extract case reproduced exit 141 on the pre-fix code). e2e-windows.ps1 has no Pester suite (it's an integration driver dot-sourcing the real installer against live Docker/k3d), so it relies on the source-level guarantee; PSScriptAnalyzer lints it in CI and still passes. Both findings surfaced by Cursor Bugbot on the release-train PR client#542. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
tracebloc-release-train
Bot
added
gate-nudge
LukasWodka
added a commit
that referenced
this pull request
Aug 3, 2026
Bugbot, release-train staging hop #542, High. The always() safety-net teardown ran k3d cluster delete via Start-Process with no -ErrorAction Stop and no null-check on $p, inside an empty catch, and the inline step did not set $ErrorActionPreference. A SPAWN failure left $p null, $p.HasExited threw into the empty catch, and the shared runner kept a leftover tbe2ewin cluster -- the exact state this net exists to clear, and the one that sends the next run's New-K3dCluster down its reuse path for a false PASS with no real create. Now: ErrorActionPreference=Stop, an explicit null-check, a timeout-kill that surfaces rather than hides, and an exit-code check. On any failure it does NOT fail the run (this is always() teardown, the run is over) but emits a ::warning:: naming the stale cluster and the manual fix, so a leftover can never be silent again. Refs: tracebloc/backend#1426
…WO-PVC prod incident) (#549) * fix(chart): jobs-manager must use Recreate, not RollingUpdate (RWO-PVC deadlock) Production incident: a client's hi-jobs-manager sat Pending for 4+ hours, blocking dataset ingestion. Root cause is entirely ours: jobs-manager mounts two ReadWriteOnce PVCs (client-pvc, client-logs-pvc; local-path, WaitForFirstConsumer) but used strategy RollingUpdate with maxSurge:1. maxSurge brings up a SECOND pod before the old one is gone. On a healthy single node this happens to work (RWO permits two pods on the same node), so it ran for days. After a routine WSL2/Docker restart the PVCs must re-provision, and now the rollout presents TWO unscheduled consumers to local-path — WaitForFirstConsumer can't pick one, so the scheduler's VolumeBinding PreBind times out ("context deadline exceeded") and the pod never schedules. The image-refresh CronJob then compounds it: it writes its "last-refreshed digest" annotation only after `kubectl rollout status` SUCCEEDS, which now never happens, so every 15 min it sees recorded != latest and restarts again — a self-perpetuating loop (19 revisions). mysql-client starves on the same wedged provisioner. jobs-manager never goes Ready, so its Service has no endpoints and the ingestor submit job's curl to jobs-manager:8080 fails. Fix: strategy Recreate (drop maxSurge) — exactly one RWO consumer at a time, so provisioning and restarts always converge, even after a node restart. This matches mysql-deployment, which already uses Recreate for the identical RWO-PVC reason. Regression guard: jobs_manager_test.yaml now asserts strategy.type == Recreate and no rollingUpdate block, so this can't silently regress. Chart 1.9.10 -> 1.9.11 (chart content change must bump the version to reach installs). Follow-ups filed for the compounding factors: image-refresh restart loop, K8S version pin reaching clients, and node-restart survival. * fix(chart): image-refresh skips a tick when jobs-manager isn't settled (no restart storm) Second half of the RWO-deadlock incident (#546, folded in to avoid a chart-version collision with the Recreate fix — same incident, same 1.9.11 bump). image-refresh records its digest annotation only after `kubectl rollout status` succeeds. When a rollout can't complete (the RWO deadlock, or any stall), the annotation never lands, so every 15-min tick sees recorded != latest and restarts AGAIN — a sequential restart storm (the incident showed 19 restarts). concurrencyPolicy: Forbid only stops OVERLAPPING jobs, not this. Fix: before touching the deployment, `kubectl rollout status --timeout=10s`; if it isn't settled (rollout in progress, or pod stuck Pending on volume binding), log and exit 0 — a restart can't help an unschedulable pod and only churns ReplicaSets, which on a single-node local-path cluster wedges provisioning further. Retry on the next tick once settled. Regression guard in image_refresh_test.yaml asserts the skip is present in the rendered script. (image_refresh_test's 2 pre-existing failures are local helm v4 schema strictness — identical on clean develop; CI's pinned helm v3.15.4 passes the suite.) Closes #546
tracebloc-release-train
Bot
added
gate-nudge
…184) (#541) * test(seal-check): run the egress-enforcement probe live in k3d CI (#1184) 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> * fix(seal-check): install k3d/helm/kubectl before create_cluster (exit 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> * fix(seal-check): drive helm test off exit code, not --logs (Job hook) 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> * fix(seal-check): address Bugbot review (autostart, paths, shellcheck) 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> * fix(seal-check): avoid SIGPIPE/pipefail false-fail in hook guard (Bugbot) `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> * ci(seal-check): bump job timeout 20m->30m (Bugbot) 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> * fix(seal-check): positive control + probe timeout + lib paths + NS (Saqlain) - 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> * fix(seal-check): --tlsv1.2 (house rule) + SA wait + required Lint list (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> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Add direct `lengthEqual` assertions on spec.egress for the two locked-down cases so a stray appended rule is caught, not just an index shift. Correction vs the review suggestion: the locked-down case is 4 rules, not 3. `egressProxy.enabled` DEFAULTS TRUE (values.yaml — only `routeWorkloads` defaults false), so the egress-proxy permit rule renders even when the case doesn't set it: DNS + MySQL + requests-proxy + egress-proxy. Both locked-down cases assert count 4. Fast-follow to the #537 review (non-blocking nit, deferred at merge). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Bugbot, release-train staging hop #542, High. The always() safety-net teardown ran k3d cluster delete via Start-Process with no -ErrorAction Stop and no null-check on $p, inside an empty catch, and the inline step did not set $ErrorActionPreference. A SPAWN failure left $p null, $p.HasExited threw into the empty catch, and the shared runner kept a leftover tbe2ewin cluster -- the exact state this net exists to clear, and the one that sends the next run's New-K3dCluster down its reuse path for a false PASS with no real create. Now: ErrorActionPreference=Stop, an explicit null-check, a timeout-kill that surfaces rather than hides, and an exit-code check. On any failure it does NOT fail the run (this is always() teardown, the run is over) but emits a ::warning:: naming the stale cluster and the manual fix, so a leftover can never be silent again. Refs: tracebloc/backend#1426
Contributor
Author
|
bugbot run |
tracebloc-release-train
Bot
added
gate-nudge
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 2303a07. Configure here.
tracebloc-release-train
Bot
added
gate-nudge
Contributor
Author
|
/fr-pass |
LukasWodka
added a commit
that referenced
this pull request
Aug 4, 2026
… semicolons (Bugbot)
The hygiene scanner reported "clean" while missing real unhardened
assertions in three shapes Bugbot flagged:
- a nested `name() { ... }` stub's column-0 `}` ended the @test scan
early (check-drift.bats had an unhardened `[ "$_drift" -ge 1 ]`
after a helm() mock) -> track brace DEPTH, not the first `}`.
- one-line `@test "x" { run ...; [ ... ]; }` bodies were consumed as a
bare opener and never scanned (common.bats had two) -> scan the
inline body after the opening `{`.
- an assertion that is not the LAST statement of a compound/one-line
body -> classify each `;`-separated statement (subsumes the earlier
last_segment hack, more correctly); paren-aware so a `;` inside a
`( )`/`$( )` does not split a hardened `! ( a; b ) || return 1`, and
comment-aware so a `;` inside a trailing comment is not split either.
Hardens the 26 assertions the improved scanner then surfaced:
cluster.bats / assess.bats and the new #542/#547 check-facts tests (all
pulled in by the develop merge), plus check-drift.bats and the two
common.bats one-liners. Adds 3 regression tests (nested braces, one-line
bodies, subshell `;`).
Full suite 804/804; hygiene 12/12; scanner clean.
shujaatTracebloc
added a commit
that referenced
this pull request
Aug 5, 2026
* test(bats): make every assertion enforce — 1240 were advisory Under Bats (verified 1.13.0) only the LAST command in a test body decides the result, so a failing assertion anywhere earlier is silently ignored: @test "middle failure ignored" { [[ "abc" == *"zzz"* ]] # FALSE [[ "abc" == *"abc"* ]] # TRUE (last) } # -> ok This suite is written multi-assertion throughout, so most assertions could not fail their test. Appending `|| return 1` makes them enforce. Scope is about twice what it first looked. It is not only `[[ ]]`: single-bracket `[ ... ]` has identical semantics and there are MORE of them (609 vs 574), plus 61 negated bare commands. 1240 assertions across 15 files — setup-linux.bats 296, cluster.bats 153, install-client-helm.bats 146, common.bats 118, preflight.bats 102, and the rest smaller. Only whole-line assertions INSIDE an @test body are touched. Helpers and setup/teardown are excluded (a bare `return` there means something different), as are the 9 control-flow `if/while` conditions and 18 lines already chained with && / ||. All files still parse (bats --count), no control-flow line was modified, and nothing was double-appended. TRIAGE RESULT: zero new failures. All 1240 were already true — the suite was accidentally correct, so there was no hidden-bug vs stale-assertion split to report. No assertion was deleted or weakened to reach green. That result only means something if the hardening has teeth, so it was proven rather than assumed. cluster.bats's "_augment_no_proxy: empty host NO_PROXY" asserts 7 substrings and only enforced the last. Deleting `localhost` from TB_NO_PROXY_DEFAULTS — the entry that keeps a corporate proxy from intercepting loopback — is a real regression, and: mutated source + ORIGINAL tests -> ok (invisible) mutated source + HARDENED tests -> not ok (caught) Guard, so the pattern cannot come back: scripts/tests/bats-hygiene.bats plus a shared scanner, scripts/tests/unenforced-assertions.awk (one implementation, used by the guard and by its own self-tests). Three tests: the suite is clean; the scanner flags an un-hardened assertion and spares a hardened one; and it ignores control flow, chained lines, helpers and HEREDOC BODIES. That last exclusion is not cosmetic — the first version flagged its own fixture, which would have made any future test embedding example bats source a false positive. The guard was mutation-tested against the real suite too: un-hardening one line in cluster.bats makes it fail, naming the exact file:line. Gates: bats scripts/tests/*.bats -> plan 693, ok 693, not ok 0 (complete TAP run, plan line checked — a truncated read can look green while half the suite never reports); shellcheck --severity=error over the CI file set -> rc=0; check-style clean; check-drift no drift; gen-manifest.sh --check current (only tests changed, and tests are not part of the hashed set). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(bats-hygiene): scanner sees internal-OR and negated-bare assertions (Bugbot #527) Two Bugbot findings on the hygiene guard this PR introduces. Both real: the guard could report "suite is clean" while assertions stayed advisory — the exact failure mode the PR exists to close. Measured semantics first (bats 1.13.0, bash 3.2 system bash), because the old header's "only the LAST command decides" was too broad. Bats does run bodies under errexit; exactly two classes escape it: [[ ... ]] bash 3.2 (macOS system bash) does not fire errexit for a failing conditional expression — a middle one is ignored ! cmd POSIX: a status inverted with '!' is never propagated, so this escapes on EVERY bash, CI included grep -q ... a plain bare command DOES fail the test — correctly not reported 1) Scanner skips internal-OR assertions — REAL. `[[ a || b ]]` is ONE assertion whose ||/&& is internal; it exits non-zero on failure like any other and needs `|| return 1` too. The scanner skipped every line merely CONTAINING ||/&&, and only matched `[`/`[[` that closed on the same line, so it missed both single-line internal-OR and multi-line forms. Rewritten to build a logical line (trailing backslash, or a newline inside the brackets) and to locate the closer that matches the opener, so only a TOP-level chain earns the exemption: `[[ a ]] || fail` still skipped, `[[ a || b ]]` flagged, and `||` appearing only inside a quoted grep pattern no longer hides an assertion. Multi-line offenders are reported joined, at their first line. Six offenders it now catches (Bugbot named two; four are the same class): install-bootstrap.bats:144 and :154 — mid-body, so genuinely advisory — common.bats:179, install-client-helm.bats:887, preflight.bats:542, summary.bats:73. All six now end in `|| return 1`. 2) Guard omits negated bare commands — REAL. The PR hardened 61 `! cmd` assertions but the guard did not cover the class, so a later unhardened one would pass unnoticed — and this is the class that is advisory on every bash, not just 3.2. The scanner now flags standalone `! cmd ...`, while sparing `! cmd || return 1`, `if ! cmd`, bare `cmd`, and `run ! cmd`. Zero live offenders: the 61 are all hardened. Failing-test-first evidence, both directions verified by flipping the change: - two new bats-hygiene tests (internal-OR incl. both continuation styles and a pattern-only `||`; negated bare commands) fail on the old scanner, pass on the new one, and assert the spared cases so the scanner cannot over-report - with the new scanner and the un-hardened files, the "suite is clean" test fails and names all six offenders - end to end on real code: blanking install.sh's "not an immutable release tag" message left install-bootstrap.bats's two path-traversal tests GREEN before the fix and fails both after — an R8 regression the suite had been ignoring Also made the scanner portable (\b and `close` are not safe in every awk) and corrected the guard's header to the measured semantics. Local: bats scripts/tests/*.bats 695/695, shellcheck --severity=error clean, bash -n clean, gen-manifest.sh --check up to date, check-style.sh clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(chart-guard): harden the 46 assertions #530 added in parallel CI went red on the merge commit, not on this branch. Diagnosis first, because the answer changes the fix: #530 ("chart-guard: cover every published chart") merged scripts/tests/ chart-version-guard.bats into develop at 2026-07-31T15:59Z — 40 minutes AFTER this branch's last green Installer-tests run (30642417378, head 91a5fdd, 15:19Z). The file was written before this convention existed, so all 46 of its standalone assertions are bare. GitHub tests refs/pull/527/merge, so the guard correctly reported them. NOT caused by the scanner rewrite. The OLD scanner, exactly as shipped in 91a5fdd, flags the same 46 lines on that file — byte-identical output: awk -f <91a5fdd's scanner> chart-version-guard.bats | wc -l -> 46 awk -f <new scanner> chart-version-guard.bats | wc -l -> 46 diff of the two -> identical They are all plain single-line `[ ... ]` / `[[ ... ]]`, none of the classes this PR's rewrite added. So the branch head would have gone red on the same merge commit with or without my commit — this is develop drift meeting a guard that only just started existing, which is the guard doing its job on the first file that arrived after it. Fix: merge develop and append `|| return 1` to the 46. No assertion reworded, deleted or weakened; the guard is not relaxed to accommodate the new file. Local, post-merge: bats scripts/tests/*.bats 718/718, scanner reports 0, shellcheck --severity=error clean (incl. the new chart-version-guard.sh), gen-manifest.sh --check up to date, check-style.sh clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(bats-hygiene): a quoted <<TAG or a herestring is not a heredoc (Bugbot #527) REAL, and the most serious of the three findings — it made the guard lie. The heredoc detector matched `<<TAG` anywhere on a line, including inside a quoted string. Once tripped, it looked for a BARE terminator line that never comes, so the scanner silently ignored every remaining line in that file while the "suite is clean" test still reported clean. Proven on the real file, not just in theory. Appending an unhardened assertion to the end of bats-hygiene.bats — after its own `printf " cat > f <<'EOF'\n"`: awk -f unenforced-assertions.awk bats-hygiene.bats -> NO OUTPUT (invisible) bats bats-hygiene.bats -> ok 1 ... assertion ... ends in '|| return 1' The guard reporting clean while a bare assertion sits in the file it is scanning is the worst failure this PR could ship, since every other claim in the PR rests on that scan. A SECOND live instance Bugbot did not name: `<<<` herestrings. The regex matched from the second `<` of `run guard_leftover_data <<< "r"`, taking tag `r`, so leftover-guard.bats was swallowed from line 131 onward — the same canary appended there was equally invisible. Bugbot's Additional Locations listed only bats-hygiene.bats#L135-138. Fix, three parts: - `quoted_at()` walks shell quoting state, so a `<<TAG` inside '...' or "..." is text, not a redirection - `<<` immediately preceded by `<` is a herestring, not an opener - safety valve: an @test at column 0 ends heredoc-skip mode, so no future mis-detection can ever hide more than one test's worth of lines Real heredocs still skip their bodies: 12 genuine openers across the suite are still detected, and the pre-existing "ignores ... heredoc bodies" test fails if the tracking is deleted rather than fixed — so "stop tracking heredocs" cannot pass as a fix. Verified by disabling it: that test flips to not ok. Worth recording that the suite-clean scan CANNOT catch that regression (no real heredoc body in the suite contains a bare bracket line), so the fixture test is the only guard on it. Two new tests, flipped in both directions: old scanner -> not ok 5 (expected line 3 to be flagged) not ok 6 (expected line 6 to be flagged) new scanner -> ok 5, ok 6 Each fixture carries several distinguishable entries and asserts the exact offender count plus the spared lines, so neither can pass by over-reporting or by a fixture too small to tell an anchored rule from a loosened one. Local: bats scripts/tests/*.bats 720/720, scanner reports 0, shellcheck --severity=error clean, gen-manifest.sh --check up to date, check-style.sh clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(bats): enforce assertions in develop's newly-merged tests (#527 hygiene) Merging develop brought in test files/tests added after this branch forked (check-facts.bats, index-invariants.bats, setup-macos-lifecycle.bats, and new preflight.bats cases) whose standalone assertions were written in the bare, advisory form. bats-hygiene.bats — the enforcing-assertion guard this PR adds — correctly flagged 134 of them. Append `|| return 1` to each so every assertion can fail its test, exactly as this PR does across the rest of the suite. Mechanical: `|| return 1` inserted before any trailing inline comment; negated bare commands (`! grep …`) get the same enforcing form. Verified by re-running the scanner to zero offenders and the full bats suite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(bats): scanner enforces on real `|| return 1`, not the substring (Bugbot) The enforcing check was a line-wide substring match for `|| return 1`, so an assertion that merely MENTIONED the marker was treated as hardened though it does not enforce: `[[ "$output" == *"|| return 1"* ]]` (marker inside a quoted pattern) or `[[ "$x" == y ]] # ... || return 1` (marker only in a trailing comment) slipped through the guard (Cursor Bugbot, Medium). Add `strip_comment` (drop an unquoted trailing comment) + `is_enforcing` (require a `|| return 1` that is outside quotes and outside the comment), reusing the existing quote walker. New bats-hygiene self-test proves both fooling shapes are flagged and a real top-level `|| return 1` is still spared. Whole-suite sweep still reports 0 offenders, so the 134 conversions in the prior commit remain correctly recognized. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(bats): scanner catches compound-line assertions; harden the 107 it revealed (Bugbot) The unenforced-assertions scanner classified a line as a bracket assertion only when it OPENED with `[[`/`[`, so a mid-line assertion — `run x; [[ ... ]]`, the last command of a compound line — was invisible. On bash 3.2 that `[[` still cannot fail the test, so 107 such assertions across preflight/check-drift/setup-* were advisory: the exact failure mode this PR closes. - Scanner: check the last `;`-segment of a compound line for a standalone bracket assertion or negated bare command (`last_segment` + `classify`). Quote-aware; `bracket_tail` distinguishes an internal `||` from a real top-level chain. - Harden the 107 revealed assertions (append `|| return 1`, before any trailing comment). No test logic changed — 304 suite tests still pass, 0 failures. - bats-hygiene.bats: regression test for the compound-line case. The other Bugbot findings on this PR (internal-OR, negated-bare, substring, false-heredoc) were already handled by earlier commits; this closes the last one. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(bats): scanner robust to nested braces, one-line tests, subshell semicolons (Bugbot) The hygiene scanner reported "clean" while missing real unhardened assertions in three shapes Bugbot flagged: - a nested `name() { ... }` stub's column-0 `}` ended the @test scan early (check-drift.bats had an unhardened `[ "$_drift" -ge 1 ]` after a helm() mock) -> track brace DEPTH, not the first `}`. - one-line `@test "x" { run ...; [ ... ]; }` bodies were consumed as a bare opener and never scanned (common.bats had two) -> scan the inline body after the opening `{`. - an assertion that is not the LAST statement of a compound/one-line body -> classify each `;`-separated statement (subsumes the earlier last_segment hack, more correctly); paren-aware so a `;` inside a `( )`/`$( )` does not split a hardened `! ( a; b ) || return 1`, and comment-aware so a `;` inside a trailing comment is not split either. Hardens the 26 assertions the improved scanner then surfaced: cluster.bats / assess.bats and the new #542/#547 check-facts tests (all pulled in by the develop merge), plus check-drift.bats and the two common.bats one-liners. Adds 3 regression tests (nested braces, one-line bodies, subshell `;`). Full suite 804/804; hygiene 12/12; scanner clean. * test(bats): top-level chain must ignore quotes/subshells; join mid-line multiline brackets (Bugbot) Two more scanner gaps Bugbot flagged on the rewrite, both real: - the `||`/`&&` "already chained" exemption matched the operator anywhere in the statement, including inside a quoted pattern (`! grep -q "a||b" f`) or a `( )` subshell -> an unhardened negated command was silently treated as chained. Now a quote- and paren-aware top-level scan (`has_toplevel_chain`). - `bracket_open` only saw a continued `[[`/`[` at the START of the logical line, so a bracket opening mid-line (`run x; [[ a ||` continued onto the next line) was never joined -> a multi-line compound bracket stayed invisible. Now also checks the last `;`-segment. Adds 2 regression tests. Full suite 806/806; hygiene 14/14; scanner clean. * test(bats): scan one-line bodies whose bracket abuts the group closer (Bugbot) A one-liner with no `;` before `}` (`{ … [ a ] }`, or the no-space `[ a ]}` / `[[ a ]]}` where the closer is not recognised) left a `}` in the assertion's post-closer tail, so it read as non-standalone and stayed invisible. Strip the one-liner's group-closing `}` before classifying. Valid bats needs a `;` before `}` (verified: `f() { [ 1 = 1 ] }` is a bash syntax error), which already splits the assertion off — so this is defensive for the degenerate shapes, not a live suite offender. Regression test 15. Full suite 807/807; hygiene 15/15; scanner clean. * test(bats): make the bracket-closer finder quote-aware (Bugbot) after_close matched a blank-delimited `]]`/`]` by a word-boundary heuristic but never walked quote state — the one structural walker that wasn't quote-aware. A closer inside a quoted pattern (`[[ "$x" == "a ]] b" ]]`, `[ "$x" = "] y" ]`) was mistaken for the real closer, so the assertion read as non-standalone and an unhardened offender could slip through. Require the closer position to be unquoted (reuses quoted_at), matching split_segments / has_toplevel_chain / brace_delta / after_first_brace / strip_group_close. Regression test 16. Full suite 808/808; hygiene 16/16; scanner clean. * test(bats): harden the 24 assertions that arrived via the develop merge The hygiene gate went red on its own merge commit: develop gained gpu-nvidia.bats (2 advisory assertions) and the #582 network-profile block in preflight.bats (22 more) after this branch's sweep. Same mechanical treatment -- append `|| return 1`, comments preserved in place. Scanner reports 0 offenders; gpu-nvidia, preflight and bats-hygiene suites pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(bats): a || return 1 inside a subshell is not hardening (Bugbot) is_enforcing scanned for an unquoted `|| return 1` anywhere in the statement, so `! ( cmd || return 1 )` was spared — but that return only exits the subshell while the `!` still escapes errexit, leaving the statement advisory. Rewritten on the same quote+paren walker as has_toplevel_chain: only a top-level `|| return 1` counts. Fixture pins both subshell shapes (`( )` and `$( )`) flagged and both top-level shapes spared; the full-suite scan stays clean, so no real assertion was relying on the loophole. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(bats): a <<TAG in a comment is not a heredoc opener (Bugbot) heredoc_tag_of was quote-aware but not comment-aware, so a trailing comment DOCUMENTING heredocs opened skip mode with no terminator coming and the rest of the @test body was silently swallowed — live in this very suite, where bats-hygiene.bats comments mention <<TAG. Scan the comment-stripped line; strip_comment returns a prefix, so positions stay aligned for the quote and herestring look-arounds. Fixture pins: comment-mention doesn't skip, a real heredoc still does, and scanning resumes after its terminator. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: shujaat hasan <shujaat@tracebloc.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated promotion by the release train (RFC-0008 D14). Head is the train-managed
release-train/to-stagingbranch (a mirror ofdevelop), so it never collides with a human PR. Merged only when the fr-gate is green.Note
Medium Risk
Changes jobs-manager rollout strategy and image-refresh behavior on every cluster using those features; egress lockdown is now exercised live in CI but chart defaults are mostly guarded by tests rather than flipped here.
Overview
Client chart 1.9.11 addresses production incidents on single-node clusters: jobs-manager switches from
RollingUpdatetoRecreateso two pods never fight the same ReadWriteOnce PVCs, and the image-refresh script skips a tick whenkubectl rollout statusshows the deployment is not settled—avoiding repeated restarts while a rollout or volume bind is stuck (#545/#546). Helm unit tests lock in Recreate, the settle guard, egress lockdown NetworkPolicy shape whenallowExternalHttps: false, and the egress-enforcement test Job name used by live CI.CI and installers: Helm CI pins kubeconform by version and SHA-256 before install, adds a k3d seal-check-e2e job running
e2e-seal-check.sh(lockdown on,helm teston the egress probe), and widens path filters. Installer CI runscheck-facts.sh --checkagainst newscripts/spec/facts.envso bash and PowerShell tool pins andREADY_TIMEOUTcannot drift (#410-class); bats covercheck-facts.sh. A nightly self-hostedwindows-e2eworkflow ande2e-windows.ps1exercise the real PowerShell installer (credential-free), withdocs/WINDOWS-E2E.mdfor runner setup. Minor workflow doc fix for/fr-pass(staging-only FR gate).Reviewed by Cursor Bugbot for commit 2303a07. Bugbot is set up for automated code reviews on this repo. Configure here.