Add proper tests for SplitYamlProvider

The SplitYamlProvider itself now requires a directory matching the
zone name under its directory to contain all YAML files. This doesn't
actually change the intended usage at all, just how the configuration
file is laid out.

Signed-off-by: Christian Funkhouser <cfunkhouser@heroku.com>

octodns/provider/yaml.py

@@ -157,6 +157,9 @@ class SplitYamlProvider(YamlProvider):
         super(SplitYamlProvider, self).__init__(id, directory, *args, **kwargs)
         self.log = logging.getLogger('SplitYamlProvider[{}]'.format(id))
 
+    def _zone_directory(self, zone):
+        return join(self.directory, zone.name)
+
     def populate(self, zone, target=False, lenient=False):
         self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name,
                        target, lenient)
@@ -167,7 +170,7 @@ class SplitYamlProvider(YamlProvider):
             return False
 
         before = len(zone.records)
-        yaml_filenames = list_all_yaml_files(self.directory)
+        yaml_filenames = list_all_yaml_files(self._zone_directory(zone))
         self.log.info('populate:   found %s YAML files', len(yaml_filenames))
         for yaml_filename in yaml_filenames:
             self._populate_from_file(yaml_filename, zone, lenient)
@@ -177,23 +180,24 @@ class SplitYamlProvider(YamlProvider):
         return False
 
     def _do_apply(self, desired, data):
+        zone_dir = self._zone_directory(desired)
+        if not isdir(zone_dir):
+            makedirs(zone_dir)
+
         catchall = dict()
         for record, config in data.items():
             if record in _CATCHALL_RECORD_NAMES:
                 catchall[record] = config
                 continue
-            filename = join(self.directory, '{}.yaml'.format(record))
+            filename = join(zone_dir, '{}.yaml'.format(record))
             self.log.debug('_apply:   writing filename=%s', filename)
             with open(filename, 'w') as fh:
                 record_data = {record: config}
                 safe_dump(record_data, fh)
         if catchall:
-            dname = desired.name
             # Scrub the trailing . to make filenames more sane.
-            if dname.endswith('.'):
-                dname = dname[:-1]
-            filename = join(
-                self.directory, '${}.yaml'.format(dname))
+            dname = desired.name[:-1]
+            filename = join(zone_dir, '${}.yaml'.format(dname))
             self.log.debug('_apply:   writing catchall filename=%s', filename)
             with open(filename, 'w') as fh:
                 safe_dump(catchall, fh)

tests/config/dynamic.tests.yaml

@@ -29,6 +29,7 @@ a:
         pool: ams
       - geos:
           - NA-US-CA
+          - NA-US-NC
           - NA-US-OR
           - NA-US-WA
         pool: sea
@@ -65,6 +66,7 @@ aaaa:
         pool: ams
       - geos:
           - NA-US-CA
+          - NA-US-NC
           - NA-US-OR
           - NA-US-WA
         pool: sea
@@ -100,6 +102,7 @@ cname:
         pool: ams
       - geos:
           - NA-US-CA
+          - NA-US-NC
           - NA-US-OR
           - NA-US-WA
         pool: sea
@@ -159,6 +162,7 @@ real-ish-a:
       - geos:
         # TODO: require sorted
         - NA-US-CA
+        - NA-US-NC
         - NA-US-OR
         - NA-US-WA
         pool: us-west-2

tests/config/simple-split.yaml

@@ -3,7 +3,7 @@ manager:
 providers:
   in:
     class: octodns.provider.yaml.SplitYamlProvider
-    directory: tests/config
+    directory: tests/config/split
   dump:
     class: octodns.provider.yaml.SplitYamlProvider
     directory: env/YAML_TMP_DIR

tests/config/split/dynamic.tests./a.yaml

