Skip to content

Conversation

@rata
Copy link
Contributor

@rata rata commented Aug 27, 2025

This backports: #12218

Fixes: #11852 in the 2.1 branch

rata added 2 commits August 27, 2025 12:02
If a Dockerfile is using a `VOLUME` directive and the directory exists
in the rootfs, like in this example:

	FROM docker.io/library/alpine:latest
	VOLUME [ "/run" ]

The alpine container image already contains a "/run" directory. This
will force the code in WithVolumes() to copy its  content to the new
volume created for the VOLUME directive. This copies the content as well
as the ownership.

However, as we perform the mounts from the host POV without being inside
a userns, the idmap option will just shift the IDs in ways that will
screw up the ownerships when copied. We should only use the idmap option
when running the container inside a userns, so the ownerships are fine
(the userns will do a shift and the idmap another, to make it all seem
as if there was no UID/GID shift in the first place).

This PR does just that, remove the idmap option from mounts so we copy
the files without any ID transformations. It's simpler and easier to
reason about if we just don't mount with the idmap option here: all
files are copied just fine without ID transformations and ID
transformation is applied via the idmap option at mount time when
running the pod.

Also, note that `VOLUME` directives that refer to directories that don't
exist on the rootfs work fine (`VOLUME [ "/rata" ]` for example), as
there is no copy done in that case so the permissions weren't changed.

Signed-off-by: Rodrigo Campos <[email protected]>
(cherry picked from commit 41953f7)
Signed-off-by: Rodrigo Campos <[email protected]>
Signed-off-by: Rodrigo Campos <[email protected]>
(cherry picked from commit f0ee598)
Signed-off-by: Rodrigo Campos <[email protected]>
@k8s-ci-robot
Copy link

Hi @rata. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@github-project-automation github-project-automation bot moved this from Needs Triage to Review In Progress in Pull Request Review Aug 27, 2025
@fuweid fuweid merged commit 885991b into containerd:release/2.1 Aug 27, 2025
53 checks passed
@github-project-automation github-project-automation bot moved this from Review In Progress to Done in Pull Request Review Aug 27, 2025
@rata rata deleted the backport-userns-volume-fixes-2.1 branch August 28, 2025 07:41
@austinvazquez austinvazquez changed the title [release/2.1] Backport userns container image volume with copy-up fixes [release/2.1] Fix userns with container image VOLUME mounts that need copy Nov 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants