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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Use cmd_check to validate config files #37

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Use cmd_check to validate config files #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6bff110
Use cmd_check to validate config files
Nov 4, 2018
c9dea92
Fix consul validate dependencies
Nov 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
.kitchen/
Gemfile.lock
36 changes: 25 additions & 11 deletions consul/config.sls
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,24 @@
{%- from slspath + '/map.jinja' import consul with context -%}

consul-config:
file.serialize:
Copy link
Member

@aabouzaid aabouzaid Nov 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO, manage structured files with file.managed is really bad (I tried this myself and it always requires many hacks and checks).

I'd suggest to keep using file.serialize, and create another check for syntax specifically.
We could use mod_run_check_cmd, but just realized it doesn't work as expected (it's used as an internal function for file.managed. It's already exposed externally, however it doesn't met functions requirements, where it doesn't have name,changes in returned output).

So till fixing that (mostly I will make a PR for that), let's use cmd.run with unless:

consul-config-validate: 
  cmd.run:
    - name: /usr/local/bin/consul validate /etc/consul.d/config.json
    - unless: /usr/local/bin/consul validate /etc/consul.d/config.json
    - require:
      - file: consul-config

The unless clue, will prevent running the command when it has a valid syntax. So it avoids any extra noise.

file.managed:
- name: /etc/consul.d/config.json
- formatter: json
- dataset: {{ consul.config }}
- user: {{ consul.user }}
- group: {{ consul.group }}
- mode: 0640
- source: salt://{{ slspath }}/files/template.json.jinja
- template: jinja
- context:
content:
{{ consul.config | yaml }}
- require:
- user: consul-user
{%- if consul.service %}
- file: /usr/local/bin/consul
- watch_in:
- service: consul
- check_cmd: /usr/local/bin/consul validate
- tmp_ext: '.json'
{%- endif %}

{% for script in consul.scripts %}
Expand All @@ -28,16 +34,24 @@ consul-script-install-{{ loop.index }}:
{% endfor %}

consul-script-config:
file.serialize:
file.managed:
- name: /etc/consul.d/services.json
{% if consul.service != False %}
- watch_in:
- service: consul
{% endif %}
- user: {{ consul.user }}
- group: {{ consul.group }}
- mode: 0640
- source: salt://{{ slspath }}/files/template.json.jinja
- template: jinja
- context:
content:
services:
{{ consul.register | yaml }}
- require:
- user: consul-user
- formatter: json
- dataset:
services: {{ consul.register }}
{% if consul.service != False %}
- file: /usr/local/bin/consul
- file: /etc/consul.d/config.json
- watch_in:
- service: consul
- check_cmd: /usr/local/bin/consul validate /etc/consul.d/config.json
- tmp_ext: '.json'
{% endif %}
1 change: 1 addition & 0 deletions consul/files/template.json.jinja
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{{ content | tojson(indent=2) }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tojson() Jinja filter is only available in the latest stable Salt 2018.3.3 release.
However, the built-in serializer has generic JSON filter for ages and also supports indentation. Simply like this:

content|json(indent=2)

This is more portable and reliable. As far as I understand upstream Jinja docs, the tojson() filter is designed to be used within HTML pages, and it could do some weird things when you need "raw" JSON file.

2 changes: 1 addition & 1 deletion pillar.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ consul:
user: consul
group: consul

version: 0.7.0
version: 1.2.0
download_host: releases.hashicorp.com

config:
Expand Down