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

[Martin Babinsky]

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).

-- 
Martin^3 Babinsky
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbabinsk-0018-3-ipa-dns-install-use-STARTTLS-to.patch
Type: text/x-patch
Size: 8091 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150316/31305cc4/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbabinsk-0019-4-ipa-dns-install-use-STARTTLS-to.patch
Type: text/x-patch
Size: 8133 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150316/31305cc4/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbabinsk-0020-1-ipa-dns-install-use-LDAPI-to.patch
Type: text/x-patch
Size: 10441 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150316/31305cc4/attachment-0002.bin>