openstack
/
keystone
Code Issues Proposed changes

Do not name variables as builtins 

According to HACKING.rst [1] we should not name anything as a builtin.

[1] https://github.com/openstack/keystone/blob/master/HACKING.rst#general

Change-Id: I0f34b252ea395b6c87e6738f334fbae9e194134b
This commit is contained in:
Alvaro Lopez Garcia 2013-10-21 16:57:13 +02:00
parent 2df1b7cba4
commit 269dd15cea
15 changed files with 106 additions and 103 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
keystone/assignment/backends/ldap.py
View File

@ -183,7 +183,7 @@ class Assignment(assignment.Driver):
def delete_project(self, tenant_id):
if self.project.subtree_delete_enabled:
self.project.deleteTree(id)
self.project.deleteTree(tenant_id)
else:
tenant_dn = self.project._id_to_dn(tenant_id)
self.role.roles_delete_subtree_by_project(tenant_dn)
@ -245,7 +245,7 @@ class Assignment(assignment.Driver):
# role support which will be added under bug 1101287
query = '(objectClass=%s)' % self.group.object_class
dn = None
dn = self.group._id_to_dn(id)
dn = self.group._id_to_dn(group_id)
if dn:
try:
conn = self.group.get_connection()
@ -371,9 +371,9 @@ class ProjectApi(common_ldap.EnabledEmuMixIn, common_ldap.BaseLdap):
res.add(rolegrant.user_dn)
return list(res)
def update(self, id, values):
old_obj = self.get(id)
return super(ProjectApi, self).update(id, values, old_obj)
def update(self, project_id, values):
old_obj = self.get(project_id)
return super(ProjectApi, self).update(project_id, values, old_obj)
class UserRoleAssociation(object):
@ -413,8 +413,8 @@ class RoleApi(common_ldap.BaseLdap):
self.member_attribute = (getattr(conf.ldap, 'role_member_attribute')
or self.DEFAULT_MEMBER_ATTRIBUTE)
def get(self, id, filter=None):
model = super(RoleApi, self).get(id, filter)
def get(self, role_id, role_filter=None):
model = super(RoleApi, self).get(role_id, role_filter)
return model
def create(self, values):
@ -553,10 +553,10 @@ class RoleApi(common_ldap.BaseLdap):
pass
return super(RoleApi, self).update(role_id, role)
def delete(self, id, tenant_dn):
def delete(self, role_id, tenant_dn):
conn = self.get_connection()
query = '(&(objectClass=%s)(%s=%s))' % (self.object_class,
self.id_attr, id)
self.id_attr, role_id)
try:
for role_dn, _ in conn.search_s(tenant_dn,
ldap.SCOPE_SUBTREE,
@ -566,4 +566,4 @@ class RoleApi(common_ldap.BaseLdap):
pass
finally:
conn.unbind_s()
super(RoleApi, self).delete(id)
super(RoleApi, self).delete(role_id)

18
keystone/assignment/backends/sql.py
View File

@ -387,7 +387,7 @@ class Assignment(sql.Base, assignment.Driver):
return assignment_list
# CRUD
@sql.handle_conflicts(type='project')
@sql.handle_conflicts(conflict_type='project')
def create_project(self, tenant_id, tenant):
tenant['name'] = clean.project_name(tenant['name'])
session = self.get_session()
@ -397,7 +397,7 @@ class Assignment(sql.Base, assignment.Driver):
session.flush()
return tenant_ref.to_dict()
@sql.handle_conflicts(type='project')
@sql.handle_conflicts(conflict_type='project')
def update_project(self, tenant_id, tenant):
session = self.get_session()
@ -417,7 +417,7 @@ class Assignment(sql.Base, assignment.Driver):
session.flush()
return tenant_ref.to_dict(include_extra_dict=True)
@sql.handle_conflicts(type='project')
@sql.handle_conflicts(conflict_type='project')
def delete_project(self, tenant_id):
session = self.get_session()
@ -439,7 +439,7 @@ class Assignment(sql.Base, assignment.Driver):
session.delete(tenant_ref)
session.flush()
@sql.handle_conflicts(type='metadata')
@sql.handle_conflicts(conflict_type='metadata')
def _create_metadata(self, user_id, tenant_id, metadata,
domain_id=None, group_id=None):
session = self.get_session()
@ -469,7 +469,7 @@ class Assignment(sql.Base, assignment.Driver):
session.flush()
return metadata
@sql.handle_conflicts(type='metadata')
@sql.handle_conflicts(conflict_type='metadata')
def _update_metadata(self, user_id, tenant_id, metadata,
domain_id=None, group_id=None):
session = self.get_session()
@ -501,7 +501,7 @@ class Assignment(sql.Base, assignment.Driver):
# domain crud
@sql.handle_conflicts(type='domain')
@sql.handle_conflicts(conflict_type='domain')
def create_domain(self, domain_id, domain):
session = self.get_session()
with session.begin():
@ -534,7 +534,7 @@ class Assignment(sql.Base, assignment.Driver):
raise exception.DomainNotFound(domain_id=domain_name)
return ref.to_dict()
@sql.handle_conflicts(type='domain')
@sql.handle_conflicts(conflict_type='domain')
def update_domain(self, domain_id, domain):
session = self.get_session()
with session.begin():
@ -559,7 +559,7 @@ class Assignment(sql.Base, assignment.Driver):
# role crud
@sql.handle_conflicts(type='role')
@sql.handle_conflicts(conflict_type='role')
def create_role(self, role_id, role):
session = self.get_session()
with session.begin():
@ -583,7 +583,7 @@ class Assignment(sql.Base, assignment.Driver):
session = self.get_session()
return self._get_role(session, role_id).to_dict()
@sql.handle_conflicts(type='role')
@sql.handle_conflicts(conflict_type='role')
def update_role(self, role_id, role):
session = self.get_session()
with session.begin():

80
keystone/common/ldap/core.py
View File

@ -17,7 +17,7 @@
import os.path
import ldap
from ldap import filter as ldap_filter
import ldap.filter
from keystone.common.ldap import fakeldap
from keystone import exception
@ -156,8 +156,9 @@ class BaseLdap(object):
or self.DEFAULT_EXTRA_ATTR_MAPPING)
self.extra_attr_mapping = self._parse_extra_attrs(attr_mapping)
filter = '%s_filter' % self.options_name
self.filter = getattr(conf.ldap, filter) or self.DEFAULT_FILTER
ldap_filter = '%s_filter' % self.options_name
self.ldap_filter = getattr(conf.ldap,
ldap_filter) or self.DEFAULT_FILTER
allow_create = '%s_allow_create' % self.options_name
self.allow_create = getattr(conf.ldap, allow_create)
@ -234,21 +235,21 @@ class BaseLdap(object):
return conn
def _id_to_dn_string(self, id):
def _id_to_dn_string(self, object_id):
return '%s=%s,%s' % (self.id_attr,
ldap.dn.escape_dn_chars(str(id)),
ldap.dn.escape_dn_chars(str(object_id)),
self.tree_dn)
def _id_to_dn(self, id):
def _id_to_dn(self, object_id):
if self.LDAP_SCOPE == ldap.SCOPE_ONELEVEL:
return self._id_to_dn_string(id)
return self._id_to_dn_string(object_id)
conn = self.get_connection()
try:
search_result = conn.search_s(
self.tree_dn, self.LDAP_SCOPE,
'(&(%(id_attr)s=%(id)s)(objectclass=%(objclass)s))' %
{'id_attr': self.id_attr,
'id': ldap.filter.escape_filter_chars(str(id)),
'id': ldap.filter.escape_filter_chars(str(object_id)),
'objclass': self.object_class})
finally:
conn.unbind_s()
@ -256,7 +257,7 @@ class BaseLdap(object):
dn, attrs = search_result[0]
return dn
else:
return self._id_to_dn_string(id)
return self._id_to_dn_string(object_id)
@staticmethod
def _dn_to_id(dn):
@ -331,14 +332,14 @@ class BaseLdap(object):
conn.unbind_s()
return values
def _ldap_get(self, id, filter=None):
def _ldap_get(self, object_id, ldap_filter=None):
conn = self.get_connection()
query = ('(&(%(id_attr)s=%(id)s)'
'%(filter)s'
'(objectClass=%(object_class)s))'
% {'id_attr': self.id_attr,
'id': ldap.filter.escape_filter_chars(str(id)),
'filter': (filter or self.filter or ''),
'id': ldap.filter.escape_filter_chars(str(object_id)),
'filter': (ldap_filter or self.ldap_filter or ''),
'object_class': self.object_class})
try:
attrs = list(set((self.attribute_mapping.values() +
@ -353,10 +354,11 @@ class BaseLdap(object):
except IndexError:
return None
def _ldap_get_all(self, filter=None):
def _ldap_get_all(self, ldap_filter=None):
conn = self.get_connection()
query = '(&%s(objectClass=%s))' % (filter or self.filter or '',
self.object_class)
query = '(&%s(objectClass=%s))' % (ldap_filter or
self.ldap_filter or
'', self.object_class)
try:
return conn.search_s(self.tree_dn,
self.LDAP_SCOPE,
@ -367,33 +369,33 @@ class BaseLdap(object):
finally:
conn.unbind_s()
def get(self, id, filter=None):
res = self._ldap_get(id, filter)
def get(self, object_id, ldap_filter=None):
res = self._ldap_get(object_id, ldap_filter)
if res is None:
raise self._not_found(id)
raise self._not_found(object_id)
else:
return self._ldap_res_to_model(res)
def get_by_name(self, name, filter=None):
def get_by_name(self, name, ldap_filter=None):
query = ('(%s=%s)' % (self.attribute_mapping['name'],
ldap_filter.escape_filter_chars(name)))
ldap.filter.escape_filter_chars(name)))
res = self.get_all(query)
try:
return res[0]
except IndexError:
raise self._not_found(name)
def get_all(self, filter=None):
def get_all(self, ldap_filter=None):
return [self._ldap_res_to_model(x)
for x in self._ldap_get_all(filter)]
for x in self._ldap_get_all(ldap_filter)]
def update(self, id, values, old_obj=None):
def update(self, object_id, values, old_obj=None):
if not self.allow_update:
action = _('LDAP %s update') % self.options_name
raise exception.ForbiddenAction(action=action)
if old_obj is None:
old_obj = self.get(id)
old_obj = self.get(object_id)
modlist = []
for k, v in values.iteritems():
@ -427,37 +429,37 @@ class BaseLdap(object):
if modlist:
conn = self.get_connection()
try:
conn.modify_s(self._id_to_dn(id), modlist)
conn.modify_s(self._id_to_dn(object_id), modlist)
except ldap.NO_SUCH_OBJECT:
raise self._not_found(id)
raise self._not_found(object_id)
finally:
conn.unbind_s()
return self.get(id)
return self.get(object_id)
def delete(self, id):
def delete(self, object_id):
if not self.allow_delete:
action = _('LDAP %s delete') % self.options_name
raise exception.ForbiddenAction(action=action)
conn = self.get_connection()
try:
conn.delete_s(self._id_to_dn(id))
conn.delete_s(self._id_to_dn(object_id))
except ldap.NO_SUCH_OBJECT:
raise self._not_found(id)
raise self._not_found(object_id)
finally:
conn.unbind_s()
def deleteTree(self, id):
def deleteTree(self, object_id):
conn = self.get_connection()
tree_delete_control = ldap.controls.LDAPControl(CONTROL_TREEDELETE,
0,
None)
try:
conn.delete_ext_s(self._id_to_dn(id),
conn.delete_ext_s(self._id_to_dn(object_id),
serverctrls=[tree_delete_control])
except ldap.NO_SUCH_OBJECT:
raise self._not_found(id)
raise self._not_found(object_id)
finally:
conn.unbind_s()
@ -731,23 +733,23 @@ class EnabledEmuMixIn(BaseLdap):
else:
return super(EnabledEmuMixIn, self).create(values)
def get(self, object_id, filter=None):
ref = super(EnabledEmuMixIn, self).get(object_id, filter)
def get(self, object_id, ldap_filter=None):
ref = super(EnabledEmuMixIn, self).get(object_id, ldap_filter)
if 'enabled' not in self.attribute_ignore and self.enabled_emulation:
ref['enabled'] = self._get_enabled(object_id)
return ref
def get_all(self, filter=None):
def get_all(self, ldap_filter=None):
if 'enabled' not in self.attribute_ignore and self.enabled_emulation:
# had to copy BaseLdap.get_all here to filter by DN
# had to copy BaseLdap.get_all here to ldap_filter by DN
tenant_list = [self._ldap_res_to_model(x)
for x in self._ldap_get_all(filter)
for x in self._ldap_get_all(ldap_filter)
if x[0] != self.enabled_emulation_dn]
for tenant_ref in tenant_list:
tenant_ref['enabled'] = self._get_enabled(tenant_ref['id'])
return tenant_list
else:
return super(EnabledEmuMixIn, self).get_all(filter)
return super(EnabledEmuMixIn, self).get_all(ldap_filter)
def update(self, object_id, values, old_obj=None):
if 'enabled' not in self.attribute_ignore and self.enabled_emulation:

5
keystone/common/sql/core.py
View File

@ -298,7 +298,7 @@ class Base(object):
self._sessionmaker = None
def handle_conflicts(type='object'):
def handle_conflicts(conflict_type='object'):
"""Converts IntegrityError into HTTP 409 Conflict."""
def decorator(method):
@functools.wraps(method)
@ -306,6 +306,7 @@ def handle_conflicts(type='object'):
try:
return method(*args, **kwargs)
except (IntegrityError, OperationalError) as e:
raise exception.Conflict(type=type, details=str(e.orig))
raise exception.Conflict(type=conflict_type,
details=str(e.orig))
return wrapper
return decorator

6
keystone/common/sql/migration.py
View File

@ -80,10 +80,10 @@ def db_version_control(version=None, repo_path=None):
def find_migrate_repo(package=None):
"""Get the path for the migrate repository."""
if package is None:
file = __file__
filename = __file__
else:
file = package.__file__
path = os.path.join(os.path.abspath(os.path.dirname(file)),
filename = package.__file__
path = os.path.join(os.path.abspath(os.path.dirname(filename)),
'migrate_repo')
assert os.path.exists(path)
return path

2
keystone/contrib/endpoint_filter/backends/sql.py
View File

@ -37,7 +37,7 @@ class EndpointFilter(sql.Base):
def db_sync(self, version=None):
migration.db_sync(version=version)
@sql.handle_conflicts(type='project_endpoint')
@sql.handle_conflicts(conflict_type='project_endpoint')
def add_endpoint_to_project(self, endpoint_id, project_id):
session = self.get_session()
with session.begin():

4
keystone/credential/backends/sql.py
View File

@ -39,7 +39,7 @@ class Credential(sql.Base, credential.Driver):
# credential crud
@sql.handle_conflicts(type='credential')
@sql.handle_conflicts(conflict_type='credential')
def create_credential(self, credential_id, credential):
session = self.get_session()
with session.begin():
@ -66,7 +66,7 @@ class Credential(sql.Base, credential.Driver):
session = self.get_session()
return self._get_credential(session, credential_id).to_dict()
@sql.handle_conflicts(type='credential')
@sql.handle_conflicts(conflict_type='credential')
def update_credential(self, credential_id, credential):
session = self.get_session()
with session.begin():

16
keystone/identity/backends/ldap.py
View File

@ -269,16 +269,16 @@ class GroupApi(common_ldap.BaseLdap):
data.pop('description')
return super(GroupApi, self).create(data)
def delete(self, id):
def delete(self, group_id):
if self.subtree_delete_enabled:
super(GroupApi, self).deleteTree(id)
super(GroupApi, self).deleteTree(group_id)
else:
# TODO(spzala): this is only placeholder for group and domain
# role support which will be added under bug 1101287
query = '(objectClass=%s)' % self.object_class
dn = None
dn = self._id_to_dn(id)
dn = self._id_to_dn(group_id)
if dn:
try:
conn = self.get_connection()
@ -290,11 +290,11 @@ class GroupApi(common_ldap.BaseLdap):
pass
finally:
conn.unbind_s()
super(GroupApi, self).delete(id)
super(GroupApi, self).delete(group_id)
def update(self, id, values):
old_obj = self.get(id)
return super(GroupApi, self).update(id, values, old_obj)
def update(self, group_id, values):
old_obj = self.get(group_id)
return super(GroupApi, self).update(group_id, values, old_obj)
def add_user(self, user_dn, group_id, user_id):
conn = self.get_connection()
@ -330,7 +330,7 @@ class GroupApi(common_ldap.BaseLdap):
query = '(&(objectClass=%s)(%s=%s)%s)' % (self.object_class,
self.member_attribute,
user_dn,
self.filter or '')
self.ldap_filter or '')
memberships = self.get_all(query)
return memberships

8
keystone/identity/backends/sql.py
View File

@ -110,7 +110,7 @@ class Identity(sql.Base, identity.Driver):
# user crud
@sql.handle_conflicts(type='user')
@sql.handle_conflicts(conflict_type='user')
def create_user(self, user_id, user):
user = utils.hash_user_password(user)
session = self.get_session()
@ -146,7 +146,7 @@ class Identity(sql.Base, identity.Driver):
raise exception.UserNotFound(user_id=user_name)
return identity.filter_user(user_ref.to_dict())
@sql.handle_conflicts(type='user')
@sql.handle_conflicts(conflict_type='user')
def update_user(self, user_id, user):
session = self.get_session()
if 'id' in user and user_id != user['id']:
@ -237,7 +237,7 @@ class Identity(sql.Base, identity.Driver):
# group crud
@sql.handle_conflicts(type='group')
@sql.handle_conflicts(conflict_type='group')
def create_group(self, group_id, group):
session = self.get_session()
with session.begin():
@ -261,7 +261,7 @@ class Identity(sql.Base, identity.Driver):
session = self.get_session()
return self._get_group(session, group_id).to_dict()
@sql.handle_conflicts(type='group')
@sql.handle_conflicts(conflict_type='group')
def update_group(self, group_id, group):
session = self.get_session()

10
keystone/identity/core.py
View File

@ -121,16 +121,16 @@ class DomainConfigs(dict):
return
for r, d, f in os.walk(conf_dir):
for file in f:
if file.startswith('keystone.') and file.endswith('.conf'):
names = file.split('.')
for fname in f:
if fname.startswith('keystone.') and fname.endswith('.conf'):
names = fname.split('.')
if len(names) == 3:
self._load_config(assignment_api,
[os.path.join(r, file)],
[os.path.join(r, fname)],
names[1])
else:
msg = (_('Ignoring file (%s) while scanning domain '
'config directory') % file)
'config directory') % fname)
LOG.debug(msg)
def get_domain_driver(self, domain_id):

4
keystone/policy/backends/sql.py
View File

@ -34,7 +34,7 @@ class Policy(sql.Base, rules.Policy):
def db_sync(self, version=None):
migration.db_sync(version=version)
@sql.handle_conflicts(type='policy')
@sql.handle_conflicts(conflict_type='policy')
def create_policy(self, policy_id, policy):
session = self.get_session()
@ -63,7 +63,7 @@ class Policy(sql.Base, rules.Policy):
return self._get_policy(session, policy_id).to_dict()
@sql.handle_conflicts(type='policy')
@sql.handle_conflicts(conflict_type='policy')
def update_policy(self, policy_id, policy):
session = self.get_session()

8
keystone/tests/test_backend.py
View File

@ -2934,14 +2934,14 @@ class TokenCacheInvalidation(object):
# Create an equivalent of a scoped token
token_dict = {'user': self.user, 'tenant': self.tenant,
'metadata': {}, 'id': 'placeholder'}
id, data = self.token_provider_api.issue_v2_token(token_dict)
self.scoped_token_id = id
token_id, data = self.token_provider_api.issue_v2_token(token_dict)
self.scoped_token_id = token_id
# ..and an un-scoped one
token_dict = {'user': self.user, 'tenant': None,
'metadata': {}, 'id': 'placeholder'}
id, data = self.token_provider_api.issue_v2_token(token_dict)
self.unscoped_token_id = id
token_id, data = self.token_provider_api.issue_v2_token(token_dict)
self.unscoped_token_id = token_id
# Validate them, in the various ways possible - this will load the
# responses into the token cache.

