From 96241159a825f29ee43681b675e95e80cb26b70d Mon Sep 17 00:00:00 2001 From: Jake Correnti Date: Mon, 17 Jul 2023 12:57:51 -0400 Subject: [PATCH] Fix multiple filter options logic for `podman volume ls ` Fixes a bug where `podman volume ls` with multiple `label` filters would return volumes that matched *any* of the filters, not *all* of them. Adapts generating volume filter functions to be more in line with how it is done for containers and pods. Fixes: #19219 Signed-off-by: Jake Correnti --- docs/source/markdown/podman-volume-ls.1.md.in | 5 + pkg/api/handlers/compat/volumes.go | 24 ++- pkg/api/handlers/libpod/volumes.go | 22 ++- pkg/domain/filters/volumes.go | 178 ++++++++---------- pkg/domain/infra/abi/volumes.go | 23 ++- test/e2e/volume_ls_test.go | 29 +-- 6 files changed, 151 insertions(+), 130 deletions(-) diff --git a/docs/source/markdown/podman-volume-ls.1.md.in b/docs/source/markdown/podman-volume-ls.1.md.in index 3ad5ae82e5..6a45d1161e 100644 --- a/docs/source/markdown/podman-volume-ls.1.md.in +++ b/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** | diff --git a/pkg/api/handlers/compat/volumes.go b/pkg/api/handlers/compat/volumes.go index 27b3114a55..e17a5c3be0 100644 --- a/pkg/api/handlers/compat/volumes.go +++ b/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) diff --git a/pkg/api/handlers/libpod/volumes.go b/pkg/api/handlers/libpod/volumes.go index 7ec85e3c04..bb2bdc3709 100644 --- a/pkg/api/handlers/libpod/volumes.go +++ b/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) diff --git a/pkg/domain/filters/volumes.go b/pkg/domain/filters/volumes.go index 59121f389f..45edd2a84c 100644 --- a/pkg/domain/filters/volumes.go +++ b/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 } diff --git a/pkg/domain/infra/abi/volumes.go b/pkg/domain/infra/abi/volumes.go index 5430f134f3..cd58acb284 100644 --- a/pkg/domain/infra/abi/volumes.go +++ b/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 diff --git a/test/e2e/volume_ls_test.go b/test/e2e/volume_ls_test.go index 7601780ada..2bdbe1798a 100644 --- a/test/e2e/volume_ls_test.go +++ b/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)) }) })