Partial support for dynamic A+AAAA records on Azure

octodns/provider/azuredns.py

@@ -125,6 +125,9 @@ class _AzureRecord(object):
         self.params['ttl'] = record.ttl
 
     def _params_for_A(self, data, key_name, azure_class):
+        if self._record.dynamic and self.traffic_manager:
+            return {'target_resource': self.traffic_manager}
+
         try:
             values = data['values']
         except KeyError:
@@ -132,6 +135,9 @@ class _AzureRecord(object):
         return {key_name: [azure_class(ipv4_address=v) for v in values]}
 
     def _params_for_AAAA(self, data, key_name, azure_class):
+        if self._record.dynamic and self.traffic_manager:
+            return {'target_resource': self.traffic_manager}
+
         try:
             values = data['values']
         except KeyError:
@@ -263,7 +269,10 @@ def _root_traffic_manager_name(record):
     # ATM names can only have letters, numbers and hyphens
     # replace dots with double hyphens to ensure unique mapping,
     # hoping that real life FQDNs won't have double hyphens
-    return record.fqdn[:-1].replace('.', '--')
+    name = record.fqdn[:-1].replace('.', '--')
+    if record._type != 'CNAME':
+        name += '-{}'.format(record._type)
+    return name
 
 
 def _rule_traffic_manager_name(pool, record):
@@ -608,9 +617,33 @@ class AzureProvider(BaseProvider):
                           lenient=lenient)
 
     def _data_for_A(self, azrecord):
+        if azrecord.a_records is None:
+            if azrecord.target_resource.id:
+                return self._data_for_dynamic(azrecord)
+            else:
+                # dynamic record alias is broken, return dummy value and apply
+                # will likely overwrite/fix it
+                self.log.warn('_data_for_A: Missing Traffic Manager '
+                              'alias for dynamic A record %s, forcing '
+                              're-link by setting an invalid value',
+                              azrecord.fqdn)
+                return {'values': ['255.255.255.255']}
+
         return {'values': [ar.ipv4_address for ar in azrecord.a_records]}
 
     def _data_for_AAAA(self, azrecord):
+        if azrecord.aaaa_records is None:
+            if azrecord.target_resource.id:
+                return self._data_for_dynamic(azrecord)
+            else:
+                # dynamic record alias is broken, return dummy value and apply
+                # will likely overwrite/fix it
+                self.log.warn('_data_for_AAAA: Missing Traffic Manager '
+                              'alias for dynamic AAAA record %s, forcing '
+                              're-link by setting an invalid value',
+                              azrecord.fqdn)
+                return {'values': ['::1']}
+
         return {'values': [ar.ipv6_address for ar in azrecord.aaaa_records]}
 
     def _data_for_CAA(self, azrecord):
@@ -667,6 +700,7 @@ class AzureProvider(BaseProvider):
         default = set()
         pools = defaultdict(lambda: {'fallback': None, 'values': []})
         rules = []
+        typ = _parse_azure_type(azrecord.type)
 
         # top level profile
         root_profile = self._get_tm_profile_by_id(azrecord.target_resource.id)
@@ -781,8 +815,10 @@ class AzureProvider(BaseProvider):
 
                 for pool_ep in endpoints:
                     val = pool_ep.target
+                    if typ == 'CNAME':
+                        val = _check_endswith_dot(val)
                     pool['values'].append({
-                        'value': _check_endswith_dot(val),
+                        'value': val,
                         'weight': pool_ep.weight or 1,
                     })
                     if pool_ep.name.endswith('--default--'):
@@ -805,23 +841,58 @@ class AzureProvider(BaseProvider):
                 'pools': pools,
                 'rules': rules,
             },
-            'value': _check_endswith_dot(default[0]),
         }
 
+        if typ == 'CNAME':
+            data['value'] = _check_endswith_dot(default[0])
+        else:
+            data['values'] = default
+
         return data
 
     def _extra_changes(self, existing, desired, changes):
         changed = set()
 
-        # Abort if there are non-CNAME dynamic records
+        # Abort if there are unsupported dynamic records
         for change in changes:
             record = change.record
             changed.add(record)
             typ = record._type
             dynamic = getattr(record, 'dynamic', False)
