Skip to content

Commit 2c3ecbd

Browse files
refactor(podman): simplify inspected image metadata and test fixtures
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
1 parent 800e3b8 commit 2c3ecbd

3 files changed

Lines changed: 44 additions & 70 deletions

File tree

‎crates/openshell-driver-podman/src/client.rs‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1305,22 +1305,13 @@ mod tests {
13051305
.expect("image inspect should parse");
13061306

13071307
assert_eq!(image.id, "sha256:immutable");
1308-
assert_eq!(
1309-
image.config.as_ref().map(|config| config.user.as_str()),
1310-
Some("app:staff")
1311-
);
1312-
assert_eq!(
1313-
image
1314-
.config
1315-
.as_ref()
1316-
.map(|config| config.working_dir.as_str()),
1317-
Some("/workspace/project")
1318-
);
1308+
let config = image.config.expect("fixture image config");
1309+
assert_eq!(config.user, "app:staff");
1310+
assert_eq!(config.working_dir, "/workspace/project");
13191311
assert!(
1320-
image
1321-
.config
1312+
config
1313+
.volumes
13221314
.as_ref()
1323-
.and_then(|config| config.volumes.as_ref())
13241315
.is_some_and(|volumes| volumes.contains_key("/workspace/project/cache"))
13251316
);
13261317
handle.await.expect("stub task should finish");

‎crates/openshell-driver-podman/src/container.rs‎

Lines changed: 38 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -228,18 +228,16 @@ impl ResolvedPodmanImage {
228228
/// - a malformed or reserved working directory;
229229
/// - for a custom working directory, an image volume with an invalid or
230230
/// reserved target, or one covering the resolved workspace.
231-
pub fn from_inspect(inspected: &ImageInspect) -> Result<Self, ComputeDriverError> {
232-
let image_config = inspected.config.as_ref();
233-
let workspace_root = driver_mounts::resolve_oci_workspace_root(
234-
image_config.map_or("", |config| config.working_dir.as_str()),
235-
)
236-
.map_err(ComputeDriverError::Precondition)?;
231+
pub fn from_inspect(inspected: ImageInspect) -> Result<Self, ComputeDriverError> {
232+
let config = inspected.config.unwrap_or_default();
233+
let workspace_root = driver_mounts::resolve_oci_workspace_root(&config.working_dir)
234+
.map_err(ComputeDriverError::Precondition)?;
237235
for control_path in PODMAN_WORKLOAD_CONTROL_PATHS {
238236
driver_mounts::validate_workspace_control_path(&workspace_root, control_path)
239237
.map_err(ComputeDriverError::Precondition)?;
240238
}
241239
if workspace_root != driver_mounts::DEFAULT_WORKSPACE_ROOT
242-
&& let Some(volumes) = image_config.and_then(|config| config.volumes.as_ref())
240+
&& let Some(volumes) = config.volumes.as_ref()
243241
{
244242
for volume in volumes.keys() {
245243
validate_podman_mount_target(volume).map_err(|error| {
@@ -257,31 +255,31 @@ impl ResolvedPodmanImage {
257255
}
258256
}
259257
Ok(Self {
260-
id: inspected.id.clone(),
261-
oci_user: image_config
262-
.map_or("", |config| config.user.as_str())
263-
.to_string(),
264-
environment: image_config.map_or_else(Vec::new, |config| config.env.clone()),
258+
id: inspected.id,
259+
oci_user: config.user,
260+
environment: config.env,
265261
workspace_root,
266262
})
267263
}
268264

269-
/// Image reference without inspected metadata; uses the managed workspace.
270-
#[cfg(test)]
271-
fn unpinned(image: &str) -> Self {
272-
Self {
273-
id: image.to_string(),
274-
oci_user: String::new(),
275-
environment: Vec::new(),
276-
workspace_root: driver_mounts::DEFAULT_WORKSPACE_ROOT.to_string(),
277-
}
278-
}
279-
280265
pub(crate) fn uses_managed_workspace(&self) -> bool {
281266
self.workspace_root == driver_mounts::DEFAULT_WORKSPACE_ROOT
282267
}
283268
}
284269

270+
#[cfg(test)]
271+
fn resolved_image(id: &str, user: &str, working_dir: &str) -> ResolvedPodmanImage {
272+
ResolvedPodmanImage::from_inspect(ImageInspect {
273+
id: id.to_string(),
274+
config: Some(crate::client::ImageConfig {
275+
user: user.to_string(),
276+
working_dir: working_dir.to_string(),
277+
..Default::default()
278+
}),
279+
})
280+
.expect("fixture image metadata should be valid")
281+
}
282+
285283
// ---------------------------------------------------------------------------
286284
// Typed container spec structs for the Podman libpod create API.
287285
// ---------------------------------------------------------------------------
@@ -1131,7 +1129,7 @@ pub fn build_container_spec_with_token_and_gpu_devices(
11311129
gpu_device_ids: Option<&[String]>,
11321130
) -> Result<Value, ComputeDriverError> {
11331131
let image = resolve_image(sandbox, config);
1134-
let resolved_image = ResolvedPodmanImage::unpinned(image);
1132+
let resolved_image = resolved_image(image, "", "");
11351133
build_container_spec_for_image(
11361134
sandbox,
11371135
config,
@@ -1811,22 +1809,6 @@ mod tests {
18111809
static ENV_LOCK: std::sync::LazyLock<std::sync::Mutex<()>> =
18121810
std::sync::LazyLock::new(|| std::sync::Mutex::new(()));
18131811

1814-
fn resolved_image(id: &str, user: &str, working_dir: &str) -> ResolvedPodmanImage {
1815-
ResolvedPodmanImage::from_inspect(&ImageInspect {
1816-
id: id.to_string(),
1817-
config: Some(ImageConfig {
1818-
user: user.to_string(),
1819-
env: vec![
1820-
"LD_PRELOAD=/hostile.so".into(),
1821-
"HTTP_PROXY=http://bypass".into(),
1822-
],
1823-
working_dir: working_dir.to_string(),
1824-
volumes: None,
1825-
}),
1826-
})
1827-
.unwrap()
1828-
}
1829-
18301812
#[test]
18311813
fn isolated_pair_keeps_privileges_network_and_secrets_out_of_workload() {
18321814
let sandbox = DriverSandbox {
@@ -1847,7 +1829,11 @@ mod tests {
18471829
"sha256:image".into(),
18481830
)
18491831
.unwrap();
1850-
let image = resolved_image("sha256:image", "1000:1001", "");
1832+
let mut image = resolved_image("sha256:image", "1000:1001", "");
1833+
image.environment = vec![
1834+
"LD_PRELOAD=/hostile.so".into(),
1835+
"HTTP_PROXY=http://bypass".into(),
1836+
];
18511837
let specs = build_isolation_specs(IsolationSpecInput {
18521838
sandbox: &sandbox,
18531839
config: &config,
@@ -2013,14 +1999,14 @@ mod tests {
20131999
}),
20142000
};
20152001

2016-
assert!(ResolvedPodmanImage::from_inspect(&inspect("/workspace")).is_err());
2017-
let image = ResolvedPodmanImage::from_inspect(&inspect("/workspace/project/cache"))
2002+
assert!(ResolvedPodmanImage::from_inspect(inspect("/workspace")).is_err());
2003+
let image = ResolvedPodmanImage::from_inspect(inspect("/workspace/project/cache"))
20182004
.expect("image volumes nested below the workspace remain valid");
20192005
assert_eq!(image.workspace_root, "/workspace/project");
20202006

20212007
let mut fallback = inspect("/etc/openshell");
20222008
fallback.config.as_mut().unwrap().working_dir = "/sandbox".into();
2023-
ResolvedPodmanImage::from_inspect(&fallback)
2009+
ResolvedPodmanImage::from_inspect(fallback)
20242010
.expect("existing /sandbox images keep their image-volume behavior");
20252011
}
20262012

@@ -3130,19 +3116,16 @@ mod tests {
31303116

31313117
#[test]
31323118
fn resolved_workspace_rejects_covering_driver_mount() {
3133-
use openshell_core::proto::compute::v1::{DriverSandboxSpec, DriverSandboxTemplate};
3134-
31353119
let image = resolved_image("sha256:immutable", "1000:1000", "/workspace/project");
31363120
let mut sandbox = test_sandbox("test-id", "test-name");
3137-
sandbox.spec = Some(DriverSandboxSpec {
3138-
template: Some(DriverSandboxTemplate {
3139-
driver_config: Some(json_struct(serde_json::json!({
3140-
"mounts": [{"type": "tmpfs", "target": "/workspace"}]
3141-
}))),
3142-
..Default::default()
3143-
}),
3144-
..Default::default()
3145-
});
3121+
sandbox
3122+
.spec
3123+
.get_or_insert_default()
3124+
.template
3125+
.get_or_insert_default()
3126+
.driver_config = Some(json_struct(serde_json::json!({
3127+
"mounts": [{"type": "tmpfs", "target": "/workspace"}]
3128+
})));
31463129
let error = build_container_spec_for_image(
31473130
&sandbox,
31483131
&test_config(),

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -969,7 +969,7 @@ impl PodmanComputeDriver {
969969
)));
970970
}
971971
let resolved_image =
972-
container::ResolvedPodmanImage::from_inspect(&inspected_image)?;
972+
container::ResolvedPodmanImage::from_inspect(inspected_image)?;
973973

974974
for mount_image in container::podman_driver_image_mount_sources(
975975
sandbox,

0 commit comments

Comments
 (0)