Remove support for pulling v2 schema1

[thaJeztah]

• carry of Remove v2 schema1 push #39384 (rebased and adjusted for changes since that was opened)
• closes Remove v2 schema1 push #39384

Last commit is a quick push / reminder of what needs to be done:

WIP: remove DOCKER_ALLOW_SCHEMA1_PUSH_DONOTUSE hack

This PR will still fail, as we need to find a different solution to provision a "v2 schema1" registry to test pulling these manifests.

Some solutions;

• create an image with the registry and images pre-loaded
• export the registry's storage, and load it for tests
• look for a suitable image on Docker Hub, and pull that (Docker Hub may currently be converting manifests; need to check)

---

Manifest v2 schema1 was deprecated in 4866f51 (#39365) and this PR
removes the push code for v2 schema1.

- Human readable description for the release notes

Remove support for pulling legacy v2, schema 1 images and remove `DOCKER_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE` environment-variable.

- A picture of a cute animal (not mandatory but encouraged)

[thaJeztah]

Looks like we have one test depending on an ancient image;

=== RUN   TestDockerNetworkSuite/TestConntrackFlowsLeak
    docker_cli_network_unix_test.go:1751: assertion failed: 
        Command:  /usr/local/cli/docker run -d --name server --net testbind -p 8080:8080/udp appropriate/nc sh -c while true; do echo hello | nc -w 1 -lu 8080; done
        ExitCode: 125
        Error:    exit status 125
        Stdout:   
        Stderr:   Unable to find image 'appropriate/nc:latest' locally
        /usr/local/cli/docker: Error response from daemon: unsupported manifest media type and no default available: application/vnd.docker.distribution.manifest.v1+prettyjws.
        See '/usr/local/cli/docker run --help'.
        
        
        Failures:
        ExitCode was 125 expected 0
        Expected no error
    docker_cli_network_unix_test.go:46: [db3f1e5df27d0] daemon is not started
    --- FAIL: TestDockerNetworkSuite/TestConntrackFlowsLeak (0.32s)

Image is literally just an apk install netcat-openbsd

IMAGE                                                                     CREATED       CREATED BY                                                                                          SIZE      COMMENT
sha256:867ff23d913e68c1c032a4c547d315fd1d2f543cb85e772fff325d4756e37732   6 years ago   /bin/sh -c #(nop) CMD ["nc"]                                                                        0B
<missing>                                                                 6 years ago   /bin/sh -c #(nop) ENTRYPOINT &{["/entrypoint.sh"]}                                                  0B
<missing>                                                                 6 years ago   /bin/sh -c #(nop) COPY file:cf241fd8b9027436b115396345e77913cf86121c3a7ef848845d9ec974280c61 in /   143B
<missing>                                                                 6 years ago   /bin/sh -c apk add --update netcat-openbsd && rm -rf /var/cache/apk/*                               3.02MB
<missing>                                                                 6 years ago   /bin/sh -c #(nop) ADD file:43a95cc218d164ff589cb91519964373d53b607469f5ccce725631916392cd88 in /    5.25MB

[thaJeztah]

I just recalled that I already looked at that test / image at some point; #34832. Let me give it another go

Original image was built from https://github.com/appropriate/docker-nc/tree/master/latest

[thaJeztah]

Whoop; CI is green now, so that's a start

[thaJeztah]

We discussed this PR in the maintainers meeting. In general, everyone seemed to agree that (permitted there's no large use of these images) it should be ok to remove this.

The provisional schema 2 v1 format became deprecated when v2 was introduced in 2015 (distribution/distribution#1068). Originally, support in Docker was meant to be removed in 2018 (#37874), but extended for some time to allow users and registries to migrate (#39365 / docker-archive#284 / docker/cli#1956).

We briefly discussed an issue we encountered some time ago where (due to some bug on Docker Hub?) images were pushed (or converted on pull?) as schema 2 v1 images, but @tianon mentioned that that issue was resolved, and hasn't been seen for quite a while, so probably not a concern.

Some other concerns were raised by @tonistiigi about the actual removal:

• Even though we have printed a warning, uses tend to ignore warnings, so they might still be caught by surprise (admitted: even our own CI turned out to be using some v1 image: TestConntrackFlowsLeak: use busybox "nc" #43031)
• The deprecation warning may not be visible to all users. When we deprecated support, we found that some registries only supported the deprecated format (quay.io, some big companies in China, and (I think) Red Hat's registry). Given that there was no option for users to address that, we made a change in distribution: modify warning logic when pulling v2 schema1 manifests #39736 to only print the warning when pulling from Docker Hub. That said; this was more than 2 Years ago now, and those registries now support the current spec.
• Containerd currently has support for the deprecated schema; this was done to provide a clean migration path for users switching from DockerShim to Containerd CRI.
• As a consequence of the above, it would still be possible to:
  • use a v2 schema1 image in a Dockerfile (FROM), because BuildKit uses containerd's distribution code (note that pushing is not possible even in that case).
  • use v2 schema1 images in Kubernetes (using containerd CRI), but pulling the same image with Docker would fail.
  • users using the DockerShim for Kubernetes on the other hand, will see these images fail (given: this might be a good incentive to migrate to using containerd CRI, as the DockerShim is being deprecated in upstream kubernetes)

For the above, it was suggested if we could synchronize this with containerd (both deprecating and removing v2 schema 1).

@cpuguy83 started a discussion with the containerd maintainers (copying from the slack conversation);

Discussing removing schema1 support in docker. It came up that "we should synchronize this with containerd". Are there thoughts on removing this in cri and ctr?

From @dmcgowan:

Is there any data on who (if anyone) is still relying on this? Previously it was quay which had a bunch of these images and a few big companies in China seemed to use it (3+ years ago at this point)
in terms of timeline, after 1.6 I am thinking of proposing 1.7 scoped to many of the new APIs we have discussed, then immediately after 1.7 ripping out deprecated stuff and starting a 2.0 beta. This would be an excellent candidate for removal then

From @fuweid:

yeah. 2018-2019，alibaba group was using schema1ed images(10%) internally. I didn't see it in 2020. Agreed to deprecate it in 1.7

So questions from the above:

• Do we have any insight (from Docker Hub?) into how many pulls are still happening for these deprecated images?
• (related); what's the proposed deprecation gonna look like for Docker Hub? I think there's some automatic conversion of images (?) will this be disabled? I don't think pulling of legacy will be disabled in other ways (correct?)
• Are we ok with removing support (for this release), before containerd (possible other tools) have removed support?
• For the docker build (with BuildKit) case; do we consider this a problem? Do we want an option to disable the v2 schema 1 support in containerd (so that BuildKit can disable it in the vendored code)?

@justincormack ^^ perhaps you have some input on this?

Let me also /cc

• @jonjohnsonjr (who opened Next steps for schema 1 deprecation? docker/roadmap#173)
• @kolyshkin @giuseppe (as a FYI, and perhaps they know if PodMan and cri-o have deprecation on their roadmap)

[giuseppe]

@vrothberg @mtrmac PTAL

[justincormack]

• We still need to do the data analysis from Docker Hub. I believe the number of pulls is very low, and we can consider converting images (or asking owners to convert) if necessary, its just a pull and push after all.
• We won't remove any automatic conversion in the short term although we also want to remove that in the long term. I suspect there is more of this going on, again we need to look at data.
• Happy to remove before containerd, and even if buildkit doesn't. We will get signal to see if it affects users which will help everyone. If users have niche use cases they can still use old software.
• buildkit should follow containerd if they use their code.

[thaJeztah]

Happy to remove before containerd, and even if buildkit doesn't. We will get signal to see if it affects users which will help everyone. If users have niche use cases they can still use old software.

I agree, I think it should be fine; we've deprecated it long time ago now, and have been delaying removal multiple times. Let me also check with the containerd maintainers if they would be ok with deprecating (not removing) this. I think it's good to warn users early, to raise awareness of this deprecated version of the spec. (cc @dmcgowan @dims @fuweid)

@vrothberg @mtrmac PTAL

Thanks @giuseppe! I wasn't sure who was the right person to ping 😅
It's of course up to each project, but I thought it would be nice if we could signal deprecation across the whole "container ecosystem"; at least I don't think anyone is a fan of having to maintain too much code to keep backward compatibility with ancient schemas 😂

[vrothberg]

Thank you for pulling us in.

at least I don't think anyone is a fan of having to maintain too much code to keep backward compatibility with ancient schemas

Totally agree 😄 I will reach out to Quay and ask if they have stats on schema 1 images. I am always happy to add things but am terrified of removals in fear of breaking users.

[mtrmac]

WRT c/image, it’s quite true that the conversion from schema1 (having to compute diffID values) is a hassle and it’s a bit attractive to drop that code, but the code already exists, and the users affected by these changes need some path forward, so I’m inclined to keep the code (of the generic copy pipeline, at least) as is, including the current schema1 support. (New transports might easily choose not to support schema1, and the generic copy pipeline’s conversion implementation would deal with that.)

Registries are already turning off schema1 (by default?), which seems sufficient to push the ecosystem forward. It might be worth removing schema1 from the default list of formats to try converting to…

My primary wish in this transition is to have distribution/distribution#2925 , to allow clients to report useful error messages. Just dropping the format support from the clients entirely is a way to make that moot, I suppose.

[mtrmac]

Thank you for pulling us in.

at least I don't think anyone is a fan of having to maintain too much code to keep backward compatibility with ancient schemas

I will reach out to Quay and ask if they have stats on schema 1 images. I am always happy to add things but am terrified of removals in fear of breaking users.

… and schema1-only on-premise server deployments? The public quay.io transition was relatively recent, I have no idea what that means about the transition schedule of private instances. Private, internet-unknown registries that are unlikely to upgrade are my primary worry WRT the client changes.

[thaJeztah]

Related;

• Remove the support for Schema 1 images containerd/containerd#11681

[vvoland]

Looking at: https://github.com/moby/moby/pull/42300/files#diff-faff1af3d8ff408964a57b2e475f69a6b7c7b71c9978cccc8f471798caac2c88

I think we don't need to comment it out?

[thaJeztah]

oh! probably this was because of Jenkins at the time; yes, likely I can remove this part

[thaJeztah]

👍 removed

[thaJeztah]

Looks like GitHub cache had some hiccups; rebased to get a fresh run

[vvoland]

Hmm, where is this error defined?

[thaJeztah]

Good question; ISTR this was a bit of a hack, because there was no other way to detect this from the request, so this may have been a "best effort" match for what the registry returned.

[vvoland]

Right, but I don't think that distribution ever returned this error string.

https://grep.app/search?f.lang=Go&q=%22tag+invalid%22 points only to some shellhub using this string.

Wondering if we could just drop it.

[thaJeztah]

LOL; looks like I found the best reference in the PR that disabled push... 5 Years ago; I think it was Docker Hub that returned it as error, so it may have been docker-hub specific; #41295 (comment)

All that said, I don't think this PR make the situation worse; it "may" work and show the more useful error, otherwise just returns the original error as-is.

Honestly, not even sure if we can hit this code at all now; hitting this code would mean that we somehow managed to get a v1 image into the daemon? So after this PR makes its way in, I think we should look at removing all the special handling here (at most "unknown / unsupported manifest type").

[vvoland]

Sure, we can handle that in a follow up

[thaJeztah]

OK, let's bring this one in .. it's been 7(!?) years since we started this in #37874 😂