-            if dynamic and typ != 'CNAME':
-                msg = '{}: Dynamic records in Azure must be of type CNAME'
-                msg = msg.format(record.fqdn)
+            if not dynamic:
+                continue
+            if typ in ['A', 'AAAA']:
+                data = dynamic._data()
+                values = set(record.values)
+                pools = data['pools'].values()
+                seen_values = set()
+                rr = False
+                fallback = False
+                for pool in pools:
+                    vals = pool['values']
+                    if len(vals) > 1:
+                        rr = True
+                    pool_values = set(val['value'] for val in vals)
+                    if pool.get('fallback'):
+                        fallback = True
+                    seen_values.update(pool_values)
+
+                if values != seen_values:
+                    msg = ('{} {}: All pool values of A/AAAA dynamic records '
+                           'must be included in top-level \'values\'.'
+                           .format(record.fqdn, record._type))
+                    raise AzureException(msg)
+
+                geo = any(r.get('geos') for r in data['rules'])
+
+                if [rr, fallback, geo].count(True) > 1:
+                    msg = ('{} {}: A/AAAA dynamic records must use at most '
+                           'one of round-robin, fallback and geo-fencing')
+                    msg = msg.format(record.fqdn, record._type)
+                    raise AzureException(msg)
+            elif typ != 'CNAME':
+                msg = ('{}: Dynamic records in Azure must be of type '
+                       'A/AAAA/CNAME').format(record.fqdn)
                 raise AzureException(msg)
 
         log = self.log.info
@@ -833,11 +904,20 @@ class AzureProvider(BaseProvider):
                 continue
 
             # let's walk through and show what will be changed even if
-            # the record is already be in list of changes
+            # the record is already in list of changes
             added = (record in changed)
 
             active = set()
             profiles = self._generate_traffic_managers(record)
+
+            # this should not happen with above checks, still adding to block
+            # undesired changes
+            if record._type in ['A', 'AAAA'] and len(profiles) > 1:
+                msg = ('Unknown error: {} {} needs more than 1 Traffic '
+                       'Managers which is not supported for A/AAAA dynamic '
+                       'records').format(record.fqdn, record._type)
+                raise AzureException(msg)
+
             for profile in profiles:
                 name = profile.name
                 if name in seen_profiles:
@@ -871,9 +951,9 @@ class AzureProvider(BaseProvider):
     def _generate_tm_profile(self, routing, endpoints, record, label=None):
         # figure out profile name and Traffic Manager FQDN
         name = _root_traffic_manager_name(record)
-        if routing == 'Weighted':
+        if routing == 'Weighted' and label:
             name = _pool_traffic_manager_name(label, record)
-        elif routing == 'Priority':
+        elif routing == 'Priority' and label:
             name = _rule_traffic_manager_name(label, record)
 
         # set appropriate endpoint types
@@ -895,7 +975,7 @@ class AzureProvider(BaseProvider):
             name=name,
             traffic_routing_method=routing,
             dns_config=DnsConfig(
-                relative_name=name,
+                relative_name=name.lower(),
                 ttl=record.ttl,
             ),
             monitor_config=_get_monitor(record),
@@ -906,7 +986,7 @@ class AzureProvider(BaseProvider):
     def _convert_tm_to_root(self, profile, record):
         profile.name = _root_traffic_manager_name(record)
         profile.id = self._profile_name_to_id(profile.name)
-        profile.dns_config.relative_name = profile.name
+        profile.dns_config.relative_name = profile.name.lower()
 
         return profile
 
@@ -914,8 +994,12 @@ class AzureProvider(BaseProvider):
         traffic_managers = []
         pools = record.dynamic.pools
         rules = record.dynamic.rules
+        typ = record._type
 
-        default = record.value[:-1]
+        if typ == 'CNAME':
+            defaults = [record.value[:-1]]
+        else:
+            defaults = record.values
         profile = self._generate_tm_profile
 
         # a pool can be re-used only with a world pool, record the pool
@@ -977,9 +1061,10 @@ class AzureProvider(BaseProvider):
                         for val in pool['values']:
                             target = val['value']
                             # strip trailing dot from CNAME value
