Re: [Freeipa-devel] [PATCHES] 0473-0477 Managed permission updater, part 1
On 03/05/2014 01:48 PM, Petr Viktorin wrote: On 03/03/2014 04:10 PM, Petr Viktorin wrote: On 02/28/2014 02:47 PM, Petr Viktorin wrote: On 02/28/2014 02:12 PM, Martin Kosek wrote: On 02/26/2014 10:44 AM, Petr Viktorin wrote: Hello, Here are a few fixes/improvements, and the first part of a managed permission updater. The patches should go in this order but don't need to be ACKed/pushed all at once. Design: http://www.freeipa.org/page/V3/Managed_Read_permissions#Default_Permission_Updater Part of the work for: https://fedorahosted.org/freeipa/ticket/3566 This part is a preview of sorts, to get the basic mechanism and the metadata format reviewed before I add all of the default read permissions. It implements the first section of Default Permission Updater in the design; Replacing legacy default permissions and Removing the global anonymous read ACI is left for later. Metadata is added for the netgroup plugin* for starters [...] 1) 476: Typo in test name: +desc='Search fo rnonexisting permission with : in the name', Will fix. Fixed 2) 477: do we want to log anything when permission is up to date? +try: +ldap.update_entry(entry) +except errors.EmptyModlist: +return I don't think that's needed, after all that's the expected behavior in all but the first upgrade. But I'll be happy to add it if you think it would be better. I've added a DEBUG message here. [...] 4) I have been quite resilient to the prefixes for the permissions, but it seems it is the easier possible approach to fix conflicts with user permissions without having to check that later for every upgrade or doing more complex stuff like multiple RDNs or different container for system and user permissions. I am now just thinking about the prefixing. Now you use this name: ipa:Read Netgroups Would it be nicer to use: IPA:Read Netgroups or IPA: Read Netgroups or even [IPA] Read Netgroups ? :-) Bikeshedding time! Everyone on the list, please chime in! Bikeshedding results from today's meeting: ipa: pviktori++ System: mkosek++ simo+ ab++ Builtin: simo++ pvo+ Default: The winner is System: , so I'll go and change to that. Done. Thanks. The patch set looks good now, I just see that the update process may not be optimal After I run the upgrade second time, it still tries to update the ACI even though there was no change done to the permission object and should thus raise errors.EmptyModlist and skip the ACI update: 2014-03-07T09:44:34Z INFO Updating managed permissions for netgroup 2014-03-07T09:44:34Z INFO Updating managed permission: System: Read Netgroups 2014-03-07T09:44:34Z DEBUG Updating ACI for managed permission: System: Read Netgroups 2014-03-07T09:44:34Z DEBUG Removing ACI u'(targetattr = cn || description || hostcategory || ipaenabledflag || ipauniqueid || nisdomainname || usercategory)(targetfilter = (objectclass=ipanisnetgroup))(version 3.0;acl permission:System: Read Netgroups;allow (read,compare,search) userdn = ldap:///all;;)' from cn=ng,cn=alt,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com 2014-03-07T09:44:34Z DEBUG Adding ACI u'(targetattr = cn || description || hostcategory || ipaenabledflag || ipauniqueid || nisdomainname || usercategory)(targetfilter = (objectclass=ipanisnetgroup))(version 3.0;acl permission:System: Read Netgroups;allow (read,compare,search) userdn = ldap:///all;;)' to cn=ng,cn=alt,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com 2014-03-07T09:44:34Z INFO No changes to ACI 2014-03-07T09:44:34Z INFO Updating managed permission: System: Read Netgroup Membership Any idea what could cause this? Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0473-0477 Managed permission updater, part 1
On 03/03/2014 04:10 PM, Petr Viktorin wrote:
On 02/28/2014 02:47 PM, Petr Viktorin wrote:
On 02/28/2014 02:12 PM, Martin Kosek wrote:
On 02/26/2014 10:44 AM, Petr Viktorin wrote:
Hello,
Here are a few fixes/improvements, and the first part of a managed
permission
updater.
The patches should go in this order but don't need to be ACKed/pushed
all at once.
Design:
http://www.freeipa.org/page/V3/Managed_Read_permissions#Default_Permission_Updater
Part of the work for: https://fedorahosted.org/freeipa/ticket/3566
This part is a preview of sorts, to get the basic mechanism and the
metadata
format reviewed before I add all of the default read permissions.
It implements the first section of Default Permission Updater in
the design;
Replacing legacy default permissions and Removing the global
anonymous read
ACI is left for later.
Metadata is added for the netgroup plugin* for starters
[...]
1) 476: Typo in test name:
+desc='Search fo rnonexisting permission with : in the
name',
Will fix.
Fixed
2) 477: do we want to log anything when permission is up to date?
+try:
+ldap.update_entry(entry)
+except errors.EmptyModlist:
+return
I don't think that's needed, after all that's the expected behavior in
all but the first upgrade.
But I'll be happy to add it if you think it would be better.
I've added a DEBUG message here.
[...]
4) I have been quite resilient to the prefixes for the permissions,
but it
seems it is the easier possible approach to fix conflicts with user
permissions
without having to check that later for every upgrade or doing more
complex
stuff like multiple RDNs or different container for system and user
permissions.
I am now just thinking about the prefixing. Now you use this name:
ipa:Read Netgroups
Would it be nicer to use:
IPA:Read Netgroups
or
IPA: Read Netgroups
or even
[IPA] Read Netgroups
? :-)
Bikeshedding time!
Everyone on the list, please chime in!
Bikeshedding results from today's meeting:
ipa: pviktori++
System: mkosek++ simo+ ab++
Builtin: simo++ pvo+
Default:
The winner is System: , so I'll go and change to that.
Done.
--
Petr³
From a4bb46871d3d1d1202e88bbbfbc8655d92ee45f3 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Wed, 12 Feb 2014 16:17:39 +0100
Subject: [PATCH] Allow indexing API object types by class
This allows code like:
from ipalib.plugins.dns import dnszone_mod
api.Command[dnszone_mod]
This form should be preferred when getting specific objects
because it ensures that the appropriate plugin is imported.
https://fedorahosted.org/freeipa/ticket/4185
---
ipalib/base.py| 15 +--
ipatests/test_ipalib/test_base.py | 12 ++--
2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/ipalib/base.py b/ipalib/base.py
index 83d778fe751e66414fe34d0d3243cdae34941ad6..91259c7f3b480ffc0e70061e1efa0135ff500478 100644
--- a/ipalib/base.py
+++ b/ipalib/base.py
@@ -287,7 +287,7 @@ class NameSpace(ReadOnly):
class Member(object):
... def __init__(self, i):
... self.i = i
-... self.name = 'member%d' % i
+... self.name = self.__name__ = 'member%d' % i
... def __repr__(self):
... return 'Member(%d)' % self.i
...
@@ -378,6 +378,14 @@ class NameSpace(ReadOnly):
unsorted_ns[0]
Member(7)
+As a special extension, NameSpace objects can be indexed by objects that
+have a __name__ attribute (e.g. classes). These lookups are converted
+to lookups on the name:
+
+ class_ns = NameSpace([Member(7), Member(3), Member(5)], sort=False)
+ unsorted_ns[Member(3)]
+Member(3)
+
The `NameSpace` class is used in many places throughout freeIPA. For a few
examples, see the `plugable.API` and the `frontend.Command` classes.
@@ -447,6 +455,7 @@ def __contains__(self, name):
Return ``True`` if namespace has a member named ``name``.
+name = getattr(name, '__name__', name)
return name in self.__map
def __getitem__(self, key):
@@ -455,12 +464,14 @@ def __getitem__(self, key):
:param key: The name or index of a member, or a slice object.
+key = getattr(key, '__name__', key)
if isinstance(key, basestring):
return self.__map[key]
if type(key) in (int, slice):
return self.__members[key]
raise TypeError(
-TYPE_ERROR % ('key', (str, int, slice), key, type(key))
+TYPE_ERROR % ('key', (str, int, slice, 'object with __name__'),
+ key, type(key))
)
def __repr__(self):
diff --git a/ipatests/test_ipalib/test_base.py b/ipatests/test_ipalib/test_base.py
index ef6c180c798632cb0cebd9b6ad89caaf744d7931..0117946bc2de8cb2aa014e42e6c6569b33f0900b 100644
--- a/ipatests/test_ipalib/test_base.py
+++
Re: [Freeipa-devel] [PATCHES] 0473-0477 Managed permission updater, part 1
On 02/28/2014 02:47 PM, Petr Viktorin wrote:
On 02/28/2014 02:12 PM, Martin Kosek wrote:
On 02/26/2014 10:44 AM, Petr Viktorin wrote:
Hello,
Here are a few fixes/improvements, and the first part of a managed
permission
updater.
The patches should go in this order but don't need to be ACKed/pushed
all at once.
Design:
http://www.freeipa.org/page/V3/Managed_Read_permissions#Default_Permission_Updater
Part of the work for: https://fedorahosted.org/freeipa/ticket/3566
This part is a preview of sorts, to get the basic mechanism and the
metadata
format reviewed before I add all of the default read permissions.
It implements the first section of Default Permission Updater in
the design;
Replacing legacy default permissions and Removing the global
anonymous read
ACI is left for later.
Metadata is added for the netgroup plugin* for starters, so
installing this
will give you two shiny new read permissions:
$ ipa permission-find ipa: --all
-
2 permissions matched
-
dn: cn=ipa:Read Netgroup Membership,cn=permissions,cn=pbac,$SUFFIX
Permission name: ipa:Read Netgroup Membership
Permissions: read, compare, search
Effective attributes: externalhost, member, memberof, memberuser
Default attributes: member, memberof, memberuser, externalhost
Bind rule type: all
Subtree: cn=ng,cn=alt,$SUFFIX
Target filter: (objectclass=ipanisnetgroup)
Type: netgroup
ipapermissiontype: V2, MANAGED, SYSTEM
objectclass: ipapermission, groupofnames, top, ipapermissionv2
dn: cn=ipa:Read Netgroups,cn=permissions,cn=pbac,$SUFFIX
Permission name: ipa:Read Netgroups
Permissions: read, compare, search
Effective attributes: cn, description, hostcategory, ipaenabledflag,
ipauniqueid, nisdomainname, usercategory
Default attributes: cn, usercategory, hostcategory, ipauniqueid,
ipaenabledflag, nisdomainname, description
Bind rule type: all
Subtree: cn=ng,cn=alt,$SUFFIX
Target filter: (objectclass=ipanisnetgroup)
Type: netgroup
ipapermissiontype: V2, MANAGED, SYSTEM
objectclass: ipapermission, groupofnames, top, ipapermissionv2
Number of entries returned 2
with corresponding ACIs at cn=ng,cn=alt,$SUFFIX:
(targetattr = externalhost || member || memberof ||
memberuser)(targetfilter
= (objectclass=ipanisnetgroup))(version 3.0;acl
permission:ipa:Read Netgroup
Membership;allow (read,compare,search) userdn = ldap:///all;;)
(targetattr = cn || description || hostcategory || ipaenabledflag ||
ipauniqueid || nisdomainname || usercategory)(targetfilter =
(objectclass=ipanisnetgroup))(version 3.0;acl permission:ipa:Read
Netgroups;allow (read,compare,search) userdn = ldap:///all;;)
Patches:
0473: Enables refactoring that will make it more clear (to humans and
machines)
what plugins code depends on.
https://fedorahosted.org/freeipa/ticket/4185
0474: Fix handling of the search term for legacy permissions
My code that's in master now handles the search term incorrectly.
This does a
better job.
0475: Fix tests that relied on some assumptions I'll be breaking
0476: Allow modifying (but not creating) permissions with : in the
name
0477: Permission updater sample metadata
1) 476: Typo in test name:
+desc='Search fo rnonexisting permission with : in the
name',
Will fix.
2) 477: do we want to log anything when permission is up to date?
+try:
+ldap.update_entry(entry)
+except errors.EmptyModlist:
+return
I don't think that's needed, after all that's the expected behavior in
all but the first upgrade.
But I'll be happy to add it if you think it would be better.
3) I am not sure if this was initiated by this patch, but when I
checked access
log for a permission-find --name ipa operation, it produced over 170
LDAP
operations, most of them asking for the same information many times. See
attached access log excerpt.
It's unrelated to this patch. I've started optimizing permission-find
with many legacy permissions, expect a patch soon.
4) I have been quite resilient to the prefixes for the permissions,
but it
seems it is the easier possible approach to fix conflicts with user
permissions
without having to check that later for every upgrade or doing more
complex
stuff like multiple RDNs or different container for system and user
permissions.
I am now just thinking about the prefixing. Now you use this name:
ipa:Read Netgroups
Would it be nicer to use:
IPA:Read Netgroups
or
IPA: Read Netgroups
or even
[IPA] Read Netgroups
? :-)
Bikeshedding time!
Everyone on the list, please chime in!
Bikeshedding results from today's meeting:
ipa: pviktori++
System: mkosek++ simo+ ab++
Builtin: simo++ pvo+
Default:
The winner is System: , so I'll go and change to that.
I don't really have a preference.
5) Are we sure we want to make our code Python 2.7 dependent?
+'ipapermright': {'read', 'search',
Re: [Freeipa-devel] [PATCHES] 0473-0477 Managed permission updater, part 1
On 02/28/2014 02:12 PM, Martin Kosek wrote:
On 02/26/2014 10:44 AM, Petr Viktorin wrote:
Hello,
Here are a few fixes/improvements, and the first part of a managed permission
updater.
The patches should go in this order but don't need to be ACKed/pushed all at
once.
Design:
http://www.freeipa.org/page/V3/Managed_Read_permissions#Default_Permission_Updater
Part of the work for: https://fedorahosted.org/freeipa/ticket/3566
This part is a preview of sorts, to get the basic mechanism and the metadata
format reviewed before I add all of the default read permissions.
It implements the first section of Default Permission Updater in the design;
Replacing legacy default permissions and Removing the global anonymous read
ACI is left for later.
Metadata is added for the netgroup plugin* for starters, so installing this
will give you two shiny new read permissions:
$ ipa permission-find ipa: --all
-
2 permissions matched
-
dn: cn=ipa:Read Netgroup Membership,cn=permissions,cn=pbac,$SUFFIX
Permission name: ipa:Read Netgroup Membership
Permissions: read, compare, search
Effective attributes: externalhost, member, memberof, memberuser
Default attributes: member, memberof, memberuser, externalhost
Bind rule type: all
Subtree: cn=ng,cn=alt,$SUFFIX
Target filter: (objectclass=ipanisnetgroup)
Type: netgroup
ipapermissiontype: V2, MANAGED, SYSTEM
objectclass: ipapermission, groupofnames, top, ipapermissionv2
dn: cn=ipa:Read Netgroups,cn=permissions,cn=pbac,$SUFFIX
Permission name: ipa:Read Netgroups
Permissions: read, compare, search
Effective attributes: cn, description, hostcategory, ipaenabledflag,
ipauniqueid, nisdomainname, usercategory
Default attributes: cn, usercategory, hostcategory, ipauniqueid,
ipaenabledflag, nisdomainname, description
Bind rule type: all
Subtree: cn=ng,cn=alt,$SUFFIX
Target filter: (objectclass=ipanisnetgroup)
Type: netgroup
ipapermissiontype: V2, MANAGED, SYSTEM
objectclass: ipapermission, groupofnames, top, ipapermissionv2
Number of entries returned 2
with corresponding ACIs at cn=ng,cn=alt,$SUFFIX:
(targetattr = externalhost || member || memberof || memberuser)(targetfilter
= (objectclass=ipanisnetgroup))(version 3.0;acl permission:ipa:Read Netgroup
Membership;allow (read,compare,search) userdn = ldap:///all;;)
(targetattr = cn || description || hostcategory || ipaenabledflag ||
ipauniqueid || nisdomainname || usercategory)(targetfilter =
(objectclass=ipanisnetgroup))(version 3.0;acl permission:ipa:Read
Netgroups;allow (read,compare,search) userdn = ldap:///all;;)
Patches:
0473: Enables refactoring that will make it more clear (to humans and machines)
what plugins code depends on.
https://fedorahosted.org/freeipa/ticket/4185
0474: Fix handling of the search term for legacy permissions
My code that's in master now handles the search term incorrectly. This does a
better job.
0475: Fix tests that relied on some assumptions I'll be breaking
0476: Allow modifying (but not creating) permissions with : in the name
0477: Permission updater sample metadata
1) 476: Typo in test name:
+desc='Search fo rnonexisting permission with : in the name',
Will fix.
2) 477: do we want to log anything when permission is up to date?
+try:
+ldap.update_entry(entry)
+except errors.EmptyModlist:
+return
I don't think that's needed, after all that's the expected behavior in
all but the first upgrade.
But I'll be happy to add it if you think it would be better.
3) I am not sure if this was initiated by this patch, but when I checked access
log for a permission-find --name ipa operation, it produced over 170 LDAP
operations, most of them asking for the same information many times. See
attached access log excerpt.
It's unrelated to this patch. I've started optimizing permission-find
with many legacy permissions, expect a patch soon.
4) I have been quite resilient to the prefixes for the permissions, but it
seems it is the easier possible approach to fix conflicts with user permissions
without having to check that later for every upgrade or doing more complex
stuff like multiple RDNs or different container for system and user permissions.
I am now just thinking about the prefixing. Now you use this name:
ipa:Read Netgroups
Would it be nicer to use:
IPA:Read Netgroups
or
IPA: Read Netgroups
or even
[IPA] Read Netgroups
? :-)
Bikeshedding time!
Everyone on the list, please chime in!
I don't really have a preference.
5) Are we sure we want to make our code Python 2.7 dependent?
+'ipapermright': {'read', 'search', 'compare'},
I did not test if we do not require some python 2.7 feature elsewhere already,
but this construct raised a warning for me.
That ship has sailed already.
Recently there was commit a8ba5e0 which explicitly
