From: Marcin Siodelski Date: Thu, 10 Jan 2013 20:22:44 +0000 (+0100) Subject: [2317] Specify option space names for DHCPv6 options. X-Git-Tag: bind10-1.0.0-rc-release~100^2~2^2~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6e2a0ba86f0607f3e9822a178a223e8a922de47a;p=thirdparty%2Fkea.git [2317] Specify option space names for DHCPv6 options. --- diff --git a/src/bin/dhcp4/config_parser.cc b/src/bin/dhcp4/config_parser.cc index e3ed19b410..82cdc95228 100644 --- a/src/bin/dhcp4/config_parser.cc +++ b/src/bin/dhcp4/config_parser.cc @@ -593,12 +593,6 @@ private: /// and data carried by the option. If parsing is successful then an /// instance of an option is created and added to the storage provided /// by the calling class. -/// -/// @todo This class parses and validates the option name. However it is -/// not used anywhere until support for option spaces is implemented -/// (see tickets #2319, #2314). When option spaces are implemented -/// there will be a way to reference the particular option using -/// its type (code) or option name. class OptionDataParser : public DhcpConfigParser { public: @@ -626,7 +620,6 @@ public: /// the configuration. /// @throw isc::InvalidOperation if failed to set storage prior to /// calling build. - /// @throw isc::BadValue if option data storage is invalid. virtual void build(ConstElementPtr option_data_entries) { if (options_ == NULL) { isc_throw(isc::InvalidOperation, "Parser logic error: storage must be set before " @@ -1019,7 +1012,7 @@ public: } else { isc_throw(DhcpConfigError, "invalid parameter: " << entry); } - + parser->build(param.second); parser->commit(); } @@ -1119,7 +1112,7 @@ private: /// Instance of option definition being created by this parser. OptionDefinitionPtr option_definition_; - + /// Name of the space the option definition belongs to. std::string option_space_name_; /// Pointer to a storage where the option definition will be @@ -1178,7 +1171,7 @@ public: } } - /// @brief Stores option definitions in the provided storage. + /// @brief Stores option definitions in the CfgMgr. void commit() { CfgMgr& cfg_mgr = CfgMgr::instance(); @@ -1207,11 +1200,6 @@ public: return (new OptionDefListParser(param_name)); } -private: - - /// Temporary storage for option definitions. It holds option - /// definitions before \ref commit is called. - // OptionDefStorage option_defs_local_; }; /// @brief this class parses a single subnet @@ -1375,10 +1363,6 @@ private: subnet_->addPool4(*it); } - // We have to get all option space names for options we are - // configuring and iterate over them to add options that belong - // to them to the subnet. - // We are going to move configured options to the Subnet object. // Configured options reside in the container where options // are grouped by space names. Thus we need to get all space names @@ -1452,13 +1436,13 @@ private: // return new DebugParser(config_id); isc_throw(NotImplemented, - "Parser error: Subnet4 parameter not supported: " + "parser error: Subnet4 parameter not supported: " << config_id); } return (f->second(config_id)); } - /// @brief returns value for a given parameter (after using inheritance) + /// @brief Returns value for a given parameter (after using inheritance) /// /// This method implements inheritance. For a given parameter name, it first /// checks if there is a global value for it and overwrites it with specific diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index cb7340ab7e..ba14edfb54 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -523,7 +523,7 @@ TEST_F(Dhcp4ParserTest, optionDefRecord) { EXPECT_EQ(100, def->getCode()); EXPECT_EQ(OPT_RECORD_TYPE, def->getType()); EXPECT_FALSE(def->getArrayType()); - + // The option comprises the record of data fields. Verify that all // fields are present and they are of the expected types. const OptionDefinition::RecordFieldsCollection& record_fields = @@ -916,7 +916,7 @@ TEST_F(Dhcp4ParserTest, optionDataTwoSpaces) { "}"; ConstElementPtr status; - + ElementPtr json = Element::fromJSON(config); EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); diff --git a/src/bin/dhcp6/config_parser.cc b/src/bin/dhcp6/config_parser.cc index f615355987..2ec932053d 100644 --- a/src/bin/dhcp6/config_parser.cc +++ b/src/bin/dhcp6/config_parser.cc @@ -86,12 +86,10 @@ typedef OptionSpaceContainer PoolStorage; -/// @brief Collection of option descriptors. -/// -/// This container allows to search options using an option code -/// or a persistency flag. This is useful when merging existing -/// options with newly configured options. -typedef isc::dhcp::Subnet::OptionContainer OptionStorage; +/// Collection of containers holding option spaces. Each container within +/// a particular option space holds so-called option descriptors. +typedef OptionSpaceContainer OptionStorage; /// @brief Global uint32 parameters that will be used as defaults. Uint32Storage uint32_defaults; @@ -102,6 +100,10 @@ StringStorage string_defaults; /// @brief Global storage for options that will be used as defaults. OptionStorage option_defaults; +/// @brief Global storage for option definitions. +OptionDefStorage option_def_defaults; + + /// @brief a dummy configuration parser /// /// This is a debugging parser. It does not configure anything, @@ -619,12 +621,6 @@ private: /// and data carried by the option. If parsing is successful then an /// instance of an option is created and added to the storage provided /// by the calling class. -/// -/// @todo This class parses and validates the option name. However it is -/// not used anywhere until support for option spaces is implemented -/// (see tickets #2319, #2314). When option spaces are implemented -/// there will be a way to reference the particular option using -/// its type (code) or option name. class OptionDataParser : public DhcpConfigParser { public: @@ -652,7 +648,6 @@ public: /// the configuration. /// @throw isc::InvalidOperation if failed to set storage prior to /// calling build. - /// @throw isc::BadValue if option data storage is invalid. virtual void build(ConstElementPtr option_data_entries) { if (options_ == NULL) { @@ -661,7 +656,8 @@ public: } BOOST_FOREACH(ConfigPair param, option_data_entries->mapValue()) { ParserPtr parser; - if (param.first == "name") { + if (param.first == "name" || param.first == "data" || + param.first == "space") { boost::shared_ptr name_parser(dynamic_cast(StringParser::factory(param.first))); if (name_parser) { @@ -675,13 +671,6 @@ public: code_parser->setStorage(&uint32_values_); parser = code_parser; } - } else if (param.first == "data") { - boost::shared_ptr - value_parser(dynamic_cast(StringParser::factory(param.first))); - if (value_parser) { - value_parser->setStorage(&string_values_); - parser = value_parser; - } } else if (param.first == "csv-format") { boost::shared_ptr value_parser(dynamic_cast(BooleanParser::factory(param.first))); @@ -726,17 +715,19 @@ public: " thus there is nothing to commit. Has build() been called?"); } uint16_t opt_type = option_descriptor_.option->getType(); - isc::dhcp::Subnet::OptionContainerTypeIndex& idx = options_->get<1>(); + Subnet::OptionContainerPtr options = options_->getItems(option_space_); + assert(options); + Subnet::OptionContainerTypeIndex& idx = options->get<1>(); // Try to find options with the particular option code in the main // storage. If found, remove these options because they will be // replaced with new one. - isc::dhcp::Subnet::OptionContainerTypeRange range = + Subnet::OptionContainerTypeRange range = idx.equal_range(opt_type); if (std::distance(range.first, range.second) > 0) { idx.erase(range.first, range.second); } // Append new option to the main storage. - options_->push_back(option_descriptor_); + options_->addItem(option_descriptor_, option_space_); } /// @brief Set storage for the parser. @@ -792,11 +783,46 @@ private: << " spaces"); } + std::string option_space = getParam("space", string_values_); + /// @todo Validate option space once #2313 is merged. + + OptionDefinitionPtr def; + if (option_space == "dhcp6" && + LibDHCP::isStandardOption(Option::V6, option_code)) { + def = LibDHCP::getOptionDef(Option::V6, option_code); + + } else if (option_space == "dhcp4") { + isc_throw(DhcpConfigError, "'dhcp4' option space name is reserved" + << " for DHCPv4 server"); + } else { + // If we are not dealing with a standard option then we + // need to search for its definition among user-configured + // options. They are expected to be in the global storage + // already. + OptionDefContainerPtr defs = option_def_defaults.getItems(option_space); + assert(defs); + const OptionDefContainerTypeIndex& idx = defs->get<1>(); + OptionDefContainerTypeRange range = idx.equal_range(option_code); + if (std::distance(range.first, range.second) > 0) { + def = *range.first; + } + if (!def) { + isc_throw(DhcpConfigError, "definition for the option '" + << option_space << "." << option_name + << "' having code '" << option_code + << "' does not exist"); + } + + } + // Get option data from the configuration database ('data' field). const std::string option_data = getParam("data", string_values_); const bool csv_format = getParam("csv-format", boolean_values_); + + // Transform string of hexadecimal digits into binary format. std::vector binary; std::vector data_tokens; + if (csv_format) { // If the option data is specified as a string of comma // separated values then we need to split this string into @@ -807,36 +833,22 @@ private: // Otherwise, the option data is specified as a string of // hexadecimal digits that we have to turn into binary format. try { - isc::util::encode::decodeHex(option_data, binary); + util::encode::decodeHex(option_data, binary); } catch (...) { isc_throw(DhcpConfigError, "Parser error: option data is not a valid" << " string of hexadecimal digits: " << option_data); } } - // Get all existing DHCPv6 option definitions. The one that matches - // our option will be picked and used to create it. - OptionDefContainer option_defs = LibDHCP::getOptionDefs(Option::V6); - // Get search index #1. It allows searching for options definitions - // using option type value. - const OptionDefContainerTypeIndex& idx = option_defs.get<1>(); - // Get all option definitions matching option code we want to create. - const OptionDefContainerTypeRange& range = idx.equal_range(option_code); - size_t num_defs = std::distance(range.first, range.second); + OptionPtr option; - // Currently we do not allow duplicated definitions and if there are - // any duplicates we issue internal server error. - if (num_defs > 1) { - isc_throw(DhcpConfigError, "Internal error: currently it is not" - << " supported to initialize multiple option definitions" - << " for the same option code. This will be supported once" - << " there option spaces are implemented."); - } else if (num_defs == 0) { + if (!def) { if (csv_format) { isc_throw(DhcpConfigError, "the CSV option data format can be" " used to specify values for an option that has a" " definition. The option with code " << option_code << " does not have a definition."); } + // @todo We have a limited set of option definitions intiialized at the moment. // In the future we want to initialize option definitions for all options. // Consequently an error will be issued if an option definition does not exist @@ -850,9 +862,21 @@ private: option_descriptor_.option = option; option_descriptor_.persistent = false; } else { - // We have exactly one option definition for the particular option code - // use it to create the option instance. - const OptionDefinitionPtr& def = *(range.first); + + // Option name should match the definition. The option name + // may seem to be redundant but in the future we may want + // to reference options and definitions using their names + // and/or option codes so keeping the option name in the + // definition of option value makes sense. + if (def->getName() != option_name) { + isc_throw(DhcpConfigError, "specified option name '" + << option_name << " does not match the " + << "option definition: '" << option_space + << "." << def->getName() << "'"); + } + + // Option definition has been found so let's use it to create + // an instance of our option. try { OptionPtr option = csv_format ? def->optionFactory(Option::V6, option_code, data_tokens) : @@ -861,11 +885,14 @@ private: option_descriptor_.option = option; option_descriptor_.persistent = false; } catch (const isc::Exception& ex) { - isc_throw(DhcpConfigError, "Parser error: option data does not match" - << " option definition (code " << option_code << "): " + isc_throw(DhcpConfigError, "option data does not match" + << " option definition (space: " << option_space + << ", code: " << option_code << "): " << ex.what()); } } + // All went good, so we can set the option space name. + option_space_ = option_space; } /// Storage for uint32 values (e.g. option code). @@ -879,6 +906,8 @@ private: OptionStorage* options_; /// Option descriptor holds newly configured option. isc::dhcp::Subnet::OptionDescriptor option_descriptor_; + /// Option space name where the option belongs to. + std::string option_space_; }; /// @brief Parser for option data values within a subnet. @@ -1013,7 +1042,7 @@ public: } else { isc_throw(DhcpConfigError, "invalid parameter: " << entry); } - + parser->build(param.second); parser->commit(); } @@ -1113,7 +1142,7 @@ private: /// Instance of option definition being created by this parser. OptionDefinitionPtr option_definition_; - + /// Name of the space the option definition belongs to. std::string option_space_name_; /// Pointer to a storage where the option definition will be @@ -1154,6 +1183,10 @@ public: /// that define option definitions. /// @throw DhcpConfigError if configuration parsing fails. void build(ConstElementPtr option_def_list) { + // Clear existing items in the global storage. + // We are going to replace all of them. + option_def_defaults.clearItems(); + if (!option_def_list) { isc_throw(DhcpConfigError, "parser error: a pointer to a list of" << " option definitions is NULL"); @@ -1162,13 +1195,13 @@ public: BOOST_FOREACH(ConstElementPtr option_def, option_def_list->listValue()) { boost::shared_ptr parser(new OptionDefParser("single-option-def")); - parser->setStorage(&option_defs_local_); + parser->setStorage(&option_def_defaults); parser->build(option_def); parser->commit(); } } - /// @brief Stores option definitions in the provided storage. + /// @brief Stores option definitions in the CfgMgr. void commit() { CfgMgr& cfg_mgr = CfgMgr::instance(); @@ -1178,10 +1211,10 @@ public: // We need to move option definitions from the temporary // storage to the global storage. BOOST_FOREACH(std::string space_name, - option_defs_local_.getOptionSpaceNames()) { + option_def_defaults.getOptionSpaceNames()) { BOOST_FOREACH(OptionDefinitionPtr def, - *option_defs_local_.getItems(space_name)) { + *option_def_defaults.getItems(space_name)) { assert(def); cfg_mgr.addOptionDef(def, space_name); } @@ -1197,11 +1230,6 @@ public: return (new OptionDefListParser(param_name)); } -private: - - /// Temporary storage for option definitions. It holds option - /// definitions before \ref commit is called. - OptionDefStorage option_defs_local_; }; /// @brief this class parses a single subnet @@ -1314,7 +1342,7 @@ private: /// /// @throw isc::dhcp::DhcpConfigError if subnet configuration parsing failed. void createSubnet() { - + // Find a subnet string. StringStorage::const_iterator it = string_values_.find("subnet"); if (it == string_values_.end()) { @@ -1367,39 +1395,52 @@ private: subnet_->addPool6(*it); } - Subnet::OptionContainerPtr options = subnet_->getOptionDescriptors("dhcp6"); - const Subnet::OptionContainerTypeIndex& idx = options->get<1>(); - - // Add subnet specific options. - BOOST_FOREACH(Subnet::OptionDescriptor desc, options_) { - Subnet::OptionContainerTypeRange range = idx.equal_range(desc.option->getType()); - if (std::distance(range.first, range.second) > 0) { - LOG_WARN(dhcp6_logger, DHCP6_CONFIG_OPTION_DUPLICATE) - .arg(desc.option->getType()).arg(addr.toText()); + // We are going to move configured options to the Subnet object. + // Configured options reside in the container where options + // are grouped by space names. Thus we need to get all space names + // and iterate over all options that belong to them. + BOOST_FOREACH(std::string option_space, options_.getOptionSpaceNames()) { + // Get all options within a particular option space. + BOOST_FOREACH(Subnet::OptionDescriptor desc, + *options_.getItems(option_space)) { + // In theory, option should be non-NULL. + assert(desc.option); + // We want to check whether an option with the particular + // option code has been already added. If so, we want + // to issue a warning. + Subnet::OptionDescriptor existing_desc = + subnet_->getOptionDescriptor("option_space", + desc.option->getType()); + if (existing_desc.option) { + LOG_WARN(dhcp6_logger, DHCP6_CONFIG_OPTION_DUPLICATE) + .arg(desc.option->getType()).arg(addr.toText()); + } + // In any case, we add the option to the subnet. + subnet_->addOption(desc.option, false, option_space); } - subnet_->addOption(desc.option, false, "dhcp6"); } // Check all global options and add them to the subnet object if // they have been configured in the global scope. If they have been // configured in the subnet scope we don't add global option because // the one configured in the subnet scope always takes precedence. - BOOST_FOREACH(Subnet::OptionDescriptor desc, option_defaults) { - // Get all options specified locally in the subnet and having - // code equal to global option's code. - Subnet::OptionContainerPtr options = subnet_->getOptionDescriptors("dhcp6"); - const Subnet::OptionContainerTypeIndex& idx = options->get<1>(); - Subnet::OptionContainerTypeRange range = idx.equal_range(desc.option->getType()); - // @todo: In the future we will be searching for options using either - // an option code or namespace. Currently we have only the option - // code available so if there is at least one option found with the - // specific code we don't add the globally configured option. - // @todo with this code the first globally configured option - // with the given code will be added to a subnet. We may - // want to issue a warning about dropping the configuration of - // a global option if one already exsists. - if (std::distance(range.first, range.second) == 0) { - subnet_->addOption(desc.option, false, "dhcp6"); + BOOST_FOREACH(std::string option_space, + option_defaults.getOptionSpaceNames()) { + // Get all global options for the particular option space. + BOOST_FOREACH(Subnet::OptionDescriptor desc, + *option_defaults.getItems(option_space)) { + assert(desc.option); + // Check if the particular option has been already added. + // This would mean that it has been configured in the + // subnet scope. Since option values configured in the + // subnet scope take precedence over globally configured + // values we don't add option from the global storage + // if there is one already. + Subnet::OptionDescriptor existing_desc = + subnet_->getOptionDescriptor(option_space, desc.option->getType()); + if (!existing_desc.option) { + subnet_->addOption(desc.option, false, option_space); + } } } } @@ -1600,18 +1641,16 @@ configureDhcp6Server(Dhcpv6Srv& , ConstElementPtr config_set) { LOG_DEBUG(dhcp6_logger, DBG_DHCP6_COMMAND, DHCP6_CONFIG_START).arg(config_set->str()); // Some of the values specified in the configuration depend on - // other values. Typically, the values in the subnet6 structure - // depend on the global values. Thus we need to make sure that - // the global values are processed first and that they can be - // accessed by the subnet6 parsers. We separate parsers that - // should process data first (independent_parsers) from those - // that must process data when the independent data is already - // processed (dependent_parsers). + // other values. Typically, the values in the subnet4 structure + // depend on the global values. Also, option values configuration + // must be performed after the option definitions configurations. + // Thus we group parsers and will fire them in the right order. ParserCollection independent_parsers; - ParserCollection dependent_parsers; + ParserPtr subnet_parser; + ParserPtr option_parser; // The subnet parsers implement data inheritance by directly - // accessing global storages. For this reason the global data + // accessing global storage. For this reason the global data // parsers must store the parsed data into global storages // immediately. This may cause data inconsistency if the // parsing operation fails after the global storage has been @@ -1620,6 +1659,7 @@ configureDhcp6Server(Dhcpv6Srv& , ConstElementPtr config_set) { Uint32Storage uint32_local(uint32_defaults); StringStorage string_local(string_defaults); OptionStorage option_local(option_defaults); + OptionDefStorage option_def_local(option_def_defaults); // answer will hold the result. ConstElementPtr answer; @@ -1627,11 +1667,21 @@ configureDhcp6Server(Dhcpv6Srv& , ConstElementPtr config_set) { // have to be restored to global storages. bool rollback = false; try { - // Iterate over all independent parsers first (all but subnet6) - // and try to parse the data. - BOOST_FOREACH(ConfigPair config_pair, config_set->mapValue()) { - if (config_pair.first != "subnet6") { - ParserPtr parser(createGlobalDhcpConfigParser(config_pair.first)); + + // Make parsers grouping. + const std::map& values_map = + config_set->mapValue(); + BOOST_FOREACH(ConfigPair config_pair, values_map) { + ParserPtr parser(createGlobalDhcpConfigParser(config_pair.first)); + if (config_pair.first == "subnet6") { + subnet_parser = parser; + + } else if (config_pair.first == "option-data") { + option_parser = parser; + + } else { + // Those parsers should be started before other + // parsers so we can call build straight away. independent_parsers.push_back(parser); parser->build(config_pair.second); // The commit operation here may modify the global storage @@ -1641,13 +1691,19 @@ configureDhcp6Server(Dhcpv6Srv& , ConstElementPtr config_set) { } } - // Process dependent configuration data. - BOOST_FOREACH(ConfigPair config_pair, config_set->mapValue()) { - if (config_pair.first == "subnet6") { - ParserPtr parser(createGlobalDhcpConfigParser(config_pair.first)); - dependent_parsers.push_back(parser); - parser->build(config_pair.second); - } + // The option values parser is the next one to be run. + std::map::const_iterator option_config = + values_map.find("option-data"); + if (option_config != values_map.end()) { + option_parser->build(option_config->second); + option_parser->commit(); + } + + // The subnet parser is the last one to be run. + std::map::const_iterator subnet_config = + values_map.find("subnet6"); + if (subnet_config != values_map.end()) { + subnet_parser->build(subnet_config->second); } } catch (const isc::Exception& ex) { @@ -1670,8 +1726,8 @@ configureDhcp6Server(Dhcpv6Srv& , ConstElementPtr config_set) { // This operation should be exception safe but let's make sure. if (!rollback) { try { - BOOST_FOREACH(ParserPtr parser, dependent_parsers) { - parser->commit(); + if (subnet_parser) { + subnet_parser->commit(); } } catch (const isc::Exception& ex) { @@ -1694,6 +1750,7 @@ configureDhcp6Server(Dhcpv6Srv& , ConstElementPtr config_set) { std::swap(uint32_defaults, uint32_local); std::swap(string_defaults, string_local); std::swap(option_defaults, option_local); + std::swap(option_def_defaults, option_def_local); return (answer); } diff --git a/src/bin/dhcp6/dhcp6.spec b/src/bin/dhcp6/dhcp6.spec index 71ca8c5914..b719ebb4ef 100644 --- a/src/bin/dhcp6/dhcp6.spec +++ b/src/bin/dhcp6/dhcp6.spec @@ -122,6 +122,11 @@ "item_type": "boolean", "item_optional": false, "item_default": False + }, + { "item_name": "space", + "item_type": "string", + "item_optional": false, + "item_default": "dhcp6" } ] } }, @@ -212,6 +217,11 @@ "item_type": "boolean", "item_optional": false, "item_default": False + }, + { "item_name": "space", + "item_type": "string", + "item_optional": false, + "item_default": "dhcp6" } ] } } ] diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index 948602ab2e..89cbe4ff2c 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -77,22 +77,32 @@ public: std::map params; if (parameter == "name") { params["name"] = param_value; - params["code"] = "80"; + params["space"] = "dhcp6"; + params["code"] = "38"; + params["data"] = "AB CDEF0105"; + params["csv-format"] = "False"; + } else if (parameter == "name") { + params["name"] = "subscriber-id"; + params["space"] = param_value; + params["code"] = "38"; params["data"] = "AB CDEF0105"; params["csv-format"] = "False"; } else if (parameter == "code") { - params["name"] = "option_foo"; + params["name"] = "subscriber-id"; + params["space"] = "dhcp6"; params["code"] = param_value; params["data"] = "AB CDEF0105"; params["csv-format"] = "False"; } else if (parameter == "data") { - params["name"] = "option_foo"; - params["code"] = "80"; + params["name"] = "subscriber-id"; + params["space"] = "dhcp6"; + params["code"] = "38"; params["data"] = param_value; params["csv-format"] = "False"; } else if (parameter == "csv-format") { - params["name"] = "option_foo"; - params["code"] = "80"; + params["name"] = "subscriber-id"; + params["space"] = "dhcp6"; + params["code"] = "38"; params["data"] = "AB CDEF0105"; params["csv-format"] = param_value; } @@ -122,6 +132,8 @@ public: } if (param.first == "name") { stream << "\"name\": \"" << param.second << "\""; + } else if (param.first == "space") { + stream << "\"space\": \"" << param.second << "\""; } else if (param.first == "code") { stream << "\"code\": " << param.second;; } else if (param.first == "data") { @@ -511,7 +523,7 @@ TEST_F(Dhcp6ParserTest, optionDefRecord) { EXPECT_EQ(100, def->getCode()); EXPECT_EQ(OPT_RECORD_TYPE, def->getType()); EXPECT_FALSE(def->getArrayType()); - + // The option comprises the record of data fields. Verify that all // fields are present and they are of the expected types. const OptionDefinition::RecordFieldsCollection& record_fields = @@ -731,8 +743,8 @@ TEST_F(Dhcp6ParserTest, optionDefInvalidRecordType) { /// The purpose of this test is to verify that it is not allowed -/// to override the standard option (that belongs to dhcp4 option -/// space) and that it is allowed to define option in the dhcp4 +/// to override the standard option (that belongs to dhcp6 option +/// space) and that it is allowed to define option in the dhcp6 /// option space that has a code which is not used by any of the /// standard options. TEST_F(Dhcp6ParserTest, optionStandardDefOverride) { @@ -804,16 +816,18 @@ TEST_F(Dhcp6ParserTest, optionDataDefaults) { "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" - " \"name\": \"option_foo\"," - " \"code\": 100," + " \"name\": \"subscriber-id\"," + " \"space\": \"dhcp6\"," + " \"code\": 38," " \"data\": \"AB CDEF0105\"," " \"csv-format\": False" " }," " {" - " \"name\": \"option_foo2\"," - " \"code\": 101," + " \"name\": \"preference\"," + " \"space\": \"dhcp6\"," + " \"code\": 7," " \"data\": \"01\"," - " \"csv-format\": False" + " \"csv-format\": True" " } ]," "\"subnet6\": [ { " " \"pool\": [ \"2001:db8:1::/80\" ]," @@ -841,31 +855,101 @@ TEST_F(Dhcp6ParserTest, optionDataDefaults) { // code so we get the range. std::pair range = - idx.equal_range(100); - // Expect single option with the code equal to 100. + idx.equal_range(D6O_SUBSCRIBER_ID); + // Expect single option with the code equal to 38. ASSERT_EQ(1, std::distance(range.first, range.second)); - const uint8_t foo_expected[] = { + const uint8_t subid_expected[] = { 0xAB, 0xCD, 0xEF, 0x01, 0x05 }; // Check if option is valid in terms of code and carried data. - testOption(*range.first, 100, foo_expected, sizeof(foo_expected)); + testOption(*range.first, D6O_SUBSCRIBER_ID, subid_expected, + sizeof(subid_expected)); - range = idx.equal_range(101); + range = idx.equal_range(D6O_PREFERENCE); ASSERT_EQ(1, std::distance(range.first, range.second)); // Do another round of testing with second option. - const uint8_t foo2_expected[] = { + const uint8_t pref_expected[] = { 0x01 }; - testOption(*range.first, 101, foo2_expected, sizeof(foo2_expected)); + testOption(*range.first, D6O_PREFERENCE, pref_expected, + sizeof(pref_expected)); // Check that options with other option codes are not returned. - for (uint16_t code = 102; code < 110; ++code) { + for (uint16_t code = 47; code < 57; ++code) { range = idx.equal_range(code); EXPECT_EQ(0, std::distance(range.first, range.second)); } } -// Goal of this test is to verify options configuration +/// The goal of this test is to verify that two options having the same +/// option code can be added to different option spaces. +TEST_F(Dhcp6ParserTest, optionDataTwoSpaces) { + + // This configuration string is to configure two options + // sharing the code 56 and having different definitions + // and belonging to the different option spaces. + // The option definition must be provided for the + // option that belongs to the 'isc' option space. + // The definition is not required for the option that + // belongs to the 'dhcp6' option space as it is the + // standard option. + string config = "{ \"interface\": [ \"all\" ]," + "\"rebind-timer\": 2000," + "\"renew-timer\": 1000," + "\"option-data\": [ {" + " \"name\": \"subscriber-id\"," + " \"space\": \"dhcp6\"," + " \"code\": 38," + " \"data\": \"AB CDEF0105\"," + " \"csv-format\": False" + " }," + " {" + " \"name\": \"foo\"," + " \"space\": \"isc\"," + " \"code\": 38," + " \"data\": \"1234\"," + " \"csv-format\": True" + " } ]," + "\"option-def\": [ {" + " \"name\": \"foo\"," + " \"code\": 38," + " \"type\": \"uint32\"," + " \"array\": False," + " \"record-types\": \"\"," + " \"space\": \"isc\"" + " } ]," + "\"subnet6\": [ { " + " \"pool\": [ \"2001:db8:1::/80\" ]," + " \"subnet\": \"2001:db8:1::/64\"" + " } ]" + "}"; + + ConstElementPtr status; + + ElementPtr json = Element::fromJSON(config); + + EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); + ASSERT_TRUE(status); + checkResult(status, 0); + + // Options should be now availabe for the subnet. + Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5")); + ASSERT_TRUE(subnet); + // Try to get the option from the space dhcp6. + Subnet::OptionDescriptor desc1 = subnet->getOptionDescriptor("dhcp6", 38); + ASSERT_TRUE(desc1.option); + EXPECT_EQ(38, desc1.option->getType()); + // Try to get the option from the space isc. + Subnet::OptionDescriptor desc2 = subnet->getOptionDescriptor("isc", 38); + ASSERT_TRUE(desc2.option); + EXPECT_EQ(38, desc1.option->getType()); + // Try to get the non-existing option from the non-existing + // option space and expect that option is not returned. + Subnet::OptionDescriptor desc3 = subnet->getOptionDescriptor("non-existing", 38); + ASSERT_FALSE(desc3.option); +} + +// The goal of this test is to verify options configuration // for a single subnet. In particular this test checks // that local options configuration overrides global // option setting. @@ -876,8 +960,9 @@ TEST_F(Dhcp6ParserTest, optionDataInSingleSubnet) { "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"option-data\": [ {" - " \"name\": \"option_foo\"," - " \"code\": 100," + " \"name\": \"subscriber-id\"," + " \"space\": \"dhcp6\"," + " \"code\": 38," " \"data\": \"AB\"," " \"csv-format\": False" " } ]," @@ -885,14 +970,16 @@ TEST_F(Dhcp6ParserTest, optionDataInSingleSubnet) { " \"pool\": [ \"2001:db8:1::/80\" ]," " \"subnet\": \"2001:db8:1::/64\", " " \"option-data\": [ {" - " \"name\": \"option_foo\"," - " \"code\": 100," + " \"name\": \"subscriber-id\"," + " \"space\": \"dhcp6\"," + " \"code\": 38," " \"data\": \"AB CDEF0105\"," " \"csv-format\": False" " }," " {" - " \"name\": \"option_foo2\"," - " \"code\": 101," + " \"name\": \"preference\"," + " \"space\": \"dhcp6\"," + " \"code\": 7," " \"data\": \"01\"," " \"csv-format\": False" " } ]" @@ -919,22 +1006,24 @@ TEST_F(Dhcp6ParserTest, optionDataInSingleSubnet) { // code so we get the range. std::pair range = - idx.equal_range(100); - // Expect single option with the code equal to 100. + idx.equal_range(D6O_SUBSCRIBER_ID); + // Expect single option with the code equal to 38. ASSERT_EQ(1, std::distance(range.first, range.second)); - const uint8_t foo_expected[] = { + const uint8_t subid_expected[] = { 0xAB, 0xCD, 0xEF, 0x01, 0x05 }; // Check if option is valid in terms of code and carried data. - testOption(*range.first, 100, foo_expected, sizeof(foo_expected)); + testOption(*range.first, D6O_SUBSCRIBER_ID, subid_expected, + sizeof(subid_expected)); - range = idx.equal_range(101); + range = idx.equal_range(D6O_PREFERENCE); ASSERT_EQ(1, std::distance(range.first, range.second)); // Do another round of testing with second option. - const uint8_t foo2_expected[] = { + const uint8_t pref_expected[] = { 0x01 }; - testOption(*range.first, 101, foo2_expected, sizeof(foo2_expected)); + testOption(*range.first, D6O_PREFERENCE, pref_expected, + sizeof(pref_expected)); } // Goal of this test is to verify options configuration @@ -949,8 +1038,9 @@ TEST_F(Dhcp6ParserTest, optionDataInMultipleSubnets) { " \"pool\": [ \"2001:db8:1::/80\" ]," " \"subnet\": \"2001:db8:1::/64\", " " \"option-data\": [ {" - " \"name\": \"option_foo\"," - " \"code\": 100," + " \"name\": \"subscriber-id\"," + " \"space\": \"dhcp6\"," + " \"code\": 38," " \"data\": \"0102030405060708090A\"," " \"csv-format\": False" " } ]" @@ -959,8 +1049,9 @@ TEST_F(Dhcp6ParserTest, optionDataInMultipleSubnets) { " \"pool\": [ \"2001:db8:2::/80\" ]," " \"subnet\": \"2001:db8:2::/64\", " " \"option-data\": [ {" - " \"name\": \"option_foo2\"," - " \"code\": 101," + " \"name\": \"user-class\"," + " \"space\": \"dhcp6\"," + " \"code\": 15," " \"data\": \"FFFEFDFCFB\"," " \"csv-format\": False" " } ]" @@ -987,15 +1078,16 @@ TEST_F(Dhcp6ParserTest, optionDataInMultipleSubnets) { // code so we get the range. std::pair range1 = - idx1.equal_range(100); - // Expect single option with the code equal to 100. + idx1.equal_range(D6O_SUBSCRIBER_ID); + // Expect single option with the code equal to 38. ASSERT_EQ(1, std::distance(range1.first, range1.second)); - const uint8_t foo_expected[] = { + const uint8_t subid_expected[] = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A }; // Check if option is valid in terms of code and carried data. - testOption(*range1.first, 100, foo_expected, sizeof(foo_expected)); + testOption(*range1.first, D6O_SUBSCRIBER_ID, subid_expected, + sizeof(subid_expected)); // Test another subnet in the same way. Subnet6Ptr subnet2 = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:2::4")); @@ -1006,13 +1098,14 @@ TEST_F(Dhcp6ParserTest, optionDataInMultipleSubnets) { const Subnet::OptionContainerTypeIndex& idx2 = options2->get<1>(); std::pair range2 = - idx2.equal_range(101); + idx2.equal_range(D6O_USER_CLASS); ASSERT_EQ(1, std::distance(range2.first, range2.second)); - const uint8_t foo2_expected[] = { + const uint8_t user_class_expected[] = { 0xFF, 0xFE, 0xFD, 0xFC, 0xFB }; - testOption(*range2.first, 101, foo2_expected, sizeof(foo2_expected)); + testOption(*range2.first, D6O_USER_CLASS, user_class_expected, + sizeof(user_class_expected)); } // Verify that empty option name is rejected in the configuration. @@ -1105,14 +1198,15 @@ TEST_F(Dhcp6ParserTest, optionDataLowerCase) { // code so we get the range. std::pair range = - idx.equal_range(80); - // Expect single option with the code equal to 100. + idx.equal_range(D6O_SUBSCRIBER_ID); + // Expect single option with the code equal to 38. ASSERT_EQ(1, std::distance(range.first, range.second)); - const uint8_t foo_expected[] = { + const uint8_t subid_expected[] = { 0x0A, 0x0B, 0x0C, 0x0D }; // Check if option is valid in terms of code and carried data. - testOption(*range.first, 80, foo_expected, sizeof(foo_expected)); + testOption(*range.first, D6O_SUBSCRIBER_ID, subid_expected, + sizeof(subid_expected)); } // Verify that specific option object is returned for standard @@ -1120,7 +1214,8 @@ TEST_F(Dhcp6ParserTest, optionDataLowerCase) { TEST_F(Dhcp6ParserTest, stdOptionData) { ConstElementPtr x; std::map params; - params["name"] = "OPTION_IA_NA"; + params["name"] = "ia-na"; + params["space"] = "dhcp6"; // Option code 3 means OPTION_IA_NA. params["code"] = "3"; params["data"] = "12345, 6789, 1516"; diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc index ee0c7250e6..1078917b47 100644 --- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc @@ -368,14 +368,16 @@ TEST_F(Dhcpv6SrvTest, advertiseOptions) { " \"pool\": [ \"2001:db8:1::/64\" ]," " \"subnet\": \"2001:db8:1::/48\", " " \"option-data\": [ {" - " \"name\": \"OPTION_DNS_SERVERS\"," + " \"name\": \"dns-servers\"," + " \"space\": \"dhcp6\"," " \"code\": 23," " \"data\": \"2001:db8:1234:FFFF::1, 2001:db8:1234:FFFF::2\"," " \"csv-format\": True" " }," " {" - " \"name\": \"OPTION_FOO\"," - " \"code\": 1000," + " \"name\": \"subscriber-id\"," + " \"space\": \"dhcp6\"," + " \"code\": 38," " \"data\": \"1234\"," " \"csv-format\": False" " } ]" @@ -404,18 +406,18 @@ TEST_F(Dhcpv6SrvTest, advertiseOptions) { // check if we get response at all ASSERT_TRUE(adv); - // We have not requested option with code 1000 so it should not + // We have not requested any options so they should not // be included in the response. - ASSERT_FALSE(adv->getOption(1000)); + ASSERT_FALSE(adv->getOption(D6O_SUBSCRIBER_ID)); ASSERT_FALSE(adv->getOption(D6O_NAME_SERVERS)); - // Let's now request option with code 1000. - // We expect that server will include this option in its reply. + // Let's now request some options. We expect that the server + // will include them in its response. boost::shared_ptr > option_oro(new OptionIntArray(Option::V6, D6O_ORO)); // Create vector with two option codes. std::vector codes(2); - codes[0] = 1000; + codes[0] = D6O_SUBSCRIBER_ID; codes[1] = D6O_NAME_SERVERS; // Pass this code to option. option_oro->setValues(codes); @@ -440,7 +442,7 @@ TEST_F(Dhcpv6SrvTest, advertiseOptions) { // There is a dummy option with code 1000 we requested from a server. // Expect that this option is in server's response. - tmp = adv->getOption(1000); + tmp = adv->getOption(D6O_SUBSCRIBER_ID); ASSERT_TRUE(tmp); // Check that the option contains valid data (from configuration).