[Discussion] API

[blopker]

Hey! First off, congratulations on becoming an official Dokku plugin. I think this plugin really is the 'killer app' for Dokku, nice work.

Anyway, I came by to see how things are going since my last PR and I think the API could be cleaned up a little. These ideas will be breaking changes, but I hope to make the plugin easier to use and easier to add features later. Feel free to say 'no'. 😄

The first issue I noticed is user on-boarding got more complicated. Right now the minimum a new user has to do is:

sudo dokku plugin:install https://github.com/dokku/dokku-letsencrypt.git
sudo dokku letsencrypt:email <app> <e-mail>
sudo dokku letsencrypt <app>

Ideally all a user should have to do is:

sudo dokku plugin:install https://github.com/dokku/dokku-letsencrypt.git
sudo dokku letsencrypt <app>

This can be easily done because letsencrypt (and simp_le) lets you get new certs without an email address. See https://github.com/kuba/simp_le/blob/master/simp_le.py#L1233

Although it's recommended to add an email it raises the barrier for entry. I think the main appeal of this plugin is taking something that used to be massively complicated and turn it in to a one-liner.

Second issue I noticed is people are asking for more configuration options. This isn't a bad thing (shows popularity!), but the plugin will need a way to add more options in the future without increasing complexity.

I propose the API is reduced to:

$ dokku help
    letsencrypt <app>                 Enable or renew letsencrypt certificate for app
    letsencrypt:revoke <app>          Revoke letsencrypt certificate for app
    letsencrypt:info <app>            Print letsencrypt info about <app>

And use environment variables to add configuration options. The email setting workflow would go from:

sudo dokku letsencrypt:email <app> <e-mail>
sudo dokku letsencrypt <app>

To:

sudo LE_EMAIL=<e-mail> dokku letsencrypt <app>

On second run config can be read from the app so to renewing is just: letsencrypt <app>.

The main advantage is this will scale when more options are requested, like setting the ACME server:

sudo LE_EMAIL=<e-mail> LE_SERVER=staging dokku letsencrypt <app>

The list of commands wont need new set/get methods for every option. The letsencrypt:info command will be the universal get command.

The other advantage of this approach is people can set up their environment globally once for all apps.

Thoughts?

[sseemayer]

Thanks for the suggestions! Here's my 2c on this and I'd be very interested what others are thinking:

Regarding allowing registrations without e-mails by default: I think that it's a bad idea to default to a choice that might cause the user problems down the road (no notifications from the CA if certificates are about to expire or other problems happen). Since the dokku letsencrypt <app> command gives an error message that will tell the user how to set an e-mail address, I don't feel like the additional step overly complicates on-boarding.

One idea would be to have a way to set a server-global Let's Encrypt e-mail address that will be defaulted to if there is no e-mail set for the app. This will reduce the amount of work for new apps by one command.

Regarding a simplification of configuration: There will be even more configuration options necessary to implement automatic renewal (#18) (such as the amount of time left on the certificate before renewal should happen). Consequently, I'm expecting the API to grow even more and it's definitely time to talk about simplifying things.

I'm currently working on some refactorings to have generic letsencrypt_get and letsencrypt_set functions in the functions file that will query/set configuration parameters to reduce the amount of boilerplate code.

I personally dislike the technique of using enivronment variables for important configuration options such as the e-mail address as these should be as visible as possible to the user. However, it might be a good idea to use environment variables for settings where we have good defaults (server selection, time before renewal, etc.)

[fruitl00p]

I would like to chime in here... My thoughts:

E-mails by default: i fully concur with @sseemayer to keep the e-mails in there but have the server wide setup option too. It might even be part of the plugin install command... But again, that would be silly. I for one applaud the current state of the plugin already for its simplicity and ease of use... (edge cases withstanding)

Simplication of configuration by environment variables would be a misnomer. By keeping config via parameters / and config files it still really simple. Also agree on sane defaults (i.e. renewal grace periods of 30 days et al) but passing everything via environment variables is not very common for dokku-plugins. Just for that reason alone I would consider command line options or reading in from the app-config... (i.e. dokku config:set LETSENCRYPT_RENEWAL_GRACE=30 et al) The only reason against this would be that the app itself would have knowledge of those configuration settings...

One might argue about the server selection options: these seem too advanced for most users and thus might 'clutter' the help... (and could be moved to a --server param on the letsencrypt <app> - command itself... Afther this the API would be simplified to:

letsencrypt <app> [<server>]                        Enable or renew letsencrypt for app
letsencrypt:revoke <app>                  Revoke letsencrypt for app
letsencrypt:email <e-mail> [<app>]          Set a host wide e-mail address used as letsencrypt contact or specific for the app if app is given

The :email is a minor change in the API but would allow both server wide and app specific to be set via a single entry... ?

[blopker]

Sweet, thanks for the feedback. It seems like I've hit on some good issues, though I agree, the environment variable solution is clunky. Just the first thing to pop in to mind.

I didn't know about it before, but I like the idea of using dokku's config plugin. It would work for both global and app specific configs and looks scalable. I don't see the app having access to these settings being an issue, as long as there are no collisions. Doing dokku config:set <app> LE_EMAIL=<e-mail> seems as good as dokku letsencrypt:email <app> <e-mail> to me and it's less code that has to be maintained in this plugin. Reading the config looks like just a matter of source ENV then source <app>/ENV.

As far as not letting people use no email, I guess it's not a problem. People can always use fake emails if they want. I'd just like to see the onboarding be a easy as possible. That being said, the user presumably already set up dokku which is pretty complicated so they should be able to handle an extra command here. 😄

[sseemayer]

Since I've pushed the automatic renewal code now, you can see that the API has blown up and I think that cleaning up the settings getters/setters will be the next important step.

I agree that the config:set approach is the way to go since it will greatly slim down the plugin code base while also getting us global settings for free.

[sseemayer]

I finally got around to actually doing the API cleanup! You can find the new version on the dev-config branch: https://github.com/dokku/dokku-letsencrypt/tree/dev-config . A nice bonus is that you can now even dokku config:set --global LE_EMAIL=my@email.com to set the e-mail globally for all apps.

I've got it working well on my server, but since this is a backwards compatibility-breaking change, I'd like to know your opinions before merging this into master 👍

[MorrisJobke]

You can find the new version on the dev-config branch: https://github.com/dokku/dokku-letsencrypt/tree/dev-config .

Could you open a pull request? Then we could comment in there. I will test this soon and provide feedback.

[sseemayer]

PR is open: #30

[sseemayer]

The eagle has landed (in master)! Closing.