fix(installer): restore Step-5 credential verify for minted creds (revert #293 → v1.8.4)#295
Merged
Merged
Conversation
…vert #293 → v1.8.4) #293 skipped verify_credentials for freshly-minted credentials on the premise that a minted client is "set to enroll" and only goes active once the pod connects, so the api-token-auth pre-verify would 400 on a valid credential. That premise was wrong. The real reason verify 400'd was a CLI bug (tracebloc/cli#125): `client create` wrote the numeric dashboard id — not the UUID username — as TRACEBLOC_CLIENT_ID, which is the auth username. The credential was genuinely invalid; verify_credentials was a correct canary that #293 masked, letting the install "succeed" while the pod crash-looped on "Unable to log in with provided credentials". With cli#125 fixed (v0.5.1), a minted credential is valid and verify passes. Restore the canary so a bad credential fails loudly at install time (Step 5) instead of silently at pod runtime: - install-client-helm.sh: drop the TRACEBLOC_CLIENT_MINTED skip branch; always verify_credentials in Step 5 (mint, adopt, and dual-mode alike). - provision.sh: drop the TRACEBLOC_CLIENT_MINTED marker (now unused). - tests + manifest.sha256: reverted to the pre-#293 state. - Chart: 1.8.3 → 1.8.4 (forward; not a version rollback). DEPENDS ON tracebloc/cli#125 (v0.5.1) being the latest CLI release — the installer fetches the CLI fresh, so the canary only passes once v0.5.1 mints the correct UUID username. Do not release this chart before v0.5.1 is out. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
👋 Heads-up — Code review queue is at 31 / 30 Above the WIP limit. The team convention is to review existing PRs before opening new work. Open PRs currently in Code review (oldest first):
Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.) |
This was referenced Jul 2, 2026
saadqbal
added a commit
that referenced
this pull request
Jul 2, 2026
…d client in place (RFC-0001 §7.2 / #838) Re-running the installer on an already-connected client did not behave per RFC-0001 §7.2. provision_client (Step 3) correctly adopts the existing client (get-or-create keyed on the cluster) and writes TRACEBLOC_CLIENT_ADOPTED=1 + the UUID + namespace to the credential file — but install-client-helm.sh had NO adopt handling. Its non-interactive path is gated on ID *and* PASSWORD both being set; adopt issues no password (the existing one stands, write-only on the backend), so Step 5 fell through to the manual "Client ID:" prompt, defaulting to the stale clientId in the previous values.yaml — a numeric dashboard id on any cluster first installed during the cli#125 window — which verify_credentials then rejected. Teach Step 5 an adopt branch: on TRACEBLOC_CLIENT_ADOPTED=1, find the live client release and `helm upgrade` it in place, reusing its stored credential (no prompt, no verify, no duplicate) and healing the stored clientId to the adopted UUID so a cli#125-era install self-corrects. Prefer --reset-then-reuse-values (Helm >= 3.14), fall back to --reuse-values. Falls through to the normal connect flow only when no live release is found. The "previous settings?" prompt is skipped on adopt. Extracted _resolve_chart_ref (shared by the fresh-install and adopt paths). bats: 3 new adopt tests (reconcile-in-place + heal, older-Helm fallback, no-release fallback); full suite green (47). No Chart.yaml bump here — ships with #295 as v1.8.4 (avoids a version conflict). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
👋 Heads-up — Code review queue is at 35 / 30 Above the WIP limit. The team convention is to review existing PRs before opening new work. Open PRs currently in Code review (oldest first):
Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.) |
shujaatTracebloc
approved these changes
Jul 2, 2026
saadqbal
added a commit
that referenced
this pull request
Jul 2, 2026
…d client in place (RFC-0001 §7.2 / #838) Re-running the installer on an already-connected client did not behave per RFC-0001 §7.2. provision_client (Step 3) correctly adopts the existing client (get-or-create keyed on the cluster) and writes TRACEBLOC_CLIENT_ADOPTED=1 + the UUID + namespace to the credential file — but install-client-helm.sh had NO adopt handling. Its non-interactive path is gated on ID *and* PASSWORD both being set; adopt issues no password (the existing one stands, write-only on the backend), so Step 5 fell through to the manual "Client ID:" prompt, defaulting to the stale clientId in the previous values.yaml — a numeric dashboard id on any cluster first installed during the cli#125 window — which verify_credentials then rejected. Teach Step 5 an adopt branch: on TRACEBLOC_CLIENT_ADOPTED=1, find the live client release and `helm upgrade` it in place, reusing its stored credential (no prompt, no verify, no duplicate) and healing the stored clientId to the adopted UUID so a cli#125-era install self-corrects. Prefer --reset-then-reuse-values (Helm >= 3.14), fall back to --reuse-values. Falls through to the normal connect flow only when no live release is found. The "previous settings?" prompt is skipped on adopt. Extracted _resolve_chart_ref (shared by the fresh-install and adopt paths). bats: 3 new adopt tests (reconcile-in-place + heal, older-Helm fallback, no-release fallback); full suite green (47). No Chart.yaml bump here — ships with #295 as v1.8.4 (avoids a version conflict). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saadqbal
added a commit
that referenced
this pull request
Jul 2, 2026
…d client in place (RFC-0001 §7.2 / #838) (#298) * fix(installer): honor TRACEBLOC_CLIENT_ADOPTED — reconcile a connected client in place (RFC-0001 §7.2 / #838) Re-running the installer on an already-connected client did not behave per RFC-0001 §7.2. provision_client (Step 3) correctly adopts the existing client (get-or-create keyed on the cluster) and writes TRACEBLOC_CLIENT_ADOPTED=1 + the UUID + namespace to the credential file — but install-client-helm.sh had NO adopt handling. Its non-interactive path is gated on ID *and* PASSWORD both being set; adopt issues no password (the existing one stands, write-only on the backend), so Step 5 fell through to the manual "Client ID:" prompt, defaulting to the stale clientId in the previous values.yaml — a numeric dashboard id on any cluster first installed during the cli#125 window — which verify_credentials then rejected. Teach Step 5 an adopt branch: on TRACEBLOC_CLIENT_ADOPTED=1, find the live client release and `helm upgrade` it in place, reusing its stored credential (no prompt, no verify, no duplicate) and healing the stored clientId to the adopted UUID so a cli#125-era install self-corrects. Prefer --reset-then-reuse-values (Helm >= 3.14), fall back to --reuse-values. Falls through to the normal connect flow only when no live release is found. The "previous settings?" prompt is skipped on adopt. Extracted _resolve_chart_ref (shared by the fresh-install and adopt paths). bats: 3 new adopt tests (reconcile-in-place + heal, older-Helm fallback, no-release fallback); full suite green (47). No Chart.yaml bump here — ships with #295 as v1.8.4 (avoids a version conflict). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(installer): adopt-reconcile must not depend on TRACEBLOC_CLIENT_ID (Bugbot) Bugbot (High): _reconcile_adopted_client read the adopted UUID from TRACEBLOC_CLIENT_ID for --set clientId, but provision_client CLEARS TRACEBLOC_CLIENT_ID on a real adopt (keeping only TB_NAMESPACE + TRACEBLOC_CLIENT_ADOPTED=1 — "let install_client_helm reconcile the existing release from the local values.yaml"). So reconcile always bailed on the empty id and fell through to the interactive prompt — exactly what this PR set out to fix. The bats tests masked it by exporting TRACEBLOC_CLIENT_ID directly. Reconcile the discovered live release in place with --reset-then-reuse-values (fallback --reuse-values), reusing its stored credential — no dependency on TRACEBLOC_CLIENT_ID, no --set clientId heal (which fought provision's design and couldn't work anyway). Tests now export ONLY TRACEBLOC_CLIENT_ADOPTED=1, matching what provision_client actually leaves in the env. Manifest regenerated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(installer): heal clientId on adopt via provision handoff (Bugbot ×2) Resolves both Bugbot findings on the adopt path, which were in tension: • High — reconcile read the UUID from TRACEBLOC_CLIENT_ID for --set clientId, but provision_client cleared it on adopt → reconcile bailed on the empty id and fell through to the interactive prompt. • Medium — the interim fix reconciled with --reuse-values only, so a cli#125-era numeric clientId was preserved and the workload still couldn't auth. provision_client now KEEPS + exports the adopted client id (its UUID username) on the adopt path (dropping only the absent password), so install_client_helm can: • heal the stored clientId to the UUID via `--set clientId=<uuid>` when present (reused values keep the correct stored password) — fixes cli#125-era installs; • reconcile WITHOUT a heal (no bail to a prompt) when no id was handed over (rebuilt host / R7 orphan) — the existing credential stands. Optional --set built as an args array (bash-3.2 safe). Tests: heal-with-UUID + no-id-reconcile-fallback for Step 5; provision adopt test asserts the id is kept. provision.bats 12/12, install-client-helm.bats green; manifest regenerated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- 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
3 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.
Draft — gated on tracebloc/cli#126 (v0.5.1). Do not merge/release until that CLI fix ships as the latest release.
Why
#293 skipped
verify_credentialsfor freshly-minted credentials, on the premise that a minted client is "set to enroll" and the api-token-auth pre-verify would 400 on a still-valid credential.That premise was wrong. The verify 400'd because of a CLI bug (cli#125):
client createwrote the numeric dashboard id — not the UUID username — asTRACEBLOC_CLIENT_ID, which is the auth username the pod sends to api-token-auth. The credential was genuinely invalid.verify_credentialswas a correct canary; #293 masked it, so the install reported success while thejobs-managerpod crash-looped onUnable to log in with provided credentials.What
With cli#126 fixed, a minted credential carries the UUID username and verify passes. Restore the canary so a bad credential fails loudly at install time (Step 5), not silently at pod runtime:
install-client-helm.sh: drop theTRACEBLOC_CLIENT_MINTEDskip branch — alwaysverify_credentialsin Step 5 (mint, adopt, dual-mode).provision.sh: drop the now-unusedTRACEBLOC_CLIENT_MINTEDmarker.manifest.sha256: reverted to the pre-fix(installer): skip Step-5 pre-verify for freshly-minted credentials (→ v1.8.3) #293 state (regenerated; matches).provision.bats12/12 andinstall-client-helm.batsgreen locally.Sequencing
Refs: RFC-0001 backend#830, installer credential handoff #838.
Note
Medium Risk
Changes install-time auth validation for all minted credential paths; wrong timing vs CLI v0.5.1 could fail good installs until the CLI fix ships.
Overview
Reverts the installer behavior that skipped
verify_credentialsfor freshly minted clients (TRACEBLOC_CLIENT_MINTED). Step 5 now always callsapi-token-authverification for env-supplied credentials—including after browserclient create—so bad usernames/passwords fail at install time instead of leavingjobs-managercrash-looping.provision.shno longer sets or clears the mint marker; tests drop the mint-skip case andmanifest.sha256is updated. Helm chart version bumps 1.8.3 → 1.8.4.Reviewed by Cursor Bugbot for commit d9e2227. Bugbot is set up for automated code reviews on this repo. Configure here.