Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Beautify lookup after #1505 #1559

Merged
merged 9 commits into from
Jul 14, 2023
19 changes: 13 additions & 6 deletions libimage/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions libimage/filters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"time"

"github.com/containers/common/pkg/config"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -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) {
Expand Down
25 changes: 11 additions & 14 deletions libimage/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
}
}
Expand Down
34 changes: 13 additions & 21 deletions libimage/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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
}
Expand All @@ -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)
Expand Down