Browse Source

[3088] Added DNS request construction to d2::NameRemoveTransaction

Added methods for constructing all three types of DNS update requests
required by d2::NameRemoveTransaction to complete the implementation of its
state machine.  Also refactored some unit test code into nc_test_utils.h
and .cc. Renamed request verification functions in nc_test_utils to match
the build request function names.
Thomas Markwalder 11 years ago
parent
commit
265d1dabd2

+ 6 - 6
src/bin/d2/d2_messages.mes

@@ -325,19 +325,19 @@ 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
+% DHCP_DDNS_FORWARD_ADD_BUILD_FAILURE 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
+% DHCP_DDNS_FORWARD_REPLACE_BUILD_FAILURE 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
+% DHCP_DDNS_REVERSE_REPLACE_BUILD_FAILURE 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
@@ -352,7 +352,7 @@ This is an error message issued after DHCP_DDNS attempts to submit DNS mapping
 entry additions have failed.  The precise reason for the failure should be
 documented in preceding log entries.
 
-% DHCP_DDNS_FORWARD_REMOVE_ADDRS_BUILD_FAILURE A DNS udpate message to remove a forward DNS Address entry could not be constructed for this request: %1  reason: %2
+% DHCP_DDNS_FORWARD_REMOVE_ADDRS_BUILD_FAILURE DNS udpate message to remove a forward DNS Address 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 (A or AAAA) removal.  This
 is due to invalid data contained in the NameChangeRequest. The request will be
@@ -378,7 +378,7 @@ This is an error message issued when DNSClient returns an unrecognized status
 while DHCP_DDNS was removing a forward address mapping.  The request will be
 aborted.  This is most likely a programmatic issue and should be reported.
 
-% DHCP_DDNS_FORWARD_REMOVE_RRS_BUILD_FAILURE A DNS udpate message to remove forward DNS RR entries could not be constructed for this request: %1  reason: %2
+% DHCP_DDNS_FORWARD_REMOVE_RRS_BUILD_FAILURE DNS udpate message to remove forward DNS RR entries 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 forward RR (DHCID RR) removal.  This is due
 to invalid data contained in the NameChangeRequest. The request will be aborted.This is most likely a configuration issue.
@@ -403,7 +403,7 @@ This is an error message issued when DNSClient returns an unrecognized status
 while DHCP_DDNS was removing forward RRs.  The request will be aborted. This is
 most likely a programmatic issue and should be reported.
 
-% DHCP_DDNS_REVERSE_REMOVE_BUILD_FAILURE A DNS update message to remove a reverse DNS entry could not be constructed from this request: %1  reason: %2
+% DHCP_DDNS_REVERSE_REMOVE_BUILD_FAILURE DNS update message to remove 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 removal.  This is
 due to invalid data contained in the NameChangeRequest. The request will be

+ 64 - 79
src/bin/d2/nc_remove.cc

@@ -13,6 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <d2/d2_log.h>
+#include <d2/d2_cfg_mgr.h>
 #include <d2/nc_remove.h>
 
 #include <boost/function.hpp>
@@ -28,7 +29,7 @@ const int NameRemoveTransaction::REMOVING_FWD_RRS_ST;
 const int NameRemoveTransaction::REMOVING_REV_PTRS_ST;
 
 // NameRemoveTransaction events
