diff --git a/nova/keymgr/barbican.py b/nova/keymgr/barbican.py index 9f68b1b5360e..9579ea0623ff 100644 --- a/nova/keymgr/barbican.py +++ b/nova/keymgr/barbican.py @@ -62,6 +62,7 @@ class BarbicanKeyManager(key_mgr.KeyManager): def __init__(self): self._barbican_client = None + self._current_context = None self._base_url = None def _get_barbican_client(self, ctxt): @@ -72,47 +73,56 @@ class BarbicanKeyManager(key_mgr.KeyManager): :raises Forbidden: if the ctxt is None """ - if not self._barbican_client: - # Confirm context is provided, if not raise forbidden - if not ctxt: - msg = _("User is not authorized to use key manager.") - LOG.error(msg) - raise exception.Forbidden(msg) + # Confirm context is provided, if not raise forbidden + if not ctxt: + msg = _("User is not authorized to use key manager.") + LOG.error(msg) + raise exception.Forbidden(msg) - try: - _SESSION = session.Session.load_from_conf_options( - CONF, - BARBICAN_OPT_GROUP) + if not hasattr(ctxt, 'project_id') or ctxt.project_id is None: + msg = _("Unable to create Barbican Client without project_id.") + LOG.error(msg) + raise exception.KeyManagerError(msg) - auth = ctxt.get_auth_plugin() - service_type, service_name, interface = (CONF. - barbican. - catalog_info. - split(':')) - region_name = CONF.barbican.os_region_name - service_parameters = {'service_type': service_type, - 'service_name': service_name, - 'interface': interface, - 'region_name': region_name} + # If same context, return cached barbican client + if self._barbican_client and self._current_context == ctxt: + return self._barbican_client - if CONF.barbican.endpoint_template: - self._base_url = (CONF.barbican.endpoint_template % - ctxt.to_dict()) - else: - self._base_url = _SESSION.get_endpoint( - auth, **service_parameters) + try: + _SESSION = session.Session.load_from_conf_options( + CONF, + BARBICAN_OPT_GROUP) - # the barbican endpoint can't have the '/v1' on the end - self._barbican_endpoint = self._base_url.rpartition('/')[0] + auth = ctxt.get_auth_plugin() + service_type, service_name, interface = (CONF. + barbican. + catalog_info. + split(':')) + region_name = CONF.barbican.os_region_name + service_parameters = {'service_type': service_type, + 'service_name': service_name, + 'interface': interface, + 'region_name': region_name} - sess = session.Session(auth=auth) - self._barbican_client = barbican_client.Client( - session=sess, - endpoint=self._barbican_endpoint) + if CONF.barbican.endpoint_template: + self._base_url = (CONF.barbican.endpoint_template % + ctxt.to_dict()) + else: + self._base_url = _SESSION.get_endpoint( + auth, **service_parameters) - except Exception as e: - with excutils.save_and_reraise_exception(): - LOG.error(_LE("Error creating Barbican client: %s"), e) + # the barbican endpoint can't have the '/v1' on the end + self._barbican_endpoint = self._base_url.rpartition('/')[0] + + sess = session.Session(auth=auth) + self._barbican_client = barbican_client.Client( + session=sess, + endpoint=self._barbican_endpoint) + self._current_context = ctxt + + except Exception as e: + with excutils.save_and_reraise_exception(): + LOG.error(_LE("Error creating Barbican client: %s"), e) return self._barbican_client diff --git a/nova/tests/unit/keymgr/test_barbican.py b/nova/tests/unit/keymgr/test_barbican.py index 36901f36601d..a1c32cfeca32 100644 --- a/nova/tests/unit/keymgr/test_barbican.py +++ b/nova/tests/unit/keymgr/test_barbican.py @@ -37,8 +37,9 @@ class BarbicanKeyManagerTestCase(test_key_mgr.KeyManagerTestCase): super(BarbicanKeyManagerTestCase, self).setUp() # Create fake auth_token - self.ctxt = mock.Mock() + self.ctxt = mock.MagicMock() self.ctxt.auth_token = "fake_token" + self.ctxt.project = "fake_project" # Create mock barbican client self._build_mock_barbican() @@ -49,6 +50,7 @@ class BarbicanKeyManagerTestCase(test_key_mgr.KeyManagerTestCase): self.pre_hex = "AIDxQp2++uAbKaTVDMXFYIu8PIugJGqkK0JLqkU0rhY=" self.hex = ("0080f1429dbefae01b29a4d50cc5c5608bbc3c8ba0246aa42b424baa4" "534ae16") + self.key_mgr._current_context = self.ctxt self.key_mgr._base_url = "http://host:9311/v1" self.addCleanup(self._restore) @@ -221,3 +223,51 @@ class BarbicanKeyManagerTestCase(test_key_mgr.KeyManagerTestCase): self.key_mgr._barbican_client = None self.assertRaises(exception.Forbidden, self.key_mgr.store_key, None, None) + + @mock.patch('keystoneclient.session.Session') + @mock.patch('barbicanclient.client.Client') + def test_get_barbican_client_new(self, mock_barbican, mock_keystone): + manager = self._create_key_manager() + manager._get_barbican_client(self.ctxt) + self.assertEqual(mock_keystone.call_count, 1) + self.assertEqual(mock_barbican.call_count, 1) + + @mock.patch('keystoneclient.session.Session') + @mock.patch('barbicanclient.client.Client') + def test_get_barbican_client_reused(self, mock_barbican, mock_keystone): + manager = self._create_key_manager() + manager._get_barbican_client(self.ctxt) + self.assertEqual(mock_keystone.call_count, 1) + self.assertEqual(mock_barbican.call_count, 1) + manager._get_barbican_client(self.ctxt) + self.assertEqual(mock_keystone.call_count, 1) + self.assertEqual(mock_barbican.call_count, 1) + + @mock.patch('keystoneclient.session.Session') + @mock.patch('barbicanclient.client.Client') + def test_get_barbican_client_not_reused(self, mock_barbican, + mock_keystone): + manager = self._create_key_manager() + manager._get_barbican_client(self.ctxt) + self.assertEqual(mock_keystone.call_count, 1) + self.assertEqual(mock_barbican.call_count, 1) + ctxt2 = mock.MagicMock() + ctxt2.auth_token = "fake_token2" + ctxt2.project = "fake_project2" + manager._get_barbican_client(ctxt2) + self.assertEqual(mock_keystone.call_count, 2) + self.assertEqual(mock_barbican.call_count, 2) + + def test_get_barbican_client_null_context(self): + self.assertRaises(exception.Forbidden, + self.key_mgr._get_barbican_client, None) + + def test_get_barbican_client_missing_project(self): + del(self.ctxt.project_id) + self.assertRaises(exception.KeyManagerError, + self.key_mgr._get_barbican_client, self.ctxt) + + def test_get_barbican_client_none_project(self): + self.ctxt.project_id = None + self.assertRaises(exception.KeyManagerError, + self.key_mgr._get_barbican_client, self.ctxt)