Merge "letsencrypt: force renewal on certificate change"

playbooks/roles/letsencrypt-acme-sh-install/files/driver.sh

@@ -45,10 +45,21 @@ if [[ ${1} == "issue" ]]; then
 elif [[ ${1} == "renew" ]]; then
     shift;
     for arg in "$@"; do
+        # NOTE(ianw) 2020-02-28 : we only set force here because of a
+        # bug/misfeature in acme.sh dns manual-mode where it does not
+        # notice that the renewal is required when we update domain
+        # names in a cert
+        # (https://github.com/acmesh-official/acme.sh/issues/2763).
+        # This is safe (i.e. will not explode our quota limits by
+        # constantly renewing) because Ansible only calls this path
+        # when TXT records have been installed for this certificate;
+        # i.e. we will never run this renewal unless it is actually
+        # required.
         $ACME_SH ${STAGING} \
             --cert-home ${CERT_HOME} \
             --no-color \
             --yes-I-know-dns-manual-mode-enough-go-ahead-please \
+            --force \
             --renew \
             $arg 2>&1 | tee -a ${LOG_FILE}
     done

playbooks/roles/letsencrypt-create-certs/tasks/main.yaml

@@ -7,7 +7,19 @@
     msg: "acme_txt_required is not defined; was letsencrypt-request-certs run?"
   when: acme_txt_required is not defined
 
+# acme_txt_keys is a list of tuples
+#
+#  (key from letsencrypt_certs, required TXT record)
+#
+# So in words, we walk acme_txt_required and keep a list of the unique
+# 0-values of each entry.  This is then the keys from
+# letsencrypt_certs that actually had updates; these are the only ones
+# we need to do a renewal for.
+- name: Generate list of changed certificates
+  set_fact:
+    acme_txt_changed: '{{ acme_txt_required|map("first")|list|unique }}'
+
 - name: Include ACME renewal
   include_tasks: acme.yaml
   loop: "{{ query('dict', letsencrypt_certs) }}"
-  when: acme_txt_required | length > 0
+  when: item.key in acme_txt_changed

playbooks/roles/letsencrypt-request-certs/tasks/acme.yaml

@@ -14,17 +14,15 @@
   environment:
     LETSENCRYPT_STAGING: '{{ "1" if letsencrypt_use_staging else "0" }}'
 
-# NOTE(ianw): The output is challenge-domain:txt-key which we split
-# into a tuple here.  acme.sh by default puts the hostname into the
-# challenge domain it outputs.  For simplicity, we don't actually make
-# use of the full challenge-domain part; our default CNAME setup
-# points "_acme-challenge.host.opendev.org" to just "acme.opendev.org"
-# -- thus we put all the keys into "top-level" TXT records directly at
-# acme.opendev.org.  letsencyrpt doesn't care; it just follows the
-# CNAME and enumerates all the TXT records in acme.opendev.org looking
-# for one that matches.  So even though we don't put it in the dns
-# records, having the hostname the TXT record is for is handy for
-# debugging, etc, so we pass it through.
+# NOTE(ianw): The output of the driver is
+#
+#  challenge-domain:TXT-key
+#
+# We don't care about the challenge-domain part (we have set all
+# _acme-challenge.hostname.o.o records as CNAMES to acme.opendev.org).
+# Record the config key along with the TXT record; later we use it to
+# check which config keys have been updated and need a refresh.
+#
 - set_fact:
-    acme_txt_required: '{{ acme_txt_required + [(item.split(":")[0], item.split(":")[1])] }}'
+    acme_txt_required: '{{ acme_txt_required + [(cert.key, item.split(":")[1])] }}'
   loop: '{{ acme_output.stdout_lines }}'