From 1fa3397aade178cc511df4e5e4d20103b6752212 Mon Sep 17 00:00:00 2001 From: Guillaume Lours Date: Tue, 21 Jul 2026 18:05:14 +0200 Subject: [PATCH] fix(config): pin type:image volume sources and pre_start hook images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rely on compose-go WithImagesResolved, which now resolves dependent images — `type: image` volume sources and pre_start hook images — with its already-digested guard, per-call memoization and sibling-service detection (compose-spec/compose-go#894, #899), rather than duplicating resolution logic CLI-side. The interpolated path gets this for free; --no-interpolate maps the raw model onto a pseudo-project keyed by service names to reuse the same resolution, and --lock-image-digests keeps type:image volumes in its output. pre_start hooks can't be carried into the lock override (hook lists are appended on merge), so generating a lock warns that hook images stay unpinned there. As a side effect, `compose publish` now fails fast on unresolvable dependent images. Fixes #13827 Signed-off-by: Guillaume Lours --- cmd/compose/config.go | 166 +++++++++++++++++++---- cmd/compose/config_test.go | 256 ++++++++++++++++++++++++++++++++++++ go.mod | 2 +- go.sum | 4 +- pkg/compose/publish_test.go | 53 ++++++++ 5 files changed, 454 insertions(+), 27 deletions(-) create mode 100644 cmd/compose/config_test.go diff --git a/cmd/compose/config.go b/cmd/compose/config.go index 8dfa847ea3..d02d87f0a0 100644 --- a/cmd/compose/config.go +++ b/cmd/compose/config.go @@ -236,6 +236,7 @@ func runConfigInterpolate(ctx context.Context, dockerCli command.Cli, opts confi } if opts.lockImageDigests { + warnHooksNotLockable(project) project = imagesOnly(project) } @@ -254,13 +255,19 @@ func runConfigInterpolate(ctx context.Context, dockerCli command.Cli, opts confi return content, nil } -// imagesOnly return project with all attributes removed but service.images +// imagesOnly return project with all attributes removed but service.images and `type: image` volumes func imagesOnly(project *types.Project) *types.Project { digests := types.Services{} for name, config := range project.Services { - digests[name] = types.ServiceConfig{ + service := types.ServiceConfig{ Image: config.Image, } + for _, vol := range config.Volumes { + if vol.Type == types.VolumeTypeImage { + service.Volumes = append(service.Volumes, vol) + } + } + digests[name] = service } project = &types.Project{Services: digests} return project @@ -284,58 +291,169 @@ func runConfigNoInterpolate(ctx context.Context, dockerCli command.Cli, opts con } if opts.lockImageDigests { - for key, e := range model { - if key != "services" { - delete(model, key) - } else { - for _, s := range e.(map[string]any) { - service := s.(map[string]any) - for key := range service { - if key != "image" { - delete(service, key) + warnModelHooksNotLockable(model) + lockModel(model) + } + + return formatModel(model, opts.Format) +} + +// hook sequences are appended when compose files are merged, so a lock override +// cannot pin a hook image without duplicating the hook: the lock file leaves +// pre_start hooks out, and their images stay unpinned once merged +const hooksNotLockableWarning = "service %q: pre_start hook images are not pinned in the override file produced by --lock-image-digests, use the full --resolve-image-digests output to pin them" + +func warnHooksNotLockable(project *types.Project) { + for name, service := range project.Services { + for _, hook := range service.PreStart { + if hook.Image != "" { + logrus.Warnf(hooksNotLockableWarning, name) + break + } + } + } +} + +func warnModelHooksNotLockable(model map[string]any) { + services, ok := model["services"].(map[string]any) + if !ok { + return + } + for name, s := range services { + service, ok := s.(map[string]any) + if !ok { + continue + } + for _, hook := range preStartHooks(service) { + if image, ok := hook["image"].(string); ok && image != "" { + logrus.Warnf(hooksNotLockableWarning, name) + break + } + } + } +} + +// lockModel removes from model all attributes but service images and `type: image` volumes +func lockModel(model map[string]any) { + for key, e := range model { + if key != "services" { + delete(model, key) + continue + } + for _, s := range e.(map[string]any) { + service := s.(map[string]any) + for key := range service { + switch key { + case "image": + case "volumes": + if volumes := imageVolumes(service); len(volumes) > 0 { + // write back as []any to keep the raw-model volumes type unchanged + filtered := make([]any, len(volumes)) + for i, volume := range volumes { + filtered[i] = volume } + service["volumes"] = filtered + } else { + delete(service, "volumes") } + default: + delete(service, key) } } } } - - return formatModel(model, opts.Format) } -func resolveImageDigests(ctx context.Context, dockerCli command.Cli, model map[string]any) (err error) { - // create a pseudo-project so we can rely on WithImagesResolved to resolve images +func resolveImageDigests(ctx context.Context, dockerCli command.Cli, model map[string]any) error { + // create a pseudo-project so we can rely on WithImagesResolved to resolve images, + // pre_start hook images and `type: image` volume sources, keyed by actual service + // names so sources referencing another service are detected as such and kept unresolved p := &types.Project{ Services: types.Services{}, } - services := model["services"].(map[string]any) + services, ok := model["services"].(map[string]any) + if !ok { + // services is optional at the top level of the compose model + return nil + } for name, s := range services { service := s.(map[string]any) - if image, ok := service["image"]; ok { - p.Services[name] = types.ServiceConfig{ - Image: image.(string), - } + config := types.ServiceConfig{} + if image, ok := service["image"].(string); ok { + config.Image = image + } + for _, hook := range preStartHooks(service) { + image, _ := hook["image"].(string) + config.PreStart = append(config.PreStart, types.ServiceHook{Image: image}) + } + for _, volume := range imageVolumes(service) { + source, _ := volume["source"].(string) + config.Volumes = append(config.Volumes, types.ServiceVolumeConfig{ + Type: types.VolumeTypeImage, + Source: source, + }) } + p.Services[name] = config } - p, err = p.WithImagesResolved(compose.ImageDigestResolver(ctx, dockerCli.ConfigFile(), dockerCli.Client())) + p, err := p.WithImagesResolved(compose.ImageDigestResolver(ctx, dockerCli.ConfigFile(), dockerCli.Client())) if err != nil { return err } - // Collect image resolved with digest and update model accordingly + // update model with image and volume-source references resolved with digest; + // fields absent from the resolved pseudo-project (empty Image / Source) are left untouched for name, s := range services { service := s.(map[string]any) config := p.Services[name] if config.Image != "" { service["image"] = config.Image } - services[name] = service + for i, hook := range preStartHooks(service) { + if image := config.PreStart[i].Image; image != "" { + hook["image"] = image + } + } + for i, volume := range imageVolumes(service) { + if source := config.Volumes[i].Source; source != "" { + volume["source"] = source + } + } } - model["services"] = services return nil } +// preStartHooks returns the pre_start hook declarations of a service raw model +func preStartHooks(service map[string]any) []map[string]any { + hooks, ok := service["pre_start"].([]any) + if !ok { + return nil + } + var result []map[string]any + for _, h := range hooks { + if hook, ok := h.(map[string]any); ok { + result = append(result, hook) + } + } + return result +} + +// imageVolumes returns the `type: image` volume declarations of a service raw model +func imageVolumes(service map[string]any) []map[string]any { + volumes, ok := service["volumes"].([]any) + if !ok { + return nil + } + var images []map[string]any + for _, v := range volumes { + volume, ok := v.(map[string]any) + if ok && volume["type"] == types.VolumeTypeImage { + images = append(images, volume) + } + } + return images +} + func formatModel(model map[string]any, format string) (content []byte, err error) { switch format { case "json": diff --git a/cmd/compose/config_test.go b/cmd/compose/config_test.go new file mode 100644 index 0000000000..a9a08f2cd7 --- /dev/null +++ b/cmd/compose/config_test.go @@ -0,0 +1,256 @@ +/* + Copyright 2020 Docker Compose CLI authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package compose + +import ( + "io" + "os" + "strings" + "testing" + + "github.com/compose-spec/compose-go/v2/types" + "github.com/docker/cli/cli/config/configfile" + "github.com/moby/moby/api/types/registry" + "github.com/moby/moby/client" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/sirupsen/logrus" + logrustest "github.com/sirupsen/logrus/hooks/test" + "go.uber.org/mock/gomock" + "gotest.tools/v3/assert" + + "github.com/docker/compose/v5/pkg/mocks" +) + +const testDigest = "sha256:1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef" + +func TestResolveImageDigests(t *testing.T) { + // distinct digests per resolved reference, so attaching a digest to the wrong image would fail + const ( + serviceDigest = "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + hookDigest = "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + volumeDigest = "sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc" + ) + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + apiClient := mocks.NewMockAPIClient(mockCtrl) + cli := mocks.NewMockCli(mockCtrl) + cli.EXPECT().Client().Return(apiClient).AnyTimes() + cli.EXPECT().ConfigFile().Return(configfile.New("")).AnyTimes() + + model := map[string]any{ + "services": map[string]any{ + "test": map[string]any{ + "image": "nginx:latest", + "pre_start": []any{ + map[string]any{"command": "echo hello", "image": "hookimage:latest"}, + // hook running in the service container: no image to resolve + map[string]any{"command": "echo hello"}, + }, + "volumes": []any{ + map[string]any{"type": "image", "source": "someimage:latest", "target": "/data"}, + // already digested: must NOT trigger any registry call and must be kept as-is + map[string]any{"type": "image", "source": "docker.io/library/pinned@" + testDigest, "target": "/pinned"}, + // source referencing another service: locally built image, must be kept as-is + map[string]any{"type": "image", "source": "builder", "target": "/built"}, + map[string]any{"type": "bind", "source": "/host", "target": "/bind"}, + "./data:/short", + }, + }, + "builder": map[string]any{ + "image": "docker.io/library/pinned@" + testDigest, + }, + // service without image: its image volumes must still be pinned, and + // "someimage:latest" being also used by "test" must be resolved only once + "data": map[string]any{ + "volumes": []any{ + map[string]any{"type": "image", "source": "someimage:latest", "target": "/data"}, + }, + }, + }, + } + + apiClient.EXPECT().DistributionInspect(gomock.Any(), "docker.io/library/nginx:latest", gomock.Any()). + Return(client.DistributionInspectResult{ + DistributionInspect: registry.DistributionInspect{Descriptor: ocispec.Descriptor{Digest: serviceDigest}}, + }, nil) + apiClient.EXPECT().DistributionInspect(gomock.Any(), "docker.io/library/someimage:latest", gomock.Any()). + Return(client.DistributionInspectResult{ + DistributionInspect: registry.DistributionInspect{Descriptor: ocispec.Descriptor{Digest: volumeDigest}}, + }, nil) + apiClient.EXPECT().DistributionInspect(gomock.Any(), "docker.io/library/hookimage:latest", gomock.Any()). + Return(client.DistributionInspectResult{ + DistributionInspect: registry.DistributionInspect{Descriptor: ocispec.Descriptor{Digest: hookDigest}}, + }, nil) + + err := resolveImageDigests(t.Context(), cli, model) + assert.NilError(t, err) + + services := model["services"].(map[string]any) + service := services["test"].(map[string]any) + assert.Equal(t, service["image"], "docker.io/library/nginx:latest@"+serviceDigest) + hooks := service["pre_start"].([]any) + assert.Equal(t, hooks[0].(map[string]any)["image"], "docker.io/library/hookimage:latest@"+hookDigest) + _, hasImage := hooks[1].(map[string]any)["image"] + assert.Assert(t, !hasImage) + volumes := service["volumes"].([]any) + assert.Equal(t, volumes[0].(map[string]any)["source"], "docker.io/library/someimage:latest@"+volumeDigest) + assert.Equal(t, volumes[1].(map[string]any)["source"], "docker.io/library/pinned@"+testDigest) + assert.Equal(t, volumes[2].(map[string]any)["source"], "builder") + assert.Equal(t, volumes[3].(map[string]any)["source"], "/host") + assert.Equal(t, volumes[4], "./data:/short") + assert.Equal(t, services["builder"].(map[string]any)["image"], "docker.io/library/pinned@"+testDigest) + dataVolumes := services["data"].(map[string]any)["volumes"].([]any) + assert.Equal(t, dataVolumes[0].(map[string]any)["source"], "docker.io/library/someimage:latest@"+volumeDigest) +} + +func TestResolveImageDigestsWithoutServices(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + cli := mocks.NewMockCli(mockCtrl) + cli.EXPECT().Client().Return(mocks.NewMockAPIClient(mockCtrl)).AnyTimes() + cli.EXPECT().ConfigFile().Return(configfile.New("")).AnyTimes() + + // top-level services is optional in the compose spec, and unlike the typed + // project the raw model gets no empty services map from normalization + model := map[string]any{ + "networks": map[string]any{"foo": nil}, + } + + err := resolveImageDigests(t.Context(), cli, model) + assert.NilError(t, err) +} + +func TestImagesOnly(t *testing.T) { + project := &types.Project{ + Name: "test", + Services: types.Services{ + "test": types.ServiceConfig{ + Name: "test", + Image: "docker.io/library/nginx@" + testDigest, + Command: types.ShellCommand{"echo", "hello"}, + // hooks can't be overridden element-wise on merge, so the lock must not carry them + PreStart: []types.ServiceHook{{Image: "docker.io/library/hookimage@" + testDigest}}, + Volumes: []types.ServiceVolumeConfig{ + {Type: types.VolumeTypeImage, Source: "docker.io/library/someimage@" + testDigest, Target: "/data"}, + {Type: types.VolumeTypeBind, Source: "/host", Target: "/bind"}, + }, + }, + }, + Networks: types.Networks{"default": types.NetworkConfig{}}, + } + + locked := imagesOnly(project) + + assert.DeepEqual(t, locked, &types.Project{ + Services: types.Services{ + "test": types.ServiceConfig{ + Image: "docker.io/library/nginx@" + testDigest, + Volumes: []types.ServiceVolumeConfig{ + {Type: types.VolumeTypeImage, Source: "docker.io/library/someimage@" + testDigest, Target: "/data"}, + }, + }, + }, + }) +} + +func TestWarnHooksNotLockable(t *testing.T) { + hook := logrustest.NewGlobal() + logrus.SetOutput(io.Discard) + defer func() { + logrus.StandardLogger().ReplaceHooks(make(logrus.LevelHooks)) + logrus.SetOutput(os.Stderr) + }() + + warnHooksNotLockable(&types.Project{ + Services: types.Services{ + "with-hook-image": types.ServiceConfig{PreStart: []types.ServiceHook{{Image: "alpine:latest"}}}, + "inline-hook": types.ServiceConfig{PreStart: []types.ServiceHook{{Command: types.ShellCommand{"echo"}}}}, + "without-hook": types.ServiceConfig{}, + }, + }) + + assert.Equal(t, len(hook.Entries), 1) + assert.Assert(t, strings.Contains(hook.Entries[0].Message, `service "with-hook-image"`)) +} + +func TestWarnModelHooksNotLockable(t *testing.T) { + hook := logrustest.NewGlobal() + logrus.SetOutput(io.Discard) + defer func() { + logrus.StandardLogger().ReplaceHooks(make(logrus.LevelHooks)) + logrus.SetOutput(os.Stderr) + }() + + warnModelHooksNotLockable(map[string]any{ + "services": map[string]any{ + "with-hook-image": map[string]any{ + "pre_start": []any{map[string]any{"image": "alpine:latest"}}, + }, + "inline-hook": map[string]any{ + "pre_start": []any{map[string]any{"command": "echo"}}, + }, + "without-hook": map[string]any{"image": "nginx"}, + }, + }) + + assert.Equal(t, len(hook.Entries), 1) + assert.Assert(t, strings.Contains(hook.Entries[0].Message, `service "with-hook-image"`)) +} + +func TestLockModel(t *testing.T) { + model := map[string]any{ + "name": "test", + "services": map[string]any{ + "a": map[string]any{ + "image": "docker.io/library/nginx@" + testDigest, + "command": "echo hello", + // hooks can't be overridden element-wise on merge, so the lock must not carry them + "pre_start": []any{ + map[string]any{"image": "docker.io/library/hookimage@" + testDigest}, + }, + "volumes": []any{ + map[string]any{"type": "image", "source": "docker.io/library/someimage@" + testDigest, "target": "/data"}, + map[string]any{"type": "bind", "source": "/host", "target": "/bind"}, + }, + }, + "b": map[string]any{ + "image": "docker.io/library/alpine@" + testDigest, + "volumes": []any{ + map[string]any{"type": "bind", "source": "/host", "target": "/bind"}, + }, + }, + }, + "networks": map[string]any{"default": nil}, + } + + lockModel(model) + + assert.DeepEqual(t, model, map[string]any{ + "services": map[string]any{ + "a": map[string]any{ + "image": "docker.io/library/nginx@" + testDigest, + // volumes keeps its []any raw-model type after filtering + "volumes": []any{ + map[string]any{"type": "image", "source": "docker.io/library/someimage@" + testDigest, "target": "/data"}, + }, + }, + "b": map[string]any{ + "image": "docker.io/library/alpine@" + testDigest, + }, + }, + }) +} diff --git a/go.mod b/go.mod index c85557e92d..776e9421cf 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/Microsoft/go-winio v0.6.2 github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d github.com/buger/goterm v1.0.4 - github.com/compose-spec/compose-go/v2 v2.13.0 + github.com/compose-spec/compose-go/v2 v2.13.1-0.20260721083329-e70e961c3a3a github.com/containerd/console v1.0.5 github.com/containerd/containerd/v2 v2.2.5 github.com/containerd/errdefs v1.0.0 diff --git a/go.sum b/go.sum index 24ce60993d..9f6bc45f43 100644 --- a/go.sum +++ b/go.sum @@ -38,8 +38,8 @@ github.com/cloudflare/circl v1.6.3 h1:9GPOhQGF9MCYUeXyMYlqTR6a5gTrgR/fBLXvUgtVcg github.com/cloudflare/circl v1.6.3/go.mod h1:2eXP6Qfat4O/Yhh8BznvKnJ+uzEoTQ6jVKJRn81BiS4= github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb h1:EDmT6Q9Zs+SbUoc7Ik9EfrFqcylYqgPZ9ANSbTAntnE= github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb/go.mod h1:ZjrT6AXHbDs86ZSdt/osfBi5qfexBrKUdONk989Wnk4= -github.com/compose-spec/compose-go/v2 v2.13.0 h1:2+2oS3v4SrtAOBdZRAZYBsBy47D571p5EXMSCppmTtE= -github.com/compose-spec/compose-go/v2 v2.13.0/go.mod h1:ZU6zlcweCZKyiB7BVfCizQT9XmkEIMFE+PRZydVcsZg= +github.com/compose-spec/compose-go/v2 v2.13.1-0.20260721083329-e70e961c3a3a h1:I1xkzPuYQi7Nq+z84ZZ/lunRtW/s7P5KbuwLks7wl5s= +github.com/compose-spec/compose-go/v2 v2.13.1-0.20260721083329-e70e961c3a3a/go.mod h1:ZU6zlcweCZKyiB7BVfCizQT9XmkEIMFE+PRZydVcsZg= github.com/containerd/cgroups/v3 v3.1.3 h1:eUNflyMddm18+yrDmZPn3jI7C5hJ9ahABE5q6dyLYXQ= github.com/containerd/cgroups/v3 v3.1.3/go.mod h1:PKZ2AcWmSBsY/tJUVhtS/rluX0b1uq1GmPO1ElCmbOw= github.com/containerd/console v1.0.5 h1:R0ymNeydRqH2DmakFNdmjR2k0t7UPuiOV/N/27/qqsc= diff --git a/pkg/compose/publish_test.go b/pkg/compose/publish_test.go index ecbaa68043..c78808ac25 100644 --- a/pkg/compose/publish_test.go +++ b/pkg/compose/publish_test.go @@ -28,8 +28,12 @@ import ( "github.com/compose-spec/compose-go/v2/loader" "github.com/compose-spec/compose-go/v2/types" + "github.com/docker/cli/cli/config/configfile" "github.com/google/go-cmp/cmp" + "github.com/moby/moby/api/types/registry" + "github.com/moby/moby/client" v1 "github.com/opencontainers/image-spec/specs-go/v1" + "go.uber.org/mock/gomock" "gotest.tools/v3/assert" "github.com/docker/compose/v5/internal" @@ -724,3 +728,52 @@ func Test_publish_decline_returns_ErrCanceled(t *testing.T) { assert.Assert(t, errors.Is(err, api.ErrCanceled), "expected api.ErrCanceled when user declines, got: %v", err) } + +func Test_generateImageDigestsOverride_resolvesDependentImages(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + apiClient, cli := prepareMocks(mockCtrl) + cli.EXPECT().ConfigFile().Return(configfile.New("")).AnyTimes() + tested := &composeService{dockerCli: cli} + + // distinct digests per resolved reference, so attaching a digest to the wrong image would fail + const ( + serviceDigest = "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + hookDigest = "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + volumeDigest = "sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc" + ) + project := &types.Project{ + Name: "test", + Services: types.Services{ + "app": types.ServiceConfig{ + Name: "app", + Image: "nginx:latest", + PreStart: []types.ServiceHook{{Image: "hookimage:latest"}}, + Volumes: []types.ServiceVolumeConfig{ + {Type: types.VolumeTypeImage, Source: "someimage:latest", Target: "/data"}, + }, + }, + }, + } + + // compose-go resolves dependent images (pre_start hooks, type: image volume sources) + // in WithImagesResolved, so publish now resolves them too: expect one registry call + // per dependent image + apiClient.EXPECT().DistributionInspect(gomock.Any(), "docker.io/library/nginx:latest", gomock.Any()). + Return(client.DistributionInspectResult{ + DistributionInspect: registry.DistributionInspect{Descriptor: v1.Descriptor{Digest: serviceDigest}}, + }, nil) + apiClient.EXPECT().DistributionInspect(gomock.Any(), "docker.io/library/hookimage:latest", gomock.Any()). + Return(client.DistributionInspectResult{ + DistributionInspect: registry.DistributionInspect{Descriptor: v1.Descriptor{Digest: hookDigest}}, + }, nil) + apiClient.EXPECT().DistributionInspect(gomock.Any(), "docker.io/library/someimage:latest", gomock.Any()). + Return(client.DistributionInspectResult{ + DistributionInspect: registry.DistributionInspect{Descriptor: v1.Descriptor{Digest: volumeDigest}}, + }, nil) + + override, err := tested.generateImageDigestsOverride(t.Context(), project) + assert.NilError(t, err) + assert.Assert(t, strings.Contains(string(override), "docker.io/library/nginx:latest@"+serviceDigest)) +}