Browse Source

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

Jeremy C. Reed 13 years ago
parent
commit
a49f5b952b

+ 11 - 0
ChangeLog

@@ -1,3 +1,14 @@
+379.	[bug]		jelte
+	Configuration commands in bindctl now check for list indices if
+	the 'identifier' argument points to a child element of a list
+	item. Previously, it was possible to 'get' non-existent values
+	by leaving out the index, e.g. "config show Auth/listen_on/port,
+	which should be config show Auth/listen_on[<index>]/port, since
+	Auth/listen_on is a list. The command without an index will now
+	show an error. It is still possible to show/set the entire list
+	("config show Auth/listen_on").
+	(Trac #1649, git 003ca8597c8d0eb558b1819dbee203fda346ba77)
+
 378.	[func]		vorner
 378.	[func]		vorner
 	It possible to start authoritative server or resolver in multiple
 	It possible to start authoritative server or resolver in multiple
 	instances, to use more than one core. Configuration is described in
 	instances, to use more than one core. Configuration is described in

+ 8 - 0
src/lib/python/isc/cc/data.py

@@ -21,6 +21,7 @@
 #
 #
 
 
 import json
 import json
+import re
 
 
 class DataNotFoundError(Exception):
 class DataNotFoundError(Exception):
     """Raised if an identifier does not exist according to a spec file,
     """Raised if an identifier does not exist according to a spec file,
@@ -86,6 +87,13 @@ def split_identifier(identifier):
     id_parts[:] = (value for value in id_parts if value != "")
     id_parts[:] = (value for value in id_parts if value != "")
     return id_parts
     return id_parts
 
 
+def identifier_has_list_index(identifier):
+    """Returns True if the given identifier string has at least one
+       list index (with [I], where I is a number"""
+    return (type(identifier) == str and
+            re.search("\[\d+\]", identifier) is not None)
+
+
 def split_identifier_list_indices(identifier):
 def split_identifier_list_indices(identifier):
     """Finds list indexes in the given identifier, which are of the
     """Finds list indexes in the given identifier, which are of the
        format [integer].
        format [integer].

+ 52 - 11
src/lib/python/isc/config/config_data.py

@@ -28,6 +28,22 @@ class ConfigDataError(Exception): pass
 
 
 BIND10_CONFIG_DATA_VERSION = 2
 BIND10_CONFIG_DATA_VERSION = 2
 
 
+# Helper functions
+def spec_part_is_list(spec_part):
+    """Returns True if the given spec_part is a dict that contains a
+       list specification, and False otherwise."""
+    return (type(spec_part) == dict and 'list_item_spec' in spec_part)
+
+def spec_part_is_map(spec_part):
+    """Returns True if the given spec_part is a dict that contains a
+       map specification, and False otherwise."""
+    return (type(spec_part) == dict and 'map_item_spec' in spec_part)
+
+def spec_part_is_named_set(spec_part):
+    """Returns True if the given spec_part is a dict that contains a
+       named_set specification, and False otherwise."""
+    return (type(spec_part) == dict and 'named_map_item_spec' in spec_part)
+
 def check_type(spec_part, value):
 def check_type(spec_part, value):
     """Does nothing if the value is of the correct type given the
     """Does nothing if the value is of the correct type given the
        specification part relevant for the value. Raises an
        specification part relevant for the value. Raises an
@@ -112,9 +128,9 @@ def _get_map_or_list(spec_part):
     """Returns the list or map specification if this is a list or a
     """Returns the list or map specification if this is a list or a
        map specification part. If not, returns the given spec_part
        map specification part. If not, returns the given spec_part
        itself"""
        itself"""
-    if "map_item_spec" in spec_part:
+    if spec_part_is_map(spec_part):
         return spec_part["map_item_spec"]
         return spec_part["map_item_spec"]
-    elif "list_item_spec" in spec_part:
+    elif spec_part_is_list(spec_part):
         return spec_part["list_item_spec"]
         return spec_part["list_item_spec"]
     else:
     else:
         return spec_part
         return spec_part
@@ -134,13 +150,13 @@ def _find_spec_part_single(cur_spec, id_part):
     # list or a map, which is internally represented by a dict with
     # list or a map, which is internally represented by a dict with
     # an element 'map_item_spec', a dict with an element 'list_item_spec',
     # an element 'map_item_spec', a dict with an element 'list_item_spec',
     # or a list (when it is the 'main' config_data element of a module).
     # or a list (when it is the 'main' config_data element of a module).
-    if type(cur_spec) == dict and 'map_item_spec' in cur_spec.keys():
+    if spec_part_is_map(cur_spec):
         for cur_spec_item in cur_spec['map_item_spec']:
         for cur_spec_item in cur_spec['map_item_spec']:
             if cur_spec_item['item_name'] == id:
             if cur_spec_item['item_name'] == id:
                 return cur_spec_item
                 return cur_spec_item
         # not found
         # not found
         raise isc.cc.data.DataNotFoundError(id + " not found")
         raise isc.cc.data.DataNotFoundError(id + " not found")
-    elif type(cur_spec) == dict and 'list_item_spec' in cur_spec.keys():
+    elif spec_part_is_list(cur_spec):
         if cur_spec['item_name'] == id:
         if cur_spec['item_name'] == id:
             return cur_spec['list_item_spec']
             return cur_spec['list_item_spec']
         # not found
         # not found
@@ -156,9 +172,22 @@ def _find_spec_part_single(cur_spec, id_part):
     else:
     else:
         raise isc.cc.data.DataNotFoundError("Not a correct config specification")
         raise isc.cc.data.DataNotFoundError("Not a correct config specification")
 
 
-def find_spec_part(element, identifier):
+def find_spec_part(element, identifier, strict_identifier = True):
     """find the data definition for the given identifier
     """find the data definition for the given identifier
-       returns either a map with 'item_name' etc, or a list of those"""
+       returns either a map with 'item_name' etc, or a list of those
+       Parameters:
+       element: The specification element to start the search in
+       identifier: The element to find (relative to element above)
+       strict_identifier: If True (the default), additional checking occurs.
+                          Currently the only check is whether a list index is
+                          specified (except for the last part of the
+                          identifier)
+       Raises a DataNotFoundError if the data is not found, or if
+       strict_identifier is True and any non-final identifier parts
+       (i.e. before the last /) identify a list element and do not contain
+       an index.
+       Returns the spec element identified by the given identifier.
+    """
     if identifier == "":
     if identifier == "":
         return element
         return element
     id_parts = identifier.split("/")
     id_parts = identifier.split("/")
@@ -171,6 +200,10 @@ def find_spec_part(element, identifier):
     # always want the 'full' spec of the item
     # always want the 'full' spec of the item
     for id_part in id_parts[:-1]:
     for id_part in id_parts[:-1]:
         cur_el = _find_spec_part_single(cur_el, id_part)
         cur_el = _find_spec_part_single(cur_el, id_part)
+        if strict_identifier and spec_part_is_list(cur_el) and\
+           not isc.cc.data.identifier_has_list_index(id_part):
+            raise isc.cc.data.DataNotFoundError(id_part +
+                                                " is a list and needs an index")
         cur_el = _get_map_or_list(cur_el)
         cur_el = _get_map_or_list(cur_el)
 
 
     cur_el = _find_spec_part_single(cur_el, id_parts[-1])
     cur_el = _find_spec_part_single(cur_el, id_parts[-1])
@@ -184,12 +217,12 @@ def spec_name_list(spec, prefix="", recurse=False):
     if prefix != "" and not prefix.endswith("/"):
     if prefix != "" and not prefix.endswith("/"):
         prefix += "/"
         prefix += "/"
     if type(spec) == dict:
     if type(spec) == dict:
-        if 'map_item_spec' in spec:
+        if spec_part_is_map(spec):
             for map_el in spec['map_item_spec']:
             for map_el in spec['map_item_spec']:
                 name = map_el['item_name']
                 name = map_el['item_name']
                 if map_el['item_type'] == 'map':
                 if map_el['item_type'] == 'map':
                     name += "/"
                     name += "/"
-                if recurse and 'map_item_spec' in map_el:
+                if recurse and spec_part_is_map(map_el):
                     result.extend(spec_name_list(map_el['map_item_spec'], prefix + map_el['item_name'], recurse))
                     result.extend(spec_name_list(map_el['map_item_spec'], prefix + map_el['item_name'], recurse))
                 else:
                 else:
                     result.append(prefix + name)
                     result.append(prefix + name)
@@ -244,7 +277,12 @@ class ConfigData:
     def get_default_value(self, identifier):
     def get_default_value(self, identifier):
         """Returns the default from the specification, or None if there
         """Returns the default from the specification, or None if there
            is no default"""
            is no default"""
-        spec = find_spec_part(self.specification.get_config_spec(), identifier)
+        # We are searching for the default value, so we can set
+        # strict_identifier to false (in fact, we need to; we may not know
+        # some list indices, or they may not exist, we are looking for
+        # a default value for a reason here).
+        spec = find_spec_part(self.specification.get_config_spec(),
+                              identifier, False)
         if spec and 'item_default' in spec:
         if spec and 'item_default' in spec:
             return spec['item_default']
             return spec['item_default']
         else:
         else:
@@ -607,7 +645,7 @@ class MultiConfigData:
            Throws DataNotFoundError if the identifier is bad
            Throws DataNotFoundError if the identifier is bad
         """
         """
         result = []
         result = []
-        if not identifier:
+        if not identifier or identifier == "/":
             # No identifier, so we need the list of current modules
             # No identifier, so we need the list of current modules
             for module in self._specifications.keys():
             for module in self._specifications.keys():
                 if all:
                 if all:
@@ -619,8 +657,11 @@ class MultiConfigData:
                     entry = _create_value_map_entry(module, 'module', None)
                     entry = _create_value_map_entry(module, 'module', None)
                     result.append(entry)
                     result.append(entry)
         else:
         else:
-            if identifier[0] == '/':
+            # Strip off start and end slashes, if they are there
+            if len(identifier) > 0 and identifier[0] == '/':
                 identifier = identifier[1:]
                 identifier = identifier[1:]
+            if len(identifier) > 0 and identifier[-1] == '/':
+                identifier = identifier[:-1]
             module, sep, id = identifier.partition('/')
             module, sep, id = identifier.partition('/')
             spec = self.get_module_spec(module)
             spec = self.get_module_spec(module)
             if spec:
             if spec:

+ 53 - 2
src/lib/python/isc/config/tests/config_data_test.py

@@ -185,6 +185,43 @@ class TestConfigData(unittest.TestCase):
         spec_part = find_spec_part(config_spec, "item6/value1")
         spec_part = find_spec_part(config_spec, "item6/value1")
         self.assertEqual({'item_name': 'value1', 'item_type': 'string', 'item_optional': True, 'item_default': 'default'}, spec_part)
         self.assertEqual({'item_name': 'value1', 'item_type': 'string', 'item_optional': True, 'item_default': 'default'}, spec_part)
 
 
+    def test_find_spec_part_lists(self):
+        # A few specific tests for list data
+        module_spec = isc.config.module_spec_from_file(self.data_path +
+                                                       os.sep +
+                                                       "spec31.spec")
+        config_spec = module_spec.get_config_spec()
+
+        expected_spec_part = {'item_name': 'number',
+                              'item_type': 'integer',
+                              'item_default': 1,
+                              'item_optional': False}
+
+        # First a check for a correct fetch
+        spec_part = find_spec_part(config_spec,
+                                   "/first_list_items[0]/second_list_items[1]/"
+                                   "map_element/list1[1]/list2[1]")
+        self.assertEqual(expected_spec_part, spec_part)
+
+        # Leaving out an index should fail by default
+        self.assertRaises(isc.cc.data.DataNotFoundError,
+                          find_spec_part, config_spec,
+                          "/first_list_items[0]/second_list_items/"
+                          "map_element/list1[1]/list2[1]")
+
+        # But not for the last element
+        spec_part = find_spec_part(config_spec,
+                                   "/first_list_items[0]/second_list_items[1]/"
+                                   "map_element/list1[1]/list2")
+        self.assertEqual(expected_spec_part, spec_part)
+
+        # And also not if strict_identifier is false (third argument)
+        spec_part = find_spec_part(config_spec,
+                                   "/first_list_items[0]/second_list_items/"
+                                   "map_element/list1[1]/list2[1]", False)
+        self.assertEqual(expected_spec_part, spec_part)
+
+
     def test_spec_name_list(self):
     def test_spec_name_list(self):
         name_list = spec_name_list(self.cd.get_module_spec().get_config_spec())
         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)
