E540 Disable manifest v2 schema 1 push by thaJeztah · Pull Request #41295 · moby/moby · GitHub
[go: up one dir, main page]
Skip to content

Disable manifest v2 schema 1 push #41295

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Aug 3, 2020
Merged

Disable manifest v2 schema 1 push #41295

merged 1 commit into from
Aug 3, 2020

Conversation

thaJeztah
Copy link
Member FF8
@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

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

@thaJeztah
Copy link
Member Author

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

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

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

@tonistiigi updated, PTAL

@thaJeztah
Copy link
Member Author

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 <github@gone.nl>
@AkihiroSuda
Copy link
Member

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

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

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

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

FF8

@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
0