Figure out docker-daemon: tagging canonicalization rules about image HOT 15 CLOSED

Note that AFAICT our projectatomic/docker and docker/docker behave differently wrt how images are tagged (or retrieved, need to look that up)

from image.

@runcom What do you mean exactly? Examples?

from image.

docker pull busybox gets the image tagged as docker.io/busybox (per docker inspect)

This, for instance, is valid for projectatomic/docker but not docker/docker - upstream does have that reference as just "busybox" (per docker inspect)

docker inspect busybox:latest finds docker.io/busybox:latest
docker inspect docker.io/busybox.latest does not find busybox:latest

These last two, same as above, in docker/docker you just get "busybox:latest" in RepoTags

from image.

This, for instance, is valid for projectatomic/docker but not docker/docker

Oh. Thanks for pointing this out, I would have never thought to look there. Does that mean that we should be vendoring in projectatomic/docker/reference instead of docker/docker/reference? (Should be doable but it is not completely trivial, we would need to check whether that affects reference matching in signatures.)

from image.

Alternatively,

• docker pull busybox gets the image tagged as docker.io/busybox (per docker inspect)
  • docker-daemon:busybox gets it tagged as docker:busybox

This suggests that the different canonicalization projectatomic/docker does might not be handled consistently, i.e. the docker pull path was changed but not the docker load path. (I wish this to be true because if it were, skopeo and atomic wouldn’t need to worry about reference canonicalization at all and we would change the docker load canonicalization.)

from image.

This suggests that the different canonicalization projectatomic/docker does might not be handled consistently, i.e. the docker pull path was changed but not the docker load path. (I wish this to be true because if it were, skopeo and atomic wouldn’t need to worry about reference canonicalization at all and we would change the docker load canonicalization.)

I fear it's like you said, and docker load doesn't get the canonicalization - I'm trying right now though.

from image.

With projectatomic/docker, after switching from docker/docker with busybox already in storage:

$ docker pull busybox
Using default tag: latest
Trying to pull repository docker.io/library/busybox ... 
latest: Pulling from docker.io/library/busybox
Digest: sha256:a59906e33509d14c036c8678d687bd4eec81ed7c4b8ce907b888c607f6a1e0e6
Status: Downloaded newer image for docker.io/busybox:latest

$ docker images | grep busybox
busybox              latest              2b8fd9751c4c        10 weeks ago        1.093 MB
docker.io/busybox    latest              2b8fd9751c4c        10 weeks ago        1.093 MB

$ docker inspect busybox | grep busybox
            "busybox:latest",
            "docker.io/busybox:latest",

on RHELs we can safely assume people are using projectatomic/docker so only the second lines are valid. Though, I feel we'll need to deal with both because we can't expect people to be using projectatomic/docker.

from image.

(Mostly notes to self:)

Yeah, this is the “Add --add-registry and --block-registry options to docker daemon” patch.

docker/docker/reference has the concept of normalization, where semantically equivalent reference.Named values are always mapped to the same string; the normalization trims the default docker.io hostname and the default /library namespace within it (docker.io/library/busybox→busybox).

• This impacts any code using reference.Named as a data structure instead of strings; even a fully qualified string input by the user is always presented in the normalized (minimal) form on output, through .Name() or .String().
• OTOH, namedRef.FullName always returns a fully qualified version (including a host name). [Throughout, .Hostname() and .RemoteName() behave consistently with .FullName(), i.e. the two repositories differ.]
• namedRef.FullName() also always returns a fully qualified namespace (adding library/ for docker.io hostname, whether explicit or implied); i.e. for both busybox and docker.io/library/busybox, namedRef.Name() is busybox and namedRef.FullName is docker.io/library/busybox.

projectatomic/docker/reference does not have this kind of normalization; instead, it introduces “unqualified” references (which do not contain a hostname on input). ONLY references with an explicit docker.io hostname are normalized by trimming the default /library, but not by trimming the hostname: busybox→busybox, library/busybox→library/busybox, docker.io/library/busybox→docker.io/busybox.

• OTOH, namedRef.FullName for unqualified references does not include a host name!
• namedRef.FullName(), because it semantically does not default the hostname to docker.io, does not return a fully qualified namespace: for busybox, both namedRef.Name() and namedRef.FullName() is busybox, and for docker.io/library/busybox, namedRef.Name() is docker.io/busybox and namedRef.FullName is docker.io/library/busybox.

In docker/docker, any image fetched by docker pull is always tagged using the (normalized! i.e. minimal) version of the input reference (tagged or digested), and if the input was a tagged reference, also using the digested version.

