From 6cbd12e684d6333413556181c00fb116c73a58fe Mon Sep 17 00:00:00 2001 From: Morten Brekkevold Date: Thu, 6 Jun 2024 14:21:45 +0200 Subject: [PATCH] Ignore traps with invalid/missing ifIndex It's not entirely clear what Zino 1 does here, but it's also not clear that we should attempt to do anything "useful" with crazy traps. This adds early ignores and tests for them. --- src/zino/trapobservers/link_traps.py | 8 +++++++- tests/trapobservers/link_traps_test.py | 17 ++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/zino/trapobservers/link_traps.py b/src/zino/trapobservers/link_traps.py index bebf51269..f6a336a50 100644 --- a/src/zino/trapobservers/link_traps.py +++ b/src/zino/trapobservers/link_traps.py @@ -29,8 +29,14 @@ def handle_trap(self, trap: TrapMessage) -> Optional[bool]: if "ifIndex" in trap.variables: ifindex = trap.variables.get("ifIndex").value else: - ifindex = -1 + _logger.warning("%s: %s trap contained no ifIndex value, ignoring", trap.agent.device.name, trap.name) + return False port = trap.agent.device.ports.get(ifindex) if ifindex > 0 else None + if not port: + _logger.warning( + "%s: %s trap referenced unknown port (ix %s), ignoring", trap.agent.device.name, trap.name, ifindex + ) + return False # TODO: The trap *might* contain an ifDescr value. If present, Zino uses that for trap processing. # Otherwise, it fetches ifDescr from its own state and uses that for trap processing. Either way, diff --git a/tests/trapobservers/link_traps_test.py b/tests/trapobservers/link_traps_test.py index d5d12aded..edccc9b6f 100644 --- a/tests/trapobservers/link_traps_test.py +++ b/tests/trapobservers/link_traps_test.py @@ -1,5 +1,5 @@ from datetime import timedelta -from unittest.mock import Mock +from unittest.mock import Mock, patch import pytest @@ -101,6 +101,21 @@ def test_when_link_trap_is_redundant_policy_should_ignore_trap(self, state_with_ localhost, localhost.ports[1], is_up=False ), "did not ignore redundant linkDown trap" + def test_when_link_trap_is_missing_ifindex_value_it_should_ignore_trap_early(self, state_with_localhost_with_port): + observer = LinkTrapObserver(state=state_with_localhost_with_port, polldevs=Mock()) + trap = Mock(variables={}) + with patch.object(observer, "handle_link_transition") as handle_link_transition: + assert not observer.handle_trap(trap) + assert not handle_link_transition.called, "handle_link_transition was called" + + def test_when_link_trap_refers_to_unknown_port_it_should_ignore_trap_early(self, state_with_localhost_with_port): + observer = LinkTrapObserver(state=state_with_localhost_with_port, polldevs=Mock()) + localhost = state_with_localhost_with_port.devices.devices["localhost"] + trap = Mock(agent=Mock(device=localhost), variables={"ifIndex": Mock(value=99)}) + with patch.object(observer, "handle_link_transition") as handle_link_transition: + assert not observer.handle_trap(trap) + assert not handle_link_transition.called, "handle_link_transition was called" + @pytest.fixture def state_with_localhost_with_port(state_with_localhost):