From ab897f65d9914f3443c57640d37c91d437b33f04 Mon Sep 17 00:00:00 2001
From: Viranch Mehta <email@viranch.me>
Date: Thu, 21 Oct 2021 13:01:32 -0700
Subject: [PATCH] Invalidate NS1 dynamic records with missing notes

---
 octodns/provider/ns1.py            | 14 +++++++---
 tests/test_octodns_provider_ns1.py | 41 +++++++++++++++++++++++++++++-
 2 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py
index ff2ba04..ba68bfd 100644
--- a/octodns/provider/ns1.py
+++ b/octodns/provider/ns1.py
@@ -771,13 +771,21 @@ 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)
+            # 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
 
         try:
             value = record['short_answers'][0]
-        except IndexError:
+        except (IndexError, KeyError):
             value = None
+
         return {
             'ttl': record['ttl'],
             'type': _type,
diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py
index 2364d92..e368f0e 100644
--- a/tests/test_octodns_provider_ns1.py
+++ b/tests/test_octodns_provider_ns1.py
@@ -1961,7 +1961,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,
@@ -2009,6 +2009,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')