diff --git a/cmd/compose/compose.go b/cmd/compose/compose.go index 52daddcb36..93e98aef58 100644 --- a/cmd/compose/compose.go +++ b/cmd/compose/compose.go @@ -31,6 +31,7 @@ import ( "github.com/compose-spec/compose-go/v2/cli" "github.com/compose-spec/compose-go/v2/dotenv" + "github.com/compose-spec/compose-go/v2/errdefs" "github.com/compose-spec/compose-go/v2/loader" composepaths "github.com/compose-spec/compose-go/v2/paths" "github.com/compose-spec/compose-go/v2/types" @@ -243,6 +244,20 @@ func defaultStringArrayVar(env string) []string { }) } +// projectOrName resolves the target project for commands that exploit the +// compose model when one is available and fall back to container labels +// otherwise. The project name follows one precedence everywhere, shared with +// toProjectName and applied identically by compose-go while loading: +// --project-name, then COMPOSE_PROJECT_NAME, then the model's name. +// +// When the model cannot be loaded: +// - an explicit --file is a hard error: the user named the file, failing +// to read it cannot be ignored; +// - no compose file around and a name available from COMPOSE_PROJECT_NAME +// is the normal file-less workflow: label-based mode, silently; +// - a compose file present but broken, with COMPOSE_PROJECT_NAME set, +// falls back to label-based mode with an explicit warning (this used to +// happen silently). func (o *ProjectOptions) projectOrName(ctx context.Context, dockerCli command.Cli, services ...string) (*types.Project, string, error) { name := o.ProjectName var project *types.Project @@ -254,8 +269,14 @@ func (o *ProjectOptions) projectOrName(ctx context.Context, dockerCli command.Cl p, _, err := o.ToProject(ctx, dockerCli, backend, services, cli.WithDiscardEnvFile, cli.WithoutEnvironmentResolution) if err != nil { + if len(o.ConfigPaths) > 0 { + return nil, "", err + } envProjectName := os.Getenv(ComposeProjectName) if envProjectName != "" { + if !errdefs.IsNotFoundError(err) { + logrus.Warnf("compose file found but could not be loaded (%s) — falling back to label-based mode for project %q", err, envProjectName) + } return nil, envProjectName, nil } return nil, "", err @@ -266,6 +287,31 @@ func (o *ProjectOptions) projectOrName(ctx context.Context, dockerCli command.Cl return project, name, nil } +// validateServiceNames rejects service arguments that don't exist in the +// loaded model — profile-disabled services are legitimate targets (commands +// like restart enable them on demand). With no model (label-based mode) no +// validation is possible: a name without containers cannot be told apart +// from a container already removed. +func validateServiceNames(project *types.Project, services []string) error { + if project == nil { + return nil + } + for _, service := range services { + if _, ok := project.Services[service]; ok { + continue + } + if _, ok := project.DisabledServices[service]; ok { + continue + } + return fmt.Errorf("no such service: %s", service) + } + return nil +} + +// toProjectName resolves the project name for commands that only need the +// name, never the model. Same precedence as projectOrName: --project-name, +// then COMPOSE_PROJECT_NAME, then the loaded model's name — the two first +// short-circuit the load entirely. func (o *ProjectOptions) toProjectName(ctx context.Context, dockerCli command.Cli) (string, error) { if o.ProjectName != "" { return o.ProjectName, nil diff --git a/cmd/compose/project_resolution_test.go b/cmd/compose/project_resolution_test.go new file mode 100644 index 0000000000..77358ee4b7 --- /dev/null +++ b/cmd/compose/project_resolution_test.go @@ -0,0 +1,148 @@ +/* + 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 ( + "os" + "path/filepath" + "testing" + + "github.com/compose-spec/compose-go/v2/types" + "github.com/docker/cli/cli/streams" + "go.uber.org/mock/gomock" + "gotest.tools/v3/assert" + + "github.com/docker/compose/v5/pkg/mocks" +) + +func projectDir(t *testing.T, composeContent string) string { + t.Helper() + dir := t.TempDir() + if composeContent != "" { + assert.NilError(t, os.WriteFile(filepath.Join(dir, "compose.yaml"), []byte(composeContent), 0o600)) + } + return dir +} + +func resolutionCli(t *testing.T) *mocks.MockCli { + t.Helper() + ctrl := gomock.NewController(t) + cli := mocks.NewMockCli(ctrl) + cli.EXPECT().Out().Return(streams.NewOut(os.Stdout)).AnyTimes() + cli.EXPECT().Err().Return(streams.NewOut(os.Stderr)).AnyTimes() + return cli +} + +const validCompose = "services:\n web:\n image: alpine\n gated:\n image: alpine\n profiles: [debug]\n" + +// The resolution matrix of projectOrName: one name precedence +// (--project-name, then COMPOSE_PROJECT_NAME, then the model), a hard error +// for an explicit --file that cannot be read, and a label-based fallback +// only when COMPOSE_PROJECT_NAME provides a name. +func TestProjectOrNameResolution(t *testing.T) { + unsetEnv := func(t *testing.T) { + t.Setenv(ComposeProjectName, "") + assert.NilError(t, os.Unsetenv(ComposeProjectName)) + } + + t.Run("broken implicit file falls back to COMPOSE_PROJECT_NAME", func(t *testing.T) { + t.Setenv(ComposeProjectName, "fallback") + dir := projectDir(t, "services: {invalid") + opts := ProjectOptions{ProjectDir: dir} + + project, name, err := opts.projectOrName(t.Context(), resolutionCli(t)) + assert.NilError(t, err) + assert.Equal(t, name, "fallback") + assert.Assert(t, project == nil) + }) + + t.Run("broken explicit --file is a hard error even with COMPOSE_PROJECT_NAME", func(t *testing.T) { + t.Setenv(ComposeProjectName, "fallback") + dir := projectDir(t, "services: {invalid") + opts := ProjectOptions{ConfigPaths: []string{filepath.Join(dir, "compose.yaml")}, ProjectDir: dir} + + _, _, err := opts.projectOrName(t.Context(), resolutionCli(t)) + assert.ErrorContains(t, err, "yaml") + }) + + t.Run("no file at all with COMPOSE_PROJECT_NAME is the file-less workflow", func(t *testing.T) { + t.Setenv(ComposeProjectName, "labels-only") + dir := projectDir(t, "") + opts := ProjectOptions{ProjectDir: dir} + + project, name, err := opts.projectOrName(t.Context(), resolutionCli(t)) + assert.NilError(t, err) + assert.Equal(t, name, "labels-only") + assert.Assert(t, project == nil) + }) + + t.Run("no file and no name errors", func(t *testing.T) { + unsetEnv(t) + dir := projectDir(t, "") + opts := ProjectOptions{ProjectDir: dir} + + _, _, err := opts.projectOrName(t.Context(), resolutionCli(t)) + assert.Assert(t, err != nil) + }) + + t.Run("COMPOSE_PROJECT_NAME overrides the loaded model's name", func(t *testing.T) { + t.Setenv(ComposeProjectName, "from-env") + dir := projectDir(t, validCompose) + opts := ProjectOptions{ProjectDir: dir} + + project, name, err := opts.projectOrName(t.Context(), resolutionCli(t)) + assert.NilError(t, err) + assert.Equal(t, name, "from-env") + assert.Assert(t, project != nil) + }) + + t.Run("--project-name without --file skips loading, even a broken file", func(t *testing.T) { + unsetEnv(t) + dir := projectDir(t, "services: {invalid") + opts := ProjectOptions{ProjectName: "explicit", ProjectDir: dir} + + project, name, err := opts.projectOrName(t.Context(), resolutionCli(t)) + assert.NilError(t, err) + assert.Equal(t, name, "explicit") + assert.Assert(t, project == nil) + }) + + t.Run("unknown requested service is rejected by the load itself", func(t *testing.T) { + unsetEnv(t) + dir := projectDir(t, validCompose) + opts := ProjectOptions{ProjectDir: dir} + + _, _, err := opts.projectOrName(t.Context(), resolutionCli(t), "typo") + assert.ErrorContains(t, err, "no such service") + }) +} + +// validateServiceNames backs the commands that don't pass their service +// arguments through the load-time selection (restart, wait): strict when a +// model is available, no-op in label-based mode. +func TestValidateServiceNames(t *testing.T) { + project := &types.Project{ + Services: types.Services{"web": {Name: "web"}}, + DisabledServices: types.Services{"gated": {Name: "gated"}}, + } + + assert.NilError(t, validateServiceNames(nil, []string{"anything"})) + assert.NilError(t, validateServiceNames(project, []string{"web"})) + // profile-disabled services are legitimate targets: restart enables them + assert.NilError(t, validateServiceNames(project, []string{"gated"})) + assert.Error(t, validateServiceNames(project, []string{"typo"}), "no such service: typo") +} diff --git a/cmd/compose/ps.go b/cmd/compose/ps.go index 805533ae8a..482ae4b97c 100644 --- a/cmd/compose/ps.go +++ b/cmd/compose/ps.go @@ -99,14 +99,10 @@ func runPs(ctx context.Context, dockerCli command.Cli, backendOptions *BackendOp } if project != nil { + // unknown requested services were already rejected while loading the + // project (service selection in ToProject) names := project.ServiceNames() - if len(services) > 0 { - for _, service := range services { - if !slices.Contains(names, service) { - return fmt.Errorf("no such service: %s", service) - } - } - } else if !opts.Orphans { + if len(services) == 0 && !opts.Orphans { // until user asks to list orphaned services, we only include those declared in project services = names } diff --git a/cmd/compose/restart.go b/cmd/compose/restart.go index a9d97c5026..c8ce7a2e58 100644 --- a/cmd/compose/restart.go +++ b/cmd/compose/restart.go @@ -59,6 +59,9 @@ func runRestart(ctx context.Context, dockerCli command.Cli, backendOptions *Back if err != nil { return err } + if err := validateServiceNames(project, services); err != nil { + return err + } if project != nil && len(services) > 0 { project, err = project.WithServicesEnabled(services...) diff --git a/cmd/compose/volumes.go b/cmd/compose/volumes.go index e0da4f82e3..cbf6865a1d 100644 --- a/cmd/compose/volumes.go +++ b/cmd/compose/volumes.go @@ -19,7 +19,6 @@ package compose import ( "context" "fmt" - "slices" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/command/formatter" @@ -57,20 +56,14 @@ func volumesCommand(p *ProjectOptions, dockerCli command.Cli, backendOptions *Ba } func runVol(ctx context.Context, dockerCli command.Cli, backendOptions *BackendOptions, services []string, options volumesOptions) error { - project, name, err := options.projectOrName(ctx, dockerCli, services...) + // unknown requested services are rejected while loading the project + // (service selection in ToProject); label-based mode has no model to + // validate against + _, name, err := options.projectOrName(ctx, dockerCli, services...) if err != nil { return err } - if project != nil { - names := project.ServiceNames() - for _, service := range services { - if !slices.Contains(names, service) { - return fmt.Errorf("no such service: %s", service) - } - } - } - backend, err := compose.NewComposeService(dockerCli, backendOptions.Options...) if err != nil { return err diff --git a/cmd/compose/wait.go b/cmd/compose/wait.go index 9d86fd314c..ef270465b8 100644 --- a/cmd/compose/wait.go +++ b/cmd/compose/wait.go @@ -63,10 +63,13 @@ func waitCommand(p *ProjectOptions, dockerCli command.Cli, backendOptions *Backe } func runWait(ctx context.Context, dockerCli command.Cli, backendOptions *BackendOptions, opts *waitOptions) (int64, error) { - _, name, err := opts.projectOrName(ctx, dockerCli) + project, name, err := opts.projectOrName(ctx, dockerCli) if err != nil { return 0, err } + if err := validateServiceNames(project, opts.services); err != nil { + return 0, err + } backend, err := compose.NewComposeService(dockerCli, backendOptions.Options...) if err != nil {