diff --git a/changelog/68844.fixed.md b/changelog/68844.fixed.md new file mode 100644 index 000000000000..9b435bb39280 --- /dev/null +++ b/changelog/68844.fixed.md @@ -0,0 +1 @@ +Added ``timeout`` parameter to ``network.traceroute`` to prevent the command from hanging indefinitely in environments with restricted network access (e.g., CI runners). diff --git a/salt/modules/network.py b/salt/modules/network.py index efc8a7a3617c..bea6ffa2c52f 100644 --- a/salt/modules/network.py +++ b/salt/modules/network.py @@ -919,7 +919,7 @@ def active_tcp(): @salt.utils.decorators.path.which("traceroute") -def traceroute(host): +def traceroute(host, timeout=0): """ Performs a traceroute to a 3rd party host @@ -929,15 +929,26 @@ def traceroute(host): .. versionchanged:: 2016.11.4 Added support for AIX + .. versionchanged:: 3008.0 + Added ``timeout`` parameter + + timeout + The maximum number of seconds to wait for the traceroute command + to complete. Defaults to ``0`` (no timeout). + CLI Example: .. code-block:: bash salt '*' network.traceroute archlinux.org + salt '*' network.traceroute archlinux.org timeout=60 """ ret = [] cmd = "traceroute {}".format(__utils__["network.sanitize_host"](host)) - out = __salt__["cmd.run"](cmd) + cmd_kwargs = {} + if timeout: + cmd_kwargs["timeout"] = int(timeout) + out = __salt__["cmd.run"](cmd, **cmd_kwargs) # Parse version of traceroute if __utils__["platform.is_sunos"]() or __utils__["platform.is_aix"](): diff --git a/tests/pytests/functional/modules/test_network.py b/tests/pytests/functional/modules/test_network.py index a05006bccd7b..3d1b8dbef006 100644 --- a/tests/pytests/functional/modules/test_network.py +++ b/tests/pytests/functional/modules/test_network.py @@ -57,11 +57,12 @@ def test_network_netstat(network): @pytest.mark.skip_if_binaries_missing("traceroute") @pytest.mark.slow_test +@pytest.mark.timeout(120) def test_network_traceroute(network, url): """ network.traceroute """ - ret = network.traceroute(url) + ret = network.traceroute(url, timeout=60) exp_out = ["hostname", "ip"] for val in ret: if not val: diff --git a/tests/pytests/unit/modules/test_network.py b/tests/pytests/unit/modules/test_network.py index 671623102aa3..c7fbf48185e4 100644 --- a/tests/pytests/unit/modules/test_network.py +++ b/tests/pytests/unit/modules/test_network.py @@ -238,6 +238,54 @@ def patched_which(binary): assert networkmod.traceroute("gentoo.org") == [] +def test_traceroute_with_timeout(): + """ + Test that traceroute passes the timeout parameter to cmd.run + """ + + def patched_which(binary): + binary_path = shutil.which(binary) + if binary_path: + return binary_path + if binary == "traceroute": + return binary + return binary_path + + mock_run = MagicMock(return_value="") + with patch("salt.utils.path.which", patched_which): + with patch.dict( + networkmod.__utils__, + {"network.sanitize_host": MagicMock(return_value="gentoo.org")}, + ): + with patch.dict(networkmod.__salt__, {"cmd.run": mock_run}): + networkmod.traceroute("gentoo.org", timeout=60) + mock_run.assert_any_call("traceroute gentoo.org", timeout=60) + + +def test_traceroute_without_timeout(): + """ + Test that traceroute does not pass timeout when not specified + """ + + def patched_which(binary): + binary_path = shutil.which(binary) + if binary_path: + return binary_path + if binary == "traceroute": + return binary + return binary_path + + mock_run = MagicMock(return_value="") + with patch("salt.utils.path.which", patched_which): + with patch.dict( + networkmod.__utils__, + {"network.sanitize_host": MagicMock(return_value="gentoo.org")}, + ): + with patch.dict(networkmod.__salt__, {"cmd.run": mock_run}): + networkmod.traceroute("gentoo.org") + mock_run.assert_any_call("traceroute gentoo.org") + + def test_dig(): """ Test for Performs a DNS lookup with dig