More ergonomic support for using a unix socket with a tonic server

[agreen17]

Adds tonic::transport::server::UdsConnectInfo to facilitate consumers using a unix domain socket on the server side. Updates the existing uds example to use this UdsConnectInfo and tokio's UnixListenerStream as the latter cuts down on consumer boilerplate. Adds an integration test for UdsConnectInfo.

Motivation

Currently using a unix socket requires consumers to provide an implementation of the tonic::transport::server::Connected trait (UdsConnectInfo).

When I started using tonic, I copy+pasted this stuff from the uds example into my application. I bet other users have done this as well, and while this is not the end of the world, it would be nice if unix sockets worked out of the box with a tonic server.

This PR aims to allow consumers to more easily use a tokio::net::UnixListener.

Solution

The UdsConnectInfo implementation here is pulled directly from the existing uds example. I pulled UnixListenerStream into the uds example to remove some boilerplate code there.

[agreen17]

Unsure about the second commit actually, as tonic probably aims to support old-ish rust toolchains and the change in question was introduced in 1.52.0 🤔 . Maybe I should remove that one

Edit: removed

[Spferical]

Unsure about the second commit actually, as tonic probably aims to support old-ish rust toolchains and the change in question was introduced in 1.52.0 thinking . Maybe I should remove that one

Edit: removed

Tonic readme says 1.56 is required for 2021 edition support, so you can probably go wild with whatever your idea was.

[Spferical]

should these be pub?

[agreen17]

Uhhh, this made me realize I'm being a bonehead here 🤦🏽 Stay tuned

[agreen17]

@Spferical Well, this made me dig in pretty deep here and I realized that the case for unix sockets is a bit weird. It doesn't seem to matter whether or not the tls feature is disabled or not - the examples use the tls feature while the integration tests do not - but in both cases the examples/tests work as expected.

I'm not sure if the tonic folks would say:

1. 🤷🏽 "Yeah, messy, whatever, it works"
2. 👎🏽 MESSY, let's create some new methods serve_with_incoming_unix/serve_with_incoming_shutdown_unix/etc etc
3. something else?

Because of this, I'm going to close this PR and start a conversation with the tonic folks based off my findings for some advice on how to proceed/if they're willing to accept the patch.

[agreen17]

@Spferical Still not really sure on the above since there was no knee-jerk reaction from the tonic maintainer who said they'd accept a PR for this.

I'm thinking I just fire off something like this to upstream tonic and see what they think. It's totally possible that this is fine as-is and we don't need to make a distinction between tcp and unix sockets at the top level. I think I'd rather leave as-is and see what they have to say (as opposed to making some large change that may or may not end up being necessary). Thoughts?

[agreen17]

Re-opening stuff from the previous PR as one of the tonic guys said they would accept a PR for this.

[agreen17]

agreen @ tuan-lappy ~/repo/tonic (agreen/add-unix-socket-support-in-server)
└─ $ ▶ cargo test --all
.
.
running 2 tests
test unix::getting_connect_info ... ok
test getting_connect_info ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.10s
.
.
.
agreen @ tuan-lappy ~/repo/tonic (agreen/add-unix-socket-support-in-server)
└─ $ ▶ echo $?
0

[Spferical]

How easy would it be to make Endpoint::try_from("unix:///tmp/uds-integration-test) work without the fake url/custom connector? Probably fine to punt.

[agreen17]

Seems like this would get a bit hairy as implementing something in Endpoint would bubble back up though tonic::transport::Channel and tonic::transport::service::Connection and we'd have to find some way to handle an optional URI inside an Endpoint through those (and update tests, etc). Seems like more trouble than it's worth considering this only saves consumers 1 LOC (although I do realize it is awkward)