libimage: harden lookup by digest #1505
Conversation
Signed-off-by: Valentin Rothberg <[email protected]>
|
OK, this is clearly breaking the |
vrothberg
force-pushed
the
bye-bye-digest-dance
branch
from
e521403 to
adf3807
Compare
Signed-off-by: Valentin Rothberg <[email protected]>
vrothberg
force-pushed
the
bye-bye-digest-dance
branch
from
adf3807 to
d82579a
Compare
Signed-off-by: Valentin Rothberg <[email protected]>
vrothberg
force-pushed
the
bye-bye-digest-dance
branch
3 times, most recently
from
dcefac6 to
c8c9030
Compare
Signed-off-by: Valentin Rothberg <[email protected]>
|
Two test cases in Podman broke since they relied on the rather loose digest matching, see https://github.com/containers/podman/pull/18888/files#diff-748a695f7f517f8721b6b3b248f5bb31a5257caaba1f8275dcd43dbb51bb7ea5. |
vrothberg
force-pushed
the
bye-bye-digest-dance
branch
from
c8c9030 to
dabd8aa
Compare
vrothberg
changed the title
Signed-off-by: Valentin Rothberg <[email protected]>
|
Both PRs are green |
The last push I see is commit 403bda7 from Jun 27 10am, before my review around Jun 27 10pm. And review comments like #1505 (comment) (“an entirely new set of tests for |
vrothberg
force-pushed
the
bye-bye-digest-dance
branch
from
403bda7 to
111402b
Compare
When looking up an image by digest, make sure that the entire repository of the specified value is considered. Previously, both the repository and the tag have been ignored and we looked for _some_ image with a matching digest. As outlined in containers#1248, Docker stopped ignoring the repository with version v20.10.20 (Oct '22) which is a compelling reason to do the same. To be clear, previously `something@digest` would look for any image with `digest` while `something` is entirely ignored. With this change, both `something` and `digest` must match the image. This change breaks two e2e tests in Podman CI which relied on the previous behavior. There is a risk of breaking users but there is a strong security argument to perform this change: if the repository does not match the (previously) returned issue, there is a fair chance of a user error. Fixes: containers#1248 Signed-off-by: Valentin Rothberg <[email protected]>
vrothberg
force-pushed
the
bye-bye-digest-dance
branch
from
111402b to
ffc7d8e
Compare
|
@mtrmac it's all up now. PRs pass without further changes: |
|
@mtrmac OK with merging? |
|
Leaving breadcrumbs here as well: once this PR gets in, we need to backport it to the v0.55 branch, then cut a new .1 release and vendor that into the v4.6 branch in Podman and backport containers/podman#19092. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, 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 |
|
Shouldn't we wait for a final ack from @mtrmac? |
|
I am confident the changes are OK as is - for now. We can wait one more day or so but this PR is blocking backports at the moment. |
|
Friendly ping, @mtrmac. Apologies for being pushy; I know there's much on your table at the moment. |
|
/lgtm |
|
(I have filed #1559 implementing some of the cleanups, so that the time I spent thinking about that pays off.) |
|
/hold cancel |
Beautify lookup after #1505
When looking up an image by digest, make sure that the entire repository
of the specified value is considered. Previously, both the repository
and the tag have been ignored and we looked for some image with a
matching digest.
As outlined in #1248, Docker stopped ignoring the repository with
version v20.10.20 (Oct '22) which is a compelling reason to do the same.
To be clear, previously
something@digestwould look for any image withdigestwhilesomethingis entirely ignored. With this change, bothsomethinganddigestmust match the image.This change breaks two e2e tests in Podman CI which relied on the
previous behavior. There is a risk of breaking users but there is a
strong security argument to perform this change: if the repository does
not match the (previously) returned image, there is a fair chance of a
user error.
Fixes: #1248
Signed-off-by: Valentin Rothberg [email protected]