@@ -0,0 +1,46 @@
+---
+a:
+  dynamic:
+    pools:
+      ams:
+        fallback: null
+        values:
+        - value: 1.1.1.1
+          weight: 1
+      iad:
+        fallback: null
+        values:
+        - value: 2.2.2.2
+          weight: 1
+        - value: 3.3.3.3
+          weight: 1
+      lax:
+        fallback: null
+        values:
+        - value: 4.4.4.4
+          weight: 1
+      sea:
+        fallback: null
+        values:
+        - value: 5.5.5.5
+          weight: 25
+        - value: 6.6.6.6
+          weight: 10
+    rules:
+    - geos:
+      - EU-GB
+      pool: iad
+    - geos:
+      - EU
+      pool: ams
+    - geos:
+      - NA-US-CA
+      - NA-US-NC
+      - NA-US-OR
+      - NA-US-WA
+      pool: sea
+    - pool: iad
+  type: A
+  values:
+  - 2.2.2.2
+  - 3.3.3.3

tests/config/split/dynamic.tests./aaaa.yaml

@@ -0,0 +1,46 @@
+---
+aaaa:
+  dynamic:
+    pools:
+      ams:
+        fallback: null
+        values:
+        - value: 2601:642:500:e210:62f8:1dff:feb8:9471
+          weight: 1
+      iad:
+        fallback: null
+        values:
+        - value: 2601:642:500:e210:62f8:1dff:feb8:9472
+          weight: 1
+        - value: 2601:642:500:e210:62f8:1dff:feb8:9473
+          weight: 1
+      lax:
+        fallback: null
+        values:
+        - value: 2601:642:500:e210:62f8:1dff:feb8:9474
+          weight: 1
+      sea:
+        fallback: null
+        values:
+        - value: 2601:642:500:e210:62f8:1dff:feb8:9475
+          weight: 1
+        - value: 2601:642:500:e210:62f8:1dff:feb8:9476
+          weight: 2
+    rules:
+    - geos:
+      - EU-GB
+      pool: iad
+    - geos:
+      - EU
+      pool: ams
+    - geos:
+      - NA-US-CA
+      - NA-US-NC
+      - NA-US-OR
+      - NA-US-WA
+      pool: sea
+    - pool: iad
+  type: AAAA
+  values:
+  - 2601:642:500:e210:62f8:1dff:feb8:947a
+  - 2601:644:500:e210:62f8:1dff:feb8:947a

tests/config/split/dynamic.tests./cname.yaml

@@ -0,0 +1,42 @@
+---
+cname:
+  dynamic:
+    pools:
+      ams:
+        fallback: null
+        values:
+        - value: target-ams.unit.tests.
+          weight: 1
+      iad:
+        fallback: null
+        values:
+        - value: target-iad.unit.tests.
+          weight: 1
+      lax:
+        fallback: null
+        values:
+        - value: target-lax.unit.tests.
+          weight: 1
+      sea:
+        fallback: null
+        values:
+        - value: target-sea-1.unit.tests.
+          weight: 100
+        - value: target-sea-2.unit.tests.
+          weight: 175
+    rules:
+    - geos:
+      - EU-GB
+      pool: iad
+    - geos:
+      - EU
+      pool: ams
+    - geos:
+      - NA-US-CA
+      - NA-US-NC
+      - NA-US-OR
+      - NA-US-WA
+      pool: sea
+    - pool: iad
+  type: CNAME
+  value: target.unit.tests.

tests/config/split/dynamic.tests./real-ish-a.yaml

