Accommodate custom templates for site conf files.

[flatrocks]

Two considerations:

1. The current implementation for site config files wraps each site's directives in a server block. While this is arguably the "proper" way to define a site, it prevents standard idioms like including a site's corresponding upstream definition in the same file (for example the standard NGINX/unicorn setup.) There is real value in combining as site .conf with its upstream, because it allows you to enable/disable/modify a site and its upstreams together.

2. In most cases, the implementor will have an existing .conf file to start with. For simple configs, it's easy to migrate the NGINX directives to use with this role. But for more complex configurations, it's not helpful to have to break down a file into an array of directives, hoping in the end this role will reassemble the directives to get right back to the original file you started with.

Without compromising existing functionality, this solution provides a way to use a custom template to define a site .conf file. The most obvious use-case for this is to copy a desired .conf file verbatim.

[jdauphant]

@flatrocks Thanks for the PR.
Good point for 1, in effect you can't disable upstreams with the site.
I will be more in solution where we detect if there is "server {" inside. Combined with the new notation it's should be nice https://github.com/jdauphant/ansible-role-nginx/releases/tag/v1.2 .
Also I have made a comment on a change that break the role.

[jdauphant]

There is an uncompatible point: "nginx_sites[item]" isn't an hash, it's a list

[flatrocks]

Thanks for the quick review.

I agree. The stinky part of this solution is that nginx_sites[item] is intended to be a list all through the implementation (and documentation,) and this does expect a hash. After digging through the source, the only place that matters is in the site.conf.j2 template (which is not used when providing a custom template, and which is why this works.) But I agree, it's "tricky" and that usually ends up causing trouble down the road.

Using a list of string directives does not give many options besides scanning the strings content. The idea to

detect if there is "server {" inside

is an interesting one. I suppose the "server { ... }" wrapper could be optionally excluded when the list of directives includes its own server block. What I don't like about that is that this makes the site.conf template a little more magical and "dsl-like." But this could work well, is easily explained, and would allow virtually any configuration.

The only other option I considered was to introduce a new hash of site definitions similar to the nginx_sites, perhaps named templated_nginx_sites (though hopefully, something less clumsy) that would work just like the nginx_sites, but would expect a template and optional template args. But providing two separate ways to create sites has its own issues.

This PR was for an immediate requirement and we're running off the flatrocks fork for now, but I am interested in a more permanent solution, and glad to pitch in if it would be helpful.

[jdauphant]

I just understand the trick with your further explication.
I will merge it in fact, can be little confusing but it's very clever.

[flatrocks]

Thanks. Let me know if you feel the documentation for the new option is weak and/or you think it would be worth adding this to the test.

[jdauphant]

@flatrocks You're welcome!
Add this to the test will be great to ensure there is no regression.