From fd3de1e08b45cfd5324b7b9949e42914012c91d0 Mon Sep 17 00:00:00 2001 From: Adam Smith Date: Sat, 29 Sep 2018 16:23:20 -0700 Subject: [PATCH 01/29] add Zone File source, reads Bind compatible zone files --- README.md | 1 + octodns/source/axfr.py | 72 ++++++++++++++++++++++ tests/test_octodns_source_axfr.py | 35 ++++++++++- tests/zones/invalid.zone. | 8 +++ tests/zones/{unit.tests.db => unit.tests.} | 0 5 files changed, 114 insertions(+), 2 deletions(-) create mode 100644 tests/zones/invalid.zone. rename tests/zones/{unit.tests.db => unit.tests.} (100%) diff --git a/README.md b/README.md index 7ef2f1c..8ecbfa9 100644 --- a/README.md +++ b/README.md @@ -163,6 +163,7 @@ The above command pulled the existing data out of Route53 and placed the results | [Rackspace](/octodns/provider/rackspace.py) | | A, AAAA, ALIAS, CNAME, MX, NS, PTR, SPF, TXT | No | | | [Route53](/octodns/provider/route53.py) | boto3 | A, AAAA, CAA, CNAME, MX, NAPTR, NS, PTR, SPF, SRV, TXT | Yes | | | [AxfrSource](/octodns/source/axfr.py) | | A, AAAA, CNAME, MX, NS, PTR, SPF, SRV, TXT | No | read-only | +| [ZoneFileSource](/octodns/source/axfr.py) | | A, AAAA, CNAME, MX, NS, PTR, SPF, SRV, TXT | No | read-only | | [TinyDnsFileSource](/octodns/source/tinydns.py) | | A, CNAME, MX, NS, PTR | No | read-only | | [YamlProvider](/octodns/provider/yaml.py) | | All | Yes | config | diff --git a/octodns/source/axfr.py b/octodns/source/axfr.py index 42d5fc0..715a36b 100644 --- a/octodns/source/axfr.py +++ b/octodns/source/axfr.py @@ -13,6 +13,8 @@ import dns.rdatatype from dns.exception import DNSException from collections import defaultdict +from os import listdir +from os.path import join import logging from ..record import Record @@ -160,3 +162,73 @@ class AxfrSource(AxfrBaseSource): }) return records + + +class ZoneFileSourceException(Exception): + pass + + +class ZoneFileSourceNotFound(ZoneFileSourceException): + + def __init__(self): + super(ZoneFileSourceNotFound, self).__init__( + 'Zone file not found') + + +class ZoneFileSourceLoadFailure(ZoneFileSourceException): + + def __init__(self, error): + super(ZoneFileSourceLoadFailure, self).__init__( + error.message) + + +class ZoneFileSource(AxfrBaseSource): + ''' + Bind compatible zone file source + + zonefile: + class: octodns.source.axfr.ZoneFileSource + # The directory holding the zone files + # Filenames should match zone name (eg. example.com.) + directory: ./zonefiles + ''' + def __init__(self, id, directory): + self.log = logging.getLogger('ZoneFileSource[{}]'.format(id)) + self.log.debug('__init__: id=%s, directory=%s', id, directory) + super(ZoneFileSource, self).__init__(id) + self.directory = directory + + self._zone_records = {} + + def _load_zone_file(self, zone_name): + zonefiles = listdir(self.directory) + if zone_name in zonefiles: + try: + z = dns.zone.from_file(join(self.directory, zone_name), + zone_name, relativize=False) + except DNSException as error: + raise ZoneFileSourceLoadFailure(error) + else: + raise ZoneFileSourceNotFound() + + return z + + def zone_records(self, zone): + if zone.name not in self._zone_records: + try: + z = self._load_zone_file(zone.name) + records = [] + for (name, ttl, rdata) in z.iterate_rdatas(): + rdtype = dns.rdatatype.to_text(rdata.rdtype) + records.append({ + "name": name.to_text(), + "ttl": ttl, + "type": rdtype, + "value": rdata.to_text() + }) + + self._zone_records[zone.name] = records + except ZoneFileSourceNotFound: + return [] + + return self._zone_records[zone.name] diff --git a/tests/test_octodns_source_axfr.py b/tests/test_octodns_source_axfr.py index d90eb5b..9251113 100644 --- a/tests/test_octodns_source_axfr.py +++ b/tests/test_octodns_source_axfr.py @@ -11,14 +11,15 @@ from dns.exception import DNSException from mock import patch from unittest import TestCase -from octodns.source.axfr import AxfrSource, AxfrSourceZoneTransferFailed +from octodns.source.axfr import AxfrSource, AxfrSourceZoneTransferFailed, \ + ZoneFileSource, ZoneFileSourceLoadFailure from octodns.zone import Zone class TestAxfrSource(TestCase): source = AxfrSource('test', 'localhost') - forward_zonefile = dns.zone.from_file('./tests/zones/unit.tests.db', + forward_zonefile = dns.zone.from_file('./tests/zones/unit.tests.', 'unit.tests', relativize=False) @patch('dns.zone.from_xfr') @@ -38,3 +39,33 @@ class TestAxfrSource(TestCase): self.source.populate(zone) self.assertEquals('Unable to Perform Zone Transfer', ctx.exception.message) + + +class TestZoneFileSource(TestCase): + source = ZoneFileSource('test', './tests/zones') + + def test_populate(self): + # Valid zone file in directory + valid = Zone('unit.tests.', []) + self.source.populate(valid) + self.assertEquals(11, len(valid.records)) + + # 2nd populate does not read file again + again = Zone('unit.tests.', []) + self.source.populate(again) + self.assertEquals(11, len(again.records)) + + # bust the cache + del self.source._zone_records[valid.name] + + # No zone file in directory + missing = Zone('missing.zone.', []) + self.source.populate(missing) + self.assertEquals(0, len(missing.records)) + + # Zone file is not valid + with self.assertRaises(ZoneFileSourceLoadFailure) as ctx: + zone = Zone('invalid.zone.', []) + self.source.populate(zone) + self.assertEquals('The DNS zone has no NS RRset at its origin.', + ctx.exception.message) diff --git a/tests/zones/invalid.zone. b/tests/zones/invalid.zone. new file mode 100644 index 0000000..c814af6 --- /dev/null +++ b/tests/zones/invalid.zone. @@ -0,0 +1,8 @@ +$ORIGIN invalid.zone. +@ IN SOA ns1.invalid.zone. root.invalid.zone. ( + 2018071501 ; Serial + 3600 ; Refresh (1 hour) + 600 ; Retry (10 minutes) + 604800 ; Expire (1 week) + 3600 ; NXDOMAIN ttl (1 hour) + ) diff --git a/tests/zones/unit.tests.db b/tests/zones/unit.tests. similarity index 100% rename from tests/zones/unit.tests.db rename to tests/zones/unit.tests. From e0c4e60c431352d6dab0c456fc8e65829dad35cc Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 15 Oct 2018 16:29:36 -0700 Subject: [PATCH 02/29] Vastly improved CloudflareProvider _apply_Update, much safer --- octodns/provider/cloudflare.py | 145 +++++++++++++--------- tests/test_octodns_provider_cloudflare.py | 76 ++++++++---- 2 files changed, 138 insertions(+), 83 deletions(-) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index ecf3e3f..d008084 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -323,7 +323,7 @@ class CloudflareProvider(BaseProvider): } } - def _gen_contents(self, record): + def _gen_data(self, record): name = record.fqdn[:-1] _type = record._type ttl = max(self.MIN_TTL, record.ttl) @@ -345,88 +345,111 @@ class CloudflareProvider(BaseProvider): new = change.new zone_id = self.zones[new.zone.name] path = '/zones/{}/dns_records'.format(zone_id) - for content in self._gen_contents(new): + for content in self._gen_data(new): self._request('POST', path, data=content) - def _hash_content(self, content): + def _hash_data(self, data): # Some of the dicts are nested so this seems about as good as any # option we have for consistently hashing them (within a single run) - return hash(dumps(content, sort_keys=True)) + return hash(dumps(data, sort_keys=True)) def _apply_Update(self, change): - - # Ugh, this is pretty complicated and ugly, mainly due to the - # sub-optimal API/semantics. Ideally we'd have a batch change API like - # Route53's to make this 100% clean and safe without all this PITA, but - # we don't so we'll have to work around that and manually do it as - # safely as possible. Note this still isn't perfect as we don't/can't - # practically take into account things like the different "types" of - # CAA records so when we "swap" there may be brief periods where things - # are invalid or even worse Cloudflare may update their validations to - # prevent dups. I see no clean way around that short of making this - # understand 100% of the details of each record type and develop an - # individual/specific ordering of changes that prevents it. That'd - # probably result in more code than this whole provider currently has - # so... :-( - - existing_contents = { - self._hash_content(c): c - for c in self._gen_contents(change.existing) - } - new_contents = { - self._hash_content(c): c - for c in self._gen_contents(change.new) - } - - # Find the things we need to add - adds = [] - for k, content in new_contents.items(): - try: - existing_contents.pop(k) - self.log.debug('_apply_Update: leaving %s', content) - except KeyError: - adds.append(content) + # Note that all CF records have a `content` field the value of which + # appears to be a unique/hashable string for the record's. It includes + # all the "value" bits, but not the secondary stuff like TTL's. E.g. + # for an A it'll include the value, for a CAA it'll include the flags, + # tag, and value, ... We'll take advantage of this to try and match up + # old & new records cleanly. In general when there are multiple records + # for a name & type each will have a distinct/consistent `content` that + # can serve as a unique identifier. zone = change.new.zone zone_id = self.zones[zone.name] - - # Find things we need to remove hostname = zone.hostname_from_fqdn(change.new.fqdn[:-1]) _type = change.new._type - # OK, work through each record from the zone + + existing = {} + # Find all of the existing CF records for this name & type for record in self.zone_records(zone): name = zone.hostname_from_fqdn(record['name']) # Use the _record_for so that we include all of standard # conversion logic r = self._record_for(zone, name, record['type'], [record], True) if hostname == r.name and _type == r._type: - # Round trip the single value through a record to contents flow - # to get a consistent _gen_contents result that matches what + # to get a consistent _gen_data result that matches what # went in to new_contents - content = self._gen_contents(r).next() + data = self._gen_data(r).next() - # If the hash of that dict isn't in new this record isn't - # needed - if self._hash_content(content) not in new_contents: - rid = record['id'] - path = '/zones/{}/dns_records/{}'.format(record['zone_id'], - rid) - try: - add_content = adds.pop(0) - self.log.debug('_apply_Update: swapping %s -> %s, %s', - content, add_content, rid) - self._request('PUT', path, data=add_content) - except IndexError: - self.log.debug('_apply_Update: removing %s, %s', - content, rid) - self._request('DELETE', path) + # Record the record_id and data for this existing record + existing[data['content']] = { + 'record_id': record['id'], + 'data': data, + } - # Any remaining adds just need to be created + # Build up a list of new CF records for this Update + new = { + d['content']: d for d in self._gen_data(change.new) + } + + # OK we now have a picture of the old & new CF records, our next step + # is to figure out which records need to be deleted + deletes = {} + for key, info in existing.items(): + if key not in new: + deletes[key] = info + # Now we need to figure out which records will need to be created + creates = {} + # And which will be updated + updates = {} + for key, data in new.items(): + if key in existing: + # To update we need to combine the new data and existing's + # record_id. old_data is just for debugging/logging purposes + old_info = existing[key] + updates[key] = { + 'record_id': old_info['record_id'], + 'data': data, + 'old_data': old_info['data'], + } + else: + creates[key] = data + + # To do this as safely as possible we'll add new things first, update + # existing things, and then remove old things. This should (try) and + # ensure that we have as many value CF records in their system as + # possible at any given time. Ideally we'd have a "batch" API that + # would allow create, delete, and upsert style stuff so operations + # could be done atomically, but that's not available so we made the + # best of it... + + # The sorts ensure a consistent order of operations, they're not + # otherwise required, just makes things deterministic + + # Creates path = '/zones/{}/dns_records'.format(zone_id) - for content in adds: - self.log.debug('_apply_Update: adding %s', content) - self._request('POST', path, data=content) + for _, data in sorted(creates.items()): + self.log.debug('_apply_Update: creating %s', data) + self._request('POST', path, data=data) + + # Updates + for _, info in sorted(updates.items()): + record_id = info['record_id'] + data = info['data'] + old_data = info['old_data'] + path = '/zones/{}/dns_records/{}'.format(zone_id, record_id) + self.log.debug('_apply_Update: updating %s, %s -> %s', + record_id, data, old_data) + self._request('PUT', path, data=data) + + # Deletes + for _, info in sorted(deletes.items()): + record_id = info['record_id'] + old_data = info['data'] + path = '/zones/{}/dns_records/{}'.format(zone_id, record_id) + self.log.debug('_apply_Update: removing %s, %s', record_id, + old_data) + self._request('DELETE', path) def _apply_Delete(self, change): existing = change.existing diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 7462b9f..71a7e92 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -287,14 +287,16 @@ class TestCloudflareProvider(TestCase): self.assertEquals(2, len(plan.changes)) self.assertEquals(2, provider.apply(plan)) self.assertTrue(plan.exists) - # recreate for update, and deletes for the 2 parts of the other + # creates a the new value and then deletes all the old provider._request.assert_has_calls([ - call('PUT', '/zones/ff12ab34cd5611334422ab3322997650/dns_records/' - 'fc12ab34cd5611334422ab3322997655', - data={'content': '3.2.3.4', - 'type': 'A', - 'name': 'ttl.unit.tests', - 'ttl': 300}), + call('POST', '/zones/42/dns_records', data={ + 'content': '3.2.3.4', + 'type': 'A', + 'name': 'ttl.unit.tests', + 'ttl': 300 + }), + call('DELETE', + '/zones/42/dns_records/fc12ab34cd5611334422ab3322997655'), call('DELETE', '/zones/ff12ab34cd5611334422ab3322997650/' 'dns_records/fc12ab34cd5611334422ab3322997653'), call('DELETE', '/zones/ff12ab34cd5611334422ab3322997650/' @@ -351,6 +353,8 @@ class TestCloudflareProvider(TestCase): }, # zone create None, None, + None, + None, ] # Add something and delete something @@ -371,17 +375,35 @@ class TestCloudflareProvider(TestCase): plan = Plan(zone, zone, [change], True) provider._apply(plan) + # get the list of zones, create a zone, add some records, update + # something, and delete something provider._request.assert_has_calls([ call('GET', '/zones', params={'page': 1}), - call('POST', '/zones', data={'jump_start': False, - 'name': 'unit.tests'}), - call('PUT', '/zones/ff12ab34cd5611334422ab3322997650/dns_records/' - 'fc12ab34cd5611334422ab3322997653', - data={'content': '4.4.4.4', 'type': 'A', 'name': - 'a.unit.tests', 'ttl': 300}), - call('POST', '/zones/42/dns_records', - data={'content': '3.3.3.3', 'type': 'A', - 'name': 'a.unit.tests', 'ttl': 300}) + call('POST', '/zones', data={ + 'jump_start': False, + 'name': 'unit.tests' + }), + call('POST', '/zones/42/dns_records', data={ + 'content': '3.3.3.3', + 'type': 'A', + 'name': 'a.unit.tests', + 'ttl': 300 + }), + call('POST', '/zones/42/dns_records', data={ + 'content': '4.4.4.4', + 'type': 'A', + 'name': 'a.unit.tests', + 'ttl': 300 + }), + call('PUT', '/zones/42/dns_records/' + 'fc12ab34cd5611334422ab3322997654', data={ + 'content': '2.2.2.2', + 'type': 'A', + 'name': 'a.unit.tests', + 'ttl': 300 + }), + call('DELETE', '/zones/42/dns_records/' + 'fc12ab34cd5611334422ab3322997653') ]) def test_update_delete(self): @@ -456,12 +478,22 @@ class TestCloudflareProvider(TestCase): plan = Plan(zone, zone, [change], True) provider._apply(plan) + # Get zones, create zone, create a record, delete a record provider._request.assert_has_calls([ call('GET', '/zones', params={'page': 1}), - call('POST', '/zones', - data={'jump_start': False, 'name': 'unit.tests'}), - call('DELETE', '/zones/ff12ab34cd5611334422ab3322997650/' - 'dns_records/fc12ab34cd5611334422ab3322997653') + call('POST', '/zones', data={ + 'jump_start': False, + 'name': 'unit.tests' + }), + call('PUT', '/zones/42/dns_records/' + 'fc12ab34cd5611334422ab3322997654', data={ + 'content': 'ns2.foo.bar.', + 'type': 'NS', + 'name': 'unit.tests', + 'ttl': 300 + }), + call('DELETE', '/zones/42/dns_records/' + 'fc12ab34cd5611334422ab3322997653') ]) def test_alias(self): @@ -498,9 +530,9 @@ class TestCloudflareProvider(TestCase): self.assertEquals('www.unit.tests.', record.value) # Make sure we transform back to CNAME going the other way - contents = provider._gen_contents(record) + contents = provider._gen_data(record) self.assertEquals({ - 'content': u'www.unit.tests.', + 'content': 'www.unit.tests.', 'name': 'unit.tests', 'ttl': 300, 'type': 'CNAME' From c75200585637afc58a0d8213eb5fcf8bb09c6a48 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 15 Oct 2018 16:34:25 -0700 Subject: [PATCH 03/29] CloudflareProvider._hash_data is no longer used --- octodns/provider/cloudflare.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index d008084..6a42668 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -7,7 +7,6 @@ from __future__ import absolute_import, division, print_function, \ from collections import defaultdict from logging import getLogger -from json import dumps from requests import Session from ..record import Record, Update @@ -348,11 +347,6 @@ class CloudflareProvider(BaseProvider): for content in self._gen_data(new): self._request('POST', path, data=content) - def _hash_data(self, data): - # Some of the dicts are nested so this seems about as good as any - # option we have for consistently hashing them (within a single run) - return hash(dumps(data, sort_keys=True)) - def _apply_Update(self, change): # Note that all CF records have a `content` field the value of which # appears to be a unique/hashable string for the record's. It includes From db8e291d5325fc99ad26f4bed6857514f135de51 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 15 Oct 2018 19:51:26 -0700 Subject: [PATCH 04/29] Implement CloudflareProvider create + delete -> update conversion --- octodns/provider/cloudflare.py | 18 ++++++++++++++ tests/test_octodns_provider_cloudflare.py | 30 +++++++++++------------ 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index 6a42668..b6df5c4 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -417,6 +417,24 @@ class CloudflareProvider(BaseProvider): # could be done atomically, but that's not available so we made the # best of it... + # However, there are record types like CNAME that can only have a + # single value. B/c of that our create and then delete approach isn't + # actually viable. To address this we'll convert as many creates & + # deletes as we can to updates. This will have a minor upside of + # resulting in fewer ops and in the case of things like CNAME where + # there's a single create and delete result in a single update instead. + create_keys = sorted(creates.keys()) + delete_keys = sorted(deletes.keys()) + for i in range(0, min(len(create_keys), len(delete_keys))): + create_key = create_keys[i] + create_data = creates.pop(create_key) + delete_info = deletes.pop(delete_keys[i]) + updates[create_key] = { + 'record_id': delete_info['record_id'], + 'data': create_data, + 'old_data': delete_info['data'], + } + # The sorts ensure a consistent order of operations, they're not # otherwise required, just makes things deterministic diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 71a7e92..f03f87c 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -289,14 +289,13 @@ class TestCloudflareProvider(TestCase): self.assertTrue(plan.exists) # creates a the new value and then deletes all the old provider._request.assert_has_calls([ - call('POST', '/zones/42/dns_records', data={ - 'content': '3.2.3.4', - 'type': 'A', - 'name': 'ttl.unit.tests', - 'ttl': 300 - }), - call('DELETE', - '/zones/42/dns_records/fc12ab34cd5611334422ab3322997655'), + call('PUT', '/zones/42/dns_records/' + 'fc12ab34cd5611334422ab3322997655', data={ + 'content': '3.2.3.4', + 'type': 'A', + 'name': 'ttl.unit.tests', + 'ttl': 300 + }), call('DELETE', '/zones/ff12ab34cd5611334422ab3322997650/' 'dns_records/fc12ab34cd5611334422ab3322997653'), call('DELETE', '/zones/ff12ab34cd5611334422ab3322997650/' @@ -383,12 +382,6 @@ class TestCloudflareProvider(TestCase): 'jump_start': False, 'name': 'unit.tests' }), - call('POST', '/zones/42/dns_records', data={ - 'content': '3.3.3.3', - 'type': 'A', - 'name': 'a.unit.tests', - 'ttl': 300 - }), call('POST', '/zones/42/dns_records', data={ 'content': '4.4.4.4', 'type': 'A', @@ -402,8 +395,13 @@ class TestCloudflareProvider(TestCase): 'name': 'a.unit.tests', 'ttl': 300 }), - call('DELETE', '/zones/42/dns_records/' - 'fc12ab34cd5611334422ab3322997653') + call('PUT', '/zones/42/dns_records/' + 'fc12ab34cd5611334422ab3322997653', data={ + 'content': '3.3.3.3', + 'type': 'A', + 'name': 'a.unit.tests', + 'ttl': 300 + }), ]) def test_update_delete(self): From 0c33d3acac47fb0bf4bc82dbad20f06fef97c6f6 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 15 Oct 2018 20:18:14 -0700 Subject: [PATCH 05/29] Handle the MX special case around content --- octodns/provider/cloudflare.py | 28 +++++++++++++++-------- tests/test_octodns_provider_cloudflare.py | 14 ++++++++++++ 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index b6df5c4..1d7c1b2 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -340,14 +340,7 @@ class CloudflareProvider(BaseProvider): }) yield content - def _apply_Create(self, change): - new = change.new - zone_id = self.zones[new.zone.name] - path = '/zones/{}/dns_records'.format(zone_id) - for content in self._gen_data(new): - self._request('POST', path, data=content) - - def _apply_Update(self, change): + def _gen_key(self, data): # Note that all CF records have a `content` field the value of which # appears to be a unique/hashable string for the record's. It includes # all the "value" bits, but not the secondary stuff like TTL's. E.g. @@ -356,7 +349,21 @@ class CloudflareProvider(BaseProvider): # old & new records cleanly. In general when there are multiple records # for a name & type each will have a distinct/consistent `content` that # can serve as a unique identifier. + # BUT... there's an exception. For some reason MX doesn't include + # priority in its `content` so it's an odd-ball. I haven't found any + # others so hopefully they don't exist :-( + if data['type'] == 'MX': + return '{} {}'.format(data['priority'], data['content']) + return data['content'] + def _apply_Create(self, change): + new = change.new + zone_id = self.zones[new.zone.name] + path = '/zones/{}/dns_records'.format(zone_id) + for content in self._gen_data(new): + self._request('POST', path, data=content) + + def _apply_Update(self, change): zone = change.new.zone zone_id = self.zones[zone.name] hostname = zone.hostname_from_fqdn(change.new.fqdn[:-1]) @@ -376,14 +383,15 @@ class CloudflareProvider(BaseProvider): data = self._gen_data(r).next() # Record the record_id and data for this existing record - existing[data['content']] = { + key = self._gen_key(data) + existing[key] = { 'record_id': record['id'], 'data': data, } # Build up a list of new CF records for this Update new = { - d['content']: d for d in self._gen_data(change.new) + self._gen_key(d): d for d in self._gen_data(change.new) } # OK we now have a picture of the old & new CF records, our next step diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index f03f87c..a04b6d3 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -536,6 +536,20 @@ class TestCloudflareProvider(TestCase): 'type': 'CNAME' }, list(contents)[0]) + def test_gen_key(self): + provider = CloudflareProvider('test', 'email', 'token') + + self.assertEqual('10 foo.bar.com.', provider._gen_key({ + 'content': 'foo.bar.com.', + 'priority': 10, + 'type': 'MX', + })) + + self.assertEqual('foo.bar.com.', provider._gen_key({ + 'content': 'foo.bar.com.', + 'type': 'CNAME', + })) + def test_cdn(self): provider = CloudflareProvider('test', 'email', 'token', True) From aee786dd017970b37e52ba878edac972205fe414 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Tue, 16 Oct 2018 07:08:01 -0700 Subject: [PATCH 06/29] Explicit handling of SRV & CAA in _gen_key, tests for those cases --- octodns/provider/cloudflare.py | 33 +++++++++++-------- tests/test_octodns_provider_cloudflare.py | 39 +++++++++++++++++------ 2 files changed, 49 insertions(+), 23 deletions(-) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index 1d7c1b2..50e4b90 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -341,19 +341,26 @@ class CloudflareProvider(BaseProvider): yield content def _gen_key(self, data): - # Note that all CF records have a `content` field the value of which - # appears to be a unique/hashable string for the record's. It includes - # all the "value" bits, but not the secondary stuff like TTL's. E.g. - # for an A it'll include the value, for a CAA it'll include the flags, - # tag, and value, ... We'll take advantage of this to try and match up - # old & new records cleanly. In general when there are multiple records - # for a name & type each will have a distinct/consistent `content` that - # can serve as a unique identifier. - # BUT... there's an exception. For some reason MX doesn't include - # priority in its `content` so it's an odd-ball. I haven't found any - # others so hopefully they don't exist :-( - if data['type'] == 'MX': - return '{} {}'.format(data['priority'], data['content']) + # Note that most CF record data has a `content` field the value of + # which is a unique/hashable string for the record's. It includes all + # the "value" bits, but not the secondary stuff like TTL's. E.g. for + # an A it'll include the value, for a CAA it'll include the flags, tag, + # and value, ... We'll take advantage of this to try and match up old & + # new records cleanly. In general when there are multiple records for a + # name & type each will have a distinct/consistent `content` that can + # serve as a unique identifier. + # BUT... there are exceptions. MX, CAA, and SRV don't have a simple + # content as things are currently implemented so we need to handle + # those explicitly and create unique/hashable strings for them. + _type = data['type'] + if _type == 'MX': + return '{priority} {content}'.format(**data) + elif _type == 'CAA': + data = data['data'] + return '{flags} {tag} {value}'.format(**data) + elif _type == 'SRV': + data = data['data'] + return '{port} {priority} {target} {weight}'.format(**data) return data['content'] def _apply_Create(self, change): diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index a04b6d3..0f769a4 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -539,16 +539,35 @@ class TestCloudflareProvider(TestCase): def test_gen_key(self): provider = CloudflareProvider('test', 'email', 'token') - self.assertEqual('10 foo.bar.com.', provider._gen_key({ - 'content': 'foo.bar.com.', - 'priority': 10, - 'type': 'MX', - })) - - self.assertEqual('foo.bar.com.', provider._gen_key({ - 'content': 'foo.bar.com.', - 'type': 'CNAME', - })) + for expected, data in ( + ('foo.bar.com.', { + 'content': 'foo.bar.com.', + 'type': 'CNAME', + }), + ('10 foo.bar.com.', { + 'content': 'foo.bar.com.', + 'priority': 10, + 'type': 'MX', + }), + ('0 tag some-value', { + 'data': { + 'flags': 0, + 'tag': 'tag', + 'value': 'some-value', + }, + 'type': 'CAA', + }), + ('42 100 thing-were-pointed.at 101', { + 'data': { + 'port': 42, + 'priority': 100, + 'target': 'thing-were-pointed.at', + 'weight': 101, + }, + 'type': 'SRV', + }), + ): + self.assertEqual(expected, provider._gen_key(data)) def test_cdn(self): provider = CloudflareProvider('test', 'email', 'token', True) From 9ff5942d1934a693b0e8bc4f5d71ef2a48659ed8 Mon Sep 17 00:00:00 2001 From: Matt Date: Sat, 1 Sep 2018 13:31:53 +1000 Subject: [PATCH 07/29] Add: Ability to manage "proxied" flag of "A", "AAAA" and "CNAME" records via YAML configuration (see "CloudflareProvider" class docstring for details). --- octodns/provider/cloudflare.py | 57 ++++++++++++++++++++++++++++++++-- 1 file changed, 55 insertions(+), 2 deletions(-) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index 50e4b90..a418169 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -6,6 +6,7 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals from collections import defaultdict +from copy import deepcopy from logging import getLogger from requests import Session @@ -27,6 +28,9 @@ class CloudflareAuthenticationError(CloudflareError): CloudflareError.__init__(self, data) +_PROXIABLE_RECORD_TYPES = {'A', 'AAAA', 'CNAME'} + + class CloudflareProvider(BaseProvider): ''' Cloudflare DNS provider @@ -221,7 +225,14 @@ class CloudflareProvider(BaseProvider): data_for = getattr(self, '_data_for_{}'.format(_type)) data = data_for(_type, records) - return Record.new(zone, name, data, source=self, lenient=lenient) + record = Record.new(zone, name, data, source=self, lenient=lenient) + + if _type in _PROXIABLE_RECORD_TYPES: + record._octodns['cloudflare'] = { + 'proxied': records[0].get('proxied', False) + } + + return record def populate(self, zone, target=False, lenient=False): self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, @@ -260,8 +271,18 @@ class CloudflareProvider(BaseProvider): def _include_change(self, change): if isinstance(change, Update): - existing = change.existing.data new = change.new.data + + # Cloudflare manages TTL of proxied records, so we should exclude + # TTL from the comparison (to prevent false-positives). + if self._record_is_proxied(change.existing): + existing = deepcopy(change.existing.data) + existing.update({ + 'ttl': new['ttl'] + }) + else: + existing = change.existing.data + new['ttl'] = max(self.MIN_TTL, new['ttl']) if new == existing: return False @@ -322,6 +343,12 @@ class CloudflareProvider(BaseProvider): } } + def _record_is_proxied(self, record): + return ( + not self.cdn and + record._octodns.get('cloudflare', {}).get('proxied', False) + ) + def _gen_data(self, record): name = record.fqdn[:-1] _type = record._type @@ -338,6 +365,12 @@ class CloudflareProvider(BaseProvider): 'type': _type, 'ttl': ttl, }) + + if _type in _PROXIABLE_RECORD_TYPES: + content.update({ + 'proxied': self._record_is_proxied(record) + }) + yield content def _gen_key(self, data): @@ -512,3 +545,23 @@ class CloudflareProvider(BaseProvider): # clear the cache self._zone_records.pop(name, None) + + def _extra_changes(self, existing, desired, changes): + extra_changes = [] + + existing_records = {r: r for r in existing.records} + changed_records = {c.record for c in changes} + + for desired_record in desired.records: + if desired_record not in existing.records: # Will be created + continue + elif desired_record in changed_records: # Already being updated + continue + + existing_record = existing_records[desired_record] + + if (self._record_is_proxied(existing_record) != + self._record_is_proxied(desired_record)): + extra_changes.append(Update(existing_record, desired_record)) + + return extra_changes From 18f29f1c6b791cab2e90d4e05b17c3b2ca164711 Mon Sep 17 00:00:00 2001 From: Matt Date: Sun, 2 Sep 2018 18:02:27 +1000 Subject: [PATCH 08/29] Alter: Existing tests. --- tests/test_octodns_provider_cloudflare.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 0f769a4..d707d62 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -294,6 +294,7 @@ class TestCloudflareProvider(TestCase): 'content': '3.2.3.4', 'type': 'A', 'name': 'ttl.unit.tests', + 'proxied': False, 'ttl': 300 }), call('DELETE', '/zones/ff12ab34cd5611334422ab3322997650/' @@ -386,6 +387,7 @@ class TestCloudflareProvider(TestCase): 'content': '4.4.4.4', 'type': 'A', 'name': 'a.unit.tests', + 'proxied': False, 'ttl': 300 }), call('PUT', '/zones/42/dns_records/' @@ -393,6 +395,7 @@ class TestCloudflareProvider(TestCase): 'content': '2.2.2.2', 'type': 'A', 'name': 'a.unit.tests', + 'proxied': False, 'ttl': 300 }), call('PUT', '/zones/42/dns_records/' @@ -400,6 +403,7 @@ class TestCloudflareProvider(TestCase): 'content': '3.3.3.3', 'type': 'A', 'name': 'a.unit.tests', + 'proxied': False, 'ttl': 300 }), ]) @@ -532,6 +536,7 @@ class TestCloudflareProvider(TestCase): self.assertEquals({ 'content': 'www.unit.tests.', 'name': 'unit.tests', + 'proxied': False, 'ttl': 300, 'type': 'CNAME' }, list(contents)[0]) From 6ceb35c2fcbeab8086056f1535b132e802e6b117 Mon Sep 17 00:00:00 2001 From: Matt Date: Thu, 6 Sep 2018 17:37:55 +1000 Subject: [PATCH 09/29] Add: New tests. --- tests/test_octodns_provider_cloudflare.py | 471 ++++++++++++++++++++++ 1 file changed, 471 insertions(+) diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index d707d62..544d0fb 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -18,6 +18,17 @@ from octodns.provider.yaml import YamlProvider from octodns.zone import Zone +def set_record_proxied_flag(record, proxied): + try: + record._octodns['cloudflare']['proxied'] = proxied + except KeyError: + record._octodns['cloudflare'] = { + 'proxied': proxied + } + + return record + + class TestCloudflareProvider(TestCase): expected = Zone('unit.tests.', []) source = YamlProvider('test', join(dirname(__file__), 'config')) @@ -757,3 +768,463 @@ class TestCloudflareProvider(TestCase): plan = provider.plan(wanted) self.assertEquals(False, hasattr(plan, 'changes')) + + def test_proxied_ignore_ttl(self): + provider = CloudflareProvider('test', 'email', 'token') + + provider.zone_records = Mock(return_value=[ + { + "id": "fc12ab34cd5611334422ab3322997642", + "type": "A", + "name": "unit.tests.a", + "content": "1.2.3.4", + "proxiable": True, + "proxied": True, + "ttl": 1, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + }, + { + "id": "fc12ab34cd5611334422ab3322997643", + "type": "AAAA", + "name": "unit.tests.aaaa", + "content": "::1", + "proxiable": True, + "proxied": True, + "ttl": 1, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + }, + { + "id": "fc12ab34cd5611334422ab3322997644", + "type": "CNAME", + "name": "unit.tests.cname", + "content": "www.unit.tests", + "proxiable": True, + "proxied": True, + "ttl": 1, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + }, + ]) + + zone = Zone('unit.tests.', []) + provider.populate(zone) + + wanted = Zone('unit.tests.', []) + wanted.add_record( + set_record_proxied_flag( + Record.new(wanted, 'unit.tests.a', { + 'ttl': 120, + 'type': 'A', + 'value': '1.2.3.4' + }), True + ) + ) + wanted.add_record( + set_record_proxied_flag( + Record.new(wanted, 'unit.tests.aaaa', { + 'ttl': 250, + 'type': 'AAAA', + 'value': '::1' + }), True + ) + ) + wanted.add_record( + set_record_proxied_flag( + Record.new(wanted, 'unit.tests.cname', { + 'ttl': 300, + 'type': 'CNAME', + 'value': 'www.unit.tests.' + }), True + ) + ) + + plan = provider.plan(wanted) + self.assertEquals(False, hasattr(plan, 'changes')) + + def test_enable_proxied(self): + provider = CloudflareProvider('test', 'email', 'token') + + provider.zone_records = Mock(return_value=[ + { + "id": "fc12ab34cd5611334422ab3322997642", + "type": "A", + "name": "unit.tests.a", + "content": "1.2.3.4", + "proxiable": True, + "proxied": False, + "ttl": 120, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + }, + { + "id": "fc12ab34cd5611334422ab3322997643", + "type": "AAAA", + "name": "unit.tests.aaaa", + "content": "::1", + "proxiable": True, + "proxied": False, + "ttl": 250, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + }, + { + "id": "fc12ab34cd5611334422ab3322997644", + "type": "CNAME", + "name": "unit.tests.cname", + "content": "www.unit.tests", + "proxiable": True, + "proxied": False, + "ttl": 300, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + }, + ]) + + zone = Zone('unit.tests.', []) + provider.populate(zone) + + wanted = Zone('unit.tests.', []) + wanted.add_record( + set_record_proxied_flag( + Record.new(wanted, 'unit.tests.a', { + 'ttl': 120, + 'type': 'A', + 'value': '1.2.3.4' + }), True + ) + ) + wanted.add_record( + set_record_proxied_flag( + Record.new(wanted, 'unit.tests.aaaa', { + 'ttl': 250, + 'type': 'AAAA', + 'value': '::1' + }), True + ) + ) + wanted.add_record( + set_record_proxied_flag( + Record.new(wanted, 'unit.tests.cname', { + 'ttl': 300, + 'type': 'CNAME', + 'value': 'www.unit.tests.' + }), True + ) + ) + + plan = provider.plan(wanted) + self.assertEquals(True, hasattr(plan, 'changes')) + + def test_disable_proxied(self): + provider = CloudflareProvider('test', 'email', 'token') + + provider.zone_records = Mock(return_value=[ + { + "id": "fc12ab34cd5611334422ab3322997642", + "type": "A", + "name": "unit.tests.a", + "content": "1.2.3.4", + "proxiable": True, + "proxied": True, + "ttl": 120, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + }, + { + "id": "fc12ab34cd5611334422ab3322997643", + "type": "AAAA", + "name": "unit.tests.aaaa", + "content": "::1", + "proxiable": True, + "proxied": True, + "ttl": 250, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + }, + { + "id": "fc12ab34cd5611334422ab3322997644", + "type": "CNAME", + "name": "unit.tests.cname", + "content": "www.unit.tests", + "proxiable": True, + "proxied": True, + "ttl": 300, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + }, + ]) + + zone = Zone('unit.tests.', []) + provider.populate(zone) + + wanted = Zone('unit.tests.', []) + wanted.add_record( + set_record_proxied_flag( + Record.new(wanted, 'unit.tests.a', { + 'ttl': 120, + 'type': 'A', + 'value': '1.2.3.4' + }), False + ) + ) + wanted.add_record( + set_record_proxied_flag( + Record.new(wanted, 'unit.tests.aaaa', { + 'ttl': 250, + 'type': 'AAAA', + 'value': '::1' + }), False + ) + ) + wanted.add_record( + set_record_proxied_flag( + Record.new(wanted, 'unit.tests.cname', { + 'ttl': 300, + 'type': 'CNAME', + 'value': 'www.unit.tests.' + }), False + ) + ) + + plan = provider.plan(wanted) + self.assertEquals(True, hasattr(plan, 'changes')) + + def test_leave_proxied_disabled(self): + provider = CloudflareProvider('test', 'email', 'token') + + provider.zone_records = Mock(return_value=[ + { + "id": "fc12ab34cd5611334422ab3322997642", + "type": "A", + "name": "unit.tests.a", + "content": "1.2.3.4", + "proxiable": True, + "proxied": False, + "ttl": 120, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + }, + { + "id": "fc12ab34cd5611334422ab3322997643", + "type": "AAAA", + "name": "unit.tests.aaaa", + "content": "::1", + "proxiable": True, + "proxied": False, + "ttl": 250, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + }, + { + "id": "fc12ab34cd5611334422ab3322997644", + "type": "CNAME", + "name": "unit.tests.cname", + "content": "www.unit.tests", + "proxiable": True, + "proxied": False, + "ttl": 300, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + }, + ]) + + zone = Zone('unit.tests.', []) + provider.populate(zone) + + wanted = Zone('unit.tests.', []) + wanted.add_record( + set_record_proxied_flag( + Record.new(wanted, 'unit.tests.a', { + 'ttl': 120, + 'type': 'A', + 'value': '1.2.3.4' + }), False + ) + ) + wanted.add_record( + set_record_proxied_flag( + Record.new(wanted, 'unit.tests.aaaa', { + 'ttl': 250, + 'type': 'AAAA', + 'value': '::1' + }), False + ) + ) + wanted.add_record( + set_record_proxied_flag( + Record.new(wanted, 'unit.tests.cname', { + 'ttl': 300, + 'type': 'CNAME', + 'value': 'www.unit.tests.' + }), False + ) + ) + + plan = provider.plan(wanted) + self.assertEquals(False, hasattr(plan, 'changes')) + + def test_leave_proxied_enabled(self): + provider = CloudflareProvider('test', 'email', 'token') + + provider.zone_records = Mock(return_value=[ + { + "id": "fc12ab34cd5611334422ab3322997642", + "type": "A", + "name": "unit.tests.a", + "content": "1.2.3.4", + "proxiable": True, + "proxied": True, + "ttl": 120, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + }, + { + "id": "fc12ab34cd5611334422ab3322997643", + "type": "AAAA", + "name": "unit.tests.aaaa", + "content": "::1", + "proxiable": True, + "proxied": True, + "ttl": 250, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + }, + { + "id": "fc12ab34cd5611334422ab3322997644", + "type": "CNAME", + "name": "unit.tests.cname", + "content": "www.unit.tests", + "proxiable": True, + "proxied": True, + "ttl": 300, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + }, + ]) + + zone = Zone('unit.tests.', []) + provider.populate(zone) + + wanted = Zone('unit.tests.', []) + wanted.add_record( + set_record_proxied_flag( + Record.new(wanted, 'unit.tests.a', { + 'ttl': 120, + 'type': 'A', + 'value': '1.2.3.4' + }), True + ) + ) + wanted.add_record( + set_record_proxied_flag( + Record.new(wanted, 'unit.tests.aaaa', { + 'ttl': 250, + 'type': 'AAAA', + 'value': '::1' + }), True + ) + ) + wanted.add_record( + set_record_proxied_flag( + Record.new(wanted, 'unit.tests.cname', { + 'ttl': 300, + 'type': 'CNAME', + 'value': 'www.unit.tests.' + }), True + ) + ) + + plan = provider.plan(wanted) + self.assertEquals(False, hasattr(plan, 'changes')) From a0eaefb3304e86489c33beef4c0b454db2757266 Mon Sep 17 00:00:00 2001 From: Matt Date: Thu, 6 Sep 2018 18:02:31 +1000 Subject: [PATCH 10/29] Add: Documentation on how to utilise the new behaviour. --- octodns/provider/cloudflare.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index a418169..255f68b 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -47,6 +47,16 @@ class CloudflareProvider(BaseProvider): # # See: https://support.cloudflare.com/hc/en-us/articles/115000830351 cdn: false + + Note: The "proxied" flag of "A", "AAAA" and "CNAME" records can be managed + via the YAML provider like so: + name: + octodons: + cloudflare: + proxied: true + ttl: 120 + type: A + value: 1.2.3.4 ''' SUPPORTS_GEO = False SUPPORTS = set(('ALIAS', 'A', 'AAAA', 'CAA', 'CNAME', 'MX', 'NS', 'SRV', From bcff231e3527098ad815f1e6c4d645bc2f7f35a2 Mon Sep 17 00:00:00 2001 From: Matt Date: Thu, 18 Oct 2018 21:25:43 +1100 Subject: [PATCH 11/29] Alter - New tests to be more unit-test-like. --- tests/test_octodns_provider_cloudflare.py | 647 ++++++++++------------ 1 file changed, 285 insertions(+), 362 deletions(-) diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 544d0fb..f186309 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -769,141 +769,16 @@ class TestCloudflareProvider(TestCase): plan = provider.plan(wanted) self.assertEquals(False, hasattr(plan, 'changes')) - def test_proxied_ignore_ttl(self): + def test_unproxiabletype_recordfor_returnsrecordwithnocloudflare(self): provider = CloudflareProvider('test', 'email', 'token') - - provider.zone_records = Mock(return_value=[ + name = "unit.tests" + _type = "NS" + zone_records = [ { - "id": "fc12ab34cd5611334422ab3322997642", - "type": "A", - "name": "unit.tests.a", - "content": "1.2.3.4", - "proxiable": True, - "proxied": True, - "ttl": 1, - "locked": False, - "zone_id": "ff12ab34cd5611334422ab3322997650", - "zone_name": "unit.tests", - "modified_on": "2017-03-11T18:01:43.420689Z", - "created_on": "2017-03-11T18:01:43.420689Z", - "meta": { - "auto_added": False - } - }, - { - "id": "fc12ab34cd5611334422ab3322997643", - "type": "AAAA", - "name": "unit.tests.aaaa", - "content": "::1", - "proxiable": True, - "proxied": True, - "ttl": 1, - "locked": False, - "zone_id": "ff12ab34cd5611334422ab3322997650", - "zone_name": "unit.tests", - "modified_on": "2017-03-11T18:01:43.420689Z", - "created_on": "2017-03-11T18:01:43.420689Z", - "meta": { - "auto_added": False - } - }, - { - "id": "fc12ab34cd5611334422ab3322997644", - "type": "CNAME", - "name": "unit.tests.cname", - "content": "www.unit.tests", - "proxiable": True, - "proxied": True, - "ttl": 1, - "locked": False, - "zone_id": "ff12ab34cd5611334422ab3322997650", - "zone_name": "unit.tests", - "modified_on": "2017-03-11T18:01:43.420689Z", - "created_on": "2017-03-11T18:01:43.420689Z", - "meta": { - "auto_added": False - } - }, - ]) - - zone = Zone('unit.tests.', []) - provider.populate(zone) - - wanted = Zone('unit.tests.', []) - wanted.add_record( - set_record_proxied_flag( - Record.new(wanted, 'unit.tests.a', { - 'ttl': 120, - 'type': 'A', - 'value': '1.2.3.4' - }), True - ) - ) - wanted.add_record( - set_record_proxied_flag( - Record.new(wanted, 'unit.tests.aaaa', { - 'ttl': 250, - 'type': 'AAAA', - 'value': '::1' - }), True - ) - ) - wanted.add_record( - set_record_proxied_flag( - Record.new(wanted, 'unit.tests.cname', { - 'ttl': 300, - 'type': 'CNAME', - 'value': 'www.unit.tests.' - }), True - ) - ) - - plan = provider.plan(wanted) - self.assertEquals(False, hasattr(plan, 'changes')) - - def test_enable_proxied(self): - provider = CloudflareProvider('test', 'email', 'token') - - provider.zone_records = Mock(return_value=[ - { - "id": "fc12ab34cd5611334422ab3322997642", - "type": "A", - "name": "unit.tests.a", - "content": "1.2.3.4", - "proxiable": True, - "proxied": False, - "ttl": 120, - "locked": False, - "zone_id": "ff12ab34cd5611334422ab3322997650", - "zone_name": "unit.tests", - "modified_on": "2017-03-11T18:01:43.420689Z", - "created_on": "2017-03-11T18:01:43.420689Z", - "meta": { - "auto_added": False - } - }, - { - "id": "fc12ab34cd5611334422ab3322997643", - "type": "AAAA", - "name": "unit.tests.aaaa", - "content": "::1", - "proxiable": True, - "proxied": False, - "ttl": 250, - "locked": False, - "zone_id": "ff12ab34cd5611334422ab3322997650", - "zone_name": "unit.tests", - "modified_on": "2017-03-11T18:01:43.420689Z", - "created_on": "2017-03-11T18:01:43.420689Z", - "meta": { - "auto_added": False - } - }, - { - "id": "fc12ab34cd5611334422ab3322997644", - "type": "CNAME", - "name": "unit.tests.cname", - "content": "www.unit.tests", + "id": "fc12ab34cd5611334422ab3322997654", + "type": _type, + "name": name, + "content": "ns2.foo.bar", "proxiable": True, "proxied": False, "ttl": 300, @@ -915,56 +790,29 @@ class TestCloudflareProvider(TestCase): "meta": { "auto_added": False } - }, - ]) - + } + ] + provider.zone_records = Mock(return_value=zone_records) zone = Zone('unit.tests.', []) provider.populate(zone) - wanted = Zone('unit.tests.', []) - wanted.add_record( - set_record_proxied_flag( - Record.new(wanted, 'unit.tests.a', { - 'ttl': 120, - 'type': 'A', - 'value': '1.2.3.4' - }), True - ) - ) - wanted.add_record( - set_record_proxied_flag( - Record.new(wanted, 'unit.tests.aaaa', { - 'ttl': 250, - 'type': 'AAAA', - 'value': '::1' - }), True - ) - ) - wanted.add_record( - set_record_proxied_flag( - Record.new(wanted, 'unit.tests.cname', { - 'ttl': 300, - 'type': 'CNAME', - 'value': 'www.unit.tests.' - }), True - ) - ) + record = provider._record_for(zone, name, _type, zone_records, False) - plan = provider.plan(wanted) - self.assertEquals(True, hasattr(plan, 'changes')) + self.assertFalse('cloudflare' in record._octodns) - def test_disable_proxied(self): + def test_proxiabletype_recordfor_retrecordwithcloudflareunproxied(self): provider = CloudflareProvider('test', 'email', 'token') - - provider.zone_records = Mock(return_value=[ + name = "multi.unit.tests" + _type = "AAAA" + zone_records = [ { "id": "fc12ab34cd5611334422ab3322997642", - "type": "A", - "name": "unit.tests.a", - "content": "1.2.3.4", + "type": _type, + "name": name, + "content": "::1", "proxiable": True, - "proxied": True, - "ttl": 120, + "proxied": False, + "ttl": 300, "locked": False, "zone_id": "ff12ab34cd5611334422ab3322997650", "zone_name": "unit.tests", @@ -973,15 +821,29 @@ class TestCloudflareProvider(TestCase): "meta": { "auto_added": False } - }, + } + ] + provider.zone_records = Mock(return_value=zone_records) + zone = Zone('unit.tests.', []) + provider.populate(zone) + + record = provider._record_for(zone, name, _type, zone_records, False) + + self.assertFalse(record._octodns['cloudflare']['proxied']) + + def test_proxiabletype_recordfor_returnsrecordwithcloudflareproxied(self): + provider = CloudflareProvider('test', 'email', 'token') + name = "multi.unit.tests" + _type = "AAAA" + zone_records = [ { - "id": "fc12ab34cd5611334422ab3322997643", - "type": "AAAA", - "name": "unit.tests.aaaa", + "id": "fc12ab34cd5611334422ab3322997642", + "type": _type, + "name": name, "content": "::1", "proxiable": True, "proxied": True, - "ttl": 250, + "ttl": 300, "locked": False, "zone_id": "ff12ab34cd5611334422ab3322997650", "zone_name": "unit.tests", @@ -990,11 +852,90 @@ class TestCloudflareProvider(TestCase): "meta": { "auto_added": False } - }, + } + ] + provider.zone_records = Mock(return_value=zone_records) + zone = Zone('unit.tests.', []) + provider.populate(zone) + + record = provider._record_for(zone, name, _type, zone_records, False) + + self.assertTrue(record._octodns['cloudflare']['proxied']) + + def test_proxiedrecordandnewttl_includechange_returnsfalse(self): + provider = CloudflareProvider('test', 'email', 'token') + zone = Zone('unit.tests.', []) + existing = set_record_proxied_flag( + Record.new(zone, 'a', { + 'ttl': 1, + 'type': 'A', + 'values': ['1.1.1.1', '2.2.2.2'] + }), True + ) + new = Record.new(zone, 'a', { + 'ttl': 300, + 'type': 'A', + 'values': ['1.1.1.1', '2.2.2.2'] + }) + change = Update(existing, new) + + include_change = provider._include_change(change) + + self.assertFalse(include_change) + + def test_unproxiabletype_gendata_returnsnoproxied(self): + provider = CloudflareProvider('test', 'email', 'token') + zone = Zone('unit.tests.', []) + record = Record.new(zone, 'a', { + 'ttl': 3600, + 'type': 'NS', + 'value': 'ns1.unit.tests.' + }) + + data = provider._gen_data(record).next() + + self.assertFalse('proxied' in data) + + def test_proxiabletype_gendata_returnsunproxied(self): + provider = CloudflareProvider('test', 'email', 'token') + zone = Zone('unit.tests.', []) + record = set_record_proxied_flag( + Record.new(zone, 'a', { + 'ttl': 300, + 'type': 'A', + 'value': '1.2.3.4' + }), False + ) + + data = provider._gen_data(record).next() + + self.assertFalse(data['proxied']) + + def test_proxiabletype_gendata_returnsproxied(self): + provider = CloudflareProvider('test', 'email', 'token') + zone = Zone('unit.tests.', []) + record = set_record_proxied_flag( + Record.new(zone, 'a', { + 'ttl': 300, + 'type': 'A', + 'value': '1.2.3.4' + }), True + ) + + data = provider._gen_data(record).next() + + self.assertTrue(data['proxied']) + + def test_createrecord_extrachanges_returnsemptylist(self): + provider = CloudflareProvider('test', 'email', 'token') + provider.zone_records = Mock(return_value=[]) + existing = Zone('unit.tests.', []) + provider.populate(existing) + provider.zone_records = Mock(return_value=[ { - "id": "fc12ab34cd5611334422ab3322997644", + "id": "fc12ab34cd5611334422ab3322997642", "type": "CNAME", - "name": "unit.tests.cname", + "name": "a.unit.tests", "content": "www.unit.tests", "proxiable": True, "proxied": True, @@ -1007,146 +948,25 @@ class TestCloudflareProvider(TestCase): "meta": { "auto_added": False } - }, + } ]) + desired = Zone('unit.tests.', []) + provider.populate(desired) + changes = existing.changes(desired, provider) - zone = Zone('unit.tests.', []) - provider.populate(zone) + extra_changes = provider._extra_changes(existing, desired, changes) - wanted = Zone('unit.tests.', []) - wanted.add_record( - set_record_proxied_flag( - Record.new(wanted, 'unit.tests.a', { - 'ttl': 120, - 'type': 'A', - 'value': '1.2.3.4' - }), False - ) - ) - wanted.add_record( - set_record_proxied_flag( - Record.new(wanted, 'unit.tests.aaaa', { - 'ttl': 250, - 'type': 'AAAA', - 'value': '::1' - }), False - ) - ) - wanted.add_record( - set_record_proxied_flag( - Record.new(wanted, 'unit.tests.cname', { - 'ttl': 300, - 'type': 'CNAME', - 'value': 'www.unit.tests.' - }), False - ) - ) + self.assertFalse(extra_changes) - plan = provider.plan(wanted) - self.assertEquals(True, hasattr(plan, 'changes')) - - def test_leave_proxied_disabled(self): + def test_updaterecord_extrachanges_returnsemptylist(self): provider = CloudflareProvider('test', 'email', 'token') - provider.zone_records = Mock(return_value=[ { "id": "fc12ab34cd5611334422ab3322997642", - "type": "A", - "name": "unit.tests.a", - "content": "1.2.3.4", - "proxiable": True, - "proxied": False, - "ttl": 120, - "locked": False, - "zone_id": "ff12ab34cd5611334422ab3322997650", - "zone_name": "unit.tests", - "modified_on": "2017-03-11T18:01:43.420689Z", - "created_on": "2017-03-11T18:01:43.420689Z", - "meta": { - "auto_added": False - } - }, - { - "id": "fc12ab34cd5611334422ab3322997643", - "type": "AAAA", - "name": "unit.tests.aaaa", - "content": "::1", - "proxiable": True, - "proxied": False, - "ttl": 250, - "locked": False, - "zone_id": "ff12ab34cd5611334422ab3322997650", - "zone_name": "unit.tests", - "modified_on": "2017-03-11T18:01:43.420689Z", - "created_on": "2017-03-11T18:01:43.420689Z", - "meta": { - "auto_added": False - } - }, - { - "id": "fc12ab34cd5611334422ab3322997644", "type": "CNAME", - "name": "unit.tests.cname", + "name": "a.unit.tests", "content": "www.unit.tests", "proxiable": True, - "proxied": False, - "ttl": 300, - "locked": False, - "zone_id": "ff12ab34cd5611334422ab3322997650", - "zone_name": "unit.tests", - "modified_on": "2017-03-11T18:01:43.420689Z", - "created_on": "2017-03-11T18:01:43.420689Z", - "meta": { - "auto_added": False - } - }, - ]) - - zone = Zone('unit.tests.', []) - provider.populate(zone) - - wanted = Zone('unit.tests.', []) - wanted.add_record( - set_record_proxied_flag( - Record.new(wanted, 'unit.tests.a', { - 'ttl': 120, - 'type': 'A', - 'value': '1.2.3.4' - }), False - ) - ) - wanted.add_record( - set_record_proxied_flag( - Record.new(wanted, 'unit.tests.aaaa', { - 'ttl': 250, - 'type': 'AAAA', - 'value': '::1' - }), False - ) - ) - wanted.add_record( - set_record_proxied_flag( - Record.new(wanted, 'unit.tests.cname', { - 'ttl': 300, - 'type': 'CNAME', - 'value': 'www.unit.tests.' - }), False - ) - ) - - plan = provider.plan(wanted) - self.assertEquals(False, hasattr(plan, 'changes')) - - def test_leave_proxied_enabled(self): - provider = CloudflareProvider('test', 'email', 'token') - - provider.zone_records = Mock(return_value=[ - { - "id": "fc12ab34cd5611334422ab3322997642", - "type": "A", - "name": "unit.tests.a", - "content": "1.2.3.4", - "proxiable": True, "proxied": True, "ttl": 120, "locked": False, @@ -1157,28 +977,15 @@ class TestCloudflareProvider(TestCase): "meta": { "auto_added": False } - }, + } + ]) + existing = Zone('unit.tests.', []) + provider.populate(existing) + provider.zone_records = Mock(return_value=[ { - "id": "fc12ab34cd5611334422ab3322997643", - "type": "AAAA", - "name": "unit.tests.aaaa", - "content": "::1", - "proxiable": True, - "proxied": True, - "ttl": 250, - "locked": False, - "zone_id": "ff12ab34cd5611334422ab3322997650", - "zone_name": "unit.tests", - "modified_on": "2017-03-11T18:01:43.420689Z", - "created_on": "2017-03-11T18:01:43.420689Z", - "meta": { - "auto_added": False - } - }, - { - "id": "fc12ab34cd5611334422ab3322997644", + "id": "fc12ab34cd5611334422ab3322997642", "type": "CNAME", - "name": "unit.tests.cname", + "name": "a.unit.tests", "content": "www.unit.tests", "proxiable": True, "proxied": True, @@ -1191,40 +998,156 @@ class TestCloudflareProvider(TestCase): "meta": { "auto_added": False } - }, + } ]) + desired = Zone('unit.tests.', []) + provider.populate(desired) + changes = existing.changes(desired, provider) - zone = Zone('unit.tests.', []) - provider.populate(zone) + extra_changes = provider._extra_changes(existing, desired, changes) - wanted = Zone('unit.tests.', []) - wanted.add_record( - set_record_proxied_flag( - Record.new(wanted, 'unit.tests.a', { - 'ttl': 120, - 'type': 'A', - 'value': '1.2.3.4' - }), True - ) + self.assertFalse(extra_changes) + + def test_deleterecord_extrachanges_returnsemptylist(self): + provider = CloudflareProvider('test', 'email', 'token') + provider.zone_records = Mock(return_value=[ + { + "id": "fc12ab34cd5611334422ab3322997642", + "type": "CNAME", + "name": "a.unit.tests", + "content": "www.unit.tests", + "proxiable": True, + "proxied": True, + "ttl": 300, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + } + ]) + existing = Zone('unit.tests.', []) + provider.populate(existing) + provider.zone_records = Mock(return_value=[]) + desired = Zone('unit.tests.', []) + provider.populate(desired) + changes = existing.changes(desired, provider) + + extra_changes = provider._extra_changes(existing, desired, changes) + + self.assertFalse(extra_changes) + + def test_proxify_extrachanges_returnsupdatelist(self): + provider = CloudflareProvider('test', 'email', 'token') + provider.zone_records = Mock(return_value=[ + { + "id": "fc12ab34cd5611334422ab3322997642", + "type": "CNAME", + "name": "a.unit.tests", + "content": "www.unit.tests", + "proxiable": True, + "proxied": False, + "ttl": 300, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + } + ]) + existing = Zone('unit.tests.', []) + provider.populate(existing) + provider.zone_records = Mock(return_value=[ + { + "id": "fc12ab34cd5611334422ab3322997642", + "type": "CNAME", + "name": "a.unit.tests", + "content": "www.unit.tests", + "proxiable": True, + "proxied": True, + "ttl": 300, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + } + ]) + desired = Zone('unit.tests.', []) + provider.populate(desired) + changes = existing.changes(desired, provider) + + extra_changes = provider._extra_changes(existing, desired, changes) + + self.assertEquals(1, len(extra_changes)) + self.assertFalse( + extra_changes[0].existing._octodns['cloudflare']['proxied'] ) - wanted.add_record( - set_record_proxied_flag( - Record.new(wanted, 'unit.tests.aaaa', { - 'ttl': 250, - 'type': 'AAAA', - 'value': '::1' - }), True - ) - ) - wanted.add_record( - set_record_proxied_flag( - Record.new(wanted, 'unit.tests.cname', { - 'ttl': 300, - 'type': 'CNAME', - 'value': 'www.unit.tests.' - }), True - ) + self.assertTrue( + extra_changes[0].new._octodns['cloudflare']['proxied'] ) - plan = provider.plan(wanted) - self.assertEquals(False, hasattr(plan, 'changes')) + def test_unproxify_extrachanges_returnsupdatelist(self): + provider = CloudflareProvider('test', 'email', 'token') + provider.zone_records = Mock(return_value=[ + { + "id": "fc12ab34cd5611334422ab3322997642", + "type": "CNAME", + "name": "a.unit.tests", + "content": "www.unit.tests", + "proxiable": True, + "proxied": True, + "ttl": 300, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + } + ]) + existing = Zone('unit.tests.', []) + provider.populate(existing) + provider.zone_records = Mock(return_value=[ + { + "id": "fc12ab34cd5611334422ab3322997642", + "type": "CNAME", + "name": "a.unit.tests", + "content": "www.unit.tests", + "proxiable": True, + "proxied": False, + "ttl": 300, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": { + "auto_added": False + } + } + ]) + desired = Zone('unit.tests.', []) + provider.populate(desired) + changes = existing.changes(desired, provider) + + extra_changes = provider._extra_changes(existing, desired, changes) + + self.assertEquals(1, len(extra_changes)) + self.assertTrue( + extra_changes[0].existing._octodns['cloudflare']['proxied'] + ) + self.assertFalse( + extra_changes[0].new._octodns['cloudflare']['proxied'] + ) From f8a98b8fbcce8504cb5f7460fe1d48964397f45c Mon Sep 17 00:00:00 2001 From: Ashe Connor Date: Tue, 30 Oct 2018 11:12:23 +1100 Subject: [PATCH 12/29] bound natsort to 5.2.x Current latest (5.4.1) FTBFS on Python 2.7. --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index f56b68d..4bf709c 100644 --- a/setup.py +++ b/setup.py @@ -34,7 +34,7 @@ setup( 'futures>=3.2.0', 'incf.countryutils>=1.0', 'ipaddress>=1.0.22', - 'natsort>=5.2.0', + 'natsort>=5.2.0,<5.3', # botocore doesn't like >=2.7.0 for some reason 'python-dateutil>=2.6.0,<2.7.0', 'requests>=2.18.4' From 4df77f2322f535164c5e865e414c89b5cdcd83db Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 29 Oct 2018 19:30:19 -0700 Subject: [PATCH 13/29] Bump requests to 2.20.0 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 1074a82..adc8268 100644 --- a/requirements.txt +++ b/requirements.txt @@ -17,7 +17,7 @@ natsort==5.2.0 nsone==0.9.100 ovh==0.4.8 python-dateutil==2.6.1 -requests==2.18.4 +requests==2.20.0 s3transfer==0.1.13 six==1.11.0 setuptools==38.5.2 From 9904086b317ce5d11cdcc46342e85e46d72e1130 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 29 Oct 2018 19:32:12 -0700 Subject: [PATCH 14/29] In setup.py too --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 4bf709c..a2882bc 100644 --- a/setup.py +++ b/setup.py @@ -37,7 +37,7 @@ setup( 'natsort>=5.2.0,<5.3', # botocore doesn't like >=2.7.0 for some reason 'python-dateutil>=2.6.0,<2.7.0', - 'requests>=2.18.4' + 'requests>=2.20.0' ], license='MIT', long_description=open('README.md').read(), From 680f8454c8b066ffa8ca143278fcbd2a0eedec2c Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 29 Oct 2018 19:38:56 -0700 Subject: [PATCH 15/29] Version bump to 0.9.3 & CHANGELOG.md update --- CHANGELOG.md | 8 ++++++++ octodns/__init__.py | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a35489e..688929a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +## v0.9.2 - 2018-10-29 - Misc. stuff sort of release + +* ZoneFile source added +* Major rework/improvements to the Cloudflare record update process, fixed bugs + and optimized it quite a bit +* Add ability to manage Cloudflare proxy flag +* Bump requests version to 2.20.0 + ## v0.9.2 - 2018-08-20 - More sources * EtcHostsProvider implementation to create static/emergency best effort diff --git a/octodns/__init__.py b/octodns/__init__.py index f5181d7..3f73cf3 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.9.2' +__VERSION__ = '0.9.3' From d074e5ea828f3f8f363566671ee1a7b9b8990434 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 29 Oct 2018 19:42:20 -0700 Subject: [PATCH 16/29] Correct CHANGELOG version number --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 688929a..0a95ecb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -## v0.9.2 - 2018-10-29 - Misc. stuff sort of release +## v0.9.3 - 2018-10-29 - Misc. stuff sort of release * ZoneFile source added * Major rework/improvements to the Cloudflare record update process, fixed bugs From 303227054575da6c54d5956a48cc74488ad60ec5 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 3 Nov 2018 14:51:44 -0700 Subject: [PATCH 17/29] Implement --version in ArgumentParser --- octodns/cmds/args.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/octodns/cmds/args.py b/octodns/cmds/args.py index c84dd92..4867ae3 100644 --- a/octodns/cmds/args.py +++ b/octodns/cmds/args.py @@ -11,6 +11,8 @@ from logging import DEBUG, INFO, WARN, Formatter, StreamHandler, \ from logging.handlers import SysLogHandler from sys import stderr, stdout +from octodns import __VERSION__ + class ArgumentParser(_Base): ''' @@ -23,6 +25,9 @@ class ArgumentParser(_Base): super(ArgumentParser, self).__init__(*args, **kwargs) def parse_args(self, default_log_level=INFO): + version = 'octoDNS {}'.format(__VERSION__) + self.add_argument('--version', action='version', version=version, + help='Print octoDNS version and exit') self.add_argument('--log-stream-stdout', action='store_true', default=False, help='Log to stdout instead of stderr') From 933a56d8f982d408bc394ec761cd019d2a57d34a Mon Sep 17 00:00:00 2001 From: Matt Date: Sun, 4 Nov 2018 10:32:06 +1100 Subject: [PATCH 18/29] Add - (non-empty) Record._octodns dict to YAML file output. --- octodns/provider/yaml.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 287fd3b..7c9a915 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -89,6 +89,8 @@ class YamlProvider(BaseProvider): if record.ttl == self.default_ttl: # ttl is the default, we don't need to store it del d['ttl'] + if record._octodns: + d['octodns'] = record._octodns data[record.name].append(d) # Flatten single element lists From 84c883c67f5668ae3e5138bd4447b9f17063584d Mon Sep 17 00:00:00 2001 From: Matt Date: Sun, 4 Nov 2018 13:16:26 +1100 Subject: [PATCH 19/29] Add - Test case. --- tests/test_octodns_provider_yaml.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 46363ed..8f1b4d3 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -60,6 +60,12 @@ class TestYamlProvider(TestCase): # There should be no changes after the round trip reloaded = Zone('unit.tests.', []) target.populate(reloaded) + self.assertDictEqual( + {'included': ['test']}, + filter( + lambda x: x.name == 'included', reloaded.records + )[0]._octodns) + self.assertFalse(zone.changes(reloaded, target=source)) # A 2nd sync should still create everything From 6a7a54be067c9e8ac36e0c9c39dc49a6fd60c23c Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 5 Nov 2018 09:48:03 -0800 Subject: [PATCH 20/29] Include TTL in Dyn CCA record values --- octodns/provider/dyn.py | 1 + 1 file changed, 1 insertion(+) diff --git a/octodns/provider/dyn.py b/octodns/provider/dyn.py index 166227b..0f487c1 100644 --- a/octodns/provider/dyn.py +++ b/octodns/provider/dyn.py @@ -529,6 +529,7 @@ class DynProvider(BaseProvider): return [{ 'flags': v.flags, 'tag': v.tag, + 'ttl': record.ttl, 'value': v.value, } for v in record.values] From c26c8b7c3fe3a8fae676a41faa37bbc7d90a0920 Mon Sep 17 00:00:00 2001 From: Bart S Date: Mon, 12 Nov 2018 16:08:05 +0100 Subject: [PATCH 21/29] Added ALIAS to the support list of proxiable record types --- octodns/provider/cloudflare.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index 255f68b..e4e0a34 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -28,7 +28,7 @@ class CloudflareAuthenticationError(CloudflareError): CloudflareError.__init__(self, data) -_PROXIABLE_RECORD_TYPES = {'A', 'AAAA', 'CNAME'} +_PROXIABLE_RECORD_TYPES = {'A', 'AAAA', 'ALIAS', 'CNAME'} class CloudflareProvider(BaseProvider): @@ -51,7 +51,7 @@ class CloudflareProvider(BaseProvider): Note: The "proxied" flag of "A", "AAAA" and "CNAME" records can be managed via the YAML provider like so: name: - octodons: + octodns: cloudflare: proxied: true ttl: 120 From dfdf81cda40e409fc4d6b604f547630f4cc00b30 Mon Sep 17 00:00:00 2001 From: Bart S Date: Mon, 12 Nov 2018 16:14:31 +0100 Subject: [PATCH 22/29] Added option to set the AWS session token to the Route53Provider --- octodns/provider/route53.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 50c734c..5f0f0d2 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -221,9 +221,12 @@ class Route53Provider(BaseProvider): access_key_id: # The AWS secret access key secret_access_key: + # The AWS session token + session_token: - Alternatively, you may leave out access_key_id and secret_access_key, - this will result in boto3 deciding authentication dynamically. + Alternatively, you may leave out access_key_id, secret_access_key + and session_token. + This will result in boto3 deciding authentication dynamically. In general the account used will need full permissions on Route53. ''' @@ -236,10 +239,11 @@ class Route53Provider(BaseProvider): HEALTH_CHECK_VERSION = '0001' def __init__(self, id, access_key_id=None, secret_access_key=None, - max_changes=1000, client_max_attempts=None, *args, **kwargs): + session_token=None, max_changes=1000, + client_max_attempts=None, *args, **kwargs): self.max_changes = max_changes - _msg = 'access_key_id={}, secret_access_key=***'.format(access_key_id) - if access_key_id is None and secret_access_key is None: + _msg = 'access_key_id={}, secret_access_key=***, session_token=***'.format(access_key_id) + if access_key_id is None and secret_access_key is None and session_token is None: _msg = 'auth=fallback' self.log = logging.getLogger('Route53Provider[{}]'.format(id)) self.log.debug('__init__: id=%s, %s', id, _msg) @@ -251,11 +255,12 @@ class Route53Provider(BaseProvider): client_max_attempts) config = Config(retries={'max_attempts': client_max_attempts}) - if access_key_id is None and secret_access_key is None: + if access_key_id is None and secret_access_key is None and session_token is None: self._conn = client('route53', config=config) else: self._conn = client('route53', aws_access_key_id=access_key_id, aws_secret_access_key=secret_access_key, + aws_session_token=session_token, config=config) self._r53_zones = None From c19fce46c08279c29cf5c23fba5c9e49bafc9dbc Mon Sep 17 00:00:00 2001 From: Bart S Date: Mon, 12 Nov 2018 16:28:51 +0100 Subject: [PATCH 23/29] Attempted lint fixes --- octodns/provider/route53.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 5f0f0d2..cfca6c0 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -225,7 +225,7 @@ class Route53Provider(BaseProvider): session_token: Alternatively, you may leave out access_key_id, secret_access_key - and session_token. + and session_token. This will result in boto3 deciding authentication dynamically. In general the account used will need full permissions on Route53. @@ -239,11 +239,15 @@ class Route53Provider(BaseProvider): HEALTH_CHECK_VERSION = '0001' def __init__(self, id, access_key_id=None, secret_access_key=None, - session_token=None, max_changes=1000, + session_token=None, max_changes=1000, client_max_attempts=None, *args, **kwargs): self.max_changes = max_changes - _msg = 'access_key_id={}, secret_access_key=***, session_token=***'.format(access_key_id) - if access_key_id is None and secret_access_key is None and session_token is None: + _msg = 'access_key_id={}, secret_access_key=***, ' \ + 'session_token=***'.format(access_key_id) + use_fallback_auth = access_key_id is None and \ + secret_access_key is None and \ + session_token is None + if use_fallback_auth: _msg = 'auth=fallback' self.log = logging.getLogger('Route53Provider[{}]'.format(id)) self.log.debug('__init__: id=%s, %s', id, _msg) @@ -255,7 +259,7 @@ class Route53Provider(BaseProvider): client_max_attempts) config = Config(retries={'max_attempts': client_max_attempts}) - if access_key_id is None and secret_access_key is None and session_token is None: + if use_fallback_auth: self._conn = client('route53', config=config) else: self._conn = client('route53', aws_access_key_id=access_key_id, From 02d120dda6d44d8d5a40acd4e9f3ed90af935b41 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 12 Nov 2018 08:40:44 -0800 Subject: [PATCH 24/29] Fixes for DNSimple's TXT ; handling --- octodns/provider/dnsimple.py | 20 ++++++++++++++++++-- tests/fixtures/dnsimple-page-2.json | 2 +- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/octodns/provider/dnsimple.py b/octodns/provider/dnsimple.py index 7a9db50..18dae68 100644 --- a/octodns/provider/dnsimple.py +++ b/octodns/provider/dnsimple.py @@ -112,7 +112,14 @@ class DnsimpleProvider(BaseProvider): _data_for_A = _data_for_multiple _data_for_AAAA = _data_for_multiple _data_for_SPF = _data_for_multiple - _data_for_TXT = _data_for_multiple + + def _data_for_TXT(self, _type, records): + return { + 'ttl': records[0]['ttl'], + 'type': _type, + # escape semicolons + 'values': [r['content'].replace(';', '\\;') for r in records] + } def _data_for_CAA(self, _type, records): values = [] @@ -290,7 +297,16 @@ class DnsimpleProvider(BaseProvider): _params_for_AAAA = _params_for_multiple _params_for_NS = _params_for_multiple _params_for_SPF = _params_for_multiple - _params_for_TXT = _params_for_multiple + + def _params_for_TXT(self, record): + for value in record.values: + yield { + # un-escape semicolons + 'content': value.replace('\\', ''), + 'name': record.name, + 'ttl': record.ttl, + 'type': record._type, + } def _params_for_CAA(self, record): for value in record.values: diff --git a/tests/fixtures/dnsimple-page-2.json b/tests/fixtures/dnsimple-page-2.json index a42c393..c12c4f4 100644 --- a/tests/fixtures/dnsimple-page-2.json +++ b/tests/fixtures/dnsimple-page-2.json @@ -133,7 +133,7 @@ "zone_id": "unit.tests", "parent_id": null, "name": "txt", - "content": "v=DKIM1\\;k=rsa\\;s=email\\;h=sha256\\;p=A/kinda+of/long/string+with+numb3rs", + "content": "v=DKIM1;k=rsa;s=email;h=sha256;p=A/kinda+of/long/string+with+numb3rs", "ttl": 600, "priority": null, "type": "TXT", From 7628f819b83b7c1f70731f41718e411163b7be98 Mon Sep 17 00:00:00 2001 From: Bart S Date: Thu, 15 Nov 2018 11:20:42 +0100 Subject: [PATCH 25/29] Added note saying session_token is optional --- octodns/provider/route53.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index cfca6c0..ab0edc3 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -221,7 +221,8 @@ class Route53Provider(BaseProvider): access_key_id: # The AWS secret access key secret_access_key: - # The AWS session token + # The AWS session token (optional) + # Only needed if using temporary security credentials session_token: Alternatively, you may leave out access_key_id, secret_access_key From 7d8f04a7468a254ca64d7ddf0614e448d39346b8 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 15 Nov 2018 09:03:36 -0800 Subject: [PATCH 26/29] Linting fix --- octodns/provider/route53.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index ab0edc3..6872608 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -246,8 +246,7 @@ class Route53Provider(BaseProvider): _msg = 'access_key_id={}, secret_access_key=***, ' \ 'session_token=***'.format(access_key_id) use_fallback_auth = access_key_id is None and \ - secret_access_key is None and \ - session_token is None + secret_access_key is None and session_token is None if use_fallback_auth: _msg = 'auth=fallback' self.log = logging.getLogger('Route53Provider[{}]'.format(id)) From 7a1875e7d3a01698776562d89a442f13fc8a1883 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 18 Nov 2018 17:44:09 -0800 Subject: [PATCH 27/29] Update natsort versions to 5.5.0 which has setuptools fixes --- requirements.txt | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements.txt b/requirements.txt index adc8268..b4d155d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -13,7 +13,7 @@ incf.countryutils==1.0 ipaddress==1.0.22 jmespath==0.9.3 msrestazure==0.4.27 -natsort==5.2.0 +natsort==5.5.0 nsone==0.9.100 ovh==0.4.8 python-dateutil==2.6.1 diff --git a/setup.py b/setup.py index a2882bc..db35997 100644 --- a/setup.py +++ b/setup.py @@ -34,7 +34,7 @@ setup( 'futures>=3.2.0', 'incf.countryutils>=1.0', 'ipaddress>=1.0.22', - 'natsort>=5.2.0,<5.3', + 'natsort>=5.5.0', # botocore doesn't like >=2.7.0 for some reason 'python-dateutil>=2.6.0,<2.7.0', 'requests>=2.20.0' From 64a453632f6f12c65821a512afd062ee07e16524 Mon Sep 17 00:00:00 2001 From: Bart S Date: Wed, 21 Nov 2018 10:09:23 +0100 Subject: [PATCH 28/29] Moved session_token to the end of the argument list --- octodns/provider/route53.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 6872608..79bc341 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -240,8 +240,8 @@ class Route53Provider(BaseProvider): HEALTH_CHECK_VERSION = '0001' def __init__(self, id, access_key_id=None, secret_access_key=None, - session_token=None, max_changes=1000, - client_max_attempts=None, *args, **kwargs): + max_changes=1000, client_max_attempts=None, + session_token=None, *args, **kwargs): self.max_changes = max_changes _msg = 'access_key_id={}, secret_access_key=***, ' \ 'session_token=***'.format(access_key_id) From 95ae90b587ba6e3939ca80fbb7dbde7a27adf56c Mon Sep 17 00:00:00 2001 From: Bart S Date: Wed, 21 Nov 2018 10:11:45 +0100 Subject: [PATCH 29/29] Removed trailing whitespace --- octodns/provider/route53.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 79bc341..4b7fe66 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -240,7 +240,7 @@ class Route53Provider(BaseProvider): HEALTH_CHECK_VERSION = '0001' def __init__(self, id, access_key_id=None, secret_access_key=None, - max_changes=1000, client_max_attempts=None, + max_changes=1000, client_max_attempts=None, session_token=None, *args, **kwargs): self.max_changes = max_changes _msg = 'access_key_id={}, secret_access_key=***, ' \