Suggestion about NewClient about lego HOT 16 CLOSED

Why not something like ?

challenges := []acme.Challenges{acme.NewHTTPChallenge(acme.NewHTTPChallengeWebServer(optPort)), acme.NewTlsSNIChallenge()}
client, err := acme.NewClient(dirURL, user, keyBits, challenges)

from lego.

The only thing about calling New...Challenge once for a client is that a client may have multiple challenges to perform, thus needing to create new ones for each challenge -- I think (@xenolf would know better).

from lego.

I think that the challenges should definitely be internal to the library and not passed in through NewClient. The "solvers" (or however we want to call them) may be passed in as the user should have the option to add custom ones to the flow.
If a user wants to customize a built-in "solver" he may construct the solver himself, otherwise the client will add them with default values to the internal slice.

All of the above is just a braindump and I am looking for feedback :)

from lego.

I think that the challenges should definitely be internal to the library and not passed in through NewClient.

If you mean that challenges shouldn't be constructed & passed into NewClient, I agree. I still like the idea of the user being able to control which challenges are allowed, though. (Then again, maybe this can be one of the things that is set later, like the priority.)

The "solvers" (or however we want to call them) may be passed in as the user should have the option to add custom ones to the flow. If a user wants to customize a built-in "solver" he may construct the solver himself, otherwise the client will add them with default values to the internal slice.

👍

from lego.

I would like to see a way to use the acme package with both arbitrary challenges (ie a way to tweak this) as well as custom solvers for those arbitrary challenges or built-in ones.

For example, once the DNS challenge is working it would be nice to use something like this with a related solver. Similar for the SNI challenge and perhaps interacting with a PaaS api.

from lego.

Perhaps more generally: it would be neat if the acme package was a framework for interacting with the api and implementing challenges and related solvers. The lego CLI utility could wrap that up in ways that make sense for default use.

Imagine:

// satisfies some challenge + solving interface
s := &myHTTPSolver{}

// ideally the client itself doesn't care too much about directories
client := acme.NewClient(...)
// or: := &acme.Client{...} using defined fields, similar to http.Client

// client has no default solvers?
// HTTP_01 is a helpful constant for "http-01"
client.Solvers.Set(acme.HTTP_01, s)

// specify bits here, domains, etc
// ends up using my solver for http-01
result, err := client.ObtainSANCertificate(...)

from lego.

Hey @dpiddy thanks for your suggestions!

Initial support for your first point will land pretty soon when we merge #32. This gets merges as soon as we have found a good API for external solvers which will then get applied to the other challenges as well.

from lego.

What about something like this?

client := acme.NewClient(acme.Options{
   //...
})

This kind of constructor will allow us to expand client functionality without making backwards-incompatible changes.

from lego.

I ran into this one today, so I just wanted to give the thread a gentle bump :)

And some other food for thought: a lot of the current interface proposals revolve around a lot of flexibility for configuring chains of solvers which might be useful for very robust fallback strategies. I suspect though that in practice a lot of users (like myself) really just want to configure one solver and just have our clients always use that all the time. If you can imagine for a moment that you've built out a custom lego-based system internally to manage your certificates and which leverages your DNS hosted over at CloudFlare, it's probably unlikely that you'll be rebuilding it anytime soon to also support an http-01 challenge — there's just no use in doing so. So one possible alternative is just to have a constructor that takes either a single solver or nil to get a default set:

func NewClient(caDirURL string, user User, keyBits int, solver Solver) (*Client, error)

... and thus optimize for the case of a single solver. We could also provide additional functions to configure more elaborate chains if a user desires.

from lego.

Thanks everybody for your suggestions.
I want to apologize it took me so long to get to this point but now is the time ;)

The changes I have in mind are along the following:

• Have NewClient be the "default" constructor. It will set up all solvers with default configurations.
• Add a new function like acme.EmptyClient() (not sure about the name yet) which returns a client instance without any solvers configured.
• Supply a function where you can add a "Provider" to a challenge. Be it a standard provider from within the acme package or one you wrote yourself. Doing this will enable the challenge to be solved or if it is already enabled will switch out the "Provider".
• Supply a function to add a ProviderLookupFunc. This enables you to return different "Providers" depending on domain.

