Merge branch 'master' into trac2071_2

ChangeLog

@@ -1,3 +1,10 @@
+456.	[build]*	muks
+	BIND 10 now compiles against log4cplus-1.1.0 (RC releases)
+	also.  Note: some older versions of log4cplus don't work any more;
+	known oldest workable version is 1.0.4.  Thanks to John Lumby for
+	sending a patch.
+	(Trac #2169, git 7d7e5269d57451191c0aef1b127d292d3615fe2c)
+
 455.	[func]*		vorner
 	The server now uses newer API for data sources. This would be an
 	internal change, however, the data sources are now configured

src/bin/auth/tests/testdata/.gitignore

@@ -0,0 +1,8 @@
+/badExampleQuery_fromWire.wire
+/examplequery_fromWire.wire
+/iqueryresponse_fromWire.wire
+/multiquestion_fromWire.wire
+/queryBadEDNS_fromWire.wire
+/shortanswer_fromWire.wire
+/simplequery_fromWire.wire
+/simpleresponse_fromWire.wire

src/bin/auth/tests/testdata/spec.spec

@@ -3,4 +3,3 @@
         "module_name": "test"
     }
 }
-

src/bin/cfgmgr/plugins/.gitignore

@@ -0,0 +1,2 @@
+/datasrc.spec
+/datasrc.spec.pre

src/bin/cfgmgr/plugins/datasrc_config_plugin.py

@@ -16,20 +16,64 @@
 from isc.config.module_spec import module_spec_from_file
 from isc.util.file import path_search
 from bind10_config import PLUGIN_PATHS
+import isc.dns
+import isc.datasrc
+import json
+import os.path
+import copy
+
 spec = module_spec_from_file(path_search('datasrc.spec', PLUGIN_PATHS))
 
 def check(config):
     """
     Check the configuration.
     """
-    # TODO: Once we have solved ticket #2051, create the list and
-    # fill it with the configuration. We probably want to have some way
-    # to not load the data sources, just the configuration. It could
-    # be hacked together by subclassing ConfigurableClientList and
-    # having empty getDataSource method. But it looks like a hack and it
-    # won't really check the params configuration.
-    #
-    # For now, we let everything pass.
+    # Check the data layout first
+    errors=[]
+    if not spec.validate_config(False, config, errors):
+        return ' '.join(errors)
+
+    classes = config.get('classes')
+    # Nothing passed here
+    if classes is None:
+        return None
+
+    for rr_class_str in classes:
+        try:
+            rr_class = isc.dns.RRClass(rr_class_str)
+        except isc.dns.InvalidRRClass as irc:
+            return "The class '" + rr_class_str + "' is invalid"
+
+        dlist = isc.datasrc.ConfigurableClientList(rr_class)
+        # We get a copy here, as we are going to mangle the configuration.
+        # But we don't want our changes to propagate outside, to the real
+        # configuration.
+        client_config = copy.deepcopy(classes.get(rr_class_str))
+
+        for client in client_config:
+            if client['type'] == 'MasterFiles':
+                if not client.get('cache-enable', False):
+                    return 'The cache must be enabled in MasterFiles type'
+                params = client.get('params')
+                if type(params) != dict:
+                    return 'Params of MasterFiles must be a named set'
+                for name in params:
+                    try:
+                        isc.dns.Name(name)
+                    except Exception as e: # There are many related exceptions
+                        return str(e)
+                    if not os.path.exists(params[name]):
+                        return "Master file " + params[name] + " does not exist"
+                # We remove the list of zones locally. We already checked them,
+                # and the client list would have skipped them anyway, as we
+                # forbid cache. But it would produce a warning and we don't
+                # want that here.
+                client['params'] = {}
+
+        try:
+            dlist.configure(json.dumps(client_config), False)
+        except isc.datasrc.Error as dse:
+            return str(dse)
     return None
 
 def load():

src/bin/cfgmgr/plugins/tests/datasrc_test.py

@@ -16,12 +16,36 @@
 # Make sure we can load the module, put it into path
 import sys
 import os
