From a01750d2b725c75d28d05807c2809ef409a08b31 Mon Sep 17 00:00:00 2001 From: Ross McFarland Date: Sun, 4 Jun 2017 13:52:16 -0700 Subject: [PATCH] Fix CloudflareProvider's root NS handling now that I have access --- README.md | 2 +- octodns/provider/cloudflare.py | 15 +++++++++++++-- tests/test_octodns_provider_cloudflare.py | 3 ++- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 07d3b47..5ed9d4f 100644 --- a/README.md +++ b/README.md @@ -149,7 +149,7 @@ The above command pulled the existing data out of Route53 and placed the results | Provider | Record Support | GeoDNS Support | Notes | |--|--|--|--| -| [CloudflareProvider](/octodns/provider/cloudflare.py) | A, AAAA, CNAME, MX, NS, SPF, TXT | No | Unknown state, don't have access to test | +| [CloudflareProvider](/octodns/provider/cloudflare.py) | A, AAAA, CNAME, MX, NS, SPF, TXT | No | | | [DnsimpleProvider](/octodns/provider/dnsimple.py) | All | No | No root NS record support | | [DynProvider](/octodns/provider/dyn.py) | All | Yes | No root NS record support | | [Ns1Provider](/octodns/provider/ns1.py) | All | No | | diff --git a/octodns/provider/cloudflare.py b/octodns/provider/cloudflare.py index d7af77e..82d5c5c 100644 --- a/octodns/provider/cloudflare.py +++ b/octodns/provider/cloudflare.py @@ -68,6 +68,9 @@ class CloudflareProvider(BaseProvider): self.log.debug('_request: status=%d', resp.status_code) if resp.status_code == 403: raise CloudflareAuthenticationError(resp.json()) + elif resp.status_code > 299: + self.log.warn("_request: status_code=%d, msg=%s", resp.status_code, + resp.content) resp.raise_for_status() return resp.json() @@ -181,13 +184,21 @@ class CloudflareProvider(BaseProvider): len(zone.records) - before) def _include_change(self, change): - if isinstance(change, Update): + ''' + Cloudflare doesn't allow management of root NS records, so no root NS + support. They also have a minimum TTL that is higher than records + elsewhere are commonly configured with so we need to prevent that + clamping from being a planned change. + ''' + if change.record._type == 'NS' and change.record.name == '': + return False + elif isinstance(change, Update): existing = change.existing.data new = change.new.data new['ttl'] = max(120, new['ttl']) if new == existing: return False - return True + return super(CloudflareProvider, self)._include_change(change) def _contents_for_multiple(self, record): for value in record.values: diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 3f652a1..751a235 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -121,7 +121,8 @@ class TestCloudflareProvider(TestCase): self.assertEquals(9, len(zone.records)) changes = self.expected.changes(zone, provider) - self.assertEquals(0, len(changes)) + # root NS change wouldn't be included in plans + self.assertEquals(1, len(changes)) # re-populating the same zone/records comes out of cache, no calls again = Zone('unit.tests.', [])