From ba3a9c0f770b7f77c7e95760d50f70216d505b8a Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Wed, 11 Jan 2023 14:55:02 +0100
Subject: [PATCH] libimage: pull: do not enforce pull if local image matches

I verified that the test fails without the code changes.

Fixes: podman/issues/17063
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
 libimage/pull.go      | 24 +++++++++++++++++-------
 libimage/pull_test.go | 23 +++++++++++++++++++++++
 2 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/libimage/pull.go b/libimage/pull.go
index 955132868..34a373da3 100644
--- a/libimage/pull.go
+++ b/libimage/pull.go
@@ -497,16 +497,26 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str
 
 	customPlatform := len(options.Architecture)+len(options.OS)+len(options.Variant) > 0
 	if customPlatform && pullPolicy != config.PullPolicyAlways && pullPolicy != config.PullPolicyNever {
-		// Unless the pull policy is always/never, 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 "newer" to make sure we're pulling down the
+		// Unless the specified platform matches the local image, 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 "newer" to make sure we're pulling down the
 		// correct image.
 		//
 		// NOTE that this is will even override --pull={false,never}.
-		pullPolicy = config.PullPolicyNewer
-		logrus.Debugf("Enforcing pull policy to %q to pull custom platform (arch: %q, os: %q, variant: %q) - local image may mistakenly specify wrong platform", pullPolicy, options.Architecture, options.OS, options.Variant)
+		localImageMatches := false
+		if localImage != nil {
+			_, matches, err := localImage.matchesPlatform(ctx, options.OS, options.Architecture, options.Variant)
+			if err != nil {
+				return nil, err
+			}
+			localImageMatches = matches
+		}
+		if !localImageMatches {
+			pullPolicy = config.PullPolicyNewer
+			logrus.Debugf("Enforcing pull policy to %q to pull custom platform (arch: %q, os: %q, variant: %q) - local image may mistakenly specify wrong platform", pullPolicy, options.Architecture, options.OS, options.Variant)
+		}
 	}
 
 	if pullPolicy == config.PullPolicyNever {
diff --git a/libimage/pull_test.go b/libimage/pull_test.go
index 28c4a86b8..50c57ba8b 100644
--- a/libimage/pull_test.go
+++ b/libimage/pull_test.go
@@ -5,9 +5,11 @@ import (
 	"fmt"
 	"os"
 	goruntime "runtime"
+	"strings"
 	"testing"
 
 	"github.com/containers/common/pkg/config"
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -94,6 +96,27 @@ func TestPullPlatforms(t *testing.T) {
 	require.NoError(t, err, "pull busybox")
 	require.Len(t, pulledImages, 1)
 
+	// Now re-pull with the platform explicitly set in the pull options. It
+	// should not repull an image (or perform a "newer" check) though but
+	// resolve to the local image.
+	//
+	// See containers/podman/issues/17063.
+	func() { // Anonymous function to make sure logrus is reset even on failure.
+		builder := strings.Builder{}
+		logrus.SetOutput(&builder)
+		logrus.SetLevel(logrus.DebugLevel)
+		defer builder.Reset()
+		defer logrus.SetOutput(os.Stderr)
+		defer logrus.SetLevel(logrus.InfoLevel)
+
+		pullOptions.Architecture = localArch
+		pullOptions.OS = localOS
+		pulledImages, err := runtime.Pull(ctx, withTag, config.PullPolicyMissing, pullOptions)
+		require.NoError(t, err, "pull busybox with same platform as before")
+		require.Len(t, pulledImages, 1)
+		require.NotContains(t, builder.String(), "local image may mistakenly specify wrong platform")
+	}()
+
 	// Repulling with a bogus architecture should yield an error and not
 	// choose the local image.
 	pullOptions.Architecture = "bogus"