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

[Petr Viktorin]

On 07/14/2014 11:43 AM, Tomas Babej wrote:
>
> On 07/12/2014 08:22 PM, Lukas Slebodnik wrote:
>> On (01/04/14 10:52), 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.
>>>
>>> --
>>> Tomas Babej
>>> Associate Software Engineer | Red Hat | Identity Management
>>> RHCE | Brno Site | IRC: tbabej | freeipa.org
>>>
>>> >From f98082d6cfd902417af94d7cd22d3cc85980a782 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
>>> ---
>> //snip
>>
>>> -        no_overlap = ranges_overlap(new_range, old_range);
>>> +        ranges_valid = check_ranges(new_range, old_range);
>>>          free_range_info(old_range);
>>>          old_range = NULL;
>>> -        if (no_overlap != 0) {
>>> +        if (ranges_valid != 0) {
>>>              ret = LDAP_CONSTRAINT_VIOLATION;
>>>
>>> -            switch (no_overlap){
>>> +            switch (ranges_valid){
>>>              case 1:
>>>                  errmsg = "New base range overlaps with existing base range.";
>>>                  break;
>>> @@ -417,6 +627,8 @@ static int ipa_range_check_pre_op(Slapi_PBlock *pb, int modtype)
>>>              case 5:
>>>                  errmsg = "New secondary rid range overlaps with existing primary rid range.";
>>>                  break;
>>> +            case 6:
>>> +                errmsg = "New ID range has invalid type. All ranges in the same domain must be of the same type.";
>>>              default:
>> There is a missing break. Ithink it is a problem, because you do not
>> want to fall through to the defaul and override errmsg.
>>
>> Smple patch is attached.
>>
>> LS
>
> ACK, thanks Lukas!
>

Pushed to master, ipa-4-1, ipa-4-0: d1d253637535017fdf82aa833df742bb418bbf65


-- 
Petr³