Base support for managing root NS records

* Zone object no longer treats them special, some tests needed adjusting
  b/c of this, some provider's tests may also need adjusting, though
  they should not plan changes since they won't (yet) have
  SUPPORTS_ROOT_NS
* _process_desired_zone filters and warns when not supported
* YamlProvider supports them
* TinyDnsBaseSource supports them

octodns/provider/base.py

@@ -94,6 +94,14 @@ class BaseProvider(BaseSource):
                 record = record.copy()
                 record.values = [record.value]
                 desired.add_record(record, replace=True)
+            elif record._type == 'NS' and record.name == '' and \
+                    not self.SUPPORTS_ROOT_NS:
+                # ignore, we can't manage root NS records
+                msg = \
+                    f'root NS record not supported for {record.fqdn}'
+                fallback = 'ignoring it'
+                self.supports_warn_or_except(msg, fallback)
+                desired.remove_record(record)
 
         return desired
 

octodns/provider/yaml.py

@@ -106,6 +106,7 @@ class YamlProvider(BaseProvider):
     SUPPORTS_DYNAMIC = True
     SUPPORTS_POOL_VALUE_STATUS = True
     SUPPORTS_MULTIVALUE_PTR = True
+    SUPPORTS_ROOT_NS = True
     SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'DNAME', 'LOC', 'MX',
                     'NAPTR', 'NS', 'PTR', 'SSHFP', 'SPF', 'SRV', 'TXT',
                     'URLFWD'))

octodns/source/base.py

@@ -10,6 +10,7 @@ class BaseSource(object):
 
     SUPPORTS_MULTIVALUE_PTR = False
     SUPPORTS_POOL_VALUE_STATUS = False
+    SUPPORTS_ROOT_NS = False
 
     def __init__(self, id):
         self.id = id

octodns/zone.py

@@ -24,12 +24,6 @@ class InvalidNodeException(Exception):
     pass
 
 
-def _is_eligible(record):
-    # Should this record be considered when computing changes
-    # We ignore all top-level NS records
-    return record._type != 'NS' or record.name != ''
-
-
 class Zone(object):
     log = getLogger('Zone')
 
@@ -118,7 +112,7 @@ class Zone(object):
         changes = []
 
         # Find diffs & removes
-        for record in filter(_is_eligible, self.records):
+        for record in self.records:
             if record.ignored:
                 continue
             elif len(record.included) > 0 and \
@@ -168,7 +162,7 @@ class Zone(object):
         # Find additions, things that are in desired, but missing in ourselves.
         # This uses set math and our special __hash__ and __cmp__ functions as
         # well
-        for record in filter(_is_eligible, desired.records - self.records):
+        for record in desired.records - self.records:
             if record.ignored:
                 continue
             elif len(record.included) > 0 and \

tests/test_octodns_manager.py

@@ -120,12 +120,12 @@ class TestManager(TestCase):
             environ['YAML_TMP_DIR'] = tmpdir.dirname
             tc = Manager(get_config_filename('simple.yaml')) \
                 .sync(dry_run=False)
-            self.assertEqual(26, tc)
+            self.assertEqual(27, tc)
 
             # try with just one of the zones
             tc = Manager(get_config_filename('simple.yaml')) \
                 .sync(dry_run=False, eligible_zones=['unit.tests.'])
-            self.assertEqual(20, tc)
+            self.assertEqual(21, tc)
 
             # the subzone, with 2 targets
             tc = Manager(get_config_filename('simple.yaml')) \
@@ -140,18 +140,18 @@ class TestManager(TestCase):
             # Again with force
             tc = Manager(get_config_filename('simple.yaml')) \
                 .sync(dry_run=False, force=True)
-            self.assertEqual(26, tc)
+            self.assertEqual(27, tc)
 
             # Again with max_workers = 1
             tc = Manager(get_config_filename('simple.yaml'), max_workers=1) \
                 .sync(dry_run=False, force=True)
-            self.assertEqual(26, tc)
+            self.assertEqual(27, tc)
 
             # Include meta
             tc = Manager(get_config_filename('simple.yaml'), max_workers=1,
                          include_meta=True) \
                 .sync(dry_run=False, force=True)
-            self.assertEqual(30, tc)
+            self.assertEqual(31, tc)
 
     def test_eligible_sources(self):
         with TemporaryDirectory() as tmpdir:
@@ -217,13 +217,13 @@ class TestManager(TestCase):
                 fh.write('---\n{}')
 
             changes = manager.compare(['in'], ['dump'], 'unit.tests.')
-            self.assertEqual(20, len(changes))
+            self.assertEqual(21, len(changes))
 
             # Compound sources with varying support
             changes = manager.compare(['in', 'nosshfp'],
                                       ['dump'],
                                       'unit.tests.')
-            self.assertEqual(19, len(changes))
+            self.assertEqual(20, len(changes))
 
             with self.assertRaises(ManagerException) as ctx:
                 manager.compare(['nope'], ['dump'], 'unit.tests.')

tests/test_octodns_provider_base.py

