Add support for binding to unix domain sockets #640

Merged

lx merged 6 commits from networkException/garage:unix-sockets into main

2023-10-03 16:23:02 +00:00

networkException commented

2023-09-29 16:53:39 +00:00

Contributor

This pull request adds support for most apis (except rpc) to bind to a unix domain socket instead of a tcp socket.

Resolves #253

This pull request adds support for most apis (except rpc) to bind to a unix domain socket instead of a tcp socket. Resolves #253

👍 2

networkException added 3 commits 2023-09-29 16:53:40 +00:00

6086a3fa07

cargo: add hyperlocal as a dependency

10195f1567

util: add helper sum type for unix and tcp socket addresses

this patch introduces a new sum type that can represent either a
tcp socket address or a unix domain socket path.

7353038a64

config: allow using paths for unix domain sockets in various places

this patch updates the config format to also allow paths in bind
addresses for unix domain sockets.

this has been added to all apis except rpc.

networkException force-pushed unix-sockets from 80be6649f7 to 8ec6a53b35

2023-09-29 16:57:59 +00:00

Compare

lx commented

2023-10-03 08:19:19 +00:00

Owner

Great work, thank you very much! Could you add relevant documentation in the reference section for the config file? (doc/book/reference-manual/configuration.md)

networkException added 1 commit 2023-10-03 15:36:53 +00:00

7907a09acc

api: allow custom unix bind mode and use 0o220 for admin server

networkException commented

2023-10-03 15:37:38 +00:00

Author

Contributor

Added some documentation :)

Should I also mention which version the feature was introduced in? Which version would will that be?

Added some documentation :) Should I also mention which version the feature was introduced in? Which version would will that be?

lx commented

2023-10-03 15:44:23 +00:00

Owner

I think there is a mistake, UNIX sockets are still not supported for rpc_bind_addr, right?

Should I also mention which version the feature was introduced in? Which version would will that be?

You can specify v0.8.5

I think there is a mistake, UNIX sockets are still not supported for `rpc_bind_addr`, right? > Should I also mention which version the feature was introduced in? Which version would will that be? You can specify v0.8.5

networkException commented

2023-10-03 15:51:23 +00:00

Author

Contributor

I think there is a mistake, UNIX sockets are still not supported for rpc_bind_addr, right?

That can easily be added. My assumption was that unix sockets are not desiderable for cluster communication, as they are scoped to a single machine / file system.

> I think there is a mistake, UNIX sockets are still not supported for `rpc_bind_addr`, right? That can easily be added. My assumption was that unix sockets are not desiderable for cluster communication, as they are scoped to a single machine / file system.

networkException force-pushed unix-sockets from f213dffd89 to dec575720a

2023-10-03 15:53:21 +00:00

Compare

networkException force-pushed unix-sockets from dec575720a to db193cd6df

2023-10-03 15:54:04 +00:00

Compare

lx commented

2023-10-03 15:54:24 +00:00

Owner

Yes that is correct, I don't feel that it is necessary to add them for RPC communication. And it would be a bit trickier to do as it would involve modifying the netapp crate as well.

My comment was just wrt your change on the config reference documentation page, you added the line saying that UNIX sockets are supported for rpc_bind_addr, which is not the case.

Yes that is correct, I don't feel that it is necessary to add them for RPC communication. And it would be a bit trickier to do as it would involve modifying the `netapp` crate as well. My comment was just wrt your change on the config reference documentation page, you added the line saying that UNIX sockets are supported for `rpc_bind_addr`, which is not the case.

networkException force-pushed unix-sockets from db193cd6df to 6f8a87814b

2023-10-03 15:56:47 +00:00

Compare

networkException commented

2023-10-03 15:57:13 +00:00

Author

Contributor

Oh yes sorry, that was a mistake