From: Razvan Becheriu Date: Wed, 14 May 2025 17:06:20 +0000 (+0300) Subject: [#3840] backport #3831 to 2_6 X-Git-Tag: Kea-2.6.3~15 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0cd926075e48cd1a2f41c2ab00098d0795803f96;p=thirdparty%2Fkea.git [#3840] backport #3831 to 2_6 --- diff --git a/doc/sphinx/arm/dhcp6-srv.rst b/doc/sphinx/arm/dhcp6-srv.rst index f924a447f7..09931616ad 100644 --- a/doc/sphinx/arm/dhcp6-srv.rst +++ b/doc/sphinx/arm/dhcp6-srv.rst @@ -6236,7 +6236,7 @@ memory lease file into its data directory. By default this directory is .. note:: - As of Kea 2.7.9, ``data-directory`` is deprecated. The duid and lease + As of Kea 2.6.3, ``data-directory`` is deprecated. The duid and lease files may only be loaded from the directory determined at compilation: ``"[kea-install-dir]/var/lib/kea"``. This path may be overridden at startup by setting the environment variable diff --git a/doc/sphinx/arm/hooks-lease-cmds.rst b/doc/sphinx/arm/hooks-lease-cmds.rst index 827ddd90b2..46e54a03e7 100644 --- a/doc/sphinx/arm/hooks-lease-cmds.rst +++ b/doc/sphinx/arm/hooks-lease-cmds.rst @@ -1089,6 +1089,16 @@ the file in an attempt to synchronize both the files and the in-memory images of the lease database. The extension ``.bak`` and the server PID number are added to the previous filename: for example, ``.bak14326``. +.. note:: + + As of Kea 2.6.3, the lease file may only be written to the data directory + determined during compilation: ``"[kea-install-dir]/var/lib/kea"``. This + path may be overridden at startup by setting the environment variable + ``KEA_DHCP_DATA_DIRECTORY`` to the desired path. If a path other than + this value is used in ``name``, Kea will emit an error and refuse to start + or, if already running, log an unrecoverable error. For ease of use in + specifying a custom file name simply omit the path portion from ``filename``. + .. note:: These commands do not replace the LFC mechanism; they should be used diff --git a/doc/sphinx/arm/logging.rst b/doc/sphinx/arm/logging.rst index 2e91edf49c..6ecb024dc2 100644 --- a/doc/sphinx/arm/logging.rst +++ b/doc/sphinx/arm/logging.rst @@ -647,6 +647,17 @@ output), ``stderr`` (messages are printed on stderr), ``syslog`` (messages are logged to syslog using a specified name). Any other value is interpreted as a filename to which messages should be written. +.. note:: + + As of Kea 2.6.3, log files may only be written to the output directory + determined during compilation as: ``"[kea-install-dir]/var/log/kea"``. This + path may be overridden at startup by setting the environment variable + ``KEA_LOG_FILE_DIR`` to the desired path. If a path other than + this value is used in ``output``, Kea will emit an error and refuse to start + or, if already running, log an unrecoverable error. For ease of use simply + omit the path component from ``output`` and specify only the file name. + + The ``flush`` (boolean) Option ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/bin/admin/tests/memfile_tests.sh.in b/src/bin/admin/tests/memfile_tests.sh.in index bd92ed672b..6cec3f1297 100644 --- a/src/bin/admin/tests/memfile_tests.sh.in +++ b/src/bin/admin/tests/memfile_tests.sh.in @@ -15,6 +15,8 @@ set -eu . "@abs_top_builddir@/src/lib/testutils/dhcp_test_lib.sh" export KEA_DHCP_DATA_DIR="@abs_top_builddir@/src/bin/admin/tests" +export KEA_LOG_FILE_DIR="@abs_top_builddir@/src/bin/admin/tests" + # Locations of memfile tools kea_admin="@abs_top_builddir@/src/bin/admin/kea-admin" diff --git a/src/bin/agent/tests/ca_process_tests.sh.in b/src/bin/agent/tests/ca_process_tests.sh.in index ab7e47b23d..a6c6343aea 100644 --- a/src/bin/agent/tests/ca_process_tests.sh.in +++ b/src/bin/agent/tests/ca_process_tests.sh.in @@ -21,6 +21,9 @@ LOG_FILE="@abs_top_builddir@/src/bin/agent/tests/test.log" # Set env KEA_HOOKS_PATH to override DEFAULT_HOOKS_PATH export KEA_HOOKS_PATH="@abs_top_builddir@/src/bin/agent/tests/.libs" +# Set env KEA_LOG_FILE_DIR to override default log path +export KEA_LOG_FILE_DIR="@abs_top_builddir@/src/bin/agent/tests" + # Control Agent configuration to be stored in the configuration file. CONFIG="{ \"Control-agent\": diff --git a/src/bin/d2/tests/d2_cfg_mgr_unittests.cc b/src/bin/d2/tests/d2_cfg_mgr_unittests.cc index 8d9fe15c1f..88da8c6812 100644 --- a/src/bin/d2/tests/d2_cfg_mgr_unittests.cc +++ b/src/bin/d2/tests/d2_cfg_mgr_unittests.cc @@ -241,7 +241,7 @@ public: }; /// @brief Convenience macros for invoking runOrConfig() -#define RUN_CONFIG_OK(a) (runConfigOrFail(a, NO_ERROR, "")) +#define RUN_CONFIG_OK(a) (static_cast(runConfigOrFail(a, NO_ERROR, ""))) #define SYNTAX_ERROR(a,b) ASSERT_TRUE(runConfigOrFail(a,SYNTAX_ERROR,b)) #define LOGIC_ERROR(a,b) ASSERT_TRUE(runConfigOrFail(a,LOGIC_ERROR,b)) diff --git a/src/bin/d2/tests/d2_process_tests.sh.in b/src/bin/d2/tests/d2_process_tests.sh.in index cf9de463a3..ec119f14b3 100644 --- a/src/bin/d2/tests/d2_process_tests.sh.in +++ b/src/bin/d2/tests/d2_process_tests.sh.in @@ -17,6 +17,10 @@ LOG_FILE="@abs_top_builddir@/src/bin/d2/tests/test.log" # D2 configuration to be stored in the configuration file. # Set env KEA_HOOKS_PATH to override DEFAULT_HOOKS_PATH export KEA_HOOKS_PATH="@abs_top_builddir@/src/bin/d2/tests/.libs" + +# Set env KEA_LOG_FILE_DIR to override default log path +export KEA_LOG_FILE_DIR="@abs_top_builddir@/src/bin/d2/tests" + CONFIG="{ \"DhcpDdns\": { diff --git a/src/bin/d2/tests/d2_process_unittests.cc b/src/bin/d2/tests/d2_process_unittests.cc index c5e3c95810..845544af5d 100644 --- a/src/bin/d2/tests/d2_process_unittests.cc +++ b/src/bin/d2/tests/d2_process_unittests.cc @@ -646,7 +646,7 @@ TEST_F(D2ProcessTest, notLoopbackTest) { // Note we don't care nor can we predict if this // succeeds or fails. The address and port may or may // not be valid on the test host. - runWithConfig(config); + static_cast(runWithConfig(config)); } /// @brief Used to permit visual inspection of logs to ensure diff --git a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc index 187afde3c8..f4960588ef 100644 --- a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -53,6 +54,7 @@ using namespace isc::hooks; using namespace isc::stats; using namespace isc::test; using namespace isc::util; +using namespace isc::process; namespace ph = std::placeholders; namespace { @@ -113,6 +115,7 @@ public: /// /// Sets socket path to its default value. CtrlChannelDhcpv4SrvTest() : interfaces_("\"*\"") { + resetLogPath(); const char* env = getenv("KEA_SOCKET_TEST_DIR"); if (env) { socket_path_ = string(env) + "/kea4.sock"; @@ -127,6 +130,7 @@ public: /// @brief Destructor ~CtrlChannelDhcpv4SrvTest() { + resetLogPath(); LeaseMgrFactory::destroy(); StatsMgr::instance().removeAll(); @@ -144,6 +148,18 @@ public: IfaceMgr::instance().detectIfaces(); } + /// @brief Sets the log path where log output may be written. + /// @param explicit_path path to use as the log path. + void setLogTestPath(const std::string explicit_path = "") { + LogConfigParser::getLogPath(true, (!explicit_path.empty() ? + explicit_path : TEST_DATA_BUILDDIR)); + } + + /// @brief Resets the log path to TEST_DATA_BUILDDIR. + void resetLogPath() { + LogConfigParser::getLogPath(true); + } + /// @brief Returns pointer to the server's IO service. /// /// @return Pointer to the server's IO service or null pointer if the server @@ -647,6 +663,7 @@ TEST_F(CtrlChannelDhcpv4SrvTest, controlChannelStats) { // Check that the "config-set" command will replace current configuration TEST_F(CtrlChannelDhcpv4SrvTest, configSet) { + setLogTestPath("/dev"); createUnixChannelServer(); // Define strings to permutate the config arguments @@ -884,6 +901,7 @@ TEST_F(CtrlChannelDhcpv4SrvTest, configHashGet) { // Verify that the "config-test" command will do what we expect. TEST_F(CtrlChannelDhcpv4SrvTest, configTest) { + setLogTestPath("/dev"); createUnixChannelServer(); // Define strings to permutate the config arguments diff --git a/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in b/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in index 987c919630..bc4dffa6bc 100644 --- a/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in +++ b/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in @@ -23,11 +23,14 @@ export KEA_LFC_EXECUTABLE="@abs_top_builddir@/src/bin/lfc/kea-lfc" HOOK_FAIL_LOAD_PATH="@abs_top_builddir@/src/bin/dhcp4/tests/.libs/libco3.so" # Path to test hooks library HOOK_FAIL_POLL_PATH="@abs_top_builddir@/src/bin/dhcp4/tests/.libs/libco4.so" -# Kea configuration to be stored in the configuration file. -# Set env KEA_HOOKS_PATH to override DEFAULT_HOOKS_PATH +# Set env KEA_HOOKS_PATH to override DEFAULT_HOOKS_PATH export KEA_HOOKS_PATH="@abs_top_builddir@/src/bin/dhcp4/tests/.libs" +# Set env KEA_LOG_FILE_DIR to override default log path. +export KEA_LOG_FILE_DIR="@abs_top_builddir@/src/bin/dhcp4/tests" + +# Kea configuration to be stored in the configuration file. CONFIG="{ \"Dhcp4\": { diff --git a/src/bin/dhcp4/tests/direct_client_unittest.cc b/src/bin/dhcp4/tests/direct_client_unittest.cc index 913885ea63..3921dabe88 100644 --- a/src/bin/dhcp4/tests/direct_client_unittest.cc +++ b/src/bin/dhcp4/tests/direct_client_unittest.cc @@ -219,7 +219,7 @@ DirectClientTest::createClientMessage(const Pkt4Ptr& msg, // local and remote address are set like it was a message sent from the // directly connected client. Pkt4Ptr received; - createPacketFromBuffer(msg, received); + static_cast(createPacketFromBuffer(msg, received)); received->setIface(iface); received->setIndex(ifindex); received->setLocalAddr(IOAddress("255.255.255.255")); diff --git a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc index 6e7103bee8..87bee8e493 100644 --- a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc @@ -559,6 +559,7 @@ TEST_F(CtrlChannelDhcpv6SrvTest, controlChannelShutdown) { // Check that the "config-set" command will replace current configuration TEST_F(CtrlChannelDhcpv6SrvTest, configSet) { + setLogTestPath("/dev"); createUnixChannelServer(); // Define strings to permutate the config arguments @@ -797,6 +798,7 @@ TEST_F(CtrlChannelDhcpv6SrvTest, configHashGet) { // Verify that the "config-test" command will do what we expect. TEST_F(CtrlChannelDhcpv6SrvTest, configTest) { + setLogTestPath("/dev"); createUnixChannelServer(); // Define strings to permutate the config arguments diff --git a/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in b/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in index 85b0c927d1..34d7267334 100644 --- a/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in +++ b/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in @@ -23,10 +23,14 @@ export KEA_LFC_EXECUTABLE="@abs_top_builddir@/src/bin/lfc/kea-lfc" HOOK_FAIL_LOAD_PATH="@abs_top_builddir@/src/bin/dhcp6/tests/.libs/libco3.so" # Path to test hooks library HOOK_FAIL_POLL_PATH="@abs_top_builddir@/src/bin/dhcp6/tests/.libs/libco4.so" -# Kea configuration to be stored in the configuration file. -# Set env KEA_HOOKS_PATH to override DEFAULT_HOOKS_PATH + +# Set env KEA_HOOKS_PATH to override DEFAULT_HOOKS_PATH export KEA_HOOKS_PATH="@abs_top_builddir@/src/bin/dhcp6/tests/.libs" +# Set env KEA_LOG_FILE_DIR to override default log path +export KEA_LOG_FILE_DIR="@abs_top_builddir@/src/bin/dhcp6/tests" + +# Kea configuration to be stored in the configuration file. CONFIG="{ \"Dhcp6\": { diff --git a/src/bin/dhcp6/tests/dhcp6_test_utils.cc b/src/bin/dhcp6/tests/dhcp6_test_utils.cc index ac05077040..35872e339e 100644 --- a/src/bin/dhcp6/tests/dhcp6_test_utils.cc +++ b/src/bin/dhcp6/tests/dhcp6_test_utils.cc @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -24,6 +25,7 @@ using namespace isc::dhcp; using namespace isc::asiolink; using namespace isc::stats; using namespace isc::util; +using namespace isc::process; using namespace boost::posix_time; namespace isc { @@ -37,6 +39,7 @@ BaseServerTest::BaseServerTest() { CfgMgr::instance().setFamily(AF_INET6); original_datadir_ = CfgMgr::instance().getDataDir(); CfgMgr::instance().getDataDir(true, TEST_DATA_BUILDDIR); + resetLogPath(); } BaseServerTest::~BaseServerTest() { @@ -59,6 +62,18 @@ BaseServerTest::~BaseServerTest() { // Revert to unit test logging, in case the test reconfigured it. isc::log::initLogger(); + resetLogPath(); +} + +void +BaseServerTest::setLogTestPath(const std::string explicit_path /* = "" */) { + LogConfigParser::getLogPath(true, (!explicit_path.empty() ? + explicit_path : TEST_DATA_BUILDDIR)); +} + +void +BaseServerTest::resetLogPath() { + LogConfigParser::getLogPath(true); } Dhcpv6SrvTest::Dhcpv6SrvTest() diff --git a/src/bin/dhcp6/tests/dhcp6_test_utils.h b/src/bin/dhcp6/tests/dhcp6_test_utils.h index fd7a11f474..938d67ca48 100644 --- a/src/bin/dhcp6/tests/dhcp6_test_utils.h +++ b/src/bin/dhcp6/tests/dhcp6_test_utils.h @@ -125,6 +125,13 @@ public: /// @brief Destructor. virtual ~BaseServerTest(); + /// @brief Sets the log path where log output may be written. + /// @param explicit_path path to use as the log path. + void setLogTestPath(const std::string explicit_path = ""); + + /// @brief Resets the log path to TEST_DATA_BUILDDIR. + void resetLogPath(); + private: /// @brief Holds the original data directory. diff --git a/src/bin/keactrl/kea-ctrl-agent.conf.pre b/src/bin/keactrl/kea-ctrl-agent.conf.pre index d8e04296ce..604ba0d6d4 100644 --- a/src/bin/keactrl/kea-ctrl-agent.conf.pre +++ b/src/bin/keactrl/kea-ctrl-agent.conf.pre @@ -73,7 +73,7 @@ // - syslog (logs to syslog) // - syslog:name (logs to syslog using specified name) // Any other value is considered a name of the file - "output": "@localstatedir@/log/kea-ctrl-agent.log" + "output": "kea-ctrl-agent.log" // Shorter log pattern suitable for use with systemd, // avoids redundant information diff --git a/src/bin/keactrl/kea-dhcp-ddns.conf.pre b/src/bin/keactrl/kea-dhcp-ddns.conf.pre index b75b51f390..0d7f11536e 100644 --- a/src/bin/keactrl/kea-dhcp-ddns.conf.pre +++ b/src/bin/keactrl/kea-dhcp-ddns.conf.pre @@ -44,7 +44,7 @@ // - syslog (logs to syslog) // - syslog:name (logs to syslog using specified name) // Any other value is considered a name of the file - "output": "@localstatedir@/log/kea-ddns.log" + "output": "kea-ddns.log" // Shorter log pattern suitable for use with systemd, // avoids redundant information diff --git a/src/bin/keactrl/kea-dhcp4.conf.pre b/src/bin/keactrl/kea-dhcp4.conf.pre index 55af9dbf61..06070508fc 100644 --- a/src/bin/keactrl/kea-dhcp4.conf.pre +++ b/src/bin/keactrl/kea-dhcp4.conf.pre @@ -436,7 +436,7 @@ // - syslog (logs to syslog) // - syslog:name (logs to syslog using specified name) // Any other value is considered a name of the file - "output": "@localstatedir@/log/kea-dhcp4.log" + "output": "kea-dhcp4.log" // Shorter log pattern suitable for use with systemd, // avoids redundant information diff --git a/src/bin/keactrl/kea-dhcp6.conf.pre b/src/bin/keactrl/kea-dhcp6.conf.pre index 271021b2f8..183866d58b 100644 --- a/src/bin/keactrl/kea-dhcp6.conf.pre +++ b/src/bin/keactrl/kea-dhcp6.conf.pre @@ -395,7 +395,7 @@ // - syslog (logs to syslog) // - syslog:name (logs to syslog using specified name) // Any other value is considered a name of the file - "output": "@localstatedir@/log/kea-dhcp6.log" + "output": "kea-dhcp6.log" // Shorter log pattern suitable for use with systemd, // avoids redundant information diff --git a/src/bin/keactrl/kea-netconf.conf.pre b/src/bin/keactrl/kea-netconf.conf.pre index c8a3878b41..bc7b3926f4 100644 --- a/src/bin/keactrl/kea-netconf.conf.pre +++ b/src/bin/keactrl/kea-netconf.conf.pre @@ -69,7 +69,7 @@ // - syslog (logs to syslog) // - syslog:name (logs to syslog using specified name) // Any other value is considered a name of a time - "output": "@localstatedir@/log/kea-netconf.log" + "output": "kea-netconf.log" // Shorter log pattern suitable for use with systemd, // avoids redundant information diff --git a/src/bin/keactrl/tests/keactrl_tests.sh.in b/src/bin/keactrl/tests/keactrl_tests.sh.in index 6c7c17b47b..f35e34c189 100644 --- a/src/bin/keactrl/tests/keactrl_tests.sh.in +++ b/src/bin/keactrl/tests/keactrl_tests.sh.in @@ -59,6 +59,10 @@ KEACTRL_BUILD_DIR="@abs_top_builddir@" KEACTRL_CFG_FILE="@abs_top_builddir@/src/bin/keactrl/tests/keactrl_test.conf" # Path to the Kea log file. LOG_FILE="@abs_top_builddir@/src/bin/keactrl/tests/test.log" + +# Set env KEA_LOG_FILE_DIR to override default log path +export KEA_LOG_FILE_DIR="@abs_top_builddir@/src/bin/keactrl/tests" + # Binaries' names wildcard_name="kea-" kea4_name="${wildcard_name}dhcp4" diff --git a/src/bin/netconf/tests/shtests/netconf_tests.sh.in b/src/bin/netconf/tests/shtests/netconf_tests.sh.in index 5432b259df..3e956c330c 100644 --- a/src/bin/netconf/tests/shtests/netconf_tests.sh.in +++ b/src/bin/netconf/tests/shtests/netconf_tests.sh.in @@ -13,6 +13,7 @@ set -eu # Path to the temporary configuration file. CFG_FILE="@abs_top_builddir@/src/bin/netconf/tests/shtests/test_config.json" # Path to the Kea log file. +export KEA_LOG_FILE_DIR="@abs_top_builddir@/src/bin/netconf/tests/shtests" LOG_FILE="@abs_top_builddir@/src/bin/netconf/tests/shtests/test.log" # Kea-netconf configuration to be stored in the configuration file. diff --git a/src/bin/shell/tests/basic_auth_tests.sh.in b/src/bin/shell/tests/basic_auth_tests.sh.in index f25a2cc862..43a27ae203 100644 --- a/src/bin/shell/tests/basic_auth_tests.sh.in +++ b/src/bin/shell/tests/basic_auth_tests.sh.in @@ -19,6 +19,9 @@ CFG_FILE="@abs_top_builddir@/src/bin/shell/tests/test_config.json" # Path to the Control Agent log file. LOG_FILE="@abs_top_builddir@/src/bin/shell/tests/test.log" +# Set env KEA_LOG_FILE_DIR to override default log path. +export KEA_LOG_FILE_DIR="@abs_top_builddir@/src/bin/shell/tests" + # Control Agent configuration to be stored in the configuration file. # todo: use actual configuration once we support it. CONFIG="{ diff --git a/src/bin/shell/tests/shell_process_tests.sh.in b/src/bin/shell/tests/shell_process_tests.sh.in index 3ed4071fb1..968983fc3d 100644 --- a/src/bin/shell/tests/shell_process_tests.sh.in +++ b/src/bin/shell/tests/shell_process_tests.sh.in @@ -19,6 +19,9 @@ CFG_FILE="@abs_top_builddir@/src/bin/shell/tests/test_config.json" # Path to the Control Agent log file. LOG_FILE="@abs_top_builddir@/src/bin/shell/tests/test.log" +# Set env KEA_LOG_FILE_DIR to override default log path. +export KEA_LOG_FILE_DIR="@abs_top_builddir@/src/bin/shell/tests" + # Control Agent configuration to be stored in the configuration file. # todo: use actual configuration once we support it. CONFIG="{ diff --git a/src/bin/shell/tests/tls_ca_process_tests.sh.in b/src/bin/shell/tests/tls_ca_process_tests.sh.in index 94d608f48e..0e4e51cc4c 100644 --- a/src/bin/shell/tests/tls_ca_process_tests.sh.in +++ b/src/bin/shell/tests/tls_ca_process_tests.sh.in @@ -15,10 +15,13 @@ set -eu . "@abs_top_builddir@/src/lib/testutils/dhcp_test_lib.sh" # Path to the temporary configuration file. -CFG_FILE="@abs_top_builddir@/src/bin/agent/tests/test_config.json" +CFG_FILE="@abs_top_builddir@/src/bin/shell/tests/test_config.json" # Path to the Control Agent log file. -LOG_FILE="@abs_top_builddir@/src/bin/agent/tests/test.log" +LOG_FILE="@abs_top_builddir@/src/bin/shell/tests/test.log" + +# Set env KEA_LOG_FILE_DIR to override default log path. +export KEA_LOG_FILE_DIR="@abs_top_builddir@/src/bin/shell/tests" # Path to the test certificate authority directory. TEST_CA_DIR="@abs_top_srcdir@/src/lib/asiolink/testutils/ca" diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds.cc b/src/hooks/dhcp/lease_cmds/lease_cmds.cc index b0baf99079..86f31f85dc 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds.cc +++ b/src/hooks/dhcp/lease_cmds/lease_cmds.cc @@ -2674,9 +2674,12 @@ LeaseCmdsImpl::leaseWriteHandler(CalloutHandle& handle) { if (file->getType() != Element::string) { isc_throw(BadValue, "'filename' parameter must be a string"); } - string filename = file->stringValue(); - if (filename.empty()) { - isc_throw(BadValue, "'filename' parameter is empty"); + + std::string filename; + try { + filename = CfgMgr::instance().validatePath(file->stringValue()); + } catch (const std::exception& ex) { + isc_throw(BadValue, "'filename' parameter is invalid: " << ex.what()); } if (v4) { @@ -2684,6 +2687,7 @@ LeaseCmdsImpl::leaseWriteHandler(CalloutHandle& handle) { } else { LeaseMgrFactory::instance().writeLeases6(filename); } + ostringstream s; s << (v4 ? "IPv4" : "IPv6") << " lease database into '" diff --git a/src/hooks/dhcp/lease_cmds/tests/lease_cmds4_unittest.cc b/src/hooks/dhcp/lease_cmds/tests/lease_cmds4_unittest.cc index 9416e34d45..825390e163 100644 --- a/src/hooks/dhcp/lease_cmds/tests/lease_cmds4_unittest.cc +++ b/src/hooks/dhcp/lease_cmds/tests/lease_cmds4_unittest.cc @@ -3429,8 +3429,23 @@ void Lease4CmdsTest::testLease4Write() { " \"filename\": \"\"\n" " }\n" "}"; - exp_rsp = "'filename' parameter is empty"; + exp_rsp = "'filename' parameter is invalid: path: '' has no filename"; testCommand(txt, CONTROL_RESULT_ERROR, exp_rsp); + + // Filename must use supported path. + txt = + "{\n" + " \"command\": \"lease4-write\",\n" + " \"arguments\": {" + " \"filename\": \"/tmp/myleases.txt\"\n" + " }\n" + "}"; + + std::ostringstream os; + os << "'filename' parameter is invalid: invalid path specified:" + << " '/tmp', supported path is '" << CfgMgr::instance().getDataDir() << "'"; + + testCommand(txt, CONTROL_RESULT_ERROR, os.str()); } TEST_F(Lease4CmdsTest, lease4AddMissingParams) { diff --git a/src/hooks/dhcp/lease_cmds/tests/lease_cmds6_unittest.cc b/src/hooks/dhcp/lease_cmds/tests/lease_cmds6_unittest.cc index 65cdda2275..2fb2d796a2 100644 --- a/src/hooks/dhcp/lease_cmds/tests/lease_cmds6_unittest.cc +++ b/src/hooks/dhcp/lease_cmds/tests/lease_cmds6_unittest.cc @@ -4129,7 +4129,7 @@ void Lease6CmdsTest::testLease6ConflictingBulkApplyAdd() { } void Lease6CmdsTest::testLease6Write() { - // lease4-write negative tests. Positive tests are in the + // lease6-write negative tests. Positive tests are in the // memfile_lease_mgr_unittest.cc file. // Initialize lease manager (true = v6, false = don't add leases) @@ -4164,8 +4164,21 @@ void Lease6CmdsTest::testLease6Write() { " \"filename\": \"\"\n" " }\n" "}"; - exp_rsp = "'filename' parameter is empty"; + exp_rsp = "'filename' parameter is invalid: path: '' has no filename"; testCommand(txt, CONTROL_RESULT_ERROR, exp_rsp); + + // Filename must use supported path. + txt = + "{\n" + " \"command\": \"lease6-write\",\n" + " \"arguments\": {" + " \"filename\": \"/tmp/myleases.txt\"\n" + " }\n" + "}"; + + std::ostringstream os; + os << "'filename' parameter is invalid: invalid path specified:" + << " '/tmp', supported path is '" << CfgMgr::instance().getDataDir() << "'"; } TEST_F(Lease6CmdsTest, lease6AddMissingParams) { diff --git a/src/lib/process/Makefile.am b/src/lib/process/Makefile.am index fc7e36270d..df251286a8 100644 --- a/src/lib/process/Makefile.am +++ b/src/lib/process/Makefile.am @@ -3,6 +3,8 @@ SUBDIRS = cfgrpt . testutils tests dhcp_data_dir = @runstatedir@/@PACKAGE@ AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib AM_CPPFLAGS += -DDATA_DIR="\"$(dhcp_data_dir)\"" +log_file_dir = @localstatedir@/log/@PACKAGE@ +AM_CPPFLAGS += -DLOGFILE_DIR="\"$(log_file_dir)\"" AM_CPPFLAGS += $(BOOST_INCLUDES) AM_CXXFLAGS = $(KEA_CXXFLAGS) diff --git a/src/lib/process/log_parser.cc b/src/lib/process/log_parser.cc index 413e8444c1..f052730cfc 100644 --- a/src/lib/process/log_parser.cc +++ b/src/lib/process/log_parser.cc @@ -13,13 +13,20 @@ #include #include #include +#include using namespace isc::data; using namespace isc::log; +using namespace isc::util::file; namespace isc { namespace process { +namespace { + // Singleton PathChecker to set and hold valid log file path. + PathCheckerPtr log_path_checker_; +}; + LogConfigParser::LogConfigParser(const ConfigPtr& storage) :config_(storage), verbose_(false) { if (!storage) { @@ -126,6 +133,28 @@ void LogConfigParser::parseConfigEntry(isc::data::ConstElementPtr entry) { config_->addLoggingInfo(info); } +std::string +LogConfigParser::getLogPath(bool reset /* = false */, const std::string explicit_path /* = "" */) { + if (!log_path_checker_ || reset) { + log_path_checker_.reset(new PathChecker(LOGFILE_DIR, "KEA_LOG_FILE_DIR")); + if (!explicit_path.empty()) { + log_path_checker_->getPath(true, explicit_path); + } + } + + return (log_path_checker_->getPath()); +} + +std::string +LogConfigParser::validatePath(const std::string logpath, + bool enforce_path /* = true */) { + if (!log_path_checker_) { + getLogPath(); + } + + return (log_path_checker_->validatePath(logpath, enforce_path)); +} + void LogConfigParser::parseOutputOptions(std::vector& destination, isc::data::ConstElementPtr output_options) { if (!output_options) { @@ -141,7 +170,20 @@ void LogConfigParser::parseOutputOptions(std::vector& destin isc_throw(BadValue, "output-options entry does not have a mandatory 'output' " "element (" << output_option->getPosition() << ")"); } - dest.output_ = output->stringValue(); + + auto output_str = output->stringValue(); + if ((output_str == "stdout") || + (output_str == "stderr") || + (output_str == "syslog")) { + dest.output_ = output_str; + } else { + try { + dest.output_ = validatePath(output_str); + } catch (const std::exception& ex) { + isc_throw(BadValue, "invalid path in `output`: " << ex.what() + << " (" << output_option->getPosition() << ")"); + } + } isc::data::ConstElementPtr maxver_ptr = output_option->get("maxver"); if (maxver_ptr) { diff --git a/src/lib/process/log_parser.h b/src/lib/process/log_parser.h index bfb80d0ddc..cf3a7f2c24 100644 --- a/src/lib/process/log_parser.h +++ b/src/lib/process/log_parser.h @@ -58,6 +58,29 @@ public: void parseConfiguration(const isc::data::ConstElementPtr& log_config, bool verbose = false); + /// @brief Fetches the supported log file path. + /// + /// The first call to this function with no arguments will set the default + /// hooks path to either the value of LOGFILE_DIR or the environment + /// variable KEA_LOG_FILE_DIR if it is defined. Subsequent calls with no + /// arguments will simply return this value. + /// + /// @param reset recalculate when true, defaults to false. + /// @param explicit_path set default log path to this value. This is + /// for testing purposes only. + /// + /// @return String containing the default log file path. + static std::string getLogPath(bool reset = false, const std::string explicit_path = ""); + + /// @brief Validates a library path against the supported path for log files. + /// + /// @param logpath path to validate. + /// @param enforce_path enables validation against the supported path. + /// If false verifies only that the path contains a file name. + /// + /// @return validated path + static std::string validatePath(const std::string logpath, bool enforce_path = true); + private: /// @brief Parses one JSON structure in Server/loggers" array diff --git a/src/lib/process/tests/d_cfg_mgr_unittests.cc b/src/lib/process/tests/d_cfg_mgr_unittests.cc index 7cfa746a85..676abb27db 100644 --- a/src/lib/process/tests/d_cfg_mgr_unittests.cc +++ b/src/lib/process/tests/d_cfg_mgr_unittests.cc @@ -260,7 +260,7 @@ TEST_F(DStubCfgMgrTest, simpleParseConfig) { string config = "{ \"bool_test\": true , \n" " \"uint32_test\": 77 , \n" " \"string_test\": \"hmmm chewy\" }"; - ASSERT_NO_THROW(fromJSON(config)); + ASSERT_NO_THROW(static_cast(fromJSON(config))); answer_ = cfg_mgr_->simpleParseConfig(config_set_, false); EXPECT_TRUE(checkAnswer(0)); @@ -272,7 +272,7 @@ TEST_F(DStubCfgMgrTest, simpleParseConfigWithCallback) { string config = "{ \"bool_test\": true , \n" " \"uint32_test\": 77 , \n" " \"string_test\": \"hmmm chewy\" }"; - ASSERT_NO_THROW(fromJSON(config)); + ASSERT_NO_THROW(static_cast(fromJSON(config))); answer_ = cfg_mgr_->simpleParseConfig(config_set_, false, []() { diff --git a/src/lib/process/tests/log_parser_unittests.cc b/src/lib/process/tests/log_parser_unittests.cc index 2a6970c664..b833ed9642 100644 --- a/src/lib/process/tests/log_parser_unittests.cc +++ b/src/lib/process/tests/log_parser_unittests.cc @@ -32,7 +32,9 @@ namespace { class LoggingTest : public ::testing::Test { public: /// @brief Constructor - LoggingTest() {} + LoggingTest() { + resetLogPath(); + } /// @brief Destructor /// @@ -40,6 +42,7 @@ class LoggingTest : public ::testing::Test { ~LoggingTest() { isc::log::initLogger(); wipeFiles(); + resetLogPath(); } /// @brief Generates a log file name suffixed with a rotation number @@ -63,6 +66,19 @@ class LoggingTest : public ::testing::Test { static_cast(remove(os.str().c_str())); } + /// @brief Sets the Hooks path from which hooks can be loaded. + /// @param explicit_path path to use as the hooks path. + void setLogTestPath(const std::string explicit_path = "") { + LogConfigParser::getLogPath(true, + (!explicit_path.empty() ? + explicit_path : TEST_DATA_BUILDDIR)); + } + + /// @brief Resets the log path to default. + void resetLogPath() { + LogConfigParser::getLogPath(true); + } + /// @brief Name of the log file static const char* TEST_LOG_NAME; @@ -309,7 +325,8 @@ TEST_F(LoggingTest, parsingFile) { EXPECT_EQ(isc::log::INFO, storage->getLoggingInfo()[0].severity_); ASSERT_EQ(1, storage->getLoggingInfo()[0].destinations_.size()); - EXPECT_EQ("logfile.txt" , storage->getLoggingInfo()[0].destinations_[0].output_); + EXPECT_EQ(LogConfigParser::validatePath("logfile.txt"), + storage->getLoggingInfo()[0].destinations_[0].output_); // Default for immediate flush is true EXPECT_TRUE(storage->getLoggingInfo()[0].destinations_[0].flush_); @@ -365,14 +382,15 @@ TEST_F(LoggingTest, multipleLoggers) { EXPECT_EQ(0, storage->getLoggingInfo()[0].debuglevel_); EXPECT_EQ(isc::log::INFO, storage->getLoggingInfo()[0].severity_); ASSERT_EQ(1, storage->getLoggingInfo()[0].destinations_.size()); - EXPECT_EQ("logfile.txt" , storage->getLoggingInfo()[0].destinations_[0].output_); + EXPECT_EQ(LogConfigParser::validatePath("logfile.txt"), + storage->getLoggingInfo()[0].destinations_[0].output_); EXPECT_TRUE(storage->getLoggingInfo()[0].destinations_[0].flush_); - EXPECT_EQ("wombat", storage->getLoggingInfo()[1].name_); EXPECT_EQ(99, storage->getLoggingInfo()[1].debuglevel_); EXPECT_EQ(isc::log::DEBUG, storage->getLoggingInfo()[1].severity_); ASSERT_EQ(1, storage->getLoggingInfo()[1].destinations_.size()); - EXPECT_EQ("logfile2.txt" , storage->getLoggingInfo()[1].destinations_[0].output_); + EXPECT_EQ(LogConfigParser::validatePath("logfile2.txt"), + storage->getLoggingInfo()[1].destinations_[0].output_); EXPECT_FALSE(storage->getLoggingInfo()[1].destinations_[0].flush_); } @@ -380,7 +398,7 @@ TEST_F(LoggingTest, multipleLoggers) { // into Configuration usable by log4cplus. This test checks that more than // one logging destination can be configured. TEST_F(LoggingTest, multipleLoggingDestinations) { - + setLogTestPath(); const char* config_txt = "{ \"loggers\": [" " {" @@ -415,7 +433,8 @@ TEST_F(LoggingTest, multipleLoggingDestinations) { EXPECT_EQ(0, storage->getLoggingInfo()[0].debuglevel_); EXPECT_EQ(isc::log::INFO, storage->getLoggingInfo()[0].severity_); ASSERT_EQ(2, storage->getLoggingInfo()[0].destinations_.size()); - EXPECT_EQ("logfile.txt" , storage->getLoggingInfo()[0].destinations_[0].output_); + EXPECT_EQ(LogConfigParser::validatePath("logfile.txt"), + storage->getLoggingInfo()[0].destinations_[0].output_); EXPECT_TRUE(storage->getLoggingInfo()[0].destinations_[0].flush_); EXPECT_EQ("stdout" , storage->getLoggingInfo()[0].destinations_[1].output_); EXPECT_TRUE(storage->getLoggingInfo()[0].destinations_[1].flush_); @@ -427,6 +446,7 @@ TEST_F(LoggingTest, multipleLoggingDestinations) { // we can correctly configure logging such that rotation occurs as // expected. TEST_F(LoggingTest, logRotate) { + setLogTestPath(); wipeFiles(); std::ostringstream os; @@ -467,7 +487,7 @@ TEST_F(LoggingTest, logRotate) { EXPECT_EQ(TEST_MAX_VERS, server_cfg->getLoggingInfo()[0].destinations_[0].maxver_); // Make sure we have the initial log file. - ASSERT_TRUE(isc::test::fileExists(TEST_LOG_NAME)); + ASSERT_TRUE(isc::test::fileExists(LogConfigParser::validatePath(TEST_LOG_NAME))); // Now generate a log we know will be large enough to force a rotation. // We borrow a one argument log message for the test. @@ -477,7 +497,7 @@ TEST_F(LoggingTest, logRotate) { for (int i = 1; i < TEST_MAX_VERS + 1; i++) { // Output the big log and make sure we get the expected rotation file. LOG_INFO(logger, DCTL_CONFIG_COMPLETE).arg(big_arg); - EXPECT_TRUE(isc::test::fileExists(logName(i).c_str())); + EXPECT_TRUE(isc::test::fileExists(LogConfigParser::validatePath(logName(i).c_str()))); } // Clean up. @@ -604,6 +624,7 @@ void testMaxSize(uint64_t maxsize_candidate, uint64_t expected_maxsize) { // Test that maxsize can be configured with high values. TEST_F(LoggingTest, maxsize) { + setLogTestPath(); testMaxSize(TEST_MAX_SIZE, TEST_MAX_SIZE); testMaxSize(std::numeric_limits::max(), std::numeric_limits::max()); testMaxSize(std::numeric_limits::max(), std::numeric_limits::max()); diff --git a/src/lib/util/filesystem.cc b/src/lib/util/filesystem.cc index d8e1cffd18..495091dad2 100644 --- a/src/lib/util/filesystem.cc +++ b/src/lib/util/filesystem.cc @@ -248,7 +248,7 @@ PathChecker::validatePath(const std::string input_path_str, auto filename = input_path.filename(); if (filename.empty()) { isc_throw(BadValue, "path: '" << input_path.str() << "' has no filename"); - } + } auto parent_path = input_path.parentPath(); if (!parent_path.empty()) {