I love this, though, when there's a fallback to V1 because V2 was unauthorized, Docker showed "not found" (which is coming from the V1 404 when getting the image). With this change you'll now get just "unauthorized".

If I wasn't clear in the previous comment you can read what I meant at http://www.projectatomic.io/blog/2016/10/how-the-container-registries-prevent-information-leakage/

/cc @stevvooe @aaronlehmann

IIRC we still don't get the correct error when pulling a non-existent image with the V2 protocol from Docker Hub. It returns a 403 or 401 instead of a 404, so the error message is about authentication or authorization rather than the image not existing. This is why TestPullNonExistingImage fails here.

cc @dmcgowan

We don't get 404 even with private registries.

@runcom @aaronlehmann I am working on a change to address this. The registry is returning a 401 when there is not sufficient scope to do the operation on the registry (such as private registry, push to unauthorized registry). This is getting pushed back to the client as needing to login since the api/server code is looking for a string containing "unauthorized" to propagate the 401 to the client, causing login.

Design +1, going to add to 1.13

LGTM design

Distribution change is at distribution/distribution#2047, once that is vendored in and updated the tests should be able to pass

@dmcgowan distribution/distribution#2047 was merged :)

Trying to figure out if the change is an acceptable UX with our current handling after #28235. The error returned for non-existent "official" repositories goes from...

$ docker pull doesnotexist
Using default tag: latest
Pulling repository docker.io/library/doesnotexist
Error: image library/doesnotexist:latest not found

to...

$ docker pull doesnotexist
Using default tag: latest
Error response from daemon: denied: requested access to the resource is denied

This error matches what is returned by the registry but is confusing to the user. I think we need to convert the messaging to be more user friendly but I am wary of saying "not found".

I think we should say exactly what it is; "The repository does not exist, or the user authenticated to this registry does not have permission to access the repository"

(that, or modified a bit)

Do we always return the same message / error code?

@thaJeztah I think you are right, working on updating the messaging. We are always getting the same error code but am able to distinguish between error with repository (denied) or tag not existing (manifest not found). That does allow providing a slightly more helpful error to the user.

Rebased, tested failing tests locally and they pass. Fingers crossed

LGTM

LGTM

@aaronlehmann any comments?

LGTM

@thaJeztah I guess we should update some doc ? 👼

@vdemeester hm not sure if anything is needed other than changelog; given that this only changes for docker hub, and not for local registries, perhaps no docs changes are needed

The only docs that would need to be changed would be related to the v1 mirrors. Using v1 mirrors was still possible using the mirror flag, however v2 can also use that flag so there is no interface change but if anyone is using they will need to know to update their mirror to v2. fyi @SvenDowideit 😉

Just discussed with @dmcgowan, let's merge this, and do a follow up for the docs @nwt can you look into that?