Skip to content

sec(chart): narrow auto-upgrade RBAC into the GPU device-plugin namespace (backend#1992) - #720

Merged
LukasWodka merged 1 commit into
developfrom
fix/1992-gpu-ns-rbac
Aug 14, 2026
Merged

sec(chart): narrow auto-upgrade RBAC into the GPU device-plugin namespace (backend#1992)#720
LukasWodka merged 1 commit into
developfrom
fix/1992-gpu-ns-rbac

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

What

The auto-upgrade ServiceAccount had no RBAC in the GPU device-plugin namespace. This adds a narrow Role + RoleBinding there.

Fixes tracebloc/backend#1992 — Bugbot High on the staging promotion PR #719, which it held.

Why

A Linux GPU install templates two namespaced objects into gpu.devicePlugin.namespace (kube-system by default):

TemplateObject
templates/gpu-device-plugin.yamlapps/v1 DaemonSetnvidia-device-plugin-daemonset / amdgpu-device-plugin-daemonset
templates/docker-registry-secret.yamlv1 Secret<release>-regcred (dockerconfigjson), when dockerRegistry.create

Nothing granted the auto-upgrade SA access to either: its Role is confined to .Release.Namespace (plus, conditionally, the node-agents namespace) and its ClusterRole enumerates only cluster-scoped kinds. So after the backend#953 cluster-admin cutover a helm upgrade --atomic --wait tick 403s on those objects, rolls back, and leaves the auto-upgrade CronJob permanently failed — on a customer GPU edge, not in CI, and disabling the very channel that would deliver its own fix.

How the rule set was derived

Not from the ticket prose. I rendered the whole chart across the GPU value matrix and enumerated every object whose metadata.namespace equals the device-plugin namespace:

gpuNs=kube-system → apps/v1 DaemonSet nvidia-device-plugin-daemonset
v1 Secret stg-regcred
gpuNs=gpu-operator → same two, relocated
gpuNs=<release ns> → nothing extra (collapses into the release-namespace Role)
vendor=amd → amdgpu-device-plugin-daemonset instead
dockerRegistry off → DaemonSet only
gpu absent/null → nothing

Confirms the DaemonSet + Secret. No ServiceAccount is templated there (the plugin uses the namespace default SA), and no SCC. One thing the ticket did not list surfaced: the Role and RoleBinding this change itself adds are also namespaced objects in that namespace, so they must be self-reproducing — see below.

Why it is narrow, not * on *

The node-agents block uses apiGroups/resources/verbs: ["*"], but that namespace is one this chart creates and owns. This one defaults to kube-system, which holds the service-account tokens of kube-controller-manager and friends. * on * there — or even a plain namespace-wide secrets: list — is cluster-admin by another route, i.e. exactly the blast radius backend#953 set out to remove. So write verbs are pinned by resourceNames:

  • daemonsets — name-free get, list, watch because helm's readiness check and three-way-merge diff must read the kind (resourceNames cannot authorize list/watch, and a 403 on that path is this very outage). update, patch, delete restricted to the plugin DaemonSet, so this cannot patch a privileged container into kube-proxy. Both vendor names are listed: flipping gpu.devicePlugin.vendor makes one upgrade delete the outgoing DaemonSet and create the incoming one, and a single-name grant would 403 on that delete.
  • secretsno namespace-wide read at all.create cannot be name-restricted by RBAC; every other verb is pinned to the one mirrored pull Secret. Granted unconditionally inside the block rather than behind tracebloc.useImagePullSecrets, so turning dockerRegistry off can still delete the Secret previously mirrored here.
  • roles/rolebindings — the two objects rendered by this block. Helm re-applies every manifest object on upgrade and their helm.sh/chart label changes on every version bump, so without this the SA could not reconcile its own grant and auto-upgrade would 403 on the tick after this one. Note the absence of escalate/bind: they are not needed, because Kubernetes' escalation check passes when the actor already holds every permission in the Role it writes — and this Role grants exactly what the binding gives the SA, so it is self-reproducing by construction. escalate on roles in kube-system, by contrast, would allow authoring a Role that reads every Secret there.

create is not name-restrictable for any kind, but it grants nothing new: the release-namespace Role is already * on *, so the SA can create these kinds somewhere regardless. What matters is that it cannot read or mutate objects it does not own in kube-system.

Gating mirrors the nil-safe default dict pattern from docker-registry-secret.yaml, so a --reset-then-reuse-values upgrade from a release predating client#564 cannot panic on a missing gpu key.

No flag-day: this rides the same release as the #953 cutover, so the upgrade that installs it is still performed by the previous cluster-admin-bound SA.

One declaration, not two

The DaemonSet name moves into a shared helper tracebloc.gpuDevicePluginName, used by both gpu-device-plugin.yaml (metadata.name) and auto-upgrade-rbac.yaml (resourceNames). A resourceNames restriction that drifts from the object it names fails closed into exactly this outage, so the two must not be independently restated. Mutating the helper reddens both auto_upgrade_test.yaml and gpu_device_plugin_test.yaml.

Tests

8 new cases in client/tests/auto_upgrade_test.yaml: not rendered by default; not rendered when gpu is explicitly null; rendered and correctly bound to the release-namespace SA when enabled; exactly the derived rule set (whole-array equality, so any added resource, added verb, widened apiGroup or dropped resourceNames fails) plus explicit no-wildcard and no-namespace-wide-secret-read guards; both vendor names present; not rendered when the device-plugin namespace is the release namespace; placed in a custom namespace.

Test Suites: 31 passed, 31 total
Tests: 454 passed, 454 total

Mutation evidence

Every mutation was verified to have actually applied before running the suite (an inert mutation and real coverage look identical in a log), and each was checked to still render — a parse error is not proof of coverage.

MutationResult
Remove the GPU Role/RoleBinding block entirely (reintroduce the bug); template still renders4 failed — the 4 rendering tests, while the "not rendered" tests stayed correctly green
"Simplify" the Role to */*/* like the node-agents block2 failed — exact-rules equaland the wildcard notContains guard fired by name
Widen secrets to a namespace-wide get,list,watch,... (the kube-system token-read hole)1 failed — exact-rules equal
Drift the shared tracebloc.gpuDevicePluginName helper3 failed across two suitesauto_upgrade_test.yaml + gpu_device_plugin_test.yaml
Drop the (ne $gpuNs .Release.Namespace) guard1 failed — the release-namespace case
Drop the roles/rolebindings self-reproduction rules1 failed — exact-rules equal

Restored → 454/454 green.

Other gates

  • make check green (lint, drift, helm lint --strict across all four ci/*-values.yaml, env-vocabulary agreement).
  • make helm-template renders all four platforms.
  • client/Chart.yaml bumped 1.9.42 → 1.9.43 — chart-version-guard.sh run locally against origin/develop: client chart content changed and client/Chart.yaml 'version:' was bumped. ✓
  • scripts/ untouched, verified by git status, so scripts/gen-manifest.sh / manifest.sha256 regeneration does not apply (the Static analysis R8 gate is not in play).

Follow-up — deliberately OUT OF SCOPE here

Defaulting gpu.devicePlugin.namespace to the release namespace would remove this RBAC hole and the pull-Secret mirror at the root: device plugins have no requirement to live in kube-system — they talk to the kubelet over a hostPath socket, so kube-system is upstream convention, not a constraint. It is out of scope here because pre-client#564 leftovers live in kube-system and the installer's adoption path hard-codes it, so the migration would become delete-and-recreate rather than adopt in place. Worth its own ticket with a migration story; not something to smuggle into a fix that has to ship on this hop.


Note

Medium Risk
Changes auto-upgrade RBAC in kube-system (or another GPU namespace) on enabled GPU installs; mistakes could break upgrades or widen secret access, but the grant is tightly name-scoped and heavily tested.

Overview
Fixes backend#1992: after the cluster-admin cutover, auto-upgrade could 403 on namespaced objects in gpu.devicePlugin.namespace (default kube-system)—the vendor DaemonSet and mirrored pull Secret—causing helm upgrade --atomic to roll back and leave the CronJob permanently failed on GPU edges.

When the device plugin is enabled and that namespace is not the release namespace, the chart now renders a narrow Role + RoleBinding there (not * on * like node-agents): DaemonSet list/watch plus name-pinned mutate on both NVIDIA and AMD plugin DaemonSets; create plus name-pinned secret verbs on <release>-regcred only; and self-reconciliation of that Role/Binding. Gating uses nil-safe default dict on gpu so old --reuse-values upgrades do not panic.

tracebloc.gpuDevicePluginName centralizes upstream DaemonSet names; gpu-device-plugin.yaml uses it for metadata.name so RBAC resourceNames cannot drift. Chart 1.9.42 → 1.9.43. Eight new auto_upgrade_test.yaml cases lock document counts, binding wiring, exact rule set, vendor flip, and custom namespace behavior.

Reviewed by Cursor Bugbot for commit ac75e24. Bugbot is set up for automated code reviews on this repo. Configure here.

…pace (backend#1992)
A Linux GPU install templates NAMESPACED objects into
gpu.devicePlugin.namespace (kube-system by default): the vendor device-plugin
DaemonSet, and — on an authenticated mirror — the mirrored dockerconfigjson
pull Secret. The auto-upgrade ServiceAccount had no grant that reaches there:
its Role is confined to the release namespace (plus, conditionally, the
node-agents namespace) and its ClusterRole enumerates only cluster-scoped
kinds. After the backend#953 cluster-admin cutover a `helm upgrade --atomic
--wait` tick therefore 403s on those two objects, rolls back, and leaves the
auto-upgrade CronJob permanently failed — on a customer GPU edge, not in CI,
and disabling the very channel that would deliver its own fix.
Add a Role + RoleBinding in that namespace, rendered only when the device
plugin is enabled AND its namespace differs from the release namespace.
The grant is deliberately NOT `*` on `*` like the node-agents block. That
namespace is one this chart creates and owns; this one defaults to kube-system,
which holds the service-account tokens of kube-controller-manager and friends,
so `*` on `*` — or even a plain namespace-wide `secrets: list` — would be
cluster-admin by another route and would undo what backend#953 was for. The
rules were derived by rendering the chart across the GPU value matrix and
enumerating what actually lands in that namespace, and write verbs are pinned
by resourceNames:
* daemonsets — name-free get/list/watch (helm's readiness check and
three-way-merge diff must read the kind, and resourceNames cannot authorize
list/watch), with update/patch/delete restricted to the plugin DaemonSet, so
this cannot patch a privileged container into kube-proxy. Both vendor names
are listed: flipping gpu.devicePlugin.vendor makes one upgrade delete the
outgoing DaemonSet, which a single-name grant would 403 on.
* secrets — no namespace-wide read at all; every verb but `create` (which RBAC
cannot name-restrict) is pinned to the one mirrored pull Secret. Granted
unconditionally inside the block, not behind useImagePullSecrets, so turning
dockerRegistry off can still DELETE the Secret previously mirrored here.
* roles/rolebindings — the two objects this block renders. Helm re-applies
every manifest object on upgrade and their helm.sh/chart label changes on
every bump, so without this the SA could not reconcile its own grant. No
`escalate`/`bind`: the grant already covers every permission it writes, so
Kubernetes' escalation check passes without them, whereas `escalate` on
roles in kube-system would allow authoring a Role that reads every Secret
there.
The DaemonSet name moves into a shared helper (tracebloc.gpuDevicePluginName)
so the resourceNames restriction and the rendered object have ONE declaration
rather than two that can drift into exactly the 403 this fixes. Mutating the
helper reddens both this suite and gpu_device_plugin_test.yaml.
No flag-day: this rides the same release as the #953 cutover, so the upgrade
that installs it is still performed by the previous cluster-admin-bound SA.
Tests: 8 new cases in client/tests/auto_upgrade_test.yaml — absent by default
and when gpu is null, rendered + correctly bound when enabled, EXACTLY the
derived rule set (whole-array equality, so any widening fails) with explicit
no-wildcard and no-namespace-wide-secret-read guards, both vendor names
present, absent when the namespace IS the release namespace, and placed in a
custom namespace. All mutation-proven; 454/454 green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodkaLukasWodka self-assigned this Aug 14, 2026
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 ac75e24. Configure here.

@shujaatTraceblocshujaatTracebloc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve — least-privilege RBAC done right (backend#1992). After the #953 cluster-admin cutover the auto-upgrade SA lost the reach to manage the GPU device-plugin objects that live in gpu.devicePlugin.namespace (kube-system by default): the vendor DaemonSet and the mirrored pull Secret. Without a grant there, helm upgrade --atomic --wait 403s, rolls back, and permanently kills auto-upgrade on customer GPU edges — disabling the channel that would ship the fix. The new Role is correctly NARROW, not * on *: kube-system holds controller-manager SA tokens, so a namespace-wide secret read or wildcard there would re-open the exact cluster-admin blast radius #953 closed. daemonsets get name-free get/list/watch (helm readiness needs list/watch, which resourceNames cannot authorize) with mutating verbs pinned to BOTH vendor DaemonSet names; secrets have NO namespace-wide read, write verbs pinned to the one mirrored pull Secret; roles/rolebindings pinned to this Role/binding for self-reconcile, with escalate/bind deliberately omitted (the grant is self-reproducing so the privilege-escalation check passes without them). Nil-safe namespace read mirrors docker-registry-secret.yaml (no panic on --reset-then-reuse-values from pre-#564); rendered only when the plugin is enabled AND its namespace differs from the release namespace; DaemonSet name derived from a shared helper so RBAC resourceNames cannot drift into a 403; no flag-day (first upgrade rides the #953 cutover under the old SA). Chart bumped 1.9.42→1.9.43 lockstep, 206 lines of helm tests. CI green, Bugbot clean.

@LukasWodka
LukasWodka merged commit 8b167b2 into developAug 14, 2026
24 checks passed
@LukasWodka
LukasWodka deleted the fix/1992-gpu-ns-rbac branch August 14, 2026 14:27
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@LukasWodka@shujaatTracebloc@claude