BIND 10 trac2317, updated. ec9d85afe154206e95b683eaa8733e0f453f4090 [2317] Improved some comments in the DHCPv4 data parsers.

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Jan 10 17:39:19 UTC 2013


The branch, trac2317 has been updated
       via  ec9d85afe154206e95b683eaa8733e0f453f4090 (commit)
       via  0b3abc02d6b943026d5e60f6376d5c20d913bcb7 (commit)
       via  beef799a018be9b0be103973c9fa477a7d0ca6fb (commit)
       via  b82c412af1ef36000368f817f25aef23cc36728a (commit)
      from  adf283b9aa964639693d23a4e809fe67657583fb (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit ec9d85afe154206e95b683eaa8733e0f453f4090
Author: Marcin Siodelski <marcin at isc.org>
Date:   Thu Jan 10 18:39:03 2013 +0100

    [2317] Improved some comments in the DHCPv4 data parsers.

commit 0b3abc02d6b943026d5e60f6376d5c20d913bcb7
Author: Marcin Siodelski <marcin at isc.org>
Date:   Thu Jan 10 18:19:21 2013 +0100

    [2317] Option data parser is dependent on option definitions parser.

commit beef799a018be9b0be103973c9fa477a7d0ca6fb
Author: Marcin Siodelski <marcin at isc.org>
Date:   Thu Jan 10 13:27:52 2013 +0100

    [2317] Group configured options by their spaces.

commit b82c412af1ef36000368f817f25aef23cc36728a
Author: Marcin Siodelski <marcin at isc.org>
Date:   Thu Jan 10 12:57:53 2013 +0100

    [2317] Configure space when setting option value.

-----------------------------------------------------------------------

Summary of changes:
 src/bin/dhcp4/config_parser.cc                |  249 ++++++++++++++++---------
 src/bin/dhcp4/dhcp4.spec                      |   10 +
 src/bin/dhcp4/tests/config_parser_unittest.cc |  112 +++++++++--
 3 files changed, 269 insertions(+), 102 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/config_parser.cc b/src/bin/dhcp4/config_parser.cc
index af598ad..e3ed19b 100644
--- a/src/bin/dhcp4/config_parser.cc
+++ b/src/bin/dhcp4/config_parser.cc
@@ -77,10 +77,10 @@ typedef OptionSpaceContainer<OptionDefContainer,
 /// no subnet object created yet to store them.
 typedef std::vector<Pool4Ptr> PoolStorage;
 
-/// @brief Collection of option descriptors. This container allows searching for
-/// options using the option code or persistency flag. This is useful when merging
-/// existing options with newly configured options.
-typedef Subnet::OptionContainer OptionStorage;
+/// Collection of containers holding option spaces. Each container within
+/// a particular option space holds so-called option descriptors.
+typedef OptionSpaceContainer<Subnet::OptionContainer,
+                             Subnet::OptionDescriptor> OptionStorage;
 
 /// @brief Global uint32 parameters that will be used as defaults.
 Uint32Storage uint32_defaults;
@@ -91,6 +91,9 @@ 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
 ///
 /// It is a debugging parser. It does not configure anything,
@@ -631,7 +634,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<StringParser>
                     name_parser(dynamic_cast<StringParser*>(StringParser::factory(param.first)));
                 if (name_parser) {
@@ -645,13 +649,6 @@ public:
                     code_parser->setStorage(&uint32_values_);
                     parser = code_parser;
                 }
-            } else if (param.first == "data") {
-                boost::shared_ptr<StringParser>
-                    value_parser(dynamic_cast<StringParser*>(StringParser::factory(param.first)));
-                if (value_parser) {
-                    value_parser->setStorage(&string_values_);
-                    parser = value_parser;
-                }
             } else if (param.first == "csv-format") {
                 boost::shared_ptr<BooleanParser>
                     value_parser(dynamic_cast<BooleanParser*>(BooleanParser::factory(param.first)));
@@ -687,16 +684,18 @@ 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();
-        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.
@@ -706,7 +705,7 @@ public:
             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.
@@ -761,9 +760,42 @@ private:
                       << " spaces");
         }
 
