Add NearlyFreeSpeech DNS Provider #1652

KittyKatt · 2022-05-27T22:45:05Z

I can test whatever I need for this and show the output of whatever command. You have to have a real domain and account to try this and I'm not sure any lego devs have an NFS account.

This is my first foray into a bigger Go project and used pretty much any best practices tools, so let me know if I've done anything wrong and I'll do my best to correct it!

ldez · 2022-05-27T23:03:53Z

Hello, in order for a PR adding a DNS provider to be accepted, you have to:

add a description to your PR
be able to maintain this provider
have a homogeneous design with the other providers
add tests (units)

make test

add tests ("live")

lego/providers/dns/cloudflare/cloudflare_test.go

Lines 125 to 151 in c938de6

    
           func TestLivePresent(t *testing.T) { 
        
           	if !envTest.IsLiveTest() { 
        
           		t.Skip("skipping live test") 
        
           	} 
        
           	envTest.RestoreEnv() 
        
           	provider, err := NewDNSProvider() 
        
           	require.NoError(t, err) 
        
           	err = provider.Present(envTest.GetDomain(), "", "123d==") 
        
           	require.NoError(t, err) 
        
           } 
        
           func TestLiveCleanUp(t *testing.T) { 
        
           	if !envTest.IsLiveTest() { 
        
           		t.Skip("skipping live test") 
        
           	} 
        
           	envTest.RestoreEnv() 
        
           	provider, err := NewDNSProvider() 
        
           	require.NoError(t, err) 
        
           	time.Sleep(2 * time.Second) 
        
           	err = provider.CleanUp(envTest.GetDomain(), "", "123d==") 
        
           	require.NoError(t, err) 
        
           }

make test

add a provider descriptor
generate CLI help, documentation, and readme.

make generate-dns

be able to do: (and put the output of this command to a comment in your PR)

rm -rf .lego

./lego -m your@email.com --dns YOUR_PROVIDER_NAME -d *.example.com -d example.com -s https://acme-staging-v02.api.letsencrypt.org/directory run

Note the wildcard domain is important.

pass the linter (golangci-lint must be installed):

make checks

do go mod tidy

providers/dns/nearlyfreespeech/nearlyfreespeech.toml

KittyKatt · 2022-05-28T00:24:18Z

* [ ]  have a homogeneous design with the other providers

I'm uncertain what good looks like here. Could you elaborate?

ldez · 2022-05-28T00:29:51Z

Hello,

your code is too inspired by other languages and it's too complex, as you are a Go newbie I will try to focus on some points.

Don't do:

req := &http.Request{
		Method: "POST",
		URL: &url.URL{
			Scheme: "https",
			Host:   "example.com",
			Path:   path,
		},
		Header: hdr,
		Body:   io.NopCloser(bytes.NewReader(encBody)),
	}

Do:

http.NewRequest(http.MethodPost, "https://example.com/foo", strings.NewReader(payload))

Don't do:

args := url.Values{
		"name": []string{rr.Name},
		"type": []string{rr.Type},
		"data": []string{rr.Data},
	}

Do:

params := url.Values{}
params.Set("name", rr.Name)
params.Set("type", rr.Type)
params.Set("data", rr.Data)

Don't do:

"/dns/"+url.PathEscape(domain)+"/addRR"

Do:

path.Join("dns", domain, "addRR")

Avoid:

http.DefaultClient

Because it's a global mutable variable.

Prefer:

&http.Client{Timeout: 5*time.Second}

There is too much to change for me to do a simple review (I don't have the time to do a detailed review, I'm sorry).
So I corrected the code in a commit, I invite you to read in detail this commit and the changes it contains.
If you want some explanation, don't hesitate to ask.

Now I'm waiting for this point:

be able to do: (and put the output of this command to a comment in your PR)
rm -rf .lego

./lego -m your@email.com --dns YOUR_PROVIDER_NAME -d *.example.com -d example.com -s https://acme-staging-v02.api.letsencrypt.org/directory run
Note the wildcard domain is important.

ldez · 2022-05-28T00:54:40Z

I see that you read some PR comments before opening your PR and I see you tried to "copy" the freemyip provider.
These are good points.

What I can recommend to you:

open an issue before creating a PR (we have a template for that, this template can help you and us)
improve your "copy" skills: we have more than 100 providers and they are mainly built with the same design.
don't forget to remove the reference to your copy source 😉
Go is a simple language: the API is simple and well documented. I recommend reading some code and docs.

KittyKatt · 2022-05-28T01:04:03Z

I absolutely was not intending to copy maliciously or sneakily. I saw the "homogeneous" comment and thought I'd try to find a provider that looked similar to what I wanted. I ended up reading through freemyip and it looked as though it would fit initially. I feel bad I left a reference in there because that's sloppy work. I actually ended up looking through mythicbeast client and implementation and reusing a ton of their code because that was pretty close in request logic from what I could tell.

