diff --git a/doc/examples/kea4/advanced.json b/doc/examples/kea4/advanced.json index 4a15f2cda3..1b0e87778a 100644 --- a/doc/examples/kea4/advanced.json +++ b/doc/examples/kea4/advanced.json @@ -44,6 +44,14 @@ "lfc-interval": 3600 }, + // This defines a control socket. If defined, Kea will open a UNIX socket + // and will listen for incoming commands. See section 15 of the Kea User's + // Guide for list of supported commands. + "control-socket": { + "socket-type": "unix", + "socket-name": "/tmp/kea4-ctrl-socket" + }, + // Addresses will be assigned with a lifetime of 4000 seconds. // The client is told to start renewing after 1000 seconds. If the server // does not respond within 2000 seconds of the lease being granted, client @@ -83,7 +91,14 @@ }, { "pools": [ { "pool": "192.0.4.1 - 192.0.4.254" } ], - "subnet": "192.0.4.0/24" + "subnet": "192.0.4.0/24", + + // Sometimes the relay may use an IPv4 address that does not match + // the subnet. This is discouraged, but there are valid cases when it + // makes sense. One case is when there is a shared subnet. + "relay": { + "ip-address": "192.168.1.1" + } } ] }, diff --git a/doc/examples/kea6/advanced.json b/doc/examples/kea6/advanced.json index b68877517b..5468a1cad2 100644 --- a/doc/examples/kea6/advanced.json +++ b/doc/examples/kea6/advanced.json @@ -1,77 +1,90 @@ -# This is an example configuration file for DHCPv6 server in Kea. -# It attempts to showcase some of the more advanced features. -# Topology wise, it's a basic scenario with one IPv6 subnet configured. -# It is assumed that one subnet (2001:db8:1::/64) is available directly -# over ethX interface. -# -# The following features are currently showcased here: -# 1. Configuration of MAC/hardware address sources in DHCPv6 +// This is an example configuration file for DHCPv6 server in Kea. +// It attempts to showcase some of the more advanced features. +// Topology wise, it's a basic scenario with one IPv6 subnet configured. +// It is assumed that one subnet (2001:db8:1::/64) is available directly +// over ethX interface. +// +// The following features are currently showcased here: +// 1. Configuration of MAC/hardware address sources in DHCPv6 +// 2. RSOO (Relay supplied options) - Some relays may insert options with the +// intention for the server to insert them into client directed messages. +// 3. Control socket. Kea can open a socket and listen for incoming +// commands. { "Dhcp6": { -# Kea is told to listen on ethX network interface only. + // Kea is told to listen on ethX network interface only. "interfaces-config": { "interfaces": [ "ethX" ] }, -# We need to specify the the database used to store leases. As of -# September 2016, four database backends are supported: MySQL, -# PostgreSQL, Cassandra, and the in-memory database, Memfile. -# We will use memfile because it doesn't require any prior set up. + // We need to specify the the database used to store leases. As of + // September 2016, four database backends are supported: MySQL, + // PostgreSQL, Cassandra, and the in-memory database, Memfile. + // We will use memfile because it doesn't require any prior set up. "lease-database": { "type": "memfile", "lfc-interval": 3600 }, -# Kea 0.9.1 introduced MAC/hardware addresses support in DHCPv6. There is -# no single reliable method of getting MAC address information in DHCPv6. -# Kea supports several methods. Depending on your network set up, some -# methods may be more preferable than others, hence the configuration -# parameter. 'mac-sources' is a list of methods. Allowed parameters are: -# any, raw, duid, ipv6-link-local, client-link-addr-option, rfc6939 (which -# is an alias for client-link-addr-option), remote-id, rfc4649 (which is an -# alias for remote-id, subscriber-id, rfc4580 (which is an alias for -# subscriber-id) and docsis. -# -# Note that the order matters. Methods are attempted one by one in the order -# specified until hardware address is obtained. If you don't care which method -# is used, using 'any' is marginally faster than enumerating them all. -# -# If mac-sources are not specified, a default value of 'any' is used. +// Kea 0.9.1 introduced MAC/hardware addresses support in DHCPv6. There is +// no single reliable method of getting MAC address information in DHCPv6. +// Kea supports several methods. Depending on your network set up, some +// methods may be more preferable than others, hence the configuration +// parameter. 'mac-sources' is a list of methods. Allowed parameters are: +// any, raw, duid, ipv6-link-local, client-link-addr-option, rfc6939 (which +// is an alias for client-link-addr-option), remote-id, rfc4649 (which is an +// alias for remote-id, subscriber-id, rfc4580 (which is an alias for +// subscriber-id) and docsis. +// +// Note that the order matters. Methods are attempted one by one in the order +// specified until hardware address is obtained. If you don't care which method +// is used, using 'any' is marginally faster than enumerating them all. +// +// If mac-sources are not specified, a default value of 'any' is used. "mac-sources": [ "client-link-addr-option", "duid", "ipv6-link-local" ], -# RFC6422 defines a mechanism called relay-supplied options option. The relay -# agent may insert certain options that the server will echo back to the -# client, if certain criteria are met. One condition is that the option must -# be RSOO-enabled (i.e. allowed to be echoed back). IANA maintains a list -# of those options here: -# http://www.iana.org/assignments/dhcpv6-parameters/dhcpv6-parameters.xhtml#options-relay-supplied -# However, it is possible to allow the server to echo back additional options. -# This entry marks options 110, 120 and 130 as RSOO-enabled. +// RFC6422 defines a mechanism called relay-supplied options option. The relay +// agent may insert certain options that the server will echo back to the +// client, if certain criteria are met. One condition is that the option must +// be RSOO-enabled (i.e. allowed to be echoed back). IANA maintains a list +// of those options here: +// http://www.iana.org/assignments/dhcpv6-parameters/dhcpv6-parameters.xhtml#options-relay-supplied +// However, it is possible to allow the server to echo back additional options. +// This entry marks options 110, 120 and 130 as RSOO-enabled. "relay-supplied-options": [ "110", "120", "130" ], -# Addresses will be assigned with preferred and valid lifetimes -# being 3000 and 4000, respectively. Client is told to start -# renewing after 1000 seconds. If the server does not respond -# after 2000 seconds since the lease was granted, client is supposed -# to start REBIND procedure (emergency renewal that allows switching -# to a different server). + + // This defines a control socket. If defined, Kea will open a UNIX socket + // and will listen for incoming commands. See section 15 of the Kea User's + // Guide for list of supported commands. + "control-socket": { + "socket-type": "unix", + "socket-name": "/tmp/kea6-ctrl-socket" + }, + +// Addresses will be assigned with preferred and valid lifetimes +// being 3000 and 4000, respectively. Client is told to start +// renewing after 1000 seconds. If the server does not respond +// after 2000 seconds since the lease was granted, client is supposed +// to start REBIND procedure (emergency renewal that allows switching +// to a different server). "preferred-lifetime": 3000, "valid-lifetime": 4000, "renew-timer": 1000, "rebind-timer": 2000, -# The following list defines subnets. Each subnet consists of at -# least subnet and pool entries. +// The following list defines subnets. Each subnet consists of at +// least subnet and pool entries. "subnet6": [ { "pools": [ { "pool": "2001:db8:1::/80" } ], -# This defines PD (prefix delegation) pools. In this case -# we have only one pool. That consists of /64 prefixes -# being delegated out of large /48 pool. Each delegated -# prefix will contain an excluded-prefix option. +// This defines PD (prefix delegation) pools. In this case +// we have only one pool. That consists of /64 prefixes +// being delegated out of large /48 pool. Each delegated +// prefix will contain an excluded-prefix option. "pd-pools": [ { "prefix": "2001:db8:abcd::", @@ -82,13 +95,21 @@ } ], "subnet": "2001:db8:1::/64", - "interface": "ethX" + "interface": "ethX", + + // Sometimes the relay may use an odd IPv6 address that's not matching + // the subnet. This is discouraged, but there are valid cases when it + // makes sense. One case is when the relay has only link-local address + // and another is when there is a shared subnet scenario. + "relay": { + "ip-address": "3000::1" + } } ] }, -# The following configures logging. It assumes that messages with at least -# informational level (info, warn, error and fatal) should be logged to stdout. +// The following configures logging. It assumes that messages with at least +// informational level (info, warn, error and fatal) should be logged to stdout. "Logging": { "loggers": [ { diff --git a/doc/guide/dhcp4-srv.xml b/doc/guide/dhcp4-srv.xml index c2423b1266..6c74a2fada 100644 --- a/doc/guide/dhcp4-srv.xml +++ b/doc/guide/dhcp4-srv.xml @@ -3203,6 +3203,9 @@ src/lib/dhcpsrv/cfg_host_operations.cc --> + If "relay" is specified, the "ip-address" parameter within + it is mandatory. +
diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml index 27fe542a90..166c932e66 100644 --- a/doc/guide/dhcp6-srv.xml +++ b/doc/guide/dhcp6-srv.xml @@ -3410,6 +3410,9 @@ If not specified, the default value is: + If "relay" is specified, the "ip-address" parameter within + it is mandatory. +
@@ -3500,7 +3503,8 @@ If not specified, the default value is: When not specified, a special value of "any" is used, which instructs the server to attempt to use all the methods in sequence and use - value returned by the first one that succeeds. + value returned by the first one that succeeds. If specified, it + has to have at least one value. Supported methods are: diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index d291e1444a..c59138ba21 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -189,8 +189,7 @@ protected: parser = new StringParser(config_id, string_values_); } else if (config_id.compare("pools") == 0) { parser = new Pools4ListParser(config_id, pools_); - } else if (config_id.compare("relay") == 0) { - parser = new RelayInfoParser(config_id, relay_info_, Option::V4); + // relay has been converted to SimpleParser already. // option-data has been converted to SimpleParser already. } else if (config_id.compare("match-client-id") == 0) { parser = new BooleanParser(config_id, boolean_values_); @@ -440,8 +439,7 @@ DhcpConfigParser* createGlobalDhcp4ConfigParser(const std::string& config_id, parser = new D2ClientConfigParser(config_id); } else if (config_id.compare("match-client-id") == 0) { parser = new BooleanParser(config_id, globalContext()->boolean_values_); - } else if (config_id.compare("control-socket") == 0) { - parser = new ControlSocketParser(config_id); + // control-socket has been converted to SimpleParser already. } else if (config_id.compare("expired-leases-processing") == 0) { parser = new ExpirationConfigParser(); } else if (config_id.compare("client-classes") == 0) { @@ -637,6 +635,13 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) { continue; } + if (config_pair.first == "control-socket") { + ControlSocketParser parser; + SrvConfigPtr srv_cfg = CfgMgr::instance().getStagingCfg(); + parser.parse(*srv_cfg, config_pair.second); + continue; + } + ParserPtr parser(createGlobalDhcp4ConfigParser(config_pair.first, config_pair.second)); LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PARSER_CREATED) diff --git a/src/bin/dhcp4/tests/parser_unittest.cc b/src/bin/dhcp4/tests/parser_unittest.cc index d39652e9fd..20f6c849d8 100644 --- a/src/bin/dhcp4/tests/parser_unittest.cc +++ b/src/bin/dhcp4/tests/parser_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2016-2017 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 @@ -7,14 +7,14 @@ #include #include #include -#include -#include -#include +#include using namespace isc::data; using namespace std; -namespace { +namespace isc { +namespace dhcp { +namespace test { /// @brief compares two JSON trees /// @@ -128,6 +128,8 @@ TEST(ParserTest, keywordDhcp4) { testParser(txt, Parser4Context::PARSER_DHCP4); } +// Tests if bash (#) comments are supported. That's the only comment type that +// was supported by the old parser. TEST(ParserTest, bashComments) { string txt= "{ \"Dhcp4\": { \"interfaces-config\": {" " \"interfaces\": [ \"*\" ]" @@ -146,7 +148,8 @@ TEST(ParserTest, bashComments) { testParser(txt, Parser4Context::PARSER_DHCP4, false); } -TEST(ParserTest, cComments) { +// Tests if C++ (//) comments can start anywhere, not just in the first line. +TEST(ParserTest, cppComments) { string txt= "{ \"Dhcp4\": { \"interfaces-config\": {" " \"interfaces\": [ \"*\" ]" "},\n" @@ -161,6 +164,7 @@ TEST(ParserTest, cComments) { testParser(txt, Parser4Context::PARSER_DHCP4, false); } +// Tests if bash (#) comments can start anywhere, not just in the first line. TEST(ParserTest, bashCommentsInline) { string txt= "{ \"Dhcp4\": { \"interfaces-config\": {" " \"interfaces\": [ \"*\" ]" @@ -176,6 +180,7 @@ TEST(ParserTest, bashCommentsInline) { testParser(txt, Parser4Context::PARSER_DHCP4, false); } +// Tests if multi-line C style comments are handled correctly. TEST(ParserTest, multilineComments) { string txt= "{ \"Dhcp4\": { \"interfaces-config\": {" " \"interfaces\": [ \"*\" ]" @@ -193,89 +198,13 @@ TEST(ParserTest, multilineComments) { testParser(txt, Parser4Context::PARSER_DHCP4, false); } -/// @brief removes comments from a JSON file -/// -/// This is rather naive implementation, but it's probably sufficient for -/// testing. It won't be able to pick any trickier cases, like # or // -/// appearing in strings, nested C++ comments etc. -/// -/// @param input_file file to be stripped of comments -/// @return a new file that has comments stripped from it -std::string decommentJSONfile(const std::string& input_file) { - ifstream f(input_file); - if (!f.is_open()) { - isc_throw(isc::BadValue, "can't open input file for reading: " + input_file); - } - - string outfile; - size_t last_slash = input_file.find_last_of("/"); - if (last_slash != string::npos) { - outfile = input_file.substr(last_slash + 1); - } else { - outfile = input_file; - } - outfile += "-decommented"; - - ofstream out(outfile); - if (!out.is_open()) { - isc_throw(isc::BadValue, "can't open output file for writing: " + input_file); - } - - bool in_comment = false; - string line; - while (std::getline(f, line)) { - // First, let's get rid of the # comments - size_t hash_pos = line.find("#"); - if (hash_pos != string::npos) { - line = line.substr(0, hash_pos); - } - - // Second, let's get rid of the // comments - size_t dblslash_pos = line.find("//"); - if (dblslash_pos != string::npos) { - line = line.substr(0, dblslash_pos); - } - - // Now the tricky part: c comments. - size_t begin_pos = line.find("/*"); - size_t end_pos = line.find("*/"); - if (in_comment && end_pos == string::npos) { - // we continue through multiline comment - line = ""; - } else { - - if (begin_pos != string::npos) { - in_comment = true; - if (end_pos != string::npos) { - // sigle line comment. Let's get rid of the content in between - line = line.replace(begin_pos, end_pos + 2, end_pos + 2 - begin_pos, ' '); - in_comment = false; - } else { - line = line.substr(0, begin_pos); - } - } else { - if (in_comment && end_pos != string::npos) { - line = line.replace(0, end_pos +2 , end_pos + 2, ' '); - in_comment = false; - } - } - } - - // Finally, write the line to the output file. - out << line << endl; - } - f.close(); - out.close(); - - return (outfile); -} /// @brief Loads specified example config file /// /// This test loads specified example file twice: first, using the legacy /// JSON file and then second time using bison parser. Two created Element /// trees are then compared. The input is decommented before it is passed -/// to legacy parser (as its support for comments is very limited). +/// to legacy parser (as legacy support for comments is very limited). /// /// @param fname name of the file to be loaded void testFile(const std::string& fname) { @@ -284,8 +213,7 @@ void testFile(const std::string& fname) { string decommented = decommentJSONfile(fname); - cout << "Attempting to load file " << fname << " (" << decommented - << ")" << endl; + cout << "Parsing file " << fname << " (" << decommented << ")" << endl; EXPECT_NO_THROW(reference_json = Element::fromJSONFile(decommented, true)); @@ -329,6 +257,11 @@ TEST(ParserTest, file) { } } +/// @brief Tests error conditions in Dhcp4Parser +/// +/// @param txt text to be parsed +/// @param parser_type type of the parser to be used in the test +/// @param msg expected content of the exception void testError(const std::string& txt, Parser4Context::ParserType parser_type, const std::string& msg) @@ -347,7 +280,7 @@ void testError(const std::string& txt, } } -// Check errors +// Verify that error conditions are handled correctly. TEST(ParserTest, errors) { // no input testError("", Parser4Context::PARSER_JSON, @@ -591,7 +524,7 @@ TEST(ParserTest, unicodeEscapes) { } } -// This test checks that all representations of a slash is recognized properly. +// This test checks that all representations of a slash are recognized properly. TEST(ParserTest, unicodeSlash) { // check the 4 possible encodings of solidus '/' ConstElementPtr result; @@ -609,3 +542,5 @@ TEST(ParserTest, unicodeSlash) { } }; +}; +}; diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index 04e1e0fcf0..914fbee53d 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -424,8 +424,7 @@ protected: parser = new StringParser(config_id, string_values_); } else if (config_id.compare("pools") == 0) { parser = new Pools6ListParser(config_id, pools_); - } else if (config_id.compare("relay") == 0) { - parser = new RelayInfoParser(config_id, relay_info_, Option::V6); + // relay has been converted to SimpleParser. } else if (config_id.compare("pd-pools") == 0) { parser = new PdPoolListParser(config_id, pools_); // option-data was here, but it is now converted to SimpleParser @@ -718,13 +717,10 @@ DhcpConfigParser* createGlobal6DhcpConfigParser(const std::string& config_id, parser = new HooksLibrariesParser(config_id); } else if (config_id.compare("dhcp-ddns") == 0) { parser = new D2ClientConfigParser(config_id); - } else if (config_id.compare("mac-sources") == 0) { - parser = new MACSourcesListConfigParser(config_id, - globalContext()); + // mac-source has been converted to SimpleParser. } else if (config_id.compare("relay-supplied-options") == 0) { parser = new RSOOListConfigParser(config_id); - } else if (config_id.compare("control-socket") == 0) { - parser = new ControlSocketParser(config_id); + // control-socket has been converted to SimpleParser. } else if (config_id.compare("expired-leases-processing") == 0) { parser = new ExpirationConfigParser(); } else if (config_id.compare("client-classes") == 0) { @@ -911,6 +907,20 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) { continue; } + if (config_pair.first == "mac-sources") { + MACSourcesListConfigParser parser; + CfgMACSource& mac_source = CfgMgr::instance().getStagingCfg()->getMACSources(); + parser.parse(mac_source, config_pair.second); + continue; + } + + if (config_pair.first == "control-socket") { + ControlSocketParser parser; + SrvConfigPtr srv_config = CfgMgr::instance().getStagingCfg(); + parser.parse(*srv_config, config_pair.second); + continue; + } + ParserPtr parser(createGlobal6DhcpConfigParser(config_pair.first, config_pair.second)); LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, DHCP6_PARSER_CREATED) diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index 051486c3c4..fc479fc12e 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012-2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2017 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 @@ -251,6 +251,9 @@ public: ASSERT_TRUE(status); comment_ = parseAnswer(rcode_, status); EXPECT_EQ(expected_code, rcode_); + if (expected_code != rcode_) { + cout << "The comment returned was: [" << comment_->stringValue() << "]" << endl; + } } /// @brief Returns an interface configuration used by the most of the @@ -762,7 +765,7 @@ TEST_F(Dhcp6ParserTest, emptySubnet) { ConstElementPtr status; EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); - + // returned value should be 0 (success) checkResult(status, 0); } @@ -4292,16 +4295,15 @@ TEST_F(Dhcp6ParserTest, reservationBogus) { } /// The goal of this test is to verify that configuration can include -/// MAC/Hardware sources. This test also checks if the aliases are -/// handled properly (rfc6939 = client-addr-relay, rfc4649 = remote-id, -/// rfc4580 = subscriber-id). -TEST_F(Dhcp6ParserTest, macSources) { +/// MAC/Hardware sources. This case uses RFC numbers to reference methods. +/// Also checks if the aliases are handled properly (rfc6939 = client-addr-relay, +/// rfc4649 = remote-id, rfc4580 = subscriber-id). +TEST_F(Dhcp6ParserTest, macSources1) { ConstElementPtr json; ASSERT_NO_THROW(json = parseDHCP6("{ " + genIfaceConfig() + "," - "\"mac-sources\": [ \"rfc6939\", \"rfc4649\", \"rfc4580\"," - "\"client-link-addr-option\", \"remote-id\", \"subscriber-id\"]," + "\"mac-sources\": [ \"rfc6939\", \"rfc4649\", \"rfc4580\" ]," "\"preferred-lifetime\": 3000," "\"rebind-timer\": 2000, " "\"renew-timer\": 1000, " @@ -4310,28 +4312,49 @@ TEST_F(Dhcp6ParserTest, macSources) { ConstElementPtr status; EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); - - // returned value should be 0 (success) checkResult(status, 0); - CfgMACSources mac_sources = CfgMgr::instance().getStagingCfg()->getMACSources().get(); - ASSERT_EQ(6, mac_sources.size()); - // Let's check the aliases. They should be recognized to their base methods. - EXPECT_EQ(HWAddr::HWADDR_SOURCE_CLIENT_ADDR_RELAY_OPTION, mac_sources[0]); - EXPECT_EQ(HWAddr::HWADDR_SOURCE_REMOTE_ID, mac_sources[1]); - EXPECT_EQ(HWAddr::HWADDR_SOURCE_SUBSCRIBER_ID, mac_sources[2]); + CfgMACSources sources = CfgMgr::instance().getStagingCfg()->getMACSources().get(); - // Let's check if the actual methods are recognized properly. - EXPECT_EQ(HWAddr::HWADDR_SOURCE_CLIENT_ADDR_RELAY_OPTION, mac_sources[3]); - EXPECT_EQ(HWAddr::HWADDR_SOURCE_REMOTE_ID, mac_sources[4]); - EXPECT_EQ(HWAddr::HWADDR_SOURCE_SUBSCRIBER_ID, mac_sources[5]); + ASSERT_EQ(3, sources.size()); + // Let's check the aliases. They should be recognized to their base methods. + EXPECT_EQ(HWAddr::HWADDR_SOURCE_CLIENT_ADDR_RELAY_OPTION, sources[0]); + EXPECT_EQ(HWAddr::HWADDR_SOURCE_REMOTE_ID, sources[1]); + EXPECT_EQ(HWAddr::HWADDR_SOURCE_SUBSCRIBER_ID, sources[2]); } -/// The goal of this test is to verify that MAC sources configuration can be -/// empty. -/// Note the Dhcp6 parser requires the list to NOT be empty?! -TEST_F(Dhcp6ParserTest, macSourcesEmpty) { +/// The goal of this test is to verify that configuration can include +/// MAC/Hardware sources. This uses specific method names. +TEST_F(Dhcp6ParserTest, macSources2) { + ConstElementPtr json; + ASSERT_NO_THROW(json = + parseDHCP6("{ " + genIfaceConfig() + "," + "\"mac-sources\": [ \"client-link-addr-option\", \"remote-id\", " + " \"subscriber-id\"]," + "\"preferred-lifetime\": 3000," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"subnet6\": [ ], " + "\"valid-lifetime\": 4000 }")); + + ConstElementPtr status; + EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); + checkResult(status, 0); + + CfgMACSources sources = CfgMgr::instance().getStagingCfg()->getMACSources().get(); + + ASSERT_EQ(3, sources.size()); + // Let's check the aliases. They should be recognized to their base methods. + EXPECT_EQ(HWAddr::HWADDR_SOURCE_CLIENT_ADDR_RELAY_OPTION, sources[0]); + EXPECT_EQ(HWAddr::HWADDR_SOURCE_REMOTE_ID, sources[1]); + EXPECT_EQ(HWAddr::HWADDR_SOURCE_SUBSCRIBER_ID, sources[2]); +} + + +/// The goal of this test is to verify that empty MAC sources configuration +/// is rejected. If specified, this has to have at least one value. +TEST_F(Dhcp6ParserTest, macSourcesEmpty) { ConstElementPtr status; EXPECT_NO_THROW(status = configureDhcp6Server(srv_, @@ -4343,11 +4366,9 @@ TEST_F(Dhcp6ParserTest, macSourcesEmpty) { "\"subnet6\": [ ], " "\"valid-lifetime\": 4000 }"))); - // returned value should be 0 (success) - checkResult(status, 0); - - CfgMACSources mac_sources = CfgMgr::instance().getStagingCfg()->getMACSources().get(); - EXPECT_EQ(0, mac_sources.size()); + // returned value should be 1 (failure), because the mac-sources must not + // be empty when specified. + checkResult(status, 1); } /// The goal of this test is to verify that MAC sources configuration can diff --git a/src/bin/dhcp6/tests/parser_unittest.cc b/src/bin/dhcp6/tests/parser_unittest.cc index 75bc5fdbd3..821ad2de2a 100644 --- a/src/bin/dhcp6/tests/parser_unittest.cc +++ b/src/bin/dhcp6/tests/parser_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2016-2017 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 @@ -7,28 +7,42 @@ #include #include #include +#include using namespace isc::data; using namespace std; -namespace { +namespace isc { +namespace dhcp { +namespace test { -void compareJSON(ConstElementPtr a, ConstElementPtr b, bool print = true) { +/// @brief compares two JSON trees +/// +/// If differences are discovered, gtest failure is reported (using EXPECT_EQ) +/// +/// @param a first to be compared +/// @param b second to be compared +void compareJSON(ConstElementPtr a, ConstElementPtr b) { ASSERT_TRUE(a); ASSERT_TRUE(b); - if (print) { - // std::cout << "JSON A: -----" << endl << a->str() << std::endl; - // std::cout << "JSON B: -----" << endl << b->str() << std::endl; - // cout << "---------" << endl << endl; - } EXPECT_EQ(a->str(), b->str()); } -void testParser(const std::string& txt, Parser6Context::ParserType parser_type) { - ElementPtr reference_json; +/// @brief Tests if the input string can be parsed with specific parser +/// +/// The input text will be passed to bison parser of specified type. +/// Then the same input text is passed to legacy JSON parser and outputs +/// from both parsers are compared. The legacy comparison can be disabled, +/// if the feature tested is not supported by the old parser (e.g. +/// new comment styles) +/// +/// @param txt text to be compared +/// @param parser_type bison parser type to be instantiated +/// @param compare whether to compare the output with legacy JSON parser +void testParser(const std::string& txt, Parser6Context::ParserType parser_type, + bool compare = true) { ConstElementPtr test_json; - ASSERT_NO_THROW(reference_json = Element::fromJSON(txt, true)); ASSERT_NO_THROW({ try { Parser6Context ctx; @@ -40,29 +54,16 @@ void testParser(const std::string& txt, Parser6Context::ParserType parser_type) }); + if (!compare) { + return; + } + // Now compare if both representations are the same. + ElementPtr reference_json; + ASSERT_NO_THROW(reference_json = Element::fromJSON(txt, true)); compareJSON(reference_json, test_json); } -void testParser2(const std::string& txt, Parser6Context::ParserType parser_type) { - ConstElementPtr test_json; - - ASSERT_NO_THROW({ - try { - Parser6Context ctx; - test_json = ctx.parseString(txt, parser_type); - } catch (const std::exception &e) { - cout << "EXCEPTION: " << e.what() << endl; - throw; - } - }); - /// @todo: Implement actual validation here. since the original - /// Element::fromJSON does not support several comment types, we don't - /// have anything to compare with. - /// std::cout << "Original text:" << txt << endl; - /// std::cout << "Parsed text :" << test_json->str() << endl; -} - TEST(ParserTest, mapInMap) { string txt = "{ \"xyzzy\": { \"foo\": 123, \"baz\": 456 } }"; testParser(txt, Parser6Context::PARSER_JSON); @@ -125,9 +126,11 @@ TEST(ParserTest, keywordDhcp6) { " \"subnet\": \"2001:db8:1::/48\", " " \"interface\": \"test\" } ],\n" "\"valid-lifetime\": 4000 } }"; - testParser2(txt, Parser6Context::PARSER_DHCP6); + testParser(txt, Parser6Context::PARSER_DHCP6); } +// Tests if bash (#) comments are supported. That's the only comment type that +// was supported by the old parser. TEST(ParserTest, bashComments) { string txt= "{ \"Dhcp6\": { \"interfaces-config\": {" " \"interfaces\": [ \"*\" ]" @@ -144,10 +147,11 @@ TEST(ParserTest, bashComments) { " \"interface\": \"eth0\"" " } ]," "\"valid-lifetime\": 4000 } }"; - testParser2(txt, Parser6Context::PARSER_DHCP6); + testParser(txt, Parser6Context::PARSER_DHCP6); } -TEST(ParserTest, cComments) { +// Tests if C++ (//) comments can start anywhere, not just in the first line. +TEST(ParserTest, cppComments) { string txt= "{ \"Dhcp6\": { \"interfaces-config\": {" " \"interfaces\": [ \"*\" ]" "},\n" @@ -160,9 +164,10 @@ TEST(ParserTest, cComments) { " \"interface\": \"eth0\"" " } ]," "\"valid-lifetime\": 4000 } }"; - testParser2(txt, Parser6Context::PARSER_DHCP6); + testParser(txt, Parser6Context::PARSER_DHCP6, false); } +// Tests if bash (#) comments can start anywhere, not just in the first line. TEST(ParserTest, bashCommentsInline) { string txt= "{ \"Dhcp6\": { \"interfaces-config\": {" " \"interfaces\": [ \"*\" ]" @@ -176,9 +181,10 @@ TEST(ParserTest, bashCommentsInline) { " \"interface\": \"eth0\"" " } ]," "\"valid-lifetime\": 4000 } }"; - testParser2(txt, Parser6Context::PARSER_DHCP6); + testParser(txt, Parser6Context::PARSER_DHCP6, false); } +// Tests if multi-line C style comments are handled correctly. TEST(ParserTest, multilineComments) { string txt= "{ \"Dhcp6\": { \"interfaces-config\": {" " \"interfaces\": [ \"*\" ]" @@ -193,17 +199,29 @@ TEST(ParserTest, multilineComments) { " \"interface\": \"eth0\"" " } ]," "\"valid-lifetime\": 4000 } }"; - testParser2(txt, Parser6Context::PARSER_DHCP6); + testParser(txt, Parser6Context::PARSER_DHCP6, false); } - -void testFile(const std::string& fname, bool print) { +/// @brief Loads specified example config file +/// +/// This test loads specified example file twice: first, using the legacy +/// JSON file and then second time using bison parser. Two created Element +/// trees are then compared. The input is decommented before it is passed +/// to legacy parser (as legacy support for comments is very limited). +/// +/// @param fname name of the file to be loaded +void testFile(const std::string& fname) { ElementPtr reference_json; ConstElementPtr test_json; - cout << "Attempting to load file " << fname << endl; + string decommented = decommentJSONfile(fname); - EXPECT_NO_THROW(reference_json = Element::fromJSONFile(fname, true)); + cout << "Parsing file " << fname << "(" << decommented << ")" << endl; + + EXPECT_NO_THROW(reference_json = Element::fromJSONFile(decommented, true)); + + // remove the temporary file + EXPECT_NO_THROW(::remove(decommented.c_str())); EXPECT_NO_THROW( try { @@ -217,9 +235,7 @@ void testFile(const std::string& fname, bool print) { ASSERT_TRUE(reference_json); ASSERT_TRUE(test_json); - compareJSON(reference_json, test_json, print); - - + compareJSON(reference_json, test_json); } // This test loads all available existing files. Each config is loaded @@ -243,10 +259,15 @@ TEST(ParserTest, file) { configs.push_back("stateless.json"); for (int i = 0; igetType()); EXPECT_EQ(ins, result->stringValue()); } +} +// This test checks that all representations of a slash is recognized properly. +TEST(ParserTest, unicodeSlash) { // check the 4 possible encodings of solidus '/' - json = "\"/\\/\\u002f\\u002F\""; + ConstElementPtr result; + string json = "\"/\\/\\u002f\\u002F\""; ASSERT_NO_THROW( try { Parser6Context ctx; @@ -520,6 +545,8 @@ TEST(ParserTest, unicodeEscapes) { }); ASSERT_EQ(Element::string, result->getType()); EXPECT_EQ("////", result->stringValue()); -} +} }; +}; +}; diff --git a/src/lib/dhcpsrv/cfg_mac_source.cc b/src/lib/dhcpsrv/cfg_mac_source.cc index 4307d407c1..9cc1dbd35f 100644 --- a/src/lib/dhcpsrv/cfg_mac_source.cc +++ b/src/lib/dhcpsrv/cfg_mac_source.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2015,2017 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 @@ -47,6 +47,16 @@ uint32_t CfgMACSource::MACSourceFromText(const std::string& name) { isc_throw(BadValue, "Can't convert '" << name << "' to any known MAC source."); } +void CfgMACSource::add(uint32_t source) { + for (CfgMACSources::const_iterator it = mac_sources_.begin(); + it != mac_sources_.end(); ++it) { + if (*it == source) { + isc_throw(InvalidParameter, "mac-source paramter " << source + << "' specified twice."); + } + } + mac_sources_.push_back(source); +} }; }; diff --git a/src/lib/dhcpsrv/cfg_mac_source.h b/src/lib/dhcpsrv/cfg_mac_source.h index cddfceedfb..11ff2870c0 100644 --- a/src/lib/dhcpsrv/cfg_mac_source.h +++ b/src/lib/dhcpsrv/cfg_mac_source.h @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2015,2017 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 @@ -52,10 +52,8 @@ class CfgMACSource { /// @param source MAC source (see constants in Pkt::HWADDR_SOURCE_*) /// /// Specified source is being added to the mac_sources_ array. - /// @todo implement add(string) version of this method. - void add(uint32_t source) { - mac_sources_.push_back(source); - } + /// @throw InvalidParameter if such a source is already defined. + void add(uint32_t source); /// @brief Provides access to the configure MAC/Hardware address sources. /// diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc index f3d58d5b52..cf3e116b76 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2017 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 @@ -177,62 +177,48 @@ template <> void ValueParser::build(ConstElementPtr value) { // ******************** MACSourcesListConfigParser ************************* -MACSourcesListConfigParser:: -MACSourcesListConfigParser(const std::string& param_name, - ParserContextPtr global_context) - : param_name_(param_name), global_context_(global_context) { - if (param_name_ != "mac-sources") { - isc_throw(BadValue, "Internal error. MAC sources configuration " - "parser called for the wrong parameter: " << param_name); - } -} - void -MACSourcesListConfigParser::build(ConstElementPtr value) { +MACSourcesListConfigParser::parse(CfgMACSource& mac_sources, ConstElementPtr value) { CfgIface cfg_iface; uint32_t source = 0; + size_t cnt = 0; // By default, there's only one source defined: ANY. // If user specified anything, we need to get rid of that default. - CfgMgr::instance().getStagingCfg()->getMACSources().clear(); + mac_sources.clear(); BOOST_FOREACH(ConstElementPtr source_elem, value->listValue()) { std::string source_str = source_elem->stringValue(); try { source = CfgMACSource::MACSourceFromText(source_str); - CfgMgr::instance().getStagingCfg()->getMACSources().add(source); + mac_sources.add(source); + ++cnt; + } catch (const InvalidParameter& ex) { + isc_throw(DhcpConfigError, "The mac-sources value '" << source_str + << "' was specified twice (" << value->getPosition() << ")"); } catch (const std::exception& ex) { isc_throw(DhcpConfigError, "Failed to convert '" << source_str << "' to any recognized MAC source:" << ex.what() << " (" << value->getPosition() << ")"); } } -} -void -MACSourcesListConfigParser::commit() { - // Nothing to do. + if (!cnt) { + isc_throw(DhcpConfigError, "If specified, MAC Sources cannot be empty"); + } } // ******************** ControlSocketParser ************************* -ControlSocketParser::ControlSocketParser(const std::string& param_name) { - if (param_name != "control-socket") { - isc_throw(BadValue, "Internal error. Control socket parser called " - " for wrong parameter:" << param_name); +void ControlSocketParser::parse(SrvConfig& srv_cfg, isc::data::ConstElementPtr value) { + if (!value) { + isc_throw(DhcpConfigError, "Logic error: specified control-socket is null"); } -} -void ControlSocketParser::build(isc::data::ConstElementPtr value) { if (value->getType() != Element::map) { - isc_throw(BadValue, "Specified control-socket is expected to be a map" + isc_throw(DhcpConfigError, "Specified control-socket is expected to be a map" ", i.e. a structure defined within { }"); } - CfgMgr::instance().getStagingCfg()->setControlSocketInfo(value); -} - -/// @brief Does nothing. -void ControlSocketParser::commit() { - // Nothing to do. + srv_cfg.setControlSocketInfo(value); } // ******************** HooksLibrariesParser ************************* @@ -804,66 +790,66 @@ OptionDefListParser::parse(CfgOptionDefPtr storage, ConstElementPtr option_def_l } //****************************** RelayInfoParser ******************************** -RelayInfoParser::RelayInfoParser(const std::string&, - const isc::dhcp::Subnet::RelayInfoPtr& relay_info, - const Option::Universe& family) - :storage_(relay_info), local_(isc::asiolink::IOAddress( - family == Option::V4 ? "0.0.0.0" : "::")), - string_values_(new StringStorage()), family_(family) { - if (!relay_info) { - isc_throw(isc::dhcp::DhcpConfigError, "parser logic error: " - << "relay-info storage may not be NULL"); - } - +RelayInfoParser::RelayInfoParser(const Option::Universe& family) + : family_(family) { }; void -RelayInfoParser::build(ConstElementPtr relay_info) { +RelayInfoParser::parse(const isc::dhcp::Subnet::RelayInfoPtr& cfg, + ConstElementPtr relay_info) { + // Let's start with some sanity checks. + if (!relay_info || !cfg) { + isc_throw(DhcpConfigError, "Logic error: RelayInfoParser::parse() called " + "with at least one NULL parameter."); + } + if (relay_info->getType() != Element::map) { + isc_throw(DhcpConfigError, "Configuration error: RelayInfoParser::parse() " + "called with non-map parameter"); + } + + // Now create the default value. + isc::asiolink::IOAddress ip(family_ == Option::V4 ? IOAddress::IPV4_ZERO_ADDRESS() + : IOAddress::IPV6_ZERO_ADDRESS()); + + // Now iterate over all parameters. Currently there's only one supported + // parameter, so it should be an easy thing to check. + bool ip_address_specified = false; BOOST_FOREACH(ConfigPair param, relay_info->mapValue()) { - ParserPtr parser(createConfigParser(param.first)); - parser->build(param.second); - parser->commit(); + if (param.first == "ip-address") { + ip_address_specified = true; + + try { + ip = asiolink::IOAddress(param.second->stringValue()); + } catch (...) { + isc_throw(DhcpConfigError, "Failed to parse ip-address " + "value: " << param.second + << " (" << param.second->getPosition() << ")"); + } + + // Check if the address family matches. + if ( (ip.isV4() && family_ != Option::V4) || + (ip.isV6() && family_ != Option::V6) ) { + isc_throw(DhcpConfigError, "ip-address field " << ip.toText() + << " does not have IP address of expected family type: " + << (family_ == Option::V4 ? "IPv4" : "IPv6") + << " (" << param.second->getPosition() << ")"); + } + } else { + isc_throw(NotImplemented, + "parser error: RelayInfoParser parameter not supported: " + << param.second); + } } - // Get the IP address - boost::scoped_ptr ip; - try { - ip.reset(new asiolink::IOAddress(string_values_->getParam("ip-address"))); - } catch (...) { - isc_throw(DhcpConfigError, "Failed to parse ip-address " - "value: " << string_values_->getParam("ip-address") - << " (" << string_values_->getPosition("ip-address") << ")"); + if (!ip_address_specified) { + isc_throw(DhcpConfigError, "'relay' specified, but mandatory 'ip-address' " + "paramter in it is missing"); } - if ( (ip->isV4() && family_ != Option::V4) || - (ip->isV6() && family_ != Option::V6) ) { - isc_throw(DhcpConfigError, "ip-address field " << ip->toText() - << " does not have IP address of expected family type: " - << (family_ == Option::V4 ? "IPv4" : "IPv6") - << " (" << string_values_->getPosition("ip-address") << ")"); - } - - local_.addr_ = *ip; -} - -isc::dhcp::ParserPtr -RelayInfoParser::createConfigParser(const std::string& parameter) { - DhcpConfigParser* parser = NULL; - if (parameter.compare("ip-address") == 0) { - parser = new StringParser(parameter, string_values_); - } else { - isc_throw(NotImplemented, - "parser error: RelayInfoParser parameter not supported: " - << parameter); - } - - return (isc::dhcp::ParserPtr(parser)); -} - -void -RelayInfoParser::commit() { - *storage_ = local_; + // Ok, we're done with parsing. Let's store the result in the structure + // we were given as configuration storage. + *cfg = isc::dhcp::Subnet::RelayInfo(ip); } //****************************** PoolsListParser ******************************** @@ -1069,6 +1055,12 @@ SubnetConfigParser::build(ConstElementPtr subnet) { continue; } + if (param.first == "relay") { + RelayInfoParser parser(global_context_->universe_); + parser.parse(relay_info_, param.second); + continue; + } + ParserPtr parser; // When unsupported parameter is specified, the function called // below will thrown an exception. We have to catch this exception diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.h b/src/lib/dhcpsrv/parsers/dhcp_parsers.h index a030580c2b..dcd7528acd 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.h +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.h @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2017 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 @@ -16,6 +16,8 @@ #include #include #include +#include +#include #include #include #include @@ -372,56 +374,33 @@ private: /// It contains a list of MAC/hardware acquisition source, i.e. methods how /// MAC address can possibly by obtained in DHCPv6. For a currently supported /// methods, see @ref isc::dhcp::Pkt::getMAC. -class MACSourcesListConfigParser : public DhcpConfigParser { +class MACSourcesListConfigParser : public isc::data::SimpleParser { public: - - /// @brief constructor - /// - /// As this is a dedicated parser, it must be used to parse - /// "mac-sources" parameter only. All other types will throw exception. - /// - /// @param param_name name of the configuration parameter being parsed - /// @param global_context Global parser context. - /// @throw BadValue if supplied parameter name is not "mac-sources" - MACSourcesListConfigParser(const std::string& param_name, - ParserContextPtr global_context); - /// @brief parses parameters value /// /// Parses configuration entry (list of sources) and adds each element /// to the sources list. /// /// @param value pointer to the content of parsed values - virtual void build(isc::data::ConstElementPtr value); - - /// @brief Does nothing. - virtual void commit(); - -private: - - // Parsed parameter name - std::string param_name_; - - /// Global parser context. - ParserContextPtr global_context_; + void parse(CfgMACSource& mac_sources, isc::data::ConstElementPtr value); }; /// @brief Parser for the control-socket structure /// /// It does not parse anything, simply stores the element in /// the staging config. -class ControlSocketParser : public DhcpConfigParser { +class ControlSocketParser : public isc::data::SimpleParser { public: - - ControlSocketParser(const std::string& param_name); - - /// @brief Stores contents of the control-socket map in the staging config. + /// @brief "Parses" control-socket structure /// + /// Since the SrvConfig structure takes the socket definition + /// as ConstElementPtr, there's really nothing to parse here. + /// It only does basic sanity checks and throws DhcpConfigError + /// if the value is null or is not a map. + /// + /// @param srv_cfg parsed values will be stored here /// @param value pointer to the content of parsed values - virtual void build(isc::data::ConstElementPtr value); - - /// @brief Does nothing. - virtual void commit(); + void parse(SrvConfig& srv_cfg, isc::data::ConstElementPtr value); }; /// @brief Parser for hooks library list @@ -849,48 +828,24 @@ protected: /// is expected that the number of parameters will increase over time. /// /// It is useful for parsing Dhcp<4/6>/subnet<4/6>[x]/relay parameters. -class RelayInfoParser : public DhcpConfigParser { +class RelayInfoParser : public isc::data::SimpleParser { public: /// @brief constructor - /// @param unused first argument is ignored, all Parser constructors - /// accept string as first argument. - /// @param relay_info is the storage in which to store the parsed /// @param family specifies protocol family (IPv4 or IPv6) - RelayInfoParser(const std::string& unused, - const isc::dhcp::Subnet::RelayInfoPtr& relay_info, - const isc::dhcp::Option::Universe& family); + RelayInfoParser(const isc::dhcp::Option::Universe& family); /// @brief parses the actual relay parameters - /// @param relay_info JSON structure holding relay parameters to parse - virtual void build(isc::data::ConstElementPtr relay_info); - - /// @brief stores parsed info in relay_info - virtual void commit(); - -protected: - - /// @brief Creates a parser for the given "relay" member element id. /// /// The elements currently supported are: /// -# ip-address /// - /// @param parser is the "item_name" for a specific member element of - /// the "relay" specification. - /// - /// @return returns a pointer to newly created parser. - isc::dhcp::ParserPtr - createConfigParser(const std::string& parser); - - /// Parsed data will be stored there on commit() - isc::dhcp::Subnet::RelayInfoPtr storage_; - - /// Local storage information (for temporary values) - isc::dhcp::Subnet::RelayInfo local_; - - /// Storage for subnet-specific string values. - StringStoragePtr string_values_; + /// @param cfg configuration will be stored here + /// @param relay_info JSON structure holding relay parameters to parse + void parse(const isc::dhcp::Subnet::RelayInfoPtr& cfg, + isc::data::ConstElementPtr relay_info); +protected: /// Protocol family (IPv4 or IPv6) Option::Universe family_; }; diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index 566be20b64..203b496ad1 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012-2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2017 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 @@ -226,48 +226,95 @@ TEST_F(DhcpParserTest, uint32ParserTest) { EXPECT_EQ(test_value, actual_value); } -/// @brief Check MACSourcesListConfigParser basic functionality -/// -/// Verifies that the parser: -/// 1. Does not allow empty for storage. -/// 2. Does not allow name other than "mac-sources" -/// 3. Parses list of mac sources and adds them to CfgMgr -TEST_F(DhcpParserTest, MacSourcesListConfigParserTest) { - - const std::string valid_name = "mac-sources"; - const std::string bogus_name = "bogus-name"; - - ParserContextPtr parser_context(new ParserContext(Option::V6)); - - // Verify that parser constructor fails if parameter name isn't "mac-sources" - EXPECT_THROW(MACSourcesListConfigParser(bogus_name, parser_context), - isc::BadValue); +/// Verifies the code that parses mac sources and adds them to CfgMgr +TEST_F(DhcpParserTest, MacSources) { // That's an equivalent of the following snippet: // "mac-sources: [ \"duid\", \"ipv6\" ]"; - ElementPtr config = Element::createList(); - config->add(Element::create("duid")); - config->add(Element::create("ipv6-link-local")); + ElementPtr values = Element::createList(); + values->add(Element::create("duid")); + values->add(Element::create("ipv6-link-local")); - boost::scoped_ptr - parser(new MACSourcesListConfigParser(valid_name, parser_context)); - - // This should parse the configuration and add eth0 and eth1 to the list - // of interfaces that server should listen on. - EXPECT_NO_THROW(parser->build(config)); - EXPECT_NO_THROW(parser->commit()); - - // Use CfgMgr instance to check if eth0 and eth1 was added, and that - // eth2 was not added. + // Let's grab server configuration from CfgMgr SrvConfigPtr cfg = CfgMgr::instance().getStagingCfg(); ASSERT_TRUE(cfg); - CfgMACSources configured_sources = cfg->getMACSources().get(); + CfgMACSource& sources = cfg->getMACSources(); + // This should parse the configuration and check that it doesn't throw. + MACSourcesListConfigParser parser; + EXPECT_NO_THROW(parser.parse(sources, values)); + + // Finally, check the sources that were configured + CfgMACSources configured_sources = cfg->getMACSources().get(); ASSERT_EQ(2, configured_sources.size()); EXPECT_EQ(HWAddr::HWADDR_SOURCE_DUID, configured_sources[0]); EXPECT_EQ(HWAddr::HWADDR_SOURCE_IPV6_LINK_LOCAL, configured_sources[1]); } +/// @brief Check MACSourcesListConfigParser rejecting empty list +/// +/// Verifies that the code rejects an empty mac-sources list. +TEST_F(DhcpParserTest, MacSourcesEmpty) { + + // That's an equivalent of the following snippet: + // "mac-sources: [ \"duid\", \"ipv6\" ]"; + ElementPtr values = Element::createList(); + + // Let's grab server configuration from CfgMgr + SrvConfigPtr cfg = CfgMgr::instance().getStagingCfg(); + ASSERT_TRUE(cfg); + CfgMACSource& sources = cfg->getMACSources(); + + // This should throw, because if specified, at least one MAC source + // has to be specified. + MACSourcesListConfigParser parser; + EXPECT_THROW(parser.parse(sources, values), DhcpConfigError); +} + +/// @brief Check MACSourcesListConfigParser rejecting empty list +/// +/// Verifies that the code rejects fake mac source. +TEST_F(DhcpParserTest, MacSourcesBogus) { + + // That's an equivalent of the following snippet: + // "mac-sources: [ \"duid\", \"ipv6\" ]"; + ElementPtr values = Element::createList(); + values->add(Element::create("from-ebay")); + values->add(Element::create("just-guess-it")); + + // Let's grab server configuration from CfgMgr + SrvConfigPtr cfg = CfgMgr::instance().getStagingCfg(); + ASSERT_TRUE(cfg); + CfgMACSource& sources = cfg->getMACSources(); + + // This should throw, because these are not valid sources. + MACSourcesListConfigParser parser; + EXPECT_THROW(parser.parse(sources, values), DhcpConfigError); +} + +/// Verifies the code that properly catches duplicate entries +/// in mac-sources definition. +TEST_F(DhcpParserTest, MacSourcesDuplicate) { + + // That's an equivalent of the following snippet: + // "mac-sources: [ \"duid\", \"ipv6\" ]"; + ElementPtr values = Element::createList(); + values->add(Element::create("ipv6-link-local")); + values->add(Element::create("duid")); + values->add(Element::create("duid")); + values->add(Element::create("duid")); + + // Let's grab server configuration from CfgMgr + SrvConfigPtr cfg = CfgMgr::instance().getStagingCfg(); + ASSERT_TRUE(cfg); + CfgMACSource& sources = cfg->getMACSources(); + + // This should parse the configuration and check that it throws. + MACSourcesListConfigParser parser; + EXPECT_THROW(parser.parse(sources, values), DhcpConfigError); +} + + /// @brief Test Fixture class which provides basic structure for testing /// configuration parsing. This is essentially the same structure provided /// by dhcp servers. @@ -2416,6 +2463,19 @@ TEST_F(ParseConfigTest, validRelayInfo4) { " }"; ElementPtr json = Element::fromJSON(config_str); + // We need to set the default ip-address to something. + Subnet::RelayInfoPtr result(new Subnet::RelayInfo(asiolink::IOAddress("0.0.0.0"))); + + RelayInfoParser parser(Option::V4); + + // Subnet4 parser will pass 0.0.0.0 to the RelayInfoParser + EXPECT_NO_THROW(parser.parse(result, json)); + EXPECT_EQ("192.0.2.1", result->addr_.toText()); +} + +/// @brief Checks that a bogus relay info structure for IPv4 is rejected. +TEST_F(ParseConfigTest, bogusRelayInfo4) { + // Invalid config (wrong family type of the ip-address field) std::string config_str_bogus1 = " {" @@ -2430,24 +2490,25 @@ TEST_F(ParseConfigTest, validRelayInfo4) { " }"; ElementPtr json_bogus2 = Element::fromJSON(config_str_bogus2); + // Invalid config (ip-address is mandatory) + std::string config_str_bogus3 = + " {" + " }"; + ElementPtr json_bogus3 = Element::fromJSON(config_str_bogus3); + // We need to set the default ip-address to something. - Subnet::RelayInfoPtr result(new Subnet::RelayInfo(asiolink::IOAddress("0.0.0.0"))); + Subnet::RelayInfoPtr result(new Subnet::RelayInfo(IOAddress::IPV4_ZERO_ADDRESS())); - boost::shared_ptr parser; + RelayInfoParser parser(Option::V4); - // Subnet4 parser will pass 0.0.0.0 to the RelayInfoParser - EXPECT_NO_THROW(parser.reset(new RelayInfoParser("ignored", result, - Option::V4))); - EXPECT_NO_THROW(parser->build(json)); - EXPECT_NO_THROW(parser->commit()); + // wrong family type + EXPECT_THROW(parser.parse(result, json_bogus1), DhcpConfigError); - EXPECT_EQ("192.0.2.1", result->addr_.toText()); + // Too large byte values in pseudo-IPv4 addr + EXPECT_THROW(parser.parse(result, json_bogus2), DhcpConfigError); - // Let's check negative scenario (wrong family type) - EXPECT_THROW(parser->build(json_bogus1), DhcpConfigError); - - // Let's check negative scenario (too large byte values in pseudo-IPv4 addr) - EXPECT_THROW(parser->build(json_bogus2), DhcpConfigError); + // Mandatory ip-address is missing. What a pity. + EXPECT_THROW(parser.parse(result, json_bogus2), DhcpConfigError); } /// @brief Checks that a valid relay info structure for IPv6 can be handled @@ -2460,6 +2521,18 @@ TEST_F(ParseConfigTest, validRelayInfo6) { " }"; ElementPtr json = Element::fromJSON(config_str); + // We need to set the default ip-address to something. + Subnet::RelayInfoPtr result(new Subnet::RelayInfo(asiolink::IOAddress("::"))); + + RelayInfoParser parser(Option::V6); + // Subnet4 parser will pass :: to the RelayInfoParser + EXPECT_NO_THROW(parser.parse(result, json)); + EXPECT_EQ("2001:db8::1", result->addr_.toText()); +} + +/// @brief Checks that a valid relay info structure for IPv6 can be handled +TEST_F(ParseConfigTest, bogusRelayInfo6) { + // Invalid config (wrong family type of the ip-address field std::string config_str_bogus1 = " {" @@ -2474,23 +2547,25 @@ TEST_F(ParseConfigTest, validRelayInfo6) { " }"; ElementPtr json_bogus2 = Element::fromJSON(config_str_bogus2); + // Missing mandatory ip-address field. + std::string config_str_bogus3 = + " {" + " }"; + ElementPtr json_bogus3 = Element::fromJSON(config_str_bogus3); + // We need to set the default ip-address to something. Subnet::RelayInfoPtr result(new Subnet::RelayInfo(asiolink::IOAddress("::"))); - boost::shared_ptr parser; - // Subnet4 parser will pass :: to the RelayInfoParser - EXPECT_NO_THROW(parser.reset(new RelayInfoParser("ignored", result, - Option::V6))); - EXPECT_NO_THROW(parser->build(json)); - EXPECT_NO_THROW(parser->commit()); + RelayInfoParser parser(Option::V6); - EXPECT_EQ("2001:db8::1", result->addr_.toText()); + // Negative scenario (wrong family type) + EXPECT_THROW(parser.parse(result, json_bogus1), DhcpConfigError); - // Let's check negative scenario (wrong family type) - EXPECT_THROW(parser->build(json_bogus1), DhcpConfigError); + // Looks like IPv6 address, but has too many colons + EXPECT_THROW(parser.parse(result, json_bogus2), DhcpConfigError); - // Unparseable text that looks like IPv6 address, but has too many colons - EXPECT_THROW(parser->build(json_bogus2), DhcpConfigError); + // Mandatory ip-address is missing. What a pity. + EXPECT_THROW(parser.parse(result, json_bogus3), DhcpConfigError); } // There's no test for ControlSocketParser, as it is tested in the DHCPv4 code diff --git a/src/lib/testutils/io_utils.cc b/src/lib/testutils/io_utils.cc index 384cb68481..25b1458856 100644 --- a/src/lib/testutils/io_utils.cc +++ b/src/lib/testutils/io_utils.cc @@ -4,6 +4,7 @@ // 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 @@ -36,7 +37,78 @@ std::string readFile(const std::string& file_path) { return (output.str()); } +std::string decommentJSONfile(const std::string& input_file) { + + using namespace std; + + ifstream f(input_file); + if (!f.is_open()) { + isc_throw(isc::BadValue, "can't open input file for reading: " + input_file); + } + + string outfile; + size_t last_slash = input_file.find_last_of("/"); + if (last_slash != string::npos) { + outfile = input_file.substr(last_slash + 1); + } else { + outfile = input_file; + } + outfile += "-decommented"; + + ofstream out(outfile); + if (!out.is_open()) { + isc_throw(isc::BadValue, "can't open output file for writing: " + input_file); + } + + bool in_comment = false; + string line; + while (std::getline(f, line)) { + // First, let's get rid of the # comments + size_t hash_pos = line.find("#"); + if (hash_pos != string::npos) { + line = line.substr(0, hash_pos); + } + + // Second, let's get rid of the // comments + size_t dblslash_pos = line.find("//"); + if (dblslash_pos != string::npos) { + line = line.substr(0, dblslash_pos); + } + + // Now the tricky part: c comments. + size_t begin_pos = line.find("/*"); + size_t end_pos = line.find("*/"); + if (in_comment && end_pos == string::npos) { + // we continue through multiline comment + line = ""; + } else { + + if (begin_pos != string::npos) { + in_comment = true; + if (end_pos != string::npos) { + // sigle line comment. Let's get rid of the content in between + line = line.replace(begin_pos, end_pos + 2, end_pos + 2 - begin_pos, ' '); + in_comment = false; + } else { + line = line.substr(0, begin_pos); + } + } else { + if (in_comment && end_pos != string::npos) { + line = line.replace(0, end_pos +2 , end_pos + 2, ' '); + in_comment = false; + } + } + } + + // Finally, write the line to the output file. + out << line << endl; + } + f.close(); + out.close(); + + return (outfile); +} + }; // end of isc::dhcp::test namespace }; // end of isc::dhcp namespace }; // end of isc namespace - diff --git a/src/lib/testutils/io_utils.h b/src/lib/testutils/io_utils.h index 9155a7412e..1aa15570a4 100644 --- a/src/lib/testutils/io_utils.h +++ b/src/lib/testutils/io_utils.h @@ -26,6 +26,20 @@ bool fileExists(const std::string& file_path); /// @return File contents. std::string readFile(const std::string& file_path); +/// @brief Removes comments from a JSON file +/// +/// Removes //, # and /* */ comments from the input file and writes its content +/// to another file. The comments are replaced with spaces, so the original +/// token locations should remain unaffected. This is rather naive +/// implementation, but it's probably sufficient for testing. It won't be able +/// to pick any trickier cases, like # or // appearing in strings, nested C++ +/// comments etc. +/// +/// @param input_file file to be stripped of comments +/// @return filename of a new file that has comments stripped from it +/// @throw BadValue if the input file cannot be opened +std::string decommentJSONfile(const std::string& input_file); + }; // end of isc::dhcp::test namespace }; // end of isc::dhcp namespace }; // end of isc namespace