From: Michal 'vorner' Vaner Date: Fri, 7 Oct 2011 13:29:04 +0000 (+0200) Subject: Merge #1262 X-Git-Tag: perftcpdns_before_epoll~37^2~21^2~13 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=929daeade2b98ad56b156eadd22308bd6d7f265a;p=thirdparty%2Fkea.git Merge #1262 Conflicts: src/bin/xfrin/tests/xfrin_test.py src/lib/python/isc/xfrin/diff.py src/lib/python/isc/xfrin/tests/diff_tests.py --- 929daeade2b98ad56b156eadd22308bd6d7f265a diff --cc src/bin/xfrin/tests/xfrin_test.py index baf83a36b9,0699b127bc..68a312427e --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@@ -274,18 -288,13 +298,22 @@@ class TestXfrinState(unittest.TestCase) RRTTL(3600)) self.ns_rrset.add_rdata(Rdata(RRType.NS(), TEST_RRCLASS, 'ns.example.com')) + self.a_rrset = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.A(), + RRTTL(3600)) + self.a_rrset.add_rdata(Rdata(RRType.A(), TEST_RRCLASS, '192.0.2.1')) + self.conn._datasrc_client = MockDataSourceClient() - self.conn._diff = Diff(MockDataSourceClient(), TEST_ZONE_NAME) + self.conn._diff = Diff(self.conn._datasrc_client, TEST_ZONE_NAME) +class TestXfrinStateBase(TestXfrinState): + def setUp(self): + super().setUp() + + def test_handle_rr_on_base(self): + # The base version of handle_rr() isn't supposed to be called + # directly (the argument doesn't matter in this test) + self.assertRaises(XfrinException, XfrinState().handle_rr, None) + class TestXfrinInitialSOA(TestXfrinState): def setUp(self): super().setUp() @@@ -331,13 -341,18 +360,16 @@@ class TestXfrinFirstData(TestXfrinState # the initial SOA. Should switch to AXFR. self.assertFalse(self.state.handle_rr(self.conn, self.ns_rrset)) self.assertEqual(type(XfrinAXFR()), type(self.conn.get_xfrstate())) + # The Diff for AXFR should be created at this point + self.assertNotEqual(None, self.conn._diff) def test_handle_ixfr_to_axfr_by_different_soa(self): - # Response contains two consecutive SOA but the serial of the second - # does not match the requested one. The only possible interpretation - # at this point is that it's an AXFR-compatible IXFR that only - # consists of the SOA RR. It will result in broken zone and should - # be rejected anyway, but at this point we should switch to AXFR. + # An unusual case: Response contains two consecutive SOA but the + # serial of the second does not match the requested one. See + # the documentation for XfrinFirstData.handle_rr(). self.assertFalse(self.state.handle_rr(self.conn, soa_rrset)) self.assertEqual(type(XfrinAXFR()), type(self.conn.get_xfrstate())) + self.assertNotEqual(None, self.conn._diff) def test_finish_message(self): self.assertTrue(self.state.finish_message(self.conn)) @@@ -1081,8 -1116,9 +1133,9 @@@ class TestIXFRResponse(TestXfrinConnect self.conn._handle_xfrin_responses() self.assertEqual(type(XfrinIXFREnd()), type(self.conn.get_xfrstate())) self.assertEqual([], self.conn._datasrc_client.diffs) - self.check_diffs([[('delete', begin_soa_rrset), ('add', soa_rrset)]], - self.conn._datasrc_client.committed_diffs) + check_diffs(self.assertEqual, - [[('remove', begin_soa_rrset), ('add', soa_rrset)]], ++ [[('delete', begin_soa_rrset), ('add', soa_rrset)]], + self.conn._datasrc_client.committed_diffs) def test_ixfr_response_multi_sequences(self): '''Similar to the previous case, but with multiple diff seqs. @@@ -1107,19 -1143,20 +1160,20 @@@ self.conn._handle_xfrin_responses() self.assertEqual(type(XfrinIXFREnd()), type(self.conn.get_xfrstate())) self.assertEqual([], self.conn._datasrc_client.diffs) - self.check_diffs([[('delete', begin_soa_rrset), - ('delete', self._create_a('192.0.2.1')), - ('add', self._create_soa('1231')), - ('add', self._create_a('192.0.2.2'))], - [('delete', self._create_soa('1231')), - ('delete', self._create_a('192.0.2.3')), - ('add', self._create_soa('1232')), - ('add', self._create_a('192.0.2.4'))], - [('delete', self._create_soa('1232')), - ('delete', self._create_a('192.0.2.5')), - ('add', soa_rrset), - ('add', self._create_a('192.0.2.6'))]], - self.conn._datasrc_client.committed_diffs) + check_diffs(self.assertEqual, - [[('remove', begin_soa_rrset), - ('remove', self._create_a('192.0.2.1')), ++ [[('delete', begin_soa_rrset), ++ ('delete', self._create_a('192.0.2.1')), + ('add', self._create_soa('1231')), + ('add', self._create_a('192.0.2.2'))], - [('remove', self._create_soa('1231')), - ('remove', self._create_a('192.0.2.3')), ++ [('delete', self._create_soa('1231')), ++ ('delete', self._create_a('192.0.2.3')), + ('add', self._create_soa('1232')), + ('add', self._create_a('192.0.2.4'))], - [('remove', self._create_soa('1232')), - ('remove', self._create_a('192.0.2.5')), ++ [('delete', self._create_soa('1232')), ++ ('delete', self._create_a('192.0.2.5')), + ('add', soa_rrset), + ('add', self._create_a('192.0.2.6'))]], + self.conn._datasrc_client.committed_diffs) def test_ixfr_response_multi_messages(self): '''Similar to the first case, but RRs span over multiple messages. @@@ -1133,8 -1170,9 +1187,9 @@@ answers=[soa_rrset]) self.conn._handle_xfrin_responses() self.assertEqual(type(XfrinIXFREnd()), type(self.conn.get_xfrstate())) - self.check_diffs([[('delete', begin_soa_rrset), ('add', soa_rrset)]], - self.conn._datasrc_client.committed_diffs) + check_diffs(self.assertEqual, - [[('remove', begin_soa_rrset), ('add', soa_rrset)]], ++ [[('delete', begin_soa_rrset), ('add', soa_rrset)]], + self.conn._datasrc_client.committed_diffs) def test_ixfr_response_broken(self): '''Test with a broken response. @@@ -1163,8 -1202,64 +1219,64 @@@ self._create_a('192.0.2.1')]) self.assertRaises(XfrinProtocolError, self.conn._handle_xfrin_responses) - self.check_diffs([[('delete', begin_soa_rrset), ('add', soa_rrset)]], - self.conn._datasrc_client.committed_diffs) + check_diffs(self.assertEqual, - [[('remove', begin_soa_rrset), ('add', soa_rrset)]], ++ [[('delete', begin_soa_rrset), ('add', soa_rrset)]], + self.conn._datasrc_client.committed_diffs) + + def test_ixfr_to_axfr_response(self): + '''AXFR-style IXFR response. + + It simply updates the zone's SOA one time. + + ''' + ns_rr = self._create_ns() + a_rr = self._create_a('192.0.2.1') + self.conn.reply_data = self.conn.create_response_data( + questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS, RRType.IXFR())], + answers=[soa_rrset, ns_rr, a_rr, soa_rrset]) + self.conn._handle_xfrin_responses() + self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate())) + self.assertEqual([], self.conn._datasrc_client.diffs) + # The SOA should be added exactly once, and in our implementation + # it should be added at the end of the sequence. + check_diffs(self.assertEqual, + [[('add', ns_rr), ('add', a_rr), ('add', soa_rrset)]], + self.conn._datasrc_client.committed_diffs) + + def test_ixfr_to_axfr_response_mismatch_soa(self): + '''AXFR-style IXFR response, but the two SOA are not the same. + + In the current implementation, we accept it and use the second SOA. + + ''' + ns_rr = self._create_ns() + a_rr = self._create_a('192.0.2.1') + self.conn.reply_data = self.conn.create_response_data( + questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS, RRType.IXFR())], + answers=[soa_rrset, ns_rr, a_rr, begin_soa_rrset]) + self.conn._handle_xfrin_responses() + self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate())) + self.assertEqual([], self.conn._datasrc_client.diffs) + check_diffs(self.assertEqual, + [[('add', ns_rr), ('add', a_rr), + ('add', begin_soa_rrset)]], + self.conn._datasrc_client.committed_diffs) + + def test_ixfr_to_axfr_response_extra(self): + '''Test with an extra RR after the end of AXFR-style IXFR session. + + The session should be rejected, and nothing should be committed. + + ''' + ns_rr = self._create_ns() + a_rr = self._create_a('192.0.2.1') + self.conn.reply_data = self.conn.create_response_data( + questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS, RRType.IXFR())], + answers=[soa_rrset, ns_rr, a_rr, soa_rrset, a_rr]) + self.assertRaises(XfrinProtocolError, + self.conn._handle_xfrin_responses) + self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate())) + self.assertEqual([], self.conn._datasrc_client.committed_diffs) class TestIXFRSession(TestXfrinConnection): '''Tests for a full IXFR session (query and response). @@@ -1187,8 -1282,9 +1299,9 @@@ # Check some details of the IXFR protocol processing self.assertEqual(type(XfrinIXFREnd()), type(self.conn.get_xfrstate())) - self.check_diffs([[('delete', begin_soa_rrset), ('add', soa_rrset)]], - self.conn._datasrc_client.committed_diffs) + check_diffs(self.assertEqual, - [[('remove', begin_soa_rrset), ('add', soa_rrset)]], ++ [[('delete', begin_soa_rrset), ('add', soa_rrset)]], + self.conn._datasrc_client.committed_diffs) # Check if the query was IXFR. qdata = self.conn.query_data[2:] diff --cc src/bin/xfrin/xfrin.py.in index f8b310ab08,acc747ba93..58878015cd --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@@ -318,8 -258,11 +321,11 @@@ class XfrinFirstData(XfrinState) else: logger.debug(DBG_XFRIN_TRACE, XFRIN_GOT_NONINCREMENTAL_RESP, conn.zone_str()) + # We are now goint to add RRs to the new zone. We need create + # a Diff object. It will be used throughtout the XFR session. + conn._diff = Diff(conn._datasrc_client, conn._zone_name, True) self.set_xfrstate(conn, XfrinAXFR()) - return False # need to revisit this RR in an update context + return False class XfrinIXFRDeleteSOA(XfrinState): def handle_rr(self, conn, rr): diff --cc src/lib/python/isc/xfrin/diff.py index 29ac31e082,8842fdb335..a2d9a7d759 --- a/src/lib/python/isc/xfrin/diff.py +++ b/src/lib/python/isc/xfrin/diff.py @@@ -59,19 -40,20 +59,21 @@@ class Diff the changes to underlying data source right away, but keeps them for a while. """ - def __init__(self, ds_client, zone): - def __init__(self, datasource, zone, replace=False): ++ def __init__(self, ds_client, zone, replace=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 a transaction in the data source. - The datasource is the one containing the zone. Zone is isc.dns.Name - object representing the name of the zone (its apex). If replace is true, - the content of the whole zone is wiped out before applying the diff. + The ds_client is the datasource client containing the zone. Zone is + isc.dns.Name object representing the name of the zone (its apex). ++ If replace is true, the content of the whole zone is wiped out before ++ applying the diff. You can also expect isc.datasrc.Error or isc.datasrc.NotImplemented exceptions. """ - self.__updater = ds_client.get_updater(zone, False) - self.__updater = datasource.get_updater(zone, replace) ++ self.__updater = ds_client.get_updater(zone, replace) if self.__updater is None: # The no such zone case raise NoSuchZone("Zone " + str(zone) + diff --cc src/lib/python/isc/xfrin/tests/diff_tests.py index c9cc588030,e1ba561939..9fab89007f --- a/src/lib/python/isc/xfrin/tests/diff_tests.py +++ b/src/lib/python/isc/xfrin/tests/diff_tests.py @@@ -44,8 -37,7 +44,9 @@@ class DiffTest(unittest.TestCase) self.__data_operations = [] self.__apply_called = False self.__commit_called = False + self.__broken_called = False + self.__warn_called = False + self.__should_replace = False # Some common values self.__rrclass = RRClass.IN() self.__type = RRType.A() @@@ -354,84 -323,14 +355,92 @@@ diff.compact() check() + def test_wrong_class(self): + """ + Test a wrong class of rrset is rejected. + """ + diff = Diff(self, Name('example.org.')) + rrset = RRset(Name('a.example.org.'), RRClass.CH(), RRType.NS(), + self.__ttl) + rrset.add_rdata(Rdata(RRType.NS(), RRClass.CH(), 'ns.example.org.')) + self.assertRaises(ValueError, diff.add_data, rrset) + self.assertRaises(ValueError, diff.delete_data, rrset) + + def __do_raise_test(self): + """ + Do a raise test. Expects that one of the operations is exchanged for + broken version. + """ + diff = Diff(self, Name('example.org.')) + diff.add_data(self.__rrset1) + diff.delete_data(self.__rrset2) + self.assertRaises(TestError, diff.commit) + self.assertTrue(self.__broken_called) + self.assertRaises(ValueError, diff.add_data, self.__rrset1) + self.assertRaises(ValueError, diff.delete_data, self.__rrset2) + self.assertRaises(ValueError, diff.commit) + self.assertRaises(ValueError, diff.apply) + + def test_raise_add(self): + """ + Test the exception from add_rrset is propagated and the diff can't be + used afterwards. + """ + self.add_rrset = self.__broken_operation + self.__do_raise_test() + + def test_raise_delete(self): + """ + Test the exception from delete_rrset is propagated and the diff can't be + used afterwards. + """ + self.delete_rrset = self.__broken_operation + self.__do_raise_test() + + def test_raise_commit(self): + """ + Test the exception from updater's commit gets propagated and it can't be + used afterwards. + """ + self.commit = self.__broken_operation + self.__do_raise_test() + + def test_ttl(self): + """ + Test the TTL handling. A warn function should have been called if they + differ, but that's all, it should not crash or raise. + """ + orig_logger = isc.xfrin.diff.logger + try: + isc.xfrin.diff.logger = self + diff = Diff(self, Name('example.org.')) + diff.add_data(self.__rrset1) + rrset2 = RRset(Name('a.example.org.'), self.__rrclass, + self.__type, RRTTL(120)) + rrset2.add_rdata(Rdata(self.__type, self.__rrclass, '192.10.2.2')) + diff.add_data(rrset2) + rrset2 = RRset(Name('a.example.org.'), self.__rrclass, + self.__type, RRTTL(6000)) + rrset2.add_rdata(Rdata(self.__type, self.__rrclass, '192.10.2.3')) + diff.add_data(rrset2) + # They should get compacted together and complain. + diff.compact() + self.assertEqual(1, len(diff.get_buffer())) + # The TTL stays on the first value, no matter if smaller or bigger + # ones come later. + self.assertEqual(self.__ttl, diff.get_buffer()[0][1].get_ttl()) + self.assertTrue(self.__warn_called) + finally: + isc.xfrin.diff.logger = orig_logger + + def test_relpace(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) + if __name__ == "__main__": isc.log.init("bind10") unittest.main()