Browse Source

[5019_rebase] Review changes:
- RSOOListConfigParser migrated
- Global parser tweaked (SrvConfig passed as parameter)
- duplicate_option_warning removed

Tomek Mrugalski 8 years ago
parent
commit
2084894153

+ 0 - 5
src/bin/dhcp4/dhcp4_messages.mes

@@ -533,11 +533,6 @@ server configuration, a set of parsers were successfully created, but
 one of them failed to commit its changes.  The reason for the failure
 one of them failed to commit its changes.  The reason for the failure
 is given in the message.
 is given in the message.
 
 
-% DHCP4_PARSER_CREATED created parser for configuration element %1
-A debug message output during a configuration update of the DHCPv4
-server, notifying that the parser for the specified configuration element
-has been successfully created.
-
 % DHCP4_PARSER_EXCEPTION failed to create or run parser for configuration element %1
 % DHCP4_PARSER_EXCEPTION failed to create or run parser for configuration element %1
 On receipt of message containing details to a change of its configuration,
 On receipt of message containing details to a change of its configuration,
 the DHCPv4 server failed to create a parser to decode the contents of
 the DHCPv4 server failed to create a parser to decode the contents of

+ 9 - 43
src/bin/dhcp4/json_config_parser.cc

@@ -196,19 +196,6 @@ protected:
         return (parser);
         return (parser);
     }
     }
 
 
-    /// @brief Issues a DHCP4 server specific warning regarding duplicate subnet
-    /// options.
-    ///
-    /// @param code is the numeric option code of the duplicate option
-    /// @param addr is the subnet address
-    /// @todo a means to know the correct logger and perhaps a common
-    /// message would allow this method to be emitted by the base class.
-    virtual void duplicate_option_warning(uint32_t code,
-                                         isc::asiolink::IOAddress& addr) {
-        LOG_WARN(dhcp4_logger, DHCP4_CONFIG_OPTION_DUPLICATE)
-            .arg(code).arg(addr.toText());
-    }
-
     /// @brief Instantiates the IPv4 Subnet based on a given IPv4 address
     /// @brief Instantiates the IPv4 Subnet based on a given IPv4 address
     /// and prefix length.
     /// and prefix length.
     ///
     ///
@@ -390,6 +377,7 @@ public:
     /// @brief Sets global parameters in staging configuration
     /// @brief Sets global parameters in staging configuration
     ///
     ///
     /// @param global global configuration scope
     /// @param global global configuration scope
+    /// @param cfg Server configuration (parsed parameters will be stored here)
     ///
     ///
     /// Currently this method sets the following global parameters:
     /// Currently this method sets the following global parameters:
     ///
     ///
@@ -399,14 +387,13 @@ public:
     ///
     ///
     /// @throw DhcpConfigError if parameters are missing or
     /// @throw DhcpConfigError if parameters are missing or
     /// or having incorrect values.
     /// or having incorrect values.
