libimage: image lookup: check platform

Check the platform when looking up images locally.  When the user
requested a custom platform and a local image doesn't match, the
image will be discarded.  Otherwise a warning will be emitted.

Also refactor the code to make it more maintainable in the future.

Fixes: containers/podman/issues/12682
Signed-off-by: Valentin Rothberg <[email protected]>

libimage/inspect.go

@@ -216,7 +216,7 @@ func (i *Image) inspectInfo(ctx context.Context) (*types.ImageInspectInfo, error
 		return nil, err
 	}
 
-	img, err := ref.NewImage(ctx, i.runtime.systemContextCopy())
+	img, err := ref.NewImage(ctx, &i.runtime.systemContext)
 	if err != nil {
 		return nil, err
 	}

libimage/platform.go

@@ -0,0 +1,53 @@
+package libimage
+
+import (
+	"context"
+	"fmt"
+	"runtime"
+)
+
+func toPlatformString(architecture, os, variant string) string {
+	if variant == "" {
+		return fmt.Sprintf("%s/%s", os, architecture)
+	}
+	return fmt.Sprintf("%s/%s/%s", os, architecture, variant)
+}
+
+// Checks whether the image matches the specified platform.
+// Returns
+//  * 1) a matching error that can be used for logging (or returning) what does not match
+//  * 2) a bool indicating whether architecture, os or variant were set (some callers need that to decide whether they need to throw an error)
+//  * 3) a fatal error that occurred prior to check for matches (e.g., storage errors etc.)
+func (i *Image) matchesPlatform(ctx context.Context, architecture, os, variant string) (error, bool, error) {
+	customPlatform := len(architecture)+len(os)+len(variant) != 0
+
+	if len(architecture) == 0 {
+		architecture = runtime.GOARCH
+	}
+	if len(os) == 0 {
+		os = runtime.GOOS
+	}
+
+	inspectInfo, err := i.inspectInfo(ctx)
+	if err != nil {
+		return nil, customPlatform, fmt.Errorf("inspecting image: %w", err)
+	}
+
+	matches := true
+	switch {
+	case architecture != inspectInfo.Architecture:
+		matches = false
+	case os != inspectInfo.Os:
+		matches = false
+	case variant != "" && variant != inspectInfo.Variant:
+		matches = false
+	}
+
+	if matches {
+		return nil, customPlatform, nil
+	}
+
+	imagePlatform := toPlatformString(inspectInfo.Architecture, inspectInfo.Os, inspectInfo.Variant)
+	expectedPlatform := toPlatformString(architecture, os, variant)
+	return fmt.Errorf("image platform (%s) does not match the expected platform (%s)", imagePlatform, expectedPlatform), customPlatform, nil
+}

libimage/platform_test.go

@@ -0,0 +1,20 @@
+package libimage
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestToPlatformString(t *testing.T) {
+	for _, test := range []struct {
+		arch, os, variant, expected string
+	}{
+		{"a", "b", "", "b/a"},
+		{"a", "b", "c", "b/a/c"},
+		{"", "", "c", "//c"}, // callers are responsible for the input
+	} {
+		platform := toPlatformString(test.arch, test.os, test.variant)
+		require.Equal(t, platform, test.expected)
+	}
+}

libimage/pull.go

@@ -160,20 +160,31 @@ func (r *Runtime) Pull(ctx context.Context, name string, pullPolicy config.PullP
 	}
 
 	localImages := []*Image{}
