Merge pull request #44003 from vvoland/invalidfilter

api/types/filters/errors.go

@@ -0,0 +1,37 @@
+package filters
+
+import "fmt"
+
+// invalidFilter indicates that the provided filter or its value is invalid
+type invalidFilter struct {
+	Filter string
+	Value  []string
+}
+
+func (e invalidFilter) Error() string {
+	msg := "invalid filter"
+	if e.Filter != "" {
+		msg += " '" + e.Filter
+		if e.Value != nil {
+			msg = fmt.Sprintf("%s=%s", msg, e.Value)
+		}
+		msg += "'"
+	}
+	return msg
+}
+
+// InvalidParameter marks this error as ErrInvalidParameter
+func (e invalidFilter) InvalidParameter() {}
+
+// unreachableCode is an error indicating that the code path was not expected to be reached.
+type unreachableCode struct {
+	Filter string
+	Value  []string
+}
+
+// System marks this error as ErrSystem
+func (e unreachableCode) System() {}
+
+func (e unreachableCode) Error() string {
+	return fmt.Sprintf("unreachable code reached for filter: %q with values: %s", e.Filter, e.Value)
+}

api/types/filters/parse.go

@@ -10,7 +10,6 @@ import (
 	"strings"
 
 	"github.com/docker/docker/api/types/versions"
-	"github.com/pkg/errors"
 )
 
 // Args stores a mapping of keys to a set of multiple values.
