Skip to content

Commit

Permalink
libimage: pull: platform checks for non-local platform
Browse files Browse the repository at this point in the history
After containers/podman/issues/10682, we decided to always re-pull
images of non-local platforms and match *any* local image. Over time, we
refined this logic to not *always* pull the image but only if there is a
*newer* one. This has slightly changed the semantics and requires to
perform platform checks when looking up a local image. Otherwise, bogus
values would match a local image and mistakenly return it.

Signed-off-by: Valentin Rothberg <[email protected]>
  • Loading branch information
vrothberg committed Jan 12, 2022
1 parent 08c2c97 commit 7a7a142
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 6 deletions.
19 changes: 13 additions & 6 deletions libimage/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"io"
"runtime"
"strings"
"time"

Expand Down Expand Up @@ -446,12 +447,18 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str
// If there's already a local image "localhost/foo", then we should
// attempt pulling that instead of doing the full short-name dance.
//
// NOTE: we must ignore the platform of a local image when doing
// lookups here, even if arch/os/variant is set. Some images set an
// incorrect or even invalid platform (see containers/podman/issues/10682).
// Doing the lookup while ignoring the platform checks prevents
// redundantly downloading the same image.
localImage, resolvedImageName, err = r.LookupImage(imageName, nil)
// NOTE that we only do platform checks if the specified values differ
// from the local platform. Unfortunately, there are many images used
// in the wild which don't set the correct value(s) in the config
// causing various issues such as containers/podman/issues/10682.
lookupImageOptions := &LookupImageOptions{Variant: options.Variant}
if options.Architecture != runtime.GOARCH {
lookupImageOptions.Architecture = options.Architecture
}
if options.OS != runtime.GOOS {
lookupImageOptions.OS = options.OS
}
localImage, resolvedImageName, err = r.LookupImage(imageName, lookupImageOptions)
if err != nil && errors.Cause(err) != storage.ErrImageUnknown {
logrus.Errorf("Looking up %s in local storage: %v", imageName, err)
}
Expand Down
7 changes: 7 additions & 0 deletions libimage/pull_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,13 @@ func TestPullPlatforms(t *testing.T) {
require.NoError(t, err, "pull busybox")
require.Len(t, pulledImages, 1)

// Repulling with a bogus architecture should yield an error and not
// choose the local image.
pullOptions.Architecture = "bogus"
_, err = runtime.Pull(ctx, withTag, config.PullPolicyNewer, pullOptions)
require.Error(t, err, "pulling with a bogus architecture must fail even if there is a local image of another architecture")
require.Contains(t, err.Error(), "no image found in manifest list for architecture bogus")

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

0 comments on commit 7a7a142

Please sign in to comment.