+import unittest
+import json
 sys.path.extend(os.environ["B10_TEST_PLUGIN_DIR"].split(':'))
+import isc.log
 
 import datasrc_config_plugin
-import unittest
 
 class DatasrcTest(unittest.TestCase):
+    def reject(self, config):
+        """
+        Just a shortcut to check the config is rejected.
+        """
+        old = json.dumps(config)
+        self.assertIsNotNone(datasrc_config_plugin.check({"classes":
+                                                         config}))
+        # There's some data mangling inside the plugin. Check it does
+        # not propagate out, as it could change the real configuration.
+        self.assertEqual(old, json.dumps(config))
+
+    def accept(self, config):
+        """
+        Just a shortcut to check the config is accepted.
+        """
+        old = json.dumps(config)
+        self.assertIsNone(datasrc_config_plugin.check({"classes":
+                                                      config}))
+        # There's some data mangling inside the plugin. Check it does
+        # not propagate out, as it could change the real configuration.
+        self.assertEqual(old, json.dumps(config))
+
     def test_load(self):
         """
         Checks the entry point returns the correct values.
@@ -32,5 +56,104 @@ class DatasrcTest(unittest.TestCase):
         # The plugin stores it's spec
         self.assertEqual(spec, datasrc_config_plugin.spec)
 
+    def test_empty(self):
+        """
+        Check an empty input is OK.
+        """
+        self.accept({})
+
+    def test_invalid_spec(self):
+        """
+        Check it rejects stuff that is not well-formed according
+        to the spec.
+        """
+        self.reject("test")
+        self.reject(13)
+        self.reject([])
+        self.reject({"IN": {}})
+        self.reject({"IN": [{"bad-name": True}]})
+
+    def test_class(self):
+        """
+        The class is rejected, if it is wrong.
+        """
+        self.reject({"BAD": []})
+        self.reject({"": []})
+        # But with a good one, it works
+        for c in ["IN", "CH", "HS"]:
+            self.accept({c: []})
+
+    def test_mem_ok(self):
+        """
+        Test we accept an in-memory data source. It doesn't really matter
+        which one it is. We just want to make sure we accept something
+        and this one does not need any kind of path mangling to find
+        plugins.
+        """
+        self.accept({"IN": [{
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": {}
+        }]})
+
+    def test_dstype_bad(self):
+        """
+        The configuration is correct by the spec, but it would be rejected
+        by the client list. Check we reject it.
+        """
+        self.reject({"IN": [{
+            "type": "No such type"
+        }]})
+
+    def test_invalid_mem_params(self):
+        """
+        The client list skips in-memory sources. So we check it locally that
+        invalid things are rejected.
+        """
+        # The 'params' key is mandatory for MasterFiles
+        self.reject({"IN": [{
+            "type": "MasterFiles",
+            "cache-enable": True
+        }]})
+        # The cache must be enabled
+        self.reject({"IN": [{
+            "type": "MasterFiles",
+            "cache-enable": False,
+            "params": {}
+        }]})
+        self.reject({"IN": [{
+            "type": "MasterFiles",
+            "params": {}
+        }]})
+        # Bad params type
+        self.reject({"IN": [{
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": []
+        }]})
+        # Bad name
+        self.reject({"IN": [{
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": {
+                "example....org.": '/file/does/not/exist'
+            }
+        }]})
+
+    def test_no_such_file_mem(self):
+        """
+        We also check the existance of master files. Not the actual content,
+        though.
+        """
+        self.reject({"IN": [{
+            "type": "MasterFiles",
+            "cache-enable": True,
+            "params": {
+                "example.org.": '/file/does/not/exist'
+            }
+        }]})
+
 if __name__ == '__main__':
-        unittest.main()
+    isc.log.init("bind10")
+    isc.log.resetUnitTestRootLogger()
+    unittest.main()

src/bin/dhcp4/ctrl_dhcp4_srv.cc

@@ -82,7 +82,7 @@ void ControlledDhcpv4Srv::sessionReader(void) {
 }
 
 void ControlledDhcpv4Srv::establishSession() {
-    
+
     string specfile;
     if (getenv("B10_FROM_BUILD")) {
         specfile = string(getenv("B10_FROM_BUILD")) +
@@ -92,9 +92,9 @@ void ControlledDhcpv4Srv::establishSession() {
     }
 
     /// @todo: Check if session is not established already. Throw, if it is.
-    
+
     cout << "b10-dhcp4: my specfile is " << specfile << endl;
-    
+
     cc_session_ = new Session(io_service_.get_io_service());
 
     config_session_ = new ModuleCCSession(specfile, *cc_session_,

src/bin/dhcp4/main.cc

@@ -81,7 +81,7 @@ main(int argc, char* argv[]) {
                          (verbose_mode ? isc::log::DEBUG : isc::log::INFO),
                          isc::log::MAX_DEBUG_LEVEL, NULL);
 
-    cout << "b10-dhcp4: My pid=" << getpid() << ", binding to port " 
+    cout << "b10-dhcp4: My pid=" << getpid() << ", binding to port "
          << port_number << ", verbose " << (verbose_mode?"yes":"no") << endl;
 
     if (argc - optind > 0) {

src/lib/config/tests/testdata/spec40.spec

@@ -10,4 +10,3 @@
     ]
   }
 }
-

src/lib/datasrc/memory/tests/.gitignore

@@ -0,0 +1 @@
+/run_unittests

src/lib/datasrc/rbtree.h

@@ -1312,10 +1312,11 @@ private:
 
     /// Split one node into two nodes for "prefix" and "suffix" parts of
     /// the labels of the original node, respectively.  The given node
-    /// will hold the suffix labels, while the new node will hold the prefix.
-    /// The newly created node represents the labels that the original node
-    /// did, so necessary data are swapped.
-    /// (Note: as commented in the code, this behavior should be changed).
+    /// will hold the prefix, while a newly created node will hold the prefix.
+    /// Note that the original node still represents the same domain name in
+    /// the entire tree.  This ensures that a pointer to a node keeps its
+    /// semantics even if the tree structure is changed (as long as the node
+    /// itself remains valid).
     void nodeFission(util::MemorySegment& mem_sgmt, RBNode<T>& node,
                      const isc::dns::LabelSequence& new_prefix,
                      const isc::dns::LabelSequence& new_suffix);
@@ -1658,6 +1659,7 @@ RBTree<T>::insert(util::MemorySegment& mem_sgmt,
             dns::LabelSequence new_prefix = current_labels;
             new_prefix.stripRight(compare_result.getCommonLabels());
             nodeFission(mem_sgmt, *current, new_prefix, common_ancestor);
+            current = current->getParent();
         }
     }
 
@@ -1696,11 +1698,6 @@ RBTree<T>::deleteAllNodes(util::MemorySegment& mem_sgmt) {
     root_ = NULL;
 }
 
-// Note: when we redesign this (still keeping the basic concept), we should
-// change this part so the newly created node will be used for the inserted
-// name (and therefore the name for the existing node doesn't change).
-// Otherwise, things like shortcut links between nodes won't work.
-// See Trac #2054.
 template <typename T>
 void
 RBTree<T>::nodeFission(util::MemorySegment& mem_sgmt, RBNode<T>& node,
@@ -1712,38 +1709,45 @@ RBTree<T>::nodeFission(util::MemorySegment& mem_sgmt, RBNode<T>& node,
     // the end of the function, and it will keep consistent behavior
     // (i.e., a weak form of strong exception guarantee) even if code
     // after the call to this function throws an exception.
-    RBNode<T>* down_node = RBNode<T>::create(mem_sgmt, new_prefix);
-    node.resetLabels(new_suffix);
-
-    std::swap(node.data_, down_node->data_);
-
-    // Swap flags bitfields; yes, this is ugly (it appears we cannot use
-    // std::swap for bitfields).  The right solution is to implement
-    // the above note regarding #2054, then we won't have to swap the
-    // flags in the first place.
-    const bool is_root = node.isSubTreeRoot();
-    const uint32_t tmp = node.flags_;
-    node.flags_ = down_node->flags_;
-    down_node->flags_ = tmp;
-    node.setSubTreeRoot(is_root);
-
-    down_node->down_ = node.getDown();
-    if (down_node->down_ != NULL) {
-        down_node->down_->parent_ = down_node;
+    RBNode<T>* up_node = RBNode<T>::create(mem_sgmt, new_suffix);
+    node.resetLabels(new_prefix);
+
+    up_node->parent_ = node.getParent();
+    if (node.getParent() != NULL) {
+        if (node.getParent()->getLeft() == &node) {
+            node.getParent()->left_ = up_node;
+        } else if (node.getParent()->getRight() == &node) {
+            node.getParent()->right_ = up_node;
+        } else {
+            node.getParent()->down_ = up_node;
+        }
+    } else {
+        this->root_ = up_node;
     }
 
-    node.down_ = down_node;
-    down_node->parent_ = &node;
+    up_node->down_ = &node;
+    node.parent_ = up_node;
 
-    // Restore the color of the node (may have gotten changed by the flags
-    // swap)
-    node.setColor(down_node->getColor());
+    // inherit the left/right pointers from the original node, and set
+    // the original node's left/right pointers to NULL.
+    up_node->left_ = node.getLeft();
+    if (node.getLeft() != NULL) {
+        node.getLeft()->parent_ = up_node;
+    }
+    up_node->right_ = node.getRight();
+    if (node.getRight() != NULL) {
+        node.getRight()->parent_ = up_node;
+    }
+    node.left_ = NULL;
+    node.right_ = NULL;
 
-    // root node of sub tree, the initial color is BLACK
-    down_node->setColor(RBNode<T>::BLACK);
+    // set color of both nodes; the initial subtree node color is BLACK
+    up_node->setColor(node.getColor());
+    node.setColor(RBNode<T>::BLACK);
 
-    // mark it as the root of a subtree
-    down_node->setSubTreeRoot(true);
+    // set the subtree root flag of both nodes
+    up_node->setSubTreeRoot(node.isSubTreeRoot());
+    node.setSubTreeRoot(true);
 
     ++node_count_;
 }

src/lib/datasrc/tests/rbtree_unittest.cc

@@ -40,7 +40,7 @@ const size_t Name::MAX_LABELS;
 
 /* The initial structure of rbtree
  *
-*              .
+ *             .
  *             |
  *             b
  *           /   \
@@ -253,6 +253,36 @@ TEST_F(RBTreeTest, subTreeRoot) {
     EXPECT_TRUE(rbtnode->getFlag(RBNode<int>::FLAG_SUBTREE_ROOT));
 }
 
+TEST_F(RBTreeTest, additionalNodeFission) {
+    // These are additional nodeFission tests added by #2054's rewrite
+    // of RBTree::nodeFission(). These test specific corner cases that
+    // are not covered by other tests.
+
+    // Insert "t.0" (which becomes the left child of its parent)
+    EXPECT_EQ(RBTree<int>::SUCCESS,
+              rbtree_expose_empty_node.insert(mem_sgmt_, Name("t.0"),
+                                              &rbtnode));
+
+    // "t.0" is not a subtree root
+    EXPECT_EQ(RBTree<int>::EXACTMATCH,
+              rbtree_expose_empty_node.find(Name("t.0"), &rbtnode));
+    EXPECT_FALSE(rbtnode->getFlag(RBNode<int>::FLAG_SUBTREE_ROOT));
+
+    // fission the node "t.0"
+    EXPECT_EQ(RBTree<int>::ALREADYEXISTS,
+              rbtree_expose_empty_node.insert(mem_sgmt_, Name("0"),
+                                              &rbtnode));
+
+    // the node "0" ("0".down_ -> "t") should not be a subtree root. "t"
+    // should be a subtree root.
+    EXPECT_FALSE(rbtnode->getFlag(RBNode<int>::FLAG_SUBTREE_ROOT));
+
+    // "t.0" should be a subtree root now.
+    EXPECT_EQ(RBTree<int>::EXACTMATCH,
+              rbtree_expose_empty_node.find(Name("t.0"), &rbtnode));
+    EXPECT_TRUE(rbtnode->getFlag(RBNode<int>::FLAG_SUBTREE_ROOT));
+}
+
 TEST_F(RBTreeTest, findName) {
     // find const rbtnode
     // exact match
@@ -470,7 +500,7 @@ TEST_F(RBTreeTest, getAbsoluteNameError) {
 }
 
 /*
- *the domain order should be:
+ * The domain order should be:
  * ., a, b, c, d.e.f, x.d.e.f, w.y.d.e.f, o.w.y.d.e.f, p.w.y.d.e.f,
  * q.w.y.d.e.f, z.d.e.f, j.z.d.e.f, g.h, i.g.h, k.g.h
  *             . (no data, can't be found)

src/lib/log/logger_impl.cc

@@ -22,6 +22,7 @@
 #include <boost/static_assert.hpp>
 
 #include <log4cplus/configurator.h>
+#include <log4cplus/loggingmacros.h>
 
 #include <log/logger.h>
 #include <log/logger_impl.h>

src/lib/log/logger_level_impl.cc

@@ -185,20 +185,22 @@ LoggerLevelImpl::logLevelFromString(const log4cplus::tstring& level) {
 
 // Convert logging level to string.  If the level is a valid debug level,
 // return the string DEBUG, else return the empty string.
-log4cplus::tstring
+LoggerLevelImpl::LogLevelString
 LoggerLevelImpl::logLevelToString(log4cplus::LogLevel level) {
+    static const tstring debug_string("DEBUG");
+    static const tstring empty_string;
     Level bindlevel = convertToBindLevel(level);
     Severity& severity = bindlevel.severity;
     int& dbglevel = bindlevel.dbglevel;
 
     if ((severity == DEBUG) &&
         ((dbglevel >= MIN_DEBUG_LEVEL) && (dbglevel <= MAX_DEBUG_LEVEL))) {
-        return (tstring("DEBUG"));
+        return (debug_string);
     }
 
     // Unknown, so return empty string for log4cplus to try other conversion
     // functions.
-    return (tstring());
+    return (empty_string);
 }
 
 // Initialization.  Register the conversion functions with the LogLevelManager.

src/lib/log/logger_level_impl.h

@@ -16,6 +16,7 @@
 #define __LOGGER_LEVEL_IMPL_H
 
 #include <log4cplus/logger.h>
+#include <log4cplus/version.h>
 #include <log/logger_level.h>
 
 namespace isc {
@@ -65,6 +66,12 @@ namespace log {
 class LoggerLevelImpl {
 public:
 
+#if (LOG4CPLUS_VERSION >= LOG4CPLUS_MAKE_VERSION(1, 1, 0))
+    typedef log4cplus::tstring const & LogLevelString;
+#else
+    typedef log4cplus::tstring LogLevelString;
+#endif
+
     /// \brief Convert BIND 10 level to log4cplus logging level
     ///
     /// Converts the BIND 10 severity level into a log4cplus logging level.
@@ -112,7 +119,7 @@ public:
     /// \param level Extended logging level
     ///
     /// \return Equivalent string.
-    static log4cplus::tstring logLevelToString(log4cplus::LogLevel level);
+    static LogLevelString logLevelToString(log4cplus::LogLevel level);
 
     /// \brief Initialize extended logging levels
     ///

src/lib/log/logger_manager_impl.cc

@@ -17,6 +17,7 @@
 
 #include <log4cplus/logger.h>
 #include <log4cplus/configurator.h>
+#include <log4cplus/hierarchy.h>
 #include <log4cplus/consoleappender.h>
 #include <log4cplus/fileappender.h>
 #include <log4cplus/syslogappender.h>

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

@@ -386,6 +386,8 @@ def _validate_format(spec, value, errors):
     return True
 
 def _validate_item(spec, full, data, errors):
+    if spec.get('item_type') == 'any':
+        return True
     if not _validate_type(spec, data, errors):
         return False
     elif type(data) == list:

tests/lettuce/features/terrain/bind10_control.py

@@ -362,4 +362,3 @@ def configure_ddns_off(step):
         config commit
         \"\"\"
     """)
-