Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix linux docker proxy #7878

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

matejkramny
Copy link

@matejkramny matejkramny commented Dec 5, 2024

Fixes #3239

I added some logging to the proxy and observed the /attach API call converting to a websocket. This websocket is streaming data to/from the container.
I suspected the issue stemming from nonstandard docker behaviour when it comes to websockets, as one of the pipes gets closed when the container does not have stdin open.

The docker client knows the container spec and when stdin on the container is closed, outbound pipe from the client is EOF whilst the inbound pipe (stdout/err from container) is streaming data.

The issue can be worked around if you set DOCKER_HOST to /mnt/wsl/rancher-desktop/run/docker.sock effectively isolating the issue to the wsl-helper proxy.

The PR is proof of concept - starts a proxy on the TCP level, and conditionally sends connections to the original proxy capable of munging the request data etc.

To verify fix, run

go build -o ./wsl-helper ./ && sudo ./wsl-helper docker-proxy serve --verbose --endpoint unix:///tmp/docker-wsl.sock --proxy-endpoint /mnt/wsl/rancher-desktop/run/docker.sock
export DOCKER_HOST=unix:///tmp/docker-wsl.sock
docker run --rm -p 8888:80 nginx

used https://github.com/jpillora/go-tcp-proxy
and https://github.com/docker/go-connections/blob/master/sockets/inmem_socket.go

@matejkramny
Copy link
Author

matejkramny commented Dec 5, 2024

image

screenshot shows docker cli closing one of the streams when stdin isn't enabled on the container

https://github.com/docker/cli/blob/master/cli/command/container/hijack.go#L132
from https://github.com/docker/cli/blob/master/cli/command/container/attach.go#L150

@matejkramny matejkramny marked this pull request as ready for review December 10, 2024 13:00
@matejkramny matejkramny changed the title [wip] Fix linux docker stdin Fix linux docker proxy Dec 10, 2024
@matejkramny
Copy link
Author

Go's httputil.ReverseProxy is not capable of proxying "half closed" tcp connections, ref linked issues starting here:

golang/go#70754

We can bypass the http proxying by going down to L4 and starting a tcp proxy between the two sockets and conditionally forward the connection to L7 when the request matches the criteria.

@matejkramny matejkramny force-pushed the fix-linux-docker-stdin branch 3 times, most recently from 7f079f7 to 2e4d09e Compare December 10, 2024 13:30
@matejkramny
Copy link
Author

Possibly fixes #2094

@jandubois would you be able to review?

Thank you

@gunamata gunamata requested a review from Nino-K December 10, 2024 17:59
Signed-off-by: Matej Kramny <git@matej.me>
@MrWako
Copy link

MrWako commented Jan 2, 2025

Great analysis @matejkramny - think it might be worth trying to get back a fix to the Go reverse proxy in the first instance, something like: golang/go#35892.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Standard output and Standard error are not being written unless docker is invoked with -i.
2 participants