Skip to content

Commit 172ba65

Browse files
AutuSnowk8s-infra-cherrypick-robot
AutuSnow
authored and
k8s-infra-cherrypick-robot
committed
cri: Fix image volumes with user namespaces
Signed-off-by: qiuxue <[email protected]>
1 parent befea1e commit 172ba65

5 files changed

Lines changed: 265 additions & 1 deletion

File tree

‎integration/image_volume_linux_test.go‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/containerd/containerd/v2/integration/images"
2929
kernel "github.com/containerd/containerd/v2/pkg/kernelversion"
3030
"github.com/containerd/containerd/v2/pkg/namespaces"
31+
"github.com/containerd/containerd/v2/pkg/sys"
3132
"github.com/containerd/errdefs"
3233
"github.com/opencontainers/image-spec/identity"
3334
"github.com/opencontainers/selinux/go-selinux"
@@ -324,3 +325,69 @@ func TestImageVolumeSetupIfContainerdRestarts(t *testing.T) {
324325
})
325326
}
326327
}
328+
329+
func TestImageVolumeWithUserNamespace(t *testing.T) {
330+
// Check if user namespace and idmap are supported
331+
if !supportsUserNS() {
332+
t.Skip("user namespace not supported")
333+
}
334+
335+
// Check if pidfd is supported
336+
if !sys.SupportsPidFD() {
337+
t.Skip("pidfd not supported")
338+
}
339+
340+
if !supportsIDMap(defaultRoot) {
341+
t.Skipf("idmap mounts not supported on: %s", defaultRoot)
342+
}
343+
344+
containerID := uint32(0)
345+
hostID := uint32(65536)
346+
size := uint32(65536)
347+
348+
containerImage := images.Get(images.Alpine)
349+
imageVolumeImage := images.Get(images.Pause)
350+
351+
podLogDir := t.TempDir()
352+
podOpts := []PodSandboxOpts{
353+
WithPodLogDirectory(podLogDir),
354+
WithPodUserNs(containerID, hostID, size),
355+
}
356+
podCtx := newPodTCtx(t, runtimeService, t.Name(), "image-volume-userns", podOpts...)
357+
defer podCtx.stop(true)
358+
359+
pullImagesByCRI(t, imageService, containerImage, imageVolumeImage)
360+
361+
// Create a container with image volume mount
362+
// Pass the user namespace ID mappings to the image volume mount so that
363+
// idmap is applied and files appear with correct ownership in the container
364+
uidMaps := []*criruntime.IDMapping{{ContainerId: containerID, HostId: hostID, Length: size}}
365+
gidMaps := []*criruntime.IDMapping{{ContainerId: containerID, HostId: hostID, Length: size}}
366+
367+
containerName := "test-container"
368+
cfg := ContainerConfig(containerName, containerImage,
369+
WithCommand("sleep", "1d"),
370+
WithIDMapImageVolumeMount(imageVolumeImage, "", "/image-mount", uidMaps, gidMaps),
371+
WithLogPath(containerName),
372+
WithUserNamespace(containerID, hostID, size),
373+
)
374+
cnID, err := podCtx.rSvc.CreateContainer(podCtx.id, cfg, podCtx.cfg)
375+
require.NoError(t, err, "failed to create container with image volume and user namespace")
376+
377+
require.NoError(t, podCtx.rSvc.StartContainer(cnID), "failed to start container")
378+
379+
// Verify that the image volume is accessible
380+
stdout, stderr, err := runtimeService.ExecSync(cnID, []string{"ls", "/image-mount/pause"}, 0)
381+
require.NoError(t, err, "failed to access image volume")
382+
require.Len(t, stderr, 0)
383+
require.Contains(t, string(stdout), "pause", "image volume should contain pause binary")
384+
385+
_, _, err = runtimeService.ExecSync(cnID, []string{"rm", "/image-mount/pause"}, 0)
386+
require.Error(t, err, "image volume should be read-only")
387+
require.Contains(t, err.Error(), "Read-only file system", "error should indicate read-only filesystem")
388+
389+
stdout, stderr, err = runtimeService.ExecSync(cnID, []string{"stat", "-c", "=%u=%g=", "/image-mount/pause"}, 0)
390+
require.NoError(t, err, "failed to stat file in image volume")
391+
require.Len(t, stderr, 0)
392+
require.Contains(t, string(stdout), "=0=0=", "files in image volume should appear as owned by root in container's user namespace")
393+
}

‎internal/cri/server/container_image_mount.go‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,14 @@ func (c *criService) mutateImageMount(
124124
}
125125
chainID := identity.ChainID(diffIDs).String()
126126

127+
// Get snapshot options with user namespace idmap labels if needed
128+
snapshotOpts, err := c.getImageVolumeSnapshotOpts(ctx, extraMount)
129+
if err != nil {
130+
return fmt.Errorf("failed to get snapshot options for image volume: %w", err)
131+
}
132+
127133
s := c.client.SnapshotService(snapshotter)
128-
mounts, err := s.Prepare(ctx, target, chainID)
134+
mounts, err := s.Prepare(ctx, target, chainID, snapshotOpts...)
129135
if err != nil {
130136
if errdefs.IsAlreadyExists(err) {
131137
mounts, err = s.Mounts(ctx, target)
@@ -160,6 +166,15 @@ func (c *criService) mutateImageMount(
160166
}
161167

162168
extraMount.HostPath = target
169+
170+
// Clear UID/GID mappings from the mount to prevent the OCI runtime from
171+
// attempting idmap on the bind mount. The idmap is already applied to the
172+
// overlay lower layers via the snapshotter when the image volume is prepared.
173+
// This must be done regardless of whether the image volume was already mounted
174+
// (e.g., by another container in the same pod).
175+
extraMount.UidMappings = nil
176+
extraMount.GidMappings = nil
177+
163178
return nil
164179
}
165180

‎internal/cri/server/container_image_mount_linux.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,16 @@
1919
package server
2020

2121
import (
22+
"context"
2223
"fmt"
2324
"os"
2425
"sync"
2526

27+
containerd "github.com/containerd/containerd/v2/client"
2628
"github.com/containerd/containerd/v2/core/mount"
29+
"github.com/containerd/containerd/v2/core/snapshots"
2730
kernel "github.com/containerd/containerd/v2/pkg/kernelversion"
31+
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
2832
)
2933

3034
var (
@@ -90,3 +94,26 @@ func ensureImageVolumeMounted(target string) (bool, error) {
9094
}
9195
return true, nil
9296
}
97+
98+
// getImageVolumeSnapshotOpts returns snapshot options with user namespace idmap labels
99+
// from the mount's UID/GID mappings. This ensures that image volumes work correctly
100+
// with user namespaces by applying idmap to the overlay lower layers.
101+
func (c *criService) getImageVolumeSnapshotOpts(ctx context.Context, mount *runtime.Mount) ([]snapshots.Opt, error) {
102+
uids, err := parseUsernsIDMap(mount.GetUidMappings())
103+
if err != nil {
104+
return nil, fmt.Errorf("failed to parse UID mappings: %w", err)
105+
}
106+
107+
gids, err := parseUsernsIDMap(mount.GetGidMappings())
108+
if err != nil {
109+
return nil, fmt.Errorf("failed to parse GID mappings: %w", err)
110+
}
111+
112+
if len(uids) == 0 || len(gids) == 0 {
113+
return nil, nil
114+
}
115+
116+
return []snapshots.Opt{
117+
containerd.WithRemapperLabels(0, uids[0].HostID, 0, gids[0].HostID, uids[0].Size),
118+
}, nil
119+
}

‎internal/cri/server/container_image_mount_linux_test.go‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
//go:build linux
2+
3+
/*
4+
Copyright The containerd Authors.
5+
6+
Licensed under the Apache License, Version 2.0 (the "License");
7+
you may not use this file except in compliance with the License.
8+
You may obtain a copy of the License at
9+
10+
http://www.apache.org/licenses/LICENSE-2.0
11+
12+
Unless required by applicable law or agreed to in writing, software
13+
distributed under the License is distributed on an "AS IS" BASIS,
14+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
See the License for the specific language governing permissions and
16+
limitations under the License.
17+
*/
18+
19+
package server
20+
21+
import (
22+
"context"
23+
"testing"
24+
25+
containerd "github.com/containerd/containerd/v2/client"
26+
"github.com/containerd/containerd/v2/core/snapshots"
27+
"github.com/stretchr/testify/assert"
28+
"github.com/stretchr/testify/require"
29+
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
30+
)
31+
32+
func TestGetImageVolumeSnapshotOpts(t *testing.T) {
33+
ctx := context.Background()
34+
35+
mappings := []*runtime.IDMapping{
36+
{
37+
ContainerId: 0,
38+
HostId: 65536,
39+
Length: 65536,
40+
},
41+
}
42+
43+
expectedOpts := optsToInfo(t, containerd.WithRemapperLabels(0, 65536, 0, 65536, 65536))
44+
45+
for _, test := range []struct {
46+
name string
47+
mount *runtime.Mount
48+
expectInfo *snapshots.Info
49+
expectError bool
50+
}{
51+
{
52+
name: "with user namespace mappings",
53+
mount: &runtime.Mount{
54+
ContainerPath: "/test",
55+
UidMappings: mappings,
56+
GidMappings: mappings,
57+
},
58+
expectInfo: expectedOpts,
59+
},
60+
{
61+
name: "without mappings",
62+
mount: &runtime.Mount{
63+
ContainerPath: "/test",
64+
},
65+
},
66+
{
67+
name: "with empty mappings",
68+
mount: &runtime.Mount{
69+
ContainerPath: "/test",
70+
UidMappings: []*runtime.IDMapping{},
71+
GidMappings: []*runtime.IDMapping{},
72+
},
73+
},
74+
{
75+
name: "with only UID mappings",
76+
mount: &runtime.Mount{
77+
ContainerPath: "/test",
78+
UidMappings: mappings,
79+
},
80+
},
81+
{
82+
name: "with only GID mappings",
83+
mount: &runtime.Mount{
84+
ContainerPath: "/test",
85+
GidMappings: mappings,
86+
},
87+
},
88+
{
89+
name: "with multiple UID mapping lines",
90+
mount: &runtime.Mount{
91+
ContainerPath: "/test",
92+
UidMappings: []*runtime.IDMapping{
93+
{
94+
ContainerId: 0,
95+
HostId: 65536,
96+
Length: 65536,
97+
},
98+
{
99+
ContainerId: 65536,
100+
HostId: 131072,
101+
Length: 65536,
102+
},
103+
},
104+
GidMappings: mappings,
105+
},
106+
expectError: true,
107+
},
108+
} {
109+
t.Run(test.name, func(t *testing.T) {
110+
c := &criService{}
111+
112+
opts, err := c.getImageVolumeSnapshotOpts(ctx, test.mount)
113+
114+
if test.expectError {
115+
assert.Error(t, err)
116+
return
117+
}
118+
require.NoError(t, err)
119+
120+
gotInfo := optsToInfo(t, opts...)
121+
if test.expectInfo == nil {
122+
assert.Nil(t, gotInfo)
123+
} else {
124+
require.NotNil(t, gotInfo)
125+
assert.Equal(t, test.expectInfo.Labels, gotInfo.Labels)
126+
}
127+
128+
if test.mount.UidMappings != nil {
129+
assert.NotNil(t, test.mount.UidMappings, "UidMappings should NOT be cleared by getImageVolumeSnapshotOpts")
130+
}
131+
if test.mount.GidMappings != nil {
132+
assert.NotNil(t, test.mount.GidMappings, "GidMappings should NOT be cleared by getImageVolumeSnapshotOpts")
133+
}
134+
})
135+
}
136+
}
137+
138+
func optsToInfo(t *testing.T, opts ...snapshots.Opt) *snapshots.Info {
139+
t.Helper()
140+
if len(opts) == 0 {
141+
return nil
142+
}
143+
var info snapshots.Info
144+
for _, opt := range opts {
145+
require.NoError(t, opt(&info))
146+
}
147+
return &info
148+
}

‎internal/cri/server/container_image_mount_other.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,13 @@
1919
package server
2020

2121
import (
22+
"context"
2223
"fmt"
2324
"os"
2425

2526
"github.com/containerd/containerd/v2/core/mount"
27+
"github.com/containerd/containerd/v2/core/snapshots"
28+
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
2629
)
2730

2831
// addVolatileOptionOnImageVolumeMount is no-op on non-linux platforms.
@@ -42,3 +45,7 @@ func ensureImageVolumeMounted(target string) (bool, error) {
4245
}
4346
return true, nil
4447
}
48+
49+
func (c *criService) getImageVolumeSnapshotOpts(ctx context.Context, mount *runtime.Mount) ([]snapshots.Opt, error) {
50+
return nil, nil
51+
}

0 commit comments

Comments
 (0)