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

Move DNS providers out of the lego executable #111

Closed
willglynn opened this issue Feb 8, 2016 · 12 comments
Closed

Move DNS providers out of the lego executable #111

willglynn opened this issue Feb 8, 2016 · 12 comments

Comments

@willglynn
Copy link
Contributor

lego could grow indefinitely by supporting every DNS provider API ever made, but this seems less than ideal (#100). I'd like to discuss how to move DNS providers out of the lego executable.

It is fortunate that DNS-01 has a very small surface area in terms of what functions are required. There's basically just two operations:

  1. Create the record _acme-challenge.whatever IN TXT "abc123"
  2. Clean up

lego can monitor the authoritative DNS servers to watch and wait for success, so communication from the DNS provider back to lego can be as little as just signaling failure.

Go limits the mechanisms by which external code can interact with a project like lego. The most straightforward mechanism is to launch a child process and interact with it, and based on my understanding of the requirements, I think that model would work fine:

  1. lego does everything ACME-related, ultimately deciding that it needs to create _acme-challenge.whatever IN TXT "abc123".
  2. lego, having been invoked with --dns=foo, spawns lego-dns-foo "_acme-challenge.whatever" "abc123", passing on its environment variables.
  3. lego-dns-foo uses FOO_USERNAME and FOO_PASSWORD to create the record indicated by its arguments, then waits.
  4. lego spins and waits for a) _acme-challenge.whatever to show the correct value, b) lego-dns-foo to exit, or c) some configurable timeout.
  5. lego sees _acme-challenge.whatever get created and proceeds in its ACME dance, yielding a completed challenge and a certificate for whatever.
  6. lego no longer needs _acme-challenge.whatever, and indicates this by closing lego-dns-foo's stdin.
  7. lego-dns-foo sees its stdin close, removes the record, and terminates successfully.

This scheme avoids unnecessary complexity (it's simple enough that a provider could be implemented with a shell script) and platform dependence (i.e. signals don't really port well to Windows). lego-dns-foo could communicate back by printing to stdout/stderr and by exiting with a failure code, which seems like it's enough.

