From fbfc3f8bb9c8d1f597089a1ba31e941990ae93f2 Mon Sep 17 00:00:00 2001 From: Andy Hawkins Date: Fri, 12 Apr 2019 20:40:28 +0100 Subject: [PATCH 01/17] Add support for TinyDNS TXT records --- octodns/source/tinydns.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) mode change 100644 => 100755 octodns/source/tinydns.py diff --git a/octodns/source/tinydns.py b/octodns/source/tinydns.py old mode 100644 new mode 100755 index 679accb..1b6be86 --- a/octodns/source/tinydns.py +++ b/octodns/source/tinydns.py @@ -20,7 +20,7 @@ from .base import BaseSource class TinyDnsBaseSource(BaseSource): SUPPORTS_GEO = False SUPPORTS_DYNAMIC = False - SUPPORTS = set(('A', 'CNAME', 'MX', 'NS')) + SUPPORTS = set(('A', 'CNAME', 'MX', 'NS', 'TXT')) split_re = re.compile(r':+') @@ -45,6 +45,22 @@ class TinyDnsBaseSource(BaseSource): 'values': values, } + def _data_for_TXT(self, _type, records): + values = [] + + for record in records: + values.append(record[0].decode('unicode-escape')) + + try: + ttl = records[0][1] + except IndexError: + ttl = self.default_ttl + return { + 'ttl': ttl, + 'type': _type, + 'values': values, + } + def _data_for_CNAME(self, _type, records): first = records[0] try: @@ -104,6 +120,7 @@ class TinyDnsBaseSource(BaseSource): 'C': 'CNAME', '+': 'A', '@': 'MX', + '\'': 'TXT', } name_re = re.compile(r'((?P.+)\.)?{}$'.format(zone.name[:-1])) From 1892489e77b8ad99061d26bc1d24860e09588408 Mon Sep 17 00:00:00 2001 From: Andy Hawkins Date: Fri, 12 Apr 2019 20:41:02 +0100 Subject: [PATCH 02/17] Add tests for TinyDNS TXT records --- tests/test_octodns_source_tinydns.py | 19 +++++++++++++++++-- tests/zones/tinydns/example.com | 4 ++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/tests/test_octodns_source_tinydns.py b/tests/test_octodns_source_tinydns.py index d2e0e21..7c8f70d 100644 --- a/tests/test_octodns_source_tinydns.py +++ b/tests/test_octodns_source_tinydns.py @@ -20,7 +20,7 @@ class TestTinyDnsFileSource(TestCase): def test_populate_normal(self): got = Zone('example.com.', []) self.source.populate(got) - self.assertEquals(11, len(got.records)) + self.assertEquals(14, len(got.records)) expected = Zone('example.com.', []) for name, data in ( @@ -86,6 +86,21 @@ class TestTinyDnsFileSource(TestCase): 'exchange': 'smtp-2-host.example.com.', }] }), + ('', { + 'type': 'TXT', + 'ttl': 300, + 'value': 'test TXT', + }), + ('colon', { + 'type': 'TXT', + 'ttl': 300, + 'value': 'test : TXT', + }), + ('nottl', { + 'type': 'TXT', + 'ttl': 3600, + 'value': 'nottl test TXT', + }), ): record = Record.new(expected, name, data) expected.add_record(record) @@ -173,4 +188,4 @@ class TestTinyDnsFileSource(TestCase): def test_ignores_subs(self): got = Zone('example.com.', ['sub']) self.source.populate(got) - self.assertEquals(10, len(got.records)) + self.assertEquals(13, len(got.records)) diff --git a/tests/zones/tinydns/example.com b/tests/zones/tinydns/example.com index 818d974..a827204 100644 --- a/tests/zones/tinydns/example.com +++ b/tests/zones/tinydns/example.com @@ -46,3 +46,7 @@ Ccname.other.foo:www.other.foo +a1.blah-asdf.subtest.com:10.2.3.5 +a2.blah-asdf.subtest.com:10.2.3.6 +a3.asdf.subtest.com:10.2.3.7 + +'example.com:test TXT:300 +'colon.example.com:test \072 TXT:300 +'nottl.example.com:nottl test TXT From a10cab351b9d324c825644cf7d3c06bda57c0f8a Mon Sep 17 00:00:00 2001 From: Andy Hawkins Date: Fri, 12 Apr 2019 21:29:58 +0100 Subject: [PATCH 03/17] Add support for TinyDNS AAAA records --- octodns/source/tinydns.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/octodns/source/tinydns.py b/octodns/source/tinydns.py index 1b6be86..beb586f 100755 --- a/octodns/source/tinydns.py +++ b/octodns/source/tinydns.py @@ -11,6 +11,7 @@ from os import listdir from os.path import join import logging import re +import textwrap from ..record import Record from ..zone import DuplicateRecordException, SubzoneRecordException @@ -20,7 +21,7 @@ from .base import BaseSource class TinyDnsBaseSource(BaseSource): SUPPORTS_GEO = False SUPPORTS_DYNAMIC = False - SUPPORTS = set(('A', 'CNAME', 'MX', 'NS', 'TXT')) + SUPPORTS = set(('A', 'CNAME', 'MX', 'NS', 'TXT', 'AAAA')) split_re = re.compile(r':+') @@ -45,6 +46,20 @@ class TinyDnsBaseSource(BaseSource): 'values': values, } + def _data_for_AAAA(self, _type, records): + values = [] + for record in records: + values.append(u":".join(textwrap.wrap(record[0], 4))) + try: + ttl = records[0][1] + except IndexError: + ttl = self.default_ttl + return { + 'ttl': ttl, + 'type': _type, + 'values': values, + } + def _data_for_TXT(self, _type, records): values = [] @@ -121,6 +136,8 @@ class TinyDnsBaseSource(BaseSource): '+': 'A', '@': 'MX', '\'': 'TXT', + '3': 'AAAA', + '6': 'AAAA', } name_re = re.compile(r'((?P.+)\.)?{}$'.format(zone.name[:-1])) From 3b98f3e0e110611eeefea8e3ff49e8a5e047c01b Mon Sep 17 00:00:00 2001 From: Andy Hawkins Date: Fri, 12 Apr 2019 21:30:45 +0100 Subject: [PATCH 04/17] Add tests for TinyDNS AAAA records --- tests/test_octodns_source_tinydns.py | 14 ++++++++++++-- tests/zones/tinydns/example.com | 3 +++ 2 files changed, 15 insertions(+), 2 deletions(-) mode change 100644 => 100755 tests/zones/tinydns/example.com diff --git a/tests/test_octodns_source_tinydns.py b/tests/test_octodns_source_tinydns.py index 7c8f70d..4850bba 100644 --- a/tests/test_octodns_source_tinydns.py +++ b/tests/test_octodns_source_tinydns.py @@ -20,7 +20,7 @@ class TestTinyDnsFileSource(TestCase): def test_populate_normal(self): got = Zone('example.com.', []) self.source.populate(got) - self.assertEquals(14, len(got.records)) + self.assertEquals(16, len(got.records)) expected = Zone('example.com.', []) for name, data in ( @@ -101,6 +101,16 @@ class TestTinyDnsFileSource(TestCase): 'ttl': 3600, 'value': 'nottl test TXT', }), + ('ipv6-3', { + 'type': 'AAAA', + 'ttl': 300, + 'value': '2a02:1348:017c:d5d0:0024:19ff:fef3:5742', + }), + ('ipv6-6', { + 'type': 'AAAA', + 'ttl': 3600, + 'value': '2a02:1348:017c:d5d0:0024:19ff:fef3:5743', + }), ): record = Record.new(expected, name, data) expected.add_record(record) @@ -188,4 +198,4 @@ class TestTinyDnsFileSource(TestCase): def test_ignores_subs(self): got = Zone('example.com.', ['sub']) self.source.populate(got) - self.assertEquals(13, len(got.records)) + self.assertEquals(15, len(got.records)) diff --git a/tests/zones/tinydns/example.com b/tests/zones/tinydns/example.com old mode 100644 new mode 100755 index a827204..6099501 --- a/tests/zones/tinydns/example.com +++ b/tests/zones/tinydns/example.com @@ -50,3 +50,6 @@ Ccname.other.foo:www.other.foo 'example.com:test TXT:300 'colon.example.com:test \072 TXT:300 'nottl.example.com:nottl test TXT + +3ipv6-3.example.com:2a021348017cd5d0002419fffef35742:300 +6ipv6-6.example.com:2a021348017cd5d0002419fffef35743 From 799e939381f1483c0da14b533635249210db5b5c Mon Sep 17 00:00:00 2001 From: Andy Hawkins Date: Wed, 17 Apr 2019 11:34:55 +0100 Subject: [PATCH 05/17] Escape semicolons read in from TinyDNS TXT records --- octodns/source/tinydns.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/octodns/source/tinydns.py b/octodns/source/tinydns.py index beb586f..dd9a8b4 100755 --- a/octodns/source/tinydns.py +++ b/octodns/source/tinydns.py @@ -64,7 +64,8 @@ class TinyDnsBaseSource(BaseSource): values = [] for record in records: - values.append(record[0].decode('unicode-escape')) + new_value = record[0].decode('unicode-escape').replace(";", "\\;") + values.append(new_value) try: ttl = records[0][1] From c89b0dbabd14d81f3ce460b0dd34b5a652c33b22 Mon Sep 17 00:00:00 2001 From: Andy Hawkins Date: Wed, 17 Apr 2019 11:35:51 +0100 Subject: [PATCH 06/17] Add tests for escaping semicolons in TinyDNS TXT records --- tests/test_octodns_source_tinydns.py | 9 +++++++-- tests/zones/tinydns/example.com | 2 ++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/test_octodns_source_tinydns.py b/tests/test_octodns_source_tinydns.py index 4850bba..3693e17 100644 --- a/tests/test_octodns_source_tinydns.py +++ b/tests/test_octodns_source_tinydns.py @@ -20,7 +20,7 @@ class TestTinyDnsFileSource(TestCase): def test_populate_normal(self): got = Zone('example.com.', []) self.source.populate(got) - self.assertEquals(16, len(got.records)) + self.assertEquals(17, len(got.records)) expected = Zone('example.com.', []) for name, data in ( @@ -111,6 +111,11 @@ class TestTinyDnsFileSource(TestCase): 'ttl': 3600, 'value': '2a02:1348:017c:d5d0:0024:19ff:fef3:5743', }), + ('semicolon', { + 'type': 'TXT', + 'ttl': 300, + 'value': 'v=DKIM1\\; k=rsa\\; p=blah', + }), ): record = Record.new(expected, name, data) expected.add_record(record) @@ -198,4 +203,4 @@ class TestTinyDnsFileSource(TestCase): def test_ignores_subs(self): got = Zone('example.com.', ['sub']) self.source.populate(got) - self.assertEquals(15, len(got.records)) + self.assertEquals(16, len(got.records)) diff --git a/tests/zones/tinydns/example.com b/tests/zones/tinydns/example.com index 6099501..32781ca 100755 --- a/tests/zones/tinydns/example.com +++ b/tests/zones/tinydns/example.com @@ -53,3 +53,5 @@ Ccname.other.foo:www.other.foo 3ipv6-3.example.com:2a021348017cd5d0002419fffef35742:300 6ipv6-6.example.com:2a021348017cd5d0002419fffef35743 + +'semicolon.example.com:v=DKIM1; k=rsa; p=blah:300 From 29e477edbdaf50798d49c85a9caef4d253130575 Mon Sep 17 00:00:00 2001 From: Andy Hawkins Date: Fri, 26 Apr 2019 09:44:03 +0100 Subject: [PATCH 07/17] Add comment about adding ':' characters into AAAA records read from TinyDNS files --- octodns/source/tinydns.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/octodns/source/tinydns.py b/octodns/source/tinydns.py index dd9a8b4..f92e41f 100755 --- a/octodns/source/tinydns.py +++ b/octodns/source/tinydns.py @@ -47,6 +47,12 @@ class TinyDnsBaseSource(BaseSource): } def _data_for_AAAA(self, _type, records): + ''' + TinyDNS files have the ipv6 address written in full, but with the + colons removed. This inserts a colon every 4th character to make + the address correct. + ''' + values = [] for record in records: values.append(u":".join(textwrap.wrap(record[0], 4))) From 8c9b9fce89e0b762882d0ea505551c042be30b17 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 26 Apr 2019 02:43:58 -0700 Subject: [PATCH 08/17] Move method doc to comment --- octodns/source/tinydns.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/octodns/source/tinydns.py b/octodns/source/tinydns.py index f92e41f..dc2bc1b 100755 --- a/octodns/source/tinydns.py +++ b/octodns/source/tinydns.py @@ -47,14 +47,11 @@ class TinyDnsBaseSource(BaseSource): } def _data_for_AAAA(self, _type, records): - ''' - TinyDNS files have the ipv6 address written in full, but with the - colons removed. This inserts a colon every 4th character to make - the address correct. - ''' - values = [] for record in records: + # TinyDNS files have the ipv6 address written in full, but with the + # colons removed. This inserts a colon every 4th character to make + # the address correct. values.append(u":".join(textwrap.wrap(record[0], 4))) try: ttl = records[0][1] From 2df87d7dfe285eaa5330f713943df448357fe5f5 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 29 Apr 2019 08:52:56 -0700 Subject: [PATCH 09/17] Improve Route53 DELETE reliability using existing rrset --- octodns/provider/route53.py | 51 +++++++++----- tests/test_octodns_provider_route53.py | 97 ++++++++++++++++++++++---- 2 files changed, 116 insertions(+), 32 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index f5185b0..a8b47b8 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -136,7 +136,7 @@ class _Route53Record(object): values_for = getattr(self, '_values_for_{}'.format(self._type)) self.values = values_for(record) - def mod(self, action): + def mod(self, action, existing_rrsets): return { 'Action': action, 'ResourceRecordSet': { @@ -268,7 +268,7 @@ class _Route53DynamicPool(_Route53Record): self.target_name) return '{}-{}'.format(self.pool_name, self.mode) - def mod(self, action): + def mod(self, action, existing_rrsets): return { 'Action': action, 'ResourceRecordSet': { @@ -311,7 +311,7 @@ class _Route53DynamicRule(_Route53Record): def identifer(self): return '{}-{}-{}'.format(self.index, self.pool_name, self.geo) - def mod(self, action): + def mod(self, action, existing_rrsets): rrset = { 'AliasTarget': { 'DNSName': self.target_dns_name, @@ -379,7 +379,7 @@ class _Route53DynamicValue(_Route53Record): def identifer(self): return '{}-{:03d}'.format(self.pool_name, self.index) - def mod(self, action): + def mod(self, action, existing_rrsets): return { 'Action': action, 'ResourceRecordSet': { @@ -404,7 +404,7 @@ class _Route53DynamicValue(_Route53Record): class _Route53GeoDefault(_Route53Record): - def mod(self, action): + def mod(self, action, existing_rrsets): return { 'Action': action, 'ResourceRecordSet': { @@ -437,15 +437,29 @@ class _Route53GeoRecord(_Route53Record): self.health_check_id = provider.get_health_check_id(record, value, creating) - def mod(self, action): + def mod(self, action, existing_rrsets): geo = self.geo + set_identifier = geo.code + + if action == 'DELETE': + # We deleting records try and find the original rrset so that we're + # 100% sure to have the complete & accurate data (this mostly + # ensures we have the right health check id when there's multiple + # potential matches) + for existing in existing_rrsets: + if set_identifier == existing.get('SetIdentifier', None): + return { + 'Action': action, + 'ResourceRecordSet': existing, + } + rrset = { 'Name': self.fqdn, 'GeoLocation': { 'CountryCode': '*' }, 'ResourceRecords': [{'Value': v} for v in geo.values], - 'SetIdentifier': geo.code, + 'SetIdentifier': set_identifier, 'TTL': self.ttl, 'Type': self._type, } @@ -927,11 +941,11 @@ class Route53Provider(BaseProvider): len(zone.records) - before, exists) return exists - def _gen_mods(self, action, records): + def _gen_mods(self, action, records, existing_rrsets): ''' Turns `_Route53*`s in to `change_resource_record_sets` `Changes` ''' - return [r.mod(action) for r in records] + return [r.mod(action, existing_rrsets) for r in records] @property def health_checks(self): @@ -1117,15 +1131,15 @@ class Route53Provider(BaseProvider): ''' return _Route53Record.new(self, record, zone_id, creating) - def _mod_Create(self, change, zone_id): + def _mod_Create(self, change, zone_id, existing_rrsets): # New is the stuff that needs to be created new_records = self._gen_records(change.new, zone_id, creating=True) # Now is a good time to clear out any unused health checks since we # know what we'll be using going forward self._gc_health_checks(change.new, new_records) - return self._gen_mods('CREATE', new_records) + return self._gen_mods('CREATE', new_records, existing_rrsets) - def _mod_Update(self, change, zone_id): + def _mod_Update(self, change, zone_id, existing_rrsets): # See comments in _Route53Record for how the set math is made to do our # bidding here. existing_records = self._gen_records(change.existing, zone_id, @@ -1148,18 +1162,18 @@ class Route53Provider(BaseProvider): if new_record in existing_records: upserts.add(new_record) - return self._gen_mods('DELETE', deletes) + \ - self._gen_mods('CREATE', creates) + \ - self._gen_mods('UPSERT', upserts) + return self._gen_mods('DELETE', deletes, existing_rrsets) + \ + self._gen_mods('CREATE', creates, existing_rrsets) + \ + self._gen_mods('UPSERT', upserts, existing_rrsets) - def _mod_Delete(self, change, zone_id): + def _mod_Delete(self, change, zone_id, existing_rrsets): # Existing is the thing that needs to be deleted existing_records = self._gen_records(change.existing, zone_id, creating=False) # Now is a good time to clear out all the health checks since we know # we're done with them self._gc_health_checks(change.existing, []) - return self._gen_mods('DELETE', existing_records) + return self._gen_mods('DELETE', existing_records, existing_rrsets) def _extra_changes_update_needed(self, record, rrset): healthcheck_host = record.healthcheck_host @@ -1271,10 +1285,11 @@ class Route53Provider(BaseProvider): batch = [] batch_rs_count = 0 zone_id = self._get_zone_id(desired.name, True) + existing_rrsets = self._load_records(zone_id) for c in changes: # Generate the mods for this change mod_type = getattr(self, '_mod_{}'.format(c.__class__.__name__)) - mods = mod_type(c, zone_id) + mods = mod_type(c, zone_id, existing_rrsets) # Order our mods to make sure targets exist before alises point to # them and we CRUD in the desired order diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 67fcb76..4a4f90a 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -874,6 +874,25 @@ class TestRoute53Provider(TestCase): 'CallerReference': ANY, }) + list_resource_record_sets_resp = { + 'ResourceRecordSets': [{ + 'Name': 'a.unit.tests.', + 'Type': 'A', + 'GeoLocation': { + 'ContinentCode': 'NA', + }, + 'ResourceRecords': [{ + 'Value': '2.2.3.4', + }], + 'TTL': 61, + }], + 'IsTruncated': False, + 'MaxItems': '100', + } + stubber.add_response('list_resource_record_sets', + list_resource_record_sets_resp, + {'HostedZoneId': 'z42'}) + stubber.add_response('list_health_checks', { 'HealthChecks': self.health_checks, @@ -1236,7 +1255,7 @@ class TestRoute53Provider(TestCase): 'HealthCheckId': '44', }) change = Create(record) - provider._mod_Create(change, 'z43') + provider._mod_Create(change, 'z43', []) stubber.assert_no_pending_responses() # gc through _mod_Update @@ -1245,7 +1264,7 @@ class TestRoute53Provider(TestCase): }) # first record is ignored for our purposes, we have to pass something change = Update(record, record) - provider._mod_Create(change, 'z43') + provider._mod_Create(change, 'z43', []) stubber.assert_no_pending_responses() # gc through _mod_Delete, expect 3 to go away, can't check order @@ -1260,7 +1279,7 @@ class TestRoute53Provider(TestCase): 'HealthCheckId': ANY, }) change = Delete(record) - provider._mod_Delete(change, 'z43') + provider._mod_Delete(change, 'z43', []) stubber.assert_no_pending_responses() # gc only AAAA, leave the A's alone @@ -1804,40 +1823,45 @@ class TestRoute53Provider(TestCase): # _get_test_plan() returns a plan with 11 modifications, 17 RRs + @patch('octodns.provider.route53.Route53Provider._load_records') @patch('octodns.provider.route53.Route53Provider._really_apply') - def test_apply_1(self, really_apply_mock): + def test_apply_1(self, really_apply_mock, _): # 18 RRs with max of 19 should only get applied in one call provider, plan = self._get_test_plan(19) provider.apply(plan) really_apply_mock.assert_called_once() + @patch('octodns.provider.route53.Route53Provider._load_records') @patch('octodns.provider.route53.Route53Provider._really_apply') - def test_apply_2(self, really_apply_mock): + def test_apply_2(self, really_apply_mock, _): # 18 RRs with max of 17 should only get applied in two calls provider, plan = self._get_test_plan(18) provider.apply(plan) self.assertEquals(2, really_apply_mock.call_count) + @patch('octodns.provider.route53.Route53Provider._load_records') @patch('octodns.provider.route53.Route53Provider._really_apply') - def test_apply_3(self, really_apply_mock): + def test_apply_3(self, really_apply_mock, _): # with a max of seven modifications, four calls provider, plan = self._get_test_plan(7) provider.apply(plan) self.assertEquals(4, really_apply_mock.call_count) + @patch('octodns.provider.route53.Route53Provider._load_records') @patch('octodns.provider.route53.Route53Provider._really_apply') - def test_apply_4(self, really_apply_mock): + def test_apply_4(self, really_apply_mock, _): # with a max of 11 modifications, two calls provider, plan = self._get_test_plan(11) provider.apply(plan) self.assertEquals(2, really_apply_mock.call_count) + @patch('octodns.provider.route53.Route53Provider._load_records') @patch('octodns.provider.route53.Route53Provider._really_apply') - def test_apply_bad(self, really_apply_mock): + def test_apply_bad(self, really_apply_mock, _): # with a max of 1 modifications, fail provider, plan = self._get_test_plan(1) @@ -1939,6 +1963,12 @@ class TestRoute53Provider(TestCase): }], [r.data for r in record.dynamic.rules]) +class DummyProvider(object): + + def get_health_check_id(self, *args, **kwargs): + return None + + class TestRoute53Records(TestCase): existing = Zone('unit.tests.', []) record_a = Record.new(existing, '', { @@ -2005,11 +2035,6 @@ class TestRoute53Records(TestCase): e = _Route53GeoDefault(None, self.record_a, False) self.assertNotEquals(a, e) - class DummyProvider(object): - - def get_health_check_id(self, *args, **kwargs): - return None - provider = DummyProvider() f = _Route53GeoRecord(provider, self.record_a, 'NA-US', self.record_a.geo['NA-US'], False) @@ -2029,6 +2054,50 @@ class TestRoute53Records(TestCase): e.__repr__() f.__repr__() + def test_geo_delete(self): + provider = DummyProvider() + geo = _Route53GeoRecord(provider, self.record_a, 'NA-US', + self.record_a.geo['NA-US'], False) + + rrset = { + 'GeoLocation': { + 'CountryCode': 'US' + }, + 'HealthCheckId': 'x12346z', + 'Name': 'unit.tests.', + 'ResourceRecords': [{ + 'Value': '2.2.2.2' + }, { + 'Value': '3.3.3.3' + }], + 'SetIdentifier': 'NA-US', + 'TTL': 99, + 'Type': 'A' + } + + candidates = [ + # Empty, will test no SetIdentifier + {}, + { + 'SetIdentifier': 'not-a-match', + }, + rrset, + ] + + # Provide a matching rrset so that we'll just use it for the delete + # rathr than building up an almost identical one, note the way we'll + # know that we got the one we passed in is that it'll have a + # HealthCheckId and one that was created wouldn't since DummyProvider + # stubs out the lookup for them + mod = geo.mod('DELETE', candidates) + self.assertEquals('x12346z', mod['ResourceRecordSet']['HealthCheckId']) + + # If we don't provide the candidate rrsets we get back exactly what we + # put in minus the healthcheck + del rrset['HealthCheckId'] + mod = geo.mod('DELETE', []) + self.assertEquals(rrset, mod['ResourceRecordSet']) + def test_new_dynamic(self): provider = Route53Provider('test', 'abc', '123') @@ -2259,7 +2328,7 @@ class TestRoute53Records(TestCase): 'Name': '_octodns-eu-central-1-pool.unit.tests.', 'SetIdentifier': 'eu-central-1-Secondary-us-east-1', 'Type': 'A'} - }], [r.mod('CREATE') for r in route53_records]) + }], [r.mod('CREATE', []) for r in route53_records]) for route53_record in route53_records: # Smoke test stringification From e4fbcf109007c9e42ffa0ba0242c565d1754dbaf Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 29 Apr 2019 09:02:45 -0700 Subject: [PATCH 10/17] Improved Route53Provider deltion for Dynamic Value too --- octodns/provider/route53.py | 17 ++++++++-- tests/test_octodns_provider_route53.py | 43 +++++++++++++++++++++++++- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index a8b47b8..c2567bb 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -380,6 +380,19 @@ class _Route53DynamicValue(_Route53Record): return '{}-{:03d}'.format(self.pool_name, self.index) def mod(self, action, existing_rrsets): + + if action == 'DELETE': + # When deleting records try and find the original rrset so that + # we're 100% sure to have the complete & accurate data (this mostly + # ensures we have the right health check id when there's multiple + # potential matches) + for existing in existing_rrsets: + if self.identifer == existing.get('SetIdentifier', None): + return { + 'Action': action, + 'ResourceRecordSet': existing, + } + return { 'Action': action, 'ResourceRecordSet': { @@ -442,8 +455,8 @@ class _Route53GeoRecord(_Route53Record): set_identifier = geo.code if action == 'DELETE': - # We deleting records try and find the original rrset so that we're - # 100% sure to have the complete & accurate data (this mostly + # When deleting records try and find the original rrset so that + # we're 100% sure to have the complete & accurate data (this mostly # ensures we have the right health check id when there's multiple # potential matches) for existing in existing_rrsets: diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 4a4f90a..a5d00e2 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -12,7 +12,8 @@ from mock import patch from octodns.record import Create, Delete, Record, Update from octodns.provider.route53 import Route53Provider, _Route53GeoDefault, \ - _Route53GeoRecord, _Route53Record, _mod_keyer, _octal_replace + _Route53DynamicValue, _Route53GeoRecord, _Route53Record, _mod_keyer, \ + _octal_replace from octodns.zone import Zone from helpers import GeoProvider @@ -2054,6 +2055,46 @@ class TestRoute53Records(TestCase): e.__repr__() f.__repr__() + def test_dynamic_value_delete(self): + provider = DummyProvider() + geo = _Route53DynamicValue(provider, self.record_a, 'iad', '2.2.2.2', + 1, 0, False) + + rrset = { + 'HealthCheckId': 'x12346z', + 'Name': '_octodns-iad-value.unit.tests.', + 'ResourceRecords': [{ + 'Value': '2.2.2.2' + }], + 'SetIdentifier': 'iad-000', + 'TTL': 99, + 'Type': 'A', + 'Weight': 1, + } + + candidates = [ + # Empty, will test no SetIdentifier + {}, + { + 'SetIdentifier': 'not-a-match', + }, + rrset, + ] + + # Provide a matching rrset so that we'll just use it for the delete + # rathr than building up an almost identical one, note the way we'll + # know that we got the one we passed in is that it'll have a + # HealthCheckId and one that was created wouldn't since DummyProvider + # stubs out the lookup for them + mod = geo.mod('DELETE', candidates) + self.assertEquals('x12346z', mod['ResourceRecordSet']['HealthCheckId']) + + # If we don't provide the candidate rrsets we get back exactly what we + # put in minus the healthcheck + rrset['HealthCheckId'] = None + mod = geo.mod('DELETE', []) + self.assertEquals(rrset, mod['ResourceRecordSet']) + def test_geo_delete(self): provider = DummyProvider() geo = _Route53GeoRecord(provider, self.record_a, 'NA-US', From 1c60ed018bb7c5f3ae4b9e90747d8d2f57a1ea9a Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 29 Apr 2019 09:49:40 -0700 Subject: [PATCH 11/17] Make sure both set-id and name match when finding rrset --- octodns/provider/route53.py | 7 +++++-- tests/test_octodns_provider_route53.py | 11 +++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index c2567bb..e8b11fa 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -387,7 +387,8 @@ class _Route53DynamicValue(_Route53Record): # ensures we have the right health check id when there's multiple # potential matches) for existing in existing_rrsets: - if self.identifer == existing.get('SetIdentifier', None): + if self.fqdn == existing.get('Name') and \ + self.identifer == existing.get('SetIdentifier', None): return { 'Action': action, 'ResourceRecordSet': existing, @@ -453,6 +454,7 @@ class _Route53GeoRecord(_Route53Record): def mod(self, action, existing_rrsets): geo = self.geo set_identifier = geo.code + fqdn = self.fqdn if action == 'DELETE': # When deleting records try and find the original rrset so that @@ -460,7 +462,8 @@ class _Route53GeoRecord(_Route53Record): # ensures we have the right health check id when there's multiple # potential matches) for existing in existing_rrsets: - if set_identifier == existing.get('SetIdentifier', None): + if fqdn == existing.get('Name') and \ + set_identifier == existing.get('SetIdentifier', None): return { 'Action': action, 'ResourceRecordSet': existing, diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index a5d00e2..added7f 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -2075,9 +2075,15 @@ class TestRoute53Records(TestCase): candidates = [ # Empty, will test no SetIdentifier {}, + # Non-matching { 'SetIdentifier': 'not-a-match', }, + # Same set-id, different name + { + 'Name': 'not-a-match', + 'SetIdentifier': 'x12346z', + }, rrset, ] @@ -2122,6 +2128,11 @@ class TestRoute53Records(TestCase): { 'SetIdentifier': 'not-a-match', }, + # Same set-id, different name + { + 'Name': 'not-a-match', + 'SetIdentifier': 'x12346z', + }, rrset, ] From 470daa8843aa77c84924b69ce08493fc6f89cd8a Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 3 May 2019 09:16:06 -0700 Subject: [PATCH 12/17] Pass at documenting dynamic record support Bit of general doc cleanup and refactoring while I'm in here. --- docs/dynamic_records.md | 126 ++++++++++++++++++++++++++++++++++++++++ docs/geo_records.md | 101 ++++++++++++++++++++++++++++++++ docs/records.md | 102 +------------------------------- 3 files changed, 230 insertions(+), 99 deletions(-) create mode 100644 docs/dynamic_records.md create mode 100644 docs/geo_records.md diff --git a/docs/dynamic_records.md b/docs/dynamic_records.md new file mode 100644 index 0000000..8a7cd09 --- /dev/null +++ b/docs/dynamic_records.md @@ -0,0 +1,126 @@ +## Dynamic Record Support + +Dynamic records provide support for GeoDNS and weighting to records. `A` and `AAAA` are fully supported and reasonably well tested for both Dyn (via Traffic Directors) and Route53. There is preliminary support for `CNAME` records, but caution should be exercised as they have not been thoroughly tested. + +Configuring GeoDNS is complex and the details of the functionality vary widely from provider to provider. octoDNS has an opinionated view mostly to give a reasonably consistent behavior across providers which is similar to the overall philosophy and approach of octoDNS itself. It may not fit your needs or use cases, in which case please open an issue for discussion. We expect this functionality to grow and evolve over time as it's more widely used. + +### An Annotated Example + +```yaml + +--- +test: + # This is a dynamic record when used with providers that support it + dynamic: + # These are the pools of records that can be referenced and thus used by rules + pools: + apac: + # An optional fallback, if all of the records in this pool fail this pool should be tried + fallback: na + # One or more values for this pool + values: + - value: 1.1.1.1 + - value: 2.2.2.2 + eu: + fallback: na + values: + - value: 3.3.3.3 + # Weight for this value, if omitted the default is 1 + weight: 2 + - value: 4.4.4.4 + weight: 3 + na: + # Implicit fallback to the default pool (below) + values: + - value: 5.5.5.5 + - value: 6.6.6.6 + - value: 7.7.7.7 + # Rules that assign queries to pools + rules: + - geos: + # Geos used in matching queries + - AS + - OC + # The pool to service the query from + pool: apac + - geos: + - AF + - EU + pool: eu + # No geos means match all queries + - pool: na + ttl: 60 + type: A + # These values become a non-healthchecked default pool + values: + - 5.5.5.5 + - 6.6.6.6 + - 7.7.7.7 +``` + +#### Geo Codes + +Geo codes consist of one to three parts depending on the scope of the area being targeted. Examples of these look like: + +* 'NA-US-KY' - North America, United States, Kentucky +* 'NA-US' - North America, United States +* 'NA' - North America + +The first portion is the continent: + +* 'AF': 14, # Continental Africa +* 'AN': 17, # Continental Antarctica +* 'AS': 15, # Continental Asia +* 'EU': 13, # Continental Europe +* 'NA': 11, # Continental North America +* 'OC': 16, # Continental Australia/Oceania +* 'SA': 12, # Continental South America + +The second is the two-letter ISO Country Code https://en.wikipedia.org/wiki/ISO_3166-2 and the third is the ISO Country Code Subdivision as per https://en.wikipedia.org/wiki/ISO_3166-2:US. Change the code at the end for the country you are subdividing. Note that these may not always be supported depending on the providers in use. + +### Health Checks + +octoDNS will automatically configure the provider to monitor each IP and check for a 200 response for **https:///_dns**. + +These checks can be customized via the `healthcheck` configuration options. + +```yaml + +--- +test: + ... + octodns: + healthcheck: + host: my-host-name + path: /dns-health-check + port: 443 + protocol: HTTPS + ... +``` + +| Key | Description | Default | +|--|--|--| +| host | FQDN for host header and SNI | - | +| path | path to check | _dns | +| port | port to check | 443 | +| protocol | HTTP/HTTPS | HTTPS | + +#### Route53 Healtch Check Options + +| Key | Description | Default | +|--|--|--| +| measure_latency | Show latency in AWS console | true | + +```yaml + +--- + octodns: + healthcheck: + host: my-host-name + path: /dns-health-check + port: 443 + protocol: HTTPS + route53: + healthcheck: + measure_latency: false +``` diff --git a/docs/geo_records.md b/docs/geo_records.md new file mode 100644 index 0000000..e365f57 --- /dev/null +++ b/docs/geo_records.md @@ -0,0 +1,101 @@ +## Geo Record Support + +Note: Geo DNS records are still supported for the time being, but it is still strongy encouraged that you look at [Dynamic Records](/docs/dynamic_records.md) instead as they are a superset of functionality. + +GeoDNS is currently supported for `A` and `AAAA` records on the Dyn (via Traffic Directors) and Route53 providers. Records with geo information pushed to providers without support for them will be managed as non-geo records using the base values. + +Configuring GeoDNS is complex and the details of the functionality vary widely from provider to provider. OctoDNS has an opinionated view of how GeoDNS should be set up and does its best to map that to each provider's offering in a way that will result in similar behavior. It may not fit your needs or use cases, in which case please open an issue for discussion. We expect this functionality to grow and evolve over time as it's more widely used. + +The following is an example of GeoDNS with three entries NA-US-CA, NA-US-NY, OC-AU. Octodns creates another one labeled 'default' with the details for the actual A record, This default record is the failover record if the monitoring check fails. + +```yaml +--- +? '' +: type: TXT + value: v=spf1 -all +test: + geo: + NA-US-NY: + - 111.111.111.1 + NA-US-CA: + - 111.111.111.2 + OC-AU: + - 111.111.111.3 + EU: + - 111.111.111.4 + ttl: 300 + type: A + value: 111.111.111.5 +``` + + +The geo labels breakdown based on: + +1. + - 'AF': 14, # Continental Africa + - 'AN': 17, # Continental Antarctica + - 'AS': 15, # Continental Asia + - 'EU': 13, # Continental Europe + - 'NA': 11, # Continental North America + - 'OC': 16, # Continental Australia/Oceania + - 'SA': 12, # Continental South America + +2. ISO Country Code https://en.wikipedia.org/wiki/ISO_3166-2 + +3. ISO Country Code Subdivision as per https://en.wikipedia.org/wiki/ISO_3166-2:US (change the code at the end for the country you are subdividing) * these may not always be supported depending on the provider. + +So the example is saying: + +- North America - United States - New York: gets served an "A" record of 111.111.111.1 +- North America - United States - California: gets served an "A" record of 111.111.111.2 +- Oceania - Australia: Gets served an "A" record of 111.111.111.3 +- Europe: gets an "A" record of 111.111.111.4 +- Everyone else gets an "A" record of 111.111.111.5 + +### Health Checks + +Octodns will automatically set up monitors check for a 200 response for **https:///_dns**. + +These checks can be configured by adding a `healthcheck` configuration to the record: + +```yaml +--- +test: + geo: + AS: + - 1.2.3.4 + EU: + - 2.3.4.5 + octodns: + healthcheck: + host: my-host-name + path: /dns-health-check + port: 443 + protocol: HTTPS +``` + +| Key | Description | Default | +|--|--|--| +| host | FQDN for host header and SNI | - | +| path | path to check | _dns | +| port | port to check | 443 | +| protocol | HTTP/HTTPS | HTTPS | + +#### Route53 Healtch Check Options + +| Key | Description | Default | +|--|--|--| +| measure_latency | Show latency in AWS console | true | + +```yaml +--- + octodns: + healthcheck: + host: my-host-name + path: /dns-health-check + port: 443 + protocol: HTTPS + route53: + healthcheck: + measure_latency: false +``` diff --git a/docs/records.md b/docs/records.md index 9b494cf..c7ede69 100644 --- a/docs/records.md +++ b/docs/records.md @@ -20,106 +20,10 @@ Underlying provider support for each of these varies and some providers have ext Adding new record types to OctoDNS is relatively straightforward, but will require careful evaluation of each provider to determine whether or not it will be supported and the addition of code in each to handle and test the new type. -## GeoDNS support - -GeoDNS is currently supported for `A` and `AAAA` records on the Dyn (via Traffic Directors) and Route53 providers. Records with geo information pushed to providers without support for them will be managed as non-geo records using the base values. - -Configuring GeoDNS is complex and the details of the functionality vary widely from provider to provider. OctoDNS has an opinionated view of how GeoDNS should be set up and does its best to map that to each provider's offering in a way that will result in similar behavior. It may not fit your needs or use cases, in which case please open an issue for discussion. We expect this functionality to grow and evolve over time as it's more widely used. - -The following is an example of GeoDNS with three entries NA-US-CA, NA-US-NY, OC-AU. Octodns creates another one labeled 'default' with the details for the actual A record, This default record is the failover record if the monitoring check fails. - -```yaml ---- -? '' -: type: TXT - value: v=spf1 -all -test: - geo: - NA-US-NY: - - 111.111.111.1 - NA-US-CA: - - 111.111.111.2 - OC-AU: - - 111.111.111.3 - EU: - - 111.111.111.4 - ttl: 300 - type: A - value: 111.111.111.5 -``` - - -The geo labels breakdown based on: - -1. - - 'AF': 14, # Continental Africa - - 'AN': 17, # Continental Antarctica - - 'AS': 15, # Continental Asia - - 'EU': 13, # Continental Europe - - 'NA': 11, # Continental North America - - 'OC': 16, # Continental Australia/Oceania - - 'SA': 12, # Continental South America - -2. ISO Country Code https://en.wikipedia.org/wiki/ISO_3166-2 - -3. ISO Country Code Subdevision as per https://en.wikipedia.org/wiki/ISO_3166-2:US (change the code at the end for the country you are subdividing) * these may not always be supported depending on the provider. - -So the example is saying: - -- North America - United States - New York: gets served an "A" record of 111.111.111.1 -- North America - United States - California: gets served an "A" record of 111.111.111.2 -- Oceania - Australia: Gets served an "A" record of 111.111.111.3 -- Europe: gets an "A" record of 111.111.111.4 -- Everyone else gets an "A" record of 111.111.111.5 - -### Health Checks - -Octodns will automatically set up monitors for each IP and check for a 200 response for **https:///_dns**. - -These checks can be configured by adding a `healthcheck` configuration to the record: - -```yaml ---- -test: - geo: - AS: - - 1.2.3.4 - EU: - - 2.3.4.5 - octodns: - healthcheck: - host: my-host-name - path: /dns-health-check - port: 443 - protocol: HTTPS -``` - -| Key | Description | Default | -|--|--|--| -| host | FQDN for host header and SNI | - | -| path | path to check | _dns | -| port | port to check | 443 | -| protocol | HTTP/HTTPS | HTTPS | - -#### Route53 Healtch Check Options - -| Key | Description | Default | -|--|--|--| -| measure_latency | Show latency in AWS console | true | - -```yaml ---- - octodns: - healthcheck: - host: my-host-name - path: /dns-health-check - port: 443 - protocol: HTTPS - route53: - healthcheck: - measure_latency: false -``` +## Advanced Record Support (GeoDNS, Weighting) +* [Dynamic Records](/docs/dynamic_records.md) +* [Geo Records](/docs/geo_records.md) ## Config (`YamlProvider`) From f2a63e06b79f7b29121a051761d2f75f0071652d Mon Sep 17 00:00:00 2001 From: Theo Julienne Date: Mon, 6 May 2019 07:58:14 -0700 Subject: [PATCH 13/17] Apply suggestions from code review Co-Authored-By: ross --- docs/records.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/records.md b/docs/records.md index c7ede69..609383c 100644 --- a/docs/records.md +++ b/docs/records.md @@ -22,8 +22,8 @@ Adding new record types to OctoDNS is relatively straightforward, but will requi ## Advanced Record Support (GeoDNS, Weighting) -* [Dynamic Records](/docs/dynamic_records.md) -* [Geo Records](/docs/geo_records.md) +* [Dynamic Records](/docs/dynamic_records.md) - the preferred method for configuring geo-location, weights, and healthcheck based fallback between pools of services. +* [Geo Records](/docs/geo_records.md) - the original implementation of geo-location based records, now superseded by Dynamic Records (above) ## Config (`YamlProvider`) From 7668e15bb66d7b03a87a488608336bf703ce33c6 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 6 May 2019 08:36:50 -0700 Subject: [PATCH 14/17] v0.9.5 version bump and CHANGELOG updates --- CHANGELOG.md | 10 ++++++++-- octodns/__init__.py | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77bfc50..adb1f8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ -## v0.9.5 - 2019-??-?? - The big one, with all the dynamic stuff +## v0.9.5 - 2019-05-06 - The big one, with all the dynamic stuff * dynamic record support, essentially a v2 version of geo records with a lot - more flexibility and power. Also support dynamic CNAME records. + more flexibility and power. Also support dynamic CNAME records (alpha) * Route53Provider dynamic record support * DynProvider dynamic record support * SUPPORTS_DYNAMIC is an optional property, defaults to False @@ -9,7 +9,13 @@ * CloudflareProvider SRV record unpacking fix * DNSMadeEasy provider uses supports to avoid blowing up on unknown record types +* Updates to AzureProvider lib versions * Normalize MX/CNAME/ALIAS/PTR value to lower case +* SplitYamlProvider support added +* DynProvider fix for Traffic Directors association to records, explicit rather + than "looks close enough" +* TinyDNS support for TXT and AAAA records and fixes to ; escaping +* pre-commit hook requires 100% code coverage ## v0.9.4 - 2019-01-28 - The one with a bunch of stuff, before the big one diff --git a/octodns/__init__.py b/octodns/__init__.py index 6125bf1..939c293 100644 --- a/octodns/__init__.py +++ b/octodns/__init__.py @@ -3,4 +3,4 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals -__VERSION__ = '0.9.4' +__VERSION__ = '0.9.5' From d49bf262205e525a484949a2dd489c58011ff559 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Tue, 14 May 2019 20:25:14 -0700 Subject: [PATCH 15/17] Handle Route53 extra check much more thoroughly by breaking down name Also adds thorough tests --- octodns/provider/route53.py | 20 ++++++++++++++++---- tests/test_octodns_provider_route53.py | 26 +++++++++++++++++++++++--- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index e8b11fa..1516f43 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -1250,15 +1250,27 @@ class Route53Provider(BaseProvider): '%s', record.fqdn, record._type) fqdn = record.fqdn + _type = record._type # loop through all the r53 rrsets for rrset in self._load_records(zone_id): name = rrset['Name'] + # Break off the first piece of the name, it'll let us figure out if + # this is an rrset we're interested in. + maybe_meta, rest = name.split('.', 1) + + if not maybe_meta.startswith('_octodns-') or \ + not maybe_meta.endswith('-value') or \ + '-default-' in name: + # We're only interested in non-default dynamic value records, + # as that's where healthchecks live + continue + + if rest != fqdn or _type != rrset['Type']: + # rrset isn't for the current record + continue - if record._type == rrset['Type'] and name.endswith(fqdn) and \ - name.startswith('_octodns-') and '-value.' in name and \ - '-default-' not in name and \ - self._extra_changes_update_needed(record, rrset): + if self._extra_changes_update_needed(record, rrset): # no good, doesn't have the right health check, needs an update self.log.info('_extra_changes_dynamic_needs_update: ' 'health-check caused update of %s:%s', diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index added7f..dbd443f 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -1673,7 +1673,7 @@ class TestRoute53Provider(TestCase): desired.add_record(record) list_resource_record_sets_resp = { 'ResourceRecordSets': [{ - # other name + # Not dynamic value and other name 'Name': 'unit.tests.', 'Type': 'A', 'GeoLocation': { @@ -1684,7 +1684,7 @@ class TestRoute53Provider(TestCase): }], 'TTL': 61, }, { - # matching name, other type + # Not dynamic value, matching name, other type 'Name': 'a.unit.tests.', 'Type': 'AAAA', 'ResourceRecords': [{ @@ -1693,7 +1693,7 @@ class TestRoute53Provider(TestCase): 'TTL': 61, }, { # default value pool - 'Name': '_octodns-default-pool.a.unit.tests.', + 'Name': '_octodns-default-value.a.unit.tests.', 'Type': 'A', 'GeoLocation': { 'CountryCode': '*', @@ -1702,6 +1702,26 @@ class TestRoute53Provider(TestCase): 'Value': '1.2.3.4', }], 'TTL': 61, + }, { + # different record + 'Name': '_octodns-two-value.other.unit.tests.', + 'Type': 'A', + 'GeoLocation': { + 'CountryCode': '*', + }, + 'ResourceRecords': [{ + 'Value': '1.2.3.4', + }], + 'TTL': 61, + }, { + # same everything, but different type + 'Name': '_octodns-one-value.a.unit.tests.', + 'Type': 'AAAA', + 'ResourceRecords': [{ + 'Value': '2001:0db8:3c4d:0015:0000:0000:1a2f:1a4b' + }], + 'TTL': 61, + 'HealthCheckId': '42', }, { # match 'Name': '_octodns-one-value.a.unit.tests.', From ee0416de9a77064153b4283d6d4a709c4510125a Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Tue, 14 May 2019 20:32:36 -0700 Subject: [PATCH 16/17] Cover more Route53 extra check edge cases and ensure it tests what we're after --- tests/test_octodns_provider_route53.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index dbd443f..849ea2b 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -1683,6 +1683,9 @@ class TestRoute53Provider(TestCase): 'Value': '1.2.3.4', }], 'TTL': 61, + # All the non-matches have a different Id so we'll fail if they + # match + 'HealthCheckId': '33', }, { # Not dynamic value, matching name, other type 'Name': 'a.unit.tests.', @@ -1691,6 +1694,7 @@ class TestRoute53Provider(TestCase): 'Value': '2001:0db8:3c4d:0015:0000:0000:1a2f:1a4b' }], 'TTL': 61, + 'HealthCheckId': '33', }, { # default value pool 'Name': '_octodns-default-value.a.unit.tests.', @@ -1702,6 +1706,7 @@ class TestRoute53Provider(TestCase): 'Value': '1.2.3.4', }], 'TTL': 61, + 'HealthCheckId': '33', }, { # different record 'Name': '_octodns-two-value.other.unit.tests.', @@ -1713,6 +1718,7 @@ class TestRoute53Provider(TestCase): 'Value': '1.2.3.4', }], 'TTL': 61, + 'HealthCheckId': '33', }, { # same everything, but different type 'Name': '_octodns-one-value.a.unit.tests.', @@ -1721,7 +1727,16 @@ class TestRoute53Provider(TestCase): 'Value': '2001:0db8:3c4d:0015:0000:0000:1a2f:1a4b' }], 'TTL': 61, - 'HealthCheckId': '42', + 'HealthCheckId': '33', + }, { + # same everything, sub + 'Name': '_octodns-one-value.sub.a.unit.tests.', + 'Type': 'A', + 'ResourceRecords': [{ + 'Value': '1.2.3.4', + }], + 'TTL': 61, + 'HealthCheckId': '33', }, { # match 'Name': '_octodns-one-value.a.unit.tests.', From 53c2b8d19434b3049accbd7e3fdc08dddde49bec Mon Sep 17 00:00:00 2001 From: ItsAlex Date: Wed, 15 May 2019 12:30:11 +0200 Subject: [PATCH 17/17] fix: prevent digital ocean provider to crash if records type is not supported --- octodns/provider/digitalocean.py | 4 ++++ tests/fixtures/digitalocean-page-1.json | 11 +++++++++++ 2 files changed, 15 insertions(+) diff --git a/octodns/provider/digitalocean.py b/octodns/provider/digitalocean.py index 98a78ad..e192543 100644 --- a/octodns/provider/digitalocean.py +++ b/octodns/provider/digitalocean.py @@ -223,6 +223,10 @@ class DigitalOceanProvider(BaseProvider): values = defaultdict(lambda: defaultdict(list)) for record in self.zone_records(zone): _type = record['type'] + if _type not in self.SUPPORTS: + self.log.warning('populate: skipping unsupported %s record', + _type) + continue values[record['name']][record['type']].append(record) before = len(zone.records) diff --git a/tests/fixtures/digitalocean-page-1.json b/tests/fixtures/digitalocean-page-1.json index db231ba..c931411 100644 --- a/tests/fixtures/digitalocean-page-1.json +++ b/tests/fixtures/digitalocean-page-1.json @@ -1,5 +1,16 @@ { "domain_records": [{ + "id": null, + "type": "SOA", + "name": "@", + "data": null, + "priority": null, + "port": null, + "ttl": null, + "weight": null, + "flags": null, + "tag": null + }, { "id": 11189874, "type": "NS", "name": "@",