diff --git a/changelog/58202.fixed b/changelog/58202.fixed new file mode 100644 index 000000000000..b4cf0c0add78 --- /dev/null +++ b/changelog/58202.fixed @@ -0,0 +1 @@ +Fix salt.modules.pip:is_installed doesn't handle locally installed packages diff --git a/salt/modules/pip.py b/salt/modules/pip.py index 92f43b04e2b4..3b4e60393f0e 100644 --- a/salt/modules/pip.py +++ b/salt/modules/pip.py @@ -1235,8 +1235,12 @@ def freeze(bin_env=None, user=None, cwd=None, use_vt=False, env_vars=None, **kwa return result["stdout"].splitlines() -def list_(prefix=None, bin_env=None, user=None, cwd=None, env_vars=None, **kwargs): +def list_freeze_parse( + prefix=None, bin_env=None, user=None, cwd=None, env_vars=None, **kwargs +): """ + .. versionadded:: 3007.0 + Filter list of installed apps from ``freeze`` and check to see if ``prefix`` exists in the list of packages installed. @@ -1252,7 +1256,7 @@ def list_(prefix=None, bin_env=None, user=None, cwd=None, env_vars=None, **kwarg .. code-block:: bash - salt '*' pip.list salt + salt '*' pip.list_freeze_parse salt """ cwd = _pip_bin_env(cwd, bin_env) @@ -1301,6 +1305,73 @@ def list_(prefix=None, bin_env=None, user=None, cwd=None, env_vars=None, **kwarg return packages +def list_(prefix=None, bin_env=None, user=None, cwd=None, env_vars=None, **kwargs): + """ + .. versionchanged:: 3007.0 + + Output list of installed apps from ``pip list`` in JSON format and check to + see if ``prefix`` exists in the list of packages installed. + + .. note:: + + If the version of pip available is older than 9.0.0, parsing the + ``freeze`` function output will be used to determine the name and + version of installed modules. + + CLI Example: + + .. code-block:: bash + + salt '*' pip.list salt + """ + + packages = {} + cwd = _pip_bin_env(cwd, bin_env) + cur_version = version(bin_env, cwd, user=user) + + # Pip started supporting the ability to output json starting with 9.0.0 + min_version = "9.0" + if salt.utils.versions.compare(ver1=cur_version, oper="<", ver2=min_version): + return list_freeze_parse( + prefix=prefix, + bin_env=bin_env, + user=user, + cwd=cwd, + env_vars=env_vars, + **kwargs + ) + + cmd = _get_pip_bin(bin_env) + cmd.extend(["list", "--format=json"]) + + cmd_kwargs = dict(cwd=cwd, runas=user, python_shell=False) + if kwargs: + cmd_kwargs.update(**kwargs) + if bin_env and os.path.isdir(bin_env): + cmd_kwargs["env"] = {"VIRTUAL_ENV": bin_env} + if env_vars: + cmd_kwargs.setdefault("env", {}).update(_format_env_vars(env_vars)) + + result = __salt__["cmd.run_all"](cmd, **cmd_kwargs) + + if result["retcode"]: + raise CommandExecutionError(result["stderr"], info=result) + + try: + pkgs = salt.utils.json.loads(result["stdout"], strict=False) + except ValueError: + raise CommandExecutionError("Invalid JSON", info=result) + + for pkg in pkgs: + if prefix: + if pkg["name"].lower().startswith(prefix.lower()): + packages[pkg["name"]] = pkg["version"] + else: + packages[pkg["name"]] = pkg["version"] + + return packages + + def version(bin_env=None, cwd=None, user=None): """ .. versionadded:: 0.17.0 @@ -1423,19 +1494,13 @@ def list_upgrades(bin_env=None, user=None, cwd=None): return packages -def is_installed(pkgname=None, bin_env=None, user=None, cwd=None): +def is_installed(pkgname, bin_env=None, user=None, cwd=None): """ .. versionadded:: 2018.3.0 + .. versionchanged:: 3007.0 - Filter list of installed apps from ``freeze`` and return True or False if - ``pkgname`` exists in the list of packages installed. - - .. note:: - If the version of pip available is older than 8.0.3, the packages - wheel, setuptools, and distribute will not be reported by this function - even if they are installed. Unlike :py:func:`pip.freeze - `, this function always reports the version of - pip which is installed. + Filter list of installed modules and return True if ``pkgname`` exists in + the list of packages installed. CLI Example: @@ -1445,30 +1510,11 @@ def is_installed(pkgname=None, bin_env=None, user=None, cwd=None): """ cwd = _pip_bin_env(cwd, bin_env) - for line in freeze(bin_env=bin_env, user=user, cwd=cwd): - if line.startswith("-f") or line.startswith("#"): - # ignore -f line as it contains --find-links directory - # ignore comment lines - continue - elif line.startswith("-e hg+not trust"): - # ignore hg + not trust problem - continue - elif line.startswith("-e"): - line = line.split("-e ")[1] - version_, name = line.split("#egg=") - elif len(line.split("===")) >= 2: - name = line.split("===")[0] - version_ = line.split("===")[1] - elif len(line.split("==")) >= 2: - name = line.split("==")[0] - version_ = line.split("==")[1] - else: - logger.error("Can't parse line '%s'", line) - continue + pkgs = list_(prefix=pkgname, bin_env=bin_env, user=user, cwd=cwd) - if pkgname: - if pkgname == name.lower(): - return True + for pkg in pkgs: + if pkg.lower() == pkgname.lower(): + return True return False diff --git a/tests/pytests/unit/modules/test_pip.py b/tests/pytests/unit/modules/test_pip.py index 0e47395ad076..9e8c276790e3 100644 --- a/tests/pytests/unit/modules/test_pip.py +++ b/tests/pytests/unit/modules/test_pip.py @@ -1,5 +1,6 @@ import os import sys +from textwrap import dedent import pytest @@ -1422,7 +1423,7 @@ def test_freeze_command_with_all(): ) -def test_list_command(): +def test_list_freeze_parse_command(): eggs = [ "M2Crypto==0.21.1", "-e git+git@github.com:s0undt3ch/salt-testing.git@9ed81aa2f918d59d3706e56b18f0782d1ea43bf8#egg=SaltTesting-dev", @@ -1434,7 +1435,7 @@ def test_list_command(): mock = MagicMock(return_value={"retcode": 0, "stdout": "\n".join(eggs)}) with patch.dict(pip.__salt__, {"cmd.run_all": mock}): with patch("salt.modules.pip.version", MagicMock(return_value=mock_version)): - ret = pip.list_() + ret = pip.list_freeze_parse() expected = [sys.executable, "-m", "pip", "freeze"] mock.assert_called_with( expected, @@ -1458,11 +1459,11 @@ def test_list_command(): with patch("salt.modules.pip.version", MagicMock(return_value="6.1.1")): pytest.raises( CommandExecutionError, - pip.list_, + pip.list_freeze_parse, ) -def test_list_command_with_all(): +def test_list_freeze_parse_command_with_all(): eggs = [ "M2Crypto==0.21.1", "-e git+git@github.com:s0undt3ch/salt-testing.git@9ed81aa2f918d59d3706e56b18f0782d1ea43bf8#egg=SaltTesting-dev", @@ -1479,7 +1480,7 @@ def test_list_command_with_all(): mock = MagicMock(return_value={"retcode": 0, "stdout": "\n".join(eggs)}) with patch.dict(pip.__salt__, {"cmd.run_all": mock}): with patch("salt.modules.pip.version", MagicMock(return_value=mock_version)): - ret = pip.list_() + ret = pip.list_freeze_parse() expected = [sys.executable, "-m", "pip", "freeze", "--all"] mock.assert_called_with( expected, @@ -1504,11 +1505,11 @@ def test_list_command_with_all(): with patch("salt.modules.pip.version", MagicMock(return_value="6.1.1")): pytest.raises( CommandExecutionError, - pip.list_, + pip.list_freeze_parse, ) -def test_list_command_with_prefix(): +def test_list_freeze_parse_command_with_prefix(): eggs = [ "M2Crypto==0.21.1", "-e git+git@github.com:s0undt3ch/salt-testing.git@9ed81aa2f918d59d3706e56b18f0782d1ea43bf8#egg=SaltTesting-dev", @@ -1519,7 +1520,7 @@ def test_list_command_with_prefix(): mock = MagicMock(return_value={"retcode": 0, "stdout": "\n".join(eggs)}) with patch.dict(pip.__salt__, {"cmd.run_all": mock}): with patch("salt.modules.pip.version", MagicMock(return_value="6.1.1")): - ret = pip.list_(prefix="bb") + ret = pip.list_freeze_parse(prefix="bb") expected = [sys.executable, "-m", "pip", "freeze"] mock.assert_called_with( expected, @@ -1680,7 +1681,7 @@ def test_resolve_requirements_chain_function(): def test_when_upgrade_is_called_and_there_are_available_upgrades_it_should_call_correct_command( expected_user, ): - fake_run_all = MagicMock(return_value={"retcode": 0, "stdout": ""}) + fake_run_all = MagicMock(return_value={"retcode": 0, "stdout": "{}"}) pip_user = expected_user with patch.dict(pip.__salt__, {"cmd.run_all": fake_run_all}), patch( "salt.modules.pip.list_upgrades", autospec=True, return_value=[pip_user] @@ -1692,7 +1693,7 @@ def test_when_upgrade_is_called_and_there_are_available_upgrades_it_should_call_ pip.upgrade(user=pip_user) fake_run_all.assert_any_call( - ["some-other-pip", "install", "-U", "freeze", "--all", pip_user], + ["some-other-pip", "install", "-U", "list", "--format=json", pip_user], runas=pip_user, cwd=None, use_vt=False, @@ -1805,3 +1806,76 @@ def test_install_target_from_VENV_PIP_TARGET_in_resulting_command(): use_vt=False, python_shell=False, ) + + +def test_list(): + json_out = dedent( + """ + [ + { + "name": "idemenv", + "version": "0.2.0", + "editable_project_location": "/home/debian/idemenv" + }, + { + "name": "MarkupSafe", + "version": "2.1.1" + }, + { + "name": "pip", + "version": "22.3.1" + }, + { + "name": "pop", + "version": "23.0.0" + }, + { + "name": "salt", + "version": "3006.0+0na.5b18e86" + }, + { + "name": "typing_extensions", + "version": "4.4.0" + }, + { + "name": "unattended-upgrades", + "version": "0.1" + }, + { + "name": "yarl", + "version": "1.8.2" + } + ] + """ + ) + mock_version = "22.3.1" + 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=mock_version)): + ret = pip.list_() + expected = [sys.executable, "-m", "pip", "list", "--format=json"] + mock.assert_called_with( + expected, + cwd=None, + runas=None, + python_shell=False, + ) + assert ret == { + "MarkupSafe": "2.1.1", + "idemenv": "0.2.0", + "pip": "22.3.1", + "pop": "23.0.0", + "salt": "3006.0+0na.5b18e86", + "typing_extensions": "4.4.0", + "unattended-upgrades": "0.1", + "yarl": "1.8.2", + } + + # Non zero returncode raises exception? + mock = MagicMock(return_value={"retcode": 1, "stderr": "CABOOOOMMM!"}) + with patch.dict(pip.__salt__, {"cmd.run_all": mock}): + with patch("salt.modules.pip.version", MagicMock(return_value="22.3.1")): + pytest.raises( + CommandExecutionError, + pip.list_, + )