lenses/nginx.aug: Allow types block to be defined

[tupyy]

This PR allow types block to be parsed.

Fixes: #834.

Signed-off-by: Cosmin Tupangiu cosmin@redhat.com

[georgehansper]

Hello @tupyy

I'm starting with a bit of background here, just so you can follow my thinking

The basic idea used for the nginx lens is to use the "directive" as the path label For example, an nginx directive like these

        error_page 404 /404.html;
        error_page 500 502 503 504 /50x.html;

would become an augeas paths like these

/files/etc/nginx/nginx.conf/http/server/error_page[1] = "404 /404.html"
/files/etc/nginx/nginx.conf/http/server/error_page[2] = "500 502 503 504 /50x.html"

This works quite well in most cases, as nginx directives generally do not contain the path-seperator '/'

Where things get difficult is within "mapping" blocks, such as map or geo or types

For these blocks, the first string on the line is a string (or IP address) that might be contained in a variable. The range of possible values for this variable is not restricted, and may contain a '/' character

In the case of the geo block, the value may be in CIDR notation, and the existing lens handles this by splitting the key at the '/' and creating a child node 'mask' eg.

/files/etc/nginx/conf.d/test.conf/server/geo
/files/etc/nginx/conf.d/test.conf/server/geo/#geo = "$remote_addr"
/files/etc/nginx/conf.d/test.conf/server/geo/192.168.1.0 = "1"
/files/etc/nginx/conf.d/test.conf/server/geo/192.168.1.0/mask = "24"

This works reasonably well, too, I think splitting the address like this makes it harder to read than the following (hypothetical):

/files/etc/nginx/conf.d/test.conf/server/geo/192.168.1.0/24 = "1"

I'm not suggesting that we change the existing behaviour in general, but we could create a specific case for the (new) types block For example, the block:

types {
                text/plain   txt text log;
                text/csv     csv;
}

We could generate a tree that looks like this, specifically for the types block:

/files/etc/nginx/mime.types/types/mediatype[1] = "text/plain"
/files/etc/nginx/mime.types/types/extensions[1] = "txt text log"
/files/etc/nginx/mime.types/types/mediatype[2] = "text/csv"
/files/etc/nginx/mime.types/types/extensions[2] = "csv"

Or we could generate a tree that looks like this:

/files/etc/nginx/mime.types/types/text[1]/plain = "txt text log"
/files/etc/nginx/mime.types/types/text[2]/csv = "csv"

Getting to the specifics of this Pull Request, there are a couple issues Firstly, the definition of sub_type_word needs to be updated, to possibly this

-let sub_type_work = /[A-Za-z.-]+[0-9_]*/
+let sub_type_word = ( /[A-Za-z0-9.-][A-Za-z0-9._+-]*/ - Rx.integer )

With that, the test will pass FYI: I used the following command to run the individual test: rm tests/lens-nginx.sh.log ; time make -C tests lens-nginx.sh.log

But the lens still fails to load the file /etc/nginx/mime.types when it encounters the following line:

application/opc-nodeset+xml     ;

I am finding this difficult to resolve within the definition of the "simple" lens as it stands

So maybe it is best if we stop trying to use the "simple" definition for every kind of block

I don't like the idea of breaking backwards compatibility, but maybe we need to consider keeping backwards compatability only for the likely use-cases. By that, I mean that the "mask" child node is only likely to appear in the "geo" block, and handle the other blocks differently

I have tried various things already, and I could not find a "one-size-fits-all" approach that actually worked

At the high level, what I am suggesting is

a) Keep the "simple" lens just for simple things - ie those without a '/' as the first word b) Create a specific lens for the "types" block c) Create a specific backwards compatible lens for the "geo" block d) Do either (b) or (c) for the "map" block

What do you think?

[tupyy]

Hi @georgehansper My first approach was exactly b. But I thought I'd complicate my life. I'll give it a try to implement b and c. Thanks.