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 DNS Provider for TransIP #703

Merged
merged 17 commits into from
Nov 6, 2018
Merged

Add DNS Provider for TransIP #703

merged 17 commits into from
Nov 6, 2018

Conversation

mdbraber
Copy link
Contributor

@mdbraber mdbraber commented Nov 5, 2018

I've written a DNS provider for the Dutch provider TransIP (transip.nl). They've just launched a new official Go Library which this project uses. Let me know if there are any questions before merging!

@mdbraber
Copy link
Contributor Author

mdbraber commented Nov 5, 2018

Output of test:

$ go test -v -cover
=== RUN   TestNewDNSProvider
=== RUN   TestNewDNSProvider/success
=== RUN   TestNewDNSProvider/missing_all_credentials
=== RUN   TestNewDNSProvider/missing_account_name
=== RUN   TestNewDNSProvider/missing_private_key_path
=== RUN   TestNewDNSProvider/could_not_open_private_key_path
--- PASS: TestNewDNSProvider (0.00s)
    --- PASS: TestNewDNSProvider/success (0.00s)
    --- PASS: TestNewDNSProvider/missing_all_credentials (0.00s)
    --- PASS: TestNewDNSProvider/missing_account_name (0.00s)
    --- PASS: TestNewDNSProvider/missing_private_key_path (0.00s)
    --- PASS: TestNewDNSProvider/could_not_open_private_key_path (0.00s)
=== RUN   TestNewDNSProviderConfig
=== RUN   TestNewDNSProviderConfig/success
=== RUN   TestNewDNSProviderConfig/missing_all_credentials
=== RUN   TestNewDNSProviderConfig/missing_account_name
=== RUN   TestNewDNSProviderConfig/missing_private_key_path
=== RUN   TestNewDNSProviderConfig/could_not_open_private_key_path
--- PASS: TestNewDNSProviderConfig (0.00s)
    --- PASS: TestNewDNSProviderConfig/success (0.00s)
    --- PASS: TestNewDNSProviderConfig/missing_all_credentials (0.00s)
    --- PASS: TestNewDNSProviderConfig/missing_account_name (0.00s)
    --- PASS: TestNewDNSProviderConfig/missing_private_key_path (0.00s)
    --- PASS: TestNewDNSProviderConfig/could_not_open_private_key_path (0.00s)
=== RUN   TestLivePresent
--- SKIP: TestLivePresent (0.00s)
    transip_test.go:140: skipping live test
=== RUN   TestLiveCleanUp
--- SKIP: TestLiveCleanUp (0.00s)
    transip_test.go:153: skipping live test
PASS
coverage: 25.6% of statements
ok      github.com/mdbraber/lego/providers/dns/transip  0.013s

@ldez ldez changed the title Add TransIP as DNS provider [updated] Add DNS Provider for TransIP Nov 5, 2018
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

Tests fail

Could you do a test with the following command and give me the output:

rm -rf ./lego
./lego -m your@email.com -a -x http-01 -x tls-alpn-01 --dns transip -s https://acme-staging-v02.api.letsencrypt.org/directory -d mydomain.com -d *.mydomain.com run

providers/dns/transip/transip.go Outdated Show resolved Hide resolved
@mdbraber
Copy link
Contributor Author

mdbraber commented Nov 6, 2018

I've updated the domainName / subdomain logic, tests and cleaned up some other parts of the code. Please take a look at the domainName / subdomain logic - maybe there's a cleaner way, but this works too. Also updated the branch.

Here's the output of the command you requested:

mdbraber-mbp:lego mdbraber$ make build && TRANSIP_ACCOUNT_NAME="mdbraber" TRANSIP_PRIVATE_KEY_PATH="/Users/mdbraber/transip.key" ./lego -m m@mdbraber.com -a -x http-01 -x tls-alpn-01 --dns transip --dns-resolvers ns0.transip.nl -s https://acme-staging-v02.api.letsencrypt.org/directory -d mdbraber.net -d *.mdbraber.net run
rm -rf dist/ builds/ cover.out
go build
2018/11/06 09:12:03 [INFO] [mdbraber.net, *.mdbraber.net] acme: Obtaining bundled SAN certificate
2018/11/06 09:12:04 [INFO] [*.mdbraber.net] AuthURL: https://acme-staging-v02.api.letsencrypt.org/acme/authz/6cJliCi531DXAnZtsGO3eGPpXZQVDqLlfunbsG_ZWJ8
2018/11/06 09:12:04 [INFO] [mdbraber.net] AuthURL: https://acme-staging-v02.api.letsencrypt.org/acme/authz/BmgGpyhKMLhvUxIu__SXmkFgqsAFQmctYaZgXmXirTI
2018/11/06 09:12:04 [INFO] [mdbraber.net] acme: Authorization already valid; skipping challenge
2018/11/06 09:12:04 [INFO] [mdbraber.net] acme: Preparing to solve DNS-01
2018/11/06 09:12:05 [INFO] [mdbraber.net] acme: Trying to solve DNS-01
2018/11/06 09:12:05 [INFO] [mdbraber.net] Checking DNS record propagation using [ns0.transip.nl:53]
2018/11/06 09:12:05 [INFO] Wait [timeout: 10m0s, interval: 10s]
2018/11/06 09:12:20 [INFO] [mdbraber.net] The server validated our request
2018/11/06 09:12:22 [INFO] [mdbraber.net, *.mdbraber.net] acme: Validations succeeded; requesting certificates
2018/11/06 09:12:23 [INFO] [mdbraber.net] Server responded with a certificate.

