[master] Merge branch 'trac3259'

doc/Makefile.am

@@ -8,20 +8,22 @@ EXTRA_DIST += devel/qa.dox
 
 EXTRA_DIST += images/isc-logo.png
 
-nobase_dist_doc_DATA  = examples/kea4/single-subnet.json
-nobase_dist_doc_DATA += examples/kea4/several-subnets.json
+nobase_dist_doc_DATA  = examples/ddns/sample1.json
+nobase_dist_doc_DATA += examples/ddns/template.json
+nobase_dist_doc_DATA += examples/kea4/hooks.json
+nobase_dist_doc_DATA += examples/kea4/leases-expiration.json
 nobase_dist_doc_DATA += examples/kea4/multiple-options.json
 nobase_dist_doc_DATA += examples/kea4/reservations.json
-nobase_dist_doc_DATA += examples/kea4/leases-expiration.json
-nobase_dist_doc_DATA += examples/kea6/simple.json
-nobase_dist_doc_DATA += examples/kea6/several-subnets.json
-nobase_dist_doc_DATA += examples/kea6/multiple-options.json
+nobase_dist_doc_DATA += examples/kea4/several-subnets.json
+nobase_dist_doc_DATA += examples/kea4/single-subnet.json
 nobase_dist_doc_DATA += examples/kea6/advanced.json
-nobase_dist_doc_DATA += examples/kea6/stateless.json
-nobase_dist_doc_DATA += examples/kea6/reservations.json
+nobase_dist_doc_DATA += examples/kea6/hooks.json
 nobase_dist_doc_DATA += examples/kea6/leases-expiration.json
-nobase_dist_doc_DATA += examples/ddns/sample1.json
-nobase_dist_doc_DATA += examples/ddns/template.json
+nobase_dist_doc_DATA += examples/kea6/multiple-options.json
+nobase_dist_doc_DATA += examples/kea6/reservations.json
+nobase_dist_doc_DATA += examples/kea6/several-subnets.json
+nobase_dist_doc_DATA += examples/kea6/simple.json
+nobase_dist_doc_DATA += examples/kea6/stateless.json
 
 devel:
 	mkdir -p html

doc/examples/kea4/hooks.json

+# This is an example configuration file for the DHCPv4 server in Kea
+# illustrating the configuration of hooks libraries.  It uses a basic scenario
+# of one IPv4 subnet configured with the default values for all parameters.
+
+{"Dhcp4":
+
+{
+# Kea is told to listen on the ethX interface only.
+  "interfaces-config": {
+    "interfaces": [ "ethX" ]
+  },
+
+# Set up the storage for leases.
+  "lease-database": {
+    "type": "memfile"
+  },
+
+# Define a single subnet.
+  "subnet4": [
+    {
+      "pools": [ { "pool": "192.0.2.1 - 19.2.0.2.200" } ],
+      "subnet": "192.0.2.0/24",
+      "interface": "ethX"
+    }
+  ],
+
+# Set up the hooks libraries.  For this example, we assume that two libraries
+# are loaded, called "security" and "charging".  Note that order is important:
+# "security" is specified first so if both libraries supply a hook function
+# for a given hook, the function in "security" will be called before that in
+# "charging".
+
+  "hooks-libraries": [
+     {
+        "library": "/opt/lib/security.so"
+     },
+     {
+        "library": "/opt/lib/charging.so"
+     }
+  ]
+}
+
+}

doc/examples/kea6/hooks.json

+# This is an example configuration file for the DHCPv6 server in Kea
+# illustrating the configuration of hooks libraries.  It uses a basic scenario
+# of one IPv6 subnet configured with the default values for all parameters.
+
+{"Dhcp6":
+
+{
+# Kea is told to listen on the ethX interface only.
+  "interfaces-config": {
+    "interfaces": [ "ethX" ]
+  },
+
+# Set up the storage for leases.
+  "lease-database": {
+    "type": "memfile"
+  },
+
+# Define a single subnet.
+  "subnet6": [
+    {
+      "pools": [ { "pool": "2001:db8:1::/80" } ],
+      "subnet": "2001:db8:1::/64",
+      "interface": "ethX"
+    }
+  ],
+
+# Set up the hooks libraries.  For this example, we assume that two libraries
+# are loaded, called "security" and "charging".  Note that order is important:
+# "security" is specified first so if both libraries supply a hook function
+# for a given hook, the function in "security" will be called before that in
+# "charging".
+
+  "hooks-libraries": [
+     {
+        "library": "/opt/lib/security.so"
+     },
+     {
+        "library": "/opt/lib/charging.so"
+     }
+  ]
+}
+
+}

