-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Add support for MX and SRV records in pdns provider #4648
Add support for MX and SRV records in pdns provider #4648
Conversation
Welcome @saravanan30erd! |
Hi @saravanan30erd. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
I'm not sure to understand the context of this PR. Anyway, no PR on the code merge without test. |
Yes. PDNS API expects trailing dot for both these MX and SRV records. Without trailing dots, we are getting the errors mentioned in this issue #3930 I have added required trailing dots for these two records now same as CNAME and ALIAS records, after adding trailing dots we can create MX and SRV records in PDNS using external dns without any issues. Trailing dots are added already to CNAME and ALIAS records here https://github.com/kubernetes-sigs/external-dns/blob/master/provider/pdns/pdns.go#L317-L318 |
@saravanan30erd Then wdyt of rewriting in this PR description "Errors to be fixed" with something like "Fixed errors" ? Anyway, once it's tested, it can be reviewed. |
Done, changed the PR description.
Sure, will add the tests. |
@mloiseleur I have updated the test cases for this change. Please review it, thanks. |
/lgtm |
Hi @mloiseleur Thanks for the review. Is there anything I can do on my end to help move it forward? Please let me know if there's anything I can assist with. |
/ok-to-test |
@saravanan30erd One of the other maintainer will review it when he has time. BTW, we have started the process to move the providers out of tree. Feel free to host your own pdns provider. |
@Raffo Gentle Reminder. I wanted to kindly check if you’ve had a chance to review this PR. |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Raffo The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
This PR enables MX and SRV record support for PowerDNS provider in ExternalDNS. Especially for PowerDNS API, it requires explicit trailing dot for records having FQDN such as CNAME, MX, SRV, etc.. For CNAME record, its already covered here https://github.com/kubernetes-sigs/external-dns/blob/master/provider/pdns/pdns.go#L317-L318 so I added trailing dot for MX and SRV records now.
Errors Fixed
Error from PDNS API for SRV record creation
Error from PDNS API for MX record creation
The above errors is fixed by this PR.
Fixes #3930 #4267
Checklist