[5007] Addressed more review comments

src/bin/dhcp6/dhcp6_srv.cc
    Dhcpv6Srv::createNameChangeRequests() - modified logic DNS-updated needed
    logic to include changes to the FQDN flags.

src/bin/dhcp6/tests/fqdn_unittest.cc
    TEST_F(FqdnDhcpv6SrvTest, processRequestRenewFqdnFlags) - new test to
    verify the permutations of DNS direction flags on NCR generation

diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 1f12e4e80d00693223ed4951362e683ed135cc9a..f8d7571a6a7a043ca454cd7e8e96ef444931e91e 100644 (file)
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -1270,13 +1270,16 @@ Dhcpv6Srv::createNameChangeRequests(const Pkt6Ptr& answer,
             continue;
         }
 
-        // see if the lease for iaadr is in changed_leases, and if so
-        // if the FQDN is different, if not continue
+        // If the lease for iaaddr is in the list of changed leases, we need
+        // to determine if the changes included changes to the FQDN. If there
+        // were  changes to the FQDN then we need to update DNS, otherwise
+        // we do not.
         bool extended_only = false;
         for (Lease6Collection::const_iterator l = ctx.currentIA().changed_leases_.begin();
              l != ctx.currentIA().changed_leases_.end(); ++l) {
             if ((*l)->addr_ == iaaddr->getAddress()) {
-                if ((*l)->hostname_ == opt_fqdn->getDomainName()) {
+                if ((*l)->hostname_ == opt_fqdn->getDomainName() &&
+                    (*l)->fqdn_fwd_ == do_fwd && (*l)->fqdn_rev_ == do_rev) {
                     extended_only = true;
                     break;
                 }
@@ -1287,7 +1290,6 @@ Dhcpv6Srv::createNameChangeRequests(const Pkt6Ptr& answer,
             continue;
         }
 
-
         // Create new NameChangeRequest. Use the domain name from the FQDN.
         // This is an FQDN included in the response to the client, so it
         // holds a fully qualified domain-name already (not partial).

diff --git a/src/bin/dhcp6/tests/fqdn_unittest.cc b/src/bin/dhcp6/tests/fqdn_unittest.cc
index 9fb951b65a9417d3ea138beaa9c6c4f2d7831a0a..d645995f9da0cb074febe0ef3fcafd66a774d849 100644 (file)
--- a/src/bin/dhcp6/tests/fqdn_unittest.cc
+++ b/src/bin/dhcp6/tests/fqdn_unittest.cc
@@ -1114,6 +1114,83 @@ TEST_F(FqdnDhcpv6SrvTest, processRequestRenewSameFqdn) {
     ASSERT_EQ(0, d2_mgr_.getQueueSize());
 }
 
+// Tests that renewals using the same domain name but differing values for
+// the directional update flags result in NCRs or not, accordingly.
+// If the new leases's flags are the same as the previous lease's flags,
+// then no requests should be generated.  If at lease one of the new lease's
+// flags differ from the previous lease, then:
+// A: A removal NCR should be created based on the previous leases's flags
+// if at least one of those flags are true
+// B: An add NCR should be created based on the new lease's flags, if at
+// least one of those flags are true
+TEST_F(FqdnDhcpv6SrvTest, processRequestRenewFqdnFlags) {
+    // Create a Request message with FQDN option but with N flag = 1, which
+    // means no updates should be done. This should result in no NCRs.
+    testProcessMessage(DHCPV6_REQUEST, "myhost.example.com",
+                       "myhost.example.com.", Option6ClientFqdn::FLAG_N);
+    // Queue should be empty.
+    ASSERT_EQ(0, d2_mgr_.getQueueSize());
+
+    // Now renew with Both N and S = 0.  This means the server should only
+    // do reverse updates and should result in a reverse-only NCR.
+    testProcessMessage(DHCPV6_RENEW, "myhost.example.com",
+                       "myhost.example.com.", 0);
+    // We should a only have reverse-only ADD, no remove.
+    ASSERT_EQ(1, d2_mgr_.getQueueSize());
+    verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, false,
+                            "2001:db8:1:1::dead:beef",
+                            "000201415AA33D1187D148275136FA30300478"
+                            "FAAAA3EBD29826B5C907B2C9268A6F52",
+                            0, 4000);
+
+    // Renew again with the same flags, this should not generate any NCRs.
+    testProcessMessage(DHCPV6_RENEW, "myhost.example.com",
+                       "myhost.example.com.", 0);
+    // Queue should be empty.
+    ASSERT_EQ(0, d2_mgr_.getQueueSize());
+
+
+    // Renew with both N and S flags = 0. This tells the server to update
+    // both directions, which should change forward flag to true. This should
+    // generate a reverse only remove and a dual add.
+    testProcessMessage(DHCPV6_RENEW, "myhost.example.com",
+                       "myhost.example.com.", Option6ClientFqdn::FLAG_S);
+
+    // We need the lease for the expiration value.
+    lease_ = LeaseMgrFactory::
+             instance().getLease6(Lease::TYPE_NA,
+                                  IOAddress("2001:db8:1:1::dead:beef"));
+    ASSERT_TRUE(lease_);
+
+    // We should have two NCRs, one remove and one add.
+    ASSERT_EQ(2, d2_mgr_.getQueueSize());
+    verifyNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, true, false,
+                            "2001:db8:1:1::dead:beef",
+                            "000201415AA33D1187D148275136FA30300478"
+                            "FAAAA3EBD29826B5C907B2C9268A6F52",
+                            lease_->cltt_ + lease_->valid_lft_, 4000);
+
+    verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
+                            "2001:db8:1:1::dead:beef",
+                            "000201415AA33D1187D148275136FA30300478"
+                            "FAAAA3EBD29826B5C907B2C9268A6F52",
+                            0, 4000);
+
+    // Lastly, we renew with the N flag = 1 (which means no updates) so we
+    // should have a dual direction remove NCR but NO add NCR.
+    testProcessMessage(DHCPV6_RENEW, "myhost.example.com",
+                       "myhost.example.com.", Option6ClientFqdn::FLAG_N);
+    // We should only have the removal NCR.
+    ASSERT_EQ(1, d2_mgr_.getQueueSize());
+    verifyNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, true, true,
+                            "2001:db8:1:1::dead:beef",
+                            "000201415AA33D1187D148275136FA30300478"
+                            "FAAAA3EBD29826B5C907B2C9268A6F52",
+                            lease_->cltt_ + lease_->valid_lft_, 4000);
+
+}
+
+
 TEST_F(FqdnDhcpv6SrvTest, processRequestRelease) {
     // Create a Request message with FQDN option and generate server's
     // response using processRequest function. This will result in the