diff --git a/src/bin/xfrout/xfrout.py.in b/src/bin/xfrout/xfrout.py.in index 40bad8565d..ac22fe4db0 100755 --- a/src/bin/xfrout/xfrout.py.in +++ b/src/bin/xfrout/xfrout.py.in @@ -559,7 +559,7 @@ class XfroutServer: #self._log = None self._listen_sock_file = UNIX_SOCKET_FILE self._shutdown_event = threading.Event() - self._cc = isc.config.ModuleCCSession(SPECFILE_LOCATION, self.config_handler, self.command_handler) + self._cc = isc.config.ModuleCCSession(SPECFILE_LOCATION, self.config_handler, self.command_handler, None, True) self._config_data = self._cc.get_full_config() self._cc.start() self._cc.add_remote_config(AUTH_SPECFILE_LOCATION); diff --git a/src/lib/config/ccsession.cc b/src/lib/config/ccsession.cc index 857de635ad..dd2be3d2af 100644 --- a/src/lib/config/ccsession.cc +++ b/src/lib/config/ccsession.cc @@ -247,7 +247,9 @@ readLoggersConf(std::vector& specs, } // end anonymous namespace void -my_logconfig_handler(const std::string&n, ConstElementPtr new_config, const ConfigData& config_data) { +default_logconfig_handler(const std::string& module_name, + ConstElementPtr new_config, + const ConfigData& config_data) { config_data.getModuleSpec().validateConfig(new_config, true); std::vector specs; @@ -353,7 +355,7 @@ ModuleCCSession::ModuleCCSession( // Keep track of logging settings automatically if (handle_logging) { - addRemoteConfig("Logging", my_logconfig_handler, false); + addRemoteConfig("Logging", default_logconfig_handler, false); } if (start_immediately) { diff --git a/src/lib/config/ccsession.h b/src/lib/config/ccsession.h index 53aab781d2..0d4b7f3894 100644 --- a/src/lib/config/ccsession.h +++ b/src/lib/config/ccsession.h @@ -354,6 +354,25 @@ private: ModuleSpec fetchRemoteSpec(const std::string& module, bool is_filename); }; +/// \brief Default handler for logging config updates +/// +/// When CCSession is initialized with handle_logging set to true, +/// this callback will be used to update the logger when a configuration +/// change comes in. +/// +/// This function updates the (global) loggers by initializing a +/// LoggerManager and passing the settings as specified in the given +/// configuration update. +/// +/// \param module_name The name of the module +/// \param new_config The modified configuration values +/// \param config_data The full config data for the (remote) logging +/// module. +void +default_logconfig_handler(const std::string& module_name, + isc::data::ConstElementPtr new_config, + const ConfigData& config_data); + } } #endif // __CCSESSION_H diff --git a/src/lib/python/isc/config/ccsession.py b/src/lib/python/isc/config/ccsession.py index 84809f1b54..0869160bfc 100644 --- a/src/lib/python/isc/config/ccsession.py +++ b/src/lib/python/isc/config/ccsession.py @@ -39,6 +39,10 @@ from isc.cc import Session from isc.config.config_data import ConfigData, MultiConfigData, BIND10_CONFIG_DATA_VERSION import isc +from isc.util.file import path_search +import bind10_config +from isc.log import log_config_update +import json class ModuleCCSessionError(Exception): pass @@ -116,6 +120,18 @@ def create_command(command_name, params = None): msg = { 'command': cmd } return msg +def default_logconfig_handler(new_config, config_data): + errors = [] + + if config_data.get_module_spec().validate_config(False, new_config, errors): + isc.log.log_config_update(json.dumps(new_config), + json.dumps(config_data.get_module_spec().get_full_spec())) + else: + # no logging here yet, TODO: log these errors + print("Error in logging configuration, ignoring config update: ") + for err in errors: + print(err) + class ModuleCCSession(ConfigData): """This class maintains a connection to the command channel, as well as configuration options for modules. The module provides @@ -126,7 +142,7 @@ class ModuleCCSession(ConfigData): callbacks are called when 'check_command' is called on the ModuleCCSession""" - def __init__(self, spec_file_name, config_handler, command_handler, cc_session = None): + def __init__(self, spec_file_name, config_handler, command_handler, cc_session = None, handle_logging_config = False): """Initialize a ModuleCCSession. This does *NOT* send the specification and request the configuration yet. Use start() for that once the ModuleCCSession has been initialized. @@ -149,6 +165,11 @@ class ModuleCCSession(ConfigData): self._session.group_subscribe(self._module_name, "*") self._remote_module_configs = {} + self._remote_module_callbacks = {} + + if handle_logging_config: + self.add_remote_config(path_search('logging.spec', bind10_config.PLUGIN_PATHS), + default_logconfig_handler) def __del__(self): # If the CC Session obejct has been closed, it returns @@ -218,6 +239,9 @@ class ModuleCCSession(ConfigData): newc = self._remote_module_configs[module_name].get_local_config() isc.cc.data.merge(newc, new_config) self._remote_module_configs[module_name].set_local_config(newc) + if self._remote_module_callbacks[module_name] != None: + self._remote_module_callbacks[module_name](new_config, + self._remote_module_configs[module_name]) # For other modules, we're not supposed to answer return @@ -260,7 +284,7 @@ class ModuleCCSession(ConfigData): and return an answer created with create_answer()""" self._command_handler = command_handler - def add_remote_config(self, spec_file_name): + def add_remote_config(self, spec_file_name, config_update_callback = None): """Gives access to the configuration of a different module. These remote module options can at this moment only be accessed through get_remote_config_value(). This function @@ -289,9 +313,12 @@ class ModuleCCSession(ConfigData): if rcode == 0: if value != None and module_spec.validate_config(False, value): module_cfg.set_local_config(value); + if config_update_callback is not None: + config_update_callback(value, module_cfg) # all done, add it self._remote_module_configs[module_name] = module_cfg + self._remote_module_callbacks[module_name] = config_update_callback return module_name def remove_remote_config(self, module_name): @@ -299,6 +326,7 @@ class ModuleCCSession(ConfigData): if module_name in self._remote_module_configs: self._session.group_unsubscribe(module_name) del self._remote_module_configs[module_name] + del self._remote_module_callbacks[module_name] def get_remote_config_value(self, module_name, identifier): """Returns the current setting for the given identifier at the diff --git a/src/lib/python/isc/config/tests/Makefile.am b/src/lib/python/isc/config/tests/Makefile.am index 60da781630..3a1abe7893 100644 --- a/src/lib/python/isc/config/tests/Makefile.am +++ b/src/lib/python/isc/config/tests/Makefile.am @@ -14,6 +14,7 @@ endif for pytest in $(PYTESTS) ; do \ echo Running test: $$pytest ; \ env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python \ + B10_TEST_PLUGIN_DIR=$(abs_top_srcdir)/src/bin/cfgmgr/plugins \ CONFIG_TESTDATA_PATH=$(abs_top_srcdir)/src/lib/config/tests/testdata \ CONFIG_WR_TESTDATA_PATH=$(abs_top_builddir)/src/lib/config/tests/testdata \ $(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \ diff --git a/src/lib/python/isc/config/tests/ccsession_test.py b/src/lib/python/isc/config/tests/ccsession_test.py index a0cafd6446..830cbd762d 100644 --- a/src/lib/python/isc/config/tests/ccsession_test.py +++ b/src/lib/python/isc/config/tests/ccsession_test.py @@ -22,6 +22,7 @@ import os from isc.config.ccsession import * from isc.config.config_data import BIND10_CONFIG_DATA_VERSION from unittest_fakesession import FakeModuleCCSession, WouldBlockForever +import bind10_config class TestHelperFunctions(unittest.TestCase): def test_parse_answer(self): @@ -604,7 +605,43 @@ class TestModuleCCSession(unittest.TestCase): self.assertEqual(len(fake_session.message_queue), 1) mccs.check_command() self.assertEqual(len(fake_session.message_queue), 0) - + + def test_logconfig_handler(self): + # test whether default_logconfig_handler reacts nicely to + # bad data. We assume the actual logger output is tested + # elsewhere + self.assertRaises(TypeError, default_logconfig_handler); + self.assertRaises(TypeError, default_logconfig_handler, 1); + + spec = isc.config.module_spec_from_file( + path_search('logging.spec', bind10_config.PLUGIN_PATHS)) + config_data = ConfigData(spec) + + self.assertRaises(TypeError, default_logconfig_handler, 1, config_data) + + default_logconfig_handler({}, config_data) + + # Wrong data should not raise, but simply not be accepted + # This would log a lot of errors, so we may want to suppress that later + default_logconfig_handler({ "bad_data": "indeed" }, config_data) + default_logconfig_handler({ "bad_data": 1}, config_data) + default_logconfig_handler({ "bad_data": 1123 }, config_data) + default_logconfig_handler({ "bad_data": True }, config_data) + default_logconfig_handler({ "bad_data": False }, config_data) + default_logconfig_handler({ "bad_data": 1.1 }, config_data) + default_logconfig_handler({ "bad_data": [] }, config_data) + default_logconfig_handler({ "bad_data": [[],[],[[1, 3, False, "foo" ]]] }, + config_data) + default_logconfig_handler({ "bad_data": [ 1, 2, { "b": { "c": "d" } } ] }, + config_data) + + # Try a correct config + log_conf = {"loggers": + [{"name": "b10-xfrout", "output_options": + [{"output": "/tmp/bind10.log", + "destination": "file", + "flush": True}]}]} + default_logconfig_handler(log_conf, config_data) class fakeData: def decode(self): diff --git a/src/lib/python/isc/log/Makefile.am b/src/lib/python/isc/log/Makefile.am index 26735e77ae..b228caf428 100644 --- a/src/lib/python/isc/log/Makefile.am +++ b/src/lib/python/isc/log/Makefile.am @@ -15,6 +15,9 @@ log_la_CXXFLAGS = $(AM_CXXFLAGS) $(PYTHON_CXXFLAGS) log_la_LDFLAGS = $(PYTHON_LDFLAGS) log_la_LDFLAGS += -module log_la_LIBADD = $(top_builddir)/src/lib/log/liblog.la +log_la_LIBADD += $(top_builddir)/src/lib/cc/libcc.la +log_la_LIBADD += $(top_builddir)/src/lib/config/libcfgclient.la +log_la_LIBADD += $(top_builddir)/src/lib/exceptions/libexceptions.la log_la_LIBADD += $(PYTHON_LIB) # This is not installed, it helps locate the module during tests diff --git a/src/lib/python/isc/log/__init__.py b/src/lib/python/isc/log/__init__.py index ad77dff1f6..596d71c0cd 100644 --- a/src/lib/python/isc/log/__init__.py +++ b/src/lib/python/isc/log/__init__.py @@ -23,7 +23,14 @@ # Should we look there? Or define something in bind10_config? import os +import sys + cwd = os.getcwd() -pos = cwd.rfind('/src/') -import sys; sys.path.insert(0, cwd[:pos] + '/src/lib/python/isc/log/.libs') +base = os.path.split(cwd)[0] + +for base in sys.path: + loglibdir = os.path.join(base, 'isc/log/.libs') + if os.path.exists(loglibdir): + sys.path.append(loglibdir) + from log import * diff --git a/src/lib/python/isc/log/log.cc b/src/lib/python/isc/log/log.cc index b29c0058aa..a671b4bbca 100644 --- a/src/lib/python/isc/log/log.cc +++ b/src/lib/python/isc/log/log.cc @@ -22,6 +22,8 @@ #include #include +#include + #include #include @@ -49,6 +51,14 @@ namespace { // NULL and will use the global dictionary. MessageDictionary* testDictionary = NULL; +// To propagate python exceptions trough our code +// This exception is used to signal to the calling function that a +// proper Python Exception has already been set, and the caller +// should now return NULL. +// Since it is only used internally, and should not pass any +// information itself, is is not derived from std::exception +class InternalError {}; + PyObject* setTestDictionary(PyObject*, PyObject* args) { PyObject* enableO; @@ -177,6 +187,47 @@ init(PyObject*, PyObject* args) { Py_RETURN_NONE; } +PyObject* +logConfigUpdate(PyObject*, PyObject* args) { + // we have no wrappers for ElementPtr and ConfigData, + // So we expect JSON strings and convert them. + // The new_config object is assumed to have been validated. + + const char* new_config_json; + const char* mod_spec_json; + if (!PyArg_ParseTuple(args, "ss", + &new_config_json, &mod_spec_json)) { + return (NULL); + } + + try { + isc::data::ConstElementPtr new_config = + isc::data::Element::fromJSON(new_config_json); + isc::data::ConstElementPtr mod_spec_e = + isc::data::Element::fromJSON(mod_spec_json); + isc::config::ModuleSpec mod_spec(mod_spec_e); + isc::config::ConfigData config_data(mod_spec); + isc::config::default_logconfig_handler("logging", new_config, + config_data); + + Py_RETURN_NONE; + } catch (const isc::data::JSONError& je) { + std::string error_msg = std::string("JSON format error: ") + je.what(); + PyErr_SetString(PyExc_TypeError, error_msg.c_str()); + } catch (const isc::data::TypeError& de) { + PyErr_SetString(PyExc_TypeError, "argument 1 of log_config_update " + "is not a map of config data"); + } catch (const isc::config::ModuleSpecError& mse) { + PyErr_SetString(PyExc_TypeError, "argument 2 of log_config_update " + "is not a correct module specification"); + } catch (const std::exception& e) { + PyErr_SetString(PyExc_RuntimeError, e.what()); + } catch (...) { + PyErr_SetString(PyExc_RuntimeError, "Unknown C++ exception"); + } + return (NULL); +} + PyMethodDef methods[] = { {"set_test_dictionary", setTestDictionary, METH_VARARGS, "Set or unset testing mode for message dictionary. In testing, " @@ -198,6 +249,19 @@ PyMethodDef methods[] = { "logging severity (one of 'DEBUG', 'INFO', 'WARN', 'ERROR' or " "'FATAL'), a debug level (integer in the range 0-99) and a file name " "of a dictionary with message text translations."}, + {"log_config_update", logConfigUpdate, METH_VARARGS, + "Update logger settings. This method is automatically used when " + "ModuleCCSession is initialized with handle_logging_config set " + "to True. When called, the first argument is the new logging " + "configuration (in JSON format). The second argument is " + "the raw specification (as returned from " + "ConfigData.get_module_spec().get_full_spec(), and converted to " + "JSON format).\n" + "Raises a TypeError if either argument is not a (correct) JSON " + "string, or if the spec is not a correct spec.\n" + "If this call succeeds, the global logger settings have " + "been updated." + }, {NULL, NULL, 0, NULL} }; diff --git a/src/lib/python/isc/log/tests/Makefile.am b/src/lib/python/isc/log/tests/Makefile.am index 0eacbb1d9f..b6b6b19b77 100644 --- a/src/lib/python/isc/log/tests/Makefile.am +++ b/src/lib/python/isc/log/tests/Makefile.am @@ -23,5 +23,6 @@ endif echo Running test: $$pytest ; \ $(LIBRARY_PATH_PLACEHOLDER) \ env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/lib/python/isc/log:$(abs_top_builddir)/src/lib/log/python/.libs \ + B10_TEST_PLUGIN_DIR=$(abs_top_srcdir)/src/bin/cfgmgr/plugins \ $(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \ done diff --git a/src/lib/python/isc/log/tests/log_test.py b/src/lib/python/isc/log/tests/log_test.py index a463d59e02..4292b6c3cf 100644 --- a/src/lib/python/isc/log/tests/log_test.py +++ b/src/lib/python/isc/log/tests/log_test.py @@ -16,6 +16,9 @@ # This tests it can be loaded, nothing more yet import isc.log import unittest +import json +import bind10_config +from isc.config.ccsession import path_search class LogDict(unittest.TestCase): def setUp(self): @@ -52,6 +55,33 @@ class Manager(unittest.TestCase): # ignore errors like missing file? isc.log.init("root", "INFO", 0, "/no/such/file"); + def test_log_config_update(self): + log_spec = json.dumps(isc.config.module_spec_from_file(path_search('logging.spec', bind10_config.PLUGIN_PATHS)).get_full_spec()) + + self.assertRaises(TypeError, isc.log.log_config_update) + self.assertRaises(TypeError, isc.log.log_config_update, 1) + self.assertRaises(TypeError, isc.log.log_config_update, 1, 1) + self.assertRaises(TypeError, isc.log.log_config_update, 1, 1, 1) + + self.assertRaises(TypeError, isc.log.log_config_update, 1, log_spec) + self.assertRaises(TypeError, isc.log.log_config_update, [], log_spec) + self.assertRaises(TypeError, isc.log.log_config_update, "foo", log_spec) + self.assertRaises(TypeError, isc.log.log_config_update, "{ '", log_spec) + + # empty should pass + isc.log.log_config_update("{}", log_spec) + + # bad spec + self.assertRaises(TypeError, isc.log.log_config_update, "{}", json.dumps({"foo": "bar"})) + + # Try a correct one + log_conf = json.dumps({"loggers": + [{"name": "b10-xfrout", "output_options": + [{"output": "/tmp/bind10.log", + "destination": "file", + "flush": True}]}]}) + isc.log.log_config_update(log_conf, log_spec) + class Logger(unittest.TestCase): def tearDown(self): isc.log.reset()