Respect digests.Options in sif:, tarball:, and for configs in docker-daemon: and docker-archive: #531
Conversation
|
Not to self: Absolutely untested at this point. |
|
✅ A new PR has been created in buildah to vendor these changes: containers/buildah#6574 |
|
Packit jobs failed. @containers/packit-build please check. |
1 similar comment
|
Packit jobs failed. @containers/packit-build please check. |
mtrmac
force-pushed
the
choose-digest-tarball
branch
from
05a8d27 to
071cb96
Compare
|
Now marked ready for review (but I still didn’t try this in practice). |
mtrmac
force-pushed
the
choose-digest-tarball
branch
from
071cb96 to
4953ceb
Compare
|
Manually-smoke-tested:
|
| // don’t matter. | ||
|
|
||
| if mustUse := options.Digests.MustUseSet(); mustUse != "" && mustUse != digest.Canonical { | ||
| logrus.Debugf("%q does not implement digest choices for image sources; request to use %s is ignored", |
There was a problem hiding this comment.
I'm thinking if this should be a warning or not. It seems like request is being ignored which sounds like something we should clearly see in the logs.
There was a problem hiding this comment.
I think this is going to end up visible in podman manifest and buildah (and eventually undesirable), but we’re pretty far away from that point, and using a warning here means we won’t forget to consider that situation. Updated.
|
I've read the PR and also the surrounding code and it all makes sense to me and I don't see anything obviously wrong. But I'm no an expert here. So giving cautious +1. |
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Currently it is only called in oci/archive and openshift to forward to another transport; real users will be added later. Should not change behavior (yet). Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This will make it a bit easier to use NewImageSourceOptions for transports that might only implement the public types.ImageReference. This commit does not (yet) add any callers, so this should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
…r-daemon: Note that we DO NOT use options.Digest for the layer IDs; that would require extra CPU cost to digest files more than once. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
mtrmac
force-pushed
the
choose-digest-tarball
branch
from
4953ceb to
7347041
Compare
|
✅ A new PR has been created in buildah to vendor these changes: containers/buildah#6643 |
3 participants
DO NOT MERGE: This is on top of #530 .Add
private.ImageReference.NewImageSourceWithOptions, and use it to passdigests.Optionsto transports that might need to compute digests.Fully implement that in
sif:andtarball:.For
docker-deamon:anddocker-archive:, usedigests.Optionsfor the config digest, but do not use it for layers, to avoid an extra cost. See the detailed added comment for rationale. This also does not modify the destination parts of these transports at all.Also 2 sort-of-unrelated bug fixes.
See individual commit messages for details.
Cc: @lsm5 (low priority)