libimage: fix reference filters #1503
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vrothberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Make sure that reference filters properly work on digests as well. To keep things simple, try to lookup an image for the user-specified value and compare IDs. This will implicitly fix #containers/podman/issues/18445 and probably more (unknown) issues. Fixes: #containers/podman/issues/18445 Signed-off-by: Valentin Rothberg <[email protected]>
vrothberg
force-pushed
the
fix-podman-18445
branch
from
3e25180 to
a146adf
Compare
[NO NEW TESTS NEEDED] Signed-off-by: Valentin Rothberg <[email protected]>
|
Let's wait for containers/podman#18884 to turn green before merging. I want to be extra sure that the changes don't have any unintended side effect. |
|
LGTM |
|
/lgtm |
libimage: fix reference filters Signed-off-by: Valentin Rothberg <[email protected]>
libimage: fix reference filters Signed-off-by: Valentin Rothberg <[email protected]>
libimage: fix reference filters Signed-off-by: Valentin Rothberg <[email protected]>
libimage: fix reference filters Signed-off-by: Valentin Rothberg <[email protected]>
libimage: fix reference filters Signed-off-by: Valentin Rothberg <[email protected]>
| return func(img *Image) (bool, error) { | ||
| if lookedUp != nil { | ||
| if lookedUp.ID() == img.ID() { |
There was a problem hiding this comment.
This seems imprecise to me.
The code below seems fairly clear that fedora:38 is intended to match example.com/any/namespaces/fedora:38; and even that wildcards work, so fe*ra:38 is intended to match.
Is that correct?
If so, fe*ra@sha256:$digest should also match — but LookupImage can (I hope?) never resolve fe*ra.
IOW, if the underlying problem is that img.NamesReferences() does not contain digests, I think that needs to be solved directly here, not by relying on the LookupImage crutch (and on the lookupImageInDigestsAndRepoTags code path for matching short names) — maybe by combining img.NamesReference() with img.RepoDigests() when building candidates.
Now, RepoDigests is imprecise and relying on it would also introduce a weak variant of #1248 (where an image that exists on registry as repo1@digest1 and repo2@digest2 can be referenced using repo1@digest2, although that exists on no registry — but it’s a weaker variant because otherRepo@digest1 does not match). Ideally we should fix RepoDigests, but that’s a whole other set of PRs again.
There was a problem hiding this comment.
Even more extreme, wildcards inside the digest? fedora@sha*:a?b*c ?
Make sure that reference filters properly work on digests as well. To keep things simple, try to lookup an image for the user-specified value and compare IDs. This will implicitly fix #containers/podman/issues/18445 and probably more (unknown) issues.
Fixes: #containers/podman/issues/18445