diff --git a/CHANGELOG.md b/CHANGELOG.md index 514f375..add77cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ ## v0.9.16 - 2022-??-?? - ??? +#### Noteworthy changes + +* Foundational support for root NS record management. + * YamlProvider has it enabled and in general everyone should add root NS + records that match what is in their provider(s) as of this release if they + aren't already there. + * Note that if you created your config files with `octodns-dump`, the records + are likely already there and match what was configured at the time of the + dump. + * Other providers will add root NS support over time once they have had the + chance to investigate the functionality and implement management if + possible with whatever accomidations are required. + * Watch your providers README.md and CHANGELOG.md for support and more + information. + #### Stuff * _AggregateTarget has more complete handling of SUPPORTS* functionality, diff --git a/README.md b/README.md index 7687391..43e1db1 100644 --- a/README.md +++ b/README.md @@ -223,7 +223,7 @@ The table below lists the providers octoDNS supports. They are maintained in the | [Selectel](https://selectel.ru/en/services/additional/dns/) | [octodns_selectel](https://github.com/octodns/octodns-selectel/) | | | [TransIP](https://www.transip.eu/knowledgebase/entry/155-dns-and-nameservers/) | [octodns_transip](https://github.com/octodns/octodns-transip/) | | | [UltraDNS](https://www.home.neustar/dns-services) | [octodns_ultra](https://github.com/octodns/octodns-ultra/) | | -| [YamlProvider](/octodns/provider/yaml.py) | built-in | config | +| [YamlProvider](/octodns/provider/yaml.py) | built-in | Supports all record types and core functionality | ### Updating to use extracted providers @@ -242,7 +242,6 @@ Similar to providers, but can only serve to populate records into a zone, cannot | [AxfrSource](/octodns/source/axfr.py) | A, AAAA, CAA, CNAME, LOC, MX, NS, PTR, SPF, SRV, TXT | No | read-only | | [ZoneFileSource](/octodns/source/axfr.py) | A, AAAA, CAA, CNAME, MX, NS, PTR, SPF, SRV, TXT | No | read-only | | [TinyDnsFileSource](/octodns/source/tinydns.py) | A, CNAME, MX, NS, PTR | No | read-only | -| [YamlProvider](/octodns/provider/yaml.py) | All | Yes | config | #### Notes diff --git a/octodns/provider/base.py b/octodns/provider/base.py index 250ec26..b524017 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -95,8 +95,51 @@ class BaseProvider(BaseSource): record.values = [record.value] desired.add_record(record, replace=True) + record = desired.root_ns + if self.SUPPORTS_ROOT_NS: + if not record: + self.log.warning('root NS record supported, but no record ' + 'is configured for %s', desired.name) + else: + if record: + # we can't manage root NS records, get rid of it + 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 + def _process_existing_zone(self, existing, desired): + ''' + An opportunity for providers to modify the existing zone records before + planning. `existing` is a "shallow" copy, see `Zone.copy` for more + information + + - `desired` must not be modified in anyway, it is only for reference + - Must call `super` at an appropriate point for their work, generally + that means as the final step of the method, returning the result of + the `super` call. + - May modify `existing` directly. + - Must not modify records directly, `record.copy` should be called, + the results of which can be modified, and then `Zone.add_record` may + be used with `replace=True`. + - May call `Zone.remove_record` to remove records from `existing`. + - Must call supports_warn_or_except with information about any changes + that are made to have them logged or throw errors depending on the + provider configuration. + ''' + + existing_root_ns = existing.root_ns + if existing_root_ns and (not self.SUPPORTS_ROOT_NS or + not desired.root_ns): + self.log.info('root NS record in existing, but not supported or ' + 'not configured; ignoring it') + existing.remove_record(existing_root_ns) + + return existing + def _include_change(self, change): ''' An opportunity for providers to filter out false positives due to @@ -120,13 +163,6 @@ class BaseProvider(BaseSource): def plan(self, desired, processors=[]): self.log.info('plan: desired=%s', desired.name) - # Make a (shallow) copy of the desired state so that everything from - # now on (in this target) can modify it as they see fit without - # worrying about impacting other targets. - desired = desired.copy() - - desired = self._process_desired_zone(desired) - existing = Zone(desired.name, desired.sub_zones) exists = self.populate(existing, target=True, lenient=True) if exists is None: @@ -135,6 +171,15 @@ class BaseProvider(BaseSource): self.log.warning('Provider %s used in target mode did not return ' 'exists', self.id) + # Make a (shallow) copy of the desired state so that everything from + # now on (in this target) can modify it as they see fit without + # worrying about impacting other targets. + desired = desired.copy() + + desired = self._process_desired_zone(desired) + + existing = self._process_existing_zone(existing, desired) + for processor in processors: existing = processor.process_target_zone(existing, target=self) diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index 09ccc73..2d39c80 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -12,12 +12,22 @@ from io import StringIO class UnsafePlan(Exception): + pass + + +class RootNsChange(UnsafePlan): + + def __init__(self): + super().__init__('Root NS record change, force required') + + +class TooMuchChange(UnsafePlan): def __init__(self, why, update_pcent, update_threshold, change_count, existing_count): - msg = f'{why}, {update_pcent:.2f} is over {update_threshold:.2f} %' \ - f'({change_count}/{existing_count})' - super(UnsafePlan, self).__init__(msg) + msg = f'{why}, {update_pcent:.2f}% is over {update_threshold:.2f}% ' \ + f'({change_count}/{existing_count}), force required' + super().__init__(msg) class Plan(object): @@ -67,19 +77,32 @@ class Plan(object): len(self.existing.records) >= self.MIN_EXISTING_RECORDS: existing_record_count = len(self.existing.records) - update_pcent = self.change_counts['Update'] / existing_record_count - delete_pcent = self.change_counts['Delete'] / existing_record_count + if existing_record_count > 0: + update_pcent = (self.change_counts['Update'] / + existing_record_count) + delete_pcent = (self.change_counts['Delete'] / + existing_record_count) + else: + update_pcent = 0 + delete_pcent = 0 if update_pcent > self.update_pcent_threshold: - raise UnsafePlan('Too many updates', update_pcent * 100, - self.update_pcent_threshold * 100, - self.change_counts['Update'], - existing_record_count) + raise TooMuchChange('Too many updates', update_pcent * 100, + self.update_pcent_threshold * 100, + self.change_counts['Update'], + existing_record_count) if delete_pcent > self.delete_pcent_threshold: - raise UnsafePlan('Too many deletes', delete_pcent * 100, - self.delete_pcent_threshold * 100, - self.change_counts['Delete'], - existing_record_count) + raise TooMuchChange('Too many deletes', delete_pcent * 100, + self.delete_pcent_threshold * 100, + self.change_counts['Delete'], + existing_record_count) + + # If we have any changes of the root NS record for the zone it's a huge + # deal and force should always be required for extra care + if self.exists and any(c for c in self.changes + if c.record and c.record._type == 'NS' and + c.record.name == ''): + raise RootNsChange() def __repr__(self): creates = self.change_counts['Create'] diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 57fae3b..16a4bc1 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -111,7 +111,8 @@ class YamlProvider(BaseProvider): 'URLFWD')) def __init__(self, id, directory, default_ttl=3600, enforce_order=True, - populate_should_replace=False, *args, **kwargs): + populate_should_replace=False, supports_root_ns=True, + *args, **kwargs): klass = self.__class__.__name__ self.log = logging.getLogger(f'{klass}[{id}]') self.log.debug('__init__: id=%s, directory=%s, default_ttl=%d, ' @@ -123,6 +124,11 @@ class YamlProvider(BaseProvider): self.default_ttl = default_ttl self.enforce_order = enforce_order self.populate_should_replace = populate_should_replace + self.supports_root_ns = supports_root_ns + + @property + def SUPPORTS_ROOT_NS(self): + return self.supports_root_ns def _populate_from_file(self, filename, zone, lenient): with open(filename, 'r') as fh: 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..4cd5e91 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') @@ -42,6 +36,7 @@ class Zone(object): # We're grouping by node, it allows us to efficiently search for # duplicates and detect when CNAMEs co-exist with other records self._records = defaultdict(set) + self._root_ns = None # optional leading . to match empty hostname # optional trailing . b/c some sources don't have it on their fqdn self._name_re = re.compile(fr'\.?{name}?$') @@ -59,6 +54,12 @@ class Zone(object): return self._origin.records return set([r for _, node in self._records.items() for r in node]) + @property + def root_ns(self): + if self._origin: + return self._origin.root_ns + return self._root_ns + def hostname_from_fqdn(self, fqdn): return self._name_re.sub('', fqdn) @@ -97,11 +98,18 @@ class Zone(object): f'{record.fqdn} cannot coexist with ' 'other records') + if record._type == 'NS' and record.name == '': + self._root_ns = record + node.add(record) def remove_record(self, record): if self._origin: self.hydrate() + + if record._type == 'NS' and record.name == '': + self._root_ns = None + self._records[record.name].discard(record) # TODO: delete this @@ -118,7 +126,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 +176,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/config/always-dry-run.yaml b/tests/config/always-dry-run.yaml index 466c26b..333ecff 100644 --- a/tests/config/always-dry-run.yaml +++ b/tests/config/always-dry-run.yaml @@ -5,6 +5,7 @@ providers: dump: class: octodns.provider.yaml.YamlProvider directory: env/YAML_TMP_DIR + supports_root_ns: False zones: unit.tests.: always-dry-run: true diff --git a/tests/config/processors.yaml b/tests/config/processors.yaml index 097024b..4229707 100644 --- a/tests/config/processors.yaml +++ b/tests/config/processors.yaml @@ -5,6 +5,7 @@ providers: dump: class: octodns.provider.yaml.YamlProvider directory: env/YAML_TMP_DIR + supports_root_ns: False geo: class: helpers.GeoProvider nosshfp: diff --git a/tests/config/simple-alias-zone.yaml b/tests/config/simple-alias-zone.yaml index 32154d5..d3e7612 100644 --- a/tests/config/simple-alias-zone.yaml +++ b/tests/config/simple-alias-zone.yaml @@ -7,6 +7,7 @@ providers: dump: class: octodns.provider.yaml.YamlProvider directory: env/YAML_TMP_DIR + supports_root_ns: False zones: unit.tests.: sources: diff --git a/tests/config/simple.yaml b/tests/config/simple.yaml index cf970a9..fc4ad9f 100644 --- a/tests/config/simple.yaml +++ b/tests/config/simple.yaml @@ -4,14 +4,17 @@ providers: in: class: octodns.provider.yaml.YamlProvider directory: tests/config + supports_root_ns: False dump: class: octodns.provider.yaml.YamlProvider directory: env/YAML_TMP_DIR + supports_root_ns: False # This is sort of ugly, but it shouldn't hurt anything. It'll just write out # the target file twice where it and dump are both used dump2: class: octodns.provider.yaml.YamlProvider directory: env/YAML_TMP_DIR + supports_root_ns: False simple: class: helpers.SimpleProvider geo: diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index d63e84d..9e68030 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -216,14 +216,16 @@ class TestManager(TestCase): with open(join(tmpdir.dirname, 'unit.tests.yaml'), 'w') as fh: fh.write('---\n{}') + # compare doesn't use _process_desired_zone and thus doesn't filter + # out root NS records, that seems fine/desirable 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_plan.py b/tests/test_octodns_plan.py index 4dfad54..6c01bb8 100644 --- a/tests/test_octodns_plan.py +++ b/tests/test_octodns_plan.py @@ -9,7 +9,8 @@ from io import StringIO from logging import getLogger from unittest import TestCase -from octodns.provider.plan import Plan, PlanHtml, PlanLogger, PlanMarkdown +from octodns.provider.plan import Plan, PlanHtml, PlanLogger, PlanMarkdown, \ + RootNsChange, TooMuchChange from octodns.record import Create, Delete, Record, Update from octodns.zone import Zone @@ -110,3 +111,156 @@ class TestPlanMarkdown(TestCase): self.assertTrue('Update | a | A | 300 | 1.1.1.1;' in out) self.assertTrue('NA-US: 6.6.6.6 | test' in out) self.assertTrue('Delete | a | A | 300 | 2.2.2.2;' in out) + + +class HelperPlan(Plan): + + def __init__(self, *args, min_existing=0, **kwargs): + super().__init__(*args, **kwargs) + self.MIN_EXISTING_RECORDS = min_existing + + +class TestPlanSafety(TestCase): + existing = Zone('unit.tests.', []) + record_1 = Record.new(existing, '1', data={ + 'type': 'A', + 'ttl': 42, + 'value': '1.2.3.4', + }) + record_2 = Record.new(existing, '2', data={ + 'type': 'A', + 'ttl': 42, + 'value': '1.2.3.4', + }) + record_3 = Record.new(existing, '3', data={ + 'type': 'A', + 'ttl': 42, + 'value': '1.2.3.4', + }) + record_4 = Record.new(existing, '4', data={ + 'type': 'A', + 'ttl': 42, + 'value': '1.2.3.4', + }) + + def test_too_many_updates(self): + existing = self.existing.copy() + changes = [] + + # No records, no changes, we're good + plan = HelperPlan(existing, None, changes, True) + plan.raise_if_unsafe() + + # Four records, no changes, we're good + existing.add_record(self.record_1) + existing.add_record(self.record_2) + existing.add_record(self.record_3) + existing.add_record(self.record_4) + plan = HelperPlan(existing, None, changes, True) + plan.raise_if_unsafe() + + # Creates don't count against us + changes.append(Create(self.record_1)) + changes.append(Create(self.record_2)) + changes.append(Create(self.record_3)) + changes.append(Create(self.record_4)) + plan = HelperPlan(existing, None, changes, True) + plan.raise_if_unsafe() + + # One update, still good (25%, default threshold is 33%) + changes.append(Update(self.record_1, self.record_1)) + plan = HelperPlan(existing, None, changes, True) + plan.raise_if_unsafe() + + # Two and we're over the threshold + changes.append(Update(self.record_2, self.record_2)) + plan = HelperPlan(existing, None, changes, True) + with self.assertRaises(TooMuchChange) as ctx: + plan.raise_if_unsafe() + self.assertTrue('Too many updates', str(ctx.exception)) + + # If we require more records before applying we're still OK though + plan = HelperPlan(existing, None, changes, True, min_existing=10) + plan.raise_if_unsafe() + + def test_too_many_deletes(self): + existing = self.existing.copy() + changes = [] + + # No records, no changes, we're good + plan = HelperPlan(existing, None, changes, True) + plan.raise_if_unsafe() + + # Four records, no changes, we're good + existing.add_record(self.record_1) + existing.add_record(self.record_2) + existing.add_record(self.record_3) + existing.add_record(self.record_4) + plan = HelperPlan(existing, None, changes, True) + plan.raise_if_unsafe() + + # Creates don't count against us + changes.append(Create(self.record_1)) + changes.append(Create(self.record_2)) + changes.append(Create(self.record_3)) + changes.append(Create(self.record_4)) + plan = HelperPlan(existing, None, changes, True) + plan.raise_if_unsafe() + + # One delete, still good (25%, default threshold is 33%) + changes.append(Delete(self.record_1)) + plan = HelperPlan(existing, None, changes, True) + plan.raise_if_unsafe() + + # Two and we're over the threshold + changes.append(Delete(self.record_2)) + plan = HelperPlan(existing, None, changes, True) + with self.assertRaises(TooMuchChange) as ctx: + plan.raise_if_unsafe() + self.assertTrue('Too many deletes', str(ctx.exception)) + + # If we require more records before applying we're still OK though + plan = HelperPlan(existing, None, changes, True, min_existing=10) + plan.raise_if_unsafe() + + def test_root_ns_change(self): + existing = self.existing.copy() + changes = [] + + # No records, no changes, we're good + plan = HelperPlan(existing, None, changes, True) + plan.raise_if_unsafe() + + existing.add_record(self.record_1) + existing.add_record(self.record_2) + existing.add_record(self.record_3) + existing.add_record(self.record_4) + + # Non NS changes and we're still good + changes.append(Update(self.record_1, self.record_1)) + plan = HelperPlan(existing, None, changes, True) + plan.raise_if_unsafe() + + # Add a change to a non-root NS record, we're OK + ns_record = Record.new(existing, 'sub', data={ + 'type': 'NS', + 'ttl': 43, + 'values': ('ns1.unit.tests.', 'ns1.unit.tests.'), + }) + changes.append(Delete(ns_record)) + plan = HelperPlan(existing, None, changes, True) + plan.raise_if_unsafe() + # Remove that Delete so that we don't go over the delete threshold + changes.pop(-1) + + # Delete the root NS record and we get an unsafe + root_ns_record = Record.new(existing, '', data={ + 'type': 'NS', + 'ttl': 43, + 'values': ('ns3.unit.tests.', 'ns4.unit.tests.'), + }) + changes.append(Delete(root_ns_record)) + plan = HelperPlan(existing, None, changes, True) + with self.assertRaises(RootNsChange) as ctx: + plan.raise_if_unsafe() + self.assertTrue('Root Ns record change', str(ctx.exception)) diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index bd6c361..398e300 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 \ @@ -167,6 +167,28 @@ class TestBaseProvider(TestCase): self.assertTrue(plan) self.assertEqual(1, len(plan.changes)) + def test_plan_with_process_desired_zone_kwarg_fallback(self): + ignored = Zone('unit.tests.', []) + + class OldApiProvider(HelperProvider): + + def _process_desired_zone(self, desired): + return desired + + # No change, thus no plan + provider = OldApiProvider([]) + self.assertEqual(None, provider.plan(ignored)) + + class OtherTypeErrorProvider(HelperProvider): + + def _process_desired_zone(self, desired, exists=False): + raise TypeError('foo') + + provider = OtherTypeErrorProvider([]) + with self.assertRaises(TypeError) as ctx: + provider.plan(ignored) + self.assertEqual('foo', str(ctx.exception)) + def test_plan_with_unsupported_type(self): zone = Zone('unit.tests.', []) @@ -229,6 +251,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.', []) @@ -258,6 +299,37 @@ class TestBaseProvider(TestCase): # We filtered out the only change self.assertFalse(plan) + def test_plan_order_of_operations(self): + + class MockProvider(BaseProvider): + log = getLogger('mock-provider') + SUPPORTS = set(('A',)) + SUPPORTS_GEO = False + + def __init__(self): + super().__init__('mock-provider') + self.calls = [] + + def populate(self, *args, **kwargs): + self.calls.append('populate') + + def _process_desired_zone(self, *args, **kwargs): + self.calls.append('_process_desired_zone') + return super()._process_desired_zone(*args, **kwargs) + + def _process_existing_zone(self, *args, **kwargs): + self.calls.append('_process_existing_zone') + return super()._process_existing_zone(*args, **kwargs) + + provider = MockProvider() + + zone = Zone('unit.tests.', []) + self.assertFalse(provider.plan(zone)) + # ensure the calls were made in the expected order, populate comes + # first, then desired, then existing + self.assertEqual(['populate', '_process_desired_zone', + '_process_existing_zone'], provider.calls) + def test_process_desired_zone(self): provider = HelperProvider('test') @@ -278,10 +350,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 @@ -329,6 +397,45 @@ class TestBaseProvider(TestCase): 'obey' ) + # SUPPORTS_ROOT_NS + provider.SUPPORTS_ROOT_NS = False + zone1 = Zone('unit.tests.', []) + record1 = Record.new(zone1, '', { + 'type': 'NS', + 'ttl': 3600, + 'values': ['foo.com.', 'bar.com.'], + }) + zone1.add_record(record1) + + zone2 = provider._process_desired_zone(zone1.copy()) + self.assertEqual(0, len(zone2.records)) + + provider.SUPPORTS_ROOT_NS = True + zone2 = provider._process_desired_zone(zone1.copy()) + self.assertEqual(1, len(zone2.records)) + self.assertEqual(record1, list(zone2.records)[0]) + + def test_process_existing_zone(self): + provider = HelperProvider('test') + + # SUPPORTS_ROOT_NS + provider.SUPPORTS_ROOT_NS = False + zone1 = Zone('unit.tests.', []) + record1 = Record.new(zone1, '', { + 'type': 'NS', + 'ttl': 3600, + 'values': ['foo.com.', 'bar.com.'], + }) + zone1.add_record(record1) + + zone2 = provider._process_existing_zone(zone1.copy(), zone1) + self.assertEqual(0, len(zone2.records)) + + provider.SUPPORTS_ROOT_NS = True + zone2 = provider._process_existing_zone(zone1.copy(), zone1) + self.assertEqual(1, len(zone2.records)) + self.assertEqual(record1, list(zone2.records)[0]) + def test_safe_none(self): # No changes is safe Plan(None, None, [], True).raise_if_unsafe() @@ -552,3 +659,265 @@ class TestBaseProvider(TestCase): strict.supports_warn_or_except('Hello World!', 'Will not see') self.assertEqual('minimal: Hello World!', str(ctx.exception)) strict.log.warning.assert_not_called() + + +class TestBaseProviderSupportsRootNs(TestCase): + + class Provider(BaseProvider): + log = getLogger('Provider') + + SUPPORTS = set(('A', 'NS')) + SUPPORTS_GEO = False + SUPPORTS_ROOT_NS = False + + strict_supports = False + + def __init__(self, existing=None): + super().__init__('test') + self.existing = existing + + def populate(self, zone, target=False, lenient=False): + if self.existing: + for record in self.existing.records: + zone.add_record(record) + return True + return False + + zone = Zone('unit.tests.', []) + a_record = Record.new(zone, 'ptr', { + 'type': 'A', + 'ttl': 3600, + 'values': ['1.2.3.4', '2.3.4.5'], + }) + ns_record = Record.new(zone, 'sub', { + 'type': 'NS', + 'ttl': 3600, + 'values': ['ns2.foo.com.', 'ns2.bar.com.'], + }) + no_root = zone.copy() + no_root.add_record(a_record) + no_root.add_record(ns_record) + + root_ns_record = Record.new(zone, '', { + 'type': 'NS', + 'ttl': 3600, + 'values': ['ns1.foo.com.', 'ns1.bar.com.'], + }) + has_root = no_root.copy() + has_root.add_record(root_ns_record) + + other_root_ns_record = Record.new(zone, '', { + 'type': 'NS', + 'ttl': 3600, + 'values': ['ns4.foo.com.', 'ns4.bar.com.'], + }) + different_root = no_root.copy() + different_root.add_record(other_root_ns_record) + + # False + + def test_supports_root_ns_false_matches(self): + # provider has a matching existing root record + provider = self.Provider(self.has_root) + provider.SUPPORTS_ROOT_NS = False + + # matching root NS in the desired + plan = provider.plan(self.has_root) + + # no root ns upport on the target provider so doesn't matter, still no + # changes + self.assertFalse(plan) + + # plan again with strict_supports enabled, we should get an exception + # b/c we have something configured that can't be managed + provider.strict_supports = True + with self.assertRaises(SupportsException) as ctx: + provider.plan(self.has_root) + self.assertEqual('test: root NS record not supported for unit.tests.', + str(ctx.exception)) + + def test_supports_root_ns_false_different(self): + # provider has a non-matching existing record + provider = self.Provider(self.different_root) + provider.SUPPORTS_ROOT_NS = False + + # different root is in the desired + plan = provider.plan(self.has_root) + + # the mis-match doesn't matter since we can't manage the records + # anyway, they will have been removed from the desired and existing. + self.assertFalse(plan) + + # plan again with strict_supports enabled, we should get an exception + # b/c we have something configured that can't be managed (doesn't + # matter that it's a mis-match) + provider.strict_supports = True + with self.assertRaises(SupportsException) as ctx: + provider.plan(self.has_root) + self.assertEqual('test: root NS record not supported for unit.tests.', + str(ctx.exception)) + + def test_supports_root_ns_false_missing(self): + # provider has an existing record + provider = self.Provider(self.has_root) + provider.SUPPORTS_ROOT_NS = False + + # desired doesn't have a root + plan = provider.plan(self.no_root) + + # the mis-match doesn't matter since we can't manage the records + # anyway, they will have been removed from the desired and existing. + self.assertFalse(plan) + + # plan again with strict supports enabled, no change since desired + # isn't asking to manage root + provider.strict_supports = True + plan = provider.plan(self.no_root) + self.assertFalse(plan) + + def test_supports_root_ns_false_create_zone(self): + # provider has no existing records (create) + provider = self.Provider() + provider.SUPPORTS_ROOT_NS = False + + # case where we have a root NS in the desired + plan = provider.plan(self.has_root) + + # no support for root NS so we only create the other two records + self.assertTrue(plan) + self.assertEqual(2, len(plan.changes)) + + # plan again with strict supports enabled, we'll get an exception b/c + # the target provider can't manage something in desired + provider.strict_supports = True + with self.assertRaises(SupportsException) as ctx: + provider.plan(self.has_root) + self.assertEqual('test: root NS record not supported for unit.tests.', + str(ctx.exception)) + + def test_supports_root_ns_false_create_zone_missing(self): + # provider has no existing records (create) + provider = self.Provider() + provider.SUPPORTS_ROOT_NS = False + + # case where we have a root NS in the desired + plan = provider.plan(self.no_root) + + # no support for root NS so we only create the other two records + self.assertTrue(plan) + self.assertEqual(2, len(plan.changes)) + + # plan again with strict supports enabled, same result since we're not + # asking for a root NS it's just the 2 other changes + provider.strict_supports = True + plan = provider.plan(self.no_root) + self.assertTrue(plan) + self.assertEqual(2, len(plan.changes)) + + # True + + def test_supports_root_ns_true_matches(self): + # provider has a matching existing root record + provider = self.Provider(self.has_root) + provider.SUPPORTS_ROOT_NS = True + + # same root NS in the desired + plan = provider.plan(self.has_root) + + # root NS is supported in the target provider, but they match so no + # change + self.assertFalse(plan) + + # again with strict supports enabled, no difference + provider.strict_supports = True + plan = provider.plan(self.has_root) + self.assertFalse(plan) + + def test_supports_root_ns_true_different(self): + # provider has a non-matching existing record + provider = self.Provider(self.different_root) + provider.SUPPORTS_ROOT_NS = True + + # non-matching root NS in the desired + plan = provider.plan(self.has_root) + + # root NS mismatch in a target provider that supports it, we'll see the + # change + self.assertTrue(plan) + change = plan.changes[0] + self.assertEqual(self.other_root_ns_record, change.existing) + self.assertEqual(self.root_ns_record, change.new) + + # again with strict supports enabled, no difference, we see the change + provider.strict_supports = True + plan = provider.plan(self.has_root) + self.assertTrue(plan) + change = plan.changes[0] + self.assertEqual(self.other_root_ns_record, change.existing) + self.assertEqual(self.root_ns_record, change.new) + + def test_supports_root_ns_true_missing(self): + # provider has a matching existing root record + provider = self.Provider(self.has_root) + provider.SUPPORTS_ROOT_NS = True + + # there's no root record in the desired + plan = provider.plan(self.no_root) + + # the existing root NS in the target is left alone/as is since we + # aren't configured with one to manage + self.assertFalse(plan) + + # again with strict supports enabled, no difference as non-configured + # root NS is a special case that we always just warn about. This is + # because we can't known them before it's created and some people may + # choose to just leave them unmanaged undefinitely which has been the + # behavior up until now. + provider.strict_supports = True + plan = provider.plan(self.no_root) + self.assertFalse(plan) + + def test_supports_root_ns_true_create_zone(self): + # provider has no existing records (create) + provider = self.Provider() + provider.SUPPORTS_ROOT_NS = True + + # case where we have a root NS in the desired + plan = provider.plan(self.has_root) + + # there's no existing root record since we're creating the zone so + # we'll get a plan that creates everything, including it + self.assertTrue(plan) + self.assertEqual(3, len(plan.changes)) + change = [c for c in plan.changes + if c.new.name == '' and c.new._type == 'NS'][0] + self.assertFalse(change.existing) + self.assertEqual(self.root_ns_record, change.new) + + # again with strict supports enabled, no difference, we see all 3 + # changes + provider.strict_supports = True + plan = provider.plan(self.has_root) + self.assertTrue(plan) + self.assertEqual(3, len(plan.changes)) + change = [c for c in plan.changes + if c.new.name == '' and c.new._type == 'NS'][0] + self.assertFalse(change.existing) + self.assertEqual(self.root_ns_record, change.new) + + def test_supports_root_ns_true_create_zone_missing(self): + # provider has no existing records (create) + provider = self.Provider() + provider.SUPPORTS_ROOT_NS = True + + # we don't have a root NS configured so we'll ignore them and just + # manage the other records + plan = provider.plan(self.no_root) + self.assertEqual(2, len(plan.changes)) + + # again with strict supports enabled, we'd normally throw an exception, + # but since this is a create and we often can't know the root NS values + # before the zone is created it's special cased and will only warn + provider.strict_supports = True + plan = provider.plan(self.no_root) + self.assertEqual(2, len(plan.changes)) diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index bffe4c5..51e55eb 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -53,7 +53,7 @@ class TestYamlProvider(TestCase): directory = join(td.dirname, 'sub', 'dir') yaml_file = join(directory, 'unit.tests.yaml') dynamic_yaml_file = join(directory, 'dynamic.tests.yaml') - target = YamlProvider('test', directory) + target = YamlProvider('test', directory, supports_root_ns=False) # We add everything plan = target.plan(zone) @@ -82,6 +82,10 @@ class TestYamlProvider(TestCase): [x for x in reloaded.records if x.name == 'included'][0]._octodns) + # manually copy over the root since it will have been ignored + # when things were written out + reloaded.add_record(zone.root_ns) + self.assertFalse(zone.changes(reloaded, target=source)) # A 2nd sync should still create everything @@ -156,7 +160,8 @@ class TestYamlProvider(TestCase): self.assertEqual([], list(data.keys())) def test_empty(self): - source = YamlProvider('test', join(dirname(__file__), 'config')) + source = YamlProvider('test', join(dirname(__file__), 'config'), + supports_root_ns=False) zone = Zone('empty.', []) @@ -165,7 +170,8 @@ class TestYamlProvider(TestCase): self.assertEqual(0, len(zone.records)) def test_unsorted(self): - source = YamlProvider('test', join(dirname(__file__), 'config')) + source = YamlProvider('test', join(dirname(__file__), 'config'), + supports_root_ns=False) zone = Zone('unordered.', []) @@ -173,13 +179,14 @@ class TestYamlProvider(TestCase): source.populate(zone) source = YamlProvider('test', join(dirname(__file__), 'config'), - enforce_order=False) + enforce_order=False, supports_root_ns=False) # no exception source.populate(zone) self.assertEqual(2, len(zone.records)) def test_subzone_handling(self): - source = YamlProvider('test', join(dirname(__file__), 'config')) + source = YamlProvider('test', join(dirname(__file__), 'config'), + supports_root_ns=False) # If we add `sub` as a sub-zone we'll reject `www.sub` zone = Zone('unit.tests.', ['sub']) @@ -259,7 +266,8 @@ class TestSplitYamlProvider(TestCase): zone_dir = join(directory, 'unit.tests.tst') dynamic_zone_dir = join(directory, 'dynamic.tests.tst') target = SplitYamlProvider('test', directory, - extension='.tst') + extension='.tst', + supports_root_ns=False) # We add everything plan = target.plan(zone) @@ -287,6 +295,10 @@ class TestSplitYamlProvider(TestCase): [x for x in reloaded.records if x.name == 'included'][0]._octodns) + # manually copy over the root since it will have been ignored + # when things were written out + reloaded.add_record(zone.root_ns) + self.assertFalse(zone.changes(reloaded, target=source)) # A 2nd sync should still create everything @@ -392,9 +404,11 @@ class TestOverridingYamlProvider(TestCase): def test_provider(self): config = join(dirname(__file__), 'config') override_config = join(dirname(__file__), 'config', 'override') - base = YamlProvider('base', config, populate_should_replace=False) + base = YamlProvider('base', config, populate_should_replace=False, + supports_root_ns=False) override = YamlProvider('test', override_config, - populate_should_replace=True) + populate_should_replace=True, + supports_root_ns=False) zone = Zone('dynamic.tests.', []) 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/test_octodns_zone.py b/tests/test_octodns_zone.py index e032369..9385984 100644 --- a/tests/test_octodns_zone.py +++ b/tests/test_octodns_zone.py @@ -7,7 +7,8 @@ from __future__ import absolute_import, division, print_function, \ from unittest import TestCase -from octodns.record import ARecord, AaaaRecord, Create, Delete, Record, Update +from octodns.record import ARecord, AaaaRecord, Create, Delete, NsRecord, \ + Record, Update from octodns.zone import DuplicateRecordException, InvalidNodeException, \ SubzoneRecordException, Zone @@ -410,3 +411,50 @@ class TestZone(TestCase): self.assertTrue(copy.hydrate()) # Doesn't the second self.assertFalse(copy.hydrate()) + + def test_root_ns(self): + zone = Zone('unit.tests.', []) + + a = ARecord(zone, 'a', {'ttl': 42, 'value': '1.1.1.1'}) + zone.add_record(a) + # No root NS yet + self.assertFalse(zone.root_ns) + + non_root_ns = NsRecord(zone, 'sub', {'ttl': 42, 'values': ( + 'ns1.unit.tests.', + 'ns2.unit.tests.', + )}) + zone.add_record(non_root_ns) + # No root NS yet b/c this was a sub + self.assertFalse(zone.root_ns) + + root_ns = NsRecord(zone, '', {'ttl': 42, 'values': ( + 'ns3.unit.tests.', + 'ns4.unit.tests.', + )}) + zone.add_record(root_ns) + # Now we have a root NS + self.assertEqual(root_ns, zone.root_ns) + + # make a copy, it has a root_ns + copy = zone.copy() + self.assertEqual(root_ns, copy.root_ns) + + # remove the root NS from it and we don't + copy.remove_record(root_ns) + self.assertFalse(copy.root_ns) + + # original still does though + self.assertEqual(root_ns, zone.root_ns) + + # remove the A, still has root NS + zone.remove_record(a) + self.assertEqual(root_ns, zone.root_ns) + + # remove the sub NS, still has root NS + zone.remove_record(non_root_ns) + self.assertEqual(root_ns, zone.root_ns) + + # finally remove the root NS, no more + zone.remove_record(root_ns) + self.assertFalse(zone.root_ns) 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