diff --git a/pkg/specgenutil/volumes.go b/pkg/specgenutil/volumes.go index c70206addf..c77710ffea 100644 --- a/pkg/specgenutil/volumes.go +++ b/pkg/specgenutil/volumes.go @@ -236,22 +236,39 @@ func Mounts(mountFlag []string) (map[string]spec.Mount, map[string]*specgen.Name return finalMounts, finalNamedVolumes, finalImageVolumes, nil } -// Parse a single bind mount entry from the --mount flag. -func getBindMount(args []string) (spec.Mount, error) { - newMount := spec.Mount{ - Type: define.TypeBind, - } - - var setSource, setDest, setRORW, setSuid, setDev, setExec, setRelabel, setOwnership bool +func parseMountOptions(mountType string, args []string) (*spec.Mount, error) { + var setTmpcopyup, setRORW, setSuid, setDev, setExec, setRelabel, setOwnership bool + mnt := spec.Mount{} for _, val := range args { kv := strings.SplitN(val, "=", 2) switch kv[0] { case "bind-nonrecursive": - newMount.Options = append(newMount.Options, "bind") + if mountType != define.TypeBind { + return nil, fmt.Errorf("%q option not supported for %q mount types", kv[0], mountType) + } + mnt.Options = append(mnt.Options, "bind") + case "bind-propagation": + if mountType != define.TypeBind { + return nil, fmt.Errorf("%q option not supported for %q mount types", kv[0], mountType) + } + if len(kv) == 1 { + return nil, fmt.Errorf("%v: %w", kv[0], errOptionArg) + } + mnt.Options = append(mnt.Options, kv[1]) + case "consistency": + // Often used on MACs and mistakenly on Linux platforms. + // Since Docker ignores this option so shall we. + continue + case "idmap": + if len(kv) > 1 { + mnt.Options = append(mnt.Options, fmt.Sprintf("idmap=%s", kv[1])) + } else { + mnt.Options = append(mnt.Options, "idmap") + } case "readonly", "ro", "rw": if setRORW { - return newMount, fmt.Errorf("cannot pass 'readonly', 'ro', or 'rw' options more than once: %w", errOptionArg) + return nil, fmt.Errorf("cannot pass 'readonly', 'ro', or 'rw' mnt.Options more than once: %w", errOptionArg) } setRORW = true // Can be formatted as one of: @@ -266,121 +283,162 @@ func getBindMount(args []string) (spec.Mount, error) { } switch len(kv) { case 1: - newMount.Options = append(newMount.Options, kv[0]) + mnt.Options = append(mnt.Options, kv[0]) case 2: switch strings.ToLower(kv[1]) { case "true": - newMount.Options = append(newMount.Options, kv[0]) + mnt.Options = append(mnt.Options, kv[0]) case "false": // Set the opposite only for rw // ro's opposite is the default if kv[0] == "rw" { - newMount.Options = append(newMount.Options, "ro") + mnt.Options = append(mnt.Options, "ro") } - default: - return newMount, fmt.Errorf("'readonly', 'ro', or 'rw' must be set to true or false, instead received %q: %w", kv[1], errOptionArg) } - default: - return newMount, fmt.Errorf("badly formatted option %q: %w", val, errOptionArg) } - case "nosuid", "suid": - if setSuid { - return newMount, fmt.Errorf("cannot pass 'nosuid' and 'suid' options more than once: %w", errOptionArg) - } - setSuid = true - newMount.Options = append(newMount.Options, kv[0]) case "nodev", "dev": if setDev { - return newMount, fmt.Errorf("cannot pass 'nodev' and 'dev' options more than once: %w", errOptionArg) + return nil, fmt.Errorf("cannot pass 'nodev' and 'dev' mnt.Options more than once: %w", errOptionArg) } setDev = true - newMount.Options = append(newMount.Options, kv[0]) + mnt.Options = append(mnt.Options, kv[0]) case "noexec", "exec": if setExec { - return newMount, fmt.Errorf("cannot pass 'noexec' and 'exec' options more than once: %w", errOptionArg) + return nil, fmt.Errorf("cannot pass 'noexec' and 'exec' mnt.Options more than once: %w", errOptionArg) } setExec = true - newMount.Options = append(newMount.Options, kv[0]) - case "shared", "rshared", "private", "rprivate", "slave", "rslave", "unbindable", "runbindable", "Z", "z": - newMount.Options = append(newMount.Options, kv[0]) - case "bind-propagation": - if len(kv) == 1 { - return newMount, fmt.Errorf("%v: %w", kv[0], errOptionArg) + mnt.Options = append(mnt.Options, kv[0]) + 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, kv[0]) + case "relabel": + if setRelabel { + return nil, fmt.Errorf("cannot pass 'relabel' option more than once: %w", errOptionArg) + } + setRelabel = true + if len(kv) != 2 { + return nil, fmt.Errorf("%s mount option must be 'private' or 'shared': %w", kv[0], util.ErrBadMntOption) + } + switch kv[1] { + case "private": + mnt.Options = append(mnt.Options, "Z") + case "shared": + mnt.Options = append(mnt.Options, "z") + default: + return nil, fmt.Errorf("%s mount option must be 'private' or 'shared': %w", kv[0], util.ErrBadMntOption) } - newMount.Options = append(newMount.Options, kv[1]) + case "shared", "rshared", "private", "rprivate", "slave", "rslave", "unbindable", "runbindable", "Z", "z": + mnt.Options = append(mnt.Options, kv[0]) case "src", "source": + if mountType == define.TypeTmpfs { + return nil, fmt.Errorf("%q option not supported for %q mount types", kv[0], mountType) + } + if mnt.Source != "" { + return nil, fmt.Errorf("cannot pass %q option more than once: %w", kv[0], errOptionArg) + } if len(kv) == 1 { - return newMount, fmt.Errorf("%v: %w", kv[0], errOptionArg) + return nil, fmt.Errorf("%v: %w", kv[0], errOptionArg) } if len(kv[1]) == 0 { - return newMount, fmt.Errorf("host directory cannot be empty: %w", errOptionArg) + return nil, fmt.Errorf("host directory cannot be empty: %w", errOptionArg) } - newMount.Source = kv[1] - setSource = true + mnt.Source = kv[1] case "target", "dst", "destination": + if mnt.Destination != "" { + return nil, fmt.Errorf("cannot pass %q option more than once: %w", kv[0], errOptionArg) + } if len(kv) == 1 { - return newMount, fmt.Errorf("%v: %w", kv[0], errOptionArg) + return nil, fmt.Errorf("%v: %w", kv[0], errOptionArg) } if err := parse.ValidateVolumeCtrDir(kv[1]); err != nil { - return newMount, err + return nil, err } - newMount.Destination = unixPathClean(kv[1]) - setDest = true - case "relabel": - if setRelabel { - return newMount, fmt.Errorf("cannot pass 'relabel' option more than once: %w", errOptionArg) + mnt.Destination = unixPathClean(kv[1]) + case "tmpcopyup", "notmpcopyup": + if mountType != define.TypeTmpfs { + return nil, fmt.Errorf("%q option not supported for %q mount types", kv[0], mountType) } - setRelabel = true - if len(kv) != 2 { - return newMount, fmt.Errorf("%s mount option must be 'private' or 'shared': %w", kv[0], util.ErrBadMntOption) + if setTmpcopyup { + return nil, fmt.Errorf("cannot pass 'tmpcopyup' and 'notmpcopyup' mnt.Options more than once: %w", errOptionArg) } - switch kv[1] { - case "private": - newMount.Options = append(newMount.Options, "Z") - case "shared": - newMount.Options = append(newMount.Options, "z") - default: - return newMount, fmt.Errorf("%s mount option must be 'private' or 'shared': %w", kv[0], util.ErrBadMntOption) + setTmpcopyup = true + mnt.Options = append(mnt.Options, kv[0]) + case "tmpfs-mode": + if mountType != define.TypeTmpfs { + return nil, fmt.Errorf("%q option not supported for %q mount types", kv[0], mountType) + } + if len(kv) == 1 { + return nil, fmt.Errorf("%v: %w", kv[0], errOptionArg) } + mnt.Options = append(mnt.Options, fmt.Sprintf("mode=%s", kv[1])) + case "tmpfs-size": + if mountType != define.TypeTmpfs { + return nil, fmt.Errorf("%q option not supported for %q mount types", kv[0], mountType) + } + if len(kv) == 1 { + return nil, fmt.Errorf("%v: %w", kv[0], errOptionArg) + } + mnt.Options = append(mnt.Options, fmt.Sprintf("size=%s", kv[1])) case "U", "chown": if setOwnership { - return newMount, fmt.Errorf("cannot pass 'U' or 'chown' option more than once: %w", errOptionArg) + return nil, fmt.Errorf("cannot pass 'U' or 'chown' option more than once: %w", errOptionArg) } ok, err := validChownFlag(val) if err != nil { - return newMount, err + return nil, err } if ok { - newMount.Options = append(newMount.Options, "U") + mnt.Options = append(mnt.Options, "U") } setOwnership = true - case "idmap": - if len(kv) > 1 { - newMount.Options = append(newMount.Options, fmt.Sprintf("idmap=%s", kv[1])) - } else { - newMount.Options = append(newMount.Options, "idmap") + case "volume-label": + if mountType != define.TypeVolume { + return nil, fmt.Errorf("%q option not supported for %q mount types", kv[0], mountType) } - case "consistency": - // Often used on MACs and mistakenly on Linux platforms. - // Since Docker ignores this option so shall we. - continue + return nil, fmt.Errorf("the --volume-label option is not presently implemented") + case "volume-opt": + if mountType != define.TypeVolume { + return nil, fmt.Errorf("%q option not supported for %q mount types", kv[0], mountType) + } + mnt.Options = append(mnt.Options, val) default: - return newMount, fmt.Errorf("%s: %w", kv[0], util.ErrBadMntOption) + return nil, fmt.Errorf("%s: %w", kv[0], util.ErrBadMntOption) } } + if len(mnt.Destination) == 0 { + return nil, errNoDest + } + return &mnt, nil +} - if !setDest { +// Parse a single bind mount entry from the --mount flag. +func getBindMount(args []string) (spec.Mount, error) { + newMount := spec.Mount{ + Type: define.TypeBind, + } + var err error + mnt, err := parseMountOptions(newMount.Type, args) + if err != nil { + return newMount, err + } + + if len(mnt.Destination) == 0 { return newMount, errNoDest } - if !setSource { - newMount.Source = newMount.Destination + if len(mnt.Source) == 0 { + mnt.Source = mnt.Destination } - options, err := parse.ValidateVolumeOpts(newMount.Options) + options, err := parse.ValidateVolumeOpts(mnt.Options) if err != nil { return newMount, err } + newMount.Source = mnt.Source + newMount.Destination = mnt.Destination newMount.Options = options return newMount, nil } @@ -392,87 +450,16 @@ func getTmpfsMount(args []string) (spec.Mount, error) { Source: define.TypeTmpfs, } - var setDest, setRORW, setSuid, setDev, setExec, setTmpcopyup, setOwnership bool - - for _, val := range args { - kv := strings.SplitN(val, "=", 2) - switch kv[0] { - case "tmpcopyup", "notmpcopyup": - if setTmpcopyup { - return newMount, fmt.Errorf("cannot pass 'tmpcopyup' and 'notmpcopyup' options more than once: %w", errOptionArg) - } - setTmpcopyup = true - newMount.Options = append(newMount.Options, kv[0]) - case "ro", "rw": - if setRORW { - return newMount, fmt.Errorf("cannot pass 'ro' and 'rw' options more than once: %w", errOptionArg) - } - setRORW = true - newMount.Options = append(newMount.Options, kv[0]) - case "nosuid", "suid": - if setSuid { - return newMount, fmt.Errorf("cannot pass 'nosuid' and 'suid' options more than once: %w", errOptionArg) - } - setSuid = true - newMount.Options = append(newMount.Options, kv[0]) - case "nodev", "dev": - if setDev { - return newMount, fmt.Errorf("cannot pass 'nodev' and 'dev' options more than once: %w", errOptionArg) - } - setDev = true - newMount.Options = append(newMount.Options, kv[0]) - case "noexec", "exec": - if setExec { - return newMount, fmt.Errorf("cannot pass 'noexec' and 'exec' options more than once: %w", errOptionArg) - } - setExec = true - newMount.Options = append(newMount.Options, kv[0]) - case "tmpfs-mode": - if len(kv) == 1 { - return newMount, fmt.Errorf("%v: %w", kv[0], errOptionArg) - } - newMount.Options = append(newMount.Options, fmt.Sprintf("mode=%s", kv[1])) - case "tmpfs-size": - if len(kv) == 1 { - return newMount, fmt.Errorf("%v: %w", kv[0], errOptionArg) - } - newMount.Options = append(newMount.Options, fmt.Sprintf("size=%s", kv[1])) - case "src", "source": - return newMount, fmt.Errorf("source is not supported with tmpfs mounts") - case "target", "dst", "destination": - if len(kv) == 1 { - return newMount, fmt.Errorf("%v: %w", kv[0], errOptionArg) - } - if err := parse.ValidateVolumeCtrDir(kv[1]); err != nil { - return newMount, err - } - newMount.Destination = unixPathClean(kv[1]) - setDest = true - case "U", "chown": - if setOwnership { - return newMount, fmt.Errorf("cannot pass 'U' or 'chown' option more than once: %w", errOptionArg) - } - ok, err := validChownFlag(val) - if err != nil { - return newMount, err - } - if ok { - newMount.Options = append(newMount.Options, "U") - } - setOwnership = true - case "consistency": - // Often used on MACs and mistakenly on Linux platforms. - // Since Docker ignores this option so shall we. - continue - default: - return newMount, fmt.Errorf("%s: %w", kv[0], util.ErrBadMntOption) - } + var err error + mnt, err := parseMountOptions(newMount.Type, args) + if err != nil { + return newMount, err } - - if !setDest { + if len(mnt.Destination) == 0 { return newMount, errNoDest } - + newMount.Destination = mnt.Destination + newMount.Options = mnt.Options return newMount, nil } @@ -517,84 +504,16 @@ func getDevptsMount(args []string) (spec.Mount, error) { func getNamedVolume(args []string) (*specgen.NamedVolume, error) { newVolume := new(specgen.NamedVolume) - var setDest, setRORW, setSuid, setDev, setExec, setOwnership bool - - for _, val := range args { - kv := strings.SplitN(val, "=", 2) - switch kv[0] { - case "volume-opt": - newVolume.Options = append(newVolume.Options, val) - case "ro", "rw": - if setRORW { - return nil, fmt.Errorf("cannot pass 'ro' and 'rw' options more than once: %w", errOptionArg) - } - setRORW = true - newVolume.Options = append(newVolume.Options, kv[0]) - case "nosuid", "suid": - if setSuid { - return nil, fmt.Errorf("cannot pass 'nosuid' and 'suid' options more than once: %w", errOptionArg) - } - setSuid = true - newVolume.Options = append(newVolume.Options, kv[0]) - case "nodev", "dev": - if setDev { - return nil, fmt.Errorf("cannot pass 'nodev' and 'dev' options more than once: %w", errOptionArg) - } - setDev = true - newVolume.Options = append(newVolume.Options, kv[0]) - case "noexec", "exec": - if setExec { - return nil, fmt.Errorf("cannot pass 'noexec' and 'exec' options more than once: %w", errOptionArg) - } - setExec = true - newVolume.Options = append(newVolume.Options, kv[0]) - case "volume-label": - return nil, fmt.Errorf("the --volume-label option is not presently implemented") - case "src", "source": - if len(kv) == 1 { - return nil, fmt.Errorf("%v: %w", kv[0], errOptionArg) - } - newVolume.Name = kv[1] - case "target", "dst", "destination": - if len(kv) == 1 { - return nil, fmt.Errorf("%v: %w", kv[0], errOptionArg) - } - if err := parse.ValidateVolumeCtrDir(kv[1]); err != nil { - return nil, err - } - newVolume.Dest = unixPathClean(kv[1]) - setDest = true - case "idmap": - if len(kv) > 1 { - newVolume.Options = append(newVolume.Options, fmt.Sprintf("idmap=%s", kv[1])) - } else { - newVolume.Options = append(newVolume.Options, "idmap") - } - case "U", "chown": - if setOwnership { - return newVolume, fmt.Errorf("cannot pass 'U' or 'chown' option more than once: %w", errOptionArg) - } - ok, err := validChownFlag(val) - if err != nil { - return newVolume, err - } - if ok { - newVolume.Options = append(newVolume.Options, "U") - } - setOwnership = true - case "consistency": - // Often used on MACs and mistakenly on Linux platforms. - // Since Docker ignores this option so shall we. - continue - default: - return nil, fmt.Errorf("%s: %w", kv[0], util.ErrBadMntOption) - } + mnt, err := parseMountOptions(define.TypeVolume, args) + if err != nil { + return nil, err } - - if !setDest { + if len(mnt.Destination) == 0 { return nil, errNoDest } - + newVolume.Options = mnt.Options + newVolume.Name = mnt.Source + newVolume.Dest = mnt.Destination return newVolume, nil } diff --git a/test/system/060-mount.bats b/test/system/060-mount.bats index dd8aa3e97b..30fe10e410 100644 --- a/test/system/060-mount.bats +++ b/test/system/060-mount.bats @@ -84,6 +84,22 @@ load helpers is "$output" "" "podman image mount, no args, after umount" } +@test "podman run --mount ro=false " { + local volpath=/path/in/container + local stdopts="type=volume,destination=$volpath" + + # Variations on a theme (not by Paganini). All of these should fail. + for varopt in readonly readonly=true ro=true ro rw=false;do + run_podman 1 run --rm -q --mount $stdopts,$varopt $IMAGE touch $volpath/a + is "$output" "touch: $volpath/a: Read-only file system" "with $varopt" + done + + # All of these should pass + for varopt in rw rw=true ro=false readonly=false;do + run_podman run --rm -q --mount $stdopts,$varopt $IMAGE touch $volpath/a + done +} + @test "podman run --mount image" { skip_if_rootless "too hard to test rootless"