BIND 10 trac1789, updated. 94077743ff440dfbcfd4a723f3fd676acbfbefe4 [1789] address review comments
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Apr 11 17:26:06 UTC 2012
The branch, trac1789 has been updated
via 94077743ff440dfbcfd4a723f3fd676acbfbefe4 (commit)
from 7a0dc75cee48aceeb218147331ddc81a05376358 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 94077743ff440dfbcfd4a723f3fd676acbfbefe4
Author: Jelte Jansen <jelte at isc.org>
Date: Wed Apr 11 16:31:45 2012 +0200
[1789] address review comments
All in xfrin memory zones handlers;
- normalize zone name and class
- provide strong exception guarantee in _set_memory_zones
- fix comment in _set_db_file
- make _auth_config_update 'protected'
And in the tests:
- extracted memory zones update tests to own class
- added tests for bad data
- added tests for normalization
-----------------------------------------------------------------------
Summary of changes:
src/bin/xfrin/tests/xfrin_test.py | 122 +++++++++++++++++++++++++++---------
src/bin/xfrin/xfrin.py.in | 63 +++++++++++++------
2 files changed, 133 insertions(+), 52 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py
index 62174ee..022c965 100644
--- a/src/bin/xfrin/tests/xfrin_test.py
+++ b/src/bin/xfrin/tests/xfrin_test.py
@@ -2577,61 +2577,64 @@ class TestXfrin(unittest.TestCase):
self.common_ixfr_setup('refresh', False)
self.assertEqual(RRType.AXFR(), self.xfr.xfrin_started_request_type)
- def test_memory_zones(self):
+class TextXfrinMemoryZones(unittest.TestCase):
+ def setUp(self):
+ self.xfr = MockXfrin()
# Configuration snippet containing 2 memory datasources,
# one for IN and one for CH. Both contain a zone 'example.com'
# the IN ds also contains a zone example2.com, and a zone example3.com,
# which is of file type 'text' (and hence, should be ignored)
- config = { 'datasources': [
- { 'type': 'memory',
- 'class': 'IN',
- 'zones': [
- { 'origin': 'example.com',
- 'filetype': 'sqlite3' },
- { 'origin': 'example2.com',
- 'filetype': 'sqlite3' },
- { 'origin': 'example3.com',
- 'filetype': 'text' }
- ]
- },
- { 'type': 'memory',
- 'class': 'CH',
- 'zones': [
- { 'origin': 'example.com',
- 'filetype': 'sqlite3' }
- ]
- }
- ] }
-
+ self.config = { 'datasources': [
+ { 'type': 'memory',
+ 'class': 'IN',
+ 'zones': [
+ { 'origin': 'example.com',
+ 'filetype': 'sqlite3' },
+ { 'origin': 'EXAMPLE2.com.',
+ 'filetype': 'sqlite3' },
+ { 'origin': 'example3.com',
+ 'filetype': 'text' }
+ ]
+ },
+ { 'type': 'memory',
+ 'class': 'CH',
+ 'zones': [
+ { 'origin': 'example.com',
+ 'filetype': 'sqlite3' }
+ ]
+ }
+ ] }
+
+ def test_updates(self):
self.assertFalse(self.xfr._is_memory_zone("example.com", "IN"))
self.assertFalse(self.xfr._is_memory_zone("example2.com", "IN"))
self.assertFalse(self.xfr._is_memory_zone("example3.com", "IN"))
self.assertFalse(self.xfr._is_memory_zone("example.com", "CH"))
# add them all
- self.xfr._set_memory_zones(config, None)
+ self.xfr._set_memory_zones(self.config, None)
self.assertTrue(self.xfr._is_memory_zone("example.com", "IN"))
self.assertTrue(self.xfr._is_memory_zone("example2.com", "IN"))
self.assertFalse(self.xfr._is_memory_zone("example3.com", "IN"))
self.assertTrue(self.xfr._is_memory_zone("example.com", "CH"))
- # Remove the CH data source from the config snippet, and update
- del config['datasources'][1]
- self.xfr._set_memory_zones(config, None)
+ # Remove the CH data source from the self.config snippet, and update
+ del self.config['datasources'][1]
+ self.xfr._set_memory_zones(self.config, None)
self.assertTrue(self.xfr._is_memory_zone("example.com", "IN"))
self.assertTrue(self.xfr._is_memory_zone("example2.com", "IN"))
self.assertFalse(self.xfr._is_memory_zone("example3.com", "IN"))
self.assertFalse(self.xfr._is_memory_zone("example.com", "CH"))
# Remove example2.com from the datasource, and update
- del config['datasources'][0]['zones'][1]
- self.xfr._set_memory_zones(config, None)
+ del self.config['datasources'][0]['zones'][1]
+ self.xfr._set_memory_zones(self.config, None)
self.assertTrue(self.xfr._is_memory_zone("example.com", "IN"))
self.assertFalse(self.xfr._is_memory_zone("example2.com", "IN"))
self.assertFalse(self.xfr._is_memory_zone("example3.com", "IN"))
self.assertFalse(self.xfr._is_memory_zone("example.com", "CH"))
- # If 'datasources' is not in the config update list (i.e. its config
+ # If 'datasources' is not in the self.config update list (i.e. its self.config
# has not changed), no difference should be found
self.xfr._set_memory_zones({}, None)
self.assertTrue(self.xfr._is_memory_zone("example.com", "IN"))
@@ -2640,13 +2643,70 @@ class TestXfrin(unittest.TestCase):
self.assertFalse(self.xfr._is_memory_zone("example.com", "CH"))
# If datasources list becomes empty, everything should be removed
- config['datasources'][0]['zones'] = []
- self.xfr._set_memory_zones(config, None)
+ self.config['datasources'][0]['zones'] = []
+ self.xfr._set_memory_zones(self.config, None)
self.assertFalse(self.xfr._is_memory_zone("example.com", "IN"))
self.assertFalse(self.xfr._is_memory_zone("example2.com", "IN"))
self.assertFalse(self.xfr._is_memory_zone("example3.com", "IN"))
self.assertFalse(self.xfr._is_memory_zone("example.com", "CH"))
+ def test_normalization(self):
+ self.xfr._set_memory_zones(self.config, None)
+ # make sure it is case insensitive, root-dot-insensitive,
+ # and supports CLASSXXX notation
+ self.assertTrue(self.xfr._is_memory_zone("EXAMPLE.com", "IN"))
+ self.assertTrue(self.xfr._is_memory_zone("example2.com.", "IN"))
+ self.assertTrue(self.xfr._is_memory_zone("example.com", "CLASS3"))
+
+ def test_bad_name(self):
+ # First set it to some config
+ self.xfr._set_memory_zones(self.config, None)
+
+ # Error checking; bad owner name should result in no changes
+ self.config['datasources'][1]['zones'][0]['origin'] = ".."
+ self.xfr._set_memory_zones(self.config, None)
+ self.assertTrue(self.xfr._is_memory_zone("example.com", "IN"))
+ self.assertTrue(self.xfr._is_memory_zone("example2.com", "IN"))
+ self.assertFalse(self.xfr._is_memory_zone("example3.com", "IN"))
+ self.assertTrue(self.xfr._is_memory_zone("example.com", "CH"))
+
+ def test_bad_class(self):
+ # First set it to some config
+ self.xfr._set_memory_zones(self.config, None)
+
+ # Error checking; bad owner name should result in no changes
+ self.config['datasources'][1]['class'] = "Foo"
+ self.xfr._set_memory_zones(self.config, None)
+ self.assertTrue(self.xfr._is_memory_zone("example.com", "IN"))
+ self.assertTrue(self.xfr._is_memory_zone("example2.com", "IN"))
+ self.assertFalse(self.xfr._is_memory_zone("example3.com", "IN"))
+ self.assertTrue(self.xfr._is_memory_zone("example.com", "CH"))
+
+ def test_no_filetype(self):
+ # omitting the filetype should leave that zone out, but not
+ # the rest
+ del self.config['datasources'][1]['zones'][0]['filetype']
+ self.xfr._set_memory_zones(self.config, None)
+ self.assertTrue(self.xfr._is_memory_zone("example.com", "IN"))
+ self.assertTrue(self.xfr._is_memory_zone("example2.com", "IN"))
+ self.assertFalse(self.xfr._is_memory_zone("example3.com", "IN"))
+ self.assertFalse(self.xfr._is_memory_zone("example.com", "CH"))
+
+ def test_class_filetype(self):
+ # omitting the class should have it default to what is in the
+ # specfile for Auth.
+ AuthConfigData = isc.config.config_data.ConfigData(
+ isc.config.module_spec_from_file(xfrin.AUTH_SPECFILE_LOCATION))
+ del self.config['datasources'][0]['class']
+ self.xfr._set_memory_zones(self.config, AuthConfigData)
+ self.assertTrue(self.xfr._is_memory_zone("example.com", "IN"))
+ self.assertTrue(self.xfr._is_memory_zone("example2.com", "IN"))
+ self.assertFalse(self.xfr._is_memory_zone("example3.com", "IN"))
+ self.assertTrue(self.xfr._is_memory_zone("example.com", "CH"))
+
+
+
+
def raise_interrupt():
raise KeyboardInterrupt()
diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in
index db66f2c..a8df637 100755
--- a/src/bin/xfrin/xfrin.py.in
+++ b/src/bin/xfrin/xfrin.py.in
@@ -1270,7 +1270,7 @@ class Xfrin:
config_data = self._module_cc.get_full_config()
self.config_handler(config_data)
self._module_cc.add_remote_config(AUTH_SPECFILE_LOCATION,
- self.auth_config_handler)
+ self._auth_config_handler)
def _cc_check_command(self):
'''This is a straightforward wrapper for cc.check_command,
@@ -1317,7 +1317,7 @@ class Xfrin:
return create_answer(0)
- def auth_config_handler(self, new_config, config_data):
+ def _auth_config_handler(self, new_config, config_data):
# Config handler for changes in Auth configuration
self._set_db_file()
self._set_memory_zones(new_config, config_data)
@@ -1333,6 +1333,13 @@ class Xfrin:
in the in-memory datasource of the Auth process with file type
'sqlite3'.
"""
+ # Normalize them first, if either conversion fails, return false
+ # (they won't be in the set anyway)
+ try:
+ zone_name_str = Name(zone_name_str).to_text().lower()
+ zone_class_str = RRClass(zone_class_str).to_text()
+ except Exception:
+ return False
return (zone_name_str, zone_class_str) in self._memory_zones
def _add_memory_zone(self, zone_name_str, zone_class_str):
@@ -1341,24 +1348,39 @@ class Xfrin:
self._memory_zones.add((zone_name_str, zone_class_str))
def _set_memory_zones(self, new_config, config_data):
- """Part of the auth_config_handler function, keeps an internal set
+ """Part of the _auth_config_handler function, keeps an internal set
of zones in the datasources config subset that have 'sqlite3' as
their file type"""
- if "datasources" in new_config:
- self._clear_memory_zones()
- for datasource in new_config["datasources"]:
- if "class" in datasource:
- ds_class = datasource["class"]
- else:
- # Get the default
- ds_class =\
- config_data.get_default_value("datasources/class")
- if datasource["type"] == "memory":
- zones = []
- for zone in datasource["zones"]:
- if zone["filetype"] == "sqlite3":
- zone_name = zone["origin"]
- self._add_memory_zone(zone_name, ds_class)
+ # walk through the data and collect the memory zones
+ # If this causes any exception, assume we were passed bad data
+ # and keep the original set
+ new_memory_zones = set()
+ try:
+ if "datasources" in new_config:
+ for datasource in new_config["datasources"]:
+ if "class" in datasource:
+ ds_class = RRClass(datasource["class"])
+ else:
+ # Get the default
+ ds_class = RRClass(config_data.get_default_value(
+ "datasources/class"))
+ if datasource["type"] == "memory":
+ for zone in datasource["zones"]:
+ if "filetype" in zone and \
+ zone["filetype"] == "sqlite3":
+ zone_name = Name(zone["origin"])
+ zone_name_str = zone_name.to_text().lower()
+ new_memory_zones.add((zone_name_str,
+ ds_class.to_text()))
+ # Ok, we can use the data, update our list
+ self._clear_memory_zones()
+ for zone_str, class_str in new_memory_zones:
+ self._add_memory_zone(zone_str, class_str)
+ except Exception as eee:
+ # Something is wrong with the data. If this data even reached us,
+ # we cannot do more than assume the real module has logged and
+ # reported an error. Keep the old set.
+ return
def shutdown(self):
''' shutdown the xfrin process. the thread which is doing xfrin should be
@@ -1503,9 +1525,8 @@ class Xfrin:
db_file, is_default =\
self._module_cc.get_remote_config_value("Auth", "database_file")
if is_default and "B10_FROM_BUILD" in os.environ:
- # this too should be unnecessary, but currently the
- # 'from build' override isn't stored in the config
- # (and we don't have writable datasources yet)
+ # override the local database setting if it is default and we
+ # are running from the source tree
db_file = os.environ["B10_FROM_BUILD"] + os.sep +\
"bind10_zones.sqlite3"
self._db_file = db_file
More information about the bind10-changes
mailing list