From b926d78c5c182a13df03566ea9327dffdc9fb29f Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Mon, 3 Aug 2020 00:47:22 +0200 Subject: [PATCH 01/45] 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 aad1467d5ace2320aa1704185dfe49d77fd657e8 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2020 08:06:50 +0000 Subject: [PATCH 02/45] Bump botocore from 1.17.52 to 1.18.9 Bumps [botocore](https://github.com/boto/botocore) from 1.17.52 to 1.18.9. - [Release notes](https://github.com/boto/botocore/releases) - [Changelog](https://github.com/boto/botocore/blob/develop/CHANGELOG.rst) - [Commits](https://github.com/boto/botocore/compare/1.17.52...1.18.9) Signed-off-by: dependabot-preview[bot] --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 24f57ff..e359ffc 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,7 +2,7 @@ PyYaml==5.3.1 azure-common==1.1.25 azure-mgmt-dns==3.0.0 boto3==1.14.52 -botocore==1.17.52 +botocore==1.18.9 dnspython==1.16.0 docutils==0.16 dyn==1.8.1 From 832e481ea4ecd7dfe6d01c0421a327656194ef3e Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2020 12:55:18 +0000 Subject: [PATCH 03/45] Bump boto3 from 1.14.52 to 1.15.9 Bumps [boto3](https://github.com/boto/boto3) from 1.14.52 to 1.15.9. - [Release notes](https://github.com/boto/boto3/releases) - [Changelog](https://github.com/boto/boto3/blob/develop/CHANGELOG.rst) - [Commits](https://github.com/boto/boto3/compare/1.14.52...1.15.9) Signed-off-by: dependabot-preview[bot] --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index e359ffc..bc9a019 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,7 @@ PyYaml==5.3.1 azure-common==1.1.25 azure-mgmt-dns==3.0.0 -boto3==1.14.52 +boto3==1.15.9 botocore==1.18.9 dnspython==1.16.0 docutils==0.16 From 50f739495d10310cfb6b8425eda1a26b2b94b40c Mon Sep 17 00:00:00 2001 From: ftm-qsc <54101720+ftm-qsc@users.noreply.github.com> Date: Wed, 14 Oct 2020 20:45:49 +0200 Subject: [PATCH 04/45] docs: fixed small typo in geo_records.md Did you mean 'strongly'? --- docs/geo_records.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/geo_records.md b/docs/geo_records.md index ba99260..3777564 100644 --- a/docs/geo_records.md +++ b/docs/geo_records.md @@ -1,6 +1,6 @@ ## Geo Record Support -Note: Geo DNS records are still supported for the time being, but it is still strongy encouraged that you look at [Dynamic Records](/docs/dynamic_records.md) instead as they are a superset of functionality. +Note: Geo DNS records are still supported for the time being, but it is still strongly encouraged that you look at [Dynamic Records](/docs/dynamic_records.md) instead as they are a superset of functionality. GeoDNS is currently supported for `A` and `AAAA` records on the Dyn (via Traffic Directors) and Route53 providers. Records with geo information pushed to providers without support for them will be managed as non-geo records using the base values. From 7bf0b31367e51e99598a914e7c97cbf831e617eb Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Tue, 20 Oct 2020 19:54:35 +0200 Subject: [PATCH 05/45] 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 06/45] 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 07/45] 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 08/45] 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 09/45] 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 10/45] 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 11/45] 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 fd136b42d1583a1376103967af644ed9a9ec627e Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Sun, 25 Oct 2020 01:08:08 +0200 Subject: [PATCH 12/45] Add support for Gandi LiveDNS --- octodns/provider/gandi.py | 334 +++++++++++++++++++++++++ tests/fixtures/gandi-default-zone.json | 93 +++++++ tests/fixtures/gandi-no-changes.json | 127 ++++++++++ tests/test_octodns_provider_gandi.py | 312 +++++++++++++++++++++++ 4 files changed, 866 insertions(+) create mode 100644 octodns/provider/gandi.py create mode 100644 tests/fixtures/gandi-default-zone.json create mode 100644 tests/fixtures/gandi-no-changes.json create mode 100644 tests/test_octodns_provider_gandi.py diff --git a/octodns/provider/gandi.py b/octodns/provider/gandi.py new file mode 100644 index 0000000..e11e9a3 --- /dev/null +++ b/octodns/provider/gandi.py @@ -0,0 +1,334 @@ +# +# +# + +from __future__ import absolute_import, division, print_function, \ + unicode_literals + +from collections import defaultdict +from requests import Session +import logging + +from ..record import Record +from .base import BaseProvider + + +class GandiClientException(Exception): + pass + + +class GandiClientBadRequest(GandiClientException): + + def __init__(self, r): + super(GandiClientBadRequest, self).__init__(r.text) + + +class GandiClientUnauthorized(GandiClientException): + + def __init__(self, r): + super(GandiClientUnauthorized, self).__init__(r.text) + + +class GandiClientForbidden(GandiClientException): + + def __init__(self, r): + super(GandiClientForbidden, self).__init__(r.text) + + +class GandiClientNotFound(GandiClientException): + + def __init__(self, r): + super(GandiClientNotFound, self).__init__(r.text) + + +class GandiClient(object): + + def __init__(self, token): + session = Session() + session.headers.update({'Authorization': 'Apikey {}'.format(token)}) + self._session = session + self.endpoint = 'https://api.gandi.net/v5' + + def _request(self, method, path, params={}, data=None): + url = '{}{}'.format(self.endpoint, path) + r = self._session.request(method, url, params=params, json=data) + if r.status_code == 400: + raise GandiClientBadRequest(r) + if r.status_code == 401: + raise GandiClientUnauthorized(r) + elif r.status_code == 403: + raise GandiClientForbidden(r) + elif r.status_code == 404: + raise GandiClientNotFound(r) + r.raise_for_status() + return r + + def zone_records(self, zone_name): + records = self._request('GET', '/livedns/domains/{}/records' + .format(zone_name)).json() + + for record in records: + if record['rrset_name'] == '@': + record['rrset_name'] = '' + + return records + + def record_create(self, zone_name, data): + self._request('POST', '/livedns/domains/{}/records'.format(zone_name), + data=data) + + def record_delete(self, zone_name, record_name, record_type): + self._request('DELETE', '/livedns/domains/{}/records/{}/{}' + .format(zone_name, record_name, record_type)) + + +class GandiProvider(BaseProvider): + ''' + Gandi provider using API v5. + + gandi: + class: octodns.provider.gandi.GandiProvider + # Your API key (required) + token: XXXXXXXXXXXX + ''' + + SUPPORTS_GEO = False + SUPPORTS_DYNAMIC = False + SUPPORTS = set((['A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'DNAME', + 'MX', 'NS', 'PTR', 'SPF', 'SRV', 'SSHFP', 'TXT'])) + + def __init__(self, id, token, *args, **kwargs): + self.log = logging.getLogger('GandiProvider[{}]'.format(id)) + self.log.debug('__init__: id=%s, token=***', id) + super(GandiProvider, self).__init__(id, *args, **kwargs) + self._client = GandiClient(token) + + self._zone_records = {} + + def _data_for_multiple(self, _type, records): + return { + 'ttl': records[0]['rrset_ttl'], + 'type': _type, + 'values': [v.replace(';', '\\;') for v in + records[0]['rrset_values']] if _type == 'TXT' else + records[0]['rrset_values'] + } + + _data_for_A = _data_for_multiple + _data_for_AAAA = _data_for_multiple + _data_for_TXT = _data_for_multiple + _data_for_SPF = _data_for_multiple + _data_for_NS = _data_for_multiple + + def _data_for_CAA(self, _type, records): + values = [] + for record in records[0]['rrset_values']: + flags, tag, value = record.split(' ') + values.append({ + 'flags': flags, + 'tag': tag, + # Remove quotes around value. + 'value': value[1:-1], + }) + + return { + 'ttl': records[0]['rrset_ttl'], + 'type': _type, + 'values': values + } + + def _data_for_single(self, _type, records): + return { + 'ttl': records[0]['rrset_ttl'], + 'type': _type, + 'value': records[0]['rrset_values'][0] + } + + _data_for_ALIAS = _data_for_single + _data_for_CNAME = _data_for_single + _data_for_DNAME = _data_for_single + _data_for_PTR = _data_for_single + + def _data_for_MX(self, _type, records): + values = [] + for record in records[0]['rrset_values']: + priority, server = record.split(' ') + values.append({ + 'preference': priority, + 'exchange': server + }) + + return { + 'ttl': records[0]['rrset_ttl'], + 'type': _type, + 'values': values + } + + def _data_for_SRV(self, _type, records): + values = [] + for record in records[0]['rrset_values']: + priority, weight, port, target = record.split(' ', 3) + values.append({ + 'priority': priority, + 'weight': weight, + 'port': port, + 'target': target + }) + + return { + 'ttl': records[0]['rrset_ttl'], + 'type': _type, + 'values': values + } + + def _data_for_SSHFP(self, _type, records): + values = [] + for record in records[0]['rrset_values']: + algorithm, fingerprint_type, fingerprint = record.split(' ', 2) + values.append({ + 'algorithm': algorithm, + 'fingerprint': fingerprint, + 'fingerprint_type': fingerprint_type + }) + + return { + 'ttl': records[0]['rrset_ttl'], + 'type': _type, + 'values': values + } + + def zone_records(self, zone): + if zone.name not in self._zone_records: + try: + self._zone_records[zone.name] = \ + self._client.zone_records(zone.name[:-1]) + except GandiClientNotFound: + return [] + + return self._zone_records[zone.name] + + def populate(self, zone, target=False, lenient=False): + self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, + target, lenient) + + values = defaultdict(lambda: defaultdict(list)) + for record in self.zone_records(zone): + _type = record['rrset_type'] + if _type not in self.SUPPORTS: + continue + values[record['rrset_name']][record['rrset_type']].append(record) + + before = len(zone.records) + for name, types in values.items(): + for _type, records in types.items(): + data_for = getattr(self, '_data_for_{}'.format(_type)) + record = Record.new(zone, name, data_for(_type, records), + source=self, lenient=lenient) + zone.add_record(record, lenient=lenient) + + exists = zone.name in self._zone_records + self.log.info('populate: found %s records, exists=%s', + len(zone.records) - before, exists) + return exists + + def _record_name(self, name): + return name if name else '@' + + def _params_for_multiple(self, record): + return { + 'rrset_name': self._record_name(record.name), + 'rrset_ttl': record.ttl, + 'rrset_type': record._type, + 'rrset_values': [v.replace('\\;', ';') for v in + record.values] if record._type == 'TXT' + else record.values + } + + _params_for_A = _params_for_multiple + _params_for_AAAA = _params_for_multiple + _params_for_NS = _params_for_multiple + _params_for_TXT = _params_for_multiple + _params_for_SPF = _params_for_multiple + + def _params_for_CAA(self, record): + return { + 'rrset_name': self._record_name(record.name), + 'rrset_ttl': record.ttl, + 'rrset_type': record._type, + 'rrset_values': ['{} {} "{}"'.format(v.flags, v.tag, v.value) + for v in record.values] + } + + def _params_for_single(self, record): + return { + 'rrset_name': self._record_name(record.name), + 'rrset_ttl': record.ttl, + 'rrset_type': record._type, + 'rrset_values': [record.value] + } + + _params_for_ALIAS = _params_for_single + _params_for_CNAME = _params_for_single + _params_for_DNAME = _params_for_single + _params_for_PTR = _params_for_single + + def _params_for_MX(self, record): + return { + 'rrset_name': self._record_name(record.name), + 'rrset_ttl': record.ttl, + 'rrset_type': record._type, + 'rrset_values': ['{} {}'.format(v.preference, v.exchange) + for v in record.values] + } + + def _params_for_SRV(self, record): + return { + 'rrset_name': self._record_name(record.name), + 'rrset_ttl': record.ttl, + 'rrset_type': record._type, + 'rrset_values': ['{} {} {} {}'.format(v.priority, v.weight, v.port, + v.target) for v in record.values] + } + + def _params_for_SSHFP(self, record): + return { + 'rrset_name': self._record_name(record.name), + 'rrset_ttl': record.ttl, + 'rrset_type': record._type, + 'rrset_values': ['{} {} {}'.format(v.algorithm, v.fingerprint_type, + v.fingerprint) for v in record.values] + } + + def _apply_create(self, change): + new = change.new + data = getattr(self, '_params_for_{}'.format(new._type))(new) + self._client.record_create(new.zone.name[:-1], data) + + def _apply_update(self, change): + self._apply_delete(change) + self._apply_create(change) + + def _apply_delete(self, change): + existing = change.existing + zone = existing.zone + self._client.record_delete(zone.name[:-1], + self._record_name(existing.name), + existing._type) + + def _apply(self, plan): + desired = plan.desired + changes = plan.changes + self.log.debug('_apply: zone=%s, len(changes)=%d', desired.name, + len(changes)) + + # Force records deletion to be done before creation in order to avoid + # "CNAME record must be the only record" error when an existing CNAME + # record is replaced by an A/AAAA record. + changes.reverse() + + for change in changes: + class_name = change.__class__.__name__ + getattr(self, '_apply_{}'.format(class_name.lower()))(change) + + # Clear out the cache if any + self._zone_records.pop(desired.name, None) diff --git a/tests/fixtures/gandi-default-zone.json b/tests/fixtures/gandi-default-zone.json new file mode 100644 index 0000000..deb4cb8 --- /dev/null +++ b/tests/fixtures/gandi-default-zone.json @@ -0,0 +1,93 @@ +[ + { + "rrset_type": "A", + "rrset_ttl": 10800, + "rrset_name": "", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/%40/A", + "rrset_values": [ + "217.70.184.38" + ] + }, + { + "rrset_type": "MX", + "rrset_ttl": 10800, + "rrset_name": "", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/%40/MX", + "rrset_values": [ + "10 spool.mail.gandi.net.", + "50 fb.mail.gandi.net." + ] + }, + { + "rrset_type": "TXT", + "rrset_ttl": 10800, + "rrset_name": "", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/%40/TXT", + "rrset_values": [ + "\"v=spf1 include:_mailcust.gandi.net ?all\"" + ] + }, + { + "rrset_type": "CNAME", + "rrset_ttl": 10800, + "rrset_name": "webmail", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/webmail/CNAME", + "rrset_values": [ + "webmail.gandi.net." + ] + }, + { + "rrset_type": "CNAME", + "rrset_ttl": 10800, + "rrset_name": "www", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/www/CNAME", + "rrset_values": [ + "webredir.vip.gandi.net." + ] + }, + { + "rrset_type": "SRV", + "rrset_ttl": 10800, + "rrset_name": "_imap._tcp", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/_imap._tcp/SRV", + "rrset_values": [ + "0 0 0 ." + ] + }, + { + "rrset_type": "SRV", + "rrset_ttl": 10800, + "rrset_name": "_imaps._tcp", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/_imaps._tcp/SRV", + "rrset_values": [ + "0 1 993 mail.gandi.net." + ] + }, + { + "rrset_type": "SRV", + "rrset_ttl": 10800, + "rrset_name": "_pop3._tcp", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/_pop3._tcp/SRV", + "rrset_values": [ + "0 0 0 ." + ] + }, + { + "rrset_type": "SRV", + "rrset_ttl": 10800, + "rrset_name": "_pop3s._tcp", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/_pop3s._tcp/SRV", + "rrset_values": [ + "10 1 995 mail.gandi.net." + ] + }, + { + "rrset_type": "SRV", + "rrset_ttl": 10800, + "rrset_name": "_submission._tcp", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/_submission._tcp/SRV", + "rrset_values": [ + "0 1 465 mail.gandi.net." + ] + } +] diff --git a/tests/fixtures/gandi-no-changes.json b/tests/fixtures/gandi-no-changes.json new file mode 100644 index 0000000..9bff3cb --- /dev/null +++ b/tests/fixtures/gandi-no-changes.json @@ -0,0 +1,127 @@ +[ + { + "rrset_type": "A", + "rrset_ttl": 300, + "rrset_name": "", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/%40/A", + "rrset_values": [ + "1.2.3.4", + "1.2.3.5" + ] + }, + { + "rrset_type": "CAA", + "rrset_ttl": 3600, + "rrset_name": "", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/%40/CAA", + "rrset_values": [ + "0 issue \"ca.unit.tests\"" + ] + }, + { + "rrset_type": "SSHFP", + "rrset_ttl": 3600, + "rrset_name": "", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/%40/SSHFP", + "rrset_values": [ + "1 1 7491973e5f8b39d5327cd4e08bc81b05f7710b49", + "1 1 bf6b6825d2977c511a475bbefb88aad54a92ac73" + ] + }, + { + "rrset_type": "AAAA", + "rrset_ttl": 600, + "rrset_name": "aaaa", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/aaaa/AAAA", + "rrset_values": [ + "2601:644:500:e210:62f8:1dff:feb8:947a" + ] + }, + { + "rrset_type": "CNAME", + "rrset_ttl": 300, + "rrset_name": "cname", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/cname/CNAME", + "rrset_values": [ + "unit.tests." + ] + }, + { + "rrset_type": "CNAME", + "rrset_ttl": 3600, + "rrset_name": "excluded", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/excluded/CNAME", + "rrset_values": [ + "unit.tests." + ] + }, + { + "rrset_type": "MX", + "rrset_ttl": 300, + "rrset_name": "mx", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/mx/MX", + "rrset_values": [ + "10 smtp-4.unit.tests.", + "20 smtp-2.unit.tests.", + "30 smtp-3.unit.tests.", + "40 smtp-1.unit.tests." + ] + }, + { + "rrset_type": "PTR", + "rrset_ttl": 300, + "rrset_name": "ptr", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/ptr/PTR", + "rrset_values": [ + "foo.bar.com." + ] + }, + { + "rrset_type": "SPF", + "rrset_ttl": 600, + "rrset_name": "spf", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/spf/SPF", + "rrset_values": [ + "\"v=spf1 ip4:192.168.0.1/16-all\"" + ] + }, + { + "rrset_type": "TXT", + "rrset_ttl": 600, + "rrset_name": "txt", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/txt/TXT", + "rrset_values": [ + "\"Bah bah black sheep\"", + "\"have you any wool.\"", + "\"v=DKIM1;k=rsa;s=email;h=sha256;p=A/kinda+of/long/string+with+numb3rs\"" + ] + }, + { + "rrset_type": "A", + "rrset_ttl": 300, + "rrset_name": "www", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/www/A", + "rrset_values": [ + "2.2.3.6" + ] + }, + { + "rrset_type": "A", + "rrset_ttl": 300, + "rrset_name": "www.sub", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/www.sub/A", + "rrset_values": [ + "2.2.3.6" + ] + }, + { + "rrset_type": "SRV", + "rrset_ttl": 600, + "rrset_name": "_srv._tcp", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/_srv._tcp/SRV", + "rrset_values": [ + "10 20 30 foo-1.unit.tests.", + "12 20 30 foo-2.unit.tests." + ] + } + ] diff --git a/tests/test_octodns_provider_gandi.py b/tests/test_octodns_provider_gandi.py new file mode 100644 index 0000000..8152fcf --- /dev/null +++ b/tests/test_octodns_provider_gandi.py @@ -0,0 +1,312 @@ +# +# +# + +from __future__ import absolute_import, division, print_function, \ + unicode_literals + +from mock import Mock, call +from os.path import dirname, join +from requests import HTTPError +from requests_mock import ANY, mock as requests_mock +from six import text_type +from unittest import TestCase + +from octodns.record import Record +from octodns.provider.gandi import GandiProvider, GandiClientBadRequest, \ + GandiClientUnauthorized, GandiClientForbidden, GandiClientNotFound +from octodns.provider.yaml import YamlProvider +from octodns.zone import Zone + + +class TestGandiProvider(TestCase): + expected = Zone('unit.tests.', []) + source = YamlProvider('test', join(dirname(__file__), 'config')) + source.populate(expected) + + # We remove this record from the test zone as Gandi API reject it + # (rightfully). + expected._remove_record(Record.new(expected, 'sub', { + 'ttl': 1800, + 'type': 'NS', + 'values': [ + '6.2.3.4.', + '7.2.3.4.' + ] + })) + + def test_populate(self): + + provider = GandiProvider('test_id', 'token') + + # 400 - Bad Request. + with requests_mock() as mock: + mock.get(ANY, status_code=400, + text='{"status": "error", "errors": [{"location": ' + '"body", "name": "items", "description": ' + '"\'6.2.3.4.\': invalid hostname (param: ' + '{\'rrset_type\': u\'NS\', \'rrset_ttl\': 3600, ' + '\'rrset_name\': u\'sub\', \'rrset_values\': ' + '[u\'6.2.3.4.\', u\'7.2.3.4.\']})"}, {"location": ' + '"body", "name": "items", "description": ' + '"\'7.2.3.4.\': invalid hostname (param: ' + '{\'rrset_type\': u\'NS\', \'rrset_ttl\': 3600, ' + '\'rrset_name\': u\'sub\', \'rrset_values\': ' + '[u\'6.2.3.4.\', u\'7.2.3.4.\']})"}]}') + + with self.assertRaises(GandiClientBadRequest) as ctx: + zone = Zone('unit.tests.', []) + provider.populate(zone) + self.assertIn('"status": "error"', text_type(ctx.exception)) + + # 401 - Unauthorized. + with requests_mock() as mock: + mock.get(ANY, status_code=401, + text='{"code":401,"message":"The server could not verify ' + 'that you authorized to access the document you ' + 'requested. Either you supplied the wrong ' + 'credentials (e.g., bad api key), or your access ' + 'token has expired","object":"HTTPUnauthorized",' + '"cause":"Unauthorized"}') + + with self.assertRaises(GandiClientUnauthorized) as ctx: + zone = Zone('unit.tests.', []) + provider.populate(zone) + self.assertIn('"cause":"Unauthorized"', text_type(ctx.exception)) + + # 403 - Forbidden. + with requests_mock() as mock: + mock.get(ANY, status_code=403, + text='{"code":403,"message":"Access was denied to this ' + 'resource.","object":"HTTPForbidden","cause":' + '"Forbidden"}') + + with self.assertRaises(GandiClientForbidden) as ctx: + zone = Zone('unit.tests.', []) + provider.populate(zone) + self.assertIn('"cause":"Forbidden"', text_type(ctx.exception)) + + # General error + with requests_mock() as mock: + mock.get(ANY, status_code=502, text='Things caught fire') + + with self.assertRaises(HTTPError) as ctx: + zone = Zone('unit.tests.', []) + provider.populate(zone) + self.assertEquals(502, ctx.exception.response.status_code) + + # Non-existent zone doesn't populate anything + with requests_mock() as mock: + mock.get(ANY, status_code=404, + text='{"message": "Domain `foo.bar` not found"}') + + zone = Zone('unit.tests.', []) + provider.populate(zone) + self.assertEquals(set(), zone.records) + + # No diffs == no changes + with requests_mock() as mock: + base = 'https://api.gandi.net/v5/livedns/domains/unit.tests' \ + '/records' + with open('tests/fixtures/gandi-no-changes.json') as fh: + mock.get(base, text=fh.read()) + + zone = Zone('unit.tests.', []) + provider.populate(zone) + self.assertEquals(13, len(zone.records)) + changes = self.expected.changes(zone, provider) + self.assertEquals(0, len(changes)) + + del provider._zone_records[zone.name] + + # Default Gandi zone file. + with requests_mock() as mock: + base = 'https://api.gandi.net/v5/livedns/domains/unit.tests' \ + '/records' + with open('tests/fixtures/gandi-default-zone.json') as fh: + mock.get(base, text=fh.read()) + + zone = Zone('unit.tests.', []) + provider.populate(zone) + self.assertEquals(10, len(zone.records)) + changes = self.expected.changes(zone, provider) + self.assertEquals(22, len(changes)) + + # 2nd populate makes no network calls/all from cache + again = Zone('unit.tests.', []) + provider.populate(again) + self.assertEquals(10, len(again.records)) + + # bust the cache + del provider._zone_records[zone.name] + + def test_apply(self): + provider = GandiProvider('test_id', 'token') + + resp = Mock() + resp.json = Mock() + provider._client._request = Mock(return_value=resp) + + # non-existent domain + resp.json.side_effect = [ + GandiClientNotFound(resp), # no zone in populate + GandiClientNotFound(resp), # no domain during apply + ] + plan = provider.plan(self.expected) + + # No root NS, no ignored, no excluded + n = len(self.expected.records) - 4 + self.assertEquals(n, len(plan.changes)) + self.assertEquals(n, provider.apply(plan)) + self.assertFalse(plan.exists) + + provider._client._request.assert_has_calls([ + call('GET', '/livedns/domains/unit.tests/records'), + call('POST', '/livedns/domains/unit.tests/records', data={ + 'rrset_name': 'www.sub', + 'rrset_ttl': 300, + 'rrset_type': 'A', + 'rrset_values': ['2.2.3.6'] + }), + call('POST', '/livedns/domains/unit.tests/records', data={ + 'rrset_name': 'www', + 'rrset_ttl': 300, + 'rrset_type': 'A', + 'rrset_values': ['2.2.3.6'] + }), + call('POST', '/livedns/domains/unit.tests/records', data={ + 'rrset_name': 'txt', + 'rrset_ttl': 600, + 'rrset_type': 'TXT', + 'rrset_values': [ + 'Bah bah black sheep', + 'have you any wool.', + 'v=DKIM1;k=rsa;s=email;h=sha256;p=A/kinda+of/long/string' + '+with+numb3rs' + ] + }), + call('POST', '/livedns/domains/unit.tests/records', data={ + 'rrset_name': 'spf', + 'rrset_ttl': 600, + 'rrset_type': 'SPF', + 'rrset_values': ['v=spf1 ip4:192.168.0.1/16-all'] + }), + call('POST', '/livedns/domains/unit.tests/records', data={ + 'rrset_name': 'ptr', + 'rrset_ttl': 300, + 'rrset_type': 'PTR', + 'rrset_values': ['foo.bar.com.'] + }), + call('POST', '/livedns/domains/unit.tests/records', data={ + 'rrset_name': 'mx', + 'rrset_ttl': 300, + 'rrset_type': 'MX', + 'rrset_values': [ + '10 smtp-4.unit.tests.', + '20 smtp-2.unit.tests.', + '30 smtp-3.unit.tests.', + '40 smtp-1.unit.tests.' + ] + }), + call('POST', '/livedns/domains/unit.tests/records', data={ + 'rrset_name': 'excluded', + 'rrset_ttl': 3600, + 'rrset_type': 'CNAME', + 'rrset_values': ['unit.tests.'] + }), + call('POST', '/livedns/domains/unit.tests/records', data={ + 'rrset_name': 'cname', + 'rrset_ttl': 300, + 'rrset_type': 'CNAME', + 'rrset_values': ['unit.tests.'] + }), + call('POST', '/livedns/domains/unit.tests/records', data={ + 'rrset_name': 'aaaa', + 'rrset_ttl': 600, + 'rrset_type': 'AAAA', + 'rrset_values': ['2601:644:500:e210:62f8:1dff:feb8:947a'] + }), + call('POST', '/livedns/domains/unit.tests/records', data={ + 'rrset_name': '_srv._tcp', + 'rrset_ttl': 600, + 'rrset_type': 'SRV', + 'rrset_values': [ + '10 20 30 foo-1.unit.tests.', + '12 20 30 foo-2.unit.tests.' + ] + }), + call('POST', '/livedns/domains/unit.tests/records', data={ + 'rrset_name': '@', + 'rrset_ttl': 3600, + 'rrset_type': 'SSHFP', + 'rrset_values': [ + '1 1 7491973e5f8b39d5327cd4e08bc81b05f7710b49', + '1 1 bf6b6825d2977c511a475bbefb88aad54a92ac73' + ] + }), + call('POST', '/livedns/domains/unit.tests/records', data={ + 'rrset_name': '@', + 'rrset_ttl': 3600, + 'rrset_type': 'CAA', + 'rrset_values': ['0 issue "ca.unit.tests"'] + }), + call('POST', '/livedns/domains/unit.tests/records', data={ + 'rrset_name': '@', + 'rrset_ttl': 300, + 'rrset_type': 'A', + 'rrset_values': ['1.2.3.4', '1.2.3.5'] + }) + ]) + # expected number of total calls + self.assertEquals(14, provider._client._request.call_count) + + provider._client._request.reset_mock() + + # delete 1 and update 1 + provider._client.zone_records = Mock(return_value=[ + { + 'rrset_name': 'www', + 'rrset_ttl': 300, + 'rrset_type': 'A', + 'rrset_values': ['1.2.3.4'] + }, + { + 'rrset_name': 'www', + 'rrset_ttl': 300, + 'rrset_type': 'A', + 'rrset_values': ['2.2.3.4'] + }, + { + 'rrset_name': 'ttl', + 'rrset_ttl': 600, + 'rrset_type': 'A', + 'rrset_values': ['3.2.3.4'] + } + ]) + + # Domain exists, we don't care about return + resp.json.side_effect = ['{}'] + + wanted = Zone('unit.tests.', []) + wanted.add_record(Record.new(wanted, 'ttl', { + 'ttl': 300, + 'type': 'A', + 'value': '3.2.3.4' + })) + + plan = provider.plan(wanted) + self.assertTrue(plan.exists) + self.assertEquals(2, len(plan.changes)) + self.assertEquals(2, provider.apply(plan)) + + # recreate for update, and deletes for the 2 parts of the other + provider._client._request.assert_has_calls([ + call('DELETE', '/livedns/domains/unit.tests/records/www/A'), + call('DELETE', '/livedns/domains/unit.tests/records/ttl/A'), + call('POST', '/livedns/domains/unit.tests/records', data={ + 'rrset_name': 'ttl', + 'rrset_ttl': 300, + 'rrset_type': 'A', + 'rrset_values': ['3.2.3.4'] + }) + ], any_order=True) From 3f852442648e6e4b2971ebf1cddea5ad53fce103 Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Mon, 26 Oct 2020 20:30:15 +0100 Subject: [PATCH 13/45] Fixes incorrect domain name in gandi-no-changes.json --- tests/fixtures/gandi-no-changes.json | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/fixtures/gandi-no-changes.json b/tests/fixtures/gandi-no-changes.json index 9bff3cb..4646327 100644 --- a/tests/fixtures/gandi-no-changes.json +++ b/tests/fixtures/gandi-no-changes.json @@ -3,7 +3,7 @@ "rrset_type": "A", "rrset_ttl": 300, "rrset_name": "", - "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/%40/A", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/%40/A", "rrset_values": [ "1.2.3.4", "1.2.3.5" @@ -13,7 +13,7 @@ "rrset_type": "CAA", "rrset_ttl": 3600, "rrset_name": "", - "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/%40/CAA", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/%40/CAA", "rrset_values": [ "0 issue \"ca.unit.tests\"" ] @@ -22,7 +22,7 @@ "rrset_type": "SSHFP", "rrset_ttl": 3600, "rrset_name": "", - "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/%40/SSHFP", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/%40/SSHFP", "rrset_values": [ "1 1 7491973e5f8b39d5327cd4e08bc81b05f7710b49", "1 1 bf6b6825d2977c511a475bbefb88aad54a92ac73" @@ -32,7 +32,7 @@ "rrset_type": "AAAA", "rrset_ttl": 600, "rrset_name": "aaaa", - "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/aaaa/AAAA", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/aaaa/AAAA", "rrset_values": [ "2601:644:500:e210:62f8:1dff:feb8:947a" ] @@ -41,7 +41,7 @@ "rrset_type": "CNAME", "rrset_ttl": 300, "rrset_name": "cname", - "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/cname/CNAME", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/cname/CNAME", "rrset_values": [ "unit.tests." ] @@ -50,7 +50,7 @@ "rrset_type": "CNAME", "rrset_ttl": 3600, "rrset_name": "excluded", - "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/excluded/CNAME", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/excluded/CNAME", "rrset_values": [ "unit.tests." ] @@ -59,7 +59,7 @@ "rrset_type": "MX", "rrset_ttl": 300, "rrset_name": "mx", - "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/mx/MX", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/mx/MX", "rrset_values": [ "10 smtp-4.unit.tests.", "20 smtp-2.unit.tests.", @@ -71,7 +71,7 @@ "rrset_type": "PTR", "rrset_ttl": 300, "rrset_name": "ptr", - "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/ptr/PTR", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/ptr/PTR", "rrset_values": [ "foo.bar.com." ] @@ -80,7 +80,7 @@ "rrset_type": "SPF", "rrset_ttl": 600, "rrset_name": "spf", - "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/spf/SPF", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/spf/SPF", "rrset_values": [ "\"v=spf1 ip4:192.168.0.1/16-all\"" ] @@ -89,7 +89,7 @@ "rrset_type": "TXT", "rrset_ttl": 600, "rrset_name": "txt", - "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/txt/TXT", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/txt/TXT", "rrset_values": [ "\"Bah bah black sheep\"", "\"have you any wool.\"", @@ -100,7 +100,7 @@ "rrset_type": "A", "rrset_ttl": 300, "rrset_name": "www", - "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/www/A", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/www/A", "rrset_values": [ "2.2.3.6" ] @@ -109,7 +109,7 @@ "rrset_type": "A", "rrset_ttl": 300, "rrset_name": "www.sub", - "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/www.sub/A", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/www.sub/A", "rrset_values": [ "2.2.3.6" ] @@ -118,7 +118,7 @@ "rrset_type": "SRV", "rrset_ttl": 600, "rrset_name": "_srv._tcp", - "rrset_href": "https://api.gandi.net/v5/livedns/domains/reductioncarbone.fr/records/_srv._tcp/SRV", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/_srv._tcp/SRV", "rrset_values": [ "10 20 30 foo-1.unit.tests.", "12 20 30 foo-2.unit.tests." From de51e5f531f85909d8ad066e3d5e495f8fe9a740 Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Mon, 26 Oct 2020 22:18:35 +0100 Subject: [PATCH 14/45] Add support for DNAME records --- docs/records.md | 1 + octodns/provider/yaml.py | 4 +- octodns/record/__init__.py | 10 ++ tests/config/dynamic.tests.yaml | 34 ++++++ tests/config/split/dynamic.tests./dname.yaml | 42 +++++++ tests/config/split/unit.tests./dname.yaml | 5 + tests/config/unit.tests.yaml | 4 + tests/test_octodns_manager.py | 14 +-- tests/test_octodns_provider_constellix.py | 2 +- tests/test_octodns_provider_digitalocean.py | 2 +- tests/test_octodns_provider_dnsimple.py | 2 +- tests/test_octodns_provider_dnsmadeeasy.py | 2 +- tests/test_octodns_provider_easydns.py | 2 +- tests/test_octodns_provider_powerdns.py | 4 +- tests/test_octodns_provider_yaml.py | 43 ++++--- tests/test_octodns_record.py | 117 ++++++++++++++++++- 16 files changed, 249 insertions(+), 39 deletions(-) create mode 100644 tests/config/split/dynamic.tests./dname.yaml create mode 100644 tests/config/split/unit.tests./dname.yaml diff --git a/docs/records.md b/docs/records.md index 609383c..a28e86f 100644 --- a/docs/records.md +++ b/docs/records.md @@ -7,6 +7,7 @@ OctoDNS supports the following record types: * `A` * `AAAA` * `CNAME` +* `DNAME` * `MX` * `NAPTR` * `NS` diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 10add5a..55a1632 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -104,8 +104,8 @@ class YamlProvider(BaseProvider): ''' SUPPORTS_GEO = True SUPPORTS_DYNAMIC = True - SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'MX', 'NAPTR', 'NS', - 'PTR', 'SSHFP', 'SPF', 'SRV', 'TXT')) + SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'DNAME', 'MX', + 'NAPTR', 'NS', 'PTR', 'SSHFP', 'SPF', 'SRV', 'TXT')) def __init__(self, id, directory, default_ttl=3600, enforce_order=True, populate_should_replace=False, *args, **kwargs): diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index 849e035..08ec2ee 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -95,6 +95,7 @@ class Record(EqualityTupleMixin): 'ALIAS': AliasRecord, 'CAA': CaaRecord, 'CNAME': CnameRecord, + 'DNAME': DnameRecord, 'MX': MxRecord, 'NAPTR': NaptrRecord, 'NS': NsRecord, @@ -759,6 +760,10 @@ class CnameValue(_TargetValue): pass +class DnameValue(_TargetValue): + pass + + class ARecord(_DynamicMixin, _GeoMixin, Record): _type = 'A' _value_type = Ipv4List @@ -842,6 +847,11 @@ class CnameRecord(_DynamicMixin, _ValueMixin, Record): return reasons +class DnameRecord(_DynamicMixin, _ValueMixin, Record): + _type = 'DNAME' + _value_type = DnameValue + + class MxValue(EqualityTupleMixin): @classmethod diff --git a/tests/config/dynamic.tests.yaml b/tests/config/dynamic.tests.yaml index 4bd97a7..5595a6d 100644 --- a/tests/config/dynamic.tests.yaml +++ b/tests/config/dynamic.tests.yaml @@ -109,6 +109,40 @@ cname: - pool: iad type: CNAME value: target.unit.tests. +dname: + dynamic: + pools: + ams: + values: + - value: target-ams.unit.tests. + iad: + values: + - value: target-iad.unit.tests. + lax: + values: + - value: target-lax.unit.tests. + sea: + values: + - value: target-sea-1.unit.tests. + weight: 10 + - value: target-sea-2.unit.tests. + weight: 14 + rules: + - geos: + - EU-GB + pool: lax + - geos: + - EU + pool: ams + - geos: + - NA-US-CA + - NA-US-NC + - NA-US-OR + - NA-US-WA + pool: sea + - pool: iad + type: DNAME + value: target.unit.tests. real-ish-a: dynamic: pools: diff --git a/tests/config/split/dynamic.tests./dname.yaml b/tests/config/split/dynamic.tests./dname.yaml new file mode 100644 index 0000000..45c33fe --- /dev/null +++ b/tests/config/split/dynamic.tests./dname.yaml @@ -0,0 +1,42 @@ +--- +dname: + dynamic: + pools: + ams: + fallback: null + values: + - value: target-ams.unit.tests. + weight: 1 + iad: + fallback: null + values: + - value: target-iad.unit.tests. + weight: 1 + lax: + fallback: null + values: + - value: target-lax.unit.tests. + weight: 1 + sea: + fallback: null + values: + - value: target-sea-1.unit.tests. + weight: 10 + - value: target-sea-2.unit.tests. + weight: 14 + rules: + - geos: + - EU-GB + pool: lax + - geos: + - EU + pool: ams + - geos: + - NA-US-CA + - NA-US-NC + - NA-US-OR + - NA-US-WA + pool: sea + - pool: iad + type: DNAME + value: target.unit.tests. diff --git a/tests/config/split/unit.tests./dname.yaml b/tests/config/split/unit.tests./dname.yaml new file mode 100644 index 0000000..7cd1755 --- /dev/null +++ b/tests/config/split/unit.tests./dname.yaml @@ -0,0 +1,5 @@ +--- +dname: + ttl: 300 + type: DNAME + value: unit.tests. diff --git a/tests/config/unit.tests.yaml b/tests/config/unit.tests.yaml index 1da2465..7b84ac9 100644 --- a/tests/config/unit.tests.yaml +++ b/tests/config/unit.tests.yaml @@ -56,6 +56,10 @@ cname: ttl: 300 type: CNAME value: unit.tests. +dname: + ttl: 300 + type: DNAME + value: unit.tests. excluded: octodns: excluded: diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 9956790..7d25048 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -118,12 +118,12 @@ class TestManager(TestCase): environ['YAML_TMP_DIR'] = tmpdir.dirname tc = Manager(get_config_filename('simple.yaml')) \ .sync(dry_run=False) - self.assertEquals(21, tc) + self.assertEquals(22, tc) # try with just one of the zones tc = Manager(get_config_filename('simple.yaml')) \ .sync(dry_run=False, eligible_zones=['unit.tests.']) - self.assertEquals(15, tc) + self.assertEquals(16, tc) # the subzone, with 2 targets tc = Manager(get_config_filename('simple.yaml')) \ @@ -138,18 +138,18 @@ class TestManager(TestCase): # Again with force tc = Manager(get_config_filename('simple.yaml')) \ .sync(dry_run=False, force=True) - self.assertEquals(21, tc) + self.assertEquals(22, tc) # Again with max_workers = 1 tc = Manager(get_config_filename('simple.yaml'), max_workers=1) \ .sync(dry_run=False, force=True) - self.assertEquals(21, tc) + self.assertEquals(22, tc) # Include meta tc = Manager(get_config_filename('simple.yaml'), max_workers=1, include_meta=True) \ .sync(dry_run=False, force=True) - self.assertEquals(25, tc) + self.assertEquals(26, tc) def test_eligible_sources(self): with TemporaryDirectory() as tmpdir: @@ -183,13 +183,13 @@ class TestManager(TestCase): fh.write('---\n{}') changes = manager.compare(['in'], ['dump'], 'unit.tests.') - self.assertEquals(15, len(changes)) + self.assertEquals(16, len(changes)) # Compound sources with varying support changes = manager.compare(['in', 'nosshfp'], ['dump'], 'unit.tests.') - self.assertEquals(14, len(changes)) + self.assertEquals(15, len(changes)) with self.assertRaises(ManagerException) as ctx: manager.compare(['nope'], ['dump'], 'unit.tests.') diff --git a/tests/test_octodns_provider_constellix.py b/tests/test_octodns_provider_constellix.py index 151d0d4..c19ae29 100644 --- a/tests/test_octodns_provider_constellix.py +++ b/tests/test_octodns_provider_constellix.py @@ -138,7 +138,7 @@ class TestConstellixProvider(TestCase): plan = provider.plan(self.expected) # No root NS, no ignored, no excluded, no unsupported - n = len(self.expected.records) - 5 + n = len(self.expected.records) - 6 self.assertEquals(n, len(plan.changes)) self.assertEquals(n, provider.apply(plan)) diff --git a/tests/test_octodns_provider_digitalocean.py b/tests/test_octodns_provider_digitalocean.py index ebb5319..0ad8f72 100644 --- a/tests/test_octodns_provider_digitalocean.py +++ b/tests/test_octodns_provider_digitalocean.py @@ -163,7 +163,7 @@ class TestDigitalOceanProvider(TestCase): plan = provider.plan(self.expected) # No root NS, no ignored, no excluded, no unsupported - n = len(self.expected.records) - 7 + n = len(self.expected.records) - 8 self.assertEquals(n, len(plan.changes)) self.assertEquals(n, provider.apply(plan)) self.assertFalse(plan.exists) diff --git a/tests/test_octodns_provider_dnsimple.py b/tests/test_octodns_provider_dnsimple.py index b918962..92f32b1 100644 --- a/tests/test_octodns_provider_dnsimple.py +++ b/tests/test_octodns_provider_dnsimple.py @@ -137,7 +137,7 @@ class TestDnsimpleProvider(TestCase): plan = provider.plan(self.expected) # No root NS, no ignored, no excluded - n = len(self.expected.records) - 3 + n = len(self.expected.records) - 4 self.assertEquals(n, len(plan.changes)) self.assertEquals(n, provider.apply(plan)) self.assertFalse(plan.exists) diff --git a/tests/test_octodns_provider_dnsmadeeasy.py b/tests/test_octodns_provider_dnsmadeeasy.py index ba61b94..50fa576 100644 --- a/tests/test_octodns_provider_dnsmadeeasy.py +++ b/tests/test_octodns_provider_dnsmadeeasy.py @@ -140,7 +140,7 @@ class TestDnsMadeEasyProvider(TestCase): plan = provider.plan(self.expected) # No root NS, no ignored, no excluded, no unsupported - n = len(self.expected.records) - 5 + n = len(self.expected.records) - 6 self.assertEquals(n, len(plan.changes)) self.assertEquals(n, provider.apply(plan)) diff --git a/tests/test_octodns_provider_easydns.py b/tests/test_octodns_provider_easydns.py index 2681bf4..8df0e22 100644 --- a/tests/test_octodns_provider_easydns.py +++ b/tests/test_octodns_provider_easydns.py @@ -374,7 +374,7 @@ class TestEasyDNSProvider(TestCase): plan = provider.plan(self.expected) # No root NS, no ignored, no excluded, no unsupported - n = len(self.expected.records) - 6 + n = len(self.expected.records) - 7 self.assertEquals(n, len(plan.changes)) self.assertEquals(n, provider.apply(plan)) self.assertFalse(plan.exists) diff --git a/tests/test_octodns_provider_powerdns.py b/tests/test_octodns_provider_powerdns.py index fd877ef..c9b1d08 100644 --- a/tests/test_octodns_provider_powerdns.py +++ b/tests/test_octodns_provider_powerdns.py @@ -171,7 +171,7 @@ class TestPowerDnsProvider(TestCase): expected = Zone('unit.tests.', []) source = YamlProvider('test', join(dirname(__file__), 'config')) source.populate(expected) - expected_n = len(expected.records) - 2 + expected_n = len(expected.records) - 3 self.assertEquals(16, expected_n) # No diffs == no changes @@ -277,7 +277,7 @@ class TestPowerDnsProvider(TestCase): expected = Zone('unit.tests.', []) source = YamlProvider('test', join(dirname(__file__), 'config')) source.populate(expected) - self.assertEquals(18, len(expected.records)) + self.assertEquals(19, len(expected.records)) # A small change to a single record with requests_mock() as mock: diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index f858c05..7b285ec 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -35,10 +35,10 @@ class TestYamlProvider(TestCase): # without it we see everything source.populate(zone) - self.assertEquals(18, len(zone.records)) + self.assertEquals(19, len(zone.records)) source.populate(dynamic_zone) - self.assertEquals(5, len(dynamic_zone.records)) + self.assertEquals(6, len(dynamic_zone.records)) # Assumption here is that a clean round-trip means that everything # worked as expected, data that went in came back out and could be @@ -58,21 +58,21 @@ class TestYamlProvider(TestCase): # We add everything plan = target.plan(zone) - self.assertEquals(15, len([c for c in plan.changes + self.assertEquals(16, len([c for c in plan.changes if isinstance(c, Create)])) self.assertFalse(isfile(yaml_file)) # Now actually do it - self.assertEquals(15, target.apply(plan)) + self.assertEquals(16, target.apply(plan)) self.assertTrue(isfile(yaml_file)) # Dynamic plan plan = target.plan(dynamic_zone) - self.assertEquals(5, len([c for c in plan.changes + self.assertEquals(6, len([c for c in plan.changes if isinstance(c, Create)])) self.assertFalse(isfile(dynamic_yaml_file)) # Apply it - self.assertEquals(5, target.apply(plan)) + self.assertEquals(6, target.apply(plan)) self.assertTrue(isfile(dynamic_yaml_file)) # There should be no changes after the round trip @@ -87,7 +87,7 @@ class TestYamlProvider(TestCase): # A 2nd sync should still create everything plan = target.plan(zone) - self.assertEquals(15, len([c for c in plan.changes + self.assertEquals(16, len([c for c in plan.changes if isinstance(c, Create)])) with open(yaml_file) as fh: @@ -109,6 +109,7 @@ class TestYamlProvider(TestCase): # these are stored as singular 'value' self.assertTrue('value' in data.pop('aaaa')) self.assertTrue('value' in data.pop('cname')) + self.assertTrue('value' in data.pop('dname')) self.assertTrue('value' in data.pop('included')) self.assertTrue('value' in data.pop('ptr')) self.assertTrue('value' in data.pop('spf')) @@ -136,6 +137,10 @@ class TestYamlProvider(TestCase): self.assertTrue('value' in dyna) # self.assertTrue('dynamic' in dyna) + dyna = data.pop('dname') + self.assertTrue('value' in dyna) + # self.assertTrue('dynamic' in dyna) + dyna = data.pop('real-ish-a') self.assertTrue('values' in dyna) # self.assertTrue('dynamic' in dyna) @@ -237,10 +242,10 @@ class TestSplitYamlProvider(TestCase): # without it we see everything source.populate(zone) - self.assertEquals(18, len(zone.records)) + self.assertEquals(19, len(zone.records)) source.populate(dynamic_zone) - self.assertEquals(5, len(dynamic_zone.records)) + self.assertEquals(6, len(dynamic_zone.records)) with TemporaryDirectory() as td: # Add some subdirs to make sure that it can create them @@ -251,20 +256,20 @@ class TestSplitYamlProvider(TestCase): # We add everything plan = target.plan(zone) - self.assertEquals(15, len([c for c in plan.changes + self.assertEquals(16, len([c for c in plan.changes if isinstance(c, Create)])) self.assertFalse(isdir(zone_dir)) # Now actually do it - self.assertEquals(15, target.apply(plan)) + self.assertEquals(16, target.apply(plan)) # Dynamic plan plan = target.plan(dynamic_zone) - self.assertEquals(5, len([c for c in plan.changes + self.assertEquals(6, len([c for c in plan.changes if isinstance(c, Create)])) self.assertFalse(isdir(dynamic_zone_dir)) # Apply it - self.assertEquals(5, target.apply(plan)) + self.assertEquals(6, target.apply(plan)) self.assertTrue(isdir(dynamic_zone_dir)) # There should be no changes after the round trip @@ -279,7 +284,7 @@ class TestSplitYamlProvider(TestCase): # A 2nd sync should still create everything plan = target.plan(zone) - self.assertEquals(15, len([c for c in plan.changes + self.assertEquals(16, len([c for c in plan.changes if isinstance(c, Create)])) yaml_file = join(zone_dir, '$unit.tests.yaml') @@ -302,8 +307,8 @@ class TestSplitYamlProvider(TestCase): self.assertTrue('values' in data.pop(record_name)) # These are stored as singular "value." Again, check each file. - for record_name in ('aaaa', 'cname', 'included', 'ptr', 'spf', - 'www.sub', 'www'): + for record_name in ('aaaa', 'cname', 'dname', 'included', 'ptr', + 'spf', 'www.sub', 'www'): yaml_file = join(zone_dir, '{}.yaml'.format(record_name)) self.assertTrue(isfile(yaml_file)) with open(yaml_file) as fh: @@ -322,7 +327,7 @@ class TestSplitYamlProvider(TestCase): self.assertTrue('dynamic' in dyna) # Singular again. - for record_name in ('cname', 'simple-weighted'): + for record_name in ('cname', 'dname', 'simple-weighted'): yaml_file = join( dynamic_zone_dir, '{}.yaml'.format(record_name)) self.assertTrue(isfile(yaml_file)) @@ -386,7 +391,7 @@ class TestOverridingYamlProvider(TestCase): # Load the base, should see the 5 records base.populate(zone) got = {r.name: r for r in zone.records} - self.assertEquals(5, len(got)) + self.assertEquals(6, len(got)) # We get the "dynamic" A from the bae config self.assertTrue('dynamic' in got['a'].data) # No added @@ -395,7 +400,7 @@ class TestOverridingYamlProvider(TestCase): # Load the overrides, should replace one and add 1 override.populate(zone) got = {r.name: r for r in zone.records} - self.assertEquals(6, len(got)) + self.assertEquals(7, len(got)) # 'a' was replaced with a generic record self.assertEquals({ 'ttl': 3600, diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 08a3e7a..10e9575 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -9,10 +9,10 @@ from six import text_type from unittest import TestCase from octodns.record import ARecord, AaaaRecord, AliasRecord, CaaRecord, \ - CaaValue, CnameRecord, Create, Delete, GeoValue, MxRecord, MxValue, \ - NaptrRecord, NaptrValue, NsRecord, PtrRecord, Record, SshfpRecord, \ - SshfpValue, SpfRecord, SrvRecord, SrvValue, TxtRecord, Update, \ - ValidationError, _Dynamic, _DynamicPool, _DynamicRule + CaaValue, CnameRecord, DnameRecord, Create, Delete, GeoValue, MxRecord, \ + MxValue, NaptrRecord, NaptrValue, NsRecord, PtrRecord, Record, \ + SshfpRecord, SshfpValue, SpfRecord, SrvRecord, SrvValue, TxtRecord, \ + Update, ValidationError, _Dynamic, _DynamicPool, _DynamicRule from octodns.zone import Zone from helpers import DynamicProvider, GeoProvider, SimpleProvider @@ -55,6 +55,19 @@ class TestRecord(TestCase): }) self.assertEquals(upper_record.value, lower_record.value) + def test_dname_lowering_value(self): + upper_record = DnameRecord(self.zone, 'DnameUppwerValue', { + 'ttl': 30, + 'type': 'DNAME', + 'value': 'GITHUB.COM', + }) + lower_record = DnameRecord(self.zone, 'DnameLowerValue', { + 'ttl': 30, + 'type': 'DNAME', + 'value': 'github.com', + }) + self.assertEquals(upper_record.value, lower_record.value) + def test_ptr_lowering_value(self): upper_record = PtrRecord(self.zone, 'PtrUppwerValue', { 'ttl': 30, @@ -362,6 +375,10 @@ class TestRecord(TestCase): self.assertSingleValue(CnameRecord, 'target.foo.com.', 'other.foo.com.') + def test_dname(self): + self.assertSingleValue(DnameRecord, 'target.foo.com.', + 'other.foo.com.') + def test_mx(self): a_values = [{ 'preference': 10, @@ -1825,6 +1842,31 @@ class TestRecordValidation(TestCase): self.assertEquals(['CNAME value "foo.bar.com" missing trailing .'], ctx.exception.reasons) + def test_DNAME(self): + # A valid DNAME record. + Record.new(self.zone, 'sub', { + 'type': 'DNAME', + 'ttl': 600, + 'value': 'foo.bar.com.', + }) + + # A DNAME record can be present at the zone APEX. + Record.new(self.zone, '', { + 'type': 'DNAME', + 'ttl': 600, + 'value': 'foo.bar.com.', + }) + + # missing trailing . + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'www', { + 'type': 'DNAME', + 'ttl': 600, + 'value': 'foo.bar.com', + }) + self.assertEquals(['DNAME value "foo.bar.com" missing trailing .'], + ctx.exception.reasons) + def test_MX(self): # doesn't blow up Record.new(self.zone, '', { @@ -2628,6 +2670,73 @@ class TestDynamicRecords(TestCase): self.assertTrue(rules) self.assertEquals(cname_data['dynamic']['rules'][0], rules[0].data) + def test_simple_dname_weighted(self): + dname_data = { + 'dynamic': { + 'pools': { + 'one': { + 'values': [{ + 'value': 'one.dname.target.', + }], + }, + 'two': { + 'values': [{ + 'value': 'two.dname.target.', + }], + }, + 'three': { + 'values': [{ + 'weight': 12, + 'value': 'three-1.dname.target.', + }, { + 'weight': 32, + 'value': 'three-2.dname.target.', + }] + }, + }, + 'rules': [{ + 'geos': ['AF', 'EU'], + 'pool': 'three', + }, { + 'geos': ['NA-US-CA'], + 'pool': 'two', + }, { + 'pool': 'one', + }], + }, + 'ttl': 60, + 'value': 'dname.target.', + } + dname = DnameRecord(self.zone, 'weighted', dname_data) + self.assertEquals('DNAME', dname._type) + self.assertEquals(dname_data['ttl'], dname.ttl) + self.assertEquals(dname_data['value'], dname.value) + + dynamic = dname.dynamic + self.assertTrue(dynamic) + + pools = dynamic.pools + self.assertTrue(pools) + self.assertEquals({ + 'value': 'one.dname.target.', + 'weight': 1, + }, pools['one'].data['values'][0]) + self.assertEquals({ + 'value': 'two.dname.target.', + 'weight': 1, + }, pools['two'].data['values'][0]) + self.assertEquals([{ + 'value': 'three-1.dname.target.', + 'weight': 12, + }, { + 'value': 'three-2.dname.target.', + 'weight': 32, + }], pools['three'].data['values']) + + rules = dynamic.rules + self.assertTrue(rules) + self.assertEquals(dname_data['dynamic']['rules'][0], rules[0].data) + def test_dynamic_validation(self): # Missing pools a_data = { From bfaafeb61b92d284e835367c9de296035fcdb489 Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Mon, 26 Oct 2020 23:10:36 +0100 Subject: [PATCH 15/45] Fixes value of "rrset_name" parameter for domain APEX --- tests/fixtures/gandi-default-zone.json | 6 +++--- tests/fixtures/gandi-no-changes.json | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/fixtures/gandi-default-zone.json b/tests/fixtures/gandi-default-zone.json index deb4cb8..254b7c1 100644 --- a/tests/fixtures/gandi-default-zone.json +++ b/tests/fixtures/gandi-default-zone.json @@ -2,7 +2,7 @@ { "rrset_type": "A", "rrset_ttl": 10800, - "rrset_name": "", + "rrset_name": "@", "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/%40/A", "rrset_values": [ "217.70.184.38" @@ -11,7 +11,7 @@ { "rrset_type": "MX", "rrset_ttl": 10800, - "rrset_name": "", + "rrset_name": "@", "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/%40/MX", "rrset_values": [ "10 spool.mail.gandi.net.", @@ -21,7 +21,7 @@ { "rrset_type": "TXT", "rrset_ttl": 10800, - "rrset_name": "", + "rrset_name": "@", "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/%40/TXT", "rrset_values": [ "\"v=spf1 include:_mailcust.gandi.net ?all\"" diff --git a/tests/fixtures/gandi-no-changes.json b/tests/fixtures/gandi-no-changes.json index 4646327..1154628 100644 --- a/tests/fixtures/gandi-no-changes.json +++ b/tests/fixtures/gandi-no-changes.json @@ -2,7 +2,7 @@ { "rrset_type": "A", "rrset_ttl": 300, - "rrset_name": "", + "rrset_name": "@", "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/%40/A", "rrset_values": [ "1.2.3.4", @@ -12,7 +12,7 @@ { "rrset_type": "CAA", "rrset_ttl": 3600, - "rrset_name": "", + "rrset_name": "@", "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/%40/CAA", "rrset_values": [ "0 issue \"ca.unit.tests\"" @@ -21,7 +21,7 @@ { "rrset_type": "SSHFP", "rrset_ttl": 3600, - "rrset_name": "", + "rrset_name": "@", "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/%40/SSHFP", "rrset_values": [ "1 1 7491973e5f8b39d5327cd4e08bc81b05f7710b49", From 7161baa2628fd28d3eb2c9ceec6953a9afae359b Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Mon, 26 Oct 2020 23:23:32 +0100 Subject: [PATCH 16/45] Fixes code coverage for unsupported records types --- tests/fixtures/gandi-default-zone.json | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/fixtures/gandi-default-zone.json b/tests/fixtures/gandi-default-zone.json index 254b7c1..9b07b8e 100644 --- a/tests/fixtures/gandi-default-zone.json +++ b/tests/fixtures/gandi-default-zone.json @@ -89,5 +89,14 @@ "rrset_values": [ "0 1 465 mail.gandi.net." ] + }, + { + "rrset_type": "CDS", + "rrset_ttl": 10800, + "rrset_name": "sub", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/sub/CDS", + "rrset_values": [ + "32128 13 1 6823D9BB1B03DF714DD0EB163E20B341C96D18C0" + ] } ] From 6d17b4671ab964d1dada7319e77f4de12438de02 Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Tue, 27 Oct 2020 11:23:22 +0100 Subject: [PATCH 17/45] Handle domains not registred at Gandi or not using Gandi's DNS --- octodns/provider/gandi.py | 32 ++++++++++++++++++++++++++++ tests/fixtures/gandi-zone.json | 7 ++++++ tests/test_octodns_provider_gandi.py | 32 +++++++++++++++++++--------- 3 files changed, 61 insertions(+), 10 deletions(-) create mode 100644 tests/fixtures/gandi-zone.json diff --git a/octodns/provider/gandi.py b/octodns/provider/gandi.py index e11e9a3..1f89a80 100644 --- a/octodns/provider/gandi.py +++ b/octodns/provider/gandi.py @@ -41,6 +41,12 @@ class GandiClientNotFound(GandiClientException): super(GandiClientNotFound, self).__init__(r.text) +class GandiClientUnknownDomainName(GandiClientException): + + def __init__(self, msg): + super(GandiClientUnknownDomainName, self).__init__(msg) + + class GandiClient(object): def __init__(self, token): @@ -63,6 +69,16 @@ class GandiClient(object): r.raise_for_status() return r + def zone(self, zone_name): + return self._request('GET', '/livedns/domains/{}' + .format(zone_name)).json() + + def zone_create(self, zone_name): + return self._request('POST', '/livedns/domains', data={ + 'fqdn': zone_name, + 'zone': {} + }).json() + def zone_records(self, zone_name): records = self._request('GET', '/livedns/domains/{}/records' .format(zone_name)).json() @@ -318,9 +334,25 @@ class GandiProvider(BaseProvider): def _apply(self, plan): desired = plan.desired changes = plan.changes + zone = desired.name[:-1] self.log.debug('_apply: zone=%s, len(changes)=%d', desired.name, len(changes)) + try: + self._client.zone(zone) + except GandiClientNotFound: + self.log.info('_apply: no existing zone, trying to create it') + try: + self._client.zone_create(zone) + self.log.info('_apply: zone has been successfully created') + except GandiClientNotFound: + raise GandiClientUnknownDomainName('This domain is not ' + 'registred at Gandi. ' + 'Please register or ' + 'transfer it here ' + 'to be able to manage its ' + 'DNS zone.') + # Force records deletion to be done before creation in order to avoid # "CNAME record must be the only record" error when an existing CNAME # record is replaced by an A/AAAA record. diff --git a/tests/fixtures/gandi-zone.json b/tests/fixtures/gandi-zone.json new file mode 100644 index 0000000..e132f4c --- /dev/null +++ b/tests/fixtures/gandi-zone.json @@ -0,0 +1,7 @@ +{ + "domain_keys_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/keys", + "fqdn": "unit.tests", + "automatic_snapshots": true, + "domain_records_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records", + "domain_href": "https://api.gandi.net/v5/livedns/domains/unit.tests" +} \ No newline at end of file diff --git a/tests/test_octodns_provider_gandi.py b/tests/test_octodns_provider_gandi.py index 8152fcf..7448666 100644 --- a/tests/test_octodns_provider_gandi.py +++ b/tests/test_octodns_provider_gandi.py @@ -86,6 +86,18 @@ class TestGandiProvider(TestCase): provider.populate(zone) self.assertIn('"cause":"Forbidden"', text_type(ctx.exception)) + # 404 - Not Found. + with requests_mock() as mock: + mock.get(ANY, status_code=404, + text='{"code": 404, "message": "The resource could not ' + 'be found.", "object": "HTTPNotFound", "cause": ' + '"Not Found"}') + + with self.assertRaises(GandiClientNotFound) as ctx: + zone = Zone('unit.tests.', []) + provider._client.zone(zone) + self.assertIn('"cause": "Not Found"', text_type(ctx.exception)) + # General error with requests_mock() as mock: mock.get(ANY, status_code=502, text='Things caught fire') @@ -95,15 +107,6 @@ class TestGandiProvider(TestCase): provider.populate(zone) self.assertEquals(502, ctx.exception.response.status_code) - # Non-existent zone doesn't populate anything - with requests_mock() as mock: - mock.get(ANY, status_code=404, - text='{"message": "Domain `foo.bar` not found"}') - - zone = Zone('unit.tests.', []) - provider.populate(zone) - self.assertEquals(set(), zone.records) - # No diffs == no changes with requests_mock() as mock: base = 'https://api.gandi.net/v5/livedns/domains/unit.tests' \ @@ -147,10 +150,14 @@ class TestGandiProvider(TestCase): resp.json = Mock() provider._client._request = Mock(return_value=resp) + with open('tests/fixtures/gandi-zone.json') as fh: + zone = fh.read() + # non-existent domain resp.json.side_effect = [ GandiClientNotFound(resp), # no zone in populate GandiClientNotFound(resp), # no domain during apply + zone ] plan = provider.plan(self.expected) @@ -162,6 +169,11 @@ class TestGandiProvider(TestCase): provider._client._request.assert_has_calls([ call('GET', '/livedns/domains/unit.tests/records'), + call('GET', '/livedns/domains/unit.tests'), + call('POST', '/livedns/domains', data={ + 'fqdn': 'unit.tests', + 'zone': {} + }), call('POST', '/livedns/domains/unit.tests/records', data={ 'rrset_name': 'www.sub', 'rrset_ttl': 300, @@ -258,7 +270,7 @@ class TestGandiProvider(TestCase): }) ]) # expected number of total calls - self.assertEquals(14, provider._client._request.call_count) + self.assertEquals(16, provider._client._request.call_count) provider._client._request.reset_mock() From b280449969c5af0fe31eee1c8139e0995e54892f Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Tue, 27 Oct 2020 11:25:55 +0100 Subject: [PATCH 18/45] Add record targets normalizaltion --- octodns/provider/gandi.py | 8 ++++++++ .../{gandi-default-zone.json => gandi-records.json} | 9 +++++++++ tests/test_octodns_provider_gandi.py | 8 ++++---- 3 files changed, 21 insertions(+), 4 deletions(-) rename tests/fixtures/{gandi-default-zone.json => gandi-records.json} (92%) diff --git a/octodns/provider/gandi.py b/octodns/provider/gandi.py index 1f89a80..dcc222d 100644 --- a/octodns/provider/gandi.py +++ b/octodns/provider/gandi.py @@ -87,6 +87,14 @@ class GandiClient(object): if record['rrset_name'] == '@': record['rrset_name'] = '' + # Change relative targets to absolute ones. + if record['rrset_type'] in ['ALIAS', 'CNAME', 'DNAME', 'MX', + 'NS', 'SRV']: + for i, value in enumerate(record['rrset_values']): + if not value.endswith('.'): + record['rrset_values'][i] = '{}.{}.'.format( + value, zone_name) + return records def record_create(self, zone_name, data): diff --git a/tests/fixtures/gandi-default-zone.json b/tests/fixtures/gandi-records.json similarity index 92% rename from tests/fixtures/gandi-default-zone.json rename to tests/fixtures/gandi-records.json index 9b07b8e..01d30f7 100644 --- a/tests/fixtures/gandi-default-zone.json +++ b/tests/fixtures/gandi-records.json @@ -98,5 +98,14 @@ "rrset_values": [ "32128 13 1 6823D9BB1B03DF714DD0EB163E20B341C96D18C0" ] + }, + { + "rrset_type": "CNAME", + "rrset_ttl": 10800, + "rrset_name": "relative", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/relative/CNAME", + "rrset_values": [ + "target" + ] } ] diff --git a/tests/test_octodns_provider_gandi.py b/tests/test_octodns_provider_gandi.py index 7448666..a818919 100644 --- a/tests/test_octodns_provider_gandi.py +++ b/tests/test_octodns_provider_gandi.py @@ -126,19 +126,19 @@ class TestGandiProvider(TestCase): with requests_mock() as mock: base = 'https://api.gandi.net/v5/livedns/domains/unit.tests' \ '/records' - with open('tests/fixtures/gandi-default-zone.json') as fh: + with open('tests/fixtures/gandi-records.json') as fh: mock.get(base, text=fh.read()) zone = Zone('unit.tests.', []) provider.populate(zone) - self.assertEquals(10, len(zone.records)) + self.assertEquals(11, len(zone.records)) changes = self.expected.changes(zone, provider) - self.assertEquals(22, len(changes)) + self.assertEquals(23, len(changes)) # 2nd populate makes no network calls/all from cache again = Zone('unit.tests.', []) provider.populate(again) - self.assertEquals(10, len(again.records)) + self.assertEquals(11, len(again.records)) # bust the cache del provider._zone_records[zone.name] From eec4c4f81c100c44e3f3550ecae6c541394068c6 Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Tue, 27 Oct 2020 20:31:57 +0100 Subject: [PATCH 19/45] Remove support for dynamic DNAME records as no provider currently support them --- tests/config/dynamic.tests.yaml | 34 ---------- tests/config/split/dynamic.tests./dname.yaml | 42 ------------ tests/test_octodns_provider_yaml.py | 24 +++---- tests/test_octodns_record.py | 67 -------------------- 4 files changed, 10 insertions(+), 157 deletions(-) delete mode 100644 tests/config/split/dynamic.tests./dname.yaml diff --git a/tests/config/dynamic.tests.yaml b/tests/config/dynamic.tests.yaml index 5595a6d..4bd97a7 100644 --- a/tests/config/dynamic.tests.yaml +++ b/tests/config/dynamic.tests.yaml @@ -109,40 +109,6 @@ cname: - pool: iad type: CNAME value: target.unit.tests. -dname: - dynamic: - pools: - ams: - values: - - value: target-ams.unit.tests. - iad: - values: - - value: target-iad.unit.tests. - lax: - values: - - value: target-lax.unit.tests. - sea: - values: - - value: target-sea-1.unit.tests. - weight: 10 - - value: target-sea-2.unit.tests. - weight: 14 - rules: - - geos: - - EU-GB - pool: lax - - geos: - - EU - pool: ams - - geos: - - NA-US-CA - - NA-US-NC - - NA-US-OR - - NA-US-WA - pool: sea - - pool: iad - type: DNAME - value: target.unit.tests. real-ish-a: dynamic: pools: diff --git a/tests/config/split/dynamic.tests./dname.yaml b/tests/config/split/dynamic.tests./dname.yaml deleted file mode 100644 index 45c33fe..0000000 --- a/tests/config/split/dynamic.tests./dname.yaml +++ /dev/null @@ -1,42 +0,0 @@ ---- -dname: - dynamic: - pools: - ams: - fallback: null - values: - - value: target-ams.unit.tests. - weight: 1 - iad: - fallback: null - values: - - value: target-iad.unit.tests. - weight: 1 - lax: - fallback: null - values: - - value: target-lax.unit.tests. - weight: 1 - sea: - fallback: null - values: - - value: target-sea-1.unit.tests. - weight: 10 - - value: target-sea-2.unit.tests. - weight: 14 - rules: - - geos: - - EU-GB - pool: lax - - geos: - - EU - pool: ams - - geos: - - NA-US-CA - - NA-US-NC - - NA-US-OR - - NA-US-WA - pool: sea - - pool: iad - type: DNAME - value: target.unit.tests. diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 7b285ec..15e90da 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -38,7 +38,7 @@ class TestYamlProvider(TestCase): self.assertEquals(19, len(zone.records)) source.populate(dynamic_zone) - self.assertEquals(6, len(dynamic_zone.records)) + self.assertEquals(5, len(dynamic_zone.records)) # Assumption here is that a clean round-trip means that everything # worked as expected, data that went in came back out and could be @@ -68,11 +68,11 @@ class TestYamlProvider(TestCase): # Dynamic plan plan = target.plan(dynamic_zone) - self.assertEquals(6, len([c for c in plan.changes + self.assertEquals(5, len([c for c in plan.changes if isinstance(c, Create)])) self.assertFalse(isfile(dynamic_yaml_file)) # Apply it - self.assertEquals(6, target.apply(plan)) + self.assertEquals(5, target.apply(plan)) self.assertTrue(isfile(dynamic_yaml_file)) # There should be no changes after the round trip @@ -137,10 +137,6 @@ class TestYamlProvider(TestCase): self.assertTrue('value' in dyna) # self.assertTrue('dynamic' in dyna) - dyna = data.pop('dname') - self.assertTrue('value' in dyna) - # self.assertTrue('dynamic' in dyna) - dyna = data.pop('real-ish-a') self.assertTrue('values' in dyna) # self.assertTrue('dynamic' in dyna) @@ -245,7 +241,7 @@ class TestSplitYamlProvider(TestCase): self.assertEquals(19, len(zone.records)) source.populate(dynamic_zone) - self.assertEquals(6, len(dynamic_zone.records)) + self.assertEquals(5, len(dynamic_zone.records)) with TemporaryDirectory() as td: # Add some subdirs to make sure that it can create them @@ -265,11 +261,11 @@ class TestSplitYamlProvider(TestCase): # Dynamic plan plan = target.plan(dynamic_zone) - self.assertEquals(6, len([c for c in plan.changes + self.assertEquals(5, len([c for c in plan.changes if isinstance(c, Create)])) self.assertFalse(isdir(dynamic_zone_dir)) # Apply it - self.assertEquals(6, target.apply(plan)) + self.assertEquals(5, target.apply(plan)) self.assertTrue(isdir(dynamic_zone_dir)) # There should be no changes after the round trip @@ -327,7 +323,7 @@ class TestSplitYamlProvider(TestCase): self.assertTrue('dynamic' in dyna) # Singular again. - for record_name in ('cname', 'dname', 'simple-weighted'): + for record_name in ('cname', 'simple-weighted'): yaml_file = join( dynamic_zone_dir, '{}.yaml'.format(record_name)) self.assertTrue(isfile(yaml_file)) @@ -391,8 +387,8 @@ class TestOverridingYamlProvider(TestCase): # Load the base, should see the 5 records base.populate(zone) got = {r.name: r for r in zone.records} - self.assertEquals(6, len(got)) - # We get the "dynamic" A from the bae config + self.assertEquals(5, len(got)) + # We get the "dynamic" A from the base config self.assertTrue('dynamic' in got['a'].data) # No added self.assertFalse('added' in got) @@ -400,7 +396,7 @@ class TestOverridingYamlProvider(TestCase): # Load the overrides, should replace one and add 1 override.populate(zone) got = {r.name: r for r in zone.records} - self.assertEquals(7, len(got)) + self.assertEquals(6, len(got)) # 'a' was replaced with a generic record self.assertEquals({ 'ttl': 3600, diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 10e9575..524f8f2 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -2670,73 +2670,6 @@ class TestDynamicRecords(TestCase): self.assertTrue(rules) self.assertEquals(cname_data['dynamic']['rules'][0], rules[0].data) - def test_simple_dname_weighted(self): - dname_data = { - 'dynamic': { - 'pools': { - 'one': { - 'values': [{ - 'value': 'one.dname.target.', - }], - }, - 'two': { - 'values': [{ - 'value': 'two.dname.target.', - }], - }, - 'three': { - 'values': [{ - 'weight': 12, - 'value': 'three-1.dname.target.', - }, { - 'weight': 32, - 'value': 'three-2.dname.target.', - }] - }, - }, - 'rules': [{ - 'geos': ['AF', 'EU'], - 'pool': 'three', - }, { - 'geos': ['NA-US-CA'], - 'pool': 'two', - }, { - 'pool': 'one', - }], - }, - 'ttl': 60, - 'value': 'dname.target.', - } - dname = DnameRecord(self.zone, 'weighted', dname_data) - self.assertEquals('DNAME', dname._type) - self.assertEquals(dname_data['ttl'], dname.ttl) - self.assertEquals(dname_data['value'], dname.value) - - dynamic = dname.dynamic - self.assertTrue(dynamic) - - pools = dynamic.pools - self.assertTrue(pools) - self.assertEquals({ - 'value': 'one.dname.target.', - 'weight': 1, - }, pools['one'].data['values'][0]) - self.assertEquals({ - 'value': 'two.dname.target.', - 'weight': 1, - }, pools['two'].data['values'][0]) - self.assertEquals([{ - 'value': 'three-1.dname.target.', - 'weight': 12, - }, { - 'value': 'three-2.dname.target.', - 'weight': 32, - }], pools['three'].data['values']) - - rules = dynamic.rules - self.assertTrue(rules) - self.assertEquals(dname_data['dynamic']['rules'][0], rules[0].data) - def test_dynamic_validation(self): # Missing pools a_data = { From 3acea0d89d88a7f921429ab77e3efe9cd3fde392 Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Sat, 31 Oct 2020 01:09:37 +0100 Subject: [PATCH 20/45] 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 21/45] 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 22/45] 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 23/45] 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 24/45] 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 25/45] 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 26/45] 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 27/45] 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 28/45] 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 29/45] 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 30/45] 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 31/45] 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 32/45] 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 33/45] 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 34/45] 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 35/45] 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 36/45] 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 19798e3acfc65bc2c2b90495588fc6a57d9f668e Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 2 Nov 2020 07:26:07 -0800 Subject: [PATCH 37/45] Only allow ALIAS on APEX --- octodns/record/__init__.py | 8 +++++ tests/fixtures/constellix-records.json | 37 --------------------- tests/fixtures/dnsmadeeasy-records.json | 14 -------- tests/test_octodns_provider_constellix.py | 12 ++----- tests/test_octodns_provider_dnsmadeeasy.py | 12 ++----- tests/test_octodns_provider_mythicbeasts.py | 4 +-- tests/test_octodns_record.py | 14 ++++++-- 7 files changed, 28 insertions(+), 73 deletions(-) diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index 08ec2ee..d42b576 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -782,6 +782,14 @@ class AliasRecord(_ValueMixin, Record): _type = 'ALIAS' _value_type = AliasValue + @classmethod + def validate(cls, name, fqdn, data): + reasons = [] + if name != '': + reasons.append('non-root ALIAS not allowed') + reasons.extend(super(AliasRecord, cls).validate(name, fqdn, data)) + return reasons + class CaaValue(EqualityTupleMixin): # https://tools.ietf.org/html/rfc6844#page-5 diff --git a/tests/fixtures/constellix-records.json b/tests/fixtures/constellix-records.json index c1f1fb4..689fd53 100644 --- a/tests/fixtures/constellix-records.json +++ b/tests/fixtures/constellix-records.json @@ -523,43 +523,6 @@ "roundRobinFailover": [], "pools": [], "poolsDetail": [] -}, { - "id": 1808603, - "type": "ANAME", - "recordType": "aname", - "name": "sub", - "recordOption": "roundRobin", - "noAnswer": false, - "note": "", - "ttl": 1800, - "gtdRegion": 1, - "parentId": 123123, - "parent": "domain", - "source": "Domain", - "modifiedTs": 1565153387855, - "value": [{ - "value": "aname.unit.tests.", - "disableFlag": false - }], - "roundRobin": [{ - "value": "aname.unit.tests.", - "disableFlag": false - }], - "geolocation": null, - "recordFailover": { - "disabled": false, - "failoverType": 1, - "failoverTypeStr": "Normal (always lowest level)", - "values": [] - }, - "failover": { - "disabled": false, - "failoverType": 1, - "failoverTypeStr": "Normal (always lowest level)", - "values": [] - }, - "pools": [], - "poolsDetail": [] }, { "id": 1808520, "type": "A", diff --git a/tests/fixtures/dnsmadeeasy-records.json b/tests/fixtures/dnsmadeeasy-records.json index 4d3ba64..aefd6ce 100644 --- a/tests/fixtures/dnsmadeeasy-records.json +++ b/tests/fixtures/dnsmadeeasy-records.json @@ -320,20 +320,6 @@ "name": "", "value": "aname.unit.tests.", "id": 11189895, - "type": "ANAME" - }, { - "failover": false, - "monitor": false, - "sourceId": 123123, - "dynamicDns": false, - "failed": false, - "gtdLocation": "DEFAULT", - "hardLink": false, - "ttl": 1800, - "source": 1, - "name": "sub", - "value": "aname", - "id": 11189896, "type": "ANAME" }, { "failover": false, diff --git a/tests/test_octodns_provider_constellix.py b/tests/test_octodns_provider_constellix.py index c19ae29..bc17b50 100644 --- a/tests/test_octodns_provider_constellix.py +++ b/tests/test_octodns_provider_constellix.py @@ -42,12 +42,6 @@ class TestConstellixProvider(TestCase): 'value': 'aname.unit.tests.' })) - expected.add_record(Record.new(expected, 'sub', { - 'ttl': 1800, - 'type': 'ALIAS', - 'value': 'aname.unit.tests.' - })) - for record in list(expected.records): if record.name == 'sub' and record._type == 'NS': expected._remove_record(record) @@ -107,14 +101,14 @@ class TestConstellixProvider(TestCase): zone = Zone('unit.tests.', []) provider.populate(zone) - self.assertEquals(15, len(zone.records)) + self.assertEquals(14, len(zone.records)) changes = self.expected.changes(zone, provider) self.assertEquals(0, len(changes)) # 2nd populate makes no network calls/all from cache again = Zone('unit.tests.', []) provider.populate(again) - self.assertEquals(15, len(again.records)) + self.assertEquals(14, len(again.records)) # bust the cache del provider._zone_records[zone.name] @@ -169,7 +163,7 @@ class TestConstellixProvider(TestCase): }), ]) - self.assertEquals(18, provider._client._request.call_count) + self.assertEquals(17, provider._client._request.call_count) provider._client._request.reset_mock() diff --git a/tests/test_octodns_provider_dnsmadeeasy.py b/tests/test_octodns_provider_dnsmadeeasy.py index 50fa576..0ad059d 100644 --- a/tests/test_octodns_provider_dnsmadeeasy.py +++ b/tests/test_octodns_provider_dnsmadeeasy.py @@ -44,12 +44,6 @@ class TestDnsMadeEasyProvider(TestCase): 'value': 'aname.unit.tests.' })) - expected.add_record(Record.new(expected, 'sub', { - 'ttl': 1800, - 'type': 'ALIAS', - 'value': 'aname.unit.tests.' - })) - for record in list(expected.records): if record.name == 'sub' and record._type == 'NS': expected._remove_record(record) @@ -108,14 +102,14 @@ class TestDnsMadeEasyProvider(TestCase): zone = Zone('unit.tests.', []) provider.populate(zone) - self.assertEquals(15, len(zone.records)) + self.assertEquals(14, len(zone.records)) changes = self.expected.changes(zone, provider) self.assertEquals(0, len(changes)) # 2nd populate makes no network calls/all from cache again = Zone('unit.tests.', []) provider.populate(again) - self.assertEquals(15, len(again.records)) + self.assertEquals(14, len(again.records)) # bust the cache del provider._zone_records[zone.name] @@ -180,7 +174,7 @@ class TestDnsMadeEasyProvider(TestCase): 'port': 30 }), ]) - self.assertEquals(27, provider._client._request.call_count) + self.assertEquals(26, provider._client._request.call_count) provider._client._request.reset_mock() diff --git a/tests/test_octodns_provider_mythicbeasts.py b/tests/test_octodns_provider_mythicbeasts.py index 960bd65..f78cb0b 100644 --- a/tests/test_octodns_provider_mythicbeasts.py +++ b/tests/test_octodns_provider_mythicbeasts.py @@ -171,7 +171,7 @@ class TestMythicBeastsProvider(TestCase): def test_command_generation(self): zone = Zone('unit.tests.', []) - zone.add_record(Record.new(zone, 'prawf-alias', { + zone.add_record(Record.new(zone, '', { 'ttl': 60, 'type': 'ALIAS', 'value': 'alias.unit.tests.', @@ -228,7 +228,7 @@ class TestMythicBeastsProvider(TestCase): ) expected_commands = [ - 'ADD prawf-alias.unit.tests 60 ANAME alias.unit.tests.', + 'ADD unit.tests 60 ANAME alias.unit.tests.', 'ADD prawf-ns.unit.tests 300 NS alias.unit.tests.', 'ADD prawf-ns.unit.tests 300 NS alias2.unit.tests.', 'ADD prawf-a.unit.tests 60 A 1.2.3.4', diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 524f8f2..ffca1d0 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -1710,6 +1710,16 @@ class TestRecordValidation(TestCase): 'value': 'foo.bar.com.', }) + # root only + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'nope', { + 'type': 'ALIAS', + 'ttl': 600, + 'value': 'foo.bar.com.', + }) + self.assertEquals(['non-root ALIAS not allowed'], + ctx.exception.reasons) + # missing value with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, '', { @@ -1720,7 +1730,7 @@ class TestRecordValidation(TestCase): # missing value with self.assertRaises(ValidationError) as ctx: - Record.new(self.zone, 'www', { + Record.new(self.zone, '', { 'type': 'ALIAS', 'ttl': 600, 'value': None @@ -1729,7 +1739,7 @@ class TestRecordValidation(TestCase): # empty value with self.assertRaises(ValidationError) as ctx: - Record.new(self.zone, 'www', { + Record.new(self.zone, '', { 'type': 'ALIAS', 'ttl': 600, 'value': '' From 364b70048fbf91251438db76bed3148660cf7a83 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 2 Nov 2020 07:27:48 -0800 Subject: [PATCH 38/45] Fix coverage pragma grep --- script/coverage | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/coverage b/script/coverage index 32bdaea..bd6e4c9 100755 --- a/script/coverage +++ b/script/coverage @@ -27,7 +27,7 @@ export DYN_USERNAME= export GOOGLE_APPLICATION_CREDENTIALS= # Don't allow disabling coverage -grep -r -I --line-number "# pragma: nocover" octodns && { +grep -r -I --line-number "# pragma: +no.*cover" octodns && { echo "Code coverage should not be disabled" exit 1 } From dc9dc45ae638e3ae5384ef4583c722e2b4efdbc9 Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Mon, 2 Nov 2020 18:00:48 +0100 Subject: [PATCH 39/45] Fixes tests after merging of #620 --- tests/fixtures/gandi-no-changes.json | 9 +++++++++ tests/test_octodns_provider_gandi.py | 12 +++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/tests/fixtures/gandi-no-changes.json b/tests/fixtures/gandi-no-changes.json index 1154628..b018785 100644 --- a/tests/fixtures/gandi-no-changes.json +++ b/tests/fixtures/gandi-no-changes.json @@ -46,6 +46,15 @@ "unit.tests." ] }, + { + "rrset_type": "DNAME", + "rrset_ttl": 300, + "rrset_name": "dname", + "rrset_href": "https://api.gandi.net/v5/livedns/domains/unit.tests/records/dname/DNAME", + "rrset_values": [ + "unit.tests." + ] + }, { "rrset_type": "CNAME", "rrset_ttl": 3600, diff --git a/tests/test_octodns_provider_gandi.py b/tests/test_octodns_provider_gandi.py index a818919..3cee392 100644 --- a/tests/test_octodns_provider_gandi.py +++ b/tests/test_octodns_provider_gandi.py @@ -116,7 +116,7 @@ class TestGandiProvider(TestCase): zone = Zone('unit.tests.', []) provider.populate(zone) - self.assertEquals(13, len(zone.records)) + self.assertEquals(14, len(zone.records)) changes = self.expected.changes(zone, provider) self.assertEquals(0, len(changes)) @@ -133,7 +133,7 @@ class TestGandiProvider(TestCase): provider.populate(zone) self.assertEquals(11, len(zone.records)) changes = self.expected.changes(zone, provider) - self.assertEquals(23, len(changes)) + self.assertEquals(24, len(changes)) # 2nd populate makes no network calls/all from cache again = Zone('unit.tests.', []) @@ -226,6 +226,12 @@ class TestGandiProvider(TestCase): 'rrset_type': 'CNAME', 'rrset_values': ['unit.tests.'] }), + call('POST', '/livedns/domains/unit.tests/records', data={ + 'rrset_name': 'dname', + 'rrset_ttl': 300, + 'rrset_type': 'DNAME', + 'rrset_values': ['unit.tests.'] + }), call('POST', '/livedns/domains/unit.tests/records', data={ 'rrset_name': 'cname', 'rrset_ttl': 300, @@ -270,7 +276,7 @@ class TestGandiProvider(TestCase): }) ]) # expected number of total calls - self.assertEquals(16, provider._client._request.call_count) + self.assertEquals(17, provider._client._request.call_count) provider._client._request.reset_mock() From 05ce1344546c3f959912dedeaf5231d894543ef2 Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Mon, 2 Nov 2020 18:02:15 +0100 Subject: [PATCH 40/45] Add tests for zone creation --- tests/test_octodns_provider_gandi.py | 33 +++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/tests/test_octodns_provider_gandi.py b/tests/test_octodns_provider_gandi.py index 3cee392..5871cc9 100644 --- a/tests/test_octodns_provider_gandi.py +++ b/tests/test_octodns_provider_gandi.py @@ -14,7 +14,8 @@ from unittest import TestCase from octodns.record import Record from octodns.provider.gandi import GandiProvider, GandiClientBadRequest, \ - GandiClientUnauthorized, GandiClientForbidden, GandiClientNotFound + GandiClientUnauthorized, GandiClientForbidden, GandiClientNotFound, \ + GandiClientUnknownDomainName from octodns.provider.yaml import YamlProvider from octodns.zone import Zone @@ -146,6 +147,36 @@ class TestGandiProvider(TestCase): def test_apply(self): provider = GandiProvider('test_id', 'token') + # Zone does not exists but can be created. + with requests_mock() as mock: + mock.get(ANY, status_code=404, + text='{"code": 404, "message": "The resource could not ' + 'be found.", "object": "HTTPNotFound", "cause": ' + '"Not Found"}') + mock.post(ANY, status_code=201, + text='{"message": "Domain Created"}') + + plan = provider.plan(self.expected) + provider.apply(plan) + + # Zone does not exists and can't be created. + with requests_mock() as mock: + mock.get(ANY, status_code=404, + text='{"code": 404, "message": "The resource could not ' + 'be found.", "object": "HTTPNotFound", "cause": ' + '"Not Found"}') + mock.post(ANY, status_code=404, + text='{"code": 404, "message": "The resource could not ' + 'be found.", "object": "HTTPNotFound", "cause": ' + '"Not Found"}') + + with self.assertRaises((GandiClientNotFound, + GandiClientUnknownDomainName)) as ctx: + plan = provider.plan(self.expected) + provider.apply(plan) + self.assertIn('This domain is not registred at Gandi.', + text_type(ctx.exception)) + resp = Mock() resp.json = Mock() provider._client._request = Mock(return_value=resp) From 6ebe0858811664e612bbc8eb66aa779bf79048ea Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Mon, 2 Nov 2020 18:04:45 +0100 Subject: [PATCH 41/45] Add GandiProvider to README --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 23ac0e8..6c0982e 100644 --- a/README.md +++ b/README.md @@ -189,6 +189,7 @@ The above command pulled the existing data out of Route53 and placed the results | [EasyDNSProvider](/octodns/provider/easydns.py) | | A, AAAA, CAA, CNAME, MX, NAPTR, NS, SRV, TXT | No | | | [EtcHostsProvider](/octodns/provider/etc_hosts.py) | | A, AAAA, ALIAS, CNAME | No | | | [EnvVarSource](/octodns/source/envvar.py) | | TXT | No | read-only environment variable injection | +| [GandiProvider](/octodns/provider/gandi.py) | | A, AAAA, ALIAS, CAA, CNAME, DNAME, MX, NS, PTR, SPF, SRV, SSHFP, TXT | No | | | [GoogleCloudProvider](/octodns/provider/googlecloud.py) | google-cloud-dns | A, AAAA, CAA, CNAME, MX, NAPTR, NS, PTR, SPF, SRV, TXT | No | | | [MythicBeastsProvider](/octodns/provider/mythicbeasts.py) | Mythic Beasts | A, AAAA, ALIAS, CNAME, MX, NS, SRV, SSHFP, CAA, TXT | No | | | [Ns1Provider](/octodns/provider/ns1.py) | ns1-python | All | Yes | Missing `NA` geo target | From bb7a1a43b7e96cb03e712a3c7302292d5ccd72ce Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Mon, 2 Nov 2020 18:42:03 +0100 Subject: [PATCH 42/45] 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 From b017f90c669d17113ec53db055fbc3ad2ba02aee Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Tue, 3 Nov 2020 10:27:31 -0800 Subject: [PATCH 43/45] Add some docs around lenient and its uses --- docs/records.md | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/docs/records.md b/docs/records.md index a28e86f..74c0b1c 100644 --- a/docs/records.md +++ b/docs/records.md @@ -6,15 +6,17 @@ OctoDNS supports the following record types: * `A` * `AAAA` +* `ALIAS` +* `CAA` * `CNAME` * `DNAME` * `MX` * `NAPTR` * `NS` * `PTR` -* `SSHFP` * `SPF` * `SRV` +* `SSHFP` * `TXT` Underlying provider support for each of these varies and some providers have extra requirements or limitations. In cases where a record type is not supported by a provider OctoDNS will ignore it there and continue to manage the record elsewhere. For example `SSHFP` is supported by Dyn, but not Route53. If your source data includes an SSHFP record OctoDNS will keep it in sync on Dyn, but not consider it when evaluating the state of Route53. The best way to find out what types are supported by a provider is to look for its `supports` method. If that method exists the logic will drive which records are supported and which are ignored. If the provider does not implement the method it will fall back to `BaseProvider.supports` which indicates full support. @@ -82,3 +84,37 @@ In the above example each name had a single record, but there are cases where a Each record type has a corresponding set of required data. The easiest way to determine what's required is probably to look at the record object in [`octodns/record/__init__.py`](/octodns/record/__init__.py). You may also utilize `octodns-validate` which will throw errors about what's missing when run. `type` is required for all records. `ttl` is optional. When TTL is not specified the `YamlProvider`'s default will be used. In any situation where an array of `values` can be used you can opt to go with `value` as a single item if there's only one. + +### Lenience + +octoDNS is fairly strict in terms of standards compliance and is opinionated in terms of best practices. Examples of former include SRV record naming requirements and the latter that ALIAS records are constrained to the root of zones. The strictness and support of providers varies so you may encounter existing records that fail validation when you try to dump them or you may even have use cases for which you need to create or preserve records that don't validate. octoDNS's solution to this is the `lenient` flag. + +#### octodns-dump + +If you're trying to import a zone into octoDNS config file using `octodns-dump` which fails due to validation errors you can supply the `--lenient` argument to tell octoDNS that you acknowledge that things aren't lining up with its expectations, but you'd like it to go ahead anyway. This will do its best to populate the zone and dump the results out into an octoDNS zone file and include the non-compliant bits. If you go to use that config file octoDNS will again complain about the validation problems. You can correct them in cases where that makes sense, but if you need to preserve the non-compliant records read on for options. + +#### Record level lenience + +When there are non-compliant records configured in Yaml you can add the following to tell octoDNS to do it's best to proceed with them anyway. If you use `--lenient` above to dump a zone and you'd like to sync it as-is you can mark the problematic records this way. + +```yaml +'not-root': + octodns: + lenient: true + type: ALIAS + values: something.else.com. +``` + +#### Zone level lenience + +If you'd like to enable lenience for a whole zone you can do so with the following, thought it's strongly encouraged to mark things at record level when possible. The most common case where things may need to be done at the zone level is when using something other than `YamlProvider` as a source, e.g. syncing from `Route53Provider` to `Ns1Provider` when there are non-compliant records in the zone in Route53. + +```yaml + non-compliant-zone.com.: + octodns: + lenient: true + sources: + - route53 + targets: + - ns1 +``` From 269e737812247b3763869398342e1fb54c4952a0 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Tue, 3 Nov 2020 10:35:55 -0800 Subject: [PATCH 44/45] Add a caveat emptor clause to lenient doc section. --- docs/records.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/records.md b/docs/records.md index 74c0b1c..d287d8a 100644 --- a/docs/records.md +++ b/docs/records.md @@ -89,6 +89,8 @@ Each record type has a corresponding set of required data. The easiest way to de octoDNS is fairly strict in terms of standards compliance and is opinionated in terms of best practices. Examples of former include SRV record naming requirements and the latter that ALIAS records are constrained to the root of zones. The strictness and support of providers varies so you may encounter existing records that fail validation when you try to dump them or you may even have use cases for which you need to create or preserve records that don't validate. octoDNS's solution to this is the `lenient` flag. +It's best to think of the `lenient` flag as "I know what I'm doing and accept any problems I run across." The main reason being is that some providers may allow the non-compliant setup and others may not. The behavior of the non-compliant records may even vary from one provider to another. Caveat emptor. + #### octodns-dump If you're trying to import a zone into octoDNS config file using `octodns-dump` which fails due to validation errors you can supply the `--lenient` argument to tell octoDNS that you acknowledge that things aren't lining up with its expectations, but you'd like it to go ahead anyway. This will do its best to populate the zone and dump the results out into an octoDNS zone file and include the non-compliant bits. If you go to use that config file octoDNS will again complain about the validation problems. You can correct them in cases where that makes sense, but if you need to preserve the non-compliant records read on for options. From f3e3f19cd3ac8d7910072bda89887e1d6c2d6459 Mon Sep 17 00:00:00 2001 From: Jonathan Leroy Date: Tue, 3 Nov 2020 22:59:39 +0100 Subject: [PATCH 45/45] Suppress previous exceptions before raising GandiClientUnknownDomainName exception --- octodns/provider/gandi.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/octodns/provider/gandi.py b/octodns/provider/gandi.py index dcc222d..84ff291 100644 --- a/octodns/provider/gandi.py +++ b/octodns/provider/gandi.py @@ -354,12 +354,16 @@ class GandiProvider(BaseProvider): self._client.zone_create(zone) self.log.info('_apply: zone has been successfully created') except GandiClientNotFound: - raise GandiClientUnknownDomainName('This domain is not ' - 'registred at Gandi. ' - 'Please register or ' - 'transfer it here ' - 'to be able to manage its ' - 'DNS zone.') + # We suppress existing exception before raising + # GandiClientUnknownDomainName. + e = GandiClientUnknownDomainName('This domain is not ' + 'registred at Gandi. ' + 'Please register or ' + 'transfer it here ' + 'to be able to manage its ' + 'DNS zone.') + e.__cause__ = None + raise e # Force records deletion to be done before creation in order to avoid # "CNAME record must be the only record" error when an existing CNAME