diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go index ef67894106..33559fcfb9 100644 --- a/integration-cli/docker_api_containers_test.go +++ b/integration-cli/docker_api_containers_test.go @@ -1828,6 +1828,32 @@ func (s *DockerSuite) TestContainersAPICreateMountsValidation(c *check.C) { }...) } + if DaemonIsWindows() { + cases = append(cases, []testCase{ + { + config: containertypes.Config{ + Image: "busybox", + }, + hostConfig: containertypes.HostConfig{ + Mounts: []mounttypes.Mount{ + { + Type: "volume", + Source: "not-supported-on-windows", + Target: destPath, + VolumeOptions: &mounttypes.VolumeOptions{ + DriverConfig: &mounttypes.Driver{ + Name: "local", + Options: map[string]string{"type": "tmpfs"}, + }, + }, + }, + }, + }, + msg: `options are not supported on this platform`, + }, + }...) + } + if DaemonIsLinux() { cases = append(cases, []testCase{ { @@ -1835,14 +1861,83 @@ func (s *DockerSuite) TestContainersAPICreateMountsValidation(c *check.C) { Image: "busybox", }, hostConfig: containertypes.HostConfig{ - Mounts: []mounttypes.Mount{{ - Type: "volume", - Source: "hello3", - Target: destPath, - VolumeOptions: &mounttypes.VolumeOptions{ - DriverConfig: &mounttypes.Driver{ - Name: "local", - Options: map[string]string{"o": "size=1"}}}}}}, + Mounts: []mounttypes.Mount{ + { + Type: "volume", + Source: "missing-device-opt", + Target: destPath, + VolumeOptions: &mounttypes.VolumeOptions{ + DriverConfig: &mounttypes.Driver{ + Name: "local", + Options: map[string]string{"foobar": "foobaz"}, + }, + }, + }, + }, + }, + msg: `invalid option: "foobar"`, + }, + { + config: containertypes.Config{ + Image: "busybox", + }, + hostConfig: containertypes.HostConfig{ + Mounts: []mounttypes.Mount{ + { + Type: "volume", + Source: "missing-device-opt", + Target: destPath, + VolumeOptions: &mounttypes.VolumeOptions{ + DriverConfig: &mounttypes.Driver{ + Name: "local", + Options: map[string]string{"type": "tmpfs"}, + }, + }, + }, + }, + }, + msg: `missing required option: "device"`, + }, + { + config: containertypes.Config{ + Image: "busybox", + }, + hostConfig: containertypes.HostConfig{ + Mounts: []mounttypes.Mount{ + { + Type: "volume", + Source: "missing-type-opt", + Target: destPath, + VolumeOptions: &mounttypes.VolumeOptions{ + DriverConfig: &mounttypes.Driver{ + Name: "local", + Options: map[string]string{"device": "tmpfs"}, + }, + }, + }, + }, + }, + msg: `missing required option: "type"`, + }, + { + config: containertypes.Config{ + Image: "busybox", + }, + hostConfig: containertypes.HostConfig{ + Mounts: []mounttypes.Mount{ + { + Type: "volume", + Source: "hello4", + Target: destPath, + VolumeOptions: &mounttypes.VolumeOptions{ + DriverConfig: &mounttypes.Driver{ + Name: "local", + Options: map[string]string{"o": "size=1", "type": "tmpfs", "device": "tmpfs"}, + }, + }, + }, + }, + }, msg: "", }, { @@ -1869,7 +1964,6 @@ func (s *DockerSuite) TestContainersAPICreateMountsValidation(c *check.C) { }}}}, msg: "", }, - { config: containertypes.Config{ Image: "busybox", @@ -1888,6 +1982,7 @@ func (s *DockerSuite) TestContainersAPICreateMountsValidation(c *check.C) { c.Assert(err, checker.IsNil) defer cli.Close() + // TODO add checks for statuscode returned by API for i, x := range cases { c.Logf("case %d", i) _, err = cli.ContainerCreate(context.Background(), &x.config, &x.hostConfig, &networktypes.NetworkingConfig{}, "") diff --git a/volume/local/local.go b/volume/local/local.go index d3119cb2ff..6dc894873d 100644 --- a/volume/local/local.go +++ b/volume/local/local.go @@ -248,20 +248,12 @@ func (r *Root) Scope() string { return volume.LocalScope } -type validationError string - -func (e validationError) Error() string { - return string(e) -} - -func (e validationError) InvalidParameter() {} - func (r *Root) validateName(name string) error { if len(name) == 1 { - return validationError("volume name is too short, names should be at least two alphanumeric characters") + return errdefs.InvalidParameter(errors.New("volume name is too short, names should be at least two alphanumeric characters")) } if !volumeNameRegex.MatchString(name) { - return validationError(fmt.Sprintf("%q includes invalid characters for a local volume name, only %q are allowed. If you intended to pass a host directory, use absolute path", name, names.RestrictedNameChars)) + return errdefs.InvalidParameter(errors.Errorf("%q includes invalid characters for a local volume name, only %q are allowed. If you intended to pass a host directory, use absolute path", name, names.RestrictedNameChars)) } return nil } @@ -352,15 +344,6 @@ func (v *localVolume) unmount() error { return nil } -func validateOpts(opts map[string]string) error { - for opt := range opts { - if !validOpts[opt] { - return validationError(fmt.Sprintf("invalid option key: %q", opt)) - } - } - return nil -} - func (v *localVolume) Status() map[string]interface{} { return nil } diff --git a/volume/local/local_unix.go b/volume/local/local_unix.go index 5ee2ed894b..0b1771091f 100644 --- a/volume/local/local_unix.go +++ b/volume/local/local_unix.go @@ -14,18 +14,22 @@ import ( "syscall" "time" - "github.com/pkg/errors" - + "github.com/docker/docker/errdefs" "github.com/docker/docker/pkg/mount" + "github.com/pkg/errors" ) var ( oldVfsDir = filepath.Join("vfs", "dir") - validOpts = map[string]bool{ - "type": true, // specify the filesystem type for mount, e.g. nfs - "o": true, // generic mount options - "device": true, // device to mount from + validOpts = map[string]struct{}{ + "type": {}, // specify the filesystem type for mount, e.g. nfs + "o": {}, // generic mount options + "device": {}, // device to mount from + } + mandatoryOpts = map[string]struct{}{ + "device": {}, + "type": {}, } ) @@ -71,6 +75,23 @@ func setOpts(v *localVolume, opts map[string]string) error { return nil } +func validateOpts(opts map[string]string) error { + if len(opts) == 0 { + return nil + } + for opt := range opts { + if _, ok := validOpts[opt]; !ok { + return errdefs.InvalidParameter(errors.Errorf("invalid option: %q", opt)) + } + } + for opt := range mandatoryOpts { + if _, ok := opts[opt]; !ok { + return errdefs.InvalidParameter(errors.Errorf("missing required option: %q", opt)) + } + } + return nil +} + func (v *localVolume) mount() error { if v.opts.MountDevice == "" { return fmt.Errorf("missing device in volume options") diff --git a/volume/local/local_windows.go b/volume/local/local_windows.go index d96fc0f594..d9bc8f5298 100644 --- a/volume/local/local_windows.go +++ b/volume/local/local_windows.go @@ -4,18 +4,18 @@ package local // import "github.com/docker/docker/volume/local" import ( - "fmt" "os" "path/filepath" "strings" "syscall" "time" + + "github.com/docker/docker/errdefs" + "github.com/pkg/errors" ) type optsConfig struct{} -var validOpts map[string]bool - // scopedPath verifies that the path where the volume is located // is under Docker's root and the valid local paths. func (r *Root) scopedPath(realPath string) bool { @@ -27,7 +27,7 @@ func (r *Root) scopedPath(realPath string) bool { func setOpts(v *localVolume, opts map[string]string) error { if len(opts) > 0 { - return fmt.Errorf("options are not supported on this platform") + return errdefs.InvalidParameter(errors.New("options are not supported on this platform")) } return nil }