[SSSD] [PATCH] Improvements to nested LDAP unit tests

Lukas Slebodnik lslebodn at redhat.com
Tue Jun 24 08:49:34 UTC 2014


On (23/06/14 19:36), Jakub Hrozek wrote:
>Hi,
>
>When I was reviewing PavelB's duplicate members patch, I realized our
>nested group unit tests could use some improvements. Please see the
>attached patches and their commit messages.

>From bc8fd57de30abad8c2b5e74def22927bb2870830 Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Mon, 23 Jun 2014 17:50:45 +0200
>Subject: [PATCH 1/6] TESTS: Add confdb domain base DN to sss_test_ctx
>
>Creation of the path to the domain's confdb entry was duplicated in the
>tests. Rather than adding yet another duplication, I added the path as
>another field of the sss_test_ctx structure.
>---
>From patch, it is clean that it was not used on many places.
It will be usefull in future.

ACK

>From 532cee578213b3d0d9d13229c72adefeb126f8cf Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Mon, 23 Jun 2014 17:52:03 +0200
>Subject: [PATCH 2/6] TESTS: Use the right confdb path
>
>The nested group test only worked by accident. Its confdb settings were
>not applied because a wrong confdb path was used.
>---
ACK

>From 09ca16557b3eb556f38a959d1749160c42a33578 Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Mon, 23 Jun 2014 18:07:51 +0200
>Subject: [PATCH 3/6] TESTS: Fix group search base
>
>After fixing the confdb initialization I realized the group DN couldn't
>be parsed. This patch fixes that.
>---
> src/tests/cmocka/test_nested_groups.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/src/tests/cmocka/test_nested_groups.c b/src/tests/cmocka/test_nested_groups.c
>index 9f38e3dd388dcafcdaa61cb39136c30fa58b399a..bf678c2b2025e54e6ed50b8b33fe41a78ffec007 100644
>--- a/src/tests/cmocka/test_nested_groups.c
>+++ b/src/tests/cmocka/test_nested_groups.c
>@@ -43,7 +43,7 @@
> /* put users and groups under the same container so we can easily run the
>  * same tests cases for several search base scenarios */
> #define OBJECT_BASE_DN "cn=objects,dc=test,dc=com"
>-#define GROUP_BASE_DN "cn=groups" OBJECT_BASE_DN
>+#define GROUP_BASE_DN "cn=groups," OBJECT_BASE_DN
Could you used constant from SYSDB_GROUPS_CONTAINER from sysdb.h?

>From 0cd01ebe4f7f61e830cb06c907b04ff07200dbd1 Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Mon, 23 Jun 2014 19:06:52 +0200
>Subject: [PATCH 4/6] TESTS: Do not require replies from mocked
> sdap_get_generic_recv to be talloc contexts
>
>While it's beneficial for the real implementation of
>sdap_get_generic_recv() to move memory around, the mocked implementation
>can just pass around pointer.
>---
make sense.

ACK

>From 67a524e5ce1b7c48b99104a8726b18a090550ae4 Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Mon, 23 Jun 2014 19:07:41 +0200
>Subject: [PATCH 5/6] TESTS: Change how mock_sysdb_user() is implemented
>
>For the purpose of unit tests, it's better to create a user object with
>a UID and a name.
>---
ACK

>From 3907236536cd4157ab068311b76f0b0babde2681 Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Mon, 23 Jun 2014 19:07:59 +0200
>Subject: [PATCH 6/6] TESTS: Add more tests for nested groups processing
>
>Adds unit test for basic group retrieval functionality as well as for
>testing duplicate members in the LDAP group entry.
Could you add reference (git hash) to patch with PB's fix?

>---
> src/tests/cmocka/test_nested_groups.c | 130 +++++++++++++++++++++++++++++++++-
> 1 file changed, 129 insertions(+), 1 deletion(-)
>
>diff --git a/src/tests/cmocka/test_nested_groups.c b/src/tests/cmocka/test_nested_groups.c
>index bf678c2b2025e54e6ed50b8b33fe41a78ffec007..693d0fb33837643566d7a9ab0dc02bd5cff9c849 100644
>--- a/src/tests/cmocka/test_nested_groups.c
>+++ b/src/tests/cmocka/test_nested_groups.c
>@@ -44,6 +44,7 @@
>  * same tests cases for several search base scenarios */
> #define OBJECT_BASE_DN "cn=objects,dc=test,dc=com"
> #define GROUP_BASE_DN "cn=groups," OBJECT_BASE_DN
>+#define USER_BASE_DN "cn=users," OBJECT_BASE_DN
Could you used constant from SYSDB_USERS_CONTAINER from sysdb.h?

