Improve suma authentication concurrency

config/config.exs

@@ -191,6 +191,9 @@ config :trento, Trento.Vault,
 config :trento, Trento.SoftwareUpdates.Discovery,
   adapter: Trento.Infrastructure.SoftwareUpdates.MockSuma
 
+config :trento, Trento.Infrastructure.SoftwareUpdates.Suma,
+  auth: Trento.Infrastructure.SoftwareUpdates.Auth.SumaAuth
+
 config :trento, Trento.Infrastructure.SoftwareUpdates.MockSuma, relevant_patches: %{}
 
 config :trento, Trento.Infrastructure.SoftwareUpdates.SumaApi,

lib/trento/application.ex

@@ -25,7 +25,7 @@ defmodule Trento.Application do
         Trento.Infrastructure.Messaging.Adapter.AMQP.Publisher,
         Trento.Infrastructure.Checks.AMQP.Consumer,
         Trento.Vault,
-        Trento.Infrastructure.SoftwareUpdates.Suma,
+        Trento.Infrastructure.SoftwareUpdates.Auth.SumaAuth,
         {Task.Supervisor, name: Trento.TasksSupervisor}
         # Start a worker by calling: Trento.Worker.start_link(arg)
         # {Trento.Worker, arg}

lib/trento/infrastructure/software_updates/auth/gen.ex

@@ -0,0 +1,11 @@
+defmodule Trento.Infrastructure.SoftwareUpdates.Auth.Gen do
+  @moduledoc """
+  Behaviour of the SUMA authentication process.
+  """
+
+  alias Trento.Infrastructure.SoftwareUpdates.Auth.State
+
+  @callback authenticate() :: {:ok, %State{}} | {:error, any()}
+
+  @callback clear() :: :ok
+end

lib/trento/infrastructure/software_updates/state.ex → lib/trento/infrastructure/software_updates/auth/state.ex

@@ -1,4 +1,4 @@
-defmodule Trento.Infrastructure.SoftwareUpdates.Suma.State do
+defmodule Trento.Infrastructure.SoftwareUpdates.Auth.State do
   @moduledoc """
   State for the SUMA Software Updates discovery adapter
   """

lib/trento/infrastructure/software_updates/auth/suma_auth.ex

@@ -0,0 +1,112 @@
+defmodule Trento.Infrastructure.SoftwareUpdates.Auth.SumaAuth do
+  @moduledoc """
+  GenServer module to authenticate with SUMA
+  """
+
+  @behaviour Trento.Infrastructure.SoftwareUpdates.Auth.Gen
+
+  use GenServer, restart: :transient
+
+  alias Trento.Infrastructure.SoftwareUpdates.Auth.State
+  alias Trento.Infrastructure.SoftwareUpdates.SumaApi
+  alias Trento.SoftwareUpdates
+
+  @default_name "suma_authentication"
+
+  def start_link([]), do: start_link(@default_name)
+
+  def start_link(server_name),
+    do: GenServer.start_link(__MODULE__, %State{}, name: process_identifier(server_name))
+
+  @impl GenServer
+  def init(%State{} = state), do: {:ok, state}
+
+  def identify(server_name \\ @default_name),
+    do:
+      server_name
+      |> identification_tuple
+      |> :global.whereis_name()
+
+  @impl Trento.Infrastructure.SoftwareUpdates.Auth.Gen
+  def authenticate(server_name \\ @default_name),
+    do:
+      server_name
+      |> process_identifier
+      |> call(:authenticate)
+
+  @impl Trento.Infrastructure.SoftwareUpdates.Auth.Gen
+  def clear(server_name \\ @default_name),
+    do:
+      server_name
+      |> process_identifier
+      |> call(:clear)
+
+  @impl GenServer
+  def handle_call(:authenticate, _, %State{} = state) do
+    case setup_auth(state) do
+      {:ok, new_state} ->
+        {:reply, {:ok, new_state}, new_state}
+
+      {:error, _} = error ->
+        {:reply, error, state}
+    end
+  end
+
+  @impl GenServer
+  def handle_call(:clear, _, _), do: {:reply, :ok, %State{}}
+
+  @impl GenServer
+  def format_status(_reason, [pdict, state]) do
+    {:ok,
+     [
+       pdict,
+       %{
+         state
+         | auth: "<REDACTED>",
+           password: "<REDACTED>",
+           ca_cert: "<REDACTED>"
+       }
+     ]}
+  end
+
+  defp call(server, request), do: GenServer.call(server, request, 15_000)
+
+  defp process_identifier(server_name), do: {:global, identification_tuple(server_name)}
+
+  defp identification_tuple(server_name), do: {__MODULE__, server_name}
+
+  defp setup_auth(%State{auth: nil} = state) do
+    with {:ok, %{url: url, username: username, password: password, ca_cert: ca_cert}} <-
+           SoftwareUpdates.get_settings(),
+         :ok <- write_ca_cert_file(ca_cert),
+         {:ok, auth_cookie} <- SumaApi.login(url, username, password, ca_cert != nil) do
+      {:ok,
+       %State{
+         state
+         | url: url,
+           username: username,
+           password: password,
+           ca_cert: ca_cert,
+           auth: auth_cookie,
+           use_ca_cert: ca_cert != nil
+       }}
+    end
+  end
+
+  defp setup_auth(%State{} = state), do: {:ok, state}
+
+  defp write_ca_cert_file(nil) do
+    case File.rm_rf(SumaApi.ca_cert_path()) do
+      {:ok, _} -> :ok
+      _ -> :error
+    end
+  end
+
+  defp write_ca_cert_file(ca_cert) do
+    SumaApi.ca_cert_path()
+    |> Path.dirname()
+    |> File.mkdir_p!()
+
+    File.write(SumaApi.ca_cert_path(), ca_cert)
+  end
+end

