Parcourir la source

[3241] Add ability to build DNS update requests to d2::NameAddTransaction

Added methods for constructing all three types of DNS update requests
required by d2::NameAddTransaction to complete the implementation of its
state machine.  Also refactored some unit test code into nc_test_utils.h
and .cc, and ran much needed spell checking.
Thomas Markwalder il y a 11 ans
Parent
commit
5414d2bdab

+ 27 - 0
src/bin/d2/d2_messages.mes

@@ -324,3 +324,30 @@ This is error message issued when the application is able to construct an update
 message but the attempt to send it suffered a unexpected error. This is most
 likely a programmatic error, rather than a communications issue. Some or all
 of the DNS updates requested as part of this request did not succeed.
+
+% DHCP_DDNS_FORWARD_ADD_BUILD_FAILURE A DNS udpate message to add a forward DNS entry could not be constructed for this request: %1  reason: %2
+This is an error message issued when an error occurs attempting to construct
+the server bound packet requesting a forward address addition.  This is due
+to invalid data contained in the NameChangeRequest. The request will be aborted.
+This is most likely a configuration issue.
+
+% DHCP_DDNS_FORWARD_REPLACE_BUILD_FAILURE A DNS update message to replace a foward DNS entry could not be constructed from this request: %1  reason: %2
+This is an error message issued when an error occurs attempting to construct
+the server bound packet requesting a forward address replacement.  This is
+due to invalid data contained in the NameChangeRequest. The request will be
+aborted.  This is most likely a configuration issue.
+
+% DHCP_DDNS_REVERSE_REPLACE_BUILD_FAILURE A DNS update message to replace a reverse DNS entry could not be constructed from this request: %1  reason: %2
+This is an error message issued when an error occurs attempting to construct
+the server bound packet requesting a reverse PTR replacement.  This is
+due to invalid data contained in the NameChangeRequest. The request will be
+aborted.  This is most likely a configuration issue.
+
+% DHCP_DDNS_ADD_SUCCEEDED DHCP_DDNS successfully added the DNS mapping addition for this request: %1
+This is a debug message issued after DHCP_DDNS has submitted DNS mapping
+additions which were received and accepted by an appropriate DNS server.
+
+% DHCP_DDNS_ADD_FAILED DHCP_DDNS failed attempting to make DNS mapping additions for this request: %1
+This is an error message issued after DHCP_DDNS attempts to submit DNS mapping
+entry additions have failed.  There precise reason for the failure should be
+documented in preceding log entries.

+ 1 - 1
src/bin/d2/d2_update_message.cc

@@ -31,7 +31,7 @@ D2UpdateMessage::D2UpdateMessage(const Direction direction)
     if (direction == OUTBOUND) {
         message_.setOpcode(Opcode(Opcode::UPDATE_CODE));
         message_.setHeaderFlag(dns::Message::HEADERFLAG_QR, false);
-
+        message_.setRcode(Rcode(Rcode::NOERROR_CODE));
     }
 }
 

+ 151 - 14
src/bin/d2/nc_add.cc

@@ -13,11 +13,15 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <d2/d2_log.h>
+#include <d2/d2_cfg_mgr.h>
 #include <d2/nc_add.h>
 
 #include <boost/function.hpp>
 #include <boost/bind.hpp>
 
