mholt / caddy-dynamicdns

Caddy app that keeps your DNS records (A/AAAA) pointed at itself.
Apache License 2.0
236 stars 24 forks source link

CheckInterval is the same as TTL #25

Closed cobyge closed 2 years ago

cobyge commented 2 years ago

In the README, it is implied that check_interval is used for setting how often to check the external IP.
There is an unexpected side effect though, as the same value is used as the TTL of the record.
I believe this is unexpected (and unwanted) behavior, and also prevents putting low intervals (Anything lower than 5 minutes is rejected by Cloudflare).

I think there should be a different value for setting the TTL, as opposed to the checking interval.

mholt commented 2 years ago

I guess we could separate them. I haven't heard of a use case for checks more frequent than 5m before. Why do you need to check that often?

Because it makes sense for CheckInterval to roughly be the TTL, since we're not doing lookups more often than CheckInterval. :man_shrugging:

But if there's a good use case for this I guess we can separate them out.

cobyge commented 2 years ago

My IP address changes often enough that it sometimes becomes a problem, and I don't see any reason why not 🤷‍♂️

Because it makes sense for CheckInterval to roughly be the TTL, since we're not doing lookups more often than CheckInterval. 🤷‍♂️

This is only true if you have a single client (and even then, it's all a matter of timing). With multiple clients, it's less likely that they have the DNS cached, even if a single client still has the old record cached.

mholt commented 2 years ago

I'll implement a new TTL config parameter. What should the default be? (Empty/Zero?)

cobyge commented 2 years ago

From the (admittedly short) time I just spent playing around with libdns, it looks like leaving the field empty doesn't cause any problems (the TTL is set to the default value) I only tested this on libdns/cloudflare.
Another option is to have the default be equal to the CheckInterval.

mholt commented 2 years ago

Should be configurable now -- at least, in the JSON config!