diff --git a/client/container_exec.go b/client/container_exec.go index 061dcfaa67..1af6a4e20f 100644 --- a/client/container_exec.go +++ b/client/container_exec.go @@ -12,17 +12,17 @@ import ( // ExecCreateOptions is a small subset of the Config struct that holds the configuration // for the exec feature of docker. type ExecCreateOptions struct { - User string // User that will run the command - Privileged bool // Is the container in privileged mode - TTY bool // Attach standard streams to a tty. - ConsoleSize *[2]uint `json:",omitempty"` // Initial console size [height, width] - AttachStdin bool // Attach the standard input, makes possible user interaction - AttachStderr bool // Attach the standard error - AttachStdout bool // Attach the standard output - DetachKeys string // Escape keys for detach - Env []string // Environment variables - WorkingDir string // Working directory - Cmd []string // Execution commands and args + User string // User that will run the command + Privileged bool // Is the container in privileged mode + TTY bool // Attach standard streams to a tty. + ConsoleSize ConsoleSize // Initial terminal size [height, width], unused if TTY == false + AttachStdin bool // Attach the standard input, makes possible user interaction + AttachStderr bool // Attach the standard error + AttachStdout bool // Attach the standard output + DetachKeys string // Escape keys for detach + Env []string // Environment variables + WorkingDir string // Working directory + Cmd []string // Execution commands and args } // ExecCreateResult holds the result of creating a container exec. @@ -37,11 +37,16 @@ func (cli *Client) ExecCreate(ctx context.Context, containerID string, options E return ExecCreateResult{}, err } + consoleSize, err := getConsoleSize(options.TTY, options.ConsoleSize) + if err != nil { + return ExecCreateResult{}, err + } + req := container.ExecCreateRequest{ User: options.User, Privileged: options.Privileged, Tty: options.TTY, - ConsoleSize: options.ConsoleSize, + ConsoleSize: consoleSize, AttachStdin: options.AttachStdin, AttachStderr: options.AttachStderr, AttachStdout: options.AttachStdout, @@ -73,7 +78,7 @@ type ExecStartOptions struct { // Check if there's a tty TTY bool // Terminal size [height, width], unused if TTY == false - ConsoleSize ConsoleSize `json:",omitzero"` + ConsoleSize ConsoleSize } // ExecStartResult holds the result of starting a container exec. @@ -82,13 +87,16 @@ type ExecStartResult struct { // ExecStart starts an exec process already created in the docker host. func (cli *Client) ExecStart(ctx context.Context, execID string, options ExecStartOptions) (ExecStartResult, error) { - req := container.ExecStartRequest{ - Detach: options.Detach, - Tty: options.TTY, - } - if err := applyConsoleSize(&req, &options.ConsoleSize); err != nil { + consoleSize, err := getConsoleSize(options.TTY, options.ConsoleSize) + if err != nil { return ExecStartResult{}, err } + + req := container.ExecStartRequest{ + Detach: options.Detach, + Tty: options.TTY, + ConsoleSize: consoleSize, + } resp, err := cli.post(ctx, "/exec/"+execID+"/start", nil, req, nil) defer ensureReaderClosed(resp) return ExecStartResult{}, err @@ -126,27 +134,29 @@ type ExecAttachResult struct { // // [stdcopy.StdCopy]: https://pkg.go.dev/github.com/moby/moby/api/pkg/stdcopy#StdCopy func (cli *Client) ExecAttach(ctx context.Context, execID string, options ExecAttachOptions) (ExecAttachResult, error) { - req := container.ExecStartRequest{ - Detach: false, - Tty: options.TTY, - } - if err := applyConsoleSize(&req, &options.ConsoleSize); err != nil { + consoleSize, err := getConsoleSize(options.TTY, options.ConsoleSize) + if err != nil { return ExecAttachResult{}, err } + req := container.ExecStartRequest{ + Detach: false, + Tty: options.TTY, + ConsoleSize: consoleSize, + } response, err := cli.postHijacked(ctx, "/exec/"+execID+"/start", nil, req, http.Header{ "Content-Type": {"application/json"}, }) return ExecAttachResult{HijackedResponse: response}, err } -func applyConsoleSize(req *container.ExecStartRequest, consoleSize *ConsoleSize) error { +func getConsoleSize(hasTTY bool, consoleSize ConsoleSize) (*[2]uint, error) { if consoleSize.Height != 0 || consoleSize.Width != 0 { - if !req.Tty { - return errdefs.ErrInvalidArgument.WithMessage("console size is only supported when TTY is enabled") + if !hasTTY { + return nil, errdefs.ErrInvalidArgument.WithMessage("console size is only supported when TTY is enabled") } - req.ConsoleSize = &[2]uint{consoleSize.Height, consoleSize.Width} + return &[2]uint{consoleSize.Height, consoleSize.Width}, nil } - return nil + return nil, nil } // ExecInspectOptions holds options for inspecting a container exec. diff --git a/client/container_exec_test.go b/client/container_exec_test.go index 3de85fd980..a68fc6aef4 100644 --- a/client/container_exec_test.go +++ b/client/container_exec_test.go @@ -1,7 +1,6 @@ package client import ( - "context" "encoding/json" "fmt" "net/http" @@ -20,14 +19,14 @@ func TestExecCreateError(t *testing.T) { ) assert.NilError(t, err) - _, err = client.ExecCreate(context.Background(), "container_id", ExecCreateOptions{}) + _, err = client.ExecCreate(t.Context(), "container_id", ExecCreateOptions{}) assert.Check(t, is.ErrorType(err, cerrdefs.IsInternal)) - _, err = client.ExecCreate(context.Background(), "", ExecCreateOptions{}) + _, err = client.ExecCreate(t.Context(), "", ExecCreateOptions{}) assert.Check(t, is.ErrorType(err, cerrdefs.IsInvalidArgument)) assert.Check(t, is.ErrorContains(err, "value is empty")) - _, err = client.ExecCreate(context.Background(), " ", ExecCreateOptions{}) + _, err = client.ExecCreate(t.Context(), " ", ExecCreateOptions{}) assert.Check(t, is.ErrorType(err, cerrdefs.IsInvalidArgument)) assert.Check(t, is.ErrorContains(err, "value is empty")) } @@ -40,7 +39,7 @@ func TestExecCreateConnectionError(t *testing.T) { client, err := New(WithAPIVersionNegotiation(), WithHost("tcp://no-such-host.invalid")) assert.NilError(t, err) - _, err = client.ExecCreate(context.Background(), "container_id", ExecCreateOptions{}) + _, err = client.ExecCreate(t.Context(), "container_id", ExecCreateOptions{}) assert.Check(t, is.ErrorType(err, IsErrConnectionFailed)) } @@ -69,7 +68,7 @@ func TestExecCreate(t *testing.T) { ) assert.NilError(t, err) - res, err := client.ExecCreate(context.Background(), "container_id", ExecCreateOptions{ + res, err := client.ExecCreate(t.Context(), "container_id", ExecCreateOptions{ User: "user", }) assert.NilError(t, err) @@ -82,7 +81,7 @@ func TestExecStartError(t *testing.T) { ) assert.NilError(t, err) - _, err = client.ExecStart(context.Background(), "nothing", ExecStartOptions{}) + _, err = client.ExecStart(t.Context(), "nothing", ExecStartOptions{}) assert.Check(t, is.ErrorType(err, cerrdefs.IsInternal)) } @@ -108,7 +107,7 @@ func TestExecStart(t *testing.T) { ) assert.NilError(t, err) - _, err = client.ExecStart(context.Background(), "exec_id", ExecStartOptions{ + _, err = client.ExecStart(t.Context(), "exec_id", ExecStartOptions{ Detach: true, TTY: false, }) @@ -116,20 +115,63 @@ func TestExecStart(t *testing.T) { } func TestExecStartConsoleSize(t *testing.T) { - client, err := New( - WithMockClient(func(req *http.Request) (*http.Response, error) { - return nil, nil - }), - ) - assert.NilError(t, err) + tests := []struct { + doc string + options ExecStartOptions + expErr string + expReq container.ExecStartRequest + }{ + { + doc: "without TTY", + options: ExecStartOptions{ + Detach: true, + TTY: false, + ConsoleSize: ConsoleSize{Height: 100, Width: 200}, + }, + expErr: "console size is only supported when TTY is enabled", + }, + { + doc: "with TTY", + options: ExecStartOptions{ + Detach: true, + TTY: true, + ConsoleSize: ConsoleSize{Height: 100, Width: 200}, + }, + expReq: container.ExecStartRequest{ + Detach: true, + Tty: true, + ConsoleSize: &[2]uint{100, 200}, + }, + }, + } + for _, tc := range tests { + t.Run(tc.doc, func(t *testing.T) { + var actualReq container.ExecStartRequest + client, err := New( + WithMockClient(func(req *http.Request) (*http.Response, error) { + if tc.expErr != "" { + return nil, fmt.Errorf("should not have made API request") + } + if err := json.NewDecoder(req.Body).Decode(&actualReq); err != nil { + return nil, err + } - _, err = client.ExecStart(context.Background(), "exec_id", ExecStartOptions{ - Detach: true, - TTY: false, - ConsoleSize: ConsoleSize{Height: 100, Width: 100}, - }) - assert.Check(t, is.ErrorType(err, errdefs.IsInvalidArgument)) - assert.Check(t, is.ErrorContains(err, "console size is only supported when TTY is enabled")) + return mockJSONResponse(http.StatusOK, nil, ExecStartResult{})(req) + }), + ) + assert.NilError(t, err) + + _, err = client.ExecStart(t.Context(), "exec_id", tc.options) + if tc.expErr != "" { + assert.Check(t, is.ErrorType(err, errdefs.IsInvalidArgument)) + assert.Check(t, is.ErrorContains(err, tc.expErr)) + assert.Check(t, is.DeepEqual(actualReq, tc.expReq)) + } else { + assert.NilError(t, err) + assert.Check(t, is.DeepEqual(actualReq, tc.expReq)) + } + }) + } } func TestExecInspectError(t *testing.T) { @@ -138,7 +180,7 @@ func TestExecInspectError(t *testing.T) { ) assert.NilError(t, err) - _, err = client.ExecInspect(context.Background(), "nothing", ExecInspectOptions{}) + _, err = client.ExecInspect(t.Context(), "nothing", ExecInspectOptions{}) assert.Check(t, is.ErrorType(err, cerrdefs.IsInternal)) } @@ -157,7 +199,7 @@ func TestExecInspect(t *testing.T) { ) assert.NilError(t, err) - inspect, err := client.ExecInspect(context.Background(), "exec_id", ExecInspectOptions{}) + inspect, err := client.ExecInspect(t.Context(), "exec_id", ExecInspectOptions{}) assert.NilError(t, err) assert.Check(t, is.Equal(inspect.ID, "exec_id")) assert.Check(t, is.Equal(inspect.ContainerID, "container_id")) diff --git a/integration/container/exec_linux_test.go b/integration/container/exec_linux_test.go index fa2a376b6f..f91da3d711 100644 --- a/integration/container/exec_linux_test.go +++ b/integration/container/exec_linux_test.go @@ -21,7 +21,10 @@ func TestExecConsoleSize(t *testing.T) { result, err := container.Exec(ctx, apiClient, cID, []string{"stty", "size"}, func(ec *client.ExecCreateOptions) { ec.TTY = true - ec.ConsoleSize = &[2]uint{57, 123} + ec.ConsoleSize = client.ConsoleSize{ + Height: 57, + Width: 123, + } }, ) diff --git a/vendor/github.com/moby/moby/client/container_exec.go b/vendor/github.com/moby/moby/client/container_exec.go index 061dcfaa67..1af6a4e20f 100644 --- a/vendor/github.com/moby/moby/client/container_exec.go +++ b/vendor/github.com/moby/moby/client/container_exec.go @@ -12,17 +12,17 @@ import ( // ExecCreateOptions is a small subset of the Config struct that holds the configuration // for the exec feature of docker. type ExecCreateOptions struct { - User string // User that will run the command - Privileged bool // Is the container in privileged mode - TTY bool // Attach standard streams to a tty. - ConsoleSize *[2]uint `json:",omitempty"` // Initial console size [height, width] - AttachStdin bool // Attach the standard input, makes possible user interaction - AttachStderr bool // Attach the standard error - AttachStdout bool // Attach the standard output - DetachKeys string // Escape keys for detach - Env []string // Environment variables - WorkingDir string // Working directory - Cmd []string // Execution commands and args + User string // User that will run the command + Privileged bool // Is the container in privileged mode + TTY bool // Attach standard streams to a tty. + ConsoleSize ConsoleSize // Initial terminal size [height, width], unused if TTY == false + AttachStdin bool // Attach the standard input, makes possible user interaction + AttachStderr bool // Attach the standard error + AttachStdout bool // Attach the standard output + DetachKeys string // Escape keys for detach + Env []string // Environment variables + WorkingDir string // Working directory + Cmd []string // Execution commands and args } // ExecCreateResult holds the result of creating a container exec. @@ -37,11 +37,16 @@ func (cli *Client) ExecCreate(ctx context.Context, containerID string, options E return ExecCreateResult{}, err } + consoleSize, err := getConsoleSize(options.TTY, options.ConsoleSize) + if err != nil { + return ExecCreateResult{}, err + } + req := container.ExecCreateRequest{ User: options.User, Privileged: options.Privileged, Tty: options.TTY, - ConsoleSize: options.ConsoleSize, + ConsoleSize: consoleSize, AttachStdin: options.AttachStdin, AttachStderr: options.AttachStderr, AttachStdout: options.AttachStdout, @@ -73,7 +78,7 @@ type ExecStartOptions struct { // Check if there's a tty TTY bool // Terminal size [height, width], unused if TTY == false - ConsoleSize ConsoleSize `json:",omitzero"` + ConsoleSize ConsoleSize } // ExecStartResult holds the result of starting a container exec. @@ -82,13 +87,16 @@ type ExecStartResult struct { // ExecStart starts an exec process already created in the docker host. func (cli *Client) ExecStart(ctx context.Context, execID string, options ExecStartOptions) (ExecStartResult, error) { - req := container.ExecStartRequest{ - Detach: options.Detach, - Tty: options.TTY, - } - if err := applyConsoleSize(&req, &options.ConsoleSize); err != nil { + consoleSize, err := getConsoleSize(options.TTY, options.ConsoleSize) + if err != nil { return ExecStartResult{}, err } + + req := container.ExecStartRequest{ + Detach: options.Detach, + Tty: options.TTY, + ConsoleSize: consoleSize, + } resp, err := cli.post(ctx, "/exec/"+execID+"/start", nil, req, nil) defer ensureReaderClosed(resp) return ExecStartResult{}, err @@ -126,27 +134,29 @@ type ExecAttachResult struct { // // [stdcopy.StdCopy]: https://pkg.go.dev/github.com/moby/moby/api/pkg/stdcopy#StdCopy func (cli *Client) ExecAttach(ctx context.Context, execID string, options ExecAttachOptions) (ExecAttachResult, error) { - req := container.ExecStartRequest{ - Detach: false, - Tty: options.TTY, - } - if err := applyConsoleSize(&req, &options.ConsoleSize); err != nil { + consoleSize, err := getConsoleSize(options.TTY, options.ConsoleSize) + if err != nil { return ExecAttachResult{}, err } + req := container.ExecStartRequest{ + Detach: false, + Tty: options.TTY, + ConsoleSize: consoleSize, + } response, err := cli.postHijacked(ctx, "/exec/"+execID+"/start", nil, req, http.Header{ "Content-Type": {"application/json"}, }) return ExecAttachResult{HijackedResponse: response}, err } -func applyConsoleSize(req *container.ExecStartRequest, consoleSize *ConsoleSize) error { +func getConsoleSize(hasTTY bool, consoleSize ConsoleSize) (*[2]uint, error) { if consoleSize.Height != 0 || consoleSize.Width != 0 { - if !req.Tty { - return errdefs.ErrInvalidArgument.WithMessage("console size is only supported when TTY is enabled") + if !hasTTY { + return nil, errdefs.ErrInvalidArgument.WithMessage("console size is only supported when TTY is enabled") } - req.ConsoleSize = &[2]uint{consoleSize.Height, consoleSize.Width} + return &[2]uint{consoleSize.Height, consoleSize.Width}, nil } - return nil + return nil, nil } // ExecInspectOptions holds options for inspecting a container exec.