From 8a5601acbf74e375b2deffee44652448941868c2 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Tue, 12 Jul 2016 09:43:30 +0200 Subject: [PATCH 01/19] [4489] Created a test for read only backend initialization. --- src/lib/dhcpsrv/db_exceptions.h | 9 +++- .../generic_host_data_source_unittest.cc | 43 +++++++++++++++++++ .../tests/generic_host_data_source_unittest.h | 17 ++++++++ .../tests/mysql_host_data_source_unittest.cc | 6 +++ src/lib/dhcpsrv/testutils/schema.cc | 11 ++++- src/lib/dhcpsrv/testutils/schema.h | 6 ++- 6 files changed, 89 insertions(+), 3 deletions(-) diff --git a/src/lib/dhcpsrv/db_exceptions.h b/src/lib/dhcpsrv/db_exceptions.h index 664c43e144..d9cc4449dd 100644 --- a/src/lib/dhcpsrv/db_exceptions.h +++ b/src/lib/dhcpsrv/db_exceptions.h @@ -1,4 +1,4 @@ -// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2016 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 @@ -40,6 +40,13 @@ public: isc::Exception(file, line, what) {} }; +/// @brief Attempt to modify data in read-only database. +class ReadOnlyDb : public Exception { +public: + ReadOnlyDb(const char* file, size_t line, const char* what) : + isc::Exception(file, line, what) {} +}; + }; }; diff --git a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc index a616f4f081..c12ea64b08 100644 --- a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc @@ -12,8 +12,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -505,6 +507,47 @@ GenericHostDataSourceTest::addTestOptions(const HostPtr& host, LibDHCP::setRuntimeOptionDefs(defs); } +void +GenericHostDataSourceTest::testReadOnlyDatabase(const char* valid_db_type) { + ASSERT_TRUE(hdsptr_); + + // The database is initially opened in "read-write" mode. We can + // insert some data to the databse. + HostPtr host = initializeHost6("2001:db8::1", Host::IDENT_DUID, false); + ASSERT_NO_THROW(hdsptr_->add(host)); + + // Subnet id will be used in queries to the database. + SubnetID subnet_id = host->getIPv6SubnetID(); + + // Make sure that the host has been inserted and that the data can be + // retrieved. + ConstHostPtr host_by_id = hdsptr_->get6(subnet_id, host->getIdentifierType(), + &host->getIdentifier()[0], + host->getIdentifier().size()); + ASSERT_NO_FATAL_FAILURE(compareHosts(host, host_by_id)); + + // Close the database connection and reopen in "read-only" mode as + // specified by the "VALID_READONLY_DB" parameter. + HostDataSourceFactory::destroy(); + HostDataSourceFactory::create(connectionString(valid_db_type, + VALID_NAME, + VALID_HOST, + VALID_USER, + VALID_PASSWORD, + VALID_READONLY_DB)); + + // Check that an attempt to insert new host would result in + // exception. + host = initializeHost6("2001:db8::2", Host::IDENT_DUID, false); + ASSERT_THROW(hdsptr_->add(host), ReadOnlyDb); + + // Reading from the database should still be possible, though. + host_by_id = hdsptr_->get6(subnet_id, host->getIdentifierType(), + &host->getIdentifier()[0], + host->getIdentifier().size()); + ASSERT_NO_FATAL_FAILURE(compareHosts(host, host_by_id)); +} + void GenericHostDataSourceTest::testBasic4(const Host::IdentifierType& id) { // Make sure we have the pointer to the host data source. ASSERT_TRUE(hdsptr_); diff --git a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h index cf5421b19e..bb1e28cb4a 100644 --- a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h +++ b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h @@ -337,6 +337,23 @@ public: /// @brief Pointer to the host data source HostDataSourcePtr hdsptr_; + /// @brief Test that backend can be started in read-only mode. + /// + /// Some backends can operate when the database is read only, e.g. + /// host reservation tables are read only, or the database user has + /// read only privileges on the entire database. In such cases, the + /// Kea server administrator can specify in the backend configuration + /// that the database should be opened in read only mode, i.e. + /// INSERT, UPDATE, DELETE statements can't be issued. If any of the + /// functions updating the database is called for the backend, the + /// error is reported. The database running in read only mode can + /// be merely used to retrieve existing host reservations from the + /// database. This test verifies that this is the case. + /// + /// @param valid_db_type Parameter specifying type of backend to + /// be used, e.g. type=mysql. + void testReadOnlyDatabase(const char* valid_db_type); + /// @brief Test that checks that simple host with IPv4 reservation /// can be inserted and later retrieved. /// diff --git a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc index 450d816268..849e31b40b 100644 --- a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc @@ -167,6 +167,8 @@ TEST(MySqlHostDataSource, OpenDatabase) { destroyMySQLSchema(); } + + /// @brief Check conversion functions /// /// The server works using cltt and valid_filetime. In the database, the @@ -208,6 +210,10 @@ TEST(MySqlConnection, checkTimeConversion) { EXPECT_EQ(cltt, converted_cltt); } +TEST_F(MySqlHostDataSourceTest, testReadOnlyDatabase) { + testReadOnlyDatabase(MYSQL_VALID_TYPE); +} + // Test verifies if a host reservation can be added and later retrieved by IPv4 // address. Host uses hw address as identifier. TEST_F(MySqlHostDataSourceTest, basic4HWAddr) { diff --git a/src/lib/dhcpsrv/testutils/schema.cc b/src/lib/dhcpsrv/testutils/schema.cc index cefcbdae5c..3420cc7c7e 100644 --- a/src/lib/dhcpsrv/testutils/schema.cc +++ b/src/lib/dhcpsrv/testutils/schema.cc @@ -31,9 +31,11 @@ const char* INVALID_PASSWORD = "password=invalid"; const char* VALID_TIMEOUT = "connect-timeout=10"; const char* INVALID_TIMEOUT_1 = "connect-timeout=foo"; const char* INVALID_TIMEOUT_2 = "connect-timeout=-17"; +const char* VALID_READONLY_DB = "readonly=true"; string connectionString(const char* type, const char* name, const char* host, - const char* user, const char* password, const char* timeout) { + const char* user, const char* password, const char* timeout, + const char* readonly_db = NULL) { const string space = " "; string result = ""; @@ -75,6 +77,13 @@ string connectionString(const char* type, const char* name, const char* host, result += string(timeout); } + if (readonly_db != NULL) { + if (! result.empty()) { + result += space; + } + result += string(readonly_db); + } + return (result); } diff --git a/src/lib/dhcpsrv/testutils/schema.h b/src/lib/dhcpsrv/testutils/schema.h index 5a0471512f..fe842ff9e7 100644 --- a/src/lib/dhcpsrv/testutils/schema.h +++ b/src/lib/dhcpsrv/testutils/schema.h @@ -27,6 +27,8 @@ extern const char* INVALID_PASSWORD; extern const char* VALID_TIMEOUT; extern const char* INVALID_TIMEOUT_1; extern const char* INVALID_TIMEOUT_2; +extern const char* VALID_READONLY_DB; + /// @brief Given a combination of strings above, produce a connection string. /// /// @param type type of the database @@ -35,10 +37,12 @@ extern const char* INVALID_TIMEOUT_2; /// @param user username used to authenticate during connection attempt /// @param password password used to authenticate during connection attempt /// @param timeout timeout used during connection attempt +/// @param readonly_db specifies if database is read only /// @return string containing all specified parameters std::string connectionString(const char* type, const char* name = NULL, const char* host = NULL, const char* user = NULL, - const char* password = NULL, const char* timeout = NULL); + const char* password = NULL, const char* timeout = NULL, + const char* readonly_db = NULL); }; }; }; From 65d3817db8bdc4489c9f77673ec4cc123d22c7e0 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 28 Jul 2016 12:56:58 +0200 Subject: [PATCH 02/19] [4489] Added support for read only mode in MySQL HR backend. --- src/lib/dhcpsrv/mysql_connection.cc | 17 ++- src/lib/dhcpsrv/mysql_connection.h | 6 +- src/lib/dhcpsrv/mysql_host_data_source.cc | 141 ++++++++++++------ src/lib/dhcpsrv/mysql_host_data_source.h | 4 +- src/lib/dhcpsrv/mysql_lease_mgr.cc | 14 +- .../generic_host_data_source_unittest.cc | 12 +- .../tests/mysql_host_data_source_unittest.cc | 7 +- src/lib/util/pointer_util.h | 42 +++++- 8 files changed, 179 insertions(+), 64 deletions(-) diff --git a/src/lib/dhcpsrv/mysql_connection.cc b/src/lib/dhcpsrv/mysql_connection.cc index 30de103d53..b12bdd5e19 100644 --- a/src/lib/dhcpsrv/mysql_connection.cc +++ b/src/lib/dhcpsrv/mysql_connection.cc @@ -213,22 +213,27 @@ MySqlConnection::prepareStatement(uint32_t index, const char* text) { } void -MySqlConnection::prepareStatements(const TaggedStatement tagged_statements[], +MySqlConnection::prepareStatements(const TaggedStatement* start_statement, + const TaggedStatement* end_statement, size_t num_statements) { // Allocate space for all statements - statements_.clear(); statements_.resize(num_statements, NULL); - text_statements_.clear(); text_statements_.resize(num_statements, std::string("")); // Created the MySQL prepared statements for each DML statement. - for (int i = 0; tagged_statements[i].text != NULL; ++i) { - prepareStatement(tagged_statements[i].index, - tagged_statements[i].text); + for (const TaggedStatement* tagged_statement = start_statement; + tagged_statement != end_statement; ++tagged_statement) { + prepareStatement(tagged_statement->index, + tagged_statement->text); } } +void MySqlConnection::clearStatements() { + statements_.clear(); + text_statements_.clear(); +} + /// @brief Destructor MySqlConnection::~MySqlConnection() { // Free up the prepared statements, ignoring errors. (What would we do diff --git a/src/lib/dhcpsrv/mysql_connection.h b/src/lib/dhcpsrv/mysql_connection.h index 8acb894577..06fb34d466 100644 --- a/src/lib/dhcpsrv/mysql_connection.h +++ b/src/lib/dhcpsrv/mysql_connection.h @@ -247,9 +247,13 @@ public: /// failed. /// @throw isc::InvalidParameter 'index' is not valid for the vector. This /// represents an internal error within the code. - void prepareStatements(const TaggedStatement tagged_statements[], + void prepareStatements(const TaggedStatement* start_statement, + const TaggedStatement* end_statement, size_t num_statements); + /// @brief Clears prepared statements and text statements. + void clearStatements(); + /// @brief Open Database /// /// Opens the database using the information supplied in the parameters diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc index 150a10391e..70a9aa7a43 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.cc +++ b/src/lib/dhcpsrv/mysql_host_data_source.cc @@ -19,6 +19,7 @@ #include #include +#include #include #include @@ -1633,10 +1634,6 @@ public: /// /// The contents of the enum are indexes into the list of SQL statements enum StatementIndex { - INSERT_HOST, // Insert new host to collection - INSERT_V6_RESRV, // Insert v6 reservation - INSERT_V4_OPTION, // Insert DHCPv4 option - INSERT_V6_OPTION, // Insert DHCPv6 option GET_HOST_DHCPID, // Gets hosts by host identifier GET_HOST_ADDR, // Gets hosts by IPv4 address GET_HOST_SUBID4_DHCPID, // Gets host by IPv4 SubnetID, HW address/DUID @@ -1644,9 +1641,20 @@ public: GET_HOST_SUBID_ADDR, // Gets host by IPv4 SubnetID and IPv4 address GET_HOST_PREFIX, // Gets host by 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 + INSERT_V6_OPTION, // Insert DHCPv6 option NUM_STATEMENTS // Number of statements }; + /// @brief Index of first statement performing write to the database. + /// + /// This value is used to mark border line between queries and other + /// statements and statements performing write operation on the database, + /// such as INSERT, DELETE, UPDATE. + static const StatementIndex WRITE_STMTS_BEGIN = INSERT_HOST; + /// @brief Constructor. /// /// This constructor opens database connection and initializes prepared @@ -1777,39 +1785,20 @@ public: /// @brief MySQL connection MySqlConnection conn_; + /// @brief Indicates if the database is opened in read only mode. + bool is_readonly_; +namespace { + }; -namespace { + +/// @brief Array of tagged statements. +typedef boost::array +TaggedStatementArray; + /// @brief Prepared MySQL statements used by the backend to insert and /// retrieve hosts from the database. -TaggedStatement tagged_statements[] = { - // Inserts a host into the 'hosts' table. - {MySqlHostDataSourceImpl::INSERT_HOST, - "INSERT INTO hosts(host_id, dhcp_identifier, dhcp_identifier_type, " - "dhcp4_subnet_id, dhcp6_subnet_id, ipv4_address, hostname, " - "dhcp4_client_classes, dhcp6_client_classes) " - "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)"}, - - // Inserts a single IPv6 reservation into 'reservations' table. - {MySqlHostDataSourceImpl::INSERT_V6_RESRV, - "INSERT INTO ipv6_reservations(address, prefix_len, type, " - "dhcp6_iaid, host_id) " - "VALUES (?,?,?,?,?)"}, - - // Inserts a single DHCPv4 option into 'dhcp4_options' table. - // Using fixed scope_id = 3, which associates an option with host. - {MySqlHostDataSourceImpl::INSERT_V4_OPTION, - "INSERT INTO dhcp4_options(option_id, code, value, formatted_value, space, " - "persistent, dhcp_client_class, dhcp4_subnet_id, host_id, scope_id) " - " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 3)"}, - - // Inserts a single DHCPv6 option into 'dhcp6_options' table. - // Using fixed scope_id = 3, which associates an option with host. - {MySqlHostDataSourceImpl::INSERT_V6_OPTION, - "INSERT INTO dhcp6_options(option_id, code, value, formatted_value, space, " - "persistent, dhcp_client_class, dhcp6_subnet_id, host_id, scope_id) " - " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 3)"}, - +TaggedStatementArray tagged_statements = { { // Retrieves host information, IPv6 reservations and both DHCPv4 and // DHCPv6 options associated with the host. The LEFT JOIN clause is used // to retrieve information from 4 different tables using a single query. @@ -1931,8 +1920,32 @@ TaggedStatement tagged_statements[] = { {MySqlHostDataSourceImpl::GET_VERSION, "SELECT version, minor FROM schema_version"}, - // Marks the end of the statements table. - {MySqlHostDataSourceImpl::NUM_STATEMENTS, NULL} + // Inserts a host into the 'hosts' table. + {MySqlHostDataSourceImpl::INSERT_HOST, + "INSERT INTO hosts(host_id, dhcp_identifier, dhcp_identifier_type, " + "dhcp4_subnet_id, dhcp6_subnet_id, ipv4_address, hostname, " + "dhcp4_client_classes, dhcp6_client_classes) " + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)"}, + + // Inserts a single IPv6 reservation into 'reservations' table. + {MySqlHostDataSourceImpl::INSERT_V6_RESRV, + "INSERT INTO ipv6_reservations(address, prefix_len, type, " + "dhcp6_iaid, host_id) " + "VALUES (?,?,?,?,?)"}, + + // Inserts a single DHCPv4 option into 'dhcp4_options' table. + // Using fixed scope_id = 3, which associates an option with host. + {MySqlHostDataSourceImpl::INSERT_V4_OPTION, + "INSERT INTO dhcp4_options(option_id, code, value, formatted_value, space, " + "persistent, dhcp_client_class, dhcp4_subnet_id, host_id, scope_id) " + " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 3)"}, + + // Inserts a single DHCPv6 option into 'dhcp6_options' table. + // Using fixed scope_id = 3, which associates an option with host. + {MySqlHostDataSourceImpl::INSERT_V6_OPTION, + "INSERT INTO dhcp6_options(option_id, code, value, formatted_value, space, " + "persistent, dhcp_client_class, dhcp6_subnet_id, host_id, scope_id) " + " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 3)"}} }; }; // end anonymouse namespace @@ -1945,23 +1958,56 @@ MySqlHostDataSourceImpl(const MySqlConnection::ParameterMap& parameters) DHCP4_AND_DHCP6)), host_ipv6_reservation_exchange_(new MySqlIPv6ReservationExchange()), host_option_exchange_(new MySqlOptionExchange()), - conn_(parameters) { + conn_(parameters), + is_readonly_(false) { // Open the database. conn_.openDatabase(); - // Disable autocommit. To avoid a flush to disk on every commit, the global - // parameter innodb_flush_log_at_trx_commit should be set to 2. This will - // cause the changes to be written to the log, but flushed to disk in the - // background every second. Setting the parameter to that value will speed - // up the system, but at the risk of losing data if the system crashes. - my_bool result = mysql_autocommit(conn_.mysql_, 0); + // 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 + // until commit or rollback is explicitly called. This already + // caused issues for some unit tests which were unable to cleanup + // the database after the test because of pending transactions. + // Use of autocommit will eliminate this problem. + my_bool result = mysql_autocommit(conn_.mysql_, 1); if (result != 0) { isc_throw(DbOperationError, mysql_error(conn_.mysql_)); } - // Prepare all statements likely to be used. - conn_.prepareStatements(tagged_statements, NUM_STATEMENTS); + // Prepare query statements. Those are will be only used to retrieve + // information from the database, so they can be used even if the + // database is read only for the current user. + conn_.prepareStatements(tagged_statements.begin(), + tagged_statements.begin() + WRITE_STMTS_BEGIN, + WRITE_STMTS_BEGIN); + + std::string readonly_value = "false"; + try { + readonly_value = conn_.getParameter("readonly"); + boost::algorithm::to_lower(readonly_value); + } catch (...) { + // Parameter "readonly" hasn't been specified so we simply use + // the default value of "false". + } + + if (readonly_value == "true" || readonly_value == "1") { + is_readonly_ = true; + + } else if ((readonly_value != "false") && (readonly_value != "0")) { + isc_throw(BadValue, "invalid value '" << readonly_value + << "' specified for boolean parameter 'readonly'"); + } + + // If we are using read-write mode for the database we also prepare + // statements for INSERTS etc. + if (!is_readonly_) { + // Prepare statements for writing to the database, e.g. INSERT. + conn_.prepareStatements(tagged_statements.begin() + WRITE_STMTS_BEGIN, + tagged_statements.end(), + tagged_statements.size()); + } } MySqlHostDataSourceImpl::~MySqlHostDataSourceImpl() { @@ -2163,11 +2209,14 @@ getHost(const SubnetID& subnet_id, MySqlHostDataSource:: MySqlHostDataSource(const MySqlConnection::ParameterMap& parameters) - : impl_(new MySqlHostDataSourceImpl(parameters)) { + : impl_(new MySqlHostDataSourceImpl(parameters), + "MySQL host database backend is configured to" + " operate in read only mode") { + impl_.allowConstOnly(impl_->is_readonly_); } MySqlHostDataSource::~MySqlHostDataSource() { - delete impl_; + delete impl_.getPtr(); } void diff --git a/src/lib/dhcpsrv/mysql_host_data_source.h b/src/lib/dhcpsrv/mysql_host_data_source.h index 9ce64f2256..6ee563d83b 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.h +++ b/src/lib/dhcpsrv/mysql_host_data_source.h @@ -8,7 +8,9 @@ #define MYSQL_HOST_DATA_SOURCE_H #include +#include #include +#include namespace isc { namespace dhcp { @@ -255,7 +257,7 @@ public: private: /// @brief Pointer to the implementation of the @ref MySqlHostDataSource. - MySqlHostDataSourceImpl* impl_; + util::RestrictedConstPtr impl_; }; } diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index 410880af36..7e448428f2 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2016 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 @@ -13,6 +13,7 @@ #include #include +#include #include #include @@ -82,7 +83,8 @@ const size_t HOSTNAME_MAX_LEN = 255; /// colon separators. const size_t ADDRESS6_TEXT_MAX_LEN = 39; -TaggedStatement tagged_statements[] = { +boost::array +tagged_statements = { { {MySqlLeaseMgr::DELETE_LEASE4, "DELETE FROM lease4 WHERE address = ?"}, {MySqlLeaseMgr::DELETE_LEASE4_STATE_EXPIRED, @@ -204,9 +206,8 @@ TaggedStatement tagged_statements[] = { "prefix_len = ?, fqdn_fwd = ?, fqdn_rev = ?, " "hostname = ?, hwaddr = ?, hwtype = ?, hwaddr_source = ?, " "state = ? " - "WHERE address = ?"}, - // End of list sentinel - {MySqlLeaseMgr::NUM_STATEMENTS, NULL} + "WHERE address = ?"} + } }; }; @@ -1236,7 +1237,8 @@ MySqlLeaseMgr::MySqlLeaseMgr(const MySqlConnection::ParameterMap& parameters) } // Prepare all statements likely to be used. - conn_.prepareStatements(tagged_statements, MySqlLeaseMgr::NUM_STATEMENTS); + conn_.prepareStatements(tagged_statements.begin(), tagged_statements.end(), + tagged_statements.size()); // Create the exchange objects for use in exchanging data between the // program and the database. diff --git a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc index c12ea64b08..fe4560a40b 100644 --- a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc @@ -514,6 +514,7 @@ GenericHostDataSourceTest::testReadOnlyDatabase(const char* valid_db_type) { // The database is initially opened in "read-write" mode. We can // insert some data to the databse. HostPtr host = initializeHost6("2001:db8::1", Host::IDENT_DUID, false); + ASSERT_TRUE(host); ASSERT_NO_THROW(hdsptr_->add(host)); // Subnet id will be used in queries to the database. @@ -524,6 +525,7 @@ GenericHostDataSourceTest::testReadOnlyDatabase(const char* valid_db_type) { ConstHostPtr host_by_id = hdsptr_->get6(subnet_id, host->getIdentifierType(), &host->getIdentifier()[0], host->getIdentifier().size()); + ASSERT_TRUE(host_by_id); ASSERT_NO_FATAL_FAILURE(compareHosts(host, host_by_id)); // Close the database connection and reopen in "read-only" mode as @@ -536,15 +538,21 @@ GenericHostDataSourceTest::testReadOnlyDatabase(const char* valid_db_type) { VALID_PASSWORD, VALID_READONLY_DB)); + hdsptr_ = HostDataSourceFactory::getHostDataSourcePtr(); + // Check that an attempt to insert new host would result in // exception. - host = initializeHost6("2001:db8::2", Host::IDENT_DUID, false); - ASSERT_THROW(hdsptr_->add(host), ReadOnlyDb); + HostPtr host2 = initializeHost6("2001:db8::2", Host::IDENT_DUID, false); + ASSERT_TRUE(host2); + ASSERT_THROW(hdsptr_->add(host2), ReadOnlyDb); + ASSERT_THROW(hdsptr_->commit(), ReadOnlyDb); + ASSERT_THROW(hdsptr_->rollback(), ReadOnlyDb); // Reading from the database should still be possible, though. host_by_id = hdsptr_->get6(subnet_id, host->getIdentifierType(), &host->getIdentifier()[0], host->getIdentifier().size()); + ASSERT_TRUE(host_by_id); ASSERT_NO_FATAL_FAILURE(compareHosts(host, host_by_id)); } diff --git a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc index 849e31b40b..058b2e358f 100644 --- a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc @@ -63,8 +63,13 @@ public: /// Rolls back all pending transactions. The deletion of myhdsptr_ will close /// the database. Then reopen it and delete everything created by the test. virtual ~MySqlHostDataSourceTest() { - hdsptr_->rollback(); + try { + hdsptr_->rollback(); + } catch (...) { + // Rollback may fail if backend is in read only mode. That's ok. + } HostDataSourceFactory::destroy(); + hdsptr_.reset(); destroyMySQLSchema(); } diff --git a/src/lib/util/pointer_util.h b/src/lib/util/pointer_util.h index a775584573..ed8374722f 100644 --- a/src/lib/util/pointer_util.h +++ b/src/lib/util/pointer_util.h @@ -1,4 +1,4 @@ -// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2016 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 @@ -7,9 +7,49 @@ #ifndef POINTER_UTIL_H #define POINTER_UTIL_H +#include +#include + namespace isc { namespace util { +template +class RestrictedConstPtr { +public: + + RestrictedConstPtr(T* ptr, const std::string& error_text) + : ptr_(ptr), const_only_(false), error_text_(error_text) { + } + + void allowConstOnly(const bool const_only) { + const_only_ = const_only; + } + + T* operator->() const { + return (ptr_); + } + + T* operator->() { + if (const_only_) { + isc_throw(E, error_text_); + } + return (ptr_); + } + + T* getPtr() const { + return (ptr_); + } + +private: + + T* ptr_; + + bool const_only_; + + std::string error_text_; +}; + + /// @brief This function checks if two pointers are non-null and values /// are equal. /// From 4c07c122b28fdbfe3532c64ebf4b53b62c610936 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Tue, 12 Jul 2016 11:04:06 +0200 Subject: [PATCH 03/19] [4489] Added unit test for read only mode in PgSQL backend. --- src/lib/dhcpsrv/mysql_host_data_source.cc | 4 ---- .../tests/pgsql_host_data_source_unittest.cc | 13 ++++++++++++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc index 70a9aa7a43..a45ef83567 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.cc +++ b/src/lib/dhcpsrv/mysql_host_data_source.cc @@ -1787,8 +1787,6 @@ public: /// @brief Indicates if the database is opened in read only mode. bool is_readonly_; -namespace { - }; @@ -1948,8 +1946,6 @@ TaggedStatementArray tagged_statements = { { " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 3)"}} }; -}; // end anonymouse namespace - MySqlHostDataSourceImpl:: MySqlHostDataSourceImpl(const MySqlConnection::ParameterMap& parameters) : host_exchange_(new MySqlHostWithOptionsExchange(MySqlHostWithOptionsExchange::DHCP4_ONLY)), diff --git a/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc index b58b60ee7a..2c4b13c005 100644 --- a/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc @@ -64,8 +64,13 @@ public: /// close the database. Then reopen it and delete everything created by /// the test. virtual ~PgSqlHostDataSourceTest() { - hdsptr_->rollback(); + try { + hdsptr_->rollback(); + } catch (...) { + // Rollback may fail if backend is in read only mode. That's ok. + } HostDataSourceFactory::destroy(); + hdsptr_.reset(); destroyPgSQLSchema(); } @@ -168,6 +173,12 @@ TEST(PgSqlHostDataSource, OpenDatabase) { destroyPgSQLSchema(); } + +// This test verifies that database backend can operate in Read-Only mode. +TEST_F(PgSqlHostDataSourceTest, testReadOnlyDatabase) { + testReadOnlyDatabase(PGSQL_VALID_TYPE); +} + // Test verifies if a host reservation can be added and later retrieved by IPv4 // address. Host uses hw address as identifier. TEST_F(PgSqlHostDataSourceTest, basic4HWAddr) { From ef05ff339c5b11a93e42e7831f7dd324cc2720f1 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Wed, 13 Jul 2016 08:08:52 +0200 Subject: [PATCH 04/19] [4489] Added 'readonly' parameter to DbAccessParser. --- src/bin/dhcp4/dhcp4.spec | 6 +++ src/bin/dhcp6/dhcp6.spec | 6 +++ src/lib/dhcpsrv/database_connection.h | 9 ++++ src/lib/dhcpsrv/mysql_host_data_source.cc | 6 +-- src/lib/dhcpsrv/parsers/dbaccess_parser.cc | 5 +- .../dhcpsrv/tests/dbaccess_parser_unittest.cc | 46 ++++++++++++++++++- .../tests/mysql_host_data_source_unittest.cc | 3 ++ src/lib/dhcpsrv/testutils/schema.cc | 1 + src/lib/dhcpsrv/testutils/schema.h | 1 + 9 files changed, 76 insertions(+), 7 deletions(-) diff --git a/src/bin/dhcp4/dhcp4.spec b/src/bin/dhcp4/dhcp4.spec index 8e24e6992e..43940ef68b 100644 --- a/src/bin/dhcp4/dhcp4.spec +++ b/src/bin/dhcp4/dhcp4.spec @@ -275,6 +275,12 @@ "item_type": "integer", "item_optional": true, "item_default": 0 + }, + { + "item_name": "readonly", + "item_type": "boolean", + "item_optional": true, + "item_default": false } ] }, diff --git a/src/bin/dhcp6/dhcp6.spec b/src/bin/dhcp6/dhcp6.spec index 3571b8e2ac..d824290647 100644 --- a/src/bin/dhcp6/dhcp6.spec +++ b/src/bin/dhcp6/dhcp6.spec @@ -302,6 +302,12 @@ "item_type": "integer", "item_optional": true, "item_default": 0 + }, + { + "item_name": "readonly", + "item_type": "boolean", + "item_optional": true, + "item_default": false } ] }, diff --git a/src/lib/dhcpsrv/database_connection.h b/src/lib/dhcpsrv/database_connection.h index 2be98baf41..c18c16f28c 100644 --- a/src/lib/dhcpsrv/database_connection.h +++ b/src/lib/dhcpsrv/database_connection.h @@ -54,6 +54,15 @@ public: isc::Exception(file, line, what) {} }; +/// @brief Invalid 'readonly' value specification. +/// +/// Thrown when the value of the 'readonly' boolean parameter is invalid. +class DbInvalidReadOnly : public Exception { +public: + DbInvalidReadOnly(const char* file, size_t line, const char* what) : + isc::Exception(file, line, what) {} +}; + /// @brief Common database connection class. /// diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc index a45ef83567..c4b97757e0 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.cc +++ b/src/lib/dhcpsrv/mysql_host_data_source.cc @@ -1988,11 +1988,11 @@ MySqlHostDataSourceImpl(const MySqlConnection::ParameterMap& parameters) // the default value of "false". } - if (readonly_value == "true" || readonly_value == "1") { + if (readonly_value == "true") { is_readonly_ = true; - } else if ((readonly_value != "false") && (readonly_value != "0")) { - isc_throw(BadValue, "invalid value '" << readonly_value + } else if (readonly_value != "false") { + isc_throw(DbInvalidReadOnly, "invalid value '" << readonly_value << "' specified for boolean parameter 'readonly'"); } diff --git a/src/lib/dhcpsrv/parsers/dbaccess_parser.cc b/src/lib/dhcpsrv/parsers/dbaccess_parser.cc index 5386e9d2d6..2de5485175 100644 --- a/src/lib/dhcpsrv/parsers/dbaccess_parser.cc +++ b/src/lib/dhcpsrv/parsers/dbaccess_parser.cc @@ -53,7 +53,7 @@ DbAccessParser::build(isc::data::ConstElementPtr config_value) { // 2. Update the copy with the passed keywords. BOOST_FOREACH(ConfigPair param, config_value->mapValue()) { try { - if (param.first == "persist") { + if ((param.first == "persist") || (param.first == "readonly")) { values_copy[param.first] = (param.second->boolValue() ? "true" : "false"); @@ -72,7 +72,8 @@ DbAccessParser::build(isc::data::ConstElementPtr config_value) { } } catch (const isc::data::TypeError& ex) { // Append position of the element. - isc_throw(isc::data::TypeError, ex.what() << " (" + isc_throw(BadValue, "invalid value type specified for " + "parameter '" << param.first << "' (" << param.second->getPosition() << ")"); } } diff --git a/src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc b/src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc index 846808da1f..e71dc2530f 100644 --- a/src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc +++ b/src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc @@ -88,7 +88,7 @@ public: } // Add the keyword and value - make sure that they are quoted. - // The parameters which are not quoted are persist and + // The parameters which are not quoted are persist, readonly and // lfc-interval as they are boolean and integer respectively. result += quote + keyval[i] + quote + colon + space; if (!quoteValue(std::string(keyval[i]))) { @@ -176,7 +176,8 @@ private: /// @return true if the value of the parameter should be quoted. bool quoteValue(const std::string& parameter) const { return ((parameter != "persist") && (parameter != "lfc-interval") && - (parameter != "connect-timeout")); + (parameter != "connect-timeout") && + (parameter != "readonly")); } }; @@ -560,4 +561,45 @@ TEST_F(DbAccessParserTest, getDbAccessString) { EXPECT_EQ(dbaccess, "name=keatest type=mysql"); } +// Check that the configuration is accepted for the valid value +// of "readonly". +TEST_F(DbAccessParserTest, validReadOnly) { + const char* config[] = {"type", "mysql", + "user", "keatest", + "password", "keatest", + "name", "keatest", + "readonly", "true", + NULL}; + + string json_config = toJson(config); + ConstElementPtr json_elements = Element::fromJSON(json_config); + EXPECT_TRUE(json_elements); + + TestDbAccessParser parser("lease-database", DbAccessParser::LEASE_DB); + EXPECT_NO_THROW(parser.build(json_elements)); + + checkAccessString("Valid readonly parameter", + parser.getDbAccessParameters(), + config); +} + +// Check that for the invalid value of the "readonly" parameter +// an exception is thrown. +TEST_F(DbAccessParserTest, invalidReadOnly) { + const char* config[] = {"type", "mysql", + "user", "keatest", + "password", "keatest", + "name", "keatest", + "readonly", "1", + NULL}; + + string json_config = toJson(config); + ConstElementPtr json_elements = Element::fromJSON(json_config); + EXPECT_TRUE(json_elements); + + TestDbAccessParser parser("lease-database", DbAccessParser::LEASE_DB); + EXPECT_THROW(parser.build(json_elements), BadValue); +} + + }; // Anonymous namespace diff --git a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc index 058b2e358f..8f57f1c21e 100644 --- a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc @@ -162,6 +162,9 @@ TEST(MySqlHostDataSource, OpenDatabase) { EXPECT_THROW(HostDataSourceFactory::create(connectionString( MYSQL_VALID_TYPE, VALID_NAME, VALID_HOST, VALID_USER, VALID_PASSWORD, INVALID_TIMEOUT_2)), DbInvalidTimeout); + EXPECT_THROW(HostDataSourceFactory::create(connectionString( + MYSQL_VALID_TYPE, VALID_NAME, VALID_HOST, VALID_USER, VALID_PASSWORD, + VALID_TIMEOUT, INVALID_READONLY_DB)), DbInvalidReadOnly); // Check for missing parameters EXPECT_THROW(HostDataSourceFactory::create(connectionString( diff --git a/src/lib/dhcpsrv/testutils/schema.cc b/src/lib/dhcpsrv/testutils/schema.cc index 3420cc7c7e..f782f27f34 100644 --- a/src/lib/dhcpsrv/testutils/schema.cc +++ b/src/lib/dhcpsrv/testutils/schema.cc @@ -32,6 +32,7 @@ const char* VALID_TIMEOUT = "connect-timeout=10"; const char* INVALID_TIMEOUT_1 = "connect-timeout=foo"; const char* INVALID_TIMEOUT_2 = "connect-timeout=-17"; const char* VALID_READONLY_DB = "readonly=true"; +const char* INVALID_READONLY_DB = "readonly=5"; string connectionString(const char* type, const char* name, const char* host, const char* user, const char* password, const char* timeout, diff --git a/src/lib/dhcpsrv/testutils/schema.h b/src/lib/dhcpsrv/testutils/schema.h index fe842ff9e7..97c4627f1b 100644 --- a/src/lib/dhcpsrv/testutils/schema.h +++ b/src/lib/dhcpsrv/testutils/schema.h @@ -28,6 +28,7 @@ extern const char* VALID_TIMEOUT; extern const char* INVALID_TIMEOUT_1; extern const char* INVALID_TIMEOUT_2; extern const char* VALID_READONLY_DB; +extern const char* INVALID_READONLY_DB; /// @brief Given a combination of strings above, produce a connection string. /// From 4e0e41f0609dab1bf85ee50dab43b4d1a66d9195 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 28 Jul 2016 16:21:42 +0200 Subject: [PATCH 05/19] [4489] Added support for read only mode in PostgreSQL HR backend. --- src/lib/dhcpsrv/pgsql_connection.cc | 10 ++ src/lib/dhcpsrv/pgsql_connection.h | 11 ++ src/lib/dhcpsrv/pgsql_host_data_source.cc | 179 ++++++++++++++-------- src/lib/dhcpsrv/pgsql_host_data_source.h | 14 +- 4 files changed, 145 insertions(+), 69 deletions(-) diff --git a/src/lib/dhcpsrv/pgsql_connection.cc b/src/lib/dhcpsrv/pgsql_connection.cc index 9d722a458a..e79712c2fa 100644 --- a/src/lib/dhcpsrv/pgsql_connection.cc +++ b/src/lib/dhcpsrv/pgsql_connection.cc @@ -129,6 +129,16 @@ PgSqlConnection::prepareStatement(const PgSqlTaggedStatement& statement) { } } +void +PgSqlConnection::prepareStatements(const PgSqlTaggedStatement* start_statement, + const PgSqlTaggedStatement* end_statement) { + // Created the PostgreSQL prepared statements. + for (const PgSqlTaggedStatement* tagged_statement = start_statement; + tagged_statement != end_statement; ++tagged_statement) { + prepareStatement(*tagged_statement); + } +} + void PgSqlConnection::openDatabase() { string dbconnparameters; diff --git a/src/lib/dhcpsrv/pgsql_connection.h b/src/lib/dhcpsrv/pgsql_connection.h index 92bcd4b5ed..2ff1831203 100644 --- a/src/lib/dhcpsrv/pgsql_connection.h +++ b/src/lib/dhcpsrv/pgsql_connection.h @@ -313,6 +313,17 @@ public: /// failed. void prepareStatement(const PgSqlTaggedStatement& statement); + /// @brief Prepare statements + /// + /// Creates the prepared statements for all of the SQL statements used + /// by the PostgreSQL backend. + /// @param tagged_statements an array of statements to be compiled + /// + /// @throw isc::dhcp::DbOperationError An operation on the open database has + /// failed. + void prepareStatements(const PgSqlTaggedStatement* start_statement, + const PgSqlTaggedStatement* end_statement); + /// @brief Open Database /// /// Opens the database using the information supplied in the parameters diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.cc b/src/lib/dhcpsrv/pgsql_host_data_source.cc index 33a75ed262..2d4e57ba03 100644 --- a/src/lib/dhcpsrv/pgsql_host_data_source.cc +++ b/src/lib/dhcpsrv/pgsql_host_data_source.cc @@ -19,6 +19,7 @@ #include #include +#include #include #include @@ -35,7 +36,7 @@ namespace { /// @brief Maximum length of option value. /// The maximum size of the raw option data that may be read from the -/// database. +/// database. const size_t OPTION_VALUE_MAX_LEN = 4096; /// @brief Numeric value representing last supported identifier. @@ -1105,10 +1106,6 @@ public: /// /// The contents of the enum are indexes into the list of SQL statements enum StatementIndex { - INSERT_HOST, // Insert new host to collection - INSERT_V6_RESRV, // Insert v6 reservation - INSERT_V4_HOST_OPTION, // Insert DHCPv4 option - INSERT_V6_HOST_OPTION, // Insert DHCPv6 option GET_HOST_DHCPID, // Gets hosts by host identifier GET_HOST_ADDR, // Gets hosts by IPv4 address GET_HOST_SUBID4_DHCPID, // Gets host by IPv4 SubnetID, HW address/DUID @@ -1116,9 +1113,20 @@ public: GET_HOST_SUBID_ADDR, // Gets host by IPv4 SubnetID and IPv4 address GET_HOST_PREFIX, // Gets host by 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 + INSERT_V6_HOST_OPTION, // Insert DHCPv6 option NUM_STATEMENTS // Number of statements }; + /// @brief Index of first statement performing write to the database. + /// + /// This value is used to mark border line between queries and other + /// statements and statements performing write operation on the database, + /// such as INSERT, DELETE, UPDATE. + static const StatementIndex WRITE_STMTS_BEGIN = INSERT_HOST; + /// @brief Constructor. /// /// This constructor opens database connection and initializes prepared @@ -1258,66 +1266,25 @@ public: /// @brief MySQL connection PgSqlConnection conn_; + /// @brief Indicates if the database is opened in read only mode. + bool is_readonly_; }; namespace { +/// @brief Array of tagged statements. +typedef boost::array +TaggedStatementArray; + /// @brief Prepared PosgreSQL statements used by the backend to insert and /// retrieve reservation data from the database. -PgSqlTaggedStatement tagged_statements[] = { - // PgSqlHostDataSourceImpl::INSERT_HOST - // Inserts a host into the 'hosts' table. Returns the inserted host id. - {8, - { OID_BYTEA, OID_INT2, - OID_INT4, OID_INT4, OID_INT8, OID_VARCHAR, - OID_VARCHAR, OID_VARCHAR }, - "insert_host", - "INSERT INTO hosts(dhcp_identifier, dhcp_identifier_type, " - " dhcp4_subnet_id, dhcp6_subnet_id, ipv4_address, hostname, " - " dhcp4_client_classes, dhcp6_client_classes) " - "VALUES ($1, $2, $3, $4, $5, $6, $7, $8) RETURNING host_id" - }, - - //PgSqlHostDataSourceImpl::INSERT_V6_RESRV - // Inserts a single IPv6 reservation into 'reservations' table. - {5, - { OID_VARCHAR, OID_INT2, OID_INT4, OID_INT4, OID_INT4 }, - "insert_v6_resrv", - "INSERT INTO ipv6_reservations(address, prefix_len, type, " - " dhcp6_iaid, host_id) " - "VALUES ($1, $2, $3, $4, $5)" - }, - - // PgSqlHostDataSourceImpl::INSERT_V4_HOST_OPTION - // Inserts a single DHCPv4 option into 'dhcp4_options' table. - // Using fixed scope_id = 3, which associates an option with host. - {6, - { OID_INT2, OID_BYTEA, OID_TEXT, - OID_VARCHAR, OID_BOOL, OID_INT8}, - "insert_v4_host_option", - "INSERT INTO dhcp4_options(code, value, formatted_value, space, " - " persistent, host_id, scope_id) " - "VALUES ($1, $2, $3, $4, $5, $6, 3)" - }, - - // PgSqlHostDataSourceImpl::INSERT_V6_HOST_OPTION - // Inserts a single DHCPv6 option into 'dhcp6_options' table. - // Using fixed scope_id = 3, which associates an option with host. - {6, - { OID_INT2, OID_BYTEA, OID_TEXT, - OID_VARCHAR, OID_BOOL, OID_INT8}, - "insert_v6_host_option", - "INSERT INTO dhcp6_options(code, value, formatted_value, space, " - " persistent, host_id, scope_id) " - "VALUES ($1, $2, $3, $4, $5, $6, 3)" - }, - +TaggedStatementArray tagged_statements = { { // PgSqlHostDataSourceImpl::GET_HOST_DHCPID // Retrieves host information, IPv6 reservations and both DHCPv4 and // DHCPv6 options associated with the host. The LEFT JOIN clause is used // to retrieve information from 4 different tables using a single query. // Hence, this query returns multiple rows for a single host. - {2, + {2, { OID_BYTEA, OID_INT2 }, "get_host_dhcpid", "SELECT h.host_id, h.dhcp_identifier, h.dhcp_identifier_type, " @@ -1417,7 +1384,7 @@ PgSqlTaggedStatement tagged_statements[] = { // are returned due to left joining IPv6 reservations and DHCPv6 options. // The number of rows returned is multiplication of number of existing // IPv6 reservations and DHCPv6 options. - {2, + {2, { OID_VARCHAR, OID_INT2 }, "get_host_prefix", "SELECT h.host_id, h.dhcp_identifier, " @@ -1439,14 +1406,59 @@ PgSqlTaggedStatement tagged_statements[] = { //PgSqlHostDataSourceImpl::GET_VERSION // Retrieves MySQL schema version. - {0, + {0, { OID_NONE }, "get_version", "SELECT version, minor FROM schema_version" }, - // Marks the end of the statements table. - {0, { 0 }, NULL, NULL} + // PgSqlHostDataSourceImpl::INSERT_HOST + // Inserts a host into the 'hosts' table. Returns the inserted host id. + {8, + { OID_BYTEA, OID_INT2, + OID_INT4, OID_INT4, OID_INT8, OID_VARCHAR, + OID_VARCHAR, OID_VARCHAR }, + "insert_host", + "INSERT INTO hosts(dhcp_identifier, dhcp_identifier_type, " + " dhcp4_subnet_id, dhcp6_subnet_id, ipv4_address, hostname, " + " dhcp4_client_classes, dhcp6_client_classes) " + "VALUES ($1, $2, $3, $4, $5, $6, $7, $8) RETURNING host_id" + }, + + //PgSqlHostDataSourceImpl::INSERT_V6_RESRV + // Inserts a single IPv6 reservation into 'reservations' table. + {5, + { OID_VARCHAR, OID_INT2, OID_INT4, OID_INT4, OID_INT4 }, + "insert_v6_resrv", + "INSERT INTO ipv6_reservations(address, prefix_len, type, " + " dhcp6_iaid, host_id) " + "VALUES ($1, $2, $3, $4, $5)" + }, + + // PgSqlHostDataSourceImpl::INSERT_V4_HOST_OPTION + // Inserts a single DHCPv4 option into 'dhcp4_options' table. + // Using fixed scope_id = 3, which associates an option with host. + {6, + { OID_INT2, OID_BYTEA, OID_TEXT, + OID_VARCHAR, OID_BOOL, OID_INT8}, + "insert_v4_host_option", + "INSERT INTO dhcp4_options(code, value, formatted_value, space, " + " persistent, host_id, scope_id) " + "VALUES ($1, $2, $3, $4, $5, $6, 3)" + }, + + // PgSqlHostDataSourceImpl::INSERT_V6_HOST_OPTION + // Inserts a single DHCPv6 option into 'dhcp6_options' table. + // Using fixed scope_id = 3, which associates an option with host. + {6, + { OID_INT2, OID_BYTEA, OID_TEXT, + OID_VARCHAR, OID_BOOL, OID_INT8}, + "insert_v6_host_option", + "INSERT INTO dhcp6_options(code, value, formatted_value, space, " + " persistent, host_id, scope_id) " + "VALUES ($1, $2, $3, $4, $5, $6, 3)" + } +} }; }; // end anonymous namespace @@ -1459,20 +1471,37 @@ PgSqlHostDataSourceImpl(const PgSqlConnection::ParameterMap& parameters) DHCP4_AND_DHCP6)), host_ipv6_reservation_exchange_(new PgSqlIPv6ReservationExchange()), host_option_exchange_(new PgSqlOptionExchange()), - conn_(parameters) { + conn_(parameters), + is_readonly_(false) { // Open the database. conn_.openDatabase(); - int i = 0; - for( ; tagged_statements[i].text != NULL ; ++i) { - conn_.prepareStatement(tagged_statements[i]); + conn_.prepareStatements(tagged_statements.begin(), + tagged_statements.begin() + WRITE_STMTS_BEGIN); + + std::string readonly_value = "false"; + try { + readonly_value = conn_.getParameter("readonly"); + boost::algorithm::to_lower(readonly_value); + } catch (...) { + // Parameter "readonly" hasn't been specified so we simply use + // the default value of "false". } - // Just in case somebody foo-barred things - if (i != NUM_STATEMENTS) { - isc_throw(DbOpenError, "Number of statements prepared: " << i - << " does not match expected count:" << NUM_STATEMENTS); + if (readonly_value == "true") { + is_readonly_ = true; + + } else if (readonly_value != "false") { + isc_throw(DbInvalidReadOnly, "invalid value '" << readonly_value + << "' specified for boolean parameter 'readonly'"); + } + + // If we are using read-write mode for the database we also prepare + // statements for INSERTS etc. + if (!is_readonly_) { + conn_.prepareStatements(tagged_statements.begin() + WRITE_STMTS_BEGIN, + tagged_statements.end()); } } @@ -1637,11 +1666,14 @@ std::pair PgSqlHostDataSourceImpl::getVersion() const { PgSqlHostDataSource:: PgSqlHostDataSource(const PgSqlConnection::ParameterMap& parameters) - : impl_(new PgSqlHostDataSourceImpl(parameters)) { + : impl_(new PgSqlHostDataSourceImpl(parameters), + "PostgreSQL host database backend is configured to" + " operate in read only mode") { + impl_.allowConstOnly(impl_->is_readonly_); } PgSqlHostDataSource::~PgSqlHostDataSource() { - delete impl_; + delete impl_.getPtr(); } void @@ -1894,5 +1926,16 @@ std::pair PgSqlHostDataSource::getVersion() const { return(impl_->getVersion()); } +void +PgSqlHostDataSource::commit() { + impl_->conn_.commit(); +} + + +void +PgSqlHostDataSource::rollback() { + impl_->conn_.rollback(); +} + }; // end of isc::dhcp namespace }; // end of isc namespace diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.h b/src/lib/dhcpsrv/pgsql_host_data_source.h index 2d7ab6c10b..ab062efc3c 100644 --- a/src/lib/dhcpsrv/pgsql_host_data_source.h +++ b/src/lib/dhcpsrv/pgsql_host_data_source.h @@ -8,8 +8,10 @@ #define PGSQL_HOST_DATA_SOURCE_H #include +#include #include #include +#include namespace isc { namespace dhcp { @@ -273,10 +275,20 @@ public: /// has failed. virtual std::pair getVersion() const; + /// @brief Commit Transactions + /// + /// Commits all pending database operations. + virtual void commit(); + + /// @brief Rollback Transactions + /// + /// Rolls back all pending database operations. + virtual void rollback(); + private: /// @brief Pointer to the implementation of the @ref PgSqlHostDataSource. - PgSqlHostDataSourceImpl* impl_; + util::RestrictedConstPtr impl_; }; } From fe0424b0321a31aa64e4e1a6d600ae71980ddd8e Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 28 Jul 2016 18:33:27 +0200 Subject: [PATCH 06/19] [4489] Updated documentation for "readonly" database parameter. --- doc/guide/admin.xml | 25 ++++++++++++++++++++----- doc/guide/dhcp4-srv.xml | 32 ++++++++++++++++++++++++++++++++ doc/guide/dhcp6-srv.xml | 35 ++++++++++++++++++++++++++++++++++- 3 files changed, 86 insertions(+), 6 deletions(-) diff --git a/doc/guide/admin.xml b/doc/guide/admin.xml index 7628279c21..ccfe991d77 100644 --- a/doc/guide/admin.xml +++ b/doc/guide/admin.xml @@ -11,11 +11,13 @@ Databases and Database Version Numbers - Kea stores leases in one of several supported databases. - As future versions of Kea are released, the structure of those - databases will change. For example, Kea currently only stores - lease information: in the future, additional data - such as host - reservation details - will also be stored. + Kea supports storing leases and host reservations (i.e. static + assignments of addresses, prefixes and options) in one of + the several supported databases. As future versions of Kea + are released, the structure of those databases will change. + For example, Kea currently only stores lease information + and host reservations: in the future, additional + data - such as subnet definitions - will also be stored. @@ -705,6 +707,18 @@ $ kea-admin lease-upgrade cql -n database-name +
+ Using read only databases with host reservations + If read only database is used for storing host reservations, + Kea must be explicitly configured to operate on the database in + the read only mode. + Sections and + describe when + such configuration may be desired and how to configure Kea to + operate using read only host database. + +
+
Limitations related to the use of the SQL databases @@ -721,6 +735,7 @@ $ kea-admin lease-upgrade cql -n database-name
+ diff --git a/doc/guide/dhcp4-srv.xml b/doc/guide/dhcp4-srv.xml index 352f50f667..63a99d818f 100644 --- a/doc/guide/dhcp4-srv.xml +++ b/doc/guide/dhcp4-srv.xml @@ -539,6 +539,38 @@ If a timeout is given though, it should be an integer greater than zero. If there is no password to the account, set the password to the empty string "". (This is also the default.) + +
+Using Read Only Databases for Host Reservations + +In some deployments the database user, which name is specified in the database backend +configuration, may not have write privileges to the database. This is often +required by the policy within a given network to secure the data from being +unintentionally modified. In many cases administrators have inventory databases +deployed, which contain substantially more information about the hosts than +static reservations assigned to them. Such database can be used to create +a view of a Kea hosts database and such view is often read only. + + +Kea host database backends operate with implicit configuration to both read +and write from the database. If the host database is read only for the +particular user, the backend will fail to start and consequently the server +will refuse to start/reconfigure. If the administrator intends to use the +read only host database for retrieving reservations for clients, to assign +specific addresses and options, it is possible to explicitly configure +Kea to start in "read-only" mode. This is controlled by the +readonly boolean parameter as follows: + +"Dhcp4": { "hosts-database": { "readonly": true, ... }, ... } + +Setting this parameter to false would configure the +database backend to operate in "read-write" mode, which is also a default +configuration if the parameter is not specified. + +The readonly parameter is currently only supported +for MySQL and PostgreSQL databases. +
+
diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml index 5b1d5c5858..a12666e381 100644 --- a/doc/guide/dhcp6-srv.xml +++ b/doc/guide/dhcp6-srv.xml @@ -477,6 +477,7 @@ If a timeout is given though, it should be an integer greater than zero. If there is no password to the account, set the password to the empty string "". (This is also the default.)
+
@@ -539,8 +540,40 @@ If a timeout is given though, it should be an integer greater than zero. If there is no password to the account, set the password to the empty string "". (This is also the default.)
+ +
+Using Read Only Databases for Host Reservations + +In some deployments the database user, which name is specified in the database backend +configuration, may not have write privileges to the database. This is often +required by the policy within a given network to secure the data from being +unintentionally modified. In many cases administrators have inventory databases +deployed, which contain substantially more information about the hosts than +static reservations assigned to them. Such database can be used to create +a view of a Kea hosts database and such view is often read only. + + +Kea host database backends operate with implicit configuration to both read +and write from the database. If the host database is read only for the +particular user, the backend will fail to start and consequently the server +will refuse to start/reconfigure. If the administrator intends to use the +read only host database for retrieving reservations for clients, to assign +specific addresses and options, it is possible to explicitly configure +Kea to start in "read-only" mode. This is controlled by the +readonly boolean parameter as follows: + +"Dhcp4": { "hosts-database": { "readonly": true, ... }, ... } + +Setting this parameter to false would configure the +database backend to operate in "read-write" mode, which is also a default +configuration if the parameter is not specified. + +The readonly parameter is currently only supported +for MySQL and PostgreSQL databases.
---> + + +
Interface selection From bfb36f3d0ca1ac049eba25ae4e573df7a4c7fbd1 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 28 Jul 2016 21:08:43 +0200 Subject: [PATCH 07/19] [4489] Removed RestrictedConstPtr class. --- src/lib/dhcpsrv/mysql_host_data_source.cc | 32 ++++++++++++++--- src/lib/dhcpsrv/mysql_host_data_source.h | 3 +- src/lib/dhcpsrv/pgsql_host_data_source.cc | 32 ++++++++++++++--- src/lib/dhcpsrv/pgsql_host_data_source.h | 4 +-- src/lib/util/pointer_util.h | 42 +---------------------- 5 files changed, 57 insertions(+), 56 deletions(-) diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc index c4b97757e0..0b98fdbfb1 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.cc +++ b/src/lib/dhcpsrv/mysql_host_data_source.cc @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -1760,6 +1761,15 @@ public: StatementIndex stindex, boost::shared_ptr exchange) const; + /// @brief Throws exception if database is read only. + /// + /// This method should be called by the methods which write to the + /// database. If the backend is operating in read-only mode this + /// method will throw exception. + /// + /// @throw DbReadOnly if backend is operating in read only mode. + void checkReadOnly() const; + /// @brief Pointer to the object representing an exchange which /// can be used to retrieve hosts and DHCPv4 options. boost::shared_ptr host_exchange_; @@ -2202,21 +2212,29 @@ getHost(const SubnetID& subnet_id, return (result); } +void +MySqlHostDataSourceImpl::checkReadOnly() const { + if (is_readonly_) { + isc_throw(ReadOnlyDb, "MySQL host database backend is configured to" + " operate in read only mode"); + } +} + MySqlHostDataSource:: MySqlHostDataSource(const MySqlConnection::ParameterMap& parameters) - : impl_(new MySqlHostDataSourceImpl(parameters), - "MySQL host database backend is configured to" - " operate in read only mode") { - impl_.allowConstOnly(impl_->is_readonly_); + : impl_(new MySqlHostDataSourceImpl(parameters)) { } MySqlHostDataSource::~MySqlHostDataSource() { - delete impl_.getPtr(); + delete impl_; } void MySqlHostDataSource::add(const HostPtr& host) { + // If operating in read-only mode, throw exception. + impl_->checkReadOnly(); + // Initiate MySQL transaction as we will have to make multiple queries // to insert host information into multiple tables. If that fails on // any stage, the transaction will be rolled back by the destructor of @@ -2539,12 +2557,16 @@ std::pair MySqlHostDataSource::getVersion() const { void MySqlHostDataSource::commit() { + // If operating in read-only mode, throw exception. + impl_->checkReadOnly(); impl_->conn_.commit(); } void MySqlHostDataSource::rollback() { + // If operating in read-only mode, throw exception. + impl_->checkReadOnly(); impl_->conn_.rollback(); } diff --git a/src/lib/dhcpsrv/mysql_host_data_source.h b/src/lib/dhcpsrv/mysql_host_data_source.h index 6ee563d83b..d5e56949a0 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.h +++ b/src/lib/dhcpsrv/mysql_host_data_source.h @@ -10,7 +10,6 @@ #include #include #include -#include namespace isc { namespace dhcp { @@ -257,7 +256,7 @@ public: private: /// @brief Pointer to the implementation of the @ref MySqlHostDataSource. - util::RestrictedConstPtr impl_; + MySqlHostDataSourceImpl* impl_; }; } diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.cc b/src/lib/dhcpsrv/pgsql_host_data_source.cc index 2d4e57ba03..5fac1830a6 100644 --- a/src/lib/dhcpsrv/pgsql_host_data_source.cc +++ b/src/lib/dhcpsrv/pgsql_host_data_source.cc @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -1230,6 +1231,14 @@ public: StatementIndex stindex, boost::shared_ptr exchange) const; + /// @brief Throws exception if database is read only. + /// + /// This method should be called by the methods which write to the + /// database. If the backend is operating in read-only mode this + /// method will throw exception. + /// + /// @throw DbReadOnly if backend is operating in read only mode. + void checkReadOnly() const; /// @brief Returns PostgreSQL schema version of the open database /// @@ -1661,23 +1670,32 @@ std::pair PgSqlHostDataSourceImpl::getVersion() const { return (std::make_pair(version, minor)); } +void +PgSqlHostDataSourceImpl::checkReadOnly() const { + if (is_readonly_) { + isc_throw(ReadOnlyDb, "PostgreSQL host database backend is configured" + " to operate in read only mode"); + } +} + + /*********** PgSqlHostDataSource *********************/ PgSqlHostDataSource:: PgSqlHostDataSource(const PgSqlConnection::ParameterMap& parameters) - : impl_(new PgSqlHostDataSourceImpl(parameters), - "PostgreSQL host database backend is configured to" - " operate in read only mode") { - impl_.allowConstOnly(impl_->is_readonly_); + : impl_(new PgSqlHostDataSourceImpl(parameters)) { } PgSqlHostDataSource::~PgSqlHostDataSource() { - delete impl_.getPtr(); + delete impl_; } void PgSqlHostDataSource::add(const HostPtr& host) { + // If operating in read-only mode, throw exception. + impl_->checkReadOnly(); + // Initiate PostgreSQL transaction as we will have to make multiple queries // to insert host information into multiple tables. If that fails on // any stage, the transaction will be rolled back by the destructor of @@ -1928,12 +1946,16 @@ std::pair PgSqlHostDataSource::getVersion() const { void PgSqlHostDataSource::commit() { + // If operating in read-only mode, throw exception. + impl_->checkReadOnly(); impl_->conn_.commit(); } void PgSqlHostDataSource::rollback() { + // If operating in read-only mode, throw exception. + impl_->checkReadOnly(); impl_->conn_.rollback(); } diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.h b/src/lib/dhcpsrv/pgsql_host_data_source.h index ab062efc3c..40ad840c86 100644 --- a/src/lib/dhcpsrv/pgsql_host_data_source.h +++ b/src/lib/dhcpsrv/pgsql_host_data_source.h @@ -8,10 +8,8 @@ #define PGSQL_HOST_DATA_SOURCE_H #include -#include #include #include -#include namespace isc { namespace dhcp { @@ -288,7 +286,7 @@ public: private: /// @brief Pointer to the implementation of the @ref PgSqlHostDataSource. - util::RestrictedConstPtr impl_; + PgSqlHostDataSourceImpl* impl_; }; } diff --git a/src/lib/util/pointer_util.h b/src/lib/util/pointer_util.h index ed8374722f..a775584573 100644 --- a/src/lib/util/pointer_util.h +++ b/src/lib/util/pointer_util.h @@ -1,4 +1,4 @@ -// Copyright (C) 2015-2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015 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 @@ -7,49 +7,9 @@ #ifndef POINTER_UTIL_H #define POINTER_UTIL_H -#include -#include - namespace isc { namespace util { -template -class RestrictedConstPtr { -public: - - RestrictedConstPtr(T* ptr, const std::string& error_text) - : ptr_(ptr), const_only_(false), error_text_(error_text) { - } - - void allowConstOnly(const bool const_only) { - const_only_ = const_only; - } - - T* operator->() const { - return (ptr_); - } - - T* operator->() { - if (const_only_) { - isc_throw(E, error_text_); - } - return (ptr_); - } - - T* getPtr() const { - return (ptr_); - } - -private: - - T* ptr_; - - bool const_only_; - - std::string error_text_; -}; - - /// @brief This function checks if two pointers are non-null and values /// are equal. /// From 4b36e1e7dca1cbe88980660a78bedd66e116f513 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Fri, 29 Jul 2016 09:33:45 +0200 Subject: [PATCH 08/19] [4489] Minor edits in the User Guide. --- doc/guide/dhcp4-srv.xml | 4 ++-- doc/guide/dhcp6-srv.xml | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/guide/dhcp4-srv.xml b/doc/guide/dhcp4-srv.xml index 63a99d818f..ebf5aa6ae8 100644 --- a/doc/guide/dhcp4-srv.xml +++ b/doc/guide/dhcp4-srv.xml @@ -548,12 +548,12 @@ configuration, may not have write privileges to the database. This is often required by the policy within a given network to secure the data from being unintentionally modified. In many cases administrators have inventory databases deployed, which contain substantially more information about the hosts than -static reservations assigned to them. Such database can be used to create +static reservations assigned to them. The inventory database can be used to create a view of a Kea hosts database and such view is often read only. Kea host database backends operate with implicit configuration to both read -and write from the database. If the host database is read only for the +from and write to the database. If the host database is read only for the particular user, the backend will fail to start and consequently the server will refuse to start/reconfigure. If the administrator intends to use the read only host database for retrieving reservations for clients, to assign diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml index a12666e381..0b63c3a42b 100644 --- a/doc/guide/dhcp6-srv.xml +++ b/doc/guide/dhcp6-srv.xml @@ -549,12 +549,12 @@ configuration, may not have write privileges to the database. This is often required by the policy within a given network to secure the data from being unintentionally modified. In many cases administrators have inventory databases deployed, which contain substantially more information about the hosts than -static reservations assigned to them. Such database can be used to create +static reservations assigned to them. The inventory database can be used to create a view of a Kea hosts database and such view is often read only. -Kea host database backends operate with implicit configuration to both read -and write from the database. If the host database is read only for the +Kea host database backends operate with implicit configuration to both read from +and write to the database. If the host database is read only for the particular user, the backend will fail to start and consequently the server will refuse to start/reconfigure. If the administrator intends to use the read only host database for retrieving reservations for clients, to assign From d9a2d3637a3b1378471c30066ffac758f4ed74f7 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Fri, 29 Jul 2016 09:55:40 +0200 Subject: [PATCH 09/19] [4489] Added log statements when host database is read-only. --- src/lib/dhcpsrv/dhcpsrv_messages.mes | 12 ++++++++++++ src/lib/dhcpsrv/mysql_host_data_source.cc | 2 ++ src/lib/dhcpsrv/pgsql_host_data_source.cc | 3 +++ 3 files changed, 17 insertions(+) diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.mes b/src/lib/dhcpsrv/dhcpsrv_messages.mes index 714148548c..7bf0ba22ad 100644 --- a/src/lib/dhcpsrv/dhcpsrv_messages.mes +++ b/src/lib/dhcpsrv/dhcpsrv_messages.mes @@ -551,6 +551,12 @@ connection including database name and username needed to access it A debug message issued when the server is about to obtain schema version information from the MySQL hosts database. +% DHCPSRV_MYSQL_HOST_DB_READONLY MySQL host database operates in read-only mode +This informational message is issued when the user has configured the PostgreSQL +database in read-only mode. Kea will not be able to insert or modify +host reservations but will be able to retrieve existing ones and +assign them to the clients communicating with the server. + % DHCPSRV_MYSQL_ROLLBACK rolling back MySQL database The code has issued a rollback call. All outstanding transaction will be rolled back and not committed to the database. @@ -696,6 +702,12 @@ V6) is about to open a PostgreSQL hosts database. The parameters of the connection including database name and username needed to access it (but not the password if any) are logged. +% DHCPSRV_PGSQL_HOST_DB_READONLY PostgreSQL host database operates in read-only mode +This informational message is issued when the user has configured the PostgreSQL +database in read-only mode. Kea will not be able to insert or modify +host reservations but will be able to retrieve existing ones and +assign them to the clients communicating with the server. + % DHCPSRV_PGSQL_ROLLBACK rolling back PostgreSQL database The code has issued a rollback call. All outstanding transaction will be rolled back and not committed to the database. diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc index 0b98fdbfb1..30d40624c5 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.cc +++ b/src/lib/dhcpsrv/mysql_host_data_source.cc @@ -2013,6 +2013,8 @@ MySqlHostDataSourceImpl(const MySqlConnection::ParameterMap& parameters) conn_.prepareStatements(tagged_statements.begin() + WRITE_STMTS_BEGIN, tagged_statements.end(), tagged_statements.size()); + } else { + LOG_INFO(dhcpsrv_logger, DHCPSRV_MYSQL_HOST_DB_READONLY); } } diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.cc b/src/lib/dhcpsrv/pgsql_host_data_source.cc index 5fac1830a6..d134d3c766 100644 --- a/src/lib/dhcpsrv/pgsql_host_data_source.cc +++ b/src/lib/dhcpsrv/pgsql_host_data_source.cc @@ -1511,6 +1511,9 @@ PgSqlHostDataSourceImpl(const PgSqlConnection::ParameterMap& parameters) if (!is_readonly_) { conn_.prepareStatements(tagged_statements.begin() + WRITE_STMTS_BEGIN, tagged_statements.end()); + + } else { + LOG_INFO(dhcpsrv_logger, DHCPSRV_PGSQL_HOST_DB_READONLY); } } From e6936baabec7460a1d250c88ab2471b60f973548 Mon Sep 17 00:00:00 2001 From: Stephen Morris Date: Fri, 12 Aug 2016 11:57:22 +0100 Subject: [PATCH 10/19] [4489] Altered "size_t" to "unsigned long" to overcome compiler objections --- src/lib/dhcpsrv/mysql_host_data_source.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc index 30d40624c5..34d5f7c9a4 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.cc +++ b/src/lib/dhcpsrv/mysql_host_data_source.cc @@ -1588,7 +1588,7 @@ private: std::vector value_; /// @brief Option value length. - size_t value_len_; + unsigned long value_len_; /// @brief Formatted option value length. unsigned long formatted_value_len_; @@ -1597,7 +1597,7 @@ private: std::string space_; /// @brief Option space name length. - size_t space_len_; + unsigned long space_len_; /// @brief Boolean flag indicating if the option is always returned to /// a client or only when requested. @@ -1607,7 +1607,7 @@ private: std::string client_class_; /// @brief Length of the string holding client classes for the option. - size_t client_class_len_; + unsigned long client_class_len_; /// @brief Subnet identifier. uint32_t subnet_id_; From 134a55c708212a1d55c9b5eb9660da6590c93835 Mon Sep 17 00:00:00 2001 From: Stephen Morris Date: Fri, 12 Aug 2016 12:00:39 +0100 Subject: [PATCH 11/19] [4489] Minor edits to documentation --- doc/guide/admin.xml | 2 +- doc/guide/dhcp4-srv.xml | 20 ++++++++++---------- doc/guide/dhcp6-srv.xml | 22 +++++++++++----------- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/doc/guide/admin.xml b/doc/guide/admin.xml index ccfe991d77..e5decedaf0 100644 --- a/doc/guide/admin.xml +++ b/doc/guide/admin.xml @@ -714,7 +714,7 @@ $ kea-admin lease-upgrade cql -n database-name and describe when - such configuration may be desired and how to configure Kea to + such configuration may be reqired and how to configure Kea to operate using read only host database.
diff --git a/doc/guide/dhcp4-srv.xml b/doc/guide/dhcp4-srv.xml index ebf5aa6ae8..fa2646efee 100644 --- a/doc/guide/dhcp4-srv.xml +++ b/doc/guide/dhcp4-srv.xml @@ -541,10 +541,10 @@ If a timeout is given though, it should be an integer greater than zero.
-Using Read Only Databases for Host Reservations +Using Read-Only Databases for Host Reservations -In some deployments the database user, which name is specified in the database backend -configuration, may not have write privileges to the database. This is often +In some deployments the database user whose name is specified in the database backend +configuration may not have write privileges to the database. This is often required by the policy within a given network to secure the data from being unintentionally modified. In many cases administrators have inventory databases deployed, which contain substantially more information about the hosts than @@ -552,13 +552,13 @@ static reservations assigned to them. The inventory database can be used to crea a view of a Kea hosts database and such view is often read only. -Kea host database backends operate with implicit configuration to both read -from and write to the database. If the host database is read only for the -particular user, the backend will fail to start and consequently the server -will refuse to start/reconfigure. If the administrator intends to use the -read only host database for retrieving reservations for clients, to assign -specific addresses and options, it is possible to explicitly configure -Kea to start in "read-only" mode. This is controlled by the +Kea host database backends operate with an implicit configuration to both +read from and write to the database. If the database user does not have +write access to the host database, the backend will fail to start and the +server will refuse to start (or reconfigure). However, if access to a read +only host database is required for retrieving reservations for clients +and/or assign specific addresses and options, it is possible to explicitly +configure Kea to start in "read-only" mode. This is controlled by the readonly boolean parameter as follows: "Dhcp4": { "hosts-database": { "readonly": true, ... }, ... } diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml index 0b63c3a42b..444b95ebe8 100644 --- a/doc/guide/dhcp6-srv.xml +++ b/doc/guide/dhcp6-srv.xml @@ -542,10 +542,10 @@ If a timeout is given though, it should be an integer greater than zero.
-Using Read Only Databases for Host Reservations +Using Read-Only Databases for Host Reservations -In some deployments the database user, which name is specified in the database backend -configuration, may not have write privileges to the database. This is often +In some deployments the database user whose name is specified in the database backend +configuration may not have write privileges to the database. This is often required by the policy within a given network to secure the data from being unintentionally modified. In many cases administrators have inventory databases deployed, which contain substantially more information about the hosts than @@ -553,16 +553,16 @@ static reservations assigned to them. The inventory database can be used to crea a view of a Kea hosts database and such view is often read only. -Kea host database backends operate with implicit configuration to both read from -and write to the database. If the host database is read only for the -particular user, the backend will fail to start and consequently the server -will refuse to start/reconfigure. If the administrator intends to use the -read only host database for retrieving reservations for clients, to assign -specific addresses and options, it is possible to explicitly configure -Kea to start in "read-only" mode. This is controlled by the +Kea host database backends operate with an implicit configuration to both +read from and write to the database. If the database user does not have +write access to the host database, the backend will fail to start and the +server will refuse to start (or reconfigure). However, if access to a read +only host database is required for retrieving reservations for clients +and/or assign specific addresses and options, it is possible to explicitly +configure Kea to start in "read-only" mode. This is controlled by the readonly boolean parameter as follows: -"Dhcp4": { "hosts-database": { "readonly": true, ... }, ... } +"Dhcp6": { "hosts-database": { "readonly": true, ... }, ... } Setting this parameter to false would configure the database backend to operate in "read-write" mode, which is also a default From e7c4240dd822e5c2d4e98d1c904e3cc475bd0e53 Mon Sep 17 00:00:00 2001 From: Stephen Morris Date: Fri, 12 Aug 2016 17:41:24 +0100 Subject: [PATCH 12/19] [4489] Correct error in the declaration of MySqlHostIpv6Exchange::iaid_ --- src/lib/dhcpsrv/mysql_host_data_source.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc index 34d5f7c9a4..96ef41f9f0 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.cc +++ b/src/lib/dhcpsrv/mysql_host_data_source.cc @@ -1275,7 +1275,7 @@ private: uint8_t prefix_len_; /// @brief IAID. - uint8_t iaid_; + uint32_t iaid_; /// @name Indexes of columns holding information about IPv6 reservations. //@{ From 4a8efcb45a81052889ae1a7258bec5d49db0bc47 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Wed, 17 Aug 2016 11:22:24 +0200 Subject: [PATCH 13/19] [4489] Addressed review comments. The only review item not addressed with this commit is the implementation of unit test that operates on the read only database, i.e. the database containing tables on which the given user only has SELECT privileges. --- src/lib/dhcpsrv/database_connection.cc | 22 +++++++++- src/lib/dhcpsrv/database_connection.h | 8 ++++ src/lib/dhcpsrv/dhcpsrv_messages.mes | 4 +- src/lib/dhcpsrv/mysql_connection.cc | 13 +++--- src/lib/dhcpsrv/mysql_connection.h | 4 +- src/lib/dhcpsrv/mysql_host_data_source.cc | 40 +++++++------------ src/lib/dhcpsrv/mysql_lease_mgr.cc | 3 +- src/lib/dhcpsrv/pgsql_host_data_source.cc | 24 ++++------- .../scripts/mysql/dhcpdb_create.mysql | 5 +++ 9 files changed, 67 insertions(+), 56 deletions(-) diff --git a/src/lib/dhcpsrv/database_connection.cc b/src/lib/dhcpsrv/database_connection.cc index 983a752d96..0aa886fa88 100644 --- a/src/lib/dhcpsrv/database_connection.cc +++ b/src/lib/dhcpsrv/database_connection.cc @@ -1,10 +1,11 @@ -// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2016 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 // file, You can obtain one at http://mozilla.org/MPL/2.0/. #include +#include #include #include @@ -86,5 +87,24 @@ DatabaseConnection::redactedAccessString(const ParameterMap& parameters) { return (access); } +bool +DatabaseConnection::configuredReadOnly() const { + std::string readonly_value = "false"; + try { + readonly_value = getParameter("readonly"); + boost::algorithm::to_lower(readonly_value); + } catch (...) { + // Parameter "readonly" hasn't been specified so we simply use + // the default value of "false". + } + + if ((readonly_value != "false") && (readonly_value != "true")) { + isc_throw(DbInvalidReadOnly, "invalid value '" << readonly_value + << "' specified for boolean parameter 'readonly'"); + } + + return (readonly_value == "true"); +} + }; }; diff --git a/src/lib/dhcpsrv/database_connection.h b/src/lib/dhcpsrv/database_connection.h index c18c16f28c..e6a902babf 100644 --- a/src/lib/dhcpsrv/database_connection.h +++ b/src/lib/dhcpsrv/database_connection.h @@ -121,6 +121,14 @@ public: /// @return Redacted database access string. static std::string redactedAccessString(const ParameterMap& parameters); + /// @brief Convenience method checking if database should be opened with + /// read only access. + /// + /// @return true if "readonly" parameter is specified and set to true; + /// false if "readonly" parameter is not specified or it is specified + /// and set to false. + bool configuredReadOnly() const; + private: /// @brief List of parameters passed in dbconfig diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.mes b/src/lib/dhcpsrv/dhcpsrv_messages.mes index 7bf0ba22ad..bb8abaee1f 100644 --- a/src/lib/dhcpsrv/dhcpsrv_messages.mes +++ b/src/lib/dhcpsrv/dhcpsrv_messages.mes @@ -551,7 +551,7 @@ connection including database name and username needed to access it A debug message issued when the server is about to obtain schema version information from the MySQL hosts database. -% DHCPSRV_MYSQL_HOST_DB_READONLY MySQL host database operates in read-only mode +% DHCPSRV_MYSQL_HOST_DB_READONLY MySQL host database opened for read access only This informational message is issued when the user has configured the PostgreSQL database in read-only mode. Kea will not be able to insert or modify host reservations but will be able to retrieve existing ones and @@ -702,7 +702,7 @@ V6) is about to open a PostgreSQL hosts database. The parameters of the connection including database name and username needed to access it (but not the password if any) are logged. -% DHCPSRV_PGSQL_HOST_DB_READONLY PostgreSQL host database operates in read-only mode +% DHCPSRV_PGSQL_HOST_DB_READONLY PostgreSQL host database opened for read access only This informational message is issued when the user has configured the PostgreSQL database in read-only mode. Kea will not be able to insert or modify host reservations but will be able to retrieve existing ones and diff --git a/src/lib/dhcpsrv/mysql_connection.cc b/src/lib/dhcpsrv/mysql_connection.cc index b12bdd5e19..9569b4e8d3 100644 --- a/src/lib/dhcpsrv/mysql_connection.cc +++ b/src/lib/dhcpsrv/mysql_connection.cc @@ -214,12 +214,15 @@ MySqlConnection::prepareStatement(uint32_t index, const char* text) { void MySqlConnection::prepareStatements(const TaggedStatement* start_statement, - const TaggedStatement* end_statement, - size_t num_statements) { - // Allocate space for all statements - statements_.resize(num_statements, NULL); + const TaggedStatement* end_statement) { + size_t num_statements = std::distance(start_statement, end_statement); + if (num_statements == 0) { + return; + } - text_statements_.resize(num_statements, std::string("")); + // Expand vectors of allocated statements to hold additional statements. + statements_.resize(statements_.size() + num_statements, NULL); + text_statements_.resize(text_statements_.size() + num_statements, std::string("")); // Created the MySQL prepared statements for each DML statement. for (const TaggedStatement* tagged_statement = start_statement; diff --git a/src/lib/dhcpsrv/mysql_connection.h b/src/lib/dhcpsrv/mysql_connection.h index 06fb34d466..89ad0ec593 100644 --- a/src/lib/dhcpsrv/mysql_connection.h +++ b/src/lib/dhcpsrv/mysql_connection.h @@ -241,15 +241,13 @@ public: /// Creates the prepared statements for all of the SQL statements used /// by the MySQL backend. /// @param tagged_statements an array of statements to be compiled - /// @param num_statements number of statements in tagged_statements /// /// @throw isc::dhcp::DbOperationError An operation on the open database has /// failed. /// @throw isc::InvalidParameter 'index' is not valid for the vector. This /// represents an internal error within the code. void prepareStatements(const TaggedStatement* start_statement, - const TaggedStatement* end_statement, - size_t num_statements); + const TaggedStatement* end_statement); /// @brief Clears prepared statements and text statements. void clearStatements(); diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc index 96ef41f9f0..becad67abe 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.cc +++ b/src/lib/dhcpsrv/mysql_host_data_source.cc @@ -849,7 +849,7 @@ private: size_t start_column_; /// @brief Option id. - uint64_t option_id_; + uint32_t option_id_; /// @brief Option code. uint16_t code_; @@ -919,7 +919,7 @@ private: //@} /// @brief Option id for last processed row. - uint64_t most_recent_option_id_; + uint32_t most_recent_option_id_; }; /// @brief Pointer to the @ref OptionProcessor class. @@ -1116,7 +1116,7 @@ public: /// @brief Returns last fetched reservation id. /// /// @return Reservation id or 0 if no reservation data is fetched. - uint64_t getReservationId() const { + uint32_t getReservationId() const { if (reserv_type_null_ == MLM_FALSE) { return (reservation_id_); } @@ -1254,7 +1254,7 @@ public: private: /// @brief IPv6 reservation id. - uint64_t reservation_id_; + uint32_t reservation_id_; /// @brief IPv6 reservation type. uint8_t reserv_type_; @@ -1297,7 +1297,7 @@ private: //@} /// @brief Reservation id for last processed row. - uint64_t most_recent_reservation_id_; + uint32_t most_recent_reservation_id_; }; @@ -1633,7 +1633,10 @@ public: /// @brief Statement Tags /// - /// The contents of the enum are indexes into the list of SQL statements + /// The contents of the enum are indexes into the list of SQL statements. + /// It is assumed that the order is such that the indicies of statements + /// reading the database are less than those of statements modifying the + /// database. enum StatementIndex { GET_HOST_DHCPID, // Gets hosts by host identifier GET_HOST_ADDR, // Gets hosts by IPv4 address @@ -1986,33 +1989,18 @@ MySqlHostDataSourceImpl(const MySqlConnection::ParameterMap& parameters) // information from the database, so they can be used even if the // database is read only for the current user. conn_.prepareStatements(tagged_statements.begin(), - tagged_statements.begin() + WRITE_STMTS_BEGIN, - WRITE_STMTS_BEGIN); + tagged_statements.begin() + WRITE_STMTS_BEGIN); - std::string readonly_value = "false"; - try { - readonly_value = conn_.getParameter("readonly"); - boost::algorithm::to_lower(readonly_value); - } catch (...) { - // Parameter "readonly" hasn't been specified so we simply use - // the default value of "false". - } - - if (readonly_value == "true") { - is_readonly_ = true; - - } else if (readonly_value != "false") { - isc_throw(DbInvalidReadOnly, "invalid value '" << readonly_value - << "' specified for boolean parameter 'readonly'"); - } + // Check if the backend is explicitly configured to operate with + // read only access to the database. + is_readonly_ = conn_.configuredReadOnly(); // If we are using read-write mode for the database we also prepare // statements for INSERTS etc. if (!is_readonly_) { // Prepare statements for writing to the database, e.g. INSERT. conn_.prepareStatements(tagged_statements.begin() + WRITE_STMTS_BEGIN, - tagged_statements.end(), - tagged_statements.size()); + tagged_statements.end()); } else { LOG_INFO(dhcpsrv_logger, DHCPSRV_MYSQL_HOST_DB_READONLY); } diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index 7e448428f2..8edf4dfc5c 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -1237,8 +1237,7 @@ MySqlLeaseMgr::MySqlLeaseMgr(const MySqlConnection::ParameterMap& parameters) } // Prepare all statements likely to be used. - conn_.prepareStatements(tagged_statements.begin(), tagged_statements.end(), - tagged_statements.size()); + conn_.prepareStatements(tagged_statements.begin(), tagged_statements.end()); // Create the exchange objects for use in exchanging data between the // program and the database. diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.cc b/src/lib/dhcpsrv/pgsql_host_data_source.cc index d134d3c766..2ab5280923 100644 --- a/src/lib/dhcpsrv/pgsql_host_data_source.cc +++ b/src/lib/dhcpsrv/pgsql_host_data_source.cc @@ -1105,7 +1105,10 @@ public: /// @brief Statement Tags /// - /// The contents of the enum are indexes into the list of SQL statements + /// The contents of the enum are indexes into the list of SQL statements. + /// It is assumed that the order is such that the indicies of statements + /// reading the database are less than those of statements modifying the + /// database. enum StatementIndex { GET_HOST_DHCPID, // Gets hosts by host identifier GET_HOST_ADDR, // Gets hosts by IPv4 address @@ -1489,22 +1492,9 @@ PgSqlHostDataSourceImpl(const PgSqlConnection::ParameterMap& parameters) conn_.prepareStatements(tagged_statements.begin(), tagged_statements.begin() + WRITE_STMTS_BEGIN); - std::string readonly_value = "false"; - try { - readonly_value = conn_.getParameter("readonly"); - boost::algorithm::to_lower(readonly_value); - } catch (...) { - // Parameter "readonly" hasn't been specified so we simply use - // the default value of "false". - } - - if (readonly_value == "true") { - is_readonly_ = true; - - } else if (readonly_value != "false") { - isc_throw(DbInvalidReadOnly, "invalid value '" << readonly_value - << "' specified for boolean parameter 'readonly'"); - } + // Check if the backend is explicitly configured to operate with + // read only access to the database. + is_readonly_ = conn_.configuredReadOnly(); // If we are using read-write mode for the database we also prepare // statements for INSERTS etc. diff --git a/src/share/database/scripts/mysql/dhcpdb_create.mysql b/src/share/database/scripts/mysql/dhcpdb_create.mysql index 5d6a321200..80aaf082c2 100644 --- a/src/share/database/scripts/mysql/dhcpdb_create.mysql +++ b/src/share/database/scripts/mysql/dhcpdb_create.mysql @@ -465,6 +465,11 @@ ALTER TABLE dhcp6_options ADD CONSTRAINT fk_dhcp6_option_scope FOREIGN KEY (scope_id) REFERENCES dhcp_option_scope (scope_id); +# Add UNSIGNED to reservation_id +ALTER TABLE ipv6_reservations + MODIFY reservation_id INT UNSIGNED NOT NULL AUTO_INCREMENT; + + # Update the schema version number UPDATE schema_version SET version = '4', minor = '2'; From 540f6ebe8c343495d54ba7afa03fe8db3662cd5c Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Wed, 17 Aug 2016 13:16:48 +0200 Subject: [PATCH 14/19] [4489] Using keatest_readonly db user for read only mode tests. --- src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc | 2 +- src/lib/dhcpsrv/testutils/schema.cc | 1 + src/lib/dhcpsrv/testutils/schema.h | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc index fe4560a40b..0eb683ef81 100644 --- a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc @@ -534,7 +534,7 @@ GenericHostDataSourceTest::testReadOnlyDatabase(const char* valid_db_type) { HostDataSourceFactory::create(connectionString(valid_db_type, VALID_NAME, VALID_HOST, - VALID_USER, + VALID_READONLY_USER, VALID_PASSWORD, VALID_READONLY_DB)); diff --git a/src/lib/dhcpsrv/testutils/schema.cc b/src/lib/dhcpsrv/testutils/schema.cc index f782f27f34..b373363dc7 100644 --- a/src/lib/dhcpsrv/testutils/schema.cc +++ b/src/lib/dhcpsrv/testutils/schema.cc @@ -25,6 +25,7 @@ const char* INVALID_NAME = "name=invalidname"; const char* VALID_HOST = "host=localhost"; const char* INVALID_HOST = "host=invalidhost"; const char* VALID_USER = "user=keatest"; +const char* VALID_READONLY_USER = "user=keatest_readonly"; const char* INVALID_USER = "user=invaliduser"; const char* VALID_PASSWORD = "password=keatest"; const char* INVALID_PASSWORD = "password=invalid"; diff --git a/src/lib/dhcpsrv/testutils/schema.h b/src/lib/dhcpsrv/testutils/schema.h index 97c4627f1b..14d04abb6e 100644 --- a/src/lib/dhcpsrv/testutils/schema.h +++ b/src/lib/dhcpsrv/testutils/schema.h @@ -21,6 +21,7 @@ extern const char* INVALID_NAME; extern const char* VALID_HOST; extern const char* INVALID_HOST; extern const char* VALID_USER; +extern const char* VALID_READONLY_USER; extern const char* INVALID_USER; extern const char* VALID_PASSWORD; extern const char* INVALID_PASSWORD; From 750fa1da22069a1899e8b3263600a7ab3314e071 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 18 Aug 2016 10:30:45 +0200 Subject: [PATCH 15/19] [4489] Updated dev-guide with database prerequisites for unit tests. --- doc/Makefile.am | 2 +- doc/devel/contribute.dox | 2 +- doc/devel/mainpage.dox | 11 +- doc/devel/qa.dox | 68 ------- doc/devel/unit-tests.dox | 277 ++++++++++++++++++++++++++ src/lib/dhcpsrv/database_backends.dox | 132 ------------ 6 files changed, 286 insertions(+), 206 deletions(-) delete mode 100644 doc/devel/qa.dox create mode 100644 doc/devel/unit-tests.dox diff --git a/doc/Makefile.am b/doc/Makefile.am index 69401e6749..1976fabeec 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -4,7 +4,7 @@ EXTRA_DIST = version.ent.in Doxyfile Doxyfile-xml EXTRA_DIST += devel/config-backend.dox EXTRA_DIST += devel/contribute.dox EXTRA_DIST += devel/mainpage.dox -EXTRA_DIST += devel/qa.dox +EXTRA_DIST += devel/unit-tests.dox nobase_dist_doc_DATA = examples/ddns/sample1.json nobase_dist_doc_DATA += examples/ddns/template.json diff --git a/doc/devel/contribute.dox b/doc/devel/contribute.dox index 31028decaa..3f43307887 100644 --- a/doc/devel/contribute.dox +++ b/doc/devel/contribute.dox @@ -94,7 +94,7 @@ written and observing the test fail, you can detect the situation where a bug in the test code will cause it to pass regardless of the code being tested. -See @ref qaUnitTests for instructions on how to run unit-tests. +See @ref unitTests for instructions on how to run unit-tests. If you happen to add new files or have modified any \c Makefile.am files, it is also a good idea to check if you haven't broken the diff --git a/doc/devel/mainpage.dox b/doc/devel/mainpage.dox index ebe1623d94..7b203dd464 100644 --- a/doc/devel/mainpage.dox +++ b/doc/devel/mainpage.dox @@ -1,4 +1,4 @@ -// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2016 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 @@ -34,6 +34,12 @@ * @section contrib Contributor's Guide * - @subpage contributorGuide * + * @section buildingKeaWithUnitTests Building Kea with Unit tests + * - @subpage unitTests + * - @subpage unitTestsIntroduction + * - @subpage unitTestsEnvironmentVariables + * - @subpage unitTestsDatabaseConfig + * * @section hooksFramework Hooks Framework * - @subpage hooksdgDevelopersGuide * - @subpage dhcpv4Hooks @@ -107,9 +113,6 @@ * - @subpage configBackendAdding * - @subpage perfdhcpInternals * - * @section qualityAssurance Quality Assurance - * - @subpage qaUnitTests - * * @section miscellaneousTopics Miscellaneous Topics * - @subpage logKeaLogging * - @subpage logBasicIdeas diff --git a/doc/devel/qa.dox b/doc/devel/qa.dox deleted file mode 100644 index 8ad8c88840..0000000000 --- a/doc/devel/qa.dox +++ /dev/null @@ -1,68 +0,0 @@ -// Copyright (C) 2015 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 -// file, You can obtain one at http://mozilla.org/MPL/2.0/. - -/** - -@page qa Kea Quality Assurance processes - - @section qaUnitTests Unit-tests - -Kea uses the Google C++ Testing Framework (also called googletest or gtest) as a -base for our C++ unit-tests. See http://code.google.com/p/googletest/ for -details. We used to have Python unit-tests that were inherited from BIND10 -days. Those tests are removed now, so please do not develop any new Python -tests in Kea. If you want to write DHCP tests in Python, we encourage you to -take a look at ISC Forge: http://kea.isc.org/wiki/IscForge. You must have \c -gtest installed or at least extracted in a directory before compiling Kea -unit-tests. To enable unit-tests in Kea, use: - -@code -./configure --with-gtest=/path/to/your/gtest/dir -@endcode - -or - -@code -./configure --with-gtest-source=/path/to/your/gtest/dir -@endcode - -Depending on how you compiled or installed \c gtest (e.g. from sources -or using some package management system) one of those two switches will -find \c gtest. After that you make run unit-tests: - -@code -make check - -@endcode - -The following environment variable can affect unit-tests: - -- KEA_LOCKFILE_DIR - Specifies a directory where the logging system should - create its lock file. If not specified, it is prefix/var/run/kea, where prefix - defaults to /usr/local. This variable must not end with a slash. There is one - special value: "none", which instructs Kea to not create lock file at - all. This may cause issues if several processes log to the same file. - Also see Kea User's Guide, section 15.3. - -- KEA_LOGGER_DESTINATION - Specifies logging destination. If not set, logged - messages will not be recorded anywhere. There are 3 special values: - stdout, stderr and syslog. Any other value is interpreted as a filename. - Also see Kea User's Guide, section 15.3. - -- KEA_PIDFILE_DIR - Specifies the directory which should be used for PID files - as used by dhcp::Daemon or its derivatives. If not specified, the default is - prefix/var/run/kea, where prefix defaults to /usr/local. This variable must - not end with a slash. - -- KEA_SOCKET_TEST_DIR - if set, it specifies the directory where Unix - sockets are created. There's OS limitation on how long a Unix socket - path can be. It is typcially slightly over 100 characters. If you - happen to build and run unit-tests in deeply nested directories, this - may become a problem. KEA_SOCKET_TEST_DIR can be specified to instruct - unit-test to use a different directory. Must not end with slash (e.g. - /tmp). - - */ diff --git a/doc/devel/unit-tests.dox b/doc/devel/unit-tests.dox new file mode 100644 index 0000000000..7801e1448f --- /dev/null +++ b/doc/devel/unit-tests.dox @@ -0,0 +1,277 @@ +// Copyright (C) 2015-2016 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 +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +/** + + @page unitTests Building Kea with Unit Tests + +@section unitTestsIntroduction Introduction + +Kea uses the Google C++ Testing Framework (also called googletest or gtest) as a +base for our C++ unit-tests. See http://code.google.com/p/googletest/ for +details. We used to have Python unit-tests that were inherited from BIND10 +days. Those tests are removed now, so please do not develop any new Python +tests in Kea. If you want to write DHCP tests in Python, we encourage you to +take a look at ISC Forge: http://kea.isc.org/wiki/IscForge. You must have \c +gtest installed or at least extracted in a directory before compiling Kea +unit-tests. To enable unit-tests in Kea, use: + +@code +./configure --with-gtest=/path/to/your/gtest/dir +@endcode + +or + +@code +./configure --with-gtest-source=/path/to/your/gtest/dir +@endcode + +Depending on how you compiled or installed \c gtest (e.g. from sources +or using some package management system) one of those two switches will +find \c gtest. After that you make run unit-tests: + +@code +make check + +@endcode + +@section unitTestsEnvironmentVariables Environment Variables + +The following environment variable can affect unit-tests: + +- KEA_LOCKFILE_DIR - Specifies a directory where the logging system should + create its lock file. If not specified, it is prefix/var/run/kea, where prefix + defaults to /usr/local. This variable must not end with a slash. There is one + special value: "none", which instructs Kea to not create lock file at + all. This may cause issues if several processes log to the same file. + Also see Kea User's Guide, section 15.3. + +- KEA_LOGGER_DESTINATION - Specifies logging destination. If not set, logged + messages will not be recorded anywhere. There are 3 special values: + stdout, stderr and syslog. Any other value is interpreted as a filename. + Also see Kea User's Guide, section 15.3. + +- KEA_PIDFILE_DIR - Specifies the directory which should be used for PID files + as used by dhcp::Daemon or its derivatives. If not specified, the default is + prefix/var/run/kea, where prefix defaults to /usr/local. This variable must + not end with a slash. + +- KEA_SOCKET_TEST_DIR - if set, it specifies the directory where Unix + sockets are created. There's OS limitation on how long a Unix socket + path can be. It is typcially slightly over 100 characters. If you + happen to build and run unit-tests in deeply nested directories, this + may become a problem. KEA_SOCKET_TEST_DIR can be specified to instruct + unit-test to use a different directory. Must not end with slash (e.g. + /tmp). + +@section unitTestsDatabaseConfig Databases Configuration for Unit Tests + + With the use of databases requiring separate authorisation, there are + certain database-specific pre-requisites for successfully running the unit + tests. These are listed in the following sections. + + @subsection unitTestsDatabaseUsers Database Users Required for Unit Tests + + Unit tests validating database backends require that keatest database + is created. This database should be empty (should not include any relations). + Unit tests will create required tables for each test case, and drop these tables + when the test case ends. The unit tests also require that keatest user + is created and that this user is configured to access keatest + database with a keatest password. + + Unit tests use these credentials to create database schema, run test cases + and drop the schema. Thus, the keatest user must have sufficiently + high privileges to create and drop tables, as well as insert and modify the + data within those tables. + + The database backends, which support read only access to the host reservations + databases (currently MySQL and PostgreSQL), include unit tests verifying that + a database user, with read-only privileges, can be used to retrieve host + reservations. Those tests require that a user keatest_readonly, with + SQL SELECT privilege to the keatest database (without INSERT, UPDATE etc.), + is also created. + + The following sections provide step-by-step guidelines how to setup the + databases for running unit tests. + + @subsection mysqlUnitTestsPrerequisites MySQL Database + + A database called keatest must be created. A database user, also called + keatest (and with a password keatest) must also be created and + be given full privileges in that database. The unit tests create the schema + in the database before each test and delete it afterwards. + + In detail, the steps to create the database and user are: + + -# Log into MySQL as root: + @verbatim + % mysql -u root -p + Enter password: + : + mysql>@endverbatim\n + -# Create the test database. This must be called "keatest": + @verbatim + mysql> CREATE DATABASE keatest; + mysql>@endverbatim\n + -# Create the users under which the test client will connect to the database + (the apostrophes around the words keatest and localhost are + required): + @verbatim + mysql> CREATE USER 'keatest'@'localhost' IDENTIFIED BY 'keatest'; + mysql> CREATE USER 'keatest_readonly'@'localhost' IDENTIFIED BY 'keatest'; + mysql>@endverbatim\n + -# Grant the created users permissions to access the keatest database + (again, the apostrophes around the user names and localhost + are required): + @verbatim + mysql> GRANT ALL ON keatest.* TO 'keatest'@'localhost'; + mysql> GRANT SELECT ON keatest.* TO 'keatest_readonly'@'localhost'; + mysql>@endverbatim\n + -# Exit MySQL: + @verbatim + mysql> quit + Bye + %@endverbatim + + The unit tests are run automatically when "make check" is executed (providing + that Kea has been build with the \--with-dhcp-mysql switch (see the installation + section in the Kea Administrator + Reference Manual). + + @subsection pgsqlUnitTestsPrerequisites PostgreSQL Database + + Conceptually, the steps required to run PostgreSQL unit-tests are the same as + in MySQL. First, a database called keatest must be created. A database + user, also called keatest (that will be allowed to log in using password + keatest) must be created and given full privileges in that database. A + database user, called keatest_readonly (using password keatest) + must be created with SELECT privilege on all tables. + The unit tests create the schema in the database before each test and delete it + afterwards. + + PostgreSQL set up differs from system to system. Please consult your OS-specific + PostgreSQL documentation. The remainder of that section uses Ubuntu 13.10 x64 + (with PostgreSQL 9.0+) as an example. On Ubuntu, after installing PostgreSQL + (with sudo apt-get install postgresql), it is installed as user + postgres. To create new databases or add new users, initial commands + must be issued as user postgres: + +@verbatim +$ sudo -u postgres psql postgres +[sudo] password for thomson: +psql (9.1.12) +Type "help" for help. +postgres=# CREATE USER keatest WITH PASSWORD 'keatest'; +CREATE ROLE +postgres=# CREATE DATABASE keatest; +CREATE DATABASE +postgres=# GRANT ALL PRIVILEGES ON DATABASE keatest TO keatest; +GRANT +postgres=# \q +@endverbatim + + PostgreSQL versions earlier than 9.0 don't provide an SQL statement for granting + privileges on all tables in a database. In newer PostgreSQL versions, it is + possible to grant specific privileges on all tables within a schema. + However, this only affects tables which exist when the privileges are granted. + To ensure that the user has specific privileges to tables dynamically created + by the unit tests, the default schema privileges must be altered. + + The following example demonstrates how to create user keatest_readonly, + which has SELECT privilege to the tables within the keatest database, + in Postgres 9.0+. For earlier versions of Postgres, it is recommended to + simply grant full privileges to keatest_readonly user, using the + same steps as for the keatest user. + +@verbatim +$ psql -U postgres +Password for user postgres: +psql (9.1.12) +Type "help" for help. + +postgres=# CREATE USER keatest_readonly WITH PASSWORD 'keatest'; +CREATE ROLE +postgres=# \q + +$ psql -U keatest +Password for user keatest: +psql (9.1.12) +Type "help" for help. + +keatest=> ALTER DEFAULT PRIVILEGES IN SCHEMA public GRANT ALL ON TABLES to keatest_readonly; +ALTER DEFAULT PRIVILEGES +keatest=> \q +@endverbatim + + Note that keatest user (rather than postgres) is used to grant + privileges to the keatest_readonly user. This ensures that the SELECT + privilege is granted only on the tables that the keatest user can access + within the public schema. + + Now we should be able to log into the newly created database using both user + names: +@verbatim +$ psql -d keatest -U keatest +Password for user keatest: +psql (9.1.12) +Type "help" for help. + +keatest=> \q + +$ psql -d keatest -U keatest_readonly +Password for user keatest_readonly: +psql (9.1.12) +Type "help" for help. + +keatest=> +@endverbatim + + If instead of seeing keatest=> prompt, your login will be refused with error + code about failed peer or indent authentication, it means that PostgreSQL is + configured to check unix username and reject login attepts if PostgreSQL names + are different. To alter that, PostgreSQL configuration must be changed. + /etc/postgresql/9.1/main/pg_hba.conf config file + has to be tweaked. It may be in a different location in your system. The following + lines: + +@verbatim +local all all peer +host all all 127.0.0.1/32 md5 +host all all ::1/128 md5 +@endverbatim + + were replaced with: + +@verbatim +local all all password +host all all 127.0.0.1/32 password +host all all ::1/128 password +@endverbatim + + Another possible problem is to get no password prompt, in general because + you have no pg_hba.conf config file and everybody is by default + trusted. As it has a very bad effect on the security you should have + been warned it is a highly unsafe config. The solution is the same, + i.e., require password or md5 authentication method. If you lose + the postgres user access you can add first: +@verbatim +local all postgres trust +@endverbatim + to trust only the local postgres user. Note the postgres user can + be pgsql on some systems. + + Please consult your PostgreSQL user manual before applying those changes as + those changes may expose your other databases that you run on the same system. + In general case, it is a poor idea to run anything of value on a system + that runs tests. Use caution! + + The unit tests are run automatically when "make check" is executed (providing + that Kea has been build with the \--with-dhcp-pgsql switch (see the installation + section in the Kea Administrator + Reference Manual). + + + */ diff --git a/src/lib/dhcpsrv/database_backends.dox b/src/lib/dhcpsrv/database_backends.dox index 104318ece2..682bc5dea0 100644 --- a/src/lib/dhcpsrv/database_backends.dox +++ b/src/lib/dhcpsrv/database_backends.dox @@ -83,137 +83,5 @@ - user - database user ID under which the database is accessed. If not specified, no user ID is used - the database is assumed to be open. - @section dhcp-backend-unittest Running Unit Tests - - With the use of databases requiring separate authorisation, there are - certain database-specific pre-requisites for successfully running the unit - tests. These are listed in the following sections. - - @subsection dhcp-mysql-unittest MySQL Unit Tests - - A database called keatest must be created. A database user, also called - keatest (and with a password keatest) must also be created and - be given full privileges in that database. The unit tests create the schema - in the database before each test and delete it afterwards. - - In detail, the steps to create the database and user are: - - -# Log into MySQL as root: - @verbatim - % mysql -u root -p - Enter password: - : - mysql>@endverbatim\n - -# Create the test database. This must be called "keatest": - @verbatim - mysql> CREATE DATABASE keatest; - mysql>@endverbatim\n - -# Create the user under which the test client will connect to the database - (the apostrophes around the words keatest and localhost are - required): - @verbatim - mysql> CREATE USER 'keatest'@'localhost' IDENTIFIED BY 'keatest'; - mysql>@endverbatim\n - -# Grant the created user permissions to access the keatest database - (again, the apostrophes around the words keatest and localhost - are required): - @verbatim - mysql> GRANT ALL ON keatest.* TO 'keatest'@'localhost'; - mysql>@endverbatim\n - -# Exit MySQL: - @verbatim - mysql> quit - Bye - %@endverbatim - - The unit tests are run automatically when "make check" is executed (providing - that Kea has been build with the \--with-dhcp-mysql switch (see the installation - section in the Kea Administrator - Reference Manual). - - @subsection dhcp-pgsql-unittest PostgreSQL Unit Tests - - Conceptually, the steps required to run PostgreSQL unit-tests are the same as - in MySQL. First, a database called keatest must be created. A database - user, also called keatest (that will be allowed to log in using password - keatest) must be created and given full privileges in that database. The - unit tests create the schema in the database before each test and delete it - afterwards. - - PostgreSQL set up differs from system to system. Please consult your OS-specific - PostgreSQL documentation. The remainder of that section uses Ubuntu 13.10 x64 as - example. On Ubuntu, after installing PostgreSQL (with sudo apt-get install - postgresql), it is installed as user postgres. To create new databases - or add new users, initial commands must be issued as user postgres: - -@verbatim -$ sudo -u postgres psql postgres -[sudo] password for thomson: -psql (9.1.12) -Type "help" for help. -postgres=# CREATE USER keatest WITH PASSWORD 'keatest'; -CREATE ROLE -postgres=# CREATE DATABASE keatest; -CREATE DATABASE -postgres=# GRANT ALL PRIVILEGES ON DATABASE keatest TO keatest; -GRANT -postgres=# \q -@endverbatim - - Now we are back to our regular, unprivileged user. Try to log into the newly - created database using keatest credentials: -@verbatim -$ psql -d keatest -U keatest -Password for user keatest: -psql (9.1.12) -Type "help" for help. - -keatest=> -@endverbatim - - If instead of seeing keatest=> prompt, your login will be refused with error - code about failed peer or indent authentication, it means that PostgreSQL is - configured to check unix username and reject login attepts if PostgreSQL names - are different. To alter that, PostgreSQL configuration must be changed. - Alternatively, you may set up your environment, so the tests would be run from - unix account keatest. /etc/postgresql/9.1/main/pg_hba.conf config file - had to betweaked. It may be in a different location in your system. The following - lines: - -@verbatim -local all all peer -host all all 127.0.0.1/32 md5 -host all all ::1/128 md5 -@endverbatim - - were replaced with: - -@verbatim -local all all password -host all all 127.0.0.1/32 password -host all all ::1/128 password -@endverbatim - - Another possible problem is to get no password prompt, in general because - you have no pg_hba.conf config file and everybody is by default - trusted. As it has a very bad effect on the security you should have - been warned it is a highly unsafe config. The solution is the same, - i.e., require password or md5 authentication method. If you lose - the postgres user access you can add first: -@verbatim -local all postgres trust -@endverbatim - to trust only the local postgres user. Note the postgres user can - be pgsql on some systems. - - Please consult your PostgreSQL user manual before applying those changes as - those changes may expose your other databases that you run on the same system. - In general case, it is a poor idea to run anything of value on a system - that runs tests. Use caution! - - The unit tests are run automatically when "make check" is executed (providing - that Kea has been build with the \--with-dhcp-pgsql switch (see the installation - section in the Kea Administrator - Reference Manual). */ From ac5f18e235ad0e24b69698c077a35e98c2f09acb Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 18 Aug 2016 10:54:23 +0200 Subject: [PATCH 16/19] [4489] Pointing to the Kea Deveoper's Guide for unit testing. --- doc/guide/install.xml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/doc/guide/install.xml b/doc/guide/install.xml index 41793080c8..0f4ad19e9a 100644 --- a/doc/guide/install.xml +++ b/doc/guide/install.xml @@ -459,6 +459,17 @@ Debian and Ubuntu: Kea is built. This section covers the building of Kea with MySQL and/or PostgreSQL and the creation of the lease database. + + + + When unit tests are built with Kea (--with-gtest configuration option is specified), + the databases must be manually pre-configured for the unit tests to run. + The details of this configuration can be found in the + Kea Developer's + Guide. + + +
Building with MySQL Support From 83283d9c20c9fba8fc12067a35f43aae667d2cbb Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 18 Aug 2016 11:26:44 +0200 Subject: [PATCH 17/19] [4489] Trivial correction in logging message description. --- src/lib/dhcpsrv/dhcpsrv_messages.mes | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.mes b/src/lib/dhcpsrv/dhcpsrv_messages.mes index bb8abaee1f..e9ff369859 100644 --- a/src/lib/dhcpsrv/dhcpsrv_messages.mes +++ b/src/lib/dhcpsrv/dhcpsrv_messages.mes @@ -552,7 +552,7 @@ A debug message issued when the server is about to obtain schema version information from the MySQL hosts database. % DHCPSRV_MYSQL_HOST_DB_READONLY MySQL host database opened for read access only -This informational message is issued when the user has configured the PostgreSQL +This informational message is issued when the user has configured the MySQL database in read-only mode. Kea will not be able to insert or modify host reservations but will be able to retrieve existing ones and assign them to the clients communicating with the server. From a0f25b08ac9b9ebe665f76faa18b65e2db361549 Mon Sep 17 00:00:00 2001 From: Stephen Morris Date: Wed, 24 Aug 2016 23:43:31 +0100 Subject: [PATCH 18/19] [4489] Miscellaneous editing changes to unit test documentation --- doc/devel/unit-tests.dox | 143 ++++++++++++++++++--------------------- 1 file changed, 65 insertions(+), 78 deletions(-) diff --git a/doc/devel/unit-tests.dox b/doc/devel/unit-tests.dox index 7801e1448f..59722bf488 100644 --- a/doc/devel/unit-tests.dox +++ b/doc/devel/unit-tests.dox @@ -12,12 +12,13 @@ Kea uses the Google C++ Testing Framework (also called googletest or gtest) as a base for our C++ unit-tests. See http://code.google.com/p/googletest/ for -details. We used to have Python unit-tests that were inherited from BIND10 -days. Those tests are removed now, so please do not develop any new Python -tests in Kea. If you want to write DHCP tests in Python, we encourage you to -take a look at ISC Forge: http://kea.isc.org/wiki/IscForge. You must have \c -gtest installed or at least extracted in a directory before compiling Kea -unit-tests. To enable unit-tests in Kea, use: +details. We used to have Python unit-tests inherited from BIND10 +days but have been removed, so please do not write any new Kea unit +tests in Python. (If you want to write DHCP tests in Python, we encourage you to +take a look at ISC Forge: http://kea.isc.org/wiki/IscForge.) + +You must have \c gtest installed or at least extracted in a directory +before compiling Kea unit-tests. To enable unit-tests in Kea, use: @code ./configure --with-gtest=/path/to/your/gtest/dir @@ -31,41 +32,40 @@ or Depending on how you compiled or installed \c gtest (e.g. from sources or using some package management system) one of those two switches will -find \c gtest. After that you make run unit-tests: +find \c gtest. After that you make and run the unit-tests with: @code make check - @endcode @section unitTestsEnvironmentVariables Environment Variables -The following environment variable can affect unit-tests: +The following environment variable can affect the unit tests: - KEA_LOCKFILE_DIR - Specifies a directory where the logging system should - create its lock file. If not specified, it is prefix/var/run/kea, where prefix - defaults to /usr/local. This variable must not end with a slash. There is one - special value: "none", which instructs Kea to not create lock file at - all. This may cause issues if several processes log to the same file. - Also see Kea User's Guide, section 15.3. + create its lock file. If not specified, it is prefix/var/run/kea, + where prefix defaults to /usr/local. This variable must not end + with a slash. There is one special value, "none", which instructs Kea to + not create a lock file at all. This may cause issues if several processes + log to the same file. (Also see the Kea User's Guide, section 15.3.) -- KEA_LOGGER_DESTINATION - Specifies logging destination. If not set, logged - messages will not be recorded anywhere. There are 3 special values: +- KEA_LOGGER_DESTINATION - Specifies the logging destination. If not set, logged + messages will not be recorded anywhere. There are three special values: stdout, stderr and syslog. Any other value is interpreted as a filename. - Also see Kea User's Guide, section 15.3. + (Also see Kea User's Guide, section 15.3.) - KEA_PIDFILE_DIR - Specifies the directory which should be used for PID files - as used by dhcp::Daemon or its derivatives. If not specified, the default is - prefix/var/run/kea, where prefix defaults to /usr/local. This variable must - not end with a slash. + as used by dhcp::Daemon or its derivatives. If not specified, the + default is prefix/var/run/kea, where prefix defaults to + /usr/local. This variable must not end with a slash. - KEA_SOCKET_TEST_DIR - if set, it specifies the directory where Unix - sockets are created. There's OS limitation on how long a Unix socket - path can be. It is typcially slightly over 100 characters. If you - happen to build and run unit-tests in deeply nested directories, this - may become a problem. KEA_SOCKET_TEST_DIR can be specified to instruct - unit-test to use a different directory. Must not end with slash (e.g. - /tmp). + sockets are created. There is an operating system limitation on how + long a Unix socket path can be, typically slightly over 100 + characters. If you happen to build and run unit-tests in deeply nested + directories, this may become a problem. KEA_SOCKET_TEST_DIR can be + specified to instruct unit-test to use a different directory. It must + not end with slash. @section unitTestsDatabaseConfig Databases Configuration for Unit Tests @@ -75,36 +75,29 @@ The following environment variable can affect unit-tests: @subsection unitTestsDatabaseUsers Database Users Required for Unit Tests - Unit tests validating database backends require that keatest database - is created. This database should be empty (should not include any relations). - Unit tests will create required tables for each test case, and drop these tables - when the test case ends. The unit tests also require that keatest user - is created and that this user is configured to access keatest - database with a keatest password. - + Unit tests validating database backends require that the keatest + database is created. This database should be empty. The unit tests + also require that the keatest user is created and that this user + is configured to access the database with a password of keatest. Unit tests use these credentials to create database schema, run test cases and drop the schema. Thus, the keatest user must have sufficiently high privileges to create and drop tables, as well as insert and modify the data within those tables. - The database backends, which support read only access to the host reservations - databases (currently MySQL and PostgreSQL), include unit tests verifying that - a database user, with read-only privileges, can be used to retrieve host - reservations. Those tests require that a user keatest_readonly, with - SQL SELECT privilege to the keatest database (without INSERT, UPDATE etc.), - is also created. + The database backends which support read only access to the host + reservations databases (currently MySQL and PostgreSQL) include unit + tests verifying that a database user with read-only privileges can be + used to retrieve host reservations. Those tests require another user, + keatest_readonly, with SQL SELECT privilege to the keatest + database (i.e. without INSERT, UPDATE etc.), is also created. + keatest_readonly should also have the password keatest. The following sections provide step-by-step guidelines how to setup the databases for running unit tests. @subsection mysqlUnitTestsPrerequisites MySQL Database - A database called keatest must be created. A database user, also called - keatest (and with a password keatest) must also be created and - be given full privileges in that database. The unit tests create the schema - in the database before each test and delete it afterwards. - - In detail, the steps to create the database and user are: + The steps to create the database and users are: -# Log into MySQL as root: @verbatim @@ -117,8 +110,8 @@ The following environment variable can affect unit-tests: mysql> CREATE DATABASE keatest; mysql>@endverbatim\n -# Create the users under which the test client will connect to the database - (the apostrophes around the words keatest and localhost are - required): + (the apostrophes around the words keatest, keatest_readonly, and + localhost are required): @verbatim mysql> CREATE USER 'keatest'@'localhost' IDENTIFIED BY 'keatest'; mysql> CREATE USER 'keatest_readonly'@'localhost' IDENTIFIED BY 'keatest'; @@ -137,27 +130,19 @@ The following environment variable can affect unit-tests: %@endverbatim The unit tests are run automatically when "make check" is executed (providing - that Kea has been build with the \--with-dhcp-mysql switch (see the installation + that Kea has been build with the \c --with-dhcp-mysql switch (see the installation section in the Kea Administrator Reference Manual). @subsection pgsqlUnitTestsPrerequisites PostgreSQL Database - Conceptually, the steps required to run PostgreSQL unit-tests are the same as - in MySQL. First, a database called keatest must be created. A database - user, also called keatest (that will be allowed to log in using password - keatest) must be created and given full privileges in that database. A - database user, called keatest_readonly (using password keatest) - must be created with SELECT privilege on all tables. - The unit tests create the schema in the database before each test and delete it - afterwards. + PostgreSQL set up differs from system to system. Please consult your + operating system-specific PostgreSQL documentation. The remainder of + that section uses Ubuntu 13.10 x64 (with PostgreSQL 9.0+) as an example. - PostgreSQL set up differs from system to system. Please consult your OS-specific - PostgreSQL documentation. The remainder of that section uses Ubuntu 13.10 x64 - (with PostgreSQL 9.0+) as an example. On Ubuntu, after installing PostgreSQL - (with sudo apt-get install postgresql), it is installed as user - postgres. To create new databases or add new users, initial commands - must be issued as user postgres: + On Ubuntu, PostgreSQL is installed (with sudo apt-get install + postgresql) under user postgres. To create new databases + or add new users, initial commands must be issued under this username: @verbatim $ sudo -u postgres psql postgres @@ -180,10 +165,10 @@ postgres=# \q To ensure that the user has specific privileges to tables dynamically created by the unit tests, the default schema privileges must be altered. - The following example demonstrates how to create user keatest_readonly, + The following example demonstrates how to create the user keatest_readonly, which has SELECT privilege to the tables within the keatest database, in Postgres 9.0+. For earlier versions of Postgres, it is recommended to - simply grant full privileges to keatest_readonly user, using the + simply grant full privileges to keatest_readonly, using the same steps as for the keatest user. @verbatim @@ -206,7 +191,7 @@ ALTER DEFAULT PRIVILEGES keatest=> \q @endverbatim - Note that keatest user (rather than postgres) is used to grant + Note that the keatest user (rather than postgres) is used to grant privileges to the keatest_readonly user. This ensures that the SELECT privilege is granted only on the tables that the keatest user can access within the public schema. @@ -229,12 +214,12 @@ Type "help" for help. keatest=> @endverbatim - If instead of seeing keatest=> prompt, your login will be refused with error + If instead of seeing keatest=> prompt, your login is refused with an error code about failed peer or indent authentication, it means that PostgreSQL is - configured to check unix username and reject login attepts if PostgreSQL names - are different. To alter that, PostgreSQL configuration must be changed. - /etc/postgresql/9.1/main/pg_hba.conf config file - has to be tweaked. It may be in a different location in your system. The following + configured to check unix username and reject login attempts if PostgreSQL names + are different. To alter that, the PostgreSQL configuration must be changed - + the /etc/postgresql/9.1/main/pg_hba.conf config file + has to be altered. (It may be in a different location in your system.) The following lines: @verbatim @@ -243,7 +228,7 @@ host all all 127.0.0.1/32 md5 host all all ::1/128 md5 @endverbatim - were replaced with: +need to be replaced with: @verbatim local all all password @@ -251,12 +236,14 @@ host all all 127.0.0.1/32 password host all all ::1/128 password @endverbatim - Another possible problem is to get no password prompt, in general because - you have no pg_hba.conf config file and everybody is by default - trusted. As it has a very bad effect on the security you should have - been warned it is a highly unsafe config. The solution is the same, - i.e., require password or md5 authentication method. If you lose - the postgres user access you can add first: + Another possible problem is that you get no password prompt. This is + most probably because you have no pg_hba.conf config file + and everybody is by default trusted. As it has a very bad effect + on the security you should have been warned this is a highly unsafe + configuration. The solution is the same, i.e., require password or + md5 authentication method. + + If you lose the postgres user access you can first add: @verbatim local all postgres trust @endverbatim @@ -269,7 +256,7 @@ local all postgres trust that runs tests. Use caution! The unit tests are run automatically when "make check" is executed (providing - that Kea has been build with the \--with-dhcp-pgsql switch (see the installation + that Kea has been build with the \c --with-dhcp-pgsql switch (see the installation section in the Kea Administrator Reference Manual). From 7afd2addc6cc39dfae6bed62f52866247b9e9d68 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 25 Aug 2016 09:09:19 +0200 Subject: [PATCH 19/19] [4489] Addressed second set of review comments. Expand vector of statements depending on statement index and some little fixes in doxygen. --- src/lib/dhcpsrv/mysql_connection.cc | 15 +++++---------- src/lib/dhcpsrv/mysql_connection.h | 6 +++++- src/lib/dhcpsrv/pgsql_connection.h | 6 +++++- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/lib/dhcpsrv/mysql_connection.cc b/src/lib/dhcpsrv/mysql_connection.cc index 9569b4e8d3..06162bdf1e 100644 --- a/src/lib/dhcpsrv/mysql_connection.cc +++ b/src/lib/dhcpsrv/mysql_connection.cc @@ -12,7 +12,6 @@ #include #include -#include #include #include #include @@ -215,18 +214,14 @@ MySqlConnection::prepareStatement(uint32_t index, const char* text) { void MySqlConnection::prepareStatements(const TaggedStatement* start_statement, const TaggedStatement* end_statement) { - size_t num_statements = std::distance(start_statement, end_statement); - if (num_statements == 0) { - return; - } - - // Expand vectors of allocated statements to hold additional statements. - statements_.resize(statements_.size() + num_statements, NULL); - text_statements_.resize(text_statements_.size() + num_statements, std::string("")); - // Created the MySQL prepared statements for each DML statement. for (const TaggedStatement* tagged_statement = start_statement; tagged_statement != end_statement; ++tagged_statement) { + if (tagged_statement->index >= statements_.size()) { + statements_.resize(tagged_statement->index + 1, NULL); + text_statements_.resize(tagged_statement->index + 1, + std::string("")); + } prepareStatement(tagged_statement->index, tagged_statement->text); } diff --git a/src/lib/dhcpsrv/mysql_connection.h b/src/lib/dhcpsrv/mysql_connection.h index 89ad0ec593..4f84731bcd 100644 --- a/src/lib/dhcpsrv/mysql_connection.h +++ b/src/lib/dhcpsrv/mysql_connection.h @@ -240,7 +240,11 @@ public: /// /// Creates the prepared statements for all of the SQL statements used /// by the MySQL backend. - /// @param tagged_statements an array of statements to be compiled + /// + /// @param start_statement Pointer to the first statement in range of the + /// statements to be compiled. + /// @param end_statement Pointer to the statement marking end of the + /// range of statements to be compiled. This last statement is not compiled. /// /// @throw isc::dhcp::DbOperationError An operation on the open database has /// failed. diff --git a/src/lib/dhcpsrv/pgsql_connection.h b/src/lib/dhcpsrv/pgsql_connection.h index 2ff1831203..b0d793b611 100644 --- a/src/lib/dhcpsrv/pgsql_connection.h +++ b/src/lib/dhcpsrv/pgsql_connection.h @@ -317,7 +317,11 @@ public: /// /// Creates the prepared statements for all of the SQL statements used /// by the PostgreSQL backend. - /// @param tagged_statements an array of statements to be compiled + /// + /// @param start_statement Pointer to the first statement in range of the + /// statements to be compiled. + /// @param end_statement Pointer to the statement marking end of the + /// range of statements to be compiled. This last statement is not compiled. /// /// @throw isc::dhcp::DbOperationError An operation on the open database has /// failed.