[Freeipa-devel] [PATCHES] 0258-0265 Add schema updater based on IPA schema files

Fri Nov 15 13:09:28 UTC 2013

On 11/11/2013 04:18 PM, Ana Krivokapic wrote:
> On 11/11/2013 02:53 PM, Ana Krivokapic wrote:
>> On 11/11/2013 12:32 PM, Petr Viktorin wrote:
>>> On 11/07/2013 02:34 PM, Ana Krivokapic wrote:
>>>> On 11/01/2013 03:26 PM, Petr Viktorin wrote:
>>>>> On 09/13/2013 06:44 PM, Petr Viktorin wrote:
>>>>>> On 08/01/2013 04:52 PM, Petr Viktorin wrote:
>>>>>>> Hello,
>>>>>>> With these patches, schema updates will be based on the ldif files we
>>>>>>> use for installation.
>>>>>>>
>>>>>>> https://fedorahosted.org/freeipa/ticket/3454
>>>>>>>
>>>>>>> This is a RFE, here is the design doc:
>>>>>>> http://www.freeipa.org/page/V3/Improved_schema_updater
>>>>>>>
>>>>>> I found and filed a bug in python-ldap[0]: it sometimes ignores parts of
>>>>>> schema LDIFs when parsing them.
>>>>>> Patch 0275 works around the bug. Please apply on top of 0258-0265 (they
>>>>>> still apply cleanly).
>>>>>>
>>>>>>
>>>>>> [0] https://bugzilla.redhat.com/show_bug.cgi?id=1007820
>>>>>>
>>>>> The recent ipaldap patches resulted in a small conflict. Attaching
>>>>> rebased patches.
>>>> I have tested the patches and overall they seem to work fine. Some
>>>> questions/comments are below.
>>>>
>>>>
>>>> Patch 258:
>>>> You catch `ldap.LOCAL_ERROR` in the `connect()` function, which is
>>>> called from `__init__()`, so no need to catch it again in `__init__()`.
>>> I've added a comment instead of the try/catch
>>>
>>>> Patch 259:
>>>> ACK
>>>>
>>>> Patch 260:
>>>>
>>>>   >       # Usually the modlist order does not matter.
>>>>   >       # However, for schema updates, we want 'attributetypes' before
>>>>   >       # 'objectclasses'.
>>>>   >       # A simple sort will ensure this.
>>>>   >       modlist.sort()
>>>>
>>>> Since `modlist` is a list of tuples, it is sorted by the first elements
>>>> in the tuples, then by the seconds elements, etc. Which means the
>>>> resulting list will be sorted by the modification type first (`MOD_ADD`,
>>>> `MOD_DELETE`, etc), and by `attributetypes`/`objectclasses` second. Was
>>>> this the desired effect?
>>> I've added a sort key; it should be safer now.
>> Hmm, the key you added still retains the default sorting behavior - it will sort
>> by the first elements of the tuples first. Since we need sorting by the second
>> elements, I think the key here should be: key=lambda m: m[1].lower()

Right, I see what you mean.
I've made the change and tested it a bit.

>>>> Patch 261:
>>>> Man page updates look good, but several options in the man page have 3
>>>> dashes in the long form instead of 2. I have attached a mini-patch that
>>>> fixes this along with a couple of typos in the man page. Feel free to
>>>> squash it to your patch 261.
>>> Nice catch! Squashed.
>>>
>>>> Patch 262:
>>>> Whitespace warnings.
>>> I didn't see any with my settings; could you be more specific?
>> $ git am
>> ~/freeipa-pviktori-0262.3-Remove-schema-modifications-from-update-files.patch
>> Applying: Remove schema modifications from update files
>> /home/ana/freeipa/.git/rebase-apply/patch:497: new blank line at EOF.
>> +
>> /home/ana/freeipa/.git/rebase-apply/patch:693: new blank line at EOF.
>> +
>> warning: 2 lines add whitespace errors.

Thanks! fixed.

[...]
> I'm also seeing some errors when testing the patches. During ipa-server-install,
> restarting of DS is failing:
>
>    [22/38]: restarting directory server
> ipa         : CRITICAL Failed to restart the directory server (Command
> '/bin/systemctl restart dirsrv at IDM-LAB-ENG-BRQ-REDHAT-COM.service' returned
> non-zero exit status 1). See the installation log for details.
>
>
> The dirsrv log indicates that one of the ldif files is not syntactically valid:
>
> [11/Nov/2013:16:10:21 +0100] dse_read_one_file - The entry cn=schema in file
> /etc/dirsrv/slapd-IDM-LAB-ENG-BRQ-REDHAT-COM/schema/60basev3.ldif (lineno: 1) is
> invalid, error code 21 (Invalid syntax) - object class
> (2.16.840.1.113730.3.8.12.7 NAME 'ipaKrb5DelegationACL' SUP groupOfPrincipals
> STRUCTURAL MAY ( ipaAllowToImpersonate $ ipaAllowedTarget ) EQUALITY
> distinguishedNameMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 X-ORIGIN 'IPA v3' ):
> Failed to parse objectclass, error(2) at ( distinguishedNameMatch SYNTAX
> 1.3.6.1.4.1.1466.115.121.1.12 X-ORIGIN 'IPA v3' ))
> [11/Nov/2013:16:10:21 +0100] dse - Please edit the file to correct the reported
> problems and then restart the server.
>
> Are you seeing this in your setup?

No, ipa-server-install works for me. Does this happen every time?
I can't find an error in the syntax there.

-- 
Petr³

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0258.4-ldapupdate-Factor-out-connection-code.patch
Type: text/x-patch
Size: 4880 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131115/59e6793e/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0259.4-dsinstance-Move-the-list-of-schema-filenames-to-a-co.patch
Type: text/x-patch
Size: 2098 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131115/59e6793e/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0260.4-Add-schema-updater-based-on-IPA-schema-files.patch
Type: text/x-patch
Size: 13140 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131115/59e6793e/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0261.4-Update-the-man-page-for-ipa-ldap-updater.patch
Type: text/x-patch
Size: 3638 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131115/59e6793e/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0262.4-Remove-schema-modifications-from-update-files.patch
Type: text/x-patch
Size: 60191 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131115/59e6793e/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0263.4-Remove-schema-special-casing-from-the-LDAP-updater.patch
Type: text/x-patch
Size: 10094 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131115/59e6793e/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0264.4-Make-schema-files-conform-to-new-updater.patch
Type: text/x-patch
Size: 27262 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131115/59e6793e/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0265.4-Add-formerly-update-only-schema.patch
Type: text/x-patch
Size: 10413 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131115/59e6793e/attachment-0007.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0275.4-Unify-capitalization-of-attribute-names-in-schema-fi.patch
Type: text/x-patch
Size: 11040 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20131115/59e6793e/attachment-0008.bin>