[Freeipa-devel] [PATCH 0118] Allow to disable service (in LDAP)

[Martin Basti]

On 22/09/14 19:30, Martin Basti wrote:
> On 19/09/14 14:47, Jan Cholasta wrote:
>> Dne 19.9.2014 v 13:33 Martin Basti napsal(a):
>>> On 02/09/14 11:59, Martin Basti wrote:
>>>> On 02/09/14 09:10, Jan Cholasta wrote:
>>>>> Hi,
>>>>>
>>>>> Dne 1.9.2014 v 16:57 Martin Basti napsal(a):
>>>>>> This patch allows to disable service in LDAP to prevents service 
>>>>>> to be
>>>>>> started by "ipactl restart"
>>>>>>
>>>>>> Required by DNSSEC
>>>>>>
>>>>>> Patch attached
>>>>>
>>>>> I don't think the extra argument in ldap_enable is necessary. It
>>>>> should enable the service no matter if the entry existed before or 
>>>>> not.
>>>>>
>>>>> Similarly, in ldap_disable you should not raise an error when the
>>>>> entry is not found, because that already makes the service disabled.
>>>>>
>>>>> Honza
>>>>>
>>>> Updated patch attached
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Freeipa-devel mailing list
>>>> Freeipa-devel at redhat.com
>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>> Please review, this should be in 4.1
>>>
>>> -- 
>>> Martin Basti
>>
>> 1) ipaConfigString is case-insensitive, so you need to look for the 
>> string "enabledService" in a case-insensitive way.
>>
>>
>> 2) Don't say "failed to enable ..." when the service is already 
>> enabled. It is not a failure. Same for disabled.
>>
>>
>> 3) Missing ipaConfigString is nothing to warn about.
>>
>>
>> 4) You should log success in both ldap_enable/ldap_disable.
>>
>>
>> 5) The except below update_entry should say "except 
>> errors.EmptyModlist".
>>
>>
> Thank you,
>
> updated patch attached
>
Updated patch attached. I modified ldap_disable to use search function.

-- 
Martin Basti

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0118.4-LDAP-disable-service.patch
Type: text/x-patch
Size: 3849 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140925/2c6340ec/attachment.bin>