From: Thomas Markwalder Date: Mon, 21 Feb 2022 14:10:18 +0000 (-0500) Subject: [#2301] Backport 2246 to v2_0 X-Git-Tag: Kea-2.0.2~4 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=4f471de929163c276f39d7ad9b6c614e5d95e123;p=thirdparty%2Fkea.git [#2301] Backport 2246 to v2_0 Creates class options when loading classes from CB. --- diff --git a/ChangeLog b/ChangeLog index f830b584e8..5cd201caf8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +1956. [bug] tmark + Kea core logic now ensures options belonging + to client classes are properly created when + classes are read from configuration backends. + (Gitlab #2301) + 1955. [bug] andrei The config for an HA peer now accepts an IPv6 address as a valid value for the "url" entry. diff --git a/src/lib/dhcpsrv/cb_ctl_dhcp4.cc b/src/lib/dhcpsrv/cb_ctl_dhcp4.cc index 3c53d05223..35e18da193 100644 --- a/src/lib/dhcpsrv/cb_ctl_dhcp4.cc +++ b/src/lib/dhcpsrv/cb_ctl_dhcp4.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2019-2021 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2019-2022 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -206,6 +206,10 @@ CBControlDHCPv4::databaseConfigApply(const BackendSelector& backend_selector, // Match expressions are not initialized for classes returned from the config backend. // We have to ensure to initialize them before they can be used by the server. client_classes.initMatchExpr(AF_INET); + + // Class options also need to be created when returned from the config backend. + client_classes.createOptions(external_cfg->getCfgOptionDef()); + external_cfg->setClientClassDictionary(boost::make_shared(client_classes)); } diff --git a/src/lib/dhcpsrv/cb_ctl_dhcp6.cc b/src/lib/dhcpsrv/cb_ctl_dhcp6.cc index 2713db2201..ff9e8d1d4d 100644 --- a/src/lib/dhcpsrv/cb_ctl_dhcp6.cc +++ b/src/lib/dhcpsrv/cb_ctl_dhcp6.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2019-2021 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2019-2022 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -205,6 +205,10 @@ CBControlDHCPv6::databaseConfigApply(const db::BackendSelector& backend_selector // Match expressions are not initialized for classes returned from the config backend. // We have to ensure to initialize them before they can be used by the server. client_classes.initMatchExpr(AF_INET6); + + // Class options also need to be created when returned from the config backend. + client_classes.createOptions(external_cfg->getCfgOptionDef()); + external_cfg->setClientClassDictionary(boost::make_shared(client_classes)); } diff --git a/src/lib/dhcpsrv/client_class_def.cc b/src/lib/dhcpsrv/client_class_def.cc index cbc2fede57..308fd0ae8b 100644 --- a/src/lib/dhcpsrv/client_class_def.cc +++ b/src/lib/dhcpsrv/client_class_def.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2015-2021 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2022 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -406,6 +406,46 @@ ClientClassDictionary::initMatchExpr(uint16_t family) { } } +void +ClientClassDictionary::createOptions(const CfgOptionDefPtr& external_defs) { + for (auto c : *list_) { + // If the class has no options, skip it. + CfgOptionPtr class_options = c->getCfgOption(); + if (!class_options || class_options->empty()) { + continue; + } + + // If the class has no option definitions, use the set + // of definitions we were given as is to create its + // options. + if (!c->getCfgOptionDef()) { + class_options->createOptions(external_defs); + } else { + // Class has its own option definitions, we need a + // composite set of definitions to recreate its options. + // We make copies of both sets of definitions, then merge + // the external defs copy into the class defs copy. + // We do this because merging actually effects both sets + // of definitions and we cannot alter either set. + // Seed the composite set with the class's definitions. + CfgOptionDefPtr composite_defs(new CfgOptionDef()); + c->getCfgOptionDef()->copyTo(*composite_defs); + + // Make a copy of the external definitions and + // merge those into the composite set. This should give + // us a set of options with class definitions taking + // precedence. + CfgOptionDefPtr external_defs_copy(new CfgOptionDef()); + external_defs->copyTo(*external_defs_copy); + composite_defs->merge(*external_defs_copy); + + // Now create the class options using the composite + // set of definitions. + class_options->createOptions(composite_defs); + } + } +} + ElementPtr ClientClassDictionary::toElement() const { ElementPtr result = Element::createList(); diff --git a/src/lib/dhcpsrv/client_class_def.h b/src/lib/dhcpsrv/client_class_def.h index 012f3e9c39..b052aab180 100644 --- a/src/lib/dhcpsrv/client_class_def.h +++ b/src/lib/dhcpsrv/client_class_def.h @@ -1,4 +1,4 @@ -// Copyright (C) 2015-2021 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2022 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -403,6 +403,12 @@ public: /// @param family Class universe, e.g. AF_INET or AF_INET6. void initMatchExpr(uint16_t family); + /// @brief Iterates over the classes in the dictionary and recreates + /// the options. + /// + /// @param cfg_option_def set of option definitions to use. + void createOptions(const CfgOptionDefPtr& cfg_option_def); + /// @brief Equality operator. /// /// @param other Other client class dictionary to compare to. diff --git a/src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc b/src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc index e6318b1336..a51909aeba 100644 --- a/src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc +++ b/src/lib/dhcpsrv/tests/cb_ctl_dhcp_unittest.cc @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -385,6 +386,29 @@ public: client_class->setTest("substring(option[1].hex, 0, 8) == 'my-value'"); client_class->setId(1); client_class->setModificationTime(getTimestamp("dhcp4_client_class")); + + // Add a standard option to the class. + OptionPtr option = Option::create(Option::V4, DHO_BOOT_FILE_NAME); + OptionDescriptorPtr desc = OptionDescriptor::create(option, true, "bogus-file.txt"); + desc->space_name_ = DHCP4_OPTION_SPACE; + desc->setModificationTime(getTimestamp("dhcp4_client_class")); + client_class->getCfgOption()->add(*desc, desc->space_name_); + + // Add a custom option definition to the class. + CfgOptionDefPtr cc_cfg_option_def(new CfgOptionDef()); + def.reset(new OptionDefinition("v4str", 201, "isc", "string")); + def->setId(201); + def->setModificationTime(getTimestamp("dhcp4_client_class")); + cc_cfg_option_def->add(def); + client_class->setCfgOptionDef(cc_cfg_option_def); + + // Add a custom option to the class. + option = Option::create(Option::V4, 201); + desc = OptionDescriptor::create(option, true, "custom-stuff"); + desc->space_name_ = "isc"; + desc->setModificationTime(getTimestamp("dhcp4_client_class")); + client_class->getCfgOption()->add(*desc, desc->space_name_); + mgr.getPool()->createUpdateClientClass4(BackendSelector::UNSPEC(), ServerSelector::ALL(), client_class, ""); @@ -515,8 +539,8 @@ public: ASSERT_FALSE(audit_entries_.empty()) << "Require at least one audit entry. The test is broken!"; - ctl_.databaseConfigApply(BackendSelector::UNSPEC(), ServerSelector::ALL(), - lb_modification_time, audit_entries_); + ASSERT_NO_THROW_LOG(ctl_.databaseConfigApply(BackendSelector::UNSPEC(), ServerSelector::ALL(), + lb_modification_time, audit_entries_)); // The updates should have been merged into current configuration. auto srv_cfg = CfgMgr::instance().getCurrentCfg(); @@ -597,6 +621,23 @@ public: EXPECT_GT(found_class->getMatchExpr()->size(), 0); EXPECT_EQ("first-class", found_class->getName()); + // Check for the standard class option, make sure it has been "created". + auto cfg_option_desc = found_class->getCfgOption(); + ASSERT_TRUE(cfg_option_desc); + auto option_desc = cfg_option_desc->get(DHCP4_OPTION_SPACE, DHO_BOOT_FILE_NAME); + auto option = option_desc.option_; + ASSERT_TRUE(option); + EXPECT_EQ(OptionBuffer(option_desc.formatted_value_.begin(), + option_desc.formatted_value_.end()), + option->getData()); + + // Check for the custom class option, make sure it has been "created". + option_desc = cfg_option_desc->get("isc", 201); + option = option_desc.option_; + ASSERT_TRUE(option); + EXPECT_EQ(OptionBuffer(option_desc.formatted_value_.begin(), + option_desc.formatted_value_.end()), + option->getData()); } else { EXPECT_FALSE(found_class); } @@ -1181,6 +1222,14 @@ public: client_class->setTest("substring(option[1].hex, 0, 8) == 'my-value'"); client_class->setId(1); client_class->setModificationTime(getTimestamp("dhcp6_client_class")); + + // Add an option to the class. + OptionPtr option = Option::create(Option::V6, D6O_BOOTFILE_URL); + OptionDescriptorPtr desc = OptionDescriptor::create(option, true, "client.boot.url"); + desc->space_name_ = DHCP6_OPTION_SPACE; + desc->setModificationTime(getTimestamp("dhcp6_client_class")); + client_class->getCfgOption()->add(*desc, desc->space_name_); + mgr.getPool()->createUpdateClientClass6(BackendSelector::UNSPEC(), ServerSelector::ALL(), client_class, ""); @@ -1393,6 +1442,15 @@ public: EXPECT_GT(found_class->getMatchExpr()->size(), 0); EXPECT_EQ("first-class", found_class->getName()); + // Check for class option, make sure it has been "created". + auto cfg_option_desc = found_class->getCfgOption(); + ASSERT_TRUE(cfg_option_desc); + auto option_desc = cfg_option_desc->get(DHCP6_OPTION_SPACE, D6O_BOOTFILE_URL); + auto option = option_desc.option_; + ASSERT_TRUE(option); + EXPECT_EQ(OptionBuffer(option_desc.formatted_value_.begin(), + option_desc.formatted_value_.end()), + option->getData()); } else { EXPECT_FALSE(found_class); } diff --git a/src/lib/dhcpsrv/tests/client_class_def_unittest.cc b/src/lib/dhcpsrv/tests/client_class_def_unittest.cc index d6ac1d5aa1..15b5d9697d 100644 --- a/src/lib/dhcpsrv/tests/client_class_def_unittest.cc +++ b/src/lib/dhcpsrv/tests/client_class_def_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2015-2021 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2022 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -731,4 +731,77 @@ TEST(ClientClassDictionary, unparseDict) { runToElementTest(expected, *dictionary); } +// Tests that options have been created for all client classes in the +// dictionary. +TEST(ClientClassDictionary, createOptions) { + ClientClassDictionaryPtr dictionary(new ClientClassDictionary()); + ExpressionPtr expr; + CfgOptionPtr cfg_option; + + // First class has no options. + ASSERT_NO_THROW(dictionary->addClass("foo", expr, "", false, + false, cfg_option)); + + // Make some options for the second class. + cfg_option.reset(new CfgOption()); + OptionPtr option = Option::create(Option::V4, DHO_BOOT_FILE_NAME); + OptionDescriptorPtr desc = OptionDescriptor::create(option, true, "bogus-file.txt"); + desc->space_name_ = DHCP4_OPTION_SPACE; + cfg_option->add(*desc, desc->space_name_); + + option = Option::create(Option::V4, DHO_TFTP_SERVER_NAME); + desc = OptionDescriptor::create(option, true, "bogus-tftp-server"); + desc->space_name_ = DHCP4_OPTION_SPACE; + cfg_option->add(*desc, desc->space_name_); + + // Add the second class with options. + ASSERT_NO_THROW(dictionary->addClass("bar", expr, "", false, + false, cfg_option)); + + // Make sure first class has no options. + ASSERT_TRUE(dictionary->getClasses()); + auto classes = *(dictionary->getClasses()); + auto options = classes[0]->getCfgOption(); + ASSERT_TRUE(options->empty()); + + // Make sure second class has both options but their + // data buffers are empty. + options = classes[1]->getCfgOption(); + ASSERT_FALSE(options->empty()); + + auto option_desc = options->get("dhcp4", DHO_BOOT_FILE_NAME); + ASSERT_TRUE(option_desc.option_); + ASSERT_TRUE(option_desc.option_->getData().empty()); + + option_desc = options->get("dhcp4", DHO_TFTP_SERVER_NAME); + ASSERT_TRUE(option_desc.option_); + ASSERT_TRUE(option_desc.option_->getData().empty()); + + // Now create match expressions for all of them. + auto cfg_def = CfgMgr::instance().getCurrentCfg()->getCfgOptionDef(); + ASSERT_NO_THROW(dictionary->createOptions(cfg_def)); + + // Make sure first class still has no options. + classes = *(dictionary->getClasses()); + options = classes[0]->getCfgOption(); + ASSERT_TRUE(options->empty()); + + // Make sure second class has both options and that their + // data buffers are now correctly populated. + options = classes[1]->getCfgOption(); + ASSERT_FALSE(options->empty()); + + option_desc = options->get("dhcp4", DHO_BOOT_FILE_NAME); + option = option_desc.option_; + ASSERT_TRUE(option); + EXPECT_EQ(OptionBuffer(option_desc.formatted_value_.begin(), option_desc.formatted_value_.end()), + option->getData()); + + option_desc = options->get("dhcp4", DHO_TFTP_SERVER_NAME); + option = option_desc.option_; + ASSERT_TRUE(option); + EXPECT_EQ(OptionBuffer(option_desc.formatted_value_.begin(), option_desc.formatted_value_.end()), + option->getData()); +} + } // end of anonymous namespace