Duplicate detection in fail over did not work.

https://fedorahosted.org/sssd/ticket/1472

9 files changed, 69 insertions, 15 deletions

diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c
index 800ef131..90cfe412 100644
--- a/src/providers/ad/ad_common.c
+++ b/src/providers/ad/ad_common.c
@@ -207,6 +207,11 @@ done:
     return ret;
 }
 
+static int ad_user_data_cmp(void *ud1, void *ud2)
+{
+    return strcasecmp((char*) ud1, (char*) ud2);
+}
+
 errno_t
 ad_failover_init(TALLOC_CTX *mem_ctx, struct be_ctx *bectx,
                  const char *primary_servers,
@@ -240,7 +245,7 @@ ad_failover_init(TALLOC_CTX *mem_ctx, struct be_ctx *bectx,
         goto done;
     }
 
-    ret = be_fo_add_service(bectx, AD_SERVICE_NAME);
+    ret = be_fo_add_service(bectx, AD_SERVICE_NAME, ad_user_data_cmp);
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to create failover service!\n"));
         goto done;
diff --git a/src/providers/data_provider_fo.c b/src/providers/data_provider_fo.c
index 1c03e31c..54c0841f 100644
--- a/src/providers/data_provider_fo.c
+++ b/src/providers/data_provider_fo.c
@@ -168,7 +168,8 @@ static struct be_svc_data *be_fo_find_svc_data(struct be_ctx *ctx,
     return 0;
 }
 
-int be_fo_add_service(struct be_ctx *ctx, const char *service_name)
+int be_fo_add_service(struct be_ctx *ctx, const char *service_name,
+                      datacmp_fn user_data_cmp)
 {
     struct fo_service *service;
     struct be_svc_data *svc;
@@ -185,7 +186,8 @@ int be_fo_add_service(struct be_ctx *ctx, const char *service_name)
 
     /* if not in the be service list, try to create new one */
 
-    ret = fo_new_service(ctx->be_fo->fo_ctx, service_name, &service);
+    ret = fo_new_service(ctx->be_fo->fo_ctx, service_name, user_data_cmp,
+                         &service);
     if (ret != EOK && ret != EEXIST) {
         DEBUG(1, ("Failed to create failover service!\n"));
         return ret;
diff --git a/src/providers/dp_backend.h b/src/providers/dp_backend.h
index 4d079c00..b98d1507 100644
--- a/src/providers/dp_backend.h
+++ b/src/providers/dp_backend.h
@@ -218,7 +218,8 @@ typedef void (be_svc_callback_fn_t)(void *, struct fo_server *);
 
 int be_init_failover(struct be_ctx *ctx);
 int be_fo_is_srv_identifier(const char *server);
-int be_fo_add_service(struct be_ctx *ctx, const char *service_name);
+int be_fo_add_service(struct be_ctx *ctx, const char *service_name,
+                      datacmp_fn user_data_cmp);
 int be_fo_service_add_callback(TALLOC_CTX *memctx,
                                struct be_ctx *ctx, const char *service_name,
                                be_svc_callback_fn_t *fn, void *private_data);
diff --git a/src/providers/fail_over.c b/src/providers/fail_over.c
index facc9904..eb9c1050 100644
--- a/src/providers/fail_over.c
+++ b/src/providers/fail_over.c
@@ -66,6 +66,12 @@ struct fo_service {
     struct fo_server *active_server;
     struct fo_server *last_tried_server;
     struct fo_server *server_list;
+
+    /* Function pointed by user_data_cmp returns 0 if user_data is equal
+     * or nonzero value if not. Set to NULL if no user data comparison
+     * is needed in fail over duplicate servers detection.
+     */
+    datacmp_fn user_data_cmp;
 };
 
 struct fo_server {
@@ -393,6 +399,7 @@ service_destructor(struct fo_service *service)
 
 int
 fo_new_service(struct fo_ctx *ctx, const char *name,
+               datacmp_fn user_data_cmp,
                struct fo_service **_service)
 {
     struct fo_service *service;
@@ -420,6 +427,8 @@ fo_new_service(struct fo_ctx *ctx, const char *name,
         return ENOMEM;
     }
 
+    service->user_data_cmp = user_data_cmp;
+
     service->ctx = ctx;
     DLIST_ADD(ctx->service_list, service);
 
@@ -520,8 +529,14 @@ fo_add_srv_server(struct fo_service *service, const char *srv,
                               service->name, proto));
 
     DLIST_FOR_EACH(server, service->server_list) {
-        if (server->user_data != user_data)
-            continue;
+        /* Compare user data only if user_data_cmp and both arguments
+         * are not NULL.
+         */
+        if (server->service->user_data_cmp && user_data && server->user_data) {
+            if (server->service->user_data_cmp(server->user_data, user_data)) {
+                continue;
+            }
+        }
 
         if (fo_is_srv_lookup(server)) {
             if (((discovery_domain == NULL &&
@@ -629,10 +644,19 @@ static bool fo_server_match(struct fo_server *server,
                            int port,
                            void *user_data)
 {
-    if (server->port != port || server->user_data != user_data) {
+    if (server->port != port) {
         return false;
     }
 
+    /* Compare user data only if user_data_cmp and both arguments
+     * are not NULL.
+     */
+    if (server->service->user_data_cmp && server->user_data && user_data) {
+        if (server->service->user_data_cmp(server->user_data, user_data)) {
+            return false;
+        }
+    }
+
     if (name == NULL && server->common == NULL) {
         return true;
     }
diff --git a/src/providers/fail_over.h b/src/providers/fail_over.h
index b69e8a53..36437ed5 100644
--- a/src/providers/fail_over.h
+++ b/src/providers/fail_over.h
@@ -89,13 +89,20 @@ struct fo_options {
 struct fo_ctx *fo_context_init(TALLOC_CTX *mem_ctx,
                                struct fo_options *opts);
 
+typedef int (*datacmp_fn)(void*, void*);
+
 /*
  * Create a new service structure for 'ctx', saving it to the location pointed
  * to by '_service'. The needed memory will be allocated from 'ctx'.
  * Service name will be set to 'name'.
+ *
+ * Function pointed by user_data_cmp returns 0 if user_data is equal
+ * or nonzero value if not. Set to NULL if no user data comparison
+ * is needed in fail over duplicate servers detection.
  */
 int fo_new_service(struct fo_ctx *ctx,
                    const char *name,
+                   datacmp_fn user_data_cmp,
                    struct fo_service **_service);
 
 /*
diff --git a/src/providers/ipa/ipa_common.c b/src/providers/ipa/ipa_common.c
index bf62fcb9..6ad67845 100644
--- a/src/providers/ipa/ipa_common.c
+++ b/src/providers/ipa/ipa_common.c
@@ -861,6 +861,11 @@ done:
     return ret;
 }
 
+static int ipa_user_data_cmp(void *ud1, void *ud2)
+{
+    return strcasecmp((char*) ud1, (char*) ud2);
+}
+
 int ipa_service_init(TALLOC_CTX *memctx, struct be_ctx *ctx,
                      const char *primary_servers,
                      const char *backup_servers,
@@ -893,7 +898,7 @@ int ipa_service_init(TALLOC_CTX *memctx, struct be_ctx *ctx,
         goto done;
     }
 
-    ret = be_fo_add_service(ctx, "IPA");
+    ret = be_fo_add_service(ctx, "IPA", ipa_user_data_cmp);
     if (ret != EOK) {
         DEBUG(1, ("Failed to create failover service!\n"));
         goto done;
diff --git a/src/providers/krb5/krb5_common.c b/src/providers/krb5/krb5_common.c
index 98a2f7da..bd7a3021 100644
--- a/src/providers/krb5/krb5_common.c
+++ b/src/providers/krb5/krb5_common.c
@@ -579,6 +579,11 @@ done:
     return ret;
 }
 
+static int krb5_user_data_cmp(void *ud1, void *ud2)
+{
+    return strcasecmp((char*) ud1, (char*) ud2);
+}
+
 int krb5_service_init(TALLOC_CTX *memctx, struct be_ctx *ctx,
                       const char *service_name,
                       const char *primary_servers,
@@ -600,7 +605,7 @@ int krb5_service_init(TALLOC_CTX *memctx, struct be_ctx *ctx,
         goto done;
     }
 
-    ret = be_fo_add_service(ctx, service_name);
+    ret = be_fo_add_service(ctx, service_name, krb5_user_data_cmp);
     if (ret != EOK) {
         DEBUG(1, ("Failed to create failover service!\n"));
         goto done;
diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c
index 570ec970..ce758755 100644
--- a/src/providers/ldap/ldap_common.c
+++ b/src/providers/ldap/ldap_common.c
@@ -1198,6 +1198,11 @@ done:
     return ret;
 }
 
+static int ldap_user_data_cmp(void *ud1, void *ud2)
+{
+    return strcasecmp((char*) ud1, (char*) ud2);
+}
+
 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,
@@ -1218,7 +1223,7 @@ int sdap_service_init(TALLOC_CTX *memctx, struct be_ctx *ctx,
         goto done;
     }
 
-    ret = be_fo_add_service(ctx, service_name);
+    ret = be_fo_add_service(ctx, service_name, ldap_user_data_cmp);
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to create failover service!\n"));
         goto done;
diff --git a/src/tests/fail_over-tests.c b/src/tests/fail_over-tests.c
index 6f4843ca..3a7c4ae4 100644
--- a/src/tests/fail_over-tests.c
+++ b/src/tests/fail_over-tests.c
@@ -114,11 +114,11 @@ START_TEST(test_fo_new_service)
         sprintf(buf, "service_%d", i);
 
         check_leaks_push(ctx);
-        ret = fo_new_service(ctx->fo_ctx, buf, &services[i]);
+        ret = fo_new_service(ctx->fo_ctx, buf, NULL, &services[i]);
         fail_if(ret != EOK);
     }
 
-    ret = fo_new_service(ctx->fo_ctx, "service_3", &service);
+    ret = fo_new_service(ctx->fo_ctx, "service_3", NULL, &service);
     fail_if(ret != EEXIST);
 
     for (i = 9; i >= 0; i--) {
@@ -223,11 +223,11 @@ START_TEST(test_fo_resolve_service)
     fail_if(ctx == NULL);
 
     /* Add service. */
-    fail_if(fo_new_service(ctx->fo_ctx, "http", &service[0]) != EOK);
+    fail_if(fo_new_service(ctx->fo_ctx, "http", NULL, &service[0]) != EOK);
 
-    fail_if(fo_new_service(ctx->fo_ctx, "ldap", &service[1]) != EOK);
+    fail_if(fo_new_service(ctx->fo_ctx, "ldap", NULL, &service[1]) != EOK);
 
-    fail_if(fo_new_service(ctx->fo_ctx, "ntp", &service[2]) != EOK);
+    fail_if(fo_new_service(ctx->fo_ctx, "ntp", NULL, &service[2]) != EOK);
 
     /* Add servers. */
     fail_if(fo_add_server(service[0], "localhost", 20, NULL, true) != EOK);