pull,load: use *Image instead of re-resolving via name

Following commit fixes a `race` condition in `libimage` because in `Pull(`
after performing `copy` from remote sources it agains attempts to resolve
image via `LookupImage`, any operation between `copy` and `LookupImage` can remove
`name` from the recently pulled image. Causing race in builds.

This issue was discoverd while working on PR containers/buildah#5971
```
buildah build -t test --jobs=2 --skip-unused-stages=false .
```

Containerfile
```
FROM quay.io/jitesoft/alpine
RUN arch
FROM --platform=linux/arm64 quay.io/jitesoft/alpine AS foreign
```

Following commit also addresses the commit e2324dd
by performing the neccessary refactor.

No functional change in public exposed API, exisiting tests should pass as-is.
[NO NEW TESTS NEEDED]

Signed-off-by: flouthoc <[email protected]>

Author: flouthoc

libimage/copier.go

@@ -21,8 +21,10 @@ import (
 	"github.com/containers/image/v5/signature"
 	"github.com/containers/image/v5/signature/signer"
 	storageTransport "github.com/containers/image/v5/storage"
+	"github.com/containers/image/v5/transports"
 	"github.com/containers/image/v5/types"
 	encconfig "github.com/containers/ocicrypt/config"
+	"github.com/containers/storage"
 	"github.com/sirupsen/logrus"
 )
 
@@ -350,6 +352,12 @@ func (c *Copier) Close() error {
 // Copy the source to the destination.  Returns the bytes of the copied
 // manifest which may be used for digest computation.
 func (c *Copier) Copy(ctx context.Context, source, destination types.ImageReference) ([]byte, error) {
+	return c.copyInternal(ctx, source, destination, nil)
+}
+
+// Copy the source to the destination.  Returns the bytes of the copied
+// manifest which may be used for digest computation.
+func (c *Copier) copyInternal(ctx context.Context, source, destination types.ImageReference, reportResolvedReference *types.ImageReference) ([]byte, error) {
 	logrus.Debugf("Copying source image %s to destination image %s", source.StringWithinTransport(), destination.StringWithinTransport())
 
 	// Avoid running out of time when running inside a systemd unit by
@@ -381,6 +389,12 @@ func (c *Copier) Copy(ctx context.Context, source, destination types.ImageRefere
 			)
 		}
 
+		// This is already set when `newCopier` was called but there is an option
+		// to override it by callers if needed.
+		if reportResolvedReference != nil {
+			c.imageCopyOptions.ReportResolvedReference = reportResolvedReference
+		}
+
 		// From `man systemd.service(5)`:
 		//
 		// "If a service of Type=notify/Type=notify-reload sends "EXTEND_TIMEOUT_USEC=...", this may cause
@@ -472,6 +486,21 @@ func (c *Copier) Copy(ctx context.Context, source, destination types.ImageRefere
 	return returnManifest, retry.IfNecessary(ctx, f, &c.retryOptions)
 }
 
+func (c *Copier) copyToStorage(ctx context.Context, source, destination types.ImageReference, reportResolvedReference *types.ImageReference) (*storage.Image, error) {
+	_, err := c.copyInternal(ctx, source, destination, reportResolvedReference)
+	if err != nil {
+		return nil, fmt.Errorf("internal error: unable to copy from source %s: %w", source, err)
+	}
+	if reportResolvedReference == nil {
+		return nil, fmt.Errorf("internal error: After attempting to copy %s, resolvedReference is nil", source)
+	}
+	_, image, err := storageTransport.ResolveReference(*reportResolvedReference)
+	if err != nil {
+		return nil, fmt.Errorf("resolving an already-resolved reference %q to the pulled image: %w", transports.ImageName(*reportResolvedReference), err)
+	}
+	return image, nil
+}
+
 // checkRegistrySourcesAllows checks the $BUILD_REGISTRY_SOURCES environment
 // variable, if it's set.  The contents are expected to be a JSON-encoded
 // github.com/openshift/api/config/v1.Image, set by an OpenShift build

libimage/image_test.go

@@ -61,15 +61,11 @@ func TestImageFunctions(t *testing.T) {
 	// Just make sure that the ID has 64 characters.
 	require.True(t, len(image.ID()) == 64, "ID should be 64 characters long")
 
-	// Make sure that the image we pulled by digest is the same one we
-	// pulled by tag.
-	require.Equal(t, origDigest.String(), image.Digest().String(), "digests of pulled images should match")
-
 	// NOTE: we're recording two digests. One for the image and one for the
 	// manifest list we chose it from.
 	digests := image.Digests()
 	require.Len(t, digests, 2)
-	require.Equal(t, origDigest.String(), digests[0].String(), "first recorded digest should be the one of the image")
+	require.Contains(t, digests, origDigest, "original digest string should be present in the digests array of new pulled image")
 
 	// containers/podman/issues/12729: make sure manifest lookup returns
 	// the correct error for both digests.

libimage/load.go

@@ -30,7 +30,7 @@ func (r *Runtime) doLoadReference(ctx context.Context, ref types.ImageReference,
 	case dockerArchiveTransport.Transport.Name():
 		images, err = r.loadMultiImageDockerArchive(ctx, ref, &options.CopyOptions)
 	default:
-		images, err = r.copyFromDefault(ctx, ref, &options.CopyOptions)
+		_, images, err = r.copyFromDefault(ctx, ref, &options.CopyOptions)
 	}
 	return images, ref.Transport().Name(), err
 }
@@ -49,6 +49,9 @@ func (r *Runtime) LoadReference(ctx context.Context, ref types.ImageReference, o
 // Load loads one or more images (depending on the transport) from the
 // specified path.  The path may point to an image the following transports:
 // oci, oci-archive, dir, docker-archive.
+//
+// Load returns a string slice with names of recently loaded images.
+// If images are unnamed in the source, it returns a string slice of image IDs instead.
 func (r *Runtime) Load(ctx context.Context, path string, options *LoadOptions) ([]string, error) {
 	logrus.Debugf("Loading image from %q", path)
 
@@ -142,7 +145,8 @@ func (r *Runtime) loadMultiImageDockerArchive(ctx context.Context, ref types.Ima
 	// should.
 	path := ref.StringWithinTransport()
 	if err := fileutils.Exists(path); err != nil {
-		return r.copyFromDockerArchive(ctx, ref, options)
+		_, names, err := r.copyFromDockerArchive(ctx, ref, options)
+		return names, err
 	}
 
 	reader, err := dockerArchiveTransport.NewReader(r.systemContextCopy(), path)
@@ -163,7 +167,7 @@ func (r *Runtime) loadMultiImageDockerArchive(ctx context.Context, ref types.Ima
 	var copiedImages []string
 	for _, list := range refLists {
 		for _, listRef := range list {
-			names, err := r.copyFromDockerArchiveReaderReference(ctx, reader, listRef, options)
+			_, names, err := r.copyFromDockerArchiveReaderReference(ctx, reader, listRef, options)
 			if err != nil {
 				return nil, err
 			}