diff --git a/changelog/59520.fixed.md b/changelog/59520.fixed.md new file mode 100644 index 00000000000..6eb4a9d75ff --- /dev/null +++ b/changelog/59520.fixed.md @@ -0,0 +1 @@ +Set NO_UPDATE_NOTIFIER=1 for the npm version check so the npm module no longer stalls at load time behind a firewall/proxy diff --git a/salt/modules/npm.py b/salt/modules/npm.py index f7a40043de4..84878458563 100644 --- a/salt/modules/npm.py +++ b/salt/modules/npm.py @@ -47,7 +47,15 @@ def _check_valid_version(): npm_path = salt.utils.path.which("npm") # pylint: disable=no-member - res = salt.modules.cmdmod.run(f"{npm_path} --version", output_loglevel="quiet") + # NO_UPDATE_NOTIFIER stops npm from contacting the registry to check for + # updates. Since this runs at module-load time from __virtual__, a blocked + # registry (e.g. behind a corporate firewall/proxy) would otherwise stall + # every salt-call for minutes while npm's update-notifier times out. + res = salt.modules.cmdmod.run( + f"{npm_path} --version", + output_loglevel="quiet", + env={"NO_UPDATE_NOTIFIER": "1"}, + ) npm_version = Version(res) valid_version = Version("1.2") # pylint: enable=no-member diff --git a/tests/pytests/unit/modules/test_npm.py b/tests/pytests/unit/modules/test_npm.py index 53243964953..d3db845be99 100644 --- a/tests/pytests/unit/modules/test_npm.py +++ b/tests/pytests/unit/modules/test_npm.py @@ -196,3 +196,54 @@ def test_cache_path(): ) with patch.dict(npm.__salt__, {"cmd.run_all": mock}): assert npm.cache_path() == "/User/salt/.npm" + + +# '_check_valid_version' function tests: 3 + + +def test_check_valid_version_disables_update_notifier_59520(): + """ + _check_valid_version runs at module-load from __virtual__. It must pass + NO_UPDATE_NOTIFIER=1 to the `npm --version` call so npm does not contact + the registry (which stalls for minutes behind a firewall/proxy). It calls + the module-level import salt.modules.cmdmod.run directly, so patch that, + not __salt__["cmd.run_all"]. + """ + mock_run = MagicMock(return_value="6.14.0") + with ( + patch("salt.utils.path.which", MagicMock(return_value="/usr/bin/npm")), + patch("salt.modules.cmdmod.run", mock_run), + ): + npm._check_valid_version() + mock_run.assert_called_once_with( + "/usr/bin/npm --version", + output_loglevel="quiet", + env={"NO_UPDATE_NOTIFIER": "1"}, + ) + + +def test_check_valid_version_raises_on_old_npm_59520(): + """ + Inverse / must-not-regress: adding the env kwarg is purely additive and + must not disturb the version gate. An npm older than 1.2 must still raise + CommandExecutionError. This passes with and without the fix. + """ + mock_run = MagicMock(return_value="1.1.0") + with ( + patch("salt.utils.path.which", MagicMock(return_value="/usr/bin/npm")), + patch("salt.modules.cmdmod.run", mock_run), + ): + pytest.raises(CommandExecutionError, npm._check_valid_version) + + +def test_check_valid_version_accepts_recent_npm_59520(): + """ + Peripheral coverage: an npm at or above the minimum version returns without + raising. + """ + mock_run = MagicMock(return_value="1.2") + with ( + patch("salt.utils.path.which", MagicMock(return_value="/usr/bin/npm")), + patch("salt.modules.cmdmod.run", mock_run), + ): + assert npm._check_valid_version() is None