Migrate to unified volume handling code

Unify handling for the --volume, --mount, --volumes-from, --tmpfs
and --init flags into a single file and set of functions. This
will greatly improve readability and maintainability.

Further, properly handle superceding and conflicting mounts. Our
current patchwork has serious issues when mounts conflict, or
when a mount from --volumes-from or an image volume should be
overwritten by a user volume or named volume.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>

2 files changed, 3 insertions, 211 deletions

diff --git a/cmd/podman/shared/create.go b/cmd/podman/shared/create.go
index ebc42457b..1e185d21f 100644
--- a/cmd/podman/shared/create.go
+++ b/cmd/podman/shared/create.go
@@ -25,7 +25,6 @@ import (
 	"github.com/docker/go-connections/nat"
 	"github.com/docker/go-units"
 	"github.com/google/shlex"
-	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/selinux/go-selinux/label"
 	"github.com/opentracing/opentracing-go"
 	"github.com/pkg/errors"
@@ -341,18 +340,6 @@ func ParseCreateOpts(ctx context.Context, c *GenericCLIResults, runtime *libpod.
 		}
 		blkioWeight = uint16(u)
 	}
-	var mountList []spec.Mount
-	if mountList, err = parseMounts(c.StringArray("mount")); err != nil {
-		return nil, err
-	}
-
-	if err = parseVolumes(c.StringArray("volume")); err != nil {
-		return nil, err
-	}
-
-	if err = parseVolumesFrom(c.StringSlice("volumes-from")); err != nil {
-		return nil, err
-	}
 
 	tty := c.Bool("tty")
 
