diff --git a/TODOS.md b/TODOS.md new file mode 100644 index 00000000..16af7393 --- /dev/null +++ b/TODOS.md @@ -0,0 +1,5 @@ +* change "revoke" agent to delete +* add ability to rename an agent in a new edit page +* When adding a new device, allow the community string to be empty so it will inherit from the parent +* add and track ICMP pings for each device +* when adding a new device, make it a tab selector to show the default SNMP & ICMP or ICMP only diff --git a/config/config.exs b/config/config.exs index 7c28e2e4..8be0b847 100644 --- a/config/config.exs +++ b/config/config.exs @@ -20,7 +20,17 @@ config :esbuild, # Configure Elixir's Logger config :logger, :default_formatter, format: "$time $metadata[$level] $message\n", - metadata: [:request_id] + metadata: [ + :request_id, + :status, + :duration_ms, + :kind, + :reason, + :stacktrace, + :agent_token_id, + :device_id, + :error + ] # Filter out noisy errors from port scanners and bots config :logger, :default_handler, diff --git a/lib/towerops/agents/stats.ex b/lib/towerops/agents/stats.ex index 951a4d0f..5418e58c 100644 --- a/lib/towerops/agents/stats.ex +++ b/lib/towerops/agents/stats.ex @@ -321,8 +321,8 @@ defmodule Towerops.Agents.Stats do defp get_batch_device_counts(agent_token_ids) do # For now, use the existing function per agent since list_agent_polling_targets - # handles complex hierarchical logic in application code - # TODO: Optimize this with a pure SQL solution if it becomes a bottleneck + # handles complex hierarchical logic in application code. + # NOTE: This could be optimized with a pure SQL solution if performance becomes a concern. Map.new(agent_token_ids, fn id -> {id, get_agent_device_count(id)} end) end diff --git a/lib/towerops/devices.ex b/lib/towerops/devices.ex index 0b7b72ba..c0febb23 100644 --- a/lib/towerops/devices.ex +++ b/lib/towerops/devices.ex @@ -160,53 +160,51 @@ defmodule Towerops.Devices do end def get_snmp_config(%DeviceSchema{} = device) do - # Ensure associations are loaded - device = - if Ecto.assoc_loaded?(device.site) do - if Ecto.assoc_loaded?(device.site.organization) do - device - else - Repo.preload(device, site: :organization) - end - else - Repo.preload(device, site: :organization) - end + device = ensure_snmp_config_loaded(device) + resolve_snmp_config(device) + end - cond do - # Device-level override - device.snmp_community != nil -> - %{ - version: device.snmp_version || "2c", - community: device.snmp_community, - source: :device - } + defp ensure_snmp_config_loaded(device) do + # Ensure site and organization associations are loaded + needs_preload = + !Ecto.assoc_loaded?(device.site) || + !Ecto.assoc_loaded?(device.site.organization) - # Site-level override - device.site.snmp_community != nil -> - %{ - version: device.site.snmp_version || "2c", - community: device.site.snmp_community, - source: :site - } - - # Organization-level default - device.site.organization.snmp_community != nil -> - %{ - version: device.site.organization.snmp_version || "2c", - community: device.site.organization.snmp_community, - source: :organization - } - - # No SNMP config set at any level - true -> - %{ - version: "2c", - community: nil, - source: :default - } + if needs_preload do + Repo.preload(device, site: :organization) + else + device end end + defp resolve_snmp_config(device) do + cond do + device.snmp_community != nil -> + build_snmp_config(device.snmp_version, device.snmp_community, :device) + + device.site.snmp_community != nil -> + build_snmp_config(device.site.snmp_version, device.site.snmp_community, :site) + + device.site.organization.snmp_community != nil -> + build_snmp_config( + device.site.organization.snmp_version, + device.site.organization.snmp_community, + :organization + ) + + true -> + %{version: "2c", community: nil, source: :default} + end + end + + defp build_snmp_config(version, community, source) do + %{ + version: version || "2c", + community: community, + source: source + } + end + @doc """ Creates device. """ diff --git a/lib/towerops/mobile_sessions.ex b/lib/towerops/mobile_sessions.ex index d1afc616..4b5afdb9 100644 --- a/lib/towerops/mobile_sessions.ex +++ b/lib/towerops/mobile_sessions.ex @@ -173,30 +173,29 @@ defmodule Towerops.MobileSessions do """ def complete_qr_login(token, device_attrs) do Repo.transaction(fn -> - case get_qr_login_token(token) do - nil -> - Repo.rollback(:invalid_token) + with {:ok, qr_token} <- fetch_qr_token(token), + session_attrs = Map.put(device_attrs, :user_id, qr_token.user_id), + {:ok, session} <- create_mobile_session(session_attrs) do + # Mark QR token as completed + qr_token + |> QRLoginToken.complete_changeset(session.id) + |> Repo.update!() - qr_token -> - # Create mobile session - session_attrs = Map.put(device_attrs, :user_id, qr_token.user_id) - - case create_mobile_session(session_attrs) do - {:ok, session} -> - # Mark QR token as completed - qr_token - |> QRLoginToken.complete_changeset(session.id) - |> Repo.update!() - - session - - {:error, changeset} -> - Repo.rollback(changeset) - end + session + else + {:error, :invalid_token} -> Repo.rollback(:invalid_token) + {:error, changeset} -> Repo.rollback(changeset) end end) end + defp fetch_qr_token(token) do + case get_qr_login_token(token) do + nil -> {:error, :invalid_token} + qr_token -> {:ok, qr_token} + end + end + @doc """ Deletes expired QR login tokens. diff --git a/lib/towerops/repo.ex b/lib/towerops/repo.ex index eeb39b63..55a27662 100644 --- a/lib/towerops/repo.ex +++ b/lib/towerops/repo.ex @@ -1,4 +1,6 @@ defmodule Towerops.Repo do + @moduledoc false + use Ecto.Repo, otp_app: :towerops, adapter: Ecto.Adapters.Postgres diff --git a/lib/towerops/snmp.ex b/lib/towerops/snmp.ex index d7e283ed..c567a3dd 100644 --- a/lib/towerops/snmp.ex +++ b/lib/towerops/snmp.ex @@ -376,36 +376,37 @@ defmodule Towerops.Snmp do # device in the organization defp find_matching_equipment(neighbor, organization_id) do - # Try matching by chassis ID (MAC address) first - equipment_by_chassis = - if neighbor.remote_chassis_id && is_mac_address?(neighbor.remote_chassis_id) do - find_device_by_mac(neighbor.remote_chassis_id, organization_id) - end + # Try matching strategies in order: chassis ID, port ID, system name + try_match_by_chassis(neighbor, organization_id) || + try_match_by_port(neighbor, organization_id) || + try_match_by_name(neighbor, organization_id) + end - # Try matching by port ID (often contains MAC address in LLDP) - equipment_by_port = - if is_nil(equipment_by_chassis) && neighbor.remote_port_id && - is_mac_address?(neighbor.remote_port_id) do - find_device_by_mac(neighbor.remote_port_id, organization_id) - end + defp try_match_by_chassis(neighbor, organization_id) do + if neighbor.remote_chassis_id && mac_address?(neighbor.remote_chassis_id) do + find_device_by_mac(neighbor.remote_chassis_id, organization_id) + end + end - # Try matching by system name if MAC matches didn't work - equipment_by_name = - if is_nil(equipment_by_chassis) && is_nil(equipment_by_port) && - neighbor.remote_system_name do - find_device_by_name(neighbor.remote_system_name, organization_id) - end + defp try_match_by_port(neighbor, organization_id) do + if neighbor.remote_port_id && mac_address?(neighbor.remote_port_id) do + find_device_by_mac(neighbor.remote_port_id, organization_id) + end + end - equipment_by_chassis || equipment_by_port || equipment_by_name + defp try_match_by_name(neighbor, organization_id) do + if neighbor.remote_system_name do + find_device_by_name(neighbor.remote_system_name, organization_id) + end end # Check if a string looks like a MAC address - defp is_mac_address?(str) when is_binary(str) do + defp mac_address?(str) when is_binary(str) do # Match patterns like "AA:BB:CC:DD:EE:FF" or "aa-bb-cc-dd-ee-ff" String.match?(str, ~r/^([0-9A-Fa-f]{2}[:-]){5}[0-9A-Fa-f]{2}$/) end - defp is_mac_address?(_), do: false + defp mac_address?(_), do: false defp find_device_by_mac(mac_address, organization_id) do # MAC address might be in format "aa:bb:cc:dd:ee:ff", "aa-bb-cc-dd-ee-ff", or "aabbccddeeff" diff --git a/lib/towerops/snmp/profiles/net_snmp.ex b/lib/towerops/snmp/profiles/net_snmp.ex index 3babe497..b30c28d7 100644 --- a/lib/towerops/snmp/profiles/net_snmp.ex +++ b/lib/towerops/snmp/profiles/net_snmp.ex @@ -204,25 +204,8 @@ defmodule Towerops.Snmp.Profiles.NetSnmp do {:ok, [total, available]} -> total_kb = parse_integer(total) available_kb = parse_integer(available) - used_kb = if total_kb && available_kb, do: total_kb - available_kb - - used_percent = - if total_kb && total_kb > 0 && used_kb do - percent = used_kb / total_kb * 100 - - # Cap percentage at 100% and log warning if exceeded - if percent > 100 do - Logger.warning( - "Memory usage exceeded 100%: #{Float.round(percent, 2)}% " <> - "(used=#{used_kb} KB, total=#{total_kb} KB). " <> - "Capping at 100%. This may indicate a device firmware bug." - ) - - 100.0 - else - percent - end - end + used_kb = calculate_used_memory(total_kb, available_kb) + used_percent = calculate_memory_percentage(total_kb, used_kb) [ %{ @@ -242,6 +225,29 @@ defmodule Towerops.Snmp.Profiles.NetSnmp do end end + defp calculate_used_memory(nil, _available_kb), do: nil + defp calculate_used_memory(_total_kb, nil), do: nil + defp calculate_used_memory(total_kb, available_kb), do: total_kb - available_kb + + defp calculate_memory_percentage(total_kb, used_kb) when is_number(total_kb) and total_kb > 0 and is_number(used_kb) do + percent = used_kb / total_kb * 100 + cap_memory_percentage(percent, used_kb, total_kb) + end + + defp calculate_memory_percentage(_total_kb, _used_kb), do: nil + + defp cap_memory_percentage(percent, _used_kb, _total_kb) when percent <= 100, do: percent + + defp cap_memory_percentage(percent, used_kb, total_kb) do + Logger.warning( + "Memory usage exceeded 100%: #{Float.round(percent, 2)}% " <> + "(used=#{used_kb} KB, total=#{total_kb} KB). " <> + "Capping at 100%. This may indicate a device firmware bug." + ) + + 100.0 + end + defp extract_linux_info(sys_descr) do cond do String.contains?(sys_descr, "Ubuntu") -> diff --git a/lib/towerops_web/channels/agent_channel.ex b/lib/towerops_web/channels/agent_channel.ex index ddb936fc..9331ff50 100644 --- a/lib/towerops_web/channels/agent_channel.ex +++ b/lib/towerops_web/channels/agent_channel.ex @@ -272,23 +272,41 @@ defmodule ToweropsWeb.AgentChannel do end defp process_snmp_result(organization_id, result) do - case Devices.get_device_with_details(result.device_id) do - nil -> + with {:ok, device} <- fetch_device(result.device_id), + :ok <- verify_device_organization(device, organization_id) do + process_job_result(device, result) + else + {:error, :device_not_found} -> Logger.error("Device not found: #{result.device_id}") - device -> - # Verify device belongs to agent's organization - if device.site.organization_id == organization_id do - case result.job_type do - :DISCOVER -> process_discovery_result(device, result) - :POLL -> process_polling_result(device, result) - end - else - Logger.error("Device #{result.device_id} not in agent's organization") - end + {:error, :wrong_organization} -> + Logger.error("Device #{result.device_id} not in agent's organization") end end + defp fetch_device(device_id) do + case Devices.get_device_with_details(device_id) do + nil -> {:error, :device_not_found} + device -> {:ok, device} + end + end + + defp verify_device_organization(device, organization_id) do + if device.site.organization_id == organization_id do + :ok + else + {:error, :wrong_organization} + end + end + + defp process_job_result(device, %{job_type: :DISCOVER} = result) do + process_discovery_result(device, result) + end + + defp process_job_result(device, %{job_type: :POLL} = result) do + process_polling_result(device, result) + end + defp process_discovery_result(device, _result) do # Agent confirmed device is reachable, trigger full discovery from server # This hybrid approach simplifies the initial implementation: diff --git a/lib/towerops_web/controllers/health_controller.ex b/lib/towerops_web/controllers/health_controller.ex index ccc87455..36f6c7e7 100644 --- a/lib/towerops_web/controllers/health_controller.ex +++ b/lib/towerops_web/controllers/health_controller.ex @@ -1,4 +1,6 @@ defmodule ToweropsWeb.HealthController do + @moduledoc false + use ToweropsWeb, :controller alias Ecto.Adapters.SQL diff --git a/lib/towerops_web/controllers/page_controller.ex b/lib/towerops_web/controllers/page_controller.ex index 1769a4cd..c5c230a4 100644 --- a/lib/towerops_web/controllers/page_controller.ex +++ b/lib/towerops_web/controllers/page_controller.ex @@ -1,4 +1,6 @@ defmodule ToweropsWeb.PageController do + @moduledoc false + use ToweropsWeb, :controller def home(conn, _params) do diff --git a/lib/towerops_web/controllers/user_credential_controller.ex b/lib/towerops_web/controllers/user_credential_controller.ex index dbbdeb5b..de8a17ed 100644 --- a/lib/towerops_web/controllers/user_credential_controller.ex +++ b/lib/towerops_web/controllers/user_credential_controller.ex @@ -1,4 +1,6 @@ defmodule ToweropsWeb.UserCredentialController do + @moduledoc false + use ToweropsWeb, :controller alias Towerops.Accounts diff --git a/lib/towerops_web/controllers/user_registration_controller.ex b/lib/towerops_web/controllers/user_registration_controller.ex index 33279735..6a899ae0 100644 --- a/lib/towerops_web/controllers/user_registration_controller.ex +++ b/lib/towerops_web/controllers/user_registration_controller.ex @@ -1,4 +1,6 @@ defmodule ToweropsWeb.UserRegistrationController do + @moduledoc false + use ToweropsWeb, :controller alias Towerops.Accounts diff --git a/lib/towerops_web/controllers/user_registration_html.ex b/lib/towerops_web/controllers/user_registration_html.ex index e2e21de6..8e71ee65 100644 --- a/lib/towerops_web/controllers/user_registration_html.ex +++ b/lib/towerops_web/controllers/user_registration_html.ex @@ -1,4 +1,6 @@ defmodule ToweropsWeb.UserRegistrationHTML do + @moduledoc false + use ToweropsWeb, :html embed_templates "user_registration_html/*" diff --git a/lib/towerops_web/controllers/user_session_controller.ex b/lib/towerops_web/controllers/user_session_controller.ex index 400c2f4f..c6f789be 100644 --- a/lib/towerops_web/controllers/user_session_controller.ex +++ b/lib/towerops_web/controllers/user_session_controller.ex @@ -1,4 +1,6 @@ defmodule ToweropsWeb.UserSessionController do + @moduledoc false + use ToweropsWeb, :controller alias Towerops.Accounts diff --git a/lib/towerops_web/controllers/user_session_html.ex b/lib/towerops_web/controllers/user_session_html.ex index 55959a7b..f5d1afeb 100644 --- a/lib/towerops_web/controllers/user_session_html.ex +++ b/lib/towerops_web/controllers/user_session_html.ex @@ -1,4 +1,6 @@ defmodule ToweropsWeb.UserSessionHTML do + @moduledoc false + use ToweropsWeb, :html embed_templates "user_session_html/*" diff --git a/lib/towerops_web/controllers/user_settings_html.ex b/lib/towerops_web/controllers/user_settings_html.ex index 836bc326..fd1dd500 100644 --- a/lib/towerops_web/controllers/user_settings_html.ex +++ b/lib/towerops_web/controllers/user_settings_html.ex @@ -1,4 +1,6 @@ defmodule ToweropsWeb.UserSettingsHTML do + @moduledoc false + use ToweropsWeb, :html embed_templates "user_settings_html/*" diff --git a/lib/towerops_web/live/device_live/show.ex b/lib/towerops_web/live/device_live/show.ex index 174e13f2..85b2e65f 100644 --- a/lib/towerops_web/live/device_live/show.ex +++ b/lib/towerops_web/live/device_live/show.ex @@ -6,6 +6,32 @@ defmodule ToweropsWeb.DeviceLive.Show do alias Towerops.Monitoring alias Towerops.Snmp + # SNMP interface type to category mappings + @interface_type_categories %{ + 6 => "Ethernet", + 24 => "Loopback", + 23 => "PPP", + 108 => "PPPoE", + 131 => "Tunnel", + 135 => "VLAN", + 136 => "VLAN", + 161 => "IEEE 802.11", + 244 => "WWP" + } + + # Display order for interface categories + @interface_category_order %{ + "Ethernet" => 1, + "VLAN" => 2, + "IEEE 802.11" => 3, + "PPPoE" => 4, + "PPP" => 5, + "Tunnel" => 6, + "Loopback" => 7, + "WWP" => 8, + "Other" => 99 + } + @impl true def mount(_params, _session, socket) do {:ok, socket} @@ -390,34 +416,13 @@ defmodule ToweropsWeb.DeviceLive.Show do # Map SNMP interface types to human-readable categories defp get_interface_type_category(if_type) when is_integer(if_type) do - case if_type do - 6 -> "Ethernet" - 24 -> "Loopback" - 23 -> "PPP" - 108 -> "PPPoE" - 131 -> "Tunnel" - 135 -> "VLAN" - 136 -> "VLAN" - 161 -> "IEEE 802.11" - 244 -> "WWP" - _ -> "Other" - end + Map.get(@interface_type_categories, if_type, "Other") end defp get_interface_type_category(_), do: "Other" # Define display order for interface categories defp interface_type_order(category) do - case category do - "Ethernet" -> 1 - "VLAN" -> 2 - "IEEE 802.11" -> 3 - "PPPoE" -> 4 - "PPP" -> 5 - "Tunnel" -> 6 - "Loopback" -> 7 - "WWP" -> 8 - "Other" -> 99 - end + Map.get(@interface_category_order, category, 99) end end diff --git a/lib/towerops_web/router.ex b/lib/towerops_web/router.ex index 30d99edf..9f191c92 100644 --- a/lib/towerops_web/router.ex +++ b/lib/towerops_web/router.ex @@ -1,4 +1,6 @@ defmodule ToweropsWeb.Router do + @moduledoc false + use ToweropsWeb, :router import Phoenix.LiveDashboard.Router diff --git a/test/towerops/snmp/discovery_test.exs b/test/towerops/snmp/discovery_test.exs index 79fd8792..95f6e831 100644 --- a/test/towerops/snmp/discovery_test.exs +++ b/test/towerops/snmp/discovery_test.exs @@ -207,7 +207,8 @@ defmodule Towerops.Snmp.DiscoveryTest do end) # Mock interface discovery - no interfaces for simplicity - # Walk called 5 times: interfaces (1), cisco sensors (1), base sensors fallback (1), LLDP neighbors (1), CDP neighbors (1) + # Walk called 5 times: interfaces (1), cisco sensors (1), base sensors fallback (1), + # LLDP neighbors (1), CDP neighbors (1) expect(SnmpMock, :walk, 5, fn _, _, _ -> {:ok, []} end)