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

DNS Plugin for Infomaniak.com #309

Merged
merged 9 commits into from Jan 23, 2021
Merged

DNS Plugin for Infomaniak.com #309

merged 9 commits into from Jan 23, 2021

Conversation

Sundypha
Copy link
Contributor

Hi @rmbloger.
First of all: Thank you so much for this ACME client.

I added a DNS Plugin for the hoster Infomaniak from Switzerland.

I did use the plugin on two Windows Servers and a Windows Desktop which all worked well.
I tested:

  • example.com
  • *.example.com
  • sub.example.com
  • sub2.sub1.example.com

I've got one question. Is there a way to tell the user if something failed? e.g., the zone hasn't been found? Refer to line 69 and line 150 where I could fail gracefully.

Awaiting your review and feedback!

Best
Simon

@Sundypha
Copy link
Contributor Author

Just figured that in the function Find-InfomaniakZone I query the zone _acme-challenge.example.com which is unnecessary. Going to create a commit shortly.

@rmbolger
Copy link
Owner

Hi @Sundypha. Thanks for the submission! Everything looks great at first glance. Give me a bit to review and test.

@rmbolger rmbolger self-assigned this Jan 20, 2021
@rmbolger rmbolger added the enhancement New feature or request label Jan 20, 2021
Copy link
Owner

@rmbolger rmbolger 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. Just fix the one _acme-challenge check removal and this should be good to merge.

Posh-ACME/Plugins/Infomaniak.ps1 Outdated Show resolved Hide resolved
@rmbolger
Copy link
Owner

I've got one question. Is there a way to tell the user if something failed? e.g., the zone hasn't been found? Refer to line 69 and line 150 where I could fail gracefully.

Forgot to answer this. For things like the domain not being found which you can't really recover from, I'll just use throw "my failure message" to send an exception back up to the user so they can decide what to do.

@Sundypha
Copy link
Contributor Author

You were right. It failed when I had alias abc.example.com since it was only checking the zone example.com instead of the abc zone. Reverted the commit and added throws.

@Sundypha
Copy link
Contributor Author

Hi @rmbolger,

Did again another round of testing including timed out API calls to check debug and throw messages.
I figured now why my changes weren't persistent. VS Code opened the files at instdev.ps1 folder (\Documents). First time writing any Powershell code.

Should I add another $apiRoot in the Function Find-InfomaniakZone for consistency?

@rmbolger
Copy link
Owner

There should definitely be a local instance of $apiRoot within the function so you don't run into any unintended scoping issues. But whether you pass that instance as a parameter in the function or just hard code it doesn't really matter to me. The plugins are pretty small and having to find/replace an extra copy of the string if it ever changes is no big deal.

@Sundypha
Copy link
Contributor Author

I thought so exactly. I wasn't sure if Powershell has some kind of external scoping into functions called since it was working without but it felt awkward.

@rmbolger
Copy link
Owner

Added one last fix that it looks like you had found originally and fixed in the Remove method but forgot to copy to the Add method.

@Sundypha
Copy link
Contributor Author

Hang on, I thought about this one. I'll try if the $RecordName can be used for adding the record instead of the short version.

@Sundypha
Copy link
Contributor Author

Yes, source_idn would work, but target_idn doesn't. The API request must have target.
We could change it to $RecordName, but I want to try with alias', too.
Will report shortly.

@Sundypha
Copy link
Contributor Author

Sundypha commented Jan 22, 2021

With alias'ing the source_idn with $RecordName can't be used, so we stick with the $recShort:

Invoke-RestMethod : {"result":"error","error":{"code":"unexpected_error","description":"An unexpected error occurred"}}

Edit: Just to clarify: The source_idn only fails for the POST request.

@rmbolger rmbolger merged commit 7abfc21 into rmbolger:main Jan 23, 2021
@rmbolger
Copy link
Owner

Thanks for the effort, @Sundypha. I'll try to get this out in a release soon.

@Sundypha Sundypha deleted the dns1-infomaniak-plugin branch January 24, 2021 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants