Merge branch 'master' into trac704

configure.ac

@@ -612,6 +612,7 @@ AC_CONFIG_FILES([Makefile
                  src/bin/bindctl/Makefile
                  src/bin/bindctl/tests/Makefile
                  src/bin/cfgmgr/Makefile
+                 src/bin/cfgmgr/plugins/Makefile
                  src/bin/cfgmgr/tests/Makefile
                  src/bin/host/Makefile
                  src/bin/loadzone/Makefile

src/bin/cfgmgr/Makefile.am

@@ -1,4 +1,4 @@
-SUBDIRS = . tests
+SUBDIRS = . plugins tests
 
 pkglibexecdir = $(libexecdir)/@PACKAGE@
 

src/bin/cfgmgr/b10-cfgmgr.py.in

@@ -23,6 +23,8 @@ import isc.util.process
 import signal
 import os
 from optparse import OptionParser
+import glob
+import os.path
 
 isc.util.process.rename()
 
@@ -39,9 +41,11 @@ if "B10_FROM_SOURCE" in os.environ:
         DATA_PATH = os.environ["B10_FROM_SOURCE_LOCALSTATEDIR"]
     else:
         DATA_PATH = os.environ["B10_FROM_SOURCE"]
+    PLUGIN_PATH = [DATA_PATH + '/src/bin/cfgmgr/plugins']
 else:
     PREFIX = "@prefix@"
     DATA_PATH = "@localstatedir@/@PACKAGE@".replace("${prefix}", PREFIX)
+    PLUGIN_PATHS = ["@prefix@/share/@PACKAGE@/config_plugins"]
 DEFAULT_CONFIG_FILE = "b10-config.db"
 
 cm = None
@@ -65,6 +69,28 @@ def signal_handler(signal, frame):
     if cm:
         cm.running = False
 
+def load_plugins(path, cm):
+    """Load all python files in the given path and treat them as plugins."""
+    # Find the python files
+    plugins = glob.glob(path + os.sep + '*.py')
+    # Search this directory first, but leave the others there for the imports
+    # of the modules
+    sys.path.insert(0, path)
+    try:
+        for plugin in plugins:
+            # Generate the name of the plugin
+            filename = os.path.basename(plugin)
+            name = filename[:-3]
+            # Load it
+            module = __import__(name)
+            # Ask it to provide the spec and checking function
+            (spec, check_func) = module.load()
+            # And insert it into the manager
+            cm.set_virtual_module(spec, check_func)
+    finally:
+        # Restore the search path
+        sys.path = sys.path[1:]
+
 def main():
     options = parse_options()
     global cm
@@ -73,6 +99,8 @@ def main():
         signal.signal(signal.SIGINT, signal_handler)
         signal.signal(signal.SIGTERM, signal_handler)
         cm.read_config()
+        for ppath in PLUGIN_PATHS:
+            load_plugins(ppath, cm)
         cm.notify_boss()
         cm.run()
     except SessionError as se:

src/bin/cfgmgr/plugins/Makefile.am

@@ -0,0 +1 @@
+EXTRA_DIST = README

src/bin/cfgmgr/plugins/README

@@ -0,0 +1,34 @@
+How to write a configuration plugin
+===================================
+
+The plugins are used to describe configuration modules that have no hosting
+process. Therefore there's no process to provide their specification and to
+check them for correctness. So the plugin takes this responsibility.
+
+Each plugin is a python file installed into the
+`@prefix@/share/@PACKAGE@/config_plugins` directory (usually
+`/usr/share/bind10/config_plugins`). It is loaded automatically at startup.
+
+The entrypoint of a plugin is function called `load()`. It should return a
+tuple, first value should be the module specification (usually instance of
+`isc.config.module_spec.ModuleSpec`, loaded by `module_spec_from_file()`).
+
+The second value is a callable object. It will be called whenever the
+configuration of module needs to be checked. The only parameter will be the new
+config (as python dictionary). To accept the new configuration, return None. If
+you return a string, it is taken as an error message. If you raise an
+exception, the config is rejected as well, however it is not considered a
+graceful rejection, but a failure of the module.
+
+So, this is how a plugin could look like:
+
+  from isc.config.module_spec import module_spec_from_file
+
+  def check(config):
+      if config['bogosity'] > 1:
+          return "Too bogus to live with"
+      else:
+          return None
+
+  def load():
+      return (module_spec_from_file('module_spec.spec'), check)

src/bin/cfgmgr/tests/Makefile.am

@@ -1,7 +1,7 @@
 PYCOVERAGE_RUN = @PYCOVERAGE_RUN@
 PYTESTS = b10-cfgmgr_test.py
 
-EXTRA_DIST = $(PYTESTS)
+EXTRA_DIST = $(PYTESTS) testdata/plugins/testplugin.py
 
 # test using command-line arguments, so use check-local target instead of TESTS
 check-local:
@@ -12,6 +12,7 @@ if ENABLE_PYTHON_COVERAGE
 endif
 	for pytest in $(PYTESTS) ; do \
 	echo Running test: $$pytest ; \
+	env TESTDATA_PATH=$(abs_srcdir)/testdata \
 	env PYTHONPATH=$(abs_top_srcdir)/src/lib/python:$(abs_top_builddir)/src/lib/python:$(abs_top_builddir)/src/bin/cfgmgr \
 	$(PYCOVERAGE_RUN) $(abs_builddir)/$$pytest || exit ; \
 	done

src/bin/cfgmgr/tests/b10-cfgmgr_test.py.in

@@ -30,6 +30,7 @@ class MyConfigManager:
         self.run_called = False
         self.write_config_called = False
         self.running = True
+        self.virtual_modules = []
 
     def read_config(self):
         self.read_config_called = True
@@ -43,6 +44,24 @@ class MyConfigManager:
     def write_config(self):
         self.write_config_called = True
 
+    def set_virtual_module(self, spec, function):
+        self.virtual_modules.append((spec, function))
+
+class TestPlugins(unittest.TestCase):
+    def test_load_plugins(self):
+        """Test we can successfully find and load the mock plugin."""
+        # Let it load the plugin
+        b = __import__("b10-cfgmgr")
+        # The parameters aren't important for this test
+        cm = MyConfigManager(None, None)
+        b.load_plugins(os.environ['TESTDATA_PATH'] + os.sep + 'plugins', cm)
+        # Check exactly one plugin was loaded and the right data were fed into
+        # the cm
+        self.assertEqual(len(cm.virtual_modules), 1)
+        (spec, check) = cm.virtual_modules[0]
+        self.assertEqual(spec.get_module_name(), "mock_config_plugin")
+        self.assertEqual(check(None), "Mock config plugin")
+
 class TestConfigManagerStartup(unittest.TestCase):
     def test_cfgmgr(self):
         # some creative module use;
@@ -50,13 +69,24 @@ class TestConfigManagerStartup(unittest.TestCase):
         # this also gives us the chance to override the imported
         # module ConfigManager in it.
         b = __import__("b10-cfgmgr")
+        orig_load = b.load_plugins
+        b.PLUGIN_PATHS = ["/plugin/path"]
+        self.loaded_plugins = False
+        def load_plugins(path, cm):
+            # Check it's called with proper arguments
+            self.assertEqual(path, "/plugin/path")
+            self.assertTrue(isinstance(cm, MyConfigManager))
+            self.loaded_plugins = True
+        b.load_plugins = load_plugins
         b.ConfigManager = MyConfigManager
 
         b.main()
+        b.load_plugins = orig_load
 
         self.assertTrue(b.cm.read_config_called)
         self.assertTrue(b.cm.notify_boss_called)
         self.assertTrue(b.cm.run_called)
+        self.assertTrue(self.loaded_plugins)
         # if there are no changes, config is not written
         self.assertFalse(b.cm.write_config_called)
 
@@ -81,6 +111,7 @@ class TestConfigManagerStartup(unittest.TestCase):
 
         os.environ["B10_FROM_SOURCE"] = tmp_env_var
         b = __import__("b10-cfgmgr", globals(), locals())
+        b.PLUGIN_PATH = [] # It's enough to test plugins in one test
         b.ConfigManager = MyConfigManager
         self.assertEqual(tmp_env_var, b.DATA_PATH)
 

src/bin/cfgmgr/tests/testdata/plugins/testplugin.py

@@ -0,0 +1,34 @@
+# 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.
+
+# A test plugin. It does mostly nothing, just provides a function we can
+# recognize from the test. However, it looks like a real plugin, with the
+# (almost) correct interface, even when it's not used.
+
+class MockSpec:
+    """Mock spec, contains no data, it can only provide it's name.
+       It'll not really be used for anything."""
+    def get_module_name(self):
+        return "mock_config_plugin"
+
+def mock_check_config(config):
+    """Mock function to check config. Does nothing, only returns
+       an "error" string to indicate it's this one."""
+    return "Mock config plugin"
+
+def load():
+    """When a plugin is loaded, this is called to provide the spec and
+       checking function."""
+    return (MockSpec(), mock_check_config)

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

@@ -170,6 +170,10 @@ class ConfigManager:
         self.data_path = data_path
         self.database_filename = database_filename
         self.module_specs = {}
+        # Virtual modules are the ones which have no process running. The
+        # checking of validity is done by functions presented here instead
+        # of some other process
+        self.virtual_modules = {}
         self.config = ConfigManagerData(data_path, database_filename)
         if session:
             self.cc = session
@@ -187,11 +191,20 @@ class ConfigManager:
         """Adds a ModuleSpec"""
         self.module_specs[spec.get_module_name()] = spec
 
+    def set_virtual_module(self, spec, check_func):
+        """Adds a virtual module with its spec and checking function."""
+        self.module_specs[spec.get_module_name()] = spec
+        self.virtual_modules[spec.get_module_name()] = check_func
+
     def remove_module_spec(self, module_name):
         """Removes the full ModuleSpec for the given module_name.
+           Also removes the virtual module check function if it
+           was present.
            Does nothing if the module was not present."""
         if module_name in self.module_specs:
             del self.module_specs[module_name]
+        if module_name in self.virtual_modules:
+            del self.virtual_modules[module_name]
 
     def get_module_spec(self, module_name = None):
         """Returns the full ModuleSpec for the module with the given
@@ -299,24 +312,48 @@ class ConfigManager:
         # todo: use api (and check the data against the definition?)
         old_data = copy.deepcopy(self.config.data)
         conf_part = data.find_no_exc(self.config.data, module_name)
+        update_cmd = None
+        use_part = None
         if conf_part:
             data.merge(conf_part, cmd)
-            update_cmd = ccsession.create_command(ccsession.COMMAND_CONFIG_UPDATE,
-                                                  conf_part)
-            seq = self.cc.group_sendmsg(update_cmd, module_name)
-            try:
-                answer, env = self.cc.group_recvmsg(False, seq)
-            except isc.cc.SessionTimeout:
-                answer = ccsession.create_answer(1, "Timeout waiting for answer from " + module_name)
+            use_part = conf_part
         else:
             conf_part = data.set(self.config.data, module_name, {})
             data.merge(conf_part[module_name], cmd)
-            # send out changed info
-            update_cmd = ccsession.create_command(ccsession.COMMAND_CONFIG_UPDATE,
-                                                  conf_part[module_name])
+            use_part = conf_part[module_name]
+
+        # The command to send
+        update_cmd = ccsession.create_command(ccsession.COMMAND_CONFIG_UPDATE,
+                                              use_part)
+
+        # TODO: This design might need some revisiting. We might want some
+        # polymorphism instead of branching. But it just might turn out it
+        # will get solved by itself when we move everything to virtual modules
+        # (which is possible solution to the offline configuration problem)
+        # or when we solve the incorect behaviour here when a config is
+        # rejected (spying modules don't know it was rejected and some modules
+        # might have been commited already).
+        if module_name in self.virtual_modules:
+            # The module is virtual, so call it to get the answer
+            try:
+                error = self.virtual_modules[module_name](use_part)
+                if error is None:
+                    answer = ccsession.create_answer(0)
+                    # OK, it is successful, send the notify, but don't wait
+                    # for answer
+                    seq = self.cc.group_sendmsg(update_cmd, module_name)
+                else:
+                    answer = ccsession.create_answer(1, error)
+            # Make sure just a validating plugin don't kill the whole manager
+            except Exception as excp:
+                # Provide answer
+                answer = ccsession.create_answer(1, "Exception: " + str(excp))
+        else:
+            # Real module, send it over the wire to it
+            # send out changed info and wait for answer
             seq = self.cc.group_sendmsg(update_cmd, module_name)
-            # replace 'our' answer with that of the module
             try:
+                # replace 'our' answer with that of the module
                 answer, env = self.cc.group_recvmsg(False, seq)
             except isc.cc.SessionTimeout:
                 answer = ccsession.create_answer(1, "Timeout waiting for answer from " + module_name)

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

@@ -135,6 +135,8 @@ class TestConfigManager(unittest.TestCase):
         self.assert_(module_spec.get_module_name() not in self.cm.module_specs)
         self.cm.set_module_spec(module_spec)
         self.assert_(module_spec.get_module_name() in self.cm.module_specs)
+        self.assert_(module_spec.get_module_name() not in
+                     self.cm.virtual_modules)
 
     def test_remove_module_spec(self):
         module_spec = isc.config.module_spec.module_spec_from_file(self.data_path + os.sep + "spec1.spec")
@@ -143,6 +145,30 @@ class TestConfigManager(unittest.TestCase):
         self.assert_(module_spec.get_module_name() in self.cm.module_specs)
         self.cm.remove_module_spec(module_spec.get_module_name())
         self.assert_(module_spec.get_module_name() not in self.cm.module_specs)
+        self.assert_(module_spec.get_module_name() not in
+                     self.cm.virtual_modules)
+
+    def test_add_remove_virtual_module(self):
+        module_spec = isc.config.module_spec.module_spec_from_file(
+            self.data_path + os.sep + "spec1.spec")
+        check_func = lambda: True
+        # Make sure it's not in there before
+        self.assert_(module_spec.get_module_name() not in self.cm.module_specs)
+        self.assert_(module_spec.get_module_name() not in
+                     self.cm.virtual_modules)
+        # Add it there
+        self.cm.set_virtual_module(module_spec, check_func)
+        # Check it's in there
+        self.assert_(module_spec.get_module_name() in self.cm.module_specs)
+        self.assertEqual(self.cm.module_specs[module_spec.get_module_name()],
+                      module_spec)
+        self.assertEqual(self.cm.virtual_modules[module_spec.get_module_name()],
+                      check_func)
+        # Remove it again
+        self.cm.remove_module_spec(module_spec.get_module_name())
+        self.assert_(module_spec.get_module_name() not in self.cm.module_specs)
+        self.assert_(module_spec.get_module_name() not in
+                     self.cm.virtual_modules)
 
     def test_get_module_spec(self):
         module_spec = isc.config.module_spec.module_spec_from_file(self.data_path + os.sep + "spec1.spec")
@@ -312,6 +338,51 @@ class TestConfigManager(unittest.TestCase):
                                 },
                                 {'result': [0]})
 
+    def test_set_config_virtual(self):
+        """Test that if the module is virtual, we don't send it over the
+           message bus, but call the checking function.
+           """
+        # We run the same three times, with different return values
+        def single_test(value, returnFunc, expectedResult):
+            # Because closures can't assign to closed-in variables, we pass
+            # it trough self
+            self.called_with = None
+            def check_test(new_data):
+                self.called_with = new_data
+                return returnFunc()
+
+            # Register our virtual module
+            self.cm.set_virtual_module(self.spec, check_test)
+            # The fake session will throw now if it tries to read a response.
+            # Handy, we don't need to find a complicated way to check for it.
+            result = self.cm._handle_set_config_module(self.spec.
+                                                       get_module_name(),
+                                                       {'item1': value})
+            # Check the correct result is passed and our function was called
+            # With correct data
+            self.assertEqual(self.called_with['item1'], value)
+            self.assertEqual(result, {'result': expectedResult})
+            if expectedResult[0] == 0:
+                # Check it provided the correct notification
+                self.assertEqual(len(self.fake_session.message_queue), 1)
+                self.assertEqual({'command': [ 'config_update',
+                                 {'item1': value}]},
+                                 self.fake_session.get_message('Spec2', None))
+                # and the queue should now be empty again
+                self.assertEqual(len(self.fake_session.message_queue), 0)
+            else:
+                # It shouldn't send anything on error
+                self.assertEqual(len(self.fake_session.message_queue), 0)
+
+        # Success
+        single_test(5, lambda: None, [0])
+        # Graceful error
+        single_test(6, lambda: "Just error", [1, "Just error"])
+        # Exception from the checker
+        def raiser():
+            raise Exception("Just exception")
+        single_test(7, raiser, [1, "Exception: Just exception"])
+
     def test_set_config_all(self):
         my_ok_answer = { 'result': [ 0 ] }
 

tests/tools/badpacket/Makefile.am

@@ -20,6 +20,11 @@ badpacket_SOURCES += option_info.cc option_info.h
 badpacket_SOURCES += scan.cc scan.h
 badpacket_SOURCES += version.h
 
+badpacket_CXXFLAGS = $(AM_CXXFLAGS)
+if USE_CLANGPP
+badpacket_CXXFLAGS += -Wno-error
+endif
+
 badpacket_LDADD  = $(top_builddir)/src/lib/asiodns/libasiodns.la
 badpacket_LDADD += $(top_builddir)/src/lib/dns/libdns++.la
 badpacket_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la

tests/tools/badpacket/scan.cc

@@ -19,6 +19,12 @@
 
 #include <stdlib.h>
 
+#include <config.h>
+
+// on sunstudio, asio.hpp needs unistd.h for pipe() to be defined
+#ifdef HAVE_UNISTD_H
+#include <unistd.h>
+#endif
 #include <asio.hpp>
 
 #include <asiolink/io_address.h>