From 04fa0d3f0b83d544044475cd51de100faf17a410 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Thu, 24 Nov 2016 14:44:00 +0100 Subject: [PATCH 01/15] [5014_phase2] ListElement can now be modified It now contains vector of ElementPtr, rather than ConstElementPtr --- src/lib/cc/data.cc | 19 ++++++++++++------- src/lib/cc/data.h | 25 ++++++++++++++----------- src/lib/cc/tests/data_unittests.cc | 14 +++++++------- 3 files changed, 33 insertions(+), 25 deletions(-) diff --git a/src/lib/cc/data.cc b/src/lib/cc/data.cc index 65d9f6808f..89042b9e2e 100644 --- a/src/lib/cc/data.cc +++ b/src/lib/cc/data.cc @@ -86,7 +86,7 @@ Element::getValue(std::string&) const { } bool -Element::getValue(std::vector&) const { +Element::getValue(std::vector&) const { return (false); } @@ -116,7 +116,7 @@ Element::setValue(const std::string&) { } bool -Element::setValue(const std::vector&) { +Element::setValue(const std::vector&) { return (false); } @@ -130,13 +130,18 @@ Element::get(const int) const { throwTypeError("get(int) called on a non-list Element"); } +ElementPtr +Element::getNonConst(const int) { + throwTypeError("get(int) called on a non-list Element"); +} + void -Element::set(const size_t, ConstElementPtr) { +Element::set(const size_t, ElementPtr) { throwTypeError("set(int, element) called on a non-list Element"); } void -Element::add(ConstElementPtr) { +Element::add(ElementPtr) { throwTypeError("add() called on a non-list Element"); } @@ -507,7 +512,7 @@ fromStringstreamList(std::istream& in, const std::string& file, int& line, { int c = 0; ElementPtr list = Element::createList(Element::Position(file, line, pos)); - ConstElementPtr cur_list_element; + ElementPtr cur_list_element; skipChars(in, WHITESPACE, line, pos); while (c != EOF && c != ']') { @@ -805,8 +810,8 @@ void ListElement::toJSON(std::ostream& ss) const { ss << "[ "; - const std::vector& v = listValue(); - for (std::vector::const_iterator it = v.begin(); + const std::vector& v = listValue(); + for (std::vector::const_iterator it = v.begin(); it != v.end(); ++it) { if (it != v.begin()) { ss << ", "; diff --git a/src/lib/cc/data.h b/src/lib/cc/data.h index 791a555344..f1a08fca40 100644 --- a/src/lib/cc/data.h +++ b/src/lib/cc/data.h @@ -216,7 +216,7 @@ public: { throwTypeError("boolValue() called on non-Bool Element"); }; virtual std::string stringValue() const { throwTypeError("stringValue() called on non-string Element"); }; - virtual const std::vector& listValue() const { + virtual const std::vector& listValue() const { // replace with real exception or empty vector? throwTypeError("listValue() called on non-list Element"); }; @@ -239,7 +239,7 @@ public: virtual bool getValue(double& t) const; virtual bool getValue(bool& t) const; virtual bool getValue(std::string& t) const; - virtual bool getValue(std::vector& t) const; + virtual bool getValue(std::vector& t) const; virtual bool getValue(std::map& t) const; //@} @@ -259,7 +259,7 @@ public: virtual bool setValue(const double v); virtual bool setValue(const bool t); virtual bool setValue(const std::string& v); - virtual bool setValue(const std::vector& v); + virtual bool setValue(const std::vector& v); virtual bool setValue(const std::map& v); //@} @@ -277,15 +277,17 @@ public: /// \param i The position of the ElementPtr to return virtual ConstElementPtr get(const int i) const; + virtual ElementPtr getNonConst(const int i); + /// Sets the ElementPtr at the given index. If the index is out /// of bounds, this function throws an std::out_of_range exception. /// \param i The position of the ElementPtr to set /// \param element The ElementPtr to set at the position - virtual void set(const size_t i, ConstElementPtr element); + virtual void set(const size_t i, ElementPtr element); /// Adds an ElementPtr to the list /// \param element The ElementPtr to add - virtual void add(ConstElementPtr element); + virtual void add(ElementPtr element); /// Removes the element at the given position. If the index is out /// of nothing happens. @@ -603,29 +605,30 @@ public: }; class ListElement : public Element { - std::vector l; + std::vector l; public: ListElement(const Position& pos = ZERO_POSITION()) : Element(list, pos) {} - const std::vector& listValue() const { return (l); } + const std::vector& listValue() const { return (l); } using Element::getValue; - bool getValue(std::vector& t) const { + bool getValue(std::vector& t) const { t = l; return (true); } using Element::setValue; - bool setValue(const std::vector& v) { + bool setValue(const std::vector& v) { l = v; return (true); } using Element::get; ConstElementPtr get(int i) const { return (l.at(i)); } + ElementPtr getNonConst(int i) { return (l.at(i)); } using Element::set; - void set(size_t i, ConstElementPtr e) { + void set(size_t i, ElementPtr e) { l.at(i) = e; } - void add(ConstElementPtr e) { l.push_back(e); }; + void add(ElementPtr e) { l.push_back(e); }; using Element::remove; void remove(int i) { l.erase(l.begin() + i); }; void toJSON(std::ostream& ss) const; diff --git a/src/lib/cc/tests/data_unittests.cc b/src/lib/cc/tests/data_unittests.cc index 6b1a657e5c..1f6333fc97 100644 --- a/src/lib/cc/tests/data_unittests.cc +++ b/src/lib/cc/tests/data_unittests.cc @@ -210,7 +210,7 @@ testGetValueInt() { double d; bool b; std::string s; - std::vector v; + std::vector v; std::map m; el = Element::create(1); @@ -270,7 +270,7 @@ testGetValueDouble() { double d; bool b; std::string s; - std::vector v; + std::vector v; std::map m; el = Element::create(1.1); @@ -297,7 +297,7 @@ testGetValueBool() { double d; bool b; std::string s; - std::vector v; + std::vector v; std::map m; el = Element::create(true); @@ -324,7 +324,7 @@ testGetValueString() { double d; bool b; std::string s; - std::vector v; + std::vector v; std::map m; el = Element::create("foo"); @@ -351,7 +351,7 @@ testGetValueList() { double d; bool b; std::string s; - std::vector v; + std::vector v; std::map m; el = Element::createList(); @@ -378,7 +378,7 @@ testGetValueMap() { double d; bool b; std::string s; - std::vector v; + std::vector v; std::map m; el = Element::createMap(); @@ -406,7 +406,7 @@ TEST(Element, create_and_value_throws) { double d = 0.0; bool b = false; std::string s("asdf"); - std::vector v; + std::vector v; std::map m; ConstElementPtr tmp; From f9abab4bd00e6c5e8feca581aca4f918b5871bb2 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Tue, 29 Nov 2016 20:09:14 +0100 Subject: [PATCH 02/15] [5014_phase2] SimpleParser implemented, 4 parsers converted - SimpleParser concept implemented - Converted 4 parsers (option data, option data list, option defintion, option definition list) - updated unit-tests - converted other parsers (HostReservationParser{4,6}, ClientClassDefParser) to use those new parsers - converted kea-dhcp{4,6} to use those new parsers Conflicts: src/bin/dhcp6/json_config_parser.cc src/lib/dhcpsrv/parsers/dhcp_parsers.cc --- src/bin/dhcp4/json_config_parser.cc | 56 ++-- src/bin/dhcp4/tests/host_options_unittest.cc | 4 +- src/bin/dhcp6/json_config_parser.cc | 70 ++-- src/bin/dhcp6/tests/dhcp6_srv_unittest.cc | 2 + src/bin/dhcp6/tests/host_unittest.cc | 4 +- src/lib/dhcpsrv/Makefile.am | 4 + .../parsers/client_class_def_parser.cc | 10 +- src/lib/dhcpsrv/parsers/dhcp_parsers.cc | 308 ++++++------------ src/lib/dhcpsrv/parsers/dhcp_parsers.h | 202 ++++-------- .../parsers/host_reservation_parser.cc | 16 +- src/lib/dhcpsrv/parsers/simple_parser.cc | 253 ++++++++++++++ src/lib/dhcpsrv/parsers/simple_parser.h | 189 +++++++++++ .../dhcpsrv/tests/dhcp_parsers_unittest.cc | 75 +++-- 13 files changed, 759 insertions(+), 434 deletions(-) create mode 100644 src/lib/dhcpsrv/parsers/simple_parser.cc create mode 100644 src/lib/dhcpsrv/parsers/simple_parser.h diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index 52b554972b..e4edf89027 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -190,8 +191,7 @@ protected: parser = new Pools4ListParser(config_id, pools_); } else if (config_id.compare("relay") == 0) { parser = new RelayInfoParser(config_id, relay_info_, Option::V4); - } else if (config_id.compare("option-data") == 0) { - parser = new OptionDataListParser(config_id, options_, AF_INET); + // 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) { @@ -424,10 +424,7 @@ DhcpConfigParser* createGlobalDhcp4ConfigParser(const std::string& config_id, parser = new IfacesConfigParser4(); } else if (config_id.compare("subnet4") == 0) { parser = new Subnets4ListConfigParser(config_id); - } else if (config_id.compare("option-data") == 0) { - parser = new OptionDataListParser(config_id, CfgOptionPtr(), AF_INET); - } else if (config_id.compare("option-def") == 0) { - parser = new OptionDefListParser(config_id, globalContext()); + // option-data and option-def have been converted to SimpleParser already. } else if ((config_id.compare("version") == 0) || (config_id.compare("next-server") == 0)) { parser = new StringParser(config_id, @@ -538,7 +535,6 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { // Please do not change this order! ParserCollection independent_parsers; ParserPtr subnet_parser; - ParserPtr option_parser; ParserPtr iface_parser; ParserPtr leases_parser; ParserPtr client_classes_parser; @@ -567,10 +563,43 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { // the name of the failing parser can be retrieved in the "catch" clause. ConfigPair config_pair; try { + + // This is a way to convert ConstElementPtr to ElementPtr. + // We need a config that can be edited, because we will insert + // default values and will insert derived values as well. + std::map values; + config_set->getValue(values); + ElementPtr mutable_cfg(new MapElement()); + mutable_cfg->setValue(values); + + // Set all default values if not specified by the user. + SimpleParser::setAllDefaults(mutable_cfg, false); + + // We need definitions first + ConstElementPtr option_defs = mutable_cfg->get("option-def"); + if (option_defs) { + OptionDefListParser parser(AF_INET); + CfgOptionDefPtr cfg_option_def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef(); + parser.parse(cfg_option_def, option_defs); + } + // Make parsers grouping. const std::map& values_map = - config_set->mapValue(); + mutable_cfg->mapValue(); BOOST_FOREACH(config_pair, values_map) { + + if (config_pair.first == "option-def") { + // This is converted to SimpleParser and is handled already above. + continue; + } + + if (config_pair.first == "option-data") { + OptionDataListParser parser(AF_INET); + CfgOptionPtr cfg_option = CfgMgr::instance().getStagingCfg()->getCfgOption(); + parser.parse(cfg_option, config_pair.second); + continue; + } + ParserPtr parser(createGlobalDhcp4ConfigParser(config_pair.first, config_pair.second)); LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PARSER_CREATED) @@ -579,8 +608,6 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { subnet_parser = parser; } else if (config_pair.first == "lease-database") { leases_parser = parser; - } else if (config_pair.first == "option-data") { - option_parser = parser; } else if (config_pair.first == "interfaces-config") { // The interface parser is independent from any other // parser and can be run here before any other parsers. @@ -606,15 +633,6 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { } } - // 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()) { - config_pair.first = "option-data"; - option_parser->build(option_config->second); - option_parser->commit(); - } - // The class definitions parser is the next one to be run. std::map::const_iterator cc_config = values_map.find("client-classes"); diff --git a/src/bin/dhcp4/tests/host_options_unittest.cc b/src/bin/dhcp4/tests/host_options_unittest.cc index b2c86e0660..38dd9c68c8 100644 --- a/src/bin/dhcp4/tests/host_options_unittest.cc +++ b/src/bin/dhcp4/tests/host_options_unittest.cc @@ -203,7 +203,7 @@ const char* HOST_CONFIGS[] = { "\"valid-lifetime\": 600," "\"option-data\": [ {" " \"name\": \"vivso-suboptions\"," - " \"data\": 4491" + " \"data\": \"4491\"" "}," "{" " \"name\": \"tftp-servers\"," @@ -221,7 +221,7 @@ const char* HOST_CONFIGS[] = { " \"ip-address\": \"10.0.0.7\"," " \"option-data\": [ {" " \"name\": \"vivso-suboptions\"," - " \"data\": 4491" + " \"data\": \"4491\"" " }," " {" " \"name\": \"tftp-servers\"," diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index 995390f4cc..aaa0e2a03b 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -184,10 +185,12 @@ public: uint32_values_)); parser = code_parser; } else if (entry == "option-data") { - OptionDataListParserPtr option_parser(new OptionDataListParser(entry, - options_, - AF_INET6)); - parser = option_parser; + OptionDataListParser opts_parser(AF_INET6); + opts_parser.parse(options_, param.second); + + // OptionDataListParser is converted to SimpleParser already, + // no need to go through build/commit phases. + continue; } else if (entry == "user-context") { user_context_ = param.second; continue; // no parser to remember, simply store the value @@ -425,8 +428,7 @@ protected: parser = new RelayInfoParser(config_id, relay_info_, Option::V6); } else if (config_id.compare("pd-pools") == 0) { parser = new PdPoolListParser(config_id, pools_); - } else if (config_id.compare("option-data") == 0) { - parser = new OptionDataListParser(config_id, options_, AF_INET6); + // 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 { @@ -703,11 +705,9 @@ DhcpConfigParser* createGlobal6DhcpConfigParser(const std::string& config_id, parser = new IfacesConfigParser6(); } else if (config_id.compare("subnet6") == 0) { parser = new Subnets6ListConfigParser(config_id); - } else if (config_id.compare("option-data") == 0) { - parser = new OptionDataListParser(config_id, CfgOptionPtr(), AF_INET6); - } else if (config_id.compare("option-def") == 0) { - parser = new OptionDefListParser(config_id, globalContext()); - } else if (config_id.compare("version") == 0) { + // option-data and option-def are no longer needed here. They're now + // converted to SimpleParser and are handled in configureDhcp6Server + } else if (config_id.compare("version") == 0) { parser = new StringParser(config_id, globalContext()->string_values_); } else if (config_id.compare("lease-database") == 0) { @@ -808,7 +808,6 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { // Please do not change this order! ParserCollection independent_parsers; ParserPtr subnet_parser; - ParserPtr option_parser; ParserPtr iface_parser; ParserPtr leases_parser; ParserPtr client_classes_parser; @@ -838,10 +837,42 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { ConfigPair config_pair; try { + // This is a way to convert ConstElementPtr to ElementPtr. + // We need a config that can be edited, because we will insert + // default values and will insert derived values as well. + std::map values; + config_set->getValue(values); + ElementPtr mutable_cfg(new MapElement()); + mutable_cfg->setValue(values); + + SimpleParser::setAllDefaults(mutable_cfg, true); + // Make parsers grouping. const std::map& values_map = - config_set->mapValue(); + mutable_cfg->mapValue(); + + // We need definitions first + ConstElementPtr option_defs = mutable_cfg->get("option-def"); + if (option_defs) { + OptionDefListParser parser(AF_INET6); + CfgOptionDefPtr cfg_option_def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef(); + parser.parse(cfg_option_def, option_defs); + } + BOOST_FOREACH(config_pair, values_map) { + + if (config_pair.first == "option-def") { + // This is converted to SimpleParser and is handled already above. + continue; + } + + if (config_pair.first == "option-data") { + OptionDataListParser parser(AF_INET6); + CfgOptionPtr cfg_option = CfgMgr::instance().getStagingCfg()->getCfgOption(); + parser.parse(cfg_option, config_pair.second); + continue; + } + ParserPtr parser(createGlobal6DhcpConfigParser(config_pair.first, config_pair.second)); LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, DHCP6_PARSER_CREATED) @@ -850,9 +881,9 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { subnet_parser = parser; } else if (config_pair.first == "lease-database") { leases_parser = parser; - } else if (config_pair.first == "option-data") { + } /* else if (config_pair.first == "option-data") { option_parser = parser; - } else if (config_pair.first == "hooks-libraries") { + } */ else if (config_pair.first == "hooks-libraries") { // Executing the commit will alter currently loaded hooks // libraries. Check if the supplied libraries are valid, // but defer the commit until after everything else has @@ -878,15 +909,6 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { } } - // 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()) { - config_pair.first = "option-data"; - option_parser->build(option_config->second); - option_parser->commit(); - } - // The class definitions parser is the next one to be run. std::map::const_iterator cc_config = values_map.find("client-classes"); diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc index b1a08cb7e8..ac73374dd6 100644 --- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc @@ -2024,10 +2024,12 @@ TEST_F(Dhcpv6SrvTest, rsooOverride) { " \"option-def\": [ {" " \"name\": \"foo\"," " \"code\": 120," + " \"csv-format\": false," " \"type\": \"binary\"" " } ]," " \"option-data\": [ {" " \"code\": 120," + " \"csv-format\": false," " \"data\": \"05\"" " } ]," " \"preferred-lifetime\": 3000," diff --git a/src/bin/dhcp6/tests/host_unittest.cc b/src/bin/dhcp6/tests/host_unittest.cc index 4d0bba3971..12440cd7d5 100644 --- a/src/bin/dhcp6/tests/host_unittest.cc +++ b/src/bin/dhcp6/tests/host_unittest.cc @@ -229,7 +229,7 @@ const char* CONFIGS[] = { "\"renew-timer\": 1000, " "\"option-data\": [ {" " \"name\": \"vendor-opts\"," - " \"data\": 4491" + " \"data\": \"4491\"" "}," "{" " \"name\": \"tftp-servers\"," @@ -247,7 +247,7 @@ const char* CONFIGS[] = { " \"ip-addresses\": [ \"2001:db8:1::2\" ]," " \"option-data\": [ {" " \"name\": \"vendor-opts\"," - " \"data\": 4491" + " \"data\": \"4491\"" " }," " {" " \"name\": \"tftp-servers\"," diff --git a/src/lib/dhcpsrv/Makefile.am b/src/lib/dhcpsrv/Makefile.am index 6c8f48bb0f..11bbdf6294 100644 --- a/src/lib/dhcpsrv/Makefile.am +++ b/src/lib/dhcpsrv/Makefile.am @@ -32,6 +32,8 @@ EXTRA_DIST = EXTRA_DIST += parsers/client_class_def_parser.cc EXTRA_DIST += parsers/client_class_def_parser.h EXTRA_DIST += parsers/dhcp_config_parser.h +EXTRA_DIST += parsers/simple_parser.cc +EXTRA_DIST += parsers/simple_parser.h EXTRA_DIST += parsers/dbaccess_parser.cc EXTRA_DIST += parsers/dbaccess_parser.h EXTRA_DIST += parsers/dhcp_parsers.cc @@ -161,6 +163,8 @@ libkea_dhcpsrv_la_SOURCES += parsers/dbaccess_parser.cc libkea_dhcpsrv_la_SOURCES += parsers/dbaccess_parser.h libkea_dhcpsrv_la_SOURCES += parsers/dhcp_parsers.cc libkea_dhcpsrv_la_SOURCES += parsers/dhcp_parsers.h +libkea_dhcpsrv_la_SOURCES += parsers/simple_parser.cc +libkea_dhcpsrv_la_SOURCES += parsers/simple_parser.h libkea_dhcpsrv_la_SOURCES += parsers/duid_config_parser.cc libkea_dhcpsrv_la_SOURCES += parsers/duid_config_parser.h libkea_dhcpsrv_la_SOURCES += parsers/expiration_config_parser.cc diff --git a/src/lib/dhcpsrv/parsers/client_class_def_parser.cc b/src/lib/dhcpsrv/parsers/client_class_def_parser.cc index c939a4eaa4..679f12b922 100644 --- a/src/lib/dhcpsrv/parsers/client_class_def_parser.cc +++ b/src/lib/dhcpsrv/parsers/client_class_def_parser.cc @@ -91,12 +91,16 @@ ClientClassDefParser::build(ConstElementPtr class_def_cfg) { global_context_)); parser = exp_parser; } else if (entry == "option-data") { - OptionDataListParserPtr opts_parser; + uint16_t family = (global_context_->universe_ == Option::V4 ? AF_INET : AF_INET6); - opts_parser.reset(new OptionDataListParser(entry, options_, family)); - parser = opts_parser; + OptionDataListParser opts_parser(family); + opts_parser.parse(options_, param.second); + + // OptionDataListParser is converted to SimpleParser already, + // no need to go through build/commit phases. + continue; } else if (entry == "next-server") { StringParserPtr str_parser(new StringParser(entry, string_values_)); parser = str_parser; diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc index 12f1590e18..f0376013e7 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc @@ -376,75 +376,30 @@ HooksLibrariesParser::getLibraries(isc::hooks::HookLibsCollection& libraries, } // **************************** OptionDataParser ************************* -OptionDataParser::OptionDataParser(const std::string&, const CfgOptionPtr& cfg, - const uint16_t address_family) - : boolean_values_(new BooleanStorage()), - string_values_(new StringStorage()), uint32_values_(new Uint32Storage()), - option_descriptor_(false), cfg_(cfg), - address_family_(address_family) { - // If configuration not specified, then it is a global configuration - // scope. - if (!cfg_) { - cfg_ = CfgMgr::instance().getStagingCfg()->getCfgOption(); - } +OptionDataParser::OptionDataParser(const uint16_t address_family) + : address_family_(address_family) { } -void -OptionDataParser::build(ConstElementPtr option_data_entries) { - BOOST_FOREACH(ConfigPair param, option_data_entries->mapValue()) { - ParserPtr parser; - if (param.first == "name" || param.first == "data" || - param.first == "space") { - StringParserPtr name_parser(new StringParser(param.first, - string_values_)); - parser = name_parser; - } else if (param.first == "code") { - Uint32ParserPtr code_parser(new Uint32Parser(param.first, - uint32_values_)); - parser = code_parser; - } else if (param.first == "csv-format") { - BooleanParserPtr value_parser(new BooleanParser(param.first, - boolean_values_)); - parser = value_parser; - } else { - isc_throw(DhcpConfigError, - "option-data parameter not supported: " << param.first - << " (" << param.second->getPosition() << ")"); - } - - parser->build(param.second); - // Before we can create an option we need to get the data from - // the child parsers. The only way to do it is to invoke commit - // on them so as they store the values in appropriate storages - // that this class provided to them. Note that this will not - // modify values stored in the global storages so the configuration - // will remain consistent even parsing fails somewhere further on. - parser->commit(); - } +std::pair +OptionDataParser::parse(isc::data::ConstElementPtr single_option) { // Try to create the option instance. - createOption(option_data_entries); + std::pair opt = createOption(single_option); - if (!option_descriptor_.option_) { + if (!opt.first.option_) { isc_throw(isc::InvalidOperation, "parser logic error: no option has been configured and" " thus there is nothing to commit. Has build() been called?"); } - cfg_->add(option_descriptor_.option_, option_descriptor_.persistent_, - option_space_); -} - -void -OptionDataParser::commit() { - // Does nothing + return (opt); } OptionalValue OptionDataParser::extractCode(ConstElementPtr parent) const { uint32_t code; try { - code = uint32_values_->getParam("code"); + code = getInteger(parent, "code"); } catch (const exception&) { // The code parameter was not found. Return an unspecified @@ -454,14 +409,14 @@ OptionDataParser::extractCode(ConstElementPtr parent) const { if (code == 0) { isc_throw(DhcpConfigError, "option code must not be zero " - "(" << uint32_values_->getPosition("code", parent) << ")"); + "(" << getPosition("code", parent) << ")"); } else if (address_family_ == AF_INET && code > std::numeric_limits::max()) { isc_throw(DhcpConfigError, "invalid option code '" << code << "', it must not be greater than '" << static_cast(std::numeric_limits::max()) - << "' (" << uint32_values_->getPosition("code", parent) + << "' (" << getPosition("code", parent) << ")"); } else if (address_family_ == AF_INET6 && @@ -469,7 +424,7 @@ OptionDataParser::extractCode(ConstElementPtr parent) const { isc_throw(DhcpConfigError, "invalid option code '" << code << "', it must not exceed '" << std::numeric_limits::max() - << "' (" << uint32_values_->getPosition("code", parent) + << "' (" << getPosition("code", parent) << ")"); } @@ -481,7 +436,7 @@ OptionalValue OptionDataParser::extractName(ConstElementPtr parent) const { std::string name; try { - name = string_values_->getParam("name"); + name = getString(parent, "name"); } catch (...) { return (OptionalValue()); @@ -490,17 +445,17 @@ OptionDataParser::extractName(ConstElementPtr parent) const { if (name.find(" ") != std::string::npos) { isc_throw(DhcpConfigError, "invalid option name '" << name << "', space character is not allowed (" - << string_values_->getPosition("name", parent) << ")"); + << getPosition("name", parent) << ")"); } return (OptionalValue(name, OptionalValueState(true))); } std::string -OptionDataParser::extractData() const { +OptionDataParser::extractData(ConstElementPtr parent) const { std::string data; try { - data = string_values_->getParam("data"); + data = getString(parent, "data"); } catch (...) { // The "data" parameter was not found. Return an empty value. @@ -511,10 +466,10 @@ OptionDataParser::extractData() const { } OptionalValue -OptionDataParser::extractCSVFormat() const { +OptionDataParser::extractCSVFormat(ConstElementPtr parent) const { bool csv_format = true; try { - csv_format = boolean_values_->getParam("csv-format"); + csv_format = getBoolean(parent, "csv-format"); } catch (...) { return (OptionalValue(csv_format)); @@ -524,11 +479,11 @@ OptionDataParser::extractCSVFormat() const { } std::string -OptionDataParser::extractSpace() const { +OptionDataParser::extractSpace(ConstElementPtr parent) const { std::string space = address_family_ == AF_INET ? DHCP4_OPTION_SPACE : DHCP6_OPTION_SPACE; try { - space = string_values_->getParam("space"); + space = getString(parent, "space"); } catch (...) { return (space); @@ -556,7 +511,7 @@ OptionDataParser::extractSpace() const { // should never get here. Therefore, it is ok to call getPosition for // the space parameter here as this parameter will always be specified. isc_throw(DhcpConfigError, ex.what() << " (" - << string_values_->getPosition("space") << ")"); + << getPosition("space", parent) << ")"); } return (space); @@ -588,16 +543,16 @@ OptionDataParser::findOptionDefinition(const std::string& option_space, return (def); } -void +std::pair OptionDataParser::createOption(ConstElementPtr option_data) { const Option::Universe universe = address_family_ == AF_INET ? Option::V4 : Option::V6; OptionalValue code_param = extractCode(option_data); OptionalValue name_param = extractName(option_data); - OptionalValue csv_format_param = extractCSVFormat(); - std::string data_param = extractData(); - std::string space_param = extractSpace(); + OptionalValue csv_format_param = extractCSVFormat(option_data); + std::string data_param = extractData(option_data); + std::string space_param = extractSpace(option_data); // Require that option code or option name is specified. if (!code_param.isSpecified() && !name_param.isSpecified()) { @@ -622,7 +577,7 @@ OptionDataParser::createOption(ConstElementPtr option_data) { << space_param << "." << name_param << "' having code '" << code_param << "' does not exist (" - << string_values_->getPosition("name", option_data) + << getPosition("name", option_data) << ")"); // If there is no option definition and the option code is not specified @@ -631,7 +586,7 @@ OptionDataParser::createOption(ConstElementPtr option_data) { isc_throw(DhcpConfigError, "definition for the option '" << space_param << "." << name_param << "' does not exist (" - << string_values_->getPosition("name", option_data) + << getPosition("name", option_data) << ")"); } } @@ -664,12 +619,14 @@ OptionDataParser::createOption(ConstElementPtr option_data) { isc_throw(DhcpConfigError, "option data is not a valid" << " string of hexadecimal digits: " << data_param << " (" - << string_values_->getPosition("data", option_data) + << getPosition("data", option_data) << ")"); } } OptionPtr option; + OptionDescriptor desc(false); + if (!def) { // @todo We have a limited set of option definitions initalized at // the moment. In the future we want to initialize option definitions @@ -678,13 +635,9 @@ OptionDataParser::createOption(ConstElementPtr option_data) { // ok to create generic option if definition does not exist. OptionPtr option(new Option(universe, static_cast(code_param), binary)); - // The created option is stored in option_descriptor_ class member - // until the commit stage when it is inserted into the main storage. - // If an option with the same code exists in main storage already the - // old option is replaced. - option_descriptor_.option_ = option; - option_descriptor_.persistent_ = false; + desc.option_ = option; + desc.persistent_ = false; } else { // Option name is specified it should match the name in the definition. @@ -693,7 +646,7 @@ OptionDataParser::createOption(ConstElementPtr option_data) { << name_param << "' does not match the " << "option definition: '" << space_param << "." << def->getName() << "' (" - << string_values_->getPosition("name", option_data) + << getPosition("name", option_data) << ")"); } @@ -704,144 +657,65 @@ OptionDataParser::createOption(ConstElementPtr option_data) { !csv_format_param.isSpecified() || csv_format_param ? def->optionFactory(universe, def->getCode(), data_tokens) : def->optionFactory(universe, def->getCode(), binary); - OptionDescriptor desc(option, false); - option_descriptor_.option_ = option; - option_descriptor_.persistent_ = false; - + desc.option_ = option; + desc.persistent_ = false; } catch (const isc::Exception& ex) { isc_throw(DhcpConfigError, "option data does not match" << " option definition (space: " << space_param << ", code: " << def->getCode() << "): " << ex.what() << " (" - << string_values_->getPosition("data", option_data) + << getPosition("data", option_data) << ")"); } } // All went good, so we can set the option space name. - option_space_ = space_param; + return make_pair(desc, space_param); } // **************************** OptionDataListParser ************************* -OptionDataListParser::OptionDataListParser(const std::string&, - const CfgOptionPtr& cfg, +OptionDataListParser::OptionDataListParser(//const std::string&, + //const CfgOptionPtr& cfg, const uint16_t address_family) - : cfg_(cfg), address_family_(address_family) { + : address_family_(address_family) { } -void -OptionDataListParser::build(ConstElementPtr option_data_list) { - BOOST_FOREACH(ConstElementPtr option_value, option_data_list->listValue()) { - boost::shared_ptr - parser(new OptionDataParser("option-data", cfg_, address_family_)); - parser->build(option_value); - parsers_.push_back(parser); - } -} - -void -OptionDataListParser::commit() { - BOOST_FOREACH(ParserPtr parser, parsers_) { - parser->commit(); - } - // Append suboptions to the top-level options - if (cfg_) { - cfg_->encapsulate(); - } else { - CfgMgr::instance().getStagingCfg()->getCfgOption()->encapsulate(); +void OptionDataListParser::parse(const CfgOptionPtr& cfg, + isc::data::ConstElementPtr option_data_list) { + OptionDataParser option_parser(address_family_); + BOOST_FOREACH(ConstElementPtr data, option_data_list->listValue()) { + std::pair option = + option_parser.parse(data); + cfg->add(option.first.option_, option.first.persistent_, option.second); + cfg->encapsulate(); } } // ******************************** OptionDefParser **************************** -OptionDefParser::OptionDefParser(const std::string&, - ParserContextPtr global_context) - : boolean_values_(new BooleanStorage()), - string_values_(new StringStorage()), - uint32_values_(new Uint32Storage()), - global_context_(global_context) { +OptionDefParser::OptionDefParser(const uint16_t address_family) + : address_family_(address_family) { } -void -OptionDefParser::build(ConstElementPtr option_def) { - // Parse the elements that make up the option definition. - BOOST_FOREACH(ConfigPair param, option_def->mapValue()) { - std::string entry(param.first); - ParserPtr parser; - if (entry == "name" || entry == "type" || entry == "record-types" - || entry == "space" || entry == "encapsulate") { - StringParserPtr str_parser(new StringParser(entry, - string_values_)); - parser = str_parser; - } else if (entry == "code") { - Uint32ParserPtr code_parser(new Uint32Parser(entry, - uint32_values_)); - parser = code_parser; - } else if (entry == "array") { - BooleanParserPtr array_parser(new BooleanParser(entry, - boolean_values_)); - parser = array_parser; - } else { - isc_throw(DhcpConfigError, "invalid parameter '" << entry - << "' (" << param.second->getPosition() << ")"); - } +std::pair +OptionDefParser::parse(ConstElementPtr option_def) { - parser->build(param.second); - parser->commit(); - } - // Create an instance of option definition. - createOptionDef(option_def); + // Get mandatory parameters. + std::string name = getString(option_def, "name"); + uint32_t code = getInteger(option_def, "code"); + std::string type = getString(option_def, "type"); - try { - CfgMgr::instance().getStagingCfg()->getCfgOptionDef()-> - add(option_definition_, option_space_name_); - - } catch (const std::exception& ex) { - // Append position if there is a failure. - isc_throw(DhcpConfigError, ex.what() << " (" - << option_def->getPosition() << ")"); - } - - // All definitions have been prepared. Put them as runtime options into - // the libdhcp++. - const OptionDefSpaceContainer& container = - CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->getContainer(); - LibDHCP::setRuntimeOptionDefs(container); -} - -void -OptionDefParser::commit() { - // Do nothing. -} - -void -OptionDefParser::createOptionDef(ConstElementPtr option_def_element) { - // Check if mandatory parameters have been specified. - std::string name; - uint32_t code; - std::string type; - try { - name = string_values_->getParam("name"); - code = uint32_values_->getParam("code"); - type = string_values_->getParam("type"); - } catch (const std::exception& ex) { - isc_throw(DhcpConfigError, ex.what() << " (" - << option_def_element->getPosition() << ")"); - } - - bool array_type = boolean_values_->getOptionalParam("array", false); - std::string record_types = - string_values_->getOptionalParam("record-types", ""); - std::string space = string_values_->getOptionalParam("space", - global_context_->universe_ == Option::V4 ? DHCP4_OPTION_SPACE : - DHCP6_OPTION_SPACE); - std::string encapsulates = - string_values_->getOptionalParam("encapsulate", ""); + // Get optional parameters. Whoever called this parser, should have + // called SimpleParser::setDefaults first. + bool array_type = getBoolean(option_def, "array"); + std::string record_types = getString(option_def, "record-types"); + std::string space = getString(option_def, "space"); + std::string encapsulates = getString(option_def, "encapsulate"); if (!OptionSpace::validateName(space)) { isc_throw(DhcpConfigError, "invalid option space name '" << space << "' (" - << string_values_->getPosition("space") << ")"); + << getPosition("space", option_def) << ")"); } // Create option definition. @@ -854,14 +728,14 @@ OptionDefParser::createOptionDef(ConstElementPtr option_def_element) { isc_throw(DhcpConfigError, "option '" << space << "." << "name" << "', comprising an array of data" << " fields may not encapsulate any option space (" - << option_def_element->getPosition() << ")"); + << option_def->getPosition() << ")"); } else if (encapsulates == space) { isc_throw(DhcpConfigError, "option must not encapsulate" << " an option space it belongs to: '" << space << "." << name << "' is set to" << " encapsulate '" << space << "' (" - << option_def_element->getPosition() << ")"); + << option_def->getPosition() << ")"); } else { def.reset(new OptionDefinition(name, code, type, @@ -888,7 +762,7 @@ OptionDefParser::createOptionDef(ConstElementPtr option_def_element) { isc_throw(DhcpConfigError, "invalid record type values" << " specified for the option definition: " << ex.what() << " (" - << string_values_->getPosition("record-types") << ")"); + << getPosition("record-types", option_def) << ")"); } } @@ -897,38 +771,43 @@ OptionDefParser::createOptionDef(ConstElementPtr option_def_element) { def->validate(); } catch (const std::exception& ex) { isc_throw(DhcpConfigError, ex.what() - << " (" << option_def_element->getPosition() << ")"); + << " (" << option_def->getPosition() << ")"); } // Option definition has been created successfully. - option_space_name_ = space; - option_definition_ = def; + return make_pair(def, space); } // ******************************** OptionDefListParser ************************ -OptionDefListParser::OptionDefListParser(const std::string&, - ParserContextPtr global_context) - : global_context_(global_context) { +OptionDefListParser::OptionDefListParser(const uint16_t address_family) + : address_family_(address_family) { } void -OptionDefListParser::build(ConstElementPtr option_def_list) { +OptionDefListParser::parse(CfgOptionDefPtr storage, ConstElementPtr option_def_list) { if (!option_def_list) { isc_throw(DhcpConfigError, "parser error: a pointer to a list of" << " option definitions is NULL (" << option_def_list->getPosition() << ")"); } + OptionDefParser parser(address_family_); BOOST_FOREACH(ConstElementPtr option_def, option_def_list->listValue()) { - boost::shared_ptr - parser(new OptionDefParser("single-option-def", global_context_)); - parser->build(option_def); - } -} + OptionDefinitionTuple def; -void -OptionDefListParser::commit() { - // Do nothing. + def = parser.parse(option_def); + try { + storage->add(def.first, def.second); + } catch (const std::exception& ex) { + // Append position if there is a failure. + isc_throw(DhcpConfigError, ex.what() << " (" + << option_def->getPosition() << ")"); + } + } + + // All definitions have been prepared. Put them as runtime options into + // the libdhcp++. + LibDHCP::setRuntimeOptionDefs(storage->getContainer()); } //****************************** RelayInfoParser ******************************** @@ -1138,13 +1017,9 @@ PoolParser::build(ConstElementPtr pool_structure) { " address pools"); } - OptionDataListParserPtr option_parser(new OptionDataListParser("option-data", - options_, - address_family_)); - option_parser->build(option_data); - option_parser->commit(); - options_->copyTo(*pool->getCfgOption());; - + CfgOptionPtr cfg = pool->getCfgOption(); + OptionDataListParser option_parser(address_family_); + option_parser.parse(cfg, option_data); } catch (const std::exception& ex) { isc_throw(isc::dhcp::DhcpConfigError, ex.what() << " (" << option_data->getPosition() << ")"); @@ -1194,6 +1069,13 @@ SubnetConfigParser::build(ConstElementPtr subnet) { 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; + } + ParserPtr parser; // When unsupported parameter is specified, the function called // below will thrown an exception. We have to catch this exception diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.h b/src/lib/dhcpsrv/parsers/dhcp_parsers.h index 67f62074c1..f5cbea23b0 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.h +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.h @@ -15,7 +15,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -534,39 +536,30 @@ private: /// an option the configuration will not be accepted. If parsing /// is successful then an instance of an option is created and /// added to the storage provided by the calling class. -class OptionDataParser : public DhcpConfigParser { +class OptionDataParser : public SimpleParser { public: /// @brief Constructor. /// - /// @param dummy first argument is ignored, all Parser constructors - /// accept string as first argument. - /// @param [out] cfg Pointer to the configuration object where parsed option - /// should be stored or NULL if this is a global option. /// @param address_family Address family: @c AF_INET or @c AF_INET6. - /// @throw isc::dhcp::DhcpConfigError if options or global_context are null. - OptionDataParser(const std::string& dummy, const CfgOptionPtr& cfg, - const uint16_t address_family); + OptionDataParser(const uint16_t address_family); - /// @brief Parses the single option data. + /// @brief Parses ElementPtr containing option definition /// - /// This method parses the data of a single option from the configuration. - /// The option data includes option name, option code and data being - /// carried by this option. Eventually it creates the instance of the - /// option and adds it to the Configuration Manager. + /// This method parses ElementPtr containing the option defintion, + /// instantiates the option for it and then returns a pair + /// of option descritor (that holds that new option) and + /// a string that specifies the option space. /// - /// @param option_data_entries collection of entries that define value - /// for a particular option. - /// @throw DhcpConfigError if invalid parameter specified in - /// the configuration. - /// @throw isc::InvalidOperation if failed to set storage prior to - /// calling build. - virtual void build(isc::data::ConstElementPtr option_data_entries); - - /// @brief Does nothing. - virtual void commit(); - - /// @brief virtual destructor to ensure orderly destruction of derivations. - virtual ~OptionDataParser(){}; + /// Note: ElementPtr is expected to contain all fields. If your + /// ElementPtr does not have them, please use + /// @ref SimpleParser::setOptionDefaults to fill the missing fields + /// with default values. + /// + /// @param single_option ElementPtr containing option defintion + /// @return Option object wrapped in option description and an option + /// space + std::pair + parse(isc::data::ConstElementPtr single_option); private: /// @brief Finds an option definition within an option space @@ -594,22 +587,19 @@ private: /// options storage. If the option data parsed by \ref build function /// are invalid or insufficient this function emits an exception. /// - /// @warning this function does not check if options_ storage pointer - /// is intitialized but this check is not needed here because it is done - /// in the \ref build function. - /// /// @param option_data An element holding data for a single option being /// created. /// + /// @return created option descriptor + /// /// @throw DhcpConfigError if parameters provided in the configuration /// are invalid. - void createOption(isc::data::ConstElementPtr option_data); + std::pair + createOption(isc::data::ConstElementPtr option_data); /// @brief Retrieves parsed option code as an optional value. /// /// @param parent A data element holding full option data configuration. - /// It is used here to log a position if the element holding a code - /// is not specified and its position is therefore unavailable. /// /// @return Option code, possibly unspecified. /// @throw DhcpConfigError if option code is invalid. @@ -619,8 +609,6 @@ private: /// @brief Retrieves parsed option name as an optional value. /// /// @param parent A data element holding full option data configuration. - /// It is used here to log a position if the element holding a name - /// is not specified and its position is therefore unavailable. /// /// @return Option name, possibly unspecified. /// @throw DhcpConfigError if option name is invalid. @@ -630,13 +618,14 @@ private: /// @brief Retrieves csv-format parameter as an optional value. /// /// @return Value of the csv-format parameter, possibly unspecified. - util::OptionalValue extractCSVFormat() const; + util::OptionalValue extractCSVFormat(data::ConstElementPtr parent) const; /// @brief Retrieves option data as a string. /// + /// @param parent A data element holding full option data configuration. /// @return Option data as a string. It will return empty string if /// option data is unspecified. - std::string extractData() const; + std::string extractData(data::ConstElementPtr parent) const; /// @brief Retrieves option space name. /// @@ -644,27 +633,10 @@ private: /// 'dhcp4' or 'dhcp6' option space name is returned, depending on /// the universe specified in the parser context. /// + /// @param parent A data element holding full option data configuration. + /// /// @return Option space name. - std::string extractSpace() const; - - /// Storage for boolean values. - BooleanStoragePtr boolean_values_; - - /// Storage for string values (e.g. option name or data). - StringStoragePtr string_values_; - - /// Storage for uint32 values (e.g. option code). - Uint32StoragePtr uint32_values_; - - /// Option descriptor holds newly configured option. - OptionDescriptor option_descriptor_; - - /// Option space name where the option belongs to. - std::string option_space_; - - /// @brief Configuration holding option being parsed or NULL if the option - /// is global. - CfgOptionPtr cfg_; + std::string extractSpace(data::ConstElementPtr parent) const; /// @brief Address family: @c AF_INET or @c AF_INET6. uint16_t address_family_; @@ -680,97 +652,52 @@ typedef OptionDataParser *OptionDataParserFactory(const std::string&, /// data for a particular subnet and creates a collection of options. /// If parsing is successful, all these options are added to the Subnet /// object. -class OptionDataListParser : public DhcpConfigParser { +class OptionDataListParser : public SimpleParser { public: /// @brief Constructor. /// - /// @param dummy nominally would be param name, this is always ignored. - /// @param [out] cfg Pointer to the configuration object where options - /// should be stored or NULL if this is global option scope. /// @param address_family Address family: @c AF_INET or AF_INET6 - OptionDataListParser(const std::string& dummy, const CfgOptionPtr& cfg, - const uint16_t address_family); + OptionDataListParser(const uint16_t address_family); - /// @brief Parses entries that define options' data for a subnet. + /// @brief Parses a list of options, instantiates them and stores in cfg /// - /// This method iterates over all entries that define option data - /// for options within a single subnet and creates options' instances. + /// This method expects to get a list of options in option_data_list, + /// iterates over them, creates option objects, wraps them with + /// option descriptor and stores in specified cfg. /// - /// @param option_data_list pointer to a list of options' data sets. - /// @throw DhcpConfigError if option parsing failed. - void build(isc::data::ConstElementPtr option_data_list); - - /// @brief Commit all option values. - /// - /// This function invokes commit for all option values - /// and append suboptions to the top-level options. - void commit(); - + /// @param cfg created options will be stored here + /// @param option_data_list configuration that describes the options + void parse(const CfgOptionPtr& cfg, + isc::data::ConstElementPtr option_data_list); private: - - /// Collection of parsers; - ParserCollection parsers_; - - /// @brief Pointer to a configuration where options are stored. - CfgOptionPtr cfg_; - /// @brief Address family: @c AF_INET or @c AF_INET6 uint16_t address_family_; - }; -typedef boost::shared_ptr OptionDataListParserPtr; - +typedef std::pair OptionDefinitionTuple; /// @brief Parser for a single option definition. /// /// This parser creates an instance of a single option definition. -class OptionDefParser : public DhcpConfigParser { +class OptionDefParser : public SimpleParser { public: /// @brief Constructor. /// - /// @param dummy first argument is ignored, all Parser constructors - /// accept string as first argument. - /// @param global_context is a pointer to the global context which - /// stores global scope parameters, options, option defintions. - OptionDefParser(const std::string& dummy, ParserContextPtr global_context); + /// @param address_family Address family: @c AF_INET or AF_INET6 + OptionDefParser(const uint16_t address_family); /// @brief Parses an entry that describes single option definition. /// /// @param option_def a configuration entry to be parsed. + /// @return tuple (option definition, option space) of the parsed structure /// /// @throw DhcpConfigError if parsing was unsuccessful. - void build(isc::data::ConstElementPtr option_def); - - /// @brief Stores the parsed option definition in a storage. - void commit(); + OptionDefinitionTuple + parse(isc::data::ConstElementPtr option_def); private: - - /// @brief Create option definition from the parsed parameters. - /// - /// @param option_def_element A data element holding the configuration - /// for an option definition. - void createOptionDef(isc::data::ConstElementPtr option_def_element); - - /// 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_; - - /// Storage for boolean values. - BooleanStoragePtr boolean_values_; - - /// Storage for string values. - StringStoragePtr string_values_; - - /// Storage for uint32 values. - Uint32StoragePtr uint32_values_; - - /// 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_; }; /// @brief Parser for a list of option definitions. @@ -779,7 +706,7 @@ private: /// option definitions and creates instances of these definitions. /// If the parsing is successful, the collection of created definitions /// is put into the provided storage. -class OptionDefListParser : public DhcpConfigParser { +class OptionDefListParser : public SimpleParser { public: /// @brief Constructor. /// @@ -787,29 +714,24 @@ public: /// accept string as first argument. /// @param global_context is a pointer to the global context which /// stores global scope parameters, options, option defintions. - OptionDefListParser(const std::string& dummy, - ParserContextPtr global_context); + OptionDefListParser(const uint16_t address_family); + //const std::string& dummy, + //ParserContextPtr global_context); - /// @brief Parse configuration entries. + /// @brief Parses a list of option defintions, create them and store in cfg /// - /// This function parses configuration entries, creates instances - /// of option definitions and tries to add them to the Configuration - /// Manager. + /// This method iterates over def_list, which is a JSON list of option defintions, + /// then creates corresponding option defintions and store them in the + /// configuration structure. /// - /// @param option_def_list pointer to an element that holds entries - /// that define option definitions. - /// @throw DhcpConfigError if configuration parsing fails. - void build(isc::data::ConstElementPtr option_def_list); + /// @param def_list JSON list describing option definitions + /// @param cfg parsed option definitions will be stored here + void parse(CfgOptionDefPtr cfg, isc::data::ConstElementPtr def_list); - /// @brief Commits option definitions. - /// - /// Currently this function is no-op, because option definitions are - /// added to the Configuration Manager in the @c build method. - void commit(); +protected: - /// 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_; }; /// @brief a collection of pools diff --git a/src/lib/dhcpsrv/parsers/host_reservation_parser.cc b/src/lib/dhcpsrv/parsers/host_reservation_parser.cc index 7a4055ee1c..49c046edb3 100644 --- a/src/lib/dhcpsrv/parsers/host_reservation_parser.cc +++ b/src/lib/dhcpsrv/parsers/host_reservation_parser.cc @@ -196,8 +196,12 @@ HostReservationParser4::build(isc::data::ConstElementPtr reservation_data) { // surround it with try-catch. if (element.first == "option-data") { CfgOptionPtr cfg_option = host_->getCfgOption4(); - OptionDataListParser parser(element.first, cfg_option, AF_INET); - parser.build(element.second); + + // This parser is converted to SimpleParser already. It + // parses the Element structure immediately, there's no need + // to go through build/commit phases. + OptionDataListParser parser(AF_INET); + parser.parse(cfg_option, element.second); // Everything else should be surrounded with try-catch to append // position. @@ -255,8 +259,12 @@ HostReservationParser6::build(isc::data::ConstElementPtr reservation_data) { // appended). if (element.first == "option-data") { CfgOptionPtr cfg_option = host_->getCfgOption6(); - OptionDataListParser parser(element.first, cfg_option, AF_INET6); - parser.build(element.second); + + // This parser is converted to SimpleParser already. It + // parses the Element structure immediately, there's no need + // to go through build/commit phases. + OptionDataListParser parser(AF_INET6); + parser.parse(cfg_option, element.second); } else if (element.first == "ip-addresses" || element.first == "prefixes") { BOOST_FOREACH(ConstElementPtr prefix_element, diff --git a/src/lib/dhcpsrv/parsers/simple_parser.cc b/src/lib/dhcpsrv/parsers/simple_parser.cc new file mode 100644 index 0000000000..9ae9d3d942 --- /dev/null +++ b/src/lib/dhcpsrv/parsers/simple_parser.cc @@ -0,0 +1,253 @@ +#include +#include +#include +#include +#include + +using namespace std; +using namespace isc::data; + +namespace isc { +namespace dhcp { + +/// This table defines default values for option definitions in DHCPv4 +const SimpleDefaults OPTION4_DEF_DEFAULTS{ + { "record-types", Element::string, ""}, + { "space", Element::string, "dhcp4"}, + { "array", Element::boolean, "false"}, + { "encapsulate", Element::string, "" } +}; + +/// This table defines default values for option definitions in DHCPv6 +const SimpleDefaults OPTION6_DEF_DEFAULTS{ + { "record-types", Element::string, ""}, + { "space", Element::string, "dhcp6"}, + { "array", Element::boolean, "false"}, + { "encapsulate", Element::string, "" } +}; + +/// This table defines default values for options in DHCPv4 +const SimpleDefaults OPTION4_DEFAULTS({ + { "space", Element::string, "dhcp4"}, + { "csv-format", Element::boolean, "true"}, + { "encapsulate", Element::string, "" } +}); + +/// This table defines default values for options in DHCPv6 +const SimpleDefaults OPTION6_DEFAULTS({ + { "space", Element::string, "dhcp6"}, + { "csv-format", Element::boolean, "true"}, + { "encapsulate", Element::string, "" } +}); + +/// This table defines default values for both DHCPv4 and DHCPv6 +const SimpleDefaults GLOBAL_DEFAULTS = { + { "renew-timer", Element::integer, "900" }, + { "rebind-timer", Element::integer, "1800" }, + { "preferred-lifetime", Element::integer, "3600" }, + { "valid-lifetime", Element::integer, "7200" } +}; + +std::string +SimpleParser::getString(isc::data::ConstElementPtr scope, const std::string& name) { + ConstElementPtr x = scope->get(name); + if (!x) { + isc_throw(BadValue, "Element " << name << " not found"); + } + if (x->getType() != Element::string) { + isc_throw(BadValue, "Element " << name << " found, but is not a string"); + } + + return (x->stringValue()); +} + +int64_t +SimpleParser::getInteger(isc::data::ConstElementPtr scope, const std::string& name) { + ConstElementPtr x = scope->get(name); + if (!x) { + isc_throw(BadValue, "Element " << name << " not found"); + } + if (x->getType() != Element::integer) { + isc_throw(BadValue, "Element " << name << " found, but is not an integer"); + } + + return (x->intValue()); +} + +bool +SimpleParser::getBoolean(isc::data::ConstElementPtr scope, const std::string& name) { + ConstElementPtr x = scope->get(name); + if (!x) { + isc_throw(BadValue, "Element " << name << " not found"); + } + if (x->getType() != Element::boolean) { + isc_throw(BadValue, "Element " << name << " found, but is not a boolean"); + } + + return (x->boolValue()); +} + +const data::Element::Position& +SimpleParser::getPosition(const std::string& name, const data::ConstElementPtr parent) const { + if (!parent) { + return (data::Element::ZERO_POSITION()); + } + ConstElementPtr elem = parent->get(name); + if (!elem) { + return (data::Element::ZERO_POSITION()); + } + return (elem->getPosition()); +} + +size_t SimpleParser::setDefaults(isc::data::ElementPtr scope, + const SimpleDefaults& default_values) { + size_t cnt = 0; + + // This is the position representing a default value. As the values + // we're inserting here are not present in whatever the config file + // came from, we need to make sure it's clearly labeled as default. + const Element::Position pos("", 0, 0); + + // Let's go over all parameters we have defaults for. + BOOST_FOREACH(SimpleDefault def_value, default_values) { + + // Try if such a parameter is there. If it is, let's + // skip it, because user knows best *cough*. + ConstElementPtr x = scope->get(string(def_value.name_)); + if (x) { + // There is such a value already, skip it. + continue; + } + + // There isn't such a value defined, let's create the default + // value... + switch (def_value.type_) { + case Element::string: { + x.reset(new StringElement(def_value.value_, pos)); + break; + } + case Element::integer: { + int int_value = boost::lexical_cast(def_value.value_); + x.reset(new IntElement(int_value, pos)); + break; + } + case Element::boolean: { + bool bool_value; + if (def_value.value_ == string("true")) { + bool_value = true; + } else if (def_value.value_ == string("false")) { + bool_value = false; + } else { + isc_throw(BadValue, "Internal error. Boolean value specified as " + << def_value.value_ << ", expected true or false"); + } + x.reset(new BoolElement(bool_value, pos)); + break; + } + case Element::real: { + double dbl_value = boost::lexical_cast(def_value.value_); + x.reset(new DoubleElement(dbl_value, pos)); + break; + } + default: + // No default values for null, list or map + isc_throw(BadValue, "Internal error. Incorrect default value type."); + } + + // ... and insert it into the provided Element tree. + scope->set(def_value.name_, x); + cnt++; + } + + return (cnt); +} + +size_t SimpleParser::setGlobalDefaults(isc::data::ElementPtr global) { + return (setDefaults(global, GLOBAL_DEFAULTS)); +} + +size_t SimpleParser::setOptionDefaults(isc::data::ElementPtr option, bool v6) { + return (setDefaults(option, v6?OPTION6_DEFAULTS : OPTION4_DEFAULTS)); +} + +size_t SimpleParser::setOptionListDefaults(isc::data::ElementPtr option_list, bool v6) { + size_t cnt = 0; + BOOST_FOREACH(ElementPtr single_option, option_list->listValue()) { + cnt += setOptionDefaults(single_option, v6); + } + return (cnt); +} + +size_t SimpleParser::setOptionDefDefaults(isc::data::ElementPtr option_def, bool v6) { + return (setDefaults(option_def, v6? OPTION6_DEF_DEFAULTS : OPTION4_DEF_DEFAULTS)); +} + +size_t SimpleParser::setOptionDefListDefaults(isc::data::ElementPtr option_def_list, + bool v6) { + size_t cnt = 0; + BOOST_FOREACH(ElementPtr single_def, option_def_list->listValue()) { + cnt += setOptionDefDefaults(single_def, v6); + } + return (cnt); +} + + +size_t SimpleParser::setAllDefaults(isc::data::ElementPtr global, bool v6) { + size_t cnt = 0; + + // Set global defaults first. + //cnt = setGlobalDefaults(global); + + // Now set option defintion defaults for each specified option definition + ConstElementPtr option_defs = global->get("option-def"); + if (option_defs) { + BOOST_FOREACH(ElementPtr single_def, option_defs->listValue()) { + cnt += setOptionDefDefaults(single_def, v6); + } + } + + ConstElementPtr options = global->get("option-data"); + if (options) { + BOOST_FOREACH(ElementPtr single_option, options->listValue()) { + cnt += setOptionDefaults(single_option, v6); + } + //setOptionListDefaults(options); + } + + return (cnt); +} + +size_t +SimpleParser::deriveParams(isc::data::ConstElementPtr parent, + isc::data::ElementPtr child, + const ParamsList& params) { + if ( (parent->getType() != Element::map) || + (child->getType() != Element::map)) { + return (0); + } + + size_t cnt = 0; + BOOST_FOREACH(string param, params) { + ConstElementPtr x = parent->get(param); + if (!x) { + // Parent doesn't define this parameter, so there's + // nothing to derive from + continue; + } + + if (child->get(param)) { + // Child defines this parameter already. There's + // nothing to do here. + continue; + } + + // Copy the parameters to the child scope. + child->set(param, x); + cnt++; + } + + return (cnt); +} + +}; // end of isc::dhcp namespace +}; // end of isc namespace diff --git a/src/lib/dhcpsrv/parsers/simple_parser.h b/src/lib/dhcpsrv/parsers/simple_parser.h new file mode 100644 index 0000000000..14fc804d52 --- /dev/null +++ b/src/lib/dhcpsrv/parsers/simple_parser.h @@ -0,0 +1,189 @@ +#ifndef SIMPLE_PARSER_H +#define SIMPLE_PARSER_H + +#include +#include +#include + +namespace isc { +namespace dhcp { + +/// This array defines a single entry of default values +struct SimpleDefault { + SimpleDefault(const char* name, isc::data::Element::types type, const char* value) + :name_(name), type_(type), value_(value) {} + std::string name_; + const isc::data::Element::types type_; + const char* value_; +}; + +/// This specifies all default values in a given scope (e.g. a subnet) +typedef std::vector SimpleDefaults; + +/// This defines a list of all parameters that are derived (or inherited) between +/// contexts +typedef std::vector ParamsList; + + +/// @brief A simple parser +/// +/// This class is intended to be a simpler replacement for @ref DhcpConfigParser. +/// The simplification comes from several factors: +/// - no build/commit nonsense. There's a single step: +/// CfgStorage parse(ConstElementPtr json) +/// that converts JSON configuration into an object and returns it. +/// - almost no state kept. The only state kept in most cases is whether the +/// parsing is done in v4 or v6 context. This greatly simplifies the +/// parsers (no contexts, no child parsers list, no separate storage for +/// uint32, strings etc. In fact, there's so little state kept, that this +/// parser is mostly a collection of static methods. +/// - no optional parameters (all are mandatory). This simplifies the parser, +/// but introduces a new step before parsing where we insert the default +/// values into client configuration before parsing. This is actually a good +/// thing, because we now have a clear picture of the default parameters as +/// they're defined in a single place (the DhcpConfigParser had the defaults +/// spread out in multiple files in multiple directories). +class SimpleParser { + public: + + /// @brief Derives (inherits) parameters from parent scope to a child + /// + /// This method derives parameters from the parent scope to the child, + /// if there are no values specified in the child scope. For example, + /// this method can be used to derive timers from global scope (e.g. for + /// the whole DHCPv6 server) to a subnet scope. This method checks + /// if the child scope doesn't have more specific values defined. If + /// it doesn't, then the value from parent scope is copied over. + /// + /// @param parent scope to copy from (e.g. global) + /// @param child scope to copy from (e.g. subnet) + /// @param params names of the parameters to copy + /// @return number of parameters copied + static size_t deriveParams(isc::data::ConstElementPtr parent, + isc::data::ElementPtr child, + const ParamsList& params); + + /// @brief Sets the default values + /// + /// This method sets the default values for parameters that are not + /// defined. The list of default values is specified by default_values. + /// If not present, those will be inserted into the scope. If + /// a parameter is already present, the default value will not + /// be inserted. + /// + /// @param scope default values will be inserted here + /// @param default_values list of default values + /// @return number of parameters inserted + static size_t setDefaults(isc::data::ElementPtr scope, + const SimpleDefaults& default_values); + + /// @brief Sets global defaults + /// + /// This method sets global defaults. Note it does not set the + /// defaults for any scopes lower than global. See @ref setAllDefaults + /// for that. + /// + /// @param global scope to be filled in with defaults. + /// @return number of default values added + static size_t setGlobalDefaults(isc::data::ElementPtr global); + + /// @brief Sets option defaults for a single option + /// + /// This method sets default values for a single option. + /// + /// @param option an option data to be filled in with defaults. + /// @param v6 is it v6 (true) or v4 (false) option? + /// @return number of default values added + static size_t setOptionDefaults(isc::data::ElementPtr option, bool v6); + + /// @brief Sets option defaults for the whole options list + /// + /// This method sets default values for a list of options. + /// + /// @param option_list an option data to be filled in with defaults. + /// @param v6 is it v6 (true) or v4 (false) option? + /// @return number of default values added + static size_t setOptionListDefaults(isc::data::ElementPtr option_list, bool v6); + + /// @brief Sets option defaults for a single option definition + /// + /// This method sets default values for a single option definition. + /// + /// @param option_def an option defintion to be filled in with defaults. + /// @param v6 is it v6 (true) or v4 (false) option defintion? + /// @return number of default values added + static size_t setOptionDefDefaults(isc::data::ElementPtr option_def, bool v6); + + /// @brief Sets option defaults for the whole option definitions list + /// + /// This method sets default values for a list of option definitions. + /// + /// @param option_def_list a list of option defintions to be filled in with defaults. + /// @param v6 is it v6 (true) or v4 (false) option? + /// @return number of default values added + static size_t setOptionDefListDefaults(isc::data::ElementPtr option_def_list, + bool v6); + + /// @brief Sets defaults for the whole configuration + /// + /// This method sets global, options and option definition defaults. Note + /// it does set the defaults for scopes lower than global. If you want to + /// set the global defaults only, see @ref setGlobalDefaults for that. + /// + /// @param global scope to be filled in with defaults. + /// @return number of default values added + static size_t setAllDefaults(isc::data::ElementPtr global, bool v6); + + /// @brief Utility method that returns position of an element + /// + /// It's mostly useful for logging. + /// + /// @param name position of that element will be returned + /// @param parent parent element (optional) + /// @return position of the element specified. + const data::Element::Position& + getPosition(const std::string& name, const data::ConstElementPtr parent = + data::ConstElementPtr()) const; + + /// @destructor + /// + /// It's really simple, isn't it? + virtual ~SimpleParser() {} + + protected: + + /// @brief Returns a string parameter from a scope + /// + /// Unconditionally returns a parameter. If the parameter is not there or + /// is not of appropriate type, BadValue exception is thrown. + /// + /// @param scope specified parameter will be extracted from this scope + /// @param name name of the parameter + /// @return a string value of the parameter + static std::string getString(isc::data::ConstElementPtr scope, const std::string& name); + + /// @brief Returns an integer parameter from a scope + /// + /// Unconditionally returns a parameter. If the parameter is not there or + /// is not of appropriate type, BadValue exception is thrown. + /// + /// @param scope specified parameter will be extracted from this scope + /// @param name name of the parameter + /// @return an integer value of the parameter + static int64_t getInteger(isc::data::ConstElementPtr scope, const std::string& name); + + /// @brief Returns a boolean parameter from a scope + /// + /// Unconditionally returns a parameter. If the parameter is not there or + /// is not of appropriate type, BadValue exception is thrown. + /// + /// @param scope specified parameter will be extracted from this scope + /// @param name name of the parameter + /// @return a boolean value of the parameter + static bool getBoolean(isc::data::ConstElementPtr scope, const std::string& name); +}; + +}; +}; + +#endif diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index eeb7c96b87..77e03f4f94 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -292,7 +293,7 @@ public: /// @return returns an ConstElementPtr containing the numeric result /// code and outcome comment. isc::data::ConstElementPtr parseElementSet(isc::data::ConstElementPtr - config_set) { + config_set) { // Answer will hold the result. ConstElementPtr answer; if (!config_set) { @@ -310,6 +311,16 @@ public: const std::map& values_map = config_set->mapValue(); BOOST_FOREACH(config_pair, values_map) { + + // These are the simple parsers. No need to go through + // the ParserPtr hooplas with them. + if ( (config_pair.first == "option-data") || + (config_pair.first == "option-def")) { + continue; + } + // Remaining ones are old style parsers. Need go do + // the build/commit dance with them. + // Create the parser based on element name. ParserPtr parser(createConfigParser(config_pair.first)); // Options must be parsed last @@ -322,12 +333,27 @@ public: } } + int family = parser_context_->universe_ == Option::V4 + ? AF_INET : AF_INET6; + + // The option definition parser is the next one to be run. + std::map::const_iterator + def_config = values_map.find("option-def"); + if (def_config != values_map.end()) { + + CfgOptionDefPtr cfg_def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef(); + OptionDefListParser def_list_parser(family); + def_list_parser.parse(cfg_def, def_config->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(); + CfgOptionPtr cfg_option = CfgMgr::instance().getStagingCfg()->getCfgOption(); + + OptionDataListParser option_list_parser(family); + option_list_parser.parse(cfg_option, option_config->second); } // Everything was fine. Configuration is successful. @@ -359,17 +385,9 @@ public: /// @throw throws NotImplemented if element name isn't supported. ParserPtr createConfigParser(const std::string& config_id) { ParserPtr parser; - int family = parser_context_->universe_ == Option::V4 ? - AF_INET : AF_INET6; - if (config_id.compare("option-data") == 0) { - parser.reset(new OptionDataListParser(config_id, CfgOptionPtr(), - family)); - - } else if (config_id.compare("option-def") == 0) { - parser.reset(new OptionDefListParser(config_id, - parser_context_)); - - } else if (config_id.compare("hooks-libraries") == 0) { + // option-data and option-def converted to SimpleParser, so they + // are no longer here. + if (config_id.compare("hooks-libraries") == 0) { parser.reset(new HooksLibrariesParser(config_id)); hooks_libraries_parser_ = boost::dynamic_pointer_cast(parser); @@ -392,13 +410,15 @@ public: /// /// @return retuns 0 if the configuration parsed successfully, /// non-zero otherwise failure. - int parseConfiguration(const std::string& config) { + int parseConfiguration(const std::string& config, bool v6 = false) { int rcode_ = 1; // Turn config into elements. // Test json just to make sure its valid. ElementPtr json = Element::fromJSON(config); EXPECT_TRUE(json); if (json) { + SimpleParser::setAllDefaults(json, v6); + ConstElementPtr status = parseElementSet(json); ConstElementPtr comment = parseAnswer(rcode_, status); error_text_ = comment->stringValue(); @@ -566,7 +586,7 @@ TEST_F(ParseConfigTest, defaultSpaceOptionDefTest) { "}"; // Verify that the configuration string parses. - int rcode = parseConfiguration(config); + int rcode = parseConfiguration(config, true); ASSERT_EQ(0, rcode); @@ -828,7 +848,7 @@ TEST_F(ParseConfigTest, optionDataCSVFormatNoOptionDef) { " } ]" "}"; int rcode = 0; - ASSERT_NO_THROW(rcode = parseConfiguration(config)); + ASSERT_NO_THROW(rcode = parseConfiguration(config, true)); EXPECT_NE(0, rcode); CfgMgr::instance().clear(); @@ -844,7 +864,7 @@ TEST_F(ParseConfigTest, optionDataCSVFormatNoOptionDef) { " \"data\": \"0\"" " } ]" "}"; - ASSERT_NO_THROW(rcode = parseConfiguration(config)); + ASSERT_NO_THROW(rcode = parseConfiguration(config, true)); EXPECT_NE(0, rcode); CfgMgr::instance().clear(); @@ -859,7 +879,7 @@ TEST_F(ParseConfigTest, optionDataCSVFormatNoOptionDef) { " \"data\": \"0\"" " } ]" "}"; - ASSERT_NO_THROW(rcode = parseConfiguration(config)); + ASSERT_NO_THROW(rcode = parseConfiguration(config, true)); ASSERT_EQ(0, rcode); OptionPtr opt = getOptionPtr(DHCP6_OPTION_SPACE, 25000); ASSERT_TRUE(opt); @@ -875,10 +895,11 @@ TEST_F(ParseConfigTest, optionDataCSVFormatNoOptionDef) { " \"name\": \"foo-name\"," " \"space\": \"dhcp6\"," " \"code\": 25000," + " \"csv-format\": false," " \"data\": \"123456\"" " } ]" "}"; - ASSERT_NO_THROW(rcode = parseConfiguration(config)); + ASSERT_NO_THROW(rcode = parseConfiguration(config, true)); EXPECT_EQ(0, rcode); opt = getOptionPtr(DHCP6_OPTION_SPACE, 25000); ASSERT_TRUE(opt); @@ -899,7 +920,7 @@ TEST_F(ParseConfigTest, optionDataNoName) { " } ]" "}"; int rcode = 0; - ASSERT_NO_THROW(rcode = parseConfiguration(config)); + ASSERT_NO_THROW(rcode = parseConfiguration(config, true)); EXPECT_EQ(0, rcode); Option6AddrLstPtr opt = boost::dynamic_pointer_cast< Option6AddrLst>(getOptionPtr(DHCP6_OPTION_SPACE, 23)); @@ -919,7 +940,7 @@ TEST_F(ParseConfigTest, optionDataNoCode) { " } ]" "}"; int rcode = 0; - ASSERT_NO_THROW(rcode = parseConfiguration(config)); + ASSERT_NO_THROW(rcode = parseConfiguration(config, true)); EXPECT_EQ(0, rcode); Option6AddrLstPtr opt = boost::dynamic_pointer_cast< Option6AddrLst>(getOptionPtr(DHCP6_OPTION_SPACE, 23)); @@ -938,7 +959,7 @@ TEST_F(ParseConfigTest, optionDataMinimal) { " } ]" "}"; int rcode = 0; - ASSERT_NO_THROW(rcode = parseConfiguration(config)); + ASSERT_NO_THROW(rcode = parseConfiguration(config, true)); EXPECT_EQ(0, rcode); Option6AddrLstPtr opt = boost::dynamic_pointer_cast< Option6AddrLst>(getOptionPtr(DHCP6_OPTION_SPACE, 23)); @@ -955,7 +976,7 @@ TEST_F(ParseConfigTest, optionDataMinimal) { " } ]" "}"; rcode = 0; - ASSERT_NO_THROW(rcode = parseConfiguration(config)); + ASSERT_NO_THROW(rcode = parseConfiguration(config, true)); EXPECT_EQ(0, rcode); opt = boost::dynamic_pointer_cast(getOptionPtr(DHCP6_OPTION_SPACE, 23)); @@ -984,7 +1005,7 @@ TEST_F(ParseConfigTest, optionDataMinimalWithOptionDef) { "}"; int rcode = 0; - ASSERT_NO_THROW(rcode = parseConfiguration(config)); + ASSERT_NO_THROW(rcode = parseConfiguration(config, true)); EXPECT_EQ(0, rcode); Option6AddrLstPtr opt = boost::dynamic_pointer_cast< Option6AddrLst>(getOptionPtr(DHCP6_OPTION_SPACE, 2345)); @@ -1010,7 +1031,7 @@ TEST_F(ParseConfigTest, optionDataMinimalWithOptionDef) { "}"; rcode = 0; - ASSERT_NO_THROW(rcode = parseConfiguration(config)); + ASSERT_NO_THROW(rcode = parseConfiguration(config, true)); EXPECT_EQ(0, rcode); opt = boost::dynamic_pointer_cast(getOptionPtr(DHCP6_OPTION_SPACE, 2345)); @@ -1031,7 +1052,7 @@ TEST_F(ParseConfigTest, emptyOptionData) { "}"; int rcode = 0; - ASSERT_NO_THROW(rcode = parseConfiguration(config)); + ASSERT_NO_THROW(rcode = parseConfiguration(config, true)); EXPECT_EQ(0, rcode); const Option6AddrLstPtr opt = boost::dynamic_pointer_cast< Option6AddrLst>(getOptionPtr(DHCP6_OPTION_SPACE, D6O_DHCPV4_O_DHCPV6_SERVER)); From a529c56dadfe0fafed86258a8942f54f987dc228 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Sat, 26 Nov 2016 08:05:59 +0100 Subject: [PATCH 03/15] [5014_phase2] Easier C++11 stuff --- src/lib/dhcpsrv/parsers/simple_parser.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lib/dhcpsrv/parsers/simple_parser.cc b/src/lib/dhcpsrv/parsers/simple_parser.cc index 9ae9d3d942..5a88fca9cb 100644 --- a/src/lib/dhcpsrv/parsers/simple_parser.cc +++ b/src/lib/dhcpsrv/parsers/simple_parser.cc @@ -11,7 +11,7 @@ namespace isc { namespace dhcp { /// This table defines default values for option definitions in DHCPv4 -const SimpleDefaults OPTION4_DEF_DEFAULTS{ +const SimpleDefaults OPTION4_DEF_DEFAULTS = { { "record-types", Element::string, ""}, { "space", Element::string, "dhcp4"}, { "array", Element::boolean, "false"}, @@ -19,7 +19,7 @@ const SimpleDefaults OPTION4_DEF_DEFAULTS{ }; /// This table defines default values for option definitions in DHCPv6 -const SimpleDefaults OPTION6_DEF_DEFAULTS{ +const SimpleDefaults OPTION6_DEF_DEFAULTS = { { "record-types", Element::string, ""}, { "space", Element::string, "dhcp6"}, { "array", Element::boolean, "false"}, @@ -27,18 +27,18 @@ const SimpleDefaults OPTION6_DEF_DEFAULTS{ }; /// This table defines default values for options in DHCPv4 -const SimpleDefaults OPTION4_DEFAULTS({ +const SimpleDefaults OPTION4_DEFAULTS = { { "space", Element::string, "dhcp4"}, { "csv-format", Element::boolean, "true"}, { "encapsulate", Element::string, "" } -}); +}; /// This table defines default values for options in DHCPv6 -const SimpleDefaults OPTION6_DEFAULTS({ +const SimpleDefaults OPTION6_DEFAULTS = { { "space", Element::string, "dhcp6"}, { "csv-format", Element::boolean, "true"}, { "encapsulate", Element::string, "" } -}); +}; /// This table defines default values for both DHCPv4 and DHCPv6 const SimpleDefaults GLOBAL_DEFAULTS = { From 0bbc1df7d05e0434fa3918ac514793966f322a23 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Mon, 12 Dec 2016 20:28:27 +0100 Subject: [PATCH 04/15] [5039] Parameters inheritance implemented, doc updated --- src/lib/dhcpsrv/Makefile.am | 3 +- src/lib/dhcpsrv/libdhcpsrv.dox | 88 ++++++++++++++++++ src/lib/dhcpsrv/parsers/dhcp_parsers.h | 10 +- src/lib/dhcpsrv/parsers/simple_parser.cc | 31 ++++++- src/lib/dhcpsrv/parsers/simple_parser.h | 17 +++- .../dhcpsrv/tests/dhcp_parsers_unittest.cc | 93 +++++++++++++++++++ 6 files changed, 229 insertions(+), 13 deletions(-) diff --git a/src/lib/dhcpsrv/Makefile.am b/src/lib/dhcpsrv/Makefile.am index 11bbdf6294..289b9e5f71 100644 --- a/src/lib/dhcpsrv/Makefile.am +++ b/src/lib/dhcpsrv/Makefile.am @@ -20,7 +20,8 @@ if HAVE_CQL AM_CPPFLAGS += $(CQL_CPPFLAGS) endif -AM_CXXFLAGS = $(KEA_CXXFLAGS) +# This is a temporary workaround until we have Kea-wide support for C++11. +AM_CXXFLAGS = $(KEA_CXXFLAGS) -std=c++11 # The files in the subfolder must be explicitly specified here so # as they are copied to the distribution. The other option would diff --git a/src/lib/dhcpsrv/libdhcpsrv.dox b/src/lib/dhcpsrv/libdhcpsrv.dox index 71b5c6cc39..1c6a394618 100644 --- a/src/lib/dhcpsrv/libdhcpsrv.dox +++ b/src/lib/dhcpsrv/libdhcpsrv.dox @@ -476,4 +476,92 @@ is used by DHCPv4 and DHCPv6 components. DHCPv4-over-DHCPv6 which are relayed by a DHCPv6 relay are not yet supported. +@section dhcp6SimpleParser Simple JSON Parser + +Since the early beginnings, our configuration parsing code was a mess. It +started back in 2011 when Tomek joined ISC recently and was told to implement +Kea configuration handling in similar way as DNS Auth module. The code grew +over time (DHCP configuration is significantly more complex than DNS, with +more interdependent values) and as of Kea 1.1 release it became very difficult +to manage. The decision has been made to significantly refactor or even +partially rewrite the parser code. The design for this effort is documented +here: http://kea.isc.org/wiki/SimpleParser It discusses the original issues +and the proposed architecture. + +There are several aspects of this new approach. The base class for all parsers +is @ref isc::dhcp::SimpleParser. It simplifies the parser be rejecting the +concept of build/commit phases. Instead, there should be a single method +called parse that takes ConstElementPtr as a single parameter (that's the +JSON structures to be parsed) and returns the config structure to be used +in CfgMgr. An example of such a method can be the following: + +@code +std::pair +OptionDataParser::parse(isc::data::ConstElementPtr single_option) +@endcode + +Since each derived class will have the same parameter, but a different return +type, it's not possible to use virtual methods mechanism. That's perfectly +ok, though, as there is only a single instance of the class needed to parse +arbitrary number of parameters of the same type, so there is no need to +keep pointers to the parser object. As such there's fewer incentives to have +one generic way to handle all parsers. + +@subsection dhcp6SimpleParserDefaults Default values in Simple Parser + +Another simplification comes from the fact that almost all parameters +are mandatory in SimpleParser. One source of complexities in the old +parser was the necessity to deal with optional parameters. Simple +parser deals with that by explicitly requiring the input structure to +have all parameters filled. Obviously, it's not feasible to expect +everyone to always specify all parameters, therefore there's an easy +way to fill missing parameters with their default values. There are +several methods to do this, but the most generic one is: + +@code +static size_t +isc::dhcp::SimpleParser::setDefaults(isc::data::ElementPtr scope, + const SimpleDefaults& default_values); +@endcode + +It takes a pointer to element to be filled with default values and +vector of default values. Having those values specified in a single +place in a way that can easily be read even by non-programmers is a +big advantage of this approach. Here's an example from simple_parser.cc file: + +@code +/// This table defines default values for option definitions in DHCPv6 +const SimpleDefaults OPTION6_DEF_DEFAULTS = { + { "record-types", Element::string, ""}, + { "space", Element::string, "dhcp6"}, + { "array", Element::boolean, "false"}, + { "encapsulate", Element::string, "" } +}; +@endcode + +This array (which technically is implemented as a vector and +initialized the C++11 way) can be passed to the aforementioned +setDefaults. That code will iterate over all default values and see if +there are explicit values provided. If not, the gaps will be filled +with default values. There are also convenience methods specified for +filling in option data defaults, option definition defaults and +setAllDefaults that sets all defaults (starts with global, but then +walks down the Element tree and fills defaults in subsequent scopes). + +@subsection dhcp6SimpleParserInherits Inheriting parameters between scopes + +SimpleParser provides a mechanism to inherit parameters between scopes, +e.g. to inherit global parameters in the subnet scope if more specific +values are not defined in the subnet scope. This is achieved by calling +@code +static size_t SimpleParser::deriveParams(isc::data::ConstElementPtr parent, + isc::data::ElementPtr child, + const ParamsList& params); + +@endcode + +ParamsList is a simple vector. There will be more specific +methods implemented in the future, but for the time being only +@ref isc::dhcp::SimpleParser::inheritGlobalToSubnet is implemented. + */ diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.h b/src/lib/dhcpsrv/parsers/dhcp_parsers.h index f5cbea23b0..df829dc1db 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.h +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.h @@ -710,13 +710,11 @@ class OptionDefListParser : public SimpleParser { public: /// @brief Constructor. /// - /// @param dummy first argument is ignored, all Parser constructors - /// accept string as first argument. - /// @param global_context is a pointer to the global context which - /// stores global scope parameters, options, option defintions. + /// Stores address family that will be used to make certain decisions + /// during parsing. + /// + /// @param address_family @c AF_INET or @c AF_INET6 OptionDefListParser(const uint16_t address_family); - //const std::string& dummy, - //ParserContextPtr global_context); /// @brief Parses a list of option defintions, create them and store in cfg /// diff --git a/src/lib/dhcpsrv/parsers/simple_parser.cc b/src/lib/dhcpsrv/parsers/simple_parser.cc index 5a88fca9cb..e40ba28f45 100644 --- a/src/lib/dhcpsrv/parsers/simple_parser.cc +++ b/src/lib/dhcpsrv/parsers/simple_parser.cc @@ -40,14 +40,30 @@ const SimpleDefaults OPTION6_DEFAULTS = { { "encapsulate", Element::string, "" } }; +/// This table defines default values for DHCPv4 +const SimpleDefaults GLOBAL4_DEFAULTS = { + { "renew-timer", Element::integer, "900" }, + { "rebind-timer", Element::integer, "1800" }, + { "valid-lifetime", Element::integer, "7200" } +}; + /// This table defines default values for both DHCPv4 and DHCPv6 -const SimpleDefaults GLOBAL_DEFAULTS = { +const SimpleDefaults GLOBAL6_DEFAULTS = { { "renew-timer", Element::integer, "900" }, { "rebind-timer", Element::integer, "1800" }, { "preferred-lifetime", Element::integer, "3600" }, { "valid-lifetime", Element::integer, "7200" } }; +/// This list defines parameters that can be inherited from the global +/// scope to subnet scope. +const ParamsList INHERIT_GLOBAL_TO_SUBNET = { + "renew-timer", + "rebind-timer", + "preferred-lifetime", + "valid-lifetime" +}; + std::string SimpleParser::getString(isc::data::ConstElementPtr scope, const std::string& name) { ConstElementPtr x = scope->get(name); @@ -162,8 +178,8 @@ size_t SimpleParser::setDefaults(isc::data::ElementPtr scope, return (cnt); } -size_t SimpleParser::setGlobalDefaults(isc::data::ElementPtr global) { - return (setDefaults(global, GLOBAL_DEFAULTS)); +size_t SimpleParser::setGlobalDefaults(isc::data::ElementPtr global, bool v6) { + return (setDefaults(global, v6 ? GLOBAL6_DEFAULTS : GLOBAL4_DEFAULTS)); } size_t SimpleParser::setOptionDefaults(isc::data::ElementPtr option, bool v6) { @@ -196,7 +212,8 @@ size_t SimpleParser::setAllDefaults(isc::data::ElementPtr global, bool v6) { size_t cnt = 0; // Set global defaults first. - //cnt = setGlobalDefaults(global); + /// @todo: Uncomment as part of the ticket 5019 work. + //cnt = setGlobalDefaults(global, v6); // Now set option defintion defaults for each specified option definition ConstElementPtr option_defs = global->get("option-def"); @@ -249,5 +266,11 @@ SimpleParser::deriveParams(isc::data::ConstElementPtr parent, return (cnt); } +size_t +SimpleParser::inheritGlobalToSubnet(isc::data::ConstElementPtr global, + isc::data::ElementPtr subnet) { + return deriveParams(global, subnet, INHERIT_GLOBAL_TO_SUBNET); +} + }; // end of isc::dhcp namespace }; // end of isc namespace diff --git a/src/lib/dhcpsrv/parsers/simple_parser.h b/src/lib/dhcpsrv/parsers/simple_parser.h index 14fc804d52..16fe5c6e3e 100644 --- a/src/lib/dhcpsrv/parsers/simple_parser.h +++ b/src/lib/dhcpsrv/parsers/simple_parser.h @@ -62,6 +62,17 @@ class SimpleParser { static size_t deriveParams(isc::data::ConstElementPtr parent, isc::data::ElementPtr child, const ParamsList& params); + /// @brief Derives (inherits) parameters from global to subnet scope + /// + /// This method derives global parameters into subnet scope if they're + /// not defined there yet. + /// + /// @param global parameters will be inherited from here + /// @param subnet parameters will be inserted here + /// @return number of parameters copied + static size_t + inheritGlobalToSubnet(isc::data::ConstElementPtr global, + isc::data::ElementPtr subnet); /// @brief Sets the default values /// @@ -84,8 +95,9 @@ class SimpleParser { /// for that. /// /// @param global scope to be filled in with defaults. + /// @param v6 is it v6 (true) or v4 (false) option? /// @return number of default values added - static size_t setGlobalDefaults(isc::data::ElementPtr global); + static size_t setGlobalDefaults(isc::data::ElementPtr global, bool v6); /// @brief Sets option defaults for a single option /// @@ -131,6 +143,7 @@ class SimpleParser { /// set the global defaults only, see @ref setGlobalDefaults for that. /// /// @param global scope to be filled in with defaults. + /// @param v6 true = v6, false = v4 /// @return number of default values added static size_t setAllDefaults(isc::data::ElementPtr global, bool v6); @@ -145,7 +158,7 @@ class SimpleParser { getPosition(const std::string& name, const data::ConstElementPtr parent = data::ConstElementPtr()) const; - /// @destructor + /// Destructor /// /// It's really simple, isn't it? virtual ~SimpleParser() {} diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index 77e03f4f94..b9d5a76322 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -318,6 +318,16 @@ public: (config_pair.first == "option-def")) { continue; } + + // We also don't care about the default values that may be been + // inserted + if ( (config_pair.first == "preferred-lifetime") || + (config_pair.first == "valid-lifetime") || + (config_pair.first == "renew-timer") || + (config_pair.first == "rebind-timer")) { + continue; + } + // Remaining ones are old style parsers. Need go do // the build/commit dance with them. @@ -477,6 +487,28 @@ public: CfgMgr::instance().setD2ClientConfig(tmp); } + /// @brief Checks if specified map has an integer parameter with expected value + /// + /// @param map map to be checked + /// @param param_name name of the parameter to be checked + /// @param exp_value expected value of the parameter. + void checkIntegerValue(const ConstElementPtr& map, const std::string& param_name, + int64_t exp_value) { + + // First check if the passed element is a map. + ASSERT_EQ(Element::map, map->getType()); + + // Now try to get the element being checked + ConstElementPtr elem = map->get(param_name); + ASSERT_TRUE(elem); + + // Now check if it's indeed integer + ASSERT_EQ(Element::integer, elem->getType()); + + // Finally, check if its value meets expectation. + EXPECT_EQ(exp_value, elem->intValue()); + } + /// @brief Parsers used in the parsing of the configuration /// /// Allows the tests to interrogate the state of the parsers (if required). @@ -2393,4 +2425,65 @@ TEST_F(ParseConfigTest, validRelayInfo6) { // (see CtrlDhcpv4SrvTest.commandSocketBasic in // src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc). +// This test checks if global defaults are properly set for DHCPv6. +TEST_F(ParseConfigTest, globalDefaults6) { + + ElementPtr empty = Element::fromJSON("{ }"); + size_t num; + + EXPECT_NO_THROW(num = SimpleParser::setGlobalDefaults(empty, true)); + + // We expect at least 4 parameters to be inserted. + EXPECT_TRUE(num >= 4); + + checkIntegerValue(empty, "valid-lifetime", 7200); + checkIntegerValue(empty, "preferred-lifetime", 3600); + checkIntegerValue(empty, "rebind-timer", 1800); + checkIntegerValue(empty, "renew-timer", 900); +} + +// This test checks if global defaults are properly set for DHCPv4. +TEST_F(ParseConfigTest, DISABLED_globalDefaults4) { + + ElementPtr empty = Element::fromJSON("{ }"); + size_t num; + + EXPECT_NO_THROW(num = SimpleParser::setGlobalDefaults(empty, false)); + + // We expect at least 3 parameters to be inserted. + EXPECT_TRUE(num >= 3); + + checkIntegerValue(empty, "valid-lifetime", 7200); + checkIntegerValue(empty, "rebind-timer", 1800); + checkIntegerValue(empty, "renew-timer", 900); + + // Make sure that preferred-lifetime is not set for v4 (it's v6 only + // parameter) + EXPECT_FALSE(empty->get("preferred-lifetime")); +} + +// This test checks if the parameters can be inherited from the global +// scope to the subnet scope. +TEST_F(ParseConfigTest, inheritGlobalToSubnet) { + ElementPtr global = Element::fromJSON("{ \"renew-timer\": 1," + " \"rebind-timer\": 2," + " \"preferred-lifetime\": 3," + " \"valid-lifetime\": 4" + "}"); + ElementPtr subnet = Element::fromJSON("{ \"renew-timer\": 100 }"); + + // we should inherit 3 parameters. Renew-timer should remain intact, + // as it was already defined in the subnet scope. + size_t num; + EXPECT_NO_THROW(num = SimpleParser::inheritGlobalToSubnet(global, subnet)); + EXPECT_EQ(3, num); + + // Check the values. 3 of them are interited, while the fourth one + // was already defined in the subnet, so should not be inherited. + checkIntegerValue(subnet, "renew-timer", 100); + checkIntegerValue(subnet, "rebind-timer", 2); + checkIntegerValue(subnet, "preferred-lifetime", 3); + checkIntegerValue(subnet, "valid-lifetime", 4); +} + }; // Anonymous namespace From c623d19f6dfcdb28b356b567588d3d7da3f92b33 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Thu, 22 Dec 2016 11:55:55 +0100 Subject: [PATCH 05/15] [5039] Simple parser moved to lib/cc --- src/lib/{dhcpsrv/parsers => cc}/simple_parser.cc | 0 src/lib/{dhcpsrv/parsers => cc}/simple_parser.h | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename src/lib/{dhcpsrv/parsers => cc}/simple_parser.cc (100%) rename src/lib/{dhcpsrv/parsers => cc}/simple_parser.h (100%) diff --git a/src/lib/dhcpsrv/parsers/simple_parser.cc b/src/lib/cc/simple_parser.cc similarity index 100% rename from src/lib/dhcpsrv/parsers/simple_parser.cc rename to src/lib/cc/simple_parser.cc diff --git a/src/lib/dhcpsrv/parsers/simple_parser.h b/src/lib/cc/simple_parser.h similarity index 100% rename from src/lib/dhcpsrv/parsers/simple_parser.h rename to src/lib/cc/simple_parser.h From d352a45e2ba969c1be20c37e95c640f081bf25a4 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Thu, 22 Dec 2016 11:57:32 +0100 Subject: [PATCH 06/15] [5039] Unit-tests for SimpleParser added. --- src/lib/cc/Makefile.am | 1 + src/lib/cc/simple_parser.cc | 129 +++----------------- src/lib/cc/simple_parser.h | 97 +++------------ src/lib/cc/tests/Makefile.am | 3 +- src/lib/cc/tests/simple_parser_unittest.cc | 132 +++++++++++++++++++++ src/lib/dhcpsrv/Makefile.am | 4 - 6 files changed, 166 insertions(+), 200 deletions(-) create mode 100644 src/lib/cc/tests/simple_parser_unittest.cc diff --git a/src/lib/cc/Makefile.am b/src/lib/cc/Makefile.am index 8a734761f8..1b057bd8a5 100644 --- a/src/lib/cc/Makefile.am +++ b/src/lib/cc/Makefile.am @@ -7,6 +7,7 @@ AM_CXXFLAGS = $(KEA_CXXFLAGS) lib_LTLIBRARIES = libkea-cc.la libkea_cc_la_SOURCES = data.cc data.h libkea_cc_la_SOURCES += command_interpreter.cc command_interpreter.h +libkea_cc_la_SOURCES += simple_parser.cc simple_parser.h libkea_cc_la_LIBADD = $(top_builddir)/src/lib/exceptions/libkea-exceptions.la libkea_cc_la_LIBADD += $(BOOST_LIBS) diff --git a/src/lib/cc/simple_parser.cc b/src/lib/cc/simple_parser.cc index e40ba28f45..aeaeb469bb 100644 --- a/src/lib/cc/simple_parser.cc +++ b/src/lib/cc/simple_parser.cc @@ -1,68 +1,19 @@ -#include +// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include #include #include #include #include using namespace std; -using namespace isc::data; namespace isc { -namespace dhcp { - -/// This table defines default values for option definitions in DHCPv4 -const SimpleDefaults OPTION4_DEF_DEFAULTS = { - { "record-types", Element::string, ""}, - { "space", Element::string, "dhcp4"}, - { "array", Element::boolean, "false"}, - { "encapsulate", Element::string, "" } -}; - -/// This table defines default values for option definitions in DHCPv6 -const SimpleDefaults OPTION6_DEF_DEFAULTS = { - { "record-types", Element::string, ""}, - { "space", Element::string, "dhcp6"}, - { "array", Element::boolean, "false"}, - { "encapsulate", Element::string, "" } -}; - -/// This table defines default values for options in DHCPv4 -const SimpleDefaults OPTION4_DEFAULTS = { - { "space", Element::string, "dhcp4"}, - { "csv-format", Element::boolean, "true"}, - { "encapsulate", Element::string, "" } -}; - -/// This table defines default values for options in DHCPv6 -const SimpleDefaults OPTION6_DEFAULTS = { - { "space", Element::string, "dhcp6"}, - { "csv-format", Element::boolean, "true"}, - { "encapsulate", Element::string, "" } -}; - -/// This table defines default values for DHCPv4 -const SimpleDefaults GLOBAL4_DEFAULTS = { - { "renew-timer", Element::integer, "900" }, - { "rebind-timer", Element::integer, "1800" }, - { "valid-lifetime", Element::integer, "7200" } -}; - -/// This table defines default values for both DHCPv4 and DHCPv6 -const SimpleDefaults GLOBAL6_DEFAULTS = { - { "renew-timer", Element::integer, "900" }, - { "rebind-timer", Element::integer, "1800" }, - { "preferred-lifetime", Element::integer, "3600" }, - { "valid-lifetime", Element::integer, "7200" } -}; - -/// This list defines parameters that can be inherited from the global -/// scope to subnet scope. -const ParamsList INHERIT_GLOBAL_TO_SUBNET = { - "renew-timer", - "rebind-timer", - "preferred-lifetime", - "valid-lifetime" -}; +namespace data { std::string SimpleParser::getString(isc::data::ConstElementPtr scope, const std::string& name) { @@ -104,7 +55,7 @@ SimpleParser::getBoolean(isc::data::ConstElementPtr scope, const std::string& na } const data::Element::Position& -SimpleParser::getPosition(const std::string& name, const data::ConstElementPtr parent) const { +SimpleParser::getPosition(const std::string& name, const data::ConstElementPtr parent) { if (!parent) { return (data::Element::ZERO_POSITION()); } @@ -178,62 +129,16 @@ size_t SimpleParser::setDefaults(isc::data::ElementPtr scope, return (cnt); } -size_t SimpleParser::setGlobalDefaults(isc::data::ElementPtr global, bool v6) { - return (setDefaults(global, v6 ? GLOBAL6_DEFAULTS : GLOBAL4_DEFAULTS)); -} - -size_t SimpleParser::setOptionDefaults(isc::data::ElementPtr option, bool v6) { - return (setDefaults(option, v6?OPTION6_DEFAULTS : OPTION4_DEFAULTS)); -} - -size_t SimpleParser::setOptionListDefaults(isc::data::ElementPtr option_list, bool v6) { +size_t +SimpleParser::setListDefaults(isc::data::ElementPtr list, + const SimpleDefaults& default_values) { size_t cnt = 0; - BOOST_FOREACH(ElementPtr single_option, option_list->listValue()) { - cnt += setOptionDefaults(single_option, v6); + BOOST_FOREACH(ElementPtr entry, list->listValue()) { + cnt += setDefaults(entry, default_values); } return (cnt); } -size_t SimpleParser::setOptionDefDefaults(isc::data::ElementPtr option_def, bool v6) { - return (setDefaults(option_def, v6? OPTION6_DEF_DEFAULTS : OPTION4_DEF_DEFAULTS)); -} - -size_t SimpleParser::setOptionDefListDefaults(isc::data::ElementPtr option_def_list, - bool v6) { - size_t cnt = 0; - BOOST_FOREACH(ElementPtr single_def, option_def_list->listValue()) { - cnt += setOptionDefDefaults(single_def, v6); - } - return (cnt); -} - - -size_t SimpleParser::setAllDefaults(isc::data::ElementPtr global, bool v6) { - size_t cnt = 0; - - // Set global defaults first. - /// @todo: Uncomment as part of the ticket 5019 work. - //cnt = setGlobalDefaults(global, v6); - - // Now set option defintion defaults for each specified option definition - ConstElementPtr option_defs = global->get("option-def"); - if (option_defs) { - BOOST_FOREACH(ElementPtr single_def, option_defs->listValue()) { - cnt += setOptionDefDefaults(single_def, v6); - } - } - - ConstElementPtr options = global->get("option-data"); - if (options) { - BOOST_FOREACH(ElementPtr single_option, options->listValue()) { - cnt += setOptionDefaults(single_option, v6); - } - //setOptionListDefaults(options); - } - - return (cnt); -} - size_t SimpleParser::deriveParams(isc::data::ConstElementPtr parent, isc::data::ElementPtr child, @@ -266,11 +171,5 @@ SimpleParser::deriveParams(isc::data::ConstElementPtr parent, return (cnt); } -size_t -SimpleParser::inheritGlobalToSubnet(isc::data::ConstElementPtr global, - isc::data::ElementPtr subnet) { - return deriveParams(global, subnet, INHERIT_GLOBAL_TO_SUBNET); -} - }; // end of isc::dhcp namespace }; // end of isc namespace diff --git a/src/lib/cc/simple_parser.h b/src/lib/cc/simple_parser.h index 16fe5c6e3e..af4dd63aa9 100644 --- a/src/lib/cc/simple_parser.h +++ b/src/lib/cc/simple_parser.h @@ -1,3 +1,9 @@ +// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + #ifndef SIMPLE_PARSER_H #define SIMPLE_PARSER_H @@ -6,7 +12,7 @@ #include namespace isc { -namespace dhcp { +namespace data { /// This array defines a single entry of default values struct SimpleDefault { @@ -62,17 +68,6 @@ class SimpleParser { static size_t deriveParams(isc::data::ConstElementPtr parent, isc::data::ElementPtr child, const ParamsList& params); - /// @brief Derives (inherits) parameters from global to subnet scope - /// - /// This method derives global parameters into subnet scope if they're - /// not defined there yet. - /// - /// @param global parameters will be inherited from here - /// @param subnet parameters will be inserted here - /// @return number of parameters copied - static size_t - inheritGlobalToSubnet(isc::data::ConstElementPtr global, - isc::data::ElementPtr subnet); /// @brief Sets the default values /// @@ -88,64 +83,8 @@ class SimpleParser { static size_t setDefaults(isc::data::ElementPtr scope, const SimpleDefaults& default_values); - /// @brief Sets global defaults - /// - /// This method sets global defaults. Note it does not set the - /// defaults for any scopes lower than global. See @ref setAllDefaults - /// for that. - /// - /// @param global scope to be filled in with defaults. - /// @param v6 is it v6 (true) or v4 (false) option? - /// @return number of default values added - static size_t setGlobalDefaults(isc::data::ElementPtr global, bool v6); - - /// @brief Sets option defaults for a single option - /// - /// This method sets default values for a single option. - /// - /// @param option an option data to be filled in with defaults. - /// @param v6 is it v6 (true) or v4 (false) option? - /// @return number of default values added - static size_t setOptionDefaults(isc::data::ElementPtr option, bool v6); - - /// @brief Sets option defaults for the whole options list - /// - /// This method sets default values for a list of options. - /// - /// @param option_list an option data to be filled in with defaults. - /// @param v6 is it v6 (true) or v4 (false) option? - /// @return number of default values added - static size_t setOptionListDefaults(isc::data::ElementPtr option_list, bool v6); - - /// @brief Sets option defaults for a single option definition - /// - /// This method sets default values for a single option definition. - /// - /// @param option_def an option defintion to be filled in with defaults. - /// @param v6 is it v6 (true) or v4 (false) option defintion? - /// @return number of default values added - static size_t setOptionDefDefaults(isc::data::ElementPtr option_def, bool v6); - - /// @brief Sets option defaults for the whole option definitions list - /// - /// This method sets default values for a list of option definitions. - /// - /// @param option_def_list a list of option defintions to be filled in with defaults. - /// @param v6 is it v6 (true) or v4 (false) option? - /// @return number of default values added - static size_t setOptionDefListDefaults(isc::data::ElementPtr option_def_list, - bool v6); - - /// @brief Sets defaults for the whole configuration - /// - /// This method sets global, options and option definition defaults. Note - /// it does set the defaults for scopes lower than global. If you want to - /// set the global defaults only, see @ref setGlobalDefaults for that. - /// - /// @param global scope to be filled in with defaults. - /// @param v6 true = v6, false = v4 - /// @return number of default values added - static size_t setAllDefaults(isc::data::ElementPtr global, bool v6); + static size_t setListDefaults(isc::data::ElementPtr list, + const SimpleDefaults& default_values); /// @brief Utility method that returns position of an element /// @@ -154,14 +93,9 @@ class SimpleParser { /// @param name position of that element will be returned /// @param parent parent element (optional) /// @return position of the element specified. - const data::Element::Position& + static const data::Element::Position& getPosition(const std::string& name, const data::ConstElementPtr parent = - data::ConstElementPtr()) const; - - /// Destructor - /// - /// It's really simple, isn't it? - virtual ~SimpleParser() {} + data::ConstElementPtr()); protected: @@ -173,7 +107,8 @@ class SimpleParser { /// @param scope specified parameter will be extracted from this scope /// @param name name of the parameter /// @return a string value of the parameter - static std::string getString(isc::data::ConstElementPtr scope, const std::string& name); + static std::string getString(isc::data::ConstElementPtr scope, + const std::string& name); /// @brief Returns an integer parameter from a scope /// @@ -183,7 +118,8 @@ class SimpleParser { /// @param scope specified parameter will be extracted from this scope /// @param name name of the parameter /// @return an integer value of the parameter - static int64_t getInteger(isc::data::ConstElementPtr scope, const std::string& name); + static int64_t getInteger(isc::data::ConstElementPtr scope, + const std::string& name); /// @brief Returns a boolean parameter from a scope /// @@ -193,7 +129,8 @@ class SimpleParser { /// @param scope specified parameter will be extracted from this scope /// @param name name of the parameter /// @return a boolean value of the parameter - static bool getBoolean(isc::data::ConstElementPtr scope, const std::string& name); + static bool getBoolean(isc::data::ConstElementPtr scope, + const std::string& name); }; }; diff --git a/src/lib/cc/tests/Makefile.am b/src/lib/cc/tests/Makefile.am index 67d85bab26..81358f6a92 100644 --- a/src/lib/cc/tests/Makefile.am +++ b/src/lib/cc/tests/Makefile.am @@ -1,6 +1,6 @@ AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib AM_CPPFLAGS += $(BOOST_INCLUDES) -AM_CXXFLAGS = $(KEA_CXXFLAGS) +AM_CXXFLAGS = $(KEA_CXXFLAGS) -std=c++11 if USE_STATIC_LINK AM_LDFLAGS = -static @@ -16,6 +16,7 @@ if HAVE_GTEST TESTS += run_unittests run_unittests_SOURCES = command_interpreter_unittests.cc data_unittests.cc run_unittests_SOURCES += data_file_unittests.cc run_unittests.cc +run_unittests_SOURCES += simple_parser_unittest.cc run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS) diff --git a/src/lib/cc/tests/simple_parser_unittest.cc b/src/lib/cc/tests/simple_parser_unittest.cc new file mode 100644 index 0000000000..1726306321 --- /dev/null +++ b/src/lib/cc/tests/simple_parser_unittest.cc @@ -0,0 +1,132 @@ +// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include +#include + +using namespace isc::data; + +/// This table defines sample default values. Although these are DHCPv6 +/// specific, the mechanism is generic and can be used by any other component. +const SimpleDefaults SAMPLE_DEFAULTS = { + { "renew-timer", Element::integer, "900" }, + { "rebind-timer", Element::integer, "1800" }, + { "preferred-lifetime", Element::integer, "3600" }, + { "valid-lifetime", Element::integer, "7200" } +}; + +/// This list defines parameters that can be inherited from one scope +/// to another. Although these are DHCPv6 specific, the mechanism is generic and +/// can be used by any other component. +const ParamsList SAMPLE_INHERITS = { + "renew-timer", + "rebind-timer", + "preferred-lifetime", + "valid-lifetime" +}; + +/// @brief Simple Parser test fixture class +class SimpleParserTest : public ::testing::Test { +public: + /// @brief Checks if specified map has an integer parameter with expected value + /// + /// @param map map to be checked + /// @param param_name name of the parameter to be checked + /// @param exp_value expected value of the parameter. + void checkIntegerValue(const ConstElementPtr& map, const std::string& param_name, + int64_t exp_value) { + + // First check if the passed element is a map. + ASSERT_EQ(Element::map, map->getType()); + + // Now try to get the element being checked + ConstElementPtr elem = map->get(param_name); + ASSERT_TRUE(elem); + + // Now check if it's indeed integer + ASSERT_EQ(Element::integer, elem->getType()); + + // Finally, check if its value meets expectation. + EXPECT_EQ(exp_value, elem->intValue()); + } +}; + +// This test checks if the parameters can be inherited from the global +// scope to the subnet scope. +TEST_F(SimpleParserTest, deriveParams) { + ElementPtr global = Element::fromJSON("{ \"renew-timer\": 1," + " \"rebind-timer\": 2," + " \"preferred-lifetime\": 3," + " \"valid-lifetime\": 4" + "}"); + ElementPtr subnet = Element::fromJSON("{ \"renew-timer\": 100 }"); + + // we should inherit 3 parameters. Renew-timer should remain intact, + // as it was already defined in the subnet scope. + size_t num; + EXPECT_NO_THROW(num = SimpleParser::deriveParams(global, subnet, + SAMPLE_INHERITS)); + EXPECT_EQ(3, num); + + // Check the values. 3 of them are interited, while the fourth one + // was already defined in the subnet, so should not be inherited. + checkIntegerValue(subnet, "renew-timer", 100); + checkIntegerValue(subnet, "rebind-timer", 2); + checkIntegerValue(subnet, "preferred-lifetime", 3); + checkIntegerValue(subnet, "valid-lifetime", 4); +} + +// This test checks if global defaults are properly set for DHCPv6. +TEST_F(SimpleParserTest, setDefaults) { + + ElementPtr empty = Element::fromJSON("{ }"); + size_t num = 0; + + EXPECT_NO_THROW(num = SimpleParser::setDefaults(empty, SAMPLE_DEFAULTS)); + + // We expect at least 4 parameters to be inserted. + EXPECT_GE(num, 3); + + checkIntegerValue(empty, "valid-lifetime", 7200); + checkIntegerValue(empty, "preferred-lifetime", 3600); + checkIntegerValue(empty, "rebind-timer", 1800); + checkIntegerValue(empty, "renew-timer", 900); +} + +// This test checks if global defaults are properly set for DHCPv6. +TEST_F(SimpleParserTest, setListDefaults) { + + ElementPtr empty = Element::fromJSON("[{}, {}, {}]"); + size_t num; + + EXPECT_NO_THROW(num = SimpleParser::setListDefaults(empty, SAMPLE_DEFAULTS)); + + // We expect at least 12 parameters to be inserted (3 entries, with + // 4 parameters inserted in each) + EXPECT_EQ(12, num); + + ASSERT_EQ(Element::list, empty->getType()); + ASSERT_EQ(3, empty->size()); + + ConstElementPtr first = empty->get(0); + ConstElementPtr second = empty->get(1); + ConstElementPtr third = empty->get(2); + + checkIntegerValue(first, "valid-lifetime", 7200); + checkIntegerValue(first, "preferred-lifetime", 3600); + checkIntegerValue(first, "rebind-timer", 1800); + checkIntegerValue(first, "renew-timer", 900); + + checkIntegerValue(second, "valid-lifetime", 7200); + checkIntegerValue(second, "preferred-lifetime", 3600); + checkIntegerValue(second, "rebind-timer", 1800); + checkIntegerValue(second, "renew-timer", 900); + + checkIntegerValue(third, "valid-lifetime", 7200); + checkIntegerValue(third, "preferred-lifetime", 3600); + checkIntegerValue(third, "rebind-timer", 1800); + checkIntegerValue(third, "renew-timer", 900); +} diff --git a/src/lib/dhcpsrv/Makefile.am b/src/lib/dhcpsrv/Makefile.am index 289b9e5f71..fbd269bec6 100644 --- a/src/lib/dhcpsrv/Makefile.am +++ b/src/lib/dhcpsrv/Makefile.am @@ -33,8 +33,6 @@ EXTRA_DIST = EXTRA_DIST += parsers/client_class_def_parser.cc EXTRA_DIST += parsers/client_class_def_parser.h EXTRA_DIST += parsers/dhcp_config_parser.h -EXTRA_DIST += parsers/simple_parser.cc -EXTRA_DIST += parsers/simple_parser.h EXTRA_DIST += parsers/dbaccess_parser.cc EXTRA_DIST += parsers/dbaccess_parser.h EXTRA_DIST += parsers/dhcp_parsers.cc @@ -164,8 +162,6 @@ libkea_dhcpsrv_la_SOURCES += parsers/dbaccess_parser.cc libkea_dhcpsrv_la_SOURCES += parsers/dbaccess_parser.h libkea_dhcpsrv_la_SOURCES += parsers/dhcp_parsers.cc libkea_dhcpsrv_la_SOURCES += parsers/dhcp_parsers.h -libkea_dhcpsrv_la_SOURCES += parsers/simple_parser.cc -libkea_dhcpsrv_la_SOURCES += parsers/simple_parser.h libkea_dhcpsrv_la_SOURCES += parsers/duid_config_parser.cc libkea_dhcpsrv_la_SOURCES += parsers/duid_config_parser.h libkea_dhcpsrv_la_SOURCES += parsers/expiration_config_parser.cc From 475b2531947004a6d96cb99125da2e48d96a2b5f Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Thu, 22 Dec 2016 16:20:34 +0100 Subject: [PATCH 07/15] [5039] SimpleParser is now split into cc/SimpleParser and SimpleParser{4,6} --- src/bin/dhcp4/Makefile.am | 4 +- src/bin/dhcp4/json_config_parser.cc | 4 +- src/bin/dhcp4/simple_parser4.cc | 73 +++++++ src/bin/dhcp4/simple_parser4.h | 33 ++++ src/bin/dhcp4/tests/Makefile.am | 1 + .../dhcp4/tests/simple_parser4_unittest.cc | 91 +++++++++ src/bin/dhcp6/Makefile.am | 8 +- src/bin/dhcp6/json_config_parser.cc | 4 +- src/bin/dhcp6/simple_parser6.cc | 76 ++++++++ src/bin/dhcp6/simple_parser6.h | 35 ++++ src/bin/dhcp6/tests/Makefile.am | 1 + .../dhcp6/tests/simple_parser6_unittest.cc | 87 +++++++++ src/lib/dhcpsrv/parsers/dhcp_parsers.h | 10 +- src/lib/dhcpsrv/tests/Makefile.am | 2 +- .../dhcpsrv/tests/dhcp_parsers_unittest.cc | 182 ++++++++++-------- 15 files changed, 510 insertions(+), 101 deletions(-) create mode 100644 src/bin/dhcp4/simple_parser4.cc create mode 100644 src/bin/dhcp4/simple_parser4.h create mode 100644 src/bin/dhcp4/tests/simple_parser4_unittest.cc create mode 100644 src/bin/dhcp6/simple_parser6.cc create mode 100644 src/bin/dhcp6/simple_parser6.h create mode 100644 src/bin/dhcp6/tests/simple_parser6_unittest.cc diff --git a/src/bin/dhcp4/Makefile.am b/src/bin/dhcp4/Makefile.am index 8360677861..d395f458a4 100644 --- a/src/bin/dhcp4/Makefile.am +++ b/src/bin/dhcp4/Makefile.am @@ -3,7 +3,7 @@ SUBDIRS = . tests AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib AM_CPPFLAGS += -I$(top_srcdir)/src/bin -I$(top_builddir)/src/bin AM_CPPFLAGS += -I$(top_srcdir)/src -I$(top_builddir)/src -AM_CPPFLAGS += $(BOOST_INCLUDES) +AM_CPPFLAGS += $(BOOST_INCLUDES) -std=c++11 if HAVE_MYSQL AM_CPPFLAGS += $(MYSQL_CPPFLAGS) endif @@ -62,8 +62,8 @@ libdhcp4_la_SOURCES += json_config_parser.cc json_config_parser.h libdhcp4_la_SOURCES += dhcp4_log.cc dhcp4_log.h libdhcp4_la_SOURCES += dhcp4_srv.cc dhcp4_srv.h libdhcp4_la_SOURCES += dhcp4to6_ipc.cc dhcp4to6_ipc.h - libdhcp4_la_SOURCES += kea_controller.cc +libdhcp4_la_SOURCES += simple_parser4.cc simple_parser4.h nodist_libdhcp4_la_SOURCES = dhcp4_messages.h dhcp4_messages.cc EXTRA_DIST += dhcp4_messages.mes diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index e4edf89027..6a4c352342 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -20,7 +21,6 @@ #include #include #include -#include #include #include #include @@ -573,7 +573,7 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { mutable_cfg->setValue(values); // Set all default values if not specified by the user. - SimpleParser::setAllDefaults(mutable_cfg, false); + SimpleParser4::setAllDefaults(mutable_cfg); // We need definitions first ConstElementPtr option_defs = mutable_cfg->get("option-def"); diff --git a/src/bin/dhcp4/simple_parser4.cc b/src/bin/dhcp4/simple_parser4.cc new file mode 100644 index 0000000000..f927429fdc --- /dev/null +++ b/src/bin/dhcp4/simple_parser4.cc @@ -0,0 +1,73 @@ +// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include +#include +#include + +using namespace std; +using namespace isc::data; + +namespace isc { +namespace dhcp { + +/// This table defines default values for option definitions in DHCPv4 +const SimpleDefaults SimpleParser4::OPTION4_DEF_DEFAULTS = { + { "record-types", Element::string, ""}, + { "space", Element::string, "dhcp4"}, + { "array", Element::boolean, "false"}, + { "encapsulate", Element::string, "" } +}; + +/// This table defines default values for options in DHCPv4 +const SimpleDefaults SimpleParser4::OPTION4_DEFAULTS = { + { "space", Element::string, "dhcp4"}, + { "csv-format", Element::boolean, "true"}, + { "encapsulate", Element::string, "" } +}; + +/// This table defines default values for DHCPv4 +const SimpleDefaults SimpleParser4::GLOBAL4_DEFAULTS = { + { "renew-timer", Element::integer, "900" }, + { "rebind-timer", Element::integer, "1800" }, + { "valid-lifetime", Element::integer, "7200" } +}; + +/// This list defines parameters that can be inherited from the global +/// scope to subnet6 scope. +const ParamsList SimpleParser4::INHERIT_GLOBAL_TO_SUBNET4 = { + "renew-timer", + "rebind-timer", + "valid-lifetime" +}; + +size_t SimpleParser4::setAllDefaults(isc::data::ElementPtr global) { + size_t cnt = 0; + + // Set global defaults first. + cnt = setDefaults(global, GLOBAL4_DEFAULTS); + + // Now set option defintion defaults for each specified option definition + ConstElementPtr option_defs = global->get("option-def"); + if (option_defs) { + BOOST_FOREACH(ElementPtr option_def, option_defs->listValue()) { + cnt += SimpleParser::setDefaults(option_def, OPTION4_DEF_DEFAULTS); + } + } + + // Finally, set the defaults for option data + ConstElementPtr options = global->get("option-data"); + if (options) { + BOOST_FOREACH(ElementPtr single_option, options->listValue()) { + cnt += SimpleParser::setDefaults(single_option, OPTION4_DEFAULTS); + } + } + + return (cnt); +} + +}; +}; diff --git a/src/bin/dhcp4/simple_parser4.h b/src/bin/dhcp4/simple_parser4.h new file mode 100644 index 0000000000..5b2fc32743 --- /dev/null +++ b/src/bin/dhcp4/simple_parser4.h @@ -0,0 +1,33 @@ +// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#ifndef SIMPLE_PARSER4_H +#define SIMPLE_PARSER4_H + +#include + +namespace isc { +namespace dhcp { + +class SimpleParser4 : public isc::data::SimpleParser { +public: + /// @brief Sets all defaults for DHCPv4 configuration + /// + /// This method sets global, option data and option definitions defaults. + /// + /// @param global scope to be filled in with defaults. + /// @return number of default values added + static size_t setAllDefaults(isc::data::ElementPtr global); + + static const isc::data::SimpleDefaults OPTION4_DEF_DEFAULTS; + static const isc::data::SimpleDefaults OPTION4_DEFAULTS; + static const isc::data::SimpleDefaults GLOBAL4_DEFAULTS; + static const isc::data::ParamsList INHERIT_GLOBAL_TO_SUBNET4; +}; + +}; +}; +#endif diff --git a/src/bin/dhcp4/tests/Makefile.am b/src/bin/dhcp4/tests/Makefile.am index f0b5babb3d..cfe9c143d1 100644 --- a/src/bin/dhcp4/tests/Makefile.am +++ b/src/bin/dhcp4/tests/Makefile.am @@ -92,6 +92,7 @@ dhcp4_unittests_SOURCES += out_of_range_unittest.cc dhcp4_unittests_SOURCES += decline_unittest.cc dhcp4_unittests_SOURCES += kea_controller_unittest.cc dhcp4_unittests_SOURCES += dhcp4to6_ipc_unittest.cc +dhcp4_unittests_SOURCES += simple_parser4_unittest.cc nodist_dhcp4_unittests_SOURCES = marker_file.h test_libraries.h diff --git a/src/bin/dhcp4/tests/simple_parser4_unittest.cc b/src/bin/dhcp4/tests/simple_parser4_unittest.cc new file mode 100644 index 0000000000..9d02c049fd --- /dev/null +++ b/src/bin/dhcp4/tests/simple_parser4_unittest.cc @@ -0,0 +1,91 @@ +// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include +#include +#include +#include + +using namespace isc::data; + +namespace isc { +namespace dhcp { +namespace test { + +/// @brief DHCP Parser test fixture class +class SimpleParser4Test : public ::testing::Test { +public: + /// @brief Checks if specified map has an integer parameter with expected value + /// + /// @param map map to be checked + /// @param param_name name of the parameter to be checked + /// @param exp_value expected value of the parameter. + void checkIntegerValue(const ConstElementPtr& map, const std::string& param_name, + int64_t exp_value) { + + // First check if the passed element is a map. + ASSERT_EQ(Element::map, map->getType()); + + // Now try to get the element being checked + ConstElementPtr elem = map->get(param_name); + ASSERT_TRUE(elem); + + // Now check if it's indeed integer + ASSERT_EQ(Element::integer, elem->getType()); + + // Finally, check if its value meets expectation. + EXPECT_EQ(exp_value, elem->intValue()); + } +}; + +// This test checks if global defaults are properly set for DHCPv4. +TEST_F(SimpleParser4Test, globalDefaults4) { + + ElementPtr empty = Element::fromJSON("{ }"); + size_t num = 0; + + EXPECT_NO_THROW(num = SimpleParser4::setAllDefaults(empty)); + + + // We expect at least 3 parameters to be inserted. + EXPECT_TRUE(num >= 3); + + checkIntegerValue(empty, "valid-lifetime", 7200); + checkIntegerValue(empty, "rebind-timer", 1800); + checkIntegerValue(empty, "renew-timer", 900); + + // Make sure that preferred-lifetime is not set for v4 (it's v6 only + // parameter) + EXPECT_FALSE(empty->get("preferred-lifetime")); +} + +// This test checks if the parameters can be inherited from the global +// scope to the subnet scope. +TEST_F(SimpleParser4Test, inheritGlobalToSubnet4) { + ElementPtr global = Element::fromJSON("{ \"renew-timer\": 1," + " \"rebind-timer\": 2," + " \"preferred-lifetime\": 3," + " \"valid-lifetime\": 4" + "}"); + ElementPtr subnet = Element::fromJSON("{ \"renew-timer\": 100 }"); + + // we should inherit 3 parameters. Renew-timer should remain intact, + // as it was already defined in the subnet scope. + size_t num; + EXPECT_NO_THROW(num = SimpleParser4::deriveParams(global, subnet, + SimpleParser4::INHERIT_GLOBAL_TO_SUBNET4)); + EXPECT_EQ(2, num); + + // Check the values. 2 of them are interited, while the third one + // was already defined in the subnet, so should not be inherited. + checkIntegerValue(subnet, "renew-timer", 100); + checkIntegerValue(subnet, "rebind-timer", 2); + checkIntegerValue(subnet, "valid-lifetime", 4); +} + +}; +}; +}; diff --git a/src/bin/dhcp6/Makefile.am b/src/bin/dhcp6/Makefile.am index 47155aecf5..b1f6899e32 100644 --- a/src/bin/dhcp6/Makefile.am +++ b/src/bin/dhcp6/Makefile.am @@ -3,7 +3,7 @@ SUBDIRS = . tests AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib AM_CPPFLAGS += -I$(top_srcdir)/src/bin -I$(top_builddir)/src/bin AM_CPPFLAGS += -I$(top_srcdir)/src -I$(top_builddir)/src -AM_CPPFLAGS += $(BOOST_INCLUDES) +AM_CPPFLAGS += $(BOOST_INCLUDES) -std=c++11 if HAVE_MYSQL AM_CPPFLAGS += $(MYSQL_CPPFLAGS) endif @@ -31,8 +31,8 @@ if GENERATE_DOCS kea-dhcp6.8: kea-dhcp6.xml @XSLTPROC@ --novalid --xinclude --nonet -o $@ \ - http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl \ - $(srcdir)/kea-dhcp6.xml + http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl \ + $(srcdir)/kea-dhcp6.xml else @@ -63,8 +63,8 @@ libdhcp6_la_SOURCES += dhcp6_srv.cc dhcp6_srv.h libdhcp6_la_SOURCES += ctrl_dhcp6_srv.cc ctrl_dhcp6_srv.h libdhcp6_la_SOURCES += json_config_parser.cc json_config_parser.h libdhcp6_la_SOURCES += dhcp6to4_ipc.cc dhcp6to4_ipc.h - libdhcp6_la_SOURCES += kea_controller.cc +libdhcp6_la_SOURCES += simple_parser6.cc simple_parser6.h nodist_libdhcp6_la_SOURCES = dhcp6_messages.h dhcp6_messages.cc EXTRA_DIST += dhcp6_messages.mes diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index aaa0e2a03b..ee9ea6a98e 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -29,7 +30,6 @@ #include #include #include -#include #include #include #include @@ -845,7 +845,7 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { ElementPtr mutable_cfg(new MapElement()); mutable_cfg->setValue(values); - SimpleParser::setAllDefaults(mutable_cfg, true); + SimpleParser6::setAllDefaults(mutable_cfg); // Make parsers grouping. const std::map& values_map = diff --git a/src/bin/dhcp6/simple_parser6.cc b/src/bin/dhcp6/simple_parser6.cc new file mode 100644 index 0000000000..1190481439 --- /dev/null +++ b/src/bin/dhcp6/simple_parser6.cc @@ -0,0 +1,76 @@ +// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include +#include +#include + +using namespace std; +using namespace isc::data; + +namespace isc { +namespace dhcp { + +/// This table defines default values for option definitions in DHCPv6 +const SimpleDefaults SimpleParser6::OPTION6_DEF_DEFAULTS = { + { "record-types", Element::string, ""}, + { "space", Element::string, "dhcp6"}, + { "array", Element::boolean, "false"}, + { "encapsulate", Element::string, "" } +}; + +/// This table defines default values for options in DHCPv6 +const SimpleDefaults SimpleParser6::OPTION6_DEFAULTS = { + { "space", Element::string, "dhcp6"}, + { "csv-format", Element::boolean, "true"}, + { "encapsulate", Element::string, "" } +}; + +/// This table defines default values for both DHCPv4 and DHCPv6 +const SimpleDefaults SimpleParser6::GLOBAL6_DEFAULTS = { + { "renew-timer", Element::integer, "900" }, + { "rebind-timer", Element::integer, "1800" }, + { "preferred-lifetime", Element::integer, "3600" }, + { "valid-lifetime", Element::integer, "7200" } +}; + +/// This list defines parameters that can be inherited from the global +/// scope to subnet6 scope. +const ParamsList SimpleParser6::INHERIT_GLOBAL_TO_SUBNET6 = { + "renew-timer", + "rebind-timer", + "preferred-lifetime", + "valid-lifetime" +}; + + +size_t SimpleParser6::setAllDefaults(isc::data::ElementPtr global) { + size_t cnt = 0; + + // Set global defaults first. + cnt = setDefaults(global, GLOBAL6_DEFAULTS); + + // Now set the defaults for each specified option definition + ConstElementPtr option_defs = global->get("option-def"); + if (option_defs) { + BOOST_FOREACH(ElementPtr option_def, option_defs->listValue()) { + cnt += SimpleParser::setDefaults(option_def, OPTION6_DEF_DEFAULTS); + } + } + + // Finally, set the defaults for option data + ConstElementPtr options = global->get("option-data"); + if (options) { + BOOST_FOREACH(ElementPtr single_option, options->listValue()) { + cnt += SimpleParser::setDefaults(single_option, OPTION6_DEFAULTS); + } + } + + return (cnt); +} + +}; +}; diff --git a/src/bin/dhcp6/simple_parser6.h b/src/bin/dhcp6/simple_parser6.h new file mode 100644 index 0000000000..28521180cc --- /dev/null +++ b/src/bin/dhcp6/simple_parser6.h @@ -0,0 +1,35 @@ +// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#ifndef SIMPLE_PARSER6_H +#define SIMPLE_PARSER6_H + +#include + +namespace isc { +namespace dhcp { + +class SimpleParser6 : public isc::data::SimpleParser { +public: + + /// @brief Sets all defaults for DHCPv6 configuration + /// + /// This method sets global, option data and option definitions defaults. + /// + /// @param global scope to be filled in with defaults. + /// @return number of default values added + static size_t setAllDefaults(isc::data::ElementPtr global); + + static const isc::data::SimpleDefaults OPTION6_DEF_DEFAULTS; + static const isc::data::SimpleDefaults OPTION6_DEFAULTS; + static const isc::data::SimpleDefaults GLOBAL6_DEFAULTS; + static const isc::data::ParamsList INHERIT_GLOBAL_TO_SUBNET6; +}; + +}; +}; + +#endif diff --git a/src/bin/dhcp6/tests/Makefile.am b/src/bin/dhcp6/tests/Makefile.am index 87c619761e..a2dd2358bb 100644 --- a/src/bin/dhcp6/tests/Makefile.am +++ b/src/bin/dhcp6/tests/Makefile.am @@ -93,6 +93,7 @@ dhcp6_unittests_SOURCES += dhcp6_message_test.cc dhcp6_message_test.h dhcp6_unittests_SOURCES += kea_controller_unittest.cc dhcp6_unittests_SOURCES += dhcp6to4_ipc_unittest.cc dhcp6_unittests_SOURCES += classify_unittests.cc +dhcp6_unittests_SOURCES += simple_parser6_unittest.cc nodist_dhcp6_unittests_SOURCES = marker_file.h test_libraries.h diff --git a/src/bin/dhcp6/tests/simple_parser6_unittest.cc b/src/bin/dhcp6/tests/simple_parser6_unittest.cc new file mode 100644 index 0000000000..e5aca51151 --- /dev/null +++ b/src/bin/dhcp6/tests/simple_parser6_unittest.cc @@ -0,0 +1,87 @@ +// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +#include +#include +#include +#include + +using namespace isc; +using namespace isc::data; +using namespace isc::dhcp; + +namespace { + +/// @brief DHCP Parser test fixture class +class SimpleParser6Test : public ::testing::Test { +public: + /// @brief Checks if specified map has an integer parameter with expected value + /// + /// @param map map to be checked + /// @param param_name name of the parameter to be checked + /// @param exp_value expected value of the parameter. + void checkIntegerValue(const ConstElementPtr& map, const std::string& param_name, + int64_t exp_value) { + + // First check if the passed element is a map. + ASSERT_EQ(Element::map, map->getType()); + + // Now try to get the element being checked + ConstElementPtr elem = map->get(param_name); + ASSERT_TRUE(elem); + + // Now check if it's indeed integer + ASSERT_EQ(Element::integer, elem->getType()); + + // Finally, check if its value meets expectation. + EXPECT_EQ(exp_value, elem->intValue()); + } +}; + +// This test checks if global defaults are properly set for DHCPv6. +TEST_F(SimpleParser6Test, globalDefaults6) { + + ElementPtr empty = Element::fromJSON("{ }"); + size_t num = 0; + + EXPECT_NO_THROW(num = SimpleParser6::setAllDefaults(empty)); + + // We expect at least 4 parameters to be inserted. + EXPECT_TRUE(num >= 4); + + checkIntegerValue(empty, "valid-lifetime", 7200); + checkIntegerValue(empty, "preferred-lifetime", 3600); + checkIntegerValue(empty, "rebind-timer", 1800); + checkIntegerValue(empty, "renew-timer", 900); +} + +// This test checks if the parameters can be inherited from the global +// scope to the subnet scope. +TEST_F(SimpleParser6Test, inheritGlobalToSubnet6) { + ElementPtr global = Element::fromJSON("{ \"renew-timer\": 1," + " \"rebind-timer\": 2," + " \"preferred-lifetime\": 3," + " \"valid-lifetime\": 4" + "}"); + ElementPtr subnet = Element::fromJSON("{ \"renew-timer\": 100 }"); + + // we should inherit 3 parameters. Renew-timer should remain intact, + // as it was already defined in the subnet scope. + size_t num; + EXPECT_NO_THROW(num = SimpleParser::deriveParams(global, subnet, + SimpleParser6::INHERIT_GLOBAL_TO_SUBNET6)); + EXPECT_EQ(3, num); + + // Check the values. 3 of them are interited, while the fourth one + // was already defined in the subnet, so should not be inherited. + checkIntegerValue(subnet, "renew-timer", 100); + checkIntegerValue(subnet, "rebind-timer", 2); + checkIntegerValue(subnet, "preferred-lifetime", 3); + checkIntegerValue(subnet, "valid-lifetime", 4); +} + +}; + diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.h b/src/lib/dhcpsrv/parsers/dhcp_parsers.h index df829dc1db..365481b47e 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.h +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.h @@ -17,7 +17,7 @@ #include #include #include -#include +#include #include #include #include @@ -536,7 +536,7 @@ private: /// an option the configuration will not be accepted. If parsing /// is successful then an instance of an option is created and /// added to the storage provided by the calling class. -class OptionDataParser : public SimpleParser { +class OptionDataParser : public isc::data::SimpleParser { public: /// @brief Constructor. /// @@ -652,7 +652,7 @@ typedef OptionDataParser *OptionDataParserFactory(const std::string&, /// data for a particular subnet and creates a collection of options. /// If parsing is successful, all these options are added to the Subnet /// object. -class OptionDataListParser : public SimpleParser { +class OptionDataListParser : public isc::data::SimpleParser { public: /// @brief Constructor. /// @@ -679,7 +679,7 @@ typedef std::pair OptionDefinitionT /// @brief Parser for a single option definition. /// /// This parser creates an instance of a single option definition. -class OptionDefParser : public SimpleParser { +class OptionDefParser : public isc::data::SimpleParser { public: /// @brief Constructor. /// @@ -706,7 +706,7 @@ private: /// option definitions and creates instances of these definitions. /// If the parsing is successful, the collection of created definitions /// is put into the provided storage. -class OptionDefListParser : public SimpleParser { +class OptionDefListParser : public isc::data::SimpleParser { public: /// @brief Constructor. /// diff --git a/src/lib/dhcpsrv/tests/Makefile.am b/src/lib/dhcpsrv/tests/Makefile.am index 8729de987d..430049c7d4 100644 --- a/src/lib/dhcpsrv/tests/Makefile.am +++ b/src/lib/dhcpsrv/tests/Makefile.am @@ -8,7 +8,7 @@ AM_CPPFLAGS += -DKEA_LFC_BUILD_DIR=\"$(abs_top_builddir)/src/bin/lfc\" AM_CPPFLAGS += -DLOGGING_SPEC_FILE=\"$(abs_top_srcdir)/src/lib/dhcpsrv/logging.spec\" AM_CPPFLAGS += -DINSTALL_PROG=\"$(abs_top_srcdir)/install-sh\" -AM_CXXFLAGS = $(KEA_CXXFLAGS) +AM_CXXFLAGS = $(KEA_CXXFLAGS) -std=c++11 if USE_STATIC_LINK AM_LDFLAGS = -static diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index b9d5a76322..52e63bf782 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -16,7 +17,6 @@ #include #include #include -#include #include #include #include @@ -412,6 +412,101 @@ public: return (parser); } + /// @brief DHCP-specific method that sets global, and option specific defaults + /// + /// This method sets the defaults in the global scope, in option definitions, + /// and in option data. + /// + /// @param global pointer to the Element tree that holds configuration + /// @param global_defaults array with global default values + /// @param option_defaults array with option-data default values + /// @param option_def_defaults array with default values for option definitions + /// @return number of default values inserted. + size_t setAllDefaults(isc::data::ElementPtr global, + const SimpleDefaults& global_defaults, + const SimpleDefaults& option_defaults, + const SimpleDefaults& option_def_defaults) { + size_t cnt = 0; + // Set global defaults first. + /// @todo: Uncomment as part of the ticket 5019 work. + cnt = SimpleParser::setDefaults(global, global_defaults); + + // Now set option defintion defaults for each specified option definition + ConstElementPtr option_defs = global->get("option-def"); + if (option_defs) { + BOOST_FOREACH(ElementPtr single_def, option_defs->listValue()) { + cnt += SimpleParser::setDefaults(single_def, option_def_defaults); + } + } + + ConstElementPtr options = global->get("option-data"); + if (options) { + BOOST_FOREACH(ElementPtr single_option, options->listValue()) { + cnt += SimpleParser::setDefaults(single_option, option_defaults); + } + } + + return (cnt); + } + + /// @brief sets all default values for DHCPv4 and DHCPv6 + /// + /// This function largely duplicates what SimpleParser4 and SimpleParser6 classes + /// provide. However, since there are tons of unit-tests in dhcpsrv that need + /// this functionality and there are good reasons to keep those classes in + /// src/bin/dhcp{4,6}, the most straightforward way is to simply copy the + /// minimum code here. Hence this method. + /// + /// @param config configuration structure to be filled with default values + /// @param v6 true = DHCPv6, false = DHCPv4 + void setAllDefaults(ElementPtr config, bool v6) { + /// This table defines default values for option definitions in DHCPv6 + const SimpleDefaults OPTION6_DEF_DEFAULTS = { + { "record-types", Element::string, ""}, + { "space", Element::string, "dhcp6"}, + { "array", Element::boolean, "false"}, + { "encapsulate", Element::string, "" } + }; + + /// This table defines default values for option definitions in DHCPv4 + const SimpleDefaults OPTION4_DEF_DEFAULTS = { + { "record-types", Element::string, ""}, + { "space", Element::string, "dhcp4"}, + { "array", Element::boolean, "false"}, + { "encapsulate", Element::string, "" } + }; + + /// This table defines default values for options in DHCPv6 + const SimpleDefaults OPTION6_DEFAULTS = { + { "space", Element::string, "dhcp6"}, + { "csv-format", Element::boolean, "true"}, + { "encapsulate", Element::string, "" } + }; + + /// This table defines default values for options in DHCPv4 + const SimpleDefaults OPTION4_DEFAULTS = { + { "space", Element::string, "dhcp4"}, + { "csv-format", Element::boolean, "true"}, + { "encapsulate", Element::string, "" } + }; + + /// This table defines default values for both DHCPv4 and DHCPv6 + const SimpleDefaults GLOBAL6_DEFAULTS = { + { "renew-timer", Element::integer, "900" }, + { "rebind-timer", Element::integer, "1800" }, + { "preferred-lifetime", Element::integer, "3600" }, + { "valid-lifetime", Element::integer, "7200" } + }; + + if (v6) { + setAllDefaults(config, GLOBAL6_DEFAULTS, OPTION6_DEFAULTS, + OPTION6_DEF_DEFAULTS); + } else { + setAllDefaults(config, GLOBAL6_DEFAULTS, OPTION4_DEFAULTS, + OPTION4_DEF_DEFAULTS); + } + } + /// @brief Convenience method for parsing a configuration /// /// Given a configuration string, convert it into Elements @@ -427,7 +522,7 @@ public: ElementPtr json = Element::fromJSON(config); EXPECT_TRUE(json); if (json) { - SimpleParser::setAllDefaults(json, v6); + setAllDefaults(json, v6); ConstElementPtr status = parseElementSet(json); ConstElementPtr comment = parseAnswer(rcode_, status); @@ -487,28 +582,6 @@ public: CfgMgr::instance().setD2ClientConfig(tmp); } - /// @brief Checks if specified map has an integer parameter with expected value - /// - /// @param map map to be checked - /// @param param_name name of the parameter to be checked - /// @param exp_value expected value of the parameter. - void checkIntegerValue(const ConstElementPtr& map, const std::string& param_name, - int64_t exp_value) { - - // First check if the passed element is a map. - ASSERT_EQ(Element::map, map->getType()); - - // Now try to get the element being checked - ConstElementPtr elem = map->get(param_name); - ASSERT_TRUE(elem); - - // Now check if it's indeed integer - ASSERT_EQ(Element::integer, elem->getType()); - - // Finally, check if its value meets expectation. - EXPECT_EQ(exp_value, elem->intValue()); - } - /// @brief Parsers used in the parsing of the configuration /// /// Allows the tests to interrogate the state of the parsers (if required). @@ -2425,65 +2498,4 @@ TEST_F(ParseConfigTest, validRelayInfo6) { // (see CtrlDhcpv4SrvTest.commandSocketBasic in // src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc). -// This test checks if global defaults are properly set for DHCPv6. -TEST_F(ParseConfigTest, globalDefaults6) { - - ElementPtr empty = Element::fromJSON("{ }"); - size_t num; - - EXPECT_NO_THROW(num = SimpleParser::setGlobalDefaults(empty, true)); - - // We expect at least 4 parameters to be inserted. - EXPECT_TRUE(num >= 4); - - checkIntegerValue(empty, "valid-lifetime", 7200); - checkIntegerValue(empty, "preferred-lifetime", 3600); - checkIntegerValue(empty, "rebind-timer", 1800); - checkIntegerValue(empty, "renew-timer", 900); -} - -// This test checks if global defaults are properly set for DHCPv4. -TEST_F(ParseConfigTest, DISABLED_globalDefaults4) { - - ElementPtr empty = Element::fromJSON("{ }"); - size_t num; - - EXPECT_NO_THROW(num = SimpleParser::setGlobalDefaults(empty, false)); - - // We expect at least 3 parameters to be inserted. - EXPECT_TRUE(num >= 3); - - checkIntegerValue(empty, "valid-lifetime", 7200); - checkIntegerValue(empty, "rebind-timer", 1800); - checkIntegerValue(empty, "renew-timer", 900); - - // Make sure that preferred-lifetime is not set for v4 (it's v6 only - // parameter) - EXPECT_FALSE(empty->get("preferred-lifetime")); -} - -// This test checks if the parameters can be inherited from the global -// scope to the subnet scope. -TEST_F(ParseConfigTest, inheritGlobalToSubnet) { - ElementPtr global = Element::fromJSON("{ \"renew-timer\": 1," - " \"rebind-timer\": 2," - " \"preferred-lifetime\": 3," - " \"valid-lifetime\": 4" - "}"); - ElementPtr subnet = Element::fromJSON("{ \"renew-timer\": 100 }"); - - // we should inherit 3 parameters. Renew-timer should remain intact, - // as it was already defined in the subnet scope. - size_t num; - EXPECT_NO_THROW(num = SimpleParser::inheritGlobalToSubnet(global, subnet)); - EXPECT_EQ(3, num); - - // Check the values. 3 of them are interited, while the fourth one - // was already defined in the subnet, so should not be inherited. - checkIntegerValue(subnet, "renew-timer", 100); - checkIntegerValue(subnet, "rebind-timer", 2); - checkIntegerValue(subnet, "preferred-lifetime", 3); - checkIntegerValue(subnet, "valid-lifetime", 4); -} - }; // Anonymous namespace From 7e639550bd18a625ea19aee52e3d92045814ec79 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Thu, 22 Dec 2016 16:21:16 +0100 Subject: [PATCH 08/15] [5039] Failing unit-tests fixed --- src/bin/dhcp4/tests/config_parser_unittest.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index b517abcb4a..e303da74d3 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -669,8 +669,9 @@ TEST_F(Dhcp4ParserTest, unspecifiedRenewTimer) { Subnet4Ptr subnet = CfgMgr::instance().getStagingCfg()-> getCfgSubnets4()->selectSubnet(IOAddress("192.0.2.200")); ASSERT_TRUE(subnet); - EXPECT_TRUE(subnet->getT1().unspecified()); + EXPECT_FALSE(subnet->getT1().unspecified()); EXPECT_FALSE(subnet->getT2().unspecified()); + EXPECT_EQ(900, subnet->getT1()); // that's the default value EXPECT_EQ(2000, subnet->getT2()); EXPECT_EQ(4000, subnet->getValid()); @@ -705,7 +706,8 @@ TEST_F(Dhcp4ParserTest, unspecifiedRebindTimer) { ASSERT_TRUE(subnet); EXPECT_FALSE(subnet->getT1().unspecified()); EXPECT_EQ(1000, subnet->getT1()); - EXPECT_TRUE(subnet->getT2().unspecified()); + EXPECT_FALSE(subnet->getT2().unspecified()); + EXPECT_EQ(1800, subnet->getT2()); // that's the default value EXPECT_EQ(4000, subnet->getValid()); // Check that subnet-id is 1 From 0d579d2a207e049ce9027a435ed0d9a64fed0fe7 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Thu, 22 Dec 2016 16:36:28 +0100 Subject: [PATCH 09/15] [5039] Element::getMutableMap implemented --- src/bin/dhcp4/json_config_parser.cc | 5 +---- src/bin/dhcp6/json_config_parser.cc | 9 ++------- src/lib/cc/data.cc | 9 +++++++++ src/lib/cc/data.h | 6 ++++++ 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index 6a4c352342..2109c7b8f2 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -567,10 +567,7 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { // This is a way to convert ConstElementPtr to ElementPtr. // We need a config that can be edited, because we will insert // default values and will insert derived values as well. - std::map values; - config_set->getValue(values); - ElementPtr mutable_cfg(new MapElement()); - mutable_cfg->setValue(values); + ElementPtr mutable_cfg = Element::getMutableMap(config_set); // Set all default values if not specified by the user. SimpleParser4::setAllDefaults(mutable_cfg); diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index ee9ea6a98e..9a6c8483eb 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -840,10 +840,7 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { // This is a way to convert ConstElementPtr to ElementPtr. // We need a config that can be edited, because we will insert // default values and will insert derived values as well. - std::map values; - config_set->getValue(values); - ElementPtr mutable_cfg(new MapElement()); - mutable_cfg->setValue(values); + ElementPtr mutable_cfg = Element::getMutableMap(config_set); SimpleParser6::setAllDefaults(mutable_cfg); @@ -881,9 +878,7 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { subnet_parser = parser; } else if (config_pair.first == "lease-database") { leases_parser = parser; - } /* else if (config_pair.first == "option-data") { - option_parser = parser; - } */ else if (config_pair.first == "hooks-libraries") { + } else if (config_pair.first == "hooks-libraries") { // Executing the commit will alter currently loaded hooks // libraries. Check if the supplied libraries are valid, // but defer the commit until after everything else has diff --git a/src/lib/cc/data.cc b/src/lib/cc/data.cc index 89042b9e2e..6430fb6dee 100644 --- a/src/lib/cc/data.cc +++ b/src/lib/cc/data.cc @@ -1079,5 +1079,14 @@ void Element::preprocess(std::istream& in, std::stringstream& out) { } } +ElementPtr Element::getMutableMap(ConstElementPtr& const_map) { + std::map values; + const_map->getValue(values); + ElementPtr mutable_map(new MapElement()); + mutable_map->setValue(values); + + return (mutable_map); +} + } } diff --git a/src/lib/cc/data.h b/src/lib/cc/data.h index f1a08fca40..c4b985689c 100644 --- a/src/lib/cc/data.h +++ b/src/lib/cc/data.h @@ -523,6 +523,12 @@ public: /// \return ElementPtr with the data that is parsed. static ElementPtr fromWire(const std::string& s); //@} + + /// @brief Creates mutable map based on const map + /// + /// @param const_map const map to be used as a donor + /// @return mutable map + static ElementPtr getMutableMap(ConstElementPtr& const_map); }; /// Notes: IntElement type is changed to int64_t. From 65d187c0e6ea11817188b10d1641f6eb7a6caa4b Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Thu, 22 Dec 2016 18:43:21 +0100 Subject: [PATCH 10/15] [5039] Changes after review: - address_family_, default ctor removed from two parsers - copyright year updated - Element::getNonConst is now documented - user's guide is now clear about csv-format default value is now true --- doc/guide/dhcp4-srv.xml | 22 ++---- doc/guide/dhcp6-srv.xml | 22 ++---- src/bin/dhcp4/json_config_parser.cc | 2 +- src/bin/dhcp6/json_config_parser.cc | 2 +- src/bin/dhcp6/tests/dhcp6_srv_unittest.cc | 1 - .../dhcp6/tests/simple_parser6_unittest.cc~ | 79 +++++++++++++++++++ src/lib/cc/data.cc | 2 +- src/lib/cc/data.h | 6 +- src/lib/dhcpsrv/parsers/dhcp_parsers.cc | 9 +-- src/lib/dhcpsrv/parsers/dhcp_parsers.h | 22 ------ .../dhcpsrv/tests/dhcp_parsers_unittest.cc | 2 +- 11 files changed, 101 insertions(+), 68 deletions(-) create mode 100644 src/bin/dhcp6/tests/simple_parser6_unittest.cc~ diff --git a/doc/guide/dhcp4-srv.xml b/doc/guide/dhcp4-srv.xml index 4c7c3efa4c..6f58baf6b2 100644 --- a/doc/guide/dhcp4-srv.xml +++ b/doc/guide/dhcp4-srv.xml @@ -1578,22 +1578,12 @@ It is merely echoed by the server - csv-format - if this value is not specified - and the definition for the particular option exists, the server will assume - that the option data is specified as a list of comma separated values to be - assigned to individual fields of the DHCP option. If the definition - does not exist for this option, the server will assume that the data - parameter contains the option payload in the binary format (represented - as a string of hexadecimal digits). Note that not specifying this - parameter doesn't imply that it defaults to a fixed value, but - the configuration data interpretation also depends on the presence - of the option definition. An administrator must be aware if the - definition for the particular option exists when this parameter - is not specified. It is generally recommended to not specify this - parameter only for the options for which the definition exists, e.g. - standard options. Setting csv-format to an explicit - value will cause the server to strictly check the format of the option - data specified. + csv-format - if this value is not + specified the server will assume that the option data is specified as + a list of comma separated values to be assigned to individual fields + of the DHCP option. This behavior has changed in Kea 1.2. Older + versions used additional logic to determined whether the csv-format + should be true or false. That is no longer the case. diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml index 8fb04a40d1..fc0d79c290 100644 --- a/doc/guide/dhcp6-srv.xml +++ b/doc/guide/dhcp6-srv.xml @@ -1721,22 +1721,12 @@ should include options from the isc option space: - csv-format - if this value is not specified - and the definition for the particular option exists, the server will assume - that the option data is specified as a list of comma separated values to be - assigned to individual fields of the DHCP option. If the definition - does not exist for this option, the server will assume that the data - parameter contains the option payload in the binary format (represented - as a string of hexadecimal digits). Note that not specifying this - parameter doesn't imply that it defaults to a fixed value, but - the configuration data interpretation also depends on the presence - of the option definition. An administrator must be aware if the - definition for the particular option exists when this parameter - is not specified. It is generally recommended to not specify this - parameter only for the options for which the definition exists, e.g. - standard options. Setting csv-format to an explicit - value will cause the server to strictly check the format of the option - data specified. + csv-format - if this value is not + specified the server will assume that the option data is specified as + a list of comma separated values to be assigned to individual fields + of the DHCP option. This behavior has changed in Kea 1.2. Older + versions used additional logic to determined whether the csv-format + should be true or false. That is no longer the case. diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index 2109c7b8f2..37c1262d88 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -575,7 +575,7 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { // We need definitions first ConstElementPtr option_defs = mutable_cfg->get("option-def"); if (option_defs) { - OptionDefListParser parser(AF_INET); + OptionDefListParser parser; CfgOptionDefPtr cfg_option_def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef(); parser.parse(cfg_option_def, option_defs); } diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index 9a6c8483eb..98e1c081b0 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -851,7 +851,7 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { // We need definitions first ConstElementPtr option_defs = mutable_cfg->get("option-def"); if (option_defs) { - OptionDefListParser parser(AF_INET6); + OptionDefListParser parser; CfgOptionDefPtr cfg_option_def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef(); parser.parse(cfg_option_def, option_defs); } diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc index ac73374dd6..d3fdf665d0 100644 --- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc @@ -2024,7 +2024,6 @@ TEST_F(Dhcpv6SrvTest, rsooOverride) { " \"option-def\": [ {" " \"name\": \"foo\"," " \"code\": 120," - " \"csv-format\": false," " \"type\": \"binary\"" " } ]," " \"option-data\": [ {" diff --git a/src/bin/dhcp6/tests/simple_parser6_unittest.cc~ b/src/bin/dhcp6/tests/simple_parser6_unittest.cc~ new file mode 100644 index 0000000000..fb7dfc250d --- /dev/null +++ b/src/bin/dhcp6/tests/simple_parser6_unittest.cc~ @@ -0,0 +1,79 @@ +#include +#include +#include + +using namespace isc; +using namespace isc::data; + +namespace { + +/// @brief DHCP Parser test fixture class +class SimpleParser6Test : public ::testing::Test { + + /// @brief Checks if specified map has an integer parameter with expected value + /// + /// @param map map to be checked + /// @param param_name name of the parameter to be checked + /// @param exp_value expected value of the parameter. + void checkIntegerValue(const ConstElementPtr& map, const std::string& param_name, + int64_t exp_value) { + + // First check if the passed element is a map. + ASSERT_EQ(Element::map, map->getType()); + + // Now try to get the element being checked + ConstElementPtr elem = map->get(param_name); + ASSERT_TRUE(elem); + + // Now check if it's indeed integer + ASSERT_EQ(Element::integer, elem->getType()); + + // Finally, check if its value meets expectation. + EXPECT_EQ(exp_value, elem->intValue()); + } +}; + +// This test checks if global defaults are properly set for DHCPv6. +TEST_F(SimpleParser6Test, globalDefaults6) { + + ElementPtr empty = Element::fromJSON("{ }"); + size_t num; + + EXPECT_NO_THROW(num = SimpleParser6::setDefaults(empty)); + + // We expect at least 4 parameters to be inserted. + EXPECT_TRUE(num >= 4); + + checkIntegerValue(empty, "valid-lifetime", 7200); + checkIntegerValue(empty, "preferred-lifetime", 3600); + checkIntegerValue(empty, "rebind-timer", 1800); + checkIntegerValue(empty, "renew-timer", 900); +} + +// This test checks if the parameters can be inherited from the global +// scope to the subnet scope. +TEST_F(SimpleParser6Test, inheritGlobalToSubnet6) { + ElementPtr global = Element::fromJSON("{ \"renew-timer\": 1," + " \"rebind-timer\": 2," + " \"preferred-lifetime\": 3," + " \"valid-lifetime\": 4" + "}"); + ElementPtr subnet = Element::fromJSON("{ \"renew-timer\": 100 }"); + + // we should inherit 3 parameters. Renew-timer should remain intact, + // as it was already defined in the subnet scope. + size_t num; + EXPECT_NO_THROW(num = SimpleParser::deriveParams(global, subnet, + SimpleParser6::INHERIT_GLOBAL_TO_SUBNET6)); + EXPECT_EQ(3, num); + + // Check the values. 3 of them are interited, while the fourth one + // was already defined in the subnet, so should not be inherited. + checkIntegerValue(subnet, "renew-timer", 100); + checkIntegerValue(subnet, "rebind-timer", 2); + checkIntegerValue(subnet, "preferred-lifetime", 3); + checkIntegerValue(subnet, "valid-lifetime", 4); +} + +}; + diff --git a/src/lib/cc/data.cc b/src/lib/cc/data.cc index 6430fb6dee..fde27e7571 100644 --- a/src/lib/cc/data.cc +++ b/src/lib/cc/data.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2010-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2010-2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this diff --git a/src/lib/cc/data.h b/src/lib/cc/data.h index c4b985689c..41c6ee3013 100644 --- a/src/lib/cc/data.h +++ b/src/lib/cc/data.h @@ -1,4 +1,4 @@ -// Copyright (C) 2010-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2010-2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -277,6 +277,10 @@ public: /// \param i The position of the ElementPtr to return virtual ConstElementPtr get(const int i) const; + /// \brief returns element as non-const pointer + /// + /// \param i The position of the ElementPtr to retrieve + /// \return specified element pointer virtual ElementPtr getNonConst(const int i); /// Sets the ElementPtr at the given index. If the index is out diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc index f0376013e7..54dfaff06a 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc @@ -693,9 +693,6 @@ void OptionDataListParser::parse(const CfgOptionPtr& cfg, } // ******************************** OptionDefParser **************************** -OptionDefParser::OptionDefParser(const uint16_t address_family) - : address_family_(address_family) { -} std::pair OptionDefParser::parse(ConstElementPtr option_def) { @@ -779,10 +776,6 @@ OptionDefParser::parse(ConstElementPtr option_def) { } // ******************************** OptionDefListParser ************************ -OptionDefListParser::OptionDefListParser(const uint16_t address_family) - : address_family_(address_family) { -} - void OptionDefListParser::parse(CfgOptionDefPtr storage, ConstElementPtr option_def_list) { if (!option_def_list) { @@ -791,7 +784,7 @@ OptionDefListParser::parse(CfgOptionDefPtr storage, ConstElementPtr option_def_l << option_def_list->getPosition() << ")"); } - OptionDefParser parser(address_family_); + OptionDefParser parser; BOOST_FOREACH(ConstElementPtr option_def, option_def_list->listValue()) { OptionDefinitionTuple def; diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.h b/src/lib/dhcpsrv/parsers/dhcp_parsers.h index 365481b47e..9b69dd199e 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.h +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.h @@ -681,11 +681,6 @@ typedef std::pair OptionDefinitionT /// This parser creates an instance of a single option definition. class OptionDefParser : public isc::data::SimpleParser { public: - /// @brief Constructor. - /// - /// @param address_family Address family: @c AF_INET or AF_INET6 - OptionDefParser(const uint16_t address_family); - /// @brief Parses an entry that describes single option definition. /// /// @param option_def a configuration entry to be parsed. @@ -694,10 +689,6 @@ public: /// @throw DhcpConfigError if parsing was unsuccessful. OptionDefinitionTuple parse(isc::data::ConstElementPtr option_def); - -private: - /// @brief Address family: @c AF_INET or @c AF_INET6 - uint16_t address_family_; }; /// @brief Parser for a list of option definitions. @@ -708,14 +699,6 @@ private: /// is put into the provided storage. class OptionDefListParser : public isc::data::SimpleParser { public: - /// @brief Constructor. - /// - /// Stores address family that will be used to make certain decisions - /// during parsing. - /// - /// @param address_family @c AF_INET or @c AF_INET6 - OptionDefListParser(const uint16_t address_family); - /// @brief Parses a list of option defintions, create them and store in cfg /// /// This method iterates over def_list, which is a JSON list of option defintions, @@ -725,11 +708,6 @@ public: /// @param def_list JSON list describing option definitions /// @param cfg parsed option definitions will be stored here void parse(CfgOptionDefPtr cfg, isc::data::ConstElementPtr def_list); - -protected: - - /// @brief Address family: @c AF_INET or @c AF_INET6 - uint16_t address_family_; }; /// @brief a collection of pools diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index 52e63bf782..c29475521a 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -352,7 +352,7 @@ public: if (def_config != values_map.end()) { CfgOptionDefPtr cfg_def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef(); - OptionDefListParser def_list_parser(family); + OptionDefListParser def_list_parser; def_list_parser.parse(cfg_def, def_config->second); } From 81f4dc5039f17b5ab161840e636a0925285a4d97 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Thu, 22 Dec 2016 19:10:07 +0100 Subject: [PATCH 11/15] [5039] Doc updated, doxygen warnings removed. --- src/bin/dhcp4/dhcp4.dox | 3 + src/bin/dhcp6/dhcp6.dox | 3 + src/lib/cc/Makefile.am | 2 + src/lib/cc/cc.dox | 99 ++++++++++++++++++++++++++ src/lib/cc/simple_parser.h | 12 +++- src/lib/dhcpsrv/libdhcpsrv.dox | 87 ---------------------- src/lib/dhcpsrv/parsers/dhcp_parsers.h | 2 +- 7 files changed, 119 insertions(+), 89 deletions(-) create mode 100644 src/lib/cc/cc.dox diff --git a/src/bin/dhcp4/dhcp4.dox b/src/bin/dhcp4/dhcp4.dox index 24fe44c5ea..7397efdfa3 100644 --- a/src/bin/dhcp4/dhcp4.dox +++ b/src/bin/dhcp4/dhcp4.dox @@ -24,6 +24,9 @@ component implementation. @section dhcpv4ConfigParser Configuration Parser in DHCPv4 +Note: parsers are currently being migrated to @ref isc::data::SimpleParser. See +@ref ccSimpleParser page for details. + The common configuration parsers for the DHCP servers are located in the src/lib/dhcpsrv/parsers/ directory. Parsers specific to the DHCPv4 component are located in the src/bin/dhcp4/json_config_parser.cc. These parsers derive diff --git a/src/bin/dhcp6/dhcp6.dox b/src/bin/dhcp6/dhcp6.dox index a93ce53ded..da23fefcbc 100644 --- a/src/bin/dhcp6/dhcp6.dox +++ b/src/bin/dhcp6/dhcp6.dox @@ -24,6 +24,9 @@ component implementation. @section dhcpv6ConfigParser Configuration Parsers in DHCPv6 +Note: parsers are currently being migrated to @ref isc::data::SimpleParser. See +@ref ccSimpleParser page for details. + The common configuration parsers for the DHCP servers are located in the src/lib/dhcpsrv/parsers/ directory. Parsers specific to the DHCPv6 component are located in the src/bin/dhcp6/json_config_parser.cc. These parsers derive diff --git a/src/lib/cc/Makefile.am b/src/lib/cc/Makefile.am index 1b057bd8a5..02c46d179a 100644 --- a/src/lib/cc/Makefile.am +++ b/src/lib/cc/Makefile.am @@ -19,4 +19,6 @@ libkea_cc_la_LDFLAGS = -no-undefined -version-info 1:0:0 libkea_cc_includedir = $(pkgincludedir)/cc libkea_cc_include_HEADERS = data.h +EXTRA_DIST = cc.dox + CLEANFILES = *.gcno *.gcda diff --git a/src/lib/cc/cc.dox b/src/lib/cc/cc.dox new file mode 100644 index 0000000000..edd276f73a --- /dev/null +++ b/src/lib/cc/cc.dox @@ -0,0 +1,99 @@ +// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +/** + @page libcc libcc - Kea configuration commands library + +@section ccSimpleParser Simple JSON Parser + +Since the early beginnings, our configuration parsing code was a mess. It +started back in 2011 when Tomek joined ISC recently and was told to implement +Kea configuration handling in similar way as DNS Auth module. The code grew +over time (DHCP configuration is significantly more complex than DNS, with +more interdependent values) and as of Kea 1.1 release it became very difficult +to manage. The decision has been made to significantly refactor or even +partially rewrite the parser code. The design for this effort is documented +here: http://kea.isc.org/wiki/SimpleParser It discusses the original issues +and the proposed architecture. + +There are several aspects of this new approach. The base class for all parsers +is @ref isc::data::SimpleParser. It simplifies the parsers based on +@ref isc::dhcp::DhcpConfigParser by rejecting the +concept of build/commit phases. Instead, there should be a single method +called parse that takes ConstElementPtr as a single parameter (that's the +JSON structures to be parsed) and returns the config structure to be used +in CfgMgr. An example of such a method can be the following: + +@code +std::pair +OptionDataParser::parse(isc::data::ConstElementPtr single_option) +@endcode + +Since each derived class will have the same parameter, but a different return +type, it's not possible to use virtual methods mechanism. That's perfectly +ok, though, as there is only a single instance of the class needed to parse +arbitrary number of parameters of the same type. There is no need to +keep pointers to the parser object. As such there's fewer incentives to have +one generic way to handle all parsers. + +@subsection ccSimpleParserDefaults Default values in Simple Parser + +Another simplification comes from the fact that almost all parameters +are mandatory in SimpleParser. One source of complexities in the old +parser was the necessity to deal with optional parameters. Simple +parser deals with that by explicitly requiring the input structure to +have all parameters filled. Obviously, it's not feasible to expect +everyone to always specify all parameters, therefore there's an easy +way to fill missing parameters with their default values. There are +several methods to do this, but the most generic one is: + +@code +static size_t +isc::data::SimpleParser::setDefaults(isc::data::ElementPtr scope, + const SimpleDefaults& default_values); +@endcode + +It takes a pointer to element to be filled with default values and +vector of default values. Having those values specified in a single +place in a way that can easily be read even by non-programmers is a +big advantage of this approach. Here's an example from simple_parser.cc file: + +@code +/// This table defines default values for option definitions in DHCPv6 +const SimpleDefaults OPTION6_DEF_DEFAULTS = { + { "record-types", Element::string, ""}, + { "space", Element::string, "dhcp6"}, + { "array", Element::boolean, "false"}, + { "encapsulate", Element::string, "" } +}; +@endcode + +This array (which technically is implemented as a vector and +initialized the C++11 way) can be passed to the aforementioned +setDefaults. That code will iterate over all default values and see if +there are explicit values provided. If not, the gaps will be filled +with default values. There are also convenience methods specified for +filling in option data defaults, option definition defaults and +setAllDefaults that sets all defaults (starts with global, but then +walks down the Element tree and fills defaults in subsequent scopes). + +@subsection ccSimpleParserInherits Inheriting parameters between scopes + +SimpleParser provides a mechanism to inherit parameters between scopes, +e.g. to inherit global parameters in the subnet scope if more specific +values are not defined in the subnet scope. This is achieved by calling +@code +static size_t SimpleParser::deriveParams(isc::data::ConstElementPtr parent, + isc::data::ElementPtr child, + const ParamsList& params); + +@endcode + +ParamsList is a simple vector. There will be more specific +methods implemented in the future, but for the time being only +@ref isc::data::SimpleParser::deriveParams is implemented. + +*/ diff --git a/src/lib/cc/simple_parser.h b/src/lib/cc/simple_parser.h index af4dd63aa9..2911c64900 100644 --- a/src/lib/cc/simple_parser.h +++ b/src/lib/cc/simple_parser.h @@ -33,7 +33,8 @@ typedef std::vector ParamsList; /// @brief A simple parser /// -/// This class is intended to be a simpler replacement for @ref DhcpConfigParser. +/// This class is intended to be a simpler replacement for +/// @ref isc::dhcp::DhcpConfigParser. /// The simplification comes from several factors: /// - no build/commit nonsense. There's a single step: /// CfgStorage parse(ConstElementPtr json) @@ -83,6 +84,15 @@ class SimpleParser { static size_t setDefaults(isc::data::ElementPtr scope, const SimpleDefaults& default_values); + /// @brief Sets the default values for all entries in a list + /// + /// This is a simple utility method that iterates over all + /// parameters in a list and calls setDefaults for each + /// entry. + /// + /// @param list list to be iterated over + /// @param default_values list of default values + /// @return number of parameters inserted static size_t setListDefaults(isc::data::ElementPtr list, const SimpleDefaults& default_values); diff --git a/src/lib/dhcpsrv/libdhcpsrv.dox b/src/lib/dhcpsrv/libdhcpsrv.dox index 1c6a394618..816132a1c5 100644 --- a/src/lib/dhcpsrv/libdhcpsrv.dox +++ b/src/lib/dhcpsrv/libdhcpsrv.dox @@ -476,92 +476,5 @@ is used by DHCPv4 and DHCPv6 components. DHCPv4-over-DHCPv6 which are relayed by a DHCPv6 relay are not yet supported. -@section dhcp6SimpleParser Simple JSON Parser - -Since the early beginnings, our configuration parsing code was a mess. It -started back in 2011 when Tomek joined ISC recently and was told to implement -Kea configuration handling in similar way as DNS Auth module. The code grew -over time (DHCP configuration is significantly more complex than DNS, with -more interdependent values) and as of Kea 1.1 release it became very difficult -to manage. The decision has been made to significantly refactor or even -partially rewrite the parser code. The design for this effort is documented -here: http://kea.isc.org/wiki/SimpleParser It discusses the original issues -and the proposed architecture. - -There are several aspects of this new approach. The base class for all parsers -is @ref isc::dhcp::SimpleParser. It simplifies the parser be rejecting the -concept of build/commit phases. Instead, there should be a single method -called parse that takes ConstElementPtr as a single parameter (that's the -JSON structures to be parsed) and returns the config structure to be used -in CfgMgr. An example of such a method can be the following: - -@code -std::pair -OptionDataParser::parse(isc::data::ConstElementPtr single_option) -@endcode - -Since each derived class will have the same parameter, but a different return -type, it's not possible to use virtual methods mechanism. That's perfectly -ok, though, as there is only a single instance of the class needed to parse -arbitrary number of parameters of the same type, so there is no need to -keep pointers to the parser object. As such there's fewer incentives to have -one generic way to handle all parsers. - -@subsection dhcp6SimpleParserDefaults Default values in Simple Parser - -Another simplification comes from the fact that almost all parameters -are mandatory in SimpleParser. One source of complexities in the old -parser was the necessity to deal with optional parameters. Simple -parser deals with that by explicitly requiring the input structure to -have all parameters filled. Obviously, it's not feasible to expect -everyone to always specify all parameters, therefore there's an easy -way to fill missing parameters with their default values. There are -several methods to do this, but the most generic one is: - -@code -static size_t -isc::dhcp::SimpleParser::setDefaults(isc::data::ElementPtr scope, - const SimpleDefaults& default_values); -@endcode - -It takes a pointer to element to be filled with default values and -vector of default values. Having those values specified in a single -place in a way that can easily be read even by non-programmers is a -big advantage of this approach. Here's an example from simple_parser.cc file: - -@code -/// This table defines default values for option definitions in DHCPv6 -const SimpleDefaults OPTION6_DEF_DEFAULTS = { - { "record-types", Element::string, ""}, - { "space", Element::string, "dhcp6"}, - { "array", Element::boolean, "false"}, - { "encapsulate", Element::string, "" } -}; -@endcode - -This array (which technically is implemented as a vector and -initialized the C++11 way) can be passed to the aforementioned -setDefaults. That code will iterate over all default values and see if -there are explicit values provided. If not, the gaps will be filled -with default values. There are also convenience methods specified for -filling in option data defaults, option definition defaults and -setAllDefaults that sets all defaults (starts with global, but then -walks down the Element tree and fills defaults in subsequent scopes). - -@subsection dhcp6SimpleParserInherits Inheriting parameters between scopes - -SimpleParser provides a mechanism to inherit parameters between scopes, -e.g. to inherit global parameters in the subnet scope if more specific -values are not defined in the subnet scope. This is achieved by calling -@code -static size_t SimpleParser::deriveParams(isc::data::ConstElementPtr parent, - isc::data::ElementPtr child, - const ParamsList& params); - -@endcode - -ParamsList is a simple vector. There will be more specific -methods implemented in the future, but for the time being only -@ref isc::dhcp::SimpleParser::inheritGlobalToSubnet is implemented. */ diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.h b/src/lib/dhcpsrv/parsers/dhcp_parsers.h index 9b69dd199e..0b55c44c43 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.h +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.h @@ -552,7 +552,7 @@ public: /// /// Note: ElementPtr is expected to contain all fields. If your /// ElementPtr does not have them, please use - /// @ref SimpleParser::setOptionDefaults to fill the missing fields + /// @ref isc::data::SimpleParser::setDefaults to fill the missing fields /// with default values. /// /// @param single_option ElementPtr containing option defintion From a536d9ce45154ab4de4afbdeefb8faac5d382178 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Thu, 22 Dec 2016 19:35:04 +0100 Subject: [PATCH 12/15] [5039] getNonConst method is now const. --- src/lib/cc/data.cc | 2 +- src/lib/cc/data.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/cc/data.cc b/src/lib/cc/data.cc index fde27e7571..260a08dc76 100644 --- a/src/lib/cc/data.cc +++ b/src/lib/cc/data.cc @@ -131,7 +131,7 @@ Element::get(const int) const { } ElementPtr -Element::getNonConst(const int) { +Element::getNonConst(const int) const { throwTypeError("get(int) called on a non-list Element"); } diff --git a/src/lib/cc/data.h b/src/lib/cc/data.h index 41c6ee3013..22c9a999e6 100644 --- a/src/lib/cc/data.h +++ b/src/lib/cc/data.h @@ -281,7 +281,7 @@ public: /// /// \param i The position of the ElementPtr to retrieve /// \return specified element pointer - virtual ElementPtr getNonConst(const int i); + virtual ElementPtr getNonConst(const int i) const; /// Sets the ElementPtr at the given index. If the index is out /// of bounds, this function throws an std::out_of_range exception. @@ -633,7 +633,7 @@ public: } using Element::get; ConstElementPtr get(int i) const { return (l.at(i)); } - ElementPtr getNonConst(int i) { return (l.at(i)); } + ElementPtr getNonConst(int i) const { return (l.at(i)); } using Element::set; void set(size_t i, ElementPtr e) { l.at(i) = e; From b91411e1abdf2a75ee13e41980b20207d21f5a18 Mon Sep 17 00:00:00 2001 From: Tomek Mrugalski Date: Fri, 23 Dec 2016 16:33:32 +0100 Subject: [PATCH 13/15] [5039] Comments for default parameters added. --- src/bin/dhcp4/simple_parser4.cc | 45 ++++++++++++++++++++++++++++----- src/bin/dhcp4/simple_parser4.h | 7 +++++ src/bin/dhcp6/simple_parser6.cc | 44 +++++++++++++++++++++++++++----- src/bin/dhcp6/simple_parser6.h | 7 +++++ 4 files changed, 91 insertions(+), 12 deletions(-) diff --git a/src/bin/dhcp4/simple_parser4.cc b/src/bin/dhcp4/simple_parser4.cc index f927429fdc..1073cd2b9b 100644 --- a/src/bin/dhcp4/simple_parser4.cc +++ b/src/bin/dhcp4/simple_parser4.cc @@ -8,13 +8,29 @@ #include #include -using namespace std; using namespace isc::data; namespace isc { namespace dhcp { +/// @brief This sets of arrays define the default values and +/// values inherited (derived) between various scopes. +/// +/// Each of those is documented in @file simple_parser4.cc. This +/// is different than most other comments in Kea code. The reason +/// for placing those in .cc rather than .h file is that it +/// is expected to be one centralized place to look at for +/// the default values. This is expected to be looked at also by +/// people who are not skilled in C or C++, so they may be +/// confused with the differences between declaration and defintion. +/// As such, there's one file to look at that hopefully is readable +/// without any C or C++ skills. +/// +/// @{ -/// This table defines default values for option definitions in DHCPv4 +/// @brief This table defines default values for option definitions in DHCPv4. +/// +/// Dhcp4 may contain an array called option-def that enumerates new option +/// definitions. This array lists default values for those option definitions. const SimpleDefaults SimpleParser4::OPTION4_DEF_DEFAULTS = { { "record-types", Element::string, ""}, { "space", Element::string, "dhcp4"}, @@ -22,27 +38,44 @@ const SimpleDefaults SimpleParser4::OPTION4_DEF_DEFAULTS = { { "encapsulate", Element::string, "" } }; -/// This table defines default values for options in DHCPv4 +/// @brief This table defines default values for options in DHCPv4. +/// +/// Dhcp4 usually contains option values (option-data) defined in global, +/// subnet, class or host reservations scopes. This array lists default values +/// for those option-data declarations. const SimpleDefaults SimpleParser4::OPTION4_DEFAULTS = { { "space", Element::string, "dhcp4"}, { "csv-format", Element::boolean, "true"}, { "encapsulate", Element::string, "" } }; -/// This table defines default values for DHCPv4 +/// @brief This table defines default global values for DHCPv4 +/// +/// Some of the global parameters defined in the global scope (i.e. directly +/// in Dhcp4) are optional. If not defined, the following values will be +/// used. const SimpleDefaults SimpleParser4::GLOBAL4_DEFAULTS = { { "renew-timer", Element::integer, "900" }, { "rebind-timer", Element::integer, "1800" }, { "valid-lifetime", Element::integer, "7200" } }; -/// This list defines parameters that can be inherited from the global -/// scope to subnet6 scope. +/// @brief List of parameters that can be inherited from the global to subnet4 scope. +/// +/// Some parameters may be defined on both global (directly in Dhcp4) and +/// subnet (Dhcp4/subnet4/...) scope. If not defined in the subnet scope, +/// the value is being inherited (derived) from the global scope. This +/// array lists all of such parameters. const ParamsList SimpleParser4::INHERIT_GLOBAL_TO_SUBNET4 = { "renew-timer", "rebind-timer", "valid-lifetime" }; +/// @} + +/// --------------------------------------------------------------------------- +/// --- end of default values ------------------------------------------------- +/// --------------------------------------------------------------------------- size_t SimpleParser4::setAllDefaults(isc::data::ElementPtr global) { size_t cnt = 0; diff --git a/src/bin/dhcp4/simple_parser4.h b/src/bin/dhcp4/simple_parser4.h index 5b2fc32743..18524f9190 100644 --- a/src/bin/dhcp4/simple_parser4.h +++ b/src/bin/dhcp4/simple_parser4.h @@ -12,6 +12,12 @@ namespace isc { namespace dhcp { +/// @brief SimpleParser specialized for DHCPv4 +/// +/// This class is a @ref isc::data::SimpleParser dedicated to DHCPv4 parser. +/// In particular, it contains all the default values and names of the +/// parameters that are to be derived (inherited) between scopes. +/// For the actual values, see @file simple_parser4.cc class SimpleParser4 : public isc::data::SimpleParser { public: /// @brief Sets all defaults for DHCPv4 configuration @@ -22,6 +28,7 @@ public: /// @return number of default values added static size_t setAllDefaults(isc::data::ElementPtr global); + // see simple_parser4.cc for comments for those parameters static const isc::data::SimpleDefaults OPTION4_DEF_DEFAULTS; static const isc::data::SimpleDefaults OPTION4_DEFAULTS; static const isc::data::SimpleDefaults GLOBAL4_DEFAULTS; diff --git a/src/bin/dhcp6/simple_parser6.cc b/src/bin/dhcp6/simple_parser6.cc index 1190481439..d684177dd5 100644 --- a/src/bin/dhcp6/simple_parser6.cc +++ b/src/bin/dhcp6/simple_parser6.cc @@ -8,13 +8,29 @@ #include #include -using namespace std; using namespace isc::data; namespace isc { namespace dhcp { +/// @brief This sets of arrays define the default values and +/// values inherited (derived) between various scopes. +/// +/// Each of those is documented in @file simple_parser6.cc. This +/// is different than most other comments in Kea code. The reason +/// for placing those in .cc rather than .h file is that it +/// is expected to be one centralized place to look at for +/// the default values. This is expected to be looked at also by +/// people who are not skilled in C or C++, so they may be +/// confused with the differences between declaration and defintion. +/// As such, there's one file to look at that hopefully is readable +/// without any C or C++ skills. +/// +/// @{ -/// This table defines default values for option definitions in DHCPv6 +/// @brief This table defines default values for option definitions in DHCPv6. +/// +/// Dhcp6 may contain an array called option-def that enumerates new option +/// definitions. This array lists default values for those option definitions. const SimpleDefaults SimpleParser6::OPTION6_DEF_DEFAULTS = { { "record-types", Element::string, ""}, { "space", Element::string, "dhcp6"}, @@ -22,14 +38,22 @@ const SimpleDefaults SimpleParser6::OPTION6_DEF_DEFAULTS = { { "encapsulate", Element::string, "" } }; -/// This table defines default values for options in DHCPv6 +/// @brief This table defines default values for options in DHCPv6. +/// +/// Dhcp6 usually contains option values (option-data) defined in global, +/// subnet, class or host reservations scopes. This array lists default values +/// for those option-data declarations. const SimpleDefaults SimpleParser6::OPTION6_DEFAULTS = { { "space", Element::string, "dhcp6"}, { "csv-format", Element::boolean, "true"}, { "encapsulate", Element::string, "" } }; -/// This table defines default values for both DHCPv4 and DHCPv6 +/// @brief This table defines default global values for DHCPv6 +/// +/// Some of the global parameters defined in the global scope (i.e. directly +/// in Dhcp6) are optional. If not defined, the following values will be +/// used. const SimpleDefaults SimpleParser6::GLOBAL6_DEFAULTS = { { "renew-timer", Element::integer, "900" }, { "rebind-timer", Element::integer, "1800" }, @@ -37,15 +61,23 @@ const SimpleDefaults SimpleParser6::GLOBAL6_DEFAULTS = { { "valid-lifetime", Element::integer, "7200" } }; -/// This list defines parameters that can be inherited from the global -/// scope to subnet6 scope. +/// @brief List of parameters that can be inherited from the global to subnet6 scope. +/// +/// Some parameters may be defined on both global (directly in Dhcp6) and +/// subnet (Dhcp6/subnet6/...) scope. If not defined in the subnet scope, +/// the value is being inherited (derived) from the global scope. This +/// array lists all of such parameters. const ParamsList SimpleParser6::INHERIT_GLOBAL_TO_SUBNET6 = { "renew-timer", "rebind-timer", "preferred-lifetime", "valid-lifetime" }; +/// @} +/// --------------------------------------------------------------------------- +/// --- end of default values ------------------------------------------------- +/// --------------------------------------------------------------------------- size_t SimpleParser6::setAllDefaults(isc::data::ElementPtr global) { size_t cnt = 0; diff --git a/src/bin/dhcp6/simple_parser6.h b/src/bin/dhcp6/simple_parser6.h index 28521180cc..6093afc28e 100644 --- a/src/bin/dhcp6/simple_parser6.h +++ b/src/bin/dhcp6/simple_parser6.h @@ -12,6 +12,12 @@ namespace isc { namespace dhcp { +/// @brief SimpleParser specialized for DHCPv6 +/// +/// This class is a @ref isc::data::SimpleParser dedicated to DHCPv6 parser. +/// In particular, it contains all the default values and names of the +/// parameters that are to be derived (inherited) between scopes. +/// For the actual values, see @file simple_parser6.cc class SimpleParser6 : public isc::data::SimpleParser { public: @@ -23,6 +29,7 @@ public: /// @return number of default values added static size_t setAllDefaults(isc::data::ElementPtr global); + // see simple_parser6.cc for comments for those parameters static const isc::data::SimpleDefaults OPTION6_DEF_DEFAULTS; static const isc::data::SimpleDefaults OPTION6_DEFAULTS; static const isc::data::SimpleDefaults GLOBAL6_DEFAULTS; From 9ebc3c7eb789ad03e7180860dfb9ffeab654619c Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Sat, 24 Dec 2016 14:01:50 +0100 Subject: [PATCH 14/15] [5039] spelling --- src/lib/cc/tests/simple_parser_unittest.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/cc/tests/simple_parser_unittest.cc b/src/lib/cc/tests/simple_parser_unittest.cc index 1726306321..d61f557d07 100644 --- a/src/lib/cc/tests/simple_parser_unittest.cc +++ b/src/lib/cc/tests/simple_parser_unittest.cc @@ -71,7 +71,7 @@ TEST_F(SimpleParserTest, deriveParams) { SAMPLE_INHERITS)); EXPECT_EQ(3, num); - // Check the values. 3 of them are interited, while the fourth one + // Check the values. 3 of them are inherited, while the fourth one // was already defined in the subnet, so should not be inherited. checkIntegerValue(subnet, "renew-timer", 100); checkIntegerValue(subnet, "rebind-timer", 2); From 89151107ea46bca4ce6b45af3427c2293b86cdd1 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Sat, 24 Dec 2016 14:20:36 +0100 Subject: [PATCH 15/15] [5039] spelling (and a ~ file) --- src/bin/dhcp4/simple_parser4.cc | 4 +- .../dhcp4/tests/simple_parser4_unittest.cc | 2 +- src/bin/dhcp6/simple_parser6.cc | 2 +- .../dhcp6/tests/simple_parser6_unittest.cc | 2 +- .../dhcp6/tests/simple_parser6_unittest.cc~ | 79 ------------------- 5 files changed, 5 insertions(+), 84 deletions(-) delete mode 100644 src/bin/dhcp6/tests/simple_parser6_unittest.cc~ diff --git a/src/bin/dhcp4/simple_parser4.cc b/src/bin/dhcp4/simple_parser4.cc index 1073cd2b9b..b1c36bfbe0 100644 --- a/src/bin/dhcp4/simple_parser4.cc +++ b/src/bin/dhcp4/simple_parser4.cc @@ -21,7 +21,7 @@ namespace dhcp { /// is expected to be one centralized place to look at for /// the default values. This is expected to be looked at also by /// people who are not skilled in C or C++, so they may be -/// confused with the differences between declaration and defintion. +/// confused with the differences between declaration and definition. /// As such, there's one file to look at that hopefully is readable /// without any C or C++ skills. /// @@ -83,7 +83,7 @@ size_t SimpleParser4::setAllDefaults(isc::data::ElementPtr global) { // Set global defaults first. cnt = setDefaults(global, GLOBAL4_DEFAULTS); - // Now set option defintion defaults for each specified option definition + // Now set option definition defaults for each specified option definition ConstElementPtr option_defs = global->get("option-def"); if (option_defs) { BOOST_FOREACH(ElementPtr option_def, option_defs->listValue()) { diff --git a/src/bin/dhcp4/tests/simple_parser4_unittest.cc b/src/bin/dhcp4/tests/simple_parser4_unittest.cc index 9d02c049fd..d95e45fb3d 100644 --- a/src/bin/dhcp4/tests/simple_parser4_unittest.cc +++ b/src/bin/dhcp4/tests/simple_parser4_unittest.cc @@ -79,7 +79,7 @@ TEST_F(SimpleParser4Test, inheritGlobalToSubnet4) { SimpleParser4::INHERIT_GLOBAL_TO_SUBNET4)); EXPECT_EQ(2, num); - // Check the values. 2 of them are interited, while the third one + // Check the values. 2 of them are inherited, while the third one // was already defined in the subnet, so should not be inherited. checkIntegerValue(subnet, "renew-timer", 100); checkIntegerValue(subnet, "rebind-timer", 2); diff --git a/src/bin/dhcp6/simple_parser6.cc b/src/bin/dhcp6/simple_parser6.cc index d684177dd5..aa2844d939 100644 --- a/src/bin/dhcp6/simple_parser6.cc +++ b/src/bin/dhcp6/simple_parser6.cc @@ -21,7 +21,7 @@ namespace dhcp { /// is expected to be one centralized place to look at for /// the default values. This is expected to be looked at also by /// people who are not skilled in C or C++, so they may be -/// confused with the differences between declaration and defintion. +/// confused with the differences between declaration and definition. /// As such, there's one file to look at that hopefully is readable /// without any C or C++ skills. /// diff --git a/src/bin/dhcp6/tests/simple_parser6_unittest.cc b/src/bin/dhcp6/tests/simple_parser6_unittest.cc index e5aca51151..3721a954c1 100644 --- a/src/bin/dhcp6/tests/simple_parser6_unittest.cc +++ b/src/bin/dhcp6/tests/simple_parser6_unittest.cc @@ -75,7 +75,7 @@ TEST_F(SimpleParser6Test, inheritGlobalToSubnet6) { SimpleParser6::INHERIT_GLOBAL_TO_SUBNET6)); EXPECT_EQ(3, num); - // Check the values. 3 of them are interited, while the fourth one + // Check the values. 3 of them are inherited, while the fourth one // was already defined in the subnet, so should not be inherited. checkIntegerValue(subnet, "renew-timer", 100); checkIntegerValue(subnet, "rebind-timer", 2); diff --git a/src/bin/dhcp6/tests/simple_parser6_unittest.cc~ b/src/bin/dhcp6/tests/simple_parser6_unittest.cc~ deleted file mode 100644 index fb7dfc250d..0000000000 --- a/src/bin/dhcp6/tests/simple_parser6_unittest.cc~ +++ /dev/null @@ -1,79 +0,0 @@ -#include -#include -#include - -using namespace isc; -using namespace isc::data; - -namespace { - -/// @brief DHCP Parser test fixture class -class SimpleParser6Test : public ::testing::Test { - - /// @brief Checks if specified map has an integer parameter with expected value - /// - /// @param map map to be checked - /// @param param_name name of the parameter to be checked - /// @param exp_value expected value of the parameter. - void checkIntegerValue(const ConstElementPtr& map, const std::string& param_name, - int64_t exp_value) { - - // First check if the passed element is a map. - ASSERT_EQ(Element::map, map->getType()); - - // Now try to get the element being checked - ConstElementPtr elem = map->get(param_name); - ASSERT_TRUE(elem); - - // Now check if it's indeed integer - ASSERT_EQ(Element::integer, elem->getType()); - - // Finally, check if its value meets expectation. - EXPECT_EQ(exp_value, elem->intValue()); - } -}; - -// This test checks if global defaults are properly set for DHCPv6. -TEST_F(SimpleParser6Test, globalDefaults6) { - - ElementPtr empty = Element::fromJSON("{ }"); - size_t num; - - EXPECT_NO_THROW(num = SimpleParser6::setDefaults(empty)); - - // We expect at least 4 parameters to be inserted. - EXPECT_TRUE(num >= 4); - - checkIntegerValue(empty, "valid-lifetime", 7200); - checkIntegerValue(empty, "preferred-lifetime", 3600); - checkIntegerValue(empty, "rebind-timer", 1800); - checkIntegerValue(empty, "renew-timer", 900); -} - -// This test checks if the parameters can be inherited from the global -// scope to the subnet scope. -TEST_F(SimpleParser6Test, inheritGlobalToSubnet6) { - ElementPtr global = Element::fromJSON("{ \"renew-timer\": 1," - " \"rebind-timer\": 2," - " \"preferred-lifetime\": 3," - " \"valid-lifetime\": 4" - "}"); - ElementPtr subnet = Element::fromJSON("{ \"renew-timer\": 100 }"); - - // we should inherit 3 parameters. Renew-timer should remain intact, - // as it was already defined in the subnet scope. - size_t num; - EXPECT_NO_THROW(num = SimpleParser::deriveParams(global, subnet, - SimpleParser6::INHERIT_GLOBAL_TO_SUBNET6)); - EXPECT_EQ(3, num); - - // Check the values. 3 of them are interited, while the fourth one - // was already defined in the subnet, so should not be inherited. - checkIntegerValue(subnet, "renew-timer", 100); - checkIntegerValue(subnet, "rebind-timer", 2); - checkIntegerValue(subnet, "preferred-lifetime", 3); - checkIntegerValue(subnet, "valid-lifetime", 4); -} - -}; -