From: Thomas Markwalder <tmark@isc.org>
Date: Mon, 24 Mar 2025 14:51:17 +0000 (-0400)
Subject: [#3555] Cleanups and Changelog
X-Git-Tag: Kea-2.7.8~109
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=cf371a2d2d9140202c754acce35e1f611458f7d8;p=thirdparty%2Fkea.git

[#3555] Cleanups and Changelog

Added Changelog entry
/src/lib/dhcpsrv/alloc_engine.cc
/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
    Minor cleanups
---

diff --git a/changelog_unreleased/3555-host-reservation-and-offer-lifetime-option-conflicts-causing-persistent-naks b/changelog_unreleased/3555-host-reservation-and-offer-lifetime-option-conflicts-causing-persistent-naks
new file mode 100644
index 0000000000..bdf8bd768f
--- /dev/null
+++ b/changelog_unreleased/3555-host-reservation-and-offer-lifetime-option-conflicts-causing-persistent-naks
@@ -0,0 +1,9 @@
+[bug]		tmark
+	Corrected an issue which caused kea-dhcp4 to
+	NAK requests for an offered lease when a
+	conflicting host reservation exists and 
+	offer-lifetime is greater than zero. The 
+	server will now remove the unusable lease,
+	renew the temporary lease, and send it back
+	in a DHCPACK.
+	(Gitlab #3555)
diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index 4d4acb2deb..e4a09fd4c7 100644
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -4140,7 +4140,7 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) {
         // client's current lease (client_lease) should no longer be used and
         // so offered a different lease (existing) that was temporarily
         // allocated because offer-lifetime is greater than zero. We need to
-        // delete to unusable lease and renew the temporary lease.
+        // delete the unusable lease and renew the temporary lease.
         if (((client_lease && existing) && (client_lease->addr_ != existing->addr_) &&
              (existing->addr_ == ctx.requested_address_) &&
              (existing->belongsToClient(ctx.hwaddr_, ctx.subnet_->getMatchClientId() ?
@@ -4172,7 +4172,6 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) {
                       ALLOC_ENGINE_V4_REQUEST_EXTEND_LEASE)
                 .arg(ctx.query_->getLabel())
                 .arg(ctx.requested_address_);
-
             return (renewLease4(client_lease, ctx));
         }
     }
diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
index c63f13ca3f..09db913900 100644
--- a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
+++ b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc
@@ -2149,9 +2149,8 @@ TEST_F(AllocEngine4Test, existingLeasePlusTemporary) {
     LeaseMgrFactory::instance().addLease(lease);
     LeaseMgrFactory::instance().addLease(lease2);
 
-    // First when temporary allocations are off.
-
-    // Client requests the second lease.
+    // When temporary allocations are off and client requests the
+    // second lease, the engine should not return a lease.
     AllocEngine engine(0);
     AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_, IOAddress("192.0.2.102"),
                                     false, false, "", false);
@@ -2160,7 +2159,7 @@ TEST_F(AllocEngine4Test, existingLeasePlusTemporary) {
 
     ASSERT_FALSE(new_lease);
 
-    // Both leases should still be there.
+    // Both leases should still be in the lease back end.
     ASSERT_TRUE(LeaseMgrFactory::instance().getLease4(lease->addr_));
     ASSERT_TRUE(LeaseMgrFactory::instance().getLease4(lease2->addr_));
 
@@ -2173,11 +2172,11 @@ TEST_F(AllocEngine4Test, existingLeasePlusTemporary) {
     ctx2.query_.reset(new Pkt4(DHCPREQUEST, 1234));
     new_lease = engine.allocateLease4(ctx2);
 
-    // Allocation should renew it and remove the original lease.
+    // Allocation should return the second lease and remove the original lease.
     ASSERT_TRUE(new_lease);
     EXPECT_EQ("192.0.2.102", new_lease->addr_.toText());
 
-    // Orignal lease should be gone.
+    // Orignal lease should have been deleted from lease back.
     ASSERT_FALSE(LeaseMgrFactory::instance().getLease4(lease->addr_));
 }