Merge pull request #3458 from rhatdan/volume

Use buildah/pkg/parse volume parsing rather then internal version

2 files changed, 21 insertions, 90 deletions

diff --git a/pkg/adapter/pods.go b/pkg/adapter/pods.go
index b9d7fcd9b..2a52cfd0c 100644
--- a/pkg/adapter/pods.go
+++ b/pkg/adapter/pods.go
@@ -10,6 +10,7 @@ import (
 	"os"
 	"strings"
 
+	"github.com/containers/buildah/pkg/parse"
 	"github.com/containers/image/types"
 	"github.com/containers/libpod/cmd/podman/cliconfig"
 	"github.com/containers/libpod/cmd/podman/shared"
@@ -570,7 +571,7 @@ func (r *LocalRuntime) PlayKubeYAML(ctx context.Context, c *cliconfig.KubePlayVa
 			}
 		}
 
-		if err := createconfig.ValidateVolumeHostDir(hostPath.Path); err != nil {
+		if err := parse.ValidateVolumeHostDir(hostPath.Path); err != nil {
 			return nil, errors.Wrapf(err, "Error in parsing HostPath in YAML")
 		}
 		volumes[volume.Name] = hostPath.Path
@@ -728,7 +729,7 @@ func kubeContainerToCreateConfig(ctx context.Context, containerYAML v1.Container
 		if !exists {
 			return nil, errors.Errorf("Volume mount %s specified for container but not configured in volumes", volume.Name)
 		}
-		if err := createconfig.ValidateVolumeCtrDir(volume.MountPath); err != nil {
+		if err := parse.ValidateVolumeCtrDir(volume.MountPath); err != nil {
 			return nil, errors.Wrapf(err, "error in parsing MountPath")
 		}
 		containerConfig.Volumes = append(containerConfig.Volumes, fmt.Sprintf("%s:%s", hostPath, volume.MountPath))
diff --git a/pkg/spec/storage.go b/pkg/spec/storage.go
index 88f1f6dc1..ac7a2c30f 100644
--- a/pkg/spec/storage.go
+++ b/pkg/spec/storage.go
@@ -7,6 +7,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/containers/buildah/pkg/parse"
 	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/pkg/util"
 	"github.com/containers/storage/pkg/stringid"
@@ -248,8 +249,11 @@ func (config *CreateConfig) getVolumesFrom(runtime *libpod.Runtime) (map[string]
 	finalNamedVolumes := make(map[string]*libpod.ContainerNamedVolume)
 
 	for _, vol := range config.VolumesFrom {
-		options := []string{}
-		splitVol := strings.SplitN(vol, ":", 2)
+		var (
+			options  = []string{}
+			err      error
+			splitVol = strings.SplitN(vol, ":", 2)
+		)
 		if len(splitVol) == 2 {
 			if strings.Contains(splitVol[1], "Z") ||
 				strings.Contains(splitVol[1], "private") ||
@@ -257,12 +261,10 @@ func (config *CreateConfig) getVolumesFrom(runtime *libpod.Runtime) (map[string]
 				strings.Contains(splitVol[1], "shared") {
 				return nil, nil, errors.Errorf("invalid options %q, can only specify 'ro', 'rw', and 'z", splitVol[1])
 			}
-			options = strings.Split(splitVol[1], ",")
-			opts, err := ValidateVolumeOpts(options)
-			if err != nil {
+
+			if options, err = parse.ValidateVolumeOpts(strings.Split(splitVol[1], ",")); err != nil {
 				return nil, nil, err
 			}
-			options = opts
 		}
 		ctr, err := runtime.LookupContainer(splitVol[0])
 		if err != nil {
@@ -429,7 +431,7 @@ func getBindMount(args []string) (spec.Mount, error) {
 			if len(kv) == 1 {
 				return newMount, errors.Wrapf(optionArgError, kv[0])
 			}
-			if err := ValidateVolumeHostDir(kv[1]); err != nil {
+			if err := parse.ValidateVolumeHostDir(kv[1]); err != nil {
 				return newMount, err
 			}
 			newMount.Source = kv[1]
@@ -438,7 +440,7 @@ func getBindMount(args []string) (spec.Mount, error) {
 			if len(kv) == 1 {
 				return newMount, errors.Wrapf(optionArgError, kv[0])
 			}
-			if err := ValidateVolumeCtrDir(kv[1]); err != nil {
+			if err := parse.ValidateVolumeCtrDir(kv[1]); err != nil {
 				return newMount, err
 			}
 			newMount.Destination = kv[1]
@@ -456,12 +458,11 @@ func getBindMount(args []string) (spec.Mount, error) {
 		newMount.Source = newMount.Destination
 	}
 
-	opts, err := ValidateVolumeOpts(newMount.Options)
+	options, err := parse.ValidateVolumeOpts(newMount.Options)
 	if err != nil {
 		return newMount, err
 	}
-	newMount.Options = opts
-
+	newMount.Options = options
 	return newMount, nil
 }
 
@@ -495,7 +496,7 @@ func getTmpfsMount(args []string) (spec.Mount, error) {
 			if len(kv) == 1 {
 				return newMount, errors.Wrapf(optionArgError, kv[0])
 			}
-			if err := ValidateVolumeCtrDir(kv[1]); err != nil {
+			if err := parse.ValidateVolumeCtrDir(kv[1]); err != nil {
 				return newMount, err
 			}
 			newMount.Destination = kv[1]
@@ -539,7 +540,7 @@ func getNamedVolume(args []string) (*libpod.ContainerNamedVolume, error) {
 			if len(kv) == 1 {
 				return nil, errors.Wrapf(optionArgError, kv[0])
 			}
-			if err := ValidateVolumeCtrDir(kv[1]); err != nil {
+			if err := parse.ValidateVolumeCtrDir(kv[1]); err != nil {
 				return nil, err
 			}
 			newVolume.Dest = kv[1]
@@ -559,75 +560,6 @@ func getNamedVolume(args []string) (*libpod.ContainerNamedVolume, error) {
 	return newVolume, nil
 }
 
-// ValidateVolumeHostDir validates a volume mount's source directory
-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 validates a volume mount's destination directory.
-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 %q, must be an absolute path", ctrDir)
-	}
-	return nil
-}
-
-// ValidateVolumeOpts validates a volume's options
-func ValidateVolumeOpts(options []string) ([]string, error) {
-	var foundRootPropagation, foundRWRO, foundLabelChange, bindType int
-	finalOpts := make([]string, 0, len(options))
-	for _, opt := range options {
-		switch opt {
-		case "rw", "ro":
-			foundRWRO++
-			if foundRWRO > 1 {
-				return nil, errors.Errorf("invalid options %q, can only specify 1 'rw' or 'ro' option", strings.Join(options, ", "))
-			}
-		case "z", "Z":
-			foundLabelChange++
-			if foundLabelChange > 1 {
-				return nil, errors.Errorf("invalid options %q, can only specify 1 'z' or 'Z' option", strings.Join(options, ", "))
-			}
-		case "private", "rprivate", "shared", "rshared", "slave", "rslave":
-			foundRootPropagation++
-			if foundRootPropagation > 1 {
-				return nil, errors.Errorf("invalid options %q, can only specify 1 '[r]shared', '[r]private' or '[r]slave' option", strings.Join(options, ", "))
-			}
-		case "bind", "rbind":
-			bindType++
-			if bindType > 1 {
-				return nil, errors.Errorf("invalid options %q, can only specify 1 '[r]bind' option", strings.Join(options, ", "))
-			}
-		case "cached", "delegated":
-			// The discarded ops are OS X specific volume options
-			// introduced in a recent Docker version.
-			// They have no meaning on Linux, so here we silently
-			// drop them. This matches Docker's behavior (the options
-			// are intended to be always safe to use, even not on OS
-			// X).
-			continue
-		default:
-			return nil, errors.Errorf("invalid mount option %q", opt)
-		}
-		finalOpts = append(finalOpts, opt)
-	}
-	return finalOpts, nil
-}
-
-// GetVolumeMounts takes user provided input for bind mounts and creates Mount structs
 func (config *CreateConfig) getVolumeMounts() (map[string]spec.Mount, map[string]*libpod.ContainerNamedVolume, error) {
 	mounts := make(map[string]spec.Mount)
 	volumes := make(map[string]*libpod.ContainerNamedVolume)
@@ -639,6 +571,7 @@ func (config *CreateConfig) getVolumeMounts() (map[string]spec.Mount, map[string
 			options []string
 			src     string
 			dest    string
+			err     error
 		)
 
 		splitVol := strings.Split(vol, ":")
@@ -653,18 +586,15 @@ func (config *CreateConfig) getVolumeMounts() (map[string]spec.Mount, map[string
 			dest = splitVol[1]
 		}
 		if len(splitVol) > 2 {
-			options = strings.Split(splitVol[2], ",")
-			opts, err := ValidateVolumeOpts(options)
-			if err != nil {
+			if options, err = parse.ValidateVolumeOpts(strings.Split(splitVol[2], ",")); err != nil {
 				return nil, nil, err
 			}
-			options = opts
 		}
 
-		if err := ValidateVolumeHostDir(src); err != nil {
+		if err := parse.ValidateVolumeHostDir(src); err != nil {
 			return nil, nil, err
 		}
-		if err := ValidateVolumeCtrDir(dest); err != nil {
+		if err := parse.ValidateVolumeCtrDir(dest); err != nil {
 			return nil, nil, err
 		}