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
This commit is contained in:
parent
137f84f5e5
commit
c35a502425
6 changed files with 287 additions and 23 deletions
|
|
@ -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 """
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -141,16 +141,16 @@
|
|||
</button>
|
||||
<button
|
||||
type="button"
|
||||
phx-click="revoke_agent"
|
||||
phx-click="delete_agent"
|
||||
phx-value-id={agent.id}
|
||||
data-confirm="Are you sure you want to revoke this agent? It will no longer be able to authenticate."
|
||||
data-confirm="Are you sure you want to delete this agent? All device assignments will be removed, and devices will fall back to site/organization defaults or cloud polling."
|
||||
class="text-sm font-medium text-red-600 hover:text-red-700 dark:text-red-400 dark:hover:text-red-300"
|
||||
>
|
||||
Revoke
|
||||
Delete
|
||||
</button>
|
||||
</div>
|
||||
<% else %>
|
||||
<span class="text-xs text-zinc-500 dark:text-zinc-400">Revoked</span>
|
||||
<span class="text-xs text-zinc-500 dark:text-zinc-400">Disabled</span>
|
||||
<% end %>
|
||||
</:action>
|
||||
</.table>
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue