Merge lp:~rlane/nova/ldap-schema-modifications-1 into lp:~hudson-openstack/nova/trunk

[Ryan Lane]

Simplifies and improves ldap schema.

[Vish Ishaya (vishvananda)]

Suggestion:

Can we have the isAdmin/isNovaAdmin attribute as a flag that defaults to isNovaAdmin. I'm thinking that it would be nice if we don't force an ldap data migration if users have the old schema.

On Dec 3, 2010, at 9:47 AM, Ryan Lane wrote:

> Ryan Lane has proposed merging lp:~rlane/nova/ldap-schema-modifications-1 into lp:nova.
>
> Requested reviews:
> Nova Core (nova-core)
> Related bugs:
> #681030 Nova's LDAP schema has an unneeded requirement on the nis or bis schema
> https://bugs.launchpad.net/bugs/681030
>
> --
> https://code.launchpad.net/~rlane/nova/ldap-schema-modifications-1/+merge/42662
> Your team Nova Core is requested to review the proposed merge of lp:~rlane/nova/ldap-schema-modifications-1 into lp:nova.
> === modified file 'nova/auth/ldapdriver.py'
> --- nova/auth/ldapdriver.py 2010-11-26 17:01:50 +0000
> +++ nova/auth/ldapdriver.py 2010-12-03 17:46:55 +0000
> @@ -37,6 +37,8 @@
> flags.DEFINE_string('ldap_password', 'changeme', 'LDAP password')
> flags.DEFINE_string('ldap_user_dn', 'cn=Manager,dc=example,dc=com',
> 'DN of admin user')
> +flags.DEFINE_string('ldap_user_id_attribute', 'uid', 'Attribute to use as id')
> +flags.DEFINE_string('ldap_user_name_attribute', 'cn', 'Attribute to use as name')
> flags.DEFINE_string('ldap_user_unit', 'Users', 'OID for Users')
> flags.DEFINE_string('ldap_user_subtree', 'ou=Users,dc=example,dc=com',
> 'OU for Users')
> @@ -131,12 +133,12 @@
> 'inetOrgPerson',
> 'novaUser']),
> ('ou', [FLAGS.ldap_user_unit]),
> - ('uid', [name]),
> + (FLAGS.ldap_user_id_attribute, [name]),
> ('sn', [name]),
> - ('cn', [name]),
> + (FLAGS.ldap_user_name_attribute, [name]),
> ('secretKey', [secret_key]),
> ('accessKey', [access_key]),
> - ('isAdmin', [str(is_admin).upper()]),
> + ('isNovaAdmin', [str(is_admin).upper()]),
> ]
> self.conn.add_s(self.__uid_to_dn(name), attr)
> return self.__to_user(dict(attr))
> @@ -274,7 +276,7 @@
> if secret_key:
> attr.append((self.ldap.MOD_REPLACE, 'secretKey', secret_key))
> if admin is not None:
> - attr.append((self.ldap.MOD_REPLACE, 'isAdmin', str(admin).upper()))
> + attr.append((self.ldap.MOD_REPLACE, 'isNovaAdmin', str(admin).upper()))
> self.conn.modify_s(self.__uid_to_dn(uid), attr)
>
> def __user_exists(self, uid):
> @@ -450,11 +452,11 @@
> if attr == None:
> return None
> return {
> - 'id': attr['uid'][0],
> - 'name': attr['cn'][0],
> + 'id': attr[FLAGS.ldap_user_id_attribute][0],
> + 'name': attr[FLAGS.ldap_user_name_attribute][0],
> 'access': attr['accessKey'][0],
> 'secret': attr['secretKey'][0],
> - 'admin': (attr['isAdmin'][0] == 'TRUE')}
> + 'admin': (attr['isNovaAdmin'][0] == 'TRUE')}
>
> def __to_project(self, attr):
> """Convert ldap attributes to Project object"""
> @@ -474,9 +476,10 @@
> return dn.split(','...

[Ryan Lane (rlane)]

After thinking about it more, it would be good to get rid of the isAdmin attribute altogether and use a group for this. This will cause more queries, but I can reduce that for servers that support memberOf.

This is a more standard way of handling this in LDAP.

[Ryan Lane (rlane)]

I've found a further improvement to be made, that should likely be added before merging this...

The groupOfNames objectclass has an "owner" attribute that expects a DN. Using this we can eliminate the novaProject objectclass and attributes.

Overall I think the schema can be reduced to the novaUser objectclass, and the accessKey and secretKey attributes.

[Vish Ishaya (vishvananda)]

Yes, I had considered the same thing (owner and group instead of specific project class). It will require refining the search for roles to make sure that it doesn't pick up projects by accident.

isAdmin could be moved to a group as well. But I think it needs to appear to external services as an attribute of the user. The purpose of isAdmin is to completely bypass role checking.

Vish

On Dec 3, 2010, at 1:25 PM, Ryan Lane wrote:

> I've found a further improvement to be made, that should likely be added before merging this...
>
> The groupOfNames objectclass has an "owner" attribute that expects a DN. Using this we can eliminate the novaProject objectclass and attributes.
>
> Overall I think the schema can be reduced to the novaUser objectclass, and the accessKey and secretKey attributes.
> --
> https://code.launchpad.net/~rlane/nova/ldap-schema-modifications-1/+merge/42662
> Your team Nova Core is requested to review the proposed merge of lp:~rlane/nova/ldap-schema-modifications-1 into lp:nova.

[Ryan Lane (rlane)]

All projects should have an owner, and roles shouldn't. We can refine the search for projects to include owner in the query.

[Ryan Lane (rlane)]

Alternatively, we could require that roles and projects should be placed in separate OUs.

[Ryan Lane (rlane)]

Made suggested schema selection change. This branch now also removes the novaProject part of the schema. Only novaUser remains.

[Vish Ishaya (vishvananda)]

Minor nits.
Can you align the second line with the parens for multiline code?
                    attr.append((self.ldap.MOD_REPLACE, 'accessKey',
                    [access_key]))
becomes:
                    attr.append((self.ldap.MOD_REPLACE, 'accessKey',
                                 [access_key]))

makes it a little easier to read.

also would you mind changing <email address hidden> > <email address hidden> ?

Finally, is there a reason why you made class variables for the params
LdapDriver.project_pattern vs. self.project_pattern ?
It seems a little odd to me to change the class variables in __init__ which is the ctor for the instance of the class. Not sure if there is a prettier pattern, but perhaps if version 2 ... elif version 1 ... and using self.project_pattern would be a little nicer?

review: Needs Information

[Ryan Lane (rlane)]

I originally tried setting object variables, but it caused a bunch of tests to fail because the fake ldap driver couldn't access them. Also, one variable is used in a static method. Using class variables eliminated those problems. If there's a better way, I'd love to use it; I used this method because I couldn't get others to work.

Also just pushed the other requested fixes.

[Vish Ishaya (vishvananda)]

Understandable. LGTM

review: Approve

[Devin Carlen (devcamcar)]

Nice work, this is much cleaner.

review: Approve

[OpenStack Infra (hudson-openstack)]

Attempt to merge into lp:nova failed due to conflicts:

text conflict in nova/auth/ldapdriver.py

[Vish Ishaya (vishvananda)]

this needs a quick trunk merge and repush.

[Ryan Lane (rlane)]

Merged with trunk and resolved conflict; repushed.

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~rlane/nova/ldap-schema-modifications-1 into lp:nova failed. Below is the output from the failed tests.

nova.tests.access_unittest
  AccessTestCase
    test_001_allow_all ... [OK]
    test_002_allow_none ... [OK]
    test_003_allow_project_manager ... [OK]
    test_004_allow_sys_and_net ... [OK]
nova.tests.api_unittest
  ApiEc2TestCase
    test_authorize_revoke_security_group_cidr ... [OK]
    test_authorize_revoke_security_group_foreign_group ... [OK]
    test_create_delete_security_group ... [OK]
    test_describe_instances ... [OK]
    test_get_all_key_pairs ... [OK]
    test_get_all_security_groups ... [OK]
  XmlConversionTestCase
    test_number_conversion ... [OK]
nova.tests.auth_unittest
  AuthManagerDbTestCase
    test_004_signature_is_valid ... [OK]
    test_005_can_get_credentials ... [OK]
    test_add_user_role_doesnt_infect_project_roles ... [OK]
    test_adding_role_to_project_is_ignored_unless_added_to_user ... [OK]
    test_can_add_and_remove_user_role ... [OK]
    test_can_add_remove_user_with_role ... [OK]
    test_can_add_user_to_project ... [OK]
    test_can_create_and_get_project ... [OK]
    test_can_create_and_get_project_with_attributes ... [OK]
    test_can_create_project_with_manager ... [OK]
    test_can_delete_project ... [OK]
    test_can_delete_user ... [OK]
    test_can_generate_x509 ... [OK]
    test_can_list_project_roles ... [OK]
    test_can_list_projects ... [OK]
    test_can_list_user_roles ... [OK]
    test_can_list_users ... [OK]
    test_can_modify_project ... [OK]
    test_can_modify_users ... [OK]
    test_can_remove_project_role_but_keep_user_role ... [OK]
    test_can_remove_user_from_project ... [OK]
    test_can_remove_user_roles ... [OK]
    test_can_retrieve_project_by_user ... [OK]
    test_create_and_find_user ... [OK]
    test_create_and_find_with_properties ... [OK]
    test_create_project_assigns_m...

[Vish Ishaya (vishvananda)]

This looks like a bunk failure, reapproving.

[Vish Ishaya (vishvananda)]

something odd is happening with tarmac, trying a reapprove first to get it building properly, then we'll try again.

[Vish Ishaya (vishvananda)]

yes tarmac is complaining about some odd bzrlib error. We'll have to wait for packaging gurus to figure out what is going on.

[Vish Ishaya (vishvananda)]

I'm also noticing that you aren't in Authors or .mailmap correctly. It also seems that you haven't signed the CLA, which might be causing the merge to fail.

[Ryan Lane (rlane32)]

That's odd.

<email address hidden>

Sent from my iPhone

On Dec 22, 2010, at 6:41 PM, Vish Ishaya <email address hidden> wrote:

> yes tarmac is complaining about some odd bzrlib error. We'll have to wait for packaging gurus to figure out what is going on.
> --
> https://code.launchpad.net/~rlane/nova/ldap-schema-modifications-1/+merge/42662
> You are the owner of lp:~rlane/nova/ldap-schema-modifications-1.

[Ryan Lane (rlane32)]

I did sign the cla... I can do so again if needed.

Sent from my iPhone

On Dec 22, 2010, at 7:47 PM, Vish Ishaya <email address hidden> wrote:

> I'm also noticing that you aren't in Authors or .mailmap correctly. It also seems that you haven't signed the CLA, which might be causing the merge to fail.
> --
> https://code.launchpad.net/~rlane/nova/ldap-schema-modifications-1/+merge/42662
> You are the owner of lp:~rlane/nova/ldap-schema-modifications-1.

[Vish Ishaya (vishvananda)]

If you signed it, you probably just need to add your info to this page:

http://wiki.openstack.org/Contributors

On Dec 22, 2010, at 5:53 PM, Ryan Lane wrote:

> I did sign the cla... I can do so again if needed.
>
> Sent from my iPhone
>
> On Dec 22, 2010, at 7:47 PM, Vish Ishaya <email address hidden> wrote:
>
>> I'm also noticing that you aren't in Authors or .mailmap correctly. It also seems that you haven't signed the CLA, which might be causing the merge to fail.
>> --
>> https://code.launchpad.net/~rlane/nova/ldap-schema-modifications-1/+merge/42662
>> You are the owner of lp:~rlane/nova/ldap-schema-modifications-1.
> --
> https://code.launchpad.net/~rlane/nova/ldap-schema-modifications-1/+merge/42662
> You are reviewing the proposed merge of lp:~rlane/nova/ldap-schema-modifications-1 into lp:nova.

[Vish Ishaya (vishvananda)]

It was the lack of a commit message that was causing it to fail. Trying to merge again.

=== modified file 'nova/auth/fakeldap.py'
--- nova/auth/fakeldap.py	2010-12-22 20:59:53 +0000
+++ nova/auth/fakeldap.py	2010-12-22 23:56:00 +0000
@@ -150,6 +150,9 @@
     """Match a given key and value against an attribute list."""
     if key not in attrs:
         return False
+    # This is a wild card search. Implemented as all or nothing for now.
+    if value == "*":
+        return True
     if key != "objectclass":
         return value in attrs[key]
     # it is an objectclass check, so check subclasses

=== modified file 'nova/auth/ldapdriver.py'
--- nova/auth/ldapdriver.py	2010-12-22 20:59:53 +0000
+++ nova/auth/ldapdriver.py	2010-12-22 23:56:00 +0000
@@ -32,11 +32,16 @@
 
 
 FLAGS = flags.FLAGS
+flags.DEFINE_integer('ldap_schema_version', 2,
+                     'Current version of the LDAP schema')
 flags.DEFINE_string('ldap_url', 'ldap://localhost',
                     'Point this at your ldap server')
 flags.DEFINE_string('ldap_password', 'changeme', 'LDAP password')
 flags.DEFINE_string('ldap_user_dn', 'cn=Manager,dc=example,dc=com',
                     'DN of admin user')
+flags.DEFINE_string('ldap_user_id_attribute', 'uid', 'Attribute to use as id')
+flags.DEFINE_string('ldap_user_name_attribute', 'cn',
+                    'Attribute to use as name')
 flags.DEFINE_string('ldap_user_unit', 'Users', 'OID for Users')
 flags.DEFINE_string('ldap_user_subtree', 'ou=Users,dc=example,dc=com',
                     'OU for Users')
@@ -73,10 +78,20 @@
     Defines enter and exit and therefore supports the with/as syntax.
     """
 
+    project_pattern = '(owner=*)'
+    isadmin_attribute = 'isNovaAdmin'
+    project_attribute = 'owner'
+    project_objectclass = 'groupOfNames'
+
     def __init__(self):
         """Imports the LDAP module"""
         self.ldap = __import__('ldap')
         self.conn = None
+        if FLAGS.ldap_schema_version == 1:
+            LdapDriver.project_pattern = '(objectclass=novaProject)'
+            LdapDriver.isadmin_attribute = 'isAdmin'
+            LdapDriver.project_attribute = 'projectManager'
+            LdapDriver.project_objectclass = 'novaProject'
 
     def __enter__(self):
         """Creates the connection to LDAP"""
@@ -104,13 +119,13 @@
         """Retrieve project by id"""
         dn = 'cn=%s,%s' % (pid,
                            FLAGS.ldap_project_subtree)
-        attr = self.__find_object(dn, '(objectclass=novaProject)')
+        attr = self.__find_object(dn, LdapDriver.project_pattern)
         return self.__to_project(attr)
 
     def get_users(self):
         """Retrieve list of users"""
         attrs = self.__find_objects(FLAGS.ldap_user_subtree,
-                                  '(objectclass=novaUser)')
+                                    '(objectclass=novaUser)')
         users = []
         for attr in attrs:
             user = self.__to_user(attr)
@@ -120,7 +135,7 @@
 
     def get_projects(self, uid=None):
         """Retrieve list of projects"""
-        pattern = '(objectclass=novaProject)'
+        pattern = LdapDriver.project_pattern
         if uid:
             pattern = "(&%s(member=%s))" % (pattern, self.__uid_to_dn(uid))
         attrs = self.__find_objects(FLAGS.ldap_project_subtree,
@@ -139,23 +154,25 @@
                 # Malformed entries are useless, replace attributes found.
                 attr = []
                 if 'secretKey' in user.keys():
-                    attr.append((self.ldap.MOD_REPLACE, 'secretKey', \
-                    [secret_key]))
+                    attr.append((self.ldap.MOD_REPLACE, 'secretKey',
+                                 [secret_key]))
                 else:
-                    attr.append((self.ldap.MOD_ADD, 'secretKey', \
-                    [secret_key]))
+                    attr.append((self.ldap.MOD_ADD, 'secretKey',
+                                 [secret_key]))
                 if 'accessKey' in user.keys():
-                    attr.append((self.ldap.MOD_REPLACE, 'accessKey', \
-                    [access_key]))
-                else:
-                    attr.append((self.ldap.MOD_ADD, 'accessKey', \
-                    [access_key]))
-                if 'isAdmin' in user.keys():
-                    attr.append((self.ldap.MOD_REPLACE, 'isAdmin', \
-                    [str(is_admin).upper()]))
-                else:
-                    attr.append((self.ldap.MOD_ADD, 'isAdmin', \
-                    [str(is_admin).upper()]))
+                    attr.append((self.ldap.MOD_REPLACE, 'accessKey',
+                                 [access_key]))
+                else:
+                    attr.append((self.ldap.MOD_ADD, 'accessKey',
+                                 [access_key]))
+                if LdapDriver.isadmin_attribute in user.keys():
+                    attr.append((self.ldap.MOD_REPLACE,
+                                 LdapDriver.isadmin_attribute,
+                                 [str(is_admin).upper()]))
+                else:
+                    attr.append((self.ldap.MOD_ADD,
+                                 LdapDriver.isadmin_attribute,
+                                 [str(is_admin).upper()]))
                 self.conn.modify_s(self.__uid_to_dn(name), attr)
                 return self.get_user(name)
             else:
@@ -168,12 +185,12 @@
                                  'inetOrgPerson',
                                  'novaUser']),
                 ('ou', [FLAGS.ldap_user_unit]),
-                ('uid', [name]),
+                (FLAGS.ldap_user_id_attribute, [name]),
                 ('sn', [name]),
-                ('cn', [name]),
+                (FLAGS.ldap_user_name_attribute, [name]),
                 ('secretKey', [secret_key]),
                 ('accessKey', [access_key]),
-                ('isAdmin', [str(is_admin).upper()]),
+                (LdapDriver.isadmin_attribute, [str(is_admin).upper()]),
             ]
             self.conn.add_s(self.__uid_to_dn(name), attr)
             return self.__to_user(dict(attr))
@@ -204,10 +221,10 @@
         if not manager_dn in members:
             members.append(manager_dn)
         attr = [
-            ('objectclass', ['novaProject']),
+            ('objectclass', [LdapDriver.project_objectclass]),
             ('cn', [name]),
             ('description', [description]),
-            ('projectManager', [manager_dn]),
+            (LdapDriver.project_attribute, [manager_dn]),
             ('member', members)]
         self.conn.add_s('cn=%s,%s' % (name, FLAGS.ldap_project_subtree), attr)
         return self.__to_project(dict(attr))
@@ -223,7 +240,8 @@
                                            "manager %s doesn't exist")
                                          % manager_uid)
             manager_dn = self.__uid_to_dn(manager_uid)
-            attr.append((self.ldap.MOD_REPLACE, 'projectManager', manager_dn))
+            attr.append((self.ldap.MOD_REPLACE, LdapDriver.project_attribute,
+                         manager_dn))
         if description:
             attr.append((self.ldap.MOD_REPLACE, 'description', description))
         self.conn.modify_s('cn=%s,%s' % (project_id,
@@ -283,10 +301,9 @@
             return roles
         else:
             project_dn = 'cn=%s,%s' % (project_id, FLAGS.ldap_project_subtree)
-            roles = self.__find_objects(project_dn,
-                                        '(&(&(objectclass=groupOfNames)'
-                                        '(!(objectclass=novaProject)))'
-                                        '(member=%s))' % self.__uid_to_dn(uid))
+            query = ('(&(&(objectclass=groupOfNames)(!%s))(member=%s))' %
+                     (LdapDriver.project_pattern, self.__uid_to_dn(uid)))
+            roles = self.__find_objects(project_dn, query)
             return [role['cn'][0] for role in roles]
 
     def delete_user(self, uid):
@@ -300,14 +317,15 @@
             # Retrieve user by name
             user = self.__get_ldap_user(uid)
             if 'secretKey' in user.keys():
-                attr.append((self.ldap.MOD_DELETE, 'secretKey', \
-                user['secretKey']))
+                attr.append((self.ldap.MOD_DELETE, 'secretKey',
+                             user['secretKey']))
             if 'accessKey' in user.keys():
-                attr.append((self.ldap.MOD_DELETE, 'accessKey', \
-                user['accessKey']))
-            if 'isAdmin' in user.keys():
-                attr.append((self.ldap.MOD_DELETE, 'isAdmin', \
-                user['isAdmin']))
+                attr.append((self.ldap.MOD_DELETE, 'accessKey',
+                             user['accessKey']))
+            if LdapDriver.isadmin_attribute in user.keys():
+                attr.append((self.ldap.MOD_DELETE,
+                             LdapDriver.isadmin_attribute,
+                             user[LdapDriver.isadmin_attribute]))
             self.conn.modify_s(self.__uid_to_dn(uid), attr)
         else:
             # Delete entry
@@ -329,7 +347,8 @@
         if secret_key:
             attr.append((self.ldap.MOD_REPLACE, 'secretKey', secret_key))
         if admin is not None:
-            attr.append((self.ldap.MOD_REPLACE, 'isAdmin', str(admin).upper()))
+            attr.append((self.ldap.MOD_REPLACE, LdapDriver.isadmin_attribute,
+                         str(admin).upper()))
         self.conn.modify_s(self.__uid_to_dn(uid), attr)
 
     def __user_exists(self, uid):
@@ -347,7 +366,7 @@
     def __get_ldap_user(self, uid):
         """Retrieve LDAP user entry by id"""
         attr = self.__find_object(self.__uid_to_dn(uid),
-                                '(objectclass=novaUser)')
+                                  '(objectclass=novaUser)')
         return attr
 
     def __find_object(self, dn, query=None, scope=None):
@@ -383,19 +402,21 @@
 
     def __find_role_dns(self, tree):
         """Find dns of role objects in given tree"""
-        return self.__find_dns(tree,
-                '(&(objectclass=groupOfNames)(!(objectclass=novaProject)))')
+        query = ('(&(objectclass=groupOfNames)(!%s))' %
+                 LdapDriver.project_pattern)
+        return self.__find_dns(tree, query)
 
     def __find_group_dns_with_member(self, tree, uid):
         """Find dns of group objects in a given tree that contain member"""
-        dns = self.__find_dns(tree,
-                            '(&(objectclass=groupOfNames)(member=%s))' %
-                            self.__uid_to_dn(uid))
+        query = ('(&(objectclass=groupOfNames)(member=%s))' %
+                 self.__uid_to_dn(uid))
+        dns = self.__find_dns(tree, query)
         return dns
 
     def __group_exists(self, dn):
         """Check if group exists"""
-        return self.__find_object(dn, '(objectclass=groupOfNames)') is not None
+        query = '(objectclass=groupOfNames)'
+        return self.__find_object(dn, query) is not None
 
     @staticmethod
     def __role_to_dn(role, project_id=None):
@@ -417,9 +438,9 @@
         if member_uids is not None:
             for member_uid in member_uids:
                 if not self.__user_exists(member_uid):
-                    raise exception.NotFound(_("Group can't be created "
-                                               "because user %s doesn't exist")
-                                             % member_uid)
+                    raise exception.NotFound("Group can't be created "
+                                             "because user %s doesn't exist" %
+                                             member_uid)
                 members.append(self.__uid_to_dn(member_uid))
         dn = self.__uid_to_dn(uid)
         if not dn in members:
@@ -434,9 +455,8 @@
     def __is_in_group(self, uid, group_dn):
         """Check if user is in group"""
         if not self.__user_exists(uid):
-            raise exception.NotFound(_("User %s can't be searched in group "
-                                       "because the user doesn't exist")
-                                     % uid)
+            raise exception.NotFound("User %s can't be searched in group "
+                                     "because the user doesn't exist" % uid)
         if not self.__group_exists(group_dn):
             return False
         res = self.__find_object(group_dn,
@@ -447,12 +467,11 @@
     def __add_to_group(self, uid, group_dn):
         """Add user to group"""
         if not self.__user_exists(uid):
-            raise exception.NotFound(_("User %s can't be added to the group "
-                                       "because the user doesn't exist")
-                                     % uid)
+            raise exception.NotFound("User %s can't be added to the group "
+                                     "because the user doesn't exist" % uid)
         if not self.__group_exists(group_dn):
-            raise exception.NotFound(_("The group at dn %s doesn't exist")
-                                     % group_dn)
+            raise exception.NotFound("The group at dn %s doesn't exist" %
+                                     group_dn)
         if self.__is_in_group(uid, group_dn):
             raise exception.Duplicate(_("User %s is already a member of "
                                         "the group %s") % (uid, group_dn))
@@ -462,18 +481,17 @@
     def __remove_from_group(self, uid, group_dn):
         """Remove user from group"""
         if not self.__group_exists(group_dn):
-            raise exception.NotFound(_("The group at dn %s doesn't exist")
-                                     % group_dn)
+            raise exception.NotFound("The group at dn %s doesn't exist" %
+                                     group_dn)
         if not self.__user_exists(uid):
-            raise exception.NotFound(_("User %s can't be removed from the "
-                                       "group because the user doesn't exist")
-                                     % uid)
+            raise exception.NotFound("User %s can't be removed from the "
+                                     "group because the user doesn't exist" %
+                                     uid)
         if not self.__is_in_group(uid, group_dn):
-            raise exception.NotFound(_("User %s is not a member of the group")
-                                     % uid)
+            raise exception.NotFound("User %s is not a member of the group" %
+                                     uid)
         # NOTE(vish): remove user from group and any sub_groups
-        sub_dns = self.__find_group_dns_with_member(
-                group_dn, uid)
+        sub_dns = self.__find_group_dns_with_member(group_dn, uid)
         for sub_dn in sub_dns:
             self.__safe_remove_from_group(uid, sub_dn)
 
@@ -491,9 +509,8 @@
     def __remove_from_all(self, uid):
         """Remove user from all roles and projects"""
         if not self.__user_exists(uid):
-            raise exception.NotFound(_("User %s can't be removed from all "
-                                       "because the user doesn't exist")
-                                     % uid)
+            raise exception.NotFound("User %s can't be removed from all "
+                                     "because the user doesn't exist" % uid)
         role_dns = self.__find_group_dns_with_member(
                 FLAGS.role_project_subtree, uid)
         for role_dn in role_dns:
@@ -521,13 +538,13 @@
         if attr is None:
             return None
         if ('accessKey' in attr.keys() and 'secretKey' in attr.keys() \
-            and 'isAdmin' in attr.keys()):
+            and LdapDriver.isadmin_attribute in attr.keys()):
             return {
-                'id': attr['uid'][0],
-                'name': attr['cn'][0],
+                'id': attr[FLAGS.ldap_user_id_attribute][0],
+                'name': attr[FLAGS.ldap_user_name_attribute][0],
                 'access': attr['accessKey'][0],
                 'secret': attr['secretKey'][0],
-                'admin': (attr['isAdmin'][0] == 'TRUE')}
+                'admin': (attr[LdapDriver.isadmin_attribute][0] == 'TRUE')}
         else:
             return None
 
@@ -539,7 +556,8 @@
         return {
             'id': attr['cn'][0],
             'name': attr['cn'][0],
-            'project_manager_id': self.__dn_to_uid(attr['projectManager'][0]),
+            'project_manager_id':
+                self.__dn_to_uid(attr[LdapDriver.project_attribute][0]),
             'description': attr.get('description', [None])[0],
             'member_ids': [self.__dn_to_uid(x) for x in member_dns]}
 
@@ -549,9 +567,10 @@
         return dn.split(',')[0].split('=')[1]
 
     @staticmethod
-    def __uid_to_dn(dn):
+    def __uid_to_dn(uid):
         """Convert uid to dn"""
-        return 'uid=%s,%s' % (dn, FLAGS.ldap_user_subtree)
+        return (FLAGS.ldap_user_id_attribute + '=%s,%s'
+                % (uid, FLAGS.ldap_user_subtree))
 
 
 class FakeLdapDriver(LdapDriver):

=== modified file 'nova/auth/nova_openldap.schema'
--- nova/auth/nova_openldap.schema	2010-10-25 22:42:49 +0000
+++ nova/auth/nova_openldap.schema	2010-12-22 23:56:00 +0000
@@ -1,7 +1,9 @@
 #
 # Person object for Nova
 # inetorgperson with extra attributes
-# Author: Vishvananda Ishaya <[e-mail]>
+# Schema version: 2
+# Authors: Vishvananda Ishaya <[e-mail]>
+#          Ryan Lane <[e-mail]>
 #
 #
 
@@ -31,54 +33,18 @@
     )
 
 attributetype (
-    novaAttrs:3
-    NAME 'keyFingerprint'
-    DESC 'Fingerprint of private key'
-    EQUALITY caseIgnoreMatch
-    SUBSTR caseIgnoreSubstringsMatch
-    SYNTAX 1.3.6.1.4.1.1466.115.121.1.15
-    SINGLE-VALUE
-    )
-
-attributetype (
     novaAttrs:4
-    NAME 'isAdmin'
-    DESC 'Is user an administrator?'
+    NAME 'isNovaAdmin'
+    DESC 'Is user an nova administrator?'
     EQUALITY booleanMatch
     SYNTAX 1.3.6.1.4.1.1466.115.121.1.7
     SINGLE-VALUE
     )
 
-attributetype (
-    novaAttrs:5
-    NAME 'projectManager'
-    DESC 'Project Managers of a project'
-    SYNTAX 1.3.6.1.4.1.1466.115.121.1.12
-    )
-
 objectClass (
     novaOCs:1
     NAME 'novaUser'
     DESC 'access and secret keys'
     AUXILIARY
-    MUST ( uid )
-    MAY  ( accessKey $ secretKey $ isAdmin )
-    )
-
-objectClass (
-    novaOCs:2
-    NAME 'novaKeyPair'
-    DESC 'Key pair for User'
-    SUP top
-    STRUCTURAL
-    MUST ( cn $ sshPublicKey $ keyFingerprint )
-    )
-
-objectClass (
-    novaOCs:3
-    NAME 'novaProject'
-    DESC 'Container for project'
-    SUP groupOfNames
-    STRUCTURAL
-    MUST ( cn $ projectManager )
+    MAY  ( accessKey $ secretKey $ isNovaAdmin )
     )

=== modified file 'nova/auth/nova_sun.schema'
--- nova/auth/nova_sun.schema	2010-10-25 22:42:49 +0000
+++ nova/auth/nova_sun.schema	2010-12-22 23:56:00 +0000
@@ -1,16 +1,13 @@
 #
 # Person object for Nova
 # inetorgperson with extra attributes
-# Author: Vishvananda Ishaya <[e-mail]>
-# Modified for strict RFC 4512 compatibility by: Ryan Lane <[e-mail]>
+# Schema version: 2
+# Authors: Vishvananda Ishaya <[e-mail]>
+#          Ryan Lane <[e-mail]>
 #
 # using internet experimental oid arc as per BP64 3.1
 dn: cn=schema
 attributeTypes: ( 1.3.6.1.3.1.666.666.3.1 NAME 'accessKey' DESC 'Key for accessing data' EQUALITY caseIgnoreMatch SUBSTR caseIgnoreSubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE )
 attributeTypes: ( 1.3.6.1.3.1.666.666.3.2 NAME 'secretKey' DESC 'Secret key' EQUALITY caseIgnoreMatch SUBSTR caseIgnoreSubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE )
-attributeTypes: ( 1.3.6.1.3.1.666.666.3.3 NAME 'keyFingerprint' DESC 'Fingerprint of private key' EQUALITY caseIgnoreMatch SUBSTR caseIgnoreSubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE)
-attributeTypes: ( 1.3.6.1.3.1.666.666.3.4 NAME 'isAdmin' DESC 'Is user an administrator?' EQUALITY booleanMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 SINGLE-VALUE ) 
-attributeTypes: ( 1.3.6.1.3.1.666.666.3.5 NAME 'projectManager' DESC 'Project Managers of a project' SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 )
-objectClasses: ( 1.3.6.1.3.1.666.666.4.1 NAME 'novaUser' DESC 'access and secret keys' SUP top AUXILIARY MUST ( uid ) MAY  ( accessKey $ secretKey $ isAdmin ) ) 
-objectClasses: ( 1.3.6.1.3.1.666.666.4.2 NAME 'novaKeyPair' DESC 'Key pair for User' SUP top STRUCTURAL MUST ( cn $ sshPublicKey $ keyFingerprint ) )
-objectClasses: ( 1.3.6.1.3.1.666.666.4.3 NAME 'novaProject' DESC 'Container for project' SUP groupOfNames STRUCTURAL MUST ( cn $ projectManager ) )
+attributeTypes: ( 1.3.6.1.3.1.666.666.3.4 NAME 'isNovaAdmin' DESC 'Is user a nova administrator?' EQUALITY booleanMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 SINGLE-VALUE ) 
+objectClasses: ( 1.3.6.1.3.1.666.666.4.1 NAME 'novaUser' DESC 'access and secret keys' SUP top AUXILIARY MAY  ( accessKey $ secretKey $ isNovaAdmin ) ) 

=== modified file 'nova/auth/opendj.sh'
--- nova/auth/opendj.sh	2010-11-30 23:12:19 +0000
+++ nova/auth/opendj.sh	2010-12-22 23:56:00 +0000
@@ -32,7 +32,6 @@
 schemapath='/var/opendj/instance/config/schema'
 cp $abspath/openssh-lpk_sun.schema $schemapath/97-openssh-lpk_sun.ldif
 cp $abspath/nova_sun.schema $schemapath/98-nova_sun.ldif
-chown opendj:opendj $schemapath/97-openssh-lpk_sun.ldif
 chown opendj:opendj $schemapath/98-nova_sun.ldif
 
 cat >/etc/ldap/ldap.conf <<LDAP_CONF_EOF

=== added file 'nova/auth/openssh-lpk_openldap.schema'
--- nova/auth/openssh-lpk_openldap.schema	1970-01-01 00:00:00 +0000
+++ nova/auth/openssh-lpk_openldap.schema	2010-12-22 23:56:00 +0000
@@ -0,0 +1,19 @@
+#
+# LDAP Public Key Patch schema for use with openssh-ldappubkey
+# Author: Eric AUGE <[e-mail]>
+#
+# Based on the proposal of : Mark Ruijter
+#
+
+
+# octetString SYNTAX
+attributetype ( 1.3.6.1.4.1.24552.500.1.1.1.13 NAME 'sshPublicKey'
+	DESC 'MANDATORY: OpenSSH Public key'
+	EQUALITY octetStringMatch
+	SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 )
+
+# printableString SYNTAX yes|no
+objectclass ( 1.3.6.1.4.1.24552.500.1.1.2.0 NAME 'ldapPublicKey' SUP top AUXILIARY
+	DESC 'MANDATORY: OpenSSH LPK objectclass'
+	MAY ( sshPublicKey $ uid )
+	)

=== removed file 'nova/auth/openssh-lpk_openldap.schema'
--- nova/auth/openssh-lpk_openldap.schema	2010-10-25 22:42:49 +0000
+++ nova/auth/openssh-lpk_openldap.schema	1970-01-01 00:00:00 +0000
@@ -1,19 +0,0 @@
-#
-# LDAP Public Key Patch schema for use with openssh-ldappubkey
-# Author: Eric AUGE <[e-mail]>
-#
-# Based on the proposal of : Mark Ruijter
-#
-
-
-# octetString SYNTAX
-attributetype ( 1.3.6.1.4.1.24552.500.1.1.1.13 NAME 'sshPublicKey'
-	DESC 'MANDATORY: OpenSSH Public key'
-	EQUALITY octetStringMatch
-	SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 )
-
-# printableString SYNTAX yes|no
-objectclass ( 1.3.6.1.4.1.24552.500.1.1.2.0 NAME 'ldapPublicKey' SUP top AUXILIARY
-	DESC 'MANDATORY: OpenSSH LPK objectclass'
-	MAY ( sshPublicKey $ uid )
-	)

=== added file 'nova/auth/openssh-lpk_sun.schema'
--- nova/auth/openssh-lpk_sun.schema	1970-01-01 00:00:00 +0000
+++ nova/auth/openssh-lpk_sun.schema	2010-12-22 23:56:00 +0000
@@ -0,0 +1,10 @@
+#
+# LDAP Public Key Patch schema for use with openssh-ldappubkey
+# Author: Eric AUGE <[e-mail]>
+# 
+# Schema for Sun Directory Server.
+# Based on the original schema, modified by Stefan Fischer.
+#
+dn: cn=schema
+attributeTypes: ( 1.3.6.1.4.1.24552.500.1.1.1.13 NAME 'sshPublicKey' DESC 'MANDATORY: OpenSSH Public key' EQUALITY octetStringMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 )
+objectClasses: ( 1.3.6.1.4.1.24552.500.1.1.2.0 NAME 'ldapPublicKey' SUP top AUXILIARY DESC 'MANDATORY: OpenSSH LPK objectclass' MAY ( sshPublicKey $ uid ) )

=== removed file 'nova/auth/openssh-lpk_sun.schema'
--- nova/auth/openssh-lpk_sun.schema	2010-10-25 22:50:32 +0000
+++ nova/auth/openssh-lpk_sun.schema	1970-01-01 00:00:00 +0000
@@ -1,10 +0,0 @@
-#
-# LDAP Public Key Patch schema for use with openssh-ldappubkey
-# Author: Eric AUGE <[e-mail]>
-# 
-# Schema for Sun Directory Server.
-# Based on the original schema, modified by Stefan Fischer.
-#
-dn: cn=schema
-attributeTypes: ( 1.3.6.1.4.1.24552.500.1.1.1.13 NAME 'sshPublicKey' DESC 'MANDATORY: OpenSSH Public key' EQUALITY octetStringMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.40 )
-objectClasses: ( 1.3.6.1.4.1.24552.500.1.1.2.0 NAME 'ldapPublicKey' SUP top AUXILIARY DESC 'MANDATORY: OpenSSH LPK objectclass' MAY ( sshPublicKey $ uid ) )

=== modified file 'nova/auth/slap.sh'
--- nova/auth/slap.sh	2010-10-25 22:42:49 +0000
+++ nova/auth/slap.sh	2010-12-22 23:56:00 +0000
@@ -22,7 +22,7 @@
 
 abspath=`dirname "$(cd "${0%/*}" 2>/dev/null; echo "$PWD"/"${0##*/}")"`
 cp $abspath/openssh-lpk_openldap.schema /etc/ldap/schema/openssh-lpk_openldap.schema
-cp $abspath/nova_openldap.schema /etc/ldap/schema/nova_openldap.schema
+cp $abspath/nova_openldap.schema /etc/ldap/schema/nova.schema
 
 mv /etc/ldap/slapd.conf /etc/ldap/slapd.conf.orig
 cat >/etc/ldap/slapd.conf <<SLAPD_CONF_EOF
@@ -33,7 +33,6 @@
 include /etc/ldap/schema/core.schema
 include /etc/ldap/schema/cosine.schema
 include /etc/ldap/schema/inetorgperson.schema
-include /etc/ldap/schema/openssh-lpk_openldap.schema
 include /etc/ldap/schema/nova.schema
 pidfile /var/run/slapd/slapd.pid
 argsfile /var/run/slapd/slapd.args