diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 2790536..4551c6c 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -299,6 +299,47 @@ def _get_monitor(record): return monitor +def _check_valid_dynamic(record): + typ = record._type + dynamic = record.dynamic + if typ in ['A', 'AAAA']: + # A/AAAA records cannot be aliased to Traffic Managers that contain + # other nested Traffic Managers. Due to this limitation, A/AAAA + # dynamic records can do only one of geo-fencing, fallback and + # weighted RR. So let's validate that the record adheres to this + # limitation. + 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\'.') + raise AzureException(msg.format(record.fqdn, record._type)) + + 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') + raise AzureException(msg.format(record.fqdn, record._type)) + elif typ != 'CNAME': + # dynamic records of unsupported type + msg = '{}: Dynamic records in Azure must be of type A/AAAA/CNAME' + raise AzureException(msg.format(record.fqdn)) + + def _profile_is_match(have, desired): if have is None or desired is None: return False @@ -851,49 +892,7 @@ class AzureProvider(BaseProvider): return data def _extra_changes(self, existing, desired, changes): - changed = set() - - # 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 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) + changed = set(c.record for c in changes) log = self.log.info seen_profiles = {} @@ -903,6 +902,9 @@ class AzureProvider(BaseProvider): # Already changed, or not dynamic, no need to check it continue + # Abort if there are unsupported dynamic record configurations + _check_valid_dynamic(record) + # let's walk through and show what will be changed even if # the record is already in list of changes added = (record in changed) @@ -910,8 +912,8 @@ class AzureProvider(BaseProvider): active = set() profiles = self._generate_traffic_managers(record) - # this should not happen with above checks, still adding to block - # undesired changes + # this should not happen with above check, check again here to + # prevent 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 ' diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py index 41454e9..16fa5b1 100644 --- a/tests/test_octodns_provider_azuredns.py +++ b/tests/test_octodns_provider_azuredns.py @@ -1002,8 +1002,6 @@ class TestAzureDnsProvider(TestCase): 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 = { @@ -1068,7 +1066,9 @@ class TestAzureDnsProvider(TestCase): debug('[all_values, rr, fallback, geo] = %s', args) data = record_data(*args) + desired = Zone(name=zone.name, sub_zones=[]) record = Record.new(desired, 'foo', data) + desired.add_record(record) features = args[1:] if all_values and features.count(True) <= 1: @@ -1084,9 +1084,40 @@ class TestAzureDnsProvider(TestCase): 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)) + @patch( + 'octodns.provider.azuredns._check_valid_dynamic') + def test_extra_changes_dynamic_a_multiple_profiles(self, mock_cvd): + provider = self._get_provider() + + # bypass validity check to trigger mutliple-profiles check + mock_cvd.return_value = True + + desired = Zone(name=zone.name, sub_zones=[]) + record = Record.new(desired, 'foo', { + 'type': 'A', + 'ttl': 60, + 'values': ['11.11.11.11', '12.12.12.12', '2.2.2.2'], + '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'}, + ], + } + }) desired.add_record(record) with self.assertRaises(AzureException) as ctx: # bypass above check by setting changes to empty