-
Notifications
You must be signed in to change notification settings - Fork 546
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
Fix crane digest for v1 and platform option #907
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
This allows to use crane.Digest on images with a v1 schema
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it! |
Codecov Report
@@ Coverage Diff @@
## master #907 +/- ##
==========================================
+ Coverage 72.54% 72.57% +0.02%
==========================================
Files 108 108
Lines 4673 4689 +16
==========================================
+ Hits 3390 3403 +13
- Misses 773 776 +3
Partials 510 510
Continue to review full report at Codecov.
|
How often do you release new versions (tags)? |
We tend to release as-needed whenever there are significant changes to make available. cc @jonjohnsonjr to speak on whether/how we want |
Yeah, roughly as needed. Usually when I need to re-import this package into google or some large downstream dependent requests a fix be included in a release. We could have more discipline about this (cc @hasheddan).
This doesn't cover every base, but it's definitely an improvement, so I'm fine with it. @dvob consider situations such as this: docker-library/golang#269 (comment)
The linux/arm64/v8 image is a schema 1 image, so |
Or do you see an easy way to fix it for these cases as well? |
We'd need to move this logic somewhere else: go-containerregistry/pkg/v1/remote/index.go Lines 193 to 237 in f97c411
(probably as a And use that inside go-containerregistry/pkg/v1/remote/index.go Lines 143 to 157 in f97c411
The TODO is fine for now, I think. There is some discussion around platform matching in #892 that we can tack this work onto. |
This allows to use crane.Digest on images with a v1 schema.
Fixes #906