Re: [Freeipa-devel] [PATCH] Improve modlist generation in ldap2. Some code cleanup as bonus.
On Tue, 2010-01-05 at 15:01 +0100, Pavel Zuna wrote: > ldap2._generate_modlist now uses more sophisticated means to decide when to > use > MOD_ADD+MOD_DELETE instead of MOD_REPLACE. Before it did MOD_REPLACE only on > attributes explicitly specified in ldap2._FORCE_REPLACE_ON_UPDATE_ATTRS. Now > it > does MOD_REPLACE for all single value attributes and never for multi value. > > This patch also silently fixes a bug: ldap2 didn't check for the existence of > attributes that were being deleted by setting them to None. > > Pavel ack. pushed to master. This patch looks fine and doesn't appear to break anything, but we *really* need tests for ldap2. It's low in our stack and almost every plugin uses it, so problems here have a high cost for us time-wise. So, Pavel, please provide tests in subsequent patch. I think this modlist functionality should be split out into functions that can be tested easily without requiring an LDAP connection. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Allow creation of new connections by unshared instances of backend.Connectible.
Pavel Zuna wrote: Jason Gerard DeRose wrote: On Tue, 2010-01-05 at 14:10 +0100, Pavel Zuna wrote: The backend.Connectible base class was designed, so that only one instance of each subclass is used at a time. Connectible generates a Connection object for each thread and stores it in thread-local storage (context). Subclasses access this object through the Connectible.conn property. This is a good thing, because one instance of the class can be shared by all threads and each thread has its own connection. Unfortunately, this is also a limitation. If a thread needs a second connection (to a different host for example) - it can't do it. Not even by creating a new instance of the Connectible subclass. Ok, let's move from theory to practice: The LDAP backend is currently only used by the Executioner backend, so that plugins can connect to the IPA DS. In the migration plugin, we need a second connection to the DS we're migrating from. The last version had to use low level python-ldap calls to achieve this. In the installer we're still using legacy code from v1. Using ldap2 would be simpler and we could drop ~1000 lines code. (I already started rewriting a few parts to see if it would work.) Proposed solution: Make it possible to create unshared instances of Connectible subclasses. This would be achieved by passing shared_instance=False (couldn't come up with a better name) to the object constructor explicitly. Normally, Connection objects are stored in thread-local storage under the subclass name (e.g. "ldap2"). Unshared instances would store their Connection objects under subclass name + unique instances ID (e.g. "ldap2_218adsfka7"). This is the only solution I could come up with, that doesn't involve breaking a lot of stuff - it just adds a new way of using the code we already have. The attached patches show how it would be done. Pavel I'm fine with this approach as the solution you propose is quite unobtrusive. Is this the final patch then, or will you make further changes or bundle it with another patch? Yeah, this is the final patch. Upcoming patches will depend on it. Pavel pushed to master ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] jderose 033 Fix fuzzy digigits under Fedora12
I'm not sure why the difference, but the uidnumber, gidnumber, etc. are
being returned as `unicode` instead of `str` under Fedora12. Returning
as `unicode` is correct, but this patch allows the test to still work
under Fedora11 for the time being.
>From dafbfc22cccff32ff847a2e2eced09ac8c881378 Mon Sep 17 00:00:00 2001
From: Jason Gerard DeRose
Date: Sun, 10 Jan 2010 17:47:15 -0700
Subject: [PATCH] Fixed xmlrpc_test.fuzzy_digits for Fedora12
---
tests/test_xmlrpc/xmlrpc_test.py |2 +-
tests/util.py|2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/test_xmlrpc/xmlrpc_test.py b/tests/test_xmlrpc/xmlrpc_test.py
index 02b1f92..61fca50 100644
--- a/tests/test_xmlrpc/xmlrpc_test.py
+++ b/tests/test_xmlrpc/xmlrpc_test.py
@@ -32,7 +32,7 @@ from ipalib import errors
# Matches a gidnumber like '1391016742'
# FIXME: Does it make more sense to return gidnumber, uidnumber, etc. as `int`
# or `long`? If not, we still need to return them as `unicode` instead of `str`.
-fuzzy_digits = Fuzzy('^\d+$', type=str)
+fuzzy_digits = Fuzzy('^\d+$', type=basestring)
# Matches an ipauniqueid like u'784d85fd-eae7-11de-9d01-54520012478b'
fuzzy_uuid = Fuzzy(
diff --git a/tests/util.py b/tests/util.py
index ed8ecad..4d5fea6 100644
--- a/tests/util.py
+++ b/tests/util.py
@@ -210,7 +210,7 @@ class Fuzzy(object):
self.re = re.compile(regex)
if type is None:
type = unicode
-assert type in (unicode, str)
+assert type in (unicode, str, basestring)
self.regex = regex
self.type = type
self.test = test
--
1.6.3.3
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 346 add pki-cad support to ipactl
Pavel Zuna wrote: Rob Crittenden wrote: Add support for starting/stopping the CA to ipactl rob ack. Pavel pushed to master ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 347 do status on right service in installer
Rob Crittenden wrote: Remove one more hardcoded reference to the pki-ca service and use self.service_name instead. rob ack. Pavel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 346 add pki-cad support to ipactl
Rob Crittenden wrote: Add support for starting/stopping the CA to ipactl rob ack. Pavel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 347 do status on right service in installer
Pavel Zuna wrote: Rob Crittenden wrote: Remove one more hardcoded reference to the pki-ca service and use self.service_name instead. rob ack. Pavel pushed to master ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Add --all to LDAPCreate and make LDAP commands always display default attributes.
Pavel Zuna wrote:
This is actually an old patch that got lost in the depths of freeipa-devel.
There was just one issue with it, that it always assumed that the --all
parameter is present (because it is required in the baseclass). I fixed
it and now use the fail-safe: options.get('all', False)
The Kerberos Ticket Policy management plugin depends on it.
Pavel
ack, pushed to master
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 347 do status on right service in installer
Remove one more hardcoded reference to the pki-ca service and use self.service_name instead. rob freeipa-347-ca.patch Description: application/mbox ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 345 fix pwpolicy plugin
Rob Crittenden wrote: Allow the priority to be updated and fix the description of priority ordering. Lower wins, not higher. I also had to add the option to not normalize to a few more functions in ldap2. I have to craft a very specifically-formatted DN for it to be understood by the krb5 server. rob The patch is fine and everything seems to work. There's just some leftover debugging calls, that should probably go away. Pavel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Allow creation of new connections by unshared instances of backend.Connectible.
Jason Gerard DeRose wrote: On Tue, 2010-01-05 at 14:10 +0100, Pavel Zuna wrote: The backend.Connectible base class was designed, so that only one instance of each subclass is used at a time. Connectible generates a Connection object for each thread and stores it in thread-local storage (context). Subclasses access this object through the Connectible.conn property. This is a good thing, because one instance of the class can be shared by all threads and each thread has its own connection. Unfortunately, this is also a limitation. If a thread needs a second connection (to a different host for example) - it can't do it. Not even by creating a new instance of the Connectible subclass. Ok, let's move from theory to practice: The LDAP backend is currently only used by the Executioner backend, so that plugins can connect to the IPA DS. In the migration plugin, we need a second connection to the DS we're migrating from. The last version had to use low level python-ldap calls to achieve this. In the installer we're still using legacy code from v1. Using ldap2 would be simpler and we could drop ~1000 lines code. (I already started rewriting a few parts to see if it would work.) Proposed solution: Make it possible to create unshared instances of Connectible subclasses. This would be achieved by passing shared_instance=False (couldn't come up with a better name) to the object constructor explicitly. Normally, Connection objects are stored in thread-local storage under the subclass name (e.g. "ldap2"). Unshared instances would store their Connection objects under subclass name + unique instances ID (e.g. "ldap2_218adsfka7"). This is the only solution I could come up with, that doesn't involve breaking a lot of stuff - it just adds a new way of using the code we already have. The attached patches show how it would be done. Pavel I'm fine with this approach as the solution you propose is quite unobtrusive. Is this the final patch then, or will you make further changes or bundle it with another patch? Yeah, this is the final patch. Upcoming patches will depend on it. Pavel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 346 add pki-cad support to ipactl
Add support for starting/stopping the CA to ipactl rob freeipa-346-ipactl.patch Description: application/mbox ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Add Kerberos Ticket Policy management plugin.
Rob Crittenden wrote: Pavel Zuna wrote: Alright, here's my first shot at the Kerberos Ticket Policy management plugin. It is also a "new type" of plugin. What I mean by that is that it takes an optional primary key (username) as its first argument. If used, policy for a specific user is being managed. If not, the global policy is being managed. If there's no value defined for a specific user, the global value is displayed instead. This pattern could also be applied to the pwpolicy plugin. The pwpolicy plugin currently doesn't even use the baseldap classes and is a bit buggy*. So, if nobody minds, I'd like to rewrite it to use this pattern. It should only take a few hours. * minor bugs in pwpolicy plugin: - it says that higher number in cosPriority means higher priority, this isn't true - it is impossible to modify cosPriority using pwpolicy-mod, it throws an exception, because it tries to set it in the wrong entry Pavel I'm having a problem getting this to apply to the tip. Does this depend on some other patches? patching file ipalib/plugins/baseldap.py Hunk #5 succeeded at 346 (offset 152 lines). Hunk #6 FAILED at 363. Hunk #7 FAILED at 422. Hunk #8 FAILED at 506. Hunk #9 succeeded at 208 (offset -163 lines). Hunk #10 succeeded at 566 (offset 152 lines). Hunk #11 succeeded at 267 (offset -163 lines). Hunk #12 succeeded at 873 (offset 150 lines). 3 out of 12 hunks FAILED -- saving rejects to file ipalib/plugins/baseldap.py.rej patching file ipalib/plugins/krbtpolicy.py rob There was a forgotten patch adding --all to LDAPCreate, I reposted it on freeipa devel just now. I also generated a new version of the Kerberos Ticket Policy plugin patch (attached), just in case. Pavel 0002-Add-Kerberos-Ticket-Policy-management-plugin.patch Description: application/mbox ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] Add --all to LDAPCreate and make LDAP commands always display default attributes.
This is actually an old patch that got lost in the depths of freeipa-devel.
There was just one issue with it, that it always assumed that the --all
parameter is present (because it is required in the baseclass). I fixed it and
now use the fail-safe: options.get('all', False)
The Kerberos Ticket Policy management plugin depends on it.
Pavel
0001-Add-all-to-LDAPCreate-and-make-LDAP-commands-alway.patch
Description: application/mbox
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
