diff --git a/src/bin/dhcp4/tests/config_backend_unittest.cc b/src/bin/dhcp4/tests/config_backend_unittest.cc index dde99847c6..26d8abbb74 100644 --- a/src/bin/dhcp4/tests/config_backend_unittest.cc +++ b/src/bin/dhcp4/tests/config_backend_unittest.cc @@ -335,15 +335,19 @@ TEST_F(Dhcp4CBTest, mergeOptionDefs) { // This test verifies that externally configured options // merged correctly into staging configuration. -// @todo enable test when SrvConfig can merge options. -TEST_F(Dhcp4CBTest, DISABLED_mergeOptions) { +TEST_F(Dhcp4CBTest, mergeOptions) { string base_config = "{ \n" - " \"option-data\": [ {" - " \"name\": \"dhcp-message\"," - " \"data\": \"0A0B0C0D\"," - " \"csv-format\": false" - " } ]," + " \"option-data\": [ { \n" + " \"name\": \"dhcp-message\", \n" + " \"data\": \"0A0B0C0D\", \n" + " \"csv-format\": false \n" + " },{ \n" + " \"name\": \"host-name\", \n" + " \"data\": \"old.example.com\", \n" + " \"csv-format\": true \n" + " } \n" + " ], \n" " \"config-control\": { \n" " \"config-databases\": [ { \n" " \"type\": \"memfile\", \n" @@ -358,19 +362,28 @@ TEST_F(Dhcp4CBTest, DISABLED_mergeOptions) { extractConfig(base_config); - // Create option two and add it to first backend. - OptionDescriptorPtr opt_two(new OptionDescriptor( - createOption(Option::V4, DHO_BOOT_FILE_NAME, - true, false, "my-boot-file"))); - opt_two->space_name_ = DHCP4_OPTION_SPACE; - db1_->createUpdateOption4(ServerSelector::ALL(), opt_two); + OptionDescriptorPtr opt; - // Create option three and add it to second backend. - OptionDescriptorPtr opt_three(new OptionDescriptor( - createOption(Option::V4, DHO_BOOT_FILE_NAME, - true, false, "your-boot-file"))); - opt_three->space_name_ = DHCP4_OPTION_SPACE; - db2_->createUpdateOption4(ServerSelector::ALL(), opt_three); + // Add host-name to the first backend. + opt.reset(new OptionDescriptor( + createOption(Option::V4, DHO_HOST_NAME, + true, false, "new.example.com"))); + opt->space_name_ = DHCP4_OPTION_SPACE; + db1_->createUpdateOption4(ServerSelector::ALL(), opt); + + // Add boot-file-name to the first backend. + opt.reset(new OptionDescriptor( + createOption(Option::V4, DHO_BOOT_FILE_NAME, + true, false, "my-boot-file"))); + opt->space_name_ = DHCP4_OPTION_SPACE; + db1_->createUpdateOption4(ServerSelector::ALL(), opt); + + // Add boot-file-name to the second backend. + opt.reset(new OptionDescriptor( + createOption(Option::V4, DHO_BOOT_FILE_NAME, + true, false, "your-boot-file"))); + opt->space_name_ = DHCP4_OPTION_SPACE; + db2_->createUpdateOption4(ServerSelector::ALL(), opt); // Should parse and merge without error. ASSERT_NO_FATAL_FAILURE(configure(base_config, CONTROL_RESULT_SUCCESS, "")); @@ -381,13 +394,21 @@ TEST_F(Dhcp4CBTest, DISABLED_mergeOptions) { // Option definition from JSON should be there. CfgOptionPtr options = staging_cfg->getCfgOption(); + // dhcp-message should come from the original config. OptionDescriptor found_opt = options->get("dhcp4", DHO_DHCP_MESSAGE); ASSERT_TRUE(found_opt.option_); EXPECT_EQ("0x0A0B0C0D", found_opt.option_->toHexString()); + // host-name should come from the first back end, + // (overwriting the original). + found_opt = options->get("dhcp4", DHO_HOST_NAME); + ASSERT_TRUE(found_opt.option_); + EXPECT_EQ("new.example.com", found_opt.option_->toString()); + + // booth-file-name should come from the first back end. found_opt = options->get("dhcp4", DHO_BOOT_FILE_NAME); ASSERT_TRUE(found_opt.option_); - EXPECT_EQ("my-boot-file", found_opt.formatted_value_); + EXPECT_EQ("my-boot-file", found_opt.option_->toString()); } // This test verifies that externally configured shared-networks are diff --git a/src/lib/dhcpsrv/cfg_option.cc b/src/lib/dhcpsrv/cfg_option.cc index 71a4d5a66d..85a8d7d84c 100644 --- a/src/lib/dhcpsrv/cfg_option.cc +++ b/src/lib/dhcpsrv/cfg_option.cc @@ -11,6 +11,8 @@ #include #include #include +#include +#include #include #include #include @@ -66,6 +68,32 @@ CfgOption::add(const OptionDescriptor& desc, const std::string& option_space) { } } +void +CfgOption::replace(const OptionDescriptor& desc, const std::string& option_space) { + if (!desc.option_) { + isc_throw(isc::BadValue, "option being replaced must not be NULL"); + } + + // Check for presence of options. + OptionContainerPtr options = getAll(option_space); + if (!options) { + isc_throw(isc::BadValue, "option space " << option_space + << " does not exist"); + } + + // Find the option we want to replace. + OptionContainerTypeIndex& idx = options->get<1>(); + OptionContainerTypeIndex::const_iterator od_itr = idx.find(desc.option_->getType()); + if (od_itr == idx.end()) { + isc_throw(isc::BadValue, "cannot replace option: " + << option_space << ":" << desc.option_->getType() + << ", it does not exist"); + } + + idx.replace(od_itr, desc); +} + + std::list CfgOption::getVendorIdsSpaceNames() const { std::list ids = getVendorIds(); @@ -81,6 +109,101 @@ CfgOption::getVendorIdsSpaceNames() const { return (names); } +void +CfgOption::merge(CfgOptionDefPtr cfg_def, CfgOption& other) { + // First we merge our options into other. + // This adds my options that are not + // in other, to other (i.e we skip over + // duplicates). + mergeTo(other); + + // Create option instances based on the given definitions. + other.createOptions(cfg_def); + + // Next we copy "other" on top of ourself. + other.copyTo(*this); +} + +void +CfgOption::createOptions(CfgOptionDefPtr cfg_def) { + // Iterate over all the option descriptors in + // all the spaces and instantiate the options + // based on the given definitions. + for (auto space : getOptionSpaceNames()) { + for (auto opt_desc : *(getAll(space))) { + if (createDescriptorOption(cfg_def, space, opt_desc)) { + // Option was recreated, let's replace the descriptor. + replace(opt_desc,space); + } + } + } +} + +bool +CfgOption::createDescriptorOption(CfgOptionDefPtr cfg_def, const std::string& space, + OptionDescriptor& opt_desc) { + if (!opt_desc.option_) { + isc_throw(BadValue, + "validateCreateOption: descriptor has no option instance"); + } + + Option::Universe universe = opt_desc.option_->getUniverse(); + uint16_t code = opt_desc.option_->getType(); + + // Find the option's defintion, if it has one. + // First, check for a standard definition. + OptionDefinitionPtr def = LibDHCP::getOptionDef(space, code); + + // If there is no standard definition but the option is vendor specific, + // we should search the definition within the vendor option space. + if (!def && (space != DHCP4_OPTION_SPACE) && (space != DHCP6_OPTION_SPACE)) { + uint32_t vendor_id = LibDHCP::optionSpaceToVendorId(space); + if (vendor_id > 0) { + def = LibDHCP::getVendorOptionDef(universe, vendor_id, code); + } + } + + // Still haven't found the definition, so look for custom + // definition in the given set of configured definitions + if (!def) { + def = cfg_def->get(space, code); + } + + std::string& formatted_value = opt_desc.formatted_value_; + if (!def) { + if (!formatted_value.empty()) { + isc_throw(InvalidOperation, "option: " << space << "." << code + << " has a formatted value: '" << formatted_value + << "' but no option definition"); + } + + // If there's no definition and no formatted string, we'll + // settle for the generic option already in the descriptor. + // Indicate no-change by returning false. + return (false); + } + + try { + // Definition found. Let's replace the generic option in + // the descriptor with one created based on definition's factory. + if (formatted_value.empty()) { + // No formatted value, use data stored in the generic option. + opt_desc.option_ = def->optionFactory(universe, code, opt_desc.option_->getData()); + } else { + // Spit the value specified in comma separated values format. + std::vector split_vec; + boost::split(split_vec, formatted_value, boost::is_any_of(",")); + opt_desc.option_ = def->optionFactory(universe, code, split_vec); + } + } catch (const std::exception& ex) { + isc_throw(InvalidOperation, "could not create option: " << space << "." << code + << " from data specified, reason: " << ex.what()); + } + + // Indicate we replaced the definition. + return(true); +} + void CfgOption::mergeTo(CfgOption& other) const { // Merge non-vendor options. diff --git a/src/lib/dhcpsrv/cfg_option.h b/src/lib/dhcpsrv/cfg_option.h index 600321f7a8..060c940ea0 100644 --- a/src/lib/dhcpsrv/cfg_option.h +++ b/src/lib/dhcpsrv/cfg_option.h @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -332,6 +333,96 @@ public: /// @throw isc::BadValue if the option space is invalid. void add(const OptionDescriptor& desc, const std::string& option_space); + /// @brief Replaces the instance of an option within this collection + /// + /// This method locates the option within the given space and replaces + /// it with a copy of the given descriptor. This effectively updates + /// the contents without altering the container indexing. + /// + /// @param desc Option descriptor holding option instance and other + /// parameters pertaining to the option. + /// @param option_space Option space name. + /// + /// @throw isc::BadValue if the descriptor's option instance is null, + /// if space is invalid, or if the option does not already exist + /// in the given space. + void replace(const OptionDescriptor& desc, const std::string& option_space); + + /// @brief Merges another option configuration into this one. + /// + /// This method calls @c mergeTo() to add this configuration's + /// options into @c other (skipping any duplicates). Next it calls + /// @c createDescriptorOption() for each option descriptor in the + /// merged set. This (re)-creates each descriptor's option based on + /// the merged set of opt definitions. Finally, it calls + /// @c copyTo() to overwrite this configuration's options with + /// the merged set in @c other. + /// + /// @warning The merge operation will affect the @c other configuration. + /// Therefore, the caller must not rely on the data held in the @c other + /// object after the call to @c merge. Also, the data held in @c other must + /// not be modified after the call to @c merge because it may affect the + /// merged configuration. + /// + /// @param cfg_def set of of user-defined option definitions to use + /// when creating option instances. + /// @param option configurations to merge with. + void merge(CfgOptionDefPtr cfg_def, CfgOption& other); + + /// @brief Re-create the option in each descriptor based on given definitions + /// + /// Invokes @c createDescriptorOption() on each option descriptor in + /// each option space, passing in the the given dictionary of option + /// definitions. If the descriptor's option is re-created, then the + /// descriptor is updated by calling @c replace(). + /// + /// @param cfg_def set of of user-defined option definitions to use + /// when creating option instances. + void createOptions(CfgOptionDefPtr cfg_def); + + /// @brief Creates an option descriptor's option based on a set of option defs + /// + /// This function's primary use is to create definition specific options for + /// option descriptors fetched from a configuration backend, as part of a + /// configuration merge. + /// + /// Given an OptionDescriptor whose option_ member contains a generic option + /// (i.e has a code and/or data), this function will attempt to find a matching + /// definition and then use that definition's factory to create an option + /// instance specific to that definition. It will then replace the descriptor's + /// generic option with the specific option. + /// + /// Three sources of definitions are searched, in the following order: + /// + /// 1. Standard option definitions (@c LIBDHCP::getOptionDef)) + /// 2. Vendor option definitions (@c LIBDHCP::getVendorOptionDef)) + /// 3. User specified definitions passed in via cfg_def parameter. + /// + /// The code will use the first matching definition found. It then applies + /// the following rules: + /// + /// -# If no definition is found but the descriptor conveys a non-empty + /// formatted value, throw an error. + /// -# If not definition is found and there is no formatted value, return + /// This leaves intact the generic option in the descriptor. + /// -# If a definition is found and there is no formatted value, pass the + /// descriptor's generic option's data into the definition's factory. Replace + /// the descriptor's option with the newly created option. + /// -# If a definition is found and there is a formatted value, split + /// the value into vector of values and pass that into the definition's + /// factory. Replace the descriptor's option with the newly created option. + /// + /// @param cfg_def the user specified definitions to use + /// @param space the option space name of the option + /// @param opt_desc OptionDescriptor describing the option. + /// + /// @return True if the descriptor's option instance was replaced. + /// @throw InvalidOperation if the descriptor conveys a formatted value and + /// there is no definition matching the option code in the given space, or + /// if the definition factory invocation fails. + static bool createDescriptorOption(CfgOptionDefPtr cfg_def, const std::string& space, + OptionDescriptor& opt_desc); + /// @brief Merges this configuration to another configuration. /// /// This method iterates over the configuration items held in this @@ -339,8 +430,6 @@ public: /// as a parameter. If an item exists in the destination it is not /// copied. /// - /// @note: this method is not longer used so should become private. - /// /// @param [out] other Configuration object to merge to. void mergeTo(CfgOption& other) const; diff --git a/src/lib/dhcpsrv/cfg_shared_networks.cc b/src/lib/dhcpsrv/cfg_shared_networks.cc index f0f02d3cc8..7f5c48041f 100644 --- a/src/lib/dhcpsrv/cfg_shared_networks.cc +++ b/src/lib/dhcpsrv/cfg_shared_networks.cc @@ -19,8 +19,10 @@ CfgSharedNetworks4::hasNetworkWithServerId(const IOAddress& server_id) const { return (network_it != index.cend()); } + + void -CfgSharedNetworks4::merge(CfgSharedNetworks4& other) { +CfgSharedNetworks4::merge(CfgOptionDefPtr cfg_def, CfgSharedNetworks4& other) { auto& index = networks_.get(); // Iterate over the subnets to be merged. They will replace the existing @@ -59,6 +61,9 @@ CfgSharedNetworks4::merge(CfgSharedNetworks4& other) { index.erase(existing_network); } + // Create the network's options based on the given definitions. + (*other_network)->getCfgOption()->createOptions(cfg_def); + // Add the new/updated nework. networks_.push_back(*other_network); } diff --git a/src/lib/dhcpsrv/cfg_shared_networks.h b/src/lib/dhcpsrv/cfg_shared_networks.h index 24a219d0ce..bd7516413f 100644 --- a/src/lib/dhcpsrv/cfg_shared_networks.h +++ b/src/lib/dhcpsrv/cfg_shared_networks.h @@ -140,6 +140,7 @@ public: /// configuration: /// - All of its associated subnets are moved to the "other" network. /// - The existing network is removed from this configuration. + /// - The "other" network's option instances are created. /// - The "other" network is added to this configuration. /// /// @warning The merge operation may affect the @c other configuration. @@ -148,9 +149,11 @@ public: /// not be modified after the call to @c merge because it may affect the /// merged configuration. /// + /// @param cfg_def set of of user-defined option definitions to use + /// when creating option instances. /// @param other the shared network configuration to be merged into this /// configuration. - void merge(CfgSharedNetworks4& other); + void merge(CfgOptionDefPtr cfg_def, CfgSharedNetworks4& other); }; /// @brief Pointer to the configuration of IPv4 shared networks. diff --git a/src/lib/dhcpsrv/cfg_subnets4.cc b/src/lib/dhcpsrv/cfg_subnets4.cc index 99e64893db..fd86f6dd76 100644 --- a/src/lib/dhcpsrv/cfg_subnets4.cc +++ b/src/lib/dhcpsrv/cfg_subnets4.cc @@ -56,7 +56,7 @@ CfgSubnets4::del(const ConstSubnet4Ptr& subnet) { } void -CfgSubnets4::merge(CfgSharedNetworks4Ptr networks, +CfgSubnets4::merge(CfgOptionDefPtr cfg_def, CfgSharedNetworks4Ptr networks, CfgSubnets4& other) { auto& index = subnets_.get(); @@ -94,6 +94,12 @@ CfgSubnets4::merge(CfgSharedNetworks4Ptr networks, index.erase(subnet_it); } + // Create the subnet's options based on the given definitions. + (*other_subnet)->getCfgOption()->createOptions(cfg_def); + for (auto pool : (*other_subnet)->getPoolsWritable(Lease::TYPE_V4)) { + pool->getCfgOption()->createOptions(cfg_def); + } + // Add the "other" subnet to the our collection of subnets. subnets_.push_back(*other_subnet); diff --git a/src/lib/dhcpsrv/cfg_subnets4.h b/src/lib/dhcpsrv/cfg_subnets4.h index 79fa3664d3..27890f6a28 100644 --- a/src/lib/dhcpsrv/cfg_subnets4.h +++ b/src/lib/dhcpsrv/cfg_subnets4.h @@ -70,6 +70,8 @@ public: /// -# If it belongs to a shared network, remove it from that network /// -# Remove the subnet from this configuration and discard it /// + /// - Create the subnet's option instance, as well as any options + /// that belong to any of the subnet's pools. /// - Add the subnet from @c other to this configuration. /// - If that subnet is associated to shared network, find that network /// in @ networks and add that subnet to it. @@ -80,12 +82,15 @@ public: /// not be modified after the call to @c merge because it may affect the /// merged configuration. /// + /// @param cfg_def set of of user-defined option definitions to use + /// when creating option instances. /// @param networks collection of shared networks that to which assignments /// should be added. In other words, the list of shared networks that belong /// to the same SrvConfig instance we are merging into. /// @param other the subnet configuration to be merged into this /// configuration. - void merge(CfgSharedNetworks4Ptr networks, CfgSubnets4& other); + void merge(CfgOptionDefPtr cfg_def, CfgSharedNetworks4Ptr networks, + CfgSubnets4& other); /// @brief Returns pointer to the collection of all IPv4 subnets. /// diff --git a/src/lib/dhcpsrv/srv_config.cc b/src/lib/dhcpsrv/srv_config.cc index 34274d1c7a..c5dafca98a 100644 --- a/src/lib/dhcpsrv/srv_config.cc +++ b/src/lib/dhcpsrv/srv_config.cc @@ -166,7 +166,7 @@ SrvConfig::merge(ConfigBase& other) { if (CfgMgr::instance().getFamily() == AF_INET) { merge4(other_srv_config); } else { - // @todo merge6(); + /// @todo merge6(); } } catch (const std::bad_cast&) { isc_throw(InvalidOperation, "internal server error: must use derivation" @@ -177,21 +177,26 @@ SrvConfig::merge(ConfigBase& other) { void SrvConfig::merge4(SrvConfig& other) { - /// We merge objects in order of dependency (real or theoretical). + // We merge objects in order of dependency (real or theoretical). - /// Merge globals. + // Merge globals. mergeGlobals4(other); - /// Merge option defs + // Merge option defs. We need to do this next so we + // pass these into subsequent merges so option instances + // at each level can be created based on the merged + // definitions. cfg_option_def_->merge((*other.getCfgOptionDef())); - /// @todo merge options + // Merge options. + cfg_option_->merge(cfg_option_def_, (*other.getCfgOption())); // Merge shared networks. - cfg_shared_networks4_->merge(*(other.getCfgSharedNetworks4())); + cfg_shared_networks4_->merge(cfg_option_def_, *(other.getCfgSharedNetworks4())); - /// Merge subnets. - cfg_subnets4_->merge(getCfgSharedNetworks4(), *(other.getCfgSubnets4())); + // Merge subnets. + cfg_subnets4_->merge(cfg_option_def_, getCfgSharedNetworks4(), + *(other.getCfgSubnets4())); /// @todo merge other parts of the configuration here. } @@ -454,7 +459,7 @@ SrvConfig::toElement() const { } // Set client-classes ConstElementPtr client_classes = class_dictionary_->toElement(); - // @todo accept empty list + /// @todo accept empty list if (!client_classes->empty()) { dhcp->set("client-classes", client_classes); } diff --git a/src/lib/dhcpsrv/subnet.h b/src/lib/dhcpsrv/subnet.h index 31d6680ca7..c2c16ff28e 100644 --- a/src/lib/dhcpsrv/subnet.h +++ b/src/lib/dhcpsrv/subnet.h @@ -289,7 +289,6 @@ public: shared_network_name_ = shared_network_name; } -protected: /// @brief Returns all pools (non-const variant) /// /// The reference is only valid as long as the object that returned it. @@ -298,6 +297,8 @@ protected: /// @return a collection of all pools PoolCollection& getPoolsWritable(Lease::Type type); +protected: + /// @brief Protected constructor // /// By making the constructor protected, we make sure that no one will diff --git a/src/lib/dhcpsrv/tests/cfg_option_unittest.cc b/src/lib/dhcpsrv/tests/cfg_option_unittest.cc index 7439f70c3a..21c6a0b995 100644 --- a/src/lib/dhcpsrv/tests/cfg_option_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_option_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2019 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -8,7 +8,10 @@ #include #include #include +#include #include +#include +#include #include #include #include @@ -20,6 +23,7 @@ #include using namespace isc; +using namespace isc::asiolink; using namespace isc::dhcp; namespace { @@ -213,8 +217,68 @@ TEST_F(CfgOptionTest, add) { EXPECT_TRUE(options->empty()); } -// This test verifies that two option configurations can be merged. -TEST_F(CfgOptionTest, merge) { +// This test verifies that options can be replaced with udpated content. +TEST_F(CfgOptionTest, replace) { + CfgOption cfg; + + // Let's add some options to the config to the config. + OptionStringPtr option(new OptionString(Option::V6, 1, "one")); + ASSERT_NO_THROW(cfg.add(option, false, "isc")); + + option.reset(new OptionString(Option::V6, 2, "two")); + ASSERT_NO_THROW(cfg.add(option, false, "isc")); + + option.reset(new OptionString(Option::V6, 3, "three")); + ASSERT_NO_THROW(cfg.add(option, false, "isc")); + + // Now let's make sure we can find them and they are as expected. + OptionDescriptor desc = cfg.get("isc", 1); + ASSERT_TRUE(desc.option_); + OptionStringPtr opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("one", opstr->getValue()); + + desc = cfg.get("isc", 2); + ASSERT_TRUE(desc.option_); + opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("two", opstr->getValue()); + + desc = cfg.get("isc", 3); + ASSERT_TRUE(desc.option_); + opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("three", opstr->getValue()); + + // Now let's replace one and three. + desc.option_.reset(new OptionString(Option::V6, 1, "new one")); + ASSERT_NO_THROW(cfg.replace(desc, "isc")); + + desc.option_.reset(new OptionString(Option::V6, 3, "new three")); + ASSERT_NO_THROW(cfg.replace(desc, "isc")); + + // Now let's make sure we can find them again and they are as expected. + desc = cfg.get("isc", 1); + ASSERT_TRUE(desc.option_); + opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("new one", opstr->getValue()); + + desc = cfg.get("isc", 2); + ASSERT_TRUE(desc.option_); + opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("two", opstr->getValue()); + + desc = cfg.get("isc", 3); + ASSERT_TRUE(desc.option_); + opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("new three", opstr->getValue()); +} + +// This test verifies that one configuration can be merged into another. +TEST_F(CfgOptionTest, mergeTo) { CfgOption cfg_src; CfgOption cfg_dst; @@ -327,6 +391,214 @@ TEST_F(CfgOptionTest, copy) { EXPECT_EQ(10, container->size()); } +// This test verifies that DHCP options from one configuration +// can be used to update options in another configuration. +// In other words, options from an "other" configuration +// can be merged into an existing configuration, with any +// duplicates in other overwriting those in the existing +// configuration. +TEST_F(CfgOptionTest, validMerge) { + CfgOption this_cfg; + CfgOption other_cfg; + + // We need to create a dictionary of defintions pass into option merge. + CfgOptionDefPtr defs(new CfgOptionDef()); + defs->add((OptionDefinitionPtr(new OptionDefinition("one", 1, "uint8"))), "isc"); + defs->add((OptionDefinitionPtr(new OptionDefinition("two", 2, "uint8"))), "isc"); + defs->add((OptionDefinitionPtr(new OptionDefinition("four", 4, "uint8"))), "isc"); + defs->add((OptionDefinitionPtr(new OptionDefinition("three", 3, "uint8"))), "fluff"); + defs->add((OptionDefinitionPtr(new OptionDefinition("four", 4, "uint8"))), "fluff"); + + // Create our existing config, that gets merged into. + OptionPtr option(new Option(Option::V4, 1, OptionBuffer(1, 0x01))); + ASSERT_NO_THROW(this_cfg.add(option, false, "isc")); + + // Add option 3 to "fluff" + option.reset(new Option(Option::V4, 3, OptionBuffer(1, 0x03))); + ASSERT_NO_THROW(this_cfg.add(option, false, "fluff")); + + // Add option 4 to "fluff". + option.reset(new Option(Option::V4, 4, OptionBuffer(1, 0x04))); + ASSERT_NO_THROW(this_cfg.add(option, false, "fluff")); + + // Create our other config that will be merged from. + // Add Option 1 to "isc", this should "overwrite" the original. + option.reset(new Option(Option::V4, 1, OptionBuffer(1, 0x10))); + ASSERT_NO_THROW(other_cfg.add(option, false, "isc")); + + // Add option 2 to "isc". + option.reset(new Option(Option::V4, 2, OptionBuffer(1, 0x20))); + ASSERT_NO_THROW(other_cfg.add(option, false, "isc")); + + // Add option 4 to "isc". + option.reset(new Option(Option::V4, 4, OptionBuffer(1, 0x40))); + ASSERT_NO_THROW(other_cfg.add(option, false, "isc")); + + // Merge source configuration to the destination configuration. The options + // in the destination should be preserved. The options from the source + // configuration should be added. + ASSERT_NO_THROW(this_cfg.merge(defs, other_cfg)); + + // isc:1 should come from "other" config. + OptionDescriptor desc = this_cfg.get("isc", 1); + ASSERT_TRUE(desc.option_); + OptionUint8Ptr opint = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opint); + EXPECT_EQ(16, opint->getValue()); + + // isc:2 should come from "other" config. + desc = this_cfg.get("isc", 2); + ASSERT_TRUE(desc.option_); + opint = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opint); + EXPECT_EQ(32, opint->getValue()); + + // isc:4 should come from "other" config. + desc = this_cfg.get("isc", 4); + ASSERT_TRUE(desc.option_); + opint = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opint); + EXPECT_EQ(64, opint->getValue()); + + // fluff:3 should come from "this" config. + desc = this_cfg.get("fluff", 3); + ASSERT_TRUE(desc.option_); + opint = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opint); + EXPECT_EQ(3, opint->getValue()); + + // fluff:4 should come from "this" config. + desc = this_cfg.get("fluff", 4); + ASSERT_TRUE(desc.option_); + opint = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opint); + EXPECT_EQ(4, opint->getValue()); +} + +// This test verifies that attempting to merge options +// which are by incompatible with "known" option definitions +// are detected. +TEST_F(CfgOptionTest, mergeInvalid) { + CfgOption this_cfg; + CfgOption other_cfg; + + // Create an empty dictionary of defintions pass into option merge. + CfgOptionDefPtr defs(new CfgOptionDef()); + + // Create our other config that will be merged from. + // Add an option without a formatted value. + OptionPtr option(new Option(Option::V4, 1, OptionBuffer(1, 0x01))); + ASSERT_NO_THROW(other_cfg.add(option, false, "isc")); + + // Add an option with a formatted value. + option.reset(new Option(Option::V4, 2)); + OptionDescriptor desc(option, false, "one,two,three"); + ASSERT_NO_THROW(other_cfg.add(desc, "isc")); + + // When we attempt to merge, it should fail, recognizing that + // option 2, which has a formatted value, has no definition. + try { + this_cfg.merge(defs, other_cfg); + GTEST_FAIL() << "merge should have thrown"; + } catch (const InvalidOperation& ex) { + std::string exp_msg = "option: isc.2 has a formatted value: " + "'one,two,three' but no option definition"; + EXPECT_EQ(std::string(exp_msg), std::string(ex.what())); + } catch (const std::exception& ex) { + GTEST_FAIL() << "wrong exception thrown:" << ex.what(); + } + + // Now let's add an option definition that will force data truncated + // error for option 1. + defs->add((OptionDefinitionPtr(new OptionDefinition("one", 1, "uint16"))), "isc"); + + // When we attempt to merge, it should fail because option 1's data + // is not valid per its definition. + try { + this_cfg.merge(defs, other_cfg); + GTEST_FAIL() << "merge should have thrown"; + } catch (const InvalidOperation& ex) { + std::string exp_msg = "could not create option: isc.1" + " from data specified, reason:" + " Option 1 truncated"; + EXPECT_EQ(std::string(exp_msg), std::string(ex.what())); + } catch (const std::exception& ex) { + GTEST_FAIL() << "wrong exception thrown:" << ex.what(); + } +} + +// This test verifies the all of the valid option cases +// in createDescriptorOption(): +// 1. standard option +// 2. vendor option +// 3. user-defined, unformatted option +// 4. user-defined, formatted option +// 5. undefined, unformatted option +TEST_F(CfgOptionTest, createDescriptorOptionValid) { + // First we'll create our "known" user definitions + CfgOptionDefPtr defs(new CfgOptionDef()); + defs->add((OptionDefinitionPtr(new OptionDefinition("one", 1, "uint8"))), "isc"); + defs->add((OptionDefinitionPtr(new OptionDefinition("two", 2, "uint8", true))), "isc"); + + // We'll try a standard option first. + std::string space = "dhcp4"; + std::string value("example.org"); + OptionPtr option(new Option(Option::V4, DHO_HOST_NAME)); + option->setData(value.begin(), value.end()); + OptionDescriptorPtr desc(new OptionDescriptor(option, false)); + + bool updated = false; + ASSERT_NO_THROW(updated = CfgOption::createDescriptorOption(defs, space, *desc)); + ASSERT_TRUE(updated); + OptionStringPtr opstr = boost::dynamic_pointer_cast(desc->option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("example.org", opstr->getValue()); + + // Next we'll try a vendor option with a formatted value + space = "vendor-4491"; + value = "192.0.2.1, 192.0.2.2"; + option.reset(new Option(Option::V4, 2)); + desc.reset(new OptionDescriptor(option, false, value)); + + ASSERT_NO_THROW(updated = CfgOption::createDescriptorOption(defs, space, *desc)); + ASSERT_TRUE(updated); + Option4AddrLstPtr opaddrs = boost::dynamic_pointer_cast(desc->option_); + ASSERT_TRUE(opaddrs); + Option4AddrLst::AddressContainer exp_addresses = { IOAddress("192.0.2.1"), IOAddress("192.0.2.2") }; + EXPECT_EQ(exp_addresses, opaddrs->getAddresses()); + + // Now, a user defined uint8 option + space = "isc"; + option.reset(new Option(Option::V4, 1, OptionBuffer(1, 0x77))); + desc.reset(new OptionDescriptor(option, false)); + + ASSERT_NO_THROW(updated = CfgOption::createDescriptorOption(defs, space, *desc)); + ASSERT_TRUE(updated); + OptionUint8Ptr opint = boost::dynamic_pointer_cast(desc->option_); + ASSERT_TRUE(opint); + EXPECT_EQ(119, opint->getValue()); + + // Now, a user defined array of ints from a formatted value + option.reset(new Option(Option::V4, 2)); + desc.reset(new OptionDescriptor(option, false, "1,2,3")); + + ASSERT_NO_THROW(updated = CfgOption::createDescriptorOption(defs, space, *desc)); + ASSERT_TRUE(updated); + OptionUint8ArrayPtr oparray = boost::dynamic_pointer_cast(desc->option_); + ASSERT_TRUE(oparray); + std::vector exp_ints = { 1, 2, 3 }; + EXPECT_EQ(exp_ints, oparray->getValues()); + + // Finally, a generic, undefined option + option.reset(new Option(Option::V4, 199, OptionBuffer(1, 0x77))); + desc.reset(new OptionDescriptor(option, false)); + + ASSERT_NO_THROW(updated = CfgOption::createDescriptorOption(defs, space, *desc)); + ASSERT_FALSE(updated); + ASSERT_EQ(1, desc->option_->getData().size()); + EXPECT_EQ(0x77, desc->option_->getData()[0]); +} + // This test verifies that encapsulated options are added as sub-options // to the top level options on request. TEST_F(CfgOptionTest, encapsulate) { diff --git a/src/lib/dhcpsrv/tests/cfg_shared_networks4_unittest.cc b/src/lib/dhcpsrv/tests/cfg_shared_networks4_unittest.cc index c8e80d7bec..76991a709c 100644 --- a/src/lib/dhcpsrv/tests/cfg_shared_networks4_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_shared_networks4_unittest.cc @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -168,6 +169,11 @@ TEST(CfgSharedNetworks4Test, unparse) { // This test verifies that shared-network configurations are properly merged. TEST(CfgSharedNetworks4Test, mergeNetworks) { + // Create custom options dictionary for testing merge. We're keeping it + // simple because they are more rigorous tests elsewhere. + CfgOptionDefPtr cfg_def(new CfgOptionDef()); + cfg_def->add((OptionDefinitionPtr(new OptionDefinition("one", 1, "string"))), "isc"); + Subnet4Ptr subnet1(new Subnet4(IOAddress("192.0.1.0"), 26, 1, 2, 100, SubnetID(1))); Subnet4Ptr subnet2(new Subnet4(IOAddress("192.0.2.0"), @@ -201,12 +207,11 @@ TEST(CfgSharedNetworks4Test, mergeNetworks) { // Merge in an "empty" config. Should have the original config, still intact. CfgSharedNetworks4 cfg_from; - ASSERT_NO_THROW(cfg_to.merge(cfg_from)); + ASSERT_NO_THROW(cfg_to.merge(cfg_def, cfg_from)); ASSERT_EQ(3, cfg_to.getAll()->size()); ASSERT_NO_FATAL_FAILURE(checkMergedNetwork(cfg_to, "network1", Triplet(100), std::vector{SubnetID(1), SubnetID(2)})); - ASSERT_NO_FATAL_FAILURE(checkMergedNetwork(cfg_to, "network2", Triplet(200), std::vector())); @@ -217,6 +222,15 @@ TEST(CfgSharedNetworks4Test, mergeNetworks) { // We'll double the valid time and add subnet4 to it SharedNetwork4Ptr network1b(new SharedNetwork4("network1")); network1b->setValid(Triplet(200)); + + // Now let's a add generic option 1 to network1b. + std::string value("Yay!"); + OptionPtr option(new Option(Option::V4, 1)); + option->setData(value.begin(), value.end()); + ASSERT_NO_THROW(network1b->getCfgOption()->add(option, false, "isc")); + // Verify that our option is a generic option. + EXPECT_EQ("type=001, len=004: 59:61:79:21", option->toText()); + ASSERT_NO_THROW(network1b->add(subnet4)); // Network2 we will not touch. @@ -230,7 +244,7 @@ TEST(CfgSharedNetworks4Test, mergeNetworks) { ASSERT_NO_THROW(cfg_from.add(network1b)); ASSERT_NO_THROW(cfg_from.add(network3b)); - ASSERT_NO_THROW(cfg_to.merge(cfg_from)); + ASSERT_NO_THROW(cfg_to.merge(cfg_def, cfg_from)); // Should still have 3 networks. @@ -241,6 +255,12 @@ TEST(CfgSharedNetworks4Test, mergeNetworks) { ASSERT_NO_FATAL_FAILURE(checkMergedNetwork(cfg_to, "network1", Triplet(200), std::vector{SubnetID(1), SubnetID(2)})); + // Make sure we have option 1 and that it has been replaced with a string option. + auto network = cfg_to.getByName("network1"); + auto desc = network->getCfgOption()->get("isc", 1); + ASSERT_TRUE(desc.option_); + EXPECT_EQ("type=001, len=004: \"Yay!\" (string)", desc.option_->toText()); + // No changes to network2. ASSERT_NO_FATAL_FAILURE(checkMergedNetwork(cfg_to, "network2", Triplet(200), std::vector())); diff --git a/src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc b/src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc index c5206458ae..f883e0a9a0 100644 --- a/src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -38,11 +39,11 @@ void checkMergedSubnet(CfgSubnets4& cfg_subnets, const std::string& prefix, int exp_valid, SharedNetwork4Ptr exp_network) { - - // The subnet1 should be replaced by subnet4 but the shared network - // should not be affected. + // Look for the network by prefix. auto subnet = cfg_subnets.getByPrefix(prefix); ASSERT_TRUE(subnet) << "subnet: " << prefix << " not found"; + + // Make sure we have the one we expect. ASSERT_EQ(exp_subnet_id, subnet->getID()) << "subnet ID is wrong"; ASSERT_EQ(exp_valid, subnet->getValid()) << "subnet valid time is wrong"; @@ -139,6 +140,11 @@ TEST(CfgSubnets4Test, deleteSubnet) { // This test verifies that subnets configuration is properly merged. TEST(CfgSubnets4Test, mergeSubnets) { + // Create custom options dictionary for testing merge. We're keeping it + // simple because they are more rigorous tests elsewhere. + CfgOptionDefPtr cfg_def(new CfgOptionDef()); + cfg_def->add((OptionDefinitionPtr(new OptionDefinition("one", 1, "string"))), "isc"); + Subnet4Ptr subnet1(new Subnet4(IOAddress("192.0.1.0"), 26, 1, 2, 100, SubnetID(1))); Subnet4Ptr subnet2(new Subnet4(IOAddress("192.0.2.0"), @@ -148,7 +154,6 @@ TEST(CfgSubnets4Test, mergeSubnets) { Subnet4Ptr subnet4(new Subnet4(IOAddress("192.0.4.0"), 26, 1, 2, 100, SubnetID(4))); - // Create the "existing" list of shared networks CfgSharedNetworks4Ptr networks(new CfgSharedNetworks4()); SharedNetwork4Ptr shared_network1(new SharedNetwork4("shared-network1")); @@ -171,16 +176,14 @@ TEST(CfgSubnets4Test, mergeSubnets) { ASSERT_NO_THROW(cfg_to.add(subnet3)); ASSERT_NO_THROW(cfg_to.add(subnet4)); - // Merge in an "empty" config. Should have the original config, // still intact. CfgSubnets4 cfg_from; - ASSERT_NO_THROW(cfg_to.merge(networks, cfg_from)); + ASSERT_NO_THROW(cfg_to.merge(cfg_def, networks, cfg_from)); // We should have all four subnets, with no changes. ASSERT_EQ(4, cfg_to.getAll()->size()); - // Should be no changes to the configuration. ASSERT_NO_FATAL_FAILURE(checkMergedSubnet(cfg_to, SubnetID(1), "192.0.1.0/26", 100, shared_network1)); @@ -197,20 +200,49 @@ TEST(CfgSubnets4Test, mergeSubnets) { 26, 2, 3, 400, SubnetID(1))); subnet1b->setSharedNetworkName("shared-network1"); + // Add generic option 1 to subnet 1b. + std::string value("Yay!"); + OptionPtr option(new Option(Option::V4, 1)); + option->setData(value.begin(), value.end()); + ASSERT_NO_THROW(subnet1b->getCfgOption()->add(option, false, "isc")); + // subnet 3b updates subnet 3 and removes it from network 2 Subnet4Ptr subnet3b(new Subnet4(IOAddress("192.0.3.0"), 26, 3, 4, 500, SubnetID(3))); + // Now Add generic option 1 to subnet 3b. + value = "Team!"; + option.reset(new Option(Option::V4, 1)); + option->setData(value.begin(), value.end()); + ASSERT_NO_THROW(subnet3b->getCfgOption()->add(option, false, "isc")); + // subnet 4b updates subnet 4 and moves it from network2 to network 1 Subnet4Ptr subnet4b(new Subnet4(IOAddress("192.0.4.0"), 26, 3, 4, 500, SubnetID(4))); subnet4b->setSharedNetworkName("shared-network1"); // subnet 5 is new and belongs to network 2 + // Has two pools both with an option 1 Subnet4Ptr subnet5(new Subnet4(IOAddress("192.0.5.0"), 26, 1, 2, 300, SubnetID(5))); subnet5->setSharedNetworkName("shared-network2"); + // Add pool 1 + Pool4Ptr pool(new Pool4(IOAddress("192.0.5.10"), IOAddress("192.0.5.20"))); + value = "POOLS"; + option.reset(new Option(Option::V4, 1)); + option->setData(value.begin(), value.end()); + ASSERT_NO_THROW(pool->getCfgOption()->add(option, false, "isc")); + subnet5->addPool(pool); + + // Add pool 2 + pool.reset(new Pool4(IOAddress("192.0.5.30"), IOAddress("192.0.5.40"))); + value ="RULE!"; + option.reset(new Option(Option::V4, 1)); + option->setData(value.begin(), value.end()); + ASSERT_NO_THROW(pool->getCfgOption()->add(option, false, "isc")); + subnet5->addPool(pool); + // Add subnets to the merge from config. ASSERT_NO_THROW(cfg_from.add(subnet1b)); ASSERT_NO_THROW(cfg_from.add(subnet3b)); @@ -218,13 +250,21 @@ TEST(CfgSubnets4Test, mergeSubnets) { ASSERT_NO_THROW(cfg_from.add(subnet5)); // Merge again. - ASSERT_NO_THROW(cfg_to.merge(networks, cfg_from)); + ASSERT_NO_THROW(cfg_to.merge(cfg_def, networks, cfg_from)); ASSERT_EQ(5, cfg_to.getAll()->size()); // The subnet1 should be replaced by subnet1b. ASSERT_NO_FATAL_FAILURE(checkMergedSubnet(cfg_to, SubnetID(1), "192.0.1.0/26", 400, shared_network1)); + // Let's verify that our option is there and populated correctly. + auto subnet = cfg_to.getByPrefix("192.0.1.0/26"); + auto desc = subnet->getCfgOption()->get("isc", 1); + ASSERT_TRUE(desc.option_); + OptionStringPtr opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("Yay!", opstr->getValue()); + // The subnet2 should not be affected because it was not present. ASSERT_NO_FATAL_FAILURE(checkMergedSubnet(cfg_to, SubnetID(2), "192.0.2.0/26", 100, shared_network2)); @@ -232,6 +272,13 @@ TEST(CfgSubnets4Test, mergeSubnets) { // subnet3 should be replaced by subnet3b and no longer assigned to a network. ASSERT_NO_FATAL_FAILURE(checkMergedSubnet(cfg_to, SubnetID(3), "192.0.3.0/26", 500, no_network)); + // Let's verify that our option is there and populated correctly. + subnet = cfg_to.getByPrefix("192.0.3.0/26"); + desc = subnet->getCfgOption()->get("isc", 1); + ASSERT_TRUE(desc.option_); + opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("Team!", opstr->getValue()); // subnet4 should be replaced by subnet4b and moved to network1. ASSERT_NO_FATAL_FAILURE(checkMergedSubnet(cfg_to, SubnetID(4), @@ -240,6 +287,22 @@ TEST(CfgSubnets4Test, mergeSubnets) { // subnet5 should have been added to configuration. ASSERT_NO_FATAL_FAILURE(checkMergedSubnet(cfg_to, SubnetID(5), "192.0.5.0/26", 300, shared_network2)); + + // Let's verify that both pools have the proper options. + subnet = cfg_to.getByPrefix("192.0.5.0/26"); + const PoolPtr merged_pool = subnet->getPool(Lease::TYPE_V4, IOAddress("192.0.5.10")); + ASSERT_TRUE(merged_pool); + desc = merged_pool->getCfgOption()->get("isc", 1); + opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("POOLS", opstr->getValue()); + + const PoolPtr merged_pool2 = subnet->getPool(Lease::TYPE_V4, IOAddress("192.0.5.30")); + ASSERT_TRUE(merged_pool2); + desc = merged_pool2->getCfgOption()->get("isc", 1); + opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("RULE!", opstr->getValue()); } // This test verifies that it is possible to retrieve a subnet using an