Skip to content

Commit de694a0

Browse files
author
Razvan Becheriu
committed
[#3694] addressed review comments
1 parent 1334b83 commit de694a0

12 files changed

+73
-109
lines changed

‎ChangeLog

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
2324. [func] razvan
2+
It is not necessary to restart the server to apply changes in the
3+
TLS configuration. Running the "config-reload" command is
4+
sufficient.
5+
(Gitlab #3694)
6+
17
Kea 2.7.6 (development) released on January 29, 2025
28

39
2323. [func]* fdupont

‎src/bin/agent/ca_messages.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ extern const isc::log::MessageID CTRL_AGENT_CONFIG_SYNTAX_WARNING = "CTRL_AGENT_
1717
extern const isc::log::MessageID CTRL_AGENT_FAILED = "CTRL_AGENT_FAILED";
1818
extern const isc::log::MessageID CTRL_AGENT_HTTPS_SERVICE_REUSED = "CTRL_AGENT_HTTPS_SERVICE_REUSED";
1919
extern const isc::log::MessageID CTRL_AGENT_HTTPS_SERVICE_STARTED = "CTRL_AGENT_HTTPS_SERVICE_STARTED";
20+
extern const isc::log::MessageID CTRL_AGENT_HTTPS_SERVICE_UPDATED = "CTRL_AGENT_HTTPS_SERVICE_UPDATED";
2021
extern const isc::log::MessageID CTRL_AGENT_HTTP_SERVICE_REUSED = "CTRL_AGENT_HTTP_SERVICE_REUSED";
2122
extern const isc::log::MessageID CTRL_AGENT_HTTP_SERVICE_STARTED = "CTRL_AGENT_HTTP_SERVICE_STARTED";
2223
extern const isc::log::MessageID CTRL_AGENT_RUN_EXIT = "CTRL_AGENT_RUN_EXIT";
@@ -38,6 +39,7 @@ const char* values[] = {
3839
"CTRL_AGENT_FAILED", "application experienced a fatal error: %1",
3940
"CTRL_AGENT_HTTPS_SERVICE_REUSED", "reused HTTPS service bound to address %1:%2",
4041
"CTRL_AGENT_HTTPS_SERVICE_STARTED", "HTTPS service bound to address %1:%2",
42+
"CTRL_AGENT_HTTPS_SERVICE_UPDATED", "reused HTTPS service bound to address %1:%2 and updated TLS settings",
4143
"CTRL_AGENT_HTTP_SERVICE_REUSED", "reused HTTP service bound to address %1:%2",
4244
"CTRL_AGENT_HTTP_SERVICE_STARTED", "HTTP service bound to address %1:%2",
4345
"CTRL_AGENT_RUN_EXIT", "application is exiting the event loop",

‎src/bin/agent/ca_messages.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ extern const isc::log::MessageID CTRL_AGENT_CONFIG_SYNTAX_WARNING;
1818
extern const isc::log::MessageID CTRL_AGENT_FAILED;
1919
extern const isc::log::MessageID CTRL_AGENT_HTTPS_SERVICE_REUSED;
2020
extern const isc::log::MessageID CTRL_AGENT_HTTPS_SERVICE_STARTED;
21+
extern const isc::log::MessageID CTRL_AGENT_HTTPS_SERVICE_UPDATED;
2122
extern const isc::log::MessageID CTRL_AGENT_HTTP_SERVICE_REUSED;
2223
extern const isc::log::MessageID CTRL_AGENT_HTTP_SERVICE_STARTED;
2324
extern const isc::log::MessageID CTRL_AGENT_RUN_EXIT;

‎src/bin/agent/ca_messages.mes

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ This informational message indicates that the server has started HTTPS service
5353
on the specified address and port. All control commands should be sent to this
5454
address and port over a TLS channel.
5555

56+
% CTRL_AGENT_HTTPS_SERVICE_UPDATED reused HTTPS service bound to address %1:%2 and updated TLS settings
57+
This informational message indicates that the server has reused existing
58+
HTTPS service on the specified address and port. Note that any change in
59+
the TLS setup has been applied.
60+
5661
% CTRL_AGENT_HTTP_SERVICE_REUSED reused HTTP service bound to address %1:%2
5762
This informational message indicates that the server has reused existing
5863
HTTP service on the specified address and port.

‎src/bin/agent/ca_process.cc

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ CtrlAgentProcess::configure(isc::data::ConstElementPtr config_set,
145145
if (listener->getTlsContext()) {
146146
if (ctx->getTrustAnchor().empty()) {
147147
// Can not switch from HTTPS to HTTP
148-
LOG_INFO(agent_logger, CTRL_AGENT_HTTPS_SERVICE_REUSED)
148+
LOG_ERROR(agent_logger, CTRL_AGENT_HTTPS_SERVICE_REUSED)
149149
.arg(server_address.toText())
150150
.arg(server_port);
151151
} else {
@@ -161,10 +161,13 @@ CtrlAgentProcess::configure(isc::data::ConstElementPtr config_set,
161161
it->second->config_->setAuthConfig(ctx->getAuthConfig());
162162
it->second->config_->setHttpHeaders(ctx->getHttpHeaders());
163163
getIOService()->post([listener, tls_context]() { listener->setTlsContext(tls_context); });
164+
LOG_INFO(agent_logger, CTRL_AGENT_HTTPS_SERVICE_UPDATED)
165+
.arg(server_address.toText())
166+
.arg(server_port);
164167
}
165168
} else if (!ctx->getTrustAnchor().empty()) {
166169
// Can not switch from HTTP to HTTPS
167-
LOG_INFO(agent_logger, CTRL_AGENT_HTTP_SERVICE_REUSED)
170+
LOG_ERROR(agent_logger, CTRL_AGENT_HTTP_SERVICE_REUSED)
168171
.arg(server_address.toText())
169172
.arg(server_port);
170173
}
@@ -270,7 +273,7 @@ CtrlAgentProcess::closeCommandSockets() {
270273
// We have stopped listeners but there may be some pending handlers
271274
// related to these listeners. Need to invoke these handlers.
272275
try {
273-
getIOService()->pollOne();
276+
getIOService()->poll();
274277
} catch (...) {
275278
}
276279
}

‎src/lib/config/config_messages.cc

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,10 @@ extern const isc::log::MessageID COMMAND_WATCH_SOCKET_CLEAR_ERROR = "COMMAND_WAT
3535
extern const isc::log::MessageID COMMAND_WATCH_SOCKET_CLOSE_ERROR = "COMMAND_WATCH_SOCKET_CLOSE_ERROR";
3636
extern const isc::log::MessageID COMMAND_WATCH_SOCKET_MARK_READY_ERROR = "COMMAND_WATCH_SOCKET_MARK_READY_ERROR";
3737
extern const isc::log::MessageID HTTP_COMMAND_MGR_HTTPS_SERVICE_REUSED = "HTTP_COMMAND_MGR_HTTPS_SERVICE_REUSED";
38+
extern const isc::log::MessageID HTTP_COMMAND_MGR_HTTPS_SERVICE_UPDATED = "HTTP_COMMAND_MGR_HTTPS_SERVICE_UPDATED";
3839
extern const isc::log::MessageID HTTP_COMMAND_MGR_HTTP_SERVICE_REUSED = "HTTP_COMMAND_MGR_HTTP_SERVICE_REUSED";
3940
extern const isc::log::MessageID HTTP_COMMAND_MGR_SERVICE_STARTED = "HTTP_COMMAND_MGR_SERVICE_STARTED";
4041
extern const isc::log::MessageID HTTP_COMMAND_MGR_SERVICE_STOPPING = "HTTP_COMMAND_MGR_SERVICE_STOPPING";
41-
extern const isc::log::MessageID HTTP_COMMAND_MGR_SERVICE_STOPPING_ALL = "HTTP_COMMAND_MGR_SERVICE_STOPPING_ALL";
42-
extern const isc::log::MessageID HTTP_COMMAND_MGR_SERVICE_STOPPING_NO_DATA = "HTTP_COMMAND_MGR_SERVICE_STOPPING_NO_DATA";
4342

4443
} // namespace config
4544
} // namespace isc
@@ -75,11 +74,10 @@ const char* values[] = {
7574
"COMMAND_WATCH_SOCKET_CLOSE_ERROR", "watch socket failed to close: %1",
7675
"COMMAND_WATCH_SOCKET_MARK_READY_ERROR", "watch socket failed to mark ready: %1",
7776
"HTTP_COMMAND_MGR_HTTPS_SERVICE_REUSED", "reused HTTPS service bound to address %1:%2",
77+
"HTTP_COMMAND_MGR_HTTPS_SERVICE_UPDATED", "reused HTTPS service bound to address %1:%2 and updated TLS settings",
7878
"HTTP_COMMAND_MGR_HTTP_SERVICE_REUSED", "reused HTTP service bound to address %1:%2",
7979
"HTTP_COMMAND_MGR_SERVICE_STARTED", "started %1 service bound to address %2 port %3",
8080
"HTTP_COMMAND_MGR_SERVICE_STOPPING", "Server is stopping %1 service %2",
81-
"HTTP_COMMAND_MGR_SERVICE_STOPPING_ALL", "stopping %1 service %2",
82-
"HTTP_COMMAND_MGR_SERVICE_STOPPING_NO_DATA", "Server is stopping all services including %1 service %2",
8381
NULL
8482
};
8583

‎src/lib/config/config_messages.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,10 @@ extern const isc::log::MessageID COMMAND_WATCH_SOCKET_CLEAR_ERROR;
3636
extern const isc::log::MessageID COMMAND_WATCH_SOCKET_CLOSE_ERROR;
3737
extern const isc::log::MessageID COMMAND_WATCH_SOCKET_MARK_READY_ERROR;
3838
extern const isc::log::MessageID HTTP_COMMAND_MGR_HTTPS_SERVICE_REUSED;
39+
extern const isc::log::MessageID HTTP_COMMAND_MGR_HTTPS_SERVICE_UPDATED;
3940
extern const isc::log::MessageID HTTP_COMMAND_MGR_HTTP_SERVICE_REUSED;
4041
extern const isc::log::MessageID HTTP_COMMAND_MGR_SERVICE_STARTED;
4142
extern const isc::log::MessageID HTTP_COMMAND_MGR_SERVICE_STOPPING;
42-
extern const isc::log::MessageID HTTP_COMMAND_MGR_SERVICE_STOPPING_ALL;
43-
extern const isc::log::MessageID HTTP_COMMAND_MGR_SERVICE_STOPPING_NO_DATA;
4443

4544
} // namespace config
4645
} // namespace isc

‎src/lib/config/config_messages.mes

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,11 @@ This informational message indicates that the server has reused existing
158158
HTTPS service on the specified address and port. Note that any change in
159159
the TLS setup was ignored.
160160

161+
% HTTP_COMMAND_MGR_HTTPS_SERVICE_UPDATED reused HTTPS service bound to address %1:%2 and updated TLS settings
162+
This informational message indicates that the server has reused existing
163+
HTTPS service on the specified address and port. Note that any change in
164+
the TLS setup has been applied.
165+
161166
% HTTP_COMMAND_MGR_HTTP_SERVICE_REUSED reused HTTP service bound to address %1:%2
162167
This informational message indicates that the server has reused existing
163168
HTTP service on the specified address and port.
@@ -170,12 +175,3 @@ control commands.
170175
% HTTP_COMMAND_MGR_SERVICE_STOPPING Server is stopping %1 service %2
171176
This informational message indicates that the server has stopped
172177
HTTP/HTTPS service. When known the address and port are displayed.
173-
174-
% HTTP_COMMAND_MGR_SERVICE_STOPPING_ALL stopping %1 service %2
175-
This informational message indicates that the server has stopped
176-
HTTP/HTTPS service. When known the address and port are displayed.
177-
178-
% HTTP_COMMAND_MGR_SERVICE_STOPPING_NO_DATA Server is stopping all services including %1 service %2
179-
This informational message indicates that the server is stopping all
180-
HTTP/HTTPS services. When known the address and port are displayed for
181-
each service.

‎src/lib/config/http_command_mgr.cc

Lines changed: 19 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,9 @@ class HttpCommandMgrImpl {
5858
void closeCommandSocket(HttpSocketInfoPtr info, bool remove);
5959

6060
/// @brief Close control socket.
61-
void closeCommandSockets();
61+
///
62+
/// @param remove When true remove the listeners immediately.
63+
void closeCommandSockets(bool remove = true);
6264

6365
/// @brief Returns a const pointer to the HTTP listener.
6466
///
@@ -133,7 +135,7 @@ HttpCommandMgrImpl::openCommandSocket(const isc::data::ConstElementPtr config) {
133135
if (listener->getTlsContext()) {
134136
if (cmd_config->getTrustAnchor().empty()) {
135137
// Can not switch from HTTPS to HTTP
136-
LOG_INFO(command_logger, HTTP_COMMAND_MGR_HTTPS_SERVICE_REUSED)
138+
LOG_ERROR(command_logger, HTTP_COMMAND_MGR_HTTPS_SERVICE_REUSED)
137139
.arg(server_address.toText())
138140
.arg(server_port);
139141
} else {
@@ -151,10 +153,13 @@ HttpCommandMgrImpl::openCommandSocket(const isc::data::ConstElementPtr config) {
151153
it->second->config_->setHttpHeaders(cmd_config->getHttpHeaders());
152154
it->second->config_->setEmulateAgentResponse(cmd_config->getEmulateAgentResponse());
153155
io_service_->post([listener, tls_context]() { listener->setTlsContext(tls_context); });
156+
LOG_INFO(command_logger, HTTP_COMMAND_MGR_HTTPS_SERVICE_UPDATED)
157+
.arg(server_address.toText())
158+
.arg(server_port);
154159
}
155160
} else if (!cmd_config->getTrustAnchor().empty()) {
156161
// Can not switch from HTTP to HTTPS
157-
LOG_INFO(command_logger, HTTP_COMMAND_MGR_HTTP_SERVICE_REUSED)
162+
LOG_ERROR(command_logger, HTTP_COMMAND_MGR_HTTP_SERVICE_REUSED)
158163
.arg(server_address.toText())
159164
.arg(server_port);
160165
}
@@ -233,50 +238,22 @@ HttpCommandMgrImpl::closeCommandSocket(HttpSocketInfoPtr info, bool remove) {
233238
sockets_.erase(it);
234239
}
235240
}
236-
} else {
237-
for (auto const& data : sockets_) {
238-
ostringstream ep;
239-
use_https = !data.second->config_->getCertFile().empty();
240-
ep << "bound to address " << data.second->config_->getSocketAddress()
241-
<< " port " << data.second->config_->getSocketPort();
242-
243-
LOG_INFO(command_logger, HTTP_COMMAND_MGR_SERVICE_STOPPING_NO_DATA)
244-
.arg(use_https ? "HTTPS" : "HTTP")
245-
.arg(ep.str());
246-
data.second->listener_->stop();
241+
// We have stopped listeners but there may be some pending handlers
242+
// related to these listeners. Need to invoke these handlers.
243+
try {
244+
io_service_->pollOne();
245+
} catch (...) {
247246
}
248-
if (remove) {
249-
sockets_.clear();
250-
}
251-
}
252-
// We have stopped listeners but there may be some pending handlers
253-
// related to these listeners. Need to invoke these handlers.
254-
try {
255-
io_service_->pollOne();
256-
} catch (...) {
247+
} else {
248+
closeCommandSockets(remove);
257249
}
258250
}
259251

260252
void
261-
HttpCommandMgrImpl::closeCommandSockets() {
262-
bool use_https = false;
263-
for (auto const& data : sockets_) {
264-
ostringstream ep;
265-
use_https = !data.second->config_->getCertFile().empty();
266-
ep << "bound to address " << data.second->config_->getSocketAddress()
267-
<< " port " << data.second->config_->getSocketPort();
268-
269-
LOG_INFO(command_logger, HTTP_COMMAND_MGR_SERVICE_STOPPING_ALL)
270-
.arg(use_https ? "HTTPS" : "HTTP")
271-
.arg(ep.str());
272-
data.second->listener_->stop();
273-
}
274-
sockets_.clear();
275-
// We have stopped listeners but there may be some pending handlers
276-
// related to these listeners. Need to invoke these handlers.
277-
try {
278-
io_service_->pollOne();
279-
} catch (...) {
253+
HttpCommandMgrImpl::closeCommandSockets(bool remove) {
254+
auto copy = sockets_;
255+
for (auto const& data : copy) {
256+
closeCommandSocket(data.second, remove);
280257
}
281258
}
282259

‎src/lib/config/http_command_mgr.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,17 @@ class HttpCommandMgr : public boost::noncopyable {
6464
/// Creates http/https listener, or reuses the existing one reapplying
6565
/// changes.
6666
///
67+
/// @note This function in used internally by @ref openCommandSockets and it
68+
/// should not be used directly, except for unittests.
69+
///
6770
/// @param config Configuration information for the http control socket.
6871
void openCommandSocket(const isc::data::ConstElementPtr config);
6972

7073
/// @brief Close http control socket.
7174
///
75+
/// @note This function in used internally by @ref closeCommandSockets and it
76+
/// should not be used directly, except for unittests.
77+
///
7278
/// @param info Configuration information for the http control socket.
7379
/// @param remove When true remove the listeners immediately.
7480
void closeCommandSocket(HttpSocketInfoPtr info = HttpSocketInfoPtr(), bool remove = true);

‎src/lib/config/unix_command_mgr.cc

Lines changed: 14 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,9 @@ class UnixCommandMgrImpl {
523523
void closeCommandSocket(UnixSocketInfoPtr info);
524524

525525
/// @brief Shuts down any open unix control sockets
526-
void closeCommandSockets();
526+
///
527+
/// @param remove When true remove the listeners immediately.
528+
void closeCommandSockets(bool remove = true);
527529

528530
/// @brief Asynchronously accepts next connection.
529531
///
@@ -679,61 +681,24 @@ UnixCommandMgrImpl::closeCommandSocket(UnixSocketInfoPtr info) {
679681
if (it != sockets_.end()) {
680682
sockets_.erase(it);
681683
}
682-
} else {
683-
for (auto const& data : sockets_) {
684-
if (data.second->acceptor_ && data.second->acceptor_->isOpen()) {
685-
if (use_external_) {
686-
IfaceMgr::instance().deleteExternalSocket(data.second->acceptor_->getNative());
687-
}
688-
data.second->acceptor_->close();
689-
static_cast<void>(::remove(data.second->config_->getSocketName().c_str()));
690-
static_cast<void>(::remove(data.second->config_->getLockName().c_str()));
691-
}
692-
693-
// Stop all connections which can be closed. The only connection that won't
694-
// be closed is the one over which we have received a request to reconfigure
695-
// the server. This connection will be held until the UnixCommandMgr
696-
// responds to such request.
697-
connection_pool_.stopAll();
698-
if (data.second->lock_fd_ != -1) {
699-
close(data.second->lock_fd_);
700-
data.second->lock_fd_ = -1;
701-
}
684+
try {
685+
io_service_->pollOne();
686+
} catch (...) {
702687
}
703-
}
704-
try {
705-
io_service_->pollOne();
706-
} catch (...) {
688+
} else {
689+
closeCommandSockets(false);
707690
}
708691
}
709692

710693
void
711-
UnixCommandMgrImpl::closeCommandSockets() {
712-
for (auto const& data : sockets_) {
713-
if (data.second->acceptor_ && data.second->acceptor_->isOpen()) {
714-
if (use_external_) {
715-
IfaceMgr::instance().deleteExternalSocket(data.second->acceptor_->getNative());
716-
}
717-
data.second->acceptor_->close();
718-
static_cast<void>(::remove(data.second->config_->getSocketName().c_str()));
719-
static_cast<void>(::remove(data.second->config_->getLockName().c_str()));
720-
}
721-
722-
// Stop all connections which can be closed. The only connection that won't
723-
// be closed is the one over which we have received a request to reconfigure
724-
// the server. This connection will be held until the UnixCommandMgr
725-
// responds to such request.
726-
connection_pool_.stopAll();
727-
if (data.second->lock_fd_ != -1) {
728-
close(data.second->lock_fd_);
729-
data.second->lock_fd_ = -1;
730-
}
694+
UnixCommandMgrImpl::closeCommandSockets(bool remove) {
695+
auto copy = sockets_;
696+
for (auto const& data : copy) {
697+
closeCommandSocket(data.second);
731698
}
732-
try {
733-
io_service_->pollOne();
734-
} catch (...) {
699+
if (remove) {
700+
sockets_.clear();
735701
}
736-
sockets_.clear();
737702
}
738703

739704
void

‎src/lib/config/unix_command_mgr.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@ class UnixCommandMgr : public boost::noncopyable {
7373
///
7474
/// Creates acceptor, or reuses the existing one.
7575
///
76+
/// @note This function in used internally by @ref openCommandSockets and it
77+
/// should not be used directly, except for unittests.
78+
///
7679
/// @throw BadSocketInfo When socket configuration is invalid.
7780
/// @throw SocketError When socket operation fails.
7881
///
@@ -81,6 +84,9 @@ class UnixCommandMgr : public boost::noncopyable {
8184

8285
/// @brief Shuts down any open unix control sockets.
8386
///
87+
/// @note This function in used internally by @ref closeCommandSockets and it
88+
/// should not be used directly, except for unittests.
89+
///
8490
/// @param config Configuration information for the unix control socket.
8591
void closeCommandSocket(UnixSocketInfoPtr info = UnixSocketInfoPtr());
8692

0 commit comments

Comments
 (0)