[#3569] addressed review comments

diff --git a/src/hooks/dhcp/forensic_log/legal_log_messages.cc b/src/hooks/dhcp/forensic_log/legal_log_messages.cc
index b75505eba5b322362d25f75b88b9702b7b545c05..5a79dfe4d0a5407479846d77a67b9d841f193356 100644 (file)
--- a/src/hooks/dhcp/forensic_log/legal_log_messages.cc
+++ b/src/hooks/dhcp/forensic_log/legal_log_messages.cc
@@ -16,9 +16,9 @@ extern const isc::log::MessageID LEGAL_LOG_STORE_CLOSED = "LEGAL_LOG_STORE_CLOSE
 extern const isc::log::MessageID LEGAL_LOG_STORE_CLOSE_ERROR = "LEGAL_LOG_STORE_CLOSE_ERROR";
 extern const isc::log::MessageID LEGAL_LOG_STORE_OPEN = "LEGAL_LOG_STORE_OPEN";
 extern const isc::log::MessageID LEGAL_LOG_STORE_OPENED = "LEGAL_LOG_STORE_OPENED";
+extern const isc::log::MessageID LEGAL_LOG_SYSLOG = "LEGAL_LOG_SYSLOG";
+extern const isc::log::MessageID LEGAL_LOG_SYSLOG_STORE_OPEN = "LEGAL_LOG_SYSLOG_STORE_OPEN";
 extern const isc::log::MessageID LEGAL_LOG_UNLOAD_ERROR = "LEGAL_LOG_UNLOAD_ERROR";
-extern const isc::log::MessageID LEGAL_SYSLOG_LOG = "LEGAL_SYSLOG_LOG";
-extern const isc::log::MessageID LEGAL_SYSLOG_STORE_OPEN = "LEGAL_SYSLOG_STORE_OPEN";
 
 namespace {
 
@@ -35,9 +35,9 @@ const char* values[] = {
     "LEGAL_LOG_STORE_CLOSE_ERROR", "An error occurred closing the store: %1, error: %2",
     "LEGAL_LOG_STORE_OPEN", "opening Legal Log file: %1",
     "LEGAL_LOG_STORE_OPENED", "Legal store opened: %1",
+    "LEGAL_LOG_SYSLOG", "%1",
+    "LEGAL_LOG_SYSLOG_STORE_OPEN", "opening Legal Syslog: %1",
     "LEGAL_LOG_UNLOAD_ERROR", "An error occurred unloading the library: %1",
-    "LEGAL_SYSLOG_LOG", "%1",
-    "LEGAL_SYSLOG_STORE_OPEN", "opening Legal Syslog: %1",
     NULL
 };
 

diff --git a/src/hooks/dhcp/forensic_log/legal_log_messages.h b/src/hooks/dhcp/forensic_log/legal_log_messages.h
index ca50d8e1b17d44f2918b26e12a68f38a13b21cbb..530a70b057c317352215f3512c330edb33708788 100644 (file)
--- a/src/hooks/dhcp/forensic_log/legal_log_messages.h
+++ b/src/hooks/dhcp/forensic_log/legal_log_messages.h
@@ -17,8 +17,8 @@ extern const isc::log::MessageID LEGAL_LOG_STORE_CLOSED;
 extern const isc::log::MessageID LEGAL_LOG_STORE_CLOSE_ERROR;
 extern const isc::log::MessageID LEGAL_LOG_STORE_OPEN;
 extern const isc::log::MessageID LEGAL_LOG_STORE_OPENED;
+extern const isc::log::MessageID LEGAL_LOG_SYSLOG;
+extern const isc::log::MessageID LEGAL_LOG_SYSLOG_STORE_OPEN;
 extern const isc::log::MessageID LEGAL_LOG_UNLOAD_ERROR;
-extern const isc::log::MessageID LEGAL_SYSLOG_LOG;
-extern const isc::log::MessageID LEGAL_SYSLOG_STORE_OPEN;
 
 #endif // LEGAL_LOG_MESSAGES_H

diff --git a/src/hooks/dhcp/forensic_log/legal_log_messages.mes b/src/hooks/dhcp/forensic_log/legal_log_messages.mes
index 7d6320f88711f4bd33007ff7630880448cb66218..0ae35bd963d408c5264c5d16b7a3b8f2be3daaf5 100644 (file)
--- a/src/hooks/dhcp/forensic_log/legal_log_messages.mes
+++ b/src/hooks/dhcp/forensic_log/legal_log_messages.mes
@@ -62,12 +62,12 @@ This informational message is logged when a DHCP server (either V4 or
 V6) is about to open a legal log file. The parameters of
 the backend are logged.
 
-% LEGAL_SYSLOG_STORE_OPEN opening Legal Syslog: %1
+% LEGAL_LOG_SYSLOG_STORE_OPEN opening Legal Syslog: %1
 This informational message is logged when a DHCP server (either V4 or
 V6) is about to open a legal syslog store. The parameters of
 the backend are logged.
 
-% LEGAL_SYSLOG_LOG %1
+% LEGAL_LOG_SYSLOG %1
 This informational message contains the message being logged to syslog.
 
 % LEGAL_LOG_STORE_OPENED Legal store opened: %1

diff --git a/src/hooks/dhcp/forensic_log/legal_syslog.cc b/src/hooks/dhcp/forensic_log/legal_syslog.cc
index 16efa17f02e3d47323fb4b66a20efaa904e29b2a..85d80db478d426a46333eabe14e6d1a4039688d6 100644 (file)
--- a/src/hooks/dhcp/forensic_log/legal_syslog.cc
+++ b/src/hooks/dhcp/forensic_log/legal_syslog.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2016-2025 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2025 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -29,7 +29,7 @@ LegalSyslog::LegalSyslog(const DatabaseConnection::ParameterMap& parameters)
     LoggingInfo info;
     // Remove default destinations as we are going to replace them.
     info.clearDestinations();
-    info.name_ = "legal-log-syslog-";
+    info.name_ = "legal-log-";
     info.name_ += boost::lexical_cast<std::string>(reinterpret_cast<uint64_t>(this));
     logger_.reset(new Logger(info.name_.c_str()));
     LoggingDestination dest;
@@ -56,7 +56,7 @@ LegalSyslog::close() {
 
 void
 LegalSyslog::writeln(const string& text, const string&) {
-    LOG_INFO(*logger_, LEGAL_SYSLOG_LOG)
+    LOG_INFO(*logger_, LEGAL_LOG_SYSLOG)
         .arg(text);
 }
 
@@ -67,7 +67,7 @@ LegalSyslog::getType() const {
 
 LegalLogMgrPtr
 LegalSyslog::factory(const DatabaseConnection::ParameterMap& parameters) {
-    LOG_INFO(legal_log_logger, LEGAL_SYSLOG_STORE_OPEN)
+    LOG_INFO(legal_log_logger, LEGAL_LOG_SYSLOG_STORE_OPEN)
         .arg(DatabaseConnection::redactedAccessString(parameters));
     return (LegalLogMgrPtr(new LegalSyslog(parameters)));
 }

diff --git a/src/hooks/dhcp/forensic_log/legal_syslog.h b/src/hooks/dhcp/forensic_log/legal_syslog.h
index 781619c1828514b4b557b685acdc0683e58c9574..f72250dff8e6853e4f54a348840d311a7fce8f4b 100644 (file)
--- a/src/hooks/dhcp/forensic_log/legal_syslog.h
+++ b/src/hooks/dhcp/forensic_log/legal_syslog.h
@@ -55,7 +55,7 @@ public:
     /// @param parameters A data structure relating keywords and values
     ///        concerned with the database.
     ///
-    /// @return The Rotating File Store Backend.
+    /// @return The Syslog File Store Backend.
     static isc::dhcp::LegalLogMgrPtr
     factory(const isc::db::DatabaseConnection::ParameterMap& parameters);
 };

diff --git a/src/hooks/dhcp/forensic_log/tests/legal_log_mgr_unittests.cc b/src/hooks/dhcp/forensic_log/tests/legal_log_mgr_unittests.cc
index d91768709e2078d12275e3cef963e06b7c504a18..eb61a236113d27231db02fbbbf3d1a3a34c1738d 100644 (file)
--- a/src/hooks/dhcp/forensic_log/tests/legal_log_mgr_unittests.cc
+++ b/src/hooks/dhcp/forensic_log/tests/legal_log_mgr_unittests.cc
@@ -238,4 +238,9 @@ TEST_F(LegalLogMgrTest, wrongDatabaseType) {
                      "The type of the forensic log backend: 'awesomesql' is not supported");
 }
 
+TEST_F(LegalLogMgrTest, syslogNoParameters) {
+    db::DatabaseConnection::ParameterMap map;
+    EXPECT_THROW(LegalLogMgr::parseSyslog(ConstElementPtr(), map), BadValue);
+}
+
 } // end of anonymous namespace

diff --git a/src/lib/dhcpsrv/legal_log_mgr.cc b/src/lib/dhcpsrv/legal_log_mgr.cc
index e6905a64681ee46dc7679d24a88196a01f96baa5..263c4c7578335cbd9306f65febf759d14b2dbb76 100644 (file)
--- a/src/lib/dhcpsrv/legal_log_mgr.cc
+++ b/src/lib/dhcpsrv/legal_log_mgr.cc
@@ -68,7 +68,8 @@ LegalLogMgr::parseDatabase(const ConstElementPtr& parameters, DatabaseConnection
 
     // uint32_t
     for (char const* const& key : {
-         "connect-timeout", "reconnect-wait-time", "max-reconnect-tries"}) {
+         "connect-timeout", "reconnect-wait-time", "max-reconnect-tries",
+         "read-timeout", "write-timeout", "tcp-user-timeout"}) {
         ConstElementPtr const value(parameters->get(key));
         if (value) {
             int64_t integer_value(value->intValue());

diff --git a/src/lib/dhcpsrv/legal_log_mgr.h b/src/lib/dhcpsrv/legal_log_mgr.h
index d2c5bd654f97db2edc2b2af564f456a8ab7204e5..4b76cb2a7828b9c9ebf8ca4a36194c49e8bf7382 100644 (file)
--- a/src/lib/dhcpsrv/legal_log_mgr.h
+++ b/src/lib/dhcpsrv/legal_log_mgr.h
@@ -57,6 +57,41 @@ public:
     /// Parse the configuration and check that the various keywords are
     /// consistent.
     ///
+    /// @note The supported parameters are:
+    /// - common parameters:
+    ///       - type - one of: logfile (default), syslog, mysql, postgresql
+    ///       - request-parser-format
+    ///       - response-parser-format
+    ///       - timestamp-format
+    /// - logfile parameters:
+    ///       - path
+    ///       - base-name
+    ///       - time-unit - one of: second, day, month, year
+    ///       - prerotate
+    ///       - postrotate
+    ///       - count
+    /// - syslog parameters:
+    ///       - pattern
+    ///       - facility
+    /// - database parameters:
+    ///       - name
+    ///       - host
+    ///       - password
+    ///       - port
+    ///       - user
+    ///       - trust-anchor (MySQL only)
+    ///       - cert-file (MySQL only)
+    ///       - key-file (MySQL only)
+    ///       - cipher-list (MySQL only)
+    ///       - reconnect-wait-time
+    ///       - max-reconnect-tries
+    ///       - on-fail
+    ///       - retry-on-startup
+    ///       - connect-timeout
+    ///       - read-timeout (MySQL only)
+    ///       - write-timeout (MySQL only)
+    ///       - tcp-user-timeout (PostgreSQL only)
+    ///
     /// @param parameters The library parameters.
     /// @param map The parameter map used by LegalLogMgr objects.
     static void parseConfig(const isc::data::ConstElementPtr& parameters, isc::db::DatabaseConnection::ParameterMap& map);
@@ -68,6 +103,26 @@ public:
     /// Parse the configuration and check that the various keywords are
     /// consistent.
     ///
+    /// @note The supported parameters are:
+    /// - database parameters:
+    ///       - name
+    ///       - host
+    ///       - password
+    ///       - port
+    ///       - user
+    ///       - trust-anchor (MySQL only)
+    ///       - cert-file (MySQL only)
+    ///       - key-file (MySQL only)
+    ///       - cipher-list (MySQL only)
+    ///       - reconnect-wait-time
+    ///       - max-reconnect-tries
+    ///       - on-fail
+    ///       - retry-on-startup
+    ///       - connect-timeout
+    ///       - read-timeout (MySQL only)
+    ///       - write-timeout (MySQL only)
+    ///       - tcp-user-timeout (PostgreSQL only)
+    ///
     /// @param parameters The library parameters.
     /// @param map The parameter map used by LegalLogMgr objects.
     static void parseDatabase(const isc::data::ConstElementPtr& parameters, isc::db::DatabaseConnection::ParameterMap& map);
@@ -77,6 +132,15 @@ public:
     /// Parse the configuration and check that the various keywords are
     /// consistent.
     ///
+    /// @note The supported parameters are:
+    /// - logfile parameters:
+    ///       - path
+    ///       - base-name
+    ///       - time-unit - one of: second, day, month, year
+    ///       - prerotate
+    ///       - postrotate
+    ///       - count
+    ///
     /// @param parameters The library parameters.
     /// @param [out] map The parameter map.
     static void parseFile(const isc::data::ConstElementPtr& parameters, isc::db::DatabaseConnection::ParameterMap& map);
@@ -86,6 +150,11 @@ public:
     /// Parse the configuration and check that the various keywords are
     /// consistent.
     ///
+    /// @note The supported parameters are:
+    /// - syslog parameters:
+    ///       - pattern
+    ///       - facility
+    ///
     /// @param parameters The library parameters.
     /// @param [out] map The parameter map.
     static void parseSyslog(const isc::data::ConstElementPtr& parameters, isc::db::DatabaseConnection::ParameterMap& map);
@@ -93,6 +162,12 @@ public:
     /// @brief Parse extra parameters which are not related to backend
     /// connection.
     ///
+    /// @note The supported parameters are:
+    /// - common parameters:
+    ///       - request-parser-format
+    ///       - response-parser-format
+    ///       - timestamp-format
+    ///
     /// @param parameters The library parameters.
     /// @param [out] map The parameter map.
     static void parseExtraParameters(const isc::data::ConstElementPtr& parameters, isc::db::DatabaseConnection::ParameterMap& map);