From 3c20877775aca99122aec369f614147ee2a9cbc9 Mon Sep 17 00:00:00 2001 From: slandry Date: Wed, 22 Sep 2021 14:25:04 -0400 Subject: [PATCH] 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,