Skip to content

Implement DS record support for ClouDNS#1018

Merged
tlimoncelli merged 21 commits intoDNSControl:masterfrom
jamaltebi:master
Jan 22, 2021
Merged

Implement DS record support for ClouDNS#1018
tlimoncelli merged 21 commits intoDNSControl:masterfrom
jamaltebi:master

Conversation

@jamaltebi
Copy link
Copy Markdown
Contributor

ClouDNS does supprt DS Record ( DS record can be add just for subdomain nor for root domain).

  • Question: Can I add a DS record for a subdomain, if there are already other records for the same hostname, such as A, MX, TXT, etc.?

==> Answer: No, you can't. First and foremost, in order for you to be able to add a DS record for your subdomain, the delegation part of your subdomain must be in action. In simple words, the relevant NS records for your subdomain, the "delegators" so to say, must be added first. And to add the NS records, there must be no other records for that particular hostname.

@jamaltebi
Copy link
Copy Markdown
Contributor Author

jamaltebi commented Jan 18, 2021

@tlimoncelli I tested the DS record for ClouDNS, It's working. I am waiting for your reply

Comment thread docs/_providers/cloudns.md Outdated
Comment thread integrationTest/integration_test.go Outdated
Comment thread integrationTest/integration_test.go Outdated
Comment thread integrationTest/integration_test.go Outdated
Comment thread integrationTest/integration_test.go
Comment thread providers/cloudns/cloudnsProvider.go Outdated
Comment thread providers/cloudns/cloudnsProvider.go Outdated
Comment thread providers/cloudns/cloudnsProvider.go Outdated
Comment thread providers/cloudns/cloudnsProvider.go Outdated
Comment thread providers/cloudns/cloudnsProvider.go Outdated
@jamaltebi jamaltebi requested a review from tlimoncelli January 21, 2021 11:41
@IT-Sumpfling
Copy link
Copy Markdown
Contributor

IT-Sumpfling commented Jan 21, 2021

Perhaps wait with the review. I've just committed some changes which @jamaltebi will test.
These should

  • fix the order of NS / DS entries (so no longer necessary to list NS first or to delete in 2 passes)
  • implement a very simple rate-limit by delaying each request for 100ms (should fix decrease/delay the number of request per second sended to ClouDNS #1027)
    (ClouDNS seems to rate-limit the API to 10 req/second - but I haven't found a documentation from their side)

jamaltebi and others added 3 commits January 21, 2021 18:11
@IT-Sumpfling
Copy link
Copy Markdown
Contributor

This should be fine for now. Another way to only delay after a 429 http status code. Look at providers/desec/protocol.go for an example.

Yes. But I did not yet get around to test if ClouDNS sends 429 or another status code ...

@tlimoncelli
Copy link
Copy Markdown
Contributor

tlimoncelli commented Jan 21, 2021 via email

@IT-Sumpfling
Copy link
Copy Markdown
Contributor

IT-Sumpfling commented Jan 22, 2021

Thank you. Just to be sure - the branch / pull request should be ready for merge from our side.
integration tests run, order of ns/ds does not matter and your review-comments should be addressed.

If you need anything more, let us know.

Copy link
Copy Markdown
Contributor

@tlimoncelli tlimoncelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job!!!

@tlimoncelli tlimoncelli merged commit d7f40ed into DNSControl:master Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants