diff --git a/configure.ac b/configure.ac index 369080adbb..e072f89a85 100644 --- a/configure.ac +++ b/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 diff --git a/src/bin/cfgmgr/Makefile.am b/src/bin/cfgmgr/Makefile.am index a41448b403..fc0ed4a0c5 100644 --- a/src/bin/cfgmgr/Makefile.am +++ b/src/bin/cfgmgr/Makefile.am @@ -1,4 +1,4 @@ -SUBDIRS = . tests +SUBDIRS = . plugins tests pkglibexecdir = $(libexecdir)/@PACKAGE@ diff --git a/src/bin/cfgmgr/b10-cfgmgr.py.in b/src/bin/cfgmgr/b10-cfgmgr.py.in index 5355582344..607a6dcd29 100755 --- a/src/bin/cfgmgr/b10-cfgmgr.py.in +++ b/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: diff --git a/src/bin/cfgmgr/plugins/Makefile.am b/src/bin/cfgmgr/plugins/Makefile.am new file mode 100644 index 0000000000..952fde6edc --- /dev/null +++ b/src/bin/cfgmgr/plugins/Makefile.am @@ -0,0 +1 @@ +EXTRA_DIST = README diff --git a/src/bin/cfgmgr/plugins/README b/src/bin/cfgmgr/plugins/README new file mode 100644 index 0000000000..27fb0c0025 --- /dev/null +++ b/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) diff --git a/src/bin/cfgmgr/tests/Makefile.am b/src/bin/cfgmgr/tests/Makefile.am index 16e6223e2c..68666e6a3b 100644 --- a/src/bin/cfgmgr/tests/Makefile.am +++ b/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 diff --git a/src/bin/cfgmgr/tests/b10-cfgmgr_test.py.in b/src/bin/cfgmgr/tests/b10-cfgmgr_test.py.in index 037a106c9e..37cd0f50c1 100644 --- a/src/bin/cfgmgr/tests/b10-cfgmgr_test.py.in +++ b/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) diff --git a/src/bin/cfgmgr/tests/testdata/plugins/testplugin.py b/src/bin/cfgmgr/tests/testdata/plugins/testplugin.py new file mode 100644 index 0000000000..a50eefe164 --- /dev/null +++ b/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) diff --git a/src/lib/nsas/tests/nameserver_address_store_unittest.cc b/src/lib/nsas/tests/nameserver_address_store_unittest.cc index d389063cb0..abbac1db9e 100644 --- a/src/lib/nsas/tests/nameserver_address_store_unittest.cc +++ b/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. +/// \brief Nameserver Address Store Tests +/// +/// This file contains tests for the nameserver address store as a whole. + +#include +#include +#include +#include + +#include +#include +#include +#include + #include -/// \brief Test Deleter Objects -/// -/// 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. - -#include #include #include +#include +#include -#include -#include -#include -#include - -#include -#include - -#include "../nameserver_address_store.h" -#include "../nsas_entry_compare.h" -#include "../nameserver_entry.h" -#include "../zone_entry.h" #include "../address_request_callback.h" +#include "../nameserver_address_store.h" +#include "../nameserver_entry.h" +#include "../nsas_entry_compare.h" +#include "../zone_entry.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& 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& 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 callback) + RRsetPtr authority, + boost::shared_ptr 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 resolver_; -}; + boost::shared_ptr 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( 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 Result; + static vector 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 getCallback() { + return (boost::shared_ptr(new NSASCallback)); + } + + // Member variables + // Vector of pointers to nameserver and zone entries. std::vector > nameservers_; std::vector > zones_; - RRsetPtr authority_, empty_authority_; + // Authority sections used in the tests + RRsetPtr authority_; + RRsetPtr empty_authority_; + // ... and the resolver boost::shared_ptr resolver_; - - class NSASCallback : public AddressRequestCallback { - public: - typedef pair Result; - static vector 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 getCallback() { - return (boost::shared_ptr(new NSASCallback)); - } }; +/// Definition of the static results object vector 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 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& 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(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 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::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::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 diff --git a/src/lib/nsas/tests/nameserver_entry_unittest.cc b/src/lib/nsas/tests/nameserver_entry_unittest.cc index d489cbc7b4..7e9f6750bf 100644 --- a/src/lib/nsas/tests/nameserver_entry_unittest.cc +++ b/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 diff --git a/src/lib/nsas/tests/nsas_test.h b/src/lib/nsas/tests/nsas_test.h index 0bd5e36aeb..0fcca39632 100644 --- a/src/lib/nsas/tests/nsas_test.h +++ b/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) { diff --git a/src/lib/python/isc/config/cfgmgr.py b/src/lib/python/isc/config/cfgmgr.py index 8561378ed8..88a93e17b0 100644 --- a/src/lib/python/isc/config/cfgmgr.py +++ b/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]) - seq = self.cc.group_sendmsg(update_cmd, module_name) - # replace 'our' answer with that of the module + 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) + 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) diff --git a/src/lib/python/isc/config/tests/cfgmgr_test.py b/src/lib/python/isc/config/tests/cfgmgr_test.py index 9534e14094..b06db31797 100644 --- a/src/lib/python/isc/config/tests/cfgmgr_test.py +++ b/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 ] } diff --git a/tests/tools/badpacket/scan.cc b/tests/tools/badpacket/scan.cc index a6e7229e54..b0c605cc12 100644 --- a/tests/tools/badpacket/scan.cc +++ b/tests/tools/badpacket/scan.cc @@ -21,6 +21,10 @@ #include +// on sunstudio, asio.hpp needs unistd.h for pipe() to be defined +#ifdef HAVE_UNISTD_H +#include +#endif #include #include