Skip to content

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

Merged
SvenDowideit merged 1 commit intolibdns:masterfrom
delthas:fix-record-name
Mar 10, 2021
Merged

Convert between the API implictily absolute and DO standard record names#3
SvenDowideit merged 1 commit intolibdns:masterfrom
delthas:fix-record-name

Conversation

@delthas
Copy link
Copy Markdown
Contributor

@delthas delthas commented Sep 12, 2020

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: libdns/libdns#12

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: libdns/libdns#12
@mholt
Copy link
Copy Markdown
Contributor

mholt commented Sep 14, 2020

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
Copy link
Copy Markdown

ziyadumar commented Sep 30, 2020

caddyserver/caddy#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
Copy link
Copy Markdown

vrudikov commented Oct 8, 2020

Awesome, +1

@IronSinew
Copy link
Copy Markdown

IronSinew commented Jan 13, 2021

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
Copy link
Copy Markdown
Contributor

mholt commented Jan 13, 2021

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
Copy link
Copy Markdown

eslym commented Feb 17, 2021

seems like there is a lot of people facing this issue
caddy-dns/digitalocean#8

@mholt
Copy link
Copy Markdown
Contributor

mholt commented Feb 24, 2021

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
Copy link
Copy Markdown

Hello there,

Any news on this issue? Thanks!

@mholt mholt requested a review from SvenDowideit March 9, 2021 20:09
@SvenDowideit
Copy link
Copy Markdown
Collaborator

SvenDowideit commented Mar 9, 2021 via email

@SvenDowideit
Copy link
Copy Markdown
Collaborator

given libdns/libdns#28 , very LGTM

@SvenDowideit SvenDowideit merged commit 186c4eb into libdns:master Mar 10, 2021
@mholt
Copy link
Copy Markdown
Contributor

mholt commented Mar 10, 2021

Thanks Sven!

@bpolaszek
Copy link
Copy Markdown

Thank you @SvenDowideit ! 🙏

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 this pull request may close these issues.

8 participants