-
Notifications
You must be signed in to change notification settings - Fork 440
AUTODNS: Enable "get-zones" (ListZones, EnsureZoneExists, GetRegistrarCorrections) #3568
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
One wasn't handled, the other one can have the error path dealt with directly in a conditional.
We already advertised providers.CanGetZones, but ZoneLister's ListZones() wasn't implemented yet.
This is trying to lookup a zone (as in DNS), not a domain (as in Registrar).
This allows inspecting on the callsite if zone doesn't exist, or if its another error.
This implements ZoneCreator for AUTODNS, allowing dnscontrol to create zones if they don't already exist. The API insists nameservers and soa to be set, so we use some dummy values, they'll get overwritten later anyways. This makes use of the previous error wrapping commit, to check whether the zone exists currently.
These are sent when an API request triggers a longer-running background task. Log a FUTUREWORK, if it becomes necessary to poll for completion.
This implements registrar support, allowing to update NS records for the apex.
cc @arnoschoon |
Code looks good to me. @arnoschoon, do you approve? |
Hi @flokli , thanks a bunch for adding this functionality. I'm with @tlimoncelli on this, it looks good to me and more importantly most of the tests still execute correctly so managing records keeps working.
Except |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I said managing DNS records works properly, but I'm confused about the domain management (registrar) part but that might be a limit of my test setup since the used domain is not one I actually registered.
This is the exact failing testcase:
Highly doubt that's due to the changes in this PR. |
Which parts are you confused about? Happy to clarify things. |
Sounds like AUTODNS doesn't support 0-length TXT records. This will fix it:
|
pushed, thanks! |
Best reviewed commit-by commit.
ListZones
We already advertised
providers.CanGetZones
, butZoneLister
'sListZones()
wasn't implemented yet. Adding support is very simple, we just need toPOST zone/_search
without filters. I made some of the error paths more concise (if err := …; err != nil { … }
)EnsureZoneExists
This allows to create new zones. We do some error wrapping before to be able to distinguish a zone not existing from other failures, and then create the zone with dummy values, using the
POST /zone
endpoint.Registrar support
Use the
GET domain/{name}
endpoint to retrieve nameserver config of a domain, and usePUT domain/{name}
to update it. ThenGetRegistrarCorrections
can simply compare the list of nameservers retrieved ongetDomain
with the desired values, and if different, do a (sparse) request to update these values.I had to juggle some of the provider initialization around, to be able to use the same code for DomainServiceProvider and RegistrarType. We'll end up with two instances, but considering both deal with rate limiting it should be fine, and much more complicated if we'd try to aggregate clients based on the same settings, as rate-limits are per user.