[Freeipa-devel] [PATCH 0158] Extend ipa-range-check DS plugin to handle range types

[Petr Viktorin]

On 04/08/2014 02:26 PM, Martin Kosek wrote:
> On 04/01/2014 10:52 AM, Tomas Babej wrote:
>>
>> On 04/01/2014 10:40 AM, Alexander Bokovoy wrote:
>>> On Tue, 01 Apr 2014, Tomas Babej wrote:
>>>>  From 736b3f747188696fd4a46ca63d91a6cca942fd56 Mon Sep 17 00:00:00 2001
>>>> From: Tomas Babej <tbabej at redhat.com>
>>>> Date: Wed, 5 Mar 2014 12:28:18 +0100
>>>> Subject: [PATCH] Extend ipa-range-check DS plugin to handle range types
>>>>
>>>> The ipa-range-check plugin used to determine the range type depending
>>>> on the value of the attributes such as RID or secondary RID base. This
>>>> approached caused variety of issues since the portfolio of ID range
>>>> types expanded.
>>>>
>>>> The patch makes sure the following rules are implemented:
>>>>     * No ID range pair can overlap on base ranges, with exception
>>>>       of two ipa-ad-trust-posix ranges belonging to the same forest
>>>>     * For any ID range pair of ranges belonging to the same domain:
>>>>         * Both ID ranges must be of the same type
>>>>         * For ranges of ipa-ad-trust type or ipa-local type:
>>>>             * Primary RID ranges can not overlap
>>>>         * For ranges of ipa-local type:
>>>>             * Primary and secondary RID ranges can not overlap
>>>>             * Secondary RID ranges cannot overlap
>>>>
>>>> For the implementation part, the plugin was extended with a domain ID
>>>> to forest root domain ID mapping derivation capabilities.
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/4137
>>>>
>>>> -static int slapi_entry_to_range_info(struct slapi_entry *entry,
>>>> +struct domain_info {
>>>> +    char *domain_id;
>>>> +    char *forest_root_id;
>>>> +    struct domain_info *next;
>>>> +};
>>>> +
>>>> +static void free_domain_info(struct domain_info *info) {
>>>> +    if (info != NULL) {
>>>> +        slapi_ch_free_string(&(info->domain_id));
>>>> +        slapi_ch_free_string(&(info->forest_root_id));
>>>> +        free_domain_info(info->next);
>>>> +        free(info);
>>>> +    }
>>>> +}
>>> Please, don't use recursion in the freeing part, there is really no
>>> pressing need to do so. Just use while() like you do in
>>> get_forest_root_id():
>>>
>>>> +/* Searches for the domain_info struct with the specified domain_id
>>>> + * in the linked list. Returns the forest root domain's ID, or NULL for
>>>> + * local ranges. */
>>>> +
>>>> +static char* get_forest_root_id(struct domain_info *head, char*
>>>> domain_id) {
>>>> +
>>>> +    /* For local ranges there is no forest root domain,
>>>> +     * so consider only ranges with domain_id set */
>>>> +    if (domain_id != NULL) {
>>>> +        while(head) {
>>>> +            if (strcasecmp(head->domain_id, domain_id) == 0) {
>>>> +                return head->forest_root_id;
>>>> +            }
>>>> +            head = head->next;
>>>> +        }
>>>> +     }
>>>> +
>>>> +    return NULL;
>>>> +}
>>>> +
>>>
>>>
>>
>> Fixed, updated patch attached.
>>
>
> Pushed to master based on Alexander's ACK in patch 161.


I keep seeing a test failure that's probably caused by this change:

======================================================================
ERROR: test suite for <class 
'ipatests.test_xmlrpc.test_range_plugin.test_range'>
----------------------------------------------------------------------
Traceback (most recent call last):
   File "/usr/lib/python2.7/site-packages/nose/suite.py", line 208, in run
     self.setUp()
   File "/usr/lib/python2.7/site-packages/nose/suite.py", line 291, in setUp
     self.setupContext(ancestor)
   File "/usr/lib/python2.7/site-packages/nose/suite.py", line 314, in 
setupContext
     try_run(context, names)
   File "/usr/lib/python2.7/site-packages/nose/util.py", line 469, in 
try_run
     return func()
   File 
"/var/lib/jenkins/workspace/freeipa-intree-tests-f20/ipatests/test_xmlrpc/test_range_plugin.py", 
line 163, in setUpClass
     cls.mockldap.add_entry(testrange9_dn, testrange9_add)
   File 
"/var/lib/jenkins/workspace/freeipa-intree-tests-f20/ipatests/util.py", 
line 650, in add_entry
     self.connection.add_s(dn, ldif)
   File "/usr/lib64/python2.7/site-packages/ldap/ldapobject.py", line 
195, in add_s
     return self.result(msgid,all=1,timeout=self.timeout)
   File "/usr/lib64/python2.7/site-packages/ldap/ldapobject.py", line 
458, in result
     resp_type, resp_data, resp_msgid = self.result2(msgid,all,timeout)
   File "/usr/lib64/python2.7/site-packages/ldap/ldapobject.py", line 
462, in result2
     resp_type, resp_data, resp_msgid, resp_ctrls = 
self.result3(msgid,all,timeout)
   File "/usr/lib64/python2.7/site-packages/ldap/ldapobject.py", line 
469, in result3
     resp_ctrl_classes=resp_ctrl_classes
   File "/usr/lib64/python2.7/site-packages/ldap/ldapobject.py", line 
476, in result4
     ldap_result = 
self._ldap_call(self._l.result4,msgid,all,timeout,add_ctrls,add_intermediates,add_extop)
   File "/usr/lib64/python2.7/site-packages/ldap/ldapobject.py", line 
99, in _ldap_call
     result = func(*args,**kwargs)
OPERATIONS_ERROR: {'info': 'Range Check error', 'desc': 'Operations error'}


-- 
Petr³