From 11ddb20005676a95b5367946ffcea8eb8d3437ae Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 12 Aug 2023 11:18:33 -0700 Subject: [PATCH 01/16] Refactory YamlProvider and SplitYamlProvider into a unified class --- octodns/provider/yaml.py | 315 +++++++++++++++------------- tests/test_octodns_provider_yaml.py | 12 +- 2 files changed, 176 insertions(+), 151 deletions(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 1d495a9..e0a727a 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -28,8 +28,35 @@ class YamlProvider(BaseProvider): # (optional, default True) enforce_order: true # Whether duplicate records should replace rather than error - # (optiona, default False) + # (optional, default False) populate_should_replace: false + # The filename used to load split style zones, False means disabled. + # When enabled the provider will search for zone records split across + # multiple YAML files in a directory with the zone name. + # See "Split Details" below for more information + # (optional, default False, . is the recommended best practice when + # enabling) + split_extension: false + + Split Details + ------------- + + All files are stored in a subdirectory matching the name of the zone + (including the trailing .) of the directory config. It is a recommended + best practice that the files be named RECORD.yaml, but all files are + sourced and processed as if they were a single large file. + + A full directory structure for the zone github.com. managed under directory + "zones/" would be: + + zones/ + github.com./ + .yaml + www.yaml + ... + + Overriding Values + ----------------- Overriding values can be accomplished using multiple yaml providers in the `sources` list where subsequent providers have `populate_should_replace` @@ -98,7 +125,6 @@ class YamlProvider(BaseProvider): You can then sync our records eternally with `--config-file=external.yaml` and internally (with the custom overrides) with `--config-file=internal.yaml` - ''' SUPPORTS_GEO = True @@ -107,6 +133,10 @@ class YamlProvider(BaseProvider): SUPPORTS_DYNAMIC_SUBNETS = True SUPPORTS_MULTIVALUE_PTR = 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 = ('*', '') + def __init__( self, id, @@ -115,19 +145,25 @@ class YamlProvider(BaseProvider): enforce_order=True, populate_should_replace=False, supports_root_ns=True, + split_extension=False, + split_only=False, + split_catchall=False, *args, **kwargs, ): klass = self.__class__.__name__ self.log = logging.getLogger(f'{klass}[{id}]') self.log.debug( - '__init__: id=%s, directory=%s, default_ttl=%d, ' - 'enforce_order=%d, populate_should_replace=%d', + '__init__: id=%s, directory=%s, default_ttl=%d, enforce_order=%d, populate_should_replace=%s, supports_root_ns=%s, split_extension=%s, split_only=%s, split_catchall=%s', id, directory, default_ttl, enforce_order, populate_should_replace, + supports_root_ns, + split_extension, + split_only, + split_catchall, ) super().__init__(id, *args, **kwargs) self.directory = directory @@ -135,12 +171,15 @@ class YamlProvider(BaseProvider): self.enforce_order = enforce_order self.populate_should_replace = populate_should_replace self.supports_root_ns = supports_root_ns + self.split_extension = split_extension + self.split_only = split_only + self.split_catchall = split_catchall def copy(self): - args = dict(self.__dict__) - args['id'] = f'{args["id"]}-copy' - del args['log'] - return self.__class__(**args) + kwargs = dict(self.__dict__) + kwargs['id'] = f'{kwargs["id"]}-copy' + del kwargs['log'] + return YamlProvider(**kwargs) @property def SUPPORTS(self): @@ -162,6 +201,37 @@ class YamlProvider(BaseProvider): def SUPPORTS_ROOT_NS(self): return self.supports_root_ns + def list_zones(self): + self.log.debug('list_zones:') + zones = set() + + # TODO: don't allow both utf8 and idna versions of the same zone + extension = self.split_extension + if extension: + self.log.debug('list_zones: looking for split zones') + # look for split + # we want to leave the . + trim = len(extension) - 1 + for dirname in listdir(self.directory): + if not dirname.endswith(extension) or not isdir( + join(self.directory, dirname) + ): + continue + zones.add(dirname[:-trim]) + + if not self.split_only: + self.log.debug('list_zones: looking for zone files') + for filename in listdir(self.directory): + if ( + not filename.endswith('.yaml') + or filename.count('.') < 2 + or not isfile(join(self.directory, filename)) + ): + continue + zones.add(filename[:-4]) + + return sorted(zones) + def _populate_from_file(self, filename, zone, lenient): with open(filename, 'r') as fh: yaml_data = safe_load(fh, enforce_order=self.enforce_order) @@ -184,18 +254,6 @@ class YamlProvider(BaseProvider): '_populate_from_file: successfully loaded "%s"', filename ) - def get_filenames(self, zone): - return ( - join(self.directory, f'{zone.decoded_name}yaml'), - join(self.directory, f'{zone.name}yaml'), - ) - - def list_zones(self): - for filename in listdir(self.directory): - if not filename.endswith('.yaml') or filename.count('.') < 2: - continue - yield filename[:-4] - def populate(self, zone, target=False, lenient=False): self.log.debug( 'populate: name=%s, target=%s, lenient=%s', @@ -210,23 +268,52 @@ class YamlProvider(BaseProvider): return False before = len(zone.records) - utf8_filename, idna_filename = self.get_filenames(zone) - # we prefer utf8 - if isfile(utf8_filename): - if utf8_filename != idna_filename and isfile(idna_filename): - raise ProviderException( - f'Both UTF-8 "{utf8_filename}" and IDNA "{idna_filename}" exist for {zone.decoded_name}' - ) - filename = utf8_filename - else: - self.log.warning( - 'populate: "%s" does not exist, falling back to try idna version "%s"', - utf8_filename, - idna_filename, - ) - filename = idna_filename - self._populate_from_file(filename, zone, lenient) + sources = [] + + zone_name_utf8 = zone.name[:-1] + zone_name_idna = zone.decoded_name[:-1] + + directory = None + split_extension = self.split_extension + if split_extension: + utf8 = join(self.directory, f'{zone_name_utf8}{split_extension}') + idna = join(self.directory, f'{zone_name_idna}{split_extension}') + directory = None + if isdir(utf8): + if utf8 != idna and isdir(idna): + raise ProviderException( + f'Both UTF-8 "{utf8}" and IDNA "{idna}" exist for {zone.decoded_name}' + ) + directory = utf8 + else: + directory = idna + + for filename in listdir(directory): + if filename.endswith('.yaml'): + sources.append(join(directory, filename)) + + if not self.split_only: + utf8 = join(self.directory, f'{zone_name_utf8}.yaml') + idna = join(self.directory, f'{zone_name_idna}.yaml') + if isfile(utf8): + if utf8 != idna and isfile(idna): + raise ProviderException( + f'Both UTF-8 "{utf8}" and IDNA "{idna}" exist for {zone.decoded_name}' + ) + sources.append(utf8) + else: + sources.append(idna) + + if len(sources) == 0: + # TODO: what if we don't have any files + pass + + # determinstically order our sources + sources.sort() + + for source in sources: + self._populate_from_file(source, zone, lenient) self.log.info( 'populate: found %s records, exists=False', @@ -264,123 +351,65 @@ class YamlProvider(BaseProvider): data[k] = data[k][0] if not isdir(self.directory): + self.log.debug('_apply: creating directory=%s', self.directory) makedirs(self.directory) - self._do_apply(desired, data) - - def _do_apply(self, desired, data): - filename = join(self.directory, f'{desired.decoded_name}yaml') - self.log.debug('_apply: writing filename=%s', filename) - with open(filename, 'w') as fh: - safe_dump(dict(data), fh, allow_unicode=True) + if self.split_extension: + # we're going to do split files + decoded_name = desired.decoded_name[:-1] + directory = join( + self.directory, f'{decoded_name}{self.split_extension}' + ) + if not isdir(directory): + self.log.debug('_apply: creating split directory=%s', directory) + makedirs(directory) + + catchall = {} + for record, config in data.items(): + if self.split_catchall and record in self.CATCHALL_RECORD_NAMES: + catchall[record] = config + continue + filename = join(directory, f'{record}.yaml') + 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: + # Scrub the trailing . to make filenames more sane. + filename = join(directory, f'${decoded_name}.yaml') + self.log.debug( + '_apply: writing catchall filename=%s', filename + ) + with open(filename, 'w') as fh: + safe_dump(catchall, fh) -def _list_all_yaml_files(directory): - yaml_files = set() - for f in listdir(directory): - filename = join(directory, f) - if f.endswith('.yaml') and isfile(filename): - yaml_files.add(filename) - return list(yaml_files) + else: + # single large file + filename = join(self.directory, f'{desired.decoded_name}yaml') + self.log.debug('_apply: writing filename=%s', filename) + with open(filename, 'w') as fh: + safe_dump(dict(data), fh, allow_unicode=True) 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. 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'. + DEPRECATED: Use YamlProvider with the split_extension parameter instead. - 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 - # 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 + TO BE REMOVED: 2.0 ''' - # 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 __init__(self, id, directory, extension='.', *args, **kwargs): - super().__init__(id, directory, *args, **kwargs) - self.extension = extension - - def _zone_directory(self, zone): - filename = f'{zone.name[:-1]}{self.extension}' - return join(self.directory, filename) - - def list_zones(self): - n = len(self.extension) - 1 - for filename in listdir(self.directory): - if not filename.endswith(self.extension): - continue - yield filename[:-n] - - def populate(self, zone, target=False, lenient=False): - self.log.debug( - 'populate: name=%s, target=%s, lenient=%s', - zone.name, - target, - lenient, + def __init__(self, id, directory, *args, extension='.', **kwargs): + kwargs.update( + { + 'split_extension': extension, + 'split_only': True, + 'split_catchall': True, + } ) - - 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._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) - - self.log.info( - 'populate: found %s records, exists=False', - len(zone.records) - before, + super().__init__(id, directory, *args, **kwargs) + self.log.warning( + '__init__: DEPRECATED use YamlProvider with split_extension and optionally split_only instead, will go away in v2.0' ) - 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 self.CATCHALL_RECORD_NAMES: - catchall[record] = config - continue - filename = join(zone_dir, f'{record}.yaml') - 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: - # Scrub the trailing . to make filenames more sane. - dname = desired.name[:-1] - filename = join(zone_dir, f'${dname}.yaml') - self.log.debug('_apply: writing catchall filename=%s', filename) - with open(filename, 'w') as fh: - safe_dump(catchall, fh) diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 1cf017a..8f52901 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -3,7 +3,7 @@ # from os import makedirs -from os.path import basename, dirname, isdir, isfile, join +from os.path import dirname, isdir, isfile, join from unittest import TestCase from helpers import TemporaryDirectory @@ -13,11 +13,7 @@ from yaml.constructor import ConstructorError from octodns.idna import idna_encode from octodns.provider import ProviderException from octodns.provider.base import Plan -from octodns.provider.yaml import ( - SplitYamlProvider, - YamlProvider, - _list_all_yaml_files, -) +from octodns.provider.yaml import SplitYamlProvider, YamlProvider from octodns.record import Create, NsValue, Record, ValuesMixin from octodns.zone import SubzoneRecordException, Zone @@ -327,7 +323,7 @@ class TestSplitYamlProvider(TestCase): # This isn't great, but given the variable nature of the temp dir # names, it's necessary. - d = list(basename(f) for f in _list_all_yaml_files(directory)) + d = [join(directory, f) for f in yaml_files] self.assertEqual(len(yaml_files), len(d)) def test_zone_directory(self): @@ -573,7 +569,7 @@ class TestSplitYamlProvider(TestCase): ) copy = source.copy() self.assertEqual(source.directory, copy.directory) - self.assertEqual(source.extension, copy.extension) + self.assertEqual(source.split_extension, copy.split_extension) self.assertEqual(source.default_ttl, copy.default_ttl) self.assertEqual(source.enforce_order, copy.enforce_order) self.assertEqual( From 5b8498a550681857477aaaddd6a83f1792857759 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sat, 12 Aug 2023 19:31:38 -0700 Subject: [PATCH 02/16] Refactory yaml source logic out to make it easily testable --- octodns/provider/yaml.py | 68 +++++++++++----------- tests/test_octodns_provider_yaml.py | 90 ++++++++++++++++++++++------- 2 files changed, 103 insertions(+), 55 deletions(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index e0a727a..2579414 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -205,11 +205,9 @@ class YamlProvider(BaseProvider): self.log.debug('list_zones:') zones = set() - # TODO: don't allow both utf8 and idna versions of the same zone extension = self.split_extension if extension: self.log.debug('list_zones: looking for split zones') - # look for split # we want to leave the . trim = len(extension) - 1 for dirname in listdir(self.directory): @@ -228,10 +226,41 @@ class YamlProvider(BaseProvider): or not isfile(join(self.directory, filename)) ): continue + # trim off the yaml, leave the . zones.add(filename[:-4]) return sorted(zones) + def _split_sources(self, zone): + ext = self.split_extension + utf8 = join(self.directory, f'{zone.decoded_name[:-1]}{ext}') + idna = join(self.directory, f'{zone.name[:-1]}{ext}') + directory = None + if isdir(utf8): + if utf8 != idna and isdir(idna): + raise ProviderException( + f'Both UTF-8 "{utf8}" and IDNA "{idna}" exist for {zone.decoded_name}' + ) + directory = utf8 + else: + directory = idna + + for filename in listdir(directory): + if filename.endswith('.yaml'): + yield join(directory, filename) + + def _zone_sources(self, zone): + utf8 = join(self.directory, f'{zone.decoded_name}yaml') + idna = join(self.directory, f'{zone.name}yaml') + if isfile(utf8): + if utf8 != idna and isfile(idna): + raise ProviderException( + f'Both UTF-8 "{utf8}" and IDNA "{idna}" exist for {zone.decoded_name}' + ) + return utf8 + + return idna + def _populate_from_file(self, filename, zone, lenient): with open(filename, 'r') as fh: yaml_data = safe_load(fh, enforce_order=self.enforce_order) @@ -271,43 +300,12 @@ class YamlProvider(BaseProvider): sources = [] - zone_name_utf8 = zone.name[:-1] - zone_name_idna = zone.decoded_name[:-1] - - directory = None split_extension = self.split_extension if split_extension: - utf8 = join(self.directory, f'{zone_name_utf8}{split_extension}') - idna = join(self.directory, f'{zone_name_idna}{split_extension}') - directory = None - if isdir(utf8): - if utf8 != idna and isdir(idna): - raise ProviderException( - f'Both UTF-8 "{utf8}" and IDNA "{idna}" exist for {zone.decoded_name}' - ) - directory = utf8 - else: - directory = idna - - for filename in listdir(directory): - if filename.endswith('.yaml'): - sources.append(join(directory, filename)) + sources.extend(self._split_sources(zone)) if not self.split_only: - utf8 = join(self.directory, f'{zone_name_utf8}.yaml') - idna = join(self.directory, f'{zone_name_idna}.yaml') - if isfile(utf8): - if utf8 != idna and isfile(idna): - raise ProviderException( - f'Both UTF-8 "{utf8}" and IDNA "{idna}" exist for {zone.decoded_name}' - ) - sources.append(utf8) - else: - sources.append(idna) - - if len(sources) == 0: - # TODO: what if we don't have any files - pass + sources.append(self._zone_sources(zone)) # determinstically order our sources sources.sort() diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 8f52901..89b4bb1 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -2,8 +2,9 @@ # # -from os import makedirs +from os import makedirs, remove from os.path import dirname, isdir, isfile, join +from shutil import rmtree from unittest import TestCase from helpers import TemporaryDirectory @@ -12,12 +13,16 @@ from yaml.constructor import ConstructorError from octodns.idna import idna_encode from octodns.provider import ProviderException -from octodns.provider.base import Plan from octodns.provider.yaml import SplitYamlProvider, YamlProvider from octodns.record import Create, NsValue, Record, ValuesMixin from octodns.zone import SubzoneRecordException, Zone +def touch(filename): + with open(filename, 'w'): + pass + + class TestYamlProvider(TestCase): def test_provider(self): source = YamlProvider('test', join(dirname(__file__), 'config')) @@ -326,29 +331,74 @@ class TestSplitYamlProvider(TestCase): d = [join(directory, f) for f in yaml_files] self.assertEqual(len(yaml_files), len(d)) - def test_zone_directory(self): - source = SplitYamlProvider( - 'test', join(dirname(__file__), 'config/split'), extension='.tst' - ) + def test_split_sources(self): + with TemporaryDirectory() as td: + directory = join(td.dirname) - zone = Zone('unit.tests.', []) + provider = YamlProvider('test', directory, split_extension='.') - self.assertEqual( - join(dirname(__file__), 'config/split', 'unit.tests.tst'), - source._zone_directory(zone), - ) + zone = Zone('déjà.vu.', []) + zone_utf8 = join(directory, f'{zone.decoded_name}') + zone_idna = join(directory, f'{zone.name}') - def test_apply_handles_existing_zone_directory(self): - with TemporaryDirectory() as td: - provider = SplitYamlProvider( - 'test', join(td.dirname, 'config'), extension='.tst' + filenames = ( + '*.yaml', + '.yaml', + 'www.yaml', + f'${zone.decoded_name}yaml', ) - makedirs(join(td.dirname, 'config', 'does.exist.tst')) - zone = Zone('does.exist.', []) - self.assertTrue(isdir(provider._zone_directory(zone))) - provider.apply(Plan(None, zone, [], True)) - self.assertTrue(isdir(provider._zone_directory(zone))) + # create the utf8 zone dir + makedirs(zone_utf8) + # nothing in it so we should get nothing back + self.assertEqual([], list(provider._split_sources(zone))) + # create some record files + for filename in filenames: + touch(join(zone_utf8, filename)) + # make sure we see them + expected = [join(zone_utf8, f) for f in sorted(filenames)] + self.assertEqual(expected, sorted(provider._split_sources(zone))) + + # add a idna zone directory + makedirs(zone_idna) + for filename in filenames: + touch(join(zone_idna, filename)) + with self.assertRaises(ProviderException) as ctx: + list(provider._split_sources(zone)) + msg = str(ctx.exception) + self.assertTrue('Both UTF-8' in msg) + + # delete the utf8 version + rmtree(zone_utf8) + expected = [join(zone_idna, f) for f in sorted(filenames)] + self.assertEqual(expected, sorted(provider._split_sources(zone))) + + def test_zone_sources(self): + with TemporaryDirectory() as td: + directory = join(td.dirname) + + provider = YamlProvider('test', directory) + + zone = Zone('déjà.vu.', []) + utf8 = join(directory, f'{zone.decoded_name}yaml') + idna = join(directory, f'{zone.name}yaml') + + # create the utf8 version + touch(utf8) + # make sure that's what we get back + self.assertEqual(utf8, provider._zone_sources(zone)) + + # create idna version, both exists + touch(idna) + with self.assertRaises(ProviderException) as ctx: + provider._zone_sources(zone) + msg = str(ctx.exception) + self.assertTrue('Both UTF-8' in msg) + + # delete the utf8 version + remove(utf8) + # make sure that we get the idna one back + self.assertEqual(idna, provider._zone_sources(zone)) def test_provider(self): source = SplitYamlProvider( From 3c304aa6ee5a3a4366d61e19a8d955012bd52b8d Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 17 Aug 2023 09:27:03 -0700 Subject: [PATCH 03/16] Pass through YamlProvider doc and CHANGELOG entry --- CHANGELOG.md | 6 ++++++ octodns/provider/yaml.py | 35 +++++++++++++++++++++++++++++++---- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b8025f..3cdef09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ * New dynamic zone config support that allows wildcard entries in the octoDNS config to be expanded by the source provider(s). See [Dynamic Zone Config](/README.md#dynamic-zone-config) for more information. +* SplitYamlProvider has been deprecated and will be removed in 2.0. YamlProvider + now includes the ability to process split zones when configured to do so and + allows for more flexibility in how things are laid out than was previously + possible. This includes the ability to split some zones and not others and + even to have partially split zones with some records in the primary zone YAML + and others in a split directory. See YamlProvider documentation for more info. #### Stuff diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 2579414..c1c4db8 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -32,11 +32,27 @@ class YamlProvider(BaseProvider): populate_should_replace: false # The filename used to load split style zones, False means disabled. # When enabled the provider will search for zone records split across - # multiple YAML files in a directory with the zone name. + # multiple YAML files in the directory with split_extension appended to + # the zone name. split_extension should include the `.` # See "Split Details" below for more information # (optional, default False, . is the recommended best practice when # enabling) split_extension: false + # Disable loading of the primary zone .yaml file. If split_extension + # is defined both split files and the primary zone .yaml will be loaded + # by default. Setting this to true will disable that and rely soley on + # split files. + # (optional, default False) + split_only: false + # When writing YAML records out to disk with split_extension enabled + # each record is written out into its own file with .yaml appended to + # the name of the record. This would result in files like `.yaml` for + # the apex and `*.yaml` for a wildcard. If your OS doesn't allow such + # filenames or you would prefer to avoid them you can enable + # split_catchall to instead write those records into a file named + # `$[zone.name].yaml` + # (optional, default False) + split_catchall: false Split Details ------------- @@ -44,10 +60,11 @@ class YamlProvider(BaseProvider): All files are stored in a subdirectory matching the name of the zone (including the trailing .) of the directory config. It is a recommended best practice that the files be named RECORD.yaml, but all files are - sourced and processed as if they were a single large file. + sourced and processed ignoring the filenames so it is up to you how to + organize them. - A full directory structure for the zone github.com. managed under directory - "zones/" would be: + With `split_extension: .` the directory structure for the zone github.com. + managed under directory "zones/" would look like: zones/ github.com./ @@ -396,6 +413,16 @@ class SplitYamlProvider(YamlProvider): ''' DEPRECATED: Use YamlProvider with the split_extension parameter instead. + When migrating the following configuration options would result in the same + behavior as SplitYamlProvider + + config: + class: octodns.provider.yaml.YamlProvider + # extension is configured as split_extension + split_extension: . + split_only: true + split_catchall: true + TO BE REMOVED: 2.0 ''' From 10c31e37e7ff8c2eca02af12388a89e8f952d3f1 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 17 Aug 2023 10:31:10 -0700 Subject: [PATCH 04/16] Dump a pip freeze into ci output for ease of seeing module verisons --- script/cibuild | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/script/cibuild b/script/cibuild index f50882c..e652b72 100755 --- a/script/cibuild +++ b/script/cibuild @@ -16,7 +16,8 @@ fi echo "## environment & versions ######################################################" python --version pip --version - +echo "## modules: " +pip freeze echo "## clean up ####################################################################" find octodns tests -name "*.pyc" -exec rm {} \; rm -f *.pyc From 608e367a9bad0ecd3236d3004da3e8b2bdef50b6 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 17 Aug 2023 12:13:42 -0700 Subject: [PATCH 05/16] More extensive tests of YamlProvider.list_zones --- octodns/provider/yaml.py | 8 +++- tests/test_octodns_provider_yaml.py | 71 +++++++++++++++++++++++++++-- 2 files changed, 73 insertions(+), 6 deletions(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index c1c4db8..bd77b19 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -224,15 +224,19 @@ class YamlProvider(BaseProvider): extension = self.split_extension if extension: - self.log.debug('list_zones: looking for split zones') # we want to leave the . trim = len(extension) - 1 + self.log.debug( + 'list_zones: looking for split zones, trim=%d', trim + ) for dirname in listdir(self.directory): if not dirname.endswith(extension) or not isdir( join(self.directory, dirname) ): continue - zones.add(dirname[:-trim]) + if trim: + dirname = dirname[:-trim] + zones.add(dirname) if not self.split_only: self.log.debug('list_zones: looking for zone files') diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 89b4bb1..80ded06 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -19,8 +19,7 @@ from octodns.zone import SubzoneRecordException, Zone def touch(filename): - with open(filename, 'w'): - pass + open(filename, 'w').close() class TestYamlProvider(TestCase): @@ -297,6 +296,7 @@ xn--dj-kia8a: self.assertTrue(source.supports(DummyType(self))) def test_list_zones(self): + # test of pre-existing config that lives on disk provider = YamlProvider('test', 'tests/config') self.assertEqual( [ @@ -305,9 +305,72 @@ xn--dj-kia8a: 'subzone.unit.tests.', 'unit.tests.', ], - sorted(provider.list_zones()), + list(provider.list_zones()), ) + # some synthetic tests to explicitly exercise the full functionality + with TemporaryDirectory() as td: + directory = join(td.dirname) + + # noise + touch(join(directory, 'README.txt')) + # not a zone.name.yaml + touch(join(directory, 'production.yaml')) + + # basic yaml zone files + touch(join(directory, 'unit.test.yaml')) + touch(join(directory, 'sub.unit.test.yaml')) + touch(join(directory, 'other.tld.yaml')) + touch(join(directory, 'both.tld.yaml')) + + # split zones with . + makedirs(join(directory, 'split.test.')) + makedirs(join(directory, 'sub.split.test.')) + makedirs(join(directory, 'other.split.')) + makedirs(join(directory, 'both.tld.')) + + # split zones with .tst + makedirs(join(directory, 'split-ext.test.tst')) + makedirs(join(directory, 'sub.split-ext.test.tst')) + makedirs(join(directory, 'other-ext.split.tst')) + + provider = YamlProvider('test', directory) + + # basic, should only find zone files + self.assertEqual( + ['both.tld.', 'other.tld.', 'sub.unit.test.', 'unit.test.'], + list(provider.list_zones()), + ) + + # include stuff with . AND basic + provider.split_extension = '.' + self.assertEqual( + [ + 'both.tld.', + 'other.split.', + 'other.tld.', + 'split.test.', + 'sub.split.test.', + 'sub.unit.test.', + 'unit.test.', + ], + list(provider.list_zones()), + ) + + provider.split_extension = '.tst' + self.assertEqual( + [ + 'both.tld.', + 'other-ext.split.', + 'other.tld.', + 'split-ext.test.', + 'sub.split-ext.test.', + 'sub.unit.test.', + 'unit.test.', + ], + list(provider.list_zones()), + ) + class TestSplitYamlProvider(TestCase): def test_list_all_yaml_files(self): @@ -321,7 +384,7 @@ class TestSplitYamlProvider(TestCase): # 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() + touch(join(directory, emptyfile)) # Do the same for some fake directories for emptydir in all_dirs: makedirs(join(directory, emptydir)) From e473c32bfbd2babe6b3fe8f23a6861f601488c0f Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 17 Aug 2023 12:23:43 -0700 Subject: [PATCH 06/16] Add some directories to ignore --- tests/test_octodns_provider_yaml.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 80ded06..6e0a615 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -316,6 +316,9 @@ xn--dj-kia8a: touch(join(directory, 'README.txt')) # not a zone.name.yaml touch(join(directory, 'production.yaml')) + # non-zone directories + makedirs(join(directory, 'directory')) + makedirs(join(directory, 'never.matches')) # basic yaml zone files touch(join(directory, 'unit.test.yaml')) From 65096c0f1c8b12f926ed44006cc30d3d1f4dbfd3 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 17 Aug 2023 12:52:13 -0700 Subject: [PATCH 07/16] break up ifs having coverage issues in 3.8/9 --- octodns/provider/yaml.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index bd77b19..e518efc 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -230,9 +230,9 @@ class YamlProvider(BaseProvider): 'list_zones: looking for split zones, trim=%d', trim ) for dirname in listdir(self.directory): - if not dirname.endswith(extension) or not isdir( - join(self.directory, dirname) - ): + not_ends_with = not dirname.endswith(extension) + not_dir = not isdir(join(self.directory, dirname)) + if not_ends_with or not_dir: continue if trim: dirname = dirname[:-trim] @@ -241,11 +241,10 @@ class YamlProvider(BaseProvider): if not self.split_only: self.log.debug('list_zones: looking for zone files') for filename in listdir(self.directory): - if ( - not filename.endswith('.yaml') - or filename.count('.') < 2 - or not isfile(join(self.directory, filename)) - ): + not_ends_with = not filename.endswith('.yaml') + too_few_dots = filename.count('.') < 2 + not_file = not isfile(join(self.directory, filename)) + if not_ends_with or too_few_dots or not_file: continue # trim off the yaml, leave the . zones.add(filename[:-4]) From b0dff4fe04d81eff9976269a2397b29934bb7c56 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 17 Aug 2023 13:15:15 -0700 Subject: [PATCH 08/16] Try reordering if clauses --- 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 e518efc..a4eceeb 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -232,7 +232,7 @@ class YamlProvider(BaseProvider): for dirname in listdir(self.directory): not_ends_with = not dirname.endswith(extension) not_dir = not isdir(join(self.directory, dirname)) - if not_ends_with or not_dir: + if not_dir or not_ends_with: continue if trim: dirname = dirname[:-trim] @@ -244,7 +244,7 @@ class YamlProvider(BaseProvider): not_ends_with = not filename.endswith('.yaml') too_few_dots = filename.count('.') < 2 not_file = not isfile(join(self.directory, filename)) - if not_ends_with or too_few_dots or not_file: + if not_file or not_ends_with or too_few_dots: continue # trim off the yaml, leave the . zones.add(filename[:-4]) From 61d3ed884adf43a74705a155ce67fd3f9ab763a7 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 18 Aug 2023 07:39:47 -0700 Subject: [PATCH 09/16] More YamlProvider details testing --- tests/config/split/unit.tests.yaml | 5 +++++ tests/test_octodns_provider_yaml.py | 23 +++++++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 tests/config/split/unit.tests.yaml diff --git a/tests/config/split/unit.tests.yaml b/tests/config/split/unit.tests.yaml new file mode 100644 index 0000000..e249c43 --- /dev/null +++ b/tests/config/split/unit.tests.yaml @@ -0,0 +1,5 @@ +--- +only-zone-file: + type: TXT + value: Only included when zone file processing is enabled + diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 6e0a615..4fabb54 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -360,6 +360,7 @@ xn--dj-kia8a: list(provider.list_zones()), ) + # include stuff with .tst AND basic provider.split_extension = '.tst' self.assertEqual( [ @@ -374,6 +375,20 @@ xn--dj-kia8a: list(provider.list_zones()), ) + # only .tst + provider.split_only = True + self.assertEqual( + ['other-ext.split.', 'split-ext.test.', 'sub.split-ext.test.'], + list(provider.list_zones()), + ) + + # only . (and both zone) + provider.split_extension = '.' + self.assertEqual( + ['both.tld.', 'other.split.', 'split.test.', 'sub.split.test.'], + list(provider.list_zones()), + ) + class TestSplitYamlProvider(TestCase): def test_list_all_yaml_files(self): @@ -485,6 +500,14 @@ class TestSplitYamlProvider(TestCase): source.populate(zone) self.assertEqual(20, len(zone.records)) + # temporarily enable zone file processing too, we should see one extra + # record that came from unit.tests. + source.split_only = False + zone_both = Zone('unit.tests.', []) + source.populate(zone_both) + self.assertEqual(21, len(zone_both.records)) + source.split_only = True + source.populate(dynamic_zone) self.assertEqual(5, len(dynamic_zone.records)) From 3f7234bfd3803fd0525b5d215ea952319e86a6d8 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 18 Aug 2023 08:18:26 -0700 Subject: [PATCH 10/16] Move sources tests into correct class --- tests/test_octodns_provider_yaml.py | 46 ++++++++++++++--------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 4fabb54..37df75d 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -389,29 +389,6 @@ xn--dj-kia8a: list(provider.list_zones()), ) - -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: - touch(join(directory, emptyfile)) - # 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. - d = [join(directory, f) for f in yaml_files] - self.assertEqual(len(yaml_files), len(d)) - def test_split_sources(self): with TemporaryDirectory() as td: directory = join(td.dirname) @@ -481,6 +458,29 @@ class TestSplitYamlProvider(TestCase): # make sure that we get the idna one back self.assertEqual(idna, provider._zone_sources(zone)) + +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: + touch(join(directory, emptyfile)) + # 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. + d = [join(directory, f) for f in yaml_files] + self.assertEqual(len(yaml_files), len(d)) + def test_provider(self): source = SplitYamlProvider( 'test', From aab868f3455308f9b77b1002a4fbe76ca0f4e788 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 18 Aug 2023 09:25:06 -0700 Subject: [PATCH 11/16] Make sure the only* record isn't showing up when it shouldn't --- tests/test_octodns_provider_yaml.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 37df75d..0743510 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -499,6 +499,7 @@ class TestSplitYamlProvider(TestCase): # without it we see everything source.populate(zone) self.assertEqual(20, len(zone.records)) + self.assertFalse([r for r in zone.records if r.name.startswith('only')]) # temporarily enable zone file processing too, we should see one extra # record that came from unit.tests. @@ -506,10 +507,15 @@ class TestSplitYamlProvider(TestCase): zone_both = Zone('unit.tests.', []) source.populate(zone_both) self.assertEqual(21, len(zone_both.records)) + n = len([r for r in zone_both.records if r.name == 'only-zone-file']) + self.assertEqual(1, n) source.split_only = True source.populate(dynamic_zone) self.assertEqual(5, len(dynamic_zone.records)) + self.assertFalse( + [r for r in dynamic_zone.records if r.name.startswith('only')] + ) with TemporaryDirectory() as td: # Add some subdirs to make sure that it can create them From 779f2f44fab4da27736a2c812886ab8ab7080b12 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Fri, 18 Aug 2023 15:39:53 -0700 Subject: [PATCH 12/16] Rename split_only to disable_zonefile. More accurate and future-proof. Also improve doc a bit --- octodns/provider/yaml.py | 55 ++++++++++++++++------------- tests/test_octodns_provider_yaml.py | 6 ++-- 2 files changed, 34 insertions(+), 27 deletions(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index a4eceeb..1be77d2 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -19,31 +19,34 @@ class YamlProvider(BaseProvider): config: class: octodns.provider.yaml.YamlProvider - # The location of yaml config files (required) + + # The location of yaml config files. By default records are defined in a + # file named for the zone in this directory, the zone file, e.g. + # something.com.yaml. + # (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 + + # Whether or not to enforce sorting order when loading yaml # (optional, default True) enforce_order: true + # Whether duplicate records should replace rather than error # (optional, default False) populate_should_replace: false - # The filename used to load split style zones, False means disabled. - # When enabled the provider will search for zone records split across - # multiple YAML files in the directory with split_extension appended to - # the zone name. split_extension should include the `.` - # See "Split Details" below for more information - # (optional, default False, . is the recommended best practice when + + # The file extension used when loading split style zones, Null means + # disabled. When enabled the provider will search for zone records split + # across multiple YAML files in the directory with split_extension + # appended to the zone name, See "Split Details" below. + # split_extension should include the "." + # (optional, default null, "." is the recommended best practice when # enabling) - split_extension: false - # Disable loading of the primary zone .yaml file. If split_extension - # is defined both split files and the primary zone .yaml will be loaded - # by default. Setting this to true will disable that and rely soley on - # split files. - # (optional, default False) - split_only: false + split_extension: null + # When writing YAML records out to disk with split_extension enabled # each record is written out into its own file with .yaml appended to # the name of the record. This would result in files like `.yaml` for @@ -54,6 +57,10 @@ class YamlProvider(BaseProvider): # (optional, default False) split_catchall: false + # Disable loading of the zone .yaml files. + # (optional, default False) + disable_zonefile: false + Split Details ------------- @@ -163,15 +170,15 @@ class YamlProvider(BaseProvider): populate_should_replace=False, supports_root_ns=True, split_extension=False, - split_only=False, split_catchall=False, + disable_zonefile=False, *args, **kwargs, ): klass = self.__class__.__name__ self.log = logging.getLogger(f'{klass}[{id}]') self.log.debug( - '__init__: id=%s, directory=%s, default_ttl=%d, enforce_order=%d, populate_should_replace=%s, supports_root_ns=%s, split_extension=%s, split_only=%s, split_catchall=%s', + '__init__: id=%s, directory=%s, default_ttl=%d, enforce_order=%d, populate_should_replace=%s, supports_root_ns=%s, split_extension=%s, split_catchall=%s, disable_zonefile=%s', id, directory, default_ttl, @@ -179,8 +186,8 @@ class YamlProvider(BaseProvider): populate_should_replace, supports_root_ns, split_extension, - split_only, split_catchall, + disable_zonefile, ) super().__init__(id, *args, **kwargs) self.directory = directory @@ -189,8 +196,8 @@ class YamlProvider(BaseProvider): self.populate_should_replace = populate_should_replace self.supports_root_ns = supports_root_ns self.split_extension = split_extension - self.split_only = split_only self.split_catchall = split_catchall + self.disable_zonefile = disable_zonefile def copy(self): kwargs = dict(self.__dict__) @@ -238,7 +245,7 @@ class YamlProvider(BaseProvider): dirname = dirname[:-trim] zones.add(dirname) - if not self.split_only: + if not self.disable_zonefile: self.log.debug('list_zones: looking for zone files') for filename in listdir(self.directory): not_ends_with = not filename.endswith('.yaml') @@ -324,7 +331,7 @@ class YamlProvider(BaseProvider): if split_extension: sources.extend(self._split_sources(zone)) - if not self.split_only: + if not self.disable_zonefile: sources.append(self._zone_sources(zone)) # determinstically order our sources @@ -423,8 +430,8 @@ class SplitYamlProvider(YamlProvider): class: octodns.provider.yaml.YamlProvider # extension is configured as split_extension split_extension: . - split_only: true split_catchall: true + disable_zonefile: true TO BE REMOVED: 2.0 ''' @@ -433,11 +440,11 @@ class SplitYamlProvider(YamlProvider): kwargs.update( { 'split_extension': extension, - 'split_only': True, 'split_catchall': True, + 'disable_zonefile': True, } ) super().__init__(id, directory, *args, **kwargs) self.log.warning( - '__init__: DEPRECATED use YamlProvider with split_extension and optionally split_only instead, will go away in v2.0' + '__init__: DEPRECATED use YamlProvider with split_extension, split_catchall, and disable_zonefile instead, will go away in v2.0' ) diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 0743510..ae05b8f 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -376,7 +376,7 @@ xn--dj-kia8a: ) # only .tst - provider.split_only = True + provider.disable_zonefile = True self.assertEqual( ['other-ext.split.', 'split-ext.test.', 'sub.split-ext.test.'], list(provider.list_zones()), @@ -503,13 +503,13 @@ class TestSplitYamlProvider(TestCase): # temporarily enable zone file processing too, we should see one extra # record that came from unit.tests. - source.split_only = False + source.disable_zonefile = False zone_both = Zone('unit.tests.', []) source.populate(zone_both) self.assertEqual(21, len(zone_both.records)) n = len([r for r in zone_both.records if r.name == 'only-zone-file']) self.assertEqual(1, n) - source.split_only = True + source.disable_zonefile = True source.populate(dynamic_zone) self.assertEqual(5, len(dynamic_zone.records)) From 6f39fcc5f7f29d83c8cb6e59904829c89454e032 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 27 Aug 2023 08:37:26 -0700 Subject: [PATCH 13/16] yaml provider is either split or zonefile, not both --- octodns/provider/yaml.py | 30 +++++-------------- tests/test_octodns_provider_yaml.py | 46 ++--------------------------- 2 files changed, 11 insertions(+), 65 deletions(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 1be77d2..93b7615 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -54,12 +54,8 @@ class YamlProvider(BaseProvider): # filenames or you would prefer to avoid them you can enable # split_catchall to instead write those records into a file named # `$[zone.name].yaml` - # (optional, default False) - split_catchall: false - - # Disable loading of the zone .yaml files. - # (optional, default False) - disable_zonefile: false + # (optional, default True) + split_catchall: true Split Details ------------- @@ -170,15 +166,14 @@ class YamlProvider(BaseProvider): populate_should_replace=False, supports_root_ns=True, split_extension=False, - split_catchall=False, - disable_zonefile=False, + split_catchall=True, *args, **kwargs, ): klass = self.__class__.__name__ self.log = logging.getLogger(f'{klass}[{id}]') self.log.debug( - '__init__: id=%s, directory=%s, default_ttl=%d, enforce_order=%d, populate_should_replace=%s, supports_root_ns=%s, split_extension=%s, split_catchall=%s, disable_zonefile=%s', + '__init__: id=%s, directory=%s, default_ttl=%d, enforce_order=%d, populate_should_replace=%s, supports_root_ns=%s, split_extension=%s, split_catchall=%s', id, directory, default_ttl, @@ -187,7 +182,6 @@ class YamlProvider(BaseProvider): supports_root_ns, split_extension, split_catchall, - disable_zonefile, ) super().__init__(id, *args, **kwargs) self.directory = directory @@ -197,7 +191,6 @@ class YamlProvider(BaseProvider): self.supports_root_ns = supports_root_ns self.split_extension = split_extension self.split_catchall = split_catchall - self.disable_zonefile = disable_zonefile def copy(self): kwargs = dict(self.__dict__) @@ -245,7 +238,7 @@ class YamlProvider(BaseProvider): dirname = dirname[:-trim] zones.add(dirname) - if not self.disable_zonefile: + if not self.split_extension: self.log.debug('list_zones: looking for zone files') for filename in listdir(self.directory): not_ends_with = not filename.endswith('.yaml') @@ -331,7 +324,7 @@ class YamlProvider(BaseProvider): if split_extension: sources.extend(self._split_sources(zone)) - if not self.disable_zonefile: + if not self.split_extension: sources.append(self._zone_sources(zone)) # determinstically order our sources @@ -431,20 +424,13 @@ class SplitYamlProvider(YamlProvider): # extension is configured as split_extension split_extension: . split_catchall: true - disable_zonefile: true TO BE REMOVED: 2.0 ''' def __init__(self, id, directory, *args, extension='.', **kwargs): - kwargs.update( - { - 'split_extension': extension, - 'split_catchall': True, - 'disable_zonefile': True, - } - ) + kwargs.update({'split_extension': extension, 'split_catchall': True}) super().__init__(id, directory, *args, **kwargs) self.log.warning( - '__init__: DEPRECATED use YamlProvider with split_extension, split_catchall, and disable_zonefile instead, will go away in v2.0' + '__init__: DEPRECATED use YamlProvider with split_extension and split_catchall instead, will go away in v2.0' ) diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index ae05b8f..98eca7f 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -345,50 +345,20 @@ xn--dj-kia8a: list(provider.list_zones()), ) - # include stuff with . AND basic + # split only . provider.split_extension = '.' self.assertEqual( - [ - 'both.tld.', - 'other.split.', - 'other.tld.', - 'split.test.', - 'sub.split.test.', - 'sub.unit.test.', - 'unit.test.', - ], - list(provider.list_zones()), - ) - - # include stuff with .tst AND basic - provider.split_extension = '.tst' - self.assertEqual( - [ - 'both.tld.', - 'other-ext.split.', - 'other.tld.', - 'split-ext.test.', - 'sub.split-ext.test.', - 'sub.unit.test.', - 'unit.test.', - ], + ['both.tld.', 'other.split.', 'split.test.', 'sub.split.test.'], list(provider.list_zones()), ) # only .tst - provider.disable_zonefile = True + provider.split_extension = '.tst' self.assertEqual( ['other-ext.split.', 'split-ext.test.', 'sub.split-ext.test.'], list(provider.list_zones()), ) - # only . (and both zone) - provider.split_extension = '.' - self.assertEqual( - ['both.tld.', 'other.split.', 'split.test.', 'sub.split.test.'], - list(provider.list_zones()), - ) - def test_split_sources(self): with TemporaryDirectory() as td: directory = join(td.dirname) @@ -501,16 +471,6 @@ class TestSplitYamlProvider(TestCase): self.assertEqual(20, len(zone.records)) self.assertFalse([r for r in zone.records if r.name.startswith('only')]) - # temporarily enable zone file processing too, we should see one extra - # record that came from unit.tests. - source.disable_zonefile = False - zone_both = Zone('unit.tests.', []) - source.populate(zone_both) - self.assertEqual(21, len(zone_both.records)) - n = len([r for r in zone_both.records if r.name == 'only-zone-file']) - self.assertEqual(1, n) - source.disable_zonefile = True - source.populate(dynamic_zone) self.assertEqual(5, len(dynamic_zone.records)) self.assertFalse( From 77b1d7a1bab91d06780e4509e5bf3470915b0f5e Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Tue, 29 Aug 2023 12:34:23 -0700 Subject: [PATCH 14/16] Correct split_catchall doc --- octodns/provider/yaml.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 93b7615..2d4ee81 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -49,11 +49,12 @@ class YamlProvider(BaseProvider): # When writing YAML records out to disk with split_extension enabled # each record is written out into its own file with .yaml appended to - # the name of the record. This would result in files like `.yaml` for - # the apex and `*.yaml` for a wildcard. If your OS doesn't allow such - # filenames or you would prefer to avoid them you can enable - # split_catchall to instead write those records into a file named - # `$[zone.name].yaml` + # the name of the record. The two exceptions are for the root and + # wildcard nodes. These records are written into a file named + # `$[zone.name].yaml`. If you would prefer this catchall file not be + # used `split_catchall` can be set to False to instead write those + # records out to `.yaml` and `*.yaml` respectively. Note that some + # operating systems may not allow files with those names. # (optional, default True) split_catchall: true From 8177ee6926648bb8dd69788374167b3bf97b4020 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Tue, 29 Aug 2023 12:59:38 -0700 Subject: [PATCH 15/16] Revert "yaml provider is either split or zonefile, not both" This reverts commit 6f39fcc5f7f29d83c8cb6e59904829c89454e032. --- octodns/provider/yaml.py | 24 +++++++++++---- tests/test_octodns_provider_yaml.py | 46 +++++++++++++++++++++++++++-- 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 2d4ee81..d340bc6 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -58,6 +58,10 @@ class YamlProvider(BaseProvider): # (optional, default True) split_catchall: true + # Disable loading of the zone .yaml files. + # (optional, default False) + disable_zonefile: false + Split Details ------------- @@ -168,13 +172,14 @@ class YamlProvider(BaseProvider): supports_root_ns=True, split_extension=False, split_catchall=True, + disable_zonefile=False, *args, **kwargs, ): klass = self.__class__.__name__ self.log = logging.getLogger(f'{klass}[{id}]') self.log.debug( - '__init__: id=%s, directory=%s, default_ttl=%d, enforce_order=%d, populate_should_replace=%s, supports_root_ns=%s, split_extension=%s, split_catchall=%s', + '__init__: id=%s, directory=%s, default_ttl=%d, enforce_order=%d, populate_should_replace=%s, supports_root_ns=%s, split_extension=%s, split_catchall=%s, disable_zonefile=%s', id, directory, default_ttl, @@ -183,6 +188,7 @@ class YamlProvider(BaseProvider): supports_root_ns, split_extension, split_catchall, + disable_zonefile, ) super().__init__(id, *args, **kwargs) self.directory = directory @@ -192,6 +198,7 @@ class YamlProvider(BaseProvider): self.supports_root_ns = supports_root_ns self.split_extension = split_extension self.split_catchall = split_catchall + self.disable_zonefile = disable_zonefile def copy(self): kwargs = dict(self.__dict__) @@ -239,7 +246,7 @@ class YamlProvider(BaseProvider): dirname = dirname[:-trim] zones.add(dirname) - if not self.split_extension: + if not self.disable_zonefile: self.log.debug('list_zones: looking for zone files') for filename in listdir(self.directory): not_ends_with = not filename.endswith('.yaml') @@ -325,7 +332,7 @@ class YamlProvider(BaseProvider): if split_extension: sources.extend(self._split_sources(zone)) - if not self.split_extension: + if not self.disable_zonefile: sources.append(self._zone_sources(zone)) # determinstically order our sources @@ -425,13 +432,20 @@ class SplitYamlProvider(YamlProvider): # extension is configured as split_extension split_extension: . split_catchall: true + disable_zonefile: true TO BE REMOVED: 2.0 ''' def __init__(self, id, directory, *args, extension='.', **kwargs): - kwargs.update({'split_extension': extension, 'split_catchall': True}) + kwargs.update( + { + 'split_extension': extension, + 'split_catchall': True, + 'disable_zonefile': True, + } + ) super().__init__(id, directory, *args, **kwargs) self.log.warning( - '__init__: DEPRECATED use YamlProvider with split_extension and split_catchall instead, will go away in v2.0' + '__init__: DEPRECATED use YamlProvider with split_extension, split_catchall, and disable_zonefile instead, will go away in v2.0' ) diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 814eb81..a41a417 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -348,20 +348,50 @@ xn--dj-kia8a: list(provider.list_zones()), ) - # split only . + # include stuff with . AND basic provider.split_extension = '.' self.assertEqual( - ['both.tld.', 'other.split.', 'split.test.', 'sub.split.test.'], + [ + 'both.tld.', + 'other.split.', + 'other.tld.', + 'split.test.', + 'sub.split.test.', + 'sub.unit.test.', + 'unit.test.', + ], list(provider.list_zones()), ) - # only .tst + # include stuff with .tst AND basic provider.split_extension = '.tst' + self.assertEqual( + [ + 'both.tld.', + 'other-ext.split.', + 'other.tld.', + 'split-ext.test.', + 'sub.split-ext.test.', + 'sub.unit.test.', + 'unit.test.', + ], + list(provider.list_zones()), + ) + + # only .tst + provider.disable_zonefile = True self.assertEqual( ['other-ext.split.', 'split-ext.test.', 'sub.split-ext.test.'], list(provider.list_zones()), ) + # only . (and both zone) + provider.split_extension = '.' + self.assertEqual( + ['both.tld.', 'other.split.', 'split.test.', 'sub.split.test.'], + list(provider.list_zones()), + ) + def test_split_sources(self): with TemporaryDirectory() as td: directory = join(td.dirname) @@ -474,6 +504,16 @@ class TestSplitYamlProvider(TestCase): self.assertEqual(20, len(zone.records)) self.assertFalse([r for r in zone.records if r.name.startswith('only')]) + # temporarily enable zone file processing too, we should see one extra + # record that came from unit.tests. + source.disable_zonefile = False + zone_both = Zone('unit.tests.', []) + source.populate(zone_both) + self.assertEqual(21, len(zone_both.records)) + n = len([r for r in zone_both.records if r.name == 'only-zone-file']) + self.assertEqual(1, n) + source.disable_zonefile = True + source.populate(dynamic_zone) self.assertEqual(5, len(dynamic_zone.records)) self.assertFalse( From 93b397cbb2f5170a4875dc7357f31404e8700423 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Tue, 29 Aug 2023 13:51:53 -0700 Subject: [PATCH 16/16] Correct Split Details directory example --- 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 d340bc6..da6fdf1 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -76,7 +76,7 @@ class YamlProvider(BaseProvider): zones/ github.com./ - .yaml + $github.com.yaml www.yaml ...