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

LINODE: TTL does not support "Default" (0) #2440

Closed
str4d opened this issue Jun 16, 2023 · 7 comments · Fixed by #2442
Closed

LINODE: TTL does not support "Default" (0) #2440

str4d opened this issue Jun 16, 2023 · 7 comments · Fixed by #2442

Comments

@str4d
Copy link
Contributor

str4d commented Jun 16, 2023

Describe the bug
I set up a dnsconfig.js matching the records for a domain I manage through Linode. All of the records in Linode have their TTL column set to "Default". I do not configure an explicit TTL in dnsconfig.js for any record, leaving it at the dnscontrol default.

When I run dnscontrol preview, every record is marked as a modification such as:

#1: MODIFY A example.com: (1.2.3.4 ttl=0) -> (1.2.3.4 ttl=300), Linode ID: NNN: {"type":"A","name":"","target":"1.2.3.4","ttl_sec":300}

To Reproduce

  1. Configure a domain name to use Linode for DNS records.
  2. Add a DNS A record to Linode with some label and IP, and leave the TTL as "Default".
  3. Write a dnsconfig.js with the same A record label and IP, and no configured TTL.
  4. dnscontrol preview

Expected behavior
No modifications should be proposed.

Versions

  • DNSControl version: 4.1.1 (Homebrew)
  • DNS Provider: LINODE

Additional context
The LINODE provider documentation has a caveat that the TTLs must be from a specific list. The file that it links to does not exist in the Linode source tree anymore. When I searched for an equivalent list, I found this one that explicitly includes the option { label: 'Default', value: 0 }.

https://github.com/linode/manager/blob/07be31a25399c8583a55a3727b8a38e0a6e25cd7/packages/manager/src/features/Domains/DomainRecordDrawer.tsx#L320-L336

@str4d
Copy link
Contributor Author

str4d commented Jun 16, 2023

The dnscontrol source code has the permalink for the file referenced in the docs:

func (api *linodeProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, existingRecords models.Records) ([]*models.Correction, error) {
// Linode doesn't allow selecting an arbitrary TTL, only a set of predefined values
// We need to make sure we don't change it every time if it is as close as it's going to get
// By experimentation, Linode always rounds up. 300 -> 300, 301 -> 3600.
// https://github.com/linode/manager/blob/edd99dc4e1be5ab8190f243c3dbf8b830716255e/src/domains/components/SelectDNSSeconds.js#L19
for _, record := range dc.Records {
record.TTL = fixTTL(record.TTL)
}

From that file, it appears that Linode did support a ttl_sec value of 0 back then, which it mapped to ONE_DAY: https://github.com/linode/manager/blob/edd99dc4e1be5ab8190f243c3dbf8b830716255e/src/domains/components/SelectDNSSeconds.js#L51

@str4d
Copy link
Contributor Author

str4d commented Jun 16, 2023

Linode's own documentation confirms that the default TTL is still one day.

@TomOnTime
Copy link
Collaborator

CC @koesie10

Interesting bug!

That raises an interesting question. Should allowedTTLValues (

var allowedTTLValues = []uint32{
) be changed to include 0 or should the fixTTL function turn 0 into 86400? (I think the first option)

@str4d
Copy link
Contributor Author

str4d commented Jun 17, 2023

I personally agree that 0 should be allowed as a setting, because that is what the Linode API is returning when "Default" is set via their UI. If fixTTL turned 0 into 86400, that would still result in a MODIFY (it would be a functional no-op, but not a technical one).

The question though is whether dnscontrol has a good way to represent "Default" in its own UX. Do any other providers have a similar concept? How do they handle a TTL of 0?

@koesie10
Copy link
Contributor

I agree that 0 should be added to the allowedTTLValues. I believe this should work and does not require any further modifications.

I have created a PR for this, which also updates the docs. I'm just finding conflicting information about what 0 actually means:

I've taken the value from the API documentation since that's what the provider is using, but I'm not sure if that actually matches with what Linode does.

@str4d
Copy link
Contributor Author

str4d commented Jun 17, 2023

@koesie10 I think you were looking at the wrong part of that documentation.

data array of objects
expire_sec integerThe amount of time in seconds that may pass before this Domain is no longer authoritative.Valid values are 0, 300, 3600, 7200, 14400, 28800, 57600, 86400, 172800, 345600, 604800, 1209600, and 2419200.Any other value is rounded up to the nearest valid value.A value of 0 is equivalent to the default value of 1209600.
-- --
ttl_sec integer“Time to Live” - the amount of time in seconds that this Domain’s records may be cached by resolvers or other domain servers.Valid values are 0, 300, 3600, 7200, 14400, 28800, 57600, 86400, 172800, 345600, 604800, 1209600, and 2419200.Any other value is rounded up to the nearest valid value.A value of 0 is equivalent to the default value of 86400.
-- --

AFAICT the default is still 1 day for ttl_sec, not 14 days.

@str4d
Copy link
Contributor Author

str4d commented Jul 10, 2023

Fixed the comment as part of #2444.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants