[bug#67497,v2,4/4] In certbot's client configuration, offer multiple deploy-hooks.

The certbot program can accept multiple deploy hooks by repeating the relevant
option on the command line. This commit makes that capability available to
users.

Certificates are often used to secure multiple services. It is helpful to have
separate hooks for each service. It makes those hooks easier to maintain. It's
also easier that way to re-use a hook for another certificate that may not
serve to secure the same combination of services.

Change-Id: I3a293daee47030d9bee7f366605aa63a14e98e38
---
 doc/guix.texi            | 11 ++++++-----
 gnu/services/certbot.scm | 18 ++++++++++++++++--
 2 files changed, 22 insertions(+), 7 deletions(-)
  

Hi,

Felix Lechner <felix.lechner@lease-up.com> writes:

> The certbot program can accept multiple deploy hooks by repeating the relevant
> option on the command line. This commit makes that capability available to
> users.
>
> Certificates are often used to secure multiple services. It is helpful to have
> separate hooks for each service. It makes those hooks easier to maintain. It's
> also easier that way to re-use a hook for another certificate that may not
> serve to secure the same combination of services.

For this commit and the previous one, you can keep your nice explanatory
text, but a GNU ChangeLog must be added below, per our conventions.  I
can be terse and to the point, touching only the *changes*, especially
since you already have an explanatory text.

> Change-Id: I3a293daee47030d9bee7f366605aa63a14e98e38
> ---
>  doc/guix.texi            | 11 ++++++-----
>  gnu/services/certbot.scm | 18 ++++++++++++++++--
>  2 files changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 1b0fa4f2a3a..deb1f76d353 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -35378,7 +35378,7 @@ Certificate Services
>             (list
>              (certificate-configuration
>               (domains '("example.net" "www.example.net"))
> -             (deploy-hook %nginx-deploy-hook))
> +             (deploy-hooks '(%nginx-deploy-hook)))
>              (certificate-configuration
>               (domains '("bar.example.net")))))))
>  @end lisp
> @@ -35483,14 +35483,15 @@ Certificate Services
>  additionally @code{$CERTBOT_AUTH_OUTPUT} will contain the standard output
>  of the @code{auth-hook} script.
>  
> -@item @code{deploy-hook} (default: @code{#f})
> -Command to be run in a shell once for each successfully issued
> -certificate.  For this command, the environment variable
> +@item @code{deploy-hooks} (default: @code{'()})
> +Commands to be run in a shell once for each successfully issued
> +certificate.  For these commands, the environment variable
>  @code{$RENEWED_LINEAGE} will point to the config live subdirectory (for
>  example, @samp{"/etc/letsencrypt/live/example.com"}) containing the new
>  certificates and keys; the environment variable @code{$RENEWED_DOMAINS} will
>  contain a space-delimited list of renewed certificate domains (for
> -example, @samp{"example.com www.example.com"}.
> +example, @samp{"example.com www.example.com"}. Please note that the singular
> +field @code{deploy-hook} was replaced by this field in the plural.

Need two space before the new sentence starts.

>  
>  @item @code{start-self-signed?} (default: @code{#t})
>  Whether to generate an initial self-signed certificate during system
> diff --git a/gnu/services/certbot.scm b/gnu/services/certbot.scm
> index 08a480ed3b1..7a67b9bd7cb 100644
> --- a/gnu/services/certbot.scm
> +++ b/gnu/services/certbot.scm
> @@ -30,6 +30,7 @@ (define-module (gnu services certbot)
>    #:use-module (gnu services web)
>    #:use-module (gnu system shadow)
>    #:use-module (gnu packages tls)
> +  #:use-module (guix deprecation)
>    #:use-module (guix i18n)
>    #:use-module (guix records)
>    #:use-module (guix gexp)
> @@ -63,8 +64,11 @@ (define-record-type* <certificate-configuration>
>                         (default #f))
>    (cleanup-hook        certificate-cleanup-hook
>                         (default #f))
> +  ;; TODO: remove singular deploy-hook; is deprecated

For standalone comments, please use complete sentences, as in:

     ;; TODO: Remove singular deploy-hook, which is deprecated.

Note that it's not enough to simply document it as deprecated, you must
introduce a deprecation warning when people are using it for the
'deprecation' to count as such.  Since this record is using a plain Guix
record, this is usually done using a sanitizer with a maybe value that
warns when the value is set.

>    (deploy-hook         certificate-configuration-deploy-hook
>                         (default #f))
> +  (deploy-hooks        certificate-configuration-deploy-hooks
> +                       (default '()))
>    (start-self-signed?  certificate-configuration-start-self-signed?
>                         (default #t)))
>  
> @@ -140,7 +144,8 @@ (define certbot-command
>                (match-lambda
>                  (($ <certificate-configuration> custom-name domains challenge
>                                                  csr authentication-hook
> -                                                cleanup-hook deploy-hook)
> +                                                cleanup-hook
> +                                                deploy-hook deploy-hooks)
>                   (let ((name (or custom-name (car domains))))
>                     (append
>                      (list name
> @@ -168,7 +173,16 @@ (define certbot-command
>                          (list "--register-unsafely-without-email"))
>                      (if server (list "--server" server) '())
>                      (if rsa-key-size (list "--rsa-key-size" rsa-key-size) '())
> -                    (if deploy-hook (list "--deploy-hook" deploy-hook) '())))))
> +
> +                    (if deploy-hook
> +                        (begin
> +                          (warn-about-deprecation 'deploy-hook #f
> +                                                  #:replacement 'deploy-hooks)

Ah, I see you warned here, but that's going to happen at the time the
service is executed, right?  Which is not as good: we'd like the
deprecation to be printed as early as possible, typically when the user
reconfigures their system.  A sanitizer on the record field would
achieve that.