[5629] MySQL and PostgreSQL host backends now verify schema version

src/lib/dhcpsrv/mysql_host_data_source.cc
    MySqlHostDataSourceImpl::getVersion() - new function to
    fetch version accessible to impl constructor

    MySqlHostDataSourceImpl() - added schema validation after
    connecting

    MySqlHostDataSource::getVersion() - new uses impl method

src/lib/dhcpsrv/pgsql_host_data_source.cc
    PgSqlHostDataSourceImpl()) - added schema validation after
    connecting

    PgSqlHostDataSourceImpl::getVersion() - no longer uses
    pre-prepared statement

diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc
index 6d2c2d032a23b6e8c79e76a36721e4eefca38e66..c4dfae044be2129f9c29ee1c942fab99f548af52 100644 (file)
--- a/src/lib/dhcpsrv/mysql_host_data_source.cc
+++ b/src/lib/dhcpsrv/mysql_host_data_source.cc
@@ -1906,7 +1906,6 @@ public:
         GET_HOST_SUBID_ADDR,    // Gets host by IPv4 SubnetID and IPv4 address
         GET_HOST_PREFIX,        // Gets host by IPv6 prefix
         GET_HOST_SUBID6_ADDR,   // Gets host by IPv6 SubnetID and IPv6 prefix
-        GET_VERSION,            // Obtain version number
         INSERT_HOST,            // Insert new host to collection
         INSERT_V6_RESRV,        // Insert v6 reservation
         INSERT_V4_OPTION,       // Insert DHCPv4 option
@@ -1933,6 +1932,21 @@ public:
     /// @brief Destructor.
     ~MySqlHostDataSourceImpl();
 
+    /// @brief Returns backend version.
+    ///
+    /// The method is called by the constructor after opening the database
+    /// but prior to preparing SQL statements, to verify that the schema version
+    /// is correct. Thus it must not rely on a pre-prepared statement or
+    /// formal statement execution error checking.
+    //
+    /// @return Version number stored in the database, as a pair of unsigned
+    ///         integers. "first" is the major version number, "second" the
+    ///         minor number.
+    ///
+    /// @throw isc::dhcp::DbOperationError An operation on the open database
+    ///        has failed.
+    std::pair<uint32_t, uint32_t> getVersion() const;
+
     /// @brief Executes statements which inserts a row into one of the tables.
     ///
     /// @param stindex Index of a statement being executed.
