From d68991c48404b7c28c458e0af8f0375a69f21595 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Wed, 21 Jan 2026 13:07:30 -0600 Subject: [PATCH] fix discovery issue --- lib/towerops/snmp/discovery.ex | 138 +++++++++++++++++++++++++++++++-- 1 file changed, 133 insertions(+), 5 deletions(-) diff --git a/lib/towerops/snmp/discovery.ex b/lib/towerops/snmp/discovery.ex index 29e85382..5312aa67 100644 --- a/lib/towerops/snmp/discovery.ex +++ b/lib/towerops/snmp/discovery.ex @@ -31,6 +31,7 @@ defmodule Towerops.Snmp.Discovery do alias Towerops.Snmp.Profiles.NetSnmp alias Towerops.Snmp.Profiles.Ubiquiti alias Towerops.Snmp.Sensor + alias Towerops.Snmp.Vlan require Logger @@ -130,7 +131,8 @@ defmodule Towerops.Snmp.Discovery do {:ok, device_info} <- build_device_info(client_opts, system_info, profile), {:ok, interfaces} <- discover_interfaces_with_timeout(client_opts, profile, timeouts), {:ok, sensors} <- discover_sensors_with_timeout(client_opts, profile, timeouts), - {:ok, discovered_device} <- save_discovery_results(device, device_info, interfaces, sensors), + {:ok, vlans} <- discover_vlans_with_timeout(client_opts, profile, timeouts), + {:ok, discovered_device} <- save_discovery_results(device, device_info, interfaces, sensors, vlans), {:ok, neighbors} <- discover_neighbors_with_timeout(client_opts, discovered_device.interfaces, timeouts), :ok <- save_neighbors(discovered_device.device_id, neighbors) do _ = update_device_discovery_time(device) @@ -189,6 +191,16 @@ defmodule Towerops.Snmp.Discovery do ) end + # VLAN discovery with timeout - falls back to empty list on timeout + defp discover_vlans_with_timeout(client_opts, profile, timeouts) do + DeferredDiscovery.slow_check( + client_opts, + fn -> discover_vlans(client_opts, profile) end, + timeout: timeouts[:slow], + default: [] + ) + end + @doc """ Runs discovery for all SNMP-enabled devices in an organization. Returns a summary of successful and failed discoveries. @@ -411,20 +423,43 @@ defmodule Towerops.Snmp.Discovery do end end + @spec discover_vlans(Client.connection_opts(), profile()) :: {:ok, [map()]} + defp discover_vlans(client_opts, {:dynamic, _profile}) do + # Dynamic profiles use Base VLAN discovery + {:ok, vlans} = Base.discover_vlans(client_opts) + Logger.debug("Discovered #{length(vlans)} VLANs") + {:ok, vlans} + end + + defp discover_vlans(client_opts, profile) when is_atom(profile) do + if function_exported?(profile, :discover_vlans, 1) do + {:ok, vlans} = profile.discover_vlans(client_opts) + Logger.debug("Discovered #{length(vlans)} VLANs") + {:ok, vlans} + else + # Fall back to Base profile for VLAN discovery + {:ok, vlans} = Base.discover_vlans(client_opts) + Logger.debug("Discovered #{length(vlans)} VLANs using Base profile") + {:ok, vlans} + end + end + @spec save_discovery_results( DeviceSchema.t(), device_info(), [interface_data()], - [sensor_data()] + [sensor_data()], + [map()] ) :: {:ok, Device.t()} | {:error, term()} - defp save_discovery_results(device, device_info, interfaces, sensors) do + defp save_discovery_results(device, device_info, interfaces, sensors, vlans) do Repo.transaction(fn -> # Upsert Device snmp_device = upsert_device(device, device_info) - # Sync interfaces and sensors (preserving historical data) + # Sync interfaces, sensors, and VLANs (preserving historical data) synced_interfaces = sync_interfaces(snmp_device, interfaces) _ = sync_sensors(snmp_device, sensors) + _ = sync_vlans(snmp_device, vlans) # Return device with interfaces loaded and device_id added # Use snmp_device.device_id which references the Equipment table @@ -437,7 +472,12 @@ defmodule Towerops.Snmp.Discovery do Logger.info("upsert_device called with device_info keys: #{inspect(Map.keys(device_info))}") # Extract raw discovery data if present (using underscore prefix to distinguish from regular fields) - raw_discovery_data = Map.get(device_info, :_raw_discovery_data) + # Sanitize to ensure all binary data is JSON-encodable + raw_discovery_data = + device_info + |> Map.get(:_raw_discovery_data) + |> sanitize_for_json() + last_discovery_at = Map.get(device_info, :_last_discovery_at, DateTime.utc_now()) Logger.info("Extracted raw_discovery_data: #{inspect(raw_discovery_data != nil)}") @@ -558,6 +598,50 @@ defmodule Towerops.Snmp.Discovery do end) end + @spec sync_vlans(Device.t(), [map()]) :: [Vlan.t()] + defp sync_vlans(device, discovered_vlans) do + # Get existing VLANs for this device, indexed by vlan_id + existing_vlans = + from(v in Vlan, where: v.snmp_device_id == ^device.id) + |> Repo.all() + |> Map.new(&{&1.vlan_id, &1}) + + # Get the set of discovered vlan_ids + discovered_vlan_ids = MapSet.new(discovered_vlans, & &1.vlan_id) + + # Delete VLANs that no longer exist on the device + existing_vlan_ids = MapSet.new(Map.keys(existing_vlans)) + removed_vlan_ids = MapSet.difference(existing_vlan_ids, discovered_vlan_ids) + + if MapSet.size(removed_vlan_ids) != 0 do + removed_list = MapSet.to_list(removed_vlan_ids) + + Repo.delete_all( + from v in Vlan, + where: v.snmp_device_id == ^device.id and v.vlan_id in ^removed_list + ) + + Logger.debug("Deleted #{MapSet.size(removed_vlan_ids)} removed VLANs") + end + + # Upsert each discovered VLAN + Enum.map(discovered_vlans, fn vlan_data -> + case Map.get(existing_vlans, vlan_data.vlan_id) do + nil -> + # New VLAN - insert + %Vlan{} + |> Vlan.changeset(Map.put(vlan_data, :snmp_device_id, device.id)) + |> Repo.insert!() + + existing -> + # Existing VLAN - update + existing + |> Vlan.changeset(vlan_data) + |> Repo.update!() + end + end) + end + @spec update_device_discovery_time(DeviceSchema.t()) :: {:ok, DeviceSchema.t()} | {:error, Ecto.Changeset.t()} defp update_device_discovery_time(device) do @@ -641,4 +725,48 @@ defmodule Towerops.Snmp.Discovery do {:device_event, event_attrs} ) end + + # Sanitizes data structures to ensure they are JSON-encodable + # Converts non-UTF8 binary strings to base64 representation + @spec sanitize_for_json(term()) :: term() + defp sanitize_for_json(nil), do: nil + + # Handle DateTime and other structs before generic map clause + defp sanitize_for_json(%DateTime{} = dt), do: DateTime.to_iso8601(dt) + defp sanitize_for_json(%NaiveDateTime{} = dt), do: NaiveDateTime.to_iso8601(dt) + defp sanitize_for_json(%Date{} = d), do: Date.to_iso8601(d) + defp sanitize_for_json(%Time{} = t), do: Time.to_iso8601(t) + + # Generic struct handling - convert to map first, then sanitize + defp sanitize_for_json(%{__struct__: _} = struct) do + struct + |> Map.from_struct() + |> sanitize_for_json() + end + + defp sanitize_for_json(map) when is_map(map) do + Map.new(map, fn {k, v} -> {sanitize_for_json(k), sanitize_for_json(v)} end) + end + + defp sanitize_for_json(list) when is_list(list) do + Enum.map(list, &sanitize_for_json/1) + end + + defp sanitize_for_json(binary) when is_binary(binary) do + if String.printable?(binary) do + binary + else + # Convert non-printable binary to base64 with marker + # Use Elixir.Base to avoid conflict with Towerops.Snmp.Profiles.Base alias + %{"_binary_base64" => Elixir.Base.encode64(binary)} + end + end + + defp sanitize_for_json(tuple) when is_tuple(tuple) do + tuple + |> Tuple.to_list() + |> Enum.map(&sanitize_for_json/1) + end + + defp sanitize_for_json(other), do: other end