Merge pull request #1559 from mtrmac/digests-nits

Beautify lookup after #1505

libimage/filters.go

@@ -11,6 +11,7 @@ import (
  filtersPkg "github.com/containers/common/pkg/filters"
  "github.com/containers/common/pkg/timetype"
  "github.com/containers/image/v5/docker/reference"
+ "github.com/opencontainers/go-digest"
  "github.com/sirupsen/logrus"
 )
 
@@ -147,7 +148,11 @@ func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOp
  filter = filterID(value)
 
  case "digest":
- filter = filterDigest(value)
+ f, err := filterDigest(value)
+ if err != nil {
+ return nil, err
+ }
+ filter = f
 
  case "intermediate":
  intermediate, err := r.bool(duplicate, key, value)
@@ -395,12 +400,14 @@ func filterID(value string) filterFunc {
 }
 
 // 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 img.hasDigest(value), nil
+func filterDigest(value string) (filterFunc, error) {
+ d, err := digest.Parse(value)
+ if err != nil {
+ return nil, fmt.Errorf("invalid value %q for digest filter: %w", value, err)
  }
+ return func(img *Image) (bool, error) {
+ return img.hasDigest(d), nil
+ }, nil
 }
 
 // filterIntermediate creates an intermediate filter for images. An image is

libimage/filters_test.go

@@ -7,6 +7,7 @@ import (
  "time"
 
  "github.com/containers/common/pkg/config"
+ "github.com/stretchr/testify/assert"
  "github.com/stretchr/testify/require"
 )
 
@@ -123,6 +124,10 @@ func TestFilterDigest(t *testing.T) {
  require.Len(t, listedImages, test.matches, "%s -> %v", test.filter, listedImages)
  require.Equal(t, listedImages[0].ID(), test.id)
  }
+ _, err = runtime.ListImages(ctx, nil, &ListImagesOptions{
+ Filters: []string{"digest=this-is-not-a-digest"},
+ })
+ assert.Error(t, err)
 }
 
 func TestFilterManifest(t *testing.T) {

libimage/image.go

@@ -159,10 +159,9 @@ func (i *Image) Digests() []digest.Digest {
 
 // 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
+func (i *Image) hasDigest(wantedDigest digest.Digest) bool {
  for _, d := range i.Digests() {
- if string(d) == value {
+ if d == wantedDigest {
  return true
  }
  }
@@ -686,24 +685,22 @@ func (i *Image) NamedRepoTags() ([]reference.Named, error) {
  return repoTags, nil
 }
 
-// inRepoTags looks for the specified name/tag in the image's repo tags. If
-// `ignoreTag` is set, only the repo must match and the tag is ignored.
-func (i *Image) inRepoTags(namedTagged reference.NamedTagged, ignoreTag bool) (reference.Named, error) {
+// referenceFuzzilyMatchingRepoAndTag checks if the image’s repo (and tag if requiredTag != "") matches a fuzzy short input,
+// and if so, returns the matching reference.
+//
+// DO NOT ADD ANY NEW USERS OF THIS SEMANTICS. Rely on existing libimage calls like LookupImage instead,
+// and handle unqualified the way it does (c/image/pkg/shortnames).
+func (i *Image) referenceFuzzilyMatchingRepoAndTag(requiredRepo reference.Named, requiredTag string) (reference.Named, error) {
  repoTags, err := i.NamedRepoTags()
  if err != nil {
  return nil, err
  }
 
- name := namedTagged.Name()
- tag := namedTagged.Tag()
+ name := requiredRepo.Name()
  for _, r := range repoTags {
- if !ignoreTag {
- var repoTag string
+ if requiredTag != "" {
  tagged, isTagged := r.(reference.NamedTagged)
- if isTagged {
- repoTag = tagged.Tag()
- }
- if !isTagged || tag != repoTag {
+ if !isTagged || tagged.Tag() != requiredTag {
  continue
  }
  }

libimage/runtime.go

@@ -454,28 +454,20 @@ func (r *Runtime) lookupImageInDigestsAndRepoTags(name string, possiblyUnqualifi
  if possiblyUnqualifiedNamedReference == nil {
  return nil, "", fmt.Errorf("%s: %w", originalName, storage.ErrImageUnknown)
  }
-
- // 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 requiredDigest digest.Digest
- digested, isDigested := possiblyUnqualifiedNamedReference.(reference.Digested)
- if isDigested {
- requiredDigest = digested.Digest()
- possiblyUnqualifiedNamedReference = reference.TrimNamed(possiblyUnqualifiedNamedReference)
- name = possiblyUnqualifiedNamedReference.String()
- }
-
  if !shortnames.IsShortName(name) {
  return nil, "", fmt.Errorf("%s: %w", originalName, storage.ErrImageUnknown)
  }
 
- // Docker compat: make sure to add the "latest" tag if needed. The tag
- // will be ignored if we're looking for a digest match.
- possiblyUnqualifiedNamedReference = reference.TagNameOnly(possiblyUnqualifiedNamedReference)
- namedTagged, isNamedTagged := possiblyUnqualifiedNamedReference.(reference.NamedTagged)
- if !isNamedTagged {
- // NOTE: this should never happen since we already stripped off
- // the digest.
+ var requiredDigest digest.Digest // or ""
+ var requiredTag string // or ""
+
+ possiblyUnqualifiedNamedReference = reference.TagNameOnly(possiblyUnqualifiedNamedReference) // Docker compat: make sure to add the "latest" tag if needed.
+ if digested, ok := possiblyUnqualifiedNamedReference.(reference.Digested); ok {
+ requiredDigest = digested.Digest()
+ name = reference.TrimNamed(possiblyUnqualifiedNamedReference).String()
+ } else if namedTagged, ok := possiblyUnqualifiedNamedReference.(reference.NamedTagged); ok {
+ requiredTag = namedTagged.Tag()
+ } else { // This should never happen after the reference.TagNameOnly above.
  return nil, "", fmt.Errorf("%s: %w (could not cast to tagged)", originalName, storage.ErrImageUnknown)
  }
 
@@ -485,7 +477,7 @@ func (r *Runtime) lookupImageInDigestsAndRepoTags(name string, possiblyUnqualifi
  }
 
  for _, image := range allImages {
- named, err := image.inRepoTags(namedTagged, isDigested)
+ named, err := image.referenceFuzzilyMatchingRepoAndTag(possiblyUnqualifiedNamedReference, requiredTag)
  if err != nil {
  return nil, "", err
  }
@@ -497,8 +489,8 @@ func (r *Runtime) lookupImageInDigestsAndRepoTags(name string, possiblyUnqualifi
  return nil, "", err
  }
  if img != nil {
- if isDigested {
- if !img.hasDigest(requiredDigest.String()) {
+ if requiredDigest != "" {
+ if !img.hasDigest(requiredDigest) {
  continue
  }
  named = reference.TrimNamed(named)