Fix CloudflareProvider's root NS handling now that I have access

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 | |

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:

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.', [])