From 97feaa7823878a4f87e7caa18301eb710528e2cc Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 25 Jan 2021 15:32:30 -0800 Subject: [PATCH 1/7] Rename extention zonefile test to avoid existing unit.tests. --- tests/test_octodns_source_axfr.py | 6 +++--- .../{unit.tests.extension => ext.unit.tests.extension} | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) rename tests/zones/{unit.tests.extension => ext.unit.tests.extension} (61%) diff --git a/tests/test_octodns_source_axfr.py b/tests/test_octodns_source_axfr.py index f808166..a1d2e1c 100644 --- a/tests/test_octodns_source_axfr.py +++ b/tests/test_octodns_source_axfr.py @@ -45,12 +45,12 @@ class TestAxfrSource(TestCase): class TestZoneFileSource(TestCase): source = ZoneFileSource('test', './tests/zones') - source_extension = ZoneFileSource('test', './tests/zones', 'extension') def test_zonefiles_with_extension(self): + source = ZoneFileSource('test', './tests/zones', 'extension') # Load zonefiles with a specified file extension - valid = Zone('unit.tests.', []) - self.source_extension.populate(valid) + valid = Zone('ext.unit.tests.', []) + source.populate(valid) self.assertEquals(1, len(valid.records)) def test_populate(self): diff --git a/tests/zones/unit.tests.extension b/tests/zones/ext.unit.tests.extension similarity index 61% rename from tests/zones/unit.tests.extension rename to tests/zones/ext.unit.tests.extension index 2821d9a..2ed7ac6 100644 --- a/tests/zones/unit.tests.extension +++ b/tests/zones/ext.unit.tests.extension @@ -1,5 +1,5 @@ -$ORIGIN unit.tests. -@ 3600 IN SOA ns1.unit.tests. root.unit.tests. ( +$ORIGIN ext.unit.tests. +@ 3600 IN SOA ns1.ext.unit.tests. root.ext.unit.tests. ( 2018071501 ; Serial 3600 ; Refresh (1 hour) 600 ; Retry (10 minutes) @@ -8,5 +8,5 @@ $ORIGIN unit.tests. ) ; NS Records -@ 3600 IN NS ns1.unit.tests. -@ 3600 IN NS ns2.unit.tests. +@ 3600 IN NS ns1.ext.unit.tests. +@ 3600 IN NS ns2.ext.unit.tests. From c08d4ac88f9e8d0e02435e9ec27a57706ca62cc5 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 25 Jan 2021 15:35:37 -0800 Subject: [PATCH 2/7] Look for zone filename not zone_name in axfr directory listing --- octodns/source/axfr.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/octodns/source/axfr.py b/octodns/source/axfr.py index 1ca2c67..ed3f98f 100644 --- a/octodns/source/axfr.py +++ b/octodns/source/axfr.py @@ -229,14 +229,16 @@ class ZoneFileSource(AxfrBaseSource): self._zone_records = {} def _load_zone_file(self, zone_name): + + zone_filename = zone_name + if self.file_extension: + zone_filename = '{}{}'.format(zone_name, + self.file_extension.lstrip('.')) + zonefiles = listdir(self.directory) - if zone_name in zonefiles: + if zone_filename in zonefiles: try: - filename = zone_name - if self.file_extension: - filename = '{}{}'.format(zone_name, - self.file_extension.lstrip('.')) - z = dns.zone.from_file(join(self.directory, filename), + z = dns.zone.from_file(join(self.directory, zone_filename), zone_name, relativize=False, check_origin=self.check_origin) except DNSException as error: From a8505d66f13421f1c67242f4ecbf1fbeae85aef5 Mon Sep 17 00:00:00 2001 From: Robert Reichel Date: Fri, 29 Jan 2021 15:11:27 -0500 Subject: [PATCH 3/7] Improve checking and creating Azure DNS Zones This change improves the process for checking for AzureDNS zones by using the known set and not relying upon custom error handling. Since the provider already fetches the zones, octodns doesn't need to make a second call to check for the existence of the zone - _populate_zones already does that for us. --- octodns/provider/azuredns.py | 36 ++++++++++--------------- tests/test_octodns_provider_azuredns.py | 30 ++++++++++++++------- 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index a1ef6fe..0224f06 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -341,7 +341,8 @@ class AzureProvider(BaseProvider): self.log.debug('azure_zones: loading') list_zones = self._dns_client.zones.list_by_resource_group for zone in list_zones(self._resource_group): - self._azure_zones.add(zone.name) + if zone.name: + self._azure_zones.add(zone.name.rstrip('.')) def _check_zone(self, name, create=False): '''Checks whether a zone specified in a source exist in Azure server. @@ -356,29 +357,20 @@ class AzureProvider(BaseProvider): :type return: str or None ''' - self.log.debug('_check_zone: name=%s', name) - try: - if name in self._azure_zones: - return name - self._dns_client.zones.get(self._resource_group, name) + self.log.debug('_check_zone: name=%s create=%s', name, create) + # Check if the zone already exists in our set + if name in self._azure_zones: + return name + # If not, and its time to create, lets do it. + if create: + self.log.debug('_check_zone:no matching zone; creating %s', name) + create_zone = self._dns_client.zones.create_or_update + create_zone(self._resource_group, name, Zone(location='global')) self._azure_zones.add(name) return name - except CloudError as err: - msg = 'The Resource \'Microsoft.Network/dnszones/{}\''.format(name) - msg += ' under resource group \'{}\''.format(self._resource_group) - msg += ' was not found.' - if msg == err.message: - # Then the only error is that the zone doesn't currently exist - if create: - self.log.debug('_check_zone:no matching zone; creating %s', - name) - create_zone = self._dns_client.zones.create_or_update - create_zone(self._resource_group, name, - Zone(location='global')) - return name - else: - return - raise + else: + # Else return nothing (aka false) + return def populate(self, zone, target=False, lenient=False): '''Required function of manager.py to collect records from zone. diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py index 2b2c1e7..3c93a27 100644 --- a/tests/test_octodns_provider_azuredns.py +++ b/tests/test_octodns_provider_azuredns.py @@ -498,32 +498,42 @@ class TestAzureDnsProvider(TestCase): record_list = provider._dns_client.record_sets.list_by_dns_zone record_list.return_value = rs + zone_list = provider._dns_client.zones.list_by_resource_group + zone_list.return_value = [zone] + exists = provider.populate(zone) - self.assertTrue(exists) + self.assertEquals(len(zone.records), 17) + self.assertTrue(exists) def test_populate_zone(self): provider = self._get_provider() zone_list = provider._dns_client.zones.list_by_resource_group - zone_list.return_value = [AzureZone(location='global'), - AzureZone(location='global')] + zone_1 = AzureZone(location='global') + # This is far from ideal but the zone constructor doesn't let me set it on creation + zone_1.name = "zone-1" + zone_2 = AzureZone(location='global') + # This is far from ideal but the zone constructor doesn't let me set it on creation + zone_2.name = "zone-2" + zone_list.return_value = [zone_1, + zone_2, + zone_1] provider._populate_zones() - self.assertEquals(len(provider._azure_zones), 1) + # This should be returning two zones since two zones are the same + self.assertEquals(len(provider._azure_zones), 2) def test_bad_zone_response(self): provider = self._get_provider() _get = provider._dns_client.zones.get _get.side_effect = CloudError(Mock(status=404), 'Azure Error') - trip = False - try: - provider._check_zone('unit.test', create=False) - except CloudError: - trip = True - self.assertEquals(trip, True) + self.assertEquals( + provider._check_zone('unit.test', create=False), + None + ) def test_apply(self): provider = self._get_provider() From 83cc7fbe1a5cca6d1f2ae2910378859ca39411a5 Mon Sep 17 00:00:00 2001 From: Robert Reichel Date: Fri, 29 Jan 2021 15:19:19 -0500 Subject: [PATCH 4/7] Appease the linter --- tests/test_octodns_provider_azuredns.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py index 3c93a27..cb790ea 100644 --- a/tests/test_octodns_provider_azuredns.py +++ b/tests/test_octodns_provider_azuredns.py @@ -502,7 +502,7 @@ class TestAzureDnsProvider(TestCase): zone_list.return_value = [zone] exists = provider.populate(zone) - + self.assertEquals(len(zone.records), 17) self.assertTrue(exists) @@ -511,10 +511,12 @@ class TestAzureDnsProvider(TestCase): zone_list = provider._dns_client.zones.list_by_resource_group zone_1 = AzureZone(location='global') - # This is far from ideal but the zone constructor doesn't let me set it on creation + # This is far from ideal but the + # zone constructor doesn't let me set it on creation zone_1.name = "zone-1" zone_2 = AzureZone(location='global') - # This is far from ideal but the zone constructor doesn't let me set it on creation + # This is far from ideal but the + # zone constructor doesn't let me set it on creation zone_2.name = "zone-2" zone_list.return_value = [zone_1, zone_2, From f79ad89e82a04af07a5a15ec0c88932cbf32ddc5 Mon Sep 17 00:00:00 2001 From: Robert Reichel Date: Fri, 29 Jan 2021 15:22:13 -0500 Subject: [PATCH 5/7] Continue linter appeasement --- tests/test_octodns_provider_azuredns.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py index cb790ea..b62762e 100644 --- a/tests/test_octodns_provider_azuredns.py +++ b/tests/test_octodns_provider_azuredns.py @@ -511,7 +511,7 @@ class TestAzureDnsProvider(TestCase): zone_list = provider._dns_client.zones.list_by_resource_group zone_1 = AzureZone(location='global') - # This is far from ideal but the + # This is far from ideal but the # zone constructor doesn't let me set it on creation zone_1.name = "zone-1" zone_2 = AzureZone(location='global') @@ -533,7 +533,7 @@ class TestAzureDnsProvider(TestCase): _get = provider._dns_client.zones.get _get.side_effect = CloudError(Mock(status=404), 'Azure Error') self.assertEquals( - provider._check_zone('unit.test', create=False), + provider._check_zone('unit.test', create=False), None ) From f92fdfce172bb3748e50e0cd2a5a01f2e41e0df1 Mon Sep 17 00:00:00 2001 From: Robert Reichel Date: Fri, 29 Jan 2021 15:24:42 -0500 Subject: [PATCH 6/7] Even more desperate attempts to appease linter --- octodns/provider/azuredns.py | 1 - 1 file changed, 1 deletion(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 0224f06..6a65354 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -7,7 +7,6 @@ from __future__ import absolute_import, division, print_function, \ from azure.common.credentials import ServicePrincipalCredentials from azure.mgmt.dns import DnsManagementClient -from msrestazure.azure_exceptions import CloudError from azure.mgmt.dns.models import ARecord, AaaaRecord, CaaRecord, \ CnameRecord, MxRecord, SrvRecord, NsRecord, PtrRecord, TxtRecord, Zone From 3ab532c5af1c71c5c35a2d6450ba0ab17a55cf31 Mon Sep 17 00:00:00 2001 From: Robert Reichel Date: Fri, 29 Jan 2021 15:30:17 -0500 Subject: [PATCH 7/7] Fix test coverage --- octodns/provider/azuredns.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 6a65354..30e9af5 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -340,8 +340,7 @@ class AzureProvider(BaseProvider): self.log.debug('azure_zones: loading') list_zones = self._dns_client.zones.list_by_resource_group for zone in list_zones(self._resource_group): - if zone.name: - self._azure_zones.add(zone.name.rstrip('.')) + self._azure_zones.add(zone.name.rstrip('.')) def _check_zone(self, name, create=False): '''Checks whether a zone specified in a source exist in Azure server.