From e3ea51959d77b11293bd4f4c2e598c79489ce05e Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 1 Apr 2019 07:02:18 -0700 Subject: [PATCH 01/16] Break Record.chunked_values up to support chunked_value --- octodns/record/__init__.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index ba0ab98..d1e9e88 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -1067,15 +1067,18 @@ class _ChunkedValuesMixin(_ValuesMixin): CHUNK_SIZE = 255 _unescaped_semicolon_re = re.compile(r'\w;') + def chunked_value(self, value): + value = value.replace('"', '\\"') + vs = [value[i:i + self.CHUNK_SIZE] + for i in range(0, len(value), self.CHUNK_SIZE)] + vs = '" "'.join(vs) + return '"{}"'.format(vs) + @property def chunked_values(self): values = [] for v in self.values: - v = v.replace('"', '\\"') - vs = [v[i:i + self.CHUNK_SIZE] - for i in range(0, len(v), self.CHUNK_SIZE)] - vs = '" "'.join(vs) - values.append('"{}"'.format(vs)) + values.append(self.chunked_value(v)) return values From fa1162592fd07ae22275d3a9d51c8ee1f618d69e Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 1 Apr 2019 07:03:10 -0700 Subject: [PATCH 02/16] pre-commit should use coverage now that it enforces 100% --- .git_hooks_pre-commit | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.git_hooks_pre-commit b/.git_hooks_pre-commit index c17906b..6b3b02e 100755 --- a/.git_hooks_pre-commit +++ b/.git_hooks_pre-commit @@ -8,4 +8,4 @@ ROOT=`dirname $GIT` . $ROOT/env/bin/activate $ROOT/script/lint -$ROOT/script/test +$ROOT/script/coverage From d610a0c920c30c5db6e59e22cb67eaeaeebca0f3 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 1 Apr 2019 07:25:52 -0700 Subject: [PATCH 03/16] Implement _Route53Record._value_convert_* --- octodns/provider/route53.py | 51 +++++++++++++++------- tests/test_octodns_provider_route53.py | 58 +++++++++++++++++--------- 2 files changed, 76 insertions(+), 33 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index d02cab7..8388ff8 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -83,6 +83,15 @@ class _Route53Record(object): return '_Route53Record<{} {} {} {}>'.format(self.fqdn, self._type, self.ttl, self.values) + def _value_convert_value(self, value, record): + return value + + _value_convert_A = _value_convert_value + _value_convert_AAAA = _value_convert_value + _value_convert_NS = _value_convert_value + _value_convert_CNAME = _value_convert_value + _value_convert_PTR = _value_convert_value + def _values_for_values(self, record): return record.values @@ -90,9 +99,11 @@ class _Route53Record(object): _values_for_AAAA = _values_for_values _values_for_NS = _values_for_values + def _value_convert_CAA(self, value, record): + return '{} {} "{}"'.format(value.flags, value.tag, value.value) + def _values_for_CAA(self, record): - return ['{} {} "{}"'.format(v.flags, v.tag, v.value) - for v in record.values] + return [self._value_convert_CAA(v, record) for v in record.values] def _values_for_value(self, record): return [record.value] @@ -100,18 +111,28 @@ class _Route53Record(object): _values_for_CNAME = _values_for_value _values_for_PTR = _values_for_value + def _value_convert_MX(self, value, record): + return '{} {}'.format(value.preference, value.exchange) + def _values_for_MX(self, record): - return ['{} {}'.format(v.preference, v.exchange) - for v in record.values] + return [self._value_convert_MX(v, record) for v in record.values] + + def _value_convert_NAPTR(self, value, record): + return '{} {} "{}" "{}" "{}" {}' \ + .format(value.order, value.preference, + value.flags if value.flags else '', + value.service if value.service else '', + value.regexp if value.regexp else '', + value.replacement) def _values_for_NAPTR(self, record): - return ['{} {} "{}" "{}" "{}" {}' - .format(v.order, v.preference, - v.flags if v.flags else '', - v.service if v.service else '', - v.regexp if v.regexp else '', - v.replacement) - for v in record.values] + return [self._value_convert_NAPTR(v, record) for v in record.values] + + def _value_convert_quoted(self, value, record): + return record.chunked_value(value) + + _value_convert_SPF = _value_convert_quoted + _value_convert_TXT = _value_convert_quoted def _values_for_quoted(self, record): return record.chunked_values @@ -119,10 +140,12 @@ class _Route53Record(object): _values_for_SPF = _values_for_quoted _values_for_TXT = _values_for_quoted + def _value_for_SRV(self, value, record): + return '{} {} {} {}'.format(value.priority, value.weight, + value.port, value.target) + def _values_for_SRV(self, record): - return ['{} {} {} {}'.format(v.priority, v.weight, v.port, - v.target) - for v in record.values] + return [self._value_for_SRV(v, record) for v in record.values] class _Route53GeoDefault(_Route53Record): diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index eaff0e7..18088a6 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -1531,32 +1531,52 @@ class TestRoute53Provider(TestCase): class TestRoute53Records(TestCase): + existing = Zone('unit.tests.', []) + record_a = Record.new(existing, '', { + 'geo': { + 'NA-US': ['2.2.2.2', '3.3.3.3'], + 'OC': ['4.4.4.4', '5.5.5.5'] + }, + 'ttl': 99, + 'type': 'A', + 'values': ['9.9.9.9'] + }) - def test_route53_record(self): - existing = Zone('unit.tests.', []) - record_a = Record.new(existing, '', { - 'geo': { - 'NA-US': ['2.2.2.2', '3.3.3.3'], - 'OC': ['4.4.4.4', '5.5.5.5'] - }, - 'ttl': 99, - 'type': 'A', - 'values': ['9.9.9.9'] + def test_value_fors(self): + route53_record = _Route53Record(None, self.record_a, False) + + for value in (None, '', 'foo', 'bar', '1.2.3.4'): + self.assertEquals(value, route53_record + ._value_convert_value(value, self.record_a)) + + record_txt = Record.new(self.existing, 'txt', { + 'ttl': 98, + 'type': 'TXT', + 'value': 'Not Important', }) - a = _Route53Record(None, record_a, False) + + # We don't really have to test the details fo chunked_value as that's + # tested elsewhere, we just need to make sure that it's plumbed up and + # working + self.assertEquals('"Not Important"', route53_record + ._value_convert_quoted(record_txt.values[0], + record_txt)) + + def test_route53_record(self): + a = _Route53Record(None, self.record_a, False) self.assertEquals(a, a) - b = _Route53Record(None, Record.new(existing, '', + b = _Route53Record(None, Record.new(self.existing, '', {'ttl': 32, 'type': 'A', 'values': ['8.8.8.8', '1.1.1.1']}), False) self.assertEquals(b, b) - c = _Route53Record(None, Record.new(existing, 'other', + c = _Route53Record(None, Record.new(self.existing, 'other', {'ttl': 99, 'type': 'A', 'values': ['9.9.9.9']}), False) self.assertEquals(c, c) - d = _Route53Record(None, Record.new(existing, '', + d = _Route53Record(None, Record.new(self.existing, '', {'ttl': 42, 'type': 'MX', 'value': { 'preference': 10, @@ -1572,7 +1592,7 @@ class TestRoute53Records(TestCase): self.assertNotEquals(a, c) # Same everything, different class is not the same - e = _Route53GeoDefault(None, record_a, False) + e = _Route53GeoDefault(None, self.record_a, False) self.assertNotEquals(a, e) class DummyProvider(object): @@ -1581,11 +1601,11 @@ class TestRoute53Records(TestCase): return None provider = DummyProvider() - f = _Route53GeoRecord(provider, record_a, 'NA-US', - record_a.geo['NA-US'], False) + f = _Route53GeoRecord(provider, self.record_a, 'NA-US', + self.record_a.geo['NA-US'], False) self.assertEquals(f, f) - g = _Route53GeoRecord(provider, record_a, 'OC', - record_a.geo['OC'], False) + g = _Route53GeoRecord(provider, self.record_a, 'OC', + self.record_a.geo['OC'], False) self.assertEquals(g, g) # Geo and non-geo are not the same, using Geo as primary to get it's From b8be28c1dc55c16449a1bf98625dc33c76644690 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 1 Apr 2019 07:40:23 -0700 Subject: [PATCH 04/16] Rework get_health_check_id, just take value to check --- octodns/provider/route53.py | 27 ++++++++++++-------------- tests/test_octodns_provider_route53.py | 12 +++++++----- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 8388ff8..67725b3 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -179,8 +179,9 @@ class _Route53GeoRecord(_Route53Record): super(_Route53GeoRecord, self).__init__(provider, record, creating) self.geo = geo - self.health_check_id = provider.get_health_check_id(record, ident, - geo, creating) + value = geo.values[0] + self.health_check_id = provider.get_health_check_id(record, value, + creating) def mod(self, action): geo = self.geo @@ -574,25 +575,22 @@ class Route53Provider(BaseProvider): .get('measure_latency', True) def _health_check_equivilent(self, host, path, protocol, port, - measure_latency, health_check, - first_value=None): + measure_latency, health_check, value=None): config = health_check['HealthCheckConfig'] return host == config['FullyQualifiedDomainName'] and \ path == config['ResourcePath'] and protocol == config['Type'] \ and port == config['Port'] and \ measure_latency == config['MeasureLatency'] and \ - (first_value is None or first_value == config['IPAddress']) + (value is None or value == config['IPAddress']) - def get_health_check_id(self, record, ident, geo, create): + def get_health_check_id(self, record, value, create): # fqdn & the first value are special, we use them to match up health # checks to their records. Route53 health checks check a single ip and # we're going to assume that ips are interchangeable to avoid # health-checking each one independently fqdn = record.fqdn - first_value = geo.values[0] - self.log.debug('get_health_check_id: fqdn=%s, type=%s, geo=%s, ' - 'first_value=%s', fqdn, record._type, ident, - first_value) + self.log.debug('get_health_check_id: fqdn=%s, type=%s, value=%s', + fqdn, record._type, value) healthcheck_host = record.healthcheck_host healthcheck_path = record.healthcheck_path @@ -614,7 +612,7 @@ class Route53Provider(BaseProvider): healthcheck_port, healthcheck_latency, health_check, - first_value=first_value): + value=value): # this is the health check we're looking for self.log.debug('get_health_check_id: found match id=%s', id) return id @@ -629,7 +627,7 @@ class Route53Provider(BaseProvider): 'EnableSNI': healthcheck_protocol == 'HTTPS', 'FailureThreshold': 6, 'FullyQualifiedDomainName': healthcheck_host, - 'IPAddress': first_value, + 'IPAddress': value, 'MeasureLatency': healthcheck_latency, 'Port': healthcheck_port, 'RequestInterval': 10, @@ -647,10 +645,9 @@ class Route53Provider(BaseProvider): self._health_checks[id] = health_check self.log.info('get_health_check_id: created id=%s, host=%s, ' 'path=%s, protocol=%s, port=%d, measure_latency=%r, ' - 'first_value=%s', - id, healthcheck_host, healthcheck_path, + 'value=%s', id, healthcheck_host, healthcheck_path, healthcheck_protocol, healthcheck_port, - healthcheck_latency, first_value) + healthcheck_latency, value) return id def _gc_health_checks(self, record, new): diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 18088a6..fd01678 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -773,7 +773,8 @@ class TestRoute53Provider(TestCase): 'AF': ['4.2.3.4'], } }) - id = provider.get_health_check_id(record, 'AF', record.geo['AF'], True) + value = record.geo['AF'].values[0] + id = provider.get_health_check_id(record, value, True) self.assertEquals('42', id) def test_health_check_create(self): @@ -859,12 +860,12 @@ class TestRoute53Provider(TestCase): }) # if not allowed to create returns none - id = provider.get_health_check_id(record, 'AF', record.geo['AF'], - False) + value = record.geo['AF'].values[0] + id = provider.get_health_check_id(record, value, False) self.assertFalse(id) # when allowed to create we do - id = provider.get_health_check_id(record, 'AF', record.geo['AF'], True) + id = provider.get_health_check_id(record, value, True) self.assertEquals('42', id) stubber.assert_no_pending_responses() @@ -965,7 +966,8 @@ class TestRoute53Provider(TestCase): } }) - id = provider.get_health_check_id(record, 'AF', record.geo['AF'], True) + value = record.geo['AF'].values[0] + id = provider.get_health_check_id(record, value, True) ml = provider.health_checks[id]['HealthCheckConfig']['MeasureLatency'] self.assertEqual(False, ml) From f83eeb0a9cd3ed4d30d48b6856e5213a0c678099 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 1 Apr 2019 09:13:57 -0700 Subject: [PATCH 05/16] Add a Name tag to Route53 healthchecks for UI purposes --- octodns/provider/route53.py | 14 ++++++++++++++ tests/test_octodns_provider_route53.py | 3 +++ 2 files changed, 17 insertions(+) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 67725b3..ed7a016 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -640,6 +640,20 @@ class Route53Provider(BaseProvider): HealthCheckConfig=config) health_check = resp['HealthCheck'] id = health_check['Id'] + + # Set a Name for the benefit of the UI + name = '{}:{} - {}'.format(record.fqdn, record._type, value) + self._conn.change_tags_for_resource(ResourceType='healthcheck', + ResourceId=id, + AddTags=[{ + 'Key': 'Name', + 'Value': name, + }]) + # Manually add it to our cache + health_check['Tags'] = { + 'Name': name + } + # store the new health check so that we'll be able to find it in the # future self._health_checks[id] = health_check diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index fd01678..00d5aab 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -841,6 +841,7 @@ class TestRoute53Provider(TestCase): 'CallerReference': ANY, 'HealthCheckConfig': health_check_config, }) + stubber.add_response('change_tags_for_resource', {}) record = Record.new(self.expected, '', { 'ttl': 61, @@ -947,6 +948,8 @@ class TestRoute53Provider(TestCase): 'CallerReference': ANY, 'HealthCheckConfig': health_check_config, }) + stubber.add_response('change_tags_for_resource', {}) + stubber.add_response('change_tags_for_resource', {}) record = Record.new(self.expected, 'a', { 'ttl': 61, From ed152ce0f34cf9e36aba4697ec308048654d4361 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 1 Apr 2019 09:33:56 -0700 Subject: [PATCH 06/16] Plumb hosted_zone_id through to _Route53Record --- octodns/provider/route53.py | 26 +++++++++++++++----------- tests/test_octodns_provider_route53.py | 6 +++--- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index ed7a016..5c942f6 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -29,7 +29,7 @@ def _octal_replace(s): class _Route53Record(object): @classmethod - def new(self, provider, record, creating): + def new(self, provider, record, hosted_zone_id, creating): ret = set() if getattr(record, 'geo', False): ret.add(_Route53GeoDefault(provider, record, creating)) @@ -698,25 +698,26 @@ class Route53Provider(BaseProvider): id) self._conn.delete_health_check(HealthCheckId=id) - def _gen_records(self, record, creating=False): + def _gen_records(self, record, zone_id, creating=False): ''' Turns an octodns.Record into one or more `_Route53*`s ''' - return _Route53Record.new(self, record, creating) + return _Route53Record.new(self, record, zone_id, creating) - def _mod_Create(self, change): + def _mod_Create(self, change, zone_id): # New is the stuff that needs to be created - new_records = self._gen_records(change.new, creating=True) + 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) - def _mod_Update(self, change): + def _mod_Update(self, change, zone_id): # See comments in _Route53Record for how the set math is made to do our # bidding here. - existing_records = self._gen_records(change.existing, creating=False) - new_records = self._gen_records(change.new, creating=True) + existing_records = self._gen_records(change.existing, zone_id, + creating=False) + 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) @@ -738,9 +739,10 @@ class Route53Provider(BaseProvider): self._gen_mods('CREATE', creates) + \ self._gen_mods('UPSERT', upserts) - def _mod_Delete(self, change): + def _mod_Delete(self, change, zone_id): # Existing is the thing that needs to be deleted - existing_records = self._gen_records(change.existing, creating=False) + 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, []) @@ -822,7 +824,9 @@ class Route53Provider(BaseProvider): batch_rs_count = 0 zone_id = self._get_zone_id(desired.name, True) for c in changes: - mods = getattr(self, '_mod_{}'.format(c.__class__.__name__))(c) + # Generate the mods for this change + mod_type = getattr(self, '_mod_{}'.format(c.__class__.__name__)) + mods = mod_type(c, zone_id) mods_rs_count = sum( [len(m['ResourceRecordSet']['ResourceRecords']) for m in mods] ) diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 00d5aab..7fe8834 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -1010,7 +1010,7 @@ class TestRoute53Provider(TestCase): 'HealthCheckId': '44', }) change = Create(record) - provider._mod_Create(change) + provider._mod_Create(change, 'z43') stubber.assert_no_pending_responses() # gc through _mod_Update @@ -1019,7 +1019,7 @@ class TestRoute53Provider(TestCase): }) # first record is ignored for our purposes, we have to pass something change = Update(record, record) - provider._mod_Create(change) + provider._mod_Create(change, 'z43') stubber.assert_no_pending_responses() # gc through _mod_Delete, expect 3 to go away, can't check order @@ -1034,7 +1034,7 @@ class TestRoute53Provider(TestCase): 'HealthCheckId': ANY, }) change = Delete(record) - provider._mod_Delete(change) + provider._mod_Delete(change, 'z43') stubber.assert_no_pending_responses() # gc only AAAA, leave the A's alone From 0a6b2e2e3bf17fcbd90748c00ef54ad95f4277fc Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 1 Apr 2019 10:09:43 -0700 Subject: [PATCH 07/16] Implement Route53Provider mod ordering via a sort This will ensure that deletes come before creates which are before upserts and that records that uses aliases always come after their target (though implicitly based on sorting types and not explicitly by looking at them.) --- octodns/provider/route53.py | 34 +++++++++ tests/test_octodns_provider_route53.py | 100 ++++++++++++++++++++----- 2 files changed, 116 insertions(+), 18 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 5c942f6..7291a6b 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -235,6 +235,35 @@ class _Route53GeoRecord(_Route53Record): self.values) +_mod_keyer_action_order = { + 'DELETE': '0', # Delete things first + 'CREATE': '1', # Then Create things + 'UPSERT': '2', # Upsert things last +} + + +def _mod_keyer(mod): + rrset = mod['ResourceRecordSet'] + action_order = _mod_keyer_action_order[mod['Action']] + + # We're sorting by 3 "columns", the action, the rrset type, and finally the + # name/id of the rrset + + if rrset.get('GeoLocation', False): + return '{}3{}'.format(action_order, rrset['SetIdentifier']) + elif rrset.get('AliasTarget', False): + # We use an alias + if rrset.get('Failover', False) == 'SECONDARY': + # We're a secondary we'll ref primaries + return '{}2{}'.format(action_order, rrset['Name']) + else: + # We're a primary we'll ref values + return '{}1{}'.format(action_order, rrset['Name']) + + # We're just a plain value, these come first + return '{}0{}'.format(action_order, rrset['Name']) + + class Route53Provider(BaseProvider): ''' AWS Route53 Provider @@ -827,6 +856,11 @@ class Route53Provider(BaseProvider): # Generate the mods for this change mod_type = getattr(self, '_mod_{}'.format(c.__class__.__name__)) mods = mod_type(c, zone_id) + + # Order our mods to make sure targets exist before alises point to + # them and we CRUD in the desired order + mods.sort(key=_mod_keyer) + mods_rs_count = sum( [len(m['ResourceRecordSet']['ResourceRecords']) for m in mods] ) diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 7fe8834..1d48891 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -12,7 +12,7 @@ from mock import patch from octodns.record import Create, Delete, Record, Update from octodns.provider.route53 import Route53Provider, _Route53GeoDefault, \ - _Route53GeoRecord, _Route53Record, _octal_replace + _Route53GeoRecord, _Route53Record, _mod_keyer, _octal_replace from octodns.zone import Zone from helpers import GeoProvider @@ -529,23 +529,23 @@ class TestRoute53Provider(TestCase): }, { 'Action': 'UPSERT', 'ResourceRecordSet': { - 'GeoLocation': {'CountryCode': '*'}, + 'GeoLocation': {'CountryCode': 'US'}, + 'HealthCheckId': u'43', 'Name': 'unit.tests.', - 'ResourceRecords': [{'Value': '2.2.3.4'}, - {'Value': '3.2.3.4'}], - 'SetIdentifier': 'default', + 'ResourceRecords': [{'Value': '5.2.3.4'}, + {'Value': '6.2.3.4'}], + 'SetIdentifier': 'NA-US', 'TTL': 61, 'Type': 'A' } }, { 'Action': 'UPSERT', 'ResourceRecordSet': { - 'GeoLocation': {'CountryCode': 'US'}, - 'HealthCheckId': u'43', + 'GeoLocation': {'CountryCode': '*'}, 'Name': 'unit.tests.', - 'ResourceRecords': [{'Value': '5.2.3.4'}, - {'Value': '6.2.3.4'}], - 'SetIdentifier': 'NA-US', + 'ResourceRecords': [{'Value': '2.2.3.4'}, + {'Value': '3.2.3.4'}], + 'SetIdentifier': 'default', 'TTL': 61, 'Type': 'A' } @@ -588,21 +588,21 @@ class TestRoute53Provider(TestCase): 'Changes': [{ 'Action': 'DELETE', 'ResourceRecordSet': { - 'GeoLocation': {'CountryCode': '*'}, + 'GeoLocation': {'ContinentCode': 'OC'}, 'Name': 'simple.unit.tests.', - 'ResourceRecords': [{'Value': '1.2.3.4'}, - {'Value': '2.2.3.4'}], - 'SetIdentifier': 'default', + 'ResourceRecords': [{'Value': '3.2.3.4'}, + {'Value': '4.2.3.4'}], + 'SetIdentifier': 'OC', 'TTL': 61, 'Type': 'A'} }, { 'Action': 'DELETE', 'ResourceRecordSet': { - 'GeoLocation': {'ContinentCode': 'OC'}, + 'GeoLocation': {'CountryCode': '*'}, 'Name': 'simple.unit.tests.', - 'ResourceRecords': [{'Value': '3.2.3.4'}, - {'Value': '4.2.3.4'}], - 'SetIdentifier': 'OC', + 'ResourceRecords': [{'Value': '1.2.3.4'}, + {'Value': '2.2.3.4'}], + 'SetIdentifier': 'default', 'TTL': 61, 'Type': 'A'} }, { @@ -1623,3 +1623,67 @@ class TestRoute53Records(TestCase): a.__repr__() e.__repr__() f.__repr__() + + +class TestModKeyer(TestCase): + + def test_mod_keyer(self): + + # First "column" + + # Deletes come first + self.assertEquals('00something', _mod_keyer({ + 'Action': 'DELETE', + 'ResourceRecordSet': { + 'Name': 'something', + } + })) + + # Creates come next + self.assertEquals('10another', _mod_keyer({ + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'Name': 'another', + } + })) + + # Then upserts + self.assertEquals('20last', _mod_keyer({ + 'Action': 'UPSERT', + 'ResourceRecordSet': { + 'Name': 'last', + } + })) + + # Second "column" value records tested above + + # AliasTarget primary second (to value) + self.assertEquals('01thing', _mod_keyer({ + 'Action': 'DELETE', + 'ResourceRecordSet': { + 'AliasTarget': 'some-target', + 'Failover': 'PRIMARY', + 'Name': 'thing', + } + })) + + # AliasTarget secondary third + self.assertEquals('02thing', _mod_keyer({ + 'Action': 'DELETE', + 'ResourceRecordSet': { + 'AliasTarget': 'some-target', + 'Failover': 'SECONDARY', + 'Name': 'thing', + } + })) + + # GeoLocation fourth + self.assertEquals('03some-id', _mod_keyer({ + 'Action': 'DELETE', + 'ResourceRecordSet': { + 'GeoLocation': 'some-target', + 'SetIdentifier': 'some-id', + } + })) + + # The third "column" has already been tested above, Name/SetIdentifier From 7b59eedc44246f5fd4e0388b30ab425a2a69ac37 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 1 Apr 2019 14:17:59 -0700 Subject: [PATCH 08/16] Route53Provider dynamic support --- octodns/provider/route53.py | 365 +++++++++++++++++- tests/test_octodns_provider_route53.py | 502 ++++++++++++++++++++++++- 2 files changed, 847 insertions(+), 20 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 7291a6b..a3187b9 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -14,6 +14,7 @@ import logging import re from ..record import Record, Update +from ..record.geo import GeoCodes from .base import BaseProvider @@ -29,19 +30,84 @@ def _octal_replace(s): class _Route53Record(object): @classmethod - def new(self, provider, record, hosted_zone_id, creating): + def _new_dynamic(cls, provider, record, hosted_zone_id, creating): ret = set() - if getattr(record, 'geo', False): - ret.add(_Route53GeoDefault(provider, record, creating)) - for ident, geo in record.geo.items(): - ret.add(_Route53GeoRecord(provider, record, ident, geo, - creating)) - else: - ret.add(_Route53Record(provider, record, creating)) + + # HostedZoneId wants just the last bit, but the place we're getting + # this from looks like /hostedzone/Z424CArX3BB224 + hosted_zone_id = hosted_zone_id.split('/', 2)[-1] + + # Create the default pool + fqdn = record.fqdn + ret.add(_Route53Record(provider, record, creating, + '_octodns-default-pool.{}'.format(fqdn))) + + # Pools + for pool_name, pool in record.dynamic.pools.items(): + + # Create the primary + ret.add(_Route53DynamicPool(provider, hosted_zone_id, record, + pool_name, creating)) + + # Create the fallback + fallback = pool.data.get('fallback', False) + if fallback: + # We have an explicitly configured fallback + ret.add(_Route53DynamicPool(provider, hosted_zone_id, record, + pool_name, creating, + target_name=fallback)) + else: + # We fallback on the default + ret.add(_Route53DynamicPool(provider, hosted_zone_id, record, + pool_name, creating, + target_name='default')) + + # Create the values + for i, value in enumerate(pool.data['values']): + weight = value['weight'] + value = value['value'] + ret.add(_Route53DynamicValue(provider, record, pool_name, + value, weight, i, creating)) + + # Rules + for i, rule in enumerate(record.dynamic.rules): + pool_name = rule.data['pool'] + geos = rule.data.get('geos', []) + if geos: + for geo in geos: + ret.add(_Route53DynamicRule(provider, hosted_zone_id, + record, pool_name, i, + creating, geo=geo)) + else: + ret.add(_Route53DynamicRule(provider, hosted_zone_id, record, + pool_name, i, creating)) + return ret - def __init__(self, provider, record, creating): - self.fqdn = record.fqdn + @classmethod + def _new_geo(cls, provider, record, creating): + ret = set() + + ret.add(_Route53GeoDefault(provider, record, creating)) + for ident, geo in record.geo.items(): + ret.add(_Route53GeoRecord(provider, record, ident, geo, + creating)) + + return ret + + @classmethod + def new(cls, provider, record, hosted_zone_id, creating): + + if getattr(record, 'dynamic', False): + ret = cls._new_dynamic(provider, record, hosted_zone_id, creating) + return ret + elif getattr(record, 'geo', False): + return cls._new_geo(provider, record, creating) + + return set((_Route53Record(provider, record, creating),)) + + def __init__(self, provider, record, creating, fqdn_override=None): + self.fqdn = fqdn_override or record.fqdn self._type = record._type self.ttl = record.ttl @@ -148,6 +214,172 @@ class _Route53Record(object): return [self._value_for_SRV(v, record) for v in record.values] +class _Route53DynamicPool(_Route53Record): + + def __init__(self, provider, hosted_zone_id, record, pool_name, creating, + target_name=None): + fqdn_override = '_octodns-{}-pool.{}'.format(pool_name, record.fqdn) + super(_Route53DynamicPool, self) \ + .__init__(provider, record, creating, fqdn_override=fqdn_override) + + self.hosted_zone_id = hosted_zone_id + self.pool_name = pool_name + + self.target_name = target_name + if target_name: + # We're pointing down the chain + self.target_dns_name = '_octodns-{}-pool.{}'.format(target_name, + record.fqdn) + else: + # We're a paimary, point at our values + self.target_dns_name = '_octodns-{}-value.{}'.format(pool_name, + record.fqdn) + + @property + def mode(self): + return 'Secondary' if self.target_name else 'Primary' + + @property + def identifer(self): + if self.target_name: + return '{}-{}-{}'.format(self.pool_name, self.mode, + self.target_name) + return '{}-{}'.format(self.pool_name, self.mode) + + def mod(self, action): + return { + 'Action': action, + 'ResourceRecordSet': { + 'AliasTarget': { + 'DNSName': self.target_dns_name, + 'EvaluateTargetHealth': True, + 'HostedZoneId': self.hosted_zone_id, + }, + 'Failover': 'SECONDARY' if self.target_name else 'PRIMARY', + 'Name': self.fqdn, + 'SetIdentifier': self.identifer, + 'Type': self._type, + } + } + + def __hash__(self): + return '{}:{}:{}'.format(self.fqdn, self._type, + self.identifer).__hash__() + + def __repr__(self): + return '_Route53DynamicPool<{} {} {} {}>' \ + .format(self.fqdn, self._type, self.mode, self.target_dns_name) + + +class _Route53DynamicRule(_Route53Record): + + def __init__(self, provider, hosted_zone_id, record, pool_name, index, + creating, geo=None): + super(_Route53DynamicRule, self).__init__(provider, record, creating) + + self.hosted_zone_id = hosted_zone_id + self.geo = geo + self.pool_name = pool_name + self.index = index + + self.target_dns_name = '_octodns-{}-pool.{}'.format(pool_name, + record.fqdn) + + @property + def identifer(self): + return '{}-{}-{}'.format(self.index, self.pool_name, self.geo) + + def mod(self, action): + rrset = { + 'AliasTarget': { + 'DNSName': self.target_dns_name, + 'EvaluateTargetHealth': True, + 'HostedZoneId': self.hosted_zone_id, + }, + 'GeoLocation': { + 'CountryCode': '*' + }, + 'Name': self.fqdn, + 'SetIdentifier': self.identifer, + 'Type': self._type, + } + + if self.geo: + geo = GeoCodes.parse(self.geo) + + if geo['province_code']: + rrset['GeoLocation'] = { + 'CountryCode': geo['country_code'], + 'SubdivisionCode': geo['province_code'], + } + elif geo['country_code']: + rrset['GeoLocation'] = { + 'CountryCode': geo['country_code'] + } + else: + rrset['GeoLocation'] = { + 'ContinentCode': geo['continent_code'], + } + + return { + 'Action': action, + 'ResourceRecordSet': rrset, + } + + def __hash__(self): + return '{}:{}:{}'.format(self.fqdn, self._type, + self.identifer).__hash__() + + def __repr__(self): + return '_Route53DynamicRule<{} {} {} {} {}>' \ + .format(self.fqdn, self._type, self.index, self.geo, + self.target_dns_name) + + +class _Route53DynamicValue(_Route53Record): + + def __init__(self, provider, record, pool_name, value, weight, index, + creating): + fqdn_override = '_octodns-{}-value.{}'.format(pool_name, record.fqdn) + super(_Route53DynamicValue, self).__init__(provider, record, creating, + fqdn_override=fqdn_override) + + self.pool_name = pool_name + self.index = index + value_convert = getattr(self, '_value_convert_{}'.format(record._type)) + self.value = value_convert(value, record) + self.weight = weight + + self.health_check_id = provider.get_health_check_id(record, self.value, + creating) + + @property + def identifer(self): + return '{}-{:03d}'.format(self.pool_name, self.index) + + def mod(self, action): + return { + 'Action': action, + 'ResourceRecordSet': { + 'HealthCheckId': self.health_check_id, + 'Name': self.fqdn, + 'ResourceRecords': [{'Value': self.value}], + 'SetIdentifier': self.identifer, + 'TTL': self.ttl, + 'Type': self._type, + 'Weight': self.weight, + } + } + + def __hash__(self): + return '{}:{}:{}'.format(self.fqdn, self._type, + self.identifer).__hash__() + + def __repr__(self): + return '_Route53DynamicValue<{} {} {} {}>' \ + .format(self.fqdn, self._type, self.identifer, self.value) + + class _Route53GeoDefault(_Route53Record): def mod(self, action): @@ -285,8 +517,7 @@ class Route53Provider(BaseProvider): In general the account used will need full permissions on Route53. ''' SUPPORTS_GEO = True - # TODO: dynamic - SUPPORTS_DYNAMIC = False + SUPPORTS_DYNAMIC = True SUPPORTS = set(('A', 'AAAA', 'CAA', 'CNAME', 'MX', 'NAPTR', 'NS', 'PTR', 'SPF', 'SRV', 'TXT')) @@ -518,6 +749,76 @@ class Route53Provider(BaseProvider): return self._r53_rrsets[zone_id] + def _data_for_dynamic(self, name, _type, rrsets): + pools = defaultdict(lambda: {'values': []}) + # Data to build our rules will be collected here and "converted" into + # their final form below + rules = defaultdict(lambda: {'pool': None, 'geos': []}) + # Base/empty data + data = { + 'dynamic': { + 'pools': pools, + 'rules': [], + } + } + + # For all the rrsets that comprise this dynamic record + for rrset in rrsets: + name = rrset['Name'] + if '-pool.' in name: + # This is a pool rrset + pool_name = name.split('.', 1)[0][9:-5] + if pool_name == 'default': + # default becomes the base for the record and its + # value(s) will fill the non-dynamic values + data_for = getattr(self, '_data_for_{}'.format(_type)) + data.update(data_for(rrset)) + elif rrset['Failover'] == 'SECONDARY': + # This is a failover record, we'll ignore PRIMARY, but + # SECONDARY will tell us what the pool's fallback is + fallback_name = rrset['AliasTarget']['DNSName'] \ + .split('.', 1)[0][9:-5] + # Don't care about default fallbacks, anything else + # we'll record + if fallback_name != 'default': + pools[pool_name]['fallback'] = fallback_name + elif 'GeoLocation' in rrset: + # These are rules + _id = rrset['SetIdentifier'] + # We record rule index as the first part of set-id, the 2nd + # part just ensures uniqueness across geos and is ignored + i = int(_id.split('-', 1)[0]) + # Parse the pool name out of _octodns--pool. + pool = rrset['AliasTarget']['DNSName'].split('.', 1)[0][9:-5] + # Record the pool + rules[i]['pool'] = pool + # Record geo if we have one + geo = self._parse_geo(rrset) + if geo: + rules[i]['geos'].append(geo) + else: + # These are the pool value(s) + pool_name = rrset['SetIdentifier'][:-4] + # TODO: handle different value types + value = rrset['ResourceRecords'][0]['Value'] + pools[pool_name]['values'].append({ + 'value': value, + 'weight': rrset['Weight'], + }) + + # Convert our map of rules into an ordered list now that we have all + # the data + for _, rule in sorted(rules.items()): + r = { + 'pool': rule['pool'], + } + geos = sorted(rule['geos']) + if geos: + r['geos'] = geos + data['dynamic']['rules'].append(r) + + return data + def populate(self, zone, target=False, lenient=False): self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, target, lenient) @@ -529,21 +830,46 @@ class Route53Provider(BaseProvider): if zone_id: exists = True records = defaultdict(lambda: defaultdict(list)) + dynamic = defaultdict(lambda: defaultdict(list)) + for rrset in self._load_records(zone_id): record_name = zone.hostname_from_fqdn(rrset['Name']) record_name = _octal_replace(record_name) record_type = rrset['Type'] if record_type not in self.SUPPORTS: + # Skip stuff we don't support + continue + if record_name.startswith('_octodns-'): + # Part of a dynamic record + try: + record_name = record_name.split('.', 1)[1] + except IndexError: + record_name = '' + dynamic[record_name][record_type].append(rrset) continue - if 'AliasTarget' in rrset: - # Alias records are Route53 specific and are not - # portable, so we need to skip them - self.log.warning("%s is an Alias record. Skipping..." - % rrset['Name']) + elif 'AliasTarget' in rrset: + if rrset['AliasTarget']['DNSName'].startswith('_octodns-'): + # Part of a dynamic record + dynamic[record_name][record_type].append(rrset) + else: + # Alias records are Route53 specific and are not + # portable, so we need to skip them + self.log.warning("%s is an Alias record. Skipping..." + % rrset['Name']) continue + # A basic record (potentially including geo) data = getattr(self, '_data_for_{}'.format(record_type))(rrset) records[record_name][record_type].append(data) + # Convert the dynamic rrsets to Records + for name, types in dynamic.items(): + for _type, rrsets in types.items(): + data = self._data_for_dynamic(name, _type, rrsets) + record = Record.new(zone, name, data, source=self, + lenient=lenient) + zone.add_record(record, lenient=lenient) + + # Convert the basic (potentially with geo) rrsets to records for name, types in records.items(): for _type, data in types.items(): if len(data) > 1: @@ -590,6 +916,7 @@ class Route53Provider(BaseProvider): # ignore anything else continue checks[health_check['Id']] = health_check + more = resp['IsTruncated'] start['Marker'] = resp.get('NextMarker', None) @@ -778,6 +1105,7 @@ class Route53Provider(BaseProvider): return self._gen_mods('DELETE', existing_records) def _extra_changes(self, desired, changes, **kwargs): + # TODO: dynamic records extra changes... self.log.debug('_extra_changes: desired=%s', desired.name) zone_id = self._get_zone_id(desired.name) if not zone_id: @@ -862,7 +1190,8 @@ class Route53Provider(BaseProvider): mods.sort(key=_mod_keyer) mods_rs_count = sum( - [len(m['ResourceRecordSet']['ResourceRecords']) for m in mods] + [len(m['ResourceRecordSet'].get('ResourceRecords', '')) + for m in mods] ) if mods_rs_count > self.max_changes: diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 1d48891..7c32d8b 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -42,6 +42,202 @@ class TestOctalReplace(TestCase): self.assertEquals(expected, _octal_replace(s)) +dynamic_rrsets = [{ + 'Name': '_octodns-default-pool.unit.tests.', + 'ResourceRecords': [{'Value': '1.1.2.1'}, + {'Value': '1.1.2.2'}], + 'TTL': 60, + 'Type': 'A', +}, { + 'HealthCheckId': '76', + 'Name': '_octodns-ap-southeast-1-value.unit.tests.', + 'ResourceRecords': [{'Value': '1.4.1.1'}], + 'SetIdentifier': 'ap-southeast-1-000', + 'TTL': 60, + 'Type': 'A', + 'Weight': 2 +}, { + 'HealthCheckId': '09', + 'Name': '_octodns-ap-southeast-1-value.unit.tests.', + 'ResourceRecords': [{'Value': '1.4.1.2'}], + 'SetIdentifier': 'ap-southeast-1-001', + 'TTL': 60, + 'Type': 'A', + 'Weight': 2 +}, { + 'HealthCheckId': 'ab', + 'Name': '_octodns-eu-central-1-value.unit.tests.', + 'ResourceRecords': [{'Value': '1.3.1.1'}], + 'SetIdentifier': 'eu-central-1-000', + 'TTL': 60, + 'Type': 'A', + 'Weight': 1 +}, { + 'HealthCheckId': '1e', + 'Name': '_octodns-eu-central-1-value.unit.tests.', + 'ResourceRecords': [{'Value': '1.3.1.2'}], + 'SetIdentifier': 'eu-central-1-001', + 'TTL': 60, + 'Type': 'A', + 'Weight': 1 +}, { + 'HealthCheckId': '2a', + 'Name': '_octodns-us-east-1-value.unit.tests.', + 'ResourceRecords': [{'Value': '1.5.1.1'}], + 'SetIdentifier': 'us-east-1-000', + 'TTL': 60, + 'Type': 'A', + 'Weight': 1 +}, { + 'HealthCheckId': '61', + 'Name': '_octodns-us-east-1-value.unit.tests.', + 'ResourceRecords': [{'Value': '1.5.1.2'}], + 'SetIdentifier': 'us-east-1-001', + 'TTL': 60, + 'Type': 'A', + 'Weight': 1, +}, { + 'AliasTarget': {'DNSName': '_octodns-default-pool.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'Z2'}, + 'Failover': 'SECONDARY', + 'Name': '_octodns-us-east-1-pool.unit.tests.', + 'SetIdentifier': 'us-east-1-Secondary-default', + 'Type': 'A' +}, { + 'AliasTarget': { + 'DNSName': '_octodns-us-east-1-value.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'Z2' + }, + 'Failover': 'PRIMARY', + 'Name': '_octodns-us-east-1-pool.unit.tests.', + 'SetIdentifier': 'us-east-1-Primary', + 'Type': 'A', +}, { + 'AliasTarget': {'DNSName': '_octodns-us-east-1-pool.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'Z2'}, + 'Failover': 'SECONDARY', + 'Name': '_octodns-eu-central-1-pool.unit.tests.', + 'SetIdentifier': 'eu-central-1-Secondary-default', + 'Type': 'A' +}, { + 'AliasTarget': { + 'DNSName': '_octodns-eu-central-1-value.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'Z2' + }, + 'Failover': 'PRIMARY', + 'Name': '_octodns-eu-central-1-pool.unit.tests.', + 'SetIdentifier': 'eu-central-1-Primary', + 'Type': 'A', +}, { + 'AliasTarget': {'DNSName': '_octodns-us-east-1-pool.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'Z2'}, + 'Failover': 'SECONDARY', + 'Name': '_octodns-ap-southeast-1-pool.unit.tests.', + 'SetIdentifier': 'ap-southeast-1-Secondary-default', + 'Type': 'A' +}, { + 'AliasTarget': { + 'DNSName': '_octodns-ap-southeast-1-value.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'Z2' + }, + 'Failover': 'PRIMARY', + 'Name': '_octodns-ap-southeast-1-pool.unit.tests.', + 'SetIdentifier': 'ap-southeast-1-Primary', + 'Type': 'A', +}, { + 'AliasTarget': {'DNSName': '_octodns-ap-southeast-1-pool.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'Z2'}, + 'GeoLocation': {'CountryCode': 'JP'}, + 'Name': 'unit.tests.', + 'SetIdentifier': '1-ap-southeast-1-AS-JP', + 'Type': 'A', +}, { + 'AliasTarget': {'DNSName': '_octodns-ap-southeast-1-pool.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'Z2'}, + 'GeoLocation': {'CountryCode': 'CN'}, + 'Name': 'unit.tests.', + 'SetIdentifier': '1-ap-southeast-1-AS-CN', + 'Type': 'A', +}, { + 'AliasTarget': {'DNSName': '_octodns-eu-central-1-pool.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'Z2'}, + 'GeoLocation': {'ContinentCode': 'NA-US-FL'}, + 'Name': 'unit.tests.', + 'SetIdentifier': '2-eu-central-1-NA-US-FL', + 'Type': 'A', +}, { + 'AliasTarget': {'DNSName': '_octodns-eu-central-1-pool.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'Z2'}, + 'GeoLocation': {'ContinentCode': 'EU'}, + 'Name': 'unit.tests.', + 'SetIdentifier': '2-eu-central-1-EU', + 'Type': 'A', +}, { + 'AliasTarget': {'DNSName': '_octodns-us-east-1-pool.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'Z2'}, + 'GeoLocation': {'CountryCode': '*'}, + 'Name': 'unit.tests.', + 'SetIdentifier': '3-us-east-1-None', + 'Type': 'A', +}] + +dynamic_record_data = { + 'dynamic': { + 'pools': { + 'ap-southeast-1': { + 'fallback': 'us-east-1', + 'values': [{ + 'weight': 2, 'value': '1.4.1.1' + }, { + 'weight': 2, 'value': '1.4.1.2' + }] + }, + 'eu-central-1': { + 'fallback': 'us-east-1', + 'values': [{ + 'weight': 1, 'value': '1.3.1.1' + }, { + 'weight': 1, 'value': '1.3.1.2' + }], + }, + 'us-east-1': { + 'values': [{ + 'weight': 1, 'value': '1.5.1.1' + }, { + 'weight': 1, 'value': '1.5.1.2' + }], + } + }, + 'rules': [{ + 'geos': ['AS-CN', 'AS-JP'], + 'pool': 'ap-southeast-1', + }, { + 'geos': ['EU', 'NA-US-FL'], + 'pool': 'eu-central-1', + }, { + 'pool': 'us-east-1', + }], + }, + 'ttl': 60, + 'type': 'A', + 'values': [ + '1.1.2.1', + '1.1.2.2', + ], +} + + class TestRoute53Provider(TestCase): expected = Zone('unit.tests.', []) for name, data in ( @@ -1534,6 +1730,71 @@ class TestRoute53Provider(TestCase): ._unique_id_handlers['retry-config-route53'] ['handler']._checker.__dict__['_max_attempts']) + def test_data_for_dynamic(self): + provider = Route53Provider('test', 'abc', '123') + + data = provider._data_for_dynamic('', 'A', dynamic_rrsets) + self.assertEquals(dynamic_record_data, data) + + @patch('octodns.provider.route53.Route53Provider._get_zone_id') + @patch('octodns.provider.route53.Route53Provider._load_records') + def test_dynamic_populate(self, load_records_mock, get_zone_id_mock): + provider = Route53Provider('test', 'abc', '123') + + get_zone_id_mock.side_effect = ['z44'] + load_records_mock.side_effect = [dynamic_rrsets] + + got = Zone('unit.tests.', []) + provider.populate(got) + + self.assertEquals(1, len(got.records)) + record = list(got.records)[0] + self.assertEquals('', record.name) + self.assertEquals('A', record._type) + self.assertEquals([ + '1.1.2.1', + '1.1.2.2', + ], record.values) + self.assertTrue(record.dynamic) + + self.assertEquals({ + 'ap-southeast-1': { + 'fallback': 'us-east-1', + 'values': [{ + 'weight': 2, 'value': '1.4.1.1' + }, { + 'weight': 2, 'value': '1.4.1.2' + }] + }, + 'eu-central-1': { + 'fallback': 'us-east-1', + 'values': [{ + 'weight': 1, 'value': '1.3.1.1' + }, { + 'weight': 1, 'value': '1.3.1.2' + }], + }, + 'us-east-1': { + 'fallback': None, + 'values': [{ + 'weight': 1, 'value': '1.5.1.1' + }, { + 'weight': 1, 'value': '1.5.1.2' + }], + } + }, {k: v.data for k, v in record.dynamic.pools.items()}) + + self.assertEquals([ + { + 'geos': ['AS-CN', 'AS-JP'], + 'pool': 'ap-southeast-1', + }, { + 'geos': ['EU', 'NA-US-FL'], + 'pool': 'eu-central-1', + }, { + 'pool': 'us-east-1', + }], [r.data for r in record.dynamic.rules]) + class TestRoute53Records(TestCase): existing = Zone('unit.tests.', []) @@ -1551,8 +1812,9 @@ class TestRoute53Records(TestCase): route53_record = _Route53Record(None, self.record_a, False) for value in (None, '', 'foo', 'bar', '1.2.3.4'): - self.assertEquals(value, route53_record - ._value_convert_value(value, self.record_a)) + converted = route53_record._value_convert_value(value, + self.record_a) + self.assertEquals(value, converted) record_txt = Record.new(self.existing, 'txt', { 'ttl': 98, @@ -1624,6 +1886,242 @@ class TestRoute53Records(TestCase): e.__repr__() f.__repr__() + def test_new_dynamic(self): + provider = Route53Provider('test', 'abc', '123') + + # Just so boto won't try and make any calls + stubber = Stubber(provider._conn) + stubber.activate() + + # We'll assume we create all healthchecks here, this functionality is + # thoroughly tested elsewhere + provider._health_checks = {} + # When asked for a healthcheck return dummy info + provider.get_health_check_id = lambda r, v, c: 'hc42' + + zone = Zone('unit.tests.', []) + record = Record.new(zone, '', dynamic_record_data) + + # Convert a record into _Route53Records + route53_records = _Route53Record.new(provider, record, 'z45', + creating=True) + self.assertEquals(18, len(route53_records)) + + # Convert the route53_records into mods + self.assertEquals([{ + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'HealthCheckId': 'hc42', + 'Name': '_octodns-ap-southeast-1-value.unit.tests.', + 'ResourceRecords': [{ + 'Value': '1.4.1.2'}], + 'SetIdentifier': 'ap-southeast-1-001', + 'TTL': 60, + 'Type': 'A', + 'Weight': 2 + } + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'HealthCheckId': 'hc42', + 'Name': '_octodns-ap-southeast-1-value.unit.tests.', + 'ResourceRecords': [{ + 'Value': '1.4.1.1'}], + 'SetIdentifier': 'ap-southeast-1-000', + 'TTL': 60, + 'Type': 'A', + 'Weight': 2 + } + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'AliasTarget': { + 'DNSName': '_octodns-ap-southeast-1-pool.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'z45' + }, + 'GeoLocation': { + 'CountryCode': 'JP'}, + 'Name': 'unit.tests.', + 'SetIdentifier': '0-ap-southeast-1-AS-JP', + 'Type': 'A' + } + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'AliasTarget': { + 'DNSName': '_octodns-eu-central-1-pool.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'z45'}, + 'GeoLocation': { + 'CountryCode': 'US', + 'SubdivisionCode': 'FL', + }, + 'Name': 'unit.tests.', + 'SetIdentifier': '1-eu-central-1-NA-US-FL', + 'Type': 'A'} + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'AliasTarget': { + 'DNSName': '_octodns-us-east-1-pool.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'z45'}, + 'GeoLocation': { + 'CountryCode': '*'}, + 'Name': 'unit.tests.', + 'SetIdentifier': '2-us-east-1-None', + 'Type': 'A'} + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'AliasTarget': { + 'DNSName': '_octodns-us-east-1-pool.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'z45'}, + 'Failover': 'SECONDARY', + 'Name': '_octodns-ap-southeast-1-pool.unit.tests.', + 'SetIdentifier': 'ap-southeast-1-Secondary-us-east-1', + 'Type': 'A'} + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'AliasTarget': { + 'DNSName': '_octodns-ap-southeast-1-pool.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'z45'}, + 'GeoLocation': { + 'CountryCode': 'CN'}, + 'Name': 'unit.tests.', + 'SetIdentifier': '0-ap-southeast-1-AS-CN', + 'Type': 'A'} + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'AliasTarget': { + 'DNSName': '_octodns-us-east-1-value.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'z45'}, + 'Failover': 'PRIMARY', + 'Name': '_octodns-us-east-1-pool.unit.tests.', + 'SetIdentifier': 'us-east-1-Primary', + 'Type': 'A'} + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'AliasTarget': { + 'DNSName': '_octodns-eu-central-1-pool.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'z45'}, + 'GeoLocation': { + 'ContinentCode': 'EU'}, + 'Name': 'unit.tests.', + 'SetIdentifier': '1-eu-central-1-EU', + 'Type': 'A'} + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'AliasTarget': { + 'DNSName': '_octodns-eu-central-1-value.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'z45'}, + 'Failover': 'PRIMARY', + 'Name': '_octodns-eu-central-1-pool.unit.tests.', + 'SetIdentifier': 'eu-central-1-Primary', + 'Type': 'A'} + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'Name': '_octodns-default-pool.unit.tests.', + 'ResourceRecords': [{ + 'Value': '1.1.2.1'}, + { + 'Value': '1.1.2.2'}], + 'TTL': 60, + 'Type': 'A'} + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'HealthCheckId': 'hc42', + 'Name': '_octodns-eu-central-1-value.unit.tests.', + 'ResourceRecords': [{ + 'Value': '1.3.1.2'}], + 'SetIdentifier': 'eu-central-1-001', + 'TTL': 60, + 'Type': 'A', + 'Weight': 1} + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'HealthCheckId': 'hc42', + 'Name': '_octodns-eu-central-1-value.unit.tests.', + 'ResourceRecords': [{ + 'Value': '1.3.1.1'}], + 'SetIdentifier': 'eu-central-1-000', + 'TTL': 60, + 'Type': 'A', + 'Weight': 1} + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'AliasTarget': { + 'DNSName': '_octodns-default-pool.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'z45'}, + 'Failover': 'SECONDARY', + 'Name': '_octodns-us-east-1-pool.unit.tests.', + 'SetIdentifier': 'us-east-1-Secondary-default', + 'Type': 'A'} + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'HealthCheckId': 'hc42', + 'Name': '_octodns-us-east-1-value.unit.tests.', + 'ResourceRecords': [{ + 'Value': '1.5.1.2'}], + 'SetIdentifier': 'us-east-1-001', + 'TTL': 60, + 'Type': 'A', + 'Weight': 1} + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'HealthCheckId': 'hc42', + 'Name': '_octodns-us-east-1-value.unit.tests.', + 'ResourceRecords': [{ + 'Value': '1.5.1.1'}], + 'SetIdentifier': 'us-east-1-000', + 'TTL': 60, + 'Type': 'A', + 'Weight': 1} + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'AliasTarget': { + 'DNSName': '_octodns-ap-southeast-1-value.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'z45'}, + 'Failover': 'PRIMARY', + 'Name': '_octodns-ap-southeast-1-pool.unit.tests.', + 'SetIdentifier': 'ap-southeast-1-Primary', + 'Type': 'A'} + }, { + 'Action': 'CREATE', + 'ResourceRecordSet': { + 'AliasTarget': { + 'DNSName': '_octodns-us-east-1-pool.unit.tests.', + 'EvaluateTargetHealth': True, + 'HostedZoneId': 'z45'}, + 'Failover': 'SECONDARY', + '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]) + + for route53_record in route53_records: + # Smoke test stringification + route53_record.__repr__() + class TestModKeyer(TestCase): From af06dbec0964a746f2851182798d9c834797ff07 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 3 Apr 2019 10:29:52 -0700 Subject: [PATCH 09/16] Route53Provider for CNAME style healthchecks Note that you can't specify a Host header for these which I believe will complicate the ability to use this. Figuring that out will have to wait until I or someone else has a use case for these... --- octodns/provider/route53.py | 18 +++++++++++++--- tests/test_octodns_provider_route53.py | 30 ++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index a3187b9..6764daf 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -9,6 +9,7 @@ from boto3 import client from botocore.config import Config from collections import defaultdict from incf.countryutils.transformations import cca_to_ctca2 +from ipaddress import AddressValueError, ip_address from uuid import uuid4 import logging import re @@ -948,7 +949,15 @@ class Route53Provider(BaseProvider): self.log.debug('get_health_check_id: fqdn=%s, type=%s, value=%s', fqdn, record._type, value) - healthcheck_host = record.healthcheck_host + try: + ip_address(unicode(value)) + # We're working with an IP, host is the Host header + healthcheck_host = record.healthcheck_host + except (AddressValueError, ValueError): + # This isn't an IP, host is the value, value should be None + healthcheck_host = value + value = None + healthcheck_path = record.healthcheck_path healthcheck_protocol = record.healthcheck_protocol healthcheck_port = record.healthcheck_port @@ -983,13 +992,15 @@ class Route53Provider(BaseProvider): 'EnableSNI': healthcheck_protocol == 'HTTPS', 'FailureThreshold': 6, 'FullyQualifiedDomainName': healthcheck_host, - 'IPAddress': value, 'MeasureLatency': healthcheck_latency, 'Port': healthcheck_port, 'RequestInterval': 10, 'ResourcePath': healthcheck_path, 'Type': healthcheck_protocol, } + if value: + config['IPAddress'] = value + ref = '{}:{}:{}:{}'.format(self.HEALTH_CHECK_VERSION, record._type, record.fqdn, uuid4().hex[:12]) resp = self._conn.create_health_check(CallerReference=ref, @@ -998,7 +1009,8 @@ class Route53Provider(BaseProvider): id = health_check['Id'] # Set a Name for the benefit of the UI - name = '{}:{} - {}'.format(record.fqdn, record._type, value) + name = '{}:{} - {}'.format(record.fqdn, record._type, + value or healthcheck_host) self._conn.change_tags_for_resource(ResourceType='healthcheck', ResourceId=id, AddTags=[{ diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 7c32d8b..2496f82 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -1066,6 +1066,36 @@ class TestRoute53Provider(TestCase): self.assertEquals('42', id) stubber.assert_no_pending_responses() + # A CNAME style healthcheck, without a value + + health_check_config = { + 'EnableSNI': False, + 'FailureThreshold': 6, + 'FullyQualifiedDomainName': 'target-1.unit.tests.', + 'MeasureLatency': True, + 'Port': 8080, + 'RequestInterval': 10, + 'ResourcePath': '/_status', + 'Type': 'HTTP' + } + stubber.add_response('create_health_check', { + 'HealthCheck': { + 'Id': '42', + 'CallerReference': self.caller_ref, + 'HealthCheckConfig': health_check_config, + 'HealthCheckVersion': 1, + }, + 'Location': 'http://url', + }, { + 'CallerReference': ANY, + 'HealthCheckConfig': health_check_config, + }) + stubber.add_response('change_tags_for_resource', {}) + + id = provider.get_health_check_id(record, 'target-1.unit.tests.', True) + self.assertEquals('42', id) + stubber.assert_no_pending_responses() + def test_health_check_measure_latency(self): provider, stubber = self._get_stubbed_provider() record_true = Record.new(self.expected, 'a', { From 34744b7b347e82a56f3f618d6a3f2603af438ba8 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 3 Apr 2019 13:10:19 -0700 Subject: [PATCH 10/16] Normalize ip addresses for comparing health checks since Route53 does --- octodns/provider/route53.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 6764daf..c61e0cd 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -934,11 +934,23 @@ class Route53Provider(BaseProvider): def _health_check_equivilent(self, host, path, protocol, port, measure_latency, health_check, value=None): config = health_check['HealthCheckConfig'] + + # So interestingly Route53 normalizes IPAddress which will cause us to + # fail to find see things as equivalent. To work around this we'll + # ip_address's returned object for equivalence + # E.g 2001:4860:4860::8842 -> 2001:4860:4860:0:0:0:0:8842 + if value: + value = ip_address(unicode(value)) + config_ip_address = ip_address(unicode(config['IPAddress'])) + else: + # No value so give this a None to match value's + config_ip_address = None + return host == config['FullyQualifiedDomainName'] and \ path == config['ResourcePath'] and protocol == config['Type'] \ and port == config['Port'] and \ measure_latency == config['MeasureLatency'] and \ - (value is None or value == config['IPAddress']) + value == config_ip_address def get_health_check_id(self, record, value, create): # fqdn & the first value are special, we use them to match up health From 4db9d5cbf4cdc948559d52545e59b22c27898601 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 3 Apr 2019 13:13:13 -0700 Subject: [PATCH 11/16] Remove TODO about value types, doesn't apply for now at least --- octodns/provider/route53.py | 1 - 1 file changed, 1 deletion(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index c61e0cd..9ff9d99 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -800,7 +800,6 @@ class Route53Provider(BaseProvider): else: # These are the pool value(s) pool_name = rrset['SetIdentifier'][:-4] - # TODO: handle different value types value = rrset['ResourceRecords'][0]['Value'] pools[pool_name]['values'].append({ 'value': value, From 92179a231e7c2e028806bc0e3163eee9b9df58b2 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 3 Apr 2019 14:13:23 -0700 Subject: [PATCH 12/16] Rework and clean up Route53Provider's extra_changes check to support dynamic --- octodns/provider/route53.py | 134 ++++++++++++++++--------- tests/test_octodns_provider_route53.py | 113 +++++++++++++++++++++ 2 files changed, 197 insertions(+), 50 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 9ff9d99..97809b9 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -1127,8 +1127,83 @@ class Route53Provider(BaseProvider): self._gc_health_checks(change.existing, []) return self._gen_mods('DELETE', existing_records) + def _extra_changes_update_needed(self, record, rrset): + healthcheck_host = record.healthcheck_host + healthcheck_path = record.healthcheck_path + healthcheck_protocol = record.healthcheck_protocol + healthcheck_port = record.healthcheck_port + healthcheck_latency = self._healthcheck_measure_latency(record) + + try: + health_check_id = rrset['HealthCheckId'] + health_check = self.health_checks[health_check_id] + caller_ref = health_check['CallerReference'] + if caller_ref.startswith(self.HEALTH_CHECK_VERSION): + if self._health_check_equivilent(healthcheck_host, + healthcheck_path, + healthcheck_protocol, + healthcheck_port, + healthcheck_latency, + health_check): + # it has the right health check + return False + except (IndexError, KeyError): + # no health check id or one that isn't the right version + pass + + # no good, doesn't have the right health check, needs an update + self.log.info('_extra_changes_update_needed: health-check caused ' + 'update of %s:%s', record.fqdn, record._type) + return True + + def _extra_changes_geo_needs_update(self, zone_id, record): + # OK this is a record we don't have change for that does have geo + # information. We need to look and see if it needs to be updated b/c of + # a health check version bump or other mismatch + self.log.debug('_extra_changes_geo_needs_update: inspecting=%s, %s', + record.fqdn, record._type) + + fqdn = record.fqdn + + # loop through all the r53 rrsets + for rrset in self._load_records(zone_id): + if fqdn == rrset['Name'] and record._type == rrset['Type'] and \ + rrset.get('GeoLocation', {}).get('CountryCode', False) != '*' \ + and self._extra_changes_update_needed(record, rrset): + # no good, doesn't have the right health check, needs an update + self.log.info('_extra_changes_geo_needs_update: health-check ' + 'caused update of %s:%s', record.fqdn, + record._type) + return True + + return False + + def _extra_changes_dynamic_needs_update(self, zone_id, record): + # OK this is a record we don't have change for that does have dynamic + # information. We need to look and see if it needs to be updated b/c of + # a health check version bump or other mismatch + self.log.debug('_extra_changes_dynamic_needs_update: inspecting=%s, ' + '%s', record.fqdn, record._type) + + fqdn = record.fqdn + + # loop through all the r53 rrsets + for rrset in self._load_records(zone_id): + name = rrset['Name'] + + 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): + # 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', + record.fqdn, record._type) + return True + + return False + def _extra_changes(self, desired, changes, **kwargs): - # TODO: dynamic records extra changes... self.log.debug('_extra_changes: desired=%s', desired.name) zone_id = self._get_zone_id(desired.name) if not zone_id: @@ -1138,61 +1213,20 @@ class Route53Provider(BaseProvider): changed = set([c.record for c in changes]) # ok, now it's time for the reason we're here, we need to go over all # the desired records - extra = [] + extras = [] for record in desired.records: if record in changed: # already have a change for it, skipping continue - if not getattr(record, 'geo', False): - # record doesn't support geo, we don't need to inspect it - continue - # OK this is a record we don't have change for that does have geo - # information. We need to look and see if it needs to be updated - # b/c of a health check version bump - self.log.debug('_extra_changes: inspecting=%s, %s', record.fqdn, - record._type) - healthcheck_host = record.healthcheck_host - healthcheck_path = record.healthcheck_path - healthcheck_protocol = record.healthcheck_protocol - healthcheck_port = record.healthcheck_port - healthcheck_latency = self._healthcheck_measure_latency(record) - fqdn = record.fqdn - - # loop through all the r53 rrsets - for rrset in self._load_records(zone_id): - if fqdn != rrset['Name'] or record._type != rrset['Type']: - # not a name and type match - continue - if rrset.get('GeoLocation', {}) \ - .get('CountryCode', False) == '*': - # it's a default record - continue - # we expect a healthcheck now - try: - health_check_id = rrset['HealthCheckId'] - health_check = self.health_checks[health_check_id] - caller_ref = health_check['CallerReference'] - if caller_ref.startswith(self.HEALTH_CHECK_VERSION): - if self._health_check_equivilent(healthcheck_host, - healthcheck_path, - healthcheck_protocol, - healthcheck_port, - healthcheck_latency, - health_check): - # it has the right health check - continue - except (IndexError, KeyError): - # no health check id or one that isn't the right version - pass - # no good, doesn't have the right health check, needs an update - self.log.info('_extra_changes: health-check caused ' - 'update of %s:%s', record.fqdn, record._type) - extra.append(Update(record, record)) - # We don't need to process this record any longer - break + if getattr(record, 'geo', False): + if self._extra_changes_geo_needs_update(zone_id, record): + extras.append(Update(record, record)) + elif getattr(record, 'dynamic', False): + if self._extra_changes_dynamic_needs_update(zone_id, record): + extras.append(Update(record, record)) - return extra + return extras def _apply(self, plan): desired = plan.desired diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 2496f82..4c5ed4f 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -1616,6 +1616,119 @@ class TestRoute53Provider(TestCase): self.assertEquals(1, len(extra)) stubber.assert_no_pending_responses() + def test_extra_change_dynamic_has_health_check(self): + provider, stubber = self._get_stubbed_provider() + + list_hosted_zones_resp = { + 'HostedZones': [{ + 'Name': 'unit.tests.', + 'Id': 'z42', + 'CallerReference': 'abc', + }], + 'Marker': 'm', + 'IsTruncated': False, + 'MaxItems': '100', + } + stubber.add_response('list_hosted_zones', list_hosted_zones_resp, {}) + + # record with geo and no health check returns change + desired = Zone('unit.tests.', []) + record = Record.new(desired, 'a', { + 'ttl': 30, + 'type': 'A', + 'value': '1.2.3.4', + 'dynamic': { + 'pools': { + 'one': { + 'values': [{ + 'value': '2.2.3.4', + }], + }, + }, + 'rules': [{ + 'pool': 'one', + }], + }, + }) + desired.add_record(record) + list_resource_record_sets_resp = { + 'ResourceRecordSets': [{ + # other name + 'Name': 'unit.tests.', + 'Type': 'A', + 'GeoLocation': { + 'CountryCode': '*', + }, + 'ResourceRecords': [{ + 'Value': '1.2.3.4', + }], + 'TTL': 61, + }, { + # matching name, other type + 'Name': 'a.unit.tests.', + 'Type': 'AAAA', + 'ResourceRecords': [{ + 'Value': '2001:0db8:3c4d:0015:0000:0000:1a2f:1a4b' + }], + 'TTL': 61, + }, { + # default value pool + 'Name': '_octodns-default-pool.a.unit.tests.', + 'Type': 'A', + 'GeoLocation': { + 'CountryCode': '*', + }, + 'ResourceRecords': [{ + 'Value': '1.2.3.4', + }], + 'TTL': 61, + }, { + # match + 'Name': '_octodns-one-value.a.unit.tests.', + 'Type': 'A', + 'ResourceRecords': [{ + 'Value': '2.2.3.4', + }], + 'TTL': 61, + 'HealthCheckId': '42', + }], + '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': [{ + 'Id': '42', + 'CallerReference': self.caller_ref, + 'HealthCheckConfig': { + 'Type': 'HTTPS', + 'FullyQualifiedDomainName': 'a.unit.tests', + 'IPAddress': '2.2.3.4', + 'ResourcePath': '/_dns', + 'Type': 'HTTPS', + 'Port': 443, + 'MeasureLatency': True + }, + 'HealthCheckVersion': 2, + }], + 'IsTruncated': False, + 'MaxItems': '100', + 'Marker': '', + }) + extra = provider._extra_changes(desired=desired, changes=[]) + self.assertEquals(0, len(extra)) + stubber.assert_no_pending_responses() + + # change b/c of healthcheck path + record._octodns['healthcheck'] = { + 'path': '/_ready' + } + extra = provider._extra_changes(desired=desired, changes=[]) + self.assertEquals(1, len(extra)) + stubber.assert_no_pending_responses() + # change b/c of healthcheck host record._octodns['healthcheck'] = { 'host': 'foo.bar.io' From dbc032a2cc712d39553c14df64add12dc4f84de6 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 3 Apr 2019 18:58:46 -0700 Subject: [PATCH 13/16] Switch to using tuples for _mod_keyer --- octodns/provider/route53.py | 14 +++++++------- tests/test_octodns_provider_route53.py | 12 ++++++------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 97809b9..db6f6a7 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -469,9 +469,9 @@ class _Route53GeoRecord(_Route53Record): _mod_keyer_action_order = { - 'DELETE': '0', # Delete things first - 'CREATE': '1', # Then Create things - 'UPSERT': '2', # Upsert things last + 'DELETE': 0, # Delete things first + 'CREATE': 1, # Then Create things + 'UPSERT': 2, # Upsert things last } @@ -483,18 +483,18 @@ def _mod_keyer(mod): # name/id of the rrset if rrset.get('GeoLocation', False): - return '{}3{}'.format(action_order, rrset['SetIdentifier']) + return (action_order, 3, rrset['SetIdentifier']) elif rrset.get('AliasTarget', False): # We use an alias if rrset.get('Failover', False) == 'SECONDARY': # We're a secondary we'll ref primaries - return '{}2{}'.format(action_order, rrset['Name']) + return (action_order, 2, rrset['Name']) else: # We're a primary we'll ref values - return '{}1{}'.format(action_order, rrset['Name']) + return (action_order, 1, rrset['Name']) # We're just a plain value, these come first - return '{}0{}'.format(action_order, rrset['Name']) + return (action_order, 0, rrset['Name']) class Route53Provider(BaseProvider): diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 4c5ed4f..67fcb76 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -2273,7 +2273,7 @@ class TestModKeyer(TestCase): # First "column" # Deletes come first - self.assertEquals('00something', _mod_keyer({ + self.assertEquals((0, 0, 'something'), _mod_keyer({ 'Action': 'DELETE', 'ResourceRecordSet': { 'Name': 'something', @@ -2281,7 +2281,7 @@ class TestModKeyer(TestCase): })) # Creates come next - self.assertEquals('10another', _mod_keyer({ + self.assertEquals((1, 0, 'another'), _mod_keyer({ 'Action': 'CREATE', 'ResourceRecordSet': { 'Name': 'another', @@ -2289,7 +2289,7 @@ class TestModKeyer(TestCase): })) # Then upserts - self.assertEquals('20last', _mod_keyer({ + self.assertEquals((2, 0, 'last'), _mod_keyer({ 'Action': 'UPSERT', 'ResourceRecordSet': { 'Name': 'last', @@ -2299,7 +2299,7 @@ class TestModKeyer(TestCase): # Second "column" value records tested above # AliasTarget primary second (to value) - self.assertEquals('01thing', _mod_keyer({ + self.assertEquals((0, 1, 'thing'), _mod_keyer({ 'Action': 'DELETE', 'ResourceRecordSet': { 'AliasTarget': 'some-target', @@ -2309,7 +2309,7 @@ class TestModKeyer(TestCase): })) # AliasTarget secondary third - self.assertEquals('02thing', _mod_keyer({ + self.assertEquals((0, 2, 'thing'), _mod_keyer({ 'Action': 'DELETE', 'ResourceRecordSet': { 'AliasTarget': 'some-target', @@ -2319,7 +2319,7 @@ class TestModKeyer(TestCase): })) # GeoLocation fourth - self.assertEquals('03some-id', _mod_keyer({ + self.assertEquals((0, 3, 'some-id'), _mod_keyer({ 'Action': 'DELETE', 'ResourceRecordSet': { 'GeoLocation': 'some-target', From cc9a1648d22ad3c457e66e93c002d450b8ffafcf Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 3 Apr 2019 19:25:24 -0700 Subject: [PATCH 14/16] Pull dup'd parsing logic into a helper func, doc a singular case --- octodns/provider/route53.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index db6f6a7..c08a3a6 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -497,6 +497,11 @@ def _mod_keyer(mod): return (action_order, 0, rrset['Name']) +def _parse_pool_name(n): + # Parse the pool name out of _octodns--pool... + return n.split('.', 1)[0][9:-5] + + class Route53Provider(BaseProvider): ''' AWS Route53 Provider @@ -768,7 +773,7 @@ class Route53Provider(BaseProvider): name = rrset['Name'] if '-pool.' in name: # This is a pool rrset - pool_name = name.split('.', 1)[0][9:-5] + pool_name = _parse_pool_name(name) if pool_name == 'default': # default becomes the base for the record and its # value(s) will fill the non-dynamic values @@ -777,8 +782,8 @@ class Route53Provider(BaseProvider): elif rrset['Failover'] == 'SECONDARY': # This is a failover record, we'll ignore PRIMARY, but # SECONDARY will tell us what the pool's fallback is - fallback_name = rrset['AliasTarget']['DNSName'] \ - .split('.', 1)[0][9:-5] + fallback_name = \ + _parse_pool_name(rrset['AliasTarget']['DNSName']) # Don't care about default fallbacks, anything else # we'll record if fallback_name != 'default': @@ -789,16 +794,18 @@ class Route53Provider(BaseProvider): # We record rule index as the first part of set-id, the 2nd # part just ensures uniqueness across geos and is ignored i = int(_id.split('-', 1)[0]) - # Parse the pool name out of _octodns--pool. - pool = rrset['AliasTarget']['DNSName'].split('.', 1)[0][9:-5] + target_pool = _parse_pool_name(rrset['AliasTarget']['DNSName']) # Record the pool - rules[i]['pool'] = pool + rules[i]['pool'] = target_pool # Record geo if we have one geo = self._parse_geo(rrset) if geo: rules[i]['geos'].append(geo) else: # These are the pool value(s) + # Grab the pool name out of the SetIdentifier, format looks + # like ...-000 where 000 is a zero-padded index for the value + # it's ignored only used to make sure the value is unique pool_name = rrset['SetIdentifier'][:-4] value = rrset['ResourceRecords'][0]['Value'] pools[pool_name]['values'].append({ From 3e0a452f7735e153330e261b648c97a464390121 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 4 Apr 2019 08:37:55 -0700 Subject: [PATCH 15/16] CHANGELOG and README updates for dynamic and recent work --- CHANGELOG.md | 12 ++++++++++++ README.md | 8 ++++---- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87e70a0..5435de5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,15 @@ +## v0.9.5 - 2019-??-?? - 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. +* Route53Provider dynamic record support +* DynProvider dynamic record support +* SUPPORTS_DYNAMIC is an optional property, defaults to False +* Route53Provider health checks support disabling latency measurement +* CloudflareProvider SRV record unpacking fix +* DNSMadeEasy provider uses supports to avoid blowing up on unknown record + types + ## v0.9.4 - 2019-01-28 - The one with a bunch of stuff, before the big one * A bunch of "dynamic" stuff that'll be detailed in the next release when diff --git a/README.md b/README.md index 2451e70..b815fbc 100644 --- a/README.md +++ b/README.md @@ -149,21 +149,21 @@ The above command pulled the existing data out of Route53 and placed the results ## Supported providers -| Provider | Requirements | Record Support | GeoDNS Support | Notes | +| Provider | Requirements | Record Support | Dynamic/Geo Support | Notes | |--|--|--|--|--| | [AzureProvider](/octodns/provider/azuredns.py) | azure-mgmt-dns | A, AAAA, CNAME, MX, NS, PTR, SRV, TXT | No | | | [CloudflareProvider](/octodns/provider/cloudflare.py) | | A, AAAA, ALIAS, CAA, CNAME, MX, NS, SPF, SRV, TXT | No | CAA tags restricted | | [DigitalOceanProvider](/octodns/provider/digitalocean.py) | | A, AAAA, CAA, CNAME, MX, NS, TXT, SRV | No | CAA tags restricted | | [DnsMadeEasyProvider](/octodns/provider/dnsmadeeasy.py) | | A, AAAA, ALIAS (ANAME), CAA, CNAME, MX, NS, PTR, SPF, SRV, TXT | No | CAA tags restricted | | [DnsimpleProvider](/octodns/provider/dnsimple.py) | | All | No | CAA tags restricted | -| [DynProvider](/octodns/provider/dyn.py) | dyn | All | Yes | | +| [DynProvider](/octodns/provider/dyn.py) | dyn | All | Both | | | [EtcHostsProvider](/octodns/provider/etc_hosts.py) | | A, AAAA, ALIAS, CNAME | No | | | [GoogleCloudProvider](/octodns/provider/googlecloud.py) | google-cloud-dns | A, AAAA, CAA, CNAME, MX, NAPTR, NS, PTR, SPF, SRV, TXT | No | | -| [Ns1Provider](/octodns/provider/ns1.py) | nsone | All | Yes | No health checking for GeoDNS | +| [Ns1Provider](/octodns/provider/ns1.py) | nsone | All | Partial Geo | No health checking for GeoDNS | | [OVH](/octodns/provider/ovh.py) | ovh | A, AAAA, CNAME, MX, NAPTR, NS, PTR, SPF, SRV, SSHFP, TXT, DKIM | No | | | [PowerDnsProvider](/octodns/provider/powerdns.py) | | All | No | | | [Rackspace](/octodns/provider/rackspace.py) | | A, AAAA, ALIAS, CNAME, MX, NS, PTR, SPF, TXT | No | | -| [Route53](/octodns/provider/route53.py) | boto3 | A, AAAA, CAA, CNAME, MX, NAPTR, NS, PTR, SPF, SRV, TXT | Yes | | +| [Route53](/octodns/provider/route53.py) | boto3 | A, AAAA, CAA, CNAME, MX, NAPTR, NS, PTR, SPF, SRV, TXT | Both | CNAME health checks don't support a Host header | | [AxfrSource](/octodns/source/axfr.py) | | A, AAAA, CNAME, MX, NS, PTR, SPF, SRV, TXT | No | read-only | | [ZoneFileSource](/octodns/source/axfr.py) | | A, AAAA, CNAME, MX, NS, PTR, SPF, SRV, TXT | No | read-only | | [TinyDnsFileSource](/octodns/source/tinydns.py) | | A, CNAME, MX, NS, PTR | No | read-only | From add8cf25d49ea6923daf7d2aab4feedbcb5ba10d Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 4 Apr 2019 09:17:28 -0700 Subject: [PATCH 16/16] Bunch of comments on Route53Provider from the PR review --- octodns/provider/route53.py | 41 ++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index c08a3a6..f5185b0 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -32,13 +32,16 @@ class _Route53Record(object): @classmethod def _new_dynamic(cls, provider, record, hosted_zone_id, creating): + # Creates the RRSets that correspond to the given dynamic record ret = set() # HostedZoneId wants just the last bit, but the place we're getting # this from looks like /hostedzone/Z424CArX3BB224 hosted_zone_id = hosted_zone_id.split('/', 2)[-1] - # Create the default pool + # Create the default pool which comes from the base `values` of the + # record object. Its only used if all other values fail their + # healthchecks, which hopefully never happens. fqdn = record.fqdn ret.add(_Route53Record(provider, record, creating, '_octodns-default-pool.{}'.format(fqdn))) @@ -46,24 +49,34 @@ class _Route53Record(object): # Pools for pool_name, pool in record.dynamic.pools.items(): - # Create the primary + # Create the primary, this will be the rrset that geo targeted + # rrsets will point to when they want to use a pool of values. It's + # a primary and observes target health so if all the values for + # this pool go red, we'll use the fallback/SECONDARY just below ret.add(_Route53DynamicPool(provider, hosted_zone_id, record, pool_name, creating)) - # Create the fallback + # Create the fallback for this pool fallback = pool.data.get('fallback', False) if fallback: - # We have an explicitly configured fallback + # We have an explicitly configured fallback, another pool to + # use if all our values go red. This RRSet configures that pool + # as the next best option ret.add(_Route53DynamicPool(provider, hosted_zone_id, record, pool_name, creating, target_name=fallback)) else: - # We fallback on the default + # We fallback on the default, no explicit fallback so if all of + # this pool's values go red we'll fallback to the base + # (non-health-checked) default pool of values ret.add(_Route53DynamicPool(provider, hosted_zone_id, record, pool_name, creating, target_name='default')) - # Create the values + # Create the values for this pool. These are health checked and in + # general each unique value will have an associated healthcheck. + # The PRIMARY pool up above will point to these RRSets which will + # be served out according to their weights for i, value in enumerate(pool.data['values']): weight = value['weight'] value = value['value'] @@ -76,10 +89,15 @@ class _Route53Record(object): geos = rule.data.get('geos', []) if geos: for geo in geos: + # Create a RRSet for each geo in each rule that uses the + # desired target pool ret.add(_Route53DynamicRule(provider, hosted_zone_id, record, pool_name, i, creating, geo=geo)) else: + # There's no geo's for this rule so it's the catchall that will + # just point things that don't match any geo rules to the + # specified pool ret.add(_Route53DynamicRule(provider, hosted_zone_id, record, pool_name, i, creating)) @@ -87,6 +105,7 @@ class _Route53Record(object): @classmethod def _new_geo(cls, provider, record, creating): + # Creates the RRSets that correspond to the given geo record ret = set() ret.add(_Route53GeoDefault(provider, record, creating)) @@ -98,6 +117,7 @@ class _Route53Record(object): @classmethod def new(cls, provider, record, hosted_zone_id, creating): + # Creates the RRSets that correspond to the given record if getattr(record, 'dynamic', False): ret = cls._new_dynamic(provider, record, hosted_zone_id, creating) @@ -105,6 +125,7 @@ class _Route53Record(object): elif getattr(record, 'geo', False): return cls._new_geo(provider, record, creating) + # Its a simple record that translates into a single RRSet return set((_Route53Record(provider, record, creating),)) def __init__(self, provider, record, creating, fqdn_override=None): @@ -480,7 +501,11 @@ def _mod_keyer(mod): action_order = _mod_keyer_action_order[mod['Action']] # We're sorting by 3 "columns", the action, the rrset type, and finally the - # name/id of the rrset + # name/id of the rrset. This ensures that Route53 won't see a RRSet that + # targets another that hasn't been seen yet. I.e. targets must come before + # things that target them. We sort on types of things rather than + # explicitly looking for targeting relationships since that's sufficent and + # easier to grok/do. if rrset.get('GeoLocation', False): return (action_order, 3, rrset['SetIdentifier']) @@ -756,6 +781,8 @@ class Route53Provider(BaseProvider): return self._r53_rrsets[zone_id] def _data_for_dynamic(self, name, _type, rrsets): + # This converts a bunch of RRSets into their corresponding dynamic + # Record. It's used by populate. pools = defaultdict(lambda: {'values': []}) # Data to build our rules will be collected here and "converted" into # their final form below