diff --git a/builder/dockerfile/copy.go b/builder/dockerfile/copy.go index 39abe76896..6d8b19beb1 100644 --- a/builder/dockerfile/copy.go +++ b/builder/dockerfile/copy.go @@ -109,22 +109,15 @@ func copierFromDispatchRequest(req dispatchRequest, download sourceDownloader, i } func (o *copier) createCopyInstruction(sourcesAndDest instructions.SourcesAndDest, cmdName string) (copyInstruction, error) { - inst := copyInstruction{cmdName: cmdName} - - // Work in platform-specific filepath semantics - // TODO: This OS switch for paths is NOT correct and should not be supported. - // Maintained for backwards compatibility - pathOS := runtime.GOOS - if o.platform != nil { - pathOS = o.platform.OS + inst := copyInstruction{ + cmdName: cmdName, + dest: filepath.FromSlash(sourcesAndDest.DestPath), } - inst.dest = fromSlash(sourcesAndDest.DestPath, pathOS) - separator := string(separator(pathOS)) infos, err := o.getCopyInfosForSourcePaths(sourcesAndDest.SourcePaths, inst.dest) if err != nil { return inst, errors.Wrapf(err, "%s failed", cmdName) } - if len(infos) > 1 && !strings.HasSuffix(inst.dest, separator) { + if len(infos) > 1 && !strings.HasSuffix(inst.dest, string(os.PathSeparator)) { return inst, errors.Errorf("When using %s with more than one source file, the destination must be a directory and end with a /", cmdName) } inst.infos = infos @@ -191,6 +184,9 @@ func (o *copier) Cleanup() { // TODO: allowWildcards can probably be removed by refactoring this function further. func (o *copier) calcCopyInfo(origPath string, allowWildcards bool) ([]copyInfo, error) { imageSource := o.imageSource + if err := validateCopySourcePath(imageSource, origPath); err != nil { + return nil, err + } // TODO: do this when creating copier. Requires validateCopySourcePath // (and other below) to be aware of the difference sources. Why is it only @@ -215,20 +211,13 @@ func (o *copier) calcCopyInfo(origPath string, allowWildcards bool) ([]copyInfo, return nil, errors.Errorf("missing build context") } - root := o.source.Root() - - if err := validateCopySourcePath(imageSource, origPath, root.OS()); err != nil { - return nil, err - } - - // Work in source OS specific filepath semantics - // For LCOW, this is NOT the daemon OS. - origPath = root.FromSlash(origPath) - origPath = strings.TrimPrefix(origPath, string(root.Separator())) - origPath = strings.TrimPrefix(origPath, "."+string(root.Separator())) + // Work in daemon-specific OS filepath semantics + origPath = filepath.FromSlash(origPath) + origPath = strings.TrimPrefix(origPath, string(os.PathSeparator)) + origPath = strings.TrimPrefix(origPath, "."+string(os.PathSeparator)) // Deal with wildcards - if allowWildcards && containsWildcards(origPath, root.OS()) { + if allowWildcards && containsWildcards(origPath) { return o.copyWithWildcards(origPath) } @@ -262,19 +251,6 @@ func (o *copier) calcCopyInfo(origPath string, allowWildcards bool) ([]copyInfo, return newCopyInfos(newCopyInfoFromSource(o.source, origPath, hash)), nil } -func containsWildcards(name, platform string) bool { - isWindows := platform == "windows" - for i := 0; i < len(name); i++ { - ch := name[i] - if ch == '\\' && !isWindows { - i++ - } else if ch == '*' || ch == '?' || ch == '[' { - return true - } - } - return false -} - func (o *copier) storeInPathCache(im *imageMount, path string, hash string) { if im != nil { o.pathCache.Store(im.ImageID()+path, hash) @@ -549,33 +525,23 @@ func copyDirectory(archiver Archiver, source, dest *copyEndpoint, identity *idto return errors.Wrapf(err, "failed to copy directory") } if identity != nil { - // TODO: @gupta-ak. Investigate how LCOW permission mappings will work. return fixPermissions(source.path, dest.path, *identity, !destExists) } return nil } func copyFile(archiver Archiver, source, dest *copyEndpoint, identity *idtools.Identity) error { - if runtime.GOOS == "windows" && dest.driver.OS() == "linux" { - // LCOW - if err := dest.driver.MkdirAll(dest.driver.Dir(dest.path), 0755); err != nil { - return errors.Wrapf(err, "failed to create new directory") + if identity == nil { + // Use system.MkdirAll here, which is a custom version of os.MkdirAll + // modified for use on Windows to handle volume GUID paths. These paths + // are of the form \\?\Volume{}\. An example would be: + // \\?\Volume{dae8d3ac-b9a1-11e9-88eb-e8554b2ba1db}\bin\busybox.exe + if err := system.MkdirAll(filepath.Dir(dest.path), 0755); err != nil { + return err } } else { - // Normal containers - if identity == nil { - // Use system.MkdirAll here, which is a custom version of os.MkdirAll - // modified for use on Windows to handle volume GUID paths. These paths - // are of the form \\?\Volume{}\. An example would be: - // \\?\Volume{dae8d3ac-b9a1-11e9-88eb-e8554b2ba1db}\bin\busybox.exe - - if err := system.MkdirAll(filepath.Dir(dest.path), 0755); err != nil { - return err - } - } else { - if err := idtools.MkdirAllAndChownNew(filepath.Dir(dest.path), 0755, *identity); err != nil { - return errors.Wrapf(err, "failed to create new directory") - } + if err := idtools.MkdirAllAndChownNew(filepath.Dir(dest.path), 0755, *identity); err != nil { + return errors.Wrapf(err, "failed to create new directory") } } @@ -583,7 +549,6 @@ func copyFile(archiver Archiver, source, dest *copyEndpoint, identity *idtools.I return errors.Wrapf(err, "failed to copy file") } if identity != nil { - // TODO: @gupta-ak. Investigate how LCOW permission mappings will work. return fixPermissions(source.path, dest.path, *identity, false) } return nil diff --git a/builder/dockerfile/copy_unix.go b/builder/dockerfile/copy_unix.go index d2a16e0220..6cc85e53f3 100644 --- a/builder/dockerfile/copy_unix.go +++ b/builder/dockerfile/copy_unix.go @@ -4,7 +4,9 @@ package dockerfile // import "github.com/docker/docker/builder/dockerfile" import ( "os" + "path" "path/filepath" + "strings" "github.com/docker/docker/pkg/containerfs" "github.com/docker/docker/pkg/idtools" @@ -43,6 +45,34 @@ func fixPermissions(source, destination string, identity idtools.Identity, overr }) } -func validateCopySourcePath(imageSource *imageMount, origPath, platform string) error { +// normalizeDest normalises the destination of a COPY/ADD command in a +// platform semantically consistent way. +func normalizeDest(workingDir, requested string) (string, error) { + dest := filepath.FromSlash(requested) + endsInSlash := strings.HasSuffix(dest, string(os.PathSeparator)) + + if !path.IsAbs(requested) { + dest = path.Join("/", filepath.ToSlash(workingDir), dest) + // Make sure we preserve any trailing slash + if endsInSlash { + dest += "/" + } + } + return dest, nil +} + +func containsWildcards(name string) bool { + for i := 0; i < len(name); i++ { + ch := name[i] + if ch == '\\' { + i++ + } else if ch == '*' || ch == '?' || ch == '[' { + return true + } + } + return false +} + +func validateCopySourcePath(imageSource *imageMount, origPath string) error { return nil } diff --git a/builder/dockerfile/copy_windows.go b/builder/dockerfile/copy_windows.go index 83640ebf42..1a3a488516 100644 --- a/builder/dockerfile/copy_windows.go +++ b/builder/dockerfile/copy_windows.go @@ -79,12 +79,66 @@ func fixPermissionsWindows(source, destination, SID string) error { return windows.SetNamedSecurityInfo(destination, windows.SE_FILE_OBJECT, windows.OWNER_SECURITY_INFORMATION|windows.DACL_SECURITY_INFORMATION, sid, nil, dacl, nil) } -func validateCopySourcePath(imageSource *imageMount, origPath, platform string) error { - // validate windows paths from other images + LCOW - if imageSource == nil || platform != "windows" { - return nil +// normalizeDest normalises the destination of a COPY/ADD command in a +// platform semantically consistent way. +func normalizeDest(workingDir, requested string) (string, error) { + dest := filepath.FromSlash(requested) + endsInSlash := strings.HasSuffix(dest, string(os.PathSeparator)) + + // We are guaranteed that the working directory is already consistent, + // However, Windows also has, for now, the limitation that ADD/COPY can + // only be done to the system drive, not any drives that might be present + // as a result of a bind mount. + // + // So... if the path requested is Linux-style absolute (/foo or \\foo), + // we assume it is the system drive. If it is a Windows-style absolute + // (DRIVE:\\foo), error if DRIVE is not C. And finally, ensure we + // strip any configured working directories drive letter so that it + // can be subsequently legitimately converted to a Windows volume-style + // pathname. + + // Not a typo - filepath.IsAbs, not system.IsAbs on this next check as + // we only want to validate where the DriveColon part has been supplied. + if filepath.IsAbs(dest) { + if strings.ToUpper(string(dest[0])) != "C" { + return "", fmt.Errorf("Windows does not support destinations not on the system drive (C:)") + } + dest = dest[2:] // Strip the drive letter } + // Cannot handle relative where WorkingDir is not the system drive. + if len(workingDir) > 0 { + if ((len(workingDir) > 1) && !system.IsAbs(workingDir[2:])) || (len(workingDir) == 1) { + return "", fmt.Errorf("Current WorkingDir %s is not platform consistent", workingDir) + } + if !system.IsAbs(dest) { + if string(workingDir[0]) != "C" { + return "", fmt.Errorf("Windows does not support relative paths when WORKDIR is not the system drive") + } + dest = filepath.Join(string(os.PathSeparator), workingDir[2:], dest) + // Make sure we preserve any trailing slash + if endsInSlash { + dest += string(os.PathSeparator) + } + } + } + return dest, nil +} + +func containsWildcards(name string) bool { + for i := 0; i < len(name); i++ { + ch := name[i] + if ch == '*' || ch == '?' || ch == '[' { + return true + } + } + return false +} + +func validateCopySourcePath(imageSource *imageMount, origPath string) error { + if imageSource == nil { + return nil + } origPath = filepath.FromSlash(origPath) p := strings.ToLower(filepath.Clean(origPath)) if !filepath.IsAbs(p) { @@ -92,13 +146,13 @@ func validateCopySourcePath(imageSource *imageMount, origPath, platform string) if p[len(p)-2:] == ":." { // case where clean returns weird c:. paths p = p[:len(p)-1] } - p += "\\" + p += `\` } else { - p = filepath.Join("c:\\", p) + p = filepath.Join(`c:\`, p) } } if _, ok := pathDenyList[p]; ok { - return errors.New("copy from c:\\ or c:\\windows is not allowed on windows") + return errors.New(`copy from c:\ or c:\windows is not allowed on windows`) } return nil } diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index 77b51d35d9..0bc9d0e9ef 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -8,9 +8,6 @@ import ( "encoding/hex" "fmt" "io" - "os" - "path" - "path/filepath" "strings" "github.com/docker/docker/api/types" @@ -23,7 +20,6 @@ import ( "github.com/docker/docker/pkg/containerfs" "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/stringid" - "github.com/docker/docker/pkg/system" "github.com/docker/go-connections/nat" specs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" @@ -224,7 +220,7 @@ func (b *Builder) performCopy(req dispatchRequest, inst copyInstruction) error { func createDestInfo(workingDir string, inst copyInstruction, rwLayer builder.RWLayer, platform string) (copyInfo, error) { // Twiddle the destination when it's a relative path - meaning, make it // relative to the WORKINGDIR - dest, err := normalizeDest(workingDir, inst.dest, platform) + dest, err := normalizeDest(workingDir, inst.dest) if err != nil { return copyInfo{}, errors.Wrapf(err, "invalid %s", inst.cmdName) } @@ -232,63 +228,6 @@ func createDestInfo(workingDir string, inst copyInstruction, rwLayer builder.RWL return copyInfo{root: rwLayer.Root(), path: dest}, nil } -// normalizeDest normalises the destination of a COPY/ADD command in a -// platform semantically consistent way. -func normalizeDest(workingDir, requested string, platform string) (string, error) { - dest := fromSlash(requested, platform) - endsInSlash := strings.HasSuffix(dest, string(separator(platform))) - - if platform != "windows" { - if !path.IsAbs(requested) { - dest = path.Join("/", filepath.ToSlash(workingDir), dest) - // Make sure we preserve any trailing slash - if endsInSlash { - dest += "/" - } - } - return dest, nil - } - - // We are guaranteed that the working directory is already consistent, - // However, Windows also has, for now, the limitation that ADD/COPY can - // only be done to the system drive, not any drives that might be present - // as a result of a bind mount. - // - // So... if the path requested is Linux-style absolute (/foo or \\foo), - // we assume it is the system drive. If it is a Windows-style absolute - // (DRIVE:\\foo), error if DRIVE is not C. And finally, ensure we - // strip any configured working directories drive letter so that it - // can be subsequently legitimately converted to a Windows volume-style - // pathname. - - // Not a typo - filepath.IsAbs, not system.IsAbs on this next check as - // we only want to validate where the DriveColon part has been supplied. - if filepath.IsAbs(dest) { - if strings.ToUpper(string(dest[0])) != "C" { - return "", fmt.Errorf("Windows does not support destinations not on the system drive (C:)") - } - dest = dest[2:] // Strip the drive letter - } - - // Cannot handle relative where WorkingDir is not the system drive. - if len(workingDir) > 0 { - if ((len(workingDir) > 1) && !system.IsAbs(workingDir[2:])) || (len(workingDir) == 1) { - return "", fmt.Errorf("Current WorkingDir %s is not platform consistent", workingDir) - } - if !system.IsAbs(dest) { - if string(workingDir[0]) != "C" { - return "", fmt.Errorf("Windows does not support relative paths when WORKDIR is not the system drive") - } - dest = filepath.Join(string(os.PathSeparator), workingDir[2:], dest) - // Make sure we preserve any trailing slash - if endsInSlash { - dest += string(os.PathSeparator) - } - } - } - return dest, nil -} - // For backwards compat, if there's just one info then use it as the // cache look-up string, otherwise hash 'em all into one func getSourceHashFromInfos(infos []copyInfo) string { @@ -485,19 +424,3 @@ func hostConfigFromOptions(options *types.ImageBuildOptions) *container.HostConf } return hc } - -// fromSlash works like filepath.FromSlash but with a given OS platform field -func fromSlash(path, platform string) string { - if platform == "windows" { - return strings.Replace(path, "/", "\\", -1) - } - return path -} - -// separator returns a OS path separator for the given OS platform -func separator(platform string) byte { - if platform == "windows" { - return '\\' - } - return '/' -} diff --git a/builder/dockerfile/internals_windows_test.go b/builder/dockerfile/internals_windows_test.go index 1694ee8ac0..a05e1c6899 100644 --- a/builder/dockerfile/internals_windows_test.go +++ b/builder/dockerfile/internals_windows_test.go @@ -40,7 +40,7 @@ func TestNormalizeDest(t *testing.T) { } for _, testcase := range tests { msg := fmt.Sprintf("Input: %s, %s", testcase.current, testcase.requested) - actual, err := normalizeDest(testcase.current, testcase.requested, "windows") + actual, err := normalizeDest(testcase.current, testcase.requested) if testcase.etext == "" { if !assert.Check(t, err, msg) { continue