Skip to content

Commit

Permalink
Fix some tests
Browse files Browse the repository at this point in the history
  • Loading branch information
twangboy committed May 9, 2024
1 parent 7ce392d commit 9c357cb
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 51 deletions.
11 changes: 5 additions & 6 deletions changelog/61166.fixed.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
Fixes multiple issues with the cmd module on Windows. ``stderr`` is no
longer piped to ``stdout`` by default on ``cmd.run``. Scripts are called
using the ``-File`` parameter to the ``powershell.exe`` binary. ``CLIXML``
data in stderr is now removed (only applies to encoded commands). Commands can
now be sent to ``cmd.powershell`` as a list. Makes sure JSON data returned is
valid. Strips whitespace from the return when using ``runas``.
Fixes multiple issues with the cmd module on Windows. Scripts are called using
the ``-File`` parameter to the ``powershell.exe`` binary. ``CLIXML`` data in
stderr is now removed (only applies to encoded commands). Commands can now be
sent to ``cmd.powershell`` as a list. Makes sure JSON data returned is valid.
Strips whitespace from the return when using ``runas``.
17 changes: 11 additions & 6 deletions salt/modules/cmdmod.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,16 +276,21 @@ def _prep_powershell_cmd(win_shell, cmd, encoded_cmd):
stack = traceback.extract_stack(limit=3)
if stack[-3][2] == "script":
# If this is cmd.script, then we're running a file
new_cmd.extend(["-File", f"{cmd}"])
# You might be tempted to use -File here instead of -Command
# The problem with using -File is that any arguments that contain
# powershell commands themselves will not be evaluated
# See GitHub issue #56195
new_cmd.append("-Command")
if isinstance(cmd, list):
cmd = " ".join(cmd)
new_cmd.append(f"& {cmd.strip()}")
elif encoded_cmd:
new_cmd.extend(["-EncodedCommand", f"{cmd}"])
else:
# Strip whitespace
if isinstance(cmd, str):
cmd = cmd.strip()
elif isinstance(cmd, list):
cmd = " ".join(cmd).strip()
new_cmd.extend(["-Command", f"& {{{cmd}}}"])
if isinstance(cmd, list):
cmd = " ".join(cmd)
new_cmd.extend(["-Command", f"& {{{cmd.strip()}}}"])

log.debug(new_cmd)
return new_cmd
Expand Down
36 changes: 0 additions & 36 deletions tests/integration/modules/test_cmdmod.py
Original file line number Diff line number Diff line change
Expand Up @@ -596,39 +596,3 @@ def test_windows_env_handling(self):
).splitlines()
self.assertIn("abc=123", out)
self.assertIn("ABC=456", out)

@pytest.mark.slow_test
@pytest.mark.skip_unless_on_windows(reason="Minion is not Windows")
def test_windows_powershell_script_args(self):
"""
Ensure that powershell processes inline script in args
"""
val = "i like cheese"
args = (
'-SecureString (ConvertTo-SecureString -String "{}" -AsPlainText -Force)'
" -ErrorAction Stop".format(val)
)
script = "salt://issue-56195/test.ps1"
ret = self.run_function(
"cmd.script", [script], args=args, shell="powershell", saltenv="base"
)
self.assertEqual(ret["stdout"], val)

@pytest.mark.slow_test
@pytest.mark.skip_unless_on_windows(reason="Minion is not Windows")
@pytest.mark.skip_if_binaries_missing("pwsh")
def test_windows_powershell_script_args_pwsh(self):
"""
Ensure that powershell processes inline script in args with powershell
core
"""
val = "i like cheese"
args = (
'-SecureString (ConvertTo-SecureString -String "{}" -AsPlainText -Force)'
" -ErrorAction Stop".format(val)
)
script = "salt://issue-56195/test.ps1"
ret = self.run_function(
"cmd.script", [script], args=args, shell="pwsh", saltenv="base"
)
self.assertEqual(ret["stdout"], val)
5 changes: 4 additions & 1 deletion tests/pytests/functional/modules/cmd/test_powershell.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ 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
Expand Down Expand Up @@ -178,7 +179,9 @@ def test_cmd_run_encoded_cmd(shell, cmd, expected, encoded_cmd):
"""
Ensure that cmd.run supports running shell='powershell'
"""
ret = cmdmod.run(cmd=cmd, shell=shell, encoded_cmd=encoded_cmd, redirect_stderr=False)
ret = cmdmod.run(
cmd=cmd, shell=shell, encoded_cmd=encoded_cmd, redirect_stderr=False
)
assert ret == expected