+#include <util/buffer.h>
+#include <dns/rdataclass.h>
+
 namespace isc {
 namespace d2 {
 
@@ -168,7 +172,17 @@ NameAddTransaction::addingFwdAddrsHandler() {
     case SERVER_SELECTED_EVT:
         if (!getDnsUpdateRequest()) {
             // Request hasn't been constructed yet, so build it.
-            buildAddFwdAddressRequest();
+            try {
+                buildAddFwdAddressRequest();
+            } catch (const std::exception& ex) {
+                // While unlikely, the build might fail if we have invalid
+                // data.  Should that be the case, we need to fail the
+                // transaction.
+                LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_ADD_BUILD_FAILURE)
+                          .arg(getNcr()->toText())
+                          .arg(ex.what());
+                transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT);
+            }
         }
 
         // Call sendUpdate() to initiate the async send. Note it also sets
@@ -268,7 +282,17 @@ NameAddTransaction::replacingFwdAddrsHandler() {
     case SERVER_SELECTED_EVT:
         if (!getDnsUpdateRequest()) {
             // Request hasn't been constructed yet, so build it.
-            buildReplaceFwdAddressRequest();
+            try {
+                buildReplaceFwdAddressRequest();
+            } catch (const std::exception& ex) {
+                // While unlikely, the build might fail if we have invalid
+                // data.  Should that be the case, we need to fail the
+                // transaction.
+                LOG_ERROR(dctl_logger, DHCP_DDNS_FORWARD_REPLACE_BUILD_FAILURE)
+                          .arg(getNcr()->toText())
+                          .arg(ex.what());
+                transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT);
+            }
         }
 
         // Call sendUpdate() to initiate the async send. Note it also sets
@@ -403,7 +427,17 @@ NameAddTransaction::replacingRevPtrsHandler() {
     case SERVER_SELECTED_EVT:
         if (!getDnsUpdateRequest()) {
             // Request hasn't been constructed yet, so build it.
-            buildReplaceRevPtrsRequest();
+            try {
+                buildReplaceRevPtrsRequest();
+            } catch (const std::exception& ex) {
+                // While unlikely, the build might fail if we have invalid
+                // data.  Should that be the case, we need to fail the
+                // transaction.
+                LOG_ERROR(dctl_logger, DHCP_DDNS_REVERSE_REPLACE_BUILD_FAILURE)
+                          .arg(getNcr()->toText())
+                          .arg(ex.what());
+                transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT);
+            }
         }
 
         // Call sendUpdate() to initiate the async send. Note it also sets
@@ -488,7 +522,8 @@ void
 NameAddTransaction::processAddOkHandler() {
     switch(getNextEvent()) {
     case UPDATE_OK_EVT:
-        // @todo do we need a log statement here?
+        LOG_DEBUG(dctl_logger, DBGLVL_TRACE_DETAIL, DHCP_DDNS_ADD_SUCCEEDED)
+                  .arg(getNcr()->toText());
         setNcrStatus(dhcp_ddns::ST_COMPLETED);
         endModel();
         break;
@@ -503,7 +538,7 @@ void
 NameAddTransaction::processAddFailedHandler() {
     switch(getNextEvent()) {
     case UPDATE_FAILED_EVT:
-        // @todo do we need a log statement here?
+        LOG_ERROR(dctl_logger, DHCP_DDNS_ADD_FAILED).arg(getNcr()->toText());
         setNcrStatus(dhcp_ddns::ST_FAILED);
         endModel();
         break;
@@ -516,23 +551,125 @@ NameAddTransaction::processAddFailedHandler() {
 
 void
 NameAddTransaction::buildAddFwdAddressRequest() {
-    // @todo For now construct a blank outbound message.
-    D2UpdateMessagePtr msg(new D2UpdateMessage(D2UpdateMessage::OUTBOUND));
-    setDnsUpdateRequest(msg);
+    // Construct an empty request.
+    D2UpdateMessagePtr request = prepNewRequest(getForwardDomain());
+
+    // Construct dns::Name from NCR fqdn.
+    dns::Name fqdn(dns::Name(getNcr()->getFqdn()));
+
+    // First build the Prerequisite Section.
+
+    // Create 'FQDN Is Not In Use' prerequisite (RFC 2136, section 2.4.5)
+    // Add the RR to prerequisite section.
+    dns::RRsetPtr prereq(new dns::RRset(fqdn, dns::RRClass::NONE(),
+                             dns::RRType::ANY(), dns::RRTTL(0)));
+    request->addRRset(D2UpdateMessage::SECTION_PREREQUISITE, prereq);
+
+    // Next build the Update Section.
+
+    // Create the FQDN/IP 'add' RR (RFC 2136, section 2.5.1)
+    // Set the message RData to lease address.
+    // Add the RR to update section.
+    dns::RRsetPtr update(new dns::RRset(fqdn, dns::RRClass::IN(),
+                         getAddressRRType(), dns::RRTTL(0)));
+
+    addLeaseAddressRdata(update);
+    request->addRRset(D2UpdateMessage::SECTION_UPDATE, update);
+    // Now create the FQDN/DHCID 'add' RR per RFC 4701)
+    // Set the message RData to DHCID.
+    // Add the RR to update section.
+    update.reset(new dns::RRset(fqdn, dns::RRClass::IN(),
+                                dns::RRType::DHCID(), dns::RRTTL(0)));
+    addDhcidRdata(update);
+    request->addRRset(D2UpdateMessage::SECTION_UPDATE, update);
+
+    // Set the transaction's update request to the new request.
+    setDnsUpdateRequest(request);
 }
 
 void
 NameAddTransaction::buildReplaceFwdAddressRequest() {
-    // @todo For now construct a blank outbound message.
-    D2UpdateMessagePtr msg(new D2UpdateMessage(D2UpdateMessage::OUTBOUND));
-    setDnsUpdateRequest(msg);
+    // Construct an empty request.
+    D2UpdateMessagePtr request = prepNewRequest(getForwardDomain());
+
+    // Construct dns::Name from NCR fqdn.
+    dns::Name fqdn(dns::Name(getNcr()->getFqdn()));
+
+    // First build the Prerequisite Section.
+
+    // Create an 'FQDN Is In Use' prerequisite (RFC 2136, section 2.4.4)
+    // Add it to the pre-requisite section.
+    dns::RRsetPtr prereq(new dns::RRset(fqdn, dns::RRClass::ANY(),
+                               dns::RRType::ANY(), dns::RRTTL(0)));
+    request->addRRset(D2UpdateMessage::SECTION_PREREQUISITE, prereq);
+
+    // Now create an DHCID matches prerequisite RR.
+    // Set the RR's RData to DHCID.
+    // Add it to the pre-requisite section.
+    prereq.reset(new dns::RRset(fqdn, dns::RRClass::IN(),
+                 dns::RRType::DHCID(), dns::RRTTL(0)));
+    addDhcidRdata(prereq);
+    request->addRRset(D2UpdateMessage::SECTION_PREREQUISITE, prereq);
+
+    // Next build the Update Section.
+
+    // Create the FQDN/IP 'delete' RR (RFC 2136, section 2.5.1)
+    // Set the message RData to lease address.
+    // Add the RR to update section.
+    dns::RRsetPtr update(new dns::RRset(fqdn, dns::RRClass::ANY(),
+                         getAddressRRType(), dns::RRTTL(0)));
+    request->addRRset(D2UpdateMessage::SECTION_UPDATE, update);
+
+    // Create the FQDN/IP 'add' RR (RFC 2136, section 2.5.1)
+    // Set the message RData to lease address.
+    // Add the RR to update section.
+    update.reset(new dns::RRset(fqdn, dns::RRClass::IN(),
+                                getAddressRRType(), dns::RRTTL(0)));
+    addLeaseAddressRdata(update);
+    request->addRRset(D2UpdateMessage::SECTION_UPDATE, update);
+
+    // Set the transaction's update request to the new request.
+    setDnsUpdateRequest(request);
 }
 
 void
 NameAddTransaction::buildReplaceRevPtrsRequest() {
-    // @todo For now construct a blank outbound message.
-    D2UpdateMessagePtr msg(new D2UpdateMessage(D2UpdateMessage::OUTBOUND));
-    setDnsUpdateRequest(msg);
+    // Construct an empty request.
+    D2UpdateMessagePtr request = prepNewRequest(getReverseDomain());
+
+    // Create the reverse IP address "FQDN".
+    std::string rev_addr = D2CfgMgr::reverseIpAddress(getNcr()->getIpAddress());
+    dns::Name rev_ip(rev_addr);
+
+    // Reverse replacement has no prerequisites so straight on to
+    // building the Update section.
+
+    // Create the PTR 'delete' RR and add it to update section.
+    dns::RRsetPtr update(new dns::RRset(rev_ip, dns::RRClass::ANY(),
+                         dns::RRType::PTR(), dns::RRTTL(0)));
+    request->addRRset(D2UpdateMessage::SECTION_UPDATE, update);
+
+    // Create the DHCID 'delete' RR and add it to the update section.
+    update.reset(new dns::RRset(rev_ip, dns::RRClass::ANY(),
+                                dns::RRType::DHCID(), dns::RRTTL(0)));
+    request->addRRset(D2UpdateMessage::SECTION_UPDATE, update);
+
+    // Create the FQDN/IP PTR 'add' RR, add the FQDN as the PTR Rdata
+    // then add it to update section.
+    update.reset(new dns::RRset(rev_ip, dns::RRClass::IN(),
+                                dns::RRType::PTR(), dns::RRTTL(0)));
+    addPtrRdata(update);
+    request->addRRset(D2UpdateMessage::SECTION_UPDATE, update);
+
+    // Create the FQDN/IP PTR 'add' RR, add the DHCID Rdata
+    // then add it to update section.
+    update.reset(new dns::RRset(rev_ip, dns::RRClass::IN(),
+                                dns::RRType::DHCID(), dns::RRTTL(0)));
+    addDhcidRdata(update);
+    request->addRRset(D2UpdateMessage::SECTION_UPDATE, update);
+
+    // Set the transaction's update request to the new request.
+    setDnsUpdateRequest(request);
 }
 
 } // namespace isc::d2

+ 47 - 10
src/bin/d2/nc_add.h

@@ -18,6 +18,7 @@
 /// @file nc_add.h This file defines the class NameAddTransaction.
 
 #include <d2/nc_trans.h>
+#include <dns/rdata.h>
 
 namespace isc {
 namespace d2 {
@@ -220,7 +221,7 @@ protected:
     /// post a next event of IO_COMPLETED_EVT and then invoke runModel which
     /// resumes execution of the state model.
     ///
-    /// When the handler is invoked with a next event of IO_COMPELTED_EVT,
+    /// When the handler is invoked with a next event of IO_COMPLETED_EVT,
     /// the DNS update status is checked and acted upon accordingly:
     ///
     /// Transitions to:
@@ -270,7 +271,7 @@ protected:
     /// post a next event of IO_COMPLETED_EVT and then invoke runModel which
     /// resumes execution of the state model.
     ///
-    /// When the handler is invoked with a next event of IO_COMPELTED_EVT,
+    /// When the handler is invoked with a next event of IO_COMPLETED_EVT,
     /// the DNS update status is checked and acted upon accordingly:
     ///
     /// Transitions to:
@@ -321,7 +322,7 @@ protected:
     /// post a next event of IO_COMPLETED_EVT and then invoke runModel which
     /// resumes execution of the state model.
     ///
-    /// When the handler is invoked with a next event of IO_COMPELTED_EVT,
+    /// When the handler is invoked with a next event of IO_COMPLETED_EVT,
     /// the DNS update status is checked and acted upon accordingly:
     ///
     /// Transitions to:
@@ -380,25 +381,61 @@ protected:
 
     /// @brief Builds a DNS request to add an forward DNS entry for an FQDN
     ///
-    /// @todo - Method not implemented yet
+    /// Constructs a DNS update request, based upon the NCR, for adding a
+    /// forward DNS mapping.  Once constructed, the request is stored as
+    /// the transaction's DNS update request.
     ///
-    /// @throw isc::NotImplemented
+    /// The request content is adherent to RFC 4703 section 5.3.1:
+    ///
+    /// Prerequisite RRsets:
+    /// 1. An assertion that the FQDN does not exist
+    ///
+    /// Updates RRsets:
+    /// 1. An FQDN/IP RR addition    (type A for IPv4, AAAA for IPv6)
+    /// 2. An FQDN/DHCID RR addition (type DHCID)
+    ///
+    /// @throw This method does not throw but underlying methods may.
     void buildAddFwdAddressRequest();
 
     /// @brief Builds a DNS request to replace forward DNS entry for an FQDN
     ///
-    /// @todo - Method not implemented yet
+    /// Constructs a DNS update request, based upon the NCR, for replacing a
+    /// forward DNS mapping.  Once constructed, the request is stored as
+    /// the transaction's DNS update request.
+    ///
+    /// The request content is adherent to RFC 4703 section 5.3.2:
+    ///
+    /// Prerequisite RRsets:
+    /// 1. An assertion that the FQDN is in use
+    /// 2. An assertion that the FQDN/DHCID RR exists for the lease client's
+    /// DHCID.
     ///
-    /// @throw isc::NotImplemented
+    /// Updates RRsets:
+    /// 1. A deletion of any existing FQDN RRs (type A for IPv4, AAAA for IPv6)
+    /// 2. A FQDN/IP RR addition (type A for IPv4, AAAA for IPv6)
+    ///
+    /// @throw This method does not throw but underlying methods may.
     void buildReplaceFwdAddressRequest();
 
     /// @brief Builds a DNS request to replace a reverse DNS entry for an FQDN
     ///
-    /// @todo - Method not implemented yet
+    /// Constructs a DNS update request, based upon the NCR, for replacing a
+    /// reverse DNS mapping.  Once constructed, the request is stored as
+    /// the transaction's DNS update request.
+    ///
+    /// The request content is adherent to RFC 4703 section 5.4:
+    ///
+    /// Prerequisite RRsets:
+    /// - There are not prerequisites.
     ///
-    /// @throw isc::NotImplemented
+    /// Updates RRsets:
+    /// 1. A delete of any existing PTR RRs for the lease address
+    /// 2. A delete of any existing DHCID RRs for the lease address
+    /// 3. A PTR RR addition for the lease address and FQDN
+    /// 4. A DHCID RR addition for the lease address and lease client DHCID
+    ///
+    /// @throw This method does not throw but underlying methods may.
     void buildReplaceRevPtrsRequest();
-
 };
 
 /// @brief Defines a pointer to a NameChangeTransaction.

+ 83 - 1
src/bin/d2/nc_trans.cc

@@ -113,7 +113,7 @@ NameChangeTransaction::sendUpdate(bool /* use_tsig_ */) {
         // It is presumed that any throw from doUpdate is due to a programmatic
         // error, such as an unforeseen permutation of data, rather than an IO
         // failure. IO errors should be caught by the underlying asiolink
-        // mechansisms and manifested as an unsuccessful IO statu in the
+        // mechanisms and manifested as an unsuccessful IO status in the
         // DNSClient callback.  Any problem here most likely means the request
         // is corrupt in some way and cannot be completed, therefore we will
         // log it and transition it to failure.
@@ -232,6 +232,83 @@ NameChangeTransaction::setUpdateAttempts(const size_t value) {
     update_attempts_ = value;
 }
 
+D2UpdateMessagePtr
+NameChangeTransaction::prepNewRequest(DdnsDomainPtr domain) {
+    if (!domain) {
+        isc_throw(NameChangeTransactionError,
+                  "prepNewRequest - domain cannot be null");
+    }
+
+    try {
+        // Create a "blank" update request.
+        D2UpdateMessagePtr request(new D2UpdateMessage(D2UpdateMessage::
+                                                       OUTBOUND));
+        // Construct the Zone Section.
+        dns::Name zone_name(domain->getName());
+        request->setZone(zone_name, dns::RRClass::IN());
+        return (request);
+    } catch (const std::exception& ex) {
+        isc_throw(NameChangeTransactionError, "Cannot create new request :"
+                  << ex.what());
+    }
+}
+
+void
+NameChangeTransaction::addLeaseAddressRdata(dns::RRsetPtr& rrset) {
+    if (!rrset) {
+        isc_throw(NameChangeTransactionError,
+                  "addLeaseAddressRdata - RRset cannot cannot be null");
+    }
+
+    try {
+        // Manufacture an RData from the lease address then add it to the RR.
+        if (ncr_->isV4()) {
+            dns::rdata::in::A a_rdata(ncr_->getIpAddress());
+            rrset->addRdata(a_rdata);
+        } else {
+            dns::rdata::in::AAAA rdata(ncr_->getIpAddress());
+            rrset->addRdata(rdata);
+        }
+    } catch (const std::exception& ex) {
+        isc_throw(NameChangeTransactionError, "Cannot add address rdata: "
+                  << ex.what());
+    }
+}
+
+void
+NameChangeTransaction::addDhcidRdata(dns::RRsetPtr& rrset) {
+    if (!rrset) {
+        isc_throw(NameChangeTransactionError,
+                  "addDhcidRdata - RRset cannot cannot be null");
+    }
+
+    try {
+        const std::vector<uint8_t>& ncr_dhcid = ncr_->getDhcid().getBytes();
+        util::InputBuffer buffer(ncr_dhcid.data(), ncr_dhcid.size());
+        dns::rdata::in::DHCID rdata(buffer, ncr_dhcid.size());
+        rrset->addRdata(rdata);
+    } catch (const std::exception& ex) {
+        isc_throw(NameChangeTransactionError, "Cannot add DCHID rdata: "
+                  << ex.what());
+    }
+}
+
+void
+NameChangeTransaction::addPtrRdata(dns::RRsetPtr& rrset) {
+    if (!rrset) {
+        isc_throw(NameChangeTransactionError,
+                  "addPtrRdata - RRset cannot cannot be null");
+    }
+
+    try {
+        dns::rdata::generic::PTR rdata(getNcr()->getFqdn());
+        rrset->addRdata(rdata);
+    } catch (const std::exception& ex) {
+        isc_throw(NameChangeTransactionError, "Cannot add PTR rdata: "
+                  << ex.what());
+    }
+}
+
 const dhcp_ddns::NameChangeRequestPtr&
 NameChangeTransaction::getNcr() const {
     return (ncr_);
@@ -333,5 +410,10 @@ NameChangeTransaction::getUpdateAttempts() const {
     return (update_attempts_);
 }
 
+const dns::RRType&
+NameChangeTransaction::getAddressRRType() const {
+    return (ncr_->isV4() ?  dns::RRType::A(): dns::RRType::AAAA());
+}
+
 } // namespace isc::d2
 } // namespace isc

+ 57 - 9
src/bin/d2/nc_trans.h

@@ -69,7 +69,7 @@ typedef isc::dhcp_ddns::D2Dhcid TransactionKey;
 /// single update to the server and returns the response, asynchronously,
 /// through a callback.  At each point in a transaction's state model, where
 /// an update is to be sent, the model "suspends" until notified by the
-/// DNSClient via the callbacka.  Suspension is done by posting a
+/// DNSClient via the callback.  Suspension is done by posting a
 /// StateModel::NOP_EVT as the next event, stopping the state model execution.
 ///
 /// Resuming state model execution when a DNS update completes is done by a
@@ -204,7 +204,7 @@ protected:
     /// currently selected server.  Since the send is asynchronous, the method
     /// posts NOP_EVT as the next event and then returns.
     ///
-    /// @param use_tsig True if the udpate should be include a TSIG key. This
+    /// @param use_tsig True if the update should be include a TSIG key. This
     /// is not yet implemented.
     ///
     /// If an exception occurs it will be logged and and the transaction will
@@ -215,7 +215,7 @@ protected:
     ///
     /// This method adds the events common to NCR transaction processing to
     /// the set of define events.  It invokes the superclass's implementation
-    /// first to maitain the hierarchical chain of event defintion.
+    /// first to maintain the hierarchical chain of event definition.
     /// Derivations of NameChangeTransaction must invoke its implementation
     /// in like fashion.
     ///
@@ -237,7 +237,7 @@ protected:
     ///
     /// This method adds the states common to NCR transaction processing to
     /// the dictionary of states.  It invokes the superclass's implementation
-    /// first to maitain the hierarchical chain of state defintion.
+    /// first to maintain the hierarchical chain of state definition.
     /// Derivations of NameChangeTransaction must invoke its implementation
     /// in like fashion.
     ///
@@ -261,7 +261,7 @@ protected:
     /// execution encounters a model violation:  attempt to call an unmapped
     /// state, an event not valid for the current state, or an uncaught
     /// exception thrown during a state handler invocation.  When such an
-    /// error occurs the transaction is deemed inoperable, and futher model
+    /// error occurs the transaction is deemed inoperable, and further model
     /// execution cannot be performed.  It marks the transaction as failed by
     /// setting the NCR status to dhcp_ddns::ST_FAILED
     ///
@@ -271,7 +271,7 @@ protected:
     /// @brief Determines the state and next event based on update attempts.
     ///
     /// This method will post a next event of SERVER_SELECTED_EVT to the
-    /// current state if the number of udpate attempts has not reached the
+    /// current state if the number of update attempts has not reached the
     /// maximum allowed.
     ///
     /// If the maximum number of attempts has been reached, it will transition
@@ -334,7 +334,7 @@ protected:
     ///
     /// This method is used to iterate over the list of servers.  If there are
     /// no more servers in the list, it returns false.  Otherwise it sets the
-    /// the current server to the next server and creates a new DNSClient
+    /// current server to the next server and creates a new DNSClient
     /// instance.
     ///
     /// @return True if a server has been selected, false if there are no more
@@ -364,6 +364,48 @@ protected:
         return (io_service_);
     }
 
+    /// @brief Creates a new DNS update request based on the given domain.
+    ///
+    /// Constructs a new "empty", OUTBOUND, request with the message id set
+    /// and zone section populated based on the given domain.
+    ///
+    /// @return A D2UpdateMessagePtr to the new request.
+    ///
+    /// @throw NameChangeTransactionError if request cannot be constructed.
+    D2UpdateMessagePtr prepNewRequest(DdnsDomainPtr domain);
+
+    /// @brief Adds an RData for the lease address to the given RRset.
+    ///
+    /// Creates an in::A() or in:AAAA() RData instance from the NCR
+    /// lease address and adds it to the given RRset.
+    ///
+    /// @param RRset RRset to which to add the RData
+    ///
+    /// @throw NameChangeTransactionError if RData cannot be constructed or
+    /// the RData cannot be added to the given RRset.
+    void addLeaseAddressRdata(dns::RRsetPtr& rrset);
+
+    /// @brief Adds an RData for the lease client's DHCID to the given RRset.
+    ///
+    /// Creates an in::DHCID() RData instance from the NCR DHCID and adds
+    /// it to the given RRset.
+    ///
+    /// @param RRset RRset to which to add the RData
+    ///
+    /// @throw NameChangeTransactionError if RData cannot be constructed or
+    /// the RData cannot be added to the given RRset.
+    void addDhcidRdata(dns::RRsetPtr& rrset);
+
+    /// @brief Adds an RData for the lease FQDN to the given RRset.
+    ///
+    /// Creates an in::PTR() RData instance from the NCR FQDN and adds
+    /// it to the given RRset.
+    ///
+    /// @param RRset RRset to which to add the RData
+    ///
+    /// @throw NameChangeTransactionError if RData cannot be constructed or
+    /// the RData cannot be added to the given RRset.
+    void addPtrRdata(dns::RRsetPtr& rrset);
 
 public:
     /// @brief Fetches the NameChangeRequest for this transaction.
@@ -392,13 +434,13 @@ public:
 
     /// @brief Fetches the forward DdnsDomain.
     ///
-    /// @return A pointer reference to the forward DdnsDomain.  If the
+    /// @return A pointer reference to the forward DdnsDomain.  If 
     /// the request does not include a forward change, the pointer will empty.
     DdnsDomainPtr& getForwardDomain();
 
     /// @brief Fetches the reverse DdnsDomain.
     ///
-    /// @return A pointer reference to the reverse DdnsDomain.  If the
+    /// @return A pointer reference to the reverse DdnsDomain.  If 
     /// the request does not include a reverse change, the pointer will empty.
     DdnsDomainPtr& getReverseDomain();
 
@@ -444,6 +486,12 @@ public:
     /// been attempted against the current server.
     size_t getUpdateAttempts() const;
 
+    /// @brief Returns the DHCP data type for the lease address
+    ///
+    /// @return constant reference to dns::RRType::A() if the lease address
+    /// is IPv4 or dns::RRType::AAAA() if the lease address is IPv6.
+    const dns::RRType& getAddressRRType() const;
+
 private:
     /// @brief The IOService which should be used to for IO processing.
     IOServicePtr io_service_;

+ 1 - 0
src/bin/d2/tests/Makefile.am

@@ -83,6 +83,7 @@ d2_unittests_SOURCES += d2_zone_unittests.cc
 d2_unittests_SOURCES += dns_client_unittests.cc
 d2_unittests_SOURCES += labeled_value_unittests.cc
 d2_unittests_SOURCES += nc_add_unittests.cc
+d2_unittests_SOURCES += nc_test_utils.cc nc_test_utils.h
 d2_unittests_SOURCES += nc_trans_unittests.cc
 d2_unittests_SOURCES += state_model_unittests.cc
 nodist_d2_unittests_SOURCES = ../d2_messages.h ../d2_messages.cc

+ 314 - 55
src/bin/d2/tests/nc_add_unittests.cc

@@ -12,7 +12,11 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <d2/d2_cfg_mgr.h>
+#include <d2/d2_cfg_mgr.h>
 #include <d2/nc_add.h>
+#include <dns/messagerenderer.h>
+#include <nc_test_utils.h>
 
 #include <boost/function.hpp>
 #include <boost/bind.hpp>
@@ -32,28 +36,77 @@ public:
                 dhcp_ddns::NameChangeRequestPtr& ncr,
                 DdnsDomainPtr& forward_domain,
                 DdnsDomainPtr& reverse_domain)
-        : NameAddTransaction(io_service, ncr, forward_domain, reverse_domain){
+        : NameAddTransaction(io_service, ncr, forward_domain, reverse_domain),
+          simulate_send_exception_(false) {
     }
 
     virtual ~NameAddStub() {
     }
 
     /// @brief Simulates sending update requests to the DNS server
-    /// Allows state handlers which conduct IO to be tested without a server.
+    ///
+    /// This method simulates the initiation of an asynchronous send of
+    /// a DNS update request. It overrides the actual sendUpdate method in
+    /// the base class, thus avoiding an actual send, yet still increments
+    /// the update attempt count and posts a next event of NOP_EVT.
+    ///
+    /// It will also simulate an exception-based failure of sendUpdate, if
+    /// the simulate_send_exception_ flag is true.
+    ///
+    /// @param use_tsig_ Parameter is unused, but present in the base class
+    /// method.
+    ///
     virtual void sendUpdate(bool /* use_tsig_ = false */) {
+        if (simulate_send_exception_) {
+            // Make the flag a one-shot by resetting it.
+            simulate_send_exception_ = false;
+            // Transition to failed.
+            transition(PROCESS_TRANS_FAILED_ST, UPDATE_FAILED_EVT);
+            return;
+        }
+
+        // Update send attempt count and post a NOP_EVT.
         setUpdateAttempts(getUpdateAttempts() + 1);
         postNextEvent(StateModel::NOP_EVT);
     }
 
+    /// @brief Simulates receiving a response
+    ///
+    /// This method simulates the completion of a DNSClient send.  This allows
+    /// the state handler logic devoted to dealing with IO completion to be
+    /// fully exercise without requiring any actual IO.  The two primary
+    /// pieces of information gleaned from IO completion are the DNSClient
+    /// status which indicates whether or not the IO exchange was successful
+    /// and the rcode, which indicates the server's reaction to the request.
+    ///
+    /// This method updates the transaction's DNS status value to that of the
+    /// given parameter, and then constructs and DNS update response message
+    /// with the given rcode value.  To complete the simulation it then posts
+    /// a next event of IO_COMPLETED_EVT.
+    ///
+    /// @param status simulated DNSClient status
+    /// @param rcode  simulated server response code
     void fakeResponse(const DNSClient::Status& status,
                       const dns::Rcode& rcode) {
-        D2UpdateMessagePtr msg(new D2UpdateMessage(D2UpdateMessage::OUTBOUND));
+        // Set the DNS update status.  This is normally set in
+        // DNSClient IO completion handler.
         setDnsUpdateStatus(status);
+
+        // Construct an empty message with the given Rcode.
+        D2UpdateMessagePtr msg(new D2UpdateMessage(D2UpdateMessage::OUTBOUND));
         msg->setRcode(rcode);
+
+        // Set the update response to the message.
         setDnsUpdateResponse(msg);
+
+        // Post the IO completion event.
         postNextEvent(NameChangeTransaction::IO_COMPLETED_EVT);
     }
 
+    /// @brief Selects the first forward server.
+    /// Some state handlers require a server to have been selected.
+    /// This selects a server without going through the state
+    /// transition(s) to do so.
     bool selectFwdServer() {
         if (getForwardDomain()) {
             initServerSelection(getForwardDomain());
@@ -64,6 +117,10 @@ public:
         return (false);
     }
 
+    /// @brief Selects the first reverse server.
+    /// Some state handlers require a server to have been selected.
+    /// This selects a server without going through the state
+    /// transition(s) to do so.
     bool selectRevServer() {
         if (getReverseDomain()) {
             initServerSelection(getReverseDomain());
@@ -74,6 +131,8 @@ public:
         return (false);
     }
 
+    /// @brief One-shot flag which will simulate sendUpdate failure if true.
+    bool simulate_send_exception_;
 
     using StateModel::postNextEvent;
     using StateModel::setState;
@@ -92,6 +151,9 @@ public:
     using NameAddTransaction::replacingRevPtrsHandler;
     using NameAddTransaction::processAddOkHandler;
     using NameAddTransaction::processAddFailedHandler;
+    using NameAddTransaction::buildAddFwdAddressRequest;
+    using NameAddTransaction::buildReplaceFwdAddressRequest;
+    using NameAddTransaction::buildReplaceRevPtrsRequest;
 };
 
 typedef boost::shared_ptr<NameAddStub> NameAddStubPtr;
@@ -116,20 +178,21 @@ public:
     virtual ~NameAddTransactionTest() {
     }
 
-    /// @brief  Instantiates a NameAddStub test transaction
-    /// The transaction is constructed around a predefined (i.e "canned")
-    /// NameChangeRequest. The request has both forward and reverse DNS
+    /// @brief Creates a transaction which requests an IPv4 DNS update.
+    ///
+    /// The transaction is constructed around a predefined (i.e. "canned")
+    /// IPv4 NameChangeRequest. The request has both forward and reverse DNS
     /// changes requested.  Based upon the change mask, the transaction
     /// will have either the forward, reverse, or both domains populated.
     ///
     /// @param change_mask determines which change directions are requested
-    NameAddStubPtr makeCannedTransaction(int change_mask=FWD_AND_REV_CHG) {
+    NameAddStubPtr makeTransaction4(int change_mask=FWD_AND_REV_CHG) {
         const char* msg_str =
             "{"
             " \"change_type\" : 0 , "
             " \"forward_change\" : true , "
             " \"reverse_change\" : true , "
-            " \"fqdn\" : \"example.com.\" , "
+            " \"fqdn\" : \"my.forward.example.com.\" , "
             " \"ip_address\" : \"192.168.2.1\" , "
             " \"dhcid\" : \"0102030405060708\" , "
             " \"lease_expires_on\" : \"20130121132405\" , "
@@ -141,53 +204,100 @@ public:
                                               fromJSON(msg_str);
 
         // If the change mask does not include a forward change clear the
-        // forward domain; otherise create the domain and its servers.
+        // forward domain; otherwise create the domain and its servers.
         if (!(change_mask & FORWARD_CHG)) {
             ncr->setForwardChange(false);
             forward_domain_.reset();
         } else {
             // Create the forward domain and then its servers.
-            DnsServerInfoStoragePtr servers(new DnsServerInfoStorage());
-            DnsServerInfoPtr server(new DnsServerInfo("forward.example.com",
-                                       isc::asiolink::IOAddress("1.1.1.1")));
-            servers->push_back(server);
-            server.reset(new DnsServerInfo("forward2.example.com",
-                                       isc::asiolink::IOAddress("1.1.1.2")));
-            servers->push_back(server);
-            forward_domain_.reset(new DdnsDomain("example.com.", "", servers));
+            forward_domain_ = makeDomain("example.com.");
+            addDomainServer(forward_domain_, "forward.example.com",
+                            "1.1.1.1");
+            addDomainServer(forward_domain_, "forward2.example.com",
+                            "1.1.1.2");
         }
 
         // If the change mask does not include a reverse change clear the
-        // reverse domain; otherise create the domain and its servers.
+        // reverse domain; otherwise create the domain and its servers.
         if (!(change_mask & REVERSE_CHG)) {
             ncr->setReverseChange(false);
             reverse_domain_.reset();
         } else {
             // Create the reverse domain and its server.
-            DnsServerInfoStoragePtr servers(new DnsServerInfoStorage());
-            DnsServerInfoPtr server(new DnsServerInfo("reverse.example.com",
-                                                      isc::asiolink::
-                                                      IOAddress("2.2.2.2")));
-            servers->push_back(server);
-            server.reset(new DnsServerInfo("reverse2.example.com",
-                                           isc::asiolink::
-                                           IOAddress("2.2.2.3")));
-            servers->push_back(server);
-            reverse_domain_.reset(new DdnsDomain("2.168.192.in.addr.arpa.",
-                                                 "", servers));
+            reverse_domain_ = makeDomain("2.168.192.in.addr.arpa.");
+            addDomainServer(reverse_domain_, "reverse.example.com",
+                            "2.2.2.2");
+            addDomainServer(reverse_domain_, "reverse2.example.com",
+                            "2.2.2.3");
         }
 
         // Now create the test transaction as would occur in update manager.
         return (NameAddStubPtr(new NameAddStub(io_service_, ncr,
-                                      forward_domain_, reverse_domain_)));
+                                               forward_domain_,
+                                               reverse_domain_)));
     }
 
+    /// @brief Creates a transaction which requests an IPv6 DNS update.
+    ///
+    /// The transaction is constructed around a predefined (i.e. "canned")
+    /// IPv6 NameChangeRequest. The request has both forward and reverse DNS
+    /// changes requested.  Based upon the change mask, the transaction
+    /// will have either the forward, reverse, or both domains populated.
+    ///
+    /// @param change_mask determines which change directions are requested
+    NameAddStubPtr makeTransaction6(int change_mask=FWD_AND_REV_CHG) {
+        const char* msg_str =
+            "{"
+            " \"change_type\" : 0 , "
+            " \"forward_change\" : true , "
+            " \"reverse_change\" : true , "
+            " \"fqdn\" : \"my6.forward.example.com.\" , "
+            " \"ip_address\" : \"2001:1::100\" , "
+            " \"dhcid\" : \"0102030405060708\" , "
+            " \"lease_expires_on\" : \"20130121132405\" , "
+            " \"lease_length\" : 1300 "
+            "}";
+
+        // Create NameChangeRequest from JSON string.
+        dhcp_ddns::NameChangeRequestPtr ncr = makeNcrFromString(msg_str);
+
+        // If the change mask does not include a forward change clear the
+        // forward domain; otherwise create the domain and its servers.
+        if (!(change_mask & FORWARD_CHG)) {
+            ncr->setForwardChange(false);
+            forward_domain_.reset();
+        } else {
+            // Create the forward domain and then its servers.
+            forward_domain_ = makeDomain("example.com.");
+            addDomainServer(forward_domain_, "fwd6-server.example.com",
+                            "2001:1::5");
+        }
+
+        // If the change mask does not include a reverse change clear the
+        // reverse domain; otherwise create the domain and its servers.
+        if (!(change_mask & REVERSE_CHG)) {
+            ncr->setReverseChange(false);
+            reverse_domain_.reset();
+        } else {
+            // Create the reverse domain and its server.
+            reverse_domain_ = makeDomain("1.2001.ip6.arpa.");
+            addDomainServer(reverse_domain_, "rev6-server.example.com",
+                            "2001:1::6");
+        }
+
+        // Now create the test transaction as would occur in update manager.
+        return (NameAddStubPtr(new NameAddStub(io_service_, ncr,
+                                               forward_domain_,
+                                               reverse_domain_)));
+    }
+
+
     /// @brief Create a test transaction at a known point in the state model.
     ///
     /// Method prepares a new test transaction and sets its state and next
     /// event values to those given.  This makes the transaction appear to
     /// be at that point in the state model without having to transition it
-    /// through prerequiste states.   It also provides the ability to set
+    /// through prerequisite states.   It also provides the ability to set
     /// which change directions are requested: forward change only, reverse
     /// change only, or both.
     ///
@@ -196,7 +306,7 @@ public:
     /// @param change_mask determines which change directions are requested
     NameAddStubPtr prepHandlerTest(unsigned int state, unsigned int event,
                                    unsigned int change_mask = FWD_AND_REV_CHG) {
-        NameAddStubPtr name_add = makeCannedTransaction(change_mask);
+        NameAddStubPtr name_add = makeTransaction4(change_mask);
         name_add->initDictionaries();
         name_add->postNextEvent(event);
         name_add->setState(state);
@@ -248,7 +358,7 @@ TEST(NameAddTransaction, construction) {
 /// @brief Tests event and state dictionary construction and verification.
 TEST_F(NameAddTransactionTest, dictionaryCheck) {
     NameAddStubPtr name_add;
-    ASSERT_NO_THROW(name_add = makeCannedTransaction());
+    ASSERT_NO_THROW(name_add = makeTransaction4());
     // Verify that the event and state dictionary validation fails prior
     // dictionary construction.
     ASSERT_THROW(name_add->verifyEvents(), StateModelError);
@@ -263,6 +373,63 @@ TEST_F(NameAddTransactionTest, dictionaryCheck) {
     ASSERT_NO_THROW(name_add->verifyStates());
 }
 
+/// @brief Tests construction of a DNS update request for adding a forward
+/// dns entry.
+TEST_F(NameAddTransactionTest, buildForwardAdd) {
+    // Create a IPv4 forward add transaction.
+    // Verify the request builds without error.
+    // and then verify the request contents.
+    NameAddStubPtr name_add;
+    ASSERT_NO_THROW(name_add = makeTransaction4());
+    ASSERT_NO_THROW(name_add->buildAddFwdAddressRequest());
+    checkForwardAddRequest(*name_add);
+
+    // Create a IPv6 forward add transaction.
+    // Verify the request builds without error.
+    // and then verify the request contents.
+    ASSERT_NO_THROW(name_add = makeTransaction6());
+    ASSERT_NO_THROW(name_add->buildAddFwdAddressRequest());
+    checkForwardAddRequest(*name_add);
+}
+
+/// @brief Tests construction of a DNS update request for replacing a forward
+/// dns entry.
+TEST_F(NameAddTransactionTest, buildReplaceFwdAddressRequest) {
+    // Create a IPv4 forward replace transaction.
+    // Verify the request builds without error.
+    // and then verify the request contents.
+    NameAddStubPtr name_add;
+    ASSERT_NO_THROW(name_add = makeTransaction4());
+    ASSERT_NO_THROW(name_add->buildReplaceFwdAddressRequest());
+    checkForwardReplaceRequest(*name_add);
+
+    // Create a IPv6 forward replace transaction.
+    // Verify the request builds without error.
+    // and then verify the request contents.
+    ASSERT_NO_THROW(name_add = makeTransaction6());
+    ASSERT_NO_THROW(name_add->buildReplaceFwdAddressRequest());
+    checkForwardReplaceRequest(*name_add);
+}
+
+/// @brief Tests the construction of a DNS update request for replacing a
+/// reverse dns entry.
+TEST_F(NameAddTransactionTest, buildReplaceRevPtrsRequest) {
+    // Create a IPv4 reverse replace transaction.
+    // Verify the request builds without error.
+    // and then verify the request contents.
+    NameAddStubPtr name_add;
+    ASSERT_NO_THROW(name_add = makeTransaction4());
+    ASSERT_NO_THROW(name_add->buildReplaceRevPtrsRequest());
+    checkReverseReplaceRequest(*name_add);
+
+    // Create a IPv6 reverse replace transaction.
+    // Verify the request builds without error.
+    // and then verify the request contents.
+    ASSERT_NO_THROW(name_add = makeTransaction6());
+    ASSERT_NO_THROW(name_add->buildReplaceRevPtrsRequest());
+    checkReverseReplaceRequest(*name_add);
+}
+
 // Tests the readyHandler functionality.
 // It verifies behavior for the following scenarios:
 //
@@ -270,7 +437,7 @@ TEST_F(NameAddTransactionTest, dictionaryCheck) {
 // 2. Posted event is START_EVT and request includes both a forward and a
 // reverse change
 // 3. Posted event is START_EVT and request includes only a reverse change
-// 3. Posted event is invalid
+// 4. Posted event is invalid
 //
 TEST_F(NameAddTransactionTest, readyHandler) {
     NameAddStubPtr name_add;
@@ -439,9 +606,8 @@ TEST_F(NameAddTransactionTest, addingFwdAddrsHandler_FwdOnlyAddOK) {
     // Run addingFwdAddrsHandler to construct and send the request.
     EXPECT_NO_THROW(name_add->addingFwdAddrsHandler());
 
-    // Verify that an update message was constructed.
-    update_msg = name_add->getDnsUpdateRequest();
-    EXPECT_TRUE(update_msg);
+    // Verify that an update message was constructed properly.
+    checkForwardAddRequest(*name_add);
 
     // Verify that we are still in this state and next event is NOP_EVT.
     // This indicates we "sent" the message and are waiting for IO completion.
@@ -450,7 +616,7 @@ TEST_F(NameAddTransactionTest, addingFwdAddrsHandler_FwdOnlyAddOK) {
     EXPECT_EQ(NameChangeTransaction::NOP_EVT,
               name_add->getNextEvent());
 
-    // Simulate receiving a succussful update response.
+    // Simulate receiving a successful update response.
     name_add->fakeResponse(DNSClient::SUCCESS, dns::Rcode::NOERROR());
 
     // Run addingFwdAddrsHandler again to process the response.
@@ -486,7 +652,7 @@ TEST_F(NameAddTransactionTest, addingFwdAddrsHandler_fwdAndRevAddOK) {
     // Run addingFwdAddrsHandler to construct and send the request.
     EXPECT_NO_THROW(name_add->addingFwdAddrsHandler());
 
-    // Simulate receiving a succussful update response.
+    // Simulate receiving a successful update response.
     name_add->fakeResponse(DNSClient::SUCCESS, dns::Rcode::NOERROR());
 
     // Run addingFwdAddrsHandler again  to process the response.
@@ -573,7 +739,7 @@ TEST_F(NameAddTransactionTest, addingFwdAddrsHandler_OtherRcode) {
     EXPECT_FALSE(name_add->getForwardChangeCompleted());
     EXPECT_FALSE(name_add->getReverseChangeCompleted());
 
-    // We should have failed the transaction. Verifiy that we transitioned
+    // We should have failed the transaction. Verify that we transitioned
     // correctly.
     EXPECT_EQ(NameChangeTransaction::PROCESS_TRANS_FAILED_ST,
               name_add->getCurrState());
@@ -680,7 +846,7 @@ TEST_F(NameAddTransactionTest, addingFwdAddrsHandler_InvalidResponse) {
         // Run addingFwdAddrsHandler to construct send the request.
         EXPECT_NO_THROW(name_add->addingFwdAddrsHandler());
 
-        // Simulate a server IO timeout.
+        // Simulate a corrupt server response.
         name_add->setDnsUpdateStatus(DNSClient::INVALID_RESPONSE);
         name_add->postNextEvent(NameChangeTransaction::IO_COMPLETED_EVT);
 
@@ -751,9 +917,8 @@ TEST_F(NameAddTransactionTest, replacingFwdAddrsHandler_FwdOnlyAddOK) {
     // Run replacingFwdAddrsHandler to construct and send the request.
     EXPECT_NO_THROW(name_add->replacingFwdAddrsHandler());
 
-    // Verify that an update message was constructed.
-    update_msg = name_add->getDnsUpdateRequest();
-    EXPECT_TRUE(update_msg);
+    // Verify that an update message was constructed properly.
+    checkForwardReplaceRequest(*name_add);
 
     // Verify that we are still in this state and next event is NOP_EVT.
     // This indicates we "sent" the message and are waiting for IO completion.
@@ -762,7 +927,7 @@ TEST_F(NameAddTransactionTest, replacingFwdAddrsHandler_FwdOnlyAddOK) {
     EXPECT_EQ(NameChangeTransaction::NOP_EVT,
               name_add->getNextEvent());
 
-    // Simulate receiving a succussful update response.
+    // Simulate receiving a successful update response.
     name_add->fakeResponse(DNSClient::SUCCESS, dns::Rcode::NOERROR());
 
     // Run replacingFwdAddrsHandler again to process the response.
@@ -798,7 +963,7 @@ TEST_F(NameAddTransactionTest, replacingFwdAddrsHandler_FwdOnlyAddOK2) {
     // Run replacingFwdAddrsHandler to construct and send the request.
     EXPECT_NO_THROW(name_add->replacingFwdAddrsHandler());
 
-    // Simulate receiving a succussful update response.
+    // Simulate receiving a successful update response.
     name_add->fakeResponse(DNSClient::SUCCESS, dns::Rcode::NOERROR());
 
     // Run replacingFwdAddrsHandler again to process the response.
@@ -834,7 +999,7 @@ TEST_F(NameAddTransactionTest, replacingFwdAddrsHandler_FwdAndRevAddOK) {
     // Run replacingFwdAddrsHandler to construct and send the request.
     EXPECT_NO_THROW(name_add->replacingFwdAddrsHandler());
 
-    // Simulate receiving a succussful update response.
+    // Simulate receiving a successful update response.
     name_add->fakeResponse(DNSClient::SUCCESS, dns::Rcode::NOERROR());
 
     // Run replacingFwdAddrsHandler again  to process the response.
@@ -853,14 +1018,14 @@ TEST_F(NameAddTransactionTest, replacingFwdAddrsHandler_FwdAndRevAddOK) {
 }
 
 
-// Tests addingFwdAddrsHandler with the following scenario:
+// Tests replacingFwdAddrsHandler with the following scenario:
 //
 //  The request includes a forward and reverse change.
 //  Initial posted event is FQDN_IN_USE_EVT.
 //  The update request is sent without error.
 //  A server response is received which indicates the FQDN is NOT in use.
 //
-TEST_F(NameAddTransactionTest, addingFwdAddrsHandler_FqdnNotInUse) {
+TEST_F(NameAddTransactionTest, replacingFwdAddrsHandler_FqdnNotInUse) {
     NameAddStubPtr name_add;
     // Create and prep a transaction, poised to run the handler.
     ASSERT_NO_THROW(name_add =
@@ -1038,7 +1203,7 @@ TEST_F(NameAddTransactionTest, replacingFwdAddrsHandler_CorruptResponse) {
             EXPECT_TRUE(prev_msg == curr_msg);
         }
 
-        // Simulate a server corrupt response.
+        // Simulate a corrupt server response.
         name_add->setDnsUpdateStatus(DNSClient::INVALID_RESPONSE);
         name_add->postNextEvent(NameChangeTransaction::IO_COMPLETED_EVT);
 
@@ -1175,9 +1340,8 @@ TEST_F(NameAddTransactionTest, replacingRevPtrsHandler_FwdOnlyAddOK) {
     // Run replacingRevPtrsHandler to construct and send the request.
     EXPECT_NO_THROW(name_add->replacingRevPtrsHandler());
 
-    // Verify that an update message was constructed.
-    update_msg = name_add->getDnsUpdateRequest();
-    EXPECT_TRUE(update_msg);
+    // Verify that an update message was constructed properly.
+    checkReverseReplaceRequest(*name_add);
 
     // Verify that we are still in this state and next event is NOP_EVT.
     // This indicates we "sent" the message and are waiting for IO completion.
@@ -1186,7 +1350,7 @@ TEST_F(NameAddTransactionTest, replacingRevPtrsHandler_FwdOnlyAddOK) {
     EXPECT_EQ(NameChangeTransaction::NOP_EVT,
               name_add->getNextEvent());
 
-    // Simulate receiving a succussful update response.
+    // Simulate receiving a successful update response.
     name_add->fakeResponse(DNSClient::SUCCESS, dns::Rcode::NOERROR());
 
     // Run replacingRevPtrsHandler again to process the response.
@@ -1237,7 +1401,7 @@ TEST_F(NameAddTransactionTest, replacingRevPtrsHandler_OtherRcode) {
     EXPECT_FALSE(name_add->getForwardChangeCompleted());
     EXPECT_FALSE(name_add->getReverseChangeCompleted());
 
-    // We should have failed the transaction. Verifiy that we transitioned
+    // We should have failed the transaction. Verify that we transitioned
     // correctly.
     EXPECT_EQ(NameChangeTransaction::PROCESS_TRANS_FAILED_ST,
               name_add->getCurrState());
@@ -1445,5 +1609,100 @@ TEST_F(NameAddTransactionTest, processAddFailedHandler) {
     EXPECT_THROW(name_add->processAddFailedHandler(), NameAddTransactionError);
 }
 
+// Tests addingFwdAddrsHandler with the following scenario:
+//
+//  The request includes only a forward change.
+//  Initial posted event is SERVER_SELECTED_EVT.
+//  The send update request fails due to an unexpected exception.
+//
+TEST_F(NameAddTransactionTest, addingFwdAddrsHandler_sendUpdateException) {
+    NameAddStubPtr name_add;
+    // Create and prep a transaction, poised to run the handler.
+    ASSERT_NO_THROW(name_add =
+                    prepHandlerTest(NameAddTransaction::ADDING_FWD_ADDRS_ST,
+                                    NameChangeTransaction::
+                                    SERVER_SELECTED_EVT, FORWARD_CHG));
+
+    name_add->simulate_send_exception_ = true;
+
+    // Run replacingFwdAddrsHandler to construct and send the request.
+    EXPECT_NO_THROW(name_add->addingFwdAddrsHandler());
+
+    // Completion flags should be false.
+    EXPECT_FALSE(name_add->getForwardChangeCompleted());
+    EXPECT_FALSE(name_add->getReverseChangeCompleted());
+
+    // Since IO exceptions should be gracefully handled, any that occur
+    // are unanticipated, and deemed unrecoverable, so the transaction should
+    // be transitioned to failure.
+    EXPECT_EQ(NameChangeTransaction::PROCESS_TRANS_FAILED_ST,
+              name_add->getCurrState());
+    EXPECT_EQ(NameChangeTransaction::UPDATE_FAILED_EVT,
+              name_add->getNextEvent());
+}
+
+// Tests replacingFwdAddrsHandler with the following scenario:
+//
+//  The request includes only a forward change.
+//  Initial posted event is SERVER_SELECTED_EVT.
+//  The send update request fails due to an unexpected exception.
+//
+TEST_F(NameAddTransactionTest, replacingFwdAddrsHandler_SendUpdateException) {
+    NameAddStubPtr name_add;
+    // Create and prep a transaction, poised to run the handler.
+    ASSERT_NO_THROW(name_add =
+                    prepHandlerTest(NameAddTransaction::REPLACING_FWD_ADDRS_ST,
+                                    NameChangeTransaction::
+                                    SERVER_SELECTED_EVT, FORWARD_CHG));
+
+    name_add->simulate_send_exception_ = true;
+
+    // Run replacingFwdAddrsHandler to construct and send the request.
+    EXPECT_NO_THROW(name_add->replacingFwdAddrsHandler());
+
+    // Completion flags should be false.
+    EXPECT_FALSE(name_add->getForwardChangeCompleted());
+    EXPECT_FALSE(name_add->getReverseChangeCompleted());
+
+    // Since IO exceptions should be gracefully handled, any that occur
+    // are unanticipated, and deemed unrecoverable, so the transaction should
+    // be transitioned to failure.
+    EXPECT_EQ(NameChangeTransaction::PROCESS_TRANS_FAILED_ST,
+              name_add->getCurrState());
+    EXPECT_EQ(NameChangeTransaction::UPDATE_FAILED_EVT,
+              name_add->getNextEvent());
+}
+
+// Tests replacingRevPtrHandler with the following scenario:
+//
+//  The request includes only a reverse change.
+//  Initial posted event is SERVER_SELECTED_EVT.
+//  The send update request fails due to an unexpected exception.
+//
+TEST_F(NameAddTransactionTest, replacingRevPtrsHandler_SendUpdateException) {
+    NameAddStubPtr name_add;
+    // Create and prep a transaction, poised to run the handler.
+    ASSERT_NO_THROW(name_add =
+                    prepHandlerTest(NameAddTransaction::REPLACING_REV_PTRS_ST,
+                                    NameChangeTransaction::
+                                    SERVER_SELECTED_EVT, REVERSE_CHG));
+
+    name_add->simulate_send_exception_ = true;
+
+    // Run replacingRevPtrsHandler to construct and send the request.
+    EXPECT_NO_THROW(name_add->replacingRevPtrsHandler());
+
+    // Completion flags should be false.
+    EXPECT_FALSE(name_add->getForwardChangeCompleted());
+    EXPECT_FALSE(name_add->getReverseChangeCompleted());
+
+    // Since IO exceptions should be gracefully handled, any that occur
+    // are unanticipated, and deemed unrecoverable, so the transaction should
+    // be transitioned to failure.
+    EXPECT_EQ(NameChangeTransaction::PROCESS_TRANS_FAILED_ST,
+              name_add->getCurrState());
+    EXPECT_EQ(NameChangeTransaction::UPDATE_FAILED_EVT,
+              name_add->getNextEvent());
+}
 
 }

+ 394 - 0
src/bin/d2/tests/nc_test_utils.cc

@@ -0,0 +1,394 @@
+// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <d2/d2_cfg_mgr.h>
+#include <dns/opcode.h>
+#include <dns/messagerenderer.h>
+#include <nc_test_utils.h>
+
+#include <gtest/gtest.h>
+
+using namespace std;
+using namespace isc;
+using namespace isc::d2;
+
+namespace isc {
+namespace d2 {
+
+const char* TEST_DNS_SERVER_IP = "127.0.0.1";
+size_t TEST_DNS_SERVER_PORT = 5301;
+
+FauxServer::FauxServer(asiolink::IOService& io_service,
+                       asiolink::IOAddress& address, size_t port)
+    :io_service_(io_service), address_(address), port_(port),
+     server_socket_() {
+    server_socket_.reset(new asio::ip::udp::socket(io_service_.get_io_service(),
+                                                   asio::ip::udp::v4()));
+    server_socket_->set_option(asio::socket_base::reuse_address(true));
+    server_socket_->bind(asio::ip::udp::endpoint(address_.getAddress(), port_));
+}
+
+FauxServer::FauxServer(asiolink::IOService& io_service,
+                       DnsServerInfo& server)
+    :io_service_(io_service), address_(server.getIpAddress()),
+     port_(server.getPort()), server_socket_() {
+    server_socket_.reset(new asio::ip::udp::socket(io_service_.get_io_service(),
+                                                   asio::ip::udp::v4()));
+    server_socket_->set_option(asio::socket_base::reuse_address(true));
+    server_socket_->bind(asio::ip::udp::endpoint(address_.getAddress(), port_));
+}
+
+
+FauxServer::~FauxServer() {
+}
+
+void
+FauxServer::receive (const ResponseMode& response_mode,
+                     const dns::Rcode& response_rcode) {
+    server_socket_->async_receive_from(asio::buffer(receive_buffer_,
+                                                    sizeof(receive_buffer_)),
+                                       remote_,
+                                       boost::bind(&FauxServer::requestHandler,
+                                                   this, _1, _2,
+                                                   response_mode,
+                                                   response_rcode));
+}
+
+void
+FauxServer::requestHandler(const asio::error_code& error,
+                           std::size_t bytes_recvd,
+                           const ResponseMode& response_mode,
+                           const dns::Rcode& response_rcode) {
+    // If we encountered an error or received no data then fail.
+    // We expect the client to send good requests.
+    if (error.value() != 0 || bytes_recvd < 1) {
+        ADD_FAILURE() << "FauxServer receive failed" << error.message();
+        return;
+    }
+
+    // We have a successfully received data. We need to turn it into
+    // a request in order to build a proper response.
+    // Note D2UpdateMessage is geared towards making requests and
+    // reading responses.  This is the opposite perspective so we have
+    // to a bit of roll-your-own here.
+    dns::Message request(dns::Message::PARSE);
+    util::InputBuffer request_buf(receive_buffer_, bytes_recvd);
+    try {
+        request.fromWire(request_buf);
+    } catch (const std::exception& ex) {
+        // If the request cannot be parsed, then fail the test.
+        // We expect the client to send good requests.
+        ADD_FAILURE() << "FauxServer request is corrupt:" << ex.what();
+        return;
+    }
+
+    // The request parsed ok, so let's build a response.
+    // We must use the QID we received in the response or IOFetch will
+    // toss the response out, resulting in eventual timeout.
+    // We fill in the zone with data we know is from the "server".
+    dns::Message response(dns::Message::RENDER);
+    response.setQid(request.getQid());
+    dns::Question question(dns::Name("response.example.com"),
+                           dns::RRClass::ANY(), dns::RRType::SOA());
+    response.addQuestion(question);
+    response.setOpcode(dns::Opcode(dns::Opcode::UPDATE_CODE));
+    response.setHeaderFlag(dns::Message::HEADERFLAG_QR, true);
+
+    // Set the response Rcode to value passed in, default is NOERROR.
+    response.setRcode(response_rcode);
+
+    // Render the response to a buffer.
+    dns::MessageRenderer renderer;
+    util::OutputBuffer response_buf(TEST_MSG_MAX);
+    renderer.setBuffer(&response_buf);
+    response.toWire(renderer);
+
+    // If mode is to ship garbage, then stomp on part of the rendered
+    // message.
+    if (response_mode == CORRUPT_RESP) {
+        response_buf.writeUint16At(0xFFFF, 2);
+    }
+
+    // Ship the reponse via synchronous send.
+    try {
+        int cnt = server_socket_->send_to(asio::
+                                          buffer(response_buf.getData(),
+                                                 response_buf.getLength()),
+                                          remote_);
+        // Make sure we sent what we expect to send.
+        if (cnt != response_buf.getLength()) {
+            ADD_FAILURE() << "FauxServer sent: " << cnt << " expected: "
+                          << response_buf.getLength();
+        }
+    } catch (const std::exception& ex) {
+        ADD_FAILURE() << "FauxServer send failed: " << ex.what();
+    }
+}
+
+
+void
+checkRRCount(const D2UpdateMessagePtr& request,
+             D2UpdateMessage::UpdateMsgSection section, int count) {
+    dns::RRsetIterator rrset_it = request->beginSection(section);
+    dns::RRsetIterator rrset_end = request->endSection(section);
+
+    ASSERT_EQ(count, std::distance(rrset_it, rrset_end));
+}
+
+void
+checkZone(const D2UpdateMessagePtr& request, const std::string& exp_zone_name) {
+    // Verify the zone section info.
+    D2ZonePtr zone = request->getZone();
+    EXPECT_TRUE(zone);
+    EXPECT_EQ(exp_zone_name, zone->getName().toText());
+    EXPECT_EQ(dns::RRClass::IN().getCode(), zone->getClass().getCode());
+}
+
+void
+checkRR(dns::RRsetPtr rrset, const std::string& exp_name,
+              const dns::RRClass& exp_class, const dns::RRType& exp_type,
+              unsigned int exp_ttl, dhcp_ddns::NameChangeRequestPtr ncr) {
+    // Verify the FQDN/DHCID RR fields.
+    EXPECT_EQ(exp_name, rrset->getName().toText());
+    EXPECT_EQ(exp_class.getCode(), rrset->getClass().getCode());
+    EXPECT_EQ(exp_type.getCode(), rrset->getType().getCode());
+    EXPECT_EQ(exp_ttl, rrset->getTTL().getValue());
+    if (exp_type == dns::RRType::ANY() || exp_class == dns::RRClass::ANY()) {
+        // ANY types do not have RData
+        ASSERT_EQ(0, rrset->getRdataCount());
+        return;
+    }
+
+    ASSERT_EQ(1, rrset->getRdataCount());
+    dns::RdataIteratorPtr rdata_it = rrset->getRdataIterator();
+    ASSERT_TRUE(rdata_it);
+
+    if ((exp_type == dns::RRType::A()) ||
+        (exp_type == dns::RRType::AAAA())) {
+        // should have lease rdata
+        EXPECT_EQ(ncr->getIpAddress(), rdata_it->getCurrent().toText());
+    } else if (exp_type == dns::RRType::PTR())  {
+        // should have PTR rdata
+        EXPECT_EQ(ncr->getFqdn(), rdata_it->getCurrent().toText());
+    } else if (exp_type == dns::RRType::DHCID()) {
+        // should have DHCID rdata
+        const std::vector<uint8_t>& ncr_dhcid = ncr->getDhcid().getBytes();
+        util::InputBuffer buffer(ncr_dhcid.data(), ncr_dhcid.size());
+        dns::rdata::in::DHCID rdata_ref(buffer, ncr_dhcid.size());
+        EXPECT_EQ(0, rdata_ref.compare(rdata_it->getCurrent()));
+    } else {
+        // we got a problem
+        FAIL();
+    }
+}
+
+dns::RRsetPtr
+getRRFromSection(const D2UpdateMessagePtr& request,
+                 D2UpdateMessage::UpdateMsgSection section, int index) {
+    dns::RRsetIterator rrset_it = request->beginSection(section);
+    dns::RRsetIterator rrset_end = request->endSection(section);
+
+    if (std::distance(rrset_it, rrset_end) <= index) {
+        // Return an empty pointer if index is out of range.
+        return (dns::RRsetPtr());
+    }
+
+    std::advance(rrset_it, index);
+    return (*rrset_it);
+}
+
+dhcp_ddns::NameChangeRequestPtr makeNcrFromString(const std::string& ncr_str) {
+    return (dhcp_ddns::NameChangeRequest::fromJSON(ncr_str));
+}
+
+DdnsDomainPtr makeDomain(const std::string& zone_name,
+                         const std::string& key_name) {
+    DdnsDomainPtr domain;
+    DnsServerInfoStoragePtr servers(new DnsServerInfoStorage());
+    domain.reset(new DdnsDomain(zone_name, key_name, servers));
+    return (domain);
+}
+
+void addDomainServer(DdnsDomainPtr& domain, const std::string& name,
+                     const std::string& ip, const size_t port) {
+    DnsServerInfoPtr server(new DnsServerInfo(name, asiolink::IOAddress(ip),
+                                              port));
+    domain->getServers()->push_back(server);
+}
+
+// Verifies that the contents of the given transaction's  DNS update request
+// is correct for adding a forward DNS entry
+void checkForwardAddRequest(NameChangeTransaction& tran) {
+    const D2UpdateMessagePtr& request = tran.getDnsUpdateRequest();
+    ASSERT_TRUE(request);
+
+    // Safety check.
+    dhcp_ddns::NameChangeRequestPtr ncr = tran.getNcr();
+    ASSERT_TRUE(ncr);
+
+    std::string exp_zone_name = tran.getForwardDomain()->getName();
+    std::string exp_fqdn = ncr->getFqdn();
+    const dns::RRType& exp_ip_rr_type = tran.getAddressRRType();
+
+    // Verify the zone section.
+    checkZone(request, exp_zone_name);
+
+    // Verify the PREREQUISITE SECTION
+    // Should be 1 which tests for FQDN does not exist.
+    dns::RRsetPtr rrset;
+    checkRRCount(request, D2UpdateMessage::SECTION_PREREQUISITE, 1);
+    ASSERT_TRUE(rrset = getRRFromSection(request, D2UpdateMessage::
+                                                  SECTION_PREREQUISITE, 0));
+    checkRR(rrset, exp_fqdn, dns::RRClass::NONE(), dns::RRType::ANY(), 0, ncr);
+
+    // Verify the UPDATE SECTION
+    // Should be 2 RRs: 1 to add the FQDN/IP and one to add the DHCID RR
+    checkRRCount(request, D2UpdateMessage::SECTION_UPDATE, 2);
+
+    // First, Verify the FQDN/IP add RR.
+    ASSERT_TRUE(rrset = getRRFromSection(request, D2UpdateMessage::
+                                                  SECTION_UPDATE, 0));
+    checkRR(rrset, exp_fqdn, dns::RRClass::IN(), exp_ip_rr_type, 0, ncr);
+
+    // Now, verify the DHCID add RR.
+    ASSERT_TRUE(rrset = getRRFromSection(request, D2UpdateMessage::
+                                                  SECTION_UPDATE, 1));
+    checkRR(rrset, exp_fqdn, dns::RRClass::IN(), dns::RRType::DHCID(), 0, ncr);
+
+    // Verify there are no RRs in the ADDITIONAL Section.
+    checkRRCount(request, D2UpdateMessage::SECTION_ADDITIONAL, 0);
+
+    // Verify that it will render toWire without throwing.
+    dns::MessageRenderer renderer;
+    ASSERT_NO_THROW(request->toWire(renderer));
+}
+
+// Verifies that the contents of the given transaction's  DNS update request
+// is correct for replacing a forward DNS entry
+void checkForwardReplaceRequest(NameChangeTransaction& tran) {
+    const D2UpdateMessagePtr& request = tran.getDnsUpdateRequest();
+    ASSERT_TRUE(request);
+
+    // Safety check.
+    dhcp_ddns::NameChangeRequestPtr ncr = tran.getNcr();
+    ASSERT_TRUE(ncr);
+
+    std::string exp_zone_name = tran.getForwardDomain()->getName();
+    std::string exp_fqdn = ncr->getFqdn();
+    const dns::RRType& exp_ip_rr_type = tran.getAddressRRType();
+
+    // Verify the zone section.
+    checkZone(request, exp_zone_name);
+
+    // Verify the PREREQUISITE SECTION
+    // Should be 2,  1 which tests for FQDN does exist and the other
+    // checks for a matching DHCID.
+    dns::RRsetPtr rrset;
+    checkRRCount(request, D2UpdateMessage::SECTION_PREREQUISITE, 2);
+
+    // Verify the FQDN test RR.
+    ASSERT_TRUE(rrset = getRRFromSection(request, D2UpdateMessage::
+                                                  SECTION_PREREQUISITE, 0));
+    checkRR(rrset, exp_fqdn, dns::RRClass::ANY(), dns::RRType::ANY(), 0, ncr);
+
+    // Verify the DHCID test RR.
+    ASSERT_TRUE(rrset = getRRFromSection(request, D2UpdateMessage::
+                                                  SECTION_PREREQUISITE, 1));
+    checkRR(rrset, exp_fqdn, dns::RRClass::IN(), dns::RRType::DHCID(), 0, ncr);
+
+    // Verify the UPDATE SECTION
+    // Should be 2,  1 which deletes the existing FQDN/IP and one that
+    // adds the new one.
+    checkRRCount(request, D2UpdateMessage::SECTION_UPDATE, 2);
+
+    // Verify the FQDN delete RR.
+    ASSERT_TRUE(rrset = getRRFromSection(request, D2UpdateMessage::
+                                                  SECTION_UPDATE, 0));
+    checkRR(rrset, exp_fqdn, dns::RRClass::ANY(), exp_ip_rr_type, 0, ncr);
+
+    // Verify the FQDN/IP add RR.
+    ASSERT_TRUE(rrset = getRRFromSection(request, D2UpdateMessage::
+                                                  SECTION_UPDATE, 1));
+    checkRR(rrset, exp_fqdn, dns::RRClass::IN(), exp_ip_rr_type, 0, ncr);
+
+    // Verify there are no RRs in the ADDITIONAL Section.
+    checkRRCount(request, D2UpdateMessage::SECTION_ADDITIONAL, 0);
+
+    // Verify that it will render toWire without throwing.
+    dns::MessageRenderer renderer;
+    ASSERT_NO_THROW(request->toWire(renderer));
+}
+
+// Verifies that the contents of the given transaction's  DNS update request
+// is correct for replacing a reverse DNS entry
+void checkReverseReplaceRequest(NameChangeTransaction& tran) {
+    const D2UpdateMessagePtr& request = tran.getDnsUpdateRequest();
+    ASSERT_TRUE(request);
+
+    // Safety check.
+    dhcp_ddns::NameChangeRequestPtr ncr = tran.getNcr();
+    ASSERT_TRUE(ncr);
+
+    std::string exp_zone_name = tran.getReverseDomain()->getName();
+    std::string exp_rev_addr = D2CfgMgr::reverseIpAddress(ncr->getIpAddress());
+
+    // Verify the zone section.
+    checkZone(request, exp_zone_name);
+
+    // Verify there are no RRs in the PREREQUISITE Section.
+    checkRRCount(request, D2UpdateMessage::SECTION_PREREQUISITE, 0);
+
+    // Verify the UPDATE Section.
+    // It should contain 4 RRs:
+    // 1. A delete all PTR RRs for the given IP
+    // 2. A delete of all DHCID RRs for the given IP
+    // 3. An add of the new PTR RR
+    // 4. An add of the new DHCID RR
+    dns::RRsetPtr rrset;
+    checkRRCount(request, D2UpdateMessage::SECTION_UPDATE, 4);
+
+    // Verify the PTR delete RR.
+    ASSERT_TRUE(rrset = getRRFromSection(request, D2UpdateMessage::
+                                                  SECTION_UPDATE, 0));
+    checkRR(rrset, exp_rev_addr, dns::RRClass::ANY(), dns::RRType::PTR(),
+            0, ncr);
+
+    // Verify the DHCID delete RR.
+    ASSERT_TRUE(rrset = getRRFromSection(request, D2UpdateMessage::
+                                                  SECTION_UPDATE, 1));
+    checkRR(rrset, exp_rev_addr, dns::RRClass::ANY(), dns::RRType::DHCID(),
+            0, ncr);
+
+    // Verify the PTR add RR.
+    ASSERT_TRUE(rrset = getRRFromSection(request, D2UpdateMessage::
+                                                  SECTION_UPDATE, 2));
+    checkRR(rrset, exp_rev_addr, dns::RRClass::IN(), dns::RRType::PTR(),
+            0, ncr);
+
+    // Verify the DHCID add RR.
+    ASSERT_TRUE(rrset = getRRFromSection(request, D2UpdateMessage::
+                                                  SECTION_UPDATE, 3));
+    checkRR(rrset, exp_rev_addr, dns::RRClass::IN(), dns::RRType::DHCID(),
+            0, ncr);
+
+    // Verify there are no RRs in the ADDITIONAL Section.
+    checkRRCount(request, D2UpdateMessage::SECTION_ADDITIONAL, 0);
+
+    // Verify that it will render toWire without throwing.
+    dns::MessageRenderer renderer;
+    ASSERT_NO_THROW(request->toWire(renderer));
+}
+
+}; // namespace isc::d2
+}; // namespace isc

+ 221 - 0
src/bin/d2/tests/nc_test_utils.h

@@ -0,0 +1,221 @@
+// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef NC_TEST_UTILS_H
+#define NC_TEST_UTILS_H
+
+/// @file nc_test_utils.h prototypes for functions related transaction testing.
+
+#include <d2/nc_trans.h>
+
+#include <asio/ip/udp.hpp>
+#include <asio/socket_base.hpp>
+
+namespace isc {
+namespace d2 {
+
+extern const char* TEST_DNS_SERVER_IP;
+extern size_t TEST_DNS_SERVER_PORT;
+
+// Not extern'ed to allow use as array size
+const int TEST_MSG_MAX = 1024;
+
+typedef boost::shared_ptr<asio::ip::udp::socket> SocketPtr;
+
+/// @brief This class simulates a DNS server.  It is capable of performing
+/// an asynchronous read, governed by an IOService, and responding to received
+/// requests in a given manner.
+class FauxServer {
+public:
+    enum  ResponseMode {
+        USE_RCODE,   // Generate a response with a given RCODE
+        CORRUPT_RESP  // Generate a corrupt response
+    };
+
+    asiolink::IOService& io_service_;
+    const asiolink::IOAddress& address_;
+    size_t port_;
+    SocketPtr server_socket_;
+    asio::ip::udp::endpoint remote_;
+    uint8_t receive_buffer_[TEST_MSG_MAX];
+
+    /// @brief Constructor
+    ///
+    /// @param io_service IOService to be used for socket IO.
+    /// @param address  IP address at which the server should listen.
+    /// @param port Port number at which the server should listen.
+    FauxServer(asiolink::IOService& io_service, asiolink::IOAddress& address,
+               size_t port);
+
+    /// @brief Constructor
+    ///
+    /// @param io_service IOService to be used for socket IO.
+    /// @param server DnServerInfo of server the DNS server. This supplies the
+    /// server's ip address and port.
+    FauxServer(asiolink::IOService& io_service, DnsServerInfo& server);
+
+    /// @brief Destructor
+    virtual ~FauxServer();
+
+    /// @brief Initiates an asynchronous receive
+    ///
+    /// Starts the server listening for requests.  Upon completion of the
+    /// listen, the callback method, requestHandler, is invoked.
+    ///
+    /// @param response_mode Selects how the server responds to a request
+    /// @param response_rcode The Rcode value set in the response. Not used
+    /// for all modes.
+    void receive (const ResponseMode& response_mode,
+                  const dns::Rcode& response_rcode=dns::Rcode::NOERROR());
+
+    /// @brief Socket IO Completion callback
+    ///
+    /// This method servers as the Server's UDP socket receive callback handler.
+    /// When the receive completes the handler is invoked with the
+    /// @param error result code of the receive (determined by asio layer)
+    /// @param bytes_recvd number of bytes received, if any
+    /// @param response_mode type of response the handler should produce
+    /// @param response_rcode value of Rcode in the response constructed by
+    /// handler
+    void requestHandler(const asio::error_code& error,
+                        std::size_t bytes_recvd,
+                        const ResponseMode& response_mode,
+                        const dns::Rcode& response_rcode);
+};
+
+/// @brief Tests the number of RRs in a request section against a given count.
+///
+/// This function actually returns the number of RRsetPtrs in a section. Since
+/// D2 only uses RRsets with a single RData in each (i.e. 1 RR), it is used
+/// as the number of RRs.  The dns::Message::getRRCount() cannot be used for
+/// this as it returns the number of RDatas in an RRSet which does NOT equate
+/// to the number of RRs. RRs with no RData, those with class or type of ANY,
+/// are not counted.
+///
+/// @param request DNS update request to test
+/// @param section enum value of the section to count
+/// @param count the expected number of RRs
+extern void checkRRCount(const D2UpdateMessagePtr& request,
+                         D2UpdateMessage::UpdateMsgSection section, int count);
+
+/// @brief Tests the zone content of a given request.
+///
+/// @param request DNS update request to validate
+/// @param exp_zone_name expected value of the zone name in the zone section
+extern void checkZone(const D2UpdateMessagePtr& request,
+                      const std::string& exp_zone_name);
+
+/// @brief Tests the contents of an RRset
+///
+/// @param rrset Pointer the RRset to test
+/// @param exp_name expected value of RRset name (FQDN or reverse ip)
+/// @param exp_class expected RRClass value of RRset
+/// @param exp_typ expected RRType value of RRset
+/// @param exp_ttl  expected TTL value of RRset
+/// @param ncr NameChangeRequest on which the RRset is based
+extern void checkRR(dns::RRsetPtr rrset, const std::string& exp_name,
+                    const dns::RRClass& exp_class, const dns::RRType& exp_type,
+                    unsigned int exp_ttl, dhcp_ddns::NameChangeRequestPtr ncr);
+
+/// @brief Fetches an RR(set) from a given section of a request
+///
+/// @param request DNS update request from which the RR should come
+/// @param section enum value of the section from which the RR should come
+/// @param index zero-based index of the RR of interest.
+///
+/// @return Pointer to the RR of interest, empty pointer if the index is out
+/// of range.
+extern dns::RRsetPtr getRRFromSection(const D2UpdateMessagePtr& request,
+                                      D2UpdateMessage::UpdateMsgSection section,
+                                      int index);
+/// @brief Creates a NameChangeRequest from a JSON string
+///
+/// @param ncr_str JSON string form of a NameChangeRequest.  Example:
+/// @code
+///        const char* msg_str =
+///            "{"
+///            " \"change_type\" : 0 , "
+///            " \"forward_change\" : true , "
+///            " \"reverse_change\" : true , "
+///            " \"fqdn\" : \"my.example.com.\" , "
+///            " \"ip_address\" : \"192.168.2.1\" , "
+///            " \"dhcid\" : \"0102030405060708\" , "
+///            " \"lease_expires_on\" : \"20130121132405\" , "
+///            " \"lease_length\" : 1300 "
+///            "}";
+///
+/// @endcode
+
+/// @brief Verifies a forward mapping addition DNS update request
+///
+/// Tests that the DNS Update request for a given transaction, is correct for
+/// adding a forward DNS mapping.
+///
+/// @param tran Transaction containing the request to be verified.
+extern void checkForwardAddRequest(NameChangeTransaction& tran);
+
+/// @brief Verifies a forward mapping replacement DNS update request
+///
+/// Tests that the DNS Update request for a given transaction, is correct for
+/// replacing a forward DNS mapping.
+///
+/// @param tran Transaction containing the request to be verified.
+extern void checkForwardReplaceRequest(NameChangeTransaction& tran);
+
+/// @brief Verifies a reverse mapping replacement DNS update request
+///
+/// Tests that the DNS Update request for a given transaction, is correct for
+/// replacing a reverse DNS mapping.
+///
+/// @param tran Transaction containing the request to be verified.
+extern void checkReverseReplaceRequest(NameChangeTransaction& tran);
+
+/// @brief Creates a NameChangeRequest from JSON string.
+///
+/// @param ncr_str string of JSON text from which to make the request.
+///
+/// @return Pointer to newly created request.
+///
+/// @throw Underlying methods may throw.
+extern
+dhcp_ddns::NameChangeRequestPtr makeNcrFromString(const std::string& ncr_str);
+
+/// @brief Creates a DdnsDomain with the one server.
+///
+/// @param zone_name zone name of the domain
+/// @param key_name TSIG key name of the TSIG key for this domain
+///
+/// @throw Underlying methods may throw.
+extern DdnsDomainPtr makeDomain(const std::string& zone_name,
+                                const std::string& key_name = "");
+
+/// @brief Creates a DnsServerInfo and adds it to the given DdnsDomain.
+///
+/// The server is created and added to the domain, without duplicate entry
+/// checking.
+///
+/// @param domain DdnsDomain to which to add the server
+/// @param name new server's host name of the server
+/// @param ip new server's ip address
+/// @param port new server's port
+///
+/// @throw Underlying methods may throw.
+extern void addDomainServer(DdnsDomainPtr& domain, const std::string& name,
+                            const std::string& ip = TEST_DNS_SERVER_IP,
+                            const size_t port = TEST_DNS_SERVER_PORT);
+
+}; // namespace isc::d2
+}; // namespace isc
+
+#endif

+ 137 - 185
src/bin/d2/tests/nc_trans_unittests.cc

@@ -19,6 +19,7 @@
 #include <log/logger_support.h>
 #include <log/macros.h>
 #include <util/buffer.h>
+#include <nc_test_utils.h>
 
 #include <asio/ip/udp.hpp>
 #include <asio/socket_base.hpp>
@@ -35,8 +36,6 @@ using namespace isc::d2;
 
 namespace {
 
-const size_t MAX_MSG_SIZE = 1024;
-
 /// @brief Test derivation of NameChangeTransaction for exercising state
 /// model mechanics.
 ///
@@ -104,7 +103,7 @@ public:
         return (false);
     }
 
-    /// @brief Empty handler used to statisfy map verification.
+    /// @brief Empty handler used to satisfy map verification.
     void dummyHandler() {
         isc_throw(NameChangeTransactionError,
                   "dummyHandler - invalid event: " << getContextStr());
@@ -259,153 +258,16 @@ public:
     using NameChangeTransaction::transition;
     using NameChangeTransaction::retryTransition;
     using NameChangeTransaction::sendUpdate;
+    using NameChangeTransaction::prepNewRequest;
+    using NameChangeTransaction::addLeaseAddressRdata;
+    using NameChangeTransaction::addDhcidRdata;
+    using NameChangeTransaction::addPtrRdata;
 };
 
 // Declare them so Gtest can see them.
 const int NameChangeStub::DOING_UPDATE_ST;
 const int NameChangeStub::SEND_UPDATE_EVT;
 
-typedef boost::shared_ptr<asio::ip::udp::socket> SocketPtr;
-
-/// @brief This class simulates a DNS server.  It is capable of performing
-/// an asynchronous read, governed by an IOService, and responding to received
-/// requests in a given manner.
-class FauxServer {
-public:
-    enum  ResponseMode {
-        USE_RCODE,   // Generate a response with a given RCODE
-        CORRUPT_RESP  // Generate a corrupt response
-    };
-
-    asiolink::IOService& io_service_;
-    asiolink::IOAddress& address_;
-    size_t port_;
-    SocketPtr server_socket_;
-    asio::ip::udp::endpoint remote_;
-    uint8_t receive_buffer_[MAX_MSG_SIZE];
-
-    /// @brief Constructor
-    ///
-    /// @param io_service IOService to be used for socket IO.
-    /// @param address  IP address at which the server should listen.
-    /// @param port Port number at which the server should listen.
-    FauxServer(asiolink::IOService& io_service, asiolink::IOAddress& address,
-               size_t port)
-        : io_service_(io_service), address_(address), port_(port),
-          server_socket_() {
-        server_socket_.reset(new asio::ip::udp::
-                             socket(io_service_.get_io_service(),
-                                    asio::ip::udp::v4()));
-        server_socket_->set_option(asio::socket_base::reuse_address(true));
-        server_socket_->bind(asio::ip::udp::
-                             endpoint(address_.getAddress(), port_));
-    }
-
-    /// @brief Destructor
-    virtual ~FauxServer() {
-    }
-
-    /// @brief Initiates an asyncrhonrous receive
-    ///
-    /// Starts the server listening for requests.  Upon completion of the
-    /// the listen, the callback method, requestHandler, is invoked.
-    ///
-    /// @param response_mode Selects how the server responds to a request
-    /// @param response_rcode The Rcode value set in the response. Not used
-    /// for all modes.
-    void receive (const ResponseMode& response_mode,
-                  const dns::Rcode& response_rcode=dns::Rcode::NOERROR()) {
-
-        server_socket_->async_receive_from(asio::buffer(receive_buffer_,
-                                                   sizeof(receive_buffer_)),
-                                      remote_,
-                                      boost::bind(&FauxServer::requestHandler,
-                                                  this, _1, _2,
-                                                  response_mode,
-                                                  response_rcode));
-    }
-
-    /// @brief Socket IO Completion callback
-    ///
-    /// This method servers as the Server's UDP socket receive callback handler.
-    /// When the receive completes the handler is invoked with the
-    /// @param error result code of the recieve (determined by asio layer)
-    /// @param bytes_recvd number of bytes received, if any
-    /// @param response_mode type of response the handler should produce
-    /// @param response_rcode value of Rcode in the response constructed by
-    /// handler
-    void requestHandler(const asio::error_code& error,
-                        std::size_t bytes_recvd,
-                        const ResponseMode& response_mode,
-                        const dns::Rcode& response_rcode) {
-
-        // If we encountered an error or received no data then fail.
-        // We expect the client to send good requests.
-        if (error.value() != 0 || bytes_recvd < 1) {
-            ADD_FAILURE() << "FauxServer receive failed" << error.message();
-            return;
-        }
-
-        // We have a successfully received data. We need to turn it into
-        // a request in order to build a proper response.
-        // Note D2UpdateMessage is geared towards making requests and
-        // reading responses.  This is the opposite perspective so we have
-        // to a bit of roll-your-own here.
-        dns::Message request(dns::Message::PARSE);
-        util::InputBuffer request_buf(receive_buffer_, bytes_recvd);
-        try {
-            request.fromWire(request_buf);
-        } catch (const std::exception& ex) {
-            // If the request cannot be parsed, then fail the test.
-            // We expect the client to send good requests.
-            ADD_FAILURE() << "FauxServer request is corrupt:" << ex.what();
-            return;
-        }
-
-        // The request parsed ok, so let's build a response.
-        // We must use the QID we received in the response or IOFetch will
-        // toss the response out, resulting in eventual timeout.
-        // We fill in the zone with data we know is from the "server".
-        dns::Message response(dns::Message::RENDER);
-        response.setQid(request.getQid());
-        dns::Question question(dns::Name("response.example.com"),
-                               dns::RRClass::ANY(), dns::RRType::SOA());
-        response.addQuestion(question);
-        response.setOpcode(dns::Opcode(dns::Opcode::UPDATE_CODE));
-        response.setHeaderFlag(dns::Message::HEADERFLAG_QR, true);
-
-        // Set the response Rcode to value passed in, default is NOERROR.
-        response.setRcode(response_rcode);
-
-        // Render the response to a buffer.
-        dns::MessageRenderer renderer;
-        util::OutputBuffer response_buf(MAX_MSG_SIZE);
-        renderer.setBuffer(&response_buf);
-        response.toWire(renderer);
-
-        // If mode is to ship garbage, then stomp on part of the rendered
-        // message.
-        if (response_mode == CORRUPT_RESP) {
-            response_buf.writeUint16At(0xFFFF, 2);
-        }
-
-        // Ship the reponse via synchronous send.
-        try {
-            int cnt = server_socket_->send_to(asio::
-                                              buffer(response_buf.getData(),
-                                                     response_buf.getLength()),
-                                              remote_);
-            // Make sure we sent what we expect to send.
-            if (cnt != response_buf.getLength()) {
-                ADD_FAILURE() << "FauxServer sent: " << cnt << " expected: "
-                          << response_buf.getLength();
-            }
-        } catch (const std::exception& ex) {
-            ADD_FAILURE() << "FauxServer send failed: " << ex.what();
-        }
-    }
-};
-
 /// @brief Defines a pointer to a NameChangeStubPtr instance.
 typedef boost::shared_ptr<NameChangeStub> NameChangeStubPtr;
 
@@ -445,7 +307,7 @@ public:
 
     /// @brief IO Timer expiration handler
     ///
-    /// Stops the IOSerivce and FAILs the current test.
+    /// Stops the IOSerivce and fails the current test.
     void timesUp() {
         io_service_->stop();
         FAIL() << "Test Time: " << run_time_ << " expired";
@@ -462,7 +324,7 @@ public:
             " \"change_type\" : 0 , "
             " \"forward_change\" : true , "
             " \"reverse_change\" : true , "
-            " \"fqdn\" : \"example.com.\" , "
+            " \"fqdn\" : \"my.example.com.\" , "
             " \"ip_address\" : \"192.168.2.1\" , "
             " \"dhcid\" : \"0102030405060708\" , "
             " \"lease_expires_on\" : \"20130121132405\" , "
@@ -470,31 +332,20 @@ public:
             "}";
 
         // Create the request from JSON.
-        dhcp_ddns::NameChangeRequestPtr ncr;
-        DnsServerInfoStoragePtr servers(new DnsServerInfoStorage());
-        DnsServerInfoPtr server;
-        ncr = dhcp_ddns::NameChangeRequest::fromJSON(msg_str);
+        dhcp_ddns::NameChangeRequestPtr ncr = dhcp_ddns::NameChangeRequest::
+                                              fromJSON(msg_str);
 
         // Make forward DdnsDomain with 2 forward servers.
-        server.reset(new DnsServerInfo("forward.example.com",
-                                       isc::asiolink::IOAddress("127.0.0.1"),
-                                       5301));
-        servers->push_back(server);
-        server.reset(new DnsServerInfo("forward2.example.com",
-                                       isc::asiolink::IOAddress("127.0.0.1"),
-                                       5302));
-
-        servers->push_back(server);
-        forward_domain_.reset(new DdnsDomain("example.com.", "", servers));
+        forward_domain_ = makeDomain("example.com.");
+        addDomainServer(forward_domain_, "forward.example.com",
+                        "127.0.0.1", 5301);
+        addDomainServer(forward_domain_, "forward2.example.com",
+                        "127.0.0.1", 5302);
 
         // Make reverse DdnsDomain with one reverse server.
-        servers.reset(new DnsServerInfoStorage());
-        server.reset(new DnsServerInfo("reverse.example.com",
-                                       isc::asiolink::IOAddress("127.0.0.1"),
-                                       5301));
-        servers->push_back(server);
-        reverse_domain_.reset(new DdnsDomain("2.168.192.in.addr.arpa.",
-                                             "", servers));
+        reverse_domain_ = makeDomain("2.168.192.in.addr.arpa.");
+        addDomainServer(reverse_domain_, "reverse.example.com",
+                        "127.0.0.1", 5301);
 
         // Instantiate the transaction as would be done by update manager.
         return (NameChangeStubPtr(new NameChangeStub(io_service_, ncr,
@@ -539,7 +390,7 @@ TEST(NameChangeTransaction, construction) {
     ASSERT_NO_THROW(reverse_domain.reset(new DdnsDomain("*", "", servers)));
 
     // Verify that construction with a null IOServicePtr fails.
-    // @todo Subject to change if multi-threading is implemenated.
+    // @todo Subject to change if multi-threading is implemented.
     IOServicePtr empty;
     EXPECT_THROW(NameChangeTransaction(empty, ncr,
                                        forward_domain, reverse_domain),
@@ -626,7 +477,7 @@ TEST_F(NameChangeTransactionTest, accessors) {
 
 /// @brief Tests DNS update request accessor methods.
 TEST_F(NameChangeTransactionTest, dnsUpdateRequestAccessors) {
-    // Create a transction.
+    // Create a transaction.
     NameChangeStubPtr name_change;
     ASSERT_NO_THROW(name_change = makeCannedTransaction());
 
@@ -652,7 +503,7 @@ TEST_F(NameChangeTransactionTest, dnsUpdateRequestAccessors) {
 
 /// @brief Tests DNS update request accessor methods.
 TEST_F(NameChangeTransactionTest, dnsUpdateResponseAccessors) {
-    // Create a transction.
+    // Create a transaction.
     NameChangeStubPtr name_change;
     ASSERT_NO_THROW(name_change = makeCannedTransaction());
 
@@ -829,9 +680,9 @@ TEST_F(NameChangeTransactionTest, modelFailure) {
     EXPECT_EQ(dhcp_ddns::ST_FAILED, name_change->getNcrStatus());
 }
 
-/// @brief Tests the ability to use startTransaction to initate the state
+/// @brief Tests the ability to use startTransaction to initiate the state
 /// model execution, and DNSClient callback, operator(), to resume the
-/// the model with a update successful outcome.
+/// model with a update successful outcome.
 TEST_F(NameChangeTransactionTest, successfulUpdateTest) {
     NameChangeStubPtr name_change;
     ASSERT_NO_THROW(name_change = makeCannedTransaction());
@@ -867,7 +718,7 @@ TEST_F(NameChangeTransactionTest, successfulUpdateTest) {
 
 /// @brief Tests the ability to use startTransaction to initate the state
 /// model execution, and DNSClient callback, operator(), to resume the
-/// the model with a update failure outcome.
+/// model with a update failure outcome.
 TEST_F(NameChangeTransactionTest, failedUpdateTest) {
     NameChangeStubPtr name_change;
     ASSERT_NO_THROW(name_change = makeCannedTransaction());
@@ -915,7 +766,7 @@ TEST_F(NameChangeTransactionTest, updateAttempts) {
 
 /// @brief Tests retryTransition method
 ///
-/// Verifes that while the maximum number of update attempts has not
+/// Verifies that while the maximum number of update attempts has not
 /// been exceeded, the method will leave the state unchanged but post a
 /// SERVER_SELECTED_EVT.  Once the maximum is exceeded, the method should
 /// transition to the state given with a next event of SERVER_IO_ERROR_EVT.
@@ -946,7 +797,7 @@ TEST_F(NameChangeTransactionTest, retryTransition) {
     ASSERT_NO_THROW(name_change->retryTransition(
                     NameChangeTransaction::PROCESS_TRANS_FAILED_ST));
 
-    // Since the number of udpate attempts is less than the maximum allowed
+    // Since the number of update attempts is less than the maximum allowed
     // we should remain in our current state but with next event of
     // SERVER_SELECTED_EVT posted.
     ASSERT_EQ(NameChangeStub::DOING_UPDATE_ST,
@@ -961,8 +812,8 @@ TEST_F(NameChangeTransactionTest, retryTransition) {
     ASSERT_NO_THROW(name_change->retryTransition(
                     NameChangeTransaction::PROCESS_TRANS_FAILED_ST));
 
-    // Since we have exceeded maximum attempts, we should tranisition to
-    // PROCESS_UPDATE_FAILD_ST with a next event of SERVER_IO_ERROR_EVT.
+    // Since we have exceeded maximum attempts, we should transition to
+    // PROCESS_UPDATE_FAILED_ST with a next event of SERVER_IO_ERROR_EVT.
     ASSERT_EQ(NameChangeTransaction::PROCESS_TRANS_FAILED_ST,
               name_change->getCurrState());
     ASSERT_EQ(NameChangeTransaction::SERVER_IO_ERROR_EVT,
@@ -986,7 +837,7 @@ TEST_F(NameChangeTransactionTest, sendUpdateDoUpdateFailure) {
     ASSERT_NO_THROW(name_change->setDnsUpdateRequest(req));
 
     // Verify that sendUpdate does not throw, but it should fail because
-    // the requset won't render.
+    // the request won't render.
     ASSERT_NO_THROW(name_change->sendUpdate());
 
     // Verify that we transition to failed state and event.
@@ -1013,7 +864,7 @@ TEST_F(NameChangeTransactionTest, sendUpdateTimeout) {
     // Set the flag to use the NameChangeStub's DNSClient callback.
     name_change->use_stub_callback_ = true;
 
-    // Invoke sendUdpate.
+    // Invoke sendUpdate.
     ASSERT_NO_THROW(name_change->sendUpdate());
 
     // Update attempt count should be 1, next event should be NOP_EVT.
@@ -1040,8 +891,7 @@ TEST_F(NameChangeTransactionTest, sendUpdateCorruptResponse) {
     ASSERT_TRUE(name_change->selectFwdServer());
 
     // Create a server and start it listening.
-    asiolink::IOAddress address("127.0.0.1");
-    FauxServer server(*io_service_, address, 5301);
+    FauxServer server(*io_service_, *(name_change->getCurrentServer()));
     server.receive(FauxServer::CORRUPT_RESP);
 
     // Create a valid request for the transaction.
@@ -1054,7 +904,7 @@ TEST_F(NameChangeTransactionTest, sendUpdateCorruptResponse) {
     // Set the flag to use the NameChangeStub's DNSClient callback.
     name_change->use_stub_callback_ = true;
 
-    // Invoke sendUdpate.
+    // Invoke sendUpdate.
     ASSERT_NO_THROW(name_change->sendUpdate());
 
     // Update attempt count should be 1, next event should be NOP_EVT.
@@ -1079,8 +929,7 @@ TEST_F(NameChangeTransactionTest, sendUpdate) {
     ASSERT_TRUE(name_change->selectFwdServer());
 
     // Create a server and start it listening.
-    asiolink::IOAddress address("127.0.0.1");
-    FauxServer server(*io_service_, address, 5301);
+    FauxServer server(*io_service_, *(name_change->getCurrentServer()));
     server.receive (FauxServer::USE_RCODE, dns::Rcode::NOERROR());
 
     // Create a valid request for the transaction.
@@ -1093,7 +942,7 @@ TEST_F(NameChangeTransactionTest, sendUpdate) {
     // Set the flag to use the NameChangeStub's DNSClient callback.
     name_change->use_stub_callback_ = true;
 
-    // Invoke sendUdpate.
+    // Invoke sendUpdate.
     ASSERT_NO_THROW(name_change->sendUpdate());
 
     // Update attempt count should be 1, next event should be NOP_EVT.
@@ -1119,4 +968,107 @@ TEST_F(NameChangeTransactionTest, sendUpdate) {
     EXPECT_EQ("response.example.com.", zone->getName().toText());
 }
 
+/// @brief Tests the prepNewRequest method
+TEST_F(NameChangeTransactionTest, prepNewRequest) {
+    NameChangeStubPtr name_change;
+    ASSERT_NO_THROW(name_change = makeCannedTransaction());
+    D2UpdateMessagePtr request;
+
+    // prepNewRequest should fail on empty domain.
+    ASSERT_THROW(request = name_change->prepNewRequest(DdnsDomainPtr()),
+        NameChangeTransactionError);
+
+    // Verify that prepNewRequest fails on invalid zone name.
+    // @todo This test becomes obsolete if/when DdsnDomain enforces valid
+    // names as is done in dns::Name.
+    DdnsDomainPtr bsDomain = makeDomain(".badname","");
+    ASSERT_THROW(request = name_change->prepNewRequest(bsDomain),
+        NameChangeTransactionError);
+
+    // Verify that prepNewRequest properly constructs a message given
+    // valid input.
+    ASSERT_NO_THROW(request = name_change->prepNewRequest(forward_domain_));
+    checkZone(request, forward_domain_->getName());
+}
+
+/// @brief Tests the addLeaseAddressRData method
+TEST_F(NameChangeTransactionTest, addLeaseAddressRData) {
+    NameChangeStubPtr name_change;
+    ASSERT_NO_THROW(name_change = makeCannedTransaction());
+    dhcp_ddns::NameChangeRequestPtr ncr = name_change->getNcr();
+
+    // Test a lease rdata add failure.
+    // As you cannot stuff an invalid address into an NCR, the only failure
+    // that can be induced is a mismatch between the RData and the RRset.
+    // Attempt to add a lease address Rdata, this should fail.
+    // Create an Any class/Any type RRset, they are not allowed to contain
+    // rdata.
+    dns::RRsetPtr rrset(new dns::RRset(dns::Name("bs"), dns::RRClass::ANY(),
+                         dns::RRType::ANY(), dns::RRTTL(0)));
+    ASSERT_THROW(name_change->addLeaseAddressRdata(rrset), std::exception);
+
+    // Verify we can add a lease RData to an valid RRset.
+    rrset.reset(new dns::RRset(dns::Name("bs"), dns::RRClass::IN(),
+                               name_change->getAddressRRType(), dns::RRTTL(0)));
+    ASSERT_NO_THROW(name_change->addLeaseAddressRdata(rrset));
+
+    // Verify the Rdata was added and the value is correct.
+    ASSERT_EQ(1, rrset->getRdataCount());
+    dns::RdataIteratorPtr rdata_it = rrset->getRdataIterator();
+    ASSERT_TRUE(rdata_it);
+    EXPECT_EQ(ncr->getIpAddress(), rdata_it->getCurrent().toText());
+
 }
+
+/// @brief Tests the addDhcidRData method
+TEST_F(NameChangeTransactionTest, addDhcidRdata) {
+    NameChangeStubPtr name_change;
+    ASSERT_NO_THROW(name_change = makeCannedTransaction());
+    dhcp_ddns::NameChangeRequestPtr ncr = name_change->getNcr();
+
+    // Test a DHCID rdata add failure.
+    dns::RRsetPtr rrset(new dns::RRset(dns::Name("bs"), dns::RRClass::ANY(),
+                         dns::RRType::ANY(), dns::RRTTL(0)));
+    ASSERT_THROW(name_change->addDhcidRdata(rrset), std::exception);
+
+    // Verify we can add a lease RData to an valid RRset.
+    rrset.reset(new dns::RRset(dns::Name("bs"), dns::RRClass::IN(),
+                               dns::RRType::DHCID(), dns::RRTTL(0)));
+    ASSERT_NO_THROW(name_change->addDhcidRdata(rrset));
+
+    // Verify the Rdata was added and the value is correct.
+    ASSERT_EQ(1, rrset->getRdataCount());
+    dns::RdataIteratorPtr rdata_it = rrset->getRdataIterator();
+    ASSERT_TRUE(rdata_it);
+
+    const std::vector<uint8_t>& ncr_dhcid = ncr->getDhcid().getBytes();
+    util::InputBuffer buffer(ncr_dhcid.data(), ncr_dhcid.size());
+    dns::rdata::in::DHCID rdata_ref(buffer, ncr_dhcid.size());
+    EXPECT_EQ(0, rdata_ref.compare(rdata_it->getCurrent()));
+}
+
+/// @brief Tests the addPtrData method
+TEST_F(NameChangeTransactionTest, addPtrRdata) {
+    NameChangeStubPtr name_change;
+    ASSERT_NO_THROW(name_change = makeCannedTransaction());
+    dhcp_ddns::NameChangeRequestPtr ncr = name_change->getNcr();
+
+    // Test a PTR rdata add failure.
+    dns::RRsetPtr rrset(new dns::RRset(dns::Name("bs"), dns::RRClass::ANY(),
+                         dns::RRType::ANY(), dns::RRTTL(0)));
+    ASSERT_THROW(name_change->addPtrRdata(rrset), std::exception);
+
+    // Verify we can add a PTR RData to an valid RRset.
+    rrset.reset(new dns::RRset(dns::Name("bs"), dns::RRClass::IN(),
+                               dns::RRType::PTR(), dns::RRTTL(0)));
+    ASSERT_NO_THROW(name_change->addPtrRdata(rrset));
+
+    // Verify the Rdata was added and the value is correct.
+    ASSERT_EQ(1, rrset->getRdataCount());
+    dns::RdataIteratorPtr rdata_it = rrset->getRdataIterator();
+    ASSERT_TRUE(rdata_it);
+
+    EXPECT_EQ(ncr->getFqdn(), rdata_it->getCurrent().toText());
+}
+
+}; // anonymous namespace

+ 14 - 11
src/lib/dhcp_ddns/ncr_msg.cc

@@ -190,6 +190,7 @@ operator<<(std::ostream& os, const D2Dhcid& dhcid) {
 NameChangeRequest::NameChangeRequest()
     : change_type_(CHG_ADD), forward_change_(false),
     reverse_change_(false), fqdn_(""), ip_address_(""),
+    ip_io_address_("0.0.0.0"),
     dhcid_(), lease_expires_on_(), lease_length_(0), status_(ST_NEW) {
 }
 
@@ -201,9 +202,13 @@ NameChangeRequest::NameChangeRequest(const NameChangeType change_type,
             const uint32_t lease_length)
     : change_type_(change_type), forward_change_(forward_change),
     reverse_change_(reverse_change), fqdn_(fqdn), ip_address_(ip_address),
+    ip_io_address_("0.0.0.0"),
     dhcid_(dhcid), lease_expires_on_(lease_expires_on),
     lease_length_(lease_length), status_(ST_NEW) {
 
+    // User setter to validate address.
+    setIpAddress(ip_address_);
+
     // Validate the contents. This will throw a NcrMessageError if anything
     // is invalid.
     validateContent();
@@ -359,20 +364,12 @@ NameChangeRequest::toJSON() const  {
 void
 NameChangeRequest::validateContent() {
     //@todo This is an initial implementation which provides a minimal amount
-    // of validation.  FQDN, DHCID, and IP Address members are all currently
+    // of validation.  FQDN and DHCID members are all currently
     // strings, these may be replaced with richer classes.
     if (fqdn_ == "") {
         isc_throw(NcrMessageError, "FQDN cannot be blank");
     }
 
-    // Validate IP Address.
-    try {
-        isc::asiolink::IOAddress io_addr(ip_address_);
-    } catch (const isc::asiolink::IOError& ex) {
-        isc_throw(NcrMessageError,
-                  "Invalid ip address string for ip_address: " << ip_address_);
-    }
-
     // Validate the DHCID.
     if (dhcid_.getBytes().size()  == 0) {
         isc_throw(NcrMessageError, "DHCID cannot be blank");
@@ -483,10 +480,16 @@ NameChangeRequest::setFqdn(const std::string& value) {
 
 void
 NameChangeRequest::setIpAddress(const std::string& value) {
-    ip_address_ = value;
+    // Validate IP Address.
+    try {
+        ip_address_ = value;
+        ip_io_address_ = isc::asiolink::IOAddress(ip_address_);
+    } catch (const isc::asiolink::IOError& ex) {
+        isc_throw(NcrMessageError,
+                  "Invalid ip address string for ip_address: " << ip_address_);
+    }
 }
 
-
 void
 NameChangeRequest::setIpAddress(isc::data::ConstElementPtr element) {
     setIpAddress(element->stringValue());

+ 31 - 2
src/lib/dhcp_ddns/ncr_msg.h

@@ -155,7 +155,7 @@ public:
     /// @brief Returns a reference to the DHCID byte vector.
     ///
     /// @return a reference to the vector.
-    const std::vector<uint8_t>& getBytes() {
+    const std::vector<uint8_t>& getBytes() const {
         return (bytes_);
     }
 
@@ -411,13 +411,34 @@ public:
     /// Element
     void setFqdn(isc::data::ConstElementPtr element);
 
-    /// @brief Fetches the request IP address.
+    /// @brief Fetches the request IP address string.
     ///
     /// @return a string containing the IP address
     const std::string& getIpAddress() const {
         return (ip_address_);
     }
 
+    /// @brief Fetches the request IP address as an IOAddress.
+    ///
+    /// @return a asiolink::IOAddress containing the IP address
+    const asiolink::IOAddress& getIpIoAddress() const {
+        return (ip_io_address_);
+    }
+
+    /// @brief Returns true if the lease address is a IPv4 lease.
+    ///
+    /// @return boolean true if the lease address family is AF_INET.
+    bool isV4 () {
+        return (ip_io_address_.isV4());
+    }
+
+    /// @brief Returns true if the lease address is a IPv6 lease.
+    ///
+    /// @return boolean true if the lease address family is AF_INET6.
+    bool isV6 () {
+        return (ip_io_address_.isV6());
+    }
+
     /// @brief Sets the IP address to the given value.
     ///
     /// @param value contains the new value to assign to the IP address
@@ -570,6 +591,14 @@ private:
     /// @brief The ip address leased to the FQDN.
     std::string ip_address_;
 
+    /// @brief The ip address leased to the FQDN as an IOAddress.
+    ///
+    /// The lease address is used in many places, sometimes as a string
+    /// and sometimes as an IOAddress.  To avoid converting back and forth
+    /// continually over the life span of an NCR, we do it once when the
+    /// ip address is actually set.
+    asiolink::IOAddress ip_io_address_;
+
     /// @brief The lease client's unique DHCID.
     /// @todo Currently, this is uses D2Dhcid it but may be replaced with
     /// dns::DHCID which provides additional validation.

+ 22 - 0
src/lib/dhcp_ddns/tests/ncr_unittests.cc

@@ -575,5 +575,27 @@ TEST(NameChangeRequestTest, toFromBufferTest) {
     ASSERT_EQ(final_str, msg_str);
 }
 
+/// @brief Tests ip address modification and validation
+TEST(NameChangeRequestTest, ipAddresses) {
+    NameChangeRequest ncr;
+
+    // Verify that a valid IPv4 address works.
+    ASSERT_NO_THROW(ncr.setIpAddress("192.168.1.1"));
+    const asiolink::IOAddress& io_addr4 = ncr.getIpIoAddress();
+    EXPECT_EQ(ncr.getIpAddress(), io_addr4.toText());
+    EXPECT_TRUE(ncr.isV4());
+    EXPECT_FALSE(ncr.isV6());
+
+    // Verify that a valid IPv6 address works.
+    ASSERT_NO_THROW(ncr.setIpAddress("2001:1::f3"));
+    const asiolink::IOAddress& io_addr6 = ncr.getIpIoAddress();
+    EXPECT_EQ(ncr.getIpAddress(), io_addr6.toText());
+    EXPECT_FALSE(ncr.isV4());
+    EXPECT_TRUE(ncr.isV6());
+
+    // Verify that an valid address fails.
+    ASSERT_THROW(ncr.setIpAddress("x001:1::f3"),NcrMessageError);
+}
+
 } // end of anonymous namespace