Skip to content

Feature/node data broker validating admission policy - #439

Open
Shankar-v27 wants to merge 1 commit into
NVIDIA:mainfrom
Shankar-v27:feature/node-data-broker-validating-admission-policy
Open

Feature/node data broker validating admission policy#439
Shankar-v27 wants to merge 1 commit into
NVIDIA:mainfrom
Shankar-v27:feature/node-data-broker-validating-admission-policy

Conversation

@Shankar-v27

Copy link
Copy Markdown
Contributor

Description

Closes #386

This PR updates the Topograph Helm chart to follow the principle of least privilege by rendering only the RBAC permissions required for the configured provider and engine.

Summary

  • Dynamically compute the required Node verbs based on the enabled providers and engines.
  • Render Kubernetes RBAC rules (pods, pods/exec, daemonsets, configmaps, etc.) only when they are required.
  • Skip rendering the ClusterRole and ClusterRoleBinding when no Kubernetes API permissions are needed.
  • Update the Helm RBAC unit tests to cover the new permission matrix.
  • Update the changelog.

Motivation

Previously, the chart could grant permissions that were unnecessary for a given deployment configuration. This change minimizes the RBAC footprint by ensuring only the permissions required for the selected configuration are rendered, reducing the attack surface while preserving existing functionality.

Testing

  • Updated Helm RBAC unit tests to validate the generated permission matrix.
  • Verified the rendered templates through static review.
  • Rebased onto the latest main and resolved merge conflicts.

Checklist

  • I have read the project's contributing guidelines.
  • Tests have been updated where applicable.
  • Documentation has been updated where applicable.
  • All commits are signed off (git commit -s).

…lege (NVIDIA#386)

Signed-off-by: Shankar V <shankarvelmurugan2018@gmail.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 29, 2026

Copy link
Copy Markdown

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.

@greptile-apps

greptile-apps Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes the Topograph Helm chart’s API-server RBAC conditional on the selected provider and engine.

  • Computes the required Kubernetes resources and Node verbs for each supported configuration.
  • Omits the ClusterRole and ClusterRoleBinding when no Kubernetes API access is required.
  • Expands Helm unit coverage for the resulting permission matrix and documents the security improvement.

Confidence Score: 5/5

The PR appears safe to merge, with the rendered permissions matching the Kubernetes API operations required by the covered provider and engine configurations.

The conditional resource rules preserve Node get and update access for the Kubernetes engine, list and patch access for Slinky, provider-specific pod and DaemonSet access for InfiniBand, and NFD-specific resources while cleanly omitting unused RBAC.

Important Files Changed

Filename Overview
charts/topograph/templates/rbac.yaml Introduces configuration-driven RBAC resource gating and consolidates Node permissions without identifying a broken runtime permission path.
charts/topograph/tests/rbac_test.yaml Updates existing assertions and adds coverage for provider/engine permission combinations, including complete RBAC omission.
CHANGELOG.md Documents the least-privilege RBAC behavior introduced by the chart change.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    C[Provider and engine configuration] --> P{Pods required?}
    C --> E{Pod exec required?}
    C --> N{Node access required?}
    C --> D{DaemonSet access required?}
    C --> M{ConfigMap access required?}
    P --> R{Any Kubernetes permission required?}
    E --> R
    N --> R
    D --> R
    M --> R
    R -->|Yes| CR[Render ClusterRole and ClusterRoleBinding]
    R -->|No| OMIT[Omit cluster-scoped RBAC]
    C --> F{NFD engine?}
    F -->|Yes| NS[Render NFD namespaced Role and RoleBinding]
    F -->|No| DONE[No NFD-specific RBAC]
Loading

Reviews (1): Last reviewed commit: "fix(chart): gate Topograph API server RB..." | Re-trigger Greptile

Sign up for free to 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.

RBAC: engine/provider-gate the main Topograph ClusterRole (node/pod/daemonset verbs)

1 participant