From 0d025a72a300166c0443446aca0d9c1e8ed26571 Mon Sep 17 00:00:00 2001 From: slandry Date: Fri, 17 Sep 2021 16:50:41 -0400 Subject: [PATCH 1/3] Add functionality to get r53 zone id by name --- octodns/provider/route53.py | 17 +++++- tests/test_octodns_provider_route53.py | 76 ++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 2 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 0ecce72..2bf59a5 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -610,9 +610,11 @@ class Route53Provider(BaseProvider): def __init__(self, id, access_key_id=None, secret_access_key=None, max_changes=1000, client_max_attempts=None, - session_token=None, delegation_set_id=None, *args, **kwargs): + session_token=None, delegation_set_id=None, + get_zones_by_name=False, *args, **kwargs): self.max_changes = max_changes self.delegation_set_id = delegation_set_id + self.get_zones_by_name = get_zones_by_name _msg = f'access_key_id={access_key_id}, secret_access_key=***, ' \ 'session_token=***' use_fallback_auth = access_key_id is None and \ @@ -661,7 +663,18 @@ class Route53Provider(BaseProvider): def _get_zone_id(self, name, create=False): self.log.debug('_get_zone_id: name=%s', name) - if name in self.r53_zones: + if self.get_zones_by_name: + # attempt to get zone by name + # limited to one as this should be unique + response = self._conn.list_hosted_zones_by_name( + DNSName=name, MaxItems="1" + ) + if len(response['HostedZones']) == 1: + # if there is a single response + id = response['HostedZones'][0]['Id'] + self.log.debug(id) + return id + elif name in self.r53_zones: id = self.r53_zones[name] self.log.debug('_get_zone_id: id=%s', id) return id diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 8bc4562..136f442 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -1745,6 +1745,82 @@ class TestRoute53Provider(TestCase): self.assertEquals([], extra) stubber.assert_no_pending_responses() + def test_no_changes_with_get_zones_by_name(self): + provider = Route53Provider( + 'test', 'abc', '123', get_zones_by_name=True) + + # Use the stubber + stubber = Stubber(provider._conn) + stubber.activate() + + list_hosted_zones_by_name_resp = { + 'HostedZones': [{ + 'Id': 'z42', + 'Name': 'unit.tests.', + 'CallerReference': 'abc', + 'Config': { + 'Comment': 'string', + 'PrivateZone': False + }, + 'ResourceRecordSetCount': 123, + }, ], + 'DNSName': 'unit.tests.', + 'HostedZoneId': 'z42', + 'IsTruncated': False, + 'MaxItems': 'string' + } + + stubber.add_response( + 'list_hosted_zones_by_name', + list_hosted_zones_by_name_resp, + {'DNSName': 'unit.tests.', 'MaxItems': '1'} + ) + + # empty is empty + desired = Zone('unit.tests.', []) + extra = provider._extra_changes(desired=desired, changes=[]) + self.assertEquals([], extra) + stubber.assert_no_pending_responses() + + def test_plan_with_get_zones_by_name(self): + provider = Route53Provider( + 'test', 'abc', '123', get_zones_by_name=True) + + # Use the stubber + stubber = Stubber(provider._conn) + stubber.activate() + + # this is an empty response + # zone name not found + list_hosted_zones_by_name_resp = { + 'HostedZones': [], + 'DNSName': 'unit.tests.', + 'HostedZoneId': 'z42', + 'IsTruncated': False, + 'MaxItems': 'string' + } + + # list_hosted_zones_by_name gets called 3 times in this process + # so adding 3 responses + stubber.add_response( + 'list_hosted_zones_by_name', + list_hosted_zones_by_name_resp, + {'DNSName': 'unit.tests.', 'MaxItems': '1'} + ) + stubber.add_response( + 'list_hosted_zones_by_name', + list_hosted_zones_by_name_resp, + {'DNSName': 'unit.tests.', 'MaxItems': '1'} + ) + stubber.add_response( + 'list_hosted_zones_by_name', + list_hosted_zones_by_name_resp, + {'DNSName': 'unit.tests.', 'MaxItems': '1'} + ) + + plan = provider.plan(self.expected) + self.assertEquals(9, len(plan.changes)) + def test_extra_change_no_health_check(self): provider, stubber = self._get_stubbed_provider() From da1b732f29f598c863032df05cece11acca5d84f Mon Sep 17 00:00:00 2001 From: slandry Date: Wed, 22 Sep 2021 12:26:19 -0400 Subject: [PATCH 2/3] add conditional and test for zone not exists by name --- octodns/provider/route53.py | 11 ++++---- tests/test_octodns_provider_route53.py | 37 ++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 2bf59a5..cd0b720 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -669,11 +669,12 @@ class Route53Provider(BaseProvider): response = self._conn.list_hosted_zones_by_name( DNSName=name, MaxItems="1" ) - if len(response['HostedZones']) == 1: - # if there is a single response - id = response['HostedZones'][0]['Id'] - self.log.debug(id) - return id + if len(response['HostedZones']) != 0: + # if there is a response that starts with the name + if response['HostedZones'][0]['Name'].startswith(name): + id = response['HostedZones'][0]['Id'] + self.log.debug('get_zones_by_name: id=%s', id) + return id elif name in self.r53_zones: id = self.r53_zones[name] self.log.debug('_get_zone_id: id=%s', id) diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 136f442..4b3b759 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -1782,6 +1782,43 @@ class TestRoute53Provider(TestCase): self.assertEquals([], extra) stubber.assert_no_pending_responses() + def test_zone_not_found_get_zones_by_name(self): + provider = Route53Provider( + 'test', 'abc', '123', get_zones_by_name=True) + + # Use the stubber + stubber = Stubber(provider._conn) + stubber.activate() + + list_hosted_zones_by_name_resp = { + 'HostedZones': [{ + 'Id': 'z43', + 'Name': 'bad.tests.', + 'CallerReference': 'abc', + 'Config': { + 'Comment': 'string', + 'PrivateZone': False + }, + 'ResourceRecordSetCount': 123, + }, ], + 'DNSName': 'unit.tests.', + 'HostedZoneId': 'z42', + 'IsTruncated': False, + 'MaxItems': 'string' + } + + stubber.add_response( + 'list_hosted_zones_by_name', + list_hosted_zones_by_name_resp, + {'DNSName': 'unit.tests.', 'MaxItems': '1'} + ) + + # empty is empty + desired = Zone('unit.tests.', []) + extra = provider._extra_changes(desired=desired, changes=[]) + self.assertEquals([], extra) + stubber.assert_no_pending_responses() + def test_plan_with_get_zones_by_name(self): provider = Route53Provider( 'test', 'abc', '123', get_zones_by_name=True) From 3c20877775aca99122aec369f614147ee2a9cbc9 Mon Sep 17 00:00:00 2001 From: slandry Date: Wed, 22 Sep 2021 14:25:04 -0400 Subject: [PATCH 3/3] rewrite property function to better handle caching of zones --- octodns/provider/route53.py | 71 +++++++++++++++----------- tests/test_octodns_provider_route53.py | 45 +++++++++++----- 2 files changed, 71 insertions(+), 45 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index cd0b720..3f6f17e 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -643,40 +643,49 @@ class Route53Provider(BaseProvider): self._r53_rrsets = {} self._health_checks = None - @property - def r53_zones(self): - if self._r53_zones is None: - self.log.debug('r53_zones: loading') - zones = {} - more = True - start = {} - while more: - resp = self._conn.list_hosted_zones(**start) - for z in resp['HostedZones']: - zones[z['Name']] = z['Id'] - more = resp['IsTruncated'] - start['Marker'] = resp.get('NextMarker', None) - - self._r53_zones = zones + def _get_zone_id_by_name(self, name): + # attempt to get zone by name + # limited to one as this should be unique + id = None + resp = self._conn.list_hosted_zones_by_name( + DNSName=name, MaxItems="1" + ) + if len(resp['HostedZones']) != 0: + # if there is a response that starts with the name + if resp['HostedZones'][0]['Name'].startswith(name): + id = resp['HostedZones'][0]['Id'] + self.log.debug('get_zones_by_name: id=%s', id) + return id - return self._r53_zones + def update_r53_zones(self, name): + if self._r53_zones is None: + if self.get_zones_by_name: + id = self._get_zone_id_by_name(name) + zones = {} + zones[name] = id + self._r53_zones = zones + else: + self.log.debug('r53_zones: loading') + zones = {} + more = True + start = {} + while more: + resp = self._conn.list_hosted_zones(**start) + for z in resp['HostedZones']: + zones[z['Name']] = z['Id'] + more = resp['IsTruncated'] + start['Marker'] = resp.get('NextMarker', None) + self._r53_zones = zones + else: + if name not in self._r53_zones and self.get_zones_by_name: + id = self._get_zone_id_by_name(name) + self._r53_zones[name] = id def _get_zone_id(self, name, create=False): self.log.debug('_get_zone_id: name=%s', name) - if self.get_zones_by_name: - # attempt to get zone by name - # limited to one as this should be unique - response = self._conn.list_hosted_zones_by_name( - DNSName=name, MaxItems="1" - ) - if len(response['HostedZones']) != 0: - # if there is a response that starts with the name - if response['HostedZones'][0]['Name'].startswith(name): - id = response['HostedZones'][0]['Id'] - self.log.debug('get_zones_by_name: id=%s', id) - return id - elif name in self.r53_zones: - id = self.r53_zones[name] + self.update_r53_zones(name) + if name in self._r53_zones: + id = self._r53_zones[name] self.log.debug('_get_zone_id: id=%s', id) return id if create: @@ -691,7 +700,7 @@ class Route53Provider(BaseProvider): else: resp = self._conn.create_hosted_zone(Name=name, CallerReference=ref) - self.r53_zones[name] = id = resp['HostedZone']['Id'] + self._r53_zones[name] = id = resp['HostedZone']['Id'] return id return None diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 4b3b759..770d6a8 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -1753,7 +1753,7 @@ class TestRoute53Provider(TestCase): stubber = Stubber(provider._conn) stubber.activate() - list_hosted_zones_by_name_resp = { + list_hosted_zones_by_name_resp_1 = { 'HostedZones': [{ 'Id': 'z42', 'Name': 'unit.tests.', @@ -1770,9 +1770,26 @@ class TestRoute53Provider(TestCase): 'MaxItems': 'string' } + list_hosted_zones_by_name_resp_2 = { + 'HostedZones': [{ + 'Id': 'z43', + 'Name': 'unit2.tests.', + 'CallerReference': 'abc', + 'Config': { + 'Comment': 'string', + 'PrivateZone': False + }, + 'ResourceRecordSetCount': 123, + }, ], + 'DNSName': 'unit2.tests.', + 'HostedZoneId': 'z43', + 'IsTruncated': False, + 'MaxItems': 'string' + } + stubber.add_response( 'list_hosted_zones_by_name', - list_hosted_zones_by_name_resp, + list_hosted_zones_by_name_resp_1, {'DNSName': 'unit.tests.', 'MaxItems': '1'} ) @@ -1782,6 +1799,18 @@ class TestRoute53Provider(TestCase): self.assertEquals([], extra) stubber.assert_no_pending_responses() + stubber.add_response( + 'list_hosted_zones_by_name', + list_hosted_zones_by_name_resp_2, + {'DNSName': 'unit2.tests.', 'MaxItems': '1'} + ) + + # empty is empty + desired = Zone('unit2.tests.', []) + extra = provider._extra_changes(desired=desired, changes=[]) + self.assertEquals([], extra) + stubber.assert_no_pending_responses() + def test_zone_not_found_get_zones_by_name(self): provider = Route53Provider( 'test', 'abc', '123', get_zones_by_name=True) @@ -1837,18 +1866,6 @@ class TestRoute53Provider(TestCase): 'MaxItems': 'string' } - # list_hosted_zones_by_name gets called 3 times in this process - # so adding 3 responses - stubber.add_response( - 'list_hosted_zones_by_name', - list_hosted_zones_by_name_resp, - {'DNSName': 'unit.tests.', 'MaxItems': '1'} - ) - stubber.add_response( - 'list_hosted_zones_by_name', - list_hosted_zones_by_name_resp, - {'DNSName': 'unit.tests.', 'MaxItems': '1'} - ) stubber.add_response( 'list_hosted_zones_by_name', list_hosted_zones_by_name_resp,