Merge pull request #699 from octodns/ns1-fallback-only-support

Ns1 fallback only support

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'],

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))

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')