From 8a13ccab466c9ab9e641959238e1de18355f1f6d Mon Sep 17 00:00:00 2001 From: trnsnt Date: Mon, 18 Sep 2017 14:59:41 +0200 Subject: [PATCH 01/24] Add OVH as octodns provider --- README.md | 1 + octodns/provider/ovh.py | 322 ++++++++++++++++++++++++++ requirements.txt | 1 + tests/test_octodns_provider_ovh.py | 359 +++++++++++++++++++++++++++++ 4 files changed, 683 insertions(+) create mode 100644 octodns/provider/ovh.py create mode 100644 tests/test_octodns_provider_ovh.py diff --git a/README.md b/README.md index 1f103f1..afe92ac 100644 --- a/README.md +++ b/README.md @@ -154,6 +154,7 @@ The above command pulled the existing data out of Route53 and placed the results | [DnsimpleProvider](/octodns/provider/dnsimple.py) | All | No | CAA tags restricted | | [DynProvider](/octodns/provider/dyn.py) | All | Yes | | | [Ns1Provider](/octodns/provider/ns1.py) | All | No | | +| [OVH](/octodns/provider/ovh.py) | A, AAAA, CNAME, MX, NAPTR, NS, PTR, SPF, SRV, SSHFP, TXT | No | | | [PowerDnsProvider](/octodns/provider/powerdns.py) | All | No | | | [Route53](/octodns/provider/route53.py) | A, AAAA, CAA, CNAME, MX, NAPTR, NS, PTR, SPF, SRV, TXT | Yes | | | [TinyDNSSource](/octodns/source/tinydns.py) | A, CNAME, MX, NS, PTR | No | read-only | diff --git a/octodns/provider/ovh.py b/octodns/provider/ovh.py new file mode 100644 index 0000000..b890862 --- /dev/null +++ b/octodns/provider/ovh.py @@ -0,0 +1,322 @@ +# +# +# + +from __future__ import absolute_import, division, print_function, \ + unicode_literals + +import logging +from collections import defaultdict + +import ovh + +from octodns.record import Record +from .base import BaseProvider + + +class OvhProvider(BaseProvider): + """ + OVH provider using API v6 + + ovh: + class: octodns.provider.ovh.OvhProvider + # OVH api v6 endpoint + endpoint: ovh-eu + # API application key + application_key: 1234 + # API application secret + application_secret: 1234 + # API consumer key + consumer_key: 1234 + """ + + SUPPORTS_GEO = False + + SUPPORTS = set(('A', 'AAAA', 'CNAME', 'MX', 'NAPTR', 'NS', 'PTR', 'SPF', + 'SRV', 'SSHFP', 'TXT')) + + def __init__(self, id, endpoint, application_key, application_secret, + consumer_key, *args, **kwargs): + self.log = logging.getLogger('OvhProvider[{}]'.format(id)) + self.log.debug('__init__: id=%s, endpoint=%s, application_key=%s, ' + 'application_secret=***, consumer_key=%s', id, endpoint, + application_key, consumer_key) + super(OvhProvider, self).__init__(id, *args, **kwargs) + self._client = ovh.Client( + endpoint=endpoint, + application_key=application_key, + application_secret=application_secret, + consumer_key=consumer_key, + ) + + def populate(self, zone, target=False, lenient=False): + self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, + target, lenient) + zone_name = zone.name[:-1] + records = self.get_records(zone_name=zone_name) + + values = defaultdict(lambda: defaultdict(list)) + for record in records: + values[record['subDomain']][record['fieldType']].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) + + self.log.info('populate: found %s records', + len(zone.records) - before) + + def _apply(self, plan): + desired = plan.desired + changes = plan.changes + zone_name = desired.name[:-1] + self.log.info('_apply: zone=%s, len(changes)=%d', desired.name, + len(changes)) + for change in changes: + class_name = change.__class__.__name__ + getattr(self, '_apply_{}'.format(class_name).lower())(zone_name, + change) + + # We need to refresh the zone to really apply the changes + self._client.post('/domain/zone/{}/refresh'.format(zone_name)) + + def _apply_create(self, zone_name, change): + new = change.new + params_for = getattr(self, '_params_for_{}'.format(new._type)) + for params in params_for(new): + self.create_record(zone_name, params) + + def _apply_update(self, zone_name, change): + self._apply_delete(zone_name, change) + self._apply_create(zone_name, change) + + def _apply_delete(self, zone_name, change): + existing = change.existing + self.delete_records(zone_name, existing._type, existing.name) + + @staticmethod + def _data_for_multiple(_type, records): + return { + 'ttl': records[0]['ttl'], + 'type': _type, + 'values': [record['target'] for record in records] + } + + @staticmethod + def _data_for_single(_type, records): + record = records[0] + return { + 'ttl': record['ttl'], + 'type': _type, + 'value': record['target'] + } + + @staticmethod + def _data_for_MX(_type, records): + values = [] + for record in records: + preference, exchange = record['target'].split(' ', 1) + values.append({ + 'preference': preference, + 'exchange': exchange, + }) + return { + 'ttl': records[0]['ttl'], + 'type': _type, + 'values': values, + } + + @staticmethod + def _data_for_NAPTR(_type, records): + values = [] + for record in records: + order, preference, flags, service, regexp, replacement = record[ + 'target'].split(' ', 5) + values.append({ + 'flags': flags[1:-1], + 'order': order, + 'preference': preference, + 'regexp': regexp[1:-1], + 'replacement': replacement, + 'service': service[1:-1], + }) + return { + 'type': _type, + 'ttl': records[0]['ttl'], + 'values': values + } + + @staticmethod + def _data_for_SRV(_type, records): + values = [] + for record in records: + priority, weight, port, target = record['target'].split(' ', 3) + values.append({ + 'port': port, + 'priority': priority, + 'target': '{}.'.format(target), + 'weight': weight + }) + return { + 'type': _type, + 'ttl': records[0]['ttl'], + 'values': values + } + + @staticmethod + def _data_for_SSHFP(_type, records): + values = [] + for record in records: + algorithm, fingerprint_type, fingerprint = record['target'].split( + ' ', 2) + values.append({ + 'algorithm': algorithm, + 'fingerprint': fingerprint, + 'fingerprint_type': fingerprint_type + }) + return { + 'type': _type, + 'ttl': records[0]['ttl'], + 'values': values + } + + _data_for_A = _data_for_multiple + _data_for_AAAA = _data_for_multiple + _data_for_NS = _data_for_multiple + _data_for_TXT = _data_for_multiple + _data_for_SPF = _data_for_multiple + _data_for_PTR = _data_for_single + _data_for_CNAME = _data_for_single + + @staticmethod + def _params_for_multiple(record): + for value in record.values: + yield { + 'target': value, + 'subDomain': record.name, + 'ttl': record.ttl, + 'fieldType': record._type, + } + + @staticmethod + def _params_for_single(record): + yield { + 'target': record.value, + 'subDomain': record.name, + 'ttl': record.ttl, + 'fieldType': record._type + } + + @staticmethod + def _params_for_MX(record): + for value in record.values: + yield { + 'target': '%d %s' % (value.preference, value.exchange), + 'subDomain': record.name, + 'ttl': record.ttl, + 'fieldType': record._type + } + + @staticmethod + def _params_for_NAPTR(record): + for value in record.values: + content = '{} {} "{}" "{}" "{}" {}' \ + .format(value.order, value.preference, value.flags, + value.service, value.regexp, value.replacement) + yield { + 'target': content, + 'subDomain': record.name, + 'ttl': record.ttl, + 'fieldType': record._type + } + + @staticmethod + def _params_for_SRV(record): + for value in record.values: + yield { + 'subDomain': '{} {} {} {}'.format(value.priority, + value.weight, value.port, + value.target), + 'target': record.name, + 'ttl': record.ttl, + 'fieldType': record._type + } + + @staticmethod + def _params_for_SSHFP(record): + for value in record.values: + yield { + 'subDomain': '{} {} {}'.format(value.algorithm, + value.fingerprint_type, + value.fingerprint), + 'target': record.name, + 'ttl': record.ttl, + 'fieldType': record._type + } + + _params_for_A = _params_for_multiple + _params_for_AAAA = _params_for_multiple + _params_for_NS = _params_for_multiple + _params_for_SPF = _params_for_multiple + _params_for_TXT = _params_for_multiple + + _params_for_CNAME = _params_for_single + _params_for_PTR = _params_for_single + + def get_records(self, zone_name): + """ + List all records of a DNS zone + :param zone_name: Name of zone + :return: list of id's records + """ + records = self._client.get('/domain/zone/{}/record'.format(zone_name)) + return [self.get_record(zone_name, record_id) for record_id in records] + + def get_record(self, zone_name, record_id): + """ + Get record with given id + :param zone_name: Name of the zone + :param record_id: Id of the record + :return: Value of the record + """ + return self._client.get( + '/domain/zone/{}/record/{}'.format(zone_name, record_id)) + + def delete_records(self, zone_name, record_type, subdomain): + """ + Delete record from have fieldType=type and subDomain=subdomain + :param zone_name: Name of the zone + :param record_type: fieldType + :param subdomain: subDomain + """ + records = self._client.get('/domain/zone/{}/record'.format(zone_name), + fieldType=record_type, subDomain=subdomain) + for record in records: + self.delete_record(zone_name, record) + + def delete_record(self, zone_name, record_id): + """ + Delete record with a given id + :param zone_name: Name of the zone + :param record_id: Id of the record + """ + self.log.debug('Delete record: zone: %s, id %s', zone_name, + record_id) + self._client.delete( + '/domain/zone/{}/record/{}'.format(zone_name, record_id)) + + def create_record(self, zone_name, params): + """ + Create a record + :param zone_name: Name of the zone + :param params: {'fieldType': 'A', 'ttl': 60, 'subDomain': 'www', + 'target': '1.2.3.4' + """ + self.log.debug('Create record: zone: %s, id %s', zone_name, + params) + return self._client.post('/domain/zone/{}/record'.format(zone_name), + **params) diff --git a/requirements.txt b/requirements.txt index d2be70f..a7d1d94 100644 --- a/requirements.txt +++ b/requirements.txt @@ -15,6 +15,7 @@ jmespath==0.9.3 msrestazure==0.4.10 natsort==5.0.3 nsone==0.9.14 +ovh==0.4.7 python-dateutil==2.6.1 requests==2.13.0 s3transfer==0.1.10 diff --git a/tests/test_octodns_provider_ovh.py b/tests/test_octodns_provider_ovh.py new file mode 100644 index 0000000..2816748 --- /dev/null +++ b/tests/test_octodns_provider_ovh.py @@ -0,0 +1,359 @@ +# +# +# + +from __future__ import absolute_import, division, print_function, \ + unicode_literals + +from unittest import TestCase + +from mock import patch, call +from ovh import APIError + +from octodns.provider.ovh import OvhProvider +from octodns.record import Record +from octodns.zone import Zone + + +class TestOvhProvider(TestCase): + api_record = [] + + zone = Zone('unit.tests.', []) + expected = set() + + # A, subdomain='' + api_record.append({ + 'fieldType': 'A', + 'ttl': 100, + 'target': '1.2.3.4', + 'subDomain': '', + 'id': 1 + }) + expected.add(Record.new(zone, '', { + 'ttl': 100, + 'type': 'A', + 'value': '1.2.3.4', + })) + + # A, subdomain='sub + api_record.append({ + 'fieldType': 'A', + 'ttl': 200, + 'target': '1.2.3.4', + 'subDomain': 'sub', + 'id': 2 + }) + expected.add(Record.new(zone, 'sub', { + 'ttl': 200, + 'type': 'A', + 'value': '1.2.3.4', + })) + + # CNAME + api_record.append({ + 'fieldType': 'CNAME', + 'ttl': 300, + 'target': 'unit.tests.', + 'subDomain': 'www2', + 'id': 3 + }) + expected.add(Record.new(zone, 'www2', { + 'ttl': 300, + 'type': 'CNAME', + 'value': 'unit.tests.', + })) + + # MX + api_record.append({ + 'fieldType': 'MX', + 'ttl': 400, + 'target': '10 mx1.unit.tests.', + 'subDomain': '', + 'id': 4 + }) + expected.add(Record.new(zone, '', { + 'ttl': 400, + 'type': 'MX', + 'values': [{ + 'preference': 10, + 'exchange': 'mx1.unit.tests.', + }] + })) + + # NAPTR + api_record.append({ + 'fieldType': 'NAPTR', + 'ttl': 500, + 'target': '10 100 "S" "SIP+D2U" "!^.*$!sip:info@bar.example.com!" .', + 'subDomain': 'naptr', + 'id': 5 + }) + expected.add(Record.new(zone, 'naptr', { + 'ttl': 500, + 'type': 'NAPTR', + 'values': [{ + 'flags': 'S', + 'order': 10, + 'preference': 100, + 'regexp': '!^.*$!sip:info@bar.example.com!', + 'replacement': '.', + 'service': 'SIP+D2U', + }] + })) + + # NS + api_record.append({ + 'fieldType': 'NS', + 'ttl': 600, + 'target': 'ns1.unit.tests.', + 'subDomain': '', + 'id': 6 + }) + api_record.append({ + 'fieldType': 'NS', + 'ttl': 600, + 'target': 'ns2.unit.tests.', + 'subDomain': '', + 'id': 7 + }) + expected.add(Record.new(zone, '', { + 'ttl': 600, + 'type': 'NS', + 'values': ['ns1.unit.tests.', 'ns2.unit.tests.'], + })) + + # NS with sub + api_record.append({ + 'fieldType': 'NS', + 'ttl': 700, + 'target': 'ns3.unit.tests.', + 'subDomain': 'www3', + 'id': 8 + }) + api_record.append({ + 'fieldType': 'NS', + 'ttl': 700, + 'target': 'ns4.unit.tests.', + 'subDomain': 'www3', + 'id': 9 + }) + expected.add(Record.new(zone, 'www3', { + 'ttl': 700, + 'type': 'NS', + 'values': ['ns3.unit.tests.', 'ns4.unit.tests.'], + })) + + api_record.append({ + 'fieldType': 'SRV', + 'ttl': 800, + 'target': '10 20 30 foo-1.unit.tests.', + 'subDomain': '_srv._tcp', + 'id': 10 + }) + api_record.append({ + 'fieldType': 'SRV', + 'ttl': 800, + 'target': '40 50 60 foo-2.unit.tests.', + 'subDomain': '_srv._tcp', + 'id': 11 + }) + expected.add(Record.new(zone, '_srv._tcp', { + 'ttl': 800, + 'type': 'SRV', + 'values': [{ + 'priority': 10, + 'weight': 20, + 'port': 30, + 'target': 'foo-1.unit.tests.', + }, { + 'priority': 40, + 'weight': 50, + 'port': 60, + 'target': 'foo-2.unit.tests.', + }] + })) + + # PTR + api_record.append({ + 'fieldType': 'PTR', + 'ttl': 900, + 'target': 'unit.tests.', + 'subDomain': '4', + 'id': 12 + }) + expected.add(Record.new(zone, '4', { + 'ttl': 900, + 'type': 'PTR', + 'value': 'unit.tests.' + })) + + # SPF + api_record.append({ + 'fieldType': 'SPF', + 'ttl': 1000, + 'target': 'v=spf1 include:unit.texts.rerirect ~all', + 'subDomain': '', + 'id': 13 + }) + expected.add(Record.new(zone, '', { + 'ttl': 1000, + 'type': 'SPF', + 'value': 'v=spf1 include:unit.texts.rerirect ~all' + })) + + # SSHFP + api_record.append({ + 'fieldType': 'SSHFP', + 'ttl': 1100, + 'target': '1 1 bf6b6825d2977c511a475bbefb88aad54a92ac73 ', + 'subDomain': '', + 'id': 14 + }) + expected.add(Record.new(zone, '', { + 'ttl': 1100, + 'type': 'SSHFP', + 'value': { + 'algorithm': 1, + 'fingerprint': 'bf6b6825d2977c511a475bbefb88aad54a92ac73', + 'fingerprint_type': 1 + } + })) + + # AAAA + api_record.append({ + 'fieldType': 'AAAA', + 'ttl': 1200, + 'target': '1:1ec:1::1', + 'subDomain': '', + 'id': 15 + }) + expected.add(Record.new(zone, '', { + 'ttl': 200, + 'type': 'AAAA', + 'value': '1:1ec:1::1', + })) + + @patch('ovh.Client') + def test_populate(self, client_mock): + provider = OvhProvider('test', 'endpoint', 'application_key', + 'application_secret', 'consumer_key') + + with patch.object(provider._client, 'get') as get_mock: + zone = Zone('unit.tests.', []) + get_mock.side_effect = APIError('boom') + with self.assertRaises(APIError) as ctx: + provider.populate(zone) + self.assertEquals(get_mock.side_effect, ctx.exception) + + with patch.object(provider._client, 'get') as get_mock: + zone = Zone('unit.tests.', []) + get_returns = [[record['id'] for record in self.api_record]] + get_returns += self.api_record + get_mock.side_effect = get_returns + provider.populate(zone) + self.assertEquals(self.expected, zone.records) + + @patch('ovh.Client') + def test_apply(self, client_mock): + provider = OvhProvider('test', 'endpoint', 'application_key', + 'application_secret', 'consumer_key') + + desired = Zone('unit.tests.', []) + + for r in self.expected: + desired.add_record(r) + + with patch.object(provider._client, 'post') as get_mock: + plan = provider.plan(desired) + get_mock.side_effect = APIError('boom') + with self.assertRaises(APIError) as ctx: + provider.apply(plan) + self.assertEquals(get_mock.side_effect, ctx.exception) + + with patch.object(provider._client, 'get') as get_mock: + get_returns = [[1, 2], { + 'fieldType': 'A', + 'ttl': 600, + 'target': '5.6.7.8', + 'subDomain': '', + 'id': 100 + }, {'fieldType': 'A', + 'ttl': 600, + 'target': '5.6.7.8', + 'subDomain': 'fake', + 'id': 101 + }] + get_mock.side_effect = get_returns + + plan = provider.plan(desired) + + with patch.object(provider._client, 'post') as post_mock: + with patch.object(provider._client, 'delete') as delete_mock: + with patch.object(provider._client, 'get') as get_mock: + get_mock.side_effect = [[100], [101]] + provider.apply(plan) + wanted_calls = [ + call(u'/domain/zone/unit.tests/record', + fieldType=u'A', + subDomain=u'', target=u'1.2.3.4', ttl=100), + call(u'/domain/zone/unit.tests/record', + fieldType=u'SRV', + subDomain=u'10 20 30 foo-1.unit.tests.', + target='_srv._tcp', ttl=800), + call(u'/domain/zone/unit.tests/record', + fieldType=u'SRV', + subDomain=u'40 50 60 foo-2.unit.tests.', + target='_srv._tcp', ttl=800), + call(u'/domain/zone/unit.tests/record', + fieldType=u'PTR', subDomain='4', + target=u'unit.tests.', ttl=900), + call(u'/domain/zone/unit.tests/record', + fieldType=u'NS', subDomain='www3', + target=u'ns3.unit.tests.', ttl=700), + call(u'/domain/zone/unit.tests/record', + fieldType=u'NS', subDomain='www3', + target=u'ns4.unit.tests.', ttl=700), + call(u'/domain/zone/unit.tests/record', + fieldType=u'SSHFP', + subDomain=u'1 1 bf6b6825d2977c511a475bbefb88a' + u'ad54' + u'a92ac73', + target=u'', ttl=1100), + call(u'/domain/zone/unit.tests/record', + fieldType=u'AAAA', subDomain=u'', + target=u'1:1ec:1::1', ttl=200), + call(u'/domain/zone/unit.tests/record', + fieldType=u'MX', subDomain=u'', + target=u'10 mx1.unit.tests.', ttl=400), + call(u'/domain/zone/unit.tests/record', + fieldType=u'CNAME', subDomain='www2', + target=u'unit.tests.', ttl=300), + call(u'/domain/zone/unit.tests/record', + fieldType=u'SPF', subDomain=u'', + target=u'v=spf1 include:unit.texts.' + u'rerirect ~all', + ttl=1000), + call(u'/domain/zone/unit.tests/record', + fieldType=u'A', + subDomain='sub', target=u'1.2.3.4', ttl=200), + call(u'/domain/zone/unit.tests/record', + fieldType=u'NAPTR', subDomain='naptr', + target=u'10 100 "S" "SIP+D2U" "!^.*$!sip:' + u'info@bar' + u'.example.com!" .', + ttl=500), + call(u'/domain/zone/unit.tests/refresh')] + + post_mock.assert_has_calls(wanted_calls) + + # Get for delete calls + get_mock.assert_has_calls( + [call(u'/domain/zone/unit.tests/record', + fieldType=u'A', subDomain=u''), + call(u'/domain/zone/unit.tests/record', + fieldType=u'A', subDomain='fake')] + ) + # 2 delete calls, one for update + one for delete + delete_mock.assert_has_calls( + [call(u'/domain/zone/unit.tests/record/100'), + call(u'/domain/zone/unit.tests/record/101')]) From feec443bb4f1127673c738869251032066a7ef76 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 29 Sep 2017 16:14:22 -0700 Subject: [PATCH 02/24] Require setuptools new enough to publish to pypi --- requirements-dev.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements-dev.txt b/requirements-dev.txt index 265f407..5cdf252 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -4,3 +4,4 @@ nose pep8 pyflakes requests_mock +setuptools>=36.4.0 From 74631a6a71779f59be50daed27044e11a314cbc6 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 29 Sep 2017 16:23:51 -0700 Subject: [PATCH 03/24] 0.8.7 version bump --- CHANGELOG.md | 13 +++++++++++++ octodns/__init__.py | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d58d4de..9c97cdc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,16 @@ +## v0.8.7 - 2017-09-29 - OVH support + +Adds an OVH provider. + +## v0.8.6 - 2017-09-06 - CAA record type, + +Misc fixes and improvments. + +* Azure TXT record fix +* PowerDNS api support for https +* Configurable Route53 max retries and max-attempts +* Improved key ordering error message + ## v0.8.5 - 2017-07-21 - Azure, NS1 escaping, & large zones Relatively small delta this go around. No major themes or anything, just steady diff --git a/octodns/__init__.py b/octodns/__init__.py index bfb1905..3740dec 100644 --- a/octodns/__init__.py +++ b/octodns/__init__.py @@ -3,4 +3,4 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals -__VERSION__ = '0.8.6' +__VERSION__ = '0.8.7' From 70120bedc82aca9103aacb4b25dc12a379b9a65d Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 5 Oct 2017 10:04:29 -0700 Subject: [PATCH 04/24] Implement "chunked" TXT/SPF value support for long values This implements it transparently at Record level. Providers that need things to be chunked (seems to just be Route53 an Dyn) switch to use `chunked_values`, but everything else can stick with `values`. I've run through each provider I have access to verifying that things operate as expected/required. OVH and Azure are untested. --- octodns/provider/dyn.py | 2 +- octodns/provider/route53.py | 3 +- octodns/record.py | 35 ++++++++++++++-------- tests/test_octodns_record.py | 56 ++++++++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 15 deletions(-) diff --git a/octodns/provider/dyn.py b/octodns/provider/dyn.py index 3b7b9ea..721b3a7 100644 --- a/octodns/provider/dyn.py +++ b/octodns/provider/dyn.py @@ -455,7 +455,7 @@ class DynProvider(BaseProvider): return [{ 'txtdata': v, 'ttl': record.ttl, - } for v in record.values] + } for v in record.chunked_values] def _kwargs_for_SRV(self, record): return [{ diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 0600511..7623648 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -114,8 +114,7 @@ class _Route53Record(object): for v in record.values] def _values_for_quoted(self, record): - return ['"{}"'.format(v.replace('"', '\\"')) - for v in record.values] + return record.chunked_values _values_for_SPF = _values_for_quoted _values_for_TXT = _values_for_quoted diff --git a/octodns/record.py b/octodns/record.py index 8ef80be..554d98b 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -704,8 +704,8 @@ class SshfpRecord(_ValuesMixin, Record): _unescaped_semicolon_re = re.compile(r'\w;') -class SpfRecord(_ValuesMixin, Record): - _type = 'SPF' +class _ChunkedValuesMixin(_ValuesMixin): + CHUNK_SIZE = 255 @classmethod def _validate_value(cls, value): @@ -714,9 +714,29 @@ class SpfRecord(_ValuesMixin, Record): return [] def _process_values(self, values): + ret = [] + for v in values: + if v and v[0] == '"': + v = v[1:-1] + ret.append(v.replace('" "', '')) + return ret + + @property + def chunked_values(self): + values = [] + for v in self.values: + v = v.replace('"', '\\"') + vs = [v[i:i + self.CHUNK_SIZE] + for i in range(0, len(v), self.CHUNK_SIZE)] + vs = '" "'.join(vs) + values.append('"{}"'.format(vs)) return values +class SpfRecord(_ChunkedValuesMixin, Record): + _type = 'SPF' + + class SrvValue(object): @classmethod @@ -797,14 +817,5 @@ class SrvRecord(_ValuesMixin, Record): return [SrvValue(v) for v in values] -class TxtRecord(_ValuesMixin, Record): +class TxtRecord(_ChunkedValuesMixin, Record): _type = 'TXT' - - @classmethod - def _validate_value(cls, value): - if _unescaped_semicolon_re.search(value): - return ['unescaped ;'] - return [] - - def _process_values(self, values): - return values diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 51676a3..41b63a9 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -1490,3 +1490,59 @@ class TestRecordValidation(TestCase): 'value': 'this has some; semi-colons\; in it', }) self.assertEquals(['unescaped ;'], ctx.exception.reasons) + + def test_TXT_long_value_chunking(self): + expected = '"Lorem ipsum dolor sit amet, consectetur adipiscing ' \ + 'elit, seddo eiusmod tempor incididunt ut labore et dolore ' \ + 'magna aliqua. Ut enim ad minim veniam, quis nostrud ' \ + 'exercitation ullamco laboris nisi ut aliquip ex ea commodo ' \ + 'consequat. Duis aute irure dolor in" " reprehenderit in ' \ + 'voluptate velit esse cillum dolore eu fugiat nulla pariatur. ' \ + 'Excepteur sint occaecat cupidatat non proident, sunt in culpa ' \ + 'qui officia deserunt mollit anim id est laborum."' + + # Single string + single = Record.new(self.zone, '', { + 'type': 'TXT', + 'ttl': 600, + 'values': [ + 'hello world', + 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed' + 'do eiusmod tempor incididunt ut labore et dolore magna ' + 'aliqua. Ut enim ad minim veniam, quis nostrud exercitation ' + 'ullamco laboris nisi ut aliquip ex ea commodo consequat. ' + 'Duis aute irure dolor in reprehenderit in voluptate velit ' + 'esse cillum dolore eu fugiat nulla pariatur. Excepteur sint ' + 'occaecat cupidatat non proident, sunt in culpa qui officia ' + 'deserunt mollit anim id est laborum.', + 'this has some\; semi-colons\; in it', + ] + }) + self.assertEquals(3, len(single.values)) + self.assertEquals(3, len(single.chunked_values)) + # Note we are checking that this normalizes the chunking, not that we + # get out what we put in. + self.assertEquals(expected, single.chunked_values[0]) + + # Chunked + chunked = Record.new(self.zone, '', { + 'type': 'TXT', + 'ttl': 600, + 'values': [ + '"hello world"', + '"Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed' + 'do eiusmod tempor incididunt ut labore et dolore magna ' + 'aliqua. Ut enim ad minim veniam, quis nostrud exercitation ' + 'ullamco laboris nisi ut aliquip ex" " ea commodo consequat. ' + 'Duis aute irure dolor in reprehenderit in voluptate velit ' + 'esse cillum dolore eu fugiat nulla pariatur. Excepteur sint ' + 'occaecat cupidatat non proident, sunt in culpa qui officia ' + 'deserunt mollit anim id est laborum."', + '"this has some\; semi-colons\; in it"', + ] + }) + self.assertEquals(expected, chunked.chunked_values[0]) + # should be single values, no quoting + self.assertEquals(single.values, chunked.values) + # should be chunked values, with quoting + self.assertEquals(single.chunked_values, chunked.chunked_values) From 30efda329559cf82eb7666a3ccd5fdcda6be986e Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 9 Oct 2017 09:00:15 -0700 Subject: [PATCH 05/24] Make long TXT record concat cleaerer --- tests/test_octodns_record.py | 40 ++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 41b63a9..46a5e65 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -1493,28 +1493,30 @@ class TestRecordValidation(TestCase): def test_TXT_long_value_chunking(self): expected = '"Lorem ipsum dolor sit amet, consectetur adipiscing ' \ - 'elit, seddo eiusmod tempor incididunt ut labore et dolore ' \ + 'elit, sed do eiusmod tempor incididunt ut labore et dolore ' \ 'magna aliqua. Ut enim ad minim veniam, quis nostrud ' \ 'exercitation ullamco laboris nisi ut aliquip ex ea commodo ' \ - 'consequat. Duis aute irure dolor in" " reprehenderit in ' \ + 'consequat. Duis aute irure dolor i" "n reprehenderit in ' \ 'voluptate velit esse cillum dolore eu fugiat nulla pariatur. ' \ 'Excepteur sint occaecat cupidatat non proident, sunt in culpa ' \ 'qui officia deserunt mollit anim id est laborum."' + long_value = 'Lorem ipsum dolor sit amet, consectetur adipiscing ' \ + 'elit, sed do eiusmod tempor incididunt ut labore et dolore ' \ + 'magna aliqua. Ut enim ad minim veniam, quis nostrud ' \ + 'exercitation ullamco laboris nisi ut aliquip ex ea commodo ' \ + 'consequat. Duis aute irure dolor in reprehenderit in ' \ + 'voluptate velit esse cillum dolore eu fugiat nulla ' \ + 'pariatur. Excepteur sint occaecat cupidatat non proident, ' \ + 'sunt in culpa qui officia deserunt mollit anim id est ' \ + 'laborum.' # Single string single = Record.new(self.zone, '', { 'type': 'TXT', 'ttl': 600, 'values': [ 'hello world', - 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed' - 'do eiusmod tempor incididunt ut labore et dolore magna ' - 'aliqua. Ut enim ad minim veniam, quis nostrud exercitation ' - 'ullamco laboris nisi ut aliquip ex ea commodo consequat. ' - 'Duis aute irure dolor in reprehenderit in voluptate velit ' - 'esse cillum dolore eu fugiat nulla pariatur. Excepteur sint ' - 'occaecat cupidatat non proident, sunt in culpa qui officia ' - 'deserunt mollit anim id est laborum.', + long_value, 'this has some\; semi-colons\; in it', ] }) @@ -1524,20 +1526,22 @@ class TestRecordValidation(TestCase): # get out what we put in. self.assertEquals(expected, single.chunked_values[0]) + long_split_value = '"Lorem ipsum dolor sit amet, consectetur ' \ + 'adipiscing elit, sed do eiusmod tempor incididunt ut ' \ + 'labore et dolore magna aliqua. Ut enim ad minim veniam, ' \ + 'quis nostrud exercitation ullamco laboris nisi ut aliquip ' \ + 'ex" " ea commodo consequat. Duis aute irure dolor in ' \ + 'reprehenderit in voluptate velit esse cillum dolore eu ' \ + 'fugiat nulla pariatur. Excepteur sint occaecat cupidatat ' \ + 'non proident, sunt in culpa qui officia deserunt mollit ' \ + 'anim id est laborum."' # Chunked chunked = Record.new(self.zone, '', { 'type': 'TXT', 'ttl': 600, 'values': [ '"hello world"', - '"Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed' - 'do eiusmod tempor incididunt ut labore et dolore magna ' - 'aliqua. Ut enim ad minim veniam, quis nostrud exercitation ' - 'ullamco laboris nisi ut aliquip ex" " ea commodo consequat. ' - 'Duis aute irure dolor in reprehenderit in voluptate velit ' - 'esse cillum dolore eu fugiat nulla pariatur. Excepteur sint ' - 'occaecat cupidatat non proident, sunt in culpa qui officia ' - 'deserunt mollit anim id est laborum."', + long_split_value, '"this has some\; semi-colons\; in it"', ] }) From 6f0a12ae01a6fb0f0368264bcf48f8cc9089658d Mon Sep 17 00:00:00 2001 From: Petter Hassberg Date: Mon, 2 Oct 2017 14:16:29 +0200 Subject: [PATCH 06/24] Add .idea (intelliJ) files to gitignore. --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index c45a684..1bca124 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,4 @@ output/ tmp/ build/ config/ +.idea/ \ No newline at end of file From ed783b5ff2899828d2480a63cecf55ed04a9e6e7 Mon Sep 17 00:00:00 2001 From: Petter Hassberg Date: Mon, 2 Oct 2017 14:17:09 +0200 Subject: [PATCH 07/24] Add proposed google cloud provider. Proposed google cloud provider for #23 --- octodns/provider/googlecloud.py | 363 ++++++++++++++++++ requirements.txt | 1 + script/test | 1 + tests/test_octodns_provider_googlecloud.py | 421 +++++++++++++++++++++ 4 files changed, 786 insertions(+) create mode 100644 octodns/provider/googlecloud.py create mode 100644 tests/test_octodns_provider_googlecloud.py diff --git a/octodns/provider/googlecloud.py b/octodns/provider/googlecloud.py new file mode 100644 index 0000000..dff11fc --- /dev/null +++ b/octodns/provider/googlecloud.py @@ -0,0 +1,363 @@ +# +# +# + +from __future__ import absolute_import, division, print_function, \ + unicode_literals + +import re +import shlex +import time +from logging import getLogger + +from google.cloud import dns + +from .base import BaseProvider +from ..record import Record + + +class _GoogleCloudRecordSetMaker(object): + """Wrapper to make google cloud client resource record sets from OctoDNS + Records. + + googlecloud.py: + class: octodns.provider.googlecloud._GoogleCloudRecordSetMaker + An _GoogleCloudRecordSetMaker creates google cloued client resource + records which can be used to update the Google Cloud DNS zones. + """ + + def __init__(self, gcloud_zone, record): + self.gcloud_zone = gcloud_zone + self.record = record + + self._record_set_func = getattr( + self, '_record_set_from_{}'.format(record._type)) + + def get_record_set(self): + return self._record_set_func(self.record) + + def _record_set_from_A(self, record): + return self.gcloud_zone.resource_record_set( + record.fqdn, record._type, record.ttl, record.values) + + _record_set_from_AAAA = _record_set_from_A + + def _record_set_from_CAA(self, record): + return self.gcloud_zone.resource_record_set( + record.fqdn, record._type, record.ttl, [ + '{flags} {tag} {value}'.format(**record.data['value'])]) + + def _record_set_from_CNAME(self, record): + return self.gcloud_zone.resource_record_set( + record.fqdn, record._type, record.ttl, [record.value]) + + def _record_set_from_MX(self, record): + return self.gcloud_zone.resource_record_set( + record.fqdn, record._type, record.ttl, [ + '{preference} {exchange}'.format(**v.data) + for v in record.values]) + + def _record_set_from_NAPTR(self, record): + return self.gcloud_zone.resource_record_set( + record.fqdn, record._type, record.ttl, [ + '{order} {preference} "{flags}" "{service}" ' + '"{regexp}" {replacement}' + .format(**v.data) for v in record.values]) + + _record_set_from_NS = _record_set_from_A + + _record_set_from_PTR = _record_set_from_CNAME + + _record_set_from_SPF = _record_set_from_A + + def _record_set_from_SRV(self, record): + return self.gcloud_zone.resource_record_set( + record.fqdn, record._type, record.ttl, [ + '{priority} {weight} {port} {target}' + .format(**v.data) for v in record.values]) + + def _record_set_from_TXT(self, record): + if 'values' in record.data: + val = record.data['values'] + else: + val = [record.data['value']] + + return self.gcloud_zone.resource_record_set( + record.fqdn, record._type, record.ttl, val) + + +class GoogleCloudProvider(BaseProvider): + """ + Google Cloud DNS provider + + google_cloud: + class: octodns.provider.googlecloud.GoogleCloudProvider + # Credentials file for a service_account or other account can be + # specified with the GOOGLE_APPLICATION_CREDENTIALS environment + # variable. (https://console.cloud.google.com/apis/credentials) + # + # The project to work on (not required) + # project: foobar + """ + + SUPPORTS = set(('A', 'AAAA', 'CAA', 'CNAME', 'MX', 'NAPTR', + 'NS', 'PTR', 'SPF', 'SRV', 'TXT')) + SUPPORTS_GEO = False + + def __init__(self, id, project=None, *args, **kwargs): + + # Logger + self.log = getLogger('GoogleCloudProvider[{}]'.format(id)) + self.id = id + + super(GoogleCloudProvider, self).__init__(id, *args, **kwargs) + self.gcloud_client = dns.Client(project=project) + + def _apply(self, plan): + """Required function of manager.py to actually apply a record change. + + :param plan: Contains the zones and changes to be made + :type plan: octodns.provider.base.Plan + + :type return: void + """ + desired = plan.desired + changes = plan.changes + + self.log.debug('_apply: zone=%s, len(changes)=%d', desired.name, + len(changes)) + + # Get gcloud zone, or create one if none existed before. + gcloud_zone = self._get_gcloud_zone(desired.name, create=True) + + gcloud_changes = gcloud_zone.changes() + + for change in changes: + class_name = change.__class__.__name__ + if class_name in 'Create': + gcloud_changes.add_record_set( + self._record_to_record_set(gcloud_zone, change.record)) + elif class_name == 'Delete': + gcloud_changes.delete_record_set( + self._record_to_record_set(gcloud_zone, change.record)) + elif class_name == 'Update': + gcloud_changes.delete_record_set( + self._record_to_record_set(gcloud_zone, change.existing)) + gcloud_changes.add_record_set( + self._record_to_record_set(gcloud_zone, change.new)) + else: + raise RuntimeError('Change type "{}" for change "{!s}" ' + 'is none of "Create", "Delete" or "Update' + .format(class_name, change)) + + gcloud_changes.create() + i = 1 + while gcloud_changes.status != 'done': + self.log.debug("Waiting for changes to complete") + time.sleep(i) + gcloud_changes.reload() + if i < 30: + i += 2 + + def _create_gcloud_zone(self, dns_name): + """Creates a google cloud ManagedZone with dns_name, and zone named + derived from it. calls .create() method and returns it. + + :param dns_name: fqdn of zone to create + :type dns_name: str + + :type return: new google.cloud.dns.ManagedZone + """ + # Zone name must begin with a letter, end with a letter or digit, + # and only contain lowercase letters, digits or dashes + zone_name = re.sub("[^a-z0-9-]", "", + dns_name[:-1].replace('.', "-")) + # make sure that the end result did not end up wo leading letter + if re.match('[^a-z]', zone_name[0]): + # I cannot think of a situation where a zone name derived from + # a domain name would'nt start with leading letter and thereby + # violate the constraint, however if such a situation is + # encountered, add a leading "a" here. + zone_name = "a%s" % zone_name + + gcloud_zone = self.gcloud_client.zone( + name=zone_name, + dns_name=dns_name + ) + gcloud_zone.create(client=self.gcloud_client) + + self.log.info("Created zone %s. Fqdn %s." % + (zone_name, dns_name)) + + return gcloud_zone + + def _get_gcloud_records(self, gcloud_zone, page_token=None): + """ Generator function which yields ResourceRecordSet for the managed + gcloud zone, until there are no more records to pull. + + :param gcloud_zone: zone to pull records from + :type gcloud_zone: google.cloud.dns.ManagedZone + :param page_token: page token for the page to get + + :return: a resource record set + :type return: google.cloud.dns.ResourceRecordSet + """ + gcloud_iterator = gcloud_zone.list_resource_record_sets( + page_token=page_token) + for gcloud_record in gcloud_iterator: + yield gcloud_record + # This is to get results which may be on a "paged" page. + # (if more than max_results) entries. + if gcloud_iterator.next_page_token: + for gcloud_record in self._get_gcloud_records( + gcloud_zone, gcloud_iterator.next_page_token): + # yield from is in python 3 only. + yield gcloud_record + + def _get_gcloud_zone(self, dns_name, page_token=None, create=False): + """Return the ManagedZone which has has the matching dns_name, or + None if no such zone exist, unless create=True, then create a new + one and return it. + + :param dns_name: fqdn of dns name for zone to get. + :type dns_name: str + :param page_token: page token for the page to get + :type page_token: str + :param create: if true, create ManagedZone if it does not exist + already + + :type return: new google.cloud.dns.ManagedZone + """ + # Find the google name for the incoming zone + gcloud_zones = self.gcloud_client.list_zones(page_token=page_token) + for gcloud_zone in gcloud_zones: + if gcloud_zone.dns_name == dns_name: + return gcloud_zone + else: + # Zone not found. Check if there are more results which could be + # retrieved by checking "next_page_token". + if gcloud_zones.next_page_token: + return self._get_gcloud_zone(dns_name, + gcloud_zones.next_page_token) + else: + # Nothing found, either return None or else create zone and + # return that one (if create=True) + self.log.debug('_get_gcloud_zone: zone name=%s, ' + 'was not found by %s.', + dns_name, self.gcloud_client) + if create: + return self._create_gcloud_zone(dns_name) + + @staticmethod + def _record_to_record_set(gcloud_zone, record): + """create google.cloud.dns.ResourceRecordSet from ocdodns.Record + + :param record: a record object + :type record: ocdodns.Record + :param gcloud_zone: a google gcloud zone + :type gcloud_zone: google.cloud.dns.ManagedZone + :type return: google.cloud.dns.ResourceRecordSet + """ + grm = _GoogleCloudRecordSetMaker(gcloud_zone, record) + + return grm.get_record_set() + + def populate(self, zone, target=False, lenient=False): + """Required function of manager.py to collect records from zone. + + :param zone: A dns zone + :type zone: octodns.zone.Zone + :param target: Unused. + :type target: bool + :param lenient: Unused. Check octodns.manager for usage. + :type lenient: bool + + :type return: void + """ + + self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, + target, lenient) + before = len(zone.records) + + gcloud_zone = self._get_gcloud_zone(zone.name) + + _records = set() + if gcloud_zone: + for gcloud_record in self._get_gcloud_records(gcloud_zone): + if gcloud_record.record_type.upper() in self.SUPPORTS: + _records.add(gcloud_record) + for gcloud_record in _records: + record_name = gcloud_record.name + if record_name.endswith(zone.name): + # google cloud always return fqdn. Make relative record + # here. "root" records will then get the '' record_name, + # which is also the way dyn likes it. + record_name = record_name[:-(len(zone.name) + 1)] + typ = gcloud_record.record_type.upper() + data = getattr(self, '_data_for_{}'.format(typ)) + data = data(gcloud_record) + data['type'] = typ + data['ttl'] = gcloud_record.ttl + self.log.debug('populate: adding record {} records: {!s}' + .format(record_name, data)) + record = Record.new(zone, record_name, data, source=self) + zone.add_record(record) + + self.log.info('populate: found %s records', len(zone.records) - before) + + def _data_for_A(self, gcloud_record): + return { + 'values': gcloud_record.rrdatas + } + + _data_for_AAAA = _data_for_A + + def _data_for_CAA(self, gcloud_record): + return { + 'values': [{ + 'flags': v[0], + 'tag': v[1], + 'value': v[2]} + for v in [shlex.split(g) for g in gcloud_record.rrdatas]]} + + def _data_for_CNAME(self, gcloud_record): + return { + 'value': gcloud_record.rrdatas[0] + } + + def _data_for_MX(self, gcloud_record): + return {'values': [{ + "preference": v[0], + "exchange": v[1]} + for v in [shlex.split(g) for g in gcloud_record.rrdatas]]} + + def _data_for_NAPTR(self, gcloud_record): + return {'values': [{ + 'order': v[0], + 'preference': v[1], + 'flags': v[2], + 'service': v[3], + 'regexp': v[4], + 'replacement': v[5]} + for v in [shlex.split(g) for g in gcloud_record.rrdatas]]} + + _data_for_NS = _data_for_A + + _data_for_PTR = _data_for_CNAME + + _data_for_SPF = _data_for_A + + def _data_for_SRV(self, gcloud_record): + return {'values': [{ + 'priority': v[0], + 'weight': v[1], + 'port': v[2], + 'target': v[3]} + for v in [shlex.split(g) for g in gcloud_record.rrdatas]]} + + def _data_for_TXT(self, gcloud_record): + if len(gcloud_record.rrdatas) > 1: + return { + 'values': gcloud_record.rrdatas} + return { + 'value': gcloud_record.rrdatas[0]} diff --git a/requirements.txt b/requirements.txt index a7d1d94..80fbe1e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -9,6 +9,7 @@ dnspython==1.15.0 docutils==0.14 dyn==1.8.0 futures==3.1.1 +google-cloud==0.27.0 incf.countryutils==1.0 ipaddress==1.0.18 jmespath==0.9.3 diff --git a/script/test b/script/test index 3ee6e9c..41edfd8 100755 --- a/script/test +++ b/script/test @@ -24,5 +24,6 @@ export DNSIMPLE_TOKEN= export DYN_CUSTOMER= export DYN_PASSWORD= export DYN_USERNAME= +export GOOGLE_APPLICATION_CREDENTIALS= nosetests "$@" diff --git a/tests/test_octodns_provider_googlecloud.py b/tests/test_octodns_provider_googlecloud.py new file mode 100644 index 0000000..568ac4f --- /dev/null +++ b/tests/test_octodns_provider_googlecloud.py @@ -0,0 +1,421 @@ +# +# +# + +from __future__ import absolute_import, division, print_function, \ + unicode_literals + +from octodns.record import Create, Delete, Update, Record +from octodns.provider.googlecloud import GoogleCloudProvider, \ + _GoogleCloudRecordSetMaker + +from octodns.zone import Zone +from octodns.provider.base import Plan + +from unittest import TestCase +from mock import Mock, patch, PropertyMock + +zone = Zone(name='unit.tests.', sub_zones=[]) +octo_records = [] +octo_records.append(Record.new(zone, '', { + 'ttl': 0, + 'type': 'A', + 'values': ['1.2.3.4', '10.10.10.10']})) +octo_records.append(Record.new(zone, 'a', { + 'ttl': 1, + 'type': 'A', + 'values': ['1.2.3.4', '1.1.1.1']})) +octo_records.append(Record.new(zone, 'aa', { + 'ttl': 9001, + 'type': 'A', + 'values': ['1.2.4.3']})) +octo_records.append(Record.new(zone, 'aaa', { + 'ttl': 2, + 'type': 'A', + 'values': ['1.1.1.3']})) +octo_records.append(Record.new(zone, 'cname', { + 'ttl': 3, + 'type': 'CNAME', + 'value': 'a.unit.tests.'})) +octo_records.append(Record.new(zone, 'mx1', { + 'ttl': 3, + 'type': 'MX', + 'values': [{ + 'priority': 10, + 'value': 'mx1.unit.tests.', + }, { + 'priority': 20, + 'value': 'mx2.unit.tests.', + }]})) +octo_records.append(Record.new(zone, 'mx2', { + 'ttl': 3, + 'type': 'MX', + 'values': [{ + 'priority': 10, + 'value': 'mx1.unit.tests.', + }]})) +octo_records.append(Record.new(zone, '', { + 'ttl': 4, + 'type': 'NS', + 'values': ['ns1.unit.tests.', 'ns2.unit.tests.']})) +octo_records.append(Record.new(zone, 'foo', { + 'ttl': 5, + 'type': 'NS', + 'value': 'ns1.unit.tests.'})) +octo_records.append(Record.new(zone, '_srv._tcp', { + 'ttl': 6, + 'type': 'SRV', + 'values': [{ + 'priority': 10, + 'weight': 20, + 'port': 30, + 'target': 'foo-1.unit.tests.', + }, { + 'priority': 12, + 'weight': 30, + 'port': 30, + 'target': 'foo-2.unit.tests.', + }]})) +octo_records.append(Record.new(zone, '_srv2._tcp', { + 'ttl': 7, + 'type': 'SRV', + 'values': [{ + 'priority': 12, + 'weight': 17, + 'port': 1, + 'target': 'srvfoo.unit.tests.', + }]})) +octo_records.append(Record.new(zone, 'txt1', { + 'ttl': 8, + 'type': 'TXT', + 'value': 'txt singleton test'})) +octo_records.append(Record.new(zone, 'txt2', { + 'ttl': 9, + 'type': 'TXT', + 'values': ['txt multiple test', 'txt multiple test 2']})) +octo_records.append(Record.new(zone, 'naptr', { + 'ttl': 9, + 'type': 'NAPTR', + 'values': [{ + 'order': 100, + 'preference': 10, + 'flags': 'S', + 'service': 'SIP+D2U', + 'regexp': "!^.*$!sip:customer-service@unit.tests!", + 'replacement': '_sip._udp.unit.tests.' + }]})) +octo_records.append(Record.new(zone, 'caa', { + 'ttl': 9, + 'type': 'CAA', + 'value': { + 'flags': 0, + 'tag': 'issue', + 'value': 'ca.unit.tests', + }})) +for record in octo_records: + zone.add_record(record) + +# This is the format which the google API likes. +resource_record_sets = [ + ('unit.tests.', u'A', 0, [u'1.2.3.4', u'10.10.10.10']), + (u'a.unit.tests.', u'A', 1, [u'1.1.1.1', u'1.2.3.4']), + (u'aa.unit.tests.', u'A', 9001, [u'1.2.4.3']), + (u'aaa.unit.tests.', u'A', 2, [u'1.1.1.3']), + (u'cname.unit.tests.', u'CNAME', 3, [u'a.unit.tests.']), + (u'mx1.unit.tests.', u'MX', 3, + [u'10 mx1.unit.tests.', u'20 mx2.unit.tests.']), + (u'mx2.unit.tests.', u'MX', 3, [u'10 mx1.unit.tests.']), + ('unit.tests.', u'NS', 4, [u'ns1.unit.tests.', u'ns2.unit.tests.']), + (u'foo.unit.tests.', u'NS', 5, [u'ns1.unit.tests.']), + (u'_srv._tcp.unit.tests.', u'SRV', 6, + [u'10 20 30 foo-1.unit.tests.', u'12 30 30 foo-2.unit.tests.']), + (u'_srv2._tcp.unit.tests.', u'SRV', 7, [u'12 17 1 srvfoo.unit.tests.']), + (u'txt1.unit.tests.', u'TXT', 8, [u'txt singleton test']), + (u'txt2.unit.tests.', u'TXT', 9, + [u'txt multiple test', u'txt multiple test 2']), + (u'naptr.unit.tests.', u'NAPTR', 9, [ + u'100 10 "S" "SIP+D2U" "!^.*$!sip:customer-service@unit.tests!"' + u' _sip._udp.unit.tests.']), + (u'caa.unit.tests.', u'CAA', 9, [u'0 issue ca.unit.tests']) +] + + +class DummyResourceRecordSet: + def __init__(self, record_name, record_type, ttl, rrdatas): + self.name = record_name + self.record_type = record_type + self.ttl = ttl + self.rrdatas = rrdatas + + def __eq__(self, other): + try: + return self.name == other.name \ + and self.record_type == other.record_type \ + and self.ttl == other.ttl \ + and sorted(self.rrdatas) == sorted(other.rrdatas) + except: + return False + + def __repr__(self): + return "{} {} {} {!s}"\ + .format(self.name, self.record_type, self.ttl, self.rrdatas) + + def __hash__(self): + return hash(repr(self)) + + +class DummyGoogleCloudZone: + def __init__(self, dns_name): + self.dns_name = dns_name + + def resource_record_set(self, *args): + return DummyResourceRecordSet(*args) + + def list_resource_record_sets(self, *args): + pass + + +class DummyIterator: + """Returns a mock DummyIterator object to use in testing. + This is because API calls for google cloud DNS, if paged, contains a + "next_page_token", which can be used to grab a subsequent + iterator with more results. + + :type return: DummyIterator + """ + def __init__(self, list_of_stuff, page_token=None): + self.iterable = iter(list_of_stuff) + self.next_page_token = page_token + + def __iter__(self): + return self + + def next(self): + return self.iterable.next() + + +class TestGoogleCloudRecordSetMaker(TestCase): + def test_get_record_set(self): + mz = DummyGoogleCloudZone('unit.tests.') + record_sets = [] + for record in octo_records: + mm = _GoogleCloudRecordSetMaker(mz, record) + record_sets.append(mm.get_record_set()) + + self.assertEqual( + len(octo_records), + len(record_sets)) + + +class TestGoogleCloudProvider(TestCase): + @patch('octodns.provider.googlecloud.dns') + def _get_provider(*args): + '''Returns a mock GoogleCloudProvider object to use in testing. + + :type return: GoogleCloudProvider + ''' + return GoogleCloudProvider(id=1, project="mock") + + @patch('octodns.provider.googlecloud.time.sleep') + @patch('octodns.provider.googlecloud.dns') + def test__apply(self, *_): + class DummyDesired: + def __init__(self, name, changes): + self.name = name + self.changes = changes + + apply_z = Zone("unit.tests.", []) + create_r = Record.new(apply_z, '', { + 'ttl': 0, + 'type': 'A', + 'values': ['1.2.3.4', '10.10.10.10']}) + delete_r = Record.new(apply_z, 'a', { + 'ttl': 1, + 'type': 'A', + 'values': ['1.2.3.4', '1.1.1.1']}) + update_existing_r = Record.new(apply_z, 'aa', { + 'ttl': 9001, + 'type': 'A', + 'values': ['1.2.4.3']}) + update_new_r = Record.new(apply_z, 'aa', { + 'ttl': 666, + 'type': 'A', + 'values': ['1.4.3.2']}) + + gcloud_zone_mock = DummyGoogleCloudZone("unit.tests.") + status_mock = Mock() + return_values_for_status = iter( + ['', '', '', '', '', '', '', '', '', '', '', '', '', '', '', + '', '', '', 'done']) + type(status_mock).status = PropertyMock( + side_effect=return_values_for_status.next) + gcloud_zone_mock.changes = Mock(return_value=status_mock) + + provider = self._get_provider() + provider.gcloud_client = Mock() + provider._get_gcloud_zone = Mock( + return_value=gcloud_zone_mock) + desired = Mock() + desired.name = Mock(return_value="unit.tests.") + changes = [] + changes.append(Create(create_r)) + changes.append(Delete(delete_r)) + changes.append(Update(existing=update_existing_r, new=update_new_r)) + + provider.apply(Plan( + existing=[update_existing_r, delete_r], + desired=desired, + changes=changes + )) + + calls_mock = gcloud_zone_mock.changes.return_value + mocked_calls = [] + for mock_call in calls_mock.add_record_set.mock_calls: + mocked_calls.append(mock_call[1][0]) + + self.assertEqual(mocked_calls, [ + DummyResourceRecordSet( + 'unit.tests.', 'A', 0, ['1.2.3.4', '10.10.10.10']), + DummyResourceRecordSet( + 'aa.unit.tests.', 'A', 666, ['1.4.3.2']) + ]) + + mocked_calls2 = [] + for mock_call in calls_mock.delete_record_set.mock_calls: + mocked_calls2.append(mock_call[1][0]) + + self.assertEqual(mocked_calls2, [ + DummyResourceRecordSet( + 'a.unit.tests.', 'A', 1, ['1.2.3.4', '1.1.1.1']), + DummyResourceRecordSet( + 'aa.unit.tests.', 'A', 9001, ['1.2.4.3']) + ]) + + unsupported_change = Mock() + unsupported_change.__len__ = Mock(return_value=1) + mock_plan = Mock() + type(mock_plan).desired = PropertyMock(return_value=DummyDesired( + "dummy name", [])) + type(mock_plan).changes = [unsupported_change] + + with self.assertRaises(RuntimeError): + provider.apply(mock_plan) + + def test__record_to_record_set(self): + provider = self._get_provider() + gcloud_zone = DummyGoogleCloudZone('unit.tests.') + for record in octo_records: + self.assertIsNotNone(provider._record_to_record_set( + gcloud_zone, record)) + + def test__get_gcloud_client(self): + provider = self._get_provider() + + self.assertIsInstance(provider, GoogleCloudProvider) + + @patch('octodns.provider.googlecloud.dns') + def test_populate(self, _): + def _get_mock_zones(page_token=None): + if not page_token: + return DummyIterator([ + DummyGoogleCloudZone('example.com.'), + DummyGoogleCloudZone('example2.com.'), + ], page_token="DUMMY_PAGE_TOKEN") + + return DummyIterator([ + google_cloud_zone + ]) + + def _get_mock_record_sets(page_token=None): + if not page_token: + return DummyIterator( + [DummyResourceRecordSet(*v) for v in + resource_record_sets[:5]], page_token="DUMMY_PAGE_TOKEN") + return DummyIterator( + [DummyResourceRecordSet(*v) for v in resource_record_sets[5:]]) + + google_cloud_zone = DummyGoogleCloudZone('unit.tests.') + + provider = self._get_provider() + provider.gcloud_client.list_zones = Mock(side_effect=_get_mock_zones) + google_cloud_zone.list_resource_record_sets = Mock( + side_effect=_get_mock_record_sets) + + self.assertEqual(provider._get_gcloud_zone("unit.tests.").dns_name, + "unit.tests.") + + test_zone = Zone('unit.tests.', []) + provider.populate(test_zone) + + # test_zone gets fed the same records as zone does, except it's in + # the format returned by google API, so after populate they should look + # excactly the same. + self.assertEqual(test_zone.records, zone.records) + + test_zone2 = Zone('nonexistant.zone.', []) + provider.populate(test_zone2, False, False) + + self.assertEqual(len(test_zone2.records), 0, + msg="Zone should not get records from wrong domain") + + provider.SUPPORTS = set() + test_zone3 = Zone('unit.tests.', []) + provider.populate(test_zone3) + self.assertEqual(len(test_zone3.records), 0) + + @patch('octodns.provider.googlecloud.dns') + def test_populate_corner_cases(self, _): + provider = self._get_provider() + test_zone = Zone('unit.tests.', []) + not_same_fqdn = DummyResourceRecordSet( + 'unit.tests.gr', u'A', 0, [u'1.2.3.4']), + + provider._get_gcloud_records = Mock( + side_effect=[not_same_fqdn]) + provider._get_gcloud_zone = Mock(return_value=DummyGoogleCloudZone( + dns_name="unit.tests.")) + + provider.populate(test_zone) + + self.assertEqual(len(test_zone.records), 1) + + self.assertEqual(test_zone.records.pop().fqdn, + u'unit.tests.gr.unit.tests.') + + def test__get_gcloud_zone(self): + provider = self._get_provider() + + provider.gcloud_client = Mock() + provider.gcloud_client.list_zones = Mock( + return_value=DummyIterator([])) + + self.assertIsNone(provider._get_gcloud_zone("nonexistant.xone"), + msg="Check that nonexistant zones return None when" + "there's no create=True flag") + + def test__create_zone(self): + provider = self._get_provider() + + provider.gcloud_client = Mock() + provider.gcloud_client.list_zones = Mock( + return_value=DummyIterator([])) + + mock_zone = provider._get_gcloud_zone( + 'nonexistant.zone.mock', create=True) + + mock_zone.create.assert_called() + provider.gcloud_client.zone.assert_called() + provider.gcloud_client.zone.assert_called_once_with( + dns_name=u'nonexistant.zone.mock', name=u'nonexistant-zone-moc') + + def test__create_zone_with_numbers_in_name(self): + provider = self._get_provider() + + provider.gcloud_client = Mock() + provider.gcloud_client.list_zones = Mock( + return_value=DummyIterator([])) + + provider._get_gcloud_zone( + '111.', create=True) + provider.gcloud_client.zone.assert_called_once_with( + dns_name=u'111.', name=u'a111') From f082d5798f10a9f68b90acccc408ef8617b6e5fb Mon Sep 17 00:00:00 2001 From: Petter Hassberg Date: Tue, 3 Oct 2017 09:12:02 +0200 Subject: [PATCH 08/24] Revert "Add .idea (intelliJ) files to gitignore." This reverts commit 5a3d844559f1d1514a171f1fa38d80a27a096ce9. --- .gitignore | 1 - 1 file changed, 1 deletion(-) diff --git a/.gitignore b/.gitignore index 1bca124..c45a684 100644 --- a/.gitignore +++ b/.gitignore @@ -11,4 +11,3 @@ output/ tmp/ build/ config/ -.idea/ \ No newline at end of file From 2a3690e8778a4bbaf5d84891a4cea23f38b68e61 Mon Sep 17 00:00:00 2001 From: Petter Hassberg Date: Tue, 3 Oct 2017 11:12:25 +0200 Subject: [PATCH 09/24] Add auth config opts to googlecloud provider Also make _data_for_SPF and _data_for_TXT the same method. --- octodns/provider/googlecloud.py | 32 +++++++++++++++------- tests/test_octodns_provider_googlecloud.py | 13 ++++++++- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/octodns/provider/googlecloud.py b/octodns/provider/googlecloud.py index dff11fc..8aa8c99 100644 --- a/octodns/provider/googlecloud.py +++ b/octodns/provider/googlecloud.py @@ -96,22 +96,34 @@ class GoogleCloudProvider(BaseProvider): # specified with the GOOGLE_APPLICATION_CREDENTIALS environment # variable. (https://console.cloud.google.com/apis/credentials) # - # The project to work on (not required) + # The project to work on (not required) # project: foobar + # + # The File with the google credentials (not required). If used, the + # "project" parameter needs to be set, else it will fall back to the + # "default credentials" + # credentials_file: ~/google_cloud_credentials_file.json + # """ SUPPORTS = set(('A', 'AAAA', 'CAA', 'CNAME', 'MX', 'NAPTR', 'NS', 'PTR', 'SPF', 'SRV', 'TXT')) SUPPORTS_GEO = False - def __init__(self, id, project=None, *args, **kwargs): + def __init__(self, id, project=None, credentials_file=None, + *args, **kwargs): + + if credentials_file: + self.gcloud_client = dns.Client.from_service_account_json( + credentials_file, project=project) + else: + self.gcloud_client = dns.Client(project=project) # Logger self.log = getLogger('GoogleCloudProvider[{}]'.format(id)) self.id = id super(GoogleCloudProvider, self).__init__(id, *args, **kwargs) - self.gcloud_client = dns.Client(project=project) def _apply(self, plan): """Required function of manager.py to actually apply a record change. @@ -345,7 +357,12 @@ class GoogleCloudProvider(BaseProvider): _data_for_PTR = _data_for_CNAME - _data_for_SPF = _data_for_A + def _data_for_SPF(self, gcloud_record): + if len(gcloud_record.rrdatas) > 1: + return { + 'values': gcloud_record.rrdatas} + return { + 'value': gcloud_record.rrdatas[0]} def _data_for_SRV(self, gcloud_record): return {'values': [{ @@ -355,9 +372,4 @@ class GoogleCloudProvider(BaseProvider): 'target': v[3]} for v in [shlex.split(g) for g in gcloud_record.rrdatas]]} - def _data_for_TXT(self, gcloud_record): - if len(gcloud_record.rrdatas) > 1: - return { - 'values': gcloud_record.rrdatas} - return { - 'value': gcloud_record.rrdatas[0]} + _data_for_TXT = _data_for_SPF diff --git a/tests/test_octodns_provider_googlecloud.py b/tests/test_octodns_provider_googlecloud.py index 568ac4f..c7b93e3 100644 --- a/tests/test_octodns_provider_googlecloud.py +++ b/tests/test_octodns_provider_googlecloud.py @@ -10,7 +10,7 @@ from octodns.provider.googlecloud import GoogleCloudProvider, \ _GoogleCloudRecordSetMaker from octodns.zone import Zone -from octodns.provider.base import Plan +from octodns.provider.base import Plan, BaseProvider from unittest import TestCase from mock import Mock, patch, PropertyMock @@ -216,6 +216,17 @@ class TestGoogleCloudProvider(TestCase): ''' return GoogleCloudProvider(id=1, project="mock") + @patch('octodns.provider.googlecloud.time.sleep') + @patch('octodns.provider.googlecloud.dns') + def test___init__(self, *_): + self.assertIsInstance(GoogleCloudProvider(id=1, + credentials_file="test", + project="unit test"), + BaseProvider) + + self.assertIsInstance(GoogleCloudProvider(id=1), + BaseProvider) + @patch('octodns.provider.googlecloud.time.sleep') @patch('octodns.provider.googlecloud.dns') def test__apply(self, *_): From 8230700ad17ebe9f8fcaf80212bec9a424d5f1ec Mon Sep 17 00:00:00 2001 From: Petter Hassberg Date: Tue, 3 Oct 2017 13:25:36 +0200 Subject: [PATCH 10/24] Consolidate googlecloud provider to single class remove _GoogleCloudRecordSetMaker into the GoogleCloudProvider, and consolidate methods. --- octodns/provider/googlecloud.py | 144 ++++++++------------- tests/test_octodns_provider_googlecloud.py | 38 +++--- 2 files changed, 72 insertions(+), 110 deletions(-) diff --git a/octodns/provider/googlecloud.py b/octodns/provider/googlecloud.py index 8aa8c99..0f20587 100644 --- a/octodns/provider/googlecloud.py +++ b/octodns/provider/googlecloud.py @@ -16,76 +16,6 @@ from .base import BaseProvider from ..record import Record -class _GoogleCloudRecordSetMaker(object): - """Wrapper to make google cloud client resource record sets from OctoDNS - Records. - - googlecloud.py: - class: octodns.provider.googlecloud._GoogleCloudRecordSetMaker - An _GoogleCloudRecordSetMaker creates google cloued client resource - records which can be used to update the Google Cloud DNS zones. - """ - - def __init__(self, gcloud_zone, record): - self.gcloud_zone = gcloud_zone - self.record = record - - self._record_set_func = getattr( - self, '_record_set_from_{}'.format(record._type)) - - def get_record_set(self): - return self._record_set_func(self.record) - - def _record_set_from_A(self, record): - return self.gcloud_zone.resource_record_set( - record.fqdn, record._type, record.ttl, record.values) - - _record_set_from_AAAA = _record_set_from_A - - def _record_set_from_CAA(self, record): - return self.gcloud_zone.resource_record_set( - record.fqdn, record._type, record.ttl, [ - '{flags} {tag} {value}'.format(**record.data['value'])]) - - def _record_set_from_CNAME(self, record): - return self.gcloud_zone.resource_record_set( - record.fqdn, record._type, record.ttl, [record.value]) - - def _record_set_from_MX(self, record): - return self.gcloud_zone.resource_record_set( - record.fqdn, record._type, record.ttl, [ - '{preference} {exchange}'.format(**v.data) - for v in record.values]) - - def _record_set_from_NAPTR(self, record): - return self.gcloud_zone.resource_record_set( - record.fqdn, record._type, record.ttl, [ - '{order} {preference} "{flags}" "{service}" ' - '"{regexp}" {replacement}' - .format(**v.data) for v in record.values]) - - _record_set_from_NS = _record_set_from_A - - _record_set_from_PTR = _record_set_from_CNAME - - _record_set_from_SPF = _record_set_from_A - - def _record_set_from_SRV(self, record): - return self.gcloud_zone.resource_record_set( - record.fqdn, record._type, record.ttl, [ - '{priority} {weight} {port} {target}' - .format(**v.data) for v in record.values]) - - def _record_set_from_TXT(self, record): - if 'values' in record.data: - val = record.data['values'] - else: - val = [record.data['value']] - - return self.gcloud_zone.resource_record_set( - record.fqdn, record._type, record.ttl, val) - - class GoogleCloudProvider(BaseProvider): """ Google Cloud DNS provider @@ -146,17 +76,20 @@ class GoogleCloudProvider(BaseProvider): for change in changes: class_name = change.__class__.__name__ + _rrset_func = getattr( + self, '_rrset_for_{}'.format(change.record._type)) + if class_name in 'Create': gcloud_changes.add_record_set( - self._record_to_record_set(gcloud_zone, change.record)) + _rrset_func(gcloud_zone, change.record)) elif class_name == 'Delete': gcloud_changes.delete_record_set( - self._record_to_record_set(gcloud_zone, change.record)) + _rrset_func(gcloud_zone, change.record)) elif class_name == 'Update': gcloud_changes.delete_record_set( - self._record_to_record_set(gcloud_zone, change.existing)) + _rrset_func(gcloud_zone, change.existing)) gcloud_changes.add_record_set( - self._record_to_record_set(gcloud_zone, change.new)) + _rrset_func(gcloud_zone, change.new)) else: raise RuntimeError('Change type "{}" for change "{!s}" ' 'is none of "Create", "Delete" or "Update' @@ -260,20 +193,6 @@ class GoogleCloudProvider(BaseProvider): if create: return self._create_gcloud_zone(dns_name) - @staticmethod - def _record_to_record_set(gcloud_zone, record): - """create google.cloud.dns.ResourceRecordSet from ocdodns.Record - - :param record: a record object - :type record: ocdodns.Record - :param gcloud_zone: a google gcloud zone - :type gcloud_zone: google.cloud.dns.ManagedZone - :type return: google.cloud.dns.ResourceRecordSet - """ - grm = _GoogleCloudRecordSetMaker(gcloud_zone, record) - - return grm.get_record_set() - def populate(self, zone, target=False, lenient=False): """Required function of manager.py to collect records from zone. @@ -373,3 +292,52 @@ class GoogleCloudProvider(BaseProvider): for v in [shlex.split(g) for g in gcloud_record.rrdatas]]} _data_for_TXT = _data_for_SPF + + def _rrset_for_A(self, gcloud_zone, record): + return gcloud_zone.resource_record_set( + record.fqdn, record._type, record.ttl, record.values) + + _rrset_for_AAAA = _rrset_for_A + + def _rrset_for_CAA(self, gcloud_zone, record): + return gcloud_zone.resource_record_set( + record.fqdn, record._type, record.ttl, [ + '{flags} {tag} {value}'.format(**record.data['value'])]) + + def _rrset_for_CNAME(self, gcloud_zone, record): + return gcloud_zone.resource_record_set( + record.fqdn, record._type, record.ttl, [record.value]) + + def _rrset_for_MX(self, gcloud_zone, record): + return gcloud_zone.resource_record_set( + record.fqdn, record._type, record.ttl, [ + '{preference} {exchange}'.format(**v.data) + for v in record.values]) + + def _rrset_for_NAPTR(self, gcloud_zone, record): + return gcloud_zone.resource_record_set( + record.fqdn, record._type, record.ttl, [ + '{order} {preference} "{flags}" "{service}" ' + '"{regexp}" {replacement}' + .format(**v.data) for v in record.values]) + + _rrset_for_NS = _rrset_for_A + + _rrset_for_PTR = _rrset_for_CNAME + + def _rrset_for_SPF(self, gcloud_zone, record): + if 'values' in record.data: + val = record.data['values'] + else: + val = [record.data['value']] + + return gcloud_zone.resource_record_set( + record.fqdn, record._type, record.ttl, val) + + def _rrset_for_SRV(self, gcloud_zone, record): + return gcloud_zone.resource_record_set( + record.fqdn, record._type, record.ttl, [ + '{priority} {weight} {port} {target}' + .format(**v.data) for v in record.values]) + + _rrset_for_TXT = _rrset_for_SPF diff --git a/tests/test_octodns_provider_googlecloud.py b/tests/test_octodns_provider_googlecloud.py index c7b93e3..76eedba 100644 --- a/tests/test_octodns_provider_googlecloud.py +++ b/tests/test_octodns_provider_googlecloud.py @@ -6,8 +6,7 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals from octodns.record import Create, Delete, Update, Record -from octodns.provider.googlecloud import GoogleCloudProvider, \ - _GoogleCloudRecordSetMaker +from octodns.provider.googlecloud import GoogleCloudProvider from octodns.zone import Zone from octodns.provider.base import Plan, BaseProvider @@ -194,19 +193,6 @@ class DummyIterator: return self.iterable.next() -class TestGoogleCloudRecordSetMaker(TestCase): - def test_get_record_set(self): - mz = DummyGoogleCloudZone('unit.tests.') - record_sets = [] - for record in octo_records: - mm = _GoogleCloudRecordSetMaker(mz, record) - record_sets.append(mm.get_record_set()) - - self.assertEqual( - len(octo_records), - len(record_sets)) - - class TestGoogleCloudProvider(TestCase): @patch('octodns.provider.googlecloud.dns') def _get_provider(*args): @@ -304,6 +290,10 @@ class TestGoogleCloudProvider(TestCase): unsupported_change = Mock() unsupported_change.__len__ = Mock(return_value=1) + type_mock = Mock() + type_mock._type = "A" + unsupported_change.record = type_mock + mock_plan = Mock() type(mock_plan).desired = PropertyMock(return_value=DummyDesired( "dummy name", [])) @@ -312,13 +302,6 @@ class TestGoogleCloudProvider(TestCase): with self.assertRaises(RuntimeError): provider.apply(mock_plan) - def test__record_to_record_set(self): - provider = self._get_provider() - gcloud_zone = DummyGoogleCloudZone('unit.tests.') - for record in octo_records: - self.assertIsNotNone(provider._record_to_record_set( - gcloud_zone, record)) - def test__get_gcloud_client(self): provider = self._get_provider() @@ -404,6 +387,17 @@ class TestGoogleCloudProvider(TestCase): msg="Check that nonexistant zones return None when" "there's no create=True flag") + def test__get_rrsets(self): + provider = self._get_provider() + dummy_gcloud_zone = DummyGoogleCloudZone("unit.tests") + for octo_record in octo_records: + _rrset_func = getattr( + provider, '_rrset_for_{}'.format(octo_record._type)) + self.assertEqual( + _rrset_func(dummy_gcloud_zone, octo_record).record_type, + octo_record._type + ) + def test__create_zone(self): provider = self._get_provider() From aabab630030ba7ad17ff705b4e89d082c9b0f413 Mon Sep 17 00:00:00 2001 From: Petter Hassberg Date: Sat, 7 Oct 2017 16:13:11 +0200 Subject: [PATCH 11/24] Refactor GoogleCloudProvider * in _rrset_for_X functions, use values instead of data attribute. * Small typo fixes and removals of redundant steps etc. * Unset GOOGLE_APPLICATION_CREDENTIALS in coverage script. --- octodns/provider/googlecloud.py | 34 ++++++++++------------ script/coverage | 1 + tests/test_octodns_provider_googlecloud.py | 12 ++++++-- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/octodns/provider/googlecloud.py b/octodns/provider/googlecloud.py index 0f20587..103c3f5 100644 --- a/octodns/provider/googlecloud.py +++ b/octodns/provider/googlecloud.py @@ -79,7 +79,7 @@ class GoogleCloudProvider(BaseProvider): _rrset_func = getattr( self, '_rrset_for_{}'.format(change.record._type)) - if class_name in 'Create': + if class_name == 'Create': gcloud_changes.add_record_set( _rrset_func(gcloud_zone, change.record)) elif class_name == 'Delete': @@ -212,17 +212,16 @@ class GoogleCloudProvider(BaseProvider): gcloud_zone = self._get_gcloud_zone(zone.name) - _records = set() if gcloud_zone: for gcloud_record in self._get_gcloud_records(gcloud_zone): - if gcloud_record.record_type.upper() in self.SUPPORTS: - _records.add(gcloud_record) - for gcloud_record in _records: + if gcloud_record.record_type.upper() not in self.SUPPORTS: + continue + record_name = gcloud_record.name if record_name.endswith(zone.name): # google cloud always return fqdn. Make relative record # here. "root" records will then get the '' record_name, - # which is also the way dyn likes it. + # which is also the way octodns likes it. record_name = record_name[:-(len(zone.name) + 1)] typ = gcloud_record.record_type.upper() data = getattr(self, '_data_for_{}'.format(typ)) @@ -302,7 +301,8 @@ class GoogleCloudProvider(BaseProvider): def _rrset_for_CAA(self, gcloud_zone, record): return gcloud_zone.resource_record_set( record.fqdn, record._type, record.ttl, [ - '{flags} {tag} {value}'.format(**record.data['value'])]) + '{} {} {}'.format(v.flags, v.tag, v.value) + for v in record.values]) def _rrset_for_CNAME(self, gcloud_zone, record): return gcloud_zone.resource_record_set( @@ -311,33 +311,29 @@ class GoogleCloudProvider(BaseProvider): def _rrset_for_MX(self, gcloud_zone, record): return gcloud_zone.resource_record_set( record.fqdn, record._type, record.ttl, [ - '{preference} {exchange}'.format(**v.data) + '{} {}'.format(v.preference, v.exchange) for v in record.values]) def _rrset_for_NAPTR(self, gcloud_zone, record): return gcloud_zone.resource_record_set( record.fqdn, record._type, record.ttl, [ - '{order} {preference} "{flags}" "{service}" ' - '"{regexp}" {replacement}' - .format(**v.data) for v in record.values]) + '{} {} "{}" "{}" "{}" {}'.format( + v.order, v.preference, v.flags, v.service, + v.regexp, v.replacement) for v in record.values]) _rrset_for_NS = _rrset_for_A _rrset_for_PTR = _rrset_for_CNAME def _rrset_for_SPF(self, gcloud_zone, record): - if 'values' in record.data: - val = record.data['values'] - else: - val = [record.data['value']] - return gcloud_zone.resource_record_set( - record.fqdn, record._type, record.ttl, val) + record.fqdn, record._type, record.ttl, record.values) def _rrset_for_SRV(self, gcloud_zone, record): return gcloud_zone.resource_record_set( record.fqdn, record._type, record.ttl, [ - '{priority} {weight} {port} {target}' - .format(**v.data) for v in record.values]) + '{} {} {} {}' + .format(v.priority, v.weight, v.port, v.target) + for v in record.values]) _rrset_for_TXT = _rrset_for_SPF diff --git a/script/coverage b/script/coverage index ca5d693..228a772 100755 --- a/script/coverage +++ b/script/coverage @@ -24,6 +24,7 @@ export DNSIMPLE_TOKEN= export DYN_CUSTOMER= export DYN_PASSWORD= export DYN_USERNAME= +export GOOGLE_APPLICATION_CREDENTIALS= coverage run --branch --source=octodns `which nosetests` --with-xunit "$@" coverage html diff --git a/tests/test_octodns_provider_googlecloud.py b/tests/test_octodns_provider_googlecloud.py index 76eedba..b498d4c 100644 --- a/tests/test_octodns_provider_googlecloud.py +++ b/tests/test_octodns_provider_googlecloud.py @@ -313,8 +313,11 @@ class TestGoogleCloudProvider(TestCase): if not page_token: return DummyIterator([ DummyGoogleCloudZone('example.com.'), + ], page_token="MOCK_PAGE_TOKEN") + elif page_token == "MOCK_PAGE_TOKEN": + return DummyIterator([ DummyGoogleCloudZone('example2.com.'), - ], page_token="DUMMY_PAGE_TOKEN") + ], page_token="MOCK_PAGE_TOKEN2") return DummyIterator([ google_cloud_zone @@ -324,7 +327,12 @@ class TestGoogleCloudProvider(TestCase): if not page_token: return DummyIterator( [DummyResourceRecordSet(*v) for v in - resource_record_sets[:5]], page_token="DUMMY_PAGE_TOKEN") + resource_record_sets[:3]], page_token="MOCK_PAGE_TOKEN") + elif page_token == "MOCK_PAGE_TOKEN": + + return DummyIterator( + [DummyResourceRecordSet(*v) for v in + resource_record_sets[3:5]], page_token="MOCK_PAGE_TOKEN2") return DummyIterator( [DummyResourceRecordSet(*v) for v in resource_record_sets[5:]]) From 4b878b844660b85f9cd1a9cc364850060cb3ec3d Mon Sep 17 00:00:00 2001 From: Petter Hassberg Date: Sat, 7 Oct 2017 19:31:23 +0200 Subject: [PATCH 12/24] Cache encountered zones in GoogleCloudProvider Cache googleclouds zones so that populate dont have to list all each time called. --- octodns/provider/googlecloud.py | 76 +++++++++++----------- tests/test_octodns_provider_googlecloud.py | 46 ++++++++----- 2 files changed, 68 insertions(+), 54 deletions(-) diff --git a/octodns/provider/googlecloud.py b/octodns/provider/googlecloud.py index 103c3f5..fb85744 100644 --- a/octodns/provider/googlecloud.py +++ b/octodns/provider/googlecloud.py @@ -53,6 +53,8 @@ class GoogleCloudProvider(BaseProvider): self.log = getLogger('GoogleCloudProvider[{}]'.format(id)) self.id = id + self._gcloud_zones = {} + super(GoogleCloudProvider, self).__init__(id, *args, **kwargs) def _apply(self, plan): @@ -70,7 +72,10 @@ class GoogleCloudProvider(BaseProvider): len(changes)) # Get gcloud zone, or create one if none existed before. - gcloud_zone = self._get_gcloud_zone(desired.name, create=True) + if desired.name not in self.gcloud_zones: + gcloud_zone = self._create_gcloud_zone(desired.name) + else: + gcloud_zone = self.gcloud_zones.get(desired.name) gcloud_changes = gcloud_zone.changes() @@ -117,13 +122,19 @@ class GoogleCloudProvider(BaseProvider): # and only contain lowercase letters, digits or dashes zone_name = re.sub("[^a-z0-9-]", "", dns_name[:-1].replace('.', "-")) - # make sure that the end result did not end up wo leading letter - if re.match('[^a-z]', zone_name[0]): - # I cannot think of a situation where a zone name derived from - # a domain name would'nt start with leading letter and thereby - # violate the constraint, however if such a situation is - # encountered, add a leading "a" here. - zone_name = "a%s" % zone_name + + # Check if there is another zone in google cloud which has the same + # name as the new one + while zone_name in [z.name for z in self.gcloud_zones.values()]: + # If there is a zone in google cloud alredy, then try suffixing the + # name with a -i where i is a number which keeps increasing until + # a free name has been reached. + m = re.match("^(.+)-([0-9]+$)", zone_name) + if m: + i = int(m.group(2)) + 1 + zone_name = "{}-{!s}".format(m.group(1), i) + else: + zone_name += "-2" gcloud_zone = self.gcloud_client.zone( name=zone_name, @@ -131,6 +142,9 @@ class GoogleCloudProvider(BaseProvider): ) gcloud_zone.create(client=self.gcloud_client) + # add this new zone to the list of zones. + self._gcloud_zones[gcloud_zone.dns_name] = gcloud_zone + self.log.info("Created zone %s. Fqdn %s." % (zone_name, dns_name)) @@ -159,39 +173,25 @@ class GoogleCloudProvider(BaseProvider): # yield from is in python 3 only. yield gcloud_record - def _get_gcloud_zone(self, dns_name, page_token=None, create=False): - """Return the ManagedZone which has has the matching dns_name, or - None if no such zone exist, unless create=True, then create a new - one and return it. + def _get_cloud_zones(self, page_token=None): + """Load all ManagedZones into the self._gcloud_zones dict which is + mapped with the dns_name as key. - :param dns_name: fqdn of dns name for zone to get. - :type dns_name: str - :param page_token: page token for the page to get - :type page_token: str - :param create: if true, create ManagedZone if it does not exist - already - - :type return: new google.cloud.dns.ManagedZone + :return: void """ - # Find the google name for the incoming zone + gcloud_zones = self.gcloud_client.list_zones(page_token=page_token) for gcloud_zone in gcloud_zones: - if gcloud_zone.dns_name == dns_name: - return gcloud_zone - else: - # Zone not found. Check if there are more results which could be - # retrieved by checking "next_page_token". - if gcloud_zones.next_page_token: - return self._get_gcloud_zone(dns_name, - gcloud_zones.next_page_token) - else: - # Nothing found, either return None or else create zone and - # return that one (if create=True) - self.log.debug('_get_gcloud_zone: zone name=%s, ' - 'was not found by %s.', - dns_name, self.gcloud_client) - if create: - return self._create_gcloud_zone(dns_name) + self._gcloud_zones[gcloud_zone.dns_name] = gcloud_zone + + if gcloud_zones.next_page_token: + self._get_cloud_zones(gcloud_zones.next_page_token) + + @property + def gcloud_zones(self): + if not self._gcloud_zones: + self._get_cloud_zones() + return self._gcloud_zones def populate(self, zone, target=False, lenient=False): """Required function of manager.py to collect records from zone. @@ -210,7 +210,7 @@ class GoogleCloudProvider(BaseProvider): target, lenient) before = len(zone.records) - gcloud_zone = self._get_gcloud_zone(zone.name) + gcloud_zone = self.gcloud_zones.get(zone.name) if gcloud_zone: for gcloud_record in self._get_gcloud_records(gcloud_zone): diff --git a/tests/test_octodns_provider_googlecloud.py b/tests/test_octodns_provider_googlecloud.py index b498d4c..fa667cc 100644 --- a/tests/test_octodns_provider_googlecloud.py +++ b/tests/test_octodns_provider_googlecloud.py @@ -164,8 +164,9 @@ class DummyResourceRecordSet: class DummyGoogleCloudZone: - def __init__(self, dns_name): + def __init__(self, dns_name, name=""): self.dns_name = dns_name + self.name = name def resource_record_set(self, *args): return DummyResourceRecordSet(*args) @@ -173,6 +174,9 @@ class DummyGoogleCloudZone: def list_resource_record_sets(self, *args): pass + def create(self, *args, **kwargs): + pass + class DummyIterator: """Returns a mock DummyIterator object to use in testing. @@ -239,7 +243,7 @@ class TestGoogleCloudProvider(TestCase): 'type': 'A', 'values': ['1.4.3.2']}) - gcloud_zone_mock = DummyGoogleCloudZone("unit.tests.") + gcloud_zone_mock = DummyGoogleCloudZone("unit.tests.", "unit-tests") status_mock = Mock() return_values_for_status = iter( ['', '', '', '', '', '', '', '', '', '', '', '', '', '', '', @@ -250,10 +254,9 @@ class TestGoogleCloudProvider(TestCase): provider = self._get_provider() provider.gcloud_client = Mock() - provider._get_gcloud_zone = Mock( - return_value=gcloud_zone_mock) + provider._gcloud_zones = {"unit.tests.": gcloud_zone_mock} desired = Mock() - desired.name = Mock(return_value="unit.tests.") + desired.name = "unit.tests." changes = [] changes.append(Create(create_r)) changes.append(Delete(delete_r)) @@ -343,7 +346,7 @@ class TestGoogleCloudProvider(TestCase): google_cloud_zone.list_resource_record_sets = Mock( side_effect=_get_mock_record_sets) - self.assertEqual(provider._get_gcloud_zone("unit.tests.").dns_name, + self.assertEqual(provider.gcloud_zones.get("unit.tests.").dns_name, "unit.tests.") test_zone = Zone('unit.tests.', []) @@ -374,8 +377,8 @@ class TestGoogleCloudProvider(TestCase): provider._get_gcloud_records = Mock( side_effect=[not_same_fqdn]) - provider._get_gcloud_zone = Mock(return_value=DummyGoogleCloudZone( - dns_name="unit.tests.")) + provider._gcloud_zones = { + "unit.tests.": DummyGoogleCloudZone("unit.tests.", "unit-tests")} provider.populate(test_zone) @@ -391,7 +394,7 @@ class TestGoogleCloudProvider(TestCase): provider.gcloud_client.list_zones = Mock( return_value=DummyIterator([])) - self.assertIsNone(provider._get_gcloud_zone("nonexistant.xone"), + self.assertIsNone(provider.gcloud_zones.get("nonexistant.xone"), msg="Check that nonexistant zones return None when" "there's no create=True flag") @@ -413,22 +416,33 @@ class TestGoogleCloudProvider(TestCase): provider.gcloud_client.list_zones = Mock( return_value=DummyIterator([])) - mock_zone = provider._get_gcloud_zone( - 'nonexistant.zone.mock', create=True) + mock_zone = provider._create_gcloud_zone("nonexistant.zone.mock") mock_zone.create.assert_called() provider.gcloud_client.zone.assert_called() provider.gcloud_client.zone.assert_called_once_with( dns_name=u'nonexistant.zone.mock', name=u'nonexistant-zone-moc') - def test__create_zone_with_numbers_in_name(self): + def test__create_zone_with_duplicate_names(self): + + def _create_dummy_zone(name, dns_name): + return DummyGoogleCloudZone(name=name, dns_name=dns_name) + provider = self._get_provider() provider.gcloud_client = Mock() + provider.gcloud_client.zone = Mock(side_effect=_create_dummy_zone) provider.gcloud_client.list_zones = Mock( return_value=DummyIterator([])) - provider._get_gcloud_zone( - '111.', create=True) - provider.gcloud_client.zone.assert_called_once_with( - dns_name=u'111.', name=u'a111') + _gcloud_zones = { + 'unit-tests': DummyGoogleCloudZone("a.unit-tests.", "unit-tests") + } + + provider._gcloud_zones = _gcloud_zones + + test_zone_1 = provider._create_gcloud_zone("unit.tests.") + self.assertEqual(test_zone_1.name, "unit-tests-2") + + test_zone_2 = provider._create_gcloud_zone("unit.tests.") + self.assertEqual(test_zone_2.name, "unit-tests-3") From e9d90bda2bd11ec740c73162c5af268183ea76fe Mon Sep 17 00:00:00 2001 From: Petter Hassberg Date: Sat, 7 Oct 2017 20:46:35 +0200 Subject: [PATCH 13/24] Add timeout logic to googlecloud provider --- octodns/provider/googlecloud.py | 21 +++++++++++++++------ tests/test_octodns_provider_googlecloud.py | 13 ++++++++++--- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/octodns/provider/googlecloud.py b/octodns/provider/googlecloud.py index fb85744..e339c74 100644 --- a/octodns/provider/googlecloud.py +++ b/octodns/provider/googlecloud.py @@ -40,6 +40,8 @@ class GoogleCloudProvider(BaseProvider): 'NS', 'PTR', 'SPF', 'SRV', 'TXT')) SUPPORTS_GEO = False + CHANGE_LOOP_WAIT = 5 + def __init__(self, id, project=None, credentials_file=None, *args, **kwargs): @@ -101,13 +103,20 @@ class GoogleCloudProvider(BaseProvider): .format(class_name, change)) gcloud_changes.create() - i = 1 - while gcloud_changes.status != 'done': - self.log.debug("Waiting for changes to complete") - time.sleep(i) + + for i in range(120): gcloud_changes.reload() - if i < 30: - i += 2 + self.log.debug("Waiting for changes to complete") + # https://cloud.google.com/dns/api/v1/changes#resource + # status can be one of either "pending" or "done" + if gcloud_changes.status != 'pending': + break + self.log.debug("Waiting for changes to complete") + time.sleep(self.CHANGE_LOOP_WAIT) + + if gcloud_changes.status != 'done': + raise RuntimeError("Timeout reached after {} seconds".format( + i * self.CHANGE_LOOP_WAIT)) def _create_gcloud_zone(self, dns_name): """Creates a google cloud ManagedZone with dns_name, and zone named diff --git a/tests/test_octodns_provider_googlecloud.py b/tests/test_octodns_provider_googlecloud.py index fa667cc..c2e976c 100644 --- a/tests/test_octodns_provider_googlecloud.py +++ b/tests/test_octodns_provider_googlecloud.py @@ -206,7 +206,6 @@ class TestGoogleCloudProvider(TestCase): ''' return GoogleCloudProvider(id=1, project="mock") - @patch('octodns.provider.googlecloud.time.sleep') @patch('octodns.provider.googlecloud.dns') def test___init__(self, *_): self.assertIsInstance(GoogleCloudProvider(id=1, @@ -246,8 +245,7 @@ class TestGoogleCloudProvider(TestCase): gcloud_zone_mock = DummyGoogleCloudZone("unit.tests.", "unit-tests") status_mock = Mock() return_values_for_status = iter( - ['', '', '', '', '', '', '', '', '', '', '', '', '', '', '', - '', '', '', 'done']) + ["pending"] * 11 + ['done', 'done']) type(status_mock).status = PropertyMock( side_effect=return_values_for_status.next) gcloud_zone_mock.changes = Mock(return_value=status_mock) @@ -291,6 +289,15 @@ class TestGoogleCloudProvider(TestCase): 'aa.unit.tests.', 'A', 9001, ['1.2.4.3']) ]) + type(status_mock).status = "pending" + + with self.assertRaises(RuntimeError): + provider.apply(Plan( + existing=[update_existing_r, delete_r], + desired=desired, + changes=changes + )) + unsupported_change = Mock() unsupported_change.__len__ = Mock(return_value=1) type_mock = Mock() From ea1871a326d04243b6f632daa588e3bf15384e8f Mon Sep 17 00:00:00 2001 From: Petter Hassberg Date: Sat, 7 Oct 2017 20:50:49 +0200 Subject: [PATCH 14/24] Add GoogleCloudProvider to README.md --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index afe92ac..a910b5b 100644 --- a/README.md +++ b/README.md @@ -153,6 +153,7 @@ The above command pulled the existing data out of Route53 and placed the results | [CloudflareProvider](/octodns/provider/cloudflare.py) | A, AAAA, CAA, CNAME, MX, NS, SPF, TXT | No | CAA tags restricted | | [DnsimpleProvider](/octodns/provider/dnsimple.py) | All | No | CAA tags restricted | | [DynProvider](/octodns/provider/dyn.py) | All | Yes | | +| [GoogleCloudProvider](/octodns/provider/googlecloud.py) | A, AAAA, CAA, CNAME, MX, NAPTR, NS, PTR, SPF, SRV, TXT | No | | | [Ns1Provider](/octodns/provider/ns1.py) | All | No | | | [OVH](/octodns/provider/ovh.py) | A, AAAA, CNAME, MX, NAPTR, NS, PTR, SPF, SRV, SSHFP, TXT | No | | | [PowerDnsProvider](/octodns/provider/powerdns.py) | All | No | | From f50db5e02b54c8c54aaedee6411d9f94b495aec6 Mon Sep 17 00:00:00 2001 From: Petter Hassberg Date: Mon, 9 Oct 2017 20:03:01 +0200 Subject: [PATCH 15/24] Use chunked_values in GoogleCloudProvider --- octodns/provider/googlecloud.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/octodns/provider/googlecloud.py b/octodns/provider/googlecloud.py index e339c74..c544d63 100644 --- a/octodns/provider/googlecloud.py +++ b/octodns/provider/googlecloud.py @@ -336,7 +336,7 @@ class GoogleCloudProvider(BaseProvider): def _rrset_for_SPF(self, gcloud_zone, record): return gcloud_zone.resource_record_set( - record.fqdn, record._type, record.ttl, record.values) + record.fqdn, record._type, record.ttl, record.chunked_values) def _rrset_for_SRV(self, gcloud_zone, record): return gcloud_zone.resource_record_set( From a012e923f61547316d3e241d9e38b22a19ee1704 Mon Sep 17 00:00:00 2001 From: Joe Williams Date: Tue, 10 Oct 2017 13:54:52 -0700 Subject: [PATCH 16/24] add ability to configure update/delete thresholds --- octodns/provider/base.py | 25 ++++++++++++++++++++----- tests/test_octodns_provider_base.py | 2 ++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/octodns/provider/base.py b/octodns/provider/base.py index d2561ba..86175e7 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -21,10 +21,14 @@ class Plan(object): MAX_SAFE_DELETE_PCENT = .3 MIN_EXISTING_RECORDS = 10 - def __init__(self, existing, desired, changes): + 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, @@ -55,14 +59,19 @@ class Plan(object): update_pcent = self.change_counts['Update'] / existing_record_count delete_pcent = self.change_counts['Delete'] / existing_record_count - if update_pcent > self.MAX_SAFE_UPDATE_PCENT: + self.log.debug('raise_if_unsafe: update_pcent_threshold=%d, ' + 'delete_pcent_threshold=%d', + self.update_pcent_threshold, + self.delete_pcent_threshold) + + 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.MAX_SAFE_DELETE_PCENT: + if delete_pcent > self.delete_pcent_threshold: raise UnsafePlan('Too many deletes, {} is over {} percent' '({}/{})'.format( delete_pcent, @@ -79,11 +88,15 @@ class Plan(object): class BaseProvider(BaseSource): - def __init__(self, id, apply_disabled=False): + def __init__(self, id, apply_disabled=False, + update_pcent_threshold=Plan.MAX_SAFE_UPDATE_PCENT, + delete_pcent_threshold=Plan.MAX_SAFE_DELETE_PCENT): super(BaseProvider, self).__init__(id) self.log.debug('__init__: id=%s, apply_disabled=%s', id, apply_disabled) self.apply_disabled = apply_disabled + self.update_pcent_threshold = update_pcent_threshold + self.delete_pcent_threshold = delete_pcent_threshold def _include_change(self, change): ''' @@ -124,7 +137,9 @@ class BaseProvider(BaseSource): changes += extra if changes: - plan = Plan(existing, desired, changes) + plan = Plan(existing, desired, changes, + self.update_pcent_threshold, + self.delete_pcent_threshold) self.log.info('plan: %s', plan) return plan self.log.info('plan: No changes') diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index e44adc0..f761405 100644 --- a/tests/test_octodns_provider_base.py +++ b/tests/test_octodns_provider_base.py @@ -23,6 +23,8 @@ class HelperProvider(BaseProvider): self.__extra_changes = extra_changes self.apply_disabled = apply_disabled self.include_change_callback = include_change_callback + self.update_pcent_threshold = Plan.MAX_SAFE_UPDATE_PCENT + self.delete_pcent_threshold = Plan.MAX_SAFE_DELETE_PCENT def populate(self, zone, target=False, lenient=False): pass From 50ac2f794cf01da8ac0cc48e5e462b019592da9a Mon Sep 17 00:00:00 2001 From: Joe Williams Date: Tue, 10 Oct 2017 14:39:25 -0700 Subject: [PATCH 17/24] add tests --- tests/test_octodns_provider_base.py | 56 +++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index f761405..fde1396 100644 --- a/tests/test_octodns_provider_base.py +++ b/tests/test_octodns_provider_base.py @@ -290,3 +290,59 @@ class TestBaseProvider(TestCase): Plan.MAX_SAFE_DELETE_PCENT))] Plan(zone, zone, changes).raise_if_unsafe() + + def test_safe_updates_min_existing_override(self): + safe_pcent = .4 + # 40% + 1 fails when more + # than MIN_EXISTING_RECORDS exist + zone = Zone('unit.tests.', []) + record = Record.new(zone, 'a', { + 'ttl': 30, + 'type': 'A', + 'value': '1.2.3.4', + }) + + for i in range(int(Plan.MIN_EXISTING_RECORDS)): + zone.add_record(Record.new(zone, str(i), { + 'ttl': 60, + 'type': 'A', + 'value': '2.3.4.5' + })) + + changes = [Update(record, record) + for i in range(int(Plan.MIN_EXISTING_RECORDS * + safe_pcent) + 1)] + + with self.assertRaises(UnsafePlan) as ctx: + Plan(zone, zone, changes, + update_pcent_threshold=safe_pcent).raise_if_unsafe() + + self.assertTrue('Too many updates' in ctx.exception.message) + + def test_safe_deletes_min_existing_override(self): + safe_pcent = .4 + # 40% + 1 fails when more + # than MIN_EXISTING_RECORDS exist + zone = Zone('unit.tests.', []) + record = Record.new(zone, 'a', { + 'ttl': 30, + 'type': 'A', + 'value': '1.2.3.4', + }) + + for i in range(int(Plan.MIN_EXISTING_RECORDS)): + zone.add_record(Record.new(zone, str(i), { + 'ttl': 60, + 'type': 'A', + 'value': '2.3.4.5' + })) + + changes = [Delete(record) + for i in range(int(Plan.MIN_EXISTING_RECORDS * + safe_pcent) + 1)] + + with self.assertRaises(UnsafePlan) as ctx: + Plan(zone, zone, changes, + delete_pcent_threshold=safe_pcent).raise_if_unsafe() + + self.assertTrue('Too many deletes' in ctx.exception.message) From 3562b0dd4cf3abc5bc84d5a7abf38ac2c7766985 Mon Sep 17 00:00:00 2001 From: Joe Williams Date: Tue, 10 Oct 2017 15:05:58 -0700 Subject: [PATCH 18/24] log this in init --- octodns/provider/base.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/octodns/provider/base.py b/octodns/provider/base.py index 86175e7..f6ff1b7 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -59,11 +59,6 @@ class Plan(object): update_pcent = self.change_counts['Update'] / existing_record_count delete_pcent = self.change_counts['Delete'] / existing_record_count - self.log.debug('raise_if_unsafe: update_pcent_threshold=%d, ' - 'delete_pcent_threshold=%d', - self.update_pcent_threshold, - self.delete_pcent_threshold) - if update_pcent > self.update_pcent_threshold: raise UnsafePlan('Too many updates, {} is over {} percent' '({}/{})'.format( @@ -92,8 +87,12 @@ class BaseProvider(BaseSource): update_pcent_threshold=Plan.MAX_SAFE_UPDATE_PCENT, delete_pcent_threshold=Plan.MAX_SAFE_DELETE_PCENT): super(BaseProvider, self).__init__(id) - self.log.debug('__init__: id=%s, apply_disabled=%s', id, - apply_disabled) + self.log.debug('__init__: id=%s, apply_disabled=%s, ' + 'update_pcent_threshold=%d, delete_pcent_threshold=%d', + id, + apply_disabled, + update_pcent_threshold, + delete_pcent_threshold) self.apply_disabled = apply_disabled self.update_pcent_threshold = update_pcent_threshold self.delete_pcent_threshold = delete_pcent_threshold From ffeceb39b19ab85789e83cb2b72ea2cdba3f1425 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 13 Oct 2017 13:15:24 -0700 Subject: [PATCH 19/24] Handle Manager.dump with an empty Zone --- octodns/manager.py | 4 +++- tests/test_octodns_manager.py | 12 ++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/octodns/manager.py b/octodns/manager.py index 8439eb6..36a3592 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -11,7 +11,7 @@ from importlib import import_module from os import environ import logging -from .provider.base import BaseProvider +from .provider.base import BaseProvider, Plan from .provider.yaml import YamlProvider from .record import Record from .yaml import safe_load @@ -362,6 +362,8 @@ class Manager(object): source.populate(zone, lenient=lenient) plan = target.plan(zone) + if plan is None: + plan = Plan(zone, zone, []) target.apply(plan) def validate_configs(self): diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 45a3b55..a5f2022 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -11,6 +11,7 @@ from unittest import TestCase from octodns.record import Record from octodns.manager import _AggregateTarget, MainThreadExecutor, Manager +from octodns.yaml import safe_load from octodns.zone import Zone from helpers import GeoProvider, NoSshFpProvider, SimpleProvider, \ @@ -211,6 +212,17 @@ class TestManager(TestCase): with self.assertRaises(IOError): manager.dump('unknown.zone.', tmpdir.dirname, False, 'in') + def test_dump_empty(self): + with TemporaryDirectory() as tmpdir: + environ['YAML_TMP_DIR'] = tmpdir.dirname + manager = Manager(get_config_filename('simple.yaml')) + + manager.dump('empty.', tmpdir.dirname, False, 'in') + + with open(join(tmpdir.dirname, 'empty.yaml')) as fh: + data = safe_load(fh, False) + self.assertFalse(data) + def test_validate_configs(self): Manager(get_config_filename('simple-validate.yaml')).validate_configs() From f45ff51062ef5377c4c9b01edb0098e5dc0a78eb Mon Sep 17 00:00:00 2001 From: Petter Hassberg Date: Sat, 14 Oct 2017 08:06:06 +0200 Subject: [PATCH 20/24] Fix various logging lines in GoogleCloudProvider. --- octodns/provider/googlecloud.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/octodns/provider/googlecloud.py b/octodns/provider/googlecloud.py index c544d63..370c750 100644 --- a/octodns/provider/googlecloud.py +++ b/octodns/provider/googlecloud.py @@ -106,7 +106,6 @@ class GoogleCloudProvider(BaseProvider): for i in range(120): gcloud_changes.reload() - self.log.debug("Waiting for changes to complete") # https://cloud.google.com/dns/api/v1/changes#resource # status can be one of either "pending" or "done" if gcloud_changes.status != 'pending': @@ -154,8 +153,7 @@ class GoogleCloudProvider(BaseProvider): # add this new zone to the list of zones. self._gcloud_zones[gcloud_zone.dns_name] = gcloud_zone - self.log.info("Created zone %s. Fqdn %s." % - (zone_name, dns_name)) + self.log.info("Created zone {}. Fqdn {}.".format(zone_name, dns_name)) return gcloud_zone From 7958618f63a3ecf541a228061b901ba6b02e34d9 Mon Sep 17 00:00:00 2001 From: Petter Hassberg Date: Sat, 14 Oct 2017 19:32:24 +0200 Subject: [PATCH 21/24] Use uuid4 for zone name in GoogleCloudProvider use uuid4().hex to ensure unique zone_name generation and thereby streamline with the other providers. --- octodns/provider/googlecloud.py | 19 +++------------- tests/test_octodns_provider_googlecloud.py | 26 ---------------------- 2 files changed, 3 insertions(+), 42 deletions(-) diff --git a/octodns/provider/googlecloud.py b/octodns/provider/googlecloud.py index 370c750..6ca0794 100644 --- a/octodns/provider/googlecloud.py +++ b/octodns/provider/googlecloud.py @@ -5,10 +5,10 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals -import re import shlex import time from logging import getLogger +from uuid import uuid4 from google.cloud import dns @@ -128,21 +128,8 @@ class GoogleCloudProvider(BaseProvider): """ # Zone name must begin with a letter, end with a letter or digit, # and only contain lowercase letters, digits or dashes - zone_name = re.sub("[^a-z0-9-]", "", - dns_name[:-1].replace('.', "-")) - - # Check if there is another zone in google cloud which has the same - # name as the new one - while zone_name in [z.name for z in self.gcloud_zones.values()]: - # If there is a zone in google cloud alredy, then try suffixing the - # name with a -i where i is a number which keeps increasing until - # a free name has been reached. - m = re.match("^(.+)-([0-9]+$)", zone_name) - if m: - i = int(m.group(2)) + 1 - zone_name = "{}-{!s}".format(m.group(1), i) - else: - zone_name += "-2" + zone_name = '{}-{}'.format( + dns_name[:-1].replace('.', '-'), uuid4().hex) gcloud_zone = self.gcloud_client.zone( name=zone_name, diff --git a/tests/test_octodns_provider_googlecloud.py b/tests/test_octodns_provider_googlecloud.py index c2e976c..adc2112 100644 --- a/tests/test_octodns_provider_googlecloud.py +++ b/tests/test_octodns_provider_googlecloud.py @@ -427,29 +427,3 @@ class TestGoogleCloudProvider(TestCase): mock_zone.create.assert_called() provider.gcloud_client.zone.assert_called() - provider.gcloud_client.zone.assert_called_once_with( - dns_name=u'nonexistant.zone.mock', name=u'nonexistant-zone-moc') - - def test__create_zone_with_duplicate_names(self): - - def _create_dummy_zone(name, dns_name): - return DummyGoogleCloudZone(name=name, dns_name=dns_name) - - provider = self._get_provider() - - provider.gcloud_client = Mock() - provider.gcloud_client.zone = Mock(side_effect=_create_dummy_zone) - provider.gcloud_client.list_zones = Mock( - return_value=DummyIterator([])) - - _gcloud_zones = { - 'unit-tests': DummyGoogleCloudZone("a.unit-tests.", "unit-tests") - } - - provider._gcloud_zones = _gcloud_zones - - test_zone_1 = provider._create_gcloud_zone("unit.tests.") - self.assertEqual(test_zone_1.name, "unit-tests-2") - - test_zone_2 = provider._create_gcloud_zone("unit.tests.") - self.assertEqual(test_zone_2.name, "unit-tests-3") From d2e1aafa8588ba8023e3c04a5785ecd3c3effd1d Mon Sep 17 00:00:00 2001 From: Nicolas KAROLAK Date: Mon, 16 Oct 2017 18:49:28 +0200 Subject: [PATCH 22/24] use . instead of source [SC2039] --- .git_hooks_pre-commit | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.git_hooks_pre-commit b/.git_hooks_pre-commit index 9cb854a..c17906b 100755 --- a/.git_hooks_pre-commit +++ b/.git_hooks_pre-commit @@ -6,6 +6,6 @@ HOOKS=`dirname $0` GIT=`dirname $HOOKS` ROOT=`dirname $GIT` -source $ROOT/env/bin/activate +. $ROOT/env/bin/activate $ROOT/script/lint $ROOT/script/test From 8352ab89efb02b7bab9138b1e11a4de8c99e6e70 Mon Sep 17 00:00:00 2001 From: Tim Hughes Date: Tue, 17 Oct 2017 16:16:17 +0100 Subject: [PATCH 23/24] adds warning to dyn provider when it cannot load a trafficdirector --- octodns/provider/dyn.py | 3 ++- tests/test_octodns_provider_dyn.py | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/octodns/provider/dyn.py b/octodns/provider/dyn.py index 721b3a7..65acf1d 100644 --- a/octodns/provider/dyn.py +++ b/octodns/provider/dyn.py @@ -290,7 +290,8 @@ class DynProvider(BaseProvider): for td in get_all_dsf_services(): try: fqdn, _type = td.label.split(':', 1) - except ValueError: + except ValueError as e: + self.log.warn("Failed to load TraficDirector '{}': {}".format(td.label, e)) continue tds[fqdn][_type] = td self._traffic_directors = dict(tds) diff --git a/tests/test_octodns_provider_dyn.py b/tests/test_octodns_provider_dyn.py index 9be253d..5e4c86b 100644 --- a/tests/test_octodns_provider_dyn.py +++ b/tests/test_octodns_provider_dyn.py @@ -8,7 +8,7 @@ from __future__ import absolute_import, division, print_function, \ from dyn.tm.errors import DynectGetError from dyn.tm.services.dsf import DSFResponsePool from json import loads -from mock import MagicMock, call, patch +from mock import MagicMock, call, patch, Mock from unittest import TestCase from octodns.record import Create, Delete, Record, Update @@ -601,6 +601,7 @@ class TestDynProviderGeo(TestCase): provider = DynProvider('test', 'cust', 'user', 'pass', True) # short-circuit session checking provider._dyn_sess = True + provider.log.warn = MagicMock() # no tds mock.side_effect = [{'data': []}] @@ -649,6 +650,8 @@ class TestDynProviderGeo(TestCase): set(tds.keys())) self.assertEquals(['A'], tds['unit.tests.'].keys()) self.assertEquals(['A'], tds['geo.unit.tests.'].keys()) + provider.log.warn.assert_called_with("Failed to load TraficDirector 'something else': need more than 1 value to unpack") + @patch('dyn.core.SessionEngine.execute') def test_traffic_director_monitor(self, mock): From f39e1d28c810138daa62a86cafdc39492bcad670 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 18 Oct 2017 10:21:10 -0700 Subject: [PATCH 24/24] Fix log formatting and lint compliance --- octodns/provider/dyn.py | 3 ++- tests/test_octodns_provider_dyn.py | 8 +++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/octodns/provider/dyn.py b/octodns/provider/dyn.py index 65acf1d..010fd31 100644 --- a/octodns/provider/dyn.py +++ b/octodns/provider/dyn.py @@ -291,7 +291,8 @@ class DynProvider(BaseProvider): try: fqdn, _type = td.label.split(':', 1) except ValueError as e: - self.log.warn("Failed to load TraficDirector '{}': {}".format(td.label, e)) + self.log.warn("Failed to load TraficDirector '%s': %s", + td.label, e.message) continue tds[fqdn][_type] = td self._traffic_directors = dict(tds) diff --git a/tests/test_octodns_provider_dyn.py b/tests/test_octodns_provider_dyn.py index 5e4c86b..4415347 100644 --- a/tests/test_octodns_provider_dyn.py +++ b/tests/test_octodns_provider_dyn.py @@ -8,7 +8,7 @@ from __future__ import absolute_import, division, print_function, \ from dyn.tm.errors import DynectGetError from dyn.tm.services.dsf import DSFResponsePool from json import loads -from mock import MagicMock, call, patch, Mock +from mock import MagicMock, call, patch from unittest import TestCase from octodns.record import Create, Delete, Record, Update @@ -650,8 +650,10 @@ class TestDynProviderGeo(TestCase): set(tds.keys())) self.assertEquals(['A'], tds['unit.tests.'].keys()) self.assertEquals(['A'], tds['geo.unit.tests.'].keys()) - provider.log.warn.assert_called_with("Failed to load TraficDirector 'something else': need more than 1 value to unpack") - + provider.log.warn.assert_called_with("Failed to load TraficDirector " + "'%s': %s", 'something else', + 'need more than 1 value to ' + 'unpack') @patch('dyn.core.SessionEngine.execute') def test_traffic_director_monitor(self, mock):