Skip to content

Commit 4915b77

Browse files
mikesir87ndeloof
authored andcommitted
fix: only pass ConsoleSize to ExecAttach when TTY is enabled
The moby/moby client (v0.2.2) validates that ConsoleSize is zero when TTY is disabled, returning "console size is only supported when TTY is enabled" otherwise. Previously, ConsoleSize was populated unconditionally from GetTtySize(), which returns real terminal dimensions when Compose is run interactively — causing post_start hooks to fail for services without `tty: true`. Fix by only reading and passing the console size when service.Tty is true. Signed-off-by: Michael Irwin <michael.irwin@docker.com> Resolves #13615
1 parent 85d6770 commit 4915b77

File tree

2 files changed

+126
-8
lines changed

2 files changed

+126
-8
lines changed

pkg/compose/hook.go

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,15 +61,17 @@ func (s composeService) runHook(ctx context.Context, ctr container.Summary, serv
6161
return s.runWaitExec(ctx, exec.ID, service, listener)
6262
}
6363

64-
height, width := s.stdout().GetTtySize()
65-
consoleSize := client.ConsoleSize{
66-
Width: width,
67-
Height: height,
64+
attachOptions := client.ExecAttachOptions{
65+
TTY: service.Tty,
6866
}
69-
attach, err := s.apiClient().ExecAttach(ctx, exec.ID, client.ExecAttachOptions{
70-
TTY: service.Tty,
71-
ConsoleSize: consoleSize,
72-
})
67+
if service.Tty {
68+
height, width := s.stdout().GetTtySize()
69+
attachOptions.ConsoleSize = client.ConsoleSize{
70+
Width: width,
71+
Height: height,
72+
}
73+
}
74+
attach, err := s.apiClient().ExecAttach(ctx, exec.ID, attachOptions)
7375
if err != nil {
7476
return err
7577
}

pkg/compose/hook_test.go

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
/*
2+
Copyright 2020 Docker Compose CLI authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package compose
18+
19+
import (
20+
"net"
21+
"os"
22+
"testing"
23+
24+
"github.com/compose-spec/compose-go/v2/types"
25+
"github.com/containerd/console"
26+
"github.com/docker/cli/cli/streams"
27+
"github.com/moby/moby/api/types/container"
28+
"github.com/moby/moby/client"
29+
"go.uber.org/mock/gomock"
30+
"gotest.tools/v3/assert"
31+
32+
"github.com/docker/compose/v5/pkg/api"
33+
"github.com/docker/compose/v5/pkg/mocks"
34+
)
35+
36+
// TestRunHook_ConsoleSize verifies that ConsoleSize is only passed to ExecAttach
37+
// when the service has TTY enabled. When TTY is disabled, passing a non-zero
38+
// ConsoleSize causes the Docker daemon to return "console size is only supported
39+
// when TTY is enabled" (regression introduced in v5.1.0).
40+
func TestRunHook_ConsoleSize(t *testing.T) {
41+
tests := []struct {
42+
name string
43+
tty bool
44+
expectedConsole client.ConsoleSize
45+
}{
46+
{
47+
name: "no tty - ConsoleSize must be zero",
48+
tty: false,
49+
expectedConsole: client.ConsoleSize{},
50+
},
51+
{
52+
name: "with tty - ConsoleSize should reflect terminal dimensions",
53+
tty: true,
54+
expectedConsole: client.ConsoleSize{Width: 80, Height: 24},
55+
},
56+
}
57+
58+
for _, tc := range tests {
59+
t.Run(tc.name, func(t *testing.T) {
60+
mockCtrl := gomock.NewController(t)
61+
defer mockCtrl.Finish()
62+
63+
mockAPI := mocks.NewMockAPIClient(mockCtrl)
64+
mockCli := mocks.NewMockCli(mockCtrl)
65+
mockCli.EXPECT().Client().Return(mockAPI).AnyTimes()
66+
mockCli.EXPECT().Err().Return(streams.NewOut(os.Stderr)).AnyTimes()
67+
68+
// Create a PTY so GetTtySize() returns real non-zero dimensions,
69+
// simulating an interactive terminal session.
70+
pty, slavePath, err := console.NewPty()
71+
assert.NilError(t, err)
72+
defer pty.Close() //nolint:errcheck
73+
assert.NilError(t, pty.Resize(console.WinSize{Height: 24, Width: 80}))
74+
75+
slaveFile, err := os.OpenFile(slavePath, os.O_RDWR, 0)
76+
assert.NilError(t, err)
77+
defer slaveFile.Close() //nolint:errcheck
78+
79+
mockCli.EXPECT().Out().Return(streams.NewOut(slaveFile)).AnyTimes()
80+
81+
service := types.ServiceConfig{
82+
Name: "test",
83+
Tty: tc.tty,
84+
}
85+
hook := types.ServiceHook{Command: []string{"echo", "hello"}}
86+
ctr := container.Summary{ID: "container123"}
87+
88+
mockAPI.EXPECT().
89+
ExecCreate(gomock.Any(), "container123", gomock.Any()).
90+
Return(client.ExecCreateResult{ID: "exec123"}, nil)
91+
92+
// Return a pipe that immediately closes so the reader gets EOF.
93+
serverConn, clientConn := net.Pipe()
94+
serverConn.Close() //nolint:errcheck
95+
mockAPI.EXPECT().
96+
ExecAttach(gomock.Any(), "exec123", client.ExecAttachOptions{
97+
TTY: tc.tty,
98+
ConsoleSize: tc.expectedConsole,
99+
}).
100+
Return(client.ExecAttachResult{
101+
HijackedResponse: client.NewHijackedResponse(clientConn, ""),
102+
}, nil)
103+
104+
mockAPI.EXPECT().
105+
ExecInspect(gomock.Any(), "exec123", gomock.Any()).
106+
Return(client.ExecInspectResult{ExitCode: 0}, nil)
107+
108+
s, err := NewComposeService(mockCli)
109+
assert.NilError(t, err)
110+
111+
noopListener := func(api.ContainerEvent) {}
112+
err = s.(*composeService).runHook(t.Context(), ctr, service, hook, noopListener)
113+
assert.NilError(t, err)
114+
})
115+
}
116+
}

0 commit comments

Comments
 (0)