-                            target = target[:-1]
+                            if typ == 'CNAME':
+                                target = target[:-1]
                             ep_name = '{}--{}'.format(pool_name, target)
-                            if target == default:
+                            if target in defaults:
                                 # mark default
                                 ep_name += '--default--'
                                 default_seen = True
@@ -1003,9 +1088,11 @@ class AzureProvider(BaseProvider):
                     # Skip Weighted profile hop for single-value pool
                     # append its value as an external endpoint to fallback
                     # rule profile
-                    target = pool['values'][0]['value'][:-1]
+                    target = pool['values'][0]['value']
+                    if typ == 'CNAME':
+                        target = target[:-1]
                     ep_name = pool_name
-                    if target == default:
+                    if target in defaults:
                         # mark default
                         ep_name += '--default--'
                         default_seen = True
@@ -1023,7 +1110,7 @@ class AzureProvider(BaseProvider):
             if not default_seen:
                 rule_endpoints.append(Endpoint(
                     name='--default--',
-                    target=default,
+                    target=defaults[0],
                     priority=priority,
                 ))
 

tests/test_octodns_provider_azuredns.py

@@ -4,6 +4,7 @@
 
 from __future__ import absolute_import, division, print_function, \
     unicode_literals
+from logging import debug
 
 from octodns.record import Create, Update, Delete, Record
 from octodns.provider.azuredns import _AzureRecord, AzureProvider, \
@@ -883,12 +884,13 @@ class TestAzureDnsProvider(TestCase):
 
         # test simple records produce no extra changes
         desired = Zone(name=existing.name, sub_zones=[])
-        desired.add_record(Record.new(desired, 'simple', data={
+        simple = Record.new(desired, 'simple', data={
             'type': record._type,
             'ttl': record.ttl,
             'value': record.value,
-        }))
-        extra = provider._extra_changes(desired, desired, [])
+        })
+        desired.add_record(simple)
+        extra = provider._extra_changes(desired, desired, [Create(simple)])
         self.assertEqual(len(extra), 0)
 
         # test an unchanged dynamic record produces no extra changes
@@ -952,28 +954,28 @@ class TestAzureDnsProvider(TestCase):
         self.assertIsInstance(extra, Update)
         self.assertEqual(extra.new, update_dynamic)
 
