Merge branch 'master' into fix-windows-filename-problems

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
@@ -330,18 +329,36 @@ 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 = 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
         for zone in list_zones(self._resource_group):
-            self._azure_zones.add(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 +373,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.

octodns/record/__init__.py

@@ -758,7 +758,9 @@ class _TargetValue(object):
             reasons.append('empty value')
         elif not data:
             reasons.append('missing value')
-        elif not FQDN(data, allow_underscores=True).is_valid:
+        # NOTE: FQDN complains if the data it receives isn't a str, it doesn't
+        # allow unicode... This is likely specific to 2.7
+        elif not FQDN(str(data), allow_underscores=True).is_valid:
             reasons.append('{} value "{}" is not a valid FQDN'
                            .format(_type, data))
         elif not data.endswith('.'):

tests/test_octodns_provider_azuredns.py

@@ -388,8 +388,12 @@ class TestAzureDnsProvider(TestCase):
 
             :type return: AzureProvider
         '''
-        return AzureProvider('mock_id', 'mock_client', 'mock_key',
-                             'mock_directory', 'mock_sub', 'mock_rg')
+        provider = AzureProvider('mock_id', 'mock_client', 'mock_key',
+                                 'mock_directory', 'mock_sub', 'mock_rg'
+                                 )
+        # Fetch the client to force it to load the creds
+        provider._dns_client
+        return provider
 
     def test_populate_records(self):
         provider = self._get_provider()
@@ -498,32 +502,44 @@ 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()