Skip to content

Disable manifest v2 schema 1 push#41295

Merged
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:disable_v1_push
Aug 3, 2020
Merged

Disable manifest v2 schema 1 push#41295
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:disable_v1_push

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Jul 29, 2020 •
edited
Loading

temporary alternative for

For CI, a temporary DOCKER_ALLOW_SCHEMA1_PUSH_DONOTUSE environment variable was added while we work out a solution for testing schema 1 pulls (which currently require pushing them to a local registry first for testing).

- What I did

- How I did it

- How to verify it

- Description for the changelog

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

@thaJeztah thaJeztah added this to the 20.03.0 milestone Jul 29, 2020
@thaJeztah
Copy link
Member Author

thaJeztah commented Jul 29, 2020

Opening as draft; this should currently fail as I'm not setting the environment variable yet in CI

@thaJeztah
Copy link
Member Author

thaJeztah commented Jul 29, 2020

Great; it's working;

=== RUN   TestDockerSchema1RegistrySuite/TestConcurrentPullMultipleTags
    --- FAIL: TestDockerSchema1RegistrySuite/TestConcurrentPullMultipleTags (0.71s)
        cli.go:29: assertion failed: 
            Command:  /usr/local/cli/docker push 127.0.0.1:5000/dockercli/busybox:recent
            ExitCode: 1
            Error:    exit status 1
            Stdout:   The push refers to repository [127.0.0.1:5000/dockercli/busybox]
            1be74353c3d0: Preparing
            1be74353c3d0: Pushed
            
            Stderr:   [DEPRECATED] support for manifest v2 schema1 has been removed. More information at https://docs.docker.com/registry/spec/deprecated-schema-v1/
            
            
            Failures:
            ExitCode was 1 expected 0
            Expected no error
        check_test.go:223: [d4255422b6b97] daemon is not started

@thaJeztah thaJeztah changed the title [WIP] Disable manifest v2 schema 1 push Disable manifest v2 schema 1 push Jul 29, 2020
@thaJeztah thaJeztah marked this pull request as ready for review July 29, 2020 20:25
@thaJeztah thaJeztah requested a review from tianon as a code owner July 29, 2020 20:25
tonistiigi
tonistiigi reviewed Jul 29, 2020
View reviewed changes
distribution/push_v2.go Outdated
Copy link
Member

@tonistiigi tonistiigi Jul 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

support for pushing

distribution/push_v2.go Outdated
Copy link
Member

@tonistiigi tonistiigi Jul 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The env should contain _UNSUPPORTED or _DONOTUSE

tonistiigi
tonistiigi reviewed Jul 29, 2020
View reviewed changes
distribution/push_v2.go Outdated
Copy link
Member

@tonistiigi tonistiigi Jul 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should return the original error. Can wrap it with removal message.

edit: Test how it looks like for example when pushing to invalid ref. Doesn't make sense to get deprecation warning about schema1 if I'm just not getting a connection to registry at all. Possibly error happens on specific way when schema2 is not supported.

@thaJeztah
Copy link
Member Author

thaJeztah commented Jul 30, 2020

@tonistiigi updated, PTAL

@thaJeztah
Copy link
Member Author

thaJeztah commented Jul 31, 2020

