[#544] leaseX-del commands may now update DNS

doc/sphinx/arm/hooks-lease-cmds.rst
    Updated leaseX-del documentation

src/hooks/dhcp/lease_cmds/lease_cmds.*
    LeaseCmdsImp::Parameters
    LeaseCmdsImpl::getParameters()
    - Added supportr for update-ddns

    LeaseCmdsImpl::lease4DelHandler()
    LeaseCmdsImpl::lease6DelHandler()
    - Added call t queueNCR()

src/hooks/dhcp/lease_cmds/tests/lease_cmds_unittest.cc
    TEST_F(LeaseCmdsTest, LeaseXDelBadUpdateDdnsParam)
    TEST_F(LeaseCmdsTest, lease4DnsRemoveD2Enabled)
    TEST_F(LeaseCmdsTest, lease4DnsRemoveD2Disabled)
    TEST_F(LeaseCmdsTest, lease6DnsRemoveD2Enabled)
    TEST_F(LeaseCmdsTest, lease6DnsRemoveD2Disabled)
    - new tests

Added a ChangeLog entry

ChangeLog

@@ -1,3 +1,9 @@
+1774.	[func]		tmark
+	leaseX-del commands now support a new parameter, update-ddns,
+	which instructs the server to remove DNS entries for a
+	lease after it has been deleted.
+	(Gitlab #544)
+
 1773.	[perf]		fdupont
 	Kea statistics now uses standard c++11 chrono library instead
 	of POSIX time library from boost.

doc/sphinx/arm/hooks-lease-cmds.rst

@@ -777,7 +777,7 @@ a pair of values: the type and the actual identifier. The currently
 supported identifiers are "hw-address" (IPv4 only), "client-id" (IPv4
 only), and "duid" (IPv6 only).
 
-An example command for deleting a lease by address is:
+An example command for deleting a lease by address is
 
 ::
 
@@ -801,6 +801,28 @@ An example IPv4 lease deletion by "hw-address" is:
      }
    }
 
+
+As of Kea 1.7.9, a new parameter, "update-ddns", is supported (IPv4 and IPv6).
+When ```true``` it instructs the server to queue a request to kea-dhcp-ddns to
+remove DNS entries after the lease is succesfully deleted if:
+
+- DDNS updating is enabled. (i.e. "dhcp-ddns":{ "enable-updates": true"})
+- The lease's hostname is not be empty.
+- At least one of the lease's DNS direction flags (fdqn_fwd or fdqn_rev) is true.
+
+This parameter defaults to false. An example of its use is shown below:
+
+::
+
+   {
+       "command": "lease4-del",
+       "arguments": {
+           "ip-address": "192.0.2.202",
+           "update-ddns": true
+       }
+   }
+
+
 ``leaseX-del`` returns a result that indicates the outcome of the
 operation. It has one of the following values: 0 (success), 1 (error),
 or 3 (empty). The empty result means that a query has been completed

src/hooks/dhcp/lease_cmds/lease_cmds.cc

@@ -111,10 +111,13 @@ public:
         /// @brief IAID identifier used for v6 leases
         uint32_t iaid;
 
+        /// @brief Indicates whether or not DNS should be updated.
+        bool updateDDNS;
+
         /// @brief Default constructor.
         Parameters()
             : addr("::"), query_type(TYPE_ADDR), lease_type(Lease::TYPE_NA),
-              iaid(0) {
+              iaid(0), updateDDNS(false) {
         }
     };
 
@@ -484,6 +487,15 @@ LeaseCmdsImpl::getParameters(bool v6, const ConstElementPtr& params) {
         isc_throw(BadValue, "Parameters missing or are not a map.");
     }
 
+    if (params->contains("update-ddns")) {
+        ConstElementPtr tmp = params->get("update-ddns");
+        if (tmp->getType() != Element::boolean) {
+            isc_throw(BadValue, "'update-ddns' is not a boolean");
+        } else {
+            x.updateDDNS = tmp->boolValue();
+        }
+    }
+
     // We support several sets of parameters for leaseX-get/lease-del:
     // lease-get(type, address)
     // lease-get(type, subnet-id, identifier-type, identifier)
@@ -581,6 +593,7 @@ LeaseCmdsImpl::getParameters(bool v6, const ConstElementPtr& params) {
                   " is not supported.");
     }
     }
