diff --git a/castellan/key_manager/barbican_key_manager.py b/castellan/key_manager/barbican_key_manager.py index 5d758710..9036aa6c 100644 --- a/castellan/key_manager/barbican_key_manager.py +++ b/castellan/key_manager/barbican_key_manager.py @@ -619,19 +619,23 @@ class BarbicanKeyManager(key_manager.KeyManager): else: raise exception.KeyManagerError(reason=e) - def delete(self, context, managed_object_id): + def delete(self, context, managed_object_id, force=False): """Deletes the specified managed object. :param context: contains information of the user and the environment for the request (castellan/context.py) :param managed_object_id: the UUID of the object to delete + :param force: specifies if the secret must be deleted even when they + have consumers. + :raises ValueError: if the secret has consumers but no force parameter + is provided or if force equals False. :raises KeyManagerError: if object deletion fails :raises ManagedObjectNotFoundError: if the object could not be found """ barbican_client = self._get_barbican_client(context) try: secret_ref = self._create_secret_ref(managed_object_id) - barbican_client.secrets.delete(secret_ref) + barbican_client.secrets.delete(secret_ref, force) except (barbican_exceptions.HTTPAuthError, barbican_exceptions.HTTPClientError, barbican_exceptions.HTTPServerError) as e: diff --git a/castellan/key_manager/key_manager.py b/castellan/key_manager/key_manager.py index ff868cb0..5640c897 100644 --- a/castellan/key_manager/key_manager.py +++ b/castellan/key_manager/key_manager.py @@ -106,7 +106,7 @@ class KeyManager(object, metaclass=abc.ABCMeta): pass @abc.abstractmethod - def delete(self, context, managed_object_id): + def delete(self, context, managed_object_id, force=False): """Deletes the specified managed object. Implementations should verify that the caller has permission to delete @@ -119,6 +119,10 @@ class KeyManager(object, metaclass=abc.ABCMeta): UUIDs of objects that belong to other users by repeatedly calling this method. That is, objects that belong to other users should be considered "non-existent" and completely invisible. + + Implementations that block the deletion of secrets with consumers + without making the "force" parameter equals True should raise + an exception. """ pass diff --git a/castellan/key_manager/not_implemented_key_manager.py b/castellan/key_manager/not_implemented_key_manager.py index 2c169a4c..11e3ebd5 100644 --- a/castellan/key_manager/not_implemented_key_manager.py +++ b/castellan/key_manager/not_implemented_key_manager.py @@ -48,7 +48,7 @@ class NotImplementedKeyManager(key_manager.KeyManager): def list(self, context, object_type=None): raise NotImplementedError() - def delete(self, context, managed_object_id): + def delete(self, context, managed_object_id, force=False): raise NotImplementedError() def add_consumer(self, context, managed_object_id, consumer_data): diff --git a/castellan/key_manager/vault_key_manager.py b/castellan/key_manager/vault_key_manager.py index 51720e37..8d907c89 100644 --- a/castellan/key_manager/vault_key_manager.py +++ b/castellan/key_manager/vault_key_manager.py @@ -347,8 +347,12 @@ class VaultKeyManager(key_manager.KeyManager): record['created'], key_id) - def delete(self, context, key_id): - """Represents deleting the key.""" + def delete(self, context, key_id, force=False): + """Represents deleting the key. + + The 'force' parameter is not used whatsoever and only kept to allow + consistency with the Barbican implementation. + """ if not key_id: raise exception.KeyManagerError('key identifier not provided') diff --git a/castellan/tests/unit/key_manager/test_barbican_key_manager.py b/castellan/tests/unit/key_manager/test_barbican_key_manager.py index db684bb5..00f3bce2 100644 --- a/castellan/tests/unit/key_manager/test_barbican_key_manager.py +++ b/castellan/tests/unit/key_manager/test_barbican_key_manager.py @@ -383,7 +383,30 @@ class BarbicanKeyManagerTestCase(test_key_manager.KeyManagerTestCase): def test_delete_key(self): self.key_mgr.delete(self.ctxt, self.key_id) - self.delete.assert_called_once_with(self.secret_ref) + self.delete.assert_called_once_with(self.secret_ref, False) + + def test_delete_secret_with_consumers_no_force_parameter(self): + self.mock_barbican.secrets.delete = mock.Mock( + side_effect=exception.KeyManagerError( + "Secret has consumers! Use the 'force' parameter.")) + self.assertRaises(exception.KeyManagerError, + self.key_mgr.delete, self.ctxt, self.key_id) + self.mock_barbican.secrets.delete.assert_called_once_with( + self.secret_ref, False) + + def test_delete_secret_with_consumers_force_parameter_false(self): + self.mock_barbican.secrets.delete = mock.Mock( + side_effect=barbican_exceptions.HTTPClientError( + "Secret has consumers! Use the 'force' parameter.")) + self.assertRaises(exception.KeyManagerError, + self.key_mgr.delete, self.ctxt, self.key_id, + force=False) + self.mock_barbican.secrets.delete.assert_called_once_with( + self.secret_ref, False) + + def test_delete_secret_with_consumers_force_parameter_true(self): + self.key_mgr.delete(self.ctxt, self.key_id, force=True) + self.delete.assert_called_once_with(self.secret_ref, True) def test_delete_unknown_key(self): self.assertRaises(exception.KeyManagerError,