Skip to content

Commit 46ed80b

Browse files
committed
fix(helm): reduce gateway Secret permissions
Remove the gateway's cluster-wide get on the copied client TLS and image-pull Secret names and its Secret list permission in every workspace mode. Source Secrets are read through a Role in the sandbox namespace, and bootstrap Secret cleanup deletes Secrets by exact name instead of listing them. - Grant get on the copied client TLS and image-pull Secrets through a Role in the sandbox namespace. The ClusterRole keeps patch on those names for server-side apply into workspace namespaces. - Delete sandbox and supervisor bootstrap Secrets by exact name, derived from the runtime generation recorded on the Sandbox and, on restart, the target generation, tolerating 404. The generation annotation is cleared only after cleanup succeeds, and each bootstrap Secret has a Pod owner reference, so garbage collection removes any generation the driver does not name. - Drop Secret list from the ClusterRole and the shared-mode sandbox Role. - Extend the managed e2e RBAC checks to Secrets with the copied names and to list. - Update the Kubernetes setup and sandbox runtime docs, compute-runtime architecture, and cluster debugging skill. Signed-off-by: Kris Hicks <khicks@nvidia.com>
1 parent 94d4425 commit 46ed80b

14 files changed

Lines changed: 228 additions & 84 deletions

File tree

‎architecture/compute-runtimes.md‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -484,11 +484,12 @@ limit. RBAC promotes sandbox CRD permissions to a ClusterRole and adds namespace
484484
Secret copies use server-side apply. Kubernetes authorizes an apply to an
485485
existing Secret as `patch`, but also requires `create` authorization when the
486486
target does not exist. RBAC cannot constrain `create` by `resourceNames`, so
487-
managed mode grants cluster-wide Secret `create`, `list`, and `delete` for
488-
generation-scoped bootstrap Secret creation and rollback recovery while keeping
489-
source reads and subsequent patches restricted to the explicitly configured TLS
490-
and image-pull Secret names. Recovery lists bootstrap Secrets by sandbox and
491-
component labels, then deletes stale generations with UID preconditions. The
487+
managed mode grants cluster-wide Secret `create` and `delete` for
488+
generation-scoped bootstrap Secret creation and rollback recovery. Patches are
489+
restricted to the explicitly configured TLS and image-pull Secret names, and
490+
source reads use a Role in the driver's source namespace. Recovery deletes the
491+
bootstrap Secrets of the recorded and target runtime generations by exact name;
492+
Pod owner references let garbage collection remove any other generation. The
492493
driver exercises these broad permissions only in gateway-owned managed
493494
namespaces. This depends on the managed-mode ownership invariant described below;
494495
the gateway ServiceAccount must not be shared with unrelated workloads.

‎crates/openshell-driver-kubernetes/src/driver.rs‎

