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

Added multiple DNS provider support #331

Closed
wants to merge 5 commits into from
Closed

Added multiple DNS provider support #331

wants to merge 5 commits into from

Conversation

CPlusPlus17
Copy link

No description provided.

default:
return fmt.Errorf("Unknown challenge %v", challenge)
}
return nil
}

func (c *Client) SetChallengeProviderDNS(challenge Challenge, p []ChallengeProvider) error {
Copy link
Member

Choose a reason for hiding this comment

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

Please add a comment to exported functions. Also use plural as we're possibly setting multiple providers?

}

func (s *dnsChallenge) Solve(chlng challenge, domain string) error {
logf("[INFO][%s] acme: Trying to solve DNS-01", domain)

if s.provider == nil {
if s.providers == nil {
Copy link
Member

Choose a reason for hiding this comment

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

Don't check for nil here, a slice will not be nil but have a len() of zero.

if err != nil {
return fmt.Errorf("Error presenting token: %s", err)
for _, v := range s.providers {
err = v.Present(domain, chlng.Token, keyAuth)
Copy link
Member

Choose a reason for hiding this comment

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

This would try to Present() every domain on every provider. Shouldn't we have a way to tell which provider a certain domain uses?

Copy link
Author

Choose a reason for hiding this comment

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

Ah I didn't thought about this use case. But normally you have one or more DNS provider per Domain. I designed the function that way. This means if you want to validate something.url.com, and you have two DNS providers, you can add both to set the right TXT record. With the old system, it was not possible to know which DNS provider will serve the answer and validation could be invalid. I'm not sure if we want to provide a mapping per domain and DNS provider. Actually I don't see such a use case.

Copy link
Member

Choose a reason for hiding this comment

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

@ManuelGysin I see where you are coming from. But I also see the use case where people want to add domains to a SAN certificate which are hosted by different DNS providers.

@ldez
Copy link
Member

ldez commented Dec 31, 2018

Closed in favor of #605.

Thanks for the contribution.

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.

3 participants