Browse Source

[master] Merge branch 'master' of ssh://git.kea.isc.org/git/kea

Francis Dupont 8 years ago
parent
commit
94e8a6e4ae

+ 15 - 2
ChangeLog

@@ -1,4 +1,17 @@
-1200	[func]		tmark
+1202.	[func]		tomek
+	Parsers for mac-sources, control-socket, and relay-info converted
+	to SimpleParser. It is no longer accepted to specify empty
+	mac-sources. Either specify actual values in mac-sources or
+	don't specify mac-sources at all.
+	(Trac #5032, git f1c9dee0936b48be28f890ffd428fbdeb87c12ea)
+
+1201.	[func]		tmark
+	kea-dhcp4 and kea-dhcp6 now support the "set-config" command.
+	The command causes the server to replace its current configuration
+	with the configuration supplied as the command's argument.
+	(Trac #5046, git 4afbdcf89c9f83d944f774d05bd401d3f2768d10)
+
+1200.	[func]		tmark
 	kea-dhcp4 and kea-dhcp6 now support the Command Channel "libreload"
 	kea-dhcp4 and kea-dhcp6 now support the Command Channel "libreload"
 	command.  The command causes the server to unload and then load all
 	command.  The command causes the server to unload and then load all
 	currently loaded hook libraries.
 	currently loaded hook libraries.
@@ -18,7 +31,7 @@
 	Created kea-ctrl-agent. This application currently doesn't
 	Created kea-ctrl-agent. This application currently doesn't
 	do anything. Future tickets will add REST API to allow for
 	do anything. Future tickets will add REST API to allow for
 	managing Kea services.
 	managing Kea services.
-        (Trac #5075, git 1ec7586da5ae1474b52d5a395fb80ee37d6d568e)
+	(Trac #5075, git 1ec7586da5ae1474b52d5a395fb80ee37d6d568e)
 
 
 1197.	[doc]		tomek
 1197.	[doc]		tomek
 	Configuration examples now set lfc-interval to a default
 	Configuration examples now set lfc-interval to a default

+ 16 - 1
doc/examples/kea4/advanced.json

@@ -44,6 +44,14 @@
         "lfc-interval": 3600
         "lfc-interval": 3600
     },
     },
 
 
+    // This defines a control socket. If defined, Kea will open a UNIX socket
+    // and will listen for incoming commands. See section 15 of the Kea User's
+    // Guide for list of supported commands.
+    "control-socket": {
+        "socket-type": "unix",
+        "socket-name": "/tmp/kea4-ctrl-socket"
+    },
+
     // Addresses will be assigned with a lifetime of 4000 seconds.
     // Addresses will be assigned with a lifetime of 4000 seconds.
     // The client is told to start renewing after 1000 seconds. If the server
     // The client is told to start renewing after 1000 seconds. If the server
     // does not respond within 2000 seconds of the lease being granted, client
     // does not respond within 2000 seconds of the lease being granted, client
@@ -83,7 +91,14 @@
         },
         },
         {
         {
             "pools": [ { "pool": "192.0.4.1 - 192.0.4.254" } ],
             "pools": [ { "pool": "192.0.4.1 - 192.0.4.254" } ],
-            "subnet": "192.0.4.0/24"
+            "subnet": "192.0.4.0/24",
+
+            // Sometimes the relay may use an IPv4 address that does not match
+            // the subnet. This is discouraged, but there are valid cases when it
+            // makes sense. One case is when there is a shared subnet.
+            "relay": {
+                "ip-address": "192.168.1.1"
+            }
         }
         }
     ]
     ]
 },
 },

+ 72 - 51
doc/examples/kea6/advanced.json

@@ -1,77 +1,90 @@
-# This is an example configuration file for DHCPv6 server in Kea.
-# It attempts to showcase some of the more advanced features.
-# Topology wise, it's a basic scenario with one IPv6 subnet configured.
-# It is assumed that one subnet (2001:db8:1::/64) is available directly
-# over ethX interface.
-#
-# The following features are currently showcased here:
-# 1. Configuration of MAC/hardware address sources in DHCPv6
+// This is an example configuration file for DHCPv6 server in Kea.
+// It attempts to showcase some of the more advanced features.
+// Topology wise, it's a basic scenario with one IPv6 subnet configured.
+// It is assumed that one subnet (2001:db8:1::/64) is available directly
+// over ethX interface.
+//
+// The following features are currently showcased here:
+// 1. Configuration of MAC/hardware address sources in DHCPv6
+// 2. RSOO (Relay supplied options) - Some relays may insert options with the
+//    intention for the server to insert them into client directed messages.
+// 3. Control socket. Kea can open a socket and listen for incoming
+//    commands.
 
 
 { "Dhcp6":
 { "Dhcp6":
 
 
 {
 {
-# Kea is told to listen on ethX network interface only.
+  // Kea is told to listen on ethX network interface only.
   "interfaces-config": {
   "interfaces-config": {
     "interfaces": [ "ethX" ]
     "interfaces": [ "ethX" ]
   },
   },
 
 
-# We need to specify the the database used to store leases. As of
-# September 2016, four database backends are supported: MySQL,
-# PostgreSQL, Cassandra, and the in-memory database, Memfile.
-# We will use memfile  because it doesn't require any prior set up.
+  // We need to specify the the database used to store leases. As of
+  // September 2016, four database backends are supported: MySQL,
+  // PostgreSQL, Cassandra, and the in-memory database, Memfile.
+  // We will use memfile  because it doesn't require any prior set up.
   "lease-database": {
   "lease-database": {
       "type": "memfile",
       "type": "memfile",
       "lfc-interval": 3600
       "lfc-interval": 3600
   },
   },
 
 
-# Kea 0.9.1 introduced MAC/hardware addresses support in DHCPv6. There is
-# no single reliable method of getting MAC address information in DHCPv6.
-# Kea supports several methods. Depending on your network set up, some
-# methods may be more preferable than others, hence the configuration
-# parameter. 'mac-sources' is a list of methods. Allowed parameters are:
-# any, raw, duid, ipv6-link-local, client-link-addr-option, rfc6939 (which
-# is an alias for client-link-addr-option), remote-id, rfc4649 (which is an
-# alias for remote-id, subscriber-id, rfc4580 (which is an alias for
-# subscriber-id) and docsis.
-#
-# Note that the order matters. Methods are attempted one by one in the order
-# specified until hardware address is obtained. If you don't care which method
-# is used, using 'any' is marginally faster than enumerating them all.
-#
-# If mac-sources are not specified, a default value of 'any' is used.
+// Kea 0.9.1 introduced MAC/hardware addresses support in DHCPv6. There is
+// no single reliable method of getting MAC address information in DHCPv6.
+// Kea supports several methods. Depending on your network set up, some
+// methods may be more preferable than others, hence the configuration
+// parameter. 'mac-sources' is a list of methods. Allowed parameters are:
+// any, raw, duid, ipv6-link-local, client-link-addr-option, rfc6939 (which
+// is an alias for client-link-addr-option), remote-id, rfc4649 (which is an
+// alias for remote-id, subscriber-id, rfc4580 (which is an alias for
+// subscriber-id) and docsis.
+//
+// Note that the order matters. Methods are attempted one by one in the order
+// specified until hardware address is obtained. If you don't care which method
+// is used, using 'any' is marginally faster than enumerating them all.
+//
+// If mac-sources are not specified, a default value of 'any' is used.
   "mac-sources": [ "client-link-addr-option", "duid", "ipv6-link-local" ],
   "mac-sources": [ "client-link-addr-option", "duid", "ipv6-link-local" ],
 
 
-# RFC6422 defines a mechanism called relay-supplied options option. The relay
-# agent may insert certain options that the server will echo back to the
-# client, if certain criteria are met. One condition is that the option must
-# be RSOO-enabled (i.e. allowed to be echoed back). IANA maintains a list
-# of those options here:
-# http://www.iana.org/assignments/dhcpv6-parameters/dhcpv6-parameters.xhtml#options-relay-supplied
-# However, it is possible to allow the server to echo back additional options.
-# This entry marks options 110, 120 and 130 as RSOO-enabled.
+// RFC6422 defines a mechanism called relay-supplied options option. The relay
+// agent may insert certain options that the server will echo back to the
+// client, if certain criteria are met. One condition is that the option must
+// be RSOO-enabled (i.e. allowed to be echoed back). IANA maintains a list
+// of those options here:
+// http://www.iana.org/assignments/dhcpv6-parameters/dhcpv6-parameters.xhtml#options-relay-supplied
+// However, it is possible to allow the server to echo back additional options.
+// This entry marks options 110, 120 and 130 as RSOO-enabled.
    "relay-supplied-options": [ "110", "120", "130" ],
    "relay-supplied-options": [ "110", "120", "130" ],
 
 
-# Addresses will be assigned with preferred and valid lifetimes
-# being 3000 and 4000, respectively. Client is told to start
-# renewing after 1000 seconds. If the server does not respond
-# after 2000 seconds since the lease was granted, client is supposed
-# to start REBIND procedure (emergency renewal that allows switching
-# to a different server).
+
+    // This defines a control socket. If defined, Kea will open a UNIX socket
+    // and will listen for incoming commands. See section 15 of the Kea User's
+    // Guide for list of supported commands.
+    "control-socket": {
+        "socket-type": "unix",
+        "socket-name": "/tmp/kea6-ctrl-socket"
+    },
+
+// Addresses will be assigned with preferred and valid lifetimes
+// being 3000 and 4000, respectively. Client is told to start
+// renewing after 1000 seconds. If the server does not respond
+// after 2000 seconds since the lease was granted, client is supposed
+// to start REBIND procedure (emergency renewal that allows switching
+// to a different server).
   "preferred-lifetime": 3000,
   "preferred-lifetime": 3000,
   "valid-lifetime": 4000,
   "valid-lifetime": 4000,
   "renew-timer": 1000,
   "renew-timer": 1000,
   "rebind-timer": 2000,
   "rebind-timer": 2000,
 
 
-# The following list defines subnets. Each subnet consists of at
-# least subnet and pool entries.
+// The following list defines subnets. Each subnet consists of at
+// least subnet and pool entries.
   "subnet6": [
   "subnet6": [
     {
     {
       "pools": [ { "pool": "2001:db8:1::/80" } ],
       "pools": [ { "pool": "2001:db8:1::/80" } ],
 
 
-# This defines PD (prefix delegation) pools. In this case
-# we have only one pool. That consists of /64 prefixes
-# being delegated out of large /48 pool. Each delegated
-# prefix will contain an excluded-prefix option.
+// This defines PD (prefix delegation) pools. In this case
+// we have only one pool. That consists of /64 prefixes
+// being delegated out of large /48 pool. Each delegated
+// prefix will contain an excluded-prefix option.
       "pd-pools": [
       "pd-pools": [
       {
       {
           "prefix": "2001:db8:abcd::",
           "prefix": "2001:db8:abcd::",
@@ -82,13 +95,21 @@
       }
       }
       ],
       ],
       "subnet": "2001:db8:1::/64",
       "subnet": "2001:db8:1::/64",
-      "interface": "ethX"
+      "interface": "ethX",
+
+      // Sometimes the relay may use an odd IPv6 address that's not matching
+      // the subnet. This is discouraged, but there are valid cases when it
+      // makes sense. One case is when the relay has only link-local address
+      // and another is when there is a shared subnet scenario.
+      "relay": {
+          "ip-address": "3000::1"
+      }
     }
     }
   ]
   ]
 },
 },
 
 
-# The following configures logging. It assumes that messages with at least
-# informational level (info, warn, error and fatal) should be logged to stdout.
+// The following configures logging. It assumes that messages with at least
+// informational level (info, warn, error and fatal) should be logged to stdout.
 "Logging": {
 "Logging": {
     "loggers": [
     "loggers": [
         {
         {

+ 61 - 0
doc/guide/ctrl-channel.xml

@@ -184,6 +184,65 @@ will be sent to Kea and the responses received from Kea printed to standard outp
       </para>
       </para>
     </section> <!-- end of command-list-commands -->
     </section> <!-- end of command-list-commands -->
 
 
+    <section id="command-set-config">
+      <title>set-config</title>
+
+      <para>
+    The <emphasis>set-config</emphasis> command instructs the server to replace
+    its current configuration with the new configuration supplied in the
+    command's arguments. The supplied configuration is expected to be the full
+    configuration for the target server along with an optional Logger
+    configuration.  While optional, the Logger configuration is highly
+    recommended as without it the server will revert to its default logging
+    configuration. The structure of the command is as follows:
+      </para>
+<screen>
+{
+    "command": "set-config",
+    "arguments":  {
+        "&#60;server&#62;": {
+        },
+        "Logging": {
+        }
+     }
+}
+</screen>
+      <para>
+    where &#60;server&#62; is the configuration element name for a given server
+    such as "Dhcp4" or "Dhcp6".  For example:
+      </para>
+<screen>
+{
+    "command": "set-config",
+    "arguments":  {
+        "Dhcp6": {
+            :
+        },
+        "Logging": {
+            :
+        }
+     }
+}
+</screen>
+      <para>
+    If the new configuration proves to be invalid the server will retain
+    its current configuration.  Please note that the new configuration is
+    retained in memory only.  If the server is restarted or a configuration
+    reload is triggered via a signal, the server will use the configuration
+    stored in its configuration file.
+
+	The server's response will contain a numeric code, "result" (0 for success,
+    non-zero on failure), and  a string, "text", describing the outcome:
+<screen>
+    {"result": 0, "text": "Configuration successful." }
+
+    or
+
+    {"result": 1, "text": "unsupported parameter: BOGUS (&#60;string&#62;:16:26)" }
+</screen>
+      </para>
+    </section> <!-- end of command-set-config -->
+
     <section id="command-shutdown">
     <section id="command-shutdown">
       <title>shutdown</title>
       <title>shutdown</title>
 
 
@@ -205,6 +264,8 @@ will be sent to Kea and the responses received from Kea printed to standard outp
       </para>
       </para>
     </section> <!-- end of command-shutdown -->
     </section> <!-- end of command-shutdown -->
 
 
+
+
     </section> <!-- end of commands supported by both servers -->
     </section> <!-- end of commands supported by both servers -->
 
 
   </chapter>
   </chapter>

+ 25 - 9
doc/guide/dhcp4-srv.xml

@@ -3203,6 +3203,9 @@ src/lib/dhcpsrv/cfg_host_operations.cc -->
 </screen>
 </screen>
       </para>
       </para>
 
 
+      <para>If "relay" is specified, the "ip-address" parameter within
+      it is mandatory.</para>
+
     </section>
     </section>
 
 
       <section id="dhcp4-srv-example-client-class-relay">
       <section id="dhcp4-srv-example-client-class-relay">
@@ -3649,17 +3652,30 @@ src/lib/dhcpsrv/cfg_host_operations.cc -->
 
 
       <para>
       <para>
         Communication over control channel is conducted using JSON structures.
         Communication over control channel is conducted using JSON structures.
-        See the Control Channel section in the Kea Developer's Guide for more details.
+        See the Control Channel section in the Kea Developer's Guide for more
+        details.
+      </para>
+
+      <para>The DHCPv4 server supports the following operational commands:
+        <itemizedlist>
+            <listitem>leases-reclaim</listitem>
+            <listitem>list-commands</listitem>
+            <listitem>set-config</listitem>
+            <listitem>shutdown</listitem>
+        </itemizedlist>
+         as described in <xref linkend="commands-common"/>.  In addition,
+         it supports the following statistics related commands:
+        <itemizedlist>
+            <listitem>statistic-get</listitem>
+            <listitem>statistic-reset</listitem>
+            <listitem>statistic-remove</listitem>
+            <listitem>statistic-get-all</listitem>
+            <listitem>statistic-reset-all</listitem>
+            <listitem>statistic-remove-all</listitem>
+        </itemizedlist>
+        as described here <xref linkend="command-stats"/>.
       </para>
       </para>
 
 
-      <para>The DHCPv4 server supports <command>statistic-get</command>,
-      <command>statistic-reset</command>, <command>statistic-remove</command>,
-      <command>statistic-get-all</command>, <command>statistic-reset-all</command>
-      and <command>statistic-remove-all</command>, specified in
-      <xref linkend="command-stats"/>. It also supports
-      <command>list-commands</command> and <command>shutdown</command>,
-      specified in <xref linkend="command-list-commands" /> and
-      <xref linkend="command-shutdown" />, respectively.</para>
     </section>
     </section>
 
 
     <section id="dhcp4-std">
     <section id="dhcp4-std">

+ 25 - 10
doc/guide/dhcp6-srv.xml

@@ -3410,6 +3410,9 @@ If not specified, the default value is:
 </screen>
 </screen>
       </para>
       </para>
 
 
+      <para>If "relay" is specified, the "ip-address" parameter within
+      it is mandatory.</para>
+
     </section>
     </section>
 
 
       <section id="dhcp6-client-class-relay">
       <section id="dhcp6-client-class-relay">
@@ -3500,7 +3503,8 @@ If not specified, the default value is:
 
 
     When not specified, a special value of "any" is used, which
     When not specified, a special value of "any" is used, which
     instructs the server to attempt to use all the methods in sequence and use
     instructs the server to attempt to use all the methods in sequence and use
-    value returned by the first one that succeeds.</para>
+    value returned by the first one that succeeds. If specified, it
+    has to have at least one value.</para>
 
 
     <para>Supported methods are:
     <para>Supported methods are:
     <itemizedlist>
     <itemizedlist>
@@ -4062,16 +4066,27 @@ If not specified, the default value is:
         See the Control Channel section in the Kea Developer's Guide for more details.
         See the Control Channel section in the Kea Developer's Guide for more details.
       </para>
       </para>
 
 
-      <para>The DHCPv6 server supports <command>statistic-get</command>,
-      <command>statistic-reset</command>, <command>statistic-remove</command>,
-      <command>statistic-get-all</command>, <command>statistic-reset-all</command>
-      and <command>statistic-remove-all</command>, specified in
-      <xref linkend="command-stats"/>. It also supports
-      <command>list-commands</command> and <command>shutdown</command>,
-      specified in <xref linkend="command-list-commands" /> and
-      <xref linkend="command-shutdown" />, respectively.</para>
-    </section>
+      <para>The DHCPv6 server supports the following operational commands:
+        <itemizedlist>
+            <listitem>leases-reclaim</listitem>
+            <listitem>list-commands</listitem>
+            <listitem>set-config</listitem>
+            <listitem>shutdown</listitem>
+        </itemizedlist>
+         as described in <xref linkend="commands-common"/>.  In addition,
+         it supports the following statistics related commands:
+        <itemizedlist>
+            <listitem>statistic-get</listitem>
+            <listitem>statistic-reset</listitem>
+            <listitem>statistic-remove</listitem>
+            <listitem>statistic-get-all</listitem>
+            <listitem>statistic-reset-all</listitem>
+            <listitem>statistic-remove-all</listitem>
+        </itemizedlist>
+        as described here <xref linkend="command-stats"/>.
+      </para>
 
 
+    </section>
 
 
       <section>
       <section>
         <title>User context in IPv6 pools</title>
         <title>User context in IPv6 pools</title>

+ 71 - 4
src/bin/dhcp4/ctrl_dhcp4_srv.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2014-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -62,7 +62,69 @@ ControlledDhcpv4Srv::commandLibReloadHandler(const string&, ConstElementPtr) {
 ConstElementPtr
 ConstElementPtr
 ControlledDhcpv4Srv::commandConfigReloadHandler(const string&,
 ControlledDhcpv4Srv::commandConfigReloadHandler(const string&,
                                                 ConstElementPtr args) {
                                                 ConstElementPtr args) {
-    return (processConfig(args));
+    // Use set-config as it handles logging and server config
+    return (commandSetConfigHandler("set-config", args));
+}
+
+ConstElementPtr
+ControlledDhcpv4Srv::commandSetConfigHandler(const string&,
+                                             ConstElementPtr args) {
+    const int status_code = 1; // 1 indicates an error
+    ConstElementPtr dhcp4;
+    string message;
+
+    // Command arguments are expected to be:
+    // { "Dhcp4": { ... }, "Logging": { ... } }
+    // The Logging component is technically optional. If it's not supplied
+    // logging will revert to default logging.
+    if (!args) {
+        message = "Missing mandatory 'arguments' parameter.";
+    } else {
+        dhcp4 = args->get("Dhcp4");
+        if (!dhcp4) {
+            message = "Missing mandatory 'Dhcp4' parameter.";
+        } else if (dhcp4->getType() != Element::map) {
+            message = "'Dhcp4' parameter expected to be a map.";
+        }
+    }
+
+    if (!message.empty()) {
+        // Something is amiss with arguments, return a failure response.
+        ConstElementPtr result = isc::config::createAnswer(status_code,
+                                                           message);
+        return (result);
+    }
+
+    // We are starting the configuration process so we should remove any
+    // staging configuration that has been created during previous
+    // configuration attempts.
+    CfgMgr::instance().rollback();
+
+    // Logging is a sibling element and must be parsed explicitly.
+    // The call to configureLogger parses the given Logging element if
+    // not null, into the staging config.  Note this does not alter the
+    // current loggers, they remain in effect until we apply the
+    // logging config below.  If no logging is supplied logging will
+    // revert to default logging.
+    Daemon::configureLogger(args->get("Logging"),
+                            CfgMgr::instance().getStagingCfg());
+
+    // Now we configure the server proper.
+    ConstElementPtr result = processConfig(dhcp4);
+
+    // If the configuration parsed successfully, apply the new logger
+    // configuration and the commit the new configuration.  We apply
+    // the logging first in case there's a configuration failure.
+    int rcode = 0;
+    isc::config::parseAnswer(rcode, result);
+    if (rcode == 0) {
+        CfgMgr::instance().getStagingCfg()->applyLoggingCfg();
+
+        // Use new configuration.
+        CfgMgr::instance().commit();
+    }
+
+    return (result);
 }
 }
 
 
 ConstElementPtr
 ConstElementPtr
@@ -116,6 +178,9 @@ ControlledDhcpv4Srv::processCommand(const string& command,
         } else if (command == "config-reload") {
         } else if (command == "config-reload") {
             return (srv->commandConfigReloadHandler(command, args));
             return (srv->commandConfigReloadHandler(command, args));
 
 
+        } else if (command == "set-config") {
+            return (srv->commandSetConfigHandler(command, args));
+
         } else if (command == "leases-reclaim") {
         } else if (command == "leases-reclaim") {
             return (srv->commandLeasesReclaimHandler(command, args));
             return (srv->commandLeasesReclaimHandler(command, args));
         }
         }
@@ -235,8 +300,6 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) {
         }
         }
     }
     }
 
 
-
-
     return (answer);
     return (answer);
 }
 }
 
 
@@ -257,6 +320,9 @@ ControlledDhcpv4Srv::ControlledDhcpv4Srv(uint16_t port /*= DHCP4_SERVER_PORT*/)
     CommandMgr::instance().registerCommand("libreload",
     CommandMgr::instance().registerCommand("libreload",
         boost::bind(&ControlledDhcpv4Srv::commandLibReloadHandler, this, _1, _2));
         boost::bind(&ControlledDhcpv4Srv::commandLibReloadHandler, this, _1, _2));
 
 
+    CommandMgr::instance().registerCommand("set-config",
+        boost::bind(&ControlledDhcpv4Srv::commandSetConfigHandler, this, _1, _2));
+
     CommandMgr::instance().registerCommand("leases-reclaim",
     CommandMgr::instance().registerCommand("leases-reclaim",
         boost::bind(&ControlledDhcpv4Srv::commandLeasesReclaimHandler, this, _1, _2));
         boost::bind(&ControlledDhcpv4Srv::commandLeasesReclaimHandler, this, _1, _2));
 
 
@@ -300,6 +366,7 @@ ControlledDhcpv4Srv::~ControlledDhcpv4Srv() {
         // Deregister any registered commands
         // Deregister any registered commands
         CommandMgr::instance().deregisterCommand("shutdown");
         CommandMgr::instance().deregisterCommand("shutdown");
         CommandMgr::instance().deregisterCommand("libreload");
         CommandMgr::instance().deregisterCommand("libreload");
+        CommandMgr::instance().deregisterCommand("set-config");
         CommandMgr::instance().deregisterCommand("leases-reclaim");
         CommandMgr::instance().deregisterCommand("leases-reclaim");
         CommandMgr::instance().deregisterCommand("statistic-get");
         CommandMgr::instance().deregisterCommand("statistic-get");
         CommandMgr::instance().deregisterCommand("statistic-reset");
         CommandMgr::instance().deregisterCommand("statistic-reset");

+ 14 - 1
src/bin/dhcp4/ctrl_dhcp4_srv.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -143,6 +143,19 @@ private:
     commandConfigReloadHandler(const std::string& command,
     commandConfigReloadHandler(const std::string& command,
                                isc::data::ConstElementPtr args);
                                isc::data::ConstElementPtr args);
 
 
+    /// @brief handler for processing 'set-config' command
+    ///
+    /// This handler processes set-config command, which processes
+    /// configuration specified in args parameter.
+    /// @param command (parameter ignored)
+    /// @param args configuration to be processed. Expected format:
+    /// map containing Dhcp4 map that contains DHCPv4 server configuration.
+    /// May also contain Logging map that specifies logging configuration.
+    ///
+    /// @return status of the command
+    isc::data::ConstElementPtr
+    commandSetConfigHandler(const std::string& command,
+                            isc::data::ConstElementPtr args);
 
 
     /// @brief Handler for processing 'leases-reclaim' command
     /// @brief Handler for processing 'leases-reclaim' command
     ///
     ///

+ 53 - 24
src/bin/dhcp4/json_config_parser.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -189,8 +189,7 @@ protected:
             parser = new StringParser(config_id, string_values_);
             parser = new StringParser(config_id, string_values_);
         } else if (config_id.compare("pools") == 0) {
         } else if (config_id.compare("pools") == 0) {
             parser = new Pools4ListParser(config_id, pools_);
             parser = new Pools4ListParser(config_id, pools_);
-        } else if (config_id.compare("relay") == 0) {
-            parser = new RelayInfoParser(config_id, relay_info_, Option::V4);
+            // relay has been converted to SimpleParser already.
         // option-data has been converted to SimpleParser already.
         // option-data has been converted to SimpleParser already.
         } else if (config_id.compare("match-client-id") == 0) {
         } else if (config_id.compare("match-client-id") == 0) {
             parser = new BooleanParser(config_id, boolean_values_);
             parser = new BooleanParser(config_id, boolean_values_);
@@ -440,8 +439,7 @@ DhcpConfigParser* createGlobalDhcp4ConfigParser(const std::string& config_id,
         parser = new D2ClientConfigParser(config_id);
         parser = new D2ClientConfigParser(config_id);
     } else if (config_id.compare("match-client-id") == 0) {
     } else if (config_id.compare("match-client-id") == 0) {
         parser = new BooleanParser(config_id, globalContext()->boolean_values_);
         parser = new BooleanParser(config_id, globalContext()->boolean_values_);
-    } else if (config_id.compare("control-socket") == 0) {
-        parser = new ControlSocketParser(config_id);
+    // control-socket has been converted to SimpleParser already.
     } else if (config_id.compare("expired-leases-processing") == 0) {
     } else if (config_id.compare("expired-leases-processing") == 0) {
         parser = new ExpirationConfigParser();
         parser = new ExpirationConfigParser();
     } else if (config_id.compare("client-classes") == 0) {
     } else if (config_id.compare("client-classes") == 0) {
@@ -497,6 +495,47 @@ void setGlobalParameters4() {
     }
     }
 }
 }
 
 
