On 04/12/2012 04:17 AM, Martin Kosek wrote:
On Wed, 2012-04-11 at 22:05 -0400, John Dennis wrote:
Revised patch attached. We'll leave the DN parameter changes till later.
This is essentially the same as the original patch with the addition of
the fixes necessary to support passing an empty container arg, an issue
Martin discovered in his review. FWIW the answer was not to make the
param required (actually it would have been adding the flag 'nonempty')
because you should be able to say you don't want to introduce a
container into the search bases (see commit comment)

I don't agree with the removal of default values for the containers and
allowing an empty value for them. Please, see my reasoning:

1) I don't think its unlikely to have ou=People and ou=groups as
containers for users/groups as they are default containers in fresh LDAP
installs. I think most of the small LDAP deployments will use these

2) I am also not sure if somebody would want to pass empty user and
group container. Users and groups won't be shared in the same container
and since we search with _ldap.SCOPE_ONELEVEL the migration would not
find users or groups in containers nested under the search base anyway.

OK. Patch is revised, restored the defaults, usercontainer and groupcontainer are now required to be non-empty. Also, basedn had been optional without a default which didn't make much sense, now basedn is a required parameter.

John Dennis <jden...@redhat.com>

Looking to carve out IT costs?
>From fe05239e22b6c754aff22dd3e149e12e9d9a5bf7 Mon Sep 17 00:00:00 2001
From: John Dennis <jden...@redhat.com>
Date: Wed, 11 Apr 2012 21:51:17 -0400
Subject: [PATCH 72-2] Validate DN & RDN parameters for migrate command
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

Ticket #2555

We were generating a traceback (server error) if a malformed RDN was
passed as a parameter to the migrate command.

* add parameter validation functions validate_dn_param() and
  validate_rdn_param() to ipalib.util. Those functions simply invoke
  the DN or RDN constructor from our dn module passing it the string
  representation. If the constructor does not throw an error it's

* Add the parameter validation function pointers to the Param objects
  in the migrate command.

* Require that usercontainer and groupcontainer is nonempty, thus
  passing --usercontainer= on the command line will produce

  ipa: ERROR: 'user_container' is required

* Fix _get_search_bases() so if a container dn is empty it it just
  uses the base dn alone instead of faulting (currently
  bullet-proofing because now the containers are required).

* Update the doc for usercontainer and groupcontainer to reflect the
  fact they are DN's not RDN's. A RDN can only be one level and it
  should be possible to have a container more than one RDN removed
  from the base.

* Make the basedn required, formerly it was optional without a default
  which really doesn't make much sense.
 ipalib/plugins/migration.py |   24 +++++++++++++++---------
 ipalib/util.py              |   16 ++++++++++++++++
 2 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/ipalib/plugins/migration.py b/ipalib/plugins/migration.py
index 873ff4c..97bea25 100644
--- a/ipalib/plugins/migration.py
+++ b/ipalib/plugins/migration.py
@@ -23,6 +23,7 @@ import ldap as _ldap
 from ipalib import api, errors, output
 from ipalib import Command, Password, Str, Flag, StrEnum
 from ipalib.cli import to_cli
+from ipalib.util import validate_dn_param
 from ipalib.dn import *
 from ipalib.plugins.user import NO_UPG_MAGIC
 if api.env.in_server and api.env.context in ['lite', 'server']:
@@ -418,25 +419,27 @@ class migrate_ds(Command):
     takes_options = (
-        Str('binddn?',
+        Str('binddn?', validate_dn_param,
             label=_('Bind DN'),
             default=u'cn=directory manager',
-        Str('usercontainer?',
+        Str('usercontainer?', validate_dn_param,
             label=_('User container'),
-            doc=_('RDN of container for users in DS relative to base DN'),
+            doc=_('DN of container for users in DS relative to base DN'),
+            flags = ['nonempty'],
-        Str('groupcontainer?',
+        Str('groupcontainer?', validate_dn_param,
             label=_('Group container'),
-            doc=_('RDN of container for groups in DS relative to base DN'),
+            doc=_('DN of container for groups in DS relative to base DN'),
+            flags = ['nonempty'],
@@ -505,7 +508,7 @@ class migrate_ds(Command):
             doc=_('Continuous operation mode. Errors are reported but the process continues'),
-        Str('basedn?',
+        Str('basedn',
             label=_('Base DN'),
             doc=_('Base DN on remote LDAP server'),
@@ -589,9 +592,12 @@ can use their Kerberos accounts.''')
     def _get_search_bases(self, options, ds_base_dn, migrate_order):
         search_bases = dict()
         for ldap_obj_name in migrate_order:
-            search_bases[ldap_obj_name] = '%s,%s' % (
-                options['%scontainer' % to_cli(ldap_obj_name)], ds_base_dn
-            )
+            container = options.get('%scontainer' % to_cli(ldap_obj_name))
+            if container:
+                search_base = str(DN(container, ds_base_dn))
+            else:
+                search_base = ds_base_dn
+            search_bases[ldap_obj_name] = search_base
         return search_bases
     def migrate(self, ldap, config, ds_ldap, ds_base_dn, options):
diff --git a/ipalib/util.py b/ipalib/util.py
index a79f41c..487d278 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -31,6 +31,7 @@ from weakref import WeakKeyDictionary
 from ipalib import errors
 from ipalib.text import _
+from ipalib.dn import DN, RDN
 from ipapython import dnsclient
 from ipapython.ipautil import decode_ssh_pubkey
@@ -484,3 +485,18 @@ def gen_dns_update_policy(realm, rrtypes=('A', 'AAAA', 'SSHFP')):
     policy += ";"
     return policy
+def validate_rdn_param(ugettext, value):
+    try:
+        rdn = RDN(value)
+    except Exception, e:
+        return str(e)
+    return None
+def validate_dn_param(ugettext, value):
+    try:
+        rdn = DN(value)
+    except Exception, e:
+        return str(e)
+    return None

Freeipa-devel mailing list

Reply via email to