4
keystone/tests/test_backend_pam.py
View File

@ -32,8 +32,8 @@ class PamIdentity(tests.TestCase):
tests.testsdir('test_overrides.conf'),
tests.testsdir('backend_pam.conf')])
self.identity_api = identity_pam.PamIdentity()
id = uuid.uuid4().hex
self.tenant_in = {'id': id, 'name': id}
tenant_id = uuid.uuid4().hex
self.tenant_in = {'id': tenant_id, 'name': tenant_id}
self.user_in = {'id': CONF.pam.userid, 'name': CONF.pam.userid}
def test_get_project(self):

12
keystone/tests/test_v3_auth.py
View File

@ -1070,9 +1070,9 @@ class TestTokenRevoking(test_v3.RestfulTestCase):
class TestAuthExternalDisabled(test_v3.RestfulTestCase):
def config_files(self):
list = self._config_file_list[:]
list.append(tests.testsdir('auth_plugin_external_disabled.conf'))
return list
cfg_list = self._config_file_list[:]
cfg_list.append(tests.testsdir('auth_plugin_external_disabled.conf'))
return cfg_list
def test_remote_user_disabled(self):
auth_data = self.build_authentication_request()['auth']
@ -1092,9 +1092,9 @@ class TestAuthExternalDomain(test_v3.RestfulTestCase):
content_type = 'json'
def config_files(self):
list = self._config_file_list[:]
list.append(tests.testsdir('auth_plugin_external_domain.conf'))
return list
cfg_list = self._config_file_list[:]
cfg_list.append(tests.testsdir('auth_plugin_external_domain.conf'))
return cfg_list
def test_remote_user_with_realm(self):
auth_data = self.build_authentication_request()['auth']

12
keystone/trust/backends/sql.py
View File

@ -44,7 +44,7 @@ class TrustRole(sql.ModelBase):
class Trust(sql.Base, trust.Driver):
@sql.handle_conflicts(type='trust')
@sql.handle_conflicts(conflict_type='trust')
def create_trust(self, trust_id, trust, roles):
session = self.get_session()
with session.begin():
@ -71,7 +71,7 @@ class Trust(sql.Base, trust.Driver):
roles.append({'id': role.role_id})
trust_dict['roles'] = roles
@sql.handle_conflicts(type='trust')
@sql.handle_conflicts(conflict_type='trust')
def get_trust(self, trust_id):
session = self.get_session()
ref = (session.query(TrustModel).
@ -88,13 +88,13 @@ class Trust(sql.Base, trust.Driver):
self._add_roles(trust_id, session, trust_dict)
return trust_dict
@sql.handle_conflicts(type='trust')
@sql.handle_conflicts(conflict_type='trust')
def list_trusts(self):
session = self.get_session()
trusts = session.query(TrustModel).filter_by(deleted_at=None)
return [trust_ref.to_dict() for trust_ref in trusts]
@sql.handle_conflicts(type='trust')
@sql.handle_conflicts(conflict_type='trust')
def list_trusts_for_trustee(self, trustee_user_id):
session = self.get_session()
trusts = (session.query(TrustModel).
@ -102,7 +102,7 @@ class Trust(sql.Base, trust.Driver):
filter_by(trustee_user_id=trustee_user_id))
return [trust_ref.to_dict() for trust_ref in trusts]
@sql.handle_conflicts(type='trust')
@sql.handle_conflicts(conflict_type='trust')
def list_trusts_for_trustor(self, trustor_user_id):
session = self.get_session()
trusts = (session.query(TrustModel).
@ -110,7 +110,7 @@ class Trust(sql.Base, trust.Driver):
filter_by(trustor_user_id=trustor_user_id))
return [trust_ref.to_dict() for trust_ref in trusts]
@sql.handle_conflicts(type='trust')
@sql.handle_conflicts(conflict_type='trust')
def delete_trust(self, trust_id):
session = self.get_session()
with session.begin():