[Freeipa-devel] [PATCH] 1088 Recover DNA ranges when deleting a master

[Petr Viktorin]

On 03/06/2013 09:52 PM, Rob Crittenden wrote:
> Petr Viktorin wrote:
[...]
>> On new installs, the ACI on cn=Posix IDs,cn=Distributed Numeric
>> Assignment Plugin,cn=plugins,cn=config is added before the entry itself.
>> I didn't test everything as I didn't get the access.
>
> It shouldn't make a difference. What isn't working?

I get a CRITICAL message in the log:

add aci:
         (targetattr=dnaNextRange || dnaNextValue || 
dnaMaxValue)(version 3.0;acl "permission:Modify DNA Range";allow (write) 
groupdn = "ldap:///cn=Modify DNA 
Range,cn=permissions,cn=pbac,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com";)
modifying entry "cn=Posix IDs,cn=Distributed Numeric Assignment 
Plugin,cn=plugins,cn=config"

2013-03-07T11:01:48Z DEBUG stderr=ldap_initialize( 
ldap://vm-081.idm.lab.eng.brq.redhat.com:389/??base )
ldap_modify: No such object (32)

2013-03-07T11:01:48Z CRITICAL Failed to load replica-acis.ldif: Command 
'/usr/bin/ldapmodify -v -f /tmp/tmpT55upM -H 
ldap://vm-081.idm.lab.eng.brq.redhat.com:389 -x -D cn=Directory Manager 
-y /tmp/tmplFeere' returned non-zero exit status 32


This particular ACI is added later by the updater. But, after failing 
here the rest of the file is skipped, and the subsequent ACIs aren't 
added in updates. Not being able to run CLEANALLRUV prevents cleanly 
deleting a replica.

[...]
>> [...]
>>> +    try:
>>> +        repl = replication.ReplicationManager(realm, hostname,
>>> dirman_passwd)
>>> +    except Exception, e:
>>> +        sys.exit("Connection failed: %s" %
>>> ipautil.convert_ldap_error(e))
>>
>> ipaldap should convert LDAP errors to IPA ones, there's no need to call
>> convert_ldap_error. Same in other places.
>
> It does in some but it isn't consistent. I removed my calls though.
>
> $ ipa-replica-manage dnarange-show -p badpassword
> Connection failed: {'desc': 'Invalid credentials'}

That's a bug. My patch 0194 should fix this, I'll check after it's 
merged. Anyway it's not a show stopper now.

[...]
>>> +        failed = 0
>>> +        for ent in entries:
>>
>>
>> This loops more than necessary and is somewhat hard to follow. Consider
>> using for-else here:
>>
>> for ...:
>>      ...
>>      if okay:
>>          break
>> else:
>>      raise error
>
> I simplified things a bit but a for/else won't work here as we need to
> check all ranges all the time. It is perfectly fine to not fit into a
> range, as long as it fits into SOME range.

Well, that's how for's (not if's) else clause works -- it's executed 
after all the looping's done if you didn't `break` out.
http://docs.python.org/2/tutorial/controlflow.html#break-and-continue-statements-and-else-clauses-on-loops
Maybe I'm just used to it and it's too esoteric to the average reader, 
though.

[...]
> Ok, I'll drop this since it doesn't affect things with the new LDAP
> backend.

Please do, you left it in by mistake.

> I also added one change related to the LDAP core changes. In the past if
> you did not have a ticket it would prompt for DM password. This was
> broken after the updates. I added an additional except in
> test_connection().

This should also be fixed soon in ipaldap. Thanks for putting up with 
the changes.

-- 
Petr³