Patchwork improve nginx-service

login
register
mail settings
Submitter Julien Lepiller
Date Nov. 4, 2016, 6:01 p.m.
Message ID <20161104190129.684844b4@lepiller.eu>
Download mbox | patch
Permalink /patch/17199/
State New
Headers show

Comments

Julien Lepiller - Nov. 4, 2016, 6:01 p.m.
On Fri, 04 Nov 2016 14:21:43 +0100
ludo@gnu.org (Ludovic Courtès) wrote:

> Hartmut Goebel <h.goebel@crazy-compilers.com> skribis:
> 
> > Am 03.11.2016 um 15:54 schrieb Ludovic Courtès:  
> >> Assuming ‘demo-website’ is bound to a package object, you should
> >> be able to do:
> >>
> >>   (root (file-append demo-website "/"))
> >>
> >> or simply:
> >>
> >>   (root demo-website)  
> >
> > Hmm, this does not work for me:
> >
> >
> > ice-9/boot-9.scm:702:27: In procedure map:
> > ice-9/boot-9.scm:702:27: In procedure string-append: Wrong type
> > (expecting string): #<package
> > taler-demo-landing-page@0.0.0-1.105bea68 ./taler/packages/demo.scm:100
> > 3e2a600>
> >
> >
> > I tried with current master branch.  
> 
> Doh!  Indeed, ‘default-nginx-vhost-config’ must be rewritten to do
> something like:
> 
>   #~(string-append #$foo #$bar …)
> 
> instead of:
> 
>   (string-append foo bar …)
> 
> More precisely, it could ‘string-append’ all the string literals yet
> use a #~(string-append …) gexp for ‘root’.
> 
> Julien?  :-)

I think it's good now. There are three patches:
1. making the service extensible (mostly what you wrote last time)
   I used an empty list as the default list of vhosts.
2. a patch to fix an issue when multiple index files and server name
   were specified (there were no space in between)
3. a patch to allow any configuration option to come from the store (I
   couldn't write something specific to the root option, but I think
   it's fine this way). I tested with the example I added in the
   documentation, and it works fine.

I hope I did it right :)

> 
> Ludo’.
Hartmut Goebel - Nov. 4, 2016, 9:28 p.m.
Hi Julien

thanks for these patches. I applied them to current master, but I did
not work out how to use the new features. I'd appreciate a more complete
example.

0001-Make-nginx-service-extensible.patch
> +@deffn {Scheme Variable} nginx-service-type +This is the type for the
> nginx web server. + +This service can be extended to add more vhosts
> than the default one. + +@example +(simple-service 'my-extra-vhost
> nginx-service-type + (list (nginx-vhost-configuration (https-port #f)
> + (root "/var/www/extra-website"))))

I do not understand this. Why would I want to to this? Why not just add
the vhost declaration when defining the service in the system
declaration? Is this for vhost-types which have preset values for some
configuration files? Then a different example might be useful, and/or
some more explanation.

It's a bit more obvious with patch 3, but even that example is not that
obvious to me and could use some more explanation.

I've build a simple service like the gtk-doc-vhost in your second
example, but it not manage to make it work: First I got "error: no
target of type 'nginx'", so I re-added "(nginx-service)" to the system
declaration. Then I got

    gnu/services/web.scm:118:34: In procedure default-nginx-vhost-config:
    gnu/services/web.scm:118:34: In procedure struct_vtable: Wrong type
    argument in position 1 (expecting struct):
    (nginx-vhost-configuration (root taler-landing-page))

Si I'd really appreciate seeing a more complete example either in the
documentation of in gn/systems/examples/

> (system* (string-append #$nginx "/sbin/nginx") - "-c" #$config-file
> "-t"))))) + "-c" #$(or config-file + (default-nginx-config
> log-directory + run-directory vhosts)) + "-t")))))

Nitpicking: I'd mode the "-t" to the front, since this is the important
difference.

