code review

[Catonano <catonano@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 8706 bytes --]

I could use some code review on my Trytond service hypothesis

As far as I understand, there are 2 steps that need to be done in order for
a Trytond service to be usable.

1) as the "postgres" role (that is as the operating system "postgres"
user), create a "tryton" role

2) as the tryton user (hence under the tryton role) run the Tryton
initialization script (trytond-admin -c <config file> -d <database name>
--all)

I feel like I should extend the postgresql service in order to create te
trytond role but I don't know how

Also, I create the trytond role with no password. But what if someone wants
to use this service for a real server ?

The role password should be a parameter somehow. Again, I'm not sure how

I borrowed some code from the postgresql service code and somewhat edited it

But I feel like an amateur neurosurgeon :-/

I could really use a review of this code

Here it is

(define-module (gnu services trytond)
  #:use-module (gnu services)
  #:use-module (gnu services shepherd)
  #:use-module (gnu system shadow)
  #:use-module (gnu packages admin)
 ;; do I really need to access the postgresql package, here ?
  #:use-module (gnu packages databases)
  #:use-module (gnu packages tryton)
  #:use-module (guix modules)
  #:use-module (guix records)
  #:use-module (guix gexp)
  #:use-module (ice-9 match)
  #:export (trytond-configuration
            trytond-configuration?
            trytond-service
            trytond-service-type
            ))

;;; Commentary:
;;;
;;; Trytond based services. Mainly Trytond and GNUHealth for now
;;;
;;; Code:

(define-record-type* <trytond-configuration>
  trytond-configuration make-trytond-configuration
  trytond-configuration?
  (trytond     trytond-configuration-trytond ;<package>
               (default trytond))
;; do I really need to access the postgresql package, here ?
  (postgresql  postgresql-configuration-trytond
               (default postgresql))
  (locale         trytond-configuration-locale
                  (default "en_US.utf8"))
  (config-file    trytond-configuration-file)
  (data-directory trytond-configuration-data-directory)
  )


(define %default-trytond-config
  (mixed-text-file "trytond.conf"
                   "[database]\n"
                   ;; how do I connect with a role that has no password  ?
                   ;; I create the trytond role without the password
                   ;; but what if someone wants to use this service for a
real server ?
                   ;; the password should be a parameter, somehow
                   ;;"uri = 'postgresql://trytond:password@/'\n"
                   "uri = 'postgresql://trytond:@/'\n" ;; is this string
gonna work ?
                   "path = /var/lib/trytond"))

(define %trytond-accounts
  (list (user-group (name "trytond") (system? #t))
        (user-account
         (name "trytond")
         (group "trytond")
         (system? #t)
         (comment "Trytond server user")
         (home-directory "/var/empty")
         (shell (file-append shadow "/sbin/nologin")))))

(define trytond-activation
  (match-lambda
    (($ <trytond-configuration> trytond postgresql locale config-file
data-directory)
     #~(begin
         (use-modules (guix build utils)
                                (gnu packages database)
                                (ice-9 match))

         (let ((trytond-user (getpwnam "trytond"))
               (postgres-user (getpwnam "postgres"))
               (create-the-trytond-role (string-append #$postgresql
                                       "/bin/createuser"
                                       "trytond"
                                        "-d")) ;; the role can create new
DBs
               (run-the-trytond-init-script (string-append #$trytond
                                      "/bin/trytond-admin"
                                      "-c"
                                      #$config-file
                                      "-d"
                                      "trytondb" ;;the database name
                                      "--all"))
               (trytond-initscript-args
                (append
                 (if #$locale
                     (list (string-append "-l " #$locale))
                     '()))))
           ;; Create data directory.
           (mkdir-p #$data-directory)
           (chown #$data-directory
                  (passwd:uid trytond-user)
                  (passwd:gid trytond-user))


           ;; Drop privileges and create the tryton role in a new
           ;; process.  Wait for it to finish before proceeding.
           ;; shouldn't this be done by extending the postgresql service ?
           ;; but how ?
           (match (primitive-fork)
             (0
              ;; Exit with a non-zero status code if an exception is thrown.
              (dynamic-wind
                (const #t)
                (lambda ()
                  (setgid (passwd:gid postgres-user))
                  (setuid (passwd:uid postgres-user))
                  (primitive-exit
                   (apply system*
                          ;; shouldn't this be done by an extension to the
postgresql service ?
                          ;; but how ?
                          create-the-trytond-role)))
                (lambda ()
                  (primitive-exit 1))))
             (pid (waitpid pid))))))))




           ;; Drop privileges and run the trytond init script in a new
           ;; process.  Wait for it to finish before proceeding.
           (match (primitive-fork)
             (0
              ;; Exit with a non-zero status code if an exception is thrown.
              (dynamic-wind
                (const #t)
                (lambda ()
                  (setgid (passwd:gid trytond-user))
                  (setuid (passwd:uid trytond-user))
                  (primitive-exit
                   (apply system*
                          run-the-trytond-init-script
                          trytond-initscript-args)))
                (lambda ()
                  (primitive-exit 1))))
             (pid (waitpid pid))))))))

(define trytond-shepherd-service
  (match-lambda
    (($ <trytond-configuration> trytond locale config-file)
     (let ((start-script
            ;; Wrapper script that switches to the 'trytond' user before
            ;; launching daemon.
            (program-file "start-trytond"
                          #~(let ((user (getpwnam "trytond"))
                                  (trytond (string-append #$trytond
                                                           "/bin/trytond")))
                              (setgid (passwd:gid user))
                              (setuid (passwd:uid user))
                              (system* trytond
                                       (string-append "-c "
                                                      #$config-file))))))
       (list (shepherd-service
              (provision '(trytond))
              (documentation "Run the Trytond daemon.")
              (requirement '(user-processes loopback postgresql))
              ;; why do I require the postgrresql service if I don't use it
?
              (start #~(make-forkexec-constructor #$start-script))
              (stop #~(make-kill-destructor))))))))

(define trytond-service-type
  (service-type (name 'trytond)
                (extensions
     ;; how is the postgresql service meant to be extended ?
                 (list (service-extension shepherd-root-service-type
                                          trytond-shepherd-service)
                       (service-extension activation-service-type
                                          trytond-activation)
                       (service-extension account-service-type
                                          (const %trytond-accounts))))))

(define* (trytond-service #:key (trytond trytond)
                          (postgresql postgresql)
                             ;;(port 5432) ;; The port is in the config file
                          (locale "en_US.utf8")
                          (config-file %default-trytond-config)
                          ;;(data-directory "/var/lib/trytond/data")
                          )
  "Return a service that runs @var{trytond}, the Tryton server component.

The Trytond daemon loads its runtime configuration from @var{config-file}
and relies on the PostgreSQL service for data storage."
  (service trytond-service-type
           (trytond-configuration
            (trytond trytond)
            (postgresql postgresql) ;; sigh
            ;;(port port)
            (locale locale)
            (config-file config-file)
            ;;(data-directory data-directory)
            )))

[-- Attachment #2: Type: text/html, Size: 12576 bytes --]