From 084914d30c2a676b5277828c06a02040cef471e7 Mon Sep 17 00:00:00 2001 From: Shankar V Date: Fri, 17 Jul 2026 21:47:06 +0530 Subject: [PATCH 1/3] Add opt-in ValidatingAdmissionPolicy for node-data-broker Signed-off-by: Shankar V --- CHANGELOG.md | 1 + .../validatingadmissionpolicy.yaml | 33 ++++++ ...broker_validatingadmissionpolicy_test.yaml | 101 ++++++++++++++++++ charts/topograph/values.schema.json | 6 ++ charts/topograph/values.yaml | 5 + docs/engines/k8s.md | 16 +++ 6 files changed, 162 insertions(+) create mode 100644 charts/topograph/templates/nodeDataBroker/validatingadmissionpolicy.yaml create mode 100644 charts/topograph/tests/node-data-broker_validatingadmissionpolicy_test.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index df5e87ea..74d59ddf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Security +- Opt-in `ValidatingAdmissionPolicy` and binding to restrict the `node-data-broker` ServiceAccount to only updating its host Node resource, mitigating privilege escalation via node spec or annotation manipulation (audit F1 compliance). - Removed unused RBAC verbs from the Topograph API server and node-data-broker ClusterRoles (least-privilege): API server `pods` rule dropped `get` (list-only), `daemonsets` rule dropped `list` (get-only), and the Slinky `configmaps` rule dropped `list`; node-data-broker `nodes` rule dropped `list` (get/update), and the InfiniBand `daemonsets`/`pods` rules dropped `list`/`get` respectively (get-only, list-only). - node-observer ClusterRole no longer grants unused `get`; `nodes` list/watch now gated on `trigger.nodeSelector`. diff --git a/charts/topograph/templates/nodeDataBroker/validatingadmissionpolicy.yaml b/charts/topograph/templates/nodeDataBroker/validatingadmissionpolicy.yaml new file mode 100644 index 00000000..8f5839cf --- /dev/null +++ b/charts/topograph/templates/nodeDataBroker/validatingadmissionpolicy.yaml @@ -0,0 +1,33 @@ +{{- if and .Values.nodeDataBroker.enabled .Values.nodeDataBroker.validatingAdmissionPolicy.enabled }} +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingAdmissionPolicy +metadata: + name: {{ include "nodeDataBroker.fullname" . }} + labels: + {{- include "nodeDataBroker.labels" . | nindent 4 }} +spec: + failurePolicy: Fail + matchConstraints: + resourceRules: + - apiGroups: [""] + apiVersions: ["v1"] + operations: ["UPDATE"] + resources: ["nodes"] + validations: + - expression: >- + request.userInfo.username != 'system:serviceaccount:{{ .Release.Namespace }}:{{ include "nodeDataBroker.serviceAccountName" . }}' || + (has(request.userInfo.extra) && + has(request.userInfo.extra['authentication.kubernetes.io/node-name']) && + request.name == request.userInfo.extra['authentication.kubernetes.io/node-name'][0]) + message: "node-data-broker ServiceAccount is only authorized to update its own Node resource (audit F1)." +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingAdmissionPolicyBinding +metadata: + name: {{ include "nodeDataBroker.fullname" . }} + labels: + {{- include "nodeDataBroker.labels" . | nindent 4 }} +spec: + policyName: {{ include "nodeDataBroker.fullname" . }} + validationActions: [Deny] +{{- end }} diff --git a/charts/topograph/tests/node-data-broker_validatingadmissionpolicy_test.yaml b/charts/topograph/tests/node-data-broker_validatingadmissionpolicy_test.yaml new file mode 100644 index 00000000..a8778e0b --- /dev/null +++ b/charts/topograph/tests/node-data-broker_validatingadmissionpolicy_test.yaml @@ -0,0 +1,101 @@ +suite: node-data-broker validating admission policy +templates: + - templates/nodeDataBroker/validatingadmissionpolicy.yaml +release: + name: chart-ci + namespace: topograph +capabilities: + majorVersion: "1" + minorVersion: "30" +tests: + - it: is omitted by default + asserts: + - hasDocuments: + count: 0 + + - it: is omitted when the component is disabled + set: + nodeDataBroker: + enabled: false + validatingAdmissionPolicy: + enabled: true + asserts: + - hasDocuments: + count: 0 + + - it: renders both resources when enabled + set: + nodeDataBroker: + enabled: true + validatingAdmissionPolicy: + enabled: true + asserts: + - hasDocuments: + count: 2 + + - it: renders correct API versions and kinds for ValidatingAdmissionPolicy + set: + nodeDataBroker: + enabled: true + validatingAdmissionPolicy: + enabled: true + documentIndex: 0 + asserts: + - isKind: + of: ValidatingAdmissionPolicy + - equal: + path: apiVersion + value: admissionregistration.k8s.io/v1 + - equal: + path: metadata.name + value: chart-ci-topograph-node-data-broker + - equal: + path: spec.failurePolicy + value: Fail + + - it: renders correct API versions and kinds for ValidatingAdmissionPolicyBinding + set: + nodeDataBroker: + enabled: true + validatingAdmissionPolicy: + enabled: true + documentIndex: 1 + asserts: + - isKind: + of: ValidatingAdmissionPolicyBinding + - equal: + path: apiVersion + value: admissionregistration.k8s.io/v1 + - equal: + path: metadata.name + value: chart-ci-topograph-node-data-broker + - equal: + path: spec.policyName + value: chart-ci-topograph-node-data-broker + - equal: + path: spec.validationActions[0] + value: Deny + + - it: configures the CEL expression and dynamic ServiceAccount matching + set: + nodeDataBroker: + enabled: true + validatingAdmissionPolicy: + enabled: true + serviceAccount: + create: true + name: custom-node-data-broker-sa + documentIndex: 0 + asserts: + - contains: + path: spec.validations[0].expression + content: "request.userInfo.username != 'system:serviceaccount:topograph:custom-node-data-broker-sa'" + - contains: + path: spec.validations[0].expression + content: "authentication.kubernetes.io/node-name" + - contains: + path: spec.validations[0].expression + content: "request.name" + - equal: + path: spec.validations[0].message + value: "node-data-broker ServiceAccount is only authorized to update its own Node resource (audit F1)." diff --git a/charts/topograph/values.schema.json b/charts/topograph/values.schema.json index a3ac793f..4886e849 100644 --- a/charts/topograph/values.schema.json +++ b/charts/topograph/values.schema.json @@ -228,6 +228,12 @@ "startupProbe": { "type": "object" }, "serviceAccount": { "type": "object" }, "rbac": { "type": "object" }, + "validatingAdmissionPolicy": { + "type": "object", + "properties": { + "enabled": { "type": "boolean" } + } + }, "env": { "type": "object" }, "initContainers": { "type": "array" }, "lifecycle": { "type": "object" }, diff --git a/charts/topograph/values.yaml b/charts/topograph/values.yaml index f79b4739..e27da55f 100644 --- a/charts/topograph/values.yaml +++ b/charts/topograph/values.yaml @@ -312,6 +312,11 @@ nodeDataBroker: rbac: create: true + # Opt-in ValidatingAdmissionPolicy to scope node writes. Requires + # Kubernetes >= 1.26 (GA in 1.30) and ServiceAccountTokenNodeBinding enabled. + validatingAdmissionPolicy: + enabled: false + env: {} initContainers: [] lifecycle: {} diff --git a/docs/engines/k8s.md b/docs/engines/k8s.md index f99fd23e..2f276bcf 100644 --- a/docs/engines/k8s.md +++ b/docs/engines/k8s.md @@ -302,6 +302,22 @@ Apply alongside the chart. A bundled template is under consideration. The node-observer `ClusterRole` grants `pods [list, watch]` unconditionally, `apps/daemonsets [list, watch]` when `nodeDataBroker.enabled=true`, and `nodes [list, watch]` only when `nodeObserver.topograph.trigger.nodeSelector` is set. Set `nodeObserver.rbac.create: false` to suppress the `ClusterRole`/`ClusterRoleBinding` when managing RBAC externally. +### Node Data Broker write-scoping via ValidatingAdmissionPolicy + +The node-data-broker's RBAC grants update permission on nodes cluster-wide. To enforce least-privilege scoping, the chart supports deploying an opt-in `ValidatingAdmissionPolicy` and its binding: + +```yaml +nodeDataBroker: + validatingAdmissionPolicy: + enabled: true +``` + +When enabled, the policy matches node `UPDATE` operations performed by the broker's ServiceAccount and validates that the name of the node being modified matches the node name claim bound to the requester's ServiceAccount token (`authentication.kubernetes.io/node-name`). + +**Prerequisites:** +1. A Kubernetes cluster with the **`ValidatingAdmissionPolicy` API enabled** (`admissionregistration.k8s.io/v1`). +2. ServiceAccount tokens carrying the node-binding identity claim (e.g., via `ServiceAccountTokenNodeBinding`). The policy denies updates from the broker ServiceAccount if this required node-name claim is missing from the request context. + ## Validation and Testing The Helm chart ships two layers of validation for operators. From fb20ac3301b6606a1a2416712d7f8582456993cb Mon Sep 17 00:00:00 2001 From: Shankar V Date: Fri, 17 Jul 2026 22:09:20 +0530 Subject: [PATCH 2/3] Address automated review feedback Signed-off-by: Shankar V --- .../nodeDataBroker/validatingadmissionpolicy.yaml | 1 + ...e-data-broker_validatingadmissionpolicy_test.yaml | 12 ++++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/charts/topograph/templates/nodeDataBroker/validatingadmissionpolicy.yaml b/charts/topograph/templates/nodeDataBroker/validatingadmissionpolicy.yaml index 8f5839cf..ae51d6b4 100644 --- a/charts/topograph/templates/nodeDataBroker/validatingadmissionpolicy.yaml +++ b/charts/topograph/templates/nodeDataBroker/validatingadmissionpolicy.yaml @@ -18,6 +18,7 @@ spec: request.userInfo.username != 'system:serviceaccount:{{ .Release.Namespace }}:{{ include "nodeDataBroker.serviceAccountName" . }}' || (has(request.userInfo.extra) && has(request.userInfo.extra['authentication.kubernetes.io/node-name']) && + size(request.userInfo.extra['authentication.kubernetes.io/node-name']) > 0 && request.name == request.userInfo.extra['authentication.kubernetes.io/node-name'][0]) message: "node-data-broker ServiceAccount is only authorized to update its own Node resource (audit F1)." --- diff --git a/charts/topograph/tests/node-data-broker_validatingadmissionpolicy_test.yaml b/charts/topograph/tests/node-data-broker_validatingadmissionpolicy_test.yaml index a8778e0b..9d1d18a2 100644 --- a/charts/topograph/tests/node-data-broker_validatingadmissionpolicy_test.yaml +++ b/charts/topograph/tests/node-data-broker_validatingadmissionpolicy_test.yaml @@ -87,15 +87,15 @@ tests: name: custom-node-data-broker-sa documentIndex: 0 asserts: - - contains: + - matchRegex: path: spec.validations[0].expression - content: "request.userInfo.username != 'system:serviceaccount:topograph:custom-node-data-broker-sa'" - - contains: + pattern: "request\\.userInfo\\.username != 'system:serviceaccount:topograph:custom-node-data-broker-sa'" + - matchRegex: path: spec.validations[0].expression - content: "authentication.kubernetes.io/node-name" - - contains: + pattern: "authentication\\.kubernetes\\.io/node-name" + - matchRegex: path: spec.validations[0].expression - content: "request.name" + pattern: "request\\.name" - equal: path: spec.validations[0].message value: "node-data-broker ServiceAccount is only authorized to update its own Node resource (audit F1)." From 733939ac99d57e1d01f1817dd7c960cdb1bf5cd1 Mon Sep 17 00:00:00 2001 From: Shankar V Date: Tue, 21 Jul 2026 20:47:47 +0530 Subject: [PATCH 3/3] fix: address review feedback for node-data-broker admission policy Signed-off-by: Shankar V --- .../validatingadmissionpolicy.yaml | 11 ++++++---- ...broker_validatingadmissionpolicy_test.yaml | 20 +++++++++++-------- charts/topograph/values.yaml | 2 +- docs/engines/k8s.md | 4 ++-- 4 files changed, 22 insertions(+), 15 deletions(-) diff --git a/charts/topograph/templates/nodeDataBroker/validatingadmissionpolicy.yaml b/charts/topograph/templates/nodeDataBroker/validatingadmissionpolicy.yaml index ae51d6b4..2a7bc115 100644 --- a/charts/topograph/templates/nodeDataBroker/validatingadmissionpolicy.yaml +++ b/charts/topograph/templates/nodeDataBroker/validatingadmissionpolicy.yaml @@ -13,13 +13,16 @@ spec: apiVersions: ["v1"] operations: ["UPDATE"] resources: ["nodes"] + matchConditions: + - name: match-node-data-broker-serviceaccount + expression: >- + request.userInfo.username == 'system:serviceaccount:{{ .Release.Namespace }}:{{ include "nodeDataBroker.serviceAccountName" . }}' validations: - expression: >- - request.userInfo.username != 'system:serviceaccount:{{ .Release.Namespace }}:{{ include "nodeDataBroker.serviceAccountName" . }}' || - (has(request.userInfo.extra) && - has(request.userInfo.extra['authentication.kubernetes.io/node-name']) && + has(request.userInfo.extra) && + 'authentication.kubernetes.io/node-name' in request.userInfo.extra && size(request.userInfo.extra['authentication.kubernetes.io/node-name']) > 0 && - request.name == request.userInfo.extra['authentication.kubernetes.io/node-name'][0]) + request.name == request.userInfo.extra['authentication.kubernetes.io/node-name'][0] message: "node-data-broker ServiceAccount is only authorized to update its own Node resource (audit F1)." --- apiVersion: admissionregistration.k8s.io/v1 diff --git a/charts/topograph/tests/node-data-broker_validatingadmissionpolicy_test.yaml b/charts/topograph/tests/node-data-broker_validatingadmissionpolicy_test.yaml index 9d1d18a2..32dc7726 100644 --- a/charts/topograph/tests/node-data-broker_validatingadmissionpolicy_test.yaml +++ b/charts/topograph/tests/node-data-broker_validatingadmissionpolicy_test.yaml @@ -87,15 +87,19 @@ tests: name: custom-node-data-broker-sa documentIndex: 0 asserts: - - matchRegex: - path: spec.validations[0].expression - pattern: "request\\.userInfo\\.username != 'system:serviceaccount:topograph:custom-node-data-broker-sa'" - - matchRegex: - path: spec.validations[0].expression - pattern: "authentication\\.kubernetes\\.io/node-name" - - matchRegex: + - equal: + path: spec.matchConditions[0].name + value: match-node-data-broker-serviceaccount + - equal: + path: spec.matchConditions[0].expression + value: "request.userInfo.username == 'system:serviceaccount:topograph:custom-node-data-broker-sa'" + - equal: path: spec.validations[0].expression - pattern: "request\\.name" + value: >- + has(request.userInfo.extra) && + 'authentication.kubernetes.io/node-name' in request.userInfo.extra && + size(request.userInfo.extra['authentication.kubernetes.io/node-name']) > 0 && + request.name == request.userInfo.extra['authentication.kubernetes.io/node-name'][0] - equal: path: spec.validations[0].message value: "node-data-broker ServiceAccount is only authorized to update its own Node resource (audit F1)." diff --git a/charts/topograph/values.yaml b/charts/topograph/values.yaml index e27da55f..5c59cbbd 100644 --- a/charts/topograph/values.yaml +++ b/charts/topograph/values.yaml @@ -313,7 +313,7 @@ nodeDataBroker: create: true # Opt-in ValidatingAdmissionPolicy to scope node writes. Requires - # Kubernetes >= 1.26 (GA in 1.30) and ServiceAccountTokenNodeBinding enabled. + # Kubernetes >= 1.30 (admissionregistration.k8s.io/v1) and ServiceAccountTokenPodNodeInfo enabled. validatingAdmissionPolicy: enabled: false diff --git a/docs/engines/k8s.md b/docs/engines/k8s.md index 2f276bcf..c2032c4d 100644 --- a/docs/engines/k8s.md +++ b/docs/engines/k8s.md @@ -315,8 +315,8 @@ nodeDataBroker: When enabled, the policy matches node `UPDATE` operations performed by the broker's ServiceAccount and validates that the name of the node being modified matches the node name claim bound to the requester's ServiceAccount token (`authentication.kubernetes.io/node-name`). **Prerequisites:** -1. A Kubernetes cluster with the **`ValidatingAdmissionPolicy` API enabled** (`admissionregistration.k8s.io/v1`). -2. ServiceAccount tokens carrying the node-binding identity claim (e.g., via `ServiceAccountTokenNodeBinding`). The policy denies updates from the broker ServiceAccount if this required node-name claim is missing from the request context. +1. A Kubernetes cluster running **Kubernetes 1.30+** with the **`ValidatingAdmissionPolicy` API enabled** (`admissionregistration.k8s.io/v1`). +2. ServiceAccount tokens carrying the node-binding identity claim (e.g., via `ServiceAccountTokenPodNodeInfo`). The policy denies updates from the broker ServiceAccount if this required node-name claim is missing from the request context. ## Validation and Testing