@@ -483,15 +520,25 @@ class TestMultiConfigData(unittest.TestCase):
         self.assertEqual(MultiConfigData.DEFAULT, status)
         self.assertEqual(MultiConfigData.DEFAULT, status)
 
 
 
 
-
     def test_get_value_maps(self):
     def test_get_value_maps(self):
         maps = self.mcd.get_value_maps()
         maps = self.mcd.get_value_maps()
         self.assertEqual([], maps)
         self.assertEqual([], maps)
         
         
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec1.spec")
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec1.spec")
         self.mcd.set_specification(module_spec)
         self.mcd.set_specification(module_spec)
+
+        expected = [{'default': False,
+                     'type': 'module',
+                     'name': 'Spec1',
+                     'value': None,
+                     'modified': False}]
+
         maps = self.mcd.get_value_maps()
         maps = self.mcd.get_value_maps()
-        self.assertEqual([{'default': False, 'type': 'module', 'name': 'Spec1', 'value': None, 'modified': False}], maps)
+        self.assertEqual(expected, maps)
+
+        maps = self.mcd.get_value_maps("/")
+        self.assertEqual(expected, maps)
+
         maps = self.mcd.get_value_maps('Spec2')
         maps = self.mcd.get_value_maps('Spec2')
         self.assertEqual([], maps)
         self.assertEqual([], maps)
         maps = self.mcd.get_value_maps('Spec1')
         maps = self.mcd.get_value_maps('Spec1')
@@ -566,6 +613,10 @@ class TestMultiConfigData(unittest.TestCase):
         maps = self.mcd.get_value_maps("/Spec22/value9")
         maps = self.mcd.get_value_maps("/Spec22/value9")
         self.assertEqual(expected, maps)
         self.assertEqual(expected, maps)
 
 
+        # A slash at the end should not produce different output
+        maps = self.mcd.get_value_maps("/Spec22/value9/")
+        self.assertEqual(expected, maps)
+
     def test_get_value_maps_named_set(self):
     def test_get_value_maps_named_set(self):
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec32.spec")
         module_spec = isc.config.module_spec_from_file(self.data_path + os.sep + "spec32.spec")
         self.mcd.set_specification(module_spec)
         self.mcd.set_specification(module_spec)