diff --git a/CHANGELOG.md b/CHANGELOG.md index e6dcb48..dca36a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -## v0.9.13 - 2021-..-.. - +## v0.9.13 - 2021-07-18 - Processors Alpha #### Noteworthy changes @@ -14,6 +14,16 @@ America list for backwards compatibility reasons. They will be added in the next releaser. +#### Stuff + +* Lots of progress on the partial/beta support for dynamic records in Azure, + still not production ready. +* NS1 fix for when a pool only exists as a fallback +* Zone level lenient flag +* Validate weight makes sense for pools with a single record +* UltraDNS support for aliases and general fixes/improvements +* Misc doc fixes and improvements + ## v0.9.12 - 2021-04-30 - Enough time has passed #### Noteworthy changes diff --git a/README.md b/README.md index 8486799..684e496 100644 --- a/README.md +++ b/README.md @@ -53,7 +53,7 @@ $ mkdir config If you'd like to install a version that has not yet been released in a repetable/safe manner you can do the following. In general octoDNS is fairly stable inbetween releases thanks to the plan and apply process, but care should be taken regardless. ```shell -$ pip install -e git+https://git@github.com/github/octodns.git@#egg=octodns +$ pip install -e git+https://git@github.com/octodns/octodns.git@#egg=octodns ``` ### Config @@ -89,7 +89,7 @@ zones: - dyn - route53 - example.net: + example.net.: alias: example.com. ``` @@ -192,7 +192,7 @@ The above command pulled the existing data out of Route53 and placed the results | Provider | Requirements | Record Support | Dynamic | Notes | |--|--|--|--|--| -| [AzureProvider](/octodns/provider/azuredns.py) | azure-identity, azure-mgmt-dns, azure-mgmt-trafficmanager | A, AAAA, CAA, CNAME, MX, NS, PTR, SRV, TXT | Alpha (CNAMEs and partial A/AAAA) | | +| [AzureProvider](/octodns/provider/azuredns.py) | azure-identity, azure-mgmt-dns, azure-mgmt-trafficmanager | A, AAAA, CAA, CNAME, MX, NS, PTR, SRV, TXT | Alpha (A, AAAA, CNAME) | | | [Akamai](/octodns/provider/edgedns.py) | edgegrid-python | A, AAAA, CNAME, MX, NAPTR, NS, PTR, SPF, SRV, SSHFP, TXT | No | | | [CloudflareProvider](/octodns/provider/cloudflare.py) | | A, AAAA, ALIAS, CAA, CNAME, LOC, MX, NS, PTR, SPF, SRV, TXT | No | CAA tags restricted | | [ConstellixProvider](/octodns/provider/constellix.py) | | A, AAAA, ALIAS (ANAME), CAA, CNAME, MX, NS, PTR, SPF, SRV, TXT | No | CAA tags restricted | diff --git a/octodns/__init__.py b/octodns/__init__.py index 1885d42..16ec066 100644 --- a/octodns/__init__.py +++ b/octodns/__init__.py @@ -3,4 +3,4 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals -__VERSION__ = '0.9.12' +__VERSION__ = '0.9.13' diff --git a/octodns/provider/azuredns.py b/octodns/provider/azuredns.py index 87bbef0..6edc1ba 100644 --- a/octodns/provider/azuredns.py +++ b/octodns/provider/azuredns.py @@ -301,39 +301,22 @@ def _get_monitor(record): 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)) + defaults = set(record.values) + if len(defaults) > 1: + pools = record.dynamic.pools + vals = set( + v['value'] + for _, pool in pools.items() + for v in pool._data()['values'] + ) + if defaults != vals: + # we don't yet support multi-value defaults, specifying all + # pool values allows for Traffic Manager profile optimization + msg = ('{} {}: Values of A/AAAA dynamic records must either ' + 'have a single value or contain all values from all ' + 'pools') + 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' @@ -748,7 +731,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 +785,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] @@ -912,14 +894,6 @@ class AzureProvider(BaseProvider): active = set() profiles = self._generate_traffic_managers(record) - # 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 ' - 'records').format(record.fqdn, record._type) - raise AzureException(msg) - for profile in profiles: name = profile.name @@ -1075,6 +1049,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 +1110,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 +1121,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, )) @@ -1168,8 +1145,7 @@ class AzureProvider(BaseProvider): return traffic_managers - def _sync_traffic_managers(self, record): - desired_profiles = self._generate_traffic_managers(record) + def _sync_traffic_managers(self, desired_profiles): seen = set() tm_sync = self._tm_client.profiles.create_or_update @@ -1228,12 +1204,22 @@ class AzureProvider(BaseProvider): record = change.new dynamic = getattr(record, 'dynamic', False) + root_profile = None + endpoints = [] if dynamic: - self._sync_traffic_managers(record) + profiles = self._generate_traffic_managers(record) + root_profile = profiles[-1] + if record._type in ['A', 'AAAA'] and len(profiles) > 1: + # A/AAAA records cannot be aliased to Traffic Managers that + # contain other nested Traffic Managers. To work around this + # limitation, we remove nesting before adding the record, and + # then add the nested endpoints later. + endpoints = root_profile.endpoints + root_profile.endpoints = [] + self._sync_traffic_managers(profiles) - profile = self._get_tm_for_dynamic_record(record) ar = _AzureRecord(self._resource_group, record, - traffic_manager=profile) + traffic_manager=root_profile) create = self._dns_client.record_sets.create_or_update create(resource_group_name=ar.resource_group, @@ -1242,6 +1228,12 @@ class AzureProvider(BaseProvider): record_type=ar.record_type, parameters=ar.params) + if endpoints: + # add nested endpoints for A/AAAA dynamic record limitation after + # record creation + root_profile.endpoints = endpoints + self._sync_traffic_managers([root_profile]) + self.log.debug('* Success Create: {}'.format(record)) def _apply_Update(self, change): @@ -1260,13 +1252,28 @@ class AzureProvider(BaseProvider): update_record = True if new_is_dynamic: - active = self._sync_traffic_managers(new) - # only TTL is configured in record, everything else goes inside - # traffic managers, so no need to update if TTL is unchanged - # and existing record is already aliased to its traffic manager - if existing.ttl == new.ttl and existing_is_dynamic: + endpoints = [] + profiles = self._generate_traffic_managers(new) + root_profile = profiles[-1] + + if new._type in ['A', 'AAAA']: + if existing_is_dynamic: + # update to the record is not needed + update_record = False + elif len(profiles) > 1: + # record needs to aliased; remove nested endpoints, we + # will add them at the end + endpoints = root_profile.endpoints + root_profile.endpoints = [] + elif existing.ttl == new.ttl and existing_is_dynamic: + # CNAME dynamic records only have TTL in them, everything else + # goes inside the aliased traffic managers; skip update if TTL + # is unchanged and existing record is already aliased to its + # traffic manager update_record = False + active = self._sync_traffic_managers(profiles) + if update_record: profile = self._get_tm_for_dynamic_record(new) ar = _AzureRecord(self._resource_group, new, @@ -1280,6 +1287,10 @@ class AzureProvider(BaseProvider): parameters=ar.params) if new_is_dynamic: + # add any pending nested endpoints + if endpoints: + root_profile.endpoints = endpoints + self._sync_traffic_managers([root_profile]) # let's cleanup unused traffic managers self._traffic_managers_gc(new, active) elif existing_is_dynamic: diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 9e720cd..4886c00 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -850,6 +850,8 @@ class Ns1Provider(BaseProvider): for monitor in self._client.monitors.values(): data = self._parse_notes(monitor['notes']) + if not data: + continue if expected_host == data['host'] and \ expected_type == data['type']: # This monitor does not belong to this record diff --git a/tests/test_octodns_provider_azuredns.py b/tests/test_octodns_provider_azuredns.py index e09de28..0af8665 100644 --- a/tests/test_octodns_provider_azuredns.py +++ b/tests/test_octodns_provider_azuredns.py @@ -4,7 +4,6 @@ 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, \ @@ -543,7 +542,14 @@ class TestAzureDnsProvider(TestCase): tm_sync = provider._tm_client.profiles.create_or_update def side_effect(rg, name, profile): - return profile + return Profile( + id=profile.id, + name=profile.name, + traffic_routing_method=profile.traffic_routing_method, + dns_config=profile.dns_config, + monitor_config=profile.monitor_config, + endpoints=profile.endpoints, + ) tm_sync.side_effect = side_effect @@ -697,13 +703,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'), ), @@ -1044,128 +1050,31 @@ class TestAzureDnsProvider(TestCase): provider._extra_changes(zone, desired, changes) self.assertTrue('duplicate endpoint' in text_type(ctx)) - def test_extra_changes_invalid_dynamic_A(self): + def test_extra_changes_A_multi_defaults(self): provider = self._get_provider() - # 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) - 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: - # 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) - - @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', { + record = Record.new(zone, 'foo', data={ 'type': 'A', 'ttl': 60, - 'values': ['11.11.11.11', '12.12.12.12', '2.2.2.2'], + 'values': ['1.1.1.1', '8.8.8.8'], 'dynamic': { 'pools': { 'one': { - 'values': [ - {'value': '11.11.11.11'}, - {'value': '12.12.12.12'}, - ], - 'fallback': 'two', - }, - 'two': { - 'values': [ - {'value': '2.2.2.2'}, - ], + 'values': [{'value': '1.1.1.1'}], }, }, 'rules': [ - {'geos': ['EU'], 'pool': 'two'}, {'pool': 'one'}, ], } }) + + # test that extra changes doesn't show any changes + desired = Zone(zone.name, sub_zones=[]) desired.add_record(record) with self.assertRaises(AzureException) as ctx: - provider._extra_changes(zone, desired, [Create(record)]) - self.assertTrue('more than 1 Traffic Managers' in text_type(ctx)) + provider._extra_changes(zone, desired, []) + self.assertEqual('single value' in text_type(ctx)) def test_generate_tm_profile(self): provider, zone, record = self._get_dynamic_package() @@ -1231,6 +1140,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 +1251,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 +1310,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 +1386,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 +1406,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 +1486,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 +1594,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 +1669,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,48 +1701,116 @@ class TestAzureDnsProvider(TestCase): record2 = provider._populate_record(zone, azrecord) self.assertEqual(record2.dynamic._data(), record.dynamic._data()) - def test_dynamic_A_geo(self): + # 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(self): provider = self._get_provider() external = 'Microsoft.Network/trafficManagerProfiles/externalEndpoints' + nested = 'Microsoft.Network/trafficManagerProfiles/nestedEndpoints' record = Record.new(zone, 'foo', data={ 'type': 'A', 'ttl': 60, - 'values': ['1.1.1.1', '2.2.2.2', '3.3.3.3'], + 'values': ['9.9.9.9'], 'dynamic': { 'pools': { 'one': { 'values': [ - {'value': '1.1.1.1'}, + {'value': '11.11.11.11'}, + {'value': '12.12.12.12'}, ], + 'fallback': 'two' }, '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'}, + {'geos': ['AF'], 'pool': 'one'}, + {'pool': 'two'}, ], } }) - # 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.assertEqual(len(profiles), 4) self.assertTrue(_profile_is_match(profiles[0], Profile( + name='foo--unit--tests-A-pool-one', + traffic_routing_method='Weighted', + dns_config=DnsConfig( + relative_name='foo--unit--tests-a-pool-one', ttl=record.ttl), + monitor_config=_get_monitor(record), + endpoints=[ + Endpoint( + name='one--11.11.11.11', + type=external, + target='11.11.11.11', + weight=1, + ), + Endpoint( + name='one--12.12.12.12', + type=external, + target='12.12.12.12', + weight=1, + ), + ], + ))) + self.assertTrue(_profile_is_match(profiles[1], Profile( + name='foo--unit--tests-A-rule-one', + traffic_routing_method='Priority', + dns_config=DnsConfig( + relative_name='foo--unit--tests-a-rule-one', ttl=record.ttl), + monitor_config=_get_monitor(record), + endpoints=[ + Endpoint( + name='one', + type=nested, + target_resource_id=profiles[0].id, + priority=1, + ), + Endpoint( + name='two', + type=external, + target='2.2.2.2', + priority=2, + ), + Endpoint( + name='--default--', + type=external, + target='9.9.9.9', + priority=3, + ), + ], + ))) + self.assertTrue(_profile_is_match(profiles[2], Profile( + name='foo--unit--tests-A-rule-two', + traffic_routing_method='Priority', + dns_config=DnsConfig( + relative_name='foo--unit--tests-a-rule-two', ttl=record.ttl), + monitor_config=_get_monitor(record), + endpoints=[ + Endpoint( + name='two', + type=external, + target='2.2.2.2', + priority=1, + ), + Endpoint( + name='--default--', + type=external, + target='9.9.9.9', + priority=2, + ), + ], + ))) + self.assertTrue(_profile_is_match(profiles[3], Profile( name='foo--unit--tests-A', traffic_routing_method='Geographic', dns_config=DnsConfig( @@ -1805,21 +1818,15 @@ class TestAzureDnsProvider(TestCase): 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', + name='one', + type=nested, + target_resource_id=profiles[1].id, geo_mapping=['GEO-AF'], ), Endpoint( - name='three--default--', - type=external, - target='3.3.3.3', + name='two', + type=nested, + target_resource_id=profiles[2].id, geo_mapping=['WORLD'], ), ], @@ -1828,9 +1835,8 @@ 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]) - # A dynamic record can only have 1 profile - tm_sync.assert_called_once() + provider._apply_Create(Create(record)) + self.assertEqual(tm_sync.call_count, len(profiles) + 1) create.assert_called_once() # test broken alias @@ -1844,6 +1850,7 @@ class TestAzureDnsProvider(TestCase): # 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 + provider._populate_traffic_managers() azrecord = RecordSet( ttl=60, target_resource=SubResource(id=profiles[-1].id), @@ -1853,134 +1860,11 @@ class TestAzureDnsProvider(TestCase): 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()) + # 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() @@ -1989,12 +1873,13 @@ class TestAzureDnsProvider(TestCase): record = Record.new(zone, 'foo', data={ 'type': 'AAAA', 'ttl': 60, - 'values': ['1::1'], + 'values': ['1::1', '2::2'], 'dynamic': { 'pools': { 'one': { 'values': [ {'value': '1::1'}, + {'value': '2::2'}, ], }, }, @@ -2008,16 +1893,22 @@ class TestAzureDnsProvider(TestCase): self.assertEqual(len(profiles), 1) self.assertTrue(_profile_is_match(profiles[0], Profile( name='foo--unit--tests-AAAA', - traffic_routing_method='Geographic', + traffic_routing_method='Weighted', dns_config=DnsConfig( relative_name='foo--unit--tests-aaaa', ttl=record.ttl), monitor_config=_get_monitor(record), endpoints=[ Endpoint( - name='one--default--', + name='one--1--1--default--', type=external, target='1::1', - geo_mapping=['WORLD'], + weight=1, + ), + Endpoint( + name='one--2--2--default--', + type=external, + target='2::2', + weight=1, ), ], ))) @@ -2050,6 +1941,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() @@ -2063,7 +1960,8 @@ class TestAzureDnsProvider(TestCase): } # test no change - seen = provider._sync_traffic_managers(record) + profiles = provider._generate_traffic_managers(record) + seen = provider._sync_traffic_managers(profiles) self.assertEqual(seen, expected_seen) tm_sync.assert_not_called() @@ -2079,7 +1977,8 @@ class TestAzureDnsProvider(TestCase): } new_record = Record.new(zone, record.name, data) tm_sync.reset_mock() - seen2 = provider._sync_traffic_managers(new_record) + profiles = provider._generate_traffic_managers(new_record) + seen2 = provider._sync_traffic_managers(profiles) self.assertEqual(seen2, expected_seen) tm_sync.assert_called_once() @@ -2089,17 +1988,14 @@ class TestAzureDnsProvider(TestCase): ) self.assertEqual(new_profile.endpoints[0].weight, 14) - @patch( - 'octodns.provider.azuredns.AzureProvider._generate_traffic_managers') - def test_sync_traffic_managers_duplicate(self, mock_gen_tms): + def test_sync_traffic_managers_duplicate(self): provider, zone, record = self._get_dynamic_package() tm_sync = provider._tm_client.profiles.create_or_update # change and duplicate profiles profile = self._get_tm_profiles(provider)[0] profile.name = 'changing_this_to_trigger_sync' - mock_gen_tms.return_value = [profile, profile] - provider._sync_traffic_managers(record) + provider._sync_traffic_managers([profile, profile]) # it should only be called once for duplicate profiles tm_sync.assert_called_once() @@ -2176,7 +2072,6 @@ class TestAzureDnsProvider(TestCase): tm_sync = provider._tm_client.profiles.create_or_update - zone = Zone(name='unit.tests.', sub_zones=[]) record = self._get_dynamic_record(zone) profiles = self._get_tm_profiles(provider) @@ -2289,6 +2184,116 @@ class TestAzureDnsProvider(TestCase): dns_update.assert_called_once() tm_delete.assert_not_called() + def test_apply_update_dynamic_A(self): + # existing is simple, new is dynamic + provider = self._get_provider() + simple_record = Record.new(zone, 'foo', data={ + 'type': 'A', + 'ttl': 3600, + 'values': ['1.1.1.1', '2.2.2.2'], + }) + dynamic_record = Record.new(zone, simple_record.name, data={ + 'type': 'A', + 'ttl': 60, + 'values': ['1.1.1.1'], + 'dynamic': { + 'pools': { + 'one': { + 'values': [ + {'value': '8.8.8.8'}, + {'value': '4.4.4.4'}, + ], + 'fallback': 'two', + }, + 'two': { + 'values': [{'value': '9.9.9.9'}], + }, + }, + 'rules': [ + {'geos': ['AF'], 'pool': 'two'}, + {'pool': 'one'}, + ], + } + }) + num_tms = len(provider._generate_traffic_managers(dynamic_record)) + change = Update(simple_record, dynamic_record) + provider._apply_Update(change) + tm_sync, dns_update, tm_delete = ( + provider._tm_client.profiles.create_or_update, + provider._dns_client.record_sets.create_or_update, + provider._tm_client.profiles.delete + ) + # sync is called once for each profile, plus 1 at the end for nested + # endpoints to workaround A/AAAA nesting limitation in Azure + self.assertEqual(tm_sync.call_count, num_tms + 1) + dns_update.assert_called_once() + tm_delete.assert_not_called() + + # both are dynamic, healthcheck port is changed to trigger sync on + # all profiles + provider = self._get_provider() + dynamic_record2 = Record.new(zone, dynamic_record.name, data={ + 'type': dynamic_record._type, + 'ttl': 300, + 'values': dynamic_record.values, + 'dynamic': dynamic_record.dynamic._data(), + 'octodns': { + 'healthcheck': {'port': 4433}, + } + }) + change = Update(dynamic_record, dynamic_record2) + provider._apply_Update(change) + tm_sync, dns_update, tm_delete = ( + provider._tm_client.profiles.create_or_update, + provider._dns_client.record_sets.create_or_update, + provider._tm_client.profiles.delete + ) + # sync is called once for each profile, extra call at the end is not + # needed when existing dynamic record is already aliased to its root + # profile + self.assertEqual(tm_sync.call_count, num_tms) + dns_update.assert_not_called() + tm_delete.assert_not_called() + + def test_apply_update_dynamic_A_singluar(self): + # existing is simple, new is dynamic that needs only one profile + provider = self._get_provider() + simple_record = Record.new(zone, 'foo', data={ + 'type': 'A', + 'ttl': 3600, + 'values': ['1.1.1.1', '2.2.2.2'], + }) + dynamic_record = Record.new(zone, simple_record.name, data={ + 'type': 'A', + 'ttl': 60, + 'values': ['1.1.1.1'], + 'dynamic': { + 'pools': { + 'one': { + 'values': [ + {'value': '8.8.8.8'}, + {'value': '1.1.1.1'}, + ], + }, + }, + 'rules': [ + {'pool': 'one'}, + ], + } + }) + num_tms = len(provider._generate_traffic_managers(dynamic_record)) + self.assertEqual(num_tms, 1) + change = Update(simple_record, dynamic_record) + provider._apply_Update(change) + tm_sync, dns_update, tm_delete = ( + provider._tm_client.profiles.create_or_update, + provider._dns_client.record_sets.create_or_update, + provider._tm_client.profiles.delete + ) + self.assertEqual(tm_sync.call_count, num_tms) + dns_update.assert_called_once() + tm_delete.assert_not_called() + def test_apply_delete_dynamic(self): provider, existing, record = self._get_dynamic_package() provider._populate_traffic_managers() diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index df517a2..e21fe0d 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -659,6 +659,18 @@ class TestNs1ProviderDynamic(TestCase): }, 'four': monitor_four, 'five': monitor_five, + 'six': { + 'config': { + 'host': '10.10.10.10', + }, + 'notes': 'non-conforming notes', + }, + 'seven': { + 'config': { + 'host': '11.11.11.11', + }, + 'notes': None, + }, } # Would match, but won't get there b/c it's not dynamic