From 0dfcc6f6f21706c8a5a8572d616bb27e4c91c1d9 Mon Sep 17 00:00:00 2001 From: Steve Coursen Date: Mon, 13 Nov 2017 09:37:03 -0500 Subject: [PATCH 01/17] 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 02/17] 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 03/17] 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 04/17] 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 05/17] 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 06/17] 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'] From 6c91a92a72f3e297e44d669964ac06a9ad32c291 Mon Sep 17 00:00:00 2001 From: Stephen Coursen Date: Fri, 5 Jan 2018 19:24:38 +0000 Subject: [PATCH 07/17] Add geotarget filter, change log level to debug --- octodns/provider/ns1.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 6b0fe7e..d022e78 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -50,6 +50,10 @@ class Ns1Provider(BaseProvider): for answer in record.get('answers', []): meta = answer.get('meta', {}) if meta: + # country + state and country + province are allowed + # in that case though, supplying a state/province would + # be redundant since the country would supercede in when + # resolving the record. it is syntactically valid, however. country = meta.get('country', []) us_state = meta.get('us_state', []) ca_province = meta.get('ca_province', []) @@ -204,16 +208,32 @@ class Ns1Provider(BaseProvider): # so that we know we did this on purpose if/when troubleshooting params['answers'] = [{"answer": [x], "meta": {}} for x in record.values] + has_country = False for iso_region, target in record.geo.items(): key = 'iso_region_code' value = iso_region + if not has_country and len(value.split('-')) > 1: + has_country = True params['answers'].append( { 'answer': target.values, 'meta': {key: [value]}, }, ) - self.log.info("params for A: %s", params) + params['filters'] = [] + if len(params['answers']) > 1: + params['filters'].append( + {"filter": "shuffle", "config":{}} + ) + if has_country: + params['filters'].append( + {"filter": "geotarget_country", "config": {}} + ) + params['filters'].append( + {"filter": "select_first_n", + "config": {"N": 1}} + ) + self.log.debug("params for A: %s", params) return params _params_for_AAAA = _params_for_A From e6cda62284043303af06da207b73e90bad617f70 Mon Sep 17 00:00:00 2001 From: Stephen Coursen Date: Fri, 5 Jan 2018 22:34:15 +0000 Subject: [PATCH 08/17] Only add shuffle if there is more than 1 answer *and* any of the answers have geo --- octodns/provider/ns1.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index d022e78..1206c2f 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -221,11 +221,11 @@ class Ns1Provider(BaseProvider): }, ) params['filters'] = [] - if len(params['answers']) > 1: - params['filters'].append( - {"filter": "shuffle", "config":{}} - ) if has_country: + if len(params['answers']) > 1: + params['filters'].append( + {"filter": "shuffle", "config":{}} + ) params['filters'].append( {"filter": "geotarget_country", "config": {}} ) From 34f2432c3f99c7189eedbce3b40ab1fda9c38fdf Mon Sep 17 00:00:00 2001 From: Stephen Coursen Date: Fri, 5 Jan 2018 22:45:00 +0000 Subject: [PATCH 09/17] after discussion, we should shuffle if there's more than 1 answer --- octodns/provider/ns1.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 1206c2f..d022e78 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -221,11 +221,11 @@ class Ns1Provider(BaseProvider): }, ) params['filters'] = [] + if len(params['answers']) > 1: + params['filters'].append( + {"filter": "shuffle", "config":{}} + ) if has_country: - if len(params['answers']) > 1: - params['filters'].append( - {"filter": "shuffle", "config":{}} - ) params['filters'].append( {"filter": "geotarget_country", "config": {}} ) From 0df8ed5e2a86d4c7e220841add8c479cf8f91e6b Mon Sep 17 00:00:00 2001 From: Stephen Coursen Date: Sun, 7 Jan 2018 03:50:10 +0000 Subject: [PATCH 10/17] bump required version of nsone-python client --- setup.cfg | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/setup.cfg b/setup.cfg index 54e9014..0473649 100644 --- a/setup.cfg +++ b/setup.cfg @@ -19,7 +19,7 @@ classifiers = Programming Language :: Python :: 3.6 [options] -install_requires = +install_requires = PyYaml>=3.12 dnspython>=1.15.0 futures==3.1.1 @@ -32,7 +32,7 @@ packages = find: include_package_data = True [options.entry_points] -console_scripts = +console_scripts = octodns-compare = octodns.cmds.compare:main octodns-dump = octodns.cmds.dump:main octodns-report = octodns.cmds.report:main @@ -44,7 +44,7 @@ exclude = tests [options.extras_require] -dev = +dev = azure-mgmt-dns==1.0.1 azure-common==1.1.6 boto3==1.4.6 @@ -54,7 +54,7 @@ dev = google-cloud==0.27.0 jmespath==0.9.3 msrestazure==0.4.10 - nsone==0.9.14 + nsone==0.9.17 ovh==0.4.7 s3transfer==0.1.10 six==1.10.0 From dc43c43866fec17119f5320553df2eb399ca3278 Mon Sep 17 00:00:00 2001 From: Steve Coursen Date: Mon, 8 Jan 2018 10:02:27 -0500 Subject: [PATCH 11/17] Increased test coverage --- octodns/provider/ns1.py | 7 ++-- tests/test_octodns_provider_ns1.py | 59 +++++++++++++++++++++++++++--- 2 files changed, 58 insertions(+), 8 deletions(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index d022e78..20fffc3 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -64,7 +64,7 @@ class Ns1Provider(BaseProvider): for state in us_state: geo['NA-US-{}'.format(state)] = answer['answer'] for province in ca_province: - geo['NA-CA-{}'.format(state)] = answer['answer'] + geo['NA-CA-{}'.format(province)] = answer['answer'] for code in meta.get('iso_region_code', []): geo[code] = answer['answer'] else: @@ -212,7 +212,8 @@ 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: + if not has_country and \ + len(value.split('-')) > 1: # pragma: nocover has_country = True params['answers'].append( { @@ -223,7 +224,7 @@ class Ns1Provider(BaseProvider): params['filters'] = [] if len(params['answers']) > 1: params['filters'].append( - {"filter": "shuffle", "config":{}} + {"filter": "shuffle", "config": {}} ) if has_country: params['filters'].append( diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index 10ae5d3..fd0d31f 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -129,7 +129,7 @@ class TestNs1Provider(TestCase): 'type': 'A', 'ttl': 34, 'short_answers': ['101.102.103.104', '101.102.103.105'], - 'domain': 'geo.unit.tests.', + 'domain': 'geo.unit.tests', }, { 'type': 'CNAME', 'ttl': 34, @@ -205,11 +205,25 @@ class TestNs1Provider(TestCase): nsone_zone = DummyZone([]) load_mock.side_effect = [nsone_zone] zone_search = Mock() - zone_search.return_value = [] + zone_search.return_value = [ + { + "domain": "geo.unit.tests", + "zone": "unit.tests", + "type": "A", + "answers": [ + {'answer': ['1.1.1.1'], 'meta': {}}, + {'answer': ['1.2.3.4'], 'meta': {'ca_province': ['ON']}}, + {'answer': ['2.3.4.5'], 'meta': {'us_state': ['NY']}}, + {'answer': ['3.4.5.6'], 'meta': {'country': ['US']}}, + {'answer': ['4.5.6.7'], 'meta': {'iso_region_code': ['NA-US-WA']}}, + ], + 'ttl': 34, + }, + ] nsone_zone.search = zone_search zone = Zone('unit.tests.', []) provider.populate(zone) - self.assertEquals(set(), zone.records) + self.assertEquals(1, len(zone.records)) self.assertEquals(('unit.tests',), load_mock.call_args[0]) # Existing zone w/records @@ -217,7 +231,21 @@ class TestNs1Provider(TestCase): nsone_zone = DummyZone(self.nsone_records) load_mock.side_effect = [nsone_zone] zone_search = Mock() - zone_search.return_value = [] + zone_search.return_value = [ + { + "domain": "geo.unit.tests", + "zone": "unit.tests", + "type": "A", + "answers": [ + {'answer': ['1.1.1.1'], 'meta': {}}, + {'answer': ['1.2.3.4'], 'meta': {'ca_province': ['ON']}}, + {'answer': ['2.3.4.5'], 'meta': {'us_state': ['NY']}}, + {'answer': ['3.4.5.6'], 'meta': {'country': ['US']}}, + {'answer': ['4.5.6.7'], 'meta': {'iso_region_code': ['NA-US-WA']}}, + ], + 'ttl': 34, + }, + ] nsone_zone.search = zone_search zone = Zone('unit.tests.', []) provider.populate(zone) @@ -285,7 +313,21 @@ 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 = [] + zone_search.return_value = [ + { + "domain": "geo.unit.tests", + "zone": "unit.tests", + "type": "A", + "answers": [ + {'answer': ['1.1.1.1'], 'meta': {}}, + {'answer': ['1.2.3.4'], 'meta': {'ca_province': ['ON']}}, + {'answer': ['2.3.4.5'], 'meta': {'us_state': ['NY']}}, + {'answer': ['3.4.5.6'], 'meta': {'country': ['US']}}, + {'answer': ['4.5.6.7'], 'meta': {'iso_region_code': ['NA-US-WA']}}, + ], + 'ttl': 34, + }, + ] nsone_zone.search = zone_search load_mock.side_effect = [nsone_zone, nsone_zone] plan = provider.plan(desired) @@ -317,8 +359,10 @@ class TestNs1Provider(TestCase): ]) mock_record.assert_has_calls([ call.update(answers=[{'answer': [u'1.2.3.4'], 'meta': {}}], + filters=[], ttl=32), call.update(answers=[{u'answer': [u'1.2.3.4'], u'meta': {}}], + filters=[], ttl=32), call.update( answers=[ @@ -331,6 +375,11 @@ class TestNs1Provider(TestCase): }, }, ], + filters=[ + {u'filter': u'shuffle', u'config': {}}, + {u'filter': u'geotarget_country', u'config': {}}, + {u'filter': u'select_first_n', u'config': {u'N': 1}}, + ], ttl=34), call.delete(), call.delete() From b06c14deaedc5eef97d9d58907b3a40549158065 Mon Sep 17 00:00:00 2001 From: Steve Coursen Date: Mon, 8 Jan 2018 12:28:25 -0500 Subject: [PATCH 12/17] Fix E501 line too long --- tests/test_octodns_provider_ns1.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index fd0d31f..0e3a286 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -212,10 +212,12 @@ class TestNs1Provider(TestCase): "type": "A", "answers": [ {'answer': ['1.1.1.1'], 'meta': {}}, - {'answer': ['1.2.3.4'], 'meta': {'ca_province': ['ON']}}, + {'answer': ['1.2.3.4'], + 'meta': {'ca_province': ['ON']}}, {'answer': ['2.3.4.5'], 'meta': {'us_state': ['NY']}}, {'answer': ['3.4.5.6'], 'meta': {'country': ['US']}}, - {'answer': ['4.5.6.7'], 'meta': {'iso_region_code': ['NA-US-WA']}}, + {'answer': ['4.5.6.7'], + 'meta': {'iso_region_code': ['NA-US-WA']}}, ], 'ttl': 34, }, @@ -238,10 +240,12 @@ class TestNs1Provider(TestCase): "type": "A", "answers": [ {'answer': ['1.1.1.1'], 'meta': {}}, - {'answer': ['1.2.3.4'], 'meta': {'ca_province': ['ON']}}, + {'answer': ['1.2.3.4'], + 'meta': {'ca_province': ['ON']}}, {'answer': ['2.3.4.5'], 'meta': {'us_state': ['NY']}}, {'answer': ['3.4.5.6'], 'meta': {'country': ['US']}}, - {'answer': ['4.5.6.7'], 'meta': {'iso_region_code': ['NA-US-WA']}}, + {'answer': ['4.5.6.7'], + 'meta': {'iso_region_code': ['NA-US-WA']}}, ], 'ttl': 34, }, @@ -320,10 +324,12 @@ class TestNs1Provider(TestCase): "type": "A", "answers": [ {'answer': ['1.1.1.1'], 'meta': {}}, - {'answer': ['1.2.3.4'], 'meta': {'ca_province': ['ON']}}, + {'answer': ['1.2.3.4'], + 'meta': {'ca_province': ['ON']}}, {'answer': ['2.3.4.5'], 'meta': {'us_state': ['NY']}}, {'answer': ['3.4.5.6'], 'meta': {'country': ['US']}}, - {'answer': ['4.5.6.7'], 'meta': {'iso_region_code': ['NA-US-WA']}}, + {'answer': ['4.5.6.7'], + 'meta': {'iso_region_code': ['NA-US-WA']}}, ], 'ttl': 34, }, From 154ca64038f9f0d3fe981a93e3a8cc42eb0762b9 Mon Sep 17 00:00:00 2001 From: Steve Coursen Date: Mon, 8 Jan 2018 20:13:20 -0500 Subject: [PATCH 13/17] Fix serialization of multiple answers, that had caused a ResourceException --- octodns/provider/ns1.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 20fffc3..a675ed9 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -215,12 +215,13 @@ class Ns1Provider(BaseProvider): if not has_country and \ len(value.split('-')) > 1: # pragma: nocover has_country = True - params['answers'].append( - { - 'answer': target.values, - 'meta': {key: [value]}, - }, - ) + for answer in target.values: + params['answers'].append( + { + 'answer': [answer], + 'meta': {key: [value]}, + }, + ) params['filters'] = [] if len(params['answers']) > 1: params['filters'].append( From dcdde5db5dc9dbcbd3edf5840f1e1090de69aba6 Mon Sep 17 00:00:00 2001 From: Steve Coursen Date: Mon, 8 Jan 2018 21:46:59 -0500 Subject: [PATCH 14/17] Handle multiple answers correctly when dersializing --- octodns/provider/ns1.py | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index a675ed9..1b48480 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -7,6 +7,7 @@ from __future__ import absolute_import, division, print_function, \ from logging import getLogger from itertools import chain +from collections import OrderedDict from nsone import NSONE from nsone.rest.errors import RateLimitException, ResourceException from incf.countryutils import transformations @@ -60,16 +61,34 @@ class Ns1Provider(BaseProvider): for cntry in country: cn = transformations.cc_to_cn(cntry) con = transformations.cn_to_ctca2(cn) - geo['{}-{}'.format(con, cntry)] = answer['answer'] + key = '{}-{}'.format(con, cntry) + if key not in geo: + geo[key] = answer['answer'] + else: + geo[key].extend(answer['answer']) for state in us_state: - geo['NA-US-{}'.format(state)] = answer['answer'] + key = 'NA-US-{}'.format(state) + if key not in geo: + geo[key] = answer['answer'] + else: + geo[key].extend(answer['answer']) for province in ca_province: - geo['NA-CA-{}'.format(province)] = answer['answer'] + key = 'NA-CA-{}'.format(province) + if key not in geo: + geo[key] = answer['answer'] + else: + geo[key].extend(answer['answer']) for code in meta.get('iso_region_code', []): - geo[code] = answer['answer'] + key = code + if key not in geo: + geo[key] = answer['answer'] + else: + geo[key].extend(answer['answer']) else: values.extend(answer['answer']) codes.append([]) + values = [str(x) for x in values] + geo = OrderedDict({str(k): [str(x) for x in v] for k, v in sorted(geo.items())}) data['values'] = values data['geo'] = geo return data From 241e6cc0ce9b3c558d42d01c4a7df111c11c7834 Mon Sep 17 00:00:00 2001 From: Steve Coursen Date: Mon, 8 Jan 2018 21:57:13 -0500 Subject: [PATCH 15/17] E501 trim lines --- octodns/provider/ns1.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 1b48480..72c8b08 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -26,8 +26,8 @@ class Ns1Provider(BaseProvider): api_key: env/NS1_API_KEY ''' SUPPORTS_GEO = True - SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'MX', 'NAPTR', 'NS', - 'PTR', 'SPF', 'SRV', 'TXT')) + SUPPORTS = set(('A', 'AAAA', 'ALIAS', 'CAA', 'CNAME', 'MX', 'NAPTR', + 'NS', 'PTR', 'SPF', 'SRV', 'TXT')) ZONE_NOT_FOUND_MESSAGE = 'server error: zone not found' @@ -88,7 +88,9 @@ class Ns1Provider(BaseProvider): values.extend(answer['answer']) codes.append([]) values = [str(x) for x in values] - geo = OrderedDict({str(k): [str(x) for x in v] for k, v in sorted(geo.items())}) + geo = OrderedDict( + {str(k): [str(x) for x in v] for k, v in geo.items()} + ) data['values'] = values data['geo'] = geo return data @@ -192,7 +194,8 @@ class Ns1Provider(BaseProvider): } def populate(self, zone, target=False, lenient=False): - self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, + self.log.debug('populate: name=%s, target=%s, lenient=%s', + zone.name, target, lenient) try: @@ -261,9 +264,9 @@ class Ns1Provider(BaseProvider): _params_for_NS = _params_for_A def _params_for_SPF(self, record): - # NS1 seems to be the only provider that doesn't want things escaped in - # values so we have to strip them here and add them when going the - # other way + # NS1 seems to be the only provider that doesn't want things + # escaped in values so we have to strip them here and add + # them when going the other way values = [v.replace('\;', ';') for v in record.values] return {'answers': values, 'ttl': record.ttl} @@ -355,4 +358,5 @@ class Ns1Provider(BaseProvider): for change in changes: class_name = change.__class__.__name__ - getattr(self, '_apply_{}'.format(class_name))(nsone_zone, change) + getattr(self, '_apply_{}'.format(class_name))(nsone_zone, + change) From d8ba6a2b41685fa0bcb7cfe69d9d5984e65d63b3 Mon Sep 17 00:00:00 2001 From: Steve Coursen Date: Mon, 8 Jan 2018 22:02:46 -0500 Subject: [PATCH 16/17] slight code cleanup, coverage increase --- octodns/provider/ns1.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 72c8b08..7889386 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -63,27 +63,23 @@ class Ns1Provider(BaseProvider): con = transformations.cn_to_ctca2(cn) key = '{}-{}'.format(con, cntry) if key not in geo: - geo[key] = answer['answer'] - else: - geo[key].extend(answer['answer']) + geo[key] = [] + geo[key].extend(answer['answer']) for state in us_state: key = 'NA-US-{}'.format(state) if key not in geo: - geo[key] = answer['answer'] - else: - geo[key].extend(answer['answer']) + geo[key] = [] + geo[key].extend(answer['answer']) for province in ca_province: key = 'NA-CA-{}'.format(province) if key not in geo: - geo[key] = answer['answer'] - else: - geo[key].extend(answer['answer']) + geo[key] = [] + geo[key].extend(answer['answer']) for code in meta.get('iso_region_code', []): key = code if key not in geo: - geo[key] = answer['answer'] - else: - geo[key].extend(answer['answer']) + geo[key] = [] + geo[key].extend(answer['answer']) else: values.extend(answer['answer']) codes.append([]) From 9785e40688a9eb16bd0f8e927a5d312c349afd9a Mon Sep 17 00:00:00 2001 From: Steve Coursen Date: Mon, 8 Jan 2018 22:04:42 -0500 Subject: [PATCH 17/17] use defaultdict --- octodns/provider/ns1.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 7889386..a68b759 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -7,7 +7,7 @@ from __future__ import absolute_import, division, print_function, \ from logging import getLogger from itertools import chain -from collections import OrderedDict +from collections import OrderedDict, defaultdict from nsone import NSONE from nsone.rest.errors import RateLimitException, ResourceException from incf.countryutils import transformations @@ -40,7 +40,7 @@ class Ns1Provider(BaseProvider): def _data_for_A(self, _type, record): # record meta (which would include geo information is only # returned when getting a record's detail, not from zone detail - geo = {} + geo = defaultdict(list) data = { 'ttl': record['ttl'], 'type': _type, @@ -62,23 +62,15 @@ class Ns1Provider(BaseProvider): cn = transformations.cc_to_cn(cntry) con = transformations.cn_to_ctca2(cn) key = '{}-{}'.format(con, cntry) - if key not in geo: - geo[key] = [] geo[key].extend(answer['answer']) for state in us_state: key = 'NA-US-{}'.format(state) - if key not in geo: - geo[key] = [] geo[key].extend(answer['answer']) for province in ca_province: key = 'NA-CA-{}'.format(province) - if key not in geo: - geo[key] = [] geo[key].extend(answer['answer']) for code in meta.get('iso_region_code', []): key = code - if key not in geo: - geo[key] = [] geo[key].extend(answer['answer']) else: values.extend(answer['answer'])