@@ -99,7 +98,7 @@ func FromJSON(p string) (Args, error) {
 	// Fallback to parsing arguments in the legacy slice format
 	deprecated := map[string][]string{}
 	if legacyErr := json.Unmarshal(raw, &deprecated); legacyErr != nil {
-		return args, invalidFilter{errors.Wrap(err, "invalid filter")}
+		return args, invalidFilter{}
 	}
 
 	args.fields = deprecatedArgs(deprecated)
@@ -196,6 +195,38 @@ func (args Args) Match(field, source string) bool {
 	return false
 }
 
+// GetBoolOrDefault returns a boolean value of the key if the key is present
+// and is intepretable as a boolean value. Otherwise the default value is returned.
+// Error is not nil only if the filter values are not valid boolean or are conflicting.
+func (args Args) GetBoolOrDefault(key string, defaultValue bool) (bool, error) {
+	fieldValues, ok := args.fields[key]
+
+	if !ok {
+		return defaultValue, nil
+	}
+
+	if len(fieldValues) == 0 {
+		return defaultValue, invalidFilter{key, nil}
+	}
+
+	isFalse := fieldValues["0"] || fieldValues["false"]
+	isTrue := fieldValues["1"] || fieldValues["true"]
+
+	conflicting := isFalse && isTrue
+	invalid := !isFalse && !isTrue
+
+	if conflicting || invalid {
+		return defaultValue, invalidFilter{key, args.Get(key)}
+	} else if isFalse {
+		return false, nil
+	} else if isTrue {
+		return true, nil
+	}
+
+	// This code shouldn't be reached.
+	return defaultValue, unreachableCode{Filter: key, Value: args.Get(key)}
+}
+
 // ExactMatch returns true if the source matches exactly one of the values.
 func (args Args) ExactMatch(key, source string) bool {
 	fieldValues, ok := args.fields[key]
@@ -246,20 +277,12 @@ func (args Args) Contains(field string) bool {
 	return ok
 }
 
-type invalidFilter struct{ error }
-
-func (e invalidFilter) Error() string {
-	return e.error.Error()
-}
-
-func (invalidFilter) InvalidParameter() {}
-
 // Validate compared the set of accepted keys against the keys in the mapping.
 // An error is returned if any mapping keys are not in the accepted set.
 func (args Args) Validate(accepted map[string]bool) error {
 	for name := range args.fields {
 		if !accepted[name] {
-			return invalidFilter{errors.New("invalid filter '" + name + "'")}
+			return invalidFilter{name, nil}
 		}
 	}
 	return nil

api/types/filters/parse_test.go

@@ -3,6 +3,7 @@ package filters // import "github.com/docker/docker/api/types/filters"
 import (
 	"encoding/json"
 	"errors"
+	"sort"
 	"testing"
 
 	"gotest.tools/v3/assert"
@@ -418,3 +419,118 @@ func TestClone(t *testing.T) {
 	f2.Add("baz", "qux")
 	assert.Check(t, is.Len(f.Get("baz"), 0))
 }
+
+func TestGetBoolOrDefault(t *testing.T) {
+	for _, tC := range []struct {
+		name          string
+		args          map[string][]string
+		defValue      bool
+		expectedErr   error
+		expectedValue bool
+	}{
+		{
+			name: "single true",
+			args: map[string][]string{
+				"dangling": {"true"},
+			},
+			defValue:      false,
+			expectedErr:   nil,
+			expectedValue: true,
+		},
+		{
+			name: "single false",
+			args: map[string][]string{
+				"dangling": {"false"},
+			},
+			defValue:      true,
+			expectedErr:   nil,
+			expectedValue: false,
+		},
+		{
+			name: "single bad value",
+			args: map[string][]string{
+				"dangling": {"potato"},
+			},
+			defValue:      true,
+			expectedErr:   invalidFilter{Filter: "dangling", Value: []string{"potato"}},
+			expectedValue: true,
+		},
+		{
+			name: "two bad values",
+			args: map[string][]string{
+				"dangling": {"banana", "potato"},
+			},
+			defValue:      true,
+			expectedErr:   invalidFilter{Filter: "dangling", Value: []string{"banana", "potato"}},
+			expectedValue: true,
+		},
+		{
+			name: "two conflicting values",
+			args: map[string][]string{
+				"dangling": {"false", "true"},
+			},
+			defValue:      false,
+			expectedErr:   invalidFilter{Filter: "dangling", Value: []string{"false", "true"}},
+			expectedValue: false,
+		},
+		{
+			name: "multiple conflicting values",
+			args: map[string][]string{
+				"dangling": {"false", "true", "1"},
+			},
+			defValue:      true,
+			expectedErr:   invalidFilter{Filter: "dangling", Value: []string{"false", "true", "1"}},
+			expectedValue: true,
+		},
+		{
+			name: "1 means true",
+			args: map[string][]string{
+				"dangling": {"1"},
+			},
+			defValue:      false,
+			expectedErr:   nil,
+			expectedValue: true,
+		},
+		{
+			name: "0 means false",
+			args: map[string][]string{
+				"dangling": {"0"},
+			},
+			defValue:      true,
+			expectedErr:   nil,
+			expectedValue: false,
+		},
+	} {
+		tC := tC
+		t.Run(tC.name, func(t *testing.T) {
+			a := NewArgs()
+
+			for key, values := range tC.args {
+				for _, value := range values {
+					a.Add(key, value)
+				}
+			}
+
+			value, err := a.GetBoolOrDefault("dangling", tC.defValue)
+
+			if tC.expectedErr == nil {
+				assert.Check(t, is.Nil(err))
+			} else {
+				assert.Check(t, is.ErrorType(err, tC.expectedErr))
+
+				// Check if error is the same.
+				expected := tC.expectedErr.(invalidFilter)
+				actual := err.(invalidFilter)
+
+				assert.Check(t, is.Equal(expected.Filter, actual.Filter))
+
+				sort.Strings(expected.Value)
+				sort.Strings(actual.Value)
+				assert.Check(t, is.DeepEqual(expected.Value, actual.Value))
+			}
+
+			assert.Check(t, is.Equal(tC.expectedValue, value))
+		})
+	}
+
+}

daemon/errors.go

@@ -109,21 +109,6 @@ func (e containerFileNotFound) Error() string {
 
 func (containerFileNotFound) NotFound() {}
 
-type invalidFilter struct {
-	filter string
-	value  interface{}
-}
-
-func (e invalidFilter) Error() string {
-	msg := "invalid filter '" + e.filter
-	if e.value != nil {
-		msg += fmt.Sprintf("=%s", e.value)
-	}
-	return msg + "'"
-}
-
-func (e invalidFilter) InvalidParameter() {}
-
 type startInvalidConfigError string
 
 func (e startInvalidConfigError) Error() string {

daemon/images/image_list.go

@@ -38,18 +38,13 @@ func (i *ImageService) Images(ctx context.Context, opts types.ImageListOptions)
 		return nil, err
 	}
 
-	var danglingOnly bool
-	if opts.Filters.Contains("dangling") {
-		if opts.Filters.ExactMatch("dangling", "true") {
-			danglingOnly = true
-		} else if !opts.Filters.ExactMatch("dangling", "false") {
-			return nil, invalidFilter{"dangling", opts.Filters.Get("dangling")}
-		}
+	danglingOnly, err := opts.Filters.GetBoolOrDefault("dangling", false)
+	if err != nil {
+		return nil, err
 	}
 
 	var (
 		beforeFilter, sinceFilter time.Time
-		err                       error
 	)
 	err = opts.Filters.WalkValues("before", func(value string) error {
 		img, err := i.GetImage(ctx, value, imagetypes.GetImageOpts{})

daemon/images/image_prune.go

@@ -46,13 +46,9 @@ func (i *ImageService) ImagesPrune(ctx context.Context, pruneFilters filters.Arg
 
 	rep := &types.ImagesPruneReport{}
 
-	danglingOnly := true
-	if pruneFilters.Contains("dangling") {
-		if pruneFilters.ExactMatch("dangling", "false") || pruneFilters.ExactMatch("dangling", "0") {
-			danglingOnly = false
-		} else if !pruneFilters.ExactMatch("dangling", "true") && !pruneFilters.ExactMatch("dangling", "1") {
-			return nil, invalidFilter{"dangling", pruneFilters.Get("dangling")}
-		}
+	danglingOnly, err := pruneFilters.GetBoolOrDefault("dangling", true)
+	if err != nil {
+		return nil, err
 	}
 
 	until, err := getUntilFromPruneFilters(pruneFilters)

daemon/images/image_search.go

@@ -7,6 +7,8 @@ import (
 	"github.com/docker/docker/api/types/filters"
 	"github.com/docker/docker/api/types/registry"
 	"github.com/docker/docker/dockerversion"
+	"github.com/docker/docker/errdefs"
+	"github.com/pkg/errors"
 )
 
 var acceptedSearchFilterTags = map[string]bool{
@@ -27,28 +29,22 @@ func (i *ImageService) SearchRegistryForImages(ctx context.Context, searchFilter
 		return nil, err
 	}
 
-	var isAutomated, isOfficial bool
-	var hasStarFilter = 0
-	if searchFilters.Contains("is-automated") {
-		if searchFilters.UniqueExactMatch("is-automated", "true") {
-			isAutomated = true
-		} else if !searchFilters.UniqueExactMatch("is-automated", "false") {
-			return nil, invalidFilter{"is-automated", searchFilters.Get("is-automated")}
-		}
+	isAutomated, err := searchFilters.GetBoolOrDefault("is-automated", false)
+	if err != nil {
+		return nil, err
 	}
-	if searchFilters.Contains("is-official") {
-		if searchFilters.UniqueExactMatch("is-official", "true") {
-			isOfficial = true
-		} else if !searchFilters.UniqueExactMatch("is-official", "false") {
-			return nil, invalidFilter{"is-official", searchFilters.Get("is-official")}
-		}
+	isOfficial, err := searchFilters.GetBoolOrDefault("is-official", false)
+	if err != nil {
+		return nil, err
 	}
+
+	hasStarFilter := 0
 	if searchFilters.Contains("stars") {
 		hasStars := searchFilters.Get("stars")
 		for _, hasStar := range hasStars {
 			iHasStar, err := strconv.Atoi(hasStar)
 			if err != nil {
-				return nil, invalidFilter{"stars", hasStar}
+				return nil, errdefs.InvalidParameter(errors.Wrapf(err, "invalid filter 'stars=%s'", hasStar))
 			}
 			if iHasStar > hasStarFilter {
 				hasStarFilter = iHasStar

daemon/images/locals.go

@@ -1,26 +1,9 @@
 package images // import "github.com/docker/docker/daemon/images"
 
 import (
-	"fmt"
-
 	metrics "github.com/docker/go-metrics"
 )
 
-type invalidFilter struct {
-	filter string
-	value  interface{}
-}
-
-func (e invalidFilter) Error() string {
-	msg := "invalid filter '" + e.filter
-	if e.value != nil {
-		msg += fmt.Sprintf("=%s", e.value)
-	}
-	return msg + "'"
-}
-
-func (e invalidFilter) InvalidParameter() {}
-
 var imageActions metrics.LabeledTimer
 
 func init() {

daemon/list.go

@@ -254,7 +254,7 @@ func (daemon *Daemon) foldFilter(ctx context.Context, view *container.View, conf
 	err := psFilters.WalkValues("exited", func(value string) error {
 		code, err := strconv.Atoi(value)
 		if err != nil {
-			return err
+			return errdefs.InvalidParameter(errors.Wrapf(err, "invalid filter 'exited=%s'", value))
 		}
 		filtExited = append(filtExited, code)
 		return nil
@@ -265,7 +265,7 @@ func (daemon *Daemon) foldFilter(ctx context.Context, view *container.View, conf
 
 	err = psFilters.WalkValues("status", func(value string) error {
 		if !container.IsValidStateString(value) {
-			return invalidFilter{"status", value}
+			return errdefs.InvalidParameter(fmt.Errorf("invalid filter 'status=%s'", value))
 		}
 
 		config.All = true
@@ -275,22 +275,15 @@ func (daemon *Daemon) foldFilter(ctx context.Context, view *container.View, conf
 		return nil, err
 	}
 
-	var taskFilter, isTask bool
-	if psFilters.Contains("is-task") {
-		if psFilters.ExactMatch("is-task", "true") {
-			taskFilter = true
-			isTask = true
-		} else if psFilters.ExactMatch("is-task", "false") {
-			taskFilter = true
-			isTask = false
-		} else {
-			return nil, invalidFilter{"is-task", psFilters.Get("is-task")}
-		}
+	taskFilter := psFilters.Contains("is-task")
+	isTask, err := psFilters.GetBoolOrDefault("is-task", false)
+	if err != nil {
+		return nil, err
 	}
 
 	err = psFilters.WalkValues("health", func(value string) error {
 		if !container.IsValidHealthString(value) {
-			return errdefs.InvalidParameter(errors.Errorf("Unrecognised filter value for health: %s", value))
+			return errdefs.InvalidParameter(fmt.Errorf("unrecognized filter value for health: %s", value))
 		}
 
 		return nil

plugin/backend_linux.go

@@ -318,12 +318,15 @@ func (pm *Manager) List(pluginFilters filters.Args) ([]types.Plugin, error) {
 	enabledOnly := false
 	disabledOnly := false
 	if pluginFilters.Contains("enabled") {
-		if pluginFilters.ExactMatch("enabled", "true") {
+		enabledFilter, err := pluginFilters.GetBoolOrDefault("enabled", false)
+		if err != nil {
+			return nil, err
+		}
+
+		if enabledFilter {
 			enabledOnly = true
-		} else if pluginFilters.ExactMatch("enabled", "false") {
-			disabledOnly = true
 		} else {
-			return nil, invalidFilter{"enabled", pluginFilters.Get("enabled")}
+			disabledOnly = true
 		}
 	}
 

plugin/errors.go

@@ -26,21 +26,6 @@ func (name errDisabled) Error() string {
 
 func (name errDisabled) Conflict() {}
 
-type invalidFilter struct {
-	filter string
-	value  []string
-}
-
-func (e invalidFilter) Error() string {
-	msg := "invalid filter '" + e.filter
-	if len(e.value) > 0 {
-		msg += fmt.Sprintf("=%s", e.value)
-	}
-	return msg + "'"
-}
-
-func (invalidFilter) InvalidParameter() {}
-
 type inUseError string
 
 func (e inUseError) Error() string {

volume/service/convert.go

@@ -114,11 +114,9 @@ func filtersToBy(filter filters.Args, acceptedFilters map[string]bool) (By, erro
 	bys = append(bys, byLabelFilter(filter))
 
 	if filter.Contains("dangling") {
-		var dangling bool
-		if filter.ExactMatch("dangling", "true") || filter.ExactMatch("dangling", "1") {
-			dangling = true
-		} else if !filter.ExactMatch("dangling", "false") && !filter.ExactMatch("dangling", "0") {
-			return nil, invalidFilter{"dangling", filter.Get("dangling")}
+		dangling, err := filter.GetBoolOrDefault("dangling", false)
+		if err != nil {
+			return nil, err
 		}
 		bys = append(bys, ByReferenced(!dangling))
 	}
@@ -138,7 +136,7 @@ func withPrune(filter filters.Args) error {
 	all := filter.Get("all")
 	switch {
 	case len(all) > 1:
-		return invalidFilter{"all", all}
+		return errdefs.InvalidParameter(fmt.Errorf("invalid filter 'all=%s': only one value is expected", all))
 	case len(all) == 1:
 		ok, err := strconv.ParseBool(all[0])
 		if err != nil {

volume/service/errors.go

@@ -1,7 +1,6 @@
 package service // import "github.com/docker/docker/volume/service"
 
 import (
-	"fmt"
 	"strings"
 )
 
@@ -94,18 +93,3 @@ func isErr(err error, expected error) bool {
 	}
 	return err == expected
 }
-
-type invalidFilter struct {
-	filter string
-	value  interface{}
-}
-
-func (e invalidFilter) Error() string {
-	msg := "invalid filter '" + e.filter
-	if e.value != nil {
-		msg += fmt.Sprintf("=%s", e.value)
-	}
-	return msg + "'"
-}
-
-func (e invalidFilter) InvalidParameter() {}