From 4d3651dd898cba1c79cf31476ed7c9b181369b2c Mon Sep 17 00:00:00 2001 From: Srinivas Sakhamuri Date: Tue, 13 Jan 2015 18:38:26 +0000 Subject: [PATCH] Fix improper parameter setup for cacert and client certs The client passes cacert incorrectly, CA cert need to be passed in verify parameter to requests library. Also at present, key file is not being used, which if it is supplied need to be passed as a tuple in cert parameter Python requests library relevant doc link http://docs.python-requests.org/en/latest/user/advanced/#ssl-cert-verification Change-Id: Ie71e760bce6a78ed1444373adfa0a133e82ec434 Closes-Bug: 1408372 --- ceilometerclient/client.py | 26 +++++++++++++++++++++++++- ceilometerclient/shell.py | 2 +- ceilometerclient/tests/test_client.py | 18 ++++++++++++++++-- ceilometerclient/v1/client.py | 2 +- ceilometerclient/v2/client.py | 18 +++--------------- 5 files changed, 46 insertions(+), 20 deletions(-) diff --git a/ceilometerclient/client.py b/ceilometerclient/client.py index feacf7c6..5fe8a288 100644 --- a/ceilometerclient/client.py +++ b/ceilometerclient/client.py @@ -222,6 +222,28 @@ def Client(version, *args, **kwargs): return client_class(*args, **kwargs) +def _adjust_params(kwargs): + timeout = kwargs.get('timeout') + if timeout is not None: + timeout = int(timeout) + if timeout <= 0: + timeout = None + + insecure = strutils.bool_from_string(kwargs.get('insecure')) + verify = kwargs.get('verify') + if verify is None: + if insecure: + verify = False + else: + verify = kwargs.get('cacert') or True + + cert = kwargs.get('cert_file') + key = kwargs.get('key_file') + if cert and key: + cert = cert, key + return {'verify': verify, 'cert': cert, 'timeout': timeout} + + def get_client(version, **kwargs): """Get an authenticated client, based on the credentials in the keyword args. @@ -274,6 +296,8 @@ def get_client(version, **kwargs): } cli_kwargs.update(kwargs) + cli_kwargs.update(_adjust_params(cli_kwargs)) + return Client(version, endpoint, **cli_kwargs) @@ -283,7 +307,7 @@ def get_auth_plugin(endpoint, **kwargs): service_type=kwargs.get('service_type'), token=kwargs.get('token'), endpoint_type=kwargs.get('endpoint_type'), - cacert=kwargs.get('ca_file'), + cacert=kwargs.get('cacert'), tenant_id=kwargs.get('project_id') or kwargs.get('tenant_id'), endpoint=endpoint, username=kwargs.get('username'), diff --git a/ceilometerclient/shell.py b/ceilometerclient/shell.py index 221eca9c..80579335 100644 --- a/ceilometerclient/shell.py +++ b/ceilometerclient/shell.py @@ -240,7 +240,7 @@ class CeilometerShell(object): client_kwargs = vars(args) client_kwargs.update(self.auth_plugin.opts) client_kwargs['auth_plugin'] = self.auth_plugin - client = ceiloclient.Client(api_version, **client_kwargs) + client = ceiloclient.get_client(api_version, **client_kwargs) # call whatever callback was selected try: args.func(client, args) diff --git a/ceilometerclient/tests/test_client.py b/ceilometerclient/tests/test_client.py index a998d931..bfcf43a0 100644 --- a/ceilometerclient/tests/test_client.py +++ b/ceilometerclient/tests/test_client.py @@ -40,7 +40,7 @@ class ClientTest(utils.BaseTestCase): env = dict((k, v) for k, v in env.items() if k not in exclude) - return client.Client(api_version, endpoint, **env) + return client.get_client(api_version, **env) def setUp(self): super(ClientTest, self).setUp() @@ -79,7 +79,7 @@ class ClientTest(utils.BaseTestCase): del env['auth_plugin'] expected = { 'username': 'username', - 'endpoint': None, + 'endpoint': 'http://no.where', 'tenant_name': 'tenant_name', 'service_type': None, 'token': '1234', @@ -135,3 +135,17 @@ class ClientTest(utils.BaseTestCase): def test_v2_client_timeout_valid_value(self): self._test_v2_client_timeout_integer(30, 30) + + def test_v2_client_cacert_in_verify(self): + env = FAKE_ENV.copy() + env['cacert'] = '/path/to/cacert' + client = self.create_client(env) + self.assertEqual('/path/to/cacert', client.client.verify) + + def test_v2_client_certfile_and_keyfile(self): + env = FAKE_ENV.copy() + env['cert_file'] = '/path/to/cert' + env['key_file'] = '/path/to/keycert' + client = self.create_client(env) + self.assertEqual(('/path/to/cert', '/path/to/keycert'), + client.client.cert) diff --git a/ceilometerclient/v1/client.py b/ceilometerclient/v1/client.py index e8827e24..4f531bf9 100644 --- a/ceilometerclient/v1/client.py +++ b/ceilometerclient/v1/client.py @@ -39,7 +39,7 @@ class Client(object): endpoint_type=kwargs.get('endpoint_type'), original_ip=kwargs.get('original_ip'), verify=kwargs.get('verify'), - cert=kwargs.get('cacert'), + cert=kwargs.get('cert'), timeout=kwargs.get('timeout'), timings=kwargs.get('timings'), keyring_saver=kwargs.get('keyring_saver'), diff --git a/ceilometerclient/v2/client.py b/ceilometerclient/v2/client.py index d0cf7fff..19f847de 100644 --- a/ceilometerclient/v2/client.py +++ b/ceilometerclient/v2/client.py @@ -15,8 +15,6 @@ # License for the specific language governing permissions and limitations # under the License. -from oslo.utils import strutils - from ceilometerclient import client as ceiloclient from ceilometerclient.openstack.common.apiclient import client from ceilometerclient.v2 import alarms @@ -48,24 +46,14 @@ class Client(object): """Initialize a new client for the Ceilometer v2 API.""" self.auth_plugin = kwargs.get('auth_plugin') \ or ceiloclient.get_auth_plugin(*args, **kwargs) - - timeout = kwargs.get('timeout') - if timeout is not None: - timeout = int(timeout) - if timeout <= 0: - timeout = None - - insecure = strutils.bool_from_string(kwargs.get('insecure')) - verify = kwargs.get('verify', not insecure) - self.client = client.HTTPClient( auth_plugin=self.auth_plugin, region_name=kwargs.get('region_name'), endpoint_type=kwargs.get('endpoint_type'), original_ip=kwargs.get('original_ip'), - verify=verify, - cert=kwargs.get('cacert'), - timeout=timeout, + verify=kwargs.get('verify'), + cert=kwargs.get('cert'), + timeout=kwargs.get('timeout'), timings=kwargs.get('timings'), keyring_saver=kwargs.get('keyring_saver'), debug=kwargs.get('debug'),