Pass --disable-pip-version-check from pip.list and pip.freeze (#68214)

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.

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:

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,
+            )