Some negatives of this approach are:

  • lego-dns-foo doesn't get run until late in the operation, which means DNS-related configuration errors aren't detected until late in the operation. This could be mitigated by launching lego-dns-foo with no arguments before doing anything else (i.e. "don't actually create any records, but do fail immediately if you would fail immediately"), or by adding a separate dns-test function which creates a TXT record and verifies its content outside an ACME exchange (lego dns-test feature request #110).
  • go get github.com/xenolf/lego becomes insufficient. (That's sort of the point, though.) Would it be okay to make users say go get github.com/xenolf/lego-dns-foo? Do we want users to have a kitchen sink option?

Thoughts?

@willglynn willglynn changed the title Moving DNS providers out of the lego executable Move DNS providers out of the lego executable Feb 8, 2016
@mholt
Copy link
Contributor

mholt commented Feb 8, 2016

I wonder how many DNS providers really require a whole library to be imported in order to set and delete TXT records. For example, when I implemented DigitalOcean's DNS solver I simply used net/http.

If we can strip down the code of each provider to just using standard library functions, then I'm OK with adding more providers as long as they go in their own package.

But if each provider is going to add another tree of dependencies, then... I'd rather avoid that.

@xi2
Copy link
Contributor

xi2 commented Feb 8, 2016

I agree with @mholt, that stripping down the code might be the way forward. I started to write a challenge solver for Gandi API before realizing I could make http-01 work with the Stdlib reverse proxy. However, I got far enough to realize that the (huge) Gandi Go package was not necessary and I could just marshall/unmarshall the XML myself quite easily, since as you say it's only a couple of functions needed.

If anyone has a desperate need for a Gandi solver I can finish off what I did. Just leave a comment here. But as I said, I no longer need it just now.

@willglynn
Copy link
Contributor Author

I agree that it's possible to implement many providers in terms of net/http and standard library crypto. On the other hand, take where we are now as a baseline: lego currently uses libraries for AWS Route53, CloudFlare, and DNSimple, so it needs ongoing work to track changes in those libraries. Removing them would mean re-implementing the relevant parts of those libraries, and the ongoing work switches from tracking library changes to tracking protocol changes.

I think it's pragmatic to re-use existing libraries, though admittedly the scale of the AWS client library colors my thinking. (I use both ~/.aws/credentials and credentials from EC2 instance metadata, hence #102.) Obviously using libraries brings undesirable dependency baggage, hence this proposal to externalize DNS challenge providers instead of rewriting minimal versions of them.

@willglynn
Copy link
Contributor Author

Hmm… maybe it makes sense to use both strategies? Keep zero-dependency providers internal, and spin off e.g. lego-dns-route53?

@xi2
Copy link
Contributor

xi2 commented Feb 8, 2016

That is a fair point you make @willglynn that tracking libaries is easier than tracking protocols. Especially if the person who writes the solver disappears. Each solver will likely have a different author and they may not stick around to maintain it. But I guess it comes down to balance and a sort of cost/benefit analysis as to whether to use the lib or not.

@xi2
Copy link
Contributor

xi2 commented Feb 8, 2016

Thinking about this some more, maybe this would work:

Have each DNS solver in a seperate package, say under:

github.com/xenolf/lego/acme/dns-XXXX

Keep the plain acme package just for the non-dns solvers

An API user would then just import whichever solver they want, and get the bloat they want.

For the CLI app, lego, we could have the default go get action build it with no DNS solver and be nice and light. Then have a build tag, eg go get -tags dnssolvers build it with every dns solver built in, no matter how huge. That would be quite straight forward. On the github download page we could have both precompiled binaries available ("fat" and "thin"), or maybe just "fat" would do.

@willglynn
Copy link
Contributor Author

I've been tinkering with moving DNS providers out of package acme, and I have this in a branch right now. (All the providers are still in a single package at the moment, but it need not stay that way.) There's a registry which lets cli.go ask "which providers do I have?" and to instantiate them as directed. Each provider starts with code like:

func init() {
    Registry.addProvider("digitalocean", "DO_AUTH_TOKEN", func() (acme.ChallengeProvider, error) {
        return NewDNSProviderDigitalOcean(os.Getenv("DO_AUTH_TOKEN"))
    })
}

This model would suit conditional compilation just fine.

I should also mention an additional motivation for my original proposal: I have a use case for an external DNS challenge provider.

One of my clients has a bizarre environment that I expect no one else will ever encounter. I could plop a provider in lego/acme/dns_challenge_bizarre.go, but I wouldn't open a PR because it plainly doesn't belong upstream. Internal-only means I need to maintain a local fork of lego, but supporting external providers means I could use my local --dns=bizarre with a normal, unmodified lego build.

(Besides being simple, the child process approach also means I could write lego-dns-bizarre in a different language, which would be another win in this particular case.)

If we're leaning towards keeping lego's usual providers self-contained (though possibly not compiled in by default), this particular itch could be scratched separately by adding a --dns=external provider that fork/exec's a child process.

@xenolf
Copy link
Member

xenolf commented Feb 9, 2016

I'm sorry that I'm a bit late to the party but I had to think about this.

As I said in #100 I'm in favor of moving the DNS providers apart from the manual provider out of the ACME package to lessen the dependency burden. The manual provider should remain in the package as I think every challenge should be solvable by just importing the package.

Concerning the other providers, I think it makes sense to put them into their own packages to allow people to only import what they need... Using the DO provider should not mean to import the huge AWS library for example.

I imagine the folder structure as follows:
lego/acme/providers/dns/cloudflare.go

This gives us the freedom to add providers for the other challenges as well. (Some have already been requested like a manual provider for HTTP-01)

For the CLI the best method would probably be to support build tags in order to allow people to customize their build by using go get.

As far as "plugins" to the CLI go I'm torn back and forth. I intended the CLI to be simple and only a thin wrapper around the library and this would kinda break it.

Looking forward to your thoughts.

@xi2
Copy link
Contributor

xi2 commented Feb 9, 2016

That is a good point about keeping the manual DNS provider within the core acme package, and within whatever the "basic" build of the CLI is.

@willglynn
Copy link
Contributor Author

I took a stab at some reshuffling in #112.

I put the DNS providers in lego/dns_providers, rather than lego/acme/dns_providers, because there really isn't anything ACME-specific about them. The DNS challenge providers themselves are bits of code that can create and destroy TXT records, but they're totally oblivious to the details.

greping that branch, the only reason providers import the acme package at all is so acme.DNS01Record() can tell them the details of the TXT records they're intended to create. I'd rather create a DNSProvider interface that directly operates on TXT records (and then adapt that interface to conform to acme.ChallengeProvider) instead of having every provider do this independently. Put another way, I think the DNS providers should be described in terms of the records they need to create and destroy, instead of being described in terms of the broader ACME exchange – hence why I made it a sibling instead of a child to acme.

Moving to one package per provider makes total sense to me. Consuming applications can then import lego/acme, which would have direct dependencies on a minimal set of providers, including a manual DNS-01 challenge provider. If the consuming application wants DigitalOcean, they could import …/digitalocean. We could create an additional package like …/all that would pull in all the providers for convenience, so you could depend on that and get everything even as new providers are added.

I think the registry populated on init() solution in #112 is a sound approach. This lets consumers (lego CLI and otherwise) easily get a list of everything that's available, no matter how it got linked into the binary. It's also by no means mandatory; if you know you want the Digital Ocean provider, you could import and call it directly without talking to a registry.

Speaking of available: I think lego should default to including everything. Build tags are a fine thing to offer users that have a reason to want slimmed-down binaries, but convenience is very important for the masses. Features that aren't available out of the box (i.e. things that require recompilation) tend to suffer.

@xenolf
Copy link
Member

xenolf commented Feb 11, 2016

The DNS challenge providers themselves are bits of code that can create and destroy TXT records, but they're totally oblivious to the details.

You are right, I am not objected to making them siblings.

I agree that lego should to import all providers by default. People who are concerned about the size can customize the build by passing in buildflags.

I would also like to think about how providers to other challenges would fit into your approach. I know that at the moment we have more providers for DNS-01 compared to the other two challenges but I expect that to change and I don't want to introduce any barriers for the future.

@xenolf
Copy link
Member

xenolf commented Mar 14, 2016

With the merge of #144, I guess we can close this. Feel free to reopen if there is still something left to discuss.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants