cri: Fix image volumes with user namespaces by AutuSnow · Pull Request #12816 · containerd/containerd

AutuSnow · 2026-01-24T05:12:45Z

/kind bug
Fixes image volumes failing with "invalid argument" error when used with user namespaces.

Fixes #12812

This PR fixes a compatibility issue between image volumes and user namespaces by:

Retrieving the sandbox's user namespace configuration
Creating idmap labels using the existing snapshotterRemapOpts() function
Passing these labels to the snapshotter's Prepare() call
Allowing the overlay snapshotter to apply uidmap/gidmap options to lower layers

It only affects pods with hostUsers: false and image volumes.

AutuSnow · 2026-01-24T05:13:15Z

/release-note-none

AutuSnow · 2026-01-26T06:04:27Z

@rata May have a look？

rata

@AutuSnow awesome, thanks a lot for the PR!

It seems good, left a small comment. Can we have an end to end test? Is this tested in critools too? In the future we should add tests there too

rata · 2026-01-27T10:02:24Z

+	// Check if the sandbox has Linux security context configured
+	if sandbox.Config.GetLinux() == nil {
+		return nil, nil
+	}
+
+	// Get namespace options and create idmap labels if user namespace is enabled
+	nsOpts := sandbox.Config.GetLinux().GetSecurityContext().GetNamespaceOptions()
+	snapshotOpts, err := snapshotterRemapOpts(nsOpts)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get snapshotter remap options: %w", err)
+	}
+
+	return snapshotOpts, nil


I think this should call snapshotterOpts() that already calls snapshotterRemapOpts() and gets the usrens config things. Once we have the config from the sandbox, just call snapshotterOpts() and return that?

ye， Sorry I ignored this method

I think this should call snapshotterOpts() that already calls snapshotterRemapOpts() and gets the usrens config things. Once we have the config from the sandbox, just call snapshotterOpts() and return that?

Please ignore my reply above. I carefully examined the snapshotterOpts method and found that there are differences in parameter types. In addition, at the call point of mutateImageMount, only the sandbox ID is available, no container configuration, and the semantics look different. Image volume should use the sandbox configuration, and snapshotterOpts seems to be designed for container level rootfs snapshots

Ohh, okay, I see the sandbox config vs container config thing. Next time please make explicit the difference if you don't mind.

But the mutateMounts() thing is called from createContainerRequest() where we do have all that info, so we can easily share the container config with those functions, if we want, right? I think we should pass what the function needs, instead of going from the sandbox via accessors to get the config.

I'm unsure if we should use the podconfig for this or not. Long ago I've added a commit to check podconfig and containerconfig are consistent: #12816 (comment).

I'm unsure what we should do. What do you think of passing the container config and just use that for the mappings?

Ohh, okay, I see the sandbox config vs container config thing. Next time please make explicit the difference if you don't mind.

But the mutateMounts() thing is called from createContainerRequest() where we do have all that info, so we can easily share the container config with those functions, if we want, right? I think we should pass what the function needs, instead of going from the sandbox via accessors to get the config.

I'm unsure if we should use the podconfig for this or not. Long ago I've added a commit to check podconfig and containerconfig are consistent: #12816 (comment).

I'm unsure what we should do. What do you think of passing the container config and just use that for the mappings?

It seems that the commit address you posted is not correct
In my opinion, there are several considerations

Image volume is a pod level resource, and sandbox config should be used

If the two are consistent, container config can also work
These two are the trade-offs between the readability of the reader's code and the reusability of the code. In my opinion, resources at the pod level should be configured at the pod level, which will benefit the later scalability

Wait, the mappings are part of the mount message from cri! : https://github.com/kubernetes/cri-api/blob/ccff438b5dae5751e028196c6d4e167b0f745122/pkg/apis/runtime/v1/api.proto#L251-L253.

We should just use the mappings from there.

We should already have the mappings in mutateImageMount():

containerd/internal/cri/server/container_image_mount.go

Line 71 in 317286a

extraMount *runtime.Mount,

. That is the cri runtime mount struct, that has the field.

So we should just get it from there. Sorry I didn't realize yesterday, it was a crazy day.

