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

support unix domain sockets as host in grpc_client #64

Merged
merged 2 commits into from
Nov 11, 2023

Conversation

PleasantMachine9
Copy link
Contributor

@PleasantMachine9 PleasantMachine9 commented Dec 4, 2021

This allows communication with a grpc server that is listening
on the HTTP/2-over-UDS scheme.

UDS sockets compared with loopback TCP:

  • UDS needs no loopback IP interface
  • UDS can use filesystem-based access control
  • UDS has no TCP/IP stack encapsulation overhead
    (higher throughput, less CPU use depending on use-case)
  • For UDS, the HTTP/2 pseudo-header :authority needs a workaround,
    as the hostname is not a string but a tuple, and it may contain
    nul or slash characters which are not valid to send as values.
    The grpc-go client implementation sets the :authority to
    the hardcoded string "localhost", so that behavior is followed,
    see client: set auth header to localhost for unix target grpc/grpc-go#3730 and support unix-abstract schema grpc/grpc-go#4079

If the {local, UnixSockPath} tuple is passed along
from grpcbox to chatterbox, then the functionality
is working well, so there is not much impact on grpcbox aside
from updating the :authority field appropriately.

Note: the UDS usage is not documented in chatterbox,
their spec says that the host must be type string()
but it causes no issues with their library if that is not the case,
as they just pass the value through to gen_tcp
which supports the unix socket notation.

This allows communication with a grpc server that is listening
on the HTTP/2-over-UDS scheme.

UDS sockets compared with loopback TCP:
* UDS needs no loopback IP interface
* UDS can use filesystem-based access control
* UDS has no TCP/IP stack encapsulation overhead
  (higher throughput, less CPU use depending on use-case)
* For UDS, the HTTP/2 pseudo-header `:authority` needs a workaround,
  as the hostname is not a string but a tuple, and it may contain
  nul or slash characters which are not valid to send as values.
  The grpc-go client implementation sets the `:authority` to
  the hardcoded string `"localhost"`, so that behavior is followed,
  see grpc/grpc-go#3730

If the `{local, UnixSockPath}` tuple is passed along
from grpcbox to chatterbox, then the functionality
is working well, so there is not much impact on grpcbox aside
from updating the `:authority` field appropriately.

Note: the UDS usage is not documented in `chatterbox`,
their spec says that the host must be type `string()`
but it causes no issues with their library if that is not the case,
as they just pass the value through to `gen_tcp`
which supports the unix socket notation.
@@ -9,7 +9,7 @@
acceptor_pool,
gproc,
ctx]},
{env, [{client, #{channels => [%% {default_channel, round_robin, [{http, "localhost", 8080, []}], #{}}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed this example, the round_robin option should have been inside the last argument (#{}) but it is the default value anyway so just removed it.

@PleasantMachine9
Copy link
Contributor Author

Testing the grpcbox client

I used the grpc/examples/python/helloworld:

Result:

1> helloworld_greeter_client:say_hello(#{name => "erlang"}).
{ok,#{message => <<"Hello, erlang!">>},
    #{headers =>
          #{<<":status">> => <<"200">>,
            <<"accept-encoding">> => <<"identity,gzip">>,
            <<"grpc-accept-encoding">> => <<"identity,deflate,gzip">>},
      trailers => #{}}}

@codecov
Copy link

codecov bot commented Dec 5, 2021

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (1d044a3) 39.53% compared to head (0aefc23) 39.48%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #64      +/-   ##
==========================================
- Coverage   39.53%   39.48%   -0.06%     
==========================================
  Files          28       28              
  Lines        2122     2125       +3     
==========================================
  Hits          839      839              
- Misses       1283     1286       +3     
Files Coverage Δ
src/grpcbox_subchannel.erl 54.54% <0.00%> (-4.00%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@PleasantMachine9
Copy link
Contributor Author

src/grpcbox_subchannel.erl

Since the server side does not work with unix sockets (in this PR), not sure how this could be tested in-repo.

@tsloughter
Copy link
Owner

Nice! This was actually on my todo list :) Will review soon.

@PleasantMachine9
Copy link
Contributor Author

PleasantMachine9 commented Dec 23, 2021

On topic of testing difficulties in-repo, I did make an experimental commit for the server-side support too out of curiosity, but it seems quite a bit more complicated than client side because socket listener options are very different compared to TCP/IP. Notably

  • anything referencing "ip" is not applicable and so has to be made optional (and handled when they are undefined)
  • gen_tcp is told to listen on UDS by the special socket option ifaddr with the tuple {ifaddr, {local, UnixPath}}, as opposed to the current option {ip, IpAddr}

In any case, for viewing, that experiment is at PleasantMachine9@7c33d16 but I think it would be a separate PR anyhow.

@tsloughter tsloughter merged commit 593a10c into tsloughter:main Nov 11, 2023
4 of 6 checks passed
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.

2 participants