[SSSD] [PATCH] Every time use permissive control in function memberof_mod.
Jakub Hrozek
jhrozek at redhat.com
Wed Jul 17 12:40:45 UTC 2013
On Wed, Jul 17, 2013 at 08:23:57AM -0400, Simo Sorce wrote:
> On Wed, 2013-07-17 at 14:18 +0200, Jakub Hrozek wrote:
> > On Tue, Jul 16, 2013 at 03:01:31PM +0200, Lukas Slebodnik wrote:
> > > ehlo,
> > >
> > > Group structure is described in ticket https://fedorahosted.org/sssd/ticket/1846
> > > Please ensure that sysdb cache is empty.
> > >
> > > call command "getent group cyclegroup1"
> > >
> > > sssd_be will call following function:
> > > a) sdap_store_group_with_gid (name:cyclegroup1)
> > > --added attributes [ ghost: user1, origMember: user1, ... ]
> > > b) sdap_store_group_with_gid (name:cyclegroup2)
> > > --added attributes [ ghost: user2, origMember: user2, ... ]
> > > c) sdap_save_grpmem (name:cyclegroup1)
> > > --added attributes [ member:cyclegroup2, ... ]
> > > d) sdap_save_grpmem (name:cyclegroup2)
> > > --added attributes [ member:cyclegroup1, ... ]
> > >
> > > Subtransaction "d)" will fail and log file contains:
> > > [ldb] (0x4000): will add 2 ghost users to 1 parents
> > > ... snip ...
> > > [ldb] (0x4000): cancel ldb transaction (nesting: 2)
> > > [sysdb_set_entry_attr] (0x0080): ldb_modify failed: [Attribute or value exists]
> > > [sysdb_set_entry_attr] (0x0400): Error: 17 (File exists)
> > > [sysdb_store_group] (0x0400): Error: 17 (File exists)
> > > [sdap_save_grpmem] (0x0040): Failed to save user cyclegroup2
> > > [sdap_save_groups] (0x0040): Failed to store group 1 members.
> > > [ldb] (0x4000): commit ldb transaction (nesting: 1)
> > > [ldb] (0x4000): commit ldb transaction (nesting: 0)
> > >
> > > Attached patch should fix this bug.
> > >
> > > LS
> >
> > I personally don't have a problem with the patch. It *might* hide
> > legitimate bugs but on the other hand the call was issued so that the
> > attribute is added and if it was added already during processing of a
> > cycle, then fine.
> >
> > But the memberof plugin is delicate code, so I would also like to get
> > Simo's opinion. Simo, are you OK with this approach or would you prefer
> > do detect the cycle instead?
>
> Ideally we would detect, but using the permissive control is ok for now.
OK. Ack to Lukas's patch, then. Thanks for the second look!
>
> > Please note that the function that Lukas added the permissive control to
> > is called to distribute both ghost and memberuid attributes.
>
> Yup and I appreciate the complexity around these functions, so go with
> the current fix and maybe open a ticket to revisit the issue when there
> is more time.
Sure:
https://fedorahosted.org/sssd/ticket/2021
we already have another memberof fix scheduled (#1654), so it makes
sense to do them at once to avoid re-learning the memberof plugin twice.
More information about the sssd-devel
mailing list