-    void parse(ConstElementPtr global) {
+    void parse(SrvConfigPtr cfg, ConstElementPtr global) {
 
 
         // Set whether v4 server is supposed to echo back client-id
         // Set whether v4 server is supposed to echo back client-id
         // (yes = RFC6842 compatible, no = backward compatibility)
         // (yes = RFC6842 compatible, no = backward compatibility)
         bool echo_client_id = getBoolean(global, "echo-client-id");
         bool echo_client_id = getBoolean(global, "echo-client-id");
         CfgMgr::instance().echoClientId(echo_client_id);
         CfgMgr::instance().echoClientId(echo_client_id);
 
 
-        SrvConfigPtr srv_cfg = CfgMgr::instance().getStagingCfg();
         std::string name;
         std::string name;
         ConstElementPtr value;
         ConstElementPtr value;
         try {
         try {
@@ -414,14 +401,14 @@ public:
             name = "decline-probation-period";
             name = "decline-probation-period";
             value = global->get(name);
             value = global->get(name);
             uint32_t probation_period = getUint32(name, value);
             uint32_t probation_period = getUint32(name, value);
-            srv_cfg->setDeclinePeriod(probation_period);
+            cfg->setDeclinePeriod(probation_period);
 
 
             // Set the DHCPv4-over-DHCPv6 interserver port.
             // Set the DHCPv4-over-DHCPv6 interserver port.
             name = "dhcp4o6-port";
             name = "dhcp4o6-port";
             value = global->get(name);
             value = global->get(name);
             // @todo Change for uint16_t
             // @todo Change for uint16_t
             uint32_t dhcp4o6_port = getUint32(name, value);
             uint32_t dhcp4o6_port = getUint32(name, value);
-            srv_cfg->setDhcp4o6Port(dhcp4o6_port);
+            cfg->setDhcp4o6Port(dhcp4o6_port);
         } catch (const isc::data::TypeError& ex) {
         } catch (const isc::data::TypeError& ex) {
             isc_throw(DhcpConfigError,
             isc_throw(DhcpConfigError,
                       "invalid value type specified for parameter '" << name
                       "invalid value type specified for parameter '" << name
@@ -553,16 +540,6 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
     // for option definitions. This is equivalent to commiting empty container.
     // for option definitions. This is equivalent to commiting empty container.
     LibDHCP::setRuntimeOptionDefs(OptionDefSpaceContainer());
     LibDHCP::setRuntimeOptionDefs(OptionDefSpaceContainer());
 
 
-    // Some of the values specified in the configuration depend on
-    // 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:
-    // all parsers other than: lease-database, subnet4 and option-data parser,
-    // then: option-data parser, subnet4 parser, lease-database parser.
-    // Please do not change this order!
-    ParserCollection independent_parsers;
-
     // Some of the parsers alter the state of the system in a way that can't
     // Some of the parsers alter the state of the system in a way that can't
     // easily be undone. (Or alter it in a way such that undoing the change has
     // easily be undone. (Or alter it in a way such that undoing the change has
     // the same risk of failure as doing the change.)
     // the same risk of failure as doing the change.)
@@ -720,21 +697,10 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
                 continue;
                 continue;
             }
             }
 
 
-            /// @todo: 5116 ticket: remove this chunk below once all parser
-            /// tickets are done.
-            ParserPtr parser(createGlobalDhcp4ConfigParser(config_pair.first,
-                                                           config_pair.second));
-            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PARSER_CREATED)
-                      .arg(config_pair.first);
-
-            // 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
-            // but we need it so as the subnet6 parser can access the
-            // parsed data.
-            parser->commit();
+            // If we got here, no code handled this parameter, so we bail out.
+            isc_throw(DhcpConfigError,
+                      "unsupported global configuration parameter: " << config_pair.first
+                      << " (" << config_pair.second->getPosition() << ")");
         }
         }
 
 
         // Setup the command channel.
         // Setup the command channel.
@@ -742,7 +708,7 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
 
 
         // Apply global options in the staging config.
         // Apply global options in the staging config.
         Dhcp4ConfigParser global_parser;
         Dhcp4ConfigParser global_parser;
-        global_parser.parse(mutable_cfg);
+        global_parser.parse(srv_cfg, mutable_cfg);
 
 
     } catch (const isc::Exception& ex) {
     } catch (const isc::Exception& ex) {
         LOG_ERROR(dhcp4_logger, DHCP4_PARSER_FAIL)
         LOG_ERROR(dhcp4_logger, DHCP4_PARSER_FAIL)

+ 15 - 26
src/bin/dhcp6/json_config_parser.cc

@@ -543,30 +543,17 @@ public:
 /// The options on this list can be specified using an option code or option
 /// The options on this list can be specified using an option code or option
 /// name. Therefore, the values on the list should always be enclosed in
 /// name. Therefore, the values on the list should always be enclosed in
 /// "quotes".
 /// "quotes".
-class RSOOListConfigParser : public DhcpConfigParser {
+class RSOOListConfigParser : public isc::data::SimpleParser {
 public:
 public:
 
 
-    /// @brief constructor
-    ///
-    /// As this is a dedicated parser, it must be used to parse
-    /// "relay-supplied-options" parameter only. All other types will throw exception.
-    ///
-    /// @param param_name name of the configuration parameter being parsed
-    /// @throw BadValue if supplied parameter name is not "relay-supplied-options"
-    RSOOListConfigParser(const std::string& param_name) {
-        if (param_name != "relay-supplied-options") {
-            isc_throw(BadValue, "Internal error. RSOO configuration "
-                      "parser called for the wrong parameter: " << param_name);
-        }
-    }
-
     /// @brief parses parameters value
     /// @brief parses parameters value
     ///
     ///
     /// Parses configuration entry (list of sources) and adds each element
     /// Parses configuration entry (list of sources) and adds each element
     /// to the RSOO list.
     /// to the RSOO list.
     ///
     ///
     /// @param value pointer to the content of parsed values
     /// @param value pointer to the content of parsed values
-    virtual void build(isc::data::ConstElementPtr value) {
+    /// @param cfg server configuration (RSOO will be stored here)
+    void parse(SrvConfigPtr cfg, isc::data::ConstElementPtr value) {
         try {
         try {
             BOOST_FOREACH(ConstElementPtr source_elem, value->listValue()) {
             BOOST_FOREACH(ConstElementPtr source_elem, value->listValue()) {
                 std::string option_str = source_elem->stringValue();
                 std::string option_str = source_elem->stringValue();
@@ -603,7 +590,7 @@ public:
                                   " relay-supplied-options");
                                   " relay-supplied-options");
                     }
                     }
                 }
                 }
-                CfgMgr::instance().getStagingCfg()->getCfgRSOO()->enable(code);
+                cfg->getCfgRSOO()->enable(code);
             }
             }
         } catch (const std::exception& ex) {
         } catch (const std::exception& ex) {
             // Rethrow exception with the appended position of the parsed
             // Rethrow exception with the appended position of the parsed
@@ -611,9 +598,6 @@ public:
             isc_throw(DhcpConfigError, ex.what() << " (" << value->getPosition() << ")");
             isc_throw(DhcpConfigError, ex.what() << " (" << value->getPosition() << ")");
         }
         }
     }
     }
