Re: [PATCH] improve nginx-service

[Julien Lepiller <julien@lepiller.eu>]

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

On Mon, 24 Oct 2016 22:51:27 +0200
ludo@gnu.org (Ludovic Courtès) wrote:

> Hi,
> 
> Julien Lepiller <julien@lepiller.eu> skribis:
> 
> > On Wed, 19 Oct 2016 23:04:14 +0200
> > ludo@gnu.org (Ludovic Courtès) wrote:
> >  
> >> Hi Julien,
> >> 
> >> This looks like a great improvement to me!  Sounds nicer than
> >> fiddling with config files.
> >> 
> >> I suppose we could make ‘nginx-service-type’ extensible (info
> >> "(guix) Service Types and Services") so that people can write
> >> services that define new vhosts?  
> >
> > You mean something like udev-service-type where you could extend it
> > with a list of vhosts?  
> 
> Yes, exactly.  So for example one could write a service for some
> high-level Web service that would in turn create an nginx vhost.
> WDYT?

Sounds great. I tried to work on it, but I'm stuck.

I'm trying to implement a service that extends nginx-service-type with
a vhost (default-nginx-vhost-service-type), and it creates a root
directory for a default empty index file. Now I would like to pass the
absolute path of this directory to the <nginx-vhost-configuration>, but
it is too early for that. System configuration with this new service
fails with:

