From c35a502425ab55e30eb07156dc5d9250974fde44 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Sat, 17 Jan 2026 15:40:09 -0600 Subject: [PATCH] Change agent revoke to delete with full cleanup Replace the revoke functionality with delete to properly clean up agent references. When an agent is deleted: - All direct device assignments are removed - Agent is removed from site defaults - Agent is removed from organization defaults - Devices automatically fall back to site/org agents or cloud polling This ensures no orphaned references and provides better clarity about what happens to device monitoring when an agent is removed. - Add delete_agent_token/1 function in Agents context - Update LiveView to use delete_agent event instead of revoke_agent - Update UI button from "Revoke" to "Delete" with clearer confirmation message - Add comprehensive tests for delete functionality and fallback behavior - All 792 tests passing, no Credo warnings --- lib/towerops/agents.ex | 40 ++++ lib/towerops_web/live/agent_live/index.ex | 19 +- .../live/agent_live/index.html.heex | 8 +- test/towerops/agents_test.exs | 216 ++++++++++++++++++ .../live/agent_live/index_test.exs | 16 +- test/towerops_web/live/agent_live_test.exs | 11 +- 6 files changed, 287 insertions(+), 23 deletions(-) diff --git a/lib/towerops/agents.ex b/lib/towerops/agents.ex index 7ad6579d..40c135d4 100644 --- a/lib/towerops/agents.ex +++ b/lib/towerops/agents.ex @@ -162,6 +162,46 @@ defmodule Towerops.Agents do |> Repo.update() end + @doc """ + Deletes an agent token and cleans up all references. + + This function: + 1. Removes all direct device assignments for this agent + 2. Removes this agent from any sites using it as default + 3. Removes this agent from any organizations using it as default + 4. Deletes the agent token + + Devices that were assigned to this agent will fall back to: + - Site-level agent (if configured) + - Organization-level default agent (if configured) + - Cloud polling (if no fallback exists) + + ## Examples + + iex> delete_agent_token(id) + {:ok, %AgentToken{}} + + """ + def delete_agent_token(id) do + Repo.transaction(fn -> + agent_token = Repo.get!(AgentToken, id) + + # 1. Remove all direct device assignments + Repo.delete_all(from(a in AgentAssignment, where: a.agent_token_id == ^id)) + + # 2. Remove this agent from any sites using it as default + Repo.update_all(from(s in Towerops.Sites.Site, where: s.agent_token_id == ^id), set: [agent_token_id: nil]) + + # 3. Remove this agent from any organizations using it as default + Repo.update_all(from(o in Towerops.Organizations.Organization, where: o.default_agent_token_id == ^id), + set: [default_agent_token_id: nil] + ) + + # 4. Delete the agent token + Repo.delete!(agent_token) + end) + end + ## Assignment management @doc """ diff --git a/lib/towerops_web/live/agent_live/index.ex b/lib/towerops_web/live/agent_live/index.ex index 82e484a8..5dd8a17c 100644 --- a/lib/towerops_web/live/agent_live/index.ex +++ b/lib/towerops_web/live/agent_live/index.ex @@ -138,25 +138,36 @@ defmodule ToweropsWeb.AgentLive.Index do end @impl true - def handle_event("revoke_agent", %{"id" => id}, socket) do - case Agents.revoke_agent_token(id) do + def handle_event("delete_agent", %{"id" => id}, socket) do + case Agents.delete_agent_token(id) do {:ok, _} -> organization = socket.assigns.current_organization agent_tokens = Agents.list_organization_agent_tokens(organization.id) + # Recalculate device counts after deletion + equipment_counts = + Map.new(agent_tokens, fn token -> + direct = Agents.count_assigned_devices(token.id) + total = length(Agents.list_agent_polling_targets(token.id)) + {token.id, %{direct: direct, total: total}} + end) + # Refresh health statistics agent_health_stats = Stats.get_organization_agent_health(organization.id) + assignment_breakdown = Stats.get_device_assignment_breakdown(organization.id) offline_agents = Stats.get_offline_agents(organization.id) {:noreply, socket |> assign(:agent_tokens, agent_tokens) + |> assign(:device_counts, equipment_counts) |> assign(:agent_health_stats, agent_health_stats) + |> assign(:assignment_breakdown, assignment_breakdown) |> assign(:offline_agents, offline_agents) - |> put_flash(:info, "Agent revoked successfully")} + |> put_flash(:info, "Agent deleted successfully. Devices now fall back to site/org defaults or cloud polling.")} {:error, _} -> - {:noreply, put_flash(socket, :error, "Failed to revoke agent")} + {:noreply, put_flash(socket, :error, "Failed to delete agent")} end end diff --git a/lib/towerops_web/live/agent_live/index.html.heex b/lib/towerops_web/live/agent_live/index.html.heex index f579f89e..af239511 100644 --- a/lib/towerops_web/live/agent_live/index.html.heex +++ b/lib/towerops_web/live/agent_live/index.html.heex @@ -141,16 +141,16 @@ <% else %> - Revoked + Disabled <% end %> diff --git a/test/towerops/agents_test.exs b/test/towerops/agents_test.exs index e7a1c982..22373c56 100644 --- a/test/towerops/agents_test.exs +++ b/test/towerops/agents_test.exs @@ -83,6 +83,222 @@ defmodule Towerops.AgentsTest do end end + describe "delete_agent_token/1" do + setup %{organization: org} do + {:ok, site} = + Towerops.Sites.create_site(%{ + name: "Test Site", + organization_id: org.id + }) + + {:ok, agent_token, _token} = Agents.create_agent_token(org.id, "Test Agent") + + %{site: site, agent_token: agent_token} + end + + test "deletes agent token from database", %{agent_token: agent_token} do + assert {:ok, _} = Agents.delete_agent_token(agent_token.id) + + assert_raise Ecto.NoResultsError, fn -> + Agents.get_agent_token!(agent_token.id) + end + end + + test "removes all direct device assignments", %{ + site: site, + agent_token: agent_token + } do + {:ok, device1} = + Towerops.Devices.create_device(%{ + name: "Device 1", + ip_address: "192.168.1.1", + site_id: site.id + }) + + {:ok, device2} = + Towerops.Devices.create_device(%{ + name: "Device 2", + ip_address: "192.168.1.2", + site_id: site.id + }) + + # Assign both devices to the agent + {:ok, _} = Agents.assign_device_to_agent(agent_token.id, device1.id) + {:ok, _} = Agents.assign_device_to_agent(agent_token.id, device2.id) + + assert Agents.count_assigned_devices(agent_token.id) == 2 + + # Delete the agent + assert {:ok, _} = Agents.delete_agent_token(agent_token.id) + + # Verify assignments are removed + assert Agents.get_device_assignment(device1.id) == nil + assert Agents.get_device_assignment(device2.id) == nil + end + + test "devices fall back to site agent when available", %{ + organization: org, + site: site, + agent_token: agent_token_to_delete + } do + # Create another agent that will be the site default + {:ok, site_agent, _} = Agents.create_agent_token(org.id, "Site Agent") + + # Set site to use the site_agent + {:ok, _site} = Towerops.Sites.update_site(site, %{agent_token_id: site_agent.id}) + + {:ok, device} = + Towerops.Devices.create_device(%{ + name: "Device", + ip_address: "192.168.1.1", + site_id: site.id + }) + + # Directly assign device to agent_to_delete (overriding site default) + {:ok, _} = Agents.assign_device_to_agent(agent_token_to_delete.id, device.id) + + # Device should currently use agent_to_delete + device = Repo.preload(device, site: [organization: :default_agent_token]) + assert Agents.get_effective_agent_token(device) == agent_token_to_delete.id + + # Delete the agent + assert {:ok, _} = Agents.delete_agent_token(agent_token_to_delete.id) + + # Device should now fall back to site agent + device = device |> Repo.reload!() |> Repo.preload(site: [organization: :default_agent_token]) + assert Agents.get_effective_agent_token(device) == site_agent.id + end + + test "devices fall back to organization default when available", %{ + organization: org, + site: site, + agent_token: agent_token_to_delete + } do + # Create another agent that will be the org default + {:ok, org_agent, _} = Agents.create_agent_token(org.id, "Org Agent") + + # Set organization to use the org_agent + {:ok, _org} = + Towerops.Organizations.update_organization(org, %{default_agent_token_id: org_agent.id}) + + {:ok, device} = + Towerops.Devices.create_device(%{ + name: "Device", + ip_address: "192.168.1.1", + site_id: site.id + }) + + # Directly assign device to agent_to_delete (overriding org default) + {:ok, _} = Agents.assign_device_to_agent(agent_token_to_delete.id, device.id) + + # Device should currently use agent_to_delete + device = Repo.preload(device, site: [organization: :default_agent_token]) + assert Agents.get_effective_agent_token(device) == agent_token_to_delete.id + + # Delete the agent + assert {:ok, _} = Agents.delete_agent_token(agent_token_to_delete.id) + + # Device should now fall back to org default + device = device |> Repo.reload!() |> Repo.preload(site: [organization: :default_agent_token]) + assert Agents.get_effective_agent_token(device) == org_agent.id + end + + test "devices have no agent when no fallback exists (cloud polling)", %{ + site: site, + agent_token: agent_token + } do + {:ok, device} = + Towerops.Devices.create_device(%{ + name: "Device", + ip_address: "192.168.1.1", + site_id: site.id + }) + + # Directly assign device to agent (no site or org defaults) + {:ok, _} = Agents.assign_device_to_agent(agent_token.id, device.id) + + # Device should currently use the agent + device = Repo.preload(device, site: [organization: :default_agent_token]) + assert Agents.get_effective_agent_token(device) == agent_token.id + + # Delete the agent + assert {:ok, _} = Agents.delete_agent_token(agent_token.id) + + # Device should now have no agent (cloud polling) + device = device |> Repo.reload!() |> Repo.preload(site: [organization: :default_agent_token]) + assert Agents.get_effective_agent_token(device) == nil + end + + test "removes agent from site defaults when deleting", %{ + organization: org, + site: site, + agent_token: agent_token + } do + # Set site to use this agent + {:ok, _site} = Towerops.Sites.update_site(site, %{agent_token_id: agent_token.id}) + + # Create another agent as fallback + {:ok, fallback_agent, _} = Agents.create_agent_token(org.id, "Fallback Agent") + + {:ok, _org} = + Towerops.Organizations.update_organization(org, %{default_agent_token_id: fallback_agent.id}) + + {:ok, device} = + Towerops.Devices.create_device(%{ + name: "Device", + ip_address: "192.168.1.1", + site_id: site.id + }) + + # Device should use site agent + device = Repo.preload(device, site: [organization: :default_agent_token]) + assert Agents.get_effective_agent_token(device) == agent_token.id + + # Delete the agent + assert {:ok, _} = Agents.delete_agent_token(agent_token.id) + + # Site should no longer have this agent + site = Repo.reload!(site) + assert site.agent_token_id == nil + + # Device should fall back to org default + device = device |> Repo.reload!() |> Repo.preload(site: [organization: :default_agent_token]) + assert Agents.get_effective_agent_token(device) == fallback_agent.id + end + + test "removes agent from organization defaults when deleting", %{ + organization: org, + site: site, + agent_token: agent_token + } do + # Set organization to use this agent + {:ok, _org} = + Towerops.Organizations.update_organization(org, %{default_agent_token_id: agent_token.id}) + + {:ok, device} = + Towerops.Devices.create_device(%{ + name: "Device", + ip_address: "192.168.1.1", + site_id: site.id + }) + + # Device should use org default + device = Repo.preload(device, site: [organization: :default_agent_token]) + assert Agents.get_effective_agent_token(device) == agent_token.id + + # Delete the agent + assert {:ok, _} = Agents.delete_agent_token(agent_token.id) + + # Organization should no longer have this agent as default + org = Repo.reload!(org) + assert org.default_agent_token_id == nil + + # Device should have no agent + device = device |> Repo.reload!() |> Repo.preload(site: [organization: :default_agent_token]) + assert Agents.get_effective_agent_token(device) == nil + end + end + describe "update_agent_token_heartbeat/3" do test "updates last_seen_at and metadata", %{organization: org} do {:ok, agent_token, _token} = Agents.create_agent_token(org.id, "Test Agent") diff --git a/test/towerops_web/live/agent_live/index_test.exs b/test/towerops_web/live/agent_live/index_test.exs index 95b99e04..2a2526de 100644 --- a/test/towerops_web/live/agent_live/index_test.exs +++ b/test/towerops_web/live/agent_live/index_test.exs @@ -119,9 +119,9 @@ defmodule ToweropsWeb.AgentLive.IndexTest do end end - describe "revoke_agent event" do - test "revokes agent token successfully", %{conn: conn, user: user, organization: organization} do - {:ok, agent_token, _token} = Agents.create_agent_token(organization.id, "Agent to Revoke") + describe "delete_agent event" do + test "deletes agent token successfully", %{conn: conn, user: user, organization: organization} do + {:ok, agent_token, _token} = Agents.create_agent_token(organization.id, "Agent to Delete") conn = conn @@ -130,15 +130,13 @@ defmodule ToweropsWeb.AgentLive.IndexTest do {:ok, view, _html} = live(conn) - html = render_click(view, "revoke_agent", %{"id" => agent_token.id}) + html = render_click(view, "delete_agent", %{"id" => agent_token.id}) - assert html =~ "Agent revoked successfully" + assert html =~ "Agent deleted successfully" - # Verify agent was revoked (still in list but with enabled: false) + # Verify agent was deleted (no longer in list) agent_tokens = Agents.list_organization_agent_tokens(organization.id) - refute Enum.empty?(agent_tokens) - revoked_agent = hd(agent_tokens) - refute revoked_agent.enabled + assert Enum.empty?(agent_tokens) end end diff --git a/test/towerops_web/live/agent_live_test.exs b/test/towerops_web/live/agent_live_test.exs index b6c02206..4bddcbed 100644 --- a/test/towerops_web/live/agent_live_test.exs +++ b/test/towerops_web/live/agent_live_test.exs @@ -44,21 +44,20 @@ defmodule ToweropsWeb.AgentLiveTest do assert html =~ "Authentication Token" end - test "revokes agent", %{conn: conn, organization: organization} do + test "deletes agent", %{conn: conn, organization: organization} do {:ok, agent_token, _token} = Agents.create_agent_token(organization.id, "Agent 1") {:ok, view, _html} = live(conn, ~p"/orgs/#{organization.slug}/agents") html = view - |> element("button", "Revoke") + |> element("button", "Delete") |> render_click() - assert html =~ "Revoked" + assert html =~ "Agent deleted successfully" - # Verify agent was actually revoked - updated_token = Towerops.Repo.get!(Agents.AgentToken, agent_token.id) - refute updated_token.enabled + # Verify agent was actually deleted + assert is_nil(Towerops.Repo.get(Agents.AgentToken, agent_token.id)) end test "closes token modal", %{conn: conn, organization: organization} do