From: Thomas Markwalder Date: Mon, 19 Sep 2016 13:08:54 +0000 (-0400) Subject: [5007] Addressed more review comments X-Git-Tag: trac5049_base~15^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6d14c6bdd7011e05b1e211ca0752e20be4c5adfa;p=thirdparty%2Fkea.git [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 1f12e4e80d..f8d7571a6a 100644 --- 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 9fb951b65a..d645995f9d 100644 --- 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