Jelajahi Sumber

Remove validationError type, and use errdefs.InvalidParameter

Using `errors.Errorf()` passes the error with the stack trace for
debugging purposes.

Also using `errdefs.InvalidParameter` for Windows, so that the API
will return a 4xx status, instead of a 5xx, and added tests for
both validations.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sebastiaan van Stijn 6 tahun lalu
induk
melakukan
11b88be247

+ 48 - 0
integration-cli/docker_api_containers_test.go

@@ -1828,8 +1828,55 @@ 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() {
 	if DaemonIsLinux() {
 		cases = append(cases, []testCase{
 		cases = append(cases, []testCase{
+			{
+				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{"foobar": "foobaz"},
+								},
+							},
+						},
+					},
+				},
+				msg: `invalid option: "foobar"`,
+			},
 			{
 			{
 				config: containertypes.Config{
 				config: containertypes.Config{
 					Image: "busybox",
 					Image: "busybox",
@@ -1935,6 +1982,7 @@ func (s *DockerSuite) TestContainersAPICreateMountsValidation(c *check.C) {
 	c.Assert(err, checker.IsNil)
 	c.Assert(err, checker.IsNil)
 	defer cli.Close()
 	defer cli.Close()
 
 
+	// TODO add checks for statuscode returned by API
 	for i, x := range cases {
 	for i, x := range cases {
 		c.Logf("case %d", i)
 		c.Logf("case %d", i)
 		_, err = cli.ContainerCreate(context.Background(), &x.config, &x.hostConfig, &networktypes.NetworkingConfig{}, "")
 		_, err = cli.ContainerCreate(context.Background(), &x.config, &x.hostConfig, &networktypes.NetworkingConfig{}, "")

+ 3 - 11
volume/local/local.go

@@ -248,20 +248,12 @@ func (r *Root) Scope() string {
 	return volume.LocalScope
 	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 {
 func (r *Root) validateName(name string) error {
 	if len(name) == 1 {
 	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) {
 	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
 	return nil
 }
 }
@@ -358,7 +350,7 @@ func validateOpts(opts map[string]string) error {
 	}
 	}
 	for opt := range opts {
 	for opt := range opts {
 		if _, ok := validOpts[opt]; !ok {
 		if _, ok := validOpts[opt]; !ok {
-			return validationError(fmt.Sprintf("invalid option key: %q", opt))
+			return errdefs.InvalidParameter(errors.Errorf("invalid option: %q", opt))
 		}
 		}
 	}
 	}
 	for opt := range mandatoryOpts {
 	for opt := range mandatoryOpts {

+ 1 - 2
volume/local/local_unix.go

@@ -14,9 +14,8 @@ import (
 	"syscall"
 	"syscall"
 	"time"
 	"time"
 
 
-	"github.com/pkg/errors"
-
 	"github.com/docker/docker/pkg/mount"
 	"github.com/docker/docker/pkg/mount"
+	"github.com/pkg/errors"
 )
 )
 
 
 var (
 var (

+ 4 - 2
volume/local/local_windows.go

@@ -4,12 +4,14 @@
 package local // import "github.com/docker/docker/volume/local"
 package local // import "github.com/docker/docker/volume/local"
 
 
 import (
 import (
-	"fmt"
 	"os"
 	"os"
 	"path/filepath"
 	"path/filepath"
 	"strings"
 	"strings"
 	"syscall"
 	"syscall"
 	"time"
 	"time"
+
+	"github.com/docker/docker/errdefs"
+	"github.com/pkg/errors"
 )
 )
 
 
 type optsConfig struct{}
 type optsConfig struct{}
@@ -30,7 +32,7 @@ func (r *Root) scopedPath(realPath string) bool {
 
 
 func setOpts(v *localVolume, opts map[string]string) error {
 func setOpts(v *localVolume, opts map[string]string) error {
 	if len(opts) > 0 {
 	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
 	return nil
 }
 }