[SSSD] [PATCH] [RFC] refactor nested group processing

Pavel Březina pbrezina at redhat.com
Wed Feb 13 14:53:10 UTC 2013


On 02/13/2013 02:24 PM, Jakub Hrozek wrote:
> On Wed, Feb 13, 2013 at 01:22:40PM +0100, Pavel Březina wrote:
>> Hi,
>> I started working on https://fedorahosted.org/sssd/ticket/1784 and I'd
>> like to get some comments so I know I'm heading the right direction.
>>
>> *Current state:*
>> There is one tevent request per a nesting level. The request continues
>> with many "step" and other-named functions. The path is different for
>> deref and noderef processing, but the two branches intersect many
>> times. It uses a lot of recursion. It often translates return codes.
>>
>> All of this make it hard to understand the process, hard to maintain
>> and especially it is hard to step out of the process at chosen point.
>>
>> *New approach:*
>> There will be several tevent request per nesting level:
>>             main-req (from sdap_nested_group_send)
>>                 |
>>          nesting-level-req
>>            /        \
>>     noderef-req   deref-req
>>                      |
>>               possible-noderef-req
>
>                       ^^^^^
> Shouldn't this request get back to the nesting-level-req ? Keep in mind
> that on one level you can have several groups, some of them might have
> enough missing members to be dereferenced some of them might not.

Yes. I think it will not be needed.

> What is the nesting-level-req going to do? Check if the entry is already
> in the hash (or the sysdb as we can search both users and groups by
> originalDN in one go) and count how many members are missing?

The idea is to do here as much as possible, to keep the look ups simple. 
So yes, this is definitely good description.

In more general way: everything that is present in both deref and
noderef and doesn't need some special knowledge.

> The noderef branch is going to search bythe DN for users first and then
> groups, right?

If we don't know what object belongs to dn, we search users first and 
then groups. Yes.

>
>> It is closed API with the following interface:
>> - sdap_nested_group_send()
>> - sdap_nested_group_recv()
>>
>
> When is the groups being saved? In sdap_nested_group_done() or in the
> consumer of sdap_nested_group_recv()?

Groups are saved to sysdb at once in the consumer of _recv() (current 
behaviour). I'm not against moving it to _done().

>> The code is far from complete, I have finished only the main interface
>> so far. Otherwise it is just a skeleton. Especially
>> sdap_nested_group_(deref|single)* functions are just a generated code
>> so you don't have to bother reading them.
>
> Looks good, but please don't name the functions that save the group into
> hash with _save_ that makes it sound like they are saving into the
> sysdb. Also the attr variable could have been named name for better
> readability.
>
> But I think the concept is sound.
>
> I'm looking forward to see the complete patch along with unit tests :-)

Yeah, I need to check out your responder unit test.




More information about the sssd-devel mailing list