diff --git a/octodns/manager.py b/octodns/manager.py index 0faa0bf..40a9c4f 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -9,6 +9,7 @@ from __future__ import ( unicode_literals, ) +from collections import deque from concurrent.futures import ThreadPoolExecutor from importlib import import_module from os import environ @@ -102,6 +103,8 @@ class Manager(object): plan = p[1] return len(plan.changes[0].record.zone.name) if plan.changes else 0 + # TODO: all of this should get broken up, mainly so that it's not so huge + # and each bit can be cleanly tested independently def __init__(self, config_file, max_workers=None, include_meta=False): version = self._try_version('octodns', version=__VERSION__) self.log.info( @@ -185,25 +188,6 @@ class Manager(object): 'Incorrect processor config for ' + processor_name ) - zone_tree = {} - # Sort so we iterate on the deepest nodes first, ensuring if a parent - # zone exists it will be seen after the subzone, thus we can easily - # reparent children to their parent zone from the tree root. - for name in sorted( - self.config['zones'].keys(), key=lambda s: 0 - s.count('.') - ): - # Trim the trailing dot from FQDN - name = name[:-1] - this = {} - for sz in [k for k in zone_tree.keys() if k.endswith(name)]: - # Found a zone in tree root that is our child, slice the - # name and move its tree under ours. - this[sz[: -(len(name) + 1)]] = zone_tree.pop(sz) - # Add to tree root where it will be reparented as we iterate up - # the tree. - zone_tree[name] = this - self.zone_tree = zone_tree - self.plan_outputs = {} plan_outputs = manager_config.get( 'plan_outputs', @@ -244,6 +228,8 @@ class Manager(object): 'Incorrect plan_output config for ' + plan_output_name ) + self._configured_sub_zones = None + def _try_version(self, module_name, module=None, version=None): try: # Always try and use the official lookup first @@ -314,23 +300,36 @@ class Manager(object): return kwargs def configured_sub_zones(self, zone_name): - name = zone_name[:-1] - where = self.zone_tree - while True: - # Find parent if it exists - parent = next((k for k in where if name.endswith(k)), None) - if not parent: - # The zone_name in the tree has been reached, stop searching. - break - # Move down the tree and slice name to get the remainder for the - # next round of the search. - where = where[parent] - name = name[: -(len(parent) + 1)] - # `where` is now pointing at the dictionary of children for zone_name - # in the tree - sub_zone_names = where.keys() - self.log.debug('configured_sub_zones: subs=%s', sub_zone_names) - return set(sub_zone_names) + if self._configured_sub_zones is None: + # First time through we compute all the sub-zones + + configured_sub_zones = {} + + # Get a list of all of our zone names. Sort them from shortest to + # longest so that parents will always come before their subzones + zones = sorted( + self.config['zones'].keys(), key=lambda z: len(z), reverse=True + ) + zones = deque(zones) + # Until we're done processing zones + while zones: + # Grab the one we'lre going to work on now + zone = zones.pop() + dotted = f'.{zone}' + trimmer = len(dotted) + subs = set() + # look at all the zone names that come after it + for candidate in zones: + # If they end with this zone's dotted name, it's a sub + if candidate.endswith(dotted): + # We want subs to exclude the zone portion + subs.add(candidate[:-trimmer]) + + configured_sub_zones[zone] = subs + + self._configured_sub_zones = configured_sub_zones + + return self._configured_sub_zones.get(zone_name, set()) def _populate_and_plan( self, @@ -708,6 +707,7 @@ class Manager(object): clz = SplitYamlProvider target = clz('dump', output_dir) + # TODO: use get_zone??? zone = Zone(zone, self.configured_sub_zones(zone)) for source in sources: source.populate(zone, lenient=lenient) diff --git a/octodns/zone.py b/octodns/zone.py index f93b373..1bc3724 100644 --- a/octodns/zone.py +++ b/octodns/zone.py @@ -73,19 +73,23 @@ class Zone(object): name = record.name - if not lenient and any((name.endswith(sz) for sz in self.sub_zones)): - if name not in self.sub_zones: - # it's a record for something under a sub-zone - raise SubzoneRecordException( - f'Record {record.fqdn} is under ' 'a managed subzone' - ) - elif record._type != 'NS': - # It's a non NS record for exactly a sub-zone - raise SubzoneRecordException( - f'Record {record.fqdn} a ' - 'managed sub-zone and not of ' - 'type NS' - ) + if not lenient: + if name in self.sub_zones: + # It's an exact match for a sub-zone + if not record._type == 'NS': + # and not a NS record, this should be in the sub + raise SubzoneRecordException( + f'Record {record.fqdn} is a managed sub-zone and not of type NS' + ) + else: + # It's not an exact match so there has to be a `.` before the + # sub-zone for it to belong in there + for sub_zone in self.sub_zones: + if name.endswith(f'.{sub_zone}'): + # this should be in a sub + raise SubzoneRecordException( + f'Record {record.fqdn} is under a managed subzone' + ) if replace: # will remove it if it exists diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index ab987d2..bdacb08 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -712,6 +712,125 @@ class TestManager(TestCase): ), ) + def test_subzone_handling(self): + manager = Manager(get_config_filename('simple.yaml')) + + # tree with multiple branches, one that skips + manager.config['zones'] = { + 'unit.tests.': {}, + 'sub.unit.tests.': {}, + 'another.sub.unit.tests.': {}, + 'skipped.alevel.unit.tests.': {}, + } + + self.assertEqual( + {'another.sub', 'sub', 'skipped.alevel'}, + manager.configured_sub_zones('unit.tests.'), + ) + self.assertEqual( + {'another'}, manager.configured_sub_zones('sub.unit.tests.') + ) + self.assertEqual( + set(), manager.configured_sub_zones('another.sub.unit.tests.') + ) + self.assertEqual( + set(), manager.configured_sub_zones('skipped.alevel.unit.tests.') + ) + + # unknown zone names return empty set + self.assertEqual(set(), manager.configured_sub_zones('unknown.tests.')) + + # two parallel trees, make sure they don't interfere + manager.config['zones'] = { + 'unit.tests.': {}, + 'unit2.tests.': {}, + 'sub.unit.tests.': {}, + 'sub.unit2.tests.': {}, + 'another.sub.unit.tests.': {}, + 'another.sub.unit2.tests.': {}, + 'skipped.alevel.unit.tests.': {}, + 'skipped.alevel.unit2.tests.': {}, + } + manager._configured_sub_zones = None + self.assertEqual( + {'another.sub', 'sub', 'skipped.alevel'}, + manager.configured_sub_zones('unit.tests.'), + ) + self.assertEqual( + {'another'}, manager.configured_sub_zones('sub.unit.tests.') + ) + self.assertEqual( + set(), manager.configured_sub_zones('another.sub.unit.tests.') + ) + self.assertEqual( + set(), manager.configured_sub_zones('skipped.alevel.unit.tests.') + ) + self.assertEqual( + {'another.sub', 'sub', 'skipped.alevel'}, + manager.configured_sub_zones('unit2.tests.'), + ) + self.assertEqual( + {'another'}, manager.configured_sub_zones('sub.unit2.tests.') + ) + self.assertEqual( + set(), manager.configured_sub_zones('another.sub.unit2.tests.') + ) + self.assertEqual( + set(), manager.configured_sub_zones('skipped.alevel.unit2.tests.') + ) + + # zones that end with names of others + manager.config['zones'] = { + 'unit.tests.': {}, + 'uunit.tests.': {}, + 'uuunit.tests.': {}, + } + manager._configured_sub_zones = None + self.assertEqual(set(), manager.configured_sub_zones('unit.tests.')) + self.assertEqual(set(), manager.configured_sub_zones('uunit.tests.')) + self.assertEqual(set(), manager.configured_sub_zones('uuunit.tests.')) + + # skipping multiple levels + manager.config['zones'] = { + 'unit.tests.': {}, + 'foo.bar.baz.unit.tests.': {}, + } + manager._configured_sub_zones = None + self.assertEqual( + {'foo.bar.baz'}, manager.configured_sub_zones('unit.tests.') + ) + self.assertEqual( + set(), manager.configured_sub_zones('foo.bar.baz.unit.tests.') + ) + + # different TLDs + manager.config['zones'] = { + 'unit.tests.': {}, + 'foo.unit.tests.': {}, + 'unit.org.': {}, + 'bar.unit.org.': {}, + } + manager._configured_sub_zones = None + self.assertEqual({'foo'}, manager.configured_sub_zones('unit.tests.')) + self.assertEqual(set(), manager.configured_sub_zones('foo.unit.tests.')) + self.assertEqual({'bar'}, manager.configured_sub_zones('unit.org.')) + self.assertEqual(set(), manager.configured_sub_zones('bar.unit.org.')) + + # starting a beyond 2 levels + manager.config['zones'] = { + 'foo.unit.tests.': {}, + 'bar.foo.unit.tests.': {}, + 'bleep.bloop.foo.unit.tests.': {}, + } + manager._configured_sub_zones = None + self.assertEqual( + {'bar', 'bleep.bloop'}, + manager.configured_sub_zones('foo.unit.tests.'), + ) + self.assertEqual( + set(), manager.configured_sub_zones('bar.foo.unit.tests.') + ) + class TestMainThreadExecutor(TestCase): def test_success(self): diff --git a/tests/test_octodns_zone.py b/tests/test_octodns_zone.py index b884397..37e893f 100644 --- a/tests/test_octodns_zone.py +++ b/tests/test_octodns_zone.py @@ -208,6 +208,16 @@ class TestZone(TestCase): zone.add_record(record, lenient=True) self.assertEqual(set([record]), zone.records) + # A that happens to end with a string that matches a sub (no .) is OK + zone = Zone('unit.tests.', set(['sub', 'barred'])) + record = Record.new( + zone, + 'foo.bar_sub', + {'ttl': 3600, 'type': 'A', 'values': ['1.2.3.4', '2.3.4.5']}, + ) + zone.add_record(record) + self.assertEqual(1, len(zone.records)) + def test_ignored_records(self): zone_normal = Zone('unit.tests.', []) zone_ignored = Zone('unit.tests.', [])