Re: [Freeipa-devel] [PATCH 0024] do not log BINDs to non-existent users as errors
Dne 30.3.2015 v 14:10 Petr Spacek napsal(a): On 25.3.2015 17:07, Martin Babinsky wrote: https://fedorahosted.org/freeipa/ticket/4889 ACK Pushed to: master: 4192cce80eb22172696b11bf24457f7467b711fc ipa-4-1: ede3298fdf8092567b7cfec4053c0db45725f882 -- Jan Cholasta -- 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
Re: [Freeipa-devel] [PATCH 0222] DNSSEC: do not log into files
Dne 30.3.2015 v 13:55 Petr Spacek napsal(a): On 26.3.2015 16:33, Martin Basti wrote: We want to log DNSSEC daemons only into console (journald). This patch also fixes unexpected log file in /var/lib/softhsm/.ipa/log/default.log Patch attached. ACK Pushed to: master: 1216da8b9f2100cacebbeb8fe2dd91e22b954ba7 ipa-4-1: e27b9d18cee86b7634a0ec23042985c23096098e -- Jan Cholasta -- 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
Re: [Freeipa-devel] [PATCH] 809 speed up convert_attribute_members
Hi, Dne 31.3.2015 v 12:11 Petr Vobornik napsal(a): A workaround to avoid usage of slow LDAPEntry._sync_attr #4946. I originally wanted to avoid DN processing as well but we can't do that because of DNs which are encoded - e.g. contains '+' or ','. Therefore patch 811 - faster DN implementation is very useful. Also patch 809 is useful to avoid high load of 389. https://fedorahosted.org/freeipa/ticket/4965 1) +dn = container_dns.get(ldap_obj_name, None) +if not dn: +ldap_obj = self.api.Object[ldap_obj_name] +dn = DN(ldap_obj.container_dn, api.env.basedn) +container_dns[ldap_obj_name] = dn +return dn a) The second argument of .get() is None by default b) not dn matches None as well as empty DNs, use dn is not None (it's not that there could be empty DNs here, but let's not give a potential reader the wrong idea) c) It would be better to catch KeyError rather than call .get() and check the result: try: dn = container_dns[ldap_obj_name] except KeyError: dn = ... container_dns[ldap_obj_name] = dn 2) Does get_new_attr() actually provide any speed up? Unless I'm missing something, it just mirrors the virtual member attributes already readily available from entry_attrs in new_attrs. 3) get_container_dn() and get_new_attr() do not need to be functions, since each is called just from a single spot. 4) memberdn = DN(member) could be one for loop up. Here's what I ended up with trying to fix the above (untested): for attr in self.attribute_members: try: value = entry_attrs.raw[attr] except KeyError: continue del entry_attrs[attr] ldap_objs = {} for ldap_obj_name in self.attribute_members[attr]: ldap_obj = self.api.Object[ldap_obj_name] container_dn = DN(ldap_obj.container_dn, api.env.basedn) ldap_objs[container_dn] = ldap_obj for member in value: memberdn = DN(member) try: ldap_obj = ldap_objs[DN(*memberdn[1:])] except KeyError: continue new_attr = '%s_%s' % (attr, ldap_obj.name) new_value = ldap_obj.get_primary_key_from_dn(memberdn) entry_attrs.setdefault(new_attr, []).append(new_value) Honza -- Jan Cholasta -- 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
Re: [Freeipa-devel] [PATCH] 811 performance: faster DN implementation
On 03/31/2015 12:11 PM, Petr Vobornik wrote: The only different thing is a lack of utf-8 encoded str support(as input). I don't know how much important the support is. I don't think that support is too important (assuming IPA doesn't use it!). However, the behavior with this patch is dangerous. It allows unicode and ASCII strings, but fails on non-ASCII strings. That means things will usually work, but as soon as a non-ASCII component is introduced at the wrong place, you get an error. Restoring support for utf-8 encoded str looks easy to do; here's a patch you can squash in. Or did I miss something? maybe it could be attached to ticket https://fedorahosted.org/freeipa/ticket/4947 - DN code was optimized to be faster if DNs are created from string. This is the major use case, since most DNs come from LDAP. With this patch, DN creation is almost 8-10x faster (with 30K-100K DNs). Second mojor use case - deepcopy in LDAPEntry is about 20x faster - done by custom __deepcopy__ function. The major change is that DN is no longer internally composed of RDNs and AVAs but it rather keeps the data in open ldap format - the same as output of str2dn function. Therefore, for immutable DNs, no other transformations are required on instantiation. The format is: DN: [RDN, RDN,...] RDN: [AVA, AVA,...] AVA: ['utf-8 encoded str - attr', 'utf-8 encode str -value', FLAG] FLAG: int Further indexing of DN object constructs an RDN which is just an encapsulation of the RDN part of open ldap representation. Indexing of RDN constructs AVA in the same fashion. Obtained EditableAVA, EditableRDN from EditableDN shares the respected lists of the open ldap repr. so that the change of value or attr is reflected in parent object. Looks good. A couple of comments: RDN.to_openldap: _avas always has 3 components, right? I'd prefer `list(a)` over `[a[0], a[1], a[2]]`. Similarly for tuple in in __add__ and RDN._avas_from_sequence. DN._rdns_from_value: the error message at the end is wrong, RDN is also accepted. (And, `type(value)` would be more informative than `value.__class__.__name__`.) You can optimize __deepcopy__ for immutable DNs even further: just return self! In DN.find rfind, RDNs are not accepted but the error message says they are. You removed the newline at end of file. -- Petr Viktorin From 1461bc75748d5bdcf301242c3e31c044542f5bdd Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Thu, 2 Apr 2015 10:40:15 +0200 Subject: [PATCH] Restore support for creating DNs from utf-8 encoded strings --- ipapython/dn.py| 4 ++- ipatests/test_ipapython/test_dn.py | 60 +++--- 2 files changed, 33 insertions(+), 31 deletions(-) diff --git a/ipapython/dn.py b/ipapython/dn.py index 5d1f15c..dc536ce 100644 --- a/ipapython/dn.py +++ b/ipapython/dn.py @@ -1225,7 +1225,9 @@ def _copy_rdns(self, rdns=None): def _rdns_from_value(self, value): if isinstance(value, basestring): try: -rdns = str2dn(value.encode('utf-8')) +if isinstance(value, unicode): +value = value.encode('utf-8') +rdns = str2dn(value) if self.is_mutable: self._copy_rdns(rdns) # AVAs to be list instead of tuple except DECODING_ERROR: diff --git a/ipatests/test_ipapython/test_dn.py b/ipatests/test_ipapython/test_dn.py index 2a7739b..394be5b 100644 --- a/ipatests/test_ipapython/test_dn.py +++ b/ipatests/test_ipapython/test_dn.py @@ -1864,11 +1864,11 @@ def test_i18n(self): self.assertEqual(ava1.attr, self.arabic_hello_unicode) self.assertEqual(str(ava1), self.arabic_hello_utf8+'=foo') -# ava1 = AVA_class(self.arabic_hello_utf8, 'foo') -# self.assertIsInstance(ava1.attr, unicode) -# self.assertIsInstance(ava1.value, unicode) -# self.assertEqual(ava1.attr, self.arabic_hello_unicode) -# self.assertEqual(str(ava1), self.arabic_hello_utf8+'=foo') +ava1 = AVA_class(self.arabic_hello_utf8, 'foo') +self.assertIsInstance(ava1.attr, unicode) +self.assertIsInstance(ava1.value, unicode) +self.assertEqual(ava1.attr, self.arabic_hello_unicode) +self.assertEqual(str(ava1), self.arabic_hello_utf8+'=foo') # test value i18n ava1 = AVA_class('cn', self.arabic_hello_unicode) @@ -1877,11 +1877,11 @@ def test_i18n(self): self.assertEqual(ava1.value, self.arabic_hello_unicode) self.assertEqual(str(ava1), 'cn='+self.arabic_hello_utf8) -# ava1 = AVA_class('cn', self.arabic_hello_utf8) -# self.assertIsInstance(ava1.attr, unicode) -# self.assertIsInstance(ava1.value, unicode) -# self.assertEqual(ava1.value, self.arabic_hello_unicode) -# self.assertEqual(str(ava1),
Re: [Freeipa-devel] [PATCH 0212] Server Upgrade: Fix comments
Dne 24.3.2015 v 14:23 David Kupka napsal(a): On 03/24/2015 12:04 PM, Martin Basti wrote: On 24/03/15 09:54, Martin Basti wrote: On 23/03/15 15:36, Martin Basti wrote: Attached patch fixes comments which I forgot to edit in 'make upgrade deterministic' patchset I missed some dictionaries which should be lists. Updated patch attached. -- Martin Basti Updated patch attached Thanks for the patch, LGTM, ACK. Pushed to master: b5e941d49b3571a3f257be645dabb429754c94b0 -- Jan Cholasta -- 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
Re: [Freeipa-devel] [PATCH] 0001-2 ipatests: SOA record Maintenance tests
Dne 27.3.2015 v 16:54 Martin Basti napsal(a): On 27/03/15 16:34, Aleš Mareček wrote: Greetings! Martin, thanks for your review and comments! I changed the name of the patch and setup my git variables properly. I also re-tested it and got all passed. I'm sending a new patch that is attached. - Original Message - From: Martin Basti mba...@redhat.com To: Aleš Mareček amare...@redhat.com, freeipa-devel@redhat.com Sent: Tuesday, March 24, 2015 4:39:21 PM Subject: Re: [Freeipa-devel] [PATCH] 0001 ipatests: SOA record Maintenance tests On 24/03/15 15:06, Aleš Mareček wrote: Greetings! This is my very first patch, ticket#4746. Have a nice day! - alich - Thank you for the patch. Just nitpicks: 1) +cleanup_commands = [ +('dnszone_del', [zone6], {'continue': True}), +('dnszone_del', [zone6b], {'continue': True}), +] would be better do it in this way, continue option will to try remove all zones: +cleanup_commands = [ +('dnszone_del', [zone6, zone6b], {'continue': True}), +] Done. 2) I'm fine with zone6b, but was there any reason to create zone6b, instead of reusing zone 1 or 2 or 3? Because of some updates needs, I didn't want to break anything existing thus I created new. 3) Please fix whitespace errors. $ git am freeipa-alich-0001-ipatests-added-tests-for-SOA-record-Maintenance.patch Applying: ipatests - added tests for SOA record Maintenance /home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:482: trailing whitespace. /home/mbasti/work/freeipa-devel/.git/rebase-apply/patch:758: new blank line at EOF. + warning: 2 lines add whitespace errors. Done. $ git am freeipa-alich-0001-2-Ipatests-DNS-SOA-Record-Maintenance.patch Applying: Ipatests DNS SOA Record Maintenance $ 4) I know the dns plugin tests are so far from PEP8, but try to keep PEP8 in new code Done, only 1 line persisted that I didn't want to break: zone6_unresolvable_ns_relative_dnsname = DNSName(zone6_unresolvable_ns_relative) Otherwise test works as expected. Martin^2 -- Martin Basti Thanks! - alich - Thank you, ACK. Pushed to: master: ca96ecbf40038d09814f99f19bf47246352dfa0c ipa-4-1: 8f94ac1e7c24b3bf33c5211d3e327c9a51390fb1 -- Jan Cholasta -- 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
Re: [Freeipa-devel] [PATCH 0021] fix improper handling of boolean option during KRA install
Dne 25.3.2015 v 16:48 Martin Basti napsal(a): On 17/03/15 17:51, Martin Babinsky wrote: https://fedorahosted.org/freeipa/ticket/4530 ACK -- Martin Basti Pushed to master: c311af06f60cfdb73be9c0aecb9ddc559db1a055 -- Jan Cholasta -- 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
Re: [Freeipa-devel] [PATCH 0223] Fix ldap2 do not create shared instance by default
On 02/04/15 14:11, Jan Cholasta wrote: Hi, Dne 1.4.2015 v 17:16 Martin Basti napsal(a): Since API is not singleton anymore, ldap2 instance should not be shared between all APIs. Patch attached. Works for me. However, it's not the ldap2 instance that was shared, but rather the underlying LDAP connection. Honza Reworded patch attached. -- Martin Basti From fde4f6434fa746c69ca97b68bbf7e8d08448c21e Mon Sep 17 00:00:00 2001 From: Martin Basti mba...@redhat.com Date: Wed, 25 Mar 2015 15:34:16 +0100 Subject: [PATCH] Fix ldap2 shared connection Since API is not singleton anymore, ldap2 connections should not be shared by default. --- ipalib/backend.py| 2 +- ipaserver/plugins/ldap2.py | 2 +- ipatests/test_ipalib/test_backend.py | 12 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/ipalib/backend.py b/ipalib/backend.py index 4c1001d4d47613537b64c314a2d22769a27f4c69..fcbbd254afc797019e9ea63214b1ee034b8c13f8 100644 --- a/ipalib/backend.py +++ b/ipalib/backend.py @@ -46,7 +46,7 @@ class Connectible(Backend): `request.destroy_context()` can properly close all open connections. -def __init__(self, shared_instance=True): +def __init__(self, shared_instance=False): Backend.__init__(self) if shared_instance: self.id = self.name diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py index 3211b3390fb979f090467445905513d33e537e17..fd4ed29903fb2f3afe0f4b74467bf53df49654fa 100644 --- a/ipaserver/plugins/ldap2.py +++ b/ipaserver/plugins/ldap2.py @@ -61,7 +61,7 @@ class ldap2(LDAPClient, CrudBackend): LDAP Backend Take 2. -def __init__(self, shared_instance=True, ldap_uri=None, base_dn=None, +def __init__(self, shared_instance=False, ldap_uri=None, base_dn=None, schema=None): self.__ldap_uri = None diff --git a/ipatests/test_ipalib/test_backend.py b/ipatests/test_ipalib/test_backend.py index c69757cb3d68ebc12f9c91572d37603738357c4e..121c4745bd1dfebfbeed75ba1b46b4420064fe63 100644 --- a/ipatests/test_ipalib/test_backend.py +++ b/ipatests/test_ipalib/test_backend.py @@ -76,7 +76,7 @@ class test_Connectible(ClassChecker): object.__setattr__(self, 'args', args) object.__setattr__(self, 'kw', kw) return 'The connection.' -o = example() +o = example(shared_instance=True) args = ('Arg1', 'Arg2', 'Arg3') kw = dict(key1='Val1', key2='Val2', key3='Val3') assert not hasattr(context, 'example') @@ -104,7 +104,7 @@ class test_Connectible(ClassChecker): class example(self.cls): pass for klass in (self.cls, example): -o = klass() +o = klass(shared_instance=True) e = raises(NotImplementedError, o.create_connection) assert str(e) == '%s.create_connection()' % klass.__name__ @@ -114,7 +114,7 @@ class test_Connectible(ClassChecker): class example(self.cls): destroy_connection = Disconnect() -o = example() +o = example(shared_instance=True) m = disconnect: 'context.%s' does not exist in thread %r e = raises(StandardError, o.disconnect) @@ -131,7 +131,7 @@ class test_Connectible(ClassChecker): class example(self.cls): pass for klass in (self.cls, example): -o = klass() +o = klass(shared_instance=True) e = raises(NotImplementedError, o.destroy_connection) assert str(e) == '%s.destroy_connection()' % klass.__name__ @@ -142,7 +142,7 @@ class test_Connectible(ClassChecker): class example(self.cls): pass for klass in (self.cls, example): -o = klass() +o = klass(shared_instance=True) assert o.isconnected() is False conn = 'whatever' setattr(context, klass.__name__, conn) @@ -157,7 +157,7 @@ class test_Connectible(ClassChecker): class example(self.cls): pass for klass in (self.cls, example): -o = klass() +o = klass(shared_instance=True) e = raises(AttributeError, getattr, o, 'conn') assert str(e) == msg % ( klass.__name__, threading.currentThread().getName() -- 2.1.0 -- 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
Re: [Freeipa-devel] [PATCH 0223] Fix ldap2 do not create shared instance by default
Dne 2.4.2015 v 14:18 Martin Basti napsal(a): On 02/04/15 14:11, Jan Cholasta wrote: Hi, Dne 1.4.2015 v 17:16 Martin Basti napsal(a): Since API is not singleton anymore, ldap2 instance should not be shared between all APIs. Patch attached. Works for me. However, it's not the ldap2 instance that was shared, but rather the underlying LDAP connection. Honza Reworded patch attached. Thanks, ACK. Pushed to master: b92136cba287e38d9c2f41c3163f5a6b0b62ca17 -- Jan Cholasta -- 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
[Freeipa-devel] [PATCH] 0688 rename_managed: Remove use of EditableDN
This removes the last use of dn.Editable* from IPA code. For cases where an EditableDN was used, lists/generators of RDNs tend to work better IMO. When this is merged, Editable* can be removed (which I don't want to do right now since there's a patch on review that would conflict). -- Petr Viktorin From 16fee8dd83d71789e97e85a88993d499cca34d9c Mon Sep 17 00:00:00 2001 From: Petr Viktorin pvikt...@redhat.com Date: Wed, 1 Apr 2015 16:37:51 +0200 Subject: [PATCH] rename_managed: Remove use of EditableDN This was the last use of EditableDN in IPA; the class can now be removed. --- ipaserver/install/plugins/rename_managed.py | 51 ++--- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/ipaserver/install/plugins/rename_managed.py b/ipaserver/install/plugins/rename_managed.py index adb814c1799ebbdb57118acf1ba6a52550f2f818..c5e701a588bf34b0f86de197859e9c55e8db5dd0 100644 --- a/ipaserver/install/plugins/rename_managed.py +++ b/ipaserver/install/plugins/rename_managed.py @@ -21,7 +21,7 @@ from ipaserver.install.plugins.baseupdate import PreUpdate, PostUpdate from ipalib import api, errors from ipapython import ipautil -from ipapython.dn import DN, EditableDN +from ipapython.dn import DN def entry_to_update(entry): @@ -41,7 +41,21 @@ def entry_to_update(entry): return update + class GenerateUpdateMixin(object): +def _dn_replace(self, dn, old, new): +Replace all occurences of old AVAs in a DN by new + +If a replacement is made, return the resulting DN. +Otherwise, log, an raise ValueError. + +new_dn = DN(new if ava == old else ava for ava in dn) +if new_dn == dn: +self.error(unable to replace '%s' with '%s' in '%s', + old, new, dn) +raise ValueError('no replacement made') +return new_dn + def generate_update(self, deletes=False): We need to separate the deletes that need to happen from the @@ -82,14 +96,13 @@ def generate_update(self, deletes=False): pass else: # Compute the new dn by replacing the old container with the new container -new_dn = EditableDN(entry.dn) -if new_dn.replace(old_template_container, new_template_container) != 1: -self.error(unable to replace '%s' with '%s' in '%s', - old_template_container, new_template_container, entry.dn) +try: +new_dn = self._dn_replace(entry.dn, + old=old_template_container, + new=new_template_container) +except ValueError: continue -new_dn = DN(new_dn) - # The old attributes become defaults for the new entry new_update = {'dn': new_dn, 'default': entry_to_update(entry)} @@ -103,23 +116,21 @@ def generate_update(self, deletes=False): else: # Update the template dn by replacing the old containter with the new container -old_dn = entry['managedtemplate'][0] -new_dn = EditableDN(old_dn) -if new_dn.replace(old_template_container, new_template_container) != 1: -self.error(unable to replace '%s' with '%s' in '%s', - old_template_container, new_template_container, old_dn) +try: +new_dn = self._dn_replace(entry['managedtemplate'][0], + old=old_template_container, + new=new_template_container) +except ValueError: continue -new_dn = DN(new_dn) entry['managedtemplate'] = new_dn -# Edit the dn, then convert it back to an immutable DN -old_dn = entry.dn -new_dn = EditableDN(old_dn) -if new_dn.replace(old_definition_container, new_definition_container) != 1: -self.error(unable to replace '%s' with '%s' in '%s', - old_definition_container, new_definition_container, old_dn) +# Update the entry dn similarly +try: +new_dn = self._dn_replace(entry.dn, + old=old_definition_container, + new=new_definition_container) +except ValueError: continue -new_dn = DN(new_dn) # The old attributes become defaults for the new entry new_update = {'dn': new_dn, -- 2.1.0 -- Manage your subscription for the
Re: [Freeipa-devel] [PATCH 0045] Add message for skipping NTP configuration during client install
On Thu, Apr 2, 2015 at 8:59 AM, Martin Basti mba...@redhat.com wrote: On 30/03/15 15:25, Gabe Alford wrote: Hello, With the merging of ticket 4842 https://fedorahosted.org/freeipa/ticket/4842, I believe that half of ticket 3092 https://fedorahosted.org/freeipa/ticket/3092 has been done. This patch just adds a message that says that NTP configuration was skipped which I believe should finish 3092 https://fedorahosted.org/freeipa/ticket/3092. Thanks, Gabe Hello, thank you for the patch. 1) IMO there should be: if *not* options.conf_ntp So, if --no-ntp is not specified, print message that the client is skipping NTP sync? 2) wouldnt be better to use just else? I actually ran ipa-client-install with no options on a system where I used 'else', and it printed the skipping NTP sync when it should not have. That is why the patch does not use 'else'. Martin -- Martin Basti -- 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
Re: [Freeipa-devel] [PATCH 0045] Add message for skipping NTP configuration during client install
On 30/03/15 15:25, Gabe Alford wrote: Hello, With the merging of ticket 4842 https://fedorahosted.org/freeipa/ticket/4842, I believe that half of ticket 3092 https://fedorahosted.org/freeipa/ticket/3092 has been done. This patch just adds a message that says that NTP configuration was skipped which I believe should finish 3092 https://fedorahosted.org/freeipa/ticket/3092. Thanks, Gabe Hello, thank you for the patch. 1) IMO there should be: if *not* options.conf_ntp 2) wouldnt be better to use just else? Martin -- Martin Basti -- 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