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

Consider supporting Unix Socket on Windows (starting with RS4) #36442

Open
simonferquel opened this issue Feb 28, 2018 · 28 comments
Open

Consider supporting Unix Socket on Windows (starting with RS4) #36442

simonferquel opened this issue Feb 28, 2018 · 28 comments
Labels
kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. platform/windows

Comments

@simonferquel
Copy link
Contributor

Description

Windows 10 and Server 2016 RS4 (upcoming update to Windows due 18/03) will bring support for Unix Sockets (address family AF_UNIX). If we could support them in Moby, this could bring better feature parity with Linux, and it would make it easier for users of WSL. There are 2 levels of support we can achieve:

Make docker daemon on Windows listen on a Unix socket - the easy stuff

The same way as we do on linux (listening on /var/run/docker.sock), we could listen on c:\ProgramData\Docker\docker.sock (or any file path passed trough the --HOST flag). This would allow WSL users to connect to the daemon without having to expose anything trough TCP. This would also make it easier for admins to secure access to the daemon (using ntfs rights).

This could be done easily entirely in the Moby code-base.

Make Unix socket files bind-mountable in containers - the not so easy one

The goal here is to make it easy to bind mount a Unix Socket in a container (windows or lcow), the same way that we do on Linux. It would for exemple allow to bind mount Docker socket itself inside a container (windows or lcow), like many Linux containers do (and as we do ourself with Docker EE).

This might require some additional work on Windows HCS, but we should ask for confirmation from Microsoft.

@thaJeztah thaJeztah added platform/windows kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. labels Feb 28, 2018
@thaJeztah
Copy link
Member

/cc @darrenstahlmsft @jhowardmsft @johnstep PTAL

@sunilmut
Copy link

@simonferquel - good call. We are already looking into (1) above. But, do note that this requires changes not only in Moby, but also in golang to support unix sockets on Windows, and of course docker CLI.

@darstahl
Copy link
Contributor

FYI @sunilmut works with John Howard and myself on the Linux on Windows team here at Microsoft. He's the go-to for AF_UNIX work both in WSL and Moby 😄

@olljanat
Copy link
Contributor

@darstahl sounds good. Any timeplan?

I understood that AF_UNIX support for Windows it selves was published with 1803 but is there already someone working on to add support golang? (I was not able to find any issue of it).

@gitfool
Copy link

gitfool commented Sep 1, 2018

👍

@glenkim
Copy link

glenkim commented Mar 15, 2019

FYI golang support for unix sockets in windows has been added golang/go#26072

@thaJeztah
Copy link
Member

@jhowardmsft with the above being included in Golang, do you know what would be needed in the code in this repository to make this work?

@simonferquel
Copy link
Contributor Author

@thaJeztah I prototyped something similar for docker desktop (having the docker desktop proxy listen on a unix socket and connect to it from wsl). It is exactly the same code as on Linux/MacOS. If we don't make it the default, we don't even have to do a platform check for that, the golang API just returns an error if it is not supported on you Windows version.

@thaJeztah
Copy link
Member

@simonferquel nice! care to open it as a PR? 🤗 🎉 😇

@simonferquel
Copy link
Contributor Author

I guess I just have to wait for moby/moby to bump to golang 1.12.x :)

@thaJeztah
Copy link
Member

Its in the golang.org/x/sys package

type RawSockaddrUnix struct {
Family uint16
Path [UNIX_PATH_MAX]int8
}

