Merge pull request #735 from viranch/azuredns-fix-dynamic-updates

Azure DNS dynamic record bug fixes

octodns/provider/azuredns.py

@@ -748,7 +748,10 @@ class AzureProvider(BaseProvider):
         if root_profile.traffic_routing_method != 'Geographic':
             # This record does not use geo fencing, so we skip the Geographic
             # profile hop; let's pretend to be a geo-profile's only endpoint
-            geo_ep = Endpoint(target_resource_id=root_profile.id)
+            geo_ep = Endpoint(
+                name=root_profile.endpoints[0].name.split('--', 1)[0],
+                target_resource_id=root_profile.id
+            )
             geo_ep.target_resource = root_profile
             endpoints = [geo_ep]
         else:
@@ -799,18 +802,14 @@ class AzureProvider(BaseProvider):
                         geos.append(GeoCodes.country_to_code(code))
 
             # build fallback chain from second level priority profile
-            if geo_ep.target_resource_id:
-                target = geo_ep.target_resource
-                if target.traffic_routing_method == 'Priority':
-                    rule_endpoints = target.endpoints
-                    rule_endpoints.sort(key=lambda e: e.priority)
-                else:
-                    # Weighted
-                    geo_ep.name = target.endpoints[0].name.split('--', 1)[0]
-                    rule_endpoints = [geo_ep]
+            if geo_ep.target_resource_id and \
+               geo_ep.target_resource.traffic_routing_method == 'Priority':
+                rule_endpoints = geo_ep.target_resource.endpoints
+                rule_endpoints.sort(key=lambda e: e.priority)
             else:
-                # this geo directly points to the default, so we skip the
-                # Priority profile hop and directly use an external endpoint;
+                # this geo directly points to a pool containing the default
+                # so we skip the Priority profile hop and directly use an
+                # external endpoint or Weighted profile
                 # let's pretend to be a Priority profile's only endpoint
                 rule_endpoints = [geo_ep]
 
@@ -1075,6 +1074,9 @@ class AzureProvider(BaseProvider):
                             if typ == 'CNAME':
                                 target = target[:-1]
                             ep_name = '{}--{}'.format(pool_name, target)
+                            # Endpoint names cannot have colons, drop them
+                            # from IPv6 addresses
+                            ep_name = ep_name.replace(':', '-')
                             if target in defaults:
                                 # mark default
                                 ep_name += '--default--'
@@ -1133,7 +1135,7 @@ class AzureProvider(BaseProvider):
 
                 # append rule profile to top-level geo profile
                 geo_endpoints.append(Endpoint(
-                    name='rule-{}'.format(rule.data['pool']),
+                    name=rule.data['pool'],
                     target_resource_id=rule_profile.id,
                     geo_mapping=geos,
                 ))
@@ -1144,7 +1146,7 @@ class AzureProvider(BaseProvider):
                 if rule_ep.target_resource_id:
                     # point directly to the Weighted pool profile
                     geo_endpoints.append(Endpoint(
-                        name='rule-{}'.format(rule.data['pool']),
+                        name=rule_ep.name,
                         target_resource_id=rule_ep.target_resource_id,
                         geo_mapping=geos,
                     ))

tests/test_octodns_provider_azuredns.py

