[3770_rebase] Implemented logic checking in DHCPv4.

src/bin/dhcp4/json_config_parser.cc

@@ -406,7 +406,8 @@ void configureCommandChannel() {
 }
 
 isc::data::ConstElementPtr
-configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
+configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set,
+                     bool check_only) {
     if (!config_set) {
         ConstElementPtr answer = isc::config::createAnswer(1,
                                  string("Can't parse NULL config"));
@@ -583,9 +584,6 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
                       << " (" << config_pair.second->getPosition() << ")");
         }
 
-        // Setup the command channel.
-        configureCommandChannel();
-
         // Apply global options in the staging config.
         Dhcp4ConfigParser global_parser;
         global_parser.parse(srv_cfg, mutable_cfg);
@@ -608,12 +606,25 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
         rollback = true;
     }
 
+    if (check_only) {
+        rollback = true;
+        if (!answer) {
+            answer = isc::config::createAnswer(0,
+            "Configuration seems sane. Control-socket, hook-libraries, and D2 "
+            "configuration were sanity checked, but not applied.");
+        }
+    }
+
     // So far so good, there was no parsing error so let's commit the
     // configuration. This will add created subnets and option values into
     // the server's configuration.
     // This operation should be exception safe but let's make sure.
     if (!rollback) {
         try {
+
+            // Setup the command channel.
+            configureCommandChannel();
+
             // No need to commit interface names as this is handled by the
             // CfgMgr::commit() function.
 

src/bin/dhcp4/json_config_parser.h

@@ -40,6 +40,10 @@ class Dhcpv4Srv;
 /// extra parameter is a reference to DHCPv4 server component. It is currently
 /// not used and CfgMgr::instance() is accessed instead.
 ///
+/// Test-only mode added. If check_only flag is set to true, the configuration
+/// is parsed, but the actual change is not applied. The goal is to have
+/// the ability to test configuration.
+///
 /// This method does not throw. It catches all exceptions and returns them as
 /// reconfiguration statuses. It may return the following response codes:
 /// 0 - configuration successful
@@ -48,10 +52,12 @@ class Dhcpv4Srv;
 /// values in to server's configuration)
 ///
 /// @param config_set a new configuration (JSON) for DHCPv4 server
+/// @param check_only whether this configuration is for testing only
 /// @return answer that contains result of reconfiguration
 isc::data::ConstElementPtr
 configureDhcp4Server(Dhcpv4Srv&,
-                     isc::data::ConstElementPtr config_set);
+                     isc::data::ConstElementPtr config_set,
+                     bool check_only = false);
 
 }; // end of isc::dhcp namespace
 }; // end of isc namespace

src/bin/dhcp4/main.cc

@@ -9,6 +9,8 @@
 #include <dhcp4/ctrl_dhcp4_srv.h>
 #include <dhcp4/dhcp4_log.h>
 #include <dhcp4/parser_context.h>
+#include <dhcp4/json_config_parser.h>
+#include <cc/command_interpreter.h>
 #include <dhcpsrv/cfgmgr.h>
 #include <log/logger_support.h>
 #include <log/logger_manager.h>
@@ -124,6 +126,14 @@ main(int argc, char* argv[]) {
 
     if (check_mode) {
         try {
+
+            // We need to initialize logging, in case any error messages are to be printed.
+            // This is just a test, so we don't care about lockfile.
+            setenv("KEA_LOCKFILE_DIR", "none", 0);
+            CfgMgr::instance().setDefaultLoggerName(DHCP4_ROOT_LOGGER_NAME);
+            Daemon::loggerInit(DHCP4_ROOT_LOGGER_NAME, verbose_mode);
+
+            // Check the syntax first.
             Parser4Context parser;
             ConstElementPtr json;
             json = parser.parseFile(config_file, Parser4Context::PARSER_DHCP4);
@@ -134,9 +144,27 @@ main(int argc, char* argv[]) {
             if (verbose_mode) {
                 cerr << "Syntax check OK" << endl;
             }
-            return (EXIT_SUCCESS);
+
+            // Check the logic next.
+            ConstElementPtr dhcp4 = json->get("Dhcp4");
+            if (!dhcp4) {
+                cerr << "Missing mandatory Dhcp4 element" << endl;
+                return (EXIT_FAILURE);
+            }
+            ControlledDhcpv4Srv server(0);
+            ConstElementPtr answer;
+            answer = configureDhcp4Server(server, dhcp4, true);
+
+            int status_code = 0;
+            answer = isc::config::parseAnswer(status_code, answer);
+            if (status_code == 0) {
+                return (EXIT_SUCCESS);
+            } else {
+                cerr << "Error encountered: " << answer->stringValue() << endl;
+                return (EXIT_FAILURE);
+            }
         } catch (const std::exception& ex) {
-            cerr << "Syntax check failed with " << ex.what() << endl;
+            cerr << "Syntax check failed with: " << ex.what() << endl;
         }
         return (EXIT_FAILURE);
     }

src/bin/dhcp4/tests/dhcp4_process_tests.sh.in

@@ -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
 # License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -58,6 +58,9 @@ CONFIG="{
     }
 }"
 # Invalid configuration (syntax error) to check that Kea can check syntax.
