Browse Source

[5122] Contexts removed from SubnetConfigParser

Tomek Mrugalski 8 years ago
parent
commit
523deee862

+ 1 - 40
src/bin/dhcp4/json_config_parser.cc

@@ -109,7 +109,7 @@ public:
     ///
     /// stores global scope parameters, options, option definitions.
     Subnet4ConfigParser()
-        :SubnetConfigParser("", globalContext(), IOAddress("0.0.0.0")) {
+        :SubnetConfigParser(AF_INET) {
     }
 
     /// @brief Parses a single IPv4 subnet configuration and adds to the
@@ -157,45 +157,6 @@ public:
 
 protected:
 
-    /// @brief Creates parsers for entries in subnet definition.
-    ///
-    /// @param config_id name of the entry
-    ///
-    /// @return parser object for specified entry name. Note the caller is
-    /// responsible for deleting the parser created.
-    /// @throw isc::dhcp::DhcpConfigError if trying to create a parser
-    /// for unknown config element
-    DhcpConfigParser* createSubnetConfigParser(const std::string& config_id) {
-        DhcpConfigParser* parser = NULL;
-        if ((config_id.compare("valid-lifetime") == 0)  ||
-            (config_id.compare("renew-timer") == 0)  ||
-            (config_id.compare("rebind-timer") == 0) ||
-            (config_id.compare("id") == 0)) {
-            parser = new Uint32Parser(config_id, uint32_values_);
-        } else if ((config_id.compare("subnet") == 0) ||
-                   (config_id.compare("interface") == 0) ||
-                   (config_id.compare("client-class") == 0) ||
-                   (config_id.compare("next-server") == 0) ||
-                   (config_id.compare("reservation-mode") == 0)) {
-            parser = new StringParser(config_id, string_values_);
-        // pools has been converted to SimpleParser already.
-        // relay has been converted to SimpleParser already.
-        // option-data has been converted to SimpleParser already.
-        } else if (config_id.compare("match-client-id") == 0) {
-            parser = new BooleanParser(config_id, boolean_values_);
-        } else if (config_id.compare("4o6-subnet") == 0) {
-            parser = new StringParser(config_id, string_values_);
-        } else if (config_id.compare("4o6-interface") == 0) {
-            parser = new StringParser(config_id, string_values_);
-        } else if (config_id.compare("4o6-interface-id") == 0) {
-            parser = new StringParser(config_id, string_values_);
-        } else {
-            isc_throw(NotImplemented, "unsupported parameter: " << config_id);
-        }
-
-        return (parser);
-    }
-
     /// @brief Instantiates the IPv4 Subnet based on a given IPv4 address
     /// and prefix length.
     ///

+ 1 - 1
src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc

@@ -583,7 +583,7 @@ TEST_F(CtrlChannelDhcpv4SrvTest, set_config) {
 
     // Should fail with a syntax error
     EXPECT_EQ("{ \"result\": 1, "
-              "\"text\": \"unsupported parameter: BOGUS (<string>:20:26)\" }",
+              "\"text\": \"subnet configuration failed (<string>:20:17): String parameter subnet not found(<string>:20:17)\" }",
               response);
 
     // Check that the config was not lost

+ 1 - 37
src/bin/dhcp6/json_config_parser.cc

@@ -302,7 +302,7 @@ public:
     ///
     /// stores global scope parameters, options, option definitions.
     Subnet6ConfigParser()
-        :SubnetConfigParser("", globalContext(), IOAddress("::")) {
+        :SubnetConfigParser(AF_INET6) {
     }
 
     /// @brief Parses a single IPv6 subnet configuration and adds to the
@@ -355,42 +355,6 @@ public:
     }
 
 protected:
-
-    /// @brief creates parsers for entries in subnet definition
-    ///
-    /// @param config_id name of the entry
-    ///
-    /// @return parser object for specified entry name. Note the caller is
-    /// responsible for deleting the parser created.
-    /// @throw isc::dhcp::DhcpConfigError if trying to create a parser
-    /// for unknown config element
-    DhcpConfigParser* createSubnetConfigParser(const std::string& config_id) {
-        DhcpConfigParser* parser = NULL;
-        if ((config_id.compare("preferred-lifetime") == 0)  ||
-            (config_id.compare("valid-lifetime") == 0)  ||
-            (config_id.compare("renew-timer") == 0)  ||
-            (config_id.compare("rebind-timer") == 0) ||
-            (config_id.compare("id") == 0)) {
-            parser = new Uint32Parser(config_id, uint32_values_);
-        } else if ((config_id.compare("subnet") == 0) ||
-                   (config_id.compare("interface") == 0) ||
-                   (config_id.compare("client-class") == 0) ||
-                   (config_id.compare("interface-id") == 0) ||
-                   (config_id.compare("reservation-mode") == 0)) {
-            parser = new StringParser(config_id, string_values_);
-        // pools has been converted to SimpleParser.
-        // relay has been converted to SimpleParser.
-        // pd-pools has been converted to SimpleParser.
-        // option-data was here, but it is now converted to SimpleParser
-        } else if (config_id.compare("rapid-commit") == 0) {
-            parser = new BooleanParser(config_id, boolean_values_);
-        } else {
-            isc_throw(NotImplemented, "unsupported parameter: " << config_id);
-        }
-
-        return (parser);
-    }
-
     /// @brief Issues a DHCP6 server specific warning regarding duplicate subnet
     /// options.
     ///

+ 1 - 1
src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc

@@ -446,7 +446,7 @@ TEST_F(CtrlChannelDhcpv6SrvTest, set_config) {
 
     // Should fail with a syntax error
     EXPECT_EQ("{ \"result\": 1, "
-              "\"text\": \"unsupported parameter: BOGUS (<string>:21:26)\" }",
+              "\"text\": \"subnet configuration failed (<string>:21:17): String parameter subnet not found(<string>:21:17)\" }",
               response);
 
     // Check that the config was not lost

+ 18 - 103
src/lib/dhcpsrv/parsers/dhcp_parsers.cc

@@ -979,85 +979,28 @@ PoolParser::parse(PoolStoragePtr pools,
 
 //****************************** SubnetConfigParser *************************
 
-SubnetConfigParser::SubnetConfigParser(const std::string&,
-                                       ParserContextPtr global_context,
-                                       const isc::asiolink::IOAddress& default_addr)
-    : uint32_values_(new Uint32Storage()),
-      string_values_(new StringStorage()),
-      boolean_values_(new BooleanStorage()),
-      pools_(new PoolStorage()),
-      global_context_(global_context),
-      relay_info_(new isc::dhcp::Subnet::RelayInfo(default_addr)),
+SubnetConfigParser::SubnetConfigParser(uint16_t family)
+    : pools_(new PoolStorage()),
+      address_family_(family),
       options_(new CfgOption()) {
-    // The first parameter should always be "subnet", but we don't check
-    // against that here in case some wants to reuse this parser somewhere.
-    if (!global_context_) {
-        isc_throw(isc::dhcp::DhcpConfigError, "parser logic error: "
-                 << "context storage may not be NULL");
-    }
-
+    string addr = family == AF_INET ? "0.0.0.0" : "::";
+    relay_info_.reset(new isc::dhcp::Subnet::RelayInfo(IOAddress(addr)));
 }
 
 SubnetPtr
 SubnetConfigParser::parse(ConstElementPtr subnet) {
-    BOOST_FOREACH(ConfigPair param, subnet->mapValue()) {
-        // Pools has been converted to SimpleParser.
-        if (param.first == "pools") {
-            continue;
-        }
-
-        // PdPools has been converted to SimpleParser.
-        if ((param.first == "pd-pools") &&
-            (global_context_->universe_ == Option::V6)) {
-            continue;
-        }
-
-        // Host reservations must be parsed after subnet specific parameters.
-        // Note that the reservation parsing will be invoked by the build()
-        // in the derived classes, i.e. Subnet4ConfigParser and
-        // Subnet6ConfigParser.
-        if (param.first == "reservations") {
-            continue;
-        }
 
-        if (param.first == "option-data") {
-            uint16_t family = global_context_->universe_ == Option::V4 ? AF_INET : AF_INET6;
-            OptionDataListParser opt_parser(family);
-            opt_parser.parse(options_, param.second);
-            continue;
-        }
-
-        if (param.first == "relay") {
-            RelayInfoParser parser(global_context_->universe_);
-            parser.parse(relay_info_, param.second);
-            continue;
-        }
-
-        ParserPtr parser;
-        // When unsupported parameter is specified, the function called
-        // below will thrown an exception. We have to catch this exception
-        // to append the line number where the parameter is.
-        try {
-            parser.reset(createSubnetConfigParser(param.first));
-
-        } catch (const std::exception& ex) {
-            isc_throw(DhcpConfigError, ex.what() << " ("
-                      << param.second->getPosition() << ")");
-        }
-        parser->build(param.second);
-        parsers_.push_back(parser);
+    ConstElementPtr options_params = subnet->find("option-data");
+    if (options_params) {
+        OptionDataListParser opt_parser(address_family_);
+        opt_parser.parse(options_, options_params);
     }
 
-    // In order to create new subnet we need to get the data out
-    // of the child parsers first. The only way to do it is to
-    // invoke commit on them because it will make them write
-    // parsed data into storages we have supplied.
-    // Note that triggering commits on child parsers does not
-    // affect global data because we supplied pointers to storages
-    // local to this object. Thus, even if this method fails
-    // later on, the configuration remains consistent.
-    BOOST_FOREACH(ParserPtr parser, parsers_) {
-        parser->commit();
+    ConstElementPtr relay_params = subnet->find("relay");
+    if (relay_params) {
+        Option::Universe u = (address_family_ == AF_INET) ? Option::V4 : Option::V6;
+        RelayInfoParser parser(u);
+        parser.parse(relay_info_, relay_params);
     }
 
     // Create a subnet.
@@ -1110,9 +1053,10 @@ SubnetConfigParser::createSubnet(ConstElementPtr params) {
     // need to get all characters preceding "/".
     size_t pos = subnet_txt.find("/");
     if (pos == string::npos) {
+        ConstElementPtr elem = params->find("subnet");
         isc_throw(DhcpConfigError,
                   "Invalid subnet syntax (prefix/len expected):" << subnet_txt
-                  << " (" << string_values_->getPosition("subnet") << ")");
+                  << " (" << elem->getPosition() << ")");
     }
 
     // Try to create the address object. It also validates that
@@ -1136,10 +1080,11 @@ SubnetConfigParser::createSubnet(ConstElementPtr params) {
     std::string iface = getString(params, "interface");
     if (!iface.empty()) {
         if (!IfaceMgr::instance().getIface(iface)) {
+            ConstElementPtr error = params->find("interface");
             isc_throw(DhcpConfigError, "Specified network interface name " << iface
                       << " for subnet " << subnet_->toText()
                       << " is not present" << " in the system ("
-                      << string_values_->getPosition("interface") << ")");
+                      << error->getPosition() << ")");
         }
 
         subnet_->setIface(iface);
@@ -1175,36 +1120,6 @@ SubnetConfigParser::createSubnet(ConstElementPtr params) {
     options_->copyTo(*subnet_->getCfgOption());
 }
 
-isc::dhcp::Triplet<uint32_t>
-SubnetConfigParser::getParam(const std::string& name) {
-    uint32_t value = 0;
-    try {
-        // look for local value
-        value = uint32_values_->getParam(name);
-    } catch (const DhcpConfigError &) {
-        try {
-            // no local, use global value
-            value = global_context_->uint32_values_->getParam(name);
-        } catch (const DhcpConfigError &) {
-            isc_throw(DhcpConfigError, "Mandatory parameter " << name
-                      << " missing (no global default and no subnet-"
-                      << "specific value)");
-        }
-    }
-
-    return (Triplet<uint32_t>(value));
-}
-
-isc::dhcp::Triplet<uint32_t>
-SubnetConfigParser::getOptionalParam(const std::string& name) {
-    try {
-        return (getParam(name));
-    } catch (const DhcpConfigError &) {
-        // No error. We will return an unspecified value.
-    }
-    return (Triplet<uint32_t>());
-}
-
 //**************************** D2ClientConfigParser **********************
 
 uint32_t

+ 4 - 50
src/lib/dhcpsrv/parsers/dhcp_parsers.h

@@ -832,8 +832,8 @@ public:
     ///
     /// @param global_context
     /// @param default_addr default IP address (0.0.0.0 for IPv4, :: for IPv6)
-    SubnetConfigParser(const std::string&, ParserContextPtr global_context,
-                       const isc::asiolink::IOAddress& default_addr);
+    /// @param family address family: @c AF_INET or @c AF_INET6
+    SubnetConfigParser(uint16_t family);
 
     /// @brief virtual destructor (does nothing)
     virtual ~SubnetConfigParser() { }
@@ -850,16 +850,6 @@ protected:
     /// @throw isc::DhcpConfigError if subnet configuration parsing failed.
     SubnetPtr parse(isc::data::ConstElementPtr subnet);
 
-    /// @brief creates parsers for entries in subnet definition
-    ///
-    /// @param config_id name of the entry
-    ///
-    /// @return parser object for specified entry name
-    /// @throw isc::dhcp::DhcpConfigError if trying to create a parser
-    ///        for unknown config element
-    virtual DhcpConfigParser*
-    createSubnetConfigParser(const std::string& config_id) = 0;
-
     /// @brief Instantiates the subnet based on a given IP prefix and prefix
     /// length.
     ///
@@ -869,29 +859,6 @@ protected:
     virtual void initSubnet(isc::data::ConstElementPtr params,
                             isc::asiolink::IOAddress addr, uint8_t len) = 0;
 
-    /// @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
-    /// value if such value was defined in subnet.
-    ///
-    /// @param name name of the parameter
-    /// @return triplet with the parameter name
-    /// @throw DhcpConfigError when requested parameter is not present
-    isc::dhcp::Triplet<uint32_t> getParam(const std::string& name);
-
-    /// @brief Returns optional value for a given parameter.
-    ///
-    /// This method checks if an optional parameter has been specified for
-    /// a subnet. If not, it will try to use a global value. If the global
-    /// value is not specified it will return an object representing an
-    /// unspecified value.
-    ///
-    /// @param name name of the configuration parameter.
-    /// @return An optional value or a @c Triplet object representing
-    /// unspecified value.
-    isc::dhcp::Triplet<uint32_t> getOptionalParam(const std::string& name);
-
     /// @brief Attempts to convert text representation to HRMode enum.
     ///
     /// Allowed values are "disabled", "off" (alias for disabled),
@@ -915,27 +882,14 @@ private:
 
 protected:
 
-    /// Storage for subnet-specific integer values.
-    Uint32StoragePtr uint32_values_;
-
-    /// Storage for subnet-specific string values.
-    StringStoragePtr string_values_;
-
-    /// Storage for subnet-specific boolean values.
-    BooleanStoragePtr boolean_values_;
-
     /// Storage for pools belonging to this subnet.
     PoolStoragePtr pools_;
 
-    /// Parsers are stored here.
-    ParserCollection parsers_;
-
     /// Pointer to the created subnet object.
     isc::dhcp::SubnetPtr subnet_;
 
-    /// Parsing context which contains global values, options and option
-    /// definitions.
-    ParserContextPtr global_context_;
+    /// @brief Address family: @c AF_INET or @c AF_INET6
+    uint16_t address_family_;
 
     /// Pointer to relay information
     isc::dhcp::Subnet::RelayInfoPtr relay_info_;