diff --git a/CHANGELOG.md b/CHANGELOG.md index 56d6678..7a8dcf9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ * Support for Provider.list_zones to enable dynamic zone config when operating as a source +* Support for auto-ttl without proxied as records can be configured that way, + see auto-ttl in README.md for more info ## v0.0.3 - 2023-09-20 - All the commits fit to release diff --git a/README.md b/README.md index c7ce4af..a7c4307 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,23 @@ name: octodns: cloudflare: proxied: true + # auto-ttl true is implied by proxied true, but can be explicitly + # configured to be more complete + #auto-ttl: true + # with proxied=true, the TTL here will be ignored by CloudflareProvider + ttl: 120 + type: A + value: 1.2.3.4 +``` + +Note: All record types support "auto" ttl, which is effecitvely equivilent to 300s. + +```yaml +name: + octodns: + cloudflare: + auto-ttl: true + # with proxied=true, the TTL here will be ignored by CloudflareProvider ttl: 120 type: A value: 1.2.3.4 diff --git a/octodns_cloudflare/__init__.py b/octodns_cloudflare/__init__.py index cdb3299..b763eee 100644 --- a/octodns_cloudflare/__init__.py +++ b/octodns_cloudflare/__init__.py @@ -3,7 +3,6 @@ # from collections import defaultdict -from copy import deepcopy from logging import getLogger from time import sleep from urllib.parse import urlsplit @@ -422,7 +421,8 @@ def zone_records(self, zone): def _record_for(self, zone, name, _type, records, lenient): # rewrite Cloudflare proxied records - if self.cdn and records[0]['proxied']: + proxied = records[0].get('proxied', False) + if self.cdn and proxied: data = self._data_for_cdn(name, _type, records) else: # Cloudflare supports ALIAS semantics with root CNAMEs @@ -434,10 +434,16 @@ def _record_for(self, zone, name, _type, records, lenient): record = Record.new(zone, name, data, source=self, lenient=lenient) - if _type in _PROXIABLE_RECORD_TYPES: - record._octodns['cloudflare'] = { - 'proxied': records[0].get('proxied', False) - } + proxied = proxied and _type in _PROXIABLE_RECORD_TYPES + auto_ttl = records[0]['ttl'] == 1 + if proxied: + self.log.debug('_record_for: proxied=True, auto-ttl=True') + record._octodns['cloudflare'] = {'proxied': True, 'auto-ttl': True} + elif auto_ttl: + # auto-ttl can still be set on any record type, signaled by a ttl=1, + # even if proxied is false. + self.log.debug('_record_for: auto-ttl=True') + record._octodns['cloudflare'] = {'auto-ttl': True} return record @@ -518,20 +524,43 @@ def _include_change(self, change): raise SupportsException(msg) if isinstance(change, Update): - new = change.new.data - - # Cloudflare manages TTL of proxied records, so we should exclude - # TTL from the comparison (to prevent false-positives). - if self._record_is_proxied(change.existing): - existing = deepcopy(change.existing.data) - existing.update({'ttl': new['ttl']}) - elif change.new._type == 'URLFWD': - existing = deepcopy(change.existing.data) - existing.update({'ttl': new['ttl']}) - else: - existing = change.existing.data - + new = change.new + new_is_proxied = self._record_is_proxied(new) + new_is_just_auto_ttl = self._record_is_just_auto_ttl(new) + new_is_urlfwd = new._type == 'URLFWD' + new = new.data + + existing = change.existing + existing_is_proxied = self._record_is_proxied(existing) + existing_is_just_auto_ttl = self._record_is_just_auto_ttl(existing) + existing_is_urlfwd = existing._type == 'URLFWD' + existing = existing.data + + if ( + (new_is_proxied != existing_is_proxied) + or (new_is_just_auto_ttl != existing_is_just_auto_ttl) + or (new_is_urlfwd != existing_is_urlfwd) + ): + # changes in special flags, definitely need this change + return True + + # at this point we know that all the special flags match in new and + # existing so we can focus on the actual record details, so we can + # ignore octodns.cloudflare + new.get('octodns', {}).pop('cloudflare', None) + existing.get('octodns', {}).pop('cloudflare', None) + + # TTLs are ignored for these, best way to do that is to just copy + # it over so they'll match + if new_is_proxied or new_is_just_auto_ttl or new_is_urlfwd: + new['ttl'] = existing['ttl'] + + # Cloudflare has a minimum TTL, we need to clamp the TTL values so + # that we ignore a desired state (new) where we can't support the + # TTL new['ttl'] = max(self.MIN_TTL, new['ttl']) + existing['ttl'] = max(self.MIN_TTL, existing['ttl']) + if new == existing: return False @@ -688,10 +717,24 @@ def _record_is_proxied(self, record): 'proxied', False ) + def _record_is_just_auto_ttl(self, record): + 'This tests if it is strictly auto-ttl and not proxied' + return ( + not self._record_is_proxied(record) + and not self.cdn + and record._octodns.get('cloudflare', {}).get('auto-ttl', False) + ) + def _gen_data(self, record): name = record.fqdn[:-1] _type = record._type - ttl = max(self.MIN_TTL, record.ttl) + proxied = self._record_is_proxied(record) + if proxied or self._record_is_just_auto_ttl(record): + # proxied implies auto-ttl, and auto-ttl can be enabled on its own, + # when either is the case we tell Cloudflare with ttl=1 + ttl = 1 + else: + ttl = max(self.MIN_TTL, record.ttl) # Cloudflare supports ALIAS semantics with a root CNAME if _type == 'ALIAS': @@ -1008,9 +1051,13 @@ def _extra_changes(self, existing, desired, changes): elif desired_record in changed_records: # Already being updated continue - if self._record_is_proxied( - existing_record - ) != self._record_is_proxied(desired_record): + if ( + self._record_is_proxied(existing_record) + != self._record_is_proxied(desired_record) + ) or ( + self._record_is_just_auto_ttl(existing_record) + != self._record_is_just_auto_ttl(desired_record) + ): extra_changes.append(Update(existing_record, desired_record)) return extra_changes diff --git a/tests/test_octodns_provider_cloudflare.py b/tests/test_octodns_provider_cloudflare.py index 8f69b02..921a09b 100644 --- a/tests/test_octodns_provider_cloudflare.py +++ b/tests/test_octodns_provider_cloudflare.py @@ -29,6 +29,15 @@ def set_record_proxied_flag(record, proxied): return record +def set_record_auto_ttl_flag(record, auto_ttl): + try: + record._octodns['cloudflare']['auto-ttl'] = auto_ttl + except KeyError: + record._octodns['cloudflare'] = {'auto-ttl': auto_ttl} + + return record + + class TestCloudflareProvider(TestCase): expected = Zone('unit.tests.', []) source = YamlProvider('test', join(dirname(__file__), 'config')) @@ -938,9 +947,6 @@ def test_pagerules(self): provider.pagerules = True # plan again, this time we expect both calls and a record plan = provider.plan(zone) - from pprint import pprint - - pprint({'plan': plan, 'changes': plan.changes}) self.assertEqual(1, len(plan.changes)) change = list(plan.changes)[0] self.assertEqual('URLFWD', change.record._type) @@ -1487,7 +1493,8 @@ def test_unproxiabletype_recordfor_returnsrecordwithnocloudflare(self): record = provider._record_for(zone, name, _type, zone_records, False) - self.assertFalse('cloudflare' in record._octodns) + self.assertFalse(record._octodns.get('auto-ttl', False)) + self.assertFalse(record._octodns.get('proxied', False)) def test_proxiabletype_recordfor_retrecordwithcloudflareunproxied(self): provider = CloudflareProvider('test', 'email', 'token') @@ -1516,7 +1523,9 @@ def test_proxiabletype_recordfor_retrecordwithcloudflareunproxied(self): record = provider._record_for(zone, name, _type, zone_records, False) - self.assertFalse(record._octodns['cloudflare']['proxied']) + self.assertFalse( + record._octodns.get('cloudflare', {}).get('proxied', False) + ) def test_proxiabletype_recordfor_returnsrecordwithcloudflareproxied(self): provider = CloudflareProvider('test', 'email', 'token') @@ -1530,7 +1539,7 @@ def test_proxiabletype_recordfor_returnsrecordwithcloudflareproxied(self): "content": "::1", "proxiable": True, "proxied": True, - "ttl": 300, + "ttl": 1, "locked": False, "zone_id": "ff12ab34cd5611334422ab3322997650", "zone_name": "unit.tests", @@ -1545,8 +1554,37 @@ def test_proxiabletype_recordfor_returnsrecordwithcloudflareproxied(self): record = provider._record_for(zone, name, _type, zone_records, False) + self.assertTrue(record._octodns['cloudflare']['auto-ttl']) self.assertTrue(record._octodns['cloudflare']['proxied']) + def test_record_for_auto_ttl_no_proxied(self): + provider = CloudflareProvider('test', 'email', 'token') + name = "proxied.unit.tests" + _type = "A" + zone_records = [ + { + "id": "fc12ab34cd5611334422ab3322997642", + "type": _type, + "name": name, + "content": "4.3.2.1", + "proxiable": True, + "proxied": False, + "ttl": 1, + "locked": False, + "zone_id": "ff12ab34cd5611334422ab3322997650", + "zone_name": "unit.tests", + "modified_on": "2017-03-11T18:01:43.420689Z", + "created_on": "2017-03-11T18:01:43.420689Z", + "meta": {"auto_added": False}, + } + ] + + zone = Zone('unit.tests.', []) + record = provider._record_for(zone, name, _type, zone_records, False) + + self.assertTrue(record._octodns['cloudflare']['auto-ttl']) + self.assertFalse(record._octodns['cloudflare'].get('proxied', False)) + def test_proxiedrecordandnewttl_includechange_returnsfalse(self): provider = CloudflareProvider('test', 'email', 'token') zone = Zone('unit.tests.', []) @@ -1558,10 +1596,38 @@ def test_proxiedrecordandnewttl_includechange_returnsfalse(self): ), True, ) - new = Record.new( - zone, - 'a', - {'ttl': 300, 'type': 'A', 'values': ['1.1.1.1', '2.2.2.2']}, + new = set_record_proxied_flag( + Record.new( + zone, + 'a', + {'ttl': 300, 'type': 'A', 'values': ['1.1.1.1', '2.2.2.2']}, + ), + True, + ) + change = Update(existing, new) + + include_change = provider._include_change(change) + + self.assertFalse(include_change) + + def test_auto_ttl_ignores_ttl_change(self): + provider = CloudflareProvider('test', 'email', 'token') + zone = Zone('unit.tests.', []) + existing = set_record_auto_ttl_flag( + Record.new( + zone, + 'a', + {'ttl': 1, 'type': 'A', 'values': ['1.1.1.1', '2.2.2.2']}, + ), + True, + ) + new = set_record_auto_ttl_flag( + Record.new( + zone, + 'a', + {'ttl': 300, 'type': 'A', 'values': ['1.1.1.1', '2.2.2.2']}, + ), + True, ) change = Update(existing, new) @@ -1569,6 +1635,105 @@ def test_proxiedrecordandnewttl_includechange_returnsfalse(self): self.assertFalse(include_change) + # if flag is false, would return the change + existing = set_record_auto_ttl_flag(existing, False) + include_change = provider._include_change(change) + self.assertTrue(include_change) + + def test_include_change_special_flags(self): + provider = CloudflareProvider('test', 'email', 'token') + + zone = Zone('unit.tests.', []) + a1_plain = Record.new( + zone, 'www', {'ttl': 300, 'type': 'A', 'value': '1.2.3.4'} + ) + a1_proxied = set_record_proxied_flag( + Record.new( + zone, 'www', {'ttl': 300, 'type': 'A', 'value': '1.2.3.4'} + ), + True, + ) + a1_auto_ttl = set_record_auto_ttl_flag( + Record.new( + zone, 'www', {'ttl': 300, 'type': 'A', 'value': '1.2.3.4'} + ), + True, + ) + + # plain <-> proxied + self.assertTrue(provider._include_change(Update(a1_plain, a1_proxied))) + self.assertTrue(provider._include_change(Update(a1_proxied, a1_plain))) + # plain <-> auto-ttl + self.assertTrue(provider._include_change(Update(a1_plain, a1_auto_ttl))) + self.assertTrue(provider._include_change(Update(a1_auto_ttl, a1_plain))) + # proxied <-> auto-ttl + self.assertTrue( + provider._include_change(Update(a1_proxied, a1_auto_ttl)) + ) + self.assertTrue( + provider._include_change(Update(a1_auto_ttl, a1_proxied)) + ) + + # no special flag changes + self.assertFalse(provider._include_change(Update(a1_plain, a1_plain))) + self.assertFalse( + provider._include_change(Update(a1_proxied, a1_proxied)) + ) + self.assertFalse( + provider._include_change(Update(a1_auto_ttl, a1_auto_ttl)) + ) + + def test_include_change_min_ttl(self): + provider = CloudflareProvider('test', 'email', 'token') + + zone = Zone('unit.tests.', []) + below1 = Record.new( + zone, 'www', {'ttl': 42, 'type': 'A', 'value': '1.2.3.4'} + ) + below2 = Record.new( + zone, 'www', {'ttl': 119, 'type': 'A', 'value': '1.2.3.4'} + ) + edge = Record.new( + zone, 'www', {'ttl': 120, 'type': 'A', 'value': '1.2.3.4'} + ) + above1 = Record.new( + zone, 'www', {'ttl': 121, 'type': 'A', 'value': '1.2.3.4'} + ) + above2 = Record.new( + zone, 'www', {'ttl': 500, 'type': 'A', 'value': '1.2.3.4'} + ) + + # both below + self.assertFalse(provider._include_change(Update(below1, below1))) + self.assertFalse(provider._include_change(Update(below1, below2))) + self.assertFalse(provider._include_change(Update(below2, below1))) + self.assertFalse(provider._include_change(Update(below2, below2))) + + # one below, other at + self.assertFalse(provider._include_change(Update(below1, edge))) + self.assertFalse(provider._include_change(Update(below2, edge))) + self.assertFalse(provider._include_change(Update(edge, below1))) + self.assertFalse(provider._include_change(Update(edge, below2))) + + # both at + self.assertFalse(provider._include_change(Update(edge, edge))) + + # one at, other above + self.assertTrue(provider._include_change(Update(edge, above1))) + self.assertTrue(provider._include_change(Update(edge, above2))) + self.assertTrue(provider._include_change(Update(above1, edge))) + self.assertTrue(provider._include_change(Update(above2, edge))) + + # both above + self.assertTrue(provider._include_change(Update(above1, above2))) + self.assertTrue(provider._include_change(Update(above2, above1))) + self.assertFalse(provider._include_change(Update(above1, above1))) + self.assertFalse(provider._include_change(Update(above2, above2))) + + # one below, one above + self.assertTrue(provider._include_change(Update(below2, above1))) + self.assertTrue(provider._include_change(Update(above1, below2))) + def test_unproxiabletype_gendata_returnsnoproxied(self): provider = CloudflareProvider('test', 'email', 'token') zone = Zone('unit.tests.', []) @@ -1772,7 +1937,9 @@ def test_proxify_extrachanges_returnsupdatelist(self): self.assertEqual(1, len(extra_changes)) self.assertFalse( - extra_changes[0].existing._octodns['cloudflare']['proxied'] + extra_changes[0] + .existing._octodns.get('cloudflare', {}) + .get('proxied', False) ) self.assertTrue(extra_changes[0].new._octodns['cloudflare']['proxied']) @@ -1828,7 +1995,11 @@ def test_unproxify_extrachanges_returnsupdatelist(self): self.assertTrue( extra_changes[0].existing._octodns['cloudflare']['proxied'] ) - self.assertFalse(extra_changes[0].new._octodns['cloudflare']['proxied']) + self.assertFalse( + extra_changes[0] + .new._octodns.get('cloudflare', {}) + .get('proxied', False) + ) def test_emailless_auth(self): provider = CloudflareProvider( @@ -1970,25 +2141,31 @@ def test_no_spf_create(self): provider = CloudflareProvider('test', 'email', 'token', retry_period=0) zone = Zone('unit.tests.', []) - a = Record.new(zone, 'a', {'type': 'A', 'ttl': 42, 'value': '1.2.3.4'}) - spf = Record.new( - zone, 'spf', {'type': 'SPF', 'ttl': 43, 'value': 'blahblah'} + a1 = Record.new( + zone, 'a', {'type': 'A', 'ttl': 420, 'value': '1.2.3.4'} + ) + a2 = a1.copy() + a2.ttl += 1 + spf1 = Record.new( + zone, 'spf', {'type': 'SPF', 'ttl': 430, 'value': 'blahblah'} ) + spf2 = spf1.copy() + spf2.ttl += 1 # A is always included - self.assertTrue(provider._include_change(Create(a))) - self.assertTrue(provider._include_change(Update(a, a))) - self.assertTrue(provider._include_change(Delete(a))) + self.assertTrue(provider._include_change(Create(a1))) + self.assertTrue(provider._include_change(Update(a1, a2))) + self.assertTrue(provider._include_change(Delete(a1))) # SPF can't be created, updates and deletes are OK with self.assertRaises(SupportsException) as ctx: - provider._include_change(Create(spf)) + provider._include_change(Create(spf1)) self.assertEqual( 'test: creating new SPF records not supported, use TXT instead', str(ctx.exception), ) - self.assertTrue(provider._include_change(Update(spf, spf))) - self.assertTrue(provider._include_change(Delete(spf))) + self.assertTrue(provider._include_change(Update(spf1, spf2))) + self.assertTrue(provider._include_change(Delete(spf1))) def test_sshfp(self): self.maxDiff = None