fix(chart): drop unused RBAC verbs from API server and node-data-broker ClusterRoles

[ArangoGutierrez]

Description

Removes provably-unused RBAC verbs from the Topograph API server and node-data-broker ClusterRoles, tightening them toward least privilege. No runtime behavior changes — every dropped verb has zero call sites in the codebase.

Dropped verbs:

• API server (topograph): pods: get, daemonsets: list, configmaps: list (slinky engine rule)
• node-data-broker: nodes: list, daemonsets: list (IB rule), pods: get (IB rule)

Each removal was cross-referenced against actual client-go usage: the observer/broker/engine code paths use List/Watch/Get/Update/Create only where retained, and never exercise the dropped verbs (e.g. no Pods().Get, no DaemonSets().List, no ConfigMaps().List anywhere; the broker uses no informers and never lists nodes). The retained verbs are unchanged, so no supported provider/engine combination loses a grant it uses.

Helm-unittest coverage was extended with discriminating contains/notContains assertions in rbac_test.yaml and subchart_rbac_test.yaml (each mutation-verified to fail if a dropped verb reappears), and the render snapshot was refreshed.

Checklist

• I am familiar with the Contributing Guidelines.
• New or existing tests cover these changes.
• The documentation is up to date with these changes.
• All commits are signed off per DCO (git commit -s).

[copy-pr-bot]

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[ArangoGutierrez]

@dmitsh when you have a moment — this is the low-risk half of an RBAC least-privilege pass on the chart: it drops verbs that no code path exercises (API server pods:get, daemonsets:list, configmaps:list; broker nodes:list, daemonsets:list, pods:get). No behavior change; retained verbs are untouched. helm-unittest coverage was extended with discriminating assertions and the render snapshot refreshed — make chart-test is green.

The larger, discuss-first changes from the same audit are tracked separately in #386 (engine/provider-gate the main role), #387 (hardened securityContext defaults, breaking), and #388 (opt-in VAP for the broker). Thanks!

[greptile-apps]

Greptile Summary

This PR removes provably-unused RBAC verbs from the topograph API server and node-data-broker ClusterRoles, tightening permissions toward least privilege. The Go source code was verified: no Pods().Get, DaemonSets().List, ConfigMaps().List, or Nodes().List call sites exist in the node-data-broker binary; the API server similarly has no Pods().Get or DaemonSets().List usage. Helm unit tests are extended with discriminating contains/notContains assertions and the render snapshot is refreshed to match.

• charts/topograph/templates/rbac.yaml: drops pods: get, daemonsets: list, and configmaps: list (slinky-only) from the API server ClusterRole.
• charts/topograph/charts/node-data-broker/templates/rbac.yaml: drops nodes: list, daemonsets: list (IB rule), and pods: get (IB rule) from the broker ClusterRole.
• Tests: rbac_test.yaml and subchart_rbac_test.yaml add positive and negative (mutation) assertions for every changed rule; snapshot file updated accordingly.

Confidence Score: 5/5

Safe to merge — only Helm RBAC templates, snapshot, and test files are changed; retained verbs are backed by concrete Go call sites.

Every dropped verb was cross-referenced against the actual Go client-go call sites: the codebase has no Pods().Get, DaemonSets().List, ConfigMaps().List, or Nodes().List in the node-data-broker binary. The GetDaemonSetPods helper uses DaemonSets().Get and Pods().List — both retained. The broker's apply function uses only Nodes().Get and Nodes().Update — both retained. Tests cover both the positive (new verb set present) and negative (old verb set absent) cases, making accidental reversion detectable.

No files require special attention.

Important Files Changed

