[WIP]Fix: Paramiko ssh config support

CHANGELOG.rst

@@ -79,6 +79,11 @@ In development
   improvement)
 * Throw a more user-friendly exception if rendering a dynamic configuration value inside the config
   fails. (improvement)
+* Add support for ssh config file for ParamikoSSHrunner. Now ``ssh_config_path`` can be set
+  in ``st2.config`` and can be used to access remote hosts when ``use_ssh_config`` is set to
+  ``True``. However, to access remote hosts, action paramters like username and
+  password/private_key, if provided with action, will have precedence over the config file
+  entry for the host. #2941 #3032 [Eric Edgar] (improvement)
 
 2.0.1 - September 30, 2016
 --------------------------

conf/st2.conf.sample

@@ -136,7 +136,7 @@ collection_interval = 600
 
 [keyvalue]
 # Location of the symmetric encryption key for encrypting values in kvstore. This key should be in JSON and should've been generated using keyczar.
-encryption_key_path = 
+encryption_key_path =
 # Allow encryption of values in key value stored qualified as "secret".
 enable_encryption = True
 
@@ -223,6 +223,8 @@ max_parallel_actions = 50
 remote_dir = /tmp
 # Use the .ssh/config file. Useful to override ports etc.
 use_ssh_config = False
+# Path to the ssh config file. Useful to override the default path
+ssh_config_path = ~/.ssh/config
 # How partial success of actions run on multiple nodes should be treated.
 allow_partial_failure = False
 

st2actions/st2actions/config.py

@@ -71,9 +71,12 @@ def _register_action_runner_opts():
         cfg.IntOpt('max_parallel_actions', default=50,
                    help='Max number of parallel remote SSH actions that should be run.  ' +
                         'Works only with Paramiko SSH runner.'),
-        cfg.BoolOpt('use_ssh_config',
-                    default=False,
-                    help='Use the .ssh/config file. Useful to override ports etc.')
+        cfg.BoolOpt('use_ssh_config', default=False,
+                    help='Use the .ssh/config file. Useful to override ports etc.'),
+        cfg.StrOpt('ssh_config_path',
+                   default='~/.ssh/config',
+                   help='Path to the ssh config file.')
+
     ]
     CONF.register_opts(ssh_runner_opts, group='ssh_runner')
 

st2actions/tests/unit/test_paramiko_ssh.py

@@ -35,6 +35,7 @@ def setUp(self):
         Creates the object patching the actual connection.
         """
         cfg.CONF.set_override(name='ssh_key_file', override=None, group='system_user')
+        cfg.CONF.set_override(name='use_ssh_config', override=False, group='ssh_runner')
 
         conn_params = {'hostname': 'dummy.host.org',
                        'port': 8822,
@@ -43,6 +44,46 @@ def setUp(self):
                        'timeout': '600'}
         self.ssh_cli = ParamikoSSHClient(**conn_params)
 
+    @patch('paramiko.SSHClient', Mock)
+    @patch.object(ParamikoSSHClient, '_is_key_file_needs_passphrase',
+                  MagicMock(return_value=False))
+    @patch('paramiko.ProxyCommand')
+    def test_set_proxycommand(self, mock_ProxyCommand):
+        """
+        Loads proxy commands from ssh config file
+        """
+        ssh_config_path = os.path.join(get_resources_base_path(),
+                                       'ssh', 'dummy_ssh_config')
+        cfg.CONF.set_override(name='ssh_config_path', override=ssh_config_path,
+                              group='ssh_runner')
+        cfg.CONF.set_override(name='use_ssh_config', override=True,
+                              group='ssh_runner')
+
+        conn_params = {'hostname': 'dummy.host.org'}
+        mock = ParamikoSSHClient(**conn_params)
+        mock.connect()
+        mock_ProxyCommand.assert_called_once_with('ssh -q -W dummy.host.org:22 dummy_bastion')
+
+    @patch('paramiko.SSHClient', Mock)
+    @patch.object(ParamikoSSHClient, '_is_key_file_needs_passphrase',
+                  MagicMock(return_value=False))
+    @patch('paramiko.ProxyCommand')
+    def test_fail_set_proxycommand(self, mock_ProxyCommand):
+        """
+        Loads proxy commands from ssh config file
+        """
+        ssh_config_path = os.path.join(get_resources_base_path(),
+                                       'ssh', 'dummy_ssh_config_fail')
+        cfg.CONF.set_override(name='ssh_config_path',
+                              override=ssh_config_path, group='ssh_runner')
+        cfg.CONF.set_override(name='use_ssh_config',
+                              override=True, group='ssh_runner')
+
+        conn_params = {'hostname': 'dummy.host.org'}
+        mock = ParamikoSSHClient(**conn_params)
+        self.assertRaises(Exception, mock.connect)
+        mock_ProxyCommand.assert_not_called()
+
     @patch('paramiko.SSHClient', Mock)
     @patch.object(ParamikoSSHClient, '_is_key_file_needs_passphrase',
                   MagicMock(return_value=False))

st2common/st2common/runners/paramiko_ssh.py

@@ -81,7 +81,10 @@ class ParamikoSSHClient(object):
     # Connect socket timeout
     CONNECT_TIMEOUT = 60
 
-    def __init__(self, hostname, port=22, username=None, password=None, bastion_host=None,
+    # Default SSH port
+    SSH_PORT = 22
+
+    def __init__(self, hostname, port=SSH_PORT, username=None, password=None, bastion_host=None,
                  key_files=None, key_material=None, timeout=None, passphrase=None):
         """
         Authentication is always attempted in the following order:
