[SSSD] [PATCH] refactor nested group processing
Lukas Slebodnik
lslebodn at redhat.com
Thu Mar 28 15:33:51 UTC 2013
On (27/03/13 11:03), Lukas Slebodnik wrote:
>On (25/03/13 14:54), Jakub Hrozek wrote:
>>On Mon, Mar 25, 2013 at 02:48:44PM +0100, Pavel Březina wrote:
>>> On 03/22/2013 10:59 PM, Lukas Slebodnik wrote:
>>> >On (21/03/13 16:10), Pavel Březina wrote:
>>> >>On 03/21/2013 03:43 PM, Lukas Slebodnik wrote:
>>> >>>On (22/02/13 13:46), Pavel Brezina wrote:
>>> >>>>----- Original Message -----
>>> >>>>>From: "Pavel Březina" <pbrezina at redhat.com>
>>> >>>>>To: sssd-devel at lists.fedorahosted.org
>>> >>>>>Sent: Thursday, February 21, 2013 11:43:26 AM
>>> >>>>>Subject: [SSSD] [PATCH] refactor nested group processing
>>> >>>>>
>>> >>>>>https://fedorahosted.org/sssd/ticket/1784
>>> >>>>
>>> >>>>I should have probably add that this is the final version. I will attach unit test once I get familiar with cmocka, but it does not really prevent reviewing the patch.
>>> >>>
>>> >>>I would like to test your patches, but the second patch no longer applies
>>> >>>on the current master. Could you resend rebased patches?
>>> >>
>>> >>Thanks. Rebased patches are attached. I did not test it again though,
>>> >>but it compiles fine and the diff looks all right. It may be
>>> >>interesting to determine what patch broke it.
>>> >>
>>> >
>>> >I had problems with you patches.
>>> >The simplest cycle schema:
>>> > ________
>>> > | CG1 |------------>sssduser1
>>> > |______|<---|
>>> > | |
>>> > | |
>>> > | ________
>>> > |---->| CG2 |--->sssduser2
>>> > |______|
>>> >
>>> >sssd client configured with ipa-client-install and two additional options
>>> >entry_cache_timeout = 5
>>> >memcache_timeout = 1
>>> >
>>> >How to reproduce.
>>> >1. remove cache(also logs, but it is not important)
>>> >2. service sssd start
>>> >3. run commands
>>> >
>>> >sh-4.2$ getent group cyclegroup1
>>> >cyclegroup1:*:30007:sssduser1,sssduser2
>>> >sh-4.2$ getent group cyclegroup2
>>> >cyclegroup2:*:30008:sssduser2
>>> > #after second (memcache_timeout)
>>> >sh-4.2$ getent group cyclegroup2
>>> >cyclegroup2:*:30008:sssduser2,sssduser1
>>> >
>>> >-----------------------------
>>> >
>>> >Also sometimes with the same groups scheme one user appeared two times,
>>> >but I could not exactly reproduce steps.
>>>
>>> The problem is even in master. Seems to be another ghost users issue.
>>>
>>
>>I agree, the EEXIST error is coming from memberof plugin:
>>https://fedorahosted.org/sssd/ticket/1846
>>
>>> >
>>> >If you could not reproduce problem, I could send you off the list
>>> >logs, cache, any configurations, rpms or even provide access to IPA server.
>>> >
>>> >And last notice, commit 4709ff46db0dbe073aef061b796d2fd7adeaf18f
>>> >with title "LDAP: If deref search fails, try again without deref"
>>> >cause little problem with applying patches.
>>> >
>>> >LS
>>>
>>> I amended my patches to keep the functionality introduced in this
>>> commit. I.e. if deref fails with ENOTSUP, we will try again without
>>> dereference.
>>>
>
>I tested functionality of your patches and except bug from ticket
>(https://fedorahosted.org/sssd/ticket/1846) everything work like expected.
>Tested cases:
> --nonexisting group
> --empty group
> --user in group
> --nested groups
> --user in more nested group(no duplication)
> --cycle
> --deref
> --nonderef
>
>I have to say, that I learnd a lot of new things.
>
>(mini Ack) -> So someone else could look to code.
>I am not yet expert in tevent coding style
>
>LS
You introduced compile time warning. I did not noticed it first time,
because clang do not consider this to be a warning.
src/providers/ldap/sdap_async_nested_groups.c: In function 'sdap_nested_group_single_done':
src/providers/ldap/sdap_async_nested_groups.c:1143:41: warning: 'type'
may be used uninitialized in this function [-Wmaybe-uninitialized]
src/providers/ldap/sdap_async_nested_groups.c:1128:36: note: 'type' was declared here
Second similar warning is already in master.
src/providers/ldap/sdap_async_connection.c: In function 'sdap_cli_kinit_done':
src/providers/ldap/sdap_async_connection.c:1558:28: warning:
'expire_time' may be used uninitialized in this function [-Wmaybe-uninitialized]
src/providers/ldap/sdap_async_connection.c:1545:12: note: 'expire_time' was declared here
Both warning is the same type, like fixed in commit
http://git.fedorahosted.org/cgit/sssd.git/commit/?id=2d75dbbc6549acb1f749b5366b492ae087fdc0d8
LS
More information about the sssd-devel
mailing list