From 603d7db0a75602488a93e8aaa2928c2c7f7025ad Mon Sep 17 00:00:00 2001 From: Mayur Das Date: Mon, 22 Jun 2026 22:42:54 +0530 Subject: [PATCH] feat(mount): support --mount type=image with image-subpath Mount an image's filesystem into a container read-only, matching Docker: --mount type=image,source=,destination=. The source image is ensured and unpacked, a read-only snapshot view of its rootfs is created and mounted at the destination, and the view is removed when the container is deleted. The image-subpath option exposes a single directory of the image rootfs at the destination instead of the whole rootfs. An OCI overlay mount cannot select a subdirectory, so a subpath mount materializes the read-only view on a host directory under the data root, resolves the subpath, and bind-mounts the resolved directory read-only into the container. The host materialization path is recorded on a container label and unmounted and removed on container deletion, alongside the snapshot view. The subpath is normalized and bounded to the rootfs at parse time (rejecting absolute paths and traversal that escapes), then opened in the materialized rootfs with os.OpenInRoot, which requires it to exist and rejects absolute or escaping symlinks rather than following them against the host. Any symlink left in the path is relative and stays inside the rootfs when mount(2) follows it, so the unresolved path is used as the bind source. Docker rejects those same subpaths. The whole-rootfs path hands the snapshotter mount straight to the runtime, which owns its lifecycle. Labels in the reserved nerdctl/ namespace are stripped from an image's config labels. --label already rejects that prefix, but image config labels bypassed it, which would let an image forge internal container state - including the image-mount host paths that nerdctl rm unmounts and deletes. Mounting the same image at multiple destinations is supported; the corresponding tests are skipped on Docker, which rejects mounting the same image more than once. Signed-off-by: Mayur Das --- .../container_run_mount_image_linux_test.go | 175 +++++++++++++++++- docs/command-reference.md | 2 +- pkg/cmd/container/create.go | 48 ++++- pkg/cmd/container/remove.go | 17 +- pkg/cmd/container/run_mount.go | 135 +++++++++++--- pkg/labels/labels.go | 5 + pkg/mountutil/mountutil.go | 7 + pkg/mountutil/mountutil_linux.go | 53 +++++- pkg/mountutil/mountutil_linux_test.go | 93 +++++++++- 9 files changed, 486 insertions(+), 49 deletions(-) diff --git a/cmd/nerdctl/container/container_run_mount_image_linux_test.go b/cmd/nerdctl/container/container_run_mount_image_linux_test.go index 4e7435f6130..ebf16309a4a 100644 --- a/cmd/nerdctl/container/container_run_mount_image_linux_test.go +++ b/cmd/nerdctl/container/container_run_mount_image_linux_test.go @@ -95,10 +95,131 @@ func TestRunMountTypeImageReadOnly(t *testing.T) { testCase.Run(t) } +// TestRunMountTypeImageSubpath verifies that image-subpath exposes only the +// selected directory of the image rootfs at the destination: the image's +// /etc/os-release is reachable as /os-release. +func TestRunMountTypeImageSubpath(t *testing.T) { + testCase := nerdtest.Setup() + + testCase.Command = func(data test.Data, helpers test.Helpers) test.TestableCommand { + return helpers.Command("run", "--rm", + "--mount", fmt.Sprintf("type=image,source=%s,destination=/mnt/img,image-subpath=etc", testutil.CommonImage), + testutil.CommonImage, "cat", "/mnt/img/os-release") + } + + testCase.Expected = func(data test.Data, helpers test.Helpers) *test.Expected { + return &test.Expected{ + ExitCode: expect.ExitCodeSuccess, + Output: expect.Contains("Alpine"), + } + } + + testCase.Run(t) +} + +// TestRunMountTypeImageSubpathMultiple verifies that two image-subpath mounts of +// the same image at different destinations each expose their own subdirectory, +// exercising the multi-mount label round-trip and cleanup. +func TestRunMountTypeImageSubpathMultiple(t *testing.T) { + testCase := nerdtest.Setup() + // nerdctl-only: Docker keys an image mount by its source image and rejects + // mounting the same image twice ("mount already exists with name"). + testCase.Require = require.Not(nerdtest.Docker) + + testCase.Command = func(data test.Data, helpers test.Helpers) test.TestableCommand { + return helpers.Command("run", "--rm", + "--mount", fmt.Sprintf("type=image,source=%s,destination=/mnt/etc,image-subpath=etc", testutil.CommonImage), + "--mount", fmt.Sprintf("type=image,source=%s,destination=/mnt/bin,image-subpath=bin", testutil.CommonImage), + testutil.CommonImage, "ls", "/mnt/etc", "/mnt/bin") + } + + testCase.Expected = func(data test.Data, helpers test.Helpers) *test.Expected { + return &test.Expected{ + ExitCode: expect.ExitCodeSuccess, + } + } + + testCase.Run(t) +} + +// TestRunMountTypeImageSubpathAbsoluteSymlink verifies a subpath through an +// absolute symlink (Alpine's /bin/sh -> /bin/busybox) is rejected, not followed +// against the host. Docker rejects it too but with its own message. +func TestRunMountTypeImageSubpathAbsoluteSymlink(t *testing.T) { + testCase := nerdtest.Setup() + testCase.Require = require.Not(nerdtest.Docker) + + testCase.Command = func(data test.Data, helpers test.Helpers) test.TestableCommand { + return helpers.Command("run", "--rm", + "--mount", fmt.Sprintf("type=image,source=%s,destination=/mnt/img,image-subpath=bin/sh", testutil.CommonImage), + testutil.CommonImage, "true") + } + + testCase.Expected = func(data test.Data, helpers test.Helpers) *test.Expected { + return &test.Expected{ + ExitCode: expect.ExitCodeGenericFail, + Errors: []error{fmt.Errorf("path escapes from parent")}, + } + } + + testCase.Run(t) +} + +// TestRunMountTypeImageSubpathRelativeSymlink verifies a subpath through a +// relative symlink that stays inside the rootfs resolves to the image's own +// target, as it does with Docker. +func TestRunMountTypeImageSubpathRelativeSymlink(t *testing.T) { + testCase := nerdtest.Setup() + testCase.Require = nerdtest.Build + + testCase.Setup = func(data test.Data, helpers test.Helpers) { + dockerfile := fmt.Sprintf(`FROM %s +RUN mkdir -p /data/real /links && echo hello > /data/real/f && ln -s ../data/real /links/rel +`, testutil.CommonImage) + data.Temp().Save(dockerfile, "Dockerfile") + helpers.Ensure("build", "-t", data.Identifier("img"), data.Temp().Path()) + } + + testCase.Command = func(data test.Data, helpers test.Helpers) test.TestableCommand { + return helpers.Command("run", "--rm", + "--mount", fmt.Sprintf("type=image,source=%s,destination=/mnt/img,image-subpath=links/rel", data.Identifier("img")), + testutil.CommonImage, "cat", "/mnt/img/f") + } + + testCase.Expected = test.Expects(expect.ExitCodeSuccess, nil, expect.Equals("hello\n")) + + testCase.Cleanup = func(data test.Data, helpers test.Helpers) { + helpers.Anyhow("rmi", data.Identifier("img")) + } + + testCase.Run(t) +} + +// TestRunMountTypeImageSubpathReadOnly verifies an image-subpath mount is +// read-only so writing fails, matching Docker. +func TestRunMountTypeImageSubpathReadOnly(t *testing.T) { + testCase := nerdtest.Setup() + + testCase.Command = func(data test.Data, helpers test.Helpers) test.TestableCommand { + return helpers.Command("run", "--rm", + "--mount", fmt.Sprintf("type=image,source=%s,destination=/mnt/img,image-subpath=etc", testutil.CommonImage), + testutil.CommonImage, "touch", "/mnt/img/should-fail") + } + + testCase.Expected = func(data test.Data, helpers test.Helpers) *test.Expected { + return &test.Expected{ + ExitCode: expect.ExitCodeGenericFail, + Errors: []error{fmt.Errorf("Read-only file system")}, + } + } + + testCase.Run(t) +} + // TestRunMountTypeImageErrors verifies that an image mount missing its source, -// or using the not-yet-supported image-subpath option, is rejected. Docker -// implements image-subpath, so that case diverges and the test is not run -// against Docker. +// or using the not-yet-supported subpath option, or an image-subpath that +// escapes the rootfs, is rejected. These are nerdctl-specific behaviours here, +// so the test is not run against Docker. func TestRunMountTypeImageErrors(t *testing.T) { testCase := nerdtest.Setup() testCase.Require = require.Not(nerdtest.Docker) @@ -118,16 +239,58 @@ func TestRunMountTypeImageErrors(t *testing.T) { }, }, { - Description: "image-subpath not supported", + Description: "subpath not supported", + Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { + return helpers.Command("run", "--rm", + "--mount", fmt.Sprintf("type=image,source=%s,destination=/mnt/img,subpath=etc", testutil.CommonImage), + testutil.CommonImage, "true") + }, + Expected: func(data test.Data, helpers test.Helpers) *test.Expected { + return &test.Expected{ + ExitCode: expect.ExitCodeGenericFail, + Errors: []error{fmt.Errorf("subpath")}, + } + }, + }, + { + Description: "image-subpath parent traversal rejected", + Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { + return helpers.Command("run", "--rm", + "--mount", fmt.Sprintf("type=image,source=%s,destination=/mnt/img,image-subpath=../etc", testutil.CommonImage), + testutil.CommonImage, "true") + }, + Expected: func(data test.Data, helpers test.Helpers) *test.Expected { + return &test.Expected{ + ExitCode: expect.ExitCodeGenericFail, + Errors: []error{fmt.Errorf("escapes")}, + } + }, + }, + { + Description: "image-subpath absolute rejected", + Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { + return helpers.Command("run", "--rm", + "--mount", fmt.Sprintf("type=image,source=%s,destination=/mnt/img,image-subpath=/etc", testutil.CommonImage), + testutil.CommonImage, "true") + }, + Expected: func(data test.Data, helpers test.Helpers) *test.Expected { + return &test.Expected{ + ExitCode: expect.ExitCodeGenericFail, + Errors: []error{fmt.Errorf("relative")}, + } + }, + }, + { + Description: "empty image-subpath rejected", Command: func(data test.Data, helpers test.Helpers) test.TestableCommand { return helpers.Command("run", "--rm", - "--mount", fmt.Sprintf("type=image,source=%s,destination=/mnt/img,image-subpath=etc", testutil.CommonImage), + "--mount", fmt.Sprintf("type=image,source=%s,destination=/mnt/img,image-subpath=", testutil.CommonImage), testutil.CommonImage, "true") }, Expected: func(data test.Data, helpers test.Helpers) *test.Expected { return &test.Expected{ ExitCode: expect.ExitCodeGenericFail, - Errors: []error{fmt.Errorf("image-subpath")}, + Errors: []error{fmt.Errorf("value is empty")}, } }, }, diff --git a/docs/command-reference.md b/docs/command-reference.md index c0455e60508..75cacd30d10 100644 --- a/docs/command-reference.md +++ b/docs/command-reference.md @@ -332,7 +332,7 @@ Volume flags: - Options specific to `image`: - :whale: `src`, `source`: image reference (mandatory). - :whale: Currently, the image filesystem is mounted read-only. - - unimplemented options: `image-subpath` + - :whale: `image-subpath`: relative path inside the image rootfs to mount instead of the whole rootfs. The value is normalized (`a/../b` means `b`) and must resolve inside the rootfs: an empty value, an absolute path, a path escaping the rootfs, and a path through an absolute symlink (such as Alpine's `/bin/sh`) are rejected. A value that normalizes to the rootfs itself, such as `.`, mounts the whole rootfs. - :whale: `--volumes-from`: Mount volumes from the specified container(s), e.g. "--volumes-from my-container". Rootfs flags: diff --git a/pkg/cmd/container/create.go b/pkg/cmd/container/create.go index e2f89159d95..b2cfd880331 100644 --- a/pkg/cmd/container/create.go +++ b/pkg/cmd/container/create.go @@ -95,20 +95,24 @@ func Create(ctx context.Context, client *containerd.Client, args []string, netMa internalLabels.platform = options.Platform internalLabels.namespace = options.GOptions.Namespace - // If creation fails after image-mount views are created, remove them so the - // snapshots do not leak (the cleanup label is only persisted on success). + // If creation fails after image-mount state is created, tear it down so the + // snapshots and host mounts do not leak (the cleanup labels are only persisted + // on success). defer func() { if retErr == nil { return } - var keys []string + var keys, hostpaths []string for _, mp := range internalLabels.mountPoints { if mp.ImageMountSnapshot != "" { keys = append(keys, mp.ImageMountSnapshot) } + if mp.ImageMountHostpath != "" { + hostpaths = append(hostpaths, mp.ImageMountHostpath) + } } - if len(keys) > 0 { - removeImageMountViews(ctx, client.SnapshotService(options.GOptions.Snapshotter), keys) + if len(keys) > 0 || len(hostpaths) > 0 { + removeImageMounts(ctx, client.SnapshotService(options.GOptions.Snapshotter), hostpaths, keys) } }() @@ -234,7 +238,7 @@ func Create(ctx context.Context, client *containerd.Client, args []string, netMa // containerd.WithImageConfigLabels resets the container labels, so running it // later would clear labels set by other opts (e.g. the restart policy). if ensuredImage != nil { - cOpts = append(cOpts, containerd.WithImageConfigLabels(ensuredImage.Image)) + cOpts = append(cOpts, containerd.WithImageConfigLabels(ensuredImage.Image), withoutReservedLabels()) } opts = append(opts, rootfsOpts...) cOpts = append(cOpts, rootfsCOpts...) @@ -740,6 +744,22 @@ func withNerdctlOCIHook(cmd string, args []string) (oci.SpecOpts, error) { }, nil } +// withoutReservedLabels drops labels in the internal "nerdctl/" namespace. It runs +// right after WithImageConfigLabels, the one path that can set them without going +// through --label (which rejects the prefix): otherwise an image could forge +// internal state, e.g. the image-mount host paths that `nerdctl rm` deletes. +func withoutReservedLabels() containerd.NewContainerOpts { + return func(_ context.Context, _ *containerd.Client, c *containers.Container) error { + for k := range c.Labels { + if strings.HasPrefix(k, labels.Prefix) { + log.L.Warnf("Ignoring reserved label %q set by the image config", k) + delete(c.Labels, k) + } + } + return nil + } +} + func withContainerLabels(label, labelFile []string) ([]containerd.NewContainerOpts, error) { var opts []containerd.NewContainerOpts @@ -894,13 +914,16 @@ func withInternalLabels(internalLabels internalLabels) (containerd.NewContainerO m[labels.AnonymousVolumes] = string(anonVolumeJSON) } - // Record the snapshot keys of any type=image mount views so they can be - // removed when the container is deleted. - var imageMountSnapshots []string + // Record the snapshot keys and host materialization paths of any type=image + // mounts so they can be removed when the container is deleted. + var imageMountSnapshots, imageMountHostpaths []string for _, mp := range internalLabels.mountPoints { if mp.ImageMountSnapshot != "" { imageMountSnapshots = append(imageMountSnapshots, mp.ImageMountSnapshot) } + if mp.ImageMountHostpath != "" { + imageMountHostpaths = append(imageMountHostpaths, mp.ImageMountHostpath) + } } if len(imageMountSnapshots) > 0 { b, err := json.Marshal(imageMountSnapshots) @@ -909,6 +932,13 @@ func withInternalLabels(internalLabels internalLabels) (containerd.NewContainerO } m[labels.ImageMountSnapshots] = string(b) } + if len(imageMountHostpaths) > 0 { + b, err := json.Marshal(imageMountHostpaths) + if err != nil { + return nil, err + } + m[labels.ImageMountHostpaths] = string(b) + } if internalLabels.pidFile != "" { m[labels.PIDFile] = internalLabels.pidFile diff --git a/pkg/cmd/container/remove.go b/pkg/cmd/container/remove.go index ceffe0374b7..d00332bcc02 100644 --- a/pkg/cmd/container/remove.go +++ b/pkg/cmd/container/remove.go @@ -286,15 +286,22 @@ func RemoveContainer(ctx context.Context, c containerd.Container, globalOptions } } - // Remove the read-only views backing type=image mounts - soft failure. + // Tear down type=image mount state (host materializations and read-only + // views) backing this container - soft failure. + var imageMountKeys, imageMountHostpaths []string if snapshotsJSON, ok := containerLabels[labels.ImageMountSnapshots]; ok { - var keys []string - if err = json.Unmarshal([]byte(snapshotsJSON), &keys); err != nil { + if err = json.Unmarshal([]byte(snapshotsJSON), &imageMountKeys); err != nil { log.G(ctx).WithError(err).Warnf("failed to unmarshal image-mount snapshots for container %q", id) - } else { - removeImageMountViews(ctx, client.SnapshotService(imageMountSnapshotter), keys) } } + if hostpathsJSON, ok := containerLabels[labels.ImageMountHostpaths]; ok { + if err = json.Unmarshal([]byte(hostpathsJSON), &imageMountHostpaths); err != nil { + log.G(ctx).WithError(err).Warnf("failed to unmarshal image-mount host paths for container %q", id) + } + } + if len(imageMountKeys) > 0 || len(imageMountHostpaths) > 0 { + removeImageMounts(ctx, client.SnapshotService(imageMountSnapshotter), imageMountHostpaths, imageMountKeys) + } }() // Get the task. diff --git a/pkg/cmd/container/run_mount.go b/pkg/cmd/container/run_mount.go index d6bdb97e603..a416755a92f 100644 --- a/pkg/cmd/container/run_mount.go +++ b/pkg/cmd/container/run_mount.go @@ -127,19 +127,20 @@ func parseMountFlags(volStore volumestore.VolumeStore, options types.ContainerCr const gcRootLabel = "containerd.io/gc.root" // setupImageMount ensures and unpacks ref, then creates a read-only GC-rooted -// snapshot view of its rootfs. Image mounts are always read-only, matching -// Docker. It returns the OCI mount for destination and the view's snapshot key. -func setupImageMount(ctx context.Context, client *containerd.Client, options types.ContainerCreateOptions, ref, destination string) (specs.Mount, string, error) { +// snapshot view of its rootfs. It returns the OCI mount for destination, the +// view's snapshot key, and the host path a subpath was materialized on (empty +// for a whole-rootfs mount). The view is removed if setup fails after creating it. +func setupImageMount(ctx context.Context, client *containerd.Client, options types.ContainerCreateOptions, ref, destination, subpath string) (_ specs.Mount, _ string, _ string, retErr error) { ensured, err := imgutil.EnsureImage(ctx, client, ref, options.ImagePullOpt) if err != nil { - return specs.Mount{}, "", fmt.Errorf("failed to ensure image %q for image mount: %w", ref, err) + return specs.Mount{}, "", "", fmt.Errorf("failed to ensure image %q for image mount: %w", ref, err) } if err := ensured.Image.Unpack(ctx, options.GOptions.Snapshotter); err != nil { - return specs.Mount{}, "", fmt.Errorf("failed to unpack image %q for image mount: %w", ref, err) + return specs.Mount{}, "", "", fmt.Errorf("failed to unpack image %q for image mount: %w", ref, err) } diffIDs, err := ensured.Image.RootFS(ctx) if err != nil { - return specs.Mount{}, "", fmt.Errorf("failed to get rootfs of image %q for image mount: %w", ref, err) + return specs.Mount{}, "", "", fmt.Errorf("failed to get rootfs of image %q for image mount: %w", ref, err) } chainID := identity.ChainID(diffIDs).String() @@ -149,16 +150,31 @@ func setupImageMount(ctx context.Context, client *containerd.Client, options typ gcRootLabel: time.Now().UTC().Format(time.RFC3339), })) if err != nil { - return specs.Mount{}, "", fmt.Errorf("failed to create read-only view of image %q: %w", ref, err) + return specs.Mount{}, "", "", fmt.Errorf("failed to create read-only view of image %q: %w", ref, err) } - // overlayfs and native snapshotters each yield a single mount for a view. - if len(mounts) != 1 { - if rmErr := s.Remove(ctx, snapshotKey); rmErr != nil && !errdefs.IsNotFound(rmErr) { - log.G(ctx).WithError(rmErr).Warnf("failed to remove image-mount snapshot %q", snapshotKey) + defer func() { + if retErr == nil { + return + } + if err := s.Remove(ctx, snapshotKey); err != nil && !errdefs.IsNotFound(err) { + log.G(ctx).WithError(err).Warnf("failed to remove image-mount snapshot %q", snapshotKey) + } + }() + + if subpath != "" { + m, hostMountpoint, err := setupImageSubpathMount(ctx, options, ref, destination, subpath, snapshotKey, mounts) + if err != nil { + return specs.Mount{}, "", "", err } - return specs.Mount{}, "", fmt.Errorf("image mount expects exactly one mount from the snapshotter, got %d", len(mounts)) + return m, snapshotKey, hostMountpoint, nil } + // Whole rootfs: hand the snapshotter's mount straight to the OCI runtime, + // which mounts and unmounts it with the container. overlayfs and native + // snapshotters each yield exactly one mount for a view. + if len(mounts) != 1 { + return specs.Mount{}, "", "", fmt.Errorf("image mount expects exactly one mount from the snapshotter, got %d", len(mounts)) + } m := mounts[0] opts := m.Options // A view without an upper dir is already read-only; make it explicit for @@ -171,13 +187,79 @@ func setupImageMount(ctx context.Context, client *containerd.Client, options typ Source: m.Source, Destination: destination, Options: opts, - }, snapshotKey, nil + }, snapshotKey, "", nil +} + +// setupImageSubpathMount materializes the view on a host dir under the data root +// and returns a read-only bind mount of subpath plus that dir. The dir is +// unmounted and removed if setup fails; otherwise it lives until container +// deletion so the mount survives restarts. +func setupImageSubpathMount(ctx context.Context, options types.ContainerCreateOptions, ref, destination, subpath, snapshotKey string, mounts []mount.Mount) (_ specs.Mount, _ string, retErr error) { + // Keyed by snapshot key so the dir is unique per view. + hostMountpoint := filepath.Join(options.GOptions.DataRoot, "image-mounts", snapshotKey) + // mount.All can apply some mounts before failing, so unmount before RemoveAll + // recurses. Both are no-ops on a missing or never-mounted dir. + defer func() { + if retErr == nil { + return + } + if err := mount.UnmountAll(hostMountpoint, 0); err != nil { + log.G(ctx).WithError(err).Warnf("failed to unmount image-mount host path %q", hostMountpoint) + } + if err := os.RemoveAll(hostMountpoint); err != nil { + log.G(ctx).WithError(err).Warnf("failed to remove image-mount host path %q", hostMountpoint) + } + }() + + if err := os.MkdirAll(hostMountpoint, 0o700); err != nil { + return specs.Mount{}, "", fmt.Errorf("failed to create image-mount host dir: %w", err) + } + if err := mount.All(mounts, hostMountpoint); err != nil { + return specs.Mount{}, "", fmt.Errorf("failed to materialize image %q for subpath mount: %w", ref, err) + } + source, err := resolveImageSubpath(hostMountpoint, subpath) + if err != nil { + return specs.Mount{}, "", fmt.Errorf("image-subpath %q in image %q: %w", subpath, ref, err) + } + return specs.Mount{ + Type: "bind", + Source: source, + Destination: destination, + Options: []string{"rbind", "ro"}, + }, hostMountpoint, nil +} + +// resolveImageSubpath returns the bind source for subpath under the materialized +// rootfs. The scoped lookup requires it to exist and rejects absolute or escaping +// symlinks, so any symlink left in the path is relative and stays inside the +// rootfs when mount(2) follows it. +func resolveImageSubpath(rootfs, subpath string) (string, error) { + f, err := os.OpenInRoot(rootfs, subpath) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return "", errors.New("does not exist in the image") + } + return "", err + } + defer f.Close() + return f.Name(), nil } -// removeImageMountViews removes the snapshotter views created for type=image -// mounts. NotFound is ignored; other failures are logged but not fatal. -func removeImageMountViews(ctx context.Context, s snapshots.Snapshotter, keys []string) { - for _, k := range keys { +// removeImageMounts tears down type=image mount state for a container: it +// unmounts and removes any host materialization directories (image-subpath), +// then removes the read-only snapshot views. NotFound is ignored; other +// failures are logged but not fatal. +func removeImageMounts(ctx context.Context, s snapshots.Snapshotter, hostpaths, snapshotKeys []string) { + // Unmount host materializations before removing the views they hold open. + for _, p := range hostpaths { + if err := mount.UnmountAll(p, 0); err != nil { + log.G(ctx).WithError(err).Warnf("failed to unmount image-mount host path %q", p) + } + if err := os.RemoveAll(p); err != nil { + log.G(ctx).WithError(err).Warnf("failed to remove image-mount host path %q", p) + } + } + for _, k := range snapshotKeys { if err := s.Remove(ctx, k); err != nil && !errdefs.IsNotFound(err) { log.G(ctx).WithError(err).Warnf("failed to remove image-mount snapshot %q", k) } @@ -190,14 +272,15 @@ func generateMountOpts(ctx context.Context, client *containerd.Client, ensuredIm volStore volumestore.VolumeStore, options types.ContainerCreateOptions) (opts []oci.SpecOpts, anonVolumes []string, mountPoints []*mountutil.Processed, retErr error) { //nolint:prealloc var ( - userMounts []specs.Mount - imageMountViews []string + userMounts []specs.Mount + imageMountViews []string + imageMountHostpaths []string ) - // Remove any image-mount views created here if this function fails, so a - // partial setup does not leak snapshots. + // Tear down any image-mount state created here if this function fails, so a + // partial setup does not leak snapshots or host mounts. defer func() { - if retErr != nil && len(imageMountViews) > 0 { - removeImageMountViews(ctx, client.SnapshotService(options.GOptions.Snapshotter), imageMountViews) + if retErr != nil && (len(imageMountViews) > 0 || len(imageMountHostpaths) > 0) { + removeImageMounts(ctx, client.SnapshotService(options.GOptions.Snapshotter), imageMountHostpaths, imageMountViews) } }() mounted := make(map[string]struct{}) @@ -299,13 +382,17 @@ func generateMountOpts(ctx context.Context, client *containerd.Client, ensuredIm // type=image: build the read-only view now and record its snapshot // key for cleanup on container removal. if x.Type == mountutil.Image { - m, snapshotKey, err := setupImageMount(ctx, client, options, x.Mount.Source, x.Mount.Destination) + m, snapshotKey, hostMountpoint, err := setupImageMount(ctx, client, options, x.Mount.Source, x.Mount.Destination, x.ImageSubpath) if err != nil { return nil, nil, nil, err } imageMountViews = append(imageMountViews, snapshotKey) + if hostMountpoint != "" { + imageMountHostpaths = append(imageMountHostpaths, hostMountpoint) + } ociMounts[i] = m x.ImageMountSnapshot = snapshotKey + x.ImageMountHostpath = hostMountpoint mounted[filepath.Clean(x.Mount.Destination)] = struct{}{} continue } diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go index 9f689ff7cff..c665e3dece5 100644 --- a/pkg/labels/labels.go +++ b/pkg/labels/labels.go @@ -84,6 +84,11 @@ const ( // the read-only views backing `--mount type=image`, removed on container deletion. ImageMountSnapshots = Prefix + "image-mount-snapshots" + // ImageMountHostpaths is a JSON-marshalled []string of host directories where + // `--mount type=image,image-subpath=...` rootfs views are materialized; each + // must be unmounted and removed on container deletion. + ImageMountHostpaths = Prefix + "image-mount-hostpaths" + // Platform is the normalized platform string like "linux/ppc64le". Platform = Prefix + "platform" diff --git a/pkg/mountutil/mountutil.go b/pkg/mountutil/mountutil.go index 350d4443376..986edd4dea8 100644 --- a/pkg/mountutil/mountutil.go +++ b/pkg/mountutil/mountutil.go @@ -55,6 +55,13 @@ type Processed struct { // ImageMountSnapshot is the snapshotter key of the read-only view for a // type=image mount; empty for other mount types. ImageMountSnapshot string + // ImageSubpath is the relative path inside a type=image rootfs to expose at + // the destination, instead of the whole rootfs. Empty means the whole rootfs. + ImageSubpath string + // ImageMountHostpath is the host directory where a type=image rootfs is + // materialized so an image-subpath can be bind-mounted from it. It must be + // unmounted and removed on container deletion. Empty when no subpath is used. + ImageMountHostpath string } type volumeSpec struct { diff --git a/pkg/mountutil/mountutil_linux.go b/pkg/mountutil/mountutil_linux.go index 46ed18cb892..78aa67a75ea 100644 --- a/pkg/mountutil/mountutil_linux.go +++ b/pkg/mountutil/mountutil_linux.go @@ -21,6 +21,7 @@ import ( "fmt" "io/fs" "os" + "path" "path/filepath" "strconv" "strings" @@ -360,6 +361,29 @@ func ProcessFlagTmpfs(s string) (*Processed, error) { return res, nil } +// validateImageSubpath normalizes an image-subpath, rejecting absolute paths and +// ones escaping the rootfs. A value resolving to the rootfs itself returns empty, +// the whole-rootfs case Docker accepts. Image paths are always forward-slash. +func validateImageSubpath(p string) (string, error) { + if p == "" { + return "", nil + } + if path.IsAbs(p) { + return "", fmt.Errorf("image-subpath must be relative to the image rootfs, got %q", p) + } + clean := path.Clean(p) + // Clean collapses ".." segments; anything still leading with ".." escapes root. + if clean == ".." || strings.HasPrefix(clean, "../") { + return "", fmt.Errorf("image-subpath %q escapes the image rootfs", p) + } + // "." is the whole rootfs (e.g. from "a/.."); treat it as no subpath so the + // caller mounts the full image view, matching Docker. + if clean == "." { + return "", nil + } + return clean, nil +} + func ProcessFlagMount(s string, volStore volumestore.VolumeStore, ociRuntime string) (*Processed, error) { fields := strings.Split(s, ",") var ( @@ -371,6 +395,8 @@ func ProcessFlagMount(s string, volStore volumestore.VolumeStore, ociRuntime str bindRecursive string // "enabled", "disabled", "writable", or "readonly" volumeNoCopy bool rwOption string + imageSubpath string + imageSubpathSet bool tmpfsSize int64 tmpfsMode os.FileMode err error @@ -445,9 +471,11 @@ func ProcessFlagMount(s string, volStore volumestore.VolumeStore, ociRuntime str rwOption = key } case "image-subpath": - // image-subpath is Docker's option to mount a subdirectory of the - // image; it is not implemented yet. - return nil, fmt.Errorf("mount option %q is not yet supported", key) + // Selects a directory inside a type=image rootfs; validated below once + // the mount type is known. Presence is tracked separately from the + // value so that an explicit empty value is not read as "unset". + imageSubpath = value + imageSubpathSet = true case "bind-propagation": // here don't validate the propagation value // parseVolumeOptions will do that. @@ -493,6 +521,18 @@ func ProcessFlagMount(s string, volStore volumestore.VolumeStore, ociRuntime str return nil, fmt.Errorf("the option 'volume-nocopy' is only supported for volume mounts") } + // Check presence, not value: an explicit empty image-subpath is an error on + // every type, and on other types the option itself is rejected before falling + // through to the legacy bind/volume/tmpfs handlers. Both match Docker. + if imageSubpathSet { + if imageSubpath == "" { + return nil, fmt.Errorf("invalid value for image-subpath: value is empty") + } + if mountType != Image { + return nil, fmt.Errorf("image-subpath is only supported for type=image") + } + } + // type=image's source is an image reference resolved later with a containerd // client; validate the intent here. Like Docker, an image mount is always // read-only: a readonly/ro option is accepted for compatibility but the @@ -504,6 +544,12 @@ func ProcessFlagMount(s string, volStore volumestore.VolumeStore, ociRuntime str if dst == "" { return nil, fmt.Errorf("type=image requires a destination") } + // Bound the subpath at parse time; symlinks are checked once the rootfs + // is materialized. + cleanSubpath, err := validateImageSubpath(imageSubpath) + if err != nil { + return nil, err + } return &Processed{ Type: Image, // Mode "ro" so inspect/label metadata reports the mount read-only. @@ -513,6 +559,7 @@ func ProcessFlagMount(s string, volStore volumestore.VolumeStore, ociRuntime str Source: src, Destination: cleanMount(dst), }, + ImageSubpath: cleanSubpath, }, nil } diff --git a/pkg/mountutil/mountutil_linux_test.go b/pkg/mountutil/mountutil_linux_test.go index 016b6b619b4..a93287ff9d9 100644 --- a/pkg/mountutil/mountutil_linux_test.go +++ b/pkg/mountutil/mountutil_linux_test.go @@ -645,8 +645,98 @@ func TestProcessFlagMountImage(t *testing.T) { }, }, { + // bare subpath is not a type=image option; image-subpath is. + rawSpec: "type=image,source=alpine:latest,destination=/mnt/img,subpath=etc", + err: "subpath", + }, + { + // image-subpath selects a directory inside the image rootfs. rawSpec: "type=image,source=alpine:latest,destination=/mnt/img,image-subpath=etc", - err: "image-subpath", + wants: &Processed{ + Type: Image, + Mode: "ro", + Mount: specs.Mount{Type: Image, Source: "alpine:latest", Destination: "/mnt/img"}, + ImageSubpath: "etc", + }, + }, + { + // image-subpath is normalized: leading ./ and trailing / are stripped. + rawSpec: "type=image,source=alpine:latest,destination=/mnt/img,image-subpath=./etc/", + wants: &Processed{ + Type: Image, + Mode: "ro", + Mount: specs.Mount{Type: Image, Source: "alpine:latest", Destination: "/mnt/img"}, + ImageSubpath: "etc", + }, + }, + { + // parent traversal must be rejected before the mount is built. + rawSpec: "type=image,source=alpine:latest,destination=/mnt/img,image-subpath=../etc", + err: "escapes", + }, + { + // traversal that normalizes back above root must be rejected. + rawSpec: "type=image,source=alpine:latest,destination=/mnt/img,image-subpath=a/b/../../../etc", + err: "escapes", + }, + { + // a path normalizing to "." is the whole rootfs; like Docker, this is + // the no-subpath case rather than an error. + rawSpec: "type=image,source=alpine:latest,destination=/mnt/img,image-subpath=.", + wants: &Processed{ + Type: Image, + Mode: "ro", + Mount: specs.Mount{Type: Image, Source: "alpine:latest", Destination: "/mnt/img"}, + }, + }, + { + // "a/.." also normalizes to the rootfs, so it is the whole-rootfs mount. + rawSpec: "type=image,source=alpine:latest,destination=/mnt/img,image-subpath=a/..", + wants: &Processed{ + Type: Image, + Mode: "ro", + Mount: specs.Mount{Type: Image, Source: "alpine:latest", Destination: "/mnt/img"}, + }, + }, + { + // nested subpath is normalized and preserved. + rawSpec: "type=image,source=alpine:latest,destination=/mnt/img,image-subpath=usr/lib", + wants: &Processed{ + Type: Image, + Mode: "ro", + Mount: specs.Mount{Type: Image, Source: "alpine:latest", Destination: "/mnt/img"}, + ImageSubpath: "usr/lib", + }, + }, + { + // absolute image-subpath is rejected; it must be relative to the rootfs. + rawSpec: "type=image,source=alpine:latest,destination=/mnt/img,image-subpath=/etc", + err: "relative", + }, + { + // image-subpath only applies to type=image. + rawSpec: "type=bind,source=/tmp,destination=/mnt,image-subpath=etc", + err: "only supported for type=image", + }, + { + // an explicitly empty image-subpath is an error, not "unset": Docker + // rejects it on every mount type. + rawSpec: "type=image,source=alpine:latest,destination=/mnt/img,image-subpath=", + err: "value is empty", + }, + { + rawSpec: "type=bind,source=/tmp,destination=/mnt,image-subpath=", + err: "value is empty", + }, + { + // a subpath whose ".." segments cancel out is normalized, like Docker. + rawSpec: "type=image,source=alpine:latest,destination=/mnt/img,image-subpath=a/../etc", + wants: &Processed{ + Type: Image, + Mode: "ro", + Mount: specs.Mount{Type: Image, Source: "alpine:latest", Destination: "/mnt/img"}, + ImageSubpath: "etc", + }, }, } for _, tt := range tests { @@ -662,6 +752,7 @@ func TestProcessFlagMountImage(t *testing.T) { assert.Equal(t, got.Mount.Type, tt.wants.Mount.Type) assert.Equal(t, got.Mount.Source, tt.wants.Mount.Source) assert.Equal(t, got.Mount.Destination, tt.wants.Mount.Destination) + assert.Equal(t, got.ImageSubpath, tt.wants.ImageSubpath) }) } }