Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Full support for auto-ttl independant of proxied #65

Merged
merged 9 commits into from
Nov 22, 2023
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
17 changes: 17 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
ross marked this conversation as resolved.
Show resolved Hide resolved

```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
Expand Down
43 changes: 36 additions & 7 deletions octodns_cloudflare/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -421,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
Expand All @@ -433,10 +434,16 @@ def _record_for(self, zone, name, _type, records, lenient):

record = Record.new(zone, name, data, source=self, lenient=lenient)

auto_ttl = records[0]['ttl'] == 1
if _type in _PROXIABLE_RECORD_TYPES:
record._octodns['cloudflare'] = {
'proxied': records[0].get('proxied', False)
'proxied': proxied,
'auto-ttl': auto_ttl,
}
else:
# auto-ttl can still be set, signaled by a ttl=1, even if
# proxied is false or if the record isn't even a proxyable type
record._octodns['cloudflare'] = {'auto-ttl': auto_ttl}

return record

Expand Down Expand Up @@ -520,10 +527,14 @@ def _include_change(self, change):
new = change.new.data

# Cloudflare manages TTL of proxied records, so we should exclude
# TTL from the comparison (to prevent false-positives).
# TTL from the comparison (to prevent false-positives), this is even
# true of non-proxied records when auto-ttl is enabled
if self._record_is_proxied(change.existing):
existing = deepcopy(change.existing.data)
existing.update({'ttl': new['ttl']})
elif self._record_is_just_auto_ttl(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']})
Expand Down Expand Up @@ -687,10 +698,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':
Expand Down Expand Up @@ -1007,9 +1032,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
42 changes: 40 additions & 2 deletions tests/test_octodns_provider_cloudflare.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'))
Expand Down Expand Up @@ -1487,7 +1496,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')
Expand Down Expand Up @@ -1530,7 +1540,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",
Expand All @@ -1545,6 +1555,7 @@ 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_proxiedrecordandnewttl_includechange_returnsfalse(self):
Expand All @@ -1569,6 +1580,33 @@ def test_proxiedrecordandnewttl_includechange_returnsfalse(self):

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 = Record.new(
zone,
'a',
{'ttl': 300, 'type': 'A', 'values': ['1.1.1.1', '2.2.2.2']},
)
change = Update(existing, new)

include_change = provider._include_change(change)

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_unproxiabletype_gendata_returnsnoproxied(self):
provider = CloudflareProvider('test', 'email', 'token')
zone = Zone('unit.tests.', [])
Expand Down