Wait, the mappings are part of the mount message from cri! : https://github.com/kubernetes/cri-api/blob/ccff438b5dae5751e028196c6d4e167b0f745122/pkg/apis/runtime/v1/api.proto#L251-L253.

We should just use the mappings from there.

We should already have the mappings in mutateImageMount():

containerd/internal/cri/server/container_image_mount.go

Line 71 in 317286a

extraMount *runtime.Mount,

. That is the cri runtime mount struct, that has the field.
So we should just get it from there. Sorry I didn't realize yesterday, it was a crazy day.

ye， I understand now. Maybe I was too sleepy yesterday, so I didn't find it, or I was confused

rata

@AutuSnow I reviewed a few minutes before you requested already :)

AutuSnow · 2026-01-28T09:51:11Z

@rata I have modified it

rata · 2026-01-28T11:36:43Z

@AutuSnow did you forget to push? I don't see any changes regarding this

AutuSnow · 2026-01-28T11:58:00Z

@AutuSnow did you forget to push? I don't see any changes regarding this

It seems that I forgot to push it just now. Now I can look at it again. It's really a forgetful brain

AutuSnow · 2026-01-28T13:25:38Z

@rata I think the CI reports an error because of insufficient memory

rata

Please, delete the .idea folder, I guess it was added by mistake.

Otherwise it looks fine, but there is a bug. CI is failing the test you added, not OOM. Search the logs (using the github search, not the one on the browser) for "fail:". You will see the test is failing: https://github.com/containerd/containerd/actions/runs/21437274115/job/61731279761?pr=12816#step:20:2230

Can you please fix it (not removing the assert, of course :D)

rata · 2026-01-28T13:56:14Z

Please, delete this file

rata · 2026-01-28T14:33:27Z

+	stdout, stderr, err = runtimeService.ExecSync(cnID, []string{"stat", "-c", "%u:%g", "/image-mount/pause"}, 0)
+	require.NoError(t, err, "failed to stat file in image volume")
+	require.Len(t, stderr, 0)
+	require.Contains(t, string(stdout), "0:0", "files in image volume should appear as owned by root in container's user namespace")


This is failing the test: https://github.com/containerd/containerd/actions/runs/21437274115/job/61731279761?pr=12816#step:20:2230.

I will solve it immediately

No problem. Just ping me when this is solved, so I take another look :)

No problem. Just ping me when this is solved, so I take another look :)

I will give priority to solving this problem when I get up tomorrow, and you will see the perfect CI results tomorrow

That is nice. But please do have a good rest :)

That is nice. But please do have a good rest :)

Thank you for your concern. Unit testing seems to be ready now

rata

@AutuSnow thanks a lot! It looks quite well, left some small comments on the tests.

Do you happen to have a k8s pod yaml handy, so I can test this locally too?

rata · 2026-01-29T11:02:58Z

+	stdout, stderr, err = runtimeService.ExecSync(cnID, []string{"stat", "-c", "%u:%g", "/image-mount/pause"}, 0)
+	require.NoError(t, err, "failed to stat file in image volume")
+	require.Len(t, stderr, 0)
+	require.Contains(t, string(stdout), "0:0", "files in image volume should appear as owned by root in container's user namespace")


This will be true if it returns 1000:05. In other tests (I think in the k/k repo) we are using stat -c '=%u=%g='. This will print something like =0=0=. This way you can match for that string exactly and we can be sure the UID is 0 and not some bigger number that ends with 0 (like 1000).

This will be true if it returns 1000:05. In other tests (I think in the k/k repo) we are using stat -c '=%u=%g='. This will print something like =0=0=. This way you can match for that string exactly and we can be sure the UID is 0 and not some bigger number that ends with 0 (like 1000).

Oh, my god, why didn't I take this into consideration, but I have finished the modification now

rata · 2026-01-29T11:05:23Z

+				UidMappings: []*runtime.IDMapping{
+					{
+						ContainerId: 0,
+						HostId:      65536,
+						Length:      65536,
+					},
+				},


