Skip to content

feat(docker,podman): add SELinux label support for bind mounts#2092

Merged
drew merged 2 commits into
NVIDIA:mainfrom
bergmannf:feat/docker-selinux-bind-labels
Jul 2, 2026
Merged

feat(docker,podman): add SELinux label support for bind mounts#2092
drew merged 2 commits into
NVIDIA:mainfrom
bergmannf:feat/docker-selinux-bind-labels

Conversation

@bergmannf

Copy link
Copy Markdown
Contributor

Summary

Add optional selinux_label field to bind mount driver configs for both Docker and Podman drivers. On SELinux-enabled hosts (Fedora, RHEL), bind-mounted paths need relabelling so container processes can access them. This was missing from the bind mount support added in #1785.

Related Issue

Extends #1785

Changes

  • Add shared SelinuxLabel enum (shared / private) to openshell-core::driver_mounts
  • Docker driver: move user bind mounts from the structured Mount API (which lacks SELinux support) to the legacy string-format Binds field, appending :z or :Z when selinux_label is set
  • Podman driver: add selinux_label to PodmanDriverMountConfig::Bind and push z/Z to the mount options vec
  • Update docs/reference/sandbox-compute-drivers.mdx mount schema tables for both drivers

Testing

  • cargo fmt --all -- --check passes
  • cargo clippy --workspace passes
  • Unit tests added/updated (5 new tests across Docker and Podman drivers)
  • Manually tested with Podman on Fedora
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)

@copy-pr-bot

copy-pr-bot Bot commented Jul 1, 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 1, 2026

Copy link
Copy Markdown

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

@bergmannf bergmannf force-pushed the feat/docker-selinux-bind-labels branch from 9372e33 to dc6a562 Compare July 1, 2026 14:32
@bergmannf

Copy link
Copy Markdown
Contributor Author

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

@bergmannf

Copy link
Copy Markdown
Contributor Author

recheck

@maxamillion

Copy link
Copy Markdown
Collaborator

/ok to test dc6a562

@maxamillion

Copy link
Copy Markdown
Collaborator

@drew @TaylorMutch I'm good with the podman changes here but I don't know enough about docker to know if falling back to the legacy string-format Binds is a problem or not and would like and extra +1 (cc @ericcurtin)

@ericcurtin

ericcurtin commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

@maxamillion could we do CI maybe? Using the new RHEL 10 runners?

https://github.blog/changelog/2026-06-25-red-hat-enterprise-linux-runner-images-are-now-in-public-preview/

Install Docker Engine:

curl -fsSL https://get.docker.com | bash
sed -i "s#/usr/bin/dockerd#/usr/bin/dockerd --ip-forward-no-drop#g" /usr/lib/systemd/system/docker.service 

Test on both podman/docker on RHEL 10 in CI (above is a change required to get podman and docker running on the same machine).

@ericcurtin

Copy link
Copy Markdown
Contributor

But to answer the question:

I believe the string-format binds is the only way to do it correctly today in docker.

I still think the RHEL 10 podman/docker builds could be useful though to test cross-compatibility.

The Docker Engine structured Mount API does not support SELinux
relabelling (:z / :Z). Move user-supplied bind mounts from the
structured `mounts` field to the legacy string-format `binds` field,
which does support these options.

Add a shared `SelinuxLabel` enum (shared/private) to openshell-core so
both Docker and Podman drivers accept an optional `selinux_label` field
on bind mount configs. For Docker, labels are appended to the bind
string; for Podman, they are pushed to the mount options vec.

Signed-off-by: Florian Bergmann <fbergman@redhat.com>
@bergmannf bergmannf force-pushed the feat/docker-selinux-bind-labels branch from dc6a562 to 087148e Compare July 1, 2026 15:27
@TaylorMutch

Copy link
Copy Markdown
Collaborator

/ok to test 087148e

@ericcurtin

Copy link
Copy Markdown
Contributor

Opened #2093 to add the RHEL 10 Docker/Podman/SELinux CI suite discussed above. It installs Docker alongside Podman on a bare RHEL 10 runner (the get.docker.com + --ip-forward-no-drop steps from #2092 (comment)), confirms SELinux is enforcing, and runs both e2e:docker and e2e:podman against the same host, failing on any AVC denials. It needs an org-provisioned RHEL 10 larger runner before it can actually execute — flagged in the PR description.

@maxamillion

Copy link
Copy Markdown
Collaborator

@ericcurtin good call, I would like to setup RHEL 10 runners here for CI as well. I'll see if we can get that added.

@krishicks krishicks left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docker treats structured mounts and legacy binds differently with respect to missing source directories: it errors if the source path is missing on a structured mount, but silently creates the missing source as an empty, root-owned directory on a legacy bind. Since this PR moves bind mounts from the structured Mount API onto the legacy Binds field, should we make sure we preserve this fail-fast behavior, and any other beneficial behavior specific to the structured mount syntax?

From the docs:

If you use --volume to bind-mount a file or directory that does not yet exist on the Docker host, Docker automatically creates the directory on the host for you. It's always created as a directory.

By default, --mount does not automatically create a directory if the specified mount path does not exist on the host. Instead, it produces an error

Comment thread crates/openshell-driver-docker/src/lib.rs Outdated
Moving user bind mounts from the structured Mount API to the legacy
Binds field changed Docker's behavior for missing source directories:
the legacy path silently creates them as empty root-owned dirs instead
of erroring. Add an explicit Path::exists() check to preserve the
fail-fast behavior operators expect.

Signed-off-by: Florian Bergmann <fbergman@redhat.com>
@bergmannf bergmannf force-pushed the feat/docker-selinux-bind-labels branch from 7b31056 to 60ddd00 Compare July 2, 2026 07:04
@bergmannf

Copy link
Copy Markdown
Contributor Author

Docker treats structured mounts and legacy binds differently with respect to missing source directories: it errors if the source path is missing on a structured mount, but silently creates the missing source as an empty, root-owned directory on a legacy bind. Since this PR moves bind mounts from the structured Mount API onto the legacy Binds field, should we make sure we preserve this fail-fast behavior, and any other beneficial behavior specific to the structured mount syntax?

From the docs:

If you use --volume to bind-mount a file or directory that does not yet exist on the Docker host, Docker automatically creates the directory on the host for you. It's always created as a directory.
By default, --mount does not automatically create a directory if the specified mount path does not exist on the host. Instead, it produces an error

Thanks for the review - I've added the explicit check for Docker. For Podman this seems to already do the right thing via the API: trying to create a sandbox with a non-existent source path like so will already return an error:

+ openshell sandbox create --name dev --driver-config-json '{"podman":{"mounts":[{"type":"bind","source":"/tmp/does/not/exist","target":"/sandbox/work","read_only":false, "selinux_label": "private"}]}}' --tty -- bash
Error:   × code: 'Internal error', message: "create sandbox failed: podman API error (500): statfs /tmp/does/not/exist: no such file or

@krishicks

Copy link
Copy Markdown
Collaborator

/ok to test 60ddd00

@drew drew merged commit 43bb030 into NVIDIA:main Jul 2, 2026
32 checks passed
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.

6 participants