[master] Merge branch 'trac3918' (KEA_SOCKET_TEST_DIR variable)

Conflicts:
	src/lib/util/tests/socketsession_unittest.cc

doc/Makefile.am

@@ -4,6 +4,7 @@ EXTRA_DIST  = version.ent.in differences.txt Doxyfile Doxyfile-xml
 EXTRA_DIST += devel/config-backend.dox
 EXTRA_DIST += devel/contribute.dox
 EXTRA_DIST += devel/mainpage.dox
+EXTRA_DIST += devel/qa.dox
 
 EXTRA_DIST += images/isc-logo.png
 

doc/devel/contribute.dox

@@ -1,4 +1,4 @@
-// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013,2015  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
@@ -78,33 +78,7 @@ change. That is even more true for new code: if you write a new
 function, method or a class, you definitely should write unit-tests
 for it.
 
-Kea uses the Google C++ Testing Framework (also called googletest or
-gtest) as a base for our C++ unit-tests. See
-http://code.google.com/p/googletest/ for details. We still have some Python
-unit-tests that we inherited from BIND10 days, but those tests are being
-removed, so please do not develop any new Python tests in Kea. (If you
-want to write DHCP tests in Python, we encourage you to take a look
-at ISC Forge: http://kea.isc.org/wiki/IscForge). You must
-have \c gtest installed or at least extracted in a directory before
-compiling Kea unit-tests. To enable unit-tests in Kea, use:
-
-@code
-./configure --with-gtest=/path/to/your/gtest/dir
-@endcode
-
-or
-
-@code
-./configure --with-gtest-source=/path/to/your/gtest/dir
-@endcode
-
-Depending on how you compiled or installed \c gtest (e.g. from sources
-or using some package management system) one of those two switches will
-find \c gtest. After that you make run unit-tests:
-
-@code
-make check
-@endcode
+See @ref qaUnitTests for instructions on how to run unit-tests.
 
 If you happen to add new files or have modified any \c Makefile.am
 files, it is also a good idea to check if you haven't broken the
@@ -162,8 +136,9 @@ critical for whatever reason, it may also be mentioned in release notes.
 @section contributorGuideExtra Extra steps
 
 If you are interested in knowing the results of more in-depth testing,
-you are welcome to visit the Kea build farm:
-http://git.kea.isc.org/~tester/builder/KEA-builder-new.html.  This is a
+you are welcome to visit the ISC Jenkins page: https://jenkins.isc.org
+(Our old Kea build farm http://git.kea.isc.org/~tester/builder/KEA-builder-new.html
+is being migrated to Jenkins).  This is a
 live result page with all tests being run on various systems.  Besides
 basic unit-tests, we also have reports from valgrind (memory debugger),
 cppcheck and clang-analyzer (static code analyzers), Lettuce system

doc/devel/mainpage.dox

@@ -105,6 +105,9 @@
  *   - @subpage configBackendAdding
  * - @subpage perfdhcpInternals
  *
+ * @section qa Quality Assurance
+ *   - @subpage qaUnitTests
+ *
  * @section miscellaneousTopics Miscellaneous Topics
  * - @subpage logKeaLogging
  *   - @subpage logBasicIdeas

doc/devel/qa.dox

@@ -0,0 +1,71 @@
+// Copyright (C) 2015  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.
+
+/**
+
+@page qa Kea Quality Assurance processes
+
+ @section qaUnitTests Unit-tests
+
+Kea uses the Google C++ Testing Framework (also called googletest or gtest) as a
+base for our C++ unit-tests. See http://code.google.com/p/googletest/ for
+details. We used to have Python unit-tests that were inherited from BIND10
+days. Those tests are removed now, so please do not develop any new Python
+tests in Kea. If you want to write DHCP tests in Python, we encourage you to
+take a look at ISC Forge: http://kea.isc.org/wiki/IscForge. You must have \c
+gtest installed or at least extracted in a directory before compiling Kea
+unit-tests. To enable unit-tests in Kea, use:
+
+@code
+./configure --with-gtest=/path/to/your/gtest/dir
+@endcode
+
+or
+
+@code
+./configure --with-gtest-source=/path/to/your/gtest/dir
+@endcode
+
+Depending on how you compiled or installed \c gtest (e.g. from sources
+or using some package management system) one of those two switches will
+find \c gtest. After that you make run unit-tests:
+
+@code
+make check
+
+@endcode
+
+The following environment variable can affect unit-tests:
+
+- KEA_SOCKET_TEST_DIR - if set, it specifies the directory where Unix
+  sockets are created. There's OS limitation on how long a Unix socket
+  path can be. It is typcially slightly over 100 characters. If you
+  happen to build and run unit-tests in deeply nested directories, this
+  may become a problem. KEA_SOCKET_TEST_DIR can be specified to instruct
+  unit-test to use a different directory. Must not end with slash (e.g.
+  /tmp).
+
+- KEA_LOCKFILE_DIR - Specifies a directory where the logging system should
+  create its lock file. If not specified, it is prefix/var/run/kea, where prefix
+  defaults to /usr/local. This variable must not end with a slash. There is one
+  special value: "none", which instructs Kea to not create lock file at
+  all. This may cause issues if several processes log to the same file.
+  Also see Kea User's Guide, section 15.3.
+
+- KEA_LOGGER_DESTINATION - Specifies logging destination. If not set, logged
+  messages will not be recorded anywhere. There are 3 special values:
+  stdout, stderr and syslog. Any other value is interpreted as a filename.
+  Also see Kea User's Guide, section 15.3.
+
+ */

doc/guide/dhcp4-srv.xml

@@ -2876,6 +2876,15 @@ temporarily override a list of interface names and listen on all interfaces.
       </para>
 
       <para>
+	The length of the path specified by the <command>socket-name</command>
+	parameter is restricted by the maximum length for the unix socket name
+	on your operating system, i.e. the size of the <command>sun_path</command>
+	field in the <command>sockaddr_un</command> structure, decreased by 1.
+	This value varies on different operating systems between 91 and 107
+	characters. The typical values are 107 on Linux and 103 on FreeBSD.
+      </para>
+
+      <para>
         Communication over control channel is conducted using JSON structures.
         See the Control Channel section in the Kea Developer's Guide for more details.
       </para>

doc/guide/dhcp6-srv.xml

@@ -2864,6 +2864,15 @@ should include options from the isc option space:
       </para>
 
       <para>
+	The length of the path specified by the <command>socket-name</command>
+	parameter is restricted by the maximum length for the unix socket name
+	on your operating system, i.e. the size of the <command>sun_path</command>
+	field in the <command>sockaddr_un</command> structure, decreased by 1.
+	This value varies on different operating systems between 91 and 107
+	characters. The typical values are 107 on Linux and 103 on FreeBSD.
+      </para>
+
+      <para>
         Communication over control channel is conducted using JSON structures.
         See the Control Channel section in the Kea Developer's Guide for more details.
       </para>

src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc

@@ -28,6 +28,7 @@
 
 #include <sys/select.h>
 #include <sys/ioctl.h>
+#include <cstdlib>
 
 using namespace std;
 using namespace isc::config;
@@ -71,8 +72,16 @@ public:
             return (false);
         }
 
