Merge branch 'trac736'

src/bin/cfgmgr/plugins/Makefile.am

@@ -1,5 +1,6 @@
 SUBDIRS = tests
 EXTRA_DIST = README tsig_keys.py tsig_keys.spec
+EXTRA_DIST += logging.spec b10logging.py
 
 config_plugindir = @prefix@/share/@PACKAGE@/config_plugins
 config_plugin_DATA = tsig_keys.py tsig_keys.spec

src/bin/cfgmgr/plugins/b10logging.py

@@ -0,0 +1,96 @@
+# Copyright (C) 2011  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+# This is the configuration plugin for logging options
+# The name is 'b10logging' because logging.py is an existing module
+#
+# For a technical background, see
+# http://bind10.isc.org/wiki/LoggingCppApiDesign
+#
+
+from isc.config.module_spec import module_spec_from_file
+from isc.util.file import path_search
+from bind10_config import PLUGIN_PATHS
+spec = module_spec_from_file(path_search('logging.spec', PLUGIN_PATHS))
+
+ALLOWED_SEVERITIES = [ 'default',
+                       'debug',
+                       'info',
+                       'warn',
+                       'error',
+                       'fatal',
+                       'none' ]
+ALLOWED_DESTINATIONS = [ 'console',
+                         'file',
+                         'syslog' ]
+ALLOWED_STREAMS = [ 'stdout',
+                    'stderr' ]
+
+def check(config):
+    # Check the data layout first
+    errors=[]
+    if not spec.validate_config(False, config, errors):
+        return ' '.join(errors)
+    # The 'layout' is ok, now check for specific values
+    if 'loggers' in config:
+        for logger in config['loggers']:
+            # name should always be present
+            name = logger['name']
+
+            if 'severity' in logger and\
+               logger['severity'].lower() not in ALLOWED_SEVERITIES:
+                errors.append("bad severity value for logger " + name +
+                              ": " + logger['severity'])
+            if 'output_options' in logger:
+                for output_option in logger['output_options']:
+                    if 'destination' in output_option:
+                        destination = output_option['destination'].lower()
+                        if destination not in ALLOWED_DESTINATIONS:
+                            errors.append("bad destination for logger " +
+                            name + ": " + output_option['destination'])
+                        else:
+                            # if left to default, output is stdout, and
+                            # it will not show in the updated config,
+                            # so 1. we only need to check it if present,
+                            # and 2. if destination is changed, so should
+                            # output. So first check checks 'in', and the
+                            # others 'not in' for 'output'
+                            if destination == "console" and\
+                               'output' in output_option and\
+                               output_option['output'] not in ALLOWED_STREAMS:
+                                errors.append("bad output for logger " + name +
+                                              ": " + output_option['stream'] +
+                                              ", must be stdout or stderr")
+                            elif destination == "file" and\
+                                 'output' not in output_option or\
+                                 output_option['output'] == "":
+                                    errors.append("destination set to file but "
+                                                  "output not set to any "
+                                                  "filename for logger "
+                                                  + name)
+                            elif destination == "syslog" and\
+                                 'output' not in output_option or\
+                                 output_option['output'] == "":
+                                    errors.append("destination set to syslog but "
+                                                  "output not set to any facility"
+                                                  " for logger " + name)
+
+    if errors:
+        return ', '.join(errors)
+    return None
+
+def load():
+    return (spec, check)
+

src/bin/cfgmgr/plugins/logging.spec

@@ -0,0 +1,81 @@
+{
+    "module_spec": {
+        "module_name": "Logging",
+        "module_description": "Logging options",
+        "config_data": [
+            {
+                "item_name": "loggers",
+                "item_type": "list",
+                "item_optional": false,
+                "item_default": [],
+                "list_item_spec": {
+                  "item_name": "logger",
+                  "item_type": "map",
+                  "item_optional": false,
+                  "item_default": {},
+                  "map_item_spec": [
+                  {  "item_name": "name",
+                     "item_type": "string",
+                     "item_optional": false,
+                     "item_default": ""
+                  },
+                  {  "item_name": "severity",
+                     "item_type": "string",
+                     "item_optional": false,
+                     "item_default": "INFO"
+                  },
+                  {  "item_name": "debuglevel",
+                     "item_type": "integer",
+                     "item_optional": false,
+                     "item_default": 0
+                  },
+                  {  "item_name": "additive",
+                     "item_type": "boolean",
+                     "item_optional": false,
+                     "item_default": false
+                  },
+                  { "item_name": "output_options",
+                    "item_type": "list",
+                    "item_optional": false,
+                    "item_default": [],
+                    "list_item_spec": {
+                      "item_name": "output_option",
+                      "item_type": "map",
+                      "item_optional": false,
+                      "item_default": {},
+                      "map_item_spec": [
+                      { "item_name": "destination",
+                        "item_type": "string",
+                        "item_optional": false,
+                        "item_default": "console"
+                      },
+                      { "item_name": "output",
+                        "item_type": "string",
+                        "item_optional": false,
+                        "item_default": "stdout"
+                      },
+                      { "item_name": "flush",
+                        "item_type": "boolean",
+                        "item_optional": false,
+                        "item_default": false
+                      },
+                      { "item_name": "maxsize",
+                        "item_type": "integer",
+                        "item_optional": false,
+                        "item_default": 0
+                      },
+                      { "item_name": "maxver",
+                        "item_type": "integer",
+                        "item_optional": false,
+                        "item_default": 0
+                      }
+                      ]
+                    }
+                  }
+                  ]
+                }
+            }
+        ],
+        "commands": []
+    }
+}

src/bin/resolver/main.cc

@@ -208,7 +208,8 @@ main(int argc, char* argv[]) {
         cc_session = new Session(io_service.get_io_service());
         config_session = new ModuleCCSession(specfile, *cc_session,
                                              my_config_handler,
-                                             my_command_handler);
+                                             my_command_handler,
+                                             true, true);
         LOG_DEBUG(resolver_logger, RESOLVER_DBG_INIT, RESOLVER_CONFIGCHAN);
 
         // FIXME: This does not belong here, but inside Boss

src/lib/config/ccsession.cc

@@ -35,6 +35,10 @@
 #include <config/config_log.h>
 #include <config/ccsession.h>
 
+#include <log/logger_support.h>
+#include <log/logger_specification.h>
+#include <log/logger_manager.h>
+
 using namespace std;
 
 using isc::data::Element;
@@ -151,6 +155,115 @@ parseCommand(ConstElementPtr& arg, ConstElementPtr command) {
     }
 }
 
+namespace {
+// Temporary workaround functions for missing functionality in
+// getValue() (main problem described in ticket #993)
+// This returns either the value set for the given relative id,
+// or its default value
+// (intentially defined here so this interface does not get
+// included in ConfigData as it is)
+ConstElementPtr getValueOrDefault(ConstElementPtr config_part,
+                                  const std::string& relative_id,
+                                  const ConfigData& config_data,
+                                  const std::string& full_id) {
+    if (config_part->contains(relative_id)) {
+        return config_part->get(relative_id);
+    } else {
+        return config_data.getDefaultValue(full_id);
+    }
+}
+
+// Reads a output_option subelement of a logger configuration,
+// and sets the values thereing to the given OutputOption struct,
+// or defaults values if they are not provided (from config_data).
+void
+readOutputOptionConf(isc::log::OutputOption& output_option,
+                     ConstElementPtr output_option_el,
+                     const ConfigData& config_data)
+{
+    ConstElementPtr destination_el = getValueOrDefault(output_option_el,
+                                    "destination", config_data,
+                                    "loggers/output_options/destination");
+    output_option.destination = isc::log::getDestination(destination_el->stringValue());
+    ConstElementPtr output_el = getValueOrDefault(output_option_el,
+                                    "output", config_data,
+                                    "loggers/output_options/output");
+    if (output_option.destination == isc::log::OutputOption::DEST_CONSOLE) {
+        output_option.stream = isc::log::getStream(output_el->stringValue());
+    } else if (output_option.destination == isc::log::OutputOption::DEST_FILE) {
+        output_option.filename = output_el->stringValue();
+    } else if (output_option.destination == isc::log::OutputOption::DEST_SYSLOG) {
+        output_option.facility = output_el->stringValue();
+    }
+    output_option.flush = getValueOrDefault(output_option_el,
+                              "flush", config_data,
+                              "loggers/output_options/flush")->boolValue();
+    output_option.maxsize = getValueOrDefault(output_option_el,
+                                "maxsize", config_data,
+                                "loggers/output_options/maxsize")->intValue();
+    output_option.maxver = getValueOrDefault(output_option_el,
+                               "maxver", config_data,
+                               "loggers/output_options/maxver")->intValue();
+}
+
+// Reads a full 'loggers' configuration, and adds the loggers therein
+// to the given vector, fills in blanks with defaults from config_data
+void
+readLoggersConf(std::vector<isc::log::LoggerSpecification>& specs,
+                ConstElementPtr logger,
+                const ConfigData& config_data)
+{
+    const std::string lname = logger->get("name")->stringValue();
+    ConstElementPtr severity_el = getValueOrDefault(logger,
+                                      "severity", config_data,
+                                      "loggers/severity");
+    isc::log::Severity severity = isc::log::getSeverity(
+                                      severity_el->stringValue());
+    int dbg_level = getValueOrDefault(logger, "debuglevel",
+                                      config_data,
+                                      "loggers/debuglevel")->intValue();
+    bool additive = getValueOrDefault(logger, "additive", config_data,
+                                      "loggers/additive")->boolValue();
+
+    isc::log::LoggerSpecification logger_spec(
+        lname, severity, dbg_level, additive
+    );
+
+    if (logger->contains("output_options")) {
+        BOOST_FOREACH(ConstElementPtr output_option_el,
+                      logger->get("output_options")->listValue()) {
+            // create outputoptions
+            isc::log::OutputOption output_option;
+            readOutputOptionConf(output_option,
+                                 output_option_el,
+                                 config_data);
+            logger_spec.addOutputOption(output_option);
+        }
+    }
+
+    specs.push_back(logger_spec);
+}
+
+} // end anonymous namespace
+
+void
+my_logconfig_handler(const std::string&n, ConstElementPtr new_config, const ConfigData& config_data) {
+    config_data.getModuleSpec().validateConfig(new_config, true);
+
+    std::vector<isc::log::LoggerSpecification> specs;
+
+    if (new_config->contains("loggers")) {
+        BOOST_FOREACH(ConstElementPtr logger,
+                      new_config->get("loggers")->listValue()) {
+            readLoggersConf(specs, logger, config_data);
+        }
+    }
+
+    isc::log::LoggerManager logger_manager;
+    logger_manager.process(specs.begin(), specs.end());
+}
+
+
 ModuleSpec
 ModuleCCSession::readModuleSpecification(const std::string& filename) {
     std::ifstream file;
@@ -193,7 +306,8 @@ ModuleCCSession::ModuleCCSession(
         isc::data::ConstElementPtr new_config),
     isc::data::ConstElementPtr(*command_handler)(
         const std::string& command, isc::data::ConstElementPtr args),
-    bool start_immediately
+    bool start_immediately,
+    bool handle_logging
     ) :
     started_(false),
     session_(session)
@@ -207,10 +321,8 @@ ModuleCCSession::ModuleCCSession(
 
     session_.establish(NULL);
     session_.subscribe(module_name_, "*");
-    //session_.subscribe("Boss", "*");
-    //session_.subscribe("statistics", "*");
-    // send the data specification
 
+    // send the data specification
     ConstElementPtr spec_msg = createCommand("module_spec",
                                              module_specification_.getFullSpec());
     unsigned int seq = session_.group_sendmsg(spec_msg, "ConfigManager");
@@ -239,9 +351,15 @@ ModuleCCSession::ModuleCCSession(
         }
     }
 
