From e31ef64e6ea82bcdbfb01114495ff52db3bba840 Mon Sep 17 00:00:00 2001 From: Madhuri Kumari Date: Mon, 30 May 2016 17:54:06 +0530 Subject: [PATCH] Delete certs when deleting bay Currently Magnum fails to delete certificates when barbican cert manager is used. The code was copied from neutron-lbaas and they have different usecase. In our case, certificate is managed by Magnum not users, so we should delete certificates when deleting bay. So this patch deletes all the certs related to a bay. Change-Id: I5aab01641b9447153911680c5f68e5fe2c5a1409 Closes-bug: #1587033 --- .../cert_manager/barbican_cert_manager.py | 29 +------------------ magnum/conductor/handlers/bay_conductor.py | 2 +- magnum/tests/functional/api/v1/test_bay.py | 3 ++ .../unit/common/cert_manager/test_barbican.py | 24 ++------------- .../conductor/handlers/test_bay_conductor.py | 11 +++++-- 5 files changed, 16 insertions(+), 53 deletions(-) diff --git a/magnum/common/cert_manager/barbican_cert_manager.py b/magnum/common/cert_manager/barbican_cert_manager.py index 68e847b2ba..d35b72d0c2 100644 --- a/magnum/common/cert_manager/barbican_cert_manager.py +++ b/magnum/common/cert_manager/barbican_cert_manager.py @@ -190,34 +190,7 @@ class CertManager(cert_manager.CertManager): @staticmethod def delete_cert(cert_ref, service_name='Magnum', resource_ref=None, **kwargs): - """Deregister as a consumer for the specified cert. - - :param cert_ref: the UUID of the cert to retrieve - :param service_name: Friendly name for the consuming service - :param resource_ref: Full HATEOAS reference to the consuming resource - - :raises Exception: if deregistration fails - """ - connection = get_admin_clients().barbican() - - LOG.info(_LI( - "Deregistering as a consumer of {0} in Barbican." - ).format(cert_ref)) - try: - connection.containers.remove_consumer( - container_ref=cert_ref, - name=service_name, - url=resource_ref - ) - except Exception: - with excutils.save_and_reraise_exception(): - LOG.exception(_LE( - "Error deregistering as a consumer of {0}" - ).format(cert_ref)) - - @staticmethod - def _actually_delete_cert(cert_ref): - """Deletes the specified cert. Very dangerous. Do not recommend. + """Deletes the specified cert. :param cert_ref: the UUID of the cert to delete :raises Exception: if certificate deletion fails diff --git a/magnum/conductor/handlers/bay_conductor.py b/magnum/conductor/handlers/bay_conductor.py index 6231763eee..b21d5504ff 100644 --- a/magnum/conductor/handlers/bay_conductor.py +++ b/magnum/conductor/handlers/bay_conductor.py @@ -229,7 +229,7 @@ class Handler(object): context, taxonomy.ACTION_DELETE, taxonomy.OUTCOME_PENDING) osc.heat().stacks.delete(stack_id) except exc.HTTPNotFound: - LOG.info(_LI('The stack %s was not be found during bay' + LOG.info(_LI('The stack %s was not found during bay' ' deletion.'), stack_id) try: trust_manager.delete_trustee_and_trust(osc, context, bay) diff --git a/magnum/tests/functional/api/v1/test_bay.py b/magnum/tests/functional/api/v1/test_bay.py index 23a443e503..6e227afc41 100644 --- a/magnum/tests/functional/api/v1/test_bay.py +++ b/magnum/tests/functional/api/v1/test_bay.py @@ -114,6 +114,9 @@ class BayTest(base.BaseMagnumTest): resp, model = self.bay_client.delete_bay(bay_id) self.assertEqual(204, resp.status) self.bay_client.wait_for_bay_to_delete(bay_id) + self.assertRaises( + exceptions.NotFound, + self.cert_client.get_cert, bay_id) return resp, model def _get_bay_by_id(self, bay_id): diff --git a/magnum/tests/unit/common/cert_manager/test_barbican.py b/magnum/tests/unit/common/cert_manager/test_barbican.py index 4f25b4d6a3..061bc84949 100644 --- a/magnum/tests/unit/common/cert_manager/test_barbican.py +++ b/magnum/tests/unit/common/cert_manager/test_barbican.py @@ -274,33 +274,13 @@ class TestBarbicanManager(base.BaseTestCase): @patch('magnum.common.clients.OpenStackClients.barbican') def test_delete_cert(self, mock_barbican): - # Mock out the client - bc = mock.MagicMock() - mock_barbican.return_value = bc - - # Attempt to deregister as a consumer - bcm.CertManager.delete_cert( - cert_ref=self.container_ref, - resource_ref=self.container_ref, - service_name='Magnum' - ) - - # remove_consumer should be called once with the container_ref - bc.containers.remove_consumer.assert_called_once_with( - container_ref=self.container_ref, - url=self.container_ref, - name='Magnum' - ) - - @patch('magnum.common.clients.OpenStackClients.barbican') - def test_actually_delete_cert(self, mock_barbican): # Mock out the client bc = mock.MagicMock() bc.containers.get.return_value = self.container mock_barbican.return_value = bc - # Attempt to store a cert - bcm.CertManager._actually_delete_cert( + # Attempt to delete a cert + bcm.CertManager.delete_cert( cert_ref=self.container_ref ) diff --git a/magnum/tests/unit/conductor/handlers/test_bay_conductor.py b/magnum/tests/unit/conductor/handlers/test_bay_conductor.py index 5b409d548b..af5642247b 100644 --- a/magnum/tests/unit/conductor/handlers/test_bay_conductor.py +++ b/magnum/tests/unit/conductor/handlers/test_bay_conductor.py @@ -448,8 +448,9 @@ class TestHandler(db_base.DbTestCase): template='some template yaml', timeout_mins=timeout) + @patch('magnum.conductor.handlers.bay_conductor.cert_manager') @patch('magnum.common.clients.OpenStackClients') - def test_bay_delete(self, mock_openstack_client_class): + def test_bay_delete(self, mock_openstack_client_class, cert_manager): osc = mock.MagicMock() mock_openstack_client_class.return_value = osc osc.heat.side_effect = exc.HTTPNotFound @@ -465,12 +466,16 @@ class TestHandler(db_base.DbTestCase): 'magnum.bay.delete', notifications[1].event_type) self.assertEqual( taxonomy.OUTCOME_SUCCESS, notifications[1].payload['outcome']) + self.assertEqual(1, + cert_manager.delete_certificates_from_bay.call_count) # The bay has been destroyed self.assertRaises(exception.BayNotFound, objects.Bay.get, self.context, self.bay.uuid) + @patch('magnum.conductor.handlers.bay_conductor.cert_manager') @patch('magnum.common.clients.OpenStackClients') - def test_bay_delete_conflict(self, mock_openstack_client_class): + def test_bay_delete_conflict(self, mock_openstack_client_class, + cert_manager): osc = mock.MagicMock() mock_openstack_client_class.return_value = osc osc.heat.side_effect = exc.HTTPConflict @@ -489,6 +494,8 @@ class TestHandler(db_base.DbTestCase): 'magnum.bay.delete', notifications[1].event_type) self.assertEqual( taxonomy.OUTCOME_FAILURE, notifications[1].payload['outcome']) + self.assertEqual(0, + cert_manager.delete_certificates_from_bay.call_count) class TestHeatPoller(base.TestCase):