Skip to content
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 Porkbun (closes #667) #1283

Merged
merged 1 commit into from
Jun 21, 2022
Merged

Conversation

guidopetri
Copy link
Contributor

This PR adds support for Porkbun and closes #667.

Porkbun API doesn't have credentials to authenticate with, then access domains; you just access the endpoint and pass in your secret keys via the POST body. As a result, I called _list_records in the _authenticate function to determine whether the user had permissions to edit the DNS records for that domain.

I ran both the provider file and the test file through black but let me know if that's not desired or if a different code style is preferred (black was in the poetry.lock so I figured that you used that).

I also censored out my IP address that is returned by the /ping endpoint from Porkbun, and I replaced the domain name I used with example.xyz. Hopefully I didn't miss any other sensitive data, but I'd appreciate a heads up in case you see something.

Finally, the tests, when run against the live API, fail about halfway through with a 503 error. I think the API at some point stops responding to requests because it's either overloaded or it just doesn't let a specific user hit the API that hard. I stopped the tests halfway through and continued after a bit and they worked fine, though, so the recordings should be good.

@guidopetri guidopetri force-pushed the porkbun branch 2 times, most recently from 9cbc2b6 to eec5b41 Compare June 12, 2022 01:55
@guidopetri
Copy link
Contributor Author

@adferrand , @AnalogJ , this is hopefully good to merge. Looks like I can't assign reviewers but I'd have put you two :) Let me know if there's any edits you need.

@guidopetri
Copy link
Contributor Author

Hey, any chance this could get merged in soon?

Copy link
Collaborator

@adferrand adferrand left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your contribution! I have a little suggestion for the name of the CLI flags to have more usual wording, but beside this we are good to go!

lexicon/providers/porkbun.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@adferrand adferrand left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks a lot.

@adferrand adferrand merged commit 751d282 into AnalogJ:master Jun 21, 2022
@guidopetri
Copy link
Contributor Author

Thank you! I'm looking forward to using it myself in the certbot docker image you have :P

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.

Add support for porkbun
2 participants