From 0fcde710627b8a29ff48de542fe9f88905add3ee Mon Sep 17 00:00:00 2001 From: Shane Lee Date: Wed, 17 Jul 2024 14:47:14 -0600 Subject: [PATCH] Fixes cmd.run with requisites on Windows Formats the command properly for powershell Adds changelog and tests --- changelog/66596.fixed.md | 2 + salt/modules/cmdmod.py | 7 +- .../functional/states/cmd/test_cmd_run.py | 64 +++++++++++++++++++ tests/pytests/unit/modules/test_cmdmod.py | 13 +++- tests/pytests/unit/modules/test_pip.py | 4 +- 5 files changed, 84 insertions(+), 6 deletions(-) create mode 100644 changelog/66596.fixed.md create mode 100644 tests/pytests/functional/states/cmd/test_cmd_run.py diff --git a/changelog/66596.fixed.md b/changelog/66596.fixed.md new file mode 100644 index 00000000000..a4a27151f2c --- /dev/null +++ b/changelog/66596.fixed.md @@ -0,0 +1,2 @@ +Fixed an issue with cmd.run with requirements when the shell is not the +default diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py index 0b50b14dbb9..c76357ee1ac 100644 --- a/salt/modules/cmdmod.py +++ b/salt/modules/cmdmod.py @@ -290,7 +290,11 @@ def _prep_powershell_cmd(win_shell, cmd, encoded_cmd): # Strip whitespace if isinstance(cmd, list): cmd = " ".join(cmd) - new_cmd.extend(["-Command", f"& {{{cmd.strip()}}}"]) + + if cmd.startswith("$"): + new_cmd.extend(["-Command", f"{cmd.strip()}"]) + else: + new_cmd.extend(["-Command", f"& {cmd.strip()}"]) log.debug(new_cmd) return new_cmd @@ -4104,6 +4108,7 @@ def powershell( cmd = salt.utils.stringutils.to_str(cmd) encoded_cmd = True else: + cmd = f"{{{cmd}}}" encoded_cmd = False # Retrieve the response, while overriding shell with 'powershell' diff --git a/tests/pytests/functional/states/cmd/test_cmd_run.py b/tests/pytests/functional/states/cmd/test_cmd_run.py new file mode 100644 index 00000000000..22a5ba65698 --- /dev/null +++ b/tests/pytests/functional/states/cmd/test_cmd_run.py @@ -0,0 +1,64 @@ +import os + +import pytest + +import salt.utils.path + +pytestmark = [ + pytest.mark.windows_whitelisted, + pytest.mark.skip_unless_on_windows, + pytest.mark.destructive_test, + pytest.mark.slow_test, +] + + +@pytest.fixture(params=["powershell", "pwsh"]) +def shell(request): + """ + This will run the test on powershell and powershell core (pwsh). If + powershell core is not installed that test run will be skipped + """ + + if request.param == "pwsh" and salt.utils.path.which("pwsh") is None: + pytest.skip("Powershell 7 Not Present") + return request.param + + +def test_cmd_run_unless_true(shell, cmd): + # We need a directory that we know exists that has stuff in it + win_dir = os.getenv("WINDIR") + ret = cmd.run(name="echo foo", unless=f"ls {win_dir}", shell=shell) + assert ret.filtered["result"] is True + assert ret.filtered["name"] == "echo foo" + assert ret.filtered["comment"] == "unless condition is true" + assert ret.filtered["changes"] == {} + + +def test_cmd_run_unless_false(shell, cmd): + # We need a directory that we know does not exist + win_dir = "C:\\This\\Dir\\Does\\Not\\Exist" + ret = cmd.run(name="echo foo", unless=f"ls {win_dir}", shell=shell) + assert ret.filtered["result"] is True + assert ret.filtered["name"] == "echo foo" + assert ret.filtered["comment"] == 'Command "echo foo" run' + assert ret.filtered["changes"]["stdout"] == "foo" + + +def test_cmd_run_onlyif_true(shell, cmd): + # We need a directory that we know exists that has stuff in it + win_dir = os.getenv("WINDIR") + ret = cmd.run(name="echo foo", onlyif=f"ls {win_dir}", shell=shell) + assert ret.filtered["result"] is True + assert ret.filtered["name"] == "echo foo" + assert ret.filtered["comment"] == 'Command "echo foo" run' + assert ret.filtered["changes"]["stdout"] == "foo" + + +def test_cmd_run_onlyif_false(shell, cmd): + # We need a directory that we know does not exist + win_dir = "C:\\This\\Dir\\Does\\Not\\Exist" + ret = cmd.run(name="echo foo", onlyif=f"ls {win_dir}", shell=shell) + assert ret.filtered["result"] is True + assert ret.filtered["name"] == "echo foo" + assert ret.filtered["comment"] == "onlyif condition is false" + assert ret.filtered["changes"] == {} diff --git a/tests/pytests/unit/modules/test_cmdmod.py b/tests/pytests/unit/modules/test_cmdmod.py index cfc031fc063..e1f2a604cd1 100644 --- a/tests/pytests/unit/modules/test_cmdmod.py +++ b/tests/pytests/unit/modules/test_cmdmod.py @@ -1059,7 +1059,14 @@ def test_prep_powershell_cmd_no_powershell(): ) -def test_prep_powershell_cmd(): +@pytest.mark.parametrize( + "cmd, parsed", + [ + ("Write-Host foo", "& Write-Host foo"), + ("$PSVersionTable", "$PSVersionTable"), + ], +) +def test_prep_powershell_cmd(cmd, parsed): """ Tests _prep_powershell_cmd returns correct cmd """ @@ -1068,7 +1075,7 @@ def test_prep_powershell_cmd(): "salt.utils.path.which", return_value="C:\\powershell.exe" ): ret = cmdmod._prep_powershell_cmd( - win_shell="powershell", cmd="$PSVersionTable", encoded_cmd=False + win_shell="powershell", cmd=cmd, encoded_cmd=False ) expected = [ "C:\\powershell.exe", @@ -1077,7 +1084,7 @@ def test_prep_powershell_cmd(): "-ExecutionPolicy", "Bypass", "-Command", - "& {$PSVersionTable}", + parsed, ] assert ret == expected diff --git a/tests/pytests/unit/modules/test_pip.py b/tests/pytests/unit/modules/test_pip.py index c003e74aacb..87ffe29a857 100644 --- a/tests/pytests/unit/modules/test_pip.py +++ b/tests/pytests/unit/modules/test_pip.py @@ -474,10 +474,10 @@ def test_install_venv(): ) -def test_install_log_argument_in_resulting_command(python_binary): +def test_install_log_argument_in_resulting_command(python_binary, tmp_path): with patch("os.access") as mock_path: pkg = "pep8" - log_path = "/tmp/pip-install.log" + log_path = str(tmp_path / "pip-install.log") mock = MagicMock(return_value={"retcode": 0, "stdout": ""}) with patch.dict(pip.__salt__, {"cmd.run_all": mock}): pip.install(pkg, log=log_path)