From 11b88be2471ed460c3b957463a40e8812f90d795 Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <github@gone.nl>
Date: Sat, 22 Dec 2018 02:28:30 +0100
Subject: [PATCH] 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>
---
 integration-cli/docker_api_containers_test.go | 48 +++++++++++++++++++
 volume/local/local.go                         | 14 ++----
 volume/local/local_unix.go                    |  3 +-
 volume/local/local_windows.go                 |  6 ++-
 4 files changed, 56 insertions(+), 15 deletions(-)

diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go
index d013012c9f..33559fcfb9 100644
--- a/integration-cli/docker_api_containers_test.go
+++ b/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() {
 		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{
 					Image: "busybox",
@@ -1935,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 6bf2aa9015..7981f5836a 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
 }
@@ -358,7 +350,7 @@ func validateOpts(opts map[string]string) error {
 	}
 	for opt := range opts {
 		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 {
diff --git a/volume/local/local_unix.go b/volume/local/local_unix.go
index e8f12c9030..62dfdc1748 100644
--- a/volume/local/local_unix.go
+++ b/volume/local/local_unix.go
@@ -14,9 +14,8 @@ import (
 	"syscall"
 	"time"
 
-	"github.com/pkg/errors"
-
 	"github.com/docker/docker/pkg/mount"
+	"github.com/pkg/errors"
 )
 
 var (
diff --git a/volume/local/local_windows.go b/volume/local/local_windows.go
index eb19e79747..cb8b632cc4 100644
--- a/volume/local/local_windows.go
+++ b/volume/local/local_windows.go
@@ -4,12 +4,14 @@
 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{}
@@ -30,7 +32,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
 }