From f7a9b5d0aa1b3566bd3d0b3b2539a0953d6e3d32 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Sun, 21 Jun 2020 12:48:08 +0200 Subject: [PATCH] [#1247] Updated databaseConfigApply --- .../database/tests/audit_entry_unittest.cc | 2 +- src/lib/dhcpsrv/cb_ctl_dhcp4.cc | 39 +++- src/lib/dhcpsrv/cb_ctl_dhcp6.cc | 39 +++- src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc | 167 ++++++++++++------ src/lib/process/cb_ctl_base.h | 60 ++++--- .../process/tests/cb_ctl_base_unittests.cc | 53 ++++-- 6 files changed, 252 insertions(+), 108 deletions(-) diff --git a/src/lib/database/tests/audit_entry_unittest.cc b/src/lib/database/tests/audit_entry_unittest.cc index 8a61a2dd76..dd62458f67 100644 --- a/src/lib/database/tests/audit_entry_unittest.cc +++ b/src/lib/database/tests/audit_entry_unittest.cc @@ -340,7 +340,7 @@ TEST_F(AuditEntryCollectionTest, getByObjectId) { const auto& object_id_idx = audit_entries_.get(); // Search for object id 10. - auto range = object_id_idx.equal_range(10); + auto range = object_id_idx.equal_range(10); ASSERT_EQ(1, std::distance(range.first, range.second)); EXPECT_TRUE(includes("dhcp4_subnet", 10, range.first, range.second)); diff --git a/src/lib/dhcpsrv/cb_ctl_dhcp4.cc b/src/lib/dhcpsrv/cb_ctl_dhcp4.cc index 775610d0bf..a3384ad45d 100644 --- a/src/lib/dhcpsrv/cb_ctl_dhcp4.cc +++ b/src/lib/dhcpsrv/cb_ctl_dhcp4.cc @@ -141,7 +141,11 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector, SrvConfigPtr external_cfg = CfgMgr::instance().createExternalCfg(); // First let's fetch the globals and add them to external config. - if (!globals_fetched && fetchConfigElement(audit_entries, "dhcp4_global_parameter")) { + AuditEntryCollection updated_entries; + if (!globals_fetched && !audit_entries.empty()) { + updated_entries = fetchConfigElement(audit_entries, "dhcp4_global_parameter"); + } + if (!globals_fetched && (audit_entries.empty() || !updated_entries.empty())) { data::StampedValueCollection globals; globals = getMgr().getPool()->getModifiedGlobalParameters4(backend_selector, server_selector, lb_modification_time); @@ -149,31 +153,49 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector, } // Now we fetch the option definitions and add them. - if (fetchConfigElement(audit_entries, "dhcp4_option_def")) { + if (!audit_entries.empty()) { + updated_entries = fetchConfigElement(audit_entries, "dhcp4_option_def"); + } + if (audit_entries.empty() || !updated_entries.empty()) { OptionDefContainer option_defs = getMgr().getPool()->getModifiedOptionDefs4(backend_selector, server_selector, lb_modification_time); for (auto option_def = option_defs.begin(); option_def != option_defs.end(); ++option_def) { + if (!audit_entries.empty() && !hasObjectId(updated_entries, (*option_def)->getId())) { + continue; + } external_cfg->getCfgOptionDef()->add((*option_def), (*option_def)->getOptionSpaceName()); } } // Next fetch the options. They are returned as a container of OptionDescriptors. - if (fetchConfigElement(audit_entries, "dhcp4_options")) { + if (!audit_entries.empty()) { + updated_entries = fetchConfigElement(audit_entries, "dhcp4_options"); + } + if (audit_entries.empty() || !updated_entries.empty()) { OptionContainer options = getMgr().getPool()->getModifiedOptions4(backend_selector, server_selector, lb_modification_time); for (auto option = options.begin(); option != options.end(); ++option) { + if (!audit_entries.empty() && !hasObjectId(updated_entries, (*option).getId())) { + continue; + } external_cfg->getCfgOption()->add((*option), (*option).space_name_); } } // Now fetch the shared networks. - if (fetchConfigElement(audit_entries, "dhcp4_shared_network")) { + if (!audit_entries.empty()) { + updated_entries = fetchConfigElement(audit_entries, "dhcp4_shared_network"); + } + if (audit_entries.empty() || !updated_entries.empty()) { SharedNetwork4Collection networks = getMgr().getPool()->getModifiedSharedNetworks4(backend_selector, server_selector, lb_modification_time); for (auto network = networks.begin(); network != networks.end(); ++network) { + if (!audit_entries.empty() && !hasObjectId(updated_entries, (*network)->getId())) { + continue; + } // In order to take advantage of the dynamic inheritance of global // parameters to a shared network we need to set a callback function // for each network to allow for fetching global parameters. @@ -185,11 +207,17 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector, } // Next we fetch subnets. - if (fetchConfigElement(audit_entries, "dhcp4_subnet")) { + if (!audit_entries.empty()) { + updated_entries = fetchConfigElement(audit_entries, "dhcp4_subnet"); + } + if (audit_entries.empty() || !updated_entries.empty()) { Subnet4Collection subnets = getMgr().getPool()->getModifiedSubnets4(backend_selector, server_selector, lb_modification_time); for (auto subnet = subnets.begin(); subnet != subnets.end(); ++subnet) { + if (!audit_entries.empty() && !hasObjectId(updated_entries, (*subnet)->getID())) { + continue; + } // In order to take advantage of the dynamic inheritance of global // parameters to a subnet we need to set a callback function for each // subnet to allow for fetching global parameters. @@ -202,7 +230,6 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector, if (audit_entries.empty()) { CfgMgr::instance().mergeIntoStagingCfg(external_cfg->getSequence()); - } else { CfgMgr::instance().mergeIntoCurrentCfg(external_cfg->getSequence()); } diff --git a/src/lib/dhcpsrv/cb_ctl_dhcp6.cc b/src/lib/dhcpsrv/cb_ctl_dhcp6.cc index 6e42dc8111..bc5adf16d8 100644 --- a/src/lib/dhcpsrv/cb_ctl_dhcp6.cc +++ b/src/lib/dhcpsrv/cb_ctl_dhcp6.cc @@ -140,7 +140,11 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector SrvConfigPtr external_cfg = CfgMgr::instance().createExternalCfg(); // First let's fetch the globals and add them to external config. - if (!globals_fetched && fetchConfigElement(audit_entries, "dhcp6_global_parameter")) { + AuditEntryCollection updated_entries; + if (!globals_fetched && !audit_entries.empty()) { + updated_entries = fetchConfigElement(audit_entries, "dhcp6_global_parameter"); + } + if (!globals_fetched && (audit_entries.empty() || !updated_entries.empty())) { data::StampedValueCollection globals; globals = getMgr().getPool()->getModifiedGlobalParameters6(backend_selector, server_selector, lb_modification_time); @@ -148,31 +152,49 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector } // Now we fetch the option definitions and add them. - if (fetchConfigElement(audit_entries, "dhcp6_option_def")) { + if (!audit_entries.empty()) { + updated_entries = fetchConfigElement(audit_entries, "dhcp6_option_def"); + } + if (audit_entries.empty() || !updated_entries.empty()) { OptionDefContainer option_defs = getMgr().getPool()->getModifiedOptionDefs6(backend_selector, server_selector, lb_modification_time); for (auto option_def = option_defs.begin(); option_def != option_defs.end(); ++option_def) { + if (!audit_entries.empty() && !hasObjectId(updated_entries, (*option_def)->getId())) { + continue; + } external_cfg->getCfgOptionDef()->add((*option_def), (*option_def)->getOptionSpaceName()); } } // Next fetch the options. They are returned as a container of OptionDescriptors. - if (fetchConfigElement(audit_entries, "dhcp6_options")) { + if (!audit_entries.empty()) { + updated_entries = fetchConfigElement(audit_entries, "dhcp6_options"); + } + if (audit_entries.empty() || !updated_entries.empty()) { OptionContainer options = getMgr().getPool()->getModifiedOptions6(backend_selector, server_selector, lb_modification_time); for (auto option = options.begin(); option != options.end(); ++option) { + if (!audit_entries.empty() && !hasObjectId(updated_entries, (*option).getId())) { + continue; + } external_cfg->getCfgOption()->add((*option), (*option).space_name_); } } // Now fetch the shared networks. - if (fetchConfigElement(audit_entries, "dhcp6_shared_network")) { + if (!audit_entries.empty()) { + updated_entries = fetchConfigElement(audit_entries, "dhcp6_shared_network"); + } + if (audit_entries.empty() || !updated_entries.empty()) { SharedNetwork6Collection networks = getMgr().getPool()->getModifiedSharedNetworks6(backend_selector, server_selector, lb_modification_time); for (auto network = networks.begin(); network != networks.end(); ++network) { + if (!audit_entries.empty() && !hasObjectId(updated_entries, (*network)->getId())) { + continue; + } // In order to take advantage of the dynamic inheritance of global // parameters to a shared network we need to set a callback function // for each network to allow for fetching global parameters. @@ -184,11 +206,17 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector } // Next we fetch subnets. - if (fetchConfigElement(audit_entries, "dhcp6_subnet")) { + if (!audit_entries.empty()) { + updated_entries = fetchConfigElement(audit_entries, "dhcp6_subnet"); + } + if (audit_entries.empty() || !updated_entries.empty()) { Subnet6Collection subnets = getMgr().getPool()->getModifiedSubnets6(backend_selector, server_selector, lb_modification_time); for (auto subnet = subnets.begin(); subnet != subnets.end(); ++subnet) { + if (!audit_entries.empty() && !hasObjectId(updated_entries, (*subnet)->getID())) { + continue; + } // In order to take advantage of the dynamic inheritance of global // parameters to a subnet we need to set a callback function for each // subnet to allow for fetching global parameters. @@ -201,7 +229,6 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector if (audit_entries.empty()) { CfgMgr::instance().mergeIntoStagingCfg(external_cfg->getSequence()); - } else { CfgMgr::instance().mergeIntoCurrentCfg(external_cfg->getSequence()); } diff --git a/src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc b/src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc index bd94ce325c..8e2dd41936 100644 --- a/src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc +++ b/src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc @@ -40,7 +40,8 @@ public: /// @brief Constructor. CBControlDHCPTest() - : timestamp_(), object_timestamp_(), audit_entries_() { + : timestamp_(), object_timestamp_(), audit_entries_(), + modification_id_(2345) { CfgMgr::instance().clear(); initTimestamps(); callback_name_ = std::string(""); @@ -67,10 +68,14 @@ public: /// /// @param object_type Object type to be associated with the audit /// entry. - void addCreateAuditEntry(const std::string& object_type) { - AuditEntryPtr entry(new AuditEntry(object_type, 1234, + /// @param object_id Identifier of the object to be associated with + /// the audit entry. + void addCreateAuditEntry(const std::string& object_type, + const uint64_t object_id) { + AuditEntryPtr entry(new AuditEntry(object_type, object_id, AuditEntry::ModificationType::CREATE, - 2345, "some log message")); + ++modification_id_, + "some log message")); audit_entries_.insert(entry); } @@ -86,7 +91,8 @@ public: const uint64_t object_id) { AuditEntryPtr entry(new AuditEntry(object_type, object_id, AuditEntry::ModificationType::DELETE, - 1234, "some log message")); + ++modification_id_, + "some log message")); audit_entries_.insert(entry); } @@ -139,7 +145,7 @@ public: /// object types. /// /// @param object_type Object type. - bool fetchConfigElement(const std::string& object_type) const { + bool hasConfigElement(const std::string& object_type) const { if (!audit_entries_.empty()) { const auto& index = audit_entries_.get(); auto range = index.equal_range(object_type); @@ -203,6 +209,9 @@ public: /// @brief Collection of audit entries used in the unit tests. AuditEntryCollection audit_entries_; + /// @brief Modification id counter. + uint64_t modification_id_; + /// @brief Callback name. static std::string callback_name_; @@ -453,7 +462,7 @@ public: // If there is an audit entry for global parameter and the parameter // modification time is later than last audit entry time it should // be merged. - if (fetchConfigElement("dhcp4_global_parameter") && + if (hasConfigElement("dhcp4_global_parameter") && (getTimestamp("dhcp4_global_parameter") > lb_modification_time)) { checkConfiguredGlobal(srv_cfg, "foo", Element::create("bar")); @@ -466,7 +475,7 @@ public: // modification time is later than last audit entry time it should // be merged. auto found_def = srv_cfg->getCfgOptionDef()->get("isc", "one"); - if (fetchConfigElement("dhcp4_option_def") && + if (hasConfigElement("dhcp4_option_def") && getTimestamp("dhcp4_option_def") > lb_modification_time) { ASSERT_TRUE(found_def); EXPECT_EQ(101, found_def->getCode()); @@ -481,7 +490,7 @@ public: // be merged. auto options = srv_cfg->getCfgOption(); auto found_opt = options->get("dhcp4", DHO_HOST_NAME); - if (fetchConfigElement("dhcp4_options") && + if (hasConfigElement("dhcp4_options") && (getTimestamp("dhcp4_options") > lb_modification_time)) { ASSERT_TRUE(found_opt.option_); EXPECT_EQ("new.example.com", found_opt.option_->toString()); @@ -495,7 +504,7 @@ public: // be merged. auto networks = srv_cfg->getCfgSharedNetworks4(); auto found_network = networks->getByName("one"); - if (fetchConfigElement("dhcp4_shared_network") && + if (hasConfigElement("dhcp4_shared_network") && (getTimestamp("dhcp4_shared_network") > lb_modification_time)) { ASSERT_TRUE(found_network); EXPECT_TRUE(found_network->hasFetchGlobalsFn()); @@ -508,7 +517,7 @@ public: // time is later than last audit entry time it should be merged. auto subnets = srv_cfg->getCfgSubnets4(); auto found_subnet = subnets->getSubnet(1); - if (fetchConfigElement("dhcp4_subnet") && + if (hasConfigElement("dhcp4_subnet") && (getTimestamp("dhcp4_subnet") > lb_modification_time)) { ASSERT_TRUE(found_subnet); EXPECT_TRUE(found_subnet->hasFetchGlobalsFn()); @@ -648,11 +657,16 @@ public: // types are merged into the current configuration. TEST_F(CBControlDHCPv4Test, databaseConfigApplyAll) { - addCreateAuditEntry("dhcp4_global_parameter"); - addCreateAuditEntry("dhcp4_option_def"); - addCreateAuditEntry("dhcp4_options"); - addCreateAuditEntry("dhcp4_shared_network"); - addCreateAuditEntry("dhcp4_subnet"); + addCreateAuditEntry("dhcp4_global_parameter", 1); + addCreateAuditEntry("dhcp4_global_parameter", 2); + addCreateAuditEntry("dhcp4_option_def", 1); + addCreateAuditEntry("dhcp4_option_def", 2); + addCreateAuditEntry("dhcp4_options", 1); + addCreateAuditEntry("dhcp4_options", 2); + addCreateAuditEntry("dhcp4_shared_network", 1); + addCreateAuditEntry("dhcp4_shared_network", 2); + addCreateAuditEntry("dhcp4_subnet", 1); + addCreateAuditEntry("dhcp4_subnet", 2); testDatabaseConfigApply(getTimestamp(-5)); } @@ -689,7 +703,8 @@ TEST_F(CBControlDHCPv4Test, databaseConfigApplyDeleteNonExisting) { // This test verifies that only a global parameter is merged into // the current configuration. TEST_F(CBControlDHCPv4Test, databaseConfigApplyGlobal) { - addCreateAuditEntry("dhcp4_global_parameter"); + addCreateAuditEntry("dhcp4_global_parameter", 1); + addCreateAuditEntry("dhcp4_global_parameter", 2); testDatabaseConfigApply(getTimestamp(-5)); } @@ -706,14 +721,16 @@ TEST_F(CBControlDHCPv4Test, databaseConfigApplyDeleteGlobal) { // database when the modification time is earlier than the last // fetched audit entry. TEST_F(CBControlDHCPv4Test, databaseConfigApplyGlobalNotFetched) { - addCreateAuditEntry("dhcp4_global_parameter"); + addCreateAuditEntry("dhcp4_global_parameter", 1); + addCreateAuditEntry("dhcp4_global_parameter", 2); testDatabaseConfigApply(getTimestamp(-3)); } // This test verifies that only an option definition is merged into // the current configuration. TEST_F(CBControlDHCPv4Test, databaseConfigApplyOptionDef) { - addCreateAuditEntry("dhcp4_option_def"); + addCreateAuditEntry("dhcp4_option_def", 1); + addCreateAuditEntry("dhcp4_option_def", 2); testDatabaseConfigApply(getTimestamp(-5)); } @@ -731,14 +748,16 @@ TEST_F(CBControlDHCPv4Test, databaseConfigApplyDeleteOptionDef) { // database when the modification time is earlier than the last // fetched audit entry. TEST_F(CBControlDHCPv4Test, databaseConfigApplyOptionDefNotFetched) { - addCreateAuditEntry("dhcp4_option_def"); + addCreateAuditEntry("dhcp4_option_def", 1); + addCreateAuditEntry("dhcp4_option_def", 2); testDatabaseConfigApply(getTimestamp(-3)); } // This test verifies that only a DHCPv4 option is merged into the // current configuration. TEST_F(CBControlDHCPv4Test, databaseConfigApplyOption) { - addCreateAuditEntry("dhcp4_options"); + addCreateAuditEntry("dhcp4_options", 1); + addCreateAuditEntry("dhcp4_options", 2); testDatabaseConfigApply(getTimestamp(-5)); } @@ -755,14 +774,16 @@ TEST_F(CBControlDHCPv4Test, databaseConfigApplyDeleteOption) { // database when the modification time is earlier than the last // fetched audit entry. TEST_F(CBControlDHCPv4Test, databaseConfigApplyOptionNotFetched) { - addCreateAuditEntry("dhcp4_options"); + addCreateAuditEntry("dhcp4_options", 1); + addCreateAuditEntry("dhcp4_options", 2); testDatabaseConfigApply(getTimestamp(-3)); } // This test verifies that only a shared network is merged into the // current configuration. TEST_F(CBControlDHCPv4Test, databaseConfigApplySharedNetwork) { - addCreateAuditEntry("dhcp4_shared_network"); + addCreateAuditEntry("dhcp4_shared_network", 1); + addCreateAuditEntry("dhcp4_shared_network", 2); testDatabaseConfigApply(getTimestamp(-5)); } @@ -779,15 +800,18 @@ TEST_F(CBControlDHCPv4Test, databaseConfigApplyDeleteSharedNetwork) { // database when the modification time is earlier than the last // fetched audit entry. TEST_F(CBControlDHCPv4Test, databaseConfigApplySharedNetworkNotFetched) { - addCreateAuditEntry("dhcp4_shared_network"); + addCreateAuditEntry("dhcp4_shared_network", 1); + addCreateAuditEntry("dhcp4_shared_network", 2); testDatabaseConfigApply(getTimestamp(-3)); } // This test verifies that only a subnet is merged into the current // configuration. TEST_F(CBControlDHCPv4Test, databaseConfigApplySubnet) { - addCreateAuditEntry("dhcp4_shared_network"); - addCreateAuditEntry("dhcp4_subnet"); + addCreateAuditEntry("dhcp4_shared_network", 1); + addCreateAuditEntry("dhcp4_shared_network", 2); + addCreateAuditEntry("dhcp4_subnet", 1); + addCreateAuditEntry("dhcp4_subnet", 2); testDatabaseConfigApply(getTimestamp(-5)); } @@ -803,7 +827,8 @@ TEST_F(CBControlDHCPv4Test, databaseConfigApplyDeleteSubnet) { // when the modification time is earlier than the last fetched audit // entry. TEST_F(CBControlDHCPv4Test, databaseConfigApplySubnetNotFetched) { - addCreateAuditEntry("dhcp4_subnet"); + addCreateAuditEntry("dhcp4_subnet", 1); + addCreateAuditEntry("dhcp4_subnet", 2); testDatabaseConfigApply(getTimestamp(-3)); } @@ -818,11 +843,16 @@ TEST_F(CBControlDHCPv4Test, databaseConfigApplyHook) { "cb4_updated", cb4_updated_callout)); // Create audit entries. - addCreateAuditEntry("dhcp4_global_parameter"); - addCreateAuditEntry("dhcp4_option_def"); - addCreateAuditEntry("dhcp4_options"); - addCreateAuditEntry("dhcp4_shared_network"); - addCreateAuditEntry("dhcp4_subnet"); + addCreateAuditEntry("dhcp4_global_parameter", 1); + addCreateAuditEntry("dhcp4_global_parameter", 2); + addCreateAuditEntry("dhcp4_option_def", 1); + addCreateAuditEntry("dhcp4_option_def", 2); + addCreateAuditEntry("dhcp4_options", 1); + addCreateAuditEntry("dhcp4_options", 2); + addCreateAuditEntry("dhcp4_shared_network", 1); + addCreateAuditEntry("dhcp4_shared_network", 2); + addCreateAuditEntry("dhcp4_subnet", 1); + addCreateAuditEntry("dhcp4_subnet", 2); // Run the test. testDatabaseConfigApply(getTimestamp(-5)); @@ -1078,7 +1108,7 @@ public: // If there is an audit entry for global parameter and the parameter // modification time is later than last audit entry time it should // be merged. - if (fetchConfigElement("dhcp6_global_parameter") && + if (hasConfigElement("dhcp6_global_parameter") && (getTimestamp("dhcp6_global_parameter") > lb_modification_time)) { checkConfiguredGlobal(srv_cfg, "foo", Element::create("bar")); @@ -1091,7 +1121,7 @@ public: // modification time is later than last audit entry time it should // be merged. auto found_def = srv_cfg->getCfgOptionDef()->get("isc", "one"); - if (fetchConfigElement("dhcp6_option_def") && + if (hasConfigElement("dhcp6_option_def") && getTimestamp("dhcp6_option_def") > lb_modification_time) { ASSERT_TRUE(found_def); EXPECT_EQ(101, found_def->getCode()); @@ -1106,7 +1136,7 @@ public: // be merged. auto options = srv_cfg->getCfgOption(); auto found_opt = options->get("dhcp6", D6O_BOOTFILE_URL); - if (fetchConfigElement("dhcp6_options") && + if (hasConfigElement("dhcp6_options") && (getTimestamp("dhcp6_options") > lb_modification_time)) { ASSERT_TRUE(found_opt.option_); EXPECT_EQ("some.bootfile", found_opt.option_->toString()); @@ -1120,7 +1150,7 @@ public: // be merged. auto networks = srv_cfg->getCfgSharedNetworks6(); auto found_network = networks->getByName("one"); - if (fetchConfigElement("dhcp6_shared_network") && + if (hasConfigElement("dhcp6_shared_network") && (getTimestamp("dhcp6_shared_network") > lb_modification_time)) { ASSERT_TRUE(found_network); EXPECT_TRUE(found_network->hasFetchGlobalsFn()); @@ -1133,7 +1163,7 @@ public: // time is later than last audit entry time it should be merged. auto subnets = srv_cfg->getCfgSubnets6(); auto found_subnet = subnets->getSubnet(1); - if (fetchConfigElement("dhcp6_subnet") && + if (hasConfigElement("dhcp6_subnet") && (getTimestamp("dhcp6_subnet") > lb_modification_time)) { ASSERT_TRUE(found_subnet); EXPECT_TRUE(found_subnet->hasFetchGlobalsFn()); @@ -1272,11 +1302,16 @@ public: // types are merged into the current configuration. TEST_F(CBControlDHCPv6Test, databaseConfigApplyAll) { - addCreateAuditEntry("dhcp6_global_parameter"); - addCreateAuditEntry("dhcp6_option_def"); - addCreateAuditEntry("dhcp6_options"); - addCreateAuditEntry("dhcp6_shared_network"); - addCreateAuditEntry("dhcp6_subnet"); + addCreateAuditEntry("dhcp6_global_parameter", 1); + addCreateAuditEntry("dhcp6_global_parameter", 2); + addCreateAuditEntry("dhcp6_option_def", 1); + addCreateAuditEntry("dhcp6_option_def", 2); + addCreateAuditEntry("dhcp6_options", 1); + addCreateAuditEntry("dhcp6_options", 2); + addCreateAuditEntry("dhcp6_shared_network", 1); + addCreateAuditEntry("dhcp6_shared_network", 2); + addCreateAuditEntry("dhcp6_subnet", 1); + addCreateAuditEntry("dhcp6_subnet", 2); testDatabaseConfigApply(getTimestamp(-5)); } @@ -1313,7 +1348,8 @@ TEST_F(CBControlDHCPv6Test, databaseConfigApplyDeleteNonExisting) { // This test verifies that only a global parameter is merged into // the current configuration. TEST_F(CBControlDHCPv6Test, databaseConfigApplyGlobal) { - addCreateAuditEntry("dhcp6_global_parameter"); + addCreateAuditEntry("dhcp6_global_parameter", 1); + addCreateAuditEntry("dhcp6_global_parameter", 2); testDatabaseConfigApply(getTimestamp(-5)); } @@ -1330,14 +1366,16 @@ TEST_F(CBControlDHCPv6Test, databaseConfigApplyDeleteGlobal) { // database when the modification time is earlier than the last // fetched audit entry. TEST_F(CBControlDHCPv6Test, databaseConfigApplyGlobalNotFetched) { - addCreateAuditEntry("dhcp6_global_parameter"); + addCreateAuditEntry("dhcp6_global_parameter", 1); + addCreateAuditEntry("dhcp6_global_parameter", 2); testDatabaseConfigApply(getTimestamp(-3)); } // This test verifies that only an option definition is merged into // the current configuration. TEST_F(CBControlDHCPv6Test, databaseConfigApplyOptionDef) { - addCreateAuditEntry("dhcp6_option_def"); + addCreateAuditEntry("dhcp6_option_def", 1); + addCreateAuditEntry("dhcp6_option_def", 2); testDatabaseConfigApply(getTimestamp(-5)); } @@ -1355,14 +1393,16 @@ TEST_F(CBControlDHCPv6Test, databaseConfigApplyDeleteOptionDef) { // database when the modification time is earlier than the last // fetched audit entry. TEST_F(CBControlDHCPv6Test, databaseConfigApplyOptionDefNotFetched) { - addCreateAuditEntry("dhcp6_option_def"); + addCreateAuditEntry("dhcp6_option_def", 1); + addCreateAuditEntry("dhcp6_option_def", 2); testDatabaseConfigApply(getTimestamp(-3)); } // This test verifies that only a DHCPv6 option is merged into the // current configuration. TEST_F(CBControlDHCPv6Test, databaseConfigApplyOption) { - addCreateAuditEntry("dhcp6_options"); + addCreateAuditEntry("dhcp6_options", 1); + addCreateAuditEntry("dhcp6_options", 2); testDatabaseConfigApply(getTimestamp(-5)); } @@ -1379,14 +1419,16 @@ TEST_F(CBControlDHCPv6Test, databaseConfigApplyDeleteOption) { // database when the modification time is earlier than the last // fetched audit entry. TEST_F(CBControlDHCPv6Test, databaseConfigApplyOptionNotFetched) { - addCreateAuditEntry("dhcp6_options"); + addCreateAuditEntry("dhcp6_options", 1); + addCreateAuditEntry("dhcp6_options", 2); testDatabaseConfigApply(getTimestamp(-3)); } // This test verifies that only a shared network is merged into the // current configuration. TEST_F(CBControlDHCPv6Test, databaseConfigApplySharedNetwork) { - addCreateAuditEntry("dhcp6_shared_network"); + addCreateAuditEntry("dhcp6_shared_network", 1); + addCreateAuditEntry("dhcp6_shared_network", 2); testDatabaseConfigApply(getTimestamp(-5)); } @@ -1403,15 +1445,18 @@ TEST_F(CBControlDHCPv6Test, databaseConfigApplyDeleteSharedNetwork) { // database when the modification time is earlier than the last // fetched audit entry. TEST_F(CBControlDHCPv6Test, databaseConfigApplySharedNetworkNotFetched) { - addCreateAuditEntry("dhcp6_shared_network"); + addCreateAuditEntry("dhcp6_shared_network", 1); + addCreateAuditEntry("dhcp6_shared_network", 2); testDatabaseConfigApply(getTimestamp(-3)); } // This test verifies that only a subnet is merged into the current // configuration. TEST_F(CBControlDHCPv6Test, databaseConfigApplySubnet) { - addCreateAuditEntry("dhcp6_shared_network"); - addCreateAuditEntry("dhcp6_subnet"); + addCreateAuditEntry("dhcp6_shared_network", 1); + addCreateAuditEntry("dhcp6_shared_network", 2); + addCreateAuditEntry("dhcp6_subnet", 1); + addCreateAuditEntry("dhcp6_subnet", 2); testDatabaseConfigApply(getTimestamp(-5)); } @@ -1427,7 +1472,8 @@ TEST_F(CBControlDHCPv6Test, databaseConfigApplyDeleteSubnet) { // when the modification time is earlier than the last fetched audit // entry. TEST_F(CBControlDHCPv6Test, databaseConfigApplySubnetNotFetched) { - addCreateAuditEntry("dhcp6_subnet"); + addCreateAuditEntry("dhcp6_subnet", 1); + addCreateAuditEntry("dhcp6_subnet", 2); testDatabaseConfigApply(getTimestamp(-3)); } @@ -1442,11 +1488,16 @@ TEST_F(CBControlDHCPv6Test, databaseConfigApplyHook) { "cb6_updated", cb6_updated_callout)); // Create audit entries. - addCreateAuditEntry("dhcp6_global_parameter"); - addCreateAuditEntry("dhcp6_option_def"); - addCreateAuditEntry("dhcp6_options"); - addCreateAuditEntry("dhcp6_shared_network"); - addCreateAuditEntry("dhcp6_subnet"); + addCreateAuditEntry("dhcp6_global_parameter", 1); + addCreateAuditEntry("dhcp6_global_parameter", 2); + addCreateAuditEntry("dhcp6_option_def", 1); + addCreateAuditEntry("dhcp6_option_def", 2); + addCreateAuditEntry("dhcp6_options", 1); + addCreateAuditEntry("dhcp6_options", 2); + addCreateAuditEntry("dhcp6_shared_network", 1); + addCreateAuditEntry("dhcp6_shared_network", 2); + addCreateAuditEntry("dhcp6_subnet", 1); + addCreateAuditEntry("dhcp6_subnet", 2); // Run the test. testDatabaseConfigApply(getTimestamp(-5)); diff --git a/src/lib/process/cb_ctl_base.h b/src/lib/process/cb_ctl_base.h index c74140185f..2b27bbaa1f 100644 --- a/src/lib/process/cb_ctl_base.h +++ b/src/lib/process/cb_ctl_base.h @@ -242,38 +242,35 @@ public: protected: - /// @brief Checks if there are new or updated configuration elements of - /// specific type to be fetched from the database. + /// @brief Returns audit entries for new or updated configuration + /// elements of specific type to be fetched from the database. /// /// This is convenience method invoked from the implementations of the - /// @c databaseConfigApply function. This method is invoked in two cases: - /// when the server starts up and fetches the entire configuration available - /// for it and when it should fetch the updates to the existing configuration. - /// In the former case, the collection of audit entries is always empty. - /// In the latter case it contains audit entries indicating the updates - /// to be fetched. This method checks if the implementation of the - /// @c databaseConfigApply should fetch updated configuration of the - /// configuration elements of the specific type. Therefore, it returns true - /// if the audit entries collection is empty (the former case described - /// above) or the audit entries collection contains CREATE or UPDATE - /// entries of the specific type. + /// @c databaseConfigApply function. This method is invoked when the + /// server should fetch the updates to the existing configuration. + /// The collection of audit entries contains audit entries indicating + /// the updates to be fetched. This method returns audit entries for + /// updated configuration elements of the specific type the + /// @c databaseConfigApply should fetch. The returned collection od + /// audit entries contains CREATE or UPDATE entries of the specific type. /// - /// @return true if there are new or updated configuration elements to - /// be fetched from the database or the audit entries collection is empty. - bool fetchConfigElement(const db::AuditEntryCollection& audit_entries, - const std::string& object_type) const { - if (!audit_entries.empty()) { - const auto& index = audit_entries.get(); - auto range = index.equal_range(object_type); - for (auto it = range.first; it != range.second; ++it) { - if ((*it)->getModificationType() != db::AuditEntry::ModificationType::DELETE) { - return (true); - } + /// @param audit_entries collection od audit entries to filter. + /// @param object_type object type to filter with. + /// @return audit entries collection with CREATE or UPDATE entries + /// of the specific type be fetched from the database. + db::AuditEntryCollection + fetchConfigElement(const db::AuditEntryCollection& audit_entries, + const std::string& object_type) const { + db::AuditEntryCollection result; + const auto& index = audit_entries.get(); + auto range = index.equal_range(object_type); + for (auto it = range.first; it != range.second; ++it) { + if ((*it)->getModificationType() != db::AuditEntry::ModificationType::DELETE) { + result.insert(*it); } - return (false); } - return (true); + return (result); } /// @brief Server specific method to fetch and apply back end @@ -359,6 +356,17 @@ protected: uint64_t last_audit_entry_id_; }; +/// @brief Checks if an object is in a collection od audit entries. +/// +/// @param audit_entries collection od audit entries to search for. +/// @param object_id object identifier. +inline bool +hasObjectId(const db::AuditEntryCollection& audit_entries, + const uint64_t& object_id) { + const auto& object_id_idx = audit_entries.get(); + return (object_id_idx.count(object_id) > 0); +} + } // end of namespace isc::process } // end of namespace isc diff --git a/src/lib/process/tests/cb_ctl_base_unittests.cc b/src/lib/process/tests/cb_ctl_base_unittests.cc index 1d53fc9ce4..914a385dff 100644 --- a/src/lib/process/tests/cb_ctl_base_unittests.cc +++ b/src/lib/process/tests/cb_ctl_base_unittests.cc @@ -428,14 +428,14 @@ TEST_F(CBControlBaseTest, reset) { EXPECT_EQ(0, cb_ctl_.getLastAuditEntryId()); } -// This test verifies that it is correctly determined whether the -// server should fetch the particular configuration element. +// This test verifies that it is correctly determined what entries the +// server should fetch for the particular configuration element. TEST_F(CBControlBaseTest, fetchConfigElement) { db::AuditEntryCollection audit_entries; - // When audit entries collection is empty it indicates that this - // is the case of the full server reconfiguration. Always indicate - // that the configuration elements must be fetched. - EXPECT_TRUE(cb_ctl_.fetchConfigElement(audit_entries, "my_object_type")); + db::AuditEntryCollection updated; + // When audit entries collection is empty any subset is empty too. + updated = cb_ctl_.fetchConfigElement(audit_entries, "my_object_type"); + EXPECT_TRUE(updated.empty()); // Now test the case that there is a DELETE audit entry. In this case // our function should indicate that the configuration should not be @@ -446,22 +446,53 @@ TEST_F(CBControlBaseTest, fetchConfigElement) { AuditEntry::ModificationType::DELETE, 2345, "added audit entry")); audit_entries.insert(audit_entry); - EXPECT_FALSE(cb_ctl_.fetchConfigElement(audit_entries, "my_object_type")); + updated = cb_ctl_.fetchConfigElement(audit_entries, "my_object_type"); + EXPECT_TRUE(updated.empty()); + EXPECT_TRUE(hasObjectId(audit_entries, 1234)); + EXPECT_FALSE(hasObjectId(audit_entries, 5678)); + EXPECT_FALSE(hasObjectId(updated, 1234)); // Add another audit entry which indicates creation of the configuration element. - // This time we should get 'true'. + // This time we should get it. audit_entry.reset(new AuditEntry("my_object_type", 5678, AuditEntry::ModificationType::CREATE, 6789, "added audit entry")); audit_entries.insert(audit_entry); - EXPECT_TRUE(cb_ctl_.fetchConfigElement(audit_entries, "my_object_type")); + updated = cb_ctl_.fetchConfigElement(audit_entries, "my_object_type"); + ASSERT_EQ(1, updated.size()); + AuditEntryPtr updated_entry = (*updated.begin()); + ASSERT_TRUE(updated_entry); + EXPECT_EQ("my_object_type", updated_entry->getObjectType()); + EXPECT_EQ(5678, updated_entry->getObjectId()); + EXPECT_EQ(AuditEntry::ModificationType::CREATE, updated_entry->getModificationType()); + EXPECT_TRUE(hasObjectId(audit_entries, 5678)); + EXPECT_TRUE(hasObjectId(updated, 5678)); + EXPECT_FALSE(hasObjectId(updated, 1234)); // Also we should get 'true' for the UPDATE case. - audit_entry.reset(new AuditEntry("another_object_type", + audit_entry.reset(new AuditEntry("my_object_type", 5678, AuditEntry::ModificationType::UPDATE, 6790, "added audit entry")); audit_entries.insert(audit_entry); - EXPECT_TRUE(cb_ctl_.fetchConfigElement(audit_entries, "another_object_type")); + updated = cb_ctl_.fetchConfigElement(audit_entries, "my_object_type"); + EXPECT_EQ(2, updated.size()); + bool saw_create = false; + bool saw_update = false; + for (auto entry : updated) { + EXPECT_EQ("my_object_type", entry->getObjectType()); + EXPECT_EQ(5678, entry->getObjectId()); + if (AuditEntry::ModificationType::CREATE == entry->getModificationType()) { + EXPECT_FALSE(saw_create); + saw_create = true; + } else if (AuditEntry::ModificationType::UPDATE == entry->getModificationType()) { + EXPECT_FALSE(saw_update); + saw_update = true; + } + } + EXPECT_TRUE(saw_create); + EXPECT_TRUE(saw_update); + EXPECT_TRUE(hasObjectId(updated, 5678)); + EXPECT_FALSE(hasObjectId(updated, 1234)); } // This test verifies that true is return when the server successfully -- 2.47.2