From db9546b6df3671efe1a5727f43ba925531362354 Mon Sep 17 00:00:00 2001 From: qiuxue Date: Sat, 24 Jan 2026 13:12:04 +0800 Subject: [PATCH] cri: Fix image volumes with user namespaces Signed-off-by: qiuxue --- integration/image_volume_linux_test.go | 67 ++++++++ internal/cri/server/container_image_mount.go | 17 +- .../cri/server/container_image_mount_linux.go | 27 ++++ .../container_image_mount_linux_test.go | 148 ++++++++++++++++++ .../cri/server/container_image_mount_other.go | 7 + 5 files changed, 265 insertions(+), 1 deletion(-) create mode 100644 internal/cri/server/container_image_mount_linux_test.go diff --git a/integration/image_volume_linux_test.go b/integration/image_volume_linux_test.go index 440f824556..6602ee61a7 100644 --- a/integration/image_volume_linux_test.go +++ b/integration/image_volume_linux_test.go @@ -28,6 +28,7 @@ import ( "github.com/containerd/containerd/v2/integration/images" kernel "github.com/containerd/containerd/v2/pkg/kernelversion" "github.com/containerd/containerd/v2/pkg/namespaces" + "github.com/containerd/containerd/v2/pkg/sys" "github.com/containerd/errdefs" "github.com/opencontainers/image-spec/identity" "github.com/opencontainers/selinux/go-selinux" @@ -324,3 +325,69 @@ func TestImageVolumeSetupIfContainerdRestarts(t *testing.T) { }) } } + +func TestImageVolumeWithUserNamespace(t *testing.T) { + // Check if user namespace and idmap are supported + if !supportsUserNS() { + t.Skip("user namespace not supported") + } + + // Check if pidfd is supported + if !sys.SupportsPidFD() { + t.Skip("pidfd not supported") + } + + if !supportsIDMap(defaultRoot) { + t.Skipf("idmap mounts not supported on: %s", defaultRoot) + } + + containerID := uint32(0) + hostID := uint32(65536) + size := uint32(65536) + + containerImage := images.Get(images.Alpine) + imageVolumeImage := images.Get(images.Pause) + + podLogDir := t.TempDir() + podOpts := []PodSandboxOpts{ + WithPodLogDirectory(podLogDir), + WithPodUserNs(containerID, hostID, size), + } + podCtx := newPodTCtx(t, runtimeService, t.Name(), "image-volume-userns", podOpts...) + defer podCtx.stop(true) + + pullImagesByCRI(t, imageService, containerImage, imageVolumeImage) + + // Create a container with image volume mount + // Pass the user namespace ID mappings to the image volume mount so that + // idmap is applied and files appear with correct ownership in the container + uidMaps := []*criruntime.IDMapping{{ContainerId: containerID, HostId: hostID, Length: size}} + gidMaps := []*criruntime.IDMapping{{ContainerId: containerID, HostId: hostID, Length: size}} + + containerName := "test-container" + cfg := ContainerConfig(containerName, containerImage, + WithCommand("sleep", "1d"), + WithIDMapImageVolumeMount(imageVolumeImage, "", "/image-mount", uidMaps, gidMaps), + WithLogPath(containerName), + WithUserNamespace(containerID, hostID, size), + ) + cnID, err := podCtx.rSvc.CreateContainer(podCtx.id, cfg, podCtx.cfg) + require.NoError(t, err, "failed to create container with image volume and user namespace") + + require.NoError(t, podCtx.rSvc.StartContainer(cnID), "failed to start container") + + // Verify that the image volume is accessible + stdout, stderr, err := runtimeService.ExecSync(cnID, []string{"ls", "/image-mount/pause"}, 0) + require.NoError(t, err, "failed to access image volume") + require.Len(t, stderr, 0) + require.Contains(t, string(stdout), "pause", "image volume should contain pause binary") + + _, _, err = runtimeService.ExecSync(cnID, []string{"rm", "/image-mount/pause"}, 0) + require.Error(t, err, "image volume should be read-only") + require.Contains(t, err.Error(), "Read-only file system", "error should indicate read-only filesystem") + + 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") +} diff --git a/internal/cri/server/container_image_mount.go b/internal/cri/server/container_image_mount.go index fb2c21a082..5e83d596ec 100644 --- a/internal/cri/server/container_image_mount.go +++ b/internal/cri/server/container_image_mount.go @@ -124,8 +124,14 @@ func (c *criService) mutateImageMount( } chainID := identity.ChainID(diffIDs).String() + // Get snapshot options with user namespace idmap labels if needed + snapshotOpts, err := c.getImageVolumeSnapshotOpts(ctx, extraMount) + if err != nil { + return fmt.Errorf("failed to get snapshot options for image volume: %w", err) + } + s := c.client.SnapshotService(snapshotter) - mounts, err := s.Prepare(ctx, target, chainID) + mounts, err := s.Prepare(ctx, target, chainID, snapshotOpts...) if err != nil { if errdefs.IsAlreadyExists(err) { mounts, err = s.Mounts(ctx, target) @@ -160,6 +166,15 @@ func (c *criService) mutateImageMount( } extraMount.HostPath = target + + // Clear UID/GID mappings from the mount to prevent the OCI runtime from + // attempting idmap on the bind mount. The idmap is already applied to the + // overlay lower layers via the snapshotter when the image volume is prepared. + // This must be done regardless of whether the image volume was already mounted + // (e.g., by another container in the same pod). + extraMount.UidMappings = nil + extraMount.GidMappings = nil + return nil } diff --git a/internal/cri/server/container_image_mount_linux.go b/internal/cri/server/container_image_mount_linux.go index ff5356a125..3a755df627 100644 --- a/internal/cri/server/container_image_mount_linux.go +++ b/internal/cri/server/container_image_mount_linux.go @@ -19,12 +19,16 @@ package server import ( + "context" "fmt" "os" "sync" + containerd "github.com/containerd/containerd/v2/client" "github.com/containerd/containerd/v2/core/mount" + "github.com/containerd/containerd/v2/core/snapshots" kernel "github.com/containerd/containerd/v2/pkg/kernelversion" + runtime "k8s.io/cri-api/pkg/apis/runtime/v1" ) var ( @@ -90,3 +94,26 @@ func ensureImageVolumeMounted(target string) (bool, error) { } return true, nil } + +// getImageVolumeSnapshotOpts returns snapshot options with user namespace idmap labels +// from the mount's UID/GID mappings. This ensures that image volumes work correctly +// with user namespaces by applying idmap to the overlay lower layers. +func (c *criService) getImageVolumeSnapshotOpts(ctx context.Context, mount *runtime.Mount) ([]snapshots.Opt, error) { + uids, err := parseUsernsIDMap(mount.GetUidMappings()) + if err != nil { + return nil, fmt.Errorf("failed to parse UID mappings: %w", err) + } + + gids, err := parseUsernsIDMap(mount.GetGidMappings()) + if err != nil { + return nil, fmt.Errorf("failed to parse GID mappings: %w", err) + } + + if len(uids) == 0 || len(gids) == 0 { + return nil, nil + } + + return []snapshots.Opt{ + containerd.WithRemapperLabels(0, uids[0].HostID, 0, gids[0].HostID, uids[0].Size), + }, nil +} diff --git a/internal/cri/server/container_image_mount_linux_test.go b/internal/cri/server/container_image_mount_linux_test.go new file mode 100644 index 0000000000..8f25ac70ac --- /dev/null +++ b/internal/cri/server/container_image_mount_linux_test.go @@ -0,0 +1,148 @@ +//go:build linux + +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package server + +import ( + "context" + "testing" + + containerd "github.com/containerd/containerd/v2/client" + "github.com/containerd/containerd/v2/core/snapshots" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + runtime "k8s.io/cri-api/pkg/apis/runtime/v1" +) + +func TestGetImageVolumeSnapshotOpts(t *testing.T) { + ctx := context.Background() + + mappings := []*runtime.IDMapping{ + { + ContainerId: 0, + HostId: 65536, + Length: 65536, + }, + } + + expectedOpts := optsToInfo(t, containerd.WithRemapperLabels(0, 65536, 0, 65536, 65536)) + + for _, test := range []struct { + name string + mount *runtime.Mount + expectInfo *snapshots.Info + expectError bool + }{ + { + name: "with user namespace mappings", + mount: &runtime.Mount{ + ContainerPath: "/test", + UidMappings: mappings, + GidMappings: mappings, + }, + expectInfo: expectedOpts, + }, + { + name: "without mappings", + mount: &runtime.Mount{ + ContainerPath: "/test", + }, + }, + { + name: "with empty mappings", + mount: &runtime.Mount{ + ContainerPath: "/test", + UidMappings: []*runtime.IDMapping{}, + GidMappings: []*runtime.IDMapping{}, + }, + }, + { + name: "with only UID mappings", + mount: &runtime.Mount{ + ContainerPath: "/test", + UidMappings: mappings, + }, + }, + { + name: "with only GID mappings", + mount: &runtime.Mount{ + ContainerPath: "/test", + GidMappings: mappings, + }, + }, + { + name: "with multiple UID mapping lines", + mount: &runtime.Mount{ + ContainerPath: "/test", + UidMappings: []*runtime.IDMapping{ + { + ContainerId: 0, + HostId: 65536, + Length: 65536, + }, + { + ContainerId: 65536, + HostId: 131072, + Length: 65536, + }, + }, + GidMappings: mappings, + }, + expectError: true, + }, + } { + t.Run(test.name, func(t *testing.T) { + c := &criService{} + + opts, err := c.getImageVolumeSnapshotOpts(ctx, test.mount) + + if test.expectError { + assert.Error(t, err) + return + } + require.NoError(t, err) + + gotInfo := optsToInfo(t, opts...) + if test.expectInfo == nil { + assert.Nil(t, gotInfo) + } else { + require.NotNil(t, gotInfo) + assert.Equal(t, test.expectInfo.Labels, gotInfo.Labels) + } + + if test.mount.UidMappings != nil { + assert.NotNil(t, test.mount.UidMappings, "UidMappings should NOT be cleared by getImageVolumeSnapshotOpts") + } + if test.mount.GidMappings != nil { + assert.NotNil(t, test.mount.GidMappings, "GidMappings should NOT be cleared by getImageVolumeSnapshotOpts") + } + }) + } +} + +func optsToInfo(t *testing.T, opts ...snapshots.Opt) *snapshots.Info { + t.Helper() + if len(opts) == 0 { + return nil + } + var info snapshots.Info + for _, opt := range opts { + require.NoError(t, opt(&info)) + } + return &info +} diff --git a/internal/cri/server/container_image_mount_other.go b/internal/cri/server/container_image_mount_other.go index 41deb972d1..7388e39415 100644 --- a/internal/cri/server/container_image_mount_other.go +++ b/internal/cri/server/container_image_mount_other.go @@ -19,10 +19,13 @@ package server import ( + "context" "fmt" "os" "github.com/containerd/containerd/v2/core/mount" + "github.com/containerd/containerd/v2/core/snapshots" + runtime "k8s.io/cri-api/pkg/apis/runtime/v1" ) // addVolatileOptionOnImageVolumeMount is no-op on non-linux platforms. @@ -42,3 +45,7 @@ func ensureImageVolumeMounted(target string) (bool, error) { } return true, nil } + +func (c *criService) getImageVolumeSnapshotOpts(ctx context.Context, mount *runtime.Mount) ([]snapshots.Opt, error) { + return nil, nil +}