+    // Keep track of logging settings automatically
+    if (handle_logging) {
+        addRemoteConfig("Logging", my_logconfig_handler, false);
+    }
+
     if (start_immediately) {
         start();
     }
+
 }
 
 void
@@ -361,6 +479,11 @@ ModuleCCSession::checkCommand() {
             }
         } catch (const CCSessionError& re) {
             LOG_ERROR(config_logger, CONFIG_CCSESSION_MSG).arg(re.what());
+        } catch (const std::exception& stde) {
+            // No matter what unexpected error happens, we do not want
+            // to crash because of an incoming event, so we log the
+            // exception and continue to run
+            LOG_ERROR(config_logger, CONFIG_CCSESSION_MSG_INTERNAL).arg(stde.what());
         }
         if (!isNull(answer)) {
             session_.reply(routing, answer);
@@ -382,9 +505,7 @@ ModuleCCSession::fetchRemoteSpec(const std::string& module, bool is_filename) {
         ConstElementPtr cmd(createCommand("get_module_spec",
                             Element::fromJSON("{\"module_name\": \"" + module +
                                               "\"}")));
-        const unsigned int seq = session_.group_sendmsg(cmd, "ConfigManager");
-
-        // Wait for the answer
+        unsigned int seq = session_.group_sendmsg(cmd, "ConfigManager");
         ConstElementPtr env, answer;
         session_.group_recvmsg(env, answer, false, seq);
         int rcode;
@@ -407,7 +528,8 @@ ModuleCCSession::fetchRemoteSpec(const std::string& module, bool is_filename) {
 std::string
 ModuleCCSession::addRemoteConfig(const std::string& spec_name,
                                  void (*handler)(const std::string& module,
-                                          ConstElementPtr),
+                                                 ConstElementPtr,
+                                                 const ConfigData&),
                                  bool spec_is_filename)
 {
     // First get the module name, specification and default config
@@ -439,7 +561,7 @@ ModuleCCSession::addRemoteConfig(const std::string& spec_name,
     remote_module_configs_[module_name] = rmod_config;
     if (handler) {
         remote_module_handlers_[module_name] = handler;
-        handler(module_name, local_config);
+        handler(module_name, local_config, rmod_config);
     }
 
     // Make sure we get updates in future
@@ -487,7 +609,7 @@ ModuleCCSession::updateRemoteConfig(const std::string& module_name,
         std::map<std::string, RemoteHandler>::iterator hit =
             remote_module_handlers_.find(module_name);
         if (hit != remote_module_handlers_.end()) {
-            hit->second(module_name, new_config);
+            hit->second(module_name, new_config, it->second);
         }
     }
 }

src/lib/config/ccsession.h

@@ -161,6 +161,7 @@ public:
      * configuration of the local module needs to be updated.
      * This must refer to a valid object of a concrete derived class of
      * AbstractSession without establishing the session.
+     *
      * Note: the design decision on who is responsible for establishing the
      * session is in flux, and may change in near future.
      *
@@ -171,11 +172,14 @@ public:
      * @param command_handler A callback function pointer to be called when
      * a control command from a remote agent needs to be performed on the
      * local module.
-     * @start_immediately If true (default), start listening to new commands
+     * @param start_immediately If true (default), start listening to new commands
      * and configuration changes asynchronously at the end of the constructor;
      * if false, it will be delayed until the start() method is explicitly
      * called. (This is a short term workaround for an initialization trouble.
      * We'll need to develop a cleaner solution, and then remove this knob)
+     * @param handle_logging If true, the ModuleCCSession will automatically
+     * take care of logging configuration through the virtual Logging config
+     * module.
      */
     ModuleCCSession(const std::string& spec_file_name,
                     isc::cc::AbstractSession& session,
@@ -184,7 +188,8 @@ public:
                     isc::data::ConstElementPtr(*command_handler)(
                         const std::string& command,
                         isc::data::ConstElementPtr args) = NULL,
-                    bool start_immediately = true
+                    bool start_immediately = true,
+                    bool handle_logging = false
                     );
 
     /// Start receiving new commands and configuration changes asynchronously.
@@ -283,7 +288,8 @@ public:
     std::string addRemoteConfig(const std::string& spec_name,
                                 void (*handler)(const std::string& module_name,
                                                 isc::data::ConstElementPtr
-                                                update) = NULL,
+                                                update,
+                                                const ConfigData& config_data) = NULL,
                                 bool spec_is_filename = true);
 
     /**
@@ -337,7 +343,8 @@ private:
         isc::data::ConstElementPtr args);
 
     typedef void (*RemoteHandler)(const std::string&,
-                                  isc::data::ConstElementPtr);
+                                  isc::data::ConstElementPtr,
+                                  const ConfigData&);
     std::map<std::string, ConfigData> remote_module_configs_;
     std::map<std::string, RemoteHandler> remote_module_handlers_;
 

src/lib/config/config_data.cc

@@ -21,6 +21,63 @@
 
 using namespace isc::data;
 
+namespace {
+
+// Returns the '_spec' part of a list or map specification (recursively,
+// i.e. if it is a list of lists or maps, will return the spec of the
+// inner-most list or map).
+//
+// \param spec_part the list or map specification (part)
+// \return the value of spec_part's "list_item_spec" or "map_item_spec",
+//         or the original spec_part, if it is not a MapElement or does
+//         not contain "list_item_spec" or "map_item_spec"
+ConstElementPtr findListOrMapSubSpec(ConstElementPtr spec_part) {
+    while (spec_part->getType() == Element::map &&
+           (spec_part->contains("list_item_spec") ||
+            spec_part->contains("map_item_spec"))) {
+        if (spec_part->contains("list_item_spec")) {
+            spec_part = spec_part->get("list_item_spec");
+        } else {
+            spec_part = spec_part->get("map_item_spec");
+        }
+    }
+    return spec_part;
+}
+
+// Returns a specific Element in a given specification ListElement
+//
+// \exception DataNotFoundError if the given identifier does not
+// point to an existing element. Since we are dealing with the
+// specification here, and not the config data itself, this should
+// not happen, and is a code bug.
+//
+// \param spec_part ListElement to find the element in
+// \param id_part the name of the element to find (must match the value
+//                "item_name" in the list item
+// \param id_full the full identifier id_part is a part of, this is
+//                used to better report any errors
+ConstElementPtr findItemInSpecList(ConstElementPtr spec_part,
+                                   const std::string& id_part,
+                                   const std::string& id_full)
+{
+    bool found = false;
+    BOOST_FOREACH(ConstElementPtr list_el, spec_part->listValue()) {
+        if (list_el->getType() == Element::map &&
+            list_el->contains("item_name") &&
+            list_el->get("item_name")->stringValue() == id_part) {
+            spec_part = list_el;
+            found = true;
+        }
+    }
+    if (!found) {
+        isc_throw(isc::config::DataNotFoundError,
+                  id_part + " in " + id_full + " not found");
+    }
+    return (spec_part);
+}
+
+} // anonymous namespace
+
 namespace isc {
 namespace config {
 
@@ -36,11 +93,10 @@ namespace config {
 // validated and conforms to the specification.
 static ConstElementPtr
 find_spec_part(ConstElementPtr spec, const std::string& identifier) {
-    //std::cout << "[XX] find_spec_part for " << identifier << std::endl;
     if (!spec) {
         isc_throw(DataNotFoundError, "Empty specification");
     }
-    //std::cout << "in: " << std::endl << spec << std::endl;
+
     ConstElementPtr spec_part = spec;
     if (identifier == "") {
         isc_throw(DataNotFoundError, "Empty identifier");
@@ -49,59 +105,44 @@ find_spec_part(ConstElementPtr spec, const std::string& identifier) {
     size_t sep = id.find('/');
     while(sep != std::string::npos) {
         std::string part = id.substr(0, sep);
-        //std::cout << "[XX] id part: " << part << std::endl;
+
         if (spec_part->getType() == Element::list) {
-            bool found = false;
-            BOOST_FOREACH(ConstElementPtr list_el, spec_part->listValue()) {
-                if (list_el->getType() == Element::map &&
-                    list_el->contains("item_name") &&
-                    list_el->get("item_name")->stringValue() == part) {
-                    spec_part = list_el;
-                    found = true;
-                }
-            }
-            if (!found) {
-                isc_throw(DataNotFoundError, identifier);
-            }
+            spec_part = findItemInSpecList(spec_part, part, identifier);
+        } else {
+            isc_throw(DataNotFoundError,
+                      "Not a list of spec items: " + spec_part->str());
         }
         id = id.substr(sep + 1);
         sep = id.find("/");
+
+        // As long as we are not in the 'final' element as specified
+        // by the identifier, we want to automatically traverse list
+        // and map specifications
+        if (id != "" && id != "/") {
+            spec_part = findListOrMapSubSpec(spec_part);
+        }
     }
     if (id != "" && id != "/") {
         if (spec_part->getType() == Element::list) {
-            bool found = false;
-            BOOST_FOREACH(ConstElementPtr list_el, spec_part->listValue()) {
-                if (list_el->getType() == Element::map &&
-                    list_el->contains("item_name") &&
-                    list_el->get("item_name")->stringValue() == id) {
-                    spec_part = list_el;
-                    found = true;
-                }
-            }
-            if (!found) {
-                isc_throw(DataNotFoundError, identifier);
-            }
+            spec_part = findItemInSpecList(spec_part, id, identifier);
         } else if (spec_part->getType() == Element::map) {
             if (spec_part->contains("map_item_spec")) {
-                bool found = false;
-                BOOST_FOREACH(ConstElementPtr list_el,
-                              spec_part->get("map_item_spec")->listValue()) {
-                    if (list_el->getType() == Element::map &&
-                        list_el->contains("item_name") &&
-                        list_el->get("item_name")->stringValue() == id) {
-                        spec_part = list_el;
-                        found = true;
-                    }
-                }
-                if (!found) {
-                    isc_throw(DataNotFoundError, identifier);
-                }
+                spec_part = findItemInSpecList(
+                                spec_part->get("map_item_spec"),
+                                id, identifier);
             } else {
-                isc_throw(DataNotFoundError, identifier);
+                // Either we already have the element we are looking
+                // for, or we are trying to reach something that does
+                // not exist (i.e. the code does not match the spec)
+                if (!spec_part->contains("item_name") ||
+                    spec_part->get("item_name")->stringValue() != id) {
+                    isc_throw(DataNotFoundError, "Element above " + id +
+                                                 " in " + identifier +
+                                                 " is not a map: " + spec_part->str());
+                }
             }
         }
     }
-    //std::cout << "[XX] found spec part: " << std::endl << spec_part << std::endl;
     return (spec_part);
 }
 
@@ -164,6 +205,17 @@ ConfigData::getValue(bool& is_default, const std::string& identifier) const {
     return (value);
 }
 
+ConstElementPtr
+ConfigData::getDefaultValue(const std::string& identifier) const {
+    ConstElementPtr spec_part =
+        find_spec_part(_module_spec.getConfigSpec(), identifier);
+    if (spec_part->contains("item_default")) {
+        return spec_part->get("item_default");
+    } else {
+        isc_throw(DataNotFoundError, "No default for " + identifier);
+    }
+}
+
 /// Returns an ElementPtr pointing to a ListElement containing
 /// StringElements with the names of the options at the given
 /// identifier. If recurse is true, maps will be expanded as well

src/lib/config/config_data.h

@@ -57,6 +57,16 @@ public:
     ///        value that is to be returned
     isc::data::ConstElementPtr getValue(const std::string& identifier) const;
 
+    /// Returns the default value for the given identifier.
+    ///
+    /// \exception DataNotFoundError if the given identifier does not
+    ///            exist, or if the given value has no specified default
+    ///
+    /// \param identifier The identifier pointing to the configuration
+    ///        value for which the default is to be returned
+    /// \return ElementPtr containing the default value
+    isc::data::ConstElementPtr getDefaultValue(const std::string& identifier) const;
+
     /// Returns the value currently set for the given identifier
     /// If no value is set, the default value (as specified by the
     /// .spec file) is returned. If there is no value and no default,

src/lib/config/configdef.mes

@@ -48,3 +48,10 @@ channel. The message does not appear to be a valid command, and is
 missing a required element or contains an unknown data format. This
 most likely means that another BIND10 module is sending a bad message.
 The message itself is ignored by this module.
+
+% CCSESSION_MSG_INTERNAL error handling CC session message: %1
+There was an internal problem handling an incoming message on the
+command and control channel. An unexpected exception was thrown. This
+most likely points to an internal inconsistency in the module code. The
+exception message is appended to the log error, and the module will
+continue to run, but will not send back an answer.

src/lib/config/tests/ccsession_unittests.cc

@@ -160,6 +160,10 @@ TEST_F(CCSessionTest, session1) {
     EXPECT_EQ("ConfigManager", group);
     EXPECT_EQ("*", to);
     EXPECT_EQ(0, session.getMsgQueue()->size());
+
+    // without explicit argument, the session should not automatically
+    // subscribe to logging config
+    EXPECT_FALSE(session.haveSubscription("Logging", "*"));
 }
 
 TEST_F(CCSessionTest, session2) {
@@ -351,7 +355,9 @@ int remote_item1(0);
 ConstElementPtr remote_config;
 ModuleCCSession *remote_mccs(NULL);
 
-void remoteHandler(const std::string& module_name, ConstElementPtr config) {
+void remoteHandler(const std::string& module_name,
+                   ConstElementPtr config,
+                   const ConfigData&) {
     remote_module_name = module_name;
     remote_item1 = remote_mccs->getRemoteConfigValue("Spec2", "item1")->
         intValue();
@@ -595,6 +601,27 @@ TEST_F(CCSessionTest, delayedStart) {
                  FakeSession::DoubleRead);
 }
 
+TEST_F(CCSessionTest, loggingStart) {
+    // provide the logging module spec
+    ConstElementPtr log_spec = moduleSpecFromFile(LOG_SPEC_FILE).getFullSpec();
+    session.getMessages()->add(createAnswer(0, log_spec));
+    // just give an empty config
+    session.getMessages()->add(createAnswer(0, el("{}")));
+    ModuleCCSession mccs(ccspecfile("spec2.spec"), session, NULL, NULL,
+                         true, true);
+    EXPECT_TRUE(session.haveSubscription("Logging", "*"));
+}
+
+TEST_F(CCSessionTest, loggingStartBadSpec) {
+    // provide the logging module spec
+    session.getMessages()->add(createAnswer(0, el("{}")));
+    // just give an empty config
+    session.getMessages()->add(createAnswer(0, el("{}")));
+    EXPECT_THROW(new ModuleCCSession(ccspecfile("spec2.spec"), session,
+                 NULL, NULL, true, true), ModuleSpecError);
+    EXPECT_FALSE(session.haveSubscription("Logging", "*"));
+}
+
 // Similar to the above, but more implicitly by calling addRemoteConfig().
 // We should construct ModuleCCSession with start_immediately being false
 // if we need to call addRemoteConfig().

src/lib/config/tests/config_data_unittests.cc

@@ -64,21 +64,35 @@ TEST(ConfigData, getValue) {
     EXPECT_EQ("{  }", cd.getValue(is_default, "value6/")->str());
     EXPECT_TRUE(is_default);
     EXPECT_EQ("[  ]", cd.getValue("value8")->str());
+    EXPECT_EQ("[  ]", cd.getDefaultValue("value8")->str());
+    EXPECT_EQ("empty", cd.getValue("value8/a")->stringValue());
 
     EXPECT_THROW(cd.getValue("")->str(), DataNotFoundError);
     EXPECT_THROW(cd.getValue("/")->str(), DataNotFoundError);
     EXPECT_THROW(cd.getValue("no_such_item")->str(), DataNotFoundError);
     EXPECT_THROW(cd.getValue("value6/a")->str(), DataNotFoundError);
     EXPECT_THROW(cd.getValue("value6/no_such_item")->str(), DataNotFoundError);
-    EXPECT_THROW(cd.getValue("value8/a")->str(), DataNotFoundError);
-    EXPECT_THROW(cd.getValue("value8/a")->str(), DataNotFoundError);
-    EXPECT_THROW(cd.getValue("value8/a")->str(), DataNotFoundError);
+    EXPECT_THROW(cd.getValue("value8/b")->str(), DataNotFoundError);
 
     ModuleSpec spec1 = moduleSpecFromFile(std::string(TEST_DATA_PATH) + "/spec1.spec");
     ConfigData cd1 = ConfigData(spec1);
     EXPECT_THROW(cd1.getValue("anything")->str(), DataNotFoundError);
 }
 
+TEST(ConfigData, getDefaultValue) {
+    ModuleSpec spec31 = moduleSpecFromFile(std::string(TEST_DATA_PATH) + "/spec31.spec");
+    ConfigData cd = ConfigData(spec31);
+    EXPECT_EQ("[  ]", cd.getDefaultValue("first_list_items")->str());
+    EXPECT_EQ("\"foo\"", cd.getDefaultValue("first_list_items/foo")->str());
+    EXPECT_EQ("{  }", cd.getDefaultValue("first_list_items/second_list_items/map_element")->str());
+    EXPECT_EQ("[  ]", cd.getDefaultValue("first_list_items/second_list_items/map_element/list1")->str());
+    EXPECT_EQ("1", cd.getDefaultValue("first_list_items/second_list_items/map_element/list1/number")->str());
+
+    EXPECT_THROW(cd.getDefaultValue("doesnotexist")->str(), DataNotFoundError);
+    EXPECT_THROW(cd.getDefaultValue("first_list_items/second_list_items/map_element/list1/doesnotexist")->str(), DataNotFoundError);
+}
+
+
 TEST(ConfigData, setLocalConfig) {
     ModuleSpec spec2 = moduleSpecFromFile(std::string(TEST_DATA_PATH) + "/spec2.spec");
     ConfigData cd = ConfigData(spec2);

src/lib/config/tests/data_def_unittests_config.h.in

@@ -13,3 +13,4 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #define TEST_DATA_PATH "@abs_srcdir@/testdata"
+#define LOG_SPEC_FILE "@abs_top_srcdir@/src/bin/cfgmgr/plugins/logging.spec"

src/lib/log/tests/Makefile.am

@@ -38,6 +38,7 @@ run_unittests_CXXFLAGS += -Wno-unused-variable
 endif
 run_unittests_LDADD  = $(GTEST_LDADD)
 run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la
+run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
 run_unittests_LDADD += $(top_builddir)/src/lib/util/libutil.la
 run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
 run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la

src/lib/python/isc/config/cfgmgr.py

@@ -214,7 +214,7 @@ class ConfigManager:
            is returned"""
         if module_name:
             if module_name in self.module_specs:
-                return self.module_specs[module_name]
+                return self.module_specs[module_name].get_full_spec()
             else:
                 # TODO: log error?
                 return {}

src/lib/python/isc/config/tests/cfgmgr_test.py

@@ -176,7 +176,7 @@ class TestConfigManager(unittest.TestCase):
         self.cm.set_module_spec(module_spec)
         self.assert_(module_spec.get_module_name() in self.cm.module_specs)
         module_spec2 = self.cm.get_module_spec(module_spec.get_module_name())
-        self.assertEqual(module_spec, module_spec2)
+        self.assertEqual(module_spec.get_full_spec(), module_spec2)
 
         self.assertEqual({}, self.cm.get_module_spec("nosuchmodule"))
 

src/lib/server_common/keyring.cc

@@ -27,7 +27,8 @@ KeyringPtr keyring;
 namespace {
 
 void
-updateKeyring(const std::string&, ConstElementPtr data) {
+updateKeyring(const std::string&, ConstElementPtr data,
+              const isc::config::ConfigData&) {
     ConstElementPtr list(data->get("keys"));
     KeyringPtr load(new TSIGKeyRing);