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

Feature: DNS API for IONOS cloud #5110

Merged
merged 22 commits into from
Jun 15, 2024

Conversation

zak905
Copy link
Contributor

@zak905 zak905 commented Apr 24, 2024

This change set extends the existing DNS API script for IONOS dns_ionos.sh to support the IONOS Cloud API. IONOS users have two alternatives for managing their DNS zones: the IONOS Cloud API and the IONOS DNS API (which is supported by the current implementation). The objective is to allow users of the IONOS Cloud API to also use the script.

As explained in the script header comment, the script decides which API to use based on the provided credentials. If IONOS_PREFIX and IONOS_SECRET are provided, the script will use the IONOS DNS API, else if IONOS_TOKEN is provided the script will use the IONOS Cloud API.

Note: This contribution is done by IONOS DNS cloud team, the team responsible directly for the service.

Copy link

Welcome
First thing: don't send PR to the master branch, please send to the dev branch instead.
Please make sure you've read our DNS API Dev Guide and DNS-API-Test.
Then reply on this message, otherwise, your code will not be reviewed or merged.
We look forward to reviewing your Pull request shortly ✨
注意: 必须通过了 DNS-API-Test 才会被 review. 无论是修改, 还是新加的 dns api, 都必须确保通过这个测试.

@zak905 zak905 changed the base branch from master to dev May 6, 2024 08:10
@zak905 zak905 marked this pull request as ready for review May 6, 2024 09:29
@zak905
Copy link
Contributor Author

zak905 commented May 6, 2024

Hi @Neilpang,

can you please take a look when you have a chance ?

The DNS tests are passing successfully: https://github.com/ionos-cloud/acme.sh/actions/runs/8941008104

To make the tests pass, I had to make some changes to the acmetest project so that the TXT record content is randomized: acmesh-official/acmetest#26

For now, this PR uses my own fork, as you can see here: https://github.com/ionos-cloud/acme.sh/blob/add_ionos_cloud_script/.github/workflows/DNS.yml#L70. I have created a PR in the acmetest repo as well: acmesh-official/acmetest#26

Also how can I update the documentation?

@Neilpang
Copy link
Member

Neilpang commented May 6, 2024

remove this change please:
image

@zak905
Copy link
Contributor Author

zak905 commented May 6, 2024

@Neilpang it's removed now. However, as I mentioned above, this will make DNS tests fail. acmesh-official/acmetest#26 is needed to make them pass, because it seems like somehow a test case starts before the previous one removes the TXT record. Inserting a TXT record with the same name and the same content is not allowed by our API ( maybe it's not the case for other DNS providers), so a non OK response is returned.

@zak905
Copy link
Contributor Author

zak905 commented May 7, 2024

Thanks for merging the acmetest PR. Here is the successful run for the latest triggered action: https://github.com/ionos-cloud/acme.sh/actions/runs/8969436076

@zak905
Copy link
Contributor Author

zak905 commented Jun 7, 2024

@Neilpang I just wanted to draw your attention to this PR. Any thoughts ?

dnsapi/dns_ionos_cloud.sh Outdated Show resolved Hide resolved
dnsapi/dns_ionos_cloud.sh Outdated Show resolved Hide resolved
@Neilpang Neilpang merged commit f7f8ea9 into acmesh-official:dev Jun 15, 2024
13 checks passed
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.

2 participants