diff --git a/src/bin/auth/auth_srv.cc b/src/bin/auth/auth_srv.cc index 654c732735..afae60018d 100644 --- a/src/bin/auth/auth_srv.cc +++ b/src/bin/auth/auth_srv.cc @@ -47,6 +47,8 @@ #include #include +#include + #include #include #include @@ -837,11 +839,14 @@ AuthSrv::getListenAddresses() const { void AuthSrv::setListenAddresses(const AddressList& addresses) { - installListenAddresses(addresses, impl_->listen_addresses_, *dnss_); + // For UDP servers we specify the "SYNC_OK" option because in our usage + // it can act in the synchronous mode. + installListenAddresses(addresses, impl_->listen_addresses_, *dnss_, + DNSService::SERVER_SYNC_OK); } void -AuthSrv::setDNSService(isc::asiodns::DNSService& dnss) { +AuthSrv::setDNSService(isc::asiodns::DNSServiceBase& dnss) { dnss_ = &dnss; } diff --git a/src/bin/auth/auth_srv.h b/src/bin/auth/auth_srv.h index 382bda75b6..32b53dd763 100644 --- a/src/bin/auth/auth_srv.h +++ b/src/bin/auth/auth_srv.h @@ -385,7 +385,7 @@ public: const; /// \brief Assign an ASIO DNS Service queue to this Auth object - void setDNSService(isc::asiodns::DNSService& dnss); + void setDNSService(isc::asiodns::DNSServiceBase& dnss); /// \brief Sets the keyring used for verifying and signing /// @@ -401,7 +401,7 @@ private: isc::asiolink::SimpleCallback* checkin_; isc::asiodns::DNSLookup* dns_lookup_; isc::asiodns::DNSAnswer* dns_answer_; - isc::asiodns::DNSService* dnss_; + isc::asiodns::DNSServiceBase* dnss_; }; #endif // __AUTH_SRV_H diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc index dd2a36fa41..977b495471 100644 --- a/src/bin/auth/tests/auth_srv_unittest.cc +++ b/src/bin/auth/tests/auth_srv_unittest.cc @@ -41,6 +41,7 @@ #include #include #include +#include #include #include @@ -75,7 +76,7 @@ const char* const CONFIG_INMEMORY_EXAMPLE = class AuthSrvTest : public SrvTestBase { protected: AuthSrvTest() : - dnss_(ios_, NULL, NULL, NULL), + dnss_(), server(true, xfrout), rrclass(RRClass::IN()), // The empty string is expected value of the parameter of @@ -135,8 +136,7 @@ protected: opcode.getCode(), QR_FLAG, 1, 0, 0, 0); } - IOService ios_; - DNSService dnss_; + MockDNSService dnss_; MockSession statistics_session; MockXfroutClient xfrout; AuthSrv server; diff --git a/src/bin/auth/tests/config_unittest.cc b/src/bin/auth/tests/config_unittest.cc index 78245e6a84..fb5067e792 100644 --- a/src/bin/auth/tests/config_unittest.cc +++ b/src/bin/auth/tests/config_unittest.cc @@ -37,14 +37,13 @@ using namespace isc::dns; using namespace isc::data; using namespace isc::datasrc; using namespace isc::asiodns; -using namespace isc::asiolink; using namespace isc::testutils; namespace { class AuthConfigTest : public ::testing::Test { protected: AuthConfigTest() : - dnss_(ios_, NULL, NULL, NULL), + dnss_(), rrclass(RRClass::IN()), server(true, xfrout), // The empty string is expected value of the parameter of @@ -54,8 +53,7 @@ protected: { server.setDNSService(dnss_); } - IOService ios_; - DNSService dnss_; + MockDNSService dnss_; const RRClass rrclass; MockXfroutClient xfrout; AuthSrv server; @@ -147,6 +145,14 @@ TEST_F(AuthConfigTest, invalidListenAddressConfig) { // Try setting addresses trough config TEST_F(AuthConfigTest, listenAddressConfig) { isc::testutils::portconfig::listenAddressConfig(server); + + // listenAddressConfig should have attempted to create 4 DNS server + // objects: two IP addresses, TCP and UDP for each. For UDP, the "SYNC_OK" + // option should have been specified. + EXPECT_EQ(2, dnss_.getTCPFdParams().size()); + EXPECT_EQ(2, dnss_.getUDPFdParams().size()); + EXPECT_EQ(DNSService::SERVER_SYNC_OK, dnss_.getUDPFdParams().at(0).options); + EXPECT_EQ(DNSService::SERVER_SYNC_OK, dnss_.getUDPFdParams().at(1).options); } class MemoryDatasrcConfigTest : public AuthConfigTest { diff --git a/src/bin/resolver/resolver.cc b/src/bin/resolver/resolver.cc index 711379ed3c..9536608ed9 100644 --- a/src/bin/resolver/resolver.cc +++ b/src/bin/resolver/resolver.cc @@ -92,7 +92,7 @@ public: queryShutdown(); } - void querySetup(DNSService& dnss, + void querySetup(DNSServiceBase& dnss, isc::nsas::NameserverAddressStore& nsas, isc::cache::ResolverCache& cache) { @@ -121,10 +121,10 @@ public: } void setForwardAddresses(const AddressList& upstream, - DNSService *dnss) + DNSServiceBase* dnss) { upstream_ = upstream; - if (dnss) { + if (dnss != NULL) { if (!upstream_.empty()) { BOOST_FOREACH(const AddressPair& address, upstream) { LOG_INFO(resolver_logger, RESOLVER_FORWARD_ADDRESS) @@ -137,10 +137,10 @@ public: } void setRootAddresses(const AddressList& upstream_root, - DNSService *dnss) + DNSServiceBase* dnss) { upstream_root_ = upstream_root; - if (dnss) { + if (dnss != NULL) { if (!upstream_root_.empty()) { BOOST_FOREACH(const AddressPair& address, upstream_root) { LOG_INFO(resolver_logger, RESOLVER_SET_ROOT_ADDRESS) @@ -377,7 +377,7 @@ Resolver::~Resolver() { } void -Resolver::setDNSService(isc::asiodns::DNSService& dnss) { +Resolver::setDNSService(isc::asiodns::DNSServiceBase& dnss) { dnss_ = &dnss; } diff --git a/src/bin/resolver/resolver.h b/src/bin/resolver/resolver.h index 096f522a7f..e91192e931 100644 --- a/src/bin/resolver/resolver.h +++ b/src/bin/resolver/resolver.h @@ -104,7 +104,7 @@ public: bool startup = false); /// \brief Assign an ASIO IO Service queue to this Resolver object - void setDNSService(isc::asiodns::DNSService& dnss); + void setDNSService(isc::asiodns::DNSServiceBase& dnss); /// \brief Assign a NameserverAddressStore to this Resolver object void setNameserverAddressStore(isc::nsas::NameserverAddressStore &nsas); @@ -113,7 +113,7 @@ public: void setCache(isc::cache::ResolverCache& cache); /// \brief Return this object's ASIO IO Service queue - isc::asiodns::DNSService& getDNSService() const { return (*dnss_); } + isc::asiodns::DNSServiceBase& getDNSService() const { return (*dnss_); } /// \brief Returns this object's NSAS isc::nsas::NameserverAddressStore& getNameserverAddressStore() const { @@ -258,7 +258,7 @@ public: private: ResolverImpl* impl_; - isc::asiodns::DNSService* dnss_; + isc::asiodns::DNSServiceBase* dnss_; isc::asiolink::SimpleCallback* checkin_; isc::asiodns::DNSLookup* dns_lookup_; isc::asiodns::DNSAnswer* dns_answer_; diff --git a/src/bin/resolver/tests/resolver_config_unittest.cc b/src/bin/resolver/tests/resolver_config_unittest.cc index ea42ba000e..369ca5c6bb 100644 --- a/src/bin/resolver/tests/resolver_config_unittest.cc +++ b/src/bin/resolver/tests/resolver_config_unittest.cc @@ -48,6 +48,7 @@ #include #include +#include #include #include @@ -76,15 +77,13 @@ public: class ResolverConfig : public ::testing::Test { protected: - IOService ios; - DNSService dnss; + MockDNSService dnss; Resolver server; scoped_ptr endpoint; scoped_ptr query_message; scoped_ptr client; scoped_ptr request; ResolverConfig() : - dnss(ios, NULL, NULL, NULL), // The empty string is expected value of the parameter of // requestSocket, not the app_name (there's no fallback, it checks // the empty string is passed). @@ -320,6 +319,15 @@ TEST_F(ResolverConfig, invalidForwardAddresses) { // Try setting the addresses directly TEST_F(ResolverConfig, listenAddresses) { isc::testutils::portconfig::listenAddresses(server); + + // listenAddressConfig should have attempted to create 4 DNS server + // objects: two IP addresses, TCP and UDP for each. For UDP, the "SYNC_OK" + // option (or anything else) should have NOT been specified. + EXPECT_EQ(2, dnss.getTCPFdParams().size()); + EXPECT_EQ(2, dnss.getUDPFdParams().size()); + EXPECT_EQ(DNSService::SERVER_DEFAULT, dnss.getUDPFdParams().at(0).options); + EXPECT_EQ(DNSService::SERVER_DEFAULT, dnss.getUDPFdParams().at(1).options); + // Check it requests the correct addresses const char* tokens[] = { "TCP:127.0.0.1:53210:1", diff --git a/src/lib/asiodns/dns_service.h b/src/lib/asiodns/dns_service.h index 702f724408..865aee4f89 100644 --- a/src/lib/asiodns/dns_service.h +++ b/src/lib/asiodns/dns_service.h @@ -82,6 +82,8 @@ public: virtual void addServerUDPFromFD(int fd, int af, ServerFlag options = SERVER_DEFAULT) = 0; virtual void clearServers() = 0; + + virtual asiolink::IOService& getIOService() = 0; }; /// \brief Handle DNS Queries @@ -92,9 +94,6 @@ public: /// server implementations. As such, it handles asio, including config /// updates (through the 'Checkinprovider'), and listening sockets. class DNSService : public DNSServiceBase { -public: - using DNSServiceBase::ServerFlag; - /// /// \name Constructors and Destructor /// @@ -208,7 +207,7 @@ public: /// \brief Return the IO Service Object /// /// \return IOService object for this DNS service. - asiolink::IOService& getIOService() { return (io_service_);} + virtual asiolink::IOService& getIOService() { return (io_service_);} private: DNSServiceImpl* impl_; diff --git a/src/lib/resolve/recursive_query.cc b/src/lib/resolve/recursive_query.cc index 12b7c3ba4b..8d03c1c55d 100644 --- a/src/lib/resolve/recursive_query.cc +++ b/src/lib/resolve/recursive_query.cc @@ -128,7 +128,7 @@ typedef std::vector > AddressVector; // mishandles this in its name mangling, and wouldn't compile. // We can probably use a typedef, but need to move it to a central // location and use it consistently. -RecursiveQuery::RecursiveQuery(DNSService& dns_service, +RecursiveQuery::RecursiveQuery(DNSServiceBase& dns_service, isc::nsas::NameserverAddressStore& nsas, isc::cache::ResolverCache& cache, const std::vector >& upstream, diff --git a/src/lib/resolve/recursive_query.h b/src/lib/resolve/recursive_query.h index 9af2d72906..a819a94fa8 100644 --- a/src/lib/resolve/recursive_query.h +++ b/src/lib/resolve/recursive_query.h @@ -87,7 +87,7 @@ public: /// \param lookup_timeout Timeout value for when we give up, in ms /// \param retries how many times we try again (0 means just send and /// and return if it returs). - RecursiveQuery(DNSService& dns_service, + RecursiveQuery(DNSServiceBase& dns_service, isc::nsas::NameserverAddressStore& nsas, isc::cache::ResolverCache& cache, const std::vector >& @@ -178,7 +178,7 @@ public: void setTestServer(const std::string& address, uint16_t port); private: - DNSService& dns_service_; + DNSServiceBase& dns_service_; isc::nsas::NameserverAddressStore& nsas_; isc::cache::ResolverCache& cache_; boost::shared_ptr > > diff --git a/src/lib/server_common/portconfig.cc b/src/lib/server_common/portconfig.cc index 2797bd3d4e..171f560354 100644 --- a/src/lib/server_common/portconfig.cc +++ b/src/lib/server_common/portconfig.cc @@ -84,7 +84,9 @@ namespace { vector current_sockets; void -setAddresses(DNSServiceBase& service, const AddressList& addresses) { +setAddresses(DNSServiceBase& service, const AddressList& addresses, + DNSService::ServerFlag server_options) +{ service.clearServers(); BOOST_FOREACH(const string& token, current_sockets) { socketRequestor().releaseSocket(token); @@ -99,35 +101,32 @@ setAddresses(DNSServiceBase& service, const AddressList& addresses) { address.first, address.second, SocketRequestor::SHARE_SAME)); current_sockets.push_back(tcp.second); - if (!test_mode) { - service.addServerTCPFromFD(tcp.first, af); - } + service.addServerTCPFromFD(tcp.first, af); const SocketRequestor::SocketID udp(socketRequestor().requestSocket(SocketRequestor::UDP, address.first, address.second, SocketRequestor::SHARE_SAME)); current_sockets.push_back(udp.second); - if (!test_mode) { - service.addServerUDPFromFD(udp.first, af); - } + service.addServerUDPFromFD(udp.first, af, server_options); } } } void -installListenAddresses(const AddressList& newAddresses, - AddressList& addressStore, - isc::asiodns::DNSServiceBase& service) +installListenAddresses(const AddressList& new_addresses, + AddressList& address_store, + DNSServiceBase& service, + DNSService::ServerFlag server_options) { try { LOG_DEBUG(logger, DBG_TRACE_BASIC, SRVCOMM_SET_LISTEN); - BOOST_FOREACH(const AddressPair& addr, newAddresses) { + BOOST_FOREACH(const AddressPair& addr, new_addresses) { LOG_DEBUG(logger, DBG_TRACE_VALUES, SRVCOMM_ADDRESS_VALUE). arg(addr.first).arg(addr.second); } - setAddresses(service, newAddresses); - addressStore = newAddresses; + setAddresses(service, new_addresses, server_options); + address_store = new_addresses; } catch (const SocketRequestor::NonFatalSocketError& e) { /* * If one of the addresses isn't set successfully, we will restore @@ -144,14 +143,14 @@ installListenAddresses(const AddressList& newAddresses, */ LOG_ERROR(logger, SRVCOMM_ADDRESS_FAIL).arg(e.what()); try { - setAddresses(service, addressStore); + setAddresses(service, address_store, server_options); } catch (const SocketRequestor::NonFatalSocketError& e2) { LOG_FATAL(logger, SRVCOMM_ADDRESS_UNRECOVERABLE).arg(e2.what()); // If we can't set the new ones, nor the old ones, at least // releasing everything should work. If it doesn't, there isn't // anything else we could do. - setAddresses(service, AddressList()); - addressStore.clear(); + setAddresses(service, AddressList(), server_options); + address_store.clear(); } //Anyway the new configure has problem, we need to notify configure //manager the new configure doesn't work diff --git a/src/lib/server_common/portconfig.h b/src/lib/server_common/portconfig.h index bc36fbe1b9..0795728ce1 100644 --- a/src/lib/server_common/portconfig.h +++ b/src/lib/server_common/portconfig.h @@ -15,22 +15,15 @@ #ifndef ISC_SERVER_COMMON_PORTCONFIG_H #define ISC_SERVER_COMMON_PORTCONFIG_H +#include + +#include + #include #include #include #include -#include - -/* - * Some forward declarations. - */ -namespace isc { -namespace asiodns { -class DNSServiceBase; -} -} - namespace isc { namespace server_common { /** @@ -88,39 +81,42 @@ AddressList parseAddresses(isc::data::ConstElementPtr addresses, const std::string& elemName); -/** - * \brief Changes current listening addresses and ports. - * - * Removes all sockets we currently listen on and starts listening on the - * addresses and ports requested in newAddresses. - * - * If it fails to set up the new addresses, it attempts to roll back to the - * previous addresses (but it still propagates the exception). If the rollback - * fails as well, it doesn't abort the application (to allow reconfiguration), - * but removes all the sockets it listened on. One of the exceptions is - * propagated. - * - * The ports are requested from the socket creator through boss. Therefore - * you need to initialize the SocketRequestor before using this function. - * - * \param newAddresses are the addresses you want to listen on. - * \param addressStore is the place you store your current addresses. It is - * used when there's a need for rollback. The newAddresses are copied here - * when the change is successful. - * \param dnsService is the DNSService object we use now. The requests from - * the new sockets are handled using this dnsService (and all current - * sockets on the service are closed first). - * \throw asiolink::IOError when initialization or closing of socket fails. - * \throw isc::server_common::SocketRequestor::Socket error when the - * boss/socket creator doesn't want to give us the socket. - * \throw std::bad_alloc when allocation fails. - * \throw isc::InvalidOperation when the function is called and the - * SocketRequestor isn't initialized yet. - */ +/// \brief Changes current listening addresses and ports. +/// +/// Removes all sockets we currently listen on and starts listening on the +/// addresses and ports requested in new_addresses. +/// +/// If it fails to set up the new addresses, it attempts to roll back to the +/// previous addresses (but it still propagates the exception). If the rollback +/// fails as well, it doesn't abort the application (to allow reconfiguration), +/// but removes all the sockets it listened on. One of the exceptions is +/// propagated. +/// +/// The ports are requested from the socket creator through boss. Therefore +/// you need to initialize the SocketRequestor before using this function. +/// +/// \param new_addresses are the addresses you want to listen on. +/// \param address_store is the place you store your current addresses. It is +/// used when there's a need for rollback. The new_addresses are copied +/// here when the change is successful. +/// \param dns_service is the DNSService object we use now. The requests from +/// the new sockets are handled using this dns_service (and all current +/// sockets on the service are closed first). +/// \param server_options specifies optional properties for the servers +/// created via \c dns_service. +/// +/// \throw asiolink::IOError when initialization or closing of socket fails. +/// \throw isc::server_common::SocketRequestor::Socket error when the +/// boss/socket creator doesn't want to give us the socket. +/// \throw std::bad_alloc when allocation fails. +/// \throw isc::InvalidOperation when the function is called and the +/// SocketRequestor isn't initialized yet. void -installListenAddresses(const AddressList& newAddresses, - AddressList& addressStore, - asiodns::DNSServiceBase& dnsService); +installListenAddresses(const AddressList& new_addresses, + AddressList& address_store, + asiodns::DNSServiceBase& dns_service, + asiodns::DNSService::ServerFlag server_options = + asiodns::DNSService::SERVER_DEFAULT); } } diff --git a/src/lib/testutils/dnsmessage_test.h b/src/lib/testutils/dnsmessage_test.h index 8a46e0e23e..5c7401101e 100644 --- a/src/lib/testutils/dnsmessage_test.h +++ b/src/lib/testutils/dnsmessage_test.h @@ -12,6 +12,9 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. +#ifndef __ISC_TESTUTILS_DNSMESSAGETEST_H +#define __ISC_TESTUTILS_DNSMESSAGETEST_H 1 + #include #include #include @@ -339,6 +342,7 @@ rrsetsCheck(const std::string& expected, } // end of namespace testutils } // end of namespace isc +#endif // __ISC_TESTUTILS_DNSMESSAGETEST_H // Local Variables: // mode: c++ diff --git a/src/lib/testutils/mockups.h b/src/lib/testutils/mockups.h index 5f2b06251f..b5def4badb 100644 --- a/src/lib/testutils/mockups.h +++ b/src/lib/testutils/mockups.h @@ -12,8 +12,13 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. +#ifndef __ISC_TESTUTILS_MOCKUPS_H +#define __ISC_TESTUTILS_MOCKUPS_H 1 + #include +#include + #include #include @@ -21,6 +26,9 @@ #include +#include +#include + namespace isc { namespace testutils { @@ -96,6 +104,45 @@ private: bool receive_ok_; }; +// This mock object does nothing except for recording passed parameters +// to addServerXXX methods so the test code subsequently checks the parameters. +class MockDNSService : public isc::asiodns::DNSServiceBase { +public: + // A helper tuple of parameters passed to addServerUDPFromFD(). + struct UDPFdParams { + int fd; + int af; + ServerFlag options; + }; + + virtual void addServerTCPFromFD(int fd, int af) { + tcp_fd_params_.push_back(std::pair(fd, af)); + } + virtual void addServerUDPFromFD(int fd, int af, ServerFlag options) { + UDPFdParams params = { fd, af, options }; + udp_fd_params_.push_back(params); + } + virtual void clearServers() {} + + virtual asiolink::IOService& getIOService() { + isc_throw(isc::Unexpected, + "MockDNSService::getIOService() shouldn't be called"); + } + + // These two allow the tests to check how the servers have been created + // through this object. + const std::vector >& getTCPFdParams() const { + return (tcp_fd_params_); + } + const std::vector& getUDPFdParams() const { + return (udp_fd_params_); + } + +private: + std::vector > tcp_fd_params_; + std::vector udp_fd_params_; +}; + // A nonoperative DNSServer object to be used in calls to processMessage(). class MockServer : public isc::asiodns::DNSServer { public: @@ -154,6 +201,7 @@ private: } // end of testutils } // end of isc +#endif // __ISC_TESTUTILS_MOCKUPS_H // Local Variables: // mode: c++ diff --git a/src/lib/testutils/portconfig.h b/src/lib/testutils/portconfig.h index b538478deb..ce1bb39c7b 100644 --- a/src/lib/testutils/portconfig.h +++ b/src/lib/testutils/portconfig.h @@ -12,8 +12,8 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. -#ifndef TESTUTILS_PORTCONFIG_H -#define TESTUTILS_PORTCONFIG_H +#ifndef __ISC_TESTUTILS_PORTCONFIG_H +#define __ISC_TESTUTILS_PORTCONFIG_H #include #include @@ -186,4 +186,4 @@ invalidListenAddressConfig(Server& server) { } } -#endif +#endif // __ISC_TESTUTILS_PORTCONFIG_H diff --git a/src/lib/testutils/socket_request.h b/src/lib/testutils/socket_request.h index 2c14120379..f64b106222 100644 --- a/src/lib/testutils/socket_request.h +++ b/src/lib/testutils/socket_request.h @@ -12,6 +12,9 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. +#ifndef __ISC_TESTUTILS_SOCKETREQUEST_H +#define __ISC_TESTUTILS_SOCKETREQUEST_H 1 + #include #include @@ -64,7 +67,7 @@ public: /// not fall back to this value if its share_name is left empty, if /// you want to check the code relies on the requestor to use the /// app name, you set this to empty string. - TestSocketRequestor(asiodns::DNSService& dnss, + TestSocketRequestor(asiodns::DNSServiceBase& dnss, server_common::portconfig::AddressList& store, uint16_t expect_port, const std::string& expected_app) : @@ -216,7 +219,7 @@ public: } private: - asiodns::DNSService& dnss_; + asiodns::DNSServiceBase& dnss_; server_common::portconfig::AddressList& store_; const uint16_t expect_port_; const std::string expected_app_; @@ -224,3 +227,4 @@ private: } } +#endif // __ISC_TESTUTILS_SOCKETREQUEST_H diff --git a/src/lib/testutils/srv_test.h b/src/lib/testutils/srv_test.h index be32b55814..b5f64b5d6a 100644 --- a/src/lib/testutils/srv_test.h +++ b/src/lib/testutils/srv_test.h @@ -12,6 +12,9 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. +#ifndef __ISC_TESTUTILS_SRVTEST_H +#define __ISC_TESTUTILS_SRVTEST_H 1 + #include #include #include @@ -106,6 +109,7 @@ protected: }; } // end of namespace testutils } // end of namespace isc +#endif // __ISC_TESTUTILS_SRVTEST_H // Local Variables: // mode: c++