In projectatomic/docker, only qualified images are, after fetching, tagged using the input reference (normalized to drop /library, but including the host name!); when fetching an unqualified reference, the code fetches an image from the first matching registry in the "default registry list”, and tags it (tagged+digested as above) using only a qualified variant of the reference (i.e. with a hostname); not the input unqualified reference.

• This happens only in the pull path; other ways to tag an image do not convert the reference into a qualified one.

This is compensated for in the reference lookup code (reference/store.Get), which looks up qualified references directly as expected EDITalways looks up the input reference directly first, but for unqualified references it also iterates through the “default registry list” and uses the first qualified variant which exists.

So, docker pull busybox tags the image as docker.io/busybox and future references to both docker.io/busybox and busybox look up docker.io/busybox; the raw busybox tag is never fetched.

OTOH, an explicit docker tag busybox, or a docker load which includes an unqualified busybox:something tag, tags the image using this unqualified name; future references to this unqualified name do not look up the unqualified tag and only search for docker.io/busybox (and other hostnames in the "default registry list"). EDIT this image can then only be referenced using this unqualified name, references to a hostname-qualified name do not look up the unqualified tag.

from image.

@runcom If ^^^ is correct (I will take another look on Monday, actually testing all of that instead of just reading code), it seems to me that it would be best for projectatomic/docker to modify reference.store.AddTag to convert attempts to tag with an unqualified reference to tagging with a reference qualified with the first hostname on the default registry list.

This should in one place deal with docker tag, docker load, and all other places where it is possible to tag using an unqualified reference which can then never be found.

Though, admittedly, it is not at all clear that the user intent was to use the first hostname on the list for unqualified references… OTOH just storing an unqualified tag and extending the lookup code to also look up for unqualified tags seems worse, then we can have docker.io/busybox, r.a.redhat.com/busybox and busybox pointing at three different images…

The only certain thing is that I am confused and I will need to return to this on Monday :)

(Sure, the docker-daemon: destination code could ask for the image to be tagged using several tags, e.g. both busybox and docker.io/library/busybox, and AFAICS that should be harmless on the normalizing docker/docker implementation and it would make the image accessible with projectatomic/docker without patching projectatomic/docker; but the docker tag situation would still be broken.)

from image.

This is compensated for in the reference lookup code (reference/store.Get), which looks up qualified references directly as expected, but for unqualified references it also iterates through the “default registry list” and uses the first qualified variant which exists.

Correction: Every reference, qualified or unqualified, is looked up directly as expected. So, the case of docker tag something $whatever; docker inspect $whatever always works fine.

from image.

Overall, the lookup behavior of projectatomic/docker ends up being like this: row indicates how the image was tagged, column what is being looked up. d = docker.io/, l = library/, busybox (e.g. db means docker.io/busybox). ✔️ = tag is found, ❌ = not found.

(With docker/docker, the table is all ✔️ ; all of these references are canonicalized to busybox.)

Locally, the situation is somewhat surprising but understandable: unqualified references have an indeterminate host name, so busybox and library/busybox are treated as separate because with an indeterminate host name the docker.io/library special casing does not apply, and docker.io/*/busybox does not find indeterminatehostname/*/busybox because otherwise it would be reasonable to expect indeterminatehostname/*/busybox to match anyregistryinthedefaultlist/*/busybox at the same time, which does not really make sense.

The trouble with this is (as identified earlier by Scott McCarty) is that docker tag $something busybox:tag; docker push busybox:tag; docker pull busybox:tag does not update the busybox:tag tag, only the docker.io/busybox:tag tag. That does seem clearly wrong.

The fallout for docker-daemon: is that the generated tarball for docker load should include a fully qualified name; docker/docker will normalize it away, but in projectatomic/docker this gets moves us from the top row of the table to the two lower, more interoperable, rows of the table.

from image.

docker/docker will normalize it away, but in projectatomic/docker this gets moves us from the top row of the table to the two lower, more interoperable, rows of the table.

… and, equally important, this way docker load will update the same tags as docker pull. (OTOH, if the user does have a busybox tag, now neither docker pull nor docker load will update it, and references to busybox will use that tag instead of the result of pull/load.)

from image.

@mtrmac could you open a bugzilla so I can fix this in our projectatomic/docker repo with a reproducer and expected behavior?

from image.

@mtrmac could you open a bugzilla so I can fix this in our projectatomic/docker repo with a reproducer and expected behavior?

Filed as https://bugzilla.redhat.com/show_bug.cgi?id=1397198 , feel free to reassign to a different version or product. Now searching in bugzilla, it seems there may be other concerns / risks as well, but those are up to projectatomic/docker; as far as docker-daemon: is concerned, I think I know what to do.

from image.

For containers/image, this was fixed by #165 . The bugzilla above tracks projectatomic/docker.

from image.