URL: https://github.com/SSSD/sssd/pull/721 Author: jhrozek Title: #721: AD/IPA: Reset subdomain service name, not domain name Action: opened
PR body: """ Related: https://pagure.io/SSSD/sssd/issue/3911 Since commit 778f241e78241b0d6b8734148175f8dee804f494 the subdomain fail over services use the "sd_" prefix. This was done to make it easier, until the whole failover design works better with subdomains, to see which services belong to the main domain from tools. However, some parts of the code would still just use the domain name for the failover service, which meant the service was not found, notably when trying to reset services: (Thu Dec 13 05:29:31 2018) [sssd[be[testrelm.test]]] [ipa_srv_ad_acct_retried] (0x0400): Subdomain re-set, will retry lookup (Thu Dec 13 05:29:31 2018) [sssd[be[testrelm.test]]] [be_fo_reset_svc] (0x1000): Resetting all servers in service ipaad2016.test (Thu Dec 13 05:29:31 2018) [sssd[be[testrelm.test]]] [be_fo_reset_svc] (0x0080): Cannot retrieve service [ipaad2016.test] This patch switches to reading the service names from the ad_options and the sdap_service structures that are contained within ad_options. """ To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/721/head:pr721 git checkout pr721
From f6196ca4d90f17a17c3a275a178ca6659451762d Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Wed, 19 Dec 2018 14:12:25 +0100 Subject: [PATCH 1/2] AD/IPA: Reset subdomain service name, not domain name Related: https://pagure.io/SSSD/sssd/issue/3911 Since commit 778f241e78241b0d6b8734148175f8dee804f494 the subdomain fail over services use the "sd_" prefix. This was done to make it easier, until the whole failover design works better with subdomains, to see which services belong to the main domain from tools. However, some parts of the code would still just use the domain name for the failover service, which meant the service was not found, notably when trying to reset services: (Thu Dec 13 05:29:31 2018) [sssd[be[testrelm.test]]] [ipa_srv_ad_acct_retried] (0x0400): Subdomain re-set, will retry lookup (Thu Dec 13 05:29:31 2018) [sssd[be[testrelm.test]]] [be_fo_reset_svc] (0x1000): Resetting all servers in service ipaad2016.test (Thu Dec 13 05:29:31 2018) [sssd[be[testrelm.test]]] [be_fo_reset_svc] (0x0080): Cannot retrieve service [ipaad2016.test] This patch switches to reading the service names from the ad_options and the sdap_service structures that are contained within ad_options. --- src/providers/ad/ad_common.c | 13 +++++++++++++ src/providers/ad/ad_common.h | 4 ++++ src/providers/ipa/ipa_subdomains_id.c | 11 ++++++++++- src/providers/ldap/ldap_common.c | 11 +++++++++++ src/providers/ldap/ldap_common.h | 3 +++ 5 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c index 0d154ca57..cb5912838 100644 --- a/src/providers/ad/ad_common.c +++ b/src/providers/ad/ad_common.c @@ -839,6 +839,19 @@ ad_failover_init(TALLOC_CTX *mem_ctx, struct be_ctx *bectx, return ret; } +void +ad_failover_reset(struct be_ctx *bectx, + struct ad_service *adsvc) +{ + if (adsvc == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "NULL service\n"); + return; + } + + sdap_service_reset_fo(bectx, adsvc->sdap); + sdap_service_reset_fo(bectx, adsvc->gc); +} + static void ad_resolve_callback(void *private_data, struct fo_server *server) { diff --git a/src/providers/ad/ad_common.h b/src/providers/ad/ad_common.h index cb4dda750..662276cb6 100644 --- a/src/providers/ad/ad_common.h +++ b/src/providers/ad/ad_common.h @@ -148,6 +148,10 @@ ad_failover_init(TALLOC_CTX *mem_ctx, struct be_ctx *ctx, bool use_kdcinfo, struct ad_service **_service); +void +ad_failover_reset(struct be_ctx *bectx, + struct ad_service *adsvc); + errno_t ad_get_id_options(struct ad_options *ad_opts, struct confdb_ctx *cdb, diff --git a/src/providers/ipa/ipa_subdomains_id.c b/src/providers/ipa/ipa_subdomains_id.c index 48cf74460..b841f0a52 100644 --- a/src/providers/ipa/ipa_subdomains_id.c +++ b/src/providers/ipa/ipa_subdomains_id.c @@ -1757,6 +1757,7 @@ static void ipa_srv_ad_acct_lookup_done(struct tevent_req *subreq) static void ipa_srv_ad_acct_retried(struct tevent_req *subreq) { errno_t ret; + struct ad_id_ctx *ad_id_ctx; struct tevent_req *req = tevent_req_callback_data(subreq, struct tevent_req); struct ipa_srv_ad_acct_state *state = tevent_req_data(req, @@ -1772,7 +1773,15 @@ static void ipa_srv_ad_acct_retried(struct tevent_req *subreq) } DEBUG(SSSDBG_TRACE_FUNC, "Subdomain re-set, will retry lookup\n"); - be_fo_reset_svc(state->be_ctx, state->obj_dom->name); + ad_id_ctx = ipa_get_ad_id_ctx(state->ipa_ctx, state->obj_dom); + if (ad_id_ctx == NULL || ad_id_ctx->ad_options == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "No AD ID ctx or no ID CTX options?\n"); + state->dp_error = DP_ERR_FATAL; + tevent_req_error(req, EINVAL); + return; + } + + ad_failover_reset(state->be_ctx, ad_id_ctx->ad_options->service); ret = ipa_srv_ad_acct_lookup_step(req); if (ret != EOK) { diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c index 9cd8ec09c..237749aae 100644 --- a/src/providers/ldap/ldap_common.c +++ b/src/providers/ldap/ldap_common.c @@ -520,6 +520,17 @@ static int ldap_user_data_cmp(void *ud1, void *ud2) return strcasecmp((char*) ud1, (char*) ud2); } +void sdap_service_reset_fo(struct be_ctx *ctx, + struct sdap_service *service) +{ + if (service == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "NULL service\n"); + return; + } + + be_fo_reset_svc(ctx, service->name); +} + int sdap_service_init(TALLOC_CTX *memctx, struct be_ctx *ctx, const char *service_name, const char *dns_service_name, const char *urls, const char *backup_urls, diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h index 6c08d789b..89d819fb9 100644 --- a/src/providers/ldap/ldap_common.h +++ b/src/providers/ldap/ldap_common.h @@ -171,6 +171,9 @@ int sdap_service_init(TALLOC_CTX *memctx, struct be_ctx *ctx, const char *urls, const char *backup_urls, struct sdap_service **_service); +void sdap_service_reset_fo(struct be_ctx *ctx, + struct sdap_service *service); + const char *sdap_gssapi_realm(struct dp_option *opts); int sdap_gssapi_init(TALLOC_CTX *mem_ctx, From 9ecf31ffde9c1e1096d9b3310f05294cc10cfc1b Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Thu, 20 Dec 2018 15:30:03 +0100 Subject: [PATCH 2/2] IPA: Add explicit return after tevent_req_error When working on another patch I realized that we don't use explicit return after failing a request. This could be potentially fatal as the code would continue, perhaps with data that is not defined. --- src/providers/ipa/ipa_subdomains_id.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/providers/ipa/ipa_subdomains_id.c b/src/providers/ipa/ipa_subdomains_id.c index b841f0a52..9958d9dfe 100644 --- a/src/providers/ipa/ipa_subdomains_id.c +++ b/src/providers/ipa/ipa_subdomains_id.c @@ -1770,6 +1770,7 @@ static void ipa_srv_ad_acct_retried(struct tevent_req *subreq) "Failed to re-set subdomain [%d]: %s\n", ret, sss_strerror(ret)); state->dp_error = DP_ERR_FATAL; tevent_req_error(req, ret); + return; } DEBUG(SSSDBG_TRACE_FUNC, "Subdomain re-set, will retry lookup\n"); @@ -1789,6 +1790,7 @@ static void ipa_srv_ad_acct_retried(struct tevent_req *subreq) "Failed to look up AD acct [%d]: %s\n", ret, sss_strerror(ret)); state->dp_error = DP_ERR_FATAL; tevent_req_error(req, ret); + return; } }
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
