From 1cf6f327ea55b5f7f3754bba5ca80d7053e00a31 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Wed, 21 Jan 2026 10:36:19 -0600 Subject: [PATCH] feat: add IP address discovery (Phase 1.3) - Create snmp_ip_addresses table with migration - Add IpAddress schema with ip_type validation (ipv4/ipv6) - Add prefix_length validation based on ip_type - Implement discover_ip_addresses/1 in Base profile - Extract IP addresses from IP-MIB ipAdEntTable - Support multiple IPs per interface with subnet masks - Add 12 schema tests and 4 discovery tests --- lib/towerops/snmp/ip_address.ex | 87 +++++++ lib/towerops/snmp/profiles/base.ex | 119 +++++++++ ...0260121163134_create_snmp_ip_addresses.exs | 28 ++ test/towerops/snmp/ip_address_test.exs | 245 ++++++++++++++++++ test/towerops/snmp/profiles/base_test.exs | 84 ++++++ 5 files changed, 563 insertions(+) create mode 100644 lib/towerops/snmp/ip_address.ex create mode 100644 priv/repo/migrations/20260121163134_create_snmp_ip_addresses.exs create mode 100644 test/towerops/snmp/ip_address_test.exs diff --git a/lib/towerops/snmp/ip_address.ex b/lib/towerops/snmp/ip_address.ex new file mode 100644 index 00000000..58a19dcb --- /dev/null +++ b/lib/towerops/snmp/ip_address.ex @@ -0,0 +1,87 @@ +defmodule Towerops.Snmp.IpAddress do + @moduledoc """ + SNMP IP address schema for tracking IP addresses assigned to interfaces. + + IP addresses are discovered via: + - IP-MIB ipAddrTable (IPv4) + - IPV6-MIB ipv6AddrTable (IPv6) + + Each interface can have multiple IP addresses (primary and secondary/alias). + """ + use Ecto.Schema + + import Ecto.Changeset + + alias Ecto.Association.NotLoaded + alias Towerops.Snmp.Interface + + @valid_ip_types ~w(ipv4 ipv6) + + @primary_key {:id, :binary_id, autogenerate: true} + @foreign_key_type :binary_id + schema "snmp_ip_addresses" do + field :ip_address, :string + field :subnet_mask, :string + field :prefix_length, :integer + field :ip_type, :string + field :is_primary, :boolean, default: false + field :last_checked_at, :utc_datetime + field :metadata, :map, default: %{} + + belongs_to :snmp_interface, Interface + + timestamps(type: :utc_datetime) + end + + @type t :: %__MODULE__{ + id: Ecto.UUID.t(), + ip_address: String.t(), + subnet_mask: String.t() | nil, + prefix_length: integer() | nil, + ip_type: String.t(), + is_primary: boolean(), + last_checked_at: DateTime.t() | nil, + metadata: map(), + snmp_interface_id: Ecto.UUID.t(), + snmp_interface: NotLoaded.t() | Interface.t(), + inserted_at: DateTime.t(), + updated_at: DateTime.t() + } + + @doc false + def changeset(ip_address, attrs) do + ip_address + |> cast(attrs, [ + :snmp_interface_id, + :ip_address, + :subnet_mask, + :prefix_length, + :ip_type, + :is_primary, + :last_checked_at, + :metadata + ]) + |> validate_required([:snmp_interface_id, :ip_address, :ip_type]) + |> validate_inclusion(:ip_type, @valid_ip_types) + |> validate_prefix_length() + |> unique_constraint([:snmp_interface_id, :ip_address]) + |> foreign_key_constraint(:snmp_interface_id) + end + + defp validate_prefix_length(changeset) do + case get_field(changeset, :prefix_length) do + nil -> + changeset + + prefix -> + ip_type = get_field(changeset, :ip_type) + max_prefix = if ip_type == "ipv6", do: 128, else: 32 + + if prefix >= 0 and prefix <= max_prefix do + changeset + else + add_error(changeset, :prefix_length, "must be between 0 and #{max_prefix}") + end + end + end +end diff --git a/lib/towerops/snmp/profiles/base.ex b/lib/towerops/snmp/profiles/base.ex index 84271782..3eb884d2 100644 --- a/lib/towerops/snmp/profiles/base.ex +++ b/lib/towerops/snmp/profiles/base.ex @@ -77,6 +77,12 @@ defmodule Towerops.Snmp.Profiles.Base do dot1q_vlan_static_row_status: "1.3.6.1.2.1.17.7.1.4.3.1.5" } + @ip_mib_oids %{ + # IP-MIB - IPv4 address discovery + ip_ad_ent_if_index: "1.3.6.1.2.1.4.20.1.2", + ip_ad_ent_net_mask: "1.3.6.1.2.1.4.20.1.3" + } + # ENTITY-MIB physical class values to track for state sensors # 6 = powerSupply, 7 = fan @state_sensor_classes [6, 7] @@ -354,6 +360,58 @@ defmodule Towerops.Snmp.Profiles.Base do end end + @doc """ + Discovers IP addresses from IP-MIB. + Returns a list of IP address maps with ip_address, if_index, subnet_mask, and prefix_length. + """ + @spec discover_ip_addresses(Client.connection_opts()) :: {:ok, [map()]} + def discover_ip_addresses(client_opts) do + # Walk IP address to interface mapping + case Client.walk(client_opts, @ip_mib_oids.ip_ad_ent_if_index) do + {:ok, if_index_results} when is_map(if_index_results) and map_size(if_index_results) > 0 -> + do_discover_ip_addresses(client_opts, if_index_results) + + {:ok, _} -> + Logger.debug("No IP addresses found in IP-MIB") + {:ok, []} + + {:error, reason} -> + Logger.debug("IP-MIB walk failed: #{inspect(reason)}") + {:ok, []} + end + end + + defp do_discover_ip_addresses(client_opts, if_index_results) do + # Fetch subnet masks + mask_map = + case Client.walk(client_opts, @ip_mib_oids.ip_ad_ent_net_mask) do + {:ok, results} when is_map(results) -> build_ip_mask_map(results) + _ -> %{} + end + + # Build IP address list + ip_addresses = + if_index_results + |> Enum.map(fn {oid, if_index} -> + ip_address = extract_ip_from_oid(oid) + subnet_mask = Map.get(mask_map, ip_address) + prefix_length = subnet_mask_to_prefix(subnet_mask) + + %{ + ip_address: ip_address, + if_index: if_index, + subnet_mask: subnet_mask, + prefix_length: prefix_length, + ip_type: "ipv4", + last_checked_at: DateTime.utc_now() + } + end) + |> Enum.reject(fn ip -> ip.ip_address == nil end) + + Logger.debug("Discovered #{length(ip_addresses)} IP addresses from IP-MIB") + {:ok, ip_addresses} + end + @doc """ Identifies device manufacturer and model from sysDescr and sysObjectID. Can be overridden by vendor-specific profiles. @@ -712,6 +770,67 @@ defmodule Towerops.Snmp.Profiles.Base do defp vlan_row_status_to_status(3), do: "suspended" defp vlan_row_status_to_status(_), do: "active" + # IP address helper functions + + defp build_ip_mask_map(walk_map) when is_map(walk_map) do + Map.new(walk_map, fn {oid, value} -> + ip_address = extract_ip_from_oid(oid) + mask = format_ip_address(value) + {ip_address, mask} + end) + end + + defp build_ip_mask_map(_), do: %{} + + defp extract_ip_from_oid(oid) when is_binary(oid) do + # OID format: base_oid.ip1.ip2.ip3.ip4 + # Extract last 4 octets as IP address + parts = String.split(oid, ".") + + if length(parts) >= 4 do + parts + |> Enum.take(-4) + |> Enum.join(".") + end + rescue + _ -> nil + end + + defp extract_ip_from_oid(_), do: nil + + defp format_ip_address({a, b, c, d}) when is_integer(a) do + "#{a}.#{b}.#{c}.#{d}" + end + + defp format_ip_address(value) when is_binary(value), do: value + defp format_ip_address(_), do: nil + + defp subnet_mask_to_prefix(nil), do: nil + + defp subnet_mask_to_prefix(mask) when is_binary(mask) do + case String.split(mask, ".") do + [a, b, c, d] -> + [a, b, c, d] + |> Enum.map(&String.to_integer/1) + |> Enum.map(&count_bits/1) + |> Enum.sum() + + _ -> + nil + end + rescue + _ -> nil + end + + defp subnet_mask_to_prefix(_), do: nil + + defp count_bits(octet) when is_integer(octet) do + # Count the number of 1 bits in the octet + octet + |> Integer.digits(2) + |> Enum.count(&(&1 == 1)) + end + @doc """ Collects raw debug data for troubleshooting. Includes system OIDs and interface/sensor tables. diff --git a/priv/repo/migrations/20260121163134_create_snmp_ip_addresses.exs b/priv/repo/migrations/20260121163134_create_snmp_ip_addresses.exs new file mode 100644 index 00000000..b521f488 --- /dev/null +++ b/priv/repo/migrations/20260121163134_create_snmp_ip_addresses.exs @@ -0,0 +1,28 @@ +defmodule Towerops.Repo.Migrations.CreateSnmpIpAddresses do + use Ecto.Migration + + def change do + create table(:snmp_ip_addresses, primary_key: false) do + add :id, :binary_id, primary_key: true + + add :snmp_interface_id, + references(:snmp_interfaces, type: :binary_id, on_delete: :delete_all), + null: false + + add :ip_address, :string, null: false + add :subnet_mask, :string + add :prefix_length, :integer + add :ip_type, :string, null: false, default: "ipv4" + add :is_primary, :boolean, default: false + add :last_checked_at, :utc_datetime + add :metadata, :map, default: %{} + + timestamps(type: :utc_datetime) + end + + create index(:snmp_ip_addresses, [:snmp_interface_id]) + create unique_index(:snmp_ip_addresses, [:snmp_interface_id, :ip_address]) + create index(:snmp_ip_addresses, [:ip_type]) + create index(:snmp_ip_addresses, [:is_primary]) + end +end diff --git a/test/towerops/snmp/ip_address_test.exs b/test/towerops/snmp/ip_address_test.exs new file mode 100644 index 00000000..0d675a0d --- /dev/null +++ b/test/towerops/snmp/ip_address_test.exs @@ -0,0 +1,245 @@ +defmodule Towerops.Snmp.IpAddressTest do + use Towerops.DataCase, async: true + + import Towerops.AccountsFixtures + + alias Towerops.Snmp.Device + alias Towerops.Snmp.Interface + alias Towerops.Snmp.IpAddress + + 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 + }) + + {:ok, device_schema} = + Towerops.Devices.create_device(%{ + name: "Test Router", + ip_address: "192.168.1.1", + snmp_enabled: true, + snmp_version: "2c", + snmp_community: "public", + snmp_port: 161, + site_id: site.id + }) + + snmp_device = + %Device{} + |> Device.changeset(%{ + device_id: device_schema.id, + sys_name: "test-router", + sys_descr: "Test Router" + }) + |> Repo.insert!() + + interface = + %Interface{} + |> Interface.changeset(%{ + snmp_device_id: snmp_device.id, + if_index: 1, + if_descr: "eth0", + if_type: 6, + if_admin_status: "up", + if_oper_status: "up" + }) + |> Repo.insert!() + + %{device: device_schema, snmp_device: snmp_device, interface: interface} + end + + describe "changeset/2" do + test "valid changeset with all required fields", %{interface: interface} do + attrs = %{ + snmp_interface_id: interface.id, + ip_address: "192.168.1.100", + ip_type: "ipv4" + } + + changeset = IpAddress.changeset(%IpAddress{}, attrs) + assert changeset.valid? + end + + test "valid changeset with all fields", %{interface: interface} do + attrs = %{ + snmp_interface_id: interface.id, + ip_address: "192.168.1.100", + subnet_mask: "255.255.255.0", + prefix_length: 24, + ip_type: "ipv4", + is_primary: true, + last_checked_at: DateTime.utc_now(), + metadata: %{"source" => "ip-mib"} + } + + changeset = IpAddress.changeset(%IpAddress{}, attrs) + assert changeset.valid? + assert get_field(changeset, :ip_address) == "192.168.1.100" + assert get_field(changeset, :subnet_mask) == "255.255.255.0" + assert get_field(changeset, :prefix_length) == 24 + assert get_field(changeset, :is_primary) == true + end + + test "valid changeset with IPv6 address", %{interface: interface} do + attrs = %{ + snmp_interface_id: interface.id, + ip_address: "2001:db8::1", + prefix_length: 64, + ip_type: "ipv6" + } + + changeset = IpAddress.changeset(%IpAddress{}, attrs) + assert changeset.valid? + assert get_field(changeset, :ip_type) == "ipv6" + end + + test "invalid changeset without snmp_interface_id" do + attrs = %{ + ip_address: "192.168.1.100", + ip_type: "ipv4" + } + + changeset = IpAddress.changeset(%IpAddress{}, attrs) + refute changeset.valid? + assert "can't be blank" in errors_on(changeset).snmp_interface_id + end + + test "invalid changeset without ip_address", %{interface: interface} do + attrs = %{ + snmp_interface_id: interface.id, + ip_type: "ipv4" + } + + changeset = IpAddress.changeset(%IpAddress{}, attrs) + refute changeset.valid? + assert "can't be blank" in errors_on(changeset).ip_address + end + + test "invalid changeset without ip_type", %{interface: interface} do + attrs = %{ + snmp_interface_id: interface.id, + ip_address: "192.168.1.100" + } + + changeset = IpAddress.changeset(%IpAddress{}, attrs) + refute changeset.valid? + assert "can't be blank" in errors_on(changeset).ip_type + end + + test "invalid ip_type value", %{interface: interface} do + attrs = %{ + snmp_interface_id: interface.id, + ip_address: "192.168.1.100", + ip_type: "invalid" + } + + changeset = IpAddress.changeset(%IpAddress{}, attrs) + refute changeset.valid? + assert "is invalid" in errors_on(changeset).ip_type + end + + test "valid ip_type values", %{interface: interface} do + for ip_type <- ~w(ipv4 ipv6) do + attrs = %{ + snmp_interface_id: interface.id, + ip_address: + if(ip_type == "ipv4", do: "192.168.1.#{:rand.uniform(254)}", else: "2001:db8::#{:rand.uniform(100)}"), + ip_type: ip_type + } + + changeset = IpAddress.changeset(%IpAddress{}, attrs) + assert changeset.valid?, "ip_type '#{ip_type}' should be valid" + end + end + + test "validates prefix_length range for IPv4", %{interface: interface} do + for invalid_prefix <- [-1, 33, 128] do + attrs = %{ + snmp_interface_id: interface.id, + ip_address: "192.168.1.100", + ip_type: "ipv4", + prefix_length: invalid_prefix + } + + changeset = IpAddress.changeset(%IpAddress{}, attrs) + refute changeset.valid?, "prefix_length #{invalid_prefix} should be invalid for IPv4" + end + end + + test "accepts valid prefix_length for IPv4", %{interface: interface} do + for valid_prefix <- [0, 8, 16, 24, 32] do + attrs = %{ + snmp_interface_id: interface.id, + ip_address: "192.168.1.100", + ip_type: "ipv4", + prefix_length: valid_prefix + } + + changeset = IpAddress.changeset(%IpAddress{}, attrs) + assert changeset.valid?, "prefix_length #{valid_prefix} should be valid for IPv4" + end + end + end + + describe "unique constraint" do + test "prevents duplicate IP addresses on same interface", %{interface: interface} do + attrs = %{ + snmp_interface_id: interface.id, + ip_address: "192.168.1.100", + ip_type: "ipv4" + } + + # Insert first IP + changeset = IpAddress.changeset(%IpAddress{}, attrs) + assert {:ok, _ip} = Repo.insert(changeset) + + # Attempt to insert duplicate + changeset2 = IpAddress.changeset(%IpAddress{}, attrs) + + assert {:error, changeset} = Repo.insert(changeset2) + errors = errors_on(changeset) + + assert "has already been taken" in Map.get(errors, :ip_address, []) or + "has already been taken" in Map.get(errors, :snmp_interface_id, []) + end + + test "allows same IP address on different interfaces", %{snmp_device: snmp_device, interface: interface} do + # Create second interface + interface2 = + %Interface{} + |> Interface.changeset(%{ + snmp_device_id: snmp_device.id, + if_index: 2, + if_descr: "eth1", + if_type: 6, + if_admin_status: "up", + if_oper_status: "up" + }) + |> Repo.insert!() + + attrs1 = %{ + snmp_interface_id: interface.id, + ip_address: "192.168.1.100", + ip_type: "ipv4" + } + + attrs2 = %{ + snmp_interface_id: interface2.id, + ip_address: "192.168.1.100", + ip_type: "ipv4" + } + + changeset1 = IpAddress.changeset(%IpAddress{}, attrs1) + assert {:ok, _ip1} = Repo.insert(changeset1) + + changeset2 = IpAddress.changeset(%IpAddress{}, attrs2) + assert {:ok, _ip2} = Repo.insert(changeset2) + end + end +end diff --git a/test/towerops/snmp/profiles/base_test.exs b/test/towerops/snmp/profiles/base_test.exs index cdfef311..95760fb9 100644 --- a/test/towerops/snmp/profiles/base_test.exs +++ b/test/towerops/snmp/profiles/base_test.exs @@ -687,4 +687,88 @@ defmodule Towerops.Snmp.Profiles.BaseTest do refute 4095 in vlan_ids end end + + describe "discover_ip_addresses/1" do + test "discovers IPv4 addresses from IP-MIB" do + stub(SnmpMock, :walk, fn _, oid, _ -> + case oid do + # ipAdEntIfIndex - maps IP to interface index + "1.3.6.1.2.1.4.20.1.2" -> + {:ok, + [ + %{oid: "1.3.6.1.2.1.4.20.1.2.192.168.1.1", value: {:integer, 1}}, + %{oid: "1.3.6.1.2.1.4.20.1.2.192.168.1.100", value: {:integer, 1}}, + %{oid: "1.3.6.1.2.1.4.20.1.2.10.0.0.1", value: {:integer, 2}} + ]} + + # ipAdEntNetMask - subnet masks + "1.3.6.1.2.1.4.20.1.3" -> + {:ok, + [ + %{oid: "1.3.6.1.2.1.4.20.1.3.192.168.1.1", value: {:ip_address, {255, 255, 255, 0}}}, + %{oid: "1.3.6.1.2.1.4.20.1.3.192.168.1.100", value: {:ip_address, {255, 255, 255, 0}}}, + %{oid: "1.3.6.1.2.1.4.20.1.3.10.0.0.1", value: {:ip_address, {255, 0, 0, 0}}} + ]} + + _ -> + {:ok, []} + end + end) + + assert {:ok, ip_addresses} = Base.discover_ip_addresses(@client_opts) + + assert length(ip_addresses) == 3 + + ip1 = Enum.find(ip_addresses, &(&1.ip_address == "192.168.1.1")) + assert ip1.if_index == 1 + assert ip1.subnet_mask == "255.255.255.0" + assert ip1.prefix_length == 24 + assert ip1.ip_type == "ipv4" + + ip2 = Enum.find(ip_addresses, &(&1.ip_address == "10.0.0.1")) + assert ip2.if_index == 2 + assert ip2.subnet_mask == "255.0.0.0" + assert ip2.prefix_length == 8 + end + + test "returns empty list when IP-MIB not supported" do + stub(SnmpMock, :walk, fn _, _, _ -> {:ok, []} end) + + assert {:ok, ip_addresses} = Base.discover_ip_addresses(@client_opts) + assert ip_addresses == [] + end + + test "handles walk errors gracefully" do + stub(SnmpMock, :walk, fn _, _, _ -> {:error, :timeout} end) + + assert {:ok, ip_addresses} = Base.discover_ip_addresses(@client_opts) + assert ip_addresses == [] + end + + test "handles missing subnet mask gracefully" do + stub(SnmpMock, :walk, fn _, oid, _ -> + case oid do + "1.3.6.1.2.1.4.20.1.2" -> + {:ok, + [ + %{oid: "1.3.6.1.2.1.4.20.1.2.192.168.1.1", value: {:integer, 1}} + ]} + + "1.3.6.1.2.1.4.20.1.3" -> + {:ok, []} + + _ -> + {:ok, []} + end + end) + + assert {:ok, ip_addresses} = Base.discover_ip_addresses(@client_opts) + + assert length(ip_addresses) == 1 + ip = hd(ip_addresses) + assert ip.ip_address == "192.168.1.1" + assert ip.subnet_mask == nil + assert ip.prefix_length == nil + end + end end