release-train: staging -> main - #613
Merged
Merged
Conversation
…install's (backend#2863) (#608) * fix(cluster): refuse a mutating command on a cluster that is not the install's (backend#2863) Every command resolved its target cluster from the ambient kubeconfig + current-context while binding only the NAMESPACE from the active client. On a machine whose current-context points elsewhere -- a laptop that also administers a managed cluster, the normal case for anyone running both -- a mutating command acted on that other cluster: `data ingest` staged a private dataset onto it, `data delete` dropped a table and removed files from its shared PVC, `resources set` rolled its jobs-manager, and `tracebloc delete` uninstalled a release of the same name. The namespace binding made it MORE likely, not less: it supplied a namespace that probably exists on the other cluster too, so discovery succeeded and nothing on screen looked wrong. - record the cluster anchor (kube-system UID -- the same value the backend client record keys on) at `client create`, clear it on offboard - resolveClusterTarget takes a `mutates bool`: the compiler makes every caller decide, so a new command cannot forget an opt-in helper - `delete` guards separately and EARLY -- it shells out to helm and never resolved a target -- so a refusal happens before the credential is revoked Deliberately asymmetric: a mismatch refuses, an unreadable identity refuses for data commands (we are about to write to a cluster we cannot name), and an unrecorded anchor warns and proceeds so existing installs are not locked out. `delete` inverts the middle one: an unreachable cluster is the main reason to offboard, so it must not block. Ten mutations proven to redden their own test, each with the anchor asserted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(version): 0.10.18 — v0.10.17 is released and this PR changes published files --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
… (backend#2872) (#609) * docs(resources): stop calling requests == limits "the chart contract" (backend#2872) Four comments in this package explained the envelope by asserting a QoS class or a chart-wide contract. The requests == limits part is accurate here -- BuildEnvSpec writes RESOURCE_REQUESTS and RESOURCE_LIMITS as the same string, so a CPU training pod does come out Guaranteed. What was false is the framing: * "the chart contract" -- the chart DERIVE path (DERIVE_JOB_ENVELOPE) has written no cpu limit at all since backend#2418, so equal requests and limits is this package's choice for the explicit envelope, not an invariant of the chart. Calling it a contract is what let the claim survive after #2418 falsified it everywhere else. * a GPU pod is BestEffort no matter what this writes: client-runtime's GPU path sets only nvidia.com/gpu and ephemeral-storage, and neither counts toward the QoS class (backend#2871). * the "cpu=2,memory=8Gi" literal named as RESOURCE_LIMITS is stale -- the built-in fallback has been the contract floor cpu=1,memory=2Gi since backend#2254. Includes a test FAILURE MESSAGE (provenance_test.go), which is where a wrong explanation does the most damage: it is read only by whoever just broke the thing, at the moment they are deciding what the rule was. Comment-only; no behaviour change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(version): 0.10.18 — v0.10.17 is released and this PR changes published files --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
release-train: develop -> staging
Contributor
Author
|
bugbot run |
![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 521d663. Configure here.
tracebloc-release-train
Bot
added
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
1 participant
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-mainbranch (a mirror ofstaging), so it never collides with a human PR. Merged only when the fr-gate is green.Note
High Risk
Changes safety gates for destructive cluster operations (delete, ingest teardown, resources, offboard); wrong behavior could still block legitimate use or, if broken, allow cross-cluster writes.
Overview
Fixes backend#2863: mutating CLI commands could hit the cluster from
kubectl’s current context while only the namespace came from the active client, so ingest/delete/resources/seal/offboard could change the wrong cluster without obvious symptoms.The profile now stores
active_client_cluster_id(kube-system UID, same as the backend) whensetActiveClientruns.resolveClusterTargettakes a requiredmutatesflag; mutating callers runguardActiveClientClusterbefore PVC work and refuse on identity mismatch or unreadable cluster ID, with warn-and-proceed when no anchor exists (legacy configs). Read-only paths (data list,resources show) stay ungated.tracebloc deleteadds a parallel check before revoke/teardown (refuse on confirmed wrong cluster; do not block when the cluster is unreachable or unanchored) and clears the anchor on offboard.Also exports
cluster.ClusterIDFromfor identity checks on an already-built clientset, bumps 0.10.18, and tightensinternal/resourcescomments about QoS vs chart derive behavior (backend#2872).Reviewed by Cursor Bugbot for commit 521d663. Bugbot is set up for automated code reviews on this repo. Configure here.