Nice, thanks @aaronlehmann!

LGTM

LGTM

Thanks Nishant. If you want to use this in github.com/moby/moby/pull/28173, make sure to import the github.com/docker/distribution/reference package instead of github.com/docker/docker/reference. Both packages implement these functions - this is unfortunate, and there's work in progress to remove the duplication, but it hasn't gone in yet.

For now I think it's okay to manipulate the reference string manually, and we can switch to this corrected WithDigest function as a followup action item.

Current coverage is 51.41% (diff: 100%)

Merging #2044 into master will decrease coverage by 10.08%

@@             master      #2044   diff @@
==========================================
  Files           126        126           
  Lines         11263      11275     +12   
  Methods           0          0           
  Messages          0          0           
  Branches          0          0           
==========================================
- Hits           6927       5797   -1130   
- Misses         3449       4732   +1283   
+ Partials        887        746    -141   

Powered by Codecov. Last update a2611c7...bc1e7aa

@aaronlehmann Note that docker will not save this tag. If I remember this correctly then this was made so that busybox:v1@sha123 would match busybox@sha123. I haven't tested if this change modifies this behavior or not.

@tonistiigi: That's probably okay. We want to display the tag in service inspect but it doesn't need to be saved in the reference store.

Will we need to make any updates where comparisons are done? I think the correct thing for the caller to do if both tag/digest are not wanted is to strip the reference down to just a name before calling this function. Not sure we currently have a helper for this.

I'm not sure. The reference store seems to use ref.String(). Unless something else is removing the tag before it's saved, we would have to make a change like that somewhere to preserve the behavior.

But is it bad if the image is now stored as busybox:v1@sha123? It might be good to preserve that information actually. The worst case is that we have to re-pull if someone specifies the image without the tag name, but we'd already have all the layers.

Either way, we need to test this stuff when we vendor.

But is it bad if the image is now stored as busybox:v1@sha123?

It's not only matching(although I find current behavior useful). Api exports RepoTags and RepoDigests for an image, not a single list of references. There is fair bit of magic(from @dmcgowan) to show them side-by-side in the cli but I don't think there is any logic for keeping the tag-digest pairs.

I think this is failing the trust tests for me, the tests are sensitive but looking for "repo@digest" in FROM line but are now getting "repo:tag@digest"

https://jenkins.dockerproject.org/job/Docker-PRs/35163/console

Created #2053 to address this

@dmcgowan when can we vendor the distribution repo into Docker next?

@nishanttotla I will be vendoring master with moby/moby#28235 once CI completes on that linked issue. I should have all the vendoring issues fixed in that PR.

@dmcgowan thanks!