BIND 10 trac3352, updated. 4fc2c2cb250f93809d3287bdf9d50f5cbbc0fd17 [3352] Added test to not create NCRs if FQDN flags indicate none required
BIND 10 source code commits
bind10-changes at lists.isc.org
Sun Mar 9 14:49:22 UTC 2014
The branch, trac3352 has been updated
via 4fc2c2cb250f93809d3287bdf9d50f5cbbc0fd17 (commit)
via 614d16ac5d403ff87883c2881f01e1dda3214e75 (commit)
from 4003a67bb649d47e9e39ba1bded31f8919614e1e (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 4fc2c2cb250f93809d3287bdf9d50f5cbbc0fd17
Author: Thomas Markwalder <tmark at isc.org>
Date: Sun Mar 9 10:46:28 2014 -0400
[3352] Added test to not create NCRs if FQDN flags indicate none required
Dhcpv6Srv::createNameChangeRequests was not catching cases where FQDN flags
indicate NO changes should be made. This causes an exception to thrown by
the NCR constructor stating that both forward and reverse flags cannot be
false. Also moved the check up in the method, above of the options loop.
commit 614d16ac5d403ff87883c2881f01e1dda3214e75
Author: Thomas Markwalder <tmark at isc.org>
Date: Sun Mar 9 10:24:40 2014 -0400
[3352] Augment DHPC6 FQDN tests to properly check handling of FQDN flags
Unit tests did not detect that Dhcpv6Srv::createNameChangeRequests attempts
to make NCRs when the response FQDN flags indicate that none should be created.
FqdnDhcpv6Srv::testFqdn has been expanded to invoke this method and check the
outcome.
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp6/dhcp6_srv.cc | 23 +++++++------
src/bin/dhcp6/tests/fqdn_unittest.cc | 59 +++++++++++++++++++++++++++-------
2 files changed, 61 insertions(+), 21 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 8511ef7..a07bec8 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -1032,6 +1032,19 @@ Dhcpv6Srv::createNameChangeRequests(const Pkt6Ptr& answer) {
return;
}
+ // Get the update directions that should be performed based on our
+ // response FQDN flags.
+ bool do_fwd = false;
+ bool do_rev = false;
+ CfgMgr::instance().getD2ClientMgr().getUpdateDirections(*opt_fqdn,
+ do_fwd, do_rev);
+ if (!do_fwd && !do_rev) {
+ // Flags indicate there is Nothing to do, get out now.
+ // The Most likely scenario is that we are honoring the client's
+ // request that no updates be done.
+ return;
+ }
+
// Get the Client Id. It is mandatory and a function creating a response
// would have thrown an exception if it was missing. Thus throwning
// Unexpected if it is missing as it is a programming error.
@@ -1071,16 +1084,8 @@ Dhcpv6Srv::createNameChangeRequests(const Pkt6Ptr& answer) {
// 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).
- // Get the IP address from the lease. Also, use the S flag to determine
- // if forward change should be performed. This flag will always be
- // set if server has taken responsibility for the forward update.
- // Use the inverse of the N flag to determine if a reverse change
- // should be performed.
+ // Get the IP address from the lease.
NameChangeRequestPtr ncr;
- bool do_fwd = false;
- bool do_rev = false;
- CfgMgr::instance().getD2ClientMgr().getUpdateDirections(*opt_fqdn,
- do_fwd, do_rev);
ncr.reset(new NameChangeRequest(isc::dhcp_ddns::CHG_ADD,
do_fwd, do_rev,
opt_fqdn->getDomainName(),
diff --git a/src/bin/dhcp6/tests/fqdn_unittest.cc b/src/bin/dhcp6/tests/fqdn_unittest.cc
index 7a871dd..fe5542f 100644
--- a/src/bin/dhcp6/tests/fqdn_unittest.cc
+++ b/src/bin/dhcp6/tests/fqdn_unittest.cc
@@ -172,6 +172,7 @@ public:
pkt->addOption(oro);
}
+ OptionPtr tmp = pkt->getOption(D6O_CLIENTID);
return (pkt);
}
@@ -264,13 +265,16 @@ public:
}
/// @brief Verifies if the DHCPv6 server processes DHCPv6 Client FQDN option
- /// as expected.
+ /// as expected and subsequent interpretation of this processing when
+ /// creating NCRs, if any should be created.
///
/// This function simulates generation of the client's message holding FQDN.
/// It then calls the server's @c Dhcpv6Srv::processClientFqdn option to
/// generate server's response to the FQDN. This function returns the FQDN
/// which should be appended to the server's response to the client.
- /// This function verifies that the FQDN option returned is correct.
+ /// This function verifies that the FQDN option returned is correct
+ /// Optionally, this function then proceeds to call createNameChangeRequests
+ /// to verify if that method interprets the FQDN information properly.
///
/// @param msg_type A type of the client's message.
/// @param in_flags A value of flags field to be set for the FQDN carried
@@ -282,22 +286,34 @@ public:
/// @param exp_flags A value of flags expected in the FQDN sent by a server.
/// @param exp_domain_name A domain name expected in the FQDN sent by a
/// server.
+ /// @param create_ncr_check if true, calls createNameChangeRequests method
+ /// and tests the outcome.
+ /// @param exp_fwd indicates whether or not a forward change is expected
+ /// @param exp_fwd indicates whether or not a reverse change is expected
void testFqdn(const uint16_t msg_type,
const uint8_t in_flags,
const std::string& in_domain_name,
const Option6ClientFqdn::DomainNameType in_domain_type,
const uint8_t exp_flags,
- const std::string& exp_domain_name) {
+ const std::string& exp_domain_name,
+ const bool create_ncr_check,
+ const bool exp_fwd = true, const bool exp_rev = true) {
Pkt6Ptr question = generateMessage(msg_type,
in_flags,
in_domain_name,
in_domain_type,
true);
+
ASSERT_TRUE(getClientFqdnOption(question));
- Pkt6Ptr answer(new Pkt6(msg_type == DHCPV6_SOLICIT ? DHCPV6_ADVERTISE :
- DHCPV6_REPLY, question->getTransid()));
+ Pkt6Ptr answer = generateMessageWithIds(msg_type == DHCPV6_SOLICIT
+ ? DHCPV6_ADVERTISE :
+ DHCPV6_REPLY);
+
+ // Create three IAs, each having different address.
+ addIA(1234, IOAddress("2001:db8:1::1"), answer);
+
ASSERT_NO_THROW(srv_->processClientFqdn(question, answer));
Option6ClientFqdnPtr answ_fqdn = boost::dynamic_pointer_cast<
Option6ClientFqdn>(answer->getOption(D6O_CLIENT_FQDN));
@@ -325,6 +341,25 @@ public:
EXPECT_EQ(Option6ClientFqdn::FULL, answ_fqdn->getDomainNameType());
}
+
+ if (create_ncr_check) {
+ // Call createNameChangeRequests
+ ASSERT_NO_THROW(srv_->createNameChangeRequests(answer));
+ if (exp_fwd || exp_rev) {
+ // Should have created 1 NCR.
+ NameChangeRequestPtr ncr;
+ ASSERT_EQ(1, d2_mgr_.getQueueSize());
+ ASSERT_NO_THROW(ncr = d2_mgr_.peekAt(0));
+ ASSERT_TRUE(ncr);
+ EXPECT_EQ(dhcp_ddns::CHG_ADD, ncr->getChangeType());
+ EXPECT_EQ(exp_fwd, ncr->isForwardChange());
+ EXPECT_EQ(exp_rev, ncr->isReverseChange());
+ ASSERT_NO_THROW(d2_mgr_.runReadyIO());
+ } else {
+ // Should not have created any NCRs.
+ EXPECT_EQ(0, d2_mgr_.getQueueSize());
+ }
+ }
}
/// @brief Tests that the client's message holding an FQDN is processed
@@ -468,7 +503,7 @@ TEST_F(FqdnDhcpv6SrvTest, serverAAAAUpdate) {
testFqdn(DHCPV6_SOLICIT, Option6ClientFqdn::FLAG_S,
"myhost.example.com",
Option6ClientFqdn::FULL, Option6ClientFqdn::FLAG_S,
- "myhost.example.com.");
+ "myhost.example.com.", true, true, true);
}
// Test server's response when client provides partial domain-name and requests
@@ -476,14 +511,14 @@ TEST_F(FqdnDhcpv6SrvTest, serverAAAAUpdate) {
TEST_F(FqdnDhcpv6SrvTest, serverAAAAUpdatePartialName) {
testFqdn(DHCPV6_SOLICIT, Option6ClientFqdn::FLAG_S, "myhost",
Option6ClientFqdn::PARTIAL, Option6ClientFqdn::FLAG_S,
- "myhost.example.com.");
+ "myhost.example.com.", true, true, true);
}
// Test server's response when client provides empty domain-name and requests
// that server performs AAAA update.
TEST_F(FqdnDhcpv6SrvTest, serverAAAAUpdateNoName) {
testFqdn(DHCPV6_SOLICIT, Option6ClientFqdn::FLAG_S, "",
- Option6ClientFqdn::PARTIAL, Option6ClientFqdn::FLAG_S, "");
+ Option6ClientFqdn::PARTIAL, Option6ClientFqdn::FLAG_S, "", false);
}
// Test server's response when client requests no DNS update.
@@ -491,7 +526,7 @@ TEST_F(FqdnDhcpv6SrvTest, noUpdate) {
testFqdn(DHCPV6_SOLICIT, Option6ClientFqdn::FLAG_N,
"myhost.example.com",
Option6ClientFqdn::FULL, Option6ClientFqdn::FLAG_N,
- "myhost.example.com.");
+ "myhost.example.com.", true, false, false);
}
// Test server's response when client requests no DNS update and
@@ -502,7 +537,7 @@ TEST_F(FqdnDhcpv6SrvTest, overrideNoUpdate) {
"myhost.example.com",
Option6ClientFqdn::FULL,
(Option6ClientFqdn::FLAG_S | Option6ClientFqdn::FLAG_O),
- "myhost.example.com.");
+ "myhost.example.com.", true, true, true);
}
// Test server's response when client requests that server delegates the AAAA
@@ -511,7 +546,7 @@ TEST_F(FqdnDhcpv6SrvTest, clientAAAAUpdate) {
testFqdn(DHCPV6_SOLICIT, 0, "myhost.example.com.",
Option6ClientFqdn::FULL,
0,
- "myhost.example.com.");
+ "myhost.example.com.", true, false, true);
}
// Test server's response when client requests that server delegates the AAAA
@@ -521,7 +556,7 @@ TEST_F(FqdnDhcpv6SrvTest, clientAAAAUpdateNotAllowed) {
testFqdn(DHCPV6_SOLICIT, 0, "myhost.example.com.",
Option6ClientFqdn::FULL,
Option6ClientFqdn::FLAG_S | Option6ClientFqdn::FLAG_O,
- "myhost.example.com.");
+ "myhost.example.com.", true, true, true);
}
// Test that exception is thrown if supplied NULL answer packet when
More information about the bind10-changes
mailing list