1. docker
  2. Commit 38f8b0eb10725c40fb3c7e0719accd240cd39e22
Browse code

Validate mount paths on task create

This is intended as a minor fix for 1.12.1 so that task creation doesn't
do unexpected things when the user supplies erroneous paths.

In particular, because we're currently using hostConfig.Binds to setup
mounts, if a user uses an absolute path for a volume mount source, or a
non-absolute path for a bind mount source, the engine will do the
opposite of what the user requested since all absolute paths are
treated as binds and all non-absolute paths are treated as named
volumes.

Fixes #25253

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

Brian Goff authored on 2016/08/02 06:00:13
Showing 5 changed files
daemon/cluster/executor/container/container.go
History View file @ 38f8b0e
... ... 
@@ -53,6 +53,10 @@ func (c *containerConfig) setTask(t *api.Task) error {
53 53 
 		return ErrImageRequired
54 54 
 	}
55 55
56 
+	if err := validateMounts(container.Mounts); err != nil {
57 
+		return err
58 
+	}
59 
+
56 60 
 	// index the networks by name
57 61 
 	c.networksAttachments = make(map[string]*api.NetworkAttachment, len(t.Networks))
58 62 
 	for _, attachment := range t.Networks {
daemon/cluster/executor/container/validate.go
History View file @ 38f8b0e
59 63 
new file mode 100644
... ... 
@@ -0,0 +1,39 @@
0 
+package container
1 
+
2 
+import (
3 
+	"fmt"
4 
+	"path/filepath"
5 
+
6 
+	"github.com/docker/swarmkit/api"
7 
+)
8 
+
9 
+func validateMounts(mounts []api.Mount) error {
10 
+	for _, mount := range mounts {
11 
+		// Target must always be absolute
12 
+		if !filepath.IsAbs(mount.Target) {
13 
+			return fmt.Errorf("invalid mount target, must be an absolute path: %s", mount.Target)
14 
+		}
15 
+
16 
+		switch mount.Type {
17 
+		// The checks on abs paths are required due to the container API confusing
18 
+		// volume mounts as bind mounts when the source is absolute (and vice-versa)
19 
+		// See #25253
20 
+		// TODO: This is probably not neccessary once #22373 is merged
21 
+		case api.MountTypeBind:
22 
+			if !filepath.IsAbs(mount.Source) {
23 
+				return fmt.Errorf("invalid bind mount source, must be an absolute path: %s", mount.Source)
24 
+			}
25 
+		case api.MountTypeVolume:
26 
+			if filepath.IsAbs(mount.Source) {
27 
+				return fmt.Errorf("invalid volume mount source, must not be an absolute path: %s", mount.Source)
28 
+			}
29 
+		case api.MountTypeTmpfs:
30 
+			if mount.Source != "" {
31 
+				return fmt.Errorf("invalid tmpfs source, source must be empty")
32 
+			}
33 
+		default:
34 
+			return fmt.Errorf("invalid mount type: %s", mount.Type)
35 
+		}
36 
+	}
37 
+	return nil
38 
+}
daemon/cluster/executor/container/validate_test.go
History View file @ 38f8b0e
0 39 
new file mode 100644
... ... 
@@ -0,0 +1,118 @@
0 
+package container
1 
+
2 
+import (
3 
+	"strings"
4 
+	"testing"
5 
+
6 
+	"github.com/docker/docker/daemon"
7 
+	"github.com/docker/docker/pkg/stringid"
8 
+	"github.com/docker/swarmkit/api"
9 
+)
10 
+
11 
+func newTestControllerWithMount(m api.Mount) (*controller, error) {
12 
+	return newController(&daemon.Daemon{}, &api.Task{
13 
+		ID:        stringid.GenerateRandomID(),
14 
+		ServiceID: stringid.GenerateRandomID(),
15 
+		Spec: api.TaskSpec{
16 
+			Runtime: &api.TaskSpec_Container{
17 
+				Container: &api.ContainerSpec{
18 
+					Image: "image_name",
19 
+					Labels: map[string]string{
20 
+						"com.docker.swarm.task.id": "id",
21 
+					},
22 
+					Mounts: []api.Mount{m},
23 
+				},
24 
+			},
25 
+		},
26 
+	})
27 
+}
28 
+
29 
+func TestControllerValidateMountBind(t *testing.T) {
30 
+	// with improper source
31 
+	if _, err := newTestControllerWithMount(api.Mount{
32 
+		Type:   api.MountTypeBind,
33 
+		Source: "foo",
34 
+		Target: testAbsPath,
35 
+	}); err == nil || !strings.Contains(err.Error(), "invalid bind mount source") {
36 
+		t.Fatalf("expected  error, got: %v", err)
37 
+	}
38 
+
39 
+	// with proper source
40 
+	if _, err := newTestControllerWithMount(api.Mount{
41 
+		Type:   api.MountTypeBind,
42 
+		Source: testAbsPath,
43 
+		Target: testAbsPath,
44 
+	}); err != nil {
45 
+		t.Fatalf("expected  error, got: %v", err)
46 
+	}
47 
+}
48 
+
49 
+func TestControllerValidateMountVolume(t *testing.T) {
50 
+	// with improper source
51 
+	if _, err := newTestControllerWithMount(api.Mount{
52 
+		Type:   api.MountTypeVolume,
53 
+		Source: testAbsPath,
54 
+		Target: testAbsPath,
55 
+	}); err == nil || !strings.Contains(err.Error(), "invalid volume mount source") {
56 
+		t.Fatalf("expected error, got: %v", err)
57 
+	}
58 
+
59 
+	// with proper source
60 
+	if _, err := newTestControllerWithMount(api.Mount{
61 
+		Type:   api.MountTypeVolume,
62 
+		Source: "foo",
63 
+		Target: testAbsPath,
64 
+	}); err != nil {
65 
+		t.Fatalf("expected error, got: %v", err)
66 
+	}
67 
+}
68 
+
69 
+func TestControllerValidateMountTarget(t *testing.T) {
70 
+	// with improper target
71 
+	if _, err := newTestControllerWithMount(api.Mount{
72 
+		Type:   api.MountTypeBind,
73 
+		Source: testAbsPath,
74 
+		Target: "foo",
75 
+	}); err == nil || !strings.Contains(err.Error(), "invalid mount target") {
76 
+		t.Fatalf("expected error, got: %v", err)
77 
+	}
78 
+
79 
+	// with proper target
80 
+	if _, err := newTestControllerWithMount(api.Mount{
81 
+		Type:   api.MountTypeBind,
82 
+		Source: testAbsPath,
83 
+		Target: testAbsPath,
84 
+	}); err != nil {
85 
+		t.Fatalf("expected no error, got: %v", err)
86 
+	}
87 
+}
88 
+
89 
+func TestControllerValidateMountTmpfs(t *testing.T) {
90 
+	// with improper target
91 
+	if _, err := newTestControllerWithMount(api.Mount{
92 
+		Type:   api.MountTypeTmpfs,
93 
+		Source: "foo",
94 
+		Target: testAbsPath,
95 
+	}); err == nil || !strings.Contains(err.Error(), "invalid tmpfs source") {
96 
+		t.Fatalf("expected error, got: %v", err)
97 
+	}
98 
+
99 
+	// with proper target
100 
+	if _, err := newTestControllerWithMount(api.Mount{
101 
+		Type:   api.MountTypeTmpfs,
102 
+		Target: testAbsPath,
103 
+	}); err != nil {
104 
+		t.Fatalf("expected no error, got: %v", err)
105 
+	}
106 
+}
107 
+
108 
+func TestControllerValidateMountInvalidType(t *testing.T) {
109 
+	// with improper target
110 
+	if _, err := newTestControllerWithMount(api.Mount{
111 
+		Type:   api.Mount_MountType(9999),
112 
+		Source: "foo",
113 
+		Target: testAbsPath,
114 
+	}); err == nil || !strings.Contains(err.Error(), "invalid mount type") {
115 
+		t.Fatalf("expected error, got: %v", err)
116 
+	}
117 
+}
daemon/cluster/executor/container/validate_unix_test.go
History View file @ 38f8b0e
0 118 
new file mode 100644
... ... 
@@ -0,0 +1,7 @@
0 
+// +build !windows
1 
+
2 
+package container
3 
+
4 
+const (
5 
+	testAbsPath = "/foo"
6 
+)
daemon/cluster/executor/container/validate_windows_test.go
History View file @ 38f8b0e
0 7 
new file mode 100644
... ... 
@@ -0,0 +1,5 @@
0 
+package container
1 
+
2 
+const (
3 
+	testAbsPath = `c:\foo`
4 
+)