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

Jakub Hrozek jhrozek at redhat.com
Mon Jun 23 17:36:26 UTC 2014


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.
-------------- next part --------------
>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.
---
 src/tests/cmocka/test_dp_opts.c | 6 +-----
 src/tests/cmocka/test_dyndns.c  | 5 +----
 src/tests/common.h              | 1 +
 src/tests/common_dom.c          | 8 ++++++++
 4 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/tests/cmocka/test_dp_opts.c b/src/tests/cmocka/test_dp_opts.c
index f3029940d48e8fe72b25dfb66e3cf4a589a0960f..31dde47c75b1fada1dd03f18c59c34066c5f79a9 100644
--- a/src/tests/cmocka/test_dp_opts.c
+++ b/src/tests/cmocka/test_dp_opts.c
@@ -210,7 +210,6 @@ void opt_test_get(void **state)
     int ret;
     struct sss_test_ctx *tctx;
     struct dp_option *opts;
-    char *dompath;
     struct sss_test_conf_param params[] = {
         { "string_nodefault", "stringval2" },
         { "blob_nodefault", "blobval2" },
@@ -227,10 +226,7 @@ void opt_test_get(void **state)
                                TEST_DOM_NAME, TEST_ID_PROVIDER, params);
     assert_non_null(tctx);
 
-    dompath = talloc_asprintf(tctx, "config/domain/%s", TEST_DOM_NAME);
-    assert_non_null(dompath);
-
-    ret = dp_get_options(global_talloc_context, tctx->confdb, dompath,
+    ret = dp_get_options(global_talloc_context, tctx->confdb, tctx->conf_dom_path,
                          test_def_opts, OPT_NUM_OPTS, &opts);
     assert_int_equal(ret, EOK);
 
diff --git a/src/tests/cmocka/test_dyndns.c b/src/tests/cmocka/test_dyndns.c
index dcf6eede398ee5b3a71d5f81c932c856bc307929..db90e661cba032dc230478dbb10cae1109c37f05 100644
--- a/src/tests/cmocka/test_dyndns.c
+++ b/src/tests/cmocka/test_dyndns.c
@@ -369,10 +369,7 @@ void dyndns_test_setup(void **state)
 
     dyndns_test_ctx->be_ctx->cdb = dyndns_test_ctx->tctx->confdb;
     dyndns_test_ctx->be_ctx->ev  = dyndns_test_ctx->tctx->ev;
-    dyndns_test_ctx->be_ctx->conf_path = talloc_asprintf(dyndns_test_ctx,
-                                                         CONFDB_DOMAIN_PATH_TMPL,
-                                                         TEST_DOM_NAME);
-    assert_non_null(dyndns_test_ctx->be_ctx->conf_path);
+    dyndns_test_ctx->be_ctx->conf_path = dyndns_test_ctx->tctx->conf_dom_path;
 }
 
 void dyndns_test_teardown(void **state)
