From b926d78c5c182a13df03566ea9327dffdc9fb29f Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Mon, 3 Aug 2020 00:47:22 +0200 Subject: [PATCH 01/26] Add support for zones aliases This commit adds support for zones aliases. This allows to define one or multiple zone as aliases of an existing zone without using workarounds like simlinks and miltiple "zones" entries in the configuration file. An alias zone is share all of its content with it parent zone, only the name of the zone is different. ``` zones: example.com.: aliases: - example.net. - example.org. sources: - in targets: - out ``` Known issues: - No documentation, - Only the `octodns-sync` and `octodns-validate` commands supports aliases zones at this time, I added a loop in the manager init function which convert all alias zone to "real" ones during config validation, however I'm not sure this is the right approach. Comments welcome. --- octodns/manager.py | 34 ++++++++++++++++++++++++------ octodns/provider/yaml.py | 15 +++++++------ octodns/zone.py | 3 ++- tests/config/bad-zone-aliases.yaml | 17 +++++++++++++++ tests/config/simple-aliases.yaml | 17 +++++++++++++++ tests/test_octodns_manager.py | 14 ++++++++++-- 6 files changed, 84 insertions(+), 16 deletions(-) create mode 100644 tests/config/bad-zone-aliases.yaml create mode 100644 tests/config/simple-aliases.yaml diff --git a/octodns/manager.py b/octodns/manager.py index 0665938..2e1f6df 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -121,6 +121,20 @@ class Manager(object): raise ManagerException('Incorrect provider config for {}' .format(provider_name)) + for zone_name, zone_config in self.config['zones'].copy().items(): + if 'aliases' in zone_config: + for alias in zone_config['aliases']: + if alias in self.config['zones']: + self.log.exception('Invalid zone alias') + raise ManagerException('Invalid zone alias {}: ' + 'this zone already exists' + .format(alias)) + + self.config['zones'][alias] = zone_config + self.config['zones'][alias]['template_zone'] = zone_name + + del self.config['zones'][zone_name]['aliases'] + 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]): @@ -222,12 +236,14 @@ class Manager(object): self.log.debug('configured_sub_zones: subs=%s', sub_zone_names) return set(sub_zone_names) - def _populate_and_plan(self, zone_name, sources, targets, lenient=False): + def _populate_and_plan(self, zone_name, template_zone, sources, targets, + lenient=False): - self.log.debug('sync: populating, zone=%s, lenient=%s', - zone_name, lenient) + self.log.debug('sync: populating, zone=%s, template=%s, lenient=%s', + zone_name, template_zone, lenient) zone = Zone(zone_name, - sub_zones=self.configured_sub_zones(zone_name)) + sub_zones=self.configured_sub_zones(zone_name), + template_zone=template_zone) for source in sources: try: source.populate(zone, lenient=lenient) @@ -269,6 +285,7 @@ class Manager(object): for zone_name, config in zones: self.log.info('sync: zone=%s', zone_name) lenient = config.get('lenient', False) + template_zone = config.get('template_zone', zone_name) try: sources = config['sources'] except KeyError: @@ -318,8 +335,9 @@ class Manager(object): .format(zone_name, target)) futures.append(self._executor.submit(self._populate_and_plan, - zone_name, sources, - targets, lenient=lenient)) + zone_name, template_zone, + sources, targets, + lenient=lenient)) # Wait on all results and unpack/flatten them in to a list of target & # plan pairs. @@ -413,7 +431,9 @@ class Manager(object): def validate_configs(self): for zone_name, config in self.config['zones'].items(): - zone = Zone(zone_name, self.configured_sub_zones(zone_name)) + template_zone = config.get('template_zone', zone_name) + zone = Zone(zone_name, self.configured_sub_zones(zone_name), + template_zone) try: sources = config['sources'] diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 10add5a..878d7d5 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -139,8 +139,8 @@ class YamlProvider(BaseProvider): filename) def populate(self, zone, target=False, lenient=False): - self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, - target, lenient) + self.log.debug('populate: name=%s, template=%s, target=%s, lenient=%s', + zone.name, zone.template_zone, target, lenient) if target: # When acting as a target we ignore any existing records so that we @@ -148,7 +148,9 @@ class YamlProvider(BaseProvider): return False before = len(zone.records) - filename = join(self.directory, '{}yaml'.format(zone.name)) + filename = join(self.directory, '{}yaml'.format(zone.template_zone + if zone.template_zone + else zone.name)) self._populate_from_file(filename, zone, lenient) self.log.info('populate: found %s records, exists=False', @@ -243,11 +245,12 @@ class SplitYamlProvider(YamlProvider): super(SplitYamlProvider, self).__init__(id, directory, *args, **kwargs) def _zone_directory(self, zone): - return join(self.directory, zone.name) + return join(self.directory, zone.template_zone if zone.template_zone + else zone.name) def populate(self, zone, target=False, lenient=False): - self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, - target, lenient) + self.log.debug('populate: name=%s, template=%s, target=%s, lenient=%s', + zone.name, zone.template_zone, target, lenient) if target: # When acting as a target we ignore any existing records so that we diff --git a/octodns/zone.py b/octodns/zone.py index 5f099ac..7a5aaa6 100644 --- a/octodns/zone.py +++ b/octodns/zone.py @@ -35,13 +35,14 @@ def _is_eligible(record): class Zone(object): log = getLogger('Zone') - def __init__(self, name, sub_zones): + def __init__(self, name, sub_zones, template_zone=None): if not name[-1] == '.': raise Exception('Invalid zone name {}, missing ending dot' .format(name)) # Force everything to lowercase just to be safe self.name = text_type(name).lower() if name else name self.sub_zones = sub_zones + self.template_zone = template_zone # 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) diff --git a/tests/config/bad-zone-aliases.yaml b/tests/config/bad-zone-aliases.yaml new file mode 100644 index 0000000..4c47e3c --- /dev/null +++ b/tests/config/bad-zone-aliases.yaml @@ -0,0 +1,17 @@ +manager: + max_workers: 2 +providers: + in: + class: octodns.provider.yaml.YamlProvider + directory: tests/config + dump: + class: octodns.provider.yaml.YamlProvider + directory: env/YAML_TMP_DIR +zones: + unit.tests.: + aliases: + - unit.tests. + sources: + - in + targets: + - dump diff --git a/tests/config/simple-aliases.yaml b/tests/config/simple-aliases.yaml new file mode 100644 index 0000000..07a2d74 --- /dev/null +++ b/tests/config/simple-aliases.yaml @@ -0,0 +1,17 @@ +manager: + max_workers: 2 +providers: + in: + class: octodns.provider.yaml.YamlProvider + directory: tests/config + dump: + class: octodns.provider.yaml.YamlProvider + directory: env/YAML_TMP_DIR +zones: + unit.tests.: + aliases: + - unit-alias.tests. + sources: + - in + targets: + - dump diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 581689a..052238f 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -290,7 +290,8 @@ class TestManager(TestCase): pass # This should be ok, we'll fall back to not passing it - manager._populate_and_plan('unit.tests.', [NoLenient()], []) + manager._populate_and_plan('unit.tests.', 'unit.tests.', + [NoLenient()], []) class NoZone(SimpleProvider): @@ -299,7 +300,16 @@ class TestManager(TestCase): # This will blow up, we don't fallback for source with self.assertRaises(TypeError): - manager._populate_and_plan('unit.tests.', [NoZone()], []) + manager._populate_and_plan('unit.tests.', 'unit.tests.', + [NoZone()], []) + + def test_zone_aliases(self): + Manager(get_config_filename('simple-aliases.yaml')).validate_configs() + + with self.assertRaises(ManagerException) as ctx: + Manager(get_config_filename('bad-zone-aliases.yaml')) \ + .validate_configs() + self.assertTrue('Invalid zone alias' in text_type(ctx.exception)) class TestMainThreadExecutor(TestCase): From 7bf0b31367e51e99598a914e7c97cbf831e617eb Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Tue, 20 Oct 2020 19:54:35 +0200 Subject: [PATCH 02/26] Revert "Add support for zones aliases" This reverts commit b926d78c5c182a13df03566ea9327dffdc9fb29f. --- octodns/manager.py | 34 ++++++------------------------ octodns/provider/yaml.py | 15 ++++++------- octodns/zone.py | 3 +-- tests/config/bad-zone-aliases.yaml | 17 --------------- tests/config/simple-aliases.yaml | 17 --------------- tests/test_octodns_manager.py | 14 ++---------- 6 files changed, 16 insertions(+), 84 deletions(-) delete mode 100644 tests/config/bad-zone-aliases.yaml delete mode 100644 tests/config/simple-aliases.yaml diff --git a/octodns/manager.py b/octodns/manager.py index 137f13b..288645f 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -121,20 +121,6 @@ class Manager(object): raise ManagerException('Incorrect provider config for {}' .format(provider_name)) - for zone_name, zone_config in self.config['zones'].copy().items(): - if 'aliases' in zone_config: - for alias in zone_config['aliases']: - if alias in self.config['zones']: - self.log.exception('Invalid zone alias') - raise ManagerException('Invalid zone alias {}: ' - 'this zone already exists' - .format(alias)) - - self.config['zones'][alias] = zone_config - self.config['zones'][alias]['template_zone'] = zone_name - - del self.config['zones'][zone_name]['aliases'] - 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]): @@ -236,14 +222,12 @@ class Manager(object): self.log.debug('configured_sub_zones: subs=%s', sub_zone_names) return set(sub_zone_names) - def _populate_and_plan(self, zone_name, template_zone, sources, targets, - lenient=False): + def _populate_and_plan(self, zone_name, sources, targets, lenient=False): - self.log.debug('sync: populating, zone=%s, template=%s, lenient=%s', - zone_name, template_zone, lenient) + self.log.debug('sync: populating, zone=%s, lenient=%s', + zone_name, lenient) zone = Zone(zone_name, - sub_zones=self.configured_sub_zones(zone_name), - template_zone=template_zone) + sub_zones=self.configured_sub_zones(zone_name)) for source in sources: try: source.populate(zone, lenient=lenient) @@ -285,7 +269,6 @@ class Manager(object): for zone_name, config in zones: self.log.info('sync: zone=%s', zone_name) lenient = config.get('lenient', False) - template_zone = config.get('template_zone', zone_name) try: sources = config['sources'] except KeyError: @@ -341,9 +324,8 @@ class Manager(object): .format(zone_name, target)) futures.append(self._executor.submit(self._populate_and_plan, - zone_name, template_zone, - sources, targets, - lenient=lenient)) + zone_name, sources, + targets, lenient=lenient)) # Wait on all results and unpack/flatten them in to a list of target & # plan pairs. @@ -437,9 +419,7 @@ class Manager(object): def validate_configs(self): for zone_name, config in self.config['zones'].items(): - template_zone = config.get('template_zone', zone_name) - zone = Zone(zone_name, self.configured_sub_zones(zone_name), - template_zone) + zone = Zone(zone_name, self.configured_sub_zones(zone_name)) try: sources = config['sources'] diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 878d7d5..10add5a 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -139,8 +139,8 @@ class YamlProvider(BaseProvider): filename) def populate(self, zone, target=False, lenient=False): - self.log.debug('populate: name=%s, template=%s, target=%s, lenient=%s', - zone.name, zone.template_zone, target, lenient) + self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, + target, lenient) if target: # When acting as a target we ignore any existing records so that we @@ -148,9 +148,7 @@ class YamlProvider(BaseProvider): return False before = len(zone.records) - filename = join(self.directory, '{}yaml'.format(zone.template_zone - if zone.template_zone - else zone.name)) + filename = join(self.directory, '{}yaml'.format(zone.name)) self._populate_from_file(filename, zone, lenient) self.log.info('populate: found %s records, exists=False', @@ -245,12 +243,11 @@ class SplitYamlProvider(YamlProvider): super(SplitYamlProvider, self).__init__(id, directory, *args, **kwargs) def _zone_directory(self, zone): - return join(self.directory, zone.template_zone if zone.template_zone - else zone.name) + return join(self.directory, zone.name) def populate(self, zone, target=False, lenient=False): - self.log.debug('populate: name=%s, template=%s, target=%s, lenient=%s', - zone.name, zone.template_zone, target, lenient) + self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, + target, lenient) if target: # When acting as a target we ignore any existing records so that we diff --git a/octodns/zone.py b/octodns/zone.py index 7a5aaa6..5f099ac 100644 --- a/octodns/zone.py +++ b/octodns/zone.py @@ -35,14 +35,13 @@ def _is_eligible(record): class Zone(object): log = getLogger('Zone') - def __init__(self, name, sub_zones, template_zone=None): + def __init__(self, name, sub_zones): if not name[-1] == '.': raise Exception('Invalid zone name {}, missing ending dot' .format(name)) # Force everything to lowercase just to be safe self.name = text_type(name).lower() if name else name self.sub_zones = sub_zones - self.template_zone = template_zone # 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) diff --git a/tests/config/bad-zone-aliases.yaml b/tests/config/bad-zone-aliases.yaml deleted file mode 100644 index 4c47e3c..0000000 --- a/tests/config/bad-zone-aliases.yaml +++ /dev/null @@ -1,17 +0,0 @@ -manager: - max_workers: 2 -providers: - in: - class: octodns.provider.yaml.YamlProvider - directory: tests/config - dump: - class: octodns.provider.yaml.YamlProvider - directory: env/YAML_TMP_DIR -zones: - unit.tests.: - aliases: - - unit.tests. - sources: - - in - targets: - - dump diff --git a/tests/config/simple-aliases.yaml b/tests/config/simple-aliases.yaml deleted file mode 100644 index 07a2d74..0000000 --- a/tests/config/simple-aliases.yaml +++ /dev/null @@ -1,17 +0,0 @@ -manager: - max_workers: 2 -providers: - in: - class: octodns.provider.yaml.YamlProvider - directory: tests/config - dump: - class: octodns.provider.yaml.YamlProvider - directory: env/YAML_TMP_DIR -zones: - unit.tests.: - aliases: - - unit-alias.tests. - sources: - - in - targets: - - dump diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index b6f9cbb..9956790 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -298,8 +298,7 @@ class TestManager(TestCase): pass # This should be ok, we'll fall back to not passing it - manager._populate_and_plan('unit.tests.', 'unit.tests.', - [NoLenient()], []) + manager._populate_and_plan('unit.tests.', [NoLenient()], []) class NoZone(SimpleProvider): @@ -308,16 +307,7 @@ class TestManager(TestCase): # This will blow up, we don't fallback for source with self.assertRaises(TypeError): - manager._populate_and_plan('unit.tests.', 'unit.tests.', - [NoZone()], []) - - def test_zone_aliases(self): - Manager(get_config_filename('simple-aliases.yaml')).validate_configs() - - with self.assertRaises(ManagerException) as ctx: - Manager(get_config_filename('bad-zone-aliases.yaml')) \ - .validate_configs() - self.assertTrue('Invalid zone alias' in text_type(ctx.exception)) + manager._populate_and_plan('unit.tests.', [NoZone()], []) class TestMainThreadExecutor(TestCase): From f2a6f870b40d317b95dbd3e12e8d6ddb0324f33b Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Tue, 20 Oct 2020 22:18:48 +0200 Subject: [PATCH 03/26] Make each alias zone reference its target zone instead of listing all aliases zones in the target zone configuration --- octodns/manager.py | 39 ++++++++++++++++++++++----- octodns/provider/yaml.py | 5 ++-- octodns/zone.py | 4 ++- tests/config/simple-alias-zone.yaml | 19 +++++++++++++ tests/config/unknown-source-zone.yaml | 13 +++++++++ tests/test_octodns_manager.py | 19 +++++++++++-- 6 files changed, 87 insertions(+), 12 deletions(-) create mode 100644 tests/config/simple-alias-zone.yaml create mode 100644 tests/config/unknown-source-zone.yaml diff --git a/octodns/manager.py b/octodns/manager.py index 288645f..613be29 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -121,6 +121,23 @@ class Manager(object): raise ManagerException('Incorrect provider config for {}' .format(provider_name)) + for zone_name, zone_config in self.config['zones'].copy().items(): + if 'alias' in zone_config: + source_zone = zone_config['alias'] + # Check that the source zone is defined. + if source_zone not in self.config['zones']: + self.log.exception('Invalid alias zone') + raise ManagerException('Invalid alias zone {}: ' + 'source zone {} does not exist' + .format(zone_name, source_zone)) + self.config['zones'][zone_name] = \ + self.config['zones'][source_zone] + self.config['zones'][zone_name]['is_alias'] = True + self.config['zones'][zone_name]['file'] = source_zone + else: + self.config['zones'][zone_name]['is_alias'] = False + self.config['zones'][zone_name]['file'] = zone_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]): @@ -222,12 +239,14 @@ class Manager(object): self.log.debug('configured_sub_zones: subs=%s', sub_zone_names) return set(sub_zone_names) - def _populate_and_plan(self, zone_name, sources, targets, lenient=False): + def _populate_and_plan(self, zone_name, file, is_alias, sources, targets, + lenient=False): - self.log.debug('sync: populating, zone=%s, lenient=%s', - zone_name, lenient) + self.log.debug('sync: populating, zone=%s, file=%s, is_alias=%s, ' + 'lenient=%s', zone_name, file, is_alias, lenient) zone = Zone(zone_name, - sub_zones=self.configured_sub_zones(zone_name)) + sub_zones=self.configured_sub_zones(zone_name), file=file, + is_alias=is_alias) for source in sources: try: source.populate(zone, lenient=lenient) @@ -268,6 +287,8 @@ class Manager(object): futures = [] for zone_name, config in zones: self.log.info('sync: zone=%s', zone_name) + file = config.get('file') + is_alias = config.get('is_alias') lenient = config.get('lenient', False) try: sources = config['sources'] @@ -324,8 +345,9 @@ class Manager(object): .format(zone_name, target)) futures.append(self._executor.submit(self._populate_and_plan, - zone_name, sources, - targets, lenient=lenient)) + zone_name, file, is_alias, + sources, targets, + lenient=lenient)) # Wait on all results and unpack/flatten them in to a list of target & # plan pairs. @@ -419,7 +441,10 @@ class Manager(object): def validate_configs(self): for zone_name, config in self.config['zones'].items(): - zone = Zone(zone_name, self.configured_sub_zones(zone_name)) + file = config.get('file', False) + is_alias = config.get('is_alias', False) + zone = Zone(zone_name, self.configured_sub_zones(zone_name), + file, is_alias) try: sources = config['sources'] diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 10add5a..e486982 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -139,7 +139,8 @@ class YamlProvider(BaseProvider): filename) def populate(self, zone, target=False, lenient=False): - self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, + self.log.debug('populate: name=%s, file=%s, is_alias:%s, target=%s, ' + 'lenient=%s', zone.name, zone.file, zone.is_alias, target, lenient) if target: @@ -148,7 +149,7 @@ class YamlProvider(BaseProvider): return False before = len(zone.records) - filename = join(self.directory, '{}yaml'.format(zone.name)) + filename = join(self.directory, '{}yaml'.format(zone.file)) self._populate_from_file(filename, zone, lenient) self.log.info('populate: found %s records, exists=False', diff --git a/octodns/zone.py b/octodns/zone.py index 5f099ac..0a78f72 100644 --- a/octodns/zone.py +++ b/octodns/zone.py @@ -35,13 +35,15 @@ def _is_eligible(record): class Zone(object): log = getLogger('Zone') - def __init__(self, name, sub_zones): + def __init__(self, name, sub_zones, file=None, is_alias=False): if not name[-1] == '.': raise Exception('Invalid zone name {}, missing ending dot' .format(name)) # Force everything to lowercase just to be safe self.name = text_type(name).lower() if name else name self.sub_zones = sub_zones + self.file = text_type(file if file else name).lower() + self.is_alias = is_alias # 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) diff --git a/tests/config/simple-alias-zone.yaml b/tests/config/simple-alias-zone.yaml new file mode 100644 index 0000000..32154d5 --- /dev/null +++ b/tests/config/simple-alias-zone.yaml @@ -0,0 +1,19 @@ +manager: + max_workers: 2 +providers: + in: + class: octodns.provider.yaml.YamlProvider + directory: tests/config + dump: + class: octodns.provider.yaml.YamlProvider + directory: env/YAML_TMP_DIR +zones: + unit.tests.: + sources: + - in + targets: + - dump + + alias.tests.: + alias: unit.tests. + diff --git a/tests/config/unknown-source-zone.yaml b/tests/config/unknown-source-zone.yaml new file mode 100644 index 0000000..313853e --- /dev/null +++ b/tests/config/unknown-source-zone.yaml @@ -0,0 +1,13 @@ +manager: + max_workers: 2 +providers: + in: + class: octodns.provider.yaml.YamlProvider + directory: tests/config + dump: + class: octodns.provider.yaml.YamlProvider + directory: env/YAML_TMP_DIR +zones: + unit.tests.: + alias: unit-source.tests. + diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 9956790..1b8752e 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -298,7 +298,8 @@ class TestManager(TestCase): pass # This should be ok, we'll fall back to not passing it - manager._populate_and_plan('unit.tests.', [NoLenient()], []) + manager._populate_and_plan('unit.tests.', None, False, + [NoLenient()], []) class NoZone(SimpleProvider): @@ -307,7 +308,21 @@ class TestManager(TestCase): # This will blow up, we don't fallback for source with self.assertRaises(TypeError): - manager._populate_and_plan('unit.tests.', [NoZone()], []) + manager._populate_and_plan('unit.tests.', None, False, + [NoZone()], []) + + def test_alias_zones(self): + with TemporaryDirectory() as tmpdir: + environ['YAML_TMP_DIR'] = tmpdir.dirname + + Manager(get_config_filename('simple-alias-zone.yaml')) \ + .validate_configs() + + with self.assertRaises(ManagerException) as ctx: + Manager(get_config_filename('unknown-source-zone.yaml')) \ + .validate_configs() + self.assertTrue('Invalid alias zone' in + text_type(ctx.exception)) class TestMainThreadExecutor(TestCase): From 06c18f406317918536bd389738af68dbc4bb11ac Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Wed, 21 Oct 2020 19:11:02 +0200 Subject: [PATCH 04/26] Add zones aliases support to octodns-report command --- octodns/cmds/report.py | 2 +- octodns/manager.py | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/octodns/cmds/report.py b/octodns/cmds/report.py index 3a26052..1ccad33 100755 --- a/octodns/cmds/report.py +++ b/octodns/cmds/report.py @@ -56,7 +56,7 @@ def main(): except KeyError as e: raise Exception('Unknown source: {}'.format(e.args[0])) - zone = Zone(args.zone, manager.configured_sub_zones(args.zone)) + zone = manager.get_zone(args.zone) for source in sources: source.populate(zone) diff --git a/octodns/manager.py b/octodns/manager.py index 613be29..d131e78 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -467,3 +467,18 @@ class Manager(object): for source in sources: if isinstance(source, YamlProvider): source.populate(zone) + + def get_zone(self, zone_name): + if not zone_name[-1] == '.': + raise Exception('Invalid zone name {}, missing ending dot' + .format(zone_name)) + + for name, config in self.config['zones'].items(): + if name == zone_name: + file = config.get('file', False) + is_alias = config.get('is_alias', False) + + return Zone(name, self.configured_sub_zones(name), + file, is_alias) + + raise ManagerException('Unkown zone name {}'.format(zone_name)) From 12c3aa64a83a01f9f18a815db9a8694be971f992 Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Wed, 21 Oct 2020 19:11:25 +0200 Subject: [PATCH 05/26] Add zones aliases support to octodns-compare command --- octodns/manager.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index d131e78..6fd4239 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -399,12 +399,11 @@ class Manager(object): except KeyError as e: raise ManagerException('Unknown source: {}'.format(e.args[0])) - sub_zones = self.configured_sub_zones(zone) - za = Zone(zone, sub_zones) + za = self.get_zone(zone) for source in a: source.populate(za) - zb = Zone(zone, sub_zones) + zb = self.get_zone(zone) for source in b: source.populate(zb) From 94a8b67a3be7885c5a75ae22be455c63ba560562 Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Wed, 21 Oct 2020 19:18:27 +0200 Subject: [PATCH 06/26] Fixes linting errors --- octodns/cmds/report.py | 1 - octodns/manager.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/octodns/cmds/report.py b/octodns/cmds/report.py index 1ccad33..d0b82c0 100755 --- a/octodns/cmds/report.py +++ b/octodns/cmds/report.py @@ -17,7 +17,6 @@ from six import text_type from octodns.cmds.args import ArgumentParser from octodns.manager import Manager -from octodns.zone import Zone class AsyncResolver(Resolver): diff --git a/octodns/manager.py b/octodns/manager.py index 6fd4239..7015ad8 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -478,6 +478,6 @@ class Manager(object): is_alias = config.get('is_alias', False) return Zone(name, self.configured_sub_zones(name), - file, is_alias) + file, is_alias) raise ManagerException('Unkown zone name {}'.format(zone_name)) From 1f60a6af5e650fcdba44435038dd8d132600b2dc Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Wed, 21 Oct 2020 19:24:49 +0200 Subject: [PATCH 07/26] Fixes typo in manager.get_zone() --- octodns/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/octodns/manager.py b/octodns/manager.py index 7015ad8..eab7ac4 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -480,4 +480,4 @@ class Manager(object): return Zone(name, self.configured_sub_zones(name), file, is_alias) - raise ManagerException('Unkown zone name {}'.format(zone_name)) + raise ManagerException('Unknown zone name {}'.format(zone_name)) From 897a033443c8c66a32209a610be613173b361b06 Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Wed, 21 Oct 2020 20:02:12 +0200 Subject: [PATCH 08/26] Add tests for Manager.get_zones() --- octodns/manager.py | 4 ++-- tests/test_octodns_manager.py | 16 ++++++++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index eab7ac4..eff3a74 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -469,8 +469,8 @@ class Manager(object): def get_zone(self, zone_name): if not zone_name[-1] == '.': - raise Exception('Invalid zone name {}, missing ending dot' - .format(zone_name)) + raise ManagerException('Invalid zone name {}, missing ending dot' + .format(zone_name)) for name, config in self.config['zones'].items(): if name == zone_name: diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 1b8752e..b493540 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -286,6 +286,18 @@ class TestManager(TestCase): .validate_configs() self.assertTrue('unknown source' in text_type(ctx.exception)) + def test_get_zone(self): + Manager(get_config_filename('simple.yaml')).get_zone('unit.tests.') + + with self.assertRaises(ManagerException) as ctx: + Manager(get_config_filename('simple.yaml')).get_zone('unit.tests') + self.assertTrue('missing ending dot' in text_type(ctx.exception)) + + with self.assertRaises(ManagerException) as ctx: + Manager(get_config_filename('simple.yaml')) \ + .get_zone('unknown-zone.tests.') + self.assertTrue('Unknown zone name' in text_type(ctx.exception)) + def test_populate_lenient_fallback(self): with TemporaryDirectory() as tmpdir: environ['YAML_TMP_DIR'] = tmpdir.dirname @@ -321,8 +333,8 @@ class TestManager(TestCase): with self.assertRaises(ManagerException) as ctx: Manager(get_config_filename('unknown-source-zone.yaml')) \ .validate_configs() - self.assertTrue('Invalid alias zone' in - text_type(ctx.exception)) + self.assertTrue('Invalid alias zone' in + text_type(ctx.exception)) class TestMainThreadExecutor(TestCase): From 3acea0d89d88a7f921429ab77e3efe9cd3fde392 Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Sat, 31 Oct 2020 01:09:37 +0100 Subject: [PATCH 09/26] Handle multiples sources on aliased zones --- octodns/manager.py | 143 +++++++++++++++++++++------------------ octodns/provider/yaml.py | 5 +- octodns/zone.py | 4 +- 3 files changed, 82 insertions(+), 70 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index eff3a74..0517c6e 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -121,23 +121,6 @@ class Manager(object): raise ManagerException('Incorrect provider config for {}' .format(provider_name)) - for zone_name, zone_config in self.config['zones'].copy().items(): - if 'alias' in zone_config: - source_zone = zone_config['alias'] - # Check that the source zone is defined. - if source_zone not in self.config['zones']: - self.log.exception('Invalid alias zone') - raise ManagerException('Invalid alias zone {}: ' - 'source zone {} does not exist' - .format(zone_name, source_zone)) - self.config['zones'][zone_name] = \ - self.config['zones'][source_zone] - self.config['zones'][zone_name]['is_alias'] = True - self.config['zones'][zone_name]['file'] = source_zone - else: - self.config['zones'][zone_name]['is_alias'] = False - self.config['zones'][zone_name]['file'] = zone_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]): @@ -239,23 +222,32 @@ class Manager(object): self.log.debug('configured_sub_zones: subs=%s', sub_zone_names) return set(sub_zone_names) - def _populate_and_plan(self, zone_name, file, is_alias, sources, targets, + def _populate_and_plan(self, zone_name, sources, targets, desired=None, lenient=False): - self.log.debug('sync: populating, zone=%s, file=%s, is_alias=%s, ' - 'lenient=%s', zone_name, file, is_alias, lenient) + self.log.debug('sync: populating, zone=%s, lenient=%s', + zone_name, lenient) zone = Zone(zone_name, - sub_zones=self.configured_sub_zones(zone_name), file=file, - is_alias=is_alias) - for source in sources: - try: - source.populate(zone, lenient=lenient) - except TypeError as e: - if "keyword argument 'lenient'" not in text_type(e): - raise - self.log.warn(': provider %s does not accept lenient param', - source.__class__.__name__) - source.populate(zone) + sub_zones=self.configured_sub_zones(zone_name)) + + if not desired: + for source in sources: + try: + source.populate(zone, lenient=lenient) + except TypeError as e: + if "keyword argument 'lenient'" not in text_type(e): + raise + self.log.warn(': provider %s does not accept lenient ' + 'param', source.__class__.__name__) + source.populate(zone) + + else: + for _, records in desired._records.items(): + for record in records: + d = record.data + d['type'] = record._type + r = Record.new(zone, record.name, d, source=record.source) + zone.add_record(r, lenient=lenient) self.log.debug('sync: planning, zone=%s', zone_name) plans = [] @@ -284,11 +276,22 @@ class Manager(object): if eligible_zones: zones = [z for z in zones if z[0] in eligible_zones] + aliased_zones = {} futures = [] for zone_name, config in zones: self.log.info('sync: zone=%s', zone_name) - file = config.get('file') - is_alias = config.get('is_alias') + if 'alias' in config: + source_zone = config['alias'] + # Check that the source zone is defined. + if source_zone not in self.config['zones']: + self.log.exception('Invalid alias zone') + raise ManagerException('Invalid alias zone {}: ' + 'source zone {} does not exist' + .format(zone_name, source_zone)) + + aliased_zones[zone_name] = source_zone + continue + lenient = config.get('lenient', False) try: sources = config['sources'] @@ -345,14 +348,32 @@ class Manager(object): .format(zone_name, target)) futures.append(self._executor.submit(self._populate_and_plan, - zone_name, file, is_alias, - sources, targets, - lenient=lenient)) + zone_name, sources, + targets, lenient=lenient)) # Wait on all results and unpack/flatten them in to a list of target & # plan pairs. plans = [p for f in futures for p in f.result()] + # Populate aliases zones. + futures = [] + for zone_name, zone_source in aliased_zones.items(): + plan = [p for t, p in plans if p.desired.name == zone_source] + if not plan: + continue + + source_config = self.config['zones'][zone_source] + futures.append(self._executor.submit( + self._populate_and_plan, + zone_name, + [self.providers[s] for s in source_config['sources']], + [self.providers[t] for t in source_config['targets']], + desired=plan[0].desired, + lenient=lenient + )) + + plans += [p for f in futures for p in f.result()] + # Best effort sort plans children first so that we create/update # children zones before parents which should allow us to more safely # extract things into sub-zones. Combining a child back into a parent @@ -440,32 +461,30 @@ class Manager(object): def validate_configs(self): for zone_name, config in self.config['zones'].items(): - file = config.get('file', False) - is_alias = config.get('is_alias', False) - zone = Zone(zone_name, self.configured_sub_zones(zone_name), - file, is_alias) + zone = Zone(zone_name, self.configured_sub_zones(zone_name)) - try: - sources = config['sources'] - except KeyError: - raise ManagerException('Zone {} is missing sources' - .format(zone_name)) + if not config.get('alias'): + try: + sources = config['sources'] + except KeyError: + raise ManagerException('Zone {} is missing sources' + .format(zone_name)) - try: - # rather than using a list comprehension, we break this loop - # out so that the `except` block below can reference the - # `source` - collected = [] - for source in sources: - collected.append(self.providers[source]) - sources = collected - except KeyError: - raise ManagerException('Zone {}, unknown source: {}' - .format(zone_name, source)) + try: + # rather than using a list comprehension, we break this + # loop out so that the `except` block below can reference + # the `source` + collected = [] + for source in sources: + collected.append(self.providers[source]) + sources = collected + except KeyError: + raise ManagerException('Zone {}, unknown source: {}' + .format(zone_name, source)) - for source in sources: - if isinstance(source, YamlProvider): - source.populate(zone) + for source in sources: + if isinstance(source, YamlProvider): + source.populate(zone) def get_zone(self, zone_name): if not zone_name[-1] == '.': @@ -474,10 +493,6 @@ class Manager(object): for name, config in self.config['zones'].items(): if name == zone_name: - file = config.get('file', False) - is_alias = config.get('is_alias', False) - - return Zone(name, self.configured_sub_zones(name), - file, is_alias) + return Zone(name, self.configured_sub_zones(name)) raise ManagerException('Unknown zone name {}'.format(zone_name)) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index f1b921b..55a1632 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -139,8 +139,7 @@ class YamlProvider(BaseProvider): filename) def populate(self, zone, target=False, lenient=False): - self.log.debug('populate: name=%s, file=%s, is_alias:%s, target=%s, ' - 'lenient=%s', zone.name, zone.file, zone.is_alias, + self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, target, lenient) if target: @@ -149,7 +148,7 @@ class YamlProvider(BaseProvider): return False before = len(zone.records) - filename = join(self.directory, '{}yaml'.format(zone.file)) + filename = join(self.directory, '{}yaml'.format(zone.name)) self._populate_from_file(filename, zone, lenient) self.log.info('populate: found %s records, exists=False', diff --git a/octodns/zone.py b/octodns/zone.py index 0a78f72..5f099ac 100644 --- a/octodns/zone.py +++ b/octodns/zone.py @@ -35,15 +35,13 @@ def _is_eligible(record): class Zone(object): log = getLogger('Zone') - def __init__(self, name, sub_zones, file=None, is_alias=False): + def __init__(self, name, sub_zones): if not name[-1] == '.': raise Exception('Invalid zone name {}, missing ending dot' .format(name)) # Force everything to lowercase just to be safe self.name = text_type(name).lower() if name else name self.sub_zones = sub_zones - self.file = text_type(file if file else name).lower() - self.is_alias = is_alias # 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) From 0b3a99bb8ce593f16a7849b3eb5e8b8e361670c4 Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Sat, 31 Oct 2020 09:38:35 +0100 Subject: [PATCH 10/26] Implement Record.copy() function Flip if in _populate_and_plan() --- octodns/manager.py | 15 ++++++--------- octodns/record/__init__.py | 9 +++++++++ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index 0517c6e..d312fad 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -230,7 +230,12 @@ class Manager(object): zone = Zone(zone_name, sub_zones=self.configured_sub_zones(zone_name)) - if not desired: + if desired: + for _, records in desired._records.items(): + for record in records: + zone.add_record(record.copy(zone=zone), lenient=lenient) + + else: for source in sources: try: source.populate(zone, lenient=lenient) @@ -241,14 +246,6 @@ class Manager(object): 'param', source.__class__.__name__) source.populate(zone) - else: - for _, records in desired._records.items(): - for record in records: - d = record.data - d['type'] = record._type - r = Record.new(zone, record.name, d, source=record.source) - zone.add_record(r, lenient=lenient) - self.log.debug('sync: planning, zone=%s', zone_name) plans = [] diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index 08ec2ee..6c4e79f 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -151,6 +151,7 @@ class Record(EqualityTupleMixin): # force everything lower-case just to be safe self.name = text_type(name).lower() if name else name self.source = source + self._raw_data = data self.ttl = int(data['ttl']) self._octodns = data.get('octodns', {}) @@ -219,6 +220,14 @@ class Record(EqualityTupleMixin): if self.ttl != other.ttl: return Update(self, other) + def copy(self, zone=None): + return Record( + zone if zone else self.zone, + self.name, + self._raw_data, + self.source + ) + # NOTE: we're using __hash__ and ordering methods that consider Records # equivalent if they have the same name & _type. Values are ignored. This # is useful when computing diffs/changes. From 8679bb4899b0d622a8f304235a13f9d8f8fc5711 Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Sat, 31 Oct 2020 09:41:27 +0100 Subject: [PATCH 11/26] Remove sources argument when calling _populate_and_plan() for an alias zone --- octodns/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/octodns/manager.py b/octodns/manager.py index d312fad..45a16eb 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -363,7 +363,7 @@ class Manager(object): futures.append(self._executor.submit( self._populate_and_plan, zone_name, - [self.providers[s] for s in source_config['sources']], + [], [self.providers[t] for t in source_config['targets']], desired=plan[0].desired, lenient=lenient From 6f01a543df9b470b54575e1a00cbc5aa7c83a7a9 Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Sat, 31 Oct 2020 09:43:23 +0100 Subject: [PATCH 12/26] Implement configuration validation for alias zones --- octodns/manager.py | 46 +++++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index 45a16eb..a807a5c 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -460,28 +460,36 @@ class Manager(object): for zone_name, config in self.config['zones'].items(): zone = Zone(zone_name, self.configured_sub_zones(zone_name)) - if not config.get('alias'): - try: - sources = config['sources'] - except KeyError: - raise ManagerException('Zone {} is missing sources' - .format(zone_name)) + source_zone = config.get('alias') + if source_zone: + if source_zone not in self.config['zones']: + self.log.exception('Invalid alias zone') + raise ManagerException('Invalid alias zone {}: ' + 'source zone {} does not exist' + .format(zone_name, source_zone)) + continue - try: - # rather than using a list comprehension, we break this - # loop out so that the `except` block below can reference - # the `source` - collected = [] - for source in sources: - collected.append(self.providers[source]) - sources = collected - except KeyError: - raise ManagerException('Zone {}, unknown source: {}' - .format(zone_name, source)) + try: + sources = config['sources'] + except KeyError: + raise ManagerException('Zone {} is missing sources' + .format(zone_name)) + try: + # rather than using a list comprehension, we break this + # loop out so that the `except` block below can reference + # the `source` + collected = [] for source in sources: - if isinstance(source, YamlProvider): - source.populate(zone) + collected.append(self.providers[source]) + sources = collected + except KeyError: + raise ManagerException('Zone {}, unknown source: {}' + .format(zone_name, source)) + + for source in sources: + if isinstance(source, YamlProvider): + source.populate(zone) def get_zone(self, zone_name): if not zone_name[-1] == '.': From 4fb102e4be973609f6e356387072a30f9b2cff36 Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Sat, 31 Oct 2020 09:44:06 +0100 Subject: [PATCH 13/26] Fixes tests related to _populate_and_plan() --- tests/test_octodns_manager.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index cc39c87..b99e460 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -310,8 +310,7 @@ class TestManager(TestCase): pass # This should be ok, we'll fall back to not passing it - manager._populate_and_plan('unit.tests.', None, False, - [NoLenient()], []) + manager._populate_and_plan('unit.tests.', [NoLenient()], []) class NoZone(SimpleProvider): @@ -320,8 +319,7 @@ class TestManager(TestCase): # This will blow up, we don't fallback for source with self.assertRaises(TypeError): - manager._populate_and_plan('unit.tests.', None, False, - [NoZone()], []) + manager._populate_and_plan('unit.tests.', [NoZone()], []) def test_alias_zones(self): with TemporaryDirectory() as tmpdir: From a1e62281f6cd7cdf265930520b05ab7149f445df Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Sat, 31 Oct 2020 10:54:17 +0100 Subject: [PATCH 14/26] Fixes record copy when record is a child class of Record and as no record type specified in its data --- octodns/record/__init__.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index 6c4e79f..ae9ccb1 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -221,7 +221,11 @@ class Record(EqualityTupleMixin): return Update(self, other) def copy(self, zone=None): - return Record( + _type = getattr(self, '_type') + if not self._raw_data.get('type'): + self._raw_data['type'] = _type + + return Record.new( zone if zone else self.zone, self.name, self._raw_data, From a2aa98377d512d1f36880bbe0e2778a78436015b Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Sat, 31 Oct 2020 10:57:14 +0100 Subject: [PATCH 15/26] Add tests for Record.copy() --- tests/test_octodns_record.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 524f8f2..a286efb 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -813,6 +813,39 @@ class TestRecord(TestCase): }) self.assertTrue('Unknown record type' in text_type(ctx.exception)) + def test_record_copy(self): + a = Record.new(self.zone, 'a', { + 'ttl': 44, + 'type': 'A', + 'value': '1.2.3.4', + }) + + # Identical copy. + b = a.copy() + self.assertIsInstance(b, ARecord) + self.assertEquals('unit.tests.', b.zone.name) + self.assertEquals('a', b.name) + self.assertEquals('A', b._type) + self.assertEquals(['1.2.3.4'], b.values) + + # Copy with another zone object. + c_zone = Zone('other.tests.', []) + c = a.copy(c_zone) + self.assertIsInstance(c, ARecord) + self.assertEquals('other.tests.', c.zone.name) + self.assertEquals('a', c.name) + self.assertEquals('A', c._type) + self.assertEquals(['1.2.3.4'], c.values) + + # Record with no record type specified in data. + d_data = { + 'ttl': 600, + 'values': ['just a test'] + } + d = TxtRecord(self.zone, 'txt', d_data) + d.copy() + self.assertEquals('TXT', d._type) + def test_change(self): existing = Record.new(self.zone, 'txt', { 'ttl': 44, From b0da090723a54821d989939d2cede1aac353feff Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Sat, 31 Oct 2020 14:09:54 +0100 Subject: [PATCH 16/26] Add test for alias zones --- tests/config/unknown-source-zone.yaml | 9 +++++++-- tests/test_octodns_manager.py | 15 +++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/tests/config/unknown-source-zone.yaml b/tests/config/unknown-source-zone.yaml index 313853e..a3940ff 100644 --- a/tests/config/unknown-source-zone.yaml +++ b/tests/config/unknown-source-zone.yaml @@ -9,5 +9,10 @@ providers: directory: env/YAML_TMP_DIR zones: unit.tests.: - alias: unit-source.tests. - + sources: + - in + targets: + - dump + + alias.tests.: + alias: does-not-exists.tests. diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index b99e460..084ad08 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -167,6 +167,21 @@ class TestManager(TestCase): .sync(eligible_targets=['foo']) self.assertEquals(0, tc) + def test_aliases(self): + with TemporaryDirectory() as tmpdir: + environ['YAML_TMP_DIR'] = tmpdir.dirname + # Only allow a target that doesn't exist + tc = Manager(get_config_filename('simple-alias-zone.yaml')) \ + .sync() + self.assertEquals(0, tc) + + with self.assertRaises(ManagerException) as ctx: + tc = Manager(get_config_filename('unknown-source-zone.yaml')) \ + .sync() + self.assertEquals('Invalid alias zone alias.tests.: source zone ' + 'does-not-exists.tests. does not exist', + text_type(ctx.exception)) + def test_compare(self): with TemporaryDirectory() as tmpdir: environ['YAML_TMP_DIR'] = tmpdir.dirname From a6d8848fad8d773f5f281376b235aa90d82737ae Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Sat, 31 Oct 2020 14:19:43 +0100 Subject: [PATCH 17/26] Fixes linting issues --- tests/test_octodns_manager.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 084ad08..c9693c1 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -172,14 +172,14 @@ class TestManager(TestCase): environ['YAML_TMP_DIR'] = tmpdir.dirname # Only allow a target that doesn't exist tc = Manager(get_config_filename('simple-alias-zone.yaml')) \ - .sync() + .sync() self.assertEquals(0, tc) with self.assertRaises(ManagerException) as ctx: tc = Manager(get_config_filename('unknown-source-zone.yaml')) \ - .sync() + .sync() self.assertEquals('Invalid alias zone alias.tests.: source zone ' - 'does-not-exists.tests. does not exist', + 'does-not-exists.tests. does not exist', text_type(ctx.exception)) def test_compare(self): From 6b568f5c9dd06ad70f50bdefbf48122c3030f5ac Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Sat, 31 Oct 2020 19:07:34 +0100 Subject: [PATCH 18/26] Compare alias zones content with the one of its parent zone, even if there is no changes in the parent zone --- octodns/manager.py | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index a807a5c..4680640 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -261,7 +261,8 @@ class Manager(object): if plan: plans.append((target, plan)) - return plans + # Return the zone as it's the desired state + return plans, zone def sync(self, eligible_zones=[], eligible_sources=[], eligible_targets=[], dry_run=True, force=False): @@ -281,7 +282,8 @@ class Manager(object): source_zone = config['alias'] # Check that the source zone is defined. if source_zone not in self.config['zones']: - self.log.exception('Invalid alias zone') + self.log.error('Invalid alias zone {}, target {} does ' + 'not exist'.format(zone_name, source_zone)) raise ManagerException('Invalid alias zone {}: ' 'source zone {} does not exist' .format(zone_name, source_zone)) @@ -348,28 +350,32 @@ class Manager(object): zone_name, sources, targets, lenient=lenient)) - # Wait on all results and unpack/flatten them in to a list of target & - # plan pairs. - plans = [p for f in futures for p in f.result()] + # Wait on all results and unpack/flatten the plans and store the + # desired states in case we need them below + plans = [] + desired = {} + for future in futures: + ps, d = future.result() + desired[d.name] = d + for plan in ps: + plans.append(plan) # Populate aliases zones. futures = [] for zone_name, zone_source in aliased_zones.items(): - plan = [p for t, p in plans if p.desired.name == zone_source] - if not plan: - continue - source_config = self.config['zones'][zone_source] futures.append(self._executor.submit( self._populate_and_plan, zone_name, [], [self.providers[t] for t in source_config['targets']], - desired=plan[0].desired, + desired=desired[zone_source], lenient=lenient )) - plans += [p for f in futures for p in f.result()] + # Wait on results and unpack/flatten the plans, ignore the desired here + # as these are aliased zones + plans += [p for f in futures for p in f.result()[0]] # Best effort sort plans children first so that we create/update # children zones before parents which should allow us to more safely From 038ae422841fdc1659d4bb8d1579ff30bea4da56 Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Sat, 31 Oct 2020 20:16:26 +0100 Subject: [PATCH 19/26] Add comments and fixes some tests --- tests/test_octodns_manager.py | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index c9693c1..3106b07 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -170,11 +170,12 @@ class TestManager(TestCase): def test_aliases(self): with TemporaryDirectory() as tmpdir: environ['YAML_TMP_DIR'] = tmpdir.dirname - # Only allow a target that doesn't exist + # Alias zones with a valid target. tc = Manager(get_config_filename('simple-alias-zone.yaml')) \ .sync() self.assertEquals(0, tc) + # Alias zone with an invalid target. with self.assertRaises(ManagerException) as ctx: tc = Manager(get_config_filename('unknown-source-zone.yaml')) \ .sync() @@ -301,6 +302,17 @@ class TestManager(TestCase): .validate_configs() self.assertTrue('unknown source' in text_type(ctx.exception)) + # Alias zone using an invalid source zone. + with self.assertRaises(ManagerException) as ctx: + Manager(get_config_filename('unknown-source-zone.yaml')) \ + .validate_configs() + self.assertTrue('Invalid alias zone' in + text_type(ctx.exception)) + + # Valid config file using an alias zone. + Manager(get_config_filename('simple-alias-zone.yaml')) \ + .validate_configs() + def test_get_zone(self): Manager(get_config_filename('simple.yaml')).get_zone('unit.tests.') @@ -336,20 +348,6 @@ class TestManager(TestCase): with self.assertRaises(TypeError): manager._populate_and_plan('unit.tests.', [NoZone()], []) - def test_alias_zones(self): - with TemporaryDirectory() as tmpdir: - environ['YAML_TMP_DIR'] = tmpdir.dirname - - Manager(get_config_filename('simple-alias-zone.yaml')) \ - .validate_configs() - - with self.assertRaises(ManagerException) as ctx: - Manager(get_config_filename('unknown-source-zone.yaml')) \ - .validate_configs() - self.assertTrue('Invalid alias zone' in - text_type(ctx.exception)) - - class TestMainThreadExecutor(TestCase): def test_success(self): From 9a4812223e6c9ef3ca1cf6feb2b6c0ea7363a2ef Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Sat, 31 Oct 2020 20:19:09 +0100 Subject: [PATCH 20/26] Add missing empty line --- tests/test_octodns_manager.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 3106b07..6affa17 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -348,6 +348,7 @@ class TestManager(TestCase): with self.assertRaises(TypeError): manager._populate_and_plan('unit.tests.', [NoZone()], []) + class TestMainThreadExecutor(TestCase): def test_success(self): From fbfa46fbcc85df9af1ac2c1565289d13d7556ec7 Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Sat, 31 Oct 2020 22:51:09 +0100 Subject: [PATCH 21/26] Add documentation for zones aliases --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index 23ac0e8..96cd5bb 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,9 @@ zones: targets: - dyn - route53 + + example.net: + alias: example.com. ``` `class` is a special key that tells OctoDNS what python class should be loaded. Any other keys will be passed as configuration values to that provider. In general any sensitive or frequently rotated values should come from environmental variables. When OctoDNS sees a value that starts with `env/` it will look for that value in the process's environment and pass the result along. @@ -87,6 +90,8 @@ Further information can be found in the `docstring` of each source and provider The `max_workers` key in the `manager` section of the config enables threading to parallelize the planning portion of the sync. +In this example, `example.net` is an alias of zone `example.com`, which means they share the same sources and targets. They will therefore have identical records. + Now that we have something to tell OctoDNS about our providers & zones we need to tell it about or records. We'll keep it simple for now and just create a single `A` record at the top-level of the domain. `config/example.com.yaml` From d3be3be7342bc35b221c70e85fc289b854e869db Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Sat, 31 Oct 2020 23:26:27 +0100 Subject: [PATCH 22/26] Fix coverage issue --- octodns/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/octodns/manager.py b/octodns/manager.py index 4680640..8c3f182 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -473,7 +473,7 @@ class Manager(object): raise ManagerException('Invalid alias zone {}: ' 'source zone {} does not exist' .format(zone_name, source_zone)) - continue + continue # pragma: no cover, see Python bug #2506. try: sources = config['sources'] From e524d69f631a16b5a2327735a0dffb265be00c74 Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Sat, 31 Oct 2020 23:32:20 +0100 Subject: [PATCH 23/26] Fixes linting issue --- octodns/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/octodns/manager.py b/octodns/manager.py index 8c3f182..ff9e491 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -473,7 +473,7 @@ class Manager(object): raise ManagerException('Invalid alias zone {}: ' 'source zone {} does not exist' .format(zone_name, source_zone)) - continue # pragma: no cover, see Python bug #2506. + continue # pragma: no cover, see Python bug #2506. try: sources = config['sources'] From 95a71a268ebf0f29dd109d139e5e31cbdfbe97fc Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Sat, 31 Oct 2020 23:51:04 +0100 Subject: [PATCH 24/26] Apply workaround for python bug #2506 witout using "pragma: no cover" comment --- octodns/manager.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/octodns/manager.py b/octodns/manager.py index ff9e491..cfc9735 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -473,7 +473,10 @@ class Manager(object): raise ManagerException('Invalid alias zone {}: ' 'source zone {} does not exist' .format(zone_name, source_zone)) - continue # pragma: no cover, see Python bug #2506. + # this is just here to satisfy coverage, see + # https://github.com/nedbat/coveragepy/issues/198 + source_zone = source_zone + continue try: sources = config['sources'] From 2d4855508c89ea70933f776a2e5b967bb17f8249 Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Sun, 1 Nov 2020 23:58:40 +0100 Subject: [PATCH 25/26] Check that an alias zone source is not an alias zone --- octodns/manager.py | 16 ++++++++++++++++ tests/config/alias-zone-loop.yaml | 21 +++++++++++++++++++++ tests/test_octodns_manager.py | 17 ++++++++++++++++- 3 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 tests/config/alias-zone-loop.yaml diff --git a/octodns/manager.py b/octodns/manager.py index cfc9735..5f4af55 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -280,6 +280,7 @@ class Manager(object): self.log.info('sync: zone=%s', zone_name) if 'alias' in config: source_zone = config['alias'] + # Check that the source zone is defined. if source_zone not in self.config['zones']: self.log.error('Invalid alias zone {}, target {} does ' @@ -288,6 +289,14 @@ class Manager(object): 'source zone {} does not exist' .format(zone_name, source_zone)) + # Check that the source zone is not an alias zone itself. + if 'alias' in self.config['zones'][source_zone]: + self.log.error('Invalid alias zone {}, target {} is an ' + 'alias zone'.format(zone_name, source_zone)) + raise ManagerException('Invalid alias zone {}: source ' + 'zone {} is an alias zone' + .format(zone_name, source_zone)) + aliased_zones[zone_name] = source_zone continue @@ -473,6 +482,13 @@ class Manager(object): raise ManagerException('Invalid alias zone {}: ' 'source zone {} does not exist' .format(zone_name, source_zone)) + + if 'alias' in self.config['zones'][source_zone]: + self.log.exception('Invalid alias zone') + raise ManagerException('Invalid alias zone {}: ' + 'source zone {} is an alias zone' + .format(zone_name, source_zone)) + # this is just here to satisfy coverage, see # https://github.com/nedbat/coveragepy/issues/198 source_zone = source_zone diff --git a/tests/config/alias-zone-loop.yaml b/tests/config/alias-zone-loop.yaml new file mode 100644 index 0000000..df8b53f --- /dev/null +++ b/tests/config/alias-zone-loop.yaml @@ -0,0 +1,21 @@ +manager: + max_workers: 2 +providers: + in: + class: octodns.provider.yaml.YamlProvider + directory: tests/config + dump: + class: octodns.provider.yaml.YamlProvider + directory: env/YAML_TMP_DIR +zones: + unit.tests.: + sources: + - in + targets: + - dump + + alias.tests.: + alias: unit.tests. + + alias-loop.tests.: + alias: alias.tests. diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 6affa17..dc047e8 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -183,6 +183,14 @@ class TestManager(TestCase): 'does-not-exists.tests. does not exist', text_type(ctx.exception)) + # Alias zone that points to another alias zone. + with self.assertRaises(ManagerException) as ctx: + tc = Manager(get_config_filename('alias-zone-loop.yaml')) \ + .sync() + self.assertEquals('Invalid alias zone alias-loop.tests.: source ' + 'zone alias.tests. is an alias zone', + text_type(ctx.exception)) + def test_compare(self): with TemporaryDirectory() as tmpdir: environ['YAML_TMP_DIR'] = tmpdir.dirname @@ -306,7 +314,14 @@ class TestManager(TestCase): with self.assertRaises(ManagerException) as ctx: Manager(get_config_filename('unknown-source-zone.yaml')) \ .validate_configs() - self.assertTrue('Invalid alias zone' in + self.assertTrue('does not exist' in + text_type(ctx.exception)) + + # Alias zone that points to another alias zone. + with self.assertRaises(ManagerException) as ctx: + Manager(get_config_filename('alias-zone-loop.yaml')) \ + .validate_configs() + self.assertTrue('is an alias zone' in text_type(ctx.exception)) # Valid config file using an alias zone. From bb7a1a43b7e96cb03e712a3c7302292d5ccd72ce Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Mon, 2 Nov 2020 18:42:03 +0100 Subject: [PATCH 26/26] Implement suggested changes --- octodns/manager.py | 2 ++ octodns/record/__init__.py | 11 +++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index 5f4af55..fc05810 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -231,6 +231,8 @@ class Manager(object): sub_zones=self.configured_sub_zones(zone_name)) if desired: + # This is an alias zone, rather than populate it we'll copy the + # records over from `desired`. for _, records in desired._records.items(): for record in records: zone.add_record(record.copy(zone=zone), lenient=lenient) diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index ae9ccb1..e065620 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -151,7 +151,6 @@ class Record(EqualityTupleMixin): # force everything lower-case just to be safe self.name = text_type(name).lower() if name else name self.source = source - self._raw_data = data self.ttl = int(data['ttl']) self._octodns = data.get('octodns', {}) @@ -221,15 +220,15 @@ class Record(EqualityTupleMixin): return Update(self, other) def copy(self, zone=None): - _type = getattr(self, '_type') - if not self._raw_data.get('type'): - self._raw_data['type'] = _type + data = self.data + data['type'] = self._type return Record.new( zone if zone else self.zone, self.name, - self._raw_data, - self.source + data, + self.source, + lenient=True ) # NOTE: we're using __hash__ and ordering methods that consider Records