diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 0566710..fb0a78f 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -237,7 +237,7 @@ class Ns1Provider(BaseProvider): 'NS', 'PTR', 'SPF', 'SRV', 'TXT')) ZONE_NOT_FOUND_MESSAGE = 'server error: zone not found' - CATCHALL_PREFIX = 'catchall_' + CATCHALL_PREFIX = 'catchall__' def _update_filter(self, filter, with_disabled): if with_disabled: @@ -472,8 +472,7 @@ class Ns1Provider(BaseProvider): pool_name = answer['region'] # Get the actual pool name from the constructed pool name in case # of the catchall - if pool_name.startswith(self.CATCHALL_PREFIX): - pool_name = pool_name[len(self.CATCHALL_PREFIX):] + pool_name = pool_name.replace(self.CATCHALL_PREFIX, '') pool = pools[pool_name] meta = answer['meta'] @@ -505,8 +504,7 @@ class Ns1Provider(BaseProvider): # Rules that refer to the catchall pool would have the # CATCHALL_PREFIX in the pool name. Strip the prefix to get back # the pool name as in the config - if pool_name.startswith(self.CATCHALL_PREFIX): - pool_name = pool_name[len(self.CATCHALL_PREFIX):] + pool_name = pool_name.replace(self.CATCHALL_PREFIX, '') meta = region['meta'] notes = self._parse_notes(meta.get('note', '')) @@ -1000,12 +998,9 @@ class Ns1Provider(BaseProvider): has_country = False has_region = False regions = {} - catchall_pool_name = None - pool_usage = defaultdict(lambda: 0) for i, rule in enumerate(record.dynamic.rules): pool_name = rule.data['pool'] - pool_usage[pool_name] += 1 notes = { 'rule-order': i, @@ -1066,14 +1061,11 @@ class Ns1Provider(BaseProvider): # reuse. # (We expect only one catchall per record. Any associated # validation is expected to covered under record validation) - catchall_pool_name = pool_name - regions['{}{}'.format(self.CATCHALL_PREFIX, pool_name)] = { - 'meta': meta, - } - else: - regions[pool_name] = { - 'meta': meta, - } + pool_name = '{}{}'.format(self.CATCHALL_PREFIX, pool_name) + + regions[pool_name] = { + 'meta': meta, + } existing_monitors = self._monitors_for(record) active_monitors = set() @@ -1101,30 +1093,20 @@ class Ns1Provider(BaseProvider): } for v in record.values] # Build our list of answers + # The regions dictionary built above already has the required pool + # names. Iterate over them and add answers. + # In the case of the catchall, original pool name can be obtained + # by stripping the CATCHALL_PREFIX from the pool name answers = [] - for pool_name in sorted(pools.keys()): + for pool_name in sorted(regions.keys()): priority = 1 - answers_added = False # Dynamic/health checked - if pool_name == catchall_pool_name: - # For the catchall, use the internal pool name with the prefix - pool_label = '{}{}'.format(self.CATCHALL_PREFIX, pool_name) - self._add_answers_for_pool(answers, default_answers, pool_name, - pool_label, pool_answers, pools, - priority) - answers_added = True - - # If the catchall pool has been reused, we need the original - # pool name too, the creation of which is controlled by the - # answers_added flag - if pool_usage[pool_name] > 1: - answers_added = False - - if not answers_added: - self._add_answers_for_pool(answers, default_answers, pool_name, - pool_name, pool_answers, pools, - priority) + pool_label = pool_name + pool_name = pool_name.replace(self.CATCHALL_PREFIX, '') + self._add_answers_for_pool(answers, default_answers, pool_name, + pool_label, pool_answers, pools, + priority) # Update filters as necessary filters = self._get_updated_filter_chain(has_region, has_country) diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index 47841ac..8a0dffb 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -528,7 +528,7 @@ class TestNs1Provider(TestCase): class TestNs1ProviderDynamic(TestCase): zone = Zone('unit.tests.', []) - record_data = { + record = Record.new(zone, '', { 'dynamic': { 'pools': { 'lhr': { @@ -573,8 +573,7 @@ class TestNs1ProviderDynamic(TestCase): 'type': 'A', 'value': '1.2.3.4', 'meta': {}, - } - record = Record.new(zone, '', record_data) + }) def test_notes(self): provider = Ns1Provider('test', 'api-key') @@ -979,34 +978,6 @@ class TestNs1ProviderDynamic(TestCase): rule0['geos'] = rule0_saved_geos rule1['geos'] = rule1_saved_geos - # Test record with no reuse of the catchall - monitors_for_mock.reset_mock() - monitor_sync_mock.reset_mock() - monitors_for_mock.side_effect = [{ - '3.4.5.6': 'mid-3', - }] - monitor_sync_mock.side_effect = [ - ('mid-1', 'fid-1'), - ('mid-2', 'fid-2'), - ('mid-3', 'fid-3'), - ] - # Modify the record data before creating the Record object - rule0 = self.record_data['dynamic']['rules'][0] - rule0_saved_geos = rule0['geos'] - rule0['geos'] = ['AF', 'EU'] - rule1 = self.record_data['dynamic']['rules'].pop(1) - - # Create a local record object without reuse of catchall - zone = Zone('unit.tests.', []) - record = Record.new(zone, '', self.record_data) - ret, _ = provider._params_for_A(record) - self.assertEquals(ret['filters'], - Ns1Provider._FILTER_CHAIN_WITH_REGION(provider, - True)) - # Restore record_data - rule0['geos'] = rule0_saved_geos - self.record_data['dynamic']['rules'].insert(1, rule1) - @patch('octodns.provider.ns1.Ns1Provider._monitor_sync') @patch('octodns.provider.ns1.Ns1Provider._monitors_for') def test_params_for_dynamic_oceania(self, monitors_for_mock, @@ -1123,6 +1094,7 @@ class TestNs1ProviderDynamic(TestCase): # Test out a small, but realistic setup that covers all the options # We have country and region in the test config filters = provider._get_updated_filter_chain(True, True) + catchall_pool_name = '{}{}'.format(provider.CATCHALL_PREFIX, 'iad') ns1_record = { 'answers': [{ 'answer': ['3.4.5.6'], @@ -1166,16 +1138,16 @@ class TestNs1ProviderDynamic(TestCase): 'meta': { 'priority': 1, 'weight': 12, - 'note': 'from:catchall_iad', + 'note': 'from:{}'.format(catchall_pool_name), }, - 'region': 'catchall_iad', + 'region': catchall_pool_name, }, { 'answer': ['1.2.3.4'], 'meta': { 'priority': 2, 'note': 'from:--default--', }, - 'region': 'catchall_iad', + 'region': catchall_pool_name, }], 'domain': 'unit.tests', 'filters': filters, @@ -1194,7 +1166,7 @@ class TestNs1ProviderDynamic(TestCase): 'country': ['ZW'], }, }, - 'catchall_iad': { + catchall_pool_name: { 'meta': { 'note': 'rule-order:3', },