Fix multi-platform soci standalone convert

Author: prafgup

cmd/soci/commands/internal/standalone.go

@@ -70,22 +70,11 @@ func LoadImage(ctx context.Context, inputPath string, tmpDir string) (*Standalon
 	if err != nil {
 		return nil, fmt.Errorf("failed to read index.json from %s: %w", inputPath, err)
 	}
-	rootDesc, err := parseRootDescriptor(indexData)
+	rootDesc, err := resolveLayoutRoot(tmpDir, indexData)
 	if err != nil {
 		return nil, err
 	}
 
-	// If the root descriptor is a manifest list (e.g. from nerdctl save),
-	// resolve it to available platform manifests. This handles partial exports
-	// where the manifest list references all platforms but only a subset of
-	// platform blobs were exported.
-	if images.IsIndexType(rootDesc.MediaType) {
-		rootDesc, err = resolveManifestList(tmpDir, rootDesc)
-		if err != nil {
-			return nil, err
-		}
-	}
-
 	orasStore, err := oci.New(tmpDir)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create writable OCI store: %w", err)
@@ -139,73 +128,81 @@ func SaveImageToDir(srcDir string, desc ocispec.Descriptor, outputPath string) e
 	return os.WriteFile(filepath.Join(outputPath, "index.json"), indexData, 0644)
 }
 
