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

Fixes #491 - Add support for DomeneShop #492

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

eloekset
Copy link

An attempt at implementing support for https://domene.shop.

I haven't even tested it once, so don't merge this until it's tested. How can I deploy a PR to my own environment to test it?

@eloekset
Copy link
Author

I've tested and verified that this provider now works fine.

image

image

@eloekset
Copy link
Author

For some reason I'm not able to create a PR for the wiki, but this is the documentation related to this provider:

Domeneshop

App settings

  • Acmebot:DomeneShop:ApiKeyUser
    • API user from the token column at domene.shop
  • Acmebot:DomeneShop:ApiKeyPassword
    • API password from the secret column at domene.shop

Example API Token configuration

Navigate to https://domene.shop/admin?view=api

image

@eloekset
Copy link
Author

I suspect a bug when trying to delete a TXT record. I made another commit to this branch, which is not merged into this PR yet:
3bd785a

Are you able to confirm how this works @shibayan? I think we should try to refactor the providers to be able to write unit tests for some sample domains and sample inputs to the public methods. That would make it much easier to contribute with additional providers.

@shibayan shibayan self-requested a review July 1, 2022 06:16
@shibayan shibayan added the enhancement New feature or request label Jul 1, 2022
@eloekset
Copy link
Author

eloekset commented Jul 3, 2022

The bug has been fixed and verified with unit tests.

@shibayan
Copy link
Owner

shibayan commented Jul 4, 2022

Acmebot's DNS Provider implementation handles relative record names correctly, so I am not aware of any defects. It is left to the individual DNS Provider implementation.

@eloekset
Copy link
Author

eloekset commented Jul 4, 2022

The bug is fixed now in this commit and verified with unit tests.

I started off copying the CustomDnsProvider. For some reason the DomeneShopProvider shouldn't combine the relativeRecordName with the DnsZone.Name like the CustomDnsProvider does.
image

I guess it depends on the implementation of the DNS provider's API whether or not the DnsZone.Name should be included.

@shibayan
Copy link
Owner

It is a CustomDnsProvider-specific specification, not a bug in Acmebot's implementation; different DNS Providers require different information, so they pass neutral values.

@josteinkirkebak
Copy link

What is the status on this? Any hope of having this merged into the main branch? @shibayan

@eloekset
Copy link
Author

What is the status on this? Any hope of having this merged into the main branch? @shibayan

You can use my fork if you need support for Domeneshop and/or Namecheap 😃

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

3 participants