search

octodns / octodns-dnsimple

Dnsimple API provider for octoDNS
MIT License
0 stars 2 forks source link

Extra dot being added with SRV records #19

Closed rockpenguin closed 9 months ago

rockpenguin commented 1 year ago

Hello!

It looks like there might be a bug similar to 832 in the OVH provider where it gets in the cycle of trying to fix the trailing dot at the end of the FQDN.

********************************************************************************
* mydomain.net.
********************************************************************************
* dnsimple (DnsimpleProvider)
*   Update
*     <SrvRecord SRV 3600, _imaps._tcp.mydomain.net., [''0 1 993 imap.fastmail.com..'']> ->
*     <SrvRecord SRV 3600, _imaps._tcp.mydomain.net., [''0 1 993 imap.fastmail.com.'']> (config)
*   Update
*     <SrvRecord SRV 3600, _submission._tcp.mydomain.net., [''0 1 587 smtp.fastmail.com..'']> ->
*     <SrvRecord SRV 3600, _submission._tcp.mydomain.net., [''0 1 587 smtp.fastmail.com.'']> (config)
*   Summary: Creates=0, Updates=2, Deletes=0, Existing Records=17
********************************************************************************

And if I remove the trailing dot from one of the FQDNs in the config, octoDNS throws an error:

  File "/Users/dave/src/infrastructure/dns/env/lib/python3.11/site-packages/octodns/record/__init__.py", line 153, in new
    raise ValidationError(fqdn, reasons)
octodns.record.ValidationError: Invalid record _caldavs._tcp.mydomain.net.
  - SRV value "caldav.fastmail.com" missing trailing .

Similar to the OVH issue, if I remove the dot from the following line, then it works as hoped.

https://github.com/octodns/octodns-dnsimple/blob/93abd10c18a4cb06b832d3a26d632dd4f71b2fd8/octodns_dnsimple/__init__.py#L222

Happy to do a PR if needed. Thanks!

Dave

yzguy commented 1 year ago

Should be a straight forward fix. I would be happy to review a PR. The only concern I'd have is that is it possible relative values are returned by the API? This is the case in other providers like octodns_constellix, you can put in relative or absolute values, and they will come back in the API.

To avoid OctoDNS making a bunch of changes because it wants an absolute value and only a relative one is returned, there is logic to take a relative value from the API and make it absolute, so that the absolutes can be compared. If this is the case in DNSimple we'd want additional logic like below:

https://github.com/octodns/octodns-constellix/blob/main/octodns_constellix/__init__.py#L129-L135

This would handle the case the target is set to . or when the target is not . and target is relative (adding the zone + period) or if it's absolute (do nothing)

If no relatives are ever returned, then simply removing the . from Line 222 would be fine. I suspect this same change is what we need for this https://github.com/octodns/octodns-selectel/pull/23

TIL . as a target means the service will be blocked.

github-actions[bot] commented 9 months ago

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 7 days.