-// parseRootDescriptor unmarshals OCI index JSON and returns the manifest descriptor.
-func parseRootDescriptor(indexData []byte) (ocispec.Descriptor, error) {
-	var index ocispec.Index
-	if err := json.Unmarshal(indexData, &index); err != nil {
-		return ocispec.Descriptor{}, fmt.Errorf("failed to unmarshal index.json: %w", err)
-	}
-	if len(index.Manifests) == 0 {
+// resolveLayoutRoot returns a descriptor for the OCI image layout's root manifest.
+// It accepts index.json shapes produced by common tools: a single image manifest,
+// a single descriptor pointing at a nested manifest list (e.g. nerdctl save), or
+// a flat list of per-platform manifests (e.g. go-containerregistry layout.Write).
+// Children whose blobs are missing are filtered out.
+func resolveLayoutRoot(layoutDir string, indexData []byte) (ocispec.Descriptor, error) {
+	var top ocispec.Index
+	if err := json.Unmarshal(indexData, &top); err != nil {
+		return ocispec.Descriptor{}, fmt.Errorf("unmarshal index.json: %w", err)
+	}
+	if len(top.Manifests) == 0 {
 		return ocispec.Descriptor{}, errors.New("index.json contains no manifests")
 	}
-	return index.Manifests[0], nil
-}
-
-// resolveManifestList reads a manifest list blob and resolves it based on which
-// platform blobs are actually present in the layout. If all platforms are available,
-// it returns the original manifest list. If only one is available, it returns that
-// platform manifest directly. If multiple (but not all) are available, it writes a
-// filtered manifest list containing only the available platforms. This handles tools
-// like `nerdctl save` that export a manifest list referencing all platforms even when
-// only a subset was pulled.
-func resolveManifestList(layoutDir string, listDesc ocispec.Descriptor) (ocispec.Descriptor, error) {
-	blobPath := filepath.Join(layoutDir, "blobs", listDesc.Digest.Algorithm().String(), listDesc.Digest.Encoded())
-	listData, err := os.ReadFile(blobPath)
-	if err != nil {
-		return ocispec.Descriptor{}, fmt.Errorf("failed to read manifest list blob: %w", err)
-	}
-
-	var manifestList ocispec.Index
-	if err := json.Unmarshal(listData, &manifestList); err != nil {
-		return ocispec.Descriptor{}, fmt.Errorf("failed to unmarshal manifest list: %w", err)
+	// Single non-index entry: a plain single-platform image.
+	if len(top.Manifests) == 1 && !images.IsIndexType(top.Manifests[0].MediaType) {
+		return top.Manifests[0], nil
 	}
 
-	// Find which platform manifests have their blobs present
-	var available []ocispec.Descriptor
-	for _, desc := range manifestList.Manifests {
-		p := filepath.Join(layoutDir, "blobs", desc.Digest.Algorithm().String(), desc.Digest.Encoded())
-		if _, err := os.Stat(p); err == nil {
-			available = append(available, desc)
+	// Locate the manifest list to walk. Either index.json points at a nested list
+	// blob, or index.json is itself the list.
+	var (
+		listDesc  ocispec.Descriptor
+		listBytes = indexData
+		mediaType = top.MediaType
+	)
+	if len(top.Manifests) == 1 {
+		listDesc = top.Manifests[0]
+		mediaType = listDesc.MediaType
+		b, err := os.ReadFile(blobPath(layoutDir, listDesc.Digest))
+		if err != nil {
+			return ocispec.Descriptor{}, fmt.Errorf("read manifest list: %w", err)
 		}
+		listBytes = b
 	}
-	if len(available) == 0 {
-		return ocispec.Descriptor{}, errors.New("manifest list contains no manifests with available blobs")
+	if mediaType == "" {
+		mediaType = ocispec.MediaTypeImageIndex
 	}
 
-	// If all platforms are available, keep the original manifest list
-	if len(available) == len(manifestList.Manifests) {
-		return listDesc, nil
+	var list ocispec.Index
+	if err := json.Unmarshal(listBytes, &list); err != nil {
+		return ocispec.Descriptor{}, fmt.Errorf("unmarshal manifest list: %w", err)
 	}
 
-	// If only one platform is available, return it directly as a single manifest
-	if len(available) == 1 {
+	available := make([]ocispec.Descriptor, 0, len(list.Manifests))
+	for _, d := range list.Manifests {
+		if _, err := os.Stat(blobPath(layoutDir, d.Digest)); err == nil {
+			available = append(available, d)
+		}
+	}
+	switch {
+	case len(available) == 0:
+		return ocispec.Descriptor{}, errors.New("manifest list contains no entries with available blobs")
+	case len(available) == 1 && images.IsManifestType(available[0].MediaType):
 		return available[0], nil
+	case listDesc.Digest != "" && len(available) == len(list.Manifests):
+		// Nested list is fully available; keep its original digest.
+		return listDesc, nil
 	}
 
-	// Multiple (but not all) platforms available: write a filtered manifest list
-	manifestList.Manifests = available
-	filteredData, err := json.Marshal(manifestList)
+	list.MediaType = mediaType
+	list.Manifests = available
+	data, err := json.Marshal(list)
 	if err != nil {
-		return ocispec.Descriptor{}, fmt.Errorf("failed to marshal filtered manifest list: %w", err)
+		return ocispec.Descriptor{}, fmt.Errorf("marshal manifest list: %w", err)
 	}
-	filteredDigest := digest.FromBytes(filteredData)
-	filteredPath := filepath.Join(layoutDir, "blobs", filteredDigest.Algorithm().String(), filteredDigest.Encoded())
-	if err := os.WriteFile(filteredPath, filteredData, 0644); err != nil {
-		return ocispec.Descriptor{}, fmt.Errorf("failed to write filtered manifest list: %w", err)
+	dgst := digest.FromBytes(data)
+	if err := os.MkdirAll(filepath.Dir(blobPath(layoutDir, dgst)), 0755); err != nil {
+		return ocispec.Descriptor{}, err
 	}
-	return ocispec.Descriptor{
-		MediaType: listDesc.MediaType,
-		Digest:    filteredDigest,
-		Size:      int64(len(filteredData)),
-	}, nil
+	if err := os.WriteFile(blobPath(layoutDir, dgst), data, 0644); err != nil {
+		return ocispec.Descriptor{}, err
+	}
+	return ocispec.Descriptor{MediaType: mediaType, Digest: dgst, Size: int64(len(data))}, nil
+}
+
+func blobPath(layoutDir string, dgst digest.Digest) string {
+	return filepath.Join(layoutDir, "blobs", dgst.Algorithm().String(), dgst.Encoded())
 }

integration/convert_standalone_test.go

@@ -17,13 +17,16 @@
 package integration
 
 import (
+	"encoding/json"
 	"path/filepath"
 	"strings"
 	"testing"
 
+	"github.com/awslabs/soci-snapshotter/soci"
 	"github.com/awslabs/soci-snapshotter/util/dockershell"
 	"github.com/awslabs/soci-snapshotter/util/testutil"
 	"github.com/containerd/platforms"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
 func TestStandaloneConvertBasic(t *testing.T) {
@@ -255,10 +258,101 @@ func TestStandaloneConvertIdempotent(t *testing.T) {
 	}
 }
 
-func exportToOCIDir(sh *dockershell.Shell, imageRef, outputDir string) {
+func exportToOCIDir(sh *dockershell.Shell, imageRef, outputDir string, saveArgs ...string) {
 	exportTar := outputDir + ".export.tar"
-	sh.X("nerdctl", "save", "-o", exportTar, imageRef)
+	sh.X(append(append([]string{"nerdctl", "save"}, saveArgs...), "-o", exportTar, imageRef)...)
 	sh.X("mkdir", "-p", outputDir)
 	sh.X("tar", "-xf", exportTar, "-C", outputDir)
 	sh.X("rm", "-f", exportTar)
 }
+
+// Regression test: standalone conversion used to drop every platform after the
+// first whenever the input OCI layout contained multiple platforms.
+func TestStandaloneConvertAllPlatforms(t *testing.T) {
+	regConfig := newRegistryConfig()
+	sh, done := newShellWithRegistry(t, regConfig)
+	defer done()
+
+	rebootContainerd(t, sh, getContainerdConfigToml(t, false), getSnapshotterConfigToml(t))
+
+	// The test registry mirror only holds a single platform, so pull and save
+	// the multi-arch image directly from its public registry.
+	srcRef := dockerhub(nginxImage).ref
+
+	baseDir, err := testutil.TempDir(sh)
+	if err != nil {
+		t.Fatalf("failed to create temp dir: %v", err)
+	}
+	defer sh.X("rm", "-rf", baseDir)
+
+	inputDir := filepath.Join(baseDir, "input")
+	outputTar := filepath.Join(baseDir, "output.tar")
+
+	sh.X("nerdctl", "pull", "-q", "--all-platforms", srcRef)
+	exportToOCIDir(sh, srcRef, inputDir, "--all-platforms")
+
+	srcDigest := getImageDigest(sh, srcRef)
+	var srcPlatforms []string
+	for _, m := range readIndex(t, sh, srcDigest).Manifests {
+		if m.Platform != nil {
+			srcPlatforms = append(srcPlatforms, platforms.Format(*m.Platform))
+		}
+	}
+	if len(srcPlatforms) < 2 {
+		t.Skipf("expected multi-arch input, got %d platforms", len(srcPlatforms))
+	}
+
+	stopContainerd(t, sh)
+
+	sh.X("soci", "convert",
+		"--standalone",
+		"--all-platforms",
+		"--min-layer-size=0",
+		inputDir,
+		outputTar,
+	)
+
+	rebootContainerd(t, sh, getContainerdConfigToml(t, false), getSnapshotterConfigToml(t))
+
+	dstRef := srcRef + "-standalone-allplatforms"
+	sh.X("ctr", "images", "import", "--no-unpack", "--index-name", dstRef, outputTar)
+
+	dstDigest := getImageDigest(sh, dstRef)
+	validateConversion(t, sh, srcDigest, dstDigest)
+
+	imgs, sociIdx := map[string]bool{}, map[string]bool{}
+	for _, m := range readIndex(t, sh, dstDigest).Manifests {
+		if m.Platform == nil {
+			continue
+		}
+		key := platforms.Format(*m.Platform)
+		switch m.ArtifactType {
+		case soci.SociIndexArtifactTypeV2:
+			sociIdx[key] = true
+		case "":
+			imgs[key] = true
+		}
+	}
+	for _, p := range srcPlatforms {
+		if !imgs[p] {
+			t.Errorf("converted output missing image manifest for %s", p)
+		}
+		if !sociIdx[p] {
+			t.Errorf("converted output missing SOCI index for %s", p)
+		}
+	}
+}
+
+// readIndex fetches an OCI image index from the content store, unwrapping a
+// single-manifest wrapper (as produced by `ctr images import --index-name`).
+func readIndex(t *testing.T, sh *dockershell.Shell, indexDigest string) ocispec.Index {
+	t.Helper()
+	var idx ocispec.Index
+	if err := json.Unmarshal(sh.O("ctr", "content", "get", indexDigest), &idx); err != nil {
+		t.Fatalf("parse index %s: %v", indexDigest, err)
+	}
+	if len(idx.Manifests) == 1 && idx.Manifests[0].MediaType == ocispec.MediaTypeImageIndex {
+		return readIndex(t, sh, idx.Manifests[0].Digest.String())
+	}
+	return idx
+}