On Wed, 2011-10-05 at 13:44 -0400, Rob Crittenden wrote:
> Martin Kosek wrote:
> > Since IPA v2 server already contain predefined groups that may collide
> > with groups in migrated (IPA v1) server (for example admins, ipausers),
> > users having colliding group as their primary group may happen to belong
> > to an unknown group on new IPA v2 server.
> >
> > Implement --group-overwrite-gid option to overwrite GID of already
> > existing groups to prevent this issue.
> >
> > https://fedorahosted.org/freeipa/ticket/1866
> 
> For argument's sake, what is the user going to see the first time they 
> run this? I assume they won't think about these duplicate groups and 
> just do the migration. This means that the result may be some users 
> pointing to non-existent GIDs.

At first I was thinking about making the GID the default behavior and
just add flag "--dont-overwrite-gid. But I was afraid this could do some
damage and change GIDs where it is not required. However, I made some
improvements in this area, please see below.

> 
> If they re-run the migration with this option will it then fix 
> everything up?

Yep.

> 
> I'm wondering if we need a --test argument so people can run the 
> migration w/o writing entries to look for problems like this.
> 
> rob

If we want to do this, we would have to add a lot of LDAP query checks
since mostly try doing the LDAP write and write failures in case of an
exception.

However, I updated the patch so that user is notified about existence of
--group-overwrite-gid option better. If a migration of a group with a
GID number fails because of DuplicateError, a notice about GID is
displayed. This should make him check this situation and either use
group-mod --gidnumber=... or re-run the migration with
--group-overwrite-gid.

I also updated the Password option not to ask user for LDAP password
twice, because it makes me really mad :-)

Martin
>From 64ffbed338ba8a72d7af8d8e51d21c9e54d4965f Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Mon, 3 Oct 2011 16:01:01 +0200
Subject: [PATCH] Improve handling of GIDs when migrating groups

Since IPA v2 server already contain predefined groups that may collide
with groups in migrated (IPA v1) server (for example admins, ipausers),
users having colliding group as their primary group may happen to belong
to an unknown group on new IPA v2 server.

Implement --group-overwrite-gid option to overwrite GID of already
existing groups to prevent this issue.

https://fedorahosted.org/freeipa/ticket/1866
---
 API.txt                     |    7 ++--
 VERSION                     |    2 +-
 ipalib/plugins/migration.py |   74 ++++++++++++++++++++++++++++++++++++------
 3 files changed, 68 insertions(+), 15 deletions(-)

diff --git a/API.txt b/API.txt
index 10b3f86a8f33d53944423af46b42da1e7f377232..77ff362f943ded64a0b18443e3557f2cedf0b873 100644
--- a/API.txt
+++ b/API.txt
@@ -1701,9 +1701,9 @@ output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), 'User-friendly
 output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('value', <type 'unicode'>, "The primary_key value of the entry, e.g. 'jdoe' for a user")
 command: migrate_ds
-args: 2,13,3
+args: 2,14,3
 arg: Str('ldapuri', validate_ldapuri, cli_name='ldap_uri', label=Gettext('LDAP URI', domain='ipa', localedir=None))
-arg: Password('bindpw', cli_name='password', label=Gettext('Password', domain='ipa', localedir=None))
+arg: Password('bindpw', cli_name='password', confirm=False, label=Gettext('Password', domain='ipa', localedir=None))
 option: Str('binddn?', autofill=True, cli_name='bind_dn', default=u'cn=directory manager', label=Gettext('Bind DN', domain='ipa', localedir=None))
 option: Str('usercontainer?', autofill=True, cli_name='user_container', default=u'ou=people', label=Gettext('User container', domain='ipa', localedir=None))
 option: Str('groupcontainer?', autofill=True, cli_name='group_container', default=u'ou=groups', label=Gettext('Group container', domain='ipa', localedir=None))
