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 14, 2024
1 parent 03bb18a commit 8f33dc5
Show file tree
Hide file tree
Showing 8 changed files with 122 additions and 54 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
13 changes: 10 additions & 3 deletions salt/modules/win_dsc.py
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,9 @@ def get_config():
raise

config = dict()
if raw_config:
if not raw_config:
raise CommandExecutionError("Not Configured")
else:
# Does this Configuration contain a single resource
if "ConfigurationName" in raw_config:
# Load the single resource
Expand Down Expand Up @@ -606,11 +608,13 @@ def test_config():
"""
cmd = "Test-DscConfiguration"
try:
_pshell(cmd, ignore_retcode=True)
result = _pshell(cmd, ignore_retcode=True)
except CommandExecutionError as exc:
if "Current configuration does not exist" in exc.info["stderr"]:
raise CommandExecutionError("Not Configured")
raise
if not result:
raise CommandExecutionError("Not Configured")
return True


Expand All @@ -635,11 +639,14 @@ def get_config_status():
"Type, Mode, RebootRequested, NumberofResources"
)
try:
return _pshell(cmd, ignore_retcode=True)
result = _pshell(cmd, ignore_retcode=True)
except CommandExecutionError as exc:
if "No status information available" in exc.info["stderr"]:
raise CommandExecutionError("Not Configured")
raise
if not result:
raise CommandExecutionError("Not Configured")
return result


def get_lcm_config():
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
87 changes: 87 additions & 0 deletions tests/pytests/functional/modules/cmd/test_script.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import pytest

import salt.utils.path

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


@pytest.fixture(scope="module")
def cmd(modules):
return modules.cmd


@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(cmd, 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 = cmd.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(cmd, 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 = cmd.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
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 8f33dc5

Please sign in to comment.