cli: one project-resolution story — single name precedence, no swallowed load error, strict service validation

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,24 @@ 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:
+//   - a service argument naming a declared job is reported clearly instead,
+//     regardless of the cases below: the file loaded fine, only the
+//     target's selection failed, and every caller here treats a job as
+//     run-only;
+//   - 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
@@ -260,12 +279,20 @@ func (o *ProjectOptions) projectOrName(ctx context.Context, dockerCli command.Cl
 			// either the raw "no such service" below or, worse, silently
 			// falling back to the label-driven project next -- a job that
 			// was never run left no container behind for that fallback to
-			// find, so it would otherwise look like a successful no-op.
+			// find, so it would otherwise look like a successful no-op. This
+			// takes priority over the explicit-file hard error just below:
+			// the file loaded fine, only the target's selection failed.
 			if jobErr, replaced := jobTargetErr(ctx, dockerCli, o, services, err); replaced {
 				return nil, "", jobErr
 			}
+			if len(o.ConfigPaths) > 0 {
+				return nil, "", err
+			}
 			envProjectName := os.Getenv(ComposeProjectName)
 			if envProjectName != "" {
+				if !errdefs.IsNotFoundError(err) && !errors.Is(err, os.ErrNotExist) {
+					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
@@ -276,6 +303,39 @@ 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.
+//
+// Callers pass the unselected project (they call projectOrName with no
+// services, precisely so this function can validate the full name instead —
+// see runRestart/runWait), so project.AllJobs() already reflects every
+// declared job without a second reload.
+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
+		}
+		if _, ok := project.AllJobs()[service]; ok {
+			return fmt.Errorf("job %q can only be triggered with \"docker compose run\"", service)
+		}
+		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

cmd/compose/project_resolution_test.go

@@ -0,0 +1,163 @@
+/*
+   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"
+	"strings"
+	"testing"
+
+	"github.com/compose-spec/compose-go/v2/types"
+	"github.com/docker/cli/cli/streams"
+	logrustest "github.com/sirupsen/logrus/hooks/test"
+	"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}
+		hook := logrustest.NewGlobal()
+
+		project, name, err := opts.projectOrName(t.Context(), resolutionCli(t))
+		assert.NilError(t, err)
+		assert.Equal(t, name, "fallback")
+		assert.Assert(t, project == nil)
+		assert.Equal(t, len(hook.AllEntries()), 1)
+		assert.Assert(t, strings.Contains(hook.LastEntry().Message, "falling back to label-based mode"), hook.LastEntry().Message)
+	})
+
+	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}
+		hook := logrustest.NewGlobal()
+
+		project, name, err := opts.projectOrName(t.Context(), resolutionCli(t))
+		assert.NilError(t, err)
+		assert.Equal(t, name, "labels-only")
+		assert.Assert(t, project == nil)
+		// no file exists at all, so the "found but could not be loaded"
+		// warning would be factually wrong here — this is the normal
+		// file-less workflow, not a fallback from a broken file.
+		assert.Equal(t, len(hook.AllEntries()), 0, hook.AllEntries())
+	})
+
+	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"}},
+		Jobs:             types.Jobs{"migrate": {Name: "migrate"}},
+	}
+
+	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")
+	// restart/wait pass no services to projectOrName, so a job target never
+	// reaches jobTargetErr there -- validateServiceNames must recognize it
+	// itself instead of falling through to the generic "no such service".
+	assert.Error(t, validateServiceNames(project, []string{"migrate"}), `job "migrate" can only be triggered with "docker compose run"`)
+}

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
 		}

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...)

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

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 {

pkg/e2e/jobs_test.go

@@ -141,3 +141,21 @@ func TestDownRefusesJobWithProjectNameEnv(t *testing.T) {
 		StderrContains(`job "migrate" can only be triggered with "docker compose run"`),
 		ServiceNotCreated("migrate"))
 }
+
+// restart and wait don't pass their service arguments to projectOrName (see
+// runRestart/runWait), so jobTargetErr never sees them there -- the refusal
+// for these two instead comes from validateServiceNames itself recognizing
+// project.AllJobs(). A separate code path, so it needs its own coverage.
+func TestRestartRefusesJob(t *testing.T) {
+	NewScenario(t, "restart must refuse a job by name, naming run as the right command").
+		Step("restart fails naming the job",
+			ComposeCmd("restart", "migrate").MayFail(),
+			StderrContains(`job "migrate" can only be triggered with "docker compose run"`))
+}
+
+func TestWaitRefusesJob(t *testing.T) {
+	NewScenario(t, "wait must refuse a job by name, naming run as the right command").
+		Step("wait fails naming the job",
+			ComposeCmd("wait", "migrate").MayFail(),
+			StderrContains(`job "migrate" can only be triggered with "docker compose run"`))
+}

pkg/e2e/testdata/TestRestartRefusesJob/compose.yaml

@@ -0,0 +1,10 @@
+services:
+  web:
+    image: alpine
+    command: sleep infinity
+jobs:
+  migrate:
+    image: alpine
+    command: sh -c 'echo "migration done"'
+    triggers:
+      manual: true

pkg/e2e/testdata/TestWaitRefusesJob/compose.yaml

@@ -0,0 +1,10 @@
+services:
+  web:
+    image: alpine
+    command: sleep infinity
+jobs:
+  migrate:
+    image: alpine
+    command: sh -c 'echo "migration done"'
+    triggers:
+      manual: true