Merge pull request saltstack#190 from Ch3LL/3002.6_ssh_kwargs

[3002.6] Backport saltstack#59664 saltstack#59748

changelog/59664.fixed

@@ -0,0 +1,2 @@
+Allow "extra_filerefs" as sanitized kwargs for SSH client.
+Fix regression on "cmd.run" when passing tuples as cmd.

changelog/59748.fixed

@@ -0,0 +1 @@
+Allow all ssh kwargs as sanitized kwargs for SSH client.

salt/client/mixins.py

@@ -28,7 +28,6 @@
 import salt.utils.state
 import salt.utils.user
 import salt.utils.versions
-from salt.ext import six
 
 log = logging.getLogger(__name__)
 

salt/client/ssh/client.py

@@ -1,14 +1,8 @@
-# -*- coding: utf-8 -*-
-
-# Import Python libs
-from __future__ import absolute_import, print_function, unicode_literals
-
 import copy
 import logging
 import os
 import random
 
-# Import Salt libs
 import salt.config
 import salt.syspaths as syspaths
 import salt.utils.args
@@ -17,7 +11,7 @@
 log = logging.getLogger(__name__)
 
 
-class SSHClient(object):
+class SSHClient:
     """
     Create a client object for executing routines via the salt-ssh backend
 
@@ -58,10 +52,34 @@ def sanitize_kwargs(self, kwargs):
             ("ssh_identities_only", bool),
             ("ssh_remote_port_forwards", str),
             ("ssh_options", list),
+            ("ssh_max_procs", int),
+            ("ssh_askpass", bool),
+            ("ssh_key_deploy", bool),
+            ("ssh_update_roster", bool),
+            ("ssh_scan_ports", str),
+            ("ssh_scan_timeout", int),
+            ("ssh_timeout", int),
+            ("ssh_log_file", str),
+            ("raw_shell", bool),
+            ("refresh_cache", bool),
+            ("roster", str),
             ("roster_file", str),
             ("rosters", list),
             ("ignore_host_keys", bool),
             ("raw_shell", bool),
+            ("extra_filerefs", str),
+            ("min_extra_mods", str),
+            ("thin_extra_mods", str),
+            ("verbose", bool),
+            ("static", bool),
+            ("ssh_wipe", bool),
+            ("rand_thin_dir", bool),
+            ("regen_thin", bool),
+            ("python2_bin", str),
+            ("python3_bin", str),
+            ("ssh_run_pre_flight", bool),
+            ("no_host_keys", bool),
+            ("saltfile", str),
         ]
         sane_kwargs = {}
         for name, kind in roster_vals:
@@ -126,8 +144,7 @@ def cmd_iter(
         .. versionadded:: 2015.5.0
         """
         ssh = self._prep_ssh(tgt, fun, arg, timeout, tgt_type, kwarg, **kwargs)
