diff --git a/container/container.go b/container/container.go index 175f1b8154..c3adc95df5 100644 --- a/container/container.go +++ b/container/container.go @@ -17,6 +17,7 @@ import ( "github.com/Sirupsen/logrus" containertypes "github.com/docker/docker/api/types/container" + mounttypes "github.com/docker/docker/api/types/mount" networktypes "github.com/docker/docker/api/types/network" "github.com/docker/docker/daemon/exec" "github.com/docker/docker/daemon/logger" @@ -551,6 +552,7 @@ func (container *Container) ShouldRestart() bool { // AddMountPointWithVolume adds a new mount point configured with a volume to the container. func (container *Container) AddMountPointWithVolume(destination string, vol volume.Volume, rw bool) { container.MountPoints[destination] = &volume.MountPoint{ + Type: mounttypes.TypeVolume, Name: vol.Name(), Driver: vol.DriverName(), Destination: destination, diff --git a/daemon/create_unix.go b/daemon/create_unix.go index eecf27e1f4..2fe5c98a79 100644 --- a/daemon/create_unix.go +++ b/daemon/create_unix.go @@ -9,6 +9,7 @@ import ( "github.com/Sirupsen/logrus" containertypes "github.com/docker/docker/api/types/container" + mounttypes "github.com/docker/docker/api/types/mount" "github.com/docker/docker/container" "github.com/docker/docker/pkg/stringid" "github.com/opencontainers/runc/libcontainer/label" @@ -63,7 +64,11 @@ func (daemon *Daemon) createContainerPlatformSpecificSettings(container *contain // this is only called when the container is created. func (daemon *Daemon) populateVolumes(c *container.Container) error { for _, mnt := range c.MountPoints { - if !mnt.CopyData || mnt.Volume == nil { + if mnt.Volume == nil { + continue + } + + if mnt.Type != mounttypes.TypeVolume || !mnt.CopyData { continue } diff --git a/daemon/create_windows.go b/daemon/create_windows.go index 06e284feb0..bbf0dbe7b9 100644 --- a/daemon/create_windows.go +++ b/daemon/create_windows.go @@ -18,7 +18,7 @@ func (daemon *Daemon) createContainerPlatformSpecificSettings(container *contain for spec := range config.Volumes { - mp, err := volume.ParseMountSpec(spec, hostConfig.VolumeDriver) + mp, err := volume.ParseMountRaw(spec, hostConfig.VolumeDriver) if err != nil { return fmt.Errorf("Unrecognised volume spec: %v", err) } diff --git a/daemon/inspect_unix.go b/daemon/inspect_unix.go index 532ebbad2e..08a82235ad 100644 --- a/daemon/inspect_unix.go +++ b/daemon/inspect_unix.go @@ -68,6 +68,7 @@ func addMountPoints(container *container.Container) []types.MountPoint { mountPoints := make([]types.MountPoint, 0, len(container.MountPoints)) for _, m := range container.MountPoints { mountPoints = append(mountPoints, types.MountPoint{ + Type: m.Type, Name: m.Name, Source: m.Path(), Destination: m.Destination, diff --git a/daemon/inspect_windows.go b/daemon/inspect_windows.go index c8d8c795f8..b331c83ca3 100644 --- a/daemon/inspect_windows.go +++ b/daemon/inspect_windows.go @@ -16,6 +16,7 @@ func addMountPoints(container *container.Container) []types.MountPoint { mountPoints := make([]types.MountPoint, 0, len(container.MountPoints)) for _, m := range container.MountPoints { mountPoints = append(mountPoints, types.MountPoint{ + Type: m.Type, Name: m.Name, Source: m.Path(), Destination: m.Destination, diff --git a/daemon/mounts.go b/daemon/mounts.go index d4f24b2812..1c11f86a80 100644 --- a/daemon/mounts.go +++ b/daemon/mounts.go @@ -27,7 +27,7 @@ func (daemon *Daemon) removeMountPoints(container *container.Container, rm bool) if rm { // Do not remove named mountpoints // these are mountpoints specified like `docker run -v :/foo` - if m.Named { + if m.Spec.Source != "" { continue } err := daemon.volumes.Remove(m.Volume) diff --git a/daemon/volumes.go b/daemon/volumes.go index 4166b09d95..1deddcc94c 100644 --- a/daemon/volumes.go +++ b/daemon/volumes.go @@ -9,8 +9,11 @@ import ( "github.com/docker/docker/api/types" containertypes "github.com/docker/docker/api/types/container" + mounttypes "github.com/docker/docker/api/types/mount" "github.com/docker/docker/container" + dockererrors "github.com/docker/docker/errors" "github.com/docker/docker/volume" + "github.com/opencontainers/runc/libcontainer/label" ) var ( @@ -106,7 +109,8 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo Driver: m.Driver, Destination: m.Destination, Propagation: m.Propagation, - Named: m.Named, + Spec: m.Spec, + CopyData: false, } if len(cp.Source) == 0 { @@ -123,18 +127,18 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo // 3. Read bind mounts for _, b := range hostConfig.Binds { - // #10618 - bind, err := volume.ParseMountSpec(b, hostConfig.VolumeDriver) + bind, err := volume.ParseMountRaw(b, hostConfig.VolumeDriver) if err != nil { return err } + // #10618 _, tmpfsExists := hostConfig.Tmpfs[bind.Destination] if binds[bind.Destination] || tmpfsExists { return fmt.Errorf("Duplicate mount point '%s'", bind.Destination) } - if len(bind.Name) > 0 { + if bind.Type == mounttypes.TypeVolume { // create the volume v, err := daemon.volumes.CreateWithRef(bind.Name, bind.Driver, container.ID, nil, nil) if err != nil { @@ -144,9 +148,8 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo bind.Source = v.Path() // bind.Name is an already existing volume, we need to use that here bind.Driver = v.DriverName() - bind.Named = true - if bind.Driver == "local" { - bind = setBindModeIfNull(bind) + if bind.Driver == volume.DefaultDriverName { + setBindModeIfNull(bind) } } @@ -154,6 +157,50 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo mountPoints[bind.Destination] = bind } + for _, cfg := range hostConfig.Mounts { + mp, err := volume.ParseMountSpec(cfg) + if err != nil { + return dockererrors.NewBadRequestError(err) + } + + if binds[mp.Destination] { + return fmt.Errorf("Duplicate mount point '%s'", cfg.Target) + } + + if mp.Type == mounttypes.TypeVolume { + var v volume.Volume + if cfg.VolumeOptions != nil { + var driverOpts map[string]string + if cfg.VolumeOptions.DriverConfig != nil { + driverOpts = cfg.VolumeOptions.DriverConfig.Options + } + v, err = daemon.volumes.CreateWithRef(mp.Name, mp.Driver, container.ID, driverOpts, cfg.VolumeOptions.Labels) + } else { + v, err = daemon.volumes.CreateWithRef(mp.Name, mp.Driver, container.ID, nil, nil) + } + if err != nil { + return err + } + + if err := label.Relabel(mp.Source, container.MountLabel, false); err != nil { + return err + } + mp.Volume = v + mp.Name = v.Name() + mp.Driver = v.DriverName() + + // only use the cached path here since getting the path is not neccessary right now and calling `Path()` may be slow + if cv, ok := v.(interface { + CachedPath() string + }); ok { + mp.Source = cv.CachedPath() + } + } + + binds[mp.Destination] = true + mountPoints[mp.Destination] = mp + } + container.Lock() // 4. Cleanup old volumes that are about to be reassigned. diff --git a/daemon/volumes_unix.go b/daemon/volumes_unix.go index 89c6849575..fb07d27129 100644 --- a/daemon/volumes_unix.go +++ b/daemon/volumes_unix.go @@ -85,11 +85,10 @@ func sortMounts(m []container.Mount) []container.Mount { // setBindModeIfNull is platform specific processing to ensure the // shared mode is set to 'z' if it is null. This is called in the case // of processing a named volume and not a typical bind. -func setBindModeIfNull(bind *volume.MountPoint) *volume.MountPoint { +func setBindModeIfNull(bind *volume.MountPoint) { if bind.Mode == "" { bind.Mode = "z" } - return bind } // migrateVolume links the contents of a volume created pre Docker 1.7 diff --git a/daemon/volumes_windows.go b/daemon/volumes_windows.go index e7f9c098d9..b008dfb511 100644 --- a/daemon/volumes_windows.go +++ b/daemon/volumes_windows.go @@ -46,6 +46,6 @@ func (daemon *Daemon) setupMounts(c *container.Container) ([]container.Mount, er // setBindModeIfNull is platform specific processing which is a no-op on // Windows. -func setBindModeIfNull(bind *volume.MountPoint) *volume.MountPoint { - return bind +func setBindModeIfNull(bind *volume.MountPoint) { + return } diff --git a/docs/reference/api/docker_remote_api.md b/docs/reference/api/docker_remote_api.md index 70c4595ab0..61adcad43c 100644 --- a/docs/reference/api/docker_remote_api.md +++ b/docs/reference/api/docker_remote_api.md @@ -122,6 +122,7 @@ This section lists each version from latest to oldest. Each listing includes a * `DELETE /volumes/(name)` now accepts a `force` query parameter to force removal of volumes that were already removed out of band by the volume driver plugin. * `POST /containers/create/` and `POST /containers/(name)/update` now validates restart policies. * `POST /containers/create` now validates IPAMConfig in NetworkingConfig, and returns error for invalid IPv4 and IPv6 addresses (`--ip` and `--ip6` in `docker create/run`). +* `POST /containers/create` now takes a `Mounts` field in `HostConfig` which replaces `Binds` and `Volumes`. *note*: `Binds` and `Volumes` are still available but are exclusive with `Mounts` ### v1.24 API changes diff --git a/docs/reference/api/docker_remote_api_v1.24.md b/docs/reference/api/docker_remote_api_v1.24.md index d7647aa6bf..f5e6c6580c 100644 --- a/docs/reference/api/docker_remote_api_v1.24.md +++ b/docs/reference/api/docker_remote_api_v1.24.md @@ -334,7 +334,8 @@ Create a container "StorageOpt": {}, "CgroupParent": "", "VolumeDriver": "", - "ShmSize": 67108864 + "ShmSize": 67108864, + "Mounts": [] }, "NetworkingConfig": { "EndpointsConfig": { @@ -610,7 +611,8 @@ Return low-level information on the container `id` "VolumesFrom": null, "Ulimits": [{}], "VolumeDriver": "", - "ShmSize": 67108864 + "ShmSize": 67108864, + "Mounts": [] }, "HostnamePath": "/var/lib/docker/containers/ba033ac4401106a3b513bc9d639eee123ad78ca3616b921167cd74b20e25ed39/hostname", "HostsPath": "/var/lib/docker/containers/ba033ac4401106a3b513bc9d639eee123ad78ca3616b921167cd74b20e25ed39/hosts", diff --git a/docs/reference/api/docker_remote_api_v1.25.md b/docs/reference/api/docker_remote_api_v1.25.md index 85e65c3167..11e5017fc5 100644 --- a/docs/reference/api/docker_remote_api_v1.25.md +++ b/docs/reference/api/docker_remote_api_v1.25.md @@ -486,6 +486,24 @@ Create a container - **CgroupParent** - Path to `cgroups` under which the container's `cgroup` is created. If the path is not absolute, the path is considered to be relative to the `cgroups` path of the init process. Cgroups are created if they do not already exist. - **VolumeDriver** - Driver that this container users to mount volumes. - **ShmSize** - Size of `/dev/shm` in bytes. The size must be greater than 0. If omitted the system uses 64MB. + - **Mounts** – Specification for mounts to be added to the container. + - **Target** – Container path. + - **Source** – Mount source (e.g. a volume name, a host path). + - **Type** – The mount type (`bind`, or `volume`). + Available types (for the `Type` field): + - **bind** - Mounts a file or directory from the host into the container. Must exist prior to creating the container. + - **volume** - Creates a volume with the given name and options (or uses a pre-existing volume with the same name and options). These are **not** removed when the container is removed. + - **ReadOnly** – A boolean indicating whether the mount should be read-only. + - **BindOptions** - Optional configuration for the `bind` type. + - **Propagation** – A propagation mode with the value `[r]private`, `[r]shared`, or `[r]slave`. + - **VolumeOptions** – Optional configuration for the `volume` type. + - **NoCopy** – A boolean indicating if volume should be + populated with the data from the target. (Default false) + - **Labels** – User-defined name and labels for the volume as key/value pairs: `{"name": "value"}` + - **DriverConfig** – Map of driver-specific options. + - **Name** - Name of the driver to use to create the volume. + - **Options** - key/value map of driver specific options. + **Query parameters**: diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go index 53e3ec90bd..4ea520fe76 100644 --- a/integration-cli/docker_api_containers_test.go +++ b/integration-cli/docker_api_containers_test.go @@ -6,10 +6,12 @@ import ( "encoding/json" "fmt" "io" + "io/ioutil" "net/http" "net/http/httputil" "net/url" "os" + "path/filepath" "regexp" "strconv" "strings" @@ -17,10 +19,14 @@ import ( "github.com/docker/docker/api/types" containertypes "github.com/docker/docker/api/types/container" + mounttypes "github.com/docker/docker/api/types/mount" networktypes "github.com/docker/docker/api/types/network" "github.com/docker/docker/pkg/integration" "github.com/docker/docker/pkg/integration/checker" + "github.com/docker/docker/pkg/ioutils" + "github.com/docker/docker/pkg/mount" "github.com/docker/docker/pkg/stringid" + "github.com/docker/docker/volume" "github.com/go-check/check" ) @@ -1525,3 +1531,212 @@ func (s *DockerSuite) TestContainerApiStatsWithNetworkDisabled(c *check.C) { c.Assert(dec.Decode(&s), checker.IsNil) } } + +func (s *DockerSuite) TestContainersApiCreateMountsValidation(c *check.C) { + type m mounttypes.Mount + type hc struct{ Mounts []m } + type cfg struct { + Image string + HostConfig hc + } + type testCase struct { + config cfg + status int + msg string + } + + prefix, slash := getPrefixAndSlashFromDaemonPlatform() + destPath := prefix + slash + "foo" + notExistPath := prefix + slash + "notexist" + + cases := []testCase{ + {cfg{Image: "busybox", HostConfig: hc{Mounts: []m{{Type: "notreal", Target: destPath}}}}, http.StatusBadRequest, "mount type unknown"}, + {cfg{Image: "busybox", HostConfig: hc{Mounts: []m{{Type: "bind"}}}}, http.StatusBadRequest, "Target must not be empty"}, + {cfg{Image: "busybox", HostConfig: hc{Mounts: []m{{Type: "bind", Target: destPath}}}}, http.StatusBadRequest, "Source must not be empty"}, + {cfg{Image: "busybox", HostConfig: hc{Mounts: []m{{Type: "bind", Source: notExistPath, Target: destPath}}}}, http.StatusBadRequest, "bind source path does not exist"}, + {cfg{Image: "busybox", HostConfig: hc{Mounts: []m{{Type: "volume"}}}}, http.StatusBadRequest, "Target must not be empty"}, + {cfg{Image: "busybox", HostConfig: hc{Mounts: []m{{Type: "volume", Source: "hello", Target: destPath}}}}, http.StatusCreated, ""}, + {cfg{Image: "busybox", HostConfig: hc{Mounts: []m{{Type: "volume", Source: "hello2", Target: destPath, VolumeOptions: &mounttypes.VolumeOptions{DriverConfig: &mounttypes.Driver{Name: "local"}}}}}}, http.StatusCreated, ""}, + } + + if SameHostDaemon.Condition() { + tmpDir, err := ioutils.TempDir("", "test-mounts-api") + c.Assert(err, checker.IsNil) + defer os.RemoveAll(tmpDir) + cases = append(cases, []testCase{ + {cfg{Image: "busybox", HostConfig: hc{Mounts: []m{{Type: "bind", Source: tmpDir, Target: destPath}}}}, http.StatusCreated, ""}, + {cfg{Image: "busybox", HostConfig: hc{Mounts: []m{{Type: "bind", Source: tmpDir, Target: destPath, VolumeOptions: &mounttypes.VolumeOptions{}}}}}, http.StatusBadRequest, "VolumeOptions must not be specified"}, + }...) + } + + if DaemonIsLinux.Condition() { + cases = append(cases, []testCase{ + {cfg{Image: "busybox", HostConfig: hc{Mounts: []m{{Type: "volume", Source: "hello3", Target: destPath, VolumeOptions: &mounttypes.VolumeOptions{DriverConfig: &mounttypes.Driver{Name: "local", Options: map[string]string{"o": "size=1"}}}}}}}, http.StatusCreated, ""}, + }...) + + } + + for i, x := range cases { + c.Logf("case %d", i) + status, b, err := sockRequest("POST", "/containers/create", x.config) + c.Assert(err, checker.IsNil) + c.Assert(status, checker.Equals, x.status, check.Commentf("%s\n%v", string(b), cases[i].config)) + if len(x.msg) > 0 { + c.Assert(string(b), checker.Contains, x.msg, check.Commentf("%v", cases[i].config)) + } + } +} + +func (s *DockerSuite) TestContainerApiCreateMountsBindRead(c *check.C) { + testRequires(c, NotUserNamespace, SameHostDaemon) + // also with data in the host side + prefix, slash := getPrefixAndSlashFromDaemonPlatform() + destPath := prefix + slash + "foo" + tmpDir, err := ioutil.TempDir("", "test-mounts-api-bind") + c.Assert(err, checker.IsNil) + defer os.RemoveAll(tmpDir) + err = ioutil.WriteFile(filepath.Join(tmpDir, "bar"), []byte("hello"), 666) + c.Assert(err, checker.IsNil) + + data := map[string]interface{}{ + "Image": "busybox", + "Cmd": []string{"/bin/sh", "-c", "cat /foo/bar"}, + "HostConfig": map[string]interface{}{"Mounts": []map[string]interface{}{{"Type": "bind", "Source": tmpDir, "Target": destPath}}}, + } + status, resp, err := sockRequest("POST", "/containers/create?name=test", data) + c.Assert(err, checker.IsNil, check.Commentf(string(resp))) + c.Assert(status, checker.Equals, http.StatusCreated, check.Commentf(string(resp))) + + out, _ := dockerCmd(c, "start", "-a", "test") + c.Assert(out, checker.Equals, "hello") +} + +// Test Mounts comes out as expected for the MountPoint +func (s *DockerSuite) TestContainersApiCreateMountsCreate(c *check.C) { + prefix, slash := getPrefixAndSlashFromDaemonPlatform() + destPath := prefix + slash + "foo" + + var ( + err error + testImg string + ) + if daemonPlatform != "windows" { + testImg, err = buildImage("test-mount-config", ` + FROM busybox + RUN mkdir `+destPath+` && touch `+destPath+slash+`bar + CMD cat `+destPath+slash+`bar + `, true) + } else { + testImg = "busybox" + } + c.Assert(err, checker.IsNil) + + type testCase struct { + cfg mounttypes.Mount + expected types.MountPoint + } + + cases := []testCase{ + // use literal strings here for `Type` instead of the defined constants in the volume package to keep this honest + // Validation of the actual `Mount` struct is done in another test is not needed here + {mounttypes.Mount{Type: "volume", Target: destPath}, types.MountPoint{Driver: volume.DefaultDriverName, Type: "volume", RW: true, Destination: destPath}}, + {mounttypes.Mount{Type: "volume", Target: destPath + slash}, types.MountPoint{Driver: volume.DefaultDriverName, Type: "volume", RW: true, Destination: destPath}}, + {mounttypes.Mount{Type: "volume", Target: destPath, Source: "test1"}, types.MountPoint{Type: "volume", Name: "test1", RW: true, Destination: destPath}}, + {mounttypes.Mount{Type: "volume", Target: destPath, ReadOnly: true, Source: "test2"}, types.MountPoint{Type: "volume", Name: "test2", RW: false, Destination: destPath}}, + {mounttypes.Mount{Type: "volume", Target: destPath, Source: "test3", VolumeOptions: &mounttypes.VolumeOptions{DriverConfig: &mounttypes.Driver{Name: volume.DefaultDriverName}}}, types.MountPoint{Driver: volume.DefaultDriverName, Type: "volume", Name: "test3", RW: true, Destination: destPath}}, + } + + if SameHostDaemon.Condition() { + // setup temp dir for testing binds + tmpDir1, err := ioutil.TempDir("", "test-mounts-api-1") + c.Assert(err, checker.IsNil) + defer os.RemoveAll(tmpDir1) + cases = append(cases, []testCase{ + {mounttypes.Mount{Type: "bind", Source: tmpDir1, Target: destPath}, types.MountPoint{Type: "bind", RW: true, Destination: destPath, Source: tmpDir1}}, + {mounttypes.Mount{Type: "bind", Source: tmpDir1, Target: destPath, ReadOnly: true}, types.MountPoint{Type: "bind", RW: false, Destination: destPath, Source: tmpDir1}}, + }...) + + // for modes only supported on Linux + if DaemonIsLinux.Condition() { + tmpDir3, err := ioutils.TempDir("", "test-mounts-api-3") + c.Assert(err, checker.IsNil) + defer os.RemoveAll(tmpDir3) + + c.Assert(mount.Mount(tmpDir3, tmpDir3, "none", "bind,rw"), checker.IsNil) + c.Assert(mount.ForceMount("", tmpDir3, "none", "shared"), checker.IsNil) + + cases = append(cases, []testCase{ + {mounttypes.Mount{Type: "bind", Source: tmpDir3, Target: destPath}, types.MountPoint{Type: "bind", RW: true, Destination: destPath, Source: tmpDir3}}, + {mounttypes.Mount{Type: "bind", Source: tmpDir3, Target: destPath, ReadOnly: true}, types.MountPoint{Type: "bind", RW: false, Destination: destPath, Source: tmpDir3}}, + {mounttypes.Mount{Type: "bind", Source: tmpDir3, Target: destPath, ReadOnly: true, BindOptions: &mounttypes.BindOptions{Propagation: "shared"}}, types.MountPoint{Type: "bind", RW: false, Destination: destPath, Source: tmpDir3, Propagation: "shared"}}, + }...) + } + } + + if daemonPlatform != "windows" { // Windows does not support volume populate + cases = append(cases, []testCase{ + {mounttypes.Mount{Type: "volume", Target: destPath, VolumeOptions: &mounttypes.VolumeOptions{NoCopy: true}}, types.MountPoint{Driver: volume.DefaultDriverName, Type: "volume", RW: true, Destination: destPath}}, + {mounttypes.Mount{Type: "volume", Target: destPath + slash, VolumeOptions: &mounttypes.VolumeOptions{NoCopy: true}}, types.MountPoint{Driver: volume.DefaultDriverName, Type: "volume", RW: true, Destination: destPath}}, + {mounttypes.Mount{Type: "volume", Target: destPath, Source: "test4", VolumeOptions: &mounttypes.VolumeOptions{NoCopy: true}}, types.MountPoint{Type: "volume", Name: "test4", RW: true, Destination: destPath}}, + {mounttypes.Mount{Type: "volume", Target: destPath, Source: "test5", ReadOnly: true, VolumeOptions: &mounttypes.VolumeOptions{NoCopy: true}}, types.MountPoint{Type: "volume", Name: "test5", RW: false, Destination: destPath}}, + }...) + } + + type wrapper struct { + containertypes.Config + HostConfig containertypes.HostConfig + } + type createResp struct { + ID string `json:"Id"` + } + for i, x := range cases { + c.Logf("case %d - config: %v", i, x.cfg) + status, data, err := sockRequest("POST", "/containers/create", wrapper{containertypes.Config{Image: testImg}, containertypes.HostConfig{Mounts: []mounttypes.Mount{x.cfg}}}) + c.Assert(err, checker.IsNil, check.Commentf(string(data))) + c.Assert(status, checker.Equals, http.StatusCreated, check.Commentf(string(data))) + + var resp createResp + err = json.Unmarshal(data, &resp) + c.Assert(err, checker.IsNil, check.Commentf(string(data))) + id := resp.ID + + var mps []types.MountPoint + err = json.NewDecoder(strings.NewReader(inspectFieldJSON(c, id, "Mounts"))).Decode(&mps) + c.Assert(err, checker.IsNil) + c.Assert(mps, checker.HasLen, 1) + c.Assert(mps[0].Destination, checker.Equals, x.expected.Destination) + + if len(x.expected.Source) > 0 { + c.Assert(mps[0].Source, checker.Equals, x.expected.Source) + } + if len(x.expected.Name) > 0 { + c.Assert(mps[0].Name, checker.Equals, x.expected.Name) + } + if len(x.expected.Driver) > 0 { + c.Assert(mps[0].Driver, checker.Equals, x.expected.Driver) + } + c.Assert(mps[0].RW, checker.Equals, x.expected.RW) + c.Assert(mps[0].Type, checker.Equals, x.expected.Type) + c.Assert(mps[0].Mode, checker.Equals, x.expected.Mode) + if len(x.expected.Propagation) > 0 { + c.Assert(mps[0].Propagation, checker.Equals, x.expected.Propagation) + } + + out, _, err := dockerCmdWithError("start", "-a", id) + if (x.cfg.Type != "volume" || (x.cfg.VolumeOptions != nil && x.cfg.VolumeOptions.NoCopy)) && daemonPlatform != "windows" { + c.Assert(err, checker.NotNil, check.Commentf("%s\n%v", out, mps[0])) + } else { + c.Assert(err, checker.IsNil, check.Commentf("%s\n%v", out, mps[0])) + } + + dockerCmd(c, "rm", "-fv", id) + if x.cfg.Type == "volume" && len(x.cfg.Source) > 0 { + // This should still exist even though we removed the container + dockerCmd(c, "volume", "inspect", mps[0].Name) + } else { + // This should be removed automatically when we removed the container + out, _, err := dockerCmdWithError("volume", "inspect", mps[0].Name) + c.Assert(err, checker.NotNil, check.Commentf(out)) + } + } +} diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index f5b1eb9d81..325cab57d2 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -4272,15 +4272,9 @@ func (s *DockerSuite) TestRunVolumesMountedAsSlave(c *check.C) { func (s *DockerSuite) TestRunNamedVolumesMountedAsShared(c *check.C) { testRequires(c, DaemonIsLinux, NotUserNamespace) - out, exitcode, _ := dockerCmdWithError("run", "-v", "foo:/test:shared", "busybox", "touch", "/test/somefile") - - if exitcode == 0 { - c.Fatalf("expected non-zero exit code; received %d", exitcode) - } - - if expected := "Invalid volume specification"; !strings.Contains(out, expected) { - c.Fatalf(`Expected %q in output; got: %s`, expected, out) - } + out, exitCode, _ := dockerCmdWithError("run", "-v", "foo:/test:shared", "busybox", "touch", "/test/somefile") + c.Assert(exitCode, checker.Not(checker.Equals), 0) + c.Assert(out, checker.Contains, "invalid mount config") } func (s *DockerSuite) TestRunNamedVolumeCopyImageData(c *check.C) { diff --git a/runconfig/config.go b/runconfig/config.go index e55b9589c7..a663a47a8e 100644 --- a/runconfig/config.go +++ b/runconfig/config.go @@ -73,16 +73,29 @@ func DecodeContainerConfig(src io.Reader) (*container.Config, *container.HostCon // validateVolumesAndBindSettings validates each of the volumes and bind settings // passed by the caller to ensure they are valid. func validateVolumesAndBindSettings(c *container.Config, hc *container.HostConfig) error { + if len(hc.Mounts) > 0 { + if len(hc.Binds) > 0 { + return conflictError(fmt.Errorf("must not specify both Binds and Mounts")) + } + + if len(c.Volumes) > 0 { + return conflictError(fmt.Errorf("must not specify both Volumes and Mounts")) + } + + if len(hc.VolumeDriver) > 0 { + return conflictError(fmt.Errorf("must not specify both VolumeDriver and Mounts")) + } + } // Ensure all volumes and binds are valid. for spec := range c.Volumes { - if _, err := volume.ParseMountSpec(spec, hc.VolumeDriver); err != nil { - return fmt.Errorf("Invalid volume spec %q: %v", spec, err) + if _, err := volume.ParseMountRaw(spec, hc.VolumeDriver); err != nil { + return fmt.Errorf("invalid volume spec %q: %v", spec, err) } } for _, spec := range hc.Binds { - if _, err := volume.ParseMountSpec(spec, hc.VolumeDriver); err != nil { - return fmt.Errorf("Invalid bind mount spec %q: %v", spec, err) + if _, err := volume.ParseMountRaw(spec, hc.VolumeDriver); err != nil { + return fmt.Errorf("invalid bind mount spec %q: %v", spec, err) } } diff --git a/runconfig/errors.go b/runconfig/errors.go index d3608576e2..6602937233 100644 --- a/runconfig/errors.go +++ b/runconfig/errors.go @@ -2,6 +2,8 @@ package runconfig import ( "fmt" + + "github.com/docker/docker/errors" ) var ( @@ -38,3 +40,7 @@ var ( // ErrConflictUTSHostname conflict between the hostname and the UTS mode ErrConflictUTSHostname = fmt.Errorf("Conflicting options: hostname and the UTS mode") ) + +func conflictError(err error) error { + return errors.NewRequestConflictError(err) +} diff --git a/volume/validate.go b/volume/validate.go new file mode 100644 index 0000000000..ddba14fbc5 --- /dev/null +++ b/volume/validate.go @@ -0,0 +1,118 @@ +package volume + +import ( + "errors" + "fmt" + "os" + "path/filepath" + + "github.com/docker/docker/api/types/mount" +) + +var errBindNotExist = errors.New("bind source path does not exist") + +type validateOpts struct { + skipBindSourceCheck bool + skipAbsolutePathCheck bool +} + +func validateMountConfig(mnt *mount.Mount, options ...func(*validateOpts)) error { + opts := validateOpts{} + for _, o := range options { + o(&opts) + } + + if len(mnt.Target) == 0 { + return &errMountConfig{mnt, errMissingField("Target")} + } + + if err := validateNotRoot(mnt.Target); err != nil { + return &errMountConfig{mnt, err} + } + + if !opts.skipAbsolutePathCheck { + if err := validateAbsolute(mnt.Target); err != nil { + return &errMountConfig{mnt, err} + } + } + + switch mnt.Type { + case mount.TypeBind: + if len(mnt.Source) == 0 { + return &errMountConfig{mnt, errMissingField("Source")} + } + // Don't error out just because the propagation mode is not supported on the platform + if opts := mnt.BindOptions; opts != nil { + if len(opts.Propagation) > 0 && len(propagationModes) > 0 { + if _, ok := propagationModes[opts.Propagation]; !ok { + return &errMountConfig{mnt, fmt.Errorf("invalid propagation mode: %s", opts.Propagation)} + } + } + } + if mnt.VolumeOptions != nil { + return &errMountConfig{mnt, errExtraField("VolumeOptions")} + } + + if err := validateAbsolute(mnt.Source); err != nil { + return &errMountConfig{mnt, err} + } + + // Do not allow binding to non-existent path + if !opts.skipBindSourceCheck { + fi, err := os.Stat(mnt.Source) + if err != nil { + if !os.IsNotExist(err) { + return &errMountConfig{mnt, err} + } + return &errMountConfig{mnt, errBindNotExist} + } + if err := validateStat(fi); err != nil { + return &errMountConfig{mnt, err} + } + } + case mount.TypeVolume: + if mnt.BindOptions != nil { + return &errMountConfig{mnt, errExtraField("BindOptions")} + } + + if len(mnt.Source) == 0 && mnt.ReadOnly { + return &errMountConfig{mnt, fmt.Errorf("must not set ReadOnly mode when using anonymous volumes")} + } + + if len(mnt.Source) != 0 { + if valid, err := IsVolumeNameValid(mnt.Source); !valid { + if err == nil { + err = errors.New("invalid volume name") + } + return &errMountConfig{mnt, err} + } + } + default: + return &errMountConfig{mnt, errors.New("mount type unknown")} + } + return nil +} + +type errMountConfig struct { + mount *mount.Mount + err error +} + +func (e *errMountConfig) Error() string { + return fmt.Sprintf("invalid mount config for type %q: %v", e.mount.Type, e.err.Error()) +} + +func errExtraField(name string) error { + return fmt.Errorf("field %s must not be specified", name) +} +func errMissingField(name string) error { + return fmt.Errorf("field %s must not be empty", name) +} + +func validateAbsolute(p string) error { + p = convertSlash(p) + if filepath.IsAbs(p) { + return nil + } + return fmt.Errorf("invalid mount path: '%s' mount path must be absolute", p) +} diff --git a/volume/validate_test.go b/volume/validate_test.go new file mode 100644 index 0000000000..8732500fc0 --- /dev/null +++ b/volume/validate_test.go @@ -0,0 +1,43 @@ +package volume + +import ( + "errors" + "io/ioutil" + "os" + "strings" + "testing" + + "github.com/docker/docker/api/types/mount" +) + +func TestValidateMount(t *testing.T) { + testDir, err := ioutil.TempDir("", "test-validate-mount") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(testDir) + + cases := []struct { + input mount.Mount + expected error + }{ + {mount.Mount{Type: mount.TypeVolume}, errMissingField("Target")}, + {mount.Mount{Type: mount.TypeVolume, Target: testDestinationPath, Source: "hello"}, nil}, + {mount.Mount{Type: mount.TypeVolume, Target: testDestinationPath}, nil}, + {mount.Mount{Type: mount.TypeBind}, errMissingField("Target")}, + {mount.Mount{Type: mount.TypeBind, Target: testDestinationPath}, errMissingField("Source")}, + {mount.Mount{Type: mount.TypeBind, Target: testDestinationPath, Source: testSourcePath, VolumeOptions: &mount.VolumeOptions{}}, errExtraField("VolumeOptions")}, + {mount.Mount{Type: mount.TypeBind, Source: testSourcePath, Target: testDestinationPath}, errBindNotExist}, + {mount.Mount{Type: mount.TypeBind, Source: testDir, Target: testDestinationPath}, nil}, + {mount.Mount{Type: "invalid", Target: testDestinationPath}, errors.New("mount type unknown")}, + } + for i, x := range cases { + err := validateMountConfig(&x.input) + if err == nil && x.expected == nil { + continue + } + if (err == nil && x.expected != nil) || (x.expected == nil && err != nil) || !strings.Contains(err.Error(), x.expected.Error()) { + t.Fatalf("expected %q, got %q, case: %d", x.expected, err, i) + } + } +} diff --git a/volume/validate_test_unix.go b/volume/validate_test_unix.go new file mode 100644 index 0000000000..dd1de2f643 --- /dev/null +++ b/volume/validate_test_unix.go @@ -0,0 +1,8 @@ +// +build !windows + +package volume + +var ( + testDestinationPath = "/foo" + testSourcePath = "/foo" +) diff --git a/volume/validate_test_windows.go b/volume/validate_test_windows.go new file mode 100644 index 0000000000..d5f86ac850 --- /dev/null +++ b/volume/validate_test_windows.go @@ -0,0 +1,6 @@ +package volume + +var ( + testDestinationPath = `c:\foo` + testSourcePath = `c:\foo` +) diff --git a/volume/volume.go b/volume/volume.go index 5385a94545..ed1eb8d11f 100644 --- a/volume/volume.go +++ b/volume/volume.go @@ -3,6 +3,7 @@ package volume import ( "fmt" "os" + "path/filepath" "strings" "syscall" @@ -82,19 +83,19 @@ type ScopedVolume interface { // specifies which volume is to be used and where inside a container it should // be mounted. type MountPoint struct { - Source string // Container host directory - Destination string // Inside the container - RW bool // True if writable - Name string // Name set by user - Driver string // Volume driver to use - Volume Volume `json:"-"` + Source string // Container host directory + Destination string // Inside the container + RW bool // True if writable + Name string // Name set by user + Driver string // Volume driver to use + Type mounttypes.Type `json:",omitempty"` // Type of mount to use, see `Type` definitions + Volume Volume `json:"-"` // Note Mode is not used on Windows - Mode string `json:"Relabel"` // Originally field was `Relabel`" + Mode string `json:"Relabel,omitempty"` // Originally field was `Relabel`" // Note Propagation is not used on Windows - Propagation mounttypes.Propagation // Mount propagation string - Named bool // specifies if the mountpoint was specified by name + Propagation mounttypes.Propagation `json:",omitempty"` // Mount propagation string // Specifies if data should be copied from the container before the first mount // Use a pointer here so we can tell if the user set this value explicitly @@ -102,7 +103,8 @@ type MountPoint struct { CopyData bool `json:"-"` // ID is the opaque ID used to pass to the volume driver. // This should be set by calls to `Mount` and unset by calls to `Unmount` - ID string + ID string `json:",omitempty"` + Spec mounttypes.Mount } // Setup sets up a mount point by either mounting the volume if it is @@ -117,12 +119,15 @@ func (m *MountPoint) Setup(mountLabel string, rootUID, rootGID int) (string, err if len(m.Source) == 0 { return "", fmt.Errorf("Unable to setup mount point, neither source nor volume defined") } - // idtools.MkdirAllNewAs() produces an error if m.Source exists and is a file (not a directory) - // also, makes sure that if the directory is created, the correct remapped rootUID/rootGID will own it - if err := idtools.MkdirAllNewAs(m.Source, 0755, rootUID, rootGID); err != nil { - if perr, ok := err.(*os.PathError); ok { - if perr.Err != syscall.ENOTDIR { - return "", err + // system.MkdirAll() produces an error if m.Source exists and is a file (not a directory), + if m.Type == mounttypes.TypeBind { + // idtools.MkdirAllNewAs() produces an error if m.Source exists and is a file (not a directory) + // also, makes sure that if the directory is created, the correct remapped rootUID/rootGID will own it + if err := idtools.MkdirAllNewAs(m.Source, 0755, rootUID, rootGID); err != nil { + if perr, ok := err.(*os.PathError); ok { + if perr.Err != syscall.ENOTDIR { + return "", err + } } } } @@ -142,17 +147,6 @@ func (m *MountPoint) Path() string { return m.Source } -// Type returns the type of mount point -func (m *MountPoint) Type() string { - if m.Name != "" { - return "volume" - } - if m.Source != "" { - return "bind" - } - return "ephemeral" -} - // ParseVolumesFrom ensures that the supplied volumes-from is valid. func ParseVolumesFrom(spec string) (string, string, error) { if len(spec) == 0 { @@ -183,10 +177,125 @@ func ParseVolumesFrom(spec string) (string, string, error) { return id, mode, nil } +// ParseMountRaw parses a raw volume spec (e.g. `-v /foo:/bar:shared`) into a +// structured spec. Once the raw spec is parsed it relies on `ParseMountSpec` to +// validate the spec and create a MountPoint +func ParseMountRaw(raw, volumeDriver string) (*MountPoint, error) { + arr, err := splitRawSpec(convertSlash(raw)) + if err != nil { + return nil, err + } + + var spec mounttypes.Mount + var mode string + switch len(arr) { + case 1: + // Just a destination path in the container + spec.Target = arr[0] + case 2: + if ValidMountMode(arr[1]) { + // Destination + Mode is not a valid volume - volumes + // cannot include a mode. eg /foo:rw + return nil, errInvalidSpec(raw) + } + // Host Source Path or Name + Destination + spec.Source = arr[0] + spec.Target = arr[1] + case 3: + // HostSourcePath+DestinationPath+Mode + spec.Source = arr[0] + spec.Target = arr[1] + mode = arr[2] + default: + return nil, errInvalidSpec(raw) + } + + if !ValidMountMode(mode) { + return nil, errInvalidMode(mode) + } + + if filepath.IsAbs(spec.Source) { + spec.Type = mounttypes.TypeBind + } else { + spec.Type = mounttypes.TypeVolume + } + + spec.ReadOnly = !ReadWrite(mode) + + // cannot assume that if a volume driver is passed in that we should set it + if volumeDriver != "" && spec.Type == mounttypes.TypeVolume { + spec.VolumeOptions = &mounttypes.VolumeOptions{ + DriverConfig: &mounttypes.Driver{Name: volumeDriver}, + } + } + + if copyData, isSet := getCopyMode(mode); isSet { + if spec.VolumeOptions == nil { + spec.VolumeOptions = &mounttypes.VolumeOptions{} + } + spec.VolumeOptions.NoCopy = !copyData + } + if HasPropagation(mode) { + spec.BindOptions = &mounttypes.BindOptions{ + Propagation: GetPropagation(mode), + } + } + + mp, err := ParseMountSpec(spec, platformRawValidationOpts...) + if mp != nil { + mp.Mode = mode + } + if err != nil { + err = fmt.Errorf("%v: %v", errInvalidSpec(raw), err) + } + return mp, err +} + +// ParseMountSpec reads a mount config, validates it, and configures a mountpoint from it. +func ParseMountSpec(cfg mounttypes.Mount, options ...func(*validateOpts)) (*MountPoint, error) { + if err := validateMountConfig(&cfg, options...); err != nil { + return nil, err + } + mp := &MountPoint{ + RW: !cfg.ReadOnly, + Destination: clean(convertSlash(cfg.Target)), + Type: cfg.Type, + Spec: cfg, + } + + switch cfg.Type { + case mounttypes.TypeVolume: + if cfg.Source == "" { + mp.Name = stringid.GenerateNonCryptoID() + } else { + mp.Name = cfg.Source + } + mp.CopyData = DefaultCopyMode + + mp.Driver = DefaultDriverName + if cfg.VolumeOptions != nil { + if cfg.VolumeOptions.DriverConfig != nil { + mp.Driver = cfg.VolumeOptions.DriverConfig.Name + } + if cfg.VolumeOptions.NoCopy { + mp.CopyData = false + } + } + case mounttypes.TypeBind: + mp.Source = clean(convertSlash(cfg.Source)) + if cfg.BindOptions != nil { + if len(cfg.BindOptions.Propagation) > 0 { + mp.Propagation = cfg.BindOptions.Propagation + } + } + } + return mp, nil +} + func errInvalidMode(mode string) error { return fmt.Errorf("invalid mode: %v", mode) } func errInvalidSpec(spec string) error { - return fmt.Errorf("Invalid volume specification: '%s'", spec) + return fmt.Errorf("invalid volume specification: '%s'", spec) } diff --git a/volume/volume_copy.go b/volume/volume_copy.go index 067537fb7d..77f06a0d1f 100644 --- a/volume/volume_copy.go +++ b/volume/volume_copy.go @@ -2,11 +2,6 @@ package volume import "strings" -const ( - // DefaultCopyMode is the copy mode used by default for normal/named volumes - DefaultCopyMode = true -) - // {=isEnabled} var copyModes = map[string]bool{ "nocopy": false, diff --git a/volume/volume_copy_unix.go b/volume/volume_copy_unix.go new file mode 100644 index 0000000000..ad66e17637 --- /dev/null +++ b/volume/volume_copy_unix.go @@ -0,0 +1,8 @@ +// +build !windows + +package volume + +const ( + // DefaultCopyMode is the copy mode used by default for normal/named volumes + DefaultCopyMode = true +) diff --git a/volume/volume_copy_windows.go b/volume/volume_copy_windows.go new file mode 100644 index 0000000000..798638c878 --- /dev/null +++ b/volume/volume_copy_windows.go @@ -0,0 +1,6 @@ +package volume + +const ( + // DefaultCopyMode is the copy mode used by default for normal/named volumes + DefaultCopyMode = false +) diff --git a/volume/volume_propagation_linux.go b/volume/volume_propagation_linux.go index 77e863555f..1de57ab52b 100644 --- a/volume/volume_propagation_linux.go +++ b/volume/volume_propagation_linux.go @@ -10,9 +10,9 @@ import ( // DefaultPropagationMode defines what propagation mode should be used by // default if user has not specified one explicitly. -const DefaultPropagationMode mounttypes.Propagation = "rprivate" - // propagation modes +const DefaultPropagationMode = mounttypes.PropagationRPrivate + var propagationModes = map[mounttypes.Propagation]bool{ mounttypes.PropagationPrivate: true, mounttypes.PropagationRPrivate: true, diff --git a/volume/volume_propagation_linux_test.go b/volume/volume_propagation_linux_test.go index e579fa05ff..46d0265062 100644 --- a/volume/volume_propagation_linux_test.go +++ b/volume/volume_propagation_linux_test.go @@ -7,7 +7,7 @@ import ( "testing" ) -func TestParseMountSpecPropagation(t *testing.T) { +func TestParseMountRawPropagation(t *testing.T) { var ( valid []string invalid map[string]string @@ -34,31 +34,31 @@ func TestParseMountSpecPropagation(t *testing.T) { "/hostPath:/containerPath:ro,Z,rprivate", } invalid = map[string]string{ - "/path:/path:ro,rshared,rslave": `invalid mode: ro,rshared,rslave`, - "/path:/path:ro,z,rshared,rslave": `invalid mode: ro,z,rshared,rslave`, - "/path:shared": "Invalid volume specification", - "/path:slave": "Invalid volume specification", - "/path:private": "Invalid volume specification", - "name:/absolute-path:shared": "Invalid volume specification", - "name:/absolute-path:rshared": "Invalid volume specification", - "name:/absolute-path:slave": "Invalid volume specification", - "name:/absolute-path:rslave": "Invalid volume specification", - "name:/absolute-path:private": "Invalid volume specification", - "name:/absolute-path:rprivate": "Invalid volume specification", + "/path:/path:ro,rshared,rslave": `invalid mode`, + "/path:/path:ro,z,rshared,rslave": `invalid mode`, + "/path:shared": "invalid volume specification", + "/path:slave": "invalid volume specification", + "/path:private": "invalid volume specification", + "name:/absolute-path:shared": "invalid volume specification", + "name:/absolute-path:rshared": "invalid volume specification", + "name:/absolute-path:slave": "invalid volume specification", + "name:/absolute-path:rslave": "invalid volume specification", + "name:/absolute-path:private": "invalid volume specification", + "name:/absolute-path:rprivate": "invalid volume specification", } for _, path := range valid { - if _, err := ParseMountSpec(path, "local"); err != nil { - t.Fatalf("ParseMountSpec(`%q`) should succeed: error %q", path, err) + if _, err := ParseMountRaw(path, "local"); err != nil { + t.Fatalf("ParseMountRaw(`%q`) should succeed: error %q", path, err) } } for path, expectedError := range invalid { - if _, err := ParseMountSpec(path, "local"); err == nil { - t.Fatalf("ParseMountSpec(`%q`) should have failed validation. Err %v", path, err) + if _, err := ParseMountRaw(path, "local"); err == nil { + t.Fatalf("ParseMountRaw(`%q`) should have failed validation. Err %v", path, err) } else { if !strings.Contains(err.Error(), expectedError) { - t.Fatalf("ParseMountSpec(`%q`) error should contain %q, got %v", path, expectedError, err.Error()) + t.Fatalf("ParseMountRaw(`%q`) error should contain %q, got %v", path, expectedError, err.Error()) } } } diff --git a/volume/volume_propagation_unsupported.go b/volume/volume_propagation_unsupported.go index 3eb61f2226..7311ffc2e0 100644 --- a/volume/volume_propagation_unsupported.go +++ b/volume/volume_propagation_unsupported.go @@ -9,7 +9,7 @@ import mounttypes "github.com/docker/docker/api/types/mount" const DefaultPropagationMode mounttypes.Propagation = "" // propagation modes not supported on this platform. -var propagationModes = map[string]bool{} +var propagationModes = map[mounttypes.Propagation]bool{} // GetPropagation is not supported. Return empty string. func GetPropagation(mode string) mounttypes.Propagation { diff --git a/volume/volume_test.go b/volume/volume_test.go index 3c7cd4f9f5..61cf0a4660 100644 --- a/volume/volume_test.go +++ b/volume/volume_test.go @@ -1,12 +1,16 @@ package volume import ( + "io/ioutil" + "os" "runtime" "strings" "testing" + + "github.com/docker/docker/api/types/mount" ) -func TestParseMountSpec(t *testing.T) { +func TestParseMountRaw(t *testing.T) { var ( valid []string invalid map[string]string @@ -36,25 +40,25 @@ func TestParseMountSpec(t *testing.T) { `c:\Program Files (x86)`, // With capitals and brackets } invalid = map[string]string{ - ``: "Invalid volume specification: ", - `.`: "Invalid volume specification: ", - `..\`: "Invalid volume specification: ", - `c:\:..\`: "Invalid volume specification: ", - `c:\:d:\:xyzzy`: "Invalid volume specification: ", - `c:`: "cannot be c:", - `c:\`: `cannot be c:\`, - `c:\notexist:d:`: `The system cannot find the file specified`, - `c:\windows\system32\ntdll.dll:d:`: `Source 'c:\windows\system32\ntdll.dll' is not a directory`, - `name<:d:`: `Invalid volume specification`, - `name>:d:`: `Invalid volume specification`, - `name::d:`: `Invalid volume specification`, - `name":d:`: `Invalid volume specification`, - `name\:d:`: `Invalid volume specification`, - `name*:d:`: `Invalid volume specification`, - `name|:d:`: `Invalid volume specification`, - `name?:d:`: `Invalid volume specification`, - `name/:d:`: `Invalid volume specification`, - `d:\pathandmode:rw`: `Invalid volume specification`, + ``: "invalid volume specification: ", + `.`: "invalid volume specification: ", + `..\`: "invalid volume specification: ", + `c:\:..\`: "invalid volume specification: ", + `c:\:d:\:xyzzy`: "invalid volume specification: ", + `c:`: "cannot be `c:`", + `c:\`: "cannot be `c:`", + `c:\notexist:d:`: `source path does not exist`, + `c:\windows\system32\ntdll.dll:d:`: `source path must be a directory`, + `name<:d:`: `invalid volume specification`, + `name>:d:`: `invalid volume specification`, + `name::d:`: `invalid volume specification`, + `name":d:`: `invalid volume specification`, + `name\:d:`: `invalid volume specification`, + `name*:d:`: `invalid volume specification`, + `name|:d:`: `invalid volume specification`, + `name?:d:`: `invalid volume specification`, + `name/:d:`: `invalid volume specification`, + `d:\pathandmode:rw`: `invalid volume specification`, `con:d:`: `cannot be a reserved word for Windows filenames`, `PRN:d:`: `cannot be a reserved word for Windows filenames`, `aUx:d:`: `cannot be a reserved word for Windows filenames`, @@ -93,50 +97,50 @@ func TestParseMountSpec(t *testing.T) { "/rw:/ro", } invalid = map[string]string{ - "": "Invalid volume specification", - "./": "Invalid volume destination", - "../": "Invalid volume destination", - "/:../": "Invalid volume destination", - "/:path": "Invalid volume destination", - ":": "Invalid volume specification", - "/tmp:": "Invalid volume destination", - ":test": "Invalid volume specification", - ":/test": "Invalid volume specification", - "tmp:": "Invalid volume destination", - ":test:": "Invalid volume specification", - "::": "Invalid volume specification", - ":::": "Invalid volume specification", - "/tmp:::": "Invalid volume specification", - ":/tmp::": "Invalid volume specification", - "/path:rw": "Invalid volume specification", - "/path:ro": "Invalid volume specification", - "/rw:rw": "Invalid volume specification", - "path:ro": "Invalid volume specification", - "/path:/path:sw": `invalid mode: sw`, - "/path:/path:rwz": `invalid mode: rwz`, + "": "invalid volume specification", + "./": "mount path must be absolute", + "../": "mount path must be absolute", + "/:../": "mount path must be absolute", + "/:path": "mount path must be absolute", + ":": "invalid volume specification", + "/tmp:": "invalid volume specification", + ":test": "invalid volume specification", + ":/test": "invalid volume specification", + "tmp:": "invalid volume specification", + ":test:": "invalid volume specification", + "::": "invalid volume specification", + ":::": "invalid volume specification", + "/tmp:::": "invalid volume specification", + ":/tmp::": "invalid volume specification", + "/path:rw": "invalid volume specification", + "/path:ro": "invalid volume specification", + "/rw:rw": "invalid volume specification", + "path:ro": "invalid volume specification", + "/path:/path:sw": `invalid mode`, + "/path:/path:rwz": `invalid mode`, } } for _, path := range valid { - if _, err := ParseMountSpec(path, "local"); err != nil { - t.Fatalf("ParseMountSpec(`%q`) should succeed: error %q", path, err) + if _, err := ParseMountRaw(path, "local"); err != nil { + t.Fatalf("ParseMountRaw(`%q`) should succeed: error %q", path, err) } } for path, expectedError := range invalid { - if _, err := ParseMountSpec(path, "local"); err == nil { - t.Fatalf("ParseMountSpec(`%q`) should have failed validation. Err %v", path, err) + if mp, err := ParseMountRaw(path, "local"); err == nil { + t.Fatalf("ParseMountRaw(`%q`) should have failed validation. Err '%v' - MP: %v", path, err, mp) } else { if !strings.Contains(err.Error(), expectedError) { - t.Fatalf("ParseMountSpec(`%q`) error should contain %q, got %v", path, expectedError, err.Error()) + t.Fatalf("ParseMountRaw(`%q`) error should contain %q, got %v", path, expectedError, err.Error()) } } } } -// testParseMountSpec is a structure used by TestParseMountSpecSplit for -// specifying test cases for the ParseMountSpec() function. -type testParseMountSpec struct { +// testParseMountRaw is a structure used by TestParseMountRawSplit for +// specifying test cases for the ParseMountRaw() function. +type testParseMountRaw struct { bind string driver string expDest string @@ -147,10 +151,10 @@ type testParseMountSpec struct { fail bool } -func TestParseMountSpecSplit(t *testing.T) { - var cases []testParseMountSpec +func TestParseMountRawSplit(t *testing.T) { + var cases []testParseMountRaw if runtime.GOOS == "windows" { - cases = []testParseMountSpec{ + cases = []testParseMountRaw{ {`c:\:d:`, "local", `d:`, `c:\`, ``, "", true, false}, {`c:\:d:\`, "local", `d:\`, `c:\`, ``, "", true, false}, // TODO Windows post TP5 - Add readonly support {`c:\:d:\:ro`, "local", `d:\`, `c:\`, ``, "", false, false}, @@ -159,25 +163,26 @@ func TestParseMountSpecSplit(t *testing.T) { {`name:d::rw`, "local", `d:`, ``, `name`, "local", true, false}, {`name:d:`, "local", `d:`, ``, `name`, "local", true, false}, // TODO Windows post TP5 - Add readonly support {`name:d::ro`, "local", `d:`, ``, `name`, "local", false, false}, - {`name:c:`, "", ``, ``, ``, "", true, true}, - {`driver/name:c:`, "", ``, ``, ``, "", true, true}, + {`name:c:`, "", ``, ``, ``, DefaultDriverName, true, true}, + {`driver/name:c:`, "", ``, ``, ``, DefaultDriverName, true, true}, } } else { - cases = []testParseMountSpec{ + cases = []testParseMountRaw{ {"/tmp:/tmp1", "", "/tmp1", "/tmp", "", "", true, false}, {"/tmp:/tmp2:ro", "", "/tmp2", "/tmp", "", "", false, false}, {"/tmp:/tmp3:rw", "", "/tmp3", "/tmp", "", "", true, false}, {"/tmp:/tmp4:foo", "", "", "", "", "", false, true}, - {"name:/named1", "", "/named1", "", "name", "", true, false}, + {"name:/named1", "", "/named1", "", "name", DefaultDriverName, true, false}, {"name:/named2", "external", "/named2", "", "name", "external", true, false}, {"name:/named3:ro", "local", "/named3", "", "name", "local", false, false}, - {"local/name:/tmp:rw", "", "/tmp", "", "local/name", "", true, false}, + {"local/name:/tmp:rw", "", "/tmp", "", "local/name", DefaultDriverName, true, false}, {"/tmp:tmp", "", "", "", "", "", true, true}, } } - for _, c := range cases { - m, err := ParseMountSpec(c.bind, c.driver) + for i, c := range cases { + t.Logf("case %d", i) + m, err := ParseMountRaw(c.bind, c.driver) if c.fail { if err == nil { t.Fatalf("Expected error, was nil, for spec %s\n", c.bind) @@ -186,28 +191,79 @@ func TestParseMountSpecSplit(t *testing.T) { } if m == nil || err != nil { - t.Fatalf("ParseMountSpec failed for spec %s driver %s error %v\n", c.bind, c.driver, err.Error()) + t.Fatalf("ParseMountRaw failed for spec '%s', driver '%s', error '%v'", c.bind, c.driver, err.Error()) continue } if m.Destination != c.expDest { - t.Fatalf("Expected destination %s, was %s, for spec %s\n", c.expDest, m.Destination, c.bind) + t.Fatalf("Expected destination '%s, was %s', for spec '%s'", c.expDest, m.Destination, c.bind) } if m.Source != c.expSource { - t.Fatalf("Expected source %s, was %s, for spec %s\n", c.expSource, m.Source, c.bind) + t.Fatalf("Expected source '%s', was '%s', for spec '%s'", c.expSource, m.Source, c.bind) } if m.Name != c.expName { - t.Fatalf("Expected name %s, was %s for spec %s\n", c.expName, m.Name, c.bind) + t.Fatalf("Expected name '%s', was '%s' for spec '%s'", c.expName, m.Name, c.bind) } - if m.Driver != c.expDriver { - t.Fatalf("Expected driver %s, was %s, for spec %s\n", c.expDriver, m.Driver, c.bind) + if (m.Driver != c.expDriver) || (m.Driver == DefaultDriverName && c.expDriver == "") { + t.Fatalf("Expected driver '%s', was '%s', for spec '%s'", c.expDriver, m.Driver, c.bind) } if m.RW != c.expRW { - t.Fatalf("Expected RW %v, was %v for spec %s\n", c.expRW, m.RW, c.bind) + t.Fatalf("Expected RW '%v', was '%v' for spec '%s'", c.expRW, m.RW, c.bind) + } + } +} + +func TestParseMountSpec(t *testing.T) { + type c struct { + input mount.Mount + expected MountPoint + } + testDir, err := ioutil.TempDir("", "test-mount-config") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(testDir) + + cases := []c{ + {mount.Mount{Type: mount.TypeBind, Source: testDir, Target: testDestinationPath, ReadOnly: true}, MountPoint{Type: mount.TypeBind, Source: testDir, Destination: testDestinationPath}}, + {mount.Mount{Type: mount.TypeBind, Source: testDir, Target: testDestinationPath}, MountPoint{Type: mount.TypeBind, Source: testDir, Destination: testDestinationPath, RW: true}}, + {mount.Mount{Type: mount.TypeBind, Source: testDir + string(os.PathSeparator), Target: testDestinationPath, ReadOnly: true}, MountPoint{Type: mount.TypeBind, Source: testDir, Destination: testDestinationPath}}, + {mount.Mount{Type: mount.TypeBind, Source: testDir, Target: testDestinationPath + string(os.PathSeparator), ReadOnly: true}, MountPoint{Type: mount.TypeBind, Source: testDir, Destination: testDestinationPath}}, + {mount.Mount{Type: mount.TypeVolume, Target: testDestinationPath}, MountPoint{Type: mount.TypeVolume, Destination: testDestinationPath, RW: true, Driver: DefaultDriverName, CopyData: DefaultCopyMode}}, + {mount.Mount{Type: mount.TypeVolume, Target: testDestinationPath + string(os.PathSeparator)}, MountPoint{Type: mount.TypeVolume, Destination: testDestinationPath, RW: true, Driver: DefaultDriverName, CopyData: DefaultCopyMode}}, + } + + for i, c := range cases { + t.Logf("case %d", i) + mp, err := ParseMountSpec(c.input) + if err != nil { + t.Fatal(err) + } + + if c.expected.Type != mp.Type { + t.Fatalf("Expected mount types to match. Expected: '%s', Actual: '%s'", c.expected.Type, mp.Type) + } + if c.expected.Destination != mp.Destination { + t.Fatalf("Expected mount destination to match. Expected: '%s', Actual: '%s'", c.expected.Destination, mp.Destination) + } + if c.expected.Source != mp.Source { + t.Fatalf("Expected mount source to match. Expected: '%s', Actual: '%s'", c.expected.Source, mp.Source) + } + if c.expected.RW != mp.RW { + t.Fatalf("Expected mount writable to match. Expected: '%v', Actual: '%s'", c.expected.RW, mp.RW) + } + if c.expected.Propagation != mp.Propagation { + t.Fatalf("Expected mount propagation to match. Expected: '%v', Actual: '%s'", c.expected.Propagation, mp.Propagation) + } + if c.expected.Driver != mp.Driver { + t.Fatalf("Expected mount driver to match. Expected: '%v', Actual: '%s'", c.expected.Driver, mp.Driver) + } + if c.expected.CopyData != mp.CopyData { + t.Fatalf("Expected mount copy data to match. Expected: '%v', Actual: '%v'", c.expected.CopyData, mp.CopyData) } } } diff --git a/volume/volume_unix.go b/volume/volume_unix.go index 0803ef9cf6..0256ebb2ba 100644 --- a/volume/volume_unix.go +++ b/volume/volume_unix.go @@ -4,12 +4,20 @@ package volume import ( "fmt" + "os" "path/filepath" "strings" mounttypes "github.com/docker/docker/api/types/mount" ) +var platformRawValidationOpts = []func(o *validateOpts){ + // need to make sure to not error out if the bind source does not exist on unix + // this is supported for historical reasons, the path will be automatically + // created later. + func(o *validateOpts) { o.skipBindSourceCheck = true }, +} + // read-write modes var rwModes = map[string]bool{ "rw": true, @@ -38,103 +46,6 @@ func (m *MountPoint) HasResource(absolutePath string) bool { return err == nil && relPath != ".." && !strings.HasPrefix(relPath, fmt.Sprintf("..%c", filepath.Separator)) } -// ParseMountSpec validates the configuration of mount information is valid. -func ParseMountSpec(spec, volumeDriver string) (*MountPoint, error) { - spec = filepath.ToSlash(spec) - - mp := &MountPoint{ - RW: true, - Propagation: DefaultPropagationMode, - } - if strings.Count(spec, ":") > 2 { - return nil, errInvalidSpec(spec) - } - - arr := strings.SplitN(spec, ":", 3) - if arr[0] == "" { - return nil, errInvalidSpec(spec) - } - - switch len(arr) { - case 1: - // Just a destination path in the container - mp.Destination = filepath.Clean(arr[0]) - case 2: - if isValid := ValidMountMode(arr[1]); isValid { - // Destination + Mode is not a valid volume - volumes - // cannot include a mode. eg /foo:rw - return nil, errInvalidSpec(spec) - } - // Host Source Path or Name + Destination - mp.Source = arr[0] - mp.Destination = arr[1] - case 3: - // HostSourcePath+DestinationPath+Mode - mp.Source = arr[0] - mp.Destination = arr[1] - mp.Mode = arr[2] // Mode field is used by SELinux to decide whether to apply label - if !ValidMountMode(mp.Mode) { - return nil, errInvalidMode(mp.Mode) - } - mp.RW = ReadWrite(mp.Mode) - mp.Propagation = GetPropagation(mp.Mode) - default: - return nil, errInvalidSpec(spec) - } - - //validate the volumes destination path - mp.Destination = filepath.Clean(mp.Destination) - if !filepath.IsAbs(mp.Destination) { - return nil, fmt.Errorf("Invalid volume destination path: '%s' mount path must be absolute.", mp.Destination) - } - - // Destination cannot be "/" - if mp.Destination == "/" { - return nil, fmt.Errorf("Invalid specification: destination can't be '/' in '%s'", spec) - } - - name, source := ParseVolumeSource(mp.Source) - if len(source) == 0 { - mp.Source = "" // Clear it out as we previously assumed it was not a name - mp.Driver = volumeDriver - // Named volumes can't have propagation properties specified. - // Their defaults will be decided by docker. This is just a - // safeguard. Don't want to get into situations where named - // volumes were mounted as '[r]shared' inside container and - // container does further mounts under that volume and these - // mounts become visible on host and later original volume - // cleanup becomes an issue if container does not unmount - // submounts explicitly. - if HasPropagation(mp.Mode) { - return nil, errInvalidSpec(spec) - } - } else { - mp.Source = filepath.Clean(source) - } - - copyData, isSet := getCopyMode(mp.Mode) - // do not allow copy modes on binds - if len(name) == 0 && isSet { - return nil, errInvalidMode(mp.Mode) - } - - mp.CopyData = copyData - mp.Name = name - - return mp, nil -} - -// ParseVolumeSource parses the origin sources that's mounted into the container. -// It returns a name and a source. It looks to see if the spec passed in -// is an absolute file. If it is, it assumes the spec is a source. If not, -// it assumes the spec is a name. -func ParseVolumeSource(spec string) (string, string) { - if !filepath.IsAbs(spec) { - return spec, "" - } - return "", spec -} - // IsVolumeNameValid checks a volume name in a platform specific manner. func IsVolumeNameValid(name string) (bool, error) { return true, nil @@ -143,6 +54,10 @@ func IsVolumeNameValid(name string) (bool, error) { // ValidMountMode will make sure the mount mode is valid. // returns if it's a valid mount mode or not. func ValidMountMode(mode string) bool { + if mode == "" { + return true + } + rwModeCount := 0 labelModeCount := 0 propagationModeCount := 0 @@ -183,6 +98,41 @@ func ReadWrite(mode string) bool { return false } } - return true } + +func validateNotRoot(p string) error { + p = filepath.Clean(convertSlash(p)) + if p == "/" { + return fmt.Errorf("invalid specification: destination can't be '/'") + } + return nil +} + +func validateCopyMode(mode bool) error { + return nil +} + +func convertSlash(p string) string { + return filepath.ToSlash(p) +} + +func splitRawSpec(raw string) ([]string, error) { + if strings.Count(raw, ":") > 2 { + return nil, errInvalidSpec(raw) + } + + arr := strings.SplitN(raw, ":", 3) + if arr[0] == "" { + return nil, errInvalidSpec(raw) + } + return arr, nil +} + +func clean(p string) string { + return filepath.Clean(p) +} + +func validateStat(fi os.FileInfo) error { + return nil +} diff --git a/volume/volume_windows.go b/volume/volume_windows.go index 23e70905d0..64cb520068 100644 --- a/volume/volume_windows.go +++ b/volume/volume_windows.go @@ -7,7 +7,6 @@ import ( "regexp" "strings" - "github.com/Sirupsen/logrus" "github.com/docker/docker/pkg/system" ) @@ -21,6 +20,15 @@ var roModes = map[string]bool{ "ro": true, } +var platformRawValidationOpts = []func(*validateOpts){ + // filepath.IsAbs is weird on Windows: + // `c:` is not considered an absolute path + // `c:\` is considered an absolute path + // In any case, the regex matching below ensures absolute paths + // TODO: consider this a bug with filepath.IsAbs (?) + func(o *validateOpts) { o.skipAbsolutePathCheck = true }, +} + const ( // Spec should be in the format [source:]destination[:mode] // @@ -94,109 +102,54 @@ func (m *MountPoint) BackwardsCompatible() bool { return false } -// ParseMountSpec validates the configuration of mount information is valid. -func ParseMountSpec(spec string, volumeDriver string) (*MountPoint, error) { - var specExp = regexp.MustCompile(`^` + RXSource + RXDestination + RXMode + `$`) - - // Ensure in platform semantics for matching. The CLI will send in Unix semantics. - match := specExp.FindStringSubmatch(filepath.FromSlash(strings.ToLower(spec))) +func splitRawSpec(raw string) ([]string, error) { + specExp := regexp.MustCompile(`^` + RXSource + RXDestination + RXMode + `$`) + match := specExp.FindStringSubmatch(strings.ToLower(raw)) // Must have something back if len(match) == 0 { - return nil, errInvalidSpec(spec) + return nil, errInvalidSpec(raw) } - // Pull out the sub expressions from the named capture groups + var split []string matchgroups := make(map[string]string) + // Pull out the sub expressions from the named capture groups for i, name := range specExp.SubexpNames() { matchgroups[name] = strings.ToLower(match[i]) } - - mp := &MountPoint{ - Source: matchgroups["source"], - Destination: matchgroups["destination"], - RW: true, - } - if strings.ToLower(matchgroups["mode"]) == "ro" { - mp.RW = false - } - - // Volumes cannot include an explicitly supplied mode eg c:\path:rw - if mp.Source == "" && mp.Destination != "" && matchgroups["mode"] != "" { - return nil, errInvalidSpec(spec) - } - - // Note: No need to check if destination is absolute as it must be by - // definition of matching the regex. - - if filepath.VolumeName(mp.Destination) == mp.Destination { - // Ensure the destination path, if a drive letter, is not the c drive - if strings.ToLower(mp.Destination) == "c:" { - return nil, fmt.Errorf("Destination drive letter in '%s' cannot be c:", spec) - } - } else { - // So we know the destination is a path, not drive letter. Clean it up. - mp.Destination = filepath.Clean(mp.Destination) - // Ensure the destination path, if a path, is not the c root directory - if strings.ToLower(mp.Destination) == `c:\` { - return nil, fmt.Errorf(`Destination path in '%s' cannot be c:\`, spec) + if source, exists := matchgroups["source"]; exists { + if source != "" { + split = append(split, source) } } - - // See if the source is a name instead of a host directory - if len(mp.Source) > 0 { - validName, err := IsVolumeNameValid(mp.Source) - if err != nil { - return nil, err - } - if validName { - // OK, so the source is a name. - mp.Name = mp.Source - mp.Source = "" - - // Set the driver accordingly - mp.Driver = volumeDriver - if len(mp.Driver) == 0 { - mp.Driver = DefaultDriverName - } - } else { - // OK, so the source must be a host directory. Make sure it's clean. - mp.Source = filepath.Clean(mp.Source) + if destination, exists := matchgroups["destination"]; exists { + if destination != "" { + split = append(split, destination) } } - - // Ensure the host path source, if supplied, exists and is a directory - if len(mp.Source) > 0 { - var fi os.FileInfo - var err error - if fi, err = os.Stat(mp.Source); err != nil { - return nil, fmt.Errorf("Source directory '%s' could not be found: %s", mp.Source, err) - } - if !fi.IsDir() { - return nil, fmt.Errorf("Source '%s' is not a directory", mp.Source) + if mode, exists := matchgroups["mode"]; exists { + if mode != "" { + split = append(split, mode) } } - // Fix #26329. If the destination appears to be a file, and the source is null, // it may be because we've fallen through the possible naming regex and hit a // situation where the user intention was to map a file into a container through // a local volume, but this is not supported by the platform. - if len(mp.Source) == 0 && len(mp.Destination) > 0 { - var fi os.FileInfo - var err error - if fi, err = os.Stat(mp.Destination); err == nil { - validName, err := IsVolumeNameValid(mp.Destination) - if err != nil { - return nil, err - } - if !validName && !fi.IsDir() { - return nil, fmt.Errorf("file '%s' cannot be mapped. Only directories can be mapped on this platform", mp.Destination) + if matchgroups["source"] == "" && matchgroups["destination"] != "" { + validName, err := IsVolumeNameValid(matchgroups["destination"]) + if err != nil { + return nil, err + } + if !validName { + if fi, err := os.Stat(matchgroups["destination"]); err == nil { + if !fi.IsDir() { + return nil, fmt.Errorf("file '%s' cannot be mapped. Only directories can be mapped on this platform", matchgroups["destination"]) + } } } } - - logrus.Debugf("MP: Source '%s', Dest '%s', RW %t, Name '%s', Driver '%s'", mp.Source, mp.Destination, mp.RW, mp.Name, mp.Driver) - return mp, nil + return split, nil } // IsVolumeNameValid checks a volume name in a platform specific manner. @@ -207,7 +160,7 @@ func IsVolumeNameValid(name string) (bool, error) { } nameExp = regexp.MustCompile(`^` + RXReservedNames + `$`) if nameExp.MatchString(name) { - return false, fmt.Errorf("Volume name %q cannot be a reserved word for Windows filenames", name) + return false, fmt.Errorf("volume name %q cannot be a reserved word for Windows filenames", name) } return true, nil } @@ -215,10 +168,46 @@ func IsVolumeNameValid(name string) (bool, error) { // ValidMountMode will make sure the mount mode is valid. // returns if it's a valid mount mode or not. func ValidMountMode(mode string) bool { + if mode == "" { + return true + } return roModes[strings.ToLower(mode)] || rwModes[strings.ToLower(mode)] } // ReadWrite tells you if a mode string is a valid read-write mode or not. func ReadWrite(mode string) bool { - return rwModes[strings.ToLower(mode)] + return rwModes[strings.ToLower(mode)] || mode == "" +} + +func validateNotRoot(p string) error { + p = strings.ToLower(convertSlash(p)) + if p == "c:" || p == `c:\` { + return fmt.Errorf("destination path cannot be `c:` or `c:\\`: %v", p) + } + return nil +} + +func validateCopyMode(mode bool) error { + if mode { + return fmt.Errorf("Windows does not support copying image path content") + } + return nil +} + +func convertSlash(p string) string { + return filepath.FromSlash(p) +} + +func clean(p string) string { + if match, _ := regexp.MatchString("^[a-z]:$", p); match { + return p + } + return filepath.Clean(p) +} + +func validateStat(fi os.FileInfo) error { + if !fi.IsDir() { + return fmt.Errorf("source path must be a directory") + } + return nil }