Add DNS Provider for GoDaddy

[smerschjohann]

This PR adds support for the DNS Provider: GoDaddy

As the GoDaddy does not allow to safely delete records, the cleanup method will override the record data with "null" instead.

[sjawhar]

Why this check? I'm new to lego, but I don't see it in the other provider modules.

[smerschjohann]

GoDaddy does not allow ttls lower than 600. Therefore i set it to its minimum.

[sjawhar]

I was just getting ready to write this myself! Thanks, @smerschjohann!

[sjawhar]

The problem of GoDaddy's API not having a DELETE method for records (WHY, GODADDY!?) has been documented by other GoDaddy API clients. This python client dealt with it by GETting all records, filtering out those of specified names and types, then PUTting them back. Since we're always dealing with specific type (TXT) and names, maybe we can PUT an empty list to /v1/domains/{domain}/records/TXT/{name} to clear the record, instead of changing the record to Data: "null"?

[sjawhar]

I just tried my idea. Doesn't look like it will work. GoDaddy responds with an error if you PUT an empty list. I think your approach is the best way about it.

Another idea would be to GET /v1/domains/{domain}/records/TXT, remove the entry from the list with name {name}, and then PUT the reduced list back. If the list is empty, we'd have to add a dummy entry so GoDaddy will accept the request. The benefit is that there would only be at most one dummy record in there instead of a "null" record for each name. The downside is that it would be a data race if there are multiple requests at the same time.

The only downside to leaving "null" TXT records is if you consider the name of your internal subdomains to be sensitive.

[smerschjohann]

I tried it with this workaround (removing the added TXT records from the list), but it is unrealiable. Some users might have important TXT records which should never be deleted, this cannot be guaranteed by this method.

The only downside to leaving "null" TXT records is if you consider the name of your internal subdomains to be sensitive.

A list of every certificate signed by letsencrypt is publicly available because of the transparency logs which can be searched at crt.sh, so this is not an issue.

I think, there currently is no better way, than the implemented solution.

[sjawhar]

Agreed. Until this is merged in, I'll be using your fork in a custom build so I can get my company's Traefik instances using Let's Encrypt. Thanks again!

[sjawhar]

Think you can merge in the latest from master?

[sjawhar]

@smerschjohann After running this, I saw that the Let's Encrypt verification fails because multiple TXT records are being created for each name. I updated it to PUT instead of PATCH, resulting in only one TXT record per name. You can see my changes here; smerschjohann#1

[smerschjohann]

I picked up your patch, but I think we should not remove all TXT records blindly. I will look into it soon.

[sjawhar]

It doesn't replace all TXT records. The API takes optional /type/name parameters. So it replaces all TXT records with the name of the ACME challenge provided, so that there is only one record for each domain being verified.

https://developer.godaddy.com/doc#!/_v1_domains/recordReplaceTypeName

[smerschjohann]

ah, I see. I have overseen that, sorry.

[sjawhar]

No worries, thanks again for doing most of the work! I got this running with my changes last week at work on our development server, works great.

[xenolf]

Is this complete functionality wise? I couldn't completely follow your conversation 😄

[sjawhar]

I believe it is, yes.

[Noah-Heil]

When I attempt to use this it tells me that there is no godaddy provider. What am I doing wrong?

[sjawhar]

@Noah-Heil hmm, not sure. Are you on the godaddy branch of smerschjohann/lego?

[Noah-Heil]

This is the error that I am getting:

2017/09/27 15:07:37 Unrecognised DNS provider: godaddy

I am doing go build and then running it using ./lego

[sjawhar]

@Noah-Heil I just did the following in a fresh alpine:3.6 docker image (mostly copied from Dockerfile)

export GOPATH=/go

apk update && apk add --no-cache --virtual run-dependencies ca-certificates
apk add --no-cache --virtual build-dependencies go git musl-dev
go get -u github.com/smerschjohann/lego
cd ${GOPATH}/src/github.com/
rm -rf xenolf
mv smerschjohann xenolf
cd xenolf/lego
git checkout godaddy
go build -o /usr/bin/lego .

/usr/bin/lego --email="foo@bar.com" --domains="example.com" --dns="godaddy" run

Result

That "GoDaddy credentials missing" message is inside the GoDaddy dns provider module, so it was successfully loaded and built.

[Noah-Heil]

Thank you. I needed to delete the xenolf/ directory first and replace it with the smerschjohann for everything to work as expected.
Now that I have done that everything is working great.

Thanks a bunch!

[sjawhar]

@smerschjohann I was just thinking that we might need a check in updateRecords() that makes sure recordName isn't null or empty or anything. Just to be double-sure.

[smerschjohann]

I think this is not really needed. @xenolf I think this is ready to be merged