From 29c6224fe9eb85dd4fcfdb8589dcbe4eda2208ab Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <github@gone.nl>
Date: Sat, 14 May 2022 13:16:06 +0200
Subject: [PATCH] volume/local.Create(): validate early

This moves validation of options to the start of the Create function
to prevent hitting the filesystem and having to remove the volume
from disk.

Also addressing some minor nits w.r.t. errors.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
 volume/local/local.go            |  8 +++++---
 volume/local/local_linux_test.go | 10 ++++++++++
 volume/local/local_unix.go       | 16 +++++++++-------
 volume/local/local_windows.go    |  5 +++++
 4 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/volume/local/local.go b/volume/local/local.go
index 2fabc4f48a..479e0dfb4b 100644
--- a/volume/local/local.go
+++ b/volume/local/local.go
@@ -5,7 +5,6 @@ package local // import "github.com/docker/docker/volume/local"
 
 import (
 	"encoding/json"
-	"fmt"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -31,7 +30,7 @@ const (
 
 var (
 	// ErrNotFound is the typed error returned when the requested volume name can't be found
-	ErrNotFound = fmt.Errorf("volume not found")
+	ErrNotFound = errors.New("volume not found")
 	// volumeNameRegex ensures the name assigned for the volume is valid.
 	// This name is used to create the bind directory, so we need to avoid characters that
 	// would make the path to escape the root directory.
@@ -137,6 +136,9 @@ func (r *Root) Create(name string, opts map[string]string) (volume.Volume, error
 	if err := r.validateName(name); err != nil {
 		return nil, err
 	}
+	if err := validateOpts(opts); err != nil {
+		return nil, err
+	}
 
 	r.m.Lock()
 	defer r.m.Unlock()
@@ -204,7 +206,7 @@ func (r *Root) Remove(v volume.Volume) error {
 	}
 
 	if lv.active.count > 0 {
-		return errdefs.System(errors.Errorf("volume has active mounts"))
+		return errdefs.System(errors.New("volume has active mounts"))
 	}
 
 	if err := lv.unmount(); err != nil {
diff --git a/volume/local/local_linux_test.go b/volume/local/local_linux_test.go
index c947a929cd..da6659dc29 100644
--- a/volume/local/local_linux_test.go
+++ b/volume/local/local_linux_test.go
@@ -134,6 +134,16 @@ func TestVolCreateValidation(t *testing.T) {
 			},
 			expectedErr: `"hello world" includes invalid characters for a local volume name, only "[a-zA-Z0-9][a-zA-Z0-9_.-]" are allowed. If you intended to pass a host directory, use absolute path`,
 		},
+		{
+			doc:         "invalid: unknown option",
+			opts:        map[string]string{"hello": "world"},
+			expectedErr: `invalid option: "hello"`,
+		},
+		{
+			doc:         "invalid: invalid size",
+			opts:        map[string]string{"size": "hello"},
+			expectedErr: `invalid size: 'hello'`,
+		},
 		{
 			doc:         "invalid: size, but no quotactl",
 			opts:        map[string]string{"size": "1234"},
diff --git a/volume/local/local_unix.go b/volume/local/local_unix.go
index 1ead9b26b8..769cfda201 100644
--- a/volume/local/local_unix.go
+++ b/volume/local/local_unix.go
@@ -51,10 +51,6 @@ func (v *localVolume) setOpts(opts map[string]string) error {
 	if len(opts) == 0 {
 		return nil
 	}
-	err := validateOpts(opts)
-	if err != nil {
-		return err
-	}
 	v.opts = &optsConfig{
 		MountType:   opts["type"],
 		MountOpts:   opts["o"],
@@ -63,10 +59,10 @@ func (v *localVolume) setOpts(opts map[string]string) error {
 	if val, ok := opts["size"]; ok {
 		size, err := units.RAMInBytes(val)
 		if err != nil {
-			return err
+			return errdefs.InvalidParameter(err)
 		}
 		if size > 0 && v.quotaCtl == nil {
-			return errdefs.InvalidParameter(errors.Errorf("quota size requested but no quota support"))
+			return errdefs.InvalidParameter(errors.New("quota size requested but no quota support"))
 		}
 		v.opts.Quota.Size = uint64(size)
 	}
@@ -82,6 +78,12 @@ func validateOpts(opts map[string]string) error {
 			return errdefs.InvalidParameter(errors.Errorf("invalid option: %q", opt))
 		}
 	}
+	if val, ok := opts["size"]; ok {
+		_, err := units.RAMInBytes(val)
+		if err != nil {
+			return errdefs.InvalidParameter(err)
+		}
+	}
 	for opt, reqopts := range mandatoryOpts {
 		if _, ok := opts[opt]; ok {
 			for _, reqopt := range reqopts {
@@ -143,7 +145,7 @@ func (v *localVolume) postMount() error {
 				return err
 			}
 		} else {
-			return fmt.Errorf("size quota requested for volume but no quota support")
+			return errors.New("size quota requested for volume but no quota support")
 		}
 	}
 	return nil
diff --git a/volume/local/local_windows.go b/volume/local/local_windows.go
index abf41a980d..4f05937175 100644
--- a/volume/local/local_windows.go
+++ b/volume/local/local_windows.go
@@ -15,6 +15,11 @@ import (
 type optsConfig struct{}
 
 func (v *localVolume) setOpts(opts map[string]string) error {
+	// Windows does not support any options currently
+	return nil
+}
+
+func validateOpts(opts map[string]string) error {
 	if len(opts) > 0 {
 		return errdefs.InvalidParameter(errors.New("options are not supported on this platform"))
 	}