From 45cf38f4b40fe764aea6c1d82f1aed2287a8be0a Mon Sep 17 00:00:00 2001
From: Shane Lee <shane.d.lee@gmail.com>
Date: Tue, 27 Feb 2024 08:59:45 -0700
Subject: [PATCH] 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                   |  1 +
 salt/modules/win_pki.py                    | 12 ++++--
 tests/pytests/unit/modules/test_win_pki.py | 46 +++++++++++++++++++++-
 3 files changed, 54 insertions(+), 5 deletions(-)
 create mode 100644 changelog/64933.fixed.md

diff --git a/changelog/64933.fixed.md b/changelog/64933.fixed.md
new file mode 100644
index 00000000000..e6f233db29d
--- /dev/null
+++ b/changelog/64933.fixed.md
@@ -0,0 +1 @@
+Display a proper error when pki commands fail in the win_pki module
diff --git a/salt/modules/win_pki.py b/salt/modules/win_pki.py
index e004f0bc9e8..6cc68309ad7 100644
--- a/salt/modules/win_pki.py
+++ b/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"]
 
diff --git a/tests/pytests/unit/modules/test_win_pki.py b/tests/pytests/unit/modules/test_win_pki.py
index 600282e8bd8..e90f4368e9f 100644
--- a/tests/pytests/unit/modules/test_win_pki.py
+++ b/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