+/// @brief Initialize the command channel based on the staging configuration
+///
+/// Only close the current channel, if the new channel configuration is
+/// different.  This avoids disconnecting a client and hence not sending them
+/// a command result, unless they specifically alter the channel configuration.
+/// In that case the user simply has to accept they'll be disconnected.
+///
+void configureCommandChannel() {
+    // Get new socket configuration.
+    ConstElementPtr sock_cfg =
+        CfgMgr::instance().getStagingCfg()->getControlSocketInfo();
+
+    // Get current socket configuration.
+    ConstElementPtr current_sock_cfg =
+            CfgMgr::instance().getCurrentCfg()->getControlSocketInfo();
+
+    // Determine if the socket configuration has changed. It has if
+    // both old and new configuration is specified but respective
+    // data elements are't equal.
+    bool sock_changed = (sock_cfg && current_sock_cfg &&
+                         !sock_cfg->equals(*current_sock_cfg));
+
+    // If the previous or new socket configuration doesn't exist or
+    // the new configuration differs from the old configuration we
+    // close the exisitng socket and open a new socket as appropriate.
+    // Note that closing an existing socket means the clien will not
+    // receive the configuration result.
+    if (!sock_cfg || !current_sock_cfg || sock_changed) {
+        // Close the existing socket (if any).
+        isc::config::CommandMgr::instance().closeCommandSocket();
+
+        if (sock_cfg) {
+            // This will create a control socket and install the external
+            // socket in IfaceMgr. That socket will be monitored when
+            // Dhcp4Srv::receivePacket() calls IfaceMgr::receive4() and
+            // callback in CommandMgr will be called, if necessary.
+            isc::config::CommandMgr::instance().openCommandSocket(sock_cfg);
+        }
+    }
+}
+
 isc::data::ConstElementPtr
 isc::data::ConstElementPtr
 configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
 configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
     if (!config_set) {
     if (!config_set) {
@@ -596,6 +635,13 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
                 continue;
                 continue;
             }
             }
 
 
+            if (config_pair.first == "control-socket") {
+                ControlSocketParser parser;
+                SrvConfigPtr srv_cfg = CfgMgr::instance().getStagingCfg();
+                parser.parse(*srv_cfg, config_pair.second);
+                continue;
+            }
+
             ParserPtr parser(createGlobalDhcp4ConfigParser(config_pair.first,
             ParserPtr parser(createGlobalDhcp4ConfigParser(config_pair.first,
                                                            config_pair.second));
                                                            config_pair.second));
             LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PARSER_CREATED)
             LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_PARSER_CREATED)
@@ -646,25 +692,8 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
             subnet_parser->build(subnet_config->second);
             subnet_parser->build(subnet_config->second);
         }
         }
 
 
-        // Get command socket configuration from the config file.
-        // This code expects the following structure:
-        // {
-        //     "socket-type": "unix",
-        //     "socket-name": "/tmp/kea4.sock"
-        // }
-        ConstElementPtr sock_cfg =
-            CfgMgr::instance().getStagingCfg()->getControlSocketInfo();
-
-        // Close existing socket (if any).
-        isc::config::CommandMgr::instance().closeCommandSocket();
-        if (sock_cfg) {
-            // This will create a control socket and will install external socket
-            // in IfaceMgr. That socket will be monitored when Dhcp4Srv::receivePacket()
-            // calls IfaceMgr::receive4() and callback in CommandMgr will be called,
-            // if necessary. If there were previously open command socket, it will
-            // be closed.
-            isc::config::CommandMgr::instance().openCommandSocket(sock_cfg);
-        }
+        // Setup the command channel.
+        configureCommandChannel();
 
 
         // the leases database parser is the last to be run.
         // the leases database parser is the last to be run.
         std::map<std::string, ConstElementPtr>::const_iterator leases_config =
         std::map<std::string, ConstElementPtr>::const_iterator leases_config =

+ 4 - 32
src/bin/dhcp4/kea_controller.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -34,11 +34,6 @@ void configure(const std::string& file_name) {
     // This is a configuration backend implementation that reads the
     // This is a configuration backend implementation that reads the
     // configuration from a JSON file.
     // configuration from a JSON file.
 
 
-    // We are starting the configuration process so we should remove any
-    // staging configuration that has been created during previous
-    // configuration attempts.
-    CfgMgr::instance().rollback();
-
     isc::data::ConstElementPtr json;
     isc::data::ConstElementPtr json;
     isc::data::ConstElementPtr dhcp4;
     isc::data::ConstElementPtr dhcp4;
     isc::data::ConstElementPtr logger;
     isc::data::ConstElementPtr logger;
@@ -70,26 +65,14 @@ void configure(const std::string& file_name) {
                       " Did you forget to add { } around your configuration?");
                       " Did you forget to add { } around your configuration?");
         }
         }
 
 
-        // If there's no logging element, we'll just pass NULL pointer,
-        // which will be handled by configureLogger().
-        Daemon::configureLogger(json->get("Logging"),
-                                CfgMgr::instance().getStagingCfg());
-
-        // Get Dhcp4 component from the config
-        dhcp4 = json->get("Dhcp4");
-        if (!dhcp4) {
-            isc_throw(isc::BadValue, "no mandatory 'Dhcp4' entry in"
-                      " the configuration");
-        }
-
         // Use parsed JSON structures to configure the server
         // Use parsed JSON structures to configure the server
-        result = ControlledDhcpv4Srv::processCommand("config-reload", dhcp4);
+        result = ControlledDhcpv4Srv::processCommand("set-config", json);
         if (!result) {
         if (!result) {
             // Undetermined status of the configuration. This should never
             // Undetermined status of the configuration. This should never
             // happen, but as the configureDhcp4Server returns a pointer, it is
             // happen, but as the configureDhcp4Server returns a pointer, it is
             // theoretically possible that it will return NULL.
             // theoretically possible that it will return NULL.
             isc_throw(isc::BadValue, "undefined result of "
             isc_throw(isc::BadValue, "undefined result of "
-                      "processCommand(\"config-reload\", dhcp4)");
+                      "processCommand(\"set-config\", json)");
         }
         }
 
 
         // Now check is the returned result is successful (rcode=0) or not
         // Now check is the returned result is successful (rcode=0) or not
@@ -102,17 +85,6 @@ void configure(const std::string& file_name) {
                 "no details available";
                 "no details available";
             isc_throw(isc::BadValue, reason);
             isc_throw(isc::BadValue, reason);
         }
         }
-
-        // If configuration was parsed successfully, apply the new logger
-        // configuration to log4cplus. It is done before commit in case
-        // something goes wrong.
-        CfgMgr::instance().getStagingCfg()->applyLoggingCfg();
-
-        // Use new configuration.
-        /// @todo: This commit should be moved to
-        /// CtrlDhcp4Srv::commandConfigReloadHandler.
-        CfgMgr::instance().commit();
-
     }  catch (const std::exception& ex) {
     }  catch (const std::exception& ex) {
         // If configuration failed at any stage, we drop the staging
         // If configuration failed at any stage, we drop the staging
         // configuration and continue to use the previous one.
         // configuration and continue to use the previous one.
@@ -167,7 +139,7 @@ ControlledDhcpv4Srv::init(const std::string& file_name) {
 
 
     // We don't need to call openActiveSockets() or startD2() as these
     // We don't need to call openActiveSockets() or startD2() as these
     // methods are called in processConfig() which is called by
     // methods are called in processConfig() which is called by
-    // processCommand("reload-config", ...)
+    // processCommand("set-config", ...)
 
 
     // Set signal handlers. When the SIGHUP is received by the process
     // Set signal handlers. When the SIGHUP is received by the process
     // the server reconfiguration will be triggered. When SIGTERM or
     // the server reconfiguration will be triggered. When SIGTERM or

+ 153 - 1
src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -15,7 +15,9 @@
 #include <dhcpsrv/lease.h>
 #include <dhcpsrv/lease.h>
 #include <dhcpsrv/lease_mgr_factory.h>
 #include <dhcpsrv/lease_mgr_factory.h>
 #include <hooks/hooks_manager.h>
 #include <hooks/hooks_manager.h>
+#include <log/logger_support.h>
 #include <stats/stats_mgr.h>
 #include <stats/stats_mgr.h>
+#include <testutils/io_utils.h>
 #include <testutils/unix_control_client.h>
 #include <testutils/unix_control_client.h>
 
 
 #include "marker_file.h"
 #include "marker_file.h"
@@ -121,6 +123,13 @@ public:
         ConstElementPtr config;
         ConstElementPtr config;
         ASSERT_NO_THROW(config = parseDHCP4(config_txt));
         ASSERT_NO_THROW(config = parseDHCP4(config_txt));
         ConstElementPtr answer = server_->processConfig(config);
         ConstElementPtr answer = server_->processConfig(config);
+
+        // Commit the configuration so any subsequent reconfigurations
+        // will only close the command channel if its configuration has
+        // changed.
+        CfgMgr::instance().commit();
+
+
         ASSERT_TRUE(answer);
         ASSERT_TRUE(answer);
 
 
         int status = 0;
         int status = 0;
@@ -472,4 +481,147 @@ TEST_F(CtrlChannelDhcpv4SrvTest, controlChannelStats) {
               response);
               response);
 }
 }
 
 