Backtrace:
In guix/ui.scm:
1220: 19 [run-guix-command system "reconfigure" "/etc/config.scm"]
In ice-9/boot-9.scm:
 157: 18 [catch srfi-34 #<procedure 56353c0 at guix/ui.scm:426:2
()> ...] 157: 17 [catch system-error ...]
In guix/scripts/system.scm:
 870: 16 [#<procedure 560f270 at guix/scripts/system.scm:862:2 ()>]
 776: 15 [process-action reconfigure ("/etc/config.scm") ...]
In guix/store.scm:
1182: 14 [run-with-store # ...]
In guix/scripts/system.scm:
 788: 13 [#<procedure 5be5180 at guix/scripts/system.scm:780:8 (state)>
#] 552: 12 [perform-action reconfigure # # ...]
In gnu/system.scm:
 627: 11 [operating-system-derivation # # #f]
In gnu/services.scm:
 585: 10 [loop #]
In srfi/srfi-1.scm:
 578: 9 [map #<procedure loop (sink)> (# # #)]
In gnu/services.scm:
 585: 8 [loop #<<service> type: # parameters: #>]
In srfi/srfi-1.scm:
 578: 7 [map #<procedure loop (sink)> (# # # # ...)]
In gnu/services.scm:
 585: 6 [loop #<<service> type: # parameters: ()>]
In srfi/srfi-1.scm:
 578: 5 [map #<procedure 55bd960 at gnu/services.scm:574:4 (service)>
(# # # # ...)] In gnu/services/web.scm:
 189: 4 [nginx-shepherd-service #]
 181: 3 [nginx-action "-p" "/var/run/nginx"]
 141: 2 [nginx-config-file "/var/log/nginx" "/var/run/nginx" ...]
In srfi/srfi-1.scm:
 576: 1 [map #<procedure default-nginx-vhost-config (vhost)> (# #)]
In unknown file:
   ?: 0 [string-append "    server {\n" "      listen 80;\n" ...]

ERROR: In procedure string-append:
ERROR: In procedure string-append: Wrong type (expecting string):
#<<computed-file> name: "default-nginx-vhost" gexp: #<gexp (begin
(mkdir #<gexp-output out>) (symlink #<gexp-input #<<plain-file> name:
"index.html" content: "[...]" references: ()>:out> (string-append
#<gexp-output out> "/index.html"))) 5959270> options: (#:local-build?
#t)>

> 
> > From 8bda6fdd53b3cc7470fac67228a88e0d165134dd Mon Sep 17 00:00:00
> > 2001 From: Julien Lepiller <julien@lepiller.eu>
> > Date: Mon, 26 Sep 2016 23:55:58 +0200
> > Subject: [PATCH] services: improve nginx-service configuration
> >
> > * gnu/services/web.scm (<nginx-vhost-configuration>): New record
> > type. (config-domain-strings): New procedure.
> > (config-index-strings): New procedure.
> > (default-nginx-vhost-config): New procedure.
> > (default-nginx-config): Add vhost support and temporary directories
> > (nginx-activation): Create temporary directories
> > (nginx-service): Add vhost-list key.
> > * doc/guix.texi (Web Services): Document
> > 'nginx-vhost-configuration'.  
> 
> Applied with the minor changes below.
> 
> Thank you!
> 
> Ludo’.
> 


[-- Attachment #2: 0001-services-Make-nginx-service-type-extensible.patch --]
[-- Type: text/x-patch, Size: 7438 bytes --]

From 13748b6bfffef19080f4fa3bde2a6ae7d5c8d067 Mon Sep 17 00:00:00 2001
From: Julien Lepiller <julien@lepiller.eu>
Date: Wed, 26 Oct 2016 21:33:34 +0200
Subject: [PATCH] services: Make nginx-service-type extensible

gnu/services/web.scm (nginx-service): Removed.
gnu/services/web.scm (nginx-service-sytsem): Make extensible.
gnu/services/web.scm (default-nginx-vhost-service-type): New variable.
---
 gnu/services/web.scm | 87 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 53 insertions(+), 34 deletions(-)

diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 59e1e54..0f06c99 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -28,12 +28,13 @@
   #:use-module (guix records)
   #:use-module (guix gexp)
   #:use-module (ice-9 match)
+  #:use-module (srfi srfi-1)
   #:export (nginx-configuration
             nginx-configuration?
             nginx-vhost-configuration
             nginx-vhost-configuration?
-            nginx-service
-            nginx-service-type))
+            nginx-service-type
+            default-nginx-vhost-service-type))
 
 ;;; Commentary:
 ;;;
@@ -64,10 +65,14 @@
 (define-record-type* <nginx-configuration>
   nginx-configuration make-nginx-configuration
   nginx-configuration?
-  (nginx         nginx-configuration-nginx)         ;<package>
-  (log-directory nginx-configuration-log-directory) ;string
-  (run-directory nginx-configuration-run-directory) ;string
-  (file          nginx-configuration-file))         ;string | file-like
+  (nginx         nginx-configuration-nginx         ;<package>
+                 (default nginx))
+  (log-directory nginx-configuration-log-directory ;string
+                 (default "/var/log/nginx"))
+  (run-directory nginx-configuration-run-directory ;string
+                 (default "/var/run/nginx"))
+  (vhosts        nginx-configuration-vhosts        ;list
+                 (default (list (nginx-vhost-configuration (https-port #f))))))
 
 (define (config-domain-strings names)
  "Return a string denoting the nginx config representation of NAMES, a list
@@ -102,11 +107,15 @@ of index files."
    "      server_name " (config-domain-strings
                          (nginx-vhost-configuration-server-name vhost))
                         ";\n"
-   (if (nginx-vhost-configuration-ssl-certificate vhost)
+   (if (and 
+         (nginx-vhost-configuration-ssl-certificate vhost)
+         (nginx-vhost-configuration-https-port vhost))
        (string-append "      ssl_certificate "
                       (nginx-vhost-configuration-ssl-certificate vhost) ";\n")
        "")
-   (if (nginx-vhost-configuration-ssl-certificate-key vhost)
+   (if (and
+         (nginx-vhost-configuration-ssl-certificate-key vhost)
+         (nginx-vhost-configuration-https-port vhost))
        (string-append "      ssl_certificate_key "
                       (nginx-vhost-configuration-ssl-certificate-key vhost) ";\n")
        "")
@@ -116,7 +125,7 @@ of index files."
                               "on" "off") ";\n"
    "    }\n"))
 
-(define (default-nginx-config log-directory run-directory vhost-list)
+(define (nginx-config-file log-directory run-directory vhost-list)
   (plain-file "nginx.conf"
               (string-append
                "user nginx nginx;\n"
@@ -148,7 +157,7 @@ of index files."
 
 (define nginx-activation
   (match-lambda
-    (($ <nginx-configuration> nginx log-directory run-directory config-file)
+    (($ <nginx-configuration> nginx log-directory run-directory vhosts)
      #~(begin
          (use-modules (guix build utils))
 
@@ -161,20 +170,20 @@ of index files."
          (mkdir-p (string-append #$run-directory "/proxy_temp"))
          (mkdir-p (string-append #$run-directory "/fastcgi_temp"))
          (mkdir-p (string-append #$run-directory "/uwsgi_temp"))
-         (mkdir-p (string-append #$run-directory "/scgi_temp"))
-         ;; Check configuration file syntax.
-         (system* (string-append #$nginx "/sbin/nginx")
-                  "-c" #$config-file "-t")))))
+         (mkdir-p (string-append #$run-directory "/scgi_temp"))))))
 
 (define nginx-shepherd-service
   (match-lambda
-    (($ <nginx-configuration> nginx log-directory run-directory config-file)
+    (($ <nginx-configuration> nginx log-directory run-directory vhosts)
      (let* ((nginx-binary (file-append nginx "/sbin/nginx"))
             (nginx-action
              (lambda args
                #~(lambda _
                    (zero?
-                    (system* #$nginx-binary "-c" #$config-file #$@args))))))
+                    (system* #$nginx-binary "-c" #$(nginx-config-file
+                                                     log-directory
+                                                     run-directory
+                                                     vhosts) #$@args))))))
 
        ;; TODO: Add 'reload' action.
        (list (shepherd-service
@@ -192,21 +201,31 @@ of index files."
                        (service-extension activation-service-type
                                           nginx-activation)
                        (service-extension account-service-type
-                                          (const %nginx-accounts))))))
-
-(define* (nginx-service #:key (nginx nginx)
-                        (log-directory "/var/log/nginx")
-                        (run-directory "/var/run/nginx")
-                        (vhost-list (list (nginx-vhost-configuration)))
-                        (config-file
-                         (default-nginx-config log-directory run-directory vhost-list)))
-  "Return a service that runs NGINX, the nginx web server.
-
-The nginx daemon loads its runtime configuration from CONFIG-FILE, stores log
-files in LOG-DIRECTORY, and stores temporary runtime files in RUN-DIRECTORY."
-  (service nginx-service-type
-           (nginx-configuration
-            (nginx nginx)
-            (log-directory log-directory)
-            (run-directory run-directory)
-            (file config-file))))
+                                          (const %nginx-accounts))))
+                (compose concatenate)
+                (extend (lambda (config additional-vhosts)
+                          (match config
+                            (($ <nginx-configuration> nginx log-directory
+                                                      run-directory vhosts)
+                             (nginx-configuration
+                               (nginx nginx)
+                               (log-directory log-directory)
+                               (run-directory run-directory)
+                               (vhosts (append vhosts additional-vhosts)))))))))
+
+(define (default-nginx-vhost-configuration root)
+ (lambda (_)
+   (list (nginx-vhost-configuration (https-port #f)
+                                    (root root)))))
+
+(define default-nginx-vhost-service-type
+  (let* ((index (plain-file "index.html"
+                  "<!DOCTYPE html><html><head></head><body></body></html>"))
+         (root (computed-file "default-nginx-vhost"
+                 #~(begin
+                     (mkdir #$output)
+                     (symlink #$index (string-append #$output "/index.html"))))))
+    (service-type (name 'default-nginx-vhost)
+                  (extensions
+                   (list (service-extension nginx-service-type
+                          (default-nginx-vhost-configuration root)))))))
-- 
2.10.1