Skip to content

feat(driver-podman): add userns config - #2562

Open
giuseppe wants to merge 2 commits into
NVIDIA:mainfrom
giuseppe:user-namespaces
Open

feat(driver-podman): add userns config#2562
giuseppe wants to merge 2 commits into
NVIDIA:mainfrom
giuseppe:user-namespaces

Conversation

@giuseppe

@giuseppe giuseppe commented Jul 30, 2026

Copy link
Copy Markdown

Summary

Add user namespace support to the Podman compute driver. The userns config option maps to Podman's namespace modes with parameterized value support (auto:size=65536, keep-id:uid=1000,gid=1000). When userns is configured, the driver extracts the supervisor binary to a host-side cache and bind-mounts it instead of using an OCI image volume, because the kernel does not support idmapped mounts on overlay.

A preparatory refactor commit extracts shared supervisor binary helpers (extraction, caching, ELF validation) from the Docker driver into openshell-core::driver_utils so both drivers reuse the same code with hardened integrity checks.

Related Issue

Fixes: #2554

Changes

Commit 1: refactor(driver): extract shared supervisor binary helpers into openshell-core

  • Move extract_first_tar_entry, write_cache_binary_atomic, supervisor_cache_path,
    temp_extract_container_name, and validate_linux_elf_binary from Docker driver to
    openshell-core::driver_utils
  • Add entry-type and empty-payload checks to extract_first_tar_entry
  • Parameterize supervisor_cache_path by driver name (docker-supervisor vs podman-supervisor)

Commit 2: feat(driver-podman): add userns config with supervisor bind-mount fallback

  • Add userns field to PodmanComputeConfig (TOML userns, CLI --userns, env
    OPENSHELL_PODMAN_USERNS)
  • Split mode string on first : into Podman API nsmode + value fields
  • Set idmappings.AutoUserNs = true when base mode is auto
  • Validate mode at startup: auto/keep-id accept params, host/private/nomap reject them
  • Extract supervisor binary to host cache and bind-mount when userns is active (all modes except host)
  • Add copy_from_container API method to PodmanClient
  • Update architecture docs, gateway config reference, and driver README

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

@copy-pr-bot

copy-pr-bot Bot commented Jul 30, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown

All contributors have signed the DCO ✍️ ✅
Posted by the DCO Assistant Lite bot.

@giuseppe giuseppe changed the title feat(driver-podman): add userns config with supervisor bind-mount fallback feat(driver-podman): add userns config Jul 30, 2026
@giuseppe
giuseppe force-pushed the user-namespaces branch 2 times, most recently from 32a94ab to 31a1f54 Compare July 31, 2026 07:21
@mrunalp