+// Check that the "set-config" command will replace current configuration
+TEST_F(CtrlChannelDhcpv4SrvTest, set_config) {
+    createUnixChannelServer();
+
+    // Define strings to permutate the config arguments
+    // (Note the line feeds makes errors easy to find)
+    string set_config_txt = "{ \"command\": \"set-config\" \n";
+    string args_txt = " \"arguments\": { \n";
+    string dhcp4_cfg_txt =
+        "    \"Dhcp4\": { \n"
+        "        \"interfaces-config\": { \n"
+        "            \"interfaces\": [\"*\"] \n"
+        "        },   \n"
+        "        \"valid-lifetime\": 4000, \n"
+        "        \"renew-timer\": 1000, \n"
+        "        \"rebind-timer\": 2000, \n"
+        "        \"lease-database\": { \n"
+        "           \"type\": \"memfile\", \n"
+        "           \"persist\":false, \n"
+        "           \"lfc-interval\": 0  \n"
+        "        }, \n"
+        "       \"expired-leases-processing\": { \n"
+        "            \"reclaim-timer-wait-time\": 0, \n"
+        "            \"hold-reclaimed-time\": 0, \n"
+        "            \"flush-reclaimed-timer-wait-time\": 0 \n"
+        "        },"
+        "        \"subnet4\": [ \n";
+    string subnet1 =
+        "               {\"subnet\": \"192.2.0.0/24\", \n"
+        "                \"pools\": [{ \"pool\": \"192.2.0.1-192.2.0.50\" }]}\n";
+    string subnet2 =
+        "               {\"subnet\": \"192.2.1.0/24\", \n"
+        "                \"pools\": [{ \"pool\": \"192.2.1.1-192.2.1.50\" }]}\n";
+    string bad_subnet =
+        "               {\"BOGUS\": \"192.2.2.0/24\", \n"
+        "                \"pools\": [{ \"pool\": \"192.2.2.1-192.2.2.50\" }]}\n";
+    string subnet_footer =
+        "          ] \n";
+    string control_socket_header =
+        "       ,\"control-socket\": { \n"
+        "       \"socket-type\": \"unix\", \n"
+        "       \"socket-name\": \"";
+    string control_socket_footer =
+        "\"   \n} \n";
+    string logger_txt =
+        "    \"Logging\": { \n"
+        "        \"loggers\": [ { \n"
+        "            \"name\": \"kea\", \n"
+        "            \"severity\": \"FATAL\", \n"
+        "            \"output_options\": [{ \n"
+        "                \"output\": \"/dev/null\" \n"
+        "            }] \n"
+        "        }] \n"
+        "    } \n";
+
+    std::ostringstream os;
+
+    // Create a valid config with all the parts should parse
+    os << set_config_txt << ","
+        << args_txt
+        << dhcp4_cfg_txt
+        << subnet1
+        << subnet_footer
+        << control_socket_header
+        << socket_path_
+        << control_socket_footer
+        << "}\n"                      // close dhcp4
+        << ","
+        << logger_txt
+        << "}}";
+
+    // Send the set-config command
+    std::string response;
+    sendUnixCommand(os.str(), response);
+
+    // Verify the configuration was successful.
+    EXPECT_EQ("{ \"result\": 0, \"text\": \"Configuration successful.\" }",
+              response);
+
+    // Check that the config was indeed applied.
+    const Subnet4Collection* subnets =
+        CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getAll();
+    EXPECT_EQ(1, subnets->size());
+
+    // Create a config with malformed subnet that should fail to parse.
+    os.str("");
+    os << set_config_txt << ","
+        << args_txt
+        << dhcp4_cfg_txt
+        << bad_subnet
+        << subnet_footer
+        << control_socket_header
+        << socket_path_
+        << control_socket_footer
+        << "}\n"                      // close dhcp4
+        "}}";
+
+    // Send the set-config command
+    sendUnixCommand(os.str(), response);
+
+    // Should fail with a syntax error
+    EXPECT_EQ("{ \"result\": 1, "
+              "\"text\": \"unsupported parameter: BOGUS (<string>:20:26)\" }",
+              response);
+
+    // Check that the config was not lost
+    subnets = CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getAll();
+    EXPECT_EQ(1, subnets->size());
+
+    // Create a valid config with two subnets and no command channel.
+    // It should succeed, client should still receive the response
+    os.str("");
+    os << set_config_txt << ","
+        << args_txt
+        << dhcp4_cfg_txt
+        << subnet1
+        << ",\n"
+        << subnet2
+        << subnet_footer
+        << "}\n"                      // close dhcp4
+        << "}}";
+
+    /* Verify the control channel socket exists */
+    ASSERT_TRUE(fileExists(socket_path_));
+
+    // Send the set-config command
+    sendUnixCommand(os.str(), response);
+
+    /* Verify the control channel socket no longer exists */
+    EXPECT_FALSE(fileExists(socket_path_));
+
+    // With no command channel, should still receive the response.
+    EXPECT_EQ("{ \"result\": 0, \"text\": \"Configuration successful.\" }",
+              response);
+
+    // Check that the config was not lost
+    subnets = CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getAll();
+    EXPECT_EQ(2, subnets->size());
+
+    // Clean up after the test.
+    CfgMgr::instance().clear();
+}
+
 } // End of anonymous namespace
 } // End of anonymous namespace

+ 5 - 1
src/bin/dhcp4/tests/dhcp4_test_utils.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -24,6 +24,7 @@
 #include <dhcpsrv/lease.h>
 #include <dhcpsrv/lease.h>
 #include <dhcpsrv/lease_mgr.h>
 #include <dhcpsrv/lease_mgr.h>
 #include <dhcpsrv/lease_mgr_factory.h>
 #include <dhcpsrv/lease_mgr_factory.h>
+#include <log/logger_support.h>
 #include <stats/stats_mgr.h>
 #include <stats/stats_mgr.h>
 
 
 using namespace std;
 using namespace std;
@@ -47,6 +48,9 @@ BaseServerTest::~BaseServerTest() {
 
 
     // Revert to original data directory.
     // Revert to original data directory.
     CfgMgr::instance().setDataDir(original_datadir_);
     CfgMgr::instance().setDataDir(original_datadir_);
+
+    // Revert to unit test logging, in case the test reconfigured it.
+    isc::log::initLogger();
 }
 }
 
 
 Dhcpv4SrvTest::Dhcpv4SrvTest()
 Dhcpv4SrvTest::Dhcpv4SrvTest()

+ 1 - 2
src/bin/dhcp4/tests/kea_controller_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2014-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -63,7 +63,6 @@ public:
 
 
     ~JSONFileBackendTest() {
     ~JSONFileBackendTest() {
         LeaseMgrFactory::destroy();
         LeaseMgrFactory::destroy();
-        isc::log::setDefaultLoggingOutput();
         static_cast<void>(remove(TEST_FILE));
         static_cast<void>(remove(TEST_FILE));
         static_cast<void>(remove(TEST_INCLUDE));
         static_cast<void>(remove(TEST_INCLUDE));
     };
     };

+ 22 - 87
src/bin/dhcp4/tests/parser_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2016-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -7,14 +7,14 @@
 #include <gtest/gtest.h>
 #include <gtest/gtest.h>
 #include <cc/data.h>
 #include <cc/data.h>
 #include <dhcp4/parser_context.h>
 #include <dhcp4/parser_context.h>
-#include <fstream>
-#include <cstdio>
-#include <exceptions/exceptions.h>
+#include <testutils/io_utils.h>
 
 
 using namespace isc::data;
 using namespace isc::data;
 using namespace std;
 using namespace std;
 
 
