On 03/16/2015 05:01 PM, Martin Basti wrote:
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

Pushed to ipa-4-1 and master.

Please do not bump the base patch number for different (rebased) versions of the same patch.

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to