[3341] Allow b10-dchp-ddns to optionally support updates in a given direction

Added methods to D2CfgMgr to return whether or not forward or reverse
updates are enabled. Altered D2UpdateMgr::makeTransaction to utlilize
these new methods. Added additional logs and unittests.

src/bin/d2/d2_cfg_mgr.cc

@@ -72,6 +72,18 @@ D2CfgMgr::D2CfgMgr() : DCfgMgrBase(DCfgContextBasePtr(new D2CfgContext())) {
 D2CfgMgr::~D2CfgMgr() {
 }
 
+bool
+D2CfgMgr::forwardUpdatesEnabled() {
+    // Forward updates are not enabled if no forward servers are defined.
+    return (getD2CfgContext()->getForwardMgr()->size() > 0);
+}
+
+bool
+D2CfgMgr::reverseUpdatesEnabled() {
+    // Reverse updates are not enabled if no revese servers are defined.
+    return (getD2CfgContext()->getReverseMgr()->size() > 0);
+}
+
 bool
 D2CfgMgr::matchForward(const std::string& fqdn, DdnsDomainPtr& domain) {
     if (fqdn.empty()) {

src/bin/d2/d2_cfg_mgr.h

@@ -126,6 +126,16 @@ public:
         return (boost::dynamic_pointer_cast<D2CfgContext>(getContext()));
     }
 
+    /// @brief Returns whether not forward updates are enabled.
+    ///
+    /// @return true if forward updates are enabled, false otherwise.
+    bool forwardUpdatesEnabled();
+
+    /// @brief Returns whether not reverse updates are enabled.
+    ///
+    /// @return true if reverse updates are enabled, false otherwise.
+    bool reverseUpdatesEnabled();
+
     /// @brief Matches a given FQDN to a forward domain.
     ///
     /// This calls the matchDomain method of the forward domain manager to

src/bin/d2/d2_update_mgr.cc

@@ -132,34 +132,61 @@ D2UpdateMgr::makeTransaction(dhcp_ddns::NameChangeRequestPtr& next_ncr) {
             << key.toStr());
     }
 
+    int direction_count = 0;
     // If forward change is enabled, match to forward servers.
     DdnsDomainPtr forward_domain;
     if (next_ncr->isForwardChange()) {
-        bool matched = cfg_mgr_->matchForward(next_ncr->getFqdn(),
-                                             forward_domain);
-        // Could not find a match for forward DNS server. Log it and get out.
-        // This has the net affect of dropping the request on the floor.
-        if (!matched) {
-            LOG_ERROR(dctl_logger, DHCP_DDNS_NO_FWD_MATCH_ERROR)
-                      .arg(next_ncr->getFqdn());
-            return;
+        if (!cfg_mgr_->forwardUpdatesEnabled()) {
+            next_ncr->setForwardChange(false);
+            LOG_DEBUG(dctl_logger, DBGLVL_TRACE_DETAIL_DATA,
+                      DHCP_DDNS_FWD_REQUEST_IGNORED).arg(next_ncr->toText());
+        } else {
+            bool matched = cfg_mgr_->matchForward(next_ncr->getFqdn(),
+                                                  forward_domain);
+            // Could not find a match for forward DNS server. Log it and get
+            // out. This has the net affect of dropping the request on the
+            // floor.
+            if (!matched) {
+                LOG_ERROR(dctl_logger, DHCP_DDNS_NO_FWD_MATCH_ERROR)
+                          .arg(next_ncr->toText());
+                return;
+            }
+
+            ++direction_count;
         }
     }
 
     // If reverse change is enabled, match to reverse servers.
     DdnsDomainPtr reverse_domain;
     if (next_ncr->isReverseChange()) {
-        bool matched = cfg_mgr_->matchReverse(next_ncr->getIpAddress(),
-                                              reverse_domain);
-        // Could not find a match for reverse DNS server. Log it and get out.
-        // This has the net affect of dropping the request on the floor.
-        if (!matched) {
-            LOG_ERROR(dctl_logger, DHCP_DDNS_NO_REV_MATCH_ERROR)
-                      .arg(next_ncr->getIpAddress());
-            return;
+        if (!cfg_mgr_->reverseUpdatesEnabled()) {
+            next_ncr->setReverseChange(false);
+            LOG_DEBUG(dctl_logger, DBGLVL_TRACE_DETAIL_DATA,
+                      DHCP_DDNS_REV_REQUEST_IGNORED).arg(next_ncr->toText());
+        } else {
+            bool matched = cfg_mgr_->matchReverse(next_ncr->getIpAddress(),
+                                                  reverse_domain);
+            // Could not find a match for reverse DNS server. Log it and get
+            // out. This has the net affect of dropping the request on the
+            // floor.
+            if (!matched) {
+                LOG_ERROR(dctl_logger, DHCP_DDNS_NO_REV_MATCH_ERROR)
+                          .arg(next_ncr->toText());
+                return;
+            }
+
+            ++direction_count;
         }
     }
 
+    // If there is nothing to actually do, then the request falls on the floor.
+    // Should we log this?
+    if (!direction_count) {
+        LOG_DEBUG(dctl_logger, DBGLVL_TRACE_DETAIL_DATA,
+                  DHCP_DDNS_REQUEST_DROPPED).arg(next_ncr->toText());
+        return;
+    }
+
     // We matched to the required servers, so construct the transaction.
     // @todo If multi-threading is implemented, one would pass in an
     // empty IOServicePtr, rather than our instance value.  This would cause

src/bin/d2/d2_update_mgr.h

@@ -147,13 +147,21 @@ protected:
 
     /// @brief Create a new transaction for the given request.
     ///
-    /// This method will attempt to match the request to a list of configured
-    /// DNS servers.  If a list of servers is found, it will instantiate a
-    /// transaction for it and add the transaction to the transaction list.
+    /// This method will attempt to match the request to suitable DNS servers.
+    /// If matching servers are found, it will instantiate a transaction for
+    /// the requests, add the transaction to the transaction list, and start
+    /// the transaction.
     ///
-    /// If no servers are found that match the request, this constitutes a
-    /// configuration error.  The error will be logged and the request will
-    /// be discarded.
+    /// If updates in a given direction are disabled requests for updates in
+    /// that direction will be ignored.  For example: If a request is received
+    /// which asks for updates both directions but only forward updates are
+    /// enabled; only the forward update will be attempted.  Effectively, the
+    /// request will be treated as if it only asked for forward updates.
+    ///
+    /// If updates in a given direction are enabled, and a request asks for
+    /// updates in that direction, failing to match the request to a list
+    /// of servers is an error which will be logged and the request will be
+    /// discarded.
     ///
     /// @param ncr the NameChangeRequest for which to create a transaction.
     ///
@@ -162,7 +170,7 @@ protected:
     void makeTransaction(isc::dhcp_ddns::NameChangeRequestPtr& ncr);
 
 public:
-    /// @brief Gets the UpdateMgr's IOService.
+    /// @brief Gets the D2UpdateMgr's IOService.
     ///
     /// @return returns a reference to the IOService
     const IOServicePtr& getIOService() {

src/bin/d2/tests/d2_cfg_mgr_unittests.cc

@@ -1008,6 +1008,10 @@ TEST_F(D2CfgMgrTest, fullConfig) {
         EXPECT_EQ(3, count);
     }
 
+    // Test directional update flags.
+    EXPECT_TRUE(cfg_mgr_->forwardUpdatesEnabled());
+    EXPECT_TRUE(cfg_mgr_->reverseUpdatesEnabled());
+
     // Verify that parsing the exact same configuration a second time
     // does not cause a duplicate value errors. 
     answer_ = cfg_mgr_->parseConfig(config_set_);
@@ -1061,6 +1065,10 @@ TEST_F(D2CfgMgrTest, forwardMatch) {
     D2CfgContextPtr context;
     ASSERT_NO_THROW(context = cfg_mgr_->getD2CfgContext());
 
+    // Test directional update flags.
+    EXPECT_TRUE(cfg_mgr_->forwardUpdatesEnabled());
+    EXPECT_FALSE(cfg_mgr_->reverseUpdatesEnabled());
+
     DdnsDomainPtr match;
     // Verify that an exact match works.
     EXPECT_TRUE(cfg_mgr_->matchForward("tmark.org", match));
@@ -1227,6 +1235,10 @@ TEST_F(D2CfgMgrTest, matchReverse) {
     D2CfgContextPtr context;
     ASSERT_NO_THROW(context = cfg_mgr_->getD2CfgContext());
 
+    // Test directional update flags.
+    EXPECT_FALSE(cfg_mgr_->forwardUpdatesEnabled());
+    EXPECT_TRUE(cfg_mgr_->reverseUpdatesEnabled());
+
     DdnsDomainPtr match;
 
     // Verify an exact match.

src/bin/d2/tests/d2_update_mgr_unittests.cc

@@ -347,6 +347,109 @@ TEST_F(D2UpdateMgrTest, transactionList) {
     EXPECT_NO_THROW(update_mgr_->removeTransaction(ncr->getDhcid()));
 }
 
+/// @brief Checks transaction creation when both update directions are enabled.
+/// Verifies that when both directions are enabled and servers are matched to
+/// the request, that the transaction is created with both directions turned on.
+TEST_F(D2UpdateMgrTest, bothEnabled) {
+    // Grab a canned request for test purposes.
+    NameChangeRequestPtr& ncr = canned_ncrs_[0];
+    ncr->setReverseChange(true);
+
+    // Verify we are requesting both directions.
+    ASSERT_TRUE(ncr->isForwardChange());
+    ASSERT_TRUE(ncr->isReverseChange());
+
+    // Verify both both directions are enabled.
+    ASSERT_TRUE(cfg_mgr_->forwardUpdatesEnabled());
+    ASSERT_TRUE(cfg_mgr_->reverseUpdatesEnabled());
+
+    // Attempt to make a transaction.
+    ASSERT_NO_THROW(update_mgr_->makeTransaction(ncr));
+
+    // Verify we create a transaction with both directions turned on.
+    EXPECT_EQ(1, update_mgr_->getTransactionCount());
+    EXPECT_TRUE(ncr->isForwardChange());
+    EXPECT_TRUE(ncr->isReverseChange());
+}
+
+/// @brief Checks transaction creation when reverse updates are disabled.
+/// Verifies that when reverse updates are disabled, and there matching forward
+/// servers, that the transaction is still created but with only the forward
+/// direction turned on.
+TEST_F(D2UpdateMgrTest, reverseDisable) {
+    // Make a NCR which requests both directions.
+    NameChangeRequestPtr& ncr = canned_ncrs_[0];
+    ncr->setReverseChange(true);
+
+    // Wipe out forward domain list.
+    DdnsDomainMapPtr emptyDomains(new DdnsDomainMap());
+    cfg_mgr_->getD2CfgContext()->getReverseMgr()->setDomains(emptyDomains);
+
+    // Verify enable methods are correct.
+    ASSERT_TRUE(cfg_mgr_->forwardUpdatesEnabled());
+    ASSERT_FALSE(cfg_mgr_->reverseUpdatesEnabled());
+
+    // Attempt to make a transaction.
+    ASSERT_NO_THROW(update_mgr_->makeTransaction(ncr));
+
+    // Verify we create a transaction with only forward turned on.
+    EXPECT_EQ(1, update_mgr_->getTransactionCount());
+    EXPECT_TRUE(ncr->isForwardChange());
+    EXPECT_FALSE(ncr->isReverseChange());
+}
+
+/// @brief Checks transaction creation when forward updates are disabled.
+/// Verifies that when forward updates are disabled, and there matching reverse
+/// servers, that the transaction is still created but with only the reverse
+/// direction turned on.
+TEST_F(D2UpdateMgrTest, forwardDisabled) {
+    // Make a NCR which requests both directions.
+    NameChangeRequestPtr& ncr = canned_ncrs_[0];
+    ncr->setReverseChange(true);
+
+    // Wipe out forward domain list.
+    DdnsDomainMapPtr emptyDomains(new DdnsDomainMap());
+    cfg_mgr_->getD2CfgContext()->getForwardMgr()->setDomains(emptyDomains);
+
+    // Verify enable methods are correct.
+    ASSERT_FALSE(cfg_mgr_->forwardUpdatesEnabled());
+    ASSERT_TRUE(cfg_mgr_->reverseUpdatesEnabled());
+
+    // Attempt to make a transaction.
+    ASSERT_NO_THROW(update_mgr_->makeTransaction(ncr));
+
+    // Verify we create a transaction with only reverse turned on.
+    EXPECT_EQ(1, update_mgr_->getTransactionCount());
+    EXPECT_FALSE(ncr->isForwardChange());
+    EXPECT_TRUE(ncr->isReverseChange());
+}
+
+
+/// @brief Checks transaction creation when neither update direction is enabled.
+/// Verifies that transactions are not created when both forward and reverse
+/// directions are disabled.
+TEST_F(D2UpdateMgrTest, bothDisabled) {
+    // Grab a canned request for test purposes.
+    NameChangeRequestPtr& ncr = canned_ncrs_[0];
+    ncr->setReverseChange(true);
+    TransactionList::iterator pos;
+
+    // Wipe out both forward and reverse domain lists.
+    DdnsDomainMapPtr emptyDomains(new DdnsDomainMap());
+    cfg_mgr_->getD2CfgContext()->getForwardMgr()->setDomains(emptyDomains);
+    cfg_mgr_->getD2CfgContext()->getReverseMgr()->setDomains(emptyDomains);
+
+    // Verify enable methods are correct.
+    EXPECT_FALSE(cfg_mgr_->forwardUpdatesEnabled());
+    EXPECT_FALSE(cfg_mgr_->reverseUpdatesEnabled());
+
+    // Attempt to make a transaction.
+    ASSERT_NO_THROW(update_mgr_->makeTransaction(ncr));
+
+    // Verify that do not create a transaction.
+    EXPECT_EQ(0, update_mgr_->getTransactionCount());
+}
+
 /// @brief Tests D2UpdateManager's checkFinishedTransactions method.
 /// This test verifies that:
 /// 1. Completed transactions are removed from the transaction list.