diff --git a/daemon/containerd/image_inspect.go b/daemon/containerd/image_inspect.go index b86d53a16c..8053a87ae8 100644 --- a/daemon/containerd/image_inspect.go +++ b/daemon/containerd/image_inspect.go @@ -2,6 +2,7 @@ package containerd import ( "context" + "fmt" "sync/atomic" "time" @@ -63,13 +64,6 @@ func (i *ImageService) ImageInspect(ctx context.Context, refOrID string, opts im } } - var img dockerspec.DockerOCIImage - if multi.Best != nil { - if err := multi.Best.ReadConfig(ctx, &img); err != nil { - return nil, err - } - } - parent, err := i.getImageLabelByDigest(ctx, target.Digest, imageLabelClassicBuilderParent) if err != nil { log.G(ctx).WithError(err).Warn("failed to determine Parent property") @@ -104,29 +98,36 @@ func (i *ImageService) ImageInspect(ctx context.Context, refOrID string, opts im GraphDriverLegacy: &storage.DriverData{Name: i.snapshotter}, } + var img dockerspec.DockerOCIImage if multi.Best != nil { - imgConfig := img.Config - resp.Author = img.Author - resp.Config = &imgConfig - resp.Architecture = img.Architecture - resp.Variant = img.Variant - resp.Os = img.OS - resp.OsVersion = img.OSVersion + if err := multi.Best.ReadConfig(ctx, &img); err != nil && !cerrdefs.IsNotFound(err) { + return nil, fmt.Errorf("failed to read image config: %w", err) + } + } - if len(img.History) > 0 { - resp.Comment = img.History[len(img.History)-1].Comment - } + // Copy the config + imgConfig := img.Config + resp.Config = &imgConfig - if img.Created != nil { - resp.Created = img.Created.Format(time.RFC3339Nano) - } + resp.Author = img.Author + resp.Architecture = img.Architecture + resp.Variant = img.Variant + resp.Os = img.OS + resp.OsVersion = img.OSVersion - resp.RootFS = imagetypes.RootFS{ - Type: img.RootFS.Type, - } - for _, layer := range img.RootFS.DiffIDs { - resp.RootFS.Layers = append(resp.RootFS.Layers, layer.String()) - } + if len(img.History) > 0 { + resp.Comment = img.History[len(img.History)-1].Comment + } + + if img.Created != nil { + resp.Created = img.Created.Format(time.RFC3339Nano) + } + + resp.RootFS = imagetypes.RootFS{ + Type: img.RootFS.Type, + } + for _, layer := range img.RootFS.DiffIDs { + resp.RootFS.Layers = append(resp.RootFS.Layers, layer.String()) } return resp, nil diff --git a/daemon/containerd/image_inspect_test.go b/daemon/containerd/image_inspect_test.go index c3ade5848e..d2dcbd1a15 100644 --- a/daemon/containerd/image_inspect_test.go +++ b/daemon/containerd/image_inspect_test.go @@ -61,6 +61,32 @@ func TestImageInspect(t *testing.T) { } }) + t.Run("inspect image with one layer missing", func(t *testing.T) { + ctx := logtest.WithT(ctx, t) + service := fakeImageService(t, ctx, cs) + + img := toContainerdImage(t, specialimage.MultiLayer) + + _, err := service.images.Create(ctx, img) + assert.NilError(t, err) + + // Get the manifest to access the layers + mfst, err := c8dimages.Manifest(ctx, cs, img.Target, nil) + assert.NilError(t, err) + assert.Check(t, len(mfst.Layers) > 0, "image should have at least one layer") + + // Delete the last layer from the content store + lastLayer := mfst.Layers[len(mfst.Layers)-1] + err = cs.Delete(ctx, lastLayer.Digest) + assert.NilError(t, err) + + inspect, err := service.ImageInspect(ctx, img.Name, imagebackend.ImageInspectOpts{}) + assert.NilError(t, err) + + assert.Check(t, inspect.Config != nil) + assert.Check(t, is.Len(inspect.RootFS.Layers, len(mfst.Layers))) + }) + t.Run("inspect image with platform parameter", func(t *testing.T) { ctx := logtest.WithT(ctx, t) service := fakeImageService(t, ctx, cs) diff --git a/daemon/containerd/image_list.go b/daemon/containerd/image_list.go index 5626e34750..c8941c5b08 100644 --- a/daemon/containerd/image_list.go +++ b/daemon/containerd/image_list.go @@ -305,34 +305,33 @@ func (i *ImageService) multiPlatformSummary(ctx context.Context, img c8dimages.I mfstSummary.ImageData.Platform = *target.Platform } - if !available { - return nil - } - var dockerImage dockerspec.DockerOCIImage - if err := img.ReadConfig(ctx, &dockerImage); err != nil { + if err := img.ReadConfig(ctx, &dockerImage); err != nil && !cerrdefs.IsNotFound(err) { logger.WithError(err).Warn("failed to read image config") - return nil } - if target.Platform == nil { - mfstSummary.ImageData.Platform = dockerImage.Platform + if dockerImage.Platform.OS != "" { + if target.Platform == nil { + mfstSummary.ImageData.Platform = dockerImage.Platform + } + logger = logger.WithField("platform", mfstSummary.ImageData.Platform) } - logger = logger.WithField("platform", mfstSummary.ImageData.Platform) - chainIDs := identity.ChainIDs(dockerImage.RootFS.DiffIDs) + if dockerImage.RootFS.DiffIDs != nil { + chainIDs := identity.ChainIDs(dockerImage.RootFS.DiffIDs) - snapshotUsage, err := img.SnapshotUsage(ctx, i.snapshotterService(i.snapshotter)) - if err != nil { - logger.WithFields(log.Fields{"error": err}).Warn("failed to determine platform specific unpacked size") + snapshotUsage, err := img.SnapshotUsage(ctx, i.snapshotterService(i.snapshotter)) + if err != nil { + logger.WithFields(log.Fields{"error": err}).Warn("failed to determine platform specific unpacked size") + } + unpackedSize := snapshotUsage.Size + + mfstSummary.ImageData.Size.Unpacked = unpackedSize + mfstSummary.Size.Total += unpackedSize + summary.TotalSize += unpackedSize + + summary.AllChainIDs = append(summary.AllChainIDs, chainIDs...) } - unpackedSize := snapshotUsage.Size - - mfstSummary.ImageData.Size.Unpacked = unpackedSize - mfstSummary.Size.Total += unpackedSize - summary.TotalSize += unpackedSize - - summary.AllChainIDs = append(summary.AllChainIDs, chainIDs...) for _, c := range i.containers.List() { if c.ImageManifest != nil && c.ImageManifest.Digest == target.Digest { @@ -360,8 +359,9 @@ func (i *ImageService) multiPlatformSummary(ctx context.Context, img c8dimages.I if err != nil { if errors.Is(err, errNotManifestOrIndex) { log.G(ctx).WithFields(log.Fields{ - "error": err, - "image": img.Name, + "error": err, + "image": img.Name, + "descriptor": img.Target, }).Warn("unexpected image target (neither a manifest nor index)") } else { return nil, err @@ -443,15 +443,6 @@ func (i *ImageService) singlePlatformImage(ctx context.Context, contentStore con } } - cfgDesc, err := imageManifest.Image.Config(ctx) - if err != nil { - return nil, err - } - var cfg configLabels - if err := readJSON(ctx, contentStore, cfgDesc, &cfg); err != nil { - return nil, err - } - var unpackedSize int64 if snapshotUsage, err := imageManifest.SnapshotUsage(ctx, i.snapshotterService(i.snapshotter)); err != nil { log.G(ctx).WithFields(log.Fields{"image": imageManifest.Name(), "error": err}).Warn("failed to calculate unpacked size of image") @@ -474,7 +465,6 @@ func (i *ImageService) singlePlatformImage(ctx context.Context, contentStore con RepoDigests: repoDigests, RepoTags: repoTags, Size: totalSize, - Labels: cfg.Config.Labels, // -1 indicates that the value has not been set (avoids ambiguity // between 0 (default) and "not set". We cannot use a pointer (nil) // for this, as the JSON representation uses "omitempty", which would @@ -482,9 +472,25 @@ func (i *ImageService) singlePlatformImage(ctx context.Context, contentStore con SharedSize: -1, Containers: -1, } + + var cfg configLabels + if err := imageManifest.ReadConfig(ctx, &cfg); err != nil { + if !cerrdefs.IsNotFound(err) { + log.G(ctx).WithFields(log.Fields{ + "image": imageManifest.Name(), + "error": err, + }).Warn("failed to read image config") + } + } + if cfg.Created != nil { summary.Created = cfg.Created.Unix() } + if cfg.Config.Labels != nil { + summary.Labels = cfg.Config.Labels + } else { + summary.Labels = map[string]string{} + } return summary, nil } diff --git a/integration/image/inspect_test.go b/integration/image/inspect_test.go index c11dbd4b81..0b6547ccd5 100644 --- a/integration/image/inspect_test.go +++ b/integration/image/inspect_test.go @@ -82,6 +82,74 @@ func TestImageInspectDescriptor(t *testing.T) { assert.Check(t, inspect.Descriptor.Size > 0) } +// Regression test for: https://github.com/moby/moby/issues/51566 +// +// This can be reproduced with two image that share the same uncompressed layer +// but have a different compressed blob is pulled. +// +// Example: +// ``` +// docker pull nginx@sha256:3b7732505933ca591ce4a6d860cb713ad96a3176b82f7979a8dfa9973486a0d6 +// docker pull gotenberg/gotenberg@sha256:b116a40a1c24917e2bf3e153692da5acd2e78e7cd67e1b2d243b47c178f31c90 +// ``` +// +// In this case, it's the base debian trixie image that's used as a base. +// They're effectively the same layer (unpacked diff ID +// `sha256:1d46119d249f7719e1820e24a311aa7c453f166f714969cffe89504678eaa447`), +// but different compressed blobs: +// +// # nginx +// { +// "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", +// "size": 29777766, +// "digest": "sha256:8c7716127147648c1751940b9709b6325f2256290d3201662eca2701cadb2cdf" +// } +// +// # gotenberg +// { +// "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", +// "size": 30781333, +// "digest": "sha256:b96413fb491a5ed179bb2746ff3be6cbddd72e14c6503bea80d58e579a3b92bc" +// }, +func TestImageInspectWithoutSomeBlobs(t *testing.T) { + t.Skip("TODO(vvoland): Come up with minimal images for this test") + + skip.If(t, testEnv.DaemonInfo.OSType != "linux", "The test images are Linux-only") + + ctx := setupTest(t) + apiClient := testEnv.APIClient() + + const baseImage = "nginx@sha256:3b7732505933ca591ce4a6d860cb713ad96a3176b82f7979a8dfa9973486a0d6" + const childImage = "gotenberg/gotenberg:8.24@sha256:b116a40a1c24917e2bf3e153692da5acd2e78e7cd67e1b2d243b47c178f31c90" + + // Pull the base image first and then the child image + for _, image := range []string{baseImage, childImage} { + rdr, err := apiClient.ImagePull(ctx, image, client.ImagePullOptions{}) + assert.NilError(t, err) + assert.NilError(t, rdr.Wait(ctx)) + + t.Cleanup(func() { + _, _ = apiClient.ImageRemove(ctx, image, client.ImageRemoveOptions{}) + }) + } + + var raw bytes.Buffer + inspect, err := apiClient.ImageInspect(ctx, childImage, client.ImageInspectWithRawResponse(&raw)) + assert.NilError(t, err) + + var rawJson map[string]any + err = json.Unmarshal(raw.Bytes(), &rawJson) + assert.NilError(t, err) + + configVal, hasConfig := rawJson["Config"] + assert.Check(t, hasConfig, "Config field should exist in JSON response") + if assert.Check(t, configVal != nil, "Config should not be null in JSON response") { + assert.Check(t, is.DeepEqual(inspect.Config.Cmd, []string{"gotenberg"})) + assert.Check(t, inspect.Os != "") + assert.Check(t, inspect.Architecture != "") + } +} + func TestImageInspectWithPlatform(t *testing.T) { skip.If(t, testEnv.DaemonInfo.OSType == "windows", "The test image is a Linux image") ctx := setupTest(t)