puppet: Use certbot package timer, not our own cron job.

The certbot package installs its own systemd timer (and cron job,
which disabled itself if systemd is enabled) which updates
certificates.  This process races with the cron job which Zulip
installs -- the only difference being that Zulip respects the
`certbot.auto_renew` setting, and that it passes the deploy hook.
This means that occasionally nginx would not be reloaded, when the
systemd timer caught the expiration first.

Remove the custom cron job and `certbot-maybe-renew` script, and
reconfigure certbot to always reload nginx after deploying, using
certbot directory hooks.

Since `certbot.auto_renew` can't have an effect, remove the setting.
In turn, this removes the need for `--no-zulip-conf` to
`setup-certbot`.  `--deploy-hook` is similarly removed, as running
deploy hooks to restart nginx is now the default; pass
`--no-directory-hooks` in standalone mode to not attempt to reload
nginx.  The other property of `--deploy-hook`, of skipping symlinking
into place, is given its own flog.

docs/development/remote.md

@@ -300,7 +300,7 @@ different.
    mkdir -p /var/lib/zulip/certbot-webroot/
    # if nginx running this will fail and you need to run `service nginx stop`
    /home/zulipdev/zulip/scripts/setup/setup-certbot \
-     hostname.example.com --no-zulip-conf \
+     hostname.example.com \
      --email=username@example.com --method=standalone
    ```
 

docs/production/deployment.md

@@ -636,14 +636,6 @@ Override the default uwsgi backlog of 128 connections.
 Override the default `uwsgi` (Django) process count of 6 on hosts with
 more than 3.5GiB of RAM, 4 on hosts with less.
 
-### `[certbot]`
-
-#### `auto_renew`
-
-If set to the string `yes`, [Certbot will attempt to automatically
-renew its certificate](../production/ssl-certificates.html#certbot-recommended). Do
-no set by hand; use `scripts/setup/setup-certbot` to configure this.
-
 ### `[postfix]`
 
 #### `mailname`

docs/production/ssl-certificates.md

@@ -117,13 +117,11 @@ Zulip configures automatic renewal for you. As a result, a Zulip
 server configured with Certbot does not require any ongoing work to
 maintain a current valid SSL certificate.
 
-Specifically, the `setup-certbot` tool (and by extension, the
+The `certbot` package configures a systemd timer (similar to a cron
-installer option) enables the Certbot `auto_renew` property in
+job) that will renew any Certbot certificates that are due for
-`/etc/zulip/zulip.conf`. This, in turn, configures a cron job
+renewal. The renewal process repeats the Certbot proof-of-control
-(`/etc/cron.d/certbot`) that will renew any Certbot certificates that
+process, receives the new certificate from Certbot, installs the new
-are due for renewal. The renewal process repeats the Certbot
+certificate, and then reloads `nginx`.
-proof-of-control process, receives the new certificate from Certbot,
-installs the new certificate, and then reloads `nginx`.
 
 #### Troubleshooting
 
@@ -131,9 +129,7 @@ If your Certbot certificate expires, it is usually because of firewall
 rules preventing the Certbot renewal process (which is essentially
 identical to the initial certificate request process) from
 working. You can debug interactively by running the command from the
-cron job,
+cron job, `/usr/bin/certbot renew`, as `root`.
-`/home/zulip/deployments/current/scripts/lib/certbot-maybe-renew`, as
-`root`.
 
 ## Self-signed certificate
 

puppet/zulip/files/cron.d/certbot-renew

@@ -1,6 +0,0 @@
-SHELL=/bin/bash
-PATH=/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin
-USER=root
-
-# Cron job to renew certbot twice a day.
-52 0,12 * * * root /home/zulip/deployments/current/scripts/lib/certbot-maybe-renew

puppet/zulip/files/letsencrypt/nginx-deploy-hook.sh

@@ -0,0 +1,3 @@
+#!/usr/bin/env bash
+
+service nginx reload

puppet/zulip/manifests/nginx.pp

@@ -124,14 +124,9 @@ class zulip::nginx {
     mode    => '0644',
     source  => 'puppet:///modules/zulip/logrotate/nginx',
   }
-
+  package { 'certbot':
-  $certbot_auto_renew = zulipconf('certbot', 'auto_renew', '')
+    ensure => 'installed',
-  if $certbot_auto_renew == 'yes' {
-    package { 'certbot':
-      ensure => 'installed',
-    }
   }
-
   file { ['/var/lib/zulip', '/var/lib/zulip/certbot-webroot']:
     ensure => 'directory',
     owner  => 'zulip',

puppet/zulip/manifests/profile/app_frontend.pp

@@ -38,13 +38,28 @@ class zulip::profile::app_frontend {
     notify  => Service['nginx'],
   }
 
-  # Trigger 2x a day certbot renew
+  # We used to install a cron job, but certbot now has a systemd cron
+  # that does better.  This can be removed once upgrading from 5.0 is
+  # no longer possible.
   file { '/etc/cron.d/certbot-renew':
-    ensure => file,
+    ensure => absent,
-    owner  => 'root',
+  }
-    group  => 'root',
+
-    mode   => '0644',
+  # Reload nginx after deploying a new cert.
-    source => 'puppet:///modules/zulip/cron.d/certbot-renew',
+  file { ['/etc/letsencrypt/renewal-hooks', '/etc/letsencrypt/renewal-hooks/deploy']:
+    ensure  => directory,
+    owner   => 'root',
+    group   => 'root',
+    mode    => '0755',
+    require => Package[certbot],
+  }
+  file { '/etc/letsencrypt/renewal-hooks/deploy/001-nginx.sh':
+    ensure  => file,
+    owner   => 'root',
+    group   => 'root',
+    mode    => '0755',
+    source  => 'puppet:///modules/zulip/letsencrypt/nginx-deploy-hook.sh',
+    require => Package[certbot],
   }
 
   # Restart the server regularly to avoid potential memory leak problems.

scripts/lib/certbot-maybe-renew

@@ -1,22 +0,0 @@
-#!/usr/bin/env bash
-
-zulip_conf_get_boolean() {
-    # Get a boolean flag from zulip.conf, using the Python
-    # `configparser` library's conventions for what counts as true.
-    # Treat absent and invalid values as false.
-    value=$(crudini --get /etc/zulip/zulip.conf "$1" "$2" 2>/dev/null)
-    case "$(echo "$value" | tr '[:upper:]' '[:lower:]')" in
-        1 | yes | true | on) return 0 ;;
-        *) return 1 ;;
-    esac
-}
-
-if ! zulip_conf_get_boolean certbot auto_renew; then
-    exit 0
-fi
-
-deploy_hook="${ZULIP_CERTBOT_DEPLOY_HOOK:-service nginx reload}"
-
-certbot renew --quiet \
-    --webroot --webroot-path=/var/lib/zulip/certbot-webroot/ \
-    --deploy-hook "$deploy_hook"

scripts/lib/install

@@ -367,8 +367,11 @@ elif [ "$package_system" = yum ]; then
 fi
 
 if [ -n "$USE_CERTBOT" ]; then
+    # Puppet, which is run below, installs the post-deploy hook to
+    # reload nginx -- but it also installs nginx itself, so we're fine
+    # to run this now.
     "$ZULIP_PATH"/scripts/setup/setup-certbot \
-        --no-zulip-conf --method=standalone \
+        --method=standalone \
         "$EXTERNAL_HOST" --email "$ZULIP_ADMINISTRATOR"
 elif [ -n "$SELF_SIGNED_CERT" ]; then
     "$ZULIP_PATH"/scripts/setup/generate-self-signed-cert \
@@ -410,10 +413,6 @@ deploy_type = production
 version = $POSTGRESQL_VERSION
 EOF
 
-    if [ -n "$USE_CERTBOT" ]; then
-        crudini --set /etc/zulip/zulip.conf certbot auto_renew yes
-    fi
-
     if [ -n "$POSTGRESQL_MISSING_DICTIONARIES" ]; then
         crudini --set /etc/zulip/zulip.conf postgresql missing_dictionaries true
     fi

scripts/setup/setup-certbot

@@ -5,7 +5,7 @@ set -e
 usage() {
     cat <<EOF >&2
 Usage: $0 --email=admin@example.com [--method={webroot|standalone}] \
-[--no-zulip-conf] hostname.example.com [another.example.com]
+hostname.example.com [another.example.com]
 EOF
     exit 1
 }
@@ -16,7 +16,7 @@ if [ "$EUID" -ne 0 ]; then
 fi
 
 method=webroot
-args="$(getopt -o '' --long help,email:,method:,deploy-hook:,no-zulip-conf,agree-tos -n "$0" -- "$@")"
+args="$(getopt -o '' --long help,email:,method:,skip-symlink,agree-tos -n "$0" -- "$@")"
 eval "set -- $args"
 while true; do
     case "$1" in
@@ -30,19 +30,14 @@ while true; do
             shift
             shift
             ;;
-        --deploy-hook)
+        --skip-symlink)
-            deploy_hook=(--deploy-hook "$2")
+            skip_symlink=1
-            shift
             shift
             ;;
         --agree-tos)
             agree_tos=--agree-tos
             shift
             ;;
-        --no-zulip-conf)
-            no_zulip_conf=1
-            shift
-            ;;
         --help)
             show_help=1
             shift
@@ -71,7 +66,7 @@ fi
 
 case "$method" in
     standalone)
-        method_args=(--standalone)
+        method_args=(--standalone --no-directory-hooks)
         ;;
     webroot)
         method_args=(--webroot '--webroot-path=/var/lib/zulip/certbot-webroot/')
@@ -112,7 +107,6 @@ esac
 certbot certonly "${method_args[@]}" \
     "${HOSTNAMES[@]}" -m "$EMAIL" \
     $agree_tos \
-    "${deploy_hook[@]}" \
     --force-interactive --no-eff-email
 
 symlink_with_backup() {
@@ -125,22 +119,10 @@ symlink_with_backup() {
     ln -nsf "$1" "$2"
 }
 
-if [ ${#deploy_hook} -eq 0 ]; then
+if [ -z "$skip_symlink" ]; then
-    # If no deploy hook was specified, assume we're deploying to the default
-    # location Zulip wants.
     CERT_DIR=/etc/letsencrypt/live/"$DOMAIN"
     symlink_with_backup "$CERT_DIR"/privkey.pem /etc/ssl/private/zulip.key
     symlink_with_backup "$CERT_DIR"/fullchain.pem /etc/ssl/certs/zulip.combined-chain.crt
 fi
 
-case "$method" in
-    webroot)
-        service nginx reload
-        ;;
-esac
-
-if [ -z "$no_zulip_conf" ]; then
-    crudini --set /etc/zulip/zulip.conf certbot auto_renew yes
-fi
-
 echo "Certbot SSL certificate configuration succeeded."