Fix Junos timeout wrappers crashing on dev_timeout=None (#58108)

changelog/58108.fixed.md

@@ -0,0 +1 @@
+Fixed ``salt '*' napalm.junos_cli`` (and other Junos calls) raising ``TypeError``/``RuntimeError`` when no timeout was requested. ``napalm.junos_cli`` forwards ``dev_timeout=None`` by default, and the Junos ``_timeout_decorator``/``_timeout_decorator_cleankwargs`` wrappers treated that as a real value, so ``max(None, 0)`` raised (and setting the connection timeout to ``None`` is rejected by junos-eznc). The wrappers now coalesce ``None`` to ``0`` and only override the connection timeout when a real (>0) ``dev_timeout``/``timeout`` is given.

salt/modules/junos.py

@@ -153,8 +153,14 @@ def __exit__(self, exc_type, exc_value, exc_traceback):
 def _timeout_decorator(function):
     @wraps(function)
     def wrapper(*args, **kwargs):
-        if "dev_timeout" in kwargs or "timeout" in kwargs:
-            ldev_timeout = max(kwargs.pop("dev_timeout", 0), kwargs.pop("timeout", 0))
+        # Callers such as napalm.junos_cli pass dev_timeout=None when no timeout
+        # was requested; coalesce None to 0 so max() does not raise, and only
+        # override the connection timeout when a real (>0) value is given
+        # (junos-eznc rejects a None/0 timeout).
+        ldev_timeout = max(
+            kwargs.pop("dev_timeout", 0) or 0, kwargs.pop("timeout", 0) or 0
+        )
+        if ldev_timeout:
             conn = __proxy__["junos.conn"]()
             restore_timeout = conn.timeout
             conn.timeout = ldev_timeout
@@ -174,8 +180,12 @@ def wrapper(*args, **kwargs):
 def _timeout_decorator_cleankwargs(function):
     @wraps(function)
     def wrapper(*args, **kwargs):
-        if "dev_timeout" in kwargs or "timeout" in kwargs:
-            ldev_timeout = max(kwargs.pop("dev_timeout", 0), kwargs.pop("timeout", 0))
+        # See _timeout_decorator: dev_timeout=None must not raise, and the
+        # connection timeout is only overridden when a real (>0) value is given.
+        ldev_timeout = max(
+            kwargs.pop("dev_timeout", 0) or 0, kwargs.pop("timeout", 0) or 0
+        )
+        if ldev_timeout:
             conn = __proxy__["junos.conn"]()
             restore_timeout = conn.timeout
             conn.timeout = ldev_timeout

tests/pytests/unit/modules/test_junos.py

@@ -200,6 +200,25 @@ def function(x):
         mock_timeout.assert_has_calls(calls)
 
 
+def test__timeout_decorator_none_dev_timeout():
+    # napalm.junos_cli (and friends) forward dev_timeout=None when no timeout
+    # was requested. That must not raise, and must leave the connection timeout
+    # untouched -- only a real (>0) value overrides it (#58108).
+    with patch("jnpr.junos.Device.timeout", new_callable=PropertyMock) as mock_timeout:
+        mock_timeout.return_value = 30
+
+        def function(x):
+            return x
+
+        decorator = junos._timeout_decorator(function)
+        assert decorator("Test Mock", dev_timeout=None) == "Test Mock"
+        mock_timeout.assert_not_called()
+
+        decorator = junos._timeout_decorator_cleankwargs(function)
+        assert decorator("Test Mock", dev_timeout=None, __pub_args="abc") == "Test Mock"
+        mock_timeout.assert_not_called()
+
+
 def test_facts_refresh():
     with patch("salt.modules.saltutil.sync_grains") as mock_sync_grains:
         ret = {