From: Marcin Siodelski Date: Thu, 10 Jan 2013 11:57:53 +0000 (+0100) Subject: [2317] Configure space when setting option value. X-Git-Tag: bind10-1.0.0-rc-release~100^2~2^2~9 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b82c412af1ef36000368f817f25aef23cc36728a;p=thirdparty%2Fkea.git [2317] Configure space when setting option value. --- diff --git a/src/bin/dhcp4/config_parser.cc b/src/bin/dhcp4/config_parser.cc index af598ad670..e8ceab31b3 100644 --- a/src/bin/dhcp4/config_parser.cc +++ b/src/bin/dhcp4/config_parser.cc @@ -631,7 +631,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) { @@ -645,13 +646,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))); @@ -687,12 +681,12 @@ public: /// remain un-modified. virtual void commit() { if (options_ == NULL) { - isc_throw(isc::InvalidOperation, "Parser logic error: storage must be set before " + isc_throw(isc::InvalidOperation, "parser logic error: storage must be set before " "commiting option data."); } else if (!option_descriptor_.option) { // Before we can commit the new option should be configured. If it is not // than somebody must have called commit() before build(). - isc_throw(isc::InvalidOperation, "Parser logic error: no option has been configured and" + isc_throw(isc::InvalidOperation, "parser logic error: no option has been configured and" " thus there is nothing to commit. Has build() been called?"); } uint16_t opt_type = option_descriptor_.option->getType(); @@ -761,9 +755,32 @@ private: << " spaces"); } + std::string option_space = getParam("space", string_values_); + /// @todo Validate option space once #2313 is merged. + + OptionDefinitionPtr def; + if (option_space == "dhcp4" && + LibDHCP::isStandardOption(Option::V4, option_code)) { + def = LibDHCP::getOptionDef(Option::V4, option_code); + + } else if (option_space == "dhcp6") { + isc_throw(DhcpConfigError, "'dhcp6' option space name is reserved" + << " for DHCPv6 server"); + } else { + def = CfgMgr::instance().getOptionDef(option_space, option_code); + 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; @@ -784,24 +801,9 @@ private: << " string of hexadecimal digits: " << option_data); } } - // Get all existing DHCPv4 option definitions. The one that matches - // our option will be picked and used to create it. - OptionDefContainer option_defs = LibDHCP::getOptionDefs(Option::V4); - // 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" @@ -822,9 +824,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 in " + << "option definition: '" << 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::V4, option_code, data_tokens) : diff --git a/src/bin/dhcp4/dhcp4.spec b/src/bin/dhcp4/dhcp4.spec index 0be4da1db9..c2b755cb68 100644 --- a/src/bin/dhcp4/dhcp4.spec +++ b/src/bin/dhcp4/dhcp4.spec @@ -116,6 +116,11 @@ "item_type": "boolean", "item_optional": false, "item_default": False + }, + { "item_name": "space", + "item_type": "string", + "item_optional": false, + "item_default": "dhcp4" } ] } }, @@ -201,6 +206,11 @@ "item_type": "boolean", "item_optional": false, "item_default": False + }, + { "item_name": "space", + "item_type": "string", + "item_optional": false, + "item_default": "dhcp4" } ] } } ] diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 7201ec30c8..9a4a33fa08 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -92,21 +92,31 @@ public: std::map params; if (parameter == "name") { params["name"] = param_value; + params["space"] = "dhcp4"; + params["code"] = "56"; + params["data"] = "AB CDEF0105"; + params["csv-format"] = "False"; + } else if (parameter == "space") { + params["name"] = "dhcp-message"; + params["space"] = param_value; params["code"] = "56"; params["data"] = "AB CDEF0105"; params["csv-format"] = "False"; } else if (parameter == "code") { - params["name"] = "option_foo"; + params["name"] = "dhcp-message"; + params["space"] = "dhcp4"; params["code"] = param_value; params["data"] = "AB CDEF0105"; params["csv-format"] = "False"; } else if (parameter == "data") { - params["name"] = "option_foo"; + params["name"] = "dhcp-message"; + params["space"] = "dhcp4"; params["code"] = "56"; params["data"] = param_value; params["csv-format"] = "False"; } else if (parameter == "csv-format") { - params["name"] = "option_foo"; + params["name"] = "dhcp-message"; + params["space"] = "dhcp4"; params["code"] = "56"; params["data"] = "AB CDEF0105"; params["csv-format"] = param_value; @@ -142,6 +152,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") { @@ -803,13 +815,15 @@ TEST_F(Dhcp4ParserTest, optionDataDefaults) { "\"rebind-timer\": 2000," "\"renew-timer\": 1000," "\"option-data\": [ {" - " \"name\": \"option_foo\"," + " \"name\": \"dhcp-message\"," + " \"space\": \"dhcp4\"," " \"code\": 56," " \"data\": \"AB CDEF0105\"," " \"csv-format\": False" " }," " {" - " \"name\": \"option_foo2\"," + " \"name\": \"default-ip-ttl\"," + " \"space\": \"dhcp4\"," " \"code\": 23," " \"data\": \"01\"," " \"csv-format\": False" @@ -868,7 +882,8 @@ TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) { "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " "\"option-data\": [ {" - " \"name\": \"option_foo\"," + " \"name\": \"dhcp-message\"," + " \"space\": \"dhcp4\"," " \"code\": 56," " \"data\": \"AB\"," " \"csv-format\": False" @@ -877,13 +892,15 @@ TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) { " \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ]," " \"subnet\": \"192.0.2.0/24\", " " \"option-data\": [ {" - " \"name\": \"option_foo\"," + " \"name\": \"dhcp-message\"," + " \"space\": \"dhcp4\"," " \"code\": 56," " \"data\": \"AB CDEF0105\"," " \"csv-format\": False" " }," " {" - " \"name\": \"option_foo2\"," + " \"name\": \"default-ip-ttl\"," + " \"space\": \"dhcp4\"," " \"code\": 23," " \"data\": \"01\"," " \"csv-format\": False" @@ -940,7 +957,8 @@ TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) { " \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ]," " \"subnet\": \"192.0.2.0/24\", " " \"option-data\": [ {" - " \"name\": \"option_foo\"," + " \"name\": \"dhcp-message\"," + " \"space\": \"dhcp4\"," " \"code\": 56," " \"data\": \"0102030405060708090A\"," " \"csv-format\": False" @@ -950,7 +968,8 @@ TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) { " \"pool\": [ \"192.0.3.101 - 192.0.3.150\" ]," " \"subnet\": \"192.0.3.0/24\", " " \"option-data\": [ {" - " \"name\": \"option_foo2\"," + " \"name\": \"default-ip-ttl\"," + " \"space\": \"dhcp4\"," " \"code\": 23," " \"data\": \"FF\"," " \"csv-format\": False" @@ -1103,6 +1122,7 @@ TEST_F(Dhcp4ParserTest, stdOptionData) { ConstElementPtr x; std::map params; params["name"] = "nis-servers"; + params["space"] = "dhcp4"; // Option code 41 means nis-servers. params["code"] = "41"; // Specify option values in a CSV (user friendly) format.