diff --git a/octodns/provider/ns1.py b/octodns/provider/ns1.py index 0694627..a25911d 100644 --- a/octodns/provider/ns1.py +++ b/octodns/provider/ns1.py @@ -175,6 +175,9 @@ class Ns1Provider(BaseProvider): ns1: class: octodns.provider.ns1.Ns1Provider api_key: env/NS1_API_KEY + # Only required if using dynamic records + monitor_regions: + - lga ''' SUPPORTS_GEO = True SUPPORTS_DYNAMIC = True @@ -224,11 +227,13 @@ class Ns1Provider(BaseProvider): 'NA': ('US-CENTRAL', 'US-EAST', 'US-WEST'), } - def __init__(self, id, api_key, retry_count=4, *args, **kwargs): + def __init__(self, id, api_key, retry_count=4, monitor_regions=None, *args, + **kwargs): self.log = getLogger('Ns1Provider[{}]'.format(id)) - self.log.debug('__init__: id=%s, api_key=***, retry_count=%d', id, - retry_count) + self.log.debug('__init__: id=%s, api_key=***, retry_count=%d, ' + 'monitor_regions=%s', id, retry_count, monitor_regions) super(Ns1Provider, self).__init__(id, *args, **kwargs) + self.monitor_regions = monitor_regions self._client = Ns1Client(api_key, retry_count) @@ -679,8 +684,7 @@ class Ns1Provider(BaseProvider): 'policy': 'quorum', 'rapid_recheck': False, 'region_scope': 'fixed', - # TODO: what should we do here dal, sjc, lga, sin, ams - 'regions': ['lga'], + 'regions': self.monitor_regions, 'rules': [{ 'comparison': 'contains', 'key': 'output', @@ -977,12 +981,25 @@ class Ns1Provider(BaseProvider): self._client.records_delete(zone, domain, _type) self._monitors_gc(existing) + def _has_dynamic(self, changes): + for change in changes: + if getattr(change.record, 'dynamic', False): + return True + + return False + def _apply(self, plan): desired = plan.desired changes = plan.changes self.log.debug('_apply: zone=%s, len(changes)=%d', desired.name, len(changes)) + # Make sure that if we're going to make any dynamic changes that we + # have monitor_regions configured before touching anything so we can + # abort early and not half-apply + if self._has_dynamic(changes) and self.monitor_regions is None: + raise Ns1Exception('Monitored record, but monitor_regions not set') + domain_name = desired.name[:-1] try: ns1_zone = self._client.zones_retrieve(domain_name) diff --git a/tests/test_octodns_provider_ns1.py b/tests/test_octodns_provider_ns1.py index 0d456fe..bd4696b 100644 --- a/tests/test_octodns_provider_ns1.py +++ b/tests/test_octodns_provider_ns1.py @@ -14,6 +14,7 @@ from unittest import TestCase from octodns.record import Delete, Record, Update from octodns.provider.ns1 import Ns1Client, Ns1Exception, Ns1Provider +from octodns.provider.plan import Plan from octodns.zone import Zone @@ -1198,6 +1199,138 @@ class TestNs1ProviderDynamic(TestCase): self.assertFalse(extra) monitors_for_mock.assert_not_called() + def test_has_dynamic(self): + provider = Ns1Provider('test', 'api-key') + + desired = Zone('unit.tests.', []) + + simple = Record.new(desired, 'sim', { + 'ttl': 33, + 'type': 'A', + 'value': '1.2.3.4', + }) + + # Dynamic record, inspectable + dynamic = Record.new(desired, 'dyn', { + 'dynamic': { + 'pools': { + 'iad': { + 'values': [{ + 'value': '1.2.3.4', + }], + }, + }, + 'rules': [{ + 'pool': 'iad', + }], + }, + 'octodns': { + 'healthcheck': { + 'host': 'send.me', + 'path': '/_ping', + 'port': 80, + 'protocol': 'HTTP', + } + }, + 'ttl': 32, + 'type': 'A', + 'value': '1.2.3.4', + 'meta': {}, + }) + + simple_update = Update(simple, simple) + dynamic_update = Update(dynamic, dynamic) + + self.assertFalse(provider._has_dynamic([simple_update])) + self.assertTrue(provider._has_dynamic([dynamic_update])) + self.assertTrue(provider._has_dynamic([simple_update, dynamic_update])) + + @patch('octodns.provider.ns1.Ns1Client.zones_retrieve') + @patch('octodns.provider.ns1.Ns1Provider._apply_Update') + def test_apply_monitor_regions(self, apply_update_mock, + zones_retrieve_mock): + provider = Ns1Provider('test', 'api-key') + + desired = Zone('unit.tests.', []) + + simple = Record.new(desired, 'sim', { + 'ttl': 33, + 'type': 'A', + 'value': '1.2.3.4', + }) + + # Dynamic record, inspectable + dynamic = Record.new(desired, 'dyn', { + 'dynamic': { + 'pools': { + 'iad': { + 'values': [{ + 'value': '1.2.3.4', + }], + }, + }, + 'rules': [{ + 'pool': 'iad', + }], + }, + 'octodns': { + 'healthcheck': { + 'host': 'send.me', + 'path': '/_ping', + 'port': 80, + 'protocol': 'HTTP', + } + }, + 'ttl': 32, + 'type': 'A', + 'value': '1.2.3.4', + 'meta': {}, + }) + + simple_update = Update(simple, simple) + simple_plan = Plan(desired, desired, [simple_update], True) + dynamic_update = Update(dynamic, dynamic) + dynamic_plan = Plan(desired, desired, [dynamic_update], True) + both_plan = Plan(desired, desired, [simple_update, dynamic_update], + True) + + # always return foo, we aren't testing this part here + zones_retrieve_mock.side_effect = [ + 'foo', + 'foo', + 'foo', + 'foo', + ] + + # Doesn't blow up, and calls apply once + apply_update_mock.reset_mock() + provider._apply(simple_plan) + apply_update_mock.assert_has_calls([call('foo', simple_update)]) + + # Blows up and apply not called + apply_update_mock.reset_mock() + with self.assertRaises(Ns1Exception) as ctx: + provider._apply(dynamic_plan) + self.assertTrue('monitor_regions not set' in text_type(ctx.exception)) + apply_update_mock.assert_not_called() + + # Blows up and apply not called even though there's a simple + apply_update_mock.reset_mock() + with self.assertRaises(Ns1Exception) as ctx: + provider._apply(both_plan) + self.assertTrue('monitor_regions not set' in text_type(ctx.exception)) + apply_update_mock.assert_not_called() + + # with monitor_regions set + provider.monitor_regions = ['lga'] + + apply_update_mock.reset_mock() + provider._apply(both_plan) + apply_update_mock.assert_has_calls([ + call('foo', dynamic_update), + call('foo', simple_update), + ]) + class TestNs1Client(TestCase):