Merge pull request #42684 from thaJeztah/remove_lcow_step7

Remove LCOW (step 7): remove LCOW bits from builder/dockerfile (copy)
This commit is contained in:
Brian Goff 2021-08-05 15:16:24 -07:00 committed by GitHub
commit 6a60efc39b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 115 additions and 143 deletions

View File

@ -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{<GUID>}\<path>. 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{<GUID>}\<path>. 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

View File

@ -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
}

View File

@ -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
}

View File

@ -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 '/'
}

View File

@ -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