Merge branch 'master' into ns1-filter-chains

octodns/provider/ns1.py

@@ -779,13 +779,25 @@ class Ns1Provider(BaseProvider):
 
     def _data_for_CNAME(self, _type, record):
         if record.get('tier', 1) > 1:
-            # Advanced dynamic record
-            return self._data_for_dynamic(_type, record)
-
-        try:
-            value = record['short_answers'][0]
-        except IndexError:
+            # Advanced record, see if it's first answer has a note
+            try:
+                first_answer_note = record['answers'][0]['meta']['note']
+            except (IndexError, KeyError):
+                first_answer_note = ''
+            # If that note includes a `pool` it's a valid dynamic record
+            if 'pool:' in first_answer_note:
+                return self._data_for_dynamic(_type, record)
+            # If not, it can't be parsed. Let it be an empty record
+            self.log.warn('Cannot parse %s dynamic record due to missing '
+                          'pool name in first answer note, treating it as '
+                          'an empty record', record['domain'])
             value = None
+        else:
+            try:
+                value = record['short_answers'][0]
+            except IndexError:
+                value = None
+
         return {
             'ttl': record['ttl'],
             'type': _type,

tests/test_octodns_provider_ns1.py

@@ -1947,7 +1947,7 @@ class TestNs1ProviderDynamic(TestCase):
                 'meta': {
                     'priority': 1,
                     'weight': 12,
-                    'note': f'from:{catchall_pool_name}',
+                    'note': f'pool:iad from:{catchall_pool_name}',
                     'up': {},
                 },
                 'region': catchall_pool_name,
@@ -1995,6 +1995,45 @@ class TestNs1ProviderDynamic(TestCase):
             'value': 'value.unit.tests.',
         }, data)
 
+    def test_data_for_invalid_dynamic_CNAME(self):
+        provider = Ns1Provider('test', 'api-key')
+
+        # Potential setup created outside of octoDNS, so it could be missing
+        # notes and region names can be arbitrary
+        filters = provider._get_updated_filter_chain(False, False)
+        ns1_record = {
+            'answers': [{
+                'answer': ['iad.unit.tests.'],
+                'meta': {
+                    'priority': 1,
+                    'weight': 12,
+                    'up': {},
+                },
+                'region': 'global',
+            }, {
+                'answer': ['value.unit.tests.'],
+                'meta': {
+                    'priority': 2,
+                    'up': {},
+                },
+                'region': 'global',
+            }],
+            'domain': 'foo.unit.tests',
+            'filters': filters,
+            'regions': {
+                'global': {},
+            },
+            'tier': 3,
+            'ttl': 44,
+            'type': 'CNAME',
+        }
+        data = provider._data_for_CNAME('CNAME', ns1_record)
+        self.assertEquals({
+            'ttl': 44,
+            'type': 'CNAME',
+            'value': None,
+        }, data)
+
     @patch('ns1.rest.records.Records.retrieve')
     @patch('ns1.rest.zones.Zones.retrieve')
     @patch('octodns.provider.ns1.Ns1Provider._monitors_for')