[Freeipa-devel] [PATCHES 0018-0020] ipa-dns-install: Use LDAPI for all DS connections

[Martin Basti]

On 16/03/15 14:26, Martin Babinsky wrote:
> On 03/16/2015 01:44 PM, Martin Basti wrote:
>> On 12/03/15 17:15, Martin Babinsky wrote:
>>> On 03/12/2015 03:59 PM, Martin Babinsky wrote:
>>>> On 03/11/2015 03:13 PM, Martin Basti wrote:
>>>>> On 11/03/15 13:00, Martin Babinsky wrote:
>>>>>> These patches solve https://fedorahosted.org/freeipa/ticket/4933.
>>>>>>
>>>>>> They are to be applied to master branch. I will rebase them for
>>>>>> ipa-4-1 after the review.
>>>>>>
>>>>> Thank you for the patches.
>>>>>
>>>>> I have a few comments:
>>>>>
>>>>> IPA-4-1
>>>>> Replace simple bind with LDAPI is too big change for 4-1, we should
>>>>> start TLS if possible to avoid MINSSF>0 error. The LDAPI patches 
>>>>> should
>>>>> go only into IPA master branch.
>>>>>
>>>>> You can do something like this:
>>>>> --- a/ipaserver/install/service.py
>>>>> +++ b/ipaserver/install/service.py
>>>>> @@ -107,6 +107,10 @@ class Service(object):
>>>>>                   if not self.realm:
>>>>>                       raise errors.NotFound(reason="realm is missing
>>>>> for
>>>>> %s" % (self))
>>>>>                   conn = ipaldap.IPAdmin(ldapi=self.ldapi,
>>>>> realm=self.realm)
>>>>> +            elif self.dm_password is not None:
>>>>> +                conn = ipaldap.IPAdmin(self.fqdn, port=389,
>>>>> + cacert=paths.IPA_CA_CRT,
>>>>> +                                       start_tls=True)
>>>>>               else:
>>>>>                   conn = ipaldap.IPAdmin(self.fqdn, port=389)
>>>>>
>>>>>
>>>>> PATCH 0018:
>>>>> 1)
>>>>> please add there more chatty commit message about using LDAPI
>>>>>
>>>>> 2)
>>>>> I do not like much idea of adding 'realm' kwarg into __init__ 
>>>>> method of
>>>>> OpenDNSSECInstance
>>>>> IIUC, it is because get_masters() method, which requires realm to use
>>>>> LDAPI.
>>>>>
>>>>> You can just add ods.realm=<realm>, before call get_master() in
>>>>> ipa-dns-install
>>>>>      if options.dnssec_master:
>>>>> +        ods.realm=api.env.realm
>>>>>          dnssec_masters = ods.get_masters()
>>>>> (Honza will change it anyway during refactoring)
>>>>>
>>>>> PATCH 0019:
>>>>> 1)
>>>>> commit message deserves to be more chatty, can you explain there why
>>>>> you
>>>>> removed kerberos cache?
>>>>>
>>>>> Martin^2
>>>>>
>>>>
>>>> Attaching updated patches.
>>>>
>>>> Patch 0018 should go to both 4.1 and master branches.
>>>>
>>>> Patch 0019 should go only to master.
>>>>
>>>>
>>>>
>>>
>>> One more update.
>>>
>>> Patch 0018 is for both 4.1 and master.
>>> Patch 0019 is for master only.
>>>
>>>
>>>
>> Thank for patches
>> Patch 0018:
>> 1)
>> Works for me but needs rebase on master
>>
>> Patch 0019:
>> 1)
>> Please rename the patch/commit message, the patch changes only
>> ipa-dns-install connections not all DS operations
>>
>> 2)
>> I have some troubles with applying patch, it needs rebase due 0018
>>
>>
>> -- 
>> Martin Basti
>>
>
> Attaching updated patches:
>
> Patch 0018 is for ipa-4-1 branch.
> Patches 0019 and 0020 are for master branch.
>
> I hope they will apply cleanly this time (they did for me).
>
ACK

-- 
Martin Basti