search

libdns / digitalocean

MIT License
10 stars 16 forks source link

Convert between the API implictily absolute and DO standard record names #3

Closed delthas closed 3 years ago

delthas commented 4 years ago

Previously, this DNS provider incorrectly assumed that DNS records passed to it followed the standard notation, used by the DigitalOcean API, that absolute DNS record names end with a dot, and relative DNS record names don't.

This is incorrect because libdns uses de-facto implicitly absolute DNS record names with a dot suffix, which causes any DNS record added with eg certmagic to incorrectly be treated as relative, resulting in records like: _acme-challenge.example.org.example.org

This fixes this issue by converting between the standard dot-suffix DNS record name notation used by the DigitalOcean API and the implicitly absolute record names used by libdns.

See: https://github.com/libdns/libdns/issues/12

mholt commented 4 years ago

Thanks for the PR! We will look at this closer as soon as we decide on the best path going forward in the linked issue.

ziyadumar commented 4 years ago

https://github.com/caddyserver/caddy/issues/3766 I'm here from an issue raised in the wrong place due to record-name parsing bug! Glad to see the PR. Will this be fixed soon hopefully? :) Or is it already fixed? Eagerly awaiting for this, thank you!

vrudikov commented 4 years ago

Awesome, +1

IronSinew commented 3 years ago

Ran into this today as well. I had to remove the digitalocean resolver reference from my caddyfile to get a cert to generate, which is leaving me without a wildcard at the moment. The txt hostnames present in the dns records are akin to: _acme-challenge.domain.tld.domain.tld -- which is obviously incorrect. Is there anything that is preventing this from going in that I'm missing from the discussion thread above or other context? @mholt

mholt commented 3 years ago

The discussion in the linked thread has not been completed yet. I am awaiting feedback from a sufficient number of users before committing to a decision. We need to make sure it serves the vast majority of DNS providers first.

eslym commented 3 years ago

seems like there is a lot of people facing this issue https://github.com/caddy-dns/digitalocean/issues/8

mholt commented 3 years ago

The upstream issue has been resolved; record names passed in are now relative to zone. Feel free to patch this library accordingly! :) These new helper functions might be useful: https://pkg.go.dev/github.com/libdns/libdns#AbsoluteName

bpolaszek commented 3 years ago

Hello there,

Any news on this issue? Thanks!

SvenDowideit commented 3 years ago

I'll look at it tomorrow when I get back to work!

On Wed, Mar 10, 2021 at 6:09 AM Matt Holt @.***> wrote:

@mholt https://github.com/mholt requested your review on: #3 https://github.com/libdns/digitalocean/pull/3 Convert between the API implictily absolute and DO standard record names.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/libdns/digitalocean/pull/3#event-4431061856, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAG6TA2BESYFKJRYFLY2LLTCZW6FANCNFSM4RKBKDZQ .

--

Sven.

SvenDowideit commented 3 years ago

given https://github.com/libdns/libdns/pull/28 , very LGTM

mholt commented 3 years ago

Thanks Sven!

bpolaszek commented 3 years ago

Thank you @SvenDowideit ! 🙏