@@ -581,10 +584,96 @@ def _connect(self, host, socket=None):
 
         client = paramiko.SSHClient()
         client.set_missing_host_key_policy(paramiko.AutoAddPolicy())
-        client.connect(**conninfo)
+
+        if cfg.CONF.ssh_runner.use_ssh_config:
+            conninfo_ssh_config = self._get_conninfo_from_ssh_config_for_host(host)
+            client = self._ssh_priority(client, conninfo, conninfo_ssh_config)
+
+        else:
+            extra = {'_conninfo': conninfo}
+            self.logger.debug('Connection info', extra=extra)
+            client.connect(**conninfo)
+
+        return client
+
+    @staticmethod
+    def _ssh_priority(client, conninfo, conninfo_ssh_config):
+        logger = logging.getLogger("ParamikoSSHClient")
+
+        # No Action params.
+        if conninfo['username'] == cfg.CONF.system_user.user\
+                and conninfo['port'] == ParamikoSSHClient.SSH_PORT:
+
+            if 'username' in conninfo_ssh_config:
+
+                # Corner Case: Config file username or key_filename same as
+                # system user.
+                if conninfo_ssh_config['username'] == cfg.CONF.system_user.user\
+                        or conninfo_ssh_config['key_filename'] ==\
+                        cfg.CONF.system_user.ssh_key_file:
+                    conninfo.update(conninfo_ssh_config)
+                    client.connect(**conninfo)
+
+                else:
+                    if 'port' in conninfo_ssh_config:
+                        conninfo_ssh_config['port'] = int(conninfo_ssh_config['port'])
+                    extra = {'_sshconfig_conninfo': conninfo_ssh_config,
+                             '_default_conninfo': conninfo}
+                    logger.debug('Connection info from config',
+                                 extra=extra)
+                    client.connect(**conninfo_ssh_config)
+
+            else:
+                conninfo_ssh_config.update(conninfo)
+                extra = {'_conninfo_ssh_config': conninfo_ssh_config}
+                logger.debug('Connection info, no username, using sys. user',
+                             extra=extra)
+                try:
+                    client.connect(**conninfo_ssh_config)
+                except Exception:
+                    raise Exception('Tried with system user. Provide '
+                                    '\'User\' directive for the host in'
+                                    '.ssh/config.')
+        else:
+            extra = {'_conninfo': conninfo}
+            logger.debug('Connection info, action param. over ssh config',
+                         extra=extra)
+            client.connect(**conninfo)
 
         return client
 
+    @staticmethod
+    def _get_conninfo_from_ssh_config_for_host(host):
+        ssh_conn_info = {}
+        ssh_config = paramiko.SSHConfig()
+        user_config_file = os.path.expanduser(cfg.CONF.ssh_runner.ssh_config_path or
+                                              "~/.ssh/config")
+        try:
+            with open(user_config_file) as f:
+                ssh_config.parse(f)
+        except IOError as e:
+            raise Exception('Error accessing ssh config file %s. Code: %s Reason %s' %
+                            (user_config_file, e.errno, e.strerror))
+
+        user_config = ssh_config.lookup(host)
+        for k in ('hostname', 'port'):
+            if k in user_config:
+                ssh_conn_info[k] = user_config[k]
+
+        if 'user' in user_config:
+            ssh_conn_info['username'] = user_config['user']
+
+        if 'proxycommand' in user_config:
+            ssh_conn_info['sock'] = paramiko.ProxyCommand(user_config['proxycommand'])
+
+        if 'identityfile' in user_config:
+            ssh_conn_info['key_filename'] = user_config['identityfile']
+
+        extra = {'_get_conninfo': user_config, '_ssh_conninfo': ssh_conn_info}
+        logger = logging.getLogger("ParamikoSSHClient")
+        logger.debug('_get_conninfo', extra=extra)
+        return ssh_conn_info
+
     @staticmethod
     def _is_key_file_needs_passphrase(file):
         for cls in [paramiko.RSAKey, paramiko.DSSKey, paramiko.ECDSAKey]:

st2tests/st2tests/resources/ssh/dummy_ssh_config

@@ -0,0 +1,3 @@
+Host dummy.host.org
+   IdentityFile ~/.ssh/id_rsa_dummy
+   ProxyCommand ssh -q -W %h:%p dummy_bastion