-namespace {
+namespace isc {
+namespace dhcp {
+namespace test {
 
 
 /// @brief compares two JSON trees
 /// @brief compares two JSON trees
 ///
 ///
@@ -128,6 +128,8 @@ TEST(ParserTest, keywordDhcp4) {
      testParser(txt, Parser4Context::PARSER_DHCP4);
      testParser(txt, Parser4Context::PARSER_DHCP4);
 }
 }
 
 
+// Tests if bash (#) comments are supported. That's the only comment type that
+// was supported by the old parser.
 TEST(ParserTest, bashComments) {
 TEST(ParserTest, bashComments) {
     string txt= "{ \"Dhcp4\": { \"interfaces-config\": {"
     string txt= "{ \"Dhcp4\": { \"interfaces-config\": {"
                 "  \"interfaces\": [ \"*\" ]"
                 "  \"interfaces\": [ \"*\" ]"
@@ -146,7 +148,8 @@ TEST(ParserTest, bashComments) {
     testParser(txt, Parser4Context::PARSER_DHCP4, false);
     testParser(txt, Parser4Context::PARSER_DHCP4, false);
 }
 }
 
 
-TEST(ParserTest, cComments) {
+// Tests if C++ (//) comments can start anywhere, not just in the first line.
+TEST(ParserTest, cppComments) {
     string txt= "{ \"Dhcp4\": { \"interfaces-config\": {"
     string txt= "{ \"Dhcp4\": { \"interfaces-config\": {"
                 "  \"interfaces\": [ \"*\" ]"
                 "  \"interfaces\": [ \"*\" ]"
                 "},\n"
                 "},\n"
@@ -161,6 +164,7 @@ TEST(ParserTest, cComments) {
     testParser(txt, Parser4Context::PARSER_DHCP4, false);
     testParser(txt, Parser4Context::PARSER_DHCP4, false);
 }
 }
 
 
+// Tests if bash (#) comments can start anywhere, not just in the first line.
 TEST(ParserTest, bashCommentsInline) {
 TEST(ParserTest, bashCommentsInline) {
     string txt= "{ \"Dhcp4\": { \"interfaces-config\": {"
     string txt= "{ \"Dhcp4\": { \"interfaces-config\": {"
                 "  \"interfaces\": [ \"*\" ]"
                 "  \"interfaces\": [ \"*\" ]"
@@ -176,6 +180,7 @@ TEST(ParserTest, bashCommentsInline) {
     testParser(txt, Parser4Context::PARSER_DHCP4, false);
     testParser(txt, Parser4Context::PARSER_DHCP4, false);
 }
 }
 
 
+// Tests if multi-line C style comments are handled correctly.
 TEST(ParserTest, multilineComments) {
 TEST(ParserTest, multilineComments) {
     string txt= "{ \"Dhcp4\": { \"interfaces-config\": {"
     string txt= "{ \"Dhcp4\": { \"interfaces-config\": {"
                 "  \"interfaces\": [ \"*\" ]"
                 "  \"interfaces\": [ \"*\" ]"
@@ -193,89 +198,13 @@ TEST(ParserTest, multilineComments) {
     testParser(txt, Parser4Context::PARSER_DHCP4, false);
     testParser(txt, Parser4Context::PARSER_DHCP4, false);
 }
 }
 
 
-/// @brief removes comments from a JSON file
-///
-/// This is rather naive implementation, but it's probably sufficient for
-/// testing. It won't be able to pick any trickier cases, like # or //
-/// appearing in strings, nested C++ comments etc.
-///
-/// @param input_file file to be stripped of comments
-/// @return a new file that has comments stripped from it
-std::string decommentJSONfile(const std::string& input_file) {
-    ifstream f(input_file);
-    if (!f.is_open()) {
-        isc_throw(isc::BadValue, "can't open input file for reading: " + input_file);
-    }
-
-    string outfile;
-    size_t last_slash = input_file.find_last_of("/");
-    if (last_slash != string::npos) {
-        outfile = input_file.substr(last_slash + 1);
-    } else {
-        outfile = input_file;
-    }
-    outfile += "-decommented";
-
-    ofstream out(outfile);
-    if (!out.is_open()) {
-        isc_throw(isc::BadValue, "can't open output file for writing: " + input_file);
-    }
-
-    bool in_comment = false;
-    string line;
-    while (std::getline(f, line)) {
-        // First, let's get rid of the # comments
-        size_t hash_pos = line.find("#");
-        if (hash_pos != string::npos) {
-            line = line.substr(0, hash_pos);
-        }
-
-        // Second, let's get rid of the // comments
-        size_t dblslash_pos = line.find("//");
-        if (dblslash_pos != string::npos) {
-            line = line.substr(0, dblslash_pos);
-        }
-
-        // Now the tricky part: c comments.
-        size_t begin_pos = line.find("/*");
-        size_t end_pos = line.find("*/");
-        if (in_comment && end_pos == string::npos) {
-            // we continue through multiline comment
-            line = "";
-        } else {
-
-            if (begin_pos != string::npos) {
-                in_comment = true;
-                if (end_pos != string::npos) {
-                    // sigle line comment. Let's get rid of the content in between
-                    line = line.replace(begin_pos, end_pos + 2, end_pos + 2 - begin_pos, ' ');
-                    in_comment = false;
-                } else {
-                    line = line.substr(0, begin_pos);
-                }
-            } else {
-                if (in_comment && end_pos != string::npos) {
-                    line = line.replace(0, end_pos +2 , end_pos + 2, ' ');
-                    in_comment = false;
-                }
-            }
-        }
-
-        // Finally, write the line to the output file.
-        out << line << endl;
-    }
-    f.close();
-    out.close();
-
-    return (outfile);
-}
 
 
 /// @brief Loads specified example config file
 /// @brief Loads specified example config file
 ///
 ///
 /// This test loads specified example file twice: first, using the legacy
 /// This test loads specified example file twice: first, using the legacy
 /// JSON file and then second time using bison parser. Two created Element
 /// JSON file and then second time using bison parser. Two created Element
 /// trees are then compared. The input is decommented before it is passed
 /// trees are then compared. The input is decommented before it is passed
-/// to legacy parser (as its support for comments is very limited).
+/// to legacy parser (as legacy support for comments is very limited).
 ///
 ///
 /// @param fname name of the file to be loaded
 /// @param fname name of the file to be loaded
 void testFile(const std::string& fname) {
 void testFile(const std::string& fname) {
@@ -284,8 +213,7 @@ void testFile(const std::string& fname) {
 
 
     string decommented = decommentJSONfile(fname);
     string decommented = decommentJSONfile(fname);
 
 
-    cout << "Attempting to load file " << fname << " (" << decommented
-         << ")" << endl;
+    cout << "Parsing file " << fname << " (" << decommented << ")" << endl;
 
 
     EXPECT_NO_THROW(reference_json = Element::fromJSONFile(decommented, true));
     EXPECT_NO_THROW(reference_json = Element::fromJSONFile(decommented, true));
 
 
@@ -329,6 +257,11 @@ TEST(ParserTest, file) {
     }
     }
 }
 }
 
 
+/// @brief Tests error conditions in Dhcp4Parser
+///
+/// @param txt text to be parsed
+/// @param parser_type type of the parser to be used in the test
+/// @param msg expected content of the exception
 void testError(const std::string& txt,
 void testError(const std::string& txt,
                Parser4Context::ParserType parser_type,
                Parser4Context::ParserType parser_type,
                const std::string& msg)
                const std::string& msg)
@@ -347,7 +280,7 @@ void testError(const std::string& txt,
     }
     }
 }
 }
 
 
-// Check errors
+// Verify that error conditions are handled correctly.
 TEST(ParserTest, errors) {
 TEST(ParserTest, errors) {
     // no input
     // no input
     testError("", Parser4Context::PARSER_JSON,
     testError("", Parser4Context::PARSER_JSON,
@@ -591,7 +524,7 @@ TEST(ParserTest, unicodeEscapes) {
     }
     }
 }
 }
 
 
-// This test checks that all representations of a slash is recognized properly.
+// This test checks that all representations of a slash are recognized properly.
 TEST(ParserTest, unicodeSlash) {
 TEST(ParserTest, unicodeSlash) {
     // check the 4 possible encodings of solidus '/'
     // check the 4 possible encodings of solidus '/'
     ConstElementPtr result;
     ConstElementPtr result;
@@ -609,3 +542,5 @@ TEST(ParserTest, unicodeSlash) {
 }
 }
 
 
 };
 };
+};
+};

+ 73 - 4
src/bin/dhcp6/ctrl_dhcp6_srv.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2014-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -67,10 +67,72 @@ ControlledDhcpv6Srv::commandLibReloadHandler(const string&, ConstElementPtr) {
 
 
 ConstElementPtr
 ConstElementPtr
 ControlledDhcpv6Srv::commandConfigReloadHandler(const string&, ConstElementPtr args) {
 ControlledDhcpv6Srv::commandConfigReloadHandler(const string&, ConstElementPtr args) {
+    // Use set-config as it handles logging and server config
+    return (commandSetConfigHandler("set-config", args));
+}
+
+ConstElementPtr
+ControlledDhcpv6Srv::commandSetConfigHandler(const string&,
+                                             ConstElementPtr args) {
+    const int status_code = 1; // 1 indicates an error
+    ConstElementPtr dhcp6;
+    string message;
+
+    // Command arguments are expected to be:
+    // { "Dhcp6": { ... }, "Logging": { ... } }
+    // The Logging component is technically optional. If it's not supplied
+    // logging will revert to default logging.
+    if (!args) {
+        message = "Missing mandatory 'arguments' parameter.";
+    } else {
+        dhcp6 = args->get("Dhcp6");
+        if (!dhcp6) {
+            message = "Missing mandatory 'Dhcp6' parameter.";
+        } else if (dhcp6->getType() != Element::map) {
+            message = "'Dhcp6' parameter expected to be a map.";
+        }
+    }
 
 
-    return (processConfig(args));
+    if (!message.empty()) {
+        // Something is amiss with arguments, return a failure response.
+        ConstElementPtr result = isc::config::createAnswer(status_code,
+                                                           message);
+        return (result);
+    }
+
+    // We are starting the configuration process so we should remove any
+    // staging configuration that has been created during previous
+    // configuration attempts.
+    CfgMgr::instance().rollback();
+
+    // Logging is a sibling element and must be parsed explicitly.
+    // The call to configureLogger parses the given Logging element if
+    // not null, into the staging config.  Note this does not alter the
+    // current loggers, they remain in effect until we apply the
+    // logging config below.  If no logging is supplied logging will
+    // revert to default logging.
+    Daemon::configureLogger(args->get("Logging"),
+                            CfgMgr::instance().getStagingCfg());
+
+    // Now we configure the server proper.
+    ConstElementPtr result = processConfig(dhcp6);
+
+    // If the configuration parsed successfully, apply the new logger
+    // configuration and the commit the new configuration.  We apply
+    // the logging first in case there's a configuration failure.
+    int rcode = 0;
+    isc::config::parseAnswer(rcode, result);
+    if (rcode == 0) {
+        CfgMgr::instance().getStagingCfg()->applyLoggingCfg();
+
+        // Use new configuration.
+        CfgMgr::instance().commit();
+    }
+
+    return (result);
 }
 }
 
 
+
 ConstElementPtr
 ConstElementPtr
 ControlledDhcpv6Srv::commandLeasesReclaimHandler(const string&,
 ControlledDhcpv6Srv::commandLeasesReclaimHandler(const string&,
                                                  ConstElementPtr args) {
                                                  ConstElementPtr args) {
@@ -116,12 +178,17 @@ ControlledDhcpv6Srv::processCommand(const std::string& command,
         if (command == "shutdown") {
         if (command == "shutdown") {
             return (srv->commandShutdownHandler(command, args));
             return (srv->commandShutdownHandler(command, args));
 
 
+        /// @todo: register config-reload (see CtrlDhcpv6Srv::commandConfigReloadHandler)
+
         } else if (command == "libreload") {
         } else if (command == "libreload") {
             return (srv->commandLibReloadHandler(command, args));
             return (srv->commandLibReloadHandler(command, args));
 
 
         } else if (command == "config-reload") {
         } else if (command == "config-reload") {
             return (srv->commandConfigReloadHandler(command, args));
             return (srv->commandConfigReloadHandler(command, args));
 
 
+        } else if (command == "set-config") {
+            return (srv->commandSetConfigHandler(command, args));
+
         } else if (command == "leases-reclaim") {
         } else if (command == "leases-reclaim") {
             return (srv->commandLeasesReclaimHandler(command, args));
             return (srv->commandLeasesReclaimHandler(command, args));
         }
         }
@@ -279,11 +346,12 @@ ControlledDhcpv6Srv::ControlledDhcpv6Srv(uint16_t port)
     CommandMgr::instance().registerCommand("shutdown",
     CommandMgr::instance().registerCommand("shutdown",
         boost::bind(&ControlledDhcpv6Srv::commandShutdownHandler, this, _1, _2));
         boost::bind(&ControlledDhcpv6Srv::commandShutdownHandler, this, _1, _2));
 
 
-    /// @todo: register config-reload (see CtrlDhcpv4Srv::commandConfigReloadHandler)
-
     CommandMgr::instance().registerCommand("libreload",
     CommandMgr::instance().registerCommand("libreload",
         boost::bind(&ControlledDhcpv6Srv::commandLibReloadHandler, this, _1, _2));
         boost::bind(&ControlledDhcpv6Srv::commandLibReloadHandler, this, _1, _2));
 
 
+    CommandMgr::instance().registerCommand("set-config",
+        boost::bind(&ControlledDhcpv6Srv::commandSetConfigHandler, this, _1, _2));
+
     CommandMgr::instance().registerCommand("leases-reclaim",
     CommandMgr::instance().registerCommand("leases-reclaim",
         boost::bind(&ControlledDhcpv6Srv::commandLeasesReclaimHandler, this, _1, _2));
         boost::bind(&ControlledDhcpv6Srv::commandLeasesReclaimHandler, this, _1, _2));
 
 
@@ -327,6 +395,7 @@ ControlledDhcpv6Srv::~ControlledDhcpv6Srv() {
         // Deregister any registered commands
         // Deregister any registered commands
         CommandMgr::instance().deregisterCommand("shutdown");
         CommandMgr::instance().deregisterCommand("shutdown");
         CommandMgr::instance().deregisterCommand("libreload");
         CommandMgr::instance().deregisterCommand("libreload");
+        CommandMgr::instance().deregisterCommand("set-config");
         CommandMgr::instance().deregisterCommand("leases-reclaim");
         CommandMgr::instance().deregisterCommand("leases-reclaim");
         CommandMgr::instance().deregisterCommand("statistic-get");
         CommandMgr::instance().deregisterCommand("statistic-get");
         CommandMgr::instance().deregisterCommand("statistic-reset");
         CommandMgr::instance().deregisterCommand("statistic-reset");

+ 15 - 1
src/bin/dhcp6/ctrl_dhcp6_srv.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -143,6 +143,20 @@ private:
     commandConfigReloadHandler(const std::string& command,
     commandConfigReloadHandler(const std::string& command,
                                isc::data::ConstElementPtr args);
                                isc::data::ConstElementPtr args);
 
 
+    /// @brief handler for processing 'set-config' command
+    ///
+    /// This handler processes set-config command, which processes
+    /// configuration specified in args parameter.
+    /// @param command (parameter ignored)
+    /// @param args configuration to be processed. Expected format:
+    /// map containing Dhcp6 map that contains DHCPv6 server configuration.
+    /// May also contain Logging map that specifies logging configuration.
+    ///
+    /// @return status of the command
+    isc::data::ConstElementPtr
+    commandSetConfigHandler(const std::string& command,
+                            isc::data::ConstElementPtr args);
+
     /// @brief Handler for processing 'leases-reclaim' command
     /// @brief Handler for processing 'leases-reclaim' command
     ///
     ///
     /// This handler processes leases-reclaim command, which triggers
     /// This handler processes leases-reclaim command, which triggers

+ 61 - 27
src/bin/dhcp6/json_config_parser.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -424,8 +424,7 @@ protected:
             parser = new StringParser(config_id, string_values_);
             parser = new StringParser(config_id, string_values_);
         } else if (config_id.compare("pools") == 0) {
         } else if (config_id.compare("pools") == 0) {
             parser = new Pools6ListParser(config_id, pools_);
             parser = new Pools6ListParser(config_id, pools_);
-        } else if (config_id.compare("relay") == 0) {
-            parser = new RelayInfoParser(config_id, relay_info_, Option::V6);
+        // relay has been converted to SimpleParser.
         } else if (config_id.compare("pd-pools") == 0) {
         } else if (config_id.compare("pd-pools") == 0) {
             parser = new PdPoolListParser(config_id, pools_);
             parser = new PdPoolListParser(config_id, pools_);
         // option-data was here, but it is now converted to SimpleParser
         // option-data was here, but it is now converted to SimpleParser
@@ -718,13 +717,10 @@ DhcpConfigParser* createGlobal6DhcpConfigParser(const std::string& config_id,
         parser = new HooksLibrariesParser(config_id);
         parser = new HooksLibrariesParser(config_id);
     } else if (config_id.compare("dhcp-ddns") == 0) {
     } else if (config_id.compare("dhcp-ddns") == 0) {
         parser = new D2ClientConfigParser(config_id);
         parser = new D2ClientConfigParser(config_id);
-    } else if (config_id.compare("mac-sources") == 0) {
-        parser = new MACSourcesListConfigParser(config_id,
-                                                globalContext());
+    // mac-source has been converted to SimpleParser.
     } else if (config_id.compare("relay-supplied-options") == 0) {
     } else if (config_id.compare("relay-supplied-options") == 0) {
         parser = new RSOOListConfigParser(config_id);
         parser = new RSOOListConfigParser(config_id);
-    } else if (config_id.compare("control-socket") == 0) {
-        parser = new ControlSocketParser(config_id);
+    // control-socket has been converted to SimpleParser.
     } else if (config_id.compare("expired-leases-processing") == 0) {
     } else if (config_id.compare("expired-leases-processing") == 0) {
         parser = new ExpirationConfigParser();
         parser = new ExpirationConfigParser();
     } else if (config_id.compare("client-classes") == 0) {
     } else if (config_id.compare("client-classes") == 0) {
@@ -770,6 +766,47 @@ void setGlobalParameters6() {
     }
     }
 }
 }
 
 
+/// @brief Initialize the command channel based on the staging configuration
+///
+/// Only close the current channel, if the new channel configuration is
+/// different.  This avoids disconnecting a client and hence not sending them
+/// a command result, unless they specifically alter the channel configuration.
+/// In that case the user simply has to accept they'll be disconnected.
+///
+void configureCommandChannel() {
+    // Get new socket configuration.
+    ConstElementPtr sock_cfg =
+        CfgMgr::instance().getStagingCfg()->getControlSocketInfo();
+
+    // Get current socket configuration.
+    ConstElementPtr current_sock_cfg =
+            CfgMgr::instance().getCurrentCfg()->getControlSocketInfo();
+
+    // Determine if the socket configuration has changed. It has if
+    // both old and new configuration is specified but respective
+    // data elements are't equal.
+    bool sock_changed = (sock_cfg && current_sock_cfg &&
+                         !sock_cfg->equals(*current_sock_cfg));
+
+    // If the previous or new socket configuration doesn't exist or
+    // the new configuration differs from the old configuration we
+    // close the exisitng socket and open a new socket as appropriate.
+    // Note that closing an existing socket means the clien will not
+    // receive the configuration result.
+    if (!sock_cfg || !current_sock_cfg || sock_changed) {
+        // Close the existing socket (if any).
+        isc::config::CommandMgr::instance().closeCommandSocket();
+
+        if (sock_cfg) {
+            // This will create a control socket and install the external
+            // socket in IfaceMgr. That socket will be monitored when
+            // Dhcp4Srv::receivePacket() calls IfaceMgr::receive4() and
+            // callback in CommandMgr will be called, if necessary.
+            isc::config::CommandMgr::instance().openCommandSocket(sock_cfg);
+        }
+    }
+}
+
 isc::data::ConstElementPtr
 isc::data::ConstElementPtr
 configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) {
 configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) {
     if (!config_set) {
     if (!config_set) {
@@ -870,6 +907,20 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) {
                 continue;
                 continue;
             }
             }
 
 
+            if (config_pair.first == "mac-sources") {
+                MACSourcesListConfigParser parser;
+                CfgMACSource& mac_source = CfgMgr::instance().getStagingCfg()->getMACSources();
+                parser.parse(mac_source, config_pair.second);
+                continue;
+            }
+
+            if (config_pair.first == "control-socket") {
+                ControlSocketParser parser;
+                SrvConfigPtr srv_config = CfgMgr::instance().getStagingCfg();
+                parser.parse(*srv_config, config_pair.second);
+                continue;
+            }
+
             ParserPtr parser(createGlobal6DhcpConfigParser(config_pair.first,
             ParserPtr parser(createGlobal6DhcpConfigParser(config_pair.first,
                                                            config_pair.second));
                                                            config_pair.second));
             LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, DHCP6_PARSER_CREATED)
             LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, DHCP6_PARSER_CREATED)
@@ -921,25 +972,8 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) {
             subnet_parser->build(subnet_config->second);
             subnet_parser->build(subnet_config->second);
         }
         }
 
 
-        // Get command socket configuration from the config file.
-        // This code expects the following structure:
-        // {
-        //     "socket-type": "unix",
-        //     "socket-name": "/tmp/kea6.sock"
-        // }
-        ConstElementPtr sock_cfg =
-            CfgMgr::instance().getStagingCfg()->getControlSocketInfo();
-
-        // Close existing socket (if any).
-        isc::config::CommandMgr::instance().closeCommandSocket();
-        if (sock_cfg) {
-            // This will create a control socket and will install external socket
-            // in IfaceMgr. That socket will be monitored when Dhcp4Srv::receivePacket()
-            // calls IfaceMgr::receive4() and callback in CommandMgr will be called,
-            // if necessary. If there were previously open command socket, it will
-            // be closed.
-            isc::config::CommandMgr::instance().openCommandSocket(sock_cfg);
-        }
+        // Setup the command channel.
+        configureCommandChannel();
 
 
         // The lease database parser is the last to be run.
         // The lease database parser is the last to be run.
         std::map<std::string, ConstElementPtr>::const_iterator leases_config =
         std::map<std::string, ConstElementPtr>::const_iterator leases_config =

+ 3 - 33
src/bin/dhcp6/kea_controller.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -38,11 +38,6 @@ void configure(const std::string& file_name) {
     // This is a configuration backend implementation that reads the
     // This is a configuration backend implementation that reads the
     // configuration from a JSON file.
     // configuration from a JSON file.
 
 
-    // We are starting the configuration process so we should remove any
-    // staging configuration that has been created during previous
-    // configuration attempts.
-    CfgMgr::instance().rollback();
-
     isc::data::ConstElementPtr json;
     isc::data::ConstElementPtr json;
     isc::data::ConstElementPtr dhcp6;
     isc::data::ConstElementPtr dhcp6;
     isc::data::ConstElementPtr logger;
     isc::data::ConstElementPtr logger;
@@ -74,29 +69,14 @@ void configure(const std::string& file_name) {
                       " Did you forget to add { } around your configuration?");
                       " Did you forget to add { } around your configuration?");
         }
         }
 
 
-        // Let's configure logging before applying the configuration,
-        // so we can log things during configuration process.
-        // If there's no logging element, we'll just pass NULL pointer,
-        // which will be handled by configureLogger().
-        Daemon::configureLogger(json->get("Logging"),
-                                CfgMgr::instance().getStagingCfg());
-
-        // Get Dhcp6 component from the config
-        dhcp6 = json->get("Dhcp6");
-
-        if (!dhcp6) {
-            isc_throw(isc::BadValue, "no mandatory 'Dhcp6' entry in"
-                      " the configuration");
-        }
-
         // Use parsed JSON structures to configure the server
         // Use parsed JSON structures to configure the server
-        result = ControlledDhcpv6Srv::processCommand("config-reload", dhcp6);
+        result = ControlledDhcpv6Srv::processCommand("set-config", json);
         if (!result) {
         if (!result) {
             // Undetermined status of the configuration. This should never
             // Undetermined status of the configuration. This should never
             // happen, but as the configureDhcp6Server returns a pointer, it is
             // happen, but as the configureDhcp6Server returns a pointer, it is
             // theoretically possible that it will return NULL.
             // theoretically possible that it will return NULL.
             isc_throw(isc::BadValue, "undefined result of "
             isc_throw(isc::BadValue, "undefined result of "
-                      "processCommand(\"config-reload\", dhcp6)");
+                      "processCommand(\"set-config\", json)");
         }
         }
 
 
         // Now check is the returned result is successful (rcode=0) or not
         // Now check is the returned result is successful (rcode=0) or not
@@ -109,15 +89,6 @@ void configure(const std::string& file_name) {
                 "no details available";
                 "no details available";
             isc_throw(isc::BadValue, reason);
             isc_throw(isc::BadValue, reason);
         }
         }
-
-        // If configuration was parsed successfully, apply the new logger
-        // configuration to log4cplus. It is done before commit in case
-        // something goes wrong.
-        CfgMgr::instance().getStagingCfg()->applyLoggingCfg();
-
-        // Use new configuration.
-        CfgMgr::instance().commit();
-
     }  catch (const std::exception& ex) {
     }  catch (const std::exception& ex) {
         // If configuration failed at any stage, we drop the staging
         // If configuration failed at any stage, we drop the staging
         // configuration and continue to use the previous one.
         // configuration and continue to use the previous one.
@@ -128,7 +99,6 @@ void configure(const std::string& file_name) {
         isc_throw(isc::BadValue, "configuration error using file '"
         isc_throw(isc::BadValue, "configuration error using file '"
                   << file_name << "': " << ex.what());
                   << file_name << "': " << ex.what());
     }
     }
-
 }
 }
 
 
 /// @brief Signals handler for DHCPv6 server.
 /// @brief Signals handler for DHCPv6 server.

+ 49 - 28
src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -251,6 +251,9 @@ public:
         ASSERT_TRUE(status);
         ASSERT_TRUE(status);
         comment_ = parseAnswer(rcode_, status);
         comment_ = parseAnswer(rcode_, status);
         EXPECT_EQ(expected_code, rcode_);
         EXPECT_EQ(expected_code, rcode_);
+        if (expected_code != rcode_) {
+            cout << "The comment returned was: [" << comment_->stringValue() << "]" << endl;
+        }
     }
     }
 
 
     /// @brief Returns an interface configuration used by the most of the
     /// @brief Returns an interface configuration used by the most of the
