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

[Martin Basti]

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

>
>>>
>>>>
>>>> Martin^2
>>>>>
>>>>> Honza
>>>>>
>>>>
>>>>
>>>
>>> Honza
>>>
>> Thanks.
>> updated patch attached.
>> Martin^2
>>
>
> Honza
>


-- 
Martin Basti

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0161.3-Fix-named-working-directory-permissions.patch
Type: text/x-patch
Size: 7495 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141118/91d43166/attachment.bin>