Browse code

c8d/load: Don't ignore missing platform when requested

Commit f143f4ec5148e71a10d074ed275af8f63caff339 introduced platform support
when loading images. However when loading a specific platform variant from
a tar that contains multiple, we should not ignore cases if that platform is
missing.

Before this patch, the missing platform was silently ignored, potentially
loading an empty image:

$ docker image load -i image.tar --platform=linux/riscv64
Loaded image: alpine:latest

$ docker image ls --tree
IMAGE ID DISK USAGE CONTENT SIZE USED
alpine:latest beefdbd8a1da 0B 0B

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>

Paweł Gronowski authored on 2024/10/22 00:44:13
Showing 2 changed files
... ...
@@ -256,8 +256,6 @@ func (i *ImageService) LoadImage(ctx context.Context, inTar io.ReadCloser, platf
256 256
 	opts := []containerd.ImportOpt{
257 257
 		containerd.WithImportPlatform(pm),
258 258
 
259
-		containerd.WithSkipMissing(),
260
-
261 259
 		// Create an additional image with dangling name for imported images...
262 260
 		containerd.WithDigestRef(danglingImageName),
263 261
 		// ... but only if they don't have a name or it's invalid.
... ...
@@ -270,13 +268,51 @@ func (i *ImageService) LoadImage(ctx context.Context, inTar io.ReadCloser, platf
270 270
 		}),
271 271
 	}
272 272
 
273
+	if platform == nil {
274
+		// Allow variants to be missing if no specific platform is requested.
275
+		opts = append(opts, containerd.WithSkipMissing())
276
+	}
277
+
273 278
 	imgs, err := i.client.Import(ctx, decompressed, opts...)
274 279
 	if err != nil {
280
+		if platform != nil {
281
+			p := platforms.FormatAll(*platform)
282
+			log.G(ctx).WithFields(log.Fields{"error": err, "platform": p}).Debug("failed to import image to containerd")
283
+
284
+			// Note: ErrEmptyWalk will not be returned in most cases as
285
+			// index.json will contain a descriptor of the actual OCI index or
286
+			// Docker manifest list, so the walk is never empty.
287
+			// Even in case of a single-platform image, the manifest descriptor
288
+			// doesn't have a platform set, so it won't be filtered out by the
289
+			// FilterPlatform containerd handler.
290
+			if errors.Is(err, containerdimages.ErrEmptyWalk) {
291
+				return errdefs.NotFound(errors.Wrapf(err, "requested platform (%s) not found", p))
292
+			}
293
+			if cerrdefs.IsNotFound(err) {
294
+				return errdefs.NotFound(errors.Wrapf(err, "requested platform (%s) found, but some content is missing", p))
295
+			}
296
+		}
275 297
 		log.G(ctx).WithError(err).Debug("failed to import image to containerd")
276 298
 		return errdefs.System(err)
277 299
 	}
278 300
 
301
+	if platform != nil {
302
+		// Verify that the requested platform is available for the loaded images.
303
+		// While the ideal behavior here would be to verify whether the input
304
+		// archive actually supplied them, we're not able to determine that
305
+		// as the imported index is not returned by the import operation.
306
+		if err := i.verifyImagesProvidePlatform(ctx, imgs, *platform, pm); err != nil {
307
+			return err
308
+		}
309
+	}
310
+
279 311
 	progress := streamformatter.NewStdoutWriter(outStream)
312
+	// Unpack only an image of the host platform
313
+	unpackPm := i.hostPlatformMatcher()
314
+	// If a load of specific platform is requested, unpack it
315
+	if platform != nil {
316
+		unpackPm = pm
317
+	}
280 318
 
