Re: [PATCH] gnu: Add CUPS service.

[ludo@gnu.org (Ludovic Courtès)]

Hello!

Andy Wingo <wingo@igalia.com> skribis:

> * gnu/services/cups.scm: New file.
> * gnu/local.mk (GNU_SYSTEM_MODULES): Add gnu/services/cups.scm.
> * doc/guix.texi (Printing Services): New section.

Awesome!

> +One way you might want to customize CUPS is to enable or disable the web interface.  You can do that directly, like this:

Line too long.  :-)

> +The available configuration parameters follow.  Each parameter
> +definition is preceded by its type; for example, @samp{string-list foo}
> +indicates that the @code{foo} parameter should be specified as a list of
> +strings.  There is also a way to specify the configuration as a string,
> +if you have an old @code{cupsd.conf} file that you want to port over
> +from some other system; see the end for more details.

I like this approach.

> +@c The following documentation was initially generated by
> +@c (generate-documentation) in (gnu services cups).  Manually maintained
> +@c documentation is better, so we shouldn't hesitate to edit below as
> +@c needed.  However if the change you want to make to this documentation
> +@c can be done in an automated way, it's probably easier to change
> +@c (generate-documentation) than to make it below and have to deal with
> +@c the churn as CUPS updates.

Neat!

> +(define (validate-configuration config fields)
> +  (for-each (lambda (field)
> +              (let ((val ((configuration-field-getter field) config)))
> +                (unless ((configuration-field-predicate field) val)
> +                  (cups-configuration-field-error
> +                   (configuration-field-name field) val))))
> +            fields))

In the future I’d like to make such checks expansion-time, to the extent
possible.

> +#;
> +(define-record-type* <cups-configuration>
> +  cups-configuration make-cups-configuration
> +  cups-configuration?
> +  (cups     cups-configuration-cups ;<package>
> +                  (default cups))
> +  (config-file    cups-configuration-file)
> +  (data-directory cups-configuration-data-directory))

Remove?

> +  (define %cups-accounts
   ^^
Space.

> +;; FIXME!
> +(define (access-control? x) #f)
> +(define (serialize-access-control x) #f)

That means that fields of type ‘access-control’ are never validated and
cannot be serialized?  Is it a problem?

> +(define %cups-activation
> +  ;; Activation gexp.
> +  #~(begin
> +      (use-modules (guix build utils))

To be sure:

  (with-imported-modules '((guix build utils))
    #~(begin …))

> +      (define (build-subject parameters)
> +        (string-concatenate
> +         (map (lambda (pair)
> +                (let ((k (car pair)) (v (cdr pair)))
> +                  (define (escape-char str chr)
> +                    (string-join (string-split str chr) (string #\\ chr)))
> +                  (string-append "/" k "="
> +                                 (escape-char (escape-char v #\=) #\/))))
> +              (filter (lambda (pair) (cdr pair)) parameters))))
> +      (define* (create-self-signed-certificate-if-absent
> +                #:key private-key public-key (owner (getpwnam "root"))
> +                (common-name (gethostname))
> +                (organization-name "GuixSD")
> +                (organization-unit-name "Default Self-Signed Certificate")
> +                (subject-parameters `(("CN" . ,common-name)
> +                                      ("O" . ,organization-name)
> +                                      ("OU" . ,organization-unit-name)))
> +                (subject (build-subject subject-parameters)))
> +        ;; Note that by default, OpenSSL outputs keys in PEM format.  This
> +        ;; is what we want.
> +        (unless (file-exists? private-key)
> +          (cond
> +           ((zero? (system* (string-append #$openssl "/bin/openssl")
> +                            "genrsa" "-out" private-key "2048"))
> +            (chown private-key (passwd:uid owner) (passwd:gid owner))
> +            (chmod private-key #o400))
> +           (else
> +            (format (current-error-port)
> +                    "Failed to create private key at ~a.\n" private-key))))
> +        (unless (file-exists? public-key)
> +          (cond
> +           ((zero? (system* (string-append #$openssl "/bin/openssl")
> +                            "req" "-new" "-x509" "-key" private-key
> +                            "-out" public-key "-days" "3650"
> +                            "-batch" "-subj" subject))
> +            (chown public-key (passwd:uid owner) (passwd:gid owner))
> +            (chmod public-key #o444))
> +           (else
> +            (format (current-error-port)
> +                    "Failed to create public key at ~a.\n" public-key)))))
> +      (let ((user (getpwnam "cups")))
> +        (mkdir-p/perms "/var/run/cups" user #o755)
> +        (mkdir-p/perms "/var/spool/cups" user #o755)
> +        (mkdir-p/perms "/var/log/cups" user #o755)
> +        (mkdir-p/perms "/etc/cups" user #o755)
> +        (mkdir-p/perms "/etc/cups/ssl" user #o700)
> +        (create-self-signed-certificate-if-absent
> +         #:private-key "/etc/cups/ssl/server.key"
> +         #:public-key "/etc/cups/ssl/server.crt"
> +         #:owner (getpwnam "root")
> +         #:common-name (format #f "CUPS service on ~a" (gethostname))))))

Could you add a comment on why we need to create this X.509 certificate
and what it’s used for?

Would it be useful to allow for some parameterization (key type and
size, “-days” value(?), etc.)?

> +(define* (cups-service #:key (config (cups-configuration)))
> +  "Return a service that runs @var{cups}, the Cups database server.
> +
> +The Cups daemon loads its runtime configuration from @var{config-file}
> +and stores the database cluster in @var{data-directory}."
> +  (validate-configuration config
> +                          (if (opaque-cups-configuration? config)
> +                              opaque-cups-configuration-fields
> +                              cups-configuration-fields))
> +  (service cups-service-type config))

s/Cups/CUPS/

Nowadays I prefer to advertise the ‘service’ form so that users clearly
see what’s going on.  However, there’s the extra validation step here.

Would it work to rename the real record constructors to
‘%cups-configuration’ and ‘%opaque-cups-configuration’, and then:

  (define-syntax-rule (cups-configuration fields ...)
    (let ((config (%cups-configuration fields ...)))
      (validate-configuration config …)
      config))

… in which case we can remove the ‘cups-service’ procedure and instead
document:

  (service cups-service-type config)

WDYT?

> +;; A litle helper to make it easier to document all those fields.
> +;; A little helper to make it easier to document all those fields.

Extra line.  :-)

Thank you!

Ludo’.