Skip to content
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.

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

disable pulling legacy image formats by default #47459

Merged
merged 1 commit into from Feb 28, 2024

Conversation

thaJeztah
Copy link
Member

This patch disables pulling legacy (schema1 and schema 2, version 1) images by default.

A DOCKER_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE environment-variable is introduced to allow re-enabling this feature, aligning with the environment variable used in containerd 2.0 (CONTAINERD_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE).

With this patch, attempts to pull a legacy image produces an error:

With graphdrivers:

docker pull docker:1.0
1.0: Pulling from library/docker
[DEPRECATION NOTICE] Docker Image Format v1, and Docker Image manifest version 2, schema 1 support will be removed in an upcoming release. Suggest the author of docker.io/library/docker:1.0 to upgrade the image to the OCI Format, or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/

With the containerd image store enabled, output is slightly different as it returns the error before printing the 1.0: pulling ...:

docker pull docker:1.0
Error response from daemon: [DEPRECATION NOTICE] Docker Image Format v1 and Docker Image manifest version 2, schema 1 support is disabled by default and will be removed in an upcoming release. Suggest the author of docker.io/library/docker:1.0 to upgrade the image to the OCI Format or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/

Using the "distribution" endpoint to resolve the digest for an image also produces an error:

curl -v --unix-socket /var/run/docker.sock http://foo/distribution/docker.io/library/docker:1.0/json
*   Trying /var/run/docker.sock:0...
* Connected to foo (/var/run/docker.sock) port 80 (#0)
> GET /distribution/docker.io/library/docker:1.0/json HTTP/1.1
> Host: foo
> User-Agent: curl/7.88.1
> Accept: */*
>
< HTTP/1.1 400 Bad Request
< Api-Version: 1.45
< Content-Type: application/json
< Docker-Experimental: false
< Ostype: linux
< Server: Docker/dev (linux)
< Date: Tue, 27 Feb 2024 16:09:42 GMT
< Content-Length: 354
<
{"message":"[DEPRECATION NOTICE] Docker Image Format v1, and Docker Image manifest version 2, schema 1 support will be removed in an upcoming release. Suggest the author of docker.io/library/docker:1.0 to upgrade the image to the OCI Format, or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/"}
* Connection #0 to host foo left intact

Starting the daemon with the DOCKER_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE env-var set to a non-empty value allows pulling the image;

docker pull docker:1.0
[DEPRECATION NOTICE] Docker Image Format v1 and Docker Image manifest version 2, schema 1 support is disabled by default and will be removed in an upcoming release. Suggest the author of docker.io/library/docker:1.0 to upgrade the image to the OCI Format or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/
b0a0e6710d13: Already exists
d193ad713811: Already exists
ba7268c3149b: Already exists
c862d82a67a2: Already exists
Digest: sha256:5e7081837926c7a40e58881bbebc52044a95a62a2ea52fb240db3fc539212fe5
Status: Image is up to date for docker:1.0
docker.io/library/docker:1.0

- What I did

- How I did it

- How to verify it

- Description for the changelog

- Disable pulling of deprecated image formats by default. These image formats are deprecated, and support will be removed in a future version.

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

@thaJeztah

This comment was marked as resolved.

@thaJeztah thaJeztah marked this pull request as ready for review February 27, 2024 18:43
func DeprecatedSchema1ImageMessage(ref reference.Named) string {
return fmt.Sprintf("[DEPRECATION NOTICE] Docker Image Format v1, and Docker Image manifest version 2, schema 1 support will be removed in an upcoming release. Suggest the author of %s to upgrade the image to the OCI Format, or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/", ref)
return fmt.Sprintf("[DEPRECATION NOTICE] Docker Image Format v1 and Docker Image manifest version 2, schema 1 support is disabled by default and will be removed in an upcoming release. Suggest the author of %s to upgrade the image to the OCI Format or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/", ref)
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 var should be printed here as a hint?

Copy link
Member Author

@thaJeztah thaJeztah Feb 27, 2024

Choose a reason for hiding this comment

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

I was a bit in doubt on adding even more text to the already length error, and considered instead to include that information in the deprecation docs; we can point the "/go/" redirect to the appropriate location (which can outline the details, as well as the temporary escape hatch).

It might be good for those that need it to understand the context, and be aware that they really should not depend on it.

Happy to hear your (and other's) thoughts on that though!

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think it's better to put more info on the deprecation page. It's also an opportunity to provide more information and mention that this env var needs to be put on the daemon side (not just DOCKER_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE=1 docker pull)

distribution/errors.go Outdated Show resolved Hide resolved
This patch disables pulling legacy (schema1 and schema 2, version 1) images by
default.

A `DOCKER_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE` environment-variable is
introduced to allow re-enabling this feature, aligning with the environment
variable used in containerd 2.0 (`CONTAINERD_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE`).

With this patch, attempts to pull a legacy image produces an error:

With graphdrivers:

    docker pull docker:1.0
    1.0: Pulling from library/docker
    [DEPRECATION NOTICE] Docker Image Format v1, and Docker Image manifest version 2, schema 1 support will be removed in an upcoming release. Suggest the author of docker.io/library/docker:1.0 to upgrade the image to the OCI Format, or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/

With the containerd image store enabled, output is slightly different
as it returns the error before printing the `1.0: pulling ...`:

    docker pull docker:1.0
    Error response from daemon: [DEPRECATION NOTICE] Docker Image Format v1 and Docker Image manifest version 2, schema 1 support is disabled by default and will be removed in an upcoming release. Suggest the author of docker.io/library/docker:1.0 to upgrade the image to the OCI Format or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/

Using the "distribution" endpoint to resolve the digest for an image also
produces an error:

    curl -v --unix-socket /var/run/docker.sock http://foo/distribution/docker.io/library/docker:1.0/json
    *   Trying /var/run/docker.sock:0...
    * Connected to foo (/var/run/docker.sock) port 80 (#0)
    > GET /distribution/docker.io/library/docker:1.0/json HTTP/1.1
    > Host: foo
    > User-Agent: curl/7.88.1
    > Accept: */*
    >
    < HTTP/1.1 400 Bad Request
    < Api-Version: 1.45
    < Content-Type: application/json
    < Docker-Experimental: false
    < Ostype: linux
    < Server: Docker/dev (linux)
    < Date: Tue, 27 Feb 2024 16:09:42 GMT
    < Content-Length: 354
    <
    {"message":"[DEPRECATION NOTICE] Docker Image Format v1, and Docker Image manifest version 2, schema 1 support will be removed in an upcoming release. Suggest the author of docker.io/library/docker:1.0 to upgrade the image to the OCI Format, or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/"}
    * Connection #0 to host foo left intact

Starting the daemon with the `DOCKER_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE`
env-var set to a non-empty value allows pulling the image;

    docker pull docker:1.0
    [DEPRECATION NOTICE] Docker Image Format v1 and Docker Image manifest version 2, schema 1 support is disabled by default and will be removed in an upcoming release. Suggest the author of docker.io/library/docker:1.0 to upgrade the image to the OCI Format or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/
    b0a0e6710d13: Already exists
    d193ad713811: Already exists
    ba7268c3149b: Already exists
    c862d82a67a2: Already exists
    Digest: sha256:5e7081837926c7a40e58881bbebc52044a95a62a2ea52fb240db3fc539212fe5
    Status: Image is up to date for docker:1.0
    docker.io/library/docker:1.0

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Comment on lines +220 to 227
func DeprecatedSchema1ImageError(ref reference.Named) error {
msg := "[DEPRECATION NOTICE] Docker Image Format v1 and Docker Image manifest version 2, schema 1 support is disabled by default and will be removed in an upcoming release."
if ref != nil {
msg += " Suggest the author of " + ref.String() + " to upgrade the image to the OCI Format or Docker Image manifest v2, schema 2."
}
msg += " More information at https://docs.docker.com/go/deprecated-image-specs/"
return invalidArgumentErr{errors.New(msg)}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Updated; I decided to change this to return an error as well; that way we don't have to bother constructing an "InvalidParameter" error in all call-sides, and just do that here.

Copy link
Contributor

@vvoland vvoland left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah merged commit 81428bf into moby:master Feb 28, 2024
126 checks passed
@thaJeztah thaJeztah deleted the disable_schema1 branch February 28, 2024 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants