From e7fe397219a81d2dedb8dcb36cc8bf859b397a06 Mon Sep 17 00:00:00 2001 From: Guillaume Lours Date: Mon, 7 Sep 2026 14:13:54 +0200 Subject: [PATCH] feat(compose): warn on unsupported compose-file attributes Compose silently ignored several spec-valid attributes it doesn't honor outside Swarm mode: deploy.update_config, credential_spec, ports[].mode: host, and others had no visible effect on a plain `docker compose up`. Detection is wired into compose-go's loader as a fail-closed allowlist: every attribute path the schema declares, minus the handful this runtime deliberately doesn't implement, plus a few value-conditional checks for attributes only unsupported for one specific value. A newly-specified attribute gets flagged automatically, with no corresponding code change needed here. Depends on an unreleased compose-go change; go.mod carries a temporary replace to a fork branch until it's released. Signed-off-by: Guillaume Lours --- cmd/compose/bridge.go | 2 +- cmd/compose/build.go | 2 +- cmd/compose/completion.go | 4 +- cmd/compose/compose.go | 24 +- cmd/compose/config.go | 4 +- cmd/compose/config_test.go | 60 +- cmd/compose/publish.go | 2 +- cmd/compose/pull.go | 2 +- cmd/compose/push.go | 2 +- cmd/compose/run.go | 2 +- cmd/compose/scale.go | 2 +- cmd/compose/unsupported_attributes.go | 26 + cmd/compose/unsupported_attributes_test.go | 123 ++++ cmd/compose/viz.go | 2 +- cmd/compose/watch.go | 2 +- go.mod | 2 +- go.sum | 4 +- pkg/api/api.go | 24 + pkg/compose/create.go | 8 - pkg/compose/loader.go | 4 + pkg/compose/service_containers.go | 19 +- pkg/compose/service_containers_test.go | 19 + pkg/compose/unsupported_attributes.go | 300 ++++++++ pkg/compose/unsupported_attributes_test.go | 664 ++++++++++++++++++ .../compose.yaml | 82 +++ .../creds.json | 1 + .../labels.env | 1 + .../myconfig.txt | 1 + .../mysecret.txt | 1 + pkg/e2e/unsupported_attributes_test.go | 62 ++ 30 files changed, 1382 insertions(+), 69 deletions(-) create mode 100644 cmd/compose/unsupported_attributes.go create mode 100644 cmd/compose/unsupported_attributes_test.go create mode 100644 pkg/compose/unsupported_attributes.go create mode 100644 pkg/compose/unsupported_attributes_test.go create mode 100644 pkg/e2e/testdata/TestUnsupportedAttributesWarning/compose.yaml create mode 100644 pkg/e2e/testdata/TestUnsupportedAttributesWarning/creds.json create mode 100644 pkg/e2e/testdata/TestUnsupportedAttributesWarning/labels.env create mode 100644 pkg/e2e/testdata/TestUnsupportedAttributesWarning/myconfig.txt create mode 100644 pkg/e2e/testdata/TestUnsupportedAttributesWarning/mysecret.txt create mode 100644 pkg/e2e/unsupported_attributes_test.go diff --git a/cmd/compose/bridge.go b/cmd/compose/bridge.go index c346290546b..61c641d843b 100644 --- a/cmd/compose/bridge.go +++ b/cmd/compose/bridge.go @@ -94,7 +94,7 @@ func runConvert(ctx context.Context, dockerCli command.Cli, p *ProjectOptions, o return err } - project, _, err := p.ToProject(ctx, dockerCli, backend, nil) + project, _, err := p.ToProject(ctx, dockerCli, backend, nil, warnUnsupportedAttributes) if err != nil { return err } diff --git a/cmd/compose/build.go b/cmd/compose/build.go index a5914c762ae..3dc95367759 100644 --- a/cmd/compose/build.go +++ b/cmd/compose/build.go @@ -168,7 +168,7 @@ func runBuild(ctx context.Context, dockerCli command.Cli, backendOptions *Backen } opts.All = true // do not drop resources as build may involve some dependencies by additional_contexts - project, _, err := opts.ToProject(ctx, dockerCli, backend, nil, cli.WithoutEnvironmentResolution) + project, _, err := opts.ToProject(ctx, dockerCli, backend, nil, warnUnsupportedAttributes, cli.WithoutEnvironmentResolution) if err != nil { return err } diff --git a/cmd/compose/completion.go b/cmd/compose/completion.go index 7a472937c55..4d39a1982e0 100644 --- a/cmd/compose/completion.go +++ b/cmd/compose/completion.go @@ -47,7 +47,7 @@ func completeServiceNames(dockerCli command.Cli, p *ProjectOptions) validArgsFn // only service names are needed, so skip environment resolution: a missing // env_file must not prevent completion - project, _, err := p.ToProject(cmd.Context(), dockerCli, backend, nil, cli.WithoutEnvironmentResolution) + project, _, err := p.ToProject(cmd.Context(), dockerCli, backend, nil, skipUnsupportedAttributesWarning, cli.WithoutEnvironmentResolution) if err != nil { return nil, cobra.ShellCompDirectiveNoFileComp } @@ -95,7 +95,7 @@ func completeProfileNames(dockerCli command.Cli, p *ProjectOptions) validArgsFn // only profile names are needed, so skip environment resolution: a missing // env_file must not prevent completion - project, _, err := p.ToProject(cmd.Context(), dockerCli, backend, nil, cli.WithoutEnvironmentResolution) + project, _, err := p.ToProject(cmd.Context(), dockerCli, backend, nil, skipUnsupportedAttributesWarning, cli.WithoutEnvironmentResolution) if err != nil { return nil, cobra.ShellCompDirectiveNoFileComp } diff --git a/cmd/compose/compose.go b/cmd/compose/compose.go index 52daddcb364..7fb10455564 100644 --- a/cmd/compose/compose.go +++ b/cmd/compose/compose.go @@ -173,7 +173,7 @@ func (o *ProjectOptions) WithServices(dockerCli command.Cli, fn ProjectServicesF return err } - project, metrics, err := o.ToProject(ctx, dockerCli, backend, services, cli.WithoutEnvironmentResolution) + project, metrics, err := o.ToProject(ctx, dockerCli, backend, services, warnUnsupportedAttributes, cli.WithoutEnvironmentResolution) if err != nil { return err } @@ -252,7 +252,7 @@ func (o *ProjectOptions) projectOrName(ctx context.Context, dockerCli command.Cl return nil, "", err } - p, _, err := o.ToProject(ctx, dockerCli, backend, services, cli.WithDiscardEnvFile, cli.WithoutEnvironmentResolution) + p, _, err := o.ToProject(ctx, dockerCli, backend, services, skipUnsupportedAttributesWarning, cli.WithDiscardEnvFile, cli.WithoutEnvironmentResolution) if err != nil { envProjectName := os.Getenv(ComposeProjectName) if envProjectName != "" { @@ -281,7 +281,7 @@ func (o *ProjectOptions) toProjectName(ctx context.Context, dockerCli command.Cl return "", err } - project, _, err := o.ToProject(ctx, dockerCli, backend, nil, cli.WithDiscardEnvFile, cli.WithoutEnvironmentResolution) + project, _, err := o.ToProject(ctx, dockerCli, backend, nil, skipUnsupportedAttributesWarning, cli.WithDiscardEnvFile, cli.WithoutEnvironmentResolution) if err != nil { return "", err } @@ -306,9 +306,14 @@ func (o *ProjectOptions) ToModel(ctx context.Context, dockerCli command.Cli, ser return options.LoadModel(ctx) } -// ToProject loads a Compose project using the LoadProject API. -// Accepts optional cli.ProjectOptionsFn to control loader behavior. -func (o *ProjectOptions) ToProject(ctx context.Context, dockerCli command.Cli, backend api.Compose, services []string, po ...cli.ProjectOptionsFn) (*types.Project, tracing.Metrics, error) { +// ToProject loads a Compose project using the LoadProject API, then — when +// warn is warnUnsupportedAttributes — warns about any unsupported-attribute +// finding compose-go's loader reports during that load. Accepts optional +// cli.ProjectOptionsFn to control loader behavior. +func (o *ProjectOptions) ToProject( + ctx context.Context, dockerCli command.Cli, backend api.Compose, services []string, + warn unsupportedAttributeWarning, po ...cli.ProjectOptionsFn, +) (*types.Project, tracing.Metrics, error) { var metrics tracing.Metrics remotes := o.remoteLoaders(dockerCli) @@ -350,6 +355,13 @@ func (o *ProjectOptions) ToProject(ctx context.Context, dockerCli command.Cli, b LoadListeners: []api.LoadListener{metricsListener}, OCI: o.ociOptions(), } + if warn == warnUnsupportedAttributes { + loadOpts.OnUnsupportedAttribute = func(findings []api.UnsupportedAttribute) { + for _, finding := range findings { + logrus.Warn(finding) + } + } + } project, err := backend.LoadProject(ctx, loadOpts) if err != nil { diff --git a/cmd/compose/config.go b/cmd/compose/config.go index 9e832931085..8c4065f43f8 100644 --- a/cmd/compose/config.go +++ b/cmd/compose/config.go @@ -63,9 +63,11 @@ type configOptions struct { lockImageDigests bool } +// ToProject always warns: every config subcommand renders the resolved +// model to the user, so any unsupported-attribute finding is relevant here. func (o *configOptions) ToProject(ctx context.Context, dockerCli command.Cli, backend api.Compose, services []string, po ...cli.ProjectOptionsFn) (*types.Project, error) { po = append(po, o.toProjectOptionsFns()...) - project, _, err := o.ProjectOptions.ToProject(ctx, dockerCli, backend, services, po...) + project, _, err := o.ProjectOptions.ToProject(ctx, dockerCli, backend, services, warnUnsupportedAttributes, po...) return project, err } diff --git a/cmd/compose/config_test.go b/cmd/compose/config_test.go index a9a08f2cd77..8807850a20d 100644 --- a/cmd/compose/config_test.go +++ b/cmd/compose/config_test.go @@ -17,8 +17,6 @@ package compose import ( - "io" - "os" "strings" "testing" @@ -27,8 +25,6 @@ import ( "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" @@ -168,47 +164,37 @@ func TestImagesOnly(t *testing.T) { } 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{}, - }, + messages := captureWarnings(t, func() { + 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"`)) + assert.Equal(t, len(messages), 1) + assert.Assert(t, strings.Contains(messages[0], `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"}}, + messages := captureWarnings(t, func() { + 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"}, }, - "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"`)) + assert.Equal(t, len(messages), 1) + assert.Assert(t, strings.Contains(messages[0], `service "with-hook-image"`)) } func TestLockModel(t *testing.T) { diff --git a/cmd/compose/publish.go b/cmd/compose/publish.go index 6dba282ebc4..d58d5518d7a 100644 --- a/cmd/compose/publish.go +++ b/cmd/compose/publish.go @@ -83,7 +83,7 @@ func runPublish(ctx context.Context, dockerCli command.Cli, backendOptions *Back return err } - project, metrics, err := opts.ToProject(ctx, dockerCli, backend, nil) + project, metrics, err := opts.ToProject(ctx, dockerCli, backend, nil, warnUnsupportedAttributes) if err != nil { return err } diff --git a/cmd/compose/pull.go b/cmd/compose/pull.go index 694731155f6..4710d94f1c3 100644 --- a/cmd/compose/pull.go +++ b/cmd/compose/pull.go @@ -104,7 +104,7 @@ func runPull(ctx context.Context, dockerCli command.Cli, backendOptions *Backend return err } - project, _, err := opts.ToProject(ctx, dockerCli, backend, services, cli.WithoutEnvironmentResolution) + project, _, err := opts.ToProject(ctx, dockerCli, backend, services, warnUnsupportedAttributes, cli.WithoutEnvironmentResolution) if err != nil { return err } diff --git a/cmd/compose/push.go b/cmd/compose/push.go index 4dd23aedb6f..178dbab5842 100644 --- a/cmd/compose/push.go +++ b/cmd/compose/push.go @@ -60,7 +60,7 @@ func runPush(ctx context.Context, dockerCli command.Cli, backendOptions *Backend return err } - project, _, err := opts.ToProject(ctx, dockerCli, backend, services) + project, _, err := opts.ToProject(ctx, dockerCli, backend, services, warnUnsupportedAttributes) if err != nil { return err } diff --git a/cmd/compose/run.go b/cmd/compose/run.go index 7d5f522b435..199e86f1192 100644 --- a/cmd/compose/run.go +++ b/cmd/compose/run.go @@ -271,7 +271,7 @@ func normalizeRunFlags(f *pflag.FlagSet, name string) pflag.NormalizedName { // dependencies started by run, so hashing a different value would recreate // their containers. func runProject(ctx context.Context, dockerCli command.Cli, backend api.Compose, p *ProjectOptions, service string) (*types.Project, error) { - project, _, err := p.ToProject(ctx, dockerCli, backend, []string{service}, composecli.WithoutEnvironmentResolution) + project, _, err := p.ToProject(ctx, dockerCli, backend, []string{service}, warnUnsupportedAttributes, composecli.WithoutEnvironmentResolution) if err != nil { return nil, err } diff --git a/cmd/compose/scale.go b/cmd/compose/scale.go index 0e54fec80e3..ae474079a6d 100644 --- a/cmd/compose/scale.go +++ b/cmd/compose/scale.go @@ -68,7 +68,7 @@ func runScale(ctx context.Context, dockerCli command.Cli, backendOptions *Backen } services := slices.Sorted(maps.Keys(serviceReplicaTuples)) - project, _, err := opts.ToProject(ctx, dockerCli, backend, services, cli.WithoutEnvironmentResolution) + project, _, err := opts.ToProject(ctx, dockerCli, backend, services, warnUnsupportedAttributes, cli.WithoutEnvironmentResolution) if err != nil { return err } diff --git a/cmd/compose/unsupported_attributes.go b/cmd/compose/unsupported_attributes.go new file mode 100644 index 00000000000..a65322bdb13 --- /dev/null +++ b/cmd/compose/unsupported_attributes.go @@ -0,0 +1,26 @@ +/* + 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 + +// unsupportedAttributeWarning is a distinct type rather than a bool so every +// ToProject call site must name one of the constants below. +type unsupportedAttributeWarning int + +const ( + warnUnsupportedAttributes unsupportedAttributeWarning = iota + skipUnsupportedAttributesWarning +) diff --git a/cmd/compose/unsupported_attributes_test.go b/cmd/compose/unsupported_attributes_test.go new file mode 100644 index 00000000000..ac53a379999 --- /dev/null +++ b/cmd/compose/unsupported_attributes_test.go @@ -0,0 +1,123 @@ +/* + 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" + "path/filepath" + "testing" + + "github.com/sirupsen/logrus" + logrustest "github.com/sirupsen/logrus/hooks/test" + "gotest.tools/v3/assert" + + realcompose "github.com/docker/compose/v5/pkg/compose" +) + +// unsupportedAttrFixture writes a compose file with an attribute +// (deploy.mode) that the unsupported-attribute check always flags, so any +// code path that loads it can be checked for whether it warns or stays +// silent. +func unsupportedAttrFixture(t *testing.T) string { + t.Helper() + path := filepath.Join(t.TempDir(), "compose.yaml") + content := ` +services: + web: + image: alpine + deploy: + mode: replicated +` + assert.NilError(t, os.WriteFile(path, []byte(content), 0o644)) + return path +} + +// captureWarnings runs fn with a global logrus hook installed and returns +// every message logged during the call. +func captureWarnings(t *testing.T, fn func()) []string { + t.Helper() + hook := logrustest.NewGlobal() + logrus.SetOutput(io.Discard) + defer func() { + logrus.StandardLogger().ReplaceHooks(make(logrus.LevelHooks)) + logrus.SetOutput(os.Stderr) + }() + fn() + messages := make([]string, len(hook.Entries)) + for i, entry := range hook.Entries { + messages[i] = entry.Message + } + return messages +} + +// ToProject is the "real" load path used by commands that act on the full +// compose model (build, config, run, scale, watch, ...): it must warn. +func TestToProject_WarnsOnUnsupportedAttributes(t *testing.T) { + opts := &ProjectOptions{ConfigPaths: []string{unsupportedAttrFixture(t)}} + backend, err := realcompose.NewComposeService(nil) + assert.NilError(t, err) + + messages := captureWarnings(t, func() { + _, _, err := opts.ToProject(t.Context(), nil, backend, nil, warnUnsupportedAttributes) + assert.NilError(t, err) + }) + + assert.Assert(t, len(messages) > 0, "ToProject should warn about deploy.mode") +} + +func TestToProject_SkipsWarningWhenRequested(t *testing.T) { + opts := &ProjectOptions{ConfigPaths: []string{unsupportedAttrFixture(t)}} + backend, err := realcompose.NewComposeService(nil) + assert.NilError(t, err) + + messages := captureWarnings(t, func() { + _, _, err := opts.ToProject(t.Context(), nil, backend, nil, skipUnsupportedAttributesWarning) + assert.NilError(t, err) + }) + + assert.Equal(t, len(messages), 0) +} + +// projectOrName and toProjectName are lightweight project-name resolution +// helpers used by commands that operate on already-running +// containers/services by name (down, stop, ps, logs, ...) or by shell +// completion (completeServiceNames, completeProfileNames). Warning here +// would fire on every such invocation regardless of whether the command has +// anything to do with the flagged attribute, and even during tab-completion +// — see docker/compose#14196 review discussion. +func TestProjectOrName_DoesNotWarnOnUnsupportedAttributes(t *testing.T) { + opts := &ProjectOptions{ConfigPaths: []string{unsupportedAttrFixture(t)}} + + messages := captureWarnings(t, func() { + _, _, err := opts.projectOrName(t.Context(), nil) + assert.NilError(t, err) + }) + + assert.Equal(t, len(messages), 0) +} + +func TestToProjectName_DoesNotWarnOnUnsupportedAttributes(t *testing.T) { + opts := &ProjectOptions{ConfigPaths: []string{unsupportedAttrFixture(t)}} + + messages := captureWarnings(t, func() { + _, err := opts.toProjectName(t.Context(), nil) + assert.NilError(t, err) + }) + + assert.Equal(t, len(messages), 0) +} diff --git a/cmd/compose/viz.go b/cmd/compose/viz.go index 443d78c6261..3e085ad557d 100644 --- a/cmd/compose/viz.go +++ b/cmd/compose/viz.go @@ -73,7 +73,7 @@ func runViz(ctx context.Context, dockerCli command.Cli, backendOptions *BackendO return err } - project, _, err := opts.ToProject(ctx, dockerCli, backend, nil) + project, _, err := opts.ToProject(ctx, dockerCli, backend, nil, warnUnsupportedAttributes) if err != nil { return err } diff --git a/cmd/compose/watch.go b/cmd/compose/watch.go index 039971ce23f..3e962171329 100644 --- a/cmd/compose/watch.go +++ b/cmd/compose/watch.go @@ -72,7 +72,7 @@ func runWatch(ctx context.Context, dockerCli command.Cli, backendOptions *Backen return err } - project, _, err := watchOpts.ToProject(ctx, dockerCli, backend, services, cli.WithoutEnvironmentResolution) + project, _, err := watchOpts.ToProject(ctx, dockerCli, backend, services, warnUnsupportedAttributes, cli.WithoutEnvironmentResolution) if err != nil { return err } diff --git a/go.mod b/go.mod index 231506eebe9..9c3a77c5b08 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/Microsoft/go-winio v0.6.3-0.20251027160822-ad3df93bed29 github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d github.com/buger/goterm v1.0.4 - github.com/compose-spec/compose-go/v2 v2.15.0 + github.com/compose-spec/compose-go/v2 v2.15.1-0.20260908103050-cda18529aca7 github.com/containerd/console v1.0.5 github.com/containerd/containerd/v2 v2.3.5 github.com/containerd/errdefs v1.0.0 diff --git a/go.sum b/go.sum index 7ab5bb19061..2c2302dfa26 100644 --- a/go.sum +++ b/go.sum @@ -32,8 +32,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.15.0 h1:tdQw+eMyT+P6ZIb09JfcIVvbMmIa+PjST7cWezVLf00= -github.com/compose-spec/compose-go/v2 v2.15.0/go.mod h1:Q1+qtN4vhzEjGrnqRtzx1xa8raDZQlMUe3WJxndYNiQ= +github.com/compose-spec/compose-go/v2 v2.15.1-0.20260908103050-cda18529aca7 h1:D9jScaeUAk7DjyB71SD9zvOFPR6klw0FxPcR2bgu72c= +github.com/compose-spec/compose-go/v2 v2.15.1-0.20260908103050-cda18529aca7/go.mod h1:Q1+qtN4vhzEjGrnqRtzx1xa8raDZQlMUe3WJxndYNiQ= 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/api/api.go b/pkg/api/api.go index a8c21a17c9e..a4c6859da34 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -74,6 +74,12 @@ type ProjectLoadOptions struct { // This is optional - pass nil or empty slice if not needed. LoadListeners []LoadListener + // OnUnsupportedAttribute, when set, is invoked once during loading with + // every compose-file attribute detected as unsupported by this runtime + // outside Swarm mode. Detection only runs when this is set; leave nil to + // skip it entirely (e.g. for name-only project resolution). + OnUnsupportedAttribute func([]UnsupportedAttribute) + OCI OCIOptions } @@ -161,9 +167,27 @@ type Compose interface { // Volumes executes the equivalent to a `docker volume ls` Volumes(ctx context.Context, project string, options VolumesOptions) ([]VolumesSummary, error) // LoadProject loads and validates a Compose project from configuration files. + // Set ProjectLoadOptions.OnUnsupportedAttribute to also be notified of + // compose-file attributes accepted by the schema but not honored by this + // runtime outside Swarm mode. LoadProject(ctx context.Context, options ProjectLoadOptions) (*types.Project, error) } +// UnsupportedAttribute reports a compose-file attribute that is accepted by +// the schema but has no effect on this runtime outside Swarm mode. +type UnsupportedAttribute struct { + Service string // service name; empty for project-scoped attributes + Path string // dotted attribute path, e.g. "deploy.update_config.failure_action" + Reason string // one-line human-readable explanation, ready to print as-is +} + +func (u UnsupportedAttribute) String() string { + if u.Service == "" { + return fmt.Sprintf("%s: %s", u.Path, u.Reason) + } + return fmt.Sprintf("service %q: %s: %s", u.Service, u.Path, u.Reason) +} + type VolumesOptions struct { Services []string } diff --git a/pkg/compose/create.go b/pkg/compose/create.go index b30ce1a65c1..2f24e8504ac 100644 --- a/pkg/compose/create.go +++ b/pkg/compose/create.go @@ -1229,10 +1229,6 @@ func buildContainerConfigMounts(p types.Project, s types.ServiceConfig) ([]mount continue } - if config.UID != "" || config.GID != "" || config.Mode != nil { - logrus.Warn("config `uid`, `gid` and `mode` are not supported, they will be ignored") - } - bindMount, err := buildMount(p, types.ServiceVolumeConfig{ Type: types.VolumeTypeBind, Source: definedConfig.File, @@ -1279,10 +1275,6 @@ func buildContainerSecretMounts(p types.Project, s types.ServiceConfig) ([]mount continue } - if secret.UID != "" || secret.GID != "" || secret.Mode != nil { - logrus.Warn("secrets `uid`, `gid` and `mode` are not supported, they will be ignored") - } - if _, err := os.Stat(definedSecret.File); os.IsNotExist(err) { logrus.Warnf("secret file %s does not exist", definedSecret.Name) } diff --git a/pkg/compose/loader.go b/pkg/compose/loader.go index 9a760b06078..f2bdc5014ed 100644 --- a/pkg/compose/loader.go +++ b/pkg/compose/loader.go @@ -113,6 +113,10 @@ func (s *composeService) buildProjectOptions(options api.ProjectLoadOptions, rem cli.WithName(options.ProjectName), ) + if options.OnUnsupportedAttribute != nil { + projectOptionsFns = append(projectOptionsFns, unsupportedAttributesLoadOption(options.OnUnsupportedAttribute)) + } + return cli.NewProjectOptions(options.ConfigPaths, append(options.ProjectOptionsFns, projectOptionsFns...)...) } diff --git a/pkg/compose/service_containers.go b/pkg/compose/service_containers.go index bf6486bbfe6..5f37a0da57d 100644 --- a/pkg/compose/service_containers.go +++ b/pkg/compose/service_containers.go @@ -184,7 +184,7 @@ func (s *composeService) waitDependencies(ctx context.Context, project *types.Pr } eg.Go(func() error { - return s.waitDependency(ctx, dep, config, waitingFor) + return s.waitDependency(ctx, dependant, dep, config, waitingFor) }) } err := eg.Wait() @@ -197,7 +197,7 @@ func (s *composeService) waitDependencies(ctx context.Context, project *types.Pr // waitDependency polls the dependency's containers until its depends_on // condition is satisfied (done), definitively failed (err), or ctx is // cancelled. Each check reports (done, err): (false, nil) means keep polling. -func (s *composeService) waitDependency(ctx context.Context, dep string, config types.ServiceDependency, waitingFor Containers) error { +func (s *composeService) waitDependency(ctx context.Context, dependant, dep string, config types.ServiceDependency, waitingFor Containers) error { ticker := time.NewTicker(500 * time.Millisecond) defer ticker.Stop() for { @@ -223,7 +223,20 @@ func (s *composeService) waitDependency(ctx context.Context, dep string, config case types.ServiceConditionCompletedSuccessfully: done, err = s.checkDependencyCompleted(ctx, dep, config, waitingFor) default: - logrus.Warnf("unsupported depends_on condition: %s", config.Condition) + // Every condition this switch doesn't handle explicitly ends up + // here: ServiceConditionStarted is filtered out before this + // function is ever called (shouldWaitForDependency — "already + // managed by InDependencyOrder"), so nothing reaching this + // branch is ever a value this runtime actually understands. + // That covers a compose file with an unrecognized condition + // (rejected by schema.Validate before it can even get this far + // — see unsupported_attributes.go's comment on + // valueConditionalAttributes) as well as the two situations + // that never go through schema validation at all: a project + // rebuilt from live container labels (see projectFromName), or + // one written by an older/newer Compose version using a + // condition value this build doesn't recognize. + logrus.Warnf("service %q: unsupported depends_on condition %q, skipping wait for %q", dependant, config.Condition, dep) return nil } if done || err != nil { diff --git a/pkg/compose/service_containers_test.go b/pkg/compose/service_containers_test.go index a8db780b483..c36dda510b2 100644 --- a/pkg/compose/service_containers_test.go +++ b/pkg/compose/service_containers_test.go @@ -30,6 +30,7 @@ import ( "github.com/moby/moby/api/types/container" "github.com/moby/moby/api/types/network" "github.com/moby/moby/client" + logrustest "github.com/sirupsen/logrus/hooks/test" "go.uber.org/mock/gomock" "gotest.tools/v3/assert" @@ -812,4 +813,22 @@ func TestWaitDependencyDeadline(t *testing.T) { err := tested.(*composeService).waitDependencies(ctx, &project, "app", dependencies, containers, 0) assert.NilError(t, err) }) + + // Regression guard for the runtime fallback warning on waitDependency's + // default branch — see the comment on that branch for why it must stay. + t.Run("unsupported condition warns and returns without waiting", func(t *testing.T) { + hook := logrustest.NewGlobal() + unsupportedDeps := types.DependsOnConfig{ + "db": {Condition: "some_future_condition", Required: true}, + } + err := tested.(*composeService).waitDependencies(t.Context(), &project, "app", unsupportedDeps, containers, 2*time.Second) + assert.NilError(t, err) + + var messages []string + for _, e := range hook.AllEntries() { + messages = append(messages, e.Message) + } + joined := strings.Join(messages, "\n") + assert.Assert(t, strings.Contains(joined, `service "app": unsupported depends_on condition "some_future_condition"`), joined) + }) } diff --git a/pkg/compose/unsupported_attributes.go b/pkg/compose/unsupported_attributes.go new file mode 100644 index 00000000000..64c8ebade41 --- /dev/null +++ b/pkg/compose/unsupported_attributes.go @@ -0,0 +1,300 @@ +/* + 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 ( + "cmp" + "fmt" + "slices" + "strings" + "sync" + + "github.com/compose-spec/compose-go/v2/cli" + "github.com/compose-spec/compose-go/v2/loader" + "github.com/compose-spec/compose-go/v2/schema" + "github.com/compose-spec/compose-go/v2/tree" + + "github.com/docker/compose/v5/pkg/api" +) + +// genericUnsupportedReason is used for a finding that matches neither +// presenceUnsupportedAttributes nor valueConditionalAttributes: an attribute +// the compose-spec schema gained after this file was last updated, caught +// only because it's absent from supportedAttributePaths. This is the whole +// point of allowlisting instead of denylisting — see that var's comment. +const genericUnsupportedReason = "not honored by this runtime outside Swarm mode" + +// presenceUnsupportedAttributes lists, by path, every schema-declared +// attribute this runtime never honors outside Swarm mode regardless of its +// value — presence alone is the finding. Their paths are removed from +// schema.AttributePaths() (the full compose-spec inventory) to build +// supportedAttributePaths, the allowlist handed to compose-go via +// loader.WithSupportedAttributes: any OTHER attribute the specification +// adds later is reported too, with genericUnsupportedReason, until it's +// deliberately added here (or to valueConditionalAttributes) with a +// considered message. This is what makes the check fail-closed instead of +// silently missing whatever gets added to the spec next — the exact gap +// docker/compose#13150 was filed for. +var presenceUnsupportedAttributes = map[tree.Path]string{ + tree.NewPath("services", "*", "deploy", "mode"): "deploy.mode is only honored in Swarm mode", + tree.NewPath("services", "*", "deploy", "labels"): "deploy.labels are only applied to Swarm services", + tree.NewPath("services", "*", "deploy", "update_config"): "deploy.update_config only applies to rolling updates in Swarm mode", + tree.NewPath("services", "*", "deploy", "rollback_config"): "deploy.rollback_config only applies to rolling updates in Swarm mode", + tree.NewPath("services", "*", "deploy", "endpoint_mode"): "deploy.endpoint_mode only applies to Swarm's routing mesh", + tree.NewPath("services", "*", "deploy", "placement"): "deploy.placement is only honored by the Swarm scheduler", + tree.NewPath("services", "*", "credential_spec"): "credential_spec is only supported when running Windows containers under Swarm", + tree.NewPath("configs", "*", "labels"): "configs[].labels are only applied to Swarm config objects", + tree.NewPath("secrets", "*", "driver_opts"): "secrets[].driver_opts is only honored by Swarm secret drivers", + tree.NewPath("secrets", "*", "labels"): "secrets[].labels are only applied to Swarm secret objects", +} + +// supportedAttributePaths is the allowlist handed to compose-go: every +// compose-spec attribute path except the ones this runtime deliberately +// never honors (presenceUnsupportedAttributes) and their descendants. +// Several removed paths (deploy.update_config, deploy.rollback_config, +// deploy.placement, credential_spec) are themselves objects with their own +// schema-declared sub-fields: schema.AttributePaths() lists those +// sub-fields as separate entries too, so removing only the exact parent +// path would leave them in Supported — making compose-go's walk see a +// supported descendant below the removed node (Matcher.MayContain) and +// descend into it instead of reporting the parent as a whole, silently +// undoing the removal. Stripping every path that has a removed one as a +// prefix — not just an exact match — is what makes the parent-level removal +// actually take. +// +// Computed lazily (schema.AttributePaths() parses the embedded compose-spec +// schema on first call) rather than at package init: most compose commands +// never set OnUnsupportedAttribute at all (e.g. shell completion, or any +// command that skips the warning — see unsupportedAttributesLoadOption's +// only caller), so paying this cost eagerly for every invocation would be +// wasted on them. +var supportedAttributePaths = sync.OnceValue(buildSupportedAttributePaths) + +func buildSupportedAttributePaths() []tree.Path { + all := schema.AttributePaths() + supported := make([]tree.Path, 0, len(all)) + for _, path := range all { + if isUnderAnyOf(path, presenceUnsupportedAttributes) { + continue + } + supported = append(supported, path) + } + return supported +} + +// isUnderAnyOf reports whether path equals, or is a descendant of, one of +// removed's keys: truncated to the prefix's own length, path matches it +// under tree.Path's usual "*"/"[]" wildcard rules. +func isUnderAnyOf(path tree.Path, removed map[tree.Path]string) bool { + pathParts := path.Parts() + for prefix := range removed { + prefixParts := prefix.Parts() + if len(pathParts) < len(prefixParts) { + continue + } + if tree.NewPath(pathParts[:len(prefixParts)]...).Matches(prefix) { + return true + } + } + return false +} + +// valueConditionalAttributes lists the checks that need a value predicate, +// not mere presence: compose-go's loader defaults each of these paths to a +// common, supported value on virtually every occurrence (ports[].mode to +// "ingress", volumes[].type to "bind"/"volume", every depends_on entry has +// some condition), so presence alone would fire on nearly every service — +// only a specific value is unsupported. They can never be derived from +// schema.AttributePaths() (compose-go's allowlist only knows about presence, +// not values), so they stay hand-written regardless of how +// presenceUnsupportedAttributes evolves. +// +// ports[].mode, volumes[].type and the two file-reference checks target the +// whole node (the map compose-go hands back for that path), not a scalar +// leaf: identifying which port/volume/reference triggered the finding needs +// a sibling field (target/protocol, source) only visible from that node — a +// list index collapses to the literal token "[]" in the reported Path, +// losing identity, unlike a map key (a service or dependency name), which +// stays. +// +// depends_on.*.condition has no entry here even though it's the same kind +// of "only some values are supported" case: the compose-spec schema itself +// declares condition as a closed enum of the 3 values this runtime +// understands, so schema.Validate — which runs before this check, on every +// load — already rejects anything else. There is no unsupported value left +// for a real compose file to reach this check with (see +// TestDependsOnUnknownConditionIsRejectedBySchema). waitDependency's own +// runtime fallback (service_containers.go) still needs to warn about an +// unrecognized condition, for the two situations that never go through +// schema validation at all: a project rebuilt from container labels, or an +// older/newer Compose version's output. +var valueConditionalAttributes = []struct { + Pattern tree.Path + Detect func(value any) bool + // Report returns the api.UnsupportedAttribute(s) for a match — Service + // is always left blank here; toAPIUnsupportedAttributes fills it in + // from the finding's Path, which Report doesn't have direct access to. + Report func(finding loader.UnsupportedAttribute) []api.UnsupportedAttribute +}{ + { + Pattern: tree.NewPath("services", "*", "ports", "[]"), + Detect: func(v any) bool { + port, _ := v.(map[string]any) + return port["mode"] == "host" + }, + Report: func(finding loader.UnsupportedAttribute) []api.UnsupportedAttribute { + port, _ := finding.Value.(map[string]any) + path := fmt.Sprintf("ports[%v/%v].mode", port["target"], port["protocol"]) + return []api.UnsupportedAttribute{{Path: path, Reason: "ports[].mode: host is only honored by the Swarm routing mesh"}} + }, + }, + { + Pattern: tree.NewPath("services", "*", "volumes", "[]"), + Detect: func(v any) bool { + volume, _ := v.(map[string]any) + return volume["type"] == "cluster" + }, + Report: func(finding loader.UnsupportedAttribute) []api.UnsupportedAttribute { + volume, _ := finding.Value.(map[string]any) + path := fmt.Sprintf("volumes[%v].type", volume["source"]) + return []api.UnsupportedAttribute{{Path: path, Reason: "volumes[].type: cluster (CSI) volumes are only supported in Swarm mode"}} + }, + }, + { + Pattern: tree.NewPath("services", "*", "configs", "[]"), + Detect: hasFileReferenceOverride, + Report: fileReferenceReport("configs"), + }, + { + Pattern: tree.NewPath("services", "*", "secrets", "[]"), + Detect: hasFileReferenceOverride, + Report: fileReferenceReport("secrets"), + }, +} + +func hasFileReferenceOverride(v any) bool { + ref, _ := v.(map[string]any) + return ref["uid"] != nil || ref["gid"] != nil || ref["mode"] != nil +} + +// fileReferenceReport drives the configs/secrets entries above: uid/gid/mode +// on a service-level configs:/secrets: reference are silently ignored +// outside Swarm mode. Each non-nil sub-field on the matched reference +// produces its own finding, identified by the reference's source, so +// multiple references — or multiple flagged sub-fields on the same one — +// stay distinguishable. +func fileReferenceReport(kind string) func(loader.UnsupportedAttribute) []api.UnsupportedAttribute { + return func(finding loader.UnsupportedAttribute) []api.UnsupportedAttribute { + ref, _ := finding.Value.(map[string]any) + // source is schema-optional on the long form (compose-spec's + // service_config_or_secret has no "required"), so a malformed + // reference like `configs: [{uid: "1000"}]` is valid enough to + // reach here without one. + source, _ := ref["source"].(string) + if source == "" { + source = "(anonymous)" + } + var findings []api.UnsupportedAttribute + for _, field := range []string{"uid", "gid", "mode"} { + if ref[field] == nil { + continue + } + findings = append(findings, api.UnsupportedAttribute{ + Path: fmt.Sprintf("%s.%s.%s", kind, source, field), + Reason: field + " is not supported outside Swarm mode and will be ignored", + }) + } + return findings + } +} + +// splitServicePath splits a "services....." finding into its service +// name and the remaining dotted path (matching this package's existing +// attribute-path convention); a project-scoped finding (configs.*/secrets.*) +// has no service and keeps its full path as-is — it already carries its +// resource name as a literal segment. +func splitServicePath(path tree.Path) (service, outputPath string) { + parts := path.Parts() + if len(parts) >= 2 && parts[0] == "services" { + return parts[1], strings.Join(parts[2:], ".") + } + return "", path.String() +} + +// toAPIUnsupportedAttributes converts compose-go's raw findings to this +// package's api.UnsupportedAttribute. A finding either matches one of +// valueConditionalAttributes (which knows how to render it from Value), or +// it came from the supportedAttributePaths allowlist screening — in which +// case presenceUnsupportedAttributes supplies the reason when this is a +// known case, and genericUnsupportedReason otherwise. +func toAPIUnsupportedAttributes(findings []loader.UnsupportedAttribute) []api.UnsupportedAttribute { + var result []api.UnsupportedAttribute + for _, finding := range findings { + service, outputPath := splitServicePath(finding.Path) + + if vc, findings := matchValueConditional(finding); vc { + for i := range findings { + findings[i].Service = service + } + result = append(result, findings...) + continue + } + + reason := genericUnsupportedReason + for pattern, r := range presenceUnsupportedAttributes { + if finding.Path.Matches(pattern) { + reason = r + break + } + } + result = append(result, api.UnsupportedAttribute{Service: service, Path: outputPath, Reason: reason}) + } + slices.SortFunc(result, func(a, b api.UnsupportedAttribute) int { + if c := cmp.Compare(a.Service, b.Service); c != 0 { + return c + } + return cmp.Compare(a.Path, b.Path) + }) + return result +} + +func matchValueConditional(finding loader.UnsupportedAttribute) (bool, []api.UnsupportedAttribute) { + for _, vc := range valueConditionalAttributes { + if finding.Path.Matches(vc.Pattern) { + return true, vc.Report(finding) + } + } + return false, nil +} + +// unsupportedAttributesLoadOption registers compose-go's unsupported-attribute +// detection with the loader — both the value-conditional patterns and the +// supported-paths allowlist feed the same walk — invoking report once +// loading completes with every match translated to api.UnsupportedAttribute. +func unsupportedAttributesLoadOption(report func([]api.UnsupportedAttribute)) cli.ProjectOptionsFn { + patterns := make([]loader.UnsupportedAttributePattern, len(valueConditionalAttributes)) + for i, vc := range valueConditionalAttributes { + patterns[i] = loader.UnsupportedAttributePattern{Path: vc.Pattern, Detect: vc.Detect} + } + wrap := func(findings []loader.UnsupportedAttribute) { + report(toAPIUnsupportedAttributes(findings)) + } + return cli.WithLoadOptions( + loader.WithUnsupportedAttributesCheck(patterns, wrap), + loader.WithSupportedAttributes(supportedAttributePaths(), nil), + ) +} diff --git a/pkg/compose/unsupported_attributes_test.go b/pkg/compose/unsupported_attributes_test.go new file mode 100644 index 00000000000..12bd08e4b83 --- /dev/null +++ b/pkg/compose/unsupported_attributes_test.go @@ -0,0 +1,664 @@ +/* + 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 ( + "cmp" + "os" + "path/filepath" + "slices" + "testing" + + "github.com/compose-spec/compose-go/v2/schema" + "github.com/google/go-cmp/cmp/cmpopts" + "gotest.tools/v3/assert" + + "github.com/docker/compose/v5/pkg/api" +) + +// findingKey is the subset of api.UnsupportedAttribute we compare against a +// fixed expectation: Reason wording isn't part of the contract, only that a +// finding was raised for a given Service/Path pair (and that it carries a +// non-empty Reason, checked separately). +type findingKey struct { + Service string + Path string +} + +func findingKeys(t *testing.T, findings []api.UnsupportedAttribute) []findingKey { + t.Helper() + keys := make([]findingKey, 0, len(findings)) + for _, f := range findings { + assert.Assert(t, f.Reason != "", "finding %+v is missing a Reason", f) + keys = append(keys, findingKey{Service: f.Service, Path: f.Path}) + } + return keys +} + +// loadWithFiles writes files (name -> content) into a fresh temp dir and +// loads "compose.yaml" from it through the real compose-go loader with +// unsupported-attribute detection enabled, returning every finding. +// Detection is now inherently a load-time hook (compose-go's own +// tree-walker over the raw, un-normalized model): there is no longer a +// pure function to call directly on a hand-built types.Project, so every +// case here goes through an actual load. +func loadWithFiles(t *testing.T, files map[string]string) []api.UnsupportedAttribute { + t.Helper() + tmpDir := t.TempDir() + for name, content := range files { + path := filepath.Join(tmpDir, name) + assert.NilError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + assert.NilError(t, os.WriteFile(path, []byte(content), 0o644)) + } + + service, err := NewComposeService(nil) + assert.NilError(t, err) + + var findings []api.UnsupportedAttribute + _, err = service.LoadProject(t.Context(), api.ProjectLoadOptions{ + ConfigPaths: []string{filepath.Join(tmpDir, "compose.yaml")}, + OnUnsupportedAttribute: func(f []api.UnsupportedAttribute) { + findings = f + }, + }) + assert.NilError(t, err) + return findings +} + +func loadCompose(t *testing.T, yaml string) []api.UnsupportedAttribute { + t.Helper() + return loadWithFiles(t, map[string]string{"compose.yaml": yaml}) +} + +func assertFindings(t *testing.T, got []api.UnsupportedAttribute, expected []findingKey) { + t.Helper() + slices.SortFunc(expected, func(a, b findingKey) int { + if c := cmp.Compare(a.Service, b.Service); c != 0 { + return c + } + return cmp.Compare(a.Path, b.Path) + }) + assert.DeepEqual(t, findingKeys(t, got), expected, cmpopts.EquateEmpty()) +} + +func TestUnsupportedAttributes(t *testing.T) { + t.Run("deploy mode is unsupported", func(t *testing.T) { + got := loadCompose(t, ` +services: + web: + image: alpine + deploy: + mode: replicated +`) + assertFindings(t, got, []findingKey{{Service: "web", Path: "deploy.mode"}}) + }) + + t.Run("deploy labels is unsupported", func(t *testing.T) { + got := loadCompose(t, ` +services: + web: + image: alpine + deploy: + labels: + team: backend +`) + assertFindings(t, got, []findingKey{{Service: "web", Path: "deploy.labels"}}) + }) + + t.Run("deploy update_config is unsupported", func(t *testing.T) { + got := loadCompose(t, ` +services: + web: + image: alpine + deploy: + update_config: + parallelism: 1 +`) + assertFindings(t, got, []findingKey{{Service: "web", Path: "deploy.update_config"}}) + }) + + t.Run("deploy rollback_config is unsupported", func(t *testing.T) { + got := loadCompose(t, ` +services: + web: + image: alpine + deploy: + rollback_config: + parallelism: 1 +`) + assertFindings(t, got, []findingKey{{Service: "web", Path: "deploy.rollback_config"}}) + }) + + // Placement covers three independent sub-fields (constraints, + // preferences, max_replicas_per_node). A single finding is reported for + // the whole "deploy.placement" node regardless of which sub-field(s) + // triggered it, rather than one finding per sub-field: they're only + // ever set together in practice and placement has no further nesting + // worth distinguishing in the report. + t.Run("deploy placement constraints is unsupported", func(t *testing.T) { + got := loadCompose(t, ` +services: + web: + image: alpine + deploy: + placement: + constraints: + - node.role==manager +`) + assertFindings(t, got, []findingKey{{Service: "web", Path: "deploy.placement"}}) + }) + + t.Run("deploy placement preferences is unsupported", func(t *testing.T) { + got := loadCompose(t, ` +services: + web: + image: alpine + deploy: + placement: + preferences: + - spread: node.labels.zone +`) + assertFindings(t, got, []findingKey{{Service: "web", Path: "deploy.placement"}}) + }) + + t.Run("deploy placement max_replicas_per_node is unsupported", func(t *testing.T) { + got := loadCompose(t, ` +services: + web: + image: alpine + deploy: + placement: + max_replicas_per_node: 2 +`) + assertFindings(t, got, []findingKey{{Service: "web", Path: "deploy.placement"}}) + }) + + t.Run("deploy endpoint_mode is unsupported", func(t *testing.T) { + got := loadCompose(t, ` +services: + web: + image: alpine + deploy: + endpoint_mode: vip +`) + assertFindings(t, got, []findingKey{{Service: "web", Path: "deploy.endpoint_mode"}}) + }) + + t.Run("credential_spec is unsupported", func(t *testing.T) { + got := loadWithFiles(t, map[string]string{ + "compose.yaml": ` +services: + web: + image: alpine + credential_spec: + file: ./creds.json +`, + "creds.json": "{}", + }) + assertFindings(t, got, []findingKey{{Service: "web", Path: "credential_spec"}}) + }) + + // label_file is NOT in presenceUnsupportedAttributes: compose-go's + // loader resolves it into the service's actual Labels by default + // (Options.SkipResolveLabels, which docker/compose never sets) — it's + // fully functional here, unlike the genuinely Swarm-only attributes + // above. This guards against it being re-added by mistake. + t.Run("label_file is honored, no finding", func(t *testing.T) { + tmpDir := t.TempDir() + assert.NilError(t, os.WriteFile(filepath.Join(tmpDir, "labels.env"), []byte("team=backend\n"), 0o644)) + content := ` +services: + web: + image: alpine + label_file: + - ./labels.env +` + assert.NilError(t, os.WriteFile(filepath.Join(tmpDir, "compose.yaml"), []byte(content), 0o644)) + + service, err := NewComposeService(nil) + assert.NilError(t, err) + var got []api.UnsupportedAttribute + project, err := service.LoadProject(t.Context(), api.ProjectLoadOptions{ + ConfigPaths: []string{filepath.Join(tmpDir, "compose.yaml")}, + OnUnsupportedAttribute: func(f []api.UnsupportedAttribute) { got = f }, + }) + assert.NilError(t, err) + assertFindings(t, got, nil) + assert.Equal(t, project.Services["web"].Labels["team"], "backend") + }) + + t.Run("port mode host is unsupported", func(t *testing.T) { + got := loadCompose(t, ` +services: + web: + image: alpine + ports: + - target: 80 + published: "8080" + protocol: tcp + mode: host +`) + assertFindings(t, got, []findingKey{{Service: "web", Path: "ports[80/tcp].mode"}}) + }) + + // Each host-mode port is identified by its target port AND protocol: the + // same target port can be published for both tcp and udp, and the two + // must stay distinguishable instead of collapsing into identical lines. + t.Run("same target port on different protocols is reported separately", func(t *testing.T) { + got := loadCompose(t, ` +services: + web: + image: alpine + ports: + - target: 80 + published: "8080" + protocol: tcp + mode: host + - target: 80 + published: "8080" + protocol: udp + mode: host +`) + assertFindings(t, got, []findingKey{ + {Service: "web", Path: "ports[80/tcp].mode"}, + {Service: "web", Path: "ports[80/udp].mode"}, + }) + }) + + // compose-go's loader defaults an unset ports[].mode to "ingress" for + // every port entry, so a plain published port must NOT produce a + // finding. + t.Run("port mode ingress (the loader default) produces no finding", func(t *testing.T) { + got := loadCompose(t, ` +services: + web: + image: alpine + ports: + - "8080:80" +`) + assertFindings(t, got, nil) + }) + + t.Run("volume type cluster is unsupported", func(t *testing.T) { + got := loadCompose(t, ` +services: + web: + image: alpine + volumes: + - type: cluster + source: csi-vol + target: /data +`) + assertFindings(t, got, []findingKey{{Service: "web", Path: "volumes[csi-vol].type"}}) + }) + + // Each cluster volume is identified by its source, so multiple such + // volumes on the same service stay distinguishable. + t.Run("multiple cluster volumes on the same service are reported separately", func(t *testing.T) { + got := loadCompose(t, ` +services: + web: + image: alpine + volumes: + - type: cluster + source: csi-vol-1 + target: /data1 + - type: cluster + source: csi-vol-2 + target: /data2 +`) + assertFindings(t, got, []findingKey{ + {Service: "web", Path: "volumes[csi-vol-1].type"}, + {Service: "web", Path: "volumes[csi-vol-2].type"}, + }) + }) + + // A bind mount, the most common volume shape, must never be flagged: + // compose-go's loader defaults an unset volumes[].type to "bind" or + // "volume" for every entry, so checking for mere presence of "type" + // would fire on virtually every service using volumes. + t.Run("volume type bind (the loader default) produces no finding", func(t *testing.T) { + got := loadCompose(t, ` +services: + web: + image: alpine + volumes: + - /host:/container +`) + assertFindings(t, got, nil) + }) + + // Migrated from the ad-hoc warning previously logged in create.go: a + // config file reference's uid/gid/mode are silently ignored outside + // Swarm mode. Each non-zero sub-field is reported individually, and the + // path names the source so multiple config references stay + // distinguishable. + t.Run("service config file reference uid gid mode are unsupported", func(t *testing.T) { + got := loadWithFiles(t, map[string]string{ + "compose.yaml": ` +services: + web: + image: alpine + configs: + - source: c1 + uid: "1000" + gid: "1000" + mode: 0o400 +configs: + c1: + file: ./c1.txt +`, + "c1.txt": "hello", + }) + assertFindings(t, got, []findingKey{ + {Service: "web", Path: "configs.c1.uid"}, + {Service: "web", Path: "configs.c1.gid"}, + {Service: "web", Path: "configs.c1.mode"}, + }) + }) + + // Two config references with the same violation must stay + // distinguishable by source name, not collapse into identical findings. + t.Run("two config file references are reported separately by source", func(t *testing.T) { + got := loadWithFiles(t, map[string]string{ + "compose.yaml": ` +services: + web: + image: alpine + configs: + - source: c1 + uid: "1000" + - source: c2 + uid: "1000" +configs: + c1: + file: ./c1.txt + c2: + file: ./c2.txt +`, + "c1.txt": "hello", + "c2.txt": "hello", + }) + assertFindings(t, got, []findingKey{ + {Service: "web", Path: "configs.c1.uid"}, + {Service: "web", Path: "configs.c2.uid"}, + }) + }) + + // Migrated from the ad-hoc warning previously logged in create.go: same + // as above, for secret file references. + t.Run("service secret file reference uid gid mode are unsupported", func(t *testing.T) { + got := loadWithFiles(t, map[string]string{ + "compose.yaml": ` +services: + web: + image: alpine + secrets: + - source: s1 + uid: "1000" + gid: "1000" + mode: 0o400 +secrets: + s1: + file: ./s1.txt +`, + "s1.txt": "hello", + }) + assertFindings(t, got, []findingKey{ + {Service: "web", Path: "secrets.s1.uid"}, + {Service: "web", Path: "secrets.s1.gid"}, + {Service: "web", Path: "secrets.s1.mode"}, + }) + }) + + // A config/secret reference with no uid/gid/mode override must never be + // flagged: only the overrides are unsupported, not the reference itself. + t.Run("config file reference without overrides produces no finding", func(t *testing.T) { + got := loadWithFiles(t, map[string]string{ + "compose.yaml": ` +services: + web: + image: alpine + configs: + - source: c1 +configs: + c1: + file: ./c1.txt +`, + "c1.txt": "hello", + }) + assertFindings(t, got, nil) + }) + + // An unrecognized depends_on condition is not tested here: the + // compose-spec schema declares it as a closed enum, so schema.Validate + // rejects it before this check ever runs — see + // TestDependsOnUnknownConditionIsRejectedBySchema below. + + t.Run("depends_on known condition produces no finding", func(t *testing.T) { + got := loadCompose(t, ` +services: + web: + image: alpine + depends_on: + db: + condition: service_healthy + db: + image: alpine +`) + assertFindings(t, got, nil) + }) + + // The short-form depends_on (a plain list of service names) never + // writes an explicit condition, so it must never be flagged either. + t.Run("depends_on short form produces no finding", func(t *testing.T) { + got := loadCompose(t, ` +services: + web: + image: alpine + depends_on: + - db + db: + image: alpine +`) + assertFindings(t, got, nil) + }) + + t.Run("project-scoped config labels is unsupported", func(t *testing.T) { + got := loadWithFiles(t, map[string]string{ + "compose.yaml": ` +services: + web: + image: alpine +configs: + c1: + file: ./c1.txt + labels: + team: backend +`, + "c1.txt": "hello", + }) + assertFindings(t, got, []findingKey{{Service: "", Path: "configs.c1.labels"}}) + }) + + // Unlike secrets, the compose-spec schema has no driver/driver_opts + // property on a top-level configs entry (additionalProperties: false + // rejects it at load time), so there is no equivalent case to test here + // — a compose file setting it would fail to load entirely. + + t.Run("project-scoped secret driver_opts is unsupported", func(t *testing.T) { + got := loadWithFiles(t, map[string]string{ + "compose.yaml": ` +services: + web: + image: alpine +secrets: + s1: + file: ./s1.txt + driver_opts: + region: eu-west +`, + "s1.txt": "hello", + }) + assertFindings(t, got, []findingKey{{Service: "", Path: "secrets.s1.driver_opts"}}) + }) + + t.Run("project-scoped secret labels is unsupported", func(t *testing.T) { + got := loadWithFiles(t, map[string]string{ + "compose.yaml": ` +services: + web: + image: alpine +secrets: + s1: + file: ./s1.txt + labels: + team: backend +`, + "s1.txt": "hello", + }) + assertFindings(t, got, []findingKey{{Service: "", Path: "secrets.s1.labels"}}) + }) + + // --- False-positive traps: these MUST produce zero findings --- + + t.Run("deploy resources replicas and restart_policy are honored, no finding", func(t *testing.T) { + got := loadCompose(t, ` +services: + web: + image: alpine + deploy: + replicas: 3 + resources: + limits: + cpus: "0.50" + memory: 128M + restart_policy: + condition: on-failure +`) + assertFindings(t, got, nil) + }) + + t.Run("baseline clean service produces no finding", func(t *testing.T) { + got := loadCompose(t, ` +services: + web: + image: alpine + ports: + - "8080:80" + volumes: + - /host:/container +`) + assertFindings(t, got, nil) + }) + + t.Run("multiple findings on the same service are all reported", func(t *testing.T) { + got := loadWithFiles(t, map[string]string{ + "compose.yaml": ` +services: + web: + image: alpine + deploy: + mode: replicated + credential_spec: + file: ./creds.json +`, + "creds.json": "{}", + }) + assertFindings(t, got, []findingKey{ + {Service: "web", Path: "deploy.mode"}, + {Service: "web", Path: "credential_spec"}, + }) + }) +} + +// TestUnsupportedAttributes_SortedOutput asserts the reported findings are +// sorted: project services/configs/secrets are all Go maps, so without an +// explicit sort the order would vary between runs. +func TestUnsupportedAttributes_SortedOutput(t *testing.T) { + // 6 distinctly-ordered names: with only 2-3 services, random map + // iteration has a non-negligible chance of coincidentally coming out + // sorted, masking a regression if the production sort were ever + // removed. + got := loadCompose(t, ` +services: + zeta: + image: alpine + deploy: {mode: replicated} + echo: + image: alpine + deploy: {mode: replicated} + delta: + image: alpine + deploy: {mode: replicated} + charlie: + image: alpine + deploy: {mode: replicated} + bravo: + image: alpine + deploy: {mode: replicated} + alpha: + image: alpine + deploy: {mode: replicated} +`) + assert.Assert(t, slices.IsSortedFunc(got, func(a, b api.UnsupportedAttribute) int { + if c := cmp.Compare(a.Service, b.Service); c != 0 { + return c + } + return cmp.Compare(a.Path, b.Path) + }), "%+v", got) +} + +// TestPresenceUnsupportedAttributes_AreKnownSchemaPaths guards +// presenceUnsupportedAttributes against typos: every key must be a real +// compose-spec path, or it silently does nothing (never removed from +// supportedAttributePaths, never matched against a real finding). +func TestPresenceUnsupportedAttributes_AreKnownSchemaPaths(t *testing.T) { + all := schema.AttributePaths() + for path := range presenceUnsupportedAttributes { + assert.Assert(t, slices.Contains(all, path), "%q is not a known compose-spec attribute path", path) + } +} + +// TestDependsOnUnknownConditionIsRejectedBySchema documents why this +// package has no unsupported-attribute check for depends_on.*.condition: +// the compose-spec schema declares it as a closed enum of the 3 values this +// runtime understands, so an unrecognized value never reaches +// valueConditionalAttributes — it fails to load at all. waitDependency's +// own runtime fallback in service_containers.go covers the cases that +// bypass schema validation entirely (a project rebuilt from container +// labels, or one written by a different Compose version). +func TestDependsOnUnknownConditionIsRejectedBySchema(t *testing.T) { + tmpDir := t.TempDir() + composeFile := filepath.Join(tmpDir, "compose.yaml") + content := ` +services: + web: + image: alpine + depends_on: + db: + condition: service_ready + db: + image: alpine +` + assert.NilError(t, os.WriteFile(composeFile, []byte(content), 0o644)) + + service, err := NewComposeService(nil) + assert.NilError(t, err) + _, err = service.LoadProject(t.Context(), api.ProjectLoadOptions{ConfigPaths: []string{composeFile}}) + assert.ErrorContains(t, err, "condition") +} diff --git a/pkg/e2e/testdata/TestUnsupportedAttributesWarning/compose.yaml b/pkg/e2e/testdata/TestUnsupportedAttributesWarning/compose.yaml new file mode 100644 index 00000000000..d226b52b379 --- /dev/null +++ b/pkg/e2e/testdata/TestUnsupportedAttributesWarning/compose.yaml @@ -0,0 +1,82 @@ +services: + # Baseline: deploy.resources/replicas/restart_policy ARE honored outside + # Swarm, a port without an explicit mode defaults to mode: ingress, and + # label_file is resolved into real labels by compose-go's loader by + # default — none of these must ever produce a warning. + clean: + image: alpine + deploy: + replicas: 2 + resources: + limits: + cpus: "0.50" + memory: 128M + restart_policy: + condition: on-failure + ports: + - "9090:90" + label_file: + - ./labels.env + + # Every deploy.* sub-attribute that's silently ignored outside Swarm. + swarm-attrs: + image: alpine + credential_spec: + file: ./creds.json + deploy: + mode: replicated + labels: + team: backend + update_config: + parallelism: 1 + failure_action: rollback + rollback_config: + parallelism: 1 + placement: + constraints: + - node.role==manager + endpoint_mode: vip + + # ports[].mode: host is Swarm-only. + ports-demo: + image: alpine + ports: + - target: 80 + published: "8080" + mode: host + + # volumes[].type: cluster (CSI) has no effect outside Swarm. + cluster-vol: + image: alpine + volumes: + - type: cluster + source: my-csi-volume + target: /data + + # configs/secrets file-reference uid/gid/mode are ignored outside Swarm. + file-refs: + image: alpine + configs: + - source: myconfig + uid: "1000" + gid: "1000" + mode: 0o440 + secrets: + - source: mysecret + uid: "1000" + gid: "1000" + mode: 0o440 + +configs: + myconfig: + file: ./myconfig.txt + labels: + team: backend # configs[].labels: reachable and flagged + +secrets: + mysecret: + file: ./mysecret.txt + driver_opts: # secrets[].driver_opts: reachable and flagged (unlike configs) + region: eu-west + labels: + team: backend diff --git a/pkg/e2e/testdata/TestUnsupportedAttributesWarning/creds.json b/pkg/e2e/testdata/TestUnsupportedAttributesWarning/creds.json new file mode 100644 index 00000000000..de882ec7985 --- /dev/null +++ b/pkg/e2e/testdata/TestUnsupportedAttributesWarning/creds.json @@ -0,0 +1 @@ +{"config": "creds"} diff --git a/pkg/e2e/testdata/TestUnsupportedAttributesWarning/labels.env b/pkg/e2e/testdata/TestUnsupportedAttributesWarning/labels.env new file mode 100644 index 00000000000..5a05fe44b52 --- /dev/null +++ b/pkg/e2e/testdata/TestUnsupportedAttributesWarning/labels.env @@ -0,0 +1 @@ +SOME_LABEL=value diff --git a/pkg/e2e/testdata/TestUnsupportedAttributesWarning/myconfig.txt b/pkg/e2e/testdata/TestUnsupportedAttributesWarning/myconfig.txt new file mode 100644 index 00000000000..41789d20235 --- /dev/null +++ b/pkg/e2e/testdata/TestUnsupportedAttributesWarning/myconfig.txt @@ -0,0 +1 @@ +dummy config content diff --git a/pkg/e2e/testdata/TestUnsupportedAttributesWarning/mysecret.txt b/pkg/e2e/testdata/TestUnsupportedAttributesWarning/mysecret.txt new file mode 100644 index 00000000000..48168ff0f52 --- /dev/null +++ b/pkg/e2e/testdata/TestUnsupportedAttributesWarning/mysecret.txt @@ -0,0 +1 @@ +dummy secret content diff --git a/pkg/e2e/unsupported_attributes_test.go b/pkg/e2e/unsupported_attributes_test.go new file mode 100644 index 00000000000..39de2f09e8e --- /dev/null +++ b/pkg/e2e/unsupported_attributes_test.go @@ -0,0 +1,62 @@ +//go:build e2e + +/* + 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 e2e + +import "testing" + +// TestUnsupportedAttributesWarning locks docker/compose#13150: Compose must +// warn, at load time, when a compose-file attribute is accepted by the +// schema but has no effect on this runtime outside Swarm mode, instead of +// silently ignoring it. It also locks two false positives this check must +// NOT produce: a plain published port (the loader defaults ports[].mode to +// "ingress" for every port entry), and label_file (compose-go's loader +// resolves it into real service labels by default — it's fully functional, +// unlike the genuinely Swarm-only attributes here). configs[].driver_opts +// has no effect and no warning either, since the compose-spec schema has no +// such property on a top-level configs entry (unlike secrets[].driver_opts, +// which is schema-valid and is flagged). +func TestUnsupportedAttributesWarning(t *testing.T) { + // logrus's text formatter escapes embedded quotes in a field's value, so + // the raw output contains a literal backslash before each quote around + // the service name (`msg="service \"x\": ..."`) — don't "clean up" these + // backslashes, the checks stop matching if you do. + NewScenario(t, "compose must warn about schema-valid attributes it doesn't honor outside Swarm mode, and stay silent on the ones it does honor"). + Step("every schema-valid, silently-ignored attribute is reported, and honored/unreachable attributes are not", + ComposeCmd("config"), + OutputContains(`service \"swarm-attrs\": deploy.mode`), + OutputContains(`service \"swarm-attrs\": deploy.labels`), + OutputContains(`service \"swarm-attrs\": deploy.update_config`), + OutputContains(`service \"swarm-attrs\": deploy.rollback_config`), + OutputContains(`service \"swarm-attrs\": deploy.placement`), + OutputContains(`service \"swarm-attrs\": deploy.endpoint_mode`), + OutputContains(`service \"swarm-attrs\": credential_spec`), + OutputContains(`service \"ports-demo\": ports[80/tcp].mode`), + OutputContains(`service \"cluster-vol\": volumes[my-csi-volume].type`), + OutputContains(`service \"file-refs\": configs.myconfig.uid`), + OutputContains(`service \"file-refs\": configs.myconfig.gid`), + OutputContains(`service \"file-refs\": configs.myconfig.mode`), + OutputContains(`service \"file-refs\": secrets.mysecret.uid`), + OutputContains(`service \"file-refs\": secrets.mysecret.gid`), + OutputContains(`service \"file-refs\": secrets.mysecret.mode`), + OutputContains(`configs.myconfig.labels`), + OutputContains(`secrets.mysecret.driver_opts`), + OutputContains(`secrets.mysecret.labels`), + OutputNotContains(`service \"clean\"`), + OutputNotContains(`configs.myconfig.driver_opts`)) +}