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 support for autodns #957

Merged
merged 38 commits into from Nov 1, 2019
Merged

Add support for autodns #957

merged 38 commits into from Nov 1, 2019

Conversation

kolaente
Copy link
Contributor

@kolaente kolaente commented Aug 30, 2019

This pr adds support for autodns based on https://help.internetx.com/display/APIJSONEN

Closes #501

Checklist

  • add a description to your PR
  • be able to maintain this provider
  • have a homogeneous design with the other providers
  • add tests (units)
  • add tests ("live") (i guess? Please correct me if I'm wrong)
  • add the provider to the readme.md
  • add a provider descriptor
  • generate CLI help and documentation
  • pass the linter (golangci-lint must be installed)
  • do go mod tidy
  • 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.

@kolaente kolaente mentioned this pull request Aug 30, 2019
cmd/zz_gen_cmd_dnshelp.go Show resolved Hide resolved
providers/dns/autodns/autodns.go Outdated Show resolved Hide resolved
@ldez
Copy link
Member

ldez commented Aug 30, 2019

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")
    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
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.

make checks
  • do go mod tidy

@kolaente
Copy link
Contributor Author

kolaente commented Sep 2, 2019

@ldez I updated the original description of the pr with your checklist, in order to be able to edit it.

@kolaente
Copy link
Contributor Author

kolaente commented Sep 2, 2019

I also go a response about the _stream api endpoint from the autodns guys, they could reproduce it and are investigating why it did not work - they'll get back to me, I'll update here.

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.

Could you add:

// Timeout returns the timeout and interval to use when checking for DNS propagation.
// Adjusting here to cope with spikes in propagation times.
func (d *DNSProvider) Timeout() (timeout, interval time.Duration) {
return d.config.PropagationTimeout, d.config.PollingInterval
}

Could you add more unit tests:

func TestNewDNSProvider(t *testing.T) {
testCases := []struct {
desc string
envVars map[string]string
expected string
}{
{
desc: "success",
envVars: map[string]string{
"AURORA_USER_ID": "123",
"AURORA_KEY": "456",
},
},
{
desc: "missing credentials",
envVars: map[string]string{
"AURORA_USER_ID": "",
"AURORA_KEY": "",
},
expected: "aurora: some credentials information are missing: AURORA_USER_ID,AURORA_KEY",
},
{
desc: "missing user id",
envVars: map[string]string{
"AURORA_USER_ID": "",
"AURORA_KEY": "456",
},
expected: "aurora: some credentials information are missing: AURORA_USER_ID",
},
{
desc: "missing key",
envVars: map[string]string{
"AURORA_USER_ID": "123",
"AURORA_KEY": "",
},
expected: "aurora: some credentials information are missing: AURORA_KEY",
},
}
for _, test := range testCases {
t.Run(test.desc, func(t *testing.T) {
defer envTest.RestoreEnv()
envTest.ClearEnv()
envTest.Apply(test.envVars)
p, err := NewDNSProvider()
if len(test.expected) == 0 {
require.NoError(t, err)
require.NotNil(t, p)
require.NotNil(t, p.config)
require.NotNil(t, p.client)
} else {
require.EqualError(t, err, test.expected)
}
})
}
}
func TestNewDNSProviderConfig(t *testing.T) {
testCases := []struct {
desc string
userID string
key string
expected string
}{
{
desc: "success",
userID: "123",
key: "456",
},
{
desc: "missing credentials",
userID: "",
key: "",
expected: "aurora: some credentials information are missing",
},
{
desc: "missing user id",
userID: "",
key: "456",
expected: "aurora: some credentials information are missing",
},
{
desc: "missing key",
userID: "123",
key: "",
expected: "aurora: some credentials information are missing",
},
}
for _, test := range testCases {
t.Run(test.desc, func(t *testing.T) {
config := NewDefaultConfig()
config.UserID = test.userID
config.Key = test.key
p, err := NewDNSProviderConfig(config)
if len(test.expected) == 0 {
require.NoError(t, err)
require.NotNil(t, p)
require.NotNil(t, p.config)
require.NotNil(t, p.client)
} else {
require.EqualError(t, err, test.expected)
}
})
}
}

providers/dns/autodns/autodns.toml Outdated Show resolved Hide resolved
providers/dns/autodns/autodns.toml Show resolved Hide resolved
providers/dns/autodns/autodns.go Outdated Show resolved Hide resolved
providers/dns/autodns/autodns.go Outdated Show resolved Hide resolved
@kolaente
Copy link
Contributor Author

kolaente commented Sep 2, 2019

@ldez When implementing Timeout(), should I make it configurable?

Edit: Timeout is now implemented, configurable.

@kolaente
Copy link
Contributor Author

CI passes now.

@kolaente
Copy link
Contributor Author

@ldez What else needs to be done to get this PR merged?

@ldez ldez changed the title WIP: Add support for autodns Add support for autodns Oct 25, 2019
@ldez
Copy link
Member

ldez commented Oct 25, 2019

Could you run:

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

Note, there are 2 domains -d *.example.com -d example.com

@kolaente
Copy link
Contributor Author

@ldez I've updated the comment.

@ldez
Copy link
Member

ldez commented Oct 30, 2019

I'm annoyed because there is a bug with GitHub (I contacted them on this topic) that prevents me from editing your PR because your fork is in an organization.

So there are 3 solutions:

  • you give the rights on your repository (I do not feel very comfortable with this because it's a bit extreme as a solution)
  • I open a PR on your fork to edit your code. (a PR on PR, it's a bit weird)
  • you open a new PR by using a fork in your account. (I dislike this solution but it's a solution)

What do you prefer?

@kolaente
Copy link
Contributor Author

kolaente commented Oct 30, 2019

@ldez what do you need the rights for?

I think none of these solutions are ideal, maybe the best is to open a PR to the fork.

@ldez
Copy link
Member

ldez commented Oct 30, 2019

I need the rights to apply some review fixes.

I opened a PR iMi-digital#1

@kolaente
Copy link
Contributor Author

@ldez Merged.

@ldez ldez added this to the v3.2 milestone Oct 31, 2019
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.

Thank you 👍

@ldez
Copy link
Member

ldez commented Oct 31, 2019

could you solve this:

golangci-lint run
providers/dns/autodns/client.go:16:2: `demoEndpoint` is unused (varcheck)
	demoEndpoint    = "https://api.demo.autodns.com/v1/"
	^
Makefile:39: recipe for target 'checks' failed

@kolaente
Copy link
Contributor Author

kolaente commented Nov 1, 2019

@ldez Done.

@ldez ldez merged commit 46680f6 into go-acme:master Nov 1, 2019
@kolaente kolaente deleted the feature/autodns branch November 1, 2019 12:57
@kolaente
Copy link
Contributor Author

kolaente commented Nov 4, 2019

@ldez How do we proceed to get this in traefik? Should I send a pr after you released a new version of lego?

@ldez
Copy link
Member

ldez commented Nov 4, 2019

yes you have to wait for the next lego release.

@rmoriz

This comment has been minimized.

@kolaente
Copy link
Contributor Author

@rmoriz Could you open an issue?

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.

AutoDNS API Support?
4 participants