Lines changed: 39 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ use crate::isolation::{
1313
};
1414
use crate::sandbox_runtime::{
1515
BOUNDARY_CERTIFICATE_PATH, BOUNDARY_CONFIG_PATH, BOUNDARY_PRIVATE_KEY_PATH,
16-
SANDBOX_SECRET_COMPONENT, SUPERVISOR_SECRET_COMPONENT,
1716
SUPERVISOR_TERMINATION_GRACE_PERIOD_SECONDS, SandboxRuntimeNames, boundary_service,
1817
generate_proxy_ca_material, sandbox_bootstrap_secret,
1918
sandbox_owner_reference as sandbox_runtime_sandbox_owner_reference,
@@ -3032,8 +3031,12 @@ impl KubernetesComputeDriver {
30323031
pod_is_gone,
30333032
);
30343033
if stop_is_complete {
3035-
self.delete_sandbox_runtime_supervisor(sandbox_id, &namespace)
3036-
.await?;
3034+
self.delete_sandbox_runtime_supervisor(
3035+
sandbox_id,
3036+
&namespace,
3037+
sandbox_runtime_generation(&object).as_slice(),
3038+
)
3039+
.await?;
30373040
patch_dynamic_object_with_resource_version_retry(
30383041
&agent_sandbox_api.api,
30393042
&kube_name,
@@ -3216,7 +3219,9 @@ impl KubernetesComputeDriver {
32163219
let names = SandboxRuntimeNames::for_generation(sandbox_id, generation.as_str());
32173220
self.create_sandbox_runtime_fence(&namespace, &names)
32183221
.await?;
3219-
self.delete_sandbox_runtime_supervisor(sandbox_id, &namespace)
3222+
let mut stale_generations = sandbox_runtime_generation(&object).as_slice().to_vec();
3223+
stale_generations.push(generation.as_str());
3224+
self.delete_sandbox_runtime_supervisor(sandbox_id, &namespace, &stale_generations)
32203225
.await?;
32213226
let main_process_spec =
32223227
required_sandbox_annotation(&object, ANNOTATION_SANDBOX_RUNTIME_MAIN_PROCESS_SPEC)?;
@@ -3499,40 +3504,28 @@ impl KubernetesComputeDriver {
34993504
&self,
35003505
sandbox_id: &str,
35013506
namespace: &str,
3507+
generations: &[&str],
35023508
) -> Result<(), KubernetesDriverError> {
35033509
self.delete_sandbox_runtime_supervisor_pod(sandbox_id, namespace)
35043510
.await?;
3505-
self.delete_sandbox_runtime_generation_secrets(sandbox_id, namespace)
3511+
self.delete_sandbox_runtime_generation_secrets(sandbox_id, namespace, generations)
35063512
.await
35073513
}
35083514

3515+
/// Delete the bootstrap Secrets of the given runtime generations by exact
3516+
/// name. Garbage collection through their owner Pods removes older
3517+
/// generations.
35093518
async fn delete_sandbox_runtime_generation_secrets(
35103519
&self,
35113520
sandbox_id: &str,
35123521
namespace: &str,
3522+
generations: &[&str],
35133523
) -> Result<(), KubernetesDriverError> {
35143524
let secrets = Api::<Secret>::namespaced(self.client.clone(), namespace);
3515-
for component in [SANDBOX_SECRET_COMPONENT, SUPERVISOR_SECRET_COMPONENT] {
3516-
let selector =
3517-
format!("openshell.ai/sandbox-id={sandbox_id},openshell.ai/component={component}");
3518-
let items = secrets
3519-
.list(&ListParams::default().labels(&selector))
3520-
.await
3521-
.map_err(KubernetesDriverError::from_kube)?;
3522-
for secret in items {
3523-
let Some(name) = secret.metadata.name else {
3524-
continue;
3525-
};
3526-
match secrets
3527-
.delete(
3528-
&name,
3529-
&DeleteParams::default().preconditions(Preconditions {
3530-
uid: secret.metadata.uid,
3531-
resource_version: None,
3532-
}),
3533-
)
3534-
.await
3535-
{
3525+
for generation in generations {
3526+
let names = SandboxRuntimeNames::for_generation(sandbox_id, generation);
3527+
for name in [&names.sandbox_secret, &names.supervisor_secret] {
3528+
match secrets.delete(name, &DeleteParams::default()).await {
35363529
Ok(_) | Err(KubeError::Api(kube::core::ErrorResponse { code: 404, .. })) => {}
35373530
Err(error) => return Err(KubernetesDriverError::from_kube(error)),
35383531
}
@@ -3884,7 +3877,12 @@ impl KubernetesComputeDriver {
38843877
}
38853878
}
38863879
match self
3887-
.reconcile_sandbox_runtime_supervisor(&sandbox_id, namespace, desired_running)
3880+
.reconcile_sandbox_runtime_supervisor(
3881+
&sandbox_id,
3882+
namespace,
3883+
desired_running,
3884+
sandbox_runtime_generation(&object).as_slice(),
3885+
)
38883886
.await
38893887
{
38903888
Ok(()) => {}
@@ -3938,7 +3936,11 @@ impl KubernetesComputeDriver {
39383936
}
39393937
}
39403938
if let Err(error) = self
3941-
.delete_sandbox_runtime_supervisor(sandbox_id, namespace)
3939+
.delete_sandbox_runtime_supervisor(
3940+
sandbox_id,
3941+
namespace,
3942+
sandbox_runtime_generation(object).as_slice(),
3943+
)
39423944
.await
39433945
{
39443946
warn!(sandbox_id, %error, "could not finish sandbox-runtime suspension cleanup");
@@ -4208,10 +4210,11 @@ impl KubernetesComputeDriver {
42084210
sandbox_id: &str,
42094211
namespace: &str,
42104212
desired_running: bool,
4213+
generations: &[&str],
42114214
) -> Result<(), KubernetesDriverError> {
42124215
if !desired_running {
42134216
return self
4214-
.delete_sandbox_runtime_supervisor(sandbox_id, namespace)
4217+
.delete_sandbox_runtime_supervisor(sandbox_id, namespace, generations)
42154218
.await;
42164219
}
42174220
let names = SandboxRuntimeNames::new(sandbox_id);
@@ -8440,6 +8443,7 @@ mod tests {
84408443
"resourceVersion": "42",
84418444
"annotations": {
84428445
SANDBOX_POD_NAME_ANNOTATION: "workload-pod",
8446+
ANNOTATION_SANDBOX_RUNTIME_GENERATION: "gen-7",
84438447
ANNOTATION_SANDBOX_RUNTIME_BOOTSTRAP_PHASE:
84448448
SandboxRuntimeBootstrapPhase::Suspending.as_str()
84458449
}
@@ -8455,12 +8459,6 @@ mod tests {
84558459
serde_json::json!({"kind": "Status", "status": "Success", "code": 200}),
84568460
)
84578461
};
8458-
let no_secrets = || {
8459-
kube_test_response(
8460-
http::StatusCode::OK,
8461-
serde_json::json!({"apiVersion": "v1", "kind": "SecretList", "items": []}),
8462-
)
8463-
};
84648462
let steps = Arc::new(std::sync::Mutex::new(VecDeque::from([
84658463
(
84668464
http::Method::GET,
@@ -8504,14 +8502,14 @@ mod tests {
85048502
kube_test_not_found("pods", "os-supervisor-sandbox-1"),
85058503
),
85068504
(
8507-
http::Method::GET,
8508-
"/api/v1/namespaces/openshell/secrets",
8509-
no_secrets(),
8505+
http::Method::DELETE,
8506+
"/api/v1/namespaces/openshell/secrets/os-sandbox-sandbox-1-gen7",
8507+
success(),
85108508
),
85118509
(
8512-
http::Method::GET,
8513-
"/api/v1/namespaces/openshell/secrets",
8514-
no_secrets(),
8510+
http::Method::DELETE,
8511+
"/api/v1/namespaces/openshell/secrets/os-supervisor-sandbox-1-gen7",
8512+
kube_test_not_found("secrets", "os-supervisor-sandbox-1-gen7"),
85158513
),
85168514
(
85178515
http::Method::GET,

‎deploy/helm/openshell/templates/_helpers.tpl‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,26 @@ Namespace where sandbox pods are created. An explicit
169169
{{- .Values.server.sandboxNamespace | default .Release.Namespace -}}
170170
{{- end }}
171171

172+
{{/*
173+
Secrets the Kubernetes driver copies from the sandbox namespace into
174+
workspace namespaces, as a JSON array. Empty in shared workspace mode.
175+
*/}}
176+
{{- define "openshell.copiedWorkspaceSecretNames" -}}
177+
{{- $workspaceMode := .Values.server.drivers.kubernetes.workspaceMode | default "shared" -}}
178+
{{- $names := list -}}
179+
{{- if and (ne $workspaceMode "shared") (not .Values.server.disableTls) -}}
180+
{{- $names = append $names .Values.server.tls.clientTlsSecretName -}}
181+
{{- end -}}
182+
{{- if eq $workspaceMode "managed" -}}
183+
{{- range .Values.server.sandboxImagePullSecrets -}}
184+
{{- if .name -}}
185+
{{- $names = append $names .name -}}
186+
{{- end -}}
187+
{{- end -}}
188+
{{- end -}}
189+
{{- uniq $names | toJson -}}
190+
{{- end }}
191+
172192
{{/*
173193
Namespace where Kubernetes Secret-backed provider credentials live.
174194
*/}}

‎deploy/helm/openshell/templates/clusterrole.yaml‎

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -89,34 +89,24 @@ rules:
8989
- patch
9090
- watch
9191
{{- end }}
92-
{{- $copiedSecretNames := list }}
93-
{{- if and (ne $workspaceMode "shared") (not .Values.server.disableTls) }}
94-
{{- $copiedSecretNames = append $copiedSecretNames .Values.server.tls.clientTlsSecretName }}
95-
{{- end }}
96-
{{- if eq $workspaceMode "managed" }}
97-
{{- range .Values.server.sandboxImagePullSecrets }}
98-
{{- if .name }}
99-
{{- $copiedSecretNames = append $copiedSecretNames .name }}
100-
{{- end }}
101-
{{- end }}
102-
{{- end }}
10392
{{- if ne $workspaceMode "shared" }}
10493
- apiGroups: [""]
10594
resources: ["services"]
10695
verbs: ["create", "get"]
10796
- apiGroups: [""]
10897
resources: ["secrets"]
109-
# Bootstrap Secrets are generation-scoped. Recovery lists them by label and
110-
# deletes stale generations with UID preconditions before retrying creation.
111-
verbs: ["create", "list", "delete"]
98+
# Bootstrap Secrets are generation-scoped. Recovery deletes them by exact
99+
# name.
100+
verbs: ["create", "delete"]
112101
- apiGroups: ["networking.k8s.io"]
113102
resources: ["networkpolicies"]
114103
verbs: ["create", "get"]
115104
{{- end }}
116-
{{- $copiedSecretNames = uniq $copiedSecretNames }}
105+
{{- $copiedSecretNames := include "openshell.copiedWorkspaceSecretNames" . | fromJsonArray }}
117106
{{- if $copiedSecretNames }}
118107
# Copy only explicitly configured TLS and image-pull Secrets into workspace
119108
# namespaces. Server-side apply authorizes these requests as patch operations.
109+
# Reading the source Secrets is granted by a Role in the sandbox namespace.
120110
- apiGroups:
121111
- ""
122112
resources:
@@ -126,7 +116,6 @@ rules:
126116
- {{ . | quote }}
127117
{{- end }}
128118
verbs:
129-
- get
130119
- patch
131120
# Server-side apply uses PATCH for an existing Secret, but the API server
132121
# additionally authorizes CREATE when the named Secret does not exist yet.

‎deploy/helm/openshell/templates/role.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,9 @@ rules:
5959
verbs: ["create", "get"]
6060
- apiGroups: [""]
6161
resources: ["secrets"]
62-
# Bootstrap Secrets are generation-scoped. Recovery lists them by label and
63-
# deletes stale generations with UID preconditions before retrying creation.
64-
verbs: ["create", "list", "delete"]
62+
# Bootstrap Secrets are generation-scoped. Recovery deletes them by exact
63+
# name.
64+
verbs: ["create", "delete"]
6565
- apiGroups: ["networking.k8s.io"]
6666
resources: ["networkpolicies"]
6767
verbs: ["create", "get"]
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
{{- $copiedSecretNames := include "openshell.copiedWorkspaceSecretNames" . | fromJsonArray }}
2+
{{- if $copiedSecretNames }}
3+
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
4+
# SPDX-License-Identifier: Apache-2.0
5+
# Read the TLS and image-pull Secrets the gateway copies into workspace
6+
# namespaces.
7+
apiVersion: rbac.authorization.k8s.io/v1
8+
kind: Role
9+
metadata:
10+
name: {{ include "openshell.fullname" . }}-workspace-secret-source
11+
namespace: {{ include "openshell.sandboxNamespace" . }}
12+
labels:
13+
{{- include "openshell.labels" . | nindent 4 }}
14+
rules:
15+
- apiGroups:
16+
- ""
17+
resources:
18+
- secrets
19+
resourceNames:
20+
{{- range $copiedSecretNames }}
21+
- {{ . | quote }}
22+
{{- end }}
23+
verbs:
24+
- get
25+
{{- end }}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{{- if include "openshell.copiedWorkspaceSecretNames" . | fromJsonArray }}
2+
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
3+
# SPDX-License-Identifier: Apache-2.0
4+
apiVersion: rbac.authorization.k8s.io/v1
5+
kind: RoleBinding
6+
metadata:
7+
name: {{ include "openshell.fullname" . }}-workspace-secret-source
8+
namespace: {{ include "openshell.sandboxNamespace" . }}
9+
labels:
10+
{{- include "openshell.labels" . | nindent 4 }}
11+
roleRef:
12+
apiGroup: rbac.authorization.k8s.io
13+
kind: Role
14+
name: {{ include "openshell.fullname" . }}-workspace-secret-source
15+
subjects:
16+
- kind: ServiceAccount
17+
name: {{ include "openshell.serviceAccountName" . }}
18+
namespace: {{ .Release.Namespace }}
19+
{{- end }}

‎deploy/helm/openshell/tests/clusterrole_test.yaml‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ tests:
114114
apiGroups: [""]
115115
resources: ["secrets"]
116116
resourceNames: ["custom-client-tls"]
117-
verbs: ["get", "patch"]
117+
verbs: ["patch"]
118118

119119
- it: grants operator bootstrap secret lifecycle permissions when TLS and credential storage are disabled
120120
set:
@@ -126,7 +126,7 @@ tests:
126126
content:
127127
apiGroups: [""]
128128
resources: ["secrets"]
129-
verbs: ["create", "list", "delete"]
129+
verbs: ["create", "delete"]
130130
- notContains:
131131
path: rules
132132
content:
@@ -148,13 +148,13 @@ tests:
148148
apiGroups: [""]
149149
resources: ["secrets"]
150150
resourceNames: ["custom-client-tls", "registry-one", "registry-two"]
151-
verbs: ["get", "patch"]
151+
verbs: ["patch"]
152152
- contains:
153153
path: rules
154154
content:
155155
apiGroups: [""]
156156
resources: ["secrets"]
157-
verbs: ["create", "list", "delete"]
157+
verbs: ["create", "delete"]
158158

159159
- it: restricts managed copies to image-pull secrets when TLS is disabled
160160
set:
@@ -169,13 +169,13 @@ tests:
169169
apiGroups: [""]
170170
resources: ["secrets"]
171171
resourceNames: ["registry-one"]
172-
verbs: ["get", "patch"]
172+
verbs: ["patch"]
173173
- contains:
174174
path: rules
175175
content:
176176
apiGroups: [""]
177177
resources: ["secrets"]
178-
verbs: ["create", "list", "delete"]
178+
verbs: ["create", "delete"]
179179

180180
- it: omits secrets rule entirely in shared mode
181181
set:
@@ -209,7 +209,7 @@ tests:
209209
content:
210210
apiGroups: [""]
211211
resources: ["secrets"]
212-
verbs: ["create", "list", "delete"]
212+
verbs: ["create", "delete"]
213213

214214
- it: grants operator sandbox-runtime companion permissions
215215
set:
@@ -232,4 +232,4 @@ tests:
232232
content:
233233
apiGroups: [""]
234234
resources: ["secrets"]
235-
verbs: ["create", "list", "delete"]
235+
verbs: ["create", "delete"]

‎deploy/helm/openshell/tests/sandbox_namespace_test.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ tests:
7373
content:
7474
apiGroups: [""]
7575
resources: ["secrets"]
76-
verbs: ["create", "list", "delete"]
76+
verbs: ["create", "delete"]
7777
- notContains:
7878
path: rules
7979
content:

0 commit comments

Comments
 (0)