Fix unexpected results when filtering images #2413

Honny1 · 2025-04-07T12:16:06Z

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

mtrmac

The imageMatchesReferenceFilter logic LGTM, that makes sense. The design of the upper parts of the call stack needs a bit of cleanup.

libimage/filters.go

libimage/filters_test.go

libimage/image.go

libimage/filters.go

mtrmac · 2025-04-08T17:47:39Z

libimage/filters.go

 	if lookedUp != nil {
 		if lookedUp.ID() == img.ID() {
-			return true, nil
+			resolvedName, _, _ := strings.Cut(resolvedName, "@")


What is this doing?

In the later loop, when we are making various imprecise matches, the return value is the full refString. This seems to be dropping digests from the returned values. Why?

I still have no idea what this is doing. AFAICS LookupImage can easily be a (precise) repo:tag or repo@digest match. And the range refs loop below always collects the precise ref.String() values that match. Why is this ignoring the digest part (if that is what it is) and returning all tags (if that is what it is)?

And, even apart from the again, HasPrefix just looks implausible. Why should example.com/a match example.com/ab?

I only want to return names that match the name if the name@digest reference format is used.

So, for example, for an image with the following names:

quay.io/libpod/alpine:latest

quay.io/libpod/al:latest

quay.io/libpod/al:tag

I expect the result for filtering with the al@digest reference, these names:

quay.io/libpod/al:latest

quay.io/libpod/al:tag

Usage HasPrefix is wrong. I fixed that.

OK… It’s very unclear to me that a filter for reference=name@digest should return name:tag (and I’ll not even comment on the ignoreme thing, I guess that’s what we deserve), but it’s a conversation we can have, or maybe a non-negotiable backward compatibility constraint.

Where do short names, and hostname-less paths, come in? For ordinary tags we have the 1)2)3) candidates for matching, and that doesn’t happen here.

I think it’s a fair point to recognize that if value contains a digest, we probably go through this code path and not the later path.Match code, so we only truly need to handle digests on this path (is that the case???). But I still think that the two should be as consistent as possible.

Also consider inputs like alpine@digest matching all four of quay.io/libpod/alpine@digest, quay.io/libpod/alpine:latest, example.com/mirror/alpine@digest, example.com/mirror/alpine@latest. I don’t know whether the output should contain tags but I think it definitely should contain both host names, because that happens for tags in the 3) case.

IOW, conceptually, I think we should first decide whether the image matches, and then build resolvedNames, considering each entry of NamesReference, individually. And if that does need a digest special case, oh well, so be it.

(I don’t immediately know whether the code should be a “match” loop + ”build names” loop, or whether we should preserve the current 2 matching situations, with a “match a name to input” helper. Maybe the latter.)

Uh… what about *pine@digest? … Let’s pretend I didn’t ask, I think.

Also consider inputs like alpine@digest matching all four of quay.io/libpod/alpine@digest, quay.io/libpod/alpine:latest, example.com/mirror/alpine@digest, example.com/mirror/alpine@latest. I don’t know whether the output should contain tags but I think it definitely should contain both host names, because that happens for tags in the 3) case.

Oh, I hadn't thought of that case at all. I think for the alpine@digest case, filterReferences() should return all names and thus keep the original behaviour. I checked the Podman documentation, and the reference filter accepts the pattern of an image reference <image-name>[:<tag>]. Is it okay to do that way?

“all” as in “not filter at all”, or “the ones that match alpine”?

I don’t know what are the constraints here. Podman documentation is definitely relevant, but also Docker’s API seems to be.

And Docker code, from a very quick check, seems to possibly match the input against repo@digest values. (If a reference.Named value matches a "reference=" filter, it is assigned to either RepoDigests or to RepoTags in the output. That’s for the traditional implementation; the containerd one is doing something else with RepoDigests, I can’t investigate the details now.) I’d also need to investigate further whether this always includes any exiting digest of an image, or only digests that were used to explicitly pull repo@digest.

