[Freeipa-devel] [PATCHES] 0473-0477+0497 Managed permission updater, part 1

Thu Mar 13 13:01:14 UTC 2014

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.

-- 
Petr³

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0473.3-Allow-indexing-API-object-types-by-class.patch
Type: text/x-patch
Size: 4114 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140313/bf304010/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0474.3-permission-find-Fix-handling-of-the-search-term-for-.patch
Type: text/x-patch
Size: 3667 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140313/bf304010/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0475.3-test_permission_plugin-Fix-tests-that-make-too-broad.patch
Type: text/x-patch
Size: 6973 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140313/bf304010/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0476.3-Allow-modifying-permissions-with-in-the-name.patch
Type: text/x-patch
Size: 11003 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140313/bf304010/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0477.3-Add-Object-metadata-and-update-plugin-for-managed-pe.patch
Type: text/x-patch
Size: 9082 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140313/bf304010/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0497.3-permission-plugin-Add-top-to-the-list-of-object-clas.patch
Type: text/x-patch
Size: 1502 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140313/bf304010/attachment-0005.bin>