Skip to content

Commit a1b2f4e

Browse files
committed
fix(compute): complete server driver decoupling
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
1 parent 8d6672d commit a1b2f4e

15 files changed

Lines changed: 443 additions & 297 deletions

File tree

.agents/skills/debug-openshell-cluster/SKILL.md

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -454,8 +454,13 @@ Then inspect sandbox resources in that namespace.
454454

455455
Check the configured sandbox service account when TokenReview bootstrap or
456456
sandbox registration fails. Helm creates a dedicated sandbox service account by
457-
default and writes it to `[openshell.drivers.kubernetes].service_account_name`;
458-
the gateway rejects projected tokens from other service accounts.
457+
default. The driver receives it in
458+
`[openshell.drivers.kubernetes].service_account_name`, while the independent
459+
gateway authenticator receives it in
460+
`[openshell.gateway.sandbox_token_bootstrap].service_account_name`; the gateway
461+
rejects projected tokens from other service accounts. Confirm the bootstrap
462+
table also contains exactly one of `namespace`, `namespace_prefix`,
463+
`namespace_label`, or `namespace_file`.
459464

460465
```bash
461466
helm -n openshell get values openshell | grep -A3 sandboxServiceAccount

architecture/compute-runtimes.md

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -455,15 +455,22 @@ watcher emits only sandbox CR changes, not platform events.
455455

456456
### SA Token Authentication
457457

458-
The gateway's `K8sServiceAccountAuthenticator` adapts its `NamespaceValidator`
459-
per mode (`crates/openshell-server/src/auth/k8s_sa.rs`):
458+
The gateway owns ServiceAccount bootstrap under
459+
`[openshell.gateway.sandbox_token_bootstrap]`, independently of compute-driver
460+
selection. The Helm chart maps its workspace mode into the corresponding
461+
`NamespaceValidator` (`crates/openshell-server/src/auth/k8s_sa.rs`):
460462

461463
- **Shared:** `Exact` — accepts only the single configured namespace.
462464
- **Managed:** `Prefix` — accepts any namespace starting with `openshell-{gateway_id}-`.
463465
- **Operator:** `Allowlist` — accepts namespaces present in the dynamic
464466
`BTreeSet` populated by the label/file watchers. Starts empty (fail-closed)
465467
until the first watcher update.
466468

469+
The compiled Kubernetes registration derives the same policy from legacy
470+
driver configuration for compatibility. Operator-managed external drivers use
471+
the gateway-owned table directly; unrelated external drivers do not acquire a
472+
Kubernetes bootstrap requirement merely because the gateway runs in-cluster.
473+
467474
These checks rely on an ownership invariant. In shared and managed modes, the
468475
gateway and its trusted Agent Sandbox controller exclusively administer the
469476
sandbox namespace, Sandbox CRs, sandbox pods, and configured sandbox

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

Lines changed: 44 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -3506,9 +3506,14 @@ fn sandbox_template_to_k8s_with_validated_config(
35063506
}
35073507
apply_pod_driver_config(&mut spec, &driver_config.pod);
35083508

3509-
// Per-sandbox platform_config.host_users overrides the cluster-wide default.
3510-
let use_user_namespaces = platform_config_bool(template, "host_users")
3511-
.map_or(params.enable_user_namespaces, |host_users| !host_users);
3509+
// Per-sandbox portable intent overrides the cluster-wide default. This
3510+
// driver owns the Kubernetes-specific `hostUsers` translation. Accept the
3511+
// former platform_config encoding during rolling upgrades from gateways
3512+
// that predate the typed field.
3513+
let use_user_namespaces = template
3514+
.user_namespaces
3515+
.or_else(|| platform_config_bool(template, "host_users").map(|host_users| !host_users))
3516+
.unwrap_or(params.enable_user_namespaces);
35123517

35133518
if use_user_namespaces {
35143519
spec.insert("hostUsers".to_string(), serde_json::json!(false));
@@ -4124,7 +4129,7 @@ fn platform_config_bool(template: &SandboxTemplate, key: &str) -> Option<bool> {
41244129
let config = template.platform_config.as_ref()?;
41254130
let value = config.fields.get(key)?;
41264131
match value.kind.as_ref() {
4127-
Some(prost_types::value::Kind::BoolValue(b)) => Some(*b),
4132+
Some(prost_types::value::Kind::BoolValue(value)) => Some(*value),
41284133
_ => None,
41294134
}
41304135
}
@@ -6846,15 +6851,7 @@ mod tests {
68466851
#[test]
68476852
fn user_namespaces_per_sandbox_override_enables() {
68486853
let template = SandboxTemplate {
6849-
platform_config: Some(Struct {
6850-
fields: std::iter::once((
6851-
"host_users".to_string(),
6852-
Value {
6853-
kind: Some(Kind::BoolValue(false)),
6854-
},
6855-
))
6856-
.collect(),
6857-
}),
6854+
user_namespaces: Some(true),
68586855
..SandboxTemplate::default()
68596856
};
68606857

@@ -6870,7 +6867,7 @@ mod tests {
68706867
assert_eq!(
68716868
pod_template["spec"]["hostUsers"],
68726869
serde_json::json!(false),
6873-
"per-sandbox host_users: false must enable user namespaces"
6870+
"per-sandbox user namespace intent must set hostUsers: false"
68746871
);
68756872
let caps = pod_template["spec"]["containers"][0]["securityContext"]["capabilities"]["add"]
68766873
.as_array()
@@ -6881,15 +6878,7 @@ mod tests {
68816878
#[test]
68826879
fn user_namespaces_per_sandbox_override_disables() {
68836880
let template = SandboxTemplate {
6884-
platform_config: Some(Struct {
6885-
fields: std::iter::once((
6886-
"host_users".to_string(),
6887-
Value {
6888-
kind: Some(Kind::BoolValue(true)),
6889-
},
6890-
))
6891-
.collect(),
6892-
}),
6881+
user_namespaces: Some(false),
68936882
..SandboxTemplate::default()
68946883
};
68956884

@@ -6907,7 +6896,7 @@ mod tests {
69076896

69086897
assert!(
69096898
pod_template["spec"]["hostUsers"].is_null(),
6910-
"per-sandbox host_users: true must disable user namespaces even when cluster default is on"
6899+
"per-sandbox user namespace intent must override the cluster default"
69116900
);
69126901
let caps = pod_template["spec"]["containers"][0]["securityContext"]["capabilities"]["add"]
69136902
.as_array()
@@ -6919,6 +6908,37 @@ mod tests {
69196908
);
69206909
}
69216910

6911+
#[test]
6912+
fn user_namespaces_accepts_legacy_host_users_encoding() {
6913+
let template = SandboxTemplate {
6914+
platform_config: Some(Struct {
6915+
fields: std::iter::once((
6916+
"host_users".to_string(),
6917+
Value {
6918+
kind: Some(Kind::BoolValue(false)),
6919+
},
6920+
))
6921+
.collect(),
6922+
}),
6923+
..SandboxTemplate::default()
6924+
};
6925+
6926+
let params = SandboxPodParams::default();
6927+
let pod_template = sandbox_template_to_k8s(
6928+
&template,
6929+
false,
6930+
&std::collections::HashMap::new(),
6931+
true,
6932+
&params,
6933+
);
6934+
6935+
assert_eq!(
6936+
pod_template["spec"]["hostUsers"],
6937+
serde_json::json!(false),
6938+
"legacy host_users: false must still enable user namespaces"
6939+
);
6940+
}
6941+
69226942
#[test]
69236943
fn automount_service_account_token_is_disabled() {
69246944
let pod_template = {
@@ -7084,43 +7104,6 @@ mod tests {
70847104
);
70857105
}
70867106

7087-
#[test]
7088-
fn platform_config_bool_extracts_value() {
7089-
let template = SandboxTemplate {
7090-
platform_config: Some(Struct {
7091-
fields: std::iter::once((
7092-
"my_bool".to_string(),
7093-
Value {
7094-
kind: Some(Kind::BoolValue(true)),
7095-
},
7096-
))
7097-
.collect(),
7098-
}),
7099-
..SandboxTemplate::default()
7100-
};
7101-
7102-
assert_eq!(platform_config_bool(&template, "my_bool"), Some(true));
7103-
assert_eq!(platform_config_bool(&template, "missing"), None);
7104-
}
7105-
7106-
#[test]
7107-
fn platform_config_bool_returns_none_for_non_bool() {
7108-
let template = SandboxTemplate {
7109-
platform_config: Some(Struct {
7110-
fields: std::iter::once((
7111-
"a_string".to_string(),
7112-
Value {
7113-
kind: Some(Kind::StringValue("hello".to_string())),
7114-
},
7115-
))
7116-
.collect(),
7117-
}),
7118-
..SandboxTemplate::default()
7119-
};
7120-
7121-
assert_eq!(platform_config_bool(&template, "a_string"), None);
7122-
}
7123-
71247107
#[test]
71257108
fn log_level_propagates_as_env_var_to_sandbox_pod() {
71267109
let spec = SandboxSpec {

crates/openshell-gateway/src/lib.rs

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ fn install_in_tree_compute_drivers(registry: &mut ComputeDriverRegistry) {
3838
registration
3939
.with_telemetry_category(TelemetryComputeDriver::anonymous_category("kubernetes"))
4040
.without_mtls_user_auth()
41+
.with_token_bootstrap(kubernetes_token_bootstrap)
4142
.with_inherited_config_keys(&[
4243
"namespace",
4344
"default_image",
@@ -107,6 +108,48 @@ fn install_in_tree_compute_drivers(registry: &mut ComputeDriverRegistry) {
107108
}
108109
}
109110

111+
#[cfg(all(not(target_os = "windows"), feature = "in-tree-compute-drivers"))]
112+
fn kubernetes_token_bootstrap(
113+
context: &openshell_server::ComputeDriverBuildContext<'_>,
114+
) -> openshell_core::Result<Option<openshell_server::config_file::SandboxTokenBootstrapConfig>> {
115+
let config: openshell_driver_kubernetes::KubernetesComputeConfig = context.driver_config()?;
116+
Ok(Some(kubernetes_token_bootstrap_from_config(config)))
117+
}
118+
119+
#[cfg(all(not(target_os = "windows"), feature = "in-tree-compute-drivers"))]
120+
fn kubernetes_token_bootstrap_from_config(
121+
config: openshell_driver_kubernetes::KubernetesComputeConfig,
122+
) -> openshell_server::config_file::SandboxTokenBootstrapConfig {
123+
use openshell_driver_kubernetes::WorkspaceMode;
124+
125+
let (namespace, namespace_prefix, namespace_label, namespace_file) = match config.workspace_mode
126+
{
127+
WorkspaceMode::Shared => (Some(config.namespace), None, None, None),
128+
WorkspaceMode::Managed => (
129+
None,
130+
Some(openshell_driver_kubernetes::managed_namespace_prefix(
131+
&config.gateway_id,
132+
)),
133+
None,
134+
None,
135+
),
136+
WorkspaceMode::Operator => (
137+
None,
138+
None,
139+
config.operator_namespace_label,
140+
config.operator_namespace_file.map(std::path::PathBuf::from),
141+
),
142+
};
143+
144+
openshell_server::config_file::SandboxTokenBootstrapConfig::KubernetesServiceAccount {
145+
service_account_name: config.service_account_name,
146+
namespace,
147+
namespace_prefix,
148+
namespace_label,
149+
namespace_file,
150+
}
151+
}
152+
110153
#[cfg(all(not(target_os = "windows"), feature = "in-tree-compute-drivers"))]
111154
fn podman_tracing_setup(
112155
otlp_endpoint: Option<&str>,
@@ -288,3 +331,31 @@ fn apply_guest_tls(
288331
*key = Some(default_key.to_owned());
289332
}
290333
}
334+
335+
#[cfg(all(test, not(target_os = "windows"), feature = "in-tree-compute-drivers"))]
336+
mod tests {
337+
use super::*;
338+
use openshell_driver_kubernetes::{KubernetesComputeConfig, WorkspaceMode};
339+
use openshell_server::config_file::SandboxTokenBootstrapConfig;
340+
341+
#[test]
342+
fn kubernetes_bootstrap_compatibility_uses_managed_namespace_prefix() {
343+
let config = KubernetesComputeConfig {
344+
workspace_mode: WorkspaceMode::Managed,
345+
gateway_id: "test-gateway".to_string(),
346+
service_account_name: "sandbox-sa".to_string(),
347+
..KubernetesComputeConfig::default()
348+
};
349+
350+
assert_eq!(
351+
kubernetes_token_bootstrap_from_config(config),
352+
SandboxTokenBootstrapConfig::KubernetesServiceAccount {
353+
service_account_name: "sandbox-sa".to_string(),
354+
namespace: None,
355+
namespace_prefix: Some("openshell-test-gateway-".to_string()),
356+
namespace_label: None,
357+
namespace_file: None,
358+
}
359+
);
360+
}
361+
}

crates/openshell-gateway/src/vm.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,10 @@
2020
//! [`openshell_core::Config`] so the shared core stays free of driver-specific
2121
//! plumbing.
2222
//!
23-
//! TODO(driver-abstraction): this module still assumes the concrete VM driver
24-
//! (argv shape, guest-TLS flags, libkrun-specific settings). Once we land the
25-
//! generalized compute-driver interface, the CLI-arg plumbing below should
26-
//! be replaced with a driver-agnostic launcher that speaks gRPC to
27-
//! configure the driver — and this file should collapse to the types that
28-
//! are genuinely VM-specific (libkrun log level, vCPU / memory shape) plus a
29-
//! trait implementation registering the VM driver against the generic
30-
//! interface.
23+
//! Process launch remains deliberately VM-specific at this binary composition
24+
//! boundary: it translates gateway configuration into the standalone driver's
25+
//! argv and then connects through the same public compute-driver RPC interface
26+
//! used by operator-managed external drivers.
3127
3228
#[cfg(unix)]
3329
use hyper_util::rt::TokioIo;

crates/openshell-server/Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ openshell-router = { path = "../openshell-router" }
2727
openshell-supervisor-middleware = { path = "../openshell-supervisor-middleware" }
2828
openshell-supervisor-middleware-builtins = { path = "../openshell-supervisor-middleware-builtins" }
2929

30-
# Kubernetes client (used by the `generate-certs` subcommand)
30+
# Kubernetes client used by ServiceAccount bootstrap authentication and the
31+
# `generate-certs` subcommand.
3132
kube = { workspace = true }
3233
k8s-openapi = { workspace = true }
3334

crates/openshell-server/src/auth/k8s_sa.rs

Lines changed: 38 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -164,40 +164,53 @@ impl NamespaceValidator {
164164
}
165165
}
166166

167-
pub fn operator_namespace_allowlist(
168-
config: &crate::compute::driver_config::KubernetesSaBootstrapConfig,
167+
pub fn namespace_validator(
168+
namespace: Option<String>,
169+
namespace_prefix: Option<String>,
170+
namespace_label: Option<String>,
171+
namespace_file: Option<PathBuf>,
169172
client: kube::Client,
170173
shutdown_rx: watch::Receiver<bool>,
171-
) -> openshell_core::Result<OperatorNamespaceAllowlist> {
172-
let allowlist = OperatorNamespaceAllowlist::new();
173-
match (
174-
config.operator_namespace_label.as_deref(),
175-
config.operator_namespace_file.as_deref(),
176-
) {
177-
(Some(label), None) if !label.trim().is_empty() => {
178-
spawn_namespace_label_watcher(
174+
) -> openshell_core::Result<NamespaceValidator> {
175+
let exact = namespace.filter(|value| !value.trim().is_empty());
176+
let prefix = namespace_prefix.filter(|value| !value.trim().is_empty());
177+
let label = namespace_label.filter(|value| !value.trim().is_empty());
178+
let file = namespace_file.filter(|value| !value.as_os_str().is_empty());
179+
180+
match (exact, prefix, label, file) {
181+
(Some(namespace), None, None, None) => Ok(NamespaceValidator::Exact(namespace)),
182+
(None, Some(prefix), None, None) => Ok(NamespaceValidator::Prefix(prefix)),
183+
(None, None, label, file) if label.is_some() || file.is_some() => {
184+
Ok(NamespaceValidator::Allowlist(dynamic_namespace_allowlist(
185+
label,
186+
file,
179187
client,
180-
label.to_string(),
181-
allowlist.clone(),
182188
shutdown_rx,
183-
);
184-
}
185-
(None, Some(path)) if !path.trim().is_empty() => {
186-
spawn_namespace_file_watcher(path.into(), allowlist.clone(), shutdown_rx);
189+
)?))
187190
}
188-
(None, None) => {
189-
return Err(openshell_core::Error::config(
190-
"operator workspace mode requires operator_namespace_label or operator_namespace_file",
191-
));
191+
_ => Err(openshell_core::Error::config(
192+
"sandbox_token_bootstrap requires exactly one namespace policy: namespace, namespace_prefix, namespace_label, or namespace_file",
193+
)),
194+
}
195+
}
196+
197+
fn dynamic_namespace_allowlist(
198+
namespace_label: Option<String>,
199+
namespace_file: Option<PathBuf>,
200+
client: kube::Client,
201+
shutdown_rx: watch::Receiver<bool>,
202+
) -> openshell_core::Result<OperatorNamespaceAllowlist> {
203+
let allowlist = OperatorNamespaceAllowlist::new();
204+
match (namespace_label, namespace_file) {
205+
(Some(label), None) => {
206+
spawn_namespace_label_watcher(client, label, allowlist.clone(), shutdown_rx);
192207
}
193-
(Some(_), Some(_)) => {
194-
return Err(openshell_core::Error::config(
195-
"operator workspace mode accepts only one of operator_namespace_label or operator_namespace_file",
196-
));
208+
(None, Some(path)) => {
209+
spawn_namespace_file_watcher(path, allowlist.clone(), shutdown_rx);
197210
}
198211
_ => {
199212
return Err(openshell_core::Error::config(
200-
"operator namespace source must not be empty",
213+
"sandbox_token_bootstrap accepts only one dynamic namespace source",
201214
));
202215
}
203216
}

0 commit comments

Comments
 (0)