Change refreshing of subdomains

This patch keeps a local copy of the subdomains in the ipa subdomains plugin
context.
This has 2 advantages:
1. allows to check if anything changed w/o always hitting the sysdb.
2. later will allows us to dump this information w/o having to retrieve it
again. The timestamp also allows to avoid refreshing too often.

6 files changed, 185 insertions, 90 deletions

diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index 0e2404c7..43ac61c2 100644
--- a/src/db/sysdb.h
+++ b/src/db/sysdb.h
@@ -371,7 +371,8 @@ errno_t sysdb_get_subdomains(TALLOC_CTX *mem_ctx,
 errno_t sysdb_domain_create(struct sysdb_ctx *sysdb, const char *domain_name);
 
 errno_t sysdb_update_subdomains(struct sysdb_ctx *sysdb,
-                                struct sysdb_subdom **subdomains);
+                                int num_subdoms,
+                                struct sysdb_subdom *subdoms);
 
 errno_t sysdb_get_subdomain_context(TALLOC_CTX *mem_ctx,
                                     struct sysdb_ctx *sysdb,
diff --git a/src/db/sysdb_subdomains.c b/src/db/sysdb_subdomains.c
index 8f1df88b..8489041b 100644
--- a/src/db/sysdb_subdomains.c
+++ b/src/db/sysdb_subdomains.c
@@ -430,7 +430,8 @@ done:
 }
 
 errno_t sysdb_update_subdomains(struct sysdb_ctx *sysdb,
-                                struct sysdb_subdom **subdomains)
+                                int num_subdoms,
+                                struct sysdb_subdom *subdoms)
 {
     int ret;
     int sret;
@@ -475,9 +476,9 @@ errno_t sysdb_update_subdomains(struct sysdb_ctx *sysdb,
      * - if a subdomain already exists in sysdb, mark it for preservation
      * - if the subdomain doesn't exist in sysdb, create its bare structure
      */
-    for (c = 0; subdomains[c] != NULL; c++) {
+    for (c = 0; c < num_subdoms; c++) {
         for (d = 0; d < cur_subdomains_count; d++) {
-            if (strcasecmp(subdomains[c]->name,
+            if (strcasecmp(subdoms[c].name,
                            cur_subdomains[d]->name) == 0) {
                 keep_subdomain[d] = true;
                 /* sub-domain already in cache, nothing to do */
@@ -487,14 +488,14 @@ errno_t sysdb_update_subdomains(struct sysdb_ctx *sysdb,
 
         if (d == cur_subdomains_count) {
             DEBUG(SSSDBG_TRACE_FUNC, ("Adding sub-domain [%s].\n",
-                                      subdomains[c]->name));
-            ret = sysdb_domain_create(sysdb, subdomains[c]->name);
+                                      subdoms[c].name));
+            ret = sysdb_domain_create(sysdb, subdoms[c].name);
             if (ret != EOK) {
                 DEBUG(SSSDBG_OP_FAILURE, ("sysdb_domain_create failed.\n"));
                 goto done;
             }
 
-            ret = sysdb_add_subdomain_attributes(sysdb, subdomains[c]);
+            ret = sysdb_add_subdomain_attributes(sysdb, &subdoms[c]);
             if (ret != EOK) {
                 DEBUG(SSSDBG_OP_FAILURE,
                       ("sysdb_add_subdomain_attributes failed.\n"));
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index ba43d7d4..0717bff3 100644
--- a/src/providers/data_provider_be.c
+++ b/src/providers/data_provider_be.c
@@ -414,7 +414,6 @@ static int be_get_subdomains(DBusMessage *message, struct sbus_connection *conn)
     }
     req->force = force;
     req->domain_hint = talloc_strdup(req, domain_hint);
-    req->domain_list = NULL;
     if (!req->domain_hint) {
         err_maj = DP_ERR_FATAL;
         err_min = ENOMEM;
diff --git a/src/providers/dp_backend.h b/src/providers/dp_backend.h
index 6c9b0c0e..4d079c00 100644
--- a/src/providers/dp_backend.h
+++ b/src/providers/dp_backend.h
@@ -174,7 +174,6 @@ struct be_autofs_req {
 struct be_subdom_req {
     bool force;
     char *domain_hint;
-    struct sysdb_subdom **domain_list;
 };
 
 struct be_host_req {
diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c
index a8c05c2a..b8d07201 100644
--- a/src/providers/ipa/ipa_subdomains.c
+++ b/src/providers/ipa/ipa_subdomains.c
@@ -61,6 +61,11 @@ struct ipa_subdomains_ctx {
     struct sdap_search_base **search_bases;
     struct sdap_search_base **master_search_bases;
     struct sdap_search_base **ranges_search_bases;
+
+    /* subdomain map cache */
+    time_t last_retrieved;
+    int num_subdoms;
+    struct sysdb_subdom *subdoms;
 };
 
 static void ipa_subdomains_reply(struct be_req *be_req, int dp_err, int result)
@@ -175,89 +180,172 @@ static char *name_to_realm(TALLOC_CTX *memctx, const char *name)
     return realm;
 }
 
-static errno_t ipa_subdomains_parse_results(struct be_subdom_req *sd_data,
-                                            size_t count,
-                                            struct sysdb_attrs **reply)
+static errno_t ipa_subdom_parse(TALLOC_CTX *memctx,
+                                struct sysdb_attrs *attrs,
+                                struct sysdb_subdom *subdom)
 {
-    struct sysdb_subdom **new_domain_list = NULL;
     const char *value;
-    size_t c;
     int ret;
 
-    new_domain_list = talloc_array(sd_data, struct sysdb_subdom *, count + 1);
-    if (new_domain_list == NULL) {
-        DEBUG(SSSDBG_OP_FAILURE, ("talloc_array failed.\n"));
-        return ENOMEM;
+    ret = sysdb_attrs_get_string(attrs, IPA_CN, &value);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_get_string failed.\n"));
+        return ret;
+    }
+    if (subdom->name == NULL) {
+        subdom->name = talloc_strdup(memctx, value);
+        if (subdom->name == NULL) {
+            return ENOMEM;
+        }
+    } else if (strcmp(subdom->name, value) != 0) {
+        DEBUG(SSSDBG_OP_FAILURE, ("subdomain name mismatch!\n"));
+        return EINVAL;
     }
 
-    for (c = 0; c < count; c++) {
-        new_domain_list[c] = talloc_zero(new_domain_list,
-                                         struct sysdb_subdom);
-        if (new_domain_list[c] == NULL) {
-            DEBUG(SSSDBG_OP_FAILURE, ("talloc_zero failed.\n"));
-            ret = ENOMEM;
-            goto done;
+    if (subdom->realm == NULL) {
+        /* Add Realm as upper(domain name), this is generally always correct
+         * with AD domains */
+        subdom->realm = name_to_realm(memctx, subdom->name);
+        if (!subdom->realm) {
+            return ENOMEM;
         }
+    }
 
-        ret = sysdb_attrs_get_string(reply[c], IPA_CN, &value);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_get_string failed.\n"));
-            goto done;
+    ret = sysdb_attrs_get_string(attrs, IPA_FLATNAME, &value);
+    if (ret) {
+        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_get_string failed.\n"));
+        return ret;
+    }
+
+    /* in theory this may change, it should never happen, so we will log a
+     * warning if it does, but we will allow it for now */
+    if (subdom->flat_name != NULL) {
+        if (strcmp(subdom->flat_name, value) != 0) {
+            DEBUG(SSSDBG_TRACE_INTERNAL,
+                  ("Flat name for subdomain changed!\n"));
+            talloc_free(discard_const(subdom->flat_name));
+            subdom->flat_name = (const char *)NULL;
         }
-        new_domain_list[c]->name = talloc_strdup(new_domain_list[c], value);
-        if (new_domain_list[c]->name == NULL) {
-            DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed.\n"));
-            ret = ENOMEM;
-            goto done;
+    }
+    if (subdom->flat_name == NULL) {
+        subdom->flat_name = talloc_strdup(memctx, value);
+        if (subdom->flat_name == NULL) {
+            return ENOMEM;
         }
+    }
 
-        /* Add Realm as upper(domain name), this is generally always correct
-         * with AD domains */
-        new_domain_list[c]->realm = name_to_realm(new_domain_list[c],
-                                                  new_domain_list[c]->name);
-        if (!new_domain_list[c]->realm) {
-            DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed.\n"));
-            ret = ENOMEM;
-            goto done;
+    ret = sysdb_attrs_get_string(attrs, IPA_TRUSTED_DOMAIN_SID, &value);
+    if (ret) {
+        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_get_string failed.\n"));
+        return ret;
+    }
+
+    /* in theory this may change, it should never happen, so we will log a
+     * warning if it does, but we will allow it for now */
+    if (subdom->id != NULL) {
+        if (strcmp(subdom->id, value) != 0) {
+            DEBUG(SSSDBG_TRACE_INTERNAL,
+                  ("ID for subdomain changed!\n"));
+            talloc_free(discard_const(subdom->id));
+            subdom->flat_name = (const char *)NULL;
         }
+    }
+    if (subdom->id == NULL) {
+        subdom->id = talloc_strdup(memctx, value);
+        if (subdom->id == NULL) {
+            return ENOMEM;
+        }
+    }
 
-        ret = sysdb_attrs_get_string(reply[c], IPA_FLATNAME, &value);
-        if (ret == EOK) {
-            new_domain_list[c]->flat_name = talloc_strdup(new_domain_list[c],
-                                                          value);
-            if (new_domain_list[c]->flat_name == NULL) {
-                DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed.\n"));
-                ret = ENOMEM;
+    return EOK;
+}
+
+static errno_t ipa_subdomains_refresh(struct ipa_subdomains_ctx *ctx,
+                                      int count, struct sysdb_attrs **reply,
+                                      bool *changes)
+{
+    bool handled[count];
+    const char *value;
+    int c, h;
+    int ret;
+    int i, j;
+
+    memset(handled, 0, sizeof(bool) * count);
+
+    /* check existing subdoms in cache */
+    for (i = 0, h = 0; i < ctx->num_subdoms; i++) {
+        for (c = 0; c < count; c++) {
+            if (handled[c]) {
+                continue;
+            }
+            ret = sysdb_attrs_get_string(reply[c], IPA_CN, &value);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_get_string failed.\n"));
                 goto done;
             }
-        } else if (ret != ENOENT) {
-            DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_get_string failed.\n"));
-            goto done;
+            if (strcmp(value, ctx->subdoms[i].name) == 0) {
+                break;
+            }
         }
 
-        ret = sysdb_attrs_get_string(reply[c], IPA_TRUSTED_DOMAIN_SID, &value);
-        if (ret == EOK) {
-            new_domain_list[c]->id = talloc_strdup(new_domain_list[c], value);
-            if (new_domain_list[c]->id == NULL) {
-                DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed.\n"));
-                ret = ENOMEM;
+        if (c >= count) {
+            /* ok this subdomain does not exist anymore, let's clean up */
+            for (j = i; j < ctx->num_subdoms - 1; j++) {
+                ctx->subdoms[j] = ctx->subdoms[j + 1];
+            }
+            ctx->num_subdoms--;
+            i--;
+        } else {
+            /* ok let's try to update it */
+            ret = ipa_subdom_parse(ctx->subdoms, reply[c], &ctx->subdoms[i]);
+            if (ret) {
+                DEBUG(SSSDBG_OP_FAILURE, ("Failed to parse subdom data\n"));
                 goto done;
             }
-        } else if (ret != ENOENT) {
-            DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_get_string failed.\n"));
+            handled[c] = true;
+            h++;
+        }
+    }
+
+    if (count == h) {
+        /* all domains were already accounted for and have been updated */
+        ret = EOK;
+        goto done;
+    }
+
+    /* if we get here it means we have changes to the subdomains list */
+    *changes = true;
+
+    /* add space for unhandled domains */
+    c = count - h;
+    ctx->subdoms = talloc_realloc(ctx, ctx->subdoms,
+                                  struct sysdb_subdom,
+                                  ctx->num_subdoms + c);
+    if (ctx->subdoms == NULL) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    for (c = 0; c < count; c++) {
+        if (handled[c]) {
+            continue;
+        }
+        i = ctx->num_subdoms;
+        memset(&ctx->subdoms[i], 0, sizeof(struct sysdb_subdom));
+        ret = ipa_subdom_parse(ctx->subdoms, reply[c], &ctx->subdoms[i]);
+        if (ret) {
+            DEBUG(SSSDBG_OP_FAILURE, ("Failed to parse subdom data\n"));
             goto done;
         }
+        ctx->num_subdoms++;
     }
-    new_domain_list[c] = NULL;
 
     ret = EOK;
 
 done:
-    if (ret == EOK) {
-        talloc_free(sd_data->domain_list);
-        sd_data->domain_list = new_domain_list;
-    } else {
-        talloc_free(new_domain_list);
+    if (ret != EOK) {
+        ctx->num_subdoms = 0;
+        talloc_zfree(ctx->subdoms);
     }
 
     return ret;
@@ -444,6 +532,7 @@ static void ipa_subdomains_handler_done(struct tevent_req *req)
     struct ipa_subdomains_req_ctx *ctx;
     struct be_req *be_req;
     struct sysdb_ctx *sysdb;
+    bool refresh_has_changes = false;
 
     ctx = tevent_req_callback_data(req, struct ipa_subdomains_req_ctx);
     be_req = ctx->be_req;
@@ -480,18 +569,20 @@ static void ipa_subdomains_handler_done(struct tevent_req *req)
         goto done;
     }
 
-    ret = ipa_subdomains_parse_results(ctx->sd_data, ctx->reply_count,
-                                       ctx->reply);
+    ret = ipa_subdomains_refresh(ctx->sd_ctx, ctx->reply_count, ctx->reply,
+                                 &refresh_has_changes);
     if (ret != EOK) {
-        DEBUG(SSSDBG_OP_FAILURE,
-              ("ipa_subdomains_parse_results request failed.\n"));
+        DEBUG(SSSDBG_OP_FAILURE, ("Failed to refresh subdomains.\n"));
         goto done;
     }
 
-    ret = sysdb_update_subdomains(sysdb, ctx->sd_data->domain_list);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_update_subdomains failed.\n"));
-        goto done;
+    if (refresh_has_changes) {
+        ret = sysdb_update_subdomains(sysdb, ctx->sd_ctx->num_subdoms,
+                                      ctx->sd_ctx->subdoms);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_OP_FAILURE, ("sysdb_update_subdomains failed.\n"));
+            goto done;
+        }
     }
 
 
diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c
index 1eb9503f..d314baa9 100644
--- a/src/tests/sysdb-tests.c
+++ b/src/tests/sysdb-tests.c
@@ -3398,9 +3398,10 @@ START_TEST(test_sysdb_subdomain_create)
     errno_t ret;
     struct sysdb_subdom **cur_subdomains = NULL;
     size_t cur_subdomains_count;
-    struct sysdb_subdom *new_subdom1[] = { &dom1, NULL};
-    struct sysdb_subdom *new_subdom2[] = { &dom2, NULL};
-    struct sysdb_subdom *empty[] = { NULL};
+    struct sysdb_subdom *new_subdom1 = &dom1;
+    struct sysdb_subdom *new_subdom2 = &dom2;
+    int num_subdom1 = 1;
+    int num_subdom2 = 1;
 
     ret = setup_sysdb_tests(&test_ctx);
     fail_if(ret != EOK, "Could not set up the test");
@@ -3412,7 +3413,7 @@ START_TEST(test_sysdb_subdomain_create)
     fail_unless(cur_subdomains != NULL, "No sub-domains returned.");
     fail_unless(cur_subdomains[0] == NULL, "No empyt sub-domain list returned.");
 
-    ret = sysdb_update_subdomains(test_ctx->sysdb, new_subdom1);
+    ret = sysdb_update_subdomains(test_ctx->sysdb, num_subdom1, new_subdom1);
     fail_unless(ret == EOK, "sysdb_update_subdomains failed with [%d][%s]",
                             ret, strerror(ret));
 
@@ -3422,11 +3423,11 @@ START_TEST(test_sysdb_subdomain_create)
                             ret, strerror(ret));
     fail_unless(cur_subdomains != NULL, "No sub-domains returned.");
     fail_unless(cur_subdomains[0] != NULL, "Empyt sub-domain list returned.");
-    fail_unless(strcmp(cur_subdomains[0]->name, new_subdom1[0]->name) == 0,
+    fail_unless(strcmp(cur_subdomains[0]->name, new_subdom1[0].name) == 0,
                 "Unexpected sub-domain found, expected [%s], got [%s]",
-                new_subdom1[0]->name, cur_subdomains[0]->name);
+                new_subdom1[0].name, cur_subdomains[0]->name);
 
-    ret = sysdb_update_subdomains(test_ctx->sysdb, new_subdom2);
+    ret = sysdb_update_subdomains(test_ctx->sysdb, num_subdom2, new_subdom2);
     fail_unless(ret == EOK, "sysdb_update_subdomains failed with [%d][%s]",
                             ret, strerror(ret));
 
@@ -3436,11 +3437,11 @@ START_TEST(test_sysdb_subdomain_create)
                             ret, strerror(ret));
     fail_unless(cur_subdomains != NULL, "No sub-domains returned.");
     fail_unless(cur_subdomains[0] != NULL, "Empyt sub-domain list returned.");
-    fail_unless(strcmp(cur_subdomains[0]->name, new_subdom2[0]->name) == 0,
+    fail_unless(strcmp(cur_subdomains[0]->name, new_subdom2[0].name) == 0,
                 "Unexpected sub-domain found, expected [%s], got [%s]",
-                new_subdom2[0]->name, cur_subdomains[0]->name);
+                new_subdom2[0].name, cur_subdomains[0]->name);
 
-    ret = sysdb_update_subdomains(test_ctx->sysdb, empty);
+    ret = sysdb_update_subdomains(test_ctx->sysdb, 0, NULL);
     fail_unless(ret == EOK, "sysdb_update_subdomains failed with [%d][%s]",
                             ret, strerror(ret));
 
@@ -3457,7 +3458,8 @@ START_TEST(test_sysdb_subdomain_store_user)
 {
     struct sysdb_test_ctx *test_ctx;
     errno_t ret;
-    struct sysdb_subdom *test_subdom[] = { &dom_t, NULL};
+    struct sysdb_subdom *test_subdom = &dom_t;
+    int num_subdom = 1;
     struct sss_domain_info *subdomain = NULL;
     struct ldb_result *results = NULL;
     struct ldb_dn *base_dn = NULL;
@@ -3466,7 +3468,7 @@ START_TEST(test_sysdb_subdomain_store_user)
     ret = setup_sysdb_tests(&test_ctx);
     fail_if(ret != EOK, "Could not set up the test");
 
-    ret = sysdb_update_subdomains(test_ctx->sysdb, test_subdom);
+    ret = sysdb_update_subdomains(test_ctx->sysdb, num_subdom, test_subdom);
     fail_unless(ret == EOK, "sysdb_update_subdomains failed with [%d][%s]",
                             ret, strerror(ret));
 
@@ -3512,7 +3514,8 @@ START_TEST(test_sysdb_subdomain_user_ops)
 {
     struct sysdb_test_ctx *test_ctx;
     errno_t ret;
-    struct sysdb_subdom *test_subdom[] = { &dom_t, NULL};
+    struct sysdb_subdom *test_subdom = &dom_t;
+    int num_subdom = 1;
     struct sss_domain_info *subdomain = NULL;
     struct ldb_message *msg = NULL;
     struct ldb_dn *check_dn = NULL;
@@ -3520,7 +3523,7 @@ START_TEST(test_sysdb_subdomain_user_ops)
     ret = setup_sysdb_tests(&test_ctx);
     fail_if(ret != EOK, "Could not set up the test");
 
-    ret = sysdb_update_subdomains(test_ctx->sysdb, test_subdom);
+    ret = sysdb_update_subdomains(test_ctx->sysdb, num_subdom, test_subdom);
     fail_unless(ret == EOK, "sysdb_update_subdomains failed with [%d][%s]",
                             ret, strerror(ret));
 
@@ -3561,7 +3564,8 @@ START_TEST(test_sysdb_subdomain_group_ops)
 {
     struct sysdb_test_ctx *test_ctx;
     errno_t ret;
-    struct sysdb_subdom *test_subdom[] = { &dom_t, NULL};
+    struct sysdb_subdom *test_subdom = &dom_t;
+    int num_subdom = 1;
     struct sss_domain_info *subdomain = NULL;
     struct ldb_message *msg = NULL;
     struct ldb_dn *check_dn = NULL;
@@ -3569,7 +3573,7 @@ START_TEST(test_sysdb_subdomain_group_ops)
     ret = setup_sysdb_tests(&test_ctx);
     fail_if(ret != EOK, "Could not set up the test");
 
-    ret = sysdb_update_subdomains(test_ctx->sysdb, test_subdom);
+    ret = sysdb_update_subdomains(test_ctx->sysdb, num_subdom, test_subdom);
     fail_unless(ret == EOK, "sysdb_update_subdomains failed with [%d][%s]",
                             ret, strerror(ret));