From 827b44daba6296d05c50f88d181dc05999817256 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 8 May 2020 07:50:02 -0700 Subject: [PATCH 01/21] Add TCP health check support to Record --- octodns/record/__init__.py | 12 +++++++++--- tests/test_octodns_record.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index 384c0a2..39bae2a 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -137,7 +137,7 @@ class Record(EqualityTupleMixin): reasons.append('missing ttl') try: if data['octodns']['healthcheck']['protocol'] \ - not in ('HTTP', 'HTTPS'): + not in ('HTTP', 'HTTPS', 'TCP'): reasons.append('invalid healthcheck protocol') except KeyError: pass @@ -181,15 +181,21 @@ class Record(EqualityTupleMixin): @property def healthcheck_host(self): + healthcheck = self._octodns.get('healthcheck', {}) + if healthcheck.get('protocol', None) == 'TCP': + return None try: - return self._octodns['healthcheck']['host'] + return healthcheck['host'] except KeyError: return self.fqdn[:-1] @property def healthcheck_path(self): + healthcheck = self._octodns.get('healthcheck', {}) + if healthcheck.get('protocol', None) == 'TCP': + return None try: - return self._octodns['healthcheck']['path'] + return healthcheck['path'] except KeyError: return '/_dns' diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index f76f593..b6dc4e9 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -886,6 +886,40 @@ class TestRecord(TestCase): self.assertEquals('HTTPS', new.healthcheck_protocol) self.assertEquals(443, new.healthcheck_port) + def test_healthcheck_tcp(self): + new = Record.new(self.zone, 'a', { + 'ttl': 44, + 'type': 'A', + 'value': '1.2.3.4', + 'octodns': { + 'healthcheck': { + 'path': '/ignored', + 'host': 'completely.ignored', + 'protocol': 'TCP', + 'port': 8080, + } + } + }) + self.assertIsNone(new.healthcheck_path) + self.assertIsNone(new.healthcheck_host) + self.assertEquals('TCP', new.healthcheck_protocol) + self.assertEquals(8080, new.healthcheck_port) + + new = Record.new(self.zone, 'a', { + 'ttl': 44, + 'type': 'A', + 'value': '1.2.3.4', + 'octodns': { + 'healthcheck': { + 'protocol': 'TCP', + } + } + }) + self.assertIsNone(new.healthcheck_path) + self.assertIsNone(new.healthcheck_host) + self.assertEquals('TCP', new.healthcheck_protocol) + self.assertEquals(443, new.healthcheck_port) + def test_inored(self): new = Record.new(self.zone, 'txt', { 'ttl': 44, From 27fd6590894f4d60aafd9d65623ee1a99b1d0734 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 8 May 2020 07:57:56 -0700 Subject: [PATCH 02/21] NS1 support for TCP healthchecks --- octodns/provider/ns1.py | 24 +++++++++++++++--------- tests/test_octodns_provider_ns1.py | 7 +++++++ 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index fb0a78f..e99dc24 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -856,18 +856,13 @@ class Ns1Provider(BaseProvider): host = record.fqdn[:-1] _type = record._type - request = r'GET {path} HTTP/1.0\r\nHost: {host}\r\n' \ - r'User-agent: NS1\r\n\r\n'.format(path=record.healthcheck_path, - host=record.healthcheck_host) - - return { + ret = { 'active': True, 'config': { 'connect_timeout': 2000, 'host': value, 'port': record.healthcheck_port, 'response_timeout': 10000, - 'send': request, 'ssl': record.healthcheck_protocol == 'HTTPS', }, 'frequency': 60, @@ -881,12 +876,23 @@ class Ns1Provider(BaseProvider): 'rapid_recheck': False, 'region_scope': 'fixed', 'regions': self.monitor_regions, - 'rules': [{ + } + + if record.healthcheck_protocol != 'TCP': + # IF it's HTTP we need to send the request string + path = record.healthcheck_path + host = record.healthcheck_host + request = r'GET {path} HTTP/1.0\r\nHost: {host}\r\n' \ + r'User-agent: NS1\r\n\r\n'.format(path=path, host=host) + ret['config']['send'] = request + # We'll also expect a HTTP response + ret['rules'] = [{ 'comparison': 'contains', 'key': 'output', 'value': '200 OK', - }], - } + }] + + return ret def _monitor_is_match(self, expected, have): # Make sure what we have matches what's in expected exactly. Anything diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index 8a0dffb..2755580 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -717,6 +717,13 @@ class TestNs1ProviderDynamic(TestCase): monitor = provider._monitor_gen(self.record, value) self.assertTrue(monitor['config']['ssl']) + self.record._octodns['healthcheck']['protocol'] = 'TCP' + monitor = provider._monitor_gen(self.record, value) + # No http send done + self.assertFalse('send' in monitor['config']) + # No http response expected + self.assertFalse('rules' in monitor) + def test_monitor_is_match(self): provider = Ns1Provider('test', 'api-key') From b9575ae48439257a187d920790b622829dabd6b3 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 8 May 2020 08:13:27 -0700 Subject: [PATCH 03/21] TCP healthcheck support for Route53 --- octodns/provider/route53.py | 12 +++++++---- tests/test_octodns_provider_route53.py | 29 ++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/octodns/provider/route53.py b/octodns/provider/route53.py index 3c7b0ed..8e9ee4f 100644 --- a/octodns/provider/route53.py +++ b/octodns/provider/route53.py @@ -1046,8 +1046,11 @@ class Route53Provider(BaseProvider): # No value so give this a None to match value's config_ip_address = None - return host == config['FullyQualifiedDomainName'] and \ - path == config['ResourcePath'] and protocol == config['Type'] \ + fully_qualified_domain_name = config.get('FullyQualifiedDomainName', + None) + resource_path = config.get('ResourcePath', None) + return host == fully_qualified_domain_name and \ + path == resource_path and protocol == config['Type'] \ and port == config['Port'] and \ measure_latency == config['MeasureLatency'] and \ value == config_ip_address @@ -1103,13 +1106,14 @@ class Route53Provider(BaseProvider): config = { 'EnableSNI': healthcheck_protocol == 'HTTPS', 'FailureThreshold': 6, - 'FullyQualifiedDomainName': healthcheck_host, 'MeasureLatency': healthcheck_latency, 'Port': healthcheck_port, 'RequestInterval': 10, - 'ResourcePath': healthcheck_path, 'Type': healthcheck_protocol, } + if healthcheck_protocol != 'TCP': + config['FullyQualifiedDomainName'] = healthcheck_host + config['ResourcePath'] = healthcheck_path if value: config['IPAddress'] = value diff --git a/tests/test_octodns_provider_route53.py b/tests/test_octodns_provider_route53.py index 385b82a..543a506 100644 --- a/tests/test_octodns_provider_route53.py +++ b/tests/test_octodns_provider_route53.py @@ -1213,6 +1213,35 @@ class TestRoute53Provider(TestCase): self.assertEquals('42', id) stubber.assert_no_pending_responses() + # TCP health check + + health_check_config = { + 'EnableSNI': False, + 'FailureThreshold': 6, + 'MeasureLatency': True, + 'Port': 8080, + 'RequestInterval': 10, + 'Type': 'TCP' + } + stubber.add_response('create_health_check', { + 'HealthCheck': { + 'Id': '42', + 'CallerReference': self.caller_ref, + 'HealthCheckConfig': health_check_config, + 'HealthCheckVersion': 1, + }, + 'Location': 'http://url', + }, { + 'CallerReference': ANY, + 'HealthCheckConfig': health_check_config, + }) + stubber.add_response('change_tags_for_resource', {}) + + record._octodns['healthcheck']['protocol'] = 'TCP' + id = provider.get_health_check_id(record, 'target-1.unit.tests.', True) + self.assertEquals('42', id) + stubber.assert_no_pending_responses() + def test_health_check_measure_latency(self): provider, stubber = self._get_stubbed_provider() record_true = Record.new(self.expected, 'a', { From 8648fed1904f5c18acd3c1f866ac85d6211dccc6 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 8 May 2020 08:14:28 -0700 Subject: [PATCH 04/21] Fix Dyn python3 error with dict_values that needed a list --- octodns/provider/dyn.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/octodns/provider/dyn.py b/octodns/provider/dyn.py index f4f8e53..f7a15d2 100644 --- a/octodns/provider/dyn.py +++ b/octodns/provider/dyn.py @@ -1141,7 +1141,7 @@ class DynProvider(BaseProvider): pools[rpid] = get_response_pool(rpid, td) # now that we have full objects for the complete set of existing pools, # a list will be more useful - pools = pools.values() + pools = list(pools.values()) # Rulesets From 55f7de21ab87069196e94c4332e51d547f6e6683 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 8 May 2020 08:15:12 -0700 Subject: [PATCH 05/21] Docs and changelog for TCP health check support --- CHANGELOG.md | 4 ++++ docs/dynamic_records.md | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 27b6a80..a0a92a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## v0.9.11 - 2020-??-?? - ??????????????? + +* Added support for TCP health checking to dynamic records + ## v0.9.10 - 2020-04-20 - Dynamic NS1 and lots of misc * Added support for dynamic records to Ns1Provider, updated client and rate diff --git a/docs/dynamic_records.md b/docs/dynamic_records.md index 8a7cd09..706173a 100644 --- a/docs/dynamic_records.md +++ b/docs/dynamic_records.md @@ -103,7 +103,7 @@ test: | host | FQDN for host header and SNI | - | | path | path to check | _dns | | port | port to check | 443 | -| protocol | HTTP/HTTPS | HTTPS | +| protocol | HTTP/HTTPS/TCP | HTTPS | #### Route53 Healtch Check Options From b7e75b700d3a138073b875bfc516c39db3ec96d4 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 8 May 2020 08:44:26 -0700 Subject: [PATCH 06/21] Fix code coverage for NS1 --- octodns/provider/ns1.py | 3 +-- script/coverage | 6 ++++++ tests/test_octodns_provider_ns1.py | 10 ++++++++++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index fb0a78f..db589d6 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -766,8 +766,7 @@ class Ns1Provider(BaseProvider): for iso_region, target in record.geo.items(): key = 'iso_region_code' value = iso_region - if not has_country and \ - len(value.split('-')) > 1: # pragma: nocover + if not has_country and len(value.split('-')) > 1: has_country = True for answer in target.values: params['answers'].append( diff --git a/script/coverage b/script/coverage index ad8189e..32bdaea 100755 --- a/script/coverage +++ b/script/coverage @@ -26,6 +26,12 @@ export DYN_PASSWORD= export DYN_USERNAME= export GOOGLE_APPLICATION_CREDENTIALS= +# Don't allow disabling coverage +grep -r -I --line-number "# pragma: nocover" octodns && { + echo "Code coverage should not be disabled" + exit 1 +} + coverage run --branch --source=octodns --omit=octodns/cmds/* "$(command -v nosetests)" --with-xunit "$@" coverage html coverage xml diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index 8a0dffb..fa26a32 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -1059,6 +1059,16 @@ class TestNs1ProviderDynamic(TestCase): call(self.record, '3.4.5.6', 'mid-3'), ]) + record = Record.new(self.zone, 'geo', { + 'ttl': 34, + 'type': 'A', + 'values': ['101.102.103.104', '101.102.103.105'], + 'geo': {'EU': ['201.202.203.204']}, + 'meta': {}, + }) + params, _ = provider._params_for_geo_A(record) + self.assertEquals([], params['filters']) + def test_data_for_dynamic_A(self): provider = Ns1Provider('test', 'api-key') From 5975ae64be18c675182a05719f0066d62ba81be2 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Wed, 13 May 2020 09:47:46 -0700 Subject: [PATCH 07/21] Update NS1 _REGION_FILTER to include remove_no_georegion in config --- octodns/provider/ns1.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index b850ed9..b1efe00 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -253,7 +253,9 @@ class Ns1Provider(BaseProvider): def _REGION_FILTER(self, with_disabled): return self._update_filter({ - 'config': {}, + 'config': { + 'remove_no_georegion': True + }, 'filter': u'geofence_regional' }, with_disabled) From 74a13e4a196c367f08da8dea86fa76f2f21a1f0c Mon Sep 17 00:00:00 2001 From: Daniel Weissengruber Date: Mon, 18 May 2020 11:17:40 +0200 Subject: [PATCH 08/21] Cloudflare: Add Support for PTR Records --- README.md | 4 ++- octodns/provider/cloudflare.py | 7 +++-- .../cloudflare-dns_records-page-1.json | 2 +- .../cloudflare-dns_records-page-2.json | 21 +++++++++++-- tests/test_octodns_provider_cloudflare.py | 31 +++++++++++++++---- 5 files changed, 53 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index c2983fd..ce9be86 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,8 @@ It is similar to [Netflix/denominator](https://github.com/Netflix/denominator). ## Table of Contents +- [DNS as code - Tools for managing DNS across multiple providers](#dns-as-code---tools-for-managing-dns-across-multiple-providers) +- [Table of Contents](#table-of-contents) - [Getting started](#getting-started) - [Workspace](#workspace) - [Config](#config) @@ -178,7 +180,7 @@ The above command pulled the existing data out of Route53 and placed the results |--|--|--|--|--| | [AzureProvider](/octodns/provider/azuredns.py) | azure-mgmt-dns | A, AAAA, CAA, CNAME, MX, NS, PTR, SRV, TXT | No | | | [Akamai](/octodns/provider/edgedns.py) | edgegrid-python | A, AAAA, CNAME, MX, NAPTR, NS, PTR, SPF, SRV, SSHFP, TXT | No | | -| [CloudflareProvider](/octodns/provider/cloudflare.py) | | A, AAAA, ALIAS, CAA, CNAME, MX, NS, SPF, SRV, TXT | No | CAA tags restricted | +| [CloudflareProvider](/octodns/provider/cloudflare.py) | | A, AAAA, ALIAS, CAA, CNAME, MX, NS, PTR, SPF, SRV, TXT | No | CAA tags restricted | | [ConstellixProvider](/octodns/provider/constellix.py) | | A, AAAA, ALIAS (ANAME), CAA, CNAME, MX, NS, PTR, SPF, SRV, TXT | No | CAA tags restricted | | [DigitalOceanProvider](/octodns/provider/digitalocean.py) | | A, AAAA, CAA, CNAME, MX, NS, TXT, SRV | No | CAA tags restricted | | [DnsMadeEasyProvider](/octodns/provider/dnsmadeeasy.py) | | A, AAAA, ALIAS (ANAME), CAA, CNAME, MX, NS, PTR, SPF, SRV, TXT | No | CAA tags restricted | diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index e52771c..e38177e 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -60,8 +60,8 @@ class CloudflareProvider(BaseProvider): ''' SUPPORTS_GEO = False SUPPORTS_DYNAMIC = False - SUPPORTS = set(('ALIAS', 'A', 'AAAA', 'CAA', 'CNAME', 'MX', 'NS', 'SRV', - 'SPF', 'TXT')) + SUPPORTS = set(('ALIAS', 'A', 'AAAA', 'CAA', 'CNAME', 'MX', 'NS', 'PTR', + 'SRV', 'SPF', 'TXT')) MIN_TTL = 120 TIMEOUT = 15 @@ -173,6 +173,7 @@ class CloudflareProvider(BaseProvider): } _data_for_ALIAS = _data_for_CNAME + _data_for_PTR = _data_for_CNAME def _data_for_MX(self, _type, records): values = [] @@ -339,6 +340,8 @@ class CloudflareProvider(BaseProvider): def _contents_for_CNAME(self, record): yield {'content': record.value} + _contents_for_PTR = _contents_for_CNAME + def _contents_for_MX(self, record): for value in record.values: yield { diff --git a/tests/fixtures/cloudflare-dns_records-page-1.json b/tests/fixtures/cloudflare-dns_records-page-1.json index 3c423e2..efe0654 100644 --- a/tests/fixtures/cloudflare-dns_records-page-1.json +++ b/tests/fixtures/cloudflare-dns_records-page-1.json @@ -180,7 +180,7 @@ "per_page": 10, "total_pages": 2, "count": 10, - "total_count": 19 + "total_count": 20 }, "success": true, "errors": [], diff --git a/tests/fixtures/cloudflare-dns_records-page-2.json b/tests/fixtures/cloudflare-dns_records-page-2.json index 558aa2c..b0bbaef 100644 --- a/tests/fixtures/cloudflare-dns_records-page-2.json +++ b/tests/fixtures/cloudflare-dns_records-page-2.json @@ -157,6 +157,23 @@ "auto_added": false } }, + { + "id": "fc12ab34cd5611334422ab3322997677", + "type": "PTR", + "name": "ptr.unit.tests", + "content": "foo.bar.com", + "proxiable": true, + "proxied": false, + "ttl": 300, + "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 + } + }, { "id": "fc12ab34cd5611334422ab3322997656", "type": "SRV", @@ -212,8 +229,8 @@ "page": 2, "per_page": 11, "total_pages": 2, - "count": 9, - "total_count": 21 + "count": 10, + "total_count": 20 }, "success": true, "errors": [], diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 5bcf25f..d1069eb 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -149,7 +149,7 @@ class TestCloudflareProvider(TestCase): zone = Zone('unit.tests.', []) provider.populate(zone) - self.assertEquals(12, len(zone.records)) + self.assertEquals(13, len(zone.records)) changes = self.expected.changes(zone, provider) @@ -158,7 +158,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(12, len(again.records)) + self.assertEquals(13, len(again.records)) def test_apply(self): provider = CloudflareProvider('test', 'email', 'token') @@ -172,12 +172,12 @@ class TestCloudflareProvider(TestCase): 'id': 42, } }, # zone create - ] + [None] * 20 # individual record creates + ] + [None] * 22 # individual record creates # non-existent zone, create everything plan = provider.plan(self.expected) - self.assertEquals(12, len(plan.changes)) - self.assertEquals(12, provider.apply(plan)) + self.assertEquals(13, len(plan.changes)) + self.assertEquals(13, provider.apply(plan)) self.assertFalse(plan.exists) provider._request.assert_has_calls([ @@ -203,7 +203,7 @@ class TestCloudflareProvider(TestCase): }), ], True) # expected number of total calls - self.assertEquals(22, provider._request.call_count) + self.assertEquals(23, provider._request.call_count) provider._request.reset_mock() @@ -510,6 +510,25 @@ class TestCloudflareProvider(TestCase): 'fc12ab34cd5611334422ab3322997653') ]) + def test_ptr(self): + provider = CloudflareProvider('test', 'email', 'token') + + zone = Zone('unit.tests.', []) + # PTR record + ptr_record = Record.new(zone, 'ptr', { + 'ttl': 300, + 'type': 'PTR', + 'value': 'foo.bar.com.' + }) + + ptr_record_contents = provider._gen_data(ptr_record) + self.assertEquals({ + 'name': 'ptr.unit.tests', + 'ttl': 300, + 'type': 'PTR', + 'content': 'foo.bar.com.' + }, list(ptr_record_contents)[0]) + def test_srv(self): provider = CloudflareProvider('test', 'email', 'token') From 53d654c39d5c67005be8aedbf172f2da81da6adb Mon Sep 17 00:00:00 2001 From: Lance Hudson Date: Thu, 28 May 2020 22:17:34 -0400 Subject: [PATCH 09/21] Cloudflare: Add Support for Rate Limit --- octodns/provider/cloudflare.py | 50 ++++++++-- tests/test_octodns_provider_cloudflare.py | 110 ++++++++++++++++++++-- 2 files changed, 144 insertions(+), 16 deletions(-) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index e38177e..503eb90 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -9,6 +9,7 @@ from collections import defaultdict from copy import deepcopy from logging import getLogger from requests import Session +from time import sleep from ..record import Record, Update from .base import BaseProvider @@ -18,7 +19,7 @@ class CloudflareError(Exception): def __init__(self, data): try: message = data['errors'][0]['message'] - except (IndexError, KeyError): + except (IndexError, KeyError, TypeError): message = 'Cloudflare error' super(CloudflareError, self).__init__(message) @@ -28,6 +29,11 @@ class CloudflareAuthenticationError(CloudflareError): CloudflareError.__init__(self, data) +class CloudflareRateLimitError(CloudflareError): + def __init__(self, data): + CloudflareError.__init__(self, data) + + _PROXIABLE_RECORD_TYPES = {'A', 'AAAA', 'ALIAS', 'CNAME'} @@ -47,6 +53,11 @@ class CloudflareProvider(BaseProvider): # # See: https://support.cloudflare.com/hc/en-us/articles/115000830351 cdn: false + # Optional. Default: 4. Number of times to retry if a 429 response + # is received. + retry_count: 4 + # Optional. Default: 300. Number of seconds to wait before retrying. + retry_period: 300 Note: The "proxied" flag of "A", "AAAA" and "CNAME" records can be managed via the YAML provider like so: @@ -66,7 +77,8 @@ class CloudflareProvider(BaseProvider): MIN_TTL = 120 TIMEOUT = 15 - def __init__(self, id, email=None, token=None, cdn=False, *args, **kwargs): + def __init__(self, id, email=None, token=None, cdn=False, retry_count=4, + retry_period=300, *args, **kwargs): self.log = getLogger('CloudflareProvider[{}]'.format(id)) self.log.debug('__init__: id=%s, email=%s, token=***, cdn=%s', id, email, cdn) @@ -85,11 +97,27 @@ class CloudflareProvider(BaseProvider): 'Authorization': 'Bearer {}'.format(token), }) self.cdn = cdn + self.retry_count = retry_count + self.retry_period = retry_period self._sess = sess self._zones = None self._zone_records = {} + def _try(self, *args, **kwargs): + tries = self.retry_count + while True: # We'll raise to break after our tries expire + try: + return self._request(*args, **kwargs) + except CloudflareRateLimitError: + if tries <= 1: + raise + tries -= 1 + self.log.warn('rate limit encountered, pausing ' + 'for %ds and trying again, %d remaining', + self.retry_period, tries) + sleep(self.retry_period) + def _request(self, method, path, params=None, data=None): self.log.debug('_request: method=%s, path=%s', method, path) @@ -101,6 +129,8 @@ class CloudflareProvider(BaseProvider): raise CloudflareError(resp.json()) if resp.status_code == 403: raise CloudflareAuthenticationError(resp.json()) + if resp.status_code == 429: + raise CloudflareRateLimitError(resp.json()) resp.raise_for_status() return resp.json() @@ -111,7 +141,7 @@ class CloudflareProvider(BaseProvider): page = 1 zones = [] while page: - resp = self._request('GET', '/zones', params={'page': page}) + resp = self._try('GET', '/zones', params={'page': page}) zones += resp['result'] info = resp['result_info'] if info['count'] > 0 and info['count'] == info['per_page']: @@ -220,7 +250,7 @@ class CloudflareProvider(BaseProvider): path = '/zones/{}/dns_records'.format(zone_id) page = 1 while page: - resp = self._request('GET', path, params={'page': page}) + resp = self._try('GET', path, params={'page': page}) records += resp['result'] info = resp['result_info'] if info['count'] > 0 and info['count'] == info['per_page']: @@ -433,7 +463,7 @@ class CloudflareProvider(BaseProvider): zone_id = self.zones[new.zone.name] path = '/zones/{}/dns_records'.format(zone_id) for content in self._gen_data(new): - self._request('POST', path, data=content) + self._try('POST', path, data=content) def _apply_Update(self, change): zone = change.new.zone @@ -522,7 +552,7 @@ class CloudflareProvider(BaseProvider): path = '/zones/{}/dns_records'.format(zone_id) for _, data in sorted(creates.items()): self.log.debug('_apply_Update: creating %s', data) - self._request('POST', path, data=data) + self._try('POST', path, data=data) # Updates for _, info in sorted(updates.items()): @@ -532,7 +562,7 @@ class CloudflareProvider(BaseProvider): path = '/zones/{}/dns_records/{}'.format(zone_id, record_id) self.log.debug('_apply_Update: updating %s, %s -> %s', record_id, data, old_data) - self._request('PUT', path, data=data) + self._try('PUT', path, data=data) # Deletes for _, info in sorted(deletes.items()): @@ -541,7 +571,7 @@ class CloudflareProvider(BaseProvider): path = '/zones/{}/dns_records/{}'.format(zone_id, record_id) self.log.debug('_apply_Update: removing %s, %s', record_id, old_data) - self._request('DELETE', path) + self._try('DELETE', path) def _apply_Delete(self, change): existing = change.existing @@ -554,7 +584,7 @@ class CloudflareProvider(BaseProvider): existing_type == record['type']: path = '/zones/{}/dns_records/{}'.format(record['zone_id'], record['id']) - self._request('DELETE', path) + self._try('DELETE', path) def _apply(self, plan): desired = plan.desired @@ -569,7 +599,7 @@ class CloudflareProvider(BaseProvider): 'name': name[:-1], 'jump_start': False, } - resp = self._request('POST', '/zones', data=data) + resp = self._try('POST', '/zones', data=data) zone_id = resp['result']['id'] self.zones[name] = zone_id self._zone_records[name] = {} diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index d1069eb..08608ea 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -14,7 +14,8 @@ from unittest import TestCase from octodns.record import Record, Update from octodns.provider.base import Plan -from octodns.provider.cloudflare import CloudflareProvider +from octodns.provider.cloudflare import CloudflareProvider, \ + CloudflareRateLimitError from octodns.provider.yaml import YamlProvider from octodns.zone import Zone @@ -52,7 +53,7 @@ class TestCloudflareProvider(TestCase): empty = {'result': [], 'result_info': {'count': 0, 'per_page': 0}} def test_populate(self): - provider = CloudflareProvider('test', 'email', 'token') + provider = CloudflareProvider('test', 'email', 'token', retry_period=0) # Bad requests with requests_mock() as mock: @@ -103,6 +104,36 @@ class TestCloudflareProvider(TestCase): provider.populate(zone) self.assertEquals(502, ctx.exception.response.status_code) + # Rate Limit error + with requests_mock() as mock: + mock.get(ANY, status_code=429, + text='{"success":false,"errors":[{"code":10100,' + '"message":"More than 1200 requests per 300 seconds ' + 'reached. Please wait and consider throttling your ' + 'request speed"}],"messages":[],"result":null}') + + with self.assertRaises(Exception) as ctx: + zone = Zone('unit.tests.', []) + provider.populate(zone) + + self.assertEquals('CloudflareRateLimitError', + type(ctx.exception).__name__) + self.assertEquals('More than 1200 requests per 300 seconds ' + 'reached. Please wait and consider throttling ' + 'your request speed', text_type(ctx.exception)) + + # Rate Limit error, unknown resp + with requests_mock() as mock: + mock.get(ANY, status_code=429, text='{}') + + with self.assertRaises(Exception) as ctx: + zone = Zone('unit.tests.', []) + provider.populate(zone) + + self.assertEquals('CloudflareRateLimitError', + type(ctx.exception).__name__) + self.assertEquals('Cloudflare error', text_type(ctx.exception)) + # Non-existent zone doesn't populate anything with requests_mock() as mock: mock.get(ANY, status_code=200, json=self.empty) @@ -161,7 +192,7 @@ class TestCloudflareProvider(TestCase): self.assertEquals(13, len(again.records)) def test_apply(self): - provider = CloudflareProvider('test', 'email', 'token') + provider = CloudflareProvider('test', 'email', 'token', retry_period=0) provider._request = Mock() @@ -280,7 +311,11 @@ class TestCloudflareProvider(TestCase): # we don't care about the POST/create return values provider._request.return_value = {} - provider._request.side_effect = None + + # Test out the create rate-limit handling, then 9 successes + provider._request.side_effect = [ + CloudflareRateLimitError('{}'), + ] + ([None] * 3) wanted = Zone('unit.tests.', []) wanted.add_record(Record.new(wanted, 'nc', { @@ -316,7 +351,7 @@ class TestCloudflareProvider(TestCase): ]) def test_update_add_swap(self): - provider = CloudflareProvider('test', 'email', 'token') + provider = CloudflareProvider('test', 'email', 'token', retry_period=0) provider.zone_records = Mock(return_value=[ { @@ -357,6 +392,7 @@ class TestCloudflareProvider(TestCase): provider._request = Mock() provider._request.side_effect = [ + CloudflareRateLimitError('{}'), self.empty, # no zones { 'result': { @@ -423,7 +459,7 @@ class TestCloudflareProvider(TestCase): def test_update_delete(self): # We need another run so that we can delete, we can't both add and # delete in one go b/c of swaps - provider = CloudflareProvider('test', 'email', 'token') + provider = CloudflareProvider('test', 'email', 'token', retry_period=0) provider.zone_records = Mock(return_value=[ { @@ -464,6 +500,7 @@ class TestCloudflareProvider(TestCase): provider._request = Mock() provider._request.side_effect = [ + CloudflareRateLimitError('{}'), self.empty, # no zones { 'result': { @@ -1242,3 +1279,64 @@ class TestCloudflareProvider(TestCase): provider = CloudflareProvider('test', token='token 123') headers = provider._sess.headers self.assertEquals('Bearer token 123', headers['Authorization']) + + def test_retry_behavior(self): + provider = CloudflareProvider('test', token='token 123', + email='email 234', retry_period=0) + result = { + "success": True, + "errors": [], + "messages": [], + "result": [], + "result_info": { + "count": 1, + "per_page": 50 + } + } + zone = Zone('unit.tests.', []) + provider._request = Mock() + + # No retry required, just calls and is returned + provider._zones = None + provider._request.reset_mock() + provider._request.side_effect = [result] + self.assertEquals([], provider.zone_records(zone)) + provider._request.assert_has_calls([call('GET', '/zones', + params={'page': 1})]) + + # One retry required + provider._zones = None + provider._request.reset_mock() + provider._request.side_effect = [ + CloudflareRateLimitError('{}'), + result + ] + self.assertEquals([], provider.zone_records(zone)) + provider._request.assert_has_calls([call('GET', '/zones', + params={'page': 1})]) + + # Two retries required + provider._zones = None + provider._request.reset_mock() + provider._request.side_effect = [ + CloudflareRateLimitError('{}'), + CloudflareRateLimitError('{}'), + result + ] + self.assertEquals([], provider.zone_records(zone)) + provider._request.assert_has_calls([call('GET', '/zones', + params={'page': 1})]) + + # # Exhaust our retries + provider._zones = None + provider._request.reset_mock() + provider._request.side_effect = [ + CloudflareRateLimitError({"errors": [{"message": "first"}]}), + CloudflareRateLimitError({"errors": [{"message": "boo"}]}), + CloudflareRateLimitError({"errors": [{"message": "boo"}]}), + CloudflareRateLimitError({"errors": [{"message": "boo"}]}), + CloudflareRateLimitError({"errors": [{"message": "last"}]}), + ] + with self.assertRaises(CloudflareRateLimitError) as ctx: + provider.zone_records(zone) + self.assertEquals('last', text_type(ctx.exception)) From a939cf52b064fff9a01c341b580c2ef96ab21bb5 Mon Sep 17 00:00:00 2001 From: Lance Hudson Date: Fri, 29 May 2020 16:59:55 -0400 Subject: [PATCH 10/21] Cloudflare: Rename _try to _try_request --- octodns/provider/cloudflare.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index 503eb90..698fbee 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -104,7 +104,7 @@ class CloudflareProvider(BaseProvider): self._zones = None self._zone_records = {} - def _try(self, *args, **kwargs): + def _try_request(self, *args, **kwargs): tries = self.retry_count while True: # We'll raise to break after our tries expire try: @@ -141,7 +141,8 @@ class CloudflareProvider(BaseProvider): page = 1 zones = [] while page: - resp = self._try('GET', '/zones', params={'page': page}) + resp = self._try_request('GET', '/zones', + params={'page': page}) zones += resp['result'] info = resp['result_info'] if info['count'] > 0 and info['count'] == info['per_page']: @@ -250,7 +251,7 @@ class CloudflareProvider(BaseProvider): path = '/zones/{}/dns_records'.format(zone_id) page = 1 while page: - resp = self._try('GET', path, params={'page': page}) + resp = self._try_request('GET', path, params={'page': page}) records += resp['result'] info = resp['result_info'] if info['count'] > 0 and info['count'] == info['per_page']: @@ -463,7 +464,7 @@ class CloudflareProvider(BaseProvider): zone_id = self.zones[new.zone.name] path = '/zones/{}/dns_records'.format(zone_id) for content in self._gen_data(new): - self._try('POST', path, data=content) + self._try_request('POST', path, data=content) def _apply_Update(self, change): zone = change.new.zone @@ -552,7 +553,7 @@ class CloudflareProvider(BaseProvider): path = '/zones/{}/dns_records'.format(zone_id) for _, data in sorted(creates.items()): self.log.debug('_apply_Update: creating %s', data) - self._try('POST', path, data=data) + self._try_request('POST', path, data=data) # Updates for _, info in sorted(updates.items()): @@ -562,7 +563,7 @@ class CloudflareProvider(BaseProvider): path = '/zones/{}/dns_records/{}'.format(zone_id, record_id) self.log.debug('_apply_Update: updating %s, %s -> %s', record_id, data, old_data) - self._try('PUT', path, data=data) + self._try_request('PUT', path, data=data) # Deletes for _, info in sorted(deletes.items()): @@ -571,7 +572,7 @@ class CloudflareProvider(BaseProvider): path = '/zones/{}/dns_records/{}'.format(zone_id, record_id) self.log.debug('_apply_Update: removing %s, %s', record_id, old_data) - self._try('DELETE', path) + self._try_request('DELETE', path) def _apply_Delete(self, change): existing = change.existing @@ -584,7 +585,7 @@ class CloudflareProvider(BaseProvider): existing_type == record['type']: path = '/zones/{}/dns_records/{}'.format(record['zone_id'], record['id']) - self._try('DELETE', path) + self._try_request('DELETE', path) def _apply(self, plan): desired = plan.desired @@ -599,7 +600,7 @@ class CloudflareProvider(BaseProvider): 'name': name[:-1], 'jump_start': False, } - resp = self._try('POST', '/zones', data=data) + resp = self._try_request('POST', '/zones', data=data) zone_id = resp['result']['id'] self.zones[name] = zone_id self._zone_records[name] = {} From fe0b4c3871fd20d66a1b63851686356d157379ef Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 1 Jun 2020 10:47:35 +0000 Subject: [PATCH 11/21] Bump setuptools from 44.1.0 to 44.1.1 Bumps [setuptools](https://github.com/pypa/setuptools) from 44.1.0 to 44.1.1. - [Release notes](https://github.com/pypa/setuptools/releases) - [Changelog](https://github.com/pypa/setuptools/blob/master/CHANGES.rst) - [Commits](https://github.com/pypa/setuptools/compare/v44.1.0...v44.1.1) Signed-off-by: dependabot-preview[bot] --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 71294a8..a0bcfb1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -21,6 +21,6 @@ pycountry==19.8.18 python-dateutil==2.8.1 requests==2.23.0 s3transfer==0.3.3 -setuptools==44.1.0 +setuptools==44.1.1 six==1.14.0 transip==2.1.2 From 4217a3452bfb32e6d8c0198478ad9c324f4a2f3f Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 1 Jun 2020 12:49:15 +0000 Subject: [PATCH 12/21] Bump ns1-python from 0.15.0 to 0.16.0 Bumps [ns1-python](https://github.com/ns1/ns1-python) from 0.15.0 to 0.16.0. - [Release notes](https://github.com/ns1/ns1-python/releases) - [Changelog](https://github.com/ns1/ns1-python/blob/master/CHANGELOG.md) - [Commits](https://github.com/ns1/ns1-python/compare/v0.15.0...v0.16.0) Signed-off-by: dependabot-preview[bot] --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index a0bcfb1..1c39f3a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -14,7 +14,7 @@ ipaddress==1.0.23 jmespath==0.9.5 msrestazure==0.6.3 natsort==6.2.1 -ns1-python==0.15.0 +ns1-python==0.16.0 ovh==0.5.0 pycountry-convert==0.7.2 pycountry==19.8.18 From e6d370869f91482238fce69b242096f48f79bba5 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 1 Jun 2020 12:53:32 +0000 Subject: [PATCH 13/21] Bump six from 1.14.0 to 1.15.0 Bumps [six](https://github.com/benjaminp/six) from 1.14.0 to 1.15.0. - [Release notes](https://github.com/benjaminp/six/releases) - [Changelog](https://github.com/benjaminp/six/blob/master/CHANGES) - [Commits](https://github.com/benjaminp/six/compare/1.14.0...1.15.0) Signed-off-by: dependabot-preview[bot] --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 1c39f3a..01499f8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -22,5 +22,5 @@ python-dateutil==2.8.1 requests==2.23.0 s3transfer==0.3.3 setuptools==44.1.1 -six==1.14.0 +six==1.15.0 transip==2.1.2 From 2f2b2a32b2e22d15ced6bbc64c2e25cfc76ae69e Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 1 Jun 2020 12:57:02 +0000 Subject: [PATCH 14/21] Bump botocore from 1.16.0 to 1.16.19 Bumps [botocore](https://github.com/boto/botocore) from 1.16.0 to 1.16.19. - [Release notes](https://github.com/boto/botocore/releases) - [Changelog](https://github.com/boto/botocore/blob/develop/CHANGELOG.rst) - [Commits](https://github.com/boto/botocore/compare/1.16.0...1.16.19) Signed-off-by: dependabot-preview[bot] --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 01499f8..ddad9d0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,7 +2,7 @@ PyYaml==5.3.1 azure-common==1.1.25 azure-mgmt-dns==3.0.0 boto3==1.13.0 -botocore==1.16.0 +botocore==1.16.19 dnspython==1.16.0 docutils==0.16 dyn==1.8.1 From 7c52fdb818fe7d2288b503db36b4e56df12b2459 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 1 Jun 2020 13:19:17 +0000 Subject: [PATCH 15/21] Bump boto3 from 1.13.0 to 1.13.19 Bumps [boto3](https://github.com/boto/boto3) from 1.13.0 to 1.13.19. - [Release notes](https://github.com/boto/boto3/releases) - [Changelog](https://github.com/boto/boto3/blob/develop/CHANGELOG.rst) - [Commits](https://github.com/boto/boto3/compare/1.13.0...1.13.19) Signed-off-by: dependabot-preview[bot] --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index ddad9d0..d60be3d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,7 @@ PyYaml==5.3.1 azure-common==1.1.25 azure-mgmt-dns==3.0.0 -boto3==1.13.0 +boto3==1.13.19 botocore==1.16.19 dnspython==1.16.0 docutils==0.16 From 42c55f298e2caf219612a23e4338e28a6a574b9c Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 1 Jun 2020 13:25:22 +0000 Subject: [PATCH 16/21] Bump pycodestyle from 2.5.0 to 2.6.0 Bumps [pycodestyle](https://github.com/PyCQA/pycodestyle) from 2.5.0 to 2.6.0. - [Release notes](https://github.com/PyCQA/pycodestyle/releases) - [Changelog](https://github.com/PyCQA/pycodestyle/blob/master/CHANGES.txt) - [Commits](https://github.com/PyCQA/pycodestyle/compare/2.5.0...2.6.0) Signed-off-by: dependabot-preview[bot] --- requirements-dev.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index a16b484..485a33f 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,7 +1,7 @@ coverage mock nose -pycodestyle==2.5.0 +pycodestyle==2.6.0 pyflakes==2.2.0 readme_renderer[md]==26.0 requests_mock From a1b72ac29fc3078f721cab53fba708d1e3b4fd04 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Mon, 1 Jun 2020 06:26:48 -0700 Subject: [PATCH 17/21] Ignore E741, flags single-letter var names in comprehensions which I want to allow --- script/lint | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/lint b/script/lint index 5fd9a7d..627c5be 100755 --- a/script/lint +++ b/script/lint @@ -17,5 +17,5 @@ fi SOURCES="*.py octodns/*.py octodns/*/*.py tests/*.py" -pycodestyle --ignore=E221,E241,E251,E722,W504 $SOURCES +pycodestyle --ignore=E221,E241,E251,E722,E741,W504 $SOURCES pyflakes $SOURCES From 87a7595eaee4b9bcf06e959fa43fb808f7cb61b8 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Tue, 2 Jun 2020 15:51:41 -0700 Subject: [PATCH 18/21] Update geo_data to pick up a couple renames --- octodns/record/geo_data.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/octodns/record/geo_data.py b/octodns/record/geo_data.py index 5393db0..051d446 100644 --- a/octodns/record/geo_data.py +++ b/octodns/record/geo_data.py @@ -55,7 +55,7 @@ geo_data = \ 'SO': {'name': 'Somalia'}, 'SS': {'name': 'South Sudan'}, 'ST': {'name': 'Sao Tome and Principe'}, - 'SZ': {'name': 'Swaziland'}, + 'SZ': {'name': 'Eswatini'}, 'TD': {'name': 'Chad'}, 'TG': {'name': 'Togo'}, 'TN': {'name': 'Tunisia'}, @@ -157,7 +157,7 @@ geo_data = \ 'MC': {'name': 'Monaco'}, 'MD': {'name': 'Moldova, Republic of'}, 'ME': {'name': 'Montenegro'}, - 'MK': {'name': 'Macedonia, Republic of'}, + 'MK': {'name': 'North Macedonia'}, 'MT': {'name': 'Malta'}, 'NL': {'name': 'Netherlands'}, 'NO': {'name': 'Norway'}, From 1569d94513758bbae628930ea05730bc8e8d4b13 Mon Sep 17 00:00:00 2001 From: Dan Hanks Date: Wed, 3 Jun 2020 09:36:33 -0400 Subject: [PATCH 19/21] Add support for geo-targeting of CA provinces - For providers that support such --- octodns/record/geo.py | 11 ++++++++--- script/generate-geo-data | 4 ++-- tests/test_octodns_record_geo.py | 2 ++ 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/octodns/record/geo.py b/octodns/record/geo.py index ed54194..bd8ebc3 100644 --- a/octodns/record/geo.py +++ b/octodns/record/geo.py @@ -63,9 +63,14 @@ class GeoCodes(object): @classmethod def province_to_code(cls, province): - # We get to cheat on this one since we only support provinces in NA-US - if province not in geo_data['NA']['US']['provinces']: + # We cheat on this one a little since we only support provinces in NA-US, NA-CA + if (province not in geo_data['NA']['US']['provinces'] and + province not in geo_data['NA']['CA']['provinces']): cls.log.warn('country_to_code: unrecognized province "%s"', province) return - return 'NA-US-{}'.format(province) + if province in geo_data['NA']['US']['provinces']: + country = 'US' + if province in geo_data['NA']['CA']['provinces']: + country = 'CA' + return 'NA-{}-{}'.format(country, province) diff --git a/script/generate-geo-data b/script/generate-geo-data index 87a57b1..da49701 100755 --- a/script/generate-geo-data +++ b/script/generate-geo-data @@ -8,8 +8,8 @@ from pycountry_convert import country_alpha2_to_continent_code subs = defaultdict(dict) for subdivision in subdivisions: - # Route53 only supports US states, Dyn supports US states and CA provinces, but for now we'll just do US - if subdivision.country_code not in ('US'): + # Route53 only supports US states, Dyn (and others) support US states and CA provinces + if subdivision.country_code not in ('US', 'CA'): continue subs[subdivision.country_code][subdivision.code[3:]] = { 'name': subdivision.name diff --git a/tests/test_octodns_record_geo.py b/tests/test_octodns_record_geo.py index 5b7454c..35df6d5 100644 --- a/tests/test_octodns_record_geo.py +++ b/tests/test_octodns_record_geo.py @@ -77,4 +77,6 @@ class TestRecordGeoCodes(TestCase): def test_province_to_code(self): self.assertEquals('NA-US-OR', GeoCodes.province_to_code('OR')) self.assertEquals('NA-US-KY', GeoCodes.province_to_code('KY')) + self.assertEquals('NA-CA-AB', GeoCodes.province_to_code('AB')) + self.assertEquals('NA-CA-BC', GeoCodes.province_to_code('BC')) self.assertFalse(GeoCodes.province_to_code('XX')) From 559a3994e6339047db492c38de09bc9661755d5a Mon Sep 17 00:00:00 2001 From: Dan Hanks Date: Wed, 3 Jun 2020 10:07:35 -0400 Subject: [PATCH 20/21] Fix comment < 80 chars --- octodns/record/geo.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/octodns/record/geo.py b/octodns/record/geo.py index bd8ebc3..0a2f1a3 100644 --- a/octodns/record/geo.py +++ b/octodns/record/geo.py @@ -63,7 +63,8 @@ class GeoCodes(object): @classmethod def province_to_code(cls, province): - # We cheat on this one a little since we only support provinces in NA-US, NA-CA + # We cheat on this one a little since we only support provinces in + # NA-US, NA-CA if (province not in geo_data['NA']['US']['provinces'] and province not in geo_data['NA']['CA']['provinces']): cls.log.warn('country_to_code: unrecognized province "%s"', From 07279e48047550f8587e1037f1438b4ae91bac91 Mon Sep 17 00:00:00 2001 From: Dan Hanks Date: Wed, 3 Jun 2020 11:10:54 -0400 Subject: [PATCH 21/21] Add Canadian provinces to geo_data.py --- octodns/record/geo_data.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/octodns/record/geo_data.py b/octodns/record/geo_data.py index 051d446..13b05e6 100644 --- a/octodns/record/geo_data.py +++ b/octodns/record/geo_data.py @@ -183,7 +183,20 @@ geo_data = \ 'BQ': {'name': 'Bonaire, Sint Eustatius and Saba'}, 'BS': {'name': 'Bahamas'}, 'BZ': {'name': 'Belize'}, - 'CA': {'name': 'Canada'}, + 'CA': {'name': 'Canada', + 'provinces': {'AB': {'name': 'Alberta'}, + 'BC': {'name': 'British Columbia'}, + 'MB': {'name': 'Manitoba'}, + 'NB': {'name': 'New Brunswick'}, + 'NL': {'name': 'Newfoundland and Labrador'}, + 'NS': {'name': 'Nova Scotia'}, + 'NT': {'name': 'Northwest Territories'}, + 'NU': {'name': 'Nunavut'}, + 'ON': {'name': 'Ontario'}, + 'PE': {'name': 'Prince Edward Island'}, + 'QC': {'name': 'Quebec'}, + 'SK': {'name': 'Saskatchewan'}, + 'YT': {'name': 'Yukon Territory'}}}, 'CR': {'name': 'Costa Rica'}, 'CU': {'name': 'Cuba'}, 'CW': {'name': 'Curaçao'},