lib/trento/infrastructure/software_updates/suma.ex

@@ -5,105 +5,53 @@ defmodule Trento.Infrastructure.SoftwareUpdates.Suma do
 
   @behaviour Trento.SoftwareUpdates.Discovery.Gen
 
-  use GenServer, restart: :transient
-
-  alias Trento.Infrastructure.SoftwareUpdates.{Suma.State, SumaApi}
+  alias Trento.Infrastructure.SoftwareUpdates.Auth.State
   alias Trento.Infrastructure.SoftwareUpdates.SumaApi
-  alias Trento.SoftwareUpdates
-
-  require Logger
-
-  @default_name "suma"
-
-  def start_link([]), do: start_link(@default_name)
 
-  def start_link(server_name),
-    do: GenServer.start_link(__MODULE__, %State{}, name: process_identifier(server_name))
-
-  @impl GenServer
-  def init(%State{} = state), do: {:ok, state}
+  @impl Trento.SoftwareUpdates.Discovery.Gen
+  def setup do
+    case auth().authenticate() do
+      {:error, _} = error ->
+        error
 
-  def identify(server_name \\ @default_name),
-    do:
-      server_name
-      |> identification_tuple
-      |> :global.whereis_name()
+      {:ok, _} ->
+        :ok
+    end
+  end
 
   @impl Trento.SoftwareUpdates.Discovery.Gen
-  def setup(server_name \\ @default_name),
-    do:
-      server_name
-      |> process_identifier
-      |> call(:setup)
+  def clear, do: auth().clear()
 
   @impl Trento.SoftwareUpdates.Discovery.Gen
-  def clear(server_name \\ @default_name),
-    do:
-      server_name
-      |> process_identifier
-      |> call(:clear)
+  def get_system_id(fully_qualified_domain_name),
+    do: handle_request({:get_system_id, fully_qualified_domain_name})
 
   @impl Trento.SoftwareUpdates.Discovery.Gen
-  def get_system_id(fully_qualified_domain_name, server_name \\ @default_name),
-    do:
-      server_name
-      |> process_identifier
-      |> call({:get_system_id, fully_qualified_domain_name})
+  def get_relevant_patches(system_id),
+    do: handle_request({:get_relevant_patches, system_id})
 
   @impl Trento.SoftwareUpdates.Discovery.Gen
-  def get_relevant_patches(system_id, server_name \\ @default_name),
-    do:
-      server_name
-      |> process_identifier
-      |> call({:get_relevant_patches, system_id})
+  def get_upgradable_packages(system_id),
+    do: handle_request({:get_upgradable_packages, system_id})
 
-  @impl Trento.SoftwareUpdates.Discovery.Gen
-  def get_upgradable_packages(system_id, server_name \\ @default_name),
-    do:
-      server_name
-      |> process_identifier
-      |> call({:get_upgradable_packages, system_id})
-
-  @impl GenServer
-  def handle_call(:setup, _from, %State{} = state) do
-    case setup_auth(state) do
+  defp handle_request(request) do
+    case auth().authenticate() do
       {:ok, new_state} ->
-        {:reply, :ok, new_state}
+        request
+        |> do_handle(new_state)
+        |> handle_authentication_error(request)
 
-      {:error, _} = error ->
-        {:reply, error, state}
+      error ->
+        error
     end
   end
 
