fixes saltstack#58202 pip.is_installed doesn't handle locally install…

…ed packages

changelog/58202.fixed

@@ -0,0 +1 @@
+Fix salt.modules.pip:is_installed doesn't handle locally installed packages

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
-        <salt.modules.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
 

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 [email protected]: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 [email protected]: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 [email protected]: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_,
+            )