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

Don't require a Docker-Content-Digest header when deleting images #1497

Merged
merged 1 commit into from
Apr 25, 2022

Conversation

mtrmac
Copy link
Collaborator

@mtrmac mtrmac commented Mar 17, 2022

Per #1010 , it seems the header is not populated by AWS ECR.

We were actually computing the digest from the manifest bytes already, so this is both more robust and simpler.

WIP: Untested manually (there is a system test for skopeo delete that will exercise this), and we should get a confirmation that this fixes #1010 .

@mtrmac mtrmac force-pushed the delete-compute-digest branch 2 times, most recently from 7d304fa to c1520e2 Compare March 25, 2022 18:15
@mtrmac
Copy link
Collaborator Author

mtrmac commented Mar 25, 2022

#1010 (comment) confirms this PR works as intended. Sadly it doesn’t ultimately make a difference; still, I think it’s worthwhile to make this code path more similar to the much-more-frequently-used ones.

@mtrmac mtrmac marked this pull request as ready for review March 25, 2022 18:17
@mtrmac mtrmac changed the title WIP: Don't require a Docker-Content-Digest header when deleting images Don't require a Docker-Content-Digest header when deleting images Mar 31, 2022
@mtrmac mtrmac force-pushed the delete-compute-digest branch from c1520e2 to 377c319 Compare April 14, 2022 15:50
Per containers#1010 , it seems
the header is not populated by AWS ECR.

We were actually computing the digest from the manifest bytes
already, so this is both more robust and simpler.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@mtrmac mtrmac force-pushed the delete-compute-digest branch from 377c319 to 15e689d Compare April 22, 2022 15:47
@mtrmac mtrmac requested a review from vrothberg April 22, 2022 15:47
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

Nice catch!
LGTM

@vrothberg vrothberg merged commit be08568 into containers:main Apr 25, 2022
@mtrmac mtrmac deleted the delete-compute-digest branch April 25, 2022 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants