diff --git a/src/bin/auth/Makefile.am b/src/bin/auth/Makefile.am index 6128a4cf93..9eee9d4d54 100644 --- a/src/bin/auth/Makefile.am +++ b/src/bin/auth/Makefile.am @@ -55,7 +55,7 @@ b10_auth_SOURCES += auth_config.cc auth_config.h b10_auth_SOURCES += command.cc command.h b10_auth_SOURCES += common.h common.cc b10_auth_SOURCES += statistics.cc statistics.h -b10_auth_SOURCES += datasrc_configurator.h +b10_auth_SOURCES += datasrc_config.h datasrc_config.cc b10_auth_SOURCES += main.cc nodist_b10_auth_SOURCES = auth_messages.h auth_messages.cc diff --git a/src/bin/auth/benchmarks/Makefile.am b/src/bin/auth/benchmarks/Makefile.am index 48e552a502..c525b665d3 100644 --- a/src/bin/auth/benchmarks/Makefile.am +++ b/src/bin/auth/benchmarks/Makefile.am @@ -17,6 +17,7 @@ query_bench_SOURCES += ../auth_srv.h ../auth_srv.cc query_bench_SOURCES += ../auth_config.h ../auth_config.cc query_bench_SOURCES += ../statistics.h ../statistics.cc query_bench_SOURCES += ../auth_log.h ../auth_log.cc +query_bench_SOURCES += ../datasrc_config.h ../datasrc_config.cc nodist_query_bench_SOURCES = ../auth_messages.h ../auth_messages.cc diff --git a/src/bin/auth/benchmarks/query_bench.cc b/src/bin/auth/benchmarks/query_bench.cc index 93253db8fe..73f702baff 100644 --- a/src/bin/auth/benchmarks/query_bench.cc +++ b/src/bin/auth/benchmarks/query_bench.cc @@ -30,7 +30,7 @@ #include #include -#include +#include #include #include @@ -125,8 +125,8 @@ public: OutputBuffer& buffer) : QueryBenchMark(queries, query_message, buffer) { - DataSourceConfigurator::testReconfigure( - server_.get(), + configureDataSource( + *server_, Element::fromJSON("{\"IN\":" " [{\"type\": \"sqlite3\"," " \"params\": {" @@ -139,13 +139,13 @@ class MemoryQueryBenchMark : public QueryBenchMark { public: MemoryQueryBenchMark(const char* const zone_file, const char* const zone_origin, - const BenchQueries& queries, - Message& query_message, - OutputBuffer& buffer) : + const BenchQueries& queries, + Message& query_message, + OutputBuffer& buffer) : QueryBenchMark(queries, query_message, buffer) { - DataSourceConfigurator::testReconfigure( - server_.get(), + configureDataSource( + *server_, Element::fromJSON("{\"IN\":" " [{\"type\": \"MasterFiles\"," " \"cache-enable\": true, " diff --git a/src/bin/auth/datasrc_config.cc b/src/bin/auth/datasrc_config.cc new file mode 100644 index 0000000000..73fb5190c6 --- /dev/null +++ b/src/bin/auth/datasrc_config.cc @@ -0,0 +1,26 @@ +// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC") +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +#include +#include "auth_srv.h" +#include "datasrc_config.h" + +// This is a trivial specialization for the commonly used version. +// Defined in .cc to avoid accidental creation of multiple copies. +void +configureDataSource(AuthSrv& server, const isc::data::ConstElementPtr& config) +{ + return (configureDataSourceGeneric(server, config)); +} diff --git a/src/bin/auth/datasrc_config.h b/src/bin/auth/datasrc_config.h new file mode 100644 index 0000000000..79ace2811a --- /dev/null +++ b/src/bin/auth/datasrc_config.h @@ -0,0 +1,120 @@ +// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC") +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +#ifndef DATASRC_CONFIG_H +#define DATASRC_CONFIG_H + +#include "auth_srv.h" + +#include +#include +#include + +#include + +#include + +/// \brief Configure the authoritative server's data source lists +/// +/// This will hook into the data_sources module configuration and it will +/// keep the local copy of data source clients in the list in the authoritative +/// server. +/// +/// This function is templated. This is simply because of easier testing. +/// You don't need to pay attention to it, use the configureDataSource +/// specialization instead. +/// +/// \param server It is the server to configure. +/// \param config The configuration value to parse. It is in the form +/// as an update from the config manager. +template +void +configureDataSourceGeneric(Server& server, + const isc::data::ConstElementPtr& config) +{ + typedef boost::shared_ptr ListPtr; + typedef std::map Map; + typedef std::pair RollbackPair; + typedef std::pair + RollbackConfiguration; + + // Lock the client lists, we're going to manipulate them. + isc::util::thread::Mutex::Locker locker(server.getClientListMutex()); + + // Some structures to be able to perform a rollback + std::vector rollback_sets; + std::vector rollback_configurations; + try { + // Get the configuration and current state. + const Map& map(config->mapValue()); + const std::vector + activeVector(server.getClientListClasses()); + std::set active(activeVector.begin(), + activeVector.end()); + // Go through the configuration and change everything. + for (Map::const_iterator it(map.begin()); it != map.end(); ++it) { + const isc::dns::RRClass rrclass(it->first); + active.erase(rrclass); + ListPtr list(server.getClientList(rrclass)); + bool need_set(false); + if (list) { + rollback_configurations. + push_back(RollbackConfiguration(rrclass, + list->getConfiguration())); + } else { + list.reset(new List(rrclass)); + need_set = true; + rollback_sets.push_back(RollbackPair(rrclass, ListPtr())); + } + list->configure(it->second, true); + if (need_set) { + server.setClientList(rrclass, list); + } + } + // Remove the ones that are not in the configuration. + for (std::set::iterator it(active.begin()); + it != active.end(); ++it) { + // There seems to be no way the setClientList could throw. + // But this is just to make sure in case it did to restore + // the original. + rollback_sets.push_back( + RollbackPair(*it, server.getClientList(*it))); + server.setClientList(*it, ListPtr()); + } + } catch (...) { + // Perform a rollback of the changes. The old configuration should + // work. + for (typename std::vector::const_iterator + it(rollback_sets.begin()); it != rollback_sets.end(); ++it) { + server.setClientList(it->first, it->second); + } + for (typename std::vector::const_iterator + it(rollback_configurations.begin()); + it != rollback_configurations.end(); ++it) { + server.getClientList(it->first)->configure(it->second, true); + } + throw; + } +} + +/// \brief Concrete version of configureDataSource() for the +/// use with authoritative server implementation. +void +configureDataSource(AuthSrv& server, const isc::data::ConstElementPtr& config); + +#endif // DATASRC_CONFIG_H + +// Local Variables: +// mode: c++ +// End: diff --git a/src/bin/auth/datasrc_configurator.h b/src/bin/auth/datasrc_configurator.h deleted file mode 100644 index 6b2f5826b0..0000000000 --- a/src/bin/auth/datasrc_configurator.h +++ /dev/null @@ -1,226 +0,0 @@ -// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC") -// -// Permission to use, copy, modify, and/or distribute this software for any -// purpose with or without fee is hereby granted, provided that the above -// copyright notice and this permission notice appear in all copies. -// -// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH -// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY -// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, -// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM -// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE -// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR -// PERFORMANCE OF THIS SOFTWARE. - -#ifndef DATASRC_CONFIGURATOR_H -#define DATASRC_CONFIGURATOR_H - -#include "auth_srv.h" - -#include -#include -#include -#include - -#include - -/// \brief A class to configure the authoritative server's data source lists -/// -/// This will hook into the data_sources module configuration and it will -/// keep the local copy of data source clients in the list in the authoritative -/// server. -/// -/// The class is slightly unusual. Due to some technical limitations, the hook -/// needs to be static method. Therefore it is not possible to create instances -/// of the class. -/// -/// Also, the class is a template. This is simply because of easier testing. -/// You don't need to pay attention to it, use the DataSourceConfigurator -/// type alias instead. -template -class DataSourceConfiguratorGeneric { -private: - /// \brief Disallow creation of instances - DataSourceConfiguratorGeneric(); - /// \brief Internal method to hook into the ModuleCCSession - /// - /// It simply calls reconfigure. - static void reconfigureInternal(const std::string&, - isc::data::ConstElementPtr config, - const isc::config::ConfigData&) - { - if (config->contains("classes")) { - reconfigure(config->get("classes")); - } - } - static Server* server_; - static isc::config::ModuleCCSession* session_; - typedef boost::shared_ptr ListPtr; -public: - /// \brief Initializes the class. - /// - /// This configures which session and server should be used. - /// It hooks to the session now and downloads the configuration. - /// It is synchronous (it may block for some time). - /// - /// Note that you need to call cleanup before the server or - /// session dies, otherwise it might access them after they - /// are destroyed. - /// - /// \param session The session to hook into and to access the configuration - /// through. - /// \param server It is the server to configure. - /// \throw isc::InvalidOperation if this is called when already initialized. - /// \throw isc::InvalidParameter if any of the parameters is NULL - /// \throw isc::config::ModuleCCError if the remote configuration is not - /// available for some reason. - static void init(isc::config::ModuleCCSession* session, - Server* server) - { - if (session == NULL) { - isc_throw(isc::InvalidParameter, "The session must not be NULL"); - } - if (server == NULL) { - isc_throw(isc::InvalidParameter, "The server must not be NULL"); - } - if (server_ != NULL) { - isc_throw(isc::InvalidOperation, - "The configurator is already initialized"); - } - server_ = server; - session_ = session; - session->addRemoteConfig("data_sources", reconfigureInternal, false); - } - /// \brief Deinitializes the class. - /// - /// This detaches from the session and removes the server from internal - /// storage. The current configuration in the server is preserved. - /// - /// This can be called even if it is not initialized currently. You - /// can initialize it again after this. - static void cleanup() { - if (session_ != NULL) { - session_->removeRemoteConfig("data_sources"); - } - session_ = NULL; - server_ = NULL; - } - /// \brief Reads new configuration and replaces the old one. - /// - /// It instructs the server to replace the lists with new ones as needed. - /// You don't need to call it directly (but you could, though the benefit - /// is unknown and it would be questionable at least). It is called - /// automatically on normal updates. - /// - /// \param config The configuration value to parse. It is in the form - /// as an update from the config manager. - /// \throw InvalidOperation if it is called when not initialized. - static void reconfigure(const isc::data::ConstElementPtr& config) { - if (server_ == NULL) { - isc_throw(isc::InvalidOperation, - "Can't reconfigure while not initialized by init()"); - } - // Lock the client lists, we're going to manipulate them. - isc::util::thread::Mutex::Locker locker(server_->getClientListMutex()); - typedef std::map Map; - typedef std::pair RollbackPair; - typedef std::pair - RollbackConfiguration; - // Some structures to be able to perform a rollback - std::vector rollback_sets; - std::vector rollback_configurations; - try { - // Get the configuration and current state. - const Map& map(config->mapValue()); - const std::vector - activeVector(server_->getClientListClasses()); - std::set active(activeVector.begin(), - activeVector.end()); - // Go through the configuration and change everything. - for (Map::const_iterator it(map.begin()); it != map.end(); ++it) { - isc::dns::RRClass rrclass(it->first); - active.erase(rrclass); - ListPtr list(server_->getClientList(rrclass)); - bool need_set(false); - if (list) { - rollback_configurations. - push_back(RollbackConfiguration(rrclass, - list->getConfiguration())); - } else { - list.reset(new List(rrclass)); - need_set = true; - rollback_sets.push_back(RollbackPair(rrclass, ListPtr())); - } - list->configure(it->second, true); - if (need_set) { - server_->setClientList(rrclass, list); - } - } - // Remove the ones that are not in the configuration. - for (std::set::iterator it(active.begin()); - it != active.end(); ++it) { - // There seems to be no way the setClientList could throw. - // But this is just to make sure in case it did to restore - // the original. - rollback_sets.push_back( - RollbackPair(*it, server_->getClientList(*it))); - server_->setClientList(*it, ListPtr()); - } - } catch (...) { - // Perform a rollback of the changes. The old configuration should - // work. - for (typename std::vector::const_iterator - it(rollback_sets.begin()); it != rollback_sets.end(); ++it) { - server_->setClientList(it->first, it->second); - } - for (typename std::vector::const_iterator - it(rollback_configurations.begin()); - it != rollback_configurations.end(); ++it) { - server_->getClientList(it->first)->configure(it->second, true); - } - throw; - } - } - /// \brief Version of reconfigure for easier testing. - /// - /// This method can be used to reconfigure a server without first - /// initializing the configurator. This does not need a session. - /// Otherwise, it acts the same as reconfigure. - /// - /// This is not meant for production code. Do not use there. - /// - /// \param server The server to configure. - /// \param config The config to use. - /// \throw isc::InvalidOperation if the configurator is initialized. - /// \throw anything that reconfigure does. - static void testReconfigure(Server* server, - const isc::data::ConstElementPtr& config) - { - if (server_ != NULL) { - isc_throw(isc::InvalidOperation, "Currently initialized."); - } - try { - server_ = server; - reconfigure(config); - server_ = NULL; - } catch (...) { - server_ = NULL; - throw; - } - } -}; - -template -isc::config::ModuleCCSession* -DataSourceConfiguratorGeneric::session_(NULL); - -template -Server* DataSourceConfiguratorGeneric::server_(NULL); - -/// \brief Concrete version of DataSourceConfiguratorGeneric for the -/// use in authoritative server. -typedef DataSourceConfiguratorGeneric - DataSourceConfigurator; - -#endif diff --git a/src/bin/auth/main.cc b/src/bin/auth/main.cc index 3cca3c0f36..b425813006 100644 --- a/src/bin/auth/main.cc +++ b/src/bin/auth/main.cc @@ -14,17 +14,6 @@ #include -#include -#include -#include -#include -#include -#include -#include - -#include -#include - #include #include @@ -45,13 +34,26 @@ #include #include #include -#include +#include #include #include #include #include #include +#include + +#include +#include +#include +#include +#include +#include +#include + +#include +#include + using namespace std; using namespace isc::asiodns; using namespace isc::asiolink; @@ -70,7 +72,7 @@ namespace { /* need global var for config/command handlers. * todo: turn this around, and put handlers in the authserver * class itself? */ -AuthSrv *auth_server; +AuthSrv* auth_server; ConstElementPtr my_config_handler(ConstElementPtr new_config) { @@ -83,6 +85,29 @@ my_command_handler(const string& command, ConstElementPtr args) { return (execAuthServerCommand(*auth_server, command, args)); } +void +datasrcConfigHandler(AuthSrv* server, bool* first_time, + ModuleCCSession* config_session, const std::string&, + isc::data::ConstElementPtr config, + const isc::config::ConfigData&) +{ + assert(server != NULL); + if (config->contains("classes")) { + if (*first_time) { + // HACK: The default is not passed to the handler in the first + // callback. This one will get the default (or, current value). + // Further updates will work the usual way. + assert(config_session != NULL); + *first_time = false; + configureDataSource(*auth_server, + config_session->getRemoteConfigValue( + "data_sources", "classes")); + } else { + configureDataSource(*server, config->get("classes")); + } + } +} + void usage() { cerr << "Usage: b10-auth [-v]" @@ -191,13 +216,15 @@ main(int argc, char* argv[]) { isc::server_common::initKeyring(*config_session); auth_server->setTSIGKeyRing(&isc::server_common::keyring); - // Start the data source configuration - DataSourceConfigurator::init(config_session, auth_server); - // HACK: The default is not passed to the handler. This one will - // get the default (or, current value). Further updates will work - // the usual way. - DataSourceConfigurator::reconfigure( - config_session->getRemoteConfigValue("data_sources", "classes")); + // Start the data source configuration. We pass first_time and + // config_session for the hack described in datasrcConfigHandler. + bool first_time = true; + config_session->addRemoteConfig("data_sources", + boost::bind(datasrcConfigHandler, + auth_server, &first_time, + config_session, + _1, _2, _3), + false); // Now start asynchronous read. config_session->start(); @@ -221,7 +248,10 @@ main(int argc, char* argv[]) { xfrin_session->disconnect(); } - DataSourceConfigurator::cleanup(); + // If we haven't registered callback for data sources, this will be just + // no-op. + config_session->removeRemoteConfig("data_sources"); + delete xfrin_session; delete config_session; delete cc_session; diff --git a/src/bin/auth/tests/Makefile.am b/src/bin/auth/tests/Makefile.am index 7c01a6bd3f..6b9d385298 100644 --- a/src/bin/auth/tests/Makefile.am +++ b/src/bin/auth/tests/Makefile.am @@ -42,6 +42,7 @@ run_unittests_SOURCES += ../auth_config.h ../auth_config.cc run_unittests_SOURCES += ../command.h ../command.cc run_unittests_SOURCES += ../common.h ../common.cc run_unittests_SOURCES += ../statistics.h ../statistics.cc +run_unittests_SOURCES += ../datasrc_config.h ../datasrc_config.cc run_unittests_SOURCES += datasrc_util.h datasrc_util.cc run_unittests_SOURCES += auth_srv_unittest.cc run_unittests_SOURCES += config_unittest.cc @@ -50,7 +51,7 @@ run_unittests_SOURCES += command_unittest.cc run_unittests_SOURCES += common_unittest.cc run_unittests_SOURCES += query_unittest.cc run_unittests_SOURCES += statistics_unittest.cc -run_unittests_SOURCES += datasrc_configurator_unittest.cc +run_unittests_SOURCES += datasrc_config_unittest.cc run_unittests_SOURCES += run_unittests.cc nodist_run_unittests_SOURCES = ../auth_messages.h ../auth_messages.cc diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc index 3197e7ecae..60a9a2ac38 100644 --- a/src/bin/auth/tests/auth_srv_unittest.cc +++ b/src/bin/auth/tests/auth_srv_unittest.cc @@ -36,7 +36,7 @@ #include #include #include -#include +#include #include #include @@ -722,17 +722,17 @@ TEST_F(AuthSrvTest, notifyWithSessionMessageError) { } void -updateDatabase(AuthSrv* server, const char* params) { +updateDatabase(AuthSrv& server, const char* params) { const ConstElementPtr config(Element::fromJSON("{" "\"IN\": [{" " \"type\": \"sqlite3\"," " \"params\": " + string(params) + "}]}")); - DataSourceConfigurator::testReconfigure(server, config); + configureDataSource(server, config); } void -updateInMemory(AuthSrv* server, const char* origin, const char* filename) { +updateInMemory(AuthSrv& server, const char* origin, const char* filename) { const ConstElementPtr config(Element::fromJSON("{" "\"IN\": [{" " \"type\": \"MasterFiles\"," @@ -745,17 +745,17 @@ updateInMemory(AuthSrv* server, const char* origin, const char* filename) { " \"type\": \"static\"," " \"params\": \"" + string(STATIC_DSRC_FILE) + "\"" "}]}")); - DataSourceConfigurator::testReconfigure(server, config); + configureDataSource(server, config); } void -updateBuiltin(AuthSrv* server) { +updateBuiltin(AuthSrv& server) { const ConstElementPtr config(Element::fromJSON("{" "\"CH\": [{" " \"type\": \"static\"," " \"params\": \"" + string(STATIC_DSRC_FILE) + "\"" "}]}")); - DataSourceConfigurator::testReconfigure(server, config); + configureDataSource(server, config); } // Try giving the server a TSIG signed request and see it can anwer signed as @@ -766,7 +766,7 @@ TEST_F(AuthSrvTest, DISABLED_TSIGSigned) { // Needs builtin TEST_F(AuthSrvTest, TSIGSigned) { #endif // Prepare key, the client message, etc - updateBuiltin(&server); + updateBuiltin(server); const TSIGKey key("key:c2VjcmV0Cg==:hmac-sha1"); TSIGContext context(key); UnitTestUtil::createRequestMessage(request_message, opcode, default_qid, @@ -814,7 +814,7 @@ TEST_F(AuthSrvTest, DISABLED_builtInQueryViaDNSServer) { #else TEST_F(AuthSrvTest, builtInQueryViaDNSServer) { #endif - updateBuiltin(&server); + updateBuiltin(server); UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(), default_qid, Name("VERSION.BIND."), RRClass::CH(), RRType::TXT()); @@ -846,7 +846,7 @@ TEST_F(AuthSrvTest, DISABLED_builtInQuery) { #else TEST_F(AuthSrvTest, builtInQuery) { #endif - updateBuiltin(&server); + updateBuiltin(server); UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(), default_qid, Name("VERSION.BIND."), RRClass::CH(), RRType::TXT()); @@ -867,7 +867,7 @@ TEST_F(AuthSrvTest, DISABLED_iqueryViaDNSServer) { // Needs builtin #else TEST_F(AuthSrvTest, iqueryViaDNSServer) { // Needs builtin #endif - updateBuiltin(&server); + updateBuiltin(server); createDataFromFile("iquery_fromWire.wire"); (*server.getDNSLookupProvider())(*io_message, parse_message, response_message, @@ -889,7 +889,7 @@ TEST_F(AuthSrvTest, DISABLED_updateConfig) { #else TEST_F(AuthSrvTest, updateConfig) { #endif - updateDatabase(&server, CONFIG_TESTDB); + updateDatabase(server, CONFIG_TESTDB); // query for existent data in the installed data source. The resulting // response should have the AA flag on, and have an RR in each answer @@ -907,7 +907,7 @@ TEST_F(AuthSrvTest, DISABLED_datasourceFail) { #else TEST_F(AuthSrvTest, datasourceFail) { #endif - updateDatabase(&server, CONFIG_TESTDB); + updateDatabase(server, CONFIG_TESTDB); // This query will hit a corrupted entry of the data source (the zoneload // tool and the data source itself naively accept it). This will result @@ -927,10 +927,10 @@ TEST_F(AuthSrvTest, DISABLED_updateConfigFail) { TEST_F(AuthSrvTest, updateConfigFail) { #endif // First, load a valid data source. - updateDatabase(&server, CONFIG_TESTDB); + updateDatabase(server, CONFIG_TESTDB); // Next, try to update it with a non-existent one. This should fail. - EXPECT_THROW(updateDatabase(&server, BADCONFIG_TESTDB), + EXPECT_THROW(updateDatabase(server, BADCONFIG_TESTDB), isc::datasrc::DataSourceError); // The original data source should still exist. @@ -953,7 +953,7 @@ TEST_F(AuthSrvTest, updateWithInMemoryClient) { " \"params\": {}," " \"cache-enable\": true" "}]}")); - DataSourceConfigurator::testReconfigure(&server, config); + configureDataSource(server, config); // after successful configuration, we should have one (with empty zoneset). // The memory data source is empty, should return REFUSED rcode. @@ -974,7 +974,7 @@ TEST_F(AuthSrvTest, queryWithInMemoryClientNoDNSSEC) { // query handler class, and confirm it returns no error and a non empty // answer section. Detailed examination on the response content // for various types of queries are tested in the query tests. - updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE); + updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE); createDataFromFile("nsec3query_nodnssec_fromWire.wire"); server.processMessage(*io_message, *parse_message, *response_obuffer, @@ -993,7 +993,7 @@ TEST_F(AuthSrvTest, queryWithInMemoryClientDNSSEC) { // Similar to the previous test, but the query has the DO bit on. // The response should contain RRSIGs, and should have more RRs than // the previous case. - updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE); + updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE); createDataFromFile("nsec3query_fromWire.wire"); server.processMessage(*io_message, *parse_message, *response_obuffer, @@ -1013,7 +1013,7 @@ TEST_F(AuthSrvTest, ) { // Set up the in-memory - updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE); + updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE); // This shouldn't affect the result of class CH query UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(), @@ -1425,7 +1425,7 @@ TEST_F(AuthSrvTest, ) { // Set real inmem client to proxy - updateInMemory(&server, "example.", CONFIG_INMEMORY_EXAMPLE); + updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE); { isc::util::thread::Mutex::Locker locker(server.getClientListMutex()); boost::shared_ptr @@ -1450,16 +1450,16 @@ TEST_F(AuthSrvTest, // If non null rrset is given, it will be passed to the proxy so it can // return some faked response. void -setupThrow(AuthSrv* server, ThrowWhen throw_when, bool isc_exception, +setupThrow(AuthSrv& server, ThrowWhen throw_when, bool isc_exception, ConstRRsetPtr rrset = ConstRRsetPtr()) { updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE); - isc::util::thread::Mutex::Locker locker(server->getClientListMutex()); + isc::util::thread::Mutex::Locker locker(server.getClientListMutex()); boost::shared_ptr - list(new FakeList(server->getClientList(RRClass::IN()), throw_when, + list(new FakeList(server.getClientList(RRClass::IN()), throw_when, isc_exception, rrset)); - server->setClientList(RRClass::IN(), list); + server.setClientList(RRClass::IN(), list); } TEST_F(AuthSrvTest, @@ -1482,11 +1482,11 @@ TEST_F(AuthSrvTest, RRClass::IN(), RRType::TXT()); for (ThrowWhen* when(throws); *when != THROW_NEVER; ++when) { createRequestPacket(request_message, IPPROTO_UDP); - setupThrow(&server, *when, true); + setupThrow(server, *when, true); processAndCheckSERVFAIL(); // To be sure, check same for non-isc-exceptions createRequestPacket(request_message, IPPROTO_UDP); - setupThrow(&server, *when, false); + setupThrow(server, *when, false); processAndCheckSERVFAIL(); } } @@ -1502,7 +1502,7 @@ TEST_F(AuthSrvTest, ) { createDataFromFile("nsec3query_nodnssec_fromWire.wire"); - setupThrow(&server, THROW_AT_GET_CLASS, true); + setupThrow(server, THROW_AT_GET_CLASS, true); // getClass is not called so it should just answer server.processMessage(*io_message, *parse_message, *response_obuffer, @@ -1526,7 +1526,7 @@ TEST_F(AuthSrvTest, ConstRRsetPtr empty_rrset(new RRset(Name("foo.example"), RRClass::IN(), RRType::TXT(), RRTTL(0))); - setupThrow(&server, THROW_NEVER, true, empty_rrset); + setupThrow(server, THROW_NEVER, true, empty_rrset); // Repeat the query processing two times. Due to the faked RRset, // toWire() should throw, and it should result in SERVFAIL. diff --git a/src/bin/auth/tests/command_unittest.cc b/src/bin/auth/tests/command_unittest.cc index e57de93d67..c7994831a5 100644 --- a/src/bin/auth/tests/command_unittest.cc +++ b/src/bin/auth/tests/command_unittest.cc @@ -19,7 +19,7 @@ #include #include #include -#include +#include #include #include @@ -208,7 +208,7 @@ configureZones(AuthSrv& server) { " \"cache-enable\": true" "}]}")); - DataSourceConfigurator::testReconfigure(&server, config); + configureDataSource(server, config); zoneChecks(server); } @@ -271,7 +271,7 @@ TEST_F(AuthCommandTest, " \"cache-enable\": true," " \"cache-zones\": [\"example.org\"]" "}]}")); - DataSourceConfigurator::testReconfigure(&server_, config); + configureDataSource(server_, config); { isc::util::thread::Mutex::Locker locker(server_.getClientListMutex()); @@ -335,7 +335,7 @@ TEST_F(AuthCommandTest, " \"cache-enable\": true," " \"cache-zones\": [\"example.com\"]" "}]}")); - EXPECT_THROW(DataSourceConfigurator::testReconfigure(&server_, config2), + EXPECT_THROW(configureDataSource(server_, config2), ConfigurableClientList::ConfigurationError); result_ = execAuthServerCommand(server_, "loadzone", diff --git a/src/bin/auth/tests/datasrc_configurator_unittest.cc b/src/bin/auth/tests/datasrc_config_unittest.cc similarity index 78% rename from src/bin/auth/tests/datasrc_configurator_unittest.cc rename to src/bin/auth/tests/datasrc_config_unittest.cc index 254b2380c7..3d3aa58b91 100644 --- a/src/bin/auth/tests/datasrc_configurator_unittest.cc +++ b/src/bin/auth/tests/datasrc_config_unittest.cc @@ -12,16 +12,19 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. -#include +#include #include #include #include #include -#include + +#include #include +#include + using namespace isc; using namespace isc::cc; using namespace isc::config; @@ -32,7 +35,7 @@ using namespace boost; namespace { -class DatasrcConfiguratorTest; +class DatasrcConfigTest; class FakeList { public: @@ -57,12 +60,26 @@ private: typedef shared_ptr ListPtr; -// We use the test fixture as both parameters, this makes it possible -// to easily fake all needed methods and look that they were called. -typedef DataSourceConfiguratorGeneric Configurator; +void +testConfigureDataSource(DatasrcConfigTest& test, + const isc::data::ConstElementPtr& config) +{ + // We use the test fixture for the Server type. This makes it possible + // to easily fake all needed methods and look that they were called. + configureDataSourceGeneric(test, config); +} -class DatasrcConfiguratorTest : public ::testing::Test { +void +datasrcConfigHandler(DatasrcConfigTest* fake_server, const std::string&, + isc::data::ConstElementPtr config, + const isc::config::ConfigData&) +{ + if (config->contains("classes")) { + testConfigureDataSource(*fake_server, config->get("classes")); + } +} + +class DatasrcConfigTest : public ::testing::Test { public: // These pretend to be the server ListPtr getClientList(const RRClass& rrclass) { @@ -86,7 +103,7 @@ public: return (mutex_); } protected: - DatasrcConfiguratorTest() : + DatasrcConfigTest() : session(ElementPtr(new ListElement), ElementPtr(new ListElement), ElementPtr(new ListElement)), specfile(string(TEST_OWN_DATA_DIR) + "/spec.spec") @@ -99,25 +116,24 @@ protected: false)); } void TearDown() { - // Make sure no matter what we did, it is cleaned up. - Configurator::cleanup(); + // Make sure no matter what we did, it is cleaned up. Also check + // we really have subscribed to the configuration, and after removing + // it we actually cancel it. + EXPECT_TRUE(session.haveSubscription("data_sources", "*")); + mccs->removeRemoteConfig("data_sources"); + EXPECT_FALSE(session.haveSubscription("data_sources", "*")); } - void init(const ElementPtr& config = ElementPtr()) { + void SetUp() { session.getMessages()-> add(createAnswer(0, moduleSpecFromFile(string(PLUGIN_DATA_PATH) + "/datasrc.spec"). getFullSpec())); - if (config) { - session.getMessages()->add(createAnswer(0, config)); - } else { - session.getMessages()-> - add(createAnswer(0, ElementPtr(new MapElement))); - } - Configurator::init(mccs.get(), this); - } - void SetUp() { - init(); + session.getMessages()->add(createAnswer(0, + ElementPtr(new MapElement))); + mccs->addRemoteConfig("data_sources", + boost::bind(datasrcConfigHandler, + this, _1, _2, _3), false); } ElementPtr buildConfig(const string& config) const { const ElementPtr internal(Element::fromJSON(config)); @@ -126,10 +142,10 @@ protected: return (external); } void initializeINList() { - const ElementPtr + const ConstElementPtr config(buildConfig("{\"IN\": [{\"type\": \"xxx\"}]}")); - session.addMessage(createCommand("config_update", config), "data_sources", - "*"); + session.addMessage(createCommand("config_update", config), + "data_sources", "*"); mccs->checkCommand(); // Check it called the correct things (check that there's no IN yet and // set a new one. @@ -144,33 +160,12 @@ protected: mutable isc::util::thread::Mutex mutex_; }; -// Check the initialization (and cleanup) -TEST_F(DatasrcConfiguratorTest, initialization) { - // It can't be initialized again - EXPECT_THROW(init(), InvalidOperation); - EXPECT_TRUE(session.haveSubscription("data_sources", "*")); - // Deinitialize to make the tests reasonable - Configurator::cleanup(); - EXPECT_FALSE(session.haveSubscription("data_sources", "*")); - // We can't reconfigure now (not even manually) - EXPECT_THROW(Configurator::reconfigure(ElementPtr(new MapElement())), - InvalidOperation); - // If one of them is NULL, it does not work - EXPECT_THROW(Configurator::init(NULL, this), InvalidParameter); - EXPECT_FALSE(session.haveSubscription("data_sources", "*")); - EXPECT_THROW(Configurator::init(mccs.get(), NULL), InvalidParameter); - EXPECT_FALSE(session.haveSubscription("data_sources", "*")); - // But we can initialize it again now - EXPECT_NO_THROW(init()); - EXPECT_TRUE(session.haveSubscription("data_sources", "*")); -} - // Push there a configuration with a single list. -TEST_F(DatasrcConfiguratorTest, createList) { +TEST_F(DatasrcConfigTest, createList) { initializeINList(); } -TEST_F(DatasrcConfiguratorTest, modifyList) { +TEST_F(DatasrcConfigTest, modifyList) { // First, initialize the list initializeINList(); // And now change the configuration of the list @@ -188,7 +183,7 @@ TEST_F(DatasrcConfiguratorTest, modifyList) { } // Check we can have multiple lists at once -TEST_F(DatasrcConfiguratorTest, multiple) { +TEST_F(DatasrcConfigTest, multiple) { const ElementPtr config(buildConfig("{\"IN\": [{\"type\": \"yyy\"}], " "\"CH\": [{\"type\": \"xxx\"}]}")); @@ -208,7 +203,7 @@ TEST_F(DatasrcConfiguratorTest, multiple) { // // It's almost like above, but we initialize first with single-list // config. -TEST_F(DatasrcConfiguratorTest, updateAdd) { +TEST_F(DatasrcConfigTest, updateAdd) { initializeINList(); const ElementPtr config(buildConfig("{\"IN\": [{\"type\": \"yyy\"}], " @@ -226,7 +221,7 @@ TEST_F(DatasrcConfiguratorTest, updateAdd) { } // We delete a class list in this test. -TEST_F(DatasrcConfiguratorTest, updateDelete) { +TEST_F(DatasrcConfigTest, updateDelete) { initializeINList(); const ElementPtr config(buildConfig("{}")); @@ -243,7 +238,7 @@ TEST_F(DatasrcConfiguratorTest, updateDelete) { } // Check that we can rollback an addition if something else fails -TEST_F(DatasrcConfiguratorTest, rollbackAddition) { +TEST_F(DatasrcConfigTest, rollbackAddition) { initializeINList(); // The configuration is wrong. However, the CH one will get done first. const ElementPtr @@ -262,13 +257,13 @@ TEST_F(DatasrcConfiguratorTest, rollbackAddition) { } // Check that we can rollback a deletion if something else fails -TEST_F(DatasrcConfiguratorTest, rollbackDeletion) { +TEST_F(DatasrcConfigTest, rollbackDeletion) { initializeINList(); // Put the CH there const ElementPtr config1(Element::fromJSON("{\"IN\": [{\"type\": \"yyy\"}], " "\"CH\": [{\"type\": \"xxx\"}]}")); - Configurator::reconfigure(config1); + testConfigureDataSource(*this, config1); const ElementPtr config2(Element::fromJSON("{\"IN\": [{\"type\": 13}]}")); // This would delete CH. However, the IN one fails. @@ -276,26 +271,26 @@ TEST_F(DatasrcConfiguratorTest, rollbackDeletion) { // and there's no known way to cause an exception during the // deletions, it is not a true rollback, but the result should // be the same. - EXPECT_THROW(Configurator::reconfigure(config2), TypeError); + EXPECT_THROW(testConfigureDataSource(*this, config2), TypeError); EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf()); EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf()); } // Check that we can roll back configuration change if something // fails later on. -TEST_F(DatasrcConfiguratorTest, rollbackConfiguration) { +TEST_F(DatasrcConfigTest, rollbackConfiguration) { initializeINList(); // Put the CH there const ElementPtr config1(Element::fromJSON("{\"IN\": [{\"type\": \"yyy\"}], " "\"CH\": [{\"type\": \"xxx\"}]}")); - Configurator::reconfigure(config1); + testConfigureDataSource(*this, config1); // Now, the CH happens first. But nevertheless, it should be // restored to the previoeus version. const ElementPtr config2(Element::fromJSON("{\"IN\": [{\"type\": 13}], " "\"CH\": [{\"type\": \"yyy\"}]}")); - EXPECT_THROW(Configurator::reconfigure(config2), TypeError); + EXPECT_THROW(testConfigureDataSource(*this, config2), TypeError); EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf()); EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf()); } diff --git a/src/lib/config/ccsession.cc b/src/lib/config/ccsession.cc index daec005dda..8516d6c863 100644 --- a/src/lib/config/ccsession.cc +++ b/src/lib/config/ccsession.cc @@ -338,7 +338,7 @@ getRelatedLoggers(ConstElementPtr loggers) { // Now find the wildcard names (the one that start with "*"). BOOST_FOREACH(ConstElementPtr cur_logger, loggers->listValue()) { - std::string cur_name = cur_logger->get("name")->stringValue(); + const std::string cur_name = cur_logger->get("name")->stringValue(); // If name is '*', or starts with '*.', replace * with root // logger name. if (cur_name == "*" || cur_name.length() > 1 && @@ -671,9 +671,7 @@ ModuleCCSession::fetchRemoteSpec(const std::string& module, bool is_filename) { std::string ModuleCCSession::addRemoteConfig(const std::string& spec_name, - void (*handler)(const std::string& module, - ConstElementPtr, - const ConfigData&), + RemoteHandler handler, bool spec_is_filename) { // First get the module name, specification and default config diff --git a/src/lib/config/ccsession.h b/src/lib/config/ccsession.h index e96a33d44b..4b99a44bd8 100644 --- a/src/lib/config/ccsession.h +++ b/src/lib/config/ccsession.h @@ -283,7 +283,7 @@ public: * the configuration manager must know it). If * spec_is_filename is true (the default), then a * filename is assumed, otherwise a module name. - * \param handler The handler function called whenever there's a change. + * \param handler The handler functor called whenever there's a change. * Called once initally from this function. May be NULL * if you don't want any handler to be called and you're * fine with requesting the data through @@ -296,11 +296,11 @@ public: * \return The name of the module specified in the given specification * file */ + typedef boost::function RemoteHandler; std::string addRemoteConfig(const std::string& spec_name, - void (*handler)(const std::string& module_name, - isc::data::ConstElementPtr - update, - const ConfigData& config_data) = NULL, + RemoteHandler handler = RemoteHandler(), bool spec_is_filename = true); /** @@ -513,9 +513,6 @@ private: const std::string& command, isc::data::ConstElementPtr args); - typedef void (*RemoteHandler)(const std::string&, - isc::data::ConstElementPtr, - const ConfigData&); std::map remote_module_configs_; std::map remote_module_handlers_;