From fd191b7e4846c22eac7e9961d1ee637de30cf2b6 Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Mon, 30 Jun 2014 17:09:31 +1200 Subject: [PATCH] Implement barbican client plugin This moves the client creation code out of a Clients subclass into its own client plugin. Barbican is not an integrated project so the plugin is not registered if barbicanclient cannot be imported. The code which prevented client errors in _resolve_attribute from being raised has been removed. This will prevent invalid attribute values from being memoized. The base package has been renamed to heat_barbican since stevedore is used for client plugin loading and the base package will be installed by setuptools, the package barbican might clash with the actual Barbican project. Change-Id: I5788fb4618fef0d1c3e39cc98094bb2813e842c5 --- contrib/{barbican => heat_barbican}/README.md | 0 .../heat_barbican}/__init__.py | 0 .../heat_barbican/client.py} | 13 +++++---- .../heat_barbican}/resources/__init__.py | 0 .../heat_barbican}/resources/order.py | 19 ++++-------- .../heat_barbican}/resources/secret.py | 29 +++++++------------ .../heat_barbican}/tests/__init__.py | 0 .../heat_barbican}/tests/test_order.py | 25 ++++++++-------- .../heat_barbican}/tests/test_secret.py | 25 ++++++++-------- .../requirements.txt | 0 contrib/{barbican => heat_barbican}/setup.cfg | 5 +++- contrib/{barbican => heat_barbican}/setup.py | 0 12 files changed, 51 insertions(+), 65 deletions(-) rename contrib/{barbican => heat_barbican}/README.md (100%) rename contrib/{barbican/barbican => heat_barbican/heat_barbican}/__init__.py (100%) rename contrib/{barbican/barbican/clients.py => heat_barbican/heat_barbican/client.py} (75%) rename contrib/{barbican/barbican => heat_barbican/heat_barbican}/resources/__init__.py (100%) rename contrib/{barbican/barbican => heat_barbican/heat_barbican}/resources/order.py (88%) rename contrib/{barbican/barbican => heat_barbican/heat_barbican}/resources/secret.py (84%) rename contrib/{barbican/barbican => heat_barbican/heat_barbican}/tests/__init__.py (100%) rename contrib/{barbican/barbican => heat_barbican/heat_barbican}/tests/test_order.py (89%) rename contrib/{barbican/barbican => heat_barbican/heat_barbican}/tests/test_secret.py (89%) rename contrib/{barbican => heat_barbican}/requirements.txt (100%) rename contrib/{barbican => heat_barbican}/setup.cfg (90%) rename contrib/{barbican => heat_barbican}/setup.py (100%) diff --git a/contrib/barbican/README.md b/contrib/heat_barbican/README.md similarity index 100% rename from contrib/barbican/README.md rename to contrib/heat_barbican/README.md diff --git a/contrib/barbican/barbican/__init__.py b/contrib/heat_barbican/heat_barbican/__init__.py similarity index 100% rename from contrib/barbican/barbican/__init__.py rename to contrib/heat_barbican/heat_barbican/__init__.py diff --git a/contrib/barbican/barbican/clients.py b/contrib/heat_barbican/heat_barbican/client.py similarity index 75% rename from contrib/barbican/barbican/clients.py rename to contrib/heat_barbican/heat_barbican/client.py index ee811f7cef..7d5a6b9db3 100644 --- a/contrib/barbican/barbican/clients.py +++ b/contrib/heat_barbican/heat_barbican/client.py @@ -11,7 +11,7 @@ # License for the specific language governing permissions and limitations # under the License. -from heat.engine import clients as heat_clients +from heat.engine.clients import client_plugin try: @@ -22,9 +22,12 @@ except ImportError: auth = None -class Clients(heat_clients.OpenStackClients): +class BarbicanClientPlugin(client_plugin.ClientPlugin): - def _barbican(self): - keystone_client = self.client('keystone').client + def _create(self): + + keystone_client = self.clients('keystone').client auth_plugin = auth.KeystoneAuthV2(keystone=keystone_client) - return barbican_client.Client(auth_plugin=auth_plugin) + client = barbican_client.Client(auth_plugin=auth_plugin) + + return client diff --git a/contrib/barbican/barbican/resources/__init__.py b/contrib/heat_barbican/heat_barbican/resources/__init__.py similarity index 100% rename from contrib/barbican/barbican/resources/__init__.py rename to contrib/heat_barbican/heat_barbican/resources/__init__.py diff --git a/contrib/barbican/barbican/resources/order.py b/contrib/heat_barbican/heat_barbican/resources/order.py similarity index 88% rename from contrib/barbican/barbican/resources/order.py rename to contrib/heat_barbican/heat_barbican/resources/order.py index 18b90ffd9d..380932902d 100644 --- a/contrib/barbican/barbican/resources/order.py +++ b/contrib/heat_barbican/heat_barbican/resources/order.py @@ -15,12 +15,13 @@ import six from heat.common import exception from heat.engine import attributes +from heat.engine import clients from heat.engine import constraints from heat.engine import properties from heat.engine import resource from heat.openstack.common import log as logging -from .. import clients # noqa +from .. import client # noqa LOG = logging.getLogger(__name__) @@ -104,10 +105,6 @@ class Order(resource.Resource): SECRET_REF: attributes.Schema(_('The URI to the created secret.')), } - def __init__(self, name, json_snippet, stack): - super(Order, self).__init__(name, json_snippet, stack) - self.clients = clients.Clients(self.context) - def barbican(self): return self.clients.client('barbican') @@ -136,7 +133,7 @@ class Order(resource.Resource): try: self.barbican().orders.delete(self.resource_id) self.resource_id_set(None) - except clients.barbican_client.HTTPClientError as exc: + except client.barbican_client.HTTPClientError as exc: # This is the only exception the client raises # Inspecting the message to see if it's a 'Not Found' if 'Not Found' in six.text_type(exc): @@ -145,13 +142,7 @@ class Order(resource.Resource): raise def _resolve_attribute(self, name): - try: - order = self.barbican().orders.get(self.resource_id) - except clients.barbican_client.HTTPClientError as exc: - LOG.warn(_("Order '%(name)s' not found: %(exc)s") % - {'name': self.resource_id, 'exc': six.text_type(exc)}) - return '' - + order = self.barbican().orders.get(self.resource_id) return getattr(order, name) @@ -162,7 +153,7 @@ def resource_mapping(): def available_resource_mapping(): - if not clients.barbican_client: + if not clients.has_client('barbican'): return {} return resource_mapping() diff --git a/contrib/barbican/barbican/resources/secret.py b/contrib/heat_barbican/heat_barbican/resources/secret.py similarity index 84% rename from contrib/barbican/barbican/resources/secret.py rename to contrib/heat_barbican/heat_barbican/resources/secret.py index 9ee9a3fd7a..4eafd9df25 100644 --- a/contrib/barbican/barbican/resources/secret.py +++ b/contrib/heat_barbican/heat_barbican/resources/secret.py @@ -15,12 +15,13 @@ import six from heat.common import exception from heat.engine import attributes +from heat.engine import clients from heat.engine import constraints from heat.engine import properties from heat.engine import resource from heat.openstack.common import log as logging -from .. import clients # noqa +from .. import client # noqa LOG = logging.getLogger(__name__) @@ -107,10 +108,6 @@ class Secret(resource.Resource): ), } - def __init__(self, name, json_snippet, stack): - super(Secret, self).__init__(name, json_snippet, stack) - self.clients = clients.Clients(self.context) - def barbican(self): return self.clients.client('barbican') @@ -141,7 +138,7 @@ class Secret(resource.Resource): try: self.barbican().secrets.delete(self.resource_id) self.resource_id_set(None) - except clients.barbican_client.HTTPClientError as exc: + except client.barbican_client.HTTPClientError as exc: # This is the only exception the client raises # Inspecting the message to see if it's a 'Not Found' if 'Not Found' in six.text_type(exc): @@ -150,19 +147,13 @@ class Secret(resource.Resource): raise def _resolve_attribute(self, name): - try: - if name == self.DECRYPTED_PAYLOAD: - return self.barbican().secrets.decrypt( - self.resource_id) + if name == self.DECRYPTED_PAYLOAD: + return self.barbican().secrets.decrypt( + self.resource_id) - secret = self.barbican().secrets.get(self.resource_id) - if name == self.STATUS: - return secret.status - except clients.barbican_client.HTTPClientError as e: - msg = _("Failed to resolve '%(name)s' for %(res)s '%(id)s': %(e)s") - LOG.warn(msg % {'name': name, 'res': self.__class__.__name__, - 'id': self.resource_id, 'e': six.text_type(e)}) - return '' + secret = self.barbican().secrets.get(self.resource_id) + if name == self.STATUS: + return secret.status def resource_mapping(): @@ -172,7 +163,7 @@ def resource_mapping(): def available_resource_mapping(): - if not clients.barbican_client: + if not clients.has_client('barbican'): return {} return resource_mapping() diff --git a/contrib/barbican/barbican/tests/__init__.py b/contrib/heat_barbican/heat_barbican/tests/__init__.py similarity index 100% rename from contrib/barbican/barbican/tests/__init__.py rename to contrib/heat_barbican/heat_barbican/tests/__init__.py diff --git a/contrib/barbican/barbican/tests/test_order.py b/contrib/heat_barbican/heat_barbican/tests/test_order.py similarity index 89% rename from contrib/barbican/barbican/tests/test_order.py rename to contrib/heat_barbican/heat_barbican/tests/test_order.py index c025194383..f76caaf5ce 100644 --- a/contrib/barbican/barbican/tests/test_order.py +++ b/contrib/heat_barbican/heat_barbican/tests/test_order.py @@ -22,9 +22,9 @@ from heat.engine import scheduler from heat.tests.common import HeatTestCase from heat.tests import utils +from .. import client # noqa from ..resources import order # noqa - stack_template = ''' heat_template_version: 2013-05-23 description: Test template @@ -94,17 +94,16 @@ class TestOrder(HeatTestCase): self.assertEqual('test-order-ref', res.FnGetAtt('order_ref')) self.assertEqual('test-secret-ref', res.FnGetAtt('secret_ref')) - @mock.patch.object(order.clients, 'barbican_client', new=mock.Mock()) + @mock.patch.object(client, 'barbican_client', new=mock.Mock()) def test_attributes_handle_exceptions(self): mock_order = mock.Mock() self.barbican.orders.get.return_value = mock_order res = self._create_resource('foo', self.res_template, self.stack) - order.clients.barbican_client.HTTPClientError = Exception - not_found_exc = order.clients.barbican_client.HTTPClientError('boom') - self.barbican.orders.get.side_effect = not_found_exc - - self.assertEqual('', res.FnGetAtt('order_ref')) + client.barbican_client.HTTPClientError = Exception + self.barbican.orders.get.side_effect = Exception('boom') + self.assertRaises(client.barbican_client.HTTPClientError, + res.FnGetAtt, 'order_ref') def test_create_order_sets_resource_id(self): self.barbican.orders.create.return_value = 'foo' @@ -147,22 +146,22 @@ class TestOrder(HeatTestCase): self.assertIsNone(res.resource_id) self.barbican.orders.delete.assert_called_once_with('foo') - @mock.patch.object(order.clients, 'barbican_client', new=mock.Mock()) + @mock.patch.object(client, 'barbican_client', new=mock.Mock()) def test_handle_delete_ignores_not_found_errors(self): res = self._create_resource('foo', self.res_template, self.stack) - order.clients.barbican_client.HTTPClientError = Exception - exc = order.clients.barbican_client.HTTPClientError('Not Found. Nope.') + client.barbican_client.HTTPClientError = Exception + exc = client.barbican_client.HTTPClientError('Not Found. Nope.') self.barbican.orders.delete.side_effect = exc scheduler.TaskRunner(res.delete)() self.assertTrue(self.barbican.orders.delete.called) - @mock.patch.object(order.clients, 'barbican_client', new=mock.Mock()) + @mock.patch.object(client, 'barbican_client', new=mock.Mock()) def test_handle_delete_raises_resource_failure_on_error(self): res = self._create_resource('foo', self.res_template, self.stack) - order.clients.barbican_client.HTTPClientError = Exception - exc = order.clients.barbican_client.HTTPClientError('Boom.') + client.barbican_client.HTTPClientError = Exception + exc = client.barbican_client.HTTPClientError('Boom.') self.barbican.orders.delete.side_effect = exc exc = self.assertRaises(exception.ResourceFailure, scheduler.TaskRunner(res.delete)) diff --git a/contrib/barbican/barbican/tests/test_secret.py b/contrib/heat_barbican/heat_barbican/tests/test_secret.py similarity index 89% rename from contrib/barbican/barbican/tests/test_secret.py rename to contrib/heat_barbican/heat_barbican/tests/test_secret.py index 7c931bfa5f..ef6293404a 100644 --- a/contrib/barbican/barbican/tests/test_secret.py +++ b/contrib/heat_barbican/heat_barbican/tests/test_secret.py @@ -22,9 +22,9 @@ from heat.engine import scheduler from heat.tests.common import HeatTestCase from heat.tests import utils +from .. import client # noqa from ..resources import secret # noqa - stack_template = ''' heat_template_version: 2013-05-23 description: Test template @@ -83,13 +83,12 @@ class TestSecret(HeatTestCase): self.assertEqual('test-status', self.res.FnGetAtt('status')) self.assertEqual('foo', self.res.FnGetAtt('decrypted_payload')) - @mock.patch.object(secret.clients, 'barbican_client', new=mock.Mock()) + @mock.patch.object(client, 'barbican_client', new=mock.Mock()) def test_attributes_handles_exceptions(self): - secret.clients.barbican_client.HTTPClientError = Exception - some_error = secret.clients.barbican_client.HTTPClientError('boom') - secret.Secret.barbican.side_effect = some_error - - self.assertEqual('', self.res.FnGetAtt('status')) + client.barbican_client.HTTPClientError = Exception + self.barbican.secrets.get.side_effect = Exception('boom') + self.assertRaises(client.barbican_client.HTTPClientError, + self.res.FnGetAtt, 'order_ref') def test_create_secret_sets_resource_id(self): self.assertEqual('foo_id', self.res.resource_id) @@ -164,18 +163,18 @@ class TestSecret(HeatTestCase): self.assertIsNone(self.res.resource_id) mock_delete.assert_called_once_with('foo_id') - @mock.patch.object(secret.clients, 'barbican_client', new=mock.Mock()) + @mock.patch.object(client, 'barbican_client', new=mock.Mock()) def test_handle_delete_ignores_not_found_errors(self): - secret.clients.barbican_client.HTTPClientError = Exception - exc = secret.clients.barbican_client.HTTPClientError('Not Found.') + client.barbican_client.HTTPClientError = Exception + exc = client.barbican_client.HTTPClientError('Not Found.') self.barbican.secrets.delete.side_effect = exc scheduler.TaskRunner(self.res.delete)() self.assertTrue(self.barbican.secrets.delete.called) - @mock.patch.object(secret.clients, 'barbican_client', new=mock.Mock()) + @mock.patch.object(client, 'barbican_client', new=mock.Mock()) def test_handle_delete_raises_resource_failure_on_error(self): - secret.clients.barbican_client.HTTPClientError = Exception - exc = secret.clients.barbican_client.HTTPClientError('Boom.') + client.barbican_client.HTTPClientError = Exception + exc = client.barbican_client.HTTPClientError('Boom.') self.barbican.secrets.delete.side_effect = exc exc = self.assertRaises(exception.ResourceFailure, scheduler.TaskRunner(self.res.delete)) diff --git a/contrib/barbican/requirements.txt b/contrib/heat_barbican/requirements.txt similarity index 100% rename from contrib/barbican/requirements.txt rename to contrib/heat_barbican/requirements.txt diff --git a/contrib/barbican/setup.cfg b/contrib/heat_barbican/setup.cfg similarity index 90% rename from contrib/barbican/setup.cfg rename to contrib/heat_barbican/setup.cfg index c3eb71bf6b..a4e4015b46 100644 --- a/contrib/barbican/setup.cfg +++ b/contrib/heat_barbican/setup.cfg @@ -17,10 +17,13 @@ classifier = Programming Language :: Python :: 2.7 Programming Language :: Python :: 2.6 +packages = + heat_barbican + [files] # Copy to /usr/lib/heat for plugin loading data_files = - lib/heat/barbican = barbican/resources/* + lib/heat/barbican = heat_barbican/resources/* [global] setup-hooks = diff --git a/contrib/barbican/setup.py b/contrib/heat_barbican/setup.py similarity index 100% rename from contrib/barbican/setup.py rename to contrib/heat_barbican/setup.py