<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 12/03/15 17:15, Martin Babinsky
wrote:<br>
</div>
<blockquote cite="mid:5501BBB7.2040709@redhat.com" type="cite">On
03/12/2015 03:59 PM, Martin Babinsky wrote: <br>
<blockquote type="cite">On 03/11/2015 03:13 PM, Martin Basti
wrote: <br>
<blockquote type="cite">On 11/03/15 13:00, Martin Babinsky
wrote: <br>
<blockquote type="cite">These patches solve <a
class="moz-txt-link-freetext"
href="https://fedorahosted.org/freeipa/ticket/4933">https://fedorahosted.org/freeipa/ticket/4933</a>.
<br>
<br>
They are to be applied to master branch. I will rebase them
for <br>
ipa-4-1 after the review. <br>
<br>
</blockquote>
Thank you for the patches. <br>
<br>
I have a few comments: <br>
<br>
IPA-4-1 <br>
Replace simple bind with LDAPI is too big change for 4-1, we
should <br>
start TLS if possible to avoid MINSSF>0 error. The LDAPI
patches should <br>
go only into IPA master branch. <br>
<br>
You can do something like this: <br>
--- a/ipaserver/install/service.py <br>
+++ b/ipaserver/install/service.py <br>
@@ -107,6 +107,10 @@ class Service(object): <br>
if not self.realm: <br>
raise errors.NotFound(reason="realm is
missing for <br>
%s" % (self)) <br>
conn = ipaldap.IPAdmin(ldapi=self.ldapi, <br>
realm=self.realm) <br>
+ elif self.dm_password is not None: <br>
+ conn = ipaldap.IPAdmin(self.fqdn, port=389, <br>
+
cacert=paths.IPA_CA_CRT, <br>
+ start_tls=True) <br>
else: <br>
conn = ipaldap.IPAdmin(self.fqdn, port=389)
<br>
<br>
<br>
PATCH 0018: <br>
1) <br>
please add there more chatty commit message about using LDAPI
<br>
<br>
2) <br>
I do not like much idea of adding 'realm' kwarg into __init__
method of <br>
OpenDNSSECInstance <br>
IIUC, it is because get_masters() method, which requires realm
to use <br>
LDAPI. <br>
<br>
You can just add ods.realm=<realm>, before call
get_master() in <br>
ipa-dns-install <br>
if options.dnssec_master: <br>
+ ods.realm=api.env.realm <br>
dnssec_masters = ods.get_masters() <br>
(Honza will change it anyway during refactoring) <br>
<br>
PATCH 0019: <br>
1) <br>
commit message deserves to be more chatty, can you explain
there why you <br>
removed kerberos cache? <br>
<br>
Martin^2 <br>
<br>
</blockquote>
<br>
Attaching updated patches. <br>
<br>
Patch 0018 should go to both 4.1 and master branches. <br>
<br>
Patch 0019 should go only to master. <br>
<br>
<br>
<br>
</blockquote>
<br>
One more update. <br>
<br>
Patch 0018 is for both 4.1 and master. <br>
Patch 0019 is for master only. <br>
<br>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
</blockquote>
Thank for patches<br>
Patch 0018:<br>
1)<br>
Works for me but needs rebase on master<br>
<br>
Patch 0019:<br>
1)<br>
Please rename the patch/commit message, the patch changes only
ipa-dns-install connections not all DS operations<br>
<br>
2)<br>
I have some troubles with applying patch, it needs rebase due 0018<br>
<br>
<br>
<pre class="moz-signature" cols="72">--
Martin Basti</pre>
</body>
</html>