Browse Source

Merge branches 'work/ports/auth' and 'work/ports/boss'

Michal 'vorner' Vaner 14 years ago
parent
commit
f06ce63887

+ 2 - 0
configure.ac

@@ -681,6 +681,8 @@ AC_CONFIG_FILES([Makefile
                  src/lib/nsas/tests/Makefile
                  src/lib/nsas/tests/Makefile
                  src/lib/cache/Makefile
                  src/lib/cache/Makefile
                  src/lib/cache/tests/Makefile
                  src/lib/cache/tests/Makefile
+                 src/lib/server_common/Makefile
+                 src/lib/server_common/tests/Makefile
                ])
                ])
 AC_OUTPUT([doc/version.ent
 AC_OUTPUT([doc/version.ent
            src/bin/cfgmgr/b10-cfgmgr.py
            src/bin/cfgmgr/b10-cfgmgr.py

+ 1 - 1
doc/Doxyfile

@@ -568,7 +568,7 @@ WARN_LOGFILE           =
 # directories like "/usr/src/myproject". Separate the files or directories
 # directories like "/usr/src/myproject". Separate the files or directories
 # with spaces.
 # with spaces.
 
 
-INPUT                  = ../src/lib/cc ../src/lib/config ../src/lib/dns ../src/lib/exceptions ../src/lib/datasrc ../src/bin/auth ../src/bin/resolver ../src/lib/bench ../src/lib/log ../src/lib/asiolink/ ../src/lib/nsas ../src/lib/testutils ../src/lib/cache
+INPUT                  = ../src/lib/cc ../src/lib/config ../src/lib/dns ../src/lib/exceptions ../src/lib/datasrc ../src/bin/auth ../src/bin/resolver ../src/lib/bench ../src/lib/log ../src/lib/asiolink/ ../src/lib/nsas ../src/lib/testutils ../src/lib/cache ../src/lib/server_common/
 
 
 # This tag can be used to specify the character encoding of the source files
 # This tag can be used to specify the character encoding of the source files
 # that doxygen parses. Internally doxygen uses the UTF-8 encoding, which is
 # that doxygen parses. Internally doxygen uses the UTF-8 encoding, which is

+ 1 - 0
src/bin/auth/Makefile.am

@@ -52,6 +52,7 @@ b10_auth_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 b10_auth_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
 b10_auth_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
 b10_auth_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la
 b10_auth_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la
 b10_auth_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la
 b10_auth_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la
+b10_auth_LDADD += $(top_builddir)/src/lib/server_common/libserver_common.la
 b10_auth_LDADD += $(SQLITE_LIBS)
 b10_auth_LDADD += $(SQLITE_LIBS)
 
 
 # TODO: config.h.in is wrong because doesn't honor pkgdatadir
 # TODO: config.h.in is wrong because doesn't honor pkgdatadir

+ 35 - 0
src/bin/auth/auth.spec.pre.in

@@ -56,6 +56,41 @@
         "item_type": "integer",
         "item_type": "integer",
         "item_optional": true,
         "item_optional": true,
         "item_default": 60
         "item_default": 60
+      },
+      {
+        "item_name": "listen_on",
+        "item_type": "list",
+        "item_optional": false,
+        "item_default": [
+          {
+            "address": "::1",
+            "port": 5300
+          },
+          {
+            "address": "127.0.0.1",
+            "port": 5300
+          }
+        ],
+        "list_item_spec": {
+          "item_name": "address",
+          "item_type": "map",
+          "item_optional": false,
+          "item_default": {},
+          "map_item_spec": [
+            {
+              "item_name": "address",
+              "item_type": "string",
+              "item_optional": false,
+              "item_default": "::1"
+            },
+            {
+              "item_name": "port",
+              "item_type": "integer",
+              "item_optional": false,
+              "item_default": 5300
+            }
+          ]
+        }
       }
       }
     ],
     ],
     "commands": [
     "commands": [

+ 19 - 0
src/bin/auth/auth_srv.cc

@@ -69,6 +69,7 @@ using namespace isc::data;
 using namespace isc::config;
 using namespace isc::config;
 using namespace isc::xfr;
 using namespace isc::xfr;
 using namespace asiolink;
 using namespace asiolink;
+using namespace isc::server_common::portconfig;
 
 
 class AuthSrvImpl {
 class AuthSrvImpl {
 private:
 private:
@@ -109,6 +110,9 @@ public:
 
 
     /// Query counters for statistics
     /// Query counters for statistics
     AuthCounters counters_;
     AuthCounters counters_;
+
+    /// Addresses we listen on
+    AddressList listen_addresses_;
 private:
 private:
     std::string db_file_;
     std::string db_file_;
 
 
@@ -750,3 +754,18 @@ uint64_t
 AuthSrv::getCounter(const AuthCounters::CounterType type) const {
 AuthSrv::getCounter(const AuthCounters::CounterType type) const {
     return (impl_->counters_.getCounter(type));
     return (impl_->counters_.getCounter(type));
 }
 }
+
+const AddressList&
+AuthSrv::getListenAddresses() const {
+    return (impl_->listen_addresses_);
+}
+
+void
+AuthSrv::setListenAddresses(const AddressList& addresses) {
+    installListenAddresses(addresses, impl_->listen_addresses_, *dnss_);
+}
+
+void
+AuthSrv::setDNSService(asiolink::DNSService& dnss) {
+    dnss_ = &dnss;
+}

+ 14 - 0
src/bin/auth/auth_srv.h

@@ -25,6 +25,7 @@
 #include <config/ccsession.h>
 #include <config/ccsession.h>
 
 
 #include <asiolink/asiolink.h>
 #include <asiolink/asiolink.h>
+#include <server_common/portconfig.h>
 #include <auth/statistics.h>
 #include <auth/statistics.h>
 
 
 namespace isc {
 namespace isc {
@@ -353,11 +354,24 @@ public:
     /// \return the value of the counter.
     /// \return the value of the counter.
     uint64_t getCounter(const AuthCounters::CounterType type) const;
     uint64_t getCounter(const AuthCounters::CounterType type) const;
 
 
+    /**
+     * \brief Set and get the addresses we listen on.
+     */
+    void setListenAddresses(const isc::server_common::portconfig::AddressList&
+                            addreses);
+    const isc::server_common::portconfig::AddressList& getListenAddresses()
+        const;
+
+    /// \brief Assign an ASIO DNS Service queue to this Auth object
+    void setDNSService(asiolink::DNSService& dnss);
+
+
 private:
 private:
     AuthSrvImpl* impl_;
     AuthSrvImpl* impl_;
     asiolink::SimpleCallback* checkin_;
     asiolink::SimpleCallback* checkin_;
     asiolink::DNSLookup* dns_lookup_;
     asiolink::DNSLookup* dns_lookup_;
     asiolink::DNSAnswer* dns_answer_;
     asiolink::DNSAnswer* dns_answer_;
+    asiolink::DNSService* dnss_;
 };
 };
 
 
 #endif // __AUTH_SRV_H
 #endif // __AUTH_SRV_H

+ 0 - 47
src/bin/auth/b10-auth.xml

@@ -44,11 +44,7 @@
   <refsynopsisdiv>
   <refsynopsisdiv>
     <cmdsynopsis>
     <cmdsynopsis>
       <command>b10-auth</command>
       <command>b10-auth</command>
-      <arg><option>-4</option></arg>
-      <arg><option>-6</option></arg>
-      <arg><option>-a <replaceable>address</replaceable></option></arg>
       <arg><option>-n</option></arg>
       <arg><option>-n</option></arg>
-      <arg><option>-p <replaceable>number</replaceable></option></arg>
       <arg><option>-u <replaceable>username</replaceable></option></arg>
       <arg><option>-u <replaceable>username</replaceable></option></arg>
       <arg><option>-v</option></arg>
       <arg><option>-v</option></arg>
     </cmdsynopsis>
     </cmdsynopsis>
@@ -85,39 +81,6 @@
 
 
     <variablelist>
     <variablelist>
       <varlistentry>
       <varlistentry>
-        <term><option>-4</option></term>
-        <listitem><para>
-          Enables IPv4 only mode.
-          This switch may not be used with <option>-6</option> nor
-          <option>-a</option>.
-          By default, it listens on both IPv4 and IPv6 (if capable).
-        </para></listitem>
-      </varlistentry>
-
-      <varlistentry>
-        <term><option>-6</option></term>
-        <listitem><para>
-          Enables IPv6 only mode.
-          This switch may not be used with <option>-4</option> nor
-          <option>-a</option>.
-          By default, it listens on both IPv4 and IPv6 (if capable).
-        </para></listitem>
-      </varlistentry>
-
-      <varlistentry>
-        <term><option>-a <replaceable>address</replaceable></option></term>
-
-        <listitem>
-          <para>The IPv4 or IPv6 address to listen on.
-            This switch may not be used with <option>-4</option> nor
-            <option>-6</option>.
-            The default is to listen on all addresses.
-            (This is a short term workaround. This argument may change.)   
-          </para>                      
-         </listitem>
-      </varlistentry>
-
-      <varlistentry>
         <term><option>-n</option></term>
         <term><option>-n</option></term>
         <listitem><para>
         <listitem><para>
           Do not cache answers in memory.
           Do not cache answers in memory.
@@ -130,16 +93,6 @@
       </varlistentry>
       </varlistentry>
 
 
       <varlistentry>
       <varlistentry>
-        <term><option>-p <replaceable>number</replaceable></option></term>
-        <listitem><para>
-          The port number it listens on.
-          The default is 5300.</para>
-	  <note><simpara>This prototype runs on all interfaces
-	  and on this nonstandard port.</simpara></note>
-        </listitem>
-      </varlistentry>
-
-      <varlistentry>
         <term><option>-u <replaceable>username</replaceable></option></term>
         <term><option>-u <replaceable>username</replaceable></option></term>
         <listitem>
         <listitem>
 	  <para>
 	  <para>

+ 1 - 0
src/bin/auth/benchmarks/Makefile.am

@@ -23,4 +23,5 @@ query_bench_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la
 query_bench_LDADD += $(top_builddir)/src/lib/log/liblog.la
 query_bench_LDADD += $(top_builddir)/src/lib/log/liblog.la
 query_bench_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la
 query_bench_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la
 query_bench_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
 query_bench_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
+query_bench_LDADD += $(top_builddir)/src/lib/server_common/libserver_common.la
 query_bench_LDADD += $(SQLITE_LIBS)
 query_bench_LDADD += $(SQLITE_LIBS)

+ 59 - 0
src/bin/auth/config.cc

@@ -32,11 +32,14 @@
 #include <auth/config.h>
 #include <auth/config.h>
 #include <auth/common.h>
 #include <auth/common.h>
 
 
+#include <server_common/portconfig.h>
+
 using namespace std;
 using namespace std;
 using boost::shared_ptr;
 using boost::shared_ptr;
 using namespace isc::dns;
 using namespace isc::dns;
 using namespace isc::data;
 using namespace isc::data;
 using namespace isc::datasrc;
 using namespace isc::datasrc;
+using namespace isc::server_common::portconfig;
 
 
 namespace {
 namespace {
 // Forward declaration
 // Forward declaration
@@ -210,6 +213,60 @@ public:
     }
     }
 };
 };
 
 
+/**
+ * \brief Configuration parser for listen_on.
+ *
+ * It parses and sets the listening addresses of the server.
+ *
+ * It acts in unusual way. Since actually binding (changing) the sockets
+ * is an operation that is expected to throw often, it shouldn't happen
+ * in commit. Thefere we do it in build. But if the config is not committed
+ * then, we would have it wrong. So we store the old addresses and if
+ * commit is not called before destruction of the object, we return the
+ * old addresses (which is the same kind of dangerous operation, but it is
+ * expected that if we just managed to bind some and had the old ones binded
+ * before, it should work).
+ *
+ * We might do something better in future (like open only the ports that are
+ * extra, put them in in commit and close the old ones), but that's left out
+ * for now.
+ */
+class ListenAddressConfig : public AuthConfigParser {
+public:
+    ListenAddressConfig(AuthSrv& server) :
+        server_(server)
+    { }
+    ~ ListenAddressConfig() {
+        if (rollbackAddresses_.get() != NULL) {
+            server_.setListenAddresses(*rollbackAddresses_);
+        }
+    }
+private:
+    typedef auto_ptr<AddressList> AddrListPtr;
+public:
+    virtual void build(ConstElementPtr config) {
+        AddressList newAddresses = parseAddresses(config, "listen_on");
+        AddrListPtr old(new AddressList(server_.getListenAddresses()));
+        server_.setListenAddresses(newAddresses);
+        /*
+         * Set the rollback addresses only after successful setting of the
+         * new addresses, so we don't try to rollback if the setup is
+         * unsuccessful (the above can easily throw).
+         */
+        rollbackAddresses_ = old;
+    }
+    virtual void commit() {
+        rollbackAddresses_.release();
+    }
+private:
+    AuthSrv& server_;
+    /**
+     * This is the old address list, if we expect to roll back. When we commit,
+     * this is set to NULL.
+     */
+    AddrListPtr rollbackAddresses_;
+};
+
 // This is a generalized version of create function that can create
 // This is a generalized version of create function that can create
 // an AuthConfigParser object for "internal" use.
 // an AuthConfigParser object for "internal" use.
 AuthConfigParser*
 AuthConfigParser*
@@ -226,6 +283,8 @@ createAuthConfigParser(AuthSrv& server, const std::string& config_id,
         return (new StatisticsIntervalConfig(server));
         return (new StatisticsIntervalConfig(server));
     } else if (internal && config_id == "datasources/memory") {
     } else if (internal && config_id == "datasources/memory") {
         return (new MemoryDatasourceConfig(server));
         return (new MemoryDatasourceConfig(server));
+    } else if (config_id == "listen_on") {
+        return (new ListenAddressConfig(server));
     } else if (config_id == "_commit_throw") {
     } else if (config_id == "_commit_throw") {
         // This is for testing purpose only and should not appear in the
         // This is for testing purpose only and should not appear in the
         // actual configuration syntax.  While this could crash the caller
         // actual configuration syntax.  While this could crash the caller

+ 7 - 58
src/bin/auth/main.cc

@@ -42,6 +42,7 @@
 #include <auth/change_user.h>
 #include <auth/change_user.h>
 #include <auth/auth_srv.h>
 #include <auth/auth_srv.h>
 #include <asiolink/asiolink.h>
 #include <asiolink/asiolink.h>
+#include <log/dummylog.h>
 
 
 using namespace std;
 using namespace std;
 using namespace isc::data;
 using namespace isc::data;
@@ -55,9 +56,6 @@ namespace {
 
 
 bool verbose_mode = false;
 bool verbose_mode = false;
 
 
-// Default port current 5300 for testing purposes
-const char* DNSPORT = "5300";
-
 /* need global var for config/command handlers.
 /* need global var for config/command handlers.
  * todo: turn this around, and put handlers in the authserver
  * todo: turn this around, and put handlers in the authserver
  * class itself? */
  * class itself? */
@@ -76,13 +74,8 @@ my_command_handler(const string& command, ConstElementPtr args) {
 
 
 void
 void
 usage() {
 usage() {
-    cerr << "Usage:  b10-auth [-a address] [-p port] [-u user] [-4|-6] [-nv]"
-         << endl;
-    cerr << "\t-a: specify the address to listen on (default: all) " << endl;
-    cerr << "\t-p: specify the port to listen on (default: " << DNSPORT << ")"
+    cerr << "Usage:  b10-auth [-u user] [-nv]"
          << endl;
          << endl;
-    cerr << "\t-4: listen on all IPv4 addresses (incompatible with -a)" << endl;
-    cerr << "\t-6: listen on all IPv6 addresses (incompatible with -a)" << endl;
     cerr << "\t-n: do not cache answers in memory" << endl;
     cerr << "\t-n: do not cache answers in memory" << endl;
     cerr << "\t-u: change process UID to the specified user" << endl;
     cerr << "\t-u: change process UID to the specified user" << endl;
     cerr << "\t-v: verbose output" << endl;
     cerr << "\t-v: verbose output" << endl;
@@ -93,38 +86,20 @@ usage() {
 int
 int
 main(int argc, char* argv[]) {
 main(int argc, char* argv[]) {
     int ch;
     int ch;
-    const char* port = DNSPORT;
-    const char* address = NULL;
     const char* uid = NULL;
     const char* uid = NULL;
-    bool use_ipv4 = true, use_ipv6 = true, cache = true;
+    bool cache = true;
 
 
-    while ((ch = getopt(argc, argv, "46a:np:u:v")) != -1) {
+    while ((ch = getopt(argc, argv, ":nu:v")) != -1) {
         switch (ch) {
         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 'n':
         case 'n':
             cache = false;
             cache = false;
             break;
             break;
-        case 'a':
-            address = optarg;
-            break;
-        case 'p':
-            port = optarg;
-            break;
         case 'u':
         case 'u':
             uid = optarg;
             uid = optarg;
             break;
             break;
         case 'v':
         case 'v':
             verbose_mode = true;
             verbose_mode = true;
+            isc::log::denabled = true;
             break;
             break;
         case '?':
         case '?':
         default:
         default:
@@ -136,18 +111,6 @@ main(int argc, char* argv[]) {
         usage();
         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();
-    }
-
     int ret = 0;
     int ret = 0;
 
 
     // XXX: we should eventually pass io_service here.
     // XXX: we should eventually pass io_service here.
@@ -182,21 +145,8 @@ main(int argc, char* argv[]) {
         DNSLookup* lookup = auth_server->getDNSLookupProvider();
         DNSLookup* lookup = auth_server->getDNSLookupProvider();
         DNSAnswer* answer = auth_server->getDNSAnswerProvider();
         DNSAnswer* answer = auth_server->getDNSAnswerProvider();
 
 
-        DNSService* dns_service;
-        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.
-            dns_service = new DNSService(io_service,  *port, *address,
-                                         checkin, lookup, answer);
-        } else {
-            dns_service = new DNSService(io_service, *port, use_ipv4,
-                                         use_ipv6, checkin, lookup,
-                                         answer);
-        }
+        DNSService dns_service(io_service, checkin, lookup, answer);
+        auth_server->setDNSService(dns_service);
         cout << "[b10-auth] DNSServices created." << endl;
         cout << "[b10-auth] DNSServices created." << endl;
 
 
         cc_session = new Session(io_service.get_io_service());
         cc_session = new Session(io_service.get_io_service());
@@ -237,7 +187,6 @@ main(int argc, char* argv[]) {
         cout << "[b10-auth] Server started." << endl;
         cout << "[b10-auth] Server started." << endl;
         io_service.run();
         io_service.run();
 
 
-        delete dns_service;
     } catch (const std::exception& ex) {
     } catch (const std::exception& ex) {
         cerr << "[b10-auth] Server failed: " << ex.what() << endl;
         cerr << "[b10-auth] Server failed: " << ex.what() << endl;
         ret = 1;
         ret = 1;

+ 1 - 0
src/bin/auth/tests/Makefile.am

@@ -45,6 +45,7 @@ run_unittests_LDADD += $(top_builddir)/src/lib/cc/libcc.la
 run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 run_unittests_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la
 run_unittests_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la
 run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la
 run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la
+run_unittests_LDADD += $(top_builddir)/src/lib/server_common/libserver_common.la
 run_unittests_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la
 run_unittests_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la
 endif
 endif
 
 

+ 17 - 1
src/bin/auth/tests/auth_srv_unittest.cc

@@ -26,6 +26,8 @@
 #include <dns/rrttl.h>
 #include <dns/rrttl.h>
 #include <dns/rdataclass.h>
 #include <dns/rdataclass.h>
 
 
+#include <server_common/portconfig.h>
+
 #include <datasrc/memory_datasrc.h>
 #include <datasrc/memory_datasrc.h>
 #include <auth/auth_srv.h>
 #include <auth/auth_srv.h>
 #include <auth/common.h>
 #include <auth/common.h>
@@ -34,6 +36,7 @@
 #include <dns/tests/unittest_util.h>
 #include <dns/tests/unittest_util.h>
 #include <testutils/dnsmessage_test.h>
 #include <testutils/dnsmessage_test.h>
 #include <testutils/srv_test.h>
 #include <testutils/srv_test.h>
+#include <testutils/portconfig.h>
 
 
 using namespace std;
 using namespace std;
 using namespace isc::cc;
 using namespace isc::cc;
@@ -43,6 +46,7 @@ using namespace isc::data;
 using namespace isc::xfr;
 using namespace isc::xfr;
 using namespace asiolink;
 using namespace asiolink;
 using namespace isc::testutils;
 using namespace isc::testutils;
+using namespace isc::server_common::portconfig;
 using isc::UnitTestUtil;
 using isc::UnitTestUtil;
 
 
 namespace {
 namespace {
@@ -55,7 +59,12 @@ const char* const BADCONFIG_TESTDB =
 
 
 class AuthSrvTest : public SrvTestBase {
 class AuthSrvTest : public SrvTestBase {
 protected:
 protected:
-    AuthSrvTest() : server(true, xfrout), rrclass(RRClass::IN()) {
+    AuthSrvTest() :
+        dnss_(ios_, NULL, NULL, NULL),
+        server(true, xfrout),
+        rrclass(RRClass::IN())
+    {
+        server.setDNSService(dnss_);
         server.setXfrinSession(&notify_session);
         server.setXfrinSession(&notify_session);
         server.setStatisticsSession(&statistics_session);
         server.setStatisticsSession(&statistics_session);
     }
     }
@@ -63,6 +72,8 @@ protected:
         server.processMessage(*io_message, parse_message, response_obuffer,
         server.processMessage(*io_message, parse_message, response_obuffer,
                               &dnsserv);
                               &dnsserv);
     }
     }
+    IOService ios_;
+    DNSService dnss_;
     MockSession statistics_session;
     MockSession statistics_session;
     MockXfroutClient xfrout;
     MockXfroutClient xfrout;
     AuthSrv server;
     AuthSrv server;
@@ -650,4 +661,9 @@ TEST_F(AuthSrvTest, stop) {
     // If/when the interval timer has finer granularity we'll probably add
     // If/when the interval timer has finer granularity we'll probably add
     // our own tests here, so we keep this empty test case.
     // our own tests here, so we keep this empty test case.
 }
 }
+
+TEST_F(AuthSrvTest, listenAddresses) {
+    isc::testutils::portconfig::listenAddresses(server);
+}
+
 }
 }

+ 21 - 1
src/bin/auth/tests/config_unittest.cc

@@ -30,6 +30,7 @@
 #include <auth/common.h>
 #include <auth/common.h>
 
 
 #include <testutils/mockups.h>
 #include <testutils/mockups.h>
+#include <testutils/portconfig.h>
 
 
 using namespace isc::dns;
 using namespace isc::dns;
 using namespace isc::data;
 using namespace isc::data;
@@ -39,7 +40,15 @@ using namespace asiolink;
 namespace {
 namespace {
 class AuthConfigTest : public ::testing::Test {
 class AuthConfigTest : public ::testing::Test {
 protected:
 protected:
-    AuthConfigTest() : rrclass(RRClass::IN()), server(true, xfrout) {}
+    AuthConfigTest() :
+        dnss_(ios_, NULL, NULL, NULL),
+        rrclass(RRClass::IN()),
+        server(true, xfrout)
+    {
+        server.setDNSService(dnss_);
+    }
+    IOService ios_;
+    DNSService dnss_;
     const RRClass rrclass;
     const RRClass rrclass;
     MockXfroutClient xfrout;
     MockXfroutClient xfrout;
     AuthSrv server;
     AuthSrv server;
@@ -112,6 +121,17 @@ TEST_F(AuthConfigTest, exceptionFromCommit) {
                  FatalError);
                  FatalError);
 }
 }
 
 
+// Test invalid address configs are rejected
+TEST_F(AuthConfigTest, invalidListenAddressConfig) {
+    // This currently passes simply because the config doesn't know listen_on
+    isc::testutils::portconfig::invalidListenAddressConfig(server);
+}
+
+// Try setting addresses trough config
+TEST_F(AuthConfigTest, listenAddressConfig) {
+    isc::testutils::portconfig::listenAddressConfig(server);
+}
+
 class MemoryDatasrcConfigTest : public AuthConfigTest {
 class MemoryDatasrcConfigTest : public AuthConfigTest {
 protected:
 protected:
     MemoryDatasrcConfigTest() :
     MemoryDatasrcConfigTest() :

+ 5 - 40
src/bin/bind10/bind10.py.in

@@ -194,8 +194,8 @@ class CChannelConnectError(Exception): pass
 class BoB:
 class BoB:
     """Boss of BIND class."""
     """Boss of BIND class."""
     
     
-    def __init__(self, msgq_socket_file=None, dns_port=5300, address=None,
-                 nocache=False, verbose=False, setuid=None, username=None):
+    def __init__(self, msgq_socket_file=None, nocache=False, verbose=False,
+    setuid=None, username=None):
         """
         """
             Initialize the Boss of BIND. This is a singleton (only one can run).
             Initialize the Boss of BIND. This is a singleton (only one can run).
         
         
@@ -203,8 +203,6 @@ class BoB:
             msgq process listens on.  If verbose is True, then the boss reports
             msgq process listens on.  If verbose is True, then the boss reports
             what it is doing.
             what it is doing.
         """
         """
-        self.address = address
-        self.dns_port = dns_port
         self.cc_session = None
         self.cc_session = None
         self.ccs = None
         self.ccs = None
         self.cfg_start_auth = True
         self.cfg_start_auth = True
@@ -462,9 +460,6 @@ class BoB:
             Start the Authoritative server
             Start the Authoritative server
         """
         """
         authargs = ['b10-auth']
         authargs = ['b10-auth']
-        authargs += ['-p', str(self.dns_port)]
-        if self.address:
-            authargs += ['-a', str(self.address)]
         if self.nocache:
         if self.nocache:
             authargs += ['-n']
             authargs += ['-n']
         if self.uid:
         if self.uid:
@@ -473,8 +468,7 @@ class BoB:
             authargs += ['-v']
             authargs += ['-v']
 
 
         # ... and start
         # ... and start
-        self.start_process("b10-auth", authargs, c_channel_env,
-            self.dns_port, self.address)
+        self.start_process("b10-auth", authargs, c_channel_env)
 
 
     def start_resolver(self, c_channel_env):
     def start_resolver(self, c_channel_env):
         """
         """
@@ -787,28 +781,6 @@ def fatal_signal(signal_number, stack_frame):
     signal.signal(signal.SIGCHLD, signal.SIG_DFL)
     signal.signal(signal.SIGCHLD, signal.SIG_DFL)
     boss_of_bind.runnable = False
     boss_of_bind.runnable = False
 
 
-def check_port(option, opt_str, value, parser):
-    """Function to insure that the port we are passed is actually 
-    a valid port number. Used by OptionParser() on startup."""
-    try:
-        if opt_str in ['-p', '--port']:
-            parser.values.dns_port = isc.net.parse.port_parse(value)
-        else:
-            raise OptionValueError("Unknown option " + opt_str)
-    except ValueError as e:
-        raise OptionValueError(str(e))
-
-def check_addr(option, opt_str, value, parser):
-    """Function to insure that the address we are passed is actually 
-    a valid address. Used by OptionParser() on startup."""
-    try:
-        if opt_str in ['-a', '--address']:
-            parser.values.address = isc.net.parse.addr_parse(value)
-        else:
-            raise OptionValueError("Unknown option " + opt_str)
-    except ValueError:
-        raise OptionValueError("%s requires a valid IPv4 or IPv6 address" % opt_str)
-
 def process_rename(option, opt_str, value, parser):
 def process_rename(option, opt_str, value, parser):
     """Function that renames the process if it is requested by a option."""
     """Function that renames the process if it is requested by a option."""
     isc.util.process.rename(value)
     isc.util.process.rename(value)
@@ -821,17 +793,11 @@ def main():
 
 
     # Parse any command-line options.
     # Parse any command-line options.
     parser = OptionParser(version=VERSION)
     parser = OptionParser(version=VERSION)
-    parser.add_option("-a", "--address", dest="address", type="string",
-                      action="callback", callback=check_addr, default=None,
-                      help="address the DNS server will use (default: listen on all addresses)")
     parser.add_option("-m", "--msgq-socket-file", dest="msgq_socket_file",
     parser.add_option("-m", "--msgq-socket-file", dest="msgq_socket_file",
                       type="string", default=None,
                       type="string", default=None,
                       help="UNIX domain socket file the b10-msgq daemon will use")
                       help="UNIX domain socket file the b10-msgq daemon will use")
     parser.add_option("-n", "--no-cache", action="store_true", dest="nocache",
     parser.add_option("-n", "--no-cache", action="store_true", dest="nocache",
                       default=False, help="disable hot-spot cache in authoritative DNS server")
                       default=False, help="disable hot-spot cache in authoritative DNS server")
-    parser.add_option("-p", "--port", dest="dns_port", type="int",
-                      action="callback", callback=check_port, default=5300,
-                      help="port the DNS server will use (default 5300)")
     parser.add_option("-u", "--user", dest="user", type="string", default=None,
     parser.add_option("-u", "--user", dest="user", type="string", default=None,
                       help="Change user after startup (must run as root)")
                       help="Change user after startup (must run as root)")
     parser.add_option("-v", "--verbose", dest="verbose", action="store_true",
     parser.add_option("-v", "--verbose", dest="verbose", action="store_true",
@@ -892,9 +858,8 @@ def main():
     signal.signal(signal.SIGPIPE, signal.SIG_IGN)
     signal.signal(signal.SIGPIPE, signal.SIG_IGN)
 
 
     # Go bob!
     # Go bob!
-    boss_of_bind = BoB(options.msgq_socket_file, options.dns_port,
-                       options.address, options.nocache, options.verbose,
-                       setuid, username)
+    boss_of_bind = BoB(options.msgq_socket_file, options.nocache,
+                       options.verbose, setuid, username)
     startup_result = boss_of_bind.startup()
     startup_result = boss_of_bind.startup()
     if startup_result:
     if startup_result:
         sys.stderr.write("[bind10] Error on startup: %s\n" % startup_result)
         sys.stderr.write("[bind10] Error on startup: %s\n" % startup_result)

+ 3 - 34
src/bin/bind10/bind10.xml

@@ -20,7 +20,7 @@
 <refentry>
 <refentry>
 
 
   <refentryinfo>
   <refentryinfo>
-    <date>July 29, 2010</date>
+    <date>February 22, 2011</date>
   </refentryinfo>
   </refentryinfo>
 
 
   <refmeta>
   <refmeta>
@@ -36,24 +36,20 @@
 
 
   <docinfo>
   <docinfo>
     <copyright>
     <copyright>
-      <year>2010</year>
+      <year>2011</year>
       <holder>Internet Systems Consortium, Inc. ("ISC")</holder>
       <holder>Internet Systems Consortium, Inc. ("ISC")</holder>
     </copyright>
     </copyright>
   </docinfo>
   </docinfo>
 
 
   <refsynopsisdiv>
   <refsynopsisdiv>
     <cmdsynopsis>
     <cmdsynopsis>
-      <command>bind10</command>    
-      <arg><option>-a <replaceable>address</replaceable></option></arg>
+      <command>bind10</command>
       <arg><option>-m <replaceable>file</replaceable></option></arg>
       <arg><option>-m <replaceable>file</replaceable></option></arg>
       <arg><option>-n</option></arg>
       <arg><option>-n</option></arg>
-      <arg><option>-p <replaceable>number</replaceable></option></arg>
       <arg><option>-u <replaceable>user</replaceable></option></arg>
       <arg><option>-u <replaceable>user</replaceable></option></arg>
       <arg><option>-v</option></arg>
       <arg><option>-v</option></arg>
-      <arg><option>--address <replaceable>address</replaceable></option></arg>
       <arg><option>--msgq-socket-file <replaceable>file</replaceable></option></arg>
       <arg><option>--msgq-socket-file <replaceable>file</replaceable></option></arg>
       <arg><option>--no-cache</option></arg>
       <arg><option>--no-cache</option></arg>
-      <arg><option>--port <replaceable>number</replaceable></option></arg>
       <arg><option>--user <replaceable>user</replaceable></option></arg>
       <arg><option>--user <replaceable>user</replaceable></option></arg>
       <arg><option>--pretty-name <replaceable>name</replaceable></option></arg>
       <arg><option>--pretty-name <replaceable>name</replaceable></option></arg>
       <arg><option>--verbose</option></arg>
       <arg><option>--verbose</option></arg>
@@ -86,19 +82,6 @@
     <variablelist>
     <variablelist>
 
 
       <varlistentry>
       <varlistentry>
-        <term><option>-a</option> <replaceable>address</replaceable>, <option>--address</option> <replaceable>address</replaceable></term>
-
-        <listitem>
-	  <para>The IPv4 or IPv6 address for the
-	    <citerefentry><refentrytitle>b10-auth</refentrytitle><manvolnum>8</manvolnum></citerefentry>
-            daemon to listen on.
-            The default is to listen on all addresses. 
-            (This is a short term workaround. This argument may change.)
-          </para>
-         </listitem>
-      </varlistentry>
-
-      <varlistentry>
         <term><option>-m</option> <replaceable>file</replaceable>,
         <term><option>-m</option> <replaceable>file</replaceable>,
            <option>--msgq-socket-file</option> <replaceable>file</replaceable></term>
            <option>--msgq-socket-file</option> <replaceable>file</replaceable></term>
 
 
@@ -123,20 +106,6 @@
       </varlistentry>
       </varlistentry>
 
 
       <varlistentry>
       <varlistentry>
-        <term><option>-p</option> <replaceable>number</replaceable>, <option>--port</option> <replaceable>number</replaceable></term>
-
-        <listitem>
-          <para>The port number for the
-	    <citerefentry><refentrytitle>b10-auth</refentrytitle><manvolnum>8</manvolnum></citerefentry>
-            daemon to listen on.
-            The default is 5300.</para>
-<!-- TODO: -->
-	    <note><simpara>This prototype release uses a non-default
-	    port for domain service.</simpara></note>
-         </listitem>
-      </varlistentry>
-
-      <varlistentry>
         <term><option>-u</option> <replaceable>user</replaceable>, <option>--user</option> <replaceable>name</replaceable></term>
         <term><option>-u</option> <replaceable>user</replaceable>, <option>--user</option> <replaceable>name</replaceable></term>
 
 
         <listitem>
         <listitem>

+ 0 - 38
src/bin/bind10/tests/bind10_test.py

@@ -78,8 +78,6 @@ class TestBoB(unittest.TestCase):
         bob = BoB()
         bob = BoB()
         self.assertEqual(bob.verbose, False)
         self.assertEqual(bob.verbose, False)
         self.assertEqual(bob.msgq_socket_file, None)
         self.assertEqual(bob.msgq_socket_file, None)
-        self.assertEqual(bob.dns_port, 5300)
-        self.assertEqual(bob.address, None)
         self.assertEqual(bob.cc_session, None)
         self.assertEqual(bob.cc_session, None)
         self.assertEqual(bob.ccs, None)
         self.assertEqual(bob.ccs, None)
         self.assertEqual(bob.processes, {})
         self.assertEqual(bob.processes, {})
@@ -95,42 +93,6 @@ class TestBoB(unittest.TestCase):
         bob = BoB("alt_socket_file")
         bob = BoB("alt_socket_file")
         self.assertEqual(bob.verbose, False)
         self.assertEqual(bob.verbose, False)
         self.assertEqual(bob.msgq_socket_file, "alt_socket_file")
         self.assertEqual(bob.msgq_socket_file, "alt_socket_file")
-        self.assertEqual(bob.address, None)
-        self.assertEqual(bob.dns_port, 5300)
-        self.assertEqual(bob.cc_session, None)
-        self.assertEqual(bob.ccs, None)
-        self.assertEqual(bob.processes, {})
-        self.assertEqual(bob.dead_processes, {})
-        self.assertEqual(bob.runnable, False)
-        self.assertEqual(bob.uid, None)
-        self.assertEqual(bob.username, None)
-        self.assertEqual(bob.nocache, False)
-        self.assertEqual(bob.cfg_start_auth, True)
-        self.assertEqual(bob.cfg_start_resolver, False)
-
-    def test_init_alternate_dns_port(self):
-        bob = BoB(None, 9999)
-        self.assertEqual(bob.verbose, False)
-        self.assertEqual(bob.msgq_socket_file, None)
-        self.assertEqual(bob.dns_port, 9999)
-        self.assertEqual(bob.address, None)
-        self.assertEqual(bob.cc_session, None)
-        self.assertEqual(bob.ccs, None)
-        self.assertEqual(bob.processes, {})
-        self.assertEqual(bob.dead_processes, {})
-        self.assertEqual(bob.runnable, False)
-        self.assertEqual(bob.uid, None)
-        self.assertEqual(bob.username, None)
-        self.assertEqual(bob.nocache, False)
-        self.assertEqual(bob.cfg_start_auth, True)
-        self.assertEqual(bob.cfg_start_resolver, False)
-
-    def test_init_alternate_address(self):
-        bob = BoB(None, 1234, IPAddr('127.127.127.127'))
-        self.assertEqual(bob.verbose, False)
-        self.assertEqual(bob.msgq_socket_file, None)
-        self.assertEqual(bob.dns_port, 1234)
-        self.assertEqual(bob.address.addr, socket.inet_aton('127.127.127.127'))
         self.assertEqual(bob.cc_session, None)
         self.assertEqual(bob.cc_session, None)
         self.assertEqual(bob.ccs, None)
         self.assertEqual(bob.ccs, None)
         self.assertEqual(bob.processes, {})
         self.assertEqual(bob.processes, {})

+ 1 - 0
src/bin/resolver/Makefile.am

@@ -48,6 +48,7 @@ b10_resolver_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/log/liblog.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/log/liblog.la
+b10_resolver_LDADD += $(top_builddir)/src/lib/server_common/libserver_common.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/cache/libcache.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/cache/libcache.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la
 b10_resolver_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la
 b10_resolver_LDADD += $(top_builddir)/src/bin/auth/change_user.o
 b10_resolver_LDADD += $(top_builddir)/src/bin/auth/change_user.o

+ 22 - 98
src/bin/resolver/resolver.cc

@@ -39,6 +39,7 @@
 #include <dns/rrttl.h>
 #include <dns/rrttl.h>
 #include <dns/message.h>
 #include <dns/message.h>
 #include <dns/messagerenderer.h>
 #include <dns/messagerenderer.h>
+#include <server_common/portconfig.h>
 
 
 #include <log/dummylog.h>
 #include <log/dummylog.h>
 
 
@@ -52,8 +53,7 @@ using namespace isc::data;
 using namespace isc::config;
 using namespace isc::config;
 using isc::log::dlog;
 using isc::log::dlog;
 using namespace asiolink;
 using namespace asiolink;
-
-typedef pair<string, uint16_t> addr_t;
+using namespace isc::server_common::portconfig;
 
 
 class ResolverImpl {
 class ResolverImpl {
 private:
 private:
@@ -96,14 +96,14 @@ public:
         }
         }
     }
     }
 
 
-    void setForwardAddresses(const vector<addr_t>& upstream,
+    void setForwardAddresses(const AddressList& upstream,
         DNSService *dnss)
         DNSService *dnss)
     {
     {
         upstream_ = upstream;
         upstream_ = upstream;
         if (dnss) {
         if (dnss) {
             if (!upstream_.empty()) {
             if (!upstream_.empty()) {
                 dlog("Setting forward addresses:");
                 dlog("Setting forward addresses:");
-                BOOST_FOREACH(const addr_t& address, upstream) {
+                BOOST_FOREACH(const AddressPair& address, upstream) {
                     dlog(" " + address.first + ":" +
                     dlog(" " + address.first + ":" +
                         boost::lexical_cast<string>(address.second));
                         boost::lexical_cast<string>(address.second));
                 }
                 }
@@ -113,14 +113,14 @@ public:
         }
         }
     }
     }
 
 
-    void setRootAddresses(const vector<addr_t>& upstream_root,
+    void setRootAddresses(const AddressList& upstream_root,
                           DNSService *dnss)
                           DNSService *dnss)
     {
     {
         upstream_root_ = upstream_root;
         upstream_root_ = upstream_root;
         if (dnss) {
         if (dnss) {
             if (!upstream_root_.empty()) {
             if (!upstream_root_.empty()) {
                 dlog("Setting root addresses:");
                 dlog("Setting root addresses:");
-                BOOST_FOREACH(const addr_t& address, upstream_root) {
+                BOOST_FOREACH(const AddressPair& address, upstream_root) {
                     dlog(" " + address.first + ":" +
                     dlog(" " + address.first + ":" +
                         boost::lexical_cast<string>(address.second));
                         boost::lexical_cast<string>(address.second));
                 }
                 }
@@ -144,11 +144,11 @@ public:
     /// These members are public because Resolver accesses them directly.
     /// These members are public because Resolver accesses them directly.
     ModuleCCSession* config_session_;
     ModuleCCSession* config_session_;
     /// Addresses of the root nameserver(s)
     /// Addresses of the root nameserver(s)
-    vector<addr_t> upstream_root_;
+    AddressList upstream_root_;
     /// Addresses of the forward nameserver
     /// Addresses of the forward nameserver
-    vector<addr_t> upstream_;
+    AddressList upstream_;
     /// Addresses we listen on
     /// Addresses we listen on
-    vector<addr_t> listen_;
+    AddressList listen_;
 
 
     /// Timeout for outgoing queries in milliseconds
     /// Timeout for outgoing queries in milliseconds
     int query_timeout_;
     int query_timeout_;
@@ -453,46 +453,6 @@ ResolverImpl::processNormalQuery(const Question& question,
     rec_query_->resolve(question, answer_message, buffer, server);
     rec_query_->resolve(question, answer_message, 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,
-                "root_addresses, forward_addresses, and listen_on config element must be a list");
-        }
-    }
-    return (result);
-}
-
-}
-
 ConstElementPtr
 ConstElementPtr
 Resolver::updateConfig(ConstElementPtr config) {
 Resolver::updateConfig(ConstElementPtr config) {
     dlog("New config comes: " + config->toWire());
     dlog("New config comes: " + config->toWire());
@@ -500,11 +460,14 @@ Resolver::updateConfig(ConstElementPtr config) {
     try {
     try {
         // Parse forward_addresses
         // Parse forward_addresses
         ConstElementPtr rootAddressesE(config->get("root_addresses"));
         ConstElementPtr rootAddressesE(config->get("root_addresses"));
-        vector<addr_t> rootAddresses(parseAddresses(rootAddressesE));
+        AddressList rootAddresses(parseAddresses(rootAddressesE,
+                                                    "root_addresses"));
         ConstElementPtr forwardAddressesE(config->get("forward_addresses"));
         ConstElementPtr forwardAddressesE(config->get("forward_addresses"));
-        vector<addr_t> forwardAddresses(parseAddresses(forwardAddressesE));
+        AddressList forwardAddresses(parseAddresses(forwardAddressesE,
+                                                       "forward_addresses"));
         ConstElementPtr listenAddressesE(config->get("listen_on"));
         ConstElementPtr listenAddressesE(config->get("listen_on"));
-        vector<addr_t> listenAddresses(parseAddresses(listenAddressesE));
+        AddressList listenAddresses(parseAddresses(listenAddressesE,
+                                                      "listen_on"));
         bool set_timeouts(false);
         bool set_timeouts(false);
         int qtimeout = impl_->query_timeout_;
         int qtimeout = impl_->query_timeout_;
         int ctimeout = impl_->client_timeout_;
         int ctimeout = impl_->client_timeout_;
@@ -577,13 +540,13 @@ Resolver::updateConfig(ConstElementPtr config) {
 }
 }
 
 
 void
 void
-Resolver::setForwardAddresses(const vector<addr_t>& addresses)
+Resolver::setForwardAddresses(const AddressList& addresses)
 {
 {
     impl_->setForwardAddresses(addresses, dnss_);
     impl_->setForwardAddresses(addresses, dnss_);
 }
 }
 
 
 void
 void
-Resolver::setRootAddresses(const vector<addr_t>& addresses)
+Resolver::setRootAddresses(const AddressList& addresses)
 {
 {
     impl_->setRootAddresses(addresses, dnss_);
     impl_->setRootAddresses(addresses, dnss_);
 }
 }
@@ -593,58 +556,19 @@ Resolver::isForwarding() const {
     return (!impl_->upstream_.empty());
     return (!impl_->upstream_.empty());
 }
 }
 
 
-vector<addr_t>
+AddressList
 Resolver::getForwardAddresses() const {
 Resolver::getForwardAddresses() const {
     return (impl_->upstream_);
     return (impl_->upstream_);
 }
 }
 
 
-vector<addr_t>
+AddressList
 Resolver::getRootAddresses() const {
 Resolver::getRootAddresses() const {
     return (impl_->upstream_root_);
     return (impl_->upstream_root_);
 }
 }
 
 
-namespace {
-
-void
-setAddresses(DNSService *service, const vector<addr_t>& addresses) {
-    service->clearServers();
-    BOOST_FOREACH(const addr_t &address, addresses) {
-        service->addServer(address.second, address.first);
-    }
-}
-
-}
-
 void
 void
-Resolver::setListenAddresses(const vector<addr_t>& addresses) {
-    try {
-        dlog("Setting listen addresses:");
-        BOOST_FOREACH(const addr_t& addr, addresses) {
-            dlog(" " + addr.first + ":" +
-                        boost::lexical_cast<string>(addr.second));
-        }
-        setAddresses(dnss_, addresses);
-        impl_->listen_ = addresses;
-    }
-    catch (exception& e) {
-        /*
-         * We couldn't set it. So return it back. If that fails as well,
-         * we have a problem.
-         *
-         * If that fails, bad luck, but we are useless anyway, so just die
-         * and let boss start us again.
-         */
-        dlog(string("Unable to set new address: ") + e.what(),true);
-        try {
-            setAddresses(dnss_, impl_->listen_);
-        }
-        catch (const exception& e2) {
-            dlog(string("Unable to recover from error;"),true);
-            dlog(string("Rollback failed with: ") + e2.what(),true);
-            abort();
-        }
-        throw; // Let it fly a little bit further
-    }
+Resolver::setListenAddresses(const AddressList& addresses) {
+    installListenAddresses(addresses, impl_->listen_, *dnss_);
 }
 }
 
 
 void
 void
@@ -680,7 +604,7 @@ Resolver::getRetries() const {
     return impl_->retries_;
     return impl_->retries_;
 }
 }
 
 
-vector<addr_t>
+AddressList
 Resolver::getListenAddresses() const {
 Resolver::getListenAddresses() const {
     return (impl_->listen_);
     return (impl_->listen_);
 }
 }

+ 17 - 17
src/bin/resolver/resolver.spec.pre.in

@@ -6,48 +6,48 @@
       {
       {
         "item_name": "timeout_query",
         "item_name": "timeout_query",
         "item_type": "integer",
         "item_type": "integer",
-        "item_optional": False,
+        "item_optional": false,
         "item_default": 2000
         "item_default": 2000
       },
       },
       {
       {
         "item_name": "timeout_client",
         "item_name": "timeout_client",
         "item_type": "integer",
         "item_type": "integer",
-        "item_optional": False,
+        "item_optional": false,
         "item_default": 4000
         "item_default": 4000
       },
       },
       {
       {
         "item_name": "timeout_lookup",
         "item_name": "timeout_lookup",
         "item_type": "integer",
         "item_type": "integer",
-        "item_optional": False,
+        "item_optional": false,
         "item_default": 30000
         "item_default": 30000
       },
       },
       {
       {
         "item_name": "retries",
         "item_name": "retries",
         "item_type": "integer",
         "item_type": "integer",
-        "item_optional": False,
+        "item_optional": false,
         "item_default": 3
         "item_default": 3
       },
       },
       {
       {
         "item_name": "forward_addresses",
         "item_name": "forward_addresses",
         "item_type": "list",
         "item_type": "list",
-        "item_optional": True,
+        "item_optional": true,
         "item_default": [],
         "item_default": [],
         "list_item_spec" : {
         "list_item_spec" : {
           "item_name": "address",
           "item_name": "address",
           "item_type": "map",
           "item_type": "map",
-          "item_optional": False,
+          "item_optional": false,
           "item_default": {},
           "item_default": {},
           "map_item_spec": [
           "map_item_spec": [
             {
             {
               "item_name": "address",
               "item_name": "address",
               "item_type": "string",
               "item_type": "string",
-              "item_optional": False,
+              "item_optional": false,
               "item_default": "::1"
               "item_default": "::1"
             },
             },
             {
             {
               "item_name": "port",
               "item_name": "port",
               "item_type": "integer",
               "item_type": "integer",
-              "item_optional": False,
+              "item_optional": false,
               "item_default": 53
               "item_default": 53
             }
             }
           ]
           ]
@@ -56,24 +56,24 @@
       {
       {
         "item_name": "root_addresses",
         "item_name": "root_addresses",
         "item_type": "list",
         "item_type": "list",
-        "item_optional": True,
+        "item_optional": true,
         "item_default": [],
         "item_default": [],
         "list_item_spec" : {
         "list_item_spec" : {
           "item_name": "address",
           "item_name": "address",
           "item_type": "map",
           "item_type": "map",
-          "item_optional": False,
+          "item_optional": false,
           "item_default": {},
           "item_default": {},
           "map_item_spec": [
           "map_item_spec": [
             {
             {
               "item_name": "address",
               "item_name": "address",
               "item_type": "string",
               "item_type": "string",
-              "item_optional": False,
+              "item_optional": false,
               "item_default": "::1"
               "item_default": "::1"
             },
             },
             {
             {
               "item_name": "port",
               "item_name": "port",
               "item_type": "integer",
               "item_type": "integer",
-              "item_optional": False,
+              "item_optional": false,
               "item_default": 53
               "item_default": 53
             }
             }
           ]
           ]
@@ -82,7 +82,7 @@
       {
       {
         "item_name": "listen_on",
         "item_name": "listen_on",
         "item_type": "list",
         "item_type": "list",
-        "item_optional": False,
+        "item_optional": false,
         "item_default": [
         "item_default": [
           {
           {
             "address": "::1",
             "address": "::1",
@@ -91,24 +91,24 @@
           {
           {
             "address": "127.0.0.1",
             "address": "127.0.0.1",
             "port": 5300
             "port": 5300
-          },
+          }
         ],
         ],
         "list_item_spec": {
         "list_item_spec": {
           "item_name": "address",
           "item_name": "address",
           "item_type": "map",
           "item_type": "map",
-          "item_optional": False,
+          "item_optional": false,
           "item_default": {},
           "item_default": {},
           "map_item_spec": [
           "map_item_spec": [
             {
             {
               "item_name": "address",
               "item_name": "address",
               "item_type": "string",
               "item_type": "string",
-              "item_optional": False,
+              "item_optional": false,
               "item_default": "::1"
               "item_default": "::1"
             },
             },
             {
             {
               "item_name": "port",
               "item_name": "port",
               "item_type": "integer",
               "item_type": "integer",
-              "item_optional": False,
+              "item_optional": false,
               "item_default": 5300
               "item_default": 5300
             }
             }
           ]
           ]

+ 1 - 0
src/bin/resolver/tests/Makefile.am

@@ -37,6 +37,7 @@ run_unittests_LDADD += $(top_builddir)/src/lib/cc/libcc.la
 run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 run_unittests_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la
 run_unittests_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la
 run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la
 run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la
+run_unittests_LDADD += $(top_builddir)/src/lib/server_common/libserver_common.la
 run_unittests_LDADD += $(top_builddir)/src/lib/cache/libcache.la
 run_unittests_LDADD += $(top_builddir)/src/lib/cache/libcache.la
 run_unittests_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la
 run_unittests_LDADD += $(top_builddir)/src/lib/nsas/libnsas.la
 
 

+ 23 - 88
src/bin/resolver/tests/resolver_config_unittest.cc

@@ -24,6 +24,7 @@
 
 
 #include <dns/tests/unittest_util.h>
 #include <dns/tests/unittest_util.h>
 #include <testutils/srv_test.h>
 #include <testutils/srv_test.h>
+#include <testutils/portconfig.h>
 
 
 using namespace std;
 using namespace std;
 using namespace isc::data;
 using namespace isc::data;
@@ -42,7 +43,7 @@ class ResolverConfig : public ::testing::Test {
         {
         {
             server.setDNSService(dnss);
             server.setDNSService(dnss);
         }
         }
-        void invalidTest(const string &JOSN);
+        void invalidTest(const string &JSON, const string& name);
 };
 };
 
 
 TEST_F(ResolverConfig, forwardAddresses) {
 TEST_F(ResolverConfig, forwardAddresses) {
@@ -122,114 +123,48 @@ TEST_F(ResolverConfig, rootAddressConfig) {
 }
 }
 
 
 void
 void
-ResolverConfig::invalidTest(const string &JOSN) {
-    ElementPtr config(Element::fromJSON(JOSN));
-    EXPECT_FALSE(server.updateConfig(config)->equals(
-        *isc::config::createAnswer())) << "Accepted config " << JOSN << endl;
+ResolverConfig::invalidTest(const string &JSON, const string& name) {
+    isc::testutils::portconfig::configRejected(server, JSON, name);
 }
 }
 
 
 TEST_F(ResolverConfig, invalidForwardAddresses) {
 TEST_F(ResolverConfig, invalidForwardAddresses) {
     // Try torturing it with some invalid inputs
     // Try torturing it with some invalid inputs
     invalidTest("{"
     invalidTest("{"
         "\"forward_addresses\": \"error\""
         "\"forward_addresses\": \"error\""
-        "}");
+        "}", "Invalid type");
     invalidTest("{"
     invalidTest("{"
         "\"forward_addresses\": [{}]"
         "\"forward_addresses\": [{}]"
-        "}");
+        "}", "Empty element");
     invalidTest("{"
     invalidTest("{"
         "\"forward_addresses\": [{"
         "\"forward_addresses\": [{"
         "   \"port\": 1.5,"
         "   \"port\": 1.5,"
         "   \"address\": \"192.0.2.1\""
         "   \"address\": \"192.0.2.1\""
-        "}]}");
+        "}]}", "Float port");
     invalidTest("{"
     invalidTest("{"
         "\"forward_addresses\": [{"
         "\"forward_addresses\": [{"
         "   \"port\": -5,"
         "   \"port\": -5,"
         "   \"address\": \"192.0.2.1\""
         "   \"address\": \"192.0.2.1\""
-        "}]}");
+        "}]}", "Negative port");
     invalidTest("{"
     invalidTest("{"
         "\"forward_addresses\": [{"
         "\"forward_addresses\": [{"
         "   \"port\": 53,"
         "   \"port\": 53,"
         "   \"address\": \"bad_address\""
         "   \"address\": \"bad_address\""
-        "}]}");
+        "}]}", "Bad address");
 }
 }
 
 
+// Try setting the addresses directly
 TEST_F(ResolverConfig, listenAddresses) {
 TEST_F(ResolverConfig, 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", 5321));
-    addresses.push_back(pair<string, uint16_t>("::1", 5321));
-    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());
+    isc::testutils::portconfig::listenAddresses(server);
 }
 }
 
 
+// Try setting some addresses and a rollback
 TEST_F(ResolverConfig, listenAddressConfig) {
 TEST_F(ResolverConfig, listenAddressConfig) {
-    // Try putting there some address
-    ElementPtr config(Element::fromJSON("{"
-        "\"listen_on\": ["
-        "   {"
-        "       \"address\": \"127.0.0.1\","
-        "       \"port\": 5321"
-        "   }"
-        "]"
-        "}"));
-    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(5321, server.getListenAddresses()[0].second);
-
-    // As this is example address, the machine should not have it on
-    // any interface
-    config = Element::fromJSON("{"
-        "\"listen_on\": ["
-        "   {"
-        "       \"address\": \"192.0.2.0\","
-        "       \"port\": 5321"
-        "   }"
-        "]"
-        "}");
-    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(5321, server.getListenAddresses()[0].second);
+    isc::testutils::portconfig::listenAddressConfig(server);
 }
 }
 
 
+// Try some invalid configs are rejected
 TEST_F(ResolverConfig, invalidListenAddresses) {
 TEST_F(ResolverConfig, invalidListenAddresses) {
-    // Try torturing it with some invalid inputs
-    invalidTest("{"
-        "\"listen_on\": \"error\""
-        "}");
-    invalidTest("{"
-        "\"listen_on\": [{}]"
-        "}");
-    invalidTest("{"
-        "\"listen_on\": [{"
-        "   \"port\": 1.5,"
-        "   \"address\": \"192.0.2.1\""
-        "}]}");
-    invalidTest("{"
-        "\"listen_on\": [{"
-        "   \"port\": -5,"
-        "   \"address\": \"192.0.2.1\""
-        "}]}");
-    invalidTest("{"
-        "\"listen_on\": [{"
-        "   \"port\": 53,"
-        "   \"address\": \"bad_address\""
-        "}]}");
+    isc::testutils::portconfig::invalidListenAddressConfig(server);
 }
 }
 
 
 // Just test it sets and gets the values correctly
 // Just test it sets and gets the values correctly
@@ -264,28 +199,28 @@ TEST_F(ResolverConfig, timeoutsConfig) {
 TEST_F(ResolverConfig, invalidTimeoutsConfig) {
 TEST_F(ResolverConfig, invalidTimeoutsConfig) {
     invalidTest("{"
     invalidTest("{"
         "\"timeout_query\": \"error\""
         "\"timeout_query\": \"error\""
-        "}");
+        "}", "Wrong query element type");
     invalidTest("{"
     invalidTest("{"
         "\"timeout_query\": -2"
         "\"timeout_query\": -2"
-        "}");
+        "}", "Negative query timeout");
     invalidTest("{"
     invalidTest("{"
         "\"timeout_client\": \"error\""
         "\"timeout_client\": \"error\""
-        "}");
+        "}", "Wrong client element type");
     invalidTest("{"
     invalidTest("{"
         "\"timeout_client\": -2"
         "\"timeout_client\": -2"
-        "}");
+        "}", "Negative client timeout");
     invalidTest("{"
     invalidTest("{"
         "\"timeout_lookup\": \"error\""
         "\"timeout_lookup\": \"error\""
-        "}");
+        "}", "Wrong lookup element type");
     invalidTest("{"
     invalidTest("{"
         "\"timeout_lookup\": -2"
         "\"timeout_lookup\": -2"
-        "}");
+        "}", "Negative lookup timeout");
     invalidTest("{"
     invalidTest("{"
         "\"retries\": \"error\""
         "\"retries\": \"error\""
-        "}");
+        "}", "Wrong retries element type");
     invalidTest("{"
     invalidTest("{"
         "\"retries\": -1"
         "\"retries\": -1"
-        "}");
+        "}", "Negative number of retries");
 }
 }
 
 
 }
 }

+ 1 - 1
src/lib/Makefile.am

@@ -1,2 +1,2 @@
 SUBDIRS = exceptions dns cc config datasrc python xfr bench log \
 SUBDIRS = exceptions dns cc config datasrc python xfr bench log \
-          resolve nsas cache asiolink testutils
+          resolve nsas cache asiolink testutils server_common

+ 26 - 0
src/lib/server_common/Makefile.am

@@ -0,0 +1,26 @@
+SUBDIRS = . tests
+
+AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
+AM_CPPFLAGS += $(BOOST_INCLUDES)
+AM_CXXFLAGS = $(B10_CXXFLAGS)
+
+# Some versions of GCC warn about some versions of Boost regarding
+# missing initializer for members in its posix_time.
+# https://svn.boost.org/trac/boost/ticket/3477
+# But older GCC compilers don't have the flag.
+AM_CXXFLAGS += $(WARNING_NO_MISSING_FIELD_INITIALIZERS_CFLAG)
+
+if USE_CLANGPP
+# clang++ complains about unused function parameters in some boost header
+# files.
+AM_CXXFLAGS += -Wno-unused-parameter
+endif
+
+lib_LTLIBRARIES = libserver_common.la
+libserver_common_la_SOURCES = portconfig.h portconfig.cc
+libserver_common_la_LIBADD = $(top_builddir)/src/lib/exceptions/libexceptions.la
+libserver_common_la_LIBADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
+libserver_common_la_LIBADD += $(top_builddir)/src/lib/cc/libcc.la
+libserver_common_la_LIBADD += $(top_builddir)/src/lib/log/liblog.la
+
+CLEANFILES = *.gcno *.gcda

+ 119 - 0
src/lib/server_common/portconfig.cc

@@ -0,0 +1,119 @@
+// Copyright (C) 2011  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 <server_common/portconfig.h>
+
+#include <asiolink/io_address.h>
+#include <asiolink/dns_service.h>
+#include <log/dummylog.h>
+
+#include <boost/foreach.hpp>
+#include <boost/lexical_cast.hpp>
+
+using namespace std;
+using namespace isc::data;
+using namespace asiolink;
+using isc::log::dlog;
+
+namespace isc {
+namespace server_common {
+namespace portconfig {
+
+AddressList
+parseAddresses(isc::data::ConstElementPtr addresses,
+               const std::string& elemName)
+{
+    AddressList 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(AddressPair(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, elemName + " config element must be a list");
+        }
+    }
+    return (result);
+}
+
+namespace {
+
+void
+setAddresses(DNSService& service, const AddressList& addresses) {
+    service.clearServers();
+    BOOST_FOREACH(const AddressPair &address, addresses) {
+        service.addServer(address.second, address.first);
+    }
+}
+
+}
+
+void
+installListenAddresses(const AddressList& newAddresses,
+                       AddressList& addressStore,
+                       asiolink::DNSService& service)
+{
+    try {
+        dlog("Setting listen addresses:");
+        BOOST_FOREACH(const AddressPair& addr, newAddresses) {
+            dlog(" " + addr.first + ":" +
+                        boost::lexical_cast<string>(addr.second));
+        }
+        setAddresses(service, newAddresses);
+        addressStore = newAddresses;
+    }
+    catch (const exception& e) {
+        /*
+         * We couldn't set it. So return it back. If that fails as well,
+         * we have a problem.
+         *
+         * If that fails, bad luck, but we are useless anyway, so just die
+         * and let boss start us again.
+         */
+        dlog(string("Unable to set new address: ") + e.what(), true);
+        try {
+            setAddresses(service, addressStore);
+        }
+        catch (const exception& e2) {
+            dlog("Unable to recover from error;", true);
+            dlog(string("Rollback failed with: ") + e2.what(), true);
+            abort();
+        }
+        throw; // Let it fly a little bit further
+    }
+}
+
+}
+}
+}

+ 121 - 0
src/lib/server_common/portconfig.h

@@ -0,0 +1,121 @@
+// Copyright (C) 2011  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 ISC_SERVER_COMMON_PORTCONFIG_H
+#define ISC_SERVER_COMMON_PORTCONFIG_H
+
+#include <utility>
+#include <string>
+#include <stdint.h>
+#include <vector>
+
+#include <cc/data.h>
+
+/*
+ * Some forward declarations.
+ */
+namespace asiolink {
+class DNSService;
+}
+
+namespace isc {
+namespace server_common {
+/**
+ * \brief Utilities to configure ports and addresses.
+ *
+ * Here are some utilities to help a server to parse configuration of addresses
+ * and ports and install the configuration.
+ */
+namespace portconfig {
+
+/**
+ * \brief An address-port pair.
+ *
+ * It is just a pair of string for an address and unsigned integer for port
+ * number. Anything more fancy would be an overkill, it is used only to pass
+ * the addresses and ports around as intermediate results.
+ */
+typedef std::pair<std::string, uint16_t> AddressPair;
+
+/// \brief Bunch of address pairs
+typedef std::vector<AddressPair> AddressList;
+
+/**
+ * \brief
+ *
+ * This parses a list of address-port configurations and returns them. The
+ * configuration looks like this:
+ *
+ * \verbatim
+[
+  {
+    "address": "192.0.2.1",
+    "port": 13
+  },
+  {
+    "address": "::",
+    "port": 80
+  }
+]
+ * \endverbatim
+ * \param addresses The configuration element to parse (the list). Empty list,
+ *     null element and null pointer all mean empty list of addresses.
+ * \param elemName The name of the element, used to create descriptions for
+ *     exceptions.
+ * \return Vector of parsed address-port pairs found in the configuration.
+ * \throw isc::data::TypeError if something in the configuration is of a wrong
+ *     type (string passed to a port, element in the list that isn't hash,
+ *     etc).
+ * \throw asiolink::IOError if the provided address string can't be parsed.
+ * \throw BadValue for other invalid configurations (missing port or address
+ *     element in the hash, port number out of range).
+ * \throw std::bad_alloc when allocation fails.
+ */
+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 aborts the application (it assumes if it can't listen
+ * on the new addresses nor on the old ones, the application is useless anyway
+ * and should be restarted by Boss, not to mention that the internal state is
+ * probably broken).
+ *
+ * \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 std::bad_alloc when allocation fails.
+ */
+void
+installListenAddresses(const AddressList& newAddresses,
+                       AddressList& addressStore,
+                       asiolink::DNSService& dnsService);
+
+}
+}
+}
+
+#endif

+ 38 - 0
src/lib/server_common/tests/Makefile.am

@@ -0,0 +1,38 @@
+AM_CPPFLAGS  = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib
+AM_CPPFLAGS += $(BOOST_INCLUDES)
+AM_CPPFLAGS += -I$(top_srcdir)/src/lib/server_common
+AM_CPPFLAGS += -I$(top_builddir)/src/lib/server_common
+AM_CXXFLAGS = $(B10_CXXFLAGS)
+
+AM_LDFLAGS =
+if USE_STATIC_LINK
+AM_LDFLAGS += -static
+endif
+
+# Some versions of GCC warn about some versions of Boost regarding
+# missing initializer for members in its posix_time.
+# https://svn.boost.org/trac/boost/ticket/3477
+# But older GCC compilers don't have the flag.     
+AM_CXXFLAGS += $(WARNING_NO_MISSING_FIELD_INITIALIZERS_CFLAG)
+
+if USE_CLANGPP
+# see ../Makefile.am
+AM_CXXFLAGS += -Wno-unused-parameter
+endif
+
+CLEANFILES = *.gcno *.gcda
+
+TESTS =
+if HAVE_GTEST
+TESTS += run_unittests
+run_unittests_SOURCES  = run_unittests.cc
+run_unittests_SOURCES += portconfig_unittest.cc
+
+run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
+run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
+run_unittests_LDADD = $(GTEST_LDADD)
+
+run_unittests_LDADD += $(top_builddir)/src/lib/server_common/libserver_common.la
+endif
+
+noinst_PROGRAMS = $(TESTS)

+ 182 - 0
src/lib/server_common/tests/portconfig_unittest.cc

@@ -0,0 +1,182 @@
+// Copyright (C) 2011  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 <server_common/portconfig.h>
+
+#include <cc/data.h>
+#include <exceptions/exceptions.h>
+#include <asiolink/asiolink.h>
+
+#include <gtest/gtest.h>
+#include <string>
+
+using namespace isc::server_common::portconfig;
+using namespace isc::data;
+using namespace isc;
+using namespace std;
+using namespace asiolink;
+
+namespace {
+
+/// Testcase for parseAddresses call (struct, nobody cares about private here)
+struct ParseAddresses : public ::testing::Test {
+    AddressList result_;
+    void empty(ElementPtr config, const string& name) {
+        SCOPED_TRACE(name);
+        EXPECT_NO_THROW(result_ = parseAddresses(config, "test"));
+        EXPECT_TRUE(result_.empty());
+    }
+    template<class Exception>
+    void invalidTest(const string& json, const string& name) {
+        SCOPED_TRACE(name);
+        ElementPtr config(Element::fromJSON(json));
+        EXPECT_THROW(parseAddresses(config, "test"), Exception) <<
+            "Should throw " << typeid(Exception).name();
+    }
+};
+
+// Parse valid IPv4 address
+TEST_F(ParseAddresses, ipv4) {
+    ElementPtr config(Element::fromJSON("["
+                                        "   {"
+                                        "       \"address\": \"192.0.2.1\","
+                                        "       \"port\": 53"
+                                        "   }"
+                                        "]"));
+    EXPECT_NO_THROW(result_ = parseAddresses(config, "test"));
+    ASSERT_EQ(1, result_.size());
+    EXPECT_EQ("192.0.2.1", result_[0].first);
+    EXPECT_EQ(53, result_[0].second);
+}
+
+// Parse valid IPv6 address
+TEST_F(ParseAddresses, ipv6) {
+    ElementPtr config(Element::fromJSON("["
+                                        "   {"
+                                        "       \"address\": \"2001:db8::1\","
+                                        "       \"port\": 53"
+                                        "   }"
+                                        "]"));
+    EXPECT_NO_THROW(result_ = parseAddresses(config, "test"));
+    ASSERT_EQ(1, result_.size());
+    EXPECT_EQ("2001:db8::1", result_[0].first);
+    EXPECT_EQ(53, result_[0].second);
+}
+
+// Parse multiple addresses at once
+// (even the ports are different to see they are not mistaken)
+TEST_F(ParseAddresses, multi) {
+    ElementPtr config(Element::fromJSON("["
+                                        "   {"
+                                        "       \"address\": \"2001:db8::1\","
+                                        "       \"port\": 53"
+                                        "   },"
+                                        "   {"
+                                        "       \"address\": \"192.0.2.1\","
+                                        "       \"port\": 54"
+                                        "   }"
+                                        "]"));
+    EXPECT_NO_THROW(result_ = parseAddresses(config, "test"));
+    ASSERT_EQ(2, result_.size());
+    EXPECT_EQ("2001:db8::1", result_[0].first);
+    EXPECT_EQ(53, result_[0].second);
+    EXPECT_EQ("192.0.2.1", result_[1].first);
+    EXPECT_EQ(54, result_[1].second);
+}
+
+// Parse various versions of empty list
+TEST_F(ParseAddresses, empty) {
+    empty(Element::fromJSON("[]"), "Empty list");
+    empty(ElementPtr(new NullElement), "Null element");
+    empty(ElementPtr(), "Null pointer");
+}
+
+// Reject invalid configs
+TEST_F(ParseAddresses, invalid) {
+    invalidTest<TypeError>("{}", "Not a list");
+    invalidTest<BadValue>("[{}]", "Empty element");
+    invalidTest<TypeError>("[{"
+                           "   \"port\": 1.5,"
+                           "   \"address\": \"192.0.2.1\""
+                           "}]", "Float port");
+    invalidTest<BadValue>("[{"
+                          "   \"port\": -5,"
+                          "   \"address\": \"192.0.2.1\""
+                          "}]", "Negative port");
+    invalidTest<BadValue>("[{"
+                          "   \"port\": 1000000,"
+                          "   \"address\": \"192.0.2.1\""
+                          "}]", "Port too big");
+    invalidTest<IOError>("[{"
+                         "   \"port\": 53,"
+                         "   \"address\": \"bad_address\""
+                         "}]", "Bad address");
+}
+
+// Test fixture for installListenAddresses
+struct InstallListenAddresses : public ::testing::Test {
+    InstallListenAddresses() :
+        dnss_(ios_, NULL, NULL, NULL)
+    {
+        valid_.push_back(AddressPair("127.0.0.1", 5288));
+        valid_.push_back(AddressPair("::1", 5288));
+        invalid_.push_back(AddressPair("192.0.2.2", 1));
+    }
+    IOService ios_;
+    DNSService dnss_;
+    AddressList store_;
+    // We should be able to bind to these addresses
+    AddressList valid_;
+    // But this shouldn't work
+    AddressList invalid_;
+    // Check that the store_ addresses are the same as expected
+    void checkAddresses(const AddressList& expected, const string& name) {
+        SCOPED_TRACE(name);
+
+        ASSERT_EQ(expected.size(), store_.size()) <<
+            "Different amount of elements, not checking content";
+        // Run in parallel trough the vectors
+        for (AddressList::const_iterator ei(expected.begin()),
+             si(store_.begin()); ei != expected.end(); ++ei, ++si) {
+            EXPECT_EQ(ei->first, si->first);
+            EXPECT_EQ(ei->second, si->second);
+        }
+    }
+};
+
+// Try switching valid addresses
+TEST_F(InstallListenAddresses, valid) {
+    // First, bind to the valid addresses
+    EXPECT_NO_THROW(installListenAddresses(valid_, store_, dnss_));
+    checkAddresses(valid_, "Valid addresses");
+    // TODO Maybe some test to actually connect to them
+    // Try setting it back to nothing
+    EXPECT_NO_THROW(installListenAddresses(AddressList(), store_, dnss_));
+    checkAddresses(AddressList(), "No addresses");
+    // Try switching back again
+    EXPECT_NO_THROW(installListenAddresses(valid_, store_, dnss_));
+    checkAddresses(valid_, "Valid addresses");
+}
+
+// Try if rollback works
+TEST_F(InstallListenAddresses, rollback) {
+    // Set some addresses
+    EXPECT_NO_THROW(installListenAddresses(valid_, store_, dnss_));
+    checkAddresses(valid_, "Before rollback");
+    // This should not bind them, but should leave the original addresses
+    EXPECT_THROW(installListenAddresses(invalid_, store_, dnss_), IOError);
+    checkAddresses(valid_, "After rollback");
+}
+
+}

+ 26 - 0
src/lib/server_common/tests/run_unittests.cc

@@ -0,0 +1,26 @@
+// Copyright (C) 2011  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 <config.h>
+
+#include <gtest/gtest.h>
+
+#include <dns/tests/unittest_util.h>
+
+int
+main(int argc, char* argv[]) {
+    ::testing::InitGoogleTest(&argc, argv);
+
+    return (RUN_ALL_TESTS());
+}

+ 2 - 0
src/lib/testutils/Makefile.am

@@ -12,3 +12,5 @@ libtestutils_la_SOURCES += dnsmessage_test.h dnsmessage_test.cc
 libtestutils_la_SOURCES += mockups.h
 libtestutils_la_SOURCES += mockups.h
 libtestutils_la_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 libtestutils_la_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 endif
 endif
+
+EXTRA_DIST = portconfig.h

+ 189 - 0
src/lib/testutils/portconfig.h

@@ -0,0 +1,189 @@
+// Copyright (C) 2011  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 TESTUTILS_PORTCONFIG_H
+#define TESTUTILS_PORTCONFIG_H
+
+#include <gtest/gtest.h>
+#include <cc/data.h>
+#include <server_common/portconfig.h>
+
+namespace isc {
+namespace testutils {
+/**
+ * \brief Bits of tests for server port configuration.
+ *
+ * These are bits of tests that can be reused by server classes to check if
+ * configuration of the listening addresses work.
+ *
+ * You can put any of these functions into a TEST_F test and pass the server
+ * to it.
+ *
+ * \todo There's quite a lot of common code in the basic server handling.
+ *     We should refactor it, so both Resolver server and Auth server have
+ *     a common base class. When this is done, the common parts would be put
+ *     there and the tests would be at the base class, not here.
+ */
+namespace portconfig {
+
+/**
+ * \brief Check setting of the listening addresses directly (as a list) works.
+ *
+ * \param server The server to test against.
+ */
+template<class Server>
+void
+listenAddresses(Server& server) {
+    using namespace isc::server_common::portconfig;
+    // Default value should be fully recursive
+    EXPECT_TRUE(server.getListenAddresses().empty());
+
+    // Try putting there some addresses
+    AddressList addresses;
+    addresses.push_back(AddressPair("127.0.0.1", 5321));
+    addresses.push_back(AddressPair("::1", 5321));
+    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());
+}
+
+/**
+ * \brief Check setting of the addresses by config value.
+ *
+ * This passes an listen_on element to the server's updateConfig function.
+ * It tries little bit of switching around. It tries both setting a presumably
+ * valid addresses and then setting something that cant be bound, rolling back
+ * back to original.
+ *
+ * \param server The server object to test against.
+ */
+template<class Server>
+void
+listenAddressConfig(Server& server) {
+    using namespace isc::data;
+    // Try putting there some address
+    ElementPtr config(Element::fromJSON("{"
+                                        "\"listen_on\": ["
+                                        "   {"
+                                        "       \"address\": \"127.0.0.1\","
+                                        "       \"port\": 5321"
+                                        "   }"
+                                        "]"
+                                        "}"));
+    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(5321, server.getListenAddresses()[0].second);
+
+    // As this is example address, the machine should not have it on
+    // any interface
+    config = Element::fromJSON("{"
+                               "\"listen_on\": ["
+                               "   {"
+                               "       \"address\": \"192.0.2.0\","
+                               "       \"port\": 5321"
+                               "   }"
+                               "]"
+                               "}");
+    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(5321, server.getListenAddresses()[0].second);
+
+}
+
+/**
+ * \brief Check that given config is rejected.
+ *
+ * Try if given config is considered invalid by the server and is rejected.
+ * The value is converted from JSON to the data elements and passed to server's
+ * updateConfig method. It should not crash, but return a negative answer.
+ *
+ * It is used internally by invalidListenAddressConfig, but you can use it
+ * to test any other invalid configs.
+ *
+ * \todo It might be better to put it to some other namespace, as this is more
+ *     generic. But right now it is used only here, so until something else
+ *     needs it, it might as well stay here.
+ * \param server The server to test against.
+ * \param JSON Config to use.
+ * \param name It is used in the output if the test fails.
+ */
+template<class Server>
+void
+configRejected(Server& server, const std::string& JSON,
+               const std::string& name)
+{
+    SCOPED_TRACE(name);
+
+    using namespace isc::data;
+    ElementPtr config(Element::fromJSON(JSON));
+    EXPECT_FALSE(server.updateConfig(config)->
+                 equals(*isc::config::createAnswer())) <<
+        "Accepted invalid config " << JSON;
+}
+
+/**
+ * \brief Check some invalid address configs.
+ *
+ * It tries a series of invalid listen_on configs against the server and checks
+ * it is rejected.
+ * \param server The server to check against.
+ */
+template<class Server>
+void
+invalidListenAddressConfig(Server& server) {
+    configRejected(server, "{"
+                   "\"listen_on\": \"error\""
+                   "}", "Wrong element type");
+    configRejected(server, "{"
+                   "\"listen_on\": [{}]"
+                   "}", "Empty address element");
+    configRejected(server, "{"
+                   "\"listen_on\": [{"
+                   "   \"port\": 1.5,"
+                   "   \"address\": \"192.0.2.1\""
+                   "}]}", "Float port");
+    configRejected(server, "{"
+                   "\"listen_on\": [{"
+                   "   \"port\": -5,"
+                   "   \"address\": \"192.0.2.1\""
+                   "}]}", "Negative port");
+    configRejected(server, "{"
+                   "\"listen_on\": [{"
+                   "   \"port\": 1000000,"
+                   "   \"address\": \"192.0.2.1\""
+                   "}]}", "Huge port");
+    configRejected(server, "{"
+                   "\"listen_on\": [{"
+                   "   \"port\": 53,"
+                   "   \"address\": \"bad_address\""
+                   "}]}", "Bad address");
+}
+
+}
+}
+}
+
+#endif