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).
Thanks, Michal
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).
From 9930382acae9eb708c609355131207a0be42d1e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Thu, 6 Aug 2015 09:16:03 +0200 Subject: [PATCH 1/3] SYSDB: Add function to expire entry
Ticket: https://fedorahosted.org/sssd/ticket/2676
Added function to expire entry in sysdb using its DN.
src/db/sysdb.h | 10 ++++++- src/db/sysdb_ops.c | 51 ++++++++++++++++++++++++++++++++++++ src/tests/sysdb-tests.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 129 insertions(+), 1 deletion(-)
diff --git a/src/db/sysdb.h b/src/db/sysdb.h index 9e28b5c..a48e22d 100644 --- a/src/db/sysdb.h +++ b/src/db/sysdb.h @@ -387,6 +387,11 @@ errno_t sysdb_msg2attrs(TALLOC_CTX *mem_ctx, size_t count, /* convert an ldb error into an errno error */ int sysdb_error_to_errno(int ldberr);
+/* Convert ldb value to ldb dn */ +struct ldb_dn *sysdb_ldb_val_to_ldb_dn(TALLOC_CTX *tctx,
struct sss_domain_info *dom,
struct ldb_val *ldbval);
/* DNs related helper functions */ errno_t sysdb_get_rdn(struct sysdb_ctx *sysdb, TALLOC_CTX *mem_ctx, const char *dn, char **_name, char **_val); @@ -717,11 +722,14 @@ int sysdb_delete_entry(struct sysdb_ctx *sysdb, struct ldb_dn *dn, bool ignore_not_found);
int sysdb_delete_recursive(struct sysdb_ctx *sysdb, struct ldb_dn *dn, bool ignore_not_found);
+/* Mark entry as expired (permissive) */ +int sysdb_mark_entry_as_expired_ldb_dn(struct sss_domain_info *dom,
struct ldb_dn *ldbdn);
/* Search Entry */ int sysdb_search_entry(TALLOC_CTX *mem_ctx, struct sysdb_ctx *sysdb, diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c index d1d43eb..c582578 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -3875,3 +3875,54 @@ errno_t sysdb_handle_original_uuid(const char *orig_name,
return EOK;
}
+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.
+/* Mark entry as expired if that entry exists in database. */ +int sysdb_mark_entry_as_expired_ldb_dn(struct sss_domain_info *dom,
struct ldb_dn *ldbdn)
+{
- struct ldb_message *msg;
- int ret;
- TALLOC_CTX *tmp_ctx;
- tmp_ctx = talloc_new(NULL);
- if (tmp_ctx == NULL) {
return ENOMEM;
- }
- msg = ldb_msg_new(tmp_ctx);
- if (msg == NULL) {
ret = ENOMEM;
goto done;
- }
- msg->dn = ldbdn;
- ret = ldb_msg_add_empty(msg, SYSDB_CACHE_EXPIRE,
LDB_FLAG_MOD_REPLACE, NULL);
- if (ret != LDB_SUCCESS) {
goto done;
- }
- ret = ldb_msg_add_string(msg, SYSDB_CACHE_EXPIRE, "1");
- if (ret != LDB_SUCCESS) {
goto done;
- }
- 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?
- if (ret != LDB_SUCCESS) {
goto done;
- }
- ret = EOK;
+done:
- talloc_free(tmp_ctx);
- return ret;
+}
diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c index 24d1527..3bd03b1 100644 --- a/src/tests/sysdb-tests.c +++ b/src/tests/sysdb-tests.c @@ -6212,6 +6212,74 @@ START_TEST(test_confdb_list_all_domain_names_multi_dom) } END_TEST
+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 */
- 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
From 5d6edc83c09b36f34beb9118d1c528b5229600d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Wed, 29 Jul 2015 17:18:06 +0200 Subject: [PATCH 2/3] cleanup task: Expire all memberof targets when removing user
Ticket: https://fedorahosted.org/sssd/ticket/2676
When user 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 | 57 ++++++++++++++++++++++++++++++++++-- 1 file changed, 55 insertions(+), 2 deletions(-)
diff --git a/src/providers/ldap/ldap_id_cleanup.c b/src/providers/ldap/ldap_id_cleanup.c index be9496a..93a9362 100644 --- a/src/providers/ldap/ldap_id_cleanup.c +++ b/src/providers/ldap/ldap_id_cleanup.c @@ -169,11 +169,14 @@ done: static int cleanup_users_logged_in(hash_table_t *table, const struct ldb_message *msg);
+static int expire_memberof_target_grps(struct sss_domain_info *dom,
struct ldb_message *user);
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;
@@ -188,7 +191,7 @@ static int cleanup_users(struct sdap_options *opts, if (!tmpctx) { return ENOMEM; }
- account_cache_expiration = dp_opt_get_int(opts->basic, SDAP_ACCOUNT_CACHE_EXPIRATION); account_cache_expiration = dp_opt_get_int(opts->basic, SDAP_ACCOUNT_CACHE_EXPIRATION); DEBUG(SSSDBG_TRACE_ALL, "Cache expiration is set to %d days\n", account_cache_expiration);
@@ -271,6 +274,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 && ret != ENOENT) {
DEBUG(SSSDBG_CRIT_FAILURE,
"expire_memberof_target_grps failed: [%d]:%s\n",
ret, sss_strerror(ret));
goto done;
}}
done: @@ -278,6 +292,45 @@ done: return ret; }
+static int expire_memberof_target_grps(struct sss_domain_info *dom,
struct ldb_message *user)
+{
- struct ldb_message_element *memberof_el;
- struct ldb_dn *ldb_dn;
- int ret;
- int i;
- TALLOC_CTX *tmp_ctx;
- tmp_ctx = talloc_new(NULL);
- if (tmp_ctx == NULL) {
return ENOMEM;
- }
- memberof_el = ldb_msg_find_element(user, SYSDB_MEMBEROF);
- for (i = 0; i < memberof_el->num_values; i++) {
ldb_dn = sysdb_ldb_val_to_ldb_dn(tmp_ctx, dom,
&memberof_el->values[i]);
if (ldb_dn == NULL) {
ret = ENOMEM;
goto done;
}
ret = sysdb_mark_entry_as_expired_ldb_dn(dom, ldb_dn);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE,
"sysdb_mark_entry_as_expired_ldb_dn 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
From cf61a2037fc3cfb6ddee040e6423463a08e74d2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Thu, 6 Aug 2015 13:16:48 +0200 Subject: [PATCH 3/3] CI: Add regression test for #2676
Ticket: https://fedorahosted.org/sssd/ticket/2676
Regression test for the above ticket.
src/tests/intg/ldap_test.py | 61 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+)
diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py index bfe4e65..19968e3 100644 --- a/src/tests/intg/ldap_test.py +++ b/src/tests/intg/ldap_test.py @@ -260,3 +260,64 @@ def test_sanity_rfc2307_bis(ldap_conn, sanity_rfc2307_bis): grp.getgrnam("non_existent_group") with pytest.raises(KeyError): grp.getgrgid(1)
+@pytest.fixture +def refresh_after_cleanup_task(request, ldap_conn):
- ent_list = ldap_ent.List(LDAP_BASE_DN)
- ent_list.add_user("user1", 1001, 2001)
- ent_list.add_group_bis("group1", 2001, ["user1"])
- ent_list.add_group_bis("group2", 2002, [], ["group1"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = unindent("""\
[sssd]
config_file_version = 2
domains = LDAP
services = nss
[nss]
memcache_timeout = 0
[domain/LDAP]
ldap_auth_disable_tls_never_use_in_production = true
debug_level = 0xffff
ldap_schema = rfc2307bis
ldap_group_object_class = groupOfNames
id_provider = ldap
auth_provider = ldap
sudo_provider = ldap
ldap_uri = {ldap_conn.ds_inst.ldap_url}
ldap_search_base = {ldap_conn.ds_inst.base_dn}
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
ldap_purge_cache_timeout = 3
- """).format(**locals())
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_refresh_after_cleanup_task(ldap_conn, refresh_after_cleanup_task):
- """
- Regression test for ticket:
- https://fedorahosted.org/sssd/ticket/2676
- """
- ent.assert_group_by_name(
"group2",
dict(mem=ent.contains_only("user1")))
- ent.assert_passwd_by_name(
'user1',
dict(name='user1', passwd='*', uid=1001, gid=2001,
gecos='1001', shell='/bin/bash'))
- 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.
- ent.assert_group_by_name(
"group2",
dict(mem=ent.contains_only("user1")))
LS
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@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Michal
On 08/10/2015 04:27 PM, Michal Židek wrote:
From 725cb791d43e9f651345ec50419143c4e6bd22b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?=mzidek@redhat.com Date: Thu, 6 Aug 2015 09:16:03 +0200 Subject: [PATCH 1/3] SYSDB: Add function to expire entry
Ticket: https://fedorahosted.org/sssd/ticket/2676
Added function to expire entry in sysdb using its DN.
src/db/sysdb.h | 7 ++++- src/db/sysdb_ops.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++ src/tests/sysdb-tests.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 145 insertions(+), 1 deletion(-)
diff --git a/src/db/sysdb.h b/src/db/sysdb.h index 9e28b5c..75d7318 100644 --- a/src/db/sysdb.h +++ b/src/db/sysdb.h @@ -717,11 +717,16 @@ int sysdb_delete_entry(struct sysdb_ctx *sysdb, struct ldb_dn *dn, bool ignore_not_found);
- int sysdb_delete_recursive(struct sysdb_ctx *sysdb, struct ldb_dn *dn, bool ignore_not_found);
+/* Mark entry as expired */ +int sysdb_mark_entry_as_expired_ldb_dn(struct sss_domain_info *dom,
struct ldb_dn *ldbdn);
+int sysdb_mark_entry_as_expired_ldb_val(struct sss_domain_info *dom,
struct ldb_val *dn_val);
- /* Search Entry */ int sysdb_search_entry(TALLOC_CTX *mem_ctx, struct sysdb_ctx *sysdb,
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c index d1d43eb..6d4070a 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -3875,3 +3875,73 @@ errno_t sysdb_handle_original_uuid(const char *orig_name,
return EOK;
}
+/* Mark entry as expired */ +int sysdb_mark_entry_as_expired_ldb_dn(struct sss_domain_info *dom,
struct ldb_dn *ldbdn)
Is there a reason not to use errno_t?
+{
- struct ldb_message *msg;
- int ret;
Same here?
- TALLOC_CTX *tmp_ctx;
- tmp_ctx = talloc_new(NULL);
- if (tmp_ctx == NULL) {
return ENOMEM;
- }
- msg = ldb_msg_new(tmp_ctx);
- if (msg == NULL) {
ret = ENOMEM;
goto done;
- }
- msg->dn = ldbdn;
- ret = ldb_msg_add_empty(msg, SYSDB_CACHE_EXPIRE,
LDB_FLAG_MOD_REPLACE, NULL);
- if (ret != LDB_SUCCESS) {
ret = sysdb_error_to_errno(ret);
goto done;
- }
- ret = ldb_msg_add_string(msg, SYSDB_CACHE_EXPIRE, "1");
- if (ret != LDB_SUCCESS) {
ret = sysdb_error_to_errno(ret);
goto done;
- }
- ret = ldb_modify(dom->sysdb->ldb, msg);
- if (ret != LDB_SUCCESS) {
ret = sysdb_error_to_errno(ret);
goto done;
- }
- ret = EOK;
please add an empty line
+done:
- talloc_free(tmp_ctx);
- return ret;
+}
+int sysdb_mark_entry_as_expired_ldb_val(struct sss_domain_info *dom,
struct ldb_val *dn_val)
+{
- struct ldb_dn *ldbdn;
- int ret;
errno_t?
- TALLOC_CTX *tmpctx;
In function above you used tmp_ctx, I think it would be nice to use the same name...nitpick
- tmpctx = talloc_new(NULL);
- if (tmpctx == NULL) {
return ENOMEM;
- }
- ldbdn = ldb_dn_from_ldb_val(tmpctx, dom->sysdb->ldb, dn_val);
- if (ldbdn == NULL) {
ret = ENOMEM;
goto done;
- }
- ret = sysdb_mark_entry_as_expired_ldb_dn(dom, ldbdn);
empty line
+done:
- talloc_free(tmpctx);
- return ret;
+} diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c index 24d1527..3bd03b1 100644 --- a/src/tests/sysdb-tests.c +++ b/src/tests/sysdb-tests.c @@ -6212,6 +6212,74 @@ START_TEST(test_confdb_list_all_domain_names_multi_dom) } END_TEST
+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 */
- 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
0002-cleanup-task-Expire-all-memberof-targets-when-removi.patch
From cccdd4bc3d35e2f0c71f5a1c1c54591e7edb8eac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?=mzidek@redhat.com Date: Wed, 29 Jul 2015 17:18:06 +0200 Subject: [PATCH 2/3] cleanup task: Expire all memberof targets when removing user
Ticket: https://fedorahosted.org/sssd/ticket/2676
When user removed from cache during cleanup task, mark all
is removed ?
his memberof targets as expired to refresh member/ghost attributes on next request.
src/providers/ldap/ldap_id_cleanup.c | 50 ++++++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-)
diff --git a/src/providers/ldap/ldap_id_cleanup.c b/src/providers/ldap/ldap_id_cleanup.c index be9496a..86c8ec4 100644 --- a/src/providers/ldap/ldap_id_cleanup.c +++ b/src/providers/ldap/ldap_id_cleanup.c @@ -169,11 +169,14 @@ done: static int cleanup_users_logged_in(hash_table_t *table, const struct ldb_message *msg);
+static int expire_memberof_target_grps(struct sss_domain_info *dom,
struct ldb_message *user);
- 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;
@@ -188,7 +191,7 @@ static int cleanup_users(struct sdap_options *opts, if (!tmpctx) { return ENOMEM; }
Is this intentional?
- account_cache_expiration = dp_opt_get_int(opts->basic, SDAP_ACCOUNT_CACHE_EXPIRATION);
Are these 2 lines same?
account_cache_expiration = dp_opt_get_int(opts->basic, SDAP_ACCOUNT_CACHE_EXPIRATION); DEBUG(SSSDBG_TRACE_ALL, "Cache expiration is set to %d days\n", account_cache_expiration);
@@ -271,6 +274,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 && ret != ENOENT) {
would it be nicer to have ret != EOK instead of just ret?
DEBUG(SSSDBG_CRIT_FAILURE,
"expire_memberof_target_grps failed: [%d]:%s\n",
ret, sss_strerror(ret));
goto done;
} }
done:
@@ -278,6 +292,38 @@ done: return ret; }
+static int expire_memberof_target_grps(struct sss_domain_info *dom,
struct ldb_message *user)
+{
- struct ldb_message_element *memberof_el;
- int ret;
errno_t ?
- int i;
you might want to move definition of i into for cycle :-)
- TALLOC_CTX *tmp_ctx;
- tmp_ctx = talloc_new(NULL);
- if (tmp_ctx == NULL) {
return ENOMEM;
- }
- memberof_el = ldb_msg_find_element(user, SYSDB_MEMBEROF);
check for NULL?
- for (i = 0; i < memberof_el->num_values; i++) {
unsigned int would be better, right?
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;
empty line
+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
0003-CI-Add-regression-test-for-2676.patch
From 2eec70588ed74e6c6d2322c4fdbd4c738f32d586 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?=mzidek@redhat.com Date: Thu, 6 Aug 2015 13:16:48 +0200 Subject: [PATCH 3/3] CI: Add regression test for #2676
Ticket: https://fedorahosted.org/sssd/ticket/2676
Regression test for the above ticket.
src/tests/intg/ldap_test.py | 61 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+)
diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py index bfe4e65..b0a4dc6 100644 --- a/src/tests/intg/ldap_test.py +++ b/src/tests/intg/ldap_test.py @@ -260,3 +260,64 @@ def test_sanity_rfc2307_bis(ldap_conn, sanity_rfc2307_bis): grp.getgrnam("non_existent_group") with pytest.raises(KeyError): grp.getgrgid(1)
Why 2 empty lines? The rest of the module uses just one, right?
+@pytest.fixture +def refresh_after_cleanup_task(request, ldap_conn):
- ent_list = ldap_ent.List(LDAP_BASE_DN)
- ent_list.add_user("user1", 1001, 2001)
- ent_list.add_group_bis("group1", 2001, ["user1"])
- ent_list.add_group_bis("group2", 2002, [], ["group1"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = unindent("""\
[sssd]
config_file_version = 2
domains = LDAP
services = nss
[nss]
memcache_timeout = 0
[domain/LDAP]
ldap_auth_disable_tls_never_use_in_production = true
debug_level = 0xffff
ldap_schema = rfc2307bis
ldap_group_object_class = groupOfNames
id_provider = ldap
auth_provider = ldap
sudo_provider = ldap
ldap_uri = {ldap_conn.ds_inst.ldap_url}
ldap_search_base = {ldap_conn.ds_inst.base_dn}
entry_cache_user_timeout = 1
entry_cache_group_timeout = 5000
ldap_purge_cache_timeout = 3
- """).format(**locals())
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_refresh_after_cleanup_task(ldap_conn, refresh_after_cleanup_task):
- """
- Regression test for ticket:
+https://fedorahosted.org/sssd/ticket/2676
- """
- ent.assert_group_by_name(
"group2",
dict(mem=ent.contains_only("user1")))
- ent.assert_passwd_by_name(
'user1',
dict(name='user1', passwd='*', uid=1001, gid=2001,
gecos='1001', shell='/bin/bash'))
- time.sleep(15)
- ent.assert_group_by_name(
"group2",
dict(mem=ent.contains_only("user1")))
-- 2.1.0
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Michal, please fix these nitpicks.
On 08/31/2015 03:08 PM, Pavel Reichl wrote:
On 08/10/2015 04:27 PM, Michal Židek wrote:
From 725cb791d43e9f651345ec50419143c4e6bd22b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?=mzidek@redhat.com Date: Thu, 6 Aug 2015 09:16:03 +0200 Subject: [PATCH 1/3] SYSDB: Add function to expire entry
Ticket: https://fedorahosted.org/sssd/ticket/2676
Added function to expire entry in sysdb using its DN.
src/db/sysdb.h | 7 ++++- src/db/sysdb_ops.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++ src/tests/sysdb-tests.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 145 insertions(+), 1 deletion(-)
diff --git a/src/db/sysdb.h b/src/db/sysdb.h index 9e28b5c..75d7318 100644 --- a/src/db/sysdb.h +++ b/src/db/sysdb.h @@ -717,11 +717,16 @@ int sysdb_delete_entry(struct sysdb_ctx *sysdb, struct ldb_dn *dn, bool ignore_not_found);
- int sysdb_delete_recursive(struct sysdb_ctx *sysdb, struct ldb_dn *dn, bool ignore_not_found);
+/* Mark entry as expired */ +int sysdb_mark_entry_as_expired_ldb_dn(struct sss_domain_info *dom,
struct ldb_dn *ldbdn);
+int sysdb_mark_entry_as_expired_ldb_val(struct sss_domain_info *dom,
struct ldb_val *dn_val);
- /* Search Entry */ int sysdb_search_entry(TALLOC_CTX *mem_ctx, struct sysdb_ctx *sysdb,
diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c index d1d43eb..6d4070a 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -3875,3 +3875,73 @@ errno_t sysdb_handle_original_uuid(const char *orig_name,
return EOK;
}
+/* Mark entry as expired */ +int sysdb_mark_entry_as_expired_ldb_dn(struct sss_domain_info *dom,
struct ldb_dn *ldbdn)
Is there a reason not to use errno_t?
+{
- struct ldb_message *msg;
- int ret;
Same here?
- TALLOC_CTX *tmp_ctx;
- tmp_ctx = talloc_new(NULL);
- if (tmp_ctx == NULL) {
return ENOMEM;
- }
- msg = ldb_msg_new(tmp_ctx);
- if (msg == NULL) {
ret = ENOMEM;
goto done;
- }
- msg->dn = ldbdn;
- ret = ldb_msg_add_empty(msg, SYSDB_CACHE_EXPIRE,
LDB_FLAG_MOD_REPLACE, NULL);
- if (ret != LDB_SUCCESS) {
ret = sysdb_error_to_errno(ret);
goto done;
- }
- ret = ldb_msg_add_string(msg, SYSDB_CACHE_EXPIRE, "1");
- if (ret != LDB_SUCCESS) {
ret = sysdb_error_to_errno(ret);
goto done;
- }
- ret = ldb_modify(dom->sysdb->ldb, msg);
- if (ret != LDB_SUCCESS) {
ret = sysdb_error_to_errno(ret);
goto done;
- }
- ret = EOK;
please add an empty line
+done:
- talloc_free(tmp_ctx);
- return ret;
+}
+int sysdb_mark_entry_as_expired_ldb_val(struct sss_domain_info *dom,
struct ldb_val *dn_val)
+{
- struct ldb_dn *ldbdn;
- int ret;
errno_t?
- TALLOC_CTX *tmpctx;
In function above you used tmp_ctx, I think it would be nice to use the same name...nitpick
- tmpctx = talloc_new(NULL);
- if (tmpctx == NULL) {
return ENOMEM;
- }
- ldbdn = ldb_dn_from_ldb_val(tmpctx, dom->sysdb->ldb, dn_val);
- if (ldbdn == NULL) {
ret = ENOMEM;
goto done;
- }
- ret = sysdb_mark_entry_as_expired_ldb_dn(dom, ldbdn);
empty line
+done:
- talloc_free(tmpctx);
- return ret;
+} diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c index 24d1527..3bd03b1 100644 --- a/src/tests/sysdb-tests.c +++ b/src/tests/sysdb-tests.c @@ -6212,6 +6212,74 @@ START_TEST(test_confdb_list_all_domain_names_multi_dom) } END_TEST
+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 */
- 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
0002-cleanup-task-Expire-all-memberof-targets-when-removi.patch
From cccdd4bc3d35e2f0c71f5a1c1c54591e7edb8eac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?=mzidek@redhat.com Date: Wed, 29 Jul 2015 17:18:06 +0200 Subject: [PATCH 2/3] cleanup task: Expire all memberof targets when removing user
Ticket: https://fedorahosted.org/sssd/ticket/2676
When user removed from cache during cleanup task, mark all
is removed ?
his memberof targets as expired to refresh member/ghost attributes on next request.
src/providers/ldap/ldap_id_cleanup.c | 50 ++++++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-)
diff --git a/src/providers/ldap/ldap_id_cleanup.c b/src/providers/ldap/ldap_id_cleanup.c index be9496a..86c8ec4 100644 --- a/src/providers/ldap/ldap_id_cleanup.c +++ b/src/providers/ldap/ldap_id_cleanup.c @@ -169,11 +169,14 @@ done: static int cleanup_users_logged_in(hash_table_t *table, const struct ldb_message *msg);
+static int expire_memberof_target_grps(struct sss_domain_info *dom,
struct ldb_message *user);
- 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;
@@ -188,7 +191,7 @@ static int cleanup_users(struct sdap_options *opts, if (!tmpctx) { return ENOMEM; }
Is this intentional?
- account_cache_expiration = dp_opt_get_int(opts->basic, SDAP_ACCOUNT_CACHE_EXPIRATION);
Are these 2 lines same?
account_cache_expiration = dp_opt_get_int(opts->basic, SDAP_ACCOUNT_CACHE_EXPIRATION); DEBUG(SSSDBG_TRACE_ALL, "Cache expiration is set to %d days\n", account_cache_expiration);
@@ -271,6 +274,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 && ret != ENOENT) {
would it be nicer to have ret != EOK instead of just ret?
DEBUG(SSSDBG_CRIT_FAILURE,
"expire_memberof_target_grps failed: [%d]:%s\n",
ret, sss_strerror(ret));
goto done;
} }
done:
@@ -278,6 +292,38 @@ done: return ret; }
+static int expire_memberof_target_grps(struct sss_domain_info *dom,
struct ldb_message *user)
+{
- struct ldb_message_element *memberof_el;
- int ret;
errno_t ?
- int i;
you might want to move definition of i into for cycle :-)
- TALLOC_CTX *tmp_ctx;
- tmp_ctx = talloc_new(NULL);
- if (tmp_ctx == NULL) {
return ENOMEM;
- }
- memberof_el = ldb_msg_find_element(user, SYSDB_MEMBEROF);
check for NULL?
- for (i = 0; i < memberof_el->num_values; i++) {
unsigned int would be better, right?
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;
empty line
+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
0003-CI-Add-regression-test-for-2676.patch
From 2eec70588ed74e6c6d2322c4fdbd4c738f32d586 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?=mzidek@redhat.com Date: Thu, 6 Aug 2015 13:16:48 +0200 Subject: [PATCH 3/3] CI: Add regression test for #2676
Ticket: https://fedorahosted.org/sssd/ticket/2676
Regression test for the above ticket.
src/tests/intg/ldap_test.py | 61 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+)
diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py index bfe4e65..b0a4dc6 100644 --- a/src/tests/intg/ldap_test.py +++ b/src/tests/intg/ldap_test.py @@ -260,3 +260,64 @@ def test_sanity_rfc2307_bis(ldap_conn, sanity_rfc2307_bis): grp.getgrnam("non_existent_group") with pytest.raises(KeyError): grp.getgrgid(1)
Why 2 empty lines? The rest of the module uses just one, right?
It is PEP 8 convention. Actually the reason the rest of the file does not follow it is my fault (I asked Nick to remove the additional line because that is how we used it in other parts of the code - now I know that I was wrong). See the additional patch that adds the lines back.
+@pytest.fixture +def refresh_after_cleanup_task(request, ldap_conn):
- ent_list = ldap_ent.List(LDAP_BASE_DN)
- ent_list.add_user("user1", 1001, 2001)
- ent_list.add_group_bis("group1", 2001, ["user1"])
- ent_list.add_group_bis("group2", 2002, [], ["group1"])
- create_ldap_fixture(request, ldap_conn, ent_list)
- conf = unindent("""\
[sssd]
config_file_version = 2
domains = LDAP
services = nss
[nss]
memcache_timeout = 0
[domain/LDAP]
ldap_auth_disable_tls_never_use_in_production = true
debug_level = 0xffff
ldap_schema = rfc2307bis
ldap_group_object_class = groupOfNames
id_provider = ldap
auth_provider = ldap
sudo_provider = ldap
ldap_uri = {ldap_conn.ds_inst.ldap_url}
ldap_search_base = {ldap_conn.ds_inst.base_dn}
entry_cache_user_timeout = 1
entry_cache_group_timeout = 5000
ldap_purge_cache_timeout = 3
- """).format(**locals())
- create_conf_fixture(request, conf)
- create_sssd_fixture(request)
- return None
+def test_refresh_after_cleanup_task(ldap_conn, refresh_after_cleanup_task):
- """
- Regression test for ticket:
+https://fedorahosted.org/sssd/ticket/2676
- """
- ent.assert_group_by_name(
"group2",
dict(mem=ent.contains_only("user1")))
- ent.assert_passwd_by_name(
'user1',
dict(name='user1', passwd='*', uid=1001, gid=2001,
gecos='1001', shell='/bin/bash'))
- time.sleep(15)
- ent.assert_group_by_name(
"group2",
dict(mem=ent.contains_only("user1")))
-- 2.1.0
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Michal, please fix these nitpicks.
Done. Thank you for the review. I added also one patch to add missing AM_CFLAGS (otherwise it was not possible to build with the C99 features) and one patch to add the missing blank lines in the python test in order follow PEP 8 (at least in this relatively new code).
Michal
On 08/31/2015 06:53 PM, Michal Židek wrote:
0001-Makefile.am-Add-AM_CFLAGS-for-libsss_ldap_common.patch
From b563b44a99c91c75523ea11e18cb7f7552750817 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?=mzidek@redhat.com Date: Mon, 31 Aug 2015 18:21:44 +0200 Subject: [PATCH 1/5] Makefile.am: Add AM_CFLAGS for libsss_ldap_common
Add missing AM_CFLAGS to libsss_ldap_common_la_CFLAGS.
Makefile.am | 1 + 1 file changed, 1 insertion(+)
diff --git a/Makefile.am b/Makefile.am index f153ab0..8e8da31 100644 --- a/Makefile.am +++ b/Makefile.am @@ -2835,6 +2835,7 @@ libsss_ldap_common_la_SOURCES = \ src/util/sss_ldap.c \ $(NULL) libsss_ldap_common_la_CFLAGS = \
- $(AM_CFLAGS) \ $(KRB5_CFLAGS) \ $(NULL) libsss_ldap_common_la_LIBADD = \
-- 2.1.0
Should we also amend: * libsss_cert_la_CFLAGS * libsss_crypt_la_CFLAGS * libsss_ad_common_la_CFLAGS * libsss_krb5_common_la_CFLAGS ? [snip]
0003-cleanup-task-Expire-all-memberof-targets-when-removi.patch
From 627d15d8ec9b636116debf4516404dca625ab0b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?=mzidek@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 removed from cache during cleanup task, mark all
s/user removed/user is removed/?
[snip]
0005-intg-Fix-some-PEP-8-violations.patch
From fae54334dfe4bf8220f9c4f5820e638b575ce8ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?=mzidek@redhat.com Date: Mon, 31 Aug 2015 18:47:57 +0200 Subject: [PATCH 5/5] intg: Fix some PEP 8 violations
src/tests/intg/ds.py | 1 + src/tests/intg/ds_openldap.py | 2 ++ src/tests/intg/ent.py | 38 ++++++++++++++++++++++++++++++++++++++ src/tests/intg/ent_test.py | 21 +++++++++++++++++++++ src/tests/intg/ldap_ent.py | 4 ++++ src/tests/intg/ldap_test.py | 9 +++++++++ src/tests/intg/util.py | 3 +++ 7 files changed, 78 insertions(+)
diff --git a/src/tests/intg/ds.py b/src/tests/intg/ds.py index a871902..fe93384 100644 --- a/src/tests/intg/ds.py +++ b/src/tests/intg/ds.py @@ -19,6 +19,7 @@
import ldap
- class DS: """Abstract directory server instance."""
diff --git a/src/tests/intg/ds_openldap.py b/src/tests/intg/ds_openldap.py index 42a2c90..2ece0cf 100644 --- a/src/tests/intg/ds_openldap.py +++ b/src/tests/intg/ds_openldap.py @@ -30,6 +30,7 @@ import sys from util import * from ds import DS
- def hash_password(password): """Generate userPassword value for a password.""" salt = os.urandom(4)
@@ -37,6 +38,7 @@ def hash_password(password): hash.update(salt) return "{SSHA}" + base64.standard_b64encode(hash.digest() + salt)
- class DSOpenLDAP(DS): """OpenLDAP directory server instance."""
diff --git a/src/tests/intg/ent.py b/src/tests/intg/ent.py index b3c4121..0ba7758 100644 --- a/src/tests/intg/ent.py +++ b/src/tests/intg/ent.py @@ -25,6 +25,7 @@ _PASSWD_LIST_DESC = {None: ("user", {})} _GROUP_DESC = {"mem": ("member list", {None: ("member", {})})} _GROUP_LIST_DESC = {None: ("group", _GROUP_DESC)}
- def _get_desc(desc_map, key): """ Get an item description from a container description map.
@@ -46,6 +47,7 @@ def _get_desc(desc_map, key): else: return (pformat(key), {})
- def _diff(ent, pattern, desc_map={}): """ Describe difference between an entry and a pattern.
@@ -166,6 +168,7 @@ def _diff(ent, pattern, desc_map={}):
return None
- def contains_only(*args): """ Produce a pattern matching all list items against arguments.
@@ -173,6 +176,7 @@ def contains_only(*args): """ return list(args)
- def contains(*args): """ Produce a pattern matching a subset of list items against arguments.
@@ -180,6 +184,7 @@ def contains(*args): """ return args
- def _convert_passwd(passwd): """ Convert a passwd entry returned by pwd module to an entry dictionary.
@@ -194,14 +199,17 @@ def _convert_passwd(passwd): shell = passwd.pw_shell )
def get_passwd_by_name(name): """Get a passwd database entry by name.""" return _convert_passwd(pwd.getpwnam(name))
def get_passwd_by_uid(uid): """Get a passwd database entry by UID.""" return _convert_passwd(pwd.getpwuid(uid))
def assert_passwd_by_name(name, pattern): """Assert a passwd entry, retrieved by name, matches a pattern.""" try:
@@ -211,6 +219,7 @@ def assert_passwd_by_name(name, pattern): d = _diff(ent, pattern) assert not d, d
- def assert_passwd_by_uid(uid, pattern): """Assert a passwd entry, retrieved by UID, matches a pattern.""" try:
@@ -220,6 +229,7 @@ def assert_passwd_by_uid(uid, pattern): d = _diff(ent, pattern) assert not d, d
- def get_passwd_list(): """Get passwd database entry list with root user removed.""" passwd_list = pwd.getpwall()
@@ -229,11 +239,13 @@ def get_passwd_list(): return map(_convert_passwd, passwd_list) raise Exception("no root user found")
def assert_passwd_list(pattern): """Assert retrieved passwd list matches a pattern.""" d = _diff(get_passwd_list(), pattern, _PASSWD_LIST_DESC) assert not d, d
def _diff_each_passwd_by_name(pattern_dict): """ Describe difference between each pattern_dict value and a passwd entry
@@ -245,6 +257,7 @@ def _diff_each_passwd_by_name(pattern_dict): return str(err) return _diff(ent, pattern_dict, _PASSWD_LIST_DESC)
- def _diff_each_passwd_by_uid(pattern_dict): """ Describe difference between each pattern_dict value and a passwd entry
@@ -256,6 +269,7 @@ def _diff_each_passwd_by_uid(pattern_dict): return str(err) return _diff(ent, pattern_dict, _PASSWD_LIST_DESC)
- def _diff_each_passwd_with_name(pattern_seq): """ Describe difference between each pattern in pattern_seq sequence and a
@@ -263,6 +277,7 @@ def _diff_each_passwd_with_name(pattern_seq): """ return _diff_each_passwd_by_name(dict((p["name"], p) for p in pattern_seq))
- def _diff_each_passwd_with_uid(pattern_seq): """ Describe difference between each pattern in pattern_seq sequence and a
@@ -270,6 +285,7 @@ def _diff_each_passwd_with_uid(pattern_seq): """ return _diff_each_passwd_by_uid(dict((p["uid"], p) for p in pattern_seq))
- def assert_each_passwd_by_name(pattern_dict): """ Assert each pattern_dict value matches a passwd entry retrieved by
@@ -278,6 +294,7 @@ def assert_each_passwd_by_name(pattern_dict): d = _diff_each_passwd_by_name(pattern_dict) assert not d, d
- def assert_each_passwd_by_uid(pattern_dict): """ Assert each pattern_dict value matches a passwd entry retrieved by
@@ -286,6 +303,7 @@ def assert_each_passwd_by_uid(pattern_dict): d = _diff_each_passwd_by_uid(pattern_dict) assert not d, d
- def assert_each_passwd_with_name(pattern_seq): """ Assert each pattern in pattern_seq sequence matches a passwd entry
@@ -294,6 +312,7 @@ def assert_each_passwd_with_name(pattern_seq): d = _diff_each_passwd_with_name(pattern_seq) assert not d, d
- def assert_each_passwd_with_uid(pattern_seq): """ Assert each pattern in pattern_seq sequence matches a passwd entry
@@ -302,6 +321,7 @@ def assert_each_passwd_with_uid(pattern_seq): d = _diff_each_passwd_with_uid(pattern_seq) assert not d, d
- def _diff_passwd(pattern): """ Describe difference between passwd database and a pattern.
@@ -318,6 +338,7 @@ def _diff_passwd(pattern): return "UID retrieval mismatch: " + d return None
- def assert_passwd(pattern): """ Assert passwd database matches a pattern.
@@ -326,6 +347,7 @@ def assert_passwd(pattern): d = _diff_passwd(pattern) assert not d, d
- def _convert_group(group): """ Convert a group entry returned by grp module to an entry dictionary.
@@ -337,14 +359,17 @@ def _convert_group(group): mem = group.gr_mem )
def get_group_by_name(name): """Get a group database entry by name.""" return _convert_group(grp.getgrnam(name))
def get_group_by_gid(gid): """Get a group database entry by GID.""" return _convert_group(grp.getgrgid(gid))
def assert_group_by_name(name, pattern): """Assert a group entry, retrieved by name, matches a pattern.""" try:
@@ -354,6 +379,7 @@ def assert_group_by_name(name, pattern): d = _diff(ent, pattern, _GROUP_DESC) assert not d, d
- def assert_group_by_gid(gid, pattern): """Assert a group entry, retrieved by GID, matches a pattern.""" try:
@@ -363,6 +389,7 @@ def assert_group_by_gid(gid, pattern): d = _diff(ent, pattern, _GROUP_DESC) assert not d, d
- def get_group_list(): """Get group database entry list with root group removed.""" group_list = grp.getgrall()
@@ -372,11 +399,13 @@ def get_group_list(): return map(_convert_group, group_list) raise Exception("no root group found")
def assert_group_list(pattern): """Assert retrieved group list matches a pattern.""" d = _diff(get_group_list(), pattern, _GROUP_LIST_DESC) assert not d, d
def _diff_each_group_by_name(pattern_dict): """ Describe difference between each pattern_dict value and a group entry
@@ -388,6 +417,7 @@ def _diff_each_group_by_name(pattern_dict): return str(err) return _diff(ent, pattern_dict, _GROUP_LIST_DESC)
- def _diff_each_group_by_gid(pattern_dict): """ Describe difference between each pattern_dict value and a group entry
@@ -399,6 +429,7 @@ def _diff_each_group_by_gid(pattern_dict): return str(err) return _diff(ent, pattern_dict, _GROUP_LIST_DESC)
- def _diff_each_group_with_name(pattern_seq): """ Describe difference between each pattern in pattern_seq sequence and a
@@ -406,6 +437,7 @@ def _diff_each_group_with_name(pattern_seq): """ return _diff_each_group_by_name(dict((p["name"], p) for p in pattern_seq))
- def _diff_each_group_with_gid(pattern_seq): """ Describe difference between each pattern in pattern_seq sequence and a
@@ -413,6 +445,7 @@ def _diff_each_group_with_gid(pattern_seq): """ return _diff_each_group_by_gid(dict((p["gid"], p) for p in pattern_seq))
- def assert_each_group_by_name(pattern_dict): """ Assert each pattern_dict value matches a group entry retrieved by
@@ -421,6 +454,7 @@ def assert_each_group_by_name(pattern_dict): d = _diff_each_group_by_name(pattern_dict) assert not d, d
- def assert_each_group_by_gid(pattern_dict): """ Assert each pattern_dict value matches a group entry retrieved by
@@ -429,6 +463,7 @@ def assert_each_group_by_gid(pattern_dict): d = _diff_each_group_by_gid(pattern_dict) assert not d, d
- def assert_each_group_with_name(pattern_seq): """ Assert each pattern in pattern_seq sequence matches a group entry
@@ -437,6 +472,7 @@ def assert_each_group_with_name(pattern_seq): d = _diff_each_group_with_name(pattern_seq) assert not d, d
- def assert_each_group_with_gid(pattern_seq): """ Assert each pattern in pattern_seq sequence matches a group entry
@@ -445,6 +481,7 @@ def assert_each_group_with_gid(pattern_seq): d = _diff_each_group_with_gid(pattern_seq) assert not d, d
- def _diff_group(pattern): """ Describe difference between group database and a pattern.
@@ -461,6 +498,7 @@ def _diff_group(pattern): return "GID retrieval mismatch: " + d return None
- def assert_group(pattern): """ Assert group database matches a pattern.
diff --git a/src/tests/intg/ent_test.py b/src/tests/intg/ent_test.py index a50e82d..7c044c7 100644 --- a/src/tests/intg/ent_test.py +++ b/src/tests/intg/ent_test.py @@ -24,29 +24,34 @@ import pytest import ent from util import *
def backup_envvar_file(name): path = os.environ[name] backup_path = path + ".bak" shutil.copyfile(path, backup_path) return path
def restore_envvar_file(name): path = os.environ[name] backup_path = path + ".bak" os.rename(backup_path, path)
@pytest.fixture(scope="module") def passwd_path(request): name = "NSS_WRAPPER_PASSWD" request.addfinalizer(lambda: restore_envvar_file(name)) return backup_envvar_file(name)
@pytest.fixture(scope="module") def group_path(request): name = "NSS_WRAPPER_GROUP" request.addfinalizer(lambda: restore_envvar_file(name)) return backup_envvar_file(name)
USER1 = dict(name="user1", passwd="x", uid=1001, gid=2001, gecos="User 1", dir="/home/user1", shell="/bin/bash") USER2 = dict(name="user2", passwd="x", uid=1002, gid=2002,
@@ -77,6 +82,7 @@ GROUP_LIST = [EMPTY_GROUP, GROUP_NAME_DICT = dict((g["name"], g) for g in GROUP_LIST) GROUP_GID_DICT = dict((g["gid"], g) for g in GROUP_LIST)
- @pytest.fixture(scope="module") def users_and_groups(request, passwd_path, group_path): passwd_contents = "".join([
@@ -94,6 +100,7 @@ def users_and_groups(request, passwd_path, group_path): with open(group_path, "a") as f: f.write(group_contents)
- def test_assert_passwd_by_name(users_and_groups): ent.assert_passwd_by_name("user1", {}) ent.assert_passwd_by_name("user1", dict(name="user1", uid=1001))
@@ -111,6 +118,7 @@ def test_assert_passwd_by_name(users_and_groups): except AssertionError as e: assert str(e) == "'name' mismatch: 'user1' != 'user2'"
- def test_assert_passwd_by_uid(users_and_groups): ent.assert_passwd_by_uid(1001, {}) ent.assert_passwd_by_uid(1001, dict(name="user1", uid=1001))
@@ -146,6 +154,7 @@ def test_assert_passwd_list(users_and_groups): assert re.search("expected users not found:", str(e)) assert not re.search("unexpected users found:", str(e))
- def test_assert_each_passwd_by_name(users_and_groups): ent.assert_each_passwd_by_name({}) ent.assert_each_passwd_by_name(dict(user1=USER1))
@@ -162,6 +171,7 @@ def test_assert_each_passwd_by_name(users_and_groups): assert str(e) == \ "user 'user1' mismatch: 'name' mismatch: 'user2' != 'user1'"
- def test_assert_each_passwd_by_uid(users_and_groups): ent.assert_each_passwd_by_uid({}) ent.assert_each_passwd_by_uid({1001:USER1})
@@ -178,6 +188,7 @@ def test_assert_each_passwd_by_uid(users_and_groups): assert str(e) == \ "user 1001 mismatch: 'uid' mismatch: 1002 != 1001"
- def test_assert_each_passwd_with_name(users_and_groups): ent.assert_each_passwd_with_name([]) ent.assert_each_passwd_with_name([USER1])
@@ -194,6 +205,7 @@ def test_assert_each_passwd_with_name(users_and_groups): assert str(e) == \ "user 'user1' mismatch: 'uid' mismatch: 1002 != 1001"
- def test_assert_each_passwd_with_uid(users_and_groups): ent.assert_each_passwd_with_uid([]) ent.assert_each_passwd_with_uid([USER1])
@@ -210,6 +222,7 @@ def test_assert_each_passwd_with_uid(users_and_groups): assert str(e) == \ "user 1001 mismatch: 'name' mismatch: 'user2' != 'user1'"
- def test_assert_passwd(users_and_groups): ent.assert_passwd(ent.contains()) ent.assert_passwd(ent.contains(USER1))
@@ -229,6 +242,7 @@ def test_assert_passwd(users_and_groups): assert not re.search("expected users not found:", str(e)) assert re.search("unexpected users found:", str(e))
- def test_group_member_matching(users_and_groups): ent.assert_group_by_name("empty_group", dict(mem=ent.contains())) ent.assert_group_by_name("empty_group", dict(mem=ent.contains_only()))
@@ -281,6 +295,7 @@ def test_group_member_matching(users_and_groups): assert re.search("unexpected members found:", str(e)) assert not re.search("expected members not found:", str(e))
- def test_assert_group_by_name(users_and_groups): ent.assert_group_by_name("group1", {}) ent.assert_group_by_name("group1", dict(name="group1", gid=2001))
@@ -298,6 +313,7 @@ def test_assert_group_by_name(users_and_groups): except AssertionError as e: assert str(e) == "'name' mismatch: 'group1' != 'group2'"
- def test_assert_group_by_gid(users_and_groups): ent.assert_group_by_gid(2001, {}) ent.assert_group_by_gid(2001, dict(name="group1", gid=2001))
@@ -333,6 +349,7 @@ def test_assert_group_list(users_and_groups): assert re.search("expected groups not found:", str(e)) assert not re.search("unexpected groups found:", str(e))
- def test_assert_each_group_by_name(users_and_groups): ent.assert_each_group_by_name({}) ent.assert_each_group_by_name(dict(group1=GROUP1))
@@ -349,6 +366,7 @@ def test_assert_each_group_by_name(users_and_groups): assert str(e) == "group 'group1' mismatch: " + \ "'name' mismatch: 'group2' != 'group1'"
- def test_assert_each_group_by_gid(users_and_groups): ent.assert_each_group_by_gid({}) ent.assert_each_group_by_gid({2001:GROUP1})
@@ -365,6 +383,7 @@ def test_assert_each_group_by_gid(users_and_groups): assert str(e) == \ "group 2001 mismatch: 'gid' mismatch: 2002 != 2001"
- def test_assert_each_group_with_name(users_and_groups): ent.assert_each_group_with_name([]) ent.assert_each_group_with_name([GROUP1])
@@ -381,6 +400,7 @@ def test_assert_each_group_with_name(users_and_groups): assert str(e) == \ "group 'group1' mismatch: 'gid' mismatch: 2002 != 2001"
- def test_assert_each_group_with_gid(users_and_groups): ent.assert_each_group_with_gid([]) ent.assert_each_group_with_gid([GROUP1])
@@ -397,6 +417,7 @@ def test_assert_each_group_with_gid(users_and_groups): assert str(e) == \ "group 2001 mismatch: 'name' mismatch: 'group2' != 'group1'"
- def test_assert_group(users_and_groups): ent.assert_group(ent.contains()) ent.assert_group(ent.contains(GROUP1))
diff --git a/src/tests/intg/ldap_ent.py b/src/tests/intg/ldap_ent.py index ef2d147..30eed9d 100644 --- a/src/tests/intg/ldap_ent.py +++ b/src/tests/intg/ldap_ent.py @@ -17,6 +17,7 @@ # along with this program. If not, seehttp://www.gnu.org/licenses/. #
- def user(base_dn, uid, uidNumber, gidNumber): """ Generate an RFC2307(bis) user add-modlist for passing to ldap.add*
@@ -37,6 +38,7 @@ def user(base_dn, uid, uidNumber, gidNumber): ] )
- def group(base_dn, cn, gidNumber, member_uids=[]): """ Generate an RFC2307 group add-modlist for passing to ldap.add*.
@@ -50,6 +52,7 @@ def group(base_dn, cn, gidNumber, member_uids=[]): attr_list.append(('memberUid', member_uids)) return ("cn=" + cn + ",ou=Groups," + base_dn, attr_list)
- def group_bis(base_dn, cn, gidNumber, member_uids=[], member_gids=[]): """ Generate an RFC2307bis group add-modlist for passing to ldap.add*.
@@ -75,6 +78,7 @@ def group_bis(base_dn, cn, gidNumber, member_uids=[], member_gids=[]): ) return ("cn=" + cn + ",ou=Groups," + base_dn, attr_list)
- class List(list): """LDAP add-modlist list"""
diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py index b0a4dc6..6bd7f2a 100644 --- a/src/tests/intg/ldap_test.py +++ b/src/tests/intg/ldap_test.py @@ -34,6 +34,7 @@ from util import *
LDAP_BASE_DN="dc=example,dc=com"
- @pytest.fixture(scope="module") def ds_inst(request): """LDAP server instance fixture"""
@@ -48,6 +49,7 @@ def ds_inst(request): request.addfinalizer(lambda: ds_inst.teardown()) return ds_inst
- @pytest.fixture(scope="module") def ldap_conn(request, ds_inst): """LDAP server connection fixture"""
@@ -56,6 +58,7 @@ def ldap_conn(request, ds_inst): request.addfinalizer(lambda: ldap_conn.unbind_s()) return ldap_conn
- def create_ldap_fixture(request, ldap_conn, ent_list): """Add LDAP entries and add teardown for removing them""" for entry in ent_list:
@@ -65,6 +68,7 @@ def create_ldap_fixture(request, ldap_conn, ent_list): ldap_conn.delete_s(entry[0]) request.addfinalizer(teardown)
- def create_conf_fixture(request, contents): """Generate sssd.conf and add teardown for removing it""" conf = open(config.CONF_PATH, "w")
@@ -73,6 +77,7 @@ def create_conf_fixture(request, contents): os.chmod(config.CONF_PATH, stat.S_IRUSR | stat.S_IWUSR) request.addfinalizer(lambda: os.unlink(config.CONF_PATH))
- def create_sssd_fixture(request): """Start sssd and add teardown for stopping it and removing state""" if subprocess.call(["sssd", "-D", "-f"]) != 0:
@@ -97,6 +102,7 @@ def create_sssd_fixture(request): os.unlink(config.MCACHE_PATH + "/" + path) request.addfinalizer(teardown)
- @pytest.fixture def sanity_rfc2307(request, ldap_conn): ent_list = ldap_ent.List(LDAP_BASE_DN)
@@ -142,6 +148,7 @@ def sanity_rfc2307(request, ldap_conn): create_sssd_fixture(request) return None
- @pytest.fixture def sanity_rfc2307_bis(request, ldap_conn): ent_list = ldap_ent.List(LDAP_BASE_DN)
@@ -201,6 +208,7 @@ def sanity_rfc2307_bis(request, ldap_conn): create_sssd_fixture(request) return None
- def test_sanity_rfc2307(ldap_conn, sanity_rfc2307): passwd_pattern = ent.contains_only( dict(name='user1', passwd='*', uid=1001, gid=2001, gecos='1001', dir='/home/user1', shell='/bin/bash'),
@@ -227,6 +235,7 @@ def test_sanity_rfc2307(ldap_conn, sanity_rfc2307): with pytest.raises(KeyError): grp.getgrgid(1)
- def test_sanity_rfc2307_bis(ldap_conn, sanity_rfc2307_bis): passwd_pattern = ent.contains_only( dict(name='user1', passwd='*', uid=1001, gid=2001, gecos='1001', dir='/home/user1', shell='/bin/bash'),
diff --git a/src/tests/intg/util.py b/src/tests/intg/util.py index 5dd92b2..6e8f15d 100644 --- a/src/tests/intg/util.py +++ b/src/tests/intg/util.py @@ -23,6 +23,7 @@ import subprocess
UNINDENT_RE = re.compile("^ +", re.MULTILINE)
- def unindent(text): """ Unindent text by removing at most the number of spaces present in
@@ -35,6 +36,7 @@ def unindent(text): return match.group()[indent_ref[0]:] return UNINDENT_RE.sub(replace, text)
- def run_shell(): """ Execute an interactive shell under "screen", preserving environment.
@@ -48,6 +50,7 @@ def run_shell(): "bash -i" ])
- def first_dir(*args): """Return first argument that points to an existing directory.""" for arg in args:
-- 2.1.0
This patch fixes some of the pep8 warnings (number of issues reported by pep8 utility changed from 204 to 127). Should we file a ticket to reduce pep8 warning as much as possible? I suppose we could even add a test that would fail if new pep8 warning is introduced, but I'm little worried that it might prove to be too difficult to write such a test in quality that it will be accepted by all SSSD developers. What do you think?
Now, I'm going to test the patch.
On 09/01/2015 02:37 PM, Pavel Reichl wrote:
On 08/31/2015 06:53 PM, Michal Židek wrote:
0001-Makefile.am-Add-AM_CFLAGS-for-libsss_ldap_common.patch
From b563b44a99c91c75523ea11e18cb7f7552750817 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?=mzidek@redhat.com Date: Mon, 31 Aug 2015 18:21:44 +0200 Subject: [PATCH 1/5] Makefile.am: Add AM_CFLAGS for libsss_ldap_common
Add missing AM_CFLAGS to libsss_ldap_common_la_CFLAGS.
Makefile.am | 1 + 1 file changed, 1 insertion(+)
diff --git a/Makefile.am b/Makefile.am index f153ab0..8e8da31 100644 --- a/Makefile.am +++ b/Makefile.am @@ -2835,6 +2835,7 @@ libsss_ldap_common_la_SOURCES = \ src/util/sss_ldap.c \ $(NULL) libsss_ldap_common_la_CFLAGS = \
- $(AM_CFLAGS) \ $(KRB5_CFLAGS) \ $(NULL) libsss_ldap_common_la_LIBADD = \
-- 2.1.0
Should we also amend:
- libsss_cert_la_CFLAGS
- libsss_crypt_la_CFLAGS
- libsss_ad_common_la_CFLAGS
- libsss_krb5_common_la_CFLAGS
?
I added AM_CFLAGS to those as well.
[snip]
0003-cleanup-task-Expire-all-memberof-targets-when-removi.patch
From 627d15d8ec9b636116debf4516404dca625ab0b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?=mzidek@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 removed from cache during cleanup task, mark all
s/user removed/user is removed/?
[snip]
0005-intg-Fix-some-PEP-8-violations.patch
From fae54334dfe4bf8220f9c4f5820e638b575ce8ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?=mzidek@redhat.com Date: Mon, 31 Aug 2015 18:47:57 +0200 Subject: [PATCH 5/5] intg: Fix some PEP 8 violations
src/tests/intg/ds.py | 1 + src/tests/intg/ds_openldap.py | 2 ++ src/tests/intg/ent.py | 38 ++++++++++++++++++++++++++++++++++++++ src/tests/intg/ent_test.py | 21 +++++++++++++++++++++ src/tests/intg/ldap_ent.py | 4 ++++ src/tests/intg/ldap_test.py | 9 +++++++++ src/tests/intg/util.py | 3 +++ 7 files changed, 78 insertions(+)
diff --git a/src/tests/intg/ds.py b/src/tests/intg/ds.py index a871902..fe93384 100644 --- a/src/tests/intg/ds.py +++ b/src/tests/intg/ds.py @@ -19,6 +19,7 @@
import ldap
- class DS: """Abstract directory server instance."""
diff --git a/src/tests/intg/ds_openldap.py b/src/tests/intg/ds_openldap.py index 42a2c90..2ece0cf 100644 --- a/src/tests/intg/ds_openldap.py +++ b/src/tests/intg/ds_openldap.py @@ -30,6 +30,7 @@ import sys from util import * from ds import DS
- def hash_password(password): """Generate userPassword value for a password.""" salt = os.urandom(4)
@@ -37,6 +38,7 @@ def hash_password(password): hash.update(salt) return "{SSHA}" + base64.standard_b64encode(hash.digest() + salt)
- class DSOpenLDAP(DS): """OpenLDAP directory server instance."""
diff --git a/src/tests/intg/ent.py b/src/tests/intg/ent.py index b3c4121..0ba7758 100644 --- a/src/tests/intg/ent.py +++ b/src/tests/intg/ent.py @@ -25,6 +25,7 @@ _PASSWD_LIST_DESC = {None: ("user", {})} _GROUP_DESC = {"mem": ("member list", {None: ("member", {})})} _GROUP_LIST_DESC = {None: ("group", _GROUP_DESC)}
- def _get_desc(desc_map, key): """ Get an item description from a container description map.
@@ -46,6 +47,7 @@ def _get_desc(desc_map, key): else: return (pformat(key), {})
- def _diff(ent, pattern, desc_map={}): """ Describe difference between an entry and a pattern.
@@ -166,6 +168,7 @@ def _diff(ent, pattern, desc_map={}):
return None
- def contains_only(*args): """ Produce a pattern matching all list items against arguments.
@@ -173,6 +176,7 @@ def contains_only(*args): """ return list(args)
- def contains(*args): """ Produce a pattern matching a subset of list items against arguments.
@@ -180,6 +184,7 @@ def contains(*args): """ return args
- def _convert_passwd(passwd): """ Convert a passwd entry returned by pwd module to an entry dictionary.
@@ -194,14 +199,17 @@ def _convert_passwd(passwd): shell = passwd.pw_shell )
def get_passwd_by_name(name): """Get a passwd database entry by name.""" return _convert_passwd(pwd.getpwnam(name))
def get_passwd_by_uid(uid): """Get a passwd database entry by UID.""" return _convert_passwd(pwd.getpwuid(uid))
def assert_passwd_by_name(name, pattern): """Assert a passwd entry, retrieved by name, matches a pattern.""" try:
@@ -211,6 +219,7 @@ def assert_passwd_by_name(name, pattern): d = _diff(ent, pattern) assert not d, d
- def assert_passwd_by_uid(uid, pattern): """Assert a passwd entry, retrieved by UID, matches a pattern.""" try:
@@ -220,6 +229,7 @@ def assert_passwd_by_uid(uid, pattern): d = _diff(ent, pattern) assert not d, d
- def get_passwd_list(): """Get passwd database entry list with root user removed.""" passwd_list = pwd.getpwall()
@@ -229,11 +239,13 @@ def get_passwd_list(): return map(_convert_passwd, passwd_list) raise Exception("no root user found")
def assert_passwd_list(pattern): """Assert retrieved passwd list matches a pattern.""" d = _diff(get_passwd_list(), pattern, _PASSWD_LIST_DESC) assert not d, d
def _diff_each_passwd_by_name(pattern_dict): """ Describe difference between each pattern_dict value and a passwd entry
@@ -245,6 +257,7 @@ def _diff_each_passwd_by_name(pattern_dict): return str(err) return _diff(ent, pattern_dict, _PASSWD_LIST_DESC)
- def _diff_each_passwd_by_uid(pattern_dict): """ Describe difference between each pattern_dict value and a passwd entry
@@ -256,6 +269,7 @@ def _diff_each_passwd_by_uid(pattern_dict): return str(err) return _diff(ent, pattern_dict, _PASSWD_LIST_DESC)
- def _diff_each_passwd_with_name(pattern_seq): """ Describe difference between each pattern in pattern_seq sequence and a
@@ -263,6 +277,7 @@ def _diff_each_passwd_with_name(pattern_seq): """ return _diff_each_passwd_by_name(dict((p["name"], p) for p in pattern_seq))
- def _diff_each_passwd_with_uid(pattern_seq): """ Describe difference between each pattern in pattern_seq sequence and a
@@ -270,6 +285,7 @@ def _diff_each_passwd_with_uid(pattern_seq): """ return _diff_each_passwd_by_uid(dict((p["uid"], p) for p in pattern_seq))
- def assert_each_passwd_by_name(pattern_dict): """ Assert each pattern_dict value matches a passwd entry retrieved by
@@ -278,6 +294,7 @@ def assert_each_passwd_by_name(pattern_dict): d = _diff_each_passwd_by_name(pattern_dict) assert not d, d
- def assert_each_passwd_by_uid(pattern_dict): """ Assert each pattern_dict value matches a passwd entry retrieved by
@@ -286,6 +303,7 @@ def assert_each_passwd_by_uid(pattern_dict): d = _diff_each_passwd_by_uid(pattern_dict) assert not d, d
- def assert_each_passwd_with_name(pattern_seq): """ Assert each pattern in pattern_seq sequence matches a passwd entry
@@ -294,6 +312,7 @@ def assert_each_passwd_with_name(pattern_seq): d = _diff_each_passwd_with_name(pattern_seq) assert not d, d
- def assert_each_passwd_with_uid(pattern_seq): """ Assert each pattern in pattern_seq sequence matches a passwd entry
@@ -302,6 +321,7 @@ def assert_each_passwd_with_uid(pattern_seq): d = _diff_each_passwd_with_uid(pattern_seq) assert not d, d
- def _diff_passwd(pattern): """ Describe difference between passwd database and a pattern.
@@ -318,6 +338,7 @@ def _diff_passwd(pattern): return "UID retrieval mismatch: " + d return None
- def assert_passwd(pattern): """ Assert passwd database matches a pattern.
@@ -326,6 +347,7 @@ def assert_passwd(pattern): d = _diff_passwd(pattern) assert not d, d
- def _convert_group(group): """ Convert a group entry returned by grp module to an entry dictionary.
@@ -337,14 +359,17 @@ def _convert_group(group): mem = group.gr_mem )
def get_group_by_name(name): """Get a group database entry by name.""" return _convert_group(grp.getgrnam(name))
def get_group_by_gid(gid): """Get a group database entry by GID.""" return _convert_group(grp.getgrgid(gid))
def assert_group_by_name(name, pattern): """Assert a group entry, retrieved by name, matches a pattern.""" try:
@@ -354,6 +379,7 @@ def assert_group_by_name(name, pattern): d = _diff(ent, pattern, _GROUP_DESC) assert not d, d
- def assert_group_by_gid(gid, pattern): """Assert a group entry, retrieved by GID, matches a pattern.""" try:
@@ -363,6 +389,7 @@ def assert_group_by_gid(gid, pattern): d = _diff(ent, pattern, _GROUP_DESC) assert not d, d
- def get_group_list(): """Get group database entry list with root group removed.""" group_list = grp.getgrall()
@@ -372,11 +399,13 @@ def get_group_list(): return map(_convert_group, group_list) raise Exception("no root group found")
def assert_group_list(pattern): """Assert retrieved group list matches a pattern.""" d = _diff(get_group_list(), pattern, _GROUP_LIST_DESC) assert not d, d
def _diff_each_group_by_name(pattern_dict): """ Describe difference between each pattern_dict value and a group entry
@@ -388,6 +417,7 @@ def _diff_each_group_by_name(pattern_dict): return str(err) return _diff(ent, pattern_dict, _GROUP_LIST_DESC)
- def _diff_each_group_by_gid(pattern_dict): """ Describe difference between each pattern_dict value and a group entry
@@ -399,6 +429,7 @@ def _diff_each_group_by_gid(pattern_dict): return str(err) return _diff(ent, pattern_dict, _GROUP_LIST_DESC)
- def _diff_each_group_with_name(pattern_seq): """ Describe difference between each pattern in pattern_seq sequence and a
@@ -406,6 +437,7 @@ def _diff_each_group_with_name(pattern_seq): """ return _diff_each_group_by_name(dict((p["name"], p) for p in pattern_seq))
- def _diff_each_group_with_gid(pattern_seq): """ Describe difference between each pattern in pattern_seq sequence and a
@@ -413,6 +445,7 @@ def _diff_each_group_with_gid(pattern_seq): """ return _diff_each_group_by_gid(dict((p["gid"], p) for p in pattern_seq))
- def assert_each_group_by_name(pattern_dict): """ Assert each pattern_dict value matches a group entry retrieved by
@@ -421,6 +454,7 @@ def assert_each_group_by_name(pattern_dict): d = _diff_each_group_by_name(pattern_dict) assert not d, d
- def assert_each_group_by_gid(pattern_dict): """ Assert each pattern_dict value matches a group entry retrieved by
@@ -429,6 +463,7 @@ def assert_each_group_by_gid(pattern_dict): d = _diff_each_group_by_gid(pattern_dict) assert not d, d
- def assert_each_group_with_name(pattern_seq): """ Assert each pattern in pattern_seq sequence matches a group entry
@@ -437,6 +472,7 @@ def assert_each_group_with_name(pattern_seq): d = _diff_each_group_with_name(pattern_seq) assert not d, d
- def assert_each_group_with_gid(pattern_seq): """ Assert each pattern in pattern_seq sequence matches a group entry
@@ -445,6 +481,7 @@ def assert_each_group_with_gid(pattern_seq): d = _diff_each_group_with_gid(pattern_seq) assert not d, d
- def _diff_group(pattern): """ Describe difference between group database and a pattern.
@@ -461,6 +498,7 @@ def _diff_group(pattern): return "GID retrieval mismatch: " + d return None
- def assert_group(pattern): """ Assert group database matches a pattern.
diff --git a/src/tests/intg/ent_test.py b/src/tests/intg/ent_test.py index a50e82d..7c044c7 100644 --- a/src/tests/intg/ent_test.py +++ b/src/tests/intg/ent_test.py @@ -24,29 +24,34 @@ import pytest import ent from util import *
def backup_envvar_file(name): path = os.environ[name] backup_path = path + ".bak" shutil.copyfile(path, backup_path) return path
def restore_envvar_file(name): path = os.environ[name] backup_path = path + ".bak" os.rename(backup_path, path)
@pytest.fixture(scope="module") def passwd_path(request): name = "NSS_WRAPPER_PASSWD" request.addfinalizer(lambda: restore_envvar_file(name)) return backup_envvar_file(name)
@pytest.fixture(scope="module") def group_path(request): name = "NSS_WRAPPER_GROUP" request.addfinalizer(lambda: restore_envvar_file(name)) return backup_envvar_file(name)
USER1 = dict(name="user1", passwd="x", uid=1001, gid=2001, gecos="User 1", dir="/home/user1", shell="/bin/bash") USER2 = dict(name="user2", passwd="x", uid=1002, gid=2002,
@@ -77,6 +82,7 @@ GROUP_LIST = [EMPTY_GROUP, GROUP_NAME_DICT = dict((g["name"], g) for g in GROUP_LIST) GROUP_GID_DICT = dict((g["gid"], g) for g in GROUP_LIST)
- @pytest.fixture(scope="module") def users_and_groups(request, passwd_path, group_path): passwd_contents = "".join([
@@ -94,6 +100,7 @@ def users_and_groups(request, passwd_path, group_path): with open(group_path, "a") as f: f.write(group_contents)
- def test_assert_passwd_by_name(users_and_groups): ent.assert_passwd_by_name("user1", {}) ent.assert_passwd_by_name("user1", dict(name="user1", uid=1001))
@@ -111,6 +118,7 @@ def test_assert_passwd_by_name(users_and_groups): except AssertionError as e: assert str(e) == "'name' mismatch: 'user1' != 'user2'"
- def test_assert_passwd_by_uid(users_and_groups): ent.assert_passwd_by_uid(1001, {}) ent.assert_passwd_by_uid(1001, dict(name="user1", uid=1001))
@@ -146,6 +154,7 @@ def test_assert_passwd_list(users_and_groups): assert re.search("expected users not found:", str(e)) assert not re.search("unexpected users found:", str(e))
- def test_assert_each_passwd_by_name(users_and_groups): ent.assert_each_passwd_by_name({}) ent.assert_each_passwd_by_name(dict(user1=USER1))
@@ -162,6 +171,7 @@ def test_assert_each_passwd_by_name(users_and_groups): assert str(e) == \ "user 'user1' mismatch: 'name' mismatch: 'user2' != 'user1'"
- def test_assert_each_passwd_by_uid(users_and_groups): ent.assert_each_passwd_by_uid({}) ent.assert_each_passwd_by_uid({1001:USER1})
@@ -178,6 +188,7 @@ def test_assert_each_passwd_by_uid(users_and_groups): assert str(e) == \ "user 1001 mismatch: 'uid' mismatch: 1002 != 1001"
- def test_assert_each_passwd_with_name(users_and_groups): ent.assert_each_passwd_with_name([]) ent.assert_each_passwd_with_name([USER1])
@@ -194,6 +205,7 @@ def test_assert_each_passwd_with_name(users_and_groups): assert str(e) == \ "user 'user1' mismatch: 'uid' mismatch: 1002 != 1001"
- def test_assert_each_passwd_with_uid(users_and_groups): ent.assert_each_passwd_with_uid([]) ent.assert_each_passwd_with_uid([USER1])
@@ -210,6 +222,7 @@ def test_assert_each_passwd_with_uid(users_and_groups): assert str(e) == \ "user 1001 mismatch: 'name' mismatch: 'user2' != 'user1'"
- def test_assert_passwd(users_and_groups): ent.assert_passwd(ent.contains()) ent.assert_passwd(ent.contains(USER1))
@@ -229,6 +242,7 @@ def test_assert_passwd(users_and_groups): assert not re.search("expected users not found:", str(e)) assert re.search("unexpected users found:", str(e))
- def test_group_member_matching(users_and_groups): ent.assert_group_by_name("empty_group", dict(mem=ent.contains())) ent.assert_group_by_name("empty_group", dict(mem=ent.contains_only()))
@@ -281,6 +295,7 @@ def test_group_member_matching(users_and_groups): assert re.search("unexpected members found:", str(e)) assert not re.search("expected members not found:", str(e))
- def test_assert_group_by_name(users_and_groups): ent.assert_group_by_name("group1", {}) ent.assert_group_by_name("group1", dict(name="group1", gid=2001))
@@ -298,6 +313,7 @@ def test_assert_group_by_name(users_and_groups): except AssertionError as e: assert str(e) == "'name' mismatch: 'group1' != 'group2'"
- def test_assert_group_by_gid(users_and_groups): ent.assert_group_by_gid(2001, {}) ent.assert_group_by_gid(2001, dict(name="group1", gid=2001))
@@ -333,6 +349,7 @@ def test_assert_group_list(users_and_groups): assert re.search("expected groups not found:", str(e)) assert not re.search("unexpected groups found:", str(e))
- def test_assert_each_group_by_name(users_and_groups): ent.assert_each_group_by_name({}) ent.assert_each_group_by_name(dict(group1=GROUP1))
@@ -349,6 +366,7 @@ def test_assert_each_group_by_name(users_and_groups): assert str(e) == "group 'group1' mismatch: " + \ "'name' mismatch: 'group2' != 'group1'"
- def test_assert_each_group_by_gid(users_and_groups): ent.assert_each_group_by_gid({}) ent.assert_each_group_by_gid({2001:GROUP1})
@@ -365,6 +383,7 @@ def test_assert_each_group_by_gid(users_and_groups): assert str(e) == \ "group 2001 mismatch: 'gid' mismatch: 2002 != 2001"
- def test_assert_each_group_with_name(users_and_groups): ent.assert_each_group_with_name([]) ent.assert_each_group_with_name([GROUP1])
@@ -381,6 +400,7 @@ def test_assert_each_group_with_name(users_and_groups): assert str(e) == \ "group 'group1' mismatch: 'gid' mismatch: 2002 != 2001"
- def test_assert_each_group_with_gid(users_and_groups): ent.assert_each_group_with_gid([]) ent.assert_each_group_with_gid([GROUP1])
@@ -397,6 +417,7 @@ def test_assert_each_group_with_gid(users_and_groups): assert str(e) == \ "group 2001 mismatch: 'name' mismatch: 'group2' != 'group1'"
- def test_assert_group(users_and_groups): ent.assert_group(ent.contains()) ent.assert_group(ent.contains(GROUP1))
diff --git a/src/tests/intg/ldap_ent.py b/src/tests/intg/ldap_ent.py index ef2d147..30eed9d 100644 --- a/src/tests/intg/ldap_ent.py +++ b/src/tests/intg/ldap_ent.py @@ -17,6 +17,7 @@ # along with this program. If not, seehttp://www.gnu.org/licenses/. #
- def user(base_dn, uid, uidNumber, gidNumber): """ Generate an RFC2307(bis) user add-modlist for passing to ldap.add*
@@ -37,6 +38,7 @@ def user(base_dn, uid, uidNumber, gidNumber): ] )
- def group(base_dn, cn, gidNumber, member_uids=[]): """ Generate an RFC2307 group add-modlist for passing to ldap.add*.
@@ -50,6 +52,7 @@ def group(base_dn, cn, gidNumber, member_uids=[]): attr_list.append(('memberUid', member_uids)) return ("cn=" + cn + ",ou=Groups," + base_dn, attr_list)
- def group_bis(base_dn, cn, gidNumber, member_uids=[], member_gids=[]): """ Generate an RFC2307bis group add-modlist for passing to ldap.add*.
@@ -75,6 +78,7 @@ def group_bis(base_dn, cn, gidNumber, member_uids=[], member_gids=[]): ) return ("cn=" + cn + ",ou=Groups," + base_dn, attr_list)
- class List(list): """LDAP add-modlist list"""
diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py index b0a4dc6..6bd7f2a 100644 --- a/src/tests/intg/ldap_test.py +++ b/src/tests/intg/ldap_test.py @@ -34,6 +34,7 @@ from util import *
LDAP_BASE_DN="dc=example,dc=com"
- @pytest.fixture(scope="module") def ds_inst(request): """LDAP server instance fixture"""
@@ -48,6 +49,7 @@ def ds_inst(request): request.addfinalizer(lambda: ds_inst.teardown()) return ds_inst
- @pytest.fixture(scope="module") def ldap_conn(request, ds_inst): """LDAP server connection fixture"""
@@ -56,6 +58,7 @@ def ldap_conn(request, ds_inst): request.addfinalizer(lambda: ldap_conn.unbind_s()) return ldap_conn
- def create_ldap_fixture(request, ldap_conn, ent_list): """Add LDAP entries and add teardown for removing them""" for entry in ent_list:
@@ -65,6 +68,7 @@ def create_ldap_fixture(request, ldap_conn, ent_list): ldap_conn.delete_s(entry[0]) request.addfinalizer(teardown)
- def create_conf_fixture(request, contents): """Generate sssd.conf and add teardown for removing it""" conf = open(config.CONF_PATH, "w")
@@ -73,6 +77,7 @@ def create_conf_fixture(request, contents): os.chmod(config.CONF_PATH, stat.S_IRUSR | stat.S_IWUSR) request.addfinalizer(lambda: os.unlink(config.CONF_PATH))
- def create_sssd_fixture(request): """Start sssd and add teardown for stopping it and removing state""" if subprocess.call(["sssd", "-D", "-f"]) != 0:
@@ -97,6 +102,7 @@ def create_sssd_fixture(request): os.unlink(config.MCACHE_PATH + "/" + path) request.addfinalizer(teardown)
- @pytest.fixture def sanity_rfc2307(request, ldap_conn): ent_list = ldap_ent.List(LDAP_BASE_DN)
@@ -142,6 +148,7 @@ def sanity_rfc2307(request, ldap_conn): create_sssd_fixture(request) return None
- @pytest.fixture def sanity_rfc2307_bis(request, ldap_conn): ent_list = ldap_ent.List(LDAP_BASE_DN)
@@ -201,6 +208,7 @@ def sanity_rfc2307_bis(request, ldap_conn): create_sssd_fixture(request) return None
- def test_sanity_rfc2307(ldap_conn, sanity_rfc2307): passwd_pattern = ent.contains_only( dict(name='user1', passwd='*', uid=1001, gid=2001, gecos='1001', dir='/home/user1', shell='/bin/bash'),
@@ -227,6 +235,7 @@ def test_sanity_rfc2307(ldap_conn, sanity_rfc2307): with pytest.raises(KeyError): grp.getgrgid(1)
- def test_sanity_rfc2307_bis(ldap_conn, sanity_rfc2307_bis): passwd_pattern = ent.contains_only( dict(name='user1', passwd='*', uid=1001, gid=2001, gecos='1001', dir='/home/user1', shell='/bin/bash'),
diff --git a/src/tests/intg/util.py b/src/tests/intg/util.py index 5dd92b2..6e8f15d 100644 --- a/src/tests/intg/util.py +++ b/src/tests/intg/util.py @@ -23,6 +23,7 @@ import subprocess
UNINDENT_RE = re.compile("^ +", re.MULTILINE)
- def unindent(text): """ Unindent text by removing at most the number of spaces present in
@@ -35,6 +36,7 @@ def unindent(text): return match.group()[indent_ref[0]:] return UNINDENT_RE.sub(replace, text)
- def run_shell(): """ Execute an interactive shell under "screen", preserving environment.
@@ -48,6 +50,7 @@ def run_shell(): "bash -i" ])
- def first_dir(*args): """Return first argument that points to an existing directory.""" for arg in args:
-- 2.1.0
This patch fixes some of the pep8 warnings (number of issues reported by pep8 utility changed from 204 to 127). Should we file a ticket to reduce pep8 warning as much as possible? I suppose we could even add a test that would fail if new pep8 warning is introduced, but I'm little worried that it might prove to be too difficult to write such a test in quality that it will be accepted by all SSSD developers. What do you think?
I just wanted to add a little consistency to the intgr code (especially because I felt responsible for this particular one). I am not against filing such ticket, we should follow PEP 8 where possible, but the ticket should have low priority.
Now, I'm going to test the patch.
I also rebased the patches (there was a conflict in the intg tests).
Michal
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@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@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@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?
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 :-)
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@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@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@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
On 09/01/2015 06:23 PM, Michal Židek wrote:
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@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@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@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:
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
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
LGTM, my testing was successful. Let's just wait for CI results.
CI passed: http://sssd-ci.duckdns.org/logs/job/24/24/summary.html
ACK to all patches.
----- Original Message ----- From: "Pavel Reichl" preichl@redhat.com To: sssd-devel@lists.fedorahosted.org Sent: Tuesday, September 1, 2015 6:40:51 PM Subject: Re: [SSSD] [PATCHES] cleanup task: Expire all memberof targets when removing user
[snip] LGTM, my testing was successful. Let's just wait for CI results.
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On Tue, Sep 01, 2015 at 03:21:28PM -0400, Pavel Reichl wrote:
CI passed: http://sssd-ci.duckdns.org/logs/job/24/24/summary.html
ACK to all patches.
* master: * 3b1aa479b377e570c6dff359a1f8099289a2af75 * b0d6d14b5bcc137074383abcd2bf8039c3d74b02 * 4d8f0f92edccff1be52f2b505e76886708d32e26 * 95b2c51771b8b4568e0996061e3819dd36188e22 * 60713f738cedb6e4239604baf6619a0ca986fa49
On (03/09/15 10:00), Jakub Hrozek wrote:
On Tue, Sep 01, 2015 at 03:21:28PM -0400, Pavel Reichl wrote:
CI passed: http://sssd-ci.duckdns.org/logs/job/24/24/summary.html
ACK to all patches.
- master:
- 3b1aa479b377e570c6dff359a1f8099289a2af75
- b0d6d14b5bcc137074383abcd2bf8039c3d74b02
- 4d8f0f92edccff1be52f2b505e76886708d32e26
- 95b2c51771b8b4568e0996061e3819dd36188e22
- 60713f738cedb6e4239604baf6619a0ca986fa49
And pushed also to
sssd-1-12: * b9d6c13529ceefe351020d58ae164cb858053079 * 00f165dc9dc4e5432d7c05c2f9a1d1153cd98c57 * dd3e97aaf616ce6196615001972f2721cde417de
the last two patches could not be backported becuase we do not have integration tests in 1.12
LS
On Mon, Dec 14, 2015 at 12:50:14PM +0100, Lukas Slebodnik wrote:
On (03/09/15 10:00), Jakub Hrozek wrote:
On Tue, Sep 01, 2015 at 03:21:28PM -0400, Pavel Reichl wrote:
CI passed: http://sssd-ci.duckdns.org/logs/job/24/24/summary.html
ACK to all patches.
- master:
- 3b1aa479b377e570c6dff359a1f8099289a2af75
- b0d6d14b5bcc137074383abcd2bf8039c3d74b02
- 4d8f0f92edccff1be52f2b505e76886708d32e26
- 95b2c51771b8b4568e0996061e3819dd36188e22
- 60713f738cedb6e4239604baf6619a0ca986fa49
And pushed also to
sssd-1-12:
- b9d6c13529ceefe351020d58ae164cb858053079
- 00f165dc9dc4e5432d7c05c2f9a1d1153cd98c57
- dd3e97aaf616ce6196615001972f2721cde417de
the last two patches could not be backported becuase we do not have integration tests in 1.12
LS
Thanks for doing this. I wonder if it's time to release 1.12.6 already? This is the oneline log since 1.12.5: b9d6c13 cleanup task: Expire all memberof targets when removing user 00f165d SYSDB: Add function to expire entry dd3e97a Makefile.am: Add missing AM_CFLAGS cd126cd IPA: fix override with the same name fe022b2 IPA: Use search timeout, not enum timeout for searching overrides 6ef8bbe sssd_client: Do not use removed memory cache a956f71 sss_client: Fix underflow of active_threads 2136f71 LDAP: Fix leak of file descriptors 836bc57 BUILD: Accept krb5 1.14 for building the PAC plugin 7816654 SSSDConfigTest: Test real config without config_file_version da097a7 SSSDConfigTest: Try load saved config 2fd0163 SSSDConfig: Do not raise exception if config_file_version is missing e412f49 nss: fix UPN lookups for sub-domain users 4a89c2e GPO: fix memory leak a73c89f SDAP: Relax POSIX check d6073c9 Fix memory leak in sssdpac_verify() f252032 tests: check special characters in cleanup_groups 28dff99 LDAP: Sanitize group dn before using in filter f230eda SPEC: Update spec file for krb5_local_auth_plugin 7cac506 IPA: Remove MPG groups if getgrgid was called before getpw() 676e804 SYSDB: Index the objectSIDString attribute c35eb4a CONFDB: Assume config file version 2 if missing 7dd51b4 SPEC: Workaround for build with rpm 4.13 4b5c6ec BUILD: Repair dependecies on deprecated libraries fa4a7fb sss_client: Update integrity check of records in mmap cache d0d6956 SDAP: Remove user from cache for missing user in LDAP d119324 Updating version for the 1.12.6 release
But I guess the answer also depends on how many users of 1.12.x are there. Do you know which distributions contain 1.12.x ?
On (01/09/15 14:37), Pavel Reichl wrote:
This patch fixes some of the pep8 warnings (number of issues reported by pep8 utility changed from 204 to 127). Should we file a ticket to reduce pep8 warning as much as possible? I suppose we could even add a test that would fail if new pep8 warning is introduced, but I'm little worried that it might prove to be too difficult to write such a test in quality that it will be accepted by all SSSD developers. What do you think?
The quite recent test src/tests/intg/test_memory_cache.py does not have any pep8 warnings or errors. However there is old python code with many warnings which would prevent us from wrinting such test.
But there is still way how to test pep8 warnings in new python code.
sh$ pep8 --help //snip --diff report only lines changed according to the unified diff received on STDIN
If we fix/suppress all pep8 warnings then we can write test.
LS
sssd-devel@lists.fedorahosted.org