Compute manifest metadata when not provided#3245
Conversation
|
I think the digest calculation might depend on the content type of the manifest. Probably only want to do special processing on the signed content types rather than always. I think the code I have now works just for |
4abc424 to
4317ad0
Compare
|
This change makes it so that
|
|
Will probably need to wait for #3249 before this will be ready and passing CI |
Codecov Report
@@ Coverage Diff @@
## master #3245 +/- ##
=========================================
- Coverage 44.63% 44.54% -0.1%
=========================================
Files 113 113
Lines 12164 12198 +34
=========================================
+ Hits 5430 5434 +4
- Misses 5899 5930 +31
+ Partials 835 834 -1
Continue to review full report at Codecov.
|
remotes/docker/resolver.go
Outdated
There was a problem hiding this comment.
V1 manifest should not be the default, rather check for the specific anomalous conditions and add a comment why that condition is there. The normal format would be stating the registry which this block is correcting for and the date at which it was last tested to still have that issue. So in this case, just check for the text/plain.
Also this block can be moved higher so that the digest calculation can be done inside the other block you added, rather than splitting the logic across 2 blocks.
There was a problem hiding this comment.
Alright made that change.
As for the two block thing I was originally just trying to avoid duplicating the media type parsing code. I've just moved it to a separate function now.
remotes/docker/resolver.go
Outdated
There was a problem hiding this comment.
libtrust doesn't meet our standards for vendoring as it is deprecated and the library is not split apart to just import packages we need.
Look at the github.com/containerd/containerd/remotes/docker/schema1 package, that has pulled out just the logic which is needed. It is ok to create a function there which just takes a schema1 blob and returns a digest for it.
There was a problem hiding this comment.
Cool thanks, added a utility method there that does what I need.
9113232 to
a4a0145
Compare
This closes containerd#3238 Signed-off-by: msg555 <msg555@gmail.com>
|
Seems Windows CI is hitting #2835 a lot these days; restarted |
|
@msg555 as this was your first contribution to containerd, can you please provide your real name here for use in our release notes. Thanks! |
|
Mark Gordon |
|
@dmcgowan Is it possible to include this to 1.2.x branch? Recently released GitHub Package Registry doesn't provide Docker-Content-Digest, and stable 1.2.x cannot be used with it. |
|
Opened #3591 for consideration |
Adds a fallback to manually compute the manifest digest when it is not presented as the Docker-Content-Digest header by the registry server. This is to address #3238.