diff --git a/octodns/manager.py b/octodns/manager.py index e640a8d..030298e 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -162,23 +162,21 @@ class Manager(object): processor_name) zone_tree = {} - # sort by reversed strings so that parent zones always come first - for name in sorted(self.config['zones'].keys(), key=lambda s: s[::-1]): - # ignore trailing dots, and reverse - pieces = name[:-1].split('.')[::-1] - # where starts out at the top - where = zone_tree - # for all the pieces - for piece in pieces: - try: - where = where[piece] - # our current piece already exists, just point where at - # it's value - except KeyError: - # our current piece doesn't exist, create it - where[piece] = {} - # and then point where at it's newly created value - where = where[piece] + # 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 = {} @@ -274,21 +272,20 @@ class Manager(object): return kwargs def configured_sub_zones(self, zone_name): - # Reversed pieces of the zone name - pieces = zone_name[:-1].split('.')[::-1] - # Point where at the root of the tree + name = zone_name[:-1] where = self.zone_tree - # Until we've hit the bottom of this zone - try: - while pieces: - # Point where at the value of our current piece - where = where[pieces.pop(0)] - except KeyError: - self.log.debug('configured_sub_zones: unknown zone, %s, no subs', - zone_name) - return set() - # We're not pointed at the dict for our name, the keys of which will be - # any subzones + 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) diff --git a/octodns/zone.py b/octodns/zone.py index 4cd5e91..7e42fa3 100644 --- a/octodns/zone.py +++ b/octodns/zone.py @@ -68,10 +68,9 @@ class Zone(object): self.hydrate() name = record.name - last = name.split('.')[-1] - if not lenient and last in self.sub_zones: - if name != last: + 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') diff --git a/tests/config/simple.yaml b/tests/config/simple.yaml index fc4ad9f..4cd19c8 100644 --- a/tests/config/simple.yaml +++ b/tests/config/simple.yaml @@ -33,6 +33,11 @@ zones: targets: - dump - dump2 + sub.txt.unit.tests.: + sources: + - in + targets: + - dump empty.: sources: - in diff --git a/tests/config/sub.txt.unit.tests.yaml b/tests/config/sub.txt.unit.tests.yaml new file mode 100644 index 0000000..ed97d53 --- /dev/null +++ b/tests/config/sub.txt.unit.tests.yaml @@ -0,0 +1 @@ +--- diff --git a/tests/config/unit.tests.yaml b/tests/config/unit.tests.yaml index aa28ee5..4fecedd 100644 --- a/tests/config/unit.tests.yaml +++ b/tests/config/unit.tests.yaml @@ -162,6 +162,16 @@ sub: values: - 6.2.3.4. - 7.2.3.4. +sub.txt: + type: 'NS' + values: + - ns1.test. + - ns2.test. +subzone: + type: 'NS' + values: + - 192.0.2.1. + - 192.0.2.8. txt: ttl: 600 type: TXT diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 1c01af9..8f84e67 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -121,12 +121,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(28, 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(22, tc) # the subzone, with 2 targets tc = Manager(get_config_filename('simple.yaml')) \ @@ -141,18 +141,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(28, 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(28, 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(33, tc) def test_eligible_sources(self): with TemporaryDirectory() as tmpdir: @@ -220,13 +220,13 @@ class TestManager(TestCase): # 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(21, len(changes)) + self.assertEqual(23, len(changes)) # Compound sources with varying support changes = manager.compare(['in', 'nosshfp'], ['dump'], 'unit.tests.') - self.assertEqual(20, len(changes)) + self.assertEqual(22, len(changes)) with self.assertRaises(ManagerException) as ctx: manager.compare(['nope'], ['dump'], 'unit.tests.') diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 51e55eb..3a6ead8 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -34,7 +34,7 @@ class TestYamlProvider(TestCase): # without it we see everything source.populate(zone) - self.assertEqual(23, len(zone.records)) + self.assertEqual(25, len(zone.records)) source.populate(dynamic_zone) self.assertEqual(6, len(dynamic_zone.records)) @@ -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(22, 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(22, target.apply(plan)) self.assertTrue(isfile(yaml_file)) # Dynamic plan @@ -90,7 +90,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(22, len([c for c in plan.changes if isinstance(c, Create)])) with open(yaml_file) as fh: @@ -111,6 +111,8 @@ class TestYamlProvider(TestCase): self.assertTrue('values' in data.pop('txt')) self.assertTrue('values' in data.pop('loc')) self.assertTrue('values' in data.pop('urlfwd')) + self.assertTrue('values' in data.pop('sub.txt')) + self.assertTrue('values' in data.pop('subzone')) # these are stored as singular 'value' self.assertTrue('value' in data.pop('_imap._tcp')) self.assertTrue('value' in data.pop('_pop3._tcp'))