Skip to content

Commit 9874c52

Browse files
committed
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 <glours@users.noreply.github.com>
1 parent 61808cf commit 9874c52

5 files changed

Lines changed: 296 additions & 39 deletions

File tree

cmd/compose/hooks.go

Lines changed: 84 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ import (
2121
"encoding/json"
2222
"io"
2323
"os"
24+
"time"
2425

26+
"github.com/compose-spec/compose-go/v2/cli"
2527
"github.com/docker/cli/cli-plugins/hooks"
2628
"github.com/docker/cli/cli-plugins/metadata"
2729
"github.com/spf13/cobra"
@@ -30,14 +32,12 @@ import (
3032
"github.com/docker/compose/v5/internal/desktop"
3133
)
3234

33-
const deepLink = "docker-desktop://dashboard/logs"
34-
35-
func composeLogsHint() string {
36-
return "Filter, search, and stream logs from all your Compose services\nin one place with Docker Desktop's Logs view. " + hintLink(deepLink)
35+
func composeLogsHint(appID string) string {
36+
return "Filter, search, and stream logs from all your Compose services\nin one place with Docker Desktop's Logs view. " + hintLink(desktop.BuildLogsURL(appID))
3737
}
3838

39-
func dockerLogsHint() string {
40-
return "View and search logs for all containers in one place\nwith Docker Desktop's Logs view. " + hintLink(deepLink)
39+
func dockerLogsHint(appID string) string {
40+
return "View and search logs for all containers in one place\nwith Docker Desktop's Logs view. " + hintLink(desktop.BuildLogsURL(appID))
4141
}
4242

4343
// hintLink returns a clickable OSC 8 terminal hyperlink when ANSI is allowed,
@@ -68,36 +68,89 @@ func shouldDisableAnsi() bool {
6868
return false
6969
}
7070

71-
// hookHint defines a hint that can be returned by the hooks handler.
72-
// When checkFlags is nil, the hint is always returned for the matching command.
73-
// When checkFlags is set, the hint is only returned if the check passes.
7471
type hookHint struct {
75-
template func() string
76-
checkFlags func(flags map[string]string) bool
72+
template func(appID string) string
73+
checkFlags func(flags map[string]string) bool
74+
resolveProject bool
7775
}
7876

79-
// hooksHints maps hook root commands to their hint definitions. All current
80-
// hints promote Docker Desktop's Logs view; emission is additionally gated on
81-
// the FeatureLogsTab flag in handleHook.
8277
var hooksHints = map[string]hookHint{
83-
// standalone "docker logs" (not a compose subcommand)
78+
// "docker logs": the CLI hook payload doesn't carry the positional
79+
// container id, so the link is emitted unfiltered.
8480
"logs": {template: dockerLogsHint},
85-
"compose logs": {template: composeLogsHint},
81+
"compose logs": {template: composeLogsHint, resolveProject: true},
8682
"compose up": {
87-
template: composeLogsHint,
83+
template: composeLogsHint,
84+
resolveProject: true,
8885
checkFlags: func(flags map[string]string) bool {
89-
// Only show the hint when running in detached mode
90-
_, hasDetach := flags["detach"]
91-
_, hasD := flags["d"]
92-
return hasDetach || hasD
86+
return hasFlag(flags, "detach", "d")
9387
},
9488
},
9589
}
9690

97-
// logsTabEnabled reports whether Docker Desktop is the active engine and the
98-
// LogsTab feature flag is enabled. Overridable for tests.
99-
var logsTabEnabled = func(ctx context.Context) bool {
100-
return desktop.IsFeatureActiveStandalone(ctx, desktop.FeatureLogsTab)
91+
// Test seams. Replace via t.Cleanup; not safe to mutate from t.Parallel().
92+
var (
93+
logsTabEnabled = func(ctx context.Context) bool {
94+
return desktop.IsFeatureActiveStandalone(ctx, desktop.FeatureLogsTab)
95+
}
96+
resolveAppID = defaultResolveAppID
97+
)
98+
99+
const projectNameResolveTimeout = 250 * time.Millisecond
100+
101+
// Root-command flags whose values change which project the loader would
102+
// resolve. The hook payload exposes flag names but not values, so when any
103+
// is set we skip the appId rather than emit a wrong filter. workdir is the
104+
// deprecated alias for --project-directory; env-file can set
105+
// COMPOSE_PROJECT_NAME via the .env file it points at.
106+
var projectScopingFlags = []string{
107+
"project-name", "p",
108+
"file", "f",
109+
"project-directory", "workdir",
110+
"env-file",
111+
}
112+
113+
func defaultResolveAppID(ctx context.Context, flags map[string]string) string {
114+
workDir, err := os.Getwd()
115+
if err != nil {
116+
return ""
117+
}
118+
return resolveAppIDIn(ctx, flags, workDir)
119+
}
120+
121+
// Split from defaultResolveAppID so tests can pass a t.TempDir() instead
122+
// of mutating process state via t.Chdir.
123+
func resolveAppIDIn(ctx context.Context, flags map[string]string, workDir string) string {
124+
if hasFlag(flags, projectScopingFlags...) {
125+
return ""
126+
}
127+
ctx, cancel := context.WithTimeout(ctx, projectNameResolveTimeout)
128+
defer cancel()
129+
130+
opts, err := cli.NewProjectOptions(nil,
131+
cli.WithWorkingDirectory(workDir),
132+
cli.WithOsEnv,
133+
cli.WithDotEnv,
134+
cli.WithConfigFileEnv,
135+
cli.WithDefaultConfigPath,
136+
)
137+
if err != nil {
138+
return ""
139+
}
140+
project, err := opts.LoadProject(ctx)
141+
if err != nil {
142+
return ""
143+
}
144+
return project.Name
145+
}
146+
147+
func hasFlag(flags map[string]string, names ...string) bool {
148+
for _, n := range names {
149+
if _, ok := flags[n]; ok {
150+
return true
151+
}
152+
}
153+
return false
101154
}
102155

103156
// 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 {
141194
return nil
142195
}
143196

197+
var appID string
198+
if hint.resolveProject {
199+
appID = resolveAppID(ctx, hookData.Flags)
200+
}
201+
144202
enc := json.NewEncoder(w)
145203
enc.SetEscapeHTML(false)
146204
return enc.Encode(hooks.Response{
147205
Type: hooks.NextSteps,
148-
Template: hint.template(),
206+
Template: hint.template(appID),
149207
})
150208
}

cmd/compose/hooks_test.go

Lines changed: 140 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,26 @@ import (
2121
"context"
2222
"encoding/json"
2323
"os"
24+
"path/filepath"
2425
"strings"
2526
"testing"
2627

2728
"github.com/docker/cli/cli-plugins/hooks"
2829
"gotest.tools/v3/assert"
2930

3031
"github.com/docker/compose/v5/cmd/formatter"
32+
"github.com/docker/compose/v5/internal/desktop"
3133
)
3234

33-
// TestMain stubs the Docker Desktop feature-flag check so handleHook tests
34-
// don't attempt a live engine call. Individual tests can still override
35-
// isFeatureEnabled with their own stub + t.Cleanup to restore.
35+
const testDeepLink = "docker-desktop://dashboard/logs"
36+
37+
// TestMain stubs the Docker Desktop feature-flag check and the project
38+
// loader so handleHook tests don't make a live engine call or read a
39+
// compose file from the test runner's working directory. Individual tests
40+
// override either stub with t.Cleanup to restore.
3641
func TestMain(m *testing.M) {
3742
logsTabEnabled = func(context.Context) bool { return true }
43+
resolveAppID = func(context.Context, map[string]string) string { return "" }
3844
os.Exit(m.Run())
3945
}
4046

@@ -65,7 +71,7 @@ func TestHandleHook_UnknownCommand(t *testing.T) {
6571
func TestHandleHook_LogsCommand(t *testing.T) {
6672
tests := []struct {
6773
rootCmd string
68-
wantHint func() string
74+
wantHint func(appID string) string
6975
}{
7076
{rootCmd: "compose logs", wantHint: composeLogsHint},
7177
{rootCmd: "logs", wantHint: dockerLogsHint},
@@ -81,7 +87,7 @@ func TestHandleHook_LogsCommand(t *testing.T) {
8187

8288
msg := unmarshalResponse(t, buf.Bytes())
8389
assert.Equal(t, msg.Type, hooks.NextSteps)
84-
assert.Equal(t, msg.Template, tt.wantHint())
90+
assert.Equal(t, msg.Template, tt.wantHint(""))
8591
})
8692
}
8793
}
@@ -125,7 +131,7 @@ func TestHandleHook_ComposeUpDetached(t *testing.T) {
125131

126132
if tt.wantHint {
127133
msg := unmarshalResponse(t, buf.Bytes())
128-
assert.Equal(t, msg.Template, composeLogsHint())
134+
assert.Equal(t, msg.Template, composeLogsHint(""))
129135
} else {
130136
assert.Equal(t, buf.String(), "")
131137
}
@@ -146,8 +152,8 @@ func TestHandleHook_HintContainsOSC8Link(t *testing.T) {
146152

147153
msg := unmarshalResponse(t, buf.Bytes())
148154
// Verify the template contains the OSC 8 hyperlink sequence
149-
wantLink := formatter.OSC8Link(deepLink, deepLink)
150-
assert.Assert(t, len(wantLink) > len(deepLink), "OSC8Link should wrap the URL with escape sequences")
155+
wantLink := formatter.OSC8Link(testDeepLink, testDeepLink)
156+
assert.Assert(t, len(wantLink) > len(testDeepLink), "OSC8Link should wrap the URL with escape sequences")
151157
assert.Assert(t, strings.Contains(msg.Template, wantLink), "hint should contain OSC 8 hyperlink")
152158
}
153159

@@ -162,10 +168,57 @@ func TestHandleHook_NoColorDisablesOsc8(t *testing.T) {
162168

163169
msg := unmarshalResponse(t, buf.Bytes())
164170
// With NO_COLOR set, the hint should contain the plain URL without escape sequences
165-
assert.Assert(t, strings.Contains(msg.Template, deepLink), "hint should contain the deep link URL")
171+
assert.Assert(t, strings.Contains(msg.Template, testDeepLink), "hint should contain the deep link URL")
166172
assert.Assert(t, !strings.Contains(msg.Template, "\033"), "hint should not contain ANSI escape sequences")
167173
}
168174

175+
func TestHandleHook_AppIDEncodedInURL(t *testing.T) {
176+
prev := resolveAppID
177+
t.Cleanup(func() { resolveAppID = prev })
178+
resolveAppID = func(context.Context, map[string]string) string { return "myapp" }
179+
180+
t.Setenv("NO_COLOR", "1") // emit a plain URL we can substring-match
181+
for _, rootCmd := range []string{"compose logs", "compose up"} {
182+
t.Run(rootCmd, func(t *testing.T) {
183+
data := marshalHookData(t, hooks.Request{
184+
RootCmd: rootCmd,
185+
Flags: map[string]string{"d": "true"},
186+
})
187+
var buf bytes.Buffer
188+
err := handleHook(t.Context(), []string{data}, &buf)
189+
assert.NilError(t, err)
190+
191+
msg := unmarshalResponse(t, buf.Bytes())
192+
assert.Assert(t, strings.Contains(msg.Template, desktop.BuildLogsURL("myapp")),
193+
"hint should include the project-scoped URL, got %q", msg.Template)
194+
})
195+
}
196+
}
197+
198+
func TestHandleHook_DockerLogsIgnoresAppID(t *testing.T) {
199+
// resolveAppID is not consulted for "logs" because that hint isn't
200+
// resolveProject; assert the URL stays paramless even if a stub
201+
// would otherwise return a value.
202+
prev := resolveAppID
203+
t.Cleanup(func() { resolveAppID = prev })
204+
resolveAppID = func(context.Context, map[string]string) string {
205+
t.Fatalf("resolveAppID should not be called for docker logs")
206+
return ""
207+
}
208+
209+
t.Setenv("NO_COLOR", "1")
210+
data := marshalHookData(t, hooks.Request{RootCmd: "logs"})
211+
var buf bytes.Buffer
212+
err := handleHook(t.Context(), []string{data}, &buf)
213+
assert.NilError(t, err)
214+
215+
msg := unmarshalResponse(t, buf.Bytes())
216+
assert.Assert(t, strings.Contains(msg.Template, testDeepLink),
217+
"docker logs hint should contain the bare deep link")
218+
assert.Assert(t, !strings.Contains(msg.Template, "?appId="),
219+
"docker logs hint must not encode an appId")
220+
}
221+
169222
func TestHandleHook_FeatureFlagDisabledSuppressesHint(t *testing.T) {
170223
prev := logsTabEnabled
171224
t.Cleanup(func() { logsTabEnabled = prev })
@@ -192,10 +245,87 @@ func TestHandleHook_ComposeAnsiNeverDisablesOsc8(t *testing.T) {
192245
assert.NilError(t, err)
193246

194247
msg := unmarshalResponse(t, buf.Bytes())
195-
assert.Assert(t, strings.Contains(msg.Template, deepLink), "hint should contain the deep link URL")
248+
assert.Assert(t, strings.Contains(msg.Template, testDeepLink), "hint should contain the deep link URL")
196249
assert.Assert(t, !strings.Contains(msg.Template, "\033"), "hint should not contain ANSI escape sequences")
197250
}
198251

252+
func TestResolveAppID_ShortCircuitsOnFlag(t *testing.T) {
253+
tests := []struct {
254+
name string
255+
flags map[string]string
256+
}{
257+
{name: "long --project-name", flags: map[string]string{"project-name": ""}},
258+
{name: "short -p", flags: map[string]string{"p": ""}},
259+
{name: "long --file", flags: map[string]string{"file": ""}},
260+
{name: "short -f", flags: map[string]string{"f": ""}},
261+
{name: "long --project-directory", flags: map[string]string{"project-directory": ""}},
262+
{name: "deprecated --workdir alias", flags: map[string]string{"workdir": ""}},
263+
{name: "long --env-file", flags: map[string]string{"env-file": ""}},
264+
}
265+
for _, tt := range tests {
266+
t.Run(tt.name, func(t *testing.T) {
267+
// Use a real tmpdir as workDir so the short-circuit path is
268+
// exercised independently of the loader's file discovery.
269+
got := resolveAppIDIn(t.Context(), tt.flags, t.TempDir())
270+
assert.Equal(t, got, "")
271+
})
272+
}
273+
}
274+
275+
func TestResolveAppID_NameFromComposeFile(t *testing.T) {
276+
dir := t.TempDir()
277+
mustWrite(t, dir, "compose.yaml", "name: from-yaml\nservices:\n svc:\n image: nginx\n")
278+
unsetEnv(t, "COMPOSE_PROJECT_NAME")
279+
unsetEnv(t, "COMPOSE_FILE")
280+
281+
got := resolveAppIDIn(t.Context(), nil, dir)
282+
assert.Equal(t, got, "from-yaml")
283+
}
284+
285+
func TestResolveAppID_EnvVarOverridesYAML(t *testing.T) {
286+
dir := t.TempDir()
287+
mustWrite(t, dir, "compose.yaml", "name: from-yaml\nservices:\n svc:\n image: nginx\n")
288+
t.Setenv("COMPOSE_PROJECT_NAME", "from-env")
289+
unsetEnv(t, "COMPOSE_FILE")
290+
291+
got := resolveAppIDIn(t.Context(), nil, dir)
292+
assert.Equal(t, got, "from-env")
293+
}
294+
295+
func TestResolveAppID_NoComposeFileReturnsEmpty(t *testing.T) {
296+
unsetEnv(t, "COMPOSE_PROJECT_NAME")
297+
unsetEnv(t, "COMPOSE_FILE")
298+
299+
got := resolveAppIDIn(t.Context(), nil, t.TempDir())
300+
assert.Equal(t, got, "")
301+
}
302+
303+
// unsetEnv removes an env var for the lifetime of the test, restoring its
304+
// prior state on cleanup. t.Setenv("", "") is not equivalent to unset:
305+
// compose-go's WithConfigFileEnv treats empty as a meaningful override.
306+
func unsetEnv(t *testing.T, key string) {
307+
t.Helper()
308+
prev, had := os.LookupEnv(key)
309+
if err := os.Unsetenv(key); err != nil {
310+
t.Fatalf("unsetenv %s: %v", key, err)
311+
}
312+
t.Cleanup(func() {
313+
if !had {
314+
return
315+
}
316+
if err := os.Setenv(key, prev); err != nil {
317+
t.Errorf("restore env %s: %v", key, err)
318+
}
319+
})
320+
}
321+
322+
func mustWrite(t *testing.T, dir, name, content string) {
323+
t.Helper()
324+
if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil {
325+
t.Fatalf("write %s: %v", name, err)
326+
}
327+
}
328+
199329
func marshalHookData(t *testing.T, data hooks.Request) string {
200330
t.Helper()
201331
b, err := json.Marshal(data)

0 commit comments

Comments
 (0)