From: Marcin Siodelski Date: Mon, 12 Jul 2021 12:18:36 +0000 (+0200) Subject: [#1928] Do not re-position DHCPv6 class on update X-Git-Tag: Kea-1.9.10~71 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d413404d990b669fe958bef87823b4554bc05ab7;p=thirdparty%2Fkea.git [#1928] Do not re-position DHCPv6 class on update --- diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc index fb85fefac5..703a40439e 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc @@ -136,6 +136,7 @@ public: UPDATE_OPTION6_SHARED_NETWORK, UPDATE_OPTION6_CLIENT_CLASS, UPDATE_CLIENT_CLASS6, + UPDATE_CLIENT_CLASS6_SAME_POSITION, UPDATE_SERVER6, DELETE_GLOBAL_PARAMETER6, DELETE_ALL_GLOBAL_PARAMETERS6, @@ -3016,13 +3017,13 @@ public: /// /// @param server_selector Server selector. /// @param client_class Pointer to the upserted client class. - /// @param follow_client_class name of the class after which the + /// @param follow_class_name name of the class after which the /// new or updated class should be positioned. An empty value /// causes the class to be appended at the end of the class /// hierarchy. void createUpdateClientClass6(const ServerSelector& server_selector, const ClientClassDefPtr& client_class, - const std::string& follow_client_class) { + const std::string& follow_class_name) { // We need to evaluate class expression to see if it references any // other classes (dependencies). As part of this evaluation we will // also check if the client class depends on KNOWN/UNKNOWN built-in @@ -3062,8 +3063,8 @@ public: MySqlBinding::createInteger(client_class->getValid().getMin()), MySqlBinding::createInteger(client_class->getValid().getMax()), MySqlBinding::createBool(depend_on_known), - (follow_client_class.empty() ? MySqlBinding::createNull() : - MySqlBinding::createString(follow_client_class)), + (follow_class_name.empty() ? MySqlBinding::createNull() : + MySqlBinding::createString(follow_class_name)), MySqlBinding::createTimestamp(client_class->getModificationTime()), }; @@ -3086,7 +3087,18 @@ public: // Try to update the class. in_bindings.push_back(MySqlBinding::createString(client_class->getName())); - conn_.updateDeleteQuery(MySqlConfigBackendDHCPv6Impl::UPDATE_CLIENT_CLASS6, in_bindings); + if (follow_class_name.empty()) { + // If position is not specified, leave the class at the same position. + // Remove the binding which specifies the position and use different + // query. + in_bindings.erase(in_bindings.begin() + 7, in_bindings.begin() + 8); + conn_.updateDeleteQuery(MySqlConfigBackendDHCPv6Impl::UPDATE_CLIENT_CLASS6_SAME_POSITION, + in_bindings); + } else { + // Update with specifying the position. + conn_.updateDeleteQuery(MySqlConfigBackendDHCPv6Impl::UPDATE_CLIENT_CLASS6, + in_bindings); + } // Delete class associations with the servers and dependencies. We will re-create // them according to the new class specification. @@ -3877,18 +3889,14 @@ TaggedStatementArray tagged_statements = { { MYSQL_UPDATE_OPTION6_NO_TAG(o.scope_id = 2 AND o.dhcp_client_class = ? AND o.code = ? AND o.space = ?) }, + // Update existing client class with specifying its position. { MySqlConfigBackendDHCPv6Impl::UPDATE_CLIENT_CLASS6, - "UPDATE dhcp6_client_class SET" - " name = ?," - " test = ?," - " only_if_required = ?," - " valid_lifetime = ?," - " min_valid_lifetime = ?," - " max_valid_lifetime = ?," - " depend_on_known_directly = ?," - " follow_class_name = ?," - " modification_ts = ? " - "WHERE name = ?" + MYSQL_UPDATE_CLIENT_CLASS6("follow_class_name = ?,") + }, + + // Update existing client class without specifying its position. + { MySqlConfigBackendDHCPv6Impl::UPDATE_CLIENT_CLASS6_SAME_POSITION, + MYSQL_UPDATE_CLIENT_CLASS6("") }, // Update existing server, e.g. server description. @@ -4459,10 +4467,10 @@ MySqlConfigBackendDHCPv6::createUpdateGlobalParameter6(const ServerSelector& ser void MySqlConfigBackendDHCPv6::createUpdateClientClass6(const db::ServerSelector& server_selector, const ClientClassDefPtr& client_class, - const std::string& follow_client_class) { + const std::string& follow_class_name) { LOG_DEBUG(mysql_cb_logger, DBGLVL_TRACE_BASIC, MYSQL_CB_CREATE_UPDATE_CLIENT_CLASS6) .arg(client_class->getName()); - impl_->createUpdateClientClass6(server_selector, client_class, follow_client_class); + impl_->createUpdateClientClass6(server_selector, client_class, follow_class_name); } void diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.h b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.h index 8c80741e3b..4ab608c9f4 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.h +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.h @@ -383,14 +383,14 @@ public: /// /// @param server_selector Server selector. /// @param client_class Client class to be added or updated. - /// @param follow_client_class name of the class after which the + /// @param follow_class_name name of the class after which the /// new or updated class should be positioned. An empty value /// causes the class to be appended at the end of the class /// hierarchy. virtual void createUpdateClientClass6(const db::ServerSelector& server_selector, const ClientClassDefPtr& client_class, - const std::string& follow_client_class); + const std::string& follow_class_name); /// @brief Creates or updates a server. /// diff --git a/src/hooks/dhcp/mysql_cb/mysql_query_macros_dhcp.h b/src/hooks/dhcp/mysql_cb/mysql_query_macros_dhcp.h index 7808fd5be9..4754869ef3 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_query_macros_dhcp.h +++ b/src/hooks/dhcp/mysql_cb/mysql_query_macros_dhcp.h @@ -1077,6 +1077,21 @@ namespace { "WHERE name = ?" #endif +#ifndef MYSQL_UPDATE_CLIENT_CLASS6 +#define MYSQL_UPDATE_CLIENT_CLASS6(follow_class_name_set) \ + "UPDATE dhcp6_client_class SET" \ + " name = ?," \ + " test = ?," \ + " only_if_required = ?," \ + " valid_lifetime = ?," \ + " min_valid_lifetime = ?," \ + " max_valid_lifetime = ?," \ + " depend_on_known_directly = ?," \ + follow_class_name_set \ + " modification_ts = ? " \ + "WHERE name = ?" +#endif + #ifndef MYSQL_UPDATE_SERVER #define MYSQL_UPDATE_SERVER(table_prefix) \ "UPDATE " #table_prefix "_server " \ diff --git a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc index a9bb548557..035ce5d8a4 100644 --- a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc +++ b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp6_unittest.cc @@ -4385,7 +4385,6 @@ TEST_F(MySqlConfigBackendDHCPv6Test, setAndGetAllClientClasses6) { EXPECT_EQ("bar", (*(classes_list->begin() + 1))->getName()); EXPECT_EQ("foobar", (*(classes_list->begin() + 2))->getName()); - // Move the third class between the first and second class. ASSERT_NO_THROW(cbptr_->createUpdateClientClass6(ServerSelector::ONE("server1"), class3, "foo")); @@ -4396,6 +4395,17 @@ TEST_F(MySqlConfigBackendDHCPv6Test, setAndGetAllClientClasses6) { EXPECT_EQ("foo", (*classes_list->begin())->getName()); EXPECT_EQ("foobar", (*(classes_list->begin() + 1))->getName()); EXPECT_EQ("bar", (*(classes_list->begin() + 2))->getName()); + + // Update the foobar class without specifying its position. It should not + // be moved. + ASSERT_NO_THROW(cbptr_->createUpdateClientClass6(ServerSelector::ONE("server1"), class3, "")); + + client_classes = cbptr_->getAllClientClasses6(ServerSelector::ONE("server1")); + classes_list = client_classes.getClasses(); + ASSERT_EQ(3, classes_list->size()); + EXPECT_EQ("foo", (*classes_list->begin())->getName()); + EXPECT_EQ("foobar", (*(classes_list->begin() + 1))->getName()); + EXPECT_EQ("bar", (*(classes_list->begin() + 2))->getName()); } // This test verifies that a single class can be retrieved from the database. @@ -4871,6 +4881,11 @@ TEST_F(MySqlConfigBackendDHCPv6Test, clientClassDependencies6) { class3->setTest("member('bar')"); EXPECT_NO_THROW(cbptr_->createUpdateClientClass6(ServerSelector::ALL(), class3, "")); + // An attempt to move the first class to the end of the class hierarchy should + // fail because other classes depend on it. + EXPECT_THROW(cbptr_->createUpdateClientClass6(ServerSelector::ALL(), class1, "bar"), + DbOperationError); + // Try to change the dependency of the first class. There are other classes // having indirect dependency on KNOWN class via this class. Therefore, the // update should be unsuccessful. diff --git a/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.cc b/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.cc index db87fac7d7..8a4b2623af 100644 --- a/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.cc +++ b/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.cc @@ -591,7 +591,7 @@ TestConfigBackendDHCPv6::getModifiedClientClasses6(const db::ServerSelector& ser void TestConfigBackendDHCPv6::createUpdateClientClass6(const db::ServerSelector& server_selector, const ClientClassDefPtr& client_class, - const std::string& follow_client_class) { + const std::string&) { mergeServerTags(client_class, server_selector); auto existing_class = classes_.findClass(client_class->getName()); diff --git a/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.h b/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.h index 012120194b..1c6b1d6abd 100644 --- a/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.h +++ b/src/lib/dhcpsrv/testutils/test_config_backend_dhcp6.h @@ -360,14 +360,14 @@ public: /// /// @param server_selector Server selector. /// @param client_class Client class to be added or updated. - /// @param follow_client_class name of the class after which the + /// @param follow_class_name name of the class after which the /// new or updated class should be positioned. An empty value /// causes the class to be appended at the end of the class /// hierarchy. virtual void createUpdateClientClass6(const db::ServerSelector& server_selector, const ClientClassDefPtr& client_class, - const std::string& follow_client_class); + const std::string& follow_class_name); /// @brief Creates or updates a server. ///