diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index 56894c9a68..ced228a237 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -35,7 +35,7 @@ struct AllocEngineHooks { /// Constructor that registers hook points for AllocationEngine AllocEngineHooks() { - hook_index_lease6_select_ = HooksManager::registerHook("lease4_select"); + hook_index_lease4_select_ = HooksManager::registerHook("lease4_select"); hook_index_lease6_select_ = HooksManager::registerHook("lease6_select"); } }; @@ -577,8 +577,13 @@ Lease4Ptr AllocEngine::reuseExpiredLease(Lease4Ptr& expired, callout_handle->deleteAllArguments(); // Pass necessary arguments - // Subnet from which we do the allocation - callout_handle->setArgument("subnet4", subnet); + + // Subnet from which we do the allocation (That's as far as we can go + // with using SubnetPtr to point to Subnet4 object. Users should not + // be confused with dynamic_pointer_casts. They should get a concrete + // pointer (Subnet4Ptr) pointing to a Subnet4 object. + Subnet4Ptr subnet4 = boost::dynamic_pointer_cast(subnet); + callout_handle->setArgument("subnet4", subnet4); // Is this solicit (fake = true) or request (fake = false) callout_handle->setArgument("fake_allocation", fake_allocation); @@ -723,11 +728,17 @@ Lease4Ptr AllocEngine::createLease4(const SubnetPtr& subnet, // Pass necessary arguments - // Subnet from which we do the allocation - callout_handle->setArgument("subnet4", subnet); + // Subnet from which we do the allocation (That's as far as we can go + // with using SubnetPtr to point to Subnet4 object. Users should not + // be confused with dynamic_pointer_casts. They should get a concrete + // pointer (Subnet4Ptr) pointing to a Subnet4 object. + Subnet4Ptr subnet4 = boost::dynamic_pointer_cast(subnet); + callout_handle->setArgument("subnet4", subnet4); // Is this solicit (fake = true) or request (fake = false) callout_handle->setArgument("fake_allocation", fake_allocation); + + // Pass the intended lease as well callout_handle->setArgument("lease4", lease); // This is the first callout, so no need to clear any arguments @@ -743,7 +754,7 @@ Lease4Ptr AllocEngine::createLease4(const SubnetPtr& subnet, // Let's use whatever callout returned. Hopefully it is the same lease // we handled to it. - callout_handle->getArgument("lease6", lease); + callout_handle->getArgument("lease4", lease); } diff --git a/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc index ef9b294fc8..5a9ce29ad1 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine_unittest.cc @@ -991,6 +991,8 @@ TEST_F(AllocEngine4Test, requestReuseExpiredLease4) { detailCompareLease(lease, from_mgr); } +/// @todo write renewLease6 + // This test checks if a lease is really renewed when renewLease4 method is // called TEST_F(AllocEngine4Test, renewLease4) { @@ -1245,4 +1247,216 @@ TEST_F(HookAllocEngine6Test, change_lease6_select) { EXPECT_EQ(valid_override_, from_mgr->valid_lft_); } + +/// @brief helper class used in Hooks testing in AllocEngine4 +/// +/// It features a couple of callout functions and buffers to store +/// the data that is accessible via callouts. +class HookAllocEngine4Test : public AllocEngine4Test { +public: + HookAllocEngine4Test() { + resetCalloutBuffers(); + } + + virtual ~HookAllocEngine4Test() { + HooksManager::preCalloutsLibraryHandle().deregisterAllCallouts( + "lease4_select"); + } + + /// @brief clears out buffers, so callouts can store received arguments + void resetCalloutBuffers() { + callback_name_ = string(""); + callback_subnet4_.reset(); + callback_fake_allocation_ = false; + callback_lease4_.reset(); + callback_argument_names_.clear(); + callback_addr_original_ = IOAddress("::"); + callback_addr_updated_ = IOAddress("::"); + } + + /// callback that stores received callout name and received values + static int + lease4_select_callout(CalloutHandle& callout_handle) { + + callback_name_ = string("lease4_select"); + + callout_handle.getArgument("subnet4", callback_subnet4_); + callout_handle.getArgument("fake_allocation", callback_fake_allocation_); + callout_handle.getArgument("lease4", callback_lease4_); + + callback_addr_original_ = callback_lease4_->addr_; + + callback_argument_names_ = callout_handle.getArgumentNames(); + return (0); + } + + /// callback that overrides the lease with different values + static int + lease4_select_different_callout(CalloutHandle& callout_handle) { + + // Let's call the basic callout, so it can record all parameters + lease4_select_callout(callout_handle); + + // Now we need to tweak the least a bit + Lease4Ptr lease; + callout_handle.getArgument("lease4", lease); + callback_addr_updated_ = addr_override_; + lease->addr_ = callback_addr_updated_; + lease->t1_ = t1_override_; + lease->t2_ = t2_override_; + lease->valid_lft_ = valid_override_; + + return (0); + } + + // Values to be used in callout to override lease4 content + static const IOAddress addr_override_; + static const uint32_t t1_override_; + static const uint32_t t2_override_; + static const uint32_t valid_override_; + + // Callback will store original and overridden values here + static IOAddress callback_addr_original_; + static IOAddress callback_addr_updated_; + + // Buffers (callback will store received values here) + static string callback_name_; + static Subnet4Ptr callback_subnet4_; + static Lease4Ptr callback_lease4_; + static bool callback_fake_allocation_; + static vector callback_argument_names_; +}; + +// For some reason intialization within a class makes the linker confused. +// linker complains about undefined references if they are defined within +// the class declaration. +const IOAddress HookAllocEngine4Test::addr_override_("192.0.3.1"); +const uint32_t HookAllocEngine4Test::t1_override_ = 4000; +const uint32_t HookAllocEngine4Test::t2_override_ = 7000; +const uint32_t HookAllocEngine4Test::valid_override_ = 9000; + +IOAddress HookAllocEngine4Test::callback_addr_original_("::"); +IOAddress HookAllocEngine4Test::callback_addr_updated_("::"); + +string HookAllocEngine4Test::callback_name_; +Subnet4Ptr HookAllocEngine4Test::callback_subnet4_; +Lease4Ptr HookAllocEngine4Test::callback_lease4_; +bool HookAllocEngine4Test::callback_fake_allocation_; +vector HookAllocEngine4Test::callback_argument_names_; + +// This test checks if the lease4_select callout is executed and expected +// parameters as passed. +TEST_F(HookAllocEngine4Test, lease4_select) { + + // Note: The following order is working as expected: + // 1. create AllocEngine (that register hook points) + // 2. call loadLibraries() + // + // This order, however, causes segfault in HooksManager + // 1. call loadLibraries() + // 2. create AllocEngine (that register hook points) + + // Create allocation engine (hook names are registered in its ctor) + boost::scoped_ptr engine; + ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100))); + ASSERT_TRUE(engine); + + // Initialize Hooks Manager + vector libraries; // no libraries at this time + HooksManager::getHooksManager().loadLibraries(libraries); + + // Install pkt4_receive_callout + EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout( + "lease4_select", lease4_select_callout)); + + CalloutHandlePtr callout_handle = HooksManager::getHooksManager().createCalloutHandle(); + + Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_, + IOAddress("0.0.0.0"), + false, callout_handle); + // Check that we got a lease + ASSERT_TRUE(lease); + + // Do all checks on the lease + checkLease4(lease); + + // Check that the lease is indeed in LeaseMgr + Lease4Ptr from_mgr = LeaseMgrFactory::instance().getLease4(lease->addr_); + ASSERT_TRUE(from_mgr); + + // Check that callouts were indeed called + EXPECT_EQ("lease4_select", callback_name_); + + // Now check that the lease in LeaseMgr has the same parameters + ASSERT_TRUE(callback_lease4_); + detailCompareLease(callback_lease4_, from_mgr); + + ASSERT_TRUE(callback_subnet4_); + EXPECT_EQ(subnet_->toText(), callback_subnet4_->toText()); + + EXPECT_EQ(callback_fake_allocation_, false); + + // Check if all expected parameters are reported. It's a bit tricky, because + // order may be different. If the test starts failing, because someone tweaked + // hooks engine, we'll have to implement proper vector matching (ignoring order) + vector expected_argument_names; + expected_argument_names.push_back("fake_allocation"); + expected_argument_names.push_back("lease4"); + expected_argument_names.push_back("subnet4"); + EXPECT_TRUE(callback_argument_names_ == expected_argument_names); +} + +// This test checks if lease4_select callout is able to override the values +// in a lease4. +TEST_F(HookAllocEngine4Test, change_lease4_select) { + + // Make sure that the overridden values are different than the ones from + // subnet originally used to create the lease + ASSERT_NE(t1_override_, subnet_->getT1()); + ASSERT_NE(t2_override_, subnet_->getT2()); + ASSERT_NE(valid_override_, subnet_->getValid()); + ASSERT_FALSE(subnet_->inRange(addr_override_)); + + // Create allocation engine (hook names are registered in its ctor) + boost::scoped_ptr engine; + ASSERT_NO_THROW(engine.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100))); + ASSERT_TRUE(engine); + + // Initialize Hooks Manager + vector libraries; // no libraries at this time + HooksManager::getHooksManager().loadLibraries(libraries); + + // Install a callout + EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().registerCallout( + "lease4_select", lease4_select_different_callout)); + + // Normally, dhcpv4_srv would passed the handle when calling allocateAddress4, + // but in tests we need to create it on our own. + CalloutHandlePtr callout_handle = HooksManager::getHooksManager().createCalloutHandle(); + + // Call allocateAddress4. Callouts should be triggered here. + Lease4Ptr lease = engine->allocateAddress4(subnet_, clientid_, hwaddr_, IOAddress("0.0.0.0"), + false, callout_handle); + // Check that we got a lease + ASSERT_TRUE(lease); + + // See if the values overridden by callout are there + EXPECT_TRUE(lease->addr_.equals(addr_override_)); + EXPECT_EQ(t1_override_, lease->t1_); + EXPECT_EQ(t2_override_, lease->t2_); + EXPECT_EQ(valid_override_, lease->valid_lft_); + + // Now check if the lease is in the database + Lease4Ptr from_mgr = LeaseMgrFactory::instance().getLease4(lease->addr_); + ASSERT_TRUE(from_mgr); + + // Check if values in the database are overridden + EXPECT_TRUE(from_mgr->addr_.equals(addr_override_)); + EXPECT_EQ(t1_override_, from_mgr->t1_); + EXPECT_EQ(t2_override_, from_mgr->t2_); + EXPECT_EQ(valid_override_, from_mgr->valid_lft_); +} + + + }; // End of anonymous namespace