@@ -762,7 +765,7 @@ TEST_F(Dhcp6ParserTest, emptySubnet) {
 
 
     ConstElementPtr status;
     ConstElementPtr status;
     EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
     EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
-                    
+
     // returned value should be 0 (success)
     // returned value should be 0 (success)
     checkResult(status, 0);
     checkResult(status, 0);
 }
 }
@@ -4292,16 +4295,15 @@ TEST_F(Dhcp6ParserTest, reservationBogus) {
 }
 }
 
 
 /// The goal of this test is to verify that configuration can include
 /// The goal of this test is to verify that configuration can include
-/// MAC/Hardware sources. This test also checks if the aliases are
-/// handled properly (rfc6939 = client-addr-relay, rfc4649 = remote-id,
-/// rfc4580 = subscriber-id).
-TEST_F(Dhcp6ParserTest, macSources) {
+/// MAC/Hardware sources. This case uses RFC numbers to reference methods.
+/// Also checks if the aliases are handled properly (rfc6939 = client-addr-relay,
+/// rfc4649 = remote-id, rfc4580 = subscriber-id).
+TEST_F(Dhcp6ParserTest, macSources1) {
 
 
     ConstElementPtr json;
     ConstElementPtr json;
     ASSERT_NO_THROW(json =
     ASSERT_NO_THROW(json =
         parseDHCP6("{ " + genIfaceConfig() + ","
         parseDHCP6("{ " + genIfaceConfig() + ","
-                   "\"mac-sources\": [ \"rfc6939\", \"rfc4649\", \"rfc4580\","
-                   "\"client-link-addr-option\", \"remote-id\", \"subscriber-id\"],"
+                   "\"mac-sources\": [ \"rfc6939\", \"rfc4649\", \"rfc4580\" ],"
                    "\"preferred-lifetime\": 3000,"
                    "\"preferred-lifetime\": 3000,"
                    "\"rebind-timer\": 2000, "
                    "\"rebind-timer\": 2000, "
                    "\"renew-timer\": 1000, "
                    "\"renew-timer\": 1000, "
@@ -4310,28 +4312,49 @@ TEST_F(Dhcp6ParserTest, macSources) {
 
 
     ConstElementPtr status;
     ConstElementPtr status;
     EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
     EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
+    checkResult(status, 0);
 
 
-    // returned value should be 0 (success)
+    CfgMACSources sources = CfgMgr::instance().getStagingCfg()->getMACSources().get();
+
+    ASSERT_EQ(3, sources.size());
+    // Let's check the aliases. They should be recognized to their base methods.
+    EXPECT_EQ(HWAddr::HWADDR_SOURCE_CLIENT_ADDR_RELAY_OPTION, sources[0]);
+    EXPECT_EQ(HWAddr::HWADDR_SOURCE_REMOTE_ID, sources[1]);
+    EXPECT_EQ(HWAddr::HWADDR_SOURCE_SUBSCRIBER_ID, sources[2]);
+}
+
+/// The goal of this test is to verify that configuration can include
+/// MAC/Hardware sources. This uses specific method names.
+TEST_F(Dhcp6ParserTest, macSources2) {
+
+    ConstElementPtr json;
+    ASSERT_NO_THROW(json =
+        parseDHCP6("{ " + genIfaceConfig() + ","
+                   "\"mac-sources\": [ \"client-link-addr-option\", \"remote-id\", "
+                   "                   \"subscriber-id\"],"
+                   "\"preferred-lifetime\": 3000,"
+                   "\"rebind-timer\": 2000, "
+                   "\"renew-timer\": 1000, "
+                   "\"subnet6\": [  ], "
+                   "\"valid-lifetime\": 4000 }"));
+
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
     checkResult(status, 0);
     checkResult(status, 0);
 
 
-    CfgMACSources mac_sources = CfgMgr::instance().getStagingCfg()->getMACSources().get();
-    ASSERT_EQ(6, mac_sources.size());
+    CfgMACSources sources = CfgMgr::instance().getStagingCfg()->getMACSources().get();
+
+    ASSERT_EQ(3, sources.size());
     // Let's check the aliases. They should be recognized to their base methods.
     // Let's check the aliases. They should be recognized to their base methods.
-    EXPECT_EQ(HWAddr::HWADDR_SOURCE_CLIENT_ADDR_RELAY_OPTION, mac_sources[0]);
-    EXPECT_EQ(HWAddr::HWADDR_SOURCE_REMOTE_ID, mac_sources[1]);
-    EXPECT_EQ(HWAddr::HWADDR_SOURCE_SUBSCRIBER_ID, mac_sources[2]);
-
-    // Let's check if the actual methods are recognized properly.
-    EXPECT_EQ(HWAddr::HWADDR_SOURCE_CLIENT_ADDR_RELAY_OPTION, mac_sources[3]);
-    EXPECT_EQ(HWAddr::HWADDR_SOURCE_REMOTE_ID, mac_sources[4]);
-    EXPECT_EQ(HWAddr::HWADDR_SOURCE_SUBSCRIBER_ID, mac_sources[5]);
+    EXPECT_EQ(HWAddr::HWADDR_SOURCE_CLIENT_ADDR_RELAY_OPTION, sources[0]);
+    EXPECT_EQ(HWAddr::HWADDR_SOURCE_REMOTE_ID, sources[1]);
+    EXPECT_EQ(HWAddr::HWADDR_SOURCE_SUBSCRIBER_ID, sources[2]);
 }
 }
 
 
-/// The goal of this test is to verify that MAC sources configuration can be
-/// empty.
-/// Note the Dhcp6 parser requires the list to NOT be empty?!
-TEST_F(Dhcp6ParserTest, macSourcesEmpty) {
 
 
+/// The goal of this test is to verify that empty MAC sources configuration
+/// is rejected. If specified, this has to have at least one value.
+TEST_F(Dhcp6ParserTest, macSourcesEmpty) {
     ConstElementPtr status;
     ConstElementPtr status;
 
 
     EXPECT_NO_THROW(status = configureDhcp6Server(srv_,
     EXPECT_NO_THROW(status = configureDhcp6Server(srv_,
@@ -4343,11 +4366,9 @@ TEST_F(Dhcp6ParserTest, macSourcesEmpty) {
                               "\"subnet6\": [  ], "
                               "\"subnet6\": [  ], "
                               "\"valid-lifetime\": 4000 }")));
                               "\"valid-lifetime\": 4000 }")));
 
 
-    // returned value should be 0 (success)
-    checkResult(status, 0);
-
-    CfgMACSources mac_sources = CfgMgr::instance().getStagingCfg()->getMACSources().get();
-    EXPECT_EQ(0, mac_sources.size());
+    // returned value should be 1 (failure), because the mac-sources must not
+    // be empty when specified.
+    checkResult(status, 1);
 }
 }
 
 
 /// The goal of this test is to verify that MAC sources configuration can
 /// The goal of this test is to verify that MAC sources configuration can

+ 159 - 5
src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -15,8 +15,10 @@
 #include <dhcp6/ctrl_dhcp6_srv.h>
 #include <dhcp6/ctrl_dhcp6_srv.h>
 #include <dhcp6/tests/dhcp6_test_utils.h>
 #include <dhcp6/tests/dhcp6_test_utils.h>
 #include <hooks/hooks_manager.h>
 #include <hooks/hooks_manager.h>
+#include <log/logger_support.h>
 #include <stats/stats_mgr.h>
 #include <stats/stats_mgr.h>
 #include <testutils/unix_control_client.h>
 #include <testutils/unix_control_client.h>
+#include <testutils/io_utils.h>
 
 
 #include "marker_file.h"
 #include "marker_file.h"
 #include "test_libraries.h"
 #include "test_libraries.h"
@@ -25,6 +27,7 @@
 #include <gtest/gtest.h>
 #include <gtest/gtest.h>
 
 
 #include <sys/select.h>
 #include <sys/select.h>
+#include <sys/stat.h>
 #include <sys/ioctl.h>
 #include <sys/ioctl.h>
 #include <cstdlib>
 #include <cstdlib>
 
 
@@ -148,6 +151,12 @@ public:
         ConstElementPtr config;
         ConstElementPtr config;
         ASSERT_NO_THROW(config = parseDHCP6(config_txt));
         ASSERT_NO_THROW(config = parseDHCP6(config_txt));
         ConstElementPtr answer = server_->processConfig(config);
         ConstElementPtr answer = server_->processConfig(config);
+
+        // Commit the configuration so any subsequent reconfigurations
+        // will only close the command channel if its configuration has
+        // changed.
+        CfgMgr::instance().commit();
+
         ASSERT_TRUE(answer);
         ASSERT_TRUE(answer);
 
 
         int status = 0;
         int status = 0;
@@ -291,7 +300,7 @@ TEST_F(CtrlDhcpv6SrvTest, configReload) {
 
 
     // Use empty parameters list
     // Use empty parameters list
     // Prepare configuration file.
     // Prepare configuration file.
-    string config_txt = "{ \"interfaces-config\": {"
+    string config_txt = "{ \"Dhcp6\": { \"interfaces-config\": {"
         "  \"interfaces\": [ \"*\" ]"
         "  \"interfaces\": [ \"*\" ]"
         "},"
         "},"
         "\"preferred-lifetime\": 3000,"
         "\"preferred-lifetime\": 3000,"
@@ -310,10 +319,10 @@ TEST_F(CtrlDhcpv6SrvTest, configReload) {
         "    \"pools\": [ { \"pool\": \"2001:db8:3::/80\" } ],"
         "    \"pools\": [ { \"pool\": \"2001:db8:3::/80\" } ],"
         "    \"subnet\": \"2001:db8:3::/64\" "
         "    \"subnet\": \"2001:db8:3::/64\" "
         " } ],"
         " } ],"
-        "\"valid-lifetime\": 4000 }";
+        "\"valid-lifetime\": 4000 }}";
 
 
     ConstElementPtr config;
     ConstElementPtr config;
-    ASSERT_NO_THROW(config = parseDHCP6(config_txt));
+    ASSERT_NO_THROW(config = parseJSON(config_txt));
 
 
     // Make sure there are no subnets configured.
     // Make sure there are no subnets configured.
     CfgMgr::instance().clear();
     CfgMgr::instance().clear();
@@ -327,13 +336,158 @@ TEST_F(CtrlDhcpv6SrvTest, configReload) {
 
 
     // Check that the config was indeed applied.
     // Check that the config was indeed applied.
     const Subnet6Collection* subnets =
     const Subnet6Collection* subnets =
-        CfgMgr::instance().getStagingCfg()->getCfgSubnets6()->getAll();
+        CfgMgr::instance().getCurrentCfg()->getCfgSubnets6()->getAll();
     EXPECT_EQ(3, subnets->size());
     EXPECT_EQ(3, subnets->size());
 
 
     // Clean up after the test.
     // Clean up after the test.
     CfgMgr::instance().clear();
     CfgMgr::instance().clear();
 }
 }
 
 
+// Check that the "set-config" command will replace current configuration
+TEST_F(CtrlChannelDhcpv6SrvTest, set_config) {
+    createUnixChannelServer();
+
+    // Define strings to permutate the config arguments
+    // (Note the line feeds makes errors easy to find)
+    string set_config_txt = "{ \"command\": \"set-config\" \n";
+    string args_txt = " \"arguments\": { \n";
+    string dhcp6_cfg_txt =
+        "    \"Dhcp6\": { \n"
+        "        \"interfaces-config\": { \n"
+        "            \"interfaces\": [\"*\"] \n"
+        "        },   \n"
+        "        \"preferred-lifetime\": 3000, \n"
+        "        \"valid-lifetime\": 4000, \n"
+        "        \"renew-timer\": 1000, \n"
+        "        \"rebind-timer\": 2000, \n"
+        "        \"lease-database\": { \n"
+        "           \"type\": \"memfile\", \n"
+        "           \"persist\":false, \n"
+        "           \"lfc-interval\": 0  \n"
+        "        }, \n"
+        "        \"expired-leases-processing\": { \n"
+        "            \"reclaim-timer-wait-time\": 0, \n"
+        "            \"hold-reclaimed-time\": 0, \n"
+        "            \"flush-reclaimed-timer-wait-time\": 0 \n"
+        "        },"
+        "        \"subnet6\": [ \n";
+    string subnet1 =
+        "               {\"subnet\": \"3002::/64\", \n"
+        "                \"pools\": [{ \"pool\": \"3002::100-3002::200\" }]}\n";
+    string subnet2 =
+        "               {\"subnet\": \"3003::/64\", \n"
+        "                \"pools\": [{ \"pool\": \"3003::100-3003::200\" }]}\n";
+    string bad_subnet =
+        "               {\"BOGUS\": \"3005::/64\", \n"
+        "                \"pools\": [{ \"pool\": \"3005::100-3005::200\" }]}\n";
+    string subnet_footer =
+        "          ] \n";
+    string control_socket_header =
+        "       ,\"control-socket\": { \n"
+        "       \"socket-type\": \"unix\", \n"
+        "       \"socket-name\": \"";
+    string control_socket_footer =
+        "\"   \n} \n";
+    string logger_txt =
+        "    \"Logging\": { \n"
+        "        \"loggers\": [ { \n"
+        "            \"name\": \"kea\", \n"
+        "            \"severity\": \"FATAL\", \n"
+        "            \"output_options\": [{ \n"
+        "                \"output\": \"/dev/null\" \n"
+        "            }] \n"
+        "        }] \n"
+        "    } \n";
+
+    std::ostringstream os;
+
+    // Create a valid config with all the parts should parse
+    os << set_config_txt << ","
+        << args_txt
+        << dhcp6_cfg_txt
+        << subnet1
+        << subnet_footer
+        << control_socket_header
+        << socket_path_
+        << control_socket_footer
+        << "}\n"                      // close dhcp6
+        << ","
+        << logger_txt
+        << "}}";
+
+    // Send the set-config command
+    std::string response;
+    sendUnixCommand(os.str(), response);
+
+    // Verify the configuration was successful.
+    EXPECT_EQ("{ \"result\": 0, \"text\": \"Configuration successful.\" }",
+              response);
+
+    // Check that the config was indeed applied.
+    const Subnet6Collection* subnets =
+        CfgMgr::instance().getCurrentCfg()->getCfgSubnets6()->getAll();
+    EXPECT_EQ(1, subnets->size());
+
+    // Create a config with malformed subnet that should fail to parse.
+    os.str("");
+    os << set_config_txt << ","
+        << args_txt
+        << dhcp6_cfg_txt
+        << bad_subnet
+        << subnet_footer
+        << control_socket_header
+        << socket_path_
+        << control_socket_footer
+        << "}\n"                      // close dhcp6
+        "}}";
+
+    // Send the set-config command
+    sendUnixCommand(os.str(), response);
+
+    // Should fail with a syntax error
+    EXPECT_EQ("{ \"result\": 1, "
+              "\"text\": \"unsupported parameter: BOGUS (<string>:21:26)\" }",
+              response);
+
+    // Check that the config was not lost
+    subnets = CfgMgr::instance().getCurrentCfg()->getCfgSubnets6()->getAll();
+    EXPECT_EQ(1, subnets->size());
+
+    // Create a valid config with two subnets and no command channel.
+    // It should succeed but client will not receive a the response
+    os.str("");
+    os << set_config_txt << ","
+        << args_txt
+        << dhcp6_cfg_txt
+        << subnet1
+        << ",\n"
+        << subnet2
+        << subnet_footer
+        << "}\n"                      // close dhcp6
+        << "}}";
+
+    /* Verify the control channel socket exists */
+    ASSERT_TRUE(fileExists(socket_path_));
+
+    // Send the set-config command
+    sendUnixCommand(os.str(), response);
+
+    /* Verify the control channel socket no longer exists */
+    EXPECT_FALSE(fileExists(socket_path_));
+
+    // With no command channel, should still receive the response.
+    EXPECT_EQ("{ \"result\": 0, \"text\": \"Configuration successful.\" }",
+              response);
+
+    // Check that the config was not lost
+    subnets = CfgMgr::instance().getCurrentCfg()->getCfgSubnets6()->getAll();
+    EXPECT_EQ(2, subnets->size());
+
+    // Clean up after the test.
+    CfgMgr::instance().clear();
+}
+
+
 typedef std::map<std::string, isc::data::ConstElementPtr> ElementMap;
 typedef std::map<std::string, isc::data::ConstElementPtr> ElementMap;
 
 
 // This test checks which commands are registered by the DHCPv4 server.
 // This test checks which commands are registered by the DHCPv4 server.

+ 5 - 1
src/bin/dhcp6/tests/dhcp6_test_utils.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -10,6 +10,7 @@
 #include <dhcp6/tests/dhcp6_test_utils.h>
 #include <dhcp6/tests/dhcp6_test_utils.h>
 #include <dhcp6/json_config_parser.h>
 #include <dhcp6/json_config_parser.h>
 #include <dhcp/tests/pkt_captures.h>
 #include <dhcp/tests/pkt_captures.h>
+#include <log/logger_support.h>
 #include <util/pointer_util.h>
 #include <util/pointer_util.h>
 #include <cc/command_interpreter.h>
 #include <cc/command_interpreter.h>
 #include <stats/stats_mgr.h>
 #include <stats/stats_mgr.h>
@@ -46,6 +47,9 @@ BaseServerTest::~BaseServerTest() {
 
 
     // Revert to original data directory.
     // Revert to original data directory.
     CfgMgr::instance().setDataDir(original_datadir_);
     CfgMgr::instance().setDataDir(original_datadir_);
+
+    // Revert to unit test logging in case the test reconfigured logging.
+    isc::log::initLogger();
 }
 }
 
 
 Dhcpv6SrvTest::Dhcpv6SrvTest()
 Dhcpv6SrvTest::Dhcpv6SrvTest()

+ 75 - 48
src/bin/dhcp6/tests/parser_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2016-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -7,28 +7,42 @@
 #include <gtest/gtest.h>
 #include <gtest/gtest.h>
 #include <cc/data.h>
 #include <cc/data.h>
 #include <dhcp6/parser_context.h>
 #include <dhcp6/parser_context.h>
+#include <testutils/io_utils.h>
 
 
 using namespace isc::data;
 using namespace isc::data;
 using namespace std;
 using namespace std;
 
 
-namespace {
-
-void compareJSON(ConstElementPtr a, ConstElementPtr b, bool print = true) {
+namespace isc {
+namespace dhcp {
+namespace test {
+
+/// @brief compares two JSON trees
+///
+/// If differences are discovered, gtest failure is reported (using EXPECT_EQ)
+///
+/// @param a first to be compared
+/// @param b second to be compared
+void compareJSON(ConstElementPtr a, ConstElementPtr b) {
     ASSERT_TRUE(a);
     ASSERT_TRUE(a);
     ASSERT_TRUE(b);
     ASSERT_TRUE(b);
-    if (print) {
-        // std::cout << "JSON A: -----" << endl << a->str() << std::endl;
-        // std::cout << "JSON B: -----" << endl << b->str() << std::endl;
-        // cout << "---------" << endl << endl;
-    }
     EXPECT_EQ(a->str(), b->str());
     EXPECT_EQ(a->str(), b->str());
 }
 }
 
 
-void testParser(const std::string& txt, Parser6Context::ParserType parser_type) {
-    ElementPtr reference_json;
+/// @brief Tests if the input string can be parsed with specific parser
+///
+/// The input text will be passed to bison parser of specified type.
+/// Then the same input text is passed to legacy JSON parser and outputs
+/// from both parsers are compared. The legacy comparison can be disabled,
+/// if the feature tested is not supported by the old parser (e.g.
+/// new comment styles)
+///
+/// @param txt text to be compared
+/// @param parser_type bison parser type to be instantiated
+/// @param compare whether to compare the output with legacy JSON parser
+void testParser(const std::string& txt, Parser6Context::ParserType parser_type,
+    bool compare = true) {
     ConstElementPtr test_json;
     ConstElementPtr test_json;
 
 
-    ASSERT_NO_THROW(reference_json = Element::fromJSON(txt, true));
     ASSERT_NO_THROW({
     ASSERT_NO_THROW({
             try {
             try {
                 Parser6Context ctx;
                 Parser6Context ctx;
@@ -40,29 +54,16 @@ void testParser(const std::string& txt, Parser6Context::ParserType parser_type)
 
 
     });
     });
 
 
+    if (!compare) {
+        return;
+    }
+
     // Now compare if both representations are the same.
     // Now compare if both representations are the same.
+    ElementPtr reference_json;
+    ASSERT_NO_THROW(reference_json = Element::fromJSON(txt, true));
     compareJSON(reference_json, test_json);
     compareJSON(reference_json, test_json);
 }
 }
 
 
-void testParser2(const std::string& txt, Parser6Context::ParserType parser_type) {
-    ConstElementPtr test_json;
-
-    ASSERT_NO_THROW({
-            try {
-                Parser6Context ctx;
-                test_json = ctx.parseString(txt, parser_type);
-            } catch (const std::exception &e) {
-                cout << "EXCEPTION: " << e.what() << endl;
-                throw;
-            }
-    });
-    /// @todo: Implement actual validation here. since the original
-    /// Element::fromJSON does not support several comment types, we don't
-    /// have anything to compare with.
-    /// std::cout << "Original text:" << txt << endl;
-    /// std::cout << "Parsed text  :" << test_json->str() << endl;
-}
-
 TEST(ParserTest, mapInMap) {
 TEST(ParserTest, mapInMap) {
     string txt = "{ \"xyzzy\": { \"foo\": 123, \"baz\": 456 } }";
     string txt = "{ \"xyzzy\": { \"foo\": 123, \"baz\": 456 } }";
     testParser(txt, Parser6Context::PARSER_JSON);
     testParser(txt, Parser6Context::PARSER_JSON);
@@ -125,9 +126,11 @@ TEST(ParserTest, keywordDhcp6) {
                   "    \"subnet\": \"2001:db8:1::/48\", "
                   "    \"subnet\": \"2001:db8:1::/48\", "
                   "    \"interface\": \"test\" } ],\n"
                   "    \"interface\": \"test\" } ],\n"
                    "\"valid-lifetime\": 4000 } }";
                    "\"valid-lifetime\": 4000 } }";
-     testParser2(txt, Parser6Context::PARSER_DHCP6);
+     testParser(txt, Parser6Context::PARSER_DHCP6);
 }
 }
 
 
+// Tests if bash (#) comments are supported. That's the only comment type that
+// was supported by the old parser.
 TEST(ParserTest, bashComments) {
 TEST(ParserTest, bashComments) {
     string txt= "{ \"Dhcp6\": { \"interfaces-config\": {"
     string txt= "{ \"Dhcp6\": { \"interfaces-config\": {"
                 "  \"interfaces\": [ \"*\" ]"
                 "  \"interfaces\": [ \"*\" ]"
@@ -144,10 +147,11 @@ TEST(ParserTest, bashComments) {
                 "    \"interface\": \"eth0\""
                 "    \"interface\": \"eth0\""
                 " } ],"
                 " } ],"
                 "\"valid-lifetime\": 4000 } }";
                 "\"valid-lifetime\": 4000 } }";
-    testParser2(txt, Parser6Context::PARSER_DHCP6);
+    testParser(txt, Parser6Context::PARSER_DHCP6);
 }
 }
 
 
-TEST(ParserTest, cComments) {
+// Tests if C++ (//) comments can start anywhere, not just in the first line.
+TEST(ParserTest, cppComments) {
     string txt= "{ \"Dhcp6\": { \"interfaces-config\": {"
     string txt= "{ \"Dhcp6\": { \"interfaces-config\": {"
                 "  \"interfaces\": [ \"*\" ]"
                 "  \"interfaces\": [ \"*\" ]"
                 "},\n"
                 "},\n"
@@ -160,9 +164,10 @@ TEST(ParserTest, cComments) {
                 "    \"interface\": \"eth0\""
                 "    \"interface\": \"eth0\""
                 " } ],"
                 " } ],"
                 "\"valid-lifetime\": 4000 } }";
                 "\"valid-lifetime\": 4000 } }";
-    testParser2(txt, Parser6Context::PARSER_DHCP6);
+    testParser(txt, Parser6Context::PARSER_DHCP6, false);
 }
 }
 
 
+// Tests if bash (#) comments can start anywhere, not just in the first line.
 TEST(ParserTest, bashCommentsInline) {
 TEST(ParserTest, bashCommentsInline) {
     string txt= "{ \"Dhcp6\": { \"interfaces-config\": {"
     string txt= "{ \"Dhcp6\": { \"interfaces-config\": {"
                 "  \"interfaces\": [ \"*\" ]"
                 "  \"interfaces\": [ \"*\" ]"
@@ -176,9 +181,10 @@ TEST(ParserTest, bashCommentsInline) {
                 "    \"interface\": \"eth0\""
                 "    \"interface\": \"eth0\""
                 " } ],"
                 " } ],"
                 "\"valid-lifetime\": 4000 } }";
                 "\"valid-lifetime\": 4000 } }";
-    testParser2(txt, Parser6Context::PARSER_DHCP6);
+    testParser(txt, Parser6Context::PARSER_DHCP6, false);
 }
 }
 
 
+// Tests if multi-line C style comments are handled correctly.
 TEST(ParserTest, multilineComments) {
 TEST(ParserTest, multilineComments) {
     string txt= "{ \"Dhcp6\": { \"interfaces-config\": {"
     string txt= "{ \"Dhcp6\": { \"interfaces-config\": {"
                 "  \"interfaces\": [ \"*\" ]"
                 "  \"interfaces\": [ \"*\" ]"
@@ -193,17 +199,29 @@ TEST(ParserTest, multilineComments) {
                 "    \"interface\": \"eth0\""
                 "    \"interface\": \"eth0\""
                 " } ],"
                 " } ],"
                 "\"valid-lifetime\": 4000 } }";
                 "\"valid-lifetime\": 4000 } }";
-    testParser2(txt, Parser6Context::PARSER_DHCP6);
+    testParser(txt, Parser6Context::PARSER_DHCP6, false);
 }
 }
 
 
-
-void testFile(const std::string& fname, bool print) {
+/// @brief Loads specified example config file
+///
+/// This test loads specified example file twice: first, using the legacy
+/// JSON file and then second time using bison parser. Two created Element
+/// trees are then compared. The input is decommented before it is passed
+/// to legacy parser (as legacy support for comments is very limited).
+///
+/// @param fname name of the file to be loaded
+void testFile(const std::string& fname) {
     ElementPtr reference_json;
     ElementPtr reference_json;
     ConstElementPtr test_json;
     ConstElementPtr test_json;
 
 
-    cout << "Attempting to load file " << fname << endl;
+    string decommented = decommentJSONfile(fname);
+
+    cout << "Parsing file " << fname << "(" << decommented << ")" << endl;
+
+    EXPECT_NO_THROW(reference_json = Element::fromJSONFile(decommented, true));
 
 
-    EXPECT_NO_THROW(reference_json = Element::fromJSONFile(fname, true));
+    // remove the temporary file
+    EXPECT_NO_THROW(::remove(decommented.c_str()));
 
 
     EXPECT_NO_THROW(
     EXPECT_NO_THROW(
     try {
     try {
@@ -217,9 +235,7 @@ void testFile(const std::string& fname, bool print) {
     ASSERT_TRUE(reference_json);
     ASSERT_TRUE(reference_json);
     ASSERT_TRUE(test_json);
     ASSERT_TRUE(test_json);
 
 
-    compareJSON(reference_json, test_json, print);
-
-
+    compareJSON(reference_json, test_json);
 }
 }
 
 
 // This test loads all available existing files. Each config is loaded
 // This test loads all available existing files. Each config is loaded
@@ -243,10 +259,15 @@ TEST(ParserTest, file) {
     configs.push_back("stateless.json");
     configs.push_back("stateless.json");
 
 
     for (int i = 0; i<configs.size(); i++) {
     for (int i = 0; i<configs.size(); i++) {
-        testFile(string(CFG_EXAMPLES) + "/" + configs[i], false);
+        testFile(string(CFG_EXAMPLES) + "/" + configs[i]);
     }
     }
 }
 }
 
 
+/// @brief Tests error conditions in Dhcp6Parser
+///
+/// @param txt text to be parsed
+/// @param parser_type type of the parser to be used in the test
+/// @param msg expected content of the exception
 void testError(const std::string& txt,
 void testError(const std::string& txt,
                Parser6Context::ParserType parser_type,
                Parser6Context::ParserType parser_type,
                const std::string& msg)
                const std::string& msg)
@@ -265,7 +286,7 @@ void testError(const std::string& txt,
     }
     }
 }
 }
 
 
-// Check errors
+// Verify that error conditions are handled correctly.
 TEST(ParserTest, errors) {
 TEST(ParserTest, errors) {
     // no input
     // no input
     testError("", Parser6Context::PARSER_JSON,
     testError("", Parser6Context::PARSER_JSON,
@@ -507,9 +528,13 @@ TEST(ParserTest, unicodeEscapes) {
         ASSERT_EQ(Element::string, result->getType());
         ASSERT_EQ(Element::string, result->getType());
         EXPECT_EQ(ins, result->stringValue());
         EXPECT_EQ(ins, result->stringValue());
     }
     }
+}
 
 
+// This test checks that all representations of a slash is recognized properly.
+TEST(ParserTest, unicodeSlash) {
     // check the 4 possible encodings of solidus '/'
     // check the 4 possible encodings of solidus '/'
-    json = "\"/\\/\\u002f\\u002F\"";
+    ConstElementPtr result;
+    string json = "\"/\\/\\u002f\\u002F\"";
     ASSERT_NO_THROW(
     ASSERT_NO_THROW(
     try {
     try {
         Parser6Context ctx;
         Parser6Context ctx;
@@ -520,6 +545,8 @@ TEST(ParserTest, unicodeEscapes) {
     });
     });
     ASSERT_EQ(Element::string, result->getType());
     ASSERT_EQ(Element::string, result->getType());
     EXPECT_EQ("////", result->stringValue());
     EXPECT_EQ("////", result->stringValue());
-}       
+}
 
 
 };
 };
+};
+};

+ 29 - 3
src/lib/config/command_mgr.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2015-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -11,6 +11,7 @@
 #include <dhcp/iface_mgr.h>
 #include <dhcp/iface_mgr.h>
 #include <config/config_log.h>
 #include <config/config_log.h>
 #include <boost/bind.hpp>
 #include <boost/bind.hpp>
+#include <unistd.h>
 
 
 using namespace isc::data;
 using namespace isc::data;
 
 
@@ -145,6 +146,19 @@ CommandMgr::commandReader(int sockfd) {
         return;
         return;
     }
     }
 
 
+    // Duplicate the connection's socket in the event, the command causes the
+    // channel to close (like a reconfig).  This permits us to always have
+    // a socket on which to respond. If for some reason  we can't fall back
+    // to the connection socket.
+    int rsp_fd = dup(sockfd);
+    if (rsp_fd < 0 ) {
+        // Highly unlikely
+        const char* errmsg = strerror(errno);
+        LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_SOCKET_DUP_WARN)
+                  .arg(errmsg);
+        rsp_fd = sockfd;
+    }
+
     LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_SOCKET_READ).arg(rval).arg(sockfd);
     LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_SOCKET_READ).arg(rval).arg(sockfd);
 
 
     // Ok, we received something. Let's see if we can make any sense of it.
     // Ok, we received something. Let's see if we can make any sense of it.
@@ -163,6 +177,11 @@ CommandMgr::commandReader(int sockfd) {
 
 
     if (!rsp) {
     if (!rsp) {
         LOG_WARN(command_logger, COMMAND_RESPONSE_ERROR);
         LOG_WARN(command_logger, COMMAND_RESPONSE_ERROR);
+        // Only close the duped socket if it's different (should be)
+        if (rsp_fd != sockfd) {
+            close(rsp_fd);
+        }
+
         return;
         return;
     }
     }
 
 
@@ -179,7 +198,8 @@ CommandMgr::commandReader(int sockfd) {
     }
     }
 
 
     // Send the data back over socket.
     // Send the data back over socket.
-    rval = write(sockfd, txt.c_str(), len);
+    rval = write(rsp_fd, txt.c_str(), len);
+    int saverr = errno;
 
 
     LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_SOCKET_WRITE).arg(len).arg(sockfd);
     LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_SOCKET_WRITE).arg(len).arg(sockfd);
 
 
@@ -187,7 +207,13 @@ CommandMgr::commandReader(int sockfd) {
         // Response transmission failed. Since the response failed, it doesn't
         // Response transmission failed. Since the response failed, it doesn't
         // make sense to send any status codes. Let's log it and be done with
         // make sense to send any status codes. Let's log it and be done with
         // it.
         // it.
-        LOG_ERROR(command_logger, COMMAND_SOCKET_WRITE_FAIL).arg(len).arg(sockfd);
+        LOG_ERROR(command_logger, COMMAND_SOCKET_WRITE_FAIL)
+                  .arg(len).arg(sockfd).arg(strerror(saverr));
+    }
+
+    // Only close the duped socket if it's different (should be)
+    if (rsp_fd != sockfd) {
+        close(rsp_fd);
     }
     }
 }
 }
 
 

+ 9 - 2
src/lib/config/config_messages.mes

@@ -1,4 +1,4 @@
-# Copyright (C) 2011-2015 Internet Systems Consortium, Inc. ("ISC")
+# Copyright (C) 2011-2017 Internet Systems Consortium, Inc. ("ISC")
 #
 #
 # This Source Code Form is subject to the terms of the Mozilla Public
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -57,6 +57,13 @@ This error message indicates that the server failed to set non-blocking mode
 on just created socket. That socket was created for accepting specific
 on just created socket. That socket was created for accepting specific
 incoming connection. Additional information may be provided as third parameter.
 incoming connection. Additional information may be provided as third parameter.
 
 
+% COMMAND_SOCKET_DUP_WARN Failed to duplicate socket for response: %1
+This debug message indicates that the commandReader was unable to duplicate
+the connection socket prior to executing the command. This is most likely a
+system resource issue.  The command should still be processed and the response
+sent, unless the command caused the command channel to be closed (e.g. a
+reconfiguration command).
+
 % COMMAND_SOCKET_READ Received %1 bytes over command socket %2
 % COMMAND_SOCKET_READ Received %1 bytes over command socket %2
 This debug message indicates that specified number of bytes was received
 This debug message indicates that specified number of bytes was received
 over command socket identified by specified file descriptor.
 over command socket identified by specified file descriptor.
@@ -86,6 +93,6 @@ descriptor and path specified.
 This debug message indicates that the specified number of bytes was sent
 This debug message indicates that the specified number of bytes was sent
 over command socket identifier by the specified file descriptor.
 over command socket identifier by the specified file descriptor.
 
 
-% COMMAND_SOCKET_WRITE_FAIL Error while writing %1 bytes to command socket %2
+% COMMAND_SOCKET_WRITE_FAIL Error while writing %1 bytes to command socket %2 : %3
 This error message indicates that an error was encountered while
 This error message indicates that an error was encountered while
 attempting to send a response to the command socket.
 attempting to send a response to the command socket.

+ 11 - 1
src/lib/dhcpsrv/cfg_mac_source.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2015,2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -47,6 +47,16 @@ uint32_t CfgMACSource::MACSourceFromText(const std::string& name) {
     isc_throw(BadValue, "Can't convert '" << name << "' to any known MAC source.");
     isc_throw(BadValue, "Can't convert '" << name << "' to any known MAC source.");
 }
 }
 
 
+void CfgMACSource::add(uint32_t source) {
+    for (CfgMACSources::const_iterator it = mac_sources_.begin();
+         it != mac_sources_.end(); ++it) {
+        if (*it == source) {
+            isc_throw(InvalidParameter, "mac-source paramter " << source
+                      << "' specified twice.");
+        }
+    }
+    mac_sources_.push_back(source);
+}
 
 
 };
 };
 };
 };