+
     return (x);
 }
 
@@ -1157,6 +1170,12 @@ LeaseCmdsImpl::lease4DelHandler(CalloutHandle& handle) {
         } else {
             setErrorResponse (handle, "IPv4 lease not found.", CONTROL_RESULT_EMPTY);
         }
+
+        // Queue an NCR to remove DNS if configured and the lease has it.
+        if (p.updateDDNS) {
+            queueNCR(CHG_REMOVE, lease4);
+        }
+
     } catch (const std::exception& ex) {
         setErrorResponse(handle, ex.what());
         return (1);
@@ -1435,6 +1454,12 @@ LeaseCmdsImpl::lease6DelHandler(CalloutHandle& handle) {
         } else {
             setErrorResponse (handle, "IPv6 lease not found.", CONTROL_RESULT_EMPTY);
         }
+
+        // Queue an NCR to remove DNS if configured and the lease has it.
+        if (p.updateDDNS) {
+            queueNCR(CHG_REMOVE, lease6);
+        }
+
     } catch (const std::exception& ex) {
         setErrorResponse(handle, ex.what());
         return (1);

src/hooks/dhcp/lease_cmds/lease_cmds.h

@@ -345,7 +345,9 @@ public:
     /// criteria.
     /// It extracts the command name and arguments from the given Callouthandle,
     /// attempts to process them, and then set's the handle's "response"
-    /// argument accordingly.
+    /// argument accordingly.  If the lease is deleted successfully, then a call
+    /// to @ref isc::dhcp::queueNCR() is issued, which to generate an
+    /// CHG_REMOVE request to kea-dhcp-ddns, if appropriate.
     ///
     /// Two types of parameters are supported: (subnet-id, address) or
     /// (subnet-id, identifier-type, identifier).
@@ -381,7 +383,9 @@ public:
     /// This command attempts to delete a lease that match selected criteria.
     /// It extracts the command name and arguments from the given Callouthandle,
     /// attempts to process them, and then set's the handle's "response"
-    /// argument accordingly.
+    /// argument accordingly.  If the lease is deleted successfully, then a call
+    /// to @ref isc::dhcp::queueNCR() is issued, which to generate an
+    /// CHG_REMOVE request to kea-dhcp-ddns, if appropriate.
     ///
     /// Two types of parameters are supported: (subnet-id, address) or
     /// (subnet-id, type, iaid, identifier-type, identifier).

src/hooks/dhcp/lease_cmds/tests/lease_cmds_unittest.cc

@@ -4034,6 +4034,33 @@ TEST_F(LeaseCmdsTest, Lease4DelByAddr) {
     EXPECT_FALSE(lmptr_->getLease4(IOAddress("192.0.2.1")));
 }
 
+// Checks that leaseX-del checks update-ddns input
+TEST_F(LeaseCmdsTest, LeaseXDelBadUpdateDdnsParam) {
+
+    string cmd =
+        "{\n"
+        "    \"command\": \"lease4-del\",\n"
+        "    \"arguments\": {"
+        "        \"ip-address\": \"192.0.1.0\","
+        "        \"update-ddns\": 77"
+        "    }\n"
+        "}";
+
+    string exp_rsp = "'update-ddns' is not a boolean";
+    testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
+
+    cmd =
+        "{\n"
+        "    \"command\": \"lease6-del\",\n"
+        "    \"arguments\": {"
+        "        \"ip-address\": \"2001:db8:1::1\","
+        "        \"update-ddns\": \"bogus\""
+        "    }\n"
+        "}";
+
+    exp_rsp = "'update-ddns' is not a boolean";
+    testCommand(cmd, CONTROL_RESULT_ERROR, exp_rsp);
+}
 
 // Checks that lease4-del sanitizes its input.
 TEST_F(LeaseCmdsTest, Lease4DelByAddrBadParam) {
@@ -5242,4 +5269,324 @@ TEST_F(LeaseCmdsTest, lease6ResendDdnsEnabled) {
     }
 }
 
+// Checks that lease4-del does (or does not) generate an NCR to remove
+// DNS for a given lease based on lease content when DDNS updates are enabled.
+TEST_F(LeaseCmdsTest, lease4DnsRemoveD2Enabled) {
+    // Initialize lease manager (false = v4, true = leases)
+    initLeaseMgr(false, true);
+
+    // Structure detailing a test scenario.
+    struct Scenario {
+        std::string description_;
+        std::string hostname_;
+        bool fqdn_fwd_;
+        bool fqdn_rev_;
+        std::string udpate_ddns_;
+        bool exp_ncr_;
+    };
+
+    bool fwd = true;
+    bool rev = true;
+    bool ncr = true;
+
+    // Three test scenarios to verify each combination of true flags.
+    std::vector<Scenario> scenarios = {
+        {
+            "no_host",
+            "",
+            fwd, rev,
+            "\"update-ddns\": true",
+            !ncr
+        },
+        {
+            "no directions",
+            "myhost.example.com.",
+            !fwd, !rev,
+            "\"update-ddns\": true",
+            !ncr
+        },
+        {
+            "fwd_only",
+            "myhost.example.com.",
+            fwd, !rev,
+            "\"update-ddns\": true",
+            ncr
+        },
+        {
+            "rev_only",
+            "myhost.example.com.",
+            !fwd, rev,
+            "\"update-ddns\": true",
+            ncr
+        },
+        {
+            "both directions",
+            "myhost.example.com.",
+            fwd, rev,
+            "\"update-ddns\": true",
+            ncr
+        },
+        {
+            "default update-ddns",
+            "myhost.example.com.",
+            fwd, rev,
+            "",
+            !ncr
+        },
+        {
+            "update-ddns = false",
+            "myhost.example.com.",
+            fwd, rev,
+            "\"update-ddns\": false",
+            !ncr
+        },
+    };
+
+    for (auto scenario : scenarios) {
+        SCOPED_TRACE(scenario.description_);
+
+        // Let's create a lease with scenario attributes.
+        Lease4Ptr lease = createLease4("192.0.2.8", 44, 0x08, 0x42);
+        lease->hostname_ = scenario.hostname_;
+        lease->fqdn_rev_ = scenario.fqdn_rev_;
+        lease->fqdn_fwd_ = scenario.fqdn_fwd_;
+        ASSERT_TRUE(lmptr_->addLease(lease));
+
+        // NCR Queue should be empty.
+        ASSERT_EQ(ncrQueueSize(), 0);
+
+        // Build the command
+        std::stringstream cmd;
+        cmd <<
+            "{"
+            "    \"command\": \"lease4-del\","
+            "    \"arguments\": {"
+            "        \"ip-address\": \"192.0.2.8\"";
+
+        if (!scenario.udpate_ddns_.empty()) {
+            cmd << "," << scenario.udpate_ddns_;
+        }
+
+        cmd << "}}";
+
+        // Execute the delete command.
+        static_cast<void>(testCommand(cmd.str(), CONTROL_RESULT_SUCCESS, "IPv4 lease deleted."));
+
+        if (!scenario.exp_ncr_) {
+            // Should not have an ncr.
+            ASSERT_EQ(ncrQueueSize(), 0);
+        } else {
+            // We should have an ncr, verify it.
+            ASSERT_EQ(ncrQueueSize(), 1);
+            verifyNameChangeRequest(CHG_REMOVE, scenario.fqdn_rev_, scenario.fqdn_fwd_,
+                                    lease->addr_.toText(), lease->hostname_);
+        }
+
+        // Lease should have been deleted.
+        lease = lmptr_->getLease4(lease->addr_);
+        ASSERT_FALSE(lease);
+    }
+}
+
+// Checks that lease4-del does not generate an NCR to remove
+// DNS for a given lease based on lease content when DDNS
+// updates are disabled.
+TEST_F(LeaseCmdsTest, lease4DnsRemoveD2Disabled) {
+    // Initialize lease manager (false = v4, true = leases)
+    initLeaseMgr(true, true);
+    disableD2();
+
+    // Delete for valid, existing lease.
+    string cmd =
+        "{\n"
+        "    \"command\": \"lease4-del\",\n"
+        "    \"arguments\": {\n"
+        "        \"ip-address\": \"192.0.2.8\",\n"
+        "        \"update-ddns\": true\n"
+        "    }\n"
+        "}";
+
+
+    // Let's create a lease with scenario attributes.
+    Lease4Ptr lease = createLease4("192.0.2.8", 44, 0x08, 0x42);
+    lease->hostname_ = "myhost.example.com.";
+    lease->fqdn_rev_ = true;
+    lease->fqdn_fwd_ = true;
+    ASSERT_TRUE(lmptr_->addLease(lease));
+
+    // NCR Queue is not enabled.
+    ASSERT_EQ(ncrQueueSize(), -1);
+
+    // Execute the delete command.
+    static_cast<void>(testCommand(cmd, CONTROL_RESULT_SUCCESS, "IPv4 lease deleted."));
+
+    // NCR Queue is not enabled.
+    ASSERT_EQ(ncrQueueSize(), -1);
+
+    // Lease should have been deleted.
+    lease = lmptr_->getLease4(lease->addr_);
+    ASSERT_FALSE(lease);
+}
+
+// Checks that lease6-del does (or does not) generate an NCR to remove
+// DNS for a given lease based on lease content when DDNS updates are enabled.
+TEST_F(LeaseCmdsTest, lease6DnsRemoveD2Enabled) {
+    // Initialize lease manager (true = v6, false = no leases)
+    initLeaseMgr(true, true);
+
+    // Structure detailing a test scenario.
+    struct Scenario {
+        std::string description_;
+        std::string hostname_;
+        bool fqdn_fwd_;
+        bool fqdn_rev_;
+        std::string udpate_ddns_;
+        bool exp_ncr_;
+    };
+
+    bool fwd = true;
+    bool rev = true;
+    bool ncr = true;
+
+    // Three test scenarios to verify each combination of true flags.
+    std::vector<Scenario> scenarios = {
+        {
+            "no_host",
+            "",
+            fwd, rev,
+            "\"update-ddns\": true",
+            !ncr
+        },
+        {
+            "no directions",
+            "myhost.example.com.",
+            !fwd, !rev,
+            "\"update-ddns\": true",
+            !ncr
+        },
+        {
+            "fwd_only",
+            "myhost.example.com.",
+            fwd, !rev,
+            "\"update-ddns\": true",
+            ncr
+        },
+        {
+            "rev_only",
+            "myhost.example.com.",
+            !fwd, rev,
+            "\"update-ddns\": true",
+            ncr
+        },
+        {
+            "both directions",
+            "myhost.example.com.",
+            fwd, rev,
+            "\"update-ddns\": true",
+            ncr
+        },
+        {
+            "default update-ddns",
+            "myhost.example.com.",
+            fwd, rev,
+            "",
+            !ncr
+        },
+        {
+            "update-ddns = false",
+            "myhost.example.com.",
+            fwd, rev,
+            "\"update-ddns\": false",
+            !ncr
+        },
+    };
+
+    for (auto scenario : scenarios) {
+        SCOPED_TRACE(scenario.description_);
+
+        // Let's create a lease with scenario attributes.
+        Lease6Ptr lease = createLease6("2001:db8:1::8", 66, 0x77);
+        lease->hostname_ = scenario.hostname_;
+        lease->fqdn_rev_ = scenario.fqdn_rev_;
+        lease->fqdn_fwd_ = scenario.fqdn_fwd_;
+        ASSERT_TRUE(lmptr_->addLease(lease));
+
+        // NCR Queue should be empty.
+        ASSERT_EQ(ncrQueueSize(), 0);
+
+        // Build the command
+        std::stringstream cmd;
+        cmd <<
+            "{"
+            "    \"command\": \"lease6-del\","
+            "    \"arguments\": {"
+            "        \"subnet-id\": 66,\n"
+            "        \"ip-address\": \"2001:db8:1::8\"\n";
+
+        if (!scenario.udpate_ddns_.empty()) {
+            cmd << "," << scenario.udpate_ddns_;
+        }
+
+        cmd << "}}";
+
+        // Execute the delete command.
+        static_cast<void>(testCommand(cmd.str(), CONTROL_RESULT_SUCCESS, "IPv6 lease deleted."));
+
+        if (!scenario.exp_ncr_) {
+            // Should not have an ncr.
+            ASSERT_EQ(ncrQueueSize(), 0);
+        } else {
+            // We should have an ncr, verify it.
+            ASSERT_EQ(ncrQueueSize(), 1);
+            verifyNameChangeRequest(CHG_REMOVE, scenario.fqdn_rev_, scenario.fqdn_fwd_,
+                                    lease->addr_.toText(), lease->hostname_);
+        }
+
+        // Lease should have been deleted.
+        lease = lmptr_->getLease6(Lease::TYPE_NA, lease->addr_);
+        ASSERT_FALSE(lease);
+    }
+}
+
+// Checks that lease6-del does not generate an NCR to remove
+// DNS for a given lease based on lease content when DDNS
+// updates are disabled.
+TEST_F(LeaseCmdsTest, lease6DnsRemoveD2Disabled) {
+    // Initialize lease manager (true = v6, true = leases)
+    initLeaseMgr(true, true);
+    disableD2();
+
+    // Delete for valid, existing lease.
+    string cmd =
+        "{\n"
+        "    \"command\": \"lease6-del\",\n"
+        "    \"arguments\": {\n"
+        "        \"subnet-id\": 66,\n"
+        "        \"ip-address\": \"2001:db8:1::8\",\n"
+        "        \"update-ddns\": true\n"
+        "    }\n"
+        "}";
+
+
+    // Let's create a lease with scenario attributes.
+    Lease6Ptr lease = createLease6("2001:db8:1::8", 66, 0x77);
+    lease->hostname_ = "myhost.example.com.";
+    lease->fqdn_rev_ = true;
+    lease->fqdn_fwd_ = true;
+    ASSERT_TRUE(lmptr_->addLease(lease));
+
+    // NCR Queue is not enabled.
+    ASSERT_EQ(ncrQueueSize(), -1);
+
+    // Execute the delete command.
+    static_cast<void>(testCommand(cmd, CONTROL_RESULT_SUCCESS, "IPv6 lease deleted."));
+
+    // NCR Queue is not enabled.
+    ASSERT_EQ(ncrQueueSize(), -1);
+
+    // Lease should have been deleted.
+    lease = lmptr_->getLease6(Lease::TYPE_NA, lease->addr_);
+    ASSERT_FALSE(lease);
+}
+
 } // end of anonymous namespace

