From c71a7b0cc62facba9697f16209a84ecfb80caaff Mon Sep 17 00:00:00 2001 From: teerth sharma Date: Sun, 26 Jul 2026 17:21:25 +0530 Subject: [PATCH] fix(chart): gate main ClusterRole by engine and provider (#386) Signed-off-by: teerth sharma --- CHANGELOG.md | 4 + charts/topograph/templates/rbac.yaml | 52 ++++++-- .../render_snapshot_test.yaml.snap | 66 ---------- charts/topograph/tests/rbac_test.yaml | 119 +++++++++++++++--- 4 files changed, 145 insertions(+), 96 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f62b4529..3e8016f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - GCP and OCI simulation providers now handle partial final pagination pages without indexing past the available instances. - Non-positive `pageSize` configuration values now emit a warning and use the provider default instead of reaching provider APIs and simulation pagination loops. +### Security + +- Main Topograph API server ClusterRole rules are gated by the selected engine and provider: `nodes`, `pods`, `daemonsets`, and `configmaps` permissions render only when the engine or provider reaches the Kubernetes API, and the ClusterRole and ClusterRoleBinding are omitted entirely for non-Kubernetes combinations such as the `test` provider with the `slurm` engine. + --- ## [v1.0.0] - 2026-08-18 diff --git a/charts/topograph/templates/rbac.yaml b/charts/topograph/templates/rbac.yaml index e069c682..3b3dabe4 100644 --- a/charts/topograph/templates/rbac.yaml +++ b/charts/topograph/templates/rbac.yaml @@ -1,4 +1,6 @@ {{- if .Values.rbac.create -}} +{{- $engine := .Values.engine.name -}} +{{- $provider := .Values.provider.name -}} {{- /* pods/exec is needed by providers that exec inside pods, and by Slinky only for legacy partition discovery through `scontrol show partition` in the controller @@ -6,11 +8,11 @@ pod (falling back to a login pod when present). Slinky skips that path when dynamic nodes are enabled or when a topology entry already supplies nodes, a pod selector, or the default flat topology. */ -}} -{{- $needsPodExec := dict "value" false -}} -{{- if eq .Values.provider.name "infiniband-k8s" -}} -{{- $_ := set $needsPodExec "value" true -}} +{{- $needsPodExec := false -}} +{{- if eq $provider "infiniband-k8s" -}} +{{- $needsPodExec = true -}} {{- end -}} -{{- if eq .Values.engine.name "slinky" -}} +{{- if eq $engine "slinky" -}} {{- $params := default dict .Values.engine.params -}} {{- if not (get $params "useDynamicNodes") -}} {{- range $topology := (default dict (get $params "topologies")) -}} @@ -20,41 +22,64 @@ already supplies nodes, a pod selector, or the default flat topology. {{- $hasPodSelector := and $podSelector (or (get $podSelector "matchLabels") (get $podSelector "matchExpressions")) -}} {{- $isDefaultFlat := and (get $spec "clusterDefault") (eq (get $spec "plugin") "topology/flat") (not (get $spec "partition")) -}} {{- if and (not $hasNodes) (not $hasPodSelector) (not $isDefaultFlat) -}} -{{- $_ := set $needsPodExec "value" true -}} +{{- $needsPodExec = true -}} {{- end -}} {{- end -}} {{- end -}} {{- end -}} +{{- /* +Gate the main ClusterRole rules by engine/provider. The API server only needs +Kubernetes API access when the selected engine or provider actually calls the +API. +*/ -}} +{{- $needsNodes := false -}} +{{- if or (eq $engine "k8s") (eq $engine "slinky") (eq $engine "nfd") (eq $provider "dra") (eq $provider "infiniband-k8s") -}} +{{- $needsNodes = true -}} +{{- end -}} +{{- $needsPods := false -}} +{{- if or (eq $engine "slinky") (eq $provider "infiniband-k8s") -}} +{{- $needsPods = true -}} +{{- end -}} +{{- $needsDaemonSets := false -}} +{{- if eq $provider "infiniband-k8s" -}} +{{- $needsDaemonSets = true -}} +{{- end -}} +{{- $needsConfigMaps := false -}} +{{- if eq $engine "slinky" -}} +{{- $needsConfigMaps = true -}} +{{- end -}} +{{- if or $needsNodes $needsPods $needsDaemonSets $needsConfigMaps $needsPodExec }} apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: name: {{ include "topograph.rbacName" . }} rules: +{{- if $needsPods }} - apiGroups: [""] resources: [pods] verbs: [list] -{{- if $needsPodExec.value }} +{{- end }} +{{- if $needsPodExec }} - apiGroups: [""] resources: [pods/exec] verbs: [create] {{- end }} +{{- if $needsNodes }} - apiGroups: [""] resources: [nodes] verbs: [get,list] -{{- if eq .Values.engine.name "k8s" }} +{{- if or (eq $engine "k8s") (eq $engine "slinky") }} - apiGroups: [""] resources: [nodes] verbs: [patch] {{- end }} -{{- if eq .Values.engine.name "slinky" }} -- apiGroups: [""] - resources: [nodes] - verbs: [patch] {{- end }} +{{- if $needsDaemonSets }} - apiGroups: [apps] resources: [daemonsets] verbs: [get] -{{- if eq .Values.engine.name "slinky" }} +{{- end }} +{{- if $needsConfigMaps }} - apiGroups: [""] resources: [configmaps] verbs: [create,get,update] @@ -73,7 +98,8 @@ roleRef: kind: ClusterRole name: {{ include "topograph.rbacName" . }} apiGroup: rbac.authorization.k8s.io -{{- if eq .Values.engine.name "nfd" }} +{{- end }} +{{- if eq $engine "nfd" }} --- apiVersion: rbac.authorization.k8s.io/v1 kind: Role diff --git a/charts/topograph/tests/__snapshot__/render_snapshot_test.yaml.snap b/charts/topograph/tests/__snapshot__/render_snapshot_test.yaml.snap index 5b72eca7..0aeaf6a7 100644 --- a/charts/topograph/tests/__snapshot__/render_snapshot_test.yaml.snap +++ b/charts/topograph/tests/__snapshot__/render_snapshot_test.yaml.snap @@ -413,12 +413,6 @@ renders default values.yaml: metadata: name: chart-ci-topograph rules: - - apiGroups: - - "" - resources: - - pods - verbs: - - list - apiGroups: - "" resources: @@ -432,12 +426,6 @@ renders default values.yaml: - nodes verbs: - patch - - apiGroups: - - apps - resources: - - daemonsets - verbs: - - get 15: | apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding @@ -1011,12 +999,6 @@ renders values.k8s.gateway-api-example.yaml: metadata: name: chart-ci-topograph rules: - - apiGroups: - - "" - resources: - - pods - verbs: - - list - apiGroups: - "" resources: @@ -1030,12 +1012,6 @@ renders values.k8s.gateway-api-example.yaml: - nodes verbs: - patch - - apiGroups: - - apps - resources: - - daemonsets - verbs: - - get 16: | apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding @@ -1623,12 +1599,6 @@ renders values.k8s.gcp-federated-workload-identity-example.yaml: metadata: name: chart-ci-topograph rules: - - apiGroups: - - "" - resources: - - pods - verbs: - - list - apiGroups: - "" resources: @@ -1642,12 +1612,6 @@ renders values.k8s.gcp-federated-workload-identity-example.yaml: - nodes verbs: - patch - - apiGroups: - - apps - resources: - - daemonsets - verbs: - - get 15: | apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding @@ -2221,12 +2185,6 @@ renders values.k8s.gcp-service-account-example.yaml: metadata: name: chart-ci-topograph rules: - - apiGroups: - - "" - resources: - - pods - verbs: - - list - apiGroups: - "" resources: @@ -2240,12 +2198,6 @@ renders values.k8s.gcp-service-account-example.yaml: - nodes verbs: - patch - - apiGroups: - - apps - resources: - - daemonsets - verbs: - - get 15: | apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding @@ -3455,12 +3407,6 @@ renders values.slinky.block-example.yaml: - nodes verbs: - patch - - apiGroups: - - apps - resources: - - daemonsets - verbs: - - get - apiGroups: - "" resources: @@ -4080,12 +4026,6 @@ renders values.slinky.partition-example.yaml: - nodes verbs: - patch - - apiGroups: - - apps - resources: - - daemonsets - verbs: - - get - apiGroups: - "" resources: @@ -4680,12 +4620,6 @@ renders values.slinky.tree-example.yaml: - nodes verbs: - patch - - apiGroups: - - apps - resources: - - daemonsets - verbs: - - get - apiGroups: - "" resources: diff --git a/charts/topograph/tests/rbac_test.yaml b/charts/topograph/tests/rbac_test.yaml index a72655ed..5a496575 100644 --- a/charts/topograph/tests/rbac_test.yaml +++ b/charts/topograph/tests/rbac_test.yaml @@ -62,15 +62,9 @@ tests: - hasDocuments: count: 0 - - it: grants core node and pod read access by default + - it: grants core node read/write access by default documentIndex: 0 asserts: - - contains: - path: rules - content: - apiGroups: [""] - resources: [pods] - verbs: [list] - contains: path: rules content: @@ -83,12 +77,6 @@ tests: apiGroups: [""] resources: [nodes] verbs: [patch] - - contains: - path: rules - content: - apiGroups: [apps] - resources: [daemonsets] - verbs: [get] - it: does not grant provider- or engine-specific access by default documentIndex: 0 @@ -117,14 +105,59 @@ tests: apiGroups: [""] resources: [pods] verbs: [get, list] + - notContains: + path: rules + content: + apiGroups: [""] + resources: [pods] + verbs: [list] - notContains: path: rules content: apiGroups: [apps] resources: [daemonsets] verbs: [get, list] + - notContains: + path: rules + content: + apiGroups: [apps] + resources: [daemonsets] + verbs: [get] - - it: grants pods/exec for the infiniband-k8s provider + - it: grants node read without node write for the dra provider + set: + provider: + name: dra + engine: + name: slurm + documentIndex: 0 + asserts: + - contains: + path: rules + content: + apiGroups: [""] + resources: [nodes] + verbs: [get, list] + - notContains: + path: rules + content: + apiGroups: [""] + resources: [nodes] + verbs: [patch] + - notContains: + path: rules + content: + apiGroups: [""] + resources: [pods] + verbs: [list] + - notContains: + path: rules + content: + apiGroups: [apps] + resources: [daemonsets] + verbs: [get] + + - it: grants pods, daemonsets, and pods/exec for the infiniband-k8s provider set: provider: name: infiniband-k8s @@ -136,8 +169,20 @@ tests: apiGroups: [""] resources: [pods/exec] verbs: [create] + - contains: + path: rules + content: + apiGroups: [""] + resources: [pods] + verbs: [list] + - contains: + path: rules + content: + apiGroups: [apps] + resources: [daemonsets] + verbs: [get] - - it: grants configmaps for the slinky engine + - it: grants configmaps and pods for the slinky engine set: engine: name: slinky @@ -161,6 +206,18 @@ tests: apiGroups: [""] resources: [nodes] verbs: [patch] + - contains: + path: rules + content: + apiGroups: [""] + resources: [pods] + verbs: [list] + - notContains: + path: rules + content: + apiGroups: [apps] + resources: [daemonsets] + verbs: [get] - it: grants node patch access to the k8s engine set: @@ -181,18 +238,32 @@ tests: resources: [nodes] verbs: [update] - - it: does not grant node patch access to the slurm engine + - it: omits the ClusterRole and ClusterRoleBinding for a non-Kubernetes combo set: + provider: + name: test engine: name: slurm - documentIndex: 0 asserts: + - hasDocuments: + count: 0 + + - it: still renders the ClusterRole for the slurm engine with a Kubernetes provider + set: + provider: + name: infiniband-k8s + engine: + name: slurm + asserts: + - hasDocuments: + count: 2 - notContains: path: rules content: apiGroups: [""] resources: [nodes] verbs: [patch] + documentIndex: 0 - it: does not grant node update access to the slinky engine set: @@ -254,6 +325,20 @@ tests: resources: [nodes] verbs: [patch] documentIndex: 0 + - notContains: + path: rules + content: + apiGroups: [""] + resources: [pods] + verbs: [list] + documentIndex: 0 + - contains: + path: rules + content: + apiGroups: [""] + resources: [nodes] + verbs: [get, list] + documentIndex: 0 - isKind: of: Role documentIndex: 2