I really, really appreciate the feedback. I'm by NO means a Go developer, I work in DevOps by trade. I'm just a girl using NFS and tired of running the manual provider lol. I also wanted to treat this as a good exercise of learning by doing.

I'm working on getting back to a computer to run that command.

ldez · 2022-05-28T01:15:56Z

I absolutely was not intending to copy maliciously or sneakily.

I didn't mean to imply that at all, it was just a joke 😄.

Take the time to read my changes and feel free to ask questions.

I'm always happy when someone takes the time to implement a provider rather than just asking (or even requiring in some cases) that the maintainers do it.

Your code is complex, but it's expected as you are a Go newbie, so there is absolutely no problem with that.

Also, I rewrite your code with a purely technical approach: I didn't read the doc, I just read your code, extract the meaning, and tried to simplify and apply idiomatic Go patterns.
So maybe there are some bugs because I did that a bit quickly.

KittyKatt · 2022-05-28T01:21:35Z

100000% appreciate literally any time or effort you put into it. Everything is a learning opportunity.

I like to contribute to things I use, only makes sense that if something is provided for free and I can give back I should.

KittyKatt · 2022-05-28T01:28:29Z

Pulled your changes and something seems to be broken. I'll get on this in just a bit.

providers/dns/nearlyfreespeech/nearlyfreespeech.go

KittyKatt · 2022-05-28T02:59:20Z

That fixed it! I definitely should have seen that UnFqdn function, I literally had that file open looking at ToFqdn. D'oh.

Sanitized my API username, API key, and email address.

$ rm -rf .lego
$ NEARLYFREESPEECH_API_KEY=xx NEARLYFREESPEECH_LOGIN=xx ./dist/lego -m user@example.com --dns nearlyfreespeech -d '*.nfsdns.kittykatt.co' -d 'nfsdns.kittykatt.co' -s https://acme-staging-v02.api.letsencrypt.org/directory run
2022/05/27 21:53:30 No key found for account user@example.com. Generating a P256 key.
2022/05/27 21:53:30 Saved key to /home/katie/Repositories/projects/lego/.lego/accounts/acme-staging-v02.api.letsencrypt.org/user@example.com/keys/user@example.com.key
2022/05/27 21:53:31 Please review the TOS at https://letsencrypt.org/documents/LE-SA-v1.2-November-15-2017.pdf
Do you accept the TOS? Y/n
y
2022/05/27 21:53:33 [INFO] acme: Registering account for user@example.com
!!!! HEADS UP !!!!

Your account credentials have been saved in your Let's Encrypt
configuration directory at "/home/katie/Repositories/projects/lego/.lego/accounts".

You should make a secure backup of this folder now. This
configuration directory will also contain certificates and
private keys obtained from Let's Encrypt so making regular
backups of this folder is ideal.
2022/05/27 21:53:33 [INFO] [*.nfsdns.kittykatt.co, nfsdns.kittykatt.co] acme: Obtaining bundled SAN certificate
2022/05/27 21:53:34 [INFO] [*.nfsdns.kittykatt.co] AuthURL: https://acme-staging-v02.api.letsencrypt.org/acme/authz-v3/2556325564
2022/05/27 21:53:34 [INFO] [nfsdns.kittykatt.co] AuthURL: https://acme-staging-v02.api.letsencrypt.org/acme/authz-v3/2556325574
2022/05/27 21:53:34 [INFO] [*.nfsdns.kittykatt.co] acme: use dns-01 solver
2022/05/27 21:53:34 [INFO] [nfsdns.kittykatt.co] acme: Could not find solver for: tls-alpn-01
2022/05/27 21:53:34 [INFO] [nfsdns.kittykatt.co] acme: Could not find solver for: http-01
2022/05/27 21:53:34 [INFO] [nfsdns.kittykatt.co] acme: use dns-01 solver
2022/05/27 21:53:34 [INFO] [*.nfsdns.kittykatt.co] acme: Preparing to solve DNS-01
2022/05/27 21:53:36 [INFO] [*.nfsdns.kittykatt.co] acme: Trying to solve DNS-01
2022/05/27 21:53:36 [INFO] [*.nfsdns.kittykatt.co] acme: Checking DNS record propagation using [127.0.0.53:53]
2022/05/27 21:53:38 [INFO] Wait for propagation [timeout: 1m0s, interval: 2s]
2022/05/27 21:53:53 [INFO] [*.nfsdns.kittykatt.co] The server validated our request
2022/05/27 21:53:53 [INFO] [*.nfsdns.kittykatt.co] acme: Cleaning DNS-01 challenge
2022/05/27 21:53:56 [INFO] sequence: wait for 1m0s
2022/05/27 21:54:56 [INFO] [nfsdns.kittykatt.co] acme: Preparing to solve DNS-01
2022/05/27 21:54:59 [INFO] [nfsdns.kittykatt.co] acme: Trying to solve DNS-01
2022/05/27 21:54:59 [INFO] [nfsdns.kittykatt.co] acme: Checking DNS record propagation using [127.0.0.53:53]
2022/05/27 21:55:01 [INFO] Wait for propagation [timeout: 1m0s, interval: 2s]
2022/05/27 21:55:16 [INFO] [nfsdns.kittykatt.co] The server validated our request
2022/05/27 21:55:16 [INFO] [nfsdns.kittykatt.co] acme: Cleaning DNS-01 challenge
2022/05/27 21:55:19 [INFO] [*.nfsdns.kittykatt.co, nfsdns.kittykatt.co] acme: Validations succeeded; requesting certificates
2022/05/27 21:55:19 [INFO] [*.nfsdns.kittykatt.co] Server responded with a certificate.

