On 03/14/2014 04:27 PM, Petr Viktorin wrote: > On 03/13/2014 02:01 PM, Petr Viktorin wrote: >> On 03/07/2014 10:45 AM, Martin Kosek wrote: >>> 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? >> >> Ah, you're right. The updater tried to remove the 'top' objectclass, >> since it found that in the permission but it wasn't in >> permission.object_class. >> I added 'top' to the list in a new patch. >> >> There was a minor conflict with master; I'm attaching the whole rebased >> patchset for convenience. > > Rebased again. >
I see the last issue I found is fixed - works fine. ACK for all patches after you fix another VERSION file conflict. Thanks, Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
