File CVE-2024-24557-moby-cache-poisoning.patch of Package docker
From 537348763faeb7adc515305e87910e11bdb14b00 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= <pawel.gronowski@docker.com>
Date: Fri, 1 Dec 2023 10:24:23 +0100
Subject: [PATCH 1/5] image/cache: Compare all config fields
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Add checks for some image config fields that were missing.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
---
daemon/containerd/cache.go | 61 +---------------
image/cache/compare.go | 146 +++++++++++++++++++++++++++++++------
2 files changed, 124 insertions(+), 83 deletions(-)
diff --git a/daemon/containerd/cache.go b/daemon/containerd/cache.go
index 1fbf32c669e7f..9f957a7696886 100644
--- a/daemon/containerd/cache.go
+++ b/daemon/containerd/cache.go
@@ -11,6 +11,7 @@ import (
"github.com/docker/docker/builder"
"github.com/docker/docker/errdefs"
"github.com/docker/docker/image"
+ "github.com/docker/docker/image/cache"
"github.com/opencontainers/go-digest"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
)
@@ -98,7 +99,7 @@ func (ic *localCache) GetCache(parentID string, cfg *container.Config) (imageID
return "", err
}
- if isMatch(&cc, cfg) {
+ if cache.CompareConfig(&cc, cfg) {
childImage, err := ic.imageService.GetImage(ctx, child.String(), backend.GetImageOpts{})
if err != nil {
return "", err
@@ -206,61 +207,3 @@ func (ic *imageCache) isParent(ctx context.Context, img *image.Image, parentID i
}
return ic.isParent(ctx, p, parentID)
}
-
-// compare two Config struct. Do not compare the "Image" nor "Hostname" fields
-// If OpenStdin is set, then it differs
-func isMatch(a, b *container.Config) bool {
- if a == nil || b == nil ||
- a.OpenStdin || b.OpenStdin {
- return false
- }
- if a.AttachStdout != b.AttachStdout ||
- a.AttachStderr != b.AttachStderr ||
- a.User != b.User ||
- a.OpenStdin != b.OpenStdin ||
- a.Tty != b.Tty {
- return false
- }
-
- if len(a.Cmd) != len(b.Cmd) ||
- len(a.Env) != len(b.Env) ||
- len(a.Labels) != len(b.Labels) ||
- len(a.ExposedPorts) != len(b.ExposedPorts) ||
- len(a.Entrypoint) != len(b.Entrypoint) ||
- len(a.Volumes) != len(b.Volumes) {
- return false
- }
-
- for i := 0; i < len(a.Cmd); i++ {
- if a.Cmd[i] != b.Cmd[i] {
- return false
- }
- }
- for i := 0; i < len(a.Env); i++ {
- if a.Env[i] != b.Env[i] {
- return false
- }
- }
- for k, v := range a.Labels {
- if v != b.Labels[k] {
- return false
- }
- }
- for k := range a.ExposedPorts {
- if _, exists := b.ExposedPorts[k]; !exists {
- return false
- }
- }
-
- for i := 0; i < len(a.Entrypoint); i++ {
- if a.Entrypoint[i] != b.Entrypoint[i] {
- return false
- }
- }
- for key := range a.Volumes {
- if _, exists := b.Volumes[key]; !exists {
- return false
- }
- }
- return true
-}
diff --git a/image/cache/compare.go b/image/cache/compare.go
index e31e9c8bdf799..ec1971a207829 100644
--- a/image/cache/compare.go
+++ b/image/cache/compare.go
@@ -4,42 +4,69 @@ import (
"github.com/docker/docker/api/types/container"
)
-// compare two Config struct. Do not compare the "Image" nor "Hostname" fields
-// If OpenStdin is set, then it differs
+// TODO: Remove once containerd image service directly uses the ImageCache and
+// LocalImageCache structs.
+func CompareConfig(a, b *container.Config) bool {
+ return compare(a, b)
+}
+
+// compare two Config struct. Do not container-specific fields:
+// - Image
+// - Hostname
+// - Domainname
+// - MacAddress
func compare(a, b *container.Config) bool {
- if a == nil || b == nil ||
- a.OpenStdin || b.OpenStdin {
+ if a == nil || b == nil {
+ return false
+ }
+
+ if len(a.Env) != len(b.Env) {
return false
}
- if a.AttachStdout != b.AttachStdout ||
- a.AttachStderr != b.AttachStderr ||
- a.User != b.User ||
- a.OpenStdin != b.OpenStdin ||
- a.Tty != b.Tty {
+ if len(a.Cmd) != len(b.Cmd) {
return false
}
-
- if len(a.Cmd) != len(b.Cmd) ||
- len(a.Env) != len(b.Env) ||
- len(a.Labels) != len(b.Labels) ||
- len(a.ExposedPorts) != len(b.ExposedPorts) ||
- len(a.Entrypoint) != len(b.Entrypoint) ||
- len(a.Volumes) != len(b.Volumes) {
+ if len(a.Entrypoint) != len(b.Entrypoint) {
+ return false
+ }
+ if len(a.Shell) != len(b.Shell) {
+ return false
+ }
+ if len(a.ExposedPorts) != len(b.ExposedPorts) {
+ return false
+ }
+ if len(a.Volumes) != len(b.Volumes) {
+ return false
+ }
+ if len(a.Labels) != len(b.Labels) {
+ return false
+ }
+ if len(a.OnBuild) != len(b.OnBuild) {
return false
}
+ for i := 0; i < len(a.Env); i++ {
+ if a.Env[i] != b.Env[i] {
+ return false
+ }
+ }
+ for i := 0; i < len(a.OnBuild); i++ {
+ if a.OnBuild[i] != b.OnBuild[i] {
+ return false
+ }
+ }
for i := 0; i < len(a.Cmd); i++ {
if a.Cmd[i] != b.Cmd[i] {
return false
}
}
- for i := 0; i < len(a.Env); i++ {
- if a.Env[i] != b.Env[i] {
+ for i := 0; i < len(a.Entrypoint); i++ {
+ if a.Entrypoint[i] != b.Entrypoint[i] {
return false
}
}
- for k, v := range a.Labels {
- if v != b.Labels[k] {
+ for i := 0; i < len(a.Shell); i++ {
+ if a.Shell[i] != b.Shell[i] {
return false
}
}
@@ -48,16 +75,87 @@ func compare(a, b *container.Config) bool {
return false
}
}
+ for key := range a.Volumes {
+ if _, exists := b.Volumes[key]; !exists {
+ return false
+ }
+ }
+ for k, v := range a.Labels {
+ if v != b.Labels[k] {
+ return false
+ }
+ }
- for i := 0; i < len(a.Entrypoint); i++ {
- if a.Entrypoint[i] != b.Entrypoint[i] {
+ if a.AttachStdin != b.AttachStdin {
+ return false
+ }
+ if a.AttachStdout != b.AttachStdout {
+ return false
+ }
+ if a.AttachStderr != b.AttachStderr {
+ return false
+ }
+ if a.NetworkDisabled != b.NetworkDisabled {
+ return false
+ }
+ if a.Tty != b.Tty {
+ return false
+ }
+ if a.OpenStdin != b.OpenStdin {
+ return false
+ }
+ if a.StdinOnce != b.StdinOnce {
+ return false
+ }
+ if a.ArgsEscaped != b.ArgsEscaped {
+ return false
+ }
+ if a.User != b.User {
+ return false
+ }
+ if a.WorkingDir != b.WorkingDir {
+ return false
+ }
+ if a.StopSignal != b.StopSignal {
+ return false
+ }
+
+ if (a.StopTimeout == nil) != (b.StopTimeout == nil) {
+ return false
+ }
+ if a.StopTimeout != nil && b.StopTimeout != nil {
+ if *a.StopTimeout != *b.StopTimeout {
return false
}
}
- for key := range a.Volumes {
- if _, exists := b.Volumes[key]; !exists {
+ if (a.Healthcheck == nil) != (b.Healthcheck == nil) {
+ return false
+ }
+ if a.Healthcheck != nil && b.Healthcheck != nil {
+ if a.Healthcheck.Interval != b.Healthcheck.Interval {
+ return false
+ }
+ if a.Healthcheck.StartInterval != b.Healthcheck.StartInterval {
return false
}
+ if a.Healthcheck.StartPeriod != b.Healthcheck.StartPeriod {
+ return false
+ }
+ if a.Healthcheck.Timeout != b.Healthcheck.Timeout {
+ return false
+ }
+ if a.Healthcheck.Retries != b.Healthcheck.Retries {
+ return false
+ }
+ if len(a.Healthcheck.Test) != len(b.Healthcheck.Test) {
+ return false
+ }
+ for i := 0; i < len(a.Healthcheck.Test); i++ {
+ if a.Healthcheck.Test[i] != b.Healthcheck.Test[i] {
+ return false
+ }
+ }
}
+
return true
}
From c6156dc51bb74eef1b606376c576ce944b97bac6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= <pawel.gronowski@docker.com>
Date: Fri, 1 Dec 2023 10:24:25 +0100
Subject: [PATCH 2/5] daemon/imageStore: Mark images built locally
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Store additional image property which makes it possible to distinguish
if image was built locally.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
---
daemon/images/image_builder.go | 3 +++
daemon/images/image_commit.go | 3 +++
image/store.go | 20 ++++++++++++++++++++
3 files changed, 26 insertions(+)
diff --git a/daemon/images/image_builder.go b/daemon/images/image_builder.go
index 8849fc9ac27d0..8e902ed7eed78 100644
--- a/daemon/images/image_builder.go
+++ b/daemon/images/image_builder.go
@@ -254,6 +254,9 @@ func (i *ImageService) CreateImage(ctx context.Context, config []byte, parent st
return nil, errors.Wrapf(err, "failed to set parent %s", parent)
}
}
+ if err := i.imageStore.SetBuiltLocally(id); err != nil {
+ return nil, errors.Wrapf(err, "failed to mark image %s as built locally", id)
+ }
return i.imageStore.Get(id)
}
diff --git a/daemon/images/image_commit.go b/daemon/images/image_commit.go
index f620b41790684..00ce4fbc07c5a 100644
--- a/daemon/images/image_commit.go
+++ b/daemon/images/image_commit.go
@@ -62,6 +62,9 @@ func (i *ImageService) CommitImage(ctx context.Context, c backend.CommitConfig)
if err != nil {
return "", err
}
+ if err := i.imageStore.SetBuiltLocally(id); err != nil {
+ return "", err
+ }
if c.ParentImageID != "" {
if err := i.imageStore.SetParent(id, image.ID(c.ParentImageID)); err != nil {
diff --git a/image/store.go b/image/store.go
index 41a720bd02c7c..3671436125624 100644
--- a/image/store.go
+++ b/image/store.go
@@ -3,6 +3,7 @@ package image // import "github.com/docker/docker/image"
import (
"context"
"fmt"
+ "os"
"sync"
"time"
@@ -24,6 +25,8 @@ type Store interface {
GetParent(id ID) (ID, error)
SetLastUpdated(id ID) error
GetLastUpdated(id ID) (time.Time, error)
+ SetBuiltLocally(id ID) error
+ IsBuiltLocally(id ID) (bool, error)
Children(id ID) []ID
Map() map[ID]*Image
Heads() map[ID]*Image
@@ -295,6 +298,23 @@ func (is *store) GetLastUpdated(id ID) (time.Time, error) {
return time.Parse(time.RFC3339Nano, string(bytes))
}
+// SetBuiltLocally sets whether image can be used as a builder cache
+func (is *store) SetBuiltLocally(id ID) error {
+ return is.fs.SetMetadata(id.Digest(), "builtLocally", []byte{1})
+}
+
+// IsBuiltLocally returns whether image can be used as a builder cache
+func (is *store) IsBuiltLocally(id ID) (bool, error) {
+ bytes, err := is.fs.GetMetadata(id.Digest(), "builtLocally")
+ if err != nil || len(bytes) == 0 {
+ if errors.Is(err, os.ErrNotExist) {
+ err = nil
+ }
+ return false, err
+ }
+ return bytes[0] == 1, nil
+}
+
func (is *store) Children(id ID) []ID {
is.RLock()
defer is.RUnlock()
From 96ac22768a8a745b1fb5e4531eaca8d2ad7327ad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= <pawel.gronowski@docker.com>
Date: Fri, 1 Dec 2023 10:24:27 +0100
Subject: [PATCH 3/5] image/cache: Restrict cache candidates to locally built
images
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Restrict cache candidates only to images that were built locally.
This doesn't affect builds using `--cache-from`.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
---
image/cache/cache.go | 56 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 54 insertions(+), 2 deletions(-)
diff --git a/image/cache/cache.go b/image/cache/cache.go
index b36534e5725af..7bc4089a41300 100644
--- a/image/cache/cache.go
+++ b/image/cache/cache.go
@@ -1,11 +1,13 @@
package cache // import "github.com/docker/docker/image/cache"
import (
+ "context"
"encoding/json"
"fmt"
"reflect"
"strings"
+ "github.com/containerd/log"
containertypes "github.com/docker/docker/api/types/container"
"github.com/docker/docker/dockerversion"
"github.com/docker/docker/image"
@@ -216,6 +218,22 @@ func getImageIDAndError(img *image.Image, err error) (string, error) {
// created. nil is returned if a child cannot be found. An error is
// returned if the parent image cannot be found.
func getLocalCachedImage(imageStore image.Store, imgID image.ID, config *containertypes.Config) (*image.Image, error) {
+ if config == nil {
+ return nil, nil
+ }
+
+ isBuiltLocally := func(id image.ID) bool {
+ builtLocally, err := imageStore.IsBuiltLocally(id)
+ if err != nil {
+ log.G(context.TODO()).WithFields(log.Fields{
+ "error": err,
+ "id": id,
+ }).Warn("failed to check if image was built locally")
+ return false
+ }
+ return builtLocally
+ }
+
// Loop on the children of the given image and check the config
getMatch := func(siblings []image.ID) (*image.Image, error) {
var match *image.Image
@@ -225,6 +243,10 @@ func getLocalCachedImage(imageStore image.Store, imgID image.ID, config *contain
return nil, fmt.Errorf("unable to find image %q", id)
}
+ if !isBuiltLocally(id) {
+ continue
+ }
+
if compare(&img.ContainerConfig, config) {
// check for the most up to date match
if img.Created != nil && (match == nil || match.Created.Before(*img.Created)) {
@@ -238,11 +260,29 @@ func getLocalCachedImage(imageStore image.Store, imgID image.ID, config *contain
// In this case, this is `FROM scratch`, which isn't an actual image.
if imgID == "" {
images := imageStore.Map()
+
var siblings []image.ID
for id, img := range images {
- if img.Parent == imgID {
- siblings = append(siblings, id)
+ if img.Parent != "" {
+ continue
+ }
+
+ if !isBuiltLocally(id) {
+ continue
}
+
+ // Do a quick initial filter on the Cmd to avoid adding all
+ // non-local images with empty parent to the siblings slice and
+ // performing a full config compare.
+ //
+ // config.Cmd is set to the current Dockerfile instruction so we
+ // check it against the img.ContainerConfig.Cmd which is the
+ // command of the last layer.
+ if !strSliceEqual(img.ContainerConfig.Cmd, config.Cmd) {
+ continue
+ }
+
+ siblings = append(siblings, id)
}
return getMatch(siblings)
}
@@ -251,3 +291,15 @@ func getLocalCachedImage(imageStore image.Store, imgID image.ID, config *contain
siblings := imageStore.Children(imgID)
return getMatch(siblings)
}
+
+func strSliceEqual(a, b []string) bool {
+ if len(a) != len(b) {
+ return false
+ }
+ for i := 0; i < len(a); i++ {
+ if a[i] != b[i] {
+ return false
+ }
+ }
+ return true
+}
From 877ebbe038ee914e4243d5b7e198eda00494d757 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= <pawel.gronowski@docker.com>
Date: Fri, 1 Dec 2023 10:24:29 +0100
Subject: [PATCH 4/5] image/cache: Check image platform
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Make sure the cache candidate platform matches the requested.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
---
builder/builder.go | 3 ++-
builder/dockerfile/copy.go | 17 ++---------------
builder/dockerfile/imageprobe.go | 9 +++++----
builder/dockerfile/internals.go | 17 ++++++++++++++++-
builder/dockerfile/mockbackend_test.go | 3 ++-
daemon/containerd/cache.go | 13 ++++++++-----
image/cache/cache.go | 21 ++++++++++++++++-----
7 files changed, 51 insertions(+), 32 deletions(-)
diff --git a/builder/builder.go b/builder/builder.go
index ab3d2a9f25545..fc855f133d8e5 100644
--- a/builder/builder.go
+++ b/builder/builder.go
@@ -14,6 +14,7 @@ import (
"github.com/docker/docker/image"
"github.com/docker/docker/layer"
"github.com/opencontainers/go-digest"
+ ocispec "github.com/opencontainers/image-spec/specs-go/v1"
)
const (
@@ -85,7 +86,7 @@ type ImageCacheBuilder interface {
type ImageCache interface {
// GetCache returns a reference to a cached image whose parent equals `parent`
// and runconfig equals `cfg`. A cache miss is expected to return an empty ID and a nil error.
- GetCache(parentID string, cfg *container.Config) (imageID string, err error)
+ GetCache(parentID string, cfg *container.Config, platform ocispec.Platform) (imageID string, err error)
}
// Image represents a Docker image used by the builder.
diff --git a/builder/dockerfile/copy.go b/builder/dockerfile/copy.go
index 42b4808db4c9b..62fbfb656d260 100644
--- a/builder/dockerfile/copy.go
+++ b/builder/dockerfile/copy.go
@@ -8,7 +8,6 @@ import (
"net/url"
"os"
"path/filepath"
- "runtime"
"sort"
"strings"
"time"
@@ -74,7 +73,7 @@ type copier struct {
source builder.Source
pathCache pathCache
download sourceDownloader
- platform *ocispec.Platform
+ platform ocispec.Platform
// for cleanup. TODO: having copier.cleanup() is error prone and hard to
// follow. Code calling performCopy should manage the lifecycle of its params.
// Copier should take override source as input, not imageMount.
@@ -83,19 +82,7 @@ type copier struct {
}
func copierFromDispatchRequest(req dispatchRequest, download sourceDownloader, imageSource *imageMount) copier {
- platform := req.builder.platform
- if platform == nil {
- // May be nil if not explicitly set in API/dockerfile
- platform = &ocispec.Platform{}
- }
- if platform.OS == "" {
- // Default to the dispatch requests operating system if not explicit in API/dockerfile
- platform.OS = req.state.operatingSystem
- }
- if platform.OS == "" {
- // This is a failsafe just in case. Shouldn't be hit.
- platform.OS = runtime.GOOS
- }
+ platform := req.builder.getPlatform(req.state)
return copier{
source: req.source,
diff --git a/builder/dockerfile/imageprobe.go b/builder/dockerfile/imageprobe.go
index 18c25d6c0c629..de00c912e8554 100644
--- a/builder/dockerfile/imageprobe.go
+++ b/builder/dockerfile/imageprobe.go
@@ -6,13 +6,14 @@ import (
"github.com/containerd/log"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/builder"
+ ocispec "github.com/opencontainers/image-spec/specs-go/v1"
)
// ImageProber exposes an Image cache to the Builder. It supports resetting a
// cache.
type ImageProber interface {
Reset(ctx context.Context) error
- Probe(parentID string, runConfig *container.Config) (string, error)
+ Probe(parentID string, runConfig *container.Config, platform ocispec.Platform) (string, error)
}
type resetFunc func(context.Context) (builder.ImageCache, error)
@@ -51,11 +52,11 @@ func (c *imageProber) Reset(ctx context.Context) error {
// Probe checks if cache match can be found for current build instruction.
// It returns the cachedID if there is a hit, and the empty string on miss
-func (c *imageProber) Probe(parentID string, runConfig *container.Config) (string, error) {
+func (c *imageProber) Probe(parentID string, runConfig *container.Config, platform ocispec.Platform) (string, error) {
if c.cacheBusted {
return "", nil
}
- cacheID, err := c.cache.GetCache(parentID, runConfig)
+ cacheID, err := c.cache.GetCache(parentID, runConfig, platform)
if err != nil {
return "", err
}
@@ -74,6 +75,6 @@ func (c *nopProber) Reset(ctx context.Context) error {
return nil
}
-func (c *nopProber) Probe(_ string, _ *container.Config) (string, error) {
+func (c *nopProber) Probe(_ string, _ *container.Config, _ ocispec.Platform) (string, error) {
return "", nil
}
diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go
index efe7c66e0cd24..b6b0b8f42a28d 100644
--- a/builder/dockerfile/internals.go
+++ b/builder/dockerfile/internals.go
@@ -10,6 +10,7 @@ import (
"fmt"
"strings"
+ "github.com/containerd/containerd/platforms"
"github.com/containerd/log"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/backend"
@@ -328,7 +329,7 @@ func getShell(c *container.Config, os string) []string {
}
func (b *Builder) probeCache(dispatchState *dispatchState, runConfig *container.Config) (bool, error) {
- cachedID, err := b.imageProber.Probe(dispatchState.imageID, runConfig)
+ cachedID, err := b.imageProber.Probe(dispatchState.imageID, runConfig, b.getPlatform(dispatchState))
if cachedID == "" || err != nil {
return false, err
}
@@ -388,3 +389,17 @@ func hostConfigFromOptions(options *types.ImageBuildOptions) *container.HostConf
}
return hc
}
+
+func (b *Builder) getPlatform(state *dispatchState) ocispec.Platform {
+ // May be nil if not explicitly set in API/dockerfile
+ out := platforms.DefaultSpec()
+ if b.platform != nil {
+ out = *b.platform
+ }
+
+ if state.operatingSystem != "" {
+ out.OS = state.operatingSystem
+ }
+
+ return out
+}
diff --git a/builder/dockerfile/mockbackend_test.go b/builder/dockerfile/mockbackend_test.go
index 41c2e045e0e9f..e21e5b9ba5567 100644
--- a/builder/dockerfile/mockbackend_test.go
+++ b/builder/dockerfile/mockbackend_test.go
@@ -13,6 +13,7 @@ import (
"github.com/docker/docker/image"
"github.com/docker/docker/layer"
"github.com/opencontainers/go-digest"
+ ocispec "github.com/opencontainers/image-spec/specs-go/v1"
)
// MockBackend implements the builder.Backend interface for unit testing
@@ -106,7 +107,7 @@ type mockImageCache struct {
getCacheFunc func(parentID string, cfg *container.Config) (string, error)
}
-func (mic *mockImageCache) GetCache(parentID string, cfg *container.Config) (string, error) {
+func (mic *mockImageCache) GetCache(parentID string, cfg *container.Config, _ ocispec.Platform) (string, error) {
if mic.getCacheFunc != nil {
return mic.getCacheFunc(parentID, cfg)
}
diff --git a/daemon/containerd/cache.go b/daemon/containerd/cache.go
index 9f957a7696886..966d7b8b54be1 100644
--- a/daemon/containerd/cache.go
+++ b/daemon/containerd/cache.go
@@ -54,7 +54,7 @@ type localCache struct {
imageService *ImageService
}
-func (ic *localCache) GetCache(parentID string, cfg *container.Config) (imageID string, err error) {
+func (ic *localCache) GetCache(parentID string, cfg *container.Config, platform ocispec.Platform) (imageID string, err error) {
ctx := context.TODO()
var children []image.ID
@@ -100,8 +100,11 @@ func (ic *localCache) GetCache(parentID string, cfg *container.Config) (imageID
}
if cache.CompareConfig(&cc, cfg) {
- childImage, err := ic.imageService.GetImage(ctx, child.String(), backend.GetImageOpts{})
+ childImage, err := ic.imageService.GetImage(ctx, child.String(), backend.GetImageOpts{Platform: &platform})
if err != nil {
+ if errdefs.IsNotFound(err) {
+ continue
+ }
return "", err
}
@@ -124,10 +127,10 @@ type imageCache struct {
lc *localCache
}
-func (ic *imageCache) GetCache(parentID string, cfg *container.Config) (imageID string, err error) {
+func (ic *imageCache) GetCache(parentID string, cfg *container.Config, platform ocispec.Platform) (imageID string, err error) {
ctx := context.TODO()
- imgID, err := ic.lc.GetCache(parentID, cfg)
+ imgID, err := ic.lc.GetCache(parentID, cfg, platform)
if err != nil {
return "", err
}
@@ -143,7 +146,7 @@ func (ic *imageCache) GetCache(parentID string, cfg *container.Config) (imageID
lenHistory := 0
if parentID != "" {
- parent, err = ic.imageService.GetImage(ctx, parentID, backend.GetImageOpts{})
+ parent, err = ic.imageService.GetImage(ctx, parentID, backend.GetImageOpts{Platform: &platform})
if err != nil {
return "", err
}
diff --git a/image/cache/cache.go b/image/cache/cache.go
index 7bc4089a41300..8b7632f5e11a1 100644
--- a/image/cache/cache.go
+++ b/image/cache/cache.go
@@ -7,11 +7,13 @@ import (
"reflect"
"strings"
+ "github.com/containerd/containerd/platforms"
"github.com/containerd/log"
containertypes "github.com/docker/docker/api/types/container"
"github.com/docker/docker/dockerversion"
"github.com/docker/docker/image"
"github.com/docker/docker/layer"
+ ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
)
@@ -28,8 +30,8 @@ type LocalImageCache struct {
}
// GetCache returns the image id found in the cache
-func (lic *LocalImageCache) GetCache(imgID string, config *containertypes.Config) (string, error) {
- return getImageIDAndError(getLocalCachedImage(lic.store, image.ID(imgID), config))
+func (lic *LocalImageCache) GetCache(imgID string, config *containertypes.Config, platform ocispec.Platform) (string, error) {
+ return getImageIDAndError(getLocalCachedImage(lic.store, image.ID(imgID), config, platform))
}
// New returns an image cache, based on history objects
@@ -53,8 +55,8 @@ func (ic *ImageCache) Populate(image *image.Image) {
}
// GetCache returns the image id found in the cache
-func (ic *ImageCache) GetCache(parentID string, cfg *containertypes.Config) (string, error) {
- imgID, err := ic.localImageCache.GetCache(parentID, cfg)
+func (ic *ImageCache) GetCache(parentID string, cfg *containertypes.Config, platform ocispec.Platform) (string, error) {
+ imgID, err := ic.localImageCache.GetCache(parentID, cfg, platform)
if err != nil {
return "", err
}
@@ -217,7 +219,7 @@ func getImageIDAndError(img *image.Image, err error) (string, error) {
// of the image with imgID, that had the same config when it was
// created. nil is returned if a child cannot be found. An error is
// returned if the parent image cannot be found.
-func getLocalCachedImage(imageStore image.Store, imgID image.ID, config *containertypes.Config) (*image.Image, error) {
+func getLocalCachedImage(imageStore image.Store, imgID image.ID, config *containertypes.Config, platform ocispec.Platform) (*image.Image, error) {
if config == nil {
return nil, nil
}
@@ -247,6 +249,15 @@ func getLocalCachedImage(imageStore image.Store, imgID image.ID, config *contain
continue
}
+ imgPlatform := img.Platform()
+ // Discard old linux/amd64 images with empty platform.
+ if imgPlatform.OS == "" && imgPlatform.Architecture == "" {
+ continue
+ }
+ if !platforms.OnlyStrict(platform).Match(imgPlatform) {
+ continue
+ }
+
if compare(&img.ContainerConfig, config) {
// check for the most up to date match
if img.Created != nil && (match == nil || match.Created.Before(*img.Created)) {
From 96d461d27e8da97b64620c94a4a1b2448511c058 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= <pawel.gronowski@docker.com>
Date: Thu, 11 Jan 2024 14:04:02 +0100
Subject: [PATCH 5/5] builder/windows: Don't set ArgsEscaped for RUN cache
probe
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Previously this was done indirectly - the `compare` function didn't
check the `ArgsEscaped`.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
---
builder/dockerfile/dispatchers.go | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go
index 585d5fe033ec0..7183042012b58 100644
--- a/builder/dockerfile/dispatchers.go
+++ b/builder/dockerfile/dispatchers.go
@@ -348,9 +348,16 @@ func dispatchRun(ctx context.Context, d dispatchRequest, c *instructions.RunComm
saveCmd = prependEnvOnCmd(d.state.buildArgs, buildArgs, cmdFromArgs)
}
+ cacheArgsEscaped := argsEscaped
+ // ArgsEscaped is not persisted in the committed image on Windows.
+ // Use the original from previous build steps for cache probing.
+ if d.state.operatingSystem == "windows" {
+ cacheArgsEscaped = stateRunConfig.ArgsEscaped
+ }
+
runConfigForCacheProbe := copyRunConfig(stateRunConfig,
withCmd(saveCmd),
- withArgsEscaped(argsEscaped),
+ withArgsEscaped(cacheArgsEscaped),
withEntrypointOverride(saveCmd, nil))
if hit, err := d.builder.probeCache(d.state, runConfigForCacheProbe); err != nil || hit {
return err
