DYNDNS: improve nsupdate_msg_add_fwd()

Update nsupdate_msg_add_fwd() to group commands by address family
processed IP address belongs to.

It's better to group removing old A addresses and adding new A
addresses in a single transaction. Same goes for AAAA addresses.

Separate transaction for A and AAAA addresses updates are important
because server might block updates for one of these families and thus
the update even for the non-blocked address family would unnecessarily
fail.

For more details please see:
https://fedorahosted.org/sssd/wiki/DesignDocs/DDNSMessagesUpdate

Resolves:
https://fedorahosted.org/sssd/ticket/2495

Reviewed-by: Jakub Hrozek <jhrozek@redhat.com>

1 files changed, 39 insertions, 17 deletions

diff --git a/src/providers/dp_dyndns.c b/src/providers/dp_dyndns.c
index 782dcb60a..f717bf6cd 100644
--- a/src/providers/dp_dyndns.c
+++ b/src/providers/dp_dyndns.c
@@ -273,40 +273,62 @@ nsupdate_msg_add_fwd(char *update_msg, struct sss_iface_addr *addresses,
     char ip_addr[INET6_ADDRSTRLEN];
     errno_t ret;
 
+    /* A addresses first */
     /* Remove existing entries as needed */
     if (remove_af & DYNDNS_REMOVE_A) {
         update_msg = talloc_asprintf_append(update_msg,
-                                            "update delete %s. in A\nsend\n",
+                                            "update delete %s. in A\n",
                                             hostname);
         if (update_msg == NULL) {
             return NULL;
         }
     }
+    DLIST_FOR_EACH(new_record, addresses) {
+        if (new_record->addr->ss_family == AF_INET) {
+            ret = addr_to_str(new_record->addr, ip_addr, INET6_ADDRSTRLEN);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_MINOR_FAILURE, "addr_to_str failed: %d:[%s],\n",
+                      ret, sss_strerror(ret));
+                return NULL;
+            }
+
+            /* Format the record update */
+            update_msg = talloc_asprintf_append(update_msg,
+                                                "update add %s. %d in %s %s\n",
+                                                hostname, ttl, "A", ip_addr);
+            if (update_msg == NULL) {
+                return NULL;
+            }
+        }
+    }
+    update_msg = talloc_asprintf_append(update_msg, "send\n");
+
+    /* AAAA addresses next */
+    /* Remove existing entries as needed */
     if (remove_af & DYNDNS_REMOVE_AAAA) {
         update_msg = talloc_asprintf_append(update_msg,
-                                            "update delete %s. in AAAA\nsend\n",
+                                            "update delete %s. in AAAA\n",
                                             hostname);
         if (update_msg == NULL) {
             return NULL;
         }
     }
-
     DLIST_FOR_EACH(new_record, addresses) {
-        ret = addr_to_str(new_record->addr, ip_addr, INET6_ADDRSTRLEN);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_MINOR_FAILURE, "addr_to_str failed: %d:[%s],\n",
-                  ret, sss_strerror(ret));
-            return NULL;
-        }
+        if (new_record->addr->ss_family == AF_INET6) {
+            ret = addr_to_str(new_record->addr, ip_addr, INET6_ADDRSTRLEN);
+            if (ret != EOK) {
+                DEBUG(SSSDBG_MINOR_FAILURE, "addr_to_str failed: %d:[%s],\n",
+                      ret, sss_strerror(ret));
+                return NULL;
+            }
 
-        /* Format the record update */
-        update_msg = talloc_asprintf_append(update_msg,
-                "update add %s. %d in %s %s\n",
-                hostname, ttl,
-                new_record->addr->ss_family == AF_INET ? "A" : "AAAA",
-                ip_addr);
-        if (update_msg == NULL) {
-            return NULL;
+            /* Format the record update */
+            update_msg = talloc_asprintf_append(update_msg,
+                                                "update add %s. %d in %s %s\n",
+                                                hostname, ttl, "AAAA", ip_addr);
+            if (update_msg == NULL) {
+                return NULL;
+            }
         }
     }