From adfb077cbee0e89f038042c8c12bfa8b8833455b Mon Sep 17 00:00:00 2001 From: Tyler Bray Date: Mon, 6 Jul 2026 09:44:41 -0700 Subject: [PATCH 1/4] feat(deployments): grant k8s backend RBAC in deploy chart (AIRCORE-757 phase 6) Extends the nmp-core controller Role with the two permissions the Phase 5 k8s DeploymentBackend needs but didn't have: create/delete on apps/v1 Deployments, and a new services rule (get/list/create/delete). The DeploymentsController runs inside the existing nmp-core controller pod, so this reuses that pod's ServiceAccount/Role rather than adding a parallel one. Also documents the k8s executor config and RBAC scope in the plugin README, and adds a unit test asserting the chart's rendered RBAC rules. Signed-off-by: Tyler Bray --- k8s/helm/templates/core/controller-role.yaml | 5 +- plugins/nemo-deployments/README.md | 49 ++++++++++++ .../unit/backends/k8s/test_rbac_manifest.py | 75 +++++++++++++++++++ 3 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 plugins/nemo-deployments/tests/unit/backends/k8s/test_rbac_manifest.py diff --git a/k8s/helm/templates/core/controller-role.yaml b/k8s/helm/templates/core/controller-role.yaml index b3a5f3604a..712eb2fb96 100644 --- a/k8s/helm/templates/core/controller-role.yaml +++ b/k8s/helm/templates/core/controller-role.yaml @@ -18,7 +18,7 @@ rules: verbs: ["get", "list"] - apiGroups: ["apps"] resources: ["deployments"] - verbs: ["get", "list", "watch"] + verbs: ["get", "list", "watch", "create", "delete"] - apiGroups: ["batch"] resources: ["jobs"] verbs: ["create", "get", "list", "watch", "update", "patch", "delete"] @@ -28,6 +28,9 @@ rules: - apiGroups: [""] resources: ["configmaps"] verbs: ["get", "list", "create", "delete"] +- apiGroups: [""] + resources: ["services"] + verbs: ["get", "list", "create", "delete"] - apiGroups: [""] resources: ["secrets"] verbs: ["create", "delete"] diff --git a/plugins/nemo-deployments/README.md b/plugins/nemo-deployments/README.md index 6b2abf8d9e..c011078d04 100644 --- a/plugins/nemo-deployments/README.md +++ b/plugins/nemo-deployments/README.md @@ -56,3 +56,52 @@ from the hyphen-joined string, so pairs like ``foo``/``bar-baz`` and ``nemo_platform_plugin.k8s_naming`` (same module used by the models service). Orphan cleanup matches identity labels, not names alone; existing containers keep their old names after upgrade. + +## Kubernetes executors + +The k8s backend emits native `apps/v1.Deployment` + `v1.Service` for +`restart_policy: Always` workloads, `batch/v1.Job` for finite (`Never`/ +`OnFailure`) workloads, and `v1.PersistentVolumeClaim` for volumes — no +`k8s-nim-operator` dependency. Configure a named executor in platform YAML: + +```yaml +deployments: + executors: + - name: local-k8s + backend: k8s + config: + kubeconfig_path: /path/to/kubeconfig # unset: in-cluster config, then default kubeconfig + default_namespace: tbray-dev + request_timeout: 60 + default_executor: local-k8s +``` + +Entity-level `backend_config.k8s.namespace` overrides `default_namespace` per +deployment/volume; it must be a namespace the controller's ServiceAccount has +RBAC in (see below). + +### RBAC + +The `DeploymentsController` runs inside the `nmp-core` controller pod +(registered via the `nemo.controllers` entry point), so it reuses that pod's +existing ServiceAccount and Role rather than a dedicated one. The deploy +chart's `k8s/helm/templates/core/controller-role.yaml` grants that Role the +verbs the k8s backend needs in `.Release.Namespace`: `get`/`list`/`watch` on +pods and pods/log, `create`/`get`/`list`/`watch`/`update`/`patch`/`delete` on +`batch/v1.Job`, `get`/`list`/`create`/`delete` on PVCs, ConfigMaps, and +Services, and `get`/`list`/`watch`/`create`/`delete` on `apps/v1.Deployment`. + +This Role is namespace-scoped to the release namespace. Pointing +`backend_config.k8s.namespace` at a namespace outside the release namespace +requires additional RBAC (a RoleBinding in that namespace, or a +ClusterRole/ClusterRoleBinding) that the chart does not provision today; +namespace-per-workspace provisioning is a documented future enhancement. + +### Native sidecars + +The LoRA-adapter-style native sidecar pattern (an `init_containers` entry with +`restart_policy: "Always"`) requires Kubernetes >= 1.29. On older clusters, +omit the per-container `restart_policy` on init containers and run that +container as a regular main container instead — the compiler does not +fall back to legacy sidecar emulation (emptyDir readiness files, etc.) +automatically. diff --git a/plugins/nemo-deployments/tests/unit/backends/k8s/test_rbac_manifest.py b/plugins/nemo-deployments/tests/unit/backends/k8s/test_rbac_manifest.py new file mode 100644 index 0000000000..4ecd52c6ff --- /dev/null +++ b/plugins/nemo-deployments/tests/unit/backends/k8s/test_rbac_manifest.py @@ -0,0 +1,75 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Assert the deploy chart's controller Role grants the k8s backend's required RBAC. + +The chart under ``k8s/helm`` declares an unconditional ``k8s-nim-operator`` chart +dependency, so ``helm template`` cannot render without that subchart present in +``charts/`` (fetched from an NGC repo) — a network dependency this suite must not +require. Assertions instead parse the static YAML rule entries directly out of the +Go-template source, which is safe because the base RBAC rules (unlike the +Volcano/NIM-Operator blocks) are plain YAML with no Helm expressions inside them. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + + +def _repo_root() -> Path: + for parent in Path(__file__).resolve().parents: + if (parent / "k8s" / "helm" / "templates" / "core" / "controller-role.yaml").is_file(): + return parent + raise FileNotFoundError("Could not locate repo root containing k8s/helm/templates/core/controller-role.yaml") + + +def _controller_role_source() -> str: + role_path = _repo_root() / "k8s" / "helm" / "templates" / "core" / "controller-role.yaml" + content = role_path.read_text() + # Only the Role definition (before the "---" separator) is relevant; the + # RoleBinding that follows has no `rules:` block to collide with. + role_section, _, _ = content.partition("\n---\n") + return role_section + + +def _rule_verbs(content: str, *, api_group: str, resource: str) -> list[str]: + """Extract the verbs list for a ``- apiGroups: [...]\\n resources: [...]`` rule.""" + pattern = re.compile( + r'apiGroups:\s*\[\s*"{}"\s*\]\s*\n\s*resources:\s*\[\s*"{}"\s*\]\s*\n\s*verbs:\s*\[(.*?)\]'.format( + re.escape(api_group), re.escape(resource) + ) + ) + match = pattern.search(content) + assert match is not None, f"No rule found for apiGroups={api_group!r} resources={resource!r}" + return [verb.strip().strip('"') for verb in match.group(1).split(",")] + + +def test_deployments_rule_grants_create_and_delete() -> None: + """The k8s backend creates and deletes apps/v1.Deployment objects.""" + verbs = _rule_verbs(_controller_role_source(), api_group="apps", resource="deployments") + assert {"get", "list", "watch", "create", "delete"} <= set(verbs) + + +def test_services_rule_present() -> None: + """The k8s backend creates a v1.Service alongside restart_policy=Always Deployments.""" + verbs = _rule_verbs(_controller_role_source(), api_group="", resource="services") + assert {"get", "list", "create", "delete"} <= set(verbs) + + +@pytest.mark.parametrize( + ("api_group", "resource", "required_verbs"), + [ + ("", "pods", {"get", "list", "watch"}), + ("", "pods/log", {"get", "list"}), + ("batch", "jobs", {"create", "get", "list", "watch", "update", "patch", "delete"}), + ("", "persistentvolumeclaims", {"get", "list", "create", "delete"}), + ("", "configmaps", {"get", "list", "create", "delete"}), + ], +) +def test_base_rules_unaffected(api_group: str, resource: str, required_verbs: set[str]) -> None: + """Regression guard: pre-existing k8s backend RBAC rules are untouched.""" + verbs = _rule_verbs(_controller_role_source(), api_group=api_group, resource=resource) + assert required_verbs <= set(verbs) From 6690e4b5f495686dc04221f411ef854cca1f95df Mon Sep 17 00:00:00 2001 From: Tyler Bray Date: Mon, 6 Jul 2026 10:55:45 -0700 Subject: [PATCH 2/4] fix(deployments): use generic namespace in k8s executor README example The example used my personal dev-blue namespace (tbray-dev) as the "default" default_namespace value. Replace with the literal default from K8sExecutorConfig plus a comment explaining what belongs there, so the doc doesn't imply a personal namespace is the expected default. Signed-off-by: Tyler Bray --- plugins/nemo-deployments/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/nemo-deployments/README.md b/plugins/nemo-deployments/README.md index c011078d04..c93c56d04b 100644 --- a/plugins/nemo-deployments/README.md +++ b/plugins/nemo-deployments/README.md @@ -71,7 +71,7 @@ deployments: backend: k8s config: kubeconfig_path: /path/to/kubeconfig # unset: in-cluster config, then default kubeconfig - default_namespace: tbray-dev + default_namespace: default # namespace the controller's ServiceAccount has RBAC in request_timeout: 60 default_executor: local-k8s ``` From ffcfdf9a4de53441fb04e6ea86a1a41fcd85519c Mon Sep 17 00:00:00 2001 From: Tyler Bray Date: Mon, 6 Jul 2026 13:13:03 -0700 Subject: [PATCH 3/4] fix(deployments): correct pods/log verb claim in README RBAC summary CodeRabbit caught that the RBAC summary grouped pods and pods/log under the same get/list/watch verb set, but controller-role.yaml only grants get/list on pods/log (no watch). Split the two out. Signed-off-by: Tyler Bray --- plugins/nemo-deployments/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/nemo-deployments/README.md b/plugins/nemo-deployments/README.md index c93c56d04b..0d45cb2cb8 100644 --- a/plugins/nemo-deployments/README.md +++ b/plugins/nemo-deployments/README.md @@ -87,7 +87,7 @@ The `DeploymentsController` runs inside the `nmp-core` controller pod existing ServiceAccount and Role rather than a dedicated one. The deploy chart's `k8s/helm/templates/core/controller-role.yaml` grants that Role the verbs the k8s backend needs in `.Release.Namespace`: `get`/`list`/`watch` on -pods and pods/log, `create`/`get`/`list`/`watch`/`update`/`patch`/`delete` on +pods, `get`/`list` on pods/log, `create`/`get`/`list`/`watch`/`update`/`patch`/`delete` on `batch/v1.Job`, `get`/`list`/`create`/`delete` on PVCs, ConfigMaps, and Services, and `get`/`list`/`watch`/`create`/`delete` on `apps/v1.Deployment`. From 57fb9658d8b379a68bd3f9d5ea74b1b9051756c4 Mon Sep 17 00:00:00 2001 From: Tyler Bray Date: Tue, 7 Jul 2026 10:02:57 -0700 Subject: [PATCH 4/4] fix(deployments): address mckornfield review feedback on RBAC chart PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - test_rbac_manifest.py: extract the repeated controller-role.yaml path segments (previously duplicated in _repo_root() and _controller_role_source()) into a single _CONTROLLER_ROLE_RELATIVE_PATH constant. - README: drop the "(a RoleBinding in that namespace, or a ClusterRole/ClusterRoleBinding)" parenthetical from the RBAC section — self-evident to anyone hitting the "additional RBAC" gap it describes, and trims a sentence that was otherwise dense with cross-references. Signed-off-by: Tyler Bray --- plugins/nemo-deployments/README.md | 3 +-- .../tests/unit/backends/k8s/test_rbac_manifest.py | 8 +++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/plugins/nemo-deployments/README.md b/plugins/nemo-deployments/README.md index 0d45cb2cb8..1d5e6d174d 100644 --- a/plugins/nemo-deployments/README.md +++ b/plugins/nemo-deployments/README.md @@ -93,8 +93,7 @@ Services, and `get`/`list`/`watch`/`create`/`delete` on `apps/v1.Deployment`. This Role is namespace-scoped to the release namespace. Pointing `backend_config.k8s.namespace` at a namespace outside the release namespace -requires additional RBAC (a RoleBinding in that namespace, or a -ClusterRole/ClusterRoleBinding) that the chart does not provision today; +requires additional RBAC that the chart does not provision today; namespace-per-workspace provisioning is a documented future enhancement. ### Native sidecars diff --git a/plugins/nemo-deployments/tests/unit/backends/k8s/test_rbac_manifest.py b/plugins/nemo-deployments/tests/unit/backends/k8s/test_rbac_manifest.py index 4ecd52c6ff..c0f3add6d0 100644 --- a/plugins/nemo-deployments/tests/unit/backends/k8s/test_rbac_manifest.py +++ b/plugins/nemo-deployments/tests/unit/backends/k8s/test_rbac_manifest.py @@ -18,16 +18,18 @@ import pytest +_CONTROLLER_ROLE_RELATIVE_PATH = Path("k8s", "helm", "templates", "core", "controller-role.yaml") + def _repo_root() -> Path: for parent in Path(__file__).resolve().parents: - if (parent / "k8s" / "helm" / "templates" / "core" / "controller-role.yaml").is_file(): + if (parent / _CONTROLLER_ROLE_RELATIVE_PATH).is_file(): return parent - raise FileNotFoundError("Could not locate repo root containing k8s/helm/templates/core/controller-role.yaml") + raise FileNotFoundError(f"Could not locate repo root containing {_CONTROLLER_ROLE_RELATIVE_PATH}") def _controller_role_source() -> str: - role_path = _repo_root() / "k8s" / "helm" / "templates" / "core" / "controller-role.yaml" + role_path = _repo_root() / _CONTROLLER_ROLE_RELATIVE_PATH content = role_path.read_text() # Only the Role definition (before the "---" separator) is relevant; the # RoleBinding that follows has no `rules:` block to collide with.