-
Notifications
You must be signed in to change notification settings - Fork 12
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
Allow updating existing certs #23
Conversation
Hi @Matt-Yorkley , thanks for this! We will take it next week. Personally I haven't hacked on this repo yet, so another mate will come. cheers! |
5bd9f15
to
75a889e
Compare
tasks/certificate.yml
Outdated
{% endif %} | ||
{% if letsencrypt_staging %} --staging {% endif %}" | ||
when: letsencrypt_cert.stat.exists and certbot_force_update is defined | ||
notify: reload ngninx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Matt-Yorkley is there a typo here? ngninx
instead of nginx
. Right?
tasks/certificate.yml
Outdated
{% endif %} | ||
{% if letsencrypt_staging %} --staging {% endif %}" | ||
when: letsencrypt_cert.stat.exists and certbot_force_update is defined | ||
notify: reload ngninx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
notify: reload ngninx | |
notify: reload nginx |
by @delphaber
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Matt-Yorkley , when you fix the nginx typo I'll test this and merge it if everything ok. It's been too long for this feature, sorry I was out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. It's not really urgent, but we do have some certificates on a couple of servers that need to be updated.
75a889e
to
0781b56
Compare
The test is failing due to certbot deprecating Ubuntu Trusty (14.04). Are you using this role in any machines running this distro?? We could try to get the .deb if necessary, but I think we should simply state that we don't support trusty anymore. |
No, we're on 16 and upgrading to 18 later in the year. Deprecating 14 sounds like a good idea 👍 |
I'm gonna test this tomorrow (Tue 10) for some multi-domain odoo instance we are setting up, and if it works as expected, I'm gonna merge it too! Saying in advance just in case you had some changes stashed without publishing, @Matt-Yorkley |
Hi @Matt-Yorkley , I tried your branch in a situation where it fitted perfectly, and it didn't work at the first run. I'm gonna investigate this and see if I can fix it. It says: I think that the certificate changed the name, I'm gonna check it.
|
Ok, so it's not a problem with the certbot logic, it looks like a problem with # sdfsadfa
sh: 3: sdfsadfa: not found
# echo $?
127 It returns the same error code as the one with certbot. EDIT: I launched the exact same command manually from a bash and then sh shell, and it works well. |
I got it: unnecessary quotes. See: # "ls -l"
sh: 11: ls -l: not found
# echo $?
127 At [https://yaml-multiline.info/] I found all cases I needed to cover about quoting in yaml. |
tasks/certificate.yml
Outdated
@@ -6,10 +6,21 @@ | |||
|
|||
- name: Generate new certificate if one doesn't exist |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- name: Generate new certificate if one doesn't exist | |
- name: "Generate new certificate if one doesn't exist" |
Quote the name so that vim and possibly other syntax highlighters don't mess with unpaired single quote.
How do you see it @Matt-Yorkley ? |
Se ve bien 👍 |
Ah, I just ran this today, and I got the |
@Matt-Yorkley, did you use an old version? Or the merged one still has this issue? |
I'm pretty sure it was the merged version... |
hmmm can you debug it a bit further, then? For me the fixed worked :S I can test it again too if needed |
I just used this to update a certificate on a production server with Thanks! ❤️ ❤️ |
Hey guys, I just had to update an existing certificate on one of our servers to fix a bug, and realised it wasn't possible with the certbot role. We've had to do this a couple of times, and manually recreate the certificates on servers via the commandline.
I've added a new option here to allow recreating existing certificates when they need to be updated, for example when a new subdomain is added. I also noticed nginx had to be reloaded when changing an existing certificate, so I've added a handler for that.
The new optional task can be run using
--extra-vars "certbot_force_update=true"
.What do you think?
Relevant docs here: https://certbot.eff.org/docs/using.html#re-creating-and-updating-existing-certificates