-        for ret in ssh.run_iter(jid=kwargs.get("jid", None)):
-            yield ret
+        yield from ssh.run_iter(jid=kwargs.get("jid", None))
 
     def cmd(
         self, tgt, fun, arg=(), timeout=None, tgt_type="glob", kwarg=None, **kwargs

salt/modules/cmdmod.py

@@ -78,7 +78,7 @@ def __virtual__():
 
 
 def _log_cmd(cmd):
-    if not isinstance(cmd, list):
+    if isinstance(cmd, str):
         return cmd.split()[0].strip()
     return cmd[0].strip()
 
@@ -838,7 +838,9 @@ def _run(
         if not ignore_retcode and ret["retcode"] != 0:
             if output_loglevel < LOG_LEVELS["error"]:
                 output_loglevel = LOG_LEVELS["error"]
-            msg = "Command '{}' failed with return code: {}".format(_log_cmd(cmd), ret["retcode"])
+            msg = "Command '{}' failed with return code: {}".format(
+                _log_cmd(cmd), ret["retcode"]
+            )
             log.error(log_callback(msg))
         if ret["stdout"]:
             log.log(output_loglevel, "stdout: %s", log_callback(ret["stdout"]))
@@ -1208,7 +1210,9 @@ def run(
         if not ignore_retcode and ret["retcode"] != 0:
             if lvl < LOG_LEVELS["error"]:
                 lvl = LOG_LEVELS["error"]
-            msg = "Command '{}' failed with return code: {}".format(_log_cmd(cmd), ret["retcode"])
+            msg = "Command '{}' failed with return code: {}".format(
+                _log_cmd(cmd), ret["retcode"]
+            )
             log.error(log_callback(msg))
             if raise_err:
                 raise CommandExecutionError(

tests/integration/wheel/test_pillar_roots.py

@@ -14,11 +14,11 @@ def setUp(self):
     def tearDown(self):
         try:
             os.remove(os.path.join(self.pillar_dir, "foo"))
-        except Exception:
+        except Exception:  # pylint: disable=broad-except
             pass
         try:
             os.remove(os.path.join(self.traversed_dir, "foo"))
-        except Exception:
+        except Exception:  # pylint: disable=broad-except
             pass
         del self.wheel
 

tests/pytests/unit/client/test_ssh.py

@@ -1,8 +1,11 @@
 import pytest
+import salt.client.ssh.client
 import salt.utils.msgpack
 from salt.client import ssh
 from tests.support.mock import MagicMock, patch
 
+pytestmark = [pytest.mark.skip_if_binaries_missing("ssh", "ssh-keygen", check_all=True)]
+
 
 @pytest.fixture
 def ssh_target(tmpdir):
@@ -57,3 +60,76 @@ def test_cmd_block_python_version_error(ssh_target):
     with patch_shim:
         ret = single.cmd_block()
         assert "ERROR: Python version error. Recommendation(s) follow:" in ret[0]
+
+
+@pytest.mark.parametrize(
+    "test_opts",
+    [
+        ("extra_filerefs", "salt://foobar", True),
+        ("host", "testhost", False),
+        ("ssh_user", "testuser", True),
+        ("ssh_passwd", "testpasswd", True),
+        ("ssh_port", 23, False),
+        ("ssh_sudo", True, True),
+        ("ssh_sudo_user", "sudouser", False),
+        ("ssh_priv", "test_priv", True),
+        ("ssh_priv_passwd", "sshpasswd", True),
+        ("ssh_identities_only", True, True),
+        ("ssh_remote_port_forwards", "test", True),
+        ("ssh_options", ["test1", "test2"], True),
+        ("ssh_max_procs", 2, True),
+        ("ssh_askpass", True, True),
+        ("ssh_key_deploy", True, True),
+        ("ssh_update_roster", True, True),
+        ("ssh_scan_ports", "test", True),
+        ("ssh_scan_timeout", 1.0, True),
+        ("ssh_timeout", 1, False),
+        ("ssh_log_file", "/tmp/test", True),
+        ("raw_shell", True, True),
+        ("refresh_cache", True, True),
+        ("roster", "/test", True),
+        ("roster_file", "/test1", True),
+        ("rosters", ["test1"], False),
+        ("ignore_host_keys", True, True),
+        ("min_extra_mods", "test", True),
+        ("thin_extra_mods", "test1", True),
+        ("verbose", True, True),
+        ("static", True, True),
+        ("ssh_wipe", True, True),
+        ("rand_thin_dir", True, True),
+        ("regen_thin", True, True),
+        ("python2_bin", "python2", True),
+        ("python3_bin", "python3", True),
+        ("ssh_run_pre_flight", True, True),
+        ("no_host_keys", True, True),
+        ("saltfile", "/tmp/test", True),
+        ("doesnotexist", None, False),
+    ],
+)
+def test_ssh_kwargs(test_opts):
+    """
+    test all ssh kwargs are not excluded from kwargs
+    when preparing the SSH opts
+    """
+    opt_key = test_opts[0]
+    opt_value = test_opts[1]
+    # Is the kwarg in salt.utils.parsers?
+    in_parser = test_opts[2]
+
+    opts = {
+        "eauth": "auto",
+        "username": "test",
+        "password": "test",
+        "client": "ssh",
+        "tgt": "localhost",
+        "fun": "test.ping",
+        opt_key: opt_value,
+    }
+    client = salt.client.ssh.client.SSHClient(disable_custom_roster=True)
+    if in_parser:
+        ssh_kwargs = salt.utils.parsers.SaltSSHOptionParser().defaults
+        assert opt_key in ssh_kwargs
+
+    with patch("salt.roster.get_roster_file", MagicMock(return_value="")):
+        ssh_obj = client._prep_ssh(**opts)
+        assert ssh_obj.opts.get(opt_key, None) == opt_value