Expand Down
103 changes: 103 additions & 0 deletions tests/pytests/functional/modules/cmd/test_script.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import pytest

import salt.modules.cmdmod as cmdmod
import salt.modules.cp as cp
import salt.utils.path
import salt.utils.platform

if salt.utils.platform.is_windows():
import salt.modules.win_file as file
else:
import salt.modules.file as file

pytestmark = [
pytest.mark.core_test,
pytest.mark.windows_whitelisted,
]


@pytest.fixture(scope="module")
def configure_loader_modules(minion_opts):
return {
cmdmod: {
"__salt__": {"cp.cache_file": cp.cache_file, "file.remove": file.remove},
"__opts__": minion_opts,
},
cp: {
"__opts__": minion_opts,
},
}


@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


@pytest.fixture(scope="module")
def account():
with pytest.helpers.create_account() as _account:
yield _account


@pytest.fixture
def issue_56195(state_tree):
contents = """
[CmdLetBinding()]
Param(
[SecureString] $SecureString
)
$Credential = New-Object System.Net.NetworkCredential("DummyId", $SecureString)
$Credential.Password
"""
with pytest.helpers.temp_file("test.ps1", contents, state_tree / "issue-56195"):
yield


@pytest.mark.skip_unless_on_windows(reason="Minion is not Windows")
def test_windows_script_args_powershell(shell, issue_56195):
"""
Ensure that powershell processes an inline script with args where the args
contain powershell that needs to be rendered
"""
password = "i like cheese"
args = (
"-SecureString (ConvertTo-SecureString -String '{}' -AsPlainText -Force)"
" -ErrorAction Stop".format(password)
)
script = "salt://issue-56195/test.ps1"

ret = cmdmod.script(source=script, args=args, shell="powershell", saltenv="base")

assert ret["stdout"] == password


@pytest.mark.skip_unless_on_windows(reason="Minion is not Windows")
def test_windows_script_args_powershell_runas(shell, account, issue_56195):
"""
Ensure that powershell processes an inline script with args where the args
contain powershell that needs to be rendered
"""
password = "i like cheese"
args = (
"-SecureString (ConvertTo-SecureString -String '{}' -AsPlainText -Force)"
" -ErrorAction Stop".format(password)
)
script = "salt://issue-56195/test.ps1"

ret = cmdmod.script(
source=script,
args=args,
shell="powershell",
saltenv="base",
runas=account.username,
password=account.password,
)

assert ret["stdout"] == password
3 changes: 3 additions & 0 deletions tests/pytests/functional/modules/test_win_useradd.py
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,9 @@ def test_setpassword_int(user, account_int):
)
def test_update_str(user, value_name, new_value, info_field, expected, account_str):
setting = {value_name: new_value}
# You can't expire an account if the password never expires
if value_name == "expired":
setting.update({"password_never_expires": not new_value})
ret = user.update(account_str.username, **setting)
assert ret is True
ret = user.info(account_str.username)
Expand Down
2 changes: 2 additions & 0 deletions tests/pytests/integration/modules/test_cmdmod.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ def non_root_account():
yield account


@pytest.mark.skip_on_windows
@pytest.mark.skip_if_not_root
def test_exec_code_all(salt_call_cli, non_root_account):
ret = salt_call_cli.run(
Expand All @@ -15,6 +16,7 @@ def test_exec_code_all(salt_call_cli, non_root_account):
assert ret.returncode == 0


@pytest.mark.skip_on_windows
def test_long_stdout(salt_cli, salt_minion):
echo_str = "salt" * 1000
ret = salt_cli.run(
Expand Down
4 changes: 2 additions & 2 deletions tests/pytests/unit/modules/test_cmdmod.py
Original file line number Diff line number Diff line change
Expand Up @@ -1125,8 +1125,8 @@ def test_prep_powershell_cmd_script():
"-NoProfile",
"-ExecutionPolicy",
"Bypass",
"-File",
script,
"-Command",
f"& {script}",
]
assert ret == expected

Expand Down

0 comments on commit 9c357cb

Please sign in to comment.