[trac575] Tests for listen_on configuration

It is taken from resolver tests and made generic. Then it is reused for
both resolver and authoritative tests.

src/bin/auth/tests/auth_srv_unittest.cc

@@ -36,6 +36,7 @@
 #include <dns/tests/unittest_util.h>
 #include <testutils/dnsmessage_test.h>
 #include <testutils/srv_test.h>
+#include <testutils/portconfig.h>
 
 using namespace std;
 using namespace isc::cc;
@@ -662,23 +663,7 @@ TEST_F(AuthSrvTest, stop) {
 }
 
 TEST_F(AuthSrvTest, listenAddresses) {
-    // Default value should be not listening to anything
+    isc::testutils::portconfig::listenAddresses(server);
-    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);
-    ASSERT_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 empty list if we ask it to?
-    server.setListenAddresses(addresses);
-    EXPECT_TRUE(server.getListenAddresses().empty());
 }
+
 }

src/bin/auth/tests/config_unittest.cc

@@ -30,6 +30,7 @@
 #include <auth/common.h>
 
 #include <testutils/mockups.h>
+#include <testutils/portconfig.h>
 
 using namespace isc::dns;
 using namespace isc::data;
@@ -39,7 +40,15 @@ using namespace asiolink;
 namespace {
 class AuthConfigTest : public ::testing::Test {
 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;
     MockXfroutClient xfrout;
     AuthSrv server;
@@ -112,6 +121,17 @@ TEST_F(AuthConfigTest, exceptionFromCommit) {
                  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 {
 protected:
     MemoryDatasrcConfigTest() :

src/bin/resolver/tests/resolver_config_unittest.cc

@@ -24,6 +24,7 @@
 
 #include <dns/tests/unittest_util.h>
 #include <testutils/srv_test.h>
+#include <testutils/portconfig.h>
 
 using namespace std;
 using namespace isc::data;
@@ -42,7 +43,7 @@ class ResolverConfig : public ::testing::Test {
         {
             server.setDNSService(dnss);
         }
-        void invalidTest(const string &JOSN);
+        void invalidTest(const string &JSON, const string& name);
 };
 
 TEST_F(ResolverConfig, forwardAddresses) {
@@ -122,117 +123,48 @@ TEST_F(ResolverConfig, rootAddressConfig) {
 }
 
 void
-ResolverConfig::invalidTest(const string &JOSN) {
+ResolverConfig::invalidTest(const string &JSON, const string& name) {
-    ElementPtr config(Element::fromJSON(JOSN));
+    isc::testutils::portconfig::configRejected(server, JSON, name);
-    EXPECT_FALSE(server.updateConfig(config)->equals(
-        *isc::config::createAnswer())) << "Accepted config " << JOSN << endl;
 }
 
 TEST_F(ResolverConfig, invalidForwardAddresses) {
     // Try torturing it with some invalid inputs
     invalidTest("{"
         "\"forward_addresses\": \"error\""
-        "}");
+        "}", "Invalid type");
     invalidTest("{"
         "\"forward_addresses\": [{}]"
-        "}");
+        "}", "Empty element");
     invalidTest("{"
         "\"forward_addresses\": [{"
         "   \"port\": 1.5,"
         "   \"address\": \"192.0.2.1\""
-        "}]}");
+        "}]}", "Float port");
     invalidTest("{"
         "\"forward_addresses\": [{"
         "   \"port\": -5,"
         "   \"address\": \"192.0.2.1\""
-        "}]}");
+        "}]}", "Negative port");
     invalidTest("{"
         "\"forward_addresses\": [{"
         "   \"port\": 53,"
         "   \"address\": \"bad_address\""
-        "}]}");
+        "}]}", "Bad address");
 }
 
+// Try setting the addresses directly
 TEST_F(ResolverConfig, listenAddresses) {
-    // Default value should be fully recursive
+    isc::testutils::portconfig::listenAddresses(server);
-    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());
 }
 
+// Switch addresses
 TEST_F(ResolverConfig, DISABLED_listenAddressConfig) {
-    // Try putting there some address
+    isc::testutils::portconfig::listenAddressConfig(server);
-    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
-    // FIXME: This test aborts, because it tries to rollback and
-    //     it is impossible, since the sockets are not closed.
-    //     Once #388 is solved, enable this test.
-    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);
 }
 
+// Try some invalid configs are rejected
 TEST_F(ResolverConfig, invalidListenAddresses) {
-    // Try torturing it with some invalid inputs
+    isc::testutils::portconfig::invalidListenAddressConfig(server);
-    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\""
-        "}]}");
 }
 
 // Just test it sets and gets the values correctly
@@ -267,28 +199,28 @@ TEST_F(ResolverConfig, timeoutsConfig) {
 TEST_F(ResolverConfig, invalidTimeoutsConfig) {
     invalidTest("{"
         "\"timeout_query\": \"error\""
-        "}");
+        "}", "Wrong query element type");
     invalidTest("{"
         "\"timeout_query\": -2"
-        "}");
+        "}", "Negative query timeout");
     invalidTest("{"
         "\"timeout_client\": \"error\""
-        "}");
+        "}", "Wrong client element type");
     invalidTest("{"
         "\"timeout_client\": -2"
-        "}");
+        "}", "Negative client timeout");
     invalidTest("{"
         "\"timeout_lookup\": \"error\""
-        "}");
+        "}", "Wrong lookup element type");
     invalidTest("{"
         "\"timeout_lookup\": -2"
-        "}");
+        "}", "Negative lookup timeout");
     invalidTest("{"
         "\"retries\": \"error\""
-        "}");
+        "}", "Wrong retries element type");
     invalidTest("{"
         "\"retries\": -1"
-        "}");
+        "}", "Negative number of retries");
 }
 
 }

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_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 endif
+
+EXTRA_DIST = portconfig.h

src/lib/testutils/portconfig.h

@@ -0,0 +1,195 @@
+// 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.
+ * \todo This tests currently aborts due to #388 (it is impossible to close
+ *     sockets, so it is impossible to configure, deconfigure and then
+ *     configure them again). Once #388 is solved, enable the tests.
+ */
+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
+    // FIXME: This test aborts, because it tries to rollback and
+    //     it is impossible, since the sockets are not closed.
+    //     Once #388 is solved, enable this test.
+    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