diff --git a/src/tests/common.h b/src/tests/common.h
index 2e71b2295cfc5ba434eaca701e351de78d8533a7..9065db39ea5d0db0351447cfb2c9c14f4d9468e5 100644
--- a/src/tests/common.h
+++ b/src/tests/common.h
@@ -64,6 +64,7 @@ struct sss_test_ctx {
     struct sss_domain_info *dom;
     struct sss_names_ctx *nctx;
     char *confdb_path;
+    char *conf_dom_path;
 
     bool done;
     int error;
diff --git a/src/tests/common_dom.c b/src/tests/common_dom.c
index d3a7781c330d73370c0e6dbc629e20d546a841c3..b96db8d9084fea9ec07be88d9c5f72c6885d8f09 100644
--- a/src/tests/common_dom.c
+++ b/src/tests/common_dom.c
@@ -53,6 +53,14 @@ create_dom_test_ctx(TALLOC_CTX *mem_ctx,
         goto fail;
     }
 
+    test_ctx->conf_dom_path = talloc_asprintf(test_ctx,
+                                              CONFDB_DOMAIN_PATH_TMPL,
+                                              domain_name);
+    if (test_ctx->conf_dom_path == NULL) {
+        DEBUG(SSSDBG_CRIT_FAILURE, "talloc_asprintf failed\n");
+        goto fail;
+    }
+
     /* Connect to the conf db */
     ret = confdb_init(test_ctx, &test_ctx->confdb, test_ctx->confdb_path);
     if (ret != EOK) {
-- 
1.9.3

-------------- next part --------------
>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.
---
 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 ed9a8c8ec8bef543f2a44d23451584de8fc8f2a5..9f38e3dd388dcafcdaa61cb39136c30fa58b399a 100644
--- a/src/tests/cmocka/test_nested_groups.c
+++ b/src/tests/cmocka/test_nested_groups.c
@@ -136,7 +136,7 @@ void nested_groups_test_setup(void **state)
     test_ctx->sdap_opts = mock_sdap_options_ldap(test_ctx,
                                                  test_ctx->tctx->dom,
                                                  test_ctx->tctx->confdb,
-                                                 test_ctx->tctx->confdb_path);
+                                                 test_ctx->tctx->conf_dom_path);
     assert_non_null(test_ctx->sdap_opts);
     test_ctx->sdap_domain = test_ctx->sdap_opts->sdom;
     test_ctx->sdap_handle = mock_sdap_handle(test_ctx);
-- 
1.9.3

-------------- next part --------------
>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
 
 struct nested_groups_test_ctx {
     struct sss_test_ctx *tctx;
-- 
1.9.3

-------------- next part --------------
>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.
---
 src/tests/cmocka/common_mock_sdap.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/tests/cmocka/common_mock_sdap.c b/src/tests/cmocka/common_mock_sdap.c
index 52dcc0fad243fe84c453271c1e904384fc1e13bb..b2b57cbd23c9a9882cca38e63ea0b021546880e5 100644
--- a/src/tests/cmocka/common_mock_sdap.c
+++ b/src/tests/cmocka/common_mock_sdap.c
@@ -98,8 +98,7 @@ int sdap_get_generic_recv(struct tevent_req *req,
     TEVENT_REQ_RETURN_ON_ERROR(req);
 
     *reply_count = sss_mock_type(size_t);
-    *reply = talloc_steal(mem_ctx,
-                          sss_mock_ptr_type(struct sysdb_attrs **));
+    *reply = sss_mock_ptr_type(struct sysdb_attrs **);
 
     return EOK;
 }
-- 
1.9.3

-------------- next part --------------
>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.
---
 src/tests/cmocka/common_mock_sysdb_objects.c | 10 ++++++++++
 src/tests/cmocka/common_mock_sysdb_objects.h |  9 ++++++---
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/tests/cmocka/common_mock_sysdb_objects.c b/src/tests/cmocka/common_mock_sysdb_objects.c
index ef050de200c577ddd80d988b572833613147016e..5dc9e4e7810264611bd63ad9393db7bfc44563df 100644
--- a/src/tests/cmocka/common_mock_sysdb_objects.c
+++ b/src/tests/cmocka/common_mock_sysdb_objects.c
@@ -191,3 +191,13 @@ mock_sysdb_group_rfc2307bis(TALLOC_CTX *mem_ctx,
 
     return attrs;
 }
+
+struct sysdb_attrs *
+mock_sysdb_user(TALLOC_CTX *mem_ctx,
+                const char *base_dn,
+                uid_t uid,
+                const char *name)
+{
+    return mock_sysdb_object(mem_ctx, base_dn, name,
+                             SYSDB_UIDNUM, uid);
+}
diff --git a/src/tests/cmocka/common_mock_sysdb_objects.h b/src/tests/cmocka/common_mock_sysdb_objects.h
index 3ae8405b3212bdfeaab12c8a69407554db26e992..2d00a3f7704471172c0c004c8663cfdd518d07a6 100644
--- a/src/tests/cmocka/common_mock_sysdb_objects.h
+++ b/src/tests/cmocka/common_mock_sysdb_objects.h
@@ -35,9 +35,6 @@ _mock_sysdb_object(TALLOC_CTX *mem_ctx,
 #define mock_sysdb_object(mem_ctx, base_dn, name, ...) \
     _mock_sysdb_object(mem_ctx, base_dn, name, ##__VA_ARGS__, NULL)
 
-#define mock_sysdb_user(mem_ctx, base_dn, name, ...) \
-    mock_sysdb_object(mem_ctx, base_dn, name, ##__VA_ARGS__)
-
 struct sysdb_attrs *
 mock_sysdb_group_rfc2307bis(TALLOC_CTX *mem_ctx,
                             const char *base_dn,
@@ -45,4 +42,10 @@ mock_sysdb_group_rfc2307bis(TALLOC_CTX *mem_ctx,
                             const char *name,
                             const char **members);
 
+struct sysdb_attrs *
+mock_sysdb_user(TALLOC_CTX *mem_ctx,
+                const char *base_dn,
+                uid_t uid,
+                const char *name);
+
 #endif /* COMMON_MOCK_SYSDB_OBJECTS_H_ */
-- 
1.9.3

-------------- next part --------------
>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.
---
 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
 
 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);
+    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. */
-- 
1.9.3



More information about the sssd-devel mailing list