@mdbraber
Copy link
Contributor Author

mdbraber commented Nov 6, 2018

@ldez could you use the current 'master' branch of the gotransip API? There are several issues with the 5.8 release which are all solved in the current master. I've created an issue to ask them to create a new release: transip/gotransip#9

@ldez
Copy link
Member

ldez commented Nov 6, 2018

2018/11/06 09:12:04 [INFO] [mdbraber.net] acme: Authorization already valid; skipping challenge

could you retry?

# required before running the command (remove cache)
rm -rf ./lego

./lego -m your@email.com -a -x http-01 -x tls-alpn-01 --dns transip -s https://acme-staging-v02.api.letsencrypt.org/directory -d mydomain.com -d *.mydomain.com run

@mdbraber
Copy link
Contributor Author

mdbraber commented Nov 6, 2018

@ldez sorry, I still need to get my coffee ;-) Here's the new output:

mdbraber-mbp:lego mdbraber$ make build && TRANSIP_ACCOUNT_NAME="mdbraber" TRANSIP_PRIVATE_KEY_PATH="/Users/mdbraber/transip.key" ./lego -m m@mdbraber.com -a -x http-01 -x tls-alpn-01 --dns transip --dns-resolvers ns0.transip.nl -s https://acme-staging-v02.api.letsencrypt.org/directory -d mdbraber.net -d *.mdbraber.net run
rm -rf dist/ builds/ cover.out
go build
2018/11/06 09:57:34 [INFO] [mdbraber.net, *.mdbraber.net] acme: Obtaining bundled SAN certificate
2018/11/06 09:57:35 [INFO] [*.mdbraber.net] AuthURL: https://acme-staging-v02.api.letsencrypt.org/acme/authz/hgORnAw2B-LC3e83vxLdQHz20IzV4JgOGr1R-Klqa8g
2018/11/06 09:57:35 [INFO] [mdbraber.net] AuthURL: https://acme-staging-v02.api.letsencrypt.org/acme/authz/PFU_hbvXsMqs-PykFST2AwdocOVAJzMr_hdG92u3YUs
2018/11/06 09:57:35 [INFO] [mdbraber.net] acme: Preparing to solve DNS-01
2018/11/06 09:57:38 [INFO] [mdbraber.net] acme: Preparing to solve DNS-01
2018/11/06 09:57:39 [INFO] [mdbraber.net] acme: Trying to solve DNS-01
2018/11/06 09:57:39 [INFO] [mdbraber.net] Checking DNS record propagation using [ns0.transip.nl:53]
2018/11/06 09:57:39 [INFO] Wait [timeout: 10m0s, interval: 10s]
2018/11/06 10:00:15 [INFO] [mdbraber.net] The server validated our request
2018/11/06 10:00:15 [INFO] [mdbraber.net] acme: Trying to solve DNS-01
2018/11/06 10:00:15 [INFO] [mdbraber.net] Checking DNS record propagation using [ns0.transip.nl:53]
2018/11/06 10:00:15 [INFO] Wait [timeout: 10m0s, interval: 10s]
2018/11/06 10:00:20 [INFO] [mdbraber.net] The server validated our request
2018/11/06 10:00:22 [INFO] [mdbraber.net, *.mdbraber.net] acme: Validations succeeded; requesting certificates
2018/11/06 10:00:23 [INFO] [mdbraber.net] Server responded with a certificate.

@ldez
Copy link
Member

ldez commented Nov 6, 2018

@mdbraber I do not think a deprecated constructor needs to be introduced.
Use the NewDNSProviderConfig instead.

@mdbraber
Copy link
Contributor Author

mdbraber commented Nov 6, 2018

@ldez sorry, didn't know it was deprecated - removed now (I'm seeing most of the Caddy plugins are using the old constructor, but I'll update to the new one)

@mdbraber
Copy link
Contributor Author

mdbraber commented Nov 6, 2018

@ldez I'm not sure what to do with your latest commit - should I change anything? Sorry, got it, I overlooked the second part of the file. Now wait, where's that coffee? Anything else that still needs changing?

@ldez
Copy link
Member

ldez commented Nov 6, 2018

All is good now 👍

@mdbraber
Copy link
Contributor Author

mdbraber commented Nov 6, 2018

Great - thanks for your quick review @ldez. Looking forward to the merge so this can be used in Caddy as well (if I didn't need local redirection I'd be happy to use Traefik ;-)

Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

@ldez ldez merged commit 1837a3b into go-acme:master Nov 6, 2018
@ldez ldez added this to the v2.0 milestone Dec 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants