diff --git a/config/config.exs b/config/config.exs index 4c5ffd0e5b..988dcf3037 100644 --- a/config/config.exs +++ b/config/config.exs @@ -82,6 +82,10 @@ config :trento, :pow, # Agent heartbeat interval. Adding one extra second to the agent 5s interval to avoid glitches config :trento, Trento.Heartbeats, interval: :timer.seconds(6) +# This is passed to the frontend as the time after the last heartbeat +# to wait before displaying the deregistration button +config :trento, deregistration_debounce: :timer.seconds(0) + config :trento, Trento.Scheduler, jobs: [ heartbeat_check: [ diff --git a/config/test.exs b/config/test.exs index 19912da12f..726b4ac6b6 100644 --- a/config/test.exs +++ b/config/test.exs @@ -41,6 +41,13 @@ config :logger, level: :warn # Initialize plugs at runtime for faster test compilation config :phoenix, :plug_init_mode, :runtime +# Agent heartbeat interval. Adding one extra second to the agent 5s interval to avoid glitches +config :trento, Trento.Heartbeats, interval: :timer.seconds(6) + +# This is passed to the frontend as the time after the last heartbeat +# to wait before displaying the deregistration button +config :trento, deregistration_debounce: :timer.seconds(5) + config :trento, api_key_authentication_enabled: false, jwt_authentication_enabled: false diff --git a/lib/trento/application/integration/discovery/protocol/enrich_request_host_deregistration.ex b/lib/trento/application/integration/discovery/protocol/enrich_request_host_deregistration.ex new file mode 100644 index 0000000000..03f824f6e2 --- /dev/null +++ b/lib/trento/application/integration/discovery/protocol/enrich_request_host_deregistration.ex @@ -0,0 +1,48 @@ +defimpl Trento.Support.Middleware.Enrichable, + for: Trento.Domain.Commands.RequestHostDeregistration do + alias Trento.Domain.Commands.RequestHostDeregistration + + alias Trento.Repo + alias Trento.HostReadModel + + @heartbeat_interval Application.compile_env!(:trento, Trento.Heartbeats)[:interval] + @deregistration_debounce Application.compile_env!( + :trento, + :deregistration_debounce + ) + @total_deregistration_debounce @heartbeat_interval + @deregistration_debounce + + @spec enrich(RequestHostDeregistration.t(), map) :: + {:ok, RequestHostDeregistration.t()} + | {:error, :host_alive} + | {:error, :host_not_registered} + def enrich(%RequestHostDeregistration{host_id: host_id} = command, _) do + Repo.get(HostReadModel, host_id) + |> Repo.preload([:heartbeat_timestamp]) + |> host_deregisterable(command) + end + + defp host_deregisterable( + %HostReadModel{heartbeat_timestamp: nil, deregistered_at: nil}, + %RequestHostDeregistration{} = command + ), + do: {:ok, command} + + defp host_deregisterable( + %HostReadModel{ + heartbeat_timestamp: %Trento.Heartbeat{timestamp: timestamp}, + deregistered_at: nil + }, + %RequestHostDeregistration{} = command + ) do + if :lt == + DateTime.compare( + DateTime.utc_now(), + DateTime.add(timestamp, @total_deregistration_debounce, :millisecond) + ), + do: {:error, :host_alive}, + else: {:ok, command} + end + + defp host_deregisterable(_, _), do: {:error, :host_not_registered} +end diff --git a/lib/trento/application/read_models/host_read_model.ex b/lib/trento/application/read_models/host_read_model.ex index cb10589927..2fd75b056b 100644 --- a/lib/trento/application/read_models/host_read_model.ex +++ b/lib/trento/application/read_models/host_read_model.ex @@ -32,6 +32,10 @@ defmodule Trento.HostReadModel do foreign_key: :host_id, preload_order: [desc: :identifier] + has_one :heartbeat_timestamp, Trento.Heartbeat, + references: :id, + foreign_key: :agent_id + field :deregistered_at, :utc_datetime_usec end diff --git a/lib/trento/application/usecases/hosts/heartbeat.ex b/lib/trento/application/usecases/hosts/heartbeat.ex index 61b41d88c8..204ca2ddd0 100644 --- a/lib/trento/application/usecases/hosts/heartbeat.ex +++ b/lib/trento/application/usecases/hosts/heartbeat.ex @@ -7,6 +7,7 @@ defmodule Trento.Heartbeat do @type t :: %__MODULE__{} + @derive {Jason.Encoder, except: [:__meta__, :__struct__]} @primary_key {:agent_id, :string, autogenerate: false} schema "heartbeats" do field :timestamp, :utc_datetime_usec diff --git a/lib/trento/application/usecases/hosts/hosts.ex b/lib/trento/application/usecases/hosts/hosts.ex index d91e6e566a..d12735ecee 100644 --- a/lib/trento/application/usecases/hosts/hosts.ex +++ b/lib/trento/application/usecases/hosts/hosts.ex @@ -7,10 +7,13 @@ defmodule Trento.Hosts do alias Trento.{ HostReadModel, + Repo, SlesSubscriptionReadModel } - alias Trento.Repo + alias Trento.Support.DateService + + alias Trento.Domain.Commands.RequestHostDeregistration @spec get_all_hosts :: [HostReadModel.t()] def get_all_hosts do @@ -18,7 +21,7 @@ defmodule Trento.Hosts do |> where([h], not is_nil(h.hostname) and is_nil(h.deregistered_at)) |> order_by(asc: :hostname) |> Repo.all() - |> Repo.preload([:sles_subscriptions, :tags]) + |> Repo.preload([:sles_subscriptions, :tags, :heartbeat_timestamp]) end @spec get_all_sles_subscriptions :: non_neg_integer() @@ -36,4 +39,15 @@ defmodule Trento.Hosts do subscription_count end end + + @spec deregister_host(Ecto.UUID.t(), DateService) :: + :ok | {:error, :host_alive} | {:error, :host_not_registered} + def deregister_host(host_id, date_service \\ DateService) do + commanded().dispatch( + RequestHostDeregistration.new!(%{host_id: host_id, requested_at: date_service.utc_now()}) + ) + end + + defp commanded, + do: Application.fetch_env!(:trento, Trento.Commanded)[:adapter] end diff --git a/lib/trento_web/controllers/fallback_controller.ex b/lib/trento_web/controllers/fallback_controller.ex index 27dd2b4311..9b2c546408 100644 --- a/lib/trento_web/controllers/fallback_controller.ex +++ b/lib/trento_web/controllers/fallback_controller.ex @@ -58,6 +58,13 @@ defmodule TrentoWeb.FallbackController do |> render(:"422", reason: "Unknown discovery type.") end + def call(conn, {:error, :host_alive}) do + conn + |> put_status(:unprocessable_entity) + |> put_view(ErrorView) + |> render(:"422", reason: "Requested operation not allowed for live hosts.") + end + def call(conn, {:error, [error | _]}), do: call(conn, {:error, error}) def call(conn, {:error, _}) do diff --git a/lib/trento_web/controllers/page_controller.ex b/lib/trento_web/controllers/page_controller.ex index 87229644f7..61a55b37de 100644 --- a/lib/trento_web/controllers/page_controller.ex +++ b/lib/trento_web/controllers/page_controller.ex @@ -5,9 +5,12 @@ defmodule TrentoWeb.PageController do grafana_public_url = Application.fetch_env!(:trento, :grafana)[:public_url] check_service_base_url = Application.fetch_env!(:trento, :checks_service)[:base_url] + deregistration_debounce = Application.fetch_env!(:trento, :deregistration_debounce) + render(conn, "index.html", grafana_public_url: grafana_public_url, - check_service_base_url: check_service_base_url + check_service_base_url: check_service_base_url, + deregistration_debounce: deregistration_debounce ) end end diff --git a/lib/trento_web/controllers/v1/host_controller.ex b/lib/trento_web/controllers/v1/host_controller.ex index caf3a0d23e..f7fcab0763 100644 --- a/lib/trento_web/controllers/v1/host_controller.ex +++ b/lib/trento_web/controllers/v1/host_controller.ex @@ -9,6 +9,12 @@ defmodule TrentoWeb.V1.HostController do alias TrentoWeb.OpenApi.V1.Schema + alias TrentoWeb.OpenApi.V1.Schema.{ + BadRequest, + NotFound, + UnprocessableEntity + } + plug OpenApiSpex.Plug.CastAndValidate, json_render_error_v2: true action_fallback TrentoWeb.FallbackController @@ -29,6 +35,30 @@ defmodule TrentoWeb.V1.HostController do render(conn, "hosts.json", hosts: hosts) end + operation :delete, + summary: "Deregister a host", + description: "Deregister a host agent from Trento", + parameters: [ + id: [ + in: :path, + required: true, + type: %OpenApiSpex.Schema{type: :string, format: :uuid} + ] + ], + responses: [ + no_content: "The host has been deregistered", + not_found: NotFound.response(), + unprocessable_entity: UnprocessableEntity.response() + ] + + @spec delete(Plug.Conn.t(), map) :: Plug.Conn.t() + def delete(conn, %{id: host_id}) do + case Hosts.deregister_host(host_id) do + :ok -> send_resp(conn, 204, "") + {:error, error} -> {:error, error} + end + end + operation :heartbeat, summary: "Signal that an agent is alive", tags: ["Agent"], @@ -42,8 +72,8 @@ defmodule TrentoWeb.V1.HostController do ], responses: [ no_content: "The heartbeat has been updated", - not_found: Schema.NotFound.response(), - bad_request: Schema.BadRequest.response(), + not_found: NotFound.response(), + bad_request: BadRequest.response(), unprocessable_entity: OpenApiSpex.JsonErrorResponse.response() ] diff --git a/lib/trento_web/openapi/v1/schema/host.ex b/lib/trento_web/openapi/v1/schema/host.ex index bf4cc212cc..887adfca4c 100644 --- a/lib/trento_web/openapi/v1/schema/host.ex +++ b/lib/trento_web/openapi/v1/schema/host.ex @@ -69,6 +69,23 @@ defmodule TrentoWeb.OpenApi.V1.Schema.Host do description: "A list of the available SLES Subscriptions on a host", type: :array, items: SlesSubscription + }, + deregistered_at: %Schema{ + title: "DeregisteredAt", + description: "Timestamp of the last deregistration of the host", + type: :string, + nullable: true, + format: :"date-time" + }, + heartbeat_timestamp: %Schema{ + title: "HeartbeatTimestamp", + description: "Timestamp of the last heartbeat received from the host", + type: :object, + nullable: true, + properties: %{ + agent_id: %Schema{type: :string, description: "Host ID", format: :uuid}, + timestamp: %Schema{type: :string, format: :"date-time"} + } } } }) diff --git a/lib/trento_web/openapi/v1/schema/unprocessable_entity.ex b/lib/trento_web/openapi/v1/schema/unprocessable_entity.ex new file mode 100644 index 0000000000..e9cd04121d --- /dev/null +++ b/lib/trento_web/openapi/v1/schema/unprocessable_entity.ex @@ -0,0 +1,35 @@ +defmodule TrentoWeb.OpenApi.V1.Schema.UnprocessableEntity do + @moduledoc """ + 422 - Unprocessable Entity + """ + require OpenApiSpex + + alias OpenApiSpex.Operation + alias OpenApiSpex.Schema + + OpenApiSpex.schema(%{ + type: :object, + properties: %{ + errors: %Schema{ + type: :array, + items: %Schema{ + type: :object, + properties: %{ + title: %Schema{type: :string, example: "Invalid value"}, + detail: %Schema{type: :string, example: "null value where string expected"} + }, + required: [:title, :detail] + } + } + }, + required: [:errors] + }) + + def response do + Operation.response( + "Unprocessable Entity", + "application/json", + __MODULE__ + ) + end +end diff --git a/lib/trento_web/router.ex b/lib/trento_web/router.ex index 444bff222a..0b1766900b 100644 --- a/lib/trento_web/router.ex +++ b/lib/trento_web/router.ex @@ -94,6 +94,8 @@ defmodule TrentoWeb.Router do assigns: %{resource_type: :host}, as: :hosts_tagging + delete "/hosts/:id", HostController, :delete + delete "/hosts/:id/tags/:value", TagsController, :remove_tag, as: :hosts_tagging post "/clusters/:id/tags", TagsController, :add_tag, diff --git a/lib/trento_web/templates/page/index.html.heex b/lib/trento_web/templates/page/index.html.heex index d595200888..770db5bca1 100644 --- a/lib/trento_web/templates/page/index.html.heex +++ b/lib/trento_web/templates/page/index.html.heex @@ -3,7 +3,8 @@ diff --git a/test/support/factory.ex b/test/support/factory.ex index c43f238152..cb09be8192 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -96,7 +96,8 @@ defmodule Trento.Factory do cluster_id: Faker.UUID.v4(), heartbeat: :unknown, provider: Enum.random(Provider.values()), - provider_data: nil + provider_data: nil, + deregistered_at: nil } end diff --git a/test/trento/application/integration/discovery/protocol/enrich_request_host_deregistration_test.exs b/test/trento/application/integration/discovery/protocol/enrich_request_host_deregistration_test.exs new file mode 100644 index 0000000000..9becd4181d --- /dev/null +++ b/test/trento/application/integration/discovery/protocol/enrich_request_host_deregistration_test.exs @@ -0,0 +1,101 @@ +defmodule Trento.EnrichRequestHostDeregistrationTest do + use ExUnit.Case + use Trento.DataCase + + import Trento.Factory + + alias Trento.Domain.Commands.RequestHostDeregistration + alias Trento.Support.Middleware.Enrichable + + @heartbeat_interval Application.compile_env!(:trento, Trento.Heartbeats)[:interval] + @deregistration_debounce Application.compile_env!( + :trento, + :deregistration_debounce + ) + @total_deregistration_debounce @heartbeat_interval + @deregistration_debounce + + describe "enrich RequestHostDeregistration" do + test "should deregister host if deregistration request is outside debounce period" do + now = DateTime.utc_now() + + %{id: id} = insert(:host) + + insert(:heartbeat, + agent_id: id, + timestamp: + DateTime.add( + DateTime.utc_now(), + -(@total_deregistration_debounce + 10_000), + :millisecond + ) + ) + + command = RequestHostDeregistration.new!(%{host_id: id, requested_at: now}) + + assert {:ok, %RequestHostDeregistration{host_id: id, requested_at: now}} == + Enrichable.enrich(command, %{}) + end + + test "should deregister host if host does not have a heartbeat entry" do + now = DateTime.utc_now() + + %{id: id} = insert(:host) + + command = RequestHostDeregistration.new!(%{host_id: id, requested_at: now}) + + assert {:ok, %RequestHostDeregistration{host_id: id, requested_at: now}} == + Enrichable.enrich(command, %{}) + end + + test "should return an error if deregistration request is within debounce period" do + %{id: id} = insert(:host) + + insert(:heartbeat, + agent_id: id, + timestamp: + DateTime.add( + DateTime.utc_now(), + -(@total_deregistration_debounce - 2_000), + :millisecond + ) + ) + + command = RequestHostDeregistration.new!(%{host_id: id, requested_at: DateTime.utc_now()}) + + assert {:error, :host_alive} == Enrichable.enrich(command, %{}) + end + + test "should return an error when deregistering a host that is already deregistered" do + %{id: id} = insert(:host, deregistered_at: DateTime.utc_now()) + + insert(:heartbeat, + agent_id: id, + timestamp: + DateTime.add( + DateTime.utc_now(), + -(@total_deregistration_debounce + 10_000), + :millisecond + ) + ) + + command = RequestHostDeregistration.new!(%{host_id: id, requested_at: DateTime.utc_now()}) + + assert {:error, :host_not_registered} == Enrichable.enrich(command, %{}) + end + + test "should return an error when deregistering a host that is already deregistered and does not have a heartbeat entry" do + %{id: id} = insert(:host, deregistered_at: DateTime.utc_now()) + + command = RequestHostDeregistration.new!(%{host_id: id, requested_at: DateTime.utc_now()}) + + assert {:error, :host_not_registered} == Enrichable.enrich(command, %{}) + end + + test "should return an error if host does not exist" do + command = + RequestHostDeregistration.new!(%{host_id: UUID.uuid4(), requested_at: DateTime.utc_now()}) + + assert {:error, :host_not_registered} == Enrichable.enrich(command, %{}) + end + end +end diff --git a/test/trento/application/usecases/hosts_test.exs b/test/trento/application/usecases/hosts_test.exs index 9bf9732a50..7399836829 100644 --- a/test/trento/application/usecases/hosts_test.exs +++ b/test/trento/application/usecases/hosts_test.exs @@ -3,6 +3,7 @@ defmodule Trento.HostsTest do use Trento.DataCase import Trento.Factory + import Mox alias Trento.Hosts alias Trento.Repo @@ -11,6 +12,8 @@ defmodule Trento.HostsTest do @moduletag :integration + setup :verify_on_exit! + describe "SLES Subscriptions" do test "No SLES4SAP Subscriptions detected" do assert 0 = SlesSubscriptionReadModel |> Repo.all() |> length() diff --git a/test/trento_web/controllers/v1/host_controller_test.exs b/test/trento_web/controllers/v1/host_controller_test.exs index 548414171a..bc9d7d4083 100644 --- a/test/trento_web/controllers/v1/host_controller_test.exs +++ b/test/trento_web/controllers/v1/host_controller_test.exs @@ -11,6 +11,10 @@ defmodule TrentoWeb.V1.HostControllerTest do setup [:set_mox_from_context, :verify_on_exit!] + setup do + %{api_spec: ApiSpec.spec()} + end + describe "list" do test "should list all hosts", %{conn: conn} do %{id: host_id} = insert(:host) @@ -48,4 +52,56 @@ defmodule TrentoWeb.V1.HostControllerTest do } == resp end end + + describe "delete" do + test "should send 204 response when successful host deletion", %{conn: conn} do + %{id: host_id} = insert(:host) + + expect( + Trento.Commanded.Mock, + :dispatch, + fn %Trento.Domain.Commands.RequestHostDeregistration{host_id: ^host_id} -> + :ok + end + ) + + conn + |> delete("/api/v1/hosts/#{host_id}") + |> response(204) + end + + test "should send 422 response if the host is still alive", %{conn: conn, api_spec: api_spec} do + %{id: host_id} = insert(:host) + + expect( + Trento.Commanded.Mock, + :dispatch, + fn %Trento.Domain.Commands.RequestHostDeregistration{host_id: ^host_id} -> + {:error, :host_alive} + end + ) + + conn + |> delete("/api/v1/hosts/#{host_id}") + |> json_response(422) + |> assert_schema("UnprocessableEntity", api_spec) + end + + test "should return 404 if the host was not found", %{conn: conn, api_spec: api_spec} do + %{id: host_id} = insert(:host) + + expect( + Trento.Commanded.Mock, + :dispatch, + fn %Trento.Domain.Commands.RequestHostDeregistration{host_id: ^host_id} -> + {:error, :host_not_registered} + end + ) + + conn + |> delete("/api/v1/hosts/#{host_id}") + |> json_response(404) + |> assert_schema("NotFound", api_spec) + end + end end