You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@libcloud.apache.org by to...@apache.org on 2014/05/06 20:47:32 UTC

git commit: Deprecate "key" argument in the SSHClient class in favor of new "key_files" argument.

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 <to...@apache.org>
Authored: Tue May 6 20:22:53 2014 +0200
Committer: Tomaz Muraus <to...@apache.org>
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: