Mount volumes before copying into a container

This solves several problems with copying into volumes on a
container that is not running.

The first, and most obvious, is that we were previously entirely
unable to copy into a volume that required mounting - like
image volumes, volume plugins, and volumes that specified mount
options.

The second is that this fixed several permissions and content
issues with a fresh volume and a container that has not been run
before. A copy-up will not have occurred, so permissions on the
volume root will not have been set and content will not have been
copied into the volume.

If the container is running, this is very low cost - we maintain
a mount counter for named volumes, so it's just an increment in
the DB if the volume actually needs mounting, and a no-op if it
doesn't.

Unfortunately, we also have to fix permissions, and that is
rather more complicated. We need the final OCI spec (as we need
final UID/GID user namespace mappings), we need to do the chown
after the copy has occurred, and we need to do some ugly manual
changes to volume copyup/chown fields to make sure the copy
sticks. It's really ugly, but necessary to reach full Docker
compatibility.

Signed-off-by: Matthew Heon <[email protected]>

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
 	}