beenull/moby
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Merge pull request from GHSA-xw73-rw38-6vjc

Browse source 
[23.0 backport] image/cache: Restrict cache candidates to locally built images
This commit is contained in:
Sebastiaan van Stijn 2024-02-01 01:12:23 +01:00 committed by GitHub
commit 79de67d989
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 262 additions and 61 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
builder/builder.go
View file

@ -15,6 +15,7 @@ import (
"github.com/docker/docker/image"
"github.com/docker/docker/layer"
"github.com/docker/docker/pkg/containerfs"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
)
const (
@ -89,7 +90,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.

19
builder/dockerfile/copy.go
View file

@ -9,7 +9,6 @@ import (
"net/url"
"os"
"path/filepath"
"runtime"
"sort"
"strings"
"time"
@ -25,7 +24,7 @@ import (
"github.com/docker/docker/pkg/streamformatter"
"github.com/docker/docker/pkg/system"
"github.com/moby/buildkit/frontend/dockerfile/instructions"
specs "github.com/opencontainers/image-spec/specs-go/v1"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
)
@ -75,7 +74,7 @@ type copier struct {
source builder.Source
pathCache pathCache
download sourceDownloader
platform *specs.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.
@ -84,19 +83,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 = &specs.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,

9
builder/dockerfile/dispatchers.go
View file

@ -345,9 +345,16 @@ func dispatchRun(d dispatchRequest, c *instructions.RunCommand) error {
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

9
builder/dockerfile/imageprobe.go
View file

@ -3,6 +3,7 @@ package dockerfile // import "github.com/docker/docker/builder/dockerfile"
import (
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/builder"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/sirupsen/logrus"
)
@ -10,7 +11,7 @@ import (
// cache.
type ImageProber interface {
Reset()
Probe(parentID string, runConfig *container.Config) (string, error)
Probe(parentID string, runConfig *container.Config, platform ocispec.Platform) (string, error)
}
type imageProber struct {
@ -37,11 +38,11 @@ func (c *imageProber) Reset() {
// 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
}
@ -58,6 +59,6 @@ type nopProber struct{}
func (c *nopProber) Reset() {}
func (c *nopProber) Probe(_ string, _ *container.Config) (string, error) {
func (c *nopProber) Probe(_ string, _ *container.Config, _ ocispec.Platform) (string, error) {
return "", nil
}

17
builder/dockerfile/internals.go
View file

@ -10,6 +10,7 @@ import (
"io"
"strings"
"github.com/containerd/containerd/platforms"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/backend"
"github.com/docker/docker/api/types/container"
@ -364,7 +365,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
}
@ -424,3 +425,17 @@ func hostConfigFromOptions(options *types.ImageBuildOptions) *container.HostConf
}
return hc
}
func (b *Builder) getPlatform(state *dispatchState) specs.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
}

3
builder/dockerfile/mockbackend_test.go
View file

@ -14,6 +14,7 @@ import (
"github.com/docker/docker/image"
"github.com/docker/docker/layer"
"github.com/docker/docker/pkg/containerfs"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
)
// MockBackend implements the builder.Backend interface for unit testing
@ -111,7 +112,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)
}

3
daemon/images/image_builder.go
View file

@ -250,6 +250,9 @@ func (i *ImageService) CreateImage(config []byte, parent string) (builder.Image,
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)
}

3
daemon/images/image_commit.go
View file

@ -57,6 +57,9 @@ func (i *ImageService) CommitImage(c backend.CommitConfig) (image.ID, error) {
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 {

82
image/cache/cache.go vendored
View file

@ -6,11 +6,14 @@ import (
"reflect"
"strings"
"github.com/containerd/containerd/platforms"
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"
"github.com/sirupsen/logrus"
)
// NewLocal returns a local image cache, based on parent chain
@ -26,8 +29,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
@ -51,8 +54,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
}
@ -215,7 +218,23 @@ 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
}
isBuiltLocally := func(id image.ID) bool {
builtLocally, err := imageStore.IsBuiltLocally(id)
if err != nil {
logrus.WithFields(logrus.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 +244,25 @@ func getLocalCachedImage(imageStore image.Store, imgID image.ID, config *contain
return nil, fmt.Errorf("unable to find image %q", id)
}
if !isBuiltLocally(id) {
continue
}
imgPlatform := ocispec.Platform{
Architecture: img.Architecture,
OS: img.OS,
OSVersion: img.OSVersion,
OSFeatures: img.OSFeatures,
Variant: img.Variant,
}
// 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 match == nil || match.Created.Before(img.Created) {
@ -238,11 +276,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 +307,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
}

155
image/cache/compare.go vendored
View file

@ -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 {
return false
}
if a.AttachStdout != b.AttachStdout ||
a.AttachStderr != b.AttachStderr ||
a.User != b.User ||
a.OpenStdin != b.OpenStdin ||
a.Tty != b.Tty {
if a == nil || b == nil {
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.Env) != len(b.Env) {
return false
}
if len(a.Cmd) != len(b.Cmd) {
return false
}
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.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] {
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.Entrypoint); i++ {
if a.Entrypoint[i] != b.Entrypoint[i] {
return false
}
}
for i := 0; i < len(a.Shell); i++ {
if a.Shell[i] != b.Shell[i] {
return false
}
}
@ -48,16 +75,84 @@ func compare(a, b *container.Config) bool {
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
}
}
for k, v := range a.Labels {
if v != b.Labels[k] {
return false
}
}
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
}
}
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.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
}

20
image/store.go
View file

@ -2,6 +2,7 @@ package image // import "github.com/docker/docker/image"
import (
"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
@ -291,6 +294,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()