From 9018e796bbeb815c6f44d5275dbe00b18127a464 Mon Sep 17 00:00:00 2001 From: Adam Smith Date: Thu, 23 Nov 2017 22:17:50 -0800 Subject: [PATCH 01/21] migrate from pep8 to pycodestyle #152 --- script/lint | 2 +- setup.cfg | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/script/lint b/script/lint index 91e6d60..431ac4e 100755 --- a/script/lint +++ b/script/lint @@ -17,5 +17,5 @@ fi SOURCES="*.py octodns/*.py octodns/*/*.py tests/*.py" -pep8 --ignore=E221,E241,E251 $SOURCES +pycodestyle --ignore=E221,E241,E251,E722 $SOURCES pyflakes $SOURCES diff --git a/setup.cfg b/setup.cfg index 54e9014..31a0283 100644 --- a/setup.cfg +++ b/setup.cfg @@ -62,7 +62,7 @@ test = coverage mock nose - pep8 + pycodestyle pyflakes requests_mock setuptools>=36.4.0 From ef8d66ff9cc51be0a033c25bf0867053b063760f Mon Sep 17 00:00:00 2001 From: Adam Smith Date: Thu, 23 Nov 2017 21:49:14 -0800 Subject: [PATCH 02/21] Transform @ in Digitalocean API output to zone name --- octodns/provider/digitalocean.py | 9 +++++++-- tests/fixtures/digitalocean-page-2.json | 6 +++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/octodns/provider/digitalocean.py b/octodns/provider/digitalocean.py index 55fa10b..c68e7d6 100644 --- a/octodns/provider/digitalocean.py +++ b/octodns/provider/digitalocean.py @@ -82,18 +82,23 @@ class DigitalOceanClient(object): except KeyError: break - # change any apex record to empty string to match other provider output for record in ret: + # change any apex record to empty string if record['name'] == '@': record['name'] = '' + # change any apex value to zone name + if record['data'] == '@': + record['data'] = zone_name + return ret def record_create(self, zone_name, params): path = '/domains/{}/records'.format(zone_name) - # change empty string to @, DigitalOcean uses @ for apex record names + # change empty name string to @, DO uses @ for apex record names if params['name'] == '': params['name'] = '@' + self._request('POST', path, data=params) def record_delete(self, zone_name, record_id): diff --git a/tests/fixtures/digitalocean-page-2.json b/tests/fixtures/digitalocean-page-2.json index 8b989ae..50f17f9 100644 --- a/tests/fixtures/digitalocean-page-2.json +++ b/tests/fixtures/digitalocean-page-2.json @@ -25,7 +25,7 @@ "id": 11189891, "type": "CNAME", "name": "cname", - "data": "unit.tests", + "data": "@", "priority": null, "port": null, "ttl": 300, @@ -69,7 +69,7 @@ "id": 11189895, "type": "CNAME", "name": "included", - "data": "unit.tests", + "data": "@", "priority": null, "port": null, "ttl": 3600, @@ -86,4 +86,4 @@ "meta": { "total": 21 } -} \ No newline at end of file +} From f50d9b608708ababe3551ff0fb3cb07d3298b597 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 2 Dec 2017 10:12:28 -0800 Subject: [PATCH 03/21] Extract plan from base.py into plan.py --- octodns/manager.py | 3 +- octodns/provider/base.py | 73 +------------------------- octodns/provider/plan.py | 79 +++++++++++++++++++++++++++++ tests/test_octodns_provider_base.py | 3 +- 4 files changed, 84 insertions(+), 74 deletions(-) create mode 100644 octodns/provider/plan.py diff --git a/octodns/manager.py b/octodns/manager.py index 36a3592..e08754c 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -11,7 +11,8 @@ from importlib import import_module from os import environ import logging -from .provider.base import BaseProvider, Plan +from .provider.base import BaseProvider +from .provider.plan import Plan, PlanLogger from .provider.yaml import YamlProvider from .record import Record from .yaml import safe_load diff --git a/octodns/provider/base.py b/octodns/provider/base.py index f6ff1b7..2d4680f 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -7,78 +7,7 @@ from __future__ import absolute_import, division, print_function, \ from ..source.base import BaseSource from ..zone import Zone -from logging import getLogger - - -class UnsafePlan(Exception): - pass - - -class Plan(object): - log = getLogger('Plan') - - MAX_SAFE_UPDATE_PCENT = .3 - MAX_SAFE_DELETE_PCENT = .3 - MIN_EXISTING_RECORDS = 10 - - def __init__(self, existing, desired, changes, - update_pcent_threshold=MAX_SAFE_UPDATE_PCENT, - delete_pcent_threshold=MAX_SAFE_DELETE_PCENT): - self.existing = existing - self.desired = desired - self.changes = changes - self.update_pcent_threshold = update_pcent_threshold - self.delete_pcent_threshold = delete_pcent_threshold - - change_counts = { - 'Create': 0, - 'Delete': 0, - 'Update': 0 - } - for change in changes: - change_counts[change.__class__.__name__] += 1 - self.change_counts = change_counts - - try: - existing_n = len(self.existing.records) - except AttributeError: - existing_n = 0 - - self.log.debug('__init__: Creates=%d, Updates=%d, Deletes=%d' - 'Existing=%d', - self.change_counts['Create'], - self.change_counts['Update'], - self.change_counts['Delete'], existing_n) - - def raise_if_unsafe(self): - # TODO: what is safe really? - if self.existing and \ - len(self.existing.records) >= self.MIN_EXISTING_RECORDS: - - existing_record_count = len(self.existing.records) - update_pcent = self.change_counts['Update'] / existing_record_count - delete_pcent = self.change_counts['Delete'] / existing_record_count - - if update_pcent > self.update_pcent_threshold: - raise UnsafePlan('Too many updates, {} is over {} percent' - '({}/{})'.format( - update_pcent, - self.MAX_SAFE_UPDATE_PCENT * 100, - self.change_counts['Update'], - existing_record_count)) - if delete_pcent > self.delete_pcent_threshold: - raise UnsafePlan('Too many deletes, {} is over {} percent' - '({}/{})'.format( - delete_pcent, - self.MAX_SAFE_DELETE_PCENT * 100, - self.change_counts['Delete'], - existing_record_count)) - - def __repr__(self): - return 'Creates={}, Updates={}, Deletes={}, Existing Records={}' \ - .format(self.change_counts['Create'], self.change_counts['Update'], - self.change_counts['Delete'], - len(self.existing.records)) +from .plan import Plan class BaseProvider(BaseSource): diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py new file mode 100644 index 0000000..cde859f --- /dev/null +++ b/octodns/provider/plan.py @@ -0,0 +1,79 @@ +# +# +# + +from __future__ import absolute_import, division, print_function, \ + unicode_literals + +from logging import getLogger + + +class UnsafePlan(Exception): + pass + + +class Plan(object): + log = getLogger('Plan') + + MAX_SAFE_UPDATE_PCENT = .3 + MAX_SAFE_DELETE_PCENT = .3 + MIN_EXISTING_RECORDS = 10 + + def __init__(self, existing, desired, changes, + update_pcent_threshold=MAX_SAFE_UPDATE_PCENT, + delete_pcent_threshold=MAX_SAFE_DELETE_PCENT): + self.existing = existing + self.desired = desired + self.changes = changes + self.update_pcent_threshold = update_pcent_threshold + self.delete_pcent_threshold = delete_pcent_threshold + + change_counts = { + 'Create': 0, + 'Delete': 0, + 'Update': 0 + } + for change in changes: + change_counts[change.__class__.__name__] += 1 + self.change_counts = change_counts + + try: + existing_n = len(self.existing.records) + except AttributeError: + existing_n = 0 + + self.log.debug('__init__: Creates=%d, Updates=%d, Deletes=%d' + 'Existing=%d', + self.change_counts['Create'], + self.change_counts['Update'], + self.change_counts['Delete'], existing_n) + + def raise_if_unsafe(self): + # TODO: what is safe really? + if self.existing and \ + len(self.existing.records) >= self.MIN_EXISTING_RECORDS: + + existing_record_count = len(self.existing.records) + update_pcent = self.change_counts['Update'] / existing_record_count + delete_pcent = self.change_counts['Delete'] / existing_record_count + + if update_pcent > self.update_pcent_threshold: + raise UnsafePlan('Too many updates, {} is over {} percent' + '({}/{})'.format( + update_pcent, + self.MAX_SAFE_UPDATE_PCENT * 100, + self.change_counts['Update'], + existing_record_count)) + if delete_pcent > self.delete_pcent_threshold: + raise UnsafePlan('Too many deletes, {} is over {} percent' + '({}/{})'.format( + delete_pcent, + self.MAX_SAFE_DELETE_PCENT * 100, + self.change_counts['Delete'], + existing_record_count)) + + def __repr__(self): + return 'Creates={}, Updates={}, Deletes={}, Existing Records={}' \ + .format(self.change_counts['Create'], self.change_counts['Update'], + self.change_counts['Delete'], + len(self.existing.records)) diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index 1bf3fd7..472b008 100644 --- a/tests/test_octodns_provider_base.py +++ b/tests/test_octodns_provider_base.py @@ -9,7 +9,8 @@ from logging import getLogger from unittest import TestCase from octodns.record import Create, Delete, Record, Update -from octodns.provider.base import BaseProvider, Plan, UnsafePlan +from octodns.provider.base import BaseProvider +from octodns.provider.plan import Plan, UnsafePlan from octodns.zone import Zone From b72fba3e35d5877f3ae5d9ae5837bb66a8f8d6f4 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 2 Dec 2017 10:34:32 -0800 Subject: [PATCH 04/21] Move the log plan output to PlanLogger --- octodns/manager.py | 35 +------------------------------ octodns/provider/plan.py | 45 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 35 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index e08754c..c4617aa 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -5,7 +5,6 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals -from StringIO import StringIO from concurrent.futures import ThreadPoolExecutor from importlib import import_module from os import environ @@ -260,39 +259,7 @@ class Manager(object): # plan pairs. plans = [p for f in futures for p in f.result()] - hr = '*************************************************************' \ - '*******************\n' - buf = StringIO() - buf.write('\n') - if plans: - current_zone = None - for target, plan in plans: - if plan.desired.name != current_zone: - current_zone = plan.desired.name - buf.write(hr) - buf.write('* ') - buf.write(current_zone) - buf.write('\n') - buf.write(hr) - - buf.write('* ') - buf.write(target.id) - buf.write(' (') - buf.write(target) - buf.write(')\n* ') - for change in plan.changes: - buf.write(change.__repr__(leader='* ')) - buf.write('\n* ') - - buf.write('Summary: ') - buf.write(plan) - buf.write('\n') - else: - buf.write(hr) - buf.write('No changes were planned\n') - buf.write(hr) - buf.write('\n') - self.log.info(buf.getvalue()) + PlanLogger(self.log).output(plans) if not force: self.log.debug('sync: checking safety') diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index cde859f..ab84a10 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -5,7 +5,8 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals -from logging import getLogger +from StringIO import StringIO +from logging import INFO, getLogger class UnsafePlan(Exception): @@ -77,3 +78,45 @@ class Plan(object): .format(self.change_counts['Create'], self.change_counts['Update'], self.change_counts['Delete'], len(self.existing.records)) + + +class PlanLogger(object): + + def __init__(self, log, level=INFO): + self.log = log + self.level = level + + def output(self, plans): + hr = '*************************************************************' \ + '*******************\n' + buf = StringIO() + buf.write('\n') + if plans: + current_zone = None + for target, plan in plans: + if plan.desired.name != current_zone: + current_zone = plan.desired.name + buf.write(hr) + buf.write('* ') + buf.write(current_zone) + buf.write('\n') + buf.write(hr) + + buf.write('* ') + buf.write(target.id) + buf.write(' (') + buf.write(target) + buf.write(')\n* ') + for change in plan.changes: + buf.write(change.__repr__(leader='* ')) + buf.write('\n* ') + + buf.write('Summary: ') + buf.write(plan) + buf.write('\n') + else: + buf.write(hr) + buf.write('No changes were planned\n') + buf.write(hr) + buf.write('\n') + self.log.log(self.level, buf.getvalue()) From 31e6f99df6e73aedf14aa7a9af86cb935a3ccadf Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 2 Dec 2017 10:37:19 -0800 Subject: [PATCH 05/21] Manager.plan_outputs --- octodns/manager.py | 5 ++++- octodns/provider/plan.py | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/octodns/manager.py b/octodns/manager.py index c4617aa..a737aa4 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -68,6 +68,8 @@ class Manager(object): def __init__(self, config_file, max_workers=None, include_meta=False): self.log.info('__init__: config_file=%s', config_file) + self.plan_outputs = [PlanLogger(self.log)] + # Read our config file with open(config_file, 'r') as fh: self.config = safe_load(fh, enforce_order=False) @@ -259,7 +261,8 @@ class Manager(object): # plan pairs. plans = [p for f in futures for p in f.result()] - PlanLogger(self.log).output(plans) + for output in self.plan_outputs: + output.run(plans) if not force: self.log.debug('sync: checking safety') diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index ab84a10..009202f 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -86,7 +86,7 @@ class PlanLogger(object): self.log = log self.level = level - def output(self, plans): + def run(self, plans): hr = '*************************************************************' \ '*******************\n' buf = StringIO() From 3d0f5aeca0fcce677303d2a4663b859c104cb1b2 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 2 Dec 2017 11:40:55 -0800 Subject: [PATCH 06/21] Config-based plan_output Refactors the provider class lookup and kwarg processing so that it can be reused for plan_output. --- octodns/manager.py | 81 ++++++++++++------- octodns/provider/plan.py | 29 +++++-- tests/config/bad-plan-output-config.yaml | 7 ++ .../config/bad-plan-output-missing-class.yaml | 5 ++ tests/test_octodns_manager.py | 13 +++ tests/test_octodns_plan.py | 19 +++++ 6 files changed, 120 insertions(+), 34 deletions(-) create mode 100644 tests/config/bad-plan-output-config.yaml create mode 100644 tests/config/bad-plan-output-missing-class.yaml create mode 100644 tests/test_octodns_plan.py diff --git a/octodns/manager.py b/octodns/manager.py index a737aa4..d4debf6 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -11,7 +11,7 @@ from os import environ import logging from .provider.base import BaseProvider -from .provider.plan import Plan, PlanLogger +from .provider.plan import Plan from .provider.yaml import YamlProvider from .record import Record from .yaml import safe_load @@ -68,8 +68,6 @@ class Manager(object): def __init__(self, config_file, max_workers=None, include_meta=False): self.log.info('__init__: config_file=%s', config_file) - self.plan_outputs = [PlanLogger(self.log)] - # Read our config file with open(config_file, 'r') as fh: self.config = safe_load(fh, enforce_order=False) @@ -97,23 +95,8 @@ class Manager(object): self.log.exception('Invalid provider class') raise Exception('Provider {} is missing class' .format(provider_name)) - _class = self._get_provider_class(_class) - # Build up the arguments we need to pass to the provider - kwargs = {} - for k, v in provider_config.items(): - try: - if v.startswith('env/'): - try: - env_var = v[4:] - v = environ[env_var] - except KeyError: - self.log.exception('Invalid provider config') - raise Exception('Incorrect provider config, ' - 'missing env var {}' - .format(env_var)) - except AttributeError: - pass - kwargs[k] = v + _class = self._get_named_class('provider', _class) + kwargs = self._build_kwargs(provider_config) try: self.providers[provider_name] = _class(provider_name, **kwargs) except TypeError: @@ -141,20 +124,64 @@ class Manager(object): where = where[piece] self.zone_tree = zone_tree - def _get_provider_class(self, _class): + self.plan_outputs = {} + plan_outputs = manager_config.get('plan_outputs', { + 'logger': { + 'class': 'octodns.provider.plan.PlanLogger', + 'level': 'info' + } + }) + for plan_output_name, plan_output_config in plan_outputs.items(): + try: + _class = plan_output_config.pop('class') + except KeyError: + self.log.exception('Invalid plan_output class') + raise Exception('plan_output {} is missing class' + .format(plan_output_name)) + _class = self._get_named_class('plan_output', _class) + kwargs = self._build_kwargs(plan_output_config) + try: + self.plan_outputs[plan_output_name] = \ + _class(plan_output_name, **kwargs) + except TypeError: + self.log.exception('Invalid plan_output config') + raise Exception('Incorrect plan_output config for {}' + .format(plan_output_name)) + + def _get_named_class(self, _type, _class): try: module_name, class_name = _class.rsplit('.', 1) module = import_module(module_name) except (ImportError, ValueError): - self.log.exception('_get_provider_class: Unable to import ' + self.log.exception('_get_{}_class: Unable to import ' 'module %s', _class) - raise Exception('Unknown provider class: {}'.format(_class)) + raise Exception('Unknown {} class: {}'.format(_type, _class)) try: return getattr(module, class_name) except AttributeError: - self.log.exception('_get_provider_class: Unable to get class %s ' + self.log.exception('_get_{}_class: Unable to get class %s ' 'from module %s', class_name, module) - raise Exception('Unknown provider class: {}'.format(_class)) + raise Exception('Unknown {} class: {}'.format(_type, _class)) + + def _build_kwargs(self, source): + # Build up the arguments we need to pass to the provider + kwargs = {} + for k, v in source.items(): + try: + if v.startswith('env/'): + try: + env_var = v[4:] + v = environ[env_var] + except KeyError: + self.log.exception('Invalid provider config') + raise Exception('Incorrect provider config, ' + 'missing env var {}' + .format(env_var)) + except AttributeError: + pass + kwargs[k] = v + + return kwargs def configured_sub_zones(self, zone_name): # Reversed pieces of the zone name @@ -261,8 +288,8 @@ class Manager(object): # plan pairs. plans = [p for f in futures for p in f.result()] - for output in self.plan_outputs: - output.run(plans) + for output in self.plan_outputs.values(): + output.run(plans=plans, log=self.log) if not force: self.log.debug('sync: checking safety') diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index 009202f..b49c200 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -6,7 +6,7 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals from StringIO import StringIO -from logging import INFO, getLogger +from logging import DEBUG, ERROR, INFO, WARN, getLogger class UnsafePlan(Exception): @@ -80,13 +80,28 @@ class Plan(object): len(self.existing.records)) -class PlanLogger(object): +class _PlanOutput(object): - def __init__(self, log, level=INFO): - self.log = log - self.level = level + def __init__(self, name): + self.name = name - def run(self, plans): + +class PlanLogger(_PlanOutput): + + def __init__(self, name, level='info'): + super(PlanLogger, self).__init__(name) + try: + self.level = { + 'debug': DEBUG, + 'info': INFO, + 'warn': WARN, + 'warning': WARN, + 'error': ERROR + }[level.lower()] + except (AttributeError, KeyError): + raise Exception('Unsupported level: {}'.format(level)) + + def run(self, log, plans, *args, **kwargs): hr = '*************************************************************' \ '*******************\n' buf = StringIO() @@ -119,4 +134,4 @@ class PlanLogger(object): buf.write('No changes were planned\n') buf.write(hr) buf.write('\n') - self.log.log(self.level, buf.getvalue()) + log.log(self.level, buf.getvalue()) diff --git a/tests/config/bad-plan-output-config.yaml b/tests/config/bad-plan-output-config.yaml new file mode 100644 index 0000000..f345f89 --- /dev/null +++ b/tests/config/bad-plan-output-config.yaml @@ -0,0 +1,7 @@ +manager: + plan_outputs: + 'bad': + class: octodns.provider.plan.PlanLogger + invalid: config +providers: {} +zones: {} diff --git a/tests/config/bad-plan-output-missing-class.yaml b/tests/config/bad-plan-output-missing-class.yaml new file mode 100644 index 0000000..71b1bd5 --- /dev/null +++ b/tests/config/bad-plan-output-missing-class.yaml @@ -0,0 +1,5 @@ +manager: + plan_outputs: + 'bad': {} +providers: {} +zones: {} diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 4db2103..ada54e5 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -83,6 +83,19 @@ class TestManager(TestCase): .sync(['unknown.target.']) self.assertTrue('unknown target' in ctx.exception.message) + def test_bad_plan_output_class(self): + with self.assertRaises(Exception) as ctx: + name = 'bad-plan-output-missing-class.yaml' + Manager(get_config_filename(name)).sync() + self.assertEquals('plan_output bad is missing class', + ctx.exception.message) + + def test_bad_plan_output_config(self): + with self.assertRaises(Exception) as ctx: + Manager(get_config_filename('bad-plan-output-config.yaml')).sync() + self.assertEqual('Incorrect plan_output config for bad', + ctx.exception.message) + def test_source_only_as_a_target(self): with self.assertRaises(Exception) as ctx: Manager(get_config_filename('unknown-provider.yaml')) \ diff --git a/tests/test_octodns_plan.py b/tests/test_octodns_plan.py new file mode 100644 index 0000000..2b23b4e --- /dev/null +++ b/tests/test_octodns_plan.py @@ -0,0 +1,19 @@ +# +# +# + +from __future__ import absolute_import, division, print_function, \ + unicode_literals + +from unittest import TestCase + +from octodns.provider.plan import PlanLogger + + +class TestPlanLogger(TestCase): + + def test_invalid_level(self): + with self.assertRaises(Exception) as ctx: + PlanLogger('invalid', 'not-a-level') + self.assertEquals('Unsupported level: not-a-level', + ctx.exception.message) From 1a9d8541adece1444e5d56d69bfa7005c8d5324e Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 2 Dec 2017 11:45:11 -0800 Subject: [PATCH 07/21] Only ignore config/ at the top --- .gitignore | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index c45a684..8fcf7ea 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,8 @@ *.pyc .coverage .env +^config/ +build/ coverage.xml dist/ env/ @@ -9,5 +11,3 @@ nosetests.xml octodns.egg-info/ output/ tmp/ -build/ -config/ From 402bc2092e63eb10ff6dee76e480ecb55d64e2c2 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 2 Dec 2017 12:35:18 -0800 Subject: [PATCH 08/21] WIP: markdown plan_output support Mostly works, but doesn't yet dump out the values --- octodns/provider/plan.py | 63 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index b49c200..c7776d8 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -7,6 +7,7 @@ from __future__ import absolute_import, division, print_function, \ from StringIO import StringIO from logging import DEBUG, ERROR, INFO, WARN, getLogger +from sys import stdout class UnsafePlan(Exception): @@ -135,3 +136,65 @@ class PlanLogger(_PlanOutput): buf.write(hr) buf.write('\n') log.log(self.level, buf.getvalue()) + + +class PlanMarkdown(_PlanOutput): + + def run(self, plans, *args, **kwargs): + fh = stdout + if plans: + current_zone = None + for target, plan in plans: + if plan.desired.name != current_zone: + current_zone = plan.desired.name + fh.write('## ') + fh.write(current_zone) + fh.write('\n\n') + + fh.write('### ') + fh.write(target.id) + fh.write('\n\n') + + fh.write('| Name | Type | Existing TTL | New TTL | ' + 'Existing Value | New Value| Source |\n' + '|--|--|--|--|--|--|--|\n') + + for change in plan.changes: + existing = change.existing + new = change.new + record = change.record + fh.write('| ') + fh.write(record.name) + fh.write(' | ') + fh.write(record._type) + fh.write(' | ') + # TTL + if existing: + fh.write(str(existing.ttl)) + else: + fh.write('n/a') + fh.write(' | ') + if new: + fh.write(str(new.ttl)) + else: + fh.write('n/a') + fh.write(' | ') + # Value + if existing: + fh.write('todo') + else: + fh.write('n/a') + fh.write(' | ') + if new: + fh.write('todo') + fh.write(' | ') + fh.write(new.source.id) + else: + fh.write('n/a') + fh.write(' |\n') + + fh.write('\nSummary: ') + fh.write(str(plan)) + fh.write('\n\n') + else: + fh.write('## No changes were planned\n') From 7d83ae84c775ffa03b32f4b19995933affdcfd58 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 2 Dec 2017 15:21:28 -0800 Subject: [PATCH 09/21] Rework markdown table to use 2-rows for updates, 1 for create/delete --- octodns/provider/plan.py | 44 ++++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index c7776d8..1795a65 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -155,15 +155,16 @@ class PlanMarkdown(_PlanOutput): fh.write(target.id) fh.write('\n\n') - fh.write('| Name | Type | Existing TTL | New TTL | ' - 'Existing Value | New Value| Source |\n' - '|--|--|--|--|--|--|--|\n') + fh.write('| Operation | Name | Type | TTL | Value | Source |\n' + '|--|--|--|--|--|--|\n') for change in plan.changes: existing = change.existing new = change.new record = change.record fh.write('| ') + fh.write(change.__class__.__name__) + fh.write(' | ') fh.write(record.name) fh.write(' | ') fh.write(record._type) @@ -171,27 +172,30 @@ class PlanMarkdown(_PlanOutput): # TTL if existing: fh.write(str(existing.ttl)) - else: - fh.write('n/a') - fh.write(' | ') + fh.write(' | ') + if existing: + try: + v = existing.values + except AttributeError: + v = existing.value + fh.write(str(v)) + else: + fh.write('n/a') + fh.write(' | |\n') + if new: + fh.write('| | | | ') + if new: fh.write(str(new.ttl)) - else: - fh.write('n/a') - fh.write(' | ') - # Value - if existing: - fh.write('todo') - else: - fh.write('n/a') - fh.write(' | ') - if new: - fh.write('todo') + fh.write(' | ') + try: + v = new.values + except AttributeError: + v = new.value + fh.write(str(v)) fh.write(' | ') fh.write(new.source.id) - else: - fh.write('n/a') - fh.write(' |\n') + fh.write(' |\n') fh.write('\nSummary: ') fh.write(str(plan)) From aa20b3388f1e0621a9596d3489950a207c7b4874 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 2 Dec 2017 15:39:29 -0800 Subject: [PATCH 10/21] plan _value_stringifier --- octodns/provider/plan.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index 1795a65..6291a87 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -138,6 +138,17 @@ class PlanLogger(_PlanOutput): log.log(self.level, buf.getvalue()) +def _value_stringifier(record, sep): + try: + values = [str(v) for v in record.values] + except AttributeError: + values = [record.value] + for code, gv in getattr(record, 'geo', {}).items(): + vs = ', '.join([str(v) for v in gv.values]) + values.append('{}: {}'.format(code, vs)) + return sep.join(values) + + class PlanMarkdown(_PlanOutput): def run(self, plans, *args, **kwargs): @@ -174,11 +185,7 @@ class PlanMarkdown(_PlanOutput): fh.write(str(existing.ttl)) fh.write(' | ') if existing: - try: - v = existing.values - except AttributeError: - v = existing.value - fh.write(str(v)) + fh.write(_value_stringifier(existing, '; ')) else: fh.write('n/a') fh.write(' | |\n') @@ -188,11 +195,7 @@ class PlanMarkdown(_PlanOutput): if new: fh.write(str(new.ttl)) fh.write(' | ') - try: - v = new.values - except AttributeError: - v = new.value - fh.write(str(v)) + fh.write(_value_stringifier(new, '; ')) fh.write(' | ') fh.write(new.source.id) fh.write(' |\n') From e092afda17b182a93d491857304fcf684454860b Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 2 Dec 2017 15:58:47 -0800 Subject: [PATCH 11/21] Add PlanHtml --- .gitignore | 3 +- octodns/provider/plan.py | 71 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 66 insertions(+), 8 deletions(-) diff --git a/.gitignore b/.gitignore index 8fcf7ea..64ce76f 100644 --- a/.gitignore +++ b/.gitignore @@ -1,8 +1,7 @@ *.pyc .coverage .env -^config/ -build/ +/config/ coverage.xml dist/ env/ diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index 6291a87..9ac7685 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -151,8 +151,7 @@ def _value_stringifier(record, sep): class PlanMarkdown(_PlanOutput): - def run(self, plans, *args, **kwargs): - fh = stdout + def run(self, plans, fh=stdout, *args, **kwargs): if plans: current_zone = None for target, plan in plans: @@ -184,10 +183,7 @@ class PlanMarkdown(_PlanOutput): if existing: fh.write(str(existing.ttl)) fh.write(' | ') - if existing: - fh.write(_value_stringifier(existing, '; ')) - else: - fh.write('n/a') + fh.write(_value_stringifier(existing, '; ')) fh.write(' | |\n') if new: fh.write('| | | | ') @@ -205,3 +201,66 @@ class PlanMarkdown(_PlanOutput): fh.write('\n\n') else: fh.write('## No changes were planned\n') + + +class PlanHtml(_PlanOutput): + + def run(self, plans, fh=stdout, *args, **kwargs): + if plans: + current_zone = None + for target, plan in plans: + if plan.desired.name != current_zone: + current_zone = plan.desired.name + fh.write('

