graham/towerops
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat: allow device name to be populated from SNMP sysName

Browse source 
- Make device name field optional when SNMP is enabled (ICMP Only mode still requires name)
- Add database migration to make name column nullable
- Add conditional validation: name required only when snmp_enabled is false
- Update SNMP discovery to populate device name from sysName when empty
- Add helper text explaining SNMP name population behavior in form
- Add comprehensive tests for device name SNMP population
- Update test mocks to account for NetSnmp profile sensor discovery
This commit is contained in:
Graham McIntire 2026-01-17 16:28:43 -06:00
parent f27e1622a3
commit 560370cb0b
No known key found for this signature in database
6 changed files with 258 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
lib/towerops/devices/device.ex
View file

@ -84,7 +84,8 @@ defmodule Towerops.Devices.Device do
:snmp_community,
:snmp_port
])
|> validate_required([:name, :ip_address, :site_id])
|> validate_required([:ip_address, :site_id])
|> validate_name()
|> validate_length(:name, min: 2, max: 200)
|> validate_length(:description, max: 1000)
|> validate_ip_address()
@ -93,6 +94,19 @@ defmodule Towerops.Devices.Device do
|> foreign_key_constraint(:site_id)
end
defp validate_name(changeset) do
snmp_enabled = get_field(changeset, :snmp_enabled, false)
name = get_field(changeset, :name)
# Name is required only when SNMP is not enabled (ICMP only mode)
# When SNMP is enabled, name can be empty and will be populated from sysName during discovery
if !snmp_enabled and (is_nil(name) or name == "") do
add_error(changeset, :name, "can't be blank (required when SNMP is disabled)")
else
changeset
end
end
defp validate_ip_address(changeset) do
case get_change(changeset, :ip_address) do
nil ->

22
lib/towerops/snmp/discovery.ex
View file

@ -103,6 +103,7 @@ defmodule Towerops.Snmp.Discovery do
with {:ok, _} <- Client.test_connection(client_opts),
{:ok, system_info} <- discover_system(client_opts),
{:ok, device} <- update_device_name_from_snmp(device, system_info),
profile = select_profile(system_info),
{:ok, device_info} <- build_device_info(client_opts, system_info, profile),
{:ok, interfaces} <- discover_interfaces(client_opts, profile),
@ -354,4 +355,25 @@ defmodule Towerops.Snmp.Discovery do
Logger.info("Saved #{length(neighbors)} neighbors for device #{device_id}")
:ok
end
@spec update_device_name_from_snmp(DeviceSchema.t(), system_info()) ::
{:ok, DeviceSchema.t()} | {:error, term()}
defp update_device_name_from_snmp(device, system_info) do
# Only update device name if it's currently empty and we got a sysName from SNMP
if (is_nil(device.name) or device.name == "") and Map.has_key?(system_info, :sys_name) do
sys_name = Map.get(system_info, :sys_name)
if sys_name && sys_name != "" do
Logger.info("Updating device name from SNMP sysName: #{sys_name}")
device
|> Ecto.Changeset.change(name: sys_name)
|> Repo.update()
else
{:ok, device}
end
else
{:ok, device}
end
end
end

12
lib/towerops_web/live/device_live/form.html.heex
View file

@ -28,7 +28,17 @@
<div class="max-w-2xl">
<.form for={@form} id="device-form" phx-change="validate" phx-submit="save">
<.input field={@form[:name]} type="text" label="Device Name" required />
<.input
field={@form[:name]}
type="text"
label="Device Name"
required={@monitoring_mode == "icmp_only"}
/>
<%= if @monitoring_mode == "snmp_and_icmp" do %>
<p class="mt-1 text-sm text-zinc-600 dark:text-zinc-400">
Leave blank to automatically populate from SNMP device name (sysName) during discovery.
</p>
<% end %>
<.input field={@form[:ip_address]} type="text" label="IP Address" required />

9
priv/repo/migrations/20260117222456_make_device_name_nullable.exs Normal file
View file

@ -0,0 +1,9 @@
defmodule Towerops.Repo.Migrations.MakeDeviceNameNullable do
use Ecto.Migration
def change do
alter table(:devices) do
modify :name, :string, null: true
end
end
end

132
test/towerops/snmp/discovery_test.exs
View file

