diff --git a/CHANGELOG.md b/CHANGELOG.md index 051f9938..0a5da707 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### 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. +- Node RBAC permissions in the main Topograph API server ClusterRole are collapsed into a single rule following least privilege: the Kubernetes engine receives Node `[get, list, patch]`, the dynamic Slinky engine (`useDynamicNodes: true`) receives Node `[list, patch]`, and other node-consuming configurations receive Node `[list]` without unnecessary `patch`, `get`, or `update` access. --- diff --git a/charts/topograph/templates/rbac.yaml b/charts/topograph/templates/rbac.yaml index 3b3dabe4..753ddfe1 100644 --- a/charts/topograph/templates/rbac.yaml +++ b/charts/topograph/templates/rbac.yaml @@ -65,14 +65,16 @@ rules: verbs: [create] {{- end }} {{- if $needsNodes }} +{{- $nodeVerbs := list "list" -}} +{{- if eq $engine "k8s" -}} +{{- $nodeVerbs = list "get" "list" "patch" -}} +{{- end -}} +{{- if and (eq $engine "slinky") (get (default dict .Values.engine.params) "useDynamicNodes") -}} +{{- $nodeVerbs = list "list" "patch" -}} +{{- end }} - apiGroups: [""] resources: [nodes] - verbs: [get,list] -{{- if or (eq $engine "k8s") (eq $engine "slinky") }} -- apiGroups: [""] - resources: [nodes] - verbs: [patch] -{{- end }} + verbs: {{ $nodeVerbs | toJson }} {{- end }} {{- if $needsDaemonSets }} - apiGroups: [apps] diff --git a/charts/topograph/tests/__snapshot__/render_snapshot_test.yaml.snap b/charts/topograph/tests/__snapshot__/render_snapshot_test.yaml.snap index 7b3d83e7..dca2bd17 100644 --- a/charts/topograph/tests/__snapshot__/render_snapshot_test.yaml.snap +++ b/charts/topograph/tests/__snapshot__/render_snapshot_test.yaml.snap @@ -420,11 +420,6 @@ renders default values.yaml: verbs: - get - list - - apiGroups: - - "" - resources: - - nodes - verbs: - patch 15: | apiVersion: rbac.authorization.k8s.io/v1 @@ -1006,11 +1001,6 @@ renders values.k8s.gateway-api-example.yaml: verbs: - get - list - - apiGroups: - - "" - resources: - - nodes - verbs: - patch 16: | apiVersion: rbac.authorization.k8s.io/v1 @@ -1606,11 +1596,6 @@ renders values.k8s.gcp-federated-workload-identity-example.yaml: verbs: - get - list - - apiGroups: - - "" - resources: - - nodes - verbs: - patch 15: | apiVersion: rbac.authorization.k8s.io/v1 @@ -2192,11 +2177,6 @@ renders values.k8s.gcp-service-account-example.yaml: verbs: - get - list - - apiGroups: - - "" - resources: - - nodes - verbs: - patch 15: | apiVersion: rbac.authorization.k8s.io/v1 @@ -2798,11 +2778,6 @@ renders values.k8s.ib-example.yaml: verbs: - get - list - - apiGroups: - - "" - resources: - - nodes - verbs: - patch - apiGroups: - apps @@ -3399,14 +3374,7 @@ renders values.slinky.block-example.yaml: resources: - nodes verbs: - - get - list - - apiGroups: - - "" - resources: - - nodes - verbs: - - patch - apiGroups: - "" resources: @@ -4018,13 +3986,7 @@ renders values.slinky.partition-example.yaml: resources: - nodes verbs: - - get - list - - apiGroups: - - "" - resources: - - nodes - verbs: - patch - apiGroups: - "" @@ -4612,14 +4574,7 @@ renders values.slinky.tree-example.yaml: resources: - nodes verbs: - - get - list - - apiGroups: - - "" - resources: - - nodes - verbs: - - patch - apiGroups: - "" resources: diff --git a/charts/topograph/tests/rbac_test.yaml b/charts/topograph/tests/rbac_test.yaml index 5a496575..efd8d9b9 100644 --- a/charts/topograph/tests/rbac_test.yaml +++ b/charts/topograph/tests/rbac_test.yaml @@ -62,7 +62,7 @@ tests: - hasDocuments: count: 0 - - it: grants core node read/write access by default + - it: grants core node read and patch access by default documentIndex: 0 asserts: - contains: @@ -70,13 +70,25 @@ tests: content: apiGroups: [""] resources: [nodes] - verbs: [get, list] - - contains: + verbs: [get, list, patch] + - notContains: path: rules content: apiGroups: [""] resources: [nodes] - verbs: [patch] + verbs: [update] + - notContains: + path: rules + content: + apiGroups: [""] + resources: [pods] + verbs: [list] + - notContains: + path: rules + content: + apiGroups: [apps] + resources: [daemonsets] + verbs: [get] - it: does not grant provider- or engine-specific access by default documentIndex: 0 @@ -105,26 +117,14 @@ 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 node read without node write for the dra provider + - it: grants node read access for the dra provider set: provider: name: dra @@ -137,13 +137,25 @@ tests: content: apiGroups: [""] resources: [nodes] - verbs: [get, list] + verbs: [list] + - notContains: + path: rules + content: + apiGroups: [""] + resources: [nodes] + verbs: [list, patch] + - notContains: + path: rules + content: + apiGroups: [""] + resources: [nodes] + verbs: [get, list, patch] - notContains: path: rules content: apiGroups: [""] resources: [nodes] - verbs: [patch] + verbs: [update] - notContains: path: rules content: @@ -182,7 +194,7 @@ tests: resources: [daemonsets] verbs: [get] - - it: grants configmaps and pods for the slinky engine + - it: grants configmaps, pods, and node list for the slinky engine set: engine: name: slinky @@ -205,7 +217,19 @@ tests: content: apiGroups: [""] resources: [nodes] - verbs: [patch] + verbs: [list] + - notContains: + path: rules + content: + apiGroups: [""] + resources: [nodes] + verbs: [list, patch] + - notContains: + path: rules + content: + apiGroups: [""] + resources: [nodes] + verbs: [get, list, patch] - contains: path: rules content: @@ -230,7 +254,7 @@ tests: content: apiGroups: [""] resources: [nodes] - verbs: [patch] + verbs: [get, list, patch] - notContains: path: rules content: @@ -238,37 +262,70 @@ tests: resources: [nodes] verbs: [update] - - it: omits the ClusterRole and ClusterRoleBinding for a non-Kubernetes combo + - it: does not grant node patch access to the slurm engine set: provider: - name: test + name: dra engine: name: slurm + documentIndex: 0 asserts: - - hasDocuments: - count: 0 + - contains: + path: rules + content: + apiGroups: [""] + resources: [nodes] + verbs: [list] + - notContains: + path: rules + content: + apiGroups: [""] + resources: [nodes] + verbs: [list, patch] + - notContains: + path: rules + content: + apiGroups: [""] + resources: [nodes] + verbs: [get, list, patch] - - it: still renders the ClusterRole for the slurm engine with a Kubernetes provider + - it: does not grant node patch or update access to the non-dynamic slinky engine set: - provider: - name: infiniband-k8s engine: - name: slurm + name: slinky + documentIndex: 0 asserts: - - hasDocuments: - count: 2 + - contains: + path: rules + content: + apiGroups: [""] + resources: [nodes] + verbs: [list] - notContains: path: rules content: apiGroups: [""] resources: [nodes] - verbs: [patch] - documentIndex: 0 + verbs: [list, patch] + - notContains: + path: rules + content: + apiGroups: [""] + resources: [nodes] + verbs: [get, list, patch] + - notContains: + path: rules + content: + apiGroups: [""] + resources: [nodes] + verbs: [update] - - it: does not grant node update access to the slinky engine + - it: grants node patch access to the dynamic slinky engine set: engine: name: slinky + params: + useDynamicNodes: true documentIndex: 0 asserts: - contains: @@ -276,13 +333,13 @@ tests: content: apiGroups: [""] resources: [nodes] - verbs: [get, list] - - contains: + verbs: [list, patch] + - notContains: path: rules content: apiGroups: [""] resources: [nodes] - verbs: [patch] + verbs: [get, list, patch] - notContains: path: rules content: @@ -290,6 +347,47 @@ tests: resources: [nodes] verbs: [update] + - it: omits the ClusterRole and ClusterRoleBinding for a non-Kubernetes combo + set: + provider: + name: test + engine: + name: slurm + 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 + - contains: + path: rules + content: + apiGroups: [""] + resources: [nodes] + verbs: [list] + documentIndex: 0 + - notContains: + path: rules + content: + apiGroups: [""] + resources: [nodes] + verbs: [list, patch] + documentIndex: 0 + - notContains: + path: rules + content: + apiGroups: [""] + resources: [nodes] + verbs: [get, list, patch] + documentIndex: 0 + - it: does not render NFD namespace RBAC for an explicit non-NFD engine set: engine: @@ -318,26 +416,33 @@ tests: resources: [nodefeatures, nodefeaturegroups] verbs: [create, list, patch, delete] documentIndex: 0 - - notContains: + - contains: path: rules content: apiGroups: [""] resources: [nodes] - verbs: [patch] + verbs: [list] documentIndex: 0 - notContains: path: rules content: apiGroups: [""] - resources: [pods] - verbs: [list] + resources: [nodes] + verbs: [list, patch] documentIndex: 0 - - contains: + - notContains: path: rules content: apiGroups: [""] resources: [nodes] - verbs: [get, list] + verbs: [get, list, patch] + documentIndex: 0 + - notContains: + path: rules + content: + apiGroups: [""] + resources: [pods] + verbs: [list] documentIndex: 0 - isKind: of: Role