Browse Source

[3681] Patch as provided by Adam Kalmus:

 - schema header is now updated to 3.0
 - copyright years updated
 - doxygen comments added
 - unit-test for upgrading to 3.0 added
Tomek Mrugalski 10 years ago
parent
commit
fe06c49b19

+ 1 - 0
AUTHORS

@@ -80,6 +80,7 @@ We have received the following contributions:
    2014-12: Extract MAC address from DUID-LL and DUID-LLT types
    2015-01: Extract MAC address from remote-id
    2015-05: MySQL schema extended to cover host reservation
+   2015-04: Common MySQL Connector Pool 
 
 Kea uses log4cplus (http://sourceforge.net/projects/log4cplus/) for logging,
 Boost (http://www.boost.org/) library for almost everything, and can use Botan

+ 0 - 0
configure.ac


+ 0 - 0
src/bin/admin/scripts/mysql/Makefile.am


+ 0 - 1
src/bin/admin/tests/mysql_tests.sh.in

@@ -257,7 +257,6 @@ EOF
     ERRCODE=$?
     assert_eq 0 $ERRCODE "dhcp6_options table is missing or broken. (returned status code %d, expected %d)"
 
-
     # Verify that it reports version 3.0.
     version=$(${keaadmin} lease-version mysql -u $db_user -p $db_password -n $db_name)
 

+ 69 - 0
src/lib/dhcpsrv/data_source.cc

@@ -13,8 +13,19 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <dhcpsrv/data_source.h>
+#include <dhcpsrv/dhcpsrv_log.h>
 #include <exceptions/exceptions.h>
 
+#include <boost/algorithm/string.hpp>
+#include <boost/foreach.hpp>
+#include <boost/scoped_ptr.hpp>
+
+#include <dhcpsrv/mysql_lease_mgr.h>
+#include <dhcpsrv/pgsql_lease_mgr.h>
+
+
+using namespace std;
+
 namespace isc {
 namespace dhcp {
 
@@ -26,5 +37,63 @@ std::string DataSource::getParameter(const std::string& name) const {
     return (param->second);
 }
 
+DataSource::ParameterMap
+DataSource::parse(const std::string& dbaccess) {
+    DataSource::ParameterMap mapped_tokens;
+
+    if (!dbaccess.empty()) {
+        vector<string> tokens;
+
+        // We need to pass a string to is_any_of, not just char*. Otherwise
+        // there are cryptic warnings on Debian6 running g++ 4.4 in
+        // /usr/include/c++/4.4/bits/stl_algo.h:2178 "array subscript is above
+        // array bounds"
+        boost::split(tokens, dbaccess, boost::is_any_of(string("\t ")));
+        BOOST_FOREACH(std::string token, tokens) {
+            size_t pos = token.find("=");
+            if (pos != string::npos) {
+                string name = token.substr(0, pos);
+                string value = token.substr(pos + 1);
+                mapped_tokens.insert(make_pair(name, value));
+            } else {
+                LOG_ERROR(dhcpsrv_logger, DHCPSRV_INVALID_ACCESS).arg(dbaccess);
+                isc_throw(InvalidParameter, "Cannot parse " << token
+                          << ", expected format is name=value");
+            }
+        }
+    }
+
+    return (mapped_tokens);
+}
+
+std::string
+DataSource::redactedAccessString(const DataSource::ParameterMap& parameters) {
+    // Reconstruct the access string: start of with an empty string, then
+    // work through all the parameters in the original string and add them.
+    std::string access;
+    for (DataSource::ParameterMap::const_iterator i = parameters.begin();
+         i != parameters.end(); ++i) {
+
+        // Separate second and subsequent tokens are preceded by a space.
+        if (!access.empty()) {
+            access += " ";
+        }
+
+        // Append name of parameter...
+        access += i->first;
+        access += "=";
+
+        // ... and the value, except in the case of the password, where a
+        // redacted value is appended.
+        if (i->first == std::string("password")) {
+            access += "*****";
+        } else {
+            access += i->second;
+        }
+    }
+
+    return (access);
+}
+
 };
 };

+ 31 - 0
src/lib/dhcpsrv/data_source.h

@@ -44,12 +44,22 @@ public:
         isc::Exception(file, line, what) {}
 };
 
+
+/// @brief Common Data Source Class
+///
+/// This class provides functions that are common for establishing
+/// connection with different types of databases; enables operations
+/// on access parameters strings.
 class DataSource : public boost::noncopyable {
 
 public:
     /// Database configuration parameter map
     typedef std::map<std::string, std::string> ParameterMap;
 
+    /// @brief Constructor
+    ///
+    /// @param parameters A data structure relating keywords and values
+    ///        concerned with the database.
     DataSource(const ParameterMap& parameters)
         :parameters_(parameters) {
     }
@@ -59,6 +69,27 @@ public:
     /// @return parameter
     std::string getParameter(const std::string& name) const;
 
+    /// @brief Parse database access string
+    ///
+    /// Parses the string of "keyword=value" pairs and separates them
+    /// out into the map.
+    ///
+    /// @param dbaccess Database access string.
+    ///
+    /// @return std::map<std::string, std::string> Map of keyword/value pairs.
+    static DataSource::ParameterMap parse(const std::string& dbaccess);
+
+    /// @brief Redact database access string
+    ///
+    /// Takes the database parameters and returns a database access string
+    /// passwords replaced by asterisks. This string is used in log messages.
+    ///
+    /// @param parameters Database access parameters (output of "parse").
+    ///
+    /// @return Redacted database access string.
+    static std::string redactedAccessString(
+            const DataSource::ParameterMap& parameters);
+
 protected:
 
     /// @brief list of parameters passed in dbconfig

+ 0 - 2
src/lib/dhcpsrv/lease_mgr.h

@@ -118,8 +118,6 @@ public:
 
     /// @brief Constructor
     ///
-    /// @param parameters A data structure relating keywords and values
-    ///        concerned with the database.
     LeaseMgr() : io_service_(new asiolink::IOService())
     {}
 

+ 2 - 60
src/lib/dhcpsrv/lease_mgr_factory.cc

@@ -46,71 +46,13 @@ LeaseMgrFactory::getLeaseMgrPtr() {
     return (leaseMgrPtr);
 }
 
-DataSource::ParameterMap
-LeaseMgrFactory::parse(const std::string& dbaccess) {
-    DataSource::ParameterMap mapped_tokens;
-
-    if (!dbaccess.empty()) {
-        vector<string> tokens;
-
-        // We need to pass a string to is_any_of, not just char*. Otherwise
-        // there are cryptic warnings on Debian6 running g++ 4.4 in
-        // /usr/include/c++/4.4/bits/stl_algo.h:2178 "array subscript is above
-        // array bounds"
-        boost::split(tokens, dbaccess, boost::is_any_of(string("\t ")));
-        BOOST_FOREACH(std::string token, tokens) {
-            size_t pos = token.find("=");
-            if (pos != string::npos) {
-                string name = token.substr(0, pos);
-                string value = token.substr(pos + 1);
-                mapped_tokens.insert(make_pair(name, value));
-            } else {
-                LOG_ERROR(dhcpsrv_logger, DHCPSRV_INVALID_ACCESS).arg(dbaccess);
-                isc_throw(InvalidParameter, "Cannot parse " << token
-                          << ", expected format is name=value");
-            }
-        }
-    }
-
-    return (mapped_tokens);
-}
-
-std::string
-LeaseMgrFactory::redactedAccessString(const DataSource::ParameterMap& parameters) {
-    // Reconstruct the access string: start of with an empty string, then
-    // work through all the parameters in the original string and add them.
-    std::string access;
-    for (DataSource::ParameterMap::const_iterator i = parameters.begin();
-         i != parameters.end(); ++i) {
-
-        // Separate second and subsequent tokens are preceded by a space.
-        if (!access.empty()) {
-            access += " ";
-        }
-
-        // Append name of parameter...
-        access += i->first;
-        access += "=";
-
-        // ... and the value, except in the case of the password, where a
-        // redacted value is appended.
-        if (i->first == std::string("password")) {
-            access += "*****";
-        } else {
-            access += i->second;
-        }
-    }
-
-    return (access);
-}
-
 void
 LeaseMgrFactory::create(const std::string& dbaccess) {
     const std::string type = "type";
 
     // Parse the access string and create a redacted string for logging.
-    DataSource::ParameterMap parameters = parse(dbaccess);
-    std::string redacted = redactedAccessString(parameters);
+    DataSource::ParameterMap parameters = DataSource::parse(dbaccess);
+    std::string redacted = DataSource::redactedAccessString(parameters);
 
     // Is "type" present?
     if (parameters.find(type) == parameters.end()) {

+ 0 - 19
src/lib/dhcpsrv/lease_mgr_factory.h

@@ -99,26 +99,7 @@ public:
     ///        create() to create one before calling this method.
     static LeaseMgr& instance();
 
-    /// @brief Parse database access string
-    ///
-    /// Parses the string of "keyword=value" pairs and separates them
-    /// out into the map.
-    ///
-    /// @param dbaccess Database access string.
-    ///
-    /// @return std::map<std::string, std::string> Map of keyword/value pairs.
-    static DataSource::ParameterMap parse(const std::string& dbaccess);
 
-    /// @brief Redact database access string
-    ///
-    /// Takes the database parameters and returns a database access string
-    /// passwords replaced by asterisks. This string is used in log messages.
-    ///
-    /// @param parameters Database access parameters (output of "parse").
-    ///
-    /// @return Redacted database access string.
-    static std::string redactedAccessString(
-            const DataSource::ParameterMap& parameters);
 
 private:
     /// @brief Hold pointer to lease manager

+ 1 - 1
src/lib/dhcpsrv/mysql_connection.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above

+ 11 - 3
src/lib/dhcpsrv/mysql_connection.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -19,7 +19,7 @@
 #include <dhcpsrv/lease_mgr.h>
 #include <dhcpsrv/data_source.h>
 #include <boost/scoped_ptr.hpp>
-#include <mysql/mysql.h>		// TODO poprawić przed oddaniem
+#include <mysql.h>
 
 #include <time.h>
 
@@ -134,15 +134,23 @@ private:
     MYSQL* mysql_;      ///< Initialization context
 };
 
-// Define the current database schema values
+/// @brief Common MySQL Connector Pool
+///
+///	This class provides common operations for MySQL database connection
+///	used by both MySqlLeaseMgr and MySqlHostDataSource. It manages connecting
+///	to the database and preparing compiled statements.
 
 class MySqlConnection : public DataSource {
 public:
 
+    /// @brief Constructor
+    ///
+    /// Initialize MySqlConnection object with parameters needed for connection.
     MySqlConnection(const ParameterMap& parameters)
         : DataSource(parameters) {
     }
 
+    /// @brief Destructor
     virtual ~MySqlConnection() {
     }
 

+ 17 - 17
src/lib/dhcpsrv/tests/lease_mgr_factory_unittest.cc

@@ -41,7 +41,7 @@ public:
 // This test checks that a database access string can be parsed correctly.
 TEST_F(LeaseMgrFactoryTest, parse) {
 
-    DataSource::ParameterMap parameters = LeaseMgrFactory::parse(
+    DataSource::ParameterMap parameters = DataSource::parse(
         "user=me password=forbidden name=kea somethingelse= type=mysql");
 
     EXPECT_EQ(5, parameters.size());
@@ -57,21 +57,21 @@ TEST_F(LeaseMgrFactoryTest, parseInvalid) {
 
     // No tokens in the string, so we expect no parameters
     std::string invalid = "";
-    DataSource::ParameterMap parameters = LeaseMgrFactory::parse(invalid);
+    DataSource::ParameterMap parameters = DataSource::parse(invalid);
     EXPECT_EQ(0, parameters.size());
 
     // With spaces, there are some tokens so we expect invalid parameter
     // as there are no equals signs.
     invalid = "   \t  ";
-    EXPECT_THROW(LeaseMgrFactory::parse(invalid), isc::InvalidParameter);
+    EXPECT_THROW(DataSource::parse(invalid), isc::InvalidParameter);
 
     invalid = "   noequalshere  ";
-    EXPECT_THROW(LeaseMgrFactory::parse(invalid), isc::InvalidParameter);
+    EXPECT_THROW(DataSource::parse(invalid), isc::InvalidParameter);
 
     // A single "=" is valid string, but is placed here as the result is
     // expected to be nothing.
     invalid = "=";
-    parameters = LeaseMgrFactory::parse(invalid);
+    parameters = DataSource::parse(invalid);
     EXPECT_EQ(1, parameters.size());
     EXPECT_EQ("", parameters[""]);
 }
@@ -83,7 +83,7 @@ TEST_F(LeaseMgrFactoryTest, parseInvalid) {
 TEST_F(LeaseMgrFactoryTest, redactAccessString) {
 
     DataSource::ParameterMap parameters =
-        LeaseMgrFactory::parse("user=me password=forbidden name=kea type=mysql");
+    		DataSource::parse("user=me password=forbidden name=kea type=mysql");
     EXPECT_EQ(4, parameters.size());
     EXPECT_EQ("me", parameters["user"]);
     EXPECT_EQ("forbidden", parameters["password"]);
@@ -92,8 +92,8 @@ TEST_F(LeaseMgrFactoryTest, redactAccessString) {
 
     // Redact the result.  To check, break the redacted string down into its
     // components.
-    std::string redacted = LeaseMgrFactory::redactedAccessString(parameters);
-    parameters = LeaseMgrFactory::parse(redacted);
+    std::string redacted = DataSource::redactedAccessString(parameters);
+    parameters = DataSource::parse(redacted);
 
     EXPECT_EQ(4, parameters.size());
     EXPECT_EQ("me", parameters["user"]);
@@ -109,7 +109,7 @@ TEST_F(LeaseMgrFactoryTest, redactAccessString) {
 TEST_F(LeaseMgrFactoryTest, redactAccessStringEmptyPassword) {
 
     DataSource::ParameterMap parameters =
-        LeaseMgrFactory::parse("user=me name=kea type=mysql password=");
+    		DataSource::parse("user=me name=kea type=mysql password=");
     EXPECT_EQ(4, parameters.size());
     EXPECT_EQ("me", parameters["user"]);
     EXPECT_EQ("", parameters["password"]);
@@ -118,8 +118,8 @@ TEST_F(LeaseMgrFactoryTest, redactAccessStringEmptyPassword) {
 
     // Redact the result.  To check, break the redacted string down into its
     // components.
-    std::string redacted = LeaseMgrFactory::redactedAccessString(parameters);
-    parameters = LeaseMgrFactory::parse(redacted);
+    std::string redacted = DataSource::redactedAccessString(parameters);
+    parameters = DataSource::parse(redacted);
 
     EXPECT_EQ(4, parameters.size());
     EXPECT_EQ("me", parameters["user"]);
@@ -129,15 +129,15 @@ TEST_F(LeaseMgrFactoryTest, redactAccessStringEmptyPassword) {
 
     // ... and again to check that the position of the empty password in the
     // string does not matter.
-    parameters = LeaseMgrFactory::parse("user=me password= name=kea type=mysql");
+    parameters = DataSource::parse("user=me password= name=kea type=mysql");
     EXPECT_EQ(4, parameters.size());
     EXPECT_EQ("me", parameters["user"]);
     EXPECT_EQ("", parameters["password"]);
     EXPECT_EQ("kea", parameters["name"]);
     EXPECT_EQ("mysql", parameters["type"]);
 
-    redacted = LeaseMgrFactory::redactedAccessString(parameters);
-    parameters = LeaseMgrFactory::parse(redacted);
+    redacted = DataSource::redactedAccessString(parameters);
+    parameters = DataSource::parse(redacted);
 
     EXPECT_EQ(4, parameters.size());
     EXPECT_EQ("me", parameters["user"]);
@@ -153,7 +153,7 @@ TEST_F(LeaseMgrFactoryTest, redactAccessStringEmptyPassword) {
 TEST_F(LeaseMgrFactoryTest, redactAccessStringNoPassword) {
 
     DataSource::ParameterMap parameters =
-        LeaseMgrFactory::parse("user=me name=kea type=mysql");
+    		DataSource::parse("user=me name=kea type=mysql");
     EXPECT_EQ(3, parameters.size());
     EXPECT_EQ("me", parameters["user"]);
     EXPECT_EQ("kea", parameters["name"]);
@@ -161,8 +161,8 @@ TEST_F(LeaseMgrFactoryTest, redactAccessStringNoPassword) {
 
     // Redact the result.  To check, break the redacted string down into its
     // components.
-    std::string redacted = LeaseMgrFactory::redactedAccessString(parameters);
-    parameters = LeaseMgrFactory::parse(redacted);
+    std::string redacted = DataSource::redactedAccessString(parameters);
+    parameters = DataSource::parse(redacted);
 
     EXPECT_EQ(3, parameters.size());
     EXPECT_EQ("me", parameters["user"]);

+ 1 - 1
src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above

+ 4 - 1
src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -252,6 +252,9 @@ TEST(PgSqlOpenTest, OpenDatabase) {
         VALID_TYPE, VALID_NAME, VALID_HOST, INVALID_USER, VALID_PASSWORD)),
         DbOpenError);
 
+    // This test might fail if 'auth-method' in PostgresSQL host-based authentication
+    // file (/var/lib/pgsql/9.4/data/pg_hba.conf) is set to 'trust',
+    // which allows logging without password. 'Auth-method' should be changed to 'password'.
     EXPECT_THROW(LeaseMgrFactory::create(connectionString(
         VALID_TYPE, VALID_NAME, VALID_HOST, VALID_USER, INVALID_PASSWORD)),
         DbOpenError);

+ 86 - 1
src/lib/dhcpsrv/tests/schema_mysql_copy.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -37,6 +37,11 @@ const char* destroy_statement[] = {
     "DROP TABLE lease6_types",
     "DROP TABLE lease_hwaddr_source",
     "DROP TABLE schema_version",
+    // Schema 3.0 destroy statements
+    "DROP TABLE hosts",
+    "DROP TABLE dhcp4_options",
+    "DROP TABLE dhcp6_options",
+    "DROP TABLE ipv6_reservations",
     NULL
 };
 
@@ -125,6 +130,86 @@ const char* create_statement[] = {
     "UPDATE schema_version SET version=\"2\", minor=\"0\";",
     // Schema upgrade to 2.0 ends here.
 
+    // Schema upgrade to 3.0 starts here.
+
+    "CREATE TABLE IF NOT EXISTS hosts ("
+        "host_id INT UNSIGNED NOT NULL AUTO_INCREMENT,"
+        "dhcp_identifier VARBINARY(128) NOT NULL,"
+        "dhcp_identifier_type TINYINT NOT NULL,"
+        "dhcp4_subnet_id INT UNSIGNED NULL,"
+        "dhcp6_subnet_id INT UNSIGNED NULL,"
+        "ipv4_address INT UNSIGNED NULL,"
+        "hostname VARCHAR(255) NULL,"
+        "dhcp4_client_classes VARCHAR(255) NULL,"
+        "dhcp6_client_classes VARCHAR(255) NULL,"
+        "PRIMARY KEY (host_id),"
+        "INDEX key_dhcp4_identifier_subnet_id (dhcp_identifier ASC , dhcp_identifier_type ASC),"
+        "INDEX key_dhcp6_identifier_subnet_id (dhcp_identifier ASC , dhcp_identifier_type ASC , dhcp6_subnet_id ASC)"
+    ")  ENGINE=INNODB",
+
+    "CREATE TABLE IF NOT EXISTS ipv6_reservations ("
+        "reservation_id INT NOT NULL AUTO_INCREMENT,"
+        "address VARCHAR(39) NOT NULL,"
+        "prefix_len TINYINT(3) UNSIGNED NOT NULL DEFAULT 128,"
+        "type TINYINT(4) UNSIGNED NOT NULL DEFAULT 0,"
+        "dhcp6_iaid INT UNSIGNED NULL,"
+        "host_id INT UNSIGNED NOT NULL,"
+        "PRIMARY KEY (reservation_id),"
+        "INDEX fk_ipv6_reservations_host_idx (host_id ASC),"
+        "CONSTRAINT fk_ipv6_reservations_Host FOREIGN KEY (host_id)"
+            "REFERENCES hosts (host_id)"
+            "ON DELETE NO ACTION ON UPDATE NO ACTION"
+    ")  ENGINE=INNODB",
+
+    "CREATE TABLE IF NOT EXISTS dhcp4_options ("
+        "option_id INT UNSIGNED NOT NULL AUTO_INCREMENT,"
+        "code TINYINT UNSIGNED NOT NULL,"
+        "value BLOB NULL,"
+        "formatted_value TEXT NULL,"
+        "space VARCHAR(128) NULL,"
+        "persistent TINYINT(1) NOT NULL DEFAULT 0,"
+        "dhcp_client_class VARCHAR(128) NULL,"
+        "dhcp4_subnet_id INT NULL,"
+        "host_id INT UNSIGNED NULL,"
+        "PRIMARY KEY (option_id),"
+        "UNIQUE INDEX option_id_UNIQUE (option_id ASC),"
+        "INDEX fk_options_host1_idx (host_id ASC),"
+        "CONSTRAINT fk_options_host1 FOREIGN KEY (host_id)"
+            "REFERENCES hosts (host_id)"
+            "ON DELETE NO ACTION ON UPDATE NO ACTION"
+    ")  ENGINE=INNODB",
+
+    "CREATE TABLE IF NOT EXISTS dhcp6_options ("
+        "option_id INT UNSIGNED NOT NULL AUTO_INCREMENT,"
+        "code INT UNSIGNED NOT NULL,"
+        "value BLOB NULL,"
+        "formatted_value TEXT NULL,"
+        "space VARCHAR(128) NULL,"
+        "persistent TINYINT(1) NOT NULL DEFAULT 0,"
+        "dhcp_client_class VARCHAR(128) NULL,"
+        "dhcp6_subnet_id INT NULL,"
+        "host_id INT UNSIGNED NULL,"
+        "PRIMARY KEY (option_id),"
+        "UNIQUE INDEX option_id_UNIQUE (option_id ASC),"
+        "INDEX fk_options_host1_idx (host_id ASC),"
+        "CONSTRAINT fk_options_host10 FOREIGN KEY (host_id)"
+            "REFERENCES hosts (host_id)"
+            "ON DELETE NO ACTION ON UPDATE NO ACTION"
+    ")  ENGINE=INNODB",
+
+
+    //"DELIMITER $$ ",
+    "CREATE TRIGGER host_BDEL BEFORE DELETE ON hosts FOR EACH ROW "
+    "BEGIN "
+    "DELETE FROM ipv6_reservations WHERE ipv6_reservations.host_id = OLD.host_id; "
+    "END ",
+    //"$$ ",
+    //"DELIMITER ;",
+
+    "UPDATE schema_version SET version = '3', minor = '0';",
+
+    // This line concludes database upgrade to version 3.0.
+
     NULL
 };