@@ -1713,8 +1713,9 @@ option: List('userignoreobjectclass?', autofill=True, cli_name='user_ignore_obje
 option: List('userignoreattribute?', autofill=True, cli_name='user_ignore_attribute', default=(), label=Gettext('Ignore user attribute', domain='ipa', localedir=None), multivalue=True)
 option: List('groupignoreobjectclass?', autofill=True, cli_name='group_ignore_objectclass', default=(), label=Gettext('Ignore group object class', domain='ipa', localedir=None), multivalue=True)
 option: List('groupignoreattribute?', autofill=True, cli_name='group_ignore_attribute', default=(), label=Gettext('Ignore group attribute', domain='ipa', localedir=None), multivalue=True)
+option: Flag('groupoverwritegid', autofill=True, cli_name='group_overwrite_gid', default=False, label=Gettext('Overwrite GID', domain='ipa', localedir=None))
 option: StrEnum('schema?', autofill=True, cli_name='schema', default=u'RFC2307bis', label=Gettext('LDAP schema', domain='ipa', localedir=None), values=(u'RFC2307bis', u'RFC2307'))
-option: Flag('continue?', autofill=True, default=False)
+option: Flag('continue?', autofill=True, default=False, label=Gettext('Continue', domain='ipa', localedir=None))
 option: List('exclude_groups?', autofill=True, cli_name='exclude_groups', default=(), multivalue=True)
 option: List('exclude_users?', autofill=True, cli_name='exclude_users', default=(), multivalue=True)
 output: Output('result', <type 'dict'>, Gettext('Lists of objects migrated; categorized by type.', domain='ipa', localedir=None))
diff --git a/VERSION b/VERSION
index ff8f92b51ec1b6826d30cdafa0c59ef9be89af4e..a7547cd6dcbcec4617f04675b3f50873c26ac580 100644
--- a/VERSION
+++ b/VERSION
@@ -79,4 +79,4 @@ IPA_DATA_VERSION=20100614120000
 #                                                      #
 ########################################################
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=12
+IPA_API_VERSION_MINOR=13
diff --git a/ipalib/plugins/migration.py b/ipalib/plugins/migration.py
index 93ac114d8f30ec9f97dcb4bc59ee9ac39f50f4c4..c484b144b8df42079e9bc13c8c45e132934ee495 100644
--- a/ipalib/plugins/migration.py
+++ b/ipalib/plugins/migration.py
@@ -71,6 +71,15 @@ EXAMPLES:
  Specify the user and group container. This can be used to migrate user and
  group data from an IPA v1 server:
    ipa migrate-ds --user-container='cn=users,cn=accounts' --group-container='cn=groups,cn=accounts' ldap://ds.example.com:389
+
+ Since IPA v2 server already contain predefined groups that may collide with
+ groups in migrated (IPA v1) server (for example admins, ipausers), users having
+ colliding group as their primary group may happen to belong to an unknown group
+ on new IPA v2 server.
+ Use --group-overwrite-gid option to overwrite GID of already existing groups
+ to prevent this issue:
+    ipa migrate-ds --group-overwrite-gid --user-container='cn=users,cn=accounts' --group-container='cn=groups,cn=accounts' ldap://ds.example.com:389
+
 """)
 
 # USER MIGRATION CALLBACKS AND VARS
@@ -228,6 +237,28 @@ def _pre_migrate_group(ldap, pkey, dn, entry_attrs, failed, config, ctx, **kwarg
     return dn
 
 
+def _group_exc_callback(ldap, dn, entry_attrs, exc, options):
+    if isinstance(exc, errors.DuplicateEntry):
+        if options.get('groupoverwritegid', False) and \
+           entry_attrs.get('gidnumber') is not None:
+            try:
+                new_entry_attrs = {'gidnumber':entry_attrs['gidnumber']}
+                ldap.update_entry(dn, new_entry_attrs)
+            except errors.EmptyModlist:
+                # no change to the GID
+                pass
+            # mark as success
+            return
+        elif not options.get('groupoverwritegid', False) and \
+             entry_attrs.get('gidnumber') is not None:
+            msg = unicode(exc)
+            # add information about possibility to overwrite GID
+            msg = msg + unicode(_('. Check GID of the existing group. ' \
+                    'Use --group-overwrite-gid option to overwrite the GID'))
+            raise errors.DuplicateEntry(message=msg)
+
+    raise exc
+
 # DS MIGRATION PLUGIN
 
 def construct_filter(template, oc_list):
@@ -252,6 +283,7 @@ class migrate_ds(Command):
         # pre_callback - is called for each object just after it was
         #                retrieved from DS and before being added to IPA
         # post_callback - is called for each object after it was added to IPA
+        # exc_callback - is called when adding entry to IPA raises an exception
         #
         # {pre, post}_callback parameters:
         #  ldap - ldap2 instance connected to IPA
@@ -270,7 +302,8 @@ class migrate_ds(Command):
             'oc_blacklist_option' : 'userignoreobjectclass',
             'attr_blacklist_option' : 'userignoreattribute',
             'pre_callback' : _pre_migrate_user,
-            'post_callback' : _post_migrate_user
+            'post_callback' : _post_migrate_user,
+            'exc_callback' : None
         },
         'group': {
             'filter_template' : '(&(|%s)(cn=*))',
@@ -278,7 +311,8 @@ class migrate_ds(Command):
             'oc_blacklist_option' : 'groupignoreobjectclass',
             'attr_blacklist_option' : 'groupignoreattribute',
             'pre_callback' : _pre_migrate_group,
-            'post_callback' : None
+            'post_callback' : None,
+            'exc_callback' : _group_exc_callback,
         },
     }
     migrate_order = ('user', 'group')
@@ -292,6 +326,7 @@ class migrate_ds(Command):
         Password('bindpw',
             cli_name='password',
             label=_('Password'),
+            confirm=False,
             doc=_('bind password'),
         ),
     )
@@ -359,6 +394,12 @@ class migrate_ds(Command):
             default=tuple(),
             autofill=True,
         ),
+        Flag('groupoverwritegid',
+            cli_name='group_overwrite_gid',
+            label=_('Overwrite GID'),
+            doc=_('When migrating a group already existing in IPA domain overwrite the '\
+                  'group GID and report as success'),
+        ),
         StrEnum('schema?',
             cli_name='schema',
             label=_('LDAP schema'),
@@ -368,6 +409,7 @@ class migrate_ds(Command):
             autofill=True,
         ),
         Flag('continue?',
+            label=_('Continue'),
             doc=_('Continuous operation mode. Errors are reported but the process continues'),
             default=False,
         ),
@@ -539,16 +581,26 @@ can use their Kerberos accounts.''')
                 try:
                     ldap.add_entry(dn, entry_attrs)
                 except errors.ExecutionError, e:
-                    failed[ldap_obj_name][pkey] = unicode(e)
-                else:
-                    migrated[ldap_obj_name].append(pkey)
-
-                    callback = self.migrate_objects[ldap_obj_name]['post_callback']
+                    callback = self.migrate_objects[ldap_obj_name]['exc_callback']
                     if callable(callback):
-                        callback(
-                            ldap, pkey, dn, entry_attrs, failed[ldap_obj_name],
-                            config, context
-                        )
+                        try:
+                            callback(ldap, dn, entry_attrs, e, options)
+                        except errors.ExecutionError, e:
+                            failed[ldap_obj_name][pkey] = unicode(e)
+                            continue
+                    else:
+                        failed[ldap_obj_name][pkey] = unicode(e)
+                        continue
+
+                migrated[ldap_obj_name].append(pkey)
+
+                callback = self.migrate_objects[ldap_obj_name]['post_callback']
+                if callable(callback):
+                    callback(
+                        ldap, pkey, dn, entry_attrs, failed[ldap_obj_name],
+                        config, context,
+                        options = options,
+                    )
 
         return (migrated, failed)
 
-- 
1.7.6.4

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to