@@ -636,6 +623,8 @@ func ParseCreateOpts(ctx context.Context, c *GenericCLIResults, runtime *libpod.
 		HTTPProxy:   c.Bool("http-proxy"),
 		NoHosts:     c.Bool("no-hosts"),
 		IDMappings:  idmappings,
+		Init:        c.Bool("init"),
+		InitPath:    c.String("init-path"),
 		Image:       imageName,
 		ImageID:     imageID,
 		Interactive: c.Bool("interactive"),
@@ -696,26 +685,13 @@ func ParseCreateOpts(ctx context.Context, c *GenericCLIResults, runtime *libpod.
 		Tty:         tty,
 		User:        user,
 		UsernsMode:  usernsMode,
-		Mounts:      mountList,
+		MountsFlag:  c.StringArray("mount"),
 		Volumes:     c.StringArray("volume"),
 		WorkDir:     workDir,
 		Rootfs:      rootfs,
 		VolumesFrom: c.StringSlice("volumes-from"),
 		Syslog:      c.Bool("syslog"),
 	}
-	if c.Bool("init") {
-		initPath := c.String("init-path")
-		if initPath == "" {
-			rtc, err := runtime.GetConfig()
-			if err != nil {
-				return nil, err
-			}
-			initPath = rtc.InitPath
-		}
-		if err := config.AddContainerInitBinary(initPath); err != nil {
-			return nil, err
-		}
-	}
 
 	if config.Privileged {
 		config.LabelOpts = label.DisableSecOpt()
diff --git a/cmd/podman/shared/create_cli.go b/cmd/podman/shared/create_cli.go
index 4f9cb1699..f731e8db5 100644
--- a/cmd/podman/shared/create_cli.go
+++ b/cmd/podman/shared/create_cli.go
@@ -2,15 +2,11 @@ package shared
 
 import (
 	"fmt"
-	"os"
-	"path/filepath"
 	"strings"
 
 	"github.com/containers/libpod/cmd/podman/shared/parse"
 	cc "github.com/containers/libpod/pkg/spec"
 	"github.com/containers/libpod/pkg/sysinfo"
-	"github.com/docker/go-units"
-	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -78,186 +74,6 @@ func addWarning(warnings []string, msg string) []string {
 	return append(warnings, msg)
 }
 
-// Format supported.
-// podman run --mount type=bind,src=/etc/resolv.conf,target=/etc/resolv.conf ...
-// podman run --mount type=tmpfs,target=/dev/shm ..
-func parseMounts(mounts []string) ([]spec.Mount, error) {
-	// TODO(vrothberg): the manual parsing can be replaced with a regular expression
-	//                  to allow a more robust parsing of the mount format and to give
-	//                  precise errors regarding supported format versus suppored options.
-	var mountList []spec.Mount
-	errInvalidSyntax := errors.Errorf("incorrect mount format: should be --mount type=<bind|tmpfs>,[src=<host-dir>,]target=<ctr-dir>[,options]")
-	for _, mount := range mounts {
-		var tokenCount int
-		var mountInfo spec.Mount
-
-		arr := strings.SplitN(mount, ",", 2)
-		if len(arr) < 2 {
-			return nil, errors.Wrapf(errInvalidSyntax, "%q", mount)
-		}
-		kv := strings.Split(arr[0], "=")
-		if kv[0] != "type" {
-			return nil, errors.Wrapf(errInvalidSyntax, "%q", mount)
-		}
-		switch kv[1] {
-		case "bind":
-			mountInfo.Type = string(cc.TypeBind)
-		case "tmpfs":
-			mountInfo.Type = string(cc.TypeTmpfs)
-			mountInfo.Source = string(cc.TypeTmpfs)
-			mountInfo.Options = append(mountInfo.Options, []string{"rprivate", "noexec", "nosuid", "nodev", "size=65536k"}...)
-
-		default:
-			return nil, errors.Errorf("invalid filesystem type %q", kv[1])
-		}
-
-		tokens := strings.Split(arr[1], ",")
-		for i, val := range tokens {
-			if i == (tokenCount - 1) {
-				//Parse tokens before options.
-				break
-			}
-			kv := strings.Split(val, "=")
-			switch kv[0] {
-			case "ro", "nosuid", "nodev", "noexec":
-				mountInfo.Options = append(mountInfo.Options, kv[0])
-			case "shared", "rshared", "private", "rprivate", "slave", "rslave", "Z", "z":
-				if mountInfo.Type != "bind" {
-					return nil, errors.Errorf("%s can only be used with bind mounts", kv[0])
-				}
-				mountInfo.Options = append(mountInfo.Options, kv[0])
-			case "tmpfs-mode":
-				if mountInfo.Type != "tmpfs" {
-					return nil, errors.Errorf("%s can only be used with tmpfs mounts", kv[0])
-				}
-				mountInfo.Options = append(mountInfo.Options, fmt.Sprintf("mode=%s", kv[1]))
-			case "tmpfs-size":
-				if mountInfo.Type != "tmpfs" {
-					return nil, errors.Errorf("%s can only be used with tmpfs mounts", kv[0])
-				}
-				shmSize, err := units.FromHumanSize(kv[1])
-				if err != nil {
-					return nil, errors.Wrapf(err, "unable to translate tmpfs-size")
-				}
-
-				mountInfo.Options = append(mountInfo.Options, fmt.Sprintf("size=%d", shmSize))
-
-			case "bind-propagation":
-				if mountInfo.Type != "bind" {
-					return nil, errors.Errorf("%s can only be used with bind mounts", kv[0])
-				}
-				mountInfo.Options = append(mountInfo.Options, kv[1])
-			case "src", "source":
-				if mountInfo.Type == "tmpfs" {
-					return nil, errors.Errorf("cannot use src= on a tmpfs file system")
-				}
-				if err := ValidateVolumeHostDir(kv[1]); err != nil {
-					return nil, err
-				}
-				mountInfo.Source = kv[1]
-			case "target", "dst", "destination":
-				if err := ValidateVolumeCtrDir(kv[1]); err != nil {
-					return nil, err
-				}
-				mountInfo.Destination = kv[1]
-			default:
-				return nil, errors.Errorf("incorrect mount option : %s", kv[0])
-			}
-		}
-		mountList = append(mountList, mountInfo)
-	}
-	return mountList, nil
-}
-
-func parseVolumes(volumes []string) error {
-	for _, volume := range volumes {
-		arr := strings.SplitN(volume, ":", 3)
-		if len(arr) < 2 {
-			return errors.Errorf("incorrect volume format %q, should be host-dir:ctr-dir[:option]", volume)
-		}
-		if err := ValidateVolumeHostDir(arr[0]); err != nil {
-			return err
-		}
-		if err := ValidateVolumeCtrDir(arr[1]); err != nil {
-			return err
-		}
-		if len(arr) > 2 {
-			if err := validateVolumeOpts(arr[2]); err != nil {
-				return err
-			}
-		}
-	}
-	return nil
-}
-
-func parseVolumesFrom(volumesFrom []string) error {
-	for _, vol := range volumesFrom {
-		arr := strings.SplitN(vol, ":", 2)
-		if len(arr) == 2 {
-			if strings.Contains(arr[1], "Z") || strings.Contains(arr[1], "private") || strings.Contains(arr[1], "slave") || strings.Contains(arr[1], "shared") {
-				return errors.Errorf("invalid options %q, can only specify 'ro', 'rw', and 'z", arr[1])
-			}
-			if err := validateVolumeOpts(arr[1]); err != nil {
-				return err
-			}
-		}
-	}
-	return nil
-}
-
-// ValidateVolumeHostDir ...
-func ValidateVolumeHostDir(hostDir string) error {
-	if len(hostDir) == 0 {
-		return errors.Errorf("host directory cannot be empty")
-	}
-	if filepath.IsAbs(hostDir) {
-		if _, err := os.Stat(hostDir); err != nil {
-			return errors.Wrapf(err, "error checking path %q", hostDir)
-		}
-	}
-	// If hostDir is not an absolute path, that means the user wants to create a
-	// named volume. This will be done later on in the code.
-	return nil
-}
-
-// ValidateVolumeCtrDir ...
-func ValidateVolumeCtrDir(ctrDir string) error {
-	if len(ctrDir) == 0 {
-		return errors.Errorf("container directory cannot be empty")
-	}
-	if !filepath.IsAbs(ctrDir) {
-		return errors.Errorf("invalid container path, must be an absolute path %q", ctrDir)
-	}
-	return nil
-}
-
-func validateVolumeOpts(option string) error {
-	var foundRootPropagation, foundRWRO, foundLabelChange int
-	options := strings.Split(option, ",")
-	for _, opt := range options {
-		switch opt {
-		case "rw", "ro":
-			foundRWRO++
-			if foundRWRO > 1 {
-				return errors.Errorf("invalid options %q, can only specify 1 'rw' or 'ro' option", option)
-			}
-		case "z", "Z":
-			foundLabelChange++
-			if foundLabelChange > 1 {
-				return errors.Errorf("invalid options %q, can only specify 1 'z' or 'Z' option", option)
-			}
-		case "private", "rprivate", "shared", "rshared", "slave", "rslave":
-			foundRootPropagation++
-			if foundRootPropagation > 1 {
-				return errors.Errorf("invalid options %q, can only specify 1 '[r]shared', '[r]private' or '[r]slave' option", option)
-			}
-		default:
-			return errors.Errorf("invalid option type %q", option)
-		}
-	}
-	return nil
-}
-
 func verifyContainerResources(config *cc.CreateConfig, update bool) ([]string, error) {
 	warnings := []string{}
 	sysInfo := sysinfo.New(true)