CLOUDNS: add registrar support#3961
Merged
tlimoncelli merged 4 commits intoStackExchange:mainfrom Jan 7, 2026
Merged
Conversation
tlimoncelli
added a commit
that referenced
this pull request
Jan 6, 2026
There was already a [Limiter](https://pkg.go.dev/golang.org/x/time/rate) in use here to keep the rate of requests below the apparent limit. The ClouDNS API doesn't give any sort of proper API response when rate limit is reached. It's a 200 status code and an error message in the JSON body, and no headers that would help to track or back off for the right amount of time. There was a comment in the implementation that mentions an undocumented 10-per-second limit, while the error message they give today says that the limit is 20 per second. I kept the settings on the Limiter the same since 10 per second should be plenty fast. But it will now retry the request when the rate limit is reached. At the same time, it "steals" some reservations on `rate.Limiter` to quiet other concurrent ClouDNS API calls for about half a second. This seems to be plenty to fix my rate-limit issues. (I tested with 20 domains with ClouDNS as both registrar and DNS provider using the functionality in #3961.) When rate limit is reached, it emits a warn-level message. This follows a pattern I see in `adguardhome` and `desec` providers but I don't love it—it's less important and less actionable than other warn-level messages in the project. Co-authored-by: Tom Limoncelli <[email protected]>
Contributor
|
CC @pragmaton Wow! This is great! By the way.... in the future I plan on merging things so that one initializer is used whether the providers is a registrar or a DNS server. |
tlimoncelli
reviewed
Jan 6, 2026
| if existingStr != desiredStr { | ||
| return []*models.Correction{ | ||
| { | ||
| Msg: fmt.Sprintf("Update nameservers from '%s' to '%s'", existingStr, desiredStr), |
Contributor
|
I hope to do a release later today. I'm going to merge this. The 1 comment is minor and can be fixed later in a separate PR. Thanks for adding this! |
tlimoncelli
added a commit
that referenced
this pull request
Jan 12, 2026
Fix a format string that used `%s` where `%q` is more appropriate, to escape the string fully. This addresses the feedback in #3961 that I didn't get done in time to be merged. --------- Co-authored-by: Tom Limoncelli <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ClouDNS uses the same API for DNS and registrar-related operations. Following the pattern I saw in other provider/registrar combos (autodns, cscglobal), I implemented the registry call in the
cloudnsProviderobject type, and when used as a registry it creates an additional instance of the Provider.Incidentally this causes rate-limiting functionality not to function optimally because each of the instances—the provider and registrar—have their own requestLimit object, causing the rate limit to be tracked and applied within each separately. I'm going to follow up with a PR that will improve the API rate-limit implementation for ClouDNS.
The ClouDNS nameserver API responds with different structures apparently depending on the number of name servers that the domain has. I observed three on the same domain:
["abc.ns.cloudflare.net", "def.ns.cloudflare.net"]{"1":"pns1.cloudns.net","2":"pns2.cloudns.net","3":"pns3.cloudns.net","4":"pns4.cloudns.net"}{"1":"pns1.cloudns.net","2":"pns2.cloudns.net","3":"pns3.cloudns.net","4":"pns4.cloudns.net","5":"abc.ns.cloudflare.net","6":"","7":"","8":""}This covers all of those cases, plus the possible case where the array contains blank strings.
The new
setNameserversAPI call needed to send multiple copies of the same keynameservers[]in the query value, which is supported by the http query values object, but was not possible with the existing call pattern because thegetfunction was built to accept a string map instead of a query values object. So I refactoredgetinto a wrapper function that converts the map into a query values object, which calls the newgetWithQueryfunction with the query values object.