[#3949] Do not add suffix to qualifed host names

/src/bin/dhcp4/tests/host_unittest.cc
/src/bin/dhcp6/tests/host_unittest.cc
    Updated tests

/src/lib/dhcpsrv/d2_client_mgr.cc
    D2ClientMgr::qualifyName() - don't add suffix to
    names that end with a dot.

/src/lib/dhcpsrv/d2_client_mgr.h
    D2ClientMgr::adjustDomainName() - strip trailing dots
    from T::PARTIAL FQDNs before calling qualifyName()

/src/lib/dhcpsrv/tests/d2_client_unittest.cc
    TEST_F(D2ClientMgrParamsTest, qualifyName) - updated

Author: tmarkwalder

changelog_unreleased/3949-ddns-qualifying-suffix-added-to-fqdn-reservations-with-different-suffixes

@@ -0,0 +1,4 @@
+[bug]		tmark
+	Avoid adding the qualifying-suffix to fully qualified
+	host names specified in host reservations.
+	(Gitlab #3949)

src/bin/dhcp4/tests/host_unittest.cc

@@ -41,12 +41,13 @@ const char* CONFIGS[] = {
     "{ \"interfaces-config\": {\n"
         "      \"interfaces\": [ \"*\" ]\n"
         "},\n"
+        "\"ddns-qualifying-suffix\": \"example.org.\",\n"
         "\"host-reservation-identifiers\": [ \"circuit-id\", \"hw-address\",\n"
         "                                    \"duid\", \"client-id\" ],\n"
         "\"reservations\": [ \n"
         "{\n"
         "   \"hw-address\": \"aa:bb:cc:dd:ee:ff\",\n"
-        "   \"hostname\": \"hw-host-dynamic\"\n"
+        "   \"hostname\": \"hw-host-dynamic.already.com.\"\n"
         "},\n"
         "{\n"
         "   \"hw-address\": \"01:02:03:04:05:06\",\n"
@@ -55,12 +56,12 @@ const char* CONFIGS[] = {
         "},\n"
         "{\n"
         "   \"hw-address\": \"02:02:03:04:05:06\",\n"
-        "   \"hostname\": \"hw-host-fixed-in-range\",\n"
+        "   \"hostname\": \"hw-host-fixed-in-range.example.org.\",\n"
         "   \"ip-address\": \"10.0.0.77\"\n"
         "},\n"
         "{\n"
         "   \"duid\": \"01:02:03:04:05\",\n"
-        "   \"hostname\": \"duid-host\"\n"
+        "   \"hostname\": \"duid-host.\"\n"
         "},\n"
         "{\n"
         "   \"circuit-id\": \"'charter950'\",\n"
@@ -590,7 +591,7 @@ TEST_F(HostTest, globalHardwareDynamicAddress) {
     Dhcp4Client client(srv_, Dhcp4Client::SELECTING);
 
     client.setHWAddress("aa:bb:cc:dd:ee:ff");
-    runDoraTest(CONFIGS[0], client, "hw-host-dynamic", "10.0.0.10");
+    runDoraTest(CONFIGS[0], client, "hw-host-dynamic.already.com", "10.0.0.10");
 }
 
 // Verifies that a client matched to a global in-subnet address reservation
@@ -600,7 +601,7 @@ TEST_F(HostTest, globalHardwareFixedAddressInRange) {
     Dhcp4Client client(srv_, Dhcp4Client::SELECTING);
 
     client.setHWAddress("02:02:03:04:05:06");
-    runDoraTest(CONFIGS[0], client, "hw-host-fixed-in-range", "10.0.0.77");
+    runDoraTest(CONFIGS[0], client, "hw-host-fixed-in-range.example.org", "10.0.0.77");
 }
 
 // Verifies that a client matched to a global out-of-range address reservation
@@ -610,7 +611,7 @@ TEST_F(HostTest, globalHardwareFixedAddressOutOfRange) {
     Dhcp4Client client(srv_, Dhcp4Client::SELECTING);
 
     client.setHWAddress("01:02:03:04:05:06");
-    runDoraTest(CONFIGS[0], client, "hw-host-fixed-out-of-range", "10.0.0.10");
+    runDoraTest(CONFIGS[0], client, "hw-host-fixed-out-of-range.example.org", "10.0.0.10");
 }
 
 // Verifies that a client can be matched to a global reservation by DUID
@@ -641,7 +642,7 @@ TEST_F(HostTest, globalCircuitId) {
     // Set the circuit id
     client.setCircuitId("charter950");
 
-    runDoraTest(CONFIGS[0], client, "circuit-id-host", "10.0.0.10");
+    runDoraTest(CONFIGS[0], client, "circuit-id-host.example.org", "10.0.0.10");
 }
 
 // Verifies that a client can be matched to a global reservation by client-id
@@ -655,7 +656,7 @@ TEST_F(HostTest, globalClientID) {
     // - 11:22:33:44:55:66 - is an actual DUID for which there is a
     client.includeClientId("01:11:22:33:44:55:66");
 
-    runDoraTest(CONFIGS[0], client, "client-id-host", "10.0.0.10");
+    runDoraTest(CONFIGS[0], client, "client-id-host.example.org", "10.0.0.10");
 }
 
 // Verifies that even when a matching global reservation exists,

src/bin/dhcp6/tests/host_unittest.cc

@@ -68,6 +68,7 @@ const char* CONFIGS[] = {
         "\"preferred-lifetime\": 3000,"
         "\"rebind-timer\": 2000, "
         "\"renew-timer\": 1000, "
+        "\"ddns-qualifying-suffix\":\"example.org.\", "
         "\"subnet6\": [ "
         " { "
         "    \"id\": 1, "
@@ -82,7 +83,8 @@ const char* CONFIGS[] = {
         "    },"
         "    {"
         "        \"duid\": \"01:02:03:05\","
-        "        \"ip-addresses\": [ \"2001:db8:1:1::babf\" ]"
+        "        \"ip-addresses\": [ \"2001:db8:1:1::babf\" ],"
+        "        \"hostname\": \"hare.mad-hatter.com.\""
         "    } ]"
         " } ]"
    "}",
@@ -1551,7 +1553,7 @@ TEST_F(HostTest, basicSarrs) {
     // and lease has reserved hostname
     Lease6Ptr lease_server = checkLease(lease_client);
     ASSERT_TRUE(lease_server);
-    EXPECT_EQ("alice", lease_server->hostname_);
+    EXPECT_EQ("alice.example.org", lease_server->hostname_);
 
     // Now redo the client, adding one to the DUID
     client.clearConfig();
@@ -1569,7 +1571,7 @@ TEST_F(HostTest, basicSarrs) {
     // and that the server lease has NO hostname
     lease_server = checkLease(lease_client);
     ASSERT_TRUE(lease_server);
-    EXPECT_EQ("", lease_server->hostname_);
+    EXPECT_EQ("hare.mad-hatter.com", lease_server->hostname_);
 
     // Now redo the client with yet another DUID and verify that
     // we get a dynamic address.

src/lib/dhcpsrv/d2_client_mgr.cc

@@ -187,22 +187,17 @@ D2ClientMgr::qualifyName(const std::string& partial_name,
                          const DdnsParams& ddns_params,
                          const bool trailing_dot) const {
     std::ostringstream gen_name;
-
     gen_name << partial_name;
     std::string suffix = ddns_params.getQualifyingSuffix();
-    bool suffix_present = true;
-    if (!suffix.empty()) {
+    if (!suffix.empty() && partial_name.back() != '.') {
+        bool suffix_present = true;
         std::string str = gen_name.str();
         auto suffix_rit = suffix.rbegin();
         if (*suffix_rit == '.') {
             ++suffix_rit;
         }
 
         auto gen_rit = str.rbegin();
-        if (*gen_rit == '.') {
-            ++gen_rit;
-        }
-
         while (suffix_rit != suffix.rend()) {
             if ((gen_rit == str.rend()) || (*suffix_rit != *gen_rit)) {
                 // They don't match.

src/lib/dhcpsrv/d2_client_mgr.h

@@ -491,7 +491,6 @@ D2ClientMgr::adjustDomainName(const T& fqdn, T& fqdn_resp, const DdnsParams& ddn
     } else {
         // Sanitize the name the client sent us, if we're configured to do so.
         std::string client_name = fqdn.getDomainName();
-
         isc::util::str::StringSanitizerPtr sanitizer = ddns_params.getHostnameSanitizer();
         if (sanitizer) {
             // We need the raw text form, so we can replace escaped chars
@@ -521,6 +520,12 @@ D2ClientMgr::adjustDomainName(const T& fqdn, T& fqdn_resp, const DdnsParams& ddn
 
         // If the supplied name is partial, qualify it by adding the suffix.
         if (fqdn.getDomainNameType() == T::PARTIAL) {
+            if (client_name.back() == '.') {
+                // By definition a partial cannot end in a dot, sanitizing above
+                // may have added one.  Strip it, so we'll add the suffix (if one).
+                client_name.pop_back();
+            }
+
             fqdn_resp.setDomainName(qualifyName(client_name, ddns_params, true), T::FULL);
         } else  {
             fqdn_resp.setDomainName(client_name, T::FULL);

src/lib/dhcpsrv/tests/d2_client_unittest.cc

@@ -639,13 +639,8 @@ TEST_F(D2ClientMgrParamsTest, qualifyName) {
     qualified_name = mgr.qualifyName(partial_name, *ddns_params_, do_dot);
     EXPECT_EQ("somehost.hasdot.com.", qualified_name);
 
-    // Verify that the qualifying suffix gets appended without an
-    // extraneous dot when partial_name ends with a "."
-    qualified_name = mgr.qualifyName("somehost.", *ddns_params_, do_dot);
-    EXPECT_EQ("somehost.hasdot.com.", qualified_name);
-
-    // Verify that a name with a trailing dot does not get an extraneous
-    // dot when the suffix is blank
+    // Verify that the qualifying suffix does not get appended nor an
+    // extraneous dot added to a name with a trailing dot.
     subnet_->setDdnsQualifyingSuffix("");
     qualified_name = mgr.qualifyName("somehost.", *ddns_params_, do_dot);
     EXPECT_EQ("somehost.", qualified_name);
@@ -664,7 +659,6 @@ TEST_F(D2ClientMgrParamsTest, qualifyName) {
     // suffix is blank and trailing dot is false
     qualified_name = mgr.qualifyName("somehost.", *ddns_params_, do_not_dot);
     EXPECT_EQ("somehost", qualified_name);
-
 }
 
 /// @brief Tests the qualifyName method's ability to avoid duplicating
@@ -802,6 +796,12 @@ TEST_F(D2ClientMgrParamsTest, adjustDomainNameV4) {
             "myhost.example.com.", Option4ClientFqdn::FULL,
             "myhost.example.com.", Option4ClientFqdn::FULL
         },
+        {
+            "RCM_NEVER #4, partial client name with traling .",
+            D2ClientConfig::RCM_NEVER,
+            "myhost.", Option4ClientFqdn::PARTIAL,
+            "myhost.suffix.com.", Option4ClientFqdn::FULL
+        },
         {
             "RCM_ALWAYS #1, empty client name",
             D2ClientConfig::RCM_ALWAYS,
@@ -917,6 +917,12 @@ TEST_F(D2ClientMgrParamsTest, adjustDomainNameV6) {
             "myhost.example.com.", Option6ClientFqdn::FULL,
             "myhost.example.com.", Option6ClientFqdn::FULL
         },
+        {
+            "RCM_NEVER #4, partial client name with trailing .",
+            D2ClientConfig::RCM_NEVER,
+            "myhost.", Option6ClientFqdn::PARTIAL,
+            "myhost.suffix.com.", Option6ClientFqdn::FULL
+        },
         {
             "RCM_ALWAYS #1, empty client name",
             D2ClientConfig::RCM_ALWAYS,