search

testcontainers / testcontainers-go

Testcontainers for Go is a Go package that makes it simple to create and clean up container-based dependencies for automated integration/smoke tests. The clean, easy-to-use API enables developers to programmatically define containers that should be run as part of a test and clean up those resources when the test is done.
https://golang.testcontainers.org
MIT License
3.57k stars 492 forks source link

[Bug]: hijacked connection returned by ContainerExecAttach is not closed #1357

Open AlexanderYastrebov opened 1 year ago

AlexanderYastrebov commented 1 year ago

Testcontainers version

v0.21.0

Using the latest Testcontainers version?

Yes

Host OS

Linux

Host arch

x86

Go version

1.20

Docker version

-

Docker info

-

What happened?

Here https://github.com/testcontainers/testcontainers-go/blob/f5a4a541f71a324d1bf81f40700e5426259dbae6/docker.go#L485-L492 cli.ContainerExecAttach returns hijacked connections which should be closed by the caller:

It's up to the called to close the hijacked connection by calling types.HijackedResponse.Close.

Unfortunately Exec returns io.Reader https://github.com/testcontainers/testcontainers-go/blob/f5a4a541f71a324d1bf81f40700e5426259dbae6/container.go#L51

I think it should return io.ReadCloser that closes hijacked connection on Close()

See related #321

Relevant log output

No response

Additional information

No response

mdelapenya commented 1 year ago

Hi @AlexanderYastrebov thanks for reporting this. I now understand the issue with the Exec function not returning a ReadCloser.

I think we can update the signature as a breaking change in an upcoming minor release if needed, although I'm planning to start working on a v1 branch for an eventual v1.0.0 release of tc-go.