-        # test non-CNAME dynamic record throws exception
-        a_dynamic = Record.new(desired, record.name + '3', data={
-            'type': 'A',
+        # test dynamic record of unsupported type throws exception
+        unsupported_dynamic = Record.new(desired, record.name + '3', data={
+            'type': 'DNAME',
             'ttl': record.ttl,
-            'values': ['1.1.1.1'],
+            'value': 'default.unit.tests.',
             'dynamic': {
                 'pools': {
-                    'one': {'values': [{'value': '2.2.2.2'}]},
+                    'one': {'values': [{'value': 'one.unit.tests.'}]},
                 },
                 'rules': [
                     {'pool': 'one'},
                 ],
             },
         })
-        desired.add_record(a_dynamic)
-        changes.append(Create(a_dynamic))
+        desired.add_record(unsupported_dynamic)
+        changes = [Create(unsupported_dynamic)]
         with self.assertRaises(AzureException) as ctx:
             provider._extra_changes(existing, desired, changes)
             self.assertTrue(text_type(ctx).endswith(
                 'must be of type CNAME'
             ))
-        desired._remove_record(a_dynamic)
+        desired._remove_record(unsupported_dynamic)
 
         # test colliding ATM names throws exception
         record1 = Record.new(desired, 'sub.www', data={
@@ -997,6 +999,100 @@ class TestAzureDnsProvider(TestCase):
                 'Collision in Traffic Manager'
             ))
 
+    def test_extra_changes_invalid_dynamic_a(self):
+        provider = self._get_provider()
+
+        desired = Zone(name=zone.name, sub_zones=[])
+
+        # too many test case combinations, here's a method to generate them
+        def record_data(all_values=True, rr=True, fallback=True, geo=True):
+            data = {
+                'type': 'A',
+                'ttl': 60,
+                'dynamic': {
+                    'pools': {
+                        'one': {
+                            'values': [
+                                {'value': '11.11.11.11'},
+                                {'value': '12.12.12.12'},
+                            ],
+                            'fallback': 'two',
+                        },
+                        'two': {
+                            'values': [
+                                {'value': '2.2.2.2'},
+                            ],
+                        },
+                    },
+                    'rules': [
+                        {'geos': ['EU'], 'pool': 'two'},
+                        {'pool': 'one'},
+                    ],
+                }
+            }
+            dynamic = data['dynamic']
+            if not rr:
+                dynamic['pools']['one']['values'].pop()
+            if not fallback:
+                dynamic['pools']['one'].pop('fallback')
+            if not geo:
+                rule = dynamic['rules'].pop(0)
+                if not fallback:
+                    dynamic['pools'].pop(rule['pool'])
+            # put all pool values in default
+            data['values'] = [
+                v['value']
+                for p in dynamic['pools'].values()
+                for v in p['values']
+            ]
+            if not all_values:
+                rm = list(dynamic['pools'].values())[0]['values'][0]['value']
+                data['values'].remove(rm)
+            return data
+
+        # test all combinations
+        values = [True, False]
+        combos = [
+            [arg1, arg2, arg3, arg4]
+            for arg1 in values
+            for arg2 in values
+            for arg3 in values
+            for arg4 in values
+        ]
+        for all_values, rr, fallback, geo in combos:
+            args = [all_values, rr, fallback, geo]
+
+            if not any(args):
+                # all False, invalid use-case
+                continue
+
+            debug('[all_values, rr, fallback, geo] = %s', args)
+            data = record_data(*args)
+            record = Record.new(desired, 'foo', data)
+
+            features = args[1:]
+            if all_values and features.count(True) <= 1:
+                # assert does not raise exception
+                provider._extra_changes(zone, desired, [Create(record)])
+                continue
+
+            with self.assertRaises(AzureException) as ctx:
+                msg = text_type(ctx)
+                provider._extra_changes(zone, desired, [Create(record)])
+                if not all_values:
+                    self.assertTrue('included in top-level \'values\'' in msg)
+                else:
+                    self.assertTrue('at most one of' in msg)
+
+        # multiple profiles should also raise
+        record = Record.new(desired, 'foo',
+                            record_data(True, True, True, True))
+        desired.add_record(record)
+        with self.assertRaises(AzureException) as ctx:
+            # bypass above check by setting changes to empty
+            provider._extra_changes(zone, desired, [])
+            self.assertTrue('more than 1 Traffic Managers' in text_type(ctx))
+
     def test_generate_tm_profile(self):
         provider, zone, record = self._get_dynamic_package()
         profile_gen = provider._generate_tm_profile
@@ -1573,6 +1669,300 @@ class TestAzureDnsProvider(TestCase):
         record2 = provider._populate_record(zone, azrecord)
         self.assertEqual(record2.dynamic._data(), record.dynamic._data())
 
+    def test_dynamic_a_geo(self):
+        provider = self._get_provider()
+        external = 'Microsoft.Network/trafficManagerProfiles/externalEndpoints'
+
+        record = Record.new(zone, 'foo', data={
+            'type': 'A',
+            'ttl': 60,
+            'values': ['1.1.1.1', '2.2.2.2', '3.3.3.3'],
+            'dynamic': {
+                'pools': {
+                    'one': {
+                        'values': [
+                            {'value': '1.1.1.1'},
+                        ],
+                    },
+                    'two': {
+                        'values': [
+                            {'value': '2.2.2.2'},
+                        ],
+                    },
+                    'three': {
+                        'values': [
+                            {'value': '3.3.3.3'},
+                        ],
+                    },
+                },
+                'rules': [
+                    {'geos': ['AS'], 'pool': 'one'},
+                    {'geos': ['AF'], 'pool': 'two'},
+                    {'pool': 'three'},
+                ],
+            }
+        })
+
+        # 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)
+        self.assertTrue(_profile_is_match(profiles[0], Profile(
+            name='foo--unit--tests-A',
+            traffic_routing_method='Geographic',
+            dns_config=DnsConfig(
+                relative_name='foo--unit--tests-a', ttl=record.ttl),
+            monitor_config=_get_monitor(record),
+            endpoints=[
+                Endpoint(
+                    name='one--default--',
+                    type=external,
+                    target='1.1.1.1',
+                    geo_mapping=['GEO-AS'],
+                ),
+                Endpoint(
+                    name='two--default--',
+                    type=external,
+                    target='2.2.2.2',
+                    geo_mapping=['GEO-AF'],
+                ),
+                Endpoint(
+                    name='three--default--',
+                    type=external,
+                    target='3.3.3.3',
+                    geo_mapping=['WORLD'],
+                ),
+            ],
+        )))
+
+        # 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])
+        # A dynamic record can only have 1 profile
+        tm_sync.assert_called_once()
+        create.assert_called_once()
+
+        # test broken alias
+        azrecord = RecordSet(
+            ttl=60, target_resource=SubResource(id=None))
+        azrecord.name = record.name or '@'
+        azrecord.type = 'Microsoft.Network/dnszones/{}'.format(record._type)
+        record2 = provider._populate_record(zone, azrecord)
+        self.assertEqual(record2.values, ['255.255.255.255'])
+
+        # test that same record gets populated back from traffic managers
+        tm_list = provider._tm_client.profiles.list_by_resource_group
+        tm_list.return_value = profiles
+        azrecord = RecordSet(
+            ttl=60,
+            target_resource=SubResource(id=profiles[-1].id),
+        )
+        azrecord.name = record.name or '@'
+        azrecord.type = 'Microsoft.Network/dnszones/{}'.format(record._type)
+        record2 = provider._populate_record(zone, azrecord)
+        self.assertEqual(record2.dynamic._data(), record.dynamic._data())
+
+    def test_dynamic_a_fallback(self):
+        provider = self._get_provider()
+        external = 'Microsoft.Network/trafficManagerProfiles/externalEndpoints'
+
+        record = Record.new(zone, 'foo', data={
+            'type': 'A',
+            'ttl': 60,
+            'values': ['8.8.8.8'],
+            'dynamic': {
+                'pools': {
+                    'one': {
+                        'values': [
+                            {'value': '1.1.1.1'},
+                        ],
+                        'fallback': 'two',
+                    },
+                    'two': {
+                        'values': [
+                            {'value': '2.2.2.2'},
+                        ],
+                    },
+                },
+                'rules': [
+                    {'pool': 'one'},
+                ],
+            }
+        })
+        profiles = provider._generate_traffic_managers(record)
+
+        self.assertEqual(len(profiles), 1)
+        self.assertTrue(_profile_is_match(profiles[0], Profile(
+            name='foo--unit--tests-A',
+            traffic_routing_method='Priority',
+            dns_config=DnsConfig(
+                relative_name='foo--unit--tests-a', ttl=record.ttl),
+            monitor_config=_get_monitor(record),
+            endpoints=[
+                Endpoint(
+                    name='one',
+                    type=external,
+                    target='1.1.1.1',
+                    priority=1,
+                ),
+                Endpoint(
+                    name='two',
+                    type=external,
+                    target='2.2.2.2',
+                    priority=2,
+                ),
+                Endpoint(
+                    name='--default--',
+                    type=external,
+                    target='8.8.8.8',
+                    priority=3,
+                ),
+            ],
+        )))
+
+        # test that same record gets populated back from traffic managers
+        tm_list = provider._tm_client.profiles.list_by_resource_group
+        tm_list.return_value = profiles
+        azrecord = RecordSet(
+            ttl=60,
+            target_resource=SubResource(id=profiles[-1].id),
+        )
+        azrecord.name = record.name or '@'
+        azrecord.type = 'Microsoft.Network/dnszones/{}'.format(record._type)
+        record2 = provider._populate_record(zone, azrecord)
+        self.assertEqual(record2.dynamic._data(), record.dynamic._data())
+
+    def test_dynamic_a_weighted_rr(self):
+        provider = self._get_provider()
+        external = 'Microsoft.Network/trafficManagerProfiles/externalEndpoints'
+
+        record = Record.new(zone, 'foo', data={
+            'type': 'A',
+            'ttl': 60,
+            'values': ['1.1.1.1', '8.8.8.8'],
+            'dynamic': {
+                'pools': {
+                    'one': {
+                        'values': [
+                            {'value': '1.1.1.1', 'weight': 11},
+                            {'value': '8.8.8.8', 'weight': 8},
+                        ],
+                    },
+                },
+                'rules': [
+                    {'pool': 'one'},
+                ],
+            }
+        })
+        profiles = provider._generate_traffic_managers(record)
+
+        self.assertEqual(len(profiles), 1)
+        self.assertTrue(_profile_is_match(profiles[0], Profile(
+            name='foo--unit--tests-A',
+            traffic_routing_method='Weighted',
+            dns_config=DnsConfig(
+                relative_name='foo--unit--tests-a', ttl=record.ttl),
+            monitor_config=_get_monitor(record),
+            endpoints=[
+                Endpoint(
+                    name='one--1.1.1.1--default--',
+                    type=external,
+                    target='1.1.1.1',
+                    weight=11,
+                ),
+                Endpoint(
+                    name='one--8.8.8.8--default--',
+                    type=external,
+                    target='8.8.8.8',
+                    weight=8,
+                ),
+            ],
+        )))
+
+        # test that same record gets populated back from traffic managers
+        tm_list = provider._tm_client.profiles.list_by_resource_group
+        tm_list.return_value = profiles
+        azrecord = RecordSet(
+            ttl=60,
+            target_resource=SubResource(id=profiles[-1].id),
+        )
+        azrecord.name = record.name or '@'
+        azrecord.type = 'Microsoft.Network/dnszones/{}'.format(record._type)
+        record2 = provider._populate_record(zone, azrecord)
+        self.assertEqual(record2.dynamic._data(), record.dynamic._data())
+
+    def test_dynamic_aaaa(self):
+        provider = self._get_provider()
+        external = 'Microsoft.Network/trafficManagerProfiles/externalEndpoints'
+
+        record = Record.new(zone, 'foo', data={
+            'type': 'AAAA',
+            'ttl': 60,
+            'values': ['1::1'],
+            'dynamic': {
+                'pools': {
+                    'one': {
+                        'values': [
+                            {'value': '1::1'},
+                        ],
+                    },
+                },
+                'rules': [
+                    {'pool': 'one'},
+                ],
+            }
+        })
+        profiles = provider._generate_traffic_managers(record)
+
+        self.assertEqual(len(profiles), 1)
+        self.assertTrue(_profile_is_match(profiles[0], Profile(
+            name='foo--unit--tests-AAAA',
+            traffic_routing_method='Geographic',
+            dns_config=DnsConfig(
+                relative_name='foo--unit--tests-aaaa', ttl=record.ttl),
+            monitor_config=_get_monitor(record),
+            endpoints=[
+                Endpoint(
+                    name='one--default--',
+                    type=external,
+                    target='1::1',
+                    geo_mapping=['WORLD'],
+                ),
+            ],
+        )))
+
+        # 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(Create(record))
+        # A dynamic record can only have 1 profile
+        tm_sync.assert_called_once()
+        create.assert_called_once()
+
+        # test broken alias
+        azrecord = RecordSet(
+            ttl=60, target_resource=SubResource(id=None))
+        azrecord.name = record.name or '@'
+        azrecord.type = 'Microsoft.Network/dnszones/{}'.format(record._type)
+        record2 = provider._populate_record(zone, azrecord)
+        self.assertEqual(record2.values, ['::1'])
+
+        # test that same record gets populated back from traffic managers
+        tm_list = provider._tm_client.profiles.list_by_resource_group
+        tm_list.return_value = profiles
+        azrecord = RecordSet(
+            ttl=60,
+            target_resource=SubResource(id=profiles[-1].id),
+        )
+        azrecord.name = record.name or '@'
+        azrecord.type = 'Microsoft.Network/dnszones/{}'.format(record._type)
+        record2 = provider._populate_record(zone, azrecord)
+        self.assertEqual(record2.dynamic._data(), record.dynamic._data())
+
     def test_sync_traffic_managers(self):
         provider, zone, record = self._get_dynamic_package()
         provider._populate_traffic_managers()