diff --git a/libimage/copier.go b/libimage/copier.go index b2cd6cedb..b7fa7534d 100644 --- a/libimage/copier.go +++ b/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 { diff --git a/libimage/import.go b/libimage/import.go index a03f28853..ea366f775 100644 --- a/libimage/import.go +++ b/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 } diff --git a/libimage/manifest_list.go b/libimage/manifest_list.go index a23315da3..0618934c6 100644 --- a/libimage/manifest_list.go +++ b/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 } diff --git a/libimage/pull.go b/libimage/pull.go index ad4699c60..02b6a4233 100644 --- a/libimage/pull.go +++ b/libimage/pull.go @@ -17,11 +17,11 @@ 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" @@ -230,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 } @@ -386,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 } @@ -456,40 +456,6 @@ func (r *Runtime) copyFromRegistry(ctx context.Context, ref types.ImageReference return pulledIDs, nil } -// imageIDForPulledImage makes a best-effort guess at an image ID for -// a just-pulled image written to destName, where the pull returned manifestBytes -func (r *Runtime) imageIDForPulledImage(destName reference.Named, manifestBytes []byte) (string, error) { - // The caller, copySingleImageFromRegistry, never triggers a multi-platform copy, so manifestBytes - // is always a single-platform manifest instance. - manifestDigest, err := manifest.Digest(manifestBytes) - if err != nil { - return "", err - } - destDigestedName, err := reference.WithDigest(reference.TrimNamed(destName), manifestDigest) - if err != nil { - return "", err - } - storeRef, err := storageTransport.Transport.NewStoreReference(r.store, destDigestedName, "") - if err != nil { - return "", err - } - // With zstd:chunked partial pulls, the same image can have several - // different IDs, depending on which layers of the image were pulled using the - // partial pull (are identified by TOC, not by uncompressed digest). - // - // At this point, from just the manifest digest, we can’t tell which image - // is the one that was actually pulled. (They should all have the same contents - // unless the image author is malicious.) - // - // FIXME: To return an accurate value, c/image would need to return the image ID, - // not just manifestBytes. - _, image, err := storageTransport.ResolveReference(storeRef) - if err != nil { - return "", fmt.Errorf("looking up a just-pulled image: %w", err) - } - return image.ID, nil -} - // copySingleImageFromRegistry pulls the specified, possibly unqualified, name // from a registry. On successful pull it returns the ID of the image in local // storage (or, FIXME, a name/ID? that could be resolved in local storage) @@ -632,7 +598,8 @@ 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 "", err } @@ -673,8 +640,7 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str 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 @@ -687,11 +653,14 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str } logrus.Debugf("Pulled candidate %s successfully", candidateString) - ids, err := r.imageIDForPulledImage(candidate.Value, manifestBytes) + 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 "", err + return "", fmt.Errorf("resolving an already-resolved reference %q to the pulled image: %w", transports.ImageName(resolvedReference), err) } - return ids, nil + return image.ID, nil } if localImage != nil && pullPolicy == config.PullPolicyNewer { diff --git a/libimage/push.go b/libimage/push.go index 5db6cfbcf..dc9934480 100644 --- a/libimage/push.go +++ b/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 } diff --git a/libimage/save.go b/libimage/save.go index 46529d10f..c9cc8a507 100644 --- a/libimage/save.go +++ b/libimage/save.go @@ -119,7 +119,7 @@ func (r *Runtime) saveSingleImage(ctx context.Context, name, format, path string return err } - c, err := r.newCopier(&options.CopyOptions) + c, err := r.newCopier(&options.CopyOptions, nil) if err != nil { return err } @@ -204,7 +204,7 @@ func (r *Runtime) saveDockerArchive(ctx context.Context, names []string, path st copyOpts := options.CopyOptions copyOpts.dockerArchiveAdditionalTags = local.tags - c, err := r.newCopier(©Opts) + c, err := r.newCopier(©Opts, nil) if err != nil { return err }