[SSSD] [PATCHES] cleanup task: Expire all memberof targets when removing user

Michal Židek mzidek at redhat.com
Mon Aug 10 14:27:56 UTC 2015


On 08/10/2015 07:04 AM, Lukas Slebodnik wrote:
> On (07/08/15 20:54), Michal Židek wrote:
>> Hi,
>>
>> see the attached patches for ticket
>> https://fedorahosted.org/sssd/ticket/2676
>>
>> Removing the user during cleanup task completely
>> removes that user from local database, so that
>> user is no longer part of any group in sysdb.
>>
>> Such user may, or may not have been removed from
>> LDAP so the only way to figure out is check
>> on the next refresh otherwise we may get
>> inconsistent results.
>>
>> The third patch adds integration test for this
>> Apply and run the intgcheck without the first 2
>> patches to see the failed test (you will also see that
>> all the tests after this failed one will end with
>> errors as well, that is probably due to incomplete
>> clean up after the ldap tests and I do not intend to
>> solve this as part of this patchset but will
>> investigate it later).
>>

>> +struct ldb_dn *sysdb_ldb_val_to_ldb_dn(TALLOC_CTX *tctx,
>> +                                       struct sss_domain_info *dom,
>> +                                       struct ldb_val *ldbval)
>> +{
>> +    return ldb_dn_from_ldb_val(tctx, dom->sysdb->ldb, ldbval);
>> +}
> I would prefer to do not expose such simple functions as sysdb functions.
> I do not see a reason why should convert ldb_val -> ldb_dn
> in other modules.
>
> If wee need two versions of sysdb_mark_entry_as_expired e can still have two
> functions:
>    sysdb_mark_entry_as_expired_ldb_dn
>    sysdb_mark_entry_as_expired_ldb_val
>
> The core logic can be in one and second will be wrapper with
> ldb_dn_from_ldb_val.
>
>

I added a wrapper.

>> +    ret = sss_ldb_modify_permissive(dom->sysdb->ldb, msg);
> We should avoid using sss_ldb_modify_permissive.
> It should be used in very rare cases.
> Could you explain why do we need to it here?

I wanted to ignore missing entries. But in
the updated patches I simply ignore ENOENT in
the caller and do not use sss_ldb_modify_permissive
anymore.

>
>> +
>> +        entry_cache_timeout = 1
>> +        entry_cache_group_timeout = 5000
> It would be good either to add comment that
> entry_cache_group_timeout should be different to entry_cache_user_timeout
> or explicitly use both options in sssd.conf

Ok, I now explicitly set both.

>> +
>> +    time.sleep(10)
> This can cause issuse in CI. The first clean up task
> is fired after 10 seconds. It would be be better to
> wait a little bit longer. To be sure that clean-up
> ask already finished.
>

Changed to 15 seconds. Do you think it is enough?

>> +
>> +    ent.assert_group_by_name(
>> +        "group2",
>> +        dict(mem=ent.contains_only("user1")))
>
> LS
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>


Michal
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-SYSDB-Add-function-to-expire-entry.patch
Type: text/x-patch
Size: 6136 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150810/13f232ab/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-cleanup-task-Expire-all-memberof-targets-when-removi.patch
Type: text/x-patch
Size: 3620 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150810/13f232ab/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-CI-Add-regression-test-for-2676.patch
Type: text/x-patch
Size: 2615 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150810/13f232ab/attachment-0002.bin>


More information about the sssd-devel mailing list