From 4c4def7778f19ea18fcfaea3605f52703e5e242f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Wed, 16 Nov 2022 19:42:26 +0100 Subject: [PATCH] fix: do not prepend garbage in the container.Exec response (#624) * chore: add a test demonstrating the bug * fix: handle container exec attach using TTY * Revert "fix: handle container exec attach using TTY" This reverts commit 5733288954c0ce56c39dec986ef9c8995593bff1. * chore: handle multiplexed output --- container.go | 3 +- docker.go | 13 +++++++-- docker_exec_test.go | 67 +++++++++++++++++++++++++++++++++++++++++++++ exec/processor.go | 44 +++++++++++++++++++++++++++++ wait/exec_test.go | 3 +- wait/exit_test.go | 3 +- wait/log_test.go | 3 +- wait/wait.go | 3 +- 8 files changed, 132 insertions(+), 7 deletions(-) create mode 100644 docker_exec_test.go create mode 100644 exec/processor.go diff --git a/container.go b/container.go index ecf0186305..dc03eb82ed 100644 --- a/container.go +++ b/container.go @@ -12,6 +12,7 @@ import ( "github.com/docker/docker/pkg/archive" "github.com/docker/go-connections/nat" + tcexec "github.com/testcontainers/testcontainers-go/exec" "github.com/testcontainers/testcontainers-go/wait" ) @@ -54,7 +55,7 @@ type Container interface { State(context.Context) (*types.ContainerState, error) // returns container's running state Networks(context.Context) ([]string, error) // get container networks NetworkAliases(context.Context) (map[string][]string, error) // get container network aliases for a network - Exec(ctx context.Context, cmd []string) (int, io.Reader, error) + Exec(ctx context.Context, cmd []string, options ...tcexec.ProcessOption) (int, io.Reader, error) ContainerIP(context.Context) (string, error) // get container ip ContainerIPs(context.Context) ([]string, error) // get all container IPs CopyToContainer(ctx context.Context, fileContent []byte, containerFilePath string, fileMode int64) error diff --git a/docker.go b/docker.go index 04c70a6487..6e3b681942 100644 --- a/docker.go +++ b/docker.go @@ -33,6 +33,7 @@ import ( "github.com/moby/term" specs "github.com/opencontainers/image-spec/specs-go/v1" + tcexec "github.com/testcontainers/testcontainers-go/exec" "github.com/testcontainers/testcontainers-go/wait" ) @@ -433,7 +434,7 @@ func (c *DockerContainer) NetworkAliases(ctx context.Context) (map[string][]stri return a, nil } -func (c *DockerContainer) Exec(ctx context.Context, cmd []string) (int, io.Reader, error) { +func (c *DockerContainer) Exec(ctx context.Context, cmd []string, options ...tcexec.ProcessOption) (int, io.Reader, error) { cli := c.provider.client response, err := cli.ContainerExecCreate(ctx, c.ID, types.ExecConfig{ Cmd: cmd, @@ -450,6 +451,14 @@ func (c *DockerContainer) Exec(ctx context.Context, cmd []string) (int, io.Reade return 0, nil, err } + opt := &tcexec.ProcessOptions{ + Reader: hijack.Reader, + } + + for _, o := range options { + o.Apply(opt) + } + var exitCode int for { execResp, err := cli.ContainerExecInspect(ctx, response.ID) @@ -465,7 +474,7 @@ func (c *DockerContainer) Exec(ctx context.Context, cmd []string) (int, io.Reade time.Sleep(100 * time.Millisecond) } - return exitCode, hijack.Reader, nil + return exitCode, opt.Reader, nil } type FileFromContainer struct { diff --git a/docker_exec_test.go b/docker_exec_test.go new file mode 100644 index 0000000000..a31b1eeb01 --- /dev/null +++ b/docker_exec_test.go @@ -0,0 +1,67 @@ +package testcontainers + +import ( + "context" + "io" + "strings" + "testing" + + "github.com/stretchr/testify/require" + tcexec "github.com/testcontainers/testcontainers-go/exec" +) + +func TestExecWithMultiplexedResponse(t *testing.T) { + ctx := context.Background() + req := ContainerRequest{ + Image: nginxAlpineImage, + } + + container, err := GenericContainer(ctx, GenericContainerRequest{ + ProviderType: providerType, + ContainerRequest: req, + Started: true, + }) + + require.NoError(t, err) + terminateContainerOnEnd(t, ctx, container) + + code, reader, err := container.Exec(ctx, []string{"ls", "/usr/share/nginx"}, tcexec.Multiplexed()) + require.NoError(t, err) + require.Zero(t, code) + require.NotNil(t, reader) + + b, err := io.ReadAll(reader) + require.NoError(t, err) + require.NotNil(t, b) + + str := string(b) + require.Equal(t, "html\n", str) +} + +func TestExecWithNonMultiplexedResponse(t *testing.T) { + ctx := context.Background() + req := ContainerRequest{ + Image: nginxAlpineImage, + } + + container, err := GenericContainer(ctx, GenericContainerRequest{ + ProviderType: providerType, + ContainerRequest: req, + Started: true, + }) + + require.NoError(t, err) + terminateContainerOnEnd(t, ctx, container) + + code, reader, err := container.Exec(ctx, []string{"ls", "/usr/share/nginx"}) + require.NoError(t, err) + require.Zero(t, code) + require.NotNil(t, reader) + + b, err := io.ReadAll(reader) + require.NoError(t, err) + require.NotNil(t, b) + + str := string(b) + require.True(t, strings.HasSuffix(str, "html\n")) +} diff --git a/exec/processor.go b/exec/processor.go new file mode 100644 index 0000000000..63987e461f --- /dev/null +++ b/exec/processor.go @@ -0,0 +1,44 @@ +package exec + +import ( + "bytes" + "io" + + "github.com/docker/docker/pkg/stdcopy" +) + +// ProcessOptions defines options applicable to the reader processor +type ProcessOptions struct { + Reader io.Reader +} + +// ProcessOption defines a common interface to modify the reader processor +// These options can be passed to the Exec function in a variadic way to customize the returned Reader instance +type ProcessOption interface { + Apply(opts *ProcessOptions) +} + +type ProcessOptionFunc func(opts *ProcessOptions) + +func (fn ProcessOptionFunc) Apply(opts *ProcessOptions) { + fn(opts) +} + +func Multiplexed() ProcessOption { + return ProcessOptionFunc(func(opts *ProcessOptions) { + done := make(chan struct{}) + + var outBuff bytes.Buffer + var errBuff bytes.Buffer + go func() { + if _, err := stdcopy.StdCopy(&outBuff, &errBuff, opts.Reader); err != nil { + return + } + close(done) + }() + + <-done + + opts.Reader = &outBuff + }) +} diff --git a/wait/exec_test.go b/wait/exec_test.go index f796c8893a..7c267b5d60 100644 --- a/wait/exec_test.go +++ b/wait/exec_test.go @@ -12,6 +12,7 @@ import ( "github.com/docker/go-connections/nat" "github.com/testcontainers/testcontainers-go" + tcexec "github.com/testcontainers/testcontainers-go/exec" "github.com/testcontainers/testcontainers-go/wait" ) @@ -62,7 +63,7 @@ func (st mockExecTarget) Logs(_ context.Context) (io.ReadCloser, error) { return nil, errors.New("not implemented") } -func (st mockExecTarget) Exec(ctx context.Context, _ []string) (int, io.Reader, error) { +func (st mockExecTarget) Exec(ctx context.Context, _ []string, options ...tcexec.ProcessOption) (int, io.Reader, error) { time.Sleep(st.waitDuration) if err := ctx.Err(); err != nil { diff --git a/wait/exit_test.go b/wait/exit_test.go index 7e00136e90..9a4f7eded3 100644 --- a/wait/exit_test.go +++ b/wait/exit_test.go @@ -8,6 +8,7 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/go-connections/nat" + tcexec "github.com/testcontainers/testcontainers-go/exec" ) type exitStrategyTarget struct { @@ -30,7 +31,7 @@ func (st exitStrategyTarget) Logs(ctx context.Context) (io.ReadCloser, error) { return nil, nil } -func (st exitStrategyTarget) Exec(ctx context.Context, cmd []string) (int, io.Reader, error) { +func (st exitStrategyTarget) Exec(ctx context.Context, cmd []string, options ...tcexec.ProcessOption) (int, io.Reader, error) { return 0, nil, nil } diff --git a/wait/log_test.go b/wait/log_test.go index c209cba0d6..bb902d0c44 100644 --- a/wait/log_test.go +++ b/wait/log_test.go @@ -9,6 +9,7 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/go-connections/nat" + tcexec "github.com/testcontainers/testcontainers-go/exec" ) type noopStrategyTarget struct { @@ -31,7 +32,7 @@ func (st noopStrategyTarget) Logs(ctx context.Context) (io.ReadCloser, error) { return st.ioReaderCloser, nil } -func (st noopStrategyTarget) Exec(ctx context.Context, cmd []string) (int, io.Reader, error) { +func (st noopStrategyTarget) Exec(ctx context.Context, cmd []string, options ...tcexec.ProcessOption) (int, io.Reader, error) { return 0, nil, nil } func (st noopStrategyTarget) State(ctx context.Context) (*types.ContainerState, error) { diff --git a/wait/wait.go b/wait/wait.go index fedee44d5f..de6f581136 100644 --- a/wait/wait.go +++ b/wait/wait.go @@ -7,6 +7,7 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/go-connections/nat" + tcexec "github.com/testcontainers/testcontainers-go/exec" ) type Strategy interface { @@ -18,7 +19,7 @@ type StrategyTarget interface { Ports(ctx context.Context) (nat.PortMap, error) MappedPort(context.Context, nat.Port) (nat.Port, error) Logs(context.Context) (io.ReadCloser, error) - Exec(ctx context.Context, cmd []string) (int, io.Reader, error) + Exec(ctx context.Context, cmd []string, options ...tcexec.ProcessOption) (int, io.Reader, error) State(context.Context) (*types.ContainerState, error) }