diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index b737a19..19b33af 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -38,7 +38,9 @@ class Ns1Provider(BaseProvider): self.log = getLogger('Ns1Provider[{}]'.format(id)) self.log.debug('__init__: id=%s, api_key=***', id) super(Ns1Provider, self).__init__(id, *args, **kwargs) - self._client = NS1(apiKey=api_key) + client = NS1(apiKey=api_key) + self._records = client.records() + self._zones = client.zones() def _data_for_A(self, _type, record): # record meta (which would include geo information is only @@ -189,18 +191,29 @@ class Ns1Provider(BaseProvider): target, lenient) try: - nsone_zone = self._client.loadZone(zone.name[:-1]) - records = nsone_zone.data['records'] + nsone_zone_name = zone.name[:-1] + nsone_zone = self._zones.retrieve(nsone_zone_name) + + records = [] + geo_records = [] # change answers for certain types to always be absolute - for record in records: + for record in nsone_zone['records']: if record['type'] in ['ALIAS', 'CNAME', 'MX', 'NS', 'PTR', 'SRV']: for i, a in enumerate(record['short_answers']): if not a.endswith('.'): record['short_answers'][i] = '{}.'.format(a) - geo_records = nsone_zone.search(has_geo=True) + if record.get('tier', 1) > 1: + # Need to get the full record data for geo records + record = self._records.retrieve(nsone_zone_name, + record['domain'], + record['type']) + geo_records.append(record) + else: + records.append(record) + exists = True except ResourceException as e: if e.message != self.ZONE_NOT_FOUND_MESSAGE: @@ -299,53 +312,49 @@ class Ns1Provider(BaseProvider): for v in record.values] return {'answers': values, 'ttl': record.ttl} - def _get_name(self, record): - return record.fqdn[:-1] if record.name == '' else record.name - def _apply_Create(self, nsone_zone, change): new = change.new - name = self._get_name(new) + zone = new.zone.name[:-1] + domain = new.fqdn[:-1] _type = new._type params = getattr(self, '_params_for_{}'.format(_type))(new) - meth = getattr(nsone_zone, 'add_{}'.format(_type)) try: - meth(name, **params) + self._records.create(zone, domain, _type, **params) except RateLimitException as e: period = float(e.period) self.log.warn('_apply_Create: rate limit encountered, pausing ' 'for %ds and trying again', period) sleep(period) - meth(name, **params) + self._records.create(zone, domain, _type, **params) def _apply_Update(self, nsone_zone, change): - existing = change.existing - name = self._get_name(existing) - _type = existing._type - record = nsone_zone.loadRecord(name, _type) new = change.new + zone = new.zone.name[:-1] + domain = new.fqdn[:-1] + _type = new._type params = getattr(self, '_params_for_{}'.format(_type))(new) try: - record.update(**params) + self._records.update(zone, domain, _type, **params) except RateLimitException as e: period = float(e.period) self.log.warn('_apply_Update: rate limit encountered, pausing ' 'for %ds and trying again', period) sleep(period) - record.update(**params) + self._records.update(zone, domain, _type, **params) def _apply_Delete(self, nsone_zone, change): existing = change.existing - name = self._get_name(existing) + zone = existing.zone.name[:-1] + domain = existing.fqdn[:-1] _type = existing._type - record = nsone_zone.loadRecord(name, _type) try: - record.delete() + self._records.delete(zone, domain, _type) except RateLimitException as e: period = float(e.period) self.log.warn('_apply_Delete: rate limit encountered, pausing ' 'for %ds and trying again', period) sleep(period) - record.delete() + self._records.delete(zone, domain, _type) def _apply(self, plan): desired = plan.desired @@ -355,12 +364,12 @@ class Ns1Provider(BaseProvider): domain_name = desired.name[:-1] try: - nsone_zone = self._client.loadZone(domain_name) + nsone_zone = self._zones.retrieve(domain_name) except ResourceException as e: if e.message != self.ZONE_NOT_FOUND_MESSAGE: raise self.log.debug('_apply: no matching zone, creating') - nsone_zone = self._client.createZone(domain_name) + nsone_zone = self._zones.create(domain_name) for change in changes: class_name = change.__class__.__name__ diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index 7ef182c..9034552 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -6,7 +6,7 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals from collections import defaultdict -from mock import Mock, call, patch +from mock import call, patch from ns1.rest.errors import AuthException, RateLimitException, \ ResourceException from unittest import TestCase @@ -16,14 +16,6 @@ from octodns.provider.ns1 import Ns1Provider from octodns.zone import Zone -class DummyZone(object): - - def __init__(self, records): - self.data = { - 'records': records - } - - class TestNs1Provider(TestCase): zone = Zone('unit.tests.', []) expected = set() @@ -172,43 +164,40 @@ class TestNs1Provider(TestCase): 'domain': 'unit.tests.', }] - @patch('ns1.NS1.loadZone') - def test_populate(self, load_mock): + @patch('ns1.rest.zones.Zones.retrieve') + def test_populate(self, zone_retrieve_mock): provider = Ns1Provider('test', 'api-key') # Bad auth - load_mock.side_effect = AuthException('unauthorized') + zone_retrieve_mock.side_effect = AuthException('unauthorized') zone = Zone('unit.tests.', []) with self.assertRaises(AuthException) as ctx: provider.populate(zone) - self.assertEquals(load_mock.side_effect, ctx.exception) + self.assertEquals(zone_retrieve_mock.side_effect, ctx.exception) # General error - load_mock.reset_mock() - load_mock.side_effect = ResourceException('boom') + zone_retrieve_mock.reset_mock() + zone_retrieve_mock.side_effect = ResourceException('boom') zone = Zone('unit.tests.', []) with self.assertRaises(ResourceException) as ctx: provider.populate(zone) - self.assertEquals(load_mock.side_effect, ctx.exception) - self.assertEquals(('unit.tests',), load_mock.call_args[0]) + self.assertEquals(zone_retrieve_mock.side_effect, ctx.exception) + self.assertEquals(('unit.tests',), zone_retrieve_mock.call_args[0]) # Non-existent zone doesn't populate anything - load_mock.reset_mock() - load_mock.side_effect = \ + zone_retrieve_mock.reset_mock() + zone_retrieve_mock.side_effect = \ ResourceException('server error: zone not found') zone = Zone('unit.tests.', []) exists = provider.populate(zone) self.assertEquals(set(), zone.records) - self.assertEquals(('unit.tests',), load_mock.call_args[0]) + self.assertEquals(('unit.tests',), zone_retrieve_mock.call_args[0]) self.assertFalse(exists) # Existing zone w/o records - load_mock.reset_mock() - nsone_zone = DummyZone([]) - load_mock.side_effect = [nsone_zone] - zone_search = Mock() - zone_search.return_value = [ - { + zone_retrieve_mock.reset_mock() + nsone_zone = { + 'records': [{ "domain": "geo.unit.tests", "zone": "unit.tests", "type": "A", @@ -222,21 +211,18 @@ class TestNs1Provider(TestCase): 'meta': {'iso_region_code': ['NA-US-WA']}}, ], 'ttl': 34, - }, - ] - nsone_zone.search = zone_search + }], + } + zone_retrieve_mock.side_effect = [nsone_zone] zone = Zone('unit.tests.', []) provider.populate(zone) self.assertEquals(1, len(zone.records)) - self.assertEquals(('unit.tests',), load_mock.call_args[0]) + self.assertEquals(('unit.tests',), zone_retrieve_mock.call_args[0]) # Existing zone w/records - load_mock.reset_mock() - nsone_zone = DummyZone(self.nsone_records) - load_mock.side_effect = [nsone_zone] - zone_search = Mock() - zone_search.return_value = [ - { + zone_retrieve_mock.reset_mock() + nsone_zone = { + 'records': self.nsone_records + [{ "domain": "geo.unit.tests", "zone": "unit.tests", "type": "A", @@ -250,26 +236,23 @@ class TestNs1Provider(TestCase): 'meta': {'iso_region_code': ['NA-US-WA']}}, ], 'ttl': 34, - }, - ] - nsone_zone.search = zone_search + }], + } + zone_retrieve_mock.side_effect = [nsone_zone] zone = Zone('unit.tests.', []) provider.populate(zone) self.assertEquals(self.expected, zone.records) - self.assertEquals(('unit.tests',), load_mock.call_args[0]) + self.assertEquals(('unit.tests',), zone_retrieve_mock.call_args[0]) # Test skipping unsupported record type - load_mock.reset_mock() - nsone_zone = DummyZone(self.nsone_records + [{ - 'type': 'UNSUPPORTED', - 'ttl': 42, - 'short_answers': ['unsupported'], - 'domain': 'unsupported.unit.tests.', - }]) - load_mock.side_effect = [nsone_zone] - zone_search = Mock() - zone_search.return_value = [ - { + zone_retrieve_mock.reset_mock() + nsone_zone = { + 'records': self.nsone_records + [{ + 'type': 'UNSUPPORTED', + 'ttl': 42, + 'short_answers': ['unsupported'], + 'domain': 'unsupported.unit.tests.', + }, { "domain": "geo.unit.tests", "zone": "unit.tests", "type": "A", @@ -283,17 +266,23 @@ class TestNs1Provider(TestCase): 'meta': {'iso_region_code': ['NA-US-WA']}}, ], 'ttl': 34, - }, - ] - nsone_zone.search = zone_search + }], + } + zone_retrieve_mock.side_effect = [nsone_zone] zone = Zone('unit.tests.', []) provider.populate(zone) self.assertEquals(self.expected, zone.records) - self.assertEquals(('unit.tests',), load_mock.call_args[0]) + self.assertEquals(('unit.tests',), zone_retrieve_mock.call_args[0]) - @patch('ns1.NS1.createZone') - @patch('ns1.NS1.loadZone') - def test_sync(self, load_mock, create_mock): + @patch('ns1.rest.records.Records.delete') + @patch('ns1.rest.records.Records.update') + @patch('ns1.rest.records.Records.create') + @patch('ns1.rest.records.Records.retrieve') + @patch('ns1.rest.zones.Zones.create') + @patch('ns1.rest.zones.Zones.retrieve') + def test_sync(self, zone_retrieve_mock, zone_create_mock, + record_retrieve_mock, record_create_mock, + record_update_mock, record_delete_mock): provider = Ns1Provider('test', 'api-key') desired = Zone('unit.tests.', []) @@ -307,71 +296,98 @@ class TestNs1Provider(TestCase): self.assertTrue(plan.exists) # Fails, general error - load_mock.reset_mock() - create_mock.reset_mock() - load_mock.side_effect = ResourceException('boom') + zone_retrieve_mock.reset_mock() + zone_create_mock.reset_mock() + zone_retrieve_mock.side_effect = ResourceException('boom') with self.assertRaises(ResourceException) as ctx: provider.apply(plan) - self.assertEquals(load_mock.side_effect, ctx.exception) + self.assertEquals(zone_retrieve_mock.side_effect, ctx.exception) # Fails, bad auth - load_mock.reset_mock() - create_mock.reset_mock() - load_mock.side_effect = \ + zone_retrieve_mock.reset_mock() + zone_create_mock.reset_mock() + zone_retrieve_mock.side_effect = \ ResourceException('server error: zone not found') - create_mock.side_effect = AuthException('unauthorized') + zone_create_mock.side_effect = AuthException('unauthorized') with self.assertRaises(AuthException) as ctx: provider.apply(plan) - self.assertEquals(create_mock.side_effect, ctx.exception) + self.assertEquals(zone_create_mock.side_effect, ctx.exception) # non-existent zone, create - load_mock.reset_mock() - create_mock.reset_mock() - load_mock.side_effect = \ + zone_retrieve_mock.reset_mock() + zone_create_mock.reset_mock() + zone_retrieve_mock.side_effect = \ ResourceException('server error: zone not found') - # ugh, need a mock zone with a mock prop since we're using getattr, we - # can actually control side effects on `meth` with that. - mock_zone = Mock() - mock_zone.add_SRV = Mock() - mock_zone.add_SRV.side_effect = [ + + zone_create_mock.side_effect = ['foo'] + # Test out the create rate-limit handling, then 9 successes + record_create_mock.side_effect = [ RateLimitException('boo', period=0), - None, - ] - create_mock.side_effect = [mock_zone] + ] + ([None] * 9) + got_n = provider.apply(plan) self.assertEquals(expected_n, got_n) + # Zone was created + zone_create_mock.assert_has_calls([call('unit.tests')]) + # Checking that we got some of the expected records too + record_create_mock.assert_has_calls([ + call('unit.tests', 'unit.tests', 'A', answers=[ + {'answer': ['1.2.3.4'], 'meta': {}} + ], filters=[], ttl=32), + call('unit.tests', 'unit.tests', 'CAA', answers=[ + (0, 'issue', 'ca.unit.tests') + ], ttl=40), + call('unit.tests', 'unit.tests', 'MX', answers=[ + (10, 'mx1.unit.tests.'), (20, 'mx2.unit.tests.') + ], ttl=35), + ]) + # Update & delete - load_mock.reset_mock() - create_mock.reset_mock() - nsone_zone = DummyZone(self.nsone_records + [{ - 'type': 'A', - 'ttl': 42, - 'short_answers': ['9.9.9.9'], - 'domain': 'delete-me.unit.tests.', - }]) - nsone_zone.data['records'][0]['short_answers'][0] = '2.2.2.2' - nsone_zone.loadRecord = Mock() - zone_search = Mock() - zone_search.return_value = [ - { + zone_retrieve_mock.reset_mock() + zone_create_mock.reset_mock() + + nsone_zone = { + 'records': self.nsone_records + [{ + 'type': 'A', + 'ttl': 42, + 'short_answers': ['9.9.9.9'], + 'domain': 'delete-me.unit.tests.', + }, { "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']}}, + "short_answers": [ + '1.1.1.1', + '1.2.3.4', + '2.3.4.5', + '3.4.5.6', + '4.5.6.7', ], + 'tier': 3, # This flags it as advacned, full load required 'ttl': 34, - }, - ] - nsone_zone.search = zone_search - load_mock.side_effect = [nsone_zone, nsone_zone] + }], + } + nsone_zone['records'][0]['short_answers'][0] = '2.2.2.2' + + record_retrieve_mock.side_effect = [{ + "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']}}, + ], + 'tier': 3, + 'ttl': 34, + }] + + zone_retrieve_mock.side_effect = [nsone_zone, nsone_zone] plan = provider.plan(desired) self.assertEquals(3, len(plan.changes)) # Shouldn't rely on order so just count classes @@ -380,55 +396,42 @@ class TestNs1Provider(TestCase): classes[change.__class__] += 1 self.assertEquals(1, classes[Delete]) self.assertEquals(2, classes[Update]) - # 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 - mock_record = Mock() - mock_record.update.side_effect = [ + + record_update_mock.side_effect = [ RateLimitException('one', period=0), None, None, ] - mock_record.delete.side_effect = [ + record_delete_mock.side_effect = [ RateLimitException('two', period=0), None, None, ] - 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([ - call('unit.tests', u'A'), - call('delete-me', u'A'), - call('geo', u'A'), - ]) - 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.delete(), - call.delete(), - 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'] - }, - }, - ], + + record_update_mock.assert_has_calls([ + call('unit.tests', 'unit.tests', 'A', answers=[ + {'answer': ['1.2.3.4'], 'meta': {}}], + filters=[], + ttl=32), + call('unit.tests', 'unit.tests', 'A', answers=[ + {'answer': ['1.2.3.4'], 'meta': {}}], + filters=[], + ttl=32), + call('unit.tests', 'geo.unit.tests', 'A', answers=[ + {'answer': ['101.102.103.104'], 'meta': {}}, + {'answer': ['101.102.103.105'], 'meta': {}}, + { + 'answer': ['201.202.203.204'], + 'meta': {'iso_region_code': ['NA-US-NY']} + }], 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), + {'filter': 'shuffle', 'config': {}}, + {'filter': 'geotarget_country', 'config': {}}, + {'filter': 'select_first_n', 'config': {'N': 1}}], + ttl=34) ]) def test_escaping(self):