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

Michal Židek mzidek at redhat.com
Tue Sep 1 16:23:23 UTC 2015


On 09/01/2015 05:53 PM, Pavel Reichl wrote:
>
>
> On 09/01/2015 04:02 PM, Michal Židek wrote:
>>
>>  From 314116b09758e23626021da2f99f57f64846b26b Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Michal=20=C5=BDidek?=<mzidek at redhat.com>
>> Date: Mon, 31 Aug 2015 18:21:44 +0200
>> Subject: [PATCH 1/5] Makefile.am: Add missing AM_CFLAGS
>>
>> Some targets were missing AM_CFLAGS so
>> it was not possible to compiple C99 features
> s/compiple/compile
> [snip]
>>
>> 0002-SYSDB-Add-function-to-expire-entry.patch
>>
>>
>>  From a0fd65d5d25883e556bee34549c6df44aa3d5aba Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Michal=20=C5=BDidek?=<mzidek at redhat.com>
>> Date: Thu, 6 Aug 2015 09:16:03 +0200
>> Subject: [PATCH 2/5] SYSDB: Add function to expire entry
>>
>> Ticket:
>> https://fedorahosted.org/sssd/ticket/2676
>>
>>
>> +START_TEST(test_sysdb_mark_entry_as_expired_ldb_dn)
>> +{
>> +    errno_t ret;
>> +    struct sysdb_test_ctx *test_ctx;
>> +    const char *attrs[] = { SYSDB_CACHE_EXPIRE, NULL };
>> +    size_t count;
>> +    struct ldb_message **msgs;
>> +    uint64_t expire;
>> +    struct ldb_dn *userdn;
>> +
>> +    ret = setup_sysdb_tests(&test_ctx);
>> +    fail_if(ret != EOK, "Could not setup the test");
>> +
>> +    /* Add something to database to test against */
>> +
>> +    ret = sysdb_transaction_start(test_ctx->sysdb);
>> +    ck_assert_int_eq(ret, EOK);
>> +
>> +    ret = sysdb_add_user(test_ctx->domain, "testuser",
>> +                         2000, 0, "Test User", "/home/testuser",
>> +                         "/bin/bash",
>> +                         NULL, NULL, 500, 0);
>> +    ck_assert_int_eq(ret, EOK);
>> +
>> +    ret = sysdb_transaction_commit(test_ctx->sysdb);
>> +    ck_assert_int_eq(ret, EOK);
>> +
>> +    ret = sysdb_search_users(test_ctx, test_ctx->domain,
>> +                             "("SYSDB_UIDNUM"=2000)", attrs, &count, &msgs);
>> +    ck_assert_int_eq(ret, EOK);
>> +    ck_assert_int_eq(count, 1);
>> +
>> +    expire = ldb_msg_find_attr_as_uint64(msgs[0], SYSDB_CACHE_EXPIRE, 0);
>> +    ck_assert(expire != 1);
>> +
>> +    userdn = sysdb_user_dn(test_ctx, test_ctx->domain, "testuser");
>> +    ck_assert(userdn != NULL);
>> +
>> +    ret = sysdb_transaction_start(test_ctx->sysdb);
>> +    ck_assert_int_eq(ret, EOK);
>> +
>> +    /* Expire entry */
>> +    ret = sysdb_mark_entry_as_expired_ldb_dn(test_ctx->domain, userdn);
>> +    ck_assert_int_eq(ret, EOK);
>> +
>> +    ret = sysdb_transaction_commit(test_ctx->sysdb);
>> +    ck_assert_int_eq(ret, EOK);
>> +
>> +    ret = sysdb_search_users(test_ctx, test_ctx->domain,
>> +                             "("SYSDB_UIDNUM"=2000)", attrs, &count, &msgs);
>> +    ck_assert_int_eq(ret, EOK);
>> +    ck_assert_int_eq(count, 1);
>> +
>> +    expire = ldb_msg_find_attr_as_uint64(msgs[0], SYSDB_CACHE_EXPIRE, 0);
>> +    ck_assert_int_eq(expire, 1);
>> +
>> +    /* Attempt to mark missing entry should be OK */
> Can you improve this comment but it's just a nitpick
>> +    ret = sysdb_transaction_start(test_ctx->sysdb);
>> +    ck_assert_int_eq(ret, EOK);
>> +
>> +    ret = sysdb_mark_entry_as_expired_ldb_dn(test_ctx->domain, userdn);
>> +    ck_assert_int_eq(ret, EOK);
>> +
>> +    ret = sysdb_transaction_commit(test_ctx->sysdb);
>> +    ck_assert_int_eq(ret, EOK);
>> +}
>> +END_TEST
>> +
>>   Suite *create_sysdb_suite(void)
>>   {
>>       Suite *s = suite_create("sysdb");
>> @@ -6424,6 +6492,7 @@ Suite *create_sysdb_suite(void)
>>
>>   /* ===== Misc ===== */
>>       tcase_add_test(tc_sysdb, test_sysdb_set_get_bool);
>> +    tcase_add_test(tc_sysdb, test_sysdb_mark_entry_as_expired_ldb_dn);
>>
>>   /* Add all test cases to the test suite */
>>       suite_add_tcase(s, tc_sysdb);
>> -- 2.1.0
>>
>> 0003-cleanup-task-Expire-all-memberof-targets-when-removi.patch
>>
>>
>>  From 825ac2373bbfb1f70e13ce86b8b1a98eedfb017f Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Michal=20=C5=BDidek?=<mzidek at redhat.com>
>> Date: Wed, 29 Jul 2015 17:18:06 +0200
>> Subject: [PATCH 3/5] cleanup task: Expire all memberof targets when removing
>>   user
>>
>> Ticket:
>> https://fedorahosted.org/sssd/ticket/2676
>>
>> When user is removed from cache during cleanup task, mark all
>> his memberof targets as expired to refresh member/ghost
>> attributes on next request.
>> ---
>>   src/providers/ldap/ldap_id_cleanup.c | 52 +++++++++++++++++++++++++++++++++++-
>>   1 file changed, 51 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/providers/ldap/ldap_id_cleanup.c b/src/providers/ldap/ldap_id_cleanup.c
>> index 0cbcf04..624e746 100644
>> --- a/src/providers/ldap/ldap_id_cleanup.c
>> +++ b/src/providers/ldap/ldap_id_cleanup.c
>> @@ -175,11 +175,14 @@ done:
>>   static int cleanup_users_logged_in(hash_table_t *table,
>>                                      const struct ldb_message *msg);
>>
>> +static errno_t expire_memberof_target_grps(struct sss_domain_info *dom,
>> +                                           struct ldb_message *user);
> could you rename
> s/expire_memberof_target_grps/expire_memberof_target_groups - sorry that
> I didn't ask for this earlier
>> +
>>   static int cleanup_users(struct sdap_options *opts,
>>                            struct sss_domain_info *dom)
>>   {
>>       TALLOC_CTX *tmpctx;
>> -    const char *attrs[] = { SYSDB_NAME, SYSDB_UIDNUM, NULL };
>> +    const char *attrs[] = { SYSDB_NAME, SYSDB_UIDNUM, SYSDB_MEMBEROF, NULL };
>>       time_t now = time(NULL);
>>       char *subfilter = NULL;
>>       int account_cache_expiration;
>> @@ -277,6 +280,17 @@ static int cleanup_users(struct sdap_options *opts,
>>               DEBUG(SSSDBG_CRIT_FAILURE, "sysdb_delete_user failed: %d\n", ret);
>>               goto done;
>>           }
>> +
>> +        /* Mark all groups of which user was a member as expired in cache,
>> +         * so that its ghost/member attributes are refreshed on next
>> +         * request. */
>> +        ret = expire_memberof_target_grps(dom, msgs[i]);
>> +        if (ret != EOK && ret != ENOENT) {
>> +            DEBUG(SSSDBG_CRIT_FAILURE,
>> +                  "expire_memberof_target_grps failed: [%d]:%s\n",
>> +                  ret, sss_strerror(ret));
>> +            goto done;
>> +        }
>>       }
>>
>>   done:
>> @@ -284,6 +298,42 @@ done:
>>       return ret;
>>   }
>>
>> +static errno_t expire_memberof_target_grps(struct sss_domain_info *dom,
>> +                                           struct ldb_message *user)
>> +{
>> +    struct ldb_message_element *memberof_el = NULL;
>> +    errno_t ret;
>> +    TALLOC_CTX *tmp_ctx;
>> +
>> +    tmp_ctx = talloc_new(NULL);
>> +    if (tmp_ctx == NULL) {
>> +        return ENOMEM;
>> +    }
>> +
>> +    memberof_el = ldb_msg_find_element(user, SYSDB_MEMBEROF);
>> +    if (memberof_el == NULL) {
>> +        ret = ENOMEM;
> EOK? Could you add debug message here?

Yes, sorry this should have been EOK. Debug message here could
be too verbose (this can happen for a lot of users). So I added
just a comment.

>> +        goto done;
>> +    }
>> +
>> +    for (unsigned int i = 0; i < memberof_el->num_values; i++) {
>> +        ret = sysdb_mark_entry_as_expired_ldb_val(dom,
>> +                                                  &memberof_el->values[i]);
>> +        if (ret != EOK && ret != ENOENT) {
>> +            DEBUG(SSSDBG_CRIT_FAILURE,
>> +                  "sysdb_mark_entry_as_expired_ldb_val failed: [%d]: %s\n",
>> +                  ret, sss_strerror(ret));
>> +            goto done;
>> +        }
>> +    }
>> +
>> +    ret = EOK;
>> +
>> +done:
>> +    talloc_free(tmp_ctx);
>> +    return ret;
>> +}
>> +
>>   static int cleanup_users_logged_in(hash_table_t *table,
>>                                      const struct ldb_message *msg)
>>   {
>> -- 2.1.0
>>
> Rest LGTM and my testing passed. We are almost there :-)
>

Thanks.

Michal
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Makefile.am-Add-missing-AM_CFLAGS.patch
Type: text/x-patch
Size: 1603 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150901/06062c8e/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-SYSDB-Add-function-to-expire-entry.patch
Type: text/x-patch
Size: 6197 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150901/06062c8e/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-cleanup-task-Expire-all-memberof-targets-when-removi.patch
Type: text/x-patch
Size: 3405 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150901/06062c8e/attachment-0007.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-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/20150901/06062c8e/attachment-0008.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-intg-Fix-some-PEP-8-violations.patch
Type: text/x-patch
Size: 20833 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150901/06062c8e/attachment-0009.bin>


More information about the sssd-devel mailing list