[3688] Prevent allocation engine from allocating 0 IPv4 address.

diff --git a/src/bin/dhcp4/tests/fqdn_unittest.cc b/src/bin/dhcp4/tests/fqdn_unittest.cc
index 1257e46820675b55c4841ea025d983088f58ea1d..280ed3280a6f8d3e74bfa84183813c452ac87171 100644 (file)
--- a/src/bin/dhcp4/tests/fqdn_unittest.cc
+++ b/src/bin/dhcp4/tests/fqdn_unittest.cc
@@ -55,7 +55,6 @@ const char* CONFIGS[] = {
         "    \"reservations\": ["
         "       {"
         "         \"hw-address\": \"aa:bb:cc:dd:ee:ff\","
-        "         \"ip-address\": \"10.0.0.5\","
         "         \"hostname\":   \"unique-host.example.org\""
         "       }"
         "    ]"
@@ -83,7 +82,6 @@ const char* CONFIGS[] = {
         "    \"reservations\": ["
         "       {"
         "         \"hw-address\": \"aa:bb:cc:dd:ee:ff\","
-        "         \"ip-address\": \"10.0.0.5\","
         "         \"hostname\":   \"foobar.org\""
         "       }"
         "    ]"

diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index 0e75f1998065e026936665347db7fd6c413501f1..4e38631452b55b8a75397b98ba13f58d5eaeb0f8 100644 (file)
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -1215,6 +1215,26 @@ addressReserved(const IOAddress& address, const AllocEngine::ClientContext4& ctx
     return (false);
 }
 
+/// @brief Check if the context contains the reservation for the
+/// IPv4 address.
+///
+/// This convenience function checks if the context contains the reservation
+/// for the IPv4 address. Note that some reservations may not assign a
+/// static IPv4 address to the clients, but may rather reserve a hostname.
+/// Allocation engine should check if the existing reservation is made
+/// for the IPv4 address and if it is not, allocate the address from the
+/// dynamic pool. The allocation engine uses this function to check if
+/// the reservation is made for the IPv4 address.
+///
+/// @param ctx Client context holding the data extracted from the
+/// client's message.
+///
+/// @return true if the context contains the reservation for the IPv4 address.
+bool
+hasAddressReservation(const AllocEngine::ClientContext4& ctx) {
+    return (ctx.host_ && !ctx.host_->getIPv4Reservation().isV4Zero());
+}
+
 } // end of anonymous namespace
 
 namespace isc {
@@ -1339,7 +1359,7 @@ AllocEngine::discoverLease4(AllocEngine::ClientContext4& ctx) {
 
     // Check if there is a reservation for the client. If there is, we want to
     // assign the reserved address, rather than any other one.
-    if (ctx.host_) {
+    if (hasAddressReservation(ctx)) {
         // If the client doesn't have a lease or the leased address is different
         // than the reserved one then let's try to allocate the reserved address.
         // Otherwise the address that the client has is the one for which it
@@ -1438,7 +1458,7 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) {
             return (Lease4Ptr());
         }
 
-    } else if (ctx.host_) {
+    } else if (hasAddressReservation(ctx)) {
         // The client hasn't specified an address to allocate, so the
         // allocation engine needs to find an appropriate address.
         // If there is a reservation for the client, let's try to
@@ -1462,7 +1482,8 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) {
         // address it is possible that the client was offered this different
         // address because the reserved address is in use. We will have to
         // check if the address is in use.
-        if (ctx.host_ && (ctx.host_->getIPv4Reservation() != ctx.requested_address_)) {
+        if (hasAddressReservation(ctx) &&
+            (ctx.host_->getIPv4Reservation() != ctx.requested_address_)) {
             existing = LeaseMgrFactory::instance().getLease4(ctx.host_->getIPv4Reservation());
             // If the reserved address is not in use, i.e. the lease doesn't
             // exist or is expired, and the client is requesting a different
@@ -1476,7 +1497,8 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) {
         // The use of the out-of-pool addresses is only allowed when the requested
         // address is reserved for the client. If the address is not reserved one
         // and it doesn't belong to the dynamic pool, do not allocate it.
-        if ((!ctx.host_ || (ctx.host_->getIPv4Reservation() != ctx.requested_address_)) &&
+        if ((!hasAddressReservation(ctx) ||
+             (ctx.host_->getIPv4Reservation() != ctx.requested_address_)) &&
             !ctx.subnet_->inPool(Lease4::TYPE_V4, ctx.requested_address_)) {
             return (Lease4Ptr());
         }

diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
index fbe49699a2fbc918c68e9747f63048604a8f9ddb..8685b826540692ac078c4cd42f25083452511962 100644 (file)
--- a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
@@ -1263,7 +1263,7 @@ TEST_F(AllocEngine4Test, reservedAddressShortPool) {
     // Create short pool with only one address.
     initSubnet(IOAddress("192.0.2.100"), IOAddress("192.0.2.100"));
     // Reserve the address for a different client.
-    HostPtr host(new Host(&hwaddr2_->hwaddr_[0], hwaddr_->hwaddr_.size(),
+    HostPtr host(new Host(&hwaddr2_->hwaddr_[0], hwaddr2_->hwaddr_.size(),
                           Host::IDENT_HWADDR, subnet_->getID(),
                           SubnetID(0), IOAddress("192.0.2.100")));
     CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
@@ -1292,6 +1292,35 @@ TEST_F(AllocEngine4Test, reservedAddressShortPool) {
     EXPECT_EQ("192.0.2.100", allocated_lease->addr_.toText());
 }
 
+// This test checks that the AllocEngine allocates an address from the
+// dynamic pool if the client's reservation is made for a hostname but
+// not for an address.
+TEST_F(AllocEngine4Test, reservedHostname) {
+    AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false);
+
+    // Create a reservation for a hostname. Address is set to 0 which
+    // indicates that there is no reservation.
+    HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(),
+                          Host::IDENT_HWADDR, subnet_->getID(),
+                          SubnetID(0), IOAddress::IPV4_ZERO_ADDRESS(),
+                          "foo.example.org"));
+    CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host);
+    CfgMgr::instance().commit();
+
+    // Try to allocate a lease.
+    AllocEngine::ClientContext4 ctx(subnet_, ClientIdPtr(), hwaddr_,
+                                    IOAddress::IPV4_ZERO_ADDRESS(), false, false,
+                                    "foo.example.org", true);
+    Lease4Ptr allocated_lease = engine.allocateLease4(ctx);
+    ASSERT_TRUE(allocated_lease);
+    ASSERT_FALSE(allocated_lease->addr_.isV4Zero());
+
+    ctx.requested_address_ = allocated_lease->addr_;
+    ctx.fake_allocation_ = false;
+    allocated_lease = engine.allocateLease4(ctx);
+    ASSERT_TRUE(allocated_lease);
+}
+
 // This test checks that the AllocEngine::findReservation method finds
 // and returns host reservation for the DHCPv4 client using the data from
 // the client context. If the host reservation can't be found, it sets