Fix multiple filter options logic for podman volume ls

docs/source/markdown/podman-volume-ls.1.md.in

@@ -16,6 +16,11 @@ flag. Use the **--quiet** flag to print only the volume names.
 
 #### **--filter**, **-f**=*filter*
 
+Filter what volumes are shown in the output.
+Multiple filters can be given with multiple uses of the --filter flag.
+Filters with the same key work inclusive, with the only exception being `label`
+which is exclusive. Filters with different keys always work exclusive.
+
 Volumes can be filtered by the following attributes:
 
 | **Filter** | **Description**                                                                       |

pkg/api/handlers/compat/volumes.go

@@ -41,10 +41,14 @@ func ListVolumes(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 	}
-	volumeFilters, err := filters.GenerateVolumeFilters(*filtersMap)
-	if err != nil {
-		utils.InternalServerError(w, err)
-		return
+
+	volumeFilters := []libpod.VolumeFilter{}
+	for filter, filterValues := range *filtersMap {
+		filterFunc, err := filters.GenerateVolumeFilters(filter, filterValues)
+		if err != nil {
+			utils.InternalServerError(w, err)
+		}
+		volumeFilters = append(volumeFilters, filterFunc)
 	}
 
 	vols, err := runtime.Volumes(volumeFilters...)
@@ -270,10 +274,14 @@ func PruneVolumes(w http.ResponseWriter, r *http.Request) {
 	}
 
 	f := (url.Values)(*filterMap)
-	filterFuncs, err := filters.GeneratePruneVolumeFilters(f)
-	if err != nil {
-		utils.Error(w, http.StatusInternalServerError, fmt.Errorf("failed to parse filters for %s: %w", f.Encode(), err))
-		return
+	filterFuncs := []libpod.VolumeFilter{}
+	for filter, filterValues := range f {
+		filterFunc, err := filters.GeneratePruneVolumeFilters(filter, filterValues)
+		if err != nil {
+			utils.Error(w, http.StatusInternalServerError, fmt.Errorf("failed to parse filters for %s: %w", f.Encode(), err))
+			return
+		}
+		filterFuncs = append(filterFuncs, filterFunc)
 	}
 
 	pruned, err := runtime.PruneVolumes(r.Context(), filterFuncs)

pkg/api/handlers/libpod/volumes.go

@@ -119,10 +119,14 @@ func ListVolumes(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	volumeFilters, err := filters.GenerateVolumeFilters(*filterMap)
-	if err != nil {
-		utils.InternalServerError(w, err)
-		return
+	volumeFilters := []libpod.VolumeFilter{}
+	for filter, filterValues := range *filterMap {
+		filterFunc, err := filters.GenerateVolumeFilters(filter, filterValues)
+		if err != nil {
+			utils.InternalServerError(w, err)
+			return
+		}
+		volumeFilters = append(volumeFilters, filterFunc)
 	}
 
 	vols, err := runtime.Volumes(volumeFilters...)
@@ -162,9 +166,13 @@ func pruneVolumesHelper(r *http.Request) ([]*reports.PruneReport, error) {
 	}
 
 	f := (url.Values)(*filterMap)
-	filterFuncs, err := filters.GeneratePruneVolumeFilters(f)
-	if err != nil {
-		return nil, err
+	filterFuncs := []libpod.VolumeFilter{}
+	for filter, filterValues := range f {
+		filterFunc, err := filters.GeneratePruneVolumeFilters(filter, filterValues)
+		if err != nil {
+			return nil, err
+		}
+		filterFuncs = append(filterFuncs, filterFunc)
 	}
 
 	reports, err := runtime.PruneVolumes(r.Context(), filterFuncs)

pkg/domain/filters/volumes.go

@@ -2,48 +2,48 @@ package filters
 
 import (
 	"fmt"
-	"net/url"
-	"regexp"
 	"strings"
 
 	"github.com/containers/common/pkg/filters"
 	"github.com/containers/podman/v4/libpod"
+	"github.com/containers/podman/v4/pkg/util"
 )
 
-func GenerateVolumeFilters(filtersValues url.Values) ([]libpod.VolumeFilter, error) {
-	var vf []libpod.VolumeFilter
-	for filter, v := range filtersValues {
-		for _, val := range v {
-			switch filter {
-			case "name":
-				nameRegexp, err := regexp.Compile(val)
-				if err != nil {
-					return nil, err
+func GenerateVolumeFilters(filter string, filterValues []string) (libpod.VolumeFilter, error) {
+	switch filter {
+	case "name":
+		return func(v *libpod.Volume) bool {
+			return util.StringMatchRegexSlice(v.Name(), filterValues)
+		}, nil
+	case "driver":
+		return func(v *libpod.Volume) bool {
+			for _, val := range filterValues {
+				if v.Driver() == val {
+					return true
+				}
+			}
+			return false
+		}, nil
+	case "scope":
+		return func(v *libpod.Volume) bool {
+			for _, val := range filterValues {
+				if v.Scope() == val {
+					return true
 				}
-				vf = append(vf, func(v *libpod.Volume) bool {
-					return nameRegexp.MatchString(v.Name())
-				})
-			case "driver":
-				driverVal := val
-				vf = append(vf, func(v *libpod.Volume) bool {
-					return v.Driver() == driverVal
-				})
-			case "scope":
-				scopeVal := val
-				vf = append(vf, func(v *libpod.Volume) bool {
-					return v.Scope() == scopeVal
-				})
-			case "label":
-				filter := val
-				vf = append(vf, func(v *libpod.Volume) bool {
-					return filters.MatchLabelFilters([]string{filter}, v.Labels())
-				})
-			case "label!":
-				filter := val
-				vf = append(vf, func(v *libpod.Volume) bool {
-					return !filters.MatchLabelFilters([]string{filter}, v.Labels())
-				})
-			case "opt":
+			}
+			return false
+		}, nil
+	case "label":
+		return func(v *libpod.Volume) bool {
+			return filters.MatchLabelFilters(filterValues, v.Labels())
+		}, nil
+	case "label!":
+		return func(v *libpod.Volume) bool {
+			return !filters.MatchLabelFilters(filterValues, v.Labels())
+		}, nil
+	case "opt":
+		return func(v *libpod.Volume) bool {
+			for _, val := range filterValues {
 				filterArray := strings.SplitN(val, "=", 2)
 				filterKey := filterArray[0]
 				var filterVal string
@@ -52,82 +52,70 @@ func GenerateVolumeFilters(filtersValues url.Values) ([]libpod.VolumeFilter, err
 				} else {
 					filterVal = ""
 				}
-				vf = append(vf, func(v *libpod.Volume) bool {
-					for labelKey, labelValue := range v.Options() {
-						if labelKey == filterKey && (filterVal == "" || labelValue == filterVal) {
-							return true
-						}
+
+				for labelKey, labelValue := range v.Options() {
+					if labelKey == filterKey && (filterVal == "" || labelValue == filterVal) {
+						return true
 					}
-					return false
-				})
-			case "until":
-				f, err := createUntilFilterVolumeFunction(val)
+				}
+			}
+			return false
+		}, nil
+	case "until":
+		return createUntilFilterVolumeFunction(filterValues)
+	case "dangling":
+		for _, val := range filterValues {
+			switch strings.ToLower(val) {
+			case "true", "1", "false", "0":
+			default:
+				return nil, fmt.Errorf("%q is not a valid value for the \"dangling\" filter - must be true or false", val)
+			}
+		}
+		return func(v *libpod.Volume) bool {
+			for _, val := range filterValues {
+				dangling, err := v.IsDangling()
 				if err != nil {
-					return nil, err
+					return false
 				}
-				vf = append(vf, f)
-			case "dangling":
-				danglingVal := val
+
 				invert := false
-				switch strings.ToLower(danglingVal) {
-				case "true", "1":
-					// Do nothing
+				switch strings.ToLower(val) {
 				case "false", "0":
 					// Dangling=false requires that we
 					// invert the result of IsDangling.
 					invert = true
-				default:
-					return nil, fmt.Errorf("%q is not a valid value for the \"dangling\" filter - must be true or false", danglingVal)
 				}
-				vf = append(vf, func(v *libpod.Volume) bool {
-					dangling, err := v.IsDangling()
-					if err != nil {
-						return false
-					}
-					if invert {
-						return !dangling
-					}
-					return dangling
-				})
-			default:
-				return nil, fmt.Errorf("%q is an invalid volume filter", filter)
+				if invert {
+					dangling = !dangling
+				}
+				if dangling {
+					return true
+				}
 			}
-		}
+			return false
+		}, nil
 	}
-	return vf, nil
+	return nil, fmt.Errorf("%q is an invalid volume filter", filter)
 }
 
-func GeneratePruneVolumeFilters(filtersValues url.Values) ([]libpod.VolumeFilter, error) {
-	var vf []libpod.VolumeFilter
-	for filter, v := range filtersValues {
-		for _, val := range v {
-			filterVal := val
-			switch filter {
-			case "label":
-				vf = append(vf, func(v *libpod.Volume) bool {
-					return filters.MatchLabelFilters([]string{filterVal}, v.Labels())
-				})
-			case "label!":
-				filter := val
-				vf = append(vf, func(v *libpod.Volume) bool {
-					return !filters.MatchLabelFilters([]string{filter}, v.Labels())
-				})
-			case "until":
-				f, err := createUntilFilterVolumeFunction(filterVal)
-				if err != nil {
-					return nil, err
-				}
-				vf = append(vf, f)
-			default:
-				return nil, fmt.Errorf("%q is an invalid volume filter", filter)
-			}
-		}
+func GeneratePruneVolumeFilters(filter string, filterValues []string) (libpod.VolumeFilter, error) {
+	switch filter {
+	case "label":
+		return func(v *libpod.Volume) bool {
+			return filters.MatchLabelFilters(filterValues, v.Labels())
+		}, nil
+	case "label!":
+		return func(v *libpod.Volume) bool {
+			return !filters.MatchLabelFilters(filterValues, v.Labels())
+		}, nil
+	case "until":
+		return createUntilFilterVolumeFunction(filterValues)
 	}
-	return vf, nil
+	return nil, fmt.Errorf("%q is an invalid volume filter", filter)
 }
 
-func createUntilFilterVolumeFunction(filter string) (libpod.VolumeFilter, error) {
-	until, err := filters.ComputeUntilTimestamp([]string{filter})
+func createUntilFilterVolumeFunction(filterValues []string) (libpod.VolumeFilter, error) {
+	until, err := filters.ComputeUntilTimestamp(filterValues)
 	if err != nil {
 		return nil, err
 	}

pkg/domain/infra/abi/volumes.go

@@ -122,11 +122,15 @@ func (ic *ContainerEngine) VolumeInspect(ctx context.Context, namesOrIds []strin
 }
 
 func (ic *ContainerEngine) VolumePrune(ctx context.Context, options entities.VolumePruneOptions) ([]*reports.PruneReport, error) {
-	filterFuncs, err := filters.GenerateVolumeFilters(options.Filters)
-	if err != nil {
-		return nil, err
+	funcs := []libpod.VolumeFilter{}
+	for filter, filterValues := range options.Filters {
+		filterFunc, err := filters.GenerateVolumeFilters(filter, filterValues)
+		if err != nil {
+			return nil, err
+		}
+		funcs = append(funcs, filterFunc)
 	}
-	return ic.pruneVolumesHelper(ctx, filterFuncs)
+	return ic.pruneVolumesHelper(ctx, funcs)
 }
 
 func (ic *ContainerEngine) pruneVolumesHelper(ctx context.Context, filterFuncs []libpod.VolumeFilter) ([]*reports.PruneReport, error) {
@@ -138,10 +142,15 @@ func (ic *ContainerEngine) pruneVolumesHelper(ctx context.Context, filterFuncs [
 }
 
 func (ic *ContainerEngine) VolumeList(ctx context.Context, opts entities.VolumeListOptions) ([]*entities.VolumeListReport, error) {
-	volumeFilters, err := filters.GenerateVolumeFilters(opts.Filter)
-	if err != nil {
-		return nil, err
+	volumeFilters := []libpod.VolumeFilter{}
+	for filter, value := range opts.Filter {
+		filterFunc, err := filters.GenerateVolumeFilters(filter, value)
+		if err != nil {
+			return nil, err
+		}
+		volumeFilters = append(volumeFilters, filterFunc)
 	}
+
 	vols, err := ic.Libpod.Volumes(volumeFilters...)
 	if err != nil {
 		return nil, err

test/e2e/volume_ls_test.go

@@ -180,32 +180,35 @@ var _ = Describe("Podman volume ls", func() {
 	})
 
 	It("podman ls volume with multiple --filter flag", func() {
-		session := podmanTest.Podman([]string{"volume", "create", "--label", "foo=bar", "myvol"})
-		volName := session.OutputToString()
+		session := podmanTest.Podman([]string{"volume", "create", "--label", "a=b", "--label", "b=c", "vol1"})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
 
-		session = podmanTest.Podman([]string{"volume", "create", "--label", "foo2=bar2", "anothervol"})
-		anotherVol := session.OutputToString()
+		vol1Name := session.OutputToString()
+
+		session = podmanTest.Podman([]string{"volume", "create", "--label", "b=c", "--label", "a=b", "vol2"})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
 
-		session = podmanTest.Podman([]string{"volume", "create"})
+		vol2Name := session.OutputToString()
+
+		session = podmanTest.Podman([]string{"volume", "create", "--label", "b=c", "--label", "c=d", "vol3"})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
 
-		session = podmanTest.Podman([]string{"volume", "ls", "--filter", "label=foo", "--filter", "label=foo2"})
+		vol3Name := session.OutputToString()
+
+		session = podmanTest.Podman([]string{"volume", "ls", "-q", "--filter", "label=a=b", "--filter", "label=b=c"})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
-		Expect(session.OutputToStringArray()).To(HaveLen(3))
-		Expect(session.OutputToStringArray()[1]).To(ContainSubstring(volName))
-		Expect(session.OutputToStringArray()[2]).To(ContainSubstring(anotherVol))
+		Expect(session.OutputToStringArray()).To(HaveLen(2))
+		Expect(session.OutputToStringArray()[0]).To(Equal(vol1Name))
+		Expect(session.OutputToStringArray()[1]).To(Equal(vol2Name))
 
-		session = podmanTest.Podman([]string{"volume", "ls", "--filter", "label=foo=bar", "--filter", "label=foo2=bar2"})
+		session = podmanTest.Podman([]string{"volume", "ls", "-q", "--filter", "label=c=d", "--filter", "label=b=c"})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
-		Expect(session.OutputToStringArray()).To(HaveLen(3))
-		Expect(session.OutputToStringArray()[1]).To(ContainSubstring(volName))
-		Expect(session.OutputToStringArray()[2]).To(ContainSubstring(anotherVol))
+		Expect(session.OutputToStringArray()).To(HaveLen(1))
+		Expect(session.OutputToStringArray()[0]).To(Equal(vol3Name))
 	})
 })