-  @impl GenServer
-  def handle_call(:clear, _, _), do: {:reply, :ok, %State{}}
-
-  @impl GenServer
-  def handle_call(request, _, %State{auth: nil} = state),
-    do: authenticate_and_handle(request, state)
-
-  @impl GenServer
-  def handle_call(request, _, %State{} = state) do
-    case handle_result = do_handle(request, state) do
-      {:error, :authentication_error} ->
-        authenticate_and_handle(request, state)
-
-      _ ->
-        {:reply, handle_result, state}
-    end
+  defp handle_authentication_error({:error, :authentication_error}, request) do
+    clear()
+    handle_request(request)
   end
 
-  defp call(server, request), do: GenServer.call(server, request, 15_000)
-
-  defp authenticate_and_handle(request, state) do
-    case setup_auth(state) do
-      {:ok, new_state} ->
-        {:reply, do_handle(request, new_state), new_state}
-
-      {:error, _} = error ->
-        {:reply, error, state}
-    end
-  end
+  defp handle_authentication_error(result, _), do: result
 
   defp do_handle({:get_system_id, fully_qualified_domain_name}, %State{
          url: url,
@@ -126,40 +74,5 @@ defmodule Trento.Infrastructure.SoftwareUpdates.Suma do
        }),
        do: SumaApi.get_upgradable_packages(url, auth_cookie, system_id, use_ca_cert)
 
-  defp process_identifier(server_name), do: {:global, identification_tuple(server_name)}
-
-  defp identification_tuple(server_name), do: {__MODULE__, server_name}
-
-  defp setup_auth(%State{} = state) do
-    with {:ok, %{url: url, username: username, password: password, ca_cert: ca_cert}} <-
-           SoftwareUpdates.get_settings(),
-         :ok <- write_ca_cert_file(ca_cert),
-         {:ok, auth_cookie} <- SumaApi.login(url, username, password, ca_cert != nil) do
-      {:ok,
-       %State{
-         state
-         | url: url,
-           username: username,
-           password: password,
-           ca_cert: ca_cert,
-           auth: auth_cookie,
-           use_ca_cert: ca_cert != nil
-       }}
-    end
-  end
-
-  defp write_ca_cert_file(nil) do
-    case File.rm_rf(SumaApi.ca_cert_path()) do
-      {:ok, _} -> :ok
-      _ -> :error
-    end
-  end
-
-  defp write_ca_cert_file(ca_cert) do
-    SumaApi.ca_cert_path()
-    |> Path.dirname()
-    |> File.mkdir_p!()
-
-    File.write(SumaApi.ca_cert_path(), ca_cert)
-  end
+  defp auth, do: Application.fetch_env!(:trento, __MODULE__)[:auth]
 end

lib/trento/software_updates/discovery.ex

@@ -39,11 +39,13 @@ defmodule Trento.SoftwareUpdates.Discovery do
 
   @spec discover_software_updates :: {:ok, {list(), list()}}
   def discover_software_updates do
+    authentication = setup()
+
     {:ok,
      Hosts.get_all_hosts()
      |> ParallelStream.map(fn
        %HostReadModel{id: host_id, fully_qualified_domain_name: fully_qualified_domain_name} ->
-         case discover_host_software_updates(host_id, fully_qualified_domain_name) do
+         case discover_host_software_updates(host_id, fully_qualified_domain_name, authentication) do
            {:error, error} ->
              {:error, host_id, error}
 
@@ -104,6 +106,20 @@ defmodule Trento.SoftwareUpdates.Discovery do
     end
   end
 
+  defp discover_host_software_updates(host_id, _, {:error, error}) do
+    commanded().dispatch(
+      CompleteSoftwareUpdatesDiscovery.new!(%{
+        host_id: host_id,
+        health: SoftwareUpdatesHealth.unknown()
+      })
+    )
+
+    {:error, error}
+  end
+
+  defp discover_host_software_updates(host_id, fully_qualified_domain_name, _),
+    do: discover_host_software_updates(host_id, fully_qualified_domain_name)
+
   defp build_discovery_completion_command(host_id, relevant_patches),
     do:
       CompleteSoftwareUpdatesDiscovery.new!(%{

test/test_helper.exs

@@ -22,6 +22,14 @@ Application.put_env(:trento, Trento.SoftwareUpdates.Discovery,
   adapter: Trento.SoftwareUpdates.Discovery.Mock
 )
 
+Mox.defmock(Trento.Infrastructure.SoftwareUpdates.Auth.Mock,
+  for: Trento.Infrastructure.SoftwareUpdates.Auth.Gen
+)
+
+Application.put_env(:trento, Trento.Infrastructure.SoftwareUpdates.Suma,
+  auth: Trento.Infrastructure.SoftwareUpdates.Auth.Mock
+)
+
 Mox.defmock(Trento.Infrastructure.Messaging.Adapter.Mock,
   for: Trento.Infrastructure.Messaging.Adapter.Gen
 )