Skip to content

Commit 58a3777

Browse files
authored
fix(drivers): filter bind-backed named volumes (#1861)
* fix(podman): filter bind-backed named volumes Signed-off-by: Evan Lezar <elezar@nvidia.com> * fix(drivers): detect recursive bind-backed volumes Signed-off-by: Evan Lezar <elezar@nvidia.com> --------- Signed-off-by: Evan Lezar <elezar@nvidia.com>
1 parent b6c87a7 commit 58a3777

8 files changed

Lines changed: 321 additions & 32 deletions

File tree

architecture/compute-runtimes.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ but currently ignores them.
4343
Docker and Podman also accept per-sandbox driver-config mounts for existing
4444
runtime-managed named volumes and tmpfs mounts. Podman additionally accepts
4545
image mounts through its image-volume API. User-supplied bind and volume mounts
46-
default to read-only. Direct host bind mounts, and Docker local-driver
46+
default to read-only. Direct host bind mounts, and Docker or Podman local-driver
4747
bind-backed named volumes, are available only when explicitly enabled in the
4848
active local driver table of `gateway.toml`. Host bind mounts are an unsafe
4949
operator override because they place gateway-host filesystem state inside the

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1889,9 +1889,10 @@ fn docker_tmpfs_option(option: &str) -> Result<Vec<String>, Status> {
18891889
fn docker_volume_is_bind_backed(volume: &bollard::models::Volume) -> bool {
18901890
volume.driver == "local"
18911891
&& volume.options.get("o").is_some_and(|options| {
1892-
options
1893-
.split(',')
1894-
.any(|option| option.trim().eq_ignore_ascii_case("bind"))
1892+
options.split(',').any(|option| {
1893+
let option = option.trim();
1894+
option.eq_ignore_ascii_case("bind") || option.eq_ignore_ascii_case("rbind")
1895+
})
18951896
})
18961897
}
18971898

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -901,6 +901,20 @@ fn docker_local_volume_with_bind_option_is_bind_backed() {
901901
assert!(docker_volume_is_bind_backed(&volume));
902902
}
903903

904+
#[test]
905+
fn docker_local_volume_with_rbind_option_is_bind_backed() {
906+
let volume = inspected_volume(
907+
"local",
908+
HashMap::from([
909+
("type".to_string(), "none".to_string()),
910+
("o".to_string(), "rw,rbind".to_string()),
911+
("device".to_string(), "/tmp/openshell".to_string()),
912+
]),
913+
);
914+
915+
assert!(docker_volume_is_bind_backed(&volume));
916+
}
917+
904918
#[test]
905919
fn docker_local_volume_without_bind_option_is_not_bind_backed() {
906920
let volume = inspected_volume(

crates/openshell-driver-podman/README.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,9 @@ mount types:
5959
- `bind`: mounts an absolute host path when `[openshell.drivers.podman]`
6060
has `enable_bind_mounts = true`.
6161
- `volume`: mounts an existing Podman named volume. The driver validates that
62-
the volume exists before provisioning and never creates or removes it.
62+
the volume exists before provisioning and never creates or removes it. Podman
63+
local-driver volumes created with bind options are treated as host bind
64+
mounts and require `enable_bind_mounts = true`.
6365
- `tmpfs`: mounts an in-memory filesystem with optional `options`,
6466
`size_bytes`, and `mode`.
6567
- `image`: mounts an OCI image through Podman's image-volume API. The driver

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

Lines changed: 49 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,16 @@ pub struct PortMappingEntry {
196196
pub host_ip: String,
197197
}
198198

199+
/// A named Podman volume returned by the libpod inspect API.
200+
#[derive(Debug, Clone, Default, serde::Deserialize)]
201+
#[serde(rename_all = "PascalCase")]
202+
pub struct VolumeInspect {
203+
#[serde(default)]
204+
pub driver: String,
205+
#[serde(default)]
206+
pub options: HashMap<String, String>,
207+
}
208+
199209
/// A Podman event from the events stream.
200210
#[derive(Debug, Clone, serde::Deserialize)]
201211
#[serde(rename_all = "PascalCase")]
@@ -497,21 +507,15 @@ impl PodmanClient {
497507
}
498508
}
499509

500-
/// Return whether a named volume exists. Does not create the volume.
501-
pub async fn volume_exists(&self, name: &str) -> Result<bool, PodmanApiError> {
510+
/// Inspect a named volume. Does not create the volume.
511+
pub async fn inspect_volume(&self, name: &str) -> Result<VolumeInspect, PodmanApiError> {
502512
validate_name(name)?;
503-
match self
504-
.request_json::<Value>(
505-
hyper::Method::GET,
506-
&format!("/libpod/volumes/{name}/json"),
507-
None,
508-
)
509-
.await
510-
{
511-
Ok(_) => Ok(true),
512-
Err(PodmanApiError::NotFound(_)) => Ok(false),
513-
Err(e) => Err(e),
514-
}
513+
self.request_json(
514+
hyper::Method::GET,
515+
&format!("/libpod/volumes/{name}/json"),
516+
None,
517+
)
518+
.await
515519
}
516520

517521
// ── Network operations ───────────────────────────────────────────────
@@ -755,6 +759,8 @@ fn url_encode(s: &str) -> String {
755759
#[cfg(test)]
756760
mod tests {
757761
use super::*;
762+
use crate::test_utils::{StubResponse, spawn_podman_stub};
763+
use hyper::StatusCode;
758764

759765
#[test]
760766
fn url_encode_encodes_special_characters() {
@@ -794,4 +800,33 @@ mod tests {
794800
let exact_name = "a".repeat(MAX_NAME_LEN);
795801
assert!(validate_name(&exact_name).is_ok());
796802
}
803+
804+
#[tokio::test]
805+
async fn inspect_volume_parses_driver_options() {
806+
let (socket_path, request_log, handle) = spawn_podman_stub(
807+
"inspect-volume",
808+
vec![StubResponse::new(
809+
StatusCode::OK,
810+
r#"{"Name":"work-bind","Driver":"local","Options":{"type":"none","o":"rw,bind","device":"/srv/work"}}"#,
811+
)],
812+
);
813+
let client = PodmanClient::new(socket_path.clone());
814+
815+
let volume = client
816+
.inspect_volume("work-bind")
817+
.await
818+
.expect("volume inspect should parse");
819+
820+
assert_eq!(volume.driver, "local");
821+
assert_eq!(volume.options.get("o").map(String::as_str), Some("rw,bind"));
822+
handle.await.expect("stub task should finish");
823+
assert_eq!(
824+
request_log
825+
.lock()
826+
.expect("request log lock should not be poisoned")
827+
.as_slice(),
828+
["GET /v5.0.0/libpod/volumes/work-bind/json"]
829+
);
830+
let _ = std::fs::remove_file(socket_path);
831+
}
797832
}

0 commit comments

Comments
 (0)