From 8a192c84034e4ef427bbb95ef6080a9dcd7be432 Mon Sep 17 00:00:00 2001 From: Matt Heon Date: Mon, 11 Nov 2024 14:34:55 -0500 Subject: [PATCH] Add subpath support to volumes in `--mount` option All the backend work was done a while back for image volumes, so this is effectively just plumbing the option in for volumes in the parser logic. We do need to change the return type of the volume parser as it only worked on spec.Mount before (which does not have subpath support, so we'd have to pass it as an option and parse it again) but that is cleaner than the alternative. Fixes #20661 Signed-off-by: Matt Heon --- libpod/container.go | 2 +- pkg/specgenutil/volumes.go | 87 ++++++++++++++++++++++--------------- test/e2e/run_volume_test.go | 20 +++++++++ 3 files changed, 74 insertions(+), 35 deletions(-) diff --git a/libpod/container.go b/libpod/container.go index 4234430726..393bfe30dd 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -257,7 +257,7 @@ type ContainerNamedVolume struct { // This is used for emptyDir volumes from a kube yaml IsAnonymous bool `json:"setAnonymous,omitempty"` // SubPath determines which part of the Source will be mounted in the container - SubPath string + SubPath string `json:",omitempty"` } // ContainerOverlayVolume is an overlay volume that will be mounted into the diff --git a/pkg/specgenutil/volumes.go b/pkg/specgenutil/volumes.go index d94aba887f..91ef8e3309 100644 --- a/pkg/specgenutil/volumes.go +++ b/pkg/specgenutil/volumes.go @@ -22,6 +22,12 @@ var ( errNoDest = errors.New("must set volume destination") ) +type universalMount struct { + mount spec.Mount + // Used only with Named Volume type mounts + subPath string +} + // Parse all volume-related options in the create config into a set of mounts // and named volumes to add to the container. // Handles --volumes, --mount, and --tmpfs flags. @@ -253,10 +259,10 @@ func Mounts(mountFlag []string, configMounts []string) (map[string]spec.Mount, m return finalMounts, finalNamedVolumes, finalImageVolumes, nil } -func parseMountOptions(mountType string, args []string) (*spec.Mount, error) { +func parseMountOptions(mountType string, args []string) (*universalMount, error) { var setTmpcopyup, setRORW, setSuid, setDev, setExec, setRelabel, setOwnership, setSwap bool - mnt := spec.Mount{} + mnt := new(universalMount) for _, arg := range args { name, value, hasValue := strings.Cut(arg, "=") switch name { @@ -264,7 +270,7 @@ func parseMountOptions(mountType string, args []string) (*spec.Mount, error) { if mountType != define.TypeBind { return nil, fmt.Errorf("%q option not supported for %q mount types", name, mountType) } - mnt.Options = append(mnt.Options, define.TypeBind) + mnt.mount.Options = append(mnt.mount.Options, define.TypeBind) case "bind-propagation": if mountType != define.TypeBind { return nil, fmt.Errorf("%q option not supported for %q mount types", name, mountType) @@ -278,16 +284,16 @@ func parseMountOptions(mountType string, args []string) (*spec.Mount, error) { default: return nil, fmt.Errorf("invalid value %q", arg) } - mnt.Options = append(mnt.Options, value) + mnt.mount.Options = append(mnt.mount.Options, value) case "consistency": // Often used on MACs and mistakenly on Linux platforms. // Since Docker ignores this option so shall we. continue case "idmap": if hasValue { - mnt.Options = append(mnt.Options, fmt.Sprintf("idmap=%s", value)) + mnt.mount.Options = append(mnt.mount.Options, fmt.Sprintf("idmap=%s", value)) } else { - mnt.Options = append(mnt.Options, "idmap") + mnt.mount.Options = append(mnt.mount.Options, "idmap") } case "readonly", "ro", "rw": if setRORW { @@ -307,35 +313,35 @@ func parseMountOptions(mountType string, args []string) (*spec.Mount, error) { if hasValue { switch strings.ToLower(value) { case "true": - mnt.Options = append(mnt.Options, name) + mnt.mount.Options = append(mnt.mount.Options, name) case "false": // Set the opposite only for rw // ro's opposite is the default if name == "rw" { - mnt.Options = append(mnt.Options, "ro") + mnt.mount.Options = append(mnt.mount.Options, "ro") } } } else { - mnt.Options = append(mnt.Options, name) + mnt.mount.Options = append(mnt.mount.Options, name) } case "nodev", "dev": if setDev { return nil, fmt.Errorf("cannot pass 'nodev' and 'dev' mnt.Options more than once: %w", errOptionArg) } setDev = true - mnt.Options = append(mnt.Options, name) + mnt.mount.Options = append(mnt.mount.Options, name) case "noexec", "exec": if setExec { return nil, fmt.Errorf("cannot pass 'noexec' and 'exec' mnt.Options more than once: %w", errOptionArg) } setExec = true - mnt.Options = append(mnt.Options, name) + mnt.mount.Options = append(mnt.mount.Options, name) case "nosuid", "suid": if setSuid { return nil, fmt.Errorf("cannot pass 'nosuid' and 'suid' mnt.Options more than once: %w", errOptionArg) } setSuid = true - mnt.Options = append(mnt.Options, name) + mnt.mount.Options = append(mnt.mount.Options, name) case "noswap": if setSwap { return nil, fmt.Errorf("cannot pass 'noswap' mnt.Options more than once: %w", errOptionArg) @@ -344,7 +350,7 @@ func parseMountOptions(mountType string, args []string) (*spec.Mount, error) { return nil, fmt.Errorf("the 'noswap' option is only allowed with rootful tmpfs mounts: %w", errOptionArg) } setSwap = true - mnt.Options = append(mnt.Options, name) + mnt.mount.Options = append(mnt.mount.Options, name) case "relabel": if setRelabel { return nil, fmt.Errorf("cannot pass 'relabel' option more than once: %w", errOptionArg) @@ -355,19 +361,19 @@ func parseMountOptions(mountType string, args []string) (*spec.Mount, error) { } switch value { case "private": - mnt.Options = append(mnt.Options, "Z") + mnt.mount.Options = append(mnt.mount.Options, "Z") case "shared": - mnt.Options = append(mnt.Options, "z") + mnt.mount.Options = append(mnt.mount.Options, "z") default: return nil, fmt.Errorf("%s mount option must be 'private' or 'shared': %w", name, util.ErrBadMntOption) } case "shared", "rshared", "private", "rprivate", "slave", "rslave", "unbindable", "runbindable", "Z", "z", "no-dereference": - mnt.Options = append(mnt.Options, name) + mnt.mount.Options = append(mnt.mount.Options, name) case "src", "source": if mountType == define.TypeTmpfs { return nil, fmt.Errorf("%q option not supported for %q mount types", name, mountType) } - if mnt.Source != "" { + if mnt.mount.Source != "" { return nil, fmt.Errorf("cannot pass %q option more than once: %w", name, errOptionArg) } if !hasValue { @@ -376,9 +382,17 @@ func parseMountOptions(mountType string, args []string) (*spec.Mount, error) { if len(value) == 0 { return nil, fmt.Errorf("host directory cannot be empty: %w", errOptionArg) } - mnt.Source = value + mnt.mount.Source = value + case "subpath", "volume-subpath": + if mountType != define.TypeVolume { + return nil, fmt.Errorf("cannot set option %q on non-volume mounts", name) + } + if !hasValue { + return nil, fmt.Errorf("%v: %w", name, errOptionArg) + } + mnt.subPath = value case "target", "dst", "destination": - if mnt.Destination != "" { + if mnt.mount.Destination != "" { return nil, fmt.Errorf("cannot pass %q option more than once: %w", name, errOptionArg) } if !hasValue { @@ -387,7 +401,7 @@ func parseMountOptions(mountType string, args []string) (*spec.Mount, error) { if err := parse.ValidateVolumeCtrDir(value); err != nil { return nil, err } - mnt.Destination = unixPathClean(value) + mnt.mount.Destination = unixPathClean(value) case "tmpcopyup", "notmpcopyup": if mountType != define.TypeTmpfs { return nil, fmt.Errorf("%q option not supported for %q mount types", name, mountType) @@ -396,7 +410,7 @@ func parseMountOptions(mountType string, args []string) (*spec.Mount, error) { return nil, fmt.Errorf("cannot pass 'tmpcopyup' and 'notmpcopyup' mnt.Options more than once: %w", errOptionArg) } setTmpcopyup = true - mnt.Options = append(mnt.Options, name) + mnt.mount.Options = append(mnt.mount.Options, name) case "tmpfs-mode": if mountType != define.TypeTmpfs { return nil, fmt.Errorf("%q option not supported for %q mount types", name, mountType) @@ -404,7 +418,7 @@ func parseMountOptions(mountType string, args []string) (*spec.Mount, error) { if !hasValue { return nil, fmt.Errorf("%v: %w", name, errOptionArg) } - mnt.Options = append(mnt.Options, fmt.Sprintf("mode=%s", value)) + mnt.mount.Options = append(mnt.mount.Options, fmt.Sprintf("mode=%s", value)) case "tmpfs-size": if mountType != define.TypeTmpfs { return nil, fmt.Errorf("%q option not supported for %q mount types", name, mountType) @@ -412,7 +426,7 @@ func parseMountOptions(mountType string, args []string) (*spec.Mount, error) { if !hasValue { return nil, fmt.Errorf("%v: %w", name, errOptionArg) } - mnt.Options = append(mnt.Options, fmt.Sprintf("size=%s", value)) + mnt.mount.Options = append(mnt.mount.Options, fmt.Sprintf("size=%s", value)) case "U", "chown": if setOwnership { return nil, fmt.Errorf("cannot pass 'U' or 'chown' option more than once: %w", errOptionArg) @@ -422,7 +436,7 @@ func parseMountOptions(mountType string, args []string) (*spec.Mount, error) { return nil, err } if ok { - mnt.Options = append(mnt.Options, "U") + mnt.mount.Options = append(mnt.mount.Options, "U") } setOwnership = true case "volume-label": @@ -434,25 +448,26 @@ func parseMountOptions(mountType string, args []string) (*spec.Mount, error) { if mountType != define.TypeVolume { return nil, fmt.Errorf("%q option not supported for %q mount types", name, mountType) } - mnt.Options = append(mnt.Options, arg) + mnt.mount.Options = append(mnt.mount.Options, arg) default: return nil, fmt.Errorf("%s: %w", name, util.ErrBadMntOption) } } - if mountType != "glob" && len(mnt.Destination) == 0 { + if mountType != "glob" && len(mnt.mount.Destination) == 0 { return nil, errNoDest } - return &mnt, nil + return mnt, nil } // Parse glob mounts entry from the --mount flag. func getGlobMounts(args []string) ([]spec.Mount, error) { mounts := []spec.Mount{} - mnt, err := parseMountOptions("glob", args) + uMnt, err := parseMountOptions("glob", args) if err != nil { return nil, err } + mnt := uMnt.mount globs, err := filepath.Glob(mnt.Source) if err != nil { @@ -488,10 +503,11 @@ func getBindMount(args []string) (spec.Mount, error) { Type: define.TypeBind, } var err error - mnt, err := parseMountOptions(newMount.Type, args) + uMnt, err := parseMountOptions(newMount.Type, args) if err != nil { return newMount, err } + mnt := uMnt.mount if len(mnt.Destination) == 0 { return newMount, errNoDest @@ -519,10 +535,11 @@ func parseMemoryMount(args []string, mountType string) (spec.Mount, error) { } var err error - mnt, err := parseMountOptions(newMount.Type, args) + uMnt, err := parseMountOptions(newMount.Type, args) if err != nil { return newMount, err } + mnt := uMnt.mount if len(mnt.Destination) == 0 { return newMount, errNoDest } @@ -576,12 +593,14 @@ func getNamedVolume(args []string) (*specgen.NamedVolume, error) { if err != nil { return nil, err } - if len(mnt.Destination) == 0 { + if len(mnt.mount.Destination) == 0 { return nil, errNoDest } - newVolume.Options = mnt.Options - newVolume.Name = mnt.Source - newVolume.Dest = mnt.Destination + + newVolume.Options = mnt.mount.Options + newVolume.SubPath = mnt.subPath + newVolume.Name = mnt.mount.Source + newVolume.Dest = mnt.mount.Destination return newVolume, nil } diff --git a/test/e2e/run_volume_test.go b/test/e2e/run_volume_test.go index c3546a317d..6f184c3218 100644 --- a/test/e2e/run_volume_test.go +++ b/test/e2e/run_volume_test.go @@ -1069,4 +1069,24 @@ RUN chmod 755 /test1 /test2 /test3`, ALPINE) mountVolumeAndCheckDirectory(volName, "/test3", "test2", imgName) }) + + It("podman run --mount type=volume,subpath=", func() { + volName := "testvol" + mkvol := podmanTest.Podman([]string{"volume", "create", volName}) + mkvol.WaitWithDefaultTimeout() + Expect(mkvol).Should(ExitCleanly()) + + subvol := "/test/test2/" + pathInCtr := "/mnt" + pathToCreate := filepath.Join(pathInCtr, subvol) + popvol := podmanTest.Podman([]string{"run", "-v", fmt.Sprintf("%s:/mnt", volName), ALPINE, "sh", "-c", fmt.Sprintf("mkdir -p %s; touch %s; touch %s", pathToCreate, filepath.Join(pathToCreate, "foo"), filepath.Join(pathToCreate, "bar"))}) + popvol.WaitWithDefaultTimeout() + Expect(popvol).Should(ExitCleanly()) + + checkCtr := podmanTest.Podman([]string{"run", "--mount", fmt.Sprintf("type=volume,source=%s,target=%s,subpath=%s", volName, pathInCtr, subvol), ALPINE, "ls", pathInCtr}) + checkCtr.WaitWithDefaultTimeout() + Expect(checkCtr).To(ExitCleanly()) + Expect(checkCtr.OutputToString()).To(ContainSubstring("foo")) + Expect(checkCtr.OutputToString()).To(ContainSubstring("bar")) + }) })