From: Francis Dupont Date: Sun, 31 May 2026 10:04:19 +0000 (+0200) Subject: [#4507] Improved fatal interface-add error X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b0ca753453add03c7915ef573fe7df44ef4ce057;p=thirdparty%2Fkea.git [#4507] Improved fatal interface-add error --- diff --git a/changelog_unreleased/4507-fix-service-sockets-require-all-true-behavior b/changelog_unreleased/4507-fix-service-sockets-require-all-true-behavior new file mode 100644 index 0000000000..147b2b662d --- /dev/null +++ b/changelog_unreleased/4507-fix-service-sockets-require-all-true-behavior @@ -0,0 +1,4 @@ +[func] fdupont + Made reconfig commands which triggered a fatal error not to + return a success answer when the server is shutting down. + (Gitlab #4507) diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index 070e42b17d..1e67ef0e19 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -93,7 +93,7 @@ void signalHandler(int signo) { namespace isc { namespace dhcp { -ControlledDhcpv4Srv* ControlledDhcpv4Srv::server_ = NULL; +ControlledDhcpv4Srv* ControlledDhcpv4Srv::server_ = 0; void ControlledDhcpv4Srv::init(const std::string& file_name) { @@ -198,6 +198,11 @@ ControlledDhcpv4Srv::loadConfigFile(const std::string& file_name) { return (result); } +bool +ControlledDhcpv4Srv::getShutdown() const { + return (Dhcpv4Srv::shutdown_); +} + ConstElementPtr ControlledDhcpv4Srv::commandShutdownHandler(const string&, ConstElementPtr args) { if (!ControlledDhcpv4Srv::getInstance()) { @@ -413,6 +418,12 @@ ControlledDhcpv4Srv::commandConfigSetHandler(const string&, // the logging first in case there's a configuration failure. int rcode = 0; isc::config::parseAnswer(rcode, result); + if (getShutdown() && (rcode == CONTROL_RESULT_SUCCESS)) { + // Do not return success when a fatal error was triggered. + rcode = CONTROL_RESULT_FATAL_ERROR; + message = "Reconfiguration triggered a fatal error: shutting down."; + result = isc::config::createAnswer(rcode, message); + } if (rcode == CONTROL_RESULT_SUCCESS) { CfgMgr::instance().getStagingCfg()->applyLoggingCfg(); @@ -808,6 +819,9 @@ ControlledDhcpv4Srv::commandInterfaceAddHandler(const std::string&, ostringstream msg; if (!error) { + if (getShutdown()) { + return (isc::config::createAnswer(CONTROL_RESULT_FATAL_ERROR, "Interface configuration uodate triggered a fatal error: shutting down.")); + } return (isc::config::createAnswer(CONTROL_RESULT_SUCCESS, "Interface configuration successfully updated.")); } else { msg << "Updating used interfaces failed: " << message; diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.h b/src/bin/dhcp4/ctrl_dhcp4_srv.h index 5727090fbc..d0d6831900 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.h +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.h @@ -73,6 +73,9 @@ public: /// @param exit_value integer value to the process should exit with. virtual void shutdownServer(int exit_value); + /// @brief Return the server shutdown flag value. + bool getShutdown() const; + /// @brief Configuration processor /// /// This is a method for handling incoming configuration updates. diff --git a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc index a72df2cdc8..ef0ada5fec 100644 --- a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc @@ -2602,6 +2602,41 @@ TEST_F(CtrlChannelDhcpv4SrvTest, interfaceAdd) { EXPECT_EQ(response, expected); } +// Tests if interface-add can trigger a fatal failure. +TEST_F(CtrlChannelDhcpv4SrvTest, interfaceAddFatal) { + interfaces_ = ""; + IfacePtr eth0 = IfaceMgrTestConfig::createIface("eth0", ETH0_INDEX, + "11:22:33:44:55:66"); + auto detectIfaces = [&](bool update_only) { + if (!update_only) { + // No IPv4 address: will trigger an error. + eth0->addAddress(IOAddress("fe80::3a60:77ff:fed5:cdef")); + eth0->addAddress(IOAddress("2001:db8:1::1")); + IfaceMgr::instance().addInterface(eth0); + } + return (false); + }; + IfaceMgr::instance().setDetectCallback(detectIfaces); + IfaceMgr::instance().clearIfaces(); + IfaceMgr::instance().closeSockets(); + IfaceMgr::instance().detectIfaces(); + createUnixChannelServer(); + PktFilterPtr filter(new PktFilterTestStub()); + IfaceMgr::instance().setPacketFilter(filter); + SKIP_IF(skipped_); + // Override the on fail callback to unconditionally make the error fatal. + CfgIface::open_sockets_failed_callback_ = + [](ReconnectCtlPtr) { + ControlledDhcpv4Srv::getInstance()->shutdownServer(EXIT_FAILURE); + }; + std::string response; + + std::string command = "{ \"command\": \"interface-add\", \"arguments\": { \"interfaces\": [ \"eth0\" ] } }"; + + sendUnixCommand(command, response); + EXPECT_EQ(response, "{ \"result\": 5, \"text\": \"Interface configuration uodate triggered a fatal error: shutting down.\" }"); +} + // This test verifies that disable DHCP service command performs sanity check on // parameters. TEST_F(CtrlChannelDhcpv4SrvTest, dhcpDisableBadParam) { diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index ab38cf3d1d..188b6050fd 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -96,7 +96,7 @@ void signalHandler(int signo) { namespace isc { namespace dhcp { -ControlledDhcpv6Srv* ControlledDhcpv6Srv::server_ = NULL; +ControlledDhcpv6Srv* ControlledDhcpv6Srv::server_ = 0; void ControlledDhcpv6Srv::init(const std::string& file_name) { @@ -201,6 +201,11 @@ ControlledDhcpv6Srv::loadConfigFile(const std::string& file_name) { return (result); } +bool +ControlledDhcpv6Srv::getShutdown() const { + return (Dhcpv6Srv::shutdown_); +} + ConstElementPtr ControlledDhcpv6Srv::commandShutdownHandler(const string&, ConstElementPtr args) { if (!ControlledDhcpv6Srv::getInstance()) { @@ -416,6 +421,12 @@ ControlledDhcpv6Srv::commandConfigSetHandler(const string&, // the logging first in case there's a configuration failure. int rcode = 0; isc::config::parseAnswer(rcode, result); + if (getShutdown() && (rcode == CONTROL_RESULT_SUCCESS)) { + // Do not return success when a fatal error was triggered. + rcode = CONTROL_RESULT_FATAL_ERROR; + message = "Reconfiguration triggered a fatal error: shutting down."; + result = isc::config::createAnswer(rcode, message); + } if (rcode == CONTROL_RESULT_SUCCESS) { CfgMgr::instance().getStagingCfg()->applyLoggingCfg(); @@ -811,6 +822,9 @@ ControlledDhcpv6Srv::commandInterfaceAddHandler(const std::string&, ostringstream msg; if (!error) { + if (getShutdown()) { + return (isc::config::createAnswer(CONTROL_RESULT_FATAL_ERROR, "Interface configuration uodate triggered a fatal error: shutting down.")); + } return (isc::config::createAnswer(CONTROL_RESULT_SUCCESS, "Interface configuration successfully updated.")); } else { msg << "Updating used interfaces failed: " << message; diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.h b/src/bin/dhcp6/ctrl_dhcp6_srv.h index afad02b913..ae9751ee48 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.h +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.h @@ -73,6 +73,9 @@ public: /// @param exit_value integer value to the process should exit with. virtual void shutdownServer(int exit_value); + /// @brief Return the server shutdown flag value. + bool getShutdown() const; + /// @brief Configuration processor /// /// This is a method for handling incoming configuration updates. diff --git a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc index fa952c17ae..1375beb547 100644 --- a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc @@ -2596,6 +2596,44 @@ TEST_F(CtrlChannelDhcpv6SrvTest, interfaceAdd) { EXPECT_EQ(response, expected); } +// Tests if interface-add can trigger a fatal failure. +TEST_F(CtrlChannelDhcpv6SrvTest, interfaceAddFatal) { + interfaces_ = ""; + IfacePtr eth0 = IfaceMgrTestConfig::createIface("eth0", ETH0_INDEX, + "11:22:33:44:55:66"); + auto detectIfaces = [&](bool update_only) { + if (!update_only) { + eth0->addAddress(IOAddress("10.0.0.1")); + eth0->addAddress(IOAddress("fe80::3a60:77ff:fed5:cdef")); + eth0->addAddress(IOAddress("2001:db8:1::1")); + IfaceMgr::instance().addInterface(eth0); + } else { + // Trigger an error on interface-add. + eth0->flag_running_ = false; + } + return (false); + }; + IfaceMgr::instance().setDetectCallback(detectIfaces); + IfaceMgr::instance().clearIfaces(); + IfaceMgr::instance().closeSockets(); + IfaceMgr::instance().detectIfaces(); + createUnixChannelServer(); + PktFilter6Ptr filter(new PktFilter6TestStub()); + IfaceMgr::instance().setPacketFilter(filter); + SKIP_IF(skipped_); + // Override the on fail callback to unconditionally make the error fatal. + CfgIface::open_sockets_failed_callback_ = + [](ReconnectCtlPtr) { + ControlledDhcpv6Srv::getInstance()->shutdownServer(EXIT_FAILURE); + }; + std::string response; + + std::string command = "{ \"command\": \"interface-add\", \"arguments\": { \"interfaces\": [ \"eth0\" ] } }"; + + sendUnixCommand(command, response); + EXPECT_EQ(response, "{ \"result\": 5, \"text\": \"Interface configuration uodate triggered a fatal error: shutting down.\" }"); +} + // This test verifies that disable DHCP service command performs sanity check on // parameters. TEST_F(CtrlChannelDhcpv6SrvTest, dhcpDisableBadParam) {