') + fh.write(current_zone) + fh.write('

\n') + + fh.write('

') + fh.write(target.id) + fh.write('''

+ + + + + + + + + +''') + + for change in plan.changes: + existing = change.existing + new = change.new + record = change.record + fh.write(' \n \n \n \n') + # TTL + if existing: + fh.write(' \n \n \n \n') + if new: + fh.write(' \n \n') + + if new: + fh.write(' \n \n \n \n') + + fh.write(' \n \n \n
OperationNameTypeTTLValueSource
') + fh.write(change.__class__.__name__) + fh.write('') + fh.write(record.name) + fh.write('') + fh.write(record._type) + fh.write('') + fh.write(str(existing.ttl)) + fh.write('') + fh.write(_value_stringifier(existing, '
')) + fh.write('
') + fh.write(str(new.ttl)) + fh.write('') + fh.write(_value_stringifier(new, '
')) + fh.write('		') + fh.write(new.source.id) + fh.write('
Summary: ') + fh.write(str(plan)) + fh.write('
\n') + else: + fh.write('

No changes were planned

') From 106f59fe6cb88e50921b32bbbc8930f3d0912259 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 2 Dec 2017 16:00:00 -0800 Subject: [PATCH 12/21] No table border --- octodns/provider/plan.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index 9ac7685..697c8c3 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -218,7 +218,7 @@ class PlanHtml(_PlanOutput): fh.write('

') fh.write(target.id) fh.write('''