@@ -697,13 +697,13 @@ class TestAzureDnsProvider(TestCase):
                 endpoints=[
                     Endpoint(
                         geo_mapping=['GEO-AF', 'DE', 'US-CA', 'GEO-AP'],
-                        name='rule-one',
+                        name='one',
                         type=nested,
                         target_resource_id=id_format.format('rule-one'),
                     ),
                     Endpoint(
                         geo_mapping=['WORLD'],
-                        name='rule-two',
+                        name='two',
                         type=nested,
                         target_resource_id=id_format.format('rule-two'),
                     ),
@@ -1231,6 +1231,12 @@ class TestAzureDnsProvider(TestCase):
         record2 = provider._populate_record(zone, azrecord)
         self.assertEqual(record2.dynamic._data(), record.dynamic._data())
 
+        # test that extra changes doesn't show any changes
+        desired = Zone(zone.name, sub_zones=[])
+        desired.add_record(record)
+        changes = provider._extra_changes(zone, desired, [])
+        self.assertEqual(len(changes), 0)
+
     def test_generate_traffic_managers_middle_east(self):
         # check Asia/Middle East test case
         provider, zone, record = self._get_dynamic_package()
@@ -1336,6 +1342,12 @@ class TestAzureDnsProvider(TestCase):
         record2 = provider._populate_record(zone, azrecord)
         self.assertEqual(record2.dynamic._data(), record.dynamic._data())
 
+        # test that extra changes doesn't show any changes
+        desired = Zone(zone.name, sub_zones=[])
+        desired.add_record(record)
+        changes = provider._extra_changes(zone, desired, [])
+        self.assertEqual(len(changes), 0)
+
     def test_dynamic_fallback_is_default(self):
         # test that traffic managers are generated as expected
         provider = self._get_provider()
@@ -1389,6 +1401,12 @@ class TestAzureDnsProvider(TestCase):
         record2 = provider._populate_record(zone, azrecord)
         self.assertEqual(record2.dynamic._data(), record.dynamic._data())
 
+        # test that extra changes doesn't show any changes
+        desired = Zone(zone.name, sub_zones=[])
+        desired.add_record(record)
+        changes = provider._extra_changes(zone, desired, [])
+        self.assertEqual(len(changes), 0)
+
     def test_dynamic_pool_contains_default(self):
         # test that traffic managers are generated as expected
         provider = self._get_provider()
@@ -1459,7 +1477,7 @@ class TestAzureDnsProvider(TestCase):
             monitor_config=_get_monitor(record),
             endpoints=[
                 Endpoint(
-                    name='rule-rr',
+                    name='rr',
                     type=nested,
                     target_resource_id=profiles[0].id,
                     geo_mapping=['GEO-AF'],
@@ -1479,6 +1497,12 @@ class TestAzureDnsProvider(TestCase):
         record2 = provider._populate_record(zone, azrecord)
         self.assertEqual(record2.dynamic._data(), record.dynamic._data())
 
+        # test that extra changes doesn't show any changes
+        desired = Zone(zone.name, sub_zones=[])
+        desired.add_record(record)
+        changes = provider._extra_changes(zone, desired, [])
+        self.assertEqual(len(changes), 0)
+
     def test_dynamic_pool_contains_default_no_geo(self):
         # test that traffic managers are generated as expected
         provider = self._get_provider()
@@ -1553,6 +1577,12 @@ class TestAzureDnsProvider(TestCase):
         record2 = provider._populate_record(zone, azrecord)
         self.assertEqual(record2.dynamic._data(), record.dynamic._data())
 
+        # test that extra changes doesn't show any changes
+        desired = Zone(zone.name, sub_zones=[])
+        desired.add_record(record)
+        changes = provider._extra_changes(zone, desired, [])
+        self.assertEqual(len(changes), 0)
+
     def test_dynamic_last_pool_contains_default_no_geo(self):
         # test that traffic managers are generated as expected
         provider = self._get_provider()
@@ -1655,6 +1685,12 @@ class TestAzureDnsProvider(TestCase):
         record2 = provider._populate_record(zone, azrecord)
         self.assertEqual(record2.dynamic._data(), record.dynamic._data())
 
+        # test that extra changes doesn't show any changes
+        desired = Zone(zone.name, sub_zones=[])
+        desired.add_record(record)
+        changes = provider._extra_changes(zone, desired, [])
+        self.assertEqual(len(changes), 0)
+
     def test_dynamic_unique_traffic_managers(self):
         record = self._get_dynamic_record(zone)
         data = {
@@ -1724,19 +1760,19 @@ class TestAzureDnsProvider(TestCase):
             monitor_config=_get_monitor(record),
             endpoints=[
                 Endpoint(
-                    name='rule-iad',
+                    name='iad',
                     type=nested,
                     target_resource_id=profiles[0].id,
                     geo_mapping=['GEO-EU'],
                 ),
                 Endpoint(
-                    name='rule-lhr',
+                    name='lhr',
                     type=nested,
                     target_resource_id=profiles[1].id,
                     geo_mapping=['GB', 'WORLD'],
                 ),
                 Endpoint(
-                    name='rule-sto',
+                    name='sto',
                     type=nested,
                     target_resource_id=profiles[2].id,
                     geo_mapping=['SE'],
@@ -1756,6 +1792,12 @@ class TestAzureDnsProvider(TestCase):
         record2 = provider._populate_record(zone, azrecord)
         self.assertEqual(record2.dynamic._data(), record.dynamic._data())
 
+        # test that extra changes doesn't show any changes
+        desired = Zone(zone.name, sub_zones=[])
+        desired.add_record(record)
+        changes = provider._extra_changes(zone, desired, [])
+        self.assertEqual(len(changes), 0)
+
     def test_dynamic_A_geo(self):
         provider = self._get_provider()
         external = 'Microsoft.Network/trafficManagerProfiles/externalEndpoints'
@@ -1790,10 +1832,6 @@ class TestAzureDnsProvider(TestCase):
             }
         })
 
-        # test that extra_changes doesn't complain
-        changes = [Create(record)]
-        provider._extra_changes(zone, zone, changes)
-
         profiles = provider._generate_traffic_managers(record)
 
         self.assertEqual(len(profiles), 1)
@@ -1828,7 +1866,7 @@ class TestAzureDnsProvider(TestCase):
         # test that the record and ATM profile gets created
         tm_sync = provider._tm_client.profiles.create_or_update
         create = provider._dns_client.record_sets.create_or_update
-        provider._apply_Create(changes[0])
+        provider._apply_Create(Create(record))
         # A dynamic record can only have 1 profile
         tm_sync.assert_called_once()
         create.assert_called_once()
@@ -1853,6 +1891,12 @@ class TestAzureDnsProvider(TestCase):
         record2 = provider._populate_record(zone, azrecord)
         self.assertEqual(record2.dynamic._data(), record.dynamic._data())
 
+        # test that extra changes doesn't show any changes
+        desired = Zone(zone.name, sub_zones=[])
+        desired.add_record(record)
+        changes = provider._extra_changes(zone, desired, [])
+        self.assertEqual(len(changes), 0)
+
     def test_dynamic_A_fallback(self):
         provider = self._get_provider()
         external = 'Microsoft.Network/trafficManagerProfiles/externalEndpoints'
@@ -1860,7 +1904,7 @@ class TestAzureDnsProvider(TestCase):
         record = Record.new(zone, 'foo', data={
             'type': 'A',
             'ttl': 60,
-            'values': ['8.8.8.8'],
+            'values': ['1.1.1.1', '2.2.2.2'],
             'dynamic': {
                 'pools': {
                     'one': {
@@ -1891,23 +1935,17 @@ class TestAzureDnsProvider(TestCase):
             monitor_config=_get_monitor(record),
             endpoints=[
                 Endpoint(
-                    name='one',
+                    name='one--default--',
                     type=external,
                     target='1.1.1.1',
                     priority=1,
                 ),
                 Endpoint(
-                    name='two',
+                    name='two--default--',
                     type=external,
                     target='2.2.2.2',
                     priority=2,
                 ),
-                Endpoint(
-                    name='--default--',
-                    type=external,
-                    target='8.8.8.8',
-                    priority=3,
-                ),
             ],
         )))
 
@@ -1923,6 +1961,12 @@ class TestAzureDnsProvider(TestCase):
         record2 = provider._populate_record(zone, azrecord)
         self.assertEqual(record2.dynamic._data(), record.dynamic._data())
 
+        # test that extra changes doesn't show any changes
+        desired = Zone(zone.name, sub_zones=[])
+        desired.add_record(record)
+        changes = provider._extra_changes(zone, desired, [])
+        self.assertEqual(len(changes), 0)
+
     def test_dynamic_A_weighted_rr(self):
         provider = self._get_provider()
         external = 'Microsoft.Network/trafficManagerProfiles/externalEndpoints'
@@ -1982,6 +2026,12 @@ class TestAzureDnsProvider(TestCase):
         record2 = provider._populate_record(zone, azrecord)
         self.assertEqual(record2.dynamic._data(), record.dynamic._data())
 
+        # test that extra changes doesn't show any changes
+        desired = Zone(zone.name, sub_zones=[])
+        desired.add_record(record)
+        changes = provider._extra_changes(zone, desired, [])
+        self.assertEqual(len(changes), 0)
+
     def test_dynamic_AAAA(self):
         provider = self._get_provider()
         external = 'Microsoft.Network/trafficManagerProfiles/externalEndpoints'
@@ -2050,6 +2100,12 @@ class TestAzureDnsProvider(TestCase):
         record2 = provider._populate_record(zone, azrecord)
         self.assertEqual(record2.dynamic._data(), record.dynamic._data())
 
+        # test that extra changes doesn't show any changes
+        desired = Zone(zone.name, sub_zones=[])
+        desired.add_record(record)
+        changes = provider._extra_changes(zone, desired, [])
+        self.assertEqual(len(changes), 0)
+
     def test_sync_traffic_managers(self):
         provider, zone, record = self._get_dynamic_package()
         provider._populate_traffic_managers()