KittyKatt · 2022-05-28T14:04:09Z

Rebased off of upstream/master

Co-authored-by: Ludovic Fernandez <ldez@users.noreply.github.com>

ldez · 2022-05-28T14:06:00Z

You dropped my last commit, I will push force on your branch.

KittyKatt · 2022-05-28T14:07:15Z

You dropped my last commit, I will push force on your branch.

Whoops, thanks! I absolutely am usually more with it than that. Never rebase before coffee, folks.

ldez

LGTM

KittyKatt · 2022-06-09T17:39:03Z

Thank you!

ldez added enhancement area/dnsprovider labels May 27, 2022

ldez self-requested a review May 27, 2022 23:09

ldez requested changes May 27, 2022

View reviewed changes

providers/dns/nearlyfreespeech/nearlyfreespeech.toml Outdated Show resolved Hide resolved

providers/dns/nearlyfreespeech/nearlyfreespeech.toml Outdated Show resolved Hide resolved

ldez added the contrib/waiting-for-feedback label May 28, 2022

KittyKatt commented May 28, 2022

View reviewed changes

providers/dns/nearlyfreespeech/nearlyfreespeech.go Show resolved Hide resolved

ldez removed the contrib/waiting-for-feedback label May 28, 2022

ldez added this to the v4.8 milestone May 28, 2022

KittyKatt force-pushed the feat/nearlyfreespeech branch from 38dbe3e to 12399a4 Compare May 28, 2022 14:03

KittyKatt and others added 6 commits May 28, 2022 16:04

feat: nearlyfreespeech dns01 solver

e3c71df

Update providers/dns/nearlyfreespeech/nearlyfreespeech.toml

f9ce45d

Co-authored-by: Ludovic Fernandez <ldez@users.noreply.github.com>

review

d5348ff

review

d8658ee

review: unfqdn

ac1bb47

review: handle API errors

910f52b

ldez force-pushed the feat/nearlyfreespeech branch from 12399a4 to 910f52b Compare May 28, 2022 14:06

review: brand name

e4c8a39

ldez approved these changes Jun 6, 2022

View reviewed changes

ldez merged commit 638745c into go-acme:master Jun 6, 2022

ldez mentioned this pull request Aug 19, 2023

nearlyfreespeech: always fails with "The authentication hash was not correct" #1998

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add NearlyFreeSpeech DNS Provider #1652

Add NearlyFreeSpeech DNS Provider #1652

KittyKatt commented May 27, 2022 •

edited by ldez

Loading

ldez commented May 27, 2022 •

edited

Loading

KittyKatt commented May 28, 2022

ldez commented May 28, 2022 •

edited

Loading

ldez commented May 28, 2022

KittyKatt commented May 28, 2022 •

edited

Loading

ldez commented May 28, 2022 •

edited

Loading

KittyKatt commented May 28, 2022

KittyKatt commented May 28, 2022

KittyKatt commented May 28, 2022 •

edited by ldez

Loading

KittyKatt commented May 28, 2022

ldez commented May 28, 2022

KittyKatt commented May 28, 2022

ldez left a comment

KittyKatt commented Jun 9, 2022

Add NearlyFreeSpeech DNS Provider #1652

Add NearlyFreeSpeech DNS Provider #1652

Conversation

KittyKatt commented May 27, 2022 • edited by ldez Loading

ldez commented May 27, 2022 • edited Loading

KittyKatt commented May 28, 2022

ldez commented May 28, 2022 • edited Loading

ldez commented May 28, 2022

KittyKatt commented May 28, 2022 • edited Loading

ldez commented May 28, 2022 • edited Loading

KittyKatt commented May 28, 2022

KittyKatt commented May 28, 2022

KittyKatt commented May 28, 2022 • edited by ldez Loading

KittyKatt commented May 28, 2022

ldez commented May 28, 2022

KittyKatt commented May 28, 2022

ldez left a comment

Choose a reason for hiding this comment

KittyKatt commented Jun 9, 2022

KittyKatt commented May 27, 2022 •

edited by ldez

Loading

ldez commented May 27, 2022 •

edited

Loading

ldez commented May 28, 2022 •

edited

Loading

KittyKatt commented May 28, 2022 •

edited

Loading

ldez commented May 28, 2022 •

edited

Loading

KittyKatt commented May 28, 2022 •

edited by ldez

Loading