-// @todo none so far
+// Currently NameRemoveTransaction does not define any events.
 
 NameRemoveTransaction::
 NameRemoveTransaction(IOServicePtr& io_service,
@@ -51,7 +52,7 @@ NameRemoveTransaction::defineEvents() {
     NameChangeTransaction::defineEvents();
 
     // Define NameRemoveTransaction events.
-    // @todo none so far
+    // Currently NameRemoveTransaction does not define any events.
     // defineEvent(TBD_EVENT, "TBD_EVT");
 }
 
@@ -61,7 +62,7 @@ NameRemoveTransaction::verifyEvents() {
     NameChangeTransaction::verifyEvents();
 
     // Verify NameRemoveTransaction events.
-    // @todo none so far
+    // Currently NameRemoveTransaction does not define any events.
     // getEvent(TBD_EVENT);
 }
 
@@ -181,7 +182,7 @@ NameRemoveTransaction::removingFwdAddrsHandler() {
                 // 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, 
+                LOG_ERROR(dctl_logger,
                           DHCP_DDNS_FORWARD_REMOVE_ADDRS_BUILD_FAILURE)
                           .arg(getNcr()->toText())
                           .arg(ex.what());
@@ -245,7 +246,7 @@ NameRemoveTransaction::removingFwdAddrsHandler() {
         default:
             // Any other value and we will fail this transaction, something
             // bigger is wrong.
-            LOG_ERROR(dctl_logger, 
+            LOG_ERROR(dctl_logger,
                       DHCP_DDNS_FORWARD_REMOVE_ADDRS_BAD_DNSCLIENT_STATUS)
                       .arg(getDnsUpdateStatus())
                       .arg(getNcr()->getFqdn())
@@ -285,7 +286,7 @@ NameRemoveTransaction::removingFwdRRsHandler() {
                 // 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, 
+                LOG_ERROR(dctl_logger,
                           DHCP_DDNS_FORWARD_REMOVE_RRS_BUILD_FAILURE)
                           .arg(getNcr()->toText())
                           .arg(ex.what());
@@ -307,8 +308,8 @@ NameRemoveTransaction::removingFwdRRsHandler() {
             // @todo Not sure if NXDOMAIN is ok here, but I think so.
             // A Rcode of NXDOMAIN would mean there are no RRs for the FQDN,
             // which is fine.  We were asked to delete them, they are not there
-            // so all is well. 
-            if ((rcode == dns::Rcode::NOERROR()) || 
+            // so all is well.
+            if ((rcode == dns::Rcode::NOERROR()) ||
                 (rcode == dns::Rcode::NXDOMAIN())) {
                 // We were able to remove the forward mapping. Mark it as done.
                 setForwardChangeCompleted(true);
@@ -344,13 +345,13 @@ NameRemoveTransaction::removingFwdRRsHandler() {
                       .arg(getNcr()->getFqdn())
                       .arg(getCurrentServer()->getIpAddress());
 
-            // @note If we exhaust the IO retries for the current server 
-            // due to IO failures, we will abort the remaining updates.  
-            // The rational is that we are only in this state, if the remove 
-            // of the forward address RR succeeded (removingFwdAddrsHandler) 
+            // @note If we exhaust the IO retries for the current server
+            // due to IO failures, we will abort the remaining updates.
+            // The rational is that we are only in this state, if the remove
+            // of the forward address RR succeeded (removingFwdAddrsHandler)
             // on the current server. Therefore  we should not attempt another
             // removal on a different server.  This is perhaps a point
-            // for discussion. 
+            // for discussion.
             // @todo Should we go ahead with the reverse remove?
             retryTransition(PROCESS_TRANS_FAILED_ST);
             break;
@@ -544,7 +545,6 @@ NameRemoveTransaction::processRemoveOkHandler() {
     }
 }
 
-
 void
 NameRemoveTransaction::processRemoveFailedHandler() {
     switch(getNextEvent()) {
@@ -567,36 +567,28 @@ NameRemoveTransaction::buildRemoveFwdAddressRequest() {
     // Construct an empty request.
     D2UpdateMessagePtr request = prepNewRequest(getForwardDomain());
 
-#if 0
+    // Content on this request is based on RFC 4703, section 5.5, paragraph 4.
     // Construct dns::Name from NCR fqdn.
     dns::Name fqdn(dns::Name(getNcr()->getFqdn()));
+    // First build the Prerequisite Section
 
-    // 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)));
+    // Create an DHCID matches prerequisite RR and add it to the
+    // pre-requisite section
+    // Based on RFC 2136, section 2.4.2.
+    dns::RRsetPtr prereq(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.
+    // Next build the Update Section
 
-    // Create the FQDN/IP 'add' RR (RFC 2136, section 2.5.1)
-    // Set the message RData to lease address.
+    // Create the FQDN/IP 'delete' RR and add it to the update section.
     // Add the RR to update section.
-    dns::RRsetPtr update(new dns::RRset(fqdn, dns::RRClass::IN(),
+    // Based on 2136 section 2.5.4
+    dns::RRsetPtr update(new dns::RRset(fqdn, dns::RRClass::NONE(),
                          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);
-#endif
 
     // Set the transaction's update request to the new request.
     setDnsUpdateRequest(request);
@@ -607,43 +599,44 @@ NameRemoveTransaction::buildRemoveFwdRRsRequest() {
     // Construct an empty request.
     D2UpdateMessagePtr request = prepNewRequest(getForwardDomain());
 
-#if 0
     // Construct dns::Name from NCR fqdn.
     dns::Name fqdn(dns::Name(getNcr()->getFqdn()));
 
+    // Content on this request is based on RFC 4703, section 5.5, paragraph 5.
     // 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)));
+    // Based on RFC 2136, section 2.4.2.
+    dns::RRsetPtr prereq(new dns::RRset(fqdn, dns::RRClass::IN(),
+                         dns::RRType::DHCID(), dns::RRTTL(0)));
     addDhcidRdata(prereq);
     request->addRRset(D2UpdateMessage::SECTION_PREREQUISITE, prereq);
 
+    // Create an assertion that there are no A RRs for the FQDN.
+    // Add it to the pre-reqs.
+    // Based on RFC 2136, section 2.4.3.
+    prereq.reset(new dns::RRset(fqdn, dns::RRClass::NONE(),
+                                dns::RRType::A(), dns::RRTTL(0)));
+    request->addRRset(D2UpdateMessage::SECTION_PREREQUISITE, prereq);
+
+    // Create an assertion that there are no A RRs for the FQDN.
+    // Add it to the pre-reqs.
+    // Based on RFC 2136, section 2.4.3.
+    prereq.reset(new dns::RRset(fqdn, dns::RRClass::NONE(),
+                                dns::RRType::AAAA(), dns::RRTTL(0)));
+    request->addRRset(D2UpdateMessage::SECTION_PREREQUISITE, prereq);
+
     // Next build the Update Section.
 
-    // Create the FQDN/IP 'delete' RR (RFC 2136, section 2.5.1)
+    // Create the 'delete' of all RRs for FQDN.
     // Set the message RData to lease address.
     // Add the RR to update section.
+    // Based on RFC 2136, section 2.5.3.
     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);
+                         dns::RRType::ANY(), dns::RRTTL(0)));
     request->addRRset(D2UpdateMessage::SECTION_UPDATE, update);
-#endif
 
     // Set the transaction's update request to the new request.
     setDnsUpdateRequest(request);
@@ -654,38 +647,30 @@ NameRemoveTransaction::buildRemoveRevPtrsRequest() {
     // Construct an empty request.
     D2UpdateMessagePtr request = prepNewRequest(getReverseDomain());
 
-#if 0
     // 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);
+    // Content on this request is based on RFC 4703, section 5.5, paragraph 2.
+    // First build the Prerequisite Section.
+    // (Note that per RFC 4703, section 5.4, there is no need to validate
+    // DHCID RR for PTR entries.)
+
+    // Create an assertion that the PTRDNAME in the PTR record matches the
+    // client's FQDN for the address that was released.
+    // Based on RFC 2136, section 3.2.3
+    dns::RRsetPtr prereq(new dns::RRset(rev_ip, dns::RRClass::IN(),
+                                        dns::RRType::PTR(), dns::RRTTL(0)));
+    addPtrRdata(prereq);
+    request->addRRset(D2UpdateMessage::SECTION_PREREQUISITE, prereq);
 
-    // 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);
+    // Now, build the Update section.
 
-    // 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);
+    // Create a delete of any RRs for the FQDN and add it to update section.
+    // Based on RFC 2136, section 3.4.2.3
+    dns::RRsetPtr update(new dns::RRset(rev_ip, dns::RRClass::ANY(),
+                         dns::RRType::ANY(), dns::RRTTL(0)));
     request->addRRset(D2UpdateMessage::SECTION_UPDATE, update);
-#endif
 
     // Set the transaction's update request to the new request.
     setDnsUpdateRequest(request);

+ 43 - 10
src/bin/d2/nc_remove.h

@@ -284,7 +284,7 @@ protected:
     /// failures, we will abort the remaining updates.  The rational is that
     /// we are only in this state, if the remove of the forward address RR
     /// succeeded (removingFwdAddrsHandler) on the current server so we should
-    /// not attempt another removal on a different server.  This is perhaps a 
+    /// not attempt another removal on a different server.  This is perhaps a
     /// point for discussion. @todo Should we go ahead with the reverse remove?
     ///
     /// @throw NameRemoveTransactionError if upon entry next event is not:
@@ -369,25 +369,58 @@ protected:
     /// UPDATE_FAILED_EVT
     void processRemoveFailedHandler();
 
-    /// @brief Builds a DNS request to add an forward DNS entry for an FQDN
+    /// @brief Builds a DNS request to remove a forward DNS address for a FQDN.
     ///
-    /// @todo - Method not implemented yet
+    /// Constructs a DNS update request, based upon the NCR, for removing a
+    /// forward DNS address 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.5, paragraph 4.
+    ///
+    /// Prerequisite RRsets:
+    /// 1. An assertion that a matching DHCID RR exists
+    ///
+    /// Updates RRsets:
+    /// 1. A delete of the FQDN/IP RR (type A for IPv4, AAAA for IPv6)
+    ///
+    /// @throw This method does not throw but underlying methods may.
     void buildRemoveFwdAddressRequest();
 
-    /// @brief Builds a DNS request to replace forward DNS entry for an FQDN
+    /// @brief Builds a DNS request to remove all forward DNS RRs for a FQDN.
+    ///
+    /// Constructs a DNS update request, based upon the NCR, for removing any
+    /// remaining forward DNS RRs, once all A or AAAA entries for the FQDN
+    /// have been removed. Once constructed, the request is stored as the
+    /// transaction's DNS update request.
+    ///
+    /// The request content is adherent to RFC 4703 section 5.5, paragraph 5.
     ///
-    /// @todo - Method not implemented yet
+    /// Prerequisite RRsets:
+    /// 1. An assertion that a matching DHCID RR exists
+    /// 2. An assertion that no A RRs for the FQDN exist
+    /// 3. An assertion that no AAAA RRs for the FQDN exist
     ///
-    /// @throw isc::NotImplemented
+    /// Updates RRsets:
+    /// 1. A delete of all RRs for the FQDN
+    ///
+    /// @throw This method does not throw but underlying methods may.
     void buildRemoveFwdRRsRequest();
 
-    /// @brief Builds a DNS request to replace a reverse DNS entry for an FQDN
+    /// @brief Builds a DNS request to remove a reverse DNS entry for a FQDN
+    ///
+    /// Constructs a DNS update request, based upon the NCR, for removing 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.5, paragraph 2:
+    ///
+    /// Prerequisite RRsets:
+    /// 1. An assertion that a PTR record matching the client's FQDN exists.
     ///
-    /// @todo - Method not implemented yet
+    /// Updates RRsets:
+    /// 1. A delete of all RRs for the FQDN
     ///
-    /// @throw isc::NotImplemented
+    /// @throw This method does not throw but underlying methods may.
     void buildRemoveRevPtrsRequest();
 };
 

+ 12 - 6
src/bin/d2/nc_trans.cc

@@ -14,6 +14,7 @@
 
 #include <d2/d2_log.h>
 #include <d2/nc_trans.h>
+#include <dns/rdata.h>
 
 namespace isc {
 namespace d2 {
@@ -262,17 +263,19 @@ NameChangeTransaction::addLeaseAddressRdata(dns::RRsetPtr& rrset) {
 
     try {
         // Manufacture an RData from the lease address then add it to the RR.
+        dns::rdata::ConstRdataPtr rdata;
         if (ncr_->isV4()) {
-            dns::rdata::in::A a_rdata(ncr_->getIpAddress());
-            rrset->addRdata(a_rdata);
+            rdata.reset(new dns::rdata::in::A(ncr_->getIpAddress()));
+            rrset->addRdata(rdata);
         } else {
-            dns::rdata::in::AAAA rdata(ncr_->getIpAddress());
+            rdata.reset(new dns::rdata::in::AAAA(ncr_->getIpAddress()));
             rrset->addRdata(rdata);
         }
     } catch (const std::exception& ex) {
         isc_throw(NameChangeTransactionError, "Cannot add address rdata: "
                   << ex.what());
     }
+
 }
 
 void
@@ -283,14 +286,16 @@ NameChangeTransaction::addDhcidRdata(dns::RRsetPtr& rrset) {
     }
 
     try {
+        dns::rdata::ConstRdataPtr 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(buffer, ncr_dhcid.size());
+        rdata.reset(new dns::rdata::in::DHCID(buffer, ncr_dhcid.size()));
         rrset->addRdata(rdata);
     } catch (const std::exception& ex) {
         isc_throw(NameChangeTransactionError, "Cannot add DCHID rdata: "
                   << ex.what());
     }
+
 }
 
 void
@@ -301,7 +306,8 @@ NameChangeTransaction::addPtrRdata(dns::RRsetPtr& rrset) {
     }
 
     try {
-        dns::rdata::generic::PTR rdata(getNcr()->getFqdn());
+        dns::rdata::ConstRdataPtr rdata;
+        rdata.reset(new dns::rdata::generic::PTR(getNcr()->getFqdn()));
         rrset->addRdata(rdata);
     } catch (const std::exception& ex) {
         isc_throw(NameChangeTransactionError, "Cannot add PTR rdata: "
@@ -412,7 +418,7 @@ NameChangeTransaction::getUpdateAttempts() const {
 
 const dns::RRType&
 NameChangeTransaction::getAddressRRType() const {
-    return (ncr_->isV4() ?  dns::RRType::A(): dns::RRType::AAAA());
+    return (ncr_->isV4() ?  dns::RRType::A() : dns::RRType::AAAA());
 }
 
 } // namespace isc::d2

+ 9 - 9
src/bin/d2/tests/nc_add_unittests.cc

@@ -407,14 +407,14 @@ TEST_F(NameAddTransactionTest, buildForwardAdd) {
     NameAddStubPtr name_add;
     ASSERT_NO_THROW(name_add = makeTransaction4());
     ASSERT_NO_THROW(name_add->buildAddFwdAddressRequest());
-    checkForwardAddRequest(*name_add);
+    checkAddFwdAddressRequest(*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);
+    checkAddFwdAddressRequest(*name_add);
 }
 
 /// @brief Tests construction of a DNS update request for replacing a forward
@@ -426,14 +426,14 @@ TEST_F(NameAddTransactionTest, buildReplaceFwdAddressRequest) {
     NameAddStubPtr name_add;
     ASSERT_NO_THROW(name_add = makeTransaction4());
     ASSERT_NO_THROW(name_add->buildReplaceFwdAddressRequest());
-    checkForwardReplaceRequest(*name_add);
+    checkReplaceFwdAddressRequest(*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);
+    checkReplaceFwdAddressRequest(*name_add);
 }
 
 /// @brief Tests the construction of a DNS update request for replacing a
@@ -445,14 +445,14 @@ TEST_F(NameAddTransactionTest, buildReplaceRevPtrsRequest) {
     NameAddStubPtr name_add;
     ASSERT_NO_THROW(name_add = makeTransaction4());
     ASSERT_NO_THROW(name_add->buildReplaceRevPtrsRequest());
-    checkReverseReplaceRequest(*name_add);
+    checkReplaceRevPtrsRequest(*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);
+    checkReplaceRevPtrsRequest(*name_add);
 }
 
 // Tests the readyHandler functionality.
@@ -632,7 +632,7 @@ TEST_F(NameAddTransactionTest, addingFwdAddrsHandler_FwdOnlyAddOK) {
     EXPECT_NO_THROW(name_add->addingFwdAddrsHandler());
 
     // Verify that an update message was constructed properly.
-    checkForwardAddRequest(*name_add);
+    checkAddFwdAddressRequest(*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.
@@ -943,7 +943,7 @@ TEST_F(NameAddTransactionTest, replacingFwdAddrsHandler_FwdOnlyAddOK) {
     EXPECT_NO_THROW(name_add->replacingFwdAddrsHandler());
 
     // Verify that an update message was constructed properly.
-    checkForwardReplaceRequest(*name_add);
+    checkReplaceFwdAddressRequest(*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.
@@ -1366,7 +1366,7 @@ TEST_F(NameAddTransactionTest, replacingRevPtrsHandler_FwdOnlyAddOK) {
     EXPECT_NO_THROW(name_add->replacingRevPtrsHandler());
 
     // Verify that an update message was constructed properly.
-    checkReverseReplaceRequest(*name_add);
+    checkReplaceRevPtrsRequest(*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.

+ 51 - 52
src/bin/d2/tests/nc_remove_unittests.cc

@@ -36,7 +36,7 @@ public:
                 dhcp_ddns::NameChangeRequestPtr& ncr,
                 DdnsDomainPtr& forward_domain,
                 DdnsDomainPtr& reverse_domain)
-        : NameRemoveTransaction(io_service, ncr, forward_domain, 
+        : NameRemoveTransaction(io_service, ncr, forward_domain,
                                 reverse_domain),
           simulate_send_exception_(false),
           simulate_build_request_exception_(false) {
@@ -245,10 +245,10 @@ public:
     /// @param event value to post as the next event
     /// @param change_mask determines which change directions are requested
     /// @param family selects between an IPv4 (AF_INET) and IPv6 (AF_INET6)
-    /// transaction. 
+    /// transaction.
     NameRemoveStubPtr prepHandlerTest(unsigned int state, unsigned int event,
-                                      unsigned int change_mask 
-                                      = FWD_AND_REV_CHG, 
+                                      unsigned int change_mask
+                                      = FWD_AND_REV_CHG,
                                       short family = AF_INET) {
         NameRemoveStubPtr name_remove = (family == AF_INET ?
                                          makeTransaction4(change_mask) :
@@ -319,65 +319,64 @@ TEST_F(NameRemoveTransactionTest, dictionaryCheck) {
     ASSERT_NO_THROW(name_remove->verifyStates());
 }
 
-#if 0
 
-/// @brief Tests construction of a DNS update request for adding a forward
-/// dns entry.
-TEST_F(NameRemoveTransactionTest, buildForwardAdd) {
+/// @brief Tests construction of a DNS update request for removing forward
+/// DNS address RRs.
+TEST_F(NameRemoveTransactionTest, buildRemoveFwdAddressRequest) {
     // Create a IPv4 forward add transaction.
     // Verify the request builds without error.
     // and then verify the request contents.
     NameRemoveStubPtr name_remove;
-    ASSERT_NO_THROW(name_remove = makeTransaction4());
-    ASSERT_NO_THROW(name_remove->buildAddFwdAddressRequest());
-    checkForwardAddRequest(*name_remove);
+    ASSERT_NO_THROW(name_remove = makeTransaction4(FORWARD_CHG));
+    (name_remove->buildRemoveFwdAddressRequest());
+    ASSERT_NO_THROW(name_remove->buildRemoveFwdAddressRequest());
+    checkRemoveFwdAddressRequest(*name_remove);
 
     // Create a IPv6 forward add transaction.
     // Verify the request builds without error.
     // and then verify the request contents.
-    ASSERT_NO_THROW(name_remove = makeTransaction6());
-    ASSERT_NO_THROW(name_remove->buildAddFwdAddressRequest());
-    checkForwardAddRequest(*name_remove);
+    ASSERT_NO_THROW(name_remove = makeTransaction6(FORWARD_CHG));
+    ASSERT_NO_THROW(name_remove->buildRemoveFwdAddressRequest());
+    checkRemoveFwdAddressRequest(*name_remove);
 }
 
-/// @brief Tests construction of a DNS update request for replacing a forward
-/// dns entry.
-TEST_F(NameRemoveTransactionTest, buildReplaceFwdAddressRequest) {
+/// @brief Tests construction of a DNS update request for removing forward
+/// dns RR entries.
+TEST_F(NameRemoveTransactionTest, buildRemoveFwdRRsRequest) {
     // Create a IPv4 forward replace transaction.
     // Verify the request builds without error.
     // and then verify the request contents.
     NameRemoveStubPtr name_remove;
-    ASSERT_NO_THROW(name_remove = makeTransaction4());
-    ASSERT_NO_THROW(name_remove->buildReplaceFwdAddressRequest());
-    checkForwardReplaceRequest(*name_remove);
+    ASSERT_NO_THROW(name_remove = makeTransaction4(FORWARD_CHG));
+    ASSERT_NO_THROW(name_remove->buildRemoveFwdRRsRequest());
+    checkRemoveFwdRRsRequest(*name_remove);
 
     // Create a IPv6 forward replace transaction.
     // Verify the request builds without error.
     // and then verify the request contents.
-    ASSERT_NO_THROW(name_remove = makeTransaction6());
-    ASSERT_NO_THROW(name_remove->buildReplaceFwdAddressRequest());
-    checkForwardReplaceRequest(*name_remove);
+    ASSERT_NO_THROW(name_remove = makeTransaction6(FORWARD_CHG));
+    ASSERT_NO_THROW(name_remove->buildRemoveFwdRRsRequest());
+    checkRemoveFwdRRsRequest(*name_remove);
 }
 
-/// @brief Tests the construction of a DNS update request for replacing a
+/// @brief Tests the construction of a DNS update request for removing a
 /// reverse dns entry.
-TEST_F(NameRemoveTransactionTest, buildReplaceRevPtrsRequest) {
+TEST_F(NameRemoveTransactionTest, buildRemoveRevPtrsRequest) {
     // Create a IPv4 reverse replace transaction.
     // Verify the request builds without error.
     // and then verify the request contents.
     NameRemoveStubPtr name_remove;
-    ASSERT_NO_THROW(name_remove = makeTransaction4());
-    ASSERT_NO_THROW(name_remove->buildReplaceRevPtrsRequest());
-    checkReverseReplaceRequest(*name_remove);
+    ASSERT_NO_THROW(name_remove = makeTransaction4(REVERSE_CHG));
+    ASSERT_NO_THROW(name_remove->buildRemoveRevPtrsRequest());
+    checkRemoveRevPtrsRequest(*name_remove);
 
     // Create a IPv6 reverse replace transaction.
     // Verify the request builds without error.
     // and then verify the request contents.
-    ASSERT_NO_THROW(name_remove = makeTransaction6());
-    ASSERT_NO_THROW(name_remove->buildReplaceRevPtrsRequest());
-    checkReverseReplaceRequest(*name_remove);
+    ASSERT_NO_THROW(name_remove = makeTransaction6(REVERSE_CHG));
+    ASSERT_NO_THROW(name_remove->buildRemoveRevPtrsRequest());
+    checkRemoveRevPtrsRequest(*name_remove);
 }
-#endif
 
 // Tests the readyHandler functionality.
 // It verifies behavior for the following scenarios:
@@ -556,7 +555,7 @@ TEST_F(NameRemoveTransactionTest, removingFwdAddrsHandler_FwdOnlyOK) {
     EXPECT_NO_THROW(name_remove->removingFwdAddrsHandler());
 
     // Verify that an update message was constructed properly.
-    checkForwardRemoveAddrsRequest(*name_remove);
+    checkRemoveFwdAddressRequest(*name_remove);
 
     // 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.
@@ -649,7 +648,7 @@ TEST_F(NameRemoveTransactionTest, removingFwdAddrsHandler_OtherRcode) {
     EXPECT_NO_THROW(name_remove->removingFwdAddrsHandler());
 
     // Simulate receiving server rejection response. Per RFC, anything other
-    // than no error or FQDN not in use is failure. Arbitrarily choosing 
+    // than no error or FQDN not in use is failure. Arbitrarily choosing
     // refused.
     name_remove->fakeResponse(DNSClient::SUCCESS, dns::Rcode::REFUSED());
 
@@ -843,7 +842,7 @@ TEST_F(NameRemoveTransactionTest, removingFwdRRsHandler_FwdOnlyOK) {
     EXPECT_NO_THROW(name_remove->removingFwdRRsHandler());
 
     // Verify that an update message was constructed properly.
-    checkForwardRemoveRRsRequest(*name_remove);
+    checkRemoveFwdRRsRequest(*name_remove);
 
     // 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.
@@ -945,7 +944,7 @@ TEST_F(NameRemoveTransactionTest, removingFwdRRsHandler_FwdAndRevOK) {
               name_remove->getNextEvent());
 }
 
-// Tests replacingFwdAddrsHandler with the following scenario:
+// Tests removingFwdAddrsHandler with the following scenario:
 //
 //  The request includes a forward and reverse change.
 //  Initial posted event is UPDATE_OK_EVT.
@@ -1006,7 +1005,7 @@ TEST_F(NameRemoveTransactionTest, removingFwdRRsHandler_OtherRcode) {
     EXPECT_NO_THROW(name_remove->removingFwdRRsHandler());
 
     // Simulate receiving server rejection response. Per RFC, anything other
-    // than no error is failure (we are also treating FQDN not in use is 
+    // than no error is failure (we are also treating FQDN not in use is
     // success). Arbitrarily choosing refused.
     name_remove->fakeResponse(DNSClient::SUCCESS, dns::Rcode::REFUSED());
 
@@ -1084,7 +1083,7 @@ TEST_F(NameRemoveTransactionTest, removingFwdRRsHandler_Timeout) {
             EXPECT_EQ(NameChangeTransaction::SERVER_SELECTED_EVT,
                     name_remove->getNextEvent());
         } else {
-            // Server retries should be exhausted. 
+            // Server retries should be exhausted.
             // We should abandon the transaction.
             EXPECT_EQ(NameChangeTransaction::PROCESS_TRANS_FAILED_ST,
                       name_remove->getCurrState());
@@ -1154,7 +1153,7 @@ TEST_F(NameRemoveTransactionTest, removingFwdRRsHandler_InvalidResponse) {
             EXPECT_EQ(NameChangeTransaction::SERVER_SELECTED_EVT,
                     name_remove->getNextEvent());
         } else {
-            // Server retries should be exhausted. 
+            // Server retries should be exhausted.
             // We should abandon the transaction.
             EXPECT_EQ(NameChangeTransaction::PROCESS_TRANS_FAILED_ST,
                       name_remove->getCurrState());
@@ -1232,7 +1231,7 @@ TEST_F(NameRemoveTransactionTest, selectingRevServerHandler) {
                  NameRemoveTransactionError);
 }
 
-//************************** replacingRevPtrsHandler tests *****************
+//************************** removingRevPtrsHandler tests *****************
 
 // Tests that removingRevPtrsHandler rejects invalid events.
 TEST_F(NameRemoveTransactionTest, removingRevPtrsHandler_InvalidEvent) {
@@ -1249,14 +1248,14 @@ TEST_F(NameRemoveTransactionTest, removingRevPtrsHandler_InvalidEvent) {
                  NameRemoveTransactionError);
 }
 
-// Tests replacingRevPtrsHandler with the following scenario:
+// Tests removingRevPtrsHandler with the following scenario:
 //
 //  The request includes only a reverse change.
 //  Initial posted event is SERVER_SELECTED_EVT.
 //  The update request is sent without error.
 //  A server response is received which indicates successful update.
 //
-TEST_F(NameRemoveTransactionTest, replacingRevPtrsHandler_RevOnlyOK) {
+TEST_F(NameRemoveTransactionTest, removingRevPtrsHandler_RevOnlyOK) {
     NameRemoveStubPtr name_remove;
     // Create and prep a transaction, poised to run the handler.
     ASSERT_NO_THROW(name_remove =
@@ -1277,7 +1276,7 @@ TEST_F(NameRemoveTransactionTest, replacingRevPtrsHandler_RevOnlyOK) {
     EXPECT_NO_THROW(name_remove->removingRevPtrsHandler());
 
     // Verify that an update message was constructed properly.
-    checkReverseRemoveRequest(*name_remove);
+    checkRemoveRevPtrsRequest(*name_remove);
 
     // 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.
@@ -1304,14 +1303,14 @@ TEST_F(NameRemoveTransactionTest, replacingRevPtrsHandler_RevOnlyOK) {
               name_remove->getNextEvent());
 }
 
-// Tests replacingRevPtrsHandler with the following scenario:
+// Tests removingRevPtrsHandler with the following scenario:
 //
 //  The request includes only a reverse change.
 //  Initial posted event is SERVER_SELECTED_EVT.
 //  The update request is sent without error.
-//  A server response is received which indicates FQDN is NOT in use. 
+//  A server response is received which indicates FQDN is NOT in use.
 //
-TEST_F(NameRemoveTransactionTest, replacingRevPtrsHandler_FqdnNotInUse) {
+TEST_F(NameRemoveTransactionTest, removingRevPtrsHandler_FqdnNotInUse) {
     NameRemoveStubPtr name_remove;
     // Create and prep a transaction, poised to run the handler.
     ASSERT_NO_THROW(name_remove =
@@ -1332,7 +1331,7 @@ TEST_F(NameRemoveTransactionTest, replacingRevPtrsHandler_FqdnNotInUse) {
     EXPECT_NO_THROW(name_remove->removingRevPtrsHandler());
 
     // Verify that an update message was constructed properly.
-    checkReverseRemoveRequest(*name_remove);
+    checkRemoveRevPtrsRequest(*name_remove);
 
     // 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.
@@ -1565,7 +1564,7 @@ TEST_F(NameRemoveTransactionTest, processRemoveOkHandler) {
                     prepHandlerTest(NameChangeTransaction::PROCESS_TRANS_OK_ST,
                                     StateModel::NOP_EVT));
     // Running the handler should throw.
-    EXPECT_THROW(name_remove->processRemoveOkHandler(), 
+    EXPECT_THROW(name_remove->processRemoveOkHandler(),
                  NameRemoveTransactionError);
 }
 
@@ -1601,7 +1600,7 @@ TEST_F(NameRemoveTransactionTest, processRemoveFailedHandler) {
                                     PROCESS_TRANS_FAILED_ST,
                                     StateModel::NOP_EVT));
     // Running the handler should throw.
-    EXPECT_THROW(name_remove->processRemoveFailedHandler(), 
+    EXPECT_THROW(name_remove->processRemoveFailedHandler(),
                  NameRemoveTransactionError);
 }
 
@@ -1757,7 +1756,7 @@ TEST_F(NameRemoveTransactionTest, removingRevPtrsHandler_SendUpdateException) {
 //  Initial posted event is SERVER_SELECTED_EVT.
 //  The request build fails due to an unexpected exception.
 //
-TEST_F(NameRemoveTransactionTest, 
+TEST_F(NameRemoveTransactionTest,
        removingFwdAddrsHandler_BuildRequestException) {
     NameRemoveStubPtr name_remove;
     // Create and prep a transaction, poised to run the handler.
@@ -1797,7 +1796,7 @@ TEST_F(NameRemoveTransactionTest,
 //  Initial posted event is SERVER_SELECTED_EVT.
 //  The request build fails due to an unexpected exception.
 //
-TEST_F(NameRemoveTransactionTest, 
+TEST_F(NameRemoveTransactionTest,
        removingFwdRRsHandler_BuildRequestException) {
     NameRemoveStubPtr name_remove;
     // Create and prep a transaction, poised to run the handler.
@@ -1837,7 +1836,7 @@ TEST_F(NameRemoveTransactionTest,
 //  Initial posted event is SERVER_SELECTED_EVT.
 //  The request build fails due to an unexpected exception.
 //
-TEST_F(NameRemoveTransactionTest, 
+TEST_F(NameRemoveTransactionTest,
        removingRevPTRsHandler_BuildRequestException) {
     NameRemoveStubPtr name_remove;
     // Create and prep a transaction, poised to run the handler.

+ 119 - 14
src/bin/d2/tests/nc_test_utils.cc

@@ -29,6 +29,9 @@ namespace d2 {
 const char* TEST_DNS_SERVER_IP = "127.0.0.1";
 size_t TEST_DNS_SERVER_PORT = 5301;
 
+const bool HAS_RDATA = true;
+const bool NO_RDATA = false;
+
 //*************************** FauxServer class ***********************
 
 FauxServer::FauxServer(asiolink::IOService& io_service,
@@ -152,21 +155,21 @@ TransactionTest::TransactionTest()
 TransactionTest::~TransactionTest() {
 }
 
-void 
+void
 TransactionTest::runTimedIO(int run_time) {
     run_time_ = run_time;
     timer_.setup(boost::bind(&TransactionTest::timesUp, this), run_time_);
     io_service_->run();
 }
 
-void 
+void
 TransactionTest::timesUp() {
     io_service_->stop();
     FAIL() << "Test Time: " << run_time_ << " expired";
 }
 
-void 
-TransactionTest::setupForIPv4Transaction(dhcp_ddns::NameChangeType chg_type, 
+void
+TransactionTest::setupForIPv4Transaction(dhcp_ddns::NameChangeType chg_type,
                                          int change_mask) {
     const char* msg_str =
         "{"
@@ -194,7 +197,7 @@ TransactionTest::setupForIPv4Transaction(dhcp_ddns::NameChangeType chg_type,
     } else {
         // Create the forward domain and then its servers.
         forward_domain_ = makeDomain("example.com.");
-        addDomainServer(forward_domain_, "forward.example.com", 
+        addDomainServer(forward_domain_, "forward.example.com",
                         "127.0.0.1", 5301);
         addDomainServer(forward_domain_, "forward2.example.com",
                         "127.0.0.1", 5302);
@@ -215,7 +218,7 @@ TransactionTest::setupForIPv4Transaction(dhcp_ddns::NameChangeType chg_type,
     }
 }
 
-void 
+void
 TransactionTest::setupForIPv6Transaction(dhcp_ddns::NameChangeType chg_type,
                                          int change_mask) {
     const char* msg_str =
@@ -289,13 +292,15 @@ checkZone(const D2UpdateMessagePtr& request, const std::string& exp_zone_name) {
 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) {
+              unsigned int exp_ttl, dhcp_ddns::NameChangeRequestPtr ncr,
+              bool has_rdata) {
     // 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()) {
+    if ((!has_rdata) || 
+       (exp_type == dns::RRType::ANY() || exp_class == dns::RRClass::ANY())) {
         // ANY types do not have RData
         ASSERT_EQ(0, rrset->getRdataCount());
         return;
@@ -360,7 +365,7 @@ void addDomainServer(DdnsDomainPtr& domain, const std::string& name,
 
 // Verifies that the contents of the given transaction's  DNS update request
 // is correct for adding a forward DNS entry
-void checkForwardAddRequest(NameChangeTransaction& tran) {
+void checkAddFwdAddressRequest(NameChangeTransaction& tran) {
     const D2UpdateMessagePtr& request = tran.getDnsUpdateRequest();
     ASSERT_TRUE(request);
 
@@ -407,7 +412,7 @@ void checkForwardAddRequest(NameChangeTransaction& tran) {
 
 // Verifies that the contents of the given transaction's  DNS update request
 // is correct for replacing a forward DNS entry
-void checkForwardReplaceRequest(NameChangeTransaction& tran) {
+void checkReplaceFwdAddressRequest(NameChangeTransaction& tran) {
     const D2UpdateMessagePtr& request = tran.getDnsUpdateRequest();
     ASSERT_TRUE(request);
 
@@ -463,7 +468,7 @@ void checkForwardReplaceRequest(NameChangeTransaction& tran) {
 
 // Verifies that the contents of the given transaction's  DNS update request
 // is correct for replacing a reverse DNS entry
-void checkReverseReplaceRequest(NameChangeTransaction& tran) {
+void checkReplaceRevPtrsRequest(NameChangeTransaction& tran) {
     const D2UpdateMessagePtr& request = tran.getDnsUpdateRequest();
     ASSERT_TRUE(request);
 
@@ -521,31 +526,131 @@ void checkReverseReplaceRequest(NameChangeTransaction& tran) {
     ASSERT_NO_THROW(request->toWire(renderer));
 }
 
-void checkForwardRemoveAddrsRequest(NameChangeTransaction& tran) {
+void checkRemoveFwdAddressRequest(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();
+
+    // Verify the zone section.
+    checkZone(request, exp_zone_name);
+
+    // Verify there is 1 RR in the PREREQUISITE Section.
+    checkRRCount(request, D2UpdateMessage::SECTION_PREREQUISITE, 1);
+
+    // Verify the DHCID matching assertion RR.
+    dns::RRsetPtr rrset;
+    ASSERT_TRUE(rrset = getRRFromSection(request, D2UpdateMessage::
+                                                  SECTION_PREREQUISITE, 0));
+    checkRR(rrset, exp_fqdn, dns::RRClass::IN(), dns::RRType::DHCID(),
+            0, ncr);
+
+    // Verify there is 1 RR in the UPDATE Section.
+    checkRRCount(request, D2UpdateMessage::SECTION_UPDATE, 1);
+
+    // Verify the FQDN/IP delete RR.
+    const dns::RRType& exp_ip_rr_type = tran.getAddressRRType();
+    ASSERT_TRUE(rrset = getRRFromSection(request, D2UpdateMessage::
+                                                  SECTION_UPDATE, 0));
+    checkRR(rrset, exp_fqdn, dns::RRClass::NONE(), exp_ip_rr_type,
+            0, ncr);
+
+    // Verify that it will render toWire without throwing.
+    dns::MessageRenderer renderer;
+    ASSERT_NO_THROW(request->toWire(renderer));
 }
 
-void checkForwardRemoveRRsRequest(NameChangeTransaction& tran) {
+void checkRemoveFwdRRsRequest(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();
+
+    // Verify the zone section.
+    checkZone(request, exp_zone_name);
+
+    // Verify there is 1 RR in the PREREQUISITE Section.
+    checkRRCount(request, D2UpdateMessage::SECTION_PREREQUISITE, 3);
+
+    // Verify the DHCID matches assertion.
+    dns::RRsetPtr rrset;
+    ASSERT_TRUE(rrset = getRRFromSection(request, D2UpdateMessage::
+                                                  SECTION_PREREQUISITE, 0));
+    checkRR(rrset, exp_fqdn, dns::RRClass::IN(), dns::RRType::DHCID(),
+            0, ncr);
+
+    // Verify the NO A RRs assertion.
+    ASSERT_TRUE(rrset = getRRFromSection(request, D2UpdateMessage::
+                                                  SECTION_PREREQUISITE, 1));
+    checkRR(rrset, exp_fqdn, dns::RRClass::NONE(), dns::RRType::A(),
+            0, ncr, NO_RDATA);
+
+    // Verify the NO AAAA RRs assertion.
+    ASSERT_TRUE(rrset = getRRFromSection(request, D2UpdateMessage::
+                                                  SECTION_PREREQUISITE, 2));
+    checkRR(rrset, exp_fqdn, dns::RRClass::NONE(), dns::RRType::AAAA(),
+            0, ncr, NO_RDATA);
+
+    // Verify there is 1 RR in the UPDATE Section.
+    checkRRCount(request, D2UpdateMessage::SECTION_UPDATE, 1);
+
+    // Verify the delete all for the FQDN RR.
+    ASSERT_TRUE(rrset = getRRFromSection(request, D2UpdateMessage::
+                                                  SECTION_UPDATE, 0));
+    checkRR(rrset, exp_fqdn, dns::RRClass::ANY(), dns::RRType::ANY(),
+            0, ncr);
+
+    // Verify that it will render toWire without throwing.
+    dns::MessageRenderer renderer;
+    ASSERT_NO_THROW(request->toWire(renderer));
 }
 
-void checkReverseRemoveRequest(NameChangeTransaction& tran) {
+void checkRemoveRevPtrsRequest(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 is 1 RR in the PREREQUISITE Section.
+    checkRRCount(request, D2UpdateMessage::SECTION_PREREQUISITE, 1);
+
+    // Verify the FQDN-PTRNAME assertion RR.
+    dns::RRsetPtr rrset;
+    ASSERT_TRUE(rrset = getRRFromSection(request, D2UpdateMessage::
+                                                  SECTION_PREREQUISITE, 0));
+    checkRR(rrset, exp_rev_addr, dns::RRClass::IN(), dns::RRType::PTR(),
+            0, ncr);
+
+    // Verify there is 1 RR in the UPDATE Section.
+    checkRRCount(request, D2UpdateMessage::SECTION_UPDATE, 1);
+
+    // Verify the delete all for the FQDN RR.
+    ASSERT_TRUE(rrset = getRRFromSection(request, D2UpdateMessage::
+                                                  SECTION_UPDATE, 0));
+    checkRR(rrset, exp_rev_addr, dns::RRClass::ANY(), dns::RRType::ANY(),
+            0, ncr);
+
+    // Verify that it will render toWire without throwing.
+    dns::MessageRenderer renderer;
+    ASSERT_NO_THROW(request->toWire(renderer));
 }
 
 

+ 25 - 15
src/bin/d2/tests/nc_test_utils.h

@@ -95,7 +95,7 @@ public:
                         const dns::Rcode& response_rcode);
 };
 
-/// @brief Base class Test fixture for testing transactions. 
+/// @brief Base class Test fixture for testing transactions.
 class TransactionTest : public ::testing::Test {
 public:
     IOServicePtr io_service_;
@@ -105,9 +105,10 @@ public:
     asiolink::IntervalTimer timer_;
     int run_time_;
 
-    static const unsigned int FORWARD_CHG;
-    static const unsigned int REVERSE_CHG;
-    static const unsigned int FWD_AND_REV_CHG; 
+    /// #brief constants used to specify change directions for a transaction.
+    static const unsigned int FORWARD_CHG;      // Only forward change.
+    static const unsigned int REVERSE_CHG;      // Only reverse change.
+    static const unsigned int FWD_AND_REV_CHG;  // Both forward and reverse.
 
     TransactionTest();
     virtual ~TransactionTest();
@@ -126,7 +127,6 @@ public:
     /// Stops the IOSerivce and fails the current test.
     virtual void timesUp();
 
-    /// @todo
     /// @brief Creates a transaction which requests an IPv4 DNS update.
     ///
     /// The transaction is constructed around a predefined (i.e. "canned")
@@ -134,11 +134,13 @@ public:
     /// changes requested.  Based upon the change mask, the transaction
     /// will have either the forward, reverse, or both domains populated.
     ///
+    /// @param change_type selects the type of change requested, CHG_ADD or
+    /// CHG_REMOVE.
     /// @param change_mask determines which change directions are requested
-    void setupForIPv4Transaction(dhcp_ddns::NameChangeType chg_type,
+    /// FORWARD_CHG, REVERSE_CHG, or FWD_AND_REV_CHG.
+    void setupForIPv4Transaction(dhcp_ddns::NameChangeType change_type,
                                  int change_mask);
 
-    /// @todo
     /// @brief Creates a transaction which requests an IPv6 DNS update.
     ///
     /// The transaction is constructed around a predefined (i.e. "canned")
@@ -146,8 +148,11 @@ public:
     /// changes requested.  Based upon the change mask, the transaction
     /// will have either the forward, reverse, or both domains populated.
     ///
+    /// @param change_type selects the type of change requested, CHG_ADD or
+    /// CHG_REMOVE.
     /// @param change_mask determines which change directions are requested
-    void setupForIPv6Transaction(dhcp_ddns::NameChangeType chg_type, 
+    /// FORWARD_CHG, REVERSE_CHG, or FWD_AND_REV_CHG.
+    void setupForIPv6Transaction(dhcp_ddns::NameChangeType change_type,
                                  int change_mask);
 };
 
@@ -182,9 +187,14 @@ extern void checkZone(const D2UpdateMessagePtr& request,
 /// @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
+/// @param has_rdata if true, RRset's rdata will be checked based on it's
+/// RRType.  Set this to false if the RRset's type supports Rdata but it does
+/// not contain it.  For instance, prerequisites of type NONE have no Rdata
+/// where udpates of type NONE may.
 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);
+                    unsigned int exp_ttl, dhcp_ddns::NameChangeRequestPtr ncr,
+                    bool has_rdata=true);
 
 /// @brief Fetches an RR(set) from a given section of a request
 ///
@@ -221,7 +231,7 @@ extern dns::RRsetPtr getRRFromSection(const D2UpdateMessagePtr& request,
 /// adding a forward DNS mapping.
 ///
 /// @param tran Transaction containing the request to be verified.
-extern void checkForwardAddRequest(NameChangeTransaction& tran);
+extern void checkAddFwdAddressRequest(NameChangeTransaction& tran);
 
 /// @brief Verifies a forward mapping replacement DNS update request
 ///
@@ -229,7 +239,7 @@ extern void checkForwardAddRequest(NameChangeTransaction& tran);
 /// replacing a forward DNS mapping.
 ///
 /// @param tran Transaction containing the request to be verified.
-extern void checkForwardReplaceRequest(NameChangeTransaction& tran);
+extern void checkReplaceFwdAddressRequest(NameChangeTransaction& tran);
 
 /// @brief Verifies a reverse mapping replacement DNS update request
 ///
@@ -237,7 +247,7 @@ extern void checkForwardReplaceRequest(NameChangeTransaction& tran);
 /// replacing a reverse DNS mapping.
 ///
 /// @param tran Transaction containing the request to be verified.
-extern void checkReverseReplaceRequest(NameChangeTransaction& tran);
+extern void checkReplaceRevPtrsRequest(NameChangeTransaction& tran);
 
 /// @brief Verifies a forward address removal DNS update request
 ///
@@ -245,7 +255,7 @@ extern void checkReverseReplaceRequest(NameChangeTransaction& tran);
 /// removing the forward address DNS entry.
 ///
 /// @param tran Transaction containing the request to be verified.
-extern void checkForwardRemoveAddrsRequest(NameChangeTransaction& tran);
+extern void checkRemoveFwdAddressRequest(NameChangeTransaction& tran);
 
 /// @brief Verifies a forward RR removal DNS update request
 ///
@@ -253,7 +263,7 @@ extern void checkForwardRemoveAddrsRequest(NameChangeTransaction& tran);
 /// removing forward RR DNS entries.
 ///
 /// @param tran Transaction containing the request to be verified.
-extern void checkForwardRemoveRRsRequest(NameChangeTransaction& tran);
+extern void checkRemoveFwdRRsRequest(NameChangeTransaction& tran);
 
 /// @brief Verifies a reverse mapping removal DNS update request
 ///
@@ -261,7 +271,7 @@ extern void checkForwardRemoveRRsRequest(NameChangeTransaction& tran);
 /// removing a reverse DNS mapping.
 ///
 /// @param tran Transaction containing the request to be verified.
-extern void checkReverseRemoveRequest(NameChangeTransaction& tran);
+extern void checkRemoveRevPtrsRequest(NameChangeTransaction& tran);
 
 
 /// @brief Creates a NameChangeRequest from JSON string.

+ 7 - 26
src/bin/d2/tests/nc_trans_unittests.cc

@@ -997,19 +997,10 @@ TEST_F(NameChangeTransactionTest, addLeaseAddressRData) {
     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)));
+    dns::RRsetPtr rrset(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.
@@ -1026,14 +1017,9 @@ TEST_F(NameChangeTransactionTest, addDhcidRdata) {
     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)));
+    dns::RRsetPtr rrset(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.
@@ -1053,14 +1039,9 @@ TEST_F(NameChangeTransactionTest, addPtrRdata) {
     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)));
+    dns::RRsetPtr rrset (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.