From 75776e945d21b02a74dd068d6784d13a91416f4b Mon Sep 17 00:00:00 2001 From: Nelson Kopliku Date: Mon, 4 Mar 2024 09:09:38 +0100 Subject: [PATCH] Add a scheduled software updates discovery job (#2376) * Add a discovery function that gets relevant patches for all hosts and potentially changes health * Add discovery to software updates context and register a quantum process * fix suma genserver setup handle continue * Add missing url composition when getting relevant patches * Different approach to incrementing relevant patches counters * simplify tuples returned by softwre updates discovery --- config/config.exs | 7 + .../infrastructure/software_updates/suma.ex | 5 +- .../software_updates/suma_api.ex | 11 +- lib/trento/software_updates.ex | 15 ++ lib/trento/software_updates/discovery.ex | 90 +++++++ .../software_updates/enums/advisory_type.ex | 15 ++ test/support/factory.ex | 10 + test/test_helper.exs | 6 + .../software_updates/suma_test.exs | 22 +- .../software_updates/discovery_test.exs | 248 ++++++++++++++++++ test/trento/software_updates_test.exs | 71 +++-- 11 files changed, 458 insertions(+), 42 deletions(-) create mode 100644 lib/trento/software_updates/enums/advisory_type.ex create mode 100644 test/trento/software_updates/discovery_test.exs diff --git a/config/config.exs b/config/config.exs index f036081314..a7009ecf31 100644 --- a/config/config.exs +++ b/config/config.exs @@ -115,6 +115,13 @@ config :trento, Trento.Scheduler, task: {Trento.Hosts, :request_hosts_checks_execution, []}, run_strategy: {Quantum.RunStrategy.Random, :cluster}, overlap: false + ], + discover_software_updates: [ + # Runs every 12 hours. At 00:00 and 12:00 + schedule: "0 */12 * * *", + task: {Trento.SoftwareUpdates, :run_discovery, []}, + run_strategy: {Quantum.RunStrategy.Random, :cluster}, + overlap: false ] ], debug_logging: false diff --git a/lib/trento/infrastructure/software_updates/suma.ex b/lib/trento/infrastructure/software_updates/suma.ex index 2e06dd185e..5309fa2f0c 100644 --- a/lib/trento/infrastructure/software_updates/suma.ex +++ b/lib/trento/infrastructure/software_updates/suma.ex @@ -94,8 +94,9 @@ defmodule Trento.Infrastructure.SoftwareUpdates.Suma do Process.send(self(), {previous_message, reply_to: from}, [:nosuspend, :noconnect]) {:noreply, new_state} - {:error, reason} -> - {:stop, reason} + {:error, _} = error -> + GenServer.reply(from, error) + {:noreply, state} end end diff --git a/lib/trento/infrastructure/software_updates/suma_api.ex b/lib/trento/infrastructure/software_updates/suma_api.ex index 4a0301c54b..6016c5dc24 100644 --- a/lib/trento/infrastructure/software_updates/suma_api.ex +++ b/lib/trento/infrastructure/software_updates/suma_api.ex @@ -3,6 +3,7 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaApi do SUMA API client supporting software updates discovery. """ + require Trento.SoftwareUpdates.Enums.AdvisoryType, as: AdvisoryType require Logger @login_retries 5 @@ -34,11 +35,17 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaApi do end def get_relevant_patches(url, auth, system_id) do - response = http_executor().get_relevant_patches(url, auth, system_id) + response = + url + |> get_suma_api_url() + |> http_executor().get_relevant_patches(auth, system_id) with {:ok, %HTTPoison.Response{status_code: 200, body: body}} <- response, {:ok, %{success: true, result: result}} <- Jason.decode(body, keys: :atoms) do - {:ok, result} + {:ok, + Enum.map(result, fn %{advisory_type: advisory_type} = advisory -> + %{advisory | advisory_type: AdvisoryType.from_string(advisory_type)} + end)} else error -> Logger.error("Failed to get errata for system ID #{system_id}. Error: #{inspect(error)}") diff --git a/lib/trento/software_updates.ex b/lib/trento/software_updates.ex index 232499b02e..d4ab9f9ea5 100644 --- a/lib/trento/software_updates.ex +++ b/lib/trento/software_updates.ex @@ -7,6 +7,7 @@ defmodule Trento.SoftwareUpdates do alias Trento.Support.DateService alias Trento.Repo + alias Trento.SoftwareUpdates.Discovery alias Trento.SoftwareUpdates.Settings @type software_update_settings_save_submission :: %{ @@ -70,6 +71,20 @@ defmodule Trento.SoftwareUpdates do :ok end + @spec run_discovery :: :ok | {:error, :settings_not_configured} + def run_discovery do + case get_settings() do + {:ok, _} -> + Discovery.discover_software_updates() + :ok + + error -> + Logger.error("Software updates settings not configured. Skipping discovery.") + + error + end + end + defp has_settings?(%Settings{url: url, username: username, password: password}), do: url != nil and username != nil and password != nil diff --git a/lib/trento/software_updates/discovery.ex b/lib/trento/software_updates/discovery.ex index 81922b6201..3f61e85a39 100644 --- a/lib/trento/software_updates/discovery.ex +++ b/lib/trento/software_updates/discovery.ex @@ -3,6 +3,14 @@ defmodule Trento.SoftwareUpdates.Discovery do Software updates integration service """ + alias Trento.Hosts + alias Trento.Hosts.Commands.CompleteSoftwareUpdatesDiscovery + alias Trento.Hosts.Projections.HostReadModel + + require Trento.SoftwareUpdates.Enums.AdvisoryType, as: AdvisoryType + + require Logger + @behaviour Trento.SoftwareUpdates.Discovery.Gen @impl true @@ -13,5 +21,87 @@ defmodule Trento.SoftwareUpdates.Discovery do def get_relevant_patches(system_id), do: adapter().get_relevant_patches(system_id) + @spec discover_software_updates :: {:ok, {list(), list()}} + def discover_software_updates, + do: + {:ok, + Hosts.get_all_hosts() + |> Enum.map(&discover_host_software_updates/1) + |> Enum.split_with(fn + {:ok, _, _, _} -> true + _ -> false + end)} + + defp discover_host_software_updates(%HostReadModel{ + id: host_id, + fully_qualified_domain_name: nil + }) do + Logger.info("Host #{host_id} does not have an fqdn. Skipping software updates discovery") + {:error, host_id, :host_without_fqdn} + end + + defp discover_host_software_updates(%HostReadModel{ + id: host_id, + fully_qualified_domain_name: fully_qualified_domain_name + }) do + with {:ok, system_id} <- get_system_id(fully_qualified_domain_name), + {:ok, relevant_patches} <- get_relevant_patches(system_id), + :ok <- + host_id + |> build_discovery_completion_command(relevant_patches) + |> commanded().dispatch() do + {:ok, host_id, system_id, relevant_patches} + else + error -> + Logger.error( + "An error occurred during software updates discovery for host #{host_id}: #{inspect(error)}" + ) + + {:error, host_id, error} + end + end + + defp build_discovery_completion_command(host_id, relevant_patches), + do: + CompleteSoftwareUpdatesDiscovery.new!(%{ + host_id: host_id, + relevant_patches: + Enum.reduce( + relevant_patches, + %{ + security_advisories: 0, + bug_fixes: 0, + software_enhancements: 0 + }, + &track_relevant_patches/2 + ) + }) + + defp track_relevant_patches( + %{advisory_type: AdvisoryType.security_advisory()}, + %{ + security_advisories: security_advisories + } = patches + ), + do: %{patches | security_advisories: security_advisories + 1} + + defp track_relevant_patches( + %{advisory_type: AdvisoryType.bugfix()}, + %{ + bug_fixes: bug_fixes + } = patches + ), + do: %{patches | bug_fixes: bug_fixes + 1} + + defp track_relevant_patches( + %{advisory_type: AdvisoryType.enhancement()}, + %{ + software_enhancements: software_enhancements + } = patches + ), + do: %{patches | software_enhancements: software_enhancements + 1} + defp adapter, do: Application.fetch_env!(:trento, __MODULE__)[:adapter] + + defp commanded, do: Application.fetch_env!(:trento, Trento.Commanded)[:adapter] end diff --git a/lib/trento/software_updates/enums/advisory_type.ex b/lib/trento/software_updates/enums/advisory_type.ex new file mode 100644 index 0000000000..0857b7d561 --- /dev/null +++ b/lib/trento/software_updates/enums/advisory_type.ex @@ -0,0 +1,15 @@ +defmodule Trento.SoftwareUpdates.Enums.AdvisoryType do + @moduledoc """ + Enum representing possible advisory types. + """ + + @security_advisory "Security Advisory" + @bugfix "Bug Fix Advisory" + @enhancement "Product Enhancement Advisory" + + use Trento.Support.Enum, values: [:security_advisory, :bugfix, :enhancement] + + def from_string(@security_advisory), do: security_advisory() + def from_string(@bugfix), do: bugfix() + def from_string(@enhancement), do: enhancement() +end diff --git a/test/support/factory.ex b/test/support/factory.ex index b26e4afd6e..e90e4463c7 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -141,6 +141,7 @@ defmodule Trento.Factory do %HostReadModel{ id: Faker.UUID.v4(), hostname: Faker.StarWars.character(), + fully_qualified_domain_name: Faker.Internet.domain_name(), ip_addresses: [Faker.Internet.ip_v4_address()], agent_version: Faker.StarWars.planet(), cluster_id: Faker.UUID.v4(), @@ -806,4 +807,13 @@ defmodule Trento.Factory do eula_accepted: true } end + + def insert_software_updates_settings(attrs \\ []) do + insert( + :software_updates_settings, + attrs, + conflict_target: :id, + on_conflict: :replace_all + ) + end end diff --git a/test/test_helper.exs b/test/test_helper.exs index b3893523ed..986d2703ba 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -22,6 +22,12 @@ Application.put_env(:trento, Trento.Infrastructure.SoftwareUpdates.SumaApi, executor: Trento.Infrastructure.SoftwareUpdates.Suma.HttpExecutor.Mock ) +Mox.defmock(Trento.SoftwareUpdates.Discovery.Mock, for: Trento.SoftwareUpdates.Discovery.Gen) + +Application.put_env(:trento, Trento.SoftwareUpdates.Discovery, + adapter: Trento.SoftwareUpdates.Discovery.Mock +) + Mox.defmock(Trento.Infrastructure.Messaging.Adapter.Mock, for: Trento.Infrastructure.Messaging.Adapter.Gen ) diff --git a/test/trento/infrastructure/software_updates/suma_test.exs b/test/trento/infrastructure/software_updates/suma_test.exs index ed83e0c101..431a563854 100644 --- a/test/trento/infrastructure/software_updates/suma_test.exs +++ b/test/trento/infrastructure/software_updates/suma_test.exs @@ -280,7 +280,27 @@ defmodule Trento.Infrastructure.SoftwareUpdates.SumaTest do {:ok, %HTTPoison.Response{status_code: 200, body: Jason.encode!(suma_response_body)}} end) - assert {:ok, ^patches} = + assert {:ok, + [ + %{ + date: "2024-02-27", + advisory_name: "SUSE-15-SP4-2024-630", + advisory_type: :bugfix, + advisory_status: "stable", + id: 4182, + advisory_synopsis: "Recommended update for cloud-netconfig", + update_date: "2024-02-27" + }, + %{ + date: "2024-02-26", + advisory_name: "SUSE-15-SP4-2024-619", + advisory_type: :security_advisory, + advisory_status: "stable", + id: 4174, + advisory_synopsis: "important: Security update for java-1_8_0-ibm", + update_date: "2024-02-26" + } + ]} = Suma.get_relevant_patches(system_id, @test_integration_name) end end diff --git a/test/trento/software_updates/discovery_test.exs b/test/trento/software_updates/discovery_test.exs new file mode 100644 index 0000000000..fce7ffb9d2 --- /dev/null +++ b/test/trento/software_updates/discovery_test.exs @@ -0,0 +1,248 @@ +defmodule Trento.SoftwareUpdates.DiscoveryTest do + use ExUnit.Case + use Trento.DataCase + + import Mox + + import Trento.Factory + + alias Trento.Hosts.Commands.CompleteSoftwareUpdatesDiscovery + alias Trento.Hosts.ValueObjects.RelevantPatches + alias Trento.SoftwareUpdates.Discovery + alias Trento.SoftwareUpdates.Discovery.Mock, as: SoftwareUpdatesDiscoveryMock + + require Trento.SoftwareUpdates.Enums.AdvisoryType, as: AdvisoryType + + describe "Discovering software updates" do + test "should handle empty hosts list" do + assert {:ok, {[], []}} = Discovery.discover_software_updates() + end + + test "should handle hosts without fqdn" do + %{id: host_id1} = insert(:host, fully_qualified_domain_name: nil) + %{id: host_id2} = insert(:host, fully_qualified_domain_name: nil) + + {:ok, {[], errored_discoveries}} = Discovery.discover_software_updates() + + Enum.each([host_id1, host_id2], fn host_id -> + assert {:error, host_id, :host_without_fqdn} in errored_discoveries + end) + end + + test "should handle errors when getting a system id" do + %{id: host_id, fully_qualified_domain_name: fully_qualified_domain_name} = insert(:host) + + discovery_error = {:error, :some_error_while_getting_system_id} + + expect( + SoftwareUpdatesDiscoveryMock, + :get_system_id, + fn ^fully_qualified_domain_name -> discovery_error end + ) + + expect( + SoftwareUpdatesDiscoveryMock, + :get_relevant_patches, + 0, + fn _ -> :ok end + ) + + expect( + Trento.Commanded.Mock, + :dispatch, + 0, + fn _ -> :ok end + ) + + {:ok, {[], errored_discoveries}} = Discovery.discover_software_updates() + + assert {:error, host_id, discovery_error} in errored_discoveries + end + + test "should handle errors when getting relevant patches" do + %{id: host_id, fully_qualified_domain_name: fully_qualified_domain_name} = insert(:host) + + system_id = 100 + discovery_error = {:error, :some_error_while_getting_relevant_patches} + + expect( + SoftwareUpdatesDiscoveryMock, + :get_system_id, + fn ^fully_qualified_domain_name -> {:ok, system_id} end + ) + + expect( + SoftwareUpdatesDiscoveryMock, + :get_relevant_patches, + fn ^system_id -> discovery_error end + ) + + expect( + Trento.Commanded.Mock, + :dispatch, + 0, + fn _ -> :ok end + ) + + {:ok, {[], errored_discoveries}} = Discovery.discover_software_updates() + + assert {:error, host_id, discovery_error} in errored_discoveries + end + + test "should handle errors when dispatching discovery completion command" do + %{id: host_id, fully_qualified_domain_name: fully_qualified_domain_name} = insert(:host) + + system_id = 100 + + expect( + SoftwareUpdatesDiscoveryMock, + :get_system_id, + fn ^fully_qualified_domain_name -> {:ok, system_id} end + ) + + expect( + SoftwareUpdatesDiscoveryMock, + :get_relevant_patches, + fn ^system_id -> {:ok, [%{advisory_type: AdvisoryType.security_advisory()}]} end + ) + + dispatching_error = {:error, :error_while_dispatching_completion_command} + + expect( + Trento.Commanded.Mock, + :dispatch, + fn %CompleteSoftwareUpdatesDiscovery{host_id: ^host_id} -> dispatching_error end + ) + + {:ok, {[], errored_discoveries}} = Discovery.discover_software_updates() + + assert {:error, host_id, dispatching_error} in errored_discoveries + end + + test "should complete discovery" do + %{id: host_id1, fully_qualified_domain_name: fully_qualified_domain_name1} = + insert(:host, hostname: "host1") + + %{id: host_id2, fully_qualified_domain_name: fully_qualified_domain_name2} = + insert(:host, hostname: "host2") + + %{id: host_id3, fully_qualified_domain_name: fully_qualified_domain_name3} = + insert(:host, hostname: "host3") + + %{id: host_id4} = insert(:host, fully_qualified_domain_name: nil) + + system_id1 = 100 + system_id2 = 101 + system_id3 = 102 + + system_ids = [ + system_id1, + system_id2, + system_id3 + ] + + fqdns = [ + fully_qualified_domain_name1, + fully_qualified_domain_name2, + fully_qualified_domain_name3 + ] + + {:ok, _} = Agent.start_link(fn -> 0 end, name: :get_system_id_iteration) + + expect( + SoftwareUpdatesDiscoveryMock, + :get_system_id, + 3, + fn fqdn -> + iteration = Agent.get(:get_system_id_iteration, & &1) + + assert fqdn == Enum.at(fqdns, iteration) + + Agent.update(:get_system_id_iteration, &(&1 + 1)) + + {:ok, Enum.at(system_ids, iteration)} + end + ) + + discovered_relevant_patches = [ + %{advisory_type: AdvisoryType.security_advisory()}, + %{advisory_type: AdvisoryType.security_advisory()}, + %{advisory_type: AdvisoryType.bugfix()}, + %{advisory_type: AdvisoryType.enhancement()} + ] + + {:ok, _} = Agent.start_link(fn -> 0 end, name: :get_relevant_patches_iteration) + + expect( + SoftwareUpdatesDiscoveryMock, + :get_relevant_patches, + 3, + fn system_id -> + iteration = Agent.get(:get_relevant_patches_iteration, & &1) + + assert system_id == Enum.at(system_ids, iteration) + + get_relevant_patches_result = + case system_id do + ^system_id2 -> {:error, :some_error} + _ -> {:ok, discovered_relevant_patches} + end + + Agent.update(:get_relevant_patches_iteration, &(&1 + 1)) + + get_relevant_patches_result + end + ) + + expected_relevant_patches = %RelevantPatches{ + security_advisories: 2, + bug_fixes: 1, + software_enhancements: 1 + } + + {:ok, _} = Agent.start_link(fn -> 0 end, name: :command_dispatching_iteration) + + expect( + Trento.Commanded.Mock, + :dispatch, + 2, + fn command -> + iteration = Agent.get(:command_dispatching_iteration, & &1) + + case iteration do + 0 -> + assert %CompleteSoftwareUpdatesDiscovery{ + host_id: ^host_id1, + relevant_patches: ^expected_relevant_patches + } = command + + 1 -> + assert %CompleteSoftwareUpdatesDiscovery{ + host_id: ^host_id3, + relevant_patches: ^expected_relevant_patches + } = command + end + + Agent.update(:command_dispatching_iteration, &(&1 + 1)) + + :ok + end + ) + + assert {:ok, {successful_discoveries, errored_discoveries}} = + Discovery.discover_software_updates() + + assert length(successful_discoveries) == 2 + assert length(errored_discoveries) == 2 + + assert [ + {:ok, host_id1, system_id1, discovered_relevant_patches}, + {:ok, host_id3, system_id3, discovered_relevant_patches} + ] == + successful_discoveries + + assert {:error, host_id2, {:error, :some_error}} in errored_discoveries + assert {:error, host_id4, :host_without_fqdn} in errored_discoveries + end + end +end diff --git a/test/trento/software_updates_test.exs b/test/trento/software_updates_test.exs index 47a8fa9898..e77c1df55e 100644 --- a/test/trento/software_updates_test.exs +++ b/test/trento/software_updates_test.exs @@ -20,9 +20,9 @@ defmodule Trento.SoftwareUpdates.SettingsTest do username: username, password: password } = - insert(:software_updates_settings, [ca_cert: nil, ca_uploaded_at: nil], - conflict_target: :id, - on_conflict: :replace_all + insert_software_updates_settings( + ca_cert: nil, + ca_uploaded_at: nil ) assert {:ok, @@ -43,11 +43,9 @@ defmodule Trento.SoftwareUpdates.SettingsTest do ca_cert: ca_cert, ca_uploaded_at: ca_uploaded_at } = - insert( - :software_updates_settings, - [ca_cert: Faker.Lorem.sentence(), ca_uploaded_at: DateTime.utc_now()], - conflict_target: :id, - on_conflict: :replace_all + insert_software_updates_settings( + ca_cert: Faker.Lorem.sentence(), + ca_uploaded_at: DateTime.utc_now() ) assert {:ok, @@ -208,10 +206,7 @@ defmodule Trento.SoftwareUpdates.SettingsTest do end test "should validate partial changes to software updates settings" do - insert(:software_updates_settings, [], - conflict_target: :id, - on_conflict: :replace_all - ) + insert_software_updates_settings() change_settings_scenarios = [ %{ @@ -282,11 +277,9 @@ defmodule Trento.SoftwareUpdates.SettingsTest do ca_cert: initial_ca_cert, ca_uploaded_at: initial_ca_uploaded_at } = - insert( - :software_updates_settings, - [ca_cert: Faker.Lorem.sentence(), ca_uploaded_at: DateTime.utc_now()], - conflict_target: :id, - on_conflict: :replace_all + insert_software_updates_settings( + ca_cert: Faker.Lorem.sentence(), + ca_uploaded_at: DateTime.utc_now() ) change_submission = %{ @@ -320,11 +313,9 @@ defmodule Trento.SoftwareUpdates.SettingsTest do ca_cert: _initial_ca_cert, ca_uploaded_at: _initial_ca_uploaded_at } = - insert( - :software_updates_settings, - [ca_cert: Faker.Lorem.sentence(), ca_uploaded_at: DateTime.utc_now()], - conflict_target: :id, - on_conflict: :replace_all + insert_software_updates_settings( + ca_cert: Faker.Lorem.sentence(), + ca_uploaded_at: DateTime.utc_now() ) change_submission = %{ @@ -351,11 +342,9 @@ defmodule Trento.SoftwareUpdates.SettingsTest do ca_cert: _initial_ca_cert, ca_uploaded_at: _initial_ca_uploaded_at } = - insert( - :software_updates_settings, - [ca_cert: Faker.Lorem.sentence(), ca_uploaded_at: DateTime.utc_now()], - conflict_target: :id, - on_conflict: :replace_all + insert_software_updates_settings( + ca_cert: Faker.Lorem.sentence(), + ca_uploaded_at: DateTime.utc_now() ) now = DateTime.utc_now() @@ -400,11 +389,9 @@ defmodule Trento.SoftwareUpdates.SettingsTest do ca_cert: _initial_ca_cert, ca_uploaded_at: _initial_ca_uploaded_at } = - insert( - :software_updates_settings, - [ca_cert: Faker.Lorem.sentence(), ca_uploaded_at: DateTime.utc_now()], - conflict_target: :id, - on_conflict: :replace_all + insert_software_updates_settings( + ca_cert: Faker.Lorem.sentence(), + ca_uploaded_at: DateTime.utc_now() ) change_submission = %{ @@ -424,11 +411,9 @@ defmodule Trento.SoftwareUpdates.SettingsTest do describe "clearing software update settings" do test "should support idempotent sequential clear settings" do - insert( - :software_updates_settings, - [ca_cert: Faker.Lorem.sentence(), ca_uploaded_at: DateTime.utc_now()], - conflict_target: :id, - on_conflict: :replace_all + insert_software_updates_settings( + ca_cert: Faker.Lorem.sentence(), + ca_uploaded_at: DateTime.utc_now() ) assert {:ok, _} = SoftwareUpdates.get_settings() @@ -439,4 +424,16 @@ defmodule Trento.SoftwareUpdates.SettingsTest do end) end end + + describe "discovery" do + test "should not start discovery if settings are not configured" do + assert {:error, :settings_not_configured} = SoftwareUpdates.run_discovery() + end + + test "should start discovery if settings are configured" do + insert_software_updates_settings() + + assert :ok == SoftwareUpdates.run_discovery() + end + end end