LCOW: WORKDIR correct handling

Signed-off-by: John Howard <[email protected]>

Author: John Howard

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
 	}

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")
 	}

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)

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")
 	}

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)

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

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

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 {

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

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