octodns / octodns-cloudflare

Cloudflare DNS provider for octoDNS
MIT License
24 stars 17 forks source link

Lower `MIN_TTL` to 30 or 60 seconds (or configurable) per Cloudflare documentation #76

Closed TheDJVG closed 7 months ago

TheDJVG commented 7 months ago

While working on migrating some DNS zones I noticed that DNS records with a 60 second TTL were created with a 120 second TTL.

On the next run it would notice a difference in the records (120 TTL set, 60 TTL expected) but when an update runs no changes are made because the minimum TTL is hardcoded set to 120:

https://github.com/octodns/octodns-cloudflare/blob/baf0730c32ca859338f4759375eb80878fd5fce6/octodns_cloudflare/__init__.py#L67 https://github.com/octodns/octodns-cloudflare/blob/baf0730c32ca859338f4759375eb80878fd5fce6/octodns_cloudflare/__init__.py#L560-L564 https://github.com/octodns/octodns-cloudflare/blob/baf0730c32ca859338f4759375eb80878fd5fce6/octodns_cloudflare/__init__.py#L739

Based on the documentation the minimum TTL for non-enterprise zones is 60 seconds and 30 for enterprise zones: https://developers.cloudflare.com/dns/manage-dns-records/reference/ttl/#unproxied-records

Is there a reason why the minimum here is set to 120 seconds and could we lower it or is it acceptable make it configurable?

ross commented 7 months ago

Based on the documentation the minimum TTL for non-enterprise zones is 60 seconds and 30 for enterprise zones: https://developers.cloudflare.com/dns/manage-dns-records/reference/ttl/#unproxied-records

Interesting. I believe that's changed since the provider was originally written. At the time I was working with a (trial) enterprise account so I would have likely used the minimum value that applied to it then.

Anyway, 👍 to making it configurable. cls.MIN_TTL (constant) should become self.min_ttl the object property which would get set in the provider init like everything else.

The only question is what the default should be. 120 might make sense to avoid unexpected changes in behavior for anyone using the provider currently. If it changes I'm not sure whether the personal or enterprise would make more sense. Guess that makes me lean towards a default of 120 and documenting/linking to the official bit you link and saying that 30/60 makes should be used there based on account type.