From c59e13cde50dfcbb2f62d173b96cb8f51fc1e6eb Mon Sep 17 00:00:00 2001 From: Guillaume Lours Date: Wed, 6 May 2026 17:08:03 +0200 Subject: [PATCH] feat: pre-filter Desktop Logs view by Compose project Pass the active project name as the appId query parameter on the docker-desktop://dashboard/logs deep link, both from the post-command hint (compose up -d, compose logs) and the interactive nav menu ('l' key during compose up). The hook subprocess re-runs compose-go's project loader so the name matches what the parent computed; it skips the appId when -p, -f, --project-directory, --workdir, or --env-file is set, since the hook payload does not carry their values. docker logs stays unfiltered: the CLI hook contract does not expose the positional container id. Signed-off-by: Guillaume Lours --- cmd/compose/hooks.go | 110 +++++++++++++++++------ cmd/compose/hooks_test.go | 150 +++++++++++++++++++++++++++++--- cmd/formatter/shortcut.go | 7 +- internal/desktop/client.go | 26 ++++++ internal/desktop/client_test.go | 42 +++++++++ 5 files changed, 296 insertions(+), 39 deletions(-) diff --git a/cmd/compose/hooks.go b/cmd/compose/hooks.go index 77d42e017..1697c8686 100644 --- a/cmd/compose/hooks.go +++ b/cmd/compose/hooks.go @@ -21,7 +21,9 @@ import ( "encoding/json" "io" "os" + "time" + "github.com/compose-spec/compose-go/v2/cli" "github.com/docker/cli/cli-plugins/hooks" "github.com/docker/cli/cli-plugins/metadata" "github.com/spf13/cobra" @@ -30,14 +32,12 @@ import ( "github.com/docker/compose/v5/internal/desktop" ) -const deepLink = "docker-desktop://dashboard/logs" - -func composeLogsHint() string { - return "Filter, search, and stream logs from all your Compose services\nin one place with Docker Desktop's Logs view. " + hintLink(deepLink) +func composeLogsHint(appID string) string { + return "Filter, search, and stream logs from all your Compose services\nin one place with Docker Desktop's Logs view. " + hintLink(desktop.BuildLogsURL(appID)) } -func dockerLogsHint() string { - return "View and search logs for all containers in one place\nwith Docker Desktop's Logs view. " + hintLink(deepLink) +func dockerLogsHint(appID string) string { + return "View and search logs for all containers in one place\nwith Docker Desktop's Logs view. " + hintLink(desktop.BuildLogsURL(appID)) } // hintLink returns a clickable OSC 8 terminal hyperlink when ANSI is allowed, @@ -68,36 +68,89 @@ func shouldDisableAnsi() bool { return false } -// hookHint defines a hint that can be returned by the hooks handler. -// When checkFlags is nil, the hint is always returned for the matching command. -// When checkFlags is set, the hint is only returned if the check passes. type hookHint struct { - template func() string - checkFlags func(flags map[string]string) bool + template func(appID string) string + checkFlags func(flags map[string]string) bool + resolveProject bool } -// hooksHints maps hook root commands to their hint definitions. All current -// hints promote Docker Desktop's Logs view; emission is additionally gated on -// the FeatureLogsTab flag in handleHook. var hooksHints = map[string]hookHint{ - // standalone "docker logs" (not a compose subcommand) + // "docker logs": the CLI hook payload doesn't carry the positional + // container id, so the link is emitted unfiltered. "logs": {template: dockerLogsHint}, - "compose logs": {template: composeLogsHint}, + "compose logs": {template: composeLogsHint, resolveProject: true}, "compose up": { - template: composeLogsHint, + template: composeLogsHint, + resolveProject: true, checkFlags: func(flags map[string]string) bool { - // Only show the hint when running in detached mode - _, hasDetach := flags["detach"] - _, hasD := flags["d"] - return hasDetach || hasD + return hasFlag(flags, "detach", "d") }, }, } -// logsTabEnabled reports whether Docker Desktop is the active engine and the -// LogsTab feature flag is enabled. Overridable for tests. -var logsTabEnabled = func(ctx context.Context) bool { - return desktop.IsFeatureActiveStandalone(ctx, desktop.FeatureLogsTab) +// Test seams. Replace via t.Cleanup; not safe to mutate from t.Parallel(). +var ( + logsTabEnabled = func(ctx context.Context) bool { + return desktop.IsFeatureActiveStandalone(ctx, desktop.FeatureLogsTab) + } + resolveAppID = defaultResolveAppID +) + +const projectNameResolveTimeout = 250 * time.Millisecond + +// Root-command flags whose values change which project the loader would +// resolve. The hook payload exposes flag names but not values, so when any +// is set we skip the appId rather than emit a wrong filter. workdir is the +// deprecated alias for --project-directory; env-file can set +// COMPOSE_PROJECT_NAME via the .env file it points at. +var projectScopingFlags = []string{ + "project-name", "p", + "file", "f", + "project-directory", "workdir", + "env-file", +} + +func defaultResolveAppID(ctx context.Context, flags map[string]string) string { + workDir, err := os.Getwd() + if err != nil { + return "" + } + return resolveAppIDIn(ctx, flags, workDir) +} + +// Split from defaultResolveAppID so tests can pass a t.TempDir() instead +// of mutating process state via t.Chdir. +func resolveAppIDIn(ctx context.Context, flags map[string]string, workDir string) string { + if hasFlag(flags, projectScopingFlags...) { + return "" + } + ctx, cancel := context.WithTimeout(ctx, projectNameResolveTimeout) + defer cancel() + + opts, err := cli.NewProjectOptions(nil, + cli.WithWorkingDirectory(workDir), + cli.WithOsEnv, + cli.WithDotEnv, + cli.WithConfigFileEnv, + cli.WithDefaultConfigPath, + ) + if err != nil { + return "" + } + project, err := opts.LoadProject(ctx) + if err != nil { + return "" + } + return project.Name +} + +func hasFlag(flags map[string]string, names ...string) bool { + for _, n := range names { + if _, ok := flags[n]; ok { + return true + } + } + return false } // HooksCommand returns the hidden subcommand that the Docker CLI invokes @@ -141,10 +194,15 @@ func handleHook(ctx context.Context, args []string, w io.Writer) error { return nil } + var appID string + if hint.resolveProject { + appID = resolveAppID(ctx, hookData.Flags) + } + enc := json.NewEncoder(w) enc.SetEscapeHTML(false) return enc.Encode(hooks.Response{ Type: hooks.NextSteps, - Template: hint.template(), + Template: hint.template(appID), }) } diff --git a/cmd/compose/hooks_test.go b/cmd/compose/hooks_test.go index 2841d79a9..fc53c75ed 100644 --- a/cmd/compose/hooks_test.go +++ b/cmd/compose/hooks_test.go @@ -21,6 +21,7 @@ import ( "context" "encoding/json" "os" + "path/filepath" "strings" "testing" @@ -28,13 +29,18 @@ import ( "gotest.tools/v3/assert" "github.com/docker/compose/v5/cmd/formatter" + "github.com/docker/compose/v5/internal/desktop" ) -// TestMain stubs the Docker Desktop feature-flag check so handleHook tests -// don't attempt a live engine call. Individual tests can still override -// isFeatureEnabled with their own stub + t.Cleanup to restore. +const testDeepLink = "docker-desktop://dashboard/logs" + +// TestMain stubs the Docker Desktop feature-flag check and the project +// loader so handleHook tests don't make a live engine call or read a +// compose file from the test runner's working directory. Individual tests +// override either stub with t.Cleanup to restore. func TestMain(m *testing.M) { logsTabEnabled = func(context.Context) bool { return true } + resolveAppID = func(context.Context, map[string]string) string { return "" } os.Exit(m.Run()) } @@ -65,7 +71,7 @@ func TestHandleHook_UnknownCommand(t *testing.T) { func TestHandleHook_LogsCommand(t *testing.T) { tests := []struct { rootCmd string - wantHint func() string + wantHint func(appID string) string }{ {rootCmd: "compose logs", wantHint: composeLogsHint}, {rootCmd: "logs", wantHint: dockerLogsHint}, @@ -81,7 +87,7 @@ func TestHandleHook_LogsCommand(t *testing.T) { msg := unmarshalResponse(t, buf.Bytes()) assert.Equal(t, msg.Type, hooks.NextSteps) - assert.Equal(t, msg.Template, tt.wantHint()) + assert.Equal(t, msg.Template, tt.wantHint("")) }) } } @@ -125,7 +131,7 @@ func TestHandleHook_ComposeUpDetached(t *testing.T) { if tt.wantHint { msg := unmarshalResponse(t, buf.Bytes()) - assert.Equal(t, msg.Template, composeLogsHint()) + assert.Equal(t, msg.Template, composeLogsHint("")) } else { assert.Equal(t, buf.String(), "") } @@ -146,8 +152,8 @@ func TestHandleHook_HintContainsOSC8Link(t *testing.T) { msg := unmarshalResponse(t, buf.Bytes()) // Verify the template contains the OSC 8 hyperlink sequence - wantLink := formatter.OSC8Link(deepLink, deepLink) - assert.Assert(t, len(wantLink) > len(deepLink), "OSC8Link should wrap the URL with escape sequences") + wantLink := formatter.OSC8Link(testDeepLink, testDeepLink) + assert.Assert(t, len(wantLink) > len(testDeepLink), "OSC8Link should wrap the URL with escape sequences") assert.Assert(t, strings.Contains(msg.Template, wantLink), "hint should contain OSC 8 hyperlink") } @@ -162,10 +168,57 @@ func TestHandleHook_NoColorDisablesOsc8(t *testing.T) { msg := unmarshalResponse(t, buf.Bytes()) // With NO_COLOR set, the hint should contain the plain URL without escape sequences - assert.Assert(t, strings.Contains(msg.Template, deepLink), "hint should contain the deep link URL") + assert.Assert(t, strings.Contains(msg.Template, testDeepLink), "hint should contain the deep link URL") assert.Assert(t, !strings.Contains(msg.Template, "\033"), "hint should not contain ANSI escape sequences") } +func TestHandleHook_AppIDEncodedInURL(t *testing.T) { + prev := resolveAppID + t.Cleanup(func() { resolveAppID = prev }) + resolveAppID = func(context.Context, map[string]string) string { return "myapp" } + + t.Setenv("NO_COLOR", "1") // emit a plain URL we can substring-match + for _, rootCmd := range []string{"compose logs", "compose up"} { + t.Run(rootCmd, func(t *testing.T) { + data := marshalHookData(t, hooks.Request{ + RootCmd: rootCmd, + Flags: map[string]string{"d": "true"}, + }) + var buf bytes.Buffer + err := handleHook(t.Context(), []string{data}, &buf) + assert.NilError(t, err) + + msg := unmarshalResponse(t, buf.Bytes()) + assert.Assert(t, strings.Contains(msg.Template, desktop.BuildLogsURL("myapp")), + "hint should include the project-scoped URL, got %q", msg.Template) + }) + } +} + +func TestHandleHook_DockerLogsIgnoresAppID(t *testing.T) { + // resolveAppID is not consulted for "logs" because that hint isn't + // resolveProject; assert the URL stays paramless even if a stub + // would otherwise return a value. + prev := resolveAppID + t.Cleanup(func() { resolveAppID = prev }) + resolveAppID = func(context.Context, map[string]string) string { + t.Fatalf("resolveAppID should not be called for docker logs") + return "" + } + + t.Setenv("NO_COLOR", "1") + data := marshalHookData(t, hooks.Request{RootCmd: "logs"}) + var buf bytes.Buffer + err := handleHook(t.Context(), []string{data}, &buf) + assert.NilError(t, err) + + msg := unmarshalResponse(t, buf.Bytes()) + assert.Assert(t, strings.Contains(msg.Template, testDeepLink), + "docker logs hint should contain the bare deep link") + assert.Assert(t, !strings.Contains(msg.Template, "?appId="), + "docker logs hint must not encode an appId") +} + func TestHandleHook_FeatureFlagDisabledSuppressesHint(t *testing.T) { prev := logsTabEnabled t.Cleanup(func() { logsTabEnabled = prev }) @@ -192,10 +245,87 @@ func TestHandleHook_ComposeAnsiNeverDisablesOsc8(t *testing.T) { assert.NilError(t, err) msg := unmarshalResponse(t, buf.Bytes()) - assert.Assert(t, strings.Contains(msg.Template, deepLink), "hint should contain the deep link URL") + assert.Assert(t, strings.Contains(msg.Template, testDeepLink), "hint should contain the deep link URL") assert.Assert(t, !strings.Contains(msg.Template, "\033"), "hint should not contain ANSI escape sequences") } +func TestResolveAppID_ShortCircuitsOnFlag(t *testing.T) { + tests := []struct { + name string + flags map[string]string + }{ + {name: "long --project-name", flags: map[string]string{"project-name": ""}}, + {name: "short -p", flags: map[string]string{"p": ""}}, + {name: "long --file", flags: map[string]string{"file": ""}}, + {name: "short -f", flags: map[string]string{"f": ""}}, + {name: "long --project-directory", flags: map[string]string{"project-directory": ""}}, + {name: "deprecated --workdir alias", flags: map[string]string{"workdir": ""}}, + {name: "long --env-file", flags: map[string]string{"env-file": ""}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Use a real tmpdir as workDir so the short-circuit path is + // exercised independently of the loader's file discovery. + got := resolveAppIDIn(t.Context(), tt.flags, t.TempDir()) + assert.Equal(t, got, "") + }) + } +} + +func TestResolveAppID_NameFromComposeFile(t *testing.T) { + dir := t.TempDir() + mustWrite(t, dir, "compose.yaml", "name: from-yaml\nservices:\n svc:\n image: nginx\n") + unsetEnv(t, "COMPOSE_PROJECT_NAME") + unsetEnv(t, "COMPOSE_FILE") + + got := resolveAppIDIn(t.Context(), nil, dir) + assert.Equal(t, got, "from-yaml") +} + +func TestResolveAppID_EnvVarOverridesYAML(t *testing.T) { + dir := t.TempDir() + mustWrite(t, dir, "compose.yaml", "name: from-yaml\nservices:\n svc:\n image: nginx\n") + t.Setenv("COMPOSE_PROJECT_NAME", "from-env") + unsetEnv(t, "COMPOSE_FILE") + + got := resolveAppIDIn(t.Context(), nil, dir) + assert.Equal(t, got, "from-env") +} + +func TestResolveAppID_NoComposeFileReturnsEmpty(t *testing.T) { + unsetEnv(t, "COMPOSE_PROJECT_NAME") + unsetEnv(t, "COMPOSE_FILE") + + got := resolveAppIDIn(t.Context(), nil, t.TempDir()) + assert.Equal(t, got, "") +} + +// unsetEnv removes an env var for the lifetime of the test, restoring its +// prior state on cleanup. t.Setenv("", "") is not equivalent to unset: +// compose-go's WithConfigFileEnv treats empty as a meaningful override. +func unsetEnv(t *testing.T, key string) { + t.Helper() + prev, had := os.LookupEnv(key) + if err := os.Unsetenv(key); err != nil { + t.Fatalf("unsetenv %s: %v", key, err) + } + t.Cleanup(func() { + if !had { + return + } + if err := os.Setenv(key, prev); err != nil { + t.Errorf("restore env %s: %v", key, err) + } + }) +} + +func mustWrite(t *testing.T, dir, name, content string) { + t.Helper() + if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil { + t.Fatalf("write %s: %v", name, err) + } +} + func marshalHookData(t *testing.T, data hooks.Request) string { t.Helper() b, err := json.Marshal(data) diff --git a/cmd/formatter/shortcut.go b/cmd/formatter/shortcut.go index bb07ff72d..ffa083e2f 100644 --- a/cmd/formatter/shortcut.go +++ b/cmd/formatter/shortcut.go @@ -31,6 +31,7 @@ import ( "github.com/eiannone/keyboard" "github.com/skratchdot/open-golang/open" + "github.com/docker/compose/v5/internal/desktop" "github.com/docker/compose/v5/internal/tracing" "github.com/docker/compose/v5/pkg/api" ) @@ -238,14 +239,14 @@ func (lk *LogKeyboard) openDDComposeUI(ctx context.Context, project *types.Proje }() } -func (lk *LogKeyboard) openDDLogsView(ctx context.Context) { +func (lk *LogKeyboard) openDDLogsView(ctx context.Context, project *types.Project) { if !lk.IsLogsViewEnabled { return } go func() { _ = tracing.EventWrapFuncForErrGroup(ctx, "menu/gui/logsview", tracing.SpanOptions{}, func(ctx context.Context) error { - link := "docker-desktop://dashboard/logs" + link := desktop.BuildLogsURL(project.Name) err := open.Run(link) if err != nil { err = fmt.Errorf("could not open Docker Desktop Logs view: %w", err) @@ -336,7 +337,7 @@ func (lk *LogKeyboard) HandleKeyEvents(ctx context.Context, event keyboard.KeyEv case 'o': lk.openDDComposeUI(ctx, project) case 'l': - lk.openDDLogsView(ctx) + lk.openDDLogsView(ctx, project) } switch key := event.Key; key { case keyboard.KeyCtrlC: diff --git a/internal/desktop/client.go b/internal/desktop/client.go index 69aee3382..136d0c534 100644 --- a/internal/desktop/client.go +++ b/internal/desktop/client.go @@ -23,6 +23,7 @@ import ( "io" "net" "net/http" + "net/url" "strings" "github.com/docker/cli/cli/command" @@ -42,6 +43,31 @@ const EngineLabel = "com.docker.desktop.address" // FeatureLogsTab is the feature flag name for the Docker Desktop Logs view. const FeatureLogsTab = "LogsTab" +const logsDeepLink = "docker-desktop://dashboard/logs" + +// LogsAppIDMaxLen mirrors the byte-length cap Docker Desktop's URL handler +// applies to the appId query parameter; values longer than this are +// truncated by the receiver, so we trim ahead of time to avoid emitting +// hyperlinks that will be silently shortened. The slice in BuildLogsURL is +// a byte slice — Compose project names are restricted to the ASCII set +// `[a-z0-9_-]` by loader.NormalizeProjectName, so a byte cap and a rune +// cap coincide for any value that could legitimately reach this builder. +const LogsAppIDMaxLen = 256 + +// BuildLogsURL returns the deep link that opens Docker Desktop's Logs view, +// optionally pre-filtered to a Compose project. An empty appID yields the +// unfiltered URL. +func BuildLogsURL(appID string) string { + if appID == "" { + return logsDeepLink + } + if len(appID) > LogsAppIDMaxLen { + appID = appID[:LogsAppIDMaxLen] + } + q := url.Values{"appId": []string{appID}} + return logsDeepLink + "?" + q.Encode() +} + // identify this client in the logs var userAgent = "compose/" + internal.Version diff --git a/internal/desktop/client_test.go b/internal/desktop/client_test.go index 7bb1546bc..7a5eeaf92 100644 --- a/internal/desktop/client_test.go +++ b/internal/desktop/client_test.go @@ -18,12 +18,54 @@ package desktop import ( "os" + "strings" "testing" "time" "gotest.tools/v3/assert" ) +func TestBuildLogsURL(t *testing.T) { + tests := []struct { + name string + appID string + want string + }{ + { + name: "empty app id yields paramless url", + appID: "", + want: "docker-desktop://dashboard/logs", + }, + { + name: "simple project name", + appID: "myapp", + want: "docker-desktop://dashboard/logs?appId=myapp", + }, + { + name: "name with hyphen and digits is preserved", + appID: "my-app-2", + want: "docker-desktop://dashboard/logs?appId=my-app-2", + }, + { + name: "characters that need percent-encoding are escaped", + appID: "weird name/with spaces", + want: "docker-desktop://dashboard/logs?appId=weird+name%2Fwith+spaces", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, BuildLogsURL(tt.appID), tt.want) + }) + } +} + +func TestBuildLogsURL_TruncatesLongAppID(t *testing.T) { + long := strings.Repeat("a", LogsAppIDMaxLen+50) + got := BuildLogsURL(long) + want := "docker-desktop://dashboard/logs?appId=" + strings.Repeat("a", LogsAppIDMaxLen) + assert.Equal(t, got, want) +} + func TestClientPing(t *testing.T) { if testing.Short() { t.Skip("Skipped in short mode - test connects to Docker Desktop")