From 8a6a291e033e0d191ac8702f05fa3e83206d61f0 Mon Sep 17 00:00:00 2001 From: "Jay S. Bryant" Date: Fri, 25 Oct 2013 11:43:46 -0500 Subject: [PATCH] Enable del of other tenants resources by name Currently, due to the way that resources are being retrieved by the findall() function, an administrator can do a list, snapshot-list, etc. with the --all_tenants option and see other tenants' resources. If the admin then tries to delete the another tenants' resource by name, it fails with a 'No with a name or ID of exists.' error. The solution to this is to change the call to the list() function in findall() to set the all_tenants search option to 1. This causes the admin to get a list of all the resources that they have access to back when the search is done instead of just a list of their resources. The delete by name is then possible. The server takes care of ensuring that only resources that the user has access to are returned. This will enable delete by name for all resources that use the find_resource function. Closes-bug: 1241682 Change-Id: I4e9957b66c11b7e1081f066d189cedc5a3cb2a6c --- cinderclient/base.py | 5 ++++- cinderclient/tests/test_utils.py | 2 +- cinderclient/tests/v1/test_shell.py | 5 +++++ cinderclient/tests/v2/test_shell.py | 5 +++++ 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/cinderclient/base.py b/cinderclient/base.py index 73bdec603..30dbbd908 100644 --- a/cinderclient/base.py +++ b/cinderclient/base.py @@ -203,7 +203,10 @@ class ManagerWithFind(six.with_metaclass(abc.ABCMeta, Manager)): found = [] searches = list(kwargs.items()) - for obj in self.list(): + # Want to search for all tenants here so that when attempting to delete + # that a user like admin doesn't get a failure when trying to delete + # another tenant's volume by name. + for obj in self.list(search_opts={'all_tenants': 1}): try: if all(getattr(obj, attr) == value for (attr, value) in searches): diff --git a/cinderclient/tests/test_utils.py b/cinderclient/tests/test_utils.py index 32db04c06..92aee5123 100644 --- a/cinderclient/tests/test_utils.py +++ b/cinderclient/tests/test_utils.py @@ -55,7 +55,7 @@ class FakeManager(base.ManagerWithFind): return resource raise exceptions.NotFound(resource_id) - def list(self): + def list(self, search_opts): return self.resources diff --git a/cinderclient/tests/v1/test_shell.py b/cinderclient/tests/v1/test_shell.py index 3b431b947..6294036a0 100644 --- a/cinderclient/tests/v1/test_shell.py +++ b/cinderclient/tests/v1/test_shell.py @@ -117,6 +117,11 @@ class ShellTest(utils.TestCase): self.run_command('delete 1234') self.assert_called('DELETE', '/volumes/1234') + def test_delete_by_name(self): + self.run_command('delete sample-volume') + self.assert_called_anytime('GET', '/volumes/detail?all_tenants=1') + self.assert_called('DELETE', '/volumes/1234') + def test_delete_multiple(self): self.run_command('delete 1234 5678') self.assert_called('DELETE', '/volumes/5678') diff --git a/cinderclient/tests/v2/test_shell.py b/cinderclient/tests/v2/test_shell.py index cc29113ab..5928296b3 100644 --- a/cinderclient/tests/v2/test_shell.py +++ b/cinderclient/tests/v2/test_shell.py @@ -95,6 +95,11 @@ class ShellTest(utils.TestCase): self.run_command('delete 1234') self.assert_called('DELETE', '/volumes/1234') + def test_delete_by_name(self): + self.run_command('delete sample-volume') + self.assert_called_anytime('GET', '/volumes/detail?all_tenants=1') + self.assert_called('DELETE', '/volumes/1234') + def test_delete_multiple(self): self.run_command('delete 1234 5678') self.assert_called('DELETE', '/volumes/5678')