Failure is unrelated (I'll kick CI once more), but the change in this PR does show more details about what failed;

[2020-07-31T16:01:46.077Z] 
[2020-07-31T16:01:46.077Z] === FAIL: amd64.integration-cli TestDockerDaemonSuite/TestVolumePlugin (11.34s)
[2020-07-31T16:01:46.077Z]     --- FAIL: TestDockerDaemonSuite/TestVolumePlugin (11.34s)
[2020-07-31T16:01:46.077Z]         docker_cli_daemon_plugins_test.go:197: Could not install plugin: exit status 1 latest: Pulling from tiborvass/sample-volume-plugin
[2020-07-31T16:01:46.077Z]             Digest: sha256:00b42de88f3a3e0342e7b35fa62394b0a9ceb54d37f4c50be5d3167899994639
[2020-07-31T16:01:46.077Z]             eb9c16fbdc53: Waiting
[2020-07-31T16:01:46.077Z]             eb9c16fbdc53: Pulling fs layer
[2020-07-31T16:01:46.078Z]             failed to copy: httpReaderSeeker: failed open: failed to do request: Get https://docker-images-prod.s3.amazonaws.com/registry-v2/docker/registry/v2/blobs/sha256/eb/eb9c16fbdc53743f4d78ad80a41a1ff4c51f5bd6947e71c1256550f476eb6859/data?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=ASIA2KUBRXV6JIHJGGZE%2F20200731%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20200731T152635Z&X-Amz-Expires=1200&X-Amz-Security-Token=IQoJb3JpZ2luX2VjEHwaCXVzLWVhc3QtMSJIMEYCIQCEpTqjZ5tWNaJyLclMzc9O06cKvxbOE1BvvLoLCVSGQAIhAIgQph%2B8OKfqGM4mXdFO5neR%2B53s37aAzEQ%2BWVlytxLPKrQDCEUQABoMNzEwMDE1MDQwODkyIgyIVrvHcJXMQ47bUHcqkQOeJb2d8hgMYbwKPtNuCwTmB7R1BvasjzfdgpTZjewNcvAb8Xw2uBU1owIhNZGeO9dZfqcFL3MKNe4JfzARJ8zasSMOePPGptYMQ7IoBfjP0Mbb4FcdtWSnXpwgnwFh3E4lqjwAxWgZTNCCOBmuB2gEfRzGNgn%2FfgLPxQlhk30o%2FFRePXVNkKSHYmP9vEUrqz43pfsIZsuFgt3OEufX2Knz1zYKqrs0Yce1kvJcw9EqwKN4iXyO%2BB8%2F57WP%2BW80p5CJ1k88TcNfprJMxzeoJWcBiypmE%2BY%2BQSOeA8jogJN0XkxWqGlpnHfqxvqK8cQRxkHsKJYLuqdt0oJxr5ISiotp4j8mN7RJnL5ud2L1uRMU6fy4AL20sZ%2BrQ%2FWYkq8JmYtrmwr5X%2FucN5H2zs9ysSm04eO%2Ba8DguZI%2Fb6Y8PA%2BVgPIQ5zfBQSRStN%2FpBV%2B0HwBV7XOb3GuwXgnIgCb0RqzF5I6tuUfgkt%2BJodn4XxFc%2BTv2YztyP5OWIZvJShKQ17mtF1jtWFb6AndqQXw1QT0Z0DCajZD5BTrqAasDKQ6WELRw9p58xndipSK%2FBRNCYhSeJU7gjK%2FRKi1uk%2FsACgKlHEBYbVLmwNpAmZzczA9JMmZ8ZgdRingV21KRFh1DDlUoFqBXubuEikpg4SwY8isaexrvOSxsCnBJdOdewQG3jxtRvKPFbuikzvWnIGqiIahr3Ov6FeCpgDuCvWuHE6qbfLaZuMnxyw6xgoOjUwiA1AORJ5hOCn%2BynQaZykKB4jXzI7fshwZxbaHHwkcJtNnr05tgHcvpQ3mWe9KVaUszMCzoyAj1nKBuZP3tTM2gN%2BHsiODzxpzBQpWsrI8zVrpWdXPBrg%3D%3D&X-Amz-SignedHeaders=host&X-Amz-Signature=4d61069d28eabb022f3e8ef69545ffb1e46e4ca2d2262ec8948bcacbc22b40a9: net/http: TLS handshake timeout
[2020-07-31T16:01:46.078Z]

@thaJeztah
Disable manifest v2 schema 1 push
6302dbb 
For CI, a temporary `DOCKER_ALLOW_SCHEMA1_PUSH_DONOTUSE` environment
variable was added while we work out a solution for testing schema 1
pulls (which currently require pushing them to a local registry first
for testing).

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@AkihiroSuda
Copy link
Member

AkihiroSuda commented Aug 3, 2020

while we work out a solution for testing schema 1 pulls (which currently require pushing them to a local registry first for testing).

Could we run docker:19.03-dind? Or does skopeo support schema 1 push?

@thaJeztah
Copy link
Member Author

thaJeztah commented Aug 3, 2020

Could we run docker:19.03-dind? Or does skopeo support schema 1 push?

we were discussing to either have a registry image with the v1 images "baked in", or to load the images directly in the registry storage, or perhaps an image with just the data store for a registry. Actually pushing images wouldn't strictly be needed in that approach (but well have to try what works best)

tonistiigi
tonistiigi approved these changes Aug 3, 2020
View reviewed changes
distribution/push_v2.go
// manifest v2 schema 1 images to test-registries used for testing *pulling*
// these images.
if os.Getenv("DOCKER_ALLOW_SCHEMA1_PUSH_DONOTUSE") == "" {
if err.Error() == "tag invalid" {
Copy link
Member

@tonistiigi tonistiigi Aug 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why oh why would it fail with such a misleading error message 😞

Copy link
Member Author

@thaJeztah thaJeztah Aug 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea 😞 this is what the test returned as error, so it's a "best effort"

@thaJeztah thaJeztah merged commit c560686 into moby:master Aug 3, 2020
@thaJeztah thaJeztah deleted the disable_v1_push branch August 3, 2020 16:33
@thaJeztah thaJeztah mentioned this pull request Nov 12, 2020
Closed
This was referenced Jan 2, 2021
Open
Closed
@thaJeztah thaJeztah mentioned this pull request Nov 18, 2021
Merged
@AkihiroSuda AkihiroSuda mentioned this pull request May 2, 2022
Merged
@agners agners mentioned this pull request Sep 7, 2022
Merged
@thaJeztah thaJeztah mentioned this pull request May 21, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tonistiigi tonistiigi tonistiigi approved these changes

@tiborvass tiborvass tiborvass approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

@tianon tianon Awaiting requested review from tianon tianon is a code owner

Assignees

No one assigned

Labels

area/distribution Image Distribution impact/changelog impact/deprecation impact/documentation status/2-code-review

Projects

None yet

Milestone

20.10.0

Development

Successfully merging this pull request may close these issues.

4 participants

@thaJeztah @AkihiroSuda @tonistiigi @tiborvass