Fix IPv6 support and support listening to a specific IP #2226
Conversation
Reorder build steps such that intermediate steps can more likely be reused, in particular installed apt, npm and mix packages.
config/runtime.exs: new variable PHX_HTTP_IP for specifying IP address to bind to; Logflare.Repo, :postgres_backend_adapter: set socket-options for IPv6; SharedRepo, ContextCache.Supervisor: hand socket-options on; update docs; update .template.env;
|
Thanks for the PR, looks like a good addition for self hosting setup |
runtime.exs: adapt logic from supabase/realtime:Realtime.Database.detect_ip_version/1; use :inet.parse_address/1 instead of IP.from_string!/1; shared_repo.ex: apply code-review suggestion;
| socket_options_for_url = fn url when is_binary(url) -> | ||
| case URI.parse(url) do | ||
| %URI{host: host} -> socket_options_for_host.(host) | ||
| _ -> raise "Failed to parse URL: #{inspect(url)}" | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
| socket_options_for_url = fn url when is_binary(url) -> | |
| case URI.parse(url) do | |
| %URI{host: host} -> socket_options_for_host.(host) | |
| _ -> raise "Failed to parse URL: #{inspect(url)}" | |
| end | |
| end | |
| socket_options_for_url = fn | |
| url when is_binary(url) -> | |
| case URI.parse(url) do | |
| %URI{host: host} -> socket_options_for_host.(host) | |
| _ -> raise "Failed to parse URL: #{inspect(url)}" | |
| end | |
| _url -> nil | |
| end |
This will give an error without a fallback clause
There was a problem hiding this comment.
we can also group all the anonymous functions together by moving this function up so that they are easier to maintain and grok.
There was a problem hiding this comment.
As I am an Elixir newbie, I don't fully understand the fallback-clause. Shouldn't the nil-case be excluded by the when is_binary() clause? Why can a fallback cover for it in this "overload"?
| detect_ip_version = fn host when is_binary(host) -> | ||
| host = String.to_charlist(host) | ||
|
|
||
| cond do | ||
| match?({:ok, _}, :inet6_tcp.getaddr(host)) -> {:ok, :inet6} | ||
| match?({:ok, _}, :inet.gethostbyname(host)) -> {:ok, :inet} | ||
| true -> {:error, :nxdomain} | ||
| end | ||
| end | ||
|
|
||
| socket_options_for_host = fn host when is_binary(host) -> | ||
| case detect_ip_version.(host) do | ||
| {:ok, ip_version} -> [ip_version] | ||
| {:error, reason} -> raise "Failed to detect IP version: #{reason}" | ||
| end | ||
| end |
There was a problem hiding this comment.
| detect_ip_version = fn host when is_binary(host) -> | |
| host = String.to_charlist(host) | |
| cond do | |
| match?({:ok, _}, :inet6_tcp.getaddr(host)) -> {:ok, :inet6} | |
| match?({:ok, _}, :inet.gethostbyname(host)) -> {:ok, :inet} | |
| true -> {:error, :nxdomain} | |
| end | |
| end | |
| socket_options_for_host = fn host when is_binary(host) -> | |
| case detect_ip_version.(host) do | |
| {:ok, ip_version} -> [ip_version] | |
| {:error, reason} -> raise "Failed to detect IP version: #{reason}" | |
| end | |
| end | |
| detect_ip_version = fn host -> | |
| host = String.to_charlist(host) | |
| cond do | |
| match?({:ok, _}, :inet6_tcp.getaddr(host)) -> {:ok, :inet6} | |
| match?({:ok, _}, :inet.gethostbyname(host)) -> {:ok, :inet} | |
| true -> {:error, :nxdomain} | |
| end | |
| end | |
| socket_options_for_host = fn | |
| host when is_binary(host) -> | |
| case detect_ip_version.(host) do | |
| {:ok, ip_version} -> [ip_version] | |
| {:error, reason} -> raise "Failed to detect IP version: #{reason}" | |
| end | |
| _host -> [] | |
| end |
Needs fallback option. nested anon function then doesn't need the guard anymore if we do the check at the top level.
| @@ -45,6 +45,8 @@ defmodule Logflare.Backends.Adaptor.PostgresAdaptor.SharedRepo do | |||
| opts ++ fields | |||
| end | |||
|
|
|||
| opts = opts ++ [socket_options: config[:socket_options]] | |||
There was a problem hiding this comment.
@tvogel this shouldn't use the global config actually, we can add this in temporarily but it should be part of the backend's config instead. This would require adjusting the self-hosted backend config building.
There was a problem hiding this comment.
Why shouldn't it use the socket_options determined for the respective DB_HOSTNAME which it determines in
Map.take(config, [:username, :password, :hostname, :database, :port, :ssl])
? In which cases would you use different options?
| @@ -45,6 +45,8 @@ defmodule Logflare.Backends.Adaptor.PostgresAdaptor.SharedRepo do | |||
| opts ++ fields | |||
| end | |||
|
|
|||
| opts = opts ++ [socket_options: config[:socket_options]] | |||
There was a problem hiding this comment.
| opts = opts ++ [socket_options: config[:socket_options]] | |
| if Logflare.SingleTenant.single_tenant?() do | |
| opts = opts ++ [socket_options: config[:socket_options]] | |
| else | |
| opts | |
| end |
lets do itonly for single tenant for now
Co-authored-by: Ziinc <Ziinc@users.noreply.github.com>
|
(Sorry for the late replies - I was traveling.) |
For running logflare on fly.io with their IPv6 based private networking, I had to fix/improve a few things. Please take a look and pick what you like!
(My first exposure to Elixir/Erlang, so thanks for that!)