diff options
author | Jenkins <jenkins@review.openstack.org> | 2015-07-25 10:44:31 +0000 |
---|---|---|
committer | Gerrit Code Review <review@openstack.org> | 2015-07-25 10:44:31 +0000 |
commit | 87dbd7e218430c3d498f5ffa3160a6ac1767ef03 (patch) | |
tree | 10038112f66bd0ecfaa1db22b546be8643d12312 | |
parent | 1a2ccb001bdbb09a3b66c6aca651ce71e62734d8 (diff) | |
parent | 98326c72f732481d73f2941827a1dae75c61388b (diff) | |
download | python-keystoneclient-87dbd7e218430c3d498f5ffa3160a6ac1767ef03.tar.gz |
Merge "Prevent attempts to "filter" list() calls by globally unique IDs"
-rw-r--r-- | keystoneclient/base.py | 11 | ||||
-rw-r--r-- | keystoneclient/tests/unit/v3/test_domains.py | 6 | ||||
-rw-r--r-- | keystoneclient/tests/unit/v3/test_federation.py | 10 | ||||
-rw-r--r-- | keystoneclient/tests/unit/v3/test_role_assignments.py | 9 | ||||
-rw-r--r-- | keystoneclient/tests/unit/v3/utils.py | 14 |
5 files changed, 50 insertions, 0 deletions
diff --git a/keystoneclient/base.py b/keystoneclient/base.py index 025362b..eabbdc4 100644 --- a/keystoneclient/base.py +++ b/keystoneclient/base.py @@ -356,6 +356,17 @@ class CrudManager(Manager): @filter_kwargs def list(self, fallback_to_auth=False, **kwargs): + if 'id' in kwargs.keys(): + # Ensure that users are not trying to call things like + # ``domains.list(id='default')`` when they should have used + # ``[domains.get(domain_id='default')]`` instead. Keystone supports + # ``GET /v3/domains/{domain_id}``, not ``GET + # /v3/domains?id={domain_id}``. + raise TypeError( + _("list() got an unexpected keyword argument 'id'. To " + "retrieve a single object using a globally unique " + "identifier, try using get() instead.")) + url = self.build_url(dict_args_in_out=kwargs) try: diff --git a/keystoneclient/tests/unit/v3/test_domains.py b/keystoneclient/tests/unit/v3/test_domains.py index 9cc23e7..4dbfd73 100644 --- a/keystoneclient/tests/unit/v3/test_domains.py +++ b/keystoneclient/tests/unit/v3/test_domains.py @@ -30,6 +30,12 @@ class DomainTests(utils.TestCase, utils.CrudTests): kwargs.setdefault('name', uuid.uuid4().hex) return kwargs + def test_filter_for_default_domain_by_id(self): + ref = self.new_ref(id='default') + super(DomainTests, self).test_list_by_id( + ref=ref, + id=ref['id']) + def test_list_filter_name(self): super(DomainTests, self).test_list(name='adomain123') diff --git a/keystoneclient/tests/unit/v3/test_federation.py b/keystoneclient/tests/unit/v3/test_federation.py index 4cbae8d..5782aa6 100644 --- a/keystoneclient/tests/unit/v3/test_federation.py +++ b/keystoneclient/tests/unit/v3/test_federation.py @@ -278,6 +278,16 @@ class ProtocolTests(utils.TestCase, utils.CrudTests): for obj, ref_obj in zip(returned, expected): self.assertEqual(obj.to_dict(), ref_obj) + def test_list_by_id(self): + # The test in the parent class needs to be overridden because it + # assumes globally unique IDs, which is not the case with protocol IDs + # (which are contextualized per identity provider). + ref = self.new_ref() + super(ProtocolTests, self).test_list_by_id( + ref=ref, + identity_provider=ref['identity_provider'], + id=ref['id']) + def test_list_params(self): request_args = self.new_ref() filter_kwargs = {uuid.uuid4().hex: uuid.uuid4().hex} diff --git a/keystoneclient/tests/unit/v3/test_role_assignments.py b/keystoneclient/tests/unit/v3/test_role_assignments.py index 79d2585..e77bdcc 100644 --- a/keystoneclient/tests/unit/v3/test_role_assignments.py +++ b/keystoneclient/tests/unit/v3/test_role_assignments.py @@ -71,6 +71,15 @@ class RoleAssignmentsTests(utils.TestCase, utils.CrudTests): self.assertEqual(len(ref_list), len(returned_list)) [self.assertIsInstance(r, self.model) for r in returned_list] + def test_list_by_id(self): + # It doesn't make sense to "list role assignments by ID" at all, given + # that they don't have globally unique IDs in the first place. But + # calling RoleAssignmentsManager.list(id=...) should still raise a + # TypeError when given an unexpected keyword argument 'id', so we don't + # actually have to modify the test in the superclass... I just wanted + # to make a note here in case the superclass changes. + super(RoleAssignmentsTests, self).test_list_by_id() + def test_list_params(self): ref_list = self.TEST_USER_PROJECT_LIST self.stub_entity('GET', diff --git a/keystoneclient/tests/unit/v3/utils.py b/keystoneclient/tests/unit/v3/utils.py index 7f2d633..b5fb7ce 100644 --- a/keystoneclient/tests/unit/v3/utils.py +++ b/keystoneclient/tests/unit/v3/utils.py @@ -245,6 +245,20 @@ class CrudTests(object): return expected_path + def test_list_by_id(self, ref=None, **filter_kwargs): + """Test ``entities.list(id=x)`` being rewritten as ``GET /v3/entities/x``. + + This tests an edge case of each manager's list() implementation, to + ensure that it "does the right thing" when users call ``.list()`` + when they should have used ``.get()``. + + """ + if 'id' not in filter_kwargs: + ref = ref or self.new_ref() + filter_kwargs['id'] = ref['id'] + + self.assertRaises(TypeError, self.manager.list, **filter_kwargs) + def test_list(self, ref_list=None, expected_path=None, expected_query=None, **filter_kwargs): ref_list = ref_list or [self.new_ref(), self.new_ref()] |