Skip to content

Commit

Permalink
Implement LayerInfosForCopy changing MIME types
Browse files Browse the repository at this point in the history
Currently, horribly, types.UpdatedImage is _documented_ to ignore
ManifestUpdateOptions.LayerInfos.MediaType; so if LayerInfosForCopy
replaces a layer with a differently-compressed one, that MIME type
update is never performed.

First, work around that by adding updatedImageWithMIME as a
hopefully temporary hack that implements the edit using CompressionOperation
+ CompressionAlgorithm (which are not documented to be ignored).

Then, use it to process LayerInfosForCopy.  Because LayerInfosForCopy's
MediaType values are manifest-schema specific, we must call UpdatedImage(WithMime)
twice: once just to update the layers with LayerInfosForCopy information
using the original schema, and later to possibly convert the manifest and apply
CompressionOperation/... from results of TryReusingBlob/PutBlob.  So,
move the LayerInfosForCopy processing before determineManifestConversion
requests a MIME type change.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
  • Loading branch information
mtrmac committed Feb 12, 2021
1 parent ea27591 commit 2877b1b
Showing 1 changed file with 54 additions and 9 deletions.
63 changes: 54 additions & 9 deletions copy/copy.go
Expand Up @@ -632,6 +632,10 @@ func (c *copier) copyOneImage(ctx context.Context, policyContext *signature.Poli
return nil, "", "", err
}

if err := ic.useLayerInfosForCopy(ctx); err != nil {
return nil, "", "", err
}

destRequiresOciEncryption := (isEncrypted(src) && ic.c.ociDecryptConfig != nil) || options.OciEncryptLayers != nil
// We compute preferredManifestMIMEType only to show it in error messages.
// Without having to add this context in an error message, we would be happy enough to know only that no conversion is needed.
Expand Down Expand Up @@ -791,22 +795,36 @@ func isTTY(w io.Writer) bool {
return false
}

// copyLayers copies layers from ic.src/ic.c.rawSource to dest, using and updating ic.manifestUpdates if necessary and ic.canModifyManifest.
func (ic *imageCopier) copyLayers(ctx context.Context) error {
// useLayerInfosForCopy implements LayerInfosForCopy, if necessary.
// It may modify/replace ic.src.
func (ic *imageCopier) useLayerInfosForCopy(ctx context.Context) error {
srcInfos := ic.src.LayerInfos()
numLayers := len(srcInfos)
updatedSrcInfos, err := ic.src.LayerInfosForCopy(ctx)
if err != nil {
return err
}
srcInfosUpdated := false
if updatedSrcInfos != nil && !reflect.DeepEqual(srcInfos, updatedSrcInfos) {
if !ic.canModifyManifest {
return errors.Errorf("Copying this image requires changing layer representation, which is not possible (image is signed or the destination specifies a digest)")
}
srcInfos = updatedSrcInfos
srcInfosUpdated = true
updates := *ic.manifestUpdates // A shallow copy
updates.LayerInfos = updatedSrcInfos
// We don’t set options.InformationOnly.LayerInfos and options.InformationOnly.LayerDiffIDs,
// because (we don’t have that information, and) the current update implementations don’t need that
// when we are not converting the manifest format.
img, err := updatedImageWithMIME(ctx, ic.src, updates)
if err != nil {
return errors.Wrapf(err, "Error preparing an internally-modified image manifest")
}
ic.src = img
}
return nil
}

// copyLayers copies layers from ic.src/ic.c.rawSource to dest, using and updating ic.manifestUpdates if necessary and ic.canModifyManifest.
func (ic *imageCopier) copyLayers(ctx context.Context) error {
srcInfos := ic.src.LayerInfos()
numLayers := len(srcInfos)

type copyLayerData struct {
destInfo types.BlobInfo
Expand Down Expand Up @@ -878,8 +896,7 @@ func (ic *imageCopier) copyLayers(ctx context.Context) error {
}()

for i, srcLayer := range srcInfos {
err = copySemaphore.Acquire(ctx, 1)
if err != nil {
if err := copySemaphore.Acquire(ctx, 1); err != nil {
return errors.Wrapf(err, "Can't acquire semaphore")
}
copyGroup.Add(1)
Expand All @@ -906,7 +923,7 @@ func (ic *imageCopier) copyLayers(ctx context.Context) error {
if ic.diffIDsAreNeeded {
ic.manifestUpdates.InformationOnly.LayerDiffIDs = diffIDs
}
if srcInfosUpdated || layerDigestsDiffer(srcInfos, destInfos) {
if layerDigestsDiffer(srcInfos, destInfos) {
ic.manifestUpdates.LayerInfos = destInfos
}
return nil
Expand All @@ -925,6 +942,34 @@ func layerDigestsDiffer(a, b []types.BlobInfo) bool {
return false
}

// updatedImageWithMIME is a horrible workaround for the fact that types.Image.UpdatedImage() is documented to
// ignore ManifestUpdateOptions.LayerInfos[].MediaType.
// Other than that it behaves like img.UpdatedImage(ctx, options).
// TO DO: Move this to c/image.sourcedImage and implement directly instead of the
// CompressionAlgorithm workaround.
func updatedImageWithMIME(ctx context.Context, img types.Image, options types.ManifestUpdateOptions) (types.Image, error) {
if options.LayerInfos == nil {
return img.UpdatedImage(ctx, options)
}

optionsCopy := options
optionsCopy.LayerInfos = []types.BlobInfo{}
for _, b := range options.LayerInfos {
if b.MediaType != "" && b.CompressionOperation == types.PreserveOriginal && b.CompressionAlgorithm == nil {
// FIXME: c/image/copy has no business knowing this
switch b.MediaType {
case manifest.DockerV2Schema2LayerMediaType, imgspecv1.MediaTypeImageLayerGzip:
b.CompressionAlgorithm = &compression.Gzip
case imgspecv1.MediaTypeImageLayerZstd:
b.CompressionAlgorithm = &compression.Zstd
default: // Punt and hope for the best. This includes completely unknown blob types.
}
}
optionsCopy.LayerInfos = append(optionsCopy.LayerInfos, b)
}
return img.UpdatedImage(ctx, optionsCopy)
}

// copyUpdatedConfigAndManifest updates the image per ic.manifestUpdates, if necessary,
// stores the resulting config and manifest to the destination, and returns the stored manifest
// and its digest.
Expand Down

0 comments on commit 2877b1b

Please sign in to comment.