From 504d2e82b3510a8ebdd08d0c457abc408eb88e1b Mon Sep 17 00:00:00 2001 From: Travis Lyons Date: Tue, 12 Aug 2025 14:58:30 -0400 Subject: [PATCH] Fix volume filters to use AND instead of OR logic Volume ls and volume prune commands were incorrectly combining multiple filters with OR logic instead of AND logic. This meant that specifying multiple filters like --filter "label=a=b" --filter "label!=c=d" would return volumes that matched ANY filter rather than ALL filters, causing results to expand instead of narrow down as users expect. Changed the filtering logic in libpod/runtime_volume.go to require all filters to match (AND logic) instead of any filter matching (OR logic). Updated the function comment to reflect the correct AND behavior. Added integration tests covering: - Basic AND logic with label filters (label=a=b AND label=c=d) - Mixed positive/negative label filters (label=a=b AND label!=c=d) - Cross-filter-type combinations (label filters AND time filters) - Both volume ls and volume prune commands - Verification that within-key OR logic still works correctly Fixes: #26786 Signed-off-by: Travis Lyons Signed-off-by: Travis Lyons --- cmd/podman/parse/filters.go | 14 +- docs/source/markdown/podman-volume-ls.1.md.in | 4 +- docs/source/markdown/podman-volume-prune.1.md | 2 + libpod/runtime_volume.go | 11 +- pkg/domain/filters/volumes.go | 142 +++++++++++++++++- test/e2e/volume_ls_test.go | 113 ++++++++++++++ test/e2e/volume_prune_test.go | 70 +++++++++ 7 files changed, 345 insertions(+), 11 deletions(-) diff --git a/cmd/podman/parse/filters.go b/cmd/podman/parse/filters.go index af9157cde7..17ab3e8201 100644 --- a/cmd/podman/parse/filters.go +++ b/cmd/podman/parse/filters.go @@ -9,7 +9,19 @@ import ( func FilterArgumentsIntoFilters(filters []string) (url.Values, error) { parsedFilters := make(url.Values) for _, f := range filters { - fname, filter, hasFilter := strings.Cut(f, "=") + // Handle negative filters like label!=value by treating them as separate filter keys + var fname, filter string + var hasFilter bool + + if strings.Contains(f, "!=") { + fname, filter, hasFilter = strings.Cut(f, "!=") + if hasFilter { + fname += "!" + } + } else { + fname, filter, hasFilter = strings.Cut(f, "=") + } + if !hasFilter { return parsedFilters, fmt.Errorf("filter input must be in the form of filter=value: %s is invalid", f) } diff --git a/docs/source/markdown/podman-volume-ls.1.md.in b/docs/source/markdown/podman-volume-ls.1.md.in index 64e84b518e..1860b47716 100644 --- a/docs/source/markdown/podman-volume-ls.1.md.in +++ b/docs/source/markdown/podman-volume-ls.1.md.in @@ -18,8 +18,8 @@ flag. Use the **--quiet** flag to print only the volume names. 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. +Filters with the same key work inclusive (OR logic). Filters with different keys +always work exclusive (AND logic). Volumes can be filtered by the following attributes: diff --git a/docs/source/markdown/podman-volume-prune.1.md b/docs/source/markdown/podman-volume-prune.1.md index 3dd585f3d2..cfc551f82d 100644 --- a/docs/source/markdown/podman-volume-prune.1.md +++ b/docs/source/markdown/podman-volume-prune.1.md @@ -21,6 +21,8 @@ Provide filter values. The *filters* argument format is of `key=value`. If there is more than one *filter*, then pass multiple OPTIONS: **--filter** *foo=bar* **--filter** *bif=baz*. +Filters with the same key work inclusive (OR logic). Filters with different keys always work exclusive (AND logic). + Supported filters: | Filter | Description | diff --git a/libpod/runtime_volume.go b/libpod/runtime_volume.go index ff04eec3cc..c6538a265d 100644 --- a/libpod/runtime_volume.go +++ b/libpod/runtime_volume.go @@ -70,8 +70,8 @@ func (r *Runtime) HasVolume(name string) (bool, error) { // Volumes retrieves all volumes // Filters can be provided which will determine which volumes are included in the -// output. If multiple filters are used, a volume will be returned if -// any of the filters are matched +// output. If multiple filters are used, a volume will be returned only if +// all of the filters are matched func (r *Runtime) Volumes(filters ...VolumeFilter) ([]*Volume, error) { if !r.valid { return nil, define.ErrRuntimeStopped @@ -88,9 +88,12 @@ func (r *Runtime) Volumes(filters ...VolumeFilter) ([]*Volume, error) { volsFiltered := make([]*Volume, 0, len(vols)) for _, vol := range vols { - include := false + include := true for _, filter := range filters { - include = include || filter(vol) + if !filter(vol) { + include = false + break + } } if include { diff --git a/pkg/domain/filters/volumes.go b/pkg/domain/filters/volumes.go index 07e2cac86c..ed8f62d501 100644 --- a/pkg/domain/filters/volumes.go +++ b/pkg/domain/filters/volumes.go @@ -39,12 +39,79 @@ func GenerateVolumeFilters(filter string, filterValues []string, runtime *libpod return false }, nil case "label": + // Group filter values by label key to implement correct OR within key, AND across keys logic + labelKeyGroups := make(map[string][]string) + for _, filterValue := range filterValues { + key, value, hasValue := strings.Cut(filterValue, "=") + if !hasValue { + // Handle label key only filters (no value) + labelKeyGroups[key] = append(labelKeyGroups[key], "") + } else { + labelKeyGroups[key] = append(labelKeyGroups[key], value) + } + } + return func(v *libpod.Volume) bool { - return filters.MatchLabelFilters(filterValues, v.Labels()) + volumeLabels := v.Labels() + // ALL label keys must match (AND across keys) + for labelKey, values := range labelKeyGroups { + keyMatched := false + // ANY value within the same key can match (OR within key) + for _, value := range values { + if volumeLabel, exists := volumeLabels[labelKey]; exists { + if value == "" || volumeLabel == value { + keyMatched = true + break + } + } else if value == "" { + // Key doesn't exist but we're looking for key existence only + keyMatched = false + break + } + } + if !keyMatched { + return false + } + } + return true }, nil case "label!": + // Group filter values by label key for negative matching + labelKeyGroups := make(map[string][]string) + for _, filterValue := range filterValues { + key, value, hasValue := strings.Cut(filterValue, "=") + if !hasValue { + // Handle label key only filters (no value) + labelKeyGroups[key] = append(labelKeyGroups[key], "") + } else { + labelKeyGroups[key] = append(labelKeyGroups[key], value) + } + } + return func(v *libpod.Volume) bool { - return !filters.MatchLabelFilters(filterValues, v.Labels()) + volumeLabels := v.Labels() + // ALL label keys must NOT match (AND across keys for negation) + for labelKey, values := range labelKeyGroups { + keyMatched := false + // ANY value within the same key can match (OR within key) + for _, value := range values { + if volumeLabel, exists := volumeLabels[labelKey]; exists { + if value == "" || volumeLabel == value { + keyMatched = true + break + } + } else if value == "" { + // Key doesn't exist but we're looking for key existence only + keyMatched = false + break + } + } + // For negation, if any key matched, the filter fails + if keyMatched { + return false + } + } + return true }, nil case "opt": return func(v *libpod.Volume) bool { @@ -101,12 +168,79 @@ func GeneratePruneVolumeFilters(filter string, filterValues []string, runtime *l case "after", "since": return createAfterFilterVolumeFunction(filterValues, runtime) case "label": + // Group filter values by label key to implement correct OR within key, AND across keys logic + labelKeyGroups := make(map[string][]string) + for _, filterValue := range filterValues { + key, value, hasValue := strings.Cut(filterValue, "=") + if !hasValue { + // Handle label key only filters (no value) + labelKeyGroups[key] = append(labelKeyGroups[key], "") + } else { + labelKeyGroups[key] = append(labelKeyGroups[key], value) + } + } + return func(v *libpod.Volume) bool { - return filters.MatchLabelFilters(filterValues, v.Labels()) + volumeLabels := v.Labels() + // ALL label keys must match (AND across keys) + for labelKey, values := range labelKeyGroups { + keyMatched := false + // ANY value within the same key can match (OR within key) + for _, value := range values { + if volumeLabel, exists := volumeLabels[labelKey]; exists { + if value == "" || volumeLabel == value { + keyMatched = true + break + } + } else if value == "" { + // Key doesn't exist but we're looking for key existence only + keyMatched = false + break + } + } + if !keyMatched { + return false + } + } + return true }, nil case "label!": + // Group filter values by label key for negative matching + labelKeyGroups := make(map[string][]string) + for _, filterValue := range filterValues { + key, value, hasValue := strings.Cut(filterValue, "=") + if !hasValue { + // Handle label key only filters (no value) + labelKeyGroups[key] = append(labelKeyGroups[key], "") + } else { + labelKeyGroups[key] = append(labelKeyGroups[key], value) + } + } + return func(v *libpod.Volume) bool { - return !filters.MatchLabelFilters(filterValues, v.Labels()) + volumeLabels := v.Labels() + // ALL label keys must NOT match (AND across keys for negation) + for labelKey, values := range labelKeyGroups { + keyMatched := false + // ANY value within the same key can match (OR within key) + for _, value := range values { + if volumeLabel, exists := volumeLabels[labelKey]; exists { + if value == "" || volumeLabel == value { + keyMatched = true + break + } + } else if value == "" { + // Key doesn't exist but we're looking for key existence only + keyMatched = false + break + } + } + // For negation, if any key matched, the filter fails + if keyMatched { + return false + } + } + return true }, nil case "until": return createUntilFilterVolumeFunction(filterValues) diff --git a/test/e2e/volume_ls_test.go b/test/e2e/volume_ls_test.go index 27dce14202..51a09bf9c2 100644 --- a/test/e2e/volume_ls_test.go +++ b/test/e2e/volume_ls_test.go @@ -4,6 +4,7 @@ package integration import ( "fmt" + "time" . "github.com/containers/podman/v5/test/utils" . "github.com/onsi/ginkgo/v2" @@ -216,6 +217,118 @@ var _ = Describe("Podman volume ls", func() { Expect(session.OutputToStringArray()[0]).To(Equal(vol3Name)) }) + It("podman ls volume filters should combine with AND logic", func() { + // Create volumes with different label combinations to test AND logic + session := podmanTest.Podman([]string{"volume", "create", "--label", "a=b", "vol-with-a"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + volWithA := session.OutputToString() + + session = podmanTest.Podman([]string{"volume", "create", "--label", "c=d", "vol-with-c"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + volWithC := session.OutputToString() + + session = podmanTest.Podman([]string{"volume", "create", "--label", "a=b", "--label", "c=d", "vol-with-both"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + volWithBoth := session.OutputToString() + + // Sleep to ensure time difference for until filter + time.Sleep(1100 * time.Millisecond) + session = podmanTest.Podman([]string{"volume", "create", "--label", "a=b", "vol-new"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + volNew := session.OutputToString() + + // Test AND logic: both label=a=b AND label=c=d must match + session = podmanTest.Podman([]string{"volume", "ls", "-q", "--filter", "label=a=b", "--filter", "label=c=d"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + // Only vol-with-both should match when filters are combined with AND + Expect(session.OutputToStringArray()).To(HaveLen(1)) + Expect(session.OutputToStringArray()[0]).To(Equal(volWithBoth)) + + // Test AND logic: label=a=b AND label!=c=d must match + session = podmanTest.Podman([]string{"volume", "ls", "-q", "--filter", "label=a=b", "--filter", "label!=c=d"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + // Only volWithA and volNew should match (have a=b but not c=d) + Expect(session.OutputToStringArray()).To(HaveLen(2)) + Expect(session.OutputToStringArray()).To(ContainElement(volWithA)) + Expect(session.OutputToStringArray()).To(ContainElement(volNew)) + + // Test that individual filters still work correctly + session = podmanTest.Podman([]string{"volume", "ls", "-q", "--filter", "label=a=b"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + // Should match all volumes with a=b label + Expect(session.OutputToStringArray()).To(HaveLen(3)) + Expect(session.OutputToStringArray()).To(ContainElement(volWithA)) + Expect(session.OutputToStringArray()).To(ContainElement(volWithBoth)) + Expect(session.OutputToStringArray()).To(ContainElement(volNew)) + + session = podmanTest.Podman([]string{"volume", "ls", "-q", "--filter", "label!=c=d"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + // Should match all volumes without c=d label + Expect(session.OutputToStringArray()).To(HaveLen(2)) + Expect(session.OutputToStringArray()).To(ContainElement(volWithA)) + Expect(session.OutputToStringArray()).To(ContainElement(volNew)) + + // Test filtering for volumes with c=d label + session = podmanTest.Podman([]string{"volume", "ls", "-q", "--filter", "label=c=d"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + // Should match volumes with c=d label + Expect(session.OutputToStringArray()).To(HaveLen(2)) + Expect(session.OutputToStringArray()).To(ContainElement(volWithC)) + Expect(session.OutputToStringArray()).To(ContainElement(volWithBoth)) + }) + + It("podman ls volume filter within-key OR logic combined with cross-key AND logic", func() { + // Test that values within the same filter key use OR, but different keys use AND + session := podmanTest.Podman([]string{"volume", "create", "--label", "env=prod", "vol-prod"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + volProd := session.OutputToString() + + session = podmanTest.Podman([]string{"volume", "create", "--label", "env=dev", "vol-dev"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + volDev := session.OutputToString() + + session = podmanTest.Podman([]string{"volume", "create", "--label", "env=test", "vol-test"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + volTest := session.OutputToString() + + session = podmanTest.Podman([]string{"volume", "create", "--label", "env=prod", "--label", "team=alpha", "vol-prod-alpha"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + volProdAlpha := session.OutputToString() + + session = podmanTest.Podman([]string{"volume", "create", "--label", "env=dev", "--label", "team=alpha", "vol-dev-alpha"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + volDevAlpha := session.OutputToString() + + // Test: env=(prod OR dev) AND team=alpha - should match volumes with team=alpha AND (env=prod OR env=dev) + session = podmanTest.Podman([]string{"volume", "ls", "-q", "--filter", "label=env=prod", "--filter", "label=env=dev", "--filter", "label=team=alpha"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + result := session.OutputToStringArray() + + // Should match both volumes that have team=alpha AND have either env=prod OR env=dev + Expect(result).To(HaveLen(2)) + Expect(result).To(ContainElement(volProdAlpha)) + Expect(result).To(ContainElement(volDevAlpha)) + // Should not match volumes without team=alpha, even if they have the right env + Expect(result).NotTo(ContainElement(volProd)) + Expect(result).NotTo(ContainElement(volDev)) + Expect(result).NotTo(ContainElement(volTest)) + }) + It("podman ls volume with --filter since/after", func() { vol1 := "vol1" vol2 := "vol2" diff --git a/test/e2e/volume_prune_test.go b/test/e2e/volume_prune_test.go index f293f38729..27e16a94d2 100644 --- a/test/e2e/volume_prune_test.go +++ b/test/e2e/volume_prune_test.go @@ -192,4 +192,74 @@ var _ = Describe("Podman volume prune", func() { Expect(session.OutputToStringArray()).To(HaveLen(1)) Expect(session.OutputToStringArray()[0]).To(Equal(vol1)) }) + + It("podman volume prune filters should combine with AND logic", func() { + // Create volumes with different label combinations to test AND logic + session := podmanTest.Podman([]string{"volume", "create", "--label", "a=b", "prune-vol-with-a"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + volWithA := session.OutputToString() + + session = podmanTest.Podman([]string{"volume", "create", "--label", "c=d", "prune-vol-with-c"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + volWithC := session.OutputToString() + + session = podmanTest.Podman([]string{"volume", "create", "--label", "a=b", "--label", "c=d", "prune-vol-with-both"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + volWithBoth := session.OutputToString() + + // Verify all volumes exist before pruning + session = podmanTest.Podman([]string{"volume", "ls", "-q"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + Expect(session.OutputToStringArray()).To(HaveLen(3)) // 3 created volumes + + // Test AND logic: only volumes with both label=a=b AND label=c=d should be pruned + session = podmanTest.Podman([]string{"volume", "prune", "-f", "--filter", "label=a=b", "--filter", "label=c=d"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + + // Check remaining volumes - only volWithBoth should be pruned + session = podmanTest.Podman([]string{"volume", "ls", "-q"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + remaining := session.OutputToStringArray() + Expect(remaining).To(HaveLen(2)) // 2 remaining volumes + Expect(remaining).To(ContainElement(volWithA)) + Expect(remaining).To(ContainElement(volWithC)) + Expect(remaining).NotTo(ContainElement(volWithBoth)) + + // Clean up for next test + session = podmanTest.Podman([]string{"volume", "prune", "-f"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + + // Create new volumes for next test + session = podmanTest.Podman([]string{"volume", "create", "--label", "a=b", "prune-vol-with-a2"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + volWithA2 := session.OutputToString() + + session = podmanTest.Podman([]string{"volume", "create", "--label", "a=b", "--label", "c=d", "prune-vol-with-both2"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + volWithBoth2 := session.OutputToString() + + // Test AND logic: label=a=b AND label!=c=d should only prune volumes with a=b but without c=d + session = podmanTest.Podman([]string{"volume", "prune", "-f", "--filter", "label=a=b", "--filter", "label!=c=d"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + + // Check remaining volumes - only volWithA2 should be pruned, volWithBoth2 should remain + session = podmanTest.Podman([]string{"volume", "ls", "-q"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + remaining = session.OutputToStringArray() + Expect(remaining).To(HaveLen(1)) // 1 remaining volume + Expect(remaining).To(ContainElement(volWithBoth2)) + Expect(remaining).NotTo(ContainElement(volWithA2)) + }) + })