@@ -0,0 +1,87 @@
+---
+real-ish-a:
+  dynamic:
+    pools:
+      ap-southeast-1:
+        fallback: null
+        values:
+        - value: 1.4.1.1
+          weight: 2
+        - value: 1.4.1.2
+          weight: 2
+        - value: 1.4.2.1
+          weight: 1
+        - value: 1.4.2.2
+          weight: 1
+        - value: 1.4.3.1
+          weight: 1
+        - value: 1.4.3.2
+          weight: 1
+      eu-central-1:
+        fallback: null
+        values:
+        - value: 1.3.1.1
+          weight: 1
+        - value: 1.3.1.2
+          weight: 1
+        - value: 1.3.2.1
+          weight: 1
+        - value: 1.3.2.2
+          weight: 1
+        - value: 1.3.3.1
+          weight: 1
+        - value: 1.3.3.2
+          weight: 1
+      us-east-1:
+        fallback: null
+        values:
+        - value: 1.1.1.1
+          weight: 1
+        - value: 1.1.1.2
+          weight: 1
+        - value: 1.1.2.1
+          weight: 1
+        - value: 1.1.2.2
+          weight: 1
+        - value: 1.1.3.1
+          weight: 1
+        - value: 1.1.3.2
+          weight: 1
+      us-west-2:
+        fallback: null
+        values:
+        - value: 1.2.1.1
+          weight: 1
+        - value: 1.2.1.2
+          weight: 1
+        - value: 1.2.2.1
+          weight: 1
+        - value: 1.2.2.2
+          weight: 1
+        - value: 1.2.3.1
+          weight: 1
+        - value: 1.2.3.2
+          weight: 1
+    rules:
+    - geos:
+      - NA-US-CA
+      - NA-US-NC
+      - NA-US-OR
+      - NA-US-WA
+      pool: us-west-2
+    - geos:
+      - AS-CN
+      pool: ap-southeast-1
+    - geos:
+      - AF
+      - EU
+      pool: eu-central-1
+    - pool: us-east-1
+  type: A
+  values:
+  - 1.1.1.1
+  - 1.1.1.2
+  - 1.1.2.1
+  - 1.1.2.2
+  - 1.1.3.1
+  - 1.1.3.2

tests/config/split/dynamic.tests./simple-weighted.yaml

@@ -0,0 +1,15 @@
+---
+simple-weighted:
+  dynamic:
+    pools:
+      default:
+        fallback: null
+        values:
+        - value: one.unit.tests.
+          weight: 3
+        - value: two.unit.tests.
+          weight: 2
+    rules:
+    - pool: default
+  type: CNAME
+  value: default.unit.tests.

tests/config/split/subzone.unit.tests./12.yaml

@@ -0,0 +1,4 @@
+---
+'12':
+  type: A
+  value: 12.4.4.4

tests/config/split/subzone.unit.tests./2.yaml

@@ -0,0 +1,4 @@
+---
+'2':
+  type: A
+  value: 2.4.4.4

tests/config/split/subzone.unit.tests./test.yaml

@@ -0,0 +1,4 @@
+---
+test:
+  type: A
+  value: 4.4.4.4

tests/config/split/unit.tests./$unit.tests.yaml

@@ -0,0 +1,37 @@
+---
+? ''
+: - geo:
+      AF:
+      - 2.2.3.4
+      - 2.2.3.5
+      AS-JP:
+      - 3.2.3.4
+      - 3.2.3.5
+      NA-US:
+      - 4.2.3.4
+      - 4.2.3.5
+      NA-US-CA:
+      - 5.2.3.4
+      - 5.2.3.5
+    ttl: 300
+    type: A
+    values:
+    - 1.2.3.4
+    - 1.2.3.5
+  - type: CAA
+    value:
+      flags: 0
+      tag: issue
+      value: ca.unit.tests
+  - type: NS
+    values:
+    - 6.2.3.4.
+    - 7.2.3.4.
+  - type: SSHFP
+    values:
+    - algorithm: 1
+      fingerprint: 7491973e5f8b39d5327cd4e08bc81b05f7710b49
+      fingerprint_type: 1
+    - algorithm: 1
+      fingerprint: bf6b6825d2977c511a475bbefb88aad54a92ac73
+      fingerprint_type: 1

tests/config/split/unit.tests./_srv._tcp.yaml

@@ -0,0 +1,13 @@
+---
+_srv._tcp:
+  ttl: 600
+  type: SRV
+  values:
+  - port: 30
+    priority: 10
+    target: foo-1.unit.tests.
+    weight: 20
+  - port: 30
+    priority: 12
+    target: foo-2.unit.tests.
+    weight: 20

tests/config/split/unit.tests./aaaa.yaml

@@ -0,0 +1,5 @@
+---
+aaaa:
+  ttl: 600
+  type: AAAA
+  value: 2601:644:500:e210:62f8:1dff:feb8:947a

tests/config/split/unit.tests./cname.yaml

@@ -0,0 +1,5 @@
+---
+cname:
+  ttl: 300
+  type: CNAME
+  value: unit.tests.

tests/config/split/unit.tests./excluded.yaml

@@ -0,0 +1,7 @@
+---
+excluded:
+  octodns:
+    excluded:
+    - test
+  type: CNAME
+  value: unit.tests.

tests/config/split/unit.tests./ignored.yaml

@@ -0,0 +1,6 @@
+---
+ignored:
+  octodns:
+    ignored: true
+  type: A
+  value: 9.9.9.9

tests/config/split/unit.tests./included.yaml

@@ -0,0 +1,7 @@
+---
+included:
+  octodns:
+    included:
+    - test
+  type: CNAME
+  value: unit.tests.

tests/config/split/unit.tests./mx.yaml

@@ -0,0 +1,13 @@
+---
+mx:
+  ttl: 300
+  type: MX
+  values:
+  - exchange: smtp-4.unit.tests.
+    preference: 10
+  - exchange: smtp-2.unit.tests.
+    preference: 20
+  - exchange: smtp-3.unit.tests.
+    preference: 30
+  - exchange: smtp-1.unit.tests.
+    preference: 40

tests/config/split/unit.tests./naptr.yaml

@@ -0,0 +1,17 @@
+---
+naptr:
+  ttl: 600
+  type: NAPTR
+  values:
+  - flags: S
+    order: 10
+    preference: 100
+    regexp: '!^.*$!sip:info@bar.example.com!'
+    replacement: .
+    service: SIP+D2U
+  - flags: U
+    order: 100
+    preference: 100
+    regexp: '!^.*$!sip:info@bar.example.com!'
+    replacement: .
+    service: SIP+D2U

tests/config/split/unit.tests./ptr.yaml

@@ -0,0 +1,5 @@
+---
+ptr:
+  ttl: 300
+  type: PTR
+  value: foo.bar.com.

tests/config/split/unit.tests./spf.yaml

@@ -0,0 +1,5 @@
+---
+spf:
+  ttl: 600
+  type: SPF
+  value: v=spf1 ip4:192.168.0.1/16-all

tests/config/split/unit.tests./sub.yaml

@@ -0,0 +1,6 @@
+---
+sub:
+  type: NS
+  values:
+  - 6.2.3.4.
+  - 7.2.3.4.

tests/config/split/unit.tests./txt.yaml

@@ -0,0 +1,8 @@
+---
+txt:
+  ttl: 600
+  type: TXT
+  values:
+  - Bah bah black sheep
+  - have you any wool.
+  - v=DKIM1\;k=rsa\;s=email\;h=sha256\;p=A/kinda+of/long/string+with+numb3rs

tests/config/split/unit.tests./www.sub.yaml

@@ -0,0 +1,5 @@
+---
+www.sub:
+  ttl: 300
+  type: A
+  value: 2.2.3.6

tests/config/split/unit.tests./www.yaml

@@ -0,0 +1,5 @@
+---
+www:
+  ttl: 300
+  type: A
+  value: 2.2.3.6

tests/config/split/unordered./abc.yaml

@@ -0,0 +1,4 @@
+---
+abc:
+  type: A
+  value: 9.9.9.9

tests/config/split/unordered./xyz.yaml

@@ -0,0 +1,5 @@
+---
+xyz:
+  # t comes before v
+  value: 9.9.9.9
+  type: A

tests/test_octodns_manager.py

@@ -256,9 +256,9 @@ class TestManager(TestCase):
 
             manager.dump('unit.tests.', tmpdir.dirname, False, True, 'in')
 
-            # make sure this fails with an IOError and not a KeyError when
+            # make sure this fails with an OSError and not a KeyError when
             # tyring to find sub zones
-            with self.assertRaises(IOError):
+            with self.assertRaises(OSError):
                 manager.dump('unknown.zone.', tmpdir.dirname, False, True,
                              'in')
 

tests/test_octodns_provider_splityaml.py

