Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip checking if the manifest difest matches for docker-daemon source. Fixes #1049 #1050

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

airadier
Copy link

As described in #1049, the manifest digest specified in the repo@sha256:digest won't match the digest calculated from the locally generated manifest when using docker-daemon source.

This fix skips the verification for docker-daemon source.

@airadier airadier force-pushed the dont-check-manifest-digest-match-for-docker-daemon branch 3 times, most recently from 3db87e4 to 5273b28 Compare September 22, 2020 12:08
Signed-off-by: Alvaro Iradier <airadier@gmail.com>
@airadier airadier force-pushed the dont-check-manifest-digest-match-for-docker-daemon branch from 5273b28 to 926f91e Compare September 22, 2020 12:15
Copy link
Collaborator

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.

We shouldn’t have exceptions like this in the code, it will be very difficult to trace back unexpected behavior to this condition.

My first guess is that .DockerReference should just not return a value if it can’t return a working value, but I’ll need to take a closer look at how the method is used in callers of c/image (sadly it’s not very well-defined what the value is intended to be used for).

@bduffany
Copy link

bduffany commented Feb 10, 2022

@mtrmac I am running into this issue as well (with skopeo copy) and am interested in getting this fixed. Is there any other path forward? Maybe instead of adding a check like "skip if transport is docker-daemon," maybe something more explicit like an option/flag that says, "this digest is uncompressed and we don't expect it to match the compressed digest; the check should be skipped" ?

For context, I am using skopeo to unpack docker images in OCI format, but need to use the docker daemon to pull the image, so I am using docker pull to pull the image then exporting via skopeo copy docker-daemon:image oci:/tmp/unpacked_image. The docker:// transport is also an option but we want all image pull operations to be done by the docker daemon for de-duping purposes.

@mtrmac
Copy link
Collaborator

mtrmac commented Feb 10, 2022

Changing the DockerReference method of the docker-daemon: transport is the available way to represent the situation in the current API.

Alternatively, yes, a transport declaring that the digests from DockerReference are not to be validated is an option.

Either way that affects semantics in caller-visible ways, so we need to at least audit existing users in the GitHub.com/containers ecosystem.

@universam1
Copy link

example issue: podman pull ghcr.io/fluxcd/flagger:1.22.1

Any workaround possible?

@mtrmac
Copy link
Collaborator

mtrmac commented Aug 16, 2022

@universam1 That seems completely unrelated to the docker-daemon problem in the original PR. Please file a separate report, against Podman, and include actual/expected results and logs.

@mtrmac mtrmac added the kind/bug A defect in an existing functionality (or a PR fixing it) label Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A defect in an existing functionality (or a PR fixing it)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants