Skip to content

Commit ba9f9e1

Browse files
committed
Fix unexpected results when filtering images
The filtered image contains all Names of image. This causes the podman to list images that are not expected. For example: If an image `box:latest` is taged as `example:latest` and then images are filtered with `reference=example`, `box:latest` and `example:latest` will be output because the image has multiple names. Fixes: containers/podman#25725 Fixes: https://issues.redhat.com/browse/RUN-2726 Signed-off-by: Jan Rodák <[email protected]>
1 parent e8ccfbf commit ba9f9e1

File tree

4 files changed

+210
-97
lines changed

4 files changed

+210
-97
lines changed

libimage/filters.go

Lines changed: 123 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"errors"
88
"fmt"
99
"path"
10+
"slices"
1011
"strconv"
1112
"strings"
1213
"time"
@@ -21,13 +22,24 @@ import (
2122
// indicates that the image matches the criteria.
2223
type filterFunc func(*Image, *layerTree) (bool, error)
2324

24-
type compiledFilters map[string][]filterFunc
25+
// referenceFilterFunc is a prototype for a filter that returns a list of
26+
// references. The first return value indicates whether the image matches the
27+
// criteria. The second return value is a list of names that match the
28+
// criteria. The third return value is an error.
29+
type referenceFilterFunc func(*Image) (bool, []string, error)
30+
31+
type compiledFilters struct {
32+
filters map[string][]filterFunc
33+
referenceFilter referenceFilterFunc
34+
needsLayerTree bool
35+
}
2536

2637
// Apply the specified filters. All filters of each key must apply.
2738
// tree must be provided if compileImageFilters indicated it is necessary.
28-
func (i *Image) applyFilters(ctx context.Context, filters compiledFilters, tree *layerTree) (bool, error) {
29-
for key := range filters {
30-
for _, filter := range filters[key] {
39+
// WARNING: Application of referenceFilter sets the image names to matched names, but this only affects the values in memory, they are not written to storage.
40+
func (i *Image) applyFilters(ctx context.Context, f *compiledFilters, tree *layerTree) (bool, error) {
41+
for key := range f.filters {
42+
for _, filter := range f.filters[key] {
3143
matches, err := filter(i, tree)
3244
if err != nil {
3345
// Some images may have been corrupted in the
@@ -45,13 +57,33 @@ func (i *Image) applyFilters(ctx context.Context, filters compiledFilters, tree
4557
}
4658
}
4759
}
60+
if f.referenceFilter != nil {
61+
referenceMatch, names, err := f.referenceFilter(i)
62+
if err != nil {
63+
// Some images may have been corrupted in the
64+
// meantime, so do an extra check and make the
65+
// error non-fatal (see containers/podman/issues/12582).
66+
if errCorrupted := i.isCorrupted(ctx, ""); errCorrupted != nil {
67+
logrus.Error(errCorrupted.Error())
68+
return false, nil
69+
}
70+
return false, err
71+
}
72+
if !referenceMatch {
73+
return false, nil
74+
}
75+
if len(names) > 0 {
76+
i.setEphemeralNames(names)
77+
}
78+
}
4879
return true, nil
4980
}
5081

5182
// filterImages returns a slice of images which are passing all specified
5283
// filters.
5384
// tree must be provided if compileImageFilters indicated it is necessary.
54-
func (r *Runtime) filterImages(ctx context.Context, images []*Image, filters compiledFilters, tree *layerTree) ([]*Image, error) {
85+
// WARNING: Application of referenceFilter sets the image names to matched names, but this only affects the values in memory, they are not written to storage.
86+
func (r *Runtime) filterImages(ctx context.Context, images []*Image, filters *compiledFilters, tree *layerTree) ([]*Image, error) {
5587
result := []*Image{}
5688
for i := range images {
5789
match, err := images[i].applyFilters(ctx, filters, tree)
@@ -71,17 +103,19 @@ func (r *Runtime) filterImages(ctx context.Context, images []*Image, filters com
71103
// after, since, before, containers, dangling, id, label, readonly, reference, intermediate
72104
//
73105
// compileImageFilters returns: compiled filters, if LayerTree is needed, error
74-
func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOptions) (compiledFilters, bool, error) {
106+
func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOptions) (*compiledFilters, error) {
75107
logrus.Tracef("Parsing image filters %s", options.Filters)
76108
if len(options.Filters) == 0 {
77-
return nil, false, nil
109+
return &compiledFilters{}, nil
78110
}
79111

80112
filterInvalidValue := `invalid image filter %q: must be in the format "filter=value or filter!=value"`
81113

82114
var wantedReferenceMatches, unwantedReferenceMatches []string
83-
filters := compiledFilters{}
84-
needsLayerTree := false
115+
cf := compiledFilters{
116+
filters: map[string][]filterFunc{},
117+
needsLayerTree: false,
118+
}
85119
duplicate := map[string]string{}
86120
for _, f := range options.Filters {
87121
var key, value string
@@ -93,7 +127,7 @@ func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOp
93127
} else {
94128
split = strings.SplitN(f, "=", 2)
95129
if len(split) != 2 {
96-
return nil, false, fmt.Errorf(filterInvalidValue, f)
130+
return nil, fmt.Errorf(filterInvalidValue, f)
97131
}
98132
}
99133

@@ -103,30 +137,30 @@ func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOp
103137
case "after", "since":
104138
img, err := r.time(key, value)
105139
if err != nil {
106-
return nil, false, err
140+
return nil, err
107141
}
108142
key = "since"
109143
filter = filterAfter(img.Created())
110144

111145
case "before":
112146
img, err := r.time(key, value)
113147
if err != nil {
114-
return nil, false, err
148+
return nil, err
115149
}
116150
filter = filterBefore(img.Created())
117151

118152
case "containers":
119153
if err := r.containers(duplicate, key, value, options.IsExternalContainerFunc); err != nil {
120-
return nil, false, err
154+
return nil, err
121155
}
122156
filter = filterContainers(value, options.IsExternalContainerFunc)
123157

124158
case "dangling":
125159
dangling, err := r.bool(duplicate, key, value)
126160
if err != nil {
127-
return nil, false, err
161+
return nil, err
128162
}
129-
needsLayerTree = true
163+
cf.needsLayerTree = true
130164
filter = filterDangling(ctx, dangling)
131165

132166
case "id":
@@ -135,31 +169,31 @@ func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOp
135169
case "digest":
136170
f, err := filterDigest(value)
137171
if err != nil {
138-
return nil, false, err
172+
return nil, err
139173
}
140174
filter = f
141175

142176
case "intermediate":
143177
intermediate, err := r.bool(duplicate, key, value)
144178
if err != nil {
145-
return nil, false, err
179+
return nil, err
146180
}
147-
needsLayerTree = true
181+
cf.needsLayerTree = true
148182
filter = filterIntermediate(ctx, intermediate)
149183

150184
case "label":
151185
filter = filterLabel(ctx, value)
152186
case "readonly":
153187
readOnly, err := r.bool(duplicate, key, value)
154188
if err != nil {
155-
return nil, false, err
189+
return nil, err
156190
}
157191
filter = filterReadOnly(readOnly)
158192

159193
case "manifest":
160194
manifest, err := r.bool(duplicate, key, value)
161195
if err != nil {
162-
return nil, false, err
196+
return nil, err
163197
}
164198
filter = filterManifest(ctx, manifest)
165199

@@ -174,25 +208,23 @@ func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOp
174208
case "until":
175209
until, err := r.until(value)
176210
if err != nil {
177-
return nil, false, err
211+
return nil, err
178212
}
179213
filter = filterBefore(until)
180214

181215
default:
182-
return nil, false, fmt.Errorf(filterInvalidValue, key)
216+
return nil, fmt.Errorf(filterInvalidValue, key)
183217
}
184218
if negate {
185219
filter = negateFilter(filter)
186220
}
187-
filters[key] = append(filters[key], filter)
221+
cf.filters[key] = append(cf.filters[key], filter)
188222
}
189223

190224
// reference filters is a special case as it does an OR for positive matches
191-
// and an AND logic for negative matches
192-
filter := filterReferences(r, wantedReferenceMatches, unwantedReferenceMatches)
193-
filters["reference"] = append(filters["reference"], filter)
194-
195-
return filters, needsLayerTree, nil
225+
// and an AND logic for negative matches and the filter function type is different.
226+
cf.referenceFilter = filterReferences(r, wantedReferenceMatches, unwantedReferenceMatches)
227+
return &cf, nil
196228
}
197229

198230
func negateFilter(f filterFunc) filterFunc {
@@ -265,62 +297,100 @@ func filterManifest(ctx context.Context, value bool) filterFunc {
265297

266298
// filterReferences creates a reference filter for matching the specified wantedReferenceMatches value (OR logic)
267299
// and for matching the unwantedReferenceMatches values (AND logic)
268-
func filterReferences(r *Runtime, wantedReferenceMatches, unwantedReferenceMatches []string) filterFunc {
269-
return func(img *Image, _ *layerTree) (bool, error) {
300+
func filterReferences(r *Runtime, wantedReferenceMatches, unwantedReferenceMatches []string) referenceFilterFunc {
301+
return func(img *Image) (bool, []string, error) {
270302
// Empty reference filters, return true
271303
if len(wantedReferenceMatches) == 0 && len(unwantedReferenceMatches) == 0 {
272-
return true, nil
304+
return true, nil, nil
273305
}
274306

275-
unwantedMatched := false
276307
// Go through the unwanted matches first
308+
// TODO 6.0 podman: remove unwanted matches from the output names. https://github.com/containers/common/pull/2413#discussion_r2031749013
277309
for _, value := range unwantedReferenceMatches {
278-
matches, err := imageMatchesReferenceFilter(r, img, value)
310+
names, err := getMatchedImageNames(r, img, value)
279311
if err != nil {
280-
return false, err
312+
return false, nil, err
281313
}
282-
if matches {
283-
unwantedMatched = true
314+
if len(names) > 0 {
315+
return false, nil, nil
284316
}
285317
}
286318

287-
// If there are no wanted match filters, then return false for the image
288-
// that matched the unwanted value otherwise return true
319+
namesThatMatch := slices.Clone(img.Names())
320+
// If there are no wanted match filters, then return true for the image
321+
// that don't march matched the unwanted filters.
289322
if len(wantedReferenceMatches) == 0 {
290-
return !unwantedMatched, nil
323+
return true, namesThatMatch, nil
291324
}
292325

293-
// Go through the wanted matches
294326
// If an image matches the wanted filter but it also matches the unwanted
295327
// filter, don't add it to the output
328+
matchedNames := map[string]struct{}{}
329+
330+
// If the wanted reference is RepoDigest and image match. All names of image are returned.
331+
isRepoDigest := false
332+
296333
for _, value := range wantedReferenceMatches {
297-
matches, err := imageMatchesReferenceFilter(r, img, value)
334+
names, err := getMatchedImageNames(r, img, value)
298335
if err != nil {
299-
return false, err
336+
return false, nil, err
337+
}
338+
339+
for name := range names {
340+
repoDigests, err := img.RepoDigests()
341+
if err != nil {
342+
return false, nil, err
343+
}
344+
for _, repoDigest := range repoDigests {
345+
if name == repoDigest {
346+
isRepoDigest = true
347+
break
348+
}
349+
}
350+
351+
if isRepoDigest {
352+
break
353+
}
354+
matchedNames[name] = struct{}{}
300355
}
301-
if matches && !unwantedMatched {
302-
return true, nil
356+
357+
if isRepoDigest {
358+
break
303359
}
304360
}
305361

306-
return false, nil
362+
if isRepoDigest {
363+
return true, namesThatMatch, nil
364+
}
365+
366+
if len(matchedNames) > 0 {
367+
// Removes non-compliant names from image names
368+
namesThatMatch = slices.DeleteFunc(namesThatMatch, func(name string) bool {
369+
_, ok := matchedNames[name]
370+
return !ok
371+
})
372+
return true, namesThatMatch, nil
373+
}
374+
375+
return false, nil, nil
307376
}
308377
}
309378

310-
// imageMatchesReferenceFilter returns true if an image matches the filter value given
311-
func imageMatchesReferenceFilter(r *Runtime, img *Image, value string) (bool, error) {
312-
lookedUp, _, _ := r.LookupImage(value, nil)
379+
// getMatchedImageNames returns a set of matching image names that match the specified filter value, or an empty list if the image does not match the filter.
380+
func getMatchedImageNames(r *Runtime, img *Image, value string) (map[string]struct{}, error) {
381+
lookedUp, resolvedName, _ := r.LookupImage(value, nil)
313382
if lookedUp != nil {
314383
if lookedUp.ID() == img.ID() {
315-
return true, nil
384+
return map[string]struct{}{resolvedName: {}}, nil
316385
}
317386
}
318387

319388
refs, err := img.NamesReferences()
320389
if err != nil {
321-
return false, err
390+
return nil, err
322391
}
323392

393+
resolvedNames := map[string]struct{}{}
324394
for _, ref := range refs {
325395
refString := ref.String() // FQN with tag/digest
326396
candidates := []string{refString}
@@ -348,12 +418,12 @@ func imageMatchesReferenceFilter(r *Runtime, img *Image, value string) (bool, er
348418
// path.Match() is also used by Docker's reference.FamiliarMatch().
349419
matched, _ := path.Match(value, candidate)
350420
if matched {
351-
return true, nil
421+
resolvedNames[refString] = struct{}{}
422+
break
352423
}
353424
}
354425
}
355-
356-
return false, nil
426+
return resolvedNames, nil
357427
}
358428

359429
// filterLabel creates a label for matching the specified value.

0 commit comments

Comments
 (0)