diff --git a/config/config.exs b/config/config.exs index cc4dbd84..a03ee106 100644 --- a/config/config.exs +++ b/config/config.exs @@ -29,6 +29,7 @@ config :logger, :default_formatter, format: "$time $metadata[$level] $message\n", metadata: [ :request_id, + :remote_ip, :status, :duration_ms, :kind, diff --git a/config/dev.exs b/config/dev.exs index 63874a0c..75bd50b4 100644 --- a/config/dev.exs +++ b/config/dev.exs @@ -1,5 +1,8 @@ import Config +# Disable Honeybadger in development +config :honeybadger, environment_name: :dev, exclude_envs: [:dev] + # Do not include metadata nor timestamps in development logs config :logger, :default_formatter, format: "[$level] $message\n" diff --git a/lib/towerops/devices.ex b/lib/towerops/devices.ex index bf550e93..d2977d50 100644 --- a/lib/towerops/devices.ex +++ b/lib/towerops/devices.ex @@ -10,6 +10,7 @@ defmodule Towerops.Devices do alias Towerops.Repo alias Towerops.Workers.DeviceMonitorWorker alias Towerops.Workers.DevicePollerWorker + alias Towerops.Workers.DiscoveryWorker @doc """ Returns the list of devices for a site. @@ -289,13 +290,15 @@ defmodule Towerops.Devices do def update_device(%DeviceSchema{} = device, attrs) do old_monitoring = device.monitoring_enabled old_snmp = device.snmp_enabled + old_snmp_version = device.snmp_version + old_snmp_port = device.snmp_port case device |> DeviceSchema.changeset(attrs) |> Repo.update() do {:ok, updated_device} = result -> _ = handle_monitoring_changes(updated_device, old_monitoring) - _ = handle_snmp_changes(updated_device, old_snmp) + _ = handle_snmp_changes(updated_device, old_snmp, old_snmp_version, old_snmp_port) result error -> @@ -362,6 +365,8 @@ defmodule Towerops.Devices do cond do device.monitoring_enabled && !old_monitoring -> DeviceMonitorWorker.start_monitoring(device.id) + # Trigger discovery when monitoring is enabled (if SNMP is also enabled) + if device.snmp_enabled, do: DiscoveryWorker.enqueue(device.id) !device.monitoring_enabled && old_monitoring -> DeviceMonitorWorker.stop_monitoring(device.id) @@ -371,19 +376,45 @@ defmodule Towerops.Devices do end end - defp handle_snmp_changes(device, old_snmp) do + defp handle_snmp_changes(device, old_snmp, old_snmp_version, old_snmp_port) do + should_discover = should_trigger_discovery?(device, old_snmp, old_snmp_version, old_snmp_port) + cond do device.snmp_enabled && !old_snmp -> DevicePollerWorker.start_polling(device.id) + if should_discover, do: DiscoveryWorker.enqueue(device.id) !device.snmp_enabled && old_snmp -> DevicePollerWorker.stop_polling(device.id) + should_discover -> + DiscoveryWorker.enqueue(device.id) + true -> :ok end end + defp should_trigger_discovery?(device, old_snmp, old_snmp_version, old_snmp_port) do + cond do + # SNMP was just enabled + device.snmp_enabled && !old_snmp -> + true + + # SNMP version changed while enabled + device.snmp_enabled && device.snmp_version != old_snmp_version -> + true + + # SNMP port changed while enabled + device.snmp_enabled && device.snmp_port != old_snmp_port -> + true + + # Otherwise no discovery needed + true -> + false + end + end + ## Events @doc """ diff --git a/lib/towerops/devices/device.ex b/lib/towerops/devices/device.ex index 81eca932..3d3fa7f3 100644 --- a/lib/towerops/devices/device.ex +++ b/lib/towerops/devices/device.ex @@ -31,7 +31,7 @@ defmodule Towerops.Devices.Device do field :check_interval_seconds, :integer, default: 300 # SNMP fields - field :snmp_enabled, :boolean, default: false + field :snmp_enabled, :boolean, default: true field :snmp_version, :string, default: "2c" field :snmp_community, :string field :snmp_port, :integer, default: 161 diff --git a/lib/towerops/sites/site.ex b/lib/towerops/sites/site.ex index 01bc2e50..f3552330 100644 --- a/lib/towerops/sites/site.ex +++ b/lib/towerops/sites/site.ex @@ -77,6 +77,10 @@ defmodule Towerops.Sites.Site do |> foreign_key_constraint(:organization_id) |> foreign_key_constraint(:agent_token_id) |> foreign_key_constraint(:parent_site_id) + |> unique_constraint(:name, + name: :sites_organization_id_name_index, + message: "has already been taken for this organization" + ) |> validate_not_circular_parent() end diff --git a/lib/towerops/workers/discovery_worker.ex b/lib/towerops/workers/discovery_worker.ex index c9ebd21b..3ea0ebfc 100644 --- a/lib/towerops/workers/discovery_worker.ex +++ b/lib/towerops/workers/discovery_worker.ex @@ -34,25 +34,39 @@ defmodule Towerops.Workers.DiscoveryWorker do def perform(%Oban.Job{args: %{"device_id" => device_id}}) do case Devices.get_device_with_details(device_id) do nil -> - Logger.error("Device #{device_id} not found") - {:error, :device_not_found} + Logger.warning("Device #{device_id} not found, discarding discovery job") + :discard device -> case get_assigned_agent(device) do {agent_token_id, source} when not is_nil(agent_token_id) -> - agent = Agents.get_agent_token!(agent_token_id) - poller_type = if agent.cloud_poller, do: "cloud poller", else: "user's poller" + # Try to get the agent token, falling back if it doesn't exist + try do + agent = Agents.get_agent_token!(agent_token_id) + poller_type = if agent.cloud_poller, do: "cloud poller", else: "user's poller" - Logger.info( - "Starting SNMP discovery for device #{device_id} using #{poller_type} '#{agent.name}' (#{source} assignment)", - device_id: device_id, - agent_token_id: agent_token_id, - agent_name: agent.name, - cloud_poller: agent.cloud_poller, - source: source - ) + Logger.info( + "Starting SNMP discovery for device #{device_id} using #{poller_type} '#{agent.name}' (#{source} assignment)", + device_id: device_id, + agent_token_id: agent_token_id, + agent_name: agent.name, + cloud_poller: agent.cloud_poller, + source: source + ) - attempt_agent_discovery(device, agent_token_id, [agent_token_id]) + attempt_agent_discovery(device, agent_token_id, [agent_token_id]) + rescue + Ecto.NoResultsError -> + Logger.warning( + "Agent token #{agent_token_id} no longer exists (#{source} assignment), falling back to cloud pollers", + device_id: device_id, + agent_token_id: agent_token_id, + source: source + ) + + # Fall back to cloud poller discovery + attempt_cloud_poller_discovery(device, []) + end {nil, :none} -> Logger.info( diff --git a/lib/towerops_web/controllers/api/v1/devices_controller.ex b/lib/towerops_web/controllers/api/v1/devices_controller.ex index 886ebd8f..8a04c048 100644 --- a/lib/towerops_web/controllers/api/v1/devices_controller.ex +++ b/lib/towerops_web/controllers/api/v1/devices_controller.ex @@ -8,6 +8,7 @@ defmodule ToweropsWeb.Api.V1.DevicesController do use ToweropsWeb, :controller alias Towerops.Devices + alias Towerops.Workers.DiscoveryWorker @doc """ GET /api/v1/devices @@ -82,6 +83,11 @@ defmodule ToweropsWeb.Api.V1.DevicesController do :ok -> case Devices.create_device(device_params) do {:ok, device} -> + # Trigger SNMP discovery if enabled + if device.snmp_enabled do + DiscoveryWorker.enqueue(device.id) + end + conn |> put_status(:created) |> json(format_device(device)) diff --git a/lib/towerops_web/endpoint.ex b/lib/towerops_web/endpoint.ex index 7001e3c2..5d9733d0 100644 --- a/lib/towerops_web/endpoint.ex +++ b/lib/towerops_web/endpoint.ex @@ -45,6 +45,7 @@ defmodule ToweropsWeb.Endpoint do cookie_key: "request_logger" plug Plug.RequestId + plug ToweropsWeb.Plugs.RemoteIpLogger plug Plug.Telemetry, event_prefix: [:phoenix, :endpoint], diff --git a/lib/towerops_web/live/agent_live/index.ex b/lib/towerops_web/live/agent_live/index.ex index 02bd8119..aa07b71c 100644 --- a/lib/towerops_web/live/agent_live/index.ex +++ b/lib/towerops_web/live/agent_live/index.ex @@ -47,6 +47,7 @@ defmodule ToweropsWeb.AgentLive.Index do |> assign(:agent_tokens, agent_tokens) |> assign(:cloud_pollers, cloud_pollers) |> assign(:global_default_cloud_poller_id, global_default_cloud_poller_id) + |> assign(:selected_global_default, global_default_cloud_poller_id || "") |> assign(:device_counts, equipment_counts) |> assign(:agent_health_stats, agent_health_stats) |> assign(:assignment_breakdown, assignment_breakdown) @@ -132,11 +133,26 @@ defmodule ToweropsWeb.AgentLive.Index do case Agents.delete_agent_token(id) do {:ok, _} -> organization = socket.assigns.current_organization + + # If the deleted agent was the global default, clear it + if Scope.superuser?(socket.assigns.current_scope) && + socket.assigns.global_default_cloud_poller_id == id do + Settings.set_global_default_cloud_poller(nil) + end + agent_tokens = Agents.list_organization_agent_tokens(organization.id) # Refresh cloud pollers list if superadmin (in case a cloud poller was deleted) cloud_pollers = load_cloud_pollers_if_superuser(socket.assigns.current_scope) + # Reload global default cloud poller ID + global_default_cloud_poller_id = + if Scope.superuser?(socket.assigns.current_scope) do + Settings.get_global_default_cloud_poller() + else + socket.assigns.global_default_cloud_poller_id + end + # Recalculate device counts after deletion equipment_counts = Map.new(agent_tokens, fn token -> @@ -154,6 +170,8 @@ defmodule ToweropsWeb.AgentLive.Index do socket |> assign(:agent_tokens, agent_tokens) |> assign(:cloud_pollers, cloud_pollers) + |> assign(:global_default_cloud_poller_id, global_default_cloud_poller_id) + |> assign(:selected_global_default, global_default_cloud_poller_id || "") |> assign(:device_counts, equipment_counts) |> assign(:agent_health_stats, agent_health_stats) |> assign(:assignment_breakdown, assignment_breakdown) @@ -166,16 +184,27 @@ defmodule ToweropsWeb.AgentLive.Index do end @impl true - def handle_event("set_global_default", %{"agent_token_id" => agent_token_id}, socket) do + def handle_event("update_selected_global_default", %{"agent_token_id" => agent_token_id}, socket) do + # Just update the selected value in the dropdown, don't save yet + {:noreply, assign(socket, :selected_global_default, agent_token_id)} + end + + @impl true + def handle_event("save_global_default", _params, socket) do current_scope = socket.assigns.current_scope if Scope.superuser?(current_scope) do + agent_token_id = socket.assigns.selected_global_default # Handle empty string as nil agent_token_id = if agent_token_id == "", do: nil, else: agent_token_id - case Settings.set_global_default_cloud_poller(agent_token_id) do - {:ok, _} -> - handle_global_default_success(socket, agent_token_id) + # Validate that the agent exists if not nil + case validate_and_save_global_default(agent_token_id) do + {:ok, validated_id} -> + handle_global_default_success(socket, validated_id) + + {:error, :agent_not_found} -> + {:noreply, put_flash(socket, :error, "Selected agent no longer exists. Please choose another agent.")} {:error, _} -> {:noreply, put_flash(socket, :error, "Failed to update global default cloud poller")} @@ -267,6 +296,27 @@ defmodule ToweropsWeb.AgentLive.Index do else: "Agent created successfully" end + defp validate_and_save_global_default(nil) do + # Clearing the default is always valid + case Settings.set_global_default_cloud_poller(nil) do + {:ok, _} -> {:ok, nil} + error -> error + end + end + + defp validate_and_save_global_default(agent_token_id) do + # Verify the agent exists before saving + %Agents.AgentToken{} = Agents.get_agent_token!(agent_token_id) + + case Settings.set_global_default_cloud_poller(agent_token_id) do + {:ok, _} -> {:ok, agent_token_id} + error -> error + end + rescue + Ecto.NoResultsError -> + {:error, :agent_not_found} + end + defp handle_global_default_success(socket, agent_token_id) do message = if agent_token_id, @@ -276,6 +326,7 @@ defmodule ToweropsWeb.AgentLive.Index do {:noreply, socket |> assign(:global_default_cloud_poller_id, agent_token_id) + |> assign(:selected_global_default, agent_token_id || "") |> put_flash(:info, message)} 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 a548e5e0..827f6f51 100644 --- a/lib/towerops_web/live/agent_live/index.html.heex +++ b/lib/towerops_web/live/agent_live/index.html.heex @@ -272,44 +272,73 @@ Fallback agent for organizations without a default agent configured. Devices with no assignment at any level will use this agent.

- <.form for={%{}} phx-change="set_global_default" class="max-w-md"> -
- - + <.form for={%{}} phx-change="update_selected_global_default" class="max-w-md"> +
+
+ + +
<%= if @global_default_cloud_poller_id do %> <% selected_poller = Enum.find(@cloud_pollers, &(&1.id == @global_default_cloud_poller_id)) %> <%= if selected_poller do %> -

+

<.icon name="hero-check-circle" class="h-4 w-4 inline" /> Currently using: {selected_poller.name}

+ <% else %> +

+ <.icon name="hero-exclamation-triangle" class="h-4 w-4 inline" /> + Warning: Selected agent no longer exists. Please choose a new agent. +

<% end %> <% else %> -

- <.icon name="hero-exclamation-triangle" class="h-4 w-4 inline" /> - No global default configured. Devices without assignments will use cloud polling (nil). +

+ <.icon name="hero-information-circle" class="h-4 w-4 inline" /> + No global default configured. Devices without assignments will use direct Phoenix cluster polling.

<% end %> + +
+ <.button + type="button" + phx-click="save_global_default" + variant="primary" + disabled={@selected_global_default == (@global_default_cloud_poller_id || "")} + > + <.icon name="hero-check" class="h-4 w-4" /> Save Changes + + <%= if @selected_global_default != (@global_default_cloud_poller_id || "") do %> + + <% end %> +
diff --git a/lib/towerops_web/live/device_live/show.ex b/lib/towerops_web/live/device_live/show.ex index 4c9ebb83..42fd9f0e 100644 --- a/lib/towerops_web/live/device_live/show.ex +++ b/lib/towerops_web/live/device_live/show.ex @@ -247,6 +247,15 @@ defmodule ToweropsWeb.DeviceLive.Show do source: source, last_seen_at: agent_token.last_seen_at } + rescue + Ecto.NoResultsError -> + # Agent token no longer exists, fall back to cloud polling display + %{ + type: :cloud, + name: "Cloud Polling (agent not found)", + source: nil, + last_seen_at: nil + } end defp calculate_metrics(checks, _equipment) do diff --git a/lib/towerops_web/plugs/remote_ip_logger.ex b/lib/towerops_web/plugs/remote_ip_logger.ex new file mode 100644 index 00000000..338c494e --- /dev/null +++ b/lib/towerops_web/plugs/remote_ip_logger.ex @@ -0,0 +1,58 @@ +defmodule ToweropsWeb.Plugs.RemoteIpLogger do + @moduledoc """ + Plug that extracts the remote IP address and adds it to Logger metadata. + + When behind a proxy/load balancer (like Traefik in Kubernetes), the real client IP + is in the X-Forwarded-For or X-Real-IP headers. This plug extracts the IP and + adds it to Logger metadata so it appears in all logs for the request. + """ + + import Plug.Conn + + require Logger + + def init(opts), do: opts + + def call(conn, _opts) do + remote_ip = get_remote_ip(conn) + + # Add to Logger metadata so it appears in all logs for this request + Logger.metadata(remote_ip: remote_ip) + + conn + end + + defp get_remote_ip(conn) do + # Try X-Forwarded-For first (set by Traefik and other proxies) + case get_req_header(conn, "x-forwarded-for") do + [forwarded | _] -> + # X-Forwarded-For can be a comma-separated list; take the first (original client) + forwarded + |> String.split(",") + |> List.first() + |> String.trim() + + [] -> + get_fallback_ip(conn) + end + end + + defp get_fallback_ip(conn) do + # Try X-Real-IP header + case get_req_header(conn, "x-real-ip") do + [real_ip | _] -> + real_ip + + [] -> + format_remote_ip(conn.remote_ip) + end + end + + defp format_remote_ip({a, b, c, d}), do: "#{a}.#{b}.#{c}.#{d}" + defp format_remote_ip({a, b, c, d, e, f, g, h}), do: format_ipv6({a, b, c, d, e, f, g, h}) + defp format_remote_ip(_), do: "unknown" + + defp format_ipv6({a, b, c, d, e, f, g, h}) do + Enum.map_join([a, b, c, d, e, f, g, h], ":", &Integer.to_string(&1, 16)) + end +end diff --git a/priv/repo/migrations/20260126201725_set_snmp_enabled_default_true.exs b/priv/repo/migrations/20260126201725_set_snmp_enabled_default_true.exs new file mode 100644 index 00000000..5ce0dcb0 --- /dev/null +++ b/priv/repo/migrations/20260126201725_set_snmp_enabled_default_true.exs @@ -0,0 +1,9 @@ +defmodule Towerops.Repo.Migrations.SetSnmpEnabledDefaultTrue do + use Ecto.Migration + + def change do + alter table(:devices) do + modify :snmp_enabled, :boolean, default: true, null: false + end + end +end diff --git a/priv/repo/migrations/20260126201941_add_unique_site_name_per_org.exs b/priv/repo/migrations/20260126201941_add_unique_site_name_per_org.exs new file mode 100644 index 00000000..077f397d --- /dev/null +++ b/priv/repo/migrations/20260126201941_add_unique_site_name_per_org.exs @@ -0,0 +1,9 @@ +defmodule Towerops.Repo.Migrations.AddUniqueSiteNamePerOrg do + use Ecto.Migration + + def change do + create unique_index(:sites, [:organization_id, :name], + name: :sites_organization_id_name_index + ) + end +end diff --git a/test/towerops/devices_test.exs b/test/towerops/devices_test.exs index fd7f2a02..cafdd773 100644 --- a/test/towerops/devices_test.exs +++ b/test/towerops/devices_test.exs @@ -778,6 +778,229 @@ defmodule Towerops.EquipmentTest do end end + describe "automatic rediscovery on device updates" do + import Towerops.AccountsFixtures + + setup do + user = user_fixture() + {:ok, organization} = Towerops.Organizations.create_organization(%{name: "Test Org"}, user.id) + + {:ok, site} = + Towerops.Sites.create_site(%{ + name: "Test Site", + organization_id: organization.id + }) + + %{organization: organization, site: site, user: user} + end + + test "triggers discovery when snmp_enabled changes from false to true", %{site: site} do + {:ok, device} = + Devices.create_device(%{ + name: "Router", + ip_address: "192.168.1.1", + site_id: site.id, + snmp_enabled: false + }) + + # Clear any existing jobs + Towerops.Repo.delete_all(Oban.Job) + + # Update to enable SNMP + {:ok, _updated} = Devices.update_device(device, %{snmp_enabled: true}) + + # Check that a discovery job was enqueued + jobs = + Oban.Job + |> Towerops.Repo.all() + |> Enum.filter(&(&1.worker == "Towerops.Workers.DiscoveryWorker")) + + assert [job] = jobs + assert job.args["device_id"] == device.id + end + + test "triggers discovery when snmp_version changes while enabled", %{site: site} do + {:ok, device} = + Devices.create_device(%{ + name: "Router", + ip_address: "192.168.1.1", + site_id: site.id, + snmp_enabled: true, + snmp_version: "2c" + }) + + # Clear any existing jobs + Towerops.Repo.delete_all(Oban.Job) + + # Update SNMP version + {:ok, _updated} = Devices.update_device(device, %{snmp_version: "1"}) + + # Check that a discovery job was enqueued + jobs = + Oban.Job + |> Towerops.Repo.all() + |> Enum.filter(&(&1.worker == "Towerops.Workers.DiscoveryWorker")) + + assert [job] = jobs + assert job.args["device_id"] == device.id + end + + test "triggers discovery when snmp_port changes while enabled", %{site: site} do + {:ok, device} = + Devices.create_device(%{ + name: "Router", + ip_address: "192.168.1.1", + site_id: site.id, + snmp_enabled: true, + snmp_port: 161 + }) + + # Clear any existing jobs + Towerops.Repo.delete_all(Oban.Job) + + # Update SNMP port + {:ok, _updated} = Devices.update_device(device, %{snmp_port: 1161}) + + # Check that a discovery job was enqueued + jobs = + Oban.Job + |> Towerops.Repo.all() + |> Enum.filter(&(&1.worker == "Towerops.Workers.DiscoveryWorker")) + + assert [job] = jobs + assert job.args["device_id"] == device.id + end + + test "triggers discovery when monitoring_enabled changes to true with SNMP enabled", %{ + site: site + } do + {:ok, device} = + Devices.create_device(%{ + name: "Router", + ip_address: "192.168.1.1", + site_id: site.id, + snmp_enabled: true, + monitoring_enabled: false + }) + + # Clear any existing jobs + Towerops.Repo.delete_all(Oban.Job) + + # Enable monitoring + {:ok, _updated} = Devices.update_device(device, %{monitoring_enabled: true}) + + # Check that a discovery job was enqueued + jobs = + Oban.Job + |> Towerops.Repo.all() + |> Enum.filter(&(&1.worker == "Towerops.Workers.DiscoveryWorker")) + + assert [job] = jobs + assert job.args["device_id"] == device.id + end + + test "does not trigger discovery when monitoring_enabled changes but SNMP disabled", %{ + site: site + } do + {:ok, device} = + Devices.create_device(%{ + name: "Router", + ip_address: "192.168.1.1", + site_id: site.id, + snmp_enabled: false, + monitoring_enabled: false + }) + + # Clear any existing jobs + Towerops.Repo.delete_all(Oban.Job) + + # Enable monitoring (but SNMP is disabled) + {:ok, _updated} = Devices.update_device(device, %{monitoring_enabled: true}) + + # Check that NO discovery job was enqueued + jobs = + Oban.Job + |> Towerops.Repo.all() + |> Enum.filter(&(&1.worker == "Towerops.Workers.DiscoveryWorker")) + + assert [] = jobs + end + + test "does not trigger discovery when SNMP is disabled", %{site: site} do + {:ok, device} = + Devices.create_device(%{ + name: "Router", + ip_address: "192.168.1.1", + site_id: site.id, + snmp_enabled: true, + snmp_version: "2c" + }) + + # Clear any existing jobs + Towerops.Repo.delete_all(Oban.Job) + + # Disable SNMP + {:ok, _updated} = Devices.update_device(device, %{snmp_enabled: false}) + + # Check that NO discovery job was enqueued (SNMP is disabled) + jobs = + Oban.Job + |> Towerops.Repo.all() + |> Enum.filter(&(&1.worker == "Towerops.Workers.DiscoveryWorker")) + + assert [] = jobs + end + + test "does not trigger discovery for unrelated field changes", %{site: site} do + {:ok, device} = + Devices.create_device(%{ + name: "Router", + ip_address: "192.168.1.1", + site_id: site.id, + snmp_enabled: true + }) + + # Clear any existing jobs + Towerops.Repo.delete_all(Oban.Job) + + # Update unrelated fields (name, description) + {:ok, _updated} = Devices.update_device(device, %{name: "Updated Router"}) + + # Check that NO discovery job was enqueued + jobs = + Oban.Job + |> Towerops.Repo.all() + |> Enum.filter(&(&1.worker == "Towerops.Workers.DiscoveryWorker")) + + assert [] = jobs + end + + test "does not trigger discovery when snmp_version changes while disabled", %{site: site} do + {:ok, device} = + Devices.create_device(%{ + name: "Router", + ip_address: "192.168.1.1", + site_id: site.id, + snmp_enabled: false, + snmp_version: "2c" + }) + + # Clear any existing jobs + Towerops.Repo.delete_all(Oban.Job) + + # Update SNMP version (but SNMP is disabled) + {:ok, _updated} = Devices.update_device(device, %{snmp_version: "1"}) + + # Check that NO discovery job was enqueued (SNMP is disabled) + jobs = + Oban.Job + |> Towerops.Repo.all() + |> Enum.filter(&(&1.worker == "Towerops.Workers.DiscoveryWorker")) + + assert [] = jobs + end + end + describe "property-based tests" do import Towerops.AccountsFixtures diff --git a/test/towerops/sites_test.exs b/test/towerops/sites_test.exs index 1d9f7f2a..e5a917a6 100644 --- a/test/towerops/sites_test.exs +++ b/test/towerops/sites_test.exs @@ -162,6 +162,35 @@ defmodule Towerops.SitesTest do assert "must be 1, 2c, or 3" in errors_on(changeset).snmp_version end + test "create_site/1 prevents duplicate site names within same organization", %{ + organization: organization + } do + # Create first site + {:ok, _site} = Sites.create_site(%{name: "Duplicate Site", organization_id: organization.id}) + + # Try to create second site with same name in same organization + assert {:error, changeset} = + Sites.create_site(%{name: "Duplicate Site", organization_id: organization.id}) + + assert "has already been taken for this organization" in errors_on(changeset).name + end + + test "create_site/1 allows duplicate site names across different organizations" do + user1 = user_fixture() + user2 = user_fixture() + {:ok, org1} = Towerops.Organizations.create_organization(%{name: "Org 1"}, user1.id) + {:ok, org2} = Towerops.Organizations.create_organization(%{name: "Org 2"}, user2.id) + + # Create site in first org + assert {:ok, site1} = Sites.create_site(%{name: "Main Office", organization_id: org1.id}) + + # Should be able to create site with same name in different org + assert {:ok, site2} = Sites.create_site(%{name: "Main Office", organization_id: org2.id}) + + assert site1.name == site2.name + assert site1.organization_id != site2.organization_id + end + test "create_site/1 prevents circular parent reference", %{organization: organization} do {:ok, site} = Sites.create_site(%{name: "Test Site", organization_id: organization.id}) diff --git a/test/towerops_web/live/agent_live/index_test.exs b/test/towerops_web/live/agent_live/index_test.exs index 7f64cb76..e4c93128 100644 --- a/test/towerops_web/live/agent_live/index_test.exs +++ b/test/towerops_web/live/agent_live/index_test.exs @@ -22,7 +22,7 @@ defmodule ToweropsWeb.AgentLive.IndexTest do conn = conn |> log_in_user(user) - |> get("/orgs/#{organization.slug}/agents") + |> get("/agents") {:ok, _view, html} = live(conn) @@ -31,19 +31,19 @@ defmodule ToweropsWeb.AgentLive.IndexTest do assert html =~ agent_token2.name end - test "shows empty state when no agents exist", %{conn: conn, user: user, organization: organization} do + test "shows empty state when no agents exist", %{conn: conn, user: user, organization: _organization} do conn = conn |> log_in_user(user) - |> get("/orgs/#{organization.slug}/agents") + |> get("/agents") {:ok, _view, html} = live(conn) assert html =~ "Remote Agents" end - test "requires authentication", %{conn: conn, organization: organization} do - conn = get(conn, "/orgs/#{organization.slug}/agents") + test "requires authentication", %{conn: conn} do + conn = get(conn, "/agents") assert redirected_to(conn) == ~p"/users/log-in" end end @@ -53,7 +53,7 @@ defmodule ToweropsWeb.AgentLive.IndexTest do conn = conn |> log_in_user(user) - |> get("/orgs/#{organization.slug}/agents") + |> get("/agents") {:ok, view, _html} = live(conn) @@ -69,11 +69,11 @@ defmodule ToweropsWeb.AgentLive.IndexTest do assert hd(agent_tokens).name == "New Agent" end - test "creates agent with flat params format", %{conn: conn, user: user, organization: organization} do + test "creates agent with flat params format", %{conn: conn, user: user, organization: _organization} do conn = conn |> log_in_user(user) - |> get("/orgs/#{organization.slug}/agents") + |> get("/agents") {:ok, view, _html} = live(conn) @@ -84,11 +84,11 @@ defmodule ToweropsWeb.AgentLive.IndexTest do assert html =~ "Flat Agent" end - test "shows error on creation failure", %{conn: conn, user: user, organization: organization} do + test "shows error on creation failure", %{conn: conn, user: user, organization: _organization} do conn = conn |> log_in_user(user) - |> get("/orgs/#{organization.slug}/agents") + |> get("/agents") {:ok, view, _html} = live(conn) @@ -100,11 +100,11 @@ defmodule ToweropsWeb.AgentLive.IndexTest do end describe "close_token_modal event" do - test "closes the token modal", %{conn: conn, user: user, organization: organization} do + test "closes the token modal", %{conn: conn, user: user, organization: _organization} do conn = conn |> log_in_user(user) - |> get("/orgs/#{organization.slug}/agents") + |> get("/agents") {:ok, view, _html} = live(conn) @@ -126,7 +126,7 @@ defmodule ToweropsWeb.AgentLive.IndexTest do conn = conn |> log_in_user(user) - |> get("/orgs/#{organization.slug}/agents") + |> get("/agents") {:ok, view, _html} = live(conn) @@ -144,7 +144,7 @@ defmodule ToweropsWeb.AgentLive.IndexTest do test "shows Never connected for agents without last_seen_at", %{ conn: conn, user: user, - organization: organization + organization: _organization } do {:ok, agent_token, _token} = Agents.create_agent_token(organization.id, "Never Seen") @@ -154,7 +154,7 @@ defmodule ToweropsWeb.AgentLive.IndexTest do conn = conn |> log_in_user(user) - |> get("/orgs/#{organization.slug}/agents") + |> get("/agents") {:ok, _view, html} = live(conn) @@ -165,7 +165,7 @@ defmodule ToweropsWeb.AgentLive.IndexTest do test "shows agent status information in the page", %{ conn: conn, user: user, - organization: organization + organization: _organization } do {:ok, agent_token, _token} = Agents.create_agent_token(organization.id, "Status Agent") @@ -177,7 +177,7 @@ defmodule ToweropsWeb.AgentLive.IndexTest do conn = conn |> log_in_user(user) - |> get("/orgs/#{organization.slug}/agents") + |> get("/agents") {:ok, _view, html} = live(conn) @@ -188,11 +188,11 @@ defmodule ToweropsWeb.AgentLive.IndexTest do end describe "handle_params" do - test "handles :index action", %{conn: conn, user: user, organization: organization} do + test "handles :index action", %{conn: conn, user: user, organization: _organization} do conn = conn |> log_in_user(user) - |> get("/orgs/#{organization.slug}/agents") + |> get("/agents") {:ok, view, _html} = live(conn) @@ -202,7 +202,7 @@ defmodule ToweropsWeb.AgentLive.IndexTest do end describe "cloud poller creation - superadmin only" do - setup %{user: _user, organization: organization} do + setup %{organization: organization} do # Create a superadmin user superadmin = user_fixture() @@ -225,12 +225,12 @@ defmodule ToweropsWeb.AgentLive.IndexTest do test "superadmin can create cloud poller", %{ conn: conn, superadmin: superadmin, - organization: organization + organization: _organization } do conn = conn |> log_in_user(superadmin) - |> get("/orgs/#{organization.slug}/agents") + |> get("/agents") {:ok, view, html} = live(conn) @@ -257,12 +257,12 @@ defmodule ToweropsWeb.AgentLive.IndexTest do test "non-superadmin cannot create cloud poller", %{ conn: conn, user: user, - organization: organization + organization: _organization } do conn = conn |> log_in_user(user) - |> get("/orgs/#{organization.slug}/agents") + |> get("/agents") {:ok, view, html} = live(conn) @@ -289,12 +289,12 @@ defmodule ToweropsWeb.AgentLive.IndexTest do test "superadmin can create regular agent by not checking cloud poller", %{ conn: conn, superadmin: superadmin, - organization: organization + organization: _organization } do conn = conn |> log_in_user(superadmin) - |> get("/orgs/#{organization.slug}/agents") + |> get("/agents") {:ok, view, _html} = live(conn) @@ -321,7 +321,7 @@ defmodule ToweropsWeb.AgentLive.IndexTest do end describe "cloud poller visibility" do - setup %{user: _user, organization: organization} do + setup %{organization: organization} do # Create a superadmin user superadmin = user_fixture() @@ -351,7 +351,7 @@ defmodule ToweropsWeb.AgentLive.IndexTest do test "superadmin sees cloud pollers in separate section", %{ conn: conn, superadmin: superadmin, - organization: organization, + organization: _organization, cloud1: cloud1, cloud2: cloud2, org_agent: org_agent @@ -359,7 +359,7 @@ defmodule ToweropsWeb.AgentLive.IndexTest do conn = conn |> log_in_user(superadmin) - |> get("/orgs/#{organization.slug}/agents") + |> get("/agents") {:ok, _view, html} = live(conn) @@ -380,7 +380,7 @@ defmodule ToweropsWeb.AgentLive.IndexTest do test "non-superadmin does not see cloud pollers section", %{ conn: conn, user: user, - organization: organization, + organization: _organization, cloud1: cloud1, cloud2: cloud2, org_agent: org_agent @@ -388,7 +388,7 @@ defmodule ToweropsWeb.AgentLive.IndexTest do conn = conn |> log_in_user(user) - |> get("/orgs/#{organization.slug}/agents") + |> get("/agents") {:ok, _view, html} = live(conn) @@ -436,7 +436,7 @@ defmodule ToweropsWeb.AgentLive.IndexTest do conn = conn |> log_in_user(superadmin) - |> get("/orgs/#{organization.slug}/agents") + |> get("/agents") {:ok, view, _html} = live(conn) @@ -458,7 +458,7 @@ defmodule ToweropsWeb.AgentLive.IndexTest do conn = conn |> log_in_user(superadmin) - |> get("/orgs/#{organization.slug}/agents") + |> get("/agents") {:ok, view, _html} = live(conn) @@ -477,7 +477,7 @@ defmodule ToweropsWeb.AgentLive.IndexTest do conn = conn |> log_in_user(superadmin) - |> get("/orgs/#{organization.slug}/agents") + |> get("/agents") {:ok, view, _html} = live(conn) @@ -488,4 +488,159 @@ defmodule ToweropsWeb.AgentLive.IndexTest do assert html =~ cloud_poller.token or html =~ "docker run" or html =~ "Setup" end end + + describe "global default cloud poller management" do + setup %{organization: organization} do + # Create a superadmin user + superadmin = user_fixture() + + superadmin = + superadmin + |> Ecto.Changeset.change(%{is_superuser: true}) + |> Towerops.Repo.update!() + + # Add superadmin to the organization + {:ok, _membership} = + Towerops.Organizations.create_membership(%{ + organization_id: organization.id, + user_id: superadmin.id, + role: "admin" + }) + + {:ok, cloud_poller, _} = Agents.create_cloud_poller("Test Cloud Poller") + + %{superadmin: superadmin, cloud_poller: cloud_poller} + end + + test "superadmin can set global default cloud poller with validation", %{ + conn: conn, + superadmin: superadmin, + organization: organization, + cloud_poller: cloud_poller + } do + conn = + conn + |> log_in_user(superadmin) + |> get("/agents") + + {:ok, view, html} = live(conn) + + # Should see global default section + assert html =~ "Global Default Cloud Poller" + + # Update dropdown selection (doesn't save yet) + render_change(view, "update_selected_global_default", %{ + "agent_token_id" => cloud_poller.id + }) + + # Click save button + html = render_click(view, "save_global_default", %{}) + + assert html =~ "Global default cloud poller set successfully" + + # Verify it was saved + assert Towerops.Settings.get_global_default_cloud_poller() == cloud_poller.id + end + + test "superadmin cannot save non-existent agent as global default", %{ + conn: conn, + superadmin: superadmin, + organization: _organization + } do + conn = + conn + |> log_in_user(superadmin) + |> get("/agents") + + {:ok, view, _html} = live(conn) + + fake_id = Ecto.UUID.generate() + + # Update dropdown to fake ID + render_change(view, "update_selected_global_default", %{"agent_token_id" => fake_id}) + + # Attempt to save + html = render_click(view, "save_global_default", %{}) + + assert html =~ "Selected agent no longer exists" + + # Verify it was NOT saved + refute Towerops.Settings.get_global_default_cloud_poller() == fake_id + end + + test "superadmin can clear global default cloud poller", %{ + conn: conn, + superadmin: superadmin, + organization: organization, + cloud_poller: cloud_poller + } do + # First set a global default + Towerops.Settings.set_global_default_cloud_poller(cloud_poller.id) + + conn = + conn + |> log_in_user(superadmin) + |> get("/agents") + + {:ok, view, html} = live(conn) + + # Should show current default + assert html =~ "Currently using" + + # Select "No global default" + render_change(view, "update_selected_global_default", %{"agent_token_id" => ""}) + + # Save + html = render_click(view, "save_global_default", %{}) + + assert html =~ "Global default cloud poller cleared" + + # Verify it was cleared + assert Towerops.Settings.get_global_default_cloud_poller() == nil + end + + test "deleting cloud poller clears it from global default", %{ + conn: conn, + superadmin: superadmin, + organization: organization, + cloud_poller: cloud_poller + } do + # Set as global default + Towerops.Settings.set_global_default_cloud_poller(cloud_poller.id) + + conn = + conn + |> log_in_user(superadmin) + |> get("/agents") + + {:ok, view, _html} = live(conn) + + # Delete the cloud poller + html = render_click(view, "delete_agent", %{"id" => cloud_poller.id}) + + assert html =~ "Agent deleted successfully" + + # Verify global default was cleared (CASCADE) + assert Towerops.Settings.get_global_default_cloud_poller() == nil + + # Verify the UI reflects this + assert html =~ "No global default" or html =~ "direct Phoenix" + end + + test "non-superadmin cannot see global default section", %{ + conn: conn, + user: user, + organization: _organization + } do + conn = + conn + |> log_in_user(user) + |> get("/agents") + + {:ok, _view, html} = live(conn) + + # Should NOT see global default section + refute html =~ "Global Default Cloud Poller" + end + end end