fix: ensure Helm prerequisites on Tier 0 install path (Bugbot #383) - #395
Merged
Conversation
The Tier 0 fast path skips install_system_deps but still runs install_helm, which shells to get-helm-3 — that needs openssl (checksum) and tar (unpack). On a minimal docker-group host lacking them, the "zero-privilege" install promised no admin, then failed mid-Helm-install. Add _ensure_helm_prereqs: verify openssl+tar are present before install_helm on the Tier 0 path. We cannot install them there without sudo (Tier 0 promises no admin, and setup_pm/$PM_INSTALL aren't even run), so surface the real constraint with an actionable fail-fast instead of silently sudo-ing or dying cryptically. The full flow is unchanged (install_system_deps still installs them). Regenerate scripts/manifest.sha256 for the setup-linux.sh change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
Thanks for this — the finding is fully real (Tier 0 skips Product direction from Lukas on the remedy though: the installer should take care of its own requirements, not error out and tell the user to install tools. #396 implements that direction and supersedes the fail-fast approach here:
Leaving this PR open for you/Lukas to close once #396 lands. |
Regenerate manifest.sha256 for the merged installer set (idempotent). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
bugbot run |
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 50bc8d8. Configure here.
saadqbal
approved these changes
Jul 27, 2026
LukasWodka
added a commit
that referenced
this pull request
Jul 27, 2026
Regenerate manifest.sha256 for the merged installer set (idempotent). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
divyasinghds
added a commit
that referenced
this pull request
Jul 27, 2026
The post-upgrade Helm hook (#380) coupled a one-time, best-effort data migration to every chart upgrade, which repeatedly surfaced timing and --timeout edge cases. The backfill is a one-time-per-cluster operation, so run it manually as a standalone Job instead. The tracebloc-backfill runner (data-ingestors #393/#395) is unchanged and remains the mechanism. Removes: - templates/metadata-backfill-hook.yaml - tests/metadata_backfill_hook_test.yaml - metadataBackfill block in values.yaml + values.schema.json Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
divyasinghds
added a commit
that referenced
this pull request
Jul 27, 2026
The post-upgrade Helm hook (#380) coupled a one-time, best-effort data migration to every chart upgrade, which repeatedly surfaced timing and --timeout edge cases. The backfill is a one-time-per-cluster operation, so run it manually as a standalone Job instead. The tracebloc-backfill runner (data-ingestors #393/#395) is unchanged and remains the mechanism. Removes: - templates/metadata-backfill-hook.yaml - tests/metadata_backfill_hook_test.yaml - metadataBackfill block in values.yaml + values.schema.json Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
added a commit
that referenced
this pull request
Jul 27, 2026
…load, no get-helm-3 (#396) * feat(installer): self-serve Helm prerequisites — verified direct download, no get-helm-3 (#395) The installer now takes care of its own requirements instead of erroring out and telling the user to install tools: - install_helm fetches the pinned Helm release directly from get.helm.sh and verifies it against the published .sha256sum (fail-closed), exactly like the k3d direct download (#382). helm's get-helm-3 script is gone — it floats on the mutable helm/helm@main, performs its checksum step with openssl (absent on minimal cloud images, Bugbot #383), and its fetches are unbounded. openssl is no longer needed anywhere in the flow. - HELM_VERSION pin in common.sh (v4.2.3; 'latest' resolves at install time via get.helm.sh/helm-latest-version, same verified path; malformed tags fail closed before any fetch). - _ensure_unpack_tools: when tar/gzip are genuinely missing on the Tier 0 fast path (which skips install_system_deps), install them via the package manager — quietly as root/passwordless sudo, with an honest one-line reason when a password is needed — rather than aborting with 'go install tar'. install_system_deps drops openssl, adds gzip. Supersedes the fatal-error approach in #395 (product call: never tell the user to install tools we can install ourselves). Windows already fetches Helm directly from get.helm.sh, so the ps1 path is untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(installer): route sudo probes through _real_sudo/_have_sudo_bin (Bugbot r1) The A2 sudo shadow runs '-n true' as a command when root, and 'has sudo' is always true because the shadow function exists — use the #372 primitives like preflight_sudo/_probe_privilege do. The root-without- sudo-binary strip is dropped entirely: the shadow already handles root by executing the command directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(installer): Tier 0 unpack install — sudo keepalive + dpkg-lock wait + one combined install (Bugbot r2) _ensure_unpack_tools ran package installs on the Tier 0 path without the full flow's guards: apt could sit on the dpkg lock invisibly behind the spinner, and a long wait could outlast the just-primed sudo timestamp so the next sudo re-prompts behind the spinner and hangs. Prime, then keep the ticket warm (preflight_sudo's pattern; killed right after the install — the zero-privilege tier shouldn't hold a warm admin ticket), wait out the dpkg lock (bounded + visible), and install everything in ONE package- manager call so there's a single sudo consumer right after priming. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(installer): keepalive ownership + latest-tag isolation (Bugbot r3) - _ensure_unpack_tools kills only the keepalive IT started: on the Tier 1/2 recovery path SUDO_KEEPALIVE_PID belongs to preflight_sudo, and killing it would let later privileged steps re-prompt behind a spinner. The global is only claimed when empty (install_cleanup coverage) and only cleared when it is ours. - HELM_VERSION=latest: isolate the endpoint body with tail -1 — retry's attempt notices go to stdout and a failed-then-successful fetch would concatenate them into the captured tag, failing the anchored regex with a false 'couldn't resolve'. (install_k3d's resolver is immune: its ${var##*/} strip discards anything before the redirect URL.) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(copy-catalog): regenerate 00-install golden for the HELM_VERSION knob The PR adds a HELM_VERSION env var (common.sh), which surfaces one line in install-k8s.sh --help. emit_install reads --help live, so the golden drifted by exactly that line. Regenerated via TB_UPDATE_GOLDEN=1; verified locally (bats copy-catalog now green). * fix(installer): drop obsolete _ensure_helm_prereqs — Tier 0 no longer blocks on openssl (Bugbot #396) get-helm-3 is gone (replaced by a sha256-verified direct download), so Helm no longer needs openssl. The Tier-0 preflight still demanded openssl+tar and failed fast, aborting minimal-image installs for a dependency Helm doesn't use — and pre-empting _ensure_unpack_tools, which installs tar/gzip Tier-0-aware. Remove the function and its Tier-0 call; drop the tests that encoded the old openssl preflight (tar/gzip stays covered by _ensure_unpack_tools' own tests). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
saadqbal
pushed a commit
that referenced
this pull request
Jul 27, 2026
…lly (#404) The post-upgrade Helm hook (#380) coupled a one-time, best-effort data migration to every chart upgrade, which repeatedly surfaced timing and --timeout edge cases. The backfill is a one-time-per-cluster operation, so run it manually as a standalone Job instead. The tracebloc-backfill runner (data-ingestors #393/#395) is unchanged and remains the mechanism. Removes: - templates/metadata-backfill-hook.yaml - tests/metadata_backfill_hook_test.yaml - metadataBackfill block in values.yaml + values.schema.json Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
Resolves a High-severity Cursor Bugbot finding on the client promotion PR (#383, develop → main).
Bug: The Tier 0 fast path (
install_linux,scripts/lib/setup-linux.sh) returns after_install_userspace_toolsand never runsinstall_system_deps— but_install_userspace_tools→install_helmstill shells out toget-helm-3, which needsopenssl(checksum verification) andtar(unpack). Those packages are installed only ininstall_system_deps, and the file itself notes minimal cloud images (Amazon Linux 2023, minimal RHEL) ship neither. So on a docker-group host without them, the "zero-privilege" install promised no admin and then failed mid-Helm-install.Fix: New
_ensure_helm_prereqshelper, called on the Tier 0 path before_install_userspace_tools. It verifiesopenssl+tarare present. We deliberately do not install them there: on Tier 0 every package install requires sudo ($PM_INSTALLis sudo-prefixed andsetup_pmisn't even run on this path), which would break the "no administrator rights needed" promise. So when they're genuinely missing it fails fast with an actionable message (install them once / ask an admin, then re-run) instead of silently sudo-ing or dying cryptically. Hosts with a usable Docker almost always have both, so the common case is a silent no-op. The full flow is unchanged —install_system_depsstill installs them.Coverage: added
_ensure_helm_prereqsunit tests (present → no-op; missing → fatal naming the packages + the zero-privilege constraint) and assert it runs on the Tier 0 branch ininstall_linux.#383 (comment)
Lands on develop; the promotion PR head picks it up on the next develop sync and Bugbot re-reviews.
🤖 Generated with Claude Code
Note
Low Risk
Narrow change to the Tier 0 branch only; no sudo or package installs added, with bats and manifest checksum updated.
Overview
Fixes Bugbot #383: the Tier 0 Linux installer skips
install_system_depsbut still runsinstall_helmviaget-helm-3, which needsopensslandtar—so minimal images could fail mid-Helm after advertising a zero-privilege install.Adds
_ensure_helm_prereqs, invoked on the Tier 0 path before_install_userspace_tools. It only checks that those tools exist; it does not install packages (that would require sudo and break the Tier 0 promise). When they’re missing, the installer exits early with a message to install them or ask an admin. Tier 1/2 behavior is unchanged (install_system_depsstill provides them).Bats cover the helper (no-op vs fatal) and assert Tier 0
install_linuxcalls it;scripts/manifest.sha256is updated forsetup-linux.sh.Reviewed by Cursor Bugbot for commit 50bc8d8. Bugbot is set up for automated code reviews on this repo. Configure here.