LCOW: Implemented support for docker cp + build

This enables docker cp and ADD/COPY docker build support for LCOW.
Originally, the graphdriver.Get() interface returned a local path
to the container root filesystem. This does not work for LCOW, so
the Get() method now returns an interface that LCOW implements to
support copying to and from the container.

Signed-off-by: Akash Gupta <akagup@microsoft.com>
Upstream-commit: 7a7357dae1bcccb17e9b2d4c7c8f5c025fce56ca
Component: engine

components/engine/pkg/archive/archive.go

@@ -55,18 +55,17 @@ type (
 	}
 )
 
-// Archiver allows the reuse of most utility functions of this package
-// with a pluggable Untar function. Also, to facilitate the passing of
-// specific id mappings for untar, an archiver can be created with maps
-// which will then be passed to Untar operations
+// Archiver implements the Archiver interface and allows the reuse of most utility functions of
+// this package with a pluggable Untar function. Also, to facilitate the passing of specific id
+// mappings for untar, an Archiver can be created with maps which will then be passed to Untar operations.
 type Archiver struct {
-	Untar      func(io.Reader, string, *TarOptions) error
-	IDMappings *idtools.IDMappings
+	Untar         func(io.Reader, string, *TarOptions) error
+	IDMappingsVar *idtools.IDMappings
 }
 
 // NewDefaultArchiver returns a new Archiver without any IDMappings
 func NewDefaultArchiver() *Archiver {
-	return &Archiver{Untar: Untar, IDMappings: &idtools.IDMappings{}}
+	return &Archiver{Untar: Untar, IDMappingsVar: &idtools.IDMappings{}}
 }
 
 // breakoutError is used to differentiate errors related to breaking out
@@ -1025,8 +1024,8 @@ func (archiver *Archiver) TarUntar(src, dst string) error {
 	}
 	defer archive.Close()
 	options := &TarOptions{
-		UIDMaps: archiver.IDMappings.UIDs(),
-		GIDMaps: archiver.IDMappings.GIDs(),
+		UIDMaps: archiver.IDMappingsVar.UIDs(),
+		GIDMaps: archiver.IDMappingsVar.GIDs(),
 	}
 	return archiver.Untar(archive, dst, options)
 }
@@ -1039,8 +1038,8 @@ func (archiver *Archiver) UntarPath(src, dst string) error {
 	}
 	defer archive.Close()
 	options := &TarOptions{
-		UIDMaps: archiver.IDMappings.UIDs(),
-		GIDMaps: archiver.IDMappings.GIDs(),
+		UIDMaps: archiver.IDMappingsVar.UIDs(),
+		GIDMaps: archiver.IDMappingsVar.GIDs(),
 	}
 	return archiver.Untar(archive, dst, options)
 }
@@ -1058,10 +1057,10 @@ func (archiver *Archiver) CopyWithTar(src, dst string) error {
 		return archiver.CopyFileWithTar(src, dst)
 	}
 
-	// if this archiver is set up with ID mapping we need to create
+	// if this Archiver is set up with ID mapping we need to create
 	// the new destination directory with the remapped root UID/GID pair
 	// as owner
