From 984b7c041c5e99832b7e43e0e153146beeed9581 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <rothberg@redhat.com>
Date: Fri, 18 Jun 2021 14:45:29 +0200
Subject: [PATCH] libimage: pull: ignore platform for local image lookup

We must ignore the platform of a local image when doing lookups.  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.

Note that this has the consequence that a `--pull-never --arch=hurz` may
chose a local image of another architecture.  However, I estimate the
benefit of continuing to allow potentially invalid images higher than
not running them (and breaking workloads).

The changes required to touch the corrupted checks.  I used the occasion
to make the corrupted checks a bit cheaper.

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
---
 libimage/corrupted_test.go |  5 ++++-
 libimage/image.go          | 18 ++++++++++++++++++
 libimage/pull.go           | 17 ++++++++++++++---
 libimage/runtime.go        |  5 ++---
 4 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/libimage/corrupted_test.go b/libimage/corrupted_test.go
index b39473ea4..e683c1fa3 100644
--- a/libimage/corrupted_test.go
+++ b/libimage/corrupted_test.go
@@ -64,6 +64,9 @@ func TestCorruptedImage(t *testing.T) {
 	_, err = image.Inspect(ctx, false)
 	require.Error(t, err, "inspecting corrupted image should fail")
 
+	err = image.isCorrupted(imageName)
+	require.Error(t, err, "image is corrupted")
+
 	exists, err = runtime.Exists(imageName)
 	require.NoError(t, err, "corrupted image exists should not fail")
 	require.False(t, exists, "corrupted image should not be marked to exist")
@@ -75,7 +78,7 @@ func TestCorruptedImage(t *testing.T) {
 	require.Len(t, pulledImages, 1)
 	image = pulledImages[0]
 
-	// Inpsecting a repaired image should work.
+	// Inspecting a repaired image should work.
 	_, err = image.Inspect(ctx, false)
 	require.NoError(t, err, "inspecting repaired image should work")
 }
diff --git a/libimage/image.go b/libimage/image.go
index 3bcdbabec..f1272f507 100644
--- a/libimage/image.go
+++ b/libimage/image.go
@@ -61,6 +61,24 @@ func (i *Image) reload() error {
 	return nil
 }
 
+// isCorrupted returns an error if the image may be corrupted.
+func (i *Image) isCorrupted(name string) error {
+	// If it's a manifest list, we're good for now.
+	if _, err := i.getManifestList(); err == nil {
+		return nil
+	}
+
+	ref, err := i.StorageReference()
+	if err != nil {
+		return err
+	}
+
+	if _, err := ref.NewImage(context.Background(), nil); err != nil {
+		return errors.Errorf("Image %s exists in local storage but may be corrupted: %v", name, err)
+	}
+	return nil
+}
+
 // Names returns associated names with the image which may be a mix of tags and
 // digests.
 func (i *Image) Names() []string {
diff --git a/libimage/pull.go b/libimage/pull.go
index 22cb1a567..d4b37589a 100644
--- a/libimage/pull.go
+++ b/libimage/pull.go
@@ -350,15 +350,26 @@ 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.
 	lookupOptions := &LookupImageOptions{
-		Architecture: options.Architecture,
-		OS:           options.OS,
-		Variant:      options.Variant,
+		// NOTE: we must ignore the platform of a local image when
+		// doing lookups.  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.
+		IgnorePlatform: true,
 	}
 	localImage, resolvedImageName, err = r.LookupImage(imageName, lookupOptions)
 	if err != nil && errors.Cause(err) != storage.ErrImageUnknown {
 		logrus.Errorf("Looking up %s in local storage: %v", imageName, err)
 	}
 
+	// If the local image is corrupted, we need to repull it.
+	if localImage != nil {
+		if err := localImage.isCorrupted(imageName); err != nil {
+			logrus.Error(err)
+			localImage = nil
+		}
+	}
+
 	if pullPolicy == config.PullPolicyNever {
 		if localImage != nil {
 			logrus.Debugf("Pull policy %q but no local image has been found for %s", pullPolicy, imageName)
diff --git a/libimage/runtime.go b/libimage/runtime.go
index 58ac5864a..3cbd3dcf4 100644
--- a/libimage/runtime.go
+++ b/libimage/runtime.go
@@ -144,9 +144,8 @@ func (r *Runtime) Exists(name string) (bool, error) {
 	if image == nil {
 		return false, nil
 	}
-	// Inspect the image to make sure if it's corrupted or not.
-	if _, err := image.Inspect(context.Background(), false); err != nil {
-		logrus.Errorf("Image %s exists in local storage but may be corrupted: %v", name, err)
+	if err := image.isCorrupted(name); err != nil {
+		logrus.Error(err)
 		return false, nil
 	}
 	return true, nil