+        std::string option_space = getParam<std::string>("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 {
+            // 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<std::string>("data", string_values_);
         const bool csv_format = getParam<bool>("csv-format", boolean_values_);
+
         // Transform string of hexadecimal digits into binary format.
         std::vector<uint8_t> binary;
         std::vector<std::string> data_tokens;
@@ -784,24 +816,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 +839,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::V4, option_code, data_tokens) :
@@ -833,11 +862,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).
@@ -851,6 +883,8 @@ private:
     OptionStorage* options_;
     /// Option descriptor holds newly configured option.
     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.
@@ -1126,6 +1160,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");
@@ -1134,7 +1172,7 @@ public:
         BOOST_FOREACH(ConstElementPtr option_def, option_def_list->listValue()) {
             boost::shared_ptr<OptionDefParser>
                 parser(new OptionDefParser("single-option-def"));
-            parser->setStorage(&option_defs_local_);
+            parser->setStorage(&option_def_defaults);
             parser->build(option_def);
             parser->commit();
         }
@@ -1150,10 +1188,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);
             }
@@ -1173,7 +1211,7 @@ private:
 
     /// Temporary storage for option definitions. It holds option
     /// definitions before \ref commit is called.
-    OptionDefStorage option_defs_local_;
+    //    OptionDefStorage option_defs_local_;
 };
 
 /// @brief this class parses a single subnet
@@ -1337,39 +1375,56 @@ private:
             subnet_->addPool4(*it);
         }
 
-        Subnet::OptionContainerPtr options = subnet_->getOptionDescriptors("dhcp4");
-        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(dhcp4_logger, DHCP4_CONFIG_OPTION_DUPLICATE)
-                    .arg(desc.option->getType()).arg(addr.toText());
+        // 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
+        // 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(dhcp4_logger, DHCP4_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, "dhcp4");
         }
 
         // 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("dhcp4");
-            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, "dhcp4");
+        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);
+                }
             }
         }
     }
