From e6d86696113cce6a7bec7b5d4db8c6714bf879a7 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 31 Mar 2018 14:31:15 -0700 Subject: [PATCH] Implement healthcheck protocol and port for Dyn --- octodns/provider/dyn.py | 67 +++++++++++++++++++++++------- tests/test_octodns_provider_dyn.py | 46 ++++++++++++++++---- tests/test_octodns_record.py | 19 +++++++++ 3 files changed, 110 insertions(+), 22 deletions(-) diff --git a/octodns/provider/dyn.py b/octodns/provider/dyn.py index 09329c0..1eb013b 100644 --- a/octodns/provider/dyn.py +++ b/octodns/provider/dyn.py @@ -63,15 +63,46 @@ def _monitor_path_set(self, value): DSFMonitor.path = DSFMonitor.path.setter(_monitor_path_set) -def _monitor_update(self, host, path): +def _monitor_protocol_get(self): + return self._protocol + + +DSFMonitor.protocol = property(_monitor_protocol_get) + + +def _monitor_protocol_set(self, value): + self._protocol = value + + +DSFMonitor.protocol = DSFMonitor.protocol.setter(_monitor_protocol_set) + + +def _monitor_port_get(self): + return self._port or self._options['port'] + + +DSFMonitor.port = property(_monitor_port_get) + + +def _monitor_port_set(self, value): + if self._options is None: + self._options = {} + self._port = self._options['port'] = value + + +DSFMonitor.port = DSFMonitor.port.setter(_monitor_port_set) + + +def _monitor_update(self, host, path, protocol, port): # I can't see how to actually do this with the client lib so # I'm having to hack around it. Have to provide all the # options or else things complain return self._update({ + 'protocol': protocol, 'options': { 'host': host, 'path': path, - 'port': DynProvider.MONITOR_PORT, + 'port': port, 'timeout': DynProvider.MONITOR_TIMEOUT, 'header': DynProvider.MONITOR_HEADER, } @@ -82,6 +113,11 @@ DSFMonitor.update = _monitor_update ############################################################################### +def _monitor_matches(monitor, host, path, protocol, port): + return monitor.host != host or monitor.path != path or \ + monitor.protocol != protocol or int(monitor.port) != port + + class _CachingDynZone(DynZone): log = getLogger('_CachingDynZone') @@ -142,10 +178,6 @@ class _CachingDynZone(DynZone): self.flush_cache() -def _monitor_matches(monitor, host, path): - return monitor.host != host or monitor.path != path - - class DynProvider(BaseProvider): ''' Dynect Managed DNS provider @@ -202,7 +234,6 @@ class DynProvider(BaseProvider): } MONITOR_HEADER = 'User-Agent: Dyn Monitor' - MONITOR_PORT = 443 MONITOR_TIMEOUT = 10 _sess_create_lock = Lock() @@ -479,7 +510,9 @@ class DynProvider(BaseProvider): # Heh, when pulled from the API host & path live under options, but # when you create with the constructor they're top-level :-( if _monitor_matches(monitor, record.healthcheck_host, - record.healthcheck_path): + record.healthcheck_path, + record.healthcheck_protocol, + record.healthcheck_port): self.log.info('_extra_changes: health-check mis-match for %s', label) extra.append(Update(record, record)) @@ -586,6 +619,8 @@ class DynProvider(BaseProvider): try: try: monitor = self.traffic_director_monitors[label] + self.log.debug('_traffic_director_monitor: existing for %s', + label) except KeyError: # UNTIL 1.0 We don't have one for the new label format, see if # we still have one for the old and update it @@ -597,19 +632,23 @@ class DynProvider(BaseProvider): self.traffic_director_monitors[fqdn] del self.traffic_director_monitors[fqdn] if _monitor_matches(monitor, record.healthcheck_host, - record.healthcheck_path): + record.healthcheck_path, + record.healthcheck_protocol, + record.healthcheck_port): self.log.info('_traffic_director_monitor: updating monitor ' 'for %s', label) monitor.update(record.healthcheck_host, - record.healthcheck_path) + record.healthcheck_path, + record.healthcheck_protocol, + record.healthcheck_port) return monitor except KeyError: self.log.info('_traffic_director_monitor: creating monitor ' 'for %s', label) - monitor = DSFMonitor(label, protocol='HTTPS', response_count=2, - probe_interval=60, retries=2, - port=self.MONITOR_PORT, active='Y', - host=record.healthcheck_host, + monitor = DSFMonitor(label, protocol=record.healthcheck_protocol, + response_count=2, probe_interval=60, + retries=2, port=record.healthcheck_port, + active='Y', host=record.healthcheck_host, timeout=self.MONITOR_TIMEOUT, header=self.MONITOR_HEADER, path=record.healthcheck_path) diff --git a/tests/test_octodns_provider_dyn.py b/tests/test_octodns_provider_dyn.py index 3412e34..ac56477 100644 --- a/tests/test_octodns_provider_dyn.py +++ b/tests/test_octodns_provider_dyn.py @@ -768,7 +768,9 @@ class TestDynProviderGeo(TestCase): 'octodns': { 'healthcheck': { 'host': 'bleep.bloop', - 'path': '/_nope' + 'path': '/_nope', + 'protocol': 'HTTP', + 'port': 8080, } }, 'ttl': 60, @@ -787,10 +789,10 @@ class TestDynProviderGeo(TestCase): 'header': 'User-Agent: Dyn Monitor', 'host': 'bleep.bloop', 'path': '/_nope', - 'port': '443', + 'port': '8080', 'timeout': '10', 'probe_interval': '60', - 'protocol': 'HTTPS', + 'protocol': 'HTTP', 'response_count': '2', 'retries': '2' }, @@ -806,11 +808,12 @@ class TestDynProviderGeo(TestCase): # should have resulted an update mock.assert_has_calls([ call('/DSFMonitor/42a/', 'PUT', { + 'protocol': 'HTTP', 'options': { 'path': '/_nope', 'host': 'bleep.bloop', 'header': 'User-Agent: Dyn Monitor', - 'port': 443, + 'port': 8080, 'timeout': 10 } }) @@ -821,6 +824,8 @@ class TestDynProviderGeo(TestCase): monitor = provider._traffic_director_monitors['unit.tests.:A'] self.assertEquals('bleep.bloop', monitor.host) self.assertEquals('/_nope', monitor.path) + self.assertEquals('HTTP', monitor.protocol) + self.assertEquals('8080', monitor.port) # test upgrading an old label record = Record.new(existing, 'old-label', { @@ -1510,15 +1515,20 @@ class TestDynProviderAlias(TestCase): # patching class DummyDSFMonitor(DSFMonitor): - def __init__(self, host=None, path=None, options_host=None, - options_path=None): + def __init__(self, host=None, path=None, protocol=None, port=None, + options_host=None, options_path=None, options_protocol=None, + options_port=None): # not calling super on purpose self._host = host self._path = path + self._protocol = protocol + self._port = port if options_host: self._options = { 'host': options_host, 'path': options_path, + 'protocol': options_protocol, + 'port': options_port, } else: self._options = None @@ -1527,12 +1537,16 @@ class DummyDSFMonitor(DSFMonitor): class TestDSFMonitorMonkeyPatching(TestCase): def test_host(self): - monitor = DummyDSFMonitor(host='host.com', path='/path') + monitor = DummyDSFMonitor(host='host.com', path='/path', + protocol='HTTP', port=8080) self.assertEquals('host.com', monitor.host) self.assertEquals('/path', monitor.path) + self.assertEquals('HTTP', monitor.protocol) + self.assertEquals(8080, monitor.port) monitor = DummyDSFMonitor(options_host='host.com', - options_path='/path') + options_path='/path', + options_protocol='HTTP', options_port=8080) self.assertEquals('host.com', monitor.host) self.assertEquals('/path', monitor.path) @@ -1540,6 +1554,10 @@ class TestDSFMonitorMonkeyPatching(TestCase): self.assertEquals('other.com', monitor.host) monitor.path = '/other-path' self.assertEquals('/other-path', monitor.path) + monitor.protocol = 'HTTPS' + self.assertEquals('HTTPS', monitor.protocol) + monitor.port = 8081 + self.assertEquals(8081, monitor.port) monitor = DummyDSFMonitor() monitor.host = 'other.com' @@ -1547,3 +1565,15 @@ class TestDSFMonitorMonkeyPatching(TestCase): monitor = DummyDSFMonitor() monitor.path = '/other-path' self.assertEquals('/other-path', monitor.path) + monitor.protocol = 'HTTP' + self.assertEquals('HTTP', monitor.protocol) + monitor.port = 8080 + self.assertEquals(8080, monitor.port) + + # Just to exercise the _options init + monitor = DummyDSFMonitor() + monitor.protocol = 'HTTP' + self.assertEquals('HTTP', monitor.protocol) + monitor = DummyDSFMonitor() + monitor.port = 8080 + self.assertEquals(8080, monitor.port) diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index 494a286..16da404 100644 --- a/tests/test_octodns_record.py +++ b/tests/test_octodns_record.py @@ -1020,6 +1020,25 @@ class TestRecordValidation(TestCase): 'invalid ip address "goodbye"' ], ctx.exception.reasons) + # invalid healthcheck protocol + with self.assertRaises(ValidationError) as ctx: + Record.new(self.zone, 'a', { + 'geo': { + 'NA': ['1.2.3.5'], + 'NA-US': ['1.2.3.5', '1.2.3.6'] + }, + 'type': 'A', + 'ttl': 600, + 'value': '1.2.3.4', + 'octodns': { + 'healthcheck': { + 'protocol': 'FTP', + } + } + }) + self.assertEquals(['invalid healthcheck protocol'], + ctx.exception.reasons) + def test_AAAA(self): # doesn't blow up Record.new(self.zone, '', {