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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions api/server/router/distribution/distribution_routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"net/http"
"os"

"github.com/distribution/reference"
"github.com/docker/distribution"
Expand All @@ -12,6 +13,7 @@ import (
"github.com/docker/distribution/manifest/schema2"
"github.com/docker/docker/api/server/httputils"
"github.com/docker/docker/api/types/registry"
distributionpkg "github.com/docker/docker/distribution"
"github.com/docker/docker/errdefs"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
Expand Down Expand Up @@ -153,6 +155,9 @@ func (s *distributionRouter) fetchManifest(ctx context.Context, distrepo distrib
}
}
case *schema1.SignedManifest:
if os.Getenv("DOCKER_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE") == "" {
return registry.DistributionInspect{}, distributionpkg.DeprecatedSchema1ImageError(namedRef)
}
platform := ocispec.Platform{
Architecture: mnfstObj.Architecture,
OS: "linux",
Expand Down
8 changes: 7 additions & 1 deletion daemon/containerd/image_pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"io"
"os"
"strings"

"github.com/containerd/containerd"
Expand Down Expand Up @@ -115,7 +116,12 @@ func (i *ImageService) pullTag(ctx context.Context, ref reference.Named, platfor
var sentPullingFrom, sentSchema1Deprecation bool
ah := images.HandlerFunc(func(ctx context.Context, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) {
if desc.MediaType == images.MediaTypeDockerSchema1Manifest && !sentSchema1Deprecation {
progress.Message(out, "", distribution.DeprecatedSchema1ImageMessage(ref))
err := distribution.DeprecatedSchema1ImageError(ref)
if os.Getenv("DOCKER_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE") == "" {
log.G(context.TODO()).Warn(err.Error())
return nil, err
}
progress.Message(out, "", err.Error())
sentSchema1Deprecation = true
}
if images.IsLayerType(desc.MediaType) {
Expand Down
13 changes: 11 additions & 2 deletions distribution/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,15 @@ func (e reservedNameError) Error() string {

func (e reservedNameError) Forbidden() {}

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)
type invalidArgumentErr struct{ error }

func (invalidArgumentErr) InvalidParameter() {}

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)}
}
Comment on lines +220 to 227
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.

11 changes: 11 additions & 0 deletions distribution/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"fmt"
"io"
"os"
"strings"

"github.com/containerd/containerd/content"
Expand Down Expand Up @@ -292,6 +293,11 @@ func detectManifestBlobMediaType(dt []byte) (string, error) {
}
return mfst.MediaType, nil
case schema1.MediaTypeManifest:
if os.Getenv("DOCKER_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE") == "" {
err := DeprecatedSchema1ImageError(nil)
log.G(context.TODO()).Warn(err.Error())
return "", err
}
if mfst.Manifests != nil || mfst.Layers != nil {
return "", fmt.Errorf(`media-type: %q should not have "manifests" or "layers"`, mfst.MediaType)
}
Expand All @@ -303,6 +309,11 @@ func detectManifestBlobMediaType(dt []byte) (string, error) {
}
switch {
case mfst.FSLayers != nil && mfst.Manifests == nil && mfst.Layers == nil && mfst.Config == nil:
if os.Getenv("DOCKER_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE") == "" {
err := DeprecatedSchema1ImageError(nil)
log.G(context.TODO()).Warn(err.Error())
return "", err
}
return schema1.MediaTypeManifest, nil
case mfst.Config != nil && mfst.Manifests == nil && mfst.FSLayers == nil,
mfst.Layers != nil && mfst.Manifests == nil && mfst.FSLayers == nil:
Expand Down
2 changes: 2 additions & 0 deletions distribution/manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,7 @@ func TestDetectManifestBlobMediaType(t *testing.T) {
"mediaType and fsLayers set": {[]byte(`{"mediaType": "bananas", "fsLayers": []}`), "bananas"},
}

t.Setenv("DOCKER_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE", "1")
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
mt, err := detectManifestBlobMediaType(tc.json)
Expand Down Expand Up @@ -431,6 +432,7 @@ func TestDetectManifestBlobMediaTypeInvalid(t *testing.T) {
},
}

t.Setenv("DOCKER_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE", "1")
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
mt, err := detectManifestBlobMediaType(tc.json)
Expand Down
18 changes: 12 additions & 6 deletions distribution/pull_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -424,9 +424,12 @@ func (p *puller) pullTag(ctx context.Context, ref reference.Named, platform *oci

switch v := manifest.(type) {
case *schema1.SignedManifest:
msg := DeprecatedSchema1ImageMessage(ref)
log.G(ctx).Warn(msg)
progress.Message(p.config.ProgressOutput, "", msg)
err := DeprecatedSchema1ImageError(ref)
log.G(ctx).Warn(err.Error())
if os.Getenv("DOCKER_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE") == "" {
return false, err
}
progress.Message(p.config.ProgressOutput, "", err.Error())

id, manifestDigest, err = p.pullSchema1(ctx, ref, v, platform)
if err != nil {
Expand Down Expand Up @@ -857,9 +860,12 @@ func (p *puller) pullManifestList(ctx context.Context, ref reference.Named, mfst

switch v := manifest.(type) {
case *schema1.SignedManifest:
msg := DeprecatedSchema1ImageMessage(ref)
log.G(ctx).Warn(msg)
progress.Message(p.config.ProgressOutput, "", msg)
err := DeprecatedSchema1ImageError(ref)
log.G(ctx).Warn(err.Error())
if os.Getenv("DOCKER_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE") == "" {
return "", "", err
}
progress.Message(p.config.ProgressOutput, "", err.Error())

platform := toOCIPlatform(match.Platform)
id, _, err = p.pullSchema1(ctx, manifestRef, v, platform)
Expand Down
3 changes: 3 additions & 0 deletions hack/make/.integration-daemon-start
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ export DOCKER_ALLOW_SCHEMA1_PUSH_DONOTUSE=1
export DOCKER_GRAPHDRIVER=${DOCKER_GRAPHDRIVER:-vfs}
export DOCKER_USERLANDPROXY=${DOCKER_USERLANDPROXY:-true}

# Allow testing push/pull of legacy image formats
export DOCKER_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE=1

# example usage: DOCKER_STORAGE_OPTS="dm.basesize=20G,dm.loopdatasize=200G"
storage_params=""
if [ -n "$DOCKER_STORAGE_OPTS" ]; then
Expand Down