From 0181158953f373345b581165a35040a5ebdd2d91 Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Mon, 18 Sep 2023 15:31:16 -0700 Subject: [PATCH 1/4] Include sources only if they exist --- octodns/provider/yaml.py | 17 ++++++++++++++--- tests/config/hybrid/one.test.yaml | 4 ++++ tests/config/hybrid/two.test./$two.test.yaml | 4 ++++ .../hybrid/two.test./split-zone-file.yaml | 4 ++++ tests/test_octodns_provider_yaml.py | 18 ++++++++++++++++++ 5 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 tests/config/hybrid/one.test.yaml create mode 100644 tests/config/hybrid/two.test./$two.test.yaml create mode 100644 tests/config/hybrid/two.test./split-zone-file.yaml diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 3ca7842..697a29b 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -278,8 +278,10 @@ class YamlProvider(BaseProvider): f'Both UTF-8 "{utf8}" and IDNA "{idna}" exist for {zone.decoded_name}' ) directory = utf8 - else: + elif isdir(idna): directory = idna + else: + return [] for filename in listdir(directory): if filename.endswith('.yaml'): @@ -294,8 +296,10 @@ class YamlProvider(BaseProvider): f'Both UTF-8 "{utf8}" and IDNA "{idna}" exist for {zone.decoded_name}' ) return utf8 + elif isfile(idna): + return idna - return idna + return None def _populate_from_file(self, filename, zone, lenient): with open(filename, 'r') as fh: @@ -341,11 +345,18 @@ class YamlProvider(BaseProvider): sources.extend(self._split_sources(zone)) if not self.disable_zonefile: - sources.append(self._zone_sources(zone)) + source = self._zone_sources(zone) + if source: + sources.append(self._zone_sources(zone)) if self.shared_filename: sources.append(join(self.directory, self.shared_filename)) + if not sources: + self.log.info( + 'populate: no YAMLs found for %s', zone.decoded_name + ) + # determinstically order our sources sources.sort() diff --git a/tests/config/hybrid/one.test.yaml b/tests/config/hybrid/one.test.yaml new file mode 100644 index 0000000..d2ac6ba --- /dev/null +++ b/tests/config/hybrid/one.test.yaml @@ -0,0 +1,4 @@ +--- +flat-zone-file: + type: TXT + value: non-split flat zone file diff --git a/tests/config/hybrid/two.test./$two.test.yaml b/tests/config/hybrid/two.test./$two.test.yaml new file mode 100644 index 0000000..8019a9d --- /dev/null +++ b/tests/config/hybrid/two.test./$two.test.yaml @@ -0,0 +1,4 @@ +--- +'': + type: TXT + value: root TXT diff --git a/tests/config/hybrid/two.test./split-zone-file.yaml b/tests/config/hybrid/two.test./split-zone-file.yaml new file mode 100644 index 0000000..ea051ba --- /dev/null +++ b/tests/config/hybrid/two.test./split-zone-file.yaml @@ -0,0 +1,4 @@ +--- +split-zone-file: + type: TXT + value: split zone file diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index c289e6a..901819a 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -760,6 +760,24 @@ class TestSplitYamlProvider(TestCase): sorted(provider.list_zones()), ) + def test_hybrid_directory(self): + source = YamlProvider( + 'test', + join(dirname(__file__), 'config/hybrid'), + split_extension='.', + strict_supports=False, + ) + + # flat zone file only + zone = Zone('one.test.', []) + source.populate(zone) + self.assertEqual(1, len(zone.records)) + + # split zone only + zone = Zone('two.test.', []) + source.populate(zone) + self.assertEqual(2, len(zone.records)) + class TestOverridingYamlProvider(TestCase): def test_provider(self): From 6042cb0ec545c3ec49e30f1792c3573b7a676b9e Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Mon, 18 Sep 2023 20:34:21 -0700 Subject: [PATCH 2/4] reuse compiled source --- 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 697a29b..50a4c3a 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -347,7 +347,7 @@ class YamlProvider(BaseProvider): if not self.disable_zonefile: source = self._zone_sources(zone) if source: - sources.append(self._zone_sources(zone)) + sources.append(source) if self.shared_filename: sources.append(join(self.directory, self.shared_filename)) From 11118efe93aa87d6ccd166b48c44d6a07aeebbaa Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Mon, 18 Sep 2023 21:41:44 -0700 Subject: [PATCH 3/4] Raise exception when no yamls are found for a zone --- octodns/provider/yaml.py | 4 +--- tests/test_octodns_provider_yaml.py | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 50a4c3a..23e65ef 100644 --- a/octodns/provider/yaml.py +++ b/octodns/provider/yaml.py @@ -353,9 +353,7 @@ class YamlProvider(BaseProvider): sources.append(join(self.directory, self.shared_filename)) if not sources: - self.log.info( - 'populate: no YAMLs found for %s', zone.decoded_name - ) + raise ProviderException(f'no YAMLs found for {zone.decoded_name}') # determinstically order our sources sources.sort() diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 901819a..b98c067 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -663,8 +663,8 @@ class TestSplitYamlProvider(TestCase): zone = Zone('empty.', []) # without it we see everything - source.populate(zone) - self.assertEqual(0, len(zone.records)) + with self.assertRaises(ProviderException): + source.populate(zone) def test_unsorted(self): source = SplitYamlProvider( From 4e26de7a89504be6725ecca473f8837cbfe9a0ba Mon Sep 17 00:00:00 2001 From: Viranch Mehta Date: Tue, 19 Sep 2023 11:01:38 -0700 Subject: [PATCH 4/4] CHANGELOG entry --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 808a153..2f82be4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## v1.1.2 - 2023-09-20 - Bunch more bug fixes + +* Fix crash bug when using the YamlProvider with a directory that contains a + mix of split and non-split zone yamls. See https://github.com/octodns/octodns/issues/1066 + ## v1.1.1 - 2023-09-16 - Doh! Fix that one little thing * Address a bug in the handling of loading auto-arpa manager configuration.