You can move this to a var just before the for, like mappings := []*runtime.IDMapiing{ ... . And the assign it here for uidMappings and GidMappings, and all the other test cases.

rata · 2026-01-29T11:07:18Z

+	}
+}
+
+func TestGetImageVolumeSnapshotOpts_InvalidMappings(t *testing.T) {


Can we move these cases to the previous function and set the expectError bool? Am I missing something?

AutuSnow · 2026-01-29T12:52:50Z

@AutuSnow thanks a lot! It looks quite well, left some small comments on the tests.

Do you happen to have a k8s pod yaml handy, so I can test this locally too?

  apiVersion: v1
  kind: Pod
  metadata:
    name: test-image-volume-userns
  spec:
    hostUsers: false
    containers:
    - name: test
      image: registry.k8s.io/pause:3.9
      volumeMounts:
      - name: image-vol
        mountPath: /image-mountvolumes:
    - name: image-vol
      image:
        reference: registry.k8s.io/pause:3.9
        pullPolicy: IfNotPresent

Does this suit your idea？

AutuSnow · 2026-02-06T08:05:59Z

@rata Can you help me take another look

rata

The integration test is broken, it always skips and never runs. See for example the run on CI: https://github.com/containerd/containerd/actions/runs/21673112539/job/62486266065?pr=12816#step:20:2197. See my comment for more details.

Left some other comments. This is getting close. But please, can you verify things work in a k8s pod and all tests run fine before asking for review next time? :)

If you need help with something, don't hesitate to ask. Of course.

You can run the integration test locally with this:

PATH=/sbin:$PATH FOCUS=TestImageVolumeWithUserNamespace BUILDTAGS=no_btrfs  make cri-integration

Signed-off-by: qiuxue <[email protected]>

fuweid

LGTM

rata

LGTM! @AutuSnow Thanks a lot for the patience, all the iterations and the quick responses!

AutuSnow · 2026-02-09T09:34:42Z

LGTM! @AutuSnow Thanks a lot for the patience, all the iterations and the quick responses!

I feel that your approval is what I deserve, and I am happy that containerd has become better

rata

LGTM

estesp · 2026-02-10T21:01:16Z

/cherry-pick release/2.2

k8s-infra-cherrypick-robot · 2026-02-10T21:01:56Z

@estesp: new pull request created: #12885

Details

In response to this:

/cherry-pick release/2.2

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.

rata · 2026-02-11T09:57:17Z

@estesp do we backport this to 2.1 too? Support for OCI images as source volumes was added in this PR: #10579. That commit is part of 2.1.0 too (git tag --contains 1ec10d9ae7535ddd7b18e3c21b6cd8ff12a2f90d shows it).

But the patch doesn't apply cleanly there.

estesp · 2026-02-11T15:41:55Z

@estesp do we backport this to 2.1 too? Support for OCI images as source volumes was added in this PR: #10579. That commit is part of 2.1.0 too (git tag --contains 1ec10d9ae7535ddd7b18e3c21b6cd8ff12a2f90d shows it).

But the patch doesn't apply cleanly there.

If we had support in 2.1 then the fix should go there as well. @fuweid had set the label on the original PR to just 2.2 so I hadn't personally looked if it was applicable there.

If it won't apply cleanly, then someone will need to do a manual backport and create the PR for release/2.1 branch.

rata · 2026-02-13T12:03:01Z

@estesp verified it is affected with a real pod, and opened the backport that fixes it: #12894. Thanks!

github-project-automation Bot added this to Pull Request Review Jan 24, 2026

k8s-ci-robot added the kind/bug label Jan 24, 2026

github-project-automation Bot moved this to Needs Triage in Pull Request Review Jan 24, 2026

k8s-ci-robot added the size/L label Jan 24, 2026

dosubot Bot added the area/cri Container Runtime Interface (CRI) label Jan 24, 2026

rata reviewed Jan 27, 2026

View reviewed changes

AutuSnow requested a review from rata January 27, 2026 10:27

rata reviewed Jan 27, 2026

View reviewed changes

AutuSnow force-pushed the fix-image-volume-userns branch from d998ed2 to 9032f0d Compare January 27, 2026 14:20

This was referenced Jan 27, 2026

Add validation tests for image volumes with user namespaces kubernetes-sigs/cri-tools#1979

Open

