Skip to content

Commit

Permalink
Move check for ca_cert != nil from SumaApi to Suma GenServer
Browse files Browse the repository at this point in the history
  • Loading branch information
jamie-suse committed Mar 7, 2024
1 parent 4e8f8d0 commit 99e1bd9
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 25 deletions.
6 changes: 3 additions & 3 deletions lib/trento/infrastructure/software_updates/suma.ex
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,14 @@ defmodule Trento.Infrastructure.SoftwareUpdates.Suma do
auth: auth_cookie,
ca_cert: ca_cert
}),
do: SumaApi.get_system_id(url, auth_cookie, fully_qualified_domain_name, ca_cert)
do: SumaApi.get_system_id(url, auth_cookie, fully_qualified_domain_name, ca_cert != nil)

defp do_handle({:get_relevant_patches, system_id}, %State{
url: url,
auth: auth_cookie,
ca_cert: ca_cert
}),
do: SumaApi.get_relevant_patches(url, auth_cookie, system_id, ca_cert)
do: SumaApi.get_relevant_patches(url, auth_cookie, system_id, ca_cert != nil)

defp process_identifier(server_name), do: {:global, identification_tuple(server_name)}

Expand All @@ -116,7 +116,7 @@ defmodule Trento.Infrastructure.SoftwareUpdates.Suma do
with {:ok, %{url: url, username: username, password: password, ca_cert: ca_cert}} <-
SoftwareUpdates.get_settings(),
:ok <- maybe_write_ca_cert_file(ca_cert),
{:ok, auth_cookie} <- SumaApi.login(url, username, password, ca_cert) do
{:ok, auth_cookie} <- SumaApi.login(url, username, password, ca_cert != nil) do
{:ok,
%State{
state
Expand Down
28 changes: 14 additions & 14 deletions lib/trento/infrastructure/software_updates/suma_api.ex
Original file line number Diff line number Diff line change
Expand Up @@ -16,27 +16,27 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaApi do
url :: String.t(),
username :: String.t(),
password :: String.t(),
ca_cert :: String.t() | nil
use_ca_cert :: boolean()
) ::
{:ok, any()} | {:error, :max_login_retries_reached | any()}
def login(url, username, password, ca_cert),
def login(url, username, password, use_ca_cert),
do:
url
|> get_suma_api_url()
|> try_login(username, password, ca_cert, @login_retries)
|> try_login(username, password, use_ca_cert, @login_retries)

@spec get_system_id(
url :: String.t(),
auth :: any(),
fully_qualified_domain_name :: String.t(),
ca_cert :: String.t() | nil
use_ca_cert :: boolean()
) ::
{:ok, pos_integer()} | {:error, :system_id_not_found | :authentication_error}
def get_system_id(url, auth, fully_qualified_domain_name, ca_cert) do
def get_system_id(url, auth, fully_qualified_domain_name, use_ca_cert) do
response =
url
|> get_suma_api_url()
|> http_executor().get_system_id(auth, fully_qualified_domain_name, ca_cert != nil)
|> http_executor().get_system_id(auth, fully_qualified_domain_name, use_ca_cert)

with {:ok, %HTTPoison.Response{status_code: 200, body: body}} <- response,
{:ok, %{success: true, result: result}} <- Jason.decode(body, keys: :atoms),
Expand All @@ -59,15 +59,15 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaApi do
url :: String.t(),
auth :: any(),
system_id :: pos_integer(),
ca_cert :: String.t() | nil
use_ca_cert :: boolean()
) ::
{:ok, [map()]}
| {:error, :error_getting_patches | :authentication_error}
def get_relevant_patches(url, auth, system_id, ca_cert) do
def get_relevant_patches(url, auth, system_id, use_ca_cert) do
response =
url
|> get_suma_api_url()
|> http_executor().get_relevant_patches(auth, system_id, ca_cert != nil)
|> http_executor().get_relevant_patches(auth, system_id, use_ca_cert)

with {:ok, %HTTPoison.Response{status_code: 200, body: body}} <- response,
{:ok, %{success: true, result: result}} <- Jason.decode(body, keys: :atoms) do
Expand All @@ -94,19 +94,19 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaApi do
{:error, :max_login_retries_reached}
end

defp try_login(url, username, password, ca_cert, retry) do
case do_login(url, username, password, ca_cert) do
defp try_login(url, username, password, use_ca_cert, retry) do
case do_login(url, username, password, use_ca_cert) do
{:ok, _} = successful_login ->
successful_login

{:error, reason} ->
Logger.error("Failed to Log into SUSE Manager, retrying...", error: inspect(reason))
try_login(url, username, password, ca_cert, retry - 1)
try_login(url, username, password, use_ca_cert, retry - 1)
end
end

defp do_login(url, username, password, ca_cert) do
case http_executor().login(url, username, password, ca_cert != nil) do
defp do_login(url, username, password, use_ca_cert) do
case http_executor().login(url, username, password, use_ca_cert) do
{:ok, %HTTPoison.Response{headers: headers, status_code: 200} = response} ->
Logger.debug("Successfully logged into suma #{inspect(response)}")
{:ok, get_session_cookies(headers)}
Expand Down
11 changes: 3 additions & 8 deletions test/trento/infrastructure/software_updates/suma_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,10 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaTest do
end

test "should save existing CA certificate to local file", %{
settings: %Settings{url: url, username: username, password: password, ca_cert: ca_cert}
settings: %Settings{ca_cert: ca_cert}
} do
assert {:ok, _} = start_supervised({Suma, @test_integration_name})

base_api_url = "#{url}/rhn/manager/api"

expect(SumaApiMock, :login, fn _, _, _, true -> successful_login_response() end)

assert :ok = Suma.setup(@test_integration_name)
Expand All @@ -65,14 +63,11 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaTest do
end

test "should not save CA certificate file if no cert is provided" do
%Settings{url: url, username: username, password: password} =
insert_software_updates_settings(ca_cert: nil, ca_uploaded_at: nil)
insert_software_updates_settings(ca_cert: nil, ca_uploaded_at: nil)

assert {:ok, _} = start_supervised({Suma, @test_integration_name})

base_api_url = "#{url}/rhn/manager/api"

expect(SumaApiMock, :login, fn ^_, _, _, false -> successful_login_response() end)
expect(SumaApiMock, :login, fn _, _, _, false -> successful_login_response() end)

assert :ok = Suma.setup(@test_integration_name)

Expand Down

0 comments on commit 99e1bd9

Please sign in to comment.