+# This config has following errors:
+# - it should be interfaces-config/interfaces, not interfaces
+# - it should be subnet4/pools, no subnet4/pool
 CONFIG_BAD_SYNTAX="{
     \"Dhcp4\":
     {
@@ -92,6 +95,46 @@ CONFIG_BAD_SYNTAX="{
         ]
     }
 }"
+
+# This config has bad pool values. The pool it out of scope for the subnet
+# it is defined in. Syntactically the config is correct, though.
+CONFIG_BAD_VALUES="{
+    \"Dhcp4\":
+    {
+        \"interfaces-config\": {
+            \"interfaces\": [ ]
+        },
+        \"valid-lifetime\": 4000,
+        \"renew-timer\": 1000,
+        \"rebind-timer\": 2000,
+        \"lease-database\":
+        {
+            \"type\": \"memfile\",
+            \"persist\": false
+        },
+        \"subnet4\": [
+        {
+            \"subnet\": \"10.0.0.0/8\",
+            \"pools\": [ { \"pool\": \"192.168.0.10-192.168.0.100\" } ]
+        } ]
+    },
+
+    \"Logging\":
+    {
+        \"loggers\": [
+        {
+            \"name\": \"kea-dhcp4\",
+            \"output_options\": [
+                {
+                    \"output\": \"$LOG_FILE\"
+                }
+            ],
+            \"severity\": \"INFO\"
+        }
+        ]
+    }
+}"
+
 # Invalid configuration (negative valid-lifetime) to check that Kea
 # gracefully handles reconfiguration errors.
 CONFIG_INVALID="{
@@ -138,10 +181,18 @@ bin_path=@abs_top_builddir@/src/bin/dhcp4
 # Import common test library.
 . @abs_top_builddir@/src/lib/testutils/dhcp_test_lib.sh
 
-# This test verifies that syntax check works properly.
+# This test verifies that syntax checking works properly. This function
+# requires 3 parameters:
+# testname
+# config - string with a content of the config (will be written to a file)
+# exp_code - expected exit code returned by kea (0 - success, 1 - failure)
 syntax_check_test() {
+    local TESTNAME="${1}"
+    local CONFIG="${2}"
+    local EXP_CODE="${3}"
+
     # Log the start of the test and print test name.
-    test_start "dhcpv4_srv.syntax_check"
+    test_start $TESTNAME
     # Remove dangling Kea instances and remove log files.
     cleanup
     # Create correct configuration file.
@@ -150,22 +201,11 @@ syntax_check_test() {
     printf "Running command %s.\n" "\"${bin_path}/${bin} -t -c ${CFG_FILE}\""
     ${bin_path}/${bin} -t -c ${CFG_FILE}
     exit_code=$?
-    if [ ${exit_code} -ne 0 ]; then
-        printf "ERROR: expected exit code 0, got ${exit_code}\n"
-        clean_exit 1
-    fi
-    # Create incorrect configuration file.
-    create_config "${CONFIG_BAD_SYNTAX}"
-    # Check it
-    printf "Running command %s.\n" "\"${bin_path}/${bin} -t -c ${CFG_FILE}\""
-    printf "A syntax error should be detected\n"
-    ${bin_path}/${bin} -t -c ${CFG_FILE}
-    if [ $? -eq 0 ]; then
-        printf "ERROR: expected exit code not 0, got 0\n"
+    if [ ${exit_code} -ne $EXP_CODE ]; then
+        printf "ERROR: expected exit code ${EXP_CODE}, got ${exit_code}\n"
         clean_exit 1
     fi
-    # All ok
-    clean_exit 0
+    test_finish 0
 }
 
 # This test verifies that DHCPv4 can be reconfigured with a SIGHUP signal.
@@ -445,4 +485,6 @@ shutdown_test "dhcpv4.sigint_test" 2
 version_test "dhcpv4.version"
 logger_vars_test "dhcpv4.variables"
 lfc_timer_test
-syntax_check_test
+syntax_check_test "dhcpv4.syntax_check_success" "${CONFIG}" 0
+syntax_check_test "dhcpv4.syntax_check_bad_syntax" "${CONFIG_BAD_SYNTAX}" 1
+syntax_check_test "dhcpv4.syntax_check_bad_values" "${CONFIG_BAD_VALUES}" 1