+ 3 - 5
src/lib/dhcpsrv/cfg_mac_source.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2015,2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -52,10 +52,8 @@ class CfgMACSource {
     /// @param source MAC source (see constants in Pkt::HWADDR_SOURCE_*)
     /// @param source MAC source (see constants in Pkt::HWADDR_SOURCE_*)
     ///
     ///
     /// Specified source is being added to the mac_sources_ array.
     /// Specified source is being added to the mac_sources_ array.
-    /// @todo implement add(string) version of this method.
-    void add(uint32_t source) {
-        mac_sources_.push_back(source);
-    }
+    /// @throw InvalidParameter if such a source is already defined.
+    void add(uint32_t source);
 
 
     /// @brief Provides access to the configure MAC/Hardware address sources.
     /// @brief Provides access to the configure MAC/Hardware address sources.
     ///
     ///

+ 71 - 79
src/lib/dhcpsrv/parsers/dhcp_parsers.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -177,62 +177,48 @@ template <> void ValueParser<std::string>::build(ConstElementPtr value) {
 
 
 // ******************** MACSourcesListConfigParser *************************
 // ******************** MACSourcesListConfigParser *************************
 
 
-MACSourcesListConfigParser::
-MACSourcesListConfigParser(const std::string& param_name,
-                           ParserContextPtr global_context)
-    : param_name_(param_name), global_context_(global_context) {
-    if (param_name_ != "mac-sources") {
-        isc_throw(BadValue, "Internal error. MAC sources configuration "
-            "parser called for the wrong parameter: " << param_name);
-    }
-}
-
 void
 void
-MACSourcesListConfigParser::build(ConstElementPtr value) {
+MACSourcesListConfigParser::parse(CfgMACSource& mac_sources, ConstElementPtr value) {
     CfgIface cfg_iface;
     CfgIface cfg_iface;
     uint32_t source = 0;
     uint32_t source = 0;
+    size_t cnt = 0;
 
 
     // By default, there's only one source defined: ANY.
     // By default, there's only one source defined: ANY.
     // If user specified anything, we need to get rid of that default.
     // If user specified anything, we need to get rid of that default.
-    CfgMgr::instance().getStagingCfg()->getMACSources().clear();
+    mac_sources.clear();
 
 
     BOOST_FOREACH(ConstElementPtr source_elem, value->listValue()) {
     BOOST_FOREACH(ConstElementPtr source_elem, value->listValue()) {
         std::string source_str = source_elem->stringValue();
         std::string source_str = source_elem->stringValue();
         try {
         try {
             source = CfgMACSource::MACSourceFromText(source_str);
             source = CfgMACSource::MACSourceFromText(source_str);
-            CfgMgr::instance().getStagingCfg()->getMACSources().add(source);
+            mac_sources.add(source);
+            ++cnt;
+        } catch (const InvalidParameter& ex) {
+            isc_throw(DhcpConfigError, "The mac-sources value '" << source_str
+                      << "' was specified twice (" << value->getPosition() << ")");
         } catch (const std::exception& ex) {
         } catch (const std::exception& ex) {
             isc_throw(DhcpConfigError, "Failed to convert '"
             isc_throw(DhcpConfigError, "Failed to convert '"
                       << source_str << "' to any recognized MAC source:"
                       << source_str << "' to any recognized MAC source:"
                       << ex.what() << " (" << value->getPosition() << ")");
                       << ex.what() << " (" << value->getPosition() << ")");
         }
         }
     }
     }
-}
 
 
-void
-MACSourcesListConfigParser::commit() {
-    // Nothing to do.
+    if (!cnt) {
+        isc_throw(DhcpConfigError, "If specified, MAC Sources cannot be empty");
+    }
 }
 }
 
 
 // ******************** ControlSocketParser *************************
 // ******************** ControlSocketParser *************************
-ControlSocketParser::ControlSocketParser(const std::string& param_name) {
-    if (param_name != "control-socket") {
-        isc_throw(BadValue, "Internal error. Control socket parser called "
-                  " for wrong parameter:" << param_name);
+void ControlSocketParser::parse(SrvConfig& srv_cfg, isc::data::ConstElementPtr value) {
+    if (!value) {
+        isc_throw(DhcpConfigError, "Logic error: specified control-socket is null");
     }
     }
-}
 
 
-void ControlSocketParser::build(isc::data::ConstElementPtr value) {
     if (value->getType() != Element::map) {
     if (value->getType() != Element::map) {
-        isc_throw(BadValue, "Specified control-socket is expected to be a map"
+        isc_throw(DhcpConfigError, "Specified control-socket is expected to be a map"
                   ", i.e. a structure defined within { }");
                   ", i.e. a structure defined within { }");
     }
     }
-    CfgMgr::instance().getStagingCfg()->setControlSocketInfo(value);
-}
-
-/// @brief Does nothing.
-void ControlSocketParser::commit() {
-    // Nothing to do.
+    srv_cfg.setControlSocketInfo(value);
 }
 }
 
 
 // ******************** HooksLibrariesParser *************************
 // ******************** HooksLibrariesParser *************************
@@ -804,66 +790,66 @@ OptionDefListParser::parse(CfgOptionDefPtr storage, ConstElementPtr option_def_l
 }
 }
 
 
 //****************************** RelayInfoParser ********************************
 //****************************** RelayInfoParser ********************************
-RelayInfoParser::RelayInfoParser(const std::string&,
-                                 const isc::dhcp::Subnet::RelayInfoPtr& relay_info,
-                                 const Option::Universe& family)
-    :storage_(relay_info), local_(isc::asiolink::IOAddress(
-                                  family == Option::V4 ? "0.0.0.0" : "::")),
-     string_values_(new StringStorage()), family_(family) {
-    if (!relay_info) {
-        isc_throw(isc::dhcp::DhcpConfigError, "parser logic error: "
-                  << "relay-info storage may not be NULL");
-    }
-
+RelayInfoParser::RelayInfoParser(const Option::Universe& family)
+    : family_(family) {
 };
 };
 
 
 void
 void
-RelayInfoParser::build(ConstElementPtr relay_info) {
-
-    BOOST_FOREACH(ConfigPair param, relay_info->mapValue()) {
-        ParserPtr parser(createConfigParser(param.first));
-        parser->build(param.second);
-        parser->commit();
+RelayInfoParser::parse(const isc::dhcp::Subnet::RelayInfoPtr& cfg,
+                       ConstElementPtr relay_info) {
+    // Let's start with some sanity checks.
+    if (!relay_info || !cfg) {
+        isc_throw(DhcpConfigError, "Logic error: RelayInfoParser::parse() called "
+                  "with at least one NULL parameter.");
     }
     }
 
 
-    // Get the IP address
-    boost::scoped_ptr<asiolink::IOAddress> ip;
-    try {
-        ip.reset(new asiolink::IOAddress(string_values_->getParam("ip-address")));
-    } catch (...)  {
-        isc_throw(DhcpConfigError, "Failed to parse ip-address "
-                  "value: " << string_values_->getParam("ip-address")
-                  << " (" << string_values_->getPosition("ip-address") << ")");
+    if (relay_info->getType() != Element::map) {
+        isc_throw(DhcpConfigError, "Configuration error: RelayInfoParser::parse() "
+                  "called with non-map parameter");
     }
     }
 
 
-    if ( (ip->isV4() && family_ != Option::V4) ||
-         (ip->isV6() && family_ != Option::V6) ) {
-        isc_throw(DhcpConfigError, "ip-address field " << ip->toText()
-                  << " does not have IP address of expected family type: "
-                  << (family_ == Option::V4 ? "IPv4" : "IPv6")
-                  << " (" << string_values_->getPosition("ip-address") << ")");
-    }
+    // Now create the default value.
+    isc::asiolink::IOAddress ip(family_ == Option::V4 ? IOAddress::IPV4_ZERO_ADDRESS()
+                                : IOAddress::IPV6_ZERO_ADDRESS());
 
 
-    local_.addr_ = *ip;
-}
+    // Now iterate over all parameters. Currently there's only one supported
+    // parameter, so it should be an easy thing to check.
+    bool ip_address_specified = false;
+    BOOST_FOREACH(ConfigPair param, relay_info->mapValue()) {
+        if (param.first == "ip-address") {
+            ip_address_specified = true;
+
+            try {
+                ip = asiolink::IOAddress(param.second->stringValue());
+            } catch (...)  {
+                isc_throw(DhcpConfigError, "Failed to parse ip-address "
+                          "value: " << param.second
+                          << " (" << param.second->getPosition() << ")");
+            }
 
 
-isc::dhcp::ParserPtr
-RelayInfoParser::createConfigParser(const std::string& parameter) {
-    DhcpConfigParser* parser = NULL;
-    if (parameter.compare("ip-address") == 0) {
-        parser = new StringParser(parameter, string_values_);
-    } else {
-        isc_throw(NotImplemented,
-                  "parser error: RelayInfoParser parameter not supported: "
-                  << parameter);
+            // Check if the address family matches.
+            if ( (ip.isV4() && family_ != Option::V4) ||
+                 (ip.isV6() && family_ != Option::V6) ) {
+                isc_throw(DhcpConfigError, "ip-address field " << ip.toText()
+                          << " does not have IP address of expected family type: "
+                          << (family_ == Option::V4 ? "IPv4" : "IPv6")
+                          << " (" << param.second->getPosition() << ")");
+            }
+        } else {
+            isc_throw(NotImplemented,
+                      "parser error: RelayInfoParser parameter not supported: "
+                      << param.second);
+        }
     }
     }
 
 
-    return (isc::dhcp::ParserPtr(parser));
-}
+    if (!ip_address_specified) {
+        isc_throw(DhcpConfigError, "'relay' specified, but mandatory 'ip-address' "
+                  "paramter in it is missing");
+    }
 
 
-void
-RelayInfoParser::commit() {
-    *storage_ = local_;
+    // Ok, we're done with parsing. Let's store the result in the structure
+    // we were given as configuration storage.
+    *cfg = isc::dhcp::Subnet::RelayInfo(ip);
 }
 }
 
 
 //****************************** PoolsListParser ********************************
 //****************************** PoolsListParser ********************************
@@ -1069,6 +1055,12 @@ SubnetConfigParser::build(ConstElementPtr subnet) {
             continue;
             continue;
         }
         }
 
 
