From 4373258d9db0d67afe8ba239a7f706b46468a219 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Mon, 30 Nov 2015 15:17:23 +0100 Subject: [PATCH 1/2] [4211] Added parameter which enables/disables write of a DUID in file. --- doc/guide/dhcp6-srv.xml | 30 ++++++++++- src/bin/dhcp6/dhcp6.spec | 6 +++ src/bin/dhcp6/dhcp6_srv.cc | 14 +---- .../dhcp6/tests/ctrl_dhcp6_srv_unittest.cc | 6 ++- src/bin/dhcp6/tests/dhcp6_message_test.h | 2 +- src/bin/dhcp6/tests/dhcp6_process_tests.sh.in | 6 ++- src/bin/dhcp6/tests/dhcp6_test_utils.cc | 20 ++++++- src/bin/dhcp6/tests/dhcp6_test_utils.h | 29 +++++++++-- .../dhcp6/tests/kea_controller_unittest.cc | 6 ++- src/bin/keactrl/tests/keactrl_tests.sh.in | 4 ++ src/lib/Makefile.am | 4 +- src/lib/dhcp/tests/Makefile.am | 1 + src/lib/dhcp/tests/duid_factory_unittest.cc | 16 +----- src/lib/dhcpsrv/cfg_duid.cc | 4 +- src/lib/dhcpsrv/cfg_duid.h | 19 +++++++ src/lib/dhcpsrv/cfgmgr.cc | 7 ++- src/lib/dhcpsrv/cfgmgr.h | 7 ++- src/lib/dhcpsrv/parsers/duid_config_parser.cc | 8 +++ src/lib/dhcpsrv/parsers/duid_config_parser.h | 7 +++ src/lib/dhcpsrv/tests/Makefile.am | 1 + src/lib/dhcpsrv/tests/cfg_duid_unittest.cc | 34 ++++++++++++ .../tests/duid_config_parser_unittest.cc | 4 +- src/lib/testutils/Makefile.am | 3 +- src/lib/testutils/io_utils.cc | 52 +++++++++++++++++++ src/lib/testutils/io_utils.h | 40 ++++++++++++++ 25 files changed, 282 insertions(+), 48 deletions(-) create mode 100644 src/lib/testutils/io_utils.cc create mode 100644 src/lib/testutils/io_utils.h diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml index 56063682a2..a1f6624026 100644 --- a/doc/guide/dhcp6-srv.xml +++ b/doc/guide/dhcp6-srv.xml @@ -2533,7 +2533,7 @@ should include options from the isc option space: "server-id": { "type": "EN", "enterprise-id": 2495, - "identifier": "87ABEF7A5BB545", + "identifier": "87ABEF7A5BB545" }, ... } @@ -2577,7 +2577,7 @@ should include options from the isc option space: "server-id": { "type": "LL", "htype": 8, - "identifier": "A65DC7410F05", + "identifier": "A65DC7410F05" }, ... } @@ -2599,6 +2599,32 @@ should include options from the isc option space: location: [kea-install-dir]/var/kea/kea-dhcp6-serverid . + + In some uncommon deployments where no stable storage is + available, it is desired to configure the server to not try to + store the server identifier on the hard drive. It is controlled + by the value of persist boolean parameter: + +"Dhcp6": { + "server-id": { + "type": "EN", + "enterprise-id": 2495, + "identifier": "87ABEF7A5BB545", + "persist": false + }, + ... +} + + + The default value of the "persist" parameter is + true which configures the server to store the + server identifier on a disk. + + In the example above, the server is configured to not store + the generated server identifier on a disk. But, if the server + identifier is not modified in the configuration the same value + will be used after server restart, because entire server + identifier is explicitly specified in a configuration.
diff --git a/src/bin/dhcp6/dhcp6.spec b/src/bin/dhcp6/dhcp6.spec index 7cf097e6f6..ffe3779f73 100644 --- a/src/bin/dhcp6/dhcp6.spec +++ b/src/bin/dhcp6/dhcp6.spec @@ -38,6 +38,12 @@ "item_type": "integer", "item_optional": true, "item_default": 0 + }, + { + "item_name": "persist", + "item_type": "boolean", + "item_optional": true, + "item_default": true } ] }, diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 797a2ea422..0aaf4fe6e1 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -175,16 +175,6 @@ namespace dhcp { const std::string Dhcpv6Srv::VENDOR_CLASS_PREFIX("VENDOR_CLASS_"); -/// @brief file name of a server-id file -/// -/// Server must store its duid in persistent storage that must not change -/// between restarts. This is name of the file that is created in dataDir -/// (see isc::dhcp::CfgMgr::getDataDir()). It is a text file that uses -/// double digit hex values separated by colons format, e.g. -/// 01:ff:02:03:06:80:90:ab:cd:ef. Server will create it during first -/// run and then use it afterwards. -static const char* SERVER_DUID_FILE = "kea-dhcp6-serverid"; - Dhcpv6Srv::Dhcpv6Srv(uint16_t port) : port_(port), serverid_(), shutdown_(true), alloc_engine_() { @@ -201,8 +191,8 @@ Dhcpv6Srv::Dhcpv6Srv(uint16_t port) return; } - string duid_file = CfgMgr::instance().getDataDir() + "/" + string(SERVER_DUID_FILE); - DUIDFactory duid_factory(duid_file); + // Create a DUID instance but do not store it into a file. + DUIDFactory duid_factory; DuidPtr duid = duid_factory.get(); serverid_.reset(new Option(Option::V6, D6O_SERVERID, duid->getDuid())); diff --git a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc index 7428796271..b62f9a4257 100644 --- a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -58,9 +59,10 @@ public: using Dhcpv6Srv::receivePacket; }; -class CtrlDhcpv6SrvTest : public ::testing::Test { +class CtrlDhcpv6SrvTest : public BaseServerTest { public: - CtrlDhcpv6SrvTest() { + CtrlDhcpv6SrvTest() + : BaseServerTest() { reset(); } diff --git a/src/bin/dhcp6/tests/dhcp6_message_test.h b/src/bin/dhcp6/tests/dhcp6_message_test.h index 832fc79f90..e31c63216b 100644 --- a/src/bin/dhcp6/tests/dhcp6_message_test.h +++ b/src/bin/dhcp6/tests/dhcp6_message_test.h @@ -26,7 +26,7 @@ namespace test { /// @brief Base class for test fixure classes used to validate the DHCPv6 /// message processing by the server. -class Dhcpv6MessageTest : public isc::dhcp::test::Dhcpv6SrvTest { +class Dhcpv6MessageTest : public Dhcpv6SrvTest { public: /// @brief Constructor. /// diff --git a/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in b/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in index 22ae78afe3..834d74e4c6 100755 --- a/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in +++ b/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in @@ -26,6 +26,10 @@ CONFIG="{ { \"interfaces-config\": { \"interfaces\": [ ] }, + \"server-id\": { + \"type\": \"LLT\", + \"persist\": false + }, \"preferred-lifetime\": 3000, \"valid-lifetime\": 4000, \"renew-timer\": 1000, @@ -291,7 +295,7 @@ lfc_timer_test() { # Create a configuration with the LFC enabled, by replacing the section # with the lfc-interval and persist parameters. LFC_CONFIG=$(printf "${CONFIG}" | sed -e 's/\"lfc-interval\": 0/\"lfc-interval\": 1/g' \ - | sed -e 's/\"persist\": false/\"persist\": true/g') + | sed -e 's/\"persist\": false,/\"persist\": true,/g') # Create new configuration file. create_config "${LFC_CONFIG}" # Instruct Kea to log to the specific file. diff --git a/src/bin/dhcp6/tests/dhcp6_test_utils.cc b/src/bin/dhcp6/tests/dhcp6_test_utils.cc index 2f9f2c128d..0b9ea7b9b6 100644 --- a/src/bin/dhcp6/tests/dhcp6_test_utils.cc +++ b/src/bin/dhcp6/tests/dhcp6_test_utils.cc @@ -21,6 +21,8 @@ #include #include #include +#include +#include #include using namespace isc::data; @@ -32,10 +34,24 @@ namespace isc { namespace dhcp { namespace test { -const char* NakedDhcpv6SrvTest::DUID_FILE = "server-id-test.txt"; +const char* BaseServerTest::DUID_FILE = "kea-dhcp6-serverid"; + +BaseServerTest::BaseServerTest() + : original_datadir_(CfgMgr::instance().getDataDir()) { + CfgMgr::instance().setDataDir(TEST_DATA_BUILDDIR); +} + +BaseServerTest::~BaseServerTest() { + // Remove test DUID file. + std::ostringstream s; + s << CfgMgr::instance().getDataDir() << "/" << DUID_FILE; + static_cast(::remove(s.str().c_str())); + // Revert to original data directory. + CfgMgr::instance().setDataDir(original_datadir_); +} Dhcpv6SrvTest::Dhcpv6SrvTest() -:srv_(0) { + : NakedDhcpv6SrvTest(), srv_(0) { subnet_ = isc::dhcp::Subnet6Ptr (new isc::dhcp::Subnet6(isc::asiolink::IOAddress("2001:db8:1::"), 48, 1000, 2000, 3000, 4000)); diff --git a/src/bin/dhcp6/tests/dhcp6_test_utils.h b/src/bin/dhcp6/tests/dhcp6_test_utils.h index 454a309245..8896c5d90e 100644 --- a/src/bin/dhcp6/tests/dhcp6_test_utils.h +++ b/src/bin/dhcp6/tests/dhcp6_test_utils.h @@ -42,6 +42,30 @@ namespace isc { namespace dhcp { namespace test { +/// @brief Base class for DHCPv6 server testing. +/// +/// Currently it configures the test data path directory in +/// the @c CfgMgr. When the object is destroyed, the original +/// path is reverted. +class BaseServerTest : public ::testing::Test { +public: + + /// @brief Location of a test DUID file + static const char* DUID_FILE; + + /// @brief Constructor. + BaseServerTest(); + + /// @brief Destructor. + virtual ~BaseServerTest(); + +private: + + /// @brief Holds the original data directory. + std::string original_datadir_; + +}; + /// @brief "naked" Dhcpv6Srv class that exposes internal members class NakedDhcpv6Srv: public isc::dhcp::Dhcpv6Srv { public: @@ -131,15 +155,12 @@ public: /// @brief Test fixture for any tests requiring blank/empty configuration /// serves as base class for additional tests -class NakedDhcpv6SrvTest : public ::testing::Test { +class NakedDhcpv6SrvTest : public BaseServerTest { public: /// @brief Constructor NakedDhcpv6SrvTest(); - /// @brief Location of a test DUID file - static const char* DUID_FILE; - // Generate IA_NA or IA_PD option with specified parameters boost::shared_ptr generateIA (uint16_t type, uint32_t iaid, uint32_t t1, uint32_t t2); diff --git a/src/bin/dhcp6/tests/kea_controller_unittest.cc b/src/bin/dhcp6/tests/kea_controller_unittest.cc index fb3a8a6f1b..c0cec26720 100644 --- a/src/bin/dhcp6/tests/kea_controller_unittest.cc +++ b/src/bin/dhcp6/tests/kea_controller_unittest.cc @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -53,9 +54,10 @@ public: }; -class JSONFileBackendTest : public ::testing::Test { +class JSONFileBackendTest : public dhcp::test::BaseServerTest { public: - JSONFileBackendTest() { + JSONFileBackendTest() + : BaseServerTest() { } ~JSONFileBackendTest() { diff --git a/src/bin/keactrl/tests/keactrl_tests.sh.in b/src/bin/keactrl/tests/keactrl_tests.sh.in index c0f273ab8b..ff1623738c 100644 --- a/src/bin/keactrl/tests/keactrl_tests.sh.in +++ b/src/bin/keactrl/tests/keactrl_tests.sh.in @@ -57,6 +57,10 @@ config="{ \"interfaces-config\": { \"interfaces\": [ ] }, + \"server-id\": { + \"type\": \"LLT\", + \"persist\": false + }, \"preferred-lifetime\": 3000, \"valid-lifetime\": 4000, \"renew-timer\": 1000, diff --git a/src/lib/Makefile.am b/src/lib/Makefile.am index f4870edf18..6fd271a246 100644 --- a/src/lib/Makefile.am +++ b/src/lib/Makefile.am @@ -1,3 +1,3 @@ # The following build order must be maintained. -SUBDIRS = exceptions util log hooks cryptolink dns cc asiolink dhcp config stats \ - asiodns testutils dhcp_ddns eval dhcpsrv cfgrpt +SUBDIRS = exceptions util log hooks cryptolink dns cc asiolink testutils dhcp config \ + stats asiodns dhcp_ddns eval dhcpsrv cfgrpt diff --git a/src/lib/dhcp/tests/Makefile.am b/src/lib/dhcp/tests/Makefile.am index 9a42095c5b..05928bd7e6 100644 --- a/src/lib/dhcp/tests/Makefile.am +++ b/src/lib/dhcp/tests/Makefile.am @@ -117,6 +117,7 @@ libdhcp___unittests_LDADD += $(top_builddir)/src/lib/log/libkea-log.la libdhcp___unittests_LDADD += $(top_builddir)/src/lib/util/threads/libkea-threads.la libdhcp___unittests_LDADD += $(top_builddir)/src/lib/util/libkea-util.la libdhcp___unittests_LDADD += $(top_builddir)/src/lib/exceptions/libkea-exceptions.la +libdhcp___unittests_LDADD += $(top_builddir)/src/lib/testutils/libkea-testutils.la libdhcp___unittests_LDADD += $(LOG4CPLUS_LIBS) $(CRYPTO_LIBS) libdhcp___unittests_LDADD += $(BOOST_LIBS) $(GTEST_LDADD) endif diff --git a/src/lib/dhcp/tests/duid_factory_unittest.cc b/src/lib/dhcp/tests/duid_factory_unittest.cc index c7d5643ca9..be5b808a5f 100644 --- a/src/lib/dhcp/tests/duid_factory_unittest.cc +++ b/src/lib/dhcp/tests/duid_factory_unittest.cc @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -187,20 +188,7 @@ DUIDFactoryTest::removeDefaultFile() const { std::string DUIDFactoryTest::readDefaultFile() const { - std::ifstream ifs; - ifs.open(absolutePath(DEFAULT_DUID_FILE).c_str(), std::ifstream::in); - if (!ifs.good()) { - return (std::string()); - } - std::string buf; - std::ostringstream output; - while (!ifs.eof() && ifs.good()) { - ifs >> buf; - output << buf; - } - ifs.close(); - - return (output.str()); + return (dhcp::test::readFile(absolutePath(DEFAULT_DUID_FILE))); } std::vector diff --git a/src/lib/dhcpsrv/cfg_duid.cc b/src/lib/dhcpsrv/cfg_duid.cc index bb47867178..f9862f27e2 100644 --- a/src/lib/dhcpsrv/cfg_duid.cc +++ b/src/lib/dhcpsrv/cfg_duid.cc @@ -27,7 +27,7 @@ namespace dhcp { CfgDUID::CfgDUID() : type_(DUID::DUID_LLT), identifier_(), htype_(0), time_(0), - enterprise_id_(0) { + enterprise_id_(0), persist_(true) { } void @@ -57,7 +57,7 @@ CfgDUID::setIdentifier(const std::string& identifier_as_hex) { DuidPtr CfgDUID::create(const std::string& duid_file_path) const { // Use DUID factory to create a DUID instance. - DUIDFactory factory(duid_file_path); + DUIDFactory factory(persist() ? duid_file_path : ""); switch (getType()) { case DUID::DUID_LLT: diff --git a/src/lib/dhcpsrv/cfg_duid.h b/src/lib/dhcpsrv/cfg_duid.h index 223227dca4..0a254f06bc 100644 --- a/src/lib/dhcpsrv/cfg_duid.h +++ b/src/lib/dhcpsrv/cfg_duid.h @@ -100,6 +100,22 @@ public: enterprise_id_ = enterprise_id; } + /// @brief Checks if server identifier should be stored on disk. + /// + /// @return true if the server identifier should be stored on + /// the disk, false otherwise. + bool persist() const { + return (persist_); + } + + /// @brief Sets a boolean flag indicating if the server identifier + /// should be stored on the disk (if true) or not (if false). + /// + /// @param persist New value of the flag. + void setPersist(const bool persist) { + persist_ = persist; + } + /// @brief Creates instance of a DUID from the current configuration. /// /// @param duid_file_path Absolute path to a DUID file. @@ -123,6 +139,9 @@ private: /// @brief Enterprise id used for DUID-EN. uint32_t enterprise_id_; + /// @brief Boolean flag which indicates if server identifier should + /// be stored on the disk. + bool persist_; }; /// @name Pointers to the @c CfgDUID objects. diff --git a/src/lib/dhcpsrv/cfgmgr.cc b/src/lib/dhcpsrv/cfgmgr.cc index 77a0bd557c..f5d6994abb 100644 --- a/src/lib/dhcpsrv/cfgmgr.cc +++ b/src/lib/dhcpsrv/cfgmgr.cc @@ -67,10 +67,15 @@ CfgMgr::addOptionSpace6(const OptionSpacePtr& space) { } -std::string CfgMgr::getDataDir() { +std::string CfgMgr::getDataDir() const { return (datadir_); } +void +CfgMgr::setDataDir(const std::string& datadir) { + datadir_ = datadir; +} + bool CfgMgr::isDuplicate(const Subnet6& subnet) const { for (Subnet6Collection::const_iterator subnet_it = subnets6_.begin(); diff --git a/src/lib/dhcpsrv/cfgmgr.h b/src/lib/dhcpsrv/cfgmgr.h index cb124de95b..17c4c65b77 100644 --- a/src/lib/dhcpsrv/cfgmgr.h +++ b/src/lib/dhcpsrv/cfgmgr.h @@ -131,7 +131,12 @@ public: /// This method returns a path to writeable directory that DHCP servers /// can store data in. /// @return data directory - std::string getDataDir(); + std::string getDataDir() const; + + /// @brief Sets new data directory. + /// + /// @param datadir New data directory. + void setDataDir(const std::string& datadir); /// @brief Sets whether server should send back client-id in DHCPv4 /// diff --git a/src/lib/dhcpsrv/parsers/duid_config_parser.cc b/src/lib/dhcpsrv/parsers/duid_config_parser.cc index ed7c5054f8..9a63fbedfe 100644 --- a/src/lib/dhcpsrv/parsers/duid_config_parser.cc +++ b/src/lib/dhcpsrv/parsers/duid_config_parser.cc @@ -50,6 +50,8 @@ DUIDConfigParser::build(isc::data::ConstElementPtr duid_configuration) { setTime(element.second->intValue()); } else if (element.first == "enterprise-id") { setEnterpriseId(element.second->intValue()); + } else if (element.first == "persist") { + setPersist(element.second->boolValue()); } else { isc_throw(DhcpConfigError, "unsupported configuration " "parameter '" << element.first << "'"); @@ -118,6 +120,12 @@ DUIDConfigParser::setEnterpriseId(const int64_t enterprise_id) const { cfg->setEnterpriseId(static_cast(enterprise_id)); } +void +DUIDConfigParser::setPersist(const bool persist) { + const CfgDUIDPtr& cfg = CfgMgr::instance().getStagingCfg()->getCfgDUID(); + cfg->setPersist(persist); +} + template void DUIDConfigParser::checkRange(const std::string& parameter_name, diff --git a/src/lib/dhcpsrv/parsers/duid_config_parser.h b/src/lib/dhcpsrv/parsers/duid_config_parser.h index 99c8c8fc31..9e3f6c16f7 100644 --- a/src/lib/dhcpsrv/parsers/duid_config_parser.h +++ b/src/lib/dhcpsrv/parsers/duid_config_parser.h @@ -75,6 +75,13 @@ private: /// @param enterprise_id Enterprise id. void setEnterpriseId(const int64_t enterprise_id) const; + /// @brief Set persistence flag. + /// + /// @param persist A boolean value indicating if the server + /// identifier should be stored on the disk (if true) or + /// not (if false). + void setPersist(const bool persist); + /// @brief Verifies if the specified parameter is in range. /// /// Each numeric value must be in range of [0 .. max_value], where diff --git a/src/lib/dhcpsrv/tests/Makefile.am b/src/lib/dhcpsrv/tests/Makefile.am index a591e5466e..e57a65ecc9 100644 --- a/src/lib/dhcpsrv/tests/Makefile.am +++ b/src/lib/dhcpsrv/tests/Makefile.am @@ -162,6 +162,7 @@ libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/log/libkea-log.la libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/util/threads/libkea-threads.la libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/util/libkea-util.la libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libkea-exceptions.la +libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/testutils/libkea-testutils.la libdhcpsrv_unittests_LDADD += $(LOG4CPLUS_LIBS) $(CRYPTO_LIBS) libdhcpsrv_unittests_LDADD += $(BOOST_LIBS) $(GTEST_LDADD) endif diff --git a/src/lib/dhcpsrv/tests/cfg_duid_unittest.cc b/src/lib/dhcpsrv/tests/cfg_duid_unittest.cc index c5467123be..ed95b8f158 100644 --- a/src/lib/dhcpsrv/tests/cfg_duid_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_duid_unittest.cc @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -98,6 +99,7 @@ TEST_F(CfgDUIDTest, defaults) { EXPECT_EQ(0, cfg_duid.getHType()); EXPECT_EQ(0, cfg_duid.getTime()); EXPECT_EQ(0, cfg_duid.getEnterpriseId()); + EXPECT_TRUE(cfg_duid.persist()); } // This test verifies that it is possible to set values for the CfgDUID. @@ -109,6 +111,7 @@ TEST_F(CfgDUIDTest, setValues) { ASSERT_NO_THROW(cfg_duid.setHType(100)); ASSERT_NO_THROW(cfg_duid.setTime(32100)); ASSERT_NO_THROW(cfg_duid.setEnterpriseId(10)); + ASSERT_NO_THROW(cfg_duid.setPersist(false)); // Check that values have been set correctly. EXPECT_EQ(DUID::DUID_EN, cfg_duid.getType()); @@ -116,6 +119,7 @@ TEST_F(CfgDUIDTest, setValues) { EXPECT_EQ(100, cfg_duid.getHType()); EXPECT_EQ(32100, cfg_duid.getTime()); EXPECT_EQ(10, cfg_duid.getEnterpriseId()); + EXPECT_FALSE(cfg_duid.persist()); } // This test checks positive scenarios for setIdentifier. @@ -167,6 +171,9 @@ TEST_F(CfgDUIDTest, createLLT) { // Verify if the DUID is correct. EXPECT_EQ("00:01:00:08:00:00:11:23:12:56:43:25:a6:3f", duid->toText()); + + // Verify that the DUID file has been created. + EXPECT_TRUE(dhcp::test::fileExists(absolutePath(DUID_FILE_NAME))); } // This method checks that the DUID-EN can be created from the @@ -184,6 +191,9 @@ TEST_F(CfgDUIDTest, createEN) { // Verify if the DUID is correct. EXPECT_EQ("00:02:00:00:10:10:25:0f:3e:26:a7:62", duid->toText()); + + // Verify that the DUID file has been created. + EXPECT_TRUE(dhcp::test::fileExists(absolutePath(DUID_FILE_NAME))); } // This method checks that the DUID-LL can be created from the @@ -201,6 +211,30 @@ TEST_F(CfgDUIDTest, createLL) { // Verify if the DUID is correct. EXPECT_EQ("00:03:00:02:12:41:34:a4:b3:67", duid->toText()); + + // Verify that the DUID file has been created. + EXPECT_TRUE(dhcp::test::fileExists(absolutePath(DUID_FILE_NAME))); +} + +// This test verifies that it is possible to disable storing +// generated DUID on a hard drive. +TEST_F(CfgDUIDTest, createDisableWrite) { + CfgDUID cfg; + ASSERT_NO_THROW(cfg.setType(DUID::DUID_EN)); + ASSERT_NO_THROW(cfg.setIdentifier("250F3E26A762")); + ASSERT_NO_THROW(cfg.setEnterpriseId(0x1010)); + ASSERT_NO_THROW(cfg.setPersist(false)); + + // Generate DUID from this configuration. + DuidPtr duid; + ASSERT_NO_THROW(duid = cfg.create(absolutePath(DUID_FILE_NAME))); + ASSERT_TRUE(duid); + + // Verify if the DUID is correct. + EXPECT_EQ("00:02:00:00:10:10:25:0f:3e:26:a7:62", duid->toText()); + + // DUID persistence is disabled so there should be no DUID file. + EXPECT_FALSE(dhcp::test::fileExists(absolutePath(DUID_FILE_NAME))); } } // end of anonymous namespace diff --git a/src/lib/dhcpsrv/tests/duid_config_parser_unittest.cc b/src/lib/dhcpsrv/tests/duid_config_parser_unittest.cc index 8c5c641567..eaa90f7c52 100644 --- a/src/lib/dhcpsrv/tests/duid_config_parser_unittest.cc +++ b/src/lib/dhcpsrv/tests/duid_config_parser_unittest.cc @@ -184,7 +184,8 @@ TEST_F(DUIDConfigParserTest, allParameters) { " \"identifier\": \"ABCDEF\"," " \"time\": 100," " \"htype\": 8," - " \"enterprise-id\": 2024" + " \"enterprise-id\": 2024," + " \"persist\": false" "}")); // Verify that parameters have been set correctly. @@ -194,6 +195,7 @@ TEST_F(DUIDConfigParserTest, allParameters) { EXPECT_EQ(8, cfg_duid->getHType()); EXPECT_EQ(100, cfg_duid->getTime()); EXPECT_EQ(2024, cfg_duid->getEnterpriseId()); + EXPECT_FALSE(cfg_duid->persist()); } // Test out of range values for time. diff --git a/src/lib/testutils/Makefile.am b/src/lib/testutils/Makefile.am index e9ce78034b..3d4d8d56ad 100644 --- a/src/lib/testutils/Makefile.am +++ b/src/lib/testutils/Makefile.am @@ -7,7 +7,8 @@ noinst_SCRIPTS = dhcp_test_lib.sh if HAVE_GTEST noinst_LTLIBRARIES = libkea-testutils.la -libkea_testutils_la_SOURCES = unix_control_client.h unix_control_client.cc +libkea_testutils_la_SOURCES = io_utils.cc io_utils.h +libkea_testutils_la_SOURCES += unix_control_client.h unix_control_client.cc libkea_testutils_la_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) libkea_testutils_la_LIBADD = $(top_builddir)/src/lib/asiolink/libkea-asiolink.la libkea_testutils_la_LIBADD += $(top_builddir)/src/lib/dns/libkea-dns++.la diff --git a/src/lib/testutils/io_utils.cc b/src/lib/testutils/io_utils.cc new file mode 100644 index 0000000000..ae7dd1de3f --- /dev/null +++ b/src/lib/testutils/io_utils.cc @@ -0,0 +1,52 @@ +// Copyright (C) 2015 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 + +namespace isc { +namespace dhcp { +namespace test { + +bool fileExists(const std::string& file_path) { + std::ifstream fs(file_path.c_str()); + const bool file_exists = fs.good(); + fs.close(); + return (file_exists); +} + +std::string readFile(const std::string& file_path) { + std::ifstream ifs; + ifs.open(file_path.c_str(), std::ifstream::in); + if (!ifs.good()) { + return (std::string()); + } + std::string buf; + std::ostringstream output; + while (!ifs.eof() && ifs.good()) { + ifs >> buf; + output << buf; + } + ifs.close(); + + return (output.str()); +} + +}; // 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 new file mode 100644 index 0000000000..90c1dd7943 --- /dev/null +++ b/src/lib/testutils/io_utils.h @@ -0,0 +1,40 @@ +// Copyright (C) 2015 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. + +#ifndef TEST_IO_UTILS_H +#define TEST_IO_UTILS_H + +#include + +namespace isc { +namespace dhcp { +namespace test { + +/// @brief Checks if specified file exists. +/// +/// @param file_path Path to a file. +/// @return true if the file exists, false otherwise. +bool fileExists(const std::string& file_path); + +/// @brief Reads contents of the specified file. +/// +/// @param file_path Path to a file. +/// @return File contents. +std::string readFile(const std::string& file_path); + +}; // end of isc::dhcp::test namespace +}; // end of isc::dhcp namespace +}; // end of isc namespace + +#endif // TEST_IO_UTILS_H From 333527b07232fd6846889d4c7b69ea2523f67be3 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Tue, 1 Dec 2015 11:35:52 +0100 Subject: [PATCH 2/2] [4211] Addressed review comments. Two nits. --- doc/guide/dhcp6-srv.xml | 2 +- src/bin/dhcp6/dhcp6.spec | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml index a1f6624026..0237a24c86 100644 --- a/doc/guide/dhcp6-srv.xml +++ b/doc/guide/dhcp6-srv.xml @@ -2602,7 +2602,7 @@ should include options from the isc option space: In some uncommon deployments where no stable storage is available, it is desired to configure the server to not try to - store the server identifier on the hard drive. It is controlled + store the server identifier on the stable storage. It is controlled by the value of persist boolean parameter: "Dhcp6": { diff --git a/src/bin/dhcp6/dhcp6.spec b/src/bin/dhcp6/dhcp6.spec index ffe3779f73..14e4026593 100644 --- a/src/bin/dhcp6/dhcp6.spec +++ b/src/bin/dhcp6/dhcp6.spec @@ -43,7 +43,8 @@ "item_name": "persist", "item_type": "boolean", "item_optional": true, - "item_default": true + "item_default": true, + "item_description": "Indicates if generated server identifier should be stored in a stable storage." } ] },