-	lookupOptions := &LookupImageOptions{Architecture: options.Architecture, OS: options.OS, Variant: options.Variant}
 	for _, name := range pulledImages {
-		local, _, err := r.LookupImage(name, nil)
+		image, _, err := r.LookupImage(name, nil)
 		if err != nil {
 			return nil, errors.Wrapf(err, "error locating pulled image %q name in containers storage", name)
 		}
-		ref, err := local.StorageReference()
+
+		// Note that we can ignore the 2nd return value here. Some
+		// images may ship with "wrong" platform, but we already warn
+		// about it. Throwing an error is not (yet) the plan.
+		matchError, _, err := image.matchesPlatform(ctx, options.Architecture, options.OS, options.Variant)
 		if err != nil {
-			return nil, fmt.Errorf("creating storage reference for pulled image %q: %w", name, err)
+			return nil, fmt.Errorf("checking platform of image %s: %w", name, err)
 		}
-		if _, err := r.imageReferenceMatchesContext(ref, name, lookupOptions, options.Writer); err != nil {
-			return nil, fmt.Errorf("checking platform for pulled image %q: %w", name, err)
+
+		// If the image does not match the expected/requested platform,
+		// make sure to leave some breadcrumbs for the user.
+		if matchError != nil {
+			if options.Writer == nil {
+				logrus.Warnf("%v", matchError)
+			} else {
+				fmt.Fprintf(options.Writer, "WARNING: %v\n", matchError)
+			}
 		}
-		localImages = append(localImages, local)
+
+		localImages = append(localImages, image)
 	}
 
 	return localImages, pullError

libimage/runtime.go

@@ -3,7 +3,6 @@ package libimage
 import (
 	"context"
 	"fmt"
-	"io"
 	"os"
 	"strings"
 
@@ -379,21 +378,24 @@ func (r *Runtime) lookupImageInLocalStorage(name, candidate string, options *Loo
 		image = instance
 	}
 
-	matches, err := r.imageReferenceMatchesContext(ref, name, options, nil)
-	if err != nil {
-		return nil, err
-	}
-
-	// NOTE: if the user referenced by ID we must optimistically assume
-	// that they know what they're doing.  Given, we already did the
-	// manifest limbo above, we may already have resolved it.
-	if !matches && !strings.HasPrefix(image.ID(), candidate) {
-		return nil, nil
-	}
 	// Also print the string within the storage transport.  That may aid in
 	// debugging when using additional stores since we see explicitly where
 	// the store is and which driver (options) are used.
 	logrus.Debugf("Found image %q as %q in local containers storage (%s)", name, candidate, ref.StringWithinTransport())
+
+	// Ignore the (fatal) error since the image may be corrupted, which
+	// will bubble up at other places.  During lookup, we just return it as
+	// is.
+	if matchError, customPlatform, _ := image.matchesPlatform(context.Background(), options.Architecture, options.OS, options.Variant); matchError != nil {
+		if customPlatform {
+			logrus.Debugf("%v", matchError)
+			// Return nil if the user clearly requested a custom
+			// platform and the located image does not match.
+			return nil, nil
+		}
+		logrus.Warnf("%v", matchError)
+	}
+
 	return image, nil
 }
 
@@ -498,49 +500,6 @@ func (r *Runtime) ResolveName(name string) (string, error) {
 	return normalized.String(), nil
 }
 
-// imageReferenceMatchesContext return true if the specified reference matches
-// the platform (os, arch, variant) as specified by the lookup options.
-func (r *Runtime) imageReferenceMatchesContext(ref types.ImageReference, name string, options *LookupImageOptions, writer io.Writer) (bool, error) {
-	if options.Architecture+options.OS+options.Variant == "" {
-		return true, nil
-	}
-
-	ctx := context.Background()
-	img, err := ref.NewImage(ctx, &r.systemContext)
-	if err != nil {
-		return false, err
-	}
-	defer img.Close()
-	data, err := img.Inspect(ctx)
-	if err != nil {
-		return false, err
-	}
-
-	writeMessage := func(msg string) {
-		if writer == nil {
-			logrus.Warn(msg)
-		} else {
-			fmt.Fprintf(writer, "WARNING: %s\n", msg)
-		}
-	}
-
-	matches := true
-	if options.Architecture != "" && options.Architecture != data.Architecture {
-		writeMessage(fmt.Sprintf("requested architecture %q does not match architecture %q of image %s", options.Architecture, data.Architecture, name))
-		matches = false
-	}
-	if options.OS != "" && options.OS != data.Os {
-		writeMessage(fmt.Sprintf("requested OS %q does not match OS %q of image %s", options.OS, data.Os, name))
-		matches = false
-	}
-	if options.Variant != "" && options.Variant != data.Variant {
-		writeMessage(fmt.Sprintf("requested variant %q does not match variant %q of image %s", options.Variant, data.Variant, name))
-		matches = false
-	}
-
-	return matches, nil
-}
-
 // IsExternalContainerFunc allows for checking whether the specified container
 // is an external one.  The definition of an external container can be set by
 // callers.