From: Tomek Mrugalski Date: Wed, 23 Apr 2014 17:48:06 +0000 (+0200) Subject: [3400] Several improvements in JSON backend: X-Git-Tag: trac3434_base~32^2~25 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=6d9bb68411c8b75721f62f7601c7199d5fe605f3;p=thirdparty%2Fkea.git [3400] Several improvements in JSON backend: - file read can now ignore # comments in file - there are JSON specific unit-tests now in src/bin/dhcp6 - BIND10 backend specific tests moved to separate file --- diff --git a/src/bin/dhcp6/ctrl_json_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_json_dhcp6_srv.cc index e351a13057..6defe99911 100644 --- a/src/bin/dhcp6/ctrl_json_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_json_dhcp6_srv.cc @@ -64,7 +64,7 @@ ControlledDhcpv6Srv::init(const std::string& file_name) { try { // Read contents of the file - string config = readFile(file_name); + string config = readFile(file_name, true); // Try to parse it as JSON json = Element::fromJSON(config); diff --git a/src/bin/dhcp6/main.cc b/src/bin/dhcp6/main.cc index 34fffa4f7a..08740eb732 100644 --- a/src/bin/dhcp6/main.cc +++ b/src/bin/dhcp6/main.cc @@ -39,13 +39,13 @@ using namespace std; namespace { const char* const DHCP6_NAME = "b10-dhcp6"; -const char* const DHCP6_LOGGER_NAME = "bind10"; +const char* const DHCP6_LOGGER_NAME = "kea"; void usage() { cerr << "Usage: " << DHCP6_NAME << " [-v] [-s] [-p port_number] [-c cfgfile]" << endl; cerr << " -v: verbose output" << endl; - cerr << " -s: stand-alone mode (don't connect to BIND10)" << endl; + cerr << " -s: skip configuration (don't connect to BIND10 or don't read config file)" << endl; cerr << " -p number: specify non-standard port number 1-65535 " << "(useful for testing only)" << endl; cerr << " -c file: specify configuration file" << endl; diff --git a/src/bin/dhcp6/tests/Makefile.am b/src/bin/dhcp6/tests/Makefile.am index 1debf568c2..f437bbfb09 100644 --- a/src/bin/dhcp6/tests/Makefile.am +++ b/src/bin/dhcp6/tests/Makefile.am @@ -85,12 +85,13 @@ dhcp6_unittests_SOURCES += rebind_unittest.cc if CONFIG_BACKEND_BIND10 dhcp6_unittests_SOURCES += ../json_config_parser.cc ../json_config_parser.h ../ctrl_bind10_dhcp6_srv.cc -dhcp6_unittests_SOURCES += ctrl_dhcp6_srv_unittest.cc +dhcp6_unittests_SOURCES += ctrl_bind10_dhcp6_srv_unittest.cc dhcp6_unittests_SOURCES += config_parser_unittest.cc endif if CONFIG_BACKEND_JSON dhcp6_unittests_SOURCES += ../json_config_parser.cc ../json_config_parser.h ../ctrl_json_dhcp6_srv.cc +dhcp6_unittests_SOURCES += ctrl_json_dhcp6_srv_unittest.cc dhcp6_unittests_SOURCES += config_parser_unittest.cc endif diff --git a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/ctrl_bind10_dhcp6_srv_unittest.cc similarity index 100% rename from src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc rename to src/bin/dhcp6/tests/ctrl_bind10_dhcp6_srv_unittest.cc diff --git a/src/bin/dhcp6/tests/ctrl_json_dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/ctrl_json_dhcp6_srv_unittest.cc new file mode 100644 index 0000000000..29343ca7b4 --- /dev/null +++ b/src/bin/dhcp6/tests/ctrl_json_dhcp6_srv_unittest.cc @@ -0,0 +1,190 @@ +// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC") +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +#include + +#include +#include +#include +#include + +#include +#include + +#include +#include +#include + +#include +#include + +using namespace std; +using namespace isc; +using namespace isc::asiolink; +using namespace isc::config; +using namespace isc::data; +using namespace isc::dhcp; +using namespace isc::hooks; + +namespace { + +class NakedControlledDhcpv6Srv: public ControlledDhcpv6Srv { + // "Naked" DHCPv6 server, exposes internal fields +public: + NakedControlledDhcpv6Srv():ControlledDhcpv6Srv(0) { } +}; + + +class ControlledJSONDhcpv6SrvTest : public ::testing::Test { +public: + ControlledJSONDhcpv6SrvTest() { + } + + ~ControlledJSONDhcpv6SrvTest() { + }; + + void writeFile(const std::string& file_name, const std::string& content) { + static_cast(unlink(file_name.c_str())); + + ofstream out(file_name.c_str(), ios::trunc); + EXPECT_TRUE(out.is_open()); + out << content; + out.close(); + } + + static const char* TEST_FILE; +}; + +const char* ControlledJSONDhcpv6SrvTest::TEST_FILE = "test-config.json"; + +// This test checks if configuration can be read from a JSON file. +TEST_F(ControlledJSONDhcpv6SrvTest, jsonFile) { + + // Prepare configuration file. + string config = "{ \"interfaces\": [ \"*\" ]," + "\"preferred-lifetime\": 3000," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"subnet6\": [ { " + " \"pool\": [ \"2001:db8:1::/80\" ]," + " \"subnet\": \"2001:db8:1::/64\" " + " }," + " {" + " \"pool\": [ \"2001:db8:2::/80\" ]," + " \"subnet\": \"2001:db8:2::/64\", " + " \"id\": 0" + " }," + " {" + " \"pool\": [ \"2001:db8:3::/80\" ]," + " \"subnet\": \"2001:db8:3::/64\" " + " } ]," + "\"valid-lifetime\": 4000 }"; + writeFile(TEST_FILE, config); + + // Now initialize the server + boost::scoped_ptr srv; + ASSERT_NO_THROW( + srv.reset(new ControlledDhcpv6Srv(0)) + ); + + // And configure it using the config file. + EXPECT_TRUE(srv->init(TEST_FILE)); + + // Now check if the configuration has been applied correctly. + const Subnet6Collection* subnets = CfgMgr::instance().getSubnets6(); + ASSERT_TRUE(subnets); + ASSERT_EQ(3, subnets->size()); // We expect 3 subnets. + + + // Check subnet 1. + EXPECT_EQ("2001:db8:1::", subnets->at(0)->get().first.toText()); + EXPECT_EQ(64, subnets->at(0)->get().second); + + // Check pools in the first subnet. + const PoolCollection& pools1 = subnets->at(0)->getPools(Lease::TYPE_NA); + ASSERT_EQ(1, pools1.size()); + EXPECT_EQ("2001:db8:1::", pools1.at(0)->getFirstAddress().toText()); + EXPECT_EQ("2001:db8:1::ffff:ffff:ffff", pools1.at(0)->getLastAddress().toText()); + EXPECT_EQ(Lease::TYPE_NA, pools1.at(0)->getType()); + + // Check subnet 2. + EXPECT_EQ("2001:db8:2::", subnets->at(1)->get().first.toText()); + EXPECT_EQ(64, subnets->at(1)->get().second); + + // Check pools in the second subnet. + const PoolCollection& pools2 = subnets->at(1)->getPools(Lease::TYPE_NA); + ASSERT_EQ(1, pools2.size()); + EXPECT_EQ("2001:db8:2::", pools2.at(0)->getFirstAddress().toText()); + EXPECT_EQ("2001:db8:2::ffff:ffff:ffff", pools2.at(0)->getLastAddress().toText()); + EXPECT_EQ(Lease::TYPE_NA, pools2.at(0)->getType()); + + // And finally check subnet 3. + EXPECT_EQ("2001:db8:3::", subnets->at(2)->get().first.toText()); + EXPECT_EQ(64, subnets->at(2)->get().second); + + // ... and it's only pool. + const PoolCollection& pools3 = subnets->at(2)->getPools(Lease::TYPE_NA); + EXPECT_EQ("2001:db8:3::", pools3.at(0)->getFirstAddress().toText()); + EXPECT_EQ("2001:db8:3::ffff:ffff:ffff", pools3.at(0)->getLastAddress().toText()); + EXPECT_EQ(Lease::TYPE_NA, pools3.at(0)->getType()); +} + +// This test checks if configuration can be read from a JSON file. +TEST_F(ControlledJSONDhcpv6SrvTest, comments) { + + string config_hash_comments = "# This is a comment. It should be \n" + "#ignored. Real config starts in line below\n" + "{ \"interfaces\": [ \"*\" ]," + "\"preferred-lifetime\": 3000," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, \n" + "# comments in the middle should be ignored, too\n" + "\"subnet6\": [ { " + " \"pool\": [ \"2001:db8:1::/80\" ]," + " \"subnet\": \"2001:db8:1::/64\" " + " } ]," + "\"valid-lifetime\": 4000 }"; + + /// @todo: Implement C++-style (// ...) comments + /// @todo: Implement C-style (/* ... */) comments + + writeFile(TEST_FILE, config_hash_comments); + + // Now initialize the server + boost::scoped_ptr srv; + ASSERT_NO_THROW( + srv.reset(new ControlledDhcpv6Srv(0)) + ); + + // And configure it using config without + EXPECT_TRUE(srv->init(TEST_FILE)); + + // Now check if the configuration has been applied correctly. + const Subnet6Collection* subnets = CfgMgr::instance().getSubnets6(); + ASSERT_TRUE(subnets); + ASSERT_EQ(1, subnets->size()); + + // Check subnet 1. + EXPECT_EQ("2001:db8:1::", subnets->at(0)->get().first.toText()); + EXPECT_EQ(64, subnets->at(0)->get().second); + + // Check pools in the first subnet. + const PoolCollection& pools1 = subnets->at(0)->getPools(Lease::TYPE_NA); + ASSERT_EQ(1, pools1.size()); + EXPECT_EQ("2001:db8:1::", pools1.at(0)->getFirstAddress().toText()); + EXPECT_EQ("2001:db8:1::ffff:ffff:ffff", pools1.at(0)->getLastAddress().toText()); + EXPECT_EQ(Lease::TYPE_NA, pools1.at(0)->getType()); +} + +} // End of anonymous namespace diff --git a/src/lib/dhcpsrv/daemon.cc b/src/lib/dhcpsrv/daemon.cc index 360f414324..2c7d649460 100644 --- a/src/lib/dhcpsrv/daemon.cc +++ b/src/lib/dhcpsrv/daemon.cc @@ -49,21 +49,42 @@ void Daemon::shutdown() { Daemon::~Daemon() { } -std::string Daemon::readFile(const std::string& file_name) { +std::string Daemon::readFile(const std::string& file_name, bool ignore_comments) { - // This is the fastest method to read a file according to: - // http://insanecoding.blogspot.com/2011/11/how-to-read-in-file-in-c.html std::ifstream infile(file_name.c_str(), std::ios::in | std::ios::binary); if (infile.is_open()) { std::string contents; - infile.seekg(0, std::ios::end); - contents.resize(infile.tellg()); - infile.seekg(0, std::ios::beg); - infile.read(&contents[0], contents.size()); - infile.close(); - return(contents); + std::string line; + + if (ignore_comments) { + while (std::getline(infile, line)) { + + // If this is a comments line, replace it with empty line + // (so the line numbers will still match + if (!line.empty() && line[0] == '#') { + line = ""; + } + + // getline() removes end line charaters. Unfortunately, we need + // it for getting the line numbers right (in case we report an + // error. + contents.append(line); + contents.append("\n"); + } + return (contents); + } else { + + // This is the fastest method to read a file according to: + // http://insanecoding.blogspot.com/2011/11/how-to-read-in-file-in-c.html + infile.seekg(0, std::ios::end); + contents.resize(infile.tellg()); + infile.seekg(0, std::ios::beg); + infile.read(&contents[0], contents.size()); + infile.close(); + return(contents); + } } isc_throw(InvalidOperation, "Failed to read file " << file_name << ",error:" << errno); diff --git a/src/lib/dhcpsrv/daemon.h b/src/lib/dhcpsrv/daemon.h index 18ea678de9..dc00fb159a 100644 --- a/src/lib/dhcpsrv/daemon.h +++ b/src/lib/dhcpsrv/daemon.h @@ -51,7 +51,17 @@ public: /// or reads the /// Creates session that will be used to receive commands and updated /// configuration from cfgmgr (or indirectly from user via bindctl). - /// @return true if initialization was successful, false if it failed + /// + /// Note: This function may throw to report enountered problems. It may + /// also return false if the initialization was skipped. That may seem + /// redundant, but the idea here is that in some cases the configuration + /// was read, understood and the decision was made to not start. One + /// case where such capability could be needed is when we have a single + /// config file for Kea4 and D2, but the DNS Update is disabled. It is + /// likely that the D2 will be started, it will analyze its config file, + /// decide that it is not needed and will shut down. + /// + /// @return true if initialization was successful, false if it was not virtual bool init(const std::string& config_file); /// @brief Performs final deconfiguration. @@ -85,9 +95,13 @@ public: /// This is an utility method that is expected to be used by several daemons. /// It reads contents of a text file and return it as a string. /// + /// For now, comments are defined as lines starting with a hash. + /// /// @param file_name name of the file to read + /// @param ignore_comments whether ignore comment lines /// @return content of the file - std::string readFile(const std::string& file_name); + std::string readFile(const std::string& file_name, + bool ingore_comments = false); }; }; // end of isc::dhcp namespace diff --git a/src/lib/dhcpsrv/tests/daemon_unittest.cc b/src/lib/dhcpsrv/tests/daemon_unittest.cc index 3c5b482c77..d106f217d7 100644 --- a/src/lib/dhcpsrv/tests/daemon_unittest.cc +++ b/src/lib/dhcpsrv/tests/daemon_unittest.cc @@ -21,9 +21,6 @@ using namespace isc; using namespace isc::dhcp; -/// Temporary file name used in some tests -const char* TEMP_FILE="temp-file.json"; - namespace { /// @brief Test-friendly version of the daemon @@ -31,35 +28,106 @@ namespace { /// This class exposes internal Daemon class methods. class NakedDaemon: public Daemon { public: - using Daemon::readFile; }; -TEST(Daemon, readFile) { - NakedDaemon x; +/// @brief Test class for testing Deamon class +class DaemonTest : public ::testing::Test { +public: + + /// @brief writes specified text to a file + /// + /// That is an auxilliary funtion used in fileRead() tests. + /// + /// @param content text to be written to disk + void writeFile(const std::string& content) { + // Write sample content to disk + unlink(TEMP_FILE); + std::ofstream write_me(TEMP_FILE); + EXPECT_TRUE(write_me.is_open()); + write_me << content; + write_me.close(); + } + + /// destructor + ~DaemonTest() { + static_cast(unlink(TEMP_FILE)); + } + + /// Name of the temporary file + static const char* TEMP_FILE; + + /// The daemon implementation being tested + NakedDaemon daemon_; +}; + +/// Temporary file name used in some tests +const char* DaemonTest::TEMP_FILE="temp-file.json"; + +// Test checks whether a text file can be read from disk. +TEST_F(DaemonTest, readFile) { const char* content = "Horse doesn't eat cucumber salad"; - // Write sample content to disk - unlink(TEMP_FILE); - std::ofstream write_me(TEMP_FILE); - EXPECT_TRUE(write_me.is_open()); - write_me << content; - write_me.close(); + writeFile(content); // Check that the read content is correct - EXPECT_EQ(std::string(content), x.readFile(TEMP_FILE)); + EXPECT_EQ(std::string(content), daemon_.readFile(TEMP_FILE)); unlink(TEMP_FILE); } -TEST(Daemon, readFileError) { - NakedDaemon x; +// Test checks whether a text file can be read from disk. +TEST_F(DaemonTest, readFileMultiline) { + + const char* no_endline = "A text\nwithout\nendline in the last line"; + const char* with_endline = "A text\nwith\endline in the last line\n"; + + // Write sample content to disk + writeFile(no_endline); // Check that the read content is correct - EXPECT_THROW(x.readFile("no-such-file.txt"), isc::InvalidOperation); + EXPECT_EQ(std::string(no_endline), daemon_.readFile(TEMP_FILE)); - unlink(TEMP_FILE); + // Write sample content to disk + writeFile(with_endline); + + // Check that the read content is correct + EXPECT_EQ(std::string(with_endline), daemon_.readFile(TEMP_FILE)); +} + + +// Test checks whether comments in file are ignored as expected. +TEST_F(DaemonTest, readFileComments) { + NakedDaemon x; + + const char* commented_content = "# This is a comment\n" + "this is a normal line\n" + "Second line\n" + "# another comment"; + + // The same as above with comments removed and comment lines + // substituted with empty lines. + const char* filtered_content ="\nthis is a normal line\nSecond line\n\n"; + + // Write sample content to disk + writeFile(commented_content); + + // Check that the read content is correct (without comment elimination) + EXPECT_EQ(std::string(commented_content), x.readFile(TEMP_FILE, false)); + + // Check that the read content is correct (with comment elimination) + EXPECT_EQ(std::string(filtered_content), x.readFile(TEMP_FILE, true)); + + + +} + +// This test checks that missing file will generate an exception. +TEST_F(DaemonTest, readFileError) { + + // Check that the read content is correct + EXPECT_THROW(daemon_.readFile("no-such-file.txt"), isc::InvalidOperation); } };