Fix an issue with the win_pki execution module

Change the _cmd_run function so that it raises a CommandExecutionError
when the command fails so that it doesn't try to parse the output of the
command as JSON
Also raise a CommandExecutionError when it recieves invalid JSON

changelog/64933.fixed.md

@@ -0,0 +1 @@
+Display a proper error when pki commands fail in the win_pki module

salt/modules/win_pki.py

@@ -23,7 +23,7 @@ import salt.utils.json
 import salt.utils.platform
 import salt.utils.powershell
 import salt.utils.versions
-from salt.exceptions import SaltInvocationError
+from salt.exceptions import CommandExecutionError, SaltInvocationError
 
 _DEFAULT_CONTEXT = "LocalMachine"
 _DEFAULT_FORMAT = "cer"
@@ -73,15 +73,19 @@ def _cmd_run(cmd, as_json=False):
         "".join(cmd_full), shell="powershell", python_shell=True
     )
 
-    if cmd_ret["retcode"] != 0:
-        _LOG.error("Unable to execute command: %s\nError: %s", cmd, cmd_ret["stderr"])
+    if cmd_ret["stderr"]:
+        raise CommandExecutionError(
+            "Unable to execute command: {}\nError: {}".format(cmd, cmd_ret["stderr"])
+        )
 
     if as_json:
         try:
             items = salt.utils.json.loads(cmd_ret["stdout"], strict=False)
             return items
         except ValueError:
-            _LOG.error("Unable to parse return data as Json.")
+            raise CommandExecutionError(
+                "Unable to parse return data as JSON:\n{}".format(cmd_ret["stdout"])
+            )
 
     return cmd_ret["stdout"]
 

tests/pytests/unit/modules/test_win_pki.py

@@ -1,10 +1,11 @@
 """
-    Test cases for salt.modules.win_pki
+Test cases for salt.modules.win_pki
 """
 
 import pytest
 
 import salt.modules.win_pki as win_pki
+from salt.exceptions import CommandExecutionError
 from tests.support.mock import MagicMock, patch
 
 
@@ -181,3 +182,46 @@ def test_remove_cert(thumbprint, certs):
         "salt.modules.win_pki.get_certs", MagicMock(return_value=certs)
     ):
         assert win_pki.remove_cert(thumbprint=thumbprint[::-1])
+
+
+def test__cmd_run():
+    """
+    Test the _cmd_run function
+    """
+    mock_run = MagicMock(
+        return_value={"retcode": 0, "stderr": "", "stdout": "some result"}
+    )
+    with patch.dict(win_pki.__salt__, {"cmd.run_all": mock_run}):
+        result = win_pki._cmd_run(cmd="command")
+        assert result == "some result"
+
+
+def test__cmd_run_as_json():
+    mock_run = MagicMock(
+        return_value={"retcode": 0, "stderr": "", "stdout": '{"key": "value"}'}
+    )
+    with patch.dict(win_pki.__salt__, {"cmd.run_all": mock_run}):
+        result = win_pki._cmd_run(cmd="command", as_json=True)
+        assert result == {"key": "value"}
+
+
+def test__cmd_run_stderr():
+    mock_run = MagicMock(
+        return_value={"retcode": 0, "stderr": "some error", "stdout": ""}
+    )
+    with patch.dict(win_pki.__salt__, {"cmd.run_all": mock_run}):
+        with pytest.raises(CommandExecutionError) as exc_info:
+            win_pki._cmd_run(cmd="command")
+        expected = "Unable to execute command: command\nError: some error"
+        assert exc_info.value.args[0] == expected
+
+
+def test__cmd_run_bad_json():
+    mock_run = MagicMock(
+        return_value={"retcode": 0, "stderr": "", "stdout": "not : valid\njson"}
+    )
+    with patch.dict(win_pki.__salt__, {"cmd.run_all": mock_run}):
+        with pytest.raises(CommandExecutionError) as exc_info:
+            win_pki._cmd_run(cmd="command", as_json=True)
+        expected = "Unable to parse return data as JSON:\nnot : valid\njson"
+        assert exc_info.value.args[0] == expected