DO NOT MERGE: Precisely determine the ID of a pulled image

go.mod

@@ -2,7 +2,7 @@ module github.com/containers/common
 
 // Warning: Ensure the "go" and "toolchain" versions match exactly to prevent unwanted auto-updates
 
-go 1.22.0
+go 1.22.6
 
 require (
 	github.com/BurntSushi/toml v1.4.0
@@ -54,7 +54,7 @@ require (
 )
 
 require (
-	dario.cat/mergo v1.0.0 // indirect
+	dario.cat/mergo v1.0.1 // indirect
 	github.com/Microsoft/go-winio v0.6.2 // indirect
 	github.com/Microsoft/hcsshim v0.12.7 // indirect
 	github.com/VividCortex/ewma v1.2.0 // indirect
@@ -71,7 +71,7 @@ require (
 	github.com/docker/docker-credential-helpers v0.8.2 // indirect
 	github.com/docker/go-connections v0.5.0 // indirect
 	github.com/felixge/httpsnoop v1.0.4 // indirect
-	github.com/go-jose/go-jose/v4 v4.0.2 // indirect
+	github.com/go-jose/go-jose/v4 v4.0.4 // indirect
 	github.com/go-logr/logr v1.4.2 // indirect
 	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/go-openapi/analysis v0.23.0 // indirect
@@ -89,22 +89,22 @@ require (
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/protobuf v1.5.4 // indirect
 	github.com/google/go-cmp v0.6.0 // indirect
-	github.com/google/go-containerregistry v0.20.1 // indirect
+	github.com/google/go-containerregistry v0.20.2 // indirect
 	github.com/google/go-intervals v0.0.2 // indirect
 	github.com/google/pprof v0.0.0-20240827171923-fa2c70bbbfe5 // indirect
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/gorilla/mux v1.8.1 // indirect
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/inconshreveable/mousetrap v1.1.0 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
-	github.com/klauspost/compress v1.17.10 // indirect
+	github.com/klauspost/compress v1.17.11 // indirect
 	github.com/klauspost/pgzip v1.2.6 // indirect
 	github.com/kr/fs v0.1.0 // indirect
-	github.com/letsencrypt/boulder v0.0.0-20240418210053-89b07f4543e0 // indirect
+	github.com/letsencrypt/boulder v0.0.0-20240620165639-de9c06129bec // indirect
 	github.com/mailru/easyjson v0.7.7 // indirect
 	github.com/manifoldco/promptui v0.9.0 // indirect
 	github.com/mattn/go-runewidth v0.0.16 // indirect
-	github.com/mattn/go-sqlite3 v1.14.22 // indirect
+	github.com/mattn/go-sqlite3 v1.14.24 // indirect
 	github.com/miekg/pkcs11 v1.1.1 // indirect
 	github.com/mistifyio/go-zfs/v3 v3.0.1 // indirect
 	github.com/mitchellh/mapstructure v1.5.0 // indirect
@@ -119,17 +119,17 @@ require (
 	github.com/proglottis/gpgme v0.1.3 // indirect
 	github.com/rivo/uniseg v0.4.7 // indirect
 	github.com/secure-systems-lab/go-securesystemslib v0.8.0 // indirect
-	github.com/sigstore/fulcio v1.4.5 // indirect
+	github.com/sigstore/fulcio v1.6.4 // indirect
 	github.com/sigstore/rekor v1.3.6 // indirect
-	github.com/sigstore/sigstore v1.8.4 // indirect
+	github.com/sigstore/sigstore v1.8.9 // indirect
 	github.com/stefanberger/go-pkcs11uri v0.0.0-20230803200340-78284954bff6 // indirect
-	github.com/sylabs/sif/v2 v2.18.0 // indirect
+	github.com/sylabs/sif/v2 v2.19.1 // indirect
 	github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635 // indirect
 	github.com/tchap/go-patricia/v2 v2.3.1 // indirect
 	github.com/titanous/rocacheck v0.0.0-20171023193734-afe73141d399 // indirect
 	github.com/ulikunitz/xz v0.5.12 // indirect
 	github.com/vbatts/tar-split v0.11.6 // indirect
-	github.com/vbauerster/mpb/v8 v8.7.5 // indirect
+	github.com/vbauerster/mpb/v8 v8.8.3 // indirect
 	github.com/vishvananda/netns v0.0.4 // indirect
 	go.mongodb.org/mongo-driver v1.14.0 // indirect
 	go.mozilla.org/pkcs7 v0.0.0-20210826202110-33d05740a352 // indirect
@@ -141,7 +141,7 @@ require (
 	golang.org/x/net v0.29.0 // indirect
 	golang.org/x/text v0.19.0 // indirect
 	golang.org/x/tools v0.25.0 // indirect
-	google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094 // indirect
+	google.golang.org/genproto/googleapis/rpc v0.0.0-20240823204242-4ba0660f739c // indirect
 	google.golang.org/grpc v1.66.0 // indirect
 	google.golang.org/protobuf v1.34.2 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
@@ -151,3 +151,5 @@ retract (
 	v1.0.1 // This version is used only to publish retraction of v1.0.1.
 	v1.0.0 // We reverted to v0.… version numbers; the v1.0.0 tag was actually deleted.
 )
+
+replace github.com/containers/image/v5 => github.com/mtrmac/image/v5 v5.0.0-20241021121719-f630065724b8

libimage/copier.go

@@ -175,8 +175,8 @@ type Copier struct {
 // newCopier creates a Copier based on a runtime's system context.
 // Note that fields in options *may* overwrite the counterparts of
 // the specified system context.  Please make sure to call `(*Copier).Close()`.
-func (r *Runtime) newCopier(options *CopyOptions) (*Copier, error) {
-	return NewCopier(options, r.SystemContext())
+func (r *Runtime) newCopier(options *CopyOptions, reportResolvedReference *types.ImageReference) (*Copier, error) {
+	return NewCopier(options, r.SystemContext(), reportResolvedReference)
 }
 
 // storageAllowedPolicyScopes overrides the policy for local storage
@@ -223,7 +223,7 @@ func getDockerAuthConfig(name, passwd, creds, idToken string) (*types.DockerAuth
 // NewCopier creates a Copier based on a provided system context.
 // Note that fields in options *may* overwrite the counterparts of
 // the specified system context.  Please make sure to call `(*Copier).Close()`.
-func NewCopier(options *CopyOptions, sc *types.SystemContext) (*Copier, error) {
+func NewCopier(options *CopyOptions, sc *types.SystemContext, reportResolvedReference *types.ImageReference) (*Copier, error) {
 	c := Copier{extendTimeoutSocket: options.extendTimeoutSocket}
 	sysContextCopy := *sc
 	c.systemContext = &sysContextCopy
@@ -330,6 +330,7 @@ func NewCopier(options *CopyOptions, sc *types.SystemContext) (*Copier, error) {
 	c.imageCopyOptions.SignBySigstorePrivateKeyFile = options.SignBySigstorePrivateKeyFile
 	c.imageCopyOptions.SignSigstorePrivateKeyPassphrase = options.SignSigstorePrivateKeyPassphrase
 	c.imageCopyOptions.ReportWriter = options.Writer
+	c.imageCopyOptions.ReportResolvedReference = reportResolvedReference
 
 	defaultContainerConfig, err := config.Default()
 	if err != nil {

libimage/import.go

@@ -104,7 +104,7 @@ func (r *Runtime) Import(ctx context.Context, path string, options *ImportOption
 		return "", err
 	}
 
-	c, err := r.newCopier(&options.CopyOptions)
+	c, err := r.newCopier(&options.CopyOptions, nil)
 	if err != nil {
 		return "", err
 	}

libimage/manifest_list.go

@@ -792,7 +792,7 @@ func (m *ManifestList) Push(ctx context.Context, destination string, options *Ma
 	// NOTE: we're using the logic in copier to create a proper
 	// types.SystemContext. This prevents us from having an error prone
 	// code duplicate here.
-	copier, err := m.image.runtime.newCopier(&options.CopyOptions)
+	copier, err := m.image.runtime.newCopier(&options.CopyOptions, nil)
 	if err != nil {
 		return "", err
 	}

libimage/pull.go

@@ -17,15 +17,14 @@ import (
 	dockerArchiveTransport "github.com/containers/image/v5/docker/archive"
 	dockerDaemonTransport "github.com/containers/image/v5/docker/daemon"
 	"github.com/containers/image/v5/docker/reference"
-	"github.com/containers/image/v5/manifest"
 	ociArchiveTransport "github.com/containers/image/v5/oci/archive"
 	ociTransport "github.com/containers/image/v5/oci/layout"
 	"github.com/containers/image/v5/pkg/shortnames"
 	storageTransport "github.com/containers/image/v5/storage"
+	"github.com/containers/image/v5/transports"
 	"github.com/containers/image/v5/transports/alltransports"
 	"github.com/containers/image/v5/types"
 	"github.com/containers/storage"
-	digest "github.com/opencontainers/go-digest"
 	ociSpec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/sirupsen/logrus"
 )
@@ -231,7 +230,7 @@ func nameFromAnnotations(annotations map[string]string) string {
 // copyFromDefault is the default copier for a number of transports.  Other
 // transports require some specific dancing, sometimes Yoga.
 func (r *Runtime) copyFromDefault(ctx context.Context, ref types.ImageReference, options *CopyOptions) ([]string, error) {
-	c, err := r.newCopier(options)
+	c, err := r.newCopier(options, nil)
 	if err != nil {
 		return nil, err
 	}
@@ -387,7 +386,7 @@ func (r *Runtime) copyFromDockerArchive(ctx context.Context, ref types.ImageRefe
 
 // copyFromDockerArchiveReaderReference copies the specified readerRef from reader.
 func (r *Runtime) copyFromDockerArchiveReaderReference(ctx context.Context, reader *dockerArchiveTransport.Reader, readerRef types.ImageReference, options *CopyOptions) ([]string, error) {
-	c, err := r.newCopier(options)
+	c, err := r.newCopier(options, nil)
 	if err != nil {
 		return nil, err
 	}
@@ -421,7 +420,11 @@ func (r *Runtime) copyFromRegistry(ctx context.Context, ref types.ImageReference
 	}
 
 	if !options.AllTags {
-		return r.copySingleImageFromRegistry(ctx, inputName, pullPolicy, options)
+		pulled, err := r.copySingleImageFromRegistry(ctx, inputName, pullPolicy, options)
+		if err != nil {
+			return nil, err
+		}
+		return []string{pulled}, nil
 	}
 
 	// Copy all tags
@@ -447,68 +450,19 @@ func (r *Runtime) copyFromRegistry(ctx context.Context, ref types.ImageReference
 		if err != nil {
 			return nil, err
 		}
-		pulledIDs = append(pulledIDs, pulled...)
+		pulledIDs = append(pulledIDs, pulled)
 	}
 
 	return pulledIDs, nil
 }
 
-// imageIDsForManifest() parses the manifest of the copied image and then looks
-// up the IDs of the matching image.  There's a small slice of time, between
-// when we copy the image into local storage and when we go to look for it
-// using the name that we gave it when we copied it, when the name we wanted to
-// assign to the image could have been moved, but the image's ID will remain
-// the same until it is deleted.
-func (r *Runtime) imagesIDsForManifest(manifestBytes []byte, sys *types.SystemContext) ([]string, error) {
-	var imageDigest digest.Digest
-	manifestType := manifest.GuessMIMEType(manifestBytes)
-	if manifest.MIMETypeIsMultiImage(manifestType) {
-		list, err := manifest.ListFromBlob(manifestBytes, manifestType)
-		if err != nil {
-			return nil, fmt.Errorf("parsing manifest list: %w", err)
-		}
-		d, err := list.ChooseInstance(sys)
-		if err != nil {
-			return nil, fmt.Errorf("choosing instance from manifest list: %w", err)
-		}
-		imageDigest = d
-	} else {
-		d, err := manifest.Digest(manifestBytes)
-		if err != nil {
-			return nil, errors.New("digesting manifest")
-		}
-		imageDigest = d
-	}
-	images, err := r.store.ImagesByDigest(imageDigest)
-	if err != nil {
-		return nil, fmt.Errorf("listing images by manifest digest: %w", err)
-	}
-
-	// If you have additionStores defined and the same image stored in
-	// both storage and additional store, it can be output twice.
-	// Fixes github.com/containers/podman/issues/18647
-	results := []string{}
-	imageMap := map[string]bool{}
-	for _, image := range images {
-		if imageMap[image.ID] {
-			continue
-		}
-		imageMap[image.ID] = true
-		results = append(results, image.ID)
-	}
-	if len(results) == 0 {
-		return nil, fmt.Errorf("identifying new image by manifest digest: %w", storage.ErrImageUnknown)
-	}
-	return results, nil
-}
-
 // copySingleImageFromRegistry pulls the specified, possibly unqualified, name
 // from a registry.  On successful pull it returns the ID of the image in local
-// storage.
-func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName string, pullPolicy config.PullPolicy, options *PullOptions) ([]string, error) { //nolint:gocyclo
+// storage (or, FIXME, a name/ID? that could be resolved in local storage)
+func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName string, pullPolicy config.PullPolicy, options *PullOptions) (string, error) { //nolint:gocyclo
 	// Sanity check.
 	if err := pullPolicy.Validate(); err != nil {
-		return nil, err
+		return "", err
 	}
 
 	var (
@@ -533,6 +487,14 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str
 	if options.OS != runtime.GOOS {
 		lookupImageOptions.OS = options.OS
 	}
+	// FIXME: We sometimes return resolvedImageName from this function.
+	// The function documentation says this returns an image ID, resolvedImageName is frequently not an image ID.
+	//
+	// Ultimately Runtime.Pull looks up the returned name... again, possibly finding some other match
+	// than we did.
+	//
+	// This should be restructured so that the image we found here is returned to the caller of Pull
+	// directly, without another image -> name -> image round-trip and possible inconsistency.
 	localImage, resolvedImageName, err = r.LookupImage(imageName, lookupImageOptions)
 	if err != nil && !errors.Is(err, storage.ErrImageUnknown) {
 		logrus.Errorf("Looking up %s in local storage: %v", imageName, err)
@@ -563,23 +525,23 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str
 	if pullPolicy == config.PullPolicyNever {
 		if localImage != nil {
 			logrus.Debugf("Pull policy %q and %s resolved to local image %s", pullPolicy, imageName, resolvedImageName)
-			return []string{resolvedImageName}, nil
+			return resolvedImageName, nil
 		}
 		logrus.Debugf("Pull policy %q but no local image has been found for %s", pullPolicy, imageName)
-		return nil, fmt.Errorf("%s: %w", imageName, storage.ErrImageUnknown)
+		return "", fmt.Errorf("%s: %w", imageName, storage.ErrImageUnknown)
 	}
 
 	if pullPolicy == config.PullPolicyMissing && localImage != nil {
-		return []string{resolvedImageName}, nil
+		return resolvedImageName, nil
 	}
 
 	// If we looked up the image by ID, we cannot really pull from anywhere.
 	if localImage != nil && strings.HasPrefix(localImage.ID(), imageName) {
 		switch pullPolicy {
 		case config.PullPolicyAlways:
-			return nil, fmt.Errorf("pull policy is always but image has been referred to by ID (%s)", imageName)
+			return "", fmt.Errorf("pull policy is always but image has been referred to by ID (%s)", imageName)
 		default:
-			return []string{resolvedImageName}, nil
+			return resolvedImageName, nil
 		}
 	}
 
@@ -604,9 +566,9 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str
 	resolved, err := shortnames.Resolve(sys, imageName)
 	if err != nil {
 		if localImage != nil && pullPolicy == config.PullPolicyNewer {
-			return []string{resolvedImageName}, nil
+			return resolvedImageName, nil
 		}
-		return nil, err
+		return "", err
 	}
 
 	// NOTE: Below we print the description from the short-name resolution.
@@ -636,9 +598,10 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str
 	if socketPath, ok := os.LookupEnv("NOTIFY_SOCKET"); ok {
 		options.extendTimeoutSocket = socketPath
 	}
-	c, err := r.newCopier(&options.CopyOptions)
+	var resolvedReference types.ImageReference
+	c, err := r.newCopier(&options.CopyOptions, &resolvedReference)
 	if err != nil {
-		return nil, err
+		return "", err
 	}
 	defer c.Close()
 
@@ -648,7 +611,7 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str
 		logrus.Debugf("Attempting to pull candidate %s for %s", candidateString, imageName)
 		srcRef, err := registryTransport.NewReference(candidate.Value)
 		if err != nil {
-			return nil, err
+			return "", err
 		}
 
 		if pullPolicy == config.PullPolicyNewer && localImage != nil {
@@ -666,19 +629,18 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str
 
 		destRef, err := storageTransport.Transport.ParseStoreReference(r.store, candidate.Value.String())
 		if err != nil {
-			return nil, err
+			return "", err
 		}
 
 		if err := writeDesc(); err != nil {
-			return nil, err
+			return "", err
 		}
 		if options.Writer != nil {
 			if _, err := io.WriteString(options.Writer, fmt.Sprintf("Trying to pull %s...\n", candidateString)); err != nil {
-				return nil, err
+				return "", err
 			}
 		}
-		var manifestBytes []byte
-		if manifestBytes, err = c.Copy(ctx, srcRef, destRef); err != nil {
+		if _, err := c.Copy(ctx, srcRef, destRef); err != nil {
 			logrus.Debugf("Error pulling candidate %s: %v", candidateString, err)
 			pullErrors = append(pullErrors, err)
 			continue
@@ -691,19 +653,23 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str
 		}
 
 		logrus.Debugf("Pulled candidate %s successfully", candidateString)
-		if ids, err := r.imagesIDsForManifest(manifestBytes, sys); err == nil {
-			return ids, nil
+		if resolvedReference == nil { // resolvedReference should always be set for storageTransport destinations
+			return "", fmt.Errorf("internal error: After pulling %s, resolvedReference is nil", candidateString)
+		}
+		_, image, err := storageTransport.ResolveReference(resolvedReference)
+		if err != nil {
+			return "", fmt.Errorf("resolving an already-resolved reference %q to the pulled image: %w", transports.ImageName(resolvedReference), err)
 		}
-		return []string{candidate.Value.String()}, nil
+		return image.ID, nil
 	}
 
 	if localImage != nil && pullPolicy == config.PullPolicyNewer {
-		return []string{resolvedImageName}, nil
+		return resolvedImageName, nil
 	}
 
 	if len(pullErrors) == 0 {
-		return nil, fmt.Errorf("internal error: no image pulled (pull policy %s)", pullPolicy)
+		return "", fmt.Errorf("internal error: no image pulled (pull policy %s)", pullPolicy)
 	}
 
-	return nil, resolved.FormatPullErrors(pullErrors)
+	return "", resolved.FormatPullErrors(pullErrors)
 }

libimage/push.go

@@ -109,7 +109,7 @@ func (r *Runtime) Push(ctx context.Context, source, destination string, options
 		}
 	}
 
-	c, err := r.newCopier(&options.CopyOptions)
+	c, err := r.newCopier(&options.CopyOptions, nil)
 	if err != nil {
 		return nil, err
 	}