diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index ac7c2b07e9..4fcd8abe43 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -264,12 +264,37 @@ func workdir(b *Builder, args []string, attributes map[string]bool, original str // This is from the Dockerfile and will not necessarily be in platform // specific semantics, hence ensure it is converted. workdir := filepath.FromSlash(args[0]) - - if !system.IsAbs(workdir) { - current := filepath.FromSlash(b.runConfig.WorkingDir) - workdir = filepath.Join(string(os.PathSeparator), current, workdir) + 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 = workdir return b.commit("", b.runConfig.Cmd, fmt.Sprintf("WORKDIR %v", workdir)) diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index 6a937707f8..614c55075d 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -213,13 +213,59 @@ func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalD // Twiddle the destination when its a relative path - meaning, make it // relative to the WORKINGDIR - if !system.IsAbs(dest) { - hasSlash := strings.HasSuffix(dest, string(os.PathSeparator)) - dest = filepath.Join(string(os.PathSeparator), filepath.FromSlash(b.runConfig.WorkingDir), dest) - // Make sure we preserve any trailing slash - if hasSlash { - dest += string(os.PathSeparator) + 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) + } } } diff --git a/container/container.go b/container/container.go index e658c4b4dd..c47f124455 100644 --- a/container/container.go +++ b/container/container.go @@ -250,6 +250,13 @@ func (container *Container) GetResourcePath(path string) (string, error) { cleanPath := cleanResourcePath(path) r, e := symlink.FollowSymlinkInScope(filepath.Join(container.BaseFS, cleanPath), container.BaseFS) + + // Log this here on the daemon side as there's otherwise no indication apart + // from the error being propagated all the way back to the client. This makes + // debugging significantly easier and clearly indicates the error comes from the daemon. + if e != nil { + logrus.Errorf("Failed to FollowSymlinkInScope BaseFS %s cleanPath %s path %s %s\n", container.BaseFS, cleanPath, path, e) + } return r, e } diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index b1347284d6..0f41c8b6f6 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -2074,22 +2074,13 @@ func (s *DockerSuite) TestBuildRelativeWorkdir(c *check.C) { expected4 string expectedFinal string ) - // TODO Windows: The expectedFinal needs fixing to match Windows - // filepath semantics. However, this is a non-trivial change. @jhowardmsft - // Short story - need to make the configuration file platform semantically - // consistent, so even if `WORKDIR /test2/test3` is specified in a Dockerfile, - // the configuration should be stored as C:\test2\test3. Something similar to - // if runtime.GOOS == "windows" && len(workdir) > 2 && string(workdir[0]) == `\` { - // workdir = "C:" + workdir - // } - // in builder\dockerfile\dispatchers.go, function workdir(), but also - // ironing out all other cases where this causes other failures. + if daemonPlatform == "windows" { expected1 = `C:/` expected2 = `C:/test1` expected3 = `C:/test2` expected4 = `C:/test2/test3` - expectedFinal = `\test2\test3` + expectedFinal = `C:\test2\test3` // Note inspect is going to return Windows paths, as it's not in busybox } else { expected1 = `/` expected2 = `/test1` @@ -2117,12 +2108,238 @@ func (s *DockerSuite) TestBuildRelativeWorkdir(c *check.C) { } } +// #22181 Regression test. Validates combinations of supported +// WORKDIR dockerfile directives in Windows and non-Windows semantics. +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 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 { + c.Fatal(err) + } +} + +// #22181 Regression test. Validates combinations of supported +// COPY dockerfile directives in Windows and non-Windows semantics. +func (s *DockerSuite) TestBuildWindowsAddCopyPathProcessing(c *check.C) { + testRequires(c, DaemonIsWindows) + name := "testbuildwindowsaddcopypathprocessing" + // TODO Windows (@jhowardmsft). Needs a follow-up PR to 22181 to + // support backslash such as .\\ being equivalent to ./ and c:\\ being + // equivalent to c:/. This is not currently (nor ever has been) supported + // 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. + # Results in dir being changed to a file + WORKDIR /wc1 + COPY wc1 c:/wc1 + WORKDIR /wc2 + ADD wc2 c:/wc2 + WORKDIR c:/ + RUN sh -c "[ $(cat c:/wc1) = 'hellowc1' ]" + RUN sh -c "[ $(cat c:/wc2) = 'worldwc2' ]" + + # Trailing slash on COPY/ADD, Windows-style path. + WORKDIR /wd1 + COPY wd1 c:/wd1/ + WORKDIR /wd2 + ADD wd2 c:/wd2/ + RUN sh -c "[ $(cat c:/wd1/wd1) = 'hellowd1' ]" + 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", + "wd2": "worldwd2", + }) + if err != nil { + c.Fatal(err) + } + defer ctx.Close() + _, err = buildImageFromContext(name, ctx, false) + if err != nil { + c.Fatal(err) + } +} + +// #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" var expected string if daemonPlatform == "windows" { - expected = `\test1\test2` + expected = `C:\test1\test2` } else { expected = `/test1/test2` }