[Freeipa-devel] [PATCH 0161] Fix dyndb-ldap working dir permission

[Jan Cholasta]

Dne 18.11.2014 v 16:53 Martin Basti napsal(a):
> On 18/11/14 15:01, Jan Cholasta wrote:
>> Hi,
>>
>> Dne 13.11.2014 v 14:50 Martin Basti napsal(a):
>>> On 13/11/14 13:59, Jan Cholasta wrote:
>>>> Dne 12.11.2014 v 13:33 Martin Basti napsal(a):
>>>>> On 11/11/14 16:58, Jan Cholasta wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Dne 11.11.2014 v 16:22 Martin Basti napsal(a):
>>>>>>> Using specfile to create file doesn't work if named user is not on
>>>>>>> system.
>>>>>>> Appropriate permission have to be set during ipa-dns installation.
>>>>>>>
>>>>>>> Patch attached
>>>>>>>
>>>>>>
>>>>>> Why is the directory set up in dnskeysyncinstance instead of
>>>>>> bindinstance?
>>>>> Because, dnskeysyncinstance is the daemon which requires permission
>>>>> change.
>>>>> (dir is created by dyndb-ldap plugin)
>>>>
>>>> OK. But please rename the method to something more suitable
>>>> (fix_dyndb_ldap_workdir_permissions?) and add a docstring/comment.
>>>>
>>>> Also please change the ticket link to
>>>> <https://fedorahosted.org/freeipa/ticket/4716> (cloned from BZ).
>>>>
>>>>>
>>>>>>
>>>>>> The original patch was released with 4.1.1, shouldn't there be update
>>>>>> in ipa-upgradeconfig?
>>>>> Cases:
>>>>> 1) fresh RPM install, no named user during RPM install -> named
>>>>> doesn't
>>>>> start, user had to fix it immediately, can't wait until next release.
>>>>>
>>>>> 2) fresh RPM install,  named user -> no impact
>>>>>
>>>>> 3) upgrade IPA with DNS -> no impact
>>>>>
>>>>> 4) upgrade IPA without DNS -> after DNS installation, same as 1)
>>>>>
>>>>> 5) IPA 4.1.0 with installed DNS, upgrade to 4.1.2 -> DNSSEC will not
>>>>> work (If user doesnt use DNSSEC)
>>>>>
>>>>> Only 5) looks serious for me, so here is updated patch.
>>>>
>>>> Could you do the update without the code duplication? In similar code
>>>> an appropriate *instance method is usually called.
>>
>> The uid/gid resolution in ipa-upgradeconfig still looks like
>> duplicated code to me. I would suggest doing something along these
>> lines in ipa-upgradeconfig:
>>
>>     dnskeysync = dnskeysyncinstance.DNSKeySyncInstance()
>>     dnskeysync.set_dyndb_ldap_workdir_permissions()
>>
>> and have DNSKeySyncInstance.set_dyndb_ldap_workdir_permissions() do
>> all the real work.
>
> Updated patch attached.
> Martin^2

Thanks, ACK.

Pushed to:
master: 7c176b708eb855ea8774ad36ba72fd31952a8895
ipa-4-1: ba124045b9f39f8264a974c977beba6f15b1b1fb

-- 
Jan Cholasta