> (define nginx-shepherd-service (match-lambda - (($
> <nginx-configuration> nginx log-directory run-directory config-file) +
> (($ <nginx-configuration> nginx log-directory run-directory vhosts
> config-file) (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" #$(or config-file + (default-nginx-config +
> log-directory + run-directory + vhosts))+ #$@args))))))

To avoid duplicate code I suggest moving the "#$(or …)" part into a
private function - if this is possible.



0003-services-Accept-gexps-as-nginx-configuration-value.patch
> +@example +(simple-service 'gtk-doc-vhost nginx-service-type + (list
> (nginx-vhost-configuration

git am says: trailing whitespace

> + " root " #$(nginx-vhost-configuration-root vhost) ";\n" + " index "
> #$(config-index-strings (nginx-vhost-configuration-index vhost)) ";\n"
> + " server_tokens " #$(if (nginx-vhost-configuration-server-tokens?
> vhost) + "on" "off") ";\n"

Could you please (maybe in another patch) add a way to include
additional config lines? Both for the main server and each vhost.I'm
gioing to need this for adding locations, backends and such.

-- Regards Hartmut Goebel | Hartmut Goebel |
h.goebel@crazy-compilers.com | | www.crazy-compilers.com | compilers
which you thought are impossible |
Julien Lepiller - Nov. 4, 2016, 10:12 p.m.
On Fri, 4 Nov 2016 22:28:07 +0100
Hartmut Goebel <h.goebel@crazy-compilers.com> wrote:

> Hi Julien
> 
> thanks for these patches. I applied them to current master, but I did
> not work out how to use the new features. I'd appreciate a more
> complete example.
> 
> 0001-Make-nginx-service-extensible.patch
> > +@deffn {Scheme Variable} nginx-service-type +This is the type for
> > the nginx web server. + +This service can be extended to add more
> > vhosts than the default one. + +@example +(simple-service
> > 'my-extra-vhost nginx-service-type + (list
> > (nginx-vhost-configuration (https-port #f)
> > + (root "/var/www/extra-website"))))  
> 
> I do not understand this. Why would I want to to this? Why not just
> add the vhost declaration when defining the service in the system
> declaration? Is this for vhost-types which have preset values for some
> configuration files? Then a different example might be useful, and/or
> some more explanation.

There is no real difference between the two. This is mostly usefull for
implementing new web services in the distro, but it is also good for
separating nginx configuration from web service configuration. I guess
that's a matter of taste.

> 
> It's a bit more obvious with patch 3, but even that example is not
> that obvious to me and could use some more explanation.
> 
> I've build a simple service like the gtk-doc-vhost in your second
> example, but it not manage to make it work: First I got "error: no
> target of type 'nginx'", so I re-added "(nginx-service)" to the system
> declaration. Then I got

Yes, the example adds a service that uses the nginx service, but does
not create it.

> 
>     gnu/services/web.scm:118:34: In procedure
> default-nginx-vhost-config: gnu/services/web.scm:118:34: In procedure
> struct_vtable: Wrong type argument in position 1 (expecting struct):
>     (nginx-vhost-configuration (root taler-landing-page))

What is your configuration exactly, and what is taler-landing-page?

> 
> Si I'd really appreciate seeing a more complete example either in the
> documentation of in gn/systems/examples/
> 
> > (system* (string-append #$nginx "/sbin/nginx") - "-c" #$config-file
> > "-t"))))) + "-c" #$(or config-file + (default-nginx-config
> > log-directory + run-directory vhosts)) + "-t")))))  
> 
> Nitpicking: I'd mode the "-t" to the front, since this is the
> important difference.
> 
> > (define nginx-shepherd-service (match-lambda - (($
> > <nginx-configuration> nginx log-directory run-directory
> > config-file) + (($ <nginx-configuration> nginx log-directory
> > run-directory vhosts config-file) (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" #$(or config-file +
> > (default-nginx-config + log-directory + run-directory + vhosts))+
> > #$@args))))))  
> 
> To avoid duplicate code I suggest moving the "#$(or …)" part into a
> private function - if this is possible.
> 
> 
> 
> 0003-services-Accept-gexps-as-nginx-configuration-value.patch
> > +@example +(simple-service 'gtk-doc-vhost nginx-service-type + (list
> > (nginx-vhost-configuration  
> 
> git am says: trailing whitespace
> 
> > + " root " #$(nginx-vhost-configuration-root vhost) ";\n" + " index
> > " #$(config-index-strings (nginx-vhost-configuration-index vhost))
> > ";\n"
> > + " server_tokens " #$(if (nginx-vhost-configuration-server-tokens?
> > vhost) + "on" "off") ";\n"  
> 
> Could you please (maybe in another patch) add a way to include
> additional config lines? Both for the main server and each vhost.I'm
> gioing to need this for adding locations, backends and such.

I'd like to get these patches accepted first, but I'm already working
on adding more configuration lines.

Thanks for your review, I will fix that and come with a new patchset
quickly (with better documentation).

> 
> -- Regards Hartmut Goebel | Hartmut Goebel |
> h.goebel@crazy-compilers.com | | www.crazy-compilers.com | compilers
> which you thought are impossible |
> 
> 
>
Hartmut Goebel - Nov. 4, 2016, 10:34 p.m.
Am 04.11.2016 um 23:12 schrieb Julien Lepiller:
>> >     gnu/services/web.scm:118:34: In procedure
>> > default-nginx-vhost-config: gnu/services/web.scm:118:34: In procedure
>> > struct_vtable: Wrong type argument in position 1 (expecting struct):
>> >     (nginx-vhost-configuration (root taler-landing-page))
> What is your configuration exactly, and what is taler-landing-page?
>

The configuration is like in your gtk-doc-vhost, except that I'm not
using gtk+2 and omitting the "string-append" for the path.
Hartmut Goebel - Nov. 4, 2016, 10:58 p.m.
Am 04.11.2016 um 23:12 schrieb Julien Lepiller:
> I'd like to get these patches accepted first, but I'm already working
> on adding more configuration lines.

BTW: I just started typing down a simple "location" record type, and
started to think if these record types are really the way to go for the
configuration.

In nginx AFAIK you can define many options on the server level, on the
vhost level and per location. Additionally locations can be nested. And
some configuration options are quite complex, e.g. "listen" [1] and
maybe worth their own type. So we'll create a lot of code for both the
record types and for converting them to config-file text.

In the long run, we will get maintenance problems, I'm afraid, if we are
only allow options known to us and written into the service. On each
update we'd need to check if any option was added.

So IMHO the recard types are not the way to go. We need a more flexible
way, maybe like used for XML (see default-build.xml in
guix/build/ant-build-system.scm), but with support for default values.
Unfortunatly I'm no scheme hacker and can not propose any solution for
this :-(

[1] https://nginx.org/en/docs/http/ngx_http_core_module.html#listen
Tobias Geerinckx-Rice - Nov. 6, 2016, 12:18 p.m.
Hullo Julien,

(Sorry for replying at a random point in the thread — it's the only part
I have in this mailbox.)

On 04/11/16 23:58, Hartmut Goebel wrote:
> Am 04.11.2016 um 23:12 schrieb Julien Lepiller:
>> I'd like to get these patches accepted first, but I'm already 
>> working on adding more configuration lines.
> 
> [...]
> 
> In nginx AFAIK you can define many options on the server level, on 
> the vhost level and per location.

I've written quite a few nginx.confs over the years (too many — a Guixy
front-end would be great!) and have never encountered the term ‘vhost’.
Of course, that proves nothing.

However, a web search for ‘vhost site:nginx.org’ returns the following
as a first result[0]:

   Note: “VirtualHost” is an Apache term. NGINX does not have Virtual
   hosts, it has “Server Blocks” that use the server_name and listen
   directives to bind to tcp sockets.

I don't use Apache, so that explains that.

Kind regards,

T G-R

[0]:
https://www.nginx.com/resources/wiki/start/topics/examples/server_blocks/
Ludovic Courtès - Nov. 6, 2016, 5:19 p.m.
Hi,

Tobias Geerinckx-Rice <me@tobias.gr> skribis:

> However, a web search for ‘vhost site:nginx.org’ returns the following
> as a first result[0]:
>
>    Note: “VirtualHost” is an Apache term. NGINX does not have Virtual
>    hosts, it has “Server Blocks” that use the server_name and listen
>    directives to bind to tcp sockets.
>
> I don't use Apache, so that explains that.

Oh, good to know.  In general I think it’s best to stick to upstream’s
terminology.

Julien, what would you think of changing “virtual host” with “server
blocks” and “vhost” with “server-block” (?) in the code and
documentation?

(This is not a fun suggestion to make, but hey!)

Ludo’.
Ludovic Courtès - Nov. 6, 2016, 5:33 p.m.
Hartmut Goebel <h.goebel@crazy-compilers.com> skribis:

> BTW: I just started typing down a simple "location" record type, and
> started to think if these record types are really the way to go for the
> configuration.
>
> In nginx AFAIK you can define many options on the server level, on the
> vhost level and per location. Additionally locations can be nested. And
> some configuration options are quite complex, e.g. "listen" [1] and
> maybe worth their own type. So we'll create a lot of code for both the
> record types and for converting them to config-file text.
>
> In the long run, we will get maintenance problems, I'm afraid, if we are
> only allow options known to us and written into the service. On each
> update we'd need to check if any option was added.

You’re right: we never want our Scheme interface to config files to
be less expressive than the original config syntax.

Yet, having a Scheme interface to config files is valuable: people don’t
have to learn a new syntax, you get compile-time checks of field names,
it’s easier to customize config represented as a Scheme object, etc.

So there’s a tension here.

So far, there’s no single approach to this problem in GuixSD.  In some
cases we don’t provide a Scheme interface at all (e.g., syslogd), which
isn’t great.  In other cases (e.g., nginx, Tor), we provide a partial
Scheme interface along with a way to augment the config file using its
native syntax.  In yet other cases (CUPS, Dovecot, added by Andy, or
libc’s NSS and PAM), services have a Scheme interface that cover 100% of
their current config space, sometimes with the added ability to provide
raw inline config.

I think this last approach is the preferred approach, but it’s also more
difficult to get right in some cases.  For instance, nginx config is not
just about key/value arguments: it has blocks, a notion of scope, etc.
So nginx config syntax and semantics is more complicated than key/value
configs such as that of Dovecot or Tor.

> So IMHO the recard types are not the way to go. We need a more flexible
> way, maybe like used for XML (see default-build.xml in
> guix/build/ant-build-system.scm), but with support for default values.

We could obviously use s-expressions for everything (they are
essentially typed XML).  However, people would get zero compile-time
checks and it would also be much harder (and less efficient) to deal
with them.

Ludo’.
Julien Lepiller - Nov. 20, 2016, 12:49 p.m.
On Sun, 06 Nov 2016 18:19:03 +0100
ludo@gnu.org (Ludovic Courtès) wrote:

> Hi,
> 
> Tobias Geerinckx-Rice <me@tobias.gr> skribis:
> 
> > However, a web search for ‘vhost site:nginx.org’ returns the
> > following as a first result[0]:
> >
> >    Note: “VirtualHost” is an Apache term. NGINX does not have
> > Virtual hosts, it has “Server Blocks” that use the server_name and
> > listen directives to bind to tcp sockets.
> >
> > I don't use Apache, so that explains that.  
> 
> Oh, good to know.  In general I think it’s best to stick to upstream’s
> terminology.
> 
> Julien, what would you think of changing “virtual host” with “server
> blocks” and “vhost” with “server-block” (?) in the code and
> documentation?
Ok, sorry for the long delay, I was working on php and other things. So
I've been thinking that we should probably stick more to the way you
would write an nginx configuration file, and have an interface to it.
So here is a web.scm file that implements just that. I have a record
type for configuration entries, and record types for complex
configuration types and blocks. Since many blocks can get the same
configuration option (for instance, http, server and location blocks
can get the "root" option), I define a single procedure that returns
the string corresponding to the configuration line. So for instance,
you could write this:

(nginx-service)
(service (service-type
  (name 'foo)
  (extensions
    (list
      (service-extension
        nginx-service-type
         (const (list (nginx-block-server
                        (blocks (list))
                        (configs (list
                                  (nginx-option (type 'server_name)
                                                (value (list 'default)))
                                  (nginx-option (type 'listen)
                                                (value (nginx-listen)))
                                  (nginx-option (type 'root)
                                                (value "/srv/http"))
                                  (nginx-option (type 'index)
                                                (value (list
                                                  "index.html"))))))))))))

As you can see, it's still a bit verbose, but we could provide a few
helper functions for some common cases.

Also, it is now possible to extend the nginx service with other kind of
blocks (although I implemented only the server block, it could be use
to add upstream blocks for instance).

What do you think? should I continue in that direction, or should I go
back to what I was doing before?

> 
> (This is not a fun suggestion to make, but hey!)
> 
> Ludo’.
Hartmut Goebel - Nov. 22, 2016, 10:20 p.m.
Am 20.11.2016 um 13:49 schrieb Julien Lepiller:
> What do you think? should I continue in that direction, or should I go
> back to what I was doing before?

Thanks for working on this. I will give it a try the next days. But I
want to share two points going around in my head:

1) I propose moving the nginx service into a package
gnu/service/nginx.scm. Imagine we will implement an equal complex config
schema for apache. Then we will get a beast of code with lots of useless
"nginx-" prefixes.

2) Is it possible to add some magic to e.g. "nginx-service-type" so that
all identifiers within are searched in the "nginx" scope. Example:

      (service-extension
        nginx-service-type
         (const (list (server    ;; taken from "nginx" scope
                        (blocks (list))
                        (configs (list
                                  (option (type 'server_name)     ;; taken from "nginx" scope
                                          (value (list 'default)))
Julien Lepiller - Nov. 23, 2016, 9:26 a.m.
Le 2016-11-22 23:20, Hartmut Goebel a écrit :
> Am 20.11.2016 um 13:49 schrieb Julien Lepiller:
>> What do you think? should I continue in that direction, or should I go
>> back to what I was doing before?
> 
> Thanks for working on this. I will give it a try the next days. But I
> want to share two points going around in my head:
> 
> 1) I propose moving the nginx service into a package
> gnu/service/nginx.scm. Imagine we will implement an equal complex 
> config
> schema for apache. Then we will get a beast of code with lots of 
> useless
> "nginx-" prefixes.
> 
> 2) Is it possible to add some magic to e.g. "nginx-service-type" so 
> that
> all identifiers within are searched in the "nginx" scope. Example:
> 
>       (service-extension
>         nginx-service-type
>          (const (list (server    ;; taken from "nginx" scope
>                         (blocks (list))
>                         (configs (list
>                                   (option (type 'server_name)     ;;
> taken from "nginx" scope
>                                           (value (list 'default)))

I am quite new to scheme, but as far as I can tell, there is no notion 
of scope. Here, (option ...) can be independent from nginx or apache 
(because there is nothing specific), but probably not (server), because 
it is a notion specific to nginx and because what it can contain is 
different in the apache world and nginx world. Also, how would the two 
types be recognized when you want both apache and nginx (probably not 
useful, but it may happen)?

I've been rethinking it, and I would like to use define-configuration 
(from cups and dovecot) because it looks really good. But I don't want 
to define the same config option for each possible block it can appear 
in, and that's why I used (option). So I'm looking for a way to 
dynamically define records for each block type, so they can be used more 
consistently with what we have. Unfortunately, I don't see how I could 
do that, if that is possible at all... Ludo, any idea? I'd like to be 
able to write:

(define-record-type* <nginx-option>
   ...)

(define-syntax option
   (syntax-rule ()
     (option mname mtype mdef mdoc mblocks))
     (nginx-option (name mname) (type mtype) ...)))

(define option-list (list
   (option 'server-name server-name 'default "the name of the server that 
is served by the http block" (list 'http))
   ...))

and then be able to create the block records by filtering that list:

(define-nginx-configuration nginx-http-block
   (filter ... option-list))

(define-nginx-configuration nginx-events-block
   (filter ... option-list))

(define-nginx-configuration nginx-server-block
   (filter ... option-list))

So the user would then be able to write the service configuration as 
they would for any other service we have. What I don't know is how to 
write define-nginx-configuration (ideally it would call 
define-configuration). With it, I can probably do the rest just fine. Or 
maybe there is a better way I don't see yet?
Clément Lassieur - Nov. 25, 2016, 10:53 a.m.
> I've been rethinking it, and I would like to use define-configuration 
> (from cups and dovecot) because it looks really good. But I don't want 
> to define the same config option for each possible block it can appear 
> in, and that's why I used (option). So I'm looking for a way to 
> dynamically define records for each block type, so they can be used more 
> consistently with what we have. Unfortunately, I don't see how I could 
> do that, if that is possible at all... Ludo, any idea? I'd like to be 
> able to write:
>
> (define-record-type* <nginx-option>
>    ...)
>
> (define-syntax option
>    (syntax-rule ()
>      (option mname mtype mdef mdoc mblocks))
>      (nginx-option (name mname) (type mtype) ...)))
>
> (define option-list (list
>    (option 'server-name server-name 'default "the name of the server that 
> is served by the http block" (list 'http))
>    ...))
>
> and then be able to create the block records by filtering that list:
>
> (define-nginx-configuration nginx-http-block
>    (filter ... option-list))
>
> (define-nginx-configuration nginx-events-block
>    (filter ... option-list))
>
> (define-nginx-configuration nginx-server-block
>    (filter ... option-list))
>
> So the user would then be able to write the service configuration as 
> they would for any other service we have. What I don't know is how to 
> write define-nginx-configuration (ideally it would call 
> define-configuration). With it, I can probably do the rest just fine. Or 
> maybe there is a better way I don't see yet?

I think you are looking for "eval".

(define (list->define-configuration stem fields)
  (eval `(define-configuration ,stem ,@fields) (current-module)))

(list->define-configuration 'some-configuration filtered-list)

I use it in a service I'm working on (Prosody), to handle virtualhosts.
Hartmut Goebel - Nov. 25, 2016, 11:46 a.m.
Am 25.11.2016 um 11:53 schrieb Clément Lassieur:
> I think you are looking for "eval".
>
> (define (list->define-configuration stem fields)
>   (eval `(define-configuration ,stem ,@fields) (current-module)))

I'm curious: In other programming languages, using eval is regarded bad
programming style. Is this different in guile?
Andy Wingo - Nov. 25, 2016, 1:29 p.m.
On Fri 25 Nov 2016 12:46, Hartmut Goebel <h.goebel@goebel-consult.de> writes:

> Am 25.11.2016 um 11:53 schrieb Clément Lassieur:
>> I think you are looking for "eval".
>>
>> (define (list->define-configuration stem fields)
>>   (eval `(define-configuration ,stem ,@fields) (current-module)))
>
> I'm curious: In other programming languages, using eval is regarded bad
> programming style. Is this different in guile?

No.  Eval is generally bad style :)  If it can be avoided it should be
avoided.  I think since this is a definition form, macros are the
appropriate thing if possible (I haven't seen the larger patch).

Andy
Clément Lassieur - Nov. 26, 2016, 9:55 p.m.
Andy Wingo <wingo@igalia.com> writes:

> On Fri 25 Nov 2016 12:46, Hartmut Goebel <h.goebel@goebel-consult.de> writes:
>
>> Am 25.11.2016 um 11:53 schrieb Clément Lassieur:
>>> I think you are looking for "eval".
>>>
>>> (define (list->define-configuration stem fields)
>>>   (eval `(define-configuration ,stem ,@fields) (current-module)))
>>
>> I'm curious: In other programming languages, using eval is regarded bad
>> programming style. Is this different in guile?
>
> No.  Eval is generally bad style :)  If it can be avoided it should be
> avoided.  I think since this is a definition form, macros are the
> appropriate thing if possible (I haven't seen the larger patch).

Oh sorry for that then. The larger patch is there:
http://lists.gnu.org/archive/html/guix-devel/2016-11/msg01048.html
Ludovic Courtès - Nov. 27, 2016, 9:01 p.m.
Hi, and sorry for the delay!

julien lepiller <julien@lepiller.eu> skribis:

> Le 2016-11-22 23:20, Hartmut Goebel a écrit :
>> Am 20.11.2016 um 13:49 schrieb Julien Lepiller:
>>> What do you think? should I continue in that direction, or should I go
>>> back to what I was doing before?
>>
>> Thanks for working on this. I will give it a try the next days. But I
>> want to share two points going around in my head:
>>
>> 1) I propose moving the nginx service into a package
>> gnu/service/nginx.scm. Imagine we will implement an equal complex
>> config
>> schema for apache. Then we will get a beast of code with lots of
>> useless
>> "nginx-" prefixes.
>>
>> 2) Is it possible to add some magic to e.g. "nginx-service-type" so
>> that
>> all identifiers within are searched in the "nginx" scope. Example:
>>
>>       (service-extension
>>         nginx-service-type
>>          (const (list (server    ;; taken from "nginx" scope
>>                         (blocks (list))
>>                         (configs (list
>>                                   (option (type 'server_name)     ;;
>> taken from "nginx" scope
>>                                           (value (list 'default)))
>
> I am quite new to scheme, but as far as I can tell, there is no notion
> of scope. Here, (option ...) can be independent from nginx or apache
> (because there is nothing specific), but probably not (server),
> because it is a notion specific to nginx and because what it can
> contain is different in the apache world and nginx world. Also, how
> would the two types be recognized when you want both apache and nginx
> (probably not useful, but it may happen)?
>
> I've been rethinking it, and I would like to use define-configuration
> (from cups and dovecot) because it looks really good. But I don't want
> to define the same config option for each possible block it can appear
> in, and that's why I used (option). So I'm looking for a way to
> dynamically define records for each block type, so they can be used
> more consistently with what we have. Unfortunately, I don't see how I
> could do that, if that is possible at all... Ludo, any idea? I'd like
> to be able to write:
>
> (define-record-type* <nginx-option>
>   ...)
>
> (define-syntax option
>   (syntax-rule ()
>     (option mname mtype mdef mdoc mblocks))
>     (nginx-option (name mname) (type mtype) ...)))
>
> (define option-list (list
>   (option 'server-name server-name 'default "the name of the server
> that is served by the http block" (list 'http))
>   ...))
>
> and then be able to create the block records by filtering that list:
>
> (define-nginx-configuration nginx-http-block
>   (filter ... option-list))
>
> (define-nginx-configuration nginx-events-block
>   (filter ... option-list))
>
> (define-nginx-configuration nginx-server-block
>   (filter ... option-list))
>
> So the user would then be able to write the service configuration as
> they would for any other service we have. What I don't know is how to
> write define-nginx-configuration (ideally it would call
> define-configuration). With it, I can probably do the rest just
> fine. Or maybe there is a better way I don't see yet?

I’ve played a bit with nginx and I think its configuration syntax is far
from trivial.  Services for Dovecot, Kerberos, glibc’s name service
switch, PAM, etc. are pretty clear: it’s quite easy to find out what
data structures the config defines, and to write Scheme bindings for
those structures.

In the case of nginx, it’s much more complex: there’s a notion of scope
and inheritance (some key/value pairs are inherited from the “parent”
context), there are conditionals, variables, keys associated with more
than one value, etc.  Tricky.

So there are two ways to approach it:

  1. Choose a small subset of the functionality, like you’ve done with
     vhosts, and provide bindings for that subset.  It’ll be less
     expressive than nginx’s config files (so users need a way to
     provide nginx-language config when needed), but it can still be
     useful.  And it’s not too hard to implement.

  2. Write bindings for the whole configuration language, such that
     there is no loss of expressivity.  This is the grail, but obviously
     it requires a very good understanding of that language, and it’s
     going to be much more work.

I think it would make sense to stick to #1 in the short- to medium term.
But if you’re an nginx wizard, you might find that #2 is piece of cake;
you’ll have to decide!  :-)

Back to the original comment about “vhost”, I think we could simply
replace “vhost” with “server” in the code, since those
<nginx-vhost-configuration> objects map to ‘server’ blocks.

WDYT?

Thanks,
Ludo’.

Patch

From a1305d1280642bcff46fc736717f119bd4999e74 Mon Sep 17 00:00:00 2001
From: Julien Lepiller <julien@lepiller.eu>
Date: Fri, 4 Nov 2016 17:36:21 +0100
Subject: [PATCH 3/3] services: Accept gexps as nginx configuration value.

* gnu/services/web.scm (default-nginx-config): Use computed-file instead of plain-file.
---
 doc/guix.texi        |  8 +++++
 gnu/services/web.scm | 96 +++++++++++++++++++++++++++-------------------------
 2 files changed, 58 insertions(+), 46 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 5c42140..2b07bc4 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -10428,6 +10428,14 @@  This service can be extended to add more vhosts than the default one.
                                                  (root "/var/www/extra-website"))))
 @end example
 
+or if you want to make a package available, you can write something like:
+
+@example
+(simple-service 'gtk-doc-vhost nginx-service-type
+                (list (nginx-vhost-configuration 
+                       (root (file-append gtk+2 "/share/gtk-doc/html/gdk2/")))))
+@end example
+
 @end deffn
 
 @deftp {Data Type} nginx-vhost-configuration
diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index e36d284..df6e680 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -90,54 +90,58 @@  of index files."
        names)))
 
 (define (default-nginx-vhost-config vhost)
-  (string-append
-   "    server {\n"
-   (if (nginx-vhost-configuration-http-port vhost)
-       (string-append "      listen "
-                      (number->string (nginx-vhost-configuration-http-port vhost))
-                      ";\n")
-       "")
-   (if (nginx-vhost-configuration-https-port vhost)
-       (string-append "      listen "
-                      (number->string (nginx-vhost-configuration-https-port vhost))
-                      " ssl;\n")
-       "")
-   "      server_name " (config-domain-strings
-                         (nginx-vhost-configuration-server-name vhost))
-                        ";\n"
-   (if (nginx-vhost-configuration-ssl-certificate vhost)
-       (string-append "      ssl_certificate "
-                      (nginx-vhost-configuration-ssl-certificate vhost) ";\n")
-       "")
-   (if (nginx-vhost-configuration-ssl-certificate-key vhost)
-       (string-append "      ssl_certificate_key "
-                      (nginx-vhost-configuration-ssl-certificate-key vhost) ";\n")
-       "")
-   "      root " (nginx-vhost-configuration-root vhost) ";\n"
-   "      index " (config-index-strings (nginx-vhost-configuration-index vhost)) ";\n"
-   "      server_tokens " (if (nginx-vhost-configuration-server-tokens? vhost)
-                              "on" "off") ";\n"
-   "    }\n"))
+  #~(string-append
+     "    server {\n"
+     #$(if (nginx-vhost-configuration-http-port vhost)
+           (string-append "      listen "
+                          (number->string (nginx-vhost-configuration-http-port vhost))
+                          ";\n")
+           "")
+     #$(if (nginx-vhost-configuration-https-port vhost)
+          (string-append "      listen "
+                          (number->string (nginx-vhost-configuration-https-port vhost))
+                          " ssl;\n")
+           "")
+     "      server_name " #$(config-domain-strings
+                             (nginx-vhost-configuration-server-name vhost))
+                          ";\n"
+     #$(if (nginx-vhost-configuration-ssl-certificate vhost)
+           (string-append "      ssl_certificate "
+                          (nginx-vhost-configuration-ssl-certificate vhost) ";\n")
+           "")
+     #$(if (nginx-vhost-configuration-ssl-certificate-key vhost)
+           (string-append "      ssl_certificate_key "
+                          (nginx-vhost-configuration-ssl-certificate-key vhost) ";\n")
+           "")
+     "      root " #$(nginx-vhost-configuration-root vhost) ";\n"
+     "      index " #$(config-index-strings (nginx-vhost-configuration-index vhost)) ";\n"
+     "      server_tokens " #$(if (nginx-vhost-configuration-server-tokens? vhost)
+                                  "on" "off") ";\n"
+     "    }\n"))
 
 (define (default-nginx-config log-directory run-directory vhost-list)
-  (plain-file "nginx.conf"
-              (string-append
-               "user nginx nginx;\n"
-               "pid " run-directory "/pid;\n"
-               "error_log " log-directory "/error.log info;\n"
-               "http {\n"
-               "    client_body_temp_path " run-directory "/client_body_temp;\n"
-               "    proxy_temp_path " run-directory "/proxy_temp;\n"
-               "    fastcgi_temp_path " run-directory "/fastcgi_temp;\n"
-               "    uwsgi_temp_path " run-directory "/uwsgi_temp;\n"
-               "    scgi_temp_path " run-directory "/scgi_temp;\n"
-               "    access_log " log-directory "/access.log;\n"
-               (let ((http (map default-nginx-vhost-config vhost-list)))
-                 (do ((http http (cdr http))
-                      (block "" (string-append (car http) "\n" block )))
-                     ((null? http) block)))
-               "}\n"
-               "events {}\n")))
+  (computed-file
+    "nginx.conf"
+    #~(call-with-output-file #$output
+        (lambda (port)
+          (format port
+                  (string-append
+                     "user nginx nginx;\n"
+                     "pid " #$run-directory "/pid;\n"
+                     "error_log " #$log-directory "/error.log info;\n"
+                     "http {\n"
+                     "    client_body_temp_path " #$run-directory "/client_body_temp;\n"
+                     "    proxy_temp_path " #$run-directory "/proxy_temp;\n"
+                     "    fastcgi_temp_path " #$run-directory "/fastcgi_temp;\n"
+                     "    uwsgi_temp_path " #$run-directory "/uwsgi_temp;\n"
+                     "    scgi_temp_path " #$run-directory "/scgi_temp;\n"
+                     "    access_log " #$log-directory "/access.log;\n"
+                     #$(let ((http (map default-nginx-vhost-config vhost-list)))
+                         (do ((http http (cdr http))
+                              (block "" #~(string-append #$(car http) "\n" #$block )))
+                             ((null? http) block)))
+                     "}\n"
+                     "events {}\n"))))))
 
 (define %nginx-accounts
   (list (user-group (name "nginx") (system? #t))
-- 
2.10.2