@@ -2232,10 +2246,6 @@ TaggedStatementArray tagged_statements = { {
             "WHERE h.dhcp6_subnet_id = ? AND r.address = ? "
             "ORDER BY h.host_id, o.option_id, r.reservation_id"},
 
-    // Retrieves MySQL schema version.
-    {MySqlHostDataSourceImpl::GET_VERSION,
-            "SELECT version, minor FROM schema_version"},
-
     // Inserts a host into the 'hosts' table.
     {MySqlHostDataSourceImpl::INSERT_HOST,
          "INSERT INTO hosts(host_id, dhcp_identifier, dhcp_identifier_type, "
@@ -2293,6 +2303,17 @@ MySqlHostDataSourceImpl(const MySqlConnection::ParameterMap& parameters)
     // Open the database.
     conn_.openDatabase();
 
+    // Test schema version before we try to prepare statements.
+    std::pair<uint32_t, uint32_t> code_version(MYSQL_SCHEMA_VERSION_MAJOR,
+    MYSQL_SCHEMA_VERSION_MINOR);
+    std::pair<uint32_t, uint32_t> db_version = getVersion();
+    if (code_version != db_version) {
+        isc_throw(DbOpenError, "MySQL schema version mismatch: need version: "
+                  << code_version.first << "." << code_version.second
+                  << " found version:  " << db_version.first << "."
+                  << db_version.second);
+    }
+
     // Enable autocommit. In case transaction is explicitly used, this
     // setting will be overwritten for the transaction. However, there are
     // cases when lack of autocommit could cause transactions to hang
@@ -2341,6 +2362,67 @@ MySqlHostDataSourceImpl::~MySqlHostDataSourceImpl() {
     // closed in the destructor of the mysql_ member variable.
 }
 
+std::pair<uint32_t, uint32_t>
+MySqlHostDataSourceImpl::getVersion() const {
+    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
+              DHCPSRV_MYSQL_HOST_DB_GET_VERSION);
+
+    // Allocate a new statement.
+    MYSQL_STMT *stmt = mysql_stmt_init(conn_.mysql_);
+    if (stmt == NULL) {
+        isc_throw(DbOperationError, "unable to allocate MySQL prepared "
+                  "statement structure, reason: " << mysql_error(conn_.mysql_));
+    }
+
+    // Prepare the statement from SQL text.
+    const char* version_sql = "SELECT version, minor FROM schema_version";
+    int status = mysql_stmt_prepare(stmt, version_sql, strlen(version_sql));
+    if (status != 0) {
+        isc_throw(DbOperationError, "unable to prepare MySQL statement <"
+                  << version_sql << ">, reason: " << mysql_errno(conn_.mysql_));
+    }
+
+    // Execute the prepared statement.
+    if (mysql_stmt_execute(stmt) != 0) {
+        isc_throw(DbOperationError, "cannot execute schema version query <"
+                  << version_sql << ">, reason: " << mysql_errno(conn_.mysql_));
+    }
+
+    // Bind the output of the statement to the appropriate variables.
+    MYSQL_BIND bind[2];
+    memset(bind, 0, sizeof(bind));
+
+    uint32_t major;
+    bind[0].buffer_type = MYSQL_TYPE_LONG;
+    bind[0].is_unsigned = 1;
+    bind[0].buffer = &major;
+    bind[0].buffer_length = sizeof(major);
+
+    uint32_t minor;
+    bind[1].buffer_type = MYSQL_TYPE_LONG;
+    bind[1].is_unsigned = 1;
+    bind[1].buffer = &minor;
+    bind[1].buffer_length = sizeof(minor);
+
+    if (mysql_stmt_bind_result(stmt, bind)) {
+        isc_throw(DbOperationError, "unable to bind result set for <"
+                  << version_sql << ">, reason: " << mysql_errno(conn_.mysql_));
+    }
+
+    // Fetch the data.
+    if (mysql_stmt_fetch(stmt)) {
+        mysql_stmt_close(stmt);
+        isc_throw(DbOperationError, "unable to bind result set for <"
+                  << version_sql << ">, reason: " << mysql_errno(conn_.mysql_));
+    }
+
+    // Discard the statement and its resources
+    mysql_stmt_close(stmt);
+
+    return (std::make_pair(major, minor));
+}
+
+
 void
 MySqlHostDataSourceImpl::addStatement(StatementIndex stindex,
                                       std::vector<MYSQL_BIND>& bind) {
@@ -2977,54 +3059,7 @@ std::string MySqlHostDataSource::getDescription() const {
 }
 
 std::pair<uint32_t, uint32_t> MySqlHostDataSource::getVersion() const {
-    const MySqlHostDataSourceImpl::StatementIndex stindex =
-        MySqlHostDataSourceImpl::GET_VERSION;
-
-    LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
-              DHCPSRV_MYSQL_HOST_DB_GET_VERSION);
-
-    uint32_t major;      // Major version number
-    uint32_t minor;      // Minor version number
-
-    // Execute the prepared statement
-    int status = mysql_stmt_execute(impl_->conn_.statements_[stindex]);
-    if (status != 0) {
-        isc_throw(DbOperationError, "unable to execute <"
-                  << impl_->conn_.text_statements_[stindex]
-                  << "> - reason: " << mysql_error(impl_->conn_.mysql_));
-    }
-
-    // Bind the output of the statement to the appropriate variables.
-    MYSQL_BIND bind[2];
-    memset(bind, 0, sizeof(bind));
-
-    bind[0].buffer_type = MYSQL_TYPE_LONG;
-    bind[0].is_unsigned = 1;
-    bind[0].buffer = &major;
-    bind[0].buffer_length = sizeof(major);
-
-    bind[1].buffer_type = MYSQL_TYPE_LONG;
-    bind[1].is_unsigned = 1;
-    bind[1].buffer = &minor;
-    bind[1].buffer_length = sizeof(minor);
-
-    status = mysql_stmt_bind_result(impl_->conn_.statements_[stindex], bind);
-    if (status != 0) {
-        isc_throw(DbOperationError, "unable to bind result set: "
-                  << mysql_error(impl_->conn_.mysql_));
-    }
-
-    // Fetch the data and set up the "release" object to release associated
-    // resources when this method exits then retrieve the data.
-    // mysql_stmt_fetch return value other than 0 means error occurrence.
-    MySqlFreeResult fetch_release(impl_->conn_.statements_[stindex]);
-    status = mysql_stmt_fetch(impl_->conn_.statements_[stindex]);
-    if (status != 0) {
-        isc_throw(DbOperationError, "unable to obtain result set: "
-                  << mysql_error(impl_->conn_.mysql_));
-    }
-
-    return (std::make_pair(major, minor));
+    return(impl_->getVersion());
 }
 
 void

diff --git a/src/lib/dhcpsrv/mysql_host_data_source.h b/src/lib/dhcpsrv/mysql_host_data_source.h
index 8476284aa3940f0653575563e688633794d8f278..874a9ac1639ee157c393262deca76ad58d299c94 100644 (file)
--- a/src/lib/dhcpsrv/mysql_host_data_source.h
+++ b/src/lib/dhcpsrv/mysql_host_data_source.h
@@ -49,7 +49,8 @@ public:
     ///        concerned with the database.
     ///
     /// @throw isc::dhcp::NoDatabaseName Mandatory database name not given
-    /// @throw isc::dhcp::DbOpenError Error opening the database
+    /// @throw isc::dhcp::DbOpenError Error opening the database or the
+    /// schema version is invalid.
     /// @throw isc::dhcp::DbOperationError An operation on the open database has
     ///        failed.
     MySqlHostDataSource(const DatabaseConnection::ParameterMap& parameters);

diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc
index 4367a389e50d09fb59993bfefc56daacbd94918d..93593e9054cf64bb05423cbec4101b81293eca6e 100644 (file)
--- a/src/lib/dhcpsrv/mysql_lease_mgr.cc
+++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc
@@ -1500,13 +1500,15 @@ MySqlLeaseMgr::MySqlLeaseMgr(const MySqlConnection::ParameterMap& parameters)
     conn_.openDatabase();
 
     // Test schema version before we try to prepare statements.
-    std::pair<uint32_t, uint32_t> version = getVersion();
-    if (version.first != MYSQL_SCHEMA_VERSION_MAJOR ||
-        version.second != MYSQL_SCHEMA_VERSION_MINOR) {
-        isc_throw(DbOpenError, "MySQL schema version is: "
-                  << version.first << "." << version.second << ", need version: "
-                  << MYSQL_SCHEMA_VERSION_MAJOR << "."
-                  << MYSQL_SCHEMA_VERSION_MINOR);
+    std::pair<uint32_t, uint32_t> code_version(MYSQL_SCHEMA_VERSION_MAJOR,
+                                               MYSQL_SCHEMA_VERSION_MINOR);
+    std::pair<uint32_t, uint32_t> db_version = getVersion();
+    if (code_version != db_version) {
+        isc_throw(DbOpenError,
+                  "MySQL schema version mismatch: need version: "
+                      << code_version.first << "." << code_version.second
+                      << " found version:  " << db_version.first << "."
+                      << db_version.second);
     }
 
     // Enable autocommit.  To avoid a flush to disk on every commit, the global
@@ -2400,8 +2402,8 @@ MySqlLeaseMgr::getVersion() const {
 
     // Execute the prepared statement.
     if (mysql_stmt_execute(stmt) != 0) {
-        isc_throw(DbOperationError, "cannot execute schema version query:"
-                  << version_sql << ">, reason: " << mysql_errno(conn_.mysql_) << ")");
+        isc_throw(DbOperationError, "cannot execute schema version query <"
+                  << version_sql << ">, reason: " << mysql_errno(conn_.mysql_));
     }
 
     // Bind the output of the statement to the appropriate variables.
@@ -2421,15 +2423,15 @@ MySqlLeaseMgr::getVersion() const {
     bind[1].buffer_length = sizeof(minor);
 
     if (mysql_stmt_bind_result(stmt, bind)) {
-        isc_throw(DbOperationError, "unable to bind result set for:"
-                << version_sql << ">, reason: " << mysql_errno(conn_.mysql_) << ")");
+        isc_throw(DbOperationError, "unable to bind result set for <"
+                << version_sql << ">, reason: " << mysql_errno(conn_.mysql_));
     }
 
     // Fetch the data.
     if (mysql_stmt_fetch(stmt)) {
         mysql_stmt_close(stmt);
-        isc_throw(DbOperationError, "unable to bind result set for:"
-                << version_sql << ">, reason: " << mysql_errno(conn_.mysql_) << ")");
+        isc_throw(DbOperationError, "unable to bind result set for <"
+                << version_sql << ">, reason: " << mysql_errno(conn_.mysql_));
     }
 
     // Discard the statement and its resources

diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.cc b/src/lib/dhcpsrv/pgsql_host_data_source.cc
index ae632ef22bb57c52a85f74c3308b77d2e790f3ed..91a207efbf05a38afc30a5f3f7b84b03b1c7055a 100644 (file)
--- a/src/lib/dhcpsrv/pgsql_host_data_source.cc
+++ b/src/lib/dhcpsrv/pgsql_host_data_source.cc
@@ -1259,7 +1259,6 @@ public:
         GET_HOST_SUBID_ADDR,    // Gets host by IPv4 SubnetID and IPv4 address
         GET_HOST_PREFIX,        // Gets host by IPv6 prefix
         GET_HOST_SUBID6_ADDR,   // Gets host by IPv6 SubnetID and IPv6 prefix
-        GET_VERSION,            // Obtain version number
         INSERT_HOST,            // Insert new host to collection
         INSERT_V6_RESRV,        // Insert v6 reservation
         INSERT_V4_HOST_OPTION,  // Insert DHCPv4 option
@@ -1606,14 +1605,6 @@ TaggedStatementArray tagged_statements = { {
      "ORDER BY h.host_id, o.option_id, r.reservation_id"
     },
 
-    // PgSqlHostDataSourceImpl::GET_VERSION
-    // Retrieves PgSQL schema version.
-    {0,
-     { OID_NONE },
-     "get_version",
-     "SELECT version, minor FROM schema_version"
-    },
-
     // PgSqlHostDataSourceImpl::INSERT_HOST
     // Inserts a host into the 'hosts' table. Returns the inserted host id.
     {12,
@@ -1709,6 +1700,16 @@ PgSqlHostDataSourceImpl(const PgSqlConnection::ParameterMap& parameters)
     // Open the database.
     conn_.openDatabase();
 
+    pair<uint32_t, uint32_t> code_version(PG_SCHEMA_VERSION_MAJOR, PG_SCHEMA_VERSION_MINOR);
+    pair<uint32_t, uint32_t> db_version = getVersion();
+    if (code_version != db_version) {
+        isc_throw(DbOpenError,
+                  "PostgreSQL schema version mismatch: need version: "
+                      << code_version.first << "." << code_version.second
+                      << " found version:  " << db_version.first << "."
+                      << db_version.second);
+    }
+
     conn_.prepareStatements(tagged_statements.begin(),
                             tagged_statements.begin() + WRITE_STMTS_BEGIN);
 
@@ -1896,15 +1897,18 @@ getHost(const SubnetID& subnet_id,
 std::pair<uint32_t, uint32_t> PgSqlHostDataSourceImpl::getVersion() const {
     LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
               DHCPSRV_PGSQL_HOST_DB_GET_VERSION);
-
-    PgSqlResult r(PQexecPrepared(conn_, "get_version", 0, NULL, NULL, NULL, 0));
-    conn_.checkStatementError(r, tagged_statements[GET_VERSION]);
+    const char* version_sql =  "SELECT version, minor FROM schema_version;";
+    PgSqlResult r(PQexec(conn_, version_sql));
+    if(PQresultStatus(r) != PGRES_TUPLES_OK) {
+        isc_throw(DbOperationError, "unable to execute PostgreSQL statement <"
+                  << version_sql << ">, reason: " << PQerrorMessage(conn_));
+    }
 
     uint32_t version;
     PgSqlExchange::getColumnValue(r, 0, 0, version);
 
     uint32_t minor;
-    PgSqlExchange::getColumnValue(r, 0, 0, minor);
+    PgSqlExchange::getColumnValue(r, 0, 1, minor);
 
     return (std::make_pair(version, minor));
 }

diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.h b/src/lib/dhcpsrv/pgsql_host_data_source.h
index 5a270b449cadd75d66540d42bf1153bbb721ed31..384ff49b7016411454846f21c733791429394522 100644 (file)
--- a/src/lib/dhcpsrv/pgsql_host_data_source.h
+++ b/src/lib/dhcpsrv/pgsql_host_data_source.h
@@ -317,6 +317,11 @@ public:
 
     /// @brief Returns backend version.
     ///
+    /// The method is called by the constructor after opening the database
+    /// but prior to preparing SQL statements, to verify that the schema version
+    /// is correct. Thus it must not rely on a pre-prepared statement or
+    /// formal statement execution error checking.
+    ///
     /// @return Version number stored in the database, as a pair of unsigned
     ///         integers. "first" is the major version number, "second" the
     ///         minor number.

diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc
index f976c5eeb3b80de1ad255d913baf6daf79bbe48e..7e368db7784ac24f933527b1b3e013926d21402e 100644 (file)
--- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc
+++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc
@@ -1680,7 +1680,7 @@ PgSqlLeaseMgr::getVersion() const {
     const char* version_sql =  "SELECT version, minor FROM schema_version;";
     PgSqlResult r(PQexec(conn_, version_sql));
     if(PQresultStatus(r) != PGRES_TUPLES_OK) {
-        isc_throw(DbOperationError, "unable to execute PostgreSQL statement: "
+        isc_throw(DbOperationError, "unable to execute PostgreSQL statement <"
                   << version_sql << ", reason: " << PQerrorMessage(conn_));
     }