281 319
 	for _, img := range imgs {
282 320
 		name := img.Name
... ...
@@ -310,8 +346,7 @@ func (i *ImageService) LoadImage(ctx context.Context, inTar io.ReadCloser, platf
310 310
 				return nil
311 311
 			}
312 312
 
313
-			// Only unpack the image if it matches the host platform
314
-			if !i.hostPlatformMatcher().Match(imgPlat) {
313
+			if !unpackPm.Match(imgPlat) {
315 314
 				return nil
316 315
 			}
317 316
 
... ...
@@ -342,3 +377,57 @@ func (i *ImageService) LoadImage(ctx context.Context, inTar io.ReadCloser, platf
342 342
 
343 343
 	return nil
344 344
 }
345
+
346
+// verifyImagesProvidePlatform checks if the requested platform is loaded.
347
+// If the requested platform is not loaded, it returns an error.
348
+func (i *ImageService) verifyImagesProvidePlatform(ctx context.Context, imgs []containerdimages.Image, platform ocispec.Platform, pm platforms.Matcher) error {
349
+	if len(imgs) == 0 {
350
+		return errdefs.NotFound(fmt.Errorf("no images providing the requested platform %s found", platforms.FormatAll(platform)))
351
+	}
352
+	var incompleteImgs []string
353
+	for _, img := range imgs {
354
+		hasRequestedPlatform := false
355
+		err := i.walkImageManifests(ctx, img, func(platformImg *ImageManifest) error {
356
+			imgPlat, err := platformImg.ImagePlatform(ctx)
357
+			if err != nil {
358
+				if cerrdefs.IsNotFound(err) {
359
+					return nil
360
+				}
361
+				return errors.Wrapf(err, "failed to determine image platform")
362
+			}
363
+
364
+			if !pm.Match(imgPlat) {
365
+				return nil
366
+			}
367
+			available, err := platformImg.CheckContentAvailable(ctx)
368
+			if err != nil {
369
+				return errors.Wrapf(err, "failed to determine image content availability for platform %s", platforms.FormatAll(platform))
370
+			}
371
+
372
+			if available {
373
+				hasRequestedPlatform = true
374
+				return nil
375
+			}
376
+			return nil
377
+		})
378
+		if err != nil {
379
+			return errdefs.System(err)
380
+		}
381
+		if !hasRequestedPlatform {
382
+			incompleteImgs = append(incompleteImgs, imageFamiliarName(img))
383
+		}
384
+	}
385
+
386
+	msg := ""
387
+	switch len(incompleteImgs) {
388
+	case 0:
389
+		// Success - All images provide the requested platform.
390
+		return nil
391
+	case 1:
392
+		msg = "image %s was loaded, but doesn't provide the requested platform (%s)"
393
+	default:
394
+		msg = "images [%s] were loaded, but don't provide the requested platform (%s)"
395
+	}
396
+
397
+	return errdefs.NotFound(fmt.Errorf(msg, strings.Join(incompleteImgs, ", "), platforms.FormatAll(platform)))
398
+}
345 399
new file mode 100644
... ...
@@ -0,0 +1,111 @@
0
+package containerd
1
+
2
+import (
3
+	"bytes"
4
+	"context"
5
+	"math/rand"
6
+	"os"
7
+	"path/filepath"
8
+	"testing"
9
+
10
+	"github.com/containerd/containerd/content"
11
+	"github.com/containerd/containerd/content/local"
12
+	"github.com/containerd/containerd/namespaces"
13
+	"github.com/containerd/platforms"
14
+	"github.com/docker/docker/errdefs"
15
+	"github.com/docker/docker/internal/testutils/specialimage"
16
+	"github.com/docker/docker/pkg/archive"
17
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
18
+	"gotest.tools/v3/assert"
19
+	is "gotest.tools/v3/assert/cmp"
20
+)
21
+
22
+func TestImageLoadMissing(t *testing.T) {
23
+	linuxAmd64 := ocispec.Platform{OS: "linux", Architecture: "amd64"}
24
+	linuxArm64 := ocispec.Platform{OS: "linux", Architecture: "arm64"}
25
+	linuxArmv5 := ocispec.Platform{OS: "linux", Architecture: "arm", Variant: "v5"}
26
+
27
+	ctx := namespaces.WithNamespace(context.TODO(), "testing-"+t.Name())
28
+
29
+	store, err := local.NewLabeledStore(t.TempDir(), &memoryLabelStore{})
30
+	assert.NilError(t, err)
31
+
32
+	imgSvc := fakeImageService(t, ctx, store)
33
+	// Mock the daemon platform.
34
+	imgSvc.defaultPlatformOverride = platforms.Only(linuxAmd64)
35
+
36
+	tryLoad := func(ctx context.Context, t *testing.T, dir string, platform ocispec.Platform) error {
37
+		tarRc, err := archive.Tar(dir, archive.Uncompressed)
38
+		assert.NilError(t, err)
39
+		defer tarRc.Close()
40
+
41
+		buf := bytes.Buffer{}
42
+
43
+		defer func() {
44
+			t.Log(buf.String())
45
+		}()
46
+
47
+		return imgSvc.LoadImage(ctx, tarRc, &platform, &buf, true)
48
+	}
49
+
50
+	clearStore := func(ctx context.Context, t *testing.T) {
51
+		assert.NilError(t, store.Walk(ctx, func(info content.Info) error {
52
+			return store.Delete(ctx, info.Digest)
53
+		}), "failed to delete all content")
54
+	}
55
+
56
+	t.Run("empty index", func(t *testing.T) {
57
+		imgDataDir := t.TempDir()
58
+		_, err := specialimage.EmptyIndex(imgDataDir)
59
+		assert.NilError(t, err)
60
+
61
+		err = tryLoad(ctx, t, imgDataDir, linuxAmd64)
62
+		assert.Check(t, is.Error(err, "image emptyindex:latest was loaded, but doesn't provide the requested platform (linux/amd64)"))
63
+		assert.Check(t, is.ErrorType(err, errdefs.IsNotFound))
64
+	})
65
+	clearStore(ctx, t)
66
+
67
+	t.Run("single platform", func(t *testing.T) {
68
+		imgDataDir := t.TempDir()
69
+		r := rand.NewSource(0x9127371238)
70
+		_, err := specialimage.RandomSinglePlatform(imgDataDir, linuxAmd64, r)
71
+		assert.NilError(t, err)
72
+
73
+		err = tryLoad(ctx, t, imgDataDir, linuxArm64)
74
+		assert.Check(t, is.ErrorContains(err, "doesn't provide the requested platform (linux/arm64)"))
75
+		assert.Check(t, is.ErrorType(err, errdefs.IsNotFound))
76
+	})
77
+
78
+	clearStore(ctx, t)
79
+
80
+	t.Run("2 platform image", func(t *testing.T) {
81
+		imgDataDir := t.TempDir()
82
+		_, mfstDescs, err := specialimage.MultiPlatform(imgDataDir, "multiplatform:latest", []ocispec.Platform{linuxAmd64, linuxArm64})
83
+		assert.NilError(t, err)
84
+
85
+		t.Run("platform not included in index", func(t *testing.T) {
86
+			err = tryLoad(ctx, t, imgDataDir, linuxArmv5)
87
+			assert.Check(t, is.Error(err, "image multiplatform:latest was loaded, but doesn't provide the requested platform (linux/arm/v5)"))
88
+			assert.Check(t, is.ErrorType(err, errdefs.IsNotFound))
89
+		})
90
+
91
+		clearStore(ctx, t)
92
+
93
+		t.Run("platform blobs missing", func(t *testing.T) {
94
+			// Assumption: arm64 image is second in the index (implementation detail of specialimage.MultiPlatform)
95
+			mfstDesc := mfstDescs[1]
96
+			assert.Assert(t, mfstDesc.Platform.Architecture == linuxArm64.Architecture)
97
+			assert.Assert(t, mfstDesc.Platform.Variant == linuxArm64.Variant)
98
+
99
+			t.Log(mfstDesc.Digest)
100
+
101
+			// Delete arm64 manifest
102
+			mfstPath := filepath.Join(imgDataDir, "blobs/sha256", mfstDesc.Digest.Encoded())
103
+			assert.NilError(t, os.Remove(mfstPath))
104
+
105
+			err = tryLoad(ctx, t, imgDataDir, linuxArm64)
106
+			assert.Check(t, is.ErrorContains(err, "requested platform (linux/arm64) found, but some content is missing"))
107
+			assert.Check(t, is.ErrorType(err, errdefs.IsNotFound))
108
+		})
109
+	})
110
+}