Add validation test for image volumes with user namespaces kubernetes-sigs/cri-tools#1980

Closed

AutuSnow force-pushed the fix-image-volume-userns branch 2 times, most recently from ffce9b5 to 8b70ec4 Compare January 28, 2026 11:57

rata suggested changes Jan 28, 2026

View reviewed changes

github-project-automation Bot moved this from Needs Triage to Needs Update in Pull Request Review Jan 28, 2026

AutuSnow force-pushed the fix-image-volume-userns branch from 073b6a1 to a84b55c Compare January 28, 2026 15:32

k8s-ci-robot added the do-not-merge/contains-merge-commits label Jan 29, 2026

AutuSnow force-pushed the fix-image-volume-userns branch from 557d717 to 65028d7 Compare January 29, 2026 02:20

k8s-ci-robot removed the do-not-merge/contains-merge-commits label Jan 29, 2026

AutuSnow force-pushed the fix-image-volume-userns branch 2 times, most recently from 0509732 to 308e7a9 Compare January 29, 2026 03:02

rata reviewed Jan 29, 2026

View reviewed changes

AutuSnow force-pushed the fix-image-volume-userns branch from 99046ff to 9c0c878 Compare January 29, 2026 12:56

AutuSnow force-pushed the fix-image-volume-userns branch 2 times, most recently from 673e987 to bcd3720 Compare February 4, 2026 13:22

rata suggested changes Feb 6, 2026

View reviewed changes

cri: Fix image volumes with user namespaces

db9546b

Signed-off-by: qiuxue <[email protected]>

AutuSnow force-pushed the fix-image-volume-userns branch from bcd3720 to db9546b Compare February 6, 2026 15:41

fuweid approved these changes Feb 7, 2026

View reviewed changes

fuweid added the cherry-pick/2.2.x Change to be cherry picked to release/2.2 branch label Feb 7, 2026

rata approved these changes Feb 9, 2026

View reviewed changes

AutuSnow requested a review from rata February 9, 2026 09:35

rata approved these changes Feb 9, 2026

View reviewed changes

estesp approved these changes Feb 10, 2026

View reviewed changes

github-project-automation Bot moved this from Needs Update to Review In Progress in Pull Request Review Feb 10, 2026

estesp added this pull request to the merge queue Feb 10, 2026

Merged via the queue into containerd:main with commit 81ac9a8 Feb 10, 2026
54 checks passed

github-project-automation Bot moved this from Review In Progress to Done in Pull Request Review Feb 10, 2026

k8s-infra-cherrypick-robot mentioned this pull request Feb 10, 2026

[release/2.2] cri: Fix image volumes with user namespaces #12885

Merged

AutuSnow deleted the fix-image-volume-userns branch February 11, 2026 01:26

rata mentioned this pull request Feb 13, 2026

[release/2.1] cri: Fix image volumes with user namespaces #12894

Merged

mhulscher mentioned this pull request Mar 6, 2026

[FEATURE] Support user namespaces (hostUsers: false) awslabs/soci-snapshotter#1888

Open

chrishenzie added cherry-picked/2.1.x PR commits are cherry picked into the release/2.1 branch cherry-picked/2.2.x PR commits are cherry-picked into release/2.2 branch and removed cherry-pick/2.2.x Change to be cherry picked to release/2.2 branch labels Apr 14, 2026

Conversation

AutuSnow commented Jan 24, 2026

Uh oh!

AutuSnow commented Jan 24, 2026

Uh oh!

AutuSnow commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rata left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rata left a comment

Choose a reason for hiding this comment

Uh oh!

AutuSnow commented Jan 28, 2026

Uh oh!

rata commented Jan 28, 2026

Uh oh!

AutuSnow commented Jan 28, 2026

Uh oh!

AutuSnow commented Jan 28, 2026

Uh oh!

rata left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rata left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rata Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AutuSnow commented Jan 29, 2026

Uh oh!

AutuSnow commented Feb 6, 2026

Uh oh!

rata left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AutuSnow commented Jan 26, 2026 •

edited

Loading

rata Jan 29, 2026 •

edited

Loading

rata commented Feb 13, 2026 •

edited

Loading