diff --git a/opts/duration.go b/opts/duration.go index d55c51e622..41a27a564d 100644 --- a/opts/duration.go +++ b/opts/duration.go @@ -1,9 +1,8 @@ package opts import ( + "errors" "time" - - "github.com/pkg/errors" ) // PositiveDurationOpt is an option type for time.Duration that uses a pointer. @@ -20,7 +19,7 @@ func (d *PositiveDurationOpt) Set(s string) error { return err } if *d.DurationOpt.value < 0 { - return errors.Errorf("duration cannot be negative") + return errors.New("duration cannot be negative") } return nil } diff --git a/opts/env.go b/opts/env.go index 214d6f4400..675ddda962 100644 --- a/opts/env.go +++ b/opts/env.go @@ -1,10 +1,9 @@ package opts import ( + "errors" "os" "strings" - - "github.com/pkg/errors" ) // ValidateEnv validates an environment variable and returns it. diff --git a/opts/gpus.go b/opts/gpus.go index 492a190837..6a56c49c4a 100644 --- a/opts/gpus.go +++ b/opts/gpus.go @@ -2,12 +2,12 @@ package opts import ( "encoding/csv" + "errors" "fmt" "strconv" "strings" "github.com/docker/docker/api/types/container" - "github.com/pkg/errors" ) // GpuOpts is a Value type for parsing mounts @@ -21,7 +21,11 @@ func parseCount(s string) (int, error) { } i, err := strconv.Atoi(s) if err != nil { - return 0, errors.Wrap(err, "count must be an integer") + var numErr *strconv.NumError + if errors.As(err, &numErr) { + err = numErr.Err + } + return 0, fmt.Errorf(`invalid count (%s): value must be either "all" or an integer: %w`, s, err) } return i, nil } @@ -72,7 +76,7 @@ func (o *GpuOpts) Set(value string) error { r := csv.NewReader(strings.NewReader(val)) optFields, err := r.Read() if err != nil { - return errors.Wrap(err, "failed to read gpu options") + return fmt.Errorf("failed to read gpu options: %w", err) } req.Options = ConvertKVStringsToMap(optFields) default: diff --git a/opts/gpus_test.go b/opts/gpus_test.go index 8f620046df..ea177a1971 100644 --- a/opts/gpus_test.go +++ b/opts/gpus_test.go @@ -16,7 +16,7 @@ func TestGpusOptAll(t *testing.T) { "count=-1", } { var gpus GpuOpts - gpus.Set(testcase) + assert.Check(t, gpus.Set(testcase)) gpuReqs := gpus.Value() assert.Assert(t, is.Len(gpuReqs, 1)) assert.Check(t, is.DeepEqual(gpuReqs[0], container.DeviceRequest{ @@ -27,15 +27,21 @@ func TestGpusOptAll(t *testing.T) { } } +func TestGpusOptInvalidCount(t *testing.T) { + var gpus GpuOpts + err := gpus.Set(`count=invalid-integer`) + assert.Error(t, err, `invalid count (invalid-integer): value must be either "all" or an integer: invalid syntax`) +} + func TestGpusOpts(t *testing.T) { for _, testcase := range []string{ - "driver=nvidia,\"capabilities=compute,utility\",\"options=foo=bar,baz=qux\"", - "1,driver=nvidia,\"capabilities=compute,utility\",\"options=foo=bar,baz=qux\"", - "count=1,driver=nvidia,\"capabilities=compute,utility\",\"options=foo=bar,baz=qux\"", - "driver=nvidia,\"capabilities=compute,utility\",\"options=foo=bar,baz=qux\",count=1", + `driver=nvidia,"capabilities=compute,utility","options=foo=bar,baz=qux"`, + `1,driver=nvidia,"capabilities=compute,utility","options=foo=bar,baz=qux"`, + `count=1,driver=nvidia,"capabilities=compute,utility","options=foo=bar,baz=qux"`, + `driver=nvidia,"capabilities=compute,utility","options=foo=bar,baz=qux",count=1`, } { var gpus GpuOpts - gpus.Set(testcase) + assert.Check(t, gpus.Set(testcase)) gpuReqs := gpus.Value() assert.Assert(t, is.Len(gpuReqs, 1)) assert.Check(t, is.DeepEqual(gpuReqs[0], container.DeviceRequest{ diff --git a/opts/mount.go b/opts/mount.go index 275a4d7f87..a4219b07f9 100644 --- a/opts/mount.go +++ b/opts/mount.go @@ -135,8 +135,7 @@ func (m *MountOpt) Set(value string) error { // TODO: implicitly set propagation and error if the user specifies a propagation in a future refactor/UX polish pass // https://github.com/docker/cli/pull/4316#discussion_r1341974730 default: - return fmt.Errorf("invalid value for %s: %s (must be \"enabled\", \"disabled\", \"writable\", or \"readonly\")", - key, val) + return fmt.Errorf(`invalid value for %s: %s (must be "enabled", "disabled", "writable", or "readonly")`, key, val) } case "volume-subpath": volumeOptions().Subpath = val diff --git a/opts/network.go b/opts/network.go index c3510870e7..43b3a09d41 100644 --- a/opts/network.go +++ b/opts/network.go @@ -89,7 +89,11 @@ func (n *NetworkOpt) Set(value string) error { //nolint:gocyclo case gwPriorityOpt: netOpt.GwPriority, err = strconv.Atoi(val) if err != nil { - return fmt.Errorf("invalid gw-priority: %w", err) + var numErr *strconv.NumError + if errors.As(err, &numErr) { + err = numErr.Err + } + return fmt.Errorf("invalid gw-priority (%s): %w", val, err) } default: return errors.New("invalid field key " + key) diff --git a/opts/network_test.go b/opts/network_test.go index 71f38e98ac..ffdcbf26cd 100644 --- a/opts/network_test.go +++ b/opts/network_test.go @@ -149,6 +149,10 @@ func TestNetworkOptAdvancedSyntaxInvalid(t *testing.T) { value: "driver-opt=field1=value1,driver-opt=field2=value2", expectedError: "network name/id is not specified", }, + { + value: "gw-priority=invalid-integer", + expectedError: "invalid gw-priority (invalid-integer): invalid syntax", + }, } for _, tc := range testCases { t.Run(tc.value, func(t *testing.T) { diff --git a/opts/opts.go b/opts/opts.go index 061fda57c5..f6b337078d 100644 --- a/opts/opts.go +++ b/opts/opts.go @@ -1,6 +1,7 @@ package opts import ( + "errors" "fmt" "math/big" "net" @@ -9,8 +10,7 @@ import ( "strings" "github.com/docker/docker/api/types/filters" - units "github.com/docker/go-units" - "github.com/pkg/errors" + "github.com/docker/go-units" ) var ( diff --git a/opts/opts_test.go b/opts/opts_test.go index a69153ca25..5169d48a6c 100644 --- a/opts/opts_test.go +++ b/opts/opts_test.go @@ -136,10 +136,10 @@ func TestListOptsWithoutValidator(t *testing.T) { t.Errorf("%d != 3", o.Len()) } if !o.Get("bar") { - t.Error("o.Get(\"bar\") == false") + t.Error(`o.Get("bar") == false`) } if o.Get("baz") { - t.Error("o.Get(\"baz\") == true") + t.Error(`o.Get("baz") == true`) } o.Delete("foo") if o.String() != "[bar bar]" { diff --git a/opts/port.go b/opts/port.go index 0407355e65..f0394eb281 100644 --- a/opts/port.go +++ b/opts/port.go @@ -46,42 +46,50 @@ func (p *PortOpt) Set(value string) error { // TODO(thaJeztah): these options should not be case-insensitive. key, val, ok := strings.Cut(strings.ToLower(field), "=") if !ok || key == "" { - return fmt.Errorf("invalid field %s", field) + return fmt.Errorf("invalid field: %s", field) } switch key { case portOptProtocol: if val != string(swarm.PortConfigProtocolTCP) && val != string(swarm.PortConfigProtocolUDP) && val != string(swarm.PortConfigProtocolSCTP) { - return fmt.Errorf("invalid protocol value %s", val) + return fmt.Errorf("invalid protocol value '%s'", val) } pConfig.Protocol = swarm.PortConfigProtocol(val) case portOptMode: if val != string(swarm.PortConfigPublishModeIngress) && val != string(swarm.PortConfigPublishModeHost) { - return fmt.Errorf("invalid publish mode value %s", val) + return fmt.Errorf("invalid publish mode value (%s): must be either '%s' or '%s'", val, swarm.PortConfigPublishModeIngress, swarm.PortConfigPublishModeHost) } pConfig.PublishMode = swarm.PortConfigPublishMode(val) case portOptTargetPort: tPort, err := strconv.ParseUint(val, 10, 16) if err != nil { - return err + var numErr *strconv.NumError + if errors.As(err, &numErr) { + err = numErr.Err + } + return fmt.Errorf("invalid target port (%s): value must be an integer: %w", val, err) } pConfig.TargetPort = uint32(tPort) case portOptPublishedPort: pPort, err := strconv.ParseUint(val, 10, 16) if err != nil { - return err + var numErr *strconv.NumError + if errors.As(err, &numErr) { + err = numErr.Err + } + return fmt.Errorf("invalid published port (%s): value must be an integer: %w", val, err) } pConfig.PublishedPort = uint32(pPort) default: - return fmt.Errorf("invalid field key %s", key) + return fmt.Errorf("invalid field key: %s", key) } } if pConfig.TargetPort == 0 { - return fmt.Errorf("missing mandatory field %q", portOptTargetPort) + return fmt.Errorf("missing mandatory field '%s'", portOptTargetPort) } if pConfig.PublishMode == "" { diff --git a/opts/port_test.go b/opts/port_test.go index 06d19fce39..fdca0770f1 100644 --- a/opts/port_test.go +++ b/opts/port_test.go @@ -243,44 +243,46 @@ func TestPortOptInvalidComplexSyntax(t *testing.T) { }{ { value: "invalid,target=80", - expectedError: "invalid field", + expectedError: "invalid field: invalid", }, { value: "invalid=field", - expectedError: "invalid field", + expectedError: "invalid field key: invalid", }, { value: "protocol=invalid", - expectedError: "invalid protocol value", + expectedError: "invalid protocol value 'invalid'", }, { value: "target=invalid", - expectedError: "invalid syntax", + expectedError: "invalid target port (invalid): value must be an integer: invalid syntax", }, { value: "published=invalid", - expectedError: "invalid syntax", + expectedError: "invalid published port (invalid): value must be an integer: invalid syntax", }, { value: "mode=invalid", - expectedError: "invalid publish mode value", + expectedError: "invalid publish mode value (invalid): must be either 'ingress' or 'host'", }, { value: "published=8080,protocol=tcp,mode=ingress", - expectedError: "missing mandatory field", + expectedError: "missing mandatory field 'target'", }, { value: `target=80,protocol="tcp,mode=ingress"`, - expectedError: "non-quoted-field", + expectedError: `parse error on line 1, column 20: bare " in non-quoted-field`, }, { value: `target=80,"protocol=tcp,mode=ingress"`, - expectedError: "invalid protocol value", + expectedError: "invalid protocol value 'tcp,mode=ingress'", }, } for _, tc := range testCases { - var port PortOpt - assert.ErrorContains(t, port.Set(tc.value), tc.expectedError) + t.Run(tc.value, func(t *testing.T) { + var port PortOpt + assert.Error(t, port.Set(tc.value), tc.expectedError) + }) } }