sync with trunk, resolving a conflict.

git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac418@3747 e5f2f494-b856-4b98-b285-d166d9295462

ChangeLog

@@ -1,3 +1,27 @@
+bind10-devel-20101201 released on December 01, 2010
+
+  125.  [func]		jelte
+	Added support for addressing individual list items in bindctl
+	configuration commands; If you have an element that is a list, you
+	can use foo[X] to address a specific item, where X is an integer
+	(starting at 0)
+	(Trac #405, svn r3739)
+
+  124.  [bug]		jreed
+	Fix some wrong version reporting. Now also show the version
+	for the component and BIND 10 suite. (Trac #302, svn r3696)
+
+  123.  [bug]		jelte
+	src/bin/bindctl printed values had the form of python literals
+	(e.g. 'True'), while the input requires valid JSON (e.g. 'true').
+	Output changed to JSON format for consistency. (svn r3694)
+
+  122.  [func]		stephen
+	src/bin/bind10: Added configuration options to Boss to determine
+	whether to start the authoritative server, recursive server (or
+	both). A dummy recursor has been provided for test purposes.
+	(Trac #412, svn r3676)
+
   121.  [func]		jinmei
 	src/lib/dns: Added support for TSIG RDATA.  At this moment this is
 	not much of real use, however, because no protocol support was
@@ -15,13 +39,14 @@
 	Note: this fix is incomplete and the loadzone would still be
 	confused if the owner name is a syntactically indistinguishable
 	from a TTL specification.  This is part of a more general issue
-	and will be addressed in Trac #413.  (Trac #411, svn r3599)
+	and will be addressed in Trac #413. (Trac #411, svn r3599)
 
   118.	[func]		jinmei
-	src/lib/dns: changed the interface of AbstractRRset::getRdataIterator()
+	src/lib/dns: changed the interface of
-	so that the internal cursor would point to the first RDATA
+	AbstractRRset::getRdataIterator() so that the internal
-	automatically.  This will be a more intuitive and less error prone
+	cursor would point to the first RDATA automatically.  This
-	behavior.  This is a backward compatible change. (Trac #410, r3595)
+	will be a more intuitive and less error prone behavior.
+	This is a backward compatible change. (Trac #410, r3595)
 
   117.  [func]		jinmei
 	src/lib/datasrc: added new zone and zone table classes for the
@@ -57,15 +82,13 @@
 	Add one mixin class to override the naive serve_forever() provided
 	in python library socketserver. Instead of polling for shutdwon
 	every poll_interval seconds, one socketpair is used to wake up
-	the waiting server.(Trac #352, svn r3366)
+	the waiting server. (Trac #352, svn r3366)
 
   111.	[bug]*   zhanglikun, Michal Vaner
-	Make sure process xfrin/xfrout/zonemgr/cmdctl can be stoped
+	Make sure process xfrin/xfrout/zonemgr/cmdctl can be stopped
 	properly when user enter "ctrl+c" or 'Boss shutdown' command
-	through	bindctl.
+	through bindctl.  The ZonemgrRefresh.run_timer and
-
+	NotifyOut.dispatcher spawn a thread themselves.
-	The ZonemgrRefresh.run_timer and NotifyOut.dispatcher spawn
-	a thread themselves.
 	(Trac #335, svn r3273)
 
   110.  [func]      Michal Vaner
@@ -77,7 +100,7 @@
   109.  [func]		naokikambe
 	Added the initial version of the stats module for the statistics
 	feature of BIND 10, which supports the restricted features and
-	items and reports via bindctl command (Trac #191, r3218)
+	items and reports via bindctl command. (Trac #191, r3218)
 	Added the document of the stats module, which is about how stats
 	module collects the data (Trac #170, [wiki:StatsModule])
 
@@ -104,11 +127,11 @@
   104.	[bug]		jerry
 	bin/zonemgr: zonemgr should be attempting to refresh expired zones.
 	(Trac #336, r3139)
-				   
+ 
   103.	[bug]		jerry
 	lib/python/isc/log: Fixed an issue with python logging,
-	python log shouldn't die with OSError.(Trac #267, r3137)
+	python log shouldn't die with OSError. (Trac #267, r3137)
-				   
+ 
   102.	[build]		jinmei
 	Disable threads in ASIO to minimize build time dependency.
 	(Trac #345, r3100)
@@ -161,7 +184,7 @@ bind10-devel-20100917 released on September 17, 2010
   93.	[bug]		jinmei
 	lib/datasrc: A DS query could crash the library (and therefore,
 	e.g. the authoritative server) if some RR of the same apex name
-	is stored in the hot spot cache.  (Trac #307, svn r2923)
+	is stored in the hot spot cache. (Trac #307, svn r2923)
 
   92.	[func]*		jelte
 	libdns_python (the python wrappers for libdns++) has been renamed
@@ -341,7 +364,7 @@ bind10-devel-20100701 released on July 1, 2010
   66.  [bug]		each
 	Check for duplicate RRsets before inserting data into a message
 	section; this, among other things, will prevent multiple copies
-	of the same CNAME from showing up when there's a loop.  (Trac #69,
+	of the same CNAME from showing up when there's a loop. (Trac #69,
 	svn r2350)
     
   65.  [func]		shentingting
@@ -463,7 +486,7 @@ bind10-devel-20100602 released on June 2, 2010
 	#205, svn r1957)
 
   44.   [build]         jreed
-	Install headers for libdns and libexception.  (Trac #68,
+	Install headers for libdns and libexception. (Trac #68,
 	svn r1941)
 
   43.   [func]          jelte
@@ -471,7 +494,7 @@ bind10-devel-20100602 released on June 2, 2010
 
   42.   [func]          jelte
 	lib/python/isc/config:      Make temporary file with python
-	tempfile module instead of manual with fixed name.  (Trac
+	tempfile module instead of manual with fixed name. (Trac
 	#184, svn r1859)
 
   41.   [func]          jelte
@@ -479,7 +502,7 @@ bind10-devel-20100602 released on June 2, 2010
 
   40.   [build]         jreed
 	Report detected features and configure settings at end of
-	configure output.  (svn r1836)
+	configure output. (svn r1836)
 
   39.   [func]*         each
 	Renamed libauth to libdatasrc.
@@ -492,7 +515,7 @@ bind10-devel-20100602 released on June 2, 2010
 	(Trac #135, #151, #134, svn r1797)
 
   37.   [build]         jinmei
-	Check for the availability of python-config.  (Trac #159,
+	Check for the availability of python-config. (Trac #159,
 	svn r1794)
 
   36.	[func]		shane
@@ -537,7 +560,7 @@ bind10-devel-20100421 released on April 21, 2010
 
   27.	[build]
 	Add missing copyright license statements to various source
-	files.  (svn r1750)
+	files. (svn r1750)
 
   26.	[func]
 	Use PACKAGE_STRING (name + version) from config.h instead

README

@@ -17,8 +17,9 @@ This release includes the bind10 master process, b10-msgq message
 bus, b10-auth authoritative DNS server (with SQLite3 backend),
 b10-cmdctl remote control daemon, b10-cfgmgr configuration manager,
 b10-xfrin AXFR inbound service, b10-xfrout outgoing AXFR service,
-b10-zonemgr secondary manager, and a new libdns++ library for C++
+b10-zonemgr secondary manager, b10-stats statistics collection and
-with a python wrapper.
+reporting daemon, and a new libdns++ library for C++ with a python
+wrapper.
 
 Documentation is included and also available via the BIND 10
 website at http://bind10.isc.org/

configure.ac

@@ -2,7 +2,7 @@
 # Process this file with autoconf to produce a configure script.
 
 AC_PREREQ([2.59])
-AC_INIT(bind10-devel, 20101013, bind10-dev@isc.org)
+AC_INIT(bind10-devel, 20101201, bind10-dev@isc.org)
 AC_CONFIG_SRCDIR(README)
 AM_INIT_AUTOMAKE
 AC_CONFIG_HEADERS([config.h])
@@ -467,6 +467,7 @@ AC_CONFIG_FILES([Makefile
                  src/bin/auth/tests/Makefile
                  src/bin/auth/tests/testdata/Makefile
                  src/bin/auth/benchmarks/Makefile
+                 src/bin/recurse/Makefile
                  src/bin/xfrin/Makefile
                  src/bin/xfrin/tests/Makefile
                  src/bin/xfrout/Makefile
@@ -531,6 +532,9 @@ AC_OUTPUT([src/bin/cfgmgr/b10-cfgmgr.py
            src/bin/xfrout/xfrout.spec.pre
            src/bin/xfrout/tests/xfrout_test
            src/bin/xfrout/run_b10-xfrout.sh
+           src/bin/recurse/recurse.py
+           src/bin/recurse/recurse.spec.pre
+           src/bin/recurse/run_b10-recurse.sh
            src/bin/zonemgr/zonemgr.py
            src/bin/zonemgr/zonemgr.spec.pre
            src/bin/zonemgr/tests/zonemgr_test
@@ -573,6 +577,7 @@ AC_OUTPUT([src/bin/cfgmgr/b10-cfgmgr.py
            chmod +x src/bin/cmdctl/run_b10-cmdctl.sh
            chmod +x src/bin/xfrin/run_b10-xfrin.sh
            chmod +x src/bin/xfrout/run_b10-xfrout.sh
+           chmod +x src/bin/recurse/run_b10-recurse.sh
            chmod +x src/bin/zonemgr/run_b10-zonemgr.sh
            chmod +x src/bin/stats/tests/stats_test
            chmod +x src/bin/stats/run_b10-stats.sh

src/bin/Makefile.am

@@ -1,4 +1,4 @@
 SUBDIRS = bind10 bindctl cfgmgr loadzone msgq host cmdctl auth xfrin xfrout \
-	usermgr zonemgr stats tests
+	usermgr zonemgr stats tests recurse
 
 check-recursive: all-recursive

src/bin/auth/Makefile.am

@@ -56,6 +56,7 @@ libasio_link_a_CPPFLAGS = $(AM_CPPFLAGS)
 BUILT_SOURCES = spec_config.h 
 pkglibexec_PROGRAMS = b10-auth
 b10_auth_SOURCES = auth_srv.cc auth_srv.h
+b10_auth_SOURCES += query.cc query.h
 b10_auth_SOURCES += change_user.cc change_user.h
 b10_auth_SOURCES += common.h
 b10_auth_SOURCES += main.cc

src/bin/auth/auth_srv.h

@@ -199,6 +199,7 @@ public:
     /// is shutdown.
     ///
     void setXfrinSession(isc::cc::AbstractSession* xfrin_session);
+
 private:
     AuthSrvImpl* impl_;
 };

src/bin/auth/query.cc

@@ -0,0 +1,42 @@
+// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or 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 ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC 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.
+
+#include <dns/message.h>
+#include <dns/rcode.h>
+
+#include <datasrc/zonetable.h>
+
+#include <auth/query.h>
+
+using namespace isc::dns;
+using namespace isc::datasrc;
+
+namespace isc {
+namespace auth {
+void
+Query::process() const {
+    const ZoneTable::FindResult result = zone_table_.find(qname_);
+
+    if (result.code != ZoneTable::SUCCESS &&
+        result.code != ZoneTable::PARTIALMATCH) {
+        response_.setRcode(Rcode::SERVFAIL());
+        return;
+    }
+
+    // Right now we have no code to search the zone, so we simply return
+    // NXDOMAIN for tests.
+    response_.setRcode(Rcode::NXDOMAIN());
+}
+}
+}

src/bin/auth/query.h

@@ -0,0 +1,119 @@
+/*
+ * Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+ *
+ * Permission to use, copy, modify, and/or 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 ISC DISCLAIMS ALL WARRANTIES WITH
+ * REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS.  IN NO EVENT SHALL ISC 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.
+ */
+
+namespace isc {
+namespace dns {
+class Message;
+class Name;
+class RRType;
+}
+
+namespace datasrc {
+class ZoneTable;
+}
+
+namespace auth {
+
+/// The \c Query class represents a standard DNS query that encapsulates
+/// processing logic to answer the query.
+///
+/// Many of the design details for this class are still in flux.
+/// We'll revisit and update them as we add more functionality, for example:
+/// - zone_table parameter of the constructor.  This will eventually be
+///   replaced with a generic DataSrc object, or perhaps a notion of "view".
+/// - as a related point, we may have to pass the RR class of the query.
+///   in the initial implementation the RR class is an attribute of zone
+///   table and omitted.  It's not clear if this assumption holds with
+///   generic data sources.  On the other hand, it will help keep
+///   implementation simpler, and we might rather want to modify the design
+///   of the data source on this point.
+/// - return value of process().  rather than or in addition to setting the
+///   Rcode, we might use it as a return value of \c process().
+/// - we'll have to be able to specify whether DNSSEC is requested.
+///   It's an open question whether it should be in the constructor or via a
+///   separate attribute setter.
+/// - likewise, we'll eventually need to do per zone access control, for which
+///   we need querier's information such as its IP address.
+/// - zone_table (or DataSrc eventually) and response may better be parameters
+///   to process() instead of the constructor.
+///
+/// <b>Note:</b> The class name is intentionally the same as the one used in
+/// the datasrc library.  This is because the plan is to eventually merge
+/// the two classes.  We could give it a different name such as "AuthQuery"
+/// to avoid possible ambiguity, but it may sound redundant in that it's
+/// obvious that this class is for authoritative queries.
+/// Since the interfaces are very different for now and it's less
+/// likely to misuse one of the classes instead of the other
+/// accidentally, and since it's considered a temporary development state,
+/// we keep this name at the moment.
+class Query {
+public:
+    /// Constructor from query parameters.
+    ///
+    /// This constructor never throws an exception.
+    ///
+    /// \param zone_table The zone table wherein the answer to the query is
+    /// to be found.
+    /// \param qname The query name
+    /// \param qtype The RR type of the query
+    /// \param response The response message to store the answer to the query.
+    Query(const isc::datasrc::ZoneTable& zone_table,
+          const isc::dns::Name& qname, const isc::dns::RRType& qtype,
+          isc::dns::Message& response) :
+        zone_table_(zone_table), qname_(qname), qtype_(qtype),
+        response_(response)
+    {}
+
+    /// Process the query.
+    ///
+    /// This method first identifies the zone that best matches the query
+    /// name (and in some cases RR type when the search is dependent on the
+    /// type) and then searches the zone for an entry that best matches the
+    /// query name.
+    /// It then updates the response message accordingly; for example, a
+    /// successful search would result in adding a corresponding RRset to
+    /// the answer section of the response.
+    ///
+    /// If no matching zone is found in the zone table, the RCODE of
+    /// SERVFAIL will be set in the response.
+    /// <b>Note:</b> this is different from the error code that BIND 9 returns
+    /// by default when it's configured as an authoritative-only server (and
+    /// from the behavior of the BIND 10 datasrc library, which was implemented
+    /// to be compatible with BIND 9).
+    /// The difference comes from the fact that BIND 9 returns REFUSED as a
+    /// result of access control check on the use of its cache.
+    /// Since BIND 10's authoritative server doesn't have the notion of cache
+    /// by design, it doesn't make sense to return REFUSED.  On the other hand,
+    /// providing compatible behavior may have its own benefit, so this point
+    /// should be revisited later.
+    ///
+    /// Right now this method never throws an exception, but it may in a
+    /// future version.
+    void process() const;
+
+private:
+    const isc::datasrc::ZoneTable& zone_table_;
+    const isc::dns::Name& qname_;
+    const isc::dns::RRType& qtype_;
+    isc::dns::Message& response_;
+};
+
+}
+}
+
+// Local Variables:
+// mode: c++
+// End:

src/bin/auth/tests/Makefile.am

@@ -21,8 +21,10 @@ TESTS += run_unittests
 run_unittests_SOURCES = $(top_srcdir)/src/lib/dns/tests/unittest_util.h
 run_unittests_SOURCES += $(top_srcdir)/src/lib/dns/tests/unittest_util.cc
 run_unittests_SOURCES += ../auth_srv.h ../auth_srv.cc
+run_unittests_SOURCES += ../query.h ../query.cc
 run_unittests_SOURCES += ../change_user.h ../change_user.cc
 run_unittests_SOURCES += auth_srv_unittest.cc
+run_unittests_SOURCES += query_unittest.cc
 run_unittests_SOURCES += change_user_unittest.cc
 run_unittests_SOURCES += asio_link_unittest.cc
 run_unittests_SOURCES += run_unittests.cc

src/bin/auth/tests/query_unittest.cc

@@ -0,0 +1,70 @@
+// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or 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 ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC 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.
+
+#include <dns/message.h>
+#include <dns/name.h>
+#include <dns/rcode.h>
+#include <dns/rrtype.h>
+
+#include <datasrc/zonetable.h>
+
+#include <auth/query.h>
+
+#include <gtest/gtest.h>
+
+using namespace isc::dns;
+using namespace isc::datasrc;
+using namespace isc::auth;
+
+namespace {
+class QueryTest : public ::testing::Test {
+protected:
+    QueryTest() :
+        qname(Name("www.example.com")), qclass(RRClass::IN()),
+        qtype(RRType::A()), response(Message::RENDER),
+        query(zone_table, qname, qtype, response)
+    {
+        response.setRcode(Rcode::NOERROR());
+    }
+    ZoneTable zone_table;
+    const Name qname;
+    const RRClass qclass;
+    const RRType qtype;
+    Message response;
+    Query query;
+};
+
+TEST_F(QueryTest, noZone) {
+    // There's no zone in the zone table.  So the response should have
+    // SERVFAIL.
+    query.process();
+    EXPECT_EQ(Rcode::SERVFAIL(), response.getRcode());
+}
+
+TEST_F(QueryTest, matchZone) {
+    // add a matching zone.  since the zone is empty right now, the response
+    // should have NXDOMAIN.
+    zone_table.add(ZonePtr(new MemoryZone(qclass, Name("example.com"))));
+    query.process();
+    EXPECT_EQ(Rcode::NXDOMAIN(), response.getRcode());
+}
+
+TEST_F(QueryTest, noMatchZone) {
+    // there's a zone in the table but it doesn't match the qname.  should
+    // result in SERVFAIL.
+    zone_table.add(ZonePtr(new MemoryZone(qclass, Name("example.org"))));
+    query.process();
+    EXPECT_EQ(Rcode::SERVFAIL(), response.getRcode());
+}
+}

src/bin/bind10/bind10.py.in

@@ -15,7 +15,7 @@
 # NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
 # WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
-"""\
+"""
 This file implements the Boss of Bind (BoB, or bob) program.
 
 Its purpose is to start up the BIND 10 system, and then manage the
@@ -72,7 +72,7 @@ isc.util.process.rename(sys.argv[0])
 # This is the version that gets displayed to the user.
 # The VERSION string consists of the module name, the module version
 # number, and the overall BIND 10 version number (set in configure.ac).
-VERSION = "bind10 20100916 (BIND 10 @PACKAGE_VERSION@)"
+VERSION = "bind10 20101129 (BIND 10 @PACKAGE_VERSION@)"
 
 # This is for bind10.boottime of stats module
 _BASETIME = time.gmtime()
@@ -189,137 +189,233 @@ class ProcessInfo:
     def respawn(self):
         self._spawn()
 
+class CChannelConnectError(Exception): pass
+
 class BoB:
     """Boss of BIND class."""
     
     def __init__(self, msgq_socket_file=None, auth_port=5300, address=None,
                  nocache=False, verbose=False, setuid=None, username=None):
-        """Initialize the Boss of BIND. This is a singleton (only one
+        """
-        can run).
+            Initialize the Boss of BIND. This is a singleton (only one can run).
         
-        The msgq_socket_file specifies the UNIX domain socket file
+            The msgq_socket_file specifies the UNIX domain socket file that the
-        that the msgq process listens on.
+            msgq process listens on.  If verbose is True, then the boss reports
-        If verbose is True, then the boss reports what it is doing.
+            what it is doing.
         """
-        self.verbose = verbose
+        self.address = address
-        self.msgq_socket_file = msgq_socket_file
         self.auth_port = auth_port
-        self.address = None
-        if address:
-            self.address = address
         self.cc_session = None
         self.ccs = None
-        self.processes = {}
+        self.cfg_start_auth = True
+        self.cfg_start_recurse = False
+        self.curproc = None
         self.dead_processes = {}
+        self.msgq_socket_file = msgq_socket_file
+        self.nocache = nocache
+        self.processes = {}
         self.runnable = False
         self.uid = setuid
         self.username = username
-        self.nocache = nocache
+        self.verbose = verbose
 
     def config_handler(self, new_config):
         if self.verbose:
-            sys.stdout.write("[bind10] handling new config:\n")
+            sys.stdout.write("[bind10] Handling new configuration: " +
-            sys.stdout.write(new_config + "\n")
+                str(new_config) + "\n")
         answer = isc.config.ccsession.create_answer(0)
         return answer
         # TODO
 
     def command_handler(self, command, args):
         if self.verbose:
-            sys.stdout.write("[bind10] Boss got command:\n")
+            sys.stdout.write("[bind10] Boss got command: " + command + "\n")
-            sys.stdout.write(command + "\n")
         answer = isc.config.ccsession.create_answer(1, "command not implemented")
         if type(command) != str:
             answer = isc.config.ccsession.create_answer(1, "bad command")
         else:
-            cmd = command
+            if command == "shutdown":
-            if cmd == "shutdown":
-                sys.stdout.write("[bind10] got shutdown command\n")
                 self.runnable = False
                 answer = isc.config.ccsession.create_answer(0)
             else:
                 answer = isc.config.ccsession.create_answer(1, 
                                                             "Unknown command")
         return answer
-    
+
-    def startup(self):
+    def kill_started_processes(self):
-        """Start the BoB instance.
+        """
- 
+            Called as part of the exception handling when a process fails to
-        Returns None if successful, otherwise an string describing the
+            start, this runs through the list of started processes, killing
-        problem.
+            each one.  It then clears that list.
         """
-        # try to connect to the c-channel daemon, 
-        # to see if it is already running
-        c_channel_env = {}
-        if self.msgq_socket_file is not None:
-             c_channel_env["BIND10_MSGQ_SOCKET_FILE"] = self.msgq_socket_file 
         if self.verbose:
-            sys.stdout.write("[bind10] Checking for already running b10-msgq\n")
+            sys.stdout.write("[bind10] killing started processes:\n")
-        # try to connect, and if we can't wait a short while
+
-        try:
+        for pid in self.processes:
-            self.cc_session = isc.cc.Session(self.msgq_socket_file)
+            if self.verbose:
-            return "b10-msgq already running, or socket file not cleaned , cannot start"
+                sys.stdout.write("[bind10] - %s\n" % self.processes[pid].name)
-        except isc.cc.session.SessionError:
+            self.processes[pid].process.kill()
-            # this is the case we want, where the msgq is not running
+        self.processes = {}
-            pass
 
-        # start the c-channel daemon
+    def read_bind10_config(self):
+        """
+            Reads the parameters associated with the BoB module itself.
+
+            At present these are the components to start although arguably this
+            information should be in the configuration for the appropriate
+            module itself. (However, this would cause difficulty in the case of
+            xfrin/xfrout and zone manager as we don't need to start those if we
+            are not running the authoritative server.)
+        """
         if self.verbose:
-            if self.msgq_socket_file:
+            sys.stdout.write("[bind10] Reading Boss configuration:\n")
-                sys.stdout.write("[bind10] Starting b10-msgq\n")
+
-        try:
+        config_data = self.ccs.get_full_config()
-            c_channel = ProcessInfo("b10-msgq", ["b10-msgq"], c_channel_env,
+        self.cfg_start_auth = config_data.get("start_auth")
-                                    True, not self.verbose, uid=self.uid,
+        self.cfg_start_recurse = config_data.get("start_recurse")
-                                    username=self.username)
+
-        except Exception as e:
-            return "Unable to start b10-msgq; " + str(e)
-        self.processes[c_channel.pid] = c_channel
         if self.verbose:
-            sys.stdout.write("[bind10] Started b10-msgq (PID %d)\n" % 
+            sys.stdout.write("[bind10] - start_auth: %s\n" %
-                             c_channel.pid)
+                str(self.cfg_start_auth))
+            sys.stdout.write("[bind10] - start_recurse: %s\n" %
+                str(self.cfg_start_recurse))
+
+    def log_starting(self, process, port = None, address = None):
+        """
+            A convenience function to output a "Starting xxx" message if the
+            verbose option is set.  Putting this into a separate method ensures
+            that the output form is consistent across all processes.
+
+            The process name (passed as the first argument) is put into
+            self.curproc, and is used to indicate which process failed to
+            start if there is an error (and is used in the "Started" message
+            on success).  The optional port and address information are
+            appended to the message (if present).
+        """
+        self.curproc = process
+        if self.verbose:
+            sys.stdout.write("[bind10] Starting %s" % self.curproc)
+            if port is not None:
+                sys.stdout.write(" on port %d" % port)
+                if address is not None:
+                    sys.stdout.write(" (address %s)" % str(address))
+            sys.stdout.write("\n")
 
-        # now connect to the c-channel
+    def log_started(self, pid = None):
+        """
+            A convenience function to output a 'Started xxxx (PID yyyy)'
+            message.  As with starting_message(), this ensures a consistent
+            format.
+        """
+        if self.verbose:
+            sys.stdout.write("[bind10] Started %s" % self.curproc)
+            if pid is not None:
+                sys.stdout.write(" (PID %d)" % pid)
+            sys.stdout.write("\n")
+
+    # The next few methods start the individual processes of BIND-10.  They
+    # are called via start_all_process().  If any fail, an exception is raised
+    # which is caught by the caller of start_all_processes(); this kills
+    # processes started up to that point before terminating the program.
+
+    def start_msgq(self, c_channel_env):
+        """
+            Start the message queue and connect to the command channel.
+        """
+        self.log_starting("b10-msgq")
+        c_channel = ProcessInfo("b10-msgq", ["b10-msgq"], c_channel_env,
+                                True, not self.verbose, uid=self.uid,
+                                username=self.username)
+        self.processes[c_channel.pid] = c_channel
+        self.log_started(c_channel.pid)
+
+        # Now connect to the c-channel
         cc_connect_start = time.time()
         while self.cc_session is None:
             # if we have been trying for "a while" give up
             if (time.time() - cc_connect_start) > 5:
-                c_channel.process.kill()
+                raise CChannelConnectError("Unable to connect to c-channel after 5 seconds")
-                return "Unable to connect to c-channel after 5 seconds"
+
             # try to connect, and if we can't wait a short while
             try:
                 self.cc_session = isc.cc.Session(self.msgq_socket_file)
             except isc.cc.session.SessionError:
                 time.sleep(0.1)
 
-        # start the configuration manager
+    def start_cfgmgr(self, c_channel_env):
-        if self.verbose:
+        """
-            sys.stdout.write("[bind10] Starting b10-cfgmgr\n")
+            Starts the configuration manager process
-        try:
+        """
-            bind_cfgd = ProcessInfo("b10-cfgmgr", ["b10-cfgmgr"],
+        self.log_starting("b10-cfgmgr")
-                                    c_channel_env, uid=self.uid,
+        bind_cfgd = ProcessInfo("b10-cfgmgr", ["b10-cfgmgr"],
-                                    username=self.username)
+                                c_channel_env, uid=self.uid,
-        except Exception as e:
+                                username=self.username)
-            c_channel.process.kill()
-            return "Unable to start b10-cfgmgr; " + str(e)
         self.processes[bind_cfgd.pid] = bind_cfgd
-        if self.verbose:
+        self.log_started(bind_cfgd.pid)
-            sys.stdout.write("[bind10] Started b10-cfgmgr (PID %d)\n" % 
-                             bind_cfgd.pid)
 
         # sleep until b10-cfgmgr is fully up and running, this is a good place
         # to have a (short) timeout on synchronized groupsend/receive
         # TODO: replace the sleep by a listen for ConfigManager started
         # message
         time.sleep(1)
-        if self.verbose:
+
-            sys.stdout.write("[bind10] starting ccsession\n")
+    def start_ccsession(self, c_channel_env):
+        """
+            Start the CC Session
+
+            The argument c_channel_env is unused but is supplied to keep the
+            argument list the same for all start_xxx methods.
+        """
+        self.log_starting("ccsession")
         self.ccs = isc.config.ModuleCCSession(SPECFILE_LOCATION, 
                                       self.config_handler, self.command_handler)
         self.ccs.start()
+        self.log_started()
+
+    # A couple of utility methods for starting processes...
+
+    def start_process(self, name, args, c_channel_env, port=None, address=None):
+        """
+            Given a set of command arguments, start the process and output
+            appropriate log messages.  If the start is successful, the process
+            is added to the list of started processes.
+
+            The port and address arguments are for log messages only.
+        """
+        self.log_starting(name, port, address)
+        newproc = ProcessInfo(name, args, c_channel_env)
+        self.processes[newproc.pid] = newproc
+        self.log_started(newproc.pid)
+
+    def start_simple(self, name, c_channel_env, port=None, address=None):
+        """
+            Most of the BIND-10 processes are started with the command:
+
+                <process-name> [-v]
+
+            ... where -v is appended if verbose is enabled.  This method
+            generates the arguments from the name and starts the process.
+
+            The port and address arguments are for log messages only.
+        """
+        # Set up the command arguments.
+        args = [name]
         if self.verbose:
-            sys.stdout.write("[bind10] ccsession started\n")
+            args += ['-v']
 
-        # start b10-auth
+        # ... and start the process
+        self.start_process(name, args, c_channel_env, port, address)
+
+    # The next few methods start up the rest of the BIND-10 processes.
+    # Although many of these methods are little more than a call to
+    # start_simple, they are retained (a) for testing reasons and (b) as a place
+    # where modifications can be made if the process start-up sequence changes
+    # for a given process.
+
+    def start_auth(self, c_channel_env):
+        """
+            Start the Authoritative server
+        """
         # XXX: this must be read from the configuration manager in the future
         authargs = ['b10-auth', '-p', str(self.auth_port)]
         if self.address:
@@ -330,130 +426,115 @@ class BoB:
             authargs += ['-u', str(self.uid)]
         if self.verbose:
             authargs += ['-v']
-            sys.stdout.write("Starting b10-auth using port %d" %
-                             self.auth_port)
-            if self.address:
-                sys.stdout.write(" on %s" % str(self.address))
-            sys.stdout.write("\n")
-        try:
-            auth = ProcessInfo("b10-auth", authargs,
-                               c_channel_env)
-        except Exception as e:
-            c_channel.process.kill()
-            bind_cfgd.process.kill()
-            xfrout.process.kill()
-            return "Unable to start b10-auth; " + str(e)
-        self.processes[auth.pid] = auth
-        if self.verbose:
-            sys.stdout.write("[bind10] Started b10-auth (PID %d)\n" % auth.pid)
 
-        # everything after the authoritative server can run as non-root
+        # ... and start
-        if self.uid is not None:
+        self.start_process("b10-auth", authargs, c_channel_env,
-            posix.setuid(self.uid)
+            self.auth_port, self.address)
 
-        # start the xfrout before auth-server, to make sure every xfr-query can
+    def start_recurse(self, c_channel_env):
-        # be processed properly.
+        """
-        xfrout_args = ['b10-xfrout']
+            Start the Resolver.  At present, all these arguments and switches
-        if self.verbose:
+            are pure speculation.  As with the auth daemon, they should be
-            sys.stdout.write("[bind10] Starting b10-xfrout\n")
+            read from the configuration database.
-            xfrout_args += ['-v']
+        """
-        try:
+        self.curproc = "b10-recurse"
-            xfrout = ProcessInfo("b10-xfrout", xfrout_args, 
+        # XXX: this must be read from the configuration manager in the future
-                                 c_channel_env )
+        resargs = ['b10-recurse']
-        except Exception as e:
+        if self.uid:
-            c_channel.process.kill()
+            resargs += ['-u', str(self.uid)]
-            bind_cfgd.process.kill()
-            return "Unable to start b10-xfrout; " + str(e)
-        self.processes[xfrout.pid] = xfrout
         if self.verbose:
-            sys.stdout.write("[bind10] Started b10-xfrout (PID %d)\n" % 
+            resargs += ['-v']
-                             xfrout.pid)
 
-        # start b10-xfrin
+        # ... and start
-        xfrin_args = ['b10-xfrin']
+        self.start_process("b10-recurse", resargs, c_channel_env)
-        if self.verbose:
-            sys.stdout.write("[bind10] Starting b10-xfrin\n")
-            xfrin_args += ['-v']
-        try:
-            xfrind = ProcessInfo("b10-xfrin", xfrin_args,
-                                 c_channel_env)
-        except Exception as e:
-            c_channel.process.kill()
-            bind_cfgd.process.kill()
-            xfrout.process.kill()
-            auth.process.kill()
-            return "Unable to start b10-xfrin; " + str(e)
-        self.processes[xfrind.pid] = xfrind
-        if self.verbose:
-            sys.stdout.write("[bind10] Started b10-xfrin (PID %d)\n" % 
-                             xfrind.pid)
 
-        # start b10-zonemgr
+    def start_xfrout(self, c_channel_env):
-        zonemgr_args = ['b10-zonemgr']
+        self.start_simple("b10-xfrout", c_channel_env)
-        if self.verbose:
-            sys.stdout.write("[bind10] Starting b10-zonemgr\n")
-            zonemgr_args += ['-v']
-        try:
-            zonemgr = ProcessInfo("b10-zonemgr", zonemgr_args,
-                                 c_channel_env)
-        except Exception as e:
-            c_channel.process.kill()
-            bind_cfgd.process.kill()
-            xfrout.process.kill()
-            auth.process.kill()
-            xfrind.process.kill()
-            return "Unable to start b10-zonemgr; " + str(e)
-        self.processes[zonemgr.pid] = zonemgr 
-        if self.verbose:
-            sys.stdout.write("[bind10] Started b10-zonemgr(PID %d)\n" % 
-                             zonemgr.pid)
 
-        # start b10-stats
+    def start_xfrin(self, c_channel_env):
-        stats_args = ['b10-stats']
+        self.start_simple("b10-xfrin", c_channel_env)
-        if self.verbose:
-            sys.stdout.write("[bind10] Starting b10-stats\n")
-            stats_args += ['-v']
-        try:
-            statsd = ProcessInfo("b10-stats", stats_args,
-                                 c_channel_env)
-        except Exception as e:
-            c_channel.process.kill()
-            bind_cfgd.process.kill()
-            xfrout.process.kill()
-            auth.process.kill()
-            xfrind.process.kill()
-            zonemgr.process.kill()
-            return "Unable to start b10-stats; " + str(e)
-
-        self.processes[statsd.pid] = statsd
-        if self.verbose:
-            sys.stdout.write("[bind10] Started b10-stats (PID %d)\n" % statsd.pid)
 
-        # start the b10-cmdctl
+    def start_zonemgr(self, c_channel_env):
+        self.start_simple("b10-zonemgr", c_channel_env)
+
+    def start_stats(self, c_channel_env):
+        self.start_simple("b10-stats", c_channel_env)
+
+    def start_cmdctl(self, c_channel_env):
         # XXX: we hardcode port 8080
-        cmdctl_args = ['b10-cmdctl']
+        self.start_simple("b10-cmdctl", c_channel_env, 8080)
+
+    def start_all_processes(self, c_channel_env):
+        """
+            Starts up all the processes.  Any exception generated during the
+            starting of the processes is handled by the caller.
+        """
+        self.start_msgq(c_channel_env)
+        self.start_cfgmgr(c_channel_env)
+        self.start_ccsession(c_channel_env)
+
+        # Extract the parameters associated with Bob.  This can only be
+        # done after the CC Session is started.
+        self.read_bind10_config()
+
+        # Continue starting the processes.  The authoritative server (if
+        # selected):
+        if self.cfg_start_auth:
+            self.start_auth(c_channel_env)
+
+        # ... and resolver (if selected):
+        if self.cfg_start_recurse:
+            self.start_recurse(c_channel_env)
+
+        # Everything after the main components can run as non-root.
+        # TODO: this is only temporary - once the privileged socket creator is
+        # fully working, nothing else will run as root.
+        if self.uid is not None:
+            posix.setuid(self.uid)
+
+        # xfrin/xfrout and the zone manager are only meaningful if the
+        # authoritative server has been started.
+        if self.cfg_start_auth:
+            self.start_xfrout(c_channel_env)
+            self.start_xfrin(c_channel_env)
+            self.start_zonemgr(c_channel_env)
+
+        # ... and finally start the remaining processes
+        self.start_stats(c_channel_env)
+        self.start_cmdctl(c_channel_env)
+    
+    def startup(self):
+        """
+            Start the BoB instance.
+ 
+            Returns None if successful, otherwise an string describing the
+            problem.
+        """
+        # Try to connect to the c-channel daemon, to see if it is already
+        # running
+        c_channel_env = {}
+        if self.msgq_socket_file is not None:
+             c_channel_env["BIND10_MSGQ_SOCKET_FILE"] = self.msgq_socket_file 
         if self.verbose:
-            sys.stdout.write("[bind10] Starting b10-cmdctl on port 8080\n")
+           sys.stdout.write("[bind10] Checking for already running b10-msgq\n")
-            cmdctl_args += ['-v']
+        # try to connect, and if we can't wait a short while
+        try:
+            self.cc_session = isc.cc.Session(self.msgq_socket_file)
+            return "b10-msgq already running, or socket file not cleaned , cannot start"
+        except isc.cc.session.SessionError:
+            # this is the case we want, where the msgq is not running
+            pass
+
+        # Start all processes.  If any one fails to start, kill all started
+        # processes and exit with an error indication.
         try:
-            cmd_ctrld = ProcessInfo("b10-cmdctl", cmdctl_args,
+            self.start_all_processes(c_channel_env)
-                                    c_channel_env)
         except Exception as e:
-            c_channel.process.kill()
+            self.kill_started_processes()
-            bind_cfgd.process.kill()
+            return "Unable to start " + self.curproc + ": " + str(e)
-            xfrout.process.kill()
-            auth.process.kill()
-            xfrind.process.kill()
-            zonemgr.process.kill()
-            statsd.process.kill()
-            return "Unable to start b10-cmdctl; " + str(e)
-        self.processes[cmd_ctrld.pid] = cmd_ctrld
-        if self.verbose:
-            sys.stdout.write("[bind10] Started b10-cmdctl (PID %d)\n" % 
-                             cmd_ctrld.pid)
 
+        # Started successfully
         self.runnable = True
-
         return None
 
     def stop_all_processes(self):
@@ -462,6 +543,7 @@ class BoB:
         self.cc_session.group_sendmsg(cmd, 'Cmdctl', 'Cmdctl')
         self.cc_session.group_sendmsg(cmd, "ConfigManager", "ConfigManager")
         self.cc_session.group_sendmsg(cmd, "Auth", "Auth")
+        self.cc_session.group_sendmsg(cmd, "Recurse", "Recurse")
         self.cc_session.group_sendmsg(cmd, "Xfrout", "Xfrout")
         self.cc_session.group_sendmsg(cmd, "Xfrin", "Xfrin")
         self.cc_session.group_sendmsg(cmd, "Zonemgr", "Zonemgr")
@@ -642,11 +724,10 @@ def main():
     # Enforce line buffering on stdout, even when not a TTY
     sys.stdout = io.TextIOWrapper(sys.stdout.detach(), line_buffering=True)
 
-
     # Parse any command-line options.
     parser = OptionParser(version=VERSION)
     parser.add_option("-a", "--address", dest="address", type="string",
-                      action="callback", callback=check_addr, default='',
+                      action="callback", callback=check_addr, default=None,
                       help="address the b10-auth daemon will use (default: listen on all addresses)")
     parser.add_option("-m", "--msgq-socket-file", dest="msgq_socket_file",
                       type="string", default=None,
@@ -781,6 +862,7 @@ def main():
     # shutdown
     signal.signal(signal.SIGCHLD, signal.SIG_DFL)
     boss_of_bind.shutdown()
+    sys.stdout.write("[bind10] BIND 10 exiting\n");
     sys.exit(0)
 
 if __name__ == "__main__":

src/bin/bind10/bob.spec

@@ -3,6 +3,18 @@
     "module_name": "Boss",
     "module_description": "Master process",
     "config_data": [
+      {
+        "item_name": "start_auth",
+        "item_type": "boolean",
+        "item_optional": false,
+        "item_default": true
+      },
+      {
+        "item_name": "start_recurse",
+        "item_type": "boolean",
+        "item_optional": false,
+        "item_default": false
+      }
     ],
     "commands": [
       {

src/bin/bind10/run_bind10.sh.in

@@ -20,7 +20,7 @@ export PYTHON_EXEC
 
 BIND10_PATH=@abs_top_builddir@/src/bin/bind10
 
-PATH=@abs_top_builddir@/src/bin/msgq:@abs_top_builddir@/src/bin/auth:@abs_top_builddir@/src/bin/cfgmgr:@abs_top_builddir@/src/bin/cmdctl:@abs_top_builddir@/src/bin/stats:@abs_top_builddir@/src/bin/xfrin:@abs_top_builddir@/src/bin/xfrout:@abs_top_builddir@/src/bin/zonemgr:$PATH
+PATH=@abs_top_builddir@/src/bin/msgq:@abs_top_builddir@/src/bin/auth:@abs_top_builddir@/src/bin/cfgmgr:@abs_top_builddir@/src/bin/cmdctl:@abs_top_builddir@/src/bin/stats:@abs_top_builddir@/src/bin/xfrin:@abs_top_builddir@/src/bin/xfrout:@abs_top_builddir@/src/bin/zonemgr:@abs_top_builddir@/src/bin/recurse:$PATH
 export PATH
 
 PYTHONPATH=@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/lib/dns/python/.libs:@abs_top_builddir@/src/lib/xfr/.libs

src/bin/bind10/tests/bind10_test.py

@@ -79,43 +79,237 @@ class TestBoB(unittest.TestCase):
         self.assertEqual(bob.verbose, False)
         self.assertEqual(bob.msgq_socket_file, None)
         self.assertEqual(bob.auth_port, 5300)
-        self.assertEqual(bob.cc_session, None)
         self.assertEqual(bob.address, None)
+        self.assertEqual(bob.cc_session, None)
+        self.assertEqual(bob.ccs, None)
         self.assertEqual(bob.processes, {})
         self.assertEqual(bob.dead_processes, {})
         self.assertEqual(bob.runnable, False)
+        self.assertEqual(bob.uid, None)
+        self.assertEqual(bob.username, None)
+        self.assertEqual(bob.nocache, False)
+        self.assertEqual(bob.cfg_start_auth, True)
+        self.assertEqual(bob.cfg_start_recurse, False)
 
     def test_init_alternate_socket(self):
         bob = BoB("alt_socket_file")
         self.assertEqual(bob.verbose, False)
         self.assertEqual(bob.msgq_socket_file, "alt_socket_file")
+        self.assertEqual(bob.auth_port, 5300)
+        self.assertEqual(bob.address, None)
         self.assertEqual(bob.cc_session, None)
+        self.assertEqual(bob.ccs, None)
         self.assertEqual(bob.processes, {})
         self.assertEqual(bob.dead_processes, {})
         self.assertEqual(bob.runnable, False)
+        self.assertEqual(bob.uid, None)
+        self.assertEqual(bob.username, None)
+        self.assertEqual(bob.nocache, False)
+        self.assertEqual(bob.cfg_start_auth, True)
+        self.assertEqual(bob.cfg_start_recurse, False)
 
     def test_init_alternate_auth_port(self):
         bob = BoB(None, 9999)
         self.assertEqual(bob.verbose, False)
         self.assertEqual(bob.msgq_socket_file, None)
         self.assertEqual(bob.auth_port, 9999)
-        self.assertEqual(bob.cc_session, None)
         self.assertEqual(bob.address, None)
+        self.assertEqual(bob.cc_session, None)
+        self.assertEqual(bob.ccs, None)
         self.assertEqual(bob.processes, {})
         self.assertEqual(bob.dead_processes, {})
         self.assertEqual(bob.runnable, False)
+        self.assertEqual(bob.uid, None)
+        self.assertEqual(bob.username, None)
+        self.assertEqual(bob.nocache, False)
+        self.assertEqual(bob.cfg_start_auth, True)
+        self.assertEqual(bob.cfg_start_recurse, False)
 
     def test_init_alternate_address(self):
-        bob = BoB(None, 5300, IPAddr('127.127.127.127'))
+        bob = BoB(None, 1234, IPAddr('127.127.127.127'))
         self.assertEqual(bob.verbose, False)
-        self.assertEqual(bob.auth_port, 5300)
         self.assertEqual(bob.msgq_socket_file, None)
-        self.assertEqual(bob.cc_session, None)
+        self.assertEqual(bob.auth_port, 1234)
         self.assertEqual(bob.address.addr, socket.inet_aton('127.127.127.127'))
+        self.assertEqual(bob.cc_session, None)
+        self.assertEqual(bob.ccs, None)
         self.assertEqual(bob.processes, {})
         self.assertEqual(bob.dead_processes, {})
         self.assertEqual(bob.runnable, False)
-    # verbose testing...
+        self.assertEqual(bob.uid, None)
+        self.assertEqual(bob.username, None)
+        self.assertEqual(bob.nocache, False)
+        self.assertEqual(bob.cfg_start_auth, True)
+        self.assertEqual(bob.cfg_start_recurse, False)
+
+# Class for testing the Bob.start_all_processes() method call.
+#
+# Although testing that external processes start is outside the scope
+# of the unit test, by overriding the process start methods we can check
+# that the right processes are started depending on the configuration
+# options.
+class StartAllProcessesBob(BoB):
+    def __init__(self):
+        BoB.__init__(self)
+
+# Set flags as to which of the overridden methods has been run.
+        self.msgq = False
+        self.cfgmgr = False
+        self.ccsession = False
+        self.auth = False
+        self.recurse = False
+        self.xfrout = False
+        self.xfrin = False
+        self.zonemgr = False
+        self.stats = False
+        self.cmdctl = False
+
+    def read_bind10_config(self):
+        # Configuration options are set directly
+        pass
+
+    def start_msgq(self, c_channel_env):
+        self.msgq = True
+
+    def start_cfgmgr(self, c_channel_env):
+        self.cfgmgr = True
+
+    def start_ccsession(self, c_channel_env):
+        self.ccsession = True
+
+    def start_auth(self, c_channel_env):
+        self.auth = True
+
+    def start_recurse(self, c_channel_env):
+        self.recurse = True
+
+    def start_xfrout(self, c_channel_env):
+        self.xfrout = True
+
+    def start_xfrin(self, c_channel_env):
+        self.xfrin = True
+
+    def start_zonemgr(self, c_channel_env):
+        self.zonemgr = True
+
+    def start_stats(self, c_channel_env):
+        self.stats = True
+
+    def start_cmdctl(self, c_channel_env):
+        self.cmdctl = True
+
+# Check that the start_all_processes method starts the right combination
+# of processes.
+class TestStartAllProcessesBob(unittest.TestCase):
+    def check_preconditions(self, bob):
+        self.assertEqual(bob.msgq, False)
+        self.assertEqual(bob.cfgmgr, False)
+        self.assertEqual(bob.ccsession, False)
+        self.assertEqual(bob.auth, False)
+        self.assertEqual(bob.recurse, False)
+        self.assertEqual(bob.xfrout, False)
+        self.assertEqual(bob.xfrin, False)
+        self.assertEqual(bob.zonemgr, False)
+        self.assertEqual(bob.stats, False)
+        self.assertEqual(bob.cmdctl, False)
+
+    # Checks the processes started when starting neither auth nor recurse
+    # is specified.
+    def test_start_none(self):
+        # Created Bob and ensure initialization correct
+        bob = StartAllProcessesBob()
+        self.check_preconditions(bob)
+
+        # Start processes and check what was started
+        c_channel_env = {}
+        bob.cfg_start_auth = False
+        bob.cfg_start_recurse = False
+
+        bob.start_all_processes(c_channel_env)
+
+        self.assertEqual(bob.msgq, True)
+        self.assertEqual(bob.cfgmgr, True)
+        self.assertEqual(bob.ccsession, True)
+        self.assertEqual(bob.auth, False)
+        self.assertEqual(bob.recurse, False)
+        self.assertEqual(bob.xfrout, False)
+        self.assertEqual(bob.xfrin, False)
+        self.assertEqual(bob.zonemgr, False)
+        self.assertEqual(bob.stats, True)
+        self.assertEqual(bob.cmdctl, True)
+
+    # Checks the processes started when starting only the auth process
+    def test_start_auth(self):
+        # Created Bob and ensure initialization correct
+        bob = StartAllProcessesBob()
+        self.check_preconditions(bob)
+
+        # Start processes and check what was started
+        c_channel_env = {}
+        bob.cfg_start_auth = True
+        bob.cfg_start_recurse = False
+
+        bob.start_all_processes(c_channel_env)
+
+        self.assertEqual(bob.msgq, True)
+        self.assertEqual(bob.cfgmgr, True)
+        self.assertEqual(bob.ccsession, True)
+        self.assertEqual(bob.auth, True)
+        self.assertEqual(bob.recurse, False)
+        self.assertEqual(bob.xfrout, True)
+        self.assertEqual(bob.xfrin, True)
+        self.assertEqual(bob.zonemgr, True)
+        self.assertEqual(bob.stats, True)
+        self.assertEqual(bob.cmdctl, True)
+
+    # Checks the processes started when starting only the recurse process
+    def test_start_recurse(self):
+        # Created Bob and ensure initialization correct
+        bob = StartAllProcessesBob()
+        self.check_preconditions(bob)
+
+        # Start processes and check what was started
+        c_channel_env = {}
+        bob.cfg_start_auth = False
+        bob.cfg_start_recurse = True
+
+        bob.start_all_processes(c_channel_env)
+
+        self.assertEqual(bob.msgq, True)
+        self.assertEqual(bob.cfgmgr, True)
+        self.assertEqual(bob.ccsession, True)
+        self.assertEqual(bob.auth, False)
+        self.assertEqual(bob.recurse, True)
+        self.assertEqual(bob.xfrout, False)
+        self.assertEqual(bob.xfrin, False)
+        self.assertEqual(bob.zonemgr, False)
+        self.assertEqual(bob.stats, True)
+        self.assertEqual(bob.cmdctl, True)
+
+    # Checks the processes started when starting both auth and recurse process
+    def test_start_both(self):
+        # Created Bob and ensure initialization correct
+        bob = StartAllProcessesBob()
+        self.check_preconditions(bob)
+
+        # Start processes and check what was started
+        c_channel_env = {}
+        bob.cfg_start_auth = True
+        bob.cfg_start_recurse = True
+
+        bob.start_all_processes(c_channel_env)
+
+        self.assertEqual(bob.msgq, True)
+        self.assertEqual(bob.cfgmgr, True)
+        self.assertEqual(bob.ccsession, True)
+        self.assertEqual(bob.auth, True)
+        self.assertEqual(bob.recurse, True)
+        self.assertEqual(bob.xfrout, True)
+        self.assertEqual(bob.xfrin, True)
+        self.assertEqual(bob.zonemgr, True)
+        self.assertEqual(bob.stats, True)
+        self.assertEqual(bob.cmdctl, True)
+
 
 if __name__ == '__main__':
     unittest.main()

src/bin/bindctl/bindcmd.py

@@ -558,7 +558,7 @@ class BindCmdInterpreter(Cmd):
                     if value_map['type'] in [ 'module', 'map', 'list' ]:
                         line += "/"
                     else:
-                        line += ":\t" + str(value_map['value'])
+                        line += ":\t" + json.dumps(value_map['value'])
                     line += "\t" + value_map['type']
                     line += "\t"
                     if value_map['default']:
@@ -569,7 +569,10 @@ class BindCmdInterpreter(Cmd):
             elif cmd.command == "add":
                 self.config_data.add_value(identifier, cmd.params['value'])
             elif cmd.command == "remove":
-                self.config_data.remove_value(identifier, cmd.params['value'])
+                if 'value' in cmd.params:
+                    self.config_data.remove_value(identifier, cmd.params['value'])
+                else:
+                    self.config_data.remove_value(identifier, None)
             elif cmd.command == "set":
                 if 'identifier' not in cmd.params:
                     print("Error: missing identifier or value")

src/bin/bindctl/bindctl-source.py.in

@@ -28,7 +28,10 @@ import isc.util.process
 
 isc.util.process.rename()
 
-__version__ = 'Bindctl'
+# This is the version that gets displayed to the user.
+# The VERSION string consists of the module name, the module version
+# number, and the overall BIND 10 version number (set in configure.ac).
+VERSION = "bindctl 20101201 (BIND 10 @PACKAGE_VERSION@)"
 
 def prepare_config_commands(tool):
     '''Prepare fixed commands for local configuration editing'''
@@ -48,7 +51,7 @@ def prepare_config_commands(tool):
     cmd = CommandInfo(name = "remove", desc = "Remove entry from configuration list")
     param = ParamInfo(name = "identifier", type = "string", optional=True)
     cmd.add_param(param)
-    param = ParamInfo(name = "value", type = "string", optional=False)
+    param = ParamInfo(name = "value", type = "string", optional=True)
     cmd.add_param(param)
     module.add_command(cmd)
 
@@ -113,7 +116,7 @@ def set_bindctl_options(parser):
 
 if __name__ == '__main__':
     try:
-        parser = OptionParser(version = __version__)
+        parser = OptionParser(version = VERSION)
         set_bindctl_options(parser)
         (options, args) = parser.parse_args()
         server_addr = options.addr + ':' + str(options.port)

src/bin/msgq/msgq.py.in

@@ -38,7 +38,9 @@ import isc.cc
 isc.util.process.rename()
 
 # This is the version that gets displayed to the user.
-__version__ = "v20091030 (Paving the DNS Parking Lot)"
+# The VERSION string consists of the module name, the module version
+# number, and the overall BIND 10 version number (set in configure.ac).
+VERSION = "b10-msgq 20100818 (BIND 10 @PACKAGE_VERSION@)"
 
 class MsgQReceiveError(Exception): pass
 
@@ -421,7 +423,7 @@ if __name__ == "__main__":
         parser.values.msgq_port = intval
 
     # Parse any command-line options.
-    parser = OptionParser(version=__version__)
+    parser = OptionParser(version=VERSION)
     parser.add_option("-v", "--verbose", dest="verbose", action="store_true",
                       help="display more about what is going on")
     parser.add_option("-s", "--socket-file", dest="msgq_socket_file",
@@ -433,7 +435,7 @@ if __name__ == "__main__":
 
     # Announce startup.
     if options.verbose:
-        sys.stdout.write("[b10-msgq] MsgQ %s\n" % __version__)
+        sys.stdout.write("[b10-msgq] %s\n" % VERSION)
 
     msgq = MsgQ(options.msgq_socket_file, options.verbose)
 

src/bin/recurse/Makefile.am

@@ -0,0 +1,20 @@
+# SUBDIRS = . tests
+
+pkglibexecdir = $(libexecdir)/@PACKAGE@
+
+pkglibexec_SCRIPTS = b10-recurse
+
+b10_recursedir = $(DESTDIR)$(pkgdatadir)
+b10_recurse_DATA = recurse.spec
+
+CLEANFILES=	b10-recurse recurse.pyc recurse.py recurse.spec recurse.spec.pre
+
+recurse.spec: recurse.spec.pre
+	$(SED) -e "s|@@LOCALSTATEDIR@@|$(localstatedir)|" recurse.spec.pre >$@
+
+# TODO: does this need $$(DESTDIR) also?
+# this is done here since configure.ac AC_OUTPUT doesn't expand exec_prefix
+b10-recurse: recurse.py
+	$(SED) -e "s|@@PYTHONPATH@@|@pyexecdir@|" \
+	       -e "s|@@LOCALSTATEDIR@@|$(localstatedir)|" recurse.py >$@
+	chmod a+x $@

src/bin/recurse/README_FIRST.txt

@@ -0,0 +1,7 @@
+All the files in this directory are for testing ticket #412 only.
+
+Another ticket has created the "b10-recurse" program.  When both are merged
+into the trunk, these files should be deleted.
+
+Stephen Morris
+24 November 2010

src/bin/recurse/recurse.py.in

@@ -0,0 +1,177 @@
+#!@PYTHON@
+
+# Copyright (C) 2010  Internet Systems Consortium.
+# Copyright (C) 2010  CZ NIC
+#
+# 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.
+
+"""
+   This is a dummy recursor module, purely for testing that the changes to
+   the Boss regarding the starting of recurse/auth works.  It should be deleted
+   when the real recursor module is made available.
+"""
+
+import sys; sys.path.append ('@@PYTHONPATH@@')
+import isc
+import isc.cc
+import threading
+import struct
+import signal
+from isc.datasrc import sqlite3_ds
+from socketserver import *
+import os
+from isc.config.ccsession import *
+from isc.log.log import *
+from isc.cc import SessionError, SessionTimeout
+from isc.notify import notify_out
+import isc.util.process
+import socket
+import select
+import errno
+from optparse import OptionParser, OptionValueError
+from isc.util import socketserver_mixin
+
+try:
+    from libxfr_python import *
+    from pydnspp import *
+except ImportError as e:
+    # C++ loadable module may not be installed; even so the recurse process
+    # must keep running, so we warn about it and move forward.
+    sys.stderr.write('[b10-recurse] failed to import DNS or XFR module: %s\n' % str(e))
+
+isc.util.process.rename()
+
+if "B10_FROM_BUILD" in os.environ:
+    SPECFILE_PATH = os.environ["B10_FROM_BUILD"] + "/src/bin/recurse"
+    AUTH_SPECFILE_PATH = os.environ["B10_FROM_BUILD"] + "/src/bin/auth"
+    UNIX_SOCKET_FILE= os.environ["B10_FROM_BUILD"] + "/auth_recurse_conn"
+else:
+    PREFIX = "@prefix@"
+    DATAROOTDIR = "@datarootdir@"
+    SPECFILE_PATH = "@datadir@/@PACKAGE@".replace("${datarootdir}", DATAROOTDIR).replace("${prefix}", PREFIX)
+    AUTH_SPECFILE_PATH = SPECFILE_PATH
+    UNIX_SOCKET_FILE = "@@LOCALSTATEDIR@@/auth_recurse_conn"
+
+SPECFILE_LOCATION = SPECFILE_PATH + "/recurse.spec"
+AUTH_SPECFILE_LOCATION = AUTH_SPECFILE_PATH + os.sep + "auth.spec"
+MAX_TRANSFERS_OUT = 10
+VERBOSE_MODE = False
+
+
+RESOLVER_MAX_MESSAGE_SIZE = 65535
+
+class ResolverServer:
+    def __init__(self):
+        self._unix_socket_server = None
+        self._log = None
+        self._listen_sock_file = UNIX_SOCKET_FILE
+        self._shutdown_event = threading.Event()
+        self._cc = isc.config.ModuleCCSession(SPECFILE_LOCATION, self.config_handler, self.command_handler)
+        self._config_data = self._cc.get_full_config()
+        self._cc.start()
+        self._cc.add_remote_config(AUTH_SPECFILE_LOCATION);
+
+    def config_handler(self, new_config):
+        '''Update config data. TODO. Do error check'''
+        answer = create_answer(0)
+        for key in new_config:
+            if key not in self._config_data:
+                answer = create_answer(1, "Unknown config data: " + str(key))
+                continue
+            self._config_data[key] = new_config[key]
+
+        if self._log:
+            self._log.update_config(new_config)
+
+        if self._unix_socket_server:
+            self._unix_socket_server.update_config_data(self._config_data)
+
+        return answer
+
+
+    def shutdown(self):
+        '''
+            shutdown the recurse process.
+        '''
+
+        global recurse_server
+        recurse_server = None #Avoid shutdown is called twice
+        self._shutdown_event.set()
+        if self._unix_socket_server:
+            self._unix_socket_server.shutdown()
+        sys.exit(0)
+
+    def command_handler(self, cmd, args):
+        if cmd == "shutdown":
+            self._log.log_message("info", "Received shutdown command.")
+            self.shutdown()
+            answer = create_answer(0)
+        else:
+            answer = create_answer(1, "Unknown command:" + str(cmd))
+
+        return answer
+
+    def run(self):
+        '''Get and process all commands sent from cfgmgr or other modules. '''
+        while not self._shutdown_event.is_set():
+            self._cc.check_command(False)
+
+
+recurse_server = None
+
+def signal_handler(signal, frame):
+    if recurse_server:
+        recurse_server.shutdown()
+        sys.exit(0)
+
+def set_signal_handler():
+    signal.signal(signal.SIGTERM, signal_handler)
+    signal.signal(signal.SIGINT, signal_handler)
+
+def set_cmd_options(parser):
+    parser.add_option("-a", "--address", dest="address", type="string",
+            default="127.0.0.1", help="Address on which recursor listens")
+    parser.add_option("-n", "--nocache", dest="nocache", action="store_true",
+            help="Specify to disable the cache")
+    parser.add_option("-p", "--port", dest="port", type="string",
+            default="10", help="UID under which recursor runs")
+    parser.add_option("-u", "--uid", dest="uid", type="string",
+            default="5301", help="Port on which recursor listens")
+    parser.add_option("-v", "--verbose", dest="verbose", action="store_true",
+            help="display more about what is going on")
+
+if '__main__' == __name__:
+    try:
+        parser = OptionParser()
+        set_cmd_options(parser)
+        (options, args) = parser.parse_args()
+        VERBOSE_MODE = options.verbose
+
+        set_signal_handler()
+        recurse_server = ResolverServer()
+        recurse_server.run()
+    except KeyboardInterrupt:
+        sys.stderr.write("[b10-recurse] exit recurse process\n")
+    except SessionError as e:
+        sys.stderr.write("[b10-recurse] Error creating recurse, "
+                           "is the command channel daemon running?\n")
+    except SessionTimeout as e:
+        sys.stderr.write("[b10-recurse] Error creating recurse, "
+                           "is the configuration manager running?\n")
+    except ModuleCCSessionError as e:
+        sys.stderr.write("[b10-recurse] exit recurse process:%s\n" % str(e))
+
+    if recurse_server:
+        recurse_server.shutdown()
+

src/bin/recurse/recurse.spec.pre.in

@@ -0,0 +1,15 @@
+{
+  "module_spec": {
+     "module_name": "Recurse",
+     "config_data": [
+      ],
+      "commands": [
+        {
+          "command_name": "shutdown",
+          "command_description": "Shut down Resolver",
+          "command_args": []
+        }
+      ]
+  }
+}
+     

src/bin/recurse/run_b10-recurse.sh.in

@@ -0,0 +1,27 @@
+#! /bin/sh
+
+# Copyright (C) 2010  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.
+
+PYTHON_EXEC=${PYTHON_EXEC:-@PYTHON@}
+export PYTHON_EXEC
+
+MYPATH_PATH=@abs_top_builddir@/src/bin/recurse
+PYTHONPATH=@abs_top_srcdir@/src/lib/python:@abs_top_builddir@/src/lib/xfr/.libs:@abs_top_builddir@/src/lib/dns/python/.libs
+export PYTHONPATH
+
+cd ${MYPATH_PATH}
+${PYTHON_EXEC} b10-recurse
+

src/bin/xfrin/b10-xfrin.xml

@@ -149,7 +149,6 @@
       the authoritative server to transfer from,
       and <varname>port</varname> to define the port number on the
       authoritative server (defaults to 53).
-<!-- TODO: note: not documenting db_file since that will be removed. -->
      </para>
 <!-- TODO: later hostname for master? -->
 

src/bin/xfrout/b10-xfrout.8

@@ -2,12 +2,12 @@
 .\"     Title: b10-xfrout
 .\"    Author: [FIXME: author] [see http://docbook.sf.net/el/author]
 .\" Generator: DocBook XSL Stylesheets v1.75.2 <http://docbook.sf.net/>
-.\"      Date: September 8, 2010
+.\"      Date: December 1, 2010
 .\"    Manual: BIND10
 .\"    Source: BIND10
 .\"  Language: English
 .\"
-.TH "B10\-XFROUT" "8" "September 8, 2010" "BIND10" "BIND10"
+.TH "B10\-XFROUT" "8" "December 1, 2010" "BIND10" "BIND10"
 .\" -----------------------------------------------------------------
 .\" * set default formatting
 .\" -----------------------------------------------------------------
@@ -67,11 +67,6 @@ receives its configurations from
 The configurable settings are:
 .PP
 
-\fIdb_file\fR
-defines the path to the SQLite3 data store file\&. The default is
-/usr/local/var/bind10\-devel/zone\&.sqlite3\&.
-.PP
-
 \fItransfers_out\fR
 defines the maximum number of outgoing zone transfers that can run concurrently\&. The default is 10\&.
 .if n \{\

src/bin/xfrout/b10-xfrout.xml

@@ -21,7 +21,7 @@
 <refentry>
 
   <refentryinfo>
-    <date>September 8, 2010</date>
+    <date>December 1, 2010</date>
   </refentryinfo>
 
   <refmeta>
@@ -94,13 +94,6 @@
       The configurable settings are:
     </para>
     <para>
-      <varname>db_file</varname>
-      defines the path to the SQLite3 data store file.
-      The default is
-      <filename>/usr/local/var/bind10-devel/zone.sqlite3</filename>.
-<!-- TODO: db_file will be removed -->
-    </para>
-    <para>
       <varname>transfers_out</varname>
       defines the maximum number of outgoing zone transfers
       that can run concurrently. The default is 10.

src/bin/xfrout/xfrout.spec.pre.in

@@ -9,12 +9,6 @@
          "item_default": 10
        },
        {
-         "item_name": "db_file",
-         "item_type": "string",
-         "item_optional": false,
-         "item_default": "@@LOCALSTATEDIR@@/@PACKAGE@/zone.sqlite3"
-       },
-       {
          "item_name": "log_name",
          "item_type": "string",
          "item_optional": false,

src/lib/config/config_data.cc

@@ -128,11 +128,6 @@ spec_name_list(ElementPtr result, ConstElementPtr spec_part,
                 if (recurse && list_el->get("item_type")->stringValue() == "map") {
                     spec_name_list(result, list_el->get("map_item_spec"), new_prefix, recurse);
                 } else {
-                    if (list_el->get("item_type")->stringValue() == "map" ||
-                        list_el->get("item_type")->stringValue() == "list"
-                    ) {
-                        new_prefix += "/";
-                    }
                     result->add(Element::create(new_prefix));
                 }
             }

src/lib/config/tests/config_data_unittests.cc

@@ -120,8 +120,8 @@ TEST(ConfigData, getItemList) {
     ModuleSpec spec2 = moduleSpecFromFile(std::string(TEST_DATA_PATH) + "/spec2.spec");
     ConfigData cd = ConfigData(spec2);
 
-    EXPECT_EQ("[ \"item1\", \"item2\", \"item3\", \"item4\", \"item5/\", \"item6/\" ]", cd.getItemList()->str());
+    EXPECT_EQ("[ \"item1\", \"item2\", \"item3\", \"item4\", \"item5\", \"item6\" ]", cd.getItemList()->str());
-    EXPECT_EQ("[ \"item1\", \"item2\", \"item3\", \"item4\", \"item5/\", \"item6/value1\", \"item6/value2\" ]", cd.getItemList("", true)->str());
+    EXPECT_EQ("[ \"item1\", \"item2\", \"item3\", \"item4\", \"item5\", \"item6/value1\", \"item6/value2\" ]", cd.getItemList("", true)->str());
     EXPECT_EQ("[ \"item6/value1\", \"item6/value2\" ]", cd.getItemList("item6")->str());
 }
 
@@ -129,12 +129,12 @@ TEST(ConfigData, getFullConfig) {
     ModuleSpec spec2 = moduleSpecFromFile(std::string(TEST_DATA_PATH) + "/spec2.spec");
     ConfigData cd = ConfigData(spec2);
 
-    EXPECT_EQ("{ \"item1\": 1, \"item2\": 1.1, \"item3\": true, \"item4\": \"test\", \"item5/\": [ \"a\", \"b\" ], \"item6/value1\": \"default\", \"item6/value2\": None }", cd.getFullConfig()->str());
+    EXPECT_EQ("{ \"item1\": 1, \"item2\": 1.1, \"item3\": true, \"item4\": \"test\", \"item5\": [ \"a\", \"b\" ], \"item6/value1\": \"default\", \"item6/value2\": None }", cd.getFullConfig()->str());
     ElementPtr my_config = Element::fromJSON("{ \"item1\": 2 }");
     cd.setLocalConfig(my_config);
-    EXPECT_EQ("{ \"item1\": 2, \"item2\": 1.1, \"item3\": true, \"item4\": \"test\", \"item5/\": [ \"a\", \"b\" ], \"item6/value1\": \"default\", \"item6/value2\": None }", cd.getFullConfig()->str());
+    EXPECT_EQ("{ \"item1\": 2, \"item2\": 1.1, \"item3\": true, \"item4\": \"test\", \"item5\": [ \"a\", \"b\" ], \"item6/value1\": \"default\", \"item6/value2\": None }", cd.getFullConfig()->str());
     ElementPtr my_config2 = Element::fromJSON("{ \"item6\": { \"value1\": \"a\" } }");
     cd.setLocalConfig(my_config2);
-    EXPECT_EQ("{ \"item1\": 1, \"item2\": 1.1, \"item3\": true, \"item4\": \"test\", \"item5/\": [ \"a\", \"b\" ], \"item6/value1\": \"a\", \"item6/value2\": None }", cd.getFullConfig()->str());
+    EXPECT_EQ("{ \"item1\": 1, \"item2\": 1.1, \"item3\": true, \"item4\": \"test\", \"item5\": [ \"a\", \"b\" ], \"item6/value1\": \"a\", \"item6/value2\": None }", cd.getFullConfig()->str());
 }
 

src/lib/datasrc/static_datasrc.cc

@@ -79,6 +79,7 @@ StaticDataSrcImpl::StaticDataSrcImpl() :
     authors->addRdata(generic::TXT("JINMEI Tatuya"));
     authors->addRdata(generic::TXT("Kazunori Fujiwara"));
     authors->addRdata(generic::TXT("Michael Graff"));
+    authors->addRdata(generic::TXT("Michal Vaner"));
     authors->addRdata(generic::TXT("Naoki Kambe"));
     authors->addRdata(generic::TXT("Shane Kerr"));
     authors->addRdata(generic::TXT("Shen Tingting"));

src/lib/datasrc/tests/static_unittest.cc

@@ -63,6 +63,7 @@ protected:
         authors_data.push_back("JINMEI Tatuya");
         authors_data.push_back("Kazunori Fujiwara");
         authors_data.push_back("Michael Graff");
+        authors_data.push_back("Michal Vaner");
         authors_data.push_back("Naoki Kambe");
         authors_data.push_back("Shane Kerr");
         authors_data.push_back("Shen Tingting");

src/lib/python/isc/cc/data.py

@@ -56,20 +56,116 @@ def remove_null_items(d):
     for k in null_keys:
         del d[k]
 
-def find(element, identifier):
+def _concat_identifier(id_parts):
-    """Returns the subelement in the given data element, raises DataNotFoundError if not found"""
+    """Concatenates the given identifier parts into a string,
-    if type(identifier) != str or (type(element) != dict and identifier != ""):
+       delimited with the '/' character.
-        raise DataTypeError("identifier in merge() is not a string")
+    """
-    if type(identifier) != str or (type(element) != dict and identifier != ""):
+    return '/'.join(id_parts)
-        raise DataTypeError("element in merge() is not a dict")
+
-    id_parts = identifier.split("/")
+def split_identifier(identifier):
+    """Splits the given identifier into a list of identifier parts,
+       as delimited by the '/' character.
+       Raises a DataTypeError if identifier is not a string."""
+    if type(identifier) != str:
+        raise DataTypeError("identifier is not a string")
+    id_parts = identifier.split('/')
     id_parts[:] = (value for value in id_parts if value != "")
+    return id_parts
+
+def split_identifier_list_indices(identifier):
+    """Finds list indexes in the given identifier, which are of the
+       format [integer].
+       Identifier must be a string.
+       This will only give the list index for the last 'part' of the
+       given identifier (as delimited by the '/' sign).
+       Raises a DataTypeError if the identifier is not a string,
+       or if the format is bad.
+       Returns a tuple, where the first element is the string part of
+       the identifier, and the second element is a list of (nested) list
+       indices.
+       Examples:
+       'a/b/c' will return ('a/b/c', None)
+       'a/b/c[1]' will return ('a/b/c', [1])
+       'a/b/c[1][2][3]' will return ('a/b/c', [1, 2, 3])
+       'a[0]/b[1]/c[2]' will return ('a[0]/b[1]/c', [2])
+    """
+    if type(identifier) != str:
+        raise DataTypeError("identifier in "
+                            "split_identifier_list_indices() "
+                            "not a string: " + str(identifier))
+
+    # We only work on the final 'part' of the identifier
+    id_parts = split_identifier(identifier)
+    id_str = id_parts[-1]
+
+    i = id_str.find('[')
+    if i < 0:
+        if identifier.find(']') >= 0:
+            raise DataTypeError("Bad format in identifier: " + str(identifier))
+        return identifier, None
+
+    # keep the non-index part of that to replace later
+    id = id_str[:i]
+    indices = []
+    while i >= 0:
+        e = id_str.find(']')
+        if e < i + 1:
+            raise DataTypeError("Bad format in identifier: " + str(identifier))
+        try:
+            indices.append(int(id_str[i+1:e]))
+        except ValueError:
+            raise DataTypeError("List index in " + identifier + " not an integer")
+        id_str = id_str[e + 1:]
+        i = id_str.find('[')
+        if i > 0:
+            raise DataTypeError("Bad format in identifier: " + str(identifier))
+    if id.find(']') >= 0 or len(id_str) > 0:
+        raise DataTypeError("Bad format in identifier: " + str(identifier))
+
+    # we replace the final part of the original identifier with
+    # the stripped string
+    id_parts[-1] = id
+    id = _concat_identifier(id_parts)
+    return id, indices
+
+def _find_child_el(element, id):
+    """Finds the child of element with the given id. If the id contains
+       [i], where i is a number, and the child element is a list, the
+       i-th element of that list is returned instead of the list itself.
+       Raises a DataTypeError if the element is of wrong type, if id
+       is not a string, or if the id string contains a bad value.
+       Raises a DataNotFoundError if the element at id could not be
+       found.
+    """
+    id, list_indices = split_identifier_list_indices(id)
+    if type(element) == dict and id in element.keys():
+        result = element[id]
+    else:
+        raise DataNotFoundError(id + " in " + str(element))
+    if type(result) == list and list_indices is not None:
+        for list_index in list_indices:
+            if list_index >= len(result):
+                raise DataNotFoundError("Element " + str(list_index) + " in " + str(result))
+            result = result[list_index]
+    return result
+
+def find(element, identifier):
+    """Returns the subelement in the given data element, raises
+       DataNotFoundError if not found.
+       Returns the given element if the identifier is an empty string.
+       Raises a DataTypeError if identifier is not a string, or if
+       identifier is not empty, and element is not a dict.
+    """
+    if type(identifier) != str:
+        raise DataTypeError("identifier in find() is not a str")
+    if identifier == "":
+        return element
+    if type(element) != dict:
+        raise DataTypeError("element in find() is not a dict")
+    id_parts = split_identifier(identifier)
     cur_el = element
     for id in id_parts:
-        if type(cur_el) == dict and id in cur_el.keys():
+        cur_el = _find_child_el(cur_el, id)
-            cur_el = cur_el[id]
-        else:
-            raise DataNotFoundError(identifier + " in " + str(element))
     return cur_el
 
 def set(element, identifier, value):
@@ -83,25 +179,46 @@ def set(element, identifier, value):
     if type(element) != dict:
         raise DataTypeError("element in set() is not a dict")
     if type(identifier) != str:
-        raise DataTypeError("identifier in set() is not a string")
+        raise DataTypeError("identifier in set() is not a str")
-    id_parts = identifier.split("/")
+    id_parts = split_identifier(identifier)
-    id_parts[:] = (value for value in id_parts if value != "")
     cur_el = element
     for id in id_parts[:-1]:
-        if id in cur_el.keys():
+        try:
-            cur_el = cur_el[id]
+            cur_el = _find_child_el(cur_el, id)
-        else:
+        except DataNotFoundError:
-            if value == None:
+            if value is None:
                 # ok we are unsetting a value that wasn't set in
                 # the first place. Simply stop.
                 return
             cur_el[id] = {}
             cur_el = cur_el[id]
-    # value can be an empty list or dict, so check for None eplicitely
+
-    if value != None:
+    id, list_indices = split_identifier_list_indices(id_parts[-1])
-        cur_el[id_parts[-1]] = value
+    if list_indices is None:
-    elif id_parts[-1] in cur_el:
+        # value can be an empty list or dict, so check for None eplicitely
-        del cur_el[id_parts[-1]]
+        if value is not None:
+            cur_el[id] = value
+        else:
+            del cur_el[id]
+    else:
+        cur_el = cur_el[id]
+        # in case of nested lists, we need to get to the next to last
+        for list_index in list_indices[:-1]:
+            if type(cur_el) != list:
+                raise DataTypeError("Element at " + identifier + " is not a list")
+            if len(cur_el) <= list_index:
+                raise DataNotFoundError("List index at " + identifier + " out of range")
+            cur_el = cur_el[list_index]
+        # value can be an empty list or dict, so check for None eplicitely
+        list_index = list_indices[-1]
+        if type(cur_el) != list:
+            raise DataTypeError("Element at " + identifier + " is not a list")
+        if len(cur_el) <= list_index:
+            raise DataNotFoundError("List index at " + identifier + " out of range")
+        if value is not None:
+            cur_el[list_index] = value
+        else:
+            del cur_el[list_index]
     return element
 
 def unset(element, identifier):
@@ -116,17 +233,12 @@ def find_no_exc(element, identifier):
     """Returns the subelement in the given data element, returns None
        if not found, or if an error occurred (i.e. this function should
        never raise an exception)"""
-    if type(identifier) != str:
+    try:
+        return find(element, identifier)
+    except DataNotFoundError:
+        return None
+    except DataTypeError:
         return None
-    id_parts = identifier.split("/")
-    id_parts[:] = (value for value in id_parts if value != "")
-    cur_el = element
-    for id in id_parts:
-        if (type(cur_el) == dict and id in cur_el.keys()) or id=="":
-            cur_el = cur_el[id]
-        else:
-            return None
-    return cur_el
 
 def parse_value_str(value_str):
     """Parses the given string to a native python object. If the
@@ -139,7 +251,4 @@ def parse_value_str(value_str):
     except ValueError as ve:
         # simply return the string itself
         return value_str
-    except SyntaxError as ve:
-        # simply return the string itself
-        return value_str
 

src/lib/python/isc/cc/tests/data_test.py

@@ -70,6 +70,11 @@ class TestData(unittest.TestCase):
         c = { "a": { "b": "c" } }
         data.remove_identical(a, b)
         self.assertEqual(a, c)
+
+        self.assertRaises(data.DataTypeError, data.remove_identical,
+                          a, 1)
+        self.assertRaises(data.DataTypeError, data.remove_identical,
+                          1, b)
         
     def test_merge(self):
         d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': 2 } }
@@ -82,6 +87,45 @@ class TestData(unittest.TestCase):
         self.assertRaises(data.DataTypeError, data.merge, 1, d2)
         self.assertRaises(data.DataTypeError, data.merge, None, None)
 
+
+    def test_split_identifier_list_indices(self):
+        id, indices = data.split_identifier_list_indices('a')
+        self.assertEqual(id, 'a')
+        self.assertEqual(indices, None)
+        id, indices = data.split_identifier_list_indices('a[0]')
+        self.assertEqual(id, 'a')
+        self.assertEqual(indices, [0])
+        id, indices = data.split_identifier_list_indices('a[0][1]')
+        self.assertEqual(id, 'a')
+        self.assertEqual(indices, [0, 1])
+
+        # examples from the docstring
+        id, indices = data.split_identifier_list_indices('a/b/c')
+        self.assertEqual(id, 'a/b/c')
+        self.assertEqual(indices, None)
+        
+        id, indices = data.split_identifier_list_indices('a/b/c[1]')
+        self.assertEqual(id, 'a/b/c')
+        self.assertEqual(indices, [1])
+       
+        id, indices = data.split_identifier_list_indices('a/b/c[1][2][3]')
+        self.assertEqual(id, 'a/b/c')
+        self.assertEqual(indices, [1, 2, 3])
+        
+        id, indices = data.split_identifier_list_indices('a[0]/b[1]/c[2]')
+        self.assertEqual(id, 'a[0]/b[1]/c')
+        self.assertEqual(indices, [2])
+
+        # bad formats
+        self.assertRaises(data.DataTypeError, data.split_identifier_list_indices, 'a[')
+        self.assertRaises(data.DataTypeError, data.split_identifier_list_indices, 'a]')
+        self.assertRaises(data.DataTypeError, data.split_identifier_list_indices, 'a[[0]]')
+        self.assertRaises(data.DataTypeError, data.split_identifier_list_indices, 'a[0]a')
+        self.assertRaises(data.DataTypeError, data.split_identifier_list_indices, 'a[0]a[1]')
+
+        self.assertRaises(data.DataTypeError, data.split_identifier_list_indices, 1)
+        
+
     def test_find(self):
         d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': 2, 'more': { 'data': 'here' } } }
         self.assertEqual(data.find(d1, ''), d1)
@@ -93,19 +137,47 @@ class TestData(unittest.TestCase):
         self.assertRaises(data.DataNotFoundError, data.find, d1, 'f')
         self.assertRaises(data.DataTypeError, data.find, d1, 1)
         self.assertRaises(data.DataTypeError, data.find, None, 1)
+        self.assertRaises(data.DataTypeError, data.find, None, "foo")
         self.assertRaises(data.DataTypeError, data.find, "123", "123")
         self.assertEqual(data.find("123", ""), "123")
+
+        d2 = { 'a': [ 1, 2, 3 ] }
+        self.assertEqual(data.find(d2, 'a[0]'), 1)
+        self.assertEqual(data.find(d2, 'a[1]'), 2)
+        self.assertEqual(data.find(d2, 'a[2]'), 3)
+        self.assertRaises(data.DataNotFoundError, data.find, d2, 'a[3]')
+        self.assertRaises(data.DataTypeError, data.find, d2, 'a[a]')
+
+        d3 = { 'a': [ { 'b': [ {}, { 'c': 'd' } ] } ] }
+        self.assertEqual(data.find(d3, 'a[0]/b[1]/c'), 'd')
+        self.assertRaises(data.DataNotFoundError, data.find, d3, 'a[1]/b[1]/c')
         
     def test_set(self):
         d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': 2 } }
         d12 = { 'b': 1, 'c': { 'e': 3, 'f': [ 1 ] } }
+        d13 = { 'b': 1, 'c': { 'e': 3, 'f': [ 2 ] } }
+        d14 = { 'b': 1, 'c': { 'e': 3, 'f': [ { 'g': [ 1, 2 ] } ] } }
+        d15 = { 'b': 1, 'c': { 'e': 3, 'f': [ { 'g': [ 1, 3 ] } ] } }
         data.set(d1, 'a', None)
         data.set(d1, 'c/d', None)
         data.set(d1, 'c/e/', 3)
         data.set(d1, 'c/f', [ 1 ] )
         self.assertEqual(d1, d12)
+        data.set(d1, 'c/f[0]', 2 )
+        self.assertEqual(d1, d13)
+
+        data.set(d1, 'c/f[0]', { 'g': [ 1, 2] } )
+        self.assertEqual(d1, d14)
+        data.set(d1, 'c/f[0]/g[1]', 3)
+        self.assertEqual(d1, d15)
+        
         self.assertRaises(data.DataTypeError, data.set, d1, 1, 2)
         self.assertRaises(data.DataTypeError, data.set, 1, "", 2)
+        self.assertRaises(data.DataTypeError, data.set, d1, 'c[1]', 2)
+        self.assertRaises(data.DataTypeError, data.set, d1, 'c[1][2]', 2)
+        self.assertRaises(data.DataNotFoundError, data.set, d1, 'c/f[5]', 2)
+        self.assertRaises(data.DataNotFoundError, data.set, d1, 'c/f[5][2]', 2)
+
         d3 = {}
         e3 = data.set(d3, "does/not/exist", 123)
         self.assertEqual(d3,
@@ -114,11 +186,25 @@ class TestData(unittest.TestCase):
                          { 'does': { 'not': { 'exist': 123 } } })
 
     def test_unset(self):
-        d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': 2 } }
+        d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': [ 1, 2, 3 ] } }
         data.unset(d1, 'a')
         data.unset(d1, 'c/d')
         data.unset(d1, 'does/not/exist')
-        self.assertEqual(d1, { 'b': 1, 'c': { 'e': 2 } })
+        self.assertEqual(d1, { 'b': 1, 'c': { 'e': [ 1, 2, 3 ] } })
+        data.unset(d1, 'c/e[0]')
+        self.assertEqual(d1, { 'b': 1, 'c': { 'e': [ 2, 3 ] } })
+        data.unset(d1, 'c/e[1]')
+        self.assertEqual(d1, { 'b': 1, 'c': { 'e': [ 2 ] } })
+        # index 1 should now be out of range
+        self.assertRaises(data.DataNotFoundError, data.unset, d1, 'c/e[1]')
+        d2 = { 'a': [ { 'b': [ 1, 2 ] } ] }
+        data.unset(d2, 'a[0]/b[1]')
+        self.assertEqual(d2, { 'a': [ { 'b': [ 1 ] } ] })
+        d3 = { 'a': [ [ 1, 2 ] ] }
+        data.set(d3, "a[0][1]", 3)
+        self.assertEqual(d3, { 'a': [ [ 1, 3 ] ] })
+        data.unset(d3, 'a[0][1]')
+        self.assertEqual(d3, { 'a': [ [ 1 ] ] })
         
     def test_find_no_exc(self):
         d1 = { 'a': 'a', 'b': 1, 'c': { 'd': 'd', 'e': 2, 'more': { 'data': 'here' } } }
@@ -146,6 +232,9 @@ class TestData(unittest.TestCase):
         self.assertEqual(data.parse_value_str("{ \"a\": \"b\", \"c\": 1 }"), { 'a': 'b', 'c': 1 })
         self.assertEqual(data.parse_value_str("[ a c"), "[ a c")
 
+        self.assertEqual(data.parse_value_str(1), None)
+
+
 if __name__ == '__main__':
     #if not 'CONFIG_TESTDATA_PATH' in os.environ:
     #    print("You need to set the environment variable CONFIG_TESTDATA_PATH to point to the directory containing the test data files")

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

@@ -398,16 +398,25 @@ class UIModuleCCSession(MultiConfigData):
         module_spec = self.find_spec_part(identifier)
         if (type(module_spec) != dict or "list_item_spec" not in module_spec):
             raise isc.cc.data.DataNotFoundError(str(identifier) + " is not a list")
-        value = isc.cc.data.parse_value_str(value_str)
+
-        isc.config.config_data.check_type(module_spec, [value])
+        if value_str is None:
-        cur_list, status = self.get_value(identifier)
+            # we are directly removing an list index
-        #if not cur_list:
+            id, list_indices = isc.cc.data.split_identifier_list_indices(identifier)
-        #    cur_list = isc.cc.data.find_no_exc(self.config.data, identifier)
+            if list_indices is None:
-        if not cur_list:
+                raise DataTypeError("identifier in remove_value() does not contain a list index, and no value to remove")
-            cur_list = []
+            else:
-        if value in cur_list:
+                self.set_value(identifier, None)
-            cur_list.remove(value)
+        else:
-        self.set_value(identifier, cur_list)
+            value = isc.cc.data.parse_value_str(value_str)
+            isc.config.config_data.check_type(module_spec, [value])
+            cur_list, status = self.get_value(identifier)
+            #if not cur_list:
+            #    cur_list = isc.cc.data.find_no_exc(self.config.data, identifier)
+            if not cur_list:
+                cur_list = []
+            if value in cur_list:
+                cur_list.remove(value)
+            self.set_value(identifier, cur_list)
 
     def commit(self):
         """Commit all local changes, send them through b10-cmdctl to

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

@@ -22,6 +22,7 @@ two through the classes in ccsession)
 
 import isc.cc.data
 import isc.config.module_spec
+import ast
 
 class ConfigDataError(Exception): pass
 
@@ -56,14 +57,14 @@ def check_type(spec_part, value):
         raise isc.cc.data.DataTypeError(str(value) + " is not a map")
 
 def convert_type(spec_part, value):
-    """Convert the give value(type is string) according specification 
+    """Convert the given value(type is string) according specification 
     part relevant for the value. Raises an isc.cc.data.DataTypeError 
     exception if conversion failed.
     """
     if type(spec_part) == dict and 'item_type' in spec_part:
         data_type = spec_part['item_type']
     else:
-        raise isc.cc.data.DataTypeError(str("Incorrect specification part for type convering"))
+        raise isc.cc.data.DataTypeError(str("Incorrect specification part for type conversion"))
    
     try:
         if data_type == "integer":
@@ -81,18 +82,25 @@ def convert_type(spec_part, value):
                     ret.append(convert_type(spec_part['list_item_spec'], item))
             elif type(value) == str:    
                 value = value.split(',')
-                for item in value:    
+                for item in value:
                     sub_value = item.split()
                     for sub_item in sub_value:
-                        ret.append(convert_type(spec_part['list_item_spec'], sub_item))
+                        ret.append(convert_type(spec_part['list_item_spec'],
+                                                sub_item))
 
             if ret == []:
                 raise isc.cc.data.DataTypeError(str(value) + " is not a list")
 
             return ret
         elif data_type == "map":
-            return dict(value)
+            map = ast.literal_eval(value)
-            # todo: check types of map contents too
+            if type(map) == dict:
+                # todo: check types of map contents too
+                return map
+            else:
+                raise isc.cc.data.DataTypeError(
+                           "Value in convert_type not a string "
+                           "specifying a dict")
         else:
             return value
     except ValueError as err:
@@ -108,7 +116,11 @@ def find_spec_part(element, identifier):
     id_parts = identifier.split("/")
     id_parts[:] = (value for value in id_parts if value != "")
     cur_el = element
-    for id in id_parts:
+
+    for id_part in id_parts:
+        # strip list selector part
+        # don't need it for the spec part, so just drop it
+        id, list_indices = isc.cc.data.split_identifier_list_indices(id_part)
         if type(cur_el) == dict and 'map_item_spec' in cur_el.keys():
             found = False
             for cur_el_item in cur_el['map_item_spec']:
@@ -158,11 +170,9 @@ def spec_name_list(spec, prefix="", recurse=False):
                     result.extend(spec_name_list(list_el['map_item_spec'], prefix + list_el['item_name'], recurse))
                 else:
                     name = list_el['item_name']
-                    if list_el['item_type'] in ["list", "map"]:
-                        name += "/"
                     result.append(prefix + name)
             else:
-                raise ConfigDataError("Bad specication")
+                raise ConfigDataError("Bad specification")
     else:
         raise ConfigDataError("Bad specication")
     return result
@@ -228,6 +238,20 @@ class ConfigData:
             result[item] = value
         return result
 
+# should we just make a class for these?
+def _create_value_map_entry(name, type, value, status = None):
+    entry = {}
+    entry['name'] = name
+    entry['type'] = type
+    entry['value'] = value
+    entry['modified'] = False
+    entry['default'] = False
+    if status == MultiConfigData.LOCAL:
+        entry['modified'] = True
+    if status == MultiConfigData.DEFAULT:
+        entry['default'] = True
+    return entry
+
 class MultiConfigData:
     """This class stores the module specs, current non-default
        configuration values and 'local' (uncommitted) changes for
@@ -272,7 +296,7 @@ class MultiConfigData:
            identifier (up to the first /) is interpreted as the module
            name. Returns None if not found, or if identifier is not a
            string."""
-        if type(identifier) != str:
+        if type(identifier) != str or identifier == "":
             return None
         if identifier[0] == '/':
             identifier = identifier[1:]
@@ -336,28 +360,42 @@ class MultiConfigData:
         try:
             spec = find_spec_part(self._specifications[module].get_config_spec(), id)
             if 'item_default' in spec:
-                return spec['item_default']
+                id, list_indices = isc.cc.data.split_identifier_list_indices(id)
+                if list_indices is not None and \
+                   type(spec['item_default']) == list:
+                    if len(list_indices) == 1:
+                        default_list = spec['item_default']
+                        index = list_indices[0]
+                        if index < len(default_list):
+                            return default_list[index]
+                        else:
+                            return None
+                else:
+                    return spec['item_default']
             else:
                 return None
         except isc.cc.data.DataNotFoundError as dnfe:
             return None
 
-    def get_value(self, identifier):
+    def get_value(self, identifier, default = True):
         """Returns a tuple containing value,status.
            The value contains the configuration value for the given
            identifier. The status reports where this value came from;
            it is one of: LOCAL, CURRENT, DEFAULT or NONE, corresponding
            (local change, current setting, default as specified by the
-           specification, or not found at all)."""
+           specification, or not found at all). Does not check and
+           set DEFAULT if the argument 'default' is False (default
+           defaults to True)"""
         value = self.get_local_value(identifier)
         if value != None:
             return value, self.LOCAL
         value = self.get_current_value(identifier)
         if value != None:
             return value, self.CURRENT
-        value = self.get_default_value(identifier)
+        if default:
-        if value != None:
+            value = self.get_default_value(identifier)
-            return value, self.DEFAULT
+            if value != None:
+                return value, self.DEFAULT
         return None, self.NONE
 
     def get_value_maps(self, identifier = None):
@@ -374,12 +412,7 @@ class MultiConfigData:
         if not identifier:
             # No identifier, so we need the list of current modules
             for module in self._specifications.keys():
-                entry = {}
+                entry = _create_value_map_entry(module, 'module', None)
-                entry['name'] = module
-                entry['type'] = 'module'
-                entry['value'] = None
-                entry['modified'] = False
-                entry['default'] = False
                 result.append(entry)
         else:
             if identifier[0] == '/':
@@ -389,51 +422,41 @@ class MultiConfigData:
             if spec:
                 spec_part = find_spec_part(spec.get_config_spec(), id)
                 if type(spec_part) == list:
+                    # list of items to show
                     for item in spec_part:
-                        entry = {}
+                        value, status = self.get_value("/" + identifier\
-                        entry['name'] = item['item_name']
+                                              + "/" + item['item_name'])
-                        entry['type'] = item['item_type']
+                        entry = _create_value_map_entry(item['item_name'],
-                        value, status = self.get_value("/" + identifier + "/" + item['item_name'])
+                                                        item['item_type'],
-                        entry['value'] = value
+                                                        value, status)
-                        if status == self.LOCAL:
-                            entry['modified'] = True
-                        else:
-                            entry['modified'] = False
-                        if status == self.DEFAULT:
-                            entry['default'] = False
-                        else:
-                            entry['default'] = False
                         result.append(entry)
                 elif type(spec_part) == dict:
+                    # Sub-specification
                     item = spec_part
                     if item['item_type'] == 'list':
                         li_spec = item['list_item_spec']
-                        item_list, status =  self.get_value("/" + identifier)
+                        value, status =  self.get_value("/" + identifier)
-                        if item_list != None:
+                        if type(value) == list:
-                            for value in item_list:
+                            for list_value in value:
-                                result_part2 = {}
+                                result_part2 = _create_value_map_entry(
-                                result_part2['name'] = li_spec['item_name']
+                                                   li_spec['item_name'],
-                                result_part2['value'] = value
+                                                   li_spec['item_type'],
-                                result_part2['type'] = li_spec['item_type']
+                                                   list_value)
-                                result_part2['default'] = False
-                                result_part2['modified'] = False
                                 result.append(result_part2)
+                        elif value is not None:
+                            entry = _create_value_map_entry(
+                                        li_spec['item_name'],
+                                        li_spec['item_type'],
+                                        value, status)
+                            result.append(entry)
                     else:
-                        entry = {}
-                        entry['name'] = item['item_name']
-                        entry['type'] = item['item_type']
-                        #value, status = self.get_value("/" + identifier + "/" + item['item_name'])
                         value, status = self.get_value("/" + identifier)
-                        entry['value'] = value
+                        if value is not None:
-                        if status == self.LOCAL:
+                            entry = _create_value_map_entry(
-                            entry['modified'] = True
+                                        item['item_name'],
-                        else:
+                                        item['item_type'],
-                            entry['modified'] = False
+                                        value, status)
-                        if status == self.DEFAULT:
+                            result.append(entry)
-                            entry['default'] = False
-                        else:
-                            entry['default'] = False
-                        result.append(entry)
         return result
 
     def set_value(self, identifier, value):
@@ -441,8 +464,28 @@ class MultiConfigData:
            there is a specification for the given identifier, the type
            is checked."""
         spec_part = self.find_spec_part(identifier)
-        if spec_part != None:
+        if spec_part is not None and value is not None:
+            id, list_indices = isc.cc.data.split_identifier_list_indices(identifier)
+            if list_indices is not None \
+               and spec_part['item_type'] == 'list':
+                spec_part = spec_part['list_item_spec']
             check_type(spec_part, value)
+
+        # Since we do not support list diffs (yet?), we need to
+        # copy the currently set list of items to _local_changes
+        # if we want to modify an element in there
+        # (for any list indices specified in the full identifier)
+        id_parts = isc.cc.data.split_identifier(identifier)
+        cur_id_part = '/'
+        for id_part in id_parts:
+            id, list_indices = isc.cc.data.split_identifier_list_indices(id_part)
+            if list_indices is not None:
+                cur_list, status = self.get_value(cur_id_part + id)
+                if status != MultiConfigData.LOCAL:
+                    isc.cc.data.set(self._local_changes,
+                                    cur_id_part + id,
+                                    cur_list)
+            cur_id_part = cur_id_part + id_part + "/"
         isc.cc.data.set(self._local_changes, identifier, value)
  
     def get_config_item_list(self, identifier = None, recurse = False):

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

@@ -637,6 +637,8 @@ class TestUIModuleCCSession(unittest.TestCase):
         self.assertEqual({'Spec2': {'item5': ['foo']}}, uccs._local_changes)
         uccs.add_value("Spec2/item5", "foo")
         self.assertEqual({'Spec2': {'item5': ['foo']}}, uccs._local_changes)
+        uccs.remove_value("Spec2/item5[0]", None)
+        self.assertEqual({'Spec2': {'item5': []}}, uccs._local_changes)
 
     def test_commit(self):
         fake_conn = fakeUIConn()

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

@@ -107,6 +107,8 @@ class TestConfigData(unittest.TestCase):
         self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, "a")
         self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, [ 1, 2 ])
         self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, { "a": 1 })
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, 1, "a")
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, { 'somedict': 'somevalue' }, "a")
         
         spec_part = find_spec_part(config_spec, "value2")
         self.assertEqual(1.1, convert_type(spec_part, '1.1'))
@@ -146,6 +148,18 @@ class TestConfigData(unittest.TestCase):
         self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, [ "1", "b" ])
         self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, { "a": 1 })
 
+        spec_part = find_spec_part(config_spec, "value6")
+        self.assertEqual({}, convert_type(spec_part, '{}'))
+        self.assertEqual({ 'v61': 'a' }, convert_type(spec_part, '{ \'v61\': \'a\' }'))
+
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, 1.1)
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, True)
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, "a")
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, "1")
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, [ "a", "b" ])
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, [ "1", "b" ])
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, { "a": 1 })
+
         spec_part = find_spec_part(config_spec, "value7")
         self.assertEqual(['1', '2'], convert_type(spec_part, '1, 2'))
         self.assertEqual(['1', '2', '3'], convert_type(spec_part, '1 2  3'))
@@ -175,9 +189,9 @@ class TestConfigData(unittest.TestCase):
 
     def test_spec_name_list(self):
         name_list = spec_name_list(self.cd.get_module_spec().get_config_spec())
-        self.assertEqual(['item1', 'item2', 'item3', 'item4', 'item5/', 'item6/'], name_list)
+        self.assertEqual(['item1', 'item2', 'item3', 'item4', 'item5', 'item6'], name_list)
         name_list = spec_name_list(self.cd.get_module_spec().get_config_spec(), "", True)
-        self.assertEqual(['item1', 'item2', 'item3', 'item4', 'item5/', 'item6/value1', 'item6/value2'], name_list)
+        self.assertEqual(['item1', 'item2', 'item3', 'item4', 'item5', 'item6/value1', 'item6/value2'], name_list)
         spec_part = find_spec_part(self.cd.get_module_spec().get_config_spec(), "item6")
         name_list = spec_name_list(spec_part, "item6", True)
         self.assertEqual(['item6/value1', 'item6/value2'], name_list)
@@ -193,7 +207,7 @@ class TestConfigData(unittest.TestCase):
         name_list = spec_name_list({ "myModule": config_spec }, "", False)
         self.assertEqual(['myModule/'], name_list)
         name_list = spec_name_list({ "myModule": config_spec }, "", True)
-        self.assertEqual(['myModule/', 'myModule/value1', 'myModule/value2', 'myModule/value3', 'myModule/value4', 'myModule/value5/', 'myModule/value6/v61', 'myModule/value6/v62', 'myModule/value7/', 'myModule/value8/', 'myModule/value9/v91', 'myModule/value9/v92/v92a', 'myModule/value9/v92/v92b'], name_list)
+        self.assertEqual(['myModule/', 'myModule/value1', 'myModule/value2', 'myModule/value3', 'myModule/value4', 'myModule/value5', 'myModule/value6/v61', 'myModule/value6/v62', 'myModule/value7', 'myModule/value8', 'myModule/value9/v91', 'myModule/value9/v92/v92a', 'myModule/value9/v92/v92b'], name_list)
 
         self.assertRaises(ConfigDataError, spec_name_list, 1)
         self.assertRaises(ConfigDataError, spec_name_list, [ 'a' ])
@@ -240,19 +254,19 @@ class TestConfigData(unittest.TestCase):
 
     def test_get_item_list(self):
         name_list = self.cd.get_item_list()
-        self.assertEqual(['item1', 'item2', 'item3', 'item4', 'item5/', 'item6/'], name_list)
+        self.assertEqual(['item1', 'item2', 'item3', 'item4', 'item5', 'item6'], name_list)
         name_list = self.cd.get_item_list("", True)
-        self.assertEqual(['item1', 'item2', 'item3', 'item4', 'item5/', 'item6/value1', 'item6/value2'], name_list)
+        self.assertEqual(['item1', 'item2', 'item3', 'item4', 'item5', 'item6/value1', 'item6/value2'], name_list)
         name_list = self.cd.get_item_list("item6", False)
         self.assertEqual(['item6/value1', 'item6/value2'], name_list)
 
     def test_get_full_config(self):
         full_config = self.cd.get_full_config()
-        self.assertEqual({ "item1": 1, "item2": 1.1, "item3": True, "item4": "test", "item5/": ['a', 'b'], "item6/value1": 'default', 'item6/value2': None}, full_config)
+        self.assertEqual({ "item1": 1, "item2": 1.1, "item3": True, "item4": "test", "item5": ['a', 'b'], "item6/value1": 'default', 'item6/value2': None}, full_config)
         my_config = { "item1": 2, "item2": 2.2, "item3": False, "item4": "asdf", "item5": [ "c", "d" ] }
         self.cd.set_local_config(my_config)
         full_config = self.cd.get_full_config()
-        self.assertEqual({ "item1": 2, "item2": 2.2, "item3": False, "item4": "asdf", "item5/": [ "c", "d" ], "item6/value1": 'default', 'item6/value2': None}, full_config)
+        self.assertEqual({ "item1": 2, "item2": 2.2, "item3": False, "item4": "asdf", "item5": [ "c", "d" ], "item6/value1": 'default', 'item6/value2': None}, full_config)
 
 class TestMultiConfigData(unittest.TestCase):
     def setUp(self):
@@ -342,6 +356,12 @@ class TestMultiConfigData(unittest.TestCase):
         self.assertEqual(1, value)
         value = self.mcd.get_default_value("/Spec2/item1")
         self.assertEqual(1, value)
+        value = self.mcd.get_default_value("Spec2/item5[0]")
+        self.assertEqual('a', value)
+        value = self.mcd.get_default_value("Spec2/item5[5]")
+        self.assertEqual(None, value)
+        value = self.mcd.get_default_value("Spec2/item5[0][1]")
+        self.assertEqual(None, value)
         value = self.mcd.get_default_value("Spec2/item6/value1")
         self.assertEqual('default', value)
         value = self.mcd.get_default_value("Spec2/item6/value2")
@@ -353,20 +373,34 @@ class TestMultiConfigData(unittest.TestCase):
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec2.spec")
         self.mcd.set_specification(module_spec)
         self.mcd.set_value("Spec2/item1", 2)
-        value,status = self.mcd.get_value("Spec2/item1")
+
+        value, status = self.mcd.get_value("Spec2/item1")
         self.assertEqual(2, value)
         self.assertEqual(MultiConfigData.LOCAL, status)
-        value,status = self.mcd.get_value("Spec2/item2")
+
+        value, status = self.mcd.get_value("Spec2/item2")
         self.assertEqual(1.1, value)
         self.assertEqual(MultiConfigData.DEFAULT, status)
+
         self.mcd._current_config = { "Spec2": { "item3": False } }
-        value,status = self.mcd.get_value("Spec2/item3")
+
+        value, status = self.mcd.get_value("Spec2/item3")
         self.assertEqual(False, value)
         self.assertEqual(MultiConfigData.CURRENT, status)
-        value,status = self.mcd.get_value("Spec2/no_such_item")
+
+        value, status = self.mcd.get_value("Spec2/no_such_item")
+        self.assertEqual(None, value)
+        self.assertEqual(MultiConfigData.NONE, status)
+
+        value, status = self.mcd.get_value("Spec2/item5[0]")
+        self.assertEqual("a", value)
+        self.assertEqual(MultiConfigData.DEFAULT, status)
+
+        value, status = self.mcd.get_value("Spec2/item5[0]", False)
         self.assertEqual(None, value)
         self.assertEqual(MultiConfigData.NONE, status)
 
+
     def test_get_value_maps(self):
         maps = self.mcd.get_value_maps()
         self.assertEqual([], maps)
@@ -390,29 +424,31 @@ class TestMultiConfigData(unittest.TestCase):
         self.mcd.set_value("Spec2/item3", False)
         maps = self.mcd.get_value_maps("/Spec2")
         self.assertEqual([{'default': False, 'type': 'integer', 'name': 'item1', 'value': 2, 'modified': False},
-                          {'default': False, 'type': 'real', 'name': 'item2', 'value': 1.1, 'modified': False},
+                          {'default': True, 'type': 'real', 'name': 'item2', 'value': 1.1, 'modified': False},
                           {'default': False, 'type': 'boolean', 'name': 'item3', 'value': False, 'modified': True},
-                          {'default': False, 'type': 'string', 'name': 'item4', 'value': 'test', 'modified': False},
+                          {'default': True, 'type': 'string', 'name': 'item4', 'value': 'test', 'modified': False},
-                          {'default': False, 'type': 'list', 'name': 'item5', 'value': ['a', 'b'], 'modified': False},
+                          {'default': True, 'type': 'list', 'name': 'item5', 'value': ['a', 'b'], 'modified': False},
-                          {'default': False, 'type': 'map', 'name': 'item6', 'value': {}, 'modified': False}], maps)
+                          {'default': True, 'type': 'map', 'name': 'item6', 'value': {}, 'modified': False}], maps)
         maps = self.mcd.get_value_maps("Spec2")
         self.assertEqual([{'default': False, 'type': 'integer', 'name': 'item1', 'value': 2, 'modified': False},
-                          {'default': False, 'type': 'real', 'name': 'item2', 'value': 1.1, 'modified': False},
+                          {'default': True, 'type': 'real', 'name': 'item2', 'value': 1.1, 'modified': False},
                           {'default': False, 'type': 'boolean', 'name': 'item3', 'value': False, 'modified': True},
-                          {'default': False, 'type': 'string', 'name': 'item4', 'value': 'test', 'modified': False},
+                          {'default': True, 'type': 'string', 'name': 'item4', 'value': 'test', 'modified': False},
-                          {'default': False, 'type': 'list', 'name': 'item5', 'value': ['a', 'b'], 'modified': False},
+                          {'default': True, 'type': 'list', 'name': 'item5', 'value': ['a', 'b'], 'modified': False},
-                          {'default': False, 'type': 'map', 'name': 'item6', 'value': {}, 'modified': False}], maps)
+                          {'default': True, 'type': 'map', 'name': 'item6', 'value': {}, 'modified': False}], maps)
         maps = self.mcd.get_value_maps("/Spec2/item5")
         self.assertEqual([{'default': False, 'type': 'string', 'name': 'list_element', 'value': 'a', 'modified': False},
                           {'default': False, 'type': 'string', 'name': 'list_element', 'value': 'b', 'modified': False}], maps)
+        maps = self.mcd.get_value_maps("/Spec2/item5[0]")
+        self.assertEqual([{'default': True, 'modified': False, 'name': 'list_element', 'type': 'string', 'value': 'a'}], maps)
         maps = self.mcd.get_value_maps("/Spec2/item1")
         self.assertEqual([{'default': False, 'type': 'integer', 'name': 'item1', 'value': 2, 'modified': False}], maps)
         maps = self.mcd.get_value_maps("/Spec2/item2")
-        self.assertEqual([{'default': False, 'type': 'real', 'name': 'item2', 'value': 1.1, 'modified': False}], maps)
+        self.assertEqual([{'default': True, 'type': 'real', 'name': 'item2', 'value': 1.1, 'modified': False}], maps)
         maps = self.mcd.get_value_maps("/Spec2/item3")
         self.assertEqual([{'default': False, 'type': 'boolean', 'name': 'item3', 'value': False, 'modified': True}], maps)
         maps = self.mcd.get_value_maps("/Spec2/item4")
-        self.assertEqual([{'default': False, 'type': 'string', 'name': 'item4', 'value': 'test', 'modified': False}], maps)
+        self.assertEqual([{'default': True, 'type': 'string', 'name': 'item4', 'value': 'test', 'modified': False}], maps)
 
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec24.spec")
         self.mcd.set_specification(module_spec)
@@ -429,7 +465,19 @@ class TestMultiConfigData(unittest.TestCase):
         self.mcd.set_specification(module_spec)
         self.mcd.set_value("Spec2/item1", 2)
         self.assertRaises(isc.cc.data.DataTypeError, self.mcd.set_value, "Spec2/item1", "asdf")
+
         self.mcd.set_value("Spec2/no_such_item", 4)
+        value, status = self.mcd.get_value("Spec2/no_such_item")
+        self.assertEqual(value, 4)
+        self.assertEqual(MultiConfigData.LOCAL, status)
+
+        self.mcd.set_value("Spec2/item5[0]", "c")
+        value, status = self.mcd.get_value("Spec2/item5[0]")
+        self.assertEqual(value, "c")
+        self.assertEqual(MultiConfigData.LOCAL, status)
+
+        self.assertRaises(isc.cc.data.DataTypeError, self.mcd.set_value, "Spec2/item5[a]", "asdf")
+        
 
     def test_get_config_item_list(self):
         config_items = self.mcd.get_config_item_list()
@@ -441,13 +489,15 @@ class TestMultiConfigData(unittest.TestCase):
         config_items = self.mcd.get_config_item_list(None, False)
         self.assertEqual(['Spec2'], config_items)
         config_items = self.mcd.get_config_item_list(None, True)
-        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5/', 'Spec2/item6/value1', 'Spec2/item6/value2'], config_items)
+        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5', 'Spec2/item6/value1', 'Spec2/item6/value2'], config_items)
         config_items = self.mcd.get_config_item_list("Spec2", True)
-        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5/', 'Spec2/item6/value1', 'Spec2/item6/value2'], config_items)
+        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5', 'Spec2/item6/value1', 'Spec2/item6/value2'], config_items)
         config_items = self.mcd.get_config_item_list("Spec2")
-        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5/', 'Spec2/item6/'], config_items)
+        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5', 'Spec2/item6'], config_items)
+        config_items = self.mcd.get_config_item_list("/Spec2")
+        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5', 'Spec2/item6'], config_items)
         config_items = self.mcd.get_config_item_list("Spec2", True)
-        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5/', 'Spec2/item6/value1', 'Spec2/item6/value2'], config_items)
+        self.assertEqual(['Spec2/item1', 'Spec2/item2', 'Spec2/item3', 'Spec2/item4', 'Spec2/item5', 'Spec2/item6/value1', 'Spec2/item6/value2'], config_items)
 
 if __name__ == '__main__':
     unittest.main()