feat(docker,podman): add SELinux label support for bind mounts#2092
feat(docker,podman): add SELinux label support for bind mounts#2092bergmannf wants to merge 1 commit into
Conversation
|
All contributors have signed the DCO ✍️ ✅ |
9372e33 to
dc6a562
Compare
|
I have read the DCO document and I hereby sign the DCO. |
|
recheck |
|
/ok to test dc6a562 |
|
@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 |
|
@maxamillion could we do CI maybe? Using the new RHEL 10 runners? Install Docker Engine: 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). |
|
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>
dc6a562 to
087148e
Compare
|
/ok to test 087148e |
|
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 + |
|
@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. |
There was a problem hiding this comment.
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
| .filter(|m| !matches!(m, DockerDriverMountConfig::Bind { .. })) | ||
| .map(docker_mount_from_config) |
There was a problem hiding this comment.
Instead of filtering out bind mounts here, could you instead do
| .filter(|m| !matches!(m, DockerDriverMountConfig::Bind { .. })) | |
| .map(docker_mount_from_config) | |
| .filter_map(|m| docker_mount_from_config(m).transpose()) |
and have docker_mount_from_config match DockerDriverMountConfig::Bind { .. } => Ok(None)?
Summary
Add optional
selinux_labelfield 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
SelinuxLabelenum (shared/private) toopenshell-core::driver_mountsMountAPI (which lacks SELinux support) to the legacy string-formatBindsfield, appending:zor:Zwhenselinux_labelis setselinux_labeltoPodmanDriverMountConfig::Bindand pushz/Zto the mount options vecdocs/reference/sandbox-compute-drivers.mdxmount schema tables for both driversTesting
cargo fmt --all -- --checkpassescargo clippy --workspacepassesChecklist