From ece9788b890771ba16b030e8d73af21731163963 Mon Sep 17 00:00:00 2001 From: Aditya Raj Singh Date: Wed, 26 Aug 2026 02:47:22 +0530 Subject: [PATCH 1/3] fix: keep MikroTik scans running past MAC-less leases --- server/plugins/mikrotik_scan/mikrotik.py | 9 +-- test/plugins/test_mikrotik_scan.py | 83 ++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 4 deletions(-) create mode 100644 test/plugins/test_mikrotik_scan.py diff --git a/server/plugins/mikrotik_scan/mikrotik.py b/server/plugins/mikrotik_scan/mikrotik.py index 1bb566c8a..3a6ca1363 100755 --- a/server/plugins/mikrotik_scan/mikrotik.py +++ b/server/plugins/mikrotik_scan/mikrotik.py @@ -7,7 +7,7 @@ import sys INSTALL_PATH = os.getenv('NETALERTX_APP', '/app') sys.path.extend([f"{INSTALL_PATH}/server/plugins", f"{INSTALL_PATH}/server"]) -from plugin_helper import Plugin_Objects # noqa: E402 [flake8 lint suppression] +from plugin_helper import Plugin_Objects, normalize_mac # noqa: E402 [flake8 lint suppression] from logger import mylog, Logger # noqa: E402 [flake8 lint suppression] from helper import get_setting_value # noqa: E402 [flake8 lint suppression] from const import logPath # noqa: E402 [flake8 lint suppression] @@ -64,16 +64,17 @@ def get_entries(plugin_objects: Plugin_Objects) -> Plugin_Objects: for lease in leases: lease_id = lease.get('.id') address = lease.get('address') - mac_address = lease.get('mac-address').lower() + raw_mac_address = lease.get('mac-address') host_name = lease.get('host-name') comment = lease.get('comment') last_seen = lease.get('last-seen') status = lease.get('status') device_name = comment or host_name or "(unknown)" - mylog('verbose', f"ID: {lease_id}, Address: {address}, MAC: {mac_address}, Host Name: {host_name}, Comment: {comment}, Last Seen: {last_seen}, Status: {status}") + mylog('verbose', f"ID: {lease_id}, Address: {address}, MAC: {raw_mac_address}, Host Name: {host_name}, Comment: {comment}, Last Seen: {last_seen}, Status: {status}") - if (status == "bound"): + if status == "bound" and raw_mac_address: + mac_address = normalize_mac(raw_mac_address) plugin_objects.add_object( primaryId = mac_address, secondaryId = address, diff --git a/test/plugins/test_mikrotik_scan.py b/test/plugins/test_mikrotik_scan.py new file mode 100644 index 000000000..f7d20505a --- /dev/null +++ b/test/plugins/test_mikrotik_scan.py @@ -0,0 +1,83 @@ +"""Tests for the MikroTik DHCP lease scanner.""" + +import importlib.util +import sys +import types +from pathlib import Path +from unittest.mock import MagicMock, patch + + +def _load_mikrotik_module(): + stubbed_module_names = [] + + def stub(name, **attributes): + module = types.ModuleType(name) + for attribute, value in attributes.items(): + setattr(module, attribute, value) + sys.modules[name] = module + stubbed_module_names.append(name) + + class TrapError(Exception): + pass + + stub( + "plugin_helper", + Plugin_Objects=MagicMock, + normalize_mac=lambda mac: mac.strip().lower(), + ) + stub("logger", mylog=MagicMock(), Logger=MagicMock()) + stub("helper", get_setting_value=MagicMock(return_value="UTC")) + stub("const", logPath="/tmp") + stub("conf", tz=None) + stub("pytz", timezone=MagicMock(return_value="UTC")) + stub("librouteros", connect=MagicMock()) + stub("librouteros.exceptions", TrapError=TrapError) + + module_path = Path(__file__).resolve().parents[2] / "server" / "plugins" / "mikrotik_scan" / "mikrotik.py" + spec = importlib.util.spec_from_file_location("mikrotik_scan", module_path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + + for name in stubbed_module_names: + sys.modules.pop(name, None) + + return module + + +mikrotik = _load_mikrotik_module() + + +def _lease(lease_id, address, mac_address, status="bound"): + lease = { + ".id": lease_id, + "address": address, + "host-name": f"device-{lease_id}", + "comment": "", + "last-seen": "1m", + "status": status, + } + if mac_address is not None: + lease["mac-address"] = mac_address + return lease + + +def test_disabled_lease_without_mac_does_not_abort_remaining_leases(): + leases = [ + _lease("*1", "192.168.1.2", "aa:bb:cc:dd:ee:01"), + _lease("*2", "192.168.1.5", None, status="waiting"), + _lease("*3", "192.168.1.8", "aa:bb:cc:dd:ee:03"), + ] + api = MagicMock(return_value=leases) + plugin_objects = MagicMock() + + mikrotik.MT_USER = "user" + mikrotik.MT_PASS = "password" + mikrotik.MT_HOST = "192.168.1.1" + mikrotik.MT_PORT = 8728 + + with patch.object(mikrotik, "connect", return_value=api): + result = mikrotik.get_entries(plugin_objects) + + assert result is plugin_objects + assert plugin_objects.add_object.call_count == 2 + assert [call.kwargs["primaryId"] for call in plugin_objects.add_object.call_args_list] == ["aa:bb:cc:dd:ee:01", "aa:bb:cc:dd:ee:03"] From ea7afd6c835280435559c4feac381b419f0fdb76 Mon Sep 17 00:00:00 2001 From: Aditya Raj Singh Date: Wed, 26 Aug 2026 02:59:20 +0530 Subject: [PATCH 2/3] test: isolate MikroTik scanner module stubs --- test/plugins/test_mikrotik_scan.py | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/test/plugins/test_mikrotik_scan.py b/test/plugins/test_mikrotik_scan.py index f7d20505a..e42c07900 100644 --- a/test/plugins/test_mikrotik_scan.py +++ b/test/plugins/test_mikrotik_scan.py @@ -8,14 +8,15 @@ from unittest.mock import MagicMock, patch def _load_mikrotik_module(): - stubbed_module_names = [] + missing_module = object() + previous_modules = {} def stub(name, **attributes): + previous_modules[name] = sys.modules.get(name, missing_module) module = types.ModuleType(name) for attribute, value in attributes.items(): setattr(module, attribute, value) sys.modules[name] = module - stubbed_module_names.append(name) class TrapError(Exception): pass @@ -23,7 +24,7 @@ def _load_mikrotik_module(): stub( "plugin_helper", Plugin_Objects=MagicMock, - normalize_mac=lambda mac: mac.strip().lower(), + normalize_mac=lambda mac: mac.strip().lower().replace("-", ":"), ) stub("logger", mylog=MagicMock(), Logger=MagicMock()) stub("helper", get_setting_value=MagicMock(return_value="UTC")) @@ -36,10 +37,14 @@ def _load_mikrotik_module(): module_path = Path(__file__).resolve().parents[2] / "server" / "plugins" / "mikrotik_scan" / "mikrotik.py" spec = importlib.util.spec_from_file_location("mikrotik_scan", module_path) module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - - for name in stubbed_module_names: - sys.modules.pop(name, None) + try: + spec.loader.exec_module(module) + finally: + for name, previous_module in previous_modules.items(): + if previous_module is missing_module: + sys.modules.pop(name, None) + else: + sys.modules[name] = previous_module return module @@ -63,15 +68,15 @@ def _lease(lease_id, address, mac_address, status="bound"): def test_disabled_lease_without_mac_does_not_abort_remaining_leases(): leases = [ - _lease("*1", "192.168.1.2", "aa:bb:cc:dd:ee:01"), + _lease("*1", "192.168.1.2", "aa-bb-cc-dd-ee-01"), _lease("*2", "192.168.1.5", None, status="waiting"), - _lease("*3", "192.168.1.8", "aa:bb:cc:dd:ee:03"), + _lease("*3", "192.168.1.8", "aa-bb-cc-dd-ee-03"), ] api = MagicMock(return_value=leases) plugin_objects = MagicMock() mikrotik.MT_USER = "user" - mikrotik.MT_PASS = "password" + mikrotik.MT_PASS = None mikrotik.MT_HOST = "192.168.1.1" mikrotik.MT_PORT = 8728 From bcd44381c656fba66cfe81f977ddc3e21a7885c4 Mon Sep 17 00:00:00 2001 From: Aditya Raj Singh Date: Wed, 26 Aug 2026 03:05:02 +0530 Subject: [PATCH 3/3] test: cover bound MikroTik leases without MACs --- test/plugins/test_mikrotik_scan.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/plugins/test_mikrotik_scan.py b/test/plugins/test_mikrotik_scan.py index e42c07900..e79f217f4 100644 --- a/test/plugins/test_mikrotik_scan.py +++ b/test/plugins/test_mikrotik_scan.py @@ -66,11 +66,12 @@ def _lease(lease_id, address, mac_address, status="bound"): return lease -def test_disabled_lease_without_mac_does_not_abort_remaining_leases(): +def test_leases_without_mac_do_not_abort_remaining_leases(): leases = [ _lease("*1", "192.168.1.2", "aa-bb-cc-dd-ee-01"), _lease("*2", "192.168.1.5", None, status="waiting"), - _lease("*3", "192.168.1.8", "aa-bb-cc-dd-ee-03"), + _lease("*3", "192.168.1.6", None), + _lease("*4", "192.168.1.8", "aa-bb-cc-dd-ee-04"), ] api = MagicMock(return_value=leases) plugin_objects = MagicMock() @@ -85,4 +86,4 @@ def test_disabled_lease_without_mac_does_not_abort_remaining_leases(): assert result is plugin_objects assert plugin_objects.add_object.call_count == 2 - assert [call.kwargs["primaryId"] for call in plugin_objects.add_object.call_args_list] == ["aa:bb:cc:dd:ee:01", "aa:bb:cc:dd:ee:03"] + assert [call.kwargs["primaryId"] for call in plugin_objects.add_object.call_args_list] == ["aa:bb:cc:dd:ee:01", "aa:bb:cc:dd:ee:04"]