Skip to content

Conversation

CPlusPlus17
Copy link

No description provided.

acme/client.go Outdated
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?

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