From bc1736bc39f2f2bb87373b0b1a0e3adaca5fad5f Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Tue, 23 May 2017 09:36:15 -0700 Subject: [PATCH] NS1, add Delete support, fix apply create, flush out tests to 100% --- octodns/provider/ns1.py | 14 +- tests/test_octodns_provider_ns1.py | 299 ++++++++++++++++++----------- 2 files changed, 201 insertions(+), 112 deletions(-) diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 2982c5f..8d168f6 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -22,6 +22,7 @@ class Ns1Provider(BaseProvider): api_key: env/NS1_API_KEY ''' SUPPORTS_GEO = False + ZONE_NOT_FOUND_MESSAGE = 'server error: zone not found' def __init__(self, id, api_key, *args, **kwargs): self.log = getLogger('Ns1Provider[{}]'.format(id)) @@ -113,7 +114,7 @@ class Ns1Provider(BaseProvider): nsone_zone = self._client.loadZone(zone.name[:-1]) records = nsone_zone.data['records'] except ResourceException as e: - if e.message != 'server error: zone not found': + if e.message != self.ZONE_NOT_FOUND_MESSAGE: raise records = [] @@ -174,6 +175,13 @@ class Ns1Provider(BaseProvider): params = getattr(self, '_params_for_{}'.format(_type))(new) record.update(**params) + def _apply_Delete(self, nsone_zone, change): + existing = change.existing + name = self._get_name(existing) + _type = existing._type + record = nsone_zone.loadRecord(name, _type) + record.delete() + def _apply(self, plan): desired = plan.desired changes = plan.changes @@ -183,7 +191,9 @@ class Ns1Provider(BaseProvider): domain_name = desired.name[:-1] try: nsone_zone = self._client.loadZone(domain_name) - except ResourceException: + 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) diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index 110c0c1..acb0125 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -5,11 +5,11 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals -from mock import patch +from mock import Mock, call, patch from nsone.rest.errors import AuthException, ResourceException from unittest import TestCase -from octodns.record import Record +from octodns.record import Delete, Record, Update from octodns.provider.ns1 import Ns1Provider from octodns.zone import Zone @@ -23,9 +23,127 @@ class DummyZone(object): class TestNs1Provider(TestCase): + zone = Zone('unit.tests.', []) + expected = set() + expected.add(Record.new(zone, '', { + 'ttl': 32, + 'type': 'A', + 'value': '1.2.3.4', + })) + expected.add(Record.new(zone, 'foo', { + 'ttl': 33, + 'type': 'A', + 'values': ['1.2.3.4', '1.2.3.5'], + })) + expected.add(Record.new(zone, 'cname', { + 'ttl': 34, + 'type': 'CNAME', + 'value': 'foo.unit.tests.', + })) + expected.add(Record.new(zone, '', { + 'ttl': 35, + 'type': 'MX', + 'values': [{ + 'priority': 10, + 'value': 'mx1.unit.tests.', + }, { + 'priority': 20, + 'value': 'mx2.unit.tests.', + }] + })) + expected.add(Record.new(zone, 'naptr', { + 'ttl': 36, + 'type': 'NAPTR', + 'values': [{ + 'flags': 'U', + 'order': 100, + 'preference': 100, + 'regexp': '!^.*$!sip:info@bar.example.com!', + 'replacement': '.', + 'service': 'SIP+D2U', + }, { + 'flags': 'S', + 'order': 10, + 'preference': 100, + 'regexp': '!^.*$!sip:info@bar.example.com!', + 'replacement': '.', + 'service': 'SIP+D2U', + }] + })) + expected.add(Record.new(zone, '', { + 'ttl': 37, + 'type': 'NS', + 'values': ['ns1.unit.tests.', 'ns2.unit.tests.'], + })) + expected.add(Record.new(zone, '_srv._tcp', { + 'ttl': 38, + 'type': 'SRV', + 'values': [{ + 'priority': 10, + 'weight': 20, + 'port': 30, + 'target': 'foo-1.unit.tests.', + }, { + 'priority': 12, + 'weight': 30, + 'port': 30, + 'target': 'foo-2.unit.tests.', + }] + })) + expected.add(Record.new(zone, 'sub', { + 'ttl': 39, + 'type': 'NS', + 'values': ['ns3.unit.tests.', 'ns4.unit.tests.'], + })) + + nsone_records = [{ + 'type': 'A', + 'ttl': 32, + 'short_answers': ['1.2.3.4'], + 'domain': 'unit.tests.', + }, { + 'type': 'A', + 'ttl': 33, + 'short_answers': ['1.2.3.4', '1.2.3.5'], + 'domain': 'foo.unit.tests.', + }, { + 'type': 'CNAME', + 'ttl': 34, + 'short_answers': ['foo.unit.tests.'], + 'domain': 'cname.unit.tests.', + }, { + 'type': 'MX', + 'ttl': 35, + 'short_answers': ['10 mx1.unit.tests.', '20 mx2.unit.tests.'], + 'domain': 'unit.tests.', + }, { + 'type': 'NAPTR', + 'ttl': 36, + 'short_answers': [ + '10 100 S SIP+D2U !^.*$!sip:info@bar.example.com! .', + '100 100 U SIP+D2U !^.*$!sip:info@bar.example.com! .' + ], + 'domain': 'naptr.unit.tests.', + }, { + 'type': 'NS', + 'ttl': 37, + 'short_answers': ['ns1.unit.tests.', 'ns2.unit.tests.'], + 'domain': 'unit.tests.', + }, { + 'type': 'SRV', + 'ttl': 38, + 'short_answers': ['12 30 30 foo-2.unit.tests.', + '10 20 30 foo-1.unit.tests.'], + 'domain': '_srv._tcp.unit.tests.', + }, { + 'type': 'NS', + 'ttl': 39, + 'short_answers': ['ns3.unit.tests.', 'ns4.unit.tests.'], + 'domain': 'sub.unit.tests.', + }] @patch('nsone.NSONE.loadZone') - def test_provider(self, load_mock): + def test_populate(self, load_mock): provider = Ns1Provider('test', 'api-key') # Bad auth @@ -64,114 +182,75 @@ class TestNs1Provider(TestCase): # Existing zone w/records load_mock.reset_mock() - nsone_zone = DummyZone([{ - 'type': 'A', - 'ttl': 32, - 'short_answers': ['1.2.3.4'], - 'domain': 'unit.tests.', - }, { - 'type': 'A', - 'ttl': 33, - 'short_answers': ['1.2.3.4', '1.2.3.5'], - 'domain': 'foo.unit.tests.', - }, { - 'type': 'CNAME', - 'ttl': 34, - 'short_answers': ['foo.unit.tests.'], - 'domain': 'cname.unit.tests.', - }, { - 'type': 'MX', - 'ttl': 35, - 'short_answers': ['10 mx1.unit.tests.', '20 mx2.unit.tests.'], - 'domain': 'unit.tests.', - }, { - 'type': 'NAPTR', - 'ttl': 36, - 'short_answers': [ - '10 100 S SIP+D2U !^.*$!sip:info@bar.example.com! .', - '100 100 U SIP+D2U !^.*$!sip:info@bar.example.com! .' - ], - 'domain': 'naptr.unit.tests.', - }, { - 'type': 'NS', - 'ttl': 37, - 'short_answers': ['ns1.unit.tests.', 'ns2.unit.tests.'], - 'domain': 'unit.tests.', - }, { - 'type': 'SRV', - 'ttl': 38, - 'short_answers': ['12 20 30 foo-2.unit.tests.', - '10 20 30 foo-2.unit.tests.'], - 'domain': '_srv._tcp.unit.tests.', - }]) + nsone_zone = DummyZone(self.nsone_records) load_mock.side_effect = [nsone_zone] zone = Zone('unit.tests.', []) provider.populate(zone) - expected = set() - expected.add(Record.new(zone, '', { - 'ttl': 32, - 'type': 'A', - 'value': '1.2.3.4', - })) - expected.add(Record.new(zone, 'foo', { - 'ttl': 32, - 'type': 'A', - 'values': ['1.2.3.4', '1.2.3.5'], - })) - expected.add(Record.new(zone, 'cname', { - 'ttl': 33, - 'type': 'CNAME', - 'value': 'foo.unit.tests.', - })) - expected.add(Record.new(zone, '', { - 'ttl': 35, - 'type': 'MX', - 'values': [{ - 'priority': 10, - 'value': 'mx1.unit.tests.', - }, { - 'priority': 20, - 'value': 'mx2.unit.tests.', - }] - })) - expected.add(Record.new(zone, 'naptr', { - 'ttl': 36, - 'type': 'NAPTR', - 'values': [{ - 'flags': 'U', - 'order': 100, - 'preference': 100, - 'regexp': '!^.*$!sip:info@bar.example.com!', - 'replacement': '.', - 'service': 'SIP+D2U', - }, { - 'flags': 'S', - 'order': 10, - 'preference': 100, - 'regexp': '!^.*$!sip:info@bar.example.com!', - 'replacement': '.', - 'service': 'SIP+D2U', - }] - })) - expected.add(Record.new(zone, '', { - 'ttl': 37, - 'type': 'NS', - 'values': ['ns1.unit.tests.', 'ns2.unit.tests.'], - })) - expected.add(Record.new(zone, '_srv._tcp', { - 'ttl': 38, - 'type': 'SRV', - 'values': [{ - 'priority': 10, - 'weight': 20, - 'port': 30, - 'target': 'foo-1.unit.tests.', - }, { - 'priority': 12, - 'weight': 30, - 'port': 30, - 'target': 'foo-2.unit.tests.', - }] - })) - self.assertEquals(expected, zone.records) + self.assertEquals(self.expected, zone.records) self.assertEquals(('unit.tests',), load_mock.call_args[0]) + + @patch('nsone.NSONE.createZone') + @patch('nsone.NSONE.loadZone') + def test_sync(self, load_mock, create_mock): + provider = Ns1Provider('test', 'api-key') + + desired = Zone('unit.tests.', []) + desired.records.update(self.expected) + + plan = provider.plan(desired) + # everything except the root NS + expected_n = len(self.expected) - 1 + self.assertEquals(expected_n, len(plan.changes)) + + # Fails, general error + load_mock.reset_mock() + create_mock.reset_mock() + load_mock.side_effect = ResourceException('boom') + with self.assertRaises(ResourceException) as ctx: + provider.apply(plan) + self.assertEquals(load_mock.side_effect, ctx.exception) + + # Fails, bad auth + load_mock.reset_mock() + create_mock.reset_mock() + load_mock.side_effect = \ + ResourceException('server error: zone not found') + create_mock.side_effect = AuthException('unauthorized') + with self.assertRaises(AuthException) as ctx: + provider.apply(plan) + self.assertEquals(create_mock.side_effect, ctx.exception) + + # non-existant zone, create + load_mock.reset_mock() + create_mock.reset_mock() + load_mock.side_effect = \ + ResourceException('server error: zone not found') + create_mock.side_effect = None + got_n = provider.apply(plan) + self.assertEquals(expected_n, got_n) + + # 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() + load_mock.side_effect = [nsone_zone, nsone_zone] + plan = provider.plan(desired) + self.assertEquals(2, len(plan.changes)) + self.assertIsInstance(plan.changes[0], Update) + self.assertIsInstance(plan.changes[1], Delete) + + got_n = provider.apply(plan) + self.assertEquals(2, got_n) + nsone_zone.loadRecord.assert_has_calls([ + call('unit.tests', u'A'), + call().update(answers=[u'1.2.3.4'], ttl=32), + call('delete-me', u'A'), + call().delete() + ])