Merge remote-tracking branch 'origin' into processors

.github/workflows/main.yml

@@ -1,8 +1,5 @@
 name: OctoDNS
-on:
-  pull_request:
-    paths-ignore:
-      - '**.md'
+on: [pull_request]
 
 jobs:
   ci:

README.md

@@ -93,7 +93,7 @@ The `max_workers` key in the `manager` section of the config enables threading t
 
 In this example, `example.net` is an alias of zone `example.com`, which means they share the same sources and targets. They will therefore have identical records.
 
-Now that we have something to tell OctoDNS about our providers & zones we need to tell it about or records. We'll keep it simple for now and just create a single `A` record at the top-level of the domain.
+Now that we have something to tell OctoDNS about our providers & zones we need to tell it about our records. We'll keep it simple for now and just create a single `A` record at the top-level of the domain.
 
 `config/example.com.yaml`
 
@@ -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 |
@@ -294,7 +294,7 @@ If you have a problem or suggestion, please [open an issue](https://github.com/o
 - **GitHub Action:** [OctoDNS-Sync](https://github.com/marketplace/actions/octodns-sync)
 - **Sample Implementations.** See how others are using it
   - [`hackclub/dns`](https://github.com/hackclub/dns)
-  - [`kubernetes/k8s.io:/dns`](https://github.com/kubernetes/k8s.io/tree/master/dns)
+  - [`kubernetes/k8s.io:/dns`](https://github.com/kubernetes/k8s.io/tree/main/dns)
   - [`g0v-network/domains`](https://github.com/g0v-network/domains)
   - [`jekyll/dns`](https://github.com/jekyll/dns)
 - **Custom Sources & Providers.**

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
@@ -293,16 +294,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)
@@ -441,7 +445,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')

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)

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
 

octodns/provider/plan.py

@@ -50,7 +50,7 @@ class Plan(object):
         except AttributeError:
             existing_n = 0
 
-        self.log.debug('__init__: Creates=%d, Updates=%d, Deletes=%d'
+        self.log.debug('__init__: Creates=%d, Updates=%d, Deletes=%d '
                        'Existing=%d',
                        self.change_counts['Create'],
                        self.change_counts['Update'],

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

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))

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

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:

tests/config/plan-output-filehandle.yaml

@@ -0,0 +1,6 @@
+manager:
+  plan_outputs:
+    "doesntexist":
+      class: octodns.provider.plan.DoesntExist
+providers: {}
+zones: {}

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):
 

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))
 

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,