@@ -1568,15 +1623,13 @@ configureDhcp4Server(Dhcpv4Srv& , ConstElementPtr config_set) {
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_COMMAND, DHCP4_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 storage. For this reason the global data
@@ -1588,6 +1641,7 @@ configureDhcp4Server(Dhcpv4Srv& , 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;
@@ -1596,12 +1650,20 @@ configureDhcp4Server(Dhcpv4Srv& , ConstElementPtr config_set) {
     bool rollback = false;
 
     try {
+        // Make parsers grouping.
+        const std::map<std::string, ConstElementPtr>& values_map =
+            config_set->mapValue();
+        BOOST_FOREACH(ConfigPair config_pair, values_map) {
+            ParserPtr parser(createGlobalDhcp4ConfigParser(config_pair.first));
+            if (config_pair.first == "subnet4") {
+                subnet_parser = parser;
 
-        // Iterate over all independent parsers first (all but subnet4)
-        // and try to parse the data.
-        BOOST_FOREACH(ConfigPair config_pair, config_set->mapValue()) {
-            if (config_pair.first != "subnet4") {
-                ParserPtr parser(createGlobalDhcp4ConfigParser(config_pair.first));
+            } 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
@@ -1611,13 +1673,19 @@ configureDhcp4Server(Dhcpv4Srv& , ConstElementPtr config_set) {
             }
         }
 
-        // Process dependent configuration data.
-        BOOST_FOREACH(ConfigPair config_pair, config_set->mapValue()) {
-            if (config_pair.first == "subnet4") {
-                ParserPtr parser(createGlobalDhcp4ConfigParser(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<std::string, ConstElementPtr>::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<std::string, ConstElementPtr>::const_iterator subnet_config =
+            values_map.find("subnet4");
+        if (subnet_config != values_map.end()) {
+            subnet_parser->build(subnet_config->second);
         }
 
     } catch (const isc::Exception& ex) {
@@ -1642,8 +1710,8 @@ configureDhcp4Server(Dhcpv4Srv& , 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) {
@@ -1665,6 +1733,7 @@ configureDhcp4Server(Dhcpv4Srv& , 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/dhcp4/dhcp4.spec b/src/bin/dhcp4/dhcp4.spec
index 0be4da1..c2b755c 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 7201ec3..cb7340a 100644
--- a/src/bin/dhcp4/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp4/tests/config_parser_unittest.cc
@@ -78,8 +78,8 @@ public:
     /// @brief Create the simple configuration with single option.
     ///
     /// This function allows to set one of the parameters that configure
-    /// option value. These parameters are: "name", "code", "data" and
-    /// "csv-format".
+    /// option value. These parameters are: "name", "code", "data",
+    /// "csv-format" and "space".
     ///
     /// @param param_value string holiding option parameter value to be
     /// injected into the configuration string.
@@ -92,21 +92,31 @@ public:
         std::map<std::string, std::string> 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"
@@ -858,6 +872,74 @@ TEST_F(Dhcp4ParserTest, optionDataDefaults) {
     testOption(*range.first, 23, foo2_expected, sizeof(foo2_expected));
 }
 
+/// 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(Dhcp4ParserTest, 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 'dhcp4' option space as it is the
+    // standard option.
+    string config = "{ \"interface\": [ \"all\" ],"
+        "\"rebind-timer\": 2000,"
+        "\"renew-timer\": 1000,"
+        "\"option-data\": [ {"
+        "    \"name\": \"dhcp-message\","
+        "    \"space\": \"dhcp4\","
+        "    \"code\": 56,"
+        "    \"data\": \"AB CDEF0105\","
+        "    \"csv-format\": False"
+        " },"
+        " {"
+        "    \"name\": \"foo\","
+        "    \"space\": \"isc\","
+        "    \"code\": 56,"
+        "    \"data\": \"1234\","
+        "    \"csv-format\": True"
+        " } ],"
+        "\"option-def\": [ {"
+        "    \"name\": \"foo\","
+        "    \"code\": 56,"
+        "    \"type\": \"uint32\","
+        "    \"array\": False,"
+        "    \"record-types\": \"\","
+        "    \"space\": \"isc\""
+        " } ],"
+        "\"subnet4\": [ { "
+        "    \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
+        "    \"subnet\": \"192.0.2.0/24\""
+        " } ]"
+        "}";
+
+    ConstElementPtr status;
+   
+    ElementPtr json = Element::fromJSON(config);
+
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    ASSERT_TRUE(status);
+    checkResult(status, 0);
+
+    // Options should be now availabe for the subnet.
+    Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.200"));
+    ASSERT_TRUE(subnet);
+    // Try to get the option from the space dhcp4.
+    Subnet::OptionDescriptor desc1 = subnet->getOptionDescriptor("dhcp4", 56);
+    ASSERT_TRUE(desc1.option);
+    EXPECT_EQ(56, desc1.option->getType());
+    // Try to get the option from the space isc.
+    Subnet::OptionDescriptor desc2 = subnet->getOptionDescriptor("isc", 56);
+    ASSERT_TRUE(desc2.option);
+    EXPECT_EQ(56, 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", 56);
+    ASSERT_FALSE(desc3.option);
+}
+
 // Goal of this test is to verify options configuration
 // for a single subnet. In particular this test checks
 // that local options configuration overrides global
@@ -868,7 +950,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 +960,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 +1025,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 +1036,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 +1190,7 @@ TEST_F(Dhcp4ParserTest, stdOptionData) {
     ConstElementPtr x;
     std::map<std::string, std::string> 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.



More information about the bind10-changes mailing list