doc/guide/hooks.xml

@@ -46,25 +46,30 @@
       <command>hooks-libraries</command> keyword in the
       configuration for that process. (Note that
       the word "hooks" is plural).  The value of the keyword
-      is an array of strings, each string corresponding to a hooks library.
-      For example, to set up two hooks libraries for the DHCPv4 server, the
-      configuration would be:
+      is an array of map structures, each structure corresponding to a hooks
+      library.  For example, to set up two hooks libraries for the DHCPv4
+      server, the configuration would be:
 <screen>
 <userinput>"Dhcp4": {
     :
     "hooks-libraries": [
-       "/opt/charging.so",
-       "/opt/local/notification.so"
+        {
+            "library": "/opt/charging.so"
+        },
+        {
+            "library": "/opt/local/notification.so"
+        }
     ]
     :
 }</userinput>
 </screen>
       </para>
       <note><para>
-      At present, the libraries are specified as a simple list.  A future
-      version of Kea will support the capability of specifying a set of
-      parameters for each library. When that is added, it is likely
-      that the syntax for specifying hooks libraries will change.
+        This is a change to the syntax used in Kea 0.9.2 and earlier, where
+        hooks-libraries was a list of strings, each string being the name of
+        a library.  The change has been made in Kea 1.0 to facilitate the
+        specification of library-specific parameters, a feature that will be
+        added to a future version of Kea.
       </para></note>
       <para>
       Notes:
@@ -79,7 +84,18 @@
           <listitem><para>
           An empty list has the same effect as omitting the
           <command>hooks-libraries</command> configuration element all together.
-          </para></listitem>
+          </para>
+          <note><para>
+          There is one case where this is not true: if Kea
+          is running with a configuration that contains a
+          <command>hooks-libraries</command> item, and that item is
+          removed and the configuration reloaded, the removal will be
+          ignored and the libraries remain loaded.  As a workaround,
+          instead of removing the <command>hooks-libraries</command>
+          item, change it to an empty list.  This will be fixed in a
+          future version of Kea.
+          </para></note>
+          </listitem>
         </itemizedlist>
       </para>
       <para>

src/bin/dhcp4/dhcp4.spec

@@ -10,10 +10,18 @@
         "item_default": [],
         "list_item_spec":
         {
-          "item_name": "hooks-library",
-          "item_type": "string",
+          "item_name": "hooks-library-spec",
+          "item_type": "map",
           "item_optional": false,
-          "item_default": ""
+          "item_default": {},
+          "map_item_spec": [
+             {
+                "item_name": "library",
+                "item_type": "string",
+                "item_optional": false,
+                "item_default": ""
+             }
+          ]
         }
       },
 