Filename	Overview
charts/topograph/templates/rbac.yaml	Drops pods:get, daemonsets:list, and configmaps:list (slinky) from the API server ClusterRole; retained verbs match actual client-go call sites in the codebase.
charts/topograph/charts/node-data-broker/templates/rbac.yaml	Drops nodes:list, daemonsets:list (IB), and pods:get (IB) from the broker ClusterRole; broker uses only Nodes().Get+Update and DaemonSets().Get+Pods().List.
charts/topograph/tests/rbac_test.yaml	Adds positive contains and negative notContains mutation assertions for every dropped verb; the slinky configmaps test now correctly verifies [create,get,update] without list.
charts/topograph/tests/subchart_rbac_test.yaml	Adds discriminating contains/notContains assertions for node-data-broker nodes, daemonsets, and pods rules; well structured and mutation-detectable.
charts/topograph/tests/snapshot/render_snapshot_test.yaml.snap	Snapshot refreshed consistently across all eight render scenarios to remove the dropped verbs; no inconsistencies detected.
CHANGELOG.md	Adds a Security section entry under the upcoming release accurately describing all six dropped verbs.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph topograph["topograph API Server ClusterRole"]
        P1["pods\n✓ list\n✗ ~~get~~ (removed)"]
        N1["nodes\n✓ get, list, update"]
        DS1["daemonsets\n✓ get\n✗ ~~list~~ (removed)"]
        CM1["configmaps (slinky only)\n✓ create, get, update\n✗ ~~list~~ (removed)"]
    end

    subgraph broker["node-data-broker ClusterRole"]
        N2["nodes\n✓ get, update\n✗ ~~list~~ (removed)"]
        DS2["daemonsets (IB only)\n✓ get\n✗ ~~list~~ (removed)"]
        P2["pods (IB only)\n✓ list\n✗ ~~get~~ (removed)"]
        PE["pods/exec (IB only)\n✓ create"]
    end

    subgraph go["Go call sites verified"]
        GN["GetNodes → Nodes().List()"]
        GDS["GetDaemonSetPods → DaemonSets().Get()"]
        GPL["GetDaemonSetPods → Pods().List()"]
        GNG["broker.apply → Nodes().Get()"]
        GNU["broker.apply → Nodes().Update()"]
    end

    N1 --> GN
    DS1 --> GDS
    P1 --> GPL
    N2 --> GNG
    N2 --> GNU
    DS2 --> GDS
    P2 --> GPL

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    subgraph topograph["topograph API Server ClusterRole"]
        P1["pods\n✓ list\n✗ ~~get~~ (removed)"]
        N1["nodes\n✓ get, list, update"]
        DS1["daemonsets\n✓ get\n✗ ~~list~~ (removed)"]
        CM1["configmaps (slinky only)\n✓ create, get, update\n✗ ~~list~~ (removed)"]
    end

    subgraph broker["node-data-broker ClusterRole"]
        N2["nodes\n✓ get, update\n✗ ~~list~~ (removed)"]
        DS2["daemonsets (IB only)\n✓ get\n✗ ~~list~~ (removed)"]
        P2["pods (IB only)\n✓ list\n✗ ~~get~~ (removed)"]
        PE["pods/exec (IB only)\n✓ create"]
    end

    subgraph go["Go call sites verified"]
        GN["GetNodes → Nodes().List()"]
        GDS["GetDaemonSetPods → DaemonSets().Get()"]
        GPL["GetDaemonSetPods → Pods().List()"]
        GNG["broker.apply → Nodes().Get()"]
        GNU["broker.apply → Nodes().Update()"]
    end

    N1 --> GN
    DS1 --> GDS
    P1 --> GPL
    N2 --> GNG
    N2 --> GNU
    DS2 --> GDS
    P2 --> GPL

Loading

_{Reviews (2): Last reviewed commit: "test(chart): assert broker IB pods verb ..." | Re-trigger Greptile}

[ArangoGutierrez]

Heads-up on the red govulncheck check here: it's not from this PR (which is chart-only). It's a freshly-published Go stdlib advisory — GO-2026-5856, an ECH privacy leak in crypto/tls reachable from existing code — that turns CI red on main and every open PR. Fixed in Go 1.25.12; bumped in #390. Once #390 merges, a rebase here clears the check. Every other check (build, test, chart-test, DCO) is green.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[ArangoGutierrez]

@dmitsh this one is approved as well. One heads-up before merge: after the rebase onto your 1.26.5 bump, the presubmit didn't re-run on the current head (ab59880) — only DCO and Greptile show, and no pull-request/384 mirror was created, so build/test/chart-test/govulncheck haven't run on it. Could you kick the presubmit (or let the merge queue run it)? Once it's green it's good to merge. Happy to re-push to trigger it if that's easier on your end.