From: Marcin Siodelski Date: Mon, 11 Mar 2019 10:15:48 +0000 (+0100) Subject: [#521,!270] Removed dependency of the MySQL CB on LibDHCP option defs. X-Git-Tag: Kea-1.6.0-beta~378 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5b2c6da4f7b0c6b8c7c79c689bf4adf3edb94578;p=thirdparty%2Fkea.git [#521,!270] Removed dependency of the MySQL CB on LibDHCP option defs. --- diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc index cf74f1231c..0e104120e5 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc @@ -8,11 +8,8 @@ #include #include #include -#include #include #include -#include -#include #include #include #include @@ -605,23 +602,6 @@ MySqlConfigBackendImpl::processOptionRow(const Option::Universe& universe, code = (*(first_binding + 1))->getInteger(); } - // See if the option has standard definition. - OptionDefinitionPtr def = LibDHCP::getOptionDef(space, code); - // If there is no definition but the option is vendor specific, - // we should search the definition within the vendor option space. - if (!def && (space != DHCP4_OPTION_SPACE) && (space != DHCP6_OPTION_SPACE)) { - uint32_t vendor_id = LibDHCP::optionSpaceToVendorId(space); - if (vendor_id > 0) { - def = LibDHCP::getVendorOptionDef(universe, vendor_id, code); - } - } - - // Still haven't found the definition. Check if user has specified - // option definition in the server configuration. - if (!def) { - def = LibDHCP::getRuntimeOptionDef(space, code); - } - // Option can be stored as a blob or formatted value in the configuration. std::vector blob; if (!(*(first_binding + 2))->amNull()) { @@ -632,25 +612,7 @@ MySqlConfigBackendImpl::processOptionRow(const Option::Universe& universe, // Get formatted value if available. std::string formatted_value = (*(first_binding + 3))->getStringOrDefault(""); - OptionPtr option; - if (!def) { - // No option definition. Create generic option instance. - option.reset(new Option(universe, code, buf.begin(), buf.end())); - - } else { - // Option definition found. Use formatted value if available or - // a blob. - if (formatted_value.empty()) { - option = def->optionFactory(universe, code, buf.begin(), - buf.end()); - } else { - // Spit the value specified in comma separated values - // format. - std::vector split_vec; - boost::split(split_vec, formatted_value, boost::is_any_of(",")); - option = def->optionFactory(universe, code, split_vec); - } - } + OptionPtr option(new Option(universe, code, buf.begin(), buf.end())); // Check if the option is persistent. bool persistent = static_cast((*(first_binding + 5))->getIntegerOrDefault(0)); diff --git a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc index eeae4f65f9..90b1a52bb6 100644 --- a/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc +++ b/src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc @@ -1432,7 +1432,11 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteOption4) { opt_boot_file_name->option_->getType(), opt_boot_file_name->space_name_); ASSERT_TRUE(returned_opt_boot_file_name); - EXPECT_TRUE(returned_opt_boot_file_name->equals(*opt_boot_file_name)); + + { + SCOPED_TRACE("verify created option"); + testOptionsEquivalent(*test_options_[0], *returned_opt_boot_file_name); + } { SCOPED_TRACE("CREATE audit entry for an option"); @@ -1452,7 +1456,11 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteOption4) { opt_boot_file_name->option_->getType(), opt_boot_file_name->space_name_); ASSERT_TRUE(returned_opt_boot_file_name); - EXPECT_TRUE(returned_opt_boot_file_name->equals(*opt_boot_file_name)); + + { + SCOPED_TRACE("verify updated option"); + testOptionsEquivalent(*opt_boot_file_name, *returned_opt_boot_file_name); + } { SCOPED_TRACE("UPDATE audit entry for an option"); @@ -1507,17 +1515,26 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getAllOptions4) { // Verify that all options we put into the database were // returned. - auto option0 = index.find(test_options_[0]->option_->getType()); - ASSERT_FALSE(option0 == index.end()); - EXPECT_TRUE(option0->equals(*test_options_[0])); + { + SCOPED_TRACE("verify test_options_[0]"); + auto option0 = index.find(test_options_[0]->option_->getType()); + ASSERT_FALSE(option0 == index.end()); + testOptionsEquivalent(*test_options_[0], *option0); + } - auto option1 = index.find(test_options_[1]->option_->getType()); - ASSERT_FALSE(option1 == index.end()); - EXPECT_TRUE(option1->equals(*test_options_[1])); + { + SCOPED_TRACE("verify test_options_[1]"); + auto option1 = index.find(test_options_[1]->option_->getType()); + ASSERT_FALSE(option1 == index.end()); + testOptionsEquivalent(*test_options_[1], *option1); + } - auto option5 = index.find(test_options_[5]->option_->getType()); - ASSERT_FALSE(option5 == index.end()); - EXPECT_TRUE(option5->equals(*test_options_[5])); + { + SCOPED_TRACE("verify test_options_[5]"); + auto option5 = index.find(test_options_[5]->option_->getType()); + ASSERT_FALSE(option5 == index.end()); + testOptionsEquivalent(*test_options_[5], *option5); + } } // This test verifies that modified global options can be retrieved. @@ -1554,7 +1571,10 @@ TEST_F(MySqlConfigBackendDHCPv4Test, getModifiedOptions4) { const OptionContainerTypeIndex& index = returned_options.get<1>(); auto option0 = index.find(test_options_[0]->option_->getType()); ASSERT_FALSE(option0 == index.end()); - EXPECT_TRUE(option0->equals(*test_options_[0])); + { + SCOPED_TRACE("verify returned option"); + testOptionsEquivalent(*test_options_[0], *option0); + } } // This test verifies that subnet level option can be added, updated and @@ -1587,7 +1607,11 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteSubnetOption4) { OptionDescriptor returned_opt_boot_file_name = returned_subnet->getCfgOption()->get(DHCP4_OPTION_SPACE, DHO_BOOT_FILE_NAME); ASSERT_TRUE(returned_opt_boot_file_name.option_); - EXPECT_TRUE(returned_opt_boot_file_name.equals(*opt_boot_file_name)); + + { + SCOPED_TRACE("verify returned option"); + testOptionsEquivalent(*opt_boot_file_name, returned_opt_boot_file_name); + } { SCOPED_TRACE("UPDATE audit entry for an added subnet option"); @@ -1610,7 +1634,11 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteSubnetOption4) { returned_opt_boot_file_name = returned_subnet->getCfgOption()->get(DHCP4_OPTION_SPACE, DHO_BOOT_FILE_NAME); ASSERT_TRUE(returned_opt_boot_file_name.option_); - EXPECT_TRUE(returned_opt_boot_file_name.equals(*opt_boot_file_name)); + + { + SCOPED_TRACE("verify returned option with modified persistence"); + testOptionsEquivalent(*opt_boot_file_name, returned_opt_boot_file_name); + } { SCOPED_TRACE("UPDATE audit entry for an updated subnet option"); @@ -1681,7 +1709,11 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeletePoolOption4) { OptionDescriptor returned_opt_boot_file_name = returned_pool->getCfgOption()->get(DHCP4_OPTION_SPACE, DHO_BOOT_FILE_NAME); ASSERT_TRUE(returned_opt_boot_file_name.option_); - EXPECT_TRUE(returned_opt_boot_file_name.equals(*opt_boot_file_name)); + + { + SCOPED_TRACE("verify returned pool option"); + testOptionsEquivalent(*opt_boot_file_name, returned_opt_boot_file_name); + } { SCOPED_TRACE("UPDATE audit entry for a subnet after adding an option " @@ -1711,7 +1743,11 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeletePoolOption4) { returned_opt_boot_file_name = returned_pool1->getCfgOption()->get(DHCP4_OPTION_SPACE, DHO_BOOT_FILE_NAME); ASSERT_TRUE(returned_opt_boot_file_name.option_); - EXPECT_TRUE(returned_opt_boot_file_name.equals(*opt_boot_file_name)); + + { + SCOPED_TRACE("verify updated option with modified persistence"); + testOptionsEquivalent(*opt_boot_file_name, returned_opt_boot_file_name); + } { SCOPED_TRACE("UPDATE audit entry for a subnet when updating pool " @@ -1790,7 +1826,11 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteSharedNetworkOption4) { OptionDescriptor returned_opt_boot_file_name = returned_network->getCfgOption()->get(DHCP4_OPTION_SPACE, DHO_BOOT_FILE_NAME); ASSERT_TRUE(returned_opt_boot_file_name.option_); - EXPECT_TRUE(returned_opt_boot_file_name.equals(*opt_boot_file_name)); + + { + SCOPED_TRACE("verify returned option"); + testOptionsEquivalent(*opt_boot_file_name, returned_opt_boot_file_name); + } { SCOPED_TRACE("UPDATE audit entry for the added shared network option"); @@ -1814,7 +1854,11 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteSharedNetworkOption4) { returned_opt_boot_file_name = returned_network->getCfgOption()->get(DHCP4_OPTION_SPACE, DHO_BOOT_FILE_NAME); ASSERT_TRUE(returned_opt_boot_file_name.option_); - EXPECT_TRUE(returned_opt_boot_file_name.equals(*opt_boot_file_name)); + + { + SCOPED_TRACE("verify updated option with modified persistence"); + testOptionsEquivalent(*opt_boot_file_name, returned_opt_boot_file_name); + } { SCOPED_TRACE("UPDATE audit entry for the updated shared network option"); diff --git a/src/lib/dhcpsrv/testutils/generic_backend_unittest.cc b/src/lib/dhcpsrv/testutils/generic_backend_unittest.cc index bbb536362c..85f2fc82d9 100644 --- a/src/lib/dhcpsrv/testutils/generic_backend_unittest.cc +++ b/src/lib/dhcpsrv/testutils/generic_backend_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2018-2019 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 @@ -9,6 +9,8 @@ #include #include #include +#include +#include namespace isc { namespace dhcp { @@ -49,6 +51,50 @@ GenericBackendTest::createVendorOption(const Option::Universe& universe, return (desc); } +void +GenericBackendTest::testOptionsEquivalent(const OptionDescriptor& ref_option, + const OptionDescriptor& tested_option) const { + // Make sure that all pointers are non-null. + ASSERT_TRUE(ref_option.option_); + ASSERT_TRUE(tested_option.option_); + + // Get the reference to the tested option. Make should it has + // generic type. + Option& tested_option_reference = *tested_option.option_; + EXPECT_TRUE(typeid(tested_option_reference) == typeid(Option)); + + // Only test the binary data if the formatted value is not provided. + if (tested_option.formatted_value_.empty()) { + + // Prepare on-wire data of the option under test. + isc::util::OutputBuffer tested_option_buf(1); + tested_option.option_->pack(tested_option_buf); + const uint8_t* tested_option_buf_data = static_cast + (tested_option_buf.getData()); + std::vector tested_option_buf_vec(tested_option_buf_data, + tested_option_buf_data + tested_option_buf.getLength()); + + // Prepare on-wire data of the reference option. + isc::util::OutputBuffer ref_option_buf(1); + ref_option.option_->pack(ref_option_buf); + const uint8_t* ref_option_buf_data = static_cast + (ref_option_buf.getData()); + std::vector ref_option_buf_vec(ref_option_buf_data, + ref_option_buf_data + ref_option_buf.getLength()); + + // Compare the on-wire data. + EXPECT_EQ(ref_option_buf_vec, tested_option_buf_vec); + } + + // Compare other members of the @c OptionDescriptor, e.g. the + // tested option may contain formatted option data which can be + // later used to turn this option instance into a formatted + // option when an option definition is available. + EXPECT_EQ(ref_option.formatted_value_, tested_option.formatted_value_); + EXPECT_EQ(ref_option.persistent_, tested_option.persistent_); + EXPECT_EQ(ref_option.space_name_, tested_option.space_name_); +} + } // end of namespace isc::dhcp::test } // end of namespace isc::dhcp diff --git a/src/lib/dhcpsrv/testutils/generic_backend_unittest.h b/src/lib/dhcpsrv/testutils/generic_backend_unittest.h index 34a1b571da..c770d7cd4b 100644 --- a/src/lib/dhcpsrv/testutils/generic_backend_unittest.h +++ b/src/lib/dhcpsrv/testutils/generic_backend_unittest.h @@ -189,6 +189,59 @@ public: const bool persist, const bool formatted, const uint32_t vendor_id) const; + + /// @brief Verify the option returned by the backend against a + /// reference option. + /// + /// DHCP option value can be specified in two ways. First, it can be + /// specified as a string of hexadecimal digits which is converted to + /// a binary option value. Second, it can be specified as a string of + /// comma separated values in a user readable form. The comma separated + /// values are parsed according to the definition of the given option + /// and then stored in the respective fields of the option. The second + /// approach always requires an option definition to be known to the + /// parser. It may either be a standard option definition or a runtime + /// option definition created by a user. While standard option + /// definitions are available in the Kea header files, the custom + /// option definitions may not be available to the Config Backend + /// fetching an option from the database for the following reasons: + /// + /// - the server to which the Config Backend is attached is not the + /// one for which the configuration is being returned. + /// - the server is starting up and hasn't yet configured its runtime + /// option definitions. + /// - the Config Backend implementation is not attached to the DHCP + /// server but to the Control Agent. + /// + /// Note that the last case it currently not supported but may be + /// supported in the future. + /// + /// Since the option definitions aren't always available to the Config + /// Backend fetching the options from the database, the backend doesn't + /// interpret formatted options (those that use comma separated values + /// notation). It simply creates an @c OptionDescriptor with the generic + /// option instance (containing an option code and no option value) and + /// the other @c OptionDescriptor parameters set appropriately. The + /// @c CfgOption class contains methds that can be used on demand to + /// replace these instances with the appropriate types (derived from + /// @c Option) which represent formatted option data, if necessary. + /// + /// This test verifies that the @c OptionDescriptor returned by the + /// Config Backend is correct in that: + /// - the @c option_ member is non-null, + /// - the option instance is of a @c Option type rather than any of the + /// derived types (is a raw option), + /// - the wire data of the returned option is equal to the wire data of + /// the reference option (the reference option can be of a type derived + /// from @c Option), + /// - the @c formatted_value_, @c persistent_ and @c space_name_ members + /// of the returned option are equal to the respective members of the + /// reference option. + /// + /// @param ref_option Reference option to compare tested option to. + /// @param tested_option Option returned by the backend to be tested. + void testOptionsEquivalent(const OptionDescriptor& ref_option, + const OptionDescriptor& tested_option) const; }; } // end of namespace isc::dhcp::test