diff --git a/copy/copy.go b/copy/copy.go index 9fc0e5123b..e3c610974c 100644 --- a/copy/copy.go +++ b/copy/copy.go @@ -1232,7 +1232,7 @@ func (c *copier) copyBlobFromStream(ctx context.Context, srcStream io.Reader, sr logrus.Debugf("Using original blob without modification for encrypted blob") compressionOperation = types.PreserveOriginal inputInfo = srcInfo - } else if canModifyBlob && c.dest.DesiredLayerCompression() == types.Compress && !isCompressed { + } else if canModifyBlob && c.dest.DesiredBlobCompression(srcInfo) == types.Compress && !isCompressed { logrus.Debugf("Compressing blob on the fly") compressionOperation = types.Compress pipeReader, pipeWriter := io.Pipe() @@ -1245,7 +1245,7 @@ func (c *copier) copyBlobFromStream(ctx context.Context, srcStream io.Reader, sr destStream = pipeReader inputInfo.Digest = "" inputInfo.Size = -1 - } else if canModifyBlob && c.dest.DesiredLayerCompression() == types.Compress && isCompressed && desiredCompressionFormat.Name() != compressionFormat.Name() { + } else if canModifyBlob && c.dest.DesiredBlobCompression(srcInfo) == types.Compress && isCompressed && desiredCompressionFormat.Name() != compressionFormat.Name() { // When the blob is compressed, but the desired format is different, it first needs to be decompressed and finally // re-compressed using the desired format. logrus.Debugf("Blob will be converted") @@ -1265,7 +1265,7 @@ func (c *copier) copyBlobFromStream(ctx context.Context, srcStream io.Reader, sr destStream = pipeReader inputInfo.Digest = "" inputInfo.Size = -1 - } else if canModifyBlob && c.dest.DesiredLayerCompression() == types.Decompress && isCompressed { + } else if canModifyBlob && c.dest.DesiredBlobCompression(srcInfo) == types.Decompress && isCompressed { logrus.Debugf("Blob will be decompressed") compressionOperation = types.Decompress s, err := decompressor(destStream) diff --git a/directory/directory_dest.go b/directory/directory_dest.go index d70b6c07fb..f6b7dc8f98 100644 --- a/directory/directory_dest.go +++ b/directory/directory_dest.go @@ -107,6 +107,13 @@ func (d *dirImageDestination) DesiredLayerCompression() types.LayerCompression { return types.PreserveOriginal } +func (d *dirImageDestination) DesiredBlobCompression(info types.BlobInfo) types.LayerCompression { + if d.compress { + return types.Compress + } + return types.PreserveOriginal +} + // AcceptsForeignLayerURLs returns false iff foreign layers in manifest should be actually // uploaded to the image destination, true otherwise. func (d *dirImageDestination) AcceptsForeignLayerURLs() bool { diff --git a/directory/directory_test.go b/directory/directory_test.go index fdb0ce68c1..4a4d04bb68 100644 --- a/directory/directory_test.go +++ b/directory/directory_test.go @@ -68,8 +68,9 @@ func TestGetPutBlob(t *testing.T) { dest, err := ref.NewImageDestination(context.Background(), nil) require.NoError(t, err) defer dest.Close() - assert.Equal(t, types.PreserveOriginal, dest.DesiredLayerCompression()) - info, err := dest.PutBlob(context.Background(), bytes.NewReader(blob), types.BlobInfo{Digest: digest.Digest("sha256:digest-test"), Size: int64(9)}, cache, false) + blobInfo := types.BlobInfo{Digest: digest.Digest("sha256:digest-test"), Size: int64(9)} + assert.Equal(t, types.PreserveOriginal, dest.DesiredBlobCompression(blobInfo)) + info, err := dest.PutBlob(context.Background(), bytes.NewReader(blob), blobInfo, cache, false) assert.NoError(t, err) err = dest.Commit(context.Background(), nil) assert.NoError(t, err) diff --git a/docker/archive/dest.go b/docker/archive/dest.go index 1cf197429b..3ac5eac2ab 100644 --- a/docker/archive/dest.go +++ b/docker/archive/dest.go @@ -47,11 +47,15 @@ func newImageDestination(sys *types.SystemContext, ref archiveReference) (types. }, nil } -// DesiredLayerCompression indicates if layers must be compressed, decompressed or preserved func (d *archiveImageDestination) DesiredLayerCompression() types.LayerCompression { return types.Decompress } +// DesiredBlobCompression indicates if layers must be compressed, decompressed or preserved +func (d *archiveImageDestination) DesiredBlobCompression(info types.BlobInfo) types.LayerCompression { + return types.Decompress +} + // Reference returns the reference used to set up this destination. Note that this should directly correspond to user's intent, // e.g. it should use the public hostname instead of the result of resolving CNAMEs or following redirects. func (d *archiveImageDestination) Reference() types.ImageReference { diff --git a/docker/daemon/daemon_dest.go b/docker/daemon/daemon_dest.go index c6afd4bde0..97ba8b76f1 100644 --- a/docker/daemon/daemon_dest.go +++ b/docker/daemon/daemon_dest.go @@ -87,11 +87,15 @@ func imageLoadGoroutine(ctx context.Context, c *client.Client, reader *io.PipeRe defer resp.Body.Close() } -// DesiredLayerCompression indicates if layers must be compressed, decompressed or preserved func (d *daemonImageDestination) DesiredLayerCompression() types.LayerCompression { return types.PreserveOriginal } +// DesiredBlobCompression indicates if layers must be compressed, decompressed or preserved +func (d *daemonImageDestination) DesiredBlobCompression(info types.BlobInfo) types.LayerCompression { + return types.PreserveOriginal +} + // MustMatchRuntimeOS returns true iff the destination can store only images targeted for the current runtime architecture and OS. False otherwise. func (d *daemonImageDestination) MustMatchRuntimeOS() bool { return d.mustMatchRuntimeOS diff --git a/docker/docker_image_dest.go b/docker/docker_image_dest.go index 979100ee38..960e81c758 100644 --- a/docker/docker_image_dest.go +++ b/docker/docker_image_dest.go @@ -20,6 +20,7 @@ import ( "github.com/containers/image/v5/manifest" "github.com/containers/image/v5/pkg/blobinfocache/none" "github.com/containers/image/v5/types" + ociencspec "github.com/containers/ocicrypt/spec" "github.com/docker/distribution/registry/api/errcode" v2 "github.com/docker/distribution/registry/api/v2" "github.com/docker/distribution/registry/client" @@ -92,6 +93,25 @@ func (d *dockerImageDestination) DesiredLayerCompression() types.LayerCompressio return types.Compress } +func (d *dockerImageDestination) DesiredBlobCompression(info types.BlobInfo) types.LayerCompression { + // it's ok to compress known media types + if manifest.SupportedSchema2MediaType(info.MediaType) == nil { + return types.Compress + } + switch info.MediaType { + case imgspecv1.MediaTypeImageLayer, imgspecv1.MediaTypeImageLayerGzip, imgspecv1.MediaTypeImageLayerNonDistributable, imgspecv1.MediaTypeImageLayerNonDistributableGzip, imgspecv1.MediaTypeImageLayerNonDistributableZstd, imgspecv1.MediaTypeImageLayerZstd, ociencspec.MediaTypeLayerEnc, ociencspec.MediaTypeLayerGzipEnc: + return types.Compress + } + + // v1 manifests didn't have MediaTypes, so "" is still "known" + if info.MediaType == "" { + return types.Compress + } + + // don't compress anything else + return types.PreserveOriginal +} + // AcceptsForeignLayerURLs returns false iff foreign layers in manifest should be actually // uploaded to the image destination, true otherwise. func (d *dockerImageDestination) AcceptsForeignLayerURLs() bool { diff --git a/docker/docker_image_dest_test.go b/docker/docker_image_dest_test.go new file mode 100644 index 0000000000..3641a4ff3c --- /dev/null +++ b/docker/docker_image_dest_test.go @@ -0,0 +1,14 @@ +package docker + +import ( + "testing" + + "github.com/containers/image/v5/types" + "github.com/stretchr/testify/assert" +) + +func TestDockerKnownLayerCompression(t *testing.T) { + dest := dockerImageDestination{} + info := types.BlobInfo{MediaType: "this is not a known media type"} + assert.Equal(t, types.PreserveOriginal, dest.DesiredBlobCompression(info)) +} diff --git a/image/docker_schema2_test.go b/image/docker_schema2_test.go index 5b2a2aa4e0..6deb25ed3f 100644 --- a/image/docker_schema2_test.go +++ b/image/docker_schema2_test.go @@ -403,6 +403,9 @@ func (d *memoryImageDest) SupportsSignatures(ctx context.Context) error { func (d *memoryImageDest) DesiredLayerCompression() types.LayerCompression { panic("Unexpected call to a mock function") } +func (d *memoryImageDest) DesiredBlobCompression(info types.BlobInfo) types.LayerCompression { + panic("Unexpected call to a mock function") +} func (d *memoryImageDest) AcceptsForeignLayerURLs() bool { panic("Unexpected call to a mock function") } diff --git a/oci/archive/oci_dest.go b/oci/archive/oci_dest.go index 0509eaa83b..aa6cac783f 100644 --- a/oci/archive/oci_dest.go +++ b/oci/archive/oci_dest.go @@ -61,7 +61,11 @@ func (d *ociArchiveImageDestination) SupportsSignatures(ctx context.Context) err } func (d *ociArchiveImageDestination) DesiredLayerCompression() types.LayerCompression { - return d.unpackedDest.DesiredLayerCompression() + return d.unpackedDest.DesiredBlobCompression(types.BlobInfo{}) +} + +func (d *ociArchiveImageDestination) DesiredBlobCompression(info types.BlobInfo) types.LayerCompression { + return d.unpackedDest.DesiredBlobCompression(info) } // AcceptsForeignLayerURLs returns false iff foreign layers in manifest should be actually diff --git a/oci/layout/oci_dest.go b/oci/layout/oci_dest.go index fb0449ca52..8a201fd951 100644 --- a/oci/layout/oci_dest.go +++ b/oci/layout/oci_dest.go @@ -11,6 +11,7 @@ import ( "github.com/containers/image/v5/manifest" "github.com/containers/image/v5/types" + ociencspec "github.com/containers/ocicrypt/spec" digest "github.com/opencontainers/go-digest" imgspec "github.com/opencontainers/image-spec/specs-go" imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" @@ -88,9 +89,27 @@ func (d *ociImageDestination) DesiredLayerCompression() types.LayerCompression { if d.acceptUncompressedLayers { return types.PreserveOriginal } + return types.Compress } +func (d *ociImageDestination) DesiredBlobCompression(info types.BlobInfo) types.LayerCompression { + if d.acceptUncompressedLayers { + return types.PreserveOriginal + } + + if manifest.SupportedSchema2MediaType(info.MediaType) == nil { + return types.Compress + } + + switch info.MediaType { + case imgspecv1.MediaTypeImageLayer, imgspecv1.MediaTypeImageLayerGzip, imgspecv1.MediaTypeImageLayerNonDistributable, imgspecv1.MediaTypeImageLayerNonDistributableGzip, imgspecv1.MediaTypeImageLayerNonDistributableZstd, imgspecv1.MediaTypeImageLayerZstd, ociencspec.MediaTypeLayerEnc, ociencspec.MediaTypeLayerGzipEnc: + return types.Compress + } + + return types.PreserveOriginal +} + // AcceptsForeignLayerURLs returns false iff foreign layers in manifest should be actually // uploaded to the image destination, true otherwise. func (d *ociImageDestination) AcceptsForeignLayerURLs() bool { diff --git a/oci/layout/oci_dest_test.go b/oci/layout/oci_dest_test.go index f6feefc6ab..64cedca140 100644 --- a/oci/layout/oci_dest_test.go +++ b/oci/layout/oci_dest_test.go @@ -155,3 +155,16 @@ func putTestManifest(t *testing.T, ociRef ociReference, tmpDir string) { digest := digest.FromBytes(data).Encoded() assert.Contains(t, paths, filepath.Join(tmpDir, "blobs", "sha256", digest), "The OCI directory does not contain the new manifest data") } + +func TestOCIDesiredBlobCompression(t *testing.T) { + ref, tmpDir := refToTempOCI(t) + defer os.RemoveAll(tmpDir) + ociRef, ok := ref.(ociReference) + require.True(t, ok) + + imageDest, err := newImageDestination(nil, ociRef) + assert.NoError(t, err) + + info := types.BlobInfo{MediaType: "this is not a known media type"} + assert.Equal(t, types.PreserveOriginal, imageDest.DesiredBlobCompression(info)) +} diff --git a/openshift/openshift.go b/openshift/openshift.go index 28bfc456d5..37f621069d 100644 --- a/openshift/openshift.go +++ b/openshift/openshift.go @@ -372,6 +372,10 @@ func (d *openshiftImageDestination) DesiredLayerCompression() types.LayerCompres return types.Compress } +func (d *openshiftImageDestination) DesiredBlobCompression(info types.BlobInfo) types.LayerCompression { + return types.Compress +} + // AcceptsForeignLayerURLs returns false iff foreign layers in manifest should be actually // uploaded to the image destination, true otherwise. func (d *openshiftImageDestination) AcceptsForeignLayerURLs() bool { diff --git a/ostree/ostree_dest.go b/ostree/ostree_dest.go index 1150970559..3b2bda8c24 100644 --- a/ostree/ostree_dest.go +++ b/ostree/ostree_dest.go @@ -110,7 +110,7 @@ func (d *ostreeImageDestination) SupportsSignatures(ctx context.Context) error { } // ShouldCompressLayers returns true iff it is desirable to compress layer blobs written to this destination. -func (d *ostreeImageDestination) DesiredLayerCompression() types.LayerCompression { +func (d *ostreeImageDestination) DesiredBlobCompression() types.LayerCompression { return types.PreserveOriginal } diff --git a/storage/storage_image.go b/storage/storage_image.go index df4b67c7a7..89c7b5eaf6 100644 --- a/storage/storage_image.go +++ b/storage/storage_image.go @@ -372,6 +372,10 @@ func (s *storageImageDestination) Close() error { } func (s *storageImageDestination) DesiredLayerCompression() types.LayerCompression { + return types.PreserveOriginal +} + +func (s *storageImageDestination) DesiredBlobCompression(info types.BlobInfo) types.LayerCompression { // We ultimately have to decompress layers to populate trees on disk // and need to explicitly ask for it here, so that the layers' MIME // types can be set accordingly. diff --git a/storage/storage_test.go b/storage/storage_test.go index c4709db6e3..de1e61247c 100644 --- a/storage/storage_test.go +++ b/storage/storage_test.go @@ -392,16 +392,12 @@ func TestWriteRead(t *testing.T) { if err := dest.SupportsSignatures(context.Background()); err != nil { t.Fatalf("Destination image doesn't support signatures: %v", err) } - t.Logf("compress layers: %v", dest.DesiredLayerCompression()) - compression := archive.Uncompressed - if dest.DesiredLayerCompression() == types.Compress { - compression = archive.Gzip - } - digest, decompressedSize, size, blob := makeLayer(t, compression) - if _, err := dest.PutBlob(context.Background(), bytes.NewBuffer(blob), types.BlobInfo{ + digest, decompressedSize, size, blob := makeLayer(t, archive.Uncompressed) + info := types.BlobInfo{ Size: size, Digest: digest, - }, cache, false); err != nil { + } + if _, err := dest.PutBlob(context.Background(), bytes.NewBuffer(blob), info, cache, false); err != nil { t.Fatalf("Error saving randomly-generated layer to destination: %v", err) } t.Logf("Wrote randomly-generated layer %q (%d/%d bytes) to destination", digest, size, decompressedSize) diff --git a/types/types.go b/types/types.go index d469e03b53..9ffe03e844 100644 --- a/types/types.go +++ b/types/types.go @@ -280,7 +280,13 @@ type ImageDestination interface { // Note: It is still possible for PutSignatures to fail if SupportsSignatures returns nil. SupportsSignatures(ctx context.Context) error // DesiredLayerCompression indicates the kind of compression to apply on layers + // + // Deprecated: DesiredLayerCompression can vary by layer media type (in + // particular, destinations probably shouldn't mess with media types + // they don't understand). Use DesiredBlobCompression() instead. DesiredLayerCompression() LayerCompression + // DesiredBlobCompression indicates the kind of compression to apply on layers + DesiredBlobCompression(info BlobInfo) LayerCompression // AcceptsForeignLayerURLs returns false iff foreign layers in manifest should be actually // uploaded to the image destination, true otherwise. AcceptsForeignLayerURLs() bool