From d5b271c155926057d33c2128ff57e49274455813 Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Mon, 9 Apr 2018 10:09:30 +0200 Subject: [PATCH] add check for local volume option Description: When using local volume option such as size=10G, type=tmpfs, if we provide wrong options, we could create volume successfully. But when we are ready to use it, it will fail to start container by failing to mount the local volume(invalid option). We should check the options at when we create it. Signed-off-by: Wentao Zhang Signed-off-by: Vincent Demeester Signed-off-by: Sebastiaan van Stijn --- integration-cli/docker_api_containers_test.go | 65 ++++++++++++++++--- volume/local/local.go | 8 +++ volume/local/local_unix.go | 4 ++ volume/local/local_windows.go | 5 +- 4 files changed, 72 insertions(+), 10 deletions(-) diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go index ef67894106..d013012c9f 100644 --- a/integration-cli/docker_api_containers_test.go +++ b/integration-cli/docker_api_containers_test.go @@ -1835,14 +1835,62 @@ 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{"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 +1917,6 @@ func (s *DockerSuite) TestContainersAPICreateMountsValidation(c *check.C) { }}}}, msg: "", }, - { config: containertypes.Config{ Image: "busybox", diff --git a/volume/local/local.go b/volume/local/local.go index d3119cb2ff..1c844cf0e3 100644 --- a/volume/local/local.go +++ b/volume/local/local.go @@ -353,11 +353,19 @@ func (v *localVolume) unmount() error { } func validateOpts(opts map[string]string) error { + if len(opts) == 0 { + return nil + } for opt := range opts { if !validOpts[opt] { return validationError(fmt.Sprintf("invalid option key: %q", opt)) } } + for opt := range mandatoryOpts { + if _, ok := opts[opt]; !ok { + return errdefs.InvalidParameter(errors.Errorf("missing required option: %q", opt)) + } + } return nil } diff --git a/volume/local/local_unix.go b/volume/local/local_unix.go index 5ee2ed894b..1d25c330da 100644 --- a/volume/local/local_unix.go +++ b/volume/local/local_unix.go @@ -27,6 +27,10 @@ var ( "o": true, // generic mount options "device": true, // device to mount from } + mandatoryOpts = map[string]struct{}{ + "device": {}, + "type": {}, + } ) type optsConfig struct { diff --git a/volume/local/local_windows.go b/volume/local/local_windows.go index d96fc0f594..5748681bb0 100644 --- a/volume/local/local_windows.go +++ b/volume/local/local_windows.go @@ -14,7 +14,10 @@ import ( type optsConfig struct{} -var validOpts map[string]bool +var ( + validOpts map[string]bool + mandatoryOpts map[string]struct{} +) // scopedPath verifies that the path where the volume is located // is under Docker's root and the valid local paths.