@ -486,5 +486,137 @@ defmodule Towerops.Snmp.DiscoveryTest do
# Verify only good device was created
assert Repo.get_by(Device, device_id: device1.id)
end
test "updates device name from SNMP sysName when device name is empty", %{
organization: _organization,
site: site
} do
# Create device without a name
{:ok, device} =
Towerops.Devices.create_device(%{
name: nil,
ip_address: "192.168.1.200",
site_id: site.id,
snmp_enabled: true,
snmp_version: "2c",
snmp_community: "public",
snmp_port: 161
})
# Mock successful connection test
expect(SnmpMock, :get, fn _, "1.3.6.1.2.1.1.3.0", _ ->
{:ok, {:timeticks, 12_345}}
end)
# Mock system info discovery with sysName + UCD sensor OIDs
# 6 system info OIDs + 3 load OIDs + 2 memory OIDs = 11 total
expect(SnmpMock, :get, 11, fn _, oid, _ ->
case oid do
"1.3.6.1.2.1.1.1.0" ->
{:ok, {:octet_string, "Linux switch01 4.4.0"}}
"1.3.6.1.2.1.1.2.0" ->
{:ok, {:object_identifier, [1, 3, 6, 1, 4, 1, 8072, 3, 2, 10]}}
"1.3.6.1.2.1.1.3.0" ->
{:ok, {:timeticks, 12_345}}
"1.3.6.1.2.1.1.4.0" ->
{:ok, {:octet_string, "admin@example.com"}}
"1.3.6.1.2.1.1.5.0" ->
{:ok, {:octet_string, "discovered-switch-name"}}
"1.3.6.1.2.1.1.6.0" ->
{:ok, {:octet_string, "Server Room"}}
_ ->
{:error, :no_such_object}
end
end)
# Mock interface and sensor discovery - no interfaces/sensors for simplicity
# Walk called 10 times:
# - LM sensors: temp devices (1), temp values (1), fan devices (1), fan values (1),
# voltage devices (1), voltage values (1)
# - Base sensors fallback (1)
# - Interfaces (1)
# - Neighbors: LLDP (1), CDP (1)
expect(SnmpMock, :walk, 10, fn _, _, _ ->
{:ok, []}
end)
assert {:ok, _snmp_device} = Discovery.discover_device(device)
# Verify the device name was updated with sysName
updated_device = Towerops.Devices.get_device!(device.id)
assert updated_device.name == "discovered-switch-name"
end
test "does not update device name from SNMP when device already has a name", %{
organization: _organization,
site: site
} do
# Create device with a name
{:ok, device} =
Towerops.Devices.create_device(%{
name: "My Custom Name",
ip_address: "192.168.1.201",
site_id: site.id,
snmp_enabled: true,
snmp_version: "2c",
snmp_community: "public",
snmp_port: 161
})
# Mock successful connection test
expect(SnmpMock, :get, fn _, "1.3.6.1.2.1.1.3.0", _ ->
{:ok, {:timeticks, 12_345}}
end)
# Mock system info discovery with different sysName + UCD sensor OIDs
# 6 system info OIDs + 3 load OIDs + 2 memory OIDs = 11 total
expect(SnmpMock, :get, 11, fn _, oid, _ ->
case oid do
"1.3.6.1.2.1.1.1.0" ->
{:ok, {:octet_string, "Linux switch02 4.4.0"}}
"1.3.6.1.2.1.1.2.0" ->
{:ok, {:object_identifier, [1, 3, 6, 1, 4, 1, 8072, 3, 2, 10]}}
"1.3.6.1.2.1.1.3.0" ->
{:ok, {:timeticks, 12_345}}
"1.3.6.1.2.1.1.4.0" ->
{:ok, {:octet_string, "admin@example.com"}}
"1.3.6.1.2.1.1.5.0" ->
{:ok, {:octet_string, "different-switch-name"}}
"1.3.6.1.2.1.1.6.0" ->
{:ok, {:octet_string, "Server Room"}}
_ ->
{:error, :no_such_object}
end
end)
# Mock interface and sensor discovery
# Walk called 10 times:
# - LM sensors: temp devices (1), temp values (1), fan devices (1), fan values (1),
# voltage devices (1), voltage values (1)
# - Base sensors fallback (1)
# - Interfaces (1)
# - Neighbors: LLDP (1), CDP (1)
expect(SnmpMock, :walk, 10, fn _, _, _ ->
{:ok, []}
end)
assert {:ok, _snmp_device} = Discovery.discover_device(device)
# Verify the device name was NOT changed
updated_device = Towerops.Devices.get_device!(device.id)
assert updated_device.name == "My Custom Name"
end
end
end

69
test/towerops_web/live/device_live_test.exs
View file

@ -347,6 +347,75 @@ defmodule ToweropsWeb.DeviceLiveTest do
assert path == ~p"/users/log-in"
end
test "allows creating device without name when SNMP enabled", %{
conn: conn,
organization: organization,
site: site
} do
{:ok, view, _html} = live(conn, ~p"/orgs/#{organization.slug}/devices/new")
html =
view
|> form("#device-form",
device: %{
name: "",
ip_address: "192.168.1.150",
site_id: site.id,
snmp_community: "public"
}
)
|> render_submit()
# Should succeed without validation error
assert html =~ "Device created successfully"
refute html =~ "can&#39;t be blank"
# Verify device was created with empty name
device_list = Towerops.Devices.list_organization_devices(organization.id)
new_device = Enum.find(device_list, &(&1.ip_address == "192.168.1.150"))
assert new_device
assert new_device.name == nil or new_device.name == ""
end
test "shows helper text about SNMP name population when name is empty", %{
conn: conn,
organization: organization
} do
{:ok, _view, html} = live(conn, ~p"/orgs/#{organization.slug}/devices/new")
# Should show helper text about name being populated from SNMP
assert html =~ "Leave blank to use SNMP device name" or
html =~ "populated from SNMP" or
html =~ "sysName"
end
test "requires name when ICMP Only mode (no SNMP)", %{
conn: conn,
organization: organization,
site: site
} do
{:ok, view, _html} = live(conn, ~p"/orgs/#{organization.slug}/devices/new")
# Switch to ICMP Only mode
view
|> element("button", "ICMP Only")
|> render_click()
html =
view
|> form("#device-form",
device: %{
name: "",
ip_address: "192.168.1.151",
site_id: site.id
}
)
|> render_submit()
# Should show validation error for empty name in ICMP Only mode
assert html =~ "can&#39;t be blank" or html =~ "required"
end
test "defaults to SNMP & ICMP monitoring mode", %{conn: conn, organization: organization} do
{:ok, _view, html} = live(conn, ~p"/orgs/#{organization.slug}/devices/new")