Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collective config validation & "extra options" #36

Merged
merged 8 commits into from Dec 28, 2015

Conversation

cmacrae
Copy link
Contributor

@cmacrae cmacrae commented Dec 22, 2015

Hey Jeff!

Thanks for the great role 馃憤
Hoping you'll happily accept these contributions (with any modifications you see fit, of course).

These changes bring a collective validation of the deployed Nginx configuration and the option to define arbitrary "extra" configuration options for the top-level http block in /etc/nginx.conf.

  1. Nginx master config validation: I added a validate param on the deployment of the /etc/nginx.conf file. This will ensure the Nginx config (including any included configs) are valid, according to nginx -t. I had to move the deployment of vhosts above the deployment of the nginx.conf file, to ensure all config was in place before testing (see my commit message on cc5114d for more details).
  2. Nginx config validation handler: I added a handler that will also validate the collective Nginx configuration and have ensured this is triggered anywhere that restart nginx was being called.
    If an invalid configuration is deployed, it will run this notifier first, so, in the case that /etc/nginx.conf doesn't change, but a vhost config does, and as such does not trigger the above mentioned validate param, and it introduces some bad config, Ansible will fail upon running the validate handler, so it'll exit before it has a chance (fail) to restart the Nginx service - meaning, you're at least not left with a broken web server :)
  3. I added a nginx_extra_options variable that will allow the user to define (literally, using Nginx syntax) extra configuration they want to add to the http block. I elaborate on this in the commit: c5a2143, in the README, and lastly in an example comment in defaults/main.yml.

cmacrae added 8 commits December 22, 2015 16:05
Re-ordering the tasks in this way (having the vhosts deployed first)
allows the 'validate' param to collectively check the deployed Nginx config.

Deploying vhosts after makes it hard to check their validity, as Nginx's
config checking will operate on a "master" configuration that includes
others (checking those included, also) but would error out when checking
these individual configs if they do not contain a fully working Nginx
config (which they often don't, due to their nature).
Defining a new variable (defaults to empty) that allows users to define
extra configuration options in the top-level 'http' block.
This allows for (optionally) finer grain control.
Although the 'validate' param was added for the deployment of
/etc/nginx.conf - this validation process will only be triggered upon
changes.
So, if a vhost config is updated, but the main config isn't, the
collective config will not be verified.

I've added a new handler 'validate nginx configuration' and added this
to the 'notify' param as a first list item for vhost config changes.

Unfortunately, this will not protect against the deployment of malformed
configuration, however it will prevent the restart of Nginx in such a
situation (as the 'validate nginx configuration' handler should error
out before the 'restart nginx' handler is called).
geerlingguy added a commit that referenced this pull request Dec 28, 2015
Collective config validation & "extra options"
@geerlingguy geerlingguy merged commit a8620b9 into geerlingguy:master Dec 28, 2015
geerlingguy added a commit that referenced this pull request Dec 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants