Merge pull request #11231 from flouthoc/move-volume-dest-to-server

volume: move validating volume dest from client to server.

.gitignore

@@ -33,6 +33,7 @@ release.txt
 /test/goecho/goecho
 /test/testvol/testvol
 .vscode*
+tags
 result
 # Necessary to prevent hack/tree-status.sh false-positive
 /*runner_stats.log

libpod/options.go

@@ -268,7 +268,7 @@ func WithRegistriesConf(path string) RuntimeOption {
 	logrus.Debugf("Setting custom registries.conf: %q", path)
 	return func(rt *Runtime) error {
 		if _, err := os.Stat(path); err != nil {
-			return errors.Wrap(err, "error locating specified registries.conf")
+			return errors.Wrap(err, "locating specified registries.conf")
 		}
 		if rt.imageContext == nil {
 			rt.imageContext = &types.SystemContext{
@@ -1453,7 +1453,7 @@ func WithNamedVolumes(volumes []*ContainerNamedVolume) CtrCreateOption {
 		for _, vol := range volumes {
 			mountOpts, err := util.ProcessOptions(vol.Options, false, "")
 			if err != nil {
-				return errors.Wrapf(err, "error processing options for named volume %q mounted at %q", vol.Name, vol.Dest)
+				return errors.Wrapf(err, "processing options for named volume %q mounted at %q", vol.Name, vol.Dest)
 			}
 
 			ctr.config.NamedVolumes = append(ctr.config.NamedVolumes, &ContainerNamedVolume{

pkg/specgen/generate/storage.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/containers/common/libimage"
 	"github.com/containers/common/pkg/config"
+	"github.com/containers/common/pkg/parse"
 	"github.com/containers/podman/v3/libpod"
 	"github.com/containers/podman/v3/libpod/define"
 	"github.com/containers/podman/v3/pkg/specgen"
@@ -59,6 +60,9 @@ func finalizeMounts(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Ru
 	for _, m := range s.Mounts {
 		// Ensure that mount dest is clean, so that it can be
 		// compared against named volumes and avoid duplicate mounts.
+		if err = parse.ValidateVolumeCtrDir(m.Destination); err != nil {
+			return nil, nil, nil, err
+		}
 		cleanDestination := filepath.Clean(m.Destination)
 		if _, ok := unifiedMounts[cleanDestination]; ok {
 			return nil, nil, nil, errors.Wrapf(errDuplicateDest, "conflict in specified mounts - multiple mounts at %q", cleanDestination)
@@ -67,34 +71,54 @@ func finalizeMounts(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Ru
 	}
 
 	for _, m := range commonMounts {
-		if _, ok := unifiedMounts[m.Destination]; !ok {
+		if err = parse.ValidateVolumeCtrDir(m.Destination); err != nil {
-			unifiedMounts[m.Destination] = m
+			return nil, nil, nil, err
+		}
+		cleanDestination := filepath.Clean(m.Destination)
+		if _, ok := unifiedMounts[cleanDestination]; !ok {
+			unifiedMounts[cleanDestination] = m
 		}
 	}
 
 	for _, v := range s.Volumes {
-		if _, ok := unifiedVolumes[v.Dest]; ok {
+		if err = parse.ValidateVolumeCtrDir(v.Dest); err != nil {
-			return nil, nil, nil, errors.Wrapf(errDuplicateDest, "conflict in specified volumes - multiple volumes at %q", v.Dest)
+			return nil, nil, nil, err
 		}
-		unifiedVolumes[v.Dest] = v
+		cleanDestination := filepath.Clean(v.Dest)
+		if _, ok := unifiedVolumes[cleanDestination]; ok {
+			return nil, nil, nil, errors.Wrapf(errDuplicateDest, "conflict in specified volumes - multiple volumes at %q", cleanDestination)
+		}
+		unifiedVolumes[cleanDestination] = v
 	}
 
 	for _, v := range commonVolumes {
-		if _, ok := unifiedVolumes[v.Dest]; !ok {
+		if err = parse.ValidateVolumeCtrDir(v.Dest); err != nil {
-			unifiedVolumes[v.Dest] = v
+			return nil, nil, nil, err
+		}
+		cleanDestination := filepath.Clean(v.Dest)
+		if _, ok := unifiedVolumes[cleanDestination]; !ok {
+			unifiedVolumes[cleanDestination] = v
 		}
 	}
 
 	for _, v := range s.OverlayVolumes {
-		if _, ok := unifiedOverlays[v.Destination]; ok {
+		if err = parse.ValidateVolumeCtrDir(v.Destination); err != nil {
-			return nil, nil, nil, errors.Wrapf(errDuplicateDest, "conflict in specified volumes - multiple volumes at %q", v.Destination)
+			return nil, nil, nil, err
 		}
-		unifiedOverlays[v.Destination] = v
+		cleanDestination := filepath.Clean(v.Destination)
+		if _, ok := unifiedOverlays[cleanDestination]; ok {
+			return nil, nil, nil, errors.Wrapf(errDuplicateDest, "conflict in specified volumes - multiple volumes at %q", cleanDestination)
+		}
+		unifiedOverlays[cleanDestination] = v
 	}
 
 	for _, v := range commonOverlayVolumes {
-		if _, ok := unifiedOverlays[v.Destination]; ok {
+		if err = parse.ValidateVolumeCtrDir(v.Destination); err != nil {
-			unifiedOverlays[v.Destination] = v
+			return nil, nil, nil, err
+		}
+		cleanDestination := filepath.Clean(v.Destination)
+		if _, ok := unifiedOverlays[cleanDestination]; !ok {
+			unifiedOverlays[cleanDestination] = v
 		}
 	}
 
@@ -190,6 +214,9 @@ func getImageVolumes(ctx context.Context, img *libimage.Image, s *specgen.SpecGe
 	}
 	for volume := range inspect.Config.Volumes {
 		logrus.Debugf("Image has volume at %q", volume)
+		if err = parse.ValidateVolumeCtrDir(volume); err != nil {
+			return nil, nil, err
+		}
 		cleanDest := filepath.Clean(volume)
 		switch mode {
 		case "", "anonymous":
@@ -304,9 +331,13 @@ func getVolumesFrom(volumesFrom []string, runtime *libpod.Runtime) (map[string]s
 			if _, ok := finalMounts[namedVol.Dest]; ok {
 				logrus.Debugf("Overriding named volume mount to %s with new named volume from container %s", namedVol.Dest, ctr.ID())
 			}
+			if err = parse.ValidateVolumeCtrDir(namedVol.Dest); err != nil {
+				return nil, nil, err
+			}
 
+			cleanDest := filepath.Clean(namedVol.Dest)
 			newVol := new(specgen.NamedVolume)
-			newVol.Dest = namedVol.Dest
+			newVol.Dest = cleanDest
 			newVol.Options = namedVol.Options
 			newVol.Name = namedVol.Name
 

pkg/specgen/volumes.go

@@ -1,7 +1,6 @@
 package specgen
 
 import (
-	"path/filepath"
 	"strings"
 
 	"github.com/containers/common/pkg/parse"
@@ -93,11 +92,6 @@ func GenVolumeMounts(volumeFlag []string) (map[string]spec.Mount, map[string]*Na
 				return nil, nil, nil, errors.New("host directory cannot be empty")
 			}
 		}
-		if err := parse.ValidateVolumeCtrDir(dest); err != nil {
-			return nil, nil, nil, err
-		}
-
-		cleanDest := filepath.Clean(dest)
 
 		if strings.HasPrefix(src, "/") || strings.HasPrefix(src, ".") {
 			// This is not a named volume
@@ -120,7 +114,7 @@ func GenVolumeMounts(volumeFlag []string) (map[string]spec.Mount, map[string]*Na
 			if overlayFlag {
 				// This is a overlay volume
 				newOverlayVol := new(OverlayVolume)
-				newOverlayVol.Destination = cleanDest
+				newOverlayVol.Destination = dest
 				newOverlayVol.Source = src
 				newOverlayVol.Options = options
 
@@ -130,7 +124,7 @@ func GenVolumeMounts(volumeFlag []string) (map[string]spec.Mount, map[string]*Na
 				overlayVolumes[newOverlayVol.Destination] = newOverlayVol
 			} else {
 				newMount := spec.Mount{
-					Destination: cleanDest,
+					Destination: dest,
 					Type:        "bind",
 					Source:      src,
 					Options:     options,
@@ -144,7 +138,7 @@ func GenVolumeMounts(volumeFlag []string) (map[string]spec.Mount, map[string]*Na
 			// This is a named volume
 			newNamedVol := new(NamedVolume)
 			newNamedVol.Name = src
-			newNamedVol.Dest = cleanDest
+			newNamedVol.Dest = dest
 			newNamedVol.Options = options
 
 			if _, ok := volumes[newNamedVol.Dest]; ok {