From: Jelte Jansen Date: Fri, 25 May 2012 10:04:25 +0000 (+0200) Subject: [1456] Add 'single-update-mode' to xfrin/diff.py X-Git-Tag: trac2351_base~226^2~67^2~4 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=f47d81ec579d815ecddf8043fa6ada63df1cae16;p=thirdparty%2Fkea.git [1456] Add 'single-update-mode' to xfrin/diff.py This mode is useful for DDNS-type updates; it allows for additions and deletions to be done in any order (keeping track of them separately). It does have some extra restrictions; SOA records must be the first deleted and the first added type, and may not occur later anymore (causes ValueError). For now, I left this in python/isc/xfrin, even though it is not purely xfrin anymore. Should we move it to datasrc? or util? --- diff --git a/src/lib/python/isc/xfrin/diff.py b/src/lib/python/isc/xfrin/diff.py index 80fa909990..b8d580b9f6 100644 --- a/src/lib/python/isc/xfrin/diff.py +++ b/src/lib/python/isc/xfrin/diff.py @@ -15,11 +15,12 @@ """ This helps the XFR in process with accumulating parts of diff and applying -it to the datasource. +it to the datasource. It also has a 'single update mode' which is useful +for DDNS. The name of the module is not yet fully decided. We might want to move it -under isc.datasrc or somewhere else, because we might want to reuse it with -future DDNS process. But until then, it lives here. +under isc.datasrc or somewhere else, because we are reusing it with DDNS. +But for now, it lives here. """ import isc.dns @@ -59,7 +60,8 @@ class Diff: the changes to underlying data source right away, but keeps them for a while. """ - def __init__(self, ds_client, zone, replace=False, journaling=False): + def __init__(self, ds_client, zone, replace=False, journaling=False, + single_update_mode=False): """ Initializes the diff to a ready state. It checks the zone exists in the datasource and if not, NoSuchZone is raised. This also creates @@ -76,6 +78,15 @@ class Diff: incoming updates but does not support journaling, the Diff object will still continue applying the diffs with disabling journaling. + If single_update_mode is true, the update is expected to only contain + 1 set of changes (i.e. one set of additions, and one set of deletions). + If so, the additions and deletions are kept separately, and applied + in one go upon commit() or apply(). In this mode, additions and + deletions can be done in any order. The first addition and the + first addition still have to be the new and old SOA records, + respectively. Once apply() or commit() has been called, this + requirement is renewed (since the diff object is essentialy reset). + You can also expect isc.datasrc.Error or isc.datasrc.NotImplemented exceptions. """ @@ -91,7 +102,12 @@ class Diff: raise NoSuchZone("Zone " + str(zone) + " does not exist in the data source " + str(ds_client)) - self.__buffer = [] + self.__single_update_mode = single_update_mode + if single_update_mode: + self.__additions = [] + self.__deletions = [] + else: + self.__buffer = [] def __check_commited(self): """ @@ -109,6 +125,14 @@ class Diff: It does all the real work of add_data and delete_data, including all checks. + + Raises a ValueError in several cases: + - if the rrset contains multiple rrs + - if the class of the rrset does not match that of the update + - in single_update_mode if the first rr is not of type SOA (both + for addition and deletion) + - in single_update_mode if any later rr is of type SOA (both for + addition and deletion) """ self.__check_commited() if rr.get_rdata_count() != 1: @@ -118,10 +142,36 @@ class Diff: raise ValueError("The rrset's class " + str(rr.get_class()) + " does not match updater's " + str(self.__updater.get_class())) - self.__buffer.append((operation, rr)) - if len(self.__buffer) >= DIFF_APPLY_TRESHOLD: - # Time to auto-apply, so the data don't accumulate too much - self.apply() + if self.__single_update_mode: + if operation == 'add': + # First addition must be SOA + if len(self.__additions) == 0 and\ + rr.get_type() != isc.dns.RRType.SOA(): + raise ValueError("First addition in single update mode " + + "must be of type SOA") + # And later additions may not + elif len(self.__additions) != 0 and\ + rr.get_type() == isc.dns.RRType.SOA(): + raise ValueError("Multiple SOA records in single " + + "update mode addition") + self.__additions.append((operation, rr)) + elif operation == 'delete': + if len(self.__deletions) == 0 and\ + rr.get_type() != isc.dns.RRType.SOA(): + raise ValueError("First deletion in single update mode " + + "must be of type SOA") + # And later deletions may not + elif len(self.__deletions) != 0 and\ + rr.get_type() == isc.dns.RRType.SOA(): + raise ValueError("Multiple SOA records in single " + + "update mode deletion") + self.__deletions.append((operation, rr)) + else: + self.__buffer.append((operation, rr)) + if len(self.__buffer) >= DIFF_APPLY_TRESHOLD: + # Time to auto-apply, so the data don't accumulate too much + # This is not done for DDNS type data + self.apply() def add_data(self, rr): """ @@ -149,6 +199,7 @@ class Diff: """ self.__data_common(rr, 'delete') + def compact(self): """ Tries to compact the operations in buffer a little by putting some of @@ -175,23 +226,34 @@ class Diff: sigdata2 = rrset2.get_rdata()[0].to_text().split()[0] return sigdata1 == sigdata2 - buf = [] - for (op, rrset) in self.__buffer: - old = buf[-1][1] if len(buf) > 0 else None - if old is None or op != buf[-1][0] or \ - rrset.get_name() != old.get_name() or \ - (not same_type(rrset, old)): - buf.append((op, isc.dns.RRset(rrset.get_name(), - rrset.get_class(), - rrset.get_type(), - rrset.get_ttl()))) - if rrset.get_ttl() != buf[-1][1].get_ttl(): - logger.warn(LIBXFRIN_DIFFERENT_TTL, rrset.get_ttl(), - buf[-1][1].get_ttl(), rrset.get_name(), - rrset.get_class(), rrset.get_type()) - for rdatum in rrset.get_rdata(): - buf[-1][1].add_rdata(rdatum) - self.__buffer = buf + def compact_buffer(buffer_to_compact): + '''Internal helper function for compacting buffers, compacts the + given buffer. + Returns the compacted buffer. + ''' + buf = [] + for (op, rrset) in buffer_to_compact: + old = buf[-1][1] if len(buf) > 0 else None + if old is None or op != buf[-1][0] or \ + rrset.get_name() != old.get_name() or \ + (not same_type(rrset, old)): + buf.append((op, isc.dns.RRset(rrset.get_name(), + rrset.get_class(), + rrset.get_type(), + rrset.get_ttl()))) + if rrset.get_ttl() != buf[-1][1].get_ttl(): + logger.warn(LIBXFRIN_DIFFERENT_TTL, rrset.get_ttl(), + buf[-1][1].get_ttl(), rrset.get_name(), + rrset.get_class(), rrset.get_type()) + for rdatum in rrset.get_rdata(): + buf[-1][1].add_rdata(rdatum) + return buf + + if self.__single_update_mode: + self.__additions = compact_buffer(self.__additions) + self.__deletions = compact_buffer(self.__deletions) + else: + self.__buffer = compact_buffer(self.__buffer) def apply(self): """ @@ -209,25 +271,41 @@ class Diff: It also can raise isc.datasrc.Error. If that happens, you should stop using this object and abort the modification. """ - self.__check_commited() - # First, compact the data - self.compact() - try: - # Then pass the data inside the data source - for (operation, rrset) in self.__buffer: + def apply_buffer(buf): + ''' + Helper method to apply all operations in the given buffer + ''' + for (operation, rrset) in buf: if operation == 'add': self.__updater.add_rrset(rrset) elif operation == 'delete': self.__updater.delete_rrset(rrset) else: raise ValueError('Unknown operation ' + operation) + + self.__check_commited() + # First, compact the data + self.compact() + try: + # Then pass the data inside the data source + if self.__single_update_mode: + apply_buffer(self.__deletions) + apply_buffer(self.__additions) + else: + apply_buffer(self.__buffer) + # As everything is already in, drop the buffer except: # If there's a problem, we can't continue. self.__updater = None raise - self.__buffer = [] + # all went well, reset state of buffers + if self.__single_update_mode: + self.__additions = [] + self.__deletions = [] + else: + self.__buffer = [] def commit(self): """ @@ -259,5 +337,29 @@ class Diff: Probably useful only for testing and introspection purposes. Don't modify the list. + + Raises a ValueError if the buffer is in single_update_mode. + """ + if self.__single_update_mode: + # This operation is only valid if the multi-update mode + raise ValueError("Compound buffer requested in single-update mode") + else: + return self.__buffer + + def get_single_update_buffers(self): + """ + Returns the current buffers of changes not yet passed into the data + source. It is a tuple of the current deletions and additions, which + each are in a form like [('add', rrset), ('delete', rrset), + ('delete', rrset), ...]. + + Probably useful only for testing and introspection purposes. Don't + modify the list. + + Raises a ValueError if the buffer is not in single_update_mode. """ - return self.__buffer + if not self.__single_update_mode: + # This operation is only valid if the multi-update mode + raise ValueError("Compound buffer requested in single-update mode") + else: + return (self.__deletions, self.__additions) diff --git a/src/lib/python/isc/xfrin/tests/diff_tests.py b/src/lib/python/isc/xfrin/tests/diff_tests.py index 7c1158af92..6c545d294f 100644 --- a/src/lib/python/isc/xfrin/tests/diff_tests.py +++ b/src/lib/python/isc/xfrin/tests/diff_tests.py @@ -478,14 +478,116 @@ class DiffTest(unittest.TestCase): diff.compact() self.assertEqual(2, len(diff.get_buffer())) - def test_relpace(self): - """ + def test_replace(self): + ''' Test that when we want to replace the whole zone, it is propagated. - """ + ''' self.__should_replace = True diff = Diff(self, "example.org.", True) self.assertTrue(self.__updater_requested) + def test_get_buffer(self): + ''' + Test that the getters raise when used in the wrong mode + ''' + diff_multi = Diff(self, Name('example.org.'), single_update_mode=False) + self.assertRaises(ValueError, diff_multi.get_single_update_buffers) + self.assertEqual([], diff_multi.get_buffer()) + + diff_single = Diff(self, Name('example.org.'), single_update_mode=True) + self.assertRaises(ValueError, diff_single.get_buffer) + self.assertEqual(([], []), diff_single.get_single_update_buffers()) + + def test_single_update_mode(self): + ''' + Test single-update mode. In this mode, updates and deletes can + be done in any order, but there may only be one changeset. + For both updates and deletes, exactly one SOA rr must be given, + and it must be the first change. + ''' + + # First create some RRsets to play with + soa = RRset(Name('example.org.'), self.__rrclass, RRType.SOA(), + RRTTL(3600)) + soa.add_rdata(Rdata(soa.get_type(), soa.get_class(), + "ns.example.org. foo.example.org. 1234 28800 "+ + "7200 604800 3600")) + + a = RRset(Name('www.example.org.'), self.__rrclass, RRType.A(), + RRTTL(3600)) + a.add_rdata(Rdata(a.get_type(), a.get_class(), + "192.0.2.1")) + + a2 = RRset(Name('www.example.org.'), self.__rrclass, RRType.A(), + RRTTL(3600)) + a2.add_rdata(Rdata(a2.get_type(), a2.get_class(), + "192.0.2.2")) + + # full rrset for A (to check compact()) + a_1_2 = RRset(Name('www.example.org.'), self.__rrclass, RRType.A(), + RRTTL(3600)) + a_1_2.add_rdata(Rdata(a_1_2.get_type(), a_1_2.get_class(), + "192.0.2.1")) + a_1_2.add_rdata(Rdata(a_1_2.get_type(), a_1_2.get_class(), + "192.0.2.2")) + + diff = Diff(self, Name('example.org.'), single_update_mode=True) + + # adding a first should fail + self.assertRaises(ValueError, diff.add_data, a) + # But soa should work + diff.add_data(soa) + # And then A should as well + diff.add_data(a) + diff.add_data(a2) + # But another SOA should fail again + self.assertRaises(ValueError, diff.add_data, soa) + + # Same for delete + self.assertRaises(ValueError, diff.delete_data, a) + diff.delete_data(soa) + diff.delete_data(a) + diff.delete_data(a2) + self.assertRaises(ValueError, diff.delete_data, soa) + + # Not compacted yet, so the buffers should be as we + # filled them + (delbuf, addbuf) = diff.get_single_update_buffers() + self.assertEqual([('delete', soa), ('delete', a), ('delete', a2)], delbuf) + self.assertEqual([('add', soa), ('add', a), ('add', a2)], addbuf) + + # Compact should compact the A records in both buffers + diff.compact() + (delbuf, addbuf) = diff.get_single_update_buffers() + # need rrset equality again :/ + self.assertEqual(2, len(delbuf)) + self.assertEqual(2, len(delbuf[0])) + self.assertEqual('delete', delbuf[0][0]) + self.assertEqual(soa.to_text(), delbuf[0][1].to_text()) + self.assertEqual(2, len(delbuf[1])) + self.assertEqual('delete', delbuf[1][0]) + self.assertEqual(a_1_2.to_text(), delbuf[1][1].to_text()) + + self.assertEqual(2, len(addbuf)) + self.assertEqual(2, len(addbuf[0])) + self.assertEqual('add', addbuf[0][0]) + self.assertEqual(soa.to_text(), addbuf[0][1].to_text()) + self.assertEqual(2, len(addbuf[1])) + self.assertEqual('add', addbuf[1][0]) + self.assertEqual(a_1_2.to_text(), addbuf[1][1].to_text()) + + # Apply should reset the buffers + diff.apply() + (delbuf, addbuf) = diff.get_single_update_buffers() + self.assertEqual([], delbuf) + self.assertEqual([], addbuf) + + # Now the change has been applied, and the buffers are cleared, + # Adding non-SOA records should fail again. + self.assertRaises(ValueError, diff.add_data, a) + self.assertRaises(ValueError, diff.delete_data, a) + + if __name__ == "__main__": isc.log.init("bind10") isc.log.resetUnitTestRootLogger()