Skip to content

Commit 87c9a8c

Browse files
committed
fix(gpu): validate exact device requests
Require exact driver GPU device lists to be tied to a GPU request, allow a single exact device to use the default countless request, and require explicit matching counts for multi-device lists. Signed-off-by: Evan Lezar <elezar@nvidia.com>
1 parent ce5d047 commit 87c9a8c

10 files changed

Lines changed: 583 additions & 61 deletions

File tree

crates/openshell-core/src/gpu.rs

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,47 @@ pub fn cdi_gpu_device_ids(
6363
}
6464
}
6565

66+
/// Validate a compute-driver GPU request against driver-owned specific devices.
67+
///
68+
/// Drivers call this when a sandbox request combines portable GPU requirements
69+
/// with exact device identifiers in `driver_config`.
70+
///
71+
/// # Errors
72+
/// Returns an error when the sandbox GPU request is absent or when `gpu.count`
73+
/// does not equal the number of specific devices. A single exact device is
74+
/// compatible with the default sandbox GPU request where `gpu.count` is absent.
75+
pub fn validate_specific_gpu_device_request(
76+
gpu: Option<&DriverGpuResourceRequirements>,
77+
specific_devices: &[String],
78+
driver_config_field: &str,
79+
) -> Result<(), String> {
80+
let device_count = specific_devices.len();
81+
if device_count == 0 {
82+
return Ok(());
83+
}
84+
85+
let Some(gpu) = gpu else {
86+
return Err(format!("{driver_config_field} requires a gpu request"));
87+
};
88+
89+
let Some(count) = gpu.count else {
90+
if device_count == 1 {
91+
return Ok(());
92+
}
93+
return Err(format!(
94+
"{driver_config_field} requires an explicit gpu count matching its length ({device_count})"
95+
));
96+
};
97+
98+
if usize::try_from(count).ok() != Some(device_count) {
99+
return Err(format!(
100+
"gpu count ({count}) must match {driver_config_field} length ({device_count})"
101+
));
102+
}
103+
104+
Ok(())
105+
}
106+
66107
#[cfg(test)]
67108
mod tests {
68109
use super::*;
@@ -100,4 +141,92 @@ mod tests {
100141
])
101142
);
102143
}
144+
145+
#[test]
146+
fn validate_specific_gpu_device_request_ignores_empty_devices() {
147+
validate_specific_gpu_device_request(None, &[], "driver_config.cdi_devices")
148+
.expect("empty exact device lists should not be validated");
149+
}
150+
151+
#[test]
152+
fn validate_specific_gpu_device_request_accepts_matching_count() {
153+
let gpu = DriverGpuResourceRequirements { count: Some(2) };
154+
let specific_devices = vec![
155+
"nvidia.com/gpu=0".to_string(),
156+
"nvidia.com/gpu=1".to_string(),
157+
];
158+
159+
validate_specific_gpu_device_request(
160+
Some(&gpu),
161+
&specific_devices,
162+
"driver_config.cdi_devices",
163+
)
164+
.expect("matching count should be accepted");
165+
}
166+
167+
#[test]
168+
fn validate_specific_gpu_device_request_accepts_missing_count_for_one_device() {
169+
let gpu = DriverGpuResourceRequirements { count: None };
170+
let specific_devices = vec!["nvidia.com/gpu=0".to_string()];
171+
172+
validate_specific_gpu_device_request(
173+
Some(&gpu),
174+
&specific_devices,
175+
"driver_config.cdi_devices",
176+
)
177+
.expect("single exact device should be compatible with a default GPU request");
178+
}
179+
180+
#[test]
181+
fn validate_specific_gpu_device_request_rejects_missing_gpu_request() {
182+
let specific_devices = vec!["nvidia.com/gpu=0".to_string()];
183+
184+
let err = validate_specific_gpu_device_request(
185+
None,
186+
&specific_devices,
187+
"driver_config.cdi_devices",
188+
)
189+
.expect_err("missing GPU request should be rejected");
190+
191+
assert_eq!(err, "driver_config.cdi_devices requires a gpu request");
192+
}
193+
194+
#[test]
195+
fn validate_specific_gpu_device_request_rejects_missing_count_for_multiple_devices() {
196+
let gpu = DriverGpuResourceRequirements { count: None };
197+
let specific_devices = vec![
198+
"nvidia.com/gpu=0".to_string(),
199+
"nvidia.com/gpu=1".to_string(),
200+
];
201+
202+
let err = validate_specific_gpu_device_request(
203+
Some(&gpu),
204+
&specific_devices,
205+
"driver_config.cdi_devices",
206+
)
207+
.expect_err("missing count should be rejected for multiple devices");
208+
209+
assert_eq!(
210+
err,
211+
"driver_config.cdi_devices requires an explicit gpu count matching its length (2)"
212+
);
213+
}
214+
215+
#[test]
216+
fn validate_specific_gpu_device_request_rejects_mismatch() {
217+
let gpu = DriverGpuResourceRequirements { count: Some(2) };
218+
let specific_devices = vec!["nvidia.com/gpu=0".to_string()];
219+
220+
let err = validate_specific_gpu_device_request(
221+
Some(&gpu),
222+
&specific_devices,
223+
"driver_config.cdi_devices",
224+
)
225+
.expect_err("mismatched count should be rejected");
226+
227+
assert_eq!(
228+
err,
229+
"gpu count (2) must match driver_config.cdi_devices length (1)"
230+
);
231+
}
103232
}

