openstack
/
keystone
Code Issues Proposed changes

Use the driver to get limits 

@response_truncated was used to set the limit of returned entries. It
asked the driver about the limit and set it to hints. With
domain-specific configs, there are multiple driver instances and each of
them carries domain-specific config.

However, with domain-specific configs, the driver is not yet configured
at that point, because sometimes the manager needs to perform additional
actions in order to understand what domain it works with. Because of
that, @response_truncated always got the limit from the default driver,
not from the one actually used for the domain.

Move the logic of setting the limit from the decorator to a private
method, call it after determining the domain and driver.

Change-Id: I1748d491b047e33712380da731c272f9d471ec0a
Closes-Bug: 1495669
This commit is contained in:
Boris Bobrov 2016-01-13 18:26:51 +03:00
parent d7a9018522
commit 7b6364a4f8
5 changed files with 81 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
keystone/identity/core.py
View File

@ -789,6 +789,41 @@ class Manager(manager.Manager):
not hints.get_exact_filter_by_name('domain_id')):
hints.add_filter('domain_id', domain_id)
def _set_list_limit_in_hints(self, hints, driver):
"""Set list limit in hints from driver
If a hints list is provided, the wrapper will insert the relevant
limit into the hints so that the underlying driver call can try and
honor it. If the driver does truncate the response, it will update the
'truncated' attribute in the 'limit' entry in the hints list, which
enables the caller of this function to know if truncation has taken
place. If, however, the driver layer is unable to perform truncation,
the 'limit' entry is simply left in the hints list for the caller to
handle.
A _get_list_limit() method is required to be present in the object
class hierarchy, which returns the limit for this backend to which
we will truncate.
If a hints list is not provided in the arguments of the wrapped call
then any limits set in the config file are ignored. This allows
internal use of such wrapped methods where the entire data set is
needed as input for the calculations of some other API (e.g. get role
assignments for a given project).
This method, specific to identity manager, is used instead of more
general response_truncated, because the limit for identity entities
can be overriden in domain-specific config files. The driver to use
is determined during processing of the passed parameters and
response_truncated is designed to set the limit before any processing.
"""
if hints is None:
return
list_limit = driver._get_list_limit()
if list_limit:
hints.set_limit(list_limit)
# The actual driver calls - these are pre/post processed here as
# part of the Manager layer to make sure we:
#
@ -859,11 +894,11 @@ class Manager(manager.Manager):
return self._set_domain_id_and_mapping(
ref, domain_id, driver, mapping.EntityType.USER)
@manager.response_truncated
@domains_configured
@exception_translated('user')
def list_users(self, domain_scope=None, hints=None):
driver = self._select_identity_driver(domain_scope)
self._set_list_limit_in_hints(hints, driver)
hints = hints or driver_hints.Hints()
if driver.is_domain_aware():
# Force the domain_scope into the hint to ensure that we only get
@ -1082,12 +1117,12 @@ class Manager(manager.Manager):
"""
pass
@manager.response_truncated
@domains_configured
@exception_translated('user')
def list_groups_for_user(self, user_id, hints=None):
domain_id, driver, entity_id = (
self._get_domain_driver_and_entity_id(user_id))
self._set_list_limit_in_hints(hints, driver)
hints = hints or driver_hints.Hints()
if not driver.is_domain_aware():
# We are effectively satisfying any domain_id filter by the above
@ -1097,11 +1132,11 @@ class Manager(manager.Manager):
return self._set_domain_id_and_mapping(
ref_list, domain_id, driver, mapping.EntityType.GROUP)
@manager.response_truncated
@domains_configured
@exception_translated('group')
def list_groups(self, domain_scope=None, hints=None):
driver = self._select_identity_driver(domain_scope)
self._set_list_limit_in_hints(hints, driver)
hints = hints or driver_hints.Hints()
if driver.is_domain_aware():
# Force the domain_scope into the hint to ensure that we only get
@ -1115,12 +1150,12 @@ class Manager(manager.Manager):
return self._set_domain_id_and_mapping(
ref_list, domain_scope, driver, mapping.EntityType.GROUP)
@manager.response_truncated
@domains_configured
@exception_translated('group')
def list_users_in_group(self, group_id, hints=None):
domain_id, driver, entity_id = (
self._get_domain_driver_and_entity_id(group_id))
self._set_list_limit_in_hints(hints, driver)
hints = hints or driver_hints.Hints()
if not driver.is_domain_aware():
# We are effectively satisfying any domain_id filter by the above

2
keystone/tests/unit/config_files/domain_configs_multi_ldap/keystone.Default.conf
View File

@ -11,4 +11,4 @@ password = password
suffix = cn=example,cn=com
[identity]
driver = ldap
driver = ldap

3
keystone/tests/unit/config_files/domain_configs_multi_ldap/keystone.domain1.conf
View File

@ -8,4 +8,5 @@ password = password
suffix = cn=example,cn=com
[identity]
driver = ldap
driver = ldap
list_limit = 101

40
keystone/tests/unit/test_backend_ldap.py
View File

@ -28,6 +28,7 @@ from six.moves import range
from testtools import matchers
from keystone.common import cache
from keystone.common import driver_hints
from keystone.common import ldap as common_ldap
from keystone.common.ldap import core as common_ldap_core
from keystone import exception
@ -2353,7 +2354,8 @@ class MultiLDAPandSQLIdentity(BaseLDAPIdentity, unit.SQLDriverOverrides,
"""
self.config_fixture.config(
group='identity', domain_specific_drivers_enabled=True,
domain_config_dir=unit.TESTCONF + '/domain_configs_multi_ldap')
domain_config_dir=unit.TESTCONF + '/domain_configs_multi_ldap',
list_limit=1000)
self.config_fixture.config(group='identity_mapping',
backward_compatible_ids=False)
@ -2378,6 +2380,36 @@ class MultiLDAPandSQLIdentity(BaseLDAPIdentity, unit.SQLDriverOverrides,
self.assertNotIn('password', user_ref)
self.assertEqual(expected_user_ids, user_ids)
@mock.patch.object(common_ldap_core.BaseLdap, '_ldap_get_all')
def test_list_limit_domain_specific_inheritance(self, ldap_get_all):
# passiging hints is important, because if it's not passed, limiting
# is considered be disabled
hints = driver_hints.Hints()
self.identity_api.list_users(
domain_scope=self.domains['domain2']['id'],
hints=hints)
# since list_limit is not specified in keystone.domain2.conf, it should
# take the default, which is 1000
self.assertTrue(ldap_get_all.called)
args, kwargs = ldap_get_all.call_args
hints = args[0]
self.assertEqual(1000, hints.limit['limit'])
@mock.patch.object(common_ldap_core.BaseLdap, '_ldap_get_all')
def test_list_limit_domain_specific_override(self, ldap_get_all):
# passiging hints is important, because if it's not passed, limiting
# is considered to be disabled
hints = driver_hints.Hints()
self.identity_api.list_users(
domain_scope=self.domains['domain1']['id'],
hints=hints)
# this should have the list_limit set in Keystone.domain1.conf, which
# is 101
self.assertTrue(ldap_get_all.called)
args, kwargs = ldap_get_all.call_args
hints = args[0]
self.assertEqual(101, hints.limit['limit'])
def test_domain_segregation(self):
"""Test that separate configs have segregated the domain.
@ -2602,7 +2634,8 @@ class MultiLDAPandSQLIdentityDomainConfigsInSQL(MultiLDAPandSQLIdentity):
'user': 'cn=Admin',
'password': 'password',
'suffix': 'cn=example,cn=com'},
'identity': {'driver': 'ldap'}
'identity': {'driver': 'ldap',
'list_limit': '101'}
}
domain2_config = {
'ldap': {'url': 'fake://memory',
@ -2623,7 +2656,8 @@ class MultiLDAPandSQLIdentityDomainConfigsInSQL(MultiLDAPandSQLIdentity):
self.config_fixture.config(
group='identity', domain_specific_drivers_enabled=True,
domain_configurations_from_database=True)
domain_configurations_from_database=True,
list_limit=1000)
self.config_fixture.config(group='identity_mapping',
backward_compatible_ids=False)

3
keystone/tests/unit/test_cli.py
View File

@ -223,7 +223,8 @@ class CliDomainConfigAllTestCase(unit.SQLDriverOverrides, unit.TestCase):
'user': 'cn=Admin',
'password': 'password',
'suffix': 'cn=example,cn=com'},
'identity': {'driver': 'ldap'}
'identity': {'driver': 'ldap',
'list_limit': '101'}
}
domain2_config = {
'ldap': {'url': 'fake://memory',