Merge branch 'master' of ssh://bind10.isc.org/var/bind10/git/bind10

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/nsas/tests/nameserver_address_store_unittest.cc

@@ -12,31 +12,32 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <config.h>
-
-/// \brief Test Deleter Objects
+/// \brief Nameserver Address Store Tests
 ///
-/// This file contains tests for the "deleter" classes within the nameserver
-/// address store.  These act to delete zones from the zone hash table when
-/// the element reaches the top of the LRU list.
+/// This file contains tests for the nameserver address store as a whole.
 
-#include <dns/rrttl.h>
-#include <dns/rdataclass.h>
-#include <dns/rrclass.h>
+#include <algorithm>
+#include <cassert>
+#include <string.h>
+#include <vector>
 
-#include <gtest/gtest.h>
-#include <boost/shared_ptr.hpp>
-#include <boost/lexical_cast.hpp>
 #include <boost/foreach.hpp>
+#include <boost/lexical_cast.hpp>
+#include <boost/shared_ptr.hpp>
+#include <gtest/gtest.h>
 
-#include <string.h>
-#include <cassert>
+#include <config.h>
 
+#include <dns/rdataclass.h>
+#include <dns/rrclass.h>
+#include <dns/rrset.h>
+#include <dns/rrttl.h>
+
+#include "../address_request_callback.h"
 #include "../nameserver_address_store.h"
-#include "../nsas_entry_compare.h"
 #include "../nameserver_entry.h"
+#include "../nsas_entry_compare.h"
 #include "../zone_entry.h"
-#include "../address_request_callback.h"
 #include "nsas_test.h"
 
 using namespace isc::dns;
@@ -67,43 +68,62 @@ public:
     virtual ~DerivedNsas()
     {}
 
-    /// \brief Add Nameserver Entry to Hash and LRU Tables
+    /// \brief Add Nameserver Entry to hash and LRU tables
+    ///
+    /// \param entry Nameserver Entry to add.
     void AddNameserverEntry(boost::shared_ptr<NameserverEntry>& entry) {
         HashKey h = entry->hashKey();
         nameserver_hash_->add(entry, h);
         nameserver_lru_->add(entry);
     }
 
-    /// \brief Add Zone Entry to Hash and LRU Tables
+    /// \brief Add Zone Entry to hash and LRU tables
+    ///
+    /// \param entry Zone Entry to add.
     void AddZoneEntry(boost::shared_ptr<ZoneEntry>& entry) {
         HashKey h = entry->hashKey();
         zone_hash_->add(entry, h);
         zone_lru_->add(entry);
     }
-    /**
-     * \short Just wraps the common lookup
-     *
-     * It calls the lookup and provides the authority section
-     * if it is asked for by the resolver.
-     */
+
+    /// \brief Wrap the common lookup
+    ///
+    /// Calls the lookup and provides the authority section if it is asked
+    /// for by the resolver.
+    ///
+    /// \param name Name of zone for which an address is required
+    /// \param class_code Class of the zone
+    /// \param authority Pointer to authority section RRset to which NS
+    ///        records will be added.
+    /// \param callback Callback object used to pass result back to caller
     void lookupAndAnswer(const string& name, const RRClass& class_code,
-        RRsetPtr authority,
-        boost::shared_ptr<AddressRequestCallback> callback)
+                         RRsetPtr authority,
+                         boost::shared_ptr<AddressRequestCallback> callback)
     {
+        // Note how many requests are in the resolver's queue
         size_t size(resolver_->requests.size());
+
+        // Lookup the name.  This should generate a request for NS records.
         NameserverAddressStore::lookup(name, class_code, callback, ANY_OK);
-        // It asked something, the only thing it can ask is the NS list
         if (size < resolver_->requests.size()) {
+
+            // It asked something, the only thing it can ask is the NS list.
+            // Once answered, drop the request so no-one else sees it
             resolver_->provideNS(size, authority);
-            // Once answered, drop the request so noone else sees it
             resolver_->requests.erase(resolver_->requests.begin() + size);
+
         } else {
-            ADD_FAILURE() << "Not asked for NS";
+
+            // The test resolver's requests queue has not increased in size,
+            // so the lookup did not generate a request.
+            ADD_FAILURE() << "Lookup did not generate a request for NS records";
         }
     }
+
 private:
-    boost::shared_ptr<TestResolver> resolver_;
-};
+    boost::shared_ptr<TestResolver> resolver_; 
+                                ///< Resolver used to answer generated requests
+};                                              
 
 
 
@@ -111,6 +131,7 @@ private:
 class NameserverAddressStoreTest : public TestWithRdata {
 protected:
 
+    /// \brief Constructor
     NameserverAddressStoreTest() :
         authority_(new RRset(Name("example.net."), RRClass::IN(), RRType::NS(),
             RRTTL(128))),
@@ -118,8 +139,8 @@ protected:
             RRType::NS(), RRTTL(128))),
         resolver_(new TestResolver)
     {
-        // Constructor - initialize a set of nameserver and zone objects.  For
-        // convenience, these are stored in vectors.
+        // Initialize a set of nameserver and zone objects.  For convenience,
+        // these are stored in vectors.
         for (int i = 1; i <= 9; ++i) {
             std::string name = "nameserver" + boost::lexical_cast<std::string>(
                 i);
@@ -145,57 +166,70 @@ protected:
         NSASCallback::results.clear();
     }
 
+    /// \brief Internal callback object
+    ///
+    /// Callback object.  It just records whether the success() or
+    /// unreachable() methods were called and if success, a copy of the
+    /// Nameserver object. (The data is held in a static object that will
+    /// outlive the lifetime of the callback object.)
+    struct NSASCallback : public AddressRequestCallback {
+        typedef pair<bool, NameserverAddress> Result;
+        static vector<Result> results;
+
+        virtual void success(const NameserverAddress& address) {
+            results.push_back(Result(true, address));
+        }
+        virtual void unreachable() {
+            results.push_back(Result(false, NameserverAddress()));
+        }
+    };
+
+    /// \brief Return pointer to callback object
+    boost::shared_ptr<AddressRequestCallback> getCallback() {
+        return (boost::shared_ptr<AddressRequestCallback>(new NSASCallback));
+    }
+
+    // Member variables
+
     // Vector of pointers to nameserver and zone entries.
     std::vector<boost::shared_ptr<NameserverEntry> > nameservers_;
     std::vector<boost::shared_ptr<ZoneEntry> >       zones_;
 
-    RRsetPtr authority_, empty_authority_;
+    // Authority sections used in the tests
+    RRsetPtr authority_;
+    RRsetPtr empty_authority_;
 
+    // ... and the resolver
     boost::shared_ptr<TestResolver> resolver_;
-
-    class NSASCallback : public AddressRequestCallback {
-        public:
-            typedef pair<bool, NameserverAddress> Result;
-            static vector<Result> results;
-            virtual void success(const NameserverAddress& address) {
-                results.push_back(Result(true, address));
-            }
-            virtual void unreachable() {
-                results.push_back(Result(false, NameserverAddress()));
-            }
-    };
-
-    boost::shared_ptr<AddressRequestCallback> getCallback() {
-        return (boost::shared_ptr<AddressRequestCallback>(new NSASCallback));
-    }
 };
 
+/// Definition of the static results object
 vector<NameserverAddressStoreTest::NSASCallback::Result>
     NameserverAddressStoreTest::NSASCallback::results;
 
 
 /// \brief Remove Zone Entry from Hash Table
 ///
-/// Check that when an entry reaches the top of the zone LRU list, it is removed from the
-/// hash table as well.
+/// Check that when an entry reaches the top of the zone LRU list, it is removed
+/// from the hash table as well.
 TEST_F(NameserverAddressStoreTest, ZoneDeletionCheck) {
 
-    // Create a NSAS with a hash size of three and a LRU size of 9 (both zone and
-    // nameserver tables).
+    // Create a NSAS with a hash size of three and a LRU size of 9 (both zone
+    // and nameserver tables).
     DerivedNsas nsas(resolver_, 2, 2);
 
-    // Add six entries to the tables.  After addition the reference count of each element
-    // should be 3 - one for the entry in the zones_ vector, and one each for the entries
-    // in the LRU list and hash table.
+    // Add six entries to the tables.  After addition the reference count of
+    // each element should be 3 - one for the entry in the zones_ vector, and
+    // one each for the entries in the LRU list and hash table.
     for (int i = 1; i <= 6; ++i) {
         EXPECT_EQ(1, zones_[i].use_count());
         nsas.AddZoneEntry(zones_[i]);
         EXPECT_EQ(3, zones_[i].use_count());
     }
 
-    // Adding another entry should cause the first one to drop off the LRU list, which
-    // should also trigger the deletion of the entry from the hash table.  This should
-    // reduce its use count to 1.
+    // Adding another entry should cause the first one to drop off the LRU list,
+    // which should also trigger the deletion of the entry from the hash table.
+    //  This should reduce its use count to 1.
     EXPECT_EQ(1, zones_[7].use_count());
     nsas.AddZoneEntry(zones_[7]);
     EXPECT_EQ(3, zones_[7].use_count());
@@ -206,26 +240,26 @@ TEST_F(NameserverAddressStoreTest, ZoneDeletionCheck) {
 
 /// \brief Remove Entry from Hash Table
 ///
-/// Check that when an entry reaches the top of the LRU list, it is removed from the
-/// hash table as well.
+/// Check that when an entry reaches the top of the LRU list, it is removed from
+/// the hash table as well.
 TEST_F(NameserverAddressStoreTest, NameserverDeletionCheck) {
 
-    // Create a NSAS with a hash size of three and a LRU size of 9 (both zone and
-    // nameserver tables).
+    // Create a NSAS with a hash size of three and a LRU size of 9 (both zone
+    // and nameserver tables).
     DerivedNsas nsas(resolver_, 2, 2);
 
-    // Add six entries to the tables.  After addition the reference count of each element
-    // should be 3 - one for the entry in the nameservers_ vector, and one each for the entries
-    // in the LRU list and hash table.
+    // Add six entries to the tables.  After addition the reference count of
+    // each element should be 3 - one for the entry in the nameservers_ vector,
+    // and one each for the entries in the LRU list and hash table.
     for (int i = 1; i <= 6; ++i) {
         EXPECT_EQ(1, nameservers_[i].use_count());
         nsas.AddNameserverEntry(nameservers_[i]);
         EXPECT_EQ(3, nameservers_[i].use_count());
     }
 
-    // Adding another entry should cause the first one to drop off the LRU list, which
-    // should also trigger the deletion of the entry from the hash table.  This should
-    // reduce its use count to 1.
+    // Adding another entry should cause the first one to drop off the LRU list,
+    // which should also trigger the deletion of the entry from the hash table.
+    // This should reduce its use count to 1.
     EXPECT_EQ(1, nameservers_[7].use_count());
     nsas.AddNameserverEntry(nameservers_[7]);
     EXPECT_EQ(3, nameservers_[7].use_count());
@@ -238,14 +272,17 @@ TEST_F(NameserverAddressStoreTest, NameserverDeletionCheck) {
 /// Check if it asks correct questions and it keeps correct internal state.
 TEST_F(NameserverAddressStoreTest, emptyLookup) {
     DerivedNsas nsas(resolver_, 10, 10);
+
     // Ask it a question
     nsas.lookupAndAnswer("example.net.", RRClass::IN(), authority_,
         getCallback());
+
     // It should ask for IP addresses for ns.example.com.
     EXPECT_NO_THROW(resolver_->asksIPs(Name("ns.example.com."), 0, 1));
 
     // Ask another question for the same zone
     nsas.lookup("example.net.", RRClass::IN(), getCallback());
+
     // It should ask no more questions now
     EXPECT_EQ(2, resolver_->requests.size());
 
@@ -253,6 +290,7 @@ TEST_F(NameserverAddressStoreTest, emptyLookup) {
     authority_->setName(Name("example.com."));
     nsas.lookupAndAnswer("example.com.", RRClass::IN(), authority_,
         getCallback());
+
     // It still should ask nothing
     EXPECT_EQ(2, resolver_->requests.size());
 
@@ -272,11 +310,14 @@ TEST_F(NameserverAddressStoreTest, emptyLookup) {
 /// It should not ask anything and say it is unreachable right away.
 TEST_F(NameserverAddressStoreTest, zoneWithoutNameservers) {
     DerivedNsas nsas(resolver_, 10, 10);
+
     // Ask it a question
     nsas.lookupAndAnswer("example.net.", RRClass::IN(), empty_authority_,
         getCallback());
+
     // There should be no questions, because there's nothing to ask
     EXPECT_EQ(0, resolver_->requests.size());
+
     // And there should be one "unreachable" answer for the query
     ASSERT_EQ(1, NSASCallback::results.size());
     EXPECT_FALSE(NSASCallback::results[0].first);
@@ -289,9 +330,11 @@ TEST_F(NameserverAddressStoreTest, zoneWithoutNameservers) {
 /// without further asking.
 TEST_F(NameserverAddressStoreTest, unreachableNS) {
     DerivedNsas nsas(resolver_, 10, 10);
+
     // Ask it a question
     nsas.lookupAndAnswer("example.net.", RRClass::IN(), authority_,
         getCallback());
+
     // It should ask for IP addresses for example.com.
     EXPECT_NO_THROW(resolver_->asksIPs(Name("ns.example.com."), 0, 1));
 
@@ -299,6 +342,7 @@ TEST_F(NameserverAddressStoreTest, unreachableNS) {
     authority_->setName(Name("example.com."));
     nsas.lookupAndAnswer("example.com.", RRClass::IN(), authority_,
         getCallback());
+
     // It should ask nothing more now
     EXPECT_EQ(2, resolver_->requests.size());
 
@@ -308,12 +352,14 @@ TEST_F(NameserverAddressStoreTest, unreachableNS) {
 
     // We should have 2 answers now
     EXPECT_EQ(2, NSASCallback::results.size());
+
     // When we ask one same and one other zone with the same nameserver,
     // it should generate no questions and answer right away
     nsas.lookup("example.net.", RRClass::IN(), getCallback());
     authority_->setName(Name("example.org."));
     nsas.lookupAndAnswer("example.org.", RRClass::IN(), authority_,
         getCallback());
+
     // There should be 4 negative answers now
     EXPECT_EQ(4, NSASCallback::results.size());
     BOOST_FOREACH(const NSASCallback::Result& result, NSASCallback::results) {
@@ -326,18 +372,23 @@ TEST_F(NameserverAddressStoreTest, unreachableNS) {
 /// Does some asking, on a set of zones that share some nameservers, with
 /// slower answering, evicting data, etc.
 TEST_F(NameserverAddressStoreTest, CombinedTest) {
+
     // Create small caches, so we get some evictions
     DerivedNsas nsas(resolver_, 1, 1);
+
     // Ask for example.net. It has single nameserver out of the zone
     nsas.lookupAndAnswer("example.net.", RRClass::IN(), authority_,
         getCallback());
+
     // It should ask for the nameserver IP addresses
     EXPECT_NO_THROW(resolver_->asksIPs(Name("ns.example.com."), 0, 1));
     EXPECT_EQ(0, NSASCallback::results.size());
+
     // But we do not answer it right away. We create a new zone and
     // let this nameserver entry get out.
     rrns_->addRdata(rdata::generic::NS("example.cz"));
     nsas.lookupAndAnswer(EXAMPLE_CO_UK, RRClass::IN(), rrns_, getCallback());
+
     // It really should ask something, one of the nameservers
     // (or both)
     ASSERT_GT(resolver_->requests.size(), 2);
@@ -347,8 +398,8 @@ TEST_F(NameserverAddressStoreTest, CombinedTest) {
     EXPECT_NO_THROW(resolver_->asksIPs(name, 2, 3));
     EXPECT_EQ(0, NSASCallback::results.size());
 
-    size_t request_count(resolver_->requests.size());
     // This should still be in the hash table, so try it asks no more questions
+    size_t request_count(resolver_->requests.size());
     nsas.lookup("example.net.", RRClass::IN(), getCallback());
     EXPECT_EQ(request_count, resolver_->requests.size());
     EXPECT_EQ(0, NSASCallback::results.size());
@@ -356,6 +407,7 @@ TEST_F(NameserverAddressStoreTest, CombinedTest) {
     // We respond to one of the 3 nameservers
     EXPECT_NO_THROW(resolver_->answer(2, name, RRType::A(),
         rdata::in::A("192.0.2.1")));
+
     // That should trigger one answer
     EXPECT_EQ(1, NSASCallback::results.size());
     EXPECT_TRUE(NSASCallback::results[0].first);
@@ -363,6 +415,7 @@ TEST_F(NameserverAddressStoreTest, CombinedTest) {
         NSASCallback::results[0].second.getAddress().toText());
     EXPECT_NO_THROW(resolver_->answer(3, name, RRType::AAAA(),
         rdata::in::AAAA("2001:bd8::1")));
+
     // And there should be yet another query
     ASSERT_GT(resolver_->requests.size(), 4);
     EXPECT_NE(name, resolver_->requests[4].first->getName());
@@ -380,9 +433,9 @@ TEST_F(NameserverAddressStoreTest, CombinedTest) {
     EXPECT_EQ(request_count + 2, resolver_->requests.size());
     EXPECT_NO_THROW(resolver_->asksIPs(Name("ns.example.com."), request_count,
         request_count + 1));
-    // Now, we answer both queries for the same address
-    // and three (one for the original, one for this one) more answers should
-    // arrive
+
+    // Now, we answer both queries for the same address and three (one for the
+    // original, one for this one) more answers should arrive
     NSASCallback::results.clear();
     EXPECT_NO_THROW(resolver_->answer(0, Name("ns.example.com."), RRType::A(),
         rdata::in::A("192.0.2.2")));
@@ -395,5 +448,113 @@ TEST_F(NameserverAddressStoreTest, CombinedTest) {
     }
 }
 
+// Check that we can update the RTT associated with nameservers successfully.
+// Also checks that we can't set the RTT to zero (which would cause problems
+// with selection algorithm).
+TEST_F(NameserverAddressStoreTest, updateRTT) {
+
+    // Initialization.
+    string  zone_name = "example.net.";
+    string  ns_name = "ns.example.com."; 
+    vector<string> address;
+    address.push_back("192.0.2.1");
+    address.push_back("192.0.2.2");
+
+    uint32_t HIGH_RTT = 10000;  // 1E4; When squared, the result fits in 32 bits
+
+    DerivedNsas nsas(resolver_, 103, 107);   // Arbitrary cache sizes
+
+    // Ensure that location holding the addresses returned is empty.  We'll
+    // be using this throughout the tests.  As the full name is a bit of a
+    // mouthful, set up an alias.
+    vector<NameserverAddressStoreTest::NSASCallback::Result>& results =
+        NameserverAddressStoreTest::NSASCallback::results;
+    results.clear();
+
+    // Initialize the test resolver with the answer for the A record query
+    // for ns.example.com (the nameserver set for example.net in the class
+    // initialization).  We'll set two addresses.
+    Name ns_example_com(ns_name);
+    RRsetPtr ns_address = boost::shared_ptr<RRset>(new RRset(
+        ns_example_com, RRClass::IN(), RRType::A(), RRTTL(300)));
+    BOOST_FOREACH(string addr, address) {
+        ns_address->addRdata(rdata::in::A(addr));
+    }
+
+    // All set.  Ask for example.net.
+    boost::shared_ptr<AddressRequestCallback> callback = getCallback();
+    nsas.lookupAndAnswer(zone_name, RRClass::IN(), authority_, getCallback());
+
+    // This should generate two requests - one for A and one for AAAA.
+    EXPECT_EQ(2, resolver_->requests.size());
+
+    // Provide an answer that has two A records.  This should generate one
+    // result.
+    EXPECT_NO_THROW(resolver_->answer(0, ns_address));
+    EXPECT_EQ(1, results.size());
+
+    // We expect the lookup to be successful.  Check that the address is one of
+    // the two we've set and that the RTT associated with this nameserver is
+    // non-zero.
+    EXPECT_EQ(true, results[0].first);
+    vector<string>::iterator addr1 = find(address.begin(), address.end(),
+                                     results[0].second.getAddress().toText());
+    EXPECT_TRUE(addr1 != address.end());
+
+    // The RTT we got should be non-zero and less than the high value we are
+    // using for the test.
+    uint32_t rtt1 = results[0].second.getAddressEntry().getRTT();
+    EXPECT_NE(0, rtt1);
+    EXPECT_LT(rtt1, HIGH_RTT);
+
+    // Update the address with a very high RTT.  Owning to the way the NSAS is
+    // written, we can update the RTT but cannot read the new value back from
+    // the new object.  
+    results[0].second.updateRTT(HIGH_RTT);
+
+    // Get another nameserver.  As the probability of returning a particular
+    // address is proporational to 1/t^2, we should get the second address
+    // since the first now has a larger RTT.  However, this is not guaranteed
+    // - this is a probability (albeit small) of getting the first again. We'll
+    // allow three chances of getting the "wrong" address before we declare
+    // an error.
+    int attempt = 0;
+    vector<string>::iterator addr2 = addr1;
+    for (attempt = 0; (attempt < 3) && (*addr1 == *addr2); ++attempt) {
+        results.clear();
+        nsas.lookup(zone_name, RRClass::IN(), 
+                             getCallback(), ANY_OK);
+        addr2 = find(address.begin(), address.end(),
+                     results[0].second.getAddress().toText());
+    }
+    EXPECT_LT(attempt, 3);
+
+    // Ensure that the RTT is non-zero.
+    // obtained earlier.
+    uint32_t rtt2 = results[0].second.getAddressEntry().getRTT();
+    EXPECT_NE(0, rtt2);
+
+    // The test has shown that the code can return the two nameservers.  Now
+    // try to set the RTT for the last one returned to zero.  As there is a
+    // smoothing effect in the calculations which damps out an abrupt change
+    // in the RTT, the underlying RTT will not be set to zero immediately.  So
+    // loop a large number of times, each time setting it to zero.
+    //
+    // Between each setting of the RTT, we have to retrieve the nameserver from
+    // the NSAS again.  This means that we _could_ occasionally get the address
+    // of the one whose RTT we have raised to HIGH_RTT.  We overcome this by
+    // looping a _very_ large number of times.  Ultimately the RTT of both
+    // addresses should decay to a small value.
+    for (int i = 0; i < 2000; ++i) {   // 1000 times for each nameserver
+        results.clear();
+        nsas.lookup(zone_name, RRClass::IN(), getCallback(), ANY_OK);
+        EXPECT_EQ(1, results.size());
+        uint32_t rtt3 = results[0].second.getAddressEntry().getRTT();
+        EXPECT_NE(0, rtt3);
+        results[0].second.updateRTT(0);
+    }
+}
+
+
 } // namespace nsas
 } // namespace isc

src/lib/nsas/tests/nameserver_entry_unittest.cc

@@ -513,6 +513,11 @@ TEST_F(NameserverEntryTest, UpdateRTT) {
 
     // The rtt should be close to stable rtt value
     EXPECT_TRUE((stable_rtt - new_rtt) < (new_rtt - init_rtt));
+
+    // Finally, try updating the RTT to a very large value (large enough for
+    // RTT^2 - used in the internal calculation - to exceed a 32-bit value).
+    EXPECT_NO_THROW(vec[0].updateRTT(1000000000));  // 10^9
+
 }
 
 }   // namespace

src/lib/nsas/tests/nsas_test.h

@@ -321,20 +321,26 @@ class TestResolver : public isc::resolve::ResolverInterface {
 
         /*
          * Sends a simple answer to a query.
-         * Provide index of a query and the address to pass.
+         * 1) Provide index of a query and the address(es) to pass.
+         * 2) Provide index of query and components of address to pass.
          */
-        void answer(size_t index, const Name& name, const RRType& type,
-            const rdata::Rdata& rdata, size_t TTL = 100)
-        {
+        void answer(size_t index, RRsetPtr& set) {
             if (index >= requests.size()) {
                 throw NoSuchRequest();
             }
+            requests[index].second->success(createResponseMessage(set));
+        }
+
+        void answer(size_t index, const Name& name, const RRType& type,
+            const rdata::Rdata& rdata, size_t TTL = 100)
+        {
             RRsetPtr set(new RRset(name, RRClass::IN(),
                 type, RRTTL(TTL)));
             set->addRdata(rdata);
-            requests[index].second->success(createResponseMessage(set));
+            answer(index, set);
         }
 
+
         void provideNS(size_t index,
             RRsetPtr nameservers)
         {

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/scan.cc

@@ -21,6 +21,10 @@
 
 #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>