- 
                Notifications
    You must be signed in to change notification settings 
- Fork 176
Store repo digest as app image is pushed #770
base: master
Are you sure you want to change the base?
Conversation
f52fac5    to
    4050fd9      
    Compare
  
    | return err | ||
| } | ||
|  | ||
| srcRef.RepoDigest = "" | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand that correctly, tagging a pulled app image means we drop its repo digest? So it won't be displayed in the docker app image ls --digest column?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might be wrong, but IIUC repo digest won't be the same if you push under a distinct tag. So keeing this "pulled digest" for a tagged image would be wrong
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be the same repo digest, as tagging is just pushing a new descriptor, pointing to the same manifest/manifest list, but the repo digest should be the digest of the pointed object, not the pointer. cc @eunomie / @thaJeztah any idea?
also fix app image list --digests so it doesn't show "app build ID" as digest Signed-off-by: Nicolas De Loof <[email protected]>
Signed-off-by: Nicolas De Loof <[email protected]>
| Codecov Report
 @@            Coverage Diff            @@
##           master     #770     +/-   ##
=========================================
- Coverage   71.32%   70.43%   -0.9%     
=========================================
  Files          67       67             
  Lines        3983     3768    -215     
=========================================
- Hits         2841     2654    -187     
+ Misses        792      764     -28     
  Partials      350      350
 Continue to review full report at Codecov. 
 | 
| return errors.Wrapf(err, "could not push %q", ref) | ||
| } | ||
|  | ||
| // we can't just re-use bndl var here, as fixup did rewrite the bundle | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As stated in the specs, https://github.com/cnabio/cnab-spec/blob/master/101-bundle-json.md#invocation-images
During bundle development, it may be ideal to omit the contentDigest field and/or skip validation. Once a bundle is ready to be transmitted as a thick or thin bundle, it must have a contentDigest field.
the contentDigest can be omitted, but needs to be filled before pushing. I think that's exactly what we do here, that's why the bundle is mutated by the push.
- What I did
Added repository digest as a stored info on app images (aka "bundles")
- How I did it
added a
digesttext file in bundlestore- How to verify it
docker app image pull foo@sha256:123
docker app image push myapp
- Description for the changelog
Store App Image repository Digest
- A picture of a cute animal (not mandatory)
