testcontainers / testcontainers-hs

Docker containers for your integration tests! http://hackage.haskell.org/package/testcontainers
MIT License
56 stars 12 forks source link

[#50] Consider Docker host IP during port mapping resolution #51

Open rvem opened 3 months ago

rvem commented 3 months ago

In some cases, 'localhost' might be resolved to IPv6 '::1', while 'containerPort' will grab IPv4 host IP port mapping by default, when Docker maps different ports for host's IPv4 and IPv6 addresses, this might cause timeout errors.

To solve this issue, the Docker host is resolved from 'localhost' at container creation time, host IP protocol version is later used to deduce the corresponding port in 'containerPort' function.

Additionally, helper 'containerAddress', as well as 'TraceOpenSocket' and 'TraceHttpCall' now operate 'IP' addresses instead of plain 'Text' domains. This should be safe since we only actually consider domains within some Docker network or 'localhost' at the moment.

My solution is quite hacky since I was a bit afraid to break backward compatibility, would be happy to hear your feedback

Somewhat resolves #50.

alexbiehl commented 3 months ago

Nice, thanks for the contribution.

Did you research how Java, Ruby and Go testcontainers approach the problem?

That being said, the only way this breaks backward compat is by introducing the IP type where it was a Text before? That part of the change can be undone easily, no? Would love to avoid breaking API changes.

rvem commented 3 months ago

That part of the change can be undone easily, no?

Yeah, but I considered avoiding Text -> IP -> Text -> IP conversions. Additionally, for waitForHttp the manager expects the IPv6 host address to be wrapped with [] while getAddrInfo expects IPv6 without them. Perhaps we can introduce something like

data ContainerAddress = ContainerIP IP | ContainerAlias Text

with the Show/FromString instances. Though, this looks a bit redundant because AFAIU currently we can always deduce the container IP from inspect output

rvem commented 3 months ago

Did you research how Java, Ruby and Go testcontainers approach the problem?

testcontainers-go has a similar issue, but it was closed, see https://github.com/testcontainers/testcontainers-go/issues/551. They suggest exposing the port via IPv4 0.0.0.0 explicitly. For testcontainers-node this is also somewhat known issue, see https://github.com/testcontainers/testcontainers-node/blob/main/docs/supported-container-runtimes.md#usage-1

rvem commented 3 months ago

They suggest exposing the port via IPv4 0.0.0.0 explicitly

I also have a branch with changes that allow exposing a container port on a specific host IP, see https://github.com/serokell/testcontainers-hs/commit/5d5ca97ea2f2a913e8a21e59db040b911a058b6d. Let me know if you consider them worth another PR :sweat_smile:

However, these changes don't directly solve the root problem since ryuk port is hardcoded at the moment, and I wasn't brave enough to change it to 0.0.0.0::8080 instead of ::8080

rvem commented 3 months ago

Yeah, but I considered avoiding Text -> IP -> Text -> IP conversions

OTOH, it's quite hard to avoid them, we do Text -> IP -> Text anyway

alexbiehl commented 2 months ago

Sorry for the delay, I’ll take care of this during zurihac.t

rvem commented 2 months ago

Yeah, sure, no hurry. At the moment I'm ok with using testcontainers-hs from the fork