-	rootIDs := archiver.IDMappings.RootPair()
+	rootIDs := archiver.IDMappingsVar.RootPair()
 	// Create dst, copy src's content into it
 	logrus.Debugf("Creating dest directory: %s", dst)
 	if err := idtools.MkdirAllAndChownNew(dst, 0755, rootIDs); err != nil {
@@ -1112,7 +1111,7 @@ func (archiver *Archiver) CopyFileWithTar(src, dst string) (err error) {
 		hdr.Name = filepath.Base(dst)
 		hdr.Mode = int64(chmodTarEntry(os.FileMode(hdr.Mode)))
 
-		if err := remapIDs(archiver.IDMappings, hdr); err != nil {
+		if err := remapIDs(archiver.IDMappingsVar, hdr); err != nil {
 			return err
 		}
 
@@ -1139,6 +1138,11 @@ func (archiver *Archiver) CopyFileWithTar(src, dst string) (err error) {
 	return err
 }
 
+// IDMappings returns the IDMappings of the archiver.
+func (archiver *Archiver) IDMappings() *idtools.IDMappings {
+	return archiver.IDMappingsVar
+}
+
 func remapIDs(idMappings *idtools.IDMappings, hdr *tar.Header) error {
 	ids, err := idMappings.ToHost(idtools.IDPair{UID: hdr.Uid, GID: hdr.Gid})
 	hdr.Uid, hdr.Gid = ids.UID, ids.GID

components/engine/pkg/archive/copy.go

@@ -27,23 +27,23 @@ var (
 // path (from before being processed by utility functions from the path or
 // filepath stdlib packages) ends with a trailing `/.` or `/`. If the cleaned
 // path already ends in a `.` path segment, then another is not added. If the
-// clean path already ends in a path separator, then another is not added.
-func PreserveTrailingDotOrSeparator(cleanedPath, originalPath string) string {
+// clean path already ends in the separator, then another is not added.
+func PreserveTrailingDotOrSeparator(cleanedPath string, originalPath string, sep byte) string {
 	// Ensure paths are in platform semantics
-	cleanedPath = normalizePath(cleanedPath)
-	originalPath = normalizePath(originalPath)
+	cleanedPath = strings.Replace(cleanedPath, "/", string(sep), -1)
+	originalPath = strings.Replace(originalPath, "/", string(sep), -1)
 
 	if !specifiesCurrentDir(cleanedPath) && specifiesCurrentDir(originalPath) {
-		if !hasTrailingPathSeparator(cleanedPath) {
+		if !hasTrailingPathSeparator(cleanedPath, sep) {
 			// Add a separator if it doesn't already end with one (a cleaned
 			// path would only end in a separator if it is the root).
-			cleanedPath += string(filepath.Separator)
+			cleanedPath += string(sep)
 		}
 		cleanedPath += "."
 	}
 
-	if !hasTrailingPathSeparator(cleanedPath) && hasTrailingPathSeparator(originalPath) {
-		cleanedPath += string(filepath.Separator)
+	if !hasTrailingPathSeparator(cleanedPath, sep) && hasTrailingPathSeparator(originalPath, sep) {
+		cleanedPath += string(sep)
 	}
 
 	return cleanedPath
@@ -52,14 +52,14 @@ func PreserveTrailingDotOrSeparator(cleanedPath, originalPath string) string {
 // assertsDirectory returns whether the given path is
 // asserted to be a directory, i.e., the path ends with
 // a trailing '/' or `/.`, assuming a path separator of `/`.
-func assertsDirectory(path string) bool {
-	return hasTrailingPathSeparator(path) || specifiesCurrentDir(path)
+func assertsDirectory(path string, sep byte) bool {
+	return hasTrailingPathSeparator(path, sep) || specifiesCurrentDir(path)
 }
 
 // hasTrailingPathSeparator returns whether the given
 // path ends with the system's path separator character.
-func hasTrailingPathSeparator(path string) bool {
-	return len(path) > 0 && os.IsPathSeparator(path[len(path)-1])
+func hasTrailingPathSeparator(path string, sep byte) bool {
+	return len(path) > 0 && path[len(path)-1] == sep
 }
 
 // specifiesCurrentDir returns whether the given path specifies
@@ -72,10 +72,10 @@ func specifiesCurrentDir(path string) bool {
 // basename by first cleaning the path but preserves a trailing "." if the
 // original path specified the current directory.
 func SplitPathDirEntry(path string) (dir, base string) {
-	cleanedPath := filepath.Clean(normalizePath(path))
+	cleanedPath := filepath.Clean(filepath.FromSlash(path))
 
 	if specifiesCurrentDir(path) {
-		cleanedPath += string(filepath.Separator) + "."
+		cleanedPath += string(os.PathSeparator) + "."
 	}
 
 	return filepath.Dir(cleanedPath), filepath.Base(cleanedPath)
@@ -106,19 +106,24 @@ func TarResourceRebase(sourcePath, rebaseName string) (content io.ReadCloser, er
 	// Separate the source path between its directory and
 	// the entry in that directory which we are archiving.
 	sourceDir, sourceBase := SplitPathDirEntry(sourcePath)
-
-	filter := []string{sourceBase}
+	opts := TarResourceRebaseOpts(sourceBase, rebaseName)
 
 	logrus.Debugf("copying %q from %q", sourceBase, sourceDir)
+	return TarWithOptions(sourceDir, opts)
+}
 
-	return TarWithOptions(sourceDir, &TarOptions{
+// TarResourceRebaseOpts does not preform the Tar, but instead just creates the rebase
+// parameters to be sent to TarWithOptions (the TarOptions struct)
+func TarResourceRebaseOpts(sourceBase string, rebaseName string) *TarOptions {
+	filter := []string{sourceBase}
+	return &TarOptions{
 		Compression:      Uncompressed,
 		IncludeFiles:     filter,
 		IncludeSourceDir: true,
 		RebaseNames: map[string]string{
 			sourceBase: rebaseName,
 		},
-	})
+	}
 }
 
 // CopyInfo holds basic info about the source
@@ -281,7 +286,7 @@ func PrepareArchiveCopy(srcContent io.Reader, srcInfo, dstInfo CopyInfo) (dstDir
 			srcBase = srcInfo.RebaseName
 		}
 		return dstDir, RebaseArchiveEntries(srcContent, srcBase, dstBase), nil
-	case assertsDirectory(dstInfo.Path):
+	case assertsDirectory(dstInfo.Path, os.PathSeparator):
 		// The destination does not exist and is asserted to be created as a
 		// directory, but the source content is not a directory. This is an
 		// error condition since you cannot create a directory from a file
@@ -351,6 +356,9 @@ func RebaseArchiveEntries(srcContent io.Reader, oldBase, newBase string) io.Read
 	return rebased
 }
 
+// TODO @gupta-ak. These might have to be changed in the future to be
+// continuity driver aware as well to support LCOW.
+
 // CopyResource performs an archive copy from the given source path to the
 // given destination path. The source path MUST exist and the destination
 // path's parent directory must exist.
@@ -365,8 +373,8 @@ func CopyResource(srcPath, dstPath string, followLink bool) error {
 	dstPath = normalizePath(dstPath)
 
 	// Clean the source and destination paths.
-	srcPath = PreserveTrailingDotOrSeparator(filepath.Clean(srcPath), srcPath)
-	dstPath = PreserveTrailingDotOrSeparator(filepath.Clean(dstPath), dstPath)
+	srcPath = PreserveTrailingDotOrSeparator(filepath.Clean(srcPath), srcPath, os.PathSeparator)
+	dstPath = PreserveTrailingDotOrSeparator(filepath.Clean(dstPath), dstPath, os.PathSeparator)
 
 	if srcInfo, err = CopyInfoSourcePath(srcPath, followLink); err != nil {
 		return err
@@ -429,7 +437,8 @@ func ResolveHostSourcePath(path string, followLink bool) (resolvedPath, rebaseNa
 		// resolvedDirPath will have been cleaned (no trailing path separators) so
 		// we can manually join it with the base path element.
 		resolvedPath = resolvedDirPath + string(filepath.Separator) + basePath
-		if hasTrailingPathSeparator(path) && filepath.Base(path) != filepath.Base(resolvedPath) {
+		if hasTrailingPathSeparator(path, os.PathSeparator) &&
+			filepath.Base(path) != filepath.Base(resolvedPath) {
 			rebaseName = filepath.Base(path)
 		}
 	}
@@ -442,11 +451,13 @@ func GetRebaseName(path, resolvedPath string) (string, string) {
 	// linkTarget will have been cleaned (no trailing path separators and dot) so
 	// we can manually join it with them
 	var rebaseName string
-	if specifiesCurrentDir(path) && !specifiesCurrentDir(resolvedPath) {
+	if specifiesCurrentDir(path) &&
+		!specifiesCurrentDir(resolvedPath) {
 		resolvedPath += string(filepath.Separator) + "."
 	}
 
-	if hasTrailingPathSeparator(path) && !hasTrailingPathSeparator(resolvedPath) {
+	if hasTrailingPathSeparator(path, os.PathSeparator) &&
+		!hasTrailingPathSeparator(resolvedPath, os.PathSeparator) {
 		resolvedPath += string(filepath.Separator)
 	}
 

components/engine/pkg/chrootarchive/archive.go

@@ -16,7 +16,10 @@ func NewArchiver(idMappings *idtools.IDMappings) *archive.Archiver {
 	if idMappings == nil {
 		idMappings = &idtools.IDMappings{}
 	}
-	return &archive.Archiver{Untar: Untar, IDMappings: idMappings}
+	return &archive.Archiver{
+		Untar:         Untar,
+		IDMappingsVar: idMappings,
+	}
 }
 
 // Untar reads a stream of bytes from `archive`, parses it as a tar archive,

components/engine/pkg/containerfs/archiver.go

@@ -0,0 +1,194 @@
+package containerfs
+
+import (
+	"archive/tar"
+	"fmt"
+	"io"
+	"os"
+	"path/filepath"
+
+	"github.com/docker/docker/pkg/archive"
+	"github.com/docker/docker/pkg/idtools"
+	"github.com/docker/docker/pkg/promise"
+	"github.com/docker/docker/pkg/system"
+	"github.com/sirupsen/logrus"
+)
+
+// TarFunc provides a function definition for a custom Tar function
+type TarFunc func(string, *archive.TarOptions) (io.ReadCloser, error)
+
+// UntarFunc provides a function definition for a custom Untar function
+type UntarFunc func(io.Reader, string, *archive.TarOptions) error
+
+// Archiver provides a similar implementation of the archive.Archiver package with the rootfs abstraction
+type Archiver struct {
+	SrcDriver     Driver
+	DstDriver     Driver
+	Tar           TarFunc
+	Untar         UntarFunc
+	IDMappingsVar *idtools.IDMappings
+}
+
+// TarUntar is a convenience function which calls Tar and Untar, with the output of one piped into the other.
+// If either Tar or Untar fails, TarUntar aborts and returns the error.
+func (archiver *Archiver) TarUntar(src, dst string) error {
+	logrus.Debugf("TarUntar(%s %s)", src, dst)
+	tarArchive, err := archiver.Tar(src, &archive.TarOptions{Compression: archive.Uncompressed})
+	if err != nil {
+		return err
+	}
+	defer tarArchive.Close()
+	options := &archive.TarOptions{
+		UIDMaps: archiver.IDMappingsVar.UIDs(),
+		GIDMaps: archiver.IDMappingsVar.GIDs(),
+	}
+	return archiver.Untar(tarArchive, dst, options)
+}
+
+// UntarPath untar a file from path to a destination, src is the source tar file path.
+func (archiver *Archiver) UntarPath(src, dst string) error {
+	tarArchive, err := archiver.SrcDriver.Open(src)
+	if err != nil {
+		return err
+	}
+	defer tarArchive.Close()
+	options := &archive.TarOptions{
+		UIDMaps: archiver.IDMappingsVar.UIDs(),
+		GIDMaps: archiver.IDMappingsVar.GIDs(),
+	}
+	return archiver.Untar(tarArchive, dst, options)
+}
+
+// CopyWithTar creates a tar archive of filesystem path `src`, and
+// unpacks it at filesystem path `dst`.
+// The archive is streamed directly with fixed buffering and no
+// intermediary disk IO.
+func (archiver *Archiver) CopyWithTar(src, dst string) error {
+	srcSt, err := archiver.SrcDriver.Stat(src)
+	if err != nil {
+		return err
+	}
+	if !srcSt.IsDir() {
+		return archiver.CopyFileWithTar(src, dst)
+	}
+
+	// if this archiver is set up with ID mapping we need to create
+	// the new destination directory with the remapped root UID/GID pair
+	// as owner
+	rootIDs := archiver.IDMappingsVar.RootPair()
+	// Create dst, copy src's content into it
+	if err := idtools.MkdirAllAndChownNew(dst, 0755, rootIDs); err != nil {
+		return err
+	}
+	logrus.Debugf("Calling TarUntar(%s, %s)", src, dst)
+	return archiver.TarUntar(src, dst)
+}
+
+// CopyFileWithTar emulates the behavior of the 'cp' command-line
+// for a single file. It copies a regular file from path `src` to
+// path `dst`, and preserves all its metadata.
+func (archiver *Archiver) CopyFileWithTar(src, dst string) (err error) {
+	logrus.Debugf("CopyFileWithTar(%s, %s)", src, dst)
+	srcDriver := archiver.SrcDriver
+	dstDriver := archiver.DstDriver
+
+	srcSt, err := srcDriver.Stat(src)
+	if err != nil {
+		return err
+	}
+
+	if srcSt.IsDir() {
+		return fmt.Errorf("Can't copy a directory")
+	}
+
+	// Clean up the trailing slash. This must be done in an operating
+	// system specific manner.
+	if dst[len(dst)-1] == dstDriver.Separator() {
+		dst = dstDriver.Join(dst, srcDriver.Base(src))
+	}
+
+	// The original call was system.MkdirAll, which is just
+	// os.MkdirAll on not-Windows and changed for Windows.
+	if dstDriver.OS() == "windows" {
+		// Now we are WCOW
+		if err := system.MkdirAll(filepath.Dir(dst), 0700, ""); err != nil {
+			return err
+		}
+	} else {
+		// We can just use the driver.MkdirAll function
+		if err := dstDriver.MkdirAll(dstDriver.Dir(dst), 0700); err != nil {
+			return err
+		}
+	}
+
+	r, w := io.Pipe()
+	errC := promise.Go(func() error {
+		defer w.Close()
+
+		srcF, err := srcDriver.Open(src)
+		if err != nil {
+			return err
+		}
+		defer srcF.Close()
+
+		hdr, err := tar.FileInfoHeader(srcSt, "")
+		if err != nil {
+			return err
+		}
+		hdr.Name = dstDriver.Base(dst)
+		if dstDriver.OS() == "windows" {
+			hdr.Mode = int64(chmodTarEntry(os.FileMode(hdr.Mode)))
+		} else {
+			hdr.Mode = int64(os.FileMode(hdr.Mode))
+		}
+
+		if err := remapIDs(archiver.IDMappingsVar, hdr); err != nil {
+			return err
+		}
+
+		tw := tar.NewWriter(w)
+		defer tw.Close()
+		if err := tw.WriteHeader(hdr); err != nil {
+			return err
+		}
+		if _, err := io.Copy(tw, srcF); err != nil {
+			return err
+		}
+		return nil
+	})
+	defer func() {
+		if er := <-errC; err == nil && er != nil {
+			err = er
+		}
+	}()
+
+	err = archiver.Untar(r, dstDriver.Dir(dst), nil)
+	if err != nil {
+		r.CloseWithError(err)
+	}
+	return err
+}
+
+// IDMappings returns the IDMappings of the archiver.
+func (archiver *Archiver) IDMappings() *idtools.IDMappings {
+	return archiver.IDMappingsVar
+}
+
+func remapIDs(idMappings *idtools.IDMappings, hdr *tar.Header) error {
+	ids, err := idMappings.ToHost(idtools.IDPair{UID: hdr.Uid, GID: hdr.Gid})
+	hdr.Uid, hdr.Gid = ids.UID, ids.GID
+	return err
+}
+
+// chmodTarEntry is used to adjust the file permissions used in tar header based
+// on the platform the archival is done.
+func chmodTarEntry(perm os.FileMode) os.FileMode {
+	//perm &= 0755 // this 0-ed out tar flags (like link, regular file, directory marker etc.)
+	permPart := perm & os.ModePerm
+	noPermPart := perm &^ os.ModePerm
+	// Add the x bit: make everything +x from windows
+	permPart |= 0111
+	permPart &= 0755
+
+	return noPermPart | permPart
+}

components/engine/pkg/containerfs/containerfs.go

@@ -0,0 +1,87 @@
+package containerfs
+
+import (
+	"path/filepath"
+	"runtime"
+
+	"github.com/containerd/continuity/driver"
+	"github.com/containerd/continuity/pathdriver"
+	"github.com/docker/docker/pkg/symlink"
+)
+
+// ContainerFS is that represents a root file system
+type ContainerFS interface {
+	// Path returns the path to the root. Note that this may not exist
+	// on the local system, so the continuity operations must be used
+	Path() string
+
+	// ResolveScopedPath evaluates the given path scoped to the root.
+	// For example, if root=/a, and path=/b/c, then this function would return /a/b/c.
+	// If rawPath is true, then the function will not preform any modifications
+	// before path resolution. Otherwise, the function will clean the given path
+	// by making it an absolute path.
+	ResolveScopedPath(path string, rawPath bool) (string, error)
+
+	Driver
+}
+
+// Driver combines both continuity's Driver and PathDriver interfaces with a Platform
+// field to determine the OS.
+type Driver interface {
+	// OS returns the OS where the rootfs is located. Essentially,
+	// runtime.GOOS for everything aside from LCOW, which is "linux"
+	OS() string
+
+	// Architecture returns the hardware architecture where the
+	// container is located.
+	Architecture() string
+
+	// Driver & PathDriver provide methods to manipulate files & paths
+	driver.Driver
+	pathdriver.PathDriver
+}
+
+// NewLocalContainerFS is a helper function to implement daemon's Mount interface
+// when the graphdriver mount point is a local path on the machine.
+func NewLocalContainerFS(path string) ContainerFS {
+	return &local{
+		path:       path,
+		Driver:     driver.LocalDriver,
+		PathDriver: pathdriver.LocalPathDriver,
+	}
+}
+
+// NewLocalDriver provides file and path drivers for a local file system. They are
+// essentially a wrapper around the `os` and `filepath` functions.
+func NewLocalDriver() Driver {
+	return &local{
+		Driver:     driver.LocalDriver,
+		PathDriver: pathdriver.LocalPathDriver,
+	}
+}
+
+type local struct {
+	path string
+	driver.Driver
+	pathdriver.PathDriver
+}
+
+func (l *local) Path() string {
+	return l.path
+}
+
+func (l *local) ResolveScopedPath(path string, rawPath bool) (string, error) {
+	cleanedPath := path
+	if !rawPath {
+		cleanedPath = cleanScopedPath(path)
+	}
+	return symlink.FollowSymlinkInScope(filepath.Join(l.path, cleanedPath), l.path)
+}
+
+func (l *local) OS() string {
+	return runtime.GOOS
+}
+
+func (l *local) Architecture() string {
+	return runtime.GOARCH
+}

components/engine/pkg/containerfs/containerfs_unix.go

@@ -0,0 +1,10 @@
+// +build !windows
+
+package containerfs
+
+import "path/filepath"
+
+// cleanScopedPath preappends a to combine with a mnt path.
+func cleanScopedPath(path string) string {
+	return filepath.Join(string(filepath.Separator), path)
+}

components/engine/pkg/containerfs/containerfs_windows.go

@@ -0,0 +1,15 @@
+package containerfs
+
+import "path/filepath"
+
+// cleanScopedPath removes the C:\ syntax, and prepares to combine
+// with a volume path
+func cleanScopedPath(path string) string {
+	if len(path) >= 2 {
+		c := path[0]
+		if path[1] == ':' && ('a' <= c && c <= 'z' || 'A' <= c && c <= 'Z') {
+			path = path[2:]
+		}
+	}
+	return filepath.Join(string(filepath.Separator), path)
+}

components/engine/pkg/system/path.go

@@ -1,6 +1,13 @@
 package system
 
-import "runtime"
+import (
+	"fmt"
+	"path/filepath"
+	"runtime"
+	"strings"
+
+	"github.com/containerd/continuity/pathdriver"
+)
 
 const defaultUnixPathEnv = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
 
@@ -19,3 +26,35 @@ func DefaultPathEnv(platform string) string {
 	return defaultUnixPathEnv
 
 }
+
+// CheckSystemDriveAndRemoveDriveLetter verifies that a path, if it includes a drive letter,
+// is the system drive.
+// On Linux: this is a no-op.
+// On Windows: this does the following>
+// CheckSystemDriveAndRemoveDriveLetter verifies and manipulates a Windows path.
+// This is used, for example, when validating a user provided path in docker cp.
+// If a drive letter is supplied, it must be the system drive. The drive letter
+// is always removed. Also, it translates it to OS semantics (IOW / to \). We
+// need the path in this syntax so that it can ultimately be contatenated with
+// a Windows long-path which doesn't support drive-letters. Examples:
+// C:			--> Fail
+// C:\			--> \
+// a			--> a
+// /a			--> \a
+// d:\			--> Fail
+func CheckSystemDriveAndRemoveDriveLetter(path string, driver pathdriver.PathDriver) (string, error) {
+	if runtime.GOOS != "windows" || LCOWSupported() {
+		return path, nil
+	}
+
+	if len(path) == 2 && string(path[1]) == ":" {
+		return "", fmt.Errorf("No relative path specified in %q", path)
+	}
+	if !driver.IsAbs(path) || len(path) < 2 {
+		return filepath.FromSlash(path), nil
+	}
+	if string(path[1]) == ":" && !strings.EqualFold(string(path[0]), "c") {
+		return "", fmt.Errorf("The specified path is not on the system drive (C:)")
+	}
+	return filepath.FromSlash(path[2:]), nil
+}

components/engine/pkg/system/path_unix.go

@@ -1,9 +0,0 @@
-// +build !windows
-
-package system
-
-// CheckSystemDriveAndRemoveDriveLetter verifies that a path, if it includes a drive letter,
-// is the system drive. This is a no-op on Linux.
-func CheckSystemDriveAndRemoveDriveLetter(path string) (string, error) {
-	return path, nil
-}

components/engine/pkg/system/path_windows.go

@@ -1,33 +0,0 @@
-// +build windows
-
-package system
-
-import (
-	"fmt"
-	"path/filepath"
-	"strings"
-)
-
-// CheckSystemDriveAndRemoveDriveLetter verifies and manipulates a Windows path.
-// This is used, for example, when validating a user provided path in docker cp.
-// If a drive letter is supplied, it must be the system drive. The drive letter
-// is always removed. Also, it translates it to OS semantics (IOW / to \). We
-// need the path in this syntax so that it can ultimately be concatenated with
-// a Windows long-path which doesn't support drive-letters. Examples:
-// C:			--> Fail
-// C:\			--> \
-// a			--> a
-// /a			--> \a
-// d:\			--> Fail
-func CheckSystemDriveAndRemoveDriveLetter(path string) (string, error) {
-	if len(path) == 2 && string(path[1]) == ":" {
-		return "", fmt.Errorf("No relative path specified in %q", path)
-	}
-	if !filepath.IsAbs(path) || len(path) < 2 {
-		return filepath.FromSlash(path), nil
-	}
-	if string(path[1]) == ":" && !strings.EqualFold(string(path[0]), "c") {
-		return "", fmt.Errorf("The specified path is not on the system drive (C:)")
-	}
-	return filepath.FromSlash(path[2:]), nil
-}

components/engine/pkg/system/path_windows_test.go

@@ -2,19 +2,23 @@
 
 package system
 
-import "testing"
+import (
+	"testing"
+
+	"github.com/containerd/continuity/pathdriver"
+)
 
 // TestCheckSystemDriveAndRemoveDriveLetter tests CheckSystemDriveAndRemoveDriveLetter
 func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) {
 	// Fails if not C drive.
-	_, err := CheckSystemDriveAndRemoveDriveLetter(`d:\`)
+	_, err := CheckSystemDriveAndRemoveDriveLetter(`d:\`, pathdriver.LocalPathDriver)
 	if err == nil || (err != nil && err.Error() != "The specified path is not on the system drive (C:)") {
 		t.Fatalf("Expected error for d:")
 	}
 
 	// Single character is unchanged
 	var path string
-	if path, err = CheckSystemDriveAndRemoveDriveLetter("z"); err != nil {
+	if path, err = CheckSystemDriveAndRemoveDriveLetter("z", pathdriver.LocalPathDriver); err != nil {
 		t.Fatalf("Single character should pass")
 	}
 	if path != "z" {
@@ -22,7 +26,7 @@ func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) {
 	}
 
 	// Two characters without colon is unchanged
-	if path, err = CheckSystemDriveAndRemoveDriveLetter("AB"); err != nil {
+	if path, err = CheckSystemDriveAndRemoveDriveLetter("AB", pathdriver.LocalPathDriver); err != nil {
 		t.Fatalf("2 characters without colon should pass")
 	}
 	if path != "AB" {
@@ -30,7 +34,7 @@ func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) {
 	}
 
 	// Abs path without drive letter
-	if path, err = CheckSystemDriveAndRemoveDriveLetter(`\l`); err != nil {
+	if path, err = CheckSystemDriveAndRemoveDriveLetter(`\l`, pathdriver.LocalPathDriver); err != nil {
 		t.Fatalf("abs path no drive letter should pass")
 	}
 	if path != `\l` {
@@ -38,7 +42,7 @@ func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) {
 	}
 
 	// Abs path without drive letter, linux style
-	if path, err = CheckSystemDriveAndRemoveDriveLetter(`/l`); err != nil {
+	if path, err = CheckSystemDriveAndRemoveDriveLetter(`/l`, pathdriver.LocalPathDriver); err != nil {
 		t.Fatalf("abs path no drive letter linux style should pass")
 	}
 	if path != `\l` {
@@ -46,7 +50,7 @@ func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) {
 	}
 
 	// Drive-colon should be stripped
-	if path, err = CheckSystemDriveAndRemoveDriveLetter(`c:\`); err != nil {
+	if path, err = CheckSystemDriveAndRemoveDriveLetter(`c:\`, pathdriver.LocalPathDriver); err != nil {
 		t.Fatalf("An absolute path should pass")
 	}
 	if path != `\` {
@@ -54,7 +58,7 @@ func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) {
 	}
 
 	// Verify with a linux-style path
-	if path, err = CheckSystemDriveAndRemoveDriveLetter(`c:/`); err != nil {
+	if path, err = CheckSystemDriveAndRemoveDriveLetter(`c:/`, pathdriver.LocalPathDriver); err != nil {
 		t.Fatalf("An absolute path should pass")
 	}
 	if path != `\` {
@@ -62,7 +66,7 @@ func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) {
 	}
 
 	// Failure on c:
-	if path, err = CheckSystemDriveAndRemoveDriveLetter(`c:`); err == nil {
+	if path, err = CheckSystemDriveAndRemoveDriveLetter(`c:`, pathdriver.LocalPathDriver); err == nil {
 		t.Fatalf("c: should fail")
 	}
 	if err.Error() != `No relative path specified in "c:"` {
@@ -70,7 +74,7 @@ func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) {
 	}
 
 	// Failure on d:
-	if path, err = CheckSystemDriveAndRemoveDriveLetter(`d:`); err == nil {
+	if path, err = CheckSystemDriveAndRemoveDriveLetter(`d:`, pathdriver.LocalPathDriver); err == nil {
 		t.Fatalf("c: should fail")
 	}
 	if err.Error() != `No relative path specified in "d:"` {