-
-    /// @brief Does nothing.
-    virtual void commit() {}
 };
 };
 
 
 class Dhcp6ConfigParser : public isc::data::SimpleParser {
 class Dhcp6ConfigParser : public isc::data::SimpleParser {
@@ -622,6 +606,7 @@ public:
     /// @brief Sets global parameters in staging configuration
     /// @brief Sets global parameters in staging configuration
     ///
     ///
     /// @param global global configuration scope
     /// @param global global configuration scope
+    /// @param cfg Server configuration (parsed parameters will be stored here)
     ///
     ///
     /// Currently this method sets the following global parameters:
     /// Currently this method sets the following global parameters:
     ///
     ///
@@ -630,9 +615,8 @@ public:
     ///
     ///
     /// @throw DhcpConfigError if parameters are missing or
     /// @throw DhcpConfigError if parameters are missing or
     /// or having incorrect values.
     /// or having incorrect values.
-    void parse(ConstElementPtr global) {
+    void parse(SrvConfigPtr srv_config, ConstElementPtr global) {
 
 
-        SrvConfigPtr srv_config = CfgMgr::instance().getStagingCfg();
         std::string name;
         std::string name;
         ConstElementPtr value;
         ConstElementPtr value;
         try {
         try {
@@ -703,14 +687,13 @@ DhcpConfigParser* createGlobal6DhcpConfigParser(const std::string& config_id,
     // lease-database and hosts-database have been converted to SimpleParser already.
     // lease-database and hosts-database have been converted to SimpleParser already.
     // mac-source has been converted to SimpleParser.
     // mac-source has been converted to SimpleParser.
     // dhcp-ddns has been converted to SimpleParser
     // dhcp-ddns has been converted to SimpleParser
-    if (config_id.compare("relay-supplied-options") == 0) {
-        parser = new RSOOListConfigParser(config_id);
+    // rsoo has been converted to SimpleParser.
     // control-socket has been converted to SimpleParser.
     // control-socket has been converted to SimpleParser.
     // expired-leases-processing has been converted to SimpleParser.
     // expired-leases-processing has been converted to SimpleParser.
     // client-classes has been converted to SimpleParser.
     // client-classes has been converted to SimpleParser.
     // host-reservation-identifiers have been converted to SimpleParser already.
     // host-reservation-identifiers have been converted to SimpleParser already.
     // server-id has been migrated to SimpleParser
     // server-id has been migrated to SimpleParser
-    } else {
+    {
         isc_throw(DhcpConfigError,
         isc_throw(DhcpConfigError,
                 "unsupported global configuration parameter: "
                 "unsupported global configuration parameter: "
                   << config_id << " (" << element->getPosition() << ")");
                   << config_id << " (" << element->getPosition() << ")");
@@ -967,6 +950,12 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) {
                 continue;
                 continue;
             }
             }
 
 
+            if (config_pair.first == "relay-supplied-options") {
+                RSOOListConfigParser parser;
+                parser.parse(srv_config, config_pair.second);
+                continue;
+            }
+
             /// @todo: 5116 ticket: remove this chunk below once all parser
             /// @todo: 5116 ticket: remove this chunk below once all parser
             /// tickets are done.
             /// tickets are done.
             ParserPtr parser(createGlobal6DhcpConfigParser(config_pair.first,
             ParserPtr parser(createGlobal6DhcpConfigParser(config_pair.first,
@@ -989,7 +978,7 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) {
 
 
         // Apply global options in the staging config.
         // Apply global options in the staging config.
         Dhcp6ConfigParser global_parser;
         Dhcp6ConfigParser global_parser;
-        global_parser.parse(mutable_cfg);
+        global_parser.parse(srv_config, mutable_cfg);
 
 
     } catch (const isc::Exception& ex) {
     } catch (const isc::Exception& ex) {
         LOG_ERROR(dhcp6_logger, DHCP6_PARSER_FAIL)
         LOG_ERROR(dhcp6_logger, DHCP6_PARSER_FAIL)