diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index 4fcd8abe43..facf0f6310 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -9,8 +9,6 @@ package dockerfile import ( "fmt" - "os" - "path/filepath" "regexp" "runtime" "sort" @@ -20,7 +18,6 @@ import ( "github.com/docker/docker/api" "github.com/docker/docker/builder" "github.com/docker/docker/pkg/signal" - "github.com/docker/docker/pkg/system" runconfigopts "github.com/docker/docker/runconfig/opts" "github.com/docker/engine-api/types/container" "github.com/docker/engine-api/types/strslice" @@ -257,45 +254,17 @@ func workdir(b *Builder, args []string, attributes map[string]bool, original str return errExactlyOneArgument("WORKDIR") } - if err := b.flags.Parse(); err != nil { + err := b.flags.Parse() + if err != nil { return err } // This is from the Dockerfile and will not necessarily be in platform // specific semantics, hence ensure it is converted. - workdir := filepath.FromSlash(args[0]) - current := filepath.FromSlash(b.runConfig.WorkingDir) - if runtime.GOOS == "windows" { - // Windows is a little more complicated than Linux. This code ensures - // we end up with a workdir which is consistent in terms of platform - // semantics. This means C:\somefolder, specifically in the format: - // UPPERCASEDriveLetter-Colon-Backslash-FolderName. We are already - // guaranteed that `current`, if set, is consistent. This allows us to - // cope correctly with any of the following in a Dockerfile: - // WORKDIR a --> C:\a - // WORKDIR c:\\foo --> C:\foo - // WORKDIR \\foo --> C:\foo - // WORKDIR /foo --> C:\foo - // WORKDIR c:\\foo \ WORKDIR bar --> C:\foo --> C:\foo\bar - // WORKDIR C:/foo \ WORKDIR bar --> C:\foo --> C:\foo\bar - // WORKDIR C:/foo \ WORKDIR \\bar --> C:\foo --> C:\bar - // WORKDIR /foo \ WORKDIR c:/bar --> C:\foo --> C:\bar - if len(current) == 0 || system.IsAbs(workdir) { - if (workdir[0] == os.PathSeparator) || - (len(workdir) > 1 && string(workdir[1]) != ":") || - (len(workdir) == 1) { - workdir = filepath.Join(`C:\`, workdir) - } - } else { - workdir = filepath.Join(current, workdir) - } - workdir = strings.ToUpper(string(workdir[0])) + workdir[1:] // Upper-case drive letter - } else { - if !filepath.IsAbs(workdir) { - workdir = filepath.Join(string(os.PathSeparator), current, workdir) - } + b.runConfig.WorkingDir, err = normaliseWorkdir(b.runConfig.WorkingDir, args[0]) + if err != nil { + return err } - b.runConfig.WorkingDir = workdir return b.commit("", b.runConfig.Cmd, fmt.Sprintf("WORKDIR %v", workdir)) } diff --git a/builder/dockerfile/dispatchers_unix.go b/builder/dockerfile/dispatchers_unix.go new file mode 100644 index 0000000000..e1b9fde0bb --- /dev/null +++ b/builder/dockerfile/dispatchers_unix.go @@ -0,0 +1,23 @@ +// +build !windows + +package dockerfile + +import ( + "fmt" + "os" + "path/filepath" +) + +// normaliseWorkdir normalises a user requested working directory in a +// platform sematically consistent way. +func normaliseWorkdir(current string, requested string) (string, error) { + if requested == "" { + return "", fmt.Errorf("cannot normalise nothing") + } + current = filepath.FromSlash(current) + requested = filepath.FromSlash(requested) + if !filepath.IsAbs(requested) { + return filepath.Join(string(os.PathSeparator), current, requested), nil + } + return requested, nil +} diff --git a/builder/dockerfile/dispatchers_windows.go b/builder/dockerfile/dispatchers_windows.go new file mode 100644 index 0000000000..7a102c8552 --- /dev/null +++ b/builder/dockerfile/dispatchers_windows.go @@ -0,0 +1,45 @@ +package dockerfile + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/docker/docker/pkg/system" +) + +// normaliseWorkdir normalises a user requested working directory in a +// platform sematically consistent way. +func normaliseWorkdir(current string, requested string) (string, error) { + if requested == "" { + return "", fmt.Errorf("cannot normalise nothing") + } + + current = filepath.FromSlash(current) + requested = filepath.FromSlash(requested) + + // Target semantics is C:\somefolder, specifically in the format: + // UPPERCASEDriveLetter-Colon-Backslash-FolderName. We are already + // guaranteed that `current`, if set, is consistent. This allows us to + // cope correctly with any of the following in a Dockerfile: + // WORKDIR a --> C:\a + // WORKDIR c:\\foo --> C:\foo + // WORKDIR \\foo --> C:\foo + // WORKDIR /foo --> C:\foo + // WORKDIR c:\\foo \ WORKDIR bar --> C:\foo --> C:\foo\bar + // WORKDIR C:/foo \ WORKDIR bar --> C:\foo --> C:\foo\bar + // WORKDIR C:/foo \ WORKDIR \\bar --> C:\foo --> C:\bar + // WORKDIR /foo \ WORKDIR c:/bar --> C:\foo --> C:\bar + if len(current) == 0 || system.IsAbs(requested) { + if (requested[0] == os.PathSeparator) || + (len(requested) > 1 && string(requested[1]) != ":") || + (len(requested) == 1) { + requested = filepath.Join(`C:\`, requested) + } + } else { + requested = filepath.Join(current, requested) + } + // Upper-case drive letter + return (strings.ToUpper(string(requested[0])) + requested[1:]), nil +} diff --git a/builder/dockerfile/dispatchers_windows_test.go b/builder/dockerfile/dispatchers_windows_test.go new file mode 100644 index 0000000000..4c53713197 --- /dev/null +++ b/builder/dockerfile/dispatchers_windows_test.go @@ -0,0 +1,34 @@ +// +build windows + +package dockerfile + +import "testing" + +func TestNormaliseWorkdir(t *testing.T) { + tests := []struct{ current, requested, expected, etext string }{ + {``, ``, ``, `cannot normalise nothing`}, + {``, `a`, `C:\a`, ``}, + {``, `c:\foo`, `C:\foo`, ``}, + {``, `\foo`, `C:\foo`, ``}, + {``, `/foo`, `C:\foo`, ``}, + {``, `C:/foo`, `C:\foo`, ``}, + {`C:\foo`, `bar`, `C:\foo\bar`, ``}, + {`C:\foo`, `/bar`, `C:\bar`, ``}, + {`C:\foo`, `\bar`, `C:\bar`, ``}, + } + for _, i := range tests { + r, e := normaliseWorkdir(i.current, i.requested) + + if i.etext != "" && e == nil { + t.Fatalf("TestNormaliseWorkingDir Expected error %s", i.etext) + } + + if i.etext != "" && e.Error() != i.etext { + t.Fatalf("TestNormaliseWorkingDir Expected error %s, got %s", i.etext, e.Error()) + } + + if r != i.expected { + t.Fatalf("TestNormaliseWorkingDir Expected %s for %s %s", i.expected, i.current, i.requested) + } + } +} diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index 676930f0d6..6cfb5374de 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -200,60 +200,8 @@ func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalD // Twiddle the destination when its a relative path - meaning, make it // relative to the WORKINGDIR - - endsInSlash := strings.HasSuffix(dest, string(os.PathSeparator)) - - if runtime.GOOS == "windows" { - // On Windows, this is more complicated. We are guaranteed that the - // WorkingDir is already platform consistent meaning in the format - // UPPERCASEDriveLetter-Colon-Backslash-Foldername. However, Windows - // for now also has the limitation that ADD/COPY can only be done to - // the C: (system) drive, not any drives that might be present as a - // result of bind mounts. - // - // So... if the path specified 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 %s with a destinations not on the system drive (C:)", cmdName) - } - dest = dest[2:] // Strip the drive letter - } - - // Cannot handle relative where WorkingDir is not the system drive. - if len(b.runConfig.WorkingDir) > 0 { - if !system.IsAbs(b.runConfig.WorkingDir[2:]) { - return fmt.Errorf("Current WorkingDir %s is not platform consistent", b.runConfig.WorkingDir) - } - if !system.IsAbs(dest) { - if string(b.runConfig.WorkingDir[0]) != "C" { - return fmt.Errorf("Windows does not support %s with relative paths when WORKDIR is not the system drive", cmdName) - } - - dest = filepath.Join(string(os.PathSeparator), b.runConfig.WorkingDir[2:], dest) - - // Make sure we preserve any trailing slash - if endsInSlash { - dest += string(os.PathSeparator) - } - } - } - } else { - if !system.IsAbs(dest) { - dest = filepath.Join(string(os.PathSeparator), filepath.FromSlash(b.runConfig.WorkingDir), dest) - - // Make sure we preserve any trailing slash - if endsInSlash { - dest += string(os.PathSeparator) - } - } + if dest, err = normaliseDest(cmdName, b.runConfig.WorkingDir, dest); err != nil { + return err } for _, info := range infos { diff --git a/builder/dockerfile/internals_unix.go b/builder/dockerfile/internals_unix.go new file mode 100644 index 0000000000..6cd990d892 --- /dev/null +++ b/builder/dockerfile/internals_unix.go @@ -0,0 +1,26 @@ +// +build !windows + +package dockerfile + +import ( + "os" + "path/filepath" + "strings" + + "github.com/docker/docker/pkg/system" +) + +// normaliseDest normalises the destination of a COPY/ADD command in a +// platform semantically consistent way. +func normaliseDest(cmdName, workingDir, requested string) (string, error) { + dest := filepath.FromSlash(requested) + endsInSlash := strings.HasSuffix(requested, string(os.PathSeparator)) + if !system.IsAbs(requested) { + dest = filepath.Join(string(os.PathSeparator), filepath.FromSlash(workingDir), dest) + // Make sure we preserve any trailing slash + if endsInSlash { + dest += string(os.PathSeparator) + } + } + return dest, nil +} diff --git a/builder/dockerfile/internals_windows.go b/builder/dockerfile/internals_windows.go new file mode 100644 index 0000000000..f70360300d --- /dev/null +++ b/builder/dockerfile/internals_windows.go @@ -0,0 +1,56 @@ +package dockerfile + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/docker/docker/pkg/system" +) + +// normaliseDest normalises the destination of a COPY/ADD command in a +// platform semantically consistent way. +func normaliseDest(cmdName, 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 %s with a destinations not on the system drive (C:)", cmdName) + } + 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 %s with relative paths when WORKDIR is not the system drive", cmdName) + } + 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 +} diff --git a/builder/dockerfile/internals_windows_test.go b/builder/dockerfile/internals_windows_test.go new file mode 100644 index 0000000000..868a6671a3 --- /dev/null +++ b/builder/dockerfile/internals_windows_test.go @@ -0,0 +1,51 @@ +// +build windows + +package dockerfile + +import "testing" + +func TestNormaliseDest(t *testing.T) { + tests := []struct{ current, requested, expected, etext string }{ + {``, `D:\`, ``, `Windows does not support TEST with a destinations not on the system drive (C:)`}, + {``, `e:/`, ``, `Windows does not support TEST with a destinations not on the system drive (C:)`}, + {`invalid`, `./c1`, ``, `Current WorkingDir invalid is not platform consistent`}, + {`C:`, ``, ``, `Current WorkingDir C: is not platform consistent`}, + {`C`, ``, ``, `Current WorkingDir C is not platform consistent`}, + {`D:\`, `.`, ``, "Windows does not support TEST with relative paths when WORKDIR is not the system drive"}, + {``, `D`, `D`, ``}, + {``, `./a1`, `.\a1`, ``}, + {``, `.\b1`, `.\b1`, ``}, + {``, `/`, `\`, ``}, + {``, `\`, `\`, ``}, + {``, `c:/`, `\`, ``}, + {``, `c:\`, `\`, ``}, + {``, `.`, `.`, ``}, + {`C:\wdd`, `./a1`, `\wdd\a1`, ``}, + {`C:\wde`, `.\b1`, `\wde\b1`, ``}, + {`C:\wdf`, `/`, `\`, ``}, + {`C:\wdg`, `\`, `\`, ``}, + {`C:\wdh`, `c:/`, `\`, ``}, + {`C:\wdi`, `c:\`, `\`, ``}, + {`C:\wdj`, `.`, `\wdj`, ``}, + {`C:\wdk`, `foo/bar`, `\wdk\foo\bar`, ``}, + {`C:\wdl`, `foo\bar`, `\wdl\foo\bar`, ``}, + {`C:\wdm`, `foo/bar/`, `\wdm\foo\bar\`, ``}, + {`C:\wdn`, `foo\bar/`, `\wdn\foo\bar\`, ``}, + } + for _, i := range tests { + got, err := normaliseDest("TEST", i.current, i.requested) + if err != nil && i.etext == "" { + t.Fatalf("TestNormaliseDest Got unexpected error %q for %s %s. ", err.Error(), i.current, i.requested) + } + if i.etext != "" && ((err == nil) || (err != nil && err.Error() != i.etext)) { + if err == nil { + t.Fatalf("TestNormaliseDest Expected an error for %s %s but didn't get one", i.current, i.requested) + } else { + t.Fatalf("TestNormaliseDest Wrong error text for %s %s - %s", i.current, i.requested, err.Error()) + } + } + if i.etext == "" && got != i.expected { + t.Fatalf("TestNormaliseDest Expected %q for %q and %q. Got %q", i.expected, i.current, i.requested, got) + } + } +} diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index 9d1834256a..f48366c19f 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -2089,33 +2089,16 @@ func (s *DockerSuite) TestBuildRelativeWorkdir(c *check.C) { } } -// #22181 Regression test. Validates combinations of supported -// WORKDIR dockerfile directives in Windows and non-Windows semantics. +// #22181 Regression test. Single end-to-end test of using +// Windows semantics. Most path handling verifications are in unit tests func (s *DockerSuite) TestBuildWindowsWorkdirProcessing(c *check.C) { testRequires(c, DaemonIsWindows) name := "testbuildwindowsworkdirprocessing" _, err := buildImage(name, `FROM busybox - WORKDIR a - RUN sh -c "[ "$PWD" = "C:/a" ]" - WORKDIR c:\\foo - RUN sh -c "[ "$PWD" = "C:/foo" ]" - WORKDIR \\foo - RUN sh -c "[ "$PWD" = "C:/foo" ]" - WORKDIR /foo - RUN sh -c "[ "$PWD" = "C:/foo" ]" - WORKDIR C:/foo + WORKDIR C:\\foo WORKDIR bar RUN sh -c "[ "$PWD" = "C:/foo/bar" ]" - WORKDIR c:/foo - WORKDIR bar - RUN sh -c "[ "$PWD" = "C:/foo/bar" ]" - WORKDIR c:/foo - WORKDIR \\bar - RUN sh -c "[ "$PWD" = "C:/bar" ]" - WORKDIR /foo - WORKDIR c:\\bar - RUN sh -c "[ "$PWD" = "C:/bar" ]" `, true) if err != nil { @@ -2123,8 +2106,8 @@ func (s *DockerSuite) TestBuildWindowsWorkdirProcessing(c *check.C) { } } -// #22181 Regression test. Validates combinations of supported -// COPY dockerfile directives in Windows and non-Windows semantics. +// #22181 Regression test. Most paths handling verifications are in unit test. +// One functional test for end-to-end func (s *DockerSuite) TestBuildWindowsAddCopyPathProcessing(c *check.C) { testRequires(c, DaemonIsWindows) name := "testbuildwindowsaddcopypathprocessing" @@ -2134,50 +2117,7 @@ func (s *DockerSuite) TestBuildWindowsAddCopyPathProcessing(c *check.C) { // by docker on the Windows platform. dockerfile := ` FROM busybox - # First cases with no workdir, all end up in the root directory of the system drive - COPY a1 ./ - ADD a2 ./ - RUN sh -c "[ $(cat c:/a1) = 'helloa1' ]" - RUN sh -c "[ $(cat c:/a2) = 'worlda2' ]" - - COPY b1 / - ADD b2 / - RUN sh -c "[ $(cat c:/b1) = 'hellob1' ]" - RUN sh -c "[ $(cat c:/b2) = 'worldb2' ]" - - COPY c1 c:/ - ADD c2 c:/ - RUN sh -c "[ $(cat c:/c1) = 'helloc1' ]" - RUN sh -c "[ $(cat c:/c2) = 'worldc2' ]" - - COPY d1 c:/ - ADD d2 c:/ - RUN sh -c "[ $(cat c:/d1) = 'hellod1' ]" - RUN sh -c "[ $(cat c:/d2) = 'worldd2' ]" - - COPY e1 . - ADD e2 . - RUN sh -c "[ $(cat c:/e1) = 'helloe1' ]" - RUN sh -c "[ $(cat c:/e2) = 'worlde2' ]" - - # Now with a workdir - WORKDIR c:\\wa12 - COPY wa1 ./ - ADD wa2 ./ - RUN sh -c "[ $(cat c:/wa12/wa1) = 'hellowa1' ]" - RUN sh -c "[ $(cat c:/wa12/wa2) = 'worldwa2' ]" - - # No trailing slash on COPY/ADD, Linux-style path. - # Results in dir being changed to a file - WORKDIR /wb1 - COPY wb1 . - WORKDIR /wb2 - ADD wb2 . - WORKDIR c:/ - RUN sh -c "[ $(cat c:/wb1) = 'hellowb1' ]" - RUN sh -c "[ $(cat c:/wb2) = 'worldwb2' ]" - - # No trailing slash on COPY/ADD, Windows-style path. + # No trailing slash on COPY/ADD # Results in dir being changed to a file WORKDIR /wc1 COPY wc1 c:/wc1 @@ -2196,20 +2136,6 @@ func (s *DockerSuite) TestBuildWindowsAddCopyPathProcessing(c *check.C) { RUN sh -c "[ $(cat c:/wd2/wd2) = 'worldwd2' ]" ` ctx, err := fakeContext(dockerfile, map[string]string{ - "a1": "helloa1", - "a2": "worlda2", - "b1": "hellob1", - "b2": "worldb2", - "c1": "helloc1", - "c2": "worldc2", - "d1": "hellod1", - "d2": "worldd2", - "e1": "helloe1", - "e2": "worlde2", - "wa1": "hellowa1", - "wa2": "worldwa2", - "wb1": "hellowb1", - "wb2": "worldwb2", "wc1": "hellowc1", "wc2": "worldwc2", "wd1": "hellowd1", @@ -2225,96 +2151,6 @@ func (s *DockerSuite) TestBuildWindowsAddCopyPathProcessing(c *check.C) { } } -// #22181 Regression test. -func (s *DockerSuite) TestBuildWindowsCopyFailsNonSystemDrive(c *check.C) { - testRequires(c, DaemonIsWindows) - name := "testbuildwindowscopyfailsnonsystemdrive" - dockerfile := ` - FROM busybox - cOpY foo d:/ - ` - ctx, err := fakeContext(dockerfile, map[string]string{"foo": "hello"}) - if err != nil { - c.Fatal(err) - } - defer ctx.Close() - _, err = buildImageFromContext(name, ctx, false) - if err == nil { - c.Fatal(err) - } - if !strings.Contains(err.Error(), "Windows does not support COPY with a destinations not on the system drive (C:)") { - c.Fatal(err) - } -} - -// #22181 Regression test. -func (s *DockerSuite) TestBuildWindowsCopyFailsWorkdirNonSystemDrive(c *check.C) { - testRequires(c, DaemonIsWindows) - name := "testbuildwindowscopyfailsworkdirsystemdrive" - dockerfile := ` - FROM busybox - WORKDIR d:/ - cOpY foo . - ` - ctx, err := fakeContext(dockerfile, map[string]string{"foo": "hello"}) - if err != nil { - c.Fatal(err) - } - defer ctx.Close() - _, err = buildImageFromContext(name, ctx, false) - if err == nil { - c.Fatal(err) - } - if !strings.Contains(err.Error(), "Windows does not support COPY with relative paths when WORKDIR is not the system drive") { - c.Fatal(err) - } -} - -// #22181 Regression test. -func (s *DockerSuite) TestBuildWindowsAddFailsNonSystemDrive(c *check.C) { - testRequires(c, DaemonIsWindows) - name := "testbuildwindowsaddfailsnonsystemdrive" - dockerfile := ` - FROM busybox - AdD foo d:/ - ` - ctx, err := fakeContext(dockerfile, map[string]string{"foo": "hello"}) - if err != nil { - c.Fatal(err) - } - defer ctx.Close() - _, err = buildImageFromContext(name, ctx, false) - if err == nil { - c.Fatal(err) - } - if !strings.Contains(err.Error(), "Windows does not support ADD with a destinations not on the system drive (C:)") { - c.Fatal(err) - } -} - -// #22181 Regression test. -func (s *DockerSuite) TestBuildWindowsAddFailsWorkdirNonSystemDrive(c *check.C) { - testRequires(c, DaemonIsWindows) - name := "testbuildwindowsaddfailsworkdirsystemdrive" - dockerfile := ` - FROM busybox - WORKDIR d:/ - AdD foo . - ` - ctx, err := fakeContext(dockerfile, map[string]string{"foo": "hello"}) - if err != nil { - c.Fatal(err) - } - defer ctx.Close() - _, err = buildImageFromContext(name, ctx, false) - if err == nil { - c.Fatal(err) - } - if !strings.Contains(err.Error(), "Windows does not support ADD with relative paths when WORKDIR is not the system drive") { - c.Fatal(err) - } -} - func (s *DockerSuite) TestBuildWorkdirWithEnvVariables(c *check.C) { name := "testbuildworkdirwithenvvariables"