+        if (param.first == "relay") {
+            RelayInfoParser parser(global_context_->universe_);
+            parser.parse(relay_info_, param.second);
+            continue;
+        }
+
         ParserPtr parser;
         ParserPtr parser;
         // When unsupported parameter is specified, the function called
         // When unsupported parameter is specified, the function called
         // below will thrown an exception. We have to catch this exception
         // below will thrown an exception. We have to catch this exception

+ 21 - 66
src/lib/dhcpsrv/parsers/dhcp_parsers.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -16,6 +16,8 @@
 #include <dhcpsrv/cfg_option.h>
 #include <dhcpsrv/cfg_option.h>
 #include <dhcpsrv/subnet.h>
 #include <dhcpsrv/subnet.h>
 #include <dhcpsrv/cfg_option_def.h>
 #include <dhcpsrv/cfg_option_def.h>
+#include <dhcpsrv/cfg_mac_source.h>
+#include <dhcpsrv/srv_config.h>
 #include <dhcpsrv/parsers/dhcp_config_parser.h>
 #include <dhcpsrv/parsers/dhcp_config_parser.h>
 #include <cc/simple_parser.h>
 #include <cc/simple_parser.h>
 #include <hooks/libinfo.h>
 #include <hooks/libinfo.h>
@@ -372,56 +374,33 @@ private:
 /// It contains a list of MAC/hardware acquisition source, i.e. methods how
 /// It contains a list of MAC/hardware acquisition source, i.e. methods how
 /// MAC address can possibly by obtained in DHCPv6. For a currently supported
 /// MAC address can possibly by obtained in DHCPv6. For a currently supported
 /// methods, see @ref isc::dhcp::Pkt::getMAC.
 /// methods, see @ref isc::dhcp::Pkt::getMAC.
-class MACSourcesListConfigParser : public DhcpConfigParser {
+class MACSourcesListConfigParser : public isc::data::SimpleParser {
 public:
 public:
-
-    /// @brief constructor
-    ///
-    /// As this is a dedicated parser, it must be used to parse
-    /// "mac-sources" parameter only. All other types will throw exception.
-    ///
-    /// @param param_name name of the configuration parameter being parsed
-    /// @param global_context Global parser context.
-    /// @throw BadValue if supplied parameter name is not "mac-sources"
-    MACSourcesListConfigParser(const std::string& param_name,
-                               ParserContextPtr global_context);
-
     /// @brief parses parameters value
     /// @brief parses parameters value
     ///
     ///
     /// Parses configuration entry (list of sources) and adds each element
     /// Parses configuration entry (list of sources) and adds each element
     /// to the sources list.
     /// to the sources list.
     ///
     ///
     /// @param value pointer to the content of parsed values
     /// @param value pointer to the content of parsed values
-    virtual void build(isc::data::ConstElementPtr value);
-
-    /// @brief Does nothing.
-    virtual void commit();
-
-private:
-
-    // Parsed parameter name
-    std::string param_name_;
-
-    /// Global parser context.
-    ParserContextPtr global_context_;
+    void parse(CfgMACSource& mac_sources, isc::data::ConstElementPtr value);
 };
 };
 
 
 /// @brief Parser for the control-socket structure
 /// @brief Parser for the control-socket structure
 ///
 ///
 /// It does not parse anything, simply stores the element in
 /// It does not parse anything, simply stores the element in
 /// the staging config.
 /// the staging config.
-class ControlSocketParser : public DhcpConfigParser {
+class ControlSocketParser : public isc::data::SimpleParser {
 public:
 public:
-
-    ControlSocketParser(const std::string& param_name);
-
-    /// @brief Stores contents of the control-socket map in the staging config.
+    /// @brief "Parses" control-socket structure
     ///
     ///
+    /// Since the SrvConfig structure takes the socket definition
+    /// as ConstElementPtr, there's really nothing to parse here.
+    /// It only does basic sanity checks and throws DhcpConfigError
+    /// if the value is null or is not a map.
+    ///
+    /// @param srv_cfg parsed values will be stored here
     /// @param value pointer to the content of parsed values
     /// @param value pointer to the content of parsed values
-    virtual void build(isc::data::ConstElementPtr value);
-
-    /// @brief Does nothing.
-    virtual void commit();
+    void parse(SrvConfig& srv_cfg, isc::data::ConstElementPtr value);
 };
 };
 
 
 /// @brief Parser for hooks library list
 /// @brief Parser for hooks library list
@@ -849,48 +828,24 @@ protected:
 /// is expected that the number of parameters will increase over time.
 /// is expected that the number of parameters will increase over time.
 ///
 ///
 /// It is useful for parsing Dhcp<4/6>/subnet<4/6>[x]/relay parameters.
 /// It is useful for parsing Dhcp<4/6>/subnet<4/6>[x]/relay parameters.
-class RelayInfoParser : public DhcpConfigParser {
+class RelayInfoParser : public isc::data::SimpleParser {
 public:
 public:
 
 
     /// @brief constructor
     /// @brief constructor
-    /// @param unused first argument is ignored, all Parser constructors
-    /// accept string as first argument.
-    /// @param relay_info is the storage in which to store the parsed
     /// @param family specifies protocol family (IPv4 or IPv6)
     /// @param family specifies protocol family (IPv4 or IPv6)
-    RelayInfoParser(const std::string& unused,
-                    const isc::dhcp::Subnet::RelayInfoPtr& relay_info,
-                    const isc::dhcp::Option::Universe& family);
+    RelayInfoParser(const isc::dhcp::Option::Universe& family);
 
 
     /// @brief parses the actual relay parameters
     /// @brief parses the actual relay parameters
-    /// @param relay_info JSON structure holding relay parameters to parse
-    virtual void build(isc::data::ConstElementPtr relay_info);
-
-    /// @brief stores parsed info in relay_info
-    virtual void commit();
-
-protected:
-
-    /// @brief Creates a parser for the given "relay" member element id.
     ///
     ///
     /// The elements currently supported are:
     /// The elements currently supported are:
     /// -# ip-address
     /// -# ip-address
     ///
     ///
-    /// @param parser is the "item_name" for a specific member element of
-    /// the "relay" specification.
-    ///
-    /// @return returns a pointer to newly created parser.
-    isc::dhcp::ParserPtr
-    createConfigParser(const std::string& parser);
-
-    /// Parsed data will be stored there on commit()
-    isc::dhcp::Subnet::RelayInfoPtr storage_;
-
-    /// Local storage information (for temporary values)
-    isc::dhcp::Subnet::RelayInfo local_;
-
-    /// Storage for subnet-specific string values.
-    StringStoragePtr string_values_;
+    /// @param cfg configuration will be stored here
+    /// @param relay_info JSON structure holding relay parameters to parse
+    void parse(const isc::dhcp::Subnet::RelayInfoPtr& cfg,
+               isc::data::ConstElementPtr relay_info);
 
 
+protected:
     /// Protocol family (IPv4 or IPv6)
     /// Protocol family (IPv4 or IPv6)
     Option::Universe family_;
     Option::Universe family_;
 };
 };

+ 127 - 52
src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -226,48 +226,95 @@ TEST_F(DhcpParserTest, uint32ParserTest) {
     EXPECT_EQ(test_value, actual_value);
     EXPECT_EQ(test_value, actual_value);
 }
 }
 
 
-/// @brief Check MACSourcesListConfigParser  basic functionality
+/// Verifies the code that parses mac sources and adds them to CfgMgr
+TEST_F(DhcpParserTest, MacSources) {
+
+    // That's an equivalent of the following snippet:
+    // "mac-sources: [ \"duid\", \"ipv6\" ]";
+    ElementPtr values = Element::createList();
+    values->add(Element::create("duid"));
+    values->add(Element::create("ipv6-link-local"));
+
+    // Let's grab server configuration from CfgMgr
+    SrvConfigPtr cfg = CfgMgr::instance().getStagingCfg();
+    ASSERT_TRUE(cfg);
+    CfgMACSource& sources = cfg->getMACSources();
+
+    // This should parse the configuration and check that it doesn't throw.
+    MACSourcesListConfigParser parser;
+    EXPECT_NO_THROW(parser.parse(sources, values));
+
+    // Finally, check the sources that were configured
+    CfgMACSources configured_sources =  cfg->getMACSources().get();
+    ASSERT_EQ(2, configured_sources.size());
+    EXPECT_EQ(HWAddr::HWADDR_SOURCE_DUID, configured_sources[0]);
+    EXPECT_EQ(HWAddr::HWADDR_SOURCE_IPV6_LINK_LOCAL, configured_sources[1]);
+}
+
+/// @brief Check MACSourcesListConfigParser rejecting empty list
 ///
 ///
-/// Verifies that the parser:
-/// 1. Does not allow empty for storage.
-/// 2. Does not allow name other than "mac-sources"
-/// 3. Parses list of mac sources and adds them to CfgMgr
-TEST_F(DhcpParserTest, MacSourcesListConfigParserTest) {
+/// Verifies that the code rejects an empty mac-sources list.
+TEST_F(DhcpParserTest, MacSourcesEmpty) {
 
 
-    const std::string valid_name = "mac-sources";
-    const std::string bogus_name = "bogus-name";
+    // That's an equivalent of the following snippet:
+    // "mac-sources: [ \"duid\", \"ipv6\" ]";
+    ElementPtr values = Element::createList();
+
+    // Let's grab server configuration from CfgMgr
+    SrvConfigPtr cfg = CfgMgr::instance().getStagingCfg();
+    ASSERT_TRUE(cfg);
+    CfgMACSource& sources = cfg->getMACSources();
 
 
-    ParserContextPtr parser_context(new ParserContext(Option::V6));
+    // This should throw, because if specified, at least one MAC source
+    // has to be specified.
+    MACSourcesListConfigParser parser;
+    EXPECT_THROW(parser.parse(sources, values), DhcpConfigError);
+}
 
 
-    // Verify that parser constructor fails if parameter name isn't "mac-sources"
-    EXPECT_THROW(MACSourcesListConfigParser(bogus_name, parser_context),
-                 isc::BadValue);
+/// @brief Check MACSourcesListConfigParser rejecting empty list
+///
+/// Verifies that the code rejects fake mac source.
+TEST_F(DhcpParserTest, MacSourcesBogus) {
 
 
     // That's an equivalent of the following snippet:
     // That's an equivalent of the following snippet:
     // "mac-sources: [ \"duid\", \"ipv6\" ]";
     // "mac-sources: [ \"duid\", \"ipv6\" ]";
-    ElementPtr config = Element::createList();
-    config->add(Element::create("duid"));
-    config->add(Element::create("ipv6-link-local"));
+    ElementPtr values = Element::createList();
+    values->add(Element::create("from-ebay"));
+    values->add(Element::create("just-guess-it"));
+
+    // Let's grab server configuration from CfgMgr
+    SrvConfigPtr cfg = CfgMgr::instance().getStagingCfg();
+    ASSERT_TRUE(cfg);
+    CfgMACSource& sources = cfg->getMACSources();
+
+    // This should throw, because these are not valid sources.
+    MACSourcesListConfigParser parser;
+    EXPECT_THROW(parser.parse(sources, values), DhcpConfigError);
+}
 
 
-    boost::scoped_ptr<MACSourcesListConfigParser>
-        parser(new MACSourcesListConfigParser(valid_name, parser_context));
+/// Verifies the code that properly catches duplicate entries
+/// in mac-sources definition.
+TEST_F(DhcpParserTest, MacSourcesDuplicate) {
 
 
-    // This should parse the configuration and add eth0 and eth1 to the list
-    // of interfaces that server should listen on.
-    EXPECT_NO_THROW(parser->build(config));
-    EXPECT_NO_THROW(parser->commit());
+    // That's an equivalent of the following snippet:
+    // "mac-sources: [ \"duid\", \"ipv6\" ]";
+    ElementPtr values = Element::createList();
+    values->add(Element::create("ipv6-link-local"));
+    values->add(Element::create("duid"));
+    values->add(Element::create("duid"));
+    values->add(Element::create("duid"));
 
 
-    // Use CfgMgr instance to check if eth0 and eth1 was added, and that
-    // eth2 was not added.
+    // Let's grab server configuration from CfgMgr
     SrvConfigPtr cfg = CfgMgr::instance().getStagingCfg();
     SrvConfigPtr cfg = CfgMgr::instance().getStagingCfg();
     ASSERT_TRUE(cfg);
     ASSERT_TRUE(cfg);
-    CfgMACSources configured_sources =  cfg->getMACSources().get();
+    CfgMACSource& sources = cfg->getMACSources();
 
 
-    ASSERT_EQ(2, configured_sources.size());
-    EXPECT_EQ(HWAddr::HWADDR_SOURCE_DUID, configured_sources[0]);
-    EXPECT_EQ(HWAddr::HWADDR_SOURCE_IPV6_LINK_LOCAL, configured_sources[1]);
+    // This should parse the configuration and check that it throws.
+    MACSourcesListConfigParser parser;
+    EXPECT_THROW(parser.parse(sources, values), DhcpConfigError);
 }
 }
 
 
+
 /// @brief Test Fixture class which provides basic structure for testing
 /// @brief Test Fixture class which provides basic structure for testing
 /// configuration parsing.  This is essentially the same structure provided
 /// configuration parsing.  This is essentially the same structure provided
 /// by dhcp servers.
 /// by dhcp servers.
@@ -2416,6 +2463,19 @@ TEST_F(ParseConfigTest, validRelayInfo4) {
         "    }";
         "    }";
     ElementPtr json = Element::fromJSON(config_str);
     ElementPtr json = Element::fromJSON(config_str);
 
 
+    // We need to set the default ip-address to something.
+    Subnet::RelayInfoPtr result(new Subnet::RelayInfo(asiolink::IOAddress("0.0.0.0")));
+
+    RelayInfoParser parser(Option::V4);
+
+    // Subnet4 parser will pass 0.0.0.0 to the RelayInfoParser
+    EXPECT_NO_THROW(parser.parse(result, json));
+    EXPECT_EQ("192.0.2.1", result->addr_.toText());
+}
+
+/// @brief Checks that a bogus relay info structure for IPv4 is rejected.
+TEST_F(ParseConfigTest, bogusRelayInfo4) {
+
     // Invalid config (wrong family type of the ip-address field)
     // Invalid config (wrong family type of the ip-address field)
     std::string config_str_bogus1 =
     std::string config_str_bogus1 =
         "    {"
         "    {"
@@ -2430,24 +2490,25 @@ TEST_F(ParseConfigTest, validRelayInfo4) {
         "    }";
         "    }";
     ElementPtr json_bogus2 = Element::fromJSON(config_str_bogus2);
     ElementPtr json_bogus2 = Element::fromJSON(config_str_bogus2);
 
 
-    // We need to set the default ip-address to something.
-    Subnet::RelayInfoPtr result(new Subnet::RelayInfo(asiolink::IOAddress("0.0.0.0")));
+    // Invalid config (ip-address is mandatory)
+    std::string config_str_bogus3 =
+        "    {"
+        "    }";
+    ElementPtr json_bogus3 = Element::fromJSON(config_str_bogus3);
 
 
-    boost::shared_ptr<RelayInfoParser> parser;
+    // We need to set the default ip-address to something.
+    Subnet::RelayInfoPtr result(new Subnet::RelayInfo(IOAddress::IPV4_ZERO_ADDRESS()));
 
 
-    // Subnet4 parser will pass 0.0.0.0 to the RelayInfoParser
-    EXPECT_NO_THROW(parser.reset(new RelayInfoParser("ignored", result,
-                                                     Option::V4)));
-    EXPECT_NO_THROW(parser->build(json));
-    EXPECT_NO_THROW(parser->commit());
+    RelayInfoParser parser(Option::V4);
 
 
-    EXPECT_EQ("192.0.2.1", result->addr_.toText());
+    // wrong family type
+    EXPECT_THROW(parser.parse(result, json_bogus1), DhcpConfigError);
 
 
-    // Let's check negative scenario (wrong family type)
-    EXPECT_THROW(parser->build(json_bogus1), DhcpConfigError);
+    // Too large byte values in pseudo-IPv4 addr
+    EXPECT_THROW(parser.parse(result, json_bogus2), DhcpConfigError);
 
 
-    // Let's check negative scenario (too large byte values in pseudo-IPv4 addr)
-    EXPECT_THROW(parser->build(json_bogus2), DhcpConfigError);
+    // Mandatory ip-address is missing. What a pity.
+    EXPECT_THROW(parser.parse(result, json_bogus2), DhcpConfigError);
 }
 }
 
 
 /// @brief Checks that a valid relay info structure for IPv6 can be handled
 /// @brief Checks that a valid relay info structure for IPv6 can be handled
@@ -2460,6 +2521,18 @@ TEST_F(ParseConfigTest, validRelayInfo6) {
         "    }";
         "    }";
     ElementPtr json = Element::fromJSON(config_str);
     ElementPtr json = Element::fromJSON(config_str);
 
 
+    // We need to set the default ip-address to something.
+    Subnet::RelayInfoPtr result(new Subnet::RelayInfo(asiolink::IOAddress("::")));
+
+    RelayInfoParser parser(Option::V6);
+    // Subnet4 parser will pass :: to the RelayInfoParser
+    EXPECT_NO_THROW(parser.parse(result, json));
+    EXPECT_EQ("2001:db8::1", result->addr_.toText());
+}
+
+/// @brief Checks that a valid relay info structure for IPv6 can be handled
+TEST_F(ParseConfigTest, bogusRelayInfo6) {
+
     // Invalid config (wrong family type of the ip-address field
     // Invalid config (wrong family type of the ip-address field
     std::string config_str_bogus1 =
     std::string config_str_bogus1 =
         "    {"
         "    {"
@@ -2474,23 +2547,25 @@ TEST_F(ParseConfigTest, validRelayInfo6) {
         "    }";
         "    }";
     ElementPtr json_bogus2 = Element::fromJSON(config_str_bogus2);
     ElementPtr json_bogus2 = Element::fromJSON(config_str_bogus2);
 
 
+    // Missing mandatory ip-address field.
+    std::string config_str_bogus3 =
+        "    {"
+        "    }";
+    ElementPtr json_bogus3 = Element::fromJSON(config_str_bogus3);
+
     // We need to set the default ip-address to something.
     // We need to set the default ip-address to something.
     Subnet::RelayInfoPtr result(new Subnet::RelayInfo(asiolink::IOAddress("::")));
     Subnet::RelayInfoPtr result(new Subnet::RelayInfo(asiolink::IOAddress("::")));
 
 
-    boost::shared_ptr<RelayInfoParser> parser;
-    // Subnet4 parser will pass :: to the RelayInfoParser
-    EXPECT_NO_THROW(parser.reset(new RelayInfoParser("ignored", result,
-                                                     Option::V6)));
-    EXPECT_NO_THROW(parser->build(json));
-    EXPECT_NO_THROW(parser->commit());
+    RelayInfoParser parser(Option::V6);
 
 
-    EXPECT_EQ("2001:db8::1", result->addr_.toText());
+    // Negative scenario (wrong family type)
+    EXPECT_THROW(parser.parse(result, json_bogus1), DhcpConfigError);
 
 
-    // Let's check negative scenario (wrong family type)
-    EXPECT_THROW(parser->build(json_bogus1), DhcpConfigError);
+    // Looks like IPv6 address, but has too many colons
+    EXPECT_THROW(parser.parse(result, json_bogus2), DhcpConfigError);
 
 
-    // Unparseable text that looks like IPv6 address, but has too many colons
-    EXPECT_THROW(parser->build(json_bogus2), DhcpConfigError);
+    // Mandatory ip-address is missing. What a pity.
+    EXPECT_THROW(parser.parse(result, json_bogus3), DhcpConfigError);
 }
 }
 
 
 // There's no test for ControlSocketParser, as it is tested in the DHCPv4 code
 // There's no test for ControlSocketParser, as it is tested in the DHCPv4 code

+ 76 - 6
src/lib/testutils/io_utils.cc

@@ -1,9 +1,10 @@
-// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 
+#include <exceptions/exceptions.h>
 #include <testutils/io_utils.h>
 #include <testutils/io_utils.h>
 #include <gtest/gtest.h>
 #include <gtest/gtest.h>
 #include <fstream>
 #include <fstream>
@@ -15,10 +16,8 @@ namespace dhcp {
 namespace test {
 namespace test {
 
 
 bool fileExists(const std::string& file_path) {
 bool fileExists(const std::string& file_path) {
-    std::ifstream fs(file_path.c_str());
-    const bool file_exists = fs.good();
-    fs.close();
-    return (file_exists);
+    struct stat statbuf;
+    return(stat(file_path.c_str(), &statbuf) == 0);
 }
 }
 
 
 std::string readFile(const std::string& file_path) {
 std::string readFile(const std::string& file_path) {
@@ -38,7 +37,78 @@ std::string readFile(const std::string& file_path) {
     return (output.str());
     return (output.str());
 }
 }
 
 
+std::string decommentJSONfile(const std::string& input_file) {
+
+    using namespace std;
+
+    ifstream f(input_file);
+    if (!f.is_open()) {
+        isc_throw(isc::BadValue, "can't open input file for reading: " + input_file);
+    }
+
+    string outfile;
+    size_t last_slash = input_file.find_last_of("/");
+    if (last_slash != string::npos) {
+        outfile = input_file.substr(last_slash + 1);
+    } else {
+        outfile = input_file;
+    }
+    outfile += "-decommented";
+
+    ofstream out(outfile);
+    if (!out.is_open()) {
+        isc_throw(isc::BadValue, "can't open output file for writing: " + input_file);
+    }
+
+    bool in_comment = false;
+    string line;
+    while (std::getline(f, line)) {
+        // First, let's get rid of the # comments
+        size_t hash_pos = line.find("#");
+        if (hash_pos != string::npos) {
+            line = line.substr(0, hash_pos);
+        }
+
+        // Second, let's get rid of the // comments
+        size_t dblslash_pos = line.find("//");
+        if (dblslash_pos != string::npos) {
+            line = line.substr(0, dblslash_pos);
+        }
+
+        // Now the tricky part: c comments.
+        size_t begin_pos = line.find("/*");
+        size_t end_pos = line.find("*/");
+        if (in_comment && end_pos == string::npos) {
+            // we continue through multiline comment
+            line = "";
+        } else {
+
+            if (begin_pos != string::npos) {
+                in_comment = true;
+                if (end_pos != string::npos) {
+                    // sigle line comment. Let's get rid of the content in between
+                    line = line.replace(begin_pos, end_pos + 2, end_pos + 2 - begin_pos, ' ');
+                    in_comment = false;
+                } else {
+                    line = line.substr(0, begin_pos);
+                }
+            } else {
+                if (in_comment && end_pos != string::npos) {
+                    line = line.replace(0, end_pos +2 , end_pos + 2, ' ');
+                    in_comment = false;
+                }
+            }
+        }
+
+        // Finally, write the line to the output file.
+        out << line << endl;
+    }
+    f.close();
+    out.close();
+
+    return (outfile);
+}
+
 }; // end of isc::dhcp::test namespace
 }; // end of isc::dhcp::test namespace
 }; // end of isc::dhcp namespace
 }; // end of isc::dhcp namespace
 }; // end of isc namespace
 }; // end of isc namespace
-

+ 16 - 1
src/lib/testutils/io_utils.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -8,6 +8,7 @@
 #define TEST_IO_UTILS_H
 #define TEST_IO_UTILS_H
 
 
 #include <string>
 #include <string>
+#include <sys/stat.h>
 
 
 namespace isc {
 namespace isc {
 namespace dhcp {
 namespace dhcp {
@@ -25,6 +26,20 @@ bool fileExists(const std::string& file_path);
 /// @return File contents.
 /// @return File contents.
 std::string readFile(const std::string& file_path);
 std::string readFile(const std::string& file_path);
 
 
+/// @brief Removes comments from a JSON file
+///
+/// Removes //, # and /* */ comments from the input file and writes its content
+/// to another file. The comments are replaced with spaces, so the original
+/// token locations should remain unaffected. This is rather naive
+/// implementation, but it's probably sufficient for testing. It won't be able
+/// to pick any trickier cases, like # or // appearing in strings, nested C++
+/// comments etc.
+///
+/// @param input_file file to be stripped of comments
+/// @return filename of a new file that has comments stripped from it
+/// @throw BadValue if the input file cannot be opened
+std::string decommentJSONfile(const std::string& input_file);
+
 }; // end of isc::dhcp::test namespace
 }; // end of isc::dhcp::test namespace
 }; // end of isc::dhcp namespace
 }; // end of isc::dhcp namespace
 }; // end of isc namespace
 }; // end of isc namespace