fix: passing the adapter_opts properly (#431)

* fix: passing the adapter_opts properly

* add validation for :cred option in :adapter_opts

* make cred_opts accept only adapter_opts

* format the server/adapters/cowboy.ex file

* fix tests by replacing opts with adapter_opts in Server.start

---------

Co-authored-by: Xin Guan <[email protected]>

Author: xinguanz

lib/grpc/server/adapters/cowboy.ex

@@ -30,8 +30,10 @@ defmodule GRPC.Server.Adapters.Cowboy do
   """
   @impl true
   def start(endpoint, servers, port, opts) do
-    start_args = cowboy_start_args(endpoint, servers, port, opts)
-    start_func = if opts[:cred], do: :start_tls, else: :start_clear
+    [_ref, _trans_opts, proto_opts] =
+      start_args = cowboy_start_args(endpoint, servers, port, opts)
+
+    start_func = if cred_opts(proto_opts), do: :start_tls, else: :start_clear
 
     case apply(:cowboy, start_func, start_args) do
       {:ok, pid} ->
@@ -52,8 +54,10 @@ defmodule GRPC.Server.Adapters.Cowboy do
     [ref, trans_opts, proto_opts] = cowboy_start_args(endpoint, servers, port, opts)
     trans_opts = Map.put(trans_opts, :connection_type, :supervisor)
 
+    cred_opts = cred_opts(proto_opts)
+
     {transport, protocol} =
-      if opts[:cred] do
+      if cred_opts do
         {:ranch_ssl, :cowboy_tls}
       else
         {:ranch_tcp, :cowboy_clear}
@@ -63,7 +67,7 @@ defmodule GRPC.Server.Adapters.Cowboy do
     # So we just support both child spec versions here instead
     case :ranch.child_spec(ref, transport, trans_opts, protocol, proto_opts) do
       {ref, mfa, type, timeout, kind, modules} ->
-        scheme = if opts[:cred], do: :https, else: :http
+        scheme = if cred_opts, do: :https, else: :http
         # Wrap real mfa to print starting log
         wrapped_mfa = {__MODULE__, :start_link, [scheme, endpoint, servers, mfa]}
 
@@ -226,12 +230,12 @@ defmodule GRPC.Server.Adapters.Cowboy do
     dispatch_key = Module.concat(endpoint, Dispatch)
     :persistent_term.put(dispatch_key, dispatch)
 
-    idle_timeout = Keyword.get(opts, :idle_timeout) || :infinity
-    num_acceptors = Keyword.get(opts, :num_acceptors) || @default_num_acceptors
-    max_connections = Keyword.get(opts, :max_connections) || @default_max_connections
+    idle_timeout = Keyword.get(adapter_opts, :idle_timeout) || :infinity
+    num_acceptors = Keyword.get(adapter_opts, :num_acceptors) || @default_num_acceptors
+    max_connections = Keyword.get(adapter_opts, :max_connections) || @default_max_connections
 
     # https://ninenines.eu/docs/en/cowboy/2.7/manual/cowboy_http2/
-    opts =
+    merged_adapter_opts =
       Map.merge(
         %{
           env: %{dispatch: {:persistent_term, dispatch_key}},
@@ -245,17 +249,17 @@ defmodule GRPC.Server.Adapters.Cowboy do
           max_received_frame_rate: {10_000_000, 10_000},
           max_reset_stream_rate: {10_000, 10_000}
         },
-        Enum.into(opts, %{})
+        Enum.into(adapter_opts, %{})
       )
 
     [
       servers_name(endpoint, servers),
       %{
         num_acceptors: num_acceptors,
         max_connections: max_connections,
-        socket_opts: socket_opts(port, opts)
+        socket_opts: socket_opts(port, adapter_opts)
       },
-      opts
+      merged_adapter_opts
     ]
   end
 
@@ -270,8 +274,10 @@ defmodule GRPC.Server.Adapters.Cowboy do
         _, acc -> acc
       end)
 
-    if opts[:cred] do
-      opts[:cred].ssl ++
+    cred_opts = cred_opts(opts)
+
+    if cred_opts do
+      cred_opts.ssl ++
         [
           # These NPN/ALPN options are hardcoded in :cowboy.start_tls/3 (when calling start/3),
           # but not in :ranch.child_spec/5 (when calling child_spec/3). We must make sure they
@@ -285,6 +291,10 @@ defmodule GRPC.Server.Adapters.Cowboy do
     end
   end
 
+  defp cred_opts(opts) do
+    Kernel.get_in(opts, [:cred])
+  end
+
   defp running_info(scheme, endpoint, servers, ref) do
     {addr, port} = :ranch.get_addr(ref)
 

lib/grpc/server/supervisor.ex

@@ -68,9 +68,14 @@ defmodule GRPC.Server.Supervisor do
 
         {:error, _} ->
           raise ArgumentError,
-                "just [:endpoint, :servers, :start_server, :port,] are accepted as arguments, and any other keys for adapters should be passed as adapter_opts!"
+                "just [:endpoint, :servers, :start_server, :port, :adapter_opts] are accepted as arguments, and any other keys for adapters should be passed as adapter_opts!"
       end
 
+    case validate_cred(opts) do
+      {:ok, _cred} -> :ok
+      {:error, err} -> raise ArgumentError, err
+    end
+
     endpoint_or_servers =
       case {opts[:endpoint], opts[:servers]} do
         {endpoint, servers}
@@ -136,4 +141,15 @@ defmodule GRPC.Server.Supervisor do
     servers = GRPC.Server.servers_to_map(servers)
     adapter.child_spec(nil, servers, port, opts)
   end
+
+  defp validate_cred(opts) do
+    with cred <- Kernel.get_in(opts, [:adapter_opts, :cred]),
+         true <- cred == nil or (is_map(cred) and is_list(Map.get(cred, :ssl))) do
+      {:ok, cred}
+    else
+      _ ->
+        {:error,
+         "the :cred option must be a map with an :ssl key containing a list of SSL options"}
+    end
+  end
 end

test/grpc/client/adapters/gun_test.exs

@@ -6,7 +6,9 @@ defmodule GRPC.Client.Adapters.GunTest do
   describe "connect/2" do
     setup do
       server_credential = build(:credential)
-      {:ok, _, port} = GRPC.Server.start(FeatureServer, 0, cred: server_credential)
+
+      {:ok, _, port} =
+        GRPC.Server.start(FeatureServer, 0, adapter_opts: [cred: server_credential])
 
       on_exit(fn ->
         :ok = GRPC.Server.stop(FeatureServer)

test/grpc/integration/connection_test.exs

@@ -18,7 +18,7 @@ defmodule GRPC.Integration.ConnectionTest do
     server = FeatureServer
     File.rm(socket_path)
 
-    {:ok, _, _} = GRPC.Server.start(server, 0, ip: {:local, socket_path})
+    {:ok, _, _} = GRPC.Server.start(server, 0, adapter_opts: [ip: {:local, socket_path}])
     {:ok, channel} = GRPC.Stub.connect(socket_path, adapter_opts: [retry_timeout: 10])
 
     point = %Routeguide.Point{latitude: 409_146_138, longitude: -746_188_906}
@@ -31,7 +31,7 @@ defmodule GRPC.Integration.ConnectionTest do
 
     cred = GRPC.Factory.build(:credential, verify: :verify_peer)
 
-    {:ok, _, port} = GRPC.Server.start(server, 0, cred: cred)
+    {:ok, _, port} = GRPC.Server.start(server, 0, adapter_opts: [cred: cred])
 
     try do
       point = %Routeguide.Point{latitude: 409_146_138, longitude: -746_188_906}

test/grpc/server/adapters/cowboy_test.exs

@@ -7,11 +7,14 @@ defmodule GRPC.Server.Adapters.CowboyTest do
     test "produces the correct socket opts for ranch_tcp for inet" do
       spec =
         Cowboy.child_spec(:endpoint, [], 8080, [
-          {:foo, :bar},
-          {:ip, {127, 0, 0, 1}},
-          {:ipv6_v6only, false},
-          {:net, :inet},
-          {:baz, :foo}
+          {:adapter_opts,
+           [
+             {:foo, :bar},
+             {:ip, {127, 0, 0, 1}},
+             {:ipv6_v6only, false},
+             {:net, :inet},
+             {:baz, :foo}
+           ]}
         ])
 
       socket_opts = get_socket_opts_from_child_spec(spec)
@@ -23,11 +26,14 @@ defmodule GRPC.Server.Adapters.CowboyTest do
     test "produces the correct socket opts for ranch_tcp for inet6" do
       spec =
         Cowboy.child_spec(:endpoint, [], 8081, [
-          {:foo, :bar},
-          {:ip, {0, 0, 0, 0, 0, 0, 0, 1}},
-          {:ipv6_v6only, true},
-          {:net, :inet6},
-          {:baz, :foo}
+          {:adapter_opts,
+           [
+             {:foo, :bar},
+             {:ip, {0, 0, 0, 0, 0, 0, 0, 1}},
+             {:ipv6_v6only, true},
+             {:net, :inet6},
+             {:baz, :foo}
+           ]}
         ])
 
       socket_opts = get_socket_opts_from_child_spec(spec)