Improve image ID lookup for pulled images

- Use the image's repo, not just the digest, to be more precise
  when zstd:chunked ambiguities are involved
- Remove the multi-platform lookup code, it is never used

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This commit is contained in:
Miloslav Trmač 2024-10-15 22:36:14 +02:00
parent 7daccce4dc
commit d90a20404b
1 changed files with 24 additions and 39 deletions

View File

@ -25,7 +25,6 @@ import (
"github.com/containers/image/v5/transports/alltransports" "github.com/containers/image/v5/transports/alltransports"
"github.com/containers/image/v5/types" "github.com/containers/image/v5/types"
"github.com/containers/storage" "github.com/containers/storage"
digest "github.com/opencontainers/go-digest"
ociSpec "github.com/opencontainers/image-spec/specs-go/v1" ociSpec "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/sirupsen/logrus" "github.com/sirupsen/logrus"
) )
@ -457,52 +456,38 @@ func (r *Runtime) copyFromRegistry(ctx context.Context, ref types.ImageReference
return pulledIDs, nil return pulledIDs, nil
} }
// imageIDForManifest() parses the manifest of the copied image and then looks // imageIDForPulledImage makes a best-effort guess at an image ID for
// up the ID of the matching image. There's a small slice of time, between // a just-pulled image written to destName, where the pull returned manifestBytes
// when we copy the image into local storage and when we go to look for it func (r *Runtime) imageIDForPulledImage(destName reference.Named, manifestBytes []byte) (string, error) {
// using the name that we gave it when we copied it, when the name we wanted to // The caller, copySingleImageFromRegistry, never triggers a multi-platform copy, so manifestBytes
// assign to the image could have been moved, but the image's ID will remain // is always a single-platform manifest instance.
// the same until it is deleted. manifestDigest, err := manifest.Digest(manifestBytes)
func (r *Runtime) imageIDForManifest(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 "", fmt.Errorf("parsing manifest list: %w", err)
}
d, err := list.ChooseInstance(sys)
if err != nil {
return "", fmt.Errorf("choosing instance from manifest list: %w", err)
}
imageDigest = d
} else {
d, err := manifest.Digest(manifestBytes)
if err != nil {
return "", errors.New("digesting manifest")
}
imageDigest = d
}
images, err := r.store.ImagesByDigest(imageDigest)
if err != nil { if err != nil {
return "", fmt.Errorf("listing images by manifest digest: %w", err) return "", err
} }
destDigestedName, err := reference.WithDigest(reference.TrimNamed(destName), manifestDigest)
// If you have additionStores defined and the same image stored in if err != nil {
// both storage and additional store, it can be output twice. return "", err
// }
// Worse, with zstd:chunked partial pulls, the same image can have several 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 // different IDs, depending on which layers of the image were pulled using the
// partial pull (are identified by TOC, not by uncompressed digest). // partial pull (are identified by TOC, not by uncompressed digest).
// //
// At this point, from just the manifest digest, we cant tell which image // At this point, from just the manifest digest, we cant tell which image
// is the one that was actually pulled. (They should all have the same contents // is the one that was actually pulled. (They should all have the same contents
// unless the image author is malicious.) // unless the image author is malicious.)
// So just return the first matching image ID. //
if len(images) == 0 { // FIXME: To return an accurate value, c/image would need to return the image ID,
return "", fmt.Errorf("identifying new image by manifest digest: %w", storage.ErrImageUnknown) // not just manifestBytes.
_, image, err := storageTransport.ResolveReference(storeRef)
if err != nil {
return "", fmt.Errorf("looking up a just-pulled image: %w", err)
} }
return images[0].ID, nil return image.ID, nil
} }
// copySingleImageFromRegistry pulls the specified, possibly unqualified, name // copySingleImageFromRegistry pulls the specified, possibly unqualified, name
@ -702,7 +687,7 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str
} }
logrus.Debugf("Pulled candidate %s successfully", candidateString) logrus.Debugf("Pulled candidate %s successfully", candidateString)
ids, err := r.imageIDForManifest(manifestBytes, sys) ids, err := r.imageIDForPulledImage(candidate.Value, manifestBytes)
if err != nil { if err != nil {
return "", err return "", err
} }