src/lib/dhcpsrv/dhcpsrv_messages.cc

@@ -1,4 +1,4 @@
-// File created from ../../../src/lib/dhcpsrv/dhcpsrv_messages.mes on Mon Jun 22 2020 17:23
+// File created from ../../../src/lib/dhcpsrv/dhcpsrv_messages.mes on Thu Jul 16 2020 15:23
 
 #include <cstddef>
 #include <log/message_types.h>

src/lib/dhcpsrv/dhcpsrv_messages.h

@@ -1,4 +1,4 @@
-// File created from ../../../src/lib/dhcpsrv/dhcpsrv_messages.mes on Mon Jun 22 2020 17:23
+// File created from ../../../src/lib/dhcpsrv/dhcpsrv_messages.mes on Thu Jul 16 2020 15:23
 
 #ifndef DHCPSRV_MESSAGES_H
 #define DHCPSRV_MESSAGES_H

src/lib/dhcpsrv/memfile_lease_mgr.cc

@@ -1479,10 +1479,6 @@ Memfile_LeaseMgr::updateLease6(const Lease6Ptr& lease) {
 bool
 Memfile_LeaseMgr::deleteLeaseInternal(const Lease4Ptr& lease) {
     const isc::asiolink::IOAddress& addr = lease->addr_;
-    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
-              DHCPSRV_MEMFILE_DELETE_ADDR)
-        .arg(addr.toText());
-
     Lease4Storage::iterator l = storage4_.find(addr);
     if (l == storage4_.end()) {
         // No such lease
@@ -1518,10 +1514,6 @@ Memfile_LeaseMgr::deleteLease(const Lease4Ptr& lease) {
 bool
 Memfile_LeaseMgr::deleteLeaseInternal(const Lease6Ptr& lease) {
     const isc::asiolink::IOAddress& addr = lease->addr_;
-    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
-              DHCPSRV_MEMFILE_DELETE_ADDR)
-        .arg(addr.toText());
-
     Lease6Storage::iterator l = storage6_.find(addr);
     if (l == storage6_.end()) {
         // No such lease