From efea50efda58be66638e5d38c8e57fdf9992f204 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Fri, 20 Jul 2012 12:36:43 -0400 Subject: 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. --- src/db/sysdb.h | 3 +- src/db/sysdb_subdomains.c | 13 ++- src/providers/data_provider_be.c | 1 - src/providers/dp_backend.h | 1 - src/providers/ipa/ipa_subdomains.c | 221 ++++++++++++++++++++++++++----------- src/tests/sysdb-tests.c | 36 +++--- 6 files changed, 185 insertions(+), 90 deletions(-) diff --git a/src/db/sysdb.h b/src/db/sysdb.h index 0e2404c70..43ac61c21 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 8f1df88be..8489041bd 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 ba43d7d49..0717bff34 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 6c9b0c0eb..4d079c005 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 a8c05c2a7..b8d072010 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 1eb9503f0..d314baa95 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)); -- cgit