mrunalp commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Potential Concerns

  1. Resource leak on extraction failure — driver.rs:647-655
    Every other error path in this block calls cleanup_created() first. This one uses ?:
    let supervisor_bin_path = if self.config.userns.is_some() {
    Some(extract_supervisor_bin(&self.client, &self.config).await
    .map_err(ComputeDriverError::from)?,) // <- skips cleanup_created()
    } else { None };
    A failed extraction orphans the workspace volume and both per-sandbox secrets.

  2. Introduces two clippy warnings → mise run lint fails. tasks/rust.toml:14 runs cargo clippy --workspace --all-targets -- -D warnings. Locally:

  • clippy::too_many_arguments — build_container_spec_for_image is now 8/7 (container.rs:930)
  • clippy::useless_conversion — the redundant .map_err(ComputeDriverError::from) at driver.rs:651
  1. Duplicates the Docker driver's extraction code, minus its integrity check. openshell-driver-docker/src/lib.rs:3149-3415 already has extract_supervisor_bin_from_image, extract_supervisor_binary_bytes, extract_first_tar_entry, write_cache_binary_atomic, supervisor_cache_path, and a byte-identical temp_extract_container_name. The Docker version calls validate_linux_elf_binary both after write and on cache hit; the Podman copy trusts cache_path.is_file() blindly. Since this file becomes the sandbox's security-enforcement supervisor, a truncated, corrupt, or symlinked cache entry executes unchecked. These belong in openshell-core::driver_utils shared by both drivers.

  2. userns values other than bare auto are likely broken. Podman's Namespace splits mode and value, so auto:size=65536 or keep-id:uid=1000 would be sent as an invalid nsmode and also skip AutoUserNs. userns = "host" silently disables userns isolation and ns:/path joins an arbitrary namespace — worth an allowlist plus a config-load error instead of an obscure Podman API failure at create time.

  3. No tests for the new extraction path. The four added tests cover spec shape only. extract_first_tar_entry (which doesn't check entry type or non-empty payload — a symlinked /openshell-sandbox yields a 0-byte "binary"), supervisor_cache_path, and write_cache_binary_atomic are untested, though the Docker equivalents have tests at driver-docker/src/tests.rs:2199-2223 that could be lifted.

  4. Stale architecture doc. architecture/compute-runtimes.md:173 still says Podman delivers the supervisor via "Read-only OCI image volume" — now conditional. Line 120 mentions image volumes too. AGENTS.md requires this in the same branch.

  5. Process gaps. DCO check is failing — the contributor-assistant bot needs the sign-off comment on the PR, not just the trailer. The PR body is the unfilled template (no Summary, Related Issue, Testing, or checked boxes), and a feature needs a linked accepted issue per AGENTS.md. No pre-flight for --userns=auto, which needs a larger subuid range than the existing check_subuid_range warning covers.

Minor: extraction runs on the sandbox-create path rather than driver startup (first create per digest pays inspect + create + download + remove); the mid-file use openshell_core::driver_utils::SUPERVISOR_IMAGE_BINARY_PATH; at driver.rs:964 breaks the file's import convention; #[serde(rename = "AutoUserNs")] would be cleaner than #[allow(non_snake_case)].

@giuseppe

giuseppe commented Aug 3, 2026

Copy link
Copy Markdown
Author

I have read the DCO document and I hereby sign the DCO.

@giuseppe

giuseppe commented Aug 3, 2026

Copy link
Copy Markdown
Author

thanks! Comments addressed

@mrunalp

mrunalp commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

• ## PR review: Request changes

Findings

  1. [P2] Canonicalize namespace modes before serialization.
    Validation is case-insensitive and accepts nomap, but the container spec forwards the original value. Podman expects no-map, so nomap always fails; uppercase values such as AUTO also pass startup validation but fail creation. Normalize to Podman’s canonical values. PR code
    (

    userns: config.userns.as_deref().map(|raw| {
    let (base, params) = raw
    .split_once(':')
    .map_or((raw, None), |(b, p)| (b, Some(p)));
    UserNS {
    nsmode: base.to_string(),
    value: params.map(ToString::to_string),
    }
    ), Podman contract
    (https://github.com/containers/podman/blob/f3829809246caaa533c1af29ecf27f613d3d2fd7/pkg/specgen/namespaces.go#L52-L65).

  2. [P2] Reject or fully implement private.
    The config accepts private, but only auto receives idmappings. Podman requires at least one UID/GID mapping for a private user namespace, so every sandbox creation using this advertised mode fails. Either remove it from the allowlist or add mapping configuration. PR validation
    (

    /// Validate the optional `userns` mode against the supported allowlist.
    ///
    /// Supported modes: `auto` (with optional params, e.g. `auto:size=65536`),
    /// `host`, `keep-id` (with optional params), `private`, and `nomap`.
    /// Modes that don't accept parameters (`host`, `private`, `nomap`) are
    /// rejected when a colon-separated suffix is present.
    pub fn validate_userns(&self) -> Result<(), crate::client::PodmanApiError> {
    let Some(mode) = self.userns.as_deref() else {
    return Ok(());
    };
    let (base, has_params) = mode
    .split_once(':')
    .map_or((mode, false), |(b, _)| (b, true));
    match base.to_ascii_lowercase().as_str() {
    "auto" | "keep-id" => Ok(()),
    "host" | "private" | "nomap" => {
    if has_params {
    Err(crate::client::PodmanApiError::InvalidInput(format!(
    "userns mode '{base}' does not accept parameters",
    )))
    } else {
    Ok(())
    }
    }
    _ => Err(crate::client::PodmanApiError::InvalidInput(format!(
    "unsupported userns mode '{mode}'; \
    supported modes: auto, host, keep-id, nomap, private",
    ))),
    }
    ), Podman requirement
    (https://github.com/containers/podman/blob/f3829809246caaa533c1af29ecf27f613d3d2fd7/pkg/specgen/namespaces.go#L530-L543).

Required follow-ups

  • Add a real Podman userns=auto sandbox E2E; unit tests only verify generated JSON and currently encode the incorrect nomap value.
  • Update debug-openshell-cluster with userns/cache/bind-mount troubleshooting.

…hell-core

Move supervisor binary extraction, caching, and validation helpers from
the Docker driver into openshell-core::driver_utils so both Docker and
Podman drivers can reuse them.

Moved helpers: extract_first_tar_entry, write_cache_binary_atomic,
supervisor_cache_path, temp_extract_container_name, and
validate_linux_elf_binary.

The shared extract_first_tar_entry gains entry-type and empty-payload
checks that the Docker-local version lacked.  supervisor_cache_path
takes a driver_subdir parameter so each driver caches under its own
namespace (docker-supervisor vs podman-supervisor).

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
…lback

Add a `userns` option to the Podman compute driver that maps to
Podman's user namespace modes.  The mode string is split on the first
colon into the API's `nsmode` and `value` fields so parameterized
values like `auto:size=65536` and `keep-id:uid=1000,gid=1000` are
forwarded correctly.  When the mode is `auto`, the container spec
also sets `idmappings.AutoUserNs = true` as required by the API.

An allowlist validates the mode at startup: `auto` and `keep-id`
accept optional parameters; `host`, `private`, and `nomap` reject
them; everything else is an error.

Podman image volumes use overlay mounts internally and the kernel
does not support idmapped mounts on overlay (`mount_setattr` returns
EINVAL).  When userns is configured (any mode except `host`), the
driver extracts the supervisor binary from the image to a host-side
cache and bind-mounts it instead of using an image volume.

Configurable via TOML `userns = "auto"`, CLI `--userns`, or
environment variable `OPENSHELL_PODMAN_USERNS`.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(driver-podman): add user namespace (userns) support for sandbox containers

2 participants