What I am currently unsure about is if it makes sense to have a generic "Provider" interface as providers are only usable by their respective challenges. However, the thought of four functions to add a ProviderFunc to different challenges makes me shudder :)

Looking forward to your feedback!

from lego.

I have an app that intends to use dns challenge to issue a variety of certs. Right now I have a type LookupProvider map[string]acme.DNSProvider that I have implemented the DNSProvider interface on to lookup on multiple domains. That could easily be made part of the library, but it may get tricky with things like matching a domain from the list when you have a deeply nested subdomain or things like that.

I am definitely +1 for DefaultClient and EmptyClient. For dns it makes sense to me to have a "Use this DNSProvider" function, since there is so much config that is needed before we can do anything. Things like http may have fewer options and simpler needs.

from lego.

Sounds like the right direction!

One thing to possibly consider: a "default" client is a bit tricky here because currently all solvers require some sort of configuration. The most obvious example is that dns-01 is always going to need to be tied to some kind of DNS host, but even for http-01 you're still going to need to configure a port to listen on. I believe you're proposing to solve this by allowing providing various "provider" configuration mechanisms which will allow configuration to be injected out-of-band.

An alternative might be to just require that a client's solvers are all explicitly configured. This might be a little more of a pain in some cases, but may overall provide a more clear model with fewer additional mechanisms required to work around what are essentially configuration problems (e.g. ProviderLookupFunc).

I don't have a strong opinion on it though. Looking forward to seeing this solved!

from lego.

The thing about http-01 and tls-sni-01 challenges is that their default configurations can work out-of-the-box by using ports 80 and 443, respectively. In contrast, the dns-01 challenge doesn't have a "default" set of credentials or anything that we can just kind of assume unless told otherwise.

As much as I like being explicit, configuring those for those challenges' default values seems a bit redundant. It also becomes less obvious that if you don't use the default port, you have to forward it to your alternate port. Whereas, if I override a default setting, I am aware that I'm possibly breaking something.

from lego.

Maybe something along this line:

Creating a new client:

// Using default set of solvers
solvers = lego.Solvers{}

// Using custom set of solvers and providers
solvers = lego.Solvers{"http01": "standalone", "dns01": "cloudflare"}

// Using default provider options
providersOpts = lego.ProvidersOpts{}

// Overriding defaults and/or set required options
providersOpts = lego.ProvidersOpts{"HttpPort": 8080, "CloudFlareApiKey": "123456"}

client, err := acme.NewClient(dirURL, user, keyBits, solvers, providersOpts)

Each solver has a map of all its providers and implements a SetProvider method for creating and attaching the chosen provider:

dnsProviders := map[string]interface{}{
    "cloudflare": newCloudFlareProvider,
    "route53": newRoute53Provider,
}

func (s *dnsChallenge) SetProvider(name string, providersOpts ProvidersOpts) error {
    if f, ok := dnsProviders[name]; ok {
        provider, err := f.(func(ProvidersOpts))(providersOpts)
        if err != nil {
            return err
        }
        s.Provider = provider
        return nil
    }
    return fmt.Errorf("No such provider: %s", name)
}

Each provider has a function that returns a new instance using the ProvidersOpts:

func newCloudflareProvider(providersOpts ProvidersOpts) Provider, error {
    if len(providersOpts.CloudFlareApiKey) == 0 {
        return nil, fmt.Errorf("CloudFlareApiKey not set")
    }
    ...
}

from lego.

FYI: based on the refactoring in #76, you can now specify a custom provider like this

client := acme.NewClient(....)
cf, err := acme.NewDNSProviderCloudFlare(...)
client.SetChallengeProvider(acme.DNS01, cf)
client.ObtainCertificate(....)

from lego.

Yeah, I think between that refactoring and the ExcludeChallenges() and Set...Address() functions, this issue is now resolved. Thanks for all the feedback and contributions!

from lego.