FIXME FIXME: Only compress/decompress if the format handler allows it

Introduce SourcedImage.CanChangeLayerCompression and use it
in the copy pipeline to gate compression/decompression, or even
manifest MIME type changes.

FIXME: If we are doing this, we should also ask about the
requested compression algorithm (at least as long as the manifest/*
aditions are public API).

FIXME: Tests

FIXME: How does this interact with format conversions? Anything
to worry about?

Signed-off-by: Miloslav Trmač <mitr@redhat.com>

copy/copy.go

@@ -1412,7 +1412,8 @@ func (ic *imageCopier) copyBlobFromStream(ctx context.Context, srcStream io.Read
 	// short-circuit conditions
 	canModifyBlob := !isConfig && ic.cannotModifyManifestReason == ""
 	// === Deal with layer compression/decompression if necessary
-	canChangeCompression := canModifyBlob
+	layerCompressionChangeSupported := ic.src.CanChangeLayerCompression(srcInfo.MediaType)
+	canChangeCompression := canModifyBlob && layerCompressionChangeSupported
 	compressionMetadata := map[string]string{}
 	var inputInfo types.BlobInfo
 	var compressionOperation types.LayerCompression
@@ -1423,8 +1424,8 @@ func (ic *imageCopier) copyBlobFromStream(ctx context.Context, srcStream io.Read
 	}
 	var uploadCompressorName string
 	if canChangeCompression && isOciEncrypted(srcInfo.MediaType) {
-		// PreserveOriginal due to any compression not being able to be done on an encrypted blob unless decrypted
 		logrus.Debugf("Using original blob without modification for encrypted blob")
+		// PreserveOriginal due to any compression not being able to be done on an encrypted blob unless decrypted
 		compressionOperation = types.PreserveOriginal
 		inputInfo = srcInfo
 		srcCompressorName = internalblobinfocache.UnknownCompression
@@ -1494,7 +1495,11 @@ func (ic *imageCopier) copyBlobFromStream(ctx context.Context, srcStream io.Read
 		// LayerInfosForCopy() returned something that differs from what was in the
 		// source's manifest, and UpdatedImage() needs to call UpdateLayerInfos(),
 		// it will be able to correctly derive the MediaType for the copied blob.
-		if isCompressed {
+		//
+		// But don’t touch blobs in objects where we can’t change compression,
+		// so that src.UpdatedImage() doesn’t fail; assume for such blobs,
+		// LayerInfosForCopy() should not be making any changes in the first place.
+		if layerCompressionChangeSupported && isCompressed {
 			uploadCompressionFormat = &compressionFormat
 		} else {
 			uploadCompressionFormat = nil

internal/image/docker_schema1.go

@@ -251,3 +251,9 @@ func (m *manifestSchema1) SupportsEncryption(context.Context) bool {
 func (m *manifestSchema1) CanSubstituteLayers() bool {
 	return true
 }
+
+// CanChangeLayerCompression returns true if it is allowed to compress/decompress layers with mimeType (and the code can handle that)
+// FIXME FIXME: This does not say which compression formats are supported, and it almost certainly should.
+func (m *manifestSchema1) CanChangeLayerCompression(mimeType string) bool {
+	return true // There are no MIME types in the manifest, so we must assume a valid image.
+}

internal/image/docker_schema1_test.go

@@ -676,3 +676,12 @@ func TestManifestSchema1CanSubstituteLayers(t *testing.T) {
 		assert.True(t, m.CanSubstituteLayers())
 	}
 }
+
+func TestManifestSchema1CanChangeLayerCompression(t *testing.T) {
+	for _, m := range []genericManifest{
+		manifestSchema1FromFixture(t, "schema1.json"),
+		manifestSchema1FromComponentsLikeFixture(t),
+	} {
+		assert.True(t, m.CanChangeLayerCompression(""))
+	}
+}

internal/image/docker_schema2.go

@@ -407,3 +407,9 @@ func (m *manifestSchema2) SupportsEncryption(context.Context) bool {
 func (m *manifestSchema2) CanSubstituteLayers() bool {
 	return true
 }
+
+// CanChangeLayerCompression returns true if it is allowed to compress/decompress layers with mimeType (and the code can handle that)
+// FIXME FIXME: This does not say which compression formats are supported, and it almost certainly should.
+func (m *manifestSchema2) CanChangeLayerCompression(mimeType string) bool {
+	return m.m.CanChangeLayerCompression(mimeType)
+}

internal/image/docker_schema2_test.go

@@ -662,3 +662,14 @@ func TestManifestSchema2CanSubstituteLayers(t *testing.T) {
 		assert.True(t, m.CanSubstituteLayers())
 	}
 }
+
+func TestManifestSchema2CanChangeLayerCompression(t *testing.T) {
+	for _, m := range []genericManifest{
+		manifestSchema2FromFixture(t, mocks.ForbiddenImageSource{}, "schema2.json", false),
+		manifestSchema2FromComponentsLikeFixture(nil),
+	} {
+		assert.True(t, m.CanChangeLayerCompression(manifest.DockerV2Schema2LayerMediaType))
+		// Some projects like to use squashfs and other unspecified formats for layers; don’t touch those.
+		assert.False(t, m.CanChangeLayerCompression("a completely unknown and quite possibly invalid MIME type"))
+	}
+}

internal/image/manifest.go

@@ -56,6 +56,8 @@ type genericManifest interface {
 
 	// CanSubstituteBlobs returns true if the generic image copy code is allowed to substitute layers with their semantic equivalents.
 	CanSubstituteLayers() bool
+	// CanChangeLayerCompression returns true if it is allowed to compress/decompress layers with mimeType (and the code can handle that)
+	CanChangeLayerCompression(mimeType string) bool
 }
 
 // manifestInstanceFromBlob returns a genericManifest implementation for (manblob, mt) in src.

internal/image/oci.go

@@ -266,3 +266,9 @@ func (m *manifestOCI1) CanSubstituteLayers() bool {
 	// Only substitute layers in container images; don’t touch artifacts, the blobs have semantics unknown to us.
 	return m.m.Config.MediaType == imgspecv1.MediaTypeImageConfig
 }
+
+// CanChangeLayerCompression returns true if it is allowed to compress/decompress layers with mimeType (and the code can handle that)
+// FIXME FIXME: This does not say which compression formats are supported, and it almost certainly should.
+func (m *manifestOCI1) CanChangeLayerCompression(mimeType string) bool {
+	return m.m.CanChangeLayerCompression(mimeType)
+}

internal/image/oci_test.go

@@ -554,3 +554,14 @@ func TestManifestOCI1CanSubstituteLayers(t *testing.T) {
 	artifact := manifestOCI1FromFixture(t, mocks.ForbiddenImageSource{}, "oci1-artifact.json")
 	assert.False(t, artifact.CanSubstituteLayers())
 }
+
+func TestManifestOCI1CanChangeLayerCompression(t *testing.T) {
+	for _, m := range []genericManifest{
+		manifestOCI1FromFixture(t, mocks.ForbiddenImageSource{}, "oci1.json"),
+		manifestOCI1FromComponentsLikeFixture(nil),
+	} {
+		assert.True(t, m.CanChangeLayerCompression(imgspecv1.MediaTypeImageLayerGzip))
+		// Some projects like to use squashfs and other unspecified formats for layers; don’t touch those.
+		assert.False(t, m.CanChangeLayerCompression("a completely unknown and quite possibly invalid MIME type"))
+	}
+}

manifest/common.go

@@ -118,6 +118,18 @@ type compressionMIMETypeSet map[string]string
 const mtsUncompressed = ""        // A key in compressionMIMETypeSet for the uncompressed variant
 const mtsUnsupportedMIMEType = "" // A value in compressionMIMETypeSet that means “recognized but unsupported”
 
+// findCompressionMIMETypeSet returns a pointer to a compressionMIMETypeSet in variantTable that contains a value of mimeType, or nil if not found
+func findCompressionMIMETypeSet(variantTable []compressionMIMETypeSet, mimeType string) compressionMIMETypeSet {
+	for _, variants := range variantTable {
+		for _, mt := range variants {
+			if mt == mimeType {
+				return variants
+			}
+		}
+	}
+	return nil
+}
+
 // compressionVariantMIMEType returns a variant of mimeType for the specified algorithm (which may be nil
 // to mean "no compression"), based on variantTable.
 // The returned error will be a ManifestLayerCompressionIncompatibilityError if mimeType has variants
@@ -130,29 +142,26 @@ func compressionVariantMIMEType(variantTable []compressionMIMETypeSet, mimeType
 	if mimeType == mtsUnsupportedMIMEType { // Prevent matching against the {algo:mtsUnsupportedMIMEType} entries
 		return "", fmt.Errorf("cannot update unknown MIME type")
 	}
-	for _, variants := range variantTable {
-		for _, mt := range variants {
-			if mt == mimeType { // Found the variant
-				name := mtsUncompressed
-				if algorithm != nil {
-					name = algorithm.InternalUnstableUndocumentedMIMEQuestionMark()
-				}
-				if res, ok := variants[name]; ok {
-					if res != mtsUnsupportedMIMEType {
-						return res, nil
-					}
-					if name != mtsUncompressed {
-						return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("%s compression is not supported for type %q", name, mt)}
-					}
-					return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("uncompressed variant is not supported for type %q", mt)}
-				}
-				if name != mtsUncompressed {
-					return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("unknown compressed with algorithm %s variant for type %s", name, mt)}
-				}
-				// We can't very well say “the idea of no compression is unknown”
-				return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("uncompressed variant is not supported for type %q", mt)}
+	variants := findCompressionMIMETypeSet(variantTable, mimeType)
+	if variants != nil {
+		name := mtsUncompressed
+		if algorithm != nil {
+			name = algorithm.InternalUnstableUndocumentedMIMEQuestionMark()
+		}
+		if res, ok := variants[name]; ok {
+			if res != mtsUnsupportedMIMEType {
+				return res, nil
 			}
+			if name != mtsUncompressed {
+				return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("%s compression is not supported for type %q", name, mimeType)}
+			}
+			return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("uncompressed variant is not supported for type %q", mimeType)}
+		}
+		if name != mtsUncompressed {
+			return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("unknown compressed with algorithm %s variant for type %s", name, mimeType)}
 		}
+		// We can't very well say “the idea of no compression is unknown”
+		return "", ManifestLayerCompressionIncompatibilityError{fmt.Sprintf("uncompressed variant is not supported for type %q", mimeType)}
 	}
 	if algorithm != nil {
 		return "", fmt.Errorf("unsupported MIME type for compression: %s", mimeType)
@@ -209,3 +218,13 @@ type ManifestLayerCompressionIncompatibilityError struct {
 func (m ManifestLayerCompressionIncompatibilityError) Error() string {
 	return m.text
 }
+
+// compressionVariantsRecognizeMIMEType returns true if variantTable contains data about compressing/decompressing layers with mimeType
+// FIXME FIXME: This does not say which compression formats are supported, and it almost certainly should.
+func compressionVariantsRecognizeMIMEType(variantTable []compressionMIMETypeSet, mimeType string) bool {
+	if mimeType == mtsUnsupportedMIMEType { // Prevent matching against the {algo:mtsUnsupportedMIMEType} entries
+		return false
+	}
+	variants := findCompressionMIMETypeSet(variantTable, mimeType)
+	return variants != nil // Alternatively, this could be len(variants) > 1, but really the caller should ask about a specific algorithm.
+}

manifest/docker_schema2.go

@@ -295,3 +295,9 @@ func (m *Schema2) ImageID([]digest.Digest) (string, error) {
 	}
 	return m.ConfigDescriptor.Digest.Hex(), nil
 }
+
+// CanChangeLayerCompression returns true if it is allowed to compress/decompress layers with mimeType (and the code can handle that)
+// FIXME FIXME: This does not say which compression formats are supported, and it almost certainly should.
+func (m *Schema2) CanChangeLayerCompression(mimeType string) bool {
+	return compressionVariantsRecognizeMIMEType(schema2CompressionMIMETypeSets, mimeType)
+}

manifest/docker_schema2_test.go

@@ -203,3 +203,7 @@ func TestSchema2ImageID(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, "b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7", id)
 }
+
+func TestSchema2CanChangeLayerCompression(t *testing.T) {
+	// FIXME FIXME
+}

manifest/oci.go

@@ -249,3 +249,12 @@ func (m *OCI1) ImageID([]digest.Digest) (string, error) {
 	}
 	return m.Config.Digest.Hex(), nil
 }
+
+// CanChangeLayerCompression returns true if it is allowed to compress/decompress layers with mimeType (and the code can handle that)
+// FIXME FIXME: This does not say which compression formats are supported, and it almost certainly should.
+func (m *OCI1) CanChangeLayerCompression(mimeType string) bool {
+	if m.Config.MediaType != imgspecv1.MediaTypeImageConfig {
+		return false
+	}
+	return compressionVariantsRecognizeMIMEType(oci1CompressionMIMETypeSets, mimeType)
+}

manifest/oci_test.go

@@ -373,3 +373,7 @@ func TestOCI1ImageID(t *testing.T) {
 	var expected NonImageArtifactError
 	assert.ErrorAs(t, err, &expected)
 }
+
+func TestOCI1CanChangeLayerCompression(t *testing.T) {
+	// FIXME FIXME
+}