diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 38f9da3..a910459 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -406,7 +406,7 @@ class Ns1Provider(BaseProvider): for piece in note.split(' '): try: k, v = piece.split(':', 1) - data[k] = v + data[k] = v if v != '' else None except ValueError: pass return data @@ -479,31 +479,45 @@ class Ns1Provider(BaseProvider): # region. pools = defaultdict(lambda: {'fallback': None, 'values': []}) for answer in record['answers']: - # region (group name in the UI) is the pool name - pool_name = answer['region'] - # Get the actual pool name by removing the type - pool_name = self._parse_dynamic_pool_name(pool_name) - pool = pools[pool_name] - meta = answer['meta'] + notes = self._parse_notes(meta.get('note', '')) + value = text_type(answer['answer'][0]) - if meta['priority'] == 1: - # priority 1 means this answer is part of the pools own values - value_dict = { - 'value': value, - 'weight': int(meta.get('weight', 1)), - } - # If we have the original pool name and the catchall pool name - # in the answers, they point at the same pool. Add values only - # once - if value_dict not in pool['values']: - pool['values'].append(value_dict) + if notes.get('from', False) == '--default--': + # It's a final/default value, record it and move on + default.add(value) + continue + + # NS1 pool names can be found in notes > v0.9.11, in order to allow + # us to find fallback-only pools/values. Before that we used + # `region` (group name in the UI) and only paid attention to + # priority=1 (first level) + notes_pool_name = notes.get('pool', None) + if notes_pool_name is None: + # < v0.9.11 + if meta['priority'] != 1: + # Ignore all but priority 1 + continue + # And use region's pool name as the pool name + pool_name = self._parse_dynamic_pool_name(answer['region']) else: - # It's a fallback, we only care about it if it's a - # final/default - notes = self._parse_notes(meta.get('note', '')) - if notes.get('from', False) == '--default--': - default.add(value) + # > v0.9.11, use the notes-based name and consider all values + pool_name = notes_pool_name + + pool = pools[pool_name] + value_dict = { + 'value': value, + 'weight': int(meta.get('weight', 1)), + } + if value_dict not in pool['values']: + # If we haven't seen this value before add it to the pool + pool['values'].append(value_dict) + + # If there's a fallback recorded in the value for its pool go ahead + # and use it, another v0.9.11 thing + fallback = notes.get('fallback', None) + if fallback is not None: + pool['fallback'] = fallback # The regions objects map to rules, but it's a bit fuzzy since they're # tied to pools on the NS1 side, e.g. we can only have 1 rule per pool, @@ -528,7 +542,7 @@ class Ns1Provider(BaseProvider): rules[rule_order] = rule # The group notes field in the UI is a `note` on the region here, - # that's where we can find our pool's fallback. + # that's where we can find our pool's fallback in < v0.9.11 anyway if 'fallback' in notes: # set the fallback pool name pools[pool_name]['fallback'] = notes['fallback'] @@ -996,12 +1010,15 @@ class Ns1Provider(BaseProvider): seen.add(current_pool_name) pool = pools[current_pool_name] for answer in pool_answers[current_pool_name]: + fallback = pool.data['fallback'] answer = { 'answer': answer['answer'], 'meta': { 'priority': priority, 'note': self._encode_notes({ 'from': pool_label, + 'pool': current_pool_name, + 'fallback': fallback or '', }), 'up': { 'feed': answer['feed_id'], diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index 8c8760a..135baf8 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -628,7 +628,6 @@ class _DynamicMixin(object): if pool not in pools: reasons.append('rule {} undefined pool "{}"' .format(rule_num, pool)) - pools_seen.add(pool) elif pool in pools_seen and geos: reasons.append('rule {} invalid, target pool "{}" ' 'reused'.format(rule_num, pool)) diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index 4d6f02a..f54c7bd 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -1165,14 +1165,21 @@ class TestNs1ProviderDynamic(TestCase): # finally has a catchall. Those are examples of the two ways pools get # expanded. # - # lhr splits in two, with a region and country. + # lhr splits in two, with a region and country and includes a fallback + # + # All values now include their own `pool:` name # # well as both lhr georegion (for contients) and country. The first is # an example of a repeated target pool in a rule (only allowed when the # 2nd is a catchall.) - self.assertEquals(['from:--default--', 'from:iad__catchall', - 'from:iad__country', 'from:iad__georegion', - 'from:lhr__country', 'from:lhr__georegion'], + self.assertEquals(['fallback: from:iad__catchall pool:iad', + 'fallback: from:iad__country pool:iad', + 'fallback: from:iad__georegion pool:iad', + 'fallback: from:lhr__country pool:iad', + 'fallback: from:lhr__georegion pool:iad', + 'fallback:iad from:lhr__country pool:lhr', + 'fallback:iad from:lhr__georegion pool:lhr', + 'from:--default--'], sorted(notes.keys())) # All the iad's should match (after meta and region were removed) @@ -1551,6 +1558,115 @@ class TestNs1ProviderDynamic(TestCase): self.assertTrue( 'OC-{}'.format(c) in data4['dynamic']['rules'][0]['geos']) + # Test out fallback only pools and new-style notes + ns1_record = { + 'answers': [{ + 'answer': ['1.1.1.1'], + 'meta': { + 'priority': 1, + 'note': 'from:one__country pool:one fallback:two', + }, + 'region': 'one_country', + }, { + 'answer': ['2.2.2.2'], + 'meta': { + 'priority': 2, + 'note': 'from:one__country pool:two fallback:three', + }, + 'region': 'one_country', + }, { + 'answer': ['3.3.3.3'], + 'meta': { + 'priority': 3, + 'note': 'from:one__country pool:three fallback:', + }, + 'region': 'one_country', + }, { + 'answer': ['5.5.5.5'], + 'meta': { + 'priority': 4, + 'note': 'from:--default--', + }, + 'region': 'one_country', + }, { + 'answer': ['4.4.4.4'], + 'meta': { + 'priority': 1, + 'note': 'from:four__country pool:four fallback:', + }, + 'region': 'four_country', + }, { + 'answer': ['5.5.5.5'], + 'meta': { + 'priority': 2, + 'note': 'from:--default--', + }, + 'region': 'four_country', + }], + 'domain': 'unit.tests', + 'filters': filters, + 'regions': { + 'one__country': { + 'meta': { + 'note': 'rule-order:1 fallback:two', + 'country': ['CA'], + 'us_state': ['OR'], + }, + }, + 'four__country': { + 'meta': { + 'note': 'rule-order:2', + 'country': ['CA'], + 'us_state': ['OR'], + }, + }, + catchall_pool_name: { + 'meta': { + 'note': 'rule-order:3', + }, + } + }, + 'tier': 3, + 'ttl': 42, + } + data = provider._data_for_dynamic('A', ns1_record) + self.assertEquals({ + 'dynamic': { + 'pools': { + 'four': { + 'fallback': None, + 'values': [{'value': '4.4.4.4', 'weight': 1}] + }, + 'one': { + 'fallback': 'two', + 'values': [{'value': '1.1.1.1', 'weight': 1}] + }, + 'three': { + 'fallback': None, + 'values': [{'value': '3.3.3.3', 'weight': 1}] + }, + 'two': { + 'fallback': 'three', + 'values': [{'value': '2.2.2.2', 'weight': 1}] + }, + }, + 'rules': [{ + '_order': '1', + 'geos': ['NA-CA', 'NA-US-OR'], + 'pool': 'one' + }, { + '_order': '2', + 'geos': ['NA-CA', 'NA-US-OR'], + 'pool': 'four' + }, { + '_order': '3', 'pool': 'iad'} + ] + }, + 'ttl': 42, + 'type': 'A', + 'values': ['5.5.5.5'] + }, data) + def test_data_for_dynamic_CNAME(self): provider = Ns1Provider('test', 'api-key')