From 7a7a142de027dfc09fec5e91bae6aad83b8a8b06 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 12 Jan 2022 10:04:51 +0100 Subject: [PATCH] libimage: pull: platform checks for non-local platform 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 --- libimage/pull.go | 19 +++++++++++++------ libimage/pull_test.go | 7 +++++++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/libimage/pull.go b/libimage/pull.go index ba741c622..ff93b6ed8 100644 --- a/libimage/pull.go +++ b/libimage/pull.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "io" + "runtime" "strings" "time" @@ -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) } diff --git a/libimage/pull_test.go b/libimage/pull_test.go index bc3a9cc04..2c9a397ad 100644 --- a/libimage/pull_test.go +++ b/libimage/pull_test.go @@ -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")