From: Thomas Markwalder Date: Mon, 3 Feb 2025 20:00:23 +0000 (-0500) Subject: [#3463] Add response4 to V4 callout arguments X-Git-Tag: Kea-2.7.7~209 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=00dcacbc23a82dff81e6ab0cd9ac1e2cd2ac598a;p=thirdparty%2Fkea.git [#3463] Add response4 to V4 callout arguments /src/bin/dhcp4/dhcp4_srv.cc Dhcpv4Srv::processLocalizedQuery4(AllocEngine::ClientContext4Ptr& ctx, - Added response4 argument to lease4_offer and leases4_committed callout_handles. /src/bin/dhcp4/tests/hooks_unittest.cc Updated tests /src/hooks/dhcp/lease_cmds/lease_cmds.* lease4_offer() handling /src/hooks/dhcp/lease_cmds/lease_cmds_callouts.cc added lease4_offer callout /src/hooks/dhcp/lease_cmds/libloadtests/load_unload_unittests.cc TEST_F(LeaseCmdsCbLibLoadTest, verifyCallouts4) TEST_F(LeaseCmdsCbLibLoadTest, verifyCallouts6) - new tests /src/hooks/dhcp/lease_cmds/tests/lease_cmds4_unittest.cc TEST_F(Lease4CmdsTest, validLease4Offer) TEST_F(Lease4CmdsTest, validLease4OfferMultiThreading) TEST_F(Lease4CmdsTest, nopLeases4Committed) TEST_F(Lease4CmdsTest, nopLeases4CommittedMultiThreading) - new tests --- diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 86c6bdf7e1..978dfb45f7 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -1737,6 +1737,10 @@ Dhcpv4Srv::processLocalizedQuery4(AllocEngine::ClientContext4Ptr& ctx, // Also pass the corresponding query packet as argument callout_handle->setArgument("query4", query); + // Also pass the corresponding response packet as argument + ScopedEnableOptionsCopy response4_options_copy(rsp); + callout_handle->setArgument("response4", rsp); + Lease4CollectionPtr new_leases(new Lease4Collection()); // Filter out the new lease if it was reused so not committed. if (ctx->new_lease_ && (ctx->new_lease_->reuseable_valid_lft_ == 0)) { diff --git a/src/bin/dhcp4/tests/hooks_unittest.cc b/src/bin/dhcp4/tests/hooks_unittest.cc index 3ac93d68c7..d6f6599f96 100644 --- a/src/bin/dhcp4/tests/hooks_unittest.cc +++ b/src/bin/dhcp4/tests/hooks_unittest.cc @@ -2157,6 +2157,7 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedRequest) { // Check if all expected parameters were really received vector expected_argument_names; expected_argument_names.push_back("query4"); + expected_argument_names.push_back("response4"); expected_argument_names.push_back("deleted_leases4"); expected_argument_names.push_back("leases4"); @@ -2272,6 +2273,7 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedDecline) { // Check if all expected parameters were really received vector expected_argument_names; expected_argument_names.push_back("query4"); + expected_argument_names.push_back("response4"); expected_argument_names.push_back("deleted_leases4"); expected_argument_names.push_back("leases4"); @@ -2320,6 +2322,7 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedRelease) { // Check if all expected parameters were really received vector expected_argument_names; expected_argument_names.push_back("query4"); + expected_argument_names.push_back("response4"); expected_argument_names.push_back("deleted_leases4"); expected_argument_names.push_back("leases4"); @@ -2367,6 +2370,7 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedCache) { // Check if all expected parameters were really received vector expected_argument_names; expected_argument_names.push_back("query4"); + expected_argument_names.push_back("response4"); expected_argument_names.push_back("deleted_leases4"); expected_argument_names.push_back("leases4"); @@ -2439,6 +2443,7 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedParkRequests) { // Check if all expected parameters were really received vector expected_argument_names; expected_argument_names.push_back("query4"); + expected_argument_names.push_back("response4"); expected_argument_names.push_back("deleted_leases4"); expected_argument_names.push_back("leases4"); @@ -3677,6 +3682,7 @@ TEST_F(HooksDhcpv4SrvTest, lease4OfferDiscover) { // Check if all expected parameters were really received vector expected_argument_names; expected_argument_names.push_back("query4"); + expected_argument_names.push_back("response4"); expected_argument_names.push_back("leases4"); expected_argument_names.push_back("offer_lifetime"); expected_argument_names.push_back("old_lease"); @@ -3830,6 +3836,7 @@ TEST_F(HooksDhcpv4SrvTest, lease4OfferParkRequests) { // Check if all expected parameters were really received vector expected_argument_names; expected_argument_names.push_back("query4"); + expected_argument_names.push_back("response4"); expected_argument_names.push_back("leases4"); expected_argument_names.push_back("offer_lifetime"); expected_argument_names.push_back("old_lease"); @@ -4046,6 +4053,7 @@ TEST_F(HooksDhcpv4SrvTest, lease4OfferDiscoverDecline) { // Check if all expected parameters were really received vector expected_argument_names; expected_argument_names.push_back("query4"); + expected_argument_names.push_back("response4"); expected_argument_names.push_back("leases4"); expected_argument_names.push_back("offer_lifetime"); expected_argument_names.push_back("old_lease"); diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds.cc b/src/hooks/dhcp/lease_cmds/lease_cmds.cc index 43a940c3dd..6d0173b7ff 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds.cc +++ b/src/hooks/dhcp/lease_cmds/lease_cmds.cc @@ -473,17 +473,35 @@ public: return (isc->get("relay-info")); } - /// @brief leases4-committed hookpoint handler. + /// @brief lease4_offer hookpoint handler. + /// + /// If the offer_lifetime argument is zero, it simply returns. Otherwise + /// it evaluates the binding variables (if any), and updates the given lease's + /// user-context accordingly. This includes updating the lease in the lease + /// back end. + /// + /// @param handle Callout context - which is expected to contain the query4, + /// response4, leases4, offer_lifetime arguments. + /// @param mgr Pointer to the BindingVariableMgr singleton. + /// @throw Unexpected if there is no active lease or a processing error + /// occurs. LeaseCmdsConflict if the update fails because the lease + /// no longer exists in the back end. + static void lease4Offer(CalloutHandle& callout_handle, + BindingVariableMgrPtr mgr); + + /// @brief leases4_committed hookpoint handler. /// /// Evaluates the binding variables (if any), and updates the given lease's /// user-context accordingly. This includes updating the lease in the lease /// back end. /// - /// @param handle Callout context - which is expected to contain the query4, response4, - /// and leases4 argumeents. + /// @param handle Callout context - which is expected to contain the query4, + /// response4, and leases4 arguments. /// @param mgr Pointer to the BindingVariableMgr singleton. - /// @throw - static void leases4Committed(CalloutHandle& callout_handle, + /// @throw Unexpected if there is no active lease or a processing error + /// occurs. LeaseCmdsConflict if the update fails because the lease + /// no longer exists in the back end. + static void leases4Committed(CalloutHandle& callout_handle, BindingVariableMgrPtr mgr); }; @@ -2712,6 +2730,45 @@ LeaseCmdsImpl::leaseWriteHandler(CalloutHandle& handle) { return (0); } +void +LeaseCmdsImpl::lease4Offer(CalloutHandle& callout_handle, + BindingVariableMgrPtr mgr) { + uint32_t offer_lifetime; + callout_handle.getArgument("offer_lifetime", offer_lifetime); + if (!offer_lifetime) { + // Offers leases are not being persisted, nothing to do. + return; + } + + // Get the remaining arguments we need. + Pkt4Ptr query; + Pkt4Ptr response; + Lease4CollectionPtr leases; + + callout_handle.getArgument("query4", query); + callout_handle.getArgument("response4", response); + callout_handle.getArgument("leases4", leases); + + if (leases->empty()) { + isc_throw(Unexpected, "lease4Offer - no lease!"); + } + + Lease4Ptr lease = (*leases)[0]; + try { + if (mgr->evaluateVariables(query, response, lease)) { + LeaseMgrFactory::instance().updateLease4(lease); + } + } catch (const NoSuchLease&) { + isc_throw(LeaseCmdsConflict, "failed to update" + " the lease with address " << lease->addr_ << + " either because the lease has been" + " deleted or it has changed in the database"); + } catch (const std::exception& ex) { + isc_throw(Unexpected, "evaluating binding variables failed for: " + << query->getLabel() << ", :" << ex.what()); + } +} + void LeaseCmdsImpl::leases4Committed(CalloutHandle& callout_handle, BindingVariableMgrPtr mgr) { @@ -2724,11 +2781,9 @@ LeaseCmdsImpl::leases4Committed(CalloutHandle& callout_handle, callout_handle.getArgument("response4", response); callout_handle.getArgument("leases4", leases); - // In some cases we may have no leases, e.g. DHCPNAK. - if (leases->empty()) { - LOG_DEBUG(lease_cmds_logger, DBGLVL_TRACE_BASIC, - LEASE_CMDS_LEASES4_COMMITTED_NOTHING_TO_UPDATE) - .arg(query->getLabel()); + // In some cases we may have no lease, e.g. DHCPNAK, + // or no response e.g. DHCPRELEASE. + if (leases->empty() || !response || (response->getType() != DHCPACK)) { return; } @@ -2739,11 +2794,11 @@ LeaseCmdsImpl::leases4Committed(CalloutHandle& callout_handle, } } catch (const NoSuchLease&) { isc_throw(LeaseCmdsConflict, "failed to update" - " the lease with address " << lease->addr_ << + " the lease with address " << lease->addr_ << " either because the lease has been" " deleted or it has changed in the database"); } catch (const std::exception& ex) { - isc_throw(Unexpected, "evaluating binding variables failed for " + isc_throw(Unexpected, "evaluating binding variables failed for: " << query->getLabel() << ", :" << ex.what()); } } @@ -2844,6 +2899,12 @@ LeaseCmds::LeaseCmds() : impl_(new LeaseCmdsImpl()) { } +void +LeaseCmds::lease4Offer(CalloutHandle& callout_handle, + BindingVariableMgrPtr mgr) { + impl_->lease4Offer(callout_handle, mgr); +} + void LeaseCmds::leases4Committed(CalloutHandle& callout_handle, BindingVariableMgrPtr mgr) { diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds.h b/src/hooks/dhcp/lease_cmds/lease_cmds.h index b0ed786ab7..0075860930 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds.h +++ b/src/hooks/dhcp/lease_cmds/lease_cmds.h @@ -606,6 +606,20 @@ public: int leaseWriteHandler(hooks::CalloutHandle& handle); + /// @brief lease4_offer hookpoint handler. + /// + /// If the offer_lifetime callout argument is 0 the handler simply returns. + /// Otherwise it will evaluate the binding variables (if any) and update + /// the given lease's user-context accordingly. This includes updating the + /// lease in the lease back end. + /// + /// @param handle Callout context - which is expected to contain the query4, response4, + /// leases4 arguments, and offer_lifetime. + /// @param mgr Pointer to the BindingVariableMgr singleton. + void + lease4Offer(hooks::CalloutHandle& callout_handle, + BindingVariableMgrPtr mgr); + /// @brief leases4_committed hookpoint handler. /// /// Evaluates the binding variables (if any), and updates the given lease's @@ -613,10 +627,10 @@ public: /// back end. /// /// @param handle Callout context - which is expected to contain the query4, response4, - /// and leases4 argumeents. + /// and leases4 arguments. /// @param mgr Pointer to the BindingVariableMgr singleton. - void - leases4Committed(hooks::CalloutHandle& callout_handle, + void + leases4Committed(hooks::CalloutHandle& callout_handle, BindingVariableMgrPtr mgr); private: diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds_callouts.cc b/src/hooks/dhcp/lease_cmds/lease_cmds_callouts.cc index c3c8a99b23..2619ea5e07 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds_callouts.cc +++ b/src/hooks/dhcp/lease_cmds/lease_cmds_callouts.cc @@ -378,6 +378,28 @@ int multi_threading_compatible() { return (1); } +/// @brief lease4_offer callout implementation. +/// +/// @param handle callout handle. +int lease4_offer(CalloutHandle& handle) { + CalloutHandle::CalloutNextStep status = handle.getStatus(); + if (status == CalloutHandle::NEXT_STEP_DROP || + status == CalloutHandle::NEXT_STEP_SKIP) { + return (0); + } + + try { + LeaseCmds lease_cmds; + lease_cmds.lease4Offer(handle, binding_var_mgr); + } catch (const std::exception& ex) { + LOG_ERROR(lease_cmds_logger, LEASE_CMDS_LEASE4_OFFER_FAILED) + .arg(ex.what()); + return (1); + } + + return (0); +} + /// @brief leases4_committed callout implementation. /// /// @param handle callout handle. diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds_messages.cc b/src/hooks/dhcp/lease_cmds/lease_cmds_messages.cc index c13da0ca19..fb12339b26 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds_messages.cc +++ b/src/hooks/dhcp/lease_cmds/lease_cmds_messages.cc @@ -20,8 +20,8 @@ extern const isc::log::MessageID LEASE_CMDS_DEL6_FAILED = "LEASE_CMDS_DEL6_FAILE extern const isc::log::MessageID LEASE_CMDS_GET4_FAILED = "LEASE_CMDS_GET4_FAILED"; extern const isc::log::MessageID LEASE_CMDS_GET6_FAILED = "LEASE_CMDS_GET6_FAILED"; extern const isc::log::MessageID LEASE_CMDS_INIT_OK = "LEASE_CMDS_INIT_OK"; +extern const isc::log::MessageID LEASE_CMDS_LEASE4_OFFER_FAILED = "LEASE_CMDS_LEASE4_OFFER_FAILED"; extern const isc::log::MessageID LEASE_CMDS_LEASES4_COMMITTED_FAILED = "LEASE_CMDS_LEASES4_COMMITTED_FAILED"; -extern const isc::log::MessageID LEASE_CMDS_LEASES4_COMMITTED_NOTHING_TO_UPDATE = "LEASE_CMDS_LEASES4_COMMITTED_NOTHING_TO_UPDATE"; extern const isc::log::MessageID LEASE_CMDS_RESEND_DDNS4 = "LEASE_CMDS_RESEND_DDNS4"; extern const isc::log::MessageID LEASE_CMDS_RESEND_DDNS4_FAILED = "LEASE_CMDS_RESEND_DDNS4_FAILED"; extern const isc::log::MessageID LEASE_CMDS_RESEND_DDNS6 = "LEASE_CMDS_RESEND_DDNS6"; @@ -58,8 +58,8 @@ const char* values[] = { "LEASE_CMDS_GET4_FAILED", "lease4-get command failed (parameters: %1, reason: %2)", "LEASE_CMDS_GET6_FAILED", "lease6-get command failed (parameters: %1, reason: %2)", "LEASE_CMDS_INIT_OK", "loading Lease Commands hooks library successful", - "LEASE_CMDS_LEASES4_COMMITTED_FAILED", "processing error occured evaluating binding variables for %1, %2", - "LEASE_CMDS_LEASES4_COMMITTED_NOTHING_TO_UPDATE", "query %1, has no active lease.", + "LEASE_CMDS_LEASE4_OFFER_FAILED", "processing error occurred evaluating binding variables for %1, %2", + "LEASE_CMDS_LEASES4_COMMITTED_FAILED", "processing error occurred evaluating binding variables for %1, %2", "LEASE_CMDS_RESEND_DDNS4", "lease4-resend-ddns command successful: %1", "LEASE_CMDS_RESEND_DDNS4_FAILED", "lease4-resend-ddns command failed: %1", "LEASE_CMDS_RESEND_DDNS6", "lease6-resend-ddns command successful: %1", diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds_messages.h b/src/hooks/dhcp/lease_cmds/lease_cmds_messages.h index 056f935b35..ee45893748 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds_messages.h +++ b/src/hooks/dhcp/lease_cmds/lease_cmds_messages.h @@ -21,8 +21,8 @@ extern const isc::log::MessageID LEASE_CMDS_DEL6_FAILED; extern const isc::log::MessageID LEASE_CMDS_GET4_FAILED; extern const isc::log::MessageID LEASE_CMDS_GET6_FAILED; extern const isc::log::MessageID LEASE_CMDS_INIT_OK; +extern const isc::log::MessageID LEASE_CMDS_LEASE4_OFFER_FAILED; extern const isc::log::MessageID LEASE_CMDS_LEASES4_COMMITTED_FAILED; -extern const isc::log::MessageID LEASE_CMDS_LEASES4_COMMITTED_NOTHING_TO_UPDATE; extern const isc::log::MessageID LEASE_CMDS_RESEND_DDNS4; extern const isc::log::MessageID LEASE_CMDS_RESEND_DDNS4_FAILED; extern const isc::log::MessageID LEASE_CMDS_RESEND_DDNS6; diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds_messages.mes b/src/hooks/dhcp/lease_cmds/lease_cmds_messages.mes index 44985368da..28288a886b 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds_messages.mes +++ b/src/hooks/dhcp/lease_cmds/lease_cmds_messages.mes @@ -139,11 +139,12 @@ The lease6-wipe command is deprecated and it will be removed in the future. The lease6-wipe command has failed. Both the reason as well as the parameters passed are logged. -% LEASE_CMDS_LEASES4_COMMITTED_NOTHING_TO_UPDATE query %1, has no active lease. -This debug log is emitted when the leaes4-committed handler is invoked and -there is no active lease associated with the query. +% LEASE_CMDS_LEASE4_OFFER_FAILED processing error occurred evaluating binding variables for %1, %2 +This debug log is emitted when an error occurs in the lease4_offer +handler is invoked. The arguments detail the query and the error. This +error means binding-variable values were not added/updated for the lease. -% LEASE_CMDS_LEASES4_COMMITTED_FAILED processing error occured evaluating binding variables for %1, %2 -This debug log is emitted when an error occurs in the leaes4-committed -handler is invoked. The arguemntes detail the query and the error. This +% LEASE_CMDS_LEASES4_COMMITTED_FAILED processing error occurred evaluating binding variables for %1, %2 +This debug log is emitted when an error occurs in the leases4_committed +handler is invoked. The arguments detail the query and the error. This error means binding-variable values were not added/updated for the lease. diff --git a/src/hooks/dhcp/lease_cmds/libloadtests/load_unload_unittests.cc b/src/hooks/dhcp/lease_cmds/libloadtests/load_unload_unittests.cc index fb4aca05b5..cc125d206b 100644 --- a/src/hooks/dhcp/lease_cmds/libloadtests/load_unload_unittests.cc +++ b/src/hooks/dhcp/lease_cmds/libloadtests/load_unload_unittests.cc @@ -39,6 +39,29 @@ public: unloadLibraries(); } + /// @brief Registers hooks in the hook manager. + /// + /// Normally this is done by the server core code. + void registerHooks() { + if (isc::dhcp::CfgMgr::instance().getFamily() == AF_INET) { + hook_index_lease4_offer_ = HooksManager::registerHook("lease4_offer"); + hook_index_leasesX_committed_ = HooksManager::registerHook("leases4_committed"); + } else { + hook_index_leasesX_committed_ = HooksManager::registerHook("leases6_committed"); + } + } + + /// @brief Checks that expected callouts are present. + void calloutsPresent() { + bool result; + ASSERT_NO_THROW_LOG(result = HooksManager::calloutsPresent(hook_index_lease4_offer_)); + EXPECT_EQ(result, (isc::dhcp::CfgMgr::instance().getFamily() == AF_INET)); + + /// @todo when v6 is ready, change to always expect true. + ASSERT_NO_THROW_LOG(result = HooksManager::calloutsPresent(hook_index_leasesX_committed_)); + EXPECT_EQ(result, (isc::dhcp::CfgMgr::instance().getFamily() == AF_INET)); + } + /// @brief Creates a set of configuration parameters valid for the library. /// Note the expressions are protocol agnostic for simplicity. virtual isc::data::ElementPtr validConfigParams() { @@ -58,6 +81,10 @@ public: // Convert JSON texts to Element map. return (Element::fromJSON(valid_config)); } + + /// @brief Hook index values. + int hook_index_lease4_offer_; + int hook_index_leasesX_committed_; }; // Simple V4 test that checks the library can be loaded and unloaded several times. @@ -77,4 +104,40 @@ TEST_F(LeaseCmdsCbLibLoadTest, invalidDaemonLoad) { invalidDaemonTest("bogus"); } +// Verifies that callout functions exist after loading the library. +TEST_F(LeaseCmdsCbLibLoadTest, verifyCallouts4) { + // Set family and daemon's proc name and register hook points. + isc::dhcp::CfgMgr::instance().setFamily(AF_INET); + isc::process::Daemon::setProcName("kea-dhcp4"); + registerHooks(); + + // Add library to config and load it. + ASSERT_NO_THROW_LOG(addLibrary(lib_so_name_, valid_params_)); + ASSERT_NO_THROW_LOG(loadLibraries()); + + // Verify that expected callouts are present. + calloutsPresent(); + + // Unload the library. + ASSERT_NO_THROW_LOG(unloadLibraries()); +} + +// Verifies that callout functions exist after loading the library. +TEST_F(LeaseCmdsCbLibLoadTest, verifyCallouts6) { + // Set family and daemon's proc name and register hook points. + isc::dhcp::CfgMgr::instance().setFamily(AF_INET6); + isc::process::Daemon::setProcName("kea-dhcp6"); + registerHooks(); + + // Add library to config and load it. + ASSERT_NO_THROW_LOG(addLibrary(lib_so_name_, valid_params_)); + ASSERT_NO_THROW_LOG(loadLibraries()); + + // Verify that expected callouts are present. + calloutsPresent(); + + // Unload the library. + ASSERT_NO_THROW_LOG(unloadLibraries()); +} + } // end of anonymous namespace diff --git a/src/hooks/dhcp/lease_cmds/tests/lease_cmds4_unittest.cc b/src/hooks/dhcp/lease_cmds/tests/lease_cmds4_unittest.cc index aff427c228..fec61482f2 100644 --- a/src/hooks/dhcp/lease_cmds/tests/lease_cmds4_unittest.cc +++ b/src/hooks/dhcp/lease_cmds/tests/lease_cmds4_unittest.cc @@ -448,9 +448,20 @@ public: /// @brief Check that lease4-write works as expected. void testLease4Write(); - /// @brief Check that lease4-committed handler works as expected with + /// @brief Check that lease4_offer handler works as expected with + /// valid inputs. + void testValidLease4Offer(); + + /// @brief Check that leases4_committed handler works as expected with /// valid inputs. void testValidLeases4Committed(); + + /// @brief Check that leases4_committed handler does not throw or alter + /// the lease under NOP conditions: + /// 1. There is no repsonse packet + /// 2. Response packet is a DHCPACK + /// 3. There is no active lease + void testNopLeases4Committed(); }; void Lease4CmdsTest::testLease4AddMissingParams() { @@ -3494,7 +3505,8 @@ void Lease4CmdsTest::testLease4Write() { ss << "Scenario at line: " << line; \ SCOPED_TRACE(ss.str()); -void Lease4CmdsTest::testValidLeases4Committed() { +void +Lease4CmdsTest::testValidLease4Offer() { // Initialize lease manager (false = v4, true = add leases) initLeaseMgr(false, true); @@ -3503,6 +3515,7 @@ void Lease4CmdsTest::testValidLeases4Committed() { std::string config_; std::string orig_context_; std::string exp_context_; + uint32_t offer_lifetime_; }; std::list scenarios = { @@ -3511,8 +3524,10 @@ void Lease4CmdsTest::testValidLeases4Committed() { __LINE__, R"({})", R"({})", - R"({})" - },{ + R"({})", + 500 + }, + { // lease context has no binding-variables, two configured __LINE__, R"^({"binding-variables":[ @@ -3533,6 +3548,26 @@ void Lease4CmdsTest::testValidLeases4Committed() { "yiaddr": "192.0.2.1" } }})", + 500 + }, + { + // lease context has no binding-variables, two configured + // offer lifetime is 0. + __LINE__, + R"^({"binding-variables":[ + { + "name": "hwaddr", + "expression": "hexstring(pkt4.mac,':')", + "source": "query" + }, + { + "name": "yiaddr", + "expression": "addrtotext(pkt4.yiaddr)", + "source": "response" + }]})^", + R"({})", + R"({})", + 0 }, { // lease context has binding-variables, none configured @@ -3551,6 +3586,7 @@ void Lease4CmdsTest::testValidLeases4Committed() { "yiaddr": "192.0.2.1" } }})", + 500 }, { // Evaluated variable value is an empty string. @@ -3571,6 +3607,7 @@ void Lease4CmdsTest::testValidLeases4Committed() { "hwaddr": "" } }})", + 500 }}; // Create packet pair and lease. @@ -3581,6 +3618,139 @@ void Lease4CmdsTest::testValidLeases4Committed() { IOAddress yiaddr("192.0.2.1"); response->setYiaddr(yiaddr); + // Iterater over scenarios. + for (auto const& scenario : scenarios) { + SCOPED_LINE(scenario.line_); + + // Create and configure the manager. + BindingVariableMgrPtr mgr; + ASSERT_NO_THROW_LOG(mgr.reset(new BindingVariableMgr(AF_INET))); + ConstElementPtr config; + ASSERT_NO_THROW_LOG(config = Element::fromJSON(scenario.config_)); + ASSERT_NO_THROW_LOG(mgr->configure(config)); + + // Fetch the lease and set its user-context to the original content. + Lease4Ptr orig_lease = lmptr_->getLease4(yiaddr); + ASSERT_TRUE(orig_lease); + ConstElementPtr orig_context; + ASSERT_NO_THROW_LOG(orig_context = Element::fromJSON(scenario.orig_context_)); + orig_lease->setContext(orig_context); + ASSERT_NO_THROW_LOG(lmptr_->updateLease4(orig_lease)); + + Lease4CollectionPtr leases(new Lease4Collection()); + leases->push_back(orig_lease); + + // Create a callout handle and add the expected arguments. + CalloutHandlePtr callout_handle = HooksManager::createCalloutHandle(); + callout_handle->setArgument("query4", query); + callout_handle->setArgument("response4", response); + callout_handle->setArgument("leases4", leases); + callout_handle->setArgument("offer_lifetime", scenario.offer_lifetime_); + + // Invoke the leases4Committed handler. + LeaseCmds cmds; + ASSERT_NO_THROW_LOG(cmds.lease4Offer(*callout_handle, mgr)); + + // Fetch the lease. + Lease4Ptr after_lease = lmptr_->getLease4(yiaddr); + ASSERT_TRUE(after_lease); + + // Context contents should match the expected context content. + ConstElementPtr exp_context; + ASSERT_NO_THROW_LOG(exp_context = Element::fromJSON(scenario.exp_context_)); + ASSERT_EQ(*(after_lease->getContext()), *exp_context); + } +} + +void +Lease4CmdsTest::testValidLeases4Committed() { + // Initialize lease manager (false = v4, true = add leases) + initLeaseMgr(false, true); + + struct Scenario { + uint32_t line_; + std::string config_; + std::string orig_context_; + std::string exp_context_; + }; + + std::list scenarios = { + { + // No variables configured, nothing in lease context. + __LINE__, + R"({})", + R"({})", + R"({})" + }, + { + // lease context has no binding-variables, two configured + __LINE__, + R"^({"binding-variables":[ + { + "name": "hwaddr", + "expression": "hexstring(pkt4.mac,':')", + "source": "query" + }, + { + "name": "yiaddr", + "expression": "addrtotext(pkt4.yiaddr)", + "source": "response" + }]})^", + R"({})", + R"({"ISC":{ + "binding-variables":{ + "hwaddr": "01:02:03:04:05:06", + "yiaddr": "192.0.2.1" + } + }})", + }, + { + // lease context has binding-variables, none configured + // Current logic leaves lease untouched. + __LINE__, + R"({})", + R"({"ISC":{ + "binding-variables":{ + "hwaddr": "01:02:03:04:05:06", + "yiaddr": "192.0.2.1" + } + }})", + R"({"ISC":{ + "binding-variables":{ + "hwaddr": "01:02:03:04:05:06", + "yiaddr": "192.0.2.1" + } + }})", + }, + { + // Evaluated variable value is an empty string. + __LINE__, + R"^({"binding-variables":[ + { + "name": "hwaddr", + "expression": "''", + "source": "query" + }]})^", + R"({"ISC":{ + "binding-variables":{ + "hwaddr": "01:02:03:04:05:06" + } + }})", + R"({"ISC":{ + "binding-variables":{ + "hwaddr": "" + } + }})", + }}; + + // Create packet pair and lease. + Pkt4Ptr query(new Pkt4(DHCPREQUEST, 1234)); + query->setHWAddr(1, 6, { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 }); + + Pkt4Ptr response(new Pkt4(DHCPACK, 1234)); + IOAddress yiaddr("192.0.2.1"); + response->setYiaddr(yiaddr); + // Iterater over scenarios. for (auto const& scenario : scenarios) { SCOPED_LINE(scenario.line_); @@ -3624,6 +3794,91 @@ void Lease4CmdsTest::testValidLeases4Committed() { } } +void +Lease4CmdsTest::testNopLeases4Committed() { + // Initialize lease manager (false = v4, true = add leases) + initLeaseMgr(false, true); + + struct Scenario { + uint32_t line_; + DHCPMessageType response_type_; + bool send_lease_; + }; + + // Configure a single variable. + std::string config = + R"^({"binding-variables":[ + { + "name": "hwaddr", + "expression": "hexstring(pkt4.mac,':')", + "source": "query" + }]})^"; + + // Create and configure the manager. + BindingVariableMgrPtr mgr; + ASSERT_NO_THROW_LOG(mgr.reset(new BindingVariableMgr(AF_INET))); + ASSERT_NO_THROW_LOG(mgr->configure(Element::fromJSON(config))); + + // Scenarios should all result in no change to the lease. + std::list scenarios = { + { + // Response is not a DHCPACK. + __LINE__, + DHCPNAK, + true + }, + { + // No active lease. + __LINE__, + DHCPACK, + false + }, + { + // No response. + __LINE__, + DHCP_NOTYPE, + false + }}; + + // Create packet pair and lease. + Pkt4Ptr query(new Pkt4(DHCPREQUEST, 1234)); + query->setHWAddr(1, 6, { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 }); + IOAddress yiaddr("192.0.2.1"); + for (auto const& scenario : scenarios) { + SCOPED_LINE(scenario.line_); + + Pkt4Ptr response; + if (scenario.response_type_ != DHCP_NOTYPE) { + response.reset(new Pkt4(scenario.response_type_, 1234)); + } + + // Fetch the lease and set its user-context to the original content. + Lease4Ptr orig_lease = lmptr_->getLease4(yiaddr); + ASSERT_TRUE(orig_lease); + ASSERT_FALSE(orig_lease->getContext()); + + Lease4CollectionPtr leases(new Lease4Collection()); + if (scenario.send_lease_) { + leases->push_back(orig_lease); + } + + // Create a callout handle and add the expected arguments. + CalloutHandlePtr callout_handle = HooksManager::createCalloutHandle(); + callout_handle->setArgument("query4", query); + callout_handle->setArgument("response4", response); + callout_handle->setArgument("leases4", leases); + + // Invoke the leases4Committed handler. + LeaseCmds cmds; + ASSERT_NO_THROW_LOG(cmds.leases4Committed(*callout_handle, mgr)); + + // Fetch the lease. Context should still be empty. + Lease4Ptr after_lease = lmptr_->getLease4(yiaddr); + ASSERT_TRUE(after_lease); + ASSERT_FALSE(after_lease->getContext()); + } +} + TEST_F(Lease4CmdsTest, lease4AddMissingParams) { testLease4AddMissingParams(); } @@ -4352,6 +4607,15 @@ TEST_F(Lease4CmdsTest, lease4WriteMultiThreading) { testLease4Write(); } +TEST_F(Lease4CmdsTest, validLease4Offer) { + testValidLease4Offer(); +} + +TEST_F(Lease4CmdsTest, validLease4OfferMultiThreading) { + MultiThreadingTest mt(true); + testValidLease4Offer(); +} + TEST_F(Lease4CmdsTest, validLeases4Committed) { testValidLeases4Committed(); } @@ -4361,4 +4625,12 @@ TEST_F(Lease4CmdsTest, validLeases4CommittedMultiThreading) { testValidLeases4Committed(); } +TEST_F(Lease4CmdsTest, nopLeases4Committed) { + testNopLeases4Committed(); +} + +TEST_F(Lease4CmdsTest, nopLeases4CommittedMultiThreading) { + MultiThreadingTest mt(true); + testNopLeases4Committed(); +} } // end of anonymous namespace