Skip to content

Commit e547679

Browse files
committed
refactor: drop Desktop beta-settings check; gate hint on LogsTab flag
Docker Desktop is removing the "Enable Logs view" beta setting, so drop the /app/settings check and rely on /features alone. With the setting gate gone, the compose hook subprocess would print the Logs view hint regardless of LogsTab; add a flag check in handleHook. Consolidate engine-label discovery and feature-flag evaluation into internal/desktop. Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
1 parent baaaaa3 commit e547679

5 files changed

Lines changed: 104 additions & 164 deletions

File tree

cmd/compose/hooks.go

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package compose
1818

1919
import (
20+
"context"
2021
"encoding/json"
2122
"io"
2223
"os"
@@ -26,6 +27,7 @@ import (
2627
"github.com/spf13/cobra"
2728

2829
"github.com/docker/compose/v5/cmd/formatter"
30+
"github.com/docker/compose/v5/internal/desktop"
2931
)
3032

3133
const deepLink = "docker-desktop://dashboard/logs"
@@ -74,7 +76,9 @@ type hookHint struct {
7476
checkFlags func(flags map[string]string) bool
7577
}
7678

77-
// hooksHints maps hook root commands to their hint definitions.
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.
7882
var hooksHints = map[string]hookHint{
7983
// standalone "docker logs" (not a compose subcommand)
8084
"logs": {template: dockerLogsHint},
@@ -90,11 +94,17 @@ var hooksHints = map[string]hookHint{
9094
},
9195
}
9296

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)
101+
}
102+
93103
// HooksCommand returns the hidden subcommand that the Docker CLI invokes
94104
// after command execution when the compose plugin has hooks configured.
95105
// Docker Desktop is responsible for registering which commands trigger hooks
96-
// and for gating on feature flags/settings — the hook handler simply
97-
// responds with the appropriate hint message.
106+
// in the docker CLI config; the handler gates all hints on the LogsTab
107+
// feature flag before emitting them.
98108
func HooksCommand() *cobra.Command {
99109
return &cobra.Command{
100110
Use: metadata.HookSubcommandName,
@@ -103,12 +113,12 @@ func HooksCommand() *cobra.Command {
103113
// (plugin initialization) from running for hook invocations.
104114
PersistentPreRunE: func(*cobra.Command, []string) error { return nil },
105115
RunE: func(cmd *cobra.Command, args []string) error {
106-
return handleHook(args, cmd.OutOrStdout())
116+
return handleHook(cmd.Context(), args, cmd.OutOrStdout())
107117
},
108118
}
109119
}
110120

111-
func handleHook(args []string, w io.Writer) error {
121+
func handleHook(ctx context.Context, args []string, w io.Writer) error {
112122
if len(args) == 0 {
113123
return nil
114124
}
@@ -127,6 +137,10 @@ func handleHook(args []string, w io.Writer) error {
127137
return nil
128138
}
129139

140+
if !logsTabEnabled(ctx) {
141+
return nil
142+
}
143+
130144
enc := json.NewEncoder(w)
131145
enc.SetEscapeHTML(false)
132146
return enc.Encode(hooks.Response{

cmd/compose/hooks_test.go

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ package compose
1818

1919
import (
2020
"bytes"
21+
"context"
2122
"encoding/json"
23+
"os"
2224
"strings"
2325
"testing"
2426

@@ -28,16 +30,24 @@ import (
2830
"github.com/docker/compose/v5/cmd/formatter"
2931
)
3032

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.
36+
func TestMain(m *testing.M) {
37+
logsTabEnabled = func(context.Context) bool { return true }
38+
os.Exit(m.Run())
39+
}
40+
3141
func TestHandleHook_NoArgs(t *testing.T) {
3242
var buf bytes.Buffer
33-
err := handleHook(nil, &buf)
43+
err := handleHook(t.Context(), nil, &buf)
3444
assert.NilError(t, err)
3545
assert.Equal(t, buf.String(), "")
3646
}
3747

3848
func TestHandleHook_InvalidJSON(t *testing.T) {
3949
var buf bytes.Buffer
40-
err := handleHook([]string{"not json"}, &buf)
50+
err := handleHook(t.Context(), []string{"not json"}, &buf)
4151
assert.NilError(t, err)
4252
assert.Equal(t, buf.String(), "")
4353
}
@@ -47,7 +57,7 @@ func TestHandleHook_UnknownCommand(t *testing.T) {
4757
RootCmd: "compose push",
4858
})
4959
var buf bytes.Buffer
50-
err := handleHook([]string{data}, &buf)
60+
err := handleHook(t.Context(), []string{data}, &buf)
5161
assert.NilError(t, err)
5262
assert.Equal(t, buf.String(), "")
5363
}
@@ -66,7 +76,7 @@ func TestHandleHook_LogsCommand(t *testing.T) {
6676
RootCmd: tt.rootCmd,
6777
})
6878
var buf bytes.Buffer
69-
err := handleHook([]string{data}, &buf)
79+
err := handleHook(t.Context(), []string{data}, &buf)
7080
assert.NilError(t, err)
7181

7282
msg := unmarshalResponse(t, buf.Bytes())
@@ -110,7 +120,7 @@ func TestHandleHook_ComposeUpDetached(t *testing.T) {
110120
Flags: tt.flags,
111121
})
112122
var buf bytes.Buffer
113-
err := handleHook([]string{data}, &buf)
123+
err := handleHook(t.Context(), []string{data}, &buf)
114124
assert.NilError(t, err)
115125

116126
if tt.wantHint {
@@ -131,7 +141,7 @@ func TestHandleHook_HintContainsOSC8Link(t *testing.T) {
131141
RootCmd: "compose logs",
132142
})
133143
var buf bytes.Buffer
134-
err := handleHook([]string{data}, &buf)
144+
err := handleHook(t.Context(), []string{data}, &buf)
135145
assert.NilError(t, err)
136146

137147
msg := unmarshalResponse(t, buf.Bytes())
@@ -147,7 +157,7 @@ func TestHandleHook_NoColorDisablesOsc8(t *testing.T) {
147157
RootCmd: "compose logs",
148158
})
149159
var buf bytes.Buffer
150-
err := handleHook([]string{data}, &buf)
160+
err := handleHook(t.Context(), []string{data}, &buf)
151161
assert.NilError(t, err)
152162

153163
msg := unmarshalResponse(t, buf.Bytes())
@@ -156,13 +166,29 @@ func TestHandleHook_NoColorDisablesOsc8(t *testing.T) {
156166
assert.Assert(t, !strings.Contains(msg.Template, "\033"), "hint should not contain ANSI escape sequences")
157167
}
158168

169+
func TestHandleHook_FeatureFlagDisabledSuppressesHint(t *testing.T) {
170+
prev := logsTabEnabled
171+
t.Cleanup(func() { logsTabEnabled = prev })
172+
logsTabEnabled = func(context.Context) bool { return false }
173+
174+
for _, rootCmd := range []string{"compose logs", "logs"} {
175+
t.Run(rootCmd, func(t *testing.T) {
176+
data := marshalHookData(t, hooks.Request{RootCmd: rootCmd})
177+
var buf bytes.Buffer
178+
err := handleHook(t.Context(), []string{data}, &buf)
179+
assert.NilError(t, err)
180+
assert.Equal(t, buf.String(), "")
181+
})
182+
}
183+
}
184+
159185
func TestHandleHook_ComposeAnsiNeverDisablesOsc8(t *testing.T) {
160186
t.Setenv("COMPOSE_ANSI", "never")
161187
data := marshalHookData(t, hooks.Request{
162188
RootCmd: "compose logs",
163189
})
164190
var buf bytes.Buffer
165-
err := handleHook([]string{data}, &buf)
191+
err := handleHook(t.Context(), []string{data}, &buf)
166192
assert.NilError(t, err)
167193

168194
msg := unmarshalResponse(t, buf.Bytes())

internal/desktop/client.go

Lines changed: 47 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,11 @@ import (
2323
"io"
2424
"net"
2525
"net/http"
26-
"path/filepath"
2726
"strings"
2827

28+
"github.com/docker/cli/cli/command"
29+
cliflags "github.com/docker/cli/cli/flags"
30+
"github.com/moby/moby/client"
2931
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
3032

3133
"github.com/docker/compose/v5/internal"
@@ -137,102 +139,68 @@ func (c *Client) FeatureFlags(ctx context.Context) (FeatureFlagResponse, error)
137139
return ret, nil
138140
}
139141

140-
// SettingValue represents a Docker Desktop setting with a locked flag and a value.
141-
type SettingValue struct {
142-
Locked bool `json:"locked"`
143-
Value bool `json:"value"`
144-
}
145-
146-
// DesktopSettings represents the "desktop" section of Docker Desktop settings.
147-
type DesktopSettings struct {
148-
EnableLogsTab SettingValue `json:"enableLogsTab"`
149-
}
150-
151-
// SettingsResponse represents the Docker Desktop settings response.
152-
type SettingsResponse struct {
153-
Desktop DesktopSettings `json:"desktop"`
154-
}
155-
156-
// Settings fetches the Docker Desktop application settings.
157-
func (c *Client) Settings(ctx context.Context) (*SettingsResponse, error) {
158-
req, err := c.newRequest(ctx, http.MethodGet, "/app/settings", http.NoBody)
159-
if err != nil {
160-
return nil, err
161-
}
162-
163-
resp, err := c.client.Do(req)
142+
// IsFeatureEnabled checks the feature flag (GET /features) for a given
143+
// feature. Returns true when the feature is rolled out.
144+
func (c *Client) IsFeatureEnabled(ctx context.Context, feature string) (bool, error) {
145+
flags, err := c.FeatureFlags(ctx)
164146
if err != nil {
165-
return nil, err
166-
}
167-
defer func() {
168-
_ = resp.Body.Close()
169-
}()
170-
171-
if resp.StatusCode != http.StatusOK {
172-
return nil, fmt.Errorf("unexpected status code: %d", resp.StatusCode)
173-
}
174-
175-
var ret SettingsResponse
176-
if err := json.NewDecoder(resp.Body).Decode(&ret); err != nil {
177-
return nil, err
147+
return false, err
178148
}
179-
return &ret, nil
149+
return flags[feature].Enabled, nil
180150
}
181151

182-
// IsFeatureEnabled checks both the feature flag (GET /features) and the user
183-
// setting (GET /app/settings) for a given feature. Returns true only when the
184-
// feature is both rolled out and enabled by the user. Features without a
185-
// corresponding setting entry are considered enabled if the flag is set.
186-
func (c *Client) IsFeatureEnabled(ctx context.Context, feature string) (bool, error) {
187-
flags, err := c.FeatureFlags(ctx)
152+
// EndpointFromEngine returns the Docker Desktop API socket address advertised
153+
// by the engine in its Info labels, or "" when the active engine is not
154+
// Docker Desktop.
155+
func EndpointFromEngine(ctx context.Context, apiClient client.APIClient) (string, error) {
156+
info, err := apiClient.Info(ctx, client.InfoOptions{})
188157
if err != nil {
189-
return false, err
158+
return "", err
190159
}
191-
if !flags[feature].Enabled {
192-
return false, nil
160+
for _, l := range info.Info.Labels {
161+
k, v, ok := strings.Cut(l, "=")
162+
if ok && k == EngineLabel {
163+
return v, nil
164+
}
193165
}
166+
return "", nil
167+
}
194168

195-
check, hasCheck := featureSettingChecks[feature]
196-
if !hasCheck {
197-
// No setting to verify — feature flag alone is sufficient
198-
return true, nil
169+
// IsFeatureActive reports whether Docker Desktop is the active engine and the
170+
// given feature flag is enabled. Returns false silently on any failure — the
171+
// engine being unreachable, Desktop not being the active engine, or the flag
172+
// being off — so callers can use this as a single gating check.
173+
func IsFeatureActive(ctx context.Context, apiClient client.APIClient, feature string) bool {
174+
endpoint, err := EndpointFromEngine(ctx, apiClient)
175+
if err != nil || endpoint == "" {
176+
return false
199177
}
200178

201-
// The /app/settings endpoint is served by the backend socket, not the
202-
// docker-cli socket. Derive the backend socket path from the current
203-
// endpoint.
204-
backendEndpoint := backendSocketEndpoint(c.apiEndpoint)
205-
backendCli := NewClient(backendEndpoint)
206-
defer backendCli.Close() //nolint:errcheck
179+
c := NewClient(endpoint)
180+
defer c.Close() //nolint:errcheck
207181

208-
settings, err := backendCli.Settings(ctx)
182+
enabled, err := c.IsFeatureEnabled(ctx, feature)
209183
if err != nil {
210-
return false, err
184+
return false
211185
}
212-
return check(settings), nil
186+
return enabled
213187
}
214188

215-
// backendSocketEndpoint derives the Docker Desktop backend socket endpoint
216-
// from any socket endpoint in the same directory.
217-
//
218-
// On macOS/Linux: unix:///path/to/Data/docker-cli.sock → unix:///path/to/Data/backend.sock
219-
// On Windows: npipe://./pipe/dockerDesktopLinuxEngine → npipe://./pipe/dockerBackendApiServer
220-
func backendSocketEndpoint(endpoint string) string {
221-
if sockPath, ok := strings.CutPrefix(endpoint, "unix://"); ok {
222-
return "unix://" + filepath.Join(filepath.Dir(sockPath), "backend.sock")
189+
// IsFeatureActiveStandalone is the convenience form of IsFeatureActive for
190+
// callers without an existing engine API client (e.g. the compose plugin hook
191+
// subprocess). It builds a Docker CLI using the ambient environment to
192+
// resolve the active context, then delegates to IsFeatureActive.
193+
func IsFeatureActiveStandalone(ctx context.Context, feature string) bool {
194+
dockerCli, err := command.NewDockerCli(command.WithCombinedStreams(io.Discard))
195+
if err != nil {
196+
return false
223197
}
224-
if _, ok := strings.CutPrefix(endpoint, "npipe://"); ok {
225-
return "npipe://./pipe/dockerBackendApiServer"
198+
if err := dockerCli.Initialize(cliflags.NewClientOptions()); err != nil {
199+
return false
226200
}
227-
return endpoint
228-
}
201+
defer dockerCli.Client().Close() //nolint:errcheck
229202

230-
// featureSettingChecks maps feature flag names to their corresponding
231-
// Docker Desktop setting check functions.
232-
var featureSettingChecks = map[string]func(*SettingsResponse) bool{
233-
FeatureLogsTab: func(s *SettingsResponse) bool {
234-
return s.Desktop.EnableLogsTab.Value
235-
},
203+
return IsFeatureActive(ctx, dockerCli.Client(), feature)
236204
}
237205

238206
func (c *Client) newRequest(ctx context.Context, method, path string, body io.Reader) (*http.Request, error) {

internal/desktop/client_test.go

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -24,41 +24,6 @@ import (
2424
"gotest.tools/v3/assert"
2525
)
2626

27-
func TestBackendSocketEndpoint(t *testing.T) {
28-
tests := []struct {
29-
name string
30-
input string
31-
expected string
32-
}{
33-
{
34-
name: "macOS unix socket",
35-
input: "unix:///Users/me/Library/Containers/com.docker.docker/Data/docker-cli.sock",
36-
expected: "unix:///Users/me/Library/Containers/com.docker.docker/Data/backend.sock",
37-
},
38-
{
39-
name: "Linux unix socket",
40-
input: "unix:///run/desktop/docker-cli.sock",
41-
expected: "unix:///run/desktop/backend.sock",
42-
},
43-
{
44-
name: "Windows named pipe",
45-
input: "npipe://./pipe/dockerDesktopLinuxEngine",
46-
expected: "npipe://./pipe/dockerBackendApiServer",
47-
},
48-
{
49-
name: "unknown scheme passthrough",
50-
input: "tcp://localhost:2375",
51-
expected: "tcp://localhost:2375",
52-
},
53-
}
54-
for _, tt := range tests {
55-
t.Run(tt.name, func(t *testing.T) {
56-
result := backendSocketEndpoint(tt.input)
57-
assert.Equal(t, result, tt.expected)
58-
})
59-
}
60-
}
61-
6227
func TestClientPing(t *testing.T) {
6328
if testing.Short() {
6429
t.Skip("Skipped in short mode - test connects to Docker Desktop")

0 commit comments

Comments
 (0)