@@ -0,0 +1,219 @@
+#
+#
+#
+
+from __future__ import absolute_import, division, print_function, \
+    unicode_literals
+
+from os import makedirs
+from os.path import basename, dirname, isdir, isfile, join
+from unittest import TestCase
+from yaml import safe_load
+from yaml.constructor import ConstructorError
+
+from octodns.provider.base import Plan
+from octodns.provider.yaml import SplitYamlProvider, list_all_yaml_files
+from octodns.record import Create
+from octodns.zone import SubzoneRecordException, Zone
+
+from helpers import TemporaryDirectory
+
+
+class TestSplitYamlProvider(TestCase):
+
+    def test_list_all_yaml_files(self):
+        yaml_files = ('foo.yaml', '1.yaml', '$unit.tests.yaml')
+        all_files = ('something', 'else', '1', '$$', '-f') + yaml_files
+        all_dirs = ('dir1', 'dir2/sub', 'tricky.yaml')
+
+        with TemporaryDirectory() as td:
+            directory = join(td.dirname)
+
+            # Create some files, some of them with a .yaml extension, all of
+            # them empty.
+            for emptyfile in all_files:
+                open(join(directory, emptyfile), 'w').close()
+            # Do the same for some fake directories
+            for emptydir in all_dirs:
+                makedirs(join(directory, emptydir))
+
+            self.assertItemsEqual(
+                yaml_files,
+                # This isn't great, but given the variable nature of the temp
+                # dir names, it's necessary.
+                (basename(f) for f in list_all_yaml_files(directory)))
+
+    def test_zone_directory(self):
+        source = SplitYamlProvider(
+            'test', join(dirname(__file__), 'config/split'))
+
+        zone = Zone('unit.tests.', [])
+
+        self.assertEqual(
+            join(dirname(__file__), 'config/split/unit.tests.'),
+            source._zone_directory(zone))
+
+    def test_apply_handles_existing_zone_directory(self):
+        with TemporaryDirectory() as td:
+            provider = SplitYamlProvider('test', join(td.dirname, 'config'))
+            makedirs(join(td.dirname, 'config', 'does.exist.'))
+
+            zone = Zone('does.exist.', [])
+            self.assertTrue(isdir(provider._zone_directory(zone)))
+            provider.apply(Plan(None, zone, [], True))
+            self.assertTrue(isdir(provider._zone_directory(zone)))
+
+    def test_provider(self):
+        source = SplitYamlProvider(
+            'test', join(dirname(__file__), 'config/split'))
+
+        zone = Zone('unit.tests.', [])
+        dynamic_zone = Zone('dynamic.tests.', [])
+
+        # With target we don't add anything
+        source.populate(zone, target=source)
+        self.assertEquals(0, len(zone.records))
+
+        # without it we see everything
+        source.populate(zone)
+        self.assertEquals(18, len(zone.records))
+
+        source.populate(dynamic_zone)
+        self.assertEquals(5, 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
+        # pulled in yet again and still match up. That assumes that the input
+        # data completely exercises things. This assumption can be tested by
+        # relatively well by running
+        #   ./script/coverage tests/test_octodns_provider_yaml.py and
+        # looking at the coverage file
+        #   ./htmlcov/octodns_provider_yaml_py.html
+
+        with TemporaryDirectory() as td:
+            # Add some subdirs to make sure that it can create them
+            directory = join(td.dirname, 'sub', 'dir')
+            zone_dir = join(directory, 'unit.tests.')
+            dynamic_zone_dir = join(directory, 'dynamic.tests.')
+            target = SplitYamlProvider('test', directory)
+
+            # We add everything
+            plan = target.plan(zone)
+            self.assertEquals(15, len(filter(lambda c: isinstance(c, Create),
+                                             plan.changes)))
+            self.assertFalse(isdir(zone_dir))
+
+            # Now actually do it
+            self.assertEquals(15, target.apply(plan))
+
+            # Dynamic plan
+            plan = target.plan(dynamic_zone)
+            self.assertEquals(5, len(filter(lambda c: isinstance(c, Create),
+                                            plan.changes)))
+            self.assertFalse(isdir(dynamic_zone_dir))
+            # Apply it
+            self.assertEquals(5, target.apply(plan))
+            self.assertTrue(isdir(dynamic_zone_dir))
+
+            # There should be no changes after the round trip
+            reloaded = Zone('unit.tests.', [])
+            target.populate(reloaded)
+            self.assertDictEqual(
+                {'included': ['test']},
+                filter(
+                    lambda x: x.name == 'included', reloaded.records
+                )[0]._octodns)
+
+            self.assertFalse(zone.changes(reloaded, target=source))
+
+            # A 2nd sync should still create everything
+            plan = target.plan(zone)
+            self.assertEquals(15, len(filter(lambda c: isinstance(c, Create),
+                                             plan.changes)))
+
+            yaml_file = join(zone_dir, '$unit.tests.yaml')
+            self.assertTrue(isfile(yaml_file))
+            with open(yaml_file) as fh:
+                data = safe_load(fh.read())
+                roots = sorted(data.pop(''), key=lambda r: r['type'])
+                self.assertTrue('values' in roots[0])  # A
+                self.assertTrue('geo' in roots[0])  # geo made the trip
+                self.assertTrue('value' in roots[1])   # CAA
+                self.assertTrue('values' in roots[2])  # SSHFP
+
+            # These records are stored as plural "values." Check each file to
+            # ensure correctness.
+            for record_name in ('_srv._tcp', 'mx', 'naptr', 'sub', 'txt'):
+                yaml_file = join(zone_dir, '{}.yaml'.format(record_name))
+                self.assertTrue(isfile(yaml_file))
+                with open(yaml_file) as fh:
+                    data = safe_load(fh.read())
+                    self.assertTrue('values' in data.pop(record_name))
+
+            # These are stored as singular "value." Again, check each file.
+            for record_name in ('aaaa', 'cname', 'included', 'ptr', 'spf',
+                                'www.sub', 'www'):
+                yaml_file = join(zone_dir, '{}.yaml'.format(record_name))
+                self.assertTrue(isfile(yaml_file))
+                with open(yaml_file) as fh:
+                    data = safe_load(fh.read())
+                    self.assertTrue('value' in data.pop(record_name))
+
+            # Again with the plural, this time checking dynamic.tests.
+            for record_name in ('a', 'aaaa', 'real-ish-a'):
+                yaml_file = join(
+                    dynamic_zone_dir, '{}.yaml'.format(record_name))
+                self.assertTrue(isfile(yaml_file))
+                with open(yaml_file) as fh:
+                    data = safe_load(fh.read())
+                    dyna = data.pop(record_name)
+                    self.assertTrue('values' in dyna)
+                    self.assertTrue('dynamic' in dyna)
+
+            # Singular again.
+            for record_name in ('cname', 'simple-weighted'):
+                yaml_file = join(
+                    dynamic_zone_dir, '{}.yaml'.format(record_name))
+                self.assertTrue(isfile(yaml_file))
+                with open(yaml_file) as fh:
+                    data = safe_load(fh.read())
+                    dyna = data.pop(record_name)
+                    self.assertTrue('value' in dyna)
+                    self.assertTrue('dynamic' in dyna)
+
+    def test_empty(self):
+        source = SplitYamlProvider(
+            'test', join(dirname(__file__), 'config/split'))
+
+        zone = Zone('empty.', [])
+
+        # without it we see everything
+        source.populate(zone)
+        self.assertEquals(0, len(zone.records))
+
+    def test_unsorted(self):
+        source = SplitYamlProvider(
+            'test', join(dirname(__file__), 'config/split'))
+
+        zone = Zone('unordered.', [])
+
+        with self.assertRaises(ConstructorError):
+            source.populate(zone)
+
+        source = SplitYamlProvider(
+            'test', join(dirname(__file__), 'config/split'),
+            enforce_order=False)
+        # no exception
+        source.populate(zone)
+        self.assertEqual(2, len(zone.records))
+
+    def test_subzone_handling(self):
+        source = SplitYamlProvider(
+            'test', join(dirname(__file__), 'config/split'))
+
+        # If we add `sub` as a sub-zone we'll reject `www.sub`
+        zone = Zone('unit.tests.', ['sub'])
+        with self.assertRaises(SubzoneRecordException) as ctx:
+            source.populate(zone)
+        self.assertEquals('Record www.sub.unit.tests. is under a managed '
+                          'subzone', ctx.exception.message)