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

Fix Variomedia API #4787

Merged
merged 2 commits into from
Sep 15, 2023
Merged

Conversation

TobiasGrave
Copy link

@TobiasGrave TobiasGrave commented Sep 7, 2023

The Variomedia API integration is broken for quite some time now. There are 3 problems that have to be addressed:

  1. The determination of the root domain does not work porperly
    This has been addressed in the following pull request that never got merged: Improved determination of root domain for the Variomedia DNS API #3244

  2. The deletion of _acme-challenge DNS records fails if their content starts with -
    This is due to grep interpreting all strings starting with - as options. In order to fix this, the option -- has to be added before the search term "$txtvalue" in grep (Improved determination of root domain for the Variomedia DNS API #3244 (comment)).

  3. The deletion of _acme-challenge DNS records fails due to a recent change in the API
    The API returns a new field tags in the attributes object since June 7 with a (usually empty) list of tags (e.g. "tags":[] or "tags":["custom","dyndns"]).
    This adds more [ characters to the return string from the API, which break the inital split of the returned JSON data using cut (cut -d '[' -f2). The easiest solution that I could come up with is to delete the tags field entirely using sed (sed -E 's/,"tags":\[[^]]*\]//g').

I have made the necessary changes to fix all these problems and done some testing to confirm that the Variomedia API works again.

Note: This is a second pull request, I closed the first one (#4769) because I screwed up the testing.

@github-actions
Copy link

github-actions bot commented Sep 7, 2023

Welcome
Please make sure you're 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 ✨

@TobiasGrave
Copy link
Author

Welcome Please make sure you're 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 for Variomedia successfully completed.

fulldomain=$1
i=1
domain=$1
i=2
Copy link
Member

Choose a reason for hiding this comment

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

i should start from "1"
There is no "_acme-challenge" prefix if it's working in dns alias mode.

Copy link
Author

Choose a reason for hiding this comment

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

Ok I changed that, Github actions tests successfully completed.

@Neilpang Neilpang merged commit 8565a85 into acmesh-official:dev Sep 15, 2023
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