Add LayerInfosForCopy() to unparsed/source images

Add an LayerInfosForCopy() method to source images which gives them a
chance to provide updated values for the blobsums contained in the
image's manifest, if they want to.  Returning `nil` implies that they
have no changes to suggest compared to what's in the manifest.

When copying an image, if we can update the manifest with those new
values during copying, do so.  If we have new values, but we can't
update the manifest, copying fails.

Update storageImageSource to return its manifest and reference in
unmodified form, and supply updated blob digests via LayerInfosForCopy()
so that copying images from storage works.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>

copy/copy.go

@@ -368,6 +368,15 @@ func (ic *imageCopier) copyLayers() error {
 	srcInfos := ic.src.LayerInfos()
 	destInfos := []types.BlobInfo{}
 	diffIDs := []digest.Digest{}
+	updatedSrcInfos := ic.src.LayerInfosForCopy()
+	srcInfosUpdated := false
+	if updatedSrcInfos != nil && !reflect.DeepEqual(srcInfos, updatedSrcInfos) {
+		if !ic.canModifyManifest {
+			return errors.Errorf("Internal error: copyLayers() needs to use an updated manifest but that was known to be forbidden")
+		}
+		srcInfos = updatedSrcInfos
+		srcInfosUpdated = true
+	}
 	for _, srcLayer := range srcInfos {
 		var (
 			destInfo types.BlobInfo
@@ -396,7 +405,7 @@ func (ic *imageCopier) copyLayers() error {
 	if ic.diffIDsAreNeeded {
 		ic.manifestUpdates.InformationOnly.LayerDiffIDs = diffIDs
 	}
-	if layerDigestsDiffer(srcInfos, destInfos) {
+	if srcInfosUpdated || layerDigestsDiffer(srcInfos, destInfos) {
 		ic.manifestUpdates.LayerInfos = destInfos
 	}
 	return nil

copy/manifest_test.go

@@ -56,6 +56,9 @@ func (f fakeImageSource) OCIConfig() (*v1.Image, error) {
 func (f fakeImageSource) LayerInfos() []types.BlobInfo {
 	panic("Unexpected call to a mock function")
 }
+func (f fakeImageSource) LayerInfosForCopy() []types.BlobInfo {
+	panic("Unexpected call to a mock function")
+}
 func (f fakeImageSource) EmbeddedDockerReferenceConflicts(ref reference.Named) bool {
 	panic("Unexpected call to a mock function")
 }

directory/directory_src.go

@@ -82,3 +82,8 @@ func (s *dirImageSource) GetSignatures(ctx context.Context, instanceDigest *dige
 	}
 	return signatures, nil
 }
+
+// LayerInfosForCopy() returns updated layer info that should be used when copying, in preference to values in the manifest, if specified.
+func (s *dirImageSource) LayerInfosForCopy() []types.BlobInfo {
+	return nil
+}

docker/archive/src.go

@@ -34,3 +34,8 @@ func (s *archiveImageSource) Reference() types.ImageReference {
 func (s *archiveImageSource) Close() error {
 	return nil
 }
+
+// LayerInfosForCopy() returns updated layer info that should be used when reading, in preference to values in the manifest, if specified.
+func (s *archiveImageSource) LayerInfosForCopy() []types.BlobInfo {
+	return nil
+}

docker/daemon/daemon_src.go

@@ -81,3 +81,8 @@ func (s *daemonImageSource) Reference() types.ImageReference {
 func (s *daemonImageSource) Close() error {
 	return os.Remove(s.tarCopyPath)
 }
+
+// LayerInfosForCopy() returns updated layer info that should be used when reading, in preference to values in the manifest, if specified.
+func (s *daemonImageSource) LayerInfosForCopy() []types.BlobInfo {
+	return nil
+}

docker/docker_image_src.go

@@ -52,6 +52,11 @@ func (s *dockerImageSource) Close() error {
 	return nil
 }
 
+// LayerInfosForCopy() returns updated layer info that should be used when reading, in preference to values in the manifest, if specified.
+func (s *dockerImageSource) LayerInfosForCopy() []types.BlobInfo {
+	return nil
+}
+
 // simplifyContentType drops parameters from a HTTP media type (see https://tools.ietf.org/html/rfc7231#section-3.1.1.1)
 // Alternatively, an empty string is returned unchanged, and invalid values are "simplified" to an empty string.
 func simplifyContentType(contentType string) string {

image/docker_schema2_test.go

@@ -38,6 +38,9 @@ func (f unusedImageSource) GetBlob(info types.BlobInfo) (io.ReadCloser, int64, e
 func (f unusedImageSource) GetSignatures(context.Context, *digest.Digest) ([][]byte, error) {
 	panic("Unexpected call to a mock function")
 }
+func (f unusedImageSource) LayerInfosForCopy() []types.BlobInfo {
+	panic("Unexpected call to a mock function")
+}
 
 func manifestSchema2FromFixture(t *testing.T, src types.ImageSource, fixture string) genericManifest {
 	manifest, err := ioutil.ReadFile(filepath.Join("fixtures", fixture))

image/memory.go

@@ -61,3 +61,10 @@ func (i *memoryImage) Signatures(ctx context.Context) ([][]byte, error) {
 func (i *memoryImage) Inspect() (*types.ImageInspectInfo, error) {
 	return inspectManifest(i.genericManifest)
 }
+
+// LayerInfosForCopy returns an updated set of layer blob information which may not match the manifest.
+// The Digest field is guaranteed to be provided; Size may be -1.
+// WARNING: The list may contain duplicates, and they are semantically relevant.
+func (i *memoryImage) LayerInfosForCopy() []types.BlobInfo {
+	return nil
+}

image/sourced.go

@@ -100,3 +100,7 @@ func (i *sourcedImage) Manifest() ([]byte, string, error) {
 func (i *sourcedImage) Inspect() (*types.ImageInspectInfo, error) {
 	return inspectManifest(i.genericManifest)
 }
+
+func (i *sourcedImage) LayerInfosForCopy() []types.BlobInfo {
+	return i.UnparsedImage.LayerInfosForCopy()
+}

image/unparsed.go

@@ -93,3 +93,10 @@ func (i *UnparsedImage) Signatures(ctx context.Context) ([][]byte, error) {
 	}
 	return i.cachedSignatures, nil
 }
+
+// LayerInfosForCopy returns an updated set of layer blob information which may not match the manifest.
+// The Digest field is guaranteed to be provided; Size may be -1.
+// WARNING: The list may contain duplicates, and they are semantically relevant.
+func (i *UnparsedImage) LayerInfosForCopy() []types.BlobInfo {
+	return i.src.LayerInfosForCopy()
+}

oci/archive/oci_src.go

@@ -88,3 +88,8 @@ func (s *ociArchiveImageSource) GetBlob(info types.BlobInfo) (io.ReadCloser, int
 func (s *ociArchiveImageSource) GetSignatures(ctx context.Context, instanceDigest *digest.Digest) ([][]byte, error) {
 	return s.unpackedSrc.GetSignatures(ctx, instanceDigest)
 }
+
+// LayerInfosForCopy() returns updated layer info that should be used when reading, in preference to values in the manifest, if specified.
+func (s *ociArchiveImageSource) LayerInfosForCopy() []types.BlobInfo {
+	return nil
+}

oci/layout/oci_src.go

@@ -143,6 +143,11 @@ func (s *ociImageSource) getExternalBlob(urls []string) (io.ReadCloser, int64, e
 	return nil, 0, errWrap
 }
 
+// LayerInfosForCopy() returns updated layer info that should be used when reading, in preference to values in the manifest, if specified.
+func (s *ociImageSource) LayerInfosForCopy() []types.BlobInfo {
+	return nil
+}
+
 func getBlobSize(resp *http.Response) int64 {
 	size, err := strconv.ParseInt(resp.Header.Get("Content-Length"), 10, 64)
 	if err != nil {

openshift/openshift.go

@@ -246,6 +246,11 @@ func (s *openshiftImageSource) GetSignatures(ctx context.Context, instanceDigest
 	return sigs, nil
 }
 
+// LayerInfosForCopy() returns updated layer info that should be used when reading, in preference to values in the manifest, if specified.
+func (s *openshiftImageSource) LayerInfosForCopy() []types.BlobInfo {
+	return nil
+}
+
 // ensureImageIsResolved sets up s.docker and s.imageStreamImageName
 func (s *openshiftImageSource) ensureImageIsResolved(ctx context.Context) error {
 	if s.docker != nil {

ostree/ostree_src.go

@@ -347,3 +347,8 @@ func (s *ostreeImageSource) GetSignatures(ctx context.Context, instanceDigest *d
 	}
 	return signatures, nil
 }
+
+// LayerInfosForCopy() returns updated layer info that should be used when reading, in preference to values in the manifest, if specified.
+func (s *ostreeImageSource) LayerInfosForCopy() []types.BlobInfo {
+	return nil
+}

signature/policy_reference_match_test.go

@@ -67,6 +67,9 @@ func (ref refImageMock) Manifest() ([]byte, string, error) {
 func (ref refImageMock) Signatures(context.Context) ([][]byte, error) {
 	panic("unexpected call to a mock function")
 }
+func (ref refImageMock) LayerInfosForCopy() []types.BlobInfo {
+	panic("unexpected call to a mock function")
+}
 
 // refImageReferenceMock is a mock of types.ImageReference which returns itself in DockerReference.
 type refImageReferenceMock struct{ reference.Named }
@@ -332,6 +335,9 @@ func (ref forbiddenImageMock) Manifest() ([]byte, string, error) {
 func (ref forbiddenImageMock) Signatures(context.Context) ([][]byte, error) {
 	panic("unexpected call to a mock function")
 }
+func (ref forbiddenImageMock) LayerInfosForCopy() []types.BlobInfo {
+	panic("unexpected call to a mock function")
+}
 
 func testExactPRMAndSig(t *testing.T, prmFactory func(string) PolicyReferenceMatch, imageRef, sigRef string, result bool) {
 	prm := prmFactory(imageRef)

storage/storage_image.go

@@ -14,7 +14,6 @@ import (
 	"strings"
 	"sync/atomic"
 
-	"github.com/containers/image/docker/reference"
 	"github.com/containers/image/image"
 	"github.com/containers/image/manifest"
 	"github.com/containers/image/types"
@@ -46,11 +45,11 @@ var (
 )
 
 type storageImageSource struct {
-	imageRef        storageReference
-	ID              string
-	layerPosition   map[digest.Digest]int // Where we are in reading a blob's layers
-	updatedManifest []byte                // The edited version of the manifest, if already known, or nil
-	SignatureSizes  []int                 `json:"signature-sizes"` // List of sizes of each signature slice
+	imageRef       storageReference
+	ID             string
+	layerPosition  map[digest.Digest]int // Where we are in reading a blob's layers
+	cachedManifest []byte                // A cached copy of the manifest, if already known, or nil
+	SignatureSizes []int                 `json:"signature-sizes"` // List of sizes of each signature slice
 }
 
 type storageImageDestination struct {
@@ -72,27 +71,17 @@ type storageImageCloser struct {
 	size int64
 }
 
-// newImageSource reads an image, modifying the manifest to refer to uncompressed layers.
+// newImageSource sets up an image for reading.
 func newImageSource(imageRef storageReference) (*storageImageSource, error) {
-	// First, locate the image using the original reference.
+	// First, locate the image.
 	img, err := imageRef.resolveImage()
 	if err != nil {
 		return nil, err
 	}
 
-	// FIXME FIXME we need to preserve the ORIGINAL reference.
-	// Build the updated information that we want for the manifest.
-	// Discard a digest in the image reference, if there is one, since the updated image manifest
-	// won't match it.
-	newName := imageRef.name
-	if imageRef.digest != "" {
-		newName = reference.TrimNamed(newName)
-	}
-	newRef := newReference(imageRef.transport, verboseName(newName), img.ID, newName, "", "")
-
-	// Build the reader object, reading the image referred to by a possibly-updated reference.
+	// Build the reader object.
 	image := &storageImageSource{
-		imageRef:       *newRef,
+		imageRef:       imageRef,
 		ID:             img.ID,
 		layerPosition:  make(map[digest.Digest]int),
 		SignatureSizes: []int{},
@@ -129,7 +118,7 @@ func (s *storageImageSource) getBlobAndLayerID(info types.BlobInfo) (rc io.ReadC
 		return nil, -1, "", err
 	}
 	// Check if the blob corresponds to a diff that was used to initialize any layers.  Our
-	// callers should only ask about layers using their uncompressed digests, so no need to
+	// callers should try to retrieve layers using their uncompressed digests, so no need to
 	// check if they're using one of the compressed digests, which we can't reproduce anyway.
 	layers, err := s.imageRef.transport.store.LayersByUncompressedDigest(info.Digest)
 	// If it's not a layer, then it must be a data item.
@@ -174,50 +163,62 @@ func (s *storageImageSource) GetManifest(instanceDigest *digest.Digest) (manifes
 	if instanceDigest != nil {
 		return nil, "", ErrNoManifestLists
 	}
-	if len(s.updatedManifest) == 0 {
-		originalBlob, err := s.imageRef.transport.store.ImageBigData(s.ID, "manifest")
+	if len(s.cachedManifest) == 0 {
+		cachedBlob, err := s.imageRef.transport.store.ImageBigData(s.ID, "manifest")
 		if err != nil {
 			return nil, "", err
 		}
-		man, err := manifest.FromBlob(originalBlob, manifest.GuessMIMEType(originalBlob))
-		if err != nil {
-			return nil, "", err
-		}
-
-		simg, err := s.imageRef.transport.store.Image(s.ID)
-		if err != nil {
-			return nil, "", err
-		}
-		updatedBlobInfos := []types.BlobInfo{}
-		layerID := simg.TopLayer
-		for layerID != "" {
-			layer, err := s.imageRef.transport.store.Layer(layerID)
-			if err != nil {
-				return nil, "", err
-			}
-			if layer.UncompressedDigest == "" {
-				return nil, "", errors.Errorf("uncompressed digest for layer %q is unknown", layerID)
-			}
-			if layer.UncompressedSize < 0 {
-				return nil, "", errors.Errorf("uncompressed size for layer %q is unknown", layerID)
-			}
-			blobInfo := types.BlobInfo{
-				Digest: layer.UncompressedDigest,
-				Size:   layer.UncompressedSize,
-			}
-			updatedBlobInfos = append([]types.BlobInfo{blobInfo}, updatedBlobInfos...)
-			layerID = layer.Parent
-		}
-		if err := man.UpdateLayerInfos(updatedBlobInfos); err != nil {
-			return nil, "", err
-		}
-		updatedBlob, err := man.Serialize()
-		if err != nil {
-			return nil, "", err
-		}
-		s.updatedManifest = updatedBlob
+		s.cachedManifest = cachedBlob
 	}
-	return s.updatedManifest, manifest.GuessMIMEType(s.updatedManifest), err
+	return s.cachedManifest, manifest.GuessMIMEType(s.cachedManifest), err
+}
+
+// LayerInfosForCopy() returns the list of layer blobs that make up the root filesystem of
+// the image, after they've been decompressed.
+func (s *storageImageSource) LayerInfosForCopy() []types.BlobInfo {
+	simg, err := s.imageRef.transport.store.Image(s.ID)
+	if err != nil {
+		logrus.Errorf("error reading image %q: %v", s.ID, err)
+		return nil
+	}
+	updatedBlobInfos := []types.BlobInfo{}
+	layerID := simg.TopLayer
+	_, manifestType, err := s.GetManifest(nil)
+	if err != nil {
+		logrus.Errorf("error reading image manifest for %q: %v", s.ID, err)
+		return nil
+	}
+	uncompressedLayerType := ""
+	switch manifestType {
+	case imgspecv1.MediaTypeImageManifest:
+		uncompressedLayerType = imgspecv1.MediaTypeImageLayer
+	case manifest.DockerV2Schema1MediaType, manifest.DockerV2Schema1SignedMediaType, manifest.DockerV2Schema2MediaType:
+		// This is actually a compressed type, but there's no uncompressed type defined
+		uncompressedLayerType = manifest.DockerV2Schema2LayerMediaType
+	}
+	for layerID != "" {
+		layer, err := s.imageRef.transport.store.Layer(layerID)
+		if err != nil {
+			logrus.Errorf("error reading layer %q in image %q: %v", layerID, s.ID, err)
+			return nil
+		}
+		if layer.UncompressedDigest == "" {
+			logrus.Errorf("uncompressed digest for layer %q is unknown", layerID)
+			return nil
+		}
+		if layer.UncompressedSize < 0 {
+			logrus.Errorf("uncompressed size for layer %q is unknown", layerID)
+			return nil
+		}
+		blobInfo := types.BlobInfo{
+			Digest:    layer.UncompressedDigest,
+			Size:      layer.UncompressedSize,
+			MediaType: uncompressedLayerType,
+		}
+		updatedBlobInfos = append([]types.BlobInfo{blobInfo}, updatedBlobInfos...)
+		layerID = layer.Parent
+	}
+	return updatedBlobInfos
 }
 
 // GetSignatures() parses the image's signatures blob into a slice of byte slices.
@@ -335,8 +336,9 @@ func (s *storageImageDestination) PutBlob(stream io.Reader, blobinfo types.BlobI
 		blobSize = counter.Count
 	}
 	return types.BlobInfo{
-		Digest: blobDigest,
-		Size:   blobSize,
+		Digest:    blobDigest,
+		Size:      blobSize,
+		MediaType: blobinfo.MediaType,
 	}, nil
 }
 
@@ -850,7 +852,7 @@ func (s *storageImageSource) getSize() (int64, error) {
 	return sum, nil
 }
 
-// Size returns the image's previously-computed size.
+// Size() returns the previously-computed size of the image, with no error.
 func (s *storageImageCloser) Size() (int64, error) {
 	return s.size, nil
 }
@@ -871,8 +873,3 @@ func newImage(ctx *types.SystemContext, s storageReference) (types.ImageCloser,
 	}
 	return &storageImageCloser{ImageCloser: img, size: size}, nil
 }
-
-// Size() returns the previously-computed size of the image, with no error.
-func (s storageImage) Size() (int64, error) {
-	return s.size, nil
-}

storage/storage_test.go

@@ -1012,7 +1012,7 @@ func TestDuplicateBlob(t *testing.T) {
 		t.Fatalf("ImageSource is not a storage image")
 	}
 	layers := []string{}
-	for _, layerInfo := range img.LayerInfos() {
+	for _, layerInfo := range img.LayerInfosForCopy() {
 		rc, _, layerID, err := source.getBlobAndLayerID(layerInfo)
 		if err != nil {
 			t.Fatalf("getBlobAndLayerID(%q) returned error %v", layerInfo.Digest, err)

tarball/tarball_src.go

@@ -254,7 +254,7 @@ func (is *tarballImageSource) Reference() types.ImageReference {
 	return &is.reference
 }
 
-// UpdatedLayerInfos() returns updated layer info that should be used when reading, in preference to values in the manifest, if specified.
-func (*tarballImageSource) UpdatedLayerInfos() []types.BlobInfo {
+// LayerInfosForCopy() returns updated layer info that should be used when reading, in preference to values in the manifest, if specified.
+func (*tarballImageSource) LayerInfosForCopy() []types.BlobInfo {
 	return nil
 }

types/types.go

@@ -126,6 +126,10 @@ type ImageSource interface {
 	// (when the primary manifest is a manifest list); this never happens if the primary manifest is not a manifest list
 	// (e.g. if the source never returns manifest lists).
 	GetSignatures(ctx context.Context, instanceDigest *digest.Digest) ([][]byte, error)
+	// LayerInfosForCopy returns either nil (meaning the values in the manifest are fine), or updated values for the layer blobsums that are listed in the image's manifest.
+	// The Digest field is guaranteed to be provided; Size may be -1.
+	// WARNING: The list may contain duplicates, and they are semantically relevant.
+	LayerInfosForCopy() []BlobInfo
 }
 
 // ImageDestination is a service, possibly remote (= slow), to store components of a single image.
@@ -211,6 +215,10 @@ type UnparsedImage interface {
 	Manifest() ([]byte, string, error)
 	// Signatures is like ImageSource.GetSignatures, but the result is cached; it is OK to call this however often you need.
 	Signatures(ctx context.Context) ([][]byte, error)
+	// LayerInfosForCopy returns either nil (meaning the values in the manifest are fine), or updated values for the layer blobsums that are listed in the image's manifest.
+	// The Digest field is guaranteed to be provided, Size may be -1 and MediaType may be optionally provided.
+	// WARNING: The list may contain duplicates, and they are semantically relevant.
+	LayerInfosForCopy() []BlobInfo
 }
 
 // Image is the primary API for inspecting properties of images.