release-train: develop -> staging - #621
Merged
Merged
Conversation
… when it can't be (backend#2983) (#620) * fix(cluster): verify the mutating target from the API and fail closed when it can't be (backend#2983) A machine with no recorded cluster anchor warned "the target couldn't be verified ... Proceeding against https://127.0.0.1:8444" and then mutated anyway. The endpoint it named is a localhost port-forward — tbcluster allocates 8443/8444/8445 across fleets, so the same command shape reaches a different fleet depending on which tunnel is up on which port. The operator's only signal about which environment was about to change was a port number, and --no-input (contract: "fail on missing required values") did NOT fail closed: a CI job could ingest into the wrong fleet and get a green exit. guardActiveClientCluster now does both of the issue's directions: 1. VERIFY, don't merely record. It asks the API which client owns the cluster actually reached (keyed on the live kube-system UID) and prints that identity — name + namespace — instead of a port. This does not depend on local state, which is exactly what was missing in the field. 2. FAIL CLOSED when unverifiable. If the API can't confirm the target and no recorded anchor matches it, the guard refuses (exit non-zero, no write) unless the operator passes --i-know-the-target. A scripted --no-input caller has no human to ask, so this honours its contract; a human gets the same named escape hatch. The backend#2863 wrong-cluster refusal and its offline anchor path are kept: an anchor MISMATCH is still a hard refusal (--i-know-the-target does not override a known-wrong target, only an unverifiable one), and an anchor that MATCHES the live cluster still proceeds offline. --i-know-the-target is added to every command that mutates through resolveClusterTarget (data ingest, data delete, resources set, client status --seal), so the safety check is uniform and never a dead end. Tests: an unverifiable target under --no-input fails closed (data ingest and the destructive data delete paths); the printed identity comes from the API, not local state; the escape hatch proceeds; the anti-rot call-site sweep is updated for the new argument. VERSION 0.10.20 -> 0.10.21: 0.10.20 is already released, so this first published-path PR of the cycle carries the pending bump the version-bump-gate requires. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cluster): hard-stop a 426 in the target verifier, don't swallow it (backend#2983) Bugbot: verifyTargetFromAPI collapsed EVERY ListClients failure to "unverifiable", including a 426 UpgradeRequiredError. So a too-old CLI got waved through — an anchored machine proceeded via the offline anchor-match, and an unanchored one failed closed with a "no client registered — run client create" message instead of "tracebloc upgrade". The repo's learned rule is that HTTP 426 must be a hard failure, not a warning. verifyTargetFromAPI now returns the 426 (and only the 426) as an error; every other lookup failure stays swallowed to "couldn't confirm". The guard propagates it as a hard exit (exitFailure) BEFORE the anchor-match and fail-closed paths, so a stale local anchor can't press on past it. --i-know-the-target does not override it — upgrading is the only way through, and UpgradeRequiredError.Error() already says so. Tests (also covers two hardening asks from review): - 426 + a MATCHING anchor still hard-stops with the upgrade message (would otherwise have proceeded via anchor-match). - wrong-cluster mismatch + --i-know-the-target STILL refuses (the flag overrides an unknown target, never a known-wrong one) — mutation-proof for the security asymmetry. - unreadable-id + --i-know-the-target proceeds with a warning. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cluster): distinguish "couldn't reach the API" from "no client registered" (backend#2983) Bugbot (#515 shape): verifyTargetFromAPI returned the same "unverifiable" for an unreachable/unsigned-in API and for a successful list with no matching cluster_id. The fail-closed message then asserted "tracebloc has no client registered for it" and recommended `client create` in BOTH cases — reporting a couldn't-confirm as a confirmed absence, and pointing `client create` at an unconfirmed cluster (the adopt-vs-mint hazard the codebase guards against elsewhere via three-valued reads). verifyTargetFromAPI now returns (client, reached, err), mirroring DiscoverInClusterClientID's three-valued contract and clientSurvey.looked: - reached=true, client!=nil → verified. - reached=true, client==nil → CONFIRMED absence: the "no client registered — run client create" refusal is honest here (client create adopts an existing client on the cluster, or mints for a new one). - reached=false → couldn't ask (offline / not signed in / transient): the refusal says tracebloc couldn't be reached and points at sign-in / --context, and never asserts absence or recommends client create. - 426 still propagates as the hard-stop error (unchanged). Tests: split the fail-closed coverage into an unreachable case (asserts it does NOT say "no client registered" / "client create") and a confirmed-absence case (asserts it does). Golden updated for the two messages. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cluster): don't assert absence when the on-cluster client just isn't linked (backend#2983) Bugbot: the reached=true refusal said "tracebloc has no client registered for it" whenever no account client was ANCHORED to the live UID. But by the time the guard runs, resolveClusterTarget has already discovered a client release on this cluster — so it is never truly absent — and a legacy client's ClusterID is empty, so it can never match by UID however present it is. Asserting absence there is a couldn't-tell dressed as confirmed. Reworded the reached=true branch: the on-cluster client "isn't linked to any client in your account (it may be newly installed, or created before cluster anchoring)", and `tracebloc client create` "adopts the client already on this cluster and records it". No false absence, and the advice is exactly the adopt+backfill path. Guard behaviour (fail closed) is unchanged — only the wording. Test renamed ConfirmedAbsence → ReachedButUnmatched, now includes a legacy empty-ClusterID record in the account list and asserts the refusal does NOT say "has no client registered". Golden updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cluster): treat a Ctrl-C during target verify as a quiet interrupt, not a refusal (backend#2983) Bugbot: verifyTargetFromAPI folds every non-426 lookup error into "couldn't ask" (reached=false), including context.Canceled. So a Ctrl-C during the new ~8s backend lookup became reached=false — and on a MATCHING recorded anchor the guard then AUTHORIZED the mutation the operator just aborted; with no anchor it exited 3 with a "couldn't be reached / run login" refusal instead of a quiet 130, and later cluster calls failed under the already-cancelled context with a misleading kubeconfig error. Added interrupted(ctx): if the command's context is cancelled during a blocking guard step it returns exitInterrupted (130). It checks the context itself, so it is not fooled by verifyTargetFromAPI's inner verify timeout (which cancels only its own derived context) — a merely-slow backend still reads as reached=false. Wired at both blocking points: after the identity read and after the backend lookup, the latter BEFORE the anchor-match, so an abort can never fall through to a proceed. Test: a cancelled context during verify, with a matching anchor, exits 130 and does NOT print the anchor-match proceed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cluster): keep the Ctrl-C guard exit silent (backend#2983) Bugbot (Low): interrupted() attached ctx.Err() to the exitError, so IsSilentError was false and main() printed "Error: context canceled" on a Ctrl-C during the verify steps. exitInterrupted is meant to be a SILENT exit (IsSilentError keys on err==nil), which every other interrupt path relies on (auth.go, seal.go, client_status.go all return a bare &exitError{code: exitInterrupted}). Drop the inner err. Test now also asserts IsSilentError so the convention can't regress. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
bugbot run |
tracebloc-release-train
Bot
added
the
gate-nudge
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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 9fdc4d4. Configure here.
tracebloc-release-train
Bot
removed
the
gate-nudge
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
High Risk
Changes safety behavior for destructive cluster mutations and scripted CI (
--no-input), so jobs that previously succeeded against unverified targets will now fail unless operators adopt--i-know-the-targetor fix anchoring/login.Overview
Mutating cluster commands (
data ingest,data delete,resources set,client status --seal) no longer warn and continue when the target cluster cannot be verified. They refuse with a non-zero exit unless the operator passes--i-know-the-target.The cluster guard is reworked: it still compares the live kube-system UID to the machine’s recorded anchor and blocks known-wrong clusters (the ack flag does not bypass a mismatch). It now also asks the API which account client owns that UID and prints that identity when verified. If the API is unreachable or the cluster isn’t linked to the account—and there’s no matching local anchor—the command fails closed instead of mutating against an ambiguous localhost port-forward. HTTP 426 still hard-stops; Ctrl-C during guard steps exits quietly (130).
resolveClusterTargettakes an eighth argument for the ack flag; sharedaddKnowTargetFlagregisters the escape hatch on the mutating data/resources commands. Version 0.10.21.Reviewed by Cursor Bugbot for commit 9fdc4d4. Bugbot is set up for automated code reviews on this repo. Configure here.