Merge pull request #3198 from dtrudg/backport-3159

fix: enforce --platform for all OCI images (release-4.1)

Author: dtrudg

CHANGELOG.md

@@ -10,6 +10,8 @@
   execution of an OCI-SIF fails.
 - Fix failing builds from local images that have symbolic links for paths that
   are part of the base container environment (e.g. /var/tmp -> /tmp).
+- Fix issue where `--platform` / `--arch` did not apply when pulling an OCI
+  image to native SIF via image manifest, rather than image index.
 
 ## 4.1.4 \[2024-06-28\]
 

e2e/docker/docker.go

@@ -1411,7 +1411,7 @@ func (c ctx) testDockerPlatform(t *testing.T) {
 			exit:     0,
 		},
 		{
-			name:     "MultiArchBadPlatform",
+			name:     "MultiArchInvalidPlatform",
 			platform: "windows/m68k",
 			uri:      "docker://alpine:latest",
 			exit:     255,
@@ -1429,11 +1429,17 @@ func (c ctx) testDockerPlatform(t *testing.T) {
 			exit:     0,
 		},
 		{
-			name:     "SingleArchBadPlatform",
+			name:     "SingleArchInvalidPlatform",
 			platform: "windows/m68k",
 			uri:      "docker://ppc64le/alpine:latest",
 			exit:     255,
 		},
+		{
+			name:     "SingleArchMissingPlatform",
+			platform: "linux/arm64",
+			uri:      "docker://ppc64le/alpine:latest",
+			exit:     255,
+		},
 	}
 
 	for _, p := range []e2e.Profile{e2e.UserProfile, e2e.OCIUserProfile} {

internal/pkg/client/library/push.go

@@ -18,6 +18,7 @@ import (
 	"github.com/sylabs/singularity/v4/internal/pkg/client/ocisif"
 	"github.com/sylabs/singularity/v4/internal/pkg/client/progress"
 	"github.com/sylabs/singularity/v4/internal/pkg/remote/endpoint"
+	"github.com/sylabs/singularity/v4/internal/pkg/util/machine"
 	"github.com/sylabs/singularity/v4/pkg/sylog"
 	"golang.org/x/term"
 )
@@ -55,7 +56,7 @@ func Push(ctx context.Context, sourceFile string, destRef *scslibrary.Ref, opts
 
 // pushNative pushes a non-OCI SIF image, as a SIF, using the library client.
 func pushNative(ctx context.Context, sourceFile string, destRef *scslibrary.Ref, opts PushOptions) (uploadResponse *scslibrary.UploadImageComplete, err error) {
-	arch, err := sifArch(sourceFile)
+	arch, err := machine.SifArch(sourceFile)
 	if err != nil {
 		return nil, err
 	}
@@ -99,20 +100,6 @@ func pushNative(ctx context.Context, sourceFile string, destRef *scslibrary.Ref,
 	return libraryClient.UploadImage(ctx, f, destRef.Path, arch, destRef.Tags, opts.Description, progressBar)
 }
 
-func sifArch(filename string) (string, error) {
-	f, err := sif.LoadContainerFromPath(filename, sif.OptLoadWithFlag(os.O_RDONLY))
-	if err != nil {
-		return "", fmt.Errorf("unable to open: %v: %w", filename, err)
-	}
-	defer f.UnloadContainer()
-
-	arch := f.PrimaryArch()
-	if arch == "unknown" {
-		return arch, fmt.Errorf("unknown architecture in SIF file")
-	}
-	return arch, nil
-}
-
 // pushOCI pushes an OCI SIF image, as an OCI image, using the ocisif client.
 func pushOCI(ctx context.Context, sourceFile string, destRef *scslibrary.Ref, opts PushOptions) error {
 	sylog.Infof("Pushing an OCI-SIF to the library OCI registry. Use `--oci` to pull this image.")

internal/pkg/client/net/pull.go

@@ -196,7 +196,7 @@ func PullToFile(ctx context.Context, imgCache *cache.Handle, pullTo, pullFrom st
 
 	src, err := pull(ctx, imgCache, directTo, pullFrom)
 	if err != nil {
-		return "", fmt.Errorf("error fetching image to cache: %v", err)
+		return "", fmt.Errorf("error fetching image: %v", err)
 	}
 
 	if directTo == "" {

internal/pkg/client/oci/nativesif.go

@@ -13,6 +13,8 @@ import (
 	"github.com/sylabs/singularity/v4/internal/pkg/build"
 	"github.com/sylabs/singularity/v4/internal/pkg/cache"
 	"github.com/sylabs/singularity/v4/internal/pkg/ociimage"
+	"github.com/sylabs/singularity/v4/internal/pkg/ociplatform"
+	"github.com/sylabs/singularity/v4/internal/pkg/util/machine"
 	buildtypes "github.com/sylabs/singularity/v4/pkg/build/types"
 	"github.com/sylabs/singularity/v4/pkg/sylog"
 )
@@ -50,6 +52,18 @@ func pullNativeSIF(ctx context.Context, imgCache *cache.Handle, directTo, pullFr
 				return "", err
 			}
 		} else {
+			// Ensure what's retrieved from the cache matches the target platform
+			sifArch, err := machine.SifArch(cacheEntry.Path)
+			if err != nil {
+				return "", err
+			}
+			sifPlatform, err := ociplatform.PlatformFromArch(sifArch)
+			if err != nil {
+				return "", fmt.Errorf("could not determine OCI platform from cached image architecture %q: %w", sifArch, err)
+			}
+			if !sifPlatform.Satisfies(opts.Platform) {
+				return "", fmt.Errorf("image (%s) does not satisfy required platform (%s)", sifPlatform, opts.Platform)
+			}
 			sylog.Infof("Using cached SIF image")
 		}
 		imagePath = cacheEntry.Path

internal/pkg/client/oci/pull.go

@@ -99,7 +99,7 @@ func PullToFile(ctx context.Context, imgCache *cache.Handle, pullTo, pullFrom st
 		src, err = pullNativeSIF(ctx, imgCache, directTo, pullFrom, opts)
 	}
 	if err != nil {
-		return "", fmt.Errorf("error fetching image to cache: %v", err)
+		return "", fmt.Errorf("error fetching image: %v", err)
 	}
 
 	if directTo == "" {

internal/pkg/client/ocisif/ocisif.go

@@ -22,12 +22,13 @@ import (
 	"github.com/google/go-containerregistry/pkg/v1/remote"
 	"github.com/google/go-containerregistry/pkg/v1/types"
 	"github.com/sylabs/oci-tools/pkg/mutate"
-	ocisif "github.com/sylabs/oci-tools/pkg/sif"
+	ocitsif "github.com/sylabs/oci-tools/pkg/sif"
 	"github.com/sylabs/sif/v2/pkg/sif"
 	"github.com/sylabs/singularity/v4/internal/pkg/cache"
 	"github.com/sylabs/singularity/v4/internal/pkg/client/progress"
 	"github.com/sylabs/singularity/v4/internal/pkg/ociimage"
 	"github.com/sylabs/singularity/v4/internal/pkg/ociplatform"
+	"github.com/sylabs/singularity/v4/internal/pkg/ocisif"
 	"github.com/sylabs/singularity/v4/internal/pkg/remote/credential/ociauth"
 	"github.com/sylabs/singularity/v4/internal/pkg/util/fs"
 	"github.com/sylabs/singularity/v4/pkg/sylog"
@@ -107,6 +108,19 @@ func PullOCISIF(ctx context.Context, imgCache *cache.Handle, directTo, pullFrom
 				return "", err
 			}
 		} else {
+			// Ensure what's retrieved from the cache matches the target platform
+			fi, err := sif.LoadContainerFromPath(cacheEntry.Path)
+			if err != nil {
+				return "", err
+			}
+			defer fi.UnloadContainer()
+			img, err := ocisif.GetSingleImage(fi)
+			if err != nil {
+				return "", fmt.Errorf("while getting image: %w", err)
+			}
+			if err := ociplatform.CheckImagePlatform(opts.Platform, img); err != nil {
+				return "", err
+			}
 			sylog.Infof("Using cached OCI-SIF image")
 		}
 		imagePath = cacheEntry.Path
@@ -138,10 +152,6 @@ func createOciSif(ctx context.Context, tOpts *ociimage.TransportOptions, imgCach
 		return fmt.Errorf("while fetching OCI image: %w", err)
 	}
 
-	if err := ociplatform.CheckImagePlatform(tOpts.Platform, img); err != nil {
-		return fmt.Errorf("while checking OCI image: %w", err)
-	}
-
 	digest, err := img.Digest()
 	if err != nil {
 		return err
@@ -185,7 +195,7 @@ func writeImageToOCISif(img ggcrv1.Image, imageDest string) error {
 	ii := ggcrmutate.AppendManifests(empty.Index, ggcrmutate.IndexAddendum{
 		Add: img,
 	})
-	return ocisif.Write(imageDest, ii, ocisif.OptWriteWithSpareDescriptorCapacity(spareDescriptorCapacity))
+	return ocitsif.Write(imageDest, ii, ocitsif.OptWriteWithSpareDescriptorCapacity(spareDescriptorCapacity))
 }
 
 // convertImageToOciSif will convert an image to an oci-sif with squashfs layer
@@ -209,7 +219,7 @@ func convertImageToOciSif(img ggcrv1.Image, digest ggcrv1.Hash, imageDest, workD
 	ii := ggcrmutate.AppendManifests(empty.Index, ggcrmutate.IndexAddendum{
 		Add: img,
 	})
-	return ocisif.Write(imageDest, ii, ocisif.OptWriteWithSpareDescriptorCapacity(spareDescriptorCapacity))
+	return ocitsif.Write(imageDest, ii, ocitsif.OptWriteWithSpareDescriptorCapacity(spareDescriptorCapacity))
 }
 
 func imgLayersToSquashfs(img ggcrv1.Image, digest ggcrv1.Hash, workDir string) (sqfsImage ggcrv1.Image, err error) {
@@ -266,7 +276,7 @@ func PushOCISIF(ctx context.Context, sourceFile, destRef string, ociAuth *authn.
 	}
 	defer fi.UnloadContainer()
 
-	ix, err := ocisif.ImageIndexFromFileImage(fi)
+	ix, err := ocitsif.ImageIndexFromFileImage(fi)
 	if err != nil {
 		return fmt.Errorf("only OCI-SIF files can be pushed to docker/OCI registries")
 	}

internal/pkg/client/oras/pull.go

@@ -86,7 +86,7 @@ func PullToFile(ctx context.Context, imgCache *cache.Handle, pullTo, pullFrom st
 
 	src, err := pull(ctx, imgCache, directTo, pullFrom, ociAuth, reqAuthFile)
 	if err != nil {
-		return "", fmt.Errorf("error fetching image to cache: %v", err)
+		return "", fmt.Errorf("error fetching image: %v", err)
 	}
 
 	if directTo == "" {

internal/pkg/client/shub/pull.go

@@ -199,7 +199,7 @@ func PullToFile(ctx context.Context, imgCache *cache.Handle, pullTo, pullFrom st
 
 	src, err := pull(ctx, imgCache, directTo, pullFrom, noHTTPS)
 	if err != nil {
-		return "", fmt.Errorf("error fetching image to cache: %v", err)
+		return "", fmt.Errorf("error fetching image: %v", err)
 	}
 
 	if directTo == "" {

internal/pkg/ociimage/fetch.go

@@ -20,6 +20,7 @@ import (
 	ggcrv1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/sylabs/singularity/v4/internal/pkg/cache"
 	"github.com/sylabs/singularity/v4/internal/pkg/client/progress"
+	"github.com/sylabs/singularity/v4/internal/pkg/ociplatform"
 	"github.com/sylabs/singularity/v4/pkg/sylog"
 )
 
@@ -56,7 +57,8 @@ func cachedImage(ctx context.Context, imgCache *cache.Handle, srcImg ggcrv1.Imag
 // docker daemon, it will be pulled into the local cache - which is a
 // multi-image OCI layout. If the cache is disabled, the image will be fetched
 // into a subdirectory of the provided tmpDir. The caller is responsible for
-// cleaning up tmpDir.
+// cleaning up tmpDir. The platform of the image will be checked against
+// tOpts.Platform.
 func LocalImage(ctx context.Context, tOpts *TransportOptions, imgCache *cache.Handle, imageURI, tmpDir string) (ggcrv1.Image, error) {
 	// oci-archive tarball is a local file, but current ggcr cannot read
 	// directly. Must always extract to a layoutdir .
@@ -70,7 +72,16 @@ func LocalImage(ctx context.Context, tOpts *TransportOptions, imgCache *cache.Ha
 	}
 	// Docker tarballs and OCI layouts are already local
 	if srcType == TarballSourceSink || srcType == OCISourceSink {
-		return srcType.Image(ctx, srcRef, tOpts, nil)
+		img, err := srcType.Image(ctx, srcRef, tOpts, nil)
+		if err != nil {
+			return nil, err
+		}
+		// Verify against requested platform - ggcr doesn't filter on platform
+		// when pulling a manifest directly, only on pulling from an image index.
+		if err := ociplatform.CheckImagePlatform(tOpts.Platform, img); err != nil {
+			return nil, fmt.Errorf("while checking OCI image: %w", err)
+		}
+		return img, nil
 	}
 
 	return LocalImageLayout(ctx, tOpts, imgCache, imageURI, tmpDir)
@@ -82,7 +93,8 @@ func LocalImage(ctx context.Context, tOpts *TransportOptions, imgCache *cache.Ha
 // docker daemon, it will be pulled into the local cache - which is a
 // multi-image OCI layout. If the cache is disabled, the image will be fetched
 // into a subdirectory of the provided tmpDir. The caller is responsible for
-// cleaning up tmpDir.
+// cleaning up tmpDir. The platform of the image will be checked against
+// tOpts.Platform.
 func LocalImageLayout(ctx context.Context, tOpts *TransportOptions, imgCache *cache.Handle, imageURI, tmpDir string) (ggcrv1.Image, error) {
 	if strings.HasPrefix(imageURI, "oci-archive:") {
 		// oci-archive is a straight tar of an OCI layout, so extract to a tempDir
@@ -98,19 +110,27 @@ func LocalImageLayout(ctx context.Context, tOpts *TransportOptions, imgCache *ca
 		return nil, err
 	}
 
-	// We might already have an OCI layout at this point
-	if srcType == OCISourceSink {
-		return srcType.Image(ctx, srcRef, tOpts, nil)
-	}
-
-	// Registry / Docker Daemon images need to be fetched
 	rt := progress.NewRoundTripper(ctx, nil)
 	srcImg, err := srcType.Image(ctx, srcRef, tOpts, rt)
 	if err != nil {
 		rt.ProgressShutdown()
 		return nil, err
 	}
 
+	// Verify against requested platform - ggcr doesn't filter on platform when
+	// pulling a manifest directly, only on pulling from an image index.
+	if err := ociplatform.CheckImagePlatform(tOpts.Platform, srcImg); err != nil {
+		return nil, fmt.Errorf("while checking OCI image: %w", err)
+	}
+
+	// We might already have an OCI layout at this point - which is local.
+	if srcType == OCISourceSink {
+		rt.ProgressShutdown()
+		return srcImg, nil
+	}
+
+	// Registry / Docker Daemon images need to be fetched
+
 	if imgCache != nil && !imgCache.IsDisabled() {
 		// Ensure the image is cached, and return reference to the cached image.
 		cachedImg, err := cachedImage(ctx, imgCache, srcImg)