]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#521,!270] Removed dependency of the MySQL CB on LibDHCP option defs.
authorMarcin Siodelski <marcin@isc.org>
Mon, 11 Mar 2019 10:15:48 +0000 (11:15 +0100)
committerMarcin Siodelski <marcin@isc.org>
Thu, 14 Mar 2019 08:59:58 +0000 (04:59 -0400)
src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc
src/hooks/dhcp/mysql_cb/tests/mysql_cb_dhcp4_unittest.cc
src/lib/dhcpsrv/testutils/generic_backend_unittest.cc
src/lib/dhcpsrv/testutils/generic_backend_unittest.h

index cf74f1231cf343f6d914ad728a9ca5c0bedf9dee..0e104120e5dc120608a300a53b2ca44c869dafd3 100644 (file)
@@ -8,11 +8,8 @@
 #include <mysql_cb_impl.h>
 #include <asiolink/io_address.h>
 #include <config_backend/constants.h>
-#include <dhcp/libdhcp++.h>
 #include <dhcp/option_space.h>
 #include <util/buffer.h>
-#include <boost/algorithm/string/split.hpp>
-#include <boost/algorithm/string/classification.hpp>
 #include <mysql.h>
 #include <mysqld_error.h>
 #include <cstdint>
@@ -605,23 +602,6 @@ MySqlConfigBackendImpl::processOptionRow(const Option::Universe& universe,
         code = (*(first_binding + 1))->getInteger<uint16_t>();
     }
 
-    // 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<uint8_t> 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<std::string> 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<bool>((*(first_binding + 5))->getIntegerOrDefault<uint8_t>(0));
index eeae4f65f91a61314f85a5d0f0feaa1144867cdc..90b1a52bb6c3eb211ff437a6778760a7a62e1ee5 100644 (file)
@@ -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");
index bbb536362c9fcc32b8c23b858e892de1deb786f7..85f2fc82d9136a4be99f7230491a5b446ede4146 100644 (file)
@@ -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 <dhcp/libdhcp++.h>
 #include <dhcp/option_vendor.h>
 #include <dhcpsrv/testutils/generic_backend_unittest.h>
+#include <util/buffer.h>
+#include <typeinfo>
 
 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<const uint8_t*>
+            (tested_option_buf.getData());
+        std::vector<uint8_t> 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<const uint8_t*>
+            (ref_option_buf.getData());
+        std::vector<uint8_t> 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
index 34a1b571da11bb7fd42f35bf9a3fadacabc72e87..c770d7cd4b447400d9ae0bf8595d5ac8b4b941bc 100644 (file)
@@ -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