diff --git a/copy/single.go b/copy/single.go index 57e6060987..c233619427 100644 --- a/copy/single.go +++ b/copy/single.go @@ -684,12 +684,7 @@ func (ic *imageCopier) copyLayer(ctx context.Context, srcInfo types.BlobInfo, to logrus.Debugf("Checking if we can reuse blob %s: general substitution = %v, compression for MIME type %q = %v", srcInfo.Digest, ic.canSubstituteBlobs, srcInfo.MediaType, canChangeLayerCompression) canSubstitute := ic.canSubstituteBlobs && ic.src.CanChangeLayerCompression(srcInfo.MediaType) - // TODO: at this point we don't know whether or not a blob we end up reusing is compressed using an algorithm - // that is acceptable for use on layers in the manifest that we'll be writing later, so if we end up reusing - // a blob that's compressed with e.g. zstd, but we're only allowed to write a v2s2 manifest, this will cause - // a failure when we eventually try to update the manifest with the digest and MIME type of the reused blob. - // Fixing that will probably require passing more information to TryReusingBlob() than the current version of - // the ImageDestination interface lets us pass in. + var requiredCompression *compressiontypes.Algorithm if ic.requireCompressionFormatMatch { requiredCompression = ic.compressionFormat @@ -702,14 +697,15 @@ func (ic *imageCopier) copyLayer(ctx context.Context, srcInfo types.BlobInfo, to } reused, reusedBlob, err := ic.c.dest.TryReusingBlobWithOptions(ctx, srcInfo, private.TryReusingBlobOptions{ - Cache: ic.c.blobInfoCache, - CanSubstitute: canSubstitute, - EmptyLayer: emptyLayer, - LayerIndex: &layerIndex, - SrcRef: srcRef, - RequiredCompression: requiredCompression, - OriginalCompression: srcInfo.CompressionAlgorithm, - TOCDigest: tocDigest, + Cache: ic.c.blobInfoCache, + CanSubstitute: canSubstitute, + EmptyLayer: emptyLayer, + LayerIndex: &layerIndex, + SrcRef: srcRef, + PossibleManifestFormats: append([]string{ic.manifestConversionPlan.preferredMIMEType}, ic.manifestConversionPlan.otherMIMETypeCandidates...), + RequiredCompression: requiredCompression, + OriginalCompression: srcInfo.CompressionAlgorithm, + TOCDigest: tocDigest, }) if err != nil { return types.BlobInfo{}, "", fmt.Errorf("trying to reuse blob %s at destination: %w", srcInfo.Digest, err) diff --git a/docker/docker_image_dest.go b/docker/docker_image_dest.go index 72f2fa5f97..877d11b738 100644 --- a/docker/docker_image_dest.go +++ b/docker/docker_image_dest.go @@ -339,7 +339,8 @@ func (d *dockerImageDestination) TryReusingBlobWithOptions(ctx context.Context, return true, reusedInfo, nil } } else { - logrus.Debugf("Ignoring exact blob match case due to compression mismatch ( %s vs %s )", options.RequiredCompression.Name(), optionalCompressionName(options.OriginalCompression)) + logrus.Debugf("Ignoring exact blob match, compression %s does not match required %s or MIME types %#v", + optionalCompressionName(options.OriginalCompression), optionalCompressionName(options.RequiredCompression), options.PossibleManifestFormats) } // Then try reusing blobs from other locations. @@ -361,9 +362,11 @@ func (d *dockerImageDestination) TryReusingBlobWithOptions(ctx context.Context, } if !impl.CandidateMatchesTryReusingBlobOptions(options, compressionAlgorithm) { if !candidate.UnknownLocation { - logrus.Debugf("Ignoring candidate blob %s as reuse candidate due to compression mismatch ( %s vs %s ) in %s", candidate.Digest.String(), options.RequiredCompression.Name(), optionalCompressionName(compressionAlgorithm), candidateRepo.Name()) + logrus.Debugf("Ignoring candidate blob %s in %s, compression %s does not match required %s or MIME types %#v", candidate.Digest.String(), candidateRepo.Name(), + optionalCompressionName(compressionAlgorithm), optionalCompressionName(options.RequiredCompression), options.PossibleManifestFormats) } else { - logrus.Debugf("Ignoring candidate blob %s as reuse candidate due to compression mismatch ( %s vs %s ) with no location match, checking current repo", candidate.Digest.String(), options.RequiredCompression.Name(), optionalCompressionName(compressionAlgorithm)) + logrus.Debugf("Ignoring candidate blob %s with no known location, compression %s does not match required %s or MIME types %#v", candidate.Digest.String(), + optionalCompressionName(compressionAlgorithm), optionalCompressionName(options.RequiredCompression), options.PossibleManifestFormats) } continue } diff --git a/internal/imagedestination/impl/helpers.go b/internal/imagedestination/impl/helpers.go index 6d4d914382..a8a9f51bed 100644 --- a/internal/imagedestination/impl/helpers.go +++ b/internal/imagedestination/impl/helpers.go @@ -1,8 +1,10 @@ package impl import ( + "github.com/containers/image/v5/internal/manifest" "github.com/containers/image/v5/internal/private" compression "github.com/containers/image/v5/pkg/compression/types" + "golang.org/x/exp/slices" ) // CandidateMatchesTryReusingBlobOptions validates if compression is required by the caller while selecting a blob, if it is required @@ -20,6 +22,17 @@ func CandidateMatchesTryReusingBlobOptions(options private.TryReusingBlobOptions } } + // For candidateCompression == nil, we can’t tell the difference between “uncompressed” and “unknown”; + // and “uncompressed” is acceptable in all known formats (well, it seems to work in practice for schema1), + // so don’t impose any restrictions if candidateCompression == nil + if options.PossibleManifestFormats != nil && candidateCompression != nil { + if !slices.ContainsFunc(options.PossibleManifestFormats, func(mt string) bool { + return manifest.MIMETypeSupportsCompressionAlgorithm(mt, *candidateCompression) + }) { + return false + } + } + return true } diff --git a/internal/imagedestination/impl/helpers_test.go b/internal/imagedestination/impl/helpers_test.go index db76f0de64..93749b1bd5 100644 --- a/internal/imagedestination/impl/helpers_test.go +++ b/internal/imagedestination/impl/helpers_test.go @@ -4,26 +4,44 @@ import ( "testing" "github.com/containers/image/v5/internal/private" + "github.com/containers/image/v5/manifest" "github.com/containers/image/v5/pkg/compression" compressionTypes "github.com/containers/image/v5/pkg/compression/types" + imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/stretchr/testify/assert" ) func TestCandidateMatchesTryReusingBlobOptions(t *testing.T) { - var opts private.TryReusingBlobOptions cases := []struct { - requiredCompression *compressionTypes.Algorithm - candidateCompression *compressionTypes.Algorithm - result bool + requiredCompression *compressionTypes.Algorithm + possibleManifestFormats []string + candidateCompression *compressionTypes.Algorithm + result bool }{ - {&compression.Zstd, &compression.Zstd, true}, - {&compression.Gzip, &compression.Zstd, false}, - {&compression.Zstd, nil, false}, - {nil, &compression.Zstd, true}, + // RequiredCompression restrictions + {&compression.Zstd, nil, &compression.Zstd, true}, + {&compression.Gzip, nil, &compression.Zstd, false}, + {&compression.Zstd, nil, nil, false}, + {nil, nil, &compression.Zstd, true}, + // PossibleManifestFormats restrictions + {nil, []string{imgspecv1.MediaTypeImageManifest}, &compression.Zstd, true}, + {nil, []string{manifest.DockerV2Schema2MediaType}, &compression.Zstd, false}, + {nil, []string{manifest.DockerV2Schema2MediaType, manifest.DockerV2Schema1SignedMediaType, imgspecv1.MediaTypeImageManifest}, &compression.Zstd, true}, + {nil, nil, &compression.Zstd, true}, + {nil, []string{imgspecv1.MediaTypeImageManifest}, &compression.Gzip, true}, + {nil, []string{manifest.DockerV2Schema2MediaType}, &compression.Gzip, true}, + {nil, []string{manifest.DockerV2Schema2MediaType, manifest.DockerV2Schema1SignedMediaType, imgspecv1.MediaTypeImageManifest}, &compression.Gzip, true}, + {nil, nil, &compression.Gzip, true}, + // Some possible combinations (always 1 constraint not matching) + {&compression.Zstd, []string{manifest.DockerV2Schema2MediaType}, &compression.Zstd, false}, + {&compression.Gzip, []string{manifest.DockerV2Schema2MediaType, manifest.DockerV2Schema1SignedMediaType, imgspecv1.MediaTypeImageManifest}, &compression.Zstd, false}, } for _, c := range cases { - opts = private.TryReusingBlobOptions{RequiredCompression: c.requiredCompression} + opts := private.TryReusingBlobOptions{ + RequiredCompression: c.requiredCompression, + PossibleManifestFormats: c.possibleManifestFormats, + } assert.Equal(t, c.result, CandidateMatchesTryReusingBlobOptions(opts, c.candidateCompression)) } } diff --git a/internal/private/private.go b/internal/private/private.go index 9a52a749f8..7037755bfc 100644 --- a/internal/private/private.go +++ b/internal/private/private.go @@ -112,12 +112,13 @@ type TryReusingBlobOptions struct { // Transports, OTOH, MUST support these fields being zero-valued for types.ImageDestination callers // if they use internal/imagedestination/impl.Compat; // in that case, they will all be consistently zero-valued. - EmptyLayer bool // True if the blob is an "empty"/"throwaway" layer, and may not necessarily be physically represented. - LayerIndex *int // If the blob is a layer, a zero-based index of the layer within the image; nil otherwise. - SrcRef reference.Named // A reference to the source image that contains the input blob. - RequiredCompression *compression.Algorithm // If set, reuse blobs with a matching algorithm as per implementations in internal/imagedestination/impl.helpers.go - OriginalCompression *compression.Algorithm // Must be set if RequiredCompression is set; can be set to nil to indicate “uncompressed” or “unknown”. - TOCDigest *digest.Digest // If specified, the blob can be looked up in the destination also by its TOC digest. + EmptyLayer bool // True if the blob is an "empty"/"throwaway" layer, and may not necessarily be physically represented. + LayerIndex *int // If the blob is a layer, a zero-based index of the layer within the image; nil otherwise. + SrcRef reference.Named // A reference to the source image that contains the input blob. + PossibleManifestFormats []string // A set of possible manifest formats; at least one should support the reused layer blob. + RequiredCompression *compression.Algorithm // If set, reuse blobs with a matching algorithm as per implementations in internal/imagedestination/impl.helpers.go + OriginalCompression *compression.Algorithm // May be nil to indicate “uncompressed” or “unknown”. + TOCDigest *digest.Digest // If specified, the blob can be looked up in the destination also by its TOC digest. } // ReusedBlob is information about a blob reused in a destination.