- +
From fd9af2bd25ff6a38344773d507f4cfadd21da3a6 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 27 Dec 2017 09:54:18 -0800 Subject: [PATCH 13/21] Major reworking of Cloudflare record update --- octodns/provider/cloudflare.py | 107 +++++++++++++++++++--- tests/test_octodns_provider_cloudflare.py | 13 ++- 2 files changed, 101 insertions(+), 19 deletions(-) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index dd53b3a..a7ec5d8 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -7,6 +7,7 @@ from __future__ import absolute_import, division, print_function, \ from collections import defaultdict from logging import getLogger +from json import dumps from requests import Session from ..record import Record, Update @@ -232,25 +233,107 @@ class CloudflareProvider(BaseProvider): 'content': value.exchange } + def _gen_contents(self, record): + name = record.fqdn[:-1] + _type = record._type + ttl = max(self.MIN_TTL, record.ttl) + + contents_for = getattr(self, '_contents_for_{}'.format(_type)) + for content in contents_for(record): + content.update({ + 'name': name, + 'type': _type, + 'ttl': ttl, + }) + yield content + def _apply_Create(self, change): new = change.new zone_id = self.zones[new.zone.name] - contents_for = getattr(self, '_contents_for_{}'.format(new._type)) path = '/zones/{}/dns_records'.format(zone_id) - name = new.fqdn[:-1] - for content in contents_for(change.new): - content.update({ - 'name': name, - 'type': new._type, - # Cloudflare has a min ttl of 120s - 'ttl': max(self.MIN_TTL, new.ttl), - }) + for content in self._gen_contents(new): self._request('POST', path, data=content) + def _hash_content(self, content): + # Some of the dicts are nested so this seems about as good as any + # option we have for consistently hashing them (within a single run) + return hash(dumps(content, sort_keys=True)) + def _apply_Update(self, change): - # Create the new and delete the old - self._apply_Create(change) - self._apply_Delete(change) + + # Ugh, this is pretty complicated and ugly, mainly due to the + # sub-optimal API/semantics. Ideally we'd have a batch change API like + # Route53's to make this 100% clean and safe without all this PITA, but + # we don't so we'll have to work around that and manually do it as + # safely as possible. Note this still isn't perfect as we don't/can't + # practically take into account things like the different "types" of + # CAA records so when we "swap" there may be brief periods where things + # are invalid or even worse Cloudflare may update their validations to + # prevent dups. I see no clean way around that short of making this + # understand 100% of the details of each record type and develop an + # individual/specific ordering of changes that prevents it. That'd + # probably result in more code than this whole provider currently has + # so... :-( + + existing_contents = { + self._hash_content(c): c + for c in self._gen_contents(change.existing) + } + new_contents = { + self._hash_content(c): c + for c in self._gen_contents(change.new) + } + + # We need a list of keys to consider for diffs, use the first content + # before we muck with anything + keys = existing_contents.values()[0].keys() + + # Find the things we need to add + adds = [] + for k, content in new_contents.items(): + try: + existing_contents.pop(k) + self.log.debug('_apply_Update: leaving %s', content) + except KeyError: + adds.append(content) + + zone_id = self.zones[change.new.zone.name] + + # Find things we need to remove + name = change.new.fqdn[:-1] + _type = change.new._type + # OK, work through each record from the zone + for record in self.zone_records(change.new.zone): + if name == record['name'] and _type == record['type']: + # This is match for our name and type, we need to look at + # contents now, build a dict of the relevant keys and vals + content = {} + for k in keys: + content[k] = record[k] + # :-( + if _type in ('CNAME', 'MX', 'NS'): + content['content'] += '.' + # If the hash of that dict isn't in new this record isn't + # needed + if self._hash_content(content) not in new_contents: + rid = record['id'] + path = '/zones/{}/dns_records/{}'.format(record['zone_id'], + rid) + try: + add_content = adds.pop(0) + self.log.debug('_apply_Update: swapping %s -> %s, %s', + content, add_content, rid) + self._request('PUT', path, data=add_content) + except IndexError: + self.log.debug('_apply_Update: removing %s, %s', + content, rid) + self._request('DELETE', path) + + # Any remaining adds just need to be created + path = '/zones/{}/dns_records'.format(zone_id) + for content in adds: + self.log.debug('_apply_Update: adding %s', content) + self._request('POST', path, data=content) def _apply_Delete(self, change): existing = change.existing diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index ef8a51c..defcdea 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -267,13 +267,12 @@ class TestCloudflareProvider(TestCase): self.assertEquals(2, provider.apply(plan)) # recreate for update, and deletes for the 2 parts of the other provider._request.assert_has_calls([ - call('POST', '/zones/42/dns_records', data={ - 'content': '3.2.3.4', - 'type': 'A', - 'name': 'ttl.unit.tests', - 'ttl': 300}), - call('DELETE', '/zones/ff12ab34cd5611334422ab3322997650/' - 'dns_records/fc12ab34cd5611334422ab3322997655'), + call('PUT', '/zones/ff12ab34cd5611334422ab3322997650/dns_records/' + 'fc12ab34cd5611334422ab3322997655', + data={'content': '3.2.3.4', + 'type': 'A', + 'name': 'ttl.unit.tests', + 'ttl': 300}), call('DELETE', '/zones/ff12ab34cd5611334422ab3322997650/' 'dns_records/fc12ab34cd5611334422ab3322997653'), call('DELETE', '/zones/ff12ab34cd5611334422ab3322997650/' From 429b447238ad1155d99b7f2123a0f1e8cf018613 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 27 Dec 2017 10:20:01 -0800 Subject: [PATCH 14/21] Route53's NAPTR values should default to '' not None --- octodns/provider/route53.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 7623648..5bda074 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -385,10 +385,10 @@ class Route53Provider(BaseProvider): values.append({ 'order': order, 'preference': preference, - 'flags': flags if flags else None, - 'service': service if service else None, - 'regexp': regexp if regexp else None, - 'replacement': replacement if replacement else None, + 'flags': flags, + 'service': service, + 'regexp': regexp, + 'replacement': replacement, }) return { 'type': rrset['Type'], From ddf53b7a47355e8755e31834db329cc052290127 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 6 Jan 2018 07:50:23 -0800 Subject: [PATCH 15/21] No changes bold, not h2 --- octodns/provider/plan.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index 697c8c3..7edd682 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -263,4 +263,4 @@ class PlanHtml(_PlanOutput): fh.write(str(plan)) fh.write('\n \n
Operation Name
\n') else: - fh.write('

No changes were planned

') + fh.write('No changes were planned') From 5c94407893099098144d0c1487158ccd964ac776 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 6 Jan 2018 08:58:28 -0800 Subject: [PATCH 16/21] All requirements are >=, not exact versions --- setup.cfg | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/setup.cfg b/setup.cfg index 31a0283..4686ecd 100644 --- a/setup.cfg +++ b/setup.cfg @@ -22,12 +22,12 @@ classifiers = install_requires = PyYaml>=3.12 dnspython>=1.15.0 - futures==3.1.1 - incf.countryutils==1.0 - ipaddress==1.0.18 - natsort==5.0.3 - python-dateutil==2.6.1 - requests==2.13.0 + futures>=3.1.1 + incf.countryutils>=1.0 + ipaddress>=1.0.18 + natsort>=5.0.3 + python-dateutil>=2.6.1 + requests>=2.13.0 packages = find: include_package_data = True @@ -45,19 +45,19 @@ exclude = [options.extras_require] dev = - azure-mgmt-dns==1.0.1 - azure-common==1.1.6 - boto3==1.4.6 - botocore==1.6.8 - docutils==0.14 - dyn==1.8.0 - google-cloud==0.27.0 - jmespath==0.9.3 - msrestazure==0.4.10 - nsone==0.9.14 - ovh==0.4.7 - s3transfer==0.1.10 - six==1.10.0 + azure-mgmt-dns>=1.0.1 + azure-common>=1.1.6 + boto3>=1.4.6 + botocore>=1.6.8 + docutils>=0.14 + dyn>=1.8.0 + google-cloud>=0.27.0 + jmespath>=0.9.3 + msrestazure>=0.4.10 + nsone>=0.9.14 + ovh>=0.4.7 + s3transfer>=0.1.10 + six>=1.10.0 test = coverage mock From a7e32d2c87d82b342d85108ac9804727735b5e50 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 6 Jan 2018 10:13:23 -0800 Subject: [PATCH 17/21] Hard-ping azure lib versions, they have breaking changes --- setup.cfg | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/setup.cfg b/setup.cfg index 4686ecd..7f0ce13 100644 --- a/setup.cfg +++ b/setup.cfg @@ -45,15 +45,15 @@ exclude = [options.extras_require] dev = - azure-mgmt-dns>=1.0.1 - azure-common>=1.1.6 + azure-mgmt-dns==1.0.1 + azure-common==1.1.6 boto3>=1.4.6 botocore>=1.6.8 docutils>=0.14 dyn>=1.8.0 google-cloud>=0.27.0 jmespath>=0.9.3 - msrestazure>=0.4.10 + msrestazure==0.4.10 nsone>=0.9.14 ovh>=0.4.7 s3transfer>=0.1.10 From 3c3f63b450e5e66ea8e070bd7055fcc73b91c114 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 6 Jan 2018 15:51:48 -0800 Subject: [PATCH 18/21] Unit tests for reworked Cloudflare updates --- tests/test_octodns_provider_cloudflare.py | 166 +++++++++++++++++++++- 1 file changed, 165 insertions(+), 1 deletion(-) diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index defcdea..8a17083 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -11,7 +11,8 @@ from requests import HTTPError from requests_mock import ANY, mock as requests_mock from unittest import TestCase -from octodns.record import Record +from octodns.record import Record, Update +from octodns.provider.base import Plan from octodns.provider.cloudflare import CloudflareProvider from octodns.provider.yaml import YamlProvider from octodns.zone import Zone @@ -278,3 +279,166 @@ class TestCloudflareProvider(TestCase): call('DELETE', '/zones/ff12ab34cd5611334422ab3322997650/' 'dns_records/fc12ab34cd5611334422ab3322997654') ]) + + def test_update_add_swap(self): + provider = CloudflareProvider('test', 'email', 'token') + + provider.zone_records = Mock(return_value=[ + { + "id": "fc12ab34cd5611334422ab3322997653", + "type": "A", + "name": "a.unit.tests", + "content": "1.1.1.1", + "proxiable": True, + "proxied": False, + "ttl": 300, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + }, + { + "id": "fc12ab34cd5611334422ab3322997654", + "type": "A", + "name": "a.unit.tests", + "content": "2.2.2.2", + "proxiable": True, + "proxied": False, + "ttl": 300, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + }, + ]) + + provider._request = Mock() + provider._request.side_effect = [ + self.empty, # no zones + { + 'result': { + 'id': 42, + } + }, # zone create + None, + None, + ] + + # Add something and delete something + zone = Zone('unit.tests.', []) + existing = Record.new(zone, 'a', { + 'ttl': 300, + 'type': 'A', + # This matches the zone data above, one to swap, one to leave + 'values': ['1.1.1.1', '2.2.2.2'], + }) + new = Record.new(zone, 'a', { + 'ttl': 300, + 'type': 'A', + # This leaves one, swaps ones, and adds one + 'values': ['2.2.2.2', '3.3.3.3', '4.4.4.4'], + }) + change = Update(existing, new) + plan = Plan(zone, zone, [change]) + provider._apply(plan) + + provider._request.assert_has_calls([ + call('GET', '/zones', params={'page': 1}), + call('POST', '/zones', data={'jump_start': False, + 'name': 'unit.tests'}), + call('PUT', '/zones/ff12ab34cd5611334422ab3322997650/dns_records/' + 'fc12ab34cd5611334422ab3322997653', + data={'content': '4.4.4.4', 'type': 'A', 'name': + 'a.unit.tests', 'ttl': 300}), + call('POST', '/zones/42/dns_records', + data={'content': '3.3.3.3', 'type': 'A', + 'name': 'a.unit.tests', 'ttl': 300}) + ]) + + def test_update_delete(self): + # We need another run so that we can delete, we can't both add and + # delete in one go b/c of swaps + provider = CloudflareProvider('test', 'email', 'token') + + provider.zone_records = Mock(return_value=[ + { + "id": "fc12ab34cd5611334422ab3322997653", + "type": "NS", + "name": "unit.tests", + "content": "ns1.foo.bar", + "proxiable": True, + "proxied": False, + "ttl": 300, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + }, + { + "id": "fc12ab34cd5611334422ab3322997654", + "type": "NS", + "name": "unit.tests", + "content": "ns2.foo.bar", + "proxiable": True, + "proxied": False, + "ttl": 300, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + }, + ]) + + provider._request = Mock() + provider._request.side_effect = [ + self.empty, # no zones + { + 'result': { + 'id': 42, + } + }, # zone create + None, + None, + ] + + # Add something and delete something + zone = Zone('unit.tests.', []) + existing = Record.new(zone, '', { + 'ttl': 300, + 'type': 'NS', + # This matches the zone data above, one to delete, one to leave + 'values': ['ns1.foo.bar.', 'ns2.foo.bar.'], + }) + new = Record.new(zone, '', { + 'ttl': 300, + 'type': 'NS', + # This leaves one and deletes one + 'value': 'ns2.foo.bar.', + }) + change = Update(existing, new) + plan = Plan(zone, zone, [change]) + provider._apply(plan) + + provider._request.assert_has_calls([ + call('GET', '/zones', params={'page': 1}), + call('POST', '/zones', + data={'jump_start': False, 'name': 'unit.tests'}), + call('DELETE', '/zones/ff12ab34cd5611334422ab3322997650/' + 'dns_records/fc12ab34cd5611334422ab3322997653') + ]) From ad1d0f0fe83b83891caa39dbc77b48dfa0ca6092 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 6 Jan 2018 16:26:48 -0800 Subject: [PATCH 19/21] Fixes and unit tests for new plan output functionality --- octodns/provider/plan.py | 2 +- tests/test_octodns_plan.py | 75 +++++++++++++++++++++++++++++++++++++- 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index 7edd682..3e86826 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -143,7 +143,7 @@ def _value_stringifier(record, sep): values = [str(v) for v in record.values] except AttributeError: values = [record.value] - for code, gv in getattr(record, 'geo', {}).items(): + for code, gv in sorted(getattr(record, 'geo', {}).items()): vs = ', '.join([str(v) for v in gv.values]) values.append('{}: {}'.format(code, vs)) return sep.join(values) diff --git a/tests/test_octodns_plan.py b/tests/test_octodns_plan.py index 2b23b4e..ea35243 100644 --- a/tests/test_octodns_plan.py +++ b/tests/test_octodns_plan.py @@ -5,9 +5,15 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals +from StringIO import StringIO +from logging import getLogger from unittest import TestCase -from octodns.provider.plan import PlanLogger +from octodns.provider.plan import Plan, PlanHtml, PlanLogger, PlanMarkdown +from octodns.record import Create, Delete, Record, Update +from octodns.zone import Zone + +from helpers import SimpleProvider class TestPlanLogger(TestCase): @@ -17,3 +23,70 @@ class TestPlanLogger(TestCase): PlanLogger('invalid', 'not-a-level') self.assertEquals('Unsupported level: not-a-level', ctx.exception.message) + + +simple = SimpleProvider() +zone = Zone('unit.tests.', []) +existing = Record.new(zone, 'a', { + 'ttl': 300, + 'type': 'A', + # This matches the zone data above, one to swap, one to leave + 'values': ['1.1.1.1', '2.2.2.2'], +}) +new = Record.new(zone, 'a', { + 'geo': { + 'AF': ['5.5.5.5'], + 'NA-US': ['6.6.6.6'] + }, + 'ttl': 300, + 'type': 'A', + # This leaves one, swaps ones, and adds one + 'values': ['2.2.2.2', '3.3.3.3', '4.4.4.4'], +}, simple) +create = Create(Record.new(zone, 'b', { + 'ttl': 60, + 'type': 'CNAME', + 'value': 'foo.unit.tests.' +}, simple)) +update = Update(existing, new) +delete = Delete(new) +changes = [create, delete, update] +plans = [ + (simple, Plan(zone, zone, changes)), + (simple, Plan(zone, zone, changes)), +] + + +class TestPlanHtml(TestCase): + log = getLogger('TestPlanHtml') + + def test_empty(self): + out = StringIO() + PlanHtml('html').run([], fh=out) + self.assertEquals('No changes were planned', out.getvalue()) + + def test_simple(self): + out = StringIO() + PlanHtml('html').run(plans, fh=out) + out = out.getvalue() + self.assertTrue(' Summary: Creates=1, Updates=1, ' + 'Deletes=1, Existing Records=0' in out) + + +class TestPlanMarkdown(TestCase): + log = getLogger('TestPlanMarkdown') + + def test_empty(self): + out = StringIO() + PlanMarkdown('markdown').run([], fh=out) + self.assertEquals('## No changes were planned\n', out.getvalue()) + + def test_simple(self): + out = StringIO() + PlanMarkdown('markdown').run(plans, fh=out) + out = out.getvalue() + self.assertTrue('## unit.tests.' in out) + self.assertTrue('Create | b | CNAME | 60 | foo.unit.tests.' in out) + self.assertTrue('Update | a | A | 300 | 1.1.1.1;' in out) + self.assertTrue('NA-US: 6.6.6.6 | test' in out) + self.assertTrue('Delete | a | A | 300 | 2.2.2.2;' in out) From 0659eda451ed4d8240016ea4f38e21e8ffb1e897 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 6 Jan 2018 16:53:11 -0800 Subject: [PATCH 20/21] Add Cloudflare ALIAS record support Translates them to/from root CNAME --- README.md | 2 +- octodns/provider/cloudflare.py | 14 +++++++- tests/test_octodns_provider_cloudflare.py | 42 +++++++++++++++++++++++ 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 88223e6..cc84cf2 100644 --- a/README.md +++ b/README.md @@ -150,7 +150,7 @@ The above command pulled the existing data out of Route53 and placed the results | Provider | Record Support | GeoDNS Support | Notes | |--|--|--|--| | [AzureProvider](/octodns/provider/azuredns.py) | A, AAAA, CNAME, MX, NS, PTR, SRV, TXT | No | | -| [CloudflareProvider](/octodns/provider/cloudflare.py) | A, AAAA, CAA, CNAME, MX, NS, SPF, TXT | No | CAA tags restricted | +| [CloudflareProvider](/octodns/provider/cloudflare.py) | A, AAAA, ALIAS, CAA, CNAME, MX, NS, SPF, TXT | No | CAA tags restricted | | [DigitalOceanProvider](/octodns/provider/digitalocean.py) | A, AAAA, CAA, CNAME, MX, NS, TXT, SRV | No | CAA tags restricted | | [DnsimpleProvider](/octodns/provider/dnsimple.py) | All | No | CAA tags restricted | | [DynProvider](/octodns/provider/dyn.py) | All | Yes | | diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index a7ec5d8..9dfef6d 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -37,7 +37,8 @@ class CloudflareProvider(BaseProvider): ''' SUPPORTS_GEO = False # TODO: support SRV - SUPPORTS = set(('A', 'AAAA', 'CAA', 'CNAME', 'MX', 'NS', 'SPF', 'TXT')) + SUPPORTS = set(('ALIAS', 'A', 'AAAA', 'CAA', 'CNAME', 'MX', 'NS', 'SPF', + 'TXT')) MIN_TTL = 120 TIMEOUT = 15 @@ -124,6 +125,8 @@ class CloudflareProvider(BaseProvider): 'value': '{}.'.format(only['content']) } + _data_for_ALIAS = _data_for_CNAME + def _data_for_MX(self, _type, records): values = [] for r in records: @@ -182,6 +185,11 @@ class CloudflareProvider(BaseProvider): for name, types in values.items(): for _type, records in types.items(): + + # Cloudflare supports ALIAS semantics with root CNAMEs + if _type == 'CNAME' and name == '': + _type = 'ALIAS' + data_for = getattr(self, '_data_for_{}'.format(_type)) data = data_for(_type, records) record = Record.new(zone, name, data, source=self, @@ -238,6 +246,10 @@ class CloudflareProvider(BaseProvider): _type = record._type ttl = max(self.MIN_TTL, record.ttl) + # Cloudflare supports ALIAS semantics with a root CNAME + if _type == 'ALIAS': + _type = 'CNAME' + contents_for = getattr(self, '_contents_for_{}'.format(_type)) for content in contents_for(record): content.update({ diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 8a17083..824af9d 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -442,3 +442,45 @@ class TestCloudflareProvider(TestCase): call('DELETE', '/zones/ff12ab34cd5611334422ab3322997650/' 'dns_records/fc12ab34cd5611334422ab3322997653') ]) + + def test_alias(self): + provider = CloudflareProvider('test', 'email', 'token') + + # A CNAME for us to transform to ALIAS + provider.zone_records = Mock(return_value=[ + { + "id": "fc12ab34cd5611334422ab3322997642", + "type": "CNAME", + "name": "unit.tests", + "content": "www.unit.tests", + "proxiable": True, + "proxied": False, + "ttl": 300, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + }, + ]) + + zone = Zone('unit.tests.', []) + provider.populate(zone) + self.assertEquals(1, len(zone.records)) + record = list(zone.records)[0] + self.assertEquals('', record.name) + self.assertEquals('unit.tests.', record.fqdn) + self.assertEquals('ALIAS', record._type) + self.assertEquals('www.unit.tests.', record.value) + + # Make sure we transform back to CNAME going the other way + contents = provider._gen_contents(record) + self.assertEquals({ + 'content': u'www.unit.tests.', + 'name': 'unit.tests', + 'ttl': 300, + 'type': 'CNAME' + }, list(contents)[0]) From fdea900537cec8fced3120003c338ccd1fe6aead Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 6 Jan 2018 16:53:34 -0800 Subject: [PATCH 21/21] Correct total_count in Cloudflare record fixtures --- tests/fixtures/cloudflare-dns_records-page-1.json | 2 +- tests/fixtures/cloudflare-dns_records-page-2.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/fixtures/cloudflare-dns_records-page-1.json b/tests/fixtures/cloudflare-dns_records-page-1.json index eda4de3..3c423e2 100644 --- a/tests/fixtures/cloudflare-dns_records-page-1.json +++ b/tests/fixtures/cloudflare-dns_records-page-1.json @@ -180,7 +180,7 @@ "per_page": 10, "total_pages": 2, "count": 10, - "total_count": 17 + "total_count": 19 }, "success": true, "errors": [], diff --git a/tests/fixtures/cloudflare-dns_records-page-2.json b/tests/fixtures/cloudflare-dns_records-page-2.json index 150951b..de3d760 100644 --- a/tests/fixtures/cloudflare-dns_records-page-2.json +++ b/tests/fixtures/cloudflare-dns_records-page-2.json @@ -163,7 +163,7 @@ "per_page": 10, "total_pages": 2, "count": 9, - "total_count": 20 + "total_count": 19 }, "success": true, "errors": [],