diff --git a/README.md b/README.md index 0a26da9..3186ed2 100644 --- a/README.md +++ b/README.md @@ -204,7 +204,7 @@ The above command pulled the existing data out of Route53 and placed the results | [Rackspace](/octodns/provider/rackspace.py) | | A, AAAA, ALIAS, CNAME, MX, NS, PTR, SPF, TXT | No | | | [Route53](/octodns/provider/route53.py) | boto3 | A, AAAA, CAA, CNAME, MX, NAPTR, NS, PTR, SPF, SRV, TXT | Both | CNAME health checks don't support a Host header | | [Selectel](/octodns/provider/selectel.py) | | A, AAAA, CNAME, MX, NS, SPF, SRV, TXT | No | | -| [Transip](/octodns/provider/transip.py) | transip | A, AAAA, CNAME, MX, SRV, SPF, TXT, SSHFP, CAA | No | | +| [Transip](/octodns/provider/transip.py) | transip | A, AAAA, CNAME, MX, NS, SRV, SPF, TXT, SSHFP, CAA | No | | | [UltraDns](/octodns/provider/ultra.py) | | A, AAAA, CAA, CNAME, MX, NS, PTR, SPF, SRV, TXT | No | | | [AxfrSource](/octodns/source/axfr.py) | | A, AAAA, CAA, CNAME, LOC, MX, NS, PTR, SPF, SRV, TXT | No | read-only | | [ZoneFileSource](/octodns/source/axfr.py) | | A, AAAA, CAA, CNAME, MX, NS, PTR, SPF, SRV, TXT | No | read-only | diff --git a/octodns/manager.py b/octodns/manager.py index 9ce10ff..9b10196 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -9,6 +9,7 @@ from concurrent.futures import ThreadPoolExecutor from importlib import import_module from os import environ from six import text_type +from sys import stdout import logging from .provider.base import BaseProvider @@ -267,16 +268,19 @@ class Manager(object): return plans, zone def sync(self, eligible_zones=[], eligible_sources=[], eligible_targets=[], - dry_run=True, force=False): - self.log.info('sync: eligible_zones=%s, eligible_targets=%s, ' - 'dry_run=%s, force=%s', eligible_zones, eligible_targets, - dry_run, force) + dry_run=True, force=False, plan_output_fh=stdout): + + self.log.info( + 'sync: eligible_zones=%s, eligible_targets=%s, dry_run=%s, ' + 'force=%s, plan_output_fh=%s', + eligible_zones, eligible_targets, dry_run, force, + getattr(plan_output_fh, 'name', plan_output_fh.__class__.__name__)) zones = self.config['zones'].items() if eligible_zones: zones = [z for z in zones if z[0] in eligible_zones] - aliased_zones = {} + aliased_zones = {} futures = [] for zone_name, config in zones: self.log.info('sync: zone=%s', zone_name) @@ -402,7 +406,7 @@ class Manager(object): plans.sort(key=self._plan_keyer, reverse=True) for output in self.plan_outputs.values(): - output.run(plans=plans, log=self.log) + output.run(plans=plans, log=self.log, fh=plan_output_fh) if not force: self.log.debug('sync: checking safety') diff --git a/octodns/provider/dnsimple.py b/octodns/provider/dnsimple.py index 599eacb..a4b9350 100644 --- a/octodns/provider/dnsimple.py +++ b/octodns/provider/dnsimple.py @@ -51,8 +51,8 @@ class DnsimpleClient(object): resp.raise_for_status() return resp - def domain(self, name): - path = '/domains/{}'.format(name) + def zone(self, name): + path = '/zones/{}'.format(name) return self._request('GET', path).json() def domain_create(self, name): @@ -442,7 +442,7 @@ class DnsimpleProvider(BaseProvider): domain_name = desired.name[:-1] try: - self._client.domain(domain_name) + self._client.zone(domain_name) except DnsimpleClientNotFound: self.log.debug('_apply: no matching zone, creating domain') self._client.domain_create(domain_name) diff --git a/octodns/provider/easydns.py b/octodns/provider/easydns.py index 835fcb9..d7a75a4 100644 --- a/octodns/provider/easydns.py +++ b/octodns/provider/easydns.py @@ -59,7 +59,7 @@ class EasyDNSClient(object): self.base_path = self.SANDBOX if sandbox else self.LIVE sess = Session() sess.headers.update({'Authorization': 'Basic {}' - .format(self.auth_key)}) + .format(self.auth_key.decode('utf-8'))}) sess.headers.update({'accept': 'application/json'}) self._sess = sess diff --git a/octodns/provider/transip.py b/octodns/provider/transip.py index 7458e36..6ccbe22 100644 --- a/octodns/provider/transip.py +++ b/octodns/provider/transip.py @@ -49,8 +49,8 @@ class TransipProvider(BaseProvider): ''' SUPPORTS_GEO = False SUPPORTS_DYNAMIC = False - SUPPORTS = set( - ('A', 'AAAA', 'CNAME', 'MX', 'SRV', 'SPF', 'TXT', 'SSHFP', 'CAA')) + SUPPORTS = set(('A', 'AAAA', 'CNAME', 'MX', 'NS', 'SRV', 'SPF', 'TXT', + 'SSHFP', 'CAA')) # unsupported by OctoDNS: 'TLSA' MIN_TTL = 120 TIMEOUT = 15 diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index 8ee2eaa..8c8760a 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -531,6 +531,7 @@ class _DynamicMixin(object): pools_exist = set() pools_seen = set() + pools_seen_as_fallback = set() if not isinstance(pools, dict): reasons.append('pools must be a dict') elif not pools: @@ -573,9 +574,12 @@ class _DynamicMixin(object): 'value {}'.format(_id, value_num)) fallback = pool.get('fallback', None) - if fallback is not None and fallback not in pools: - reasons.append('undefined fallback "{}" for pool "{}"' - .format(fallback, _id)) + if fallback is not None: + if fallback in pools: + pools_seen_as_fallback.add(fallback) + else: + reasons.append('undefined fallback "{}" for pool "{}"' + .format(fallback, _id)) # Check for loops fallback = pools[_id].get('fallback', None) @@ -644,7 +648,7 @@ class _DynamicMixin(object): reasons.extend(GeoCodes.validate(geo, 'rule {} ' .format(rule_num))) - unused = pools_exist - pools_seen + unused = pools_exist - pools_seen - pools_seen_as_fallback if unused: unused = '", "'.join(sorted(unused)) reasons.append('unused pools: "{}"'.format(unused)) diff --git a/requirements.txt b/requirements.txt index 8b9c052..933ac60 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -PyYaml==5.3.1 +PyYaml==5.4 azure-common==1.1.25 azure-mgmt-dns==3.0.0 boto3==1.15.9 diff --git a/tests/config/dynamic.tests.yaml b/tests/config/dynamic.tests.yaml index 4bd97a7..d25f63a 100644 --- a/tests/config/dynamic.tests.yaml +++ b/tests/config/dynamic.tests.yaml @@ -109,6 +109,29 @@ cname: - pool: iad type: CNAME value: target.unit.tests. +pool-only-in-fallback: + dynamic: + pools: + one: + fallback: two + values: + - value: 1.1.1.1 + three: + values: + - value: 3.3.3.3 + two: + values: + - value: 2.2.2.2 + rules: + - geos: + - NA-US + pool: one + - geos: + - AS-SG + pool: three + ttl: 300 + type: A + values: [4.4.4.4] real-ish-a: dynamic: pools: diff --git a/tests/config/plan-output-filehandle.yaml b/tests/config/plan-output-filehandle.yaml new file mode 100644 index 0000000..9c9bb87 --- /dev/null +++ b/tests/config/plan-output-filehandle.yaml @@ -0,0 +1,6 @@ +manager: + plan_outputs: + "doesntexist": + class: octodns.provider.plan.DoesntExist +providers: {} +zones: {} diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 442ed49..91ee374 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -8,7 +8,6 @@ from __future__ import absolute_import, division, print_function, \ from os import environ from os.path import dirname, join from six import text_type -from unittest import TestCase from octodns.record import Record from octodns.manager import _AggregateTarget, MainThreadExecutor, Manager, \ @@ -16,6 +15,9 @@ from octodns.manager import _AggregateTarget, MainThreadExecutor, Manager, \ from octodns.yaml import safe_load from octodns.zone import Zone +from mock import MagicMock, patch +from unittest import TestCase + from helpers import DynamicProvider, GeoProvider, NoSshFpProvider, \ SimpleProvider, TemporaryDirectory @@ -371,6 +373,24 @@ class TestManager(TestCase): with self.assertRaises(TypeError): manager._populate_and_plan('unit.tests.', [NoZone()], []) + @patch('octodns.manager.Manager._get_named_class') + def test_sync_passes_file_handle(self, mock): + plan_output_mock = MagicMock() + plan_output_class_mock = MagicMock() + plan_output_class_mock.return_value = plan_output_mock + mock.return_value = plan_output_class_mock + fh_mock = MagicMock() + + Manager(get_config_filename('plan-output-filehandle.yaml') + ).sync(plan_output_fh=fh_mock) + + # Since we only care about the fh kwarg, and different _PlanOutputs are + # are free to require arbitrary kwargs anyway, we concern ourselves + # with checking the value of fh only. + plan_output_mock.run.assert_called() + _, kwargs = plan_output_mock.run.call_args + self.assertEqual(fh_mock, kwargs.get('fh')) + class TestMainThreadExecutor(TestCase): diff --git a/tests/test_octodns_provider_transip.py b/tests/test_octodns_provider_transip.py index 84cfebc..234c95e 100644 --- a/tests/test_octodns_provider_transip.py +++ b/tests/test_octodns_provider_transip.py @@ -56,10 +56,11 @@ class MockDomainService(DomainService): _dns_entries.extend(entries_for(name, record)) - # NS is not supported as a DNS Entry, - # so it should cover the if statement + # Add a non-supported type + # so it triggers the "is supported" (transip.py:115) check and + # give 100% code coverage _dns_entries.append( - DnsEntry('@', '3600', 'NS', 'ns01.transip.nl.')) + DnsEntry('@', '3600', 'BOGUS', 'ns01.transip.nl.')) self.mockupEntries = _dns_entries @@ -222,7 +223,7 @@ N4OiVz1I3rbZGYa396lpxO6ku8yCglisL1yrSP6DdEUp66ntpKVd provider._client = MockDomainService('unittest', self.bogus_key) plan = provider.plan(_expected) - self.assertEqual(14, plan.change_counts['Create']) + self.assertEqual(15, plan.change_counts['Create']) self.assertEqual(0, plan.change_counts['Update']) self.assertEqual(0, plan.change_counts['Delete']) @@ -235,7 +236,7 @@ N4OiVz1I3rbZGYa396lpxO6ku8yCglisL1yrSP6DdEUp66ntpKVd provider = TransipProvider('test', 'unittest', self.bogus_key) provider._client = MockDomainService('unittest', self.bogus_key) plan = provider.plan(_expected) - self.assertEqual(14, len(plan.changes)) + self.assertEqual(15, len(plan.changes)) changes = provider.apply(plan) self.assertEqual(changes, len(plan.changes)) diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 2ce9b4a..872fcca 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -38,7 +38,7 @@ class TestYamlProvider(TestCase): self.assertEquals(22, len(zone.records)) source.populate(dynamic_zone) - self.assertEquals(5, len(dynamic_zone.records)) + self.assertEquals(6, len(dynamic_zone.records)) # Assumption here is that a clean round-trip means that everything # worked as expected, data that went in came back out and could be @@ -68,11 +68,11 @@ class TestYamlProvider(TestCase): # Dynamic plan plan = target.plan(dynamic_zone) - self.assertEquals(5, len([c for c in plan.changes + self.assertEquals(6, len([c for c in plan.changes if isinstance(c, Create)])) self.assertFalse(isfile(dynamic_yaml_file)) # Apply it - self.assertEquals(5, target.apply(plan)) + self.assertEquals(6, target.apply(plan)) self.assertTrue(isfile(dynamic_yaml_file)) # There should be no changes after the round trip @@ -148,6 +148,10 @@ class TestYamlProvider(TestCase): self.assertTrue('value' in dyna) # self.assertTrue('dynamic' in dyna) + dyna = data.pop('pool-only-in-fallback') + self.assertTrue('value' in dyna) + # self.assertTrue('dynamic' in dyna) + # make sure nothing is left self.assertEquals([], list(data.keys())) @@ -397,7 +401,7 @@ class TestOverridingYamlProvider(TestCase): # Load the base, should see the 5 records base.populate(zone) got = {r.name: r for r in zone.records} - self.assertEquals(5, len(got)) + self.assertEquals(6, len(got)) # We get the "dynamic" A from the base config self.assertTrue('dynamic' in got['a'].data) # No added @@ -406,7 +410,7 @@ class TestOverridingYamlProvider(TestCase): # Load the overrides, should replace one and add 1 override.populate(zone) got = {r.name: r for r in zone.records} - self.assertEquals(6, len(got)) + self.assertEquals(7, len(got)) # 'a' was replaced with a generic record self.assertEquals({ 'ttl': 3600,