From: Thomas Markwalder Date: Thu, 17 Mar 2022 16:03:41 +0000 (-0400) Subject: [#2344] MySql CB use conditional bindings for lifetime triplets X-Git-Tag: Kea-2.1.4~28 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5be06a0eb46dc8ea604f0550548e5e37aa46ba40;p=thirdparty%2Fkea.git [#2344] MySql CB use conditional bindings for lifetime triplets src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc MySqlConfigBackendDHCPv4Impl::createUpdateClientClass4() - use createBinding() to conditionally create bindings for valid lifetime triplet src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc MySqlConfigBackendDHCPv6Impl::createUpdateClientClass6() - use createBinding() to conditionally create bindings for valid lifetime and preferred lifetime triplets src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc GenericConfigBackendDHCPv4Test::initTestClientClasses() - set class option defs to empty def collection to match the way we actually create them GenericConfigBackendDHCPv4Test::setAndGetAllClientClasses4Test() - test now compares fetched class content to source class content src/lib/dhcpsrv/testutils/generic_cb_dhcp6_unittest.cc GenericConfigBackendDHCPv6Test::SetUp() - set CfgMgr family GenericConfigBackendDHCPv6Test::initTestClientClasses() - set class option defs to empty def collection to match the way we actually create them, also set preferred lifetime triplet GenericConfigBackendDHCPv6Test::setAndGetAllClientClasses4Test() - test now compares fetched class content to source class content --- diff --git a/ChangeLog b/ChangeLog index 14167ec81e..44621386b2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2002. [bug] tmark + Fixed a bug in MySql config back end that caused it to + store unspecified, client-class valid and preferred life + time values as zero in the database. + (Gitlab #2344) + 2001. [bug] razvan Fixed a bug which causes client classes with empty test expressions to fail class evaluation when those classes are diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc index 1b1a064f5b..c4db02c833 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp4.cc @@ -2605,9 +2605,9 @@ public: MySqlBinding::createString(client_class->getSname()), MySqlBinding::createString(client_class->getFilename()), MySqlBinding::createBool(client_class->getRequired()), - MySqlBinding::createInteger(client_class->getValid()), - MySqlBinding::createInteger(client_class->getValid().getMin()), - MySqlBinding::createInteger(client_class->getValid().getMax()), + createBinding(client_class->getValid()), + createMinBinding(client_class->getValid()), + createMaxBinding(client_class->getValid()), MySqlBinding::createBool(depend_on_known), (follow_class_name.empty() ? MySqlBinding::createNull() : MySqlBinding::createString(follow_class_name)), diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc index 354a56ceea..8213250c62 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_dhcp6.cc @@ -3035,15 +3035,15 @@ public: MySqlBinding::createString(client_class->getName()), MySqlBinding::createString(client_class->getTest()), MySqlBinding::createBool(client_class->getRequired()), - MySqlBinding::createInteger(client_class->getValid()), - MySqlBinding::createInteger(client_class->getValid().getMin()), - MySqlBinding::createInteger(client_class->getValid().getMax()), + createBinding(client_class->getValid()), + createMinBinding(client_class->getValid()), + createMaxBinding(client_class->getValid()), MySqlBinding::createBool(depend_on_known), (follow_class_name.empty() ? MySqlBinding::createNull() : MySqlBinding::createString(follow_class_name)), - MySqlBinding::createInteger(client_class->getPreferred()), - MySqlBinding::createInteger(client_class->getPreferred().getMin()), - MySqlBinding::createInteger(client_class->getPreferred().getMax()), + createBinding(client_class->getPreferred()), + createMinBinding(client_class->getPreferred()), + createMaxBinding(client_class->getPreferred()), MySqlBinding::createTimestamp(client_class->getModificationTime()), }; diff --git a/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc b/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc index ab5ef14063..5ab0265656 100644 --- a/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc +++ b/src/lib/dhcpsrv/testutils/generic_cb_dhcp4_unittest.cc @@ -380,6 +380,7 @@ GenericConfigBackendDHCPv4Test::initTestClientClasses() { ExpressionPtr match_expr = boost::make_shared(); CfgOptionPtr cfg_option = boost::make_shared(); auto class1 = boost::make_shared("foo", match_expr, cfg_option); + class1->setCfgOptionDef(boost::make_shared()); class1->setRequired(true); class1->setNextServer(IOAddress("1.2.3.4")); class1->setSname("cool"); @@ -388,10 +389,12 @@ GenericConfigBackendDHCPv4Test::initTestClientClasses() { test_client_classes_.push_back(class1); auto class2 = boost::make_shared("bar", match_expr, cfg_option); + class2->setCfgOptionDef(boost::make_shared()); class2->setTest("member('foo')"); test_client_classes_.push_back(class2); auto class3 = boost::make_shared("foobar", match_expr, cfg_option); + class3->setCfgOptionDef(boost::make_shared()); class3->setTest("member('foo') and member('bar')"); test_client_classes_.push_back(class3); } @@ -4011,12 +4014,21 @@ GenericConfigBackendDHCPv4Test::setAndGetAllClientClasses4Test() { client_classes = cbptr_->getAllClientClasses4(ServerSelector::ONE("server1")); auto classes_list = client_classes.getClasses(); ASSERT_EQ(3, classes_list->size()); - EXPECT_EQ("foo", (*classes_list->begin())->getName()); - EXPECT_FALSE((*classes_list->begin())->getMatchExpr()); - EXPECT_EQ("bar", (*(classes_list->begin() + 1))->getName()); - EXPECT_FALSE((*(classes_list->begin() + 1))->getMatchExpr()); - EXPECT_EQ("foobar", (*(classes_list->begin() + 2))->getName()); - EXPECT_FALSE((*(classes_list->begin() + 2))->getMatchExpr()); + + auto fetched_class = classes_list->begin(); + ASSERT_EQ("foo", (*fetched_class)->getName()); + EXPECT_FALSE((*fetched_class)->getMatchExpr()); + EXPECT_EQ(class1->toElement()->str(), (*fetched_class)->toElement()->str()); + + ++fetched_class; + ASSERT_EQ("bar", (*fetched_class)->getName()); + EXPECT_FALSE((*fetched_class)->getMatchExpr()); + EXPECT_EQ(class2->toElement()->str(), (*fetched_class)->toElement()->str()); + + ++fetched_class; + ASSERT_EQ("foobar", (*fetched_class)->getName()); + EXPECT_FALSE((*fetched_class)->getMatchExpr()); + EXPECT_EQ(class3->toElement()->str(), (*fetched_class)->toElement()->str()); // Move the third class between the first and second class. ASSERT_NO_THROW_LOG(cbptr_->createUpdateClientClass4(ServerSelector::ONE("server1"), class3, "foo")); diff --git a/src/lib/dhcpsrv/testutils/generic_cb_dhcp6_unittest.cc b/src/lib/dhcpsrv/testutils/generic_cb_dhcp6_unittest.cc index 61304b8d7d..e2c782938f 100644 --- a/src/lib/dhcpsrv/testutils/generic_cb_dhcp6_unittest.cc +++ b/src/lib/dhcpsrv/testutils/generic_cb_dhcp6_unittest.cc @@ -42,6 +42,8 @@ namespace ph = std::placeholders; void GenericConfigBackendDHCPv6Test::SetUp() { + CfgMgr::instance().setFamily(AF_INET6); + // Ensure we have the proper schema with no transient data. createSchema(); @@ -415,15 +417,19 @@ GenericConfigBackendDHCPv6Test::initTestClientClasses() { ExpressionPtr match_expr = boost::make_shared(); CfgOptionPtr cfg_option = boost::make_shared(); auto class1 = boost::make_shared("foo", match_expr, cfg_option); + class1->setCfgOptionDef(boost::make_shared()); class1->setRequired(true); class1->setValid(Triplet(30, 60, 90)); + class1->setPreferred(Triplet(25, 55, 85)); test_client_classes_.push_back(class1); auto class2 = boost::make_shared("bar", match_expr, cfg_option); + class2->setCfgOptionDef(boost::make_shared()); class2->setTest("member('foo')"); test_client_classes_.push_back(class2); auto class3 = boost::make_shared("foobar", match_expr, cfg_option); + class3->setCfgOptionDef(boost::make_shared()); class3->setTest("member('foo') and member('bar')"); test_client_classes_.push_back(class3); } @@ -4167,12 +4173,21 @@ GenericConfigBackendDHCPv6Test::setAndGetAllClientClasses6Test() { client_classes = cbptr_->getAllClientClasses6(ServerSelector::ONE("server1")); auto classes_list = client_classes.getClasses(); ASSERT_EQ(3, classes_list->size()); - EXPECT_EQ("foo", (*classes_list->begin())->getName()); - EXPECT_FALSE((*classes_list->begin())->getMatchExpr()); - EXPECT_EQ("bar", (*(classes_list->begin() + 1))->getName()); - EXPECT_FALSE((*(classes_list->begin() + 1))->getMatchExpr()); - EXPECT_EQ("foobar", (*(classes_list->begin() + 2))->getName()); - EXPECT_FALSE((*(classes_list->begin() + 2))->getMatchExpr()); + + auto fetched_class = classes_list->begin(); + ASSERT_EQ("foo", (*fetched_class)->getName()); + EXPECT_FALSE((*fetched_class)->getMatchExpr()); + EXPECT_EQ(class1->toElement()->str(), (*fetched_class)->toElement()->str()); + + ++fetched_class; + ASSERT_EQ("bar", (*fetched_class)->getName()); + EXPECT_FALSE((*fetched_class)->getMatchExpr()); + EXPECT_EQ(class2->toElement()->str(), (*fetched_class)->toElement()->str()); + + ++fetched_class; + ASSERT_EQ("foobar", (*fetched_class)->getName()); + EXPECT_FALSE((*fetched_class)->getMatchExpr()); + EXPECT_EQ(class3->toElement()->str(), (*fetched_class)->toElement()->str()); // Move the third class between the first and second class. ASSERT_NO_THROW_LOG(cbptr_->createUpdateClientClass6(ServerSelector::ONE("server1"), class3, "foo"));