Repository: libcloud Updated Branches: refs/heads/trunk b20c62a97 -> 6f9b00401
Deprecate "key" argument in the SSHClient class in favor of new "key_files" argument. Also add a new "key_material" argument. This argument can contain raw string version of a private key. Note 1: "key_files" and "key_material" arguments are mutually exclusive. Note 2: "key_material" argument is not supported in the ShellOutSSHClient. Project: http://git-wip-us.apache.org/repos/asf/libcloud/repo Commit: http://git-wip-us.apache.org/repos/asf/libcloud/commit/6f9b0040 Tree: http://git-wip-us.apache.org/repos/asf/libcloud/tree/6f9b0040 Diff: http://git-wip-us.apache.org/repos/asf/libcloud/diff/6f9b0040 Branch: refs/heads/trunk Commit: 6f9b004019675a0c9c612cdaf2e45c3be42a4c74 Parents: b20c62a Author: Tomaz Muraus <[email protected]> Authored: Tue May 6 20:22:53 2014 +0200 Committer: Tomaz Muraus <[email protected]> Committed: Tue May 6 20:47:10 2014 +0200 ---------------------------------------------------------------------- CHANGES.rst | 9 +++ libcloud/compute/base.py | 2 +- libcloud/compute/ssh.py | 87 +++++++++++++++++++++------ libcloud/test/compute/test_ssh_client.py | 79 ++++++++++++++++++++++-- 4 files changed, 151 insertions(+), 26 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/libcloud/blob/6f9b0040/CHANGES.rst ---------------------------------------------------------------------- diff --git a/CHANGES.rst b/CHANGES.rst index 3d295d9..e7af0ad 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -109,6 +109,15 @@ Compute (GITHUB-289) [Jeff Moody] +- Deprecate "key" argument in the SSHClient class in favor of new "key_files" + argument. + + Also add a new "key_material" argument. This argument can contain raw string + version of a private key. + + Note 1: "key_files" and "key_material" arguments are mutually exclusive. + Note 2: "key_material" argument is not supported in the ShellOutSSHClient. + Load Balancer ~~~~~~~~~~~~~ http://git-wip-us.apache.org/repos/asf/libcloud/blob/6f9b0040/libcloud/compute/base.py ---------------------------------------------------------------------- diff --git a/libcloud/compute/base.py b/libcloud/compute/base.py index 6c34186..06a9249 100644 --- a/libcloud/compute/base.py +++ b/libcloud/compute/base.py @@ -1394,7 +1394,7 @@ class NodeDriver(BaseDriver): ssh_client = SSHClient(hostname=ssh_hostname, port=ssh_port, username=ssh_username, password=ssh_password, - key=ssh_key_file, + key_files=ssh_key_file, timeout=ssh_timeout) # Connect to the SSH server running on the node http://git-wip-us.apache.org/repos/asf/libcloud/blob/6f9b0040/libcloud/compute/ssh.py ---------------------------------------------------------------------- diff --git a/libcloud/compute/ssh.py b/libcloud/compute/ssh.py index 7fb1af2..68a4da4 100644 --- a/libcloud/compute/ssh.py +++ b/libcloud/compute/ssh.py @@ -14,8 +14,9 @@ # limitations under the License. """ -Wraps multiple ways to communicate over SSH +Wraps multiple ways to communicate over SSH. """ + have_paramiko = False try: @@ -32,6 +33,7 @@ import os import time import subprocess import logging +import warnings from os.path import split as psplit from os.path import join as pjoin @@ -50,7 +52,7 @@ class BaseSSHClient(object): """ def __init__(self, hostname, port=22, username='root', password=None, - key=None, timeout=None): + key=None, key_files=None, timeout=None): """ :type hostname: ``str`` :keyword hostname: Hostname or IP address to connect to. @@ -66,14 +68,24 @@ class BaseSSHClient(object): to unlock a private key if a password protected key is used. - :type key: ``str`` or ``list`` - :keyword key: A list of paths to the private key files to use. + :param key: Deprecated in favor of ``key_files`` argument. + + :type key_files: ``str`` or ``list`` + :keyword key_files: A list of paths to the private key files to use. """ + if key is not None: + message = ('You are using deprecated "key" argument which has ' + 'been replaced with "key_files" argument') + warnings.warn(message, DeprecationWarning) + + # key_files has precedent + key_files = key if not key_files else key_files + self.hostname = hostname self.port = port self.username = username self.password = password - self.key = key + self.key_files = key_files self.timeout = timeout def connect(self): @@ -116,8 +128,8 @@ class BaseSSHClient(object): :type path: ``str`` :keyword path: File path on the remote node. - :return: True if the file has been successfully deleted, False - otherwise. + :return: True if the file has been successfully deleted, + False otherwise. :rtype: ``bool`` """ raise NotImplementedError( @@ -139,8 +151,8 @@ class BaseSSHClient(object): """ Shutdown connection to the remote node. - :return: True if the connection has been successfully closed, False - otherwise. + :return: True if the connection has been successfully closed, + False otherwise. :rtype: ``bool`` """ raise NotImplementedError( @@ -165,7 +177,7 @@ class ParamikoSSHClient(BaseSSHClient): A SSH Client powered by Paramiko. """ def __init__(self, hostname, port=22, username='root', password=None, - key=None, timeout=None): + key=None, key_files=None, key_material=None, timeout=None): """ Authentication is always attempted in the following order: @@ -177,8 +189,19 @@ class ParamikoSSHClient(BaseSSHClient): - Plain username/password auth, if a password was given (if password is provided) """ - super(ParamikoSSHClient, self).__init__(hostname, port, username, - password, key, timeout) + if key_files and key_material: + raise ValueError(('key_files and key_material arguments are ' + 'mutually exclusive')) + + super(ParamikoSSHClient, self).__init__(hostname=hostname, port=port, + username=username, + password=password, + key=key, + key_files=key_files, + timeout=timeout) + + self.key_material = key_material + self.client = paramiko.SSHClient() self.client.set_missing_host_key_policy(paramiko.AutoAddPolicy()) self.logger = self._get_and_setup_logger() @@ -193,10 +216,13 @@ class ParamikoSSHClient(BaseSSHClient): if self.password: conninfo['password'] = self.password - if self.key: - conninfo['key_filename'] = self.key + if self.key_files: + conninfo['key_filename'] = self.key_files + + if self.key_material: + conninfo['pkey'] = self._get_pkey_object(key=self.key_material) - if not self.password and not self.key: + if not self.password and not (self.key_files or self.key_material): conninfo['allow_agent'] = True conninfo['look_for_keys'] = True @@ -345,6 +371,23 @@ class ParamikoSSHClient(BaseSSHClient): self.client.close() return True + def _get_pkey_object(self, key): + """ + Try to detect private key type and return paramiko.PKey object. + """ + + for cls in [paramiko.RSAKey, paramiko.DSSKey, paramiko.ECDSAKey]: + try: + key = cls.from_private_key(StringIO(key)) + except paramiko.ssh_exception.SSHException: + # Invalid key, try other key type + pass + else: + return key + + msg = 'Invalid or unsupported key type' + raise paramiko.ssh_exception.SSHException(msg) + class ShellOutSSHClient(BaseSSHClient): """ @@ -355,9 +398,13 @@ class ShellOutSSHClient(BaseSSHClient): """ def __init__(self, hostname, port=22, username='root', password=None, - key=None, timeout=None): - super(ShellOutSSHClient, self).__init__(hostname, port, username, - password, key, timeout) + key=None, key_files=None, timeout=None): + super(ShellOutSSHClient, self).__init__(hostname=hostname, + port=port, username=username, + password=password, + key=key, + key_files=key_files, + timeout=timeout) if self.password: raise ValueError('ShellOutSSHClient only supports key auth') @@ -403,8 +450,8 @@ class ShellOutSSHClient(BaseSSHClient): def _get_base_ssh_command(self): cmd = ['ssh'] - if self.key: - cmd += ['-i', self.key] + if self.key_files: + cmd += ['-i', self.key_files] if self.timeout: cmd += ['-oConnectTimeout=%s' % (self.timeout)] http://git-wip-us.apache.org/repos/asf/libcloud/blob/6f9b0040/libcloud/test/compute/test_ssh_client.py ---------------------------------------------------------------------- diff --git a/libcloud/test/compute/test_ssh_client.py b/libcloud/test/compute/test_ssh_client.py index d996119..857f4e2 100644 --- a/libcloud/test/compute/test_ssh_client.py +++ b/libcloud/test/compute/test_ssh_client.py @@ -20,20 +20,25 @@ from __future__ import with_statement import os import sys import tempfile -import unittest from libcloud import _init_once +from libcloud.test import LibcloudTestCase +from libcloud.test import unittest from libcloud.compute.ssh import ParamikoSSHClient from libcloud.compute.ssh import ShellOutSSHClient from libcloud.compute.ssh import have_paramiko +from libcloud.utils.py3 import StringIO + from mock import patch, Mock if not have_paramiko: ParamikoSSHClient = None # NOQA +else: + import paramiko -class ParamikoSSHClientTests(unittest.TestCase): +class ParamikoSSHClientTests(LibcloudTestCase): @patch('paramiko.SSHClient', Mock) def setUp(self): @@ -68,7 +73,7 @@ class ParamikoSSHClientTests(unittest.TestCase): self.assertLogMsg('Connecting to server') @patch('paramiko.SSHClient', Mock) - def test_create_with_key(self): + def test_deprecated_key_argument(self): conn_params = {'hostname': 'dummy.host.org', 'username': 'ubuntu', 'key': 'id_rsa'} @@ -84,6 +89,70 @@ class ParamikoSSHClientTests(unittest.TestCase): mock.client.connect.assert_called_once_with(**expected_conn) self.assertLogMsg('Connecting to server') + def test_key_files_and_key_material_arguments_are_mutual_exclusive(self): + conn_params = {'hostname': 'dummy.host.org', + 'username': 'ubuntu', + 'key_files': 'id_rsa', + 'key_material': 'key'} + + expected_msg = ('key_files and key_material arguments are mutually ' + 'exclusive') + self.assertRaisesRegexp(ValueError, expected_msg, + ParamikoSSHClient, **conn_params) + + @patch('paramiko.SSHClient', Mock) + def test_key_material_argument(self): + path = os.path.join(os.path.dirname(__file__), + 'fixtures', 'misc', 'dummy_rsa') + + with open(path, 'r') as fp: + private_key = fp.read() + + conn_params = {'hostname': 'dummy.host.org', + 'username': 'ubuntu', + 'key_material': private_key} + mock = ParamikoSSHClient(**conn_params) + mock.connect() + + pkey = paramiko.RSAKey.from_private_key(StringIO(private_key)) + expected_conn = {'username': 'ubuntu', + 'allow_agent': False, + 'hostname': 'dummy.host.org', + 'look_for_keys': False, + 'pkey': pkey, + 'port': 22} + mock.client.connect.assert_called_once_with(**expected_conn) + self.assertLogMsg('Connecting to server') + + @patch('paramiko.SSHClient', Mock) + def test_key_material_argument_invalid_key(self): + conn_params = {'hostname': 'dummy.host.org', + 'username': 'ubuntu', + 'key_material': 'id_rsa'} + + mock = ParamikoSSHClient(**conn_params) + + expected_msg = 'Invalid or unsupported key type' + self.assertRaisesRegexp(paramiko.ssh_exception.SSHException, + expected_msg, mock.connect) + + @patch('paramiko.SSHClient', Mock) + def test_create_with_key(self): + conn_params = {'hostname': 'dummy.host.org', + 'username': 'ubuntu', + 'key_files': 'id_rsa'} + mock = ParamikoSSHClient(**conn_params) + mock.connect() + + expected_conn = {'username': 'ubuntu', + 'allow_agent': False, + 'hostname': 'dummy.host.org', + 'look_for_keys': False, + 'key_filename': 'id_rsa', + 'port': 22} + mock.client.connect.assert_called_once_with(**expected_conn) + self.assertLogMsg('Connecting to server') + @patch('paramiko.SSHClient', Mock) def test_create_with_password_and_key(self): conn_params = {'hostname': 'dummy.host.org', @@ -185,11 +254,11 @@ class ParamikoSSHClientTests(unittest.TestCase): if not ParamikoSSHClient: - class ParamikoSSHClientTests(unittest.TestCase): # NOQA + class ParamikoSSHClientTests(LibcloudTestCase): # NOQA pass -class ShellOutSSHClientTests(unittest.TestCase): +class ShellOutSSHClientTests(LibcloudTestCase): def test_password_auth_not_supported(self): try:
