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.

Fixes: podman-container-tools/podman#25725
Fixes: https://issues.redhat.com/browse/RUN-2726

Signed-off-by: Jan Rodák <hony.com@seznam.cz>

Author: Honny1

libimage/filters.go

@@ -25,6 +25,7 @@ type compiledFilters map[string][]filterFunc
 
 // Apply the specified filters.  All filters of each key must apply.
 // tree must be provided if compileImageFilters indicated it is necessary.
+// 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.
 func (i *Image) applyFilters(ctx context.Context, filters compiledFilters, tree *layerTree) (bool, error) {
 	for key := range filters {
 		for _, filter := range filters[key] {
@@ -51,6 +52,7 @@ func (i *Image) applyFilters(ctx context.Context, filters compiledFilters, tree
 // filterImages returns a slice of images which are passing all specified
 // filters.
 // tree must be provided if compileImageFilters indicated it is necessary.
+// 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.
 func (r *Runtime) filterImages(ctx context.Context, images []*Image, filters compiledFilters, tree *layerTree) ([]*Image, error) {
 	result := []*Image{}
 	for i := range images {
@@ -290,15 +292,31 @@ func filterReferences(r *Runtime, wantedReferenceMatches, unwantedReferenceMatch
 			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 {
+				ref, err := reference.ParseNamed(value)
+				if err == nil {
+					if !reference.IsNameOnly(ref) {
+						namesThatMatch := []string{}
+						_, containsDigest := ref.(reference.Digested)
+						for _, name := range img.Names() {
+							if containsDigest {
+								if nameRef, err := reference.ParseNamed(name); err == nil {
+									if nameRef.Name() == ref.Name() {
+										namesThatMatch = append(namesThatMatch, name)
+									}
+								}
+							} else if name == value {
+								namesThatMatch = append(namesThatMatch, name)
+							}
+						}
+						img.setEphemeralNames(namesThatMatch)
+					}
+				}
 				return true, nil
 			}
 		}
@@ -352,7 +370,6 @@ func imageMatchesReferenceFilter(r *Runtime, img *Image, value string) (bool, er
 			}
 		}
 	}
-
 	return false, nil
 }
 

libimage/filters_test.go

@@ -42,6 +42,10 @@ func TestFilterReference(t *testing.T) {
 	require.NoError(t, err)
 	err = alpine.Tag("docker.io/library/image:latest")
 	require.NoError(t, err)
+	err = busybox.Tag("localhost/aa:tag")
+	require.NoError(t, err)
+	err = busybox.Tag("localhost/a:tag")
+	require.NoError(t, err)
 
 	allAlpineNames := []string{
 		"docker.io/library/image:another-tag",
@@ -52,6 +56,8 @@ func TestFilterReference(t *testing.T) {
 	allBusyBoxNames := []string{
 		"quay.io/libpod/busybox:latest",
 		"localhost/image:tag",
+		"localhost/a:tag",
+		"localhost/aa:tag",
 	}
 	allNames := []string{}
 	allNames = append(allNames, allBusyBoxNames...)
@@ -68,11 +74,11 @@ func TestFilterReference(t *testing.T) {
 		{[]string{"image:tag"}, 1, allBusyBoxNames},
 		{[]string{"image:another-tag"}, 1, allAlpineNames},
 		{[]string{"localhost/image"}, 1, allBusyBoxNames},
-		{[]string{"localhost/image:tag"}, 1, allBusyBoxNames},
+		{[]string{"localhost/image:tag"}, 1, []string{"localhost/image:tag"}},
 		{[]string{"library/image"}, 1, allAlpineNames},
 		{[]string{"docker.io/library/image*"}, 1, allAlpineNames},
 		{[]string{"docker.io/library/image:*"}, 1, allAlpineNames},
-		{[]string{"docker.io/library/image:another-tag"}, 1, allAlpineNames},
+		{[]string{"docker.io/library/image:another-tag"}, 1, []string{"docker.io/library/image:another-tag"}},
 		{[]string{"localhost/*"}, 2, allNames},
 		{[]string{"localhost/image:*tag"}, 1, allBusyBoxNames},
 		{[]string{"localhost/*mage:*ag"}, 2, allNames},
@@ -90,18 +96,20 @@ func TestFilterReference(t *testing.T) {
 		{[]string{"alpine", "busybox"}, 2, allNames},
 		{[]string{"*test", "!*box"}, 1, allAlpineNames},
 
-		{[]string{"quay.io/libpod/alpine@" + alpine.Digest().String()}, 1, allAlpineNames},
+		{[]string{"quay.io/libpod/alpine@" + alpine.Digest().String()}, 1, []string{"quay.io/libpod/alpine:latest"}},
 
 		{[]string{"alpine@" + alpine.Digest().String()}, 1, allAlpineNames},
 		{[]string{"alpine:latest@" + alpine.Digest().String()}, 1, allAlpineNames},
-		{[]string{"quay.io/libpod/alpine:latest@" + alpine.Digest().String()}, 1, allAlpineNames},
-		{[]string{"docker.io/library/image@" + alpine.Digest().String()}, 1, allAlpineNames},
+		{[]string{"quay.io/libpod/alpine:latest@" + alpine.Digest().String()}, 1, []string{"quay.io/libpod/alpine:latest"}},
+		{[]string{"docker.io/library/image@" + alpine.Digest().String()}, 1, []string{"docker.io/library/image:latest", "docker.io/library/image:another-tag"}},
+		{[]string{"localhost/aa@" + busybox.Digest().String()}, 1, []string{"localhost/aa:tag"}},
+		{[]string{"localhost/a@" + busybox.Digest().String()}, 1, []string{"localhost/a:tag"}},
 
 		// Make sure that tags are ignored
 		{[]string{"alpine:ignoreme@" + alpine.Digest().String()}, 1, allAlpineNames},
 		{[]string{"alpine:123@" + alpine.Digest().String()}, 1, allAlpineNames},
-		{[]string{"quay.io/libpod/alpine:hurz@" + alpine.Digest().String()}, 1, allAlpineNames},
-		{[]string{"quay.io/libpod/alpine:456@" + alpine.Digest().String()}, 1, allAlpineNames},
+		{[]string{"quay.io/libpod/alpine:hurz@" + alpine.Digest().String()}, 1, []string{"quay.io/libpod/alpine:latest"}},
+		{[]string{"quay.io/libpod/alpine:456@" + alpine.Digest().String()}, 1, []string{"quay.io/libpod/alpine:latest"}},
 
 		// Make sure that repo and digest must match
 		{[]string{"alpine:busyboxdigest@" + busybox.Digest().String()}, 0, []string{}},

libimage/image.go

@@ -112,6 +112,13 @@ func (i *Image) Names() []string {
 	return i.storageImage.Names
 }
 
+// setEphemeralNames sets the names of the image.
+//
+// WARNING: this only affects the in-memory values, they are not written into the backing storage.
+func (i *Image) setEphemeralNames(names []string) {
+	i.storageImage.Names = names
+}
+
 // NamesReferences returns Names() as references.
 func (i *Image) NamesReferences() ([]reference.Reference, error) {
 	if i.cached.namesReferences != nil {

libimage/runtime.go

@@ -599,6 +599,15 @@ func (r *Runtime) ListImagesByNames(names []string) ([]*Image, error) {
 }
 
 // ListImages lists the images in the local container storage and filter the images by ListImagesOptions
+//
+// podman images consumes the output of ListImages and produces one line for each tag in each Image.Names value,
+// rather than one line for each Image with all Names, so if options.Filters contains a reference filter, it makes
+// more sense for the user to see only the corresponding names in the output, not all the names of the deduplicated
+// image; therefore, we make the corresponding names available to the caller by overwriting the actual image names
+// with the corresponding names when the reference filter matches and the reference is a fully qualified image name
+// (i.e., contains a tag or digest, not just a bare repository name).
+//
+// This overwriting is done only in memory and is not written to storage in any way.
 func (r *Runtime) ListImages(ctx context.Context, options *ListImagesOptions) ([]*Image, error) {
 	if options == nil {
 		options = &ListImagesOptions{}