[svn] commit: r3338 - in /branches/vorner-recursor-config/src/bin/recurse: main.cc recurse.spec.pre.in recursor.cc recursor.h tests/recursor_unittest.cc
BIND 10 source code commits
bind10-changes at lists.isc.org
Sat Oct 23 15:45:47 UTC 2010
Author: vorner
Date: Sat Oct 23 15:45:47 2010
New Revision: 3338
Log:
Configuration of addresses to listen on
Modified:
branches/vorner-recursor-config/src/bin/recurse/main.cc
branches/vorner-recursor-config/src/bin/recurse/recurse.spec.pre.in
branches/vorner-recursor-config/src/bin/recurse/recursor.cc
branches/vorner-recursor-config/src/bin/recurse/recursor.h
branches/vorner-recursor-config/src/bin/recurse/tests/recursor_unittest.cc
Modified: branches/vorner-recursor-config/src/bin/recurse/main.cc
==============================================================================
--- branches/vorner-recursor-config/src/bin/recurse/main.cc (original)
+++ branches/vorner-recursor-config/src/bin/recurse/main.cc Sat Oct 23 15:45:47 2010
@@ -106,30 +106,10 @@
int
main(int argc, char* argv[]) {
int ch;
- const char* port = DNSPORT;
- const char* address = NULL;
const char* uid = NULL;
- bool use_ipv4 = true, use_ipv6 = true;
- while ((ch = getopt(argc, argv, "46a:p:u:v")) != -1) {
+ while ((ch = getopt(argc, argv, "u:v")) != -1) {
switch (ch) {
- case '4':
- // Note that -4 means "ipv4 only", we need to set "use_ipv6" here,
- // not "use_ipv4". We could use something like "ipv4_only", but
- // we found the negatively named variable could confuse the code
- // logic.
- use_ipv6 = false;
- break;
- case '6':
- // The same note as -4 applies.
- use_ipv4 = false;
- break;
- case 'a':
- address = optarg;
- break;
- case 'p':
- port = optarg;
- break;
case 'u':
uid = optarg;
break;
@@ -143,18 +123,6 @@
}
if (argc - optind > 0) {
- usage();
- }
-
- if (!use_ipv4 && !use_ipv6) {
- cerr << "[b10-auth] Error: Cannot specify both -4 and -6 "
- << "at the same time" << endl;
- usage();
- }
-
- if ((!use_ipv4 || !use_ipv6) && address != NULL) {
- cerr << "[b10-auth] Error: Cannot specify -4 or -6 "
- << "at the same time as -a" << endl;
usage();
}
@@ -180,19 +148,7 @@
DNSLookup* lookup = recursor->getDNSLookupProvider();
DNSAnswer* answer = recursor->getDNSAnswerProvider();
- if (address != NULL) {
- // XXX: we can only specify at most one explicit address.
- // This also means the server cannot run in the dual address
- // family mode if explicit addresses need to be specified.
- // We don't bother to fix this problem, however. The -a option
- // is a short term workaround until we support dynamic listening
- // port allocation.
- io_service = new IOService(*port, *address,
- checkin, lookup, answer);
- } else {
- io_service = new IOService(*port, use_ipv4, use_ipv6,
- checkin, lookup, answer);
- }
+ io_service = new IOService(checkin, lookup, answer);
recursor->setIOService(*io_service);
cout << "[b10-recurse] IOService created." << endl;
@@ -204,6 +160,7 @@
my_command_handler);
cout << "[b10-recurse] Configuration channel established." << endl;
+ // FIXME: This does not belong here, but inside Boss
if (uid != NULL) {
changeUser(uid);
}
Modified: branches/vorner-recursor-config/src/bin/recurse/recurse.spec.pre.in
==============================================================================
--- branches/vorner-recursor-config/src/bin/recurse/recurse.spec.pre.in (original)
+++ branches/vorner-recursor-config/src/bin/recurse/recurse.spec.pre.in Sat Oct 23 15:45:47 2010
@@ -37,7 +37,11 @@
{
"address": "::1",
"port": 5300
- }
+ },
+ {
+ "address": "127.0.0.1",
+ "port": 5300
+ },
],
"list_item_spec": {
"item_name": "address",
Modified: branches/vorner-recursor-config/src/bin/recurse/recursor.cc
==============================================================================
--- branches/vorner-recursor-config/src/bin/recurse/recursor.cc (original)
+++ branches/vorner-recursor-config/src/bin/recurse/recursor.cc Sat Oct 23 15:45:47 2010
@@ -58,6 +58,8 @@
using namespace isc::xfr;
using namespace asiolink;
+typedef pair<string, uint16_t> addr_t;
+
class RecursorImpl {
private:
// prohibit copy
@@ -83,15 +85,15 @@
rec_query_ = NULL;
}
- void setForwardAddresses(const vector<pair<string, uint16_t> >& upstream,
+ void setForwardAddresses(const vector<addr_t>& upstream,
IOService* ios)
{
queryShutdown();
upstream_ = upstream;
if (ios) {
if (upstream_.empty()) {
- cerr << "[b10-recurse] Asked to do full recursive." << endl <<
- ", but not implemented yet. I'll do nothing." << endl;
+ cerr << "[b10-recurse] Asked to do full recursive," << endl <<
+ "but not implemented yet. I'll do nothing." << endl;
} else {
querySetup(*ios);
}
@@ -109,7 +111,7 @@
ModuleCCSession* config_session_;
bool verbose_mode_;
/// Addresses of the forward nameserver
- vector<pair<string, uint16_t> > upstream_;
+ vector<addr_t> upstream_, listen_;
private:
@@ -447,6 +449,46 @@
rec_query_->sendQuery(question, buffer, server);
}
+namespace {
+
+vector<addr_t>
+parseAddresses(ConstElementPtr addresses) {
+ vector<addr_t> result;
+ if (addresses) {
+ if (addresses->getType() == Element::list) {
+ for (size_t i(0); i < addresses->size(); ++ i) {
+ ConstElementPtr addrPair(addresses->get(i));
+ ConstElementPtr addr(addrPair->get("address"));
+ ConstElementPtr port(addrPair->get("port"));
+ if (!addr || ! port) {
+ isc_throw(BadValue, "Address must contain both the IP"
+ "address and port");
+ }
+ try {
+ IOAddress(addr->stringValue());
+ if (port->intValue() < 0 ||
+ port->intValue() > 0xffff) {
+ isc_throw(BadValue, "Bad port value (" <<
+ port->intValue() << ")");
+ }
+ result.push_back(addr_t(addr->stringValue(),
+ port->intValue()));
+ }
+ catch (const TypeError &e) { // Better error message
+ isc_throw(TypeError,
+ "Address must be a string and port an integer");
+ }
+ }
+ } else if (addresses->getType() != Element::null) {
+ isc_throw(TypeError,
+ "forward_addresses config element must be a list");
+ }
+ }
+ return (result);
+}
+
+}
+
ConstElementPtr
Recursor::updateConfig(ConstElementPtr config) {
if (impl_->verbose_mode_) {
@@ -454,41 +496,17 @@
}
try {
// Parse forward_addresses
- vector<pair<string, uint16_t> > addresses;
- ConstElementPtr forwardAddresses(config->get("forward_addresses"));
- if (forwardAddresses) {
- if (forwardAddresses->getType() == Element::list) {
- for (size_t i(0); i < forwardAddresses->size(); ++ i) {
- ConstElementPtr addrPair(forwardAddresses->get(i));
- ConstElementPtr addr(addrPair->get("address"));
- ConstElementPtr port(addrPair->get("port"));
- if (!addr || ! port) {
- isc_throw(BadValue, "Address must contain both the IP"
- "address and port");
- }
- try {
- IOAddress(addr->stringValue());
- if (port->intValue() < 0 ||
- port->intValue() > 0xffff) {
- isc_throw(BadValue, "Bad port value (" <<
- port->intValue() << ")");
- }
- addresses.push_back(pair<string, uint16_t>(
- addr->stringValue(), port->intValue()));
- }
- catch (const TypeError &e) { // Better error message
- isc_throw(TypeError,
- "Address must be a string and port an integer");
- }
- }
- } else if (forwardAddresses->getType() != Element::null) {
- isc_throw(TypeError,
- "forward_addresses config element must be a list");
- }
- }
+ ConstElementPtr forwardAddressesE(config->get("forward_addresses"));
+ vector<addr_t> forwardAddresses(parseAddresses(forwardAddressesE));
+ ConstElementPtr listenAddressesE(config->get("listen_addresses"));
+ vector<addr_t> listenAddresses(parseAddresses(listenAddressesE));
// Everything OK, so commit the changes
- if (forwardAddresses) {
- setForwardAddresses(addresses);
+ // listenAddresses can fail to bind, so try them first
+ if (listenAddressesE) {
+ setListenAddresses(listenAddresses);
+ }
+ if (forwardAddressesE) {
+ setForwardAddresses(forwardAddresses);
}
return (isc::config::createAnswer());
} catch (const isc::Exception& error) {
@@ -500,8 +518,7 @@
}
void
-Recursor::setForwardAddresses(const vector<pair<string, uint16_t> >&
- addresses)
+Recursor::setForwardAddresses(const vector<addr_t>& addresses)
{
impl_->setForwardAddresses(addresses, io_);
}
@@ -511,6 +528,46 @@
return (!impl_->upstream_.empty());
}
-vector<pair<string, uint16_t> > Recursor::getForwardAddresses() const {
+vector<addr_t>
+Recursor::getForwardAddresses() const {
return (impl_->upstream_);
}
+
+namespace {
+
+void
+setAddresses(IOService *service, const vector<addr_t>&
+ addresses)
+{
+ service->clearServers();
+ BOOST_FOREACH(const addr_t &address, addresses) {
+ service->addServer(address.second, address.first);
+ }
+}
+
+}
+
+void
+Recursor::setListenAddresses(const vector<addr_t>&
+ addresses)
+{
+ try {
+ setAddresses(io_, addresses);
+ impl_->listen_ = addresses;
+ }
+ catch (const exception &e) {
+ /*
+ * We couldn't set it. So return it back. If that fails as well,
+ * we have a problem.
+ *
+ * FIXME: What to do in that case? Directly abort?
+ */
+ setAddresses(io_, impl_->listen_);
+ throw e;// Let it fly a little bit further
+ }
+}
+
+vector<addr_t>
+Recursor::getListenAddresses() const {
+ return (impl_->listen_);
+}
Modified: branches/vorner-recursor-config/src/bin/recurse/recursor.h
==============================================================================
--- branches/vorner-recursor-config/src/bin/recurse/recursor.h (original)
+++ branches/vorner-recursor-config/src/bin/recurse/recursor.h Sat Oct 23 15:45:47 2010
@@ -111,6 +111,13 @@
/// Return if we are in forwarding mode (if not, we are in fully recursive)
bool isForwarding() const;
+ /**
+ * Set and get the addresses we listen on.
+ */
+ void setListenAddresses(const std::vector<std::pair<std::string,
+ uint16_t> >& addresses);
+ std::vector<std::pair<std::string, uint16_t> > getListenAddresses() const;
+
private:
RecursorImpl* impl_;
asiolink::IOService* io_;
Modified: branches/vorner-recursor-config/src/bin/recurse/tests/recursor_unittest.cc
==============================================================================
--- branches/vorner-recursor-config/src/bin/recurse/tests/recursor_unittest.cc (original)
+++ branches/vorner-recursor-config/src/bin/recurse/tests/recursor_unittest.cc Sat Oct 23 15:45:47 2010
@@ -332,8 +332,14 @@
class RecursorConfig : public ::testing::Test {
public:
+ IOService service;
Recursor server;
- void invalidForwardTest(const string &JOSN);
+ RecursorConfig() :
+ service(NULL, NULL, NULL)
+ {
+ server.setIOService(service);
+ }
+ void invalidTest(const string &JOSN);
};
TEST_F(RecursorConfig, forwardAddresses) {
@@ -387,7 +393,7 @@
EXPECT_EQ(0, server.getForwardAddresses().size());
}
-void RecursorConfig::invalidForwardTest(const string &JOSN) {
+void RecursorConfig::invalidTest(const string &JOSN) {
ElementPtr config(Element::fromJSON(JOSN));
EXPECT_FALSE(server.updateConfig(config)->equals(
*isc::config::createAnswer())) << "Accepted config " << JOSN << endl;
@@ -395,27 +401,106 @@
TEST_F(RecursorConfig, invalidForwardAddresses) {
// Try torturing it with some invalid inputs
- invalidForwardTest("{"
+ invalidTest("{"
"\"forward_addresses\": \"error\""
"}");
- invalidForwardTest("{"
+ invalidTest("{"
"\"forward_addresses\": [{}]"
"}");
- invalidForwardTest("{"
+ invalidTest("{"
"\"forward_addresses\": [{"
" \"port\": 1.5,"
" \"address\": \"192.0.2.1\""
"}]}");
- invalidForwardTest("{"
+ invalidTest("{"
"\"forward_addresses\": [{"
" \"port\": -5,"
" \"address\": \"192.0.2.1\""
"}]}");
- invalidForwardTest("{"
+ invalidTest("{"
"\"forward_addresses\": [{"
" \"port\": 53,"
" \"address\": \"bad_address\""
"}]}");
}
-}
+TEST_F(RecursorConfig, listenAddresses) {
+ // Default value should be fully recursive
+ EXPECT_TRUE(server.getListenAddresses().empty());
+
+ // Try putting there some addresses
+ vector<pair<string, uint16_t> > addresses;
+ addresses.push_back(pair<string, uint16_t>("127.0.0.1", 5300));
+ addresses.push_back(pair<string, uint16_t>("::1", 5300));
+ server.setListenAddresses(addresses);
+ EXPECT_EQ(2, server.getListenAddresses().size());
+ EXPECT_EQ("::1", server.getListenAddresses()[1].first);
+
+ // Is it independent from what we do with the vector later?
+ addresses.clear();
+ EXPECT_EQ(2, server.getListenAddresses().size());
+
+ // Did it return to fully recursive?
+ server.setListenAddresses(addresses);
+ EXPECT_TRUE(server.getListenAddresses().empty());
+}
+
+TEST_F(RecursorConfig, listenAddressConfig) {
+ // Try putting there some address
+ ElementPtr config(Element::fromJSON("{"
+ "\"listen_addresses\": ["
+ " {"
+ " \"address\": \"127.0.0.1\","
+ " \"port\": 5300"
+ " }"
+ "]"
+ "}"));
+ ConstElementPtr result(server.updateConfig(config));
+ EXPECT_EQ(result->toWire(), isc::config::createAnswer()->toWire());
+ ASSERT_EQ(1, server.getListenAddresses().size());
+ EXPECT_EQ("127.0.0.1", server.getListenAddresses()[0].first);
+ EXPECT_EQ(5300, server.getListenAddresses()[0].second);
+
+ // As this is example address, the machine should not have it on
+ // any interface
+ config = Element::fromJSON("{"
+ "\"listen_addresses\": ["
+ " {"
+ " \"address\": \"192.0.2.0\","
+ " \"port\": 5300"
+ " }"
+ "]"
+ "}");
+ result = server.updateConfig(config);
+ EXPECT_FALSE(result->equals(*isc::config::createAnswer()));
+ ASSERT_EQ(1, server.getListenAddresses().size());
+ EXPECT_EQ("127.0.0.1", server.getListenAddresses()[0].first);
+ EXPECT_EQ(5300, server.getListenAddresses()[0].second);
+}
+
+TEST_F(RecursorConfig, invalidListenAddresses) {
+ // Try torturing it with some invalid inputs
+ invalidTest("{"
+ "\"listen_addresses\": \"error\""
+ "}");
+ invalidTest("{"
+ "\"listen_addresses\": [{}]"
+ "}");
+ invalidTest("{"
+ "\"listen_addresses\": [{"
+ " \"port\": 1.5,"
+ " \"address\": \"192.0.2.1\""
+ "}]}");
+ invalidTest("{"
+ "\"listen_addresses\": [{"
+ " \"port\": -5,"
+ " \"address\": \"192.0.2.1\""
+ "}]}");
+ invalidTest("{"
+ "\"listen_addresses\": [{"
+ " \"port\": 53,"
+ " \"address\": \"bad_address\""
+ "}]}");
+}
+
+}
More information about the bind10-changes
mailing list