AnalogJ / lexicon

Manipulate DNS records on various DNS providers in a standardized way.
MIT License
1.48k stars 305 forks source link

Default TTLs don't seem to be implemented #191

Closed jvanasco closed 5 years ago

jvanasco commented 6 years ago

I noticed this while working a PR for issue #172

When creating a record on Namecheap, no TTL is set:

https://github.com/AnalogJ/lexicon/blob/master/lexicon/providers/namecheap.py#L65

Looking at a few other providers this seems consistent. There is an explicit --ttl flag, it's just not used.

It might make sense to default TTLs to something short (60s) and then allow the --ttl flag to override that. With a lot of DNS systems, leaving it null will engage a default value on the serverside that could be from 30mins to 24hours, which can be a mess to work around if it gets cached.

AnalogJ commented 6 years ago

ah, thats definitely wrong. a TTL value passed in via --ttl should definitely be processed by lexicon.

You are correct, it looks like namecheap does not handle that option correctly, while other providers like cloudflare do.

I initially wanted to have a test suite that implemented a TTL test, however I ended up having to remove it due to the fact that the range of acceptable TTL values varied significantly across providers.

Once I finish work on #190, I'll come back and take a look at this. In the meantime if you could update the namecheap provider as part of your PR that would be awesome!

jvanasco commented 6 years ago

I'll include the namecheap fix in my PR

I went through the providers, and these seem to not implement the TTL option. I might have missed one or two:

trinopoty commented 6 years ago

Linode has a ttl option called 'Default' and a ttl value of 0 tells the api to apply this 'Default' ttl.

trinopoty commented 6 years ago

Also, the range of ttl values supported by different providers varies greatly and may change over time. This factor needs to be considered too.

jvanasco commented 6 years ago

Linode has a ttl option called 'Default' and a ttl value of 0 tells the api to apply this 'Default' ttl.

The current provider doesn't accept the --TTL option from the commandline though. The 0 is hardcoded into the provider and the default is managed on the server. Using the 0 as a fallback default seems like the right choice, but the commandline flag in options should override it.

trinopoty commented 6 years ago

I couldn't figure out how to deal with invalid ttl values that linode won't accept. Additionally, the provider I used as reference did not implement setting ttl from options.

jvanasco commented 6 years ago

@AnalogJ one quick question - is there a way in your tests setup that I can only one provider at a time? i can't seem to figure out how.

AnalogJ commented 6 years ago

Hey @jvanasco Yeah you can do it by running the following:

py.test tests/providers/test_{PROVIDER_NAME}.py

Its mentioned in the CONTRIBUTING.md file, but I should probably call it out better.

jvanasco commented 6 years ago

thanks!

i saw that, but it was failing.

turns out my virtualenv got stuck from one of the brownouts, and py.test was linking to an older system install, so I had to use pytest (no period). i don't exactly know how or why this happened. it makes little sense.

AnalogJ commented 6 years ago

ah that sucks :( Hopefully its all working now though