From 0544e9ed7d350d257247a93bb4889b1904809f33 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 17 Feb 2022 13:00:28 -0800 Subject: [PATCH] More thorough testing of root ns cases and associated improvements --- octodns/provider/base.py | 23 ++++---- tests/test_octodns_provider_base.py | 83 +++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 14 deletions(-) diff --git a/octodns/provider/base.py b/octodns/provider/base.py index 0af75ef..c5c81e4 100644 --- a/octodns/provider/base.py +++ b/octodns/provider/base.py @@ -98,12 +98,13 @@ class BaseProvider(BaseSource): record = desired.root_ns if self.SUPPORTS_ROOT_NS: if not record: - self.log.warning('%s: root NS record supported by provider, ' - 'but no record is configured for %s', self.id, - desired.name) + msg = 'root NS record supported, but no record is ' \ + f'configured for {desired.name}' + fallback = 'ignoring it' + self.supports_warn_or_except(msg, fallback) else: if record: - # ignore, we can't manage root NS records + # we can't manage root NS records, get rid of it msg = \ f'root NS record not supported for {record.fqdn}' fallback = 'ignoring it' @@ -133,16 +134,10 @@ class BaseProvider(BaseSource): ''' existing_root_ns = existing.root_ns - if existing_root_ns and (not desired.root_ns or not - self.SUPPORTS_ROOT_NS): - # we have an existing root NS record and either the provider - # doesn't support managing them or our desired state doesn't - # include one, either way we'll exclude the existing one from - # consideration - msg = \ - f'root NS record not supported for {existing_root_ns.fqdn}' - fallback = 'ignoring it' - self.supports_warn_or_except(msg, fallback) + if existing_root_ns and (not self.SUPPORTS_ROOT_NS or + not desired.root_ns): + self.log.info('root NS record in existing, but not supported or ' + 'not configured; ignoring it') existing.remove_record(existing_root_ns) return existing diff --git a/tests/test_octodns_provider_base.py b/tests/test_octodns_provider_base.py index 2498dca..027a07c 100644 --- a/tests/test_octodns_provider_base.py +++ b/tests/test_octodns_provider_base.py @@ -707,6 +707,7 @@ class TestBaseProviderSupportsRootNs(TestCase): self.assertFalse(plan) # plan again with strict_supports enabled, we should get an exception + # b/c we have something configured that can't be managed provider.strict_supports = True with self.assertRaises(SupportsException) as ctx: provider.plan(self.has_root) @@ -725,6 +726,15 @@ class TestBaseProviderSupportsRootNs(TestCase): # anyway, they will have been removed from the desired and existing. self.assertFalse(plan) + # plan again with strict_supports enabled, we should get an exception + # b/c we have something configured that can't be managed (doesn't + # matter that it's a mis-match) + provider.strict_supports = True + with self.assertRaises(SupportsException) as ctx: + provider.plan(self.has_root) + self.assertEqual('test: root NS record not supported for unit.tests.', + str(ctx.exception)) + def test_supports_root_ns_false_missing(self): # provider has an existing record provider = self.Provider(self.has_root) @@ -737,6 +747,12 @@ class TestBaseProviderSupportsRootNs(TestCase): # anyway, they will have been removed from the desired and existing. self.assertFalse(plan) + # plan again with strict supports enabled, no change since desired + # isn't asking to manage root + provider.strict_supports = True + plan = provider.plan(self.no_root) + self.assertFalse(plan) + def test_supports_root_ns_false_create_zone(self): # provider has no existing records (create) provider = self.Provider() @@ -749,6 +765,33 @@ class TestBaseProviderSupportsRootNs(TestCase): self.assertTrue(plan) self.assertEqual(2, len(plan.changes)) + # plan again with strict supports enabled, we'll get an exception b/c + # the target provider can't manage something in desired + provider.strict_supports = True + with self.assertRaises(SupportsException) as ctx: + provider.plan(self.has_root) + self.assertEqual('test: root NS record not supported for unit.tests.', + str(ctx.exception)) + + def test_supports_root_ns_false_create_zone_missing(self): + # provider has no existing records (create) + provider = self.Provider() + provider.SUPPORTS_ROOT_NS = False + + # case where we have a root NS in the desired + plan = provider.plan(self.no_root) + + # no support for root NS so we only create the other two records + self.assertTrue(plan) + self.assertEqual(2, len(plan.changes)) + + # plan again with strict supports enabled, same result since we're not + # asking for a root NS it's just the 2 other changes + provider.strict_supports = True + plan = provider.plan(self.no_root) + self.assertTrue(plan) + self.assertEqual(2, len(plan.changes)) + # True def test_supports_root_ns_true_matches(self): @@ -763,6 +806,11 @@ class TestBaseProviderSupportsRootNs(TestCase): # change self.assertFalse(plan) + # again with strict supports enabled, no difference + provider.strict_supports = True + plan = provider.plan(self.has_root) + self.assertFalse(plan) + def test_supports_root_ns_true_different(self): # provider has a non-matching existing record provider = self.Provider(self.different_root) @@ -778,6 +826,14 @@ class TestBaseProviderSupportsRootNs(TestCase): self.assertEqual(self.other_root_ns_record, change.existing) self.assertEqual(self.root_ns_record, change.new) + # again with strict supports enabled, no difference, we see the change + provider.strict_supports = True + plan = provider.plan(self.has_root) + self.assertTrue(plan) + change = plan.changes[0] + self.assertEqual(self.other_root_ns_record, change.existing) + self.assertEqual(self.root_ns_record, change.new) + def test_supports_root_ns_true_missing(self): # provider has a matching existing root record provider = self.Provider(self.has_root) @@ -790,6 +846,14 @@ class TestBaseProviderSupportsRootNs(TestCase): # aren't configured with one to manage self.assertFalse(plan) + # again with strict supports enabled, this time we throw an exception + # b/c it's not being managed and could be + provider.strict_supports = True + with self.assertRaises(SupportsException) as ctx: + provider.plan(self.no_root) + self.assertEqual('test: root NS record supported, but no record is ' + 'configured for unit.tests.', str(ctx.exception)) + def test_supports_root_ns_true_create_zone(self): # provider has no existing records (create) provider = self.Provider() @@ -807,6 +871,17 @@ class TestBaseProviderSupportsRootNs(TestCase): self.assertFalse(change.existing) self.assertEqual(self.root_ns_record, change.new) + # again with strict supports enabled, no difference, we see all 3 + # changes + provider.strict_supports = True + plan = provider.plan(self.has_root) + self.assertTrue(plan) + self.assertEqual(3, len(plan.changes)) + change = [c for c in plan.changes + if c.new.name == '' and c.new._type == 'NS'][0] + self.assertFalse(change.existing) + self.assertEqual(self.root_ns_record, change.new) + def test_supports_root_ns_true_create_zone_missing(self): # provider has no existing records (create) provider = self.Provider() @@ -816,3 +891,11 @@ class TestBaseProviderSupportsRootNs(TestCase): # manage the other records plan = provider.plan(self.no_root) self.assertEqual(2, len(plan.changes)) + + # again with strict supports enabled, this time we throw an exception + # b/c it's not being managed and could be + provider.strict_supports = True + with self.assertRaises(SupportsException) as ctx: + provider.plan(self.no_root) + self.assertEqual('test: root NS record supported, but no record is ' + 'configured for unit.tests.', str(ctx.exception))