diff --git a/changelog/68214.fixed.md b/changelog/68214.fixed.md new file mode 100644 index 00000000000..e15b997c339 --- /dev/null +++ b/changelog/68214.fixed.md @@ -0,0 +1 @@ +Pass ``--disable-pip-version-check`` when ``pip.list`` and ``pip.freeze`` invoke pip, so ``pip.installed`` state checks no longer hang for ~20s per call on airgapped minions while pip tries to reach PyPI for its self-version check. diff --git a/salt/modules/pip.py b/salt/modules/pip.py index d108d0815ad..0bc8b0d580c 100644 --- a/salt/modules/pip.py +++ b/salt/modules/pip.py @@ -1220,6 +1220,12 @@ def freeze(bin_env=None, user=None, cwd=None, use_vt=False, env_vars=None, **kwa else: cmd.append("--all") + # Suppress pip's outbound version-check; otherwise airgapped minions block + # on the PyPI round-trip for every ``pip freeze`` (issue #68214). The flag + # was added in pip 6.0, predating the 8.0.3 floor above, so it is always + # safe to append here. + cmd.append("--disable-pip-version-check") + cmd_kwargs = dict(runas=user, cwd=cwd, use_vt=use_vt, python_shell=False) if kwargs: cmd_kwargs.update(**kwargs) @@ -1342,7 +1348,11 @@ def list_(prefix=None, bin_env=None, user=None, cwd=None, env_vars=None, **kwarg ) cmd = _get_pip_bin(bin_env) - cmd.extend(["list", "--format=json"]) + # ``--disable-pip-version-check`` keeps ``pip list`` from making an + # outbound HTTPS call to PyPI to check for a newer pip release. On + # airgapped minions that check times out (~20s per call), which makes + # every ``pip.installed`` state re-run unacceptably slow (issue #68214). + cmd.extend(["list", "--format=json", "--disable-pip-version-check"]) cmd_kwargs = dict(cwd=cwd, runas=user, python_shell=False) if kwargs: diff --git a/tests/pytests/unit/modules/test_pip.py b/tests/pytests/unit/modules/test_pip.py index e30bf1e138d..8507ff5ec3b 100644 --- a/tests/pytests/unit/modules/test_pip.py +++ b/tests/pytests/unit/modules/test_pip.py @@ -1331,7 +1331,7 @@ def test_uninstall_timeout_argument_in_resulting_command(python_binary): def test_freeze_command(python_binary): - expected = [*python_binary, "freeze"] + expected = [*python_binary, "freeze", "--disable-pip-version-check"] eggs = [ "M2Crypto==0.21.1", "-e git+git@github.com:s0undt3ch/salt-testing.git@9ed81aa2f918d59d3706e56b18f0782d1ea43bf8#egg=SaltTesting-dev", @@ -1391,7 +1391,12 @@ def test_freeze_command_with_all(python_binary): with patch.dict(pip.__salt__, {"cmd.run_all": mock}): with patch("salt.modules.pip.version", MagicMock(return_value="9.0.1")): ret = pip.freeze() - expected = [*python_binary, "freeze", "--all"] + expected = [ + *python_binary, + "freeze", + "--all", + "--disable-pip-version-check", + ] mock.assert_called_with( expected, cwd=None, @@ -1424,7 +1429,7 @@ def test_list_freeze_parse_command(python_binary): with patch.dict(pip.__salt__, {"cmd.run_all": mock}): with patch("salt.modules.pip.version", MagicMock(return_value=mock_version)): ret = pip.list_freeze_parse() - expected = [*python_binary, "freeze"] + expected = [*python_binary, "freeze", "--disable-pip-version-check"] mock.assert_called_with( expected, cwd=None, @@ -1469,7 +1474,12 @@ def test_list_freeze_parse_command_with_all(python_binary): with patch.dict(pip.__salt__, {"cmd.run_all": mock}): with patch("salt.modules.pip.version", MagicMock(return_value=mock_version)): ret = pip.list_freeze_parse() - expected = [*python_binary, "freeze", "--all"] + expected = [ + *python_binary, + "freeze", + "--all", + "--disable-pip-version-check", + ] mock.assert_called_with( expected, cwd=None, @@ -1509,7 +1519,7 @@ def test_list_freeze_parse_command_with_prefix(python_binary): with patch.dict(pip.__salt__, {"cmd.run_all": mock}): with patch("salt.modules.pip.version", MagicMock(return_value="6.1.1")): ret = pip.list_freeze_parse(prefix="bb") - expected = [*python_binary, "freeze"] + expected = [*python_binary, "freeze", "--disable-pip-version-check"] mock.assert_called_with( expected, cwd=None, @@ -1581,7 +1591,7 @@ def test_is_installed_true(python_binary): with patch("salt.modules.pip.version", MagicMock(return_value="6.1.1")): ret = pip.is_installed(pkgname="bbfreeze") mock.assert_called_with( - [*python_binary, "freeze"], + [*python_binary, "freeze", "--disable-pip-version-check"], cwd=None, runas=None, python_shell=False, @@ -1603,7 +1613,7 @@ def test_is_installed_false(python_binary): with patch("salt.modules.pip.version", MagicMock(return_value="6.1.1")): ret = pip.is_installed(pkgname="notexist") mock.assert_called_with( - [*python_binary, "freeze"], + [*python_binary, "freeze", "--disable-pip-version-check"], cwd=None, runas=None, python_shell=False, @@ -1669,17 +1679,26 @@ def test_when_upgrade_is_called_and_there_are_available_upgrades_it_should_call_ ): fake_run_all = MagicMock(return_value={"retcode": 0, "stdout": "{}"}) pip_user = expected_user + + def all_new_commands(*args, **kwargs): + """ + Return a fresh list from each ``_get_pip_bin`` call so the mutable + ``cmd`` lists built by ``pip.upgrade`` and ``pip.list_`` cannot alias + through a shared ``return_value`` list. + """ + return ["some-other-pip"] + with patch.dict(pip.__salt__, {"cmd.run_all": fake_run_all}), patch( "salt.modules.pip.list_upgrades", autospec=True, return_value=[pip_user] ), patch( "salt.modules.pip._get_pip_bin", autospec=True, - return_value=["some-other-pip"], + side_effect=all_new_commands, ): pip.upgrade(user=pip_user) fake_run_all.assert_any_call( - ["some-other-pip", "install", "-U", "list", "--format=json", pip_user], + ["some-other-pip", "install", "-U", pip_user], runas=pip_user, cwd=None, use_vt=False, @@ -1839,7 +1858,12 @@ def test_list(python_binary): with patch.dict(pip.__salt__, {"cmd.run_all": mock}): with patch("salt.modules.pip.version", MagicMock(return_value=mock_version)): ret = pip.list_() - expected = [*python_binary, "list", "--format=json"] + expected = [ + *python_binary, + "list", + "--format=json", + "--disable-pip-version-check", + ] mock.assert_called_with( expected, cwd=None, @@ -1865,3 +1889,57 @@ def test_list(python_binary): CommandExecutionError, pip.list_, ) + + +def test_list_disables_pip_version_check_issue_68214(python_binary): + """ + Regression test for #68214: ``pip.list`` must pass + ``--disable-pip-version-check`` to ``pip list`` so that pip does not try to + reach out to PyPI to check for a newer pip release. On airgapped minions + that outbound check times out (~20s per call), making every + ``pip.installed`` state re-run unacceptably slow. + """ + json_out = "[]" + mock = MagicMock(return_value={"retcode": 0, "stdout": json_out}) + with patch.dict(pip.__salt__, {"cmd.run_all": mock}): + with patch("salt.modules.pip.version", MagicMock(return_value="22.3.1")): + pip.list_() + expected = [ + *python_binary, + "list", + "--format=json", + "--disable-pip-version-check", + ] + mock.assert_called_with( + expected, + cwd=None, + runas=None, + python_shell=False, + ) + + +def test_freeze_disables_pip_version_check_issue_68214(python_binary): + """ + Regression test for #68214: ``pip.freeze`` must pass + ``--disable-pip-version-check`` to ``pip freeze`` for the same reason as + ``pip.list`` — ``pip.list_freeze_parse`` falls back to ``pip.freeze`` on + older pip versions, and the outbound version check blocks airgapped + minions. + """ + mock = MagicMock(return_value={"retcode": 0, "stdout": ""}) + with patch.dict(pip.__salt__, {"cmd.run_all": mock}): + with patch("salt.modules.pip.version", MagicMock(return_value="9.0.1")): + pip.freeze() + expected = [ + *python_binary, + "freeze", + "--all", + "--disable-pip-version-check", + ] + mock.assert_called_with( + expected, + cwd=None, + runas=None, + use_vt=False, + python_shell=False, + )