From: Marcin Siodelski Date: Wed, 24 Apr 2024 19:04:18 +0000 (+0200) Subject: [#3344] Use origin-id instead of origin X-Git-Tag: Kea-2.5.8~28 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0025845104fbd3d30684721b28c7401c62c43cfc;p=thirdparty%2Fkea.git [#3344] Use origin-id instead of origin The HA partners now send the commands with both origin-id and origin to provide backward compatibility between different Kea versions. --- diff --git a/ChangeLog b/ChangeLog index ad94b206a7..7585b8f7da 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2225. [func] marcin + Ensure backward compatibility of High Availability + between Kea 2.5.8+ and earlier versions. It introduces + a new origin-id argument to the dhcp-enable, dhcp-disable + and ha-sync-complete-notify commands. It is ignored by the + earlier Kea versions. The origin argument is sent in addition + to the origin-id and has the format recognizable by the old + Kea versions. + (Gitlab #3344) + 2224. [func] andrei Printing the version now mentions if premium is included and shows the git commit hash for the premium source code repository diff --git a/doc/sphinx/arm/ctrl-channel.rst b/doc/sphinx/arm/ctrl-channel.rst index b6a5cd849c..d4d92e363b 100644 --- a/doc/sphinx/arm/ctrl-channel.rst +++ b/doc/sphinx/arm/ctrl-channel.rst @@ -721,12 +721,17 @@ communication between HA partners and should not be specified in the administrator's commands, as it may interfere with HA operation. The administrator should either omit this parameter or set it to "user". +Introduction of ``origin-id`` deprecates the use of the ``origin`` parameter +in the messages exchanged between the Kea HA partners. The ``origin-id`` parameter +must not be used in messages sent by the user. + :: { "command": "dhcp-disable", "arguments": { "max-period": 20, + "origin-id": 2002, "origin": "user" } } @@ -750,6 +755,10 @@ administrator's commands, as it may interfere with HA operation. The administrator should either omit this parameter or set it to "user". +Introduction of ``origin-id`` deprecates the use of the ``origin`` parameter +in the messages exchanged between the Kea HA partners. The ``origin-id`` parameter +must not be used in messages sent by the user. + :: { diff --git a/doc/sphinx/arm/hooks-ha.rst b/doc/sphinx/arm/hooks-ha.rst index 96c4bb5fe3..866599105d 100644 --- a/doc/sphinx/arm/hooks-ha.rst +++ b/doc/sphinx/arm/hooks-ha.rst @@ -2366,7 +2366,7 @@ responding to clients. "command": "ha-sync-complete-notify", "service": [ "dhcp4" ], "arguments": { - "origin": 2000, + "origin-id": 2000, "server-name": "server2" } } @@ -2375,9 +2375,11 @@ The optional ``server-name`` parameter specifies a name of one of the partners b to the HA relationship this command pertains to. This parameter can be omitted if the server receiving this command has only one HA relationship in the configuration. -The ``origin`` parameter is used to select the HA service for which the receiving server should +The ``origin-id`` parameter is used to select the HA service for which the receiving server should enable the DHCP service when it receives this notification. This is the same origin the sending server used previously to disable the DHCP service before synchronization. +The ``origin-id`` parameter deprecates the ``origin`` parameter used in some earlier +Kea versions. It elicits the response: diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index aecc2c6d7c..ef4b7de88b 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -559,9 +559,22 @@ ControlledDhcpv4Srv::commandDhcpDisableHandler(const std::string&, } } } + // 'origin-id' replaces the older parameter 'origin' since Kea 2.6.0 + // stable release. However, the 'origin' is kept for backward compatibility + // with Kea versions before 2.6.0. It is common to receive both parameters + // because HA hook library sends both in case the partner server hasn't been + // upgraded to the new version. The 'origin-id' takes precedence over the + // 'origin'. + ConstElementPtr origin_id_element = args->get("origin-id"); ConstElementPtr origin_element = args->get("origin"); - // The 'origin' parameter is optional. - if (origin_element) { + // The 'origin-id' and 'origin' arguments are optional. + if (origin_id_element) { + if (origin_id_element->getType() == Element::integer) { + type = origin_id_element->intValue(); + } else { + message << "'origin-id' argument must be a number"; + } + } else if (origin_element) { switch (origin_element->getType()) { case Element::string: origin = origin_element->stringValue(); @@ -625,9 +638,22 @@ ControlledDhcpv4Srv::commandDhcpEnableHandler(const std::string&, message << "arguments for the 'dhcp-enable' command must be a map"; } else { + // 'origin-id' replaces the older parameter 'origin' since Kea 2.6.0 + // stable release. However, the 'origin' is kept for backward compatibility + // with Kea versions before 2.6.0. It is common to receive both parameters + // because HA hook library sends both in case the partner server hasn't been + // upgraded to the new version. The 'origin-id' takes precedence over the + // 'origin'. + ConstElementPtr origin_id_element = args->get("origin-id"); ConstElementPtr origin_element = args->get("origin"); - // The 'origin' parameter is optional. - if (origin_element) { + // The 'origin-id' and 'origin' arguments are optional. + if (origin_id_element) { + if (origin_id_element->getType() == Element::integer) { + type = origin_id_element->intValue(); + } else { + message << "'origin-id' argument must be a number"; + } + } else if (origin_element) { switch (origin_element->getType()) { case Element::string: origin = origin_element->stringValue(); diff --git a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc index 04a567baf9..a0be312596 100644 --- a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc @@ -1724,6 +1724,32 @@ TEST_F(CtrlChannelDhcpv4SrvTest, dhcpDisableBadParam) { EXPECT_EQ("{ \"result\": 1, \"text\": \"'max-period' must be positive " "integer\" }", response); + sendUnixCommand("{" + " \"command\": \"dhcp-disable\"," + " \"arguments\": {" + " \"origin-id\": \"\"" + " }" + "}", response); + + // The response should be a valid JSON. + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + ASSERT_TRUE(rsp); + + EXPECT_EQ("{ \"result\": 1, \"text\": \"'origin-id' argument must be a number\" }", response); + + sendUnixCommand("{" + " \"command\": \"dhcp-disable\"," + " \"arguments\": {" + " \"origin-id\": \"foo\"" + " }" + "}", response); + + // The response should be a valid JSON. + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + ASSERT_TRUE(rsp); + + EXPECT_EQ("{ \"result\": 1, \"text\": \"'origin-id' argument must be a number\" }", response); + sendUnixCommand("{" " \"command\": \"dhcp-disable\"," " \"arguments\": {" @@ -1836,6 +1862,39 @@ TEST_F(CtrlChannelDhcpv4SrvTest, dhcpDisable) { EXPECT_TRUE(server_->network_state_->isServiceEnabled()); } +// This test verifies if it is possible to disable DHCP service using +// the origin-id. +TEST_F(CtrlChannelDhcpv4SrvTest, dhcpDisableOriginId) { + createUnixChannelServer(); + std::string response; + + EXPECT_TRUE(server_->network_state_->isServiceEnabled()); + + sendUnixCommand("{" + " \"command\": \"dhcp-disable\"," + " \"arguments\": {" + " \"origin-id\": 2002," + " \"origin\": \"user\"" + " }" + "}", response); + + ConstElementPtr rsp; + + // The response should be a valid JSON. + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + ASSERT_TRUE(rsp); + + int status; + ConstElementPtr cfg = parseAnswer(status, rsp); + EXPECT_EQ(CONTROL_RESULT_SUCCESS, status); + + EXPECT_FALSE(server_->network_state_->isServiceEnabled()); + + server_->network_state_->enableService(NetworkState::HA_REMOTE_COMMAND+2); + + EXPECT_TRUE(server_->network_state_->isServiceEnabled()); +} + // This test verifies that it is possible to disable DHCP service for a short // period of time, after which the service is automatically enabled. TEST_F(CtrlChannelDhcpv4SrvTest, dhcpDisableTemporarily) { @@ -1871,6 +1930,33 @@ TEST_F(CtrlChannelDhcpv4SrvTest, dhcpDisableTemporarily) { TEST_F(CtrlChannelDhcpv4SrvTest, dhcpEnableBadParam) { createUnixChannelServer(); std::string response; + ConstElementPtr rsp; + + sendUnixCommand("{" + " \"command\": \"dhcp-enable\"," + " \"arguments\": {" + " \"origin-id\": \"\"" + " }" + "}", response); + + // The response should be a valid JSON. + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + ASSERT_TRUE(rsp); + + EXPECT_EQ("{ \"result\": 1, \"text\": \"'origin-id' argument must be a number\" }", response); + + sendUnixCommand("{" + " \"command\": \"dhcp-enable\"," + " \"arguments\": {" + " \"origin-id\": \"foo\"" + " }" + "}", response); + + // The response should be a valid JSON. + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + ASSERT_TRUE(rsp); + + EXPECT_EQ("{ \"result\": 1, \"text\": \"'origin-id' argument must be a number\" }", response); sendUnixCommand("{" " \"command\": \"dhcp-enable\"," @@ -1878,7 +1964,6 @@ TEST_F(CtrlChannelDhcpv4SrvTest, dhcpEnableBadParam) { " \"origin\": \"\"" " }" "}", response); - ConstElementPtr rsp; // The response should be a valid JSON. EXPECT_NO_THROW(rsp = Element::fromJSON(response)); @@ -1981,6 +2066,61 @@ TEST_F(CtrlChannelDhcpv4SrvTest, dhcpEnable) { EXPECT_TRUE(server_->network_state_->isServiceEnabled()); } +// This test verifies if it is possible to enable DHCP service using +// the origin-id. +TEST_F(CtrlChannelDhcpv4SrvTest, dhcpEnableOriginId) { + createUnixChannelServer(); + std::string response; + + ConstElementPtr rsp; + + int status; + + // Disable the service using two distinct origins. + server_->network_state_->disableService(NetworkState::HA_REMOTE_COMMAND+1); + server_->network_state_->disableService(NetworkState::USER_COMMAND); + + EXPECT_FALSE(server_->network_state_->isServiceEnabled()); + + // Enable the service for the 'origin-id' of 2001. The 'origin' should + // be ignored. + sendUnixCommand("{" + " \"command\": \"dhcp-enable\"," + " \"arguments\": {" + " \"origin-id\": 2001," + " \"origin\": \"user\"" + " }" + "}", response); + + // The response should be a valid JSON. + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + ASSERT_TRUE(rsp); + + ConstElementPtr cfg = parseAnswer(status, rsp); + EXPECT_EQ(CONTROL_RESULT_SUCCESS, status); + + // The service should still be disabled. + EXPECT_FALSE(server_->network_state_->isServiceEnabled()); + + // Enable the service for the user command. + sendUnixCommand("{" + " \"command\": \"dhcp-enable\"," + " \"arguments\": {" + " \"origin-id\": 1" + " }" + "}", response); + + // The response should be a valid JSON. + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + ASSERT_TRUE(rsp); + + cfg = parseAnswer(status, rsp); + EXPECT_EQ(CONTROL_RESULT_SUCCESS, status); + + // The service should be enabled. + EXPECT_TRUE(server_->network_state_->isServiceEnabled()); +} + /// Verify that concurrent connections over the control channel can be /// established. /// @todo Future Kea 1.3 tickets will modify the behavior of the CommandMgr diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index b8314843d8..f9d89a1cee 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -561,9 +561,22 @@ ControlledDhcpv6Srv::commandDhcpDisableHandler(const std::string&, } } } + // 'origin-id' replaces the older parameter 'origin' since Kea 2.6.0 + // stable release. However, the 'origin' is kept for backward compatibility + // with Kea versions before 2.6.0. It is common to receive both parameters + // because HA hook library sends both in case the partner server hasn't been + // upgraded to the new version. The 'origin-id' takes precedence over the + // 'origin'. + ConstElementPtr origin_id_element = args->get("origin-id"); ConstElementPtr origin_element = args->get("origin"); - // The 'origin' parameter is optional. - if (origin_element) { + // The 'origin-id' and 'origin' arguments are optional. + if (origin_id_element) { + if (origin_id_element->getType() == Element::integer) { + type = origin_id_element->intValue(); + } else { + message << "'origin-id' argument must be a number"; + } + } else if (origin_element) { switch (origin_element->getType()) { case Element::string: origin = origin_element->stringValue(); @@ -627,9 +640,22 @@ ControlledDhcpv6Srv::commandDhcpEnableHandler(const std::string&, message << "arguments for the 'dhcp-enable' command must be a map"; } else { + // 'origin-id' replaces the older parameter 'origin' since Kea 2.6.0 + // stable release. However, the 'origin' is kept for backward compatibility + // with Kea versions before 2.6.0. It is common to receive both parameters + // because HA hook library sends both in case the partner server hasn't been + // upgraded to the new version. The 'origin-id' takes precedence over the + // 'origin'. + ConstElementPtr origin_id_element = args->get("origin-id"); ConstElementPtr origin_element = args->get("origin"); - // The 'origin' parameter is optional. - if (origin_element) { + // The 'origin-id' and 'origin' arguments are optional. + if (origin_id_element) { + if (origin_id_element->getType() == Element::integer) { + type = origin_id_element->intValue(); + } else { + message << "'origin-id' argument must be a number"; + } + } else if (origin_element) { switch (origin_element->getType()) { case Element::string: origin = origin_element->stringValue(); diff --git a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc index 1150b1f577..8f2dc86942 100644 --- a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc @@ -1762,6 +1762,32 @@ TEST_F(CtrlChannelDhcpv6SrvTest, dhcpDisableBadParam) { EXPECT_EQ("{ \"result\": 1, \"text\": \"'max-period' must be positive " "integer\" }", response); + sendUnixCommand("{" + " \"command\": \"dhcp-disable\"," + " \"arguments\": {" + " \"origin-id\": \"\"" + " }" + "}", response); + + // The response should be a valid JSON. + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + ASSERT_TRUE(rsp); + + EXPECT_EQ("{ \"result\": 1, \"text\": \"'origin-id' argument must be a number\" }", response); + + sendUnixCommand("{" + " \"command\": \"dhcp-disable\"," + " \"arguments\": {" + " \"origin-id\": \"foo\"" + " }" + "}", response); + + // The response should be a valid JSON. + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + ASSERT_TRUE(rsp); + + EXPECT_EQ("{ \"result\": 1, \"text\": \"'origin-id' argument must be a number\" }", response); + sendUnixCommand("{" " \"command\": \"dhcp-disable\"," " \"arguments\": {" @@ -1874,6 +1900,39 @@ TEST_F(CtrlChannelDhcpv6SrvTest, dhcpDisable) { EXPECT_TRUE(server_->network_state_->isServiceEnabled()); } +// This test verifies if it is possible to disable DHCP service using +// the origin-id. +TEST_F(CtrlChannelDhcpv6SrvTest, dhcpDisableOriginId) { + createUnixChannelServer(); + std::string response; + + EXPECT_TRUE(server_->network_state_->isServiceEnabled()); + + sendUnixCommand("{" + " \"command\": \"dhcp-disable\"," + " \"arguments\": {" + " \"origin-id\": 2002," + " \"origin\": \"user\"" + " }" + "}", response); + + ConstElementPtr rsp; + + // The response should be a valid JSON. + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + ASSERT_TRUE(rsp); + + int status; + ConstElementPtr cfg = parseAnswer(status, rsp); + EXPECT_EQ(CONTROL_RESULT_SUCCESS, status); + + EXPECT_FALSE(server_->network_state_->isServiceEnabled()); + + server_->network_state_->enableService(NetworkState::HA_REMOTE_COMMAND+2); + + EXPECT_TRUE(server_->network_state_->isServiceEnabled()); +} + // This test verifies that it is possible to disable DHCP service for a short // period of time, after which the service is automatically enabled. TEST_F(CtrlChannelDhcpv6SrvTest, dhcpDisableTemporarily) { @@ -1909,6 +1968,33 @@ TEST_F(CtrlChannelDhcpv6SrvTest, dhcpDisableTemporarily) { TEST_F(CtrlChannelDhcpv6SrvTest, dhcpEnableBadParam) { createUnixChannelServer(); std::string response; + ConstElementPtr rsp; + + sendUnixCommand("{" + " \"command\": \"dhcp-enable\"," + " \"arguments\": {" + " \"origin-id\": \"\"" + " }" + "}", response); + + // The response should be a valid JSON. + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + ASSERT_TRUE(rsp); + + EXPECT_EQ("{ \"result\": 1, \"text\": \"'origin-id' argument must be a number\" }", response); + + sendUnixCommand("{" + " \"command\": \"dhcp-enable\"," + " \"arguments\": {" + " \"origin-id\": \"foo\"" + " }" + "}", response); + + // The response should be a valid JSON. + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + ASSERT_TRUE(rsp); + + EXPECT_EQ("{ \"result\": 1, \"text\": \"'origin-id' argument must be a number\" }", response); sendUnixCommand("{" " \"command\": \"dhcp-enable\"," @@ -1916,7 +2002,6 @@ TEST_F(CtrlChannelDhcpv6SrvTest, dhcpEnableBadParam) { " \"origin\": \"\"" " }" "}", response); - ConstElementPtr rsp; // The response should be a valid JSON. EXPECT_NO_THROW(rsp = Element::fromJSON(response)); @@ -2019,6 +2104,61 @@ TEST_F(CtrlChannelDhcpv6SrvTest, dhcpEnable) { EXPECT_TRUE(server_->network_state_->isServiceEnabled()); } +// This test verifies if it is possible to enable DHCP service using +// the origin-id. +TEST_F(CtrlChannelDhcpv6SrvTest, dhcpEnableOriginId) { + createUnixChannelServer(); + std::string response; + + ConstElementPtr rsp; + + int status; + + // Disable the service using two distinct origins. + server_->network_state_->disableService(NetworkState::HA_REMOTE_COMMAND+1); + server_->network_state_->disableService(NetworkState::USER_COMMAND); + + EXPECT_FALSE(server_->network_state_->isServiceEnabled()); + + // Enable the service for the 'origin-id' of 2001. The 'origin' should + // be ignored. + sendUnixCommand("{" + " \"command\": \"dhcp-enable\"," + " \"arguments\": {" + " \"origin-id\": 2001," + " \"origin\": \"user\"" + " }" + "}", response); + + // The response should be a valid JSON. + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + ASSERT_TRUE(rsp); + + ConstElementPtr cfg = parseAnswer(status, rsp); + EXPECT_EQ(CONTROL_RESULT_SUCCESS, status); + + // The service should still be disabled. + EXPECT_FALSE(server_->network_state_->isServiceEnabled()); + + // Enable the service for the user command. + sendUnixCommand("{" + " \"command\": \"dhcp-enable\"," + " \"arguments\": {" + " \"origin-id\": 1" + " }" + "}", response); + + // The response should be a valid JSON. + EXPECT_NO_THROW(rsp = Element::fromJSON(response)); + ASSERT_TRUE(rsp); + + cfg = parseAnswer(status, rsp); + EXPECT_EQ(CONTROL_RESULT_SUCCESS, status); + + // The service should be enabled. + EXPECT_TRUE(server_->network_state_->isServiceEnabled()); +} + /// Verify that concurrent connections over the control channel can be /// established. /// @todo Future Kea 1.3 tickets will modify the behavior of the CommandMgr diff --git a/src/hooks/dhcp/high_availability/command_creator.cc b/src/hooks/dhcp/high_availability/command_creator.cc index 467a3415b0..6f73764141 100644 --- a/src/hooks/dhcp/high_availability/command_creator.cc +++ b/src/hooks/dhcp/high_availability/command_creator.cc @@ -38,12 +38,14 @@ unordered_set CommandCreator::ha_commands6_ = { }; ConstElementPtr -CommandCreator::createDHCPDisable(const unsigned int origin, +CommandCreator::createDHCPDisable(const unsigned int origin_id, const unsigned int max_period, const HAServerType& server_type) { ElementPtr args; args = Element::createMap(); - args->set("origin", Element::create(origin)); + args->set("origin-id", Element::create(origin_id)); + // Add for backward compatibility with Kea 2.4.0 and earlier. + args->set("origin", Element::create("ha-partner")); // max-period is optional. A value of 0 means that it is not specified. if (max_period > 0) { args->set("max-period", Element::create(static_cast(max_period))); @@ -54,11 +56,13 @@ CommandCreator::createDHCPDisable(const unsigned int origin, } ConstElementPtr -CommandCreator::createDHCPEnable(const unsigned int origin, +CommandCreator::createDHCPEnable(const unsigned int origin_id, const HAServerType& server_type) { ElementPtr args; args = Element::createMap(); - args->set("origin", Element::create(origin)); + args->set("origin-id", Element::create(origin_id)); + // Add for backward compatibility with Kea 2.4.0 and earlier. + args->set("origin", Element::create("ha-partner")); ConstElementPtr command = config::createCommand("dhcp-enable", args); insertService(command, server_type); return (command); @@ -257,12 +261,14 @@ CommandCreator::createMaintenanceNotify(const std::string& server_name, } ConstElementPtr -CommandCreator::createSyncCompleteNotify(const unsigned int origin, +CommandCreator::createSyncCompleteNotify(const unsigned int origin_id, const std::string& server_name, const HAServerType& server_type) { auto args = Element::createMap(); args->set("server-name", Element::create(server_name)); - args->set("origin", Element::create(origin)); + args->set("origin-id", Element::create(origin_id)); + // Add for backward compatibility with Kea 2.4.0 and earlier. + args->set("origin", Element::create("ha-partner")); auto command = config::createCommand("ha-sync-complete-notify", args); insertService(command, server_type); return (command); diff --git a/src/hooks/dhcp/high_availability/command_creator.h b/src/hooks/dhcp/high_availability/command_creator.h index 829e81fd15..78ee81320b 100644 --- a/src/hooks/dhcp/high_availability/command_creator.h +++ b/src/hooks/dhcp/high_availability/command_creator.h @@ -24,7 +24,7 @@ public: /// @brief Creates dhcp-disable command for DHCP server. /// - /// @param origin A numeric value of the origin created from the + /// @param origin_id A numeric value of the origin created from the /// @c HAService identifier. /// @param max_period The max-period time the service can stay disabled /// before automatically transitioning to enabled state. @@ -32,19 +32,19 @@ public: /// /// @return Pointer to the JSON representation of the command. static data::ConstElementPtr - createDHCPDisable(const unsigned int origin, + createDHCPDisable(const unsigned int origin_id, const unsigned int max_period, const HAServerType& server_type); /// @brief Creates dhcp-enable command for DHCP server. /// - /// @param origin A numeric value of the origin created from the + /// @param origin_id A numeric value of the origin created from the /// @c HAService identifier. /// @param server_type type of the DHCP server, i.e. v4 or v6. /// /// @return Pointer to the JSON representation of the command. static data::ConstElementPtr - createDHCPEnable(const unsigned int origin, + createDHCPEnable(const unsigned int origin_id, const HAServerType& server_type); /// @brief Creates ha-reset command. @@ -185,14 +185,14 @@ public: /// @brief Creates ha-sync-complete-notify command. /// - /// @param origin a numeric value of the origin created from the + /// @param origin_id a numeric value of the origin created from the /// @c HAService identifier to enable the DHCP service. /// @param server_name name of the server sending the command allowing /// for associating the command with the relationship. /// @param server_type type of the DHCP server, i.e. v4 or v6. /// @return Pointer to the JSON representation of the command. static data::ConstElementPtr - createSyncCompleteNotify(const unsigned int origin, + createSyncCompleteNotify(const unsigned int origin_id, const std::string& server_name, const HAServerType& server_type); diff --git a/src/hooks/dhcp/high_availability/ha_impl.cc b/src/hooks/dhcp/high_availability/ha_impl.cc index 127ceb9545..d8ea4ee910 100644 --- a/src/hooks/dhcp/high_availability/ha_impl.cc +++ b/src/hooks/dhcp/high_availability/ha_impl.cc @@ -859,15 +859,24 @@ HAImpl::syncCompleteNotifyHandler(hooks::CalloutHandle& callout_handle) { static_cast(parseCommand(args, command)); HAServicePtr service; - auto origin_value = NetworkState::HA_REMOTE_COMMAND+1; + auto origin_id_value = NetworkState::HA_REMOTE_COMMAND+1; try { if (args) { + auto origin_id = args->get("origin-id"); auto origin = args->get("origin"); - if (origin) { + // The origin-id is a new parameter replacing the origin. However, some versions + // of Kea may still send the origin parameter instead. + if (origin_id) { + if (origin_id->getType() != Element::integer) { + isc_throw(BadValue, "'origin-id' must be an integer in the 'ha-sync-complete-notify' command"); + } + origin_id_value = origin_id->intValue(); + + } else if (origin) { if (origin->getType() != Element::integer) { isc_throw(BadValue, "'origin' must be an integer in the 'ha-sync-complete-notify' command"); } - origin_value = origin->intValue(); + origin_id_value = origin->intValue(); } } @@ -881,7 +890,7 @@ HAImpl::syncCompleteNotifyHandler(hooks::CalloutHandle& callout_handle) { return; } - ConstElementPtr response = service->processSyncCompleteNotify(origin_value); + ConstElementPtr response = service->processSyncCompleteNotify(origin_id_value); callout_handle.setArgument("response", response); } diff --git a/src/hooks/dhcp/high_availability/ha_service.cc b/src/hooks/dhcp/high_availability/ha_service.cc index 997ae1311a..ebd3e531fe 100644 --- a/src/hooks/dhcp/high_availability/ha_service.cc +++ b/src/hooks/dhcp/high_availability/ha_service.cc @@ -3076,7 +3076,7 @@ HAService::asyncSyncCompleteNotify(HttpClient& http_client, } ConstElementPtr -HAService::processSyncCompleteNotify(const unsigned int origin) { +HAService::processSyncCompleteNotify(const unsigned int origin_id) { if (getCurrState() == HA_PARTNER_DOWN_ST) { sync_complete_notified_ = true; // We're in the partner-down state and the partner notified us @@ -3091,7 +3091,7 @@ HAService::processSyncCompleteNotify(const unsigned int origin) { // Release the network state lock for the remote origin because we have // acquired the local network state lock above (partner-down state), or // we don't need the lock (other states). - network_state_->enableService(origin); + network_state_->enableService(origin_id); return (createAnswer(CONTROL_RESULT_SUCCESS, "Server successfully notified about the synchronization completion.")); } diff --git a/src/hooks/dhcp/high_availability/ha_service.h b/src/hooks/dhcp/high_availability/ha_service.h index a6de2a6227..0b6bc3a086 100644 --- a/src/hooks/dhcp/high_availability/ha_service.h +++ b/src/hooks/dhcp/high_availability/ha_service.h @@ -1121,10 +1121,10 @@ public: /// In this state, the server will first have to check connectivity with /// the partner and transition to a state in which it will send lease updates. /// - /// @param origin a numeric value of the origin created from the + /// @param origin_id a numeric value of the origin created from the /// @c HAService identifier to enable the DHCP service. /// @return Pointer to the response to the ha-sync-complete-notify command. - data::ConstElementPtr processSyncCompleteNotify(const unsigned int origin); + data::ConstElementPtr processSyncCompleteNotify(const unsigned int origin_id); /// @brief Start the client and(or) listener instances. /// diff --git a/src/hooks/dhcp/high_availability/tests/command_creator_unittest.cc b/src/hooks/dhcp/high_availability/tests/command_creator_unittest.cc index 46d76cce44..99f935fc4b 100644 --- a/src/hooks/dhcp/high_availability/tests/command_creator_unittest.cc +++ b/src/hooks/dhcp/high_availability/tests/command_creator_unittest.cc @@ -156,24 +156,30 @@ TEST(CommandCreatorTest, createDHCPDisable4) { ConstElementPtr arguments; ASSERT_NO_FATAL_FAILURE(testCommandBasics(command, "dhcp-disable", "dhcp4", arguments)); - ASSERT_EQ(2, arguments->size()); + ASSERT_EQ(3, arguments->size()); ConstElementPtr max_period = arguments->get("max-period"); ASSERT_TRUE(max_period); ASSERT_EQ(Element::integer, max_period->getType()); EXPECT_EQ(20, max_period->intValue()); + ConstElementPtr origin_id = arguments->get("origin-id"); + ASSERT_TRUE(origin_id); + ASSERT_EQ(NetworkState::HA_REMOTE_COMMAND+1, origin_id->intValue()); ConstElementPtr origin = arguments->get("origin"); ASSERT_TRUE(origin); - ASSERT_EQ(NetworkState::HA_REMOTE_COMMAND+1, origin->intValue()); + ASSERT_EQ("ha-partner", origin->stringValue()); // Repeat the test but this time the max-period is not specified. command = CommandCreator::createDHCPDisable(NetworkState::HA_REMOTE_COMMAND+1, 0, HAServerType::DHCPv4); ASSERT_NO_FATAL_FAILURE(testCommandBasics(command, "dhcp-disable", "dhcp4", arguments)); - ASSERT_EQ(1, arguments->size()); + ASSERT_EQ(2, arguments->size()); + origin_id = arguments->get("origin-id"); + ASSERT_TRUE(origin_id); + ASSERT_EQ(NetworkState::HA_REMOTE_COMMAND+1, origin_id->intValue()); origin = arguments->get("origin"); ASSERT_TRUE(origin); - ASSERT_EQ(NetworkState::HA_REMOTE_COMMAND+1, origin->intValue()); + ASSERT_EQ("ha-partner", origin->stringValue()); } // This test verifies that the dhcp-enable command is correct. @@ -183,10 +189,13 @@ TEST(CommandCreatorTest, createDHCPEnable4) { HAServerType::DHCPv4); ASSERT_NO_FATAL_FAILURE(testCommandBasics(command, "dhcp-enable", "dhcp4", arguments)); - ASSERT_EQ(1, arguments->size()); + ASSERT_EQ(2, arguments->size()); + ConstElementPtr origin_id = arguments->get("origin-id"); + ASSERT_TRUE(origin_id); + ASSERT_EQ(NetworkState::HA_REMOTE_COMMAND+1, origin_id->intValue()); ConstElementPtr origin = arguments->get("origin"); ASSERT_TRUE(origin); - ASSERT_EQ(NetworkState::HA_REMOTE_COMMAND+1, origin->intValue()); + ASSERT_EQ("ha-partner", origin->stringValue()); } // This test verifies that the ha-reset command sent to DHCPv4 server is correct. @@ -302,23 +311,29 @@ TEST(CommandCreatorTest, createDHCPDisable6) { ConstElementPtr arguments; ASSERT_NO_FATAL_FAILURE(testCommandBasics(command, "dhcp-disable", "dhcp6", arguments)); - ASSERT_EQ(2, arguments->size()); + ASSERT_EQ(3, arguments->size()); ConstElementPtr max_period = arguments->get("max-period"); ASSERT_TRUE(max_period); ASSERT_EQ(Element::integer, max_period->getType()); EXPECT_EQ(20, max_period->intValue()); + ConstElementPtr origin_id = arguments->get("origin-id"); + ASSERT_TRUE(origin_id); + ASSERT_EQ(NetworkState::HA_REMOTE_COMMAND+2, origin_id->intValue()); ConstElementPtr origin = arguments->get("origin"); ASSERT_TRUE(origin); - ASSERT_EQ(NetworkState::HA_REMOTE_COMMAND+2, origin->intValue()); + ASSERT_EQ("ha-partner", origin->stringValue()); // Repeat the test but this time the max-period is not specified. command = CommandCreator::createDHCPDisable(NetworkState::HA_REMOTE_COMMAND+2, 0, HAServerType::DHCPv6); ASSERT_NO_FATAL_FAILURE(testCommandBasics(command, "dhcp-disable", "dhcp6", arguments)); - ASSERT_EQ(1, arguments->size()); + ASSERT_EQ(2, arguments->size()); + origin_id = arguments->get("origin-id"); + ASSERT_TRUE(origin_id); + ASSERT_EQ(NetworkState::HA_REMOTE_COMMAND+2, origin_id->intValue()); origin = arguments->get("origin"); ASSERT_TRUE(origin); - ASSERT_EQ(NetworkState::HA_REMOTE_COMMAND+2, origin->intValue()); + ASSERT_EQ("ha-partner", origin->stringValue()); } // This test verifies that the dhcp-enable command (DHCPv6 case) is @@ -328,10 +343,13 @@ TEST(CommandCreatorTest, createDHCPEnable6) { ConstElementPtr command = CommandCreator::createDHCPEnable(NetworkState::HA_REMOTE_COMMAND+2, HAServerType::DHCPv6); ASSERT_NO_FATAL_FAILURE(testCommandBasics(command, "dhcp-enable", "dhcp6", arguments)); - ASSERT_EQ(1, arguments->size()); + ASSERT_EQ(2, arguments->size()); + ConstElementPtr origin_id = arguments->get("origin-id"); + ASSERT_TRUE(origin_id); + ASSERT_EQ(NetworkState::HA_REMOTE_COMMAND+2, origin_id->intValue()); ConstElementPtr origin = arguments->get("origin"); ASSERT_TRUE(origin); - ASSERT_EQ(NetworkState::HA_REMOTE_COMMAND+2, origin->intValue()); + ASSERT_EQ("ha-partner", origin->stringValue()); } // This test verifies that the ha-reset command sent to DHCPv6 server is correct. @@ -542,10 +560,10 @@ TEST(CommandCreatorTest, createSyncCompleteNotify4) { ASSERT_TRUE(server_name); ASSERT_EQ(Element::string, server_name->getType()); EXPECT_EQ("server1", server_name->stringValue()); - auto origin = arguments->get("origin"); - ASSERT_TRUE(origin); - EXPECT_EQ(Element::integer, origin->getType()); - EXPECT_EQ(1, origin->intValue()); + auto origin_id = arguments->get("origin-id"); + ASSERT_TRUE(origin_id); + EXPECT_EQ(Element::integer, origin_id->getType()); + EXPECT_EQ(1, origin_id->intValue()); } // This test verifies that the ha-sync-complete-notify command sent to a @@ -558,10 +576,10 @@ TEST(CommandCreatorTest, createSyncCompleteNotify6) { ASSERT_TRUE(server_name); ASSERT_EQ(Element::string, server_name->getType()); EXPECT_EQ("server1", server_name->stringValue()); - auto origin = arguments->get("origin"); - ASSERT_TRUE(origin); - EXPECT_EQ(Element::integer, origin->getType()); - EXPECT_EQ(1, origin->intValue()); + auto origin_id = arguments->get("origin-id"); + ASSERT_TRUE(origin_id); + EXPECT_EQ(Element::integer, origin_id->getType()); + EXPECT_EQ(1, origin_id->intValue()); } } diff --git a/src/hooks/dhcp/high_availability/tests/ha_impl_unittest.cc b/src/hooks/dhcp/high_availability/tests/ha_impl_unittest.cc index 302e1d2c87..1508b5cc72 100644 --- a/src/hooks/dhcp/high_availability/tests/ha_impl_unittest.cc +++ b/src/hooks/dhcp/high_availability/tests/ha_impl_unittest.cc @@ -2304,9 +2304,10 @@ TEST_F(HAImplTest, haHeartbeatBadServerName) { checkAnswer(response, CONTROL_RESULT_ERROR, "server5 matches no configured 'server-name'"); } -// Test ha-sync-complete-notify command handler with a specified server name. -TEST_F(HAImplTest, haSyncCompleteNotify) { - ha_impl_.reset(new HAImpl()); +// Test ha-sync-complete-notify command handler with a specified server name +// and origin-id. +TEST_F(HAImplTest, haSyncCompleteNotifyOriginId) { + ha_impl_.reset(new TestHAImpl()); ha_impl_->setIOService(io_service_); ASSERT_NO_THROW(ha_impl_->configure(createValidJsonConfiguration())); @@ -2319,7 +2320,8 @@ TEST_F(HAImplTest, haSyncCompleteNotify) { "{" " \"command\": \"ha-sync-complete-notify\"," " \"arguments\": {" - " \"server-name\": \"server1\"" + " \"server-name\": \"server1\"," + " \"origin-id\": 2002" " }" "}" ); @@ -2327,6 +2329,10 @@ TEST_F(HAImplTest, haSyncCompleteNotify) { CalloutHandlePtr callout_handle = HooksManager::createCalloutHandle(); callout_handle->setArgument("command", command); + // Disable the DHCP service for origin of 2002. Running the command + // handler should unlock it. + network_state->disableService(NetworkState::HA_REMOTE_COMMAND + 2); + ASSERT_NO_THROW(ha_impl_->syncCompleteNotifyHandler(*callout_handle)); ConstElementPtr response; @@ -2335,6 +2341,57 @@ TEST_F(HAImplTest, haSyncCompleteNotify) { checkAnswer(response, CONTROL_RESULT_SUCCESS, "Server successfully notified about the synchronization completion."); + + // The command handler should keep the service disabled under the HA_LOCAL_COMMAND. + // Let's try to enable it and see if it unlocks the DHCP service. + EXPECT_FALSE(network_state->isServiceEnabled()); + network_state->enableService(NetworkState::HA_LOCAL_COMMAND); + EXPECT_TRUE(network_state->isServiceEnabled()); +} + +// Test ha-sync-complete-notify command handler with a specified server name +// and origin. +TEST_F(HAImplTest, haSyncCompleteNotifyOrigin) { + ha_impl_.reset(new TestHAImpl()); + ha_impl_->setIOService(io_service_); + ASSERT_NO_THROW(ha_impl_->configure(createValidJsonConfiguration())); + + // Starting the service is required prior to running any callouts. + NetworkStatePtr network_state(new NetworkState(NetworkState::DHCPv4)); + ASSERT_NO_THROW(ha_impl_->startServices(network_state, + HAServerType::DHCPv4)); + + ConstElementPtr command = Element::fromJSON( + "{" + " \"command\": \"ha-sync-complete-notify\"," + " \"arguments\": {" + " \"server-name\": \"server1\"," + " \"origin\": 2002" + " }" + "}" + ); + + CalloutHandlePtr callout_handle = HooksManager::createCalloutHandle(); + callout_handle->setArgument("command", command); + + // Disable the DHCP service for origin of 2002. Running the command + // handler should unlock it. + network_state->disableService(NetworkState::HA_REMOTE_COMMAND + 2); + + ASSERT_NO_THROW(ha_impl_->syncCompleteNotifyHandler(*callout_handle)); + + ConstElementPtr response; + callout_handle->getArgument("response", response); + ASSERT_TRUE(response); + + checkAnswer(response, CONTROL_RESULT_SUCCESS, + "Server successfully notified about the synchronization completion."); + + // The command handler should keep the service disabled under the HA_LOCAL_COMMAND. + // Let's try to enable it and see if it unlocks the DHCP service. + EXPECT_FALSE(network_state->isServiceEnabled()); + network_state->enableService(NetworkState::HA_LOCAL_COMMAND); + EXPECT_TRUE(network_state->isServiceEnabled()); } // Test ha-sync-complete-notify command handler without a specified server name.