crates/openshell-driver-docker/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ contract:
3232
| `apparmor=unconfined` | Avoids Docker's default profile blocking required mount operations. |
3333
| `restart_policy = unless-stopped` | Keeps managed sandboxes resumable across daemon or gateway restarts. |
3434
| `PidsLimit` | Enforces the sandbox PID budget at the Docker cgroup layer. Set `[openshell.drivers.docker].sandbox_pids_limit = 0` to inherit the Docker/runtime default. |
35-
| CDI GPU request | Uses `driver_config.cdi_devices` when set; otherwise requests all NVIDIA GPUs when the sandbox spec asks for GPU support and daemon CDI support is detected. Counted GPU requests are rejected. |
35+
| CDI GPU request | Uses `driver_config.cdi_devices` when set; otherwise requests all NVIDIA GPUs when the sandbox spec asks for GPU support and daemon CDI support is detected. Count-only GPU requests are rejected; exact CDI device lists with more than one entry require an explicit GPU count matching the device list length. |
3636

3737
The agent child process does not retain these supervisor privileges.
3838

crates/openshell-driver-docker/src/lib.rs

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ use openshell_core::driver_utils::{
2727
LABEL_MANAGED_BY, LABEL_MANAGED_BY_VALUE, LABEL_SANDBOX_ID, LABEL_SANDBOX_NAME,
2828
LABEL_SANDBOX_NAMESPACE, SUPERVISOR_IMAGE_BINARY_PATH, supervisor_image_should_refresh,
2929
};
30-
use openshell_core::gpu::{cdi_gpu_device_ids, driver_gpu_requirements};
30+
use openshell_core::gpu::{
31+
cdi_gpu_device_ids, driver_gpu_requirements, validate_specific_gpu_device_request,
32+
};
3133
use openshell_core::progress::{
3234
PROGRESS_STEP_PULLING_IMAGE, PROGRESS_STEP_REQUESTING_SANDBOX, PROGRESS_STEP_STARTING_SANDBOX,
3335
format_bytes, mark_progress_active, mark_progress_complete, mark_progress_detail,
@@ -514,23 +516,25 @@ impl DockerComputeDriver {
514516
supports_gpu: bool,
515517
driver_config: &DockerSandboxDriverConfig,
516518
) -> Result<(), Status> {
517-
if gpu_requirements.is_none() && driver_config.cdi_devices.is_some() {
518-
return Err(Status::invalid_argument(
519-
"driver_config.cdi_devices requires gpu=true",
519+
if gpu_requirements.is_some() && !supports_gpu {
520+
return Err(Status::failed_precondition(
521+
"docker GPU sandboxes require Docker CDI support. Enable CDI on the Docker daemon, then restart the OpenShell gateway/server so GPU capability is detected.",
520522
));
521523
}
522524

523-
if gpu_requirements.and_then(|gpu| gpu.count).is_some() {
525+
if let Some(cdi_devices) = driver_config.cdi_devices.as_deref() {
526+
validate_specific_gpu_device_request(
527+
gpu_requirements,
528+
cdi_devices,
529+
"driver_config.cdi_devices",
530+
)
531+
.map_err(Status::invalid_argument)?;
532+
} else if gpu_requirements.and_then(|gpu| gpu.count).is_some() {
524533
return Err(Status::invalid_argument(
525534
"docker GPU count requests are not supported; use --gpu without a count or driver_config.cdi_devices",
526535
));
527536
}
528537

529-
if gpu_requirements.is_some() && !supports_gpu {
530-
return Err(Status::failed_precondition(
531-
"docker GPU sandboxes require Docker CDI support. Enable CDI on the Docker daemon, then restart the OpenShell gateway/server so GPU capability is detected.",
532-
));
533-
}
534538
Ok(())
535539
}
536540

@@ -2129,11 +2133,12 @@ fn build_device_requests(sandbox: &DriverSandbox) -> Result<Option<Vec<DeviceReq
21292133
.cdi_devices
21302134
.unwrap_or_default();
21312135
let gpu_requirements = driver_gpu_requirements(spec.resource_requirements.as_ref());
2132-
if gpu_requirements.is_none() && !cdi_devices.is_empty() {
2133-
return Err(Status::invalid_argument(
2134-
"driver_config.cdi_devices requires gpu=true",
2135-
));
2136-
}
2136+
validate_specific_gpu_device_request(
2137+
gpu_requirements,
2138+
&cdi_devices,
2139+
"driver_config.cdi_devices",
2140+
)
2141+
.map_err(Status::invalid_argument)?;
21372142

21382143
Ok(
21392144
cdi_gpu_device_ids(gpu_requirements, &cdi_devices).map(|device_ids| {

crates/openshell-driver-docker/src/tests.rs

Lines changed: 122 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,6 +1022,20 @@ fn validate_sandbox_rejects_gpu_when_cdi_unavailable() {
10221022
assert!(err.message().contains("Docker CDI"));
10231023
}
10241024

1025+
#[test]
1026+
fn validate_sandbox_rejects_missing_gpu_support_before_request_shape() {
1027+
let config = runtime_config();
1028+
let mut sandbox = test_sandbox();
1029+
let spec = sandbox.spec.as_mut().unwrap();
1030+
spec.resource_requirements = Some(gpu_resources(Some(2)));
1031+
spec.template.as_mut().unwrap().driver_config = Some(cdi_devices_config(&["nvidia.com/gpu=0"]));
1032+
1033+
let err = DockerComputeDriver::validate_sandbox(&sandbox, &config).unwrap_err();
1034+
1035+
assert_eq!(err.code(), tonic::Code::FailedPrecondition);
1036+
assert!(err.message().contains("Docker CDI"));
1037+
}
1038+
10251039
#[test]
10261040
fn validate_sandbox_rejects_invalid_cdi_devices_before_gpu_capability() {
10271041
let config = runtime_config();
@@ -1068,6 +1082,94 @@ fn validate_sandbox_rejects_gpu_count_request() {
10681082
);
10691083
}
10701084

1085+
#[test]
1086+
fn validate_sandbox_accepts_gpu_count_matching_cdi_devices() {
1087+
let mut config = runtime_config();
1088+
config.supports_gpu = true;
1089+
let mut sandbox = test_sandbox();
1090+
let spec = sandbox.spec.as_mut().unwrap();
1091+
spec.resource_requirements = Some(gpu_resources(Some(2)));
1092+
spec.template.as_mut().unwrap().driver_config = Some(cdi_devices_config(&[
1093+
"nvidia.com/gpu=0",
1094+
"nvidia.com/gpu=1",
1095+
]));
1096+
1097+
DockerComputeDriver::validate_sandbox(&sandbox, &config)
1098+
.expect("matching explicit CDI device count should be accepted");
1099+
}
1100+
1101+
#[test]
1102+
fn validate_sandbox_accepts_single_cdi_device_without_gpu_count() {
1103+
let mut config = runtime_config();
1104+
config.supports_gpu = true;
1105+
let mut sandbox = test_sandbox();
1106+
let spec = sandbox.spec.as_mut().unwrap();
1107+
spec.resource_requirements = Some(gpu_resources(None));
1108+
spec.template.as_mut().unwrap().driver_config = Some(cdi_devices_config(&["nvidia.com/gpu=0"]));
1109+
1110+
DockerComputeDriver::validate_sandbox(&sandbox, &config)
1111+
.expect("single exact CDI device should be compatible with a default GPU request");
1112+
}
1113+
1114+
#[test]
1115+
fn validate_sandbox_rejects_multiple_cdi_devices_without_gpu_count() {
1116+
let mut config = runtime_config();
1117+
config.supports_gpu = true;
1118+
let mut sandbox = test_sandbox();
1119+
let spec = sandbox.spec.as_mut().unwrap();
1120+
spec.resource_requirements = Some(gpu_resources(None));
1121+
spec.template.as_mut().unwrap().driver_config = Some(cdi_devices_config(&[
1122+
"nvidia.com/gpu=0",
1123+
"nvidia.com/gpu=1",
1124+
]));
1125+
1126+
let err = DockerComputeDriver::validate_sandbox(&sandbox, &config).unwrap_err();
1127+
1128+
assert_eq!(err.code(), tonic::Code::InvalidArgument);
1129+
assert!(
1130+
err.message()
1131+
.contains("requires an explicit gpu count matching its length (2)")
1132+
);
1133+
}
1134+
1135+
#[test]
1136+
fn validate_sandbox_rejects_cdi_devices_without_gpu_request() {
1137+
let mut config = runtime_config();
1138+
config.supports_gpu = true;
1139+
let mut sandbox = test_sandbox();
1140+
sandbox
1141+
.spec
1142+
.as_mut()
1143+
.unwrap()
1144+
.template
1145+
.as_mut()
1146+
.unwrap()
1147+
.driver_config = Some(cdi_devices_config(&["nvidia.com/gpu=0"]));
1148+
1149+
let err = DockerComputeDriver::validate_sandbox(&sandbox, &config).unwrap_err();
1150+
1151+
assert_eq!(err.code(), tonic::Code::InvalidArgument);
1152+
assert!(err.message().contains("requires a gpu request"));
1153+
}
1154+
1155+
#[test]
1156+
fn validate_sandbox_rejects_gpu_count_mismatched_cdi_devices() {
1157+
let mut config = runtime_config();
1158+
config.supports_gpu = true;
1159+
let mut sandbox = test_sandbox();
1160+
let spec = sandbox.spec.as_mut().unwrap();
1161+
spec.resource_requirements = Some(gpu_resources(Some(2)));
1162+
spec.template.as_mut().unwrap().driver_config = Some(cdi_devices_config(&["nvidia.com/gpu=0"]));
1163+
1164+
let err = DockerComputeDriver::validate_sandbox(&sandbox, &config).unwrap_err();
1165+
1166+
assert_eq!(err.code(), tonic::Code::InvalidArgument);
1167+
assert!(
1168+
err.message()
1169+
.contains("gpu count (2) must match driver_config.cdi_devices length (1)")
1170+
);
1171+
}
1172+
10711173
#[test]
10721174
fn validate_sandbox_rejects_template_errors_before_device_config() {
10731175
let config = runtime_config();
@@ -1153,7 +1255,25 @@ fn build_container_create_body_passes_explicit_cdi_device_id_through() {
11531255
}
11541256

11551257
#[test]
1156-
fn build_container_create_body_rejects_cdi_devices_without_gpu() {
1258+
fn build_container_create_body_rejects_gpu_count_mismatched_cdi_devices() {
1259+
let mut config = runtime_config();
1260+
config.supports_gpu = true;
1261+
let mut sandbox = test_sandbox();
1262+
let spec = sandbox.spec.as_mut().unwrap();
1263+
spec.resource_requirements = Some(gpu_resources(Some(2)));
1264+
spec.template.as_mut().unwrap().driver_config = Some(cdi_devices_config(&["nvidia.com/gpu=0"]));
1265+
1266+
let err = build_container_create_body(&sandbox, &config).unwrap_err();
1267+
1268+
assert_eq!(err.code(), tonic::Code::InvalidArgument);
1269+
assert!(
1270+
err.message()
1271+
.contains("gpu count (2) must match driver_config.cdi_devices length (1)")
1272+
);
1273+
}
1274+
1275+
#[test]
1276+
fn build_container_create_body_rejects_cdi_devices_without_gpu_request() {
11571277
let mut sandbox = test_sandbox();
11581278
sandbox
11591279
.spec
@@ -1166,7 +1286,7 @@ fn build_container_create_body_rejects_cdi_devices_without_gpu() {
11661286

11671287
let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err();
11681288
assert_eq!(err.code(), tonic::Code::InvalidArgument);
1169-
assert!(err.message().contains("requires gpu=true"));
1289+
assert!(err.message().contains("requires a gpu request"));
11701290
}
11711291

11721292
#[test]

crates/openshell-driver-podman/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ The container spec in `container.rs` sets these security-critical fields:
4646
| `no_new_privileges` | `true` | Prevents privilege escalation after exec. |
4747
| `seccomp_profile_path` | `unconfined` | The supervisor installs its own policy-aware BPF filter. A container-level profile can block Landlock/seccomp syscalls during setup. |
4848
| `mounts` | Private tmpfs at `/run/netns` | Lets the supervisor create named network namespaces in rootless Podman. |
49-
| CDI GPU devices | `driver_config.cdi_devices` when set, otherwise all NVIDIA GPUs | Exposes requested GPUs to GPU-enabled sandbox containers. Counted GPU requests are rejected. |
49+
| CDI GPU devices | `driver_config.cdi_devices` when set, otherwise all NVIDIA GPUs | Exposes requested GPUs to GPU-enabled sandbox containers. Count-only GPU requests are rejected; exact CDI device lists with more than one entry require an explicit GPU count matching the device list length. |
5050

5151
The restricted agent child does not retain these supervisor privileges.
5252

0 commit comments

Comments
 (0)