-        // Prepare socket address
         struct sockaddr_un srv_addr;
+        if (socket_path.size() > sizeof(srv_addr.sun_path) - 1) {
+            ADD_FAILURE() << "Socket path specified (" << socket_path
+                          << ") is larger than " << (sizeof(srv_addr.sun_path) - 1)
+                          << " allowed.";
+            disconnectFromServer();
+            return (false);
+        }
+
+        // Prepare socket address
         memset(&srv_addr, 0, sizeof(srv_addr));
         srv_addr.sun_family = AF_UNIX;
         strncpy(srv_addr.sun_path, socket_path.c_str(),
@@ -223,7 +232,12 @@ public:
     boost::shared_ptr<NakedControlledDhcpv6Srv> server_;
 
     CtrlChannelDhcpv6SrvTest() {
-        socket_path_ = string(TEST_DATA_BUILDDIR) + "/kea6.sock";
+        const char* env = getenv("KEA_SOCKET_TEST_DIR");
+        if (env) {
+            socket_path_ = string(env) + "/kea6.sock";
+        } else {
+            socket_path_ = string(TEST_DATA_BUILDDIR) + "/kea6.sock";
+        }
         reset();
     }
 
@@ -267,8 +281,9 @@ public:
         ASSERT_TRUE(answer);
 
         int status = 0;
-        isc::config::parseAnswer(status, answer);
-        ASSERT_EQ(0, status);
+        ConstElementPtr txt = isc::config::parseAnswer(status, answer);
+        // This should succeed. If not, print the error message.
+        ASSERT_EQ(0, status) << txt->str();
 
         // Now check that the socket was indeed open.
         ASSERT_GT(isc::config::CommandMgr::instance().getControlSocketFD(), -1);

src/lib/config/command_socket_factory.cc

@@ -63,6 +63,17 @@ private:
     /// @return socket file descriptor
     int createUnixSocket(const std::string& file_name) {
 
+        struct sockaddr_un addr;
+
+        // string.size() returns number of bytes (without trailing zero)
+        // we need 1 extra byte for terminating 0.
+        if (file_name.size() > sizeof(addr.sun_path) - 1) {
+            isc_throw(SocketError, "Failed to open socket: path specified ("
+                      << file_name << ") is longer (" << file_name.size()
+                      << " bytes) than allowed "
+                      << (sizeof(addr.sun_path) - 1) << " bytes.");
+        }
+
         int fd = socket(AF_UNIX, SOCK_STREAM, 0);
         if (fd == -1) {
             isc_throw(isc::config::SocketError, "Failed to create AF_UNIX socket:"
@@ -83,10 +94,9 @@ private:
         }
 
         // Now bind the socket to the specified path.
-        struct sockaddr_un addr;
         memset(&addr, 0, sizeof(addr));
         addr.sun_family = AF_UNIX;
-        strncpy(addr.sun_path, file_name.c_str(), sizeof(addr.sun_path)-1);
+        strncpy(addr.sun_path, file_name.c_str(), sizeof(addr.sun_path) - 1);
         if (bind(fd, (struct sockaddr*)&addr, sizeof(addr))) {
             const char* errmsg = strerror(errno);
             ::close(fd);

src/lib/config/tests/command_socket_factory_unittests.cc

@@ -19,6 +19,7 @@
 #include <config/command_socket.h>
 #include <config/command_socket_factory.h>
 #include <cstdio>
+#include <cstdlib>
 
 using namespace isc::config;
 using namespace isc::data;
@@ -28,7 +29,9 @@ class CommandSocketFactoryTest : public ::testing::Test {
 public:
 
     /// Default constructor
-    CommandSocketFactoryTest() {
+    CommandSocketFactoryTest()
+        :SOCKET_NAME(getSocketPath()) {
+
         // Remove any stale socket files
         remove(SOCKET_NAME.c_str());
     }
@@ -40,12 +43,25 @@ public:
         remove(SOCKET_NAME.c_str());
     }
 
-    static const std::string SOCKET_NAME;
-};
+    /// @brief Returns socket path (using either hardcoded path or env variable)
+    /// @return path to the unix socket
+    std::string getSocketPath() {
+
+        std::string socket_path;
+        const char* env = getenv("KEA_SOCKET_TEST_DIR");
+        if (env) {
+            socket_path = std::string(env) + "/test-socket";
+        } else {
+            socket_path = std::string(TEST_DATA_BUILDDIR) + "/test-socket";
+        }
+        return (socket_path);
+    }
 
-const std::string CommandSocketFactoryTest::SOCKET_NAME =
-    std::string(TEST_DATA_BUILDDIR) + "/test-socket";
+    std::string SOCKET_NAME;
+};
 
+// This test verifies that a Unix socket can be opened properly and that input
+// parameters (socket-type and socket-name) are verified.
 TEST_F(CommandSocketFactoryTest, unixCreate) {
     // Null pointer is obviously a bad idea.
     EXPECT_THROW(CommandSocketFactory::create(ConstElementPtr()),
@@ -75,3 +91,12 @@ TEST_F(CommandSocketFactoryTest, unixCreate) {
     EXPECT_NO_THROW(sock->close());
 }
 
+// This test checks that when unix path is too long, the socket cannot be opened.
+TEST_F(CommandSocketFactoryTest, unixCreateTooLong) {
+    ElementPtr socket_info = Element::fromJSON("{ \"socket-type\": \"unix\","
+        "\"socket-name\": \"/tmp/toolongtoolongtoolongtoolongtoolongtoolong"
+        "toolongtoolongtoolongtoolongtoolongtoolongtoolongtoolongtoolong"
+        "\" }");
+
+    EXPECT_THROW(CommandSocketFactory::create(socket_info), SocketError);
+}

src/lib/util/tests/socketsession_unittest.cc

@@ -25,6 +25,7 @@
 
 #include <cerrno>
 #include <cstring>
+#include <cstdlib>
 
 #include <algorithm>
 #include <string>
@@ -51,7 +52,6 @@ using namespace isc::util::io::internal;
 
 namespace {
 
-const char* const TEST_UNIX_FILE = TEST_DATA_TOPBUILDDIR "/test.unix";
 const char* const TEST_PORT = "53535";
 const char* const TEST_PORT2 = "53536"; // use this in case we need 2 ports
 const char TEST_DATA[] = "Kea test";
@@ -155,13 +155,30 @@ private:
 
 class ForwardTest : public ::testing::Test {
 protected:
-    ForwardTest() : listen_fd_(-1), forwarder_(TEST_UNIX_FILE),
+
+    /// @brief Returns socket path (using either hardcoded path or env variable)
+    /// @return path to the unix socket
+    std::string getSocketPath() {
+
+        std::string socket_path;
+        const char* env = getenv("KEA_SOCKET_TEST_DIR");
+        if (env) {
+            socket_path = string(env) + "/test.unix";
+        } else {
+            socket_path = string(TEST_DATA_BUILDDIR) + "/test.unix";
+        }
+        return (socket_path);
+    }
+
+    ForwardTest() : listen_fd_(-1), forwarder_(getSocketPath()),
                     large_text_(65535, 'a'),
-                    test_un_len_(2 + strlen(TEST_UNIX_FILE))
+                    test_un_len_(2 + strlen(getSocketPath().c_str()))
     {
-        remove(TEST_UNIX_FILE);
+        std::string unix_file = getSocketPath();
+
+        remove(unix_file.c_str());
         test_un_.sun_family = AF_UNIX;
-        strncpy(test_un_.sun_path, TEST_UNIX_FILE, sizeof(test_un_.sun_path));
+        strncpy(test_un_.sun_path, unix_file.c_str(), sizeof(test_un_.sun_path));
 #ifdef HAVE_SA_LEN
         test_un_.sun_len = test_un_len_;
 #endif
@@ -171,7 +188,7 @@ protected:
         if (listen_fd_ != -1) {
             close(listen_fd_);
         }
-        remove(TEST_UNIX_FILE);
+        remove(getSocketPath().c_str());
     }
 
     // Start an internal "socket session server".
@@ -238,7 +255,7 @@ protected:
     // and it's a TCP socket, it will also start listening to new connection
     // requests.
     int createSocket(int family, int type, int protocol,
-                     const SockAddrInfo& sainfo, bool do_listen) 
+                     const SockAddrInfo& sainfo, bool do_listen)
     {
         int s = socket(family, type, protocol);
         if (s < 0) {
@@ -378,7 +395,7 @@ TEST_F(ForwardTest, connect) {
     EXPECT_THROW(forwarder.close(), BadValue);
 
     // Set up the receiver and connect.  It should succeed.
-    SocketSessionForwarder forwarder2(TEST_UNIX_FILE);
+    SocketSessionForwarder forwarder2(getSocketPath().c_str());
     startListen();
     forwarder2.connectToReceiver();
     // And it can be closed successfully.
@@ -396,14 +413,14 @@ TEST_F(ForwardTest, connect) {
     // Connect then destroy.  Should be internally closed, but unfortunately
     // it's not easy to test it directly.  We only check no disruption happens.
     SocketSessionForwarder* forwarderp =
-        new SocketSessionForwarder(TEST_UNIX_FILE);
+        new SocketSessionForwarder(getSocketPath().c_str());
     forwarderp->connectToReceiver();
     delete forwarderp;
 }
 
 TEST_F(ForwardTest, close) {
     // can't close before connect
-    EXPECT_THROW(SocketSessionForwarder(TEST_UNIX_FILE).close(), BadValue);
+    EXPECT_THROW(SocketSessionForwarder(getSocketPath().c_str()).close(), BadValue);
 }
 
 void
@@ -674,7 +691,7 @@ TEST_F(ForwardTest, badPush) {
 }
 
 // A subroutine for pushTooFast, continuously pushing socket sessions
-// with full-size DNS messages (65535 bytes) without receiving them.  
+// with full-size DNS messages (65535 bytes) without receiving them.
 // the push attempts will eventually fill the socket send buffer and trigger
 // an exception.  Unfortunately exactly how many we can forward depends on
 // the internal system implementation; it should be close to 3, because