[Freeipa-devel] [PATCH 0012-0012] more topology plugin fixes

[thierry bordaz]

On 06/11/2015 10:40 AM, Ludwig Krispenz wrote:
>
> On 06/11/2015 10:27 AM, thierry bordaz wrote:
>> On 06/11/2015 08:12 AM, Ludwig Krispenz wrote:
>>> Attached are two patches:
>>> - reject direct modification of segment endpoints and connectivity
>>> - better manage the rdn of a replication agreements represented by a 
>>> segment
>>>
>>>
>> Hello Ludwig,
>>
>> The patches looks good. Two questions:
>>
>>   * Patch 0012
>>                if (strcasecmp(agmt_rdn_str, topo_agmt->rdn)) {
>>     slapi_ch_free_string(&topo_agmt->rdn);
>>                     topo_agmt->rdn = slapi_ch_strdup(agmt_rdn_str);
>>                 }
>>     What is the benefit of replacing a string by the same one ?
>>
> if strcasecmp is not 0, they are not the same
Shame on me !

>>   * Patch 0013
>>     In ipa-topo-pre-mod.
>>         if (ipa_topo_is_entry_managed(pb)){
>>             if(ipa_topo_is_agmt_attr_restricted(pb)) {
>>                 errtxt = slapi_ch_smprintf("Entry and attributes are
>>     managed by topology plugin."
>>                                            "No direct modifications
>>     allowed.\n");
>>             }
>>         } else if (ipa_topo_check_connect_restrict(pb)) {
>>             errtxt = slapi_ch_smprintf("Modification of connectivity
>>     and segment nodes "
>>                                        " is not supported.\n");
>>         }
>>     If we have an external modify of replication agreement (managed
>>     by topology plugin), then it will not call
>>     'ipa_topo_check_connect_restrict'.
>>     And the modify will not be reject if for example it updates
>>     'ipaReplTopoSegmentDirection'.
>>
> this is not in an replication agreement. you cannot modify two entries 
> in the same mod. The first if catches mos to a repl agreement, the 
> second to a segment entry

Ok. Thanks for the explanations. To help reading you may add a comment 
saying the the first test is related to RA and the second to segments.

Other than that the fix is good for me. ACK


>>   * But I thought that this patch also wants to prevent external
>>     update of some connectivity attribute of the managed entries.
>>
>> Thanks
>> thierry
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150611/af53b04a/attachment.htm>