diff --git a/libimage/filters.go b/libimage/filters.go index d84511d43..369eff94a 100644 --- a/libimage/filters.go +++ b/libimage/filters.go @@ -20,14 +20,11 @@ import ( // indicates that the image matches the criteria. type filterFunc func(*Image) (bool, error) -// Apply the specified filters. At least one filter of each key must apply. +// Apply the specified filters. All filters of each key must apply. func (i *Image) applyFilters(ctx context.Context, filters map[string][]filterFunc) (bool, error) { - matches := false - for key := range filters { // and - matches = false - for _, filter := range filters[key] { // or - var err error - matches, err = filter(i) + for key := range filters { + for _, filter := range filters[key] { + matches, err := filter(i) if err != nil { // Some images may have been corrupted in the // meantime, so do an extra check and make the @@ -38,15 +35,13 @@ func (i *Image) applyFilters(ctx context.Context, filters map[string][]filterFun } return false, err } - if matches { - break + // If any filter within a group doesn't match, return false + if !matches { + return false, nil } } - if !matches { - return false, nil - } } - return matches, nil + return true, nil } // filterImages returns a slice of images which are passing all specified @@ -92,6 +87,7 @@ func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOp return tree, nil } + var wantedReferenceMatches, unwantedReferenceMatches []string filters := map[string][]filterFunc{} duplicate := map[string]string{} for _, f := range options.Filters { @@ -183,7 +179,12 @@ func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOp filter = filterManifest(ctx, manifest) case "reference": - filter = filterReferences(r, value) + if negate { + unwantedReferenceMatches = append(unwantedReferenceMatches, value) + } else { + wantedReferenceMatches = append(wantedReferenceMatches, value) + } + continue case "until": until, err := r.until(value) @@ -201,6 +202,11 @@ func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOp filters[key] = append(filters[key], filter) } + // reference filters is a special case as it does an OR for positive matches + // and an AND logic for negative matches + filter := filterReferences(r, wantedReferenceMatches, unwantedReferenceMatches) + filters["reference"] = append(filters["reference"], filter) + return filters, nil } @@ -272,55 +278,97 @@ func filterManifest(ctx context.Context, value bool) filterFunc { } } -// filterReferences creates a reference filter for matching the specified value. -func filterReferences(r *Runtime, value string) filterFunc { - lookedUp, _, _ := r.LookupImage(value, nil) +// filterReferences creates a reference filter for matching the specified wantedReferenceMatches value (OR logic) +// and for matching the unwantedReferenceMatches values (AND logic) +func filterReferences(r *Runtime, wantedReferenceMatches, unwantedReferenceMatches []string) filterFunc { return func(img *Image) (bool, error) { - if lookedUp != nil { - if lookedUp.ID() == img.ID() { + // Empty reference filters, return true + if len(wantedReferenceMatches) == 0 && len(unwantedReferenceMatches) == 0 { + return true, nil + } + + unwantedMatched := false + // Go through the unwanted matches first + for _, value := range unwantedReferenceMatches { + matches, err := imageMatchesReferenceFilter(r, img, value) + if err != nil { + return false, err + } + if matches { + unwantedMatched = true + } + } + + // If there are no wanted match filters, then return false for the image + // that matched the unwanted value otherwise return true + if len(wantedReferenceMatches) == 0 { + return !unwantedMatched, nil + } + + // Go through the wanted matches + // If an image matches the wanted filter but it also matches the unwanted + // filter, don't add it to the output + for _, value := range wantedReferenceMatches { + matches, err := imageMatchesReferenceFilter(r, img, value) + if err != nil { + return false, err + } + if matches && !unwantedMatched { return true, nil } } - refs, err := img.NamesReferences() - if err != nil { - return false, err + return false, nil + } +} + +// imageMatchesReferenceFilter returns true if an image matches the filter value given +func imageMatchesReferenceFilter(r *Runtime, img *Image, value string) (bool, error) { + lookedUp, _, _ := r.LookupImage(value, nil) + if lookedUp != nil { + if lookedUp.ID() == img.ID() { + return true, nil } + } - for _, ref := range refs { - refString := ref.String() // FQN with tag/digest - candidates := []string{refString} + refs, err := img.NamesReferences() + if err != nil { + return false, err + } - // Split the reference into 3 components (twice if digested/tagged): - // 1) Fully-qualified reference - // 2) Without domain - // 3) Without domain and path - if named, isNamed := ref.(reference.Named); isNamed { + for _, ref := range refs { + refString := ref.String() // FQN with tag/digest + candidates := []string{refString} + + // Split the reference into 3 components (twice if digested/tagged): + // 1) Fully-qualified reference + // 2) Without domain + // 3) Without domain and path + if named, isNamed := ref.(reference.Named); isNamed { + candidates = append(candidates, + reference.Path(named), // path/name without tag/digest (Path() removes it) + refString[strings.LastIndex(refString, "/")+1:]) // name with tag/digest + + trimmedString := reference.TrimNamed(named).String() + if refString != trimmedString { + tagOrDigest := refString[len(trimmedString):] candidates = append(candidates, - reference.Path(named), // path/name without tag/digest (Path() removes it) - refString[strings.LastIndex(refString, "/")+1:]) // name with tag/digest - - trimmedString := reference.TrimNamed(named).String() - if refString != trimmedString { - tagOrDigest := refString[len(trimmedString):] - candidates = append(candidates, - trimmedString, // FQN without tag/digest - reference.Path(named)+tagOrDigest, // path/name with tag/digest - trimmedString[strings.LastIndex(trimmedString, "/")+1:]) // name without tag/digest - } + trimmedString, // FQN without tag/digest + reference.Path(named)+tagOrDigest, // path/name with tag/digest + trimmedString[strings.LastIndex(trimmedString, "/")+1:]) // name without tag/digest } + } - for _, candidate := range candidates { - // path.Match() is also used by Docker's reference.FamiliarMatch(). - matched, _ := path.Match(value, candidate) - if matched { - return true, nil - } + for _, candidate := range candidates { + // path.Match() is also used by Docker's reference.FamiliarMatch(). + matched, _ := path.Match(value, candidate) + if matched { + return true, nil } } - - return false, nil } + + return false, nil } // filterLabel creates a label for matching the specified value. diff --git a/libimage/filters_test.go b/libimage/filters_test.go index 7220e1eb8..86d3a1fe4 100644 --- a/libimage/filters_test.go +++ b/libimage/filters_test.go @@ -5,6 +5,7 @@ package libimage import ( "context" "os" + "strings" "testing" "time" @@ -41,50 +42,64 @@ func TestFilterReference(t *testing.T) { require.NoError(t, err) for _, test := range []struct { - filter string + filters []string matches int }{ - {"image", 2}, - {"*mage*", 2}, - {"image:*", 2}, - {"image:tag", 1}, - {"image:another-tag", 1}, - {"localhost/image", 1}, - {"localhost/image:tag", 1}, - {"library/image", 1}, - {"docker.io/library/image*", 1}, - {"docker.io/library/image:*", 1}, - {"docker.io/library/image:another-tag", 1}, - {"localhost/*", 2}, - {"localhost/image:*tag", 1}, - {"localhost/*mage:*ag", 2}, - {"quay.io/libpod/busybox", 1}, - {"quay.io/libpod/alpine", 1}, - {"quay.io/libpod", 0}, - {"quay.io/libpod/*", 2}, - {"busybox", 1}, - {"alpine", 1}, - {"alpine@" + alpine.Digest().String(), 1}, - {"alpine:latest@" + alpine.Digest().String(), 1}, - {"quay.io/libpod/alpine@" + alpine.Digest().String(), 1}, - {"quay.io/libpod/alpine:latest@" + alpine.Digest().String(), 1}, + {[]string{"image"}, 2}, + {[]string{"*mage*"}, 2}, + {[]string{"image:*"}, 2}, + {[]string{"image:tag"}, 1}, + {[]string{"image:another-tag"}, 1}, + {[]string{"localhost/image"}, 1}, + {[]string{"localhost/image:tag"}, 1}, + {[]string{"library/image"}, 1}, + {[]string{"docker.io/library/image*"}, 1}, + {[]string{"docker.io/library/image:*"}, 1}, + {[]string{"docker.io/library/image:another-tag"}, 1}, + {[]string{"localhost/*"}, 2}, + {[]string{"localhost/image:*tag"}, 1}, + {[]string{"localhost/*mage:*ag"}, 2}, + {[]string{"quay.io/libpod/busybox"}, 1}, + {[]string{"quay.io/libpod/alpine"}, 1}, + {[]string{"quay.io/libpod"}, 0}, + {[]string{"quay.io/libpod/*"}, 2}, + {[]string{"busybox"}, 1}, + {[]string{"alpine"}, 1}, + {[]string{"alpine@" + alpine.Digest().String()}, 1}, + {[]string{"alpine:latest@" + alpine.Digest().String()}, 1}, + {[]string{"quay.io/libpod/alpine@" + alpine.Digest().String()}, 1}, + {[]string{"quay.io/libpod/alpine:latest@" + alpine.Digest().String()}, 1}, + // Make sure negate works as expected + {[]string{"!alpine"}, 1}, + {[]string{"!alpine", "!busybox"}, 0}, + {[]string{"!alpine", "busybox"}, 1}, + {[]string{"alpine", "busybox"}, 2}, + {[]string{"*test", "!*box"}, 1}, // Make sure that tags are ignored - {"alpine:ignoreme@" + alpine.Digest().String(), 1}, - {"alpine:123@" + alpine.Digest().String(), 1}, - {"quay.io/libpod/alpine:hurz@" + alpine.Digest().String(), 1}, - {"quay.io/libpod/alpine:456@" + alpine.Digest().String(), 1}, + {[]string{"alpine:ignoreme@" + alpine.Digest().String()}, 1}, + {[]string{"alpine:123@" + alpine.Digest().String()}, 1}, + {[]string{"quay.io/libpod/alpine:hurz@" + alpine.Digest().String()}, 1}, + {[]string{"quay.io/libpod/alpine:456@" + alpine.Digest().String()}, 1}, // Make sure that repo and digest must match - {"alpine:busyboxdigest@" + busybox.Digest().String(), 0}, - {"alpine:busyboxdigest@" + busybox.Digest().String(), 0}, - {"quay.io/libpod/alpine:busyboxdigest@" + busybox.Digest().String(), 0}, - {"quay.io/libpod/alpine:busyboxdigest@" + busybox.Digest().String(), 0}, + {[]string{"alpine:busyboxdigest@" + busybox.Digest().String()}, 0}, + {[]string{"alpine:busyboxdigest@" + busybox.Digest().String()}, 0}, + {[]string{"quay.io/libpod/alpine:busyboxdigest@" + busybox.Digest().String()}, 0}, + {[]string{"quay.io/libpod/alpine:busyboxdigest@" + busybox.Digest().String()}, 0}, } { + var filters []string + for _, filter := range test.filters { + if strings.HasPrefix(filter, "!") { + filters = append(filters, "reference!="+filter[1:]) + } else { + filters = append(filters, "reference="+filter) + } + } listOptions := &ListImagesOptions{ - Filters: []string{"reference=" + test.filter}, + Filters: filters, } listedImages, err := runtime.ListImages(ctx, nil, listOptions) require.NoError(t, err, "%v", test) - require.Len(t, listedImages, test.matches, "%s -> %v", test.filter, listedImages) + require.Len(t, listedImages, test.matches, "%s -> %v", test.filters, listedImages) } } diff --git a/pkg/filters/filters.go b/pkg/filters/filters.go index 3d442a530..3370a7c65 100644 --- a/pkg/filters/filters.go +++ b/pkg/filters/filters.go @@ -76,13 +76,10 @@ func FiltersFromRequest(r *http.Request) ([]string, error) { libpodFilters := make([]string, 0, len(filters)) for filterKey, filterSlice := range filters { - f := filterKey for _, filterValue := range filterSlice { - f += "=" + filterValue + libpodFilters = append(libpodFilters, fmt.Sprintf("%s=%s", filterKey, filterValue)) } - libpodFilters = append(libpodFilters, f) } - return libpodFilters, nil } diff --git a/pkg/filters/filters_test.go b/pkg/filters/filters_test.go index e53708283..3be6a6399 100644 --- a/pkg/filters/filters_test.go +++ b/pkg/filters/filters_test.go @@ -1,7 +1,12 @@ package filters import ( + "net/http" + "net/url" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestMatchLabelFilters(t *testing.T) { @@ -185,3 +190,20 @@ func TestComputeUntilTimestamp(t *testing.T) { }) } } + +func TestFiltersFromRequest(t *testing.T) { + // simulate https request + req := http.Request{ + URL: &url.URL{ + RawQuery: "all=false&filters=%7B%22label%22%3A%5B%22xyz%3Dbar%22%2C%22abc%22%5D%2C%22reference%22%3A%5B%22test%22%5D%7D", + }, + } + // call req.ParseForm so it can parse the RawQuery data + err := req.ParseForm() + require.NoError(t, err) + + expectedLibpodFilters := []string{"label=xyz=bar", "label=abc", "reference=test"} + got, err := FiltersFromRequest(&req) + require.NoError(t, err) + assert.Equal(t, expectedLibpodFilters, got) +}