diff --git a/src/bin/auth/Makefile.am b/src/bin/auth/Makefile.am index 6128a4cf93..df30a428db 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_configurator.h datasrc_configurator.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..fcfcb8add4 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_configurator.h ../datasrc_configurator.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 fa1c28e16c..07111a56d4 100644 --- a/src/bin/auth/benchmarks/query_bench.cc +++ b/src/bin/auth/benchmarks/query_bench.cc @@ -109,7 +109,6 @@ private: MockSocketSessionForwarder ddns_forwarder; protected: AuthSrvPtr server_; - DataSourceConfigurator datasrc_configurator_; private: const BenchQueries& queries_; Message& query_message_; @@ -126,7 +125,7 @@ public: OutputBuffer& buffer) : QueryBenchMark(queries, query_message, buffer) { - datasrc_configurator_.reconfigure( + configureDataSource( *server_, Element::fromJSON("{\"IN\":" " [{\"type\": \"sqlite3\"," @@ -145,7 +144,7 @@ public: OutputBuffer& buffer) : QueryBenchMark(queries, query_message, buffer) { - datasrc_configurator_.reconfigure( + configureDataSource( *server_, Element::fromJSON("{\"IN\":" " [{\"type\": \"MasterFiles\"," diff --git a/src/bin/auth/datasrc_configurator.cc b/src/bin/auth/datasrc_configurator.cc new file mode 100644 index 0000000000..5469389a0e --- /dev/null +++ b/src/bin/auth/datasrc_configurator.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_configurator.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_configurator.h b/src/bin/auth/datasrc_configurator.h index 4921d3613f..b8d21f3e98 100644 --- a/src/bin/auth/datasrc_configurator.h +++ b/src/bin/auth/datasrc_configurator.h @@ -17,110 +17,101 @@ #include "auth_srv.h" +#include #include #include +#include + #include -/// \brief A class to configure the authoritative server's data source lists +/// \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. /// -/// 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. +/// 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 -class DataSourceConfiguratorGeneric { -private: +void +configureDataSourceGeneric(Server& server, + const isc::data::ConstElementPtr& config) +{ typedef boost::shared_ptr ListPtr; -public: - /// \brief Default constructor. - DataSourceConfiguratorGeneric() {} + typedef std::map Map; + typedef std::pair RollbackPair; + typedef std::pair + RollbackConfiguration; - /// \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 server The server for which the data sources are to be - /// configured. - /// \param config The configuration value to parse. It is in the form - /// as an update from the config manager. - void reconfigure(Server& server, - const isc::data::ConstElementPtr& config) - { - // 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); - } + // 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())); } - // 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()); + list->configure(it->second, true); + if (need_set) { + server.setClientList(rrclass, list); } - } 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; } + // 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 DataSourceConfiguratorGeneric for the -/// use in authoritative server. -typedef DataSourceConfiguratorGeneric - DataSourceConfigurator; +/// use with authoritative server implementation. +void +configureDataSource(AuthSrv& server, const isc::data::ConstElementPtr& config); #endif diff --git a/src/bin/auth/main.cc b/src/bin/auth/main.cc index 5a720637ef..eecc5aa1a6 100644 --- a/src/bin/auth/main.cc +++ b/src/bin/auth/main.cc @@ -87,13 +87,13 @@ my_command_handler(const string& command, ConstElementPtr args) { } void -datasrcConfigHandler(AuthSrv* server, DataSourceConfigurator* configurator, - const std::string&, isc::data::ConstElementPtr config, +datasrcConfigHandler(AuthSrv* server, const std::string&, + isc::data::ConstElementPtr config, const isc::config::ConfigData&) { assert(server != NULL); if (config->contains("classes")) { - configurator->reconfigure(*server, config->get("classes")); + configureDataSource(*server, config->get("classes")); } } @@ -141,7 +141,6 @@ main(int argc, char* argv[]) { ModuleCCSession* config_session = NULL; XfroutClient xfrout_client(getXfroutSocketPath()); SocketSessionForwarder ddns_forwarder(getDDNSSocketPath()); - boost::scoped_ptr datasrc_configurator; try { string specfile; if (getenv("B10_FROM_BUILD")) { @@ -207,19 +206,16 @@ main(int argc, char* argv[]) { auth_server->setTSIGKeyRing(&isc::server_common::keyring); // Start the data source configuration - datasrc_configurator.reset(new DataSourceConfigurator); config_session->addRemoteConfig("data_sources", boost::bind(datasrcConfigHandler, auth_server, - datasrc_configurator.get(), _1, _2, _3), false); // 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. - datasrc_configurator->reconfigure( - *auth_server, + configureDataSource(*auth_server, config_session->getRemoteConfigValue("data_sources", "classes")); // Now start asynchronous read. @@ -244,9 +240,10 @@ main(int argc, char* argv[]) { xfrin_session->disconnect(); } - if (datasrc_configurator) { - config_session->removeRemoteConfig("data_sources"); - } + // 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..3b1873678c 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_configurator.h ../datasrc_configurator.cc run_unittests_SOURCES += datasrc_util.h datasrc_util.cc run_unittests_SOURCES += auth_srv_unittest.cc run_unittests_SOURCES += config_unittest.cc diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc index 0bdb9d5acc..00d7350594 100644 --- a/src/bin/auth/tests/auth_srv_unittest.cc +++ b/src/bin/auth/tests/auth_srv_unittest.cc @@ -183,7 +183,6 @@ protected: vector response_data; AddressList address_store_; TestSocketRequestor sock_requestor_; - DataSourceConfigurator datasrc_configurator_; }; // A helper function that builds a response to version.bind/TXT/CH that @@ -723,21 +722,17 @@ TEST_F(AuthSrvTest, notifyWithSessionMessageError) { } void -updateDatabase(AuthSrv& server, DataSourceConfigurator& datasrc_configurator, - const char* params) -{ +updateDatabase(AuthSrv& server, const char* params) { const ConstElementPtr config(Element::fromJSON("{" "\"IN\": [{" " \"type\": \"sqlite3\"," " \"params\": " + string(params) + "}]}")); - datasrc_configurator.reconfigure(server, config); + configureDataSource(server, config); } void -updateInMemory(AuthSrv& server, DataSourceConfigurator& datasrc_configurator, - const char* origin, const char* filename) -{ +updateInMemory(AuthSrv& server, const char* origin, const char* filename) { const ConstElementPtr config(Element::fromJSON("{" "\"IN\": [{" " \"type\": \"MasterFiles\"," @@ -750,17 +745,17 @@ updateInMemory(AuthSrv& server, DataSourceConfigurator& datasrc_configurator, " \"type\": \"static\"," " \"params\": \"" + string(STATIC_DSRC_FILE) + "\"" "}]}")); - datasrc_configurator.reconfigure(server, config); + configureDataSource(server, config); } void -updateBuiltin(AuthSrv& server, DataSourceConfigurator& datasrc_configurator) { +updateBuiltin(AuthSrv& server) { const ConstElementPtr config(Element::fromJSON("{" "\"CH\": [{" " \"type\": \"static\"," " \"params\": \"" + string(STATIC_DSRC_FILE) + "\"" "}]}")); - datasrc_configurator.reconfigure(server, config); + configureDataSource(server, config); } // Try giving the server a TSIG signed request and see it can anwer signed as @@ -771,7 +766,7 @@ TEST_F(AuthSrvTest, DISABLED_TSIGSigned) { // Needs builtin TEST_F(AuthSrvTest, TSIGSigned) { #endif // Prepare key, the client message, etc - updateBuiltin(server, datasrc_configurator_); + updateBuiltin(server); const TSIGKey key("key:c2VjcmV0Cg==:hmac-sha1"); TSIGContext context(key); UnitTestUtil::createRequestMessage(request_message, opcode, default_qid, @@ -819,7 +814,7 @@ TEST_F(AuthSrvTest, DISABLED_builtInQueryViaDNSServer) { #else TEST_F(AuthSrvTest, builtInQueryViaDNSServer) { #endif - updateBuiltin(server, datasrc_configurator_); + updateBuiltin(server); UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(), default_qid, Name("VERSION.BIND."), RRClass::CH(), RRType::TXT()); @@ -851,7 +846,7 @@ TEST_F(AuthSrvTest, DISABLED_builtInQuery) { #else TEST_F(AuthSrvTest, builtInQuery) { #endif - updateBuiltin(server, datasrc_configurator_); + updateBuiltin(server); UnitTestUtil::createRequestMessage(request_message, Opcode::QUERY(), default_qid, Name("VERSION.BIND."), RRClass::CH(), RRType::TXT()); @@ -872,7 +867,7 @@ TEST_F(AuthSrvTest, DISABLED_iqueryViaDNSServer) { // Needs builtin #else TEST_F(AuthSrvTest, iqueryViaDNSServer) { // Needs builtin #endif - updateBuiltin(server, datasrc_configurator_); + updateBuiltin(server); createDataFromFile("iquery_fromWire.wire"); (*server.getDNSLookupProvider())(*io_message, parse_message, response_message, @@ -894,7 +889,7 @@ TEST_F(AuthSrvTest, DISABLED_updateConfig) { #else TEST_F(AuthSrvTest, updateConfig) { #endif - updateDatabase(server, datasrc_configurator_, 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 @@ -912,7 +907,7 @@ TEST_F(AuthSrvTest, DISABLED_datasourceFail) { #else TEST_F(AuthSrvTest, datasourceFail) { #endif - updateDatabase(server, datasrc_configurator_, 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 @@ -932,11 +927,10 @@ TEST_F(AuthSrvTest, DISABLED_updateConfigFail) { TEST_F(AuthSrvTest, updateConfigFail) { #endif // First, load a valid data source. - updateDatabase(server, datasrc_configurator_, CONFIG_TESTDB); + updateDatabase(server, CONFIG_TESTDB); // Next, try to update it with a non-existent one. This should fail. - EXPECT_THROW(updateDatabase(server, datasrc_configurator_, - BADCONFIG_TESTDB), + EXPECT_THROW(updateDatabase(server, BADCONFIG_TESTDB), isc::datasrc::DataSourceError); // The original data source should still exist. @@ -959,7 +953,7 @@ TEST_F(AuthSrvTest, updateWithInMemoryClient) { " \"params\": {}," " \"cache-enable\": true" "}]}")); - datasrc_configurator_.reconfigure(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. @@ -980,8 +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, datasrc_configurator_, "example.", - CONFIG_INMEMORY_EXAMPLE); + updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE); createDataFromFile("nsec3query_nodnssec_fromWire.wire"); server.processMessage(*io_message, *parse_message, *response_obuffer, @@ -1000,8 +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, datasrc_configurator_, "example.", - CONFIG_INMEMORY_EXAMPLE); + updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE); createDataFromFile("nsec3query_fromWire.wire"); server.processMessage(*io_message, *parse_message, *response_obuffer, @@ -1021,8 +1013,7 @@ TEST_F(AuthSrvTest, ) { // Set up the in-memory - updateInMemory(server, datasrc_configurator_, "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(), @@ -1434,8 +1425,7 @@ TEST_F(AuthSrvTest, ) { // Set real inmem client to proxy - updateInMemory(server, datasrc_configurator_, "example.", - CONFIG_INMEMORY_EXAMPLE); + updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE); { isc::util::thread::Mutex::Locker locker(server.getClientListMutex()); boost::shared_ptr @@ -1460,12 +1450,10 @@ 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, DataSourceConfigurator& datasrc_configurator, - ThrowWhen throw_when, bool isc_exception, +setupThrow(AuthSrv& server, ThrowWhen throw_when, bool isc_exception, ConstRRsetPtr rrset = ConstRRsetPtr()) { - updateInMemory(server, datasrc_configurator, "example.", - CONFIG_INMEMORY_EXAMPLE); + updateInMemory(server, "example.", CONFIG_INMEMORY_EXAMPLE); isc::util::thread::Mutex::Locker locker(server.getClientListMutex()); boost::shared_ptr @@ -1494,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, datasrc_configurator_, *when, true); + setupThrow(server, *when, true); processAndCheckSERVFAIL(); // To be sure, check same for non-isc-exceptions createRequestPacket(request_message, IPPROTO_UDP); - setupThrow(server, datasrc_configurator_, *when, false); + setupThrow(server, *when, false); processAndCheckSERVFAIL(); } } @@ -1514,7 +1502,7 @@ TEST_F(AuthSrvTest, ) { createDataFromFile("nsec3query_nodnssec_fromWire.wire"); - setupThrow(server, datasrc_configurator_, 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, @@ -1538,7 +1526,7 @@ TEST_F(AuthSrvTest, ConstRRsetPtr empty_rrset(new RRset(Name("foo.example"), RRClass::IN(), RRType::TXT(), RRTTL(0))); - setupThrow(server, datasrc_configurator_, 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 68e1b12e3b..c7465263ea 100644 --- a/src/bin/auth/tests/command_unittest.cc +++ b/src/bin/auth/tests/command_unittest.cc @@ -77,7 +77,6 @@ protected: MockXfroutClient xfrout_; MockSocketSessionForwarder ddns_forwarder_; AuthSrv server_; - DataSourceConfigurator datasrc_configurator_; ConstElementPtr result_; // The shutdown command parameter ConstElementPtr param_; @@ -191,7 +190,7 @@ zoneChecks(AuthSrv& server) { } void -configureZones(AuthSrv& server, DataSourceConfigurator& datasrc_configurator) { +configureZones(AuthSrv& server) { ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR "/test1.zone.in " TEST_DATA_BUILDDIR "/test1.zone.copied")); ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR "/test2.zone.in " @@ -209,7 +208,7 @@ configureZones(AuthSrv& server, DataSourceConfigurator& datasrc_configurator) { " \"cache-enable\": true" "}]}")); - datasrc_configurator.reconfigure(server, config); + configureDataSource(server, config); zoneChecks(server); } @@ -235,7 +234,7 @@ newZoneChecks(AuthSrv& server) { } TEST_F(AuthCommandTest, loadZone) { - configureZones(server_, datasrc_configurator_); + configureZones(server_); ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR "/test1-new.zone.in " @@ -272,7 +271,7 @@ TEST_F(AuthCommandTest, " \"cache-enable\": true," " \"cache-zones\": [\"example.org\"]" "}]}")); - datasrc_configurator_.reconfigure(server_, config); + configureDataSource(server_, config); { isc::util::thread::Mutex::Locker locker(server_.getClientListMutex()); @@ -336,7 +335,7 @@ TEST_F(AuthCommandTest, " \"cache-enable\": true," " \"cache-zones\": [\"example.com\"]" "}]}")); - EXPECT_THROW(datasrc_configurator_.reconfigure(server_, config2), + EXPECT_THROW(configureDataSource(server_, config2), ConfigurableClientList::ConfigurationError); result_ = execAuthServerCommand(server_, "loadzone", @@ -351,7 +350,7 @@ TEST_F(AuthCommandTest, } TEST_F(AuthCommandTest, loadBrokenZone) { - configureZones(server_, datasrc_configurator_); + configureZones(server_); ASSERT_EQ(0, system(INSTALL_PROG " -c " TEST_DATA_DIR "/test1-broken.zone.in " @@ -364,7 +363,7 @@ TEST_F(AuthCommandTest, loadBrokenZone) { } TEST_F(AuthCommandTest, loadUnreadableZone) { - configureZones(server_, datasrc_configurator_); + configureZones(server_); // install the zone file as unreadable ASSERT_EQ(0, system(INSTALL_PROG " -c -m 000 " TEST_DATA_DIR @@ -387,7 +386,7 @@ TEST_F(AuthCommandTest, loadZoneWithoutDataSrc) { } TEST_F(AuthCommandTest, loadZoneInvalidParams) { - configureZones(server_, datasrc_configurator_); + configureZones(server_); // null arg result_ = execAuthServerCommand(server_, "loadzone", ElementPtr()); diff --git a/src/bin/auth/tests/datasrc_configurator_unittest.cc b/src/bin/auth/tests/datasrc_configurator_unittest.cc index 3821dd4429..f294536475 100644 --- a/src/bin/auth/tests/datasrc_configurator_unittest.cc +++ b/src/bin/auth/tests/datasrc_configurator_unittest.cc @@ -60,19 +60,23 @@ 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(DatasrcConfiguratorTest& 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); +} void -datasrcConfigHandler(DatasrcConfiguratorTest* fake_server, - Configurator* configurator, const std::string&, +datasrcConfigHandler(DatasrcConfiguratorTest* fake_server, const std::string&, isc::data::ConstElementPtr config, const isc::config::ConfigData&) { if (config->contains("classes")) { - configurator->reconfigure(*fake_server, config->get("classes")); + testConfigureDataSource(*fake_server, config->get("classes")); } } @@ -113,28 +117,24 @@ protected: false)); } void TearDown() { - // Make sure no matter what we did, it is cleaned up. + // 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))); - } + session.getMessages()->add(createAnswer(0, + ElementPtr(new MapElement))); mccs->addRemoteConfig("data_sources", boost::bind(datasrcConfigHandler, - this, &configurator_, - _1, _2, _3), false); - } - void SetUp() { - init(); + this, _1, _2, _3), false); } ElementPtr buildConfig(const string& config) const { const ElementPtr internal(Element::fromJSON(config)); @@ -155,31 +155,12 @@ protected: } FakeSession session; auto_ptr mccs; - Configurator configurator_; const string specfile; std::map lists_; string log_; mutable isc::util::thread::Mutex mutex_; }; -// Check the initialization (and cleanup) -TEST_F(DatasrcConfiguratorTest, DISABLED_initialization) { - // It can't be initialized again - EXPECT_THROW(init(), InvalidOperation); - EXPECT_TRUE(session.haveSubscription("data_sources", "*")); - EXPECT_FALSE(session.haveSubscription("data_sources", "*")); - // We can't reconfigure now (not even manually) - EXPECT_THROW(configurator_.reconfigure(*this, - ElementPtr(new MapElement())), - InvalidOperation); - // If the server param is NULL, it does not work - EXPECT_THROW(Configurator configurator, InvalidParameter); - EXPECT_FALSE(session.haveSubscription("data_sources", "*")); // TBD - // 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) { initializeINList(); @@ -283,7 +264,7 @@ TEST_F(DatasrcConfiguratorTest, rollbackDeletion) { const ElementPtr config1(Element::fromJSON("{\"IN\": [{\"type\": \"yyy\"}], " "\"CH\": [{\"type\": \"xxx\"}]}")); - configurator_.reconfigure(*this, config1); + testConfigureDataSource(*this, config1); const ElementPtr config2(Element::fromJSON("{\"IN\": [{\"type\": 13}]}")); // This would delete CH. However, the IN one fails. @@ -291,7 +272,7 @@ 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(*this, config2), TypeError); + EXPECT_THROW(testConfigureDataSource(*this, config2), TypeError); EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf()); EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf()); } @@ -304,13 +285,13 @@ TEST_F(DatasrcConfiguratorTest, rollbackConfiguration) { const ElementPtr config1(Element::fromJSON("{\"IN\": [{\"type\": \"yyy\"}], " "\"CH\": [{\"type\": \"xxx\"}]}")); - configurator_.reconfigure(*this, 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(*this, config2), TypeError); + EXPECT_THROW(testConfigureDataSource(*this, config2), TypeError); EXPECT_EQ("yyy", lists_[RRClass::IN()]->getConf()); EXPECT_EQ("xxx", lists_[RRClass::CH()]->getConf()); }