Mount volumes before copying into a container

libpod/container_api.go

@@ -1133,7 +1133,7 @@ func (c *Container) ShouldRestart(ctx context.Context) bool {
 
 // CopyFromArchive copies the contents from the specified tarStream to path
 // *inside* the container.
-func (c *Container) CopyFromArchive(_ context.Context, containerPath string, chown, noOverwriteDirNonDir bool, rename map[string]string, tarStream io.Reader) (func() error, error) {
+func (c *Container) CopyFromArchive(ctx context.Context, containerPath string, chown, noOverwriteDirNonDir bool, rename map[string]string, tarStream io.Reader) (func() error, error) {
 	if !c.batched {
 		c.lock.Lock()
 		defer c.lock.Unlock()
@@ -1143,7 +1143,7 @@ func (c *Container) CopyFromArchive(_ context.Context, containerPath string, cho
 		}
 	}
 
-	return c.copyFromArchive(containerPath, chown, noOverwriteDirNonDir, rename, tarStream)
+	return c.copyFromArchive(ctx, containerPath, chown, noOverwriteDirNonDir, rename, tarStream)
 }
 
 // CopyToArchive copies the contents from the specified path *inside* the

libpod/container_copy_common.go

@@ -3,6 +3,7 @@
 package libpod
 
 import (
+	"context"
 	"errors"
 	"io"
 	"path/filepath"
@@ -19,12 +20,13 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
-func (c *Container) copyFromArchive(path string, chown, noOverwriteDirNonDir bool, rename map[string]string, reader io.Reader) (func() error, error) {
+func (c *Container) copyFromArchive(ctx context.Context, path string, chown, noOverwriteDirNonDir bool, rename map[string]string, reader io.Reader) (func() error, error) {
 	var (
 		mountPoint   string
 		resolvedRoot string
 		resolvedPath string
 		unmount      func()
+		cleanupFuncs []func()
 		err          error
 	)
 
@@ -44,19 +46,114 @@ func (c *Container) copyFromArchive(path string, chown, noOverwriteDirNonDir boo
 		if err != nil {
 			return nil, err
 		}
+		c.state.Mountpoint = mountPoint
+		if err := c.save(); err != nil {
+			return nil, err
+		}
+
 		unmount = func() {
+			// These have to be first, some of them rely on container rootfs still being mounted.
+			for _, cleanupFunc := range cleanupFuncs {
+				cleanupFunc()
+			}
 			if err := c.unmount(false); err != nil {
 				logrus.Errorf("Failed to unmount container: %v", err)
 			}
+			if c.ensureState(define.ContainerStateConfigured, define.ContainerStateExited) {
+				c.state.Mountpoint = ""
+				if err := c.save(); err != nil {
+					logrus.Errorf("Writing container %s state: %v", c.ID(), err)
+				}
+			}
+		}
+
+		// Before we proceed, mount all named volumes associated with the
+		// container.
+		// This solves two issues:
+		// Firstly, it ensures that if the volume actually requires a mount, we
+		// will mount it for safe use.
+		// (For example, image volumes, volume plugins).
+		// Secondly, it copies up into the volume if necessary.
+		// This ensures that permissions are correct for copies into volumes on
+		// containers that have never started.
+		if len(c.config.NamedVolumes) > 0 {
+			for _, v := range c.config.NamedVolumes {
+				vol, err := c.mountNamedVolume(v, mountPoint)
+				if err != nil {
+					unmount()
+					return nil, err
+				}
+
+				cleanupFuncs = append(cleanupFuncs, func() {
+					vol.lock.Lock()
+					if err := vol.unmount(false); err != nil {
+						logrus.Errorf("Unmounting volume %s after container %s copy: %v", vol.Name(), c.ID(), err)
+					}
+					vol.lock.Unlock()
+				})
+			}
+
+			// This is *ugly* but unavoidable.
+			// If we are not running, generate an OCI spec.
+			// Then use that to fix permissions on all the named volumes.
+			// Has to be *after* everything is mounted.
+			newSpec, cleanupFunc, err := c.generateSpec(ctx)
+			if err != nil {
+				unmount()
+				return nil, err
+			}
+			cleanupFuncs = append(cleanupFuncs, cleanupFunc)
+
+			if err := c.saveSpec(newSpec); err != nil {
+				unmount()
+				return nil, err
+			}
 		}
 	}
 
-	resolvedRoot, resolvedPath, err = c.resolveCopyTarget(mountPoint, path)
+	resolvedRoot, resolvedPath, volume, err := c.resolveCopyTarget(mountPoint, path)
 	if err != nil {
 		unmount()
 		return nil, err
 	}
 
+	if volume != nil {
+		// This must be the first cleanup function so it fires before volume unmounts happen.
+		cleanupFuncs = append([]func(){func() {
+			// This is a gross hack to ensure correct permissions
+			// on a volume that was copied into that needed, but did
+			// not receive, a copy-up.
+			volume.lock.Lock()
+
+			if err := volume.update(); err != nil {
+				logrus.Errorf("Unable to update volume %s status: %v", volume.Name(), err)
+				volume.lock.Unlock()
+				return
+			}
+
+			if volume.state.NeedsCopyUp && volume.state.NeedsChown {
+				volume.state.NeedsCopyUp = false
+				volume.state.CopiedUp = true
+				if err := volume.save(); err != nil {
+					logrus.Errorf("Unable to save volume %s state: %v", volume.Name(), err)
+					volume.lock.Unlock()
+					return
+				}
+
+				volume.lock.Unlock()
+
+				for _, namedVol := range c.config.NamedVolumes {
+					if namedVol.Name == volume.Name() {
+						if err := c.fixVolumePermissions(namedVol); err != nil {
+							logrus.Errorf("Unable to fix volume %s permissions: %v", volume.Name(), err)
+						}
+						return
+					}
+				}
+			}
+		}}, cleanupFuncs...)
+	}
+
 	var idPair *idtools.IDPair
 	if chown {
 		// Make sure we chown the files to the container's main user and group ID.

libpod/container_copy_freebsd.go

@@ -10,6 +10,6 @@ func (c *Container) joinMountAndExec(f func() error) error {
 
 // Similarly, we can just use resolvePath for both running and stopped
 // containers.
-func (c *Container) resolveCopyTarget(mountPoint string, containerPath string) (string, string, error) {
+func (c *Container) resolveCopyTarget(mountPoint string, containerPath string) (string, string, *Volume, error) {
 	return c.resolvePath(mountPoint, containerPath)
 }

libpod/container_copy_linux.go

@@ -79,12 +79,12 @@ func (c *Container) joinMountAndExec(f func() error) error {
 	return <-errChan
 }
 
-func (c *Container) resolveCopyTarget(mountPoint string, containerPath string) (string, string, error) {
+func (c *Container) resolveCopyTarget(mountPoint string, containerPath string) (string, string, *Volume, error) {
 	// If the container is running, we will execute the copy
 	// inside the container's mount namespace so we return a path
 	// relative to the container's root.
 	if c.state.State == define.ContainerStateRunning {
-		return "/", c.pathAbs(containerPath), nil
+		return "/", c.pathAbs(containerPath), nil, nil
 	}
 	return c.resolvePath(mountPoint, containerPath)
 }

libpod/container_internal_common.go

@@ -741,7 +741,7 @@ func (c *Container) isWorkDirSymlink(resolvedPath string) bool {
 			break
 		}
 		if resolvedSymlink != "" {
-			_, resolvedSymlinkWorkdir, err := c.resolvePath(c.state.Mountpoint, resolvedSymlink)
+			_, resolvedSymlinkWorkdir, _, err := c.resolvePath(c.state.Mountpoint, resolvedSymlink)
 			if isPathOnVolume(c, resolvedSymlinkWorkdir) || isPathOnMount(c, resolvedSymlinkWorkdir) {
 				// Resolved symlink exists on external volume or mount
 				return true
@@ -780,7 +780,7 @@ func (c *Container) resolveWorkDir() error {
 		return nil
 	}
 
-	_, resolvedWorkdir, err := c.resolvePath(c.state.Mountpoint, workdir)
+	_, resolvedWorkdir, _, err := c.resolvePath(c.state.Mountpoint, workdir)
 	if err != nil {
 		return err
 	}
@@ -2963,7 +2963,11 @@ func (c *Container) fixVolumePermissions(v *ContainerNamedVolume) error {
 			return nil
 		}
 
-		st, err := os.Lstat(filepath.Join(c.state.Mountpoint, v.Dest))
+		finalPath, err := securejoin.SecureJoin(c.state.Mountpoint, v.Dest)
+		if err != nil {
+			return err
+		}
+		st, err := os.Lstat(finalPath)
 		if err == nil {
 			if stat, ok := st.Sys().(*syscall.Stat_t); ok {
 				uid, gid := int(stat.Uid), int(stat.Gid)

libpod/container_path_resolution.go

@@ -33,8 +33,8 @@ func (c *Container) pathAbs(path string) string {
 // It returns a bool, indicating whether containerPath resolves outside of
 // mountPoint (e.g., via a mount or volume), the resolved root (e.g., container
 // mount, bind mount or volume) and the resolved path on the root (absolute to
-// the host).
-func (c *Container) resolvePath(mountPoint string, containerPath string) (string, string, error) {
+// the host). If the path is on a named volume, the volume is returned.
+func (c *Container) resolvePath(mountPoint string, containerPath string) (string, string, *Volume, error) {
 	// Let's first make sure we have a path relative to the mount point.
 	pathRelativeToContainerMountPoint := c.pathAbs(containerPath)
 	resolvedPathOnTheContainerMountPoint := filepath.Join(mountPoint, pathRelativeToContainerMountPoint)
@@ -54,21 +54,17 @@ func (c *Container) resolvePath(mountPoint string, containerPath string) (string
 	for {
 		volume, err := findVolume(c, searchPath)
 		if err != nil {
-			return "", "", err
+			return "", "", nil, err
 		}
 		if volume != nil {
 			logrus.Debugf("Container path %q resolved to volume %q on path %q", containerPath, volume.Name(), searchPath)
 
-			// TODO: We really need to force the volume to mount
-			// before doing this, but that API is not exposed
-			// externally right now and doing so is beyond the scope
-			// of this commit.
 			mountPoint, err := volume.MountPoint()
 			if err != nil {
-				return "", "", err
+				return "", "", nil, err
 			}
 			if mountPoint == "" {
-				return "", "", fmt.Errorf("volume %s is not mounted, cannot copy into it", volume.Name())
+				return "", "", nil, fmt.Errorf("volume %s is not mounted, cannot copy into it", volume.Name())
 			}
 
 			// We found a matching volume for searchPath.  We now
@@ -78,9 +74,9 @@ func (c *Container) resolvePath(mountPoint string, containerPath string) (string
 			pathRelativeToVolume := strings.TrimPrefix(pathRelativeToContainerMountPoint, searchPath)
 			absolutePathOnTheVolumeMount, err := securejoin.SecureJoin(mountPoint, pathRelativeToVolume)
 			if err != nil {
-				return "", "", err
+				return "", "", nil, err
 			}
-			return mountPoint, absolutePathOnTheVolumeMount, nil
+			return mountPoint, absolutePathOnTheVolumeMount, volume, nil
 		}
 
 		if mount := findBindMount(c, searchPath); mount != nil {
@@ -92,9 +88,9 @@ func (c *Container) resolvePath(mountPoint string, containerPath string) (string
 			pathRelativeToBindMount := strings.TrimPrefix(pathRelativeToContainerMountPoint, searchPath)
 			absolutePathOnTheBindMount, err := securejoin.SecureJoin(mount.Source, pathRelativeToBindMount)
 			if err != nil {
-				return "", "", err
+				return "", "", nil, err
 			}
-			return mount.Source, absolutePathOnTheBindMount, nil
+			return mount.Source, absolutePathOnTheBindMount, nil, nil
 		}
 
 		if searchPath == "/" {
@@ -106,7 +102,7 @@ func (c *Container) resolvePath(mountPoint string, containerPath string) (string
 	}
 
 	// No volume, no bind mount but just a normal path on the container.
-	return mountPoint, resolvedPathOnTheContainerMountPoint, nil
+	return mountPoint, resolvedPathOnTheContainerMountPoint, nil, nil
 }
 
 // findVolume checks if the specified containerPath matches the destination

libpod/container_stat_common.go

@@ -21,7 +21,7 @@ import (
 func (c *Container) statOnHost(mountPoint string, containerPath string) (*copier.StatForItem, string, string, error) {
 	// Now resolve the container's path.  It may hit a volume, it may hit a
 	// bind mount, it may be relative.
-	resolvedRoot, resolvedPath, err := c.resolvePath(mountPoint, containerPath)
+	resolvedRoot, resolvedPath, _, err := c.resolvePath(mountPoint, containerPath)
 	if err != nil {
 		return nil, "", "", err
 	}