@@ -20,7 +20,7 @@ from octodns.zone import Zone
 class HelperProvider(BaseProvider):
     log = getLogger('HelperProvider')
 
-    SUPPORTS = set(('A', 'PTR'))
+    SUPPORTS = set(('A', 'NS', 'PTR'))
     SUPPORTS_MULTIVALUE_PTR = False
     SUPPORTS_DYNAMIC = False
 
@@ -36,7 +36,7 @@ class HelperProvider(BaseProvider):
         self.delete_pcent_threshold = Plan.MAX_SAFE_DELETE_PCENT
 
     def populate(self, zone, target=False, lenient=False):
-        pass
+        return True
 
     def _include_change(self, change):
         return not self.include_change_callback or \
@@ -229,6 +229,25 @@ class TestBaseProvider(TestCase):
         self.assertEqual(zone.name, another.existing.name)
         self.assertEqual(1, len(another.existing.records))
 
+    def test_plan_with_root_ns(self):
+        zone = Zone('unit.tests.', [])
+        record = Record.new(zone, '', {
+            'ttl': 30,
+            'type': 'NS',
+            'value': '1.2.3.4.',
+        })
+        zone.add_record(record)
+
+        # No root NS support, no change, thus no plan
+        provider = HelperProvider()
+        self.assertEqual(None, provider.plan(zone))
+
+        # set Support root NS records, see the record
+        provider.SUPPORTS_ROOT_NS = True
+        plan = provider.plan(zone)
+        self.assertTrue(plan)
+        self.assertEqual(1, len(plan.changes))
+
     def test_apply(self):
         ignored = Zone('unit.tests.', [])
 
@@ -278,10 +297,6 @@ class TestBaseProvider(TestCase):
         provider.SUPPORTS_MULTIVALUE_PTR = True
         zone2 = provider._process_desired_zone(zone1.copy())
         record2 = list(zone2.records)[0]
-        from pprint import pprint
-        pprint([
-            record1, record2
-        ])
         self.assertEqual(len(record2.values), 2)
 
         # SUPPORTS_DYNAMIC

tests/test_octodns_provider_yaml.py

@@ -57,12 +57,12 @@ class TestYamlProvider(TestCase):
 
             # We add everything
             plan = target.plan(zone)
-            self.assertEqual(20, len([c for c in plan.changes
+            self.assertEqual(21, len([c for c in plan.changes
                                       if isinstance(c, Create)]))
             self.assertFalse(isfile(yaml_file))
 
             # Now actually do it
-            self.assertEqual(20, target.apply(plan))
+            self.assertEqual(21, target.apply(plan))
             self.assertTrue(isfile(yaml_file))
 
             # Dynamic plan
@@ -86,7 +86,7 @@ class TestYamlProvider(TestCase):
 
             # A 2nd sync should still create everything
             plan = target.plan(zone)
-            self.assertEqual(20, len([c for c in plan.changes
+            self.assertEqual(21, len([c for c in plan.changes
                                       if isinstance(c, Create)]))
 
             with open(yaml_file) as fh:
@@ -263,12 +263,12 @@ class TestSplitYamlProvider(TestCase):
 
             # We add everything
             plan = target.plan(zone)
-            self.assertEqual(17, len([c for c in plan.changes
+            self.assertEqual(18, len([c for c in plan.changes
                                       if isinstance(c, Create)]))
             self.assertFalse(isdir(zone_dir))
 
             # Now actually do it
-            self.assertEqual(17, target.apply(plan))
+            self.assertEqual(18, target.apply(plan))
 
             # Dynamic plan
             plan = target.plan(dynamic_zone)
@@ -291,7 +291,7 @@ class TestSplitYamlProvider(TestCase):
 
             # A 2nd sync should still create everything
             plan = target.plan(zone)
-            self.assertEqual(17, len([c for c in plan.changes
+            self.assertEqual(18, len([c for c in plan.changes
                                       if isinstance(c, Create)]))
 
             yaml_file = join(zone_dir, '$unit.tests.yaml')

tests/test_octodns_source_tinydns.py

@@ -29,10 +29,15 @@ class TestTinyDnsFileSource(TestCase):
                 'ttl': 30,
                 'values': ['10.2.3.4', '10.2.3.5'],
             }),
+            ('', {
+                'type': 'NS',
+                'ttl': 3600,
+                'values': ['ns1.ns.com.', 'ns2.ns.com.'],
+            }),
             ('sub', {
                 'type': 'NS',
                 'ttl': 30,
-                'values': ['ns1.ns.com.', 'ns2.ns.com.'],
+                'values': ['ns3.ns.com.', 'ns4.ns.com.'],
             }),
             ('www', {
                 'type': 'A',

tests/zones/tinydns/example.com

@@ -32,8 +32,8 @@ Ccname.other.foo:www.other.foo
 @smtp.example.com::smtp-2-host.example.com:40:1800
 
 # NS
-.sub.example.com::ns1.ns.com:30
+.sub.example.com::ns3.ns.com:30
-.sub.example.com::ns2.ns.com:30
+.sub.example.com::ns4.ns.com:30
 
 # A, under sub
 +www.sub.example.com::1.2.3.4