From 3451c79c6ac012a3dda29fdca36234a5bfe0966f Mon Sep 17 00:00:00 2001 From: Cadu Ribeiro Date: Wed, 21 Apr 2021 12:41:11 -0300 Subject: [PATCH 1/3] Change DNSimple's Provider to query zones instead domains A quick summary of a problem with have with DNSimple provider: Let's suppose that I have the following config: zones: 30.114.195.in-addr.arpa.: sources: - config targets: - dnsimple Even if a customer has this Reverse zone configured in DNSimple, this fails with: 400 Bad Request for url: https://api.sandbox.dnsimple.com/v2/x/domains because it is trying to create a domain because the zone wasn't found. octodns.provider.dnsimple.DnsimpleClientNotFound: Not found This happens because the GET /domains endpoint at DNSimple does not bring Reverse Zones. To make this work nice, we should use /zones/ instead making it return properly the reverse zones. --- octodns/provider/dnsimple.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/octodns/provider/dnsimple.py b/octodns/provider/dnsimple.py index 599eacb..a4b9350 100644 --- a/octodns/provider/dnsimple.py +++ b/octodns/provider/dnsimple.py @@ -51,8 +51,8 @@ class DnsimpleClient(object): resp.raise_for_status() return resp - def domain(self, name): - path = '/domains/{}'.format(name) + def zone(self, name): + path = '/zones/{}'.format(name) return self._request('GET', path).json() def domain_create(self, name): @@ -442,7 +442,7 @@ class DnsimpleProvider(BaseProvider): domain_name = desired.name[:-1] try: - self._client.domain(domain_name) + self._client.zone(domain_name) except DnsimpleClientNotFound: self.log.debug('_apply: no matching zone, creating domain') self._client.domain_create(domain_name) From e90aeb5d34fe0b1d423daffb2cdd2b831fbf1775 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Thu, 22 Apr 2021 18:45:14 -0700 Subject: [PATCH 2/3] pools used as fallbacks should count as seen --- octodns/record/__init__.py | 12 ++++++++---- tests/config/dynamic.tests.yaml | 23 +++++++++++++++++++++++ tests/test_octodns_provider_yaml.py | 14 +++++++++----- 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/octodns/record/__init__.py b/octodns/record/__init__.py index 8ee2eaa..8c8760a 100644 --- a/octodns/record/__init__.py +++ b/octodns/record/__init__.py @@ -531,6 +531,7 @@ class _DynamicMixin(object): pools_exist = set() pools_seen = set() + pools_seen_as_fallback = set() if not isinstance(pools, dict): reasons.append('pools must be a dict') elif not pools: @@ -573,9 +574,12 @@ class _DynamicMixin(object): 'value {}'.format(_id, value_num)) fallback = pool.get('fallback', None) - if fallback is not None and fallback not in pools: - reasons.append('undefined fallback "{}" for pool "{}"' - .format(fallback, _id)) + if fallback is not None: + if fallback in pools: + pools_seen_as_fallback.add(fallback) + else: + reasons.append('undefined fallback "{}" for pool "{}"' + .format(fallback, _id)) # Check for loops fallback = pools[_id].get('fallback', None) @@ -644,7 +648,7 @@ class _DynamicMixin(object): reasons.extend(GeoCodes.validate(geo, 'rule {} ' .format(rule_num))) - unused = pools_exist - pools_seen + unused = pools_exist - pools_seen - pools_seen_as_fallback if unused: unused = '", "'.join(sorted(unused)) reasons.append('unused pools: "{}"'.format(unused)) diff --git a/tests/config/dynamic.tests.yaml b/tests/config/dynamic.tests.yaml index 4bd97a7..d25f63a 100644 --- a/tests/config/dynamic.tests.yaml +++ b/tests/config/dynamic.tests.yaml @@ -109,6 +109,29 @@ cname: - pool: iad type: CNAME value: target.unit.tests. +pool-only-in-fallback: + dynamic: + pools: + one: + fallback: two + values: + - value: 1.1.1.1 + three: + values: + - value: 3.3.3.3 + two: + values: + - value: 2.2.2.2 + rules: + - geos: + - NA-US + pool: one + - geos: + - AS-SG + pool: three + ttl: 300 + type: A + values: [4.4.4.4] real-ish-a: dynamic: pools: diff --git a/tests/test_octodns_provider_yaml.py b/tests/test_octodns_provider_yaml.py index 2ce9b4a..872fcca 100644 --- a/tests/test_octodns_provider_yaml.py +++ b/tests/test_octodns_provider_yaml.py @@ -38,7 +38,7 @@ class TestYamlProvider(TestCase): self.assertEquals(22, len(zone.records)) source.populate(dynamic_zone) - self.assertEquals(5, len(dynamic_zone.records)) + self.assertEquals(6, 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 @@ -68,11 +68,11 @@ class TestYamlProvider(TestCase): # Dynamic plan plan = target.plan(dynamic_zone) - self.assertEquals(5, len([c for c in plan.changes + self.assertEquals(6, len([c for c in plan.changes if isinstance(c, Create)])) self.assertFalse(isfile(dynamic_yaml_file)) # Apply it - self.assertEquals(5, target.apply(plan)) + self.assertEquals(6, target.apply(plan)) self.assertTrue(isfile(dynamic_yaml_file)) # There should be no changes after the round trip @@ -148,6 +148,10 @@ class TestYamlProvider(TestCase): self.assertTrue('value' in dyna) # self.assertTrue('dynamic' in dyna) + dyna = data.pop('pool-only-in-fallback') + self.assertTrue('value' in dyna) + # self.assertTrue('dynamic' in dyna) + # make sure nothing is left self.assertEquals([], list(data.keys())) @@ -397,7 +401,7 @@ class TestOverridingYamlProvider(TestCase): # Load the base, should see the 5 records base.populate(zone) got = {r.name: r for r in zone.records} - self.assertEquals(5, len(got)) + self.assertEquals(6, len(got)) # We get the "dynamic" A from the base config self.assertTrue('dynamic' in got['a'].data) # No added @@ -406,7 +410,7 @@ class TestOverridingYamlProvider(TestCase): # Load the overrides, should replace one and add 1 override.populate(zone) got = {r.name: r for r in zone.records} - self.assertEquals(6, len(got)) + self.assertEquals(7, len(got)) # 'a' was replaced with a generic record self.assertEquals({ 'ttl': 3600, From 3f3d500152baa55e9d88c27ced0e83101699472c Mon Sep 17 00:00:00 2001 From: Ricard Bejarano Date: Mon, 26 Apr 2021 19:06:29 +0200 Subject: [PATCH 3/3] fixed missing whitespace in plan() debug logging When debug logging is enabled, the plan() function logs lines like: Plan: DEBUG: __init__: Creates=13, Updates=0, Deletes=0Existing=0 Where a space between "Deletes=0" and "Existing=0" is missing. This adds it. --- octodns/provider/plan.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/octodns/provider/plan.py b/octodns/provider/plan.py index af6863a..69bd2b2 100644 --- a/octodns/provider/plan.py +++ b/octodns/provider/plan.py @@ -50,7 +50,7 @@ class Plan(object): except AttributeError: existing_n = 0 - self.log.debug('__init__: Creates=%d, Updates=%d, Deletes=%d' + self.log.debug('__init__: Creates=%d, Updates=%d, Deletes=%d ' 'Existing=%d', self.change_counts['Create'], self.change_counts['Update'],