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

Jakub Hrozek jhrozek at redhat.com
Wed Feb 13 13:24:18 UTC 2013


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.

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 noderef branch is going to search bythe DN for users first and then
groups, right?

> 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() ?

> 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 :-)



More information about the sssd-devel mailing list