From f55ab698e5c6fe6d9f050542b520267b0ac41560 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <rothberg@redhat.com>
Date: Sun, 27 Jun 2021 13:05:08 +0200
Subject: [PATCH] pull: custom platform: do not use local image name

Do not use the name of the locally resolved image when pulling an image
with a custom platform.  As we recently re-discovered [1], many
multi-arch images in the wild do not adhere to the OCI image spec and
either declare custom or simply wrong platforms (arch, os, variant).

To address such wrong images, we enforce the pull-always policy whenever
a custom arch, os or variant is specified.  We have to do that since we
cannot reliably perform platform matches to any image we would find in
the local containers storage.

To complete the fix, we need to ignore any local image and not use the
locally resolved name which we usually have to do (see [2]).

Let's assume we have a local image "localhost/foo" (arch=amd64).  If we
perform a `pull --arch=arm64`, we would not attempt to be pulling
`localhost/foo` but use the ordinary short-name resolution and look for
a matching alias or walk the unqualified-search registries.

In other words: short-name resolution of multi-arch images is prone to
errors but we should continue supporting images in the wild.

[1] containers/podman/issues/10682
[2] containers/buildah/issues/2904

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
---
 libimage/pull.go | 39 +++++++++++++++++++++++++--------------
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/libimage/pull.go b/libimage/pull.go
index d97a884d2..0d3a47020 100644
--- a/libimage/pull.go
+++ b/libimage/pull.go
@@ -374,18 +374,22 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str
 		}
 	}
 
-	// Unless the pull policy is "always", we must pessimistically assume
-	// that the local image has an invalid architecture (see
-	// containers/podman/issues/10682).  Hence, whenever the user requests
-	// a custom platform, set the pull policy to "always" to make sure
-	// we're pulling down the image.
-	//
-	// NOTE that this is will even override --pull={false,never}.  This is
-	// very likely a bug but a consistent one in Podman/Buildah and should
-	// be addressed at a later point.
-	if pullPolicy != config.PullPolicyAlways && len(options.Architecture)+len(options.OS)+len(options.Variant) > 0 {
-		logrus.Debugf("Enforcing pull policy to %q to support custom platform (arch: %q, os: %q, variant: %q)", "always", options.Architecture, options.OS, options.Variant)
-		pullPolicy = config.PullPolicyAlways
+	customPlatform := false
+	if len(options.Architecture)+len(options.OS)+len(options.Variant) > 0 {
+		customPlatform = true
+		// Unless the pull policy is "always", we must pessimistically assume
+		// that the local image has an invalid architecture (see
+		// containers/podman/issues/10682).  Hence, whenever the user requests
+		// a custom platform, set the pull policy to "always" to make sure
+		// we're pulling down the image.
+		//
+		// NOTE that this is will even override --pull={false,never}.  This is
+		// very likely a bug but a consistent one in Podman/Buildah and should
+		// be addressed at a later point.
+		if pullPolicy != config.PullPolicyAlways {
+			logrus.Debugf("Enforcing pull policy to %q to support custom platform (arch: %q, os: %q, variant: %q)", "always", options.Architecture, options.OS, options.Variant)
+			pullPolicy = config.PullPolicyAlways
+		}
 	}
 
 	if pullPolicy == config.PullPolicyNever {
@@ -412,8 +416,15 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str
 	}
 
 	// If we found a local image, we should use it's locally resolved name
-	// (see containers/buildah #2904).
-	if localImage != nil {
+	// (see containers/buildah/issues/2904).  An exception is if a custom
+	// platform is specified (e.g., `--arch=arm64`).  In that case, we need
+	// to pessimistically pull the image since some images declare wrong
+	// platforms making platform checks absolutely unreliable (see
+	// containers/podman/issues/10682).
+	//
+	// In other words: multi-arch support can only be as good as the images
+	// in the wild.
+	if localImage != nil && !customPlatform {
 		if imageName != resolvedImageName {
 			logrus.Debugf("Image %s resolved to local image %s which will be used for pulling", imageName, resolvedImageName)
 		}