And I see we vendored it in this commit 826da28 (#38216)

@simonferquel
Copy link
Contributor Author

Hmm, the problem is that we need higher level constructs to convert the raw win32 Handle into a net.Conn. I'd prefer to wait for golang 1.12 to be able to just call net.DialUnix on Windows.

@thaJeztah
Copy link
Member

You're in luck; master is now on Go 1.12.1 😄 #38404

@PatrickLang
Copy link

@jhowardmsft and @jterry75 - what do you think? Can this be supported in ContainerD + Moby?

@jterry75
Copy link
Contributor

Yes it can easily. As @simonferquel said, we have to do a version check all over the place but if this was a non-default configuration it would be easier to adopt.

@lowenna
Copy link
Member

lowenna commented Jun 6, 2019

(I would only do RS5+ given the imminent end-of-life for RS4....)

@wzhliang
Copy link
Contributor

Any update on this?

@hmccoll
Copy link

hmccoll commented Jul 29, 2021

Is any one working on this? What aspects are there still to do?

slonopotamus added a commit to slonopotamus/go-connections that referenced this issue Jan 23, 2022
slonopotamus added a commit to slonopotamus/go-connections that referenced this issue Jan 23, 2022
See moby/moby#36442

Signed-off-by: Marat Radchenko <marat@slonopotamus.org>
@slonopotamus
Copy link
Contributor

See docker/go-connections#98

slonopotamus added a commit to slonopotamus/go-connections that referenced this issue Jan 23, 2022
See moby/moby#36442

Signed-off-by: Marat Radchenko <marat@slonopotamus.org>
slonopotamus added a commit to slonopotamus/go-connections that referenced this issue Jan 24, 2022
See moby/moby#36442

Signed-off-by: Marat Radchenko <marat@slonopotamus.org>
slonopotamus added a commit to slonopotamus/go-connections that referenced this issue Jan 26, 2022
See moby/moby#36442

Signed-off-by: Marat Radchenko <marat@slonopotamus.org>
slonopotamus added a commit to slonopotamus/go-connections that referenced this issue Jan 26, 2022
See moby/moby#36442

Signed-off-by: Marat Radchenko <marat@slonopotamus.org>
slonopotamus added a commit to slonopotamus/go-connections that referenced this issue Aug 16, 2022
See moby/moby#36442

Signed-off-by: Marat Radchenko <marat@slonopotamus.org>
@saracen
Copy link

saracen commented Jul 12, 2023

@AkihiroSuda @thaJeztah GitLab Runner is interested in this because we want to support tunnelling over SSH and connecting to the docker unix socket (as SSH cannot tunnel to npipe).

This appears to be stuck here: docker/go-connections#98.

Is there anything I can do to get this moving again? Thank you.

@neersighted
Copy link
Member

There are a variety of solutions you can use, including the built in docker system dial-stdio for that case (as well as the generic usual suspects: netcat, socat, a custom program), as at the end of the day it's just a byte stream and the underlying transport is mostly an OS-level concern.

That being said, I would also like to see this. I believe this is mostly stuck due to the very poor state of go-connections; in general I think we're open to/interested in the idea of eliminating the package and moving the socket functionality into the daemon and the client library directly. If you're interested in attempting a PR to increase the maintainability/reduce the indirection required, feel free.

Otherwise, I'll try to bring this up in our next maintainer's call to see what people's thoughts are re: the state of the existing code and what might be needed to get Unix-socket-on-Windows support in, one way or another.

@saracen
Copy link

saracen commented Jul 12, 2023

@neersighted The PR for adding unix support to go-connections seems fairly simple.

Whilst the package is not in an ideal state, I'm not sure a refactor should be required before this support lands. Unless there's other changes in this package and the update seems risky?

I wouldn't mind contributing to eliminating the separate package as I think that's a good direction, but feel like maybe this could come later. I'll let you and other maintainers have a think before I do that though. Thank you!

@slonopotamus
Copy link
Contributor

As an author of docker/go-connections#98, I want to say. You've already spent more time discussing this topic than required to MERGE THE F*CKING THING. Especially given the fact that it was already reviewed and approved.

@neersighted
Copy link
Member

I'm sorry to hear you're frustrated; my current understanding of the state of go-connections is that things are rotted/decayed enough that there is an extreme aversion to adding anything new to it/making changes that are not provable, self-contained bug fixes.

I personally would like to see this get in, but I also understand the reluctance to perturb that package from those who have been maintaining this project far longer than I have. Like I said, I intend to bring it up and see if we can get buy-in on just merging the PR as-is, or seriously looking at replacing go-connections.

This is becoming more and more relevant due to the bitrot accelerating, e.g. #45935 is not caused by go-connections, but it is made a lot harder to reason about/indirectly a side-effect of go-connections.

@slonopotamus
Copy link
Contributor

slonopotamus commented Jul 12, 2023

There isn't much of a decay. The tests are currently broken, but docker/go-connections#100 (half a year old!) fixes them back.

If we are talking in context of npipe/AF_UNIX: you're right, go-connections is not needed, it is a redundant level of abstraction. Ideally, moby throws away all this npipe stuff and just uses AF_UNIX across all platforms.

Overall, I also agree, there isn't much value in go-connections as a separate package. Standard Go's net.* stuff covers almost all needs. If moby wants to build a bit upon that, it could easily incorporate types/functions into intself, it wouldn't be much of a burden.

@neersighted
Copy link
Member

The decay is mostly that go-connections cannot be updated because of other packages -- I need to talk to the other maintainers for more context, but something went wrong in the tlsconfig package that has prevented taking in a commit after the 0.4.0 tag.

@slonopotamus
Copy link
Contributor

That's going out of context of current issue. People want unix sockets on windows. That's trivial, just net.Dial the damn thing and you're awesome. Bringing that tls stuff into the picture is overcomplication.

@neersighted
Copy link
Member

Yes, that is what I am trying to say -- merging a change in go-connections is only a small part of the problem as the code could not be vendored into this repository in the present state. No one has looked at it for a while, and I need to ask the people who did in the past.

go-connections provides little value as an abstraction, and if the TLS issues are as currently intractable as I recall them being, it may ultimately be easier to get rid of the go-connections Unix socket/named pipe abstraction until someone else has the time to earnestly look at fixing the rest of the code in go-connections.

Dependencies/vendoring are all-or-nothing; it's impossible to avoid the tlsconfig changes in go-connections when taking in a new tag. While it's certainly possible to revert the "bad" changes in go-connections, none have been identified yet.

This is why I'm trying to temper expectations -- it's more complex than it should be, but it is, and I don't want to make promises I can't keep. If anyone does find themselves wanting to look at go-connections and trying puzzle out the issues there, you can find some context at:

slonopotamus added a commit to slonopotamus/go-connections that referenced this issue May 7, 2024
See moby/moby#36442

Signed-off-by: Marat Radchenko <marat@slonopotamus.org>
slonopotamus added a commit to slonopotamus/go-connections that referenced this issue May 14, 2024
See moby/moby#36442

Signed-off-by: Marat Radchenko <marat@slonopotamus.org>
slonopotamus added a commit to slonopotamus/go-connections that referenced this issue May 14, 2024
See moby/moby#36442

Signed-off-by: Marat Radchenko <marat@slonopotamus.org>
slonopotamus added a commit to slonopotamus/go-connections that referenced this issue May 14, 2024
See moby/moby#36442

Signed-off-by: Marat Radchenko <marat@slonopotamus.org>
slonopotamus added a commit to slonopotamus/go-connections that referenced this issue May 14, 2024
See moby/moby#36442

Signed-off-by: Marat Radchenko <marat@slonopotamus.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. platform/windows
Projects
None yet
Development

No branches or pull requests