diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index fdc28bc7cd..88da50b0e7 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -387,7 +387,7 @@ func workdir(req dispatchRequest) error { runConfig := req.state.runConfig // This is from the Dockerfile and will not necessarily be in platform // specific semantics, hence ensure it is converted. - runConfig.WorkingDir, err = normaliseWorkdir(runConfig.WorkingDir, req.args[0]) + runConfig.WorkingDir, err = normaliseWorkdir(req.builder.platform, runConfig.WorkingDir, req.args[0]) if err != nil { return err } diff --git a/builder/dockerfile/dispatchers_unix.go b/builder/dockerfile/dispatchers_unix.go index 62ee371dfe..cf4036df01 100644 --- a/builder/dockerfile/dispatchers_unix.go +++ b/builder/dockerfile/dispatchers_unix.go @@ -11,7 +11,7 @@ import ( // normaliseWorkdir normalises a user requested working directory in a // platform semantically consistent way. -func normaliseWorkdir(current string, requested string) (string, error) { +func normaliseWorkdir(_ string, current string, requested string) (string, error) { if requested == "" { return "", errors.New("cannot normalise nothing") } diff --git a/builder/dockerfile/dispatchers_unix_test.go b/builder/dockerfile/dispatchers_unix_test.go index 4aae6b460e..c86ff052c7 100644 --- a/builder/dockerfile/dispatchers_unix_test.go +++ b/builder/dockerfile/dispatchers_unix_test.go @@ -3,6 +3,7 @@ package dockerfile import ( + "runtime" "testing" ) @@ -16,7 +17,7 @@ func TestNormaliseWorkdir(t *testing.T) { } for _, test := range testCases { - normalised, err := normaliseWorkdir(test.current, test.requested) + normalised, err := normaliseWorkdir(runtime.GOOS, test.current, test.requested) if test.expectedError != "" && err == nil { t.Fatalf("NormaliseWorkdir should return an error %s, got nil", test.expectedError) diff --git a/builder/dockerfile/dispatchers_windows.go b/builder/dockerfile/dispatchers_windows.go index 71f7c9288f..14a64e2866 100644 --- a/builder/dockerfile/dispatchers_windows.go +++ b/builder/dockerfile/dispatchers_windows.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "os" + "path" "path/filepath" "regexp" "strings" @@ -15,7 +16,33 @@ var pattern = regexp.MustCompile(`^[a-zA-Z]:\.$`) // normaliseWorkdir normalises a user requested working directory in a // platform semantically consistent way. -func normaliseWorkdir(current string, requested string) (string, error) { +func normaliseWorkdir(platform string, current string, requested string) (string, error) { + if platform == "" { + platform = "windows" + } + if platform == "windows" { + return normaliseWorkdirWindows(current, requested) + } + return normaliseWorkdirUnix(current, requested) +} + +// normaliseWorkdirUnix normalises a user requested working directory in a +// platform semantically consistent way. +func normaliseWorkdirUnix(current string, requested string) (string, error) { + if requested == "" { + return "", errors.New("cannot normalise nothing") + } + current = strings.Replace(current, string(os.PathSeparator), "/", -1) + requested = strings.Replace(requested, string(os.PathSeparator), "/", -1) + if !path.IsAbs(requested) { + return path.Join(`/`, current, requested), nil + } + return requested, nil +} + +// normaliseWorkdirWindows normalises a user requested working directory in a +// platform semantically consistent way. +func normaliseWorkdirWindows(current string, requested string) (string, error) { if requested == "" { return "", errors.New("cannot normalise nothing") } diff --git a/builder/dockerfile/dispatchers_windows_test.go b/builder/dockerfile/dispatchers_windows_test.go index 3319c06582..d2edcd1874 100644 --- a/builder/dockerfile/dispatchers_windows_test.go +++ b/builder/dockerfile/dispatchers_windows_test.go @@ -5,25 +5,31 @@ package dockerfile import "testing" func TestNormaliseWorkdir(t *testing.T) { - tests := []struct{ current, requested, expected, etext string }{ - {``, ``, ``, `cannot normalise nothing`}, - {``, `C:`, ``, `C:. is not a directory. If you are specifying a drive letter, please add a trailing '\'`}, - {``, `C:.`, ``, `C:. is not a directory. If you are specifying a drive letter, please add a trailing '\'`}, - {`c:`, `\a`, ``, `c:. is not a directory. If you are specifying a drive letter, please add a trailing '\'`}, - {`c:.`, `\a`, ``, `c:. is not a directory. If you are specifying a drive letter, please add a trailing '\'`}, - {``, `a`, `C:\a`, ``}, - {``, `c:\foo`, `C:\foo`, ``}, - {``, `c:\\foo`, `C:\foo`, ``}, - {``, `\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`, ``}, + tests := []struct{ platform, current, requested, expected, etext string }{ + {"windows", ``, ``, ``, `cannot normalise nothing`}, + {"windows", ``, `C:`, ``, `C:. is not a directory. If you are specifying a drive letter, please add a trailing '\'`}, + {"windows", ``, `C:.`, ``, `C:. is not a directory. If you are specifying a drive letter, please add a trailing '\'`}, + {"windows", `c:`, `\a`, ``, `c:. is not a directory. If you are specifying a drive letter, please add a trailing '\'`}, + {"windows", `c:.`, `\a`, ``, `c:. is not a directory. If you are specifying a drive letter, please add a trailing '\'`}, + {"windows", ``, `a`, `C:\a`, ``}, + {"windows", ``, `c:\foo`, `C:\foo`, ``}, + {"windows", ``, `c:\\foo`, `C:\foo`, ``}, + {"windows", ``, `\foo`, `C:\foo`, ``}, + {"windows", ``, `\\foo`, `C:\foo`, ``}, + {"windows", ``, `/foo`, `C:\foo`, ``}, + {"windows", ``, `C:/foo`, `C:\foo`, ``}, + {"windows", `C:\foo`, `bar`, `C:\foo\bar`, ``}, + {"windows", `C:\foo`, `/bar`, `C:\bar`, ``}, + {"windows", `C:\foo`, `\bar`, `C:\bar`, ``}, + {"linux", ``, ``, ``, `cannot normalise nothing`}, + {"linux", ``, `foo`, `/foo`, ``}, + {"linux", ``, `/foo`, `/foo`, ``}, + {"linux", `/foo`, `bar`, `/foo/bar`, ``}, + {"linux", `/foo`, `/bar`, `/bar`, ``}, + {"linux", `\a`, `b\c`, `/a/b/c`, ``}, } for _, i := range tests { - r, e := normaliseWorkdir(i.current, i.requested) + r, e := normaliseWorkdir(i.platform, i.current, i.requested) if i.etext != "" && e == nil { t.Fatalf("TestNormaliseWorkingDir Expected error %s for '%s' '%s', got no error", i.etext, i.current, i.requested) diff --git a/container/container.go b/container/container.go index 0713ec6be2..40d493be6c 100644 --- a/container/container.go +++ b/container/container.go @@ -261,12 +261,17 @@ func (container *Container) WriteHostConfig() (*containertypes.HostConfig, error // SetupWorkingDirectory sets up the container's working directory as set in container.Config.WorkingDir func (container *Container) SetupWorkingDirectory(rootIDs idtools.IDPair) error { + // TODO @jhowardmsft, @gupta-ak LCOW Support. This will need revisiting. + // We will need to do remote filesystem operations here. + if container.Platform != runtime.GOOS { + return nil + } + if container.Config.WorkingDir == "" { return nil } container.Config.WorkingDir = filepath.Clean(container.Config.WorkingDir) - pth, err := container.GetResourcePath(container.Config.WorkingDir) if err != nil { return err diff --git a/daemon/container.go b/daemon/container.go index ddb8a2ba0c..79b9d412df 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -3,7 +3,10 @@ package daemon import ( "fmt" "os" + "path" "path/filepath" + "runtime" + "strings" "time" containertypes "github.com/docker/docker/api/types/container" @@ -226,13 +229,24 @@ func (daemon *Daemon) setHostConfig(container *container.Container, hostConfig * // verifyContainerSettings performs validation of the hostconfig and config // structures. -func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostConfig, config *containertypes.Config, update bool) ([]string, error) { - +func (daemon *Daemon) verifyContainerSettings(platform string, hostConfig *containertypes.HostConfig, config *containertypes.Config, update bool) ([]string, error) { // First perform verification of settings common across all platforms. if config != nil { if config.WorkingDir != "" { - config.WorkingDir = filepath.FromSlash(config.WorkingDir) // Ensure in platform semantics - if !system.IsAbs(config.WorkingDir) { + wdInvalid := false + if runtime.GOOS == platform { + config.WorkingDir = filepath.FromSlash(config.WorkingDir) // Ensure in platform semantics + if !system.IsAbs(config.WorkingDir) { + wdInvalid = true + } + } else { + // LCOW. Force Unix semantics + config.WorkingDir = strings.Replace(config.WorkingDir, string(os.PathSeparator), "/", -1) + if !path.IsAbs(config.WorkingDir) { + wdInvalid = true + } + } + if wdInvalid { return nil, fmt.Errorf("the working directory '%s' is invalid, it needs to be an absolute path", config.WorkingDir) } } diff --git a/daemon/create.go b/daemon/create.go index c82a12eeb4..c722405db6 100644 --- a/daemon/create.go +++ b/daemon/create.go @@ -39,7 +39,17 @@ func (daemon *Daemon) containerCreate(params types.ContainerCreateConfig, manage return containertypes.ContainerCreateCreatedBody{}, validationError{errors.New("Config cannot be empty in order to create a container")} } - warnings, err := daemon.verifyContainerSettings(params.HostConfig, params.Config, false) + // TODO: @jhowardmsft LCOW support - at a later point, can remove the hard-coding + // to force the platform to be linux. + // Default the platform if not supplied + if params.Platform == "" { + params.Platform = runtime.GOOS + } + if system.LCOWSupported() { + params.Platform = "linux" + } + + warnings, err := daemon.verifyContainerSettings(params.Platform, params.HostConfig, params.Config, false) if err != nil { return containertypes.ContainerCreateCreatedBody{Warnings: warnings}, validationError{err} } @@ -75,16 +85,6 @@ func (daemon *Daemon) create(params types.ContainerCreateConfig, managed bool) ( err error ) - // TODO: @jhowardmsft LCOW support - at a later point, can remove the hard-coding - // to force the platform to be linux. - // Default the platform if not supplied - if params.Platform == "" { - params.Platform = runtime.GOOS - } - if system.LCOWSupported() { - params.Platform = "linux" - } - if params.Config.Image != "" { img, err = daemon.GetImage(params.Config.Image) if err != nil { diff --git a/daemon/start.go b/daemon/start.go index f0522d3931..901f270798 100644 --- a/daemon/start.go +++ b/daemon/start.go @@ -79,7 +79,7 @@ func (daemon *Daemon) ContainerStart(name string, hostConfig *containertypes.Hos // check if hostConfig is in line with the current system settings. // It may happen cgroups are umounted or the like. - if _, err = daemon.verifyContainerSettings(container.HostConfig, nil, false); err != nil { + if _, err = daemon.verifyContainerSettings(container.Platform, container.HostConfig, nil, false); err != nil { return validationError{err} } // Adapt for old containers in case we have updates in this function and diff --git a/daemon/update.go b/daemon/update.go index 4b133b255b..ee39e05cb0 100644 --- a/daemon/update.go +++ b/daemon/update.go @@ -11,7 +11,12 @@ import ( func (daemon *Daemon) ContainerUpdate(name string, hostConfig *container.HostConfig) (container.ContainerUpdateOKBody, error) { var warnings []string - warnings, err := daemon.verifyContainerSettings(hostConfig, nil, true) + c, err := daemon.GetContainer(name) + if err != nil { + return container.ContainerUpdateOKBody{Warnings: warnings}, err + } + + warnings, err = daemon.verifyContainerSettings(c.Platform, hostConfig, nil, true) if err != nil { return container.ContainerUpdateOKBody{Warnings: warnings}, validationError{err} }