From 33a10eada434070b79712d398a0e7a4dda8a72b4 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Tue, 8 Feb 2022 10:26:49 -0800 Subject: [PATCH] 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 | 8 ++++++++ octodns/provider/yaml.py | 1 + octodns/source/base.py | 1 + octodns/zone.py | 10 ++-------- tests/test_octodns_manager.py | 14 +++++++------- tests/test_octodns_provider_base.py | 27 +++++++++++++++++++++------ tests/test_octodns_provider_yaml.py | 12 ++++++------ tests/test_octodns_source_tinydns.py | 7 ++++++- tests/zones/tinydns/example.com | 4 ++-- 9 files changed, 54 insertions(+), 30 deletions(-) diff --git a/octodns/provider/base.py b/octodns/provider/base.py index 250ec26..951daf3 100644 --- a/octodns/provider/base.py +++ b/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 diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 57fae3b..4c146bb 100644 --- a/octodns/provider/yaml.py +++ b/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')) diff --git a/octodns/source/base.py b/octodns/source/base.py index 569a1b4..dfc1613 100644 --- a/octodns/source/base.py +++ b/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 diff --git a/octodns/zone.py b/octodns/zone.py index a6cdab3..c1929cf 100644 --- a/octodns/zone.py +++ b/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 \ diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index d63e84d..4900a04 100644 --- a/tests/test_octodns_manager.py +++ b/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.') diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index bd6c361..6218076 100644 --- a/tests/test_octodns_provider_base.py +++ b/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 diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index bffe4c5..3dbeacb 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/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') diff --git a/tests/test_octodns_source_tinydns.py b/tests/test_octodns_source_tinydns.py index c17721e..bfc2cf1 100644 --- a/tests/test_octodns_source_tinydns.py +++ b/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', diff --git a/tests/zones/tinydns/example.com b/tests/zones/tinydns/example.com index 32781ca..ae8b5bd 100755 --- a/tests/zones/tinydns/example.com +++ b/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::ns2.ns.com:30 +.sub.example.com::ns3.ns.com:30 +.sub.example.com::ns4.ns.com:30 # A, under sub +www.sub.example.com::1.2.3.4