Browse code

LCOW: WORKDIR correct handling

Signed-off-by: John Howard <jhoward@microsoft.com>

John Howard authored on 2017/08/05 03:39:23
Showing 10 changed files
... ...
@@ -387,7 +387,7 @@ func workdir(req dispatchRequest) error {
387 387
 	runConfig := req.state.runConfig
388 388
 	// This is from the Dockerfile and will not necessarily be in platform
389 389
 	// specific semantics, hence ensure it is converted.
390
-	runConfig.WorkingDir, err = normaliseWorkdir(runConfig.WorkingDir, req.args[0])
390
+	runConfig.WorkingDir, err = normaliseWorkdir(req.builder.platform, runConfig.WorkingDir, req.args[0])
391 391
 	if err != nil {
392 392
 		return err
393 393
 	}
... ...
@@ -11,7 +11,7 @@ import (
11 11
 
12 12
 // normaliseWorkdir normalises a user requested working directory in a
13 13
 // platform semantically consistent way.
14
-func normaliseWorkdir(current string, requested string) (string, error) {
14
+func normaliseWorkdir(_ string, current string, requested string) (string, error) {
15 15
 	if requested == "" {
16 16
 		return "", errors.New("cannot normalise nothing")
17 17
 	}
... ...
@@ -3,6 +3,7 @@
3 3
 package dockerfile
4 4
 
5 5
 import (
6
+	"runtime"
6 7
 	"testing"
7 8
 )
8 9
 
... ...
@@ -16,7 +17,7 @@ func TestNormaliseWorkdir(t *testing.T) {
16 16
 	}
17 17
 
18 18
 	for _, test := range testCases {
19
-		normalised, err := normaliseWorkdir(test.current, test.requested)
19
+		normalised, err := normaliseWorkdir(runtime.GOOS, test.current, test.requested)
20 20
 
21 21
 		if test.expectedError != "" && err == nil {
22 22
 			t.Fatalf("NormaliseWorkdir should return an error %s, got nil", test.expectedError)
... ...
@@ -4,6 +4,7 @@ import (
4 4
 	"errors"
5 5
 	"fmt"
6 6
 	"os"
7
+	"path"
7 8
 	"path/filepath"
8 9
 	"regexp"
9 10
 	"strings"
... ...
@@ -15,7 +16,33 @@ var pattern = regexp.MustCompile(`^[a-zA-Z]:\.$`)
15 15
 
16 16
 // normaliseWorkdir normalises a user requested working directory in a
17 17
 // platform semantically consistent way.
18
-func normaliseWorkdir(current string, requested string) (string, error) {
18
+func normaliseWorkdir(platform string, current string, requested string) (string, error) {
19
+	if platform == "" {
20
+		platform = "windows"
21
+	}
22
+	if platform == "windows" {
23
+		return normaliseWorkdirWindows(current, requested)
24
+	}
25
+	return normaliseWorkdirUnix(current, requested)
26
+}
27
+
28
+// normaliseWorkdirUnix normalises a user requested working directory in a
29
+// platform semantically consistent way.
30
+func normaliseWorkdirUnix(current string, requested string) (string, error) {
31
+	if requested == "" {
32
+		return "", errors.New("cannot normalise nothing")
33
+	}
34
+	current = strings.Replace(current, string(os.PathSeparator), "/", -1)
35
+	requested = strings.Replace(requested, string(os.PathSeparator), "/", -1)
36
+	if !path.IsAbs(requested) {
37
+		return path.Join(`/`, current, requested), nil
38
+	}
39
+	return requested, nil
40
+}
41
+
42
+// normaliseWorkdirWindows normalises a user requested working directory in a
43
+// platform semantically consistent way.
44
+func normaliseWorkdirWindows(current string, requested string) (string, error) {
19 45
 	if requested == "" {
20 46
 		return "", errors.New("cannot normalise nothing")
21 47
 	}
... ...
@@ -5,25 +5,31 @@ package dockerfile
5 5
 import "testing"
6 6
 
7 7
 func TestNormaliseWorkdir(t *testing.T) {
8
-	tests := []struct{ current, requested, expected, etext string }{
9
-		{``, ``, ``, `cannot normalise nothing`},
10
-		{``, `C:`, ``, `C:. is not a directory. If you are specifying a drive letter, please add a trailing '\'`},
11
-		{``, `C:.`, ``, `C:. is not a directory. If you are specifying a drive letter, please add a trailing '\'`},
12
-		{`c:`, `\a`, ``, `c:. is not a directory. If you are specifying a drive letter, please add a trailing '\'`},
13
-		{`c:.`, `\a`, ``, `c:. is not a directory. If you are specifying a drive letter, please add a trailing '\'`},
14
-		{``, `a`, `C:\a`, ``},
15
-		{``, `c:\foo`, `C:\foo`, ``},
16
-		{``, `c:\\foo`, `C:\foo`, ``},
17
-		{``, `\foo`, `C:\foo`, ``},
18
-		{``, `\\foo`, `C:\foo`, ``},
19
-		{``, `/foo`, `C:\foo`, ``},
20
-		{``, `C:/foo`, `C:\foo`, ``},
21
-		{`C:\foo`, `bar`, `C:\foo\bar`, ``},
22
-		{`C:\foo`, `/bar`, `C:\bar`, ``},
23
-		{`C:\foo`, `\bar`, `C:\bar`, ``},
8
+	tests := []struct{ platform, current, requested, expected, etext string }{
9
+		{"windows", ``, ``, ``, `cannot normalise nothing`},
10
+		{"windows", ``, `C:`, ``, `C:. is not a directory. If you are specifying a drive letter, please add a trailing '\'`},
11
+		{"windows", ``, `C:.`, ``, `C:. is not a directory. If you are specifying a drive letter, please add a trailing '\'`},
12
+		{"windows", `c:`, `\a`, ``, `c:. is not a directory. If you are specifying a drive letter, please add a trailing '\'`},
13
+		{"windows", `c:.`, `\a`, ``, `c:. is not a directory. If you are specifying a drive letter, please add a trailing '\'`},
14
+		{"windows", ``, `a`, `C:\a`, ``},
15
+		{"windows", ``, `c:\foo`, `C:\foo`, ``},
16
+		{"windows", ``, `c:\\foo`, `C:\foo`, ``},
17
+		{"windows", ``, `\foo`, `C:\foo`, ``},
18
+		{"windows", ``, `\\foo`, `C:\foo`, ``},
19
+		{"windows", ``, `/foo`, `C:\foo`, ``},
20
+		{"windows", ``, `C:/foo`, `C:\foo`, ``},
21
+		{"windows", `C:\foo`, `bar`, `C:\foo\bar`, ``},
22
+		{"windows", `C:\foo`, `/bar`, `C:\bar`, ``},
23
+		{"windows", `C:\foo`, `\bar`, `C:\bar`, ``},
24
+		{"linux", ``, ``, ``, `cannot normalise nothing`},
25
+		{"linux", ``, `foo`, `/foo`, ``},
26
+		{"linux", ``, `/foo`, `/foo`, ``},
27
+		{"linux", `/foo`, `bar`, `/foo/bar`, ``},
28
+		{"linux", `/foo`, `/bar`, `/bar`, ``},
29
+		{"linux", `\a`, `b\c`, `/a/b/c`, ``},
24 30
 	}
25 31
 	for _, i := range tests {
26
-		r, e := normaliseWorkdir(i.current, i.requested)
32
+		r, e := normaliseWorkdir(i.platform, i.current, i.requested)
27 33
 
28 34
 		if i.etext != "" && e == nil {
29 35
 			t.Fatalf("TestNormaliseWorkingDir Expected error %s for '%s' '%s', got no error", i.etext, i.current, i.requested)
... ...
@@ -261,12 +261,17 @@ func (container *Container) WriteHostConfig() (*containertypes.HostConfig, error
261 261
 
262 262
 // SetupWorkingDirectory sets up the container's working directory as set in container.Config.WorkingDir
263 263
 func (container *Container) SetupWorkingDirectory(rootIDs idtools.IDPair) error {
264
+	// TODO @jhowardmsft, @gupta-ak LCOW Support. This will need revisiting.
265
+	// We will need to do remote filesystem operations here.
266
+	if container.Platform != runtime.GOOS {
267
+		return nil
268
+	}
269
+
264 270
 	if container.Config.WorkingDir == "" {
265 271
 		return nil
266 272
 	}
267 273
 
268 274
 	container.Config.WorkingDir = filepath.Clean(container.Config.WorkingDir)
269
-
270 275
 	pth, err := container.GetResourcePath(container.Config.WorkingDir)
271 276
 	if err != nil {
272 277
 		return err
... ...
@@ -3,7 +3,10 @@ package daemon
3 3
 import (
4 4
 	"fmt"
5 5
 	"os"
6
+	"path"
6 7
 	"path/filepath"
8
+	"runtime"
9
+	"strings"
7 10
 	"time"
8 11
 
9 12
 	containertypes "github.com/docker/docker/api/types/container"
... ...
@@ -226,13 +229,24 @@ func (daemon *Daemon) setHostConfig(container *container.Container, hostConfig *
226 226
 
227 227
 // verifyContainerSettings performs validation of the hostconfig and config
228 228
 // structures.
229
-func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostConfig, config *containertypes.Config, update bool) ([]string, error) {
230
-
229
+func (daemon *Daemon) verifyContainerSettings(platform string, hostConfig *containertypes.HostConfig, config *containertypes.Config, update bool) ([]string, error) {
231 230
 	// First perform verification of settings common across all platforms.
232 231
 	if config != nil {
233 232
 		if config.WorkingDir != "" {
234
-			config.WorkingDir = filepath.FromSlash(config.WorkingDir) // Ensure in platform semantics
235
-			if !system.IsAbs(config.WorkingDir) {
233
+			wdInvalid := false
234
+			if runtime.GOOS == platform {
235
+				config.WorkingDir = filepath.FromSlash(config.WorkingDir) // Ensure in platform semantics
236
+				if !system.IsAbs(config.WorkingDir) {
237
+					wdInvalid = true
238
+				}
239
+			} else {
240
+				// LCOW. Force Unix semantics
241
+				config.WorkingDir = strings.Replace(config.WorkingDir, string(os.PathSeparator), "/", -1)
242
+				if !path.IsAbs(config.WorkingDir) {
243
+					wdInvalid = true
244
+				}
245
+			}
246
+			if wdInvalid {
236 247
 				return nil, fmt.Errorf("the working directory '%s' is invalid, it needs to be an absolute path", config.WorkingDir)
237 248
 			}
238 249
 		}
... ...
@@ -39,7 +39,17 @@ func (daemon *Daemon) containerCreate(params types.ContainerCreateConfig, manage
39 39
 		return containertypes.ContainerCreateCreatedBody{}, validationError{errors.New("Config cannot be empty in order to create a container")}
40 40
 	}
41 41
 
42
-	warnings, err := daemon.verifyContainerSettings(params.HostConfig, params.Config, false)
42
+	// TODO: @jhowardmsft LCOW support - at a later point, can remove the hard-coding
43
+	// to force the platform to be linux.
44
+	// Default the platform if not supplied
45
+	if params.Platform == "" {
46
+		params.Platform = runtime.GOOS
47
+	}
48
+	if system.LCOWSupported() {
49
+		params.Platform = "linux"
50
+	}
51
+
52
+	warnings, err := daemon.verifyContainerSettings(params.Platform, params.HostConfig, params.Config, false)
43 53
 	if err != nil {
44 54
 		return containertypes.ContainerCreateCreatedBody{Warnings: warnings}, validationError{err}
45 55
 	}
... ...
@@ -75,16 +85,6 @@ func (daemon *Daemon) create(params types.ContainerCreateConfig, managed bool) (
75 75
 		err       error
76 76
 	)
77 77
 
78
-	// TODO: @jhowardmsft LCOW support - at a later point, can remove the hard-coding
79
-	// to force the platform to be linux.
80
-	// Default the platform if not supplied
81
-	if params.Platform == "" {
82
-		params.Platform = runtime.GOOS
83
-	}
84
-	if system.LCOWSupported() {
85
-		params.Platform = "linux"
86
-	}
87
-
88 78
 	if params.Config.Image != "" {
89 79
 		img, err = daemon.GetImage(params.Config.Image)
90 80
 		if err != nil {
... ...
@@ -79,7 +79,7 @@ func (daemon *Daemon) ContainerStart(name string, hostConfig *containertypes.Hos
79 79
 
80 80
 	// check if hostConfig is in line with the current system settings.
81 81
 	// It may happen cgroups are umounted or the like.
82
-	if _, err = daemon.verifyContainerSettings(container.HostConfig, nil, false); err != nil {
82
+	if _, err = daemon.verifyContainerSettings(container.Platform, container.HostConfig, nil, false); err != nil {
83 83
 		return validationError{err}
84 84
 	}
85 85
 	// Adapt for old containers in case we have updates in this function and
... ...
@@ -11,7 +11,12 @@ import (
11 11
 func (daemon *Daemon) ContainerUpdate(name string, hostConfig *container.HostConfig) (container.ContainerUpdateOKBody, error) {
12 12
 	var warnings []string
13 13
 
14
-	warnings, err := daemon.verifyContainerSettings(hostConfig, nil, true)
14
+	c, err := daemon.GetContainer(name)
15
+	if err != nil {
16
+		return container.ContainerUpdateOKBody{Warnings: warnings}, err
17
+	}
18
+
19
+	warnings, err = daemon.verifyContainerSettings(c.Platform, hostConfig, nil, true)
15 20
 	if err != nil {
16 21
 		return container.ContainerUpdateOKBody{Warnings: warnings}, validationError{err}
17 22
 	}