> 
> struct nested_groups_test_ctx {
>     struct sss_test_ctx *tctx;
>@@ -114,11 +115,136 @@ static void nested_groups_test_one_group_no_members(void **state)
>     assert_true(rootgroup == test_ctx->groups[0]);
> }
> 
>+static void nested_groups_test_one_group_unique_members(void **state)
>+{
>+    struct nested_groups_test_ctx *test_ctx = NULL;
>+    struct sysdb_attrs *rootgroup = NULL;
>+    struct tevent_req *req = NULL;
>+    TALLOC_CTX *req_mem_ctx = NULL;
>+    errno_t ret;
>+    const char *name;
>+    const char *users[] = { "cn=user1,"USER_BASE_DN,
>+                            "cn=user2,"USER_BASE_DN,
>+                            NULL };
>+    const struct sysdb_attrs *user1_reply[2] = { NULL };
>+    const struct sysdb_attrs *user2_reply[2] = { NULL };
>+
>+    test_ctx = talloc_get_type_abort(*state, struct nested_groups_test_ctx);
>+
>+    /* mock return values */
>+    rootgroup = mock_sysdb_group_rfc2307bis(test_ctx, GROUP_BASE_DN, 1000,
>+                                            "rootgroup", users);
>+
>+    user1_reply[0] = mock_sysdb_user(test_ctx, USER_BASE_DN, 2001, "user1");
>+    assert_non_null(user1_reply[0]);
>+    will_return(sdap_get_generic_recv, 1);
>+    will_return(sdap_get_generic_recv, user1_reply);
>+
>+    user2_reply[0] = mock_sysdb_user(test_ctx, USER_BASE_DN, 2002, "user2");
>+    assert_non_null(user2_reply[0]);
>+    will_return(sdap_get_generic_recv, 1);
>+    will_return(sdap_get_generic_recv, user2_reply);
>+
>+    sss_will_return_always(sdap_has_deref_support, false);
>+
>+    /* run test, check for memory leaks */
>+    req_mem_ctx = talloc_new(global_talloc_context);
>+    assert_non_null(req_mem_ctx);
>+    check_leaks_push(req_mem_ctx);
>+
>+    req = sdap_nested_group_send(req_mem_ctx, test_ctx->tctx->ev,
>+                                 test_ctx->sdap_domain, test_ctx->sdap_opts,
>+                                 test_ctx->sdap_handle, rootgroup);
>+    assert_non_null(req);
>+    tevent_req_set_callback(req, nested_groups_test_done, test_ctx);
>+
>+    ret = test_ev_loop(test_ctx->tctx);
>+    assert_true(check_leaks_pop(req_mem_ctx) == true);
>+    talloc_zfree(req_mem_ctx);
>+
>+    /* check return code */
>+    assert_int_equal(ret, ERR_OK);
>+
>+    /* Check the users */
>+    assert_int_equal(test_ctx->num_users, 2);
>+    assert_int_equal(test_ctx->num_groups, 1);
>+
>+    ret = sysdb_attrs_get_string(test_ctx->users[0], SYSDB_NAME, &name);
>+    assert_int_equal(ret, EOK);
>+    assert_string_equal(name, "user1");
>+
>+    ret = sysdb_attrs_get_string(test_ctx->users[1], SYSDB_NAME, &name);
>+    assert_int_equal(ret, EOK);
>+    assert_string_equal(name, "user2");
>+}
>+
>+static void nested_groups_test_one_group_dup_users(void **state)
>+{
>+    struct nested_groups_test_ctx *test_ctx = NULL;
>+    struct sysdb_attrs *rootgroup = NULL;
>+    struct tevent_req *req = NULL;
>+    TALLOC_CTX *req_mem_ctx = NULL;
>+    errno_t ret;
>+    const char *name;
>+    const char *users[] = { "cn=user1,"USER_BASE_DN,
>+                            "cn=user1,"USER_BASE_DN,
>+                            NULL };
>+    const struct sysdb_attrs *user1_reply[2] = { NULL };
>+    const struct sysdb_attrs *user2_reply[2] = { NULL };
>+
>+    test_ctx = talloc_get_type_abort(*state, struct nested_groups_test_ctx);
>+
>+    /* mock return values */
>+    rootgroup = mock_sysdb_group_rfc2307bis(test_ctx, GROUP_BASE_DN, 1000,
>+                                            "rootgroup", users);
>+
>+    user1_reply[0] = mock_sysdb_user(test_ctx, USER_BASE_DN, 2001, "user1");
>+    assert_non_null(user1_reply[0]);
>+    will_return(sdap_get_generic_recv, 1);
>+    will_return(sdap_get_generic_recv, user1_reply);
>+
>+    user2_reply[0] = mock_sysdb_user(test_ctx, USER_BASE_DN, 2001, "user1");
>+    assert_non_null(user2_reply[0]);
>+    will_return(sdap_get_generic_recv, 1);
>+    will_return(sdap_get_generic_recv, user2_reply);
>+
>+    sss_will_return_always(sdap_has_deref_support, false);
>+
>+    /* run test, check for memory leaks */
>+    req_mem_ctx = talloc_new(global_talloc_context);
>+    assert_non_null(req_mem_ctx);
>+    check_leaks_push(req_mem_ctx);
>+
>+    req = sdap_nested_group_send(req_mem_ctx, test_ctx->tctx->ev,
>+                                 test_ctx->sdap_domain, test_ctx->sdap_opts,
>+                                 test_ctx->sdap_handle, rootgroup);
>+    assert_non_null(req);
>+    tevent_req_set_callback(req, nested_groups_test_done, test_ctx);
>+
>+    ret = test_ev_loop(test_ctx->tctx);
>+    assert_true(check_leaks_pop(req_mem_ctx) == true);
>+    talloc_zfree(req_mem_ctx);
>+
>+    /* check return code */
>+    assert_int_equal(ret, ERR_OK);
                            ^^^^^^
>+
>+    /* Check the users */
>+    assert_int_equal(test_ctx->num_users, 1);
>+    assert_int_equal(test_ctx->num_groups, 1);
>+
>+    ret = sysdb_attrs_get_string(test_ctx->users[0], SYSDB_NAME, &name);
>+    assert_int_equal(ret, EOK);
                            ^^^
You mixed EOK and ERR_OK. According to git history, ERR_OK is newer.
We should decide and use just one in next patches.



>+    assert_string_equal(name, "user1");
>+}
>+
> void nested_groups_test_setup(void **state)
> {
>     struct nested_groups_test_ctx *test_ctx = NULL;
>     static struct sss_test_conf_param params[] = {
>         { "ldap_schema", "rfc2307bis" }, /* enable nested groups */
>+        { "ldap_search_base", OBJECT_BASE_DN },
>+        { "ldap_user_search_base", USER_BASE_DN },
>+        { "ldap_group_search_base", GROUP_BASE_DN },
>         { NULL, NULL }
>     };
> 
>@@ -163,7 +289,9 @@ int main(int argc, const char *argv[])
>     };
> 
>     const UnitTest tests[] = {
>-        new_test(one_group_no_members)
>+        new_test(one_group_no_members),
>+        new_test(one_group_unique_members),
>+        new_test(one_group_dup_users)
>     };
> 
>     /* Set debug level to invalid value so we can deside if -d 0 was used. */

I tested with reverted aptch "nested groups: do not fail if we get one entry
twice" and unit test fail. It's good.

PB modified function sdap_nested_group_single_step_process on two places
    1) in case SDAP_NESTED_GROUP_DN_USER
    2) in case SDAP_NESTED_GROUP_DN_GROUP

Unit test covers only the first part. Do we want ot also test the 2nd one?

LS



More information about the sssd-devel mailing list