Commit c4e3eaef authored by JINMEI Tatuya's avatar JINMEI Tatuya
Browse files

[1784] make sure auth uses the synchronous DNS server mode.

also introduce mock DNSService and use it to test the behavior, and to test
the resolver behavior doesn't change.

the essential part should be quite simple; most of the changes are
straightforward interface adjustments.
it also includes some editorial cleanups such as adding header-file guards
or style guideline fixes.
parent c8c69b2a
......@@ -47,6 +47,8 @@
#include <dns/message.h>
#include <dns/tsig.h>
#include <asiodns/dns_service.h>
#include <datasrc/query.h>
#include <datasrc/data_source.h>
#include <datasrc/memory_datasrc.h>
......@@ -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;
}
......
......@@ -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
......
......@@ -41,6 +41,7 @@
#include <dns/tests/unittest_util.h>
#include <testutils/dnsmessage_test.h>
#include <testutils/srv_test.h>
#include <testutils/mockups.h>
#include <testutils/portconfig.h>
#include <testutils/socket_request.h>
......@@ -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;
......
......@@ -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 {
......
......@@ -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;
}
......
......@@ -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_;
......
......@@ -48,6 +48,7 @@
#include <dns/tests/unittest_util.h>
#include <testutils/srv_test.h>
#include <testutils/mockups.h>
#include <testutils/portconfig.h>
#include <testutils/socket_request.h>
......@@ -76,15 +77,13 @@ public:
class ResolverConfig : public ::testing::Test {
protected:
IOService ios;
DNSService dnss;
MockDNSService dnss;
Resolver server;
scoped_ptr<const IOEndpoint> endpoint;
scoped_ptr<const IOMessage> query_message;
scoped_ptr<const Client> client;
scoped_ptr<const RequestContext> 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",
......
......@@ -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_;
......
......@@ -128,7 +128,7 @@ typedef std::vector<std::pair<std::string, uint16_t> > 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<std::pair<std::string, uint16_t> >& upstream,
......
......@@ -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<std::pair<std::string, uint16_t> >&
......@@ -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<std::vector<std::pair<std::string, uint16_t> > >
......
......@@ -84,7 +84,9 @@ namespace {
vector<string> 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
......
......@@ -15,22 +15,15 @@
#ifndef ISC_SERVER_COMMON_PORTCONFIG_H
#define ISC_SERVER_COMMON_PORTCONFIG_H
#include <cc/data.h>
#include <asiodns/dns_service.h>
#include <utility>
#include <string>
#include <stdint.h>
#include <vector>
#include <cc/data.h>
/*
* 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);
}
}
......
......@@ -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 <algorithm>
#include <functional>
#include <iosfwd>
......@@ -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++
......
......@@ -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 <config.h>
#include <exceptions/exceptions.h>
#include <cc/data.h>
#include <cc/session.h>
......@@ -21,6 +26,9 @@
#include <asiodns/asiodns.h>
#include <utility>
#include <vector>
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<int, int>(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<std::pair<int, int> >& getTCPFdParams() const {
return (tcp_fd_params_);
}
const std::vector<UDPFdParams>& getUDPFdParams() const {
return (udp_fd_params_);
}
private:
std::vector<std::pair<int, int> > tcp_fd_params_;
std::vector<UDPFdParams> 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++
......
......@@ -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 <gtest/gtest.h>
#include <cc/data.h>
......@@ -186,4 +186,4 @@ invalidListenAddressConfig(Server& server) {
}
}
#endif
#endif // __ISC_TESTUTILS_PORTCONFIG_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 <server_common/socket_request.h>
#include <server_common/portconfig.h>
......@@ -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
......@@ -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 <util/buffer.h>
#include <dns/name.h>
#include <dns/message.h>
......@@ -106,6 +109,7 @@ protected:
};
} // end of namespace testutils
} // end of namespace isc
#endif // __ISC_TESTUTILS_SRVTEST_H
// Local Variables:
// mode: c++
......