From 68484ade702c714a000f5a94267bdc57346f2607 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Tue, 18 Feb 2025 13:52:50 -0500 Subject: [PATCH] [#3463] Addressed most of review comments --- ...esponse-dhcp-options-in-lease-user-context | 2 +- doc/sphinx/arm/hooks-lease-cmds.rst | 4 +- src/bin/dhcp4/dhcp4_hooks.dox | 4 + src/bin/dhcp6/dhcp6_hooks.dox | 1 + .../dhcp/lease_cmds/binding_variables.cc | 15 +- src/hooks/dhcp/lease_cmds/binding_variables.h | 20 +- src/hooks/dhcp/lease_cmds/lease_cmds.cc | 12 +- src/hooks/dhcp/lease_cmds/lease_cmds.h | 8 +- .../dhcp/lease_cmds/lease_cmds_callouts.cc | 14 +- .../dhcp/lease_cmds/lease_cmds_messages.cc | 2 + .../dhcp/lease_cmds/lease_cmds_messages.h | 1 + .../dhcp/lease_cmds/lease_cmds_messages.mes | 6 + .../libloadtests/lease_cmds6_unittest.cc | 382 -------- src/hooks/dhcp/lease_cmds/tests/Makefile.am | 1 - .../tests/binding_variables_unittest.cc | 890 +----------------- .../tests/lease_cmds_func4_unittest.cc | 40 +- .../tests/lease_cmds_func6_unittest.cc | 12 +- .../tests/lease_cmds_func_unittest.h | 9 +- src/lib/config/unix_command_mgr.h | 4 +- src/lib/dhcpsrv/lease.h | 2 +- 20 files changed, 91 insertions(+), 1338 deletions(-) diff --git a/changelog_unreleased/3463-optionally-store-response-dhcp-options-in-lease-user-context b/changelog_unreleased/3463-optionally-store-response-dhcp-options-in-lease-user-context index 46419af3af..a526440d48 100644 --- a/changelog_unreleased/3463-optionally-store-response-dhcp-options-in-lease-user-context +++ b/changelog_unreleased/3463-optionally-store-response-dhcp-options-in-lease-user-context @@ -3,4 +3,4 @@ custom values, referred to as ``binding-variables``, within the lease's ``user-context``. Supported in both kea-dhcp4 and kea-dhcp6. - Gitlab #3463) + (Gitlab #3463) diff --git a/doc/sphinx/arm/hooks-lease-cmds.rst b/doc/sphinx/arm/hooks-lease-cmds.rst index b4e6e8fbc9..6c628350e9 100644 --- a/doc/sphinx/arm/hooks-lease-cmds.rst +++ b/doc/sphinx/arm/hooks-lease-cmds.rst @@ -17,7 +17,7 @@ of the subnet to which it is supposed to belong. The library also provides a non-programmatic way to manage user contexts associated with leases. -As of Kea 2.7.7, the library also provides support for ``binding-varriables``. Binding +As of Kea 2.7.7, the library also provides support for ``binding-variables``. Binding variables allow the user to store custom values for each lease. They are discussed here: :ref:`binding-variables`. @@ -1130,7 +1130,7 @@ lease: { "hooks-libraries": [{ - "library": "lib/kea/hooks/libdhcp_lease_cmds.so", + "library": "/path/libdhcp_lease_cmds.so" "parameters": { "binding-variables": [{ "name": "opt-222", diff --git a/src/bin/dhcp4/dhcp4_hooks.dox b/src/bin/dhcp4/dhcp4_hooks.dox index a9422953bd..6f642fed38 100644 --- a/src/bin/dhcp4/dhcp4_hooks.dox +++ b/src/bin/dhcp4/dhcp4_hooks.dox @@ -298,6 +298,7 @@ called before "subnet4_select". - @b Arguments: - name: @b query4, type: isc::dhcp::Pkt4Ptr, direction: in + - name: @b response4, type: isc::dhcp::Pkt4Ptr, direction: in - name: @b leases4, type: isc::dhcp::Lease4CollectionPtr, direction: in - name: @b deleted_leases4, type: isc::dhcp::Lease4CollectionPtr, direction: in @@ -328,6 +329,7 @@ called before "subnet4_select". - @b Arguments: - name: @b query4, type: isc::dhcp::Pkt4Ptr, direction: in + - name: @b response4, type: isc::dhcp::Pkt4Ptr, direction: in - name: @b leases4, type: isc::dhcp::Lease4CollectionPtr, direction: in - name: @b offer_lifetime, type: uint32_t, direction: in - name: @b old_lease, type: isc::dhcp::Lease4Ptr, direction: in @@ -336,6 +338,8 @@ called before "subnet4_select". DHCPDISCOVER from the client and the DHCPOFFER has been constructed but not yet sent back. The argument "query4" contains a pointer to an isc::dhcp::Pkt4 object that contains all information regarding incoming DHCPDISCOVER packet. + The "response4" argument contains a pointer to an isc::dhcp::Pkt4 object that + contains all information regarding the outgoing::DHCPOFFER packet. The "leases4" object will hold the pointer to the new candidate lease for this client. The "offer_lifetime" argument comes from the context information for the DHCPv4 lease allocation. If "offer_lifetime" is not zero, it indicates that temporary diff --git a/src/bin/dhcp6/dhcp6_hooks.dox b/src/bin/dhcp6/dhcp6_hooks.dox index d79ed82907..d91aca8d2f 100644 --- a/src/bin/dhcp6/dhcp6_hooks.dox +++ b/src/bin/dhcp6/dhcp6_hooks.dox @@ -349,6 +349,7 @@ called before "subnet6_select". - @b Arguments: - name: @b query6, type: isc::dhcp::Pkt6Ptr, direction: in + - name: @b response6, type: isc::dhcp::Pkt6Ptr, direction: in - name: @b leases6, type: isc::dhcp::Lease6CollectionPtr, direction: in - name: @b deleted_leases6, type: isc::dhcp::Lease6CollectionPtr, direction: in diff --git a/src/hooks/dhcp/lease_cmds/binding_variables.cc b/src/hooks/dhcp/lease_cmds/binding_variables.cc index 09e45a4976..a47ec9b8fd 100644 --- a/src/hooks/dhcp/lease_cmds/binding_variables.cc +++ b/src/hooks/dhcp/lease_cmds/binding_variables.cc @@ -54,9 +54,9 @@ BindingVariable::BindingVariable(const std::string& name, const data::SimpleKeywords BindingVariable::CONFIG_KEYWORDS = { - {"name", Element::string}, - {"expression", Element::string}, - {"source", Element::string} + { "name", Element::string }, + { "expression", Element::string }, + { "source", Element::string } }; BindingVariablePtr @@ -138,13 +138,6 @@ BindingVariableCache::getLastFlushTime() { return (BaseStampedElement::getModificationTime()); } -/// @brief Tag for the name index. -//struct VariableNameTag { }; - -/// @brief Tag for the source index. -//struct VariableSourceTag { }; - - BindingVariableListPtr BindingVariableCache::getAll() { util::MultiThreadingLock lock(*mutex_); @@ -153,7 +146,7 @@ BindingVariableCache::getAll() { const auto& index = variables_.get(); for (auto const& variable : index) { /// For now we'll return the pointer, w/o making a copy - /// of the varaiable itself. We never updates variables + /// of the variable itself. We never updates variables /// so we should be OK. var_list->push_back(variable); } diff --git a/src/hooks/dhcp/lease_cmds/binding_variables.h b/src/hooks/dhcp/lease_cmds/binding_variables.h index e7f7af1322..f60b51cad1 100644 --- a/src/hooks/dhcp/lease_cmds/binding_variables.h +++ b/src/hooks/dhcp/lease_cmds/binding_variables.h @@ -26,7 +26,6 @@ namespace isc { namespace lease_cmds { - // Forward declaration for pointer. class BindingVariable; @@ -61,10 +60,10 @@ public: /// protocol family. /// @throw BadValue if name if empty, or expression string fails /// to parse. - explicit BindingVariable(const std::string& name, - const std::string& expression_str, - const Source& source, - uint32_t family); + BindingVariable(const std::string& name, + const std::string& expression_str, + const Source& source, + uint32_t family); /// @brief Destructor virtual ~BindingVariable() = default; @@ -73,7 +72,7 @@ public: /// /// @param config Map Element containing parameters for a single binding variable. /// @param family Protocol family of the variable, either AF_INET or AF_INET6. - /// @return Pointer to the newly created BindingVariable instacne. + /// @return Pointer to the newly created BindingVariable instance. /// @throw DhcpConfigError if configuration parameters are invalid. static BindingVariablePtr parse(data::ConstElementPtr config, uint16_t family); @@ -176,7 +175,6 @@ typedef boost::multi_index_container< std::string, &BindingVariable::getName> >, - // Third index is by source. boost::multi_index::ordered_non_unique< boost::multi_index::tag, @@ -251,7 +249,7 @@ private: /// @brief Defines a shared pointer to a BindingVariableCache. typedef boost::shared_ptr BindingVariableCachePtr; -/// @brief Singleton which warehouses the configuraed binding variables, +/// @brief Singleton which warehouses the configured binding variables, /// and evaluation of variables for a given lease and packet pair. class BindingVariableMgr { public: @@ -259,7 +257,7 @@ public: /// /// @param family Protocol family of the expression, either /// AF_INET or AF_INET6. - explicit BindingVariableMgr(uint32_t family_); + explicit BindingVariableMgr(uint32_t family); /// @brief Destructor; ~BindingVariableMgr() = default; @@ -297,8 +295,8 @@ public: /// @param response Server response conveying the lease. Variables whose source /// is "response" will be evaluated against this packet. /// @param lease Lease whose use-context will be updated with the evaluation - /// results. If the results of the evaluation are idnentical the the lease's - /// exising binding-variables value, the lease is not altered. This allows + /// results. If the results of the evaluation are identical the the lease's + /// existing binding-variables value, the lease is not altered. This allows /// a subsequent lease store update to only occur when needed. /// @return True if the lease's user-context was udpated, false otherwise. bool evaluateVariables(dhcp::PktPtr query, dhcp::PktPtr response, diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds.cc b/src/hooks/dhcp/lease_cmds/lease_cmds.cc index db606ce315..114120dc52 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds.cc +++ b/src/hooks/dhcp/lease_cmds/lease_cmds.cc @@ -480,7 +480,7 @@ public: /// user-context accordingly. This includes updating the lease in the lease /// back end. /// - /// @param handle Callout context - which is expected to contain the query4, + /// @param callout_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 @@ -495,7 +495,7 @@ public: /// user-context accordingly. This includes updating the lease in the lease /// back end. /// - /// @param handle Callout context - which is expected to contain the query4, + /// @param callout_handle Callout context - which is expected to contain the query4, /// response4, and leases4 arguments. /// @param mgr Pointer to the BindingVariableMgr singleton. /// @throw Unexpected if a processing error occurs. LeaseCmdsConflict if the @@ -509,7 +509,7 @@ public: /// user-context accordingly. This includes updating the leases in the lease /// back end. /// - /// @param handle Callout context - which is expected to contain the query6, + /// @param callout_handle Callout context - which is expected to contain the query6, /// response6, and leases6 arguments. /// @param mgr Pointer to the BindingVariableMgr singleton. /// @throw Unexpected if there a processing error occurs. LeaseCmdsConflict @@ -2762,7 +2762,7 @@ LeaseCmdsImpl::lease4Offer(CalloutHandle& callout_handle, callout_handle.getArgument("response4", response); callout_handle.getArgument("leases4", leases); - if (leases->empty()) { + if (!leases || leases->empty() || !((*leases)[0])) { isc_throw(Unexpected, "lease4Offer - no lease!"); } @@ -2801,6 +2801,10 @@ LeaseCmdsImpl::leases4Committed(CalloutHandle& callout_handle, } Lease4Ptr lease = (*leases)[0]; + if (!lease) { + isc_throw(Unexpected, "leases4Committed - no lease!"); + } + try { if (mgr->evaluateVariables(query, response, lease)) { LeaseMgrFactory::instance().updateLease4(lease); diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds.h b/src/hooks/dhcp/lease_cmds/lease_cmds.h index a50699b877..c7038ca289 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds.h +++ b/src/hooks/dhcp/lease_cmds/lease_cmds.h @@ -609,11 +609,11 @@ public: /// @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 + /// 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, + /// @param callout_handle Callout context - which is expected to contain the query4, response4, /// leases4 arguments, and offer_lifetime. /// @param mgr Pointer to the BindingVariableMgr singleton. void @@ -626,7 +626,7 @@ public: /// 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, + /// @param callout_handle Callout context - which is expected to contain the query4, response4, /// and leases4 arguments. /// @param mgr Pointer to the BindingVariableMgr singleton. void @@ -639,7 +639,7 @@ public: /// user-context accordingly. This includes updating the leases in the lease /// back end. /// - /// @param handle Callout context - which is expected to contain the query6, response6, + /// @param callout_handle Callout context - which is expected to contain the query6, response6, /// and leases6 arguments. /// @param mgr Pointer to the BindingVariableMgr singleton. void diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds_callouts.cc b/src/hooks/dhcp/lease_cmds/lease_cmds_callouts.cc index 9744dc47b3..8f772981a2 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds_callouts.cc +++ b/src/hooks/dhcp/lease_cmds/lease_cmds_callouts.cc @@ -353,10 +353,16 @@ int load(LibraryHandle& handle) { // Instantiate the binding-variables manager singleton. binding_var_mgr.reset(new BindingVariableMgr(family)); - // Configure binding variable manager using the hook library's parameters. - ConstElementPtr json = handle.getParameters(); - if (json) { - binding_var_mgr->configure(json); + try { + // Configure binding variable manager using the hook library's parameters. + ConstElementPtr json = handle.getParameters(); + if (json) { + binding_var_mgr->configure(json); + } + } catch (const std::exception& ex) { + LOG_ERROR(lease_cmds_logger, LEASE_CMDS_LOAD_ERROR) + .arg(ex.what()); + return (1); } LOG_INFO(lease_cmds_logger, LEASE_CMDS_INIT_OK); diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds_messages.cc b/src/hooks/dhcp/lease_cmds/lease_cmds_messages.cc index 815a4f145c..b9797109ca 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds_messages.cc +++ b/src/hooks/dhcp/lease_cmds/lease_cmds_messages.cc @@ -25,6 +25,7 @@ extern const isc::log::MessageID LEASE_CMDS_LEASES4_COMMITTED_FAILED = "LEASE_CM extern const isc::log::MessageID LEASE_CMDS_LEASES6_COMMITTED_CONFLICT = "LEASE_CMDS_LEASES6_COMMITTED_CONFLICT"; extern const isc::log::MessageID LEASE_CMDS_LEASES6_COMMITTED_FAILED = "LEASE_CMDS_LEASES6_COMMITTED_FAILED"; extern const isc::log::MessageID LEASE_CMDS_LEASES6_COMMITTED_LEASE_ERROR = "LEASE_CMDS_LEASES6_COMMITTED_LEASE_ERROR"; +extern const isc::log::MessageID LEASE_CMDS_LOAD_ERROR = "LEASE_CMDS_LOAD_ERROR"; 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"; @@ -66,6 +67,7 @@ const char* values[] = { "LEASE_CMDS_LEASES6_COMMITTED_CONFLICT", "could not updating lease: %1 for: %2", "LEASE_CMDS_LEASES6_COMMITTED_FAILED", "reason: %1", "LEASE_CMDS_LEASES6_COMMITTED_LEASE_ERROR", "evaluating binding-variables for lease: %1 for: %2, reason: %3", + "LEASE_CMDS_LOAD_ERROR", "loading Lease Commands hooks library failed: %1", "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 4a696a9100..9db140f487 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds_messages.h +++ b/src/hooks/dhcp/lease_cmds/lease_cmds_messages.h @@ -26,6 +26,7 @@ extern const isc::log::MessageID LEASE_CMDS_LEASES4_COMMITTED_FAILED; extern const isc::log::MessageID LEASE_CMDS_LEASES6_COMMITTED_CONFLICT; extern const isc::log::MessageID LEASE_CMDS_LEASES6_COMMITTED_FAILED; extern const isc::log::MessageID LEASE_CMDS_LEASES6_COMMITTED_LEASE_ERROR; +extern const isc::log::MessageID LEASE_CMDS_LOAD_ERROR; 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 73ea4c024c..78fd9a3761 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds_messages.mes +++ b/src/hooks/dhcp/lease_cmds/lease_cmds_messages.mes @@ -163,3 +163,9 @@ The lease6-wipe command is deprecated and it will be removed in the future. % LEASE_CMDS_WIPE6_FAILED lease6-wipe command failed (parameters: %1, reason: %2) The lease6-wipe command has failed. Both the reason as well as the parameters passed are logged. + +% LEASE_CMDS_LOAD_ERROR loading Lease Commands hooks library failed: %1 +This error message indicates an error loading the Lease Commands +hooks library. The details of the error are provided as argument of +the log message. + diff --git a/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds6_unittest.cc b/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds6_unittest.cc index 29ec92aff4..bcf717177c 100644 --- a/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds6_unittest.cc +++ b/src/hooks/dhcp/lease_cmds/libloadtests/lease_cmds6_unittest.cc @@ -480,21 +480,6 @@ public: /// @brief Check that lease6-write works as expected. void testLease6Write(); - -#if 0 - /// @brief Check that leases6_committed handler works as expected with - /// valid inputs. - void testValidLeases6Committed(); - - /// @brief Check that leases6_committed handler does not throw or alter - /// leases under NOP conditions: - /// 1. There is no repsonse packet - /// 2. There are no leases - /// 3. There are leases but none active - void testNopLeases6Committed(); - - void testLeases6CommittedErrors(); -#endif }; void Lease6CmdsTest::testLease6AddMissingParams() { @@ -4236,344 +4221,6 @@ void Lease6CmdsTest::testLease6Write() { testCommand(txt, CONTROL_RESULT_ERROR, exp_rsp); } -#if 0 -void -Lease6CmdsTest::testValidLeases6Committed() { - // Initialize lease manager (true = v6, true = add leases) - initLeaseMgr(true, 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": "duid", - "expression": "hexstring(option[1].hex,':')", - "source": "query" - }, - { - "name": "sub-id", - "expression": "hexstring(option[38].hex, ':')", - "source": "response" - }]})^", - R"({})", - R"({"ISC":{ - "binding-variables":{ - "duid": "01:02:03:04", - "sub-id": "05:06:07:08" - } - }})", - }, - { - // Lease context has binding-variables, none configured. - // Current logic leaves lease untouched. - __LINE__, - R"({})", - R"({"ISC":{ - "binding-variables":{ - "duid": "01:02:03:04", - "sub-id": "05:06:07:08" - } - }})", - R"({"ISC":{ - "binding-variables":{ - "duid": "01:02:03:04", - "sub-id": "05:06:07:08" - } - }})", - }, - { - // Evaluated variable value is an empty string. - __LINE__, - R"^({"binding-variables":[ - { - "name": "my-var", - "expression": "''", - "source": "query" - }]})^", - R"({"ISC":{ - "binding-variables":{ - "my-var": "pre-existing value" - } - }})", - R"({"ISC":{ - "binding-variables":{ - "my-var": "" - } - }})", - } - }; - - // Create packet pair and lease. - Pkt6Ptr query(new Pkt6(DHCPV6_REQUEST, 1234)); - OptionPtr client_id(new Option(Option::V6, D6O_CLIENTID, - { 0x01, 0x02, 0x03, 0x04 })); - query->addOption(client_id); - - Pkt6Ptr response(new Pkt6(DHCPV6_REPLY, 7890)); - OptionPtr subscriber_id(new Option(Option::V6, D6O_SUBSCRIBER_ID, - { 0x05, 0x06, 0x07, 0x08 })); - response->addOption(subscriber_id); - - // Create a list of the lease addresses. - std::list lease_addrs; - lease_addrs.push_back(IOAddress("2001:db8:1::1")); - lease_addrs.push_back(IOAddress("2001:db8:1::2")); - lease_addrs.push_back(IOAddress("2001:db8:2::2")); - - // 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_INET6))); - 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. - Lease6CollectionPtr orig_leases(new Lease6Collection()); - for (auto const& address : lease_addrs) { - Lease6Ptr orig_lease = lmptr_->getLease6(Lease::TYPE_NA, address); - ASSERT_TRUE(orig_lease); - ASSERT_TRUE(orig_lease->valid_lft_ > 0); - - ConstElementPtr orig_context; - ASSERT_NO_THROW_LOG(orig_context = Element::fromJSON(scenario.orig_context_)); - orig_lease->setContext(orig_context); - ASSERT_NO_THROW_LOG(lmptr_->updateLease6(orig_lease)); - orig_leases->push_back(orig_lease); - } - - // Create a callout handle and add the expected arguments. - CalloutHandlePtr callout_handle = HooksManager::createCalloutHandle(); - callout_handle->setArgument("query6", query); - callout_handle->setArgument("response6", response); - callout_handle->setArgument("leases6", orig_leases); - - // Invoke the leases6Committed handler. - LeaseCmds cmds; - ASSERT_NO_THROW_LOG(cmds.leases6Committed(*callout_handle, mgr)); - - // Iterate over the leases and make sure the user-contexts are as expected. - for (auto const& lease : *orig_leases) { - // Fetch the lease. - Lease6Ptr after_lease = lmptr_->getLease6(Lease::TYPE_NA, lease->addr_); - 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 -Lease6CmdsTest::testNopLeases6Committed() { - // Initialize lease manager (true = v6, true = add leases) - initLeaseMgr(true, true); - - struct Scenario { - uint32_t line_; - DHCPv6MessageType response_type_; - bool send_lease_; - uint32_t valid_lft_; - }; - - // Configure a single variable. - std::string config = - R"^({"binding-variables":[ - { - "name": "duid", - "expression": "hexstring(option[1].hex,':')", - "source": "query" - }]})^"; - - // Create and configure the manager. - BindingVariableMgrPtr mgr; - ASSERT_NO_THROW_LOG(mgr.reset(new BindingVariableMgr(AF_INET6))); - ASSERT_NO_THROW_LOG(mgr->configure(Element::fromJSON(config))); - - // Scenarios should all result in no change to the lease. - std::list scenarios = { - { - // No leases. - __LINE__, - DHCPV6_REPLY, - false, - 0 - }, - { - // No active leases. - __LINE__, - DHCPV6_REPLY, - true, - 0 - }, - { - // No response. - __LINE__, - DHCPV6_NOTYPE, - true, - 1000 - }}; - - // Create query packet. - Pkt6Ptr query(new Pkt6(DHCPV6_REQUEST, 1234)); - OptionPtr client_id(new Option(Option::V6, D6O_CLIENTID, - { 0x01, 0x02, 0x03, 0x04 })); - query->addOption(client_id); - - IOAddress na_addr("2001:db8:1::1"); - for (auto const& scenario : scenarios) { - SCOPED_LINE(scenario.line_); - - // Create the response packet, if one. - Pkt6Ptr response; - if (scenario.response_type_ != DHCPV6_NOTYPE) { - response.reset(new Pkt6(scenario.response_type_, 1234)); - } - - - // Fetch the lease and verify there is no context content. - Lease6Ptr orig_lease = lmptr_->getLease6(Lease::TYPE_NA, na_addr); - ASSERT_TRUE(orig_lease); - ASSERT_FALSE(orig_lease->getContext()); - orig_lease->valid_lft_ = scenario.valid_lft_; - - Lease6CollectionPtr leases(new Lease6Collection()); - 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("query6", query); - callout_handle->setArgument("response6", response); - callout_handle->setArgument("leases6", leases); - - // Invoke the leases6Committed handler. - LeaseCmds cmds; - ASSERT_NO_THROW_LOG(cmds.leases6Committed(*callout_handle, mgr)); - - // Fetch the lease. Context should still be empty. - Lease6Ptr after_lease = lmptr_->getLease6(Lease::TYPE_NA, na_addr); - ASSERT_TRUE(after_lease); - ASSERT_FALSE(after_lease->getContext()); - } -} - -void -Lease6CmdsTest::testLeases6CommittedErrors() { - // Initialize lease manager (false = v4, true = add leases) - initLeaseMgr(true, true); - - // Create a config with two binding variables. - std::string config_str = - R"^({"binding-variables":[ - { - "name": "duid", - "expression": "hexstring(option[1].hex,':')", - "source": "query" - }, - { - "name": "sub-id", - "expression": "hexstring(option[38].hex, ':')", - "source": "response" - }]})^"; - - ConstElementPtr config; - ASSERT_NO_THROW_LOG(config = Element::fromJSON(config_str)); - - // Create the expected context contents. - std::string exp_context_str = - R"({"ISC":{ - "binding-variables":{ - "duid": "01:02:03:04", - "sub-id": "05:06:07:08" - } - }})"; - - - ConstElementPtr exp_context; - ASSERT_NO_THROW_LOG(exp_context = Element::fromJSON(exp_context_str)); - - // Create packet pair and lease. - Pkt6Ptr query(new Pkt6(DHCPV6_REQUEST, 1234)); - OptionPtr client_id(new Option(Option::V6, D6O_CLIENTID, - { 0x01, 0x02, 0x03, 0x04 })); - query->addOption(client_id); - - Pkt6Ptr response(new Pkt6(DHCPV6_REPLY, 7890)); - OptionPtr subscriber_id(new Option(Option::V6, D6O_SUBSCRIBER_ID, - { 0x05, 0x06, 0x07, 0x08 })); - response->addOption(subscriber_id); - - // Create and configure the manager. - BindingVariableMgrPtr mgr; - ASSERT_NO_THROW_LOG(mgr.reset(new BindingVariableMgr(AF_INET6))); - ASSERT_NO_THROW_LOG(mgr->configure(config)); - - // Fetch the leases. - std::list lease_addrs; - lease_addrs.push_back(IOAddress("2001:db8:1::1")); - lease_addrs.push_back(IOAddress("2001:db8:1::2")); - lease_addrs.push_back(IOAddress("2001:db8:2::2")); - - Lease6CollectionPtr orig_leases(new Lease6Collection()); - for (auto const& address : lease_addrs) { - Lease6Ptr orig_lease = lmptr_->getLease6(Lease::TYPE_NA, address); - ASSERT_TRUE(orig_lease); - orig_leases->push_back(orig_lease); - } - - // Delete the middle lease from the back end. This should cause a conflict error. - ASSERT_NO_THROW_LOG(lmptr_->deleteLease((*orig_leases)[1])); - - // Create a callout handle and add the expected arguments. - CalloutHandlePtr callout_handle = HooksManager::createCalloutHandle(); - callout_handle->setArgument("query6", query); - callout_handle->setArgument("response6", response); - callout_handle->setArgument("leases6", orig_leases); - - // Invoke the leases6Committed handler. - LeaseCmds cmds; - ASSERT_THROW_MSG(cmds.leases6Committed(*callout_handle, mgr), - Unexpected, - "1 out of 3 leases failed to update for " - "duid=[01:02:03:04], [no hwaddr info], tid=0x4d2"); - - for (auto const& lease : *orig_leases) { - // Fetch the lease. - Lease6Ptr after_lease = lmptr_->getLease6(Lease::TYPE_NA, lease->addr_); - if (after_lease) { - // Context contents should match the expected context content. - ASSERT_EQ(*(after_lease->getContext()), *exp_context); - } else { - // Middle lease should not be found. - EXPECT_EQ(lease->addr_, (*orig_leases)[1]->addr_); - } - } -} -#endif - TEST_F(Lease6CmdsTest, lease6AddMissingParams) { testLease6AddMissingParams(); } @@ -5355,33 +5002,4 @@ TEST_F(Lease6CmdsTest, lease6WriteMultiThreading) { testLease6Write(); } -#if 0 -TEST_F(Lease6CmdsTest, validLeases6Committed) { - testValidLeases6Committed(); -} - -TEST_F(Lease6CmdsTest, validLeases6CommittedMultiThreading) { - MultiThreadingTest mt(true); - testValidLeases6Committed(); -} - -TEST_F(Lease6CmdsTest, nopLeases6Committed) { - testNopLeases6Committed(); -} - -TEST_F(Lease6CmdsTest, nopLeases6CommittedMultiThreading) { - MultiThreadingTest mt(true); - testNopLeases6Committed(); -} - -TEST_F(Lease6CmdsTest, leases6CommittedErrors) { - testLeases6CommittedErrors(); -} - -TEST_F(Lease6CmdsTest, leases6CommittedErrorsMultiThreading) { - MultiThreadingTest mt(true); - testLeases6CommittedErrors(); -} -#endif - } // end of anonymous namespace diff --git a/src/hooks/dhcp/lease_cmds/tests/Makefile.am b/src/hooks/dhcp/lease_cmds/tests/Makefile.am index 6e440e87c3..f9394ba8cd 100644 --- a/src/hooks/dhcp/lease_cmds/tests/Makefile.am +++ b/src/hooks/dhcp/lease_cmds/tests/Makefile.am @@ -3,7 +3,6 @@ SUBDIRS = . AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib AM_CPPFLAGS += -I$(top_builddir)/src/hooks/dhcp/lease_cmds -I$(top_srcdir)/src/hooks/dhcp/lease_cmds AM_CPPFLAGS += $(BOOST_INCLUDES) $(CRYPTO_CFLAGS) $(CRYPTO_INCLUDES) -AM_CPPFLAGS += -DLEASE_CMDS_LIB_SO=\"$(abs_top_builddir)/src/hooks/dhcp/lease_cmds/.libs/libdhcp_lease_cmds.so\" AM_CPPFLAGS += -DINSTALL_PROG=\"$(abs_top_srcdir)/install-sh\" AM_CXXFLAGS = $(KEA_CXXFLAGS) diff --git a/src/hooks/dhcp/lease_cmds/tests/binding_variables_unittest.cc b/src/hooks/dhcp/lease_cmds/tests/binding_variables_unittest.cc index 68fd8897ca..798e77a4ab 100644 --- a/src/hooks/dhcp/lease_cmds/tests/binding_variables_unittest.cc +++ b/src/hooks/dhcp/lease_cmds/tests/binding_variables_unittest.cc @@ -57,10 +57,10 @@ TEST(BindingVariableTest, validConstructor) { std::list scenarios = { { - __LINE__, "my-var", valid_v4_exp, BindingVariable::QUERY ,AF_INET + __LINE__, "my-var", valid_v4_exp, BindingVariable::QUERY, AF_INET }, { - __LINE__, "my-var", valid_v4_exp, BindingVariable::RESPONSE ,AF_INET + __LINE__, "my-var", valid_v4_exp, BindingVariable::RESPONSE, AF_INET }, { __LINE__, "my-var", valid_v6_exp, BindingVariable::QUERY, AF_INET6 @@ -374,8 +374,7 @@ TEST(BindingVariableCacheTest, basics) { EXPECT_GT(cache->getLastFlushTime(), ref_time); } -/// @brief Verifies cache behavior for handling duplicate -/// entries. +/// @brief Verifies cache behavior for handling duplicate entries. TEST(BindingVariableCacheTest, duplicateEntries) { // Create a new cache. BindingVariableCachePtr cache(new BindingVariableCache()); @@ -533,7 +532,7 @@ TEST(BindingVariableMgrTest, clearOnConfigure) { ASSERT_TRUE(cache->getByName("four")); } -/// @brief Tests BindingVariableMgr::configure() with valid +/// @brief Tests BindingVariableMgr::configure() with invalid /// configuration scenarios. TEST(BindingVariableMgrTest, invalidConfigure) { struct Scenario { @@ -687,7 +686,7 @@ TEST(BindingVariableMgrTest, evaluateVariables4) { ASSERT_NO_THROW_LOG(changed = mgr->evaluateVariables(query, response, lease)); // If the original and expected lease contexts the same the lease should - // not have been changed, otherwse its context should have been updated. + // not have been changed, otherwise its context should have been updated. if (*orig_context == *exp_context) { ASSERT_FALSE(changed); ASSERT_EQ(*lease->getContext(), *orig_context); @@ -809,7 +808,7 @@ TEST(BindingVariableMgrTest, evaluateVariables6) { ASSERT_NO_THROW_LOG(changed = mgr->evaluateVariables(query, response, lease)); // If the original and expected lease contexts the same the lease should - // not have been changed, otherwse its context should have been updated. + // not have been changed, otherwise its context should have been updated. if (*orig_context == *exp_context) { ASSERT_FALSE(changed); ASSERT_EQ(*lease->getContext(), *orig_context); @@ -820,881 +819,4 @@ TEST(BindingVariableMgrTest, evaluateVariables6) { } } -#if 0 -class BindingVariableHandlerTest : public ::testing::Test { -public: - - /// @brief Pointer to the lease manager - isc::dhcp::LeaseMgr* lmptr_; - - /// @brief Constructor - BindingVariableHandlerTest() { - isc::dhcp::LeaseMgrFactory::destroy(); - lmptr_ = 0; - } - - /// @brief Destructor - /// - virtual ~BindingVariableHandlerTest() { - isc::dhcp::LeaseMgrFactory::destroy(); - lmptr_ = 0; - } - - /// @brief Creates an IPv4 lease - /// - /// Lease parameters: valid lifetime = 0xFFFFFFFE, cltt = 1923222072, fqdn-fwd = false, - /// fqdn-rev = true, hostname = myhost.example.com - /// - /// @param ip_address IP address for the lease. - /// @param subnet_id subnet identifier - /// @param hw_address_pattern value to be used for generating HW address by repeating - /// it 6 times. - /// @param client_id_pattern value to be used for generating client identifier by - /// repeating it 8 times. - /// @param declined controls whether the lease should be in declined state. - /// - /// @return Returns the lease created - isc::dhcp::Lease4Ptr createLease4(const std::string& ip_address, - const isc::dhcp::SubnetID& subnet_id, - const uint8_t hw_address_pattern, - const uint8_t client_id_pattern, - uint32_t pool_id = 0) { - isc::dhcp::Lease4Ptr lease(new isc::dhcp::Lease4()); - - lease->addr_ = isc::asiolink::IOAddress(ip_address); - - lease->hwaddr_.reset(new isc::dhcp::HWAddr(std::vector(6, hw_address_pattern), isc::dhcp::HTYPE_ETHER)); - lease->client_id_ = isc::dhcp::ClientIdPtr(new isc::dhcp::ClientId(std::vector(8, client_id_pattern))); - lease->valid_lft_ = 2400; - lease->updateCurrentExpirationTime(); - lease->subnet_id_ = subnet_id; - lease->pool_id_ = pool_id; - return (lease); - } - - /// @brief Creates an IPv6 lease - /// - /// @param ip_address IP address for the lease. - /// @param subnet_id subnet identifier - /// @param duid_address_pattern value to be used for generating DUID by - /// repeating it 8 times - /// @param declined controls whether the lease should be in declined state. - /// - /// @return Returns the lease created - isc::dhcp::Lease6Ptr createLease6(const std::string& ip_address, - const isc::dhcp::SubnetID& subnet_id, - const uint8_t duid_pattern, - uint32_t pool_id = 0) { - isc::dhcp::Lease6Ptr lease(new isc::dhcp::Lease6()); - - lease->addr_ = isc::asiolink::IOAddress(ip_address); - lease->type_ = isc::dhcp::Lease::TYPE_NA; - lease->prefixlen_ = 128; - lease->iaid_ = 42; - lease->duid_ = isc::dhcp::DuidPtr(new isc::dhcp::DUID(std::vector(8, duid_pattern))); - lease->preferred_lft_ = 1800; - lease->valid_lft_ = 2400; - lease->subnet_id_ = subnet_id; - lease->pool_id_ = pool_id; - - return (lease); - } - - /// @brief Initializes lease manager (and optionally populates it with a lease) - /// - /// Creates a lease manager (memfile, trimmed down to keep everything in memory - /// only) and optionally can create a lease, which is useful for leaseX-get and - /// leaseX-del type of tests. For lease details, see @ref createLease4 and - /// @ref createLease6. - /// - /// @param famiily protocol family AF_INET or AF_INET6 - /// @param insert_lease governs whether a lease should be pre-inserted - /// @param declined governs whether a lease should be in declined state - void initLeaseMgr(int family) { - isc::dhcp::LeaseMgrFactory::destroy(); - if (family == AF_INET6) { - isc::dhcp::LeaseMgrFactory::create("type=memfile persist=false " - "universe=6 extended-info-tables=true"); - lmptr_ = &(isc::dhcp::LeaseMgrFactory::instance()); - ASSERT_TRUE(lmptr_); - lmptr_->addLease(createLease6("2001:db8:1::1", 66, 0x42)); - lmptr_->addLease(createLease6("2001:db8:1::2", 66, 0x56)); - lmptr_->addLease(createLease6("2001:db8:2::1", 99, 0x42)); - lmptr_->addLease(createLease6("2001:db8:2::2", 99, 0x56)); - } else { - isc::dhcp::LeaseMgrFactory::create("type=memfile persist=false universe=4"); - lmptr_ = &(isc::dhcp::LeaseMgrFactory::instance()); - ASSERT_TRUE(lmptr_); - lmptr_->addLease(createLease4("192.0.2.1", 44, 0x08, 0x42)); - lmptr_->addLease(createLease4("192.0.2.2", 44, 0x09, 0x56)); - lmptr_->addLease(createLease4("192.0.3.1", 88, 0x08, 0x42)); - lmptr_->addLease(createLease4("192.0.3.2", 88, 0x09, 0x56)); - } - } - -#if 0 - /// @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(); -#endif - - /// @brief Check that leases6_committed handler works as expected with - /// valid inputs. - void testValidLeases6Committed(); - - /// @brief Check that leases6_committed handler does not throw or alter - /// leases under NOP conditions: - /// 1. There is no repsonse packet - /// 2. There are no leases - /// 3. There are leases but none active - void testNopLeases6Committed(); - - void testLeases6CommittedErrors(); -}; - -#if 0 -void -BindingVariableHandlerTest::testValidLease4Offer() { - initLeaseMgr(AF_INET); - - struct Scenario { - uint32_t line_; - std::string config_; - std::string orig_context_; - std::string exp_context_; - uint32_t offer_lifetime_; - }; - - std::list scenarios = { - { - // No variables configured, nothing in lease context. - __LINE__, - R"({})", - R"({})", - R"({})", - 500 - }, - { - // 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" - } - }})", - 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 - // 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" - } - }})", - 500 - }, - { - // 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": "" - } - }})", - 500 - }}; - - // Create packet pair and lease. - Pkt4Ptr query(new Pkt4(DHCPDISCOVER, 1234)); - query->setHWAddr(1, 6, { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 }); - - Pkt4Ptr response(new Pkt4(DHCPOFFER, 1234)); - 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 -BindingVariableHandlerTest::testValidLeases4Committed() { - initLeaseMgr(AF_INET); - - 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_); - - // 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); - - // Invoke the leases4Committed handler. - LeaseCmds cmds; - ASSERT_NO_THROW_LOG(cmds.leases4Committed(*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 -BindingVariableHandlerTest::testNopLeases4Committed() { - initLeaseMgr(AF_INET); - - 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()); - } -} -#endif - -void -BindingVariableHandlerTest::testValidLeases6Committed() { - initLeaseMgr(AF_INET6); - - 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": "duid", - "expression": "hexstring(option[1].hex,':')", - "source": "query" - }, - { - "name": "sub-id", - "expression": "hexstring(option[38].hex, ':')", - "source": "response" - }]})^", - R"({})", - R"({"ISC":{ - "binding-variables":{ - "duid": "01:02:03:04", - "sub-id": "05:06:07:08" - } - }})", - }, - { - // Lease context has binding-variables, none configured. - // Current logic leaves lease untouched. - __LINE__, - R"({})", - R"({"ISC":{ - "binding-variables":{ - "duid": "01:02:03:04", - "sub-id": "05:06:07:08" - } - }})", - R"({"ISC":{ - "binding-variables":{ - "duid": "01:02:03:04", - "sub-id": "05:06:07:08" - } - }})", - }, - { - // Evaluated variable value is an empty string. - __LINE__, - R"^({"binding-variables":[ - { - "name": "my-var", - "expression": "''", - "source": "query" - }]})^", - R"({"ISC":{ - "binding-variables":{ - "my-var": "pre-existing value" - } - }})", - R"({"ISC":{ - "binding-variables":{ - "my-var": "" - } - }})", - } - }; - - // Create packet pair and lease. - Pkt6Ptr query(new Pkt6(DHCPV6_REQUEST, 1234)); - OptionPtr client_id(new Option(Option::V6, D6O_CLIENTID, - { 0x01, 0x02, 0x03, 0x04 })); - query->addOption(client_id); - - Pkt6Ptr response(new Pkt6(DHCPV6_REPLY, 7890)); - OptionPtr subscriber_id(new Option(Option::V6, D6O_SUBSCRIBER_ID, - { 0x05, 0x06, 0x07, 0x08 })); - response->addOption(subscriber_id); - - // Create a list of the lease addresses. - std::list lease_addrs; - lease_addrs.push_back(IOAddress("2001:db8:1::1")); - lease_addrs.push_back(IOAddress("2001:db8:1::2")); - lease_addrs.push_back(IOAddress("2001:db8:2::2")); - - // 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_INET6))); - 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. - Lease6CollectionPtr orig_leases(new Lease6Collection()); - for (auto const& address : lease_addrs) { - Lease6Ptr orig_lease = lmptr_->getLease6(Lease::TYPE_NA, address); - ASSERT_TRUE(orig_lease); - ASSERT_TRUE(orig_lease->valid_lft_ > 0); - - ConstElementPtr orig_context; - ASSERT_NO_THROW_LOG(orig_context = Element::fromJSON(scenario.orig_context_)); - orig_lease->setContext(orig_context); - ASSERT_NO_THROW_LOG(lmptr_->updateLease6(orig_lease)); - orig_leases->push_back(orig_lease); - } - - // Create a callout handle and add the expected arguments. - CalloutHandlePtr callout_handle = HooksManager::createCalloutHandle(); - callout_handle->setArgument("query6", query); - callout_handle->setArgument("response6", response); - callout_handle->setArgument("leases6", orig_leases); - - // Invoke the leases6Committed handler. - LeaseCmds cmds; - ASSERT_NO_THROW_LOG(cmds.leases6Committed(*callout_handle, mgr)); - - // Iterate over the leases and make sure the user-contexts are as expected. - for (auto const& lease : *orig_leases) { - // Fetch the lease. - Lease6Ptr after_lease = lmptr_->getLease6(Lease::TYPE_NA, lease->addr_); - 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 -BindingVariableHandlerTest::testNopLeases6Committed() { - initLeaseMgr(AF_INET6); - - struct Scenario { - uint32_t line_; - DHCPv6MessageType response_type_; - bool send_lease_; - uint32_t valid_lft_; - }; - - // Configure a single variable. - std::string config = - R"^({"binding-variables":[ - { - "name": "duid", - "expression": "hexstring(option[1].hex,':')", - "source": "query" - }]})^"; - - // Create and configure the manager. - BindingVariableMgrPtr mgr; - ASSERT_NO_THROW_LOG(mgr.reset(new BindingVariableMgr(AF_INET6))); - ASSERT_NO_THROW_LOG(mgr->configure(Element::fromJSON(config))); - - // Scenarios should all result in no change to the lease. - std::list scenarios = { - { - // No leases. - __LINE__, - DHCPV6_REPLY, - false, - 0 - }, - { - // No active leases. - __LINE__, - DHCPV6_REPLY, - true, - 0 - }, - { - // No response. - __LINE__, - DHCPV6_NOTYPE, - true, - 1000 - }}; - - // Create query packet. - Pkt6Ptr query(new Pkt6(DHCPV6_REQUEST, 1234)); - OptionPtr client_id(new Option(Option::V6, D6O_CLIENTID, - { 0x01, 0x02, 0x03, 0x04 })); - query->addOption(client_id); - - IOAddress na_addr("2001:db8:1::1"); - for (auto const& scenario : scenarios) { - SCOPED_LINE(scenario.line_); - - // Create the response packet, if one. - Pkt6Ptr response; - if (scenario.response_type_ != DHCPV6_NOTYPE) { - response.reset(new Pkt6(scenario.response_type_, 1234)); - } - - - // Fetch the lease and verify there is no context content. - Lease6Ptr orig_lease = lmptr_->getLease6(Lease::TYPE_NA, na_addr); - ASSERT_TRUE(orig_lease); - ASSERT_FALSE(orig_lease->getContext()); - orig_lease->valid_lft_ = scenario.valid_lft_; - - Lease6CollectionPtr leases(new Lease6Collection()); - 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("query6", query); - callout_handle->setArgument("response6", response); - callout_handle->setArgument("leases6", leases); - - // Invoke the leases6Committed handler. - LeaseCmds cmds; - ASSERT_NO_THROW_LOG(cmds.leases6Committed(*callout_handle, mgr)); - - // Fetch the lease. Context should still be empty. - Lease6Ptr after_lease = lmptr_->getLease6(Lease::TYPE_NA, na_addr); - ASSERT_TRUE(after_lease); - ASSERT_FALSE(after_lease->getContext()); - } -} - -void -BindingVariableHandlerTest::testLeases6CommittedErrors() { - initLeaseMgr(AF_INET6); - - // Create a config with two binding variables. - std::string config_str = - R"^({"binding-variables":[ - { - "name": "duid", - "expression": "hexstring(option[1].hex,':')", - "source": "query" - }, - { - "name": "sub-id", - "expression": "hexstring(option[38].hex, ':')", - "source": "response" - }]})^"; - - ConstElementPtr config; - ASSERT_NO_THROW_LOG(config = Element::fromJSON(config_str)); - - // Create the expected context contents. - std::string exp_context_str = - R"({"ISC":{ - "binding-variables":{ - "duid": "01:02:03:04", - "sub-id": "05:06:07:08" - } - }})"; - - - ConstElementPtr exp_context; - ASSERT_NO_THROW_LOG(exp_context = Element::fromJSON(exp_context_str)); - - // Create packet pair and lease. - Pkt6Ptr query(new Pkt6(DHCPV6_REQUEST, 1234)); - OptionPtr client_id(new Option(Option::V6, D6O_CLIENTID, - { 0x01, 0x02, 0x03, 0x04 })); - query->addOption(client_id); - - Pkt6Ptr response(new Pkt6(DHCPV6_REPLY, 7890)); - OptionPtr subscriber_id(new Option(Option::V6, D6O_SUBSCRIBER_ID, - { 0x05, 0x06, 0x07, 0x08 })); - response->addOption(subscriber_id); - - // Create and configure the manager. - BindingVariableMgrPtr mgr; - ASSERT_NO_THROW_LOG(mgr.reset(new BindingVariableMgr(AF_INET6))); - ASSERT_NO_THROW_LOG(mgr->configure(config)); - - // Fetch the leases. - std::list lease_addrs; - lease_addrs.push_back(IOAddress("2001:db8:1::1")); - lease_addrs.push_back(IOAddress("2001:db8:1::2")); - lease_addrs.push_back(IOAddress("2001:db8:2::2")); - - Lease6CollectionPtr orig_leases(new Lease6Collection()); - for (auto const& address : lease_addrs) { - Lease6Ptr orig_lease = lmptr_->getLease6(Lease::TYPE_NA, address); - ASSERT_TRUE(orig_lease); - orig_leases->push_back(orig_lease); - } - - // Delete the middle lease from the back end. This should cause a conflict error. - ASSERT_NO_THROW_LOG(lmptr_->deleteLease((*orig_leases)[1])); - - // Create a callout handle and add the expected arguments. - CalloutHandlePtr callout_handle = HooksManager::createCalloutHandle(); - callout_handle->setArgument("query6", query); - callout_handle->setArgument("response6", response); - callout_handle->setArgument("leases6", orig_leases); - - // Invoke the leases6Committed handler. - LeaseCmds cmds; - ASSERT_THROW_MSG(cmds.leases6Committed(*callout_handle, mgr), - Unexpected, - "1 out of 3 leases failed to update for " - "duid=[01:02:03:04], [no hwaddr info], tid=0x4d2"); - - for (auto const& lease : *orig_leases) { - // Fetch the lease. - Lease6Ptr after_lease = lmptr_->getLease6(Lease::TYPE_NA, lease->addr_); - if (after_lease) { - // Context contents should match the expected context content. - ASSERT_EQ(*(after_lease->getContext()), *exp_context); - } else { - // Middle lease should not be found. - EXPECT_EQ(lease->addr_, (*orig_leases)[1]->addr_); - } - } -} - -TEST_F(BindingVariableHandlerTest, validLeases6Committed) { - testValidLeases6Committed(); -} - -TEST_F(BindingVariableHandlerTest, validLeases6CommittedMultiThreading) { - MultiThreadingTest mt(true); - testValidLeases6Committed(); -} - -TEST_F(BindingVariableHandlerTest, nopLeases6Committed) { - testNopLeases6Committed(); -} - -TEST_F(BindingVariableHandlerTest, nopLeases6CommittedMultiThreading) { - MultiThreadingTest mt(true); - testNopLeases6Committed(); -} - -TEST_F(BindingVariableHandlerTest, leases6CommittedErrors) { - testLeases6CommittedErrors(); -} - -TEST_F(BindingVariableHandlerTest, leases6CommittedErrorsMultiThreading) { - MultiThreadingTest mt(true); - testLeases6CommittedErrors(); -} -#endif - } // end of anonymous namespace diff --git a/src/hooks/dhcp/lease_cmds/tests/lease_cmds_func4_unittest.cc b/src/hooks/dhcp/lease_cmds/tests/lease_cmds_func4_unittest.cc index 1aa3488448..e0666502bc 100644 --- a/src/hooks/dhcp/lease_cmds/tests/lease_cmds_func4_unittest.cc +++ b/src/hooks/dhcp/lease_cmds/tests/lease_cmds_func4_unittest.cc @@ -32,20 +32,18 @@ using namespace isc::lease_cmds; namespace { -class LeaseCmdsFuncTest4 : public LeaseCmdsFuncTest { +/// @brief DHCPv4 Test fixture for testing lease commands hook library +/// functions and class methods. +class LeaseCmdsFuncTest4 : public LeaseCmdsFuncTest { public: /// @brief Constructor LeaseCmdsFuncTest4() = default; /// @brief Destructor - /// virtual ~LeaseCmdsFuncTest4() = default; /// @brief Creates an IPv4 lease /// - /// Lease parameters: valid lifetime = 0xFFFFFFFE, cltt = 1923222072, fqdn-fwd = false, - /// fqdn-rev = true, hostname = myhost.example.com - /// /// @param ip_address IP address for the lease. /// @param subnet_id subnet identifier /// @param hw_address_pattern value to be used for generating HW address by repeating @@ -71,13 +69,7 @@ public: return (lease); } - /// @brief Initializes lease manager (and optionally populates it with a lease) - /// - /// Creates a lease manager and initial leases. - /// only) and optionally can create a lease, which is useful for leaseX-get and - /// leaseX-del type of tests. For lease details, see @ref createLease and - /// @ref createLease6. - /// + /// @brief Initializes the lease manager and populates it with test leases. virtual void initLeaseMgr() { LeaseMgrFactory::destroy(); LeaseMgrFactory::create("type=memfile persist=false universe=4"); @@ -99,9 +91,9 @@ public: 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 + /// the lease under NOP conditions: + /// 1. There is no response packet + /// 2. Response packet is a DHCPACK /// 3. There is no active lease void testNopLeases4Committed(); }; @@ -216,7 +208,7 @@ LeaseCmdsFuncTest4::testValidLease4Offer() { IOAddress yiaddr("192.0.2.1"); response->setYiaddr(yiaddr); - // Iterater over scenarios. + // Iterates over scenarios. for (auto const& scenario : scenarios) { SCOPED_LINE(scenario.line_); @@ -245,7 +237,7 @@ LeaseCmdsFuncTest4::testValidLease4Offer() { callout_handle->setArgument("leases4", leases); callout_handle->setArgument("offer_lifetime", scenario.offer_lifetime_); - // Invoke the leases4Committed handler. + // Invoke the lease4Offer handler. LeaseCmds cmds; ASSERT_NO_THROW_LOG(cmds.lease4Offer(*callout_handle, mgr)); @@ -346,7 +338,7 @@ LeaseCmdsFuncTest4::testValidLeases4Committed() { IOAddress yiaddr("192.0.2.1"); response->setYiaddr(yiaddr); - // Iterater over scenarios. + // Iterates over scenarios. for (auto const& scenario : scenarios) { SCOPED_LINE(scenario.line_); @@ -399,12 +391,12 @@ LeaseCmdsFuncTest4::testNopLeases4Committed() { // Configure a single variable. std::string config = - R"^({"binding-variables":[ - { - "name": "hwaddr", - "expression": "hexstring(pkt4.mac,':')", - "source": "query" - }]})^"; + R"^({"binding-variables":[ + { + "name": "hwaddr", + "expression": "hexstring(pkt4.mac,':')", + "source": "query" + }]})^"; // Create and configure the manager. BindingVariableMgrPtr mgr; diff --git a/src/hooks/dhcp/lease_cmds/tests/lease_cmds_func6_unittest.cc b/src/hooks/dhcp/lease_cmds/tests/lease_cmds_func6_unittest.cc index 8f7341b22c..e18085ea6e 100644 --- a/src/hooks/dhcp/lease_cmds/tests/lease_cmds_func6_unittest.cc +++ b/src/hooks/dhcp/lease_cmds/tests/lease_cmds_func6_unittest.cc @@ -33,7 +33,9 @@ using namespace isc::lease_cmds; namespace { -class LeaseCmdsFuncTest6 : public LeaseCmdsFuncTest { +/// @brief DHCPv6 Test fixture for testing lease commands hook library +/// functions and class methods. +class LeaseCmdsFuncTest6 : public LeaseCmdsFuncTest { public: /// @brief Constructor LeaseCmdsFuncTest6() = default; @@ -41,10 +43,10 @@ public: /// @brief Destructor virtual ~LeaseCmdsFuncTest6() = default; - /// @brief Initializes lease manager (and optionally populates it with a lease) + /// @brief Initializes the lease manager and populates it with test leases. void initLeaseMgr() { LeaseMgrFactory::destroy(); - LeaseMgrFactory::create("type=memfile persist=false " + LeaseMgrFactory::create("type=memfile persist=false " "universe=6 extended-info-tables=true"); lmptr_ = &(isc::dhcp::LeaseMgrFactory::instance()); ASSERT_TRUE(lmptr_); @@ -82,7 +84,7 @@ public: /// @brief Check that leases6_committed handler does not throw or alter /// leases under NOP conditions: - /// 1. There is no repsonse packet + /// 1. There is no response packet /// 2. There are no leases /// 3. There are leases but none active void testNopLeases6Committed(); @@ -188,7 +190,7 @@ LeaseCmdsFuncTest6::testValidLeases6Committed() { lease_addrs.push_back(IOAddress("2001:db8:1::2")); lease_addrs.push_back(IOAddress("2001:db8:2::2")); - // Iterater over scenarios. + // Iterates over scenarios. for (auto const& scenario : scenarios) { SCOPED_LINE(scenario.line_); diff --git a/src/hooks/dhcp/lease_cmds/tests/lease_cmds_func_unittest.h b/src/hooks/dhcp/lease_cmds/tests/lease_cmds_func_unittest.h index 30a2f752ed..4aac8f93ac 100644 --- a/src/hooks/dhcp/lease_cmds/tests/lease_cmds_func_unittest.h +++ b/src/hooks/dhcp/lease_cmds/tests/lease_cmds_func_unittest.h @@ -12,12 +12,17 @@ namespace { +/// @brief Macro around SCOPED_TRACE() for emitting a message including +/// a source line number. +/// @param line line number to emit #define SCOPED_LINE(line) \ std::stringstream ss; \ ss << "Scenario at line: " << line; \ SCOPED_TRACE(ss.str()); -class LeaseCmdsFuncTest : public ::testing::Test { +/// @brief Text fixture for testing lease command functions and +/// class methods. Provides an intialized lease manager. +class LeaseCmdsFuncTest : public ::testing::Test { public: /// @brief Constructor LeaseCmdsFuncTest() { @@ -25,12 +30,12 @@ public: lmptr_ = 0; } + /// @brief Initializes the lease manager. virtual void SetUp() { initLeaseMgr(); } /// @brief Destructor - /// virtual ~LeaseCmdsFuncTest() { isc::dhcp::LeaseMgrFactory::destroy(); lmptr_ = 0; diff --git a/src/lib/config/unix_command_mgr.h b/src/lib/config/unix_command_mgr.h index 97b5f3681d..e0401ca432 100644 --- a/src/lib/config/unix_command_mgr.h +++ b/src/lib/config/unix_command_mgr.h @@ -87,7 +87,7 @@ public: /// @note This function in used internally by @ref closeCommandSockets and it /// should not be used directly, except for unit tests. /// - /// @param config Configuration information for the unix control socket. + /// @param info Configuration information for the unix control socket. void closeCommandSocket(UnixSocketInfoPtr info = UnixSocketInfoPtr()); /// @brief Shuts down any open unix control sockets. @@ -97,7 +97,7 @@ public: /// /// This method should be used only in tests. /// - /// @param config Configuration information for the unix control socket. + /// @param info Configuration information for the unix control socket. /// /// @return The file descriptor of the specified unix control socket. int getControlSocketFD(UnixSocketInfoPtr info = UnixSocketInfoPtr()); diff --git a/src/lib/dhcpsrv/lease.h b/src/lib/dhcpsrv/lease.h index 9d5b25bbe0..85b82c70eb 100644 --- a/src/lib/dhcpsrv/lease.h +++ b/src/lib/dhcpsrv/lease.h @@ -277,7 +277,7 @@ struct Lease : public isc::data::UserContext, public isc::data::CfgToElement { /// Update the ISC entry in the lease's user-context /// /// Adds or updates the named element within the "ISC" map - /// with the lease's "user-context". The update occurs only if + /// with the lease's user context. The update occurs only if /// the new value(s) are different than the existing values for the /// element. /// -- 2.47.3