Open str4d opened 1 year ago
AFAICT the code in pkg/js/helpers.js handles things correctly,
This is probably actually just by accident. I think the core issue here is that 0 in the TTL field is being treated as "falsey" in helpers.js
(while not present on the pathways I'm using, there are conditionals of the form if (r.TTL)
), and the TTL field is fundamentally being treated as an optional non-zero unsigned integer, rather than an optional unsigned integer. The Go code above is just enforcing that at parse time.
Fixing this would require changing both the JavaScript and Go types to separately represent None
and Some(0)
. On the JavaScript side that could probably be slipped in via e.g. using -1
to mean None
, while on the Go side a proper Optional
type would help.
These are all the spots in the Go code that map 0
to DefaultTTL
:
https://github.com/StackExchange/dnscontrol/blob/5b3bb312ea99853d1741d838d5a789cbc5eec1fb/models/record.go#L377-L379
https://github.com/StackExchange/dnscontrol/blob/5b3bb312ea99853d1741d838d5a789cbc5eec1fb/pkg/normalize/validate.go#L352-L354
https://github.com/StackExchange/dnscontrol/blob/5b3bb312ea99853d1741d838d5a789cbc5eec1fb/pkg/prettyzone/prettyzone.go#L88-L90
https://github.com/StackExchange/dnscontrol/blob/5b3bb312ea99853d1741d838d5a789cbc5eec1fb/pkg/prettyzone/prettyzone.go#L103-L105
Commenting out the first two is sufficient to invert the meaining of 0
within dnscontrol preview
: an explicit DefaultTTL(0)
or TTL(0)
gets through the LINODE
provider and the corrections disappear, but by contrast all the other domains with no TTL settings no longer take up the implicit TTL of 300, and I see corrections of the form MODIFY ttl=300 -> ttl=0
.
So, there are a few directions this could go:
TTL("default")
, but doing so is unnecessary.TTL("default")
(or similar) lets users opt into the provider's default instead of the DNSControl default.TTL(0)
is supported only for providers that explicitly support it via their API.TTL(0)
is supported for any provider, and rely on each provider to raise an error if it is unsupported."0m"
etc as that doesn't visually map well to 0
being used as a default sentinel.TTL(0)
or DefaultTTL(0)
(instead of it being silently ignored).preview / push
, providers would need to add extra logic to check whether a record's DNSControl-configured TTL happens to match the default, and the only correction would be to change the DSP's record from 0
to an explicit number; if so, suppress this correction.Another useful UX thing before, or in addition to, the above: change the way colour highlighting works, to only highlight the changed parts instead of the whole MODIFY
line. That would make it easier to determine what is actually being changed.
I implemented "Remove the non-zero restriction on TTL" in #2475, which AFAICT (as a non-Go developer) was the simplest approach.
While discussing the fact that Linode's API explicitly permits and returns a TTL of 0 to represent "Default", I asked in https://github.com/StackExchange/dnscontrol/issues/2440#issuecomment-1595543504:
It turns out that DNSControl completely ignores any
DefaultTTL(0)
orTTL(0)
settings, causing it to instead fall back on the global DNSControl default of 300 seconds. AFAICT the code inpkg/js/helpers.js
handles things correctly, but when the JSON is parsed into Go, the resultingRecordConfig
has its TTL replaced by the hard-coded global default of 300: https://github.com/StackExchange/dnscontrol/blob/5b3bb312ea99853d1741d838d5a789cbc5eec1fb/models/record.go#L377-L379This prevents the
LINODE
provider (or any other provider) from setting or preserving a TTL of 0 (as was theoretically enabled by #2442).