From 0b116a57b96c66e4e051e980d372820618937260 Mon Sep 17 00:00:00 2001 From: Robert Reichel Date: Tue, 2 Feb 2021 12:18:40 -0500 Subject: [PATCH 01/13] Modify Azure DNS Client logic to lazy load Lazy loading the Azure DNS client ensures that the necessary network calls only occur when it is time to actually do something with the client. --- octodns/provider/azuredns.py | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index a1ef6fe..888f5e3 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -323,6 +323,22 @@ class AzureProvider(BaseProvider): SUPPORTS = set(('A', 'AAAA', 'CAA', 'CNAME', 'MX', 'NS', 'PTR', 'SRV', 'TXT')) + def _get_dns_client(self): + if not self._dns_client_handle: + # Not initialized yet, we need to do that. + credentials = ServicePrincipalCredentials( + self._dns_client_client_id, + secret=self._dns_client_key, + tenant=self._dns_client_directory_id + ) + self._dns_client_handle = DnsManagementClient( + credentials, + self._dns_client_subscription_id + ) + + return self._dns_client_handle + + def __init__(self, id, client_id, key, directory_id, sub_id, resource_group, *args, **kwargs): self.log = logging.getLogger('AzureProvider[{}]'.format(id)) @@ -330,10 +346,13 @@ class AzureProvider(BaseProvider): 'key=***, directory_id:%s', id, client_id, directory_id) super(AzureProvider, self).__init__(id, *args, **kwargs) - credentials = ServicePrincipalCredentials( - client_id, secret=key, tenant=directory_id - ) - self._dns_client = DnsManagementClient(credentials, sub_id) + # Store necessary initialization params + self._dns_client_handle = None + self._dns_client_client_id = client_id + self._dns_client_key = key + self._dns_client_directory_id = directory_id + self._dns_client_subscription_id = sub_id + self._dns_client = property(_get_dns_client, _set_dns_client) self._resource_group = resource_group self._azure_zones = set() From 6146be8ec3ad861320f50f1d8542f3cd020a468b Mon Sep 17 00:00:00 2001 From: Robert Reichel Date: Tue, 2 Feb 2021 12:21:00 -0500 Subject: [PATCH 02/13] Remove unused set_dns_client property --- octodns/provider/azuredns.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 888f5e3..9561385 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -352,7 +352,7 @@ class AzureProvider(BaseProvider): self._dns_client_key = key self._dns_client_directory_id = directory_id self._dns_client_subscription_id = sub_id - self._dns_client = property(_get_dns_client, _set_dns_client) + self._dns_client = property(_get_dns_client) self._resource_group = resource_group self._azure_zones = set() From 6fb77c08109c5a4d4537280fe291fb921ad7b28d Mon Sep 17 00:00:00 2001 From: Robert Reichel Date: Tue, 2 Feb 2021 12:21:48 -0500 Subject: [PATCH 03/13] Add set DNS client logic if needed for testing --- octodns/provider/azuredns.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 9561385..2431e7d 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -337,6 +337,8 @@ class AzureProvider(BaseProvider): ) return self._dns_client_handle + def _set_dns_client(self, client) + self.dns_client_handle = client def __init__(self, id, client_id, key, directory_id, sub_id, @@ -352,7 +354,7 @@ class AzureProvider(BaseProvider): self._dns_client_key = key self._dns_client_directory_id = directory_id self._dns_client_subscription_id = sub_id - self._dns_client = property(_get_dns_client) + self._dns_client = property(_get_dns_client, _set_dns_client) self._resource_group = resource_group self._azure_zones = set() From 975376d09dd58ef9f02aa522ef3842443cb595a4 Mon Sep 17 00:00:00 2001 From: Robert Reichel Date: Tue, 2 Feb 2021 12:26:04 -0500 Subject: [PATCH 04/13] Remove trailing whitespace --- octodns/provider/azuredns.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 2431e7d..0162e18 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -327,19 +327,19 @@ class AzureProvider(BaseProvider): if not self._dns_client_handle: # Not initialized yet, we need to do that. credentials = ServicePrincipalCredentials( - self._dns_client_client_id, - secret=self._dns_client_key, + self._dns_client_client_id, + secret=self._dns_client_key, tenant=self._dns_client_directory_id ) self._dns_client_handle = DnsManagementClient( - credentials, + credentials, self._dns_client_subscription_id ) - + return self._dns_client_handle + def _set_dns_client(self, client) self.dns_client_handle = client - def __init__(self, id, client_id, key, directory_id, sub_id, resource_group, *args, **kwargs): From 5e78d07a97c2634bf41cb67b2510240ca502726b Mon Sep 17 00:00:00 2001 From: Robert Reichel Date: Tue, 2 Feb 2021 12:31:04 -0500 Subject: [PATCH 05/13] Use @property in lieu of property() --- octodns/provider/azuredns.py | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 0162e18..53b5e48 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -323,24 +323,6 @@ class AzureProvider(BaseProvider): SUPPORTS = set(('A', 'AAAA', 'CAA', 'CNAME', 'MX', 'NS', 'PTR', 'SRV', 'TXT')) - def _get_dns_client(self): - if not self._dns_client_handle: - # Not initialized yet, we need to do that. - credentials = ServicePrincipalCredentials( - self._dns_client_client_id, - secret=self._dns_client_key, - tenant=self._dns_client_directory_id - ) - self._dns_client_handle = DnsManagementClient( - credentials, - self._dns_client_subscription_id - ) - - return self._dns_client_handle - - def _set_dns_client(self, client) - self.dns_client_handle = client - def __init__(self, id, client_id, key, directory_id, sub_id, resource_group, *args, **kwargs): self.log = logging.getLogger('AzureProvider[{}]'.format(id)) @@ -354,10 +336,25 @@ class AzureProvider(BaseProvider): self._dns_client_key = key self._dns_client_directory_id = directory_id self._dns_client_subscription_id = sub_id - self._dns_client = property(_get_dns_client, _set_dns_client) + self._dns_client = None + self._resource_group = resource_group self._azure_zones = set() + @property + def _dns_client(self) + if self._dns_client is None: + credentials = ServicePrincipalCredentials( + self._dns_client_client_id, + secret=self._dns_client_key, + tenant=self._dns_client_directory_id + ) + self._dns_client = DnsManagementClient( + credentials, + self._dns_client_subscription_id + ) + return self._dns_client + def _populate_zones(self): self.log.debug('azure_zones: loading') list_zones = self._dns_client.zones.list_by_resource_group From 831d1cc30b003f5e8f9f444c5a805777183f8007 Mon Sep 17 00:00:00 2001 From: Robert Reichel Date: Tue, 2 Feb 2021 12:34:44 -0500 Subject: [PATCH 06/13] Add missing colon --- octodns/provider/azuredns.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 53b5e48..f41f892 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -342,7 +342,7 @@ class AzureProvider(BaseProvider): self._azure_zones = set() @property - def _dns_client(self) + def _dns_client(self): if self._dns_client is None: credentials = ServicePrincipalCredentials( self._dns_client_client_id, From a58371e3bbf896f8c5bd1edb91f0cfaccd7ae921 Mon Sep 17 00:00:00 2001 From: Robert Reichel Date: Tue, 2 Feb 2021 12:40:57 -0500 Subject: [PATCH 07/13] Apply suggestions from code review Co-authored-by: Ross McFarland --- octodns/provider/azuredns.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index f41f892..046ddfe 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -336,24 +336,24 @@ class AzureProvider(BaseProvider): self._dns_client_key = key self._dns_client_directory_id = directory_id self._dns_client_subscription_id = sub_id - self._dns_client = None + self.__dns_client = None self._resource_group = resource_group self._azure_zones = set() @property def _dns_client(self): - if self._dns_client is None: + if self.__dns_client is None: credentials = ServicePrincipalCredentials( self._dns_client_client_id, secret=self._dns_client_key, tenant=self._dns_client_directory_id ) - self._dns_client = DnsManagementClient( + self.__dns_client = DnsManagementClient( credentials, self._dns_client_subscription_id ) - return self._dns_client + return self.__dns_client def _populate_zones(self): self.log.debug('azure_zones: loading') From c2c05a761e6e6f83495f4bfda6f4ea5dadc47d48 Mon Sep 17 00:00:00 2001 From: Robert Reichel Date: Tue, 2 Feb 2021 13:14:29 -0500 Subject: [PATCH 08/13] Fix patching --- tests/test_octodns_provider_azuredns.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py index 2b2c1e7..19cc826 100644 --- a/tests/test_octodns_provider_azuredns.py +++ b/tests/test_octodns_provider_azuredns.py @@ -376,8 +376,6 @@ class TestAzureDnsProvider(TestCase): def _provider(self): return self._get_provider('mock_spc', 'mock_dns_client') - @patch('octodns.provider.azuredns.DnsManagementClient') - @patch('octodns.provider.azuredns.ServicePrincipalCredentials') def _get_provider(self, mock_spc, mock_dns_client): '''Returns a mock AzureProvider object to use in testing. @@ -390,7 +388,9 @@ class TestAzureDnsProvider(TestCase): ''' return AzureProvider('mock_id', 'mock_client', 'mock_key', 'mock_directory', 'mock_sub', 'mock_rg') - + + @patch('octodns.provider.azuredns.DnsManagementClient') + @patch('octodns.provider.azuredns.ServicePrincipalCredentials') def test_populate_records(self): provider = self._get_provider() @@ -501,7 +501,9 @@ class TestAzureDnsProvider(TestCase): exists = provider.populate(zone) self.assertTrue(exists) self.assertEquals(len(zone.records), 17) - + + @patch('octodns.provider.azuredns.DnsManagementClient') + @patch('octodns.provider.azuredns.ServicePrincipalCredentials') def test_populate_zone(self): provider = self._get_provider() @@ -512,7 +514,9 @@ class TestAzureDnsProvider(TestCase): provider._populate_zones() self.assertEquals(len(provider._azure_zones), 1) - + + @patch('octodns.provider.azuredns.DnsManagementClient') + @patch('octodns.provider.azuredns.ServicePrincipalCredentials') def test_bad_zone_response(self): provider = self._get_provider() @@ -539,6 +543,8 @@ class TestAzureDnsProvider(TestCase): self.assertEquals(19, provider.apply(Plan(zone, zone, deletes, True))) + @patch('octodns.provider.azuredns.DnsManagementClient') + @patch('octodns.provider.azuredns.ServicePrincipalCredentials') def test_create_zone(self): provider = self._get_provider() @@ -555,6 +561,8 @@ class TestAzureDnsProvider(TestCase): self.assertEquals(19, provider.apply(Plan(None, desired, changes, True))) + @patch('octodns.provider.azuredns.DnsManagementClient') + @patch('octodns.provider.azuredns.ServicePrincipalCredentials') def test_check_zone_no_create(self): provider = self._get_provider() From 1982fbddac3533e87a3627e6fe2a3996dd0ab0b9 Mon Sep 17 00:00:00 2001 From: Robert Reichel Date: Tue, 2 Feb 2021 13:18:04 -0500 Subject: [PATCH 09/13] Update patching --- tests/test_octodns_provider_azuredns.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py index 19cc826..030bf9f 100644 --- a/tests/test_octodns_provider_azuredns.py +++ b/tests/test_octodns_provider_azuredns.py @@ -375,7 +375,9 @@ class Test_CheckAzureAlias(TestCase): class TestAzureDnsProvider(TestCase): def _provider(self): return self._get_provider('mock_spc', 'mock_dns_client') - + + @patch('octodns.provider.azuredns.DnsManagementClient') + @patch('octodns.provider.azuredns.ServicePrincipalCredentials') def _get_provider(self, mock_spc, mock_dns_client): '''Returns a mock AzureProvider object to use in testing. @@ -386,8 +388,11 @@ class TestAzureDnsProvider(TestCase): :type return: AzureProvider ''' - return AzureProvider('mock_id', 'mock_client', 'mock_key', + provider = AzureProvider('mock_id', 'mock_client', 'mock_key', 'mock_directory', 'mock_sub', 'mock_rg') + # Fetch the client to force it to load the creds + client = provider._dns_client + return provider @patch('octodns.provider.azuredns.DnsManagementClient') @patch('octodns.provider.azuredns.ServicePrincipalCredentials') From 950090bb547aa58591d3535c2986a8b46b5ceba6 Mon Sep 17 00:00:00 2001 From: Robert Reichel Date: Tue, 2 Feb 2021 13:23:02 -0500 Subject: [PATCH 10/13] Update test_octodns_provider_azuredns.py --- tests/test_octodns_provider_azuredns.py | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py index 030bf9f..8f840f0 100644 --- a/tests/test_octodns_provider_azuredns.py +++ b/tests/test_octodns_provider_azuredns.py @@ -375,7 +375,7 @@ class Test_CheckAzureAlias(TestCase): class TestAzureDnsProvider(TestCase): def _provider(self): return self._get_provider('mock_spc', 'mock_dns_client') - + @patch('octodns.provider.azuredns.DnsManagementClient') @patch('octodns.provider.azuredns.ServicePrincipalCredentials') def _get_provider(self, mock_spc, mock_dns_client): @@ -393,9 +393,7 @@ class TestAzureDnsProvider(TestCase): # Fetch the client to force it to load the creds client = provider._dns_client return provider - - @patch('octodns.provider.azuredns.DnsManagementClient') - @patch('octodns.provider.azuredns.ServicePrincipalCredentials') + def test_populate_records(self): provider = self._get_provider() @@ -506,9 +504,7 @@ class TestAzureDnsProvider(TestCase): exists = provider.populate(zone) self.assertTrue(exists) self.assertEquals(len(zone.records), 17) - - @patch('octodns.provider.azuredns.DnsManagementClient') - @patch('octodns.provider.azuredns.ServicePrincipalCredentials') + def test_populate_zone(self): provider = self._get_provider() @@ -519,9 +515,7 @@ class TestAzureDnsProvider(TestCase): provider._populate_zones() self.assertEquals(len(provider._azure_zones), 1) - - @patch('octodns.provider.azuredns.DnsManagementClient') - @patch('octodns.provider.azuredns.ServicePrincipalCredentials') + def test_bad_zone_response(self): provider = self._get_provider() @@ -548,8 +542,6 @@ class TestAzureDnsProvider(TestCase): self.assertEquals(19, provider.apply(Plan(zone, zone, deletes, True))) - @patch('octodns.provider.azuredns.DnsManagementClient') - @patch('octodns.provider.azuredns.ServicePrincipalCredentials') def test_create_zone(self): provider = self._get_provider() @@ -566,8 +558,6 @@ class TestAzureDnsProvider(TestCase): self.assertEquals(19, provider.apply(Plan(None, desired, changes, True))) - @patch('octodns.provider.azuredns.DnsManagementClient') - @patch('octodns.provider.azuredns.ServicePrincipalCredentials') def test_check_zone_no_create(self): provider = self._get_provider() From d94db03f5b32dc95d8f63cfe7c6a3a8eec8542fd Mon Sep 17 00:00:00 2001 From: Robert Reichel Date: Tue, 2 Feb 2021 13:26:28 -0500 Subject: [PATCH 11/13] Fix lint --- tests/test_octodns_provider_azuredns.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py index 8f840f0..2b048c0 100644 --- a/tests/test_octodns_provider_azuredns.py +++ b/tests/test_octodns_provider_azuredns.py @@ -389,7 +389,8 @@ class TestAzureDnsProvider(TestCase): :type return: AzureProvider ''' provider = AzureProvider('mock_id', 'mock_client', 'mock_key', - 'mock_directory', 'mock_sub', 'mock_rg') + 'mock_directory', 'mock_sub', 'mock_rg' + ) # Fetch the client to force it to load the creds client = provider._dns_client return provider From 290a6303037a817fa7a3da80562946ef7b8c69b5 Mon Sep 17 00:00:00 2001 From: Robert Reichel Date: Tue, 2 Feb 2021 13:29:52 -0500 Subject: [PATCH 12/13] Update test_octodns_provider_azuredns.py --- tests/test_octodns_provider_azuredns.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py index 2d6f08d..b7721b7 100644 --- a/tests/test_octodns_provider_azuredns.py +++ b/tests/test_octodns_provider_azuredns.py @@ -390,7 +390,7 @@ class TestAzureDnsProvider(TestCase): ''' provider = AzureProvider('mock_id', 'mock_client', 'mock_key', 'mock_directory', 'mock_sub', 'mock_rg' - ) + ) # Fetch the client to force it to load the creds client = provider._dns_client return provider From ec0b309437369cefdda372aa9723e550201072bc Mon Sep 17 00:00:00 2001 From: Robert Reichel Date: Tue, 2 Feb 2021 13:34:15 -0500 Subject: [PATCH 13/13] Remove unused client ref --- tests/test_octodns_provider_azuredns.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py index b7721b7..9523b51 100644 --- a/tests/test_octodns_provider_azuredns.py +++ b/tests/test_octodns_provider_azuredns.py @@ -392,7 +392,7 @@ class TestAzureDnsProvider(TestCase): 'mock_directory', 'mock_sub', 'mock_rg' ) # Fetch the client to force it to load the creds - client = provider._dns_client + provider._dns_client return provider def test_populate_records(self):