diff --git a/libimage/filters.go b/libimage/filters.go index ff50321b7..995f89c78 100644 --- a/libimage/filters.go +++ b/libimage/filters.go @@ -394,10 +394,12 @@ func filterID(value string) filterFunc { } } -// filterDigest creates an digest filter for matching the specified value. +// filterDigest creates a digest filter for matching the specified value. func filterDigest(value string) filterFunc { + // TODO: return an error if value is not a digest + // if _, err := digest.Parse(value); err != nil {...} return func(img *Image) (bool, error) { - return string(img.Digest()) == value, nil + return img.hasDigest(value), nil } } diff --git a/libimage/filters_test.go b/libimage/filters_test.go index 12091b130..94e513596 100644 --- a/libimage/filters_test.go +++ b/libimage/filters_test.go @@ -63,6 +63,9 @@ func TestFilterReference(t *testing.T) { {"busybox", 1}, {"alpine", 1}, {"alpine@" + alpine.Digest().String(), 1}, + {"alpine:latest@" + alpine.Digest().String(), 1}, + {"quay.io/libpod/alpine@" + alpine.Digest().String(), 1}, + {"quay.io/libpod/alpine:latest@" + alpine.Digest().String(), 1}, } { listOptions := &ListImagesOptions{ Filters: []string{"reference=" + test.filter}, diff --git a/libimage/image.go b/libimage/image.go index da4ff8b7a..d8ffb2e1a 100644 --- a/libimage/image.go +++ b/libimage/image.go @@ -144,6 +144,9 @@ func (i *Image) ID() string { // possibly many digests that we have stored for the image, so many // applications are better off using the entire list returned by Digests(). func (i *Image) Digest() digest.Digest { + // TODO: we return the image digest or the one of the manifest list + // which can lead to issues depending on the callers' assumptions. + // Hence, deprecate in favor of Digest_s_. return i.storageImage.Digest } @@ -154,6 +157,18 @@ func (i *Image) Digests() []digest.Digest { return i.storageImage.Digests } +// hasDigest returns whether the specified value matches any digest of the +// image. +func (i *Image) hasDigest(value string) bool { + // TODO: change the argument to a typed digest.Digest + for _, d := range i.Digests() { + if string(d) == value { + return true + } + } + return false +} + // IsReadOnly returns whether the image is set read only. func (i *Image) IsReadOnly() bool { return i.storageImage.ReadOnly diff --git a/libimage/normalize.go b/libimage/normalize.go index bb3cdbc7c..9619b1a0d 100644 --- a/libimage/normalize.go +++ b/libimage/normalize.go @@ -100,22 +100,22 @@ func ToNameTagPairs(repoTags []reference.Named) ([]NameTagPair, error) { // normalizeTaggedDigestedString strips the tag off the specified string iff it // is tagged and digested. Note that the tag is entirely ignored to match // Docker behavior. -func normalizeTaggedDigestedString(s string) (string, error) { +func normalizeTaggedDigestedString(s string) (string, reference.Named, error) { // Note that the input string is not expected to be parseable, so we // return it verbatim in error cases. ref, err := reference.Parse(s) if err != nil { - return "", err + return "", nil, err } named, ok := ref.(reference.Named) if !ok { - return s, nil + return s, nil, nil } named, err = normalizeTaggedDigestedNamed(named) if err != nil { - return "", err + return "", nil, err } - return named.String(), nil + return named.String(), named, nil } // normalizeTaggedDigestedNamed strips the tag off the specified named diff --git a/libimage/normalize_test.go b/libimage/normalize_test.go index 206c0eb35..9b9dfb464 100644 --- a/libimage/normalize_test.go +++ b/libimage/normalize_test.go @@ -132,12 +132,14 @@ func TestNormalizeTaggedDigestedString(t *testing.T) { {"localhost/fedora:anothertag" + digestSuffix, "localhost/fedora" + digestSuffix}, {"localhost:5000/fedora:v1.2.3.4.5" + digestSuffix, "localhost:5000/fedora" + digestSuffix}, } { - res, err := normalizeTaggedDigestedString(test.input) + res, named, err := normalizeTaggedDigestedString(test.input) if test.expected == "" { assert.Error(t, err, "%v", test) } else { assert.NoError(t, err, "%v", test) assert.Equal(t, test.expected, res, "%v", test) + assert.NotNil(t, named, "%v", test) + assert.Equal(t, res, named.String(), "%v", test) } } } diff --git a/libimage/pull.go b/libimage/pull.go index 188ecb5ef..296d003a8 100644 --- a/libimage/pull.go +++ b/libimage/pull.go @@ -86,7 +86,7 @@ func (r *Runtime) Pull(ctx context.Context, name string, pullPolicy config.PullP // Docker compat: strip off the tag iff name is tagged and digested // (e.g., fedora:latest@sha256...). In that case, the tag is stripped // off and entirely ignored. The digest is the sole source of truth. - normalizedName, normalizeError := normalizeTaggedDigestedString(name) + normalizedName, _, normalizeError := normalizeTaggedDigestedString(name) if normalizeError != nil { return nil, normalizeError } diff --git a/libimage/runtime.go b/libimage/runtime.go index 95da83bb9..0d39d77ff 100644 --- a/libimage/runtime.go +++ b/libimage/runtime.go @@ -239,7 +239,7 @@ func (r *Runtime) LookupImage(name string, options *LookupImageOptions) (*Image, // Docker compat: strip off the tag iff name is tagged and digested // (e.g., fedora:latest@sha256...). In that case, the tag is stripped // off and entirely ignored. The digest is the sole source of truth. - normalizedName, err := normalizeTaggedDigestedString(name) + normalizedName, possiblyUnqualifiedNamedReference, err := normalizeTaggedDigestedString(name) if err != nil { return nil, "", err } @@ -259,7 +259,7 @@ func (r *Runtime) LookupImage(name string, options *LookupImageOptions) (*Image, // If the name clearly refers to a local image, try to look it up. if byFullID || byDigest { - img, err := r.lookupImageInLocalStorage(originalName, name, options) + img, err := r.lookupImageInLocalStorage(originalName, name, nil, options) if err != nil { return nil, "", err } @@ -297,7 +297,7 @@ func (r *Runtime) LookupImage(name string, options *LookupImageOptions) (*Image, } for _, candidate := range candidates { - img, err := r.lookupImageInLocalStorage(name, candidate.String(), options) + img, err := r.lookupImageInLocalStorage(name, candidate.String(), candidate, options) if err != nil { return nil, "", err } @@ -308,7 +308,7 @@ func (r *Runtime) LookupImage(name string, options *LookupImageOptions) (*Image, // The specified name may refer to a short ID. Note that this *must* // happen after the short-name expansion as done above. - img, err := r.lookupImageInLocalStorage(name, name, options) + img, err := r.lookupImageInLocalStorage(name, name, nil, options) if err != nil { return nil, "", err } @@ -316,21 +316,45 @@ func (r *Runtime) LookupImage(name string, options *LookupImageOptions) (*Image, return img, name, err } - return r.lookupImageInDigestsAndRepoTags(name, options) + return r.lookupImageInRepoTags(name, possiblyUnqualifiedNamedReference, options) } // lookupImageInLocalStorage looks up the specified candidate for name in the // storage and checks whether it's matching the system context. -func (r *Runtime) lookupImageInLocalStorage(name, candidate string, options *LookupImageOptions) (*Image, error) { +func (r *Runtime) lookupImageInLocalStorage(name, candidate string, namedCandidate reference.Named, options *LookupImageOptions) (*Image, error) { logrus.Debugf("Trying %q ...", candidate) - img, err := r.store.Image(candidate) - if err != nil && !errors.Is(err, storage.ErrImageUnknown) { - return nil, err - } - if img == nil { - return nil, nil + + var err error + var img *storage.Image + var ref types.ImageReference + + // for images pulled by tag, Image.Names does not currently contain a + // repo@digest value, so such an input would not match. + if namedCandidate != nil { + ref, err = storageTransport.Transport.NewStoreReference(r.store, namedCandidate, "") + if err != nil { + return nil, err + } + img, err = storageTransport.Transport.GetStoreImage(r.store, ref) + if err != nil { + if errors.Is(err, storage.ErrImageUnknown) { + return nil, nil + } + return nil, err + } + // NOTE: we must reparse the reference another time below since + // an ordinary image may have been referenced via its parent + // (manifest list) digest. + } else { + img, err = r.store.Image(candidate) + if err != nil { + if errors.Is(err, storage.ErrImageUnknown) { + return nil, nil + } + return nil, err + } } - ref, err := storageTransport.Transport.ParseStoreReference(r.store, img.ID) + ref, err = storageTransport.Transport.ParseStoreReference(r.store, img.ID) if err != nil { return nil, err } @@ -352,6 +376,9 @@ func (r *Runtime) lookupImageInLocalStorage(name, candidate string, options *Loo } if options.lookupManifest || options.ManifestList { if isManifestList { + if _, err := image.ToManifestList(); err != nil { + return nil, err + } return image, nil } // return ErrNotAManifestList if lookupManifest is set otherwise try resolving image. @@ -414,59 +441,40 @@ func (r *Runtime) lookupImageInLocalStorage(name, candidate string, options *Loo return image, nil } -// lookupImageInDigestsAndRepoTags attempts to match name against any image in -// the local containers storage. If name is digested, it will be compared -// against image digests. Otherwise, it will be looked up in the repo tags. -func (r *Runtime) lookupImageInDigestsAndRepoTags(name string, options *LookupImageOptions) (*Image, string, error) { - // Until now, we've tried very hard to find an image but now it is time - // for limbo. If the image includes a digest that we couldn't detect - // verbatim in the storage, we must have a look at all digests of all - // images. Those may change over time (e.g., via manifest lists). - // Both Podman and Buildah want us to do that dance. - allImages, err := r.ListImages(context.Background(), nil, nil) - if err != nil { - return nil, "", err - } +// lookupImageInRepoTags attempts to match named against any image in the local +// containers storage by comparing the name to all known repo tags. +func (r *Runtime) lookupImageInRepoTags(name string, possiblyUnqualifiedNamedReference reference.Named, options *LookupImageOptions) (*Image, string, error) { + originalName := name // we may change name below - ref, err := reference.Parse(name) // Warning! This is not ParseNormalizedNamed - if err != nil { - return nil, "", err - } - named, isNamed := ref.(reference.Named) - if !isNamed { - return nil, "", fmt.Errorf("%s: %w", name, storage.ErrImageUnknown) + if possiblyUnqualifiedNamedReference == nil { + return nil, "", fmt.Errorf("%s: %w", originalName, storage.ErrImageUnknown) } - digested, isDigested := named.(reference.Digested) + // In case of a digested reference, we strip off the digest and require + // any image matching the repo/tag to also match the specified digest. + var digestStr string + digested, isDigested := possiblyUnqualifiedNamedReference.(reference.Digested) if isDigested { - logrus.Debug("Looking for image with matching recorded digests") - digest := digested.Digest() - for _, image := range allImages { - for _, d := range image.Digests() { - if d != digest { - continue - } - // Also make sure that the matching image fits all criteria (e.g., manifest list). - if _, err := r.lookupImageInLocalStorage(name, image.ID(), options); err != nil { - return nil, "", err - } - return image, name, nil - - } - } - return nil, "", fmt.Errorf("%s: %w", name, storage.ErrImageUnknown) + digestStr = digested.Digest().String() + possiblyUnqualifiedNamedReference = reference.TrimNamed(possiblyUnqualifiedNamedReference) + name = possiblyUnqualifiedNamedReference.String() } if !shortnames.IsShortName(name) { - return nil, "", fmt.Errorf("%s: %w", name, storage.ErrImageUnknown) + return nil, "", fmt.Errorf("%s: %w", originalName, storage.ErrImageUnknown) } - named = reference.TagNameOnly(named) // Make sure to add ":latest" if needed - namedTagged, isNammedTagged := named.(reference.NamedTagged) - if !isNammedTagged { - // NOTE: this should never happen since we already know it's - // not a digested reference. - return nil, "", fmt.Errorf("%s: %w (could not cast to tagged)", name, storage.ErrImageUnknown) + possiblyUnqualifiedNamedReference = reference.TagNameOnly(possiblyUnqualifiedNamedReference) // Docker compat: make sure to add ":latest" if needed + namedTagged, isNamedTagged := possiblyUnqualifiedNamedReference.(reference.NamedTagged) + if !isNamedTagged { + // NOTE: this should never happen since we already stripped off + // the digest. + return nil, "", fmt.Errorf("%s: %w (could not cast to tagged)", originalName, storage.ErrImageUnknown) + } + + allImages, err := r.ListImages(context.Background(), nil, nil) + if err != nil { + return nil, "", err } for _, image := range allImages { @@ -477,16 +485,19 @@ func (r *Runtime) lookupImageInDigestsAndRepoTags(name string, options *LookupIm if named == nil { continue } - img, err := r.lookupImageInLocalStorage(name, named.String(), options) + img, err := r.lookupImageInLocalStorage(name, named.String(), named, options) if err != nil { return nil, "", err } if img != nil { - return img, named.String(), err + if isDigested && !img.hasDigest(digestStr) { + continue + } + return img, named.String(), nil } } - return nil, "", fmt.Errorf("%s: %w", name, storage.ErrImageUnknown) + return nil, "", fmt.Errorf("%s: %w", originalName, storage.ErrImageUnknown) } // ResolveName resolves the specified name. If the name resolves to a local