search

StackExchange / dnscontrol

Infrastructure as code for DNS!
https://dnscontrol.org/
MIT License
3.13k stars 397 forks source link

DSL does not properly validate function arguments. #64

Closed BrandonPotter closed 7 years ago

BrandonPotter commented 7 years ago

I'm a little fuzzy on why this would be happening, but it seems like a possible bug in the route53 provider.

I have a MX record for a subdomain subdomain, i.e. someone@department.example.com that should point to A record of mail4.example.com.

Using either:

MX('department', 'mail4', TTL(600))

or...

MX('department', 'mail4.example.com.', TTL(600))

... it ends up in Route 53 with a recordset value of:

0 example.com.

If I manually change the record in Route 53 to mail4 or mail4.example.com., dnscontrol picks up the change difference and changes it back to example.com., so it seems like an issue in the dnscontrol Route 53 provider.

BrandonPotter commented 7 years ago

(Edited to correct values that I screwed up in translating this to generic example.com)

BrandonPotter commented 7 years ago

Logs from dnscontrol after manually modifying the Route 53 records and running dnscontrol again...

Initialized 1 registrars and 1 dns service providers.
******************** Domain: example.com
----- Getting nameservers from: r53
----- DNS Provider: r53... 1 correction
#1: MODIFY MX department.example.com: (0 mail4.example.com. 300 priority=0) -> (0 example.com. 300 priority=0)
captncraig commented 7 years ago

Thanks for the report. Trying to reproduce now.

captncraig commented 7 years ago

Ah. I think I see the problem. Our documentation does not include all of the functions yet, including MX. My bad.

MX should specify priority, then target: MX("@", 5, "mail4", TTL(600))

Validation should also have caught this, both in the javascript, and in the backend.

BrandonPotter commented 7 years ago

D'oh, this was the only record in my file that didn't have priority in it - didn't even see that.

Can confirm works with priority parameter specified. Feel free to close.

captncraig commented 7 years ago

Gonna leave open to address the usability issue. It should be easy to check the types of the arguments to make sure we get a number and a string. And the backend should have rejected an empty target as well, instead of silently converting it to "@" somehow. And the docs.

I totally blame us on this one.

tlimoncelli commented 7 years ago

https://github.com/StackExchange/dnscontrol/pull/163 will help (and may fix) this problem.