From: Razvan Becheriu Date: Wed, 14 May 2025 18:18:04 +0000 (+0300) Subject: [#3839] backport #3831 to 2_4 X-Git-Tag: Kea-2.4.2~12 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=392a59b71f61bd78c10e9c4538ef2ba7033fd797;p=thirdparty%2Fkea.git [#3839] backport #3831 to 2_4 --- diff --git a/doc/sphinx/arm/hooks-lease-cmds.rst b/doc/sphinx/arm/hooks-lease-cmds.rst index 61ee7558ca..053609a38f 100644 --- a/doc/sphinx/arm/hooks-lease-cmds.rst +++ b/doc/sphinx/arm/hooks-lease-cmds.rst @@ -1066,6 +1066,16 @@ the file in an attempt to synchronize both the files and the in-memory images of the lease database. The extension ``.bak`` with server PID number is added to the previous filename. For example ``.bak14326``. +.. note:: + + As of Kea 2.4.2, 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 117a277ce2..029eaf8dbb 100644 --- a/doc/sphinx/arm/logging.rst +++ b/doc/sphinx/arm/logging.rst @@ -624,6 +624,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.4.2, 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 c3e8c2b6fb..f9e4554e1d 100644 --- a/src/bin/admin/tests/memfile_tests.sh.in +++ b/src/bin/admin/tests/memfile_tests.sh.in @@ -20,6 +20,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 b5331523ec..e0b8e4199e 100644 --- a/src/bin/agent/tests/ca_process_tests.sh.in +++ b/src/bin/agent/tests/ca_process_tests.sh.in @@ -24,6 +24,12 @@ CFG_FILE="@abs_top_builddir@/src/bin/agent/tests/test_config.json" # Path to the Control Agent log file. 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 7e2aee5263..88af349e3e 100644 --- a/src/bin/d2/tests/d2_cfg_mgr_unittests.cc +++ b/src/bin/d2/tests/d2_cfg_mgr_unittests.cc @@ -242,7 +242,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 408e8e0fad..556866bbaf 100644 --- a/src/bin/d2/tests/d2_process_tests.sh.in +++ b/src/bin/d2/tests/d2_process_tests.sh.in @@ -20,8 +20,13 @@ set -eu CFG_FILE="@abs_top_builddir@/src/bin/d2/tests/test_config.json" # Path to the D2 log file. LOG_FILE="@abs_top_builddir@/src/bin/d2/tests/test.log" + # 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" + # D2 configuration to be stored in the configuration file. CONFIG="{ \"DhcpDdns\": diff --git a/src/bin/d2/tests/d2_process_unittests.cc b/src/bin/d2/tests/d2_process_unittests.cc index cfff351cfa..ce321d21f5 100644 --- a/src/bin/d2/tests/d2_process_unittests.cc +++ b/src/bin/d2/tests/d2_process_unittests.cc @@ -636,7 +636,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 a135c64ea8..3f0f4d678c 100644 --- a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc @@ -21,6 +21,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(); @@ -143,6 +147,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 @@ -735,6 +751,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 @@ -971,6 +988,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 9b205b393d..d09c416e9a 100644 --- a/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in +++ b/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in @@ -31,6 +31,9 @@ HOOK_PATH="@abs_top_builddir@/src/bin/dhcp4/tests/.libs/libco3.so" # 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 27439e2baa..32030eb731 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 ac52dd8183..ed02e4c979 100644 --- a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc @@ -647,6 +647,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 @@ -884,6 +885,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 212d5f8032..a54ec2b1fc 100644 --- a/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in +++ b/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in @@ -28,9 +28,12 @@ export KEA_LFC_EXECUTABLE="@abs_top_builddir@/src/bin/lfc/kea-lfc" # Path to test hooks library HOOK_PATH="@abs_top_builddir@/src/bin/dhcp6/tests/.libs/libco3.so" -# 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 0ba02d0a56..8d0cf955e9 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; namespace isc { namespace dhcp { @@ -36,6 +38,7 @@ BaseServerTest::BaseServerTest() { CfgMgr::instance().setFamily(AF_INET6); original_datadir_ = CfgMgr::instance().getDataDir(); CfgMgr::instance().getDataDir(true, TEST_DATA_BUILDDIR); + resetLogPath(); } BaseServerTest::~BaseServerTest() { @@ -58,6 +61,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 c5150f6772..d2b2d45af6 100644 --- a/src/bin/dhcp6/tests/dhcp6_test_utils.h +++ b/src/bin/dhcp6/tests/dhcp6_test_utils.h @@ -123,6 +123,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 e6ae8b8ac6..d4d8e041f9 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 07c204283a..f88a3a393f 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 6edb8a14c7..adb5647914 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 b167e1ffe0..ae2f01f88a 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 37d15b14a1..80372eb266 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 b4b49cb625..e3ac422212 100644 --- a/src/bin/keactrl/tests/keactrl_tests.sh.in +++ b/src/bin/keactrl/tests/keactrl_tests.sh.in @@ -61,6 +61,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 b6ba611223..a5ad5909d9 100644 --- a/src/bin/netconf/tests/shtests/netconf_tests.sh.in +++ b/src/bin/netconf/tests/shtests/netconf_tests.sh.in @@ -19,6 +19,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 36ea16c2fc..d4bd058716 100644 --- a/src/bin/shell/tests/basic_auth_tests.sh.in +++ b/src/bin/shell/tests/basic_auth_tests.sh.in @@ -22,6 +22,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 5f2e00a149..cbef463c79 100644 --- a/src/bin/shell/tests/shell_process_tests.sh.in +++ b/src/bin/shell/tests/shell_process_tests.sh.in @@ -22,6 +22,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 2c23188c08..1e985bd210 100644 --- a/src/bin/shell/tests/tls_ca_process_tests.sh.in +++ b/src/bin/shell/tests/tls_ca_process_tests.sh.in @@ -20,10 +20,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 f4531aa6aa..8420d1d85e 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds.cc +++ b/src/hooks/dhcp/lease_cmds/lease_cmds.cc @@ -2671,9 +2671,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) { @@ -2681,6 +2684,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 444b4be024..b02cddfec4 100644 --- a/src/hooks/dhcp/lease_cmds/tests/lease_cmds4_unittest.cc +++ b/src/hooks/dhcp/lease_cmds/tests/lease_cmds4_unittest.cc @@ -3424,8 +3424,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 9b95b5e08d..466f4daaaa 100644 --- a/src/hooks/dhcp/lease_cmds/tests/lease_cmds6_unittest.cc +++ b/src/hooks/dhcp/lease_cmds/tests/lease_cmds6_unittest.cc @@ -4148,7 +4148,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) @@ -4183,8 +4183,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 cb20ab817c..e5a39d1811 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 258aa4eaf5..daf397cc6d 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) { @@ -113,6 +120,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) { @@ -128,7 +157,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 92209bd17f..e3dc34ddc2 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 faa3e92897..c965eb759b 100644 --- a/src/lib/process/tests/d_cfg_mgr_unittests.cc +++ b/src/lib/process/tests/d_cfg_mgr_unittests.cc @@ -261,7 +261,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)); @@ -273,7 +273,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 0341dd7b2b..bfb0900752 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; @@ -216,7 +232,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_); @@ -272,14 +289,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_); } @@ -287,7 +305,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\": [" " {" @@ -322,7 +340,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_); @@ -334,6 +353,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; @@ -374,7 +394,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. @@ -384,7 +404,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. @@ -511,6 +531,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());