From 11f4359099407b95ea5cfe593665a3b4d87c82ad Mon Sep 17 00:00:00 2001 From: Paul van Brouwershaven Date: Thu, 28 Sep 2017 14:54:53 +0200 Subject: [PATCH 01/24] Add support for included and excluded records `Included` and `Excluded` can be used to filter records for one or more specific provider(s). This can be extremely useful when certain record types are not supported by a provider and you want only that provider to receive an alternative record. See also: https://github.com/github/octodns/issues/26 --- octodns/record.py | 2 ++ octodns/zone.py | 31 ++++++++++++++++++++++++ tests/config/unit.tests.yaml | 12 ++++++++++ tests/fixtures/dnsimple-page-2.json | 32 +++++++++++++++++++++++++ tests/fixtures/powerdns-full-data.json | 12 ++++++++++ tests/test_octodns_manager.py | 14 +++++------ tests/test_octodns_provider_dnsimple.py | 6 ++--- tests/test_octodns_provider_powerdns.py | 8 +++---- tests/test_octodns_provider_yaml.py | 8 +++---- 9 files changed, 107 insertions(+), 18 deletions(-) diff --git a/octodns/record.py b/octodns/record.py index 8ef80be..8a05b9e 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -124,6 +124,8 @@ class Record(object): octodns = data.get('octodns', {}) self.ignored = octodns.get('ignored', False) + self.excluded = octodns.get('excluded', []) + self.included = octodns.get('included', []) def _data(self): return {'ttl': self.ttl} diff --git a/octodns/zone.py b/octodns/zone.py index 74e5d9e..f715e3f 100644 --- a/octodns/zone.py +++ b/octodns/zone.py @@ -110,10 +110,29 @@ class Zone(object): for record in filter(_is_eligible, self.records): if record.ignored: continue + elif len(record.included) > 0 and \ + target.__class__.__name__ not in record.included: + self.log.debug('changes: skipping record=%s %s - %s not' + ' included ', record.fqdn, record._type, + target.id) + continue + elif target.__class__.__name__ in record.excluded: + self.log.debug('changes: skipping record=%s %s - %s ' + 'excluded ', record.fqdn, record._type, + target.id) + continue try: desired_record = desired_records[record] if desired_record.ignored: continue + elif len(record.included) > 0 and \ + target.__class__.__name__ not in record.included: + self.log.debug('changes: skipping record=%s %s - %s' + 'not included ', record.fqdn, record._type, + target.id) + continue + elif target.__class__.__name__ in record.excluded: + continue except KeyError: if not target.supports(record): self.log.debug('changes: skipping record=%s %s - %s does ' @@ -141,6 +160,18 @@ class Zone(object): for record in filter(_is_eligible, desired.records - self.records): if record.ignored: continue + elif len(record.included) > 0 and \ + target.__class__.__name__ not in record.included: + self.log.debug('changes: skipping record=%s %s - %s not' + ' included ', record.fqdn, record._type, + target.id) + continue + elif target.__class__.__name__ in record.excluded: + self.log.debug('changes: skipping record=%s %s - %s ' + 'excluded ', record.fqdn, record._type, + target.id) + continue + if not target.supports(record): self.log.debug('changes: skipping record=%s %s - %s does not ' 'support it', record.fqdn, record._type, diff --git a/tests/config/unit.tests.yaml b/tests/config/unit.tests.yaml index 5241406..548dc8f 100644 --- a/tests/config/unit.tests.yaml +++ b/tests/config/unit.tests.yaml @@ -56,11 +56,23 @@ cname: ttl: 300 type: CNAME value: unit.tests. +excluded: + octodns: + excluded: + - CloudflareProvider + type: CNAME + value: excluded.unit.tests. ignored: octodns: ignored: true type: A value: 9.9.9.9 +included: + octodns: + included: + - DnsimpleProvider + type: CNAME + value: included.unit.tests. mx: ttl: 300 type: MX diff --git a/tests/fixtures/dnsimple-page-2.json b/tests/fixtures/dnsimple-page-2.json index 40aaa48..d66c8da 100644 --- a/tests/fixtures/dnsimple-page-2.json +++ b/tests/fixtures/dnsimple-page-2.json @@ -175,6 +175,38 @@ "system_record": false, "created_at": "2017-03-09T15:55:09Z", "updated_at": "2017-03-09T15:55:09Z" + }, + { + "id": 12188804, + "zone_id": "unit.tests", + "parent_id": null, + "name": "excluded", + "content": "excluded.unit.tests", + "ttl": 3600, + "priority": null, + "type": "CNAME", + "regions": [ + "global" + ], + "system_record": false, + "created_at": "2017-03-09T15:55:09Z", + "updated_at": "2017-03-09T15:55:09Z" + }, + { + "id": 12188805, + "zone_id": "unit.tests", + "parent_id": null, + "name": "included", + "content": "included.unit.tests", + "ttl": 3600, + "priority": null, + "type": "CNAME", + "regions": [ + "global" + ], + "system_record": false, + "created_at": "2017-03-09T15:55:09Z", + "updated_at": "2017-03-09T15:55:09Z" } ], "pagination": { diff --git a/tests/fixtures/powerdns-full-data.json b/tests/fixtures/powerdns-full-data.json index b8f8bf3..2d7117a 100644 --- a/tests/fixtures/powerdns-full-data.json +++ b/tests/fixtures/powerdns-full-data.json @@ -242,6 +242,18 @@ ], "ttl": 3600, "type": "CAA" + }, + { + "comments": [], + "name": "excluded.unit.tests.", + "records": [ + { + "content": "excluded.unit.tests.", + "disabled": false + } + ], + "ttl": 3600, + "type": "CNAME" } ], "serial": 2017012803, diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 45a3b55..2b947b5 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -101,12 +101,12 @@ class TestManager(TestCase): environ['YAML_TMP_DIR'] = tmpdir.dirname tc = Manager(get_config_filename('simple.yaml')) \ .sync(dry_run=False) - self.assertEquals(19, tc) + self.assertEquals(20, tc) # try with just one of the zones tc = Manager(get_config_filename('simple.yaml')) \ .sync(dry_run=False, eligible_zones=['unit.tests.']) - self.assertEquals(13, tc) + self.assertEquals(14, tc) # the subzone, with 2 targets tc = Manager(get_config_filename('simple.yaml')) \ @@ -121,18 +121,18 @@ class TestManager(TestCase): # Again with force tc = Manager(get_config_filename('simple.yaml')) \ .sync(dry_run=False, force=True) - self.assertEquals(19, tc) + self.assertEquals(20, tc) # Again with max_workers = 1 tc = Manager(get_config_filename('simple.yaml'), max_workers=1) \ .sync(dry_run=False, force=True) - self.assertEquals(19, tc) + self.assertEquals(20, tc) # Include meta tc = Manager(get_config_filename('simple.yaml'), max_workers=1, include_meta=True) \ .sync(dry_run=False, force=True) - self.assertEquals(23, tc) + self.assertEquals(24, tc) def test_eligible_targets(self): with TemporaryDirectory() as tmpdir: @@ -158,13 +158,13 @@ class TestManager(TestCase): fh.write('---\n{}') changes = manager.compare(['in'], ['dump'], 'unit.tests.') - self.assertEquals(13, len(changes)) + self.assertEquals(14, len(changes)) # Compound sources with varying support changes = manager.compare(['in', 'nosshfp'], ['dump'], 'unit.tests.') - self.assertEquals(12, len(changes)) + self.assertEquals(13, len(changes)) with self.assertRaises(Exception) as ctx: manager.compare(['nope'], ['dump'], 'unit.tests.') diff --git a/tests/test_octodns_provider_dnsimple.py b/tests/test_octodns_provider_dnsimple.py index 950d460..befb39e 100644 --- a/tests/test_octodns_provider_dnsimple.py +++ b/tests/test_octodns_provider_dnsimple.py @@ -78,14 +78,14 @@ class TestDnsimpleProvider(TestCase): zone = Zone('unit.tests.', []) provider.populate(zone) - self.assertEquals(15, len(zone.records)) + self.assertEquals(17, len(zone.records)) changes = self.expected.changes(zone, provider) self.assertEquals(0, len(changes)) # 2nd populate makes no network calls/all from cache again = Zone('unit.tests.', []) provider.populate(again) - self.assertEquals(15, len(again.records)) + self.assertEquals(17, len(again.records)) # bust the cache del provider._zone_records[zone.name] @@ -147,7 +147,7 @@ class TestDnsimpleProvider(TestCase): }), ]) # expected number of total calls - self.assertEquals(27, provider._client._request.call_count) + self.assertEquals(29, provider._client._request.call_count) provider._client._request.reset_mock() diff --git a/tests/test_octodns_provider_powerdns.py b/tests/test_octodns_provider_powerdns.py index b6e02ff..22ccdd6 100644 --- a/tests/test_octodns_provider_powerdns.py +++ b/tests/test_octodns_provider_powerdns.py @@ -78,8 +78,8 @@ class TestPowerDnsProvider(TestCase): expected = Zone('unit.tests.', []) source = YamlProvider('test', join(dirname(__file__), 'config')) source.populate(expected) - expected_n = len(expected.records) - 1 - self.assertEquals(15, expected_n) + expected_n = len(expected.records) - 2 + self.assertEquals(16, expected_n) # No diffs == no changes with requests_mock() as mock: @@ -87,7 +87,7 @@ class TestPowerDnsProvider(TestCase): zone = Zone('unit.tests.', []) provider.populate(zone) - self.assertEquals(15, len(zone.records)) + self.assertEquals(16, len(zone.records)) changes = expected.changes(zone, provider) self.assertEquals(0, len(changes)) @@ -167,7 +167,7 @@ class TestPowerDnsProvider(TestCase): expected = Zone('unit.tests.', []) source = YamlProvider('test', join(dirname(__file__), 'config')) source.populate(expected) - self.assertEquals(16, len(expected.records)) + self.assertEquals(18, len(expected.records)) # A small change to a single record with requests_mock() as mock: diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 36cd8d6..a199355 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -30,7 +30,7 @@ class TestYamlProvider(TestCase): # without it we see everything source.populate(zone) - self.assertEquals(16, len(zone.records)) + self.assertEquals(18, len(zone.records)) # Assumption here is that a clean round-trip means that everything # worked as expected, data that went in came back out and could be @@ -49,12 +49,12 @@ class TestYamlProvider(TestCase): # We add everything plan = target.plan(zone) - self.assertEquals(13, len(filter(lambda c: isinstance(c, Create), + self.assertEquals(14, len(filter(lambda c: isinstance(c, Create), plan.changes))) self.assertFalse(isfile(yaml_file)) # Now actually do it - self.assertEquals(13, target.apply(plan)) + self.assertEquals(14, target.apply(plan)) self.assertTrue(isfile(yaml_file)) # There should be no changes after the round trip @@ -64,7 +64,7 @@ class TestYamlProvider(TestCase): # A 2nd sync should still create everything plan = target.plan(zone) - self.assertEquals(13, len(filter(lambda c: isinstance(c, Create), + self.assertEquals(14, len(filter(lambda c: isinstance(c, Create), plan.changes))) with open(yaml_file) as fh: From 4b41762642e5d7f1af02280f8c45af59b372b1ed Mon Sep 17 00:00:00 2001 From: Paul van Brouwershaven Date: Fri, 29 Sep 2017 10:09:16 +0200 Subject: [PATCH 02/24] Use target.id instead of class name --- octodns/zone.py | 12 +++++------ tests/config/unit.tests.yaml | 8 +++---- .../cloudflare-dns_records-page-2.json | 21 +++++++++++++++++-- tests/fixtures/dnsimple-page-2.json | 20 ++---------------- tests/fixtures/powerdns-full-data.json | 4 ++-- tests/helpers.py | 2 ++ tests/test_octodns_provider_base.py | 1 + tests/test_octodns_provider_cloudflare.py | 12 +++++------ tests/test_octodns_provider_dnsimple.py | 10 ++++----- 9 files changed, 47 insertions(+), 43 deletions(-) diff --git a/octodns/zone.py b/octodns/zone.py index f715e3f..a25333f 100644 --- a/octodns/zone.py +++ b/octodns/zone.py @@ -111,12 +111,12 @@ class Zone(object): if record.ignored: continue elif len(record.included) > 0 and \ - target.__class__.__name__ not in record.included: + target.id not in record.included: self.log.debug('changes: skipping record=%s %s - %s not' ' included ', record.fqdn, record._type, target.id) continue - elif target.__class__.__name__ in record.excluded: + elif target.id in record.excluded: self.log.debug('changes: skipping record=%s %s - %s ' 'excluded ', record.fqdn, record._type, target.id) @@ -126,12 +126,12 @@ class Zone(object): if desired_record.ignored: continue elif len(record.included) > 0 and \ - target.__class__.__name__ not in record.included: + target.id not in record.included: self.log.debug('changes: skipping record=%s %s - %s' 'not included ', record.fqdn, record._type, target.id) continue - elif target.__class__.__name__ in record.excluded: + elif target.id in record.excluded: continue except KeyError: if not target.supports(record): @@ -161,12 +161,12 @@ class Zone(object): if record.ignored: continue elif len(record.included) > 0 and \ - target.__class__.__name__ not in record.included: + target.id not in record.included: self.log.debug('changes: skipping record=%s %s - %s not' ' included ', record.fqdn, record._type, target.id) continue - elif target.__class__.__name__ in record.excluded: + elif target.id in record.excluded: self.log.debug('changes: skipping record=%s %s - %s ' 'excluded ', record.fqdn, record._type, target.id) diff --git a/tests/config/unit.tests.yaml b/tests/config/unit.tests.yaml index 548dc8f..1da2465 100644 --- a/tests/config/unit.tests.yaml +++ b/tests/config/unit.tests.yaml @@ -59,9 +59,9 @@ cname: excluded: octodns: excluded: - - CloudflareProvider + - test type: CNAME - value: excluded.unit.tests. + value: unit.tests. ignored: octodns: ignored: true @@ -70,9 +70,9 @@ ignored: included: octodns: included: - - DnsimpleProvider + - test type: CNAME - value: included.unit.tests. + value: unit.tests. mx: ttl: 300 type: MX diff --git a/tests/fixtures/cloudflare-dns_records-page-2.json b/tests/fixtures/cloudflare-dns_records-page-2.json index 195d6de..150951b 100644 --- a/tests/fixtures/cloudflare-dns_records-page-2.json +++ b/tests/fixtures/cloudflare-dns_records-page-2.json @@ -139,14 +139,31 @@ "meta": { "auto_added": false } + }, + { + "id": "fc12ab34cd5611334422ab3322997656", + "type": "CNAME", + "name": "included.unit.tests", + "content": "unit.tests", + "proxiable": true, + "proxied": false, + "ttl": 3600, + "locked": false, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.940682Z", + "created_on": "2017-03-11T18:01:43.940682Z", + "meta": { + "auto_added": false + } } ], "result_info": { "page": 2, "per_page": 10, "total_pages": 2, - "count": 8, - "total_count": 19 + "count": 9, + "total_count": 20 }, "success": true, "errors": [], diff --git a/tests/fixtures/dnsimple-page-2.json b/tests/fixtures/dnsimple-page-2.json index d66c8da..a42c393 100644 --- a/tests/fixtures/dnsimple-page-2.json +++ b/tests/fixtures/dnsimple-page-2.json @@ -176,28 +176,12 @@ "created_at": "2017-03-09T15:55:09Z", "updated_at": "2017-03-09T15:55:09Z" }, - { - "id": 12188804, - "zone_id": "unit.tests", - "parent_id": null, - "name": "excluded", - "content": "excluded.unit.tests", - "ttl": 3600, - "priority": null, - "type": "CNAME", - "regions": [ - "global" - ], - "system_record": false, - "created_at": "2017-03-09T15:55:09Z", - "updated_at": "2017-03-09T15:55:09Z" - }, { "id": 12188805, "zone_id": "unit.tests", "parent_id": null, "name": "included", - "content": "included.unit.tests", + "content": "unit.tests", "ttl": 3600, "priority": null, "type": "CNAME", @@ -212,7 +196,7 @@ "pagination": { "current_page": 2, "per_page": 20, - "total_entries": 30, + "total_entries": 32, "total_pages": 2 } } diff --git a/tests/fixtures/powerdns-full-data.json b/tests/fixtures/powerdns-full-data.json index 2d7117a..3d445d4 100644 --- a/tests/fixtures/powerdns-full-data.json +++ b/tests/fixtures/powerdns-full-data.json @@ -245,10 +245,10 @@ }, { "comments": [], - "name": "excluded.unit.tests.", + "name": "included.unit.tests.", "records": [ { - "content": "excluded.unit.tests.", + "content": "unit.tests.", "disabled": false } ], diff --git a/tests/helpers.py b/tests/helpers.py index adac81d..632f258 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -18,6 +18,7 @@ class SimpleSource(object): class SimpleProvider(object): SUPPORTS_GEO = False SUPPORTS = set(('A',)) + id = 'test' def __init__(self, id='test'): pass @@ -34,6 +35,7 @@ class SimpleProvider(object): class GeoProvider(object): SUPPORTS_GEO = True + id = 'test' def __init__(self, id='test'): pass diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index e44adc0..eb4a120 100644 --- a/tests/test_octodns_provider_base.py +++ b/tests/test_octodns_provider_base.py @@ -17,6 +17,7 @@ class HelperProvider(BaseProvider): log = getLogger('HelperProvider') SUPPORTS = set(('A',)) + id = 'test' def __init__(self, extra_changes, apply_disabled=False, include_change_callback=None): diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 04a46e0..ef8a51c 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -118,7 +118,7 @@ class TestCloudflareProvider(TestCase): zone = Zone('unit.tests.', []) provider.populate(zone) - self.assertEquals(10, len(zone.records)) + self.assertEquals(11, len(zone.records)) changes = self.expected.changes(zone, provider) self.assertEquals(0, len(changes)) @@ -126,7 +126,7 @@ class TestCloudflareProvider(TestCase): # re-populating the same zone/records comes out of cache, no calls again = Zone('unit.tests.', []) provider.populate(again) - self.assertEquals(10, len(again.records)) + self.assertEquals(11, len(again.records)) def test_apply(self): provider = CloudflareProvider('test', 'email', 'token') @@ -140,12 +140,12 @@ class TestCloudflareProvider(TestCase): 'id': 42, } }, # zone create - ] + [None] * 17 # individual record creates + ] + [None] * 18 # individual record creates # non-existant zone, create everything plan = provider.plan(self.expected) - self.assertEquals(10, len(plan.changes)) - self.assertEquals(10, provider.apply(plan)) + self.assertEquals(11, len(plan.changes)) + self.assertEquals(11, provider.apply(plan)) provider._request.assert_has_calls([ # created the domain @@ -170,7 +170,7 @@ class TestCloudflareProvider(TestCase): }), ], True) # expected number of total calls - self.assertEquals(19, provider._request.call_count) + self.assertEquals(20, provider._request.call_count) provider._request.reset_mock() diff --git a/tests/test_octodns_provider_dnsimple.py b/tests/test_octodns_provider_dnsimple.py index befb39e..0dcef32 100644 --- a/tests/test_octodns_provider_dnsimple.py +++ b/tests/test_octodns_provider_dnsimple.py @@ -78,14 +78,14 @@ class TestDnsimpleProvider(TestCase): zone = Zone('unit.tests.', []) provider.populate(zone) - self.assertEquals(17, len(zone.records)) + self.assertEquals(16, len(zone.records)) changes = self.expected.changes(zone, provider) self.assertEquals(0, len(changes)) # 2nd populate makes no network calls/all from cache again = Zone('unit.tests.', []) provider.populate(again) - self.assertEquals(17, len(again.records)) + self.assertEquals(16, len(again.records)) # bust the cache del provider._zone_records[zone.name] @@ -129,8 +129,8 @@ class TestDnsimpleProvider(TestCase): ] plan = provider.plan(self.expected) - # No root NS, no ignored - n = len(self.expected.records) - 2 + # No root NS, no ignored, no excluded + n = len(self.expected.records) - 3 self.assertEquals(n, len(plan.changes)) self.assertEquals(n, provider.apply(plan)) @@ -147,7 +147,7 @@ class TestDnsimpleProvider(TestCase): }), ]) # expected number of total calls - self.assertEquals(29, provider._client._request.call_count) + self.assertEquals(28, provider._client._request.call_count) provider._client._request.reset_mock() From 6232c685507bc1177222f7e84b364ca65856c54e Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Tue, 24 Oct 2017 14:00:12 -0400 Subject: [PATCH 03/24] 0.8.8 version bump and changelog --- CHANGELOG.md | 12 ++++++++++++ octodns/__init__.py | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c97cdc..6777ea9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,15 @@ +## v0.8.8 - 2017-10-24 - Google Cloud DNS, Large TXT Record support + +* Added support for "chunking" TXT records where individual values were larger + than 255 chars. This is common with DKIM records involving multiple + providers. +* Added `GoogleCloudProvider` +* Configurable `UnsafePlan` thresholds to allow modification of how many + updates/deletes are allowed before a plan is declared dangerous. +* Manager.dump bug fix around empty zones. +* Prefer use of `.` over `source` in shell scripts +* `DynProvider` warns when it ignores unrecognized traffic directors. + ## v0.8.7 - 2017-09-29 - OVH support Adds an OVH provider. diff --git a/octodns/__init__.py b/octodns/__init__.py index 3740dec..2166778 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.7' +__VERSION__ = '0.8.8' From bf4f7dd42d775be65c1554cbd3fa5d85c1d56a8e Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 26 Oct 2017 06:30:38 -0500 Subject: [PATCH 04/24] Allow enabling lenient on a per-record basis with octodns.lenient ``` --- '': octodns: ignored: True lenient: True type: CNAME # not valid to have a root cname value: foo.com. --- octodns/record.py | 4 ++++ tests/test_octodns_record.py | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/octodns/record.py b/octodns/record.py index 554d98b..2f67c60 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -95,6 +95,10 @@ class Record(object): except KeyError: raise Exception('Unknown record type: "{}"'.format(_type)) reasons = _class.validate(name, data) + try: + lenient |= data['octodns']['lenient'] + except KeyError: + pass if reasons: if lenient: cls.log.warn(ValidationError.build_message(fqdn, reasons)) diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 46a5e65..e7eaa61 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -733,6 +733,16 @@ class TestRecordValidation(TestCase): }, lenient=True) self.assertEquals(('value',), ctx.exception.args) + # no exception if we're in lenient mode from config + Record.new(self.zone, 'www', { + 'octodns': { + 'lenient': True + }, + 'type': 'A', + 'ttl': -1, + 'value': '1.2.3.4', + }, lenient=True) + def test_A_and_values_mixin(self): # doesn't blow up Record.new(self.zone, '', { From 00aaa3bf4d89af1ec7dc8780071824f65c42099b Mon Sep 17 00:00:00 2001 From: Adam Smith Date: Thu, 26 Oct 2017 23:55:48 -0700 Subject: [PATCH 05/24] set default value for nsone cname to None, use first value if non-zero length --- octodns/provider/ns1.py | 6 +++++- tests/test_octodns_provider_ns1.py | 31 ++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index f7cbef1..2d3af9a 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -69,10 +69,14 @@ class Ns1Provider(BaseProvider): } def _data_for_CNAME(self, _type, record): + try: + value = record['short_answers'][0] + except IndexError: + value = None return { 'ttl': record['ttl'], 'type': _type, - 'value': record['short_answers'][0], + 'value': value, } _data_for_ALIAS = _data_for_CNAME diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index cde23b0..d4f4080 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -326,3 +326,34 @@ class TestNs1Provider(TestCase): }) self.assertEquals(['foo; bar baz; blip'], provider._params_for_TXT(record)['answers']) + + def test_data_for_CNAME(self): + provider = Ns1Provider('test', 'api-key') + + # answers from nsone + a_record = { + 'ttl': 31, + 'type': 'CNAME', + 'short_answers': ['foo.unit.tests.'] + } + a_expected = { + 'ttl': 31, + 'type': 'CNAME', + 'value': 'foo.unit.tests.' + } + self.assertEqual(a_expected, + provider._data_for_CNAME(a_record['type'], a_record)) + + # no answers from nsone + b_record = { + 'ttl': 32, + 'type': 'CNAME', + 'short_answers': [] + } + b_expected = { + 'ttl': 32, + 'type': 'CNAME', + 'value': None + } + self.assertEqual(b_expected, + provider._data_for_CNAME(b_record['type'], b_record)) From bf1896329ba1c8f5829895ecfe71e233ca0173cc Mon Sep 17 00:00:00 2001 From: Adam Smith Date: Sun, 22 Oct 2017 22:39:18 -0700 Subject: [PATCH 06/24] validate values for empty string or None value dump does not write invalid value(s) to yaml --- octodns/record.py | 42 +++++++- tests/test_octodns_provider_dnsimple.py | 8 +- tests/test_octodns_record.py | 125 +++++++++++++++++++++++- 3 files changed, 165 insertions(+), 10 deletions(-) diff --git a/octodns/record.py b/octodns/record.py index 2f67c60..f9812a6 100644 --- a/octodns/record.py +++ b/octodns/record.py @@ -211,9 +211,30 @@ class _ValuesMixin(object): values = [] try: values = data['values'] + if not values: + values = [] + reasons.append('missing value(s)') + else: + # loop through copy of values + # remove invalid value from values + for value in list(values): + if value is None: + reasons.append('missing value(s)') + values.remove(value) + elif len(value) == 0: + reasons.append('empty value') + values.remove(value) except KeyError: try: - values = [data['value']] + value = data['value'] + if value is None: + reasons.append('missing value(s)') + values = [] + elif len(value) == 0: + reasons.append('empty value') + values = [] + else: + values = [value] except KeyError: reasons.append('missing value(s)') @@ -238,10 +259,16 @@ class _ValuesMixin(object): def _data(self): ret = super(_ValuesMixin, self)._data() if len(self.values) > 1: - ret['values'] = [getattr(v, 'data', v) for v in self.values] - else: + values = [getattr(v, 'data', v) for v in self.values if v] + if len(values) > 1: + ret['values'] = values + elif len(values) == 1: + ret['value'] = values[0] + elif len(self.values) == 1: v = self.values[0] - ret['value'] = getattr(v, 'data', v) + if v: + ret['value'] = getattr(v, 'data', v) + return ret def __repr__(self): @@ -349,6 +376,10 @@ class _ValueMixin(object): value = None try: value = data['value'] + if value is None: + reasons.append('missing value') + elif value == '': + reasons.append('empty value') except KeyError: reasons.append('missing value') if value: @@ -366,7 +397,8 @@ class _ValueMixin(object): def _data(self): ret = super(_ValueMixin, self)._data() - ret['value'] = getattr(self.value, 'data', self.value) + if self.value: + ret['value'] = getattr(self.value, 'data', self.value) return ret def __repr__(self): diff --git a/tests/test_octodns_provider_dnsimple.py b/tests/test_octodns_provider_dnsimple.py index 950d460..b44d6ee 100644 --- a/tests/test_octodns_provider_dnsimple.py +++ b/tests/test_octodns_provider_dnsimple.py @@ -96,23 +96,23 @@ class TestDnsimpleProvider(TestCase): mock.get(ANY, text=fh.read()) zone = Zone('unit.tests.', []) - provider.populate(zone) + provider.populate(zone, lenient=True) self.assertEquals(set([ Record.new(zone, '', { 'ttl': 3600, 'type': 'SSHFP', 'values': [] - }), + }, lenient=True), Record.new(zone, '_srv._tcp', { 'ttl': 600, 'type': 'SRV', 'values': [] - }), + }, lenient=True), Record.new(zone, 'naptr', { 'ttl': 600, 'type': 'NAPTR', 'values': [] - }), + }, lenient=True), ]), zone.records) def test_apply(self): diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index e7eaa61..ff57975 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -96,6 +96,57 @@ class TestRecord(TestCase): DummyRecord().__repr__() + def test_values_mixin_data(self): + # no values, no value or values in data + a = ARecord(self.zone, '', { + 'type': 'A', + 'ttl': 600, + 'values': [] + }) + self.assertNotIn('values', a.data) + + # empty value, no value or values in data + b = ARecord(self.zone, '', { + 'type': 'A', + 'ttl': 600, + 'values': [''] + }) + self.assertNotIn('value', b.data) + + # empty/None values, no value or values in data + c = ARecord(self.zone, '', { + 'type': 'A', + 'ttl': 600, + 'values': ['', None] + }) + self.assertNotIn('values', c.data) + + # empty/None values and valid, value in data + c = ARecord(self.zone, '', { + 'type': 'A', + 'ttl': 600, + 'values': ['', None, '10.10.10.10'] + }) + self.assertNotIn('values', c.data) + self.assertEqual('10.10.10.10', c.data['value']) + + def test_value_mixin_data(self): + # unspecified value, no value in data + a = AliasRecord(self.zone, '', { + 'type': 'ALIAS', + 'ttl': 600, + 'value': None + }) + self.assertNotIn('value', a.data) + + # unspecified value, no value in data + a = AliasRecord(self.zone, '', { + 'type': 'ALIAS', + 'ttl': 600, + 'value': '' + }) + self.assertNotIn('value', a.data) + def test_geo(self): geo_data = {'ttl': 42, 'values': ['5.2.3.4', '6.2.3.4'], 'geo': {'AF': ['1.1.1.1'], @@ -750,6 +801,13 @@ class TestRecordValidation(TestCase): 'ttl': 600, 'value': '1.2.3.4', }) + Record.new(self.zone, '', { + 'type': 'A', + 'ttl': 600, + 'values': [ + '1.2.3.4', + ] + }) Record.new(self.zone, '', { 'type': 'A', 'ttl': 600, @@ -759,13 +817,60 @@ class TestRecordValidation(TestCase): ] }) - # missing value(s) + # missing value(s), no value or value with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, '', { 'type': 'A', 'ttl': 600, }) self.assertEquals(['missing value(s)'], ctx.exception.reasons) + + # missing value(s), empty values + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'www', { + 'type': 'A', + 'ttl': 600, + 'values': [] + }) + self.assertEquals(['missing value(s)'], ctx.exception.reasons) + + # missing value(s), None values + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'www', { + 'type': 'A', + 'ttl': 600, + 'values': None + }) + self.assertEquals(['missing value(s)'], ctx.exception.reasons) + + # missing value(s) and empty value + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'www', { + 'type': 'A', + 'ttl': 600, + 'values': [None, ''] + }) + self.assertEquals(['missing value(s)', + 'empty value'], ctx.exception.reasons) + + # missing value(s), None value + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'www', { + 'type': 'A', + 'ttl': 600, + 'value': None + }) + self.assertEquals(['missing value(s)'], ctx.exception.reasons) + + # empty value, empty string value + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'www', { + 'type': 'A', + 'ttl': 600, + 'value': '' + }) + self.assertEquals(['empty value'], ctx.exception.reasons) + # missing value(s) & ttl with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, '', { @@ -922,6 +1027,24 @@ class TestRecordValidation(TestCase): }) self.assertEquals(['missing value'], ctx.exception.reasons) + # missing value + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'www', { + 'type': 'ALIAS', + 'ttl': 600, + 'value': None + }) + self.assertEquals(['missing value'], ctx.exception.reasons) + + # empty value + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'www', { + 'type': 'ALIAS', + 'ttl': 600, + 'value': '' + }) + self.assertEquals(['empty value'], ctx.exception.reasons) + # missing trailing . with self.assertRaises(ValidationError) as ctx: Record.new(self.zone, '', { From 7352978880d5ce995073f568224609d4da74cdb5 Mon Sep 17 00:00:00 2001 From: Paul van Brouwershaven Date: Mon, 30 Oct 2017 18:33:51 +0100 Subject: [PATCH 07/24] Check excluded in desired_record --- octodns/zone.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/octodns/zone.py b/octodns/zone.py index a25333f..bbc38d0 100644 --- a/octodns/zone.py +++ b/octodns/zone.py @@ -125,13 +125,13 @@ class Zone(object): desired_record = desired_records[record] if desired_record.ignored: continue - elif len(record.included) > 0 and \ - target.id not in record.included: + elif len(desired_record.included) > 0 and \ + target.id not in desired_record.included: self.log.debug('changes: skipping record=%s %s - %s' 'not included ', record.fqdn, record._type, target.id) continue - elif target.id in record.excluded: + elif target.id in desired_record.excluded: continue except KeyError: if not target.supports(record): From 6261ded87974dbc77652e6c3294c791bf96bd021 Mon Sep 17 00:00:00 2001 From: Paul van Brouwershaven Date: Mon, 30 Oct 2017 18:34:22 +0100 Subject: [PATCH 08/24] Add more include/exclude tests --- tests/test_octodns_zone.py | 99 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/tests/test_octodns_zone.py b/tests/test_octodns_zone.py index 8d75100..94faef3 100644 --- a/tests/test_octodns_zone.py +++ b/tests/test_octodns_zone.py @@ -236,3 +236,102 @@ class TestZone(TestCase): zone.add_record(cname) with self.assertRaises(InvalidNodeException): zone.add_record(a) + + def test_excluded_records(self): + zone_normal = Zone('unit.tests.', []) + zone_excluded = Zone('unit.tests.', []) + zone_missing = Zone('unit.tests.', []) + + normal = Record.new(zone_normal, 'www', { + 'ttl': 60, + 'type': 'A', + 'value': '9.9.9.9', + }) + zone_normal.add_record(normal) + + excluded = Record.new(zone_excluded, 'www', { + 'octodns': { + 'excluded': ['test'] + }, + 'ttl': 60, + 'type': 'A', + 'value': '9.9.9.9', + }) + zone_excluded.add_record(excluded) + + provider = SimpleProvider() + + self.assertFalse(zone_normal.changes(zone_excluded, provider)) + self.assertTrue(zone_normal.changes(zone_missing, provider)) + + self.assertFalse(zone_excluded.changes(zone_normal, provider)) + self.assertFalse(zone_excluded.changes(zone_missing, provider)) + + self.assertTrue(zone_missing.changes(zone_normal, provider)) + self.assertFalse(zone_missing.changes(zone_excluded, provider)) + + def test_included_records(self): + zone_normal = Zone('unit.tests.', []) + zone_included = Zone('unit.tests.', []) + zone_missing = Zone('unit.tests.', []) + + normal = Record.new(zone_normal, 'www', { + 'ttl': 60, + 'type': 'A', + 'value': '9.9.9.9', + }) + zone_normal.add_record(normal) + + included = Record.new(zone_included, 'www', { + 'octodns': { + 'included': ['test'] + }, + 'ttl': 60, + 'type': 'A', + 'value': '9.9.9.9', + }) + zone_included.add_record(included) + + provider = SimpleProvider() + + self.assertFalse(zone_normal.changes(zone_included, provider)) + self.assertTrue(zone_normal.changes(zone_missing, provider)) + + self.assertFalse(zone_included.changes(zone_normal, provider)) + self.assertTrue(zone_included.changes(zone_missing, provider)) + + self.assertTrue(zone_missing.changes(zone_normal, provider)) + self.assertTrue(zone_missing.changes(zone_included, provider)) + + def test_not_included_records(self): + zone_normal = Zone('unit.tests.', []) + zone_included = Zone('unit.tests.', []) + zone_missing = Zone('unit.tests.', []) + + normal = Record.new(zone_normal, 'www', { + 'ttl': 60, + 'type': 'A', + 'value': '9.9.9.9', + }) + zone_normal.add_record(normal) + + included = Record.new(zone_included, 'www', { + 'octodns': { + 'included': ['not-here'] + }, + 'ttl': 60, + 'type': 'A', + 'value': '9.9.9.9', + }) + zone_included.add_record(included) + + provider = SimpleProvider() + + self.assertFalse(zone_normal.changes(zone_included, provider)) + self.assertTrue(zone_normal.changes(zone_missing, provider)) + + self.assertFalse(zone_included.changes(zone_normal, provider)) + self.assertFalse(zone_included.changes(zone_missing, provider)) + + self.assertTrue(zone_missing.changes(zone_normal, provider)) + self.assertFalse(zone_missing.changes(zone_included, provider)) From 6b1a8f8ccf4d930d6ddf5ea1d0161d4ac703f248 Mon Sep 17 00:00:00 2001 From: trnsnt Date: Mon, 23 Oct 2017 17:12:32 +0200 Subject: [PATCH 09/24] OVH: Add support of DKIM records --- octodns/provider/ovh.py | 77 +++++++++- tests/test_octodns_provider_ovh.py | 238 +++++++++++++++++++---------- 2 files changed, 231 insertions(+), 84 deletions(-) diff --git a/octodns/provider/ovh.py b/octodns/provider/ovh.py index b890862..5c2fe0d 100644 --- a/octodns/provider/ovh.py +++ b/octodns/provider/ovh.py @@ -5,6 +5,8 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals +import base64 +import binascii import logging from collections import defaultdict @@ -32,8 +34,10 @@ class OvhProvider(BaseProvider): SUPPORTS_GEO = False - SUPPORTS = set(('A', 'AAAA', 'CNAME', 'MX', 'NAPTR', 'NS', 'PTR', 'SPF', - 'SRV', 'SSHFP', 'TXT')) + # This variable is also used in populate method to filter which OVH record + # types are supported by octodns + SUPPORTS = set(('A', 'AAAA', 'CNAME', 'DKIM', 'MX', 'NAPTR', 'NS', 'PTR', + 'SPF', 'SRV', 'SSHFP', 'TXT')) def __init__(self, id, endpoint, application_key, application_secret, consumer_key, *args, **kwargs): @@ -62,6 +66,10 @@ class OvhProvider(BaseProvider): before = len(zone.records) for name, types in values.items(): for _type, records in types.items(): + if _type not in self.SUPPORTS: + self.log.warning('Not managed record of type %s, skip', + _type) + continue data_for = getattr(self, '_data_for_{}'.format(_type)) record = Record.new(zone, name, data_for(_type, records), source=self, lenient=lenient) @@ -96,7 +104,11 @@ class OvhProvider(BaseProvider): def _apply_delete(self, zone_name, change): existing = change.existing - self.delete_records(zone_name, existing._type, existing.name) + record_type = existing._type + if record_type == "TXT": + if self._is_valid_dkim(existing.values[0]): + record_type = 'DKIM' + self.delete_records(zone_name, record_type, existing.name) @staticmethod def _data_for_multiple(_type, records): @@ -184,6 +196,15 @@ class OvhProvider(BaseProvider): 'values': values } + @staticmethod + def _data_for_DKIM(_type, records): + return { + 'ttl': records[0]['ttl'], + 'type': "TXT", + 'values': [record['target'].replace(';', '\;') + for record in records] + } + _data_for_A = _data_for_multiple _data_for_AAAA = _data_for_multiple _data_for_NS = _data_for_multiple @@ -258,15 +279,63 @@ class OvhProvider(BaseProvider): 'fieldType': record._type } + def _params_for_TXT(self, record): + for value in record.values: + field_type = 'TXT' + if self._is_valid_dkim(value): + field_type = 'DKIM' + value = value.replace("\;", ";") + yield { + 'target': value, + 'subDomain': record.name, + 'ttl': record.ttl, + 'fieldType': field_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 _is_valid_dkim(self, value): + """Check if value is a valid DKIM""" + validator_dict = {'h': lambda val: val in ['sha1', 'sha256'], + 's': lambda val: val in ['*', 'email'], + 't': lambda val: val in ['y', 's'], + 'v': lambda val: val == 'DKIM1', + 'k': lambda val: val == 'rsa', + 'n': lambda _: True, + 'g': lambda _: True} + + splitted = value.split('\;') + found_key = False + for splitted_value in splitted: + sub_split = map(lambda x: x.strip(), splitted_value.split("=", 1)) + if len(sub_split) < 2: + return False + key, value = sub_split[0], sub_split[1] + if key == "p": + is_valid_key = self._is_valid_dkim_key(value) + if not is_valid_key: + return False + found_key = True + else: + is_valid_key = validator_dict.get(key, lambda _: False)(value) + if not is_valid_key: + return False + return found_key + + @staticmethod + def _is_valid_dkim_key(key): + try: + base64.decodestring(key) + except binascii.Error: + return False + return True + def get_records(self, zone_name): """ List all records of a DNS zone diff --git a/tests/test_octodns_provider_ovh.py b/tests/test_octodns_provider_ovh.py index 2816748..6a44e25 100644 --- a/tests/test_octodns_provider_ovh.py +++ b/tests/test_octodns_provider_ovh.py @@ -17,6 +17,14 @@ from octodns.zone import Zone class TestOvhProvider(TestCase): api_record = [] + valid_dkim = [] + invalid_dkim = [] + + valid_dkim_key = "p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCxLaG16G4SaE" \ + "cXVdiIxTg7gKSGbHKQLm30CHib1h9FzS9nkcyvQSyQj1rMFyqC//" \ + "tft3ohx3nvJl+bGCWxdtLYDSmir9PW54e5CTdxEh8MWRkBO3StF6" \ + "QG/tAh3aTGDmkqhIJGLb87iHvpmVKqURmEUzJPv5KPJfWLofADI+" \ + "q9lQIDAQAB" zone = Zone('unit.tests.', []) expected = set() @@ -233,6 +241,65 @@ class TestOvhProvider(TestCase): 'value': '1:1ec:1::1', })) + # DKIM + api_record.append({ + 'fieldType': 'DKIM', + 'ttl': 1300, + 'target': valid_dkim_key, + 'subDomain': 'dkim', + 'id': 16 + }) + expected.add(Record.new(zone, 'dkim', { + 'ttl': 1300, + 'type': 'TXT', + 'value': valid_dkim_key, + })) + + # TXT + api_record.append({ + 'fieldType': 'TXT', + 'ttl': 1400, + 'target': 'TXT text', + 'subDomain': 'txt', + 'id': 17 + }) + expected.add(Record.new(zone, 'txt', { + 'ttl': 1400, + 'type': 'TXT', + 'value': 'TXT text', + })) + + # LOC + # We do not have associated record for LOC, as it's not managed + api_record.append({ + 'fieldType': 'LOC', + 'ttl': 1500, + 'target': '1 1 1 N 1 1 1 E 1m 1m', + 'subDomain': '', + 'id': 18 + }) + + valid_dkim = [valid_dkim_key, + 'v=DKIM1 \; %s' % valid_dkim_key, + 'h=sha256 \; %s' % valid_dkim_key, + 'h=sha1 \; %s' % valid_dkim_key, + 's=* \; %s' % valid_dkim_key, + 's=email \; %s' % valid_dkim_key, + 't=y \; %s' % valid_dkim_key, + 't=s \; %s' % valid_dkim_key, + 'k=rsa \; %s' % valid_dkim_key, + 'n=notes \; %s' % valid_dkim_key, + 'g=granularity \; %s' % valid_dkim_key, + ] + invalid_dkim = ['p=%invalid%', # Invalid public key + 'v=DKIM1', # Missing public key + 'v=DKIM2 \; %s' % valid_dkim_key, # Invalid version + 'h=sha512 \; %s' % valid_dkim_key, # Invalid hash algo + 's=fake \; %s' % valid_dkim_key, # Invalid selector + 't=fake \; %s' % valid_dkim_key, # Invalid flag + 'u=invalid \; %s' % valid_dkim_key, # Invalid key + ] + @patch('ovh.Client') def test_populate(self, client_mock): provider = OvhProvider('test', 'endpoint', 'application_key', @@ -253,6 +320,16 @@ class TestOvhProvider(TestCase): provider.populate(zone) self.assertEquals(self.expected, zone.records) + @patch('ovh.Client') + def test_is_valid_dkim(self, client_mock): + """Test _is_valid_dkim""" + provider = OvhProvider('test', 'endpoint', 'application_key', + 'application_secret', 'consumer_key') + for dkim in self.valid_dkim: + self.assertTrue(provider._is_valid_dkim(dkim)) + for dkim in self.invalid_dkim: + self.assertFalse(provider._is_valid_dkim(dkim)) + @patch('ovh.Client') def test_apply(self, client_mock): provider = OvhProvider('test', 'endpoint', 'application_key', @@ -270,90 +347,91 @@ class TestOvhProvider(TestCase): provider.apply(plan) self.assertEquals(get_mock.side_effect, ctx.exception) + # Records get by API call 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_returns = [ + [1, 2, 3, 4], + {'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}, + {'fieldType': 'TXT', 'ttl': 600, 'target': 'fake txt record', + 'subDomain': 'txt', 'id': 102}, + {'fieldType': 'DKIM', 'ttl': 600, + 'target': 'v=DKIM1; %s' % self.valid_dkim_key, + 'subDomain': 'dkim', 'id': 103} + ] 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')] + with patch.object(provider._client, 'post') as post_mock, \ + patch.object(provider._client, 'delete') as delete_mock: + get_mock.side_effect = [[100], [101], [102], [103]] + provider.apply(plan) + wanted_calls = [ + call(u'/domain/zone/unit.tests/record', fieldType=u'TXT', + subDomain='txt', target=u'TXT text', ttl=1400), + call(u'/domain/zone/unit.tests/record', fieldType=u'DKIM', + subDomain='dkim', target=self.valid_dkim_key, + ttl=1300), + 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', target=u'', ttl=1100, + subDomain=u'1 1 bf6b6825d2977c511a475bbefb88a' + u'ad54' + u'a92ac73', + ), + 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'', ttl=1000, + target=u'v=spf1 include:unit.texts.' + u'rerirect ~all', + ), + 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', ttl=500, + target=u'10 100 "S" "SIP+D2U" "!^.*$!sip:' + u'info@bar' + u'.example.com!" .' + ), + call(u'/domain/zone/unit.tests/refresh')] - post_mock.assert_has_calls(wanted_calls) + 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')]) + # Get for delete calls + wanted_get_calls = [ + call(u'/domain/zone/unit.tests/record', fieldType=u'TXT', + subDomain='txt'), + call(u'/domain/zone/unit.tests/record', fieldType=u'DKIM', + subDomain='dkim'), + call(u'/domain/zone/unit.tests/record', fieldType=u'A', + subDomain=u''), + call(u'/domain/zone/unit.tests/record', fieldType=u'A', + subDomain='fake')] + get_mock.assert_has_calls(wanted_get_calls) + # 4 delete calls for update and delete + delete_mock.assert_has_calls( + [call(u'/domain/zone/unit.tests/record/100'), + call(u'/domain/zone/unit.tests/record/101'), + call(u'/domain/zone/unit.tests/record/102'), + call(u'/domain/zone/unit.tests/record/103')]) From ba3ad27c024cad228b5f61cbd8b429b147587645 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 2 Nov 2017 06:50:15 -0700 Subject: [PATCH 10/24] Make PowerDnsBaseProvider's timeout configurable --- octodns/provider/powerdns.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/octodns/provider/powerdns.py b/octodns/provider/powerdns.py index 20cfe8b..4527f8e 100644 --- a/octodns/provider/powerdns.py +++ b/octodns/provider/powerdns.py @@ -18,13 +18,14 @@ class PowerDnsBaseProvider(BaseProvider): 'PTR', 'SPF', 'SSHFP', 'SRV', 'TXT')) TIMEOUT = 5 - def __init__(self, id, host, api_key, port=8081, scheme="http", *args, - **kwargs): + def __init__(self, id, host, api_key, port=8081, scheme="http", + timeout=TIMEOUT, *args, **kwargs): super(PowerDnsBaseProvider, self).__init__(id, *args, **kwargs) self.host = host self.port = port self.scheme = scheme + self.timeout = timeout sess = Session() sess.headers.update({'X-API-Key': api_key}) @@ -35,7 +36,7 @@ class PowerDnsBaseProvider(BaseProvider): url = '{}://{}:{}/api/v1/servers/localhost/{}' \ .format(self.scheme, self.host, self.port, path) - resp = self._sess.request(method, url, json=data, timeout=self.TIMEOUT) + resp = self._sess.request(method, url, json=data, timeout=self.timeout) self.log.debug('_request: status=%d', resp.status_code) resp.raise_for_status() return resp From 77d2fd1eb401adbd68410c67f24a45b30eb3f622 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Beraud?= Date: Fri, 3 Nov 2017 22:31:38 +0100 Subject: [PATCH 11/24] improve setuptools capabilities --- README.md | 1 + octodns/__init__.py | 25 +++++++++++++++-- requirements-dev.txt | 7 ----- requirements.txt | 23 --------------- script/bootstrap | 4 +-- setup.cfg | 67 ++++++++++++++++++++++++++++++++++++++++++++ setup.py | 46 ++---------------------------- 7 files changed, 94 insertions(+), 79 deletions(-) delete mode 100644 requirements-dev.txt delete mode 100644 requirements.txt create mode 100644 setup.cfg diff --git a/README.md b/README.md index a910b5b..ec9164f 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,7 @@ $ cd dns $ virtualenv env ... $ source env/bin/activate +$ pip install -U setuptools $ pip install octodns $ mkdir config ``` diff --git a/octodns/__init__.py b/octodns/__init__.py index 2166778..aaaa2a5 100644 --- a/octodns/__init__.py +++ b/octodns/__init__.py @@ -1,6 +1,25 @@ -'OctoDNS: DNS as code - Tools for managing DNS across multiple providers' - from __future__ import absolute_import, division, print_function, \ unicode_literals +import pkg_resources +from os import path +from setuptools.config import read_configuration -__VERSION__ = '0.8.8' + +def _extract_version(package_name): + try: + return pkg_resources.get_distribution(package_name).version + except pkg_resources.DistributionNotFound: + _conf = read_configuration( + path.join( + path.dirname(path.dirname(__file__)), + 'setup.cfg' + ) + ) + return _conf['metadata']['version'] + + +__version__ = _extract_version('octodns') + + +if __name__ == "__main__": + print(__version__) diff --git a/requirements-dev.txt b/requirements-dev.txt deleted file mode 100644 index 5cdf252..0000000 --- a/requirements-dev.txt +++ /dev/null @@ -1,7 +0,0 @@ -coverage -mock -nose -pep8 -pyflakes -requests_mock -setuptools>=36.4.0 diff --git a/requirements.txt b/requirements.txt deleted file mode 100644 index 80fbe1e..0000000 --- a/requirements.txt +++ /dev/null @@ -1,23 +0,0 @@ -# These are known good versions. You're free to use others and things will -# likely work, but no promises are made, especilly if you go older. -PyYaml==3.12 -azure-mgmt-dns==1.0.1 -azure-common==1.1.6 -boto3==1.4.6 -botocore==1.6.8 -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 -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 -six==1.10.0 diff --git a/script/bootstrap b/script/bootstrap index 1f76914..dfbb142 100755 --- a/script/bootstrap +++ b/script/bootstrap @@ -19,10 +19,10 @@ if [ ! -d "$VENV_NAME" ]; then fi . "$VENV_NAME/bin/activate" -pip install -U -r requirements.txt +pip install -e . if [ "$ENV" != "production" ]; then - pip install -U -r requirements-dev.txt + pip install -e .[dev] fi if [ ! -L ".git/hooks/pre-commit" ]; then diff --git a/setup.cfg b/setup.cfg new file mode 100644 index 0000000..f21fdbb --- /dev/null +++ b/setup.cfg @@ -0,0 +1,67 @@ +[metadata] +name = octodns +description = "DNS as code - Tools for managing DNS across multiple providers" +long_description = file: README.md +version = 0.8.8 +author = Ross McFarland +author_email = rwmcfa1@gmail.com +url = https://github.com/github/octodns +license = MIT +keywords = dns, providers +classifiers = + License :: OSI Approved :: MIT License + Programming Language :: Python + Programming Language :: Python :: 2.7 + Programming Language :: Python :: 3 + Programming Language :: Python :: 3.3 + Programming Language :: Python :: 3.4 + Programming Language :: Python :: 3.5 + Programming Language :: Python :: 3.6 + +[options] +install_requires = + PyYaml==3.12 + azure-mgmt-dns==1.0.1 + azure-common==1.1.6 + boto3==1.4.6 + botocore==1.6.8 + 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 + 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 + six==1.10.0 +packages = find: +include_package_data = True + +[options.entry_points] +console_scripts = + octodns-compare = octodns.cmds.compare:main + octodns-dump = octodns.cmds.dump:main + octodns-report = octodns.cmds.report:main + octodns-sync = octodns.cmds.sync:main + octodns-validate = octodns.cmds.validate:main + +[options.packages.find] +exclude = + tests + +[options.extras_require] +dev = + coverage + mock + nose + pep8 + pyflakes + requests_mock + setuptools>=36.4.0 diff --git a/setup.py b/setup.py index f2b901d..2598061 100644 --- a/setup.py +++ b/setup.py @@ -1,47 +1,5 @@ #!/usr/bin/env python +from setuptools import setup -from os.path import dirname, join -import octodns -try: - from setuptools import find_packages, setup -except ImportError: - from distutils.core import find_packages, setup - -cmds = ( - 'compare', - 'dump', - 'report', - 'sync', - 'validate' -) -cmds_dir = join(dirname(__file__), 'octodns', 'cmds') -console_scripts = { - 'octodns-{name} = octodns.cmds.{name}:main'.format(name=name) - for name in cmds -} - -setup( - author='Ross McFarland', - author_email='rwmcfa1@gmail.com', - description=octodns.__doc__, - entry_points={ - 'console_scripts': console_scripts, - }, - install_requires=[ - 'PyYaml>=3.12', - 'dnspython>=1.15.0', - 'futures>=3.0.5', - 'incf.countryutils>=1.0', - 'ipaddress>=1.0.18', - 'natsort>=5.0.3', - 'python-dateutil>=2.6.0', - 'requests>=2.13.0' - ], - license='MIT', - long_description=open('README.md').read(), - name='octodns', - packages=find_packages(), - url='https://github.com/github/octodns', - version=octodns.__VERSION__, -) +setup() From dd692320c9f1779497df9b8cbc30c4d9ebc2337d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Beraud?= Date: Sat, 4 Nov 2017 22:01:18 +0100 Subject: [PATCH 12/24] Apply review comments define 3 kinds of requirements (base, dev, test) retrieve version from __init__.py define setuptools minimal version in CI install full (base, dev, test) dependencies --- MANIFEST.in | 1 - README.md | 2 +- octodns/__init__.py | 22 +--------------------- script/bootstrap | 2 +- setup.cfg | 35 ++++++++++++++++++----------------- 5 files changed, 21 insertions(+), 41 deletions(-) diff --git a/MANIFEST.in b/MANIFEST.in index 3a26904..cda90ed 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -3,6 +3,5 @@ include CONTRIBUTING.md include LICENSE include docs/* include octodns/* -include requirements*.txt include script/* include tests/* diff --git a/README.md b/README.md index ec9164f..ed1ac3b 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,7 @@ $ cd dns $ virtualenv env ... $ source env/bin/activate -$ pip install -U setuptools +$ pip install -U setuptools>⁼30.3.0 $ pip install octodns $ mkdir config ``` diff --git a/octodns/__init__.py b/octodns/__init__.py index aaaa2a5..05a5e84 100644 --- a/octodns/__init__.py +++ b/octodns/__init__.py @@ -1,25 +1,5 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals -import pkg_resources -from os import path -from setuptools.config import read_configuration -def _extract_version(package_name): - try: - return pkg_resources.get_distribution(package_name).version - except pkg_resources.DistributionNotFound: - _conf = read_configuration( - path.join( - path.dirname(path.dirname(__file__)), - 'setup.cfg' - ) - ) - return _conf['metadata']['version'] - - -__version__ = _extract_version('octodns') - - -if __name__ == "__main__": - print(__version__) +__version__ = '0.8.8' diff --git a/script/bootstrap b/script/bootstrap index dfbb142..7f4a5a8 100755 --- a/script/bootstrap +++ b/script/bootstrap @@ -22,7 +22,7 @@ fi pip install -e . if [ "$ENV" != "production" ]; then - pip install -e .[dev] + pip install -e .[dev,test] fi if [ ! -L ".git/hooks/pre-commit" ]; then diff --git a/setup.cfg b/setup.cfg index f21fdbb..70baaf6 100644 --- a/setup.cfg +++ b/setup.cfg @@ -2,7 +2,7 @@ name = octodns description = "DNS as code - Tools for managing DNS across multiple providers" long_description = file: README.md -version = 0.8.8 +version = attr: octodns.__version__ author = Ross McFarland author_email = rwmcfa1@gmail.com url = https://github.com/github/octodns @@ -20,27 +20,14 @@ classifiers = [options] install_requires = - PyYaml==3.12 - azure-mgmt-dns==1.0.1 - azure-common==1.1.6 - boto3==1.4.6 - botocore==1.6.8 - dnspython==1.15.0 - docutils==0.14 - dyn==1.8.0 + PyYaml>=3.12 + dnspython>=1.15.0 futures==3.1.1 - google-cloud==0.27.0 incf.countryutils==1.0 ipaddress==1.0.18 - 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 - six==1.10.0 packages = find: include_package_data = True @@ -57,7 +44,21 @@ exclude = tests [options.extras_require] -dev = +dev = + azure-mgmt-dns==1.0.1 + azure-common==1.1.6 + boto3==1.4.6 + botocore==1.6.8 + docutils==0.14 + dyn==1.8.0 + google-cloud==0.27.0 + jmespath==0.9.3 + msrestazure==0.4.10 + nsone==0.9.14 + ovh==0.4.7 + s3transfer==0.1.10 + six==1.10.0 +test = coverage mock nose From 2ee1a41a78ba948fb8e4e07b4991a8c0efb92cc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Beraud?= Date: Sun, 5 Nov 2017 12:37:06 +0100 Subject: [PATCH 13/24] Remove setuptools minimal version adding minimal version only for contributors --- CONTRIBUTING.md | 4 ++++ README.md | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9a5709a..36337eb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -38,6 +38,10 @@ Here are a few things you can do that will increase the likelihood of your pull - Write a [good commit message](http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html). +## Development prerequisites + +- setuptools >= 30.3.0 + ## License note We can only accept contributions that are compatible with the MIT license. diff --git a/README.md b/README.md index ed1ac3b..a910b5b 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,6 @@ $ cd dns $ virtualenv env ... $ source env/bin/activate -$ pip install -U setuptools>⁼30.3.0 $ pip install octodns $ mkdir config ``` From 7fa999953fc3229f95053d0c36a6ee14f1251c63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Beraud?= Date: Sun, 5 Nov 2017 13:38:31 +0100 Subject: [PATCH 14/24] reset changes on __init__ fix error on version --- octodns/__init__.py | 5 +++-- setup.cfg | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/octodns/__init__.py b/octodns/__init__.py index 05a5e84..2166778 100644 --- a/octodns/__init__.py +++ b/octodns/__init__.py @@ -1,5 +1,6 @@ +'OctoDNS: DNS as code - Tools for managing DNS across multiple providers' + from __future__ import absolute_import, division, print_function, \ unicode_literals - -__version__ = '0.8.8' +__VERSION__ = '0.8.8' diff --git a/setup.cfg b/setup.cfg index 70baaf6..54e9014 100644 --- a/setup.cfg +++ b/setup.cfg @@ -2,7 +2,7 @@ name = octodns description = "DNS as code - Tools for managing DNS across multiple providers" long_description = file: README.md -version = attr: octodns.__version__ +version = attr: octodns.__VERSION__ author = Ross McFarland author_email = rwmcfa1@gmail.com url = https://github.com/github/octodns From 454f7f8c8fe9757a23a33ca52ced0ebc32a7cdbe Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 8 Nov 2017 06:26:18 -0800 Subject: [PATCH 15/24] Add formal CAA support to YamlProvider --- octodns/provider/yaml.py | 4 ++-- tests/test_octodns_manager.py | 14 +++++++------- tests/test_octodns_provider_yaml.py | 14 +++++++++----- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index fe1a406..752e793 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -31,8 +31,8 @@ class YamlProvider(BaseProvider): enforce_order: True ''' SUPPORTS_GEO = True - SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CNAME', 'MX', 'NAPTR', 'NS', 'PTR', - 'SSHFP', 'SPF', 'SRV', 'TXT')) + SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'MX', 'NAPTR', 'NS', + 'PTR', 'SSHFP', 'SPF', 'SRV', 'TXT')) def __init__(self, id, directory, default_ttl=3600, enforce_order=True, *args, **kwargs): diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 36b1f4c..4db2103 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -102,12 +102,12 @@ class TestManager(TestCase): environ['YAML_TMP_DIR'] = tmpdir.dirname tc = Manager(get_config_filename('simple.yaml')) \ .sync(dry_run=False) - self.assertEquals(20, tc) + self.assertEquals(21, tc) # try with just one of the zones tc = Manager(get_config_filename('simple.yaml')) \ .sync(dry_run=False, eligible_zones=['unit.tests.']) - self.assertEquals(14, tc) + self.assertEquals(15, tc) # the subzone, with 2 targets tc = Manager(get_config_filename('simple.yaml')) \ @@ -122,18 +122,18 @@ class TestManager(TestCase): # Again with force tc = Manager(get_config_filename('simple.yaml')) \ .sync(dry_run=False, force=True) - self.assertEquals(20, tc) + self.assertEquals(21, tc) # Again with max_workers = 1 tc = Manager(get_config_filename('simple.yaml'), max_workers=1) \ .sync(dry_run=False, force=True) - self.assertEquals(20, tc) + self.assertEquals(21, tc) # Include meta tc = Manager(get_config_filename('simple.yaml'), max_workers=1, include_meta=True) \ .sync(dry_run=False, force=True) - self.assertEquals(24, tc) + self.assertEquals(25, tc) def test_eligible_targets(self): with TemporaryDirectory() as tmpdir: @@ -159,13 +159,13 @@ class TestManager(TestCase): fh.write('---\n{}') changes = manager.compare(['in'], ['dump'], 'unit.tests.') - self.assertEquals(14, len(changes)) + self.assertEquals(15, len(changes)) # Compound sources with varying support changes = manager.compare(['in', 'nosshfp'], ['dump'], 'unit.tests.') - self.assertEquals(13, len(changes)) + self.assertEquals(14, len(changes)) with self.assertRaises(Exception) as ctx: manager.compare(['nope'], ['dump'], 'unit.tests.') diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index a199355..46363ed 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -49,12 +49,12 @@ class TestYamlProvider(TestCase): # We add everything plan = target.plan(zone) - self.assertEquals(14, len(filter(lambda c: isinstance(c, Create), + self.assertEquals(15, len(filter(lambda c: isinstance(c, Create), plan.changes))) self.assertFalse(isfile(yaml_file)) # Now actually do it - self.assertEquals(14, target.apply(plan)) + self.assertEquals(15, target.apply(plan)) self.assertTrue(isfile(yaml_file)) # There should be no changes after the round trip @@ -64,15 +64,19 @@ class TestYamlProvider(TestCase): # A 2nd sync should still create everything plan = target.plan(zone) - self.assertEquals(14, len(filter(lambda c: isinstance(c, Create), + self.assertEquals(15, len(filter(lambda c: isinstance(c, Create), plan.changes))) with open(yaml_file) as fh: data = safe_load(fh.read()) + # '' has some of both + roots = sorted(data[''], key=lambda r: r['type']) + self.assertTrue('values' in roots[0]) # A + self.assertTrue('value' in roots[1]) # CAA + self.assertTrue('values' in roots[2]) # SSHFP + # these are stored as plural 'values' - for r in data['']: - self.assertTrue('values' in r) self.assertTrue('values' in data['mx']) self.assertTrue('values' in data['naptr']) self.assertTrue('values' in data['_srv._tcp']) From b4ead495f5069852e6925e64603caff649da279c Mon Sep 17 00:00:00 2001 From: Tim Hughes Date: Wed, 8 Nov 2017 14:41:48 +0000 Subject: [PATCH 16/24] adds an example of how to setup geodns to the docs --- docs/records.md | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/docs/records.md b/docs/records.md index d991311..82d9a37 100644 --- a/docs/records.md +++ b/docs/records.md @@ -26,6 +26,55 @@ GeoDNS is currently supported for `A` and `AAAA` records on the Dyn (via Traffic Configuring GeoDNS is complex and the details of the functionality vary widely from provider to provider. OctoDNS has an opinionated view of how GeoDNS should be set up and does its best to map that to each provider's offering in a way that will result in similar behavior. It may not fit your needs or use cases, in which case please open an issue for discussion. We expect this functionality to grow and evolve over time as it's more widely used. +The following is an example of GeoDNS with three entries NA-US-CA, NA-US-NY, OC-AU. Octodns creates another one labeled 'default' with the details for the actual A record, This default record is the failover record if the monitoring check fails. + +```yaml +--- +? '' +: type: TXT + value: v=spf1 -all +test: + geo: + NA-US-NY: + - 111.111.111.1 + NA-US-CA: + - 111.111.111.2 + OC-AU: + - 111.111.111.3 + EU: + - 111.111.111.4 + ttl: 300 + type: A + value: 111.111.111.5 +``` + + +The geo labels breakdown based on: + +1. + - 'AF': 14, # Continental Africa + - 'AN': 17, # Continental Antartica + - 'AS': 15, # Contentinal Asia + - 'EU': 13, # Contentinal Europe + - 'NA': 11, # Continental North America + - 'OC': 16, # Contentinal Austrailia/Oceania + - 'SA': 12, # Continental South America + +2. ISO Country Code https://en.wikipedia.org/wiki/ISO_3166-2 + +3. ISO Country Code Subdevision as per https://en.wikipedia.org/wiki/ISO_3166-2:US (change the code at the end for the country you are subdividing) * these may not always be supported depending on the provider. + +So the example is saying: + +- North America - United States - New York: gets served an "A" record of 111.111.111.1 +- North America - United States - California: gets served an "A" record of 111.111.111.2 +- Oceania - Australia: Gets served an "A" record of 111.111.111.3 +- Europe: gets an "A" record of 111.111.111.4 +- Everyone else gets an "A" record of 111.111.111.5 + + +Octodns will automatically set up a monitor and check for **https:///_dns** and check for a 200 response. + ## Config (`YamlProvider`) OctoDNS records and `YamlProvider`'s schema is essentially a 1:1 match. Properties on the objects will match keys in the config. From feec4a68215b5bc8f85b9e1d3f491ada08a84cae Mon Sep 17 00:00:00 2001 From: Adam Smith Date: Mon, 6 Nov 2017 22:32:46 -0800 Subject: [PATCH 17/24] Add DigitalOcean provider --- README.md | 1 + octodns/provider/digitalocean.py | 336 ++++++++++++++++++++ tests/fixtures/digitalocean-page-1.json | 177 +++++++++++ tests/fixtures/digitalocean-page-2.json | 89 ++++++ tests/test_octodns_provider_digitalocean.py | 241 ++++++++++++++ 5 files changed, 844 insertions(+) create mode 100644 octodns/provider/digitalocean.py create mode 100644 tests/fixtures/digitalocean-page-1.json create mode 100644 tests/fixtures/digitalocean-page-2.json create mode 100644 tests/test_octodns_provider_digitalocean.py diff --git a/README.md b/README.md index a910b5b..88223e6 100644 --- a/README.md +++ b/README.md @@ -151,6 +151,7 @@ The above command pulled the existing data out of Route53 and placed the results |--|--|--|--| | [AzureProvider](/octodns/provider/azuredns.py) | A, AAAA, CNAME, MX, NS, PTR, SRV, TXT | No | | | [CloudflareProvider](/octodns/provider/cloudflare.py) | A, AAAA, CAA, CNAME, MX, NS, SPF, TXT | No | CAA tags restricted | +| [DigitalOceanProvider](/octodns/provider/digitalocean.py) | A, AAAA, CAA, CNAME, MX, NS, TXT, SRV | No | CAA tags restricted | | [DnsimpleProvider](/octodns/provider/dnsimple.py) | All | No | CAA tags restricted | | [DynProvider](/octodns/provider/dyn.py) | All | Yes | | | [GoogleCloudProvider](/octodns/provider/googlecloud.py) | A, AAAA, CAA, CNAME, MX, NAPTR, NS, PTR, SPF, SRV, TXT | No | | diff --git a/octodns/provider/digitalocean.py b/octodns/provider/digitalocean.py new file mode 100644 index 0000000..55fa10b --- /dev/null +++ b/octodns/provider/digitalocean.py @@ -0,0 +1,336 @@ +# +# +# + +from __future__ import absolute_import, division, print_function, \ + unicode_literals + +from collections import defaultdict +from requests import Session +import logging + +from ..record import Record +from .base import BaseProvider + + +class DigitalOceanClientException(Exception): + pass + + +class DigitalOceanClientNotFound(DigitalOceanClientException): + + def __init__(self): + super(DigitalOceanClientNotFound, self).__init__('Not Found') + + +class DigitalOceanClientUnauthorized(DigitalOceanClientException): + + def __init__(self): + super(DigitalOceanClientUnauthorized, self).__init__('Unauthorized') + + +class DigitalOceanClient(object): + BASE = 'https://api.digitalocean.com/v2' + + def __init__(self, token): + sess = Session() + sess.headers.update({'Authorization': 'Bearer {}'.format(token)}) + self._sess = sess + + def _request(self, method, path, params=None, data=None): + url = '{}{}'.format(self.BASE, path) + resp = self._sess.request(method, url, params=params, json=data) + if resp.status_code == 401: + raise DigitalOceanClientUnauthorized() + if resp.status_code == 404: + raise DigitalOceanClientNotFound() + resp.raise_for_status() + return resp + + def domain(self, name): + path = '/domains/{}'.format(name) + return self._request('GET', path).json() + + def domain_create(self, name): + # Digitalocean requires an IP on zone creation + self._request('POST', '/domains', data={'name': name, + 'ip_address': '192.0.2.1'}) + + # After the zone is created, immeadiately delete the record + records = self.records(name) + for record in records: + if record['name'] == '' and record['type'] == 'A': + self.record_delete(name, record['id']) + + def records(self, zone_name): + path = '/domains/{}/records'.format(zone_name) + ret = [] + + page = 1 + while True: + data = self._request('GET', path, {'page': page}).json() + + ret += data['domain_records'] + links = data['links'] + + # https://developers.digitalocean.com/documentation/v2/#links + # pages exists if there is more than 1 page + # last doesn't exist if you're on the last page + try: + links['pages']['last'] + page += 1 + except KeyError: + break + + # change any apex record to empty string to match other provider output + for record in ret: + if record['name'] == '@': + record['name'] = '' + + return ret + + def record_create(self, zone_name, params): + path = '/domains/{}/records'.format(zone_name) + # change empty string to @, DigitalOcean uses @ for apex record names + if params['name'] == '': + params['name'] = '@' + self._request('POST', path, data=params) + + def record_delete(self, zone_name, record_id): + path = '/domains/{}/records/{}'.format(zone_name, record_id) + self._request('DELETE', path) + + +class DigitalOceanProvider(BaseProvider): + ''' + DigitalOcean DNS provider using API v2 + + digitalocean: + class: octodns.provider.digitalocean.DigitalOceanProvider + # Your DigitalOcean API token (required) + token: foo + ''' + SUPPORTS_GEO = False + SUPPORTS = set(('A', 'AAAA', 'CAA', 'CNAME', 'MX', 'NS', 'TXT', 'SRV')) + + def __init__(self, id, token, *args, **kwargs): + self.log = logging.getLogger('DigitalOceanProvider[{}]'.format(id)) + self.log.debug('__init__: id=%s, token=***', id) + super(DigitalOceanProvider, self).__init__(id, *args, **kwargs) + self._client = DigitalOceanClient(token) + + self._zone_records = {} + + def _data_for_multiple(self, _type, records): + return { + 'ttl': records[0]['ttl'], + 'type': _type, + 'values': [r['data'] for r in records] + } + + _data_for_A = _data_for_multiple + _data_for_AAAA = _data_for_multiple + + def _data_for_CAA(self, _type, records): + values = [] + for record in records: + values.append({ + 'flags': record['flags'], + 'tag': record['tag'], + 'value': record['data'], + }) + return { + 'ttl': records[0]['ttl'], + 'type': _type, + 'values': values + } + + def _data_for_CNAME(self, _type, records): + record = records[0] + return { + 'ttl': record['ttl'], + 'type': _type, + 'value': '{}.'.format(record['data']) + } + + def _data_for_MX(self, _type, records): + values = [] + for record in records: + values.append({ + 'preference': record['priority'], + 'exchange': '{}.'.format(record['data']) + }) + return { + 'ttl': records[0]['ttl'], + 'type': _type, + 'values': values + } + + def _data_for_NS(self, _type, records): + values = [] + for record in records: + data = '{}.'.format(record['data']) + values.append(data) + return { + 'ttl': records[0]['ttl'], + 'type': _type, + 'values': values, + } + + def _data_for_SRV(self, _type, records): + values = [] + for record in records: + values.append({ + 'port': record['port'], + 'priority': record['priority'], + 'target': '{}.'.format(record['data']), + 'weight': record['weight'] + }) + return { + 'type': _type, + 'ttl': records[0]['ttl'], + 'values': values + } + + def _data_for_TXT(self, _type, records): + values = [value['data'].replace(';', '\;') for value in records] + return { + 'ttl': records[0]['ttl'], + 'type': _type, + 'values': values + } + + def zone_records(self, zone): + if zone.name not in self._zone_records: + try: + self._zone_records[zone.name] = \ + self._client.records(zone.name[:-1]) + except DigitalOceanClientNotFound: + return [] + + return self._zone_records[zone.name] + + def populate(self, zone, target=False, lenient=False): + self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, + target, lenient) + + values = defaultdict(lambda: defaultdict(list)) + for record in self.zone_records(zone): + _type = record['type'] + values[record['name']][record['type']].append(record) + + before = len(zone.records) + for name, types in values.items(): + for _type, records in types.items(): + data_for = getattr(self, '_data_for_{}'.format(_type)) + record = Record.new(zone, name, data_for(_type, records), + source=self, lenient=lenient) + zone.add_record(record) + + self.log.info('populate: found %s records', + len(zone.records) - before) + + def _params_for_multiple(self, record): + for value in record.values: + yield { + 'data': value, + 'name': record.name, + 'ttl': record.ttl, + 'type': record._type + } + + _params_for_A = _params_for_multiple + _params_for_AAAA = _params_for_multiple + _params_for_NS = _params_for_multiple + + def _params_for_CAA(self, record): + for value in record.values: + yield { + 'data': '{}.'.format(value.value), + 'flags': value.flags, + 'name': record.name, + 'tag': value.tag, + 'ttl': record.ttl, + 'type': record._type + } + + def _params_for_single(self, record): + yield { + 'data': record.value, + 'name': record.name, + 'ttl': record.ttl, + 'type': record._type + } + + _params_for_CNAME = _params_for_single + + def _params_for_MX(self, record): + for value in record.values: + yield { + 'data': value.exchange, + 'name': record.name, + 'priority': value.preference, + 'ttl': record.ttl, + 'type': record._type + } + + def _params_for_SRV(self, record): + for value in record.values: + yield { + 'data': value.target, + 'name': record.name, + 'port': value.port, + 'priority': value.priority, + 'ttl': record.ttl, + 'type': record._type, + 'weight': value.weight + } + + def _params_for_TXT(self, record): + # DigitalOcean doesn't want things escaped in values so we + # have to strip them here and add them when going the other way + for value in record.values: + yield { + 'data': value.replace('\;', ';'), + 'name': record.name, + 'ttl': record.ttl, + 'type': record._type + } + + def _apply_Create(self, change): + new = change.new + params_for = getattr(self, '_params_for_{}'.format(new._type)) + for params in params_for(new): + self._client.record_create(new.zone.name[:-1], params) + + def _apply_Update(self, change): + self._apply_Delete(change) + self._apply_Create(change) + + def _apply_Delete(self, change): + existing = change.existing + zone = existing.zone + for record in self.zone_records(zone): + if existing.name == record['name'] and \ + existing._type == record['type']: + self._client.record_delete(zone.name[:-1], record['id']) + + def _apply(self, plan): + desired = plan.desired + changes = plan.changes + self.log.debug('_apply: zone=%s, len(changes)=%d', desired.name, + len(changes)) + + domain_name = desired.name[:-1] + try: + self._client.domain(domain_name) + except DigitalOceanClientNotFound: + self.log.debug('_apply: no matching zone, creating domain') + self._client.domain_create(domain_name) + + for change in changes: + class_name = change.__class__.__name__ + getattr(self, '_apply_{}'.format(class_name))(change) + + # Clear out the cache if any + self._zone_records.pop(desired.name, None) diff --git a/tests/fixtures/digitalocean-page-1.json b/tests/fixtures/digitalocean-page-1.json new file mode 100644 index 0000000..db231ba --- /dev/null +++ b/tests/fixtures/digitalocean-page-1.json @@ -0,0 +1,177 @@ +{ + "domain_records": [{ + "id": 11189874, + "type": "NS", + "name": "@", + "data": "ns1.digitalocean.com", + "priority": null, + "port": null, + "ttl": 3600, + "weight": null, + "flags": null, + "tag": null + }, { + "id": 11189875, + "type": "NS", + "name": "@", + "data": "ns2.digitalocean.com", + "priority": null, + "port": null, + "ttl": 3600, + "weight": null, + "flags": null, + "tag": null + }, { + "id": 11189876, + "type": "NS", + "name": "@", + "data": "ns3.digitalocean.com", + "priority": null, + "port": null, + "ttl": 3600, + "weight": null, + "flags": null, + "tag": null + }, { + "id": 11189877, + "type": "NS", + "name": "under", + "data": "ns1.unit.tests", + "priority": null, + "port": null, + "ttl": 3600, + "weight": null, + "flags": null, + "tag": null + }, { + "id": 11189878, + "type": "NS", + "name": "under", + "data": "ns2.unit.tests", + "priority": null, + "port": null, + "ttl": 3600, + "weight": null, + "flags": null, + "tag": null + }, { + "id": 11189879, + "type": "SRV", + "name": "_srv._tcp", + "data": "foo-1.unit.tests", + "priority": 10, + "port": 30, + "ttl": 600, + "weight": 20, + "flags": null, + "tag": null + }, { + "id": 11189880, + "type": "SRV", + "name": "_srv._tcp", + "data": "foo-2.unit.tests", + "priority": 12, + "port": 30, + "ttl": 600, + "weight": 20, + "flags": null, + "tag": null + }, { + "id": 11189881, + "type": "TXT", + "name": "txt", + "data": "Bah bah black sheep", + "priority": null, + "port": null, + "ttl": 600, + "weight": null, + "flags": null, + "tag": null + }, { + "id": 11189882, + "type": "TXT", + "name": "txt", + "data": "have you any wool.", + "priority": null, + "port": null, + "ttl": 600, + "weight": null, + "flags": null, + "tag": null + }, { + "id": 11189883, + "type": "A", + "name": "@", + "data": "1.2.3.4", + "priority": null, + "port": null, + "ttl": 300, + "weight": null, + "flags": null, + "tag": null + }, { + "id": 11189884, + "type": "A", + "name": "@", + "data": "1.2.3.5", + "priority": null, + "port": null, + "ttl": 300, + "weight": null, + "flags": null, + "tag": null + }, { + "id": 11189885, + "type": "A", + "name": "www", + "data": "2.2.3.6", + "priority": null, + "port": null, + "ttl": 300, + "weight": null, + "flags": null, + "tag": null + }, { + "id": 11189886, + "type": "MX", + "name": "mx", + "data": "smtp-4.unit.tests", + "priority": 10, + "port": null, + "ttl": 300, + "weight": null, + "flags": null, + "tag": null + }, { + "id": 11189887, + "type": "MX", + "name": "mx", + "data": "smtp-2.unit.tests", + "priority": 20, + "port": null, + "ttl": 300, + "weight": null, + "flags": null, + "tag": null + }, { + "id": 11189888, + "type": "MX", + "name": "mx", + "data": "smtp-3.unit.tests", + "priority": 30, + "port": null, + "ttl": 300, + "weight": null, + "flags": null, + "tag": null + }], + "links": { + "pages": { + "last": "https://api.digitalocean.com/v2/domains/unit.tests/records?page=2", + "next": "https://api.digitalocean.com/v2/domains/unit.tests/records?page=2" + } + }, + "meta": { + "total": 21 + } +} \ No newline at end of file diff --git a/tests/fixtures/digitalocean-page-2.json b/tests/fixtures/digitalocean-page-2.json new file mode 100644 index 0000000..8b989ae --- /dev/null +++ b/tests/fixtures/digitalocean-page-2.json @@ -0,0 +1,89 @@ +{ + "domain_records": [{ + "id": 11189889, + "type": "MX", + "name": "mx", + "data": "smtp-1.unit.tests", + "priority": 40, + "port": null, + "ttl": 300, + "weight": null, + "flags": null, + "tag": null + }, { + "id": 11189890, + "type": "AAAA", + "name": "aaaa", + "data": "2601:644:500:e210:62f8:1dff:feb8:947a", + "priority": null, + "port": null, + "ttl": 600, + "weight": null, + "flags": null, + "tag": null + }, { + "id": 11189891, + "type": "CNAME", + "name": "cname", + "data": "unit.tests", + "priority": null, + "port": null, + "ttl": 300, + "weight": null, + "flags": null, + "tag": null + }, { + "id": 11189892, + "type": "A", + "name": "www.sub", + "data": "2.2.3.6", + "priority": null, + "port": null, + "ttl": 300, + "weight": null, + "flags": null, + "tag": null + }, { + "id": 11189893, + "type": "TXT", + "name": "txt", + "data": "v=DKIM1;k=rsa;s=email;h=sha256;p=A/kinda+of/long/string+with+numb3rs", + "priority": null, + "port": null, + "ttl": 600, + "weight": null, + "flags": null, + "tag": null + }, { + "id": 11189894, + "type": "CAA", + "name": "@", + "data": "ca.unit.tests", + "priority": null, + "port": null, + "ttl": 3600, + "weight": null, + "flags": 0, + "tag": "issue" + }, { + "id": 11189895, + "type": "CNAME", + "name": "included", + "data": "unit.tests", + "priority": null, + "port": null, + "ttl": 3600, + "weight": null, + "flags": null, + "tag": null + }], + "links": { + "pages": { + "first": "https://api.digitalocean.com/v2/domains/unit.tests/records?page=1", + "prev": "https://api.digitalocean.com/v2/domains/unit.tests/records?page=1" + } + }, + "meta": { + "total": 21 + } +} \ No newline at end of file diff --git a/tests/test_octodns_provider_digitalocean.py b/tests/test_octodns_provider_digitalocean.py new file mode 100644 index 0000000..5fb47c5 --- /dev/null +++ b/tests/test_octodns_provider_digitalocean.py @@ -0,0 +1,241 @@ +# +# +# + + +from __future__ import absolute_import, division, print_function, \ + unicode_literals + +from mock import Mock, call +from os.path import dirname, join +from requests import HTTPError +from requests_mock import ANY, mock as requests_mock +from unittest import TestCase + +from octodns.record import Record +from octodns.provider.digitalocean import DigitalOceanClientNotFound, \ + DigitalOceanProvider +from octodns.provider.yaml import YamlProvider +from octodns.zone import Zone + + +class TestDigitalOceanProvider(TestCase): + expected = Zone('unit.tests.', []) + source = YamlProvider('test', join(dirname(__file__), 'config')) + source.populate(expected) + + # Our test suite differs a bit, add our NS and remove the simple one + expected.add_record(Record.new(expected, 'under', { + 'ttl': 3600, + 'type': 'NS', + 'values': [ + 'ns1.unit.tests.', + 'ns2.unit.tests.', + ] + })) + for record in list(expected.records): + if record.name == 'sub' and record._type == 'NS': + expected._remove_record(record) + break + + def test_populate(self): + provider = DigitalOceanProvider('test', 'token') + + # Bad auth + with requests_mock() as mock: + mock.get(ANY, status_code=401, + text='{"id":"unauthorized",' + '"message":"Unable to authenticate you."}') + + with self.assertRaises(Exception) as ctx: + zone = Zone('unit.tests.', []) + provider.populate(zone) + self.assertEquals('Unauthorized', ctx.exception.message) + + # General error + with requests_mock() as mock: + mock.get(ANY, status_code=502, text='Things caught fire') + + with self.assertRaises(HTTPError) as ctx: + zone = Zone('unit.tests.', []) + provider.populate(zone) + self.assertEquals(502, ctx.exception.response.status_code) + + # Non-existant zone doesn't populate anything + with requests_mock() as mock: + mock.get(ANY, status_code=404, + text='{"id":"not_found","message":"The resource you ' + 'were accessing could not be found."}') + + zone = Zone('unit.tests.', []) + provider.populate(zone) + self.assertEquals(set(), zone.records) + + # No diffs == no changes + with requests_mock() as mock: + base = 'https://api.digitalocean.com/v2/domains/unit.tests/' \ + 'records?page=' + with open('tests/fixtures/digitalocean-page-1.json') as fh: + mock.get('{}{}'.format(base, 1), text=fh.read()) + with open('tests/fixtures/digitalocean-page-2.json') as fh: + mock.get('{}{}'.format(base, 2), text=fh.read()) + + zone = Zone('unit.tests.', []) + provider.populate(zone) + self.assertEquals(12, len(zone.records)) + changes = self.expected.changes(zone, provider) + self.assertEquals(0, len(changes)) + + # 2nd populate makes no network calls/all from cache + again = Zone('unit.tests.', []) + provider.populate(again) + self.assertEquals(12, len(again.records)) + + # bust the cache + del provider._zone_records[zone.name] + + def test_apply(self): + provider = DigitalOceanProvider('test', 'token') + + resp = Mock() + resp.json = Mock() + provider._client._request = Mock(return_value=resp) + + domain_after_creation = { + "domain_records": [{ + "id": 11189874, + "type": "NS", + "name": "@", + "data": "ns1.digitalocean.com", + "priority": None, + "port": None, + "ttl": 3600, + "weight": None, + "flags": None, + "tag": None + }, { + "id": 11189875, + "type": "NS", + "name": "@", + "data": "ns2.digitalocean.com", + "priority": None, + "port": None, + "ttl": 3600, + "weight": None, + "flags": None, + "tag": None + }, { + "id": 11189876, + "type": "NS", + "name": "@", + "data": "ns3.digitalocean.com", + "priority": None, + "port": None, + "ttl": 3600, + "weight": None, + "flags": None, + "tag": None + }, { + "id": 11189877, + "type": "A", + "name": "@", + "data": "192.0.2.1", + "priority": None, + "port": None, + "ttl": 3600, + "weight": None, + "flags": None, + "tag": None + }], + "links": {}, + "meta": { + "total": 4 + } + } + + # non-existant domain, create everything + resp.json.side_effect = [ + DigitalOceanClientNotFound, # no zone in populate + DigitalOceanClientNotFound, # no domain during apply + domain_after_creation + ] + plan = provider.plan(self.expected) + + # No root NS, no ignored, no excluded, no unsupported + n = len(self.expected.records) - 7 + self.assertEquals(n, len(plan.changes)) + self.assertEquals(n, provider.apply(plan)) + + provider._client._request.assert_has_calls([ + # created the domain + call('POST', '/domains', data={'ip_address': '192.0.2.1', + 'name': 'unit.tests'}), + # get all records in newly created zone + call('GET', '/domains/unit.tests/records', {'page': 1}), + # delete the initial A record + call('DELETE', '/domains/unit.tests/records/11189877'), + # created at least one of the record with expected data + call('POST', '/domains/unit.tests/records', data={ + 'name': '_srv._tcp', + 'weight': 20, + 'data': 'foo-1.unit.tests.', + 'priority': 10, + 'ttl': 600, + 'type': 'SRV', + 'port': 30 + }), + ]) + self.assertEquals(24, provider._client._request.call_count) + + provider._client._request.reset_mock() + + # delete 1 and update 1 + provider._client.records = Mock(return_value=[ + { + 'id': 11189897, + 'name': 'www', + 'data': '1.2.3.4', + 'ttl': 300, + 'type': 'A', + }, + { + 'id': 11189898, + 'name': 'www', + 'data': '2.2.3.4', + 'ttl': 300, + 'type': 'A', + }, + { + 'id': 11189899, + 'name': 'ttl', + 'data': '3.2.3.4', + 'ttl': 600, + 'type': 'A', + } + ]) + + # Domain exists, we don't care about return + resp.json.side_effect = ['{}'] + + wanted = Zone('unit.tests.', []) + wanted.add_record(Record.new(wanted, 'ttl', { + 'ttl': 300, + 'type': 'A', + 'value': '3.2.3.4' + })) + + plan = provider.plan(wanted) + self.assertEquals(2, len(plan.changes)) + self.assertEquals(2, provider.apply(plan)) + # recreate for update, and delete for the 2 parts of the other + provider._client._request.assert_has_calls([ + call('POST', '/domains/unit.tests/records', data={ + 'data': '3.2.3.4', + 'type': 'A', + 'name': 'ttl', + 'ttl': 300 + }), + call('DELETE', '/domains/unit.tests/records/11189899'), + call('DELETE', '/domains/unit.tests/records/11189897'), + call('DELETE', '/domains/unit.tests/records/11189898') + ], any_order=True) From 75cfc4fb76a43c38db55cd9eeb511aa7e97dcb2e Mon Sep 17 00:00:00 2001 From: Adam Smith Date: Mon, 13 Nov 2017 00:19:05 -0800 Subject: [PATCH 18/24] remove default config file for octodns-validate --- octodns/cmds/validate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/octodns/cmds/validate.py b/octodns/cmds/validate.py index 85c3018..6711ec9 100755 --- a/octodns/cmds/validate.py +++ b/octodns/cmds/validate.py @@ -15,7 +15,7 @@ from octodns.manager import Manager def main(): parser = ArgumentParser(description=__doc__.split('\n')[1]) - parser.add_argument('--config-file', default='./config/production.yaml', + parser.add_argument('--config-file', required=True, help='The Manager configuration file to use') args = parser.parse_args(WARN) From 0dfcc6f6f21706c8a5a8572d616bb27e4c91c1d9 Mon Sep 17 00:00:00 2001 From: Steve Coursen Date: Mon, 13 Nov 2017 09:37:03 -0500 Subject: [PATCH 19/24] Send appropriate meta along for A and AAAA records --- octodns/provider/ns1.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 2d3af9a..1a842fd 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -22,7 +22,7 @@ class Ns1Provider(BaseProvider): class: octodns.provider.ns1.Ns1Provider api_key: env/NS1_API_KEY ''' - SUPPORTS_GEO = False + SUPPORTS_GEO = True SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'MX', 'NAPTR', 'NS', 'PTR', 'SPF', 'SRV', 'TXT')) @@ -164,7 +164,16 @@ class Ns1Provider(BaseProvider): len(zone.records) - before) def _params_for_A(self, record): - return {'answers': record.values, 'ttl': record.ttl} + params = {'answers': record.values, 'ttl': record.ttl} + if record.geo: + # purposefully set non-geo answers to have an empty meta, + # so that we know we did this on purpose if/when troubleshooting + params['answers'] = [{"answer": x, "meta":{}} for x in record.values] + for iso_region, target in record.geo.items(): + params['answers'].append({'answer': target.values, + 'meta': {'iso_region_code': [iso_region]}, + }) + return params _params_for_AAAA = _params_for_A _params_for_NS = _params_for_A From 0cc20afabd64f6ab4c2d5c8cef47ed291a06ddee Mon Sep 17 00:00:00 2001 From: Steve Coursen Date: Mon, 13 Nov 2017 13:57:43 -0500 Subject: [PATCH 20/24] pep8 cleanup --- octodns/provider/ns1.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 1a842fd..d4f0572 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -168,11 +168,15 @@ class Ns1Provider(BaseProvider): if record.geo: # purposefully set non-geo answers to have an empty meta, # so that we know we did this on purpose if/when troubleshooting - params['answers'] = [{"answer": x, "meta":{}} for x in record.values] + params['answers'] = [{"answer": x, "meta": {}} \ + for x in record.values] for iso_region, target in record.geo.items(): - params['answers'].append({'answer': target.values, - 'meta': {'iso_region_code': [iso_region]}, - }) + params['answers'].append( + { + 'answer': target.values, + 'meta': {'iso_region_code': [iso_region]}, + }, + ) return params _params_for_AAAA = _params_for_A From 2cc17ffc7ae6aa551670b9b328da334a3b1a499d Mon Sep 17 00:00:00 2001 From: Steve Coursen Date: Mon, 13 Nov 2017 13:58:12 -0500 Subject: [PATCH 21/24] pep8 cleanup --- octodns/provider/ns1.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index d4f0572..1397c49 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -168,7 +168,7 @@ class Ns1Provider(BaseProvider): if record.geo: # purposefully set non-geo answers to have an empty meta, # so that we know we did this on purpose if/when troubleshooting - params['answers'] = [{"answer": x, "meta": {}} \ + params['answers'] = [{"answer": x, "meta": {}} for x in record.values] for iso_region, target in record.geo.items(): params['answers'].append( From ce5ecc52e3edc282427ba4e6655c826be5e0886f Mon Sep 17 00:00:00 2001 From: Steve Coursen Date: Tue, 14 Nov 2017 13:14:03 -0500 Subject: [PATCH 22/24] fix broken test by updating the actual format of the answers --- octodns/provider/ns1.py | 2 +- tests/test_octodns_provider_ns1.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 1397c49..008c665 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -165,7 +165,7 @@ class Ns1Provider(BaseProvider): def _params_for_A(self, record): params = {'answers': record.values, 'ttl': record.ttl} - if record.geo: + if hasattr(record, 'geo'): # purposefully set non-geo answers to have an empty meta, # so that we know we did this on purpose if/when troubleshooting params['answers'] = [{"answer": x, "meta": {}} diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index d4f4080..4436304 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -30,11 +30,13 @@ class TestNs1Provider(TestCase): 'ttl': 32, 'type': 'A', 'value': '1.2.3.4', + 'meta': {}, })) expected.add(Record.new(zone, 'foo', { 'ttl': 33, 'type': 'A', 'values': ['1.2.3.4', '1.2.3.5'], + 'meta': {}, })) expected.add(Record.new(zone, 'cname', { 'ttl': 34, @@ -289,7 +291,7 @@ class TestNs1Provider(TestCase): call('delete-me', u'A'), ]) mock_record.assert_has_calls([ - call.update(answers=[u'1.2.3.4'], ttl=32), + call.update(answers=[{'answer': u'1.2.3.4', 'meta': {}}], ttl=32), call.delete() ]) From 61a86810ee84bd6c5c1af66dcf73ed19634b9540 Mon Sep 17 00:00:00 2001 From: Steve Coursen Date: Thu, 28 Dec 2017 16:01:22 -0500 Subject: [PATCH 23/24] add geo support for ns1 --- octodns/provider/ns1.py | 55 +++++++++++++++++++++++++----- tests/test_octodns_provider_ns1.py | 38 ++++++++++++++++++--- 2 files changed, 79 insertions(+), 14 deletions(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 008c665..e8ba8cd 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -6,11 +6,13 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals from logging import getLogger -from nsone import NSONE +from itertools import chain +from nsone import NSONE, Config from nsone.rest.errors import RateLimitException, ResourceException +from incf.countryutils import transformations from time import sleep -from ..record import Record +from ..record import _GeoMixin, Record from .base import BaseProvider @@ -35,11 +37,38 @@ class Ns1Provider(BaseProvider): self._client = NSONE(apiKey=api_key) def _data_for_A(self, _type, record): - return { + # record meta (which would include geo information is only + # returned when getting a record's detail, not from zone detail + geo = {} + data = { 'ttl': record['ttl'], 'type': _type, - 'values': record['short_answers'], } + values, codes = [], [] + if 'answers' not in record: + values = record['short_answers'] + for answer in record.get('answers', []): + meta = answer.get('meta', {}) + if meta: + country = meta.get('country', []) + us_state = meta.get('us_state', []) + ca_province = meta.get('ca_province', []) + for cntry in country: + cn = transformations.cc_to_cn(cntry) + con = transformations.cn_to_ctca2(cn) + geo['{}-{}'.format(con, cntry)] = answer['answer'] + for state in us_state: + geo['NA-US-{}'.format(state)] = answer['answer'] + for province in ca_province: + geo['NA-CA-{}'.format(state)] = answer['answer'] + for code in meta.get('iso_region_code', []): + geo[code] = answer['answer'] + else: + values.extend(answer['answer']) + codes.append([]) + data['values'] = values + data['geo'] = geo + return data _data_for_AAAA = _data_for_A @@ -146,20 +175,25 @@ class Ns1Provider(BaseProvider): try: nsone_zone = self._client.loadZone(zone.name[:-1]) records = nsone_zone.data['records'] + geo_records = nsone_zone.search(has_geo=True) except ResourceException as e: if e.message != self.ZONE_NOT_FOUND_MESSAGE: raise records = [] + geo_records = [] before = len(zone.records) - for record in records: + # geo information isn't returned from the main endpoint, so we need + # to query for all records with geo information + zone_hash = {} + for record in chain(records, geo_records): _type = record['type'] data_for = getattr(self, '_data_for_{}'.format(_type)) name = zone.hostname_from_fqdn(record['domain']) record = Record.new(zone, name, data_for(_type, record), source=self, lenient=lenient) - zone.add_record(record) - + zone_hash[(_type, name)] = record + [zone.add_record(r) for r in zone_hash.values()] self.log.info('populate: found %s records', len(zone.records) - before) @@ -168,15 +202,18 @@ class Ns1Provider(BaseProvider): if hasattr(record, 'geo'): # purposefully set non-geo answers to have an empty meta, # so that we know we did this on purpose if/when troubleshooting - params['answers'] = [{"answer": x, "meta": {}} + params['answers'] = [{"answer": [x], "meta": {}} \ for x in record.values] for iso_region, target in record.geo.items(): + key = 'iso_region_code' + value = iso_region params['answers'].append( { 'answer': target.values, - 'meta': {'iso_region_code': [iso_region]}, + 'meta': {key: [value]}, }, ) + self.log.info("params for A: %s", params) return params _params_for_AAAA = _params_for_A diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index 4436304..c8ff222 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -38,6 +38,13 @@ class TestNs1Provider(TestCase): 'values': ['1.2.3.4', '1.2.3.5'], 'meta': {}, })) + expected.add(Record.new(zone, 'geo', { + 'ttl': 34, + 'type': 'A', + 'values': ['101.102.103.104', '101.102.103.105'], + 'geo': {'NA-US-NY': ['201.202.203.204']}, + 'meta': {}, + })) expected.add(Record.new(zone, 'cname', { 'ttl': 34, 'type': 'CNAME', @@ -118,6 +125,11 @@ class TestNs1Provider(TestCase): 'ttl': 33, 'short_answers': ['1.2.3.4', '1.2.3.5'], 'domain': 'foo.unit.tests.', + }, { + 'type': 'A', + 'ttl': 34, + 'short_answers': ['101.102.103.104', '101.102.103.105'], + 'domain': 'geo.unit.tests.', }, { 'type': 'CNAME', 'ttl': 34, @@ -192,6 +204,9 @@ class TestNs1Provider(TestCase): load_mock.reset_mock() nsone_zone = DummyZone([]) load_mock.side_effect = [nsone_zone] + zone_search = Mock() + zone_search.return_value = [] + nsone_zone.search = zone_search zone = Zone('unit.tests.', []) provider.populate(zone) self.assertEquals(set(), zone.records) @@ -201,6 +216,9 @@ class TestNs1Provider(TestCase): load_mock.reset_mock() nsone_zone = DummyZone(self.nsone_records) load_mock.side_effect = [nsone_zone] + zone_search = Mock() + zone_search.return_value = [] + nsone_zone.search = zone_search zone = Zone('unit.tests.', []) provider.populate(zone) self.assertEquals(self.expected, zone.records) @@ -266,11 +284,14 @@ class TestNs1Provider(TestCase): }]) nsone_zone.data['records'][0]['short_answers'][0] = '2.2.2.2' nsone_zone.loadRecord = Mock() + zone_search = Mock() + zone_search.return_value = [] + nsone_zone.search = zone_search load_mock.side_effect = [nsone_zone, nsone_zone] plan = provider.plan(desired) - self.assertEquals(2, len(plan.changes)) + self.assertEquals(3, len(plan.changes)) self.assertIsInstance(plan.changes[0], Update) - self.assertIsInstance(plan.changes[1], Delete) + self.assertIsInstance(plan.changes[2], Delete) # ugh, we need a mock record that can be returned from loadRecord for # the update and delete targets, we can add our side effects to that to # trigger rate limit handling @@ -278,23 +299,30 @@ class TestNs1Provider(TestCase): mock_record.update.side_effect = [ RateLimitException('one', period=0), None, + None, ] mock_record.delete.side_effect = [ RateLimitException('two', period=0), None, + None, ] - nsone_zone.loadRecord.side_effect = [mock_record, mock_record] + nsone_zone.loadRecord.side_effect = [mock_record, mock_record, mock_record] got_n = provider.apply(plan) - self.assertEquals(2, got_n) + self.assertEquals(3, got_n) nsone_zone.loadRecord.assert_has_calls([ call('unit.tests', u'A'), + call('geo', u'A'), call('delete-me', u'A'), ]) mock_record.assert_has_calls([ - call.update(answers=[{'answer': u'1.2.3.4', 'meta': {}}], ttl=32), + call.update(answers=[{'answer': [u'1.2.3.4'], 'meta': {}}], ttl=32), + call.update(answers=[{u'answer': [u'1.2.3.4'], u'meta': {}}], ttl=32), + call.update(answers=[{u'answer': [u'101.102.103.104'], u'meta': {}}, {u'answer': [u'101.102.103.105'], u'meta': {}}, {u'answer': [u'201.202.203.204'], u'meta': {u'iso_region_code': [u'NA-US-NY']}}], ttl=34), + call.delete(), call.delete() ]) + def test_escaping(self): provider = Ns1Provider('test', 'api-key') From 481bbe10f6bd51eae8ccd4f88366dd18cf4d5637 Mon Sep 17 00:00:00 2001 From: Steve Coursen Date: Thu, 28 Dec 2017 16:01:56 -0500 Subject: [PATCH 24/24] add geo support for ns1 --- octodns/provider/ns1.py | 6 +++--- tests/test_octodns_provider_ns1.py | 24 ++++++++++++++++++------ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index e8ba8cd..6b0fe7e 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -7,12 +7,12 @@ from __future__ import absolute_import, division, print_function, \ from logging import getLogger from itertools import chain -from nsone import NSONE, Config +from nsone import NSONE from nsone.rest.errors import RateLimitException, ResourceException from incf.countryutils import transformations from time import sleep -from ..record import _GeoMixin, Record +from ..record import Record from .base import BaseProvider @@ -202,7 +202,7 @@ class Ns1Provider(BaseProvider): if hasattr(record, 'geo'): # purposefully set non-geo answers to have an empty meta, # so that we know we did this on purpose if/when troubleshooting - params['answers'] = [{"answer": [x], "meta": {}} \ + params['answers'] = [{"answer": [x], "meta": {}} for x in record.values] for iso_region, target in record.geo.items(): key = 'iso_region_code' diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index c8ff222..10ae5d3 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -306,7 +306,8 @@ class TestNs1Provider(TestCase): None, None, ] - nsone_zone.loadRecord.side_effect = [mock_record, mock_record, mock_record] + nsone_zone.loadRecord.side_effect = [mock_record, mock_record, + mock_record] got_n = provider.apply(plan) self.assertEquals(3, got_n) nsone_zone.loadRecord.assert_has_calls([ @@ -315,17 +316,28 @@ class TestNs1Provider(TestCase): call('delete-me', u'A'), ]) mock_record.assert_has_calls([ - call.update(answers=[{'answer': [u'1.2.3.4'], 'meta': {}}], ttl=32), - call.update(answers=[{u'answer': [u'1.2.3.4'], u'meta': {}}], ttl=32), - call.update(answers=[{u'answer': [u'101.102.103.104'], u'meta': {}}, {u'answer': [u'101.102.103.105'], u'meta': {}}, {u'answer': [u'201.202.203.204'], u'meta': {u'iso_region_code': [u'NA-US-NY']}}], ttl=34), + call.update(answers=[{'answer': [u'1.2.3.4'], 'meta': {}}], + ttl=32), + call.update(answers=[{u'answer': [u'1.2.3.4'], u'meta': {}}], + ttl=32), + call.update( + answers=[ + {u'answer': [u'101.102.103.104'], u'meta': {}}, + {u'answer': [u'101.102.103.105'], u'meta': {}}, + { + u'answer': [u'201.202.203.204'], + u'meta': { + u'iso_region_code': [u'NA-US-NY'] + }, + }, + ], + ttl=34), call.delete(), call.delete() ]) - def test_escaping(self): provider = Ns1Provider('test', 'api-key') - record = { 'ttl': 31, 'short_answers': ['foo; bar baz; blip']