“all” as in “not filter at all”, or “the ones that match alpine”?

Not filtered at all.

libimage/filters.go

libimage/image.go

Honny1 · 2025-04-14T14:55:19Z

PTAL @mtrmac

Luap99

LGTM

libimage/filters.go

mtrmac · 2025-04-16T18:51:02Z

libimage/filters.go

 	if lookedUp != nil {
 		if lookedUp.ID() == img.ID() {
-			return true, nil
+			resolvedName, _, _ := strings.Cut(resolvedName, "@")


I still have no idea what this is doing. AFAICS LookupImage can easily be a (precise) repo:tag or repo@digest match. And the range refs loop below always collects the precise ref.String() values that match. Why is this ignoring the digest part (if that is what it is) and returning all tags (if that is what it is)?

And, even apart from the again, HasPrefix just looks implausible. Why should example.com/a match example.com/ab?

Honny1 · 2025-04-17T13:11:20Z

@mtrmac PTAL, I have implemented your feedback.

libimage/runtime.go

libimage/filters.go

mtrmac · 2025-05-16T16:38:39Z

And it seems, after your arguments, that Podman's behavior is more of an undocumented feature than a bug.

A lot of the last-resort LookupImage heuristics is undocumented… I might lean towards calling them bugs, but that doesn’t matter that much.

Docker only accepts this <image-name>[:<tag>] pattern. Wildcards do not work, and Docker lists an empty list of images. And it looks like Docker is trying to match the full image name.

I see the examples as accepting wildcards. But Docker does not have the concept of short name search, short names always refer to docker.io, and the matching is against the short version of the name. See how *test matches alpine:latest, where the matching input is the short name "alpine:latest", not the fully-qualified equivalent docker.io/library/alpine:latest.

In this respect, we must always differ somehow. And we have the case of matching against repo without the host name … I can’t tell whether that is an intentional idea or a buggy hack to try to better support similarity to short name search.

Containerd uses its own filter syntax, which I am unfortunately unable to find in the documentation. So I was not able to compare the results even with Containerd.

(I meant Docker with a containerd backend — there are two fairly different underlying implementations of images in the Docker daemon. https://docs.docker.com/engine/storage/containerd/ is the UI, I think.)

So, fixing this is not a good idea, and at the very least, this behavior should be documented and closed PRs. WDYT? @mtrmac

I think the current code is clearly problematic / inconsistent. So far I don’t think we have arrived at a consistent alternative (notably the desire of containers/podman#25725 to, essentially, work on image names, and not on deduplicated images, conflicts with the reference!= filter operation on deduplicated images, not on names).

I think it’s clear enough we should not alter the list of image IDs printed by podman images (in typical cases?) during the lifetime of Podman 5, that would be an incompatible change. And I think it’s very reasonable to say (although perhaps not 100% clear) that we should not alter the list of image names printed by podman images, in typical cases, during Podman 5, either.

That would mean giving up for Podman 5… and either documenting things, or… just… saying “it’s complicated, best to use precise image names, we are not making any promises for substrings and wildcards”.

And maybe trying for Podman 6, although I don’t know about prioritizing this. @Luap99

mtrmac · 2025-05-16T16:40:50Z

Podman prints the same list of all image names for all imputes that were given to Docker.

… Are you saying that Docker has the same behavior that containers/podman#25725 is complaining about? I was assuming that the 5 tags in the samples are point at the same deduplicate image, in which case the test cases do show references being filtered. Is the test using 5 different images?

Honny1 · 2025-05-16T16:51:27Z

Podman prints the same list of all image names for all imputes that were given to Docker.

… Are you saying that Docker has the same behavior that containers/podman#25725 is complaining about? I was assuming that the 5 tags in the samples are point at the same deduplicate image, in which case the test cases do show references being filtered. Is the test using 5 different images?

No, I meant to say that I also tested Podman with the same inputs I used for the Docker examples. And the result was always the same list of 5 image tags. The test does not use 5 different images.

Honny1 · 2025-05-16T17:23:45Z

That would mean giving up for Podman 5… and either documenting things, or… just… saying “it’s complicated, best to use precise image names, we are not making any promises for substrings and wildcards”.

I used a fully qualified image name, and Podman listed all the image tags/names.

$ podman images docker.io/library/alpine:latest
REPOSITORY                TAG         IMAGE ID      CREATED       SIZE
host1.example.com/foo     bar         8d591b0b7dea  3 months ago  8.47 MB
host2.example.com/foo     bar         8d591b0b7dea  3 months ago  8.47 MB
host1.example.com/test    foo         8d591b0b7dea  3 months ago  8.47 MB
host1.example.com/test    bar         8d591b0b7dea  3 months ago  8.47 MB
docker.io/library/alpine  latest      8d591b0b7dea  3 months ago  8.47 MB

I think we could say that if a fully qualified image name is used, the output should only show the corresponding names. And in the case of short names, wildcards, and digests. Podman should display all the names of the image.

If we agree on this behavior, I'm not sure how to check if the user input is a fully qualified image name, and if we should wait for Podman 6 to make this behavior change.

mtrmac · 2025-05-16T17:31:12Z

I think we could say that if a fully qualified image name is used, the output should only show the corresponding names. And in the case of short names, wildcards, and digests. Podman should display all the names of the image.

That could work. I’d include fully-qualified names with digests in the first case: “if the input 100% unambiguously specifes precisely one image, it is a query; if it is any more ambiguous, it is a search”.

But users with such a precise input can use podman image inspect, so… this clear case might not be that important for users of podman images.

If we agree on this behavior, I'm not sure how to check if the user input is a fully qualified image name

reference.ParseNamed (instead of the usual reference.ParseNormalizedNamed), or the equivalent check that ParseNormalizedNamed().String() matches the original input, would reject short names, and fail on wildcards.

Then !reference.IsNameOnly() to require a tag and/or digest to be present.

, and if we should wait for Podman 6 to make this behavior change.

That’s still a topic — I’m leaning towards Podman 6 but that’s a very weak opinion.

Honny1 · 2025-05-19T11:37:31Z

I've updated the behavior so that the output only shows the corresponding names if a fully qualified image name is used, including name@digest. And wildcards and short names output all image names.

I'm not sure about this case:
Current status:

$ podman images --digests
REPOSITORY                TAG         DIGEST                                                                   IMAGE ID      CREATED       SIZE
docker.io/library/image   v2          sha256:a8560b36e8b8210634f77d9f7f9efd7ffa463e380b75e2e74aff4511df3ef88c  8d591b0b7dea  3 months ago  8.47 MB
docker.io/library/image   v1          sha256:a8560b36e8b8210634f77d9f7f9efd7ffa463e380b75e2e74aff4511df3ef88c  8d591b0b7dea  3 months ago  8.47 MB
docker.io/library/alpine  latest      sha256:a8560b36e8b8210634f77d9f7f9efd7ffa463e380b75e2e74aff4511df3ef88c  8d591b0b7dea  3 months ago  8.47 MB

What should be the correct output of this command?

$ podman images docker.io/library/image@sha256:a8560b36e8b8210634f77d9f7f9efd7ffa463e380b75e2e74aff4511df3ef88c

Displays all names.

REPOSITORY                TAG         IMAGE ID      CREATED       SIZE
docker.io/library/image   v2          8d591b0b7dea  3 months ago  8.47 MB
docker.io/library/image   v1          8d591b0b7dea  3 months ago  8.47 MB
docker.io/library/alpine  latest      8d591b0b7dea  3 months ago  8.47 MB

Displays all tags that match the image name. (PR implementation)

REPOSITORY                TAG         IMAGE ID      CREATED       SIZE
docker.io/library/image   v2          8d591b0b7dea  3 months ago  8.47 MB
docker.io/library/image   v1          8d591b0b7dea  3 months ago  8.47 MB

WDYT? @mtrmac

mtrmac

What should be the correct output [repo@digest]?

Yeah… I think the PR’s implementation is reasonable. In effect, we have 3 separate behaviors:

fqRepo:tag = print only that repo:tag
fqRepo@digest = print all matching tags (if any), and possibly the digest if it is in image.Names
anything else (short names, wildcards) = no filtering.

and we’d have to document them as such (with a caveat that this might change in Podman 6??).

@Luap99 / @mheon WDYT?

Procedurally, if we go one of these routes and don’t give up entirely: could you, split the filters_test.go refactor that shows matched names as a separate first commit, please? It’s not just cleaner as a matter of formal tidiness, it would mean that a second commit, actually changing behavior and updating the tests, would be explicit and self-documenting about the changes.

libimage/filters.go

libimage/runtime.go

Luap99

LGTM, I think it is fine to move ahead with this without a 6.0.
There might be a risk here that someone expects multiple names but I don't see why.
Overall since this changes the behaviour it might be best to to make a test PR against podman with this change and make sure all tests still pass first before merging. If it turns out this breaks a lot of our tests than maybe it is not a good change for a minor release

Honny1 · 2025-06-11T12:51:39Z

Test PR: containers/podman#26344

mheon · 2025-06-14T07:25:53Z

Code LGTM

mtrmac

The code, implementing a single-image filter, LGTM — but, still, #2413 (comment) .

Alternatives:

Only do the name filtering for the special case of a single filter in wantedReferenceMatches (and empty unwantedReferenceMatches?). It is a special case anyway, so let’s narrow down the case where we are changing behavior.
Treat each reference= separately, and build a set of accepted image names. (I.e. if anything in wantedReferenceMatches is a wildcard or similarly generic, include all names; if all wantedReferenceMatches elements are fully-qualified !NameOnly names, include only matches for those names
Something else?

I think it would be useful to think what the end-user documentation of the behavior will be (write a draft?). We can implement any of these behaviors, but documenting them in a way that can be understood seems harder than that.

Personally, I weakly lean towards the first option (with an explanation similar to #2413 (comment) “it is a query” vs. “it is a search”) — that would change the effect of podman images $singleInput, but any users doing something more complex with --filter would not see a change.

BTW I think Podman should eventually get a man page update documenting this.

libimage/filters.go

libimage/runtime.go

Signed-off-by: Jan Rodák <[email protected]>

Honny1 · 2025-06-24T13:57:40Z

@mtrmac I have modified the implementation so that name stripping is only done for the special case of a single reference in wantedReferenceMatches and empty unwantedReferenceMatches, i.e., when it is a query.

mtrmac

LGTM. Thanks!

libimage/filters.go

The filtered image contains all Names of image. This causes the podman to list images that are not expected. Fixes: containers/podman#25725 Fixes: https://issues.redhat.com/browse/RUN-2726 Signed-off-by: Jan Rodák <[email protected]>

Honny1 · 2025-06-25T09:56:26Z

I fixed the non-blocking nits. Can we merge this PR? @mtrmac

mtrmac · 2025-06-25T14:40:36Z

@Luap99 or @mheon , PTANL.

Luap99

/lgtm

openshift-ci · 2025-06-26T10:30:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Honny1, Luap99

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [Luap99]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Honny1 mentioned this pull request Apr 7, 2025

Fix duplicate results in podman images <image> containers/podman#25783

Closed

mtrmac reviewed Apr 7, 2025

View reviewed changes

Honny1 force-pushed the list-images branch 4 times, most recently from 3e00335 to 8dd85f6 Compare April 8, 2025 17:37

mtrmac reviewed Apr 8, 2025

View reviewed changes

Honny1 force-pushed the list-images branch 7 times, most recently from f644901 to c1d4722 Compare April 9, 2025 21:18

Honny1 requested a review from mtrmac April 9, 2025 21:31

Honny1 requested a review from Luap99 April 16, 2025 08:21

Luap99 approved these changes Apr 16, 2025

View reviewed changes

openshift-ci bot added the approved label Apr 16, 2025

mtrmac reviewed Apr 16, 2025

View reviewed changes

Honny1 force-pushed the list-images branch 3 times, most recently from 3d1f37e to 289f353 Compare April 17, 2025 13:08

Honny1 requested a review from mtrmac April 17, 2025 13:11

mtrmac reviewed Apr 29, 2025

View reviewed changes

Honny1 force-pushed the list-images branch 3 times, most recently from 73e55be to ba9f9e1 Compare April 30, 2025 17:18

mtrmac reviewed Apr 30, 2025

View reviewed changes

libimage/filters.go Outdated Show resolved Hide resolved

Honny1 force-pushed the list-images branch from 5857f57 to 0fa3826 Compare May 19, 2025 11:37

Honny1 force-pushed the list-images branch from 0fa3826 to 96058ab Compare May 19, 2025 11:41

mtrmac reviewed May 20, 2025

View reviewed changes

libimage/filters.go Outdated Show resolved Hide resolved

libimage/filters.go Outdated Show resolved Hide resolved

libimage/filters.go Outdated Show resolved Hide resolved

libimage/filters.go Outdated Show resolved Hide resolved

libimage/runtime.go Outdated Show resolved Hide resolved

Honny1 force-pushed the list-images branch from 96058ab to e18751f Compare June 9, 2025 09:54

Luap99 reviewed Jun 10, 2025

View reviewed changes

Honny1 force-pushed the list-images branch from e18751f to 28fd0a8 Compare June 11, 2025 13:37

mtrmac reviewed Jun 20, 2025

View reviewed changes

libimage/filters.go Outdated Show resolved Hide resolved

libimage/filters.go Outdated Show resolved Hide resolved

libimage/runtime.go Show resolved Hide resolved

Improve filter tests by verifying image names

8dc0e3b

Signed-off-by: Jan Rodák <[email protected]>

Honny1 force-pushed the list-images branch from 28fd0a8 to 93e56cd Compare June 24, 2025 13:32

Honny1 requested a review from mtrmac June 24, 2025 13:57

mtrmac reviewed Jun 24, 2025

View reviewed changes

libimage/filters.go Outdated Show resolved Hide resolved

libimage/filters.go Outdated Show resolved Hide resolved

Honny1 force-pushed the list-images branch from 93e56cd to ec24373 Compare June 25, 2025 08:10

Honny1 requested a review from mtrmac June 25, 2025 09:57

Luap99 approved these changes Jun 26, 2025

View reviewed changes

openshift-ci bot assigned Luap99 Jun 26, 2025

openshift-ci bot added the lgtm label Jun 26, 2025

openshift-merge-bot bot merged commit 3952c67 into containers:main Jun 26, 2025
14 checks passed

Fix unexpected results when filtering images #2413

Fix unexpected results when filtering images #2413

Uh oh!

Conversation

Honny1 commented Apr 7, 2025

Uh oh!

mtrmac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Honny1 commented Apr 14, 2025

Uh oh!

Luap99 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Honny1 commented Apr 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mtrmac commented May 16, 2025

Uh oh!

mtrmac commented May 16, 2025

Uh oh!

Honny1 commented May 16, 2025

Uh oh!

Honny1 commented May 16, 2025

Uh oh!

mtrmac commented May 16, 2025

Uh oh!

Honny1 commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtrmac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Honny1 commented May 19, 2025 •

edited

Loading

Honny1 commented Jun 25, 2025 •

edited

Loading