diff --git a/cmd/compose/hooks.go b/cmd/compose/hooks.go index 02521f19b0..77d42e0175 100644 --- a/cmd/compose/hooks.go +++ b/cmd/compose/hooks.go @@ -17,6 +17,7 @@ package compose import ( + "context" "encoding/json" "io" "os" @@ -26,6 +27,7 @@ import ( "github.com/spf13/cobra" "github.com/docker/compose/v5/cmd/formatter" + "github.com/docker/compose/v5/internal/desktop" ) const deepLink = "docker-desktop://dashboard/logs" @@ -74,7 +76,9 @@ type hookHint struct { checkFlags func(flags map[string]string) bool } -// hooksHints maps hook root commands to their hint definitions. +// 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) "logs": {template: dockerLogsHint}, @@ -90,11 +94,17 @@ var hooksHints = map[string]hookHint{ }, } +// 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) +} + // HooksCommand returns the hidden subcommand that the Docker CLI invokes // after command execution when the compose plugin has hooks configured. // Docker Desktop is responsible for registering which commands trigger hooks -// and for gating on feature flags/settings — the hook handler simply -// responds with the appropriate hint message. +// in the docker CLI config; the handler gates all hints on the LogsTab +// feature flag before emitting them. func HooksCommand() *cobra.Command { return &cobra.Command{ Use: metadata.HookSubcommandName, @@ -103,12 +113,12 @@ func HooksCommand() *cobra.Command { // (plugin initialization) from running for hook invocations. PersistentPreRunE: func(*cobra.Command, []string) error { return nil }, RunE: func(cmd *cobra.Command, args []string) error { - return handleHook(args, cmd.OutOrStdout()) + return handleHook(cmd.Context(), args, cmd.OutOrStdout()) }, } } -func handleHook(args []string, w io.Writer) error { +func handleHook(ctx context.Context, args []string, w io.Writer) error { if len(args) == 0 { return nil } @@ -127,6 +137,10 @@ func handleHook(args []string, w io.Writer) error { return nil } + if !logsTabEnabled(ctx) { + return nil + } + enc := json.NewEncoder(w) enc.SetEscapeHTML(false) return enc.Encode(hooks.Response{ diff --git a/cmd/compose/hooks_test.go b/cmd/compose/hooks_test.go index 0df020b09f..2841d79a95 100644 --- a/cmd/compose/hooks_test.go +++ b/cmd/compose/hooks_test.go @@ -18,7 +18,9 @@ package compose import ( "bytes" + "context" "encoding/json" + "os" "strings" "testing" @@ -28,16 +30,24 @@ import ( "github.com/docker/compose/v5/cmd/formatter" ) +// 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. +func TestMain(m *testing.M) { + logsTabEnabled = func(context.Context) bool { return true } + os.Exit(m.Run()) +} + func TestHandleHook_NoArgs(t *testing.T) { var buf bytes.Buffer - err := handleHook(nil, &buf) + err := handleHook(t.Context(), nil, &buf) assert.NilError(t, err) assert.Equal(t, buf.String(), "") } func TestHandleHook_InvalidJSON(t *testing.T) { var buf bytes.Buffer - err := handleHook([]string{"not json"}, &buf) + err := handleHook(t.Context(), []string{"not json"}, &buf) assert.NilError(t, err) assert.Equal(t, buf.String(), "") } @@ -47,7 +57,7 @@ func TestHandleHook_UnknownCommand(t *testing.T) { RootCmd: "compose push", }) var buf bytes.Buffer - err := handleHook([]string{data}, &buf) + err := handleHook(t.Context(), []string{data}, &buf) assert.NilError(t, err) assert.Equal(t, buf.String(), "") } @@ -66,7 +76,7 @@ func TestHandleHook_LogsCommand(t *testing.T) { RootCmd: tt.rootCmd, }) var buf bytes.Buffer - err := handleHook([]string{data}, &buf) + err := handleHook(t.Context(), []string{data}, &buf) assert.NilError(t, err) msg := unmarshalResponse(t, buf.Bytes()) @@ -110,7 +120,7 @@ func TestHandleHook_ComposeUpDetached(t *testing.T) { Flags: tt.flags, }) var buf bytes.Buffer - err := handleHook([]string{data}, &buf) + err := handleHook(t.Context(), []string{data}, &buf) assert.NilError(t, err) if tt.wantHint { @@ -131,7 +141,7 @@ func TestHandleHook_HintContainsOSC8Link(t *testing.T) { RootCmd: "compose logs", }) var buf bytes.Buffer - err := handleHook([]string{data}, &buf) + err := handleHook(t.Context(), []string{data}, &buf) assert.NilError(t, err) msg := unmarshalResponse(t, buf.Bytes()) @@ -147,7 +157,7 @@ func TestHandleHook_NoColorDisablesOsc8(t *testing.T) { RootCmd: "compose logs", }) var buf bytes.Buffer - err := handleHook([]string{data}, &buf) + err := handleHook(t.Context(), []string{data}, &buf) assert.NilError(t, err) msg := unmarshalResponse(t, buf.Bytes()) @@ -156,13 +166,29 @@ func TestHandleHook_NoColorDisablesOsc8(t *testing.T) { assert.Assert(t, !strings.Contains(msg.Template, "\033"), "hint should not contain ANSI escape sequences") } +func TestHandleHook_FeatureFlagDisabledSuppressesHint(t *testing.T) { + prev := logsTabEnabled + t.Cleanup(func() { logsTabEnabled = prev }) + logsTabEnabled = func(context.Context) bool { return false } + + for _, rootCmd := range []string{"compose logs", "logs"} { + t.Run(rootCmd, func(t *testing.T) { + data := marshalHookData(t, hooks.Request{RootCmd: rootCmd}) + var buf bytes.Buffer + err := handleHook(t.Context(), []string{data}, &buf) + assert.NilError(t, err) + assert.Equal(t, buf.String(), "") + }) + } +} + func TestHandleHook_ComposeAnsiNeverDisablesOsc8(t *testing.T) { t.Setenv("COMPOSE_ANSI", "never") data := marshalHookData(t, hooks.Request{ RootCmd: "compose logs", }) var buf bytes.Buffer - err := handleHook([]string{data}, &buf) + err := handleHook(t.Context(), []string{data}, &buf) assert.NilError(t, err) msg := unmarshalResponse(t, buf.Bytes()) diff --git a/internal/desktop/client.go b/internal/desktop/client.go index b7535e1f57..2bc4397ef0 100644 --- a/internal/desktop/client.go +++ b/internal/desktop/client.go @@ -23,9 +23,11 @@ import ( "io" "net" "net/http" - "path/filepath" "strings" + "github.com/docker/cli/cli/command" + cliflags "github.com/docker/cli/cli/flags" + "github.com/moby/moby/client" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "github.com/docker/compose/v5/internal" @@ -137,102 +139,68 @@ func (c *Client) FeatureFlags(ctx context.Context) (FeatureFlagResponse, error) return ret, nil } -// SettingValue represents a Docker Desktop setting with a locked flag and a value. -type SettingValue struct { - Locked bool `json:"locked"` - Value bool `json:"value"` -} - -// DesktopSettings represents the "desktop" section of Docker Desktop settings. -type DesktopSettings struct { - EnableLogsTab SettingValue `json:"enableLogsTab"` -} - -// SettingsResponse represents the Docker Desktop settings response. -type SettingsResponse struct { - Desktop DesktopSettings `json:"desktop"` -} - -// Settings fetches the Docker Desktop application settings. -func (c *Client) Settings(ctx context.Context) (*SettingsResponse, error) { - req, err := c.newRequest(ctx, http.MethodGet, "/app/settings", http.NoBody) - if err != nil { - return nil, err - } - - resp, err := c.client.Do(req) +// IsFeatureEnabled checks the feature flag (GET /features) for a given +// feature. Returns true when the feature is rolled out. +func (c *Client) IsFeatureEnabled(ctx context.Context, feature string) (bool, error) { + flags, err := c.FeatureFlags(ctx) if err != nil { - return nil, err - } - defer func() { - _ = resp.Body.Close() - }() - - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("unexpected status code: %d", resp.StatusCode) - } - - var ret SettingsResponse - if err := json.NewDecoder(resp.Body).Decode(&ret); err != nil { - return nil, err + return false, err } - return &ret, nil + return flags[feature].Enabled, nil } -// IsFeatureEnabled checks both the feature flag (GET /features) and the user -// setting (GET /app/settings) for a given feature. Returns true only when the -// feature is both rolled out and enabled by the user. Features without a -// corresponding setting entry are considered enabled if the flag is set. -func (c *Client) IsFeatureEnabled(ctx context.Context, feature string) (bool, error) { - flags, err := c.FeatureFlags(ctx) +// EndpointFromEngine returns the Docker Desktop API socket address advertised +// by the engine in its Info labels, or "" when the active engine is not +// Docker Desktop. +func EndpointFromEngine(ctx context.Context, apiClient client.APIClient) (string, error) { + info, err := apiClient.Info(ctx, client.InfoOptions{}) if err != nil { - return false, err + return "", err } - if !flags[feature].Enabled { - return false, nil + for _, l := range info.Info.Labels { + k, v, ok := strings.Cut(l, "=") + if ok && k == EngineLabel { + return v, nil + } } + return "", nil +} - check, hasCheck := featureSettingChecks[feature] - if !hasCheck { - // No setting to verify — feature flag alone is sufficient - return true, nil +// IsFeatureActive reports whether Docker Desktop is the active engine and the +// given feature flag is enabled. Returns false silently on any failure — the +// engine being unreachable, Desktop not being the active engine, or the flag +// being off — so callers can use this as a single gating check. +func IsFeatureActive(ctx context.Context, apiClient client.APIClient, feature string) bool { + endpoint, err := EndpointFromEngine(ctx, apiClient) + if err != nil || endpoint == "" { + return false } - // The /app/settings endpoint is served by the backend socket, not the - // docker-cli socket. Derive the backend socket path from the current - // endpoint. - backendEndpoint := backendSocketEndpoint(c.apiEndpoint) - backendCli := NewClient(backendEndpoint) - defer backendCli.Close() //nolint:errcheck + c := NewClient(endpoint) + defer c.Close() //nolint:errcheck - settings, err := backendCli.Settings(ctx) + enabled, err := c.IsFeatureEnabled(ctx, feature) if err != nil { - return false, err + return false } - return check(settings), nil + return enabled } -// backendSocketEndpoint derives the Docker Desktop backend socket endpoint -// from any socket endpoint in the same directory. -// -// On macOS/Linux: unix:///path/to/Data/docker-cli.sock → unix:///path/to/Data/backend.sock -// On Windows: npipe://./pipe/dockerDesktopLinuxEngine → npipe://./pipe/dockerBackendApiServer -func backendSocketEndpoint(endpoint string) string { - if sockPath, ok := strings.CutPrefix(endpoint, "unix://"); ok { - return "unix://" + filepath.Join(filepath.Dir(sockPath), "backend.sock") +// IsFeatureActiveStandalone is the convenience form of IsFeatureActive for +// callers without an existing engine API client (e.g. the compose plugin hook +// subprocess). It builds a Docker CLI using the ambient environment to +// resolve the active context, then delegates to IsFeatureActive. +func IsFeatureActiveStandalone(ctx context.Context, feature string) bool { + dockerCli, err := command.NewDockerCli(command.WithCombinedStreams(io.Discard)) + if err != nil { + return false } - if _, ok := strings.CutPrefix(endpoint, "npipe://"); ok { - return "npipe://./pipe/dockerBackendApiServer" + if err := dockerCli.Initialize(cliflags.NewClientOptions()); err != nil { + return false } - return endpoint -} + defer dockerCli.Client().Close() //nolint:errcheck -// featureSettingChecks maps feature flag names to their corresponding -// Docker Desktop setting check functions. -var featureSettingChecks = map[string]func(*SettingsResponse) bool{ - FeatureLogsTab: func(s *SettingsResponse) bool { - return s.Desktop.EnableLogsTab.Value - }, + return IsFeatureActive(ctx, dockerCli.Client(), feature) } func (c *Client) newRequest(ctx context.Context, method, path string, body io.Reader) (*http.Request, error) { diff --git a/internal/desktop/client_test.go b/internal/desktop/client_test.go index 085f574be9..7bb1546bca 100644 --- a/internal/desktop/client_test.go +++ b/internal/desktop/client_test.go @@ -24,41 +24,6 @@ import ( "gotest.tools/v3/assert" ) -func TestBackendSocketEndpoint(t *testing.T) { - tests := []struct { - name string - input string - expected string - }{ - { - name: "macOS unix socket", - input: "unix:///Users/me/Library/Containers/com.docker.docker/Data/docker-cli.sock", - expected: "unix:///Users/me/Library/Containers/com.docker.docker/Data/backend.sock", - }, - { - name: "Linux unix socket", - input: "unix:///run/desktop/docker-cli.sock", - expected: "unix:///run/desktop/backend.sock", - }, - { - name: "Windows named pipe", - input: "npipe://./pipe/dockerDesktopLinuxEngine", - expected: "npipe://./pipe/dockerBackendApiServer", - }, - { - name: "unknown scheme passthrough", - input: "tcp://localhost:2375", - expected: "tcp://localhost:2375", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := backendSocketEndpoint(tt.input) - assert.Equal(t, result, tt.expected) - }) - } -} - func TestClientPing(t *testing.T) { if testing.Short() { t.Skip("Skipped in short mode - test connects to Docker Desktop") diff --git a/pkg/compose/desktop.go b/pkg/compose/desktop.go index 9ca8ea4649..73fcf9eeaf 100644 --- a/pkg/compose/desktop.go +++ b/pkg/compose/desktop.go @@ -18,51 +18,18 @@ package compose import ( "context" - "strings" - - "github.com/moby/moby/client" "github.com/docker/compose/v5/internal/desktop" ) -// desktopEndpoint returns the Docker Desktop API socket address discovered -// from the Docker engine info labels. It returns "" when the active engine -// is not a Docker Desktop instance. -func (s *composeService) desktopEndpoint(ctx context.Context) (string, error) { - res, err := s.apiClient().Info(ctx, client.InfoOptions{}) - if err != nil { - return "", err - } - for _, l := range res.Info.Labels { - k, v, ok := strings.Cut(l, "=") - if ok && k == desktop.EngineLabel { - return v, nil - } - } - return "", nil -} - // isDesktopIntegrationActive returns true when Docker Desktop is the active engine. func (s *composeService) isDesktopIntegrationActive(ctx context.Context) (bool, error) { - endpoint, err := s.desktopEndpoint(ctx) + endpoint, err := desktop.EndpointFromEngine(ctx, s.apiClient()) return endpoint != "", err } -// isDesktopFeatureActive checks whether a Docker Desktop feature is both -// available (feature flag) and enabled by the user (settings). Returns false -// silently when Desktop is not running or unreachable. +// isDesktopFeatureActive checks whether a Docker Desktop feature flag is +// enabled. Returns false silently when Desktop is not running or unreachable. func (s *composeService) isDesktopFeatureActive(ctx context.Context, feature string) bool { - endpoint, err := s.desktopEndpoint(ctx) - if err != nil || endpoint == "" { - return false - } - - ddClient := desktop.NewClient(endpoint) - defer ddClient.Close() //nolint:errcheck - - enabled, err := ddClient.IsFeatureEnabled(ctx, feature) - if err != nil { - return false - } - return enabled + return desktop.IsFeatureActive(ctx, s.apiClient(), feature) }