src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -2772,7 +2772,6 @@ TEST_F(Dhcp4ParserTest, vendorOptionsCsv) {
 // of hooks libraries.
 std::string
 buildHooksLibrariesConfig(const std::vector<std::string>& libraries) {
-    const string quote("\"");
 
     // Create the first part of the configuration string.
     string config =
@@ -2786,7 +2785,7 @@ buildHooksLibrariesConfig(const std::vector<std::string>& libraries) {
         if (i > 0) {
             config += string(", ");
         }
-        config += (quote + libraries[i] + quote);
+        config += (string("{ \"library\": \"") + libraries[i] + string("\" }"));
     }
 
     // Append the remainder of the configuration.

src/bin/dhcp6/dhcp6.spec

@@ -10,10 +10,18 @@
         "item_default": [],
         "list_item_spec":
         {
-          "item_name": "hooks-library",
-          "item_type": "string",
+          "item_name": "hooks-library-spec",
+          "item_type": "map",
           "item_optional": false,
-          "item_default": ""
+          "item_default": {},
+          "map_item_spec": [
+             {
+                "item_name": "library",
+                "item_type": "string",
+                "item_optional": false,
+                "item_default": ""
+             }
+          ]
         }
       },
 

src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -2912,6 +2912,9 @@ TEST_F(Dhcp6ParserTest, DISABLED_stdOptionDataEncapsulate) {
 // of hooks libraries.
 std::string
 buildHooksLibrariesConfig(const std::vector<std::string>& libraries) {
+    const string lbrace("{");
+    const string rbrace("}");
+    const string liblabel("\"library\": ");
     const string quote("\"");
 
     // Create the first part of the configuration string.
@@ -2924,7 +2927,7 @@ buildHooksLibrariesConfig(const std::vector<std::string>& libraries) {
         if (i > 0) {
             config += string(", ");
         }
-        config += (quote + libraries[i] + quote);
+        config += (lbrace + liblabel + quote + libraries[i] + quote + rbrace);
     }
 
     // Append the remainder of the configuration.

src/lib/dhcpsrv/parsers/dhcp_parsers.cc

@@ -248,17 +248,65 @@ HooksLibrariesParser::HooksLibrariesParser(const std::string& param_name)
     }
 }
 
+// Parse the configuration.  As Kea has not yet implemented parameters, the
+// parsing code only checks that:
+//
+// 1. Each element in the hooks-libraries list is a map
+// 2. The map contains an element "library" whose value is a string: all
+//    other elements in the map are ignored.
 void
 HooksLibrariesParser::build(ConstElementPtr value) {
     // Initialize.
     libraries_.clear();
     changed_ = false;
 
-    // Extract the list of libraries.
-    BOOST_FOREACH(ConstElementPtr iface, value->listValue()) {
-        string libname = iface->str();
-        boost::erase_all(libname, "\"");
-        libraries_.push_back(libname);
+    // This is the new syntax.  Iterate through it and get each map.
+    BOOST_FOREACH(ConstElementPtr library_entry, value->listValue()) {
+        // Is it a map?
+        if (library_entry->getType() != Element::map) {
+            isc_throw(DhcpConfigError, "hooks library configuration error:"
+                " one or more entries in the hooks-libraries list is not"
+                " a map (" << library_entry->getPosition() << ")");
+        }
+
+        // Iterate iterate through each element in the map.  We check
+        // whether we have found a library element.
+        bool lib_found = false;
+        BOOST_FOREACH(ConfigPair entry_item, library_entry->mapValue()) {
+            if (entry_item.first == "library") {
+                if (entry_item.second->getType() != Element::string) {
+                    isc_throw(DhcpConfigError, "hooks library configuration"
+                        " error: value of 'library' element is not a string"
+                        " giving the path to a hooks library (" <<
+                        entry_item.second->getPosition() << ")");
+                }
+
+                // Get the name of the library and add it to the list after
+                // removing quotes.
+                string libname = (entry_item.second)->stringValue();
+
+                // Remove leading/trailing quotes and any leading/trailing
+                // spaces.
+                boost::erase_all(libname, "\"");
+                libname = isc::util::str::trim(libname);
+                if (libname.empty()) {
+                    isc_throw(DhcpConfigError, "hooks library configuration"
+                        " error: value of 'library' element must not be"
+                        " blank (" <<
+                        entry_item.second->getPosition() << ")");
+                }
+                libraries_.push_back(libname);
+
+                // Note we have found the library name.
+                lib_found = true;
+            }
+        }
+        if (! lib_found) {
+            isc_throw(DhcpConfigError, "hooks library configuration error:"
+                " one or more hooks-libraries elements are missing the"
+                " name of the library"  <<
+                " (" << library_entry->getPosition() << ")");
+        }
     }
 
     // Check if the list of libraries has changed.  If not, nothing is done

src/lib/dhcpsrv/parsers/dhcp_parsers.h

@@ -479,6 +479,29 @@ public:
     /// checks each of the libraries in the list for validity (they exist and
     /// have a "version" function that returns the correct value).
     ///
+    /// The syntax for specifying hooks libraries allow for library-specific
+    /// parameters to be specified along with the library, e.g.
+    ///
+    /// @code
+    ///      "hooks-libraries": [
+    ///          {
+    ///              "library": "hook-lib-1.so",
+    ///              "parameters": {
+    ///                  "alpha": "a string",
+    ///                  "beta": 42
+    ///              }
+    ///          },
+    ///          :
+    ///      ]
+    /// @endcode
+    ///
+    /// As Kea has not yet implemented parameters, the parsing code only checks
+    /// that:
+    ///
+    /// -# Each element in the hooks-libraries list is a map
+    /// -# The map contains an element "library" whose value is a string: all
+    ///    other elements in the map are ignored.
+    ///
     /// @param value pointer to the content of parsed values
     virtual void build(isc::data::ConstElementPtr value);
 

src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc

@@ -931,28 +931,48 @@ TEST_F(ParseConfigTest, emptyOptionData) {
     ASSERT_EQ(0, opt->getAddresses().size());
 }
 
-};  // Anonymous namespace
-
 /// The next set of tests check basic operation of the HooksLibrariesParser.
-
-
-// Utility function for setting up the "hooks-libraries" configuration.
 //
-// Returns a hooks-libraries configuration element that contains zero to
-// three libraries, depending on what arguments are supplied.
+// Convenience function to set a configuration of zero or more hooks
+// libraries:
+//
+// lib1 - No parameters
+// lib2 - Empty parameters statement
+// lib3 - Valid parameters
 std::string
 setHooksLibrariesConfig(const char* lib1 = NULL, const char* lib2 = NULL,
                         const char* lib3 = NULL) {
-    const std::string quote("\"");
-    const std::string comma_space(", ");
-
-    std::string config = std::string("{ \"hooks-libraries\": [");
+    const string lbrace("{");
+    const string rbrace("}");
+    const string quote("\"");
+    const string comma_space(", ");
+    const string library("\"library\": ");
+    const string parameters("\"parameters\": ");
+
+    string config = string("{ \"hooks-libraries\": [");
     if (lib1 != NULL) {
-        config += (quote + std::string(lib1) + quote);
+        // Library 1 has no parameters
+        config += lbrace;
+        config += library + quote + std::string(lib1) + quote;
+        config += rbrace;
+
         if (lib2 != NULL) {
-            config += (comma_space + quote + std::string(lib2) + quote);
+            // Library 2 has an empty parameters statement
+            config += comma_space + lbrace;
+            config += library + quote + std::string(lib2) + quote + comma_space;
+            config += string("\"parameters\": {}");
+            config += rbrace;
+
             if (lib3 != NULL) {
-                config += (comma_space + quote + std::string(lib3) + quote);
+                // Library 3 has valid parameters
+                config += comma_space + lbrace;
+                config += library + quote + std::string(lib3) + quote + comma_space;
+                config += string("\"parameters\": {");
+                config += string("    \"svalue\": \"string value\", ");
+                config += string("    \"ivalue\": 42, ");     // Integer value
+                config += string("    \"bvalue\": true");     // Boolean value
+                config += string("}");
+                config += rbrace;
             }
         }
     }
@@ -1242,6 +1262,78 @@ TEST_F(ParseConfigTest, reconfigureInvalidHooksLibraries) {
     EXPECT_EQ(CALLOUT_LIBRARY_1, hooks_libraries[0]);
 }
 
+// Check that if hooks-libraries contains invalid syntax, it is detected.
+TEST_F(ParseConfigTest, invalidSyntaxHooksLibraries) {
+
+    // Element holds a mixture of (valid) maps and non-maps.
+    string config1 = "{ \"hooks-libraries\": [ "
+        "{ \"library\": \"/opt/lib/lib1\" }, "
+        "\"/opt/lib/lib2\" "
+        "] }";
+    string error1 = "one or more entries in the hooks-libraries list is not"
+                    " a map";
+
+    int rcode = parseConfiguration(config1);
+    ASSERT_NE(0, rcode);
+    EXPECT_TRUE(error_text_.find(error1) != string::npos) <<
+        "Error text returned from parse failure is " << error_text_;
+
+    // Element holds valid maps, except one where the library element is not
+    // a string.
+    string config2 = "{ \"hooks-libraries\": [ "
+        "{ \"library\": \"/opt/lib/lib1\" }, "
+        "{ \"library\": 123 } "
+        "] }";
+    string error2 = "value of 'library' element is not a string giving"
+                    " the path to a hooks library";
+
+    rcode = parseConfiguration(config2);
+    ASSERT_NE(0, rcode);
+    EXPECT_TRUE(error_text_.find(error2) != string::npos) <<
+        "Error text returned from parse failure is " << error_text_;
+
+    // Element holds valid maps, except one where the library element is the
+    // empty string.
+    string config3 = "{ \"hooks-libraries\": [ "
+        "{ \"library\": \"/opt/lib/lib1\" }, "
+        "{ \"library\": \"\" } "
+        "] }";
+    string error3 = "value of 'library' element must not be blank";
+
+    rcode = parseConfiguration(config3);
+    ASSERT_NE(0, rcode);
+    EXPECT_TRUE(error_text_.find(error3) != string::npos) <<
+        "Error text returned from parse failure is " << error_text_;
+
+    // Element holds valid maps, except one where the library element is all
+    // spaces.
+    string config4 = "{ \"hooks-libraries\": [ "
+        "{ \"library\": \"/opt/lib/lib1\" }, "
+        "{ \"library\": \"      \" } "
+        "] }";
+    string error4 = "value of 'library' element must not be blank";
+
+    rcode = parseConfiguration(config4);
+    ASSERT_NE(0, rcode);
+    EXPECT_TRUE(error_text_.find(error3) != string::npos) <<
+        "Error text returned from parse failure is " << error_text_;
+
+    // Element holds valid maps, except one that does not contain a
+    // 'library' element.
+    string config5 = "{ \"hooks-libraries\": [ "
+        "{ \"library\": \"/opt/lib/lib1\" }, "
+        "{ \"parameters\": { \"alpha\": 123 } }, "
+        "{ \"library\": \"/opt/lib/lib2\" } "
+        "] }";
+    string error5 = "one or more hooks-libraries elements are missing the"
+                    " name of the library";
+
+    rcode = parseConfiguration(config5);
+    ASSERT_NE(0, rcode);
+    EXPECT_TRUE(error_text_.find(error5) != string::npos) <<
+        "Error text returned from parse failure is " << error_text_;
+}
+
 /// @brief Checks that a valid, enabled D2 client configuration works correctly.
 TEST_F(ParseConfigTest, validD2Config) {
 
@@ -2065,3 +2157,5 @@ TEST_F(ParseConfigTest, validRelayInfo6) {
 // There's no test for ControlSocketParser, as it is tested in the DHCPv4 code
 // (see CtrlDhcpv4SrvTest.commandSocketBasic in
 // src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc).
+
+};  // Anonymous namespace

src/lib/hooks/hooks_user.dox

@@ -792,19 +792,32 @@ DHCPv4 module, it must be listed in the "hooks-libraries" element of the
 @code
 "Dhcp4": {
        :
-    "hooks-libraries": [ "/usr/local/lib/example.so" ]
-       :
+    "hooks-libraries": [
+        {
+            "library": "/usr/local/lib/example.so"
+        }
+    ]
+        :
 }
 @endcode
 (Note that "hooks" is plural.)
 
+Each entry in the "hooks-libraries" list is a structure (a "map" in JSON
+parlance) that holds the following element:
+- library - the name of the library to load.  This must be a string.
+
+@note The syntax of the hooks-libraries configuration element has changed
+since kea 0.9.2 (in that version, "hooks-libraries" was just a list of
+libraries).  This change is in preparation for the introduction of
+library-specific parameters, which will be added to Kea in a version after 1.0.
+
 The DHCPv4 server will load the library and execute the callouts each time a
 request is received.
 
-@note The above assumes that the hooks library will be used with a version of
-Kea that is dynamically-linked.  For information regarding running
-hooks libraries against a statically-linked Kea, see
-@ref hooksdgStaticallyLinkedKea.
+@note All the above assumes that the hooks library will be used with a
+version of Kea that is dynamically-linked.  For information regarding
+running hooks libraries against a statically-linked Kea, see @ref
+hooksdgStaticallyLinkedKea.
 
 @section hooksdgAdvancedTopics Advanced Topics
 
@@ -1184,8 +1197,15 @@ as separate elements of the hooks-libraries configuration element, e.g.
 @code
 "Dhcp4": {
        :
-    "hooks-libraries": [ "/usr/lib/library1.so", "/opt/library2.so" ]
+    "hooks-libraries": [
+        {
+            "library": "/usr/lib/library1.so"
+        },
+        {
+            "library": "/opt/library2.so"
+        }
        :
+    ]
 }
 @endcode