From: Marcin Siodelski Date: Mon, 24 Jun 2019 13:56:18 +0000 (+0200) Subject: [#689] Include control result and error message in resp to lease6-bulk-apply X-Git-Tag: Kea-1.6.0-beta2~221 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ebd975ba72492ee33e5804e3add09878bc976841;p=thirdparty%2Fkea.git [#689] Include control result and error message in resp to lease6-bulk-apply --- diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds.cc b/src/hooks/dhcp/lease_cmds/lease_cmds.cc index c8e3973fc2..fa7c23d279 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds.cc +++ b/src/hooks/dhcp/lease_cmds/lease_cmds.cc @@ -273,10 +273,15 @@ public: /// @param lease_type lease type. /// @param lease_address lease address. /// @param duid DUID of the client. + /// @param control_result Control result: "empty" of the lease was + /// not found, "error" otherwise. + /// @param error_message Error message. ElementPtr createFailedLeaseMap(const SubnetID& subnet_id, const Lease::Type& lease_type, const IOAddress& lease_address, - const DuidPtr&duid) const; + const DuidPtr& duid, + const int control_result, + const std::string& error_message) const; }; int @@ -907,22 +912,31 @@ LeaseCmdsImpl::lease6BulkApplyHandler(CalloutHandle& handle) { ++success_count; } else { + // Lazy creation of the list of leases which failed to delete. + if (!failed_deleted_list) { + failed_deleted_list = Element::createList(); + } + // If the lease doesn't exist we also want to put it // on the list of leases which failed to delete. That // corresponds to the lease6-del command which returns // an error when the lease doesn't exist. - isc_throw(InvalidOperation, "no such lease for address " - << lease_addr.toText()); + failed_deleted_list->add(createFailedLeaseMap(p.subnet_id, p.lease_type, + p.addr, p.duid, + CONTROL_RESULT_EMPTY, + "lease not found")); } } - } catch (...) { + } catch (const std::exception& ex) { // Lazy creation of the list of leases which failed to delete. if (!failed_deleted_list) { failed_deleted_list = Element::createList(); } failed_deleted_list->add(createFailedLeaseMap(p.subnet_id, p.lease_type, - p.addr, p.duid)); + p.addr, p.duid, + CONTROL_RESULT_ERROR, + ex.what())); } } } @@ -949,7 +963,7 @@ LeaseCmdsImpl::lease6BulkApplyHandler(CalloutHandle& handle) { ++success_count; - } catch (...) { + } catch (const std::exception& ex) { // Lazy creation of the list of leases which failed to add/update. if (!failed_leases_list) { failed_leases_list = Element::createList(); @@ -957,7 +971,9 @@ LeaseCmdsImpl::lease6BulkApplyHandler(CalloutHandle& handle) { failed_leases_list->add(createFailedLeaseMap(lease->subnet_id_, lease->type_, lease->addr_, - lease->duid_)); + lease->duid_, + CONTROL_RESULT_ERROR, + ex.what())); } } } @@ -1266,7 +1282,9 @@ ElementPtr LeaseCmdsImpl::createFailedLeaseMap(const SubnetID& subnet_id, const Lease::Type& lease_type, const IOAddress& lease_address, - const DuidPtr&duid) const { + const DuidPtr& duid, + const int control_result, + const std::string& error_message) const { auto failed_lease_map = Element::createMap(); failed_lease_map->set("subnet-id", Element::create(static_cast(subnet_id))); @@ -1279,6 +1297,10 @@ LeaseCmdsImpl::createFailedLeaseMap(const SubnetID& subnet_id, failed_lease_map->set("duid", Element::create(duid->toText())); } + // Associate the result with the lease. + failed_lease_map->set("result", Element::create(control_result)); + failed_lease_map->set("error-message", Element::create(error_message)); + return (failed_lease_map); } diff --git a/src/hooks/dhcp/lease_cmds/tests/lease_cmds_unittest.cc b/src/hooks/dhcp/lease_cmds/tests/lease_cmds_unittest.cc index fd9e489ce2..3c7a977c10 100644 --- a/src/hooks/dhcp/lease_cmds/tests/lease_cmds_unittest.cc +++ b/src/hooks/dhcp/lease_cmds/tests/lease_cmds_unittest.cc @@ -494,6 +494,64 @@ public: static_cast(l->get("valid-lft")->intValue())); EXPECT_EQ(DEC_2030_TIME, l->get("cltt")->intValue()); } + + /// @brief This function checks that the JSON list contains an entry + /// indicating lease deletion, creation or update failure. + /// + /// @param failed_leases_list JSON list containing list of leases. + /// @param expected_type Expected lease type as text. + /// @param expected_ip_address Expected IP address. + /// @oaram expected_control_result Expected control result for the lease. + /// @param expected_error_msg Expected error message. Default is an empty + /// string which indicates that the error message should not be checked. + /// @param expected_subnet_id Expected subnet id. Default is -1 which means + /// that the subnet_id is not required and should not be checked. + void checkFailedLease(const ConstElementPtr& failed_leases_list, + const std::string& expected_type, + const std::string& expected_ip_address, + const int expected_control_result, + const std::string& expected_error_msg = "", + const SubnetID& expected_subnet_id = -1) { + ASSERT_TRUE(failed_leases_list); + + for (auto i = 0; i < failed_leases_list->size(); ++i) { + + auto failed_lease = failed_leases_list->get(i); + ASSERT_TRUE(failed_lease); + ASSERT_EQ(Element::map, failed_lease->getType()); + + auto ip_address = failed_lease->get("ip-address"); + ASSERT_TRUE(ip_address); + ASSERT_EQ(Element::string, ip_address->getType()); + + if (ip_address->stringValue() == expected_ip_address) { + + auto lease_type = failed_lease->get("type"); + ASSERT_TRUE(lease_type); + ASSERT_EQ(Element::string, lease_type->getType()); + + auto control_result = failed_lease->get("result"); + ASSERT_TRUE(control_result); + ASSERT_EQ(Element::integer, control_result->getType()); + + if (!expected_error_msg.empty()) { + auto error_msg = failed_lease->get("error-message"); + ASSERT_TRUE(error_msg); + ASSERT_EQ(Element::string, error_msg->getType()); + } + + if (expected_subnet_id > 0) { + auto subnet_id = failed_lease->get("subnet-id"); + ASSERT_TRUE(subnet_id); + ASSERT_EQ(Element::integer, subnet_id->getType()); + } + + return; + } + } + + ADD_FAILURE() << "expected lease not found"; + } }; // Simple test that checks the library really registers the commands. @@ -4146,6 +4204,18 @@ TEST_F(LeaseCmdsTest, Lease6BulkApplyDeleteNonExiting) { ASSERT_TRUE(failed_deleted_leases); ASSERT_EQ(Element::list, failed_deleted_leases->getType()); ASSERT_EQ(2, failed_deleted_leases->size()); + + { + SCOPED_TRACE("lease address 2001:db8:1::123"); + checkFailedLease(failed_deleted_leases, "IA_NA", "2001:db8:1::123", + CONTROL_RESULT_EMPTY, "lease not found"); + } + + { + SCOPED_TRACE("lease address 2001:db8:1::234"); + checkFailedLease(failed_deleted_leases, "IA_NA", "2001:db8:1::234", + CONTROL_RESULT_EMPTY, "lease not found"); + } } // Check that changes for other leases are not applied if one of