diff --git a/libpod/container_api.go b/libpod/container_api.go index 7c95fb9fee..d38b955b57 100644 --- a/libpod/container_api.go +++ b/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 diff --git a/libpod/container_copy_common.go b/libpod/container_copy_common.go index e32f79dc09..9085cb86a6 100644 --- a/libpod/container_copy_common.go +++ b/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. diff --git a/libpod/container_copy_freebsd.go b/libpod/container_copy_freebsd.go index 9697222d31..60d15646a6 100644 --- a/libpod/container_copy_freebsd.go +++ b/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) } diff --git a/libpod/container_copy_linux.go b/libpod/container_copy_linux.go index 149da9b381..eda2857330 100644 --- a/libpod/container_copy_linux.go +++ b/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) } diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index 4f94c22665..5ee01bdd19 100644 --- a/libpod/container_internal_common.go +++ b/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) diff --git a/libpod/container_path_resolution.go b/libpod/container_path_resolution.go index 068cd284ea..03f8e5a9c0 100644 --- a/libpod/container_path_resolution.go +++ b/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 diff --git a/libpod/container_stat_common.go b/libpod/container_stat_common.go index 9ad7acb4ee..5109c3ee1e 100644 --- a/libpod/container_stat_common.go +++ b/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 }