Add tests for issues saltstack#64531 and saltstack#52452

tests/pytests/integration/ssh/test_deploy.py

@@ -28,6 +28,119 @@ def thin_dir(salt_ssh_cli):
         shutil.rmtree(thin_dir_path, ignore_errors=True)
 
 
+@pytest.fixture(scope="module")
+def invalid_json_exe_mod(salt_run_cli, base_env_state_tree_root_dir):
+    module_contents = r"""
+import os
+import sys
+
+
+def __virtual__():
+    return "whoops"
+
+
+def test():
+    data = '{\n  "local": {\n    "whoops": "hrhrhr"\n  }\n}'
+    ctr = 0
+    for line in data.splitlines():
+        sys.stdout.write(line)
+        if ctr == 3:
+            print("Warning: Chaos is not a letter")
+        ctr += 1
+    sys.stdout.flush()
+    os._exit(0)
+"""
+    module_dir = base_env_state_tree_root_dir / "_modules"
+    module_tempfile = pytest.helpers.temp_file("whoops.py", module_contents, module_dir)
+    try:
+        with module_tempfile:
+            ret = salt_run_cli.run("saltutil.sync_modules")
+            assert ret.returncode == 0
+            assert "modules.whoops" in ret.data
+            yield
+    finally:
+        ret = salt_run_cli.run("saltutil.sync_modules")
+        assert ret.returncode == 0
+
+
+@pytest.fixture(scope="module")
+def invalid_return_exe_mod(salt_run_cli, base_env_state_tree_root_dir):
+    module_contents = r"""
+import json
+import os
+import sys
+
+
+def __virtual__():
+    return "whoopsiedoodle"
+
+
+def test(wrapped=True):
+    data = "Chaos is a ladder though"
+    if wrapped:
+        data = {"local": {"no_return_key_present": data}}
+    else:
+        data = {"no_local_key_present": data}
+
+    print(json.dumps(data))
+    sys.stdout.flush()
+    os._exit(0)
+"""
+    module_dir = base_env_state_tree_root_dir / "_modules"
+    module_tempfile = pytest.helpers.temp_file(
+        "whoopsiedoodle.py", module_contents, module_dir
+    )
+    try:
+        with module_tempfile:
+            ret = salt_run_cli.run("saltutil.sync_modules")
+            assert ret.returncode == 0
+            assert "modules.whoopsiedoodle" in ret.data
+            yield
+    finally:
+        ret = salt_run_cli.run("saltutil.sync_modules")
+        assert ret.returncode == 0
+
+
+@pytest.fixture(scope="module")
+def remote_exception_wrap_mod(salt_master):
+    module_contents = r"""
+def __virtual__():
+    return "check_exception"
+
+
+def failure():
+    # This should raise an exception
+    ret = __salt__["disk.usage"]("c")
+    return f"Probably got garbage: {ret}"
+"""
+    module_dir = pathlib.Path(salt_master.config["extension_modules"]) / "wrapper"
+    module_tempfile = pytest.helpers.temp_file(
+        "check_exception.py", module_contents, module_dir
+    )
+    with module_tempfile:
+        yield
+
+
+@pytest.fixture(scope="module")
+def remote_parsing_failure_wrap_mod(salt_master, invalid_json_exe_mod):
+    module_contents = r"""
+def __virtual__():
+    return "check_parsing"
+
+
+def failure(mod):
+    # This should raise an exception
+    ret = __salt__[f"{mod}.test"]()
+    return f"Probably got garbage: {ret}"
+"""
+    module_dir = pathlib.Path(salt_master.config["extension_modules"]) / "wrapper"
+    module_tempfile = pytest.helpers.temp_file(
+        "check_parsing.py", module_contents, module_dir
+    )
+    with module_tempfile:
+        yield
+
+
 def test_ping(salt_ssh_cli):
     """
     Test a simple ping
@@ -122,3 +235,65 @@ def test_retcode_exe_run_exception(salt_ssh_cli):
     assert isinstance(ret.data, dict)
     assert ret.data["stderr"].endswith("Exception: hehehe")
     assert ret.data["retcode"] == 1
+
+
+@pytest.mark.usefixtures("invalid_json_exe_mod")
+def test_retcode_json_decode_error(salt_ssh_cli):
+    """
+    Verify salt-ssh exits with a non-zero exit code when
+    it cannot decode the output of a command.
+    """
+    ret = salt_ssh_cli.run("whoops.test")
+    assert ret.returncode == EX_AGGREGATE
+    assert isinstance(ret.data, dict)
+    assert (
+        ret.data["stdout"]
+        == '{  "local": {    "whoops": "hrhrhr"  }Warning: Chaos is not a letter\n}'
+    )
+    assert ret.data["retcode"] == 0
+
+
+@pytest.mark.usefixtures("invalid_return_exe_mod")
+def test_retcode_invalid_return(salt_ssh_cli):
+    """
+    Verify salt-ssh exits with a non-zero exit code when
+    the decoded command output is invalid.
+    """
+    ret = salt_ssh_cli.run("whoopsiedoodle.test", "false")
+    assert ret.returncode == EX_AGGREGATE
+    assert isinstance(ret.data, dict)
+    assert ret.data["stdout"] == '{"no_local_key_present": "Chaos is a ladder though"}'
+    assert ret.data["retcode"] == 0
+
+
+@pytest.mark.usefixtures("remote_exception_wrap_mod")
+def test_wrapper_unwrapped_command_exception(salt_ssh_cli):
+    """
+    Verify salt-ssh does not return unexpected exception output to wrapper modules.
+    """
+    ret = salt_ssh_cli.run("check_exception.failure")
+    assert ret.data
+    assert "Probably got garbage" not in ret.data
+    assert ret.returncode == EX_AGGREGATE
+
+
+@pytest.mark.usefixtures("remote_parsing_failure_wrap_mod", "invalid_json_exe_mod")
+def test_wrapper_unwrapped_command_parsing_failure(salt_ssh_cli):
+    """
+    Verify salt-ssh does not return unexpected unparsable output to wrapper modules.
+    """
+    ret = salt_ssh_cli.run("check_parsing.failure", "whoops")
+    assert ret.data
+    assert "Probably got garbage" not in ret.data
+    assert ret.returncode == EX_AGGREGATE
+
+
+@pytest.mark.usefixtures("remote_parsing_failure_wrap_mod", "invalid_return_exe_mod")
+def test_wrapper_unwrapped_command_invalid_return(salt_ssh_cli):
+    """
+    Verify salt-ssh does not return unexpected unparsable output to wrapper modules.
+    """
+    ret = salt_ssh_cli.run("check_parsing.failure", "whoopsiedoodle")
+    assert ret.data
+    assert "Probably got garbage" not in ret.data
+    assert ret.returncode == EX_AGGREGATE

tests/pytests/integration/ssh/test_mine.py

@@ -8,6 +8,31 @@
 ]
 
 
+@pytest.fixture(scope="module", autouse=True)
+def pillar_tree(base_env_pillar_tree_root_dir):
+    top_file = """
+    base:
+      'localhost':
+        - mine
+      '127.0.0.1':
+        - mine
+    """
+    mine_pillar_file = """
+    mine_functions:
+      disk.usage:
+        - c
+    """
+    top_tempfile = pytest.helpers.temp_file(
+        "top.sls", top_file, base_env_pillar_tree_root_dir
+    )
+    mine_tempfile = pytest.helpers.temp_file(
+        "mine.sls", mine_pillar_file, base_env_pillar_tree_root_dir
+    )
+
+    with top_tempfile, mine_tempfile:
+        yield
+
+
 @pytest.fixture(autouse=True)
 def thin_dir(salt_ssh_cli):
     try:
@@ -29,3 +54,13 @@ def test_ssh_mine_get(salt_ssh_cli):
     assert "localhost" in ret.data
     assert "args" in ret.data["localhost"]
     assert ret.data["localhost"]["args"] == ["itworked"]
+
+
+def test_ssh_mine_get_error(salt_ssh_cli):
+    """
+    Test that a mine function returning an error is not
+    included in the output.
+    """
+    ret = salt_ssh_cli.run("mine.get", "localhost", "disk.usage")
+    assert ret.returncode == 0
+    assert not ret.data