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

#49 Handle domain list change for a certificate #50

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

MonsieurV
Copy link

For solving #49, we persist to the host machine a file containing the list of domains for a given certificate.

If this list does not match the one provided in the current configuration, the certificate is issued again.

Beware: this make certificate expansion (--expand) the default implicit behavior. Maybe we could make it an option and only try to update the certificate if the variable certbot_auto_expand is set to true.

Anyway, it now works as I would expect when using certbot_auto_renew: if we added a domain to one of our certificates, it is updated so the domain is now covered.

@domhaas
Copy link

domhaas commented Aug 12, 2018

@MonsieurV Thanks! Can confirm this works very well. One thing about the persist-list: I think it would be better to name the file not by the first domain that is configured, cause there's a new file created when you put a new domain in the first position.

@MonsieurV
Copy link
Author

@domhaas Thanks for the feedback. :)

Do you have any suggestion for the file name?

It should ideally be something derived from the domain lists (ensuring no collision if we have multiple certs) and as invariant as possible.

Maybe we could add a variable for the persisted domain list filename (defaulting to the first domain).

@mmcnl
Copy link

mmcnl commented May 17, 2019

@geerlingguy I'm curious if there are any blockers to this being merged? It would be a nice touch to have this handled automatically.

@jclendenan
Copy link

Any updates on this?

@stale
Copy link

stale bot commented Mar 6, 2020

This pull request has been marked 'stale' due to lack of recent activity. If there is no further activity, the PR will be closed in another 30 days. Thank you for your contribution!

Please read this blog post to see the reasons why I mark pull requests as stale.

@MonsieurV
Copy link
Author

I'm still using this solution and it does the job for me.

@stale
Copy link

stale bot commented Mar 6, 2020

This issue is no longer marked for closure.

@stale stale bot removed the stale label Mar 6, 2020
@geerlingguy
Copy link
Owner

I'm on the fence on this; I don't generally like to pollute managed directories with files to track state, and that's what this seems to be doing (maintaining a list of domains in a file in the letsencrypt directories).

@MonsieurV
Copy link
Author

I'm on the fence on this; I don't generally like to pollute managed directories with files to track state, and that's what this seems to be doing (maintaining a list of domains in a file in the letsencrypt directories).

Yes, that is what it does.

I've hadn't found yet a way to compare this state (domains for a cert) between Ansible runs, without actually persisting it to the filesystem.

Maybe --cert-name could be used and certonly launch at each Ansible run to be sure that domain list changes are taken into account.
See https://certbot.eff.org/docs/using.html#changing-a-certificate-s-domains

@stale
Copy link

stale bot commented Jun 4, 2020

This pull request has been marked 'stale' due to lack of recent activity. If there is no further activity, the PR will be closed in another 30 days. Thank you for your contribution!

Please read this blog post to see the reasons why I mark pull requests as stale.

@stale stale bot added the stale label Jun 4, 2020
@stale
Copy link

stale bot commented Jun 7, 2020

This pull request is no longer marked for closure.

@stale stale bot removed the stale label Jun 7, 2020
@geerlingguy
Copy link
Owner

I do want to fix this. I'm just giving it more time for consideration. I just hate adding state and complexity that this role introduces and then relies on.

@MonsieurV
Copy link
Author

MonsieurV commented Jun 7, 2020

I just hate adding state and complexity that this role introduces and then relies on.

Either we add state, or we use the state already accessible.

There is now a certbot certificates command that returns information on installed certificated (also see certbot#3656).

The ouput is like:

Saving debug log to /var/log/letsencrypt/letsencrypt.log

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Found the following certs:
  Certificate Name: www.example.com
    Serial Number: 441319d7aa87ad735838dff43791b4d208b
    Domains: www.example.com admin.example.com api.example.com blog.example.com example.com
    Expiry Date: 2020-08-04 11:56:17+00:00 (VALID: 58 days)
    Certificate Path: /etc/letsencrypt/live/www.example.com/fullchain.pem
    Private Key Path: /etc/letsencrypt/live/www.example.com/privkey.pem
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

It seems doable to parse that, extract the certificate name (which matches the first domain used by this role for each domain list entry) and extract then compare the domain list. It won't be as easy to code as adding our own state file, but the state is here.

Then we would need to run certonly with the --cert-name provided and the new list of domains.

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

Successfully merging this pull request may close these issues.

None yet

5 participants