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

libimage: enforce "latest" tag when looking up images #836

Merged
merged 1 commit into from
Nov 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions libimage/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -624,8 +624,7 @@ func (i *Image) NamedRepoTags() ([]reference.Named, error) {
}

// inRepoTags looks for the specified name/tag pair in the image's repo tags.
// Note that tag may be empty.
func (i *Image) inRepoTags(name, tag string) (reference.Named, error) {
func (i *Image) inRepoTags(namedTagged reference.NamedTagged) (reference.Named, error) {
repoTags, err := i.NamedRepoTags()
if err != nil {
return nil, err
Expand All @@ -636,8 +635,10 @@ func (i *Image) inRepoTags(name, tag string) (reference.Named, error) {
return nil, err
}

name := namedTagged.Name()
tag := namedTagged.Tag()
for _, pair := range pairs {
if tag != "" && tag != pair.Tag {
if tag != pair.Tag {
continue
}
if !strings.HasSuffix(pair.Name, name) {
Expand Down
21 changes: 13 additions & 8 deletions libimage/pull_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,35 +85,40 @@ func TestPullPlatforms(t *testing.T) {
localArch := goruntime.GOARCH
localOS := goruntime.GOOS

pulledImages, err := runtime.Pull(ctx, "busybox", config.PullPolicyAlways, pullOptions)
withTag := "busybox:musl"

pulledImages, err := runtime.Pull(ctx, withTag, config.PullPolicyAlways, pullOptions)
require.NoError(t, err, "pull busybox")
require.Len(t, pulledImages, 1)

image, _, err := runtime.LookupImage("busybox", nil)
image, _, err := runtime.LookupImage(withTag, nil)
require.NoError(t, err, "lookup busybox")
require.NotNil(t, image, "lookup busybox")

image, _, err = runtime.LookupImage("busybox", &LookupImageOptions{Architecture: localArch})
_, _, err = runtime.LookupImage("busybox", nil)
require.Error(t, err, "untagged image resolves to non-existent :latest")

image, _, err = runtime.LookupImage(withTag, &LookupImageOptions{Architecture: localArch})
require.NoError(t, err, "lookup busybox - by local arch")
require.NotNil(t, image, "lookup busybox - by local arch")

image, _, err = runtime.LookupImage("busybox", &LookupImageOptions{OS: localOS})
image, _, err = runtime.LookupImage(withTag, &LookupImageOptions{OS: localOS})
require.NoError(t, err, "lookup busybox - by local arch")
require.NotNil(t, image, "lookup busybox - by local arch")

_, _, err = runtime.LookupImage("busybox", &LookupImageOptions{Architecture: "bogus"})
_, _, err = runtime.LookupImage(withTag, &LookupImageOptions{Architecture: "bogus"})
require.Error(t, err, "lookup busybox - bogus arch")

_, _, err = runtime.LookupImage("busybox", &LookupImageOptions{OS: "bogus"})
_, _, err = runtime.LookupImage(withTag, &LookupImageOptions{OS: "bogus"})
require.Error(t, err, "lookup busybox - bogus OS")

pullOptions.Architecture = "arm"
pulledImages, err = runtime.Pull(ctx, "busybox", config.PullPolicyAlways, pullOptions)
pulledImages, err = runtime.Pull(ctx, withTag, config.PullPolicyAlways, pullOptions)
require.NoError(t, err, "pull busybox - arm")
require.Len(t, pulledImages, 1)
pullOptions.Architecture = ""

image, _, err = runtime.LookupImage("busybox", &LookupImageOptions{Architecture: "arm"})
image, _, err = runtime.LookupImage(withTag, &LookupImageOptions{Architecture: "arm"})
require.NoError(t, err, "lookup busybox - by arm")
require.NotNil(t, image, "lookup busybox - by local arch")
}
40 changes: 21 additions & 19 deletions libimage/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,16 +389,17 @@ func (r *Runtime) lookupImageInDigestsAndRepoTags(name string, options *LookupIm
return nil, "", err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(The limbo comment above might be worth moderating a bit.)

}

if !shortnames.IsShortName(name) {
named, err := reference.ParseNormalizedNamed(name)
if err != nil {
return nil, "", err
}
digested, hasDigest := named.(reference.Digested)
if !hasDigest {
return nil, "", errors.Wrap(storage.ErrImageUnknown, name)
}
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, "", errors.Wrap(storage.ErrImageUnknown, name)
}

digested, isDigested := named.(reference.Digested)
if isDigested {
logrus.Debug("Looking for image with matching recorded digests")
digest := digested.Digest()
for _, image := range allImages {
Expand All @@ -408,22 +409,23 @@ func (r *Runtime) lookupImageInDigestsAndRepoTags(name string, options *LookupIm
}
}
}
return nil, "", errors.Wrap(storage.ErrImageUnknown, name)
}

if !shortnames.IsShortName(name) {
return nil, "", errors.Wrap(storage.ErrImageUnknown, name)
vrothberg marked this conversation as resolved.
Show resolved Hide resolved
vrothberg marked this conversation as resolved.
Show resolved Hide resolved
}

// Podman compat: if we're looking for a short name but couldn't
// resolve it via the registries.conf dance, we need to look at *all*
// images and check if the name we're looking for matches a repo tag.
// Split the name into a repo/tag pair
split := strings.SplitN(name, ":", 2)
repo := split[0]
tag := ""
if len(split) == 2 {
tag = split[1]
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)
}

for _, image := range allImages {
named, err := image.inRepoTags(repo, tag)
named, err := image.inRepoTags(namedTagged)
if err != nil {
return nil, "", err
}
Expand Down