From ffc4e2f957ac575954728d9f1fbff940c285837a Mon Sep 17 00:00:00 2001 From: Christian Funkhouser Date: Wed, 3 Apr 2019 23:14:03 -0400 Subject: [PATCH 01/15] Add SplitYamlProvider SplitYamlProvider extends and behaves similarly to YamlProvider, but organizes the zone in multiple files by record, insteat of in a monolithic YAML file. YamlProvider has been slightly modified to make its extension easier. Signed-off-by: Christian Funkhouser --- octodns/provider/yaml.py | 117 ++++++++++++++++++++++++++++++++++----- 1 file changed, 104 insertions(+), 13 deletions(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index a9631a0..f5652c0 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -6,8 +6,8 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals from collections import defaultdict -from os import makedirs -from os.path import isdir, join +from os import listdir, makedirs +from os.path import isdir, isfile, join import logging from ..record import Record @@ -46,17 +46,7 @@ class YamlProvider(BaseProvider): self.default_ttl = default_ttl self.enforce_order = enforce_order - def populate(self, zone, target=False, lenient=False): - self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, - target, lenient) - - if target: - # When acting as a target we ignore any existing records so that we - # create a completely new copy - return False - - before = len(zone.records) - filename = join(self.directory, '{}yaml'.format(zone.name)) + def _populate_from_file(self, filename, zone, lenient): with open(filename, 'r') as fh: yaml_data = safe_load(fh, enforce_order=self.enforce_order) if yaml_data: @@ -69,6 +59,21 @@ class YamlProvider(BaseProvider): record = Record.new(zone, name, d, source=self, lenient=lenient) zone.add_record(record, lenient=lenient) + self.log.info( + '_populate_from_file: successfully loaded "%s"', filename) + + def populate(self, zone, target=False, lenient=False): + self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, + target, lenient) + + if target: + # When acting as a target we ignore any existing records so that we + # create a completely new copy + return False + + before = len(zone.records) + filename = join(self.directory, '{}yaml'.format(zone.name)) + self._populate_from_file(filename, zone, lenient) self.log.info('populate: found %s records, exists=False', len(zone.records) - before) @@ -102,7 +107,93 @@ class YamlProvider(BaseProvider): if not isdir(self.directory): makedirs(self.directory) + self._do_apply(desired, data) + + def _do_apply(self, desired, data): filename = join(self.directory, '{}yaml'.format(desired.name)) self.log.debug('_apply: writing filename=%s', filename) with open(filename, 'w') as fh: safe_dump(dict(data), fh) + + +# Any record name added to this set will be included in the catch-all file, +# instead of a file matching the record name. +_CATCHALL_RECORD_NAMES = ('*', '') + + +def list_all_yaml_files(directory): + yaml_files = set() + for f in listdir(directory): + filename = join(directory, '{}'.format(f)) + if f.endswith('.yaml') and isfile(filename): + yaml_files.add(filename) + return list(yaml_files) + + +class SplitYamlProvider(YamlProvider): + ''' + Core provider for records configured in multiple YAML files on disk. + + Behaves mostly similarly to YamlConfig, but interacts with multiple YAML + files, instead of a single monolitic one. The files are named RECORD.yaml, + except for any record which cannot be represented easily as a file; these + are stored in the catchall file, which is a YAML file the zone name, + prepended with a '$'. For example, a zone, 'github.com.' would have a + catch-all file named '$github.com.yaml'. + + config: + class: octodns.provider.yaml.SplitYamlProvider + # The location of yaml config files (required) + directory: ./config + # The ttl to use for records when not specified in the data + # (optional, default 3600) + default_ttl: 3600 + # Whether or not to enforce sorting order on the yaml config + # (optional, default True) + enforce_order: True + ''' + + def __init__(self, id, directory, *args, **kwargs): + super(SplitYamlProvider, self).__init__(id, directory, *args, **kwargs) + self.log = logging.getLogger('SplitYamlProvider[{}]'.format(id)) + + def populate(self, zone, target=False, lenient=False): + self.log.debug('populate: name=%s, target=%s, lenient=%s', zone.name, + target, lenient) + + if target: + # When acting as a target we ignore any existing records so that we + # create a completely new copy + return False + + before = len(zone.records) + yaml_filenames = list_all_yaml_files(self.directory) + 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) + + self.log.info('populate: found %s records, exists=False', + len(zone.records) - before) + return False + + def _do_apply(self, desired, data): + 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)) + 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)) + self.log.debug('_apply: writing catchall filename=%s', filename) + with open(filename, 'w') as fh: + safe_dump(catchall, fh) From 81d9c083fc08a98b483b4ec1680980e3356fd1db Mon Sep 17 00:00:00 2001 From: Christian Funkhouser Date: Wed, 3 Apr 2019 23:15:07 -0400 Subject: [PATCH 02/15] Add --split flag to dump Signed-off-by: Christian Funkhouser --- octodns/cmds/dump.py | 6 +++++- octodns/manager.py | 9 ++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/octodns/cmds/dump.py b/octodns/cmds/dump.py index 9f5e0aa..9927468 100755 --- a/octodns/cmds/dump.py +++ b/octodns/cmds/dump.py @@ -21,6 +21,9 @@ def main(): parser.add_argument('--lenient', action='store_true', default=False, help='Ignore record validations and do a best effort ' 'dump') + parser.add_argument('--split', action='store_true', default=False, + help='Split the dumped zone into a YAML file per ' + 'record') parser.add_argument('zone', help='Zone to dump') parser.add_argument('source', nargs='+', help='Source(s) to pull data from') @@ -28,7 +31,8 @@ def main(): args = parser.parse_args() manager = Manager(args.config_file) - manager.dump(args.zone, args.output_dir, args.lenient, *args.source) + manager.dump(args.zone, args.output_dir, args.lenient, args.split, + *args.source) if __name__ == '__main__': diff --git a/octodns/manager.py b/octodns/manager.py index eade87a..4952315 100644 --- a/octodns/manager.py +++ b/octodns/manager.py @@ -12,7 +12,7 @@ import logging from .provider.base import BaseProvider from .provider.plan import Plan -from .provider.yaml import YamlProvider +from .provider.yaml import SplitYamlProvider, YamlProvider from .record import Record from .yaml import safe_load from .zone import Zone @@ -357,7 +357,7 @@ class Manager(object): return zb.changes(za, _AggregateTarget(a + b)) - def dump(self, zone, output_dir, lenient, source, *sources): + def dump(self, zone, output_dir, lenient, split, source, *sources): ''' Dump zone data from the specified source ''' @@ -372,7 +372,10 @@ class Manager(object): except KeyError as e: raise Exception('Unknown source: {}'.format(e.args[0])) - target = YamlProvider('dump', output_dir) + clz = YamlProvider + if split: + clz = SplitYamlProvider + target = clz('dump', output_dir) zone = Zone(zone, self.configured_sub_zones(zone)) for source in sources: From 3bc0e0ad3ecf324da88ace817ae3c9ce09a9138b Mon Sep 17 00:00:00 2001 From: Christian Funkhouser Date: Thu, 4 Apr 2019 09:35:58 -0400 Subject: [PATCH 03/15] File load success is too verbose Signed-off-by: Christian Funkhouser --- octodns/provider/yaml.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index f5652c0..ab9bde9 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -59,7 +59,7 @@ class YamlProvider(BaseProvider): record = Record.new(zone, name, d, source=self, lenient=lenient) zone.add_record(record, lenient=lenient) - self.log.info( + self.log.debug( '_populate_from_file: successfully loaded "%s"', filename) def populate(self, zone, target=False, lenient=False): From ee133b3ac16243df7a1b6901929aa165cc9ec53a Mon Sep 17 00:00:00 2001 From: Christian Funkhouser Date: Thu, 4 Apr 2019 10:28:55 -0400 Subject: [PATCH 04/15] Fix tests Signed-off-by: Christian Funkhouser --- tests/test_octodns_manager.py | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 0e14bab..bd48b4a 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -221,27 +221,47 @@ class TestManager(TestCase): manager = Manager(get_config_filename('simple.yaml')) with self.assertRaises(Exception) as ctx: - manager.dump('unit.tests.', tmpdir.dirname, False, 'nope') + manager.dump('unit.tests.', tmpdir.dirname, False, False, + 'nope') self.assertEquals('Unknown source: nope', ctx.exception.message) - manager.dump('unit.tests.', tmpdir.dirname, False, 'in') + manager.dump('unit.tests.', tmpdir.dirname, False, False, 'in') # make sure this fails with an IOError and not a KeyError when # tyring to find sub zones with self.assertRaises(IOError): - manager.dump('unknown.zone.', tmpdir.dirname, False, 'in') + manager.dump('unknown.zone.', tmpdir.dirname, False, False, + 'in') def test_dump_empty(self): with TemporaryDirectory() as tmpdir: environ['YAML_TMP_DIR'] = tmpdir.dirname manager = Manager(get_config_filename('simple.yaml')) - manager.dump('empty.', tmpdir.dirname, False, 'in') + manager.dump('empty.', tmpdir.dirname, False, False, 'in') with open(join(tmpdir.dirname, 'empty.yaml')) as fh: data = safe_load(fh, False) self.assertFalse(data) + def test_dump_split(self): + with TemporaryDirectory() as tmpdir: + environ['YAML_TMP_DIR'] = tmpdir.dirname + manager = Manager(get_config_filename('simple-split.yaml')) + + with self.assertRaises(Exception) as ctx: + manager.dump('unit.tests.', tmpdir.dirname, False, True, + 'nope') + self.assertEquals('Unknown source: nope', ctx.exception.message) + + manager.dump('unit.tests.', tmpdir.dirname, False, True, 'in') + + # make sure this fails with an IOError and not a KeyError when + # tyring to find sub zones + with self.assertRaises(IOError): + manager.dump('unknown.zone.', tmpdir.dirname, False, True, + 'in') + def test_validate_configs(self): Manager(get_config_filename('simple-validate.yaml')).validate_configs() From 8639bc0af593a5f24fb0b974f9f9f923aded4512 Mon Sep 17 00:00:00 2001 From: Christian Funkhouser Date: Thu, 4 Apr 2019 10:29:55 -0400 Subject: [PATCH 05/15] Actually fix tests, include YAML Signed-off-by: Christian Funkhouser --- tests/config/simple-split.yaml | 37 ++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 tests/config/simple-split.yaml diff --git a/tests/config/simple-split.yaml b/tests/config/simple-split.yaml new file mode 100644 index 0000000..62d9f16 --- /dev/null +++ b/tests/config/simple-split.yaml @@ -0,0 +1,37 @@ +manager: + max_workers: 2 +providers: + in: + class: octodns.provider.yaml.SplitYamlProvider + directory: tests/config + dump: + class: octodns.provider.yaml.SplitYamlProvider + directory: env/YAML_TMP_DIR + # This is sort of ugly, but it shouldn't hurt anything. It'll just write out + # the target file twice where it and dump are both used + dump2: + class: octodns.provider.yaml.SplitYamlProvider + directory: env/YAML_TMP_DIR + simple: + class: helpers.SimpleProvider + geo: + class: helpers.GeoProvider + nosshfp: + class: helpers.NoSshFpProvider +zones: + unit.tests.: + sources: + - in + targets: + - dump + subzone.unit.tests.: + sources: + - in + targets: + - dump + - dump2 + empty.: + sources: + - in + targets: + - dump From 1d9553b93ad748bae1f3bff53a6dddffa3d568dd Mon Sep 17 00:00:00 2001 From: Christian Funkhouser Date: Thu, 4 Apr 2019 10:33:31 -0400 Subject: [PATCH 06/15] Appease the linter. Signed-off-by: Christian Funkhouser --- tests/test_octodns_manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index bd48b4a..4c2293e 100644 --- a/tests/test_octodns_manager.py +++ b/tests/test_octodns_manager.py @@ -222,7 +222,7 @@ class TestManager(TestCase): with self.assertRaises(Exception) as ctx: manager.dump('unit.tests.', tmpdir.dirname, False, False, - 'nope') + 'nope') self.assertEquals('Unknown source: nope', ctx.exception.message) manager.dump('unit.tests.', tmpdir.dirname, False, False, 'in') @@ -231,7 +231,7 @@ class TestManager(TestCase): # tyring to find sub zones with self.assertRaises(IOError): manager.dump('unknown.zone.', tmpdir.dirname, False, False, - 'in') + 'in') def test_dump_empty(self): with TemporaryDirectory() as tmpdir: From 98dacd2dde6b4a673af5d3e4cb27a6df5f1cdec3 Mon Sep 17 00:00:00 2001 From: Christian Funkhouser Date: Thu, 4 Apr 2019 21:41:57 -0400 Subject: [PATCH 07/15] 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 --- octodns/provider/yaml.py | 18 +- tests/config/dynamic.tests.yaml | 4 + tests/config/simple-split.yaml | 2 +- tests/config/split/dynamic.tests./a.yaml | 46 ++++ tests/config/split/dynamic.tests./aaaa.yaml | 46 ++++ tests/config/split/dynamic.tests./cname.yaml | 42 ++++ .../split/dynamic.tests./real-ish-a.yaml | 87 +++++++ .../split/dynamic.tests./simple-weighted.yaml | 15 ++ .../config/split/subzone.unit.tests./12.yaml | 4 + tests/config/split/subzone.unit.tests./2.yaml | 4 + .../split/subzone.unit.tests./test.yaml | 4 + .../config/split/unit.tests./$unit.tests.yaml | 37 +++ tests/config/split/unit.tests./_srv._tcp.yaml | 13 ++ tests/config/split/unit.tests./aaaa.yaml | 5 + tests/config/split/unit.tests./cname.yaml | 5 + tests/config/split/unit.tests./excluded.yaml | 7 + tests/config/split/unit.tests./ignored.yaml | 6 + tests/config/split/unit.tests./included.yaml | 7 + tests/config/split/unit.tests./mx.yaml | 13 ++ tests/config/split/unit.tests./naptr.yaml | 17 ++ tests/config/split/unit.tests./ptr.yaml | 5 + tests/config/split/unit.tests./spf.yaml | 5 + tests/config/split/unit.tests./sub.yaml | 6 + tests/config/split/unit.tests./txt.yaml | 8 + tests/config/split/unit.tests./www.sub.yaml | 5 + tests/config/split/unit.tests./www.yaml | 5 + tests/config/split/unordered./abc.yaml | 4 + tests/config/split/unordered./xyz.yaml | 5 + tests/test_octodns_manager.py | 4 +- tests/test_octodns_provider_splityaml.py | 219 ++++++++++++++++++ 30 files changed, 638 insertions(+), 10 deletions(-) create mode 100644 tests/config/split/dynamic.tests./a.yaml create mode 100644 tests/config/split/dynamic.tests./aaaa.yaml create mode 100644 tests/config/split/dynamic.tests./cname.yaml create mode 100644 tests/config/split/dynamic.tests./real-ish-a.yaml create mode 100644 tests/config/split/dynamic.tests./simple-weighted.yaml create mode 100644 tests/config/split/subzone.unit.tests./12.yaml create mode 100644 tests/config/split/subzone.unit.tests./2.yaml create mode 100644 tests/config/split/subzone.unit.tests./test.yaml create mode 100644 tests/config/split/unit.tests./$unit.tests.yaml create mode 100644 tests/config/split/unit.tests./_srv._tcp.yaml create mode 100644 tests/config/split/unit.tests./aaaa.yaml create mode 100644 tests/config/split/unit.tests./cname.yaml create mode 100644 tests/config/split/unit.tests./excluded.yaml create mode 100644 tests/config/split/unit.tests./ignored.yaml create mode 100644 tests/config/split/unit.tests./included.yaml create mode 100644 tests/config/split/unit.tests./mx.yaml create mode 100644 tests/config/split/unit.tests./naptr.yaml create mode 100644 tests/config/split/unit.tests./ptr.yaml create mode 100644 tests/config/split/unit.tests./spf.yaml create mode 100644 tests/config/split/unit.tests./sub.yaml create mode 100644 tests/config/split/unit.tests./txt.yaml create mode 100644 tests/config/split/unit.tests./www.sub.yaml create mode 100644 tests/config/split/unit.tests./www.yaml create mode 100644 tests/config/split/unordered./abc.yaml create mode 100644 tests/config/split/unordered./xyz.yaml create mode 100644 tests/test_octodns_provider_splityaml.py diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index ab9bde9..99fc2e0 100644 --- a/octodns/provider/yaml.py +++ b/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) diff --git a/tests/config/dynamic.tests.yaml b/tests/config/dynamic.tests.yaml index fb33aec..3d806f9 100644 --- a/tests/config/dynamic.tests.yaml +++ b/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 diff --git a/tests/config/simple-split.yaml b/tests/config/simple-split.yaml index 62d9f16..d106506 100644 --- a/tests/config/simple-split.yaml +++ b/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 diff --git a/tests/config/split/dynamic.tests./a.yaml b/tests/config/split/dynamic.tests./a.yaml new file mode 100644 index 0000000..fd748b4 --- /dev/null +++ b/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 diff --git a/tests/config/split/dynamic.tests./aaaa.yaml b/tests/config/split/dynamic.tests./aaaa.yaml new file mode 100644 index 0000000..3b1dcb7 --- /dev/null +++ b/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 diff --git a/tests/config/split/dynamic.tests./cname.yaml b/tests/config/split/dynamic.tests./cname.yaml new file mode 100644 index 0000000..a84c202 --- /dev/null +++ b/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. diff --git a/tests/config/split/dynamic.tests./real-ish-a.yaml b/tests/config/split/dynamic.tests./real-ish-a.yaml new file mode 100644 index 0000000..0338b9d --- /dev/null +++ b/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 diff --git a/tests/config/split/dynamic.tests./simple-weighted.yaml b/tests/config/split/dynamic.tests./simple-weighted.yaml new file mode 100644 index 0000000..1c722dd --- /dev/null +++ b/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. diff --git a/tests/config/split/subzone.unit.tests./12.yaml b/tests/config/split/subzone.unit.tests./12.yaml new file mode 100644 index 0000000..e5d4dff --- /dev/null +++ b/tests/config/split/subzone.unit.tests./12.yaml @@ -0,0 +1,4 @@ +--- +'12': + type: A + value: 12.4.4.4 diff --git a/tests/config/split/subzone.unit.tests./2.yaml b/tests/config/split/subzone.unit.tests./2.yaml new file mode 100644 index 0000000..812cb49 --- /dev/null +++ b/tests/config/split/subzone.unit.tests./2.yaml @@ -0,0 +1,4 @@ +--- +'2': + type: A + value: 2.4.4.4 diff --git a/tests/config/split/subzone.unit.tests./test.yaml b/tests/config/split/subzone.unit.tests./test.yaml new file mode 100644 index 0000000..bc28512 --- /dev/null +++ b/tests/config/split/subzone.unit.tests./test.yaml @@ -0,0 +1,4 @@ +--- +test: + type: A + value: 4.4.4.4 diff --git a/tests/config/split/unit.tests./$unit.tests.yaml b/tests/config/split/unit.tests./$unit.tests.yaml new file mode 100644 index 0000000..cf85a87 --- /dev/null +++ b/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 diff --git a/tests/config/split/unit.tests./_srv._tcp.yaml b/tests/config/split/unit.tests./_srv._tcp.yaml new file mode 100644 index 0000000..220731e --- /dev/null +++ b/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 diff --git a/tests/config/split/unit.tests./aaaa.yaml b/tests/config/split/unit.tests./aaaa.yaml new file mode 100644 index 0000000..845ab70 --- /dev/null +++ b/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 diff --git a/tests/config/split/unit.tests./cname.yaml b/tests/config/split/unit.tests./cname.yaml new file mode 100644 index 0000000..8664bd2 --- /dev/null +++ b/tests/config/split/unit.tests./cname.yaml @@ -0,0 +1,5 @@ +--- +cname: + ttl: 300 + type: CNAME + value: unit.tests. diff --git a/tests/config/split/unit.tests./excluded.yaml b/tests/config/split/unit.tests./excluded.yaml new file mode 100644 index 0000000..7d9cb56 --- /dev/null +++ b/tests/config/split/unit.tests./excluded.yaml @@ -0,0 +1,7 @@ +--- +excluded: + octodns: + excluded: + - test + type: CNAME + value: unit.tests. diff --git a/tests/config/split/unit.tests./ignored.yaml b/tests/config/split/unit.tests./ignored.yaml new file mode 100644 index 0000000..4d55eb2 --- /dev/null +++ b/tests/config/split/unit.tests./ignored.yaml @@ -0,0 +1,6 @@ +--- +ignored: + octodns: + ignored: true + type: A + value: 9.9.9.9 diff --git a/tests/config/split/unit.tests./included.yaml b/tests/config/split/unit.tests./included.yaml new file mode 100644 index 0000000..21d9e50 --- /dev/null +++ b/tests/config/split/unit.tests./included.yaml @@ -0,0 +1,7 @@ +--- +included: + octodns: + included: + - test + type: CNAME + value: unit.tests. diff --git a/tests/config/split/unit.tests./mx.yaml b/tests/config/split/unit.tests./mx.yaml new file mode 100644 index 0000000..87ca909 --- /dev/null +++ b/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 diff --git a/tests/config/split/unit.tests./naptr.yaml b/tests/config/split/unit.tests./naptr.yaml new file mode 100644 index 0000000..f010d2f --- /dev/null +++ b/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 diff --git a/tests/config/split/unit.tests./ptr.yaml b/tests/config/split/unit.tests./ptr.yaml new file mode 100644 index 0000000..0098b57 --- /dev/null +++ b/tests/config/split/unit.tests./ptr.yaml @@ -0,0 +1,5 @@ +--- +ptr: + ttl: 300 + type: PTR + value: foo.bar.com. diff --git a/tests/config/split/unit.tests./spf.yaml b/tests/config/split/unit.tests./spf.yaml new file mode 100644 index 0000000..9321108 --- /dev/null +++ b/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 diff --git a/tests/config/split/unit.tests./sub.yaml b/tests/config/split/unit.tests./sub.yaml new file mode 100644 index 0000000..ebd3d47 --- /dev/null +++ b/tests/config/split/unit.tests./sub.yaml @@ -0,0 +1,6 @@ +--- +sub: + type: NS + values: + - 6.2.3.4. + - 7.2.3.4. diff --git a/tests/config/split/unit.tests./txt.yaml b/tests/config/split/unit.tests./txt.yaml new file mode 100644 index 0000000..73eaba7 --- /dev/null +++ b/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 diff --git a/tests/config/split/unit.tests./www.sub.yaml b/tests/config/split/unit.tests./www.sub.yaml new file mode 100644 index 0000000..8cfd33f --- /dev/null +++ b/tests/config/split/unit.tests./www.sub.yaml @@ -0,0 +1,5 @@ +--- +www.sub: + ttl: 300 + type: A + value: 2.2.3.6 diff --git a/tests/config/split/unit.tests./www.yaml b/tests/config/split/unit.tests./www.yaml new file mode 100644 index 0000000..d6d4ab0 --- /dev/null +++ b/tests/config/split/unit.tests./www.yaml @@ -0,0 +1,5 @@ +--- +www: + ttl: 300 + type: A + value: 2.2.3.6 diff --git a/tests/config/split/unordered./abc.yaml b/tests/config/split/unordered./abc.yaml new file mode 100644 index 0000000..e0ccccc --- /dev/null +++ b/tests/config/split/unordered./abc.yaml @@ -0,0 +1,4 @@ +--- +abc: + type: A + value: 9.9.9.9 diff --git a/tests/config/split/unordered./xyz.yaml b/tests/config/split/unordered./xyz.yaml new file mode 100644 index 0000000..14db338 --- /dev/null +++ b/tests/config/split/unordered./xyz.yaml @@ -0,0 +1,5 @@ +--- +xyz: + # t comes before v + value: 9.9.9.9 + type: A diff --git a/tests/test_octodns_manager.py b/tests/test_octodns_manager.py index 4c2293e..0dd3514 100644 --- a/tests/test_octodns_manager.py +++ b/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') diff --git a/tests/test_octodns_provider_splityaml.py b/tests/test_octodns_provider_splityaml.py new file mode 100644 index 0000000..facd251 --- /dev/null +++ b/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) From ceece68de8c723d7c43252af593a6737ee7e14bf Mon Sep 17 00:00:00 2001 From: Christian Funkhouser Date: Thu, 4 Apr 2019 21:49:14 -0400 Subject: [PATCH 08/15] Persist empty. zone Signed-off-by: Christian Funkhouser --- tests/config/split/empty./.gitkeep | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 tests/config/split/empty./.gitkeep diff --git a/tests/config/split/empty./.gitkeep b/tests/config/split/empty./.gitkeep new file mode 100644 index 0000000..e69de29 From 9f34526c6103f6cb221dfeba9b87549c1fb03c58 Mon Sep 17 00:00:00 2001 From: Christian Funkhouser Date: Fri, 5 Apr 2019 16:54:09 -0400 Subject: [PATCH 09/15] Remove forklifted comment that doesn't entirely make sense Signed-off-by: Christian Funkhouser --- tests/test_octodns_provider_splityaml.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tests/test_octodns_provider_splityaml.py b/tests/test_octodns_provider_splityaml.py index facd251..ff314f4 100644 --- a/tests/test_octodns_provider_splityaml.py +++ b/tests/test_octodns_provider_splityaml.py @@ -81,15 +81,6 @@ class TestSplitYamlProvider(TestCase): 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') From 2e2fd7157ad321c2e6d918b300f35957a8978b3c Mon Sep 17 00:00:00 2001 From: Christian Funkhouser Date: Mon, 8 Apr 2019 14:55:04 -0400 Subject: [PATCH 10/15] Try the test with a fresh zone Signed-off-by: Christian Funkhouser --- tests/test_octodns_provider_splityaml.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_octodns_provider_splityaml.py b/tests/test_octodns_provider_splityaml.py index ff314f4..4a7ecfe 100644 --- a/tests/test_octodns_provider_splityaml.py +++ b/tests/test_octodns_provider_splityaml.py @@ -191,6 +191,8 @@ class TestSplitYamlProvider(TestCase): with self.assertRaises(ConstructorError): source.populate(zone) + zone = Zone('unordered.', []) + source = SplitYamlProvider( 'test', join(dirname(__file__), 'config/split'), enforce_order=False) From 250c31f8ed4997936e20d677c95ed497bac1562c Mon Sep 17 00:00:00 2001 From: Christian Funkhouser Date: Mon, 8 Apr 2019 16:46:56 -0400 Subject: [PATCH 11/15] Delete test_octodns_provider_splityaml.py Signed-off-by: Christian Funkhouser --- tests/test_octodns_provider_splityaml.py | 212 ----------------------- 1 file changed, 212 deletions(-) delete mode 100644 tests/test_octodns_provider_splityaml.py diff --git a/tests/test_octodns_provider_splityaml.py b/tests/test_octodns_provider_splityaml.py deleted file mode 100644 index 4a7ecfe..0000000 --- a/tests/test_octodns_provider_splityaml.py +++ /dev/null @@ -1,212 +0,0 @@ -# -# -# - -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)) - - 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) - - zone = Zone('unordered.', []) - - 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) From 689043cd3d0cbce25fed26e578d2e713eb3190db Mon Sep 17 00:00:00 2001 From: Christian Funkhouser Date: Mon, 8 Apr 2019 17:07:45 -0400 Subject: [PATCH 12/15] Merge SplitYamlProvider and YamlProvider tests Signed-off-by: Christian Funkhouser --- octodns/provider/yaml.py | 31 +++-- tests/test_octodns_provider_yaml.py | 199 +++++++++++++++++++++++++++- 2 files changed, 212 insertions(+), 18 deletions(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 99fc2e0..9e0fb24 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -116,20 +116,6 @@ class YamlProvider(BaseProvider): safe_dump(dict(data), fh) -# Any record name added to this set will be included in the catch-all file, -# instead of a file matching the record name. -_CATCHALL_RECORD_NAMES = ('*', '') - - -def list_all_yaml_files(directory): - yaml_files = set() - for f in listdir(directory): - filename = join(directory, '{}'.format(f)) - if f.endswith('.yaml') and isfile(filename): - yaml_files.add(filename) - return list(yaml_files) - - class SplitYamlProvider(YamlProvider): ''' Core provider for records configured in multiple YAML files on disk. @@ -153,6 +139,19 @@ class SplitYamlProvider(YamlProvider): enforce_order: True ''' + # Any record name added to this set will be included in the catch-all file, + # instead of a file matching the record name. + CATCHALL_RECORD_NAMES = ('*', '') + + @classmethod + def list_all_yaml_files(_, directory): + yaml_files = set() + for f in listdir(directory): + filename = join(directory, '{}'.format(f)) + if f.endswith('.yaml') and isfile(filename): + yaml_files.add(filename) + return list(yaml_files) + def __init__(self, id, directory, *args, **kwargs): super(SplitYamlProvider, self).__init__(id, directory, *args, **kwargs) self.log = logging.getLogger('SplitYamlProvider[{}]'.format(id)) @@ -170,7 +169,7 @@ class SplitYamlProvider(YamlProvider): return False before = len(zone.records) - yaml_filenames = list_all_yaml_files(self._zone_directory(zone)) + yaml_filenames = self.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) @@ -186,7 +185,7 @@ class SplitYamlProvider(YamlProvider): catchall = dict() for record, config in data.items(): - if record in _CATCHALL_RECORD_NAMES: + if record in self.CATCHALL_RECORD_NAMES: catchall[record] = config continue filename = join(zone_dir, '{}.yaml'.format(record)) diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 74261de..5ff97ba 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -5,13 +5,15 @@ from __future__ import absolute_import, division, print_function, \ unicode_literals -from os.path import dirname, isfile, join +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.record import Create -from octodns.provider.yaml import YamlProvider +from octodns.provider.base import Plan +from octodns.provider.yaml import SplitYamlProvider, YamlProvider from octodns.zone import SubzoneRecordException, Zone from helpers import TemporaryDirectory @@ -176,3 +178,196 @@ class TestYamlProvider(TestCase): source.populate(zone) self.assertEquals('Record www.sub.unit.tests. is under a managed ' 'subzone', ctx.exception.message) + + +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)) + + # This isn't great, but given the variable nature of the temp dir + # names, it's necessary. + got = (basename(f) + for f in SplitYamlProvider.list_all_yaml_files(directory)) + self.assertItemsEqual(yaml_files, got) + + 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)) + + 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) + + zone = Zone('unordered.', []) + + 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) From 2021a2caeaa8053fa6287ae3e53331d9f3082d40 Mon Sep 17 00:00:00 2001 From: Christian Funkhouser Date: Mon, 8 Apr 2019 17:09:29 -0400 Subject: [PATCH 13/15] Construct YamlProvider logger more cleverly. Signed-off-by: Christian Funkhouser --- octodns/provider/yaml.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 9e0fb24..da63f3c 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -37,7 +37,8 @@ class YamlProvider(BaseProvider): def __init__(self, id, directory, default_ttl=3600, enforce_order=True, *args, **kwargs): - self.log = logging.getLogger('YamlProvider[{}]'.format(id)) + self.log = logging.getLogger('{}[{}]'.format( + self.__class__.__name__, id)) self.log.debug('__init__: id=%s, directory=%s, default_ttl=%d, ' 'enforce_order=%d', id, directory, default_ttl, enforce_order) @@ -154,7 +155,6 @@ class SplitYamlProvider(YamlProvider): def __init__(self, id, directory, *args, **kwargs): 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) From f239eb1aa8da28c22f680b9bfffd2621435782fd Mon Sep 17 00:00:00 2001 From: Christian Funkhouser Date: Mon, 8 Apr 2019 17:13:28 -0400 Subject: [PATCH 14/15] Don't use classmethod for listing YAML files Signed-off-by: Christian Funkhouser --- octodns/provider/yaml.py | 20 ++++++++++---------- tests/test_octodns_provider_yaml.py | 9 +++++---- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index da63f3c..c67f62f 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -117,6 +117,15 @@ class YamlProvider(BaseProvider): safe_dump(dict(data), fh) +def _list_all_yaml_files(directory): + yaml_files = set() + for f in listdir(directory): + filename = join(directory, '{}'.format(f)) + if f.endswith('.yaml') and isfile(filename): + yaml_files.add(filename) + return list(yaml_files) + + class SplitYamlProvider(YamlProvider): ''' Core provider for records configured in multiple YAML files on disk. @@ -144,15 +153,6 @@ class SplitYamlProvider(YamlProvider): # instead of a file matching the record name. CATCHALL_RECORD_NAMES = ('*', '') - @classmethod - def list_all_yaml_files(_, directory): - yaml_files = set() - for f in listdir(directory): - filename = join(directory, '{}'.format(f)) - if f.endswith('.yaml') and isfile(filename): - yaml_files.add(filename) - return list(yaml_files) - def __init__(self, id, directory, *args, **kwargs): super(SplitYamlProvider, self).__init__(id, directory, *args, **kwargs) @@ -169,7 +169,7 @@ class SplitYamlProvider(YamlProvider): return False before = len(zone.records) - yaml_filenames = self.list_all_yaml_files(self._zone_directory(zone)) + 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) diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 5ff97ba..d5d5e37 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -13,7 +13,8 @@ from yaml.constructor import ConstructorError from octodns.record import Create from octodns.provider.base import Plan -from octodns.provider.yaml import SplitYamlProvider, YamlProvider +from octodns.provider.yaml import _list_all_yaml_files, \ + SplitYamlProvider, YamlProvider from octodns.zone import SubzoneRecordException, Zone from helpers import TemporaryDirectory @@ -200,9 +201,9 @@ class TestSplitYamlProvider(TestCase): # This isn't great, but given the variable nature of the temp dir # names, it's necessary. - got = (basename(f) - for f in SplitYamlProvider.list_all_yaml_files(directory)) - self.assertItemsEqual(yaml_files, got) + self.assertItemsEqual( + yaml_files, + (basename(f) for f in _list_all_yaml_files(directory))) def test_zone_directory(self): source = SplitYamlProvider( From a65181b61d7aa497ae11382d93226ad03e19b640 Mon Sep 17 00:00:00 2001 From: Christian Funkhouser Date: Mon, 8 Apr 2019 17:22:26 -0400 Subject: [PATCH 15/15] Document directory requirements more clearly. Signed-off-by: Christian Funkhouser --- octodns/provider/yaml.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index c67f62f..966e96e 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -131,11 +131,22 @@ class SplitYamlProvider(YamlProvider): Core provider for records configured in multiple YAML files on disk. Behaves mostly similarly to YamlConfig, but interacts with multiple YAML - files, instead of a single monolitic one. The files are named RECORD.yaml, - except for any record which cannot be represented easily as a file; these - are stored in the catchall file, which is a YAML file the zone name, - prepended with a '$'. For example, a zone, 'github.com.' would have a - catch-all file named '$github.com.yaml'. + files, instead of a single monolitic one. All files are stored in a + subdirectory matching the name of the zone (including the trailing .) of + the directory config. The files are named RECORD.yaml, except for any + record which cannot be represented easily as a file; these are stored in + the catchall file, which is a YAML file the zone name, prepended with '$'. + For example, a zone, 'github.com.' would have a catch-all file named + '$github.com.yaml'. + + A full directory structure for the zone github.com. managed under directory + "zones/" would be: + + zones/ + github.com./ + $github.com.yaml + www.yaml + ... config: class: octodns.provider.yaml.SplitYamlProvider