[#3694] addressed review comments

diff --git a/ChangeLog b/ChangeLog
index 762079570b4e136eafdaea3a6c2489e1f014b9f4..10af667ca5d2489ab004c6a18efec02eb369ad7b 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2324.  [func]          razvan
+       It is not necessary to restart the server to apply changes in the
+       TLS configuration. Running the "config-reload" command is
+       sufficient.
+       (Gitlab #3694)
+
 Kea 2.7.6 (development) released on January 29, 2025
 
 2323.  [func]*         fdupont

diff --git a/src/bin/agent/ca_messages.cc b/src/bin/agent/ca_messages.cc
index 7c511012bd3ef316b846bf5fe1a86543ba615010..b8cb1ec6afde8153ca7d5d13183ea8f68d80ee0e 100644 (file)
--- a/src/bin/agent/ca_messages.cc
+++ b/src/bin/agent/ca_messages.cc
@@ -17,6 +17,7 @@ extern const isc::log::MessageID CTRL_AGENT_CONFIG_SYNTAX_WARNING = "CTRL_AGENT_
 extern const isc::log::MessageID CTRL_AGENT_FAILED = "CTRL_AGENT_FAILED";
 extern const isc::log::MessageID CTRL_AGENT_HTTPS_SERVICE_REUSED = "CTRL_AGENT_HTTPS_SERVICE_REUSED";
 extern const isc::log::MessageID CTRL_AGENT_HTTPS_SERVICE_STARTED = "CTRL_AGENT_HTTPS_SERVICE_STARTED";
+extern const isc::log::MessageID CTRL_AGENT_HTTPS_SERVICE_UPDATED = "CTRL_AGENT_HTTPS_SERVICE_UPDATED";
 extern const isc::log::MessageID CTRL_AGENT_HTTP_SERVICE_REUSED = "CTRL_AGENT_HTTP_SERVICE_REUSED";
 extern const isc::log::MessageID CTRL_AGENT_HTTP_SERVICE_STARTED = "CTRL_AGENT_HTTP_SERVICE_STARTED";
 extern const isc::log::MessageID CTRL_AGENT_RUN_EXIT = "CTRL_AGENT_RUN_EXIT";
@@ -38,6 +39,7 @@ const char* values[] = {
     "CTRL_AGENT_FAILED", "application experienced a fatal error: %1",
     "CTRL_AGENT_HTTPS_SERVICE_REUSED", "reused HTTPS service bound to address %1:%2",
     "CTRL_AGENT_HTTPS_SERVICE_STARTED", "HTTPS service bound to address %1:%2",
+    "CTRL_AGENT_HTTPS_SERVICE_UPDATED", "reused HTTPS service bound to address %1:%2 and updated TLS settings",
     "CTRL_AGENT_HTTP_SERVICE_REUSED", "reused HTTP service bound to address %1:%2",
     "CTRL_AGENT_HTTP_SERVICE_STARTED", "HTTP service bound to address %1:%2",
     "CTRL_AGENT_RUN_EXIT", "application is exiting the event loop",

diff --git a/src/bin/agent/ca_messages.h b/src/bin/agent/ca_messages.h
index 59ad7728076df438340b7da5eefa60ec862d366c..24d97b3f61685d6a8f60c1322de0a64edc6f2eda 100644 (file)
--- a/src/bin/agent/ca_messages.h
+++ b/src/bin/agent/ca_messages.h
@@ -18,6 +18,7 @@ extern const isc::log::MessageID CTRL_AGENT_CONFIG_SYNTAX_WARNING;
 extern const isc::log::MessageID CTRL_AGENT_FAILED;
 extern const isc::log::MessageID CTRL_AGENT_HTTPS_SERVICE_REUSED;
 extern const isc::log::MessageID CTRL_AGENT_HTTPS_SERVICE_STARTED;
+extern const isc::log::MessageID CTRL_AGENT_HTTPS_SERVICE_UPDATED;
 extern const isc::log::MessageID CTRL_AGENT_HTTP_SERVICE_REUSED;
 extern const isc::log::MessageID CTRL_AGENT_HTTP_SERVICE_STARTED;
 extern const isc::log::MessageID CTRL_AGENT_RUN_EXIT;

diff --git a/src/bin/agent/ca_messages.mes b/src/bin/agent/ca_messages.mes
index fce41e1b6b30a171f034f9a862d0df80b69d73c4..f2a73ad6e4fe2406b68eaa8f9e82e5ab2a5aa4a5 100644 (file)
--- a/src/bin/agent/ca_messages.mes
+++ b/src/bin/agent/ca_messages.mes
@@ -53,6 +53,11 @@ This informational message indicates that the server has started HTTPS service
 on the specified address and port. All control commands should be sent to this
 address and port over a TLS channel.
 
+% CTRL_AGENT_HTTPS_SERVICE_UPDATED reused HTTPS service bound to address %1:%2 and updated TLS settings
+This informational message indicates that the server has reused existing
+HTTPS service on the specified address and port. Note that any change in
+the TLS setup has been applied.
+
 % CTRL_AGENT_HTTP_SERVICE_REUSED reused HTTP service bound to address %1:%2
 This informational message indicates that the server has reused existing
 HTTP service on the specified address and port.

diff --git a/src/bin/agent/ca_process.cc b/src/bin/agent/ca_process.cc
index 7ee3e142fba483f9f479722bdccec16374f6db52..319bb67dbb5404ae914ce0d8591d593be841a057 100644 (file)
--- a/src/bin/agent/ca_process.cc
+++ b/src/bin/agent/ca_process.cc
@@ -145,7 +145,7 @@ CtrlAgentProcess::configure(isc::data::ConstElementPtr config_set,
                 if (listener->getTlsContext()) {
                     if (ctx->getTrustAnchor().empty()) {
                         // Can not switch from HTTPS to HTTP
-                        LOG_INFO(agent_logger, CTRL_AGENT_HTTPS_SERVICE_REUSED)
+                        LOG_ERROR(agent_logger, CTRL_AGENT_HTTPS_SERVICE_REUSED)
                             .arg(server_address.toText())
                             .arg(server_port);
                     } else {
@@ -161,10 +161,13 @@ CtrlAgentProcess::configure(isc::data::ConstElementPtr config_set,
                         it->second->config_->setAuthConfig(ctx->getAuthConfig());
                         it->second->config_->setHttpHeaders(ctx->getHttpHeaders());
                         getIOService()->post([listener, tls_context]() { listener->setTlsContext(tls_context); });
+                        LOG_INFO(agent_logger, CTRL_AGENT_HTTPS_SERVICE_UPDATED)
+                            .arg(server_address.toText())
+                            .arg(server_port);
                     }
                 } else if (!ctx->getTrustAnchor().empty()) {
                     // Can not switch from HTTP to HTTPS
-                    LOG_INFO(agent_logger, CTRL_AGENT_HTTP_SERVICE_REUSED)
+                    LOG_ERROR(agent_logger, CTRL_AGENT_HTTP_SERVICE_REUSED)
                         .arg(server_address.toText())
                         .arg(server_port);
                 }
@@ -270,7 +273,7 @@ CtrlAgentProcess::closeCommandSockets() {
     // We have stopped listeners but there may be some pending handlers
     // related to these listeners. Need to invoke these handlers.
     try {
-        getIOService()->pollOne();
+        getIOService()->poll();
     } catch (...) {
     }
 }

diff --git a/src/lib/config/config_messages.cc b/src/lib/config/config_messages.cc
index 0e9c07473f8cf38c22fb6d553b35b9e039bc35f9..ffb64212c8ffbff6b0780895386b588b27c6acc8 100644 (file)
--- a/src/lib/config/config_messages.cc
+++ b/src/lib/config/config_messages.cc
@@ -35,11 +35,10 @@ extern const isc::log::MessageID COMMAND_WATCH_SOCKET_CLEAR_ERROR = "COMMAND_WAT
 extern const isc::log::MessageID COMMAND_WATCH_SOCKET_CLOSE_ERROR = "COMMAND_WATCH_SOCKET_CLOSE_ERROR";
 extern const isc::log::MessageID COMMAND_WATCH_SOCKET_MARK_READY_ERROR = "COMMAND_WATCH_SOCKET_MARK_READY_ERROR";
 extern const isc::log::MessageID HTTP_COMMAND_MGR_HTTPS_SERVICE_REUSED = "HTTP_COMMAND_MGR_HTTPS_SERVICE_REUSED";
+extern const isc::log::MessageID HTTP_COMMAND_MGR_HTTPS_SERVICE_UPDATED = "HTTP_COMMAND_MGR_HTTPS_SERVICE_UPDATED";
 extern const isc::log::MessageID HTTP_COMMAND_MGR_HTTP_SERVICE_REUSED = "HTTP_COMMAND_MGR_HTTP_SERVICE_REUSED";
 extern const isc::log::MessageID HTTP_COMMAND_MGR_SERVICE_STARTED = "HTTP_COMMAND_MGR_SERVICE_STARTED";
 extern const isc::log::MessageID HTTP_COMMAND_MGR_SERVICE_STOPPING = "HTTP_COMMAND_MGR_SERVICE_STOPPING";
-extern const isc::log::MessageID HTTP_COMMAND_MGR_SERVICE_STOPPING_ALL = "HTTP_COMMAND_MGR_SERVICE_STOPPING_ALL";
-extern const isc::log::MessageID HTTP_COMMAND_MGR_SERVICE_STOPPING_NO_DATA = "HTTP_COMMAND_MGR_SERVICE_STOPPING_NO_DATA";
 
 } // namespace config
 } // namespace isc
@@ -75,11 +74,10 @@ const char* values[] = {
     "COMMAND_WATCH_SOCKET_CLOSE_ERROR", "watch socket failed to close: %1",
     "COMMAND_WATCH_SOCKET_MARK_READY_ERROR", "watch socket failed to mark ready: %1",
     "HTTP_COMMAND_MGR_HTTPS_SERVICE_REUSED", "reused HTTPS service bound to address %1:%2",
+    "HTTP_COMMAND_MGR_HTTPS_SERVICE_UPDATED", "reused HTTPS service bound to address %1:%2 and updated TLS settings",
     "HTTP_COMMAND_MGR_HTTP_SERVICE_REUSED", "reused HTTP service bound to address %1:%2",
     "HTTP_COMMAND_MGR_SERVICE_STARTED", "started %1 service bound to address %2 port %3",
     "HTTP_COMMAND_MGR_SERVICE_STOPPING", "Server is stopping %1 service %2",
-    "HTTP_COMMAND_MGR_SERVICE_STOPPING_ALL", "stopping %1 service %2",
-    "HTTP_COMMAND_MGR_SERVICE_STOPPING_NO_DATA", "Server is stopping all services including %1 service %2",
     NULL
 };
 

diff --git a/src/lib/config/config_messages.h b/src/lib/config/config_messages.h
index fd55744e49dc1d650e58c730164c8dded2bc3b32..7c05eb95fee6ac949999a689e8d7a9d7254beba9 100644 (file)
--- a/src/lib/config/config_messages.h
+++ b/src/lib/config/config_messages.h
@@ -36,11 +36,10 @@ extern const isc::log::MessageID COMMAND_WATCH_SOCKET_CLEAR_ERROR;
 extern const isc::log::MessageID COMMAND_WATCH_SOCKET_CLOSE_ERROR;
 extern const isc::log::MessageID COMMAND_WATCH_SOCKET_MARK_READY_ERROR;
 extern const isc::log::MessageID HTTP_COMMAND_MGR_HTTPS_SERVICE_REUSED;
+extern const isc::log::MessageID HTTP_COMMAND_MGR_HTTPS_SERVICE_UPDATED;
 extern const isc::log::MessageID HTTP_COMMAND_MGR_HTTP_SERVICE_REUSED;
 extern const isc::log::MessageID HTTP_COMMAND_MGR_SERVICE_STARTED;
 extern const isc::log::MessageID HTTP_COMMAND_MGR_SERVICE_STOPPING;
-extern const isc::log::MessageID HTTP_COMMAND_MGR_SERVICE_STOPPING_ALL;
-extern const isc::log::MessageID HTTP_COMMAND_MGR_SERVICE_STOPPING_NO_DATA;
 
 } // namespace config
 } // namespace isc

diff --git a/src/lib/config/config_messages.mes b/src/lib/config/config_messages.mes
index 7c4fd50dc3e4f1a20211c2ef08ee8d1c95c28099..d2df251dee7f46ba5d821faef6ee0c8c32d702fb 100644 (file)
--- a/src/lib/config/config_messages.mes
+++ b/src/lib/config/config_messages.mes
@@ -158,6 +158,11 @@ This informational message indicates that the server has reused existing
 HTTPS service on the specified address and port. Note that any change in
 the TLS setup was ignored.
 
+% HTTP_COMMAND_MGR_HTTPS_SERVICE_UPDATED reused HTTPS service bound to address %1:%2 and updated TLS settings
+This informational message indicates that the server has reused existing
+HTTPS service on the specified address and port. Note that any change in
+the TLS setup has been applied.
+
 % HTTP_COMMAND_MGR_HTTP_SERVICE_REUSED reused HTTP service bound to address %1:%2
 This informational message indicates that the server has reused existing
 HTTP service on the specified address and port.
@@ -170,12 +175,3 @@ control commands.
 % HTTP_COMMAND_MGR_SERVICE_STOPPING Server is stopping %1 service %2
 This informational message indicates that the server has stopped
 HTTP/HTTPS service. When known the address and port are displayed.
-
-% HTTP_COMMAND_MGR_SERVICE_STOPPING_ALL stopping %1 service %2
-This informational message indicates that the server has stopped
-HTTP/HTTPS service. When known the address and port are displayed.
-
-% HTTP_COMMAND_MGR_SERVICE_STOPPING_NO_DATA Server is stopping all services including %1 service %2
-This informational message indicates that the server is stopping all
-HTTP/HTTPS services. When known the address and port are displayed for
-each service.

diff --git a/src/lib/config/http_command_mgr.cc b/src/lib/config/http_command_mgr.cc
index 0725d7821e498d3f34d33ef87453646b0ff0b74f..d2b8f5bbd7c29c7c9647d8f1d7377d12aebc3d56 100644 (file)
--- a/src/lib/config/http_command_mgr.cc
+++ b/src/lib/config/http_command_mgr.cc
@@ -58,7 +58,9 @@ public:
     void closeCommandSocket(HttpSocketInfoPtr info, bool remove);
 
     /// @brief Close control socket.
-    void closeCommandSockets();
+    ///
+    /// @param remove When true remove the listeners immediately.
+    void closeCommandSockets(bool remove = true);
 
     /// @brief Returns a const pointer to the HTTP listener.
     ///
@@ -133,7 +135,7 @@ HttpCommandMgrImpl::openCommandSocket(const isc::data::ConstElementPtr config) {
             if (listener->getTlsContext()) {
                 if (cmd_config->getTrustAnchor().empty()) {
                     // Can not switch from HTTPS to HTTP
-                    LOG_INFO(command_logger, HTTP_COMMAND_MGR_HTTPS_SERVICE_REUSED)
+                    LOG_ERROR(command_logger, HTTP_COMMAND_MGR_HTTPS_SERVICE_REUSED)
                         .arg(server_address.toText())
                         .arg(server_port);
                 } else {
@@ -151,10 +153,13 @@ HttpCommandMgrImpl::openCommandSocket(const isc::data::ConstElementPtr config) {
                     it->second->config_->setHttpHeaders(cmd_config->getHttpHeaders());
                     it->second->config_->setEmulateAgentResponse(cmd_config->getEmulateAgentResponse());
                     io_service_->post([listener, tls_context]() { listener->setTlsContext(tls_context); });
+                    LOG_INFO(command_logger, HTTP_COMMAND_MGR_HTTPS_SERVICE_UPDATED)
+                        .arg(server_address.toText())
+                        .arg(server_port);
                 }
             } else if (!cmd_config->getTrustAnchor().empty()) {
                 // Can not switch from HTTP to HTTPS
-                LOG_INFO(command_logger, HTTP_COMMAND_MGR_HTTP_SERVICE_REUSED)
+                LOG_ERROR(command_logger, HTTP_COMMAND_MGR_HTTP_SERVICE_REUSED)
                     .arg(server_address.toText())
                     .arg(server_port);
             }
@@ -233,50 +238,22 @@ HttpCommandMgrImpl::closeCommandSocket(HttpSocketInfoPtr info, bool remove) {
                 sockets_.erase(it);
             }
         }
-    } else {
-        for (auto const& data : sockets_) {
-            ostringstream ep;
-            use_https = !data.second->config_->getCertFile().empty();
-            ep << "bound to address " << data.second->config_->getSocketAddress()
-               << " port " << data.second->config_->getSocketPort();
-
-            LOG_INFO(command_logger, HTTP_COMMAND_MGR_SERVICE_STOPPING_NO_DATA)
-                .arg(use_https ? "HTTPS" : "HTTP")
-                .arg(ep.str());
-            data.second->listener_->stop();
+        // We have stopped listeners but there may be some pending handlers
+        // related to these listeners. Need to invoke these handlers.
+        try {
+            io_service_->pollOne();
+        } catch (...) {
         }
-        if (remove) {
-            sockets_.clear();
-        }
-    }
-    // We have stopped listeners but there may be some pending handlers
-    // related to these listeners. Need to invoke these handlers.
-    try {
-        io_service_->pollOne();
-    } catch (...) {
+    } else {
+        closeCommandSockets(remove);
     }
 }
 
 void
-HttpCommandMgrImpl::closeCommandSockets() {
-    bool use_https = false;
-    for (auto const& data : sockets_) {
-        ostringstream ep;
-        use_https = !data.second->config_->getCertFile().empty();
-        ep << "bound to address " << data.second->config_->getSocketAddress()
-           << " port " << data.second->config_->getSocketPort();
-
-        LOG_INFO(command_logger, HTTP_COMMAND_MGR_SERVICE_STOPPING_ALL)
-            .arg(use_https ? "HTTPS" : "HTTP")
-            .arg(ep.str());
-        data.second->listener_->stop();
-    }
-    sockets_.clear();
-    // We have stopped listeners but there may be some pending handlers
-    // related to these listeners. Need to invoke these handlers.
-    try {
-        io_service_->pollOne();
-    } catch (...) {
+HttpCommandMgrImpl::closeCommandSockets(bool remove) {
+    auto copy = sockets_;
+    for (auto const& data : copy) {
+        closeCommandSocket(data.second, remove);
     }
 }
 

diff --git a/src/lib/config/http_command_mgr.h b/src/lib/config/http_command_mgr.h
index 1d937d306994514e81625522139f40ab426309ea..2c48c7ab81de42794c595b50686dccad9581facd 100644 (file)
--- a/src/lib/config/http_command_mgr.h
+++ b/src/lib/config/http_command_mgr.h
@@ -64,11 +64,17 @@ public:
     /// Creates http/https listener, or reuses the existing one reapplying
     /// changes.
     ///
+    /// @note This function in used internally by @ref openCommandSockets and it
+    /// should not be used directly, except for unittests.
+    ///
     /// @param config Configuration information for the http control socket.
     void openCommandSocket(const isc::data::ConstElementPtr config);
 
     /// @brief Close http control socket.
     ///
+    /// @note This function in used internally by @ref closeCommandSockets and it
+    /// should not be used directly, except for unittests.
+    ///
     /// @param info Configuration information for the http control socket.
     /// @param remove When true remove the listeners immediately.
     void closeCommandSocket(HttpSocketInfoPtr info = HttpSocketInfoPtr(), bool remove = true);

diff --git a/src/lib/config/unix_command_mgr.cc b/src/lib/config/unix_command_mgr.cc
index 4ded7b0d81af8242866dafc8498b7eeff8b781d5..62a3601cf75e485a58df219459e2515fa3de870f 100644 (file)
--- a/src/lib/config/unix_command_mgr.cc
+++ b/src/lib/config/unix_command_mgr.cc
@@ -523,7 +523,9 @@ public:
     void closeCommandSocket(UnixSocketInfoPtr info);
 
     /// @brief Shuts down any open unix control sockets
-    void closeCommandSockets();
+    ///
+    /// @param remove When true remove the listeners immediately.
+    void closeCommandSockets(bool remove = true);
 
     /// @brief Asynchronously accepts next connection.
     ///
@@ -679,61 +681,24 @@ UnixCommandMgrImpl::closeCommandSocket(UnixSocketInfoPtr info) {
         if (it != sockets_.end()) {
             sockets_.erase(it);
         }
-    } else {
-        for (auto const& data : sockets_) {
-            if (data.second->acceptor_ && data.second->acceptor_->isOpen()) {
-                if (use_external_) {
-                    IfaceMgr::instance().deleteExternalSocket(data.second->acceptor_->getNative());
-                }
-                data.second->acceptor_->close();
-                static_cast<void>(::remove(data.second->config_->getSocketName().c_str()));
-                static_cast<void>(::remove(data.second->config_->getLockName().c_str()));
-            }
-
-            // Stop all connections which can be closed. The only connection that won't
-            // be closed is the one over which we have received a request to reconfigure
-            // the server. This connection will be held until the UnixCommandMgr
-            // responds to such request.
-            connection_pool_.stopAll();
-            if (data.second->lock_fd_ != -1) {
-                close(data.second->lock_fd_);
-                data.second->lock_fd_ = -1;
-            }
+        try {
+            io_service_->pollOne();
+        } catch (...) {
         }
-    }
-    try {
-        io_service_->pollOne();
-    } catch (...) {
+    } else {
+        closeCommandSockets(false);
     }
 }
 
 void
-UnixCommandMgrImpl::closeCommandSockets() {
-    for (auto const& data : sockets_) {
-        if (data.second->acceptor_ && data.second->acceptor_->isOpen()) {
-            if (use_external_) {
-                IfaceMgr::instance().deleteExternalSocket(data.second->acceptor_->getNative());
-            }
-            data.second->acceptor_->close();
-            static_cast<void>(::remove(data.second->config_->getSocketName().c_str()));
-            static_cast<void>(::remove(data.second->config_->getLockName().c_str()));
-        }
-
-        // Stop all connections which can be closed. The only connection that won't
-        // be closed is the one over which we have received a request to reconfigure
-        // the server. This connection will be held until the UnixCommandMgr
-        // responds to such request.
-        connection_pool_.stopAll();
-        if (data.second->lock_fd_ != -1) {
-            close(data.second->lock_fd_);
-            data.second->lock_fd_ = -1;
-        }
+UnixCommandMgrImpl::closeCommandSockets(bool remove) {
+    auto copy = sockets_;
+    for (auto const& data : copy) {
+        closeCommandSocket(data.second);
     }
-    try {
-        io_service_->pollOne();
-    } catch (...) {
+    if (remove) {
+        sockets_.clear();
     }
-    sockets_.clear();
 }
 
 void

diff --git a/src/lib/config/unix_command_mgr.h b/src/lib/config/unix_command_mgr.h
index ed73a91ae6da6e35781d4bb1f5dceedf2e2c936d..9a328df8ee9c73670549a5346b0b5aae529a568a 100644 (file)
--- a/src/lib/config/unix_command_mgr.h
+++ b/src/lib/config/unix_command_mgr.h
@@ -73,6 +73,9 @@ public:
     ///
     /// Creates acceptor, or reuses the existing one.
     ///
+    /// @note This function in used internally by @ref openCommandSockets and it
+    /// should not be used directly, except for unittests.
+    ///
     /// @throw BadSocketInfo When socket configuration is invalid.
     /// @throw SocketError When socket operation fails.
     ///
@@ -81,6 +84,9 @@ public:
 
     /// @brief Shuts down any open unix control sockets.
     ///
+    /// @note This function in used internally by @ref closeCommandSockets and it
+    /// should not be used directly, except for unittests.
+    ///
     /// @param config Configuration information for the unix control socket.
     void closeCommandSocket(UnixSocketInfoPtr info = UnixSocketInfoPtr());