Support passing client certificates for server version requests

Using the cinderclient to fetch server versions will fail with error
`OpenSSL.SSL.Error: [sslv3 alert handshake failure]` when the server
requires client certificates to be passed with these requests.

Added the optional parameter `cert` to both get_server_version
get_highest_client_server_version and methods so that users can have
the option to pass client certificates while fetching server versions.

Also support passing mTLS certificate/key to HTTPClient

Closes-Bug: #1915996
Change-Id: I57c665dd9d4b8c32e5f10994d891d1e0f5315548
Signed-off-by: sri harsha mekala <smekala@oath.com>

6 files changed, 56 insertions, 15 deletions

diff --git a/cinderclient/client.py b/cinderclient/client.py
index 2d7b80d..c473343 100644
--- a/cinderclient/client.py
+++ b/cinderclient/client.py
@@ -70,7 +70,7 @@ for svc in ('volume', 'volumev2', 'volumev3'):
     discover.add_catalog_discover_hack(svc, re.compile(r'/v[12]/\w+/?$'), '/')
 
 
-def get_server_version(url, insecure=False, cacert=None):
+def get_server_version(url, insecure=False, cacert=None, cert=None):
     """Queries the server via the naked endpoint and gets version info.
 
     :param url: url of the cinder endpoint
@@ -78,6 +78,10 @@ def get_server_version(url, insecure=False, cacert=None):
                      (https) requests
     :param cacert: Specify a CA bundle file to use in verifying a TLS
                             (https) server certificate
+    :param cert: A client certificate to pass to requests. These are of the
+                 same form as requests expects. Either a single filename
+                 containing both the certificate and key or a tuple containing
+                 the path to the certificate then a path to the key. (optional)
     :returns: APIVersion object for min and max version supported by
               the server
     """
@@ -115,7 +119,7 @@ def get_server_version(url, insecure=False, cacert=None):
                 verify_cert = cacert
             else:
                 verify_cert = True
-        response = requests.get(version_url, verify=verify_cert)
+        response = requests.get(version_url, verify=verify_cert, cert=cert)
         data = json.loads(response.text)
         versions = data['versions']
         for version in versions:
@@ -135,9 +139,10 @@ def get_server_version(url, insecure=False, cacert=None):
             api_versions.APIVersion(current_version))
 
 
-def get_highest_client_server_version(url, insecure=False, cacert=None):
+def get_highest_client_server_version(url, insecure=False,
+                                      cacert=None, cert=None):
     """Returns highest supported version by client and server as a string."""
-    min_server, max_server = get_server_version(url, insecure, cacert)
+    min_server, max_server = get_server_version(url, insecure, cacert, cert)
     max_client = api_versions.APIVersion(api_versions.MAX_VERSION)
     return min(max_server, max_client).get_string()
 
@@ -286,7 +291,7 @@ class HTTPClient(object):
                  endpoint_type='publicURL', service_type=None,
                  service_name=None, volume_service_name=None,
                  os_endpoint=None, retries=None,
-                 http_log_debug=False, cacert=None,
+                 http_log_debug=False, cacert=None, cert=None,
                  auth_system='keystone', auth_plugin=None, api_version=None,
                  logger=None, user_domain_name='Default',
                  project_domain_name='Default', global_request_id=None):
@@ -324,7 +329,7 @@ class HTTPClient(object):
         self.timeout = timeout
         self.user_domain_name = user_domain_name
         self.project_domain_name = project_domain_name
-
+        self.cert = cert
         if insecure:
             self.verify_cert = False
         else:
@@ -405,6 +410,7 @@ class HTTPClient(object):
             method,
             url,
             verify=self.verify_cert,
+            cert=self.cert,
             **kwargs)
         self.http_log_resp(resp)
 
@@ -701,7 +707,7 @@ def _construct_http_client(username=None, password=None, project_id=None,
                            os_endpoint=None, retries=None,
                            http_log_debug=False,
                            auth_system='keystone', auth_plugin=None,
-                           cacert=None, tenant_id=None,
+                           cacert=None, cert=None, tenant_id=None,
                            session=None,
                            auth=None, api_version=None,
                            **kwargs):
@@ -741,6 +747,7 @@ def _construct_http_client(username=None, password=None, project_id=None,
                           retries=retries,
                           http_log_debug=http_log_debug,
                           cacert=cacert,
+                          cert=cert,
                           auth_system=auth_system,
                           auth_plugin=auth_plugin,
                           logger=logger,
diff --git a/cinderclient/tests/unit/test_client.py b/cinderclient/tests/unit/test_client.py
index d92e1f2..fa19492 100644
--- a/cinderclient/tests/unit/test_client.py
+++ b/cinderclient/tests/unit/test_client.py
@@ -365,7 +365,9 @@ class GetAPIVersionTestCase(utils.TestCase):
 
         cinderclient.client.get_server_version(url, True)
 
-        mock_request.assert_called_once_with(expected_url, verify=False)
+        mock_request.assert_called_once_with(expected_url,
+                                             verify=False,
+                                             cert=None)
 
     @mock.patch('cinderclient.client.requests.get')
     def test_get_server_version_cacert(self, mock_request):
@@ -383,7 +385,29 @@ class GetAPIVersionTestCase(utils.TestCase):
         cacert = '/path/to/cert'
         cinderclient.client.get_server_version(url, cacert=cacert)
 
-        mock_request.assert_called_once_with(expected_url, verify=cacert)
+        mock_request.assert_called_once_with(expected_url,
+                                             verify=cacert,
+                                             cert=None)
+
+    @mock.patch('cinderclient.client.requests.get')
+    def test_get_server_version_cert(self, mock_request):
+        mock_response = utils.TestResponse({
+            "status_code": 200,
+            "text": json.dumps(fakes.fake_request_get_no_v3())
+        })
+
+        mock_request.return_value = mock_response
+
+        url = (
+            "https://192.168.122.127:8776/v3/e5526285ebd741b1819393f772f11fc3")
+        expected_url = "https://192.168.122.127:8776/"
+
+        client_cert = '/path/to/cert'
+        cinderclient.client.get_server_version(url, cert=client_cert)
+
+        mock_request.assert_called_once_with(expected_url,
+                                             verify=True,
+                                             cert=client_cert)
 
     @mock.patch('cinderclient.client.requests.get')
     @ddt.data('3.12', '3.40')
diff --git a/cinderclient/tests/unit/utils.py b/cinderclient/tests/unit/utils.py
index 680062e..2bf242f 100644
--- a/cinderclient/tests/unit/utils.py
+++ b/cinderclient/tests/unit/utils.py
@@ -27,6 +27,7 @@ REQUEST_ID = ['req-test-request-id']
 class TestCase(testtools.TestCase):
     TEST_REQUEST_BASE = {
         'verify': True,
+        'cert': None
     }
 
     def setUp(self):
diff --git a/cinderclient/v2/client.py b/cinderclient/v2/client.py
index 0086a4d..a111c51 100644
--- a/cinderclient/v2/client.py
+++ b/cinderclient/v2/client.py
@@ -56,9 +56,9 @@ class Client(object):
                  endpoint_type='publicURL', extensions=None,
                  service_type='volumev2', service_name=None,
                  volume_service_name=None, os_endpoint=None, retries=0,
-                 http_log_debug=False, cacert=None, auth_system='keystone',
-                 auth_plugin=None, session=None, api_version=None,
-                 logger=None, **kwargs):
+                 http_log_debug=False, cacert=None, cert=None,
+                 auth_system='keystone', auth_plugin=None, session=None,
+                 api_version=None, logger=None, **kwargs):
         # FIXME(comstud): Rename the api_key argument above when we
         # know it's not being used as keyword argument
         password = api_key
@@ -118,6 +118,7 @@ class Client(object):
             retries=retries,
             http_log_debug=http_log_debug,
             cacert=cacert,
+            cert=cert,
             auth_system=auth_system,
             auth_plugin=auth_plugin,
             session=session,
diff --git a/cinderclient/v3/client.py b/cinderclient/v3/client.py
index 770d9d6..8ecaf00 100644
--- a/cinderclient/v3/client.py
+++ b/cinderclient/v3/client.py
@@ -63,9 +63,9 @@ class Client(object):
                  endpoint_type='publicURL', extensions=None,
                  service_type='volumev3', service_name=None,
                  volume_service_name=None, os_endpoint=None, retries=0,
-                 http_log_debug=False, cacert=None, auth_system='keystone',
-                 auth_plugin=None, session=None, api_version=None,
-                 logger=None, **kwargs):
+                 http_log_debug=False, cacert=None, cert=None,
+                 auth_system='keystone', auth_plugin=None, session=None,
+                 api_version=None, logger=None, **kwargs):
         # FIXME(comstud): Rename the api_key argument above when we
         # know it's not being used as keyword argument
         password = api_key
@@ -131,6 +131,7 @@ class Client(object):
             retries=retries,
             http_log_debug=http_log_debug,
             cacert=cacert,
+            cert=cert,
             auth_system=auth_system,
             auth_plugin=auth_plugin,
             session=session,
diff --git a/releasenotes/notes/bug-1915996-3aaa5e2548eb7c93.yaml b/releasenotes/notes/bug-1915996-3aaa5e2548eb7c93.yaml
new file mode 100644
index 0000000..89f51d8
--- /dev/null
+++ b/releasenotes/notes/bug-1915996-3aaa5e2548eb7c93.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    `Bug #1915996 <https://bugs.launchpad.net/python-cinderclient/+bug/1915996>`_:
+    Passing client certificates for mTLS connections was not supported
+    and now has been fixed.
+