Salt: Syntax checking

[j0nes2k]

When a salt state contains YAML syntax errors, it is quite hard to find these errors at the moment. It would be great to have a basic syntax check (validation agains a schema) and/or references check (e.g. do the references given in require-statements really exist).

[thatch45]

Yes, the problem is that when we parse the data it is a data structure and we don't know if it came from YAML or some other source. Also that it needs to be passed through the jinja as well. When you find an issue that was difficult to debug please let us know what the error was and what the solution was so we can make the errors better

[dcolish]

In general, I think a litle tool that allowed you to render sls files locally and pass in the commands via the command line would be really helpful for developing these modules. This tool could easily try and load the yaml, json or any other format supported in the future.

[j0nes2k]

Thank you for your input! The main problem is that when I create a state, I write the complete file including package installations, config file copying etc. in one go. And of course there will be simple typing errors, like "reqiure" vs. "require" (just an example). When I try to push this state to my machines, I only get empty responses back (e.g. no "{}", just an empty line). Then I have to comment out parts of my state, check whether the highstate now works, comment out other parts until I find this simple error.

It would be great to have a simple check (like: "method 'reqiure' is not known" or something like that) to make it easier to debug things - or am I missing a built-in solution available in salt already?

[thatch45]

Ahh, yes, we do need better error checking, and maybe some kind of salt-lint would be nice. I am working towards something that will make this better, and there are some places where error checking is still lacking, we will keep this on the radar!

[halberom]

I've experienced the same issues, the current workaround/solution is to run "salt-call -l info state.highstate" on a minion as it always reports compile errors properly even though nothing is returned to the master. The output shows issues like typos and source file location errors. Note that it will only report the first compile errors it breaks on, so if there are a few, it'll be necessary to fix them and try again until it passes cleanly.

[thatch45]

In git the errors should be returned to the master, and yes, there are a few rounds of errors that Salt can't proceed if there are errors early on.

[mlister2006]

It would be nice to have simple YAML syntax checking for errors. e.g checking colon : , indentation etc.

test                                  # missing colon :
  - arg1
  - arg2

syntax error like above, simply pass thru to minions when running state.highstate and crashes minion (need to restart minion process)

[thatch45]

Yes, I have started on a process for this which is called master compiling, but have been unable to finish it, there has been too much demand in too many places!
I will get back to it eventually and make this happen!

[alecthomas]

I would very much like to see some better validation...some bugs are currently extremely hard to track down :

For example, people new to salt are inclined to believe that the top-level directives in an sls file are a list, not a mapping, so they frequently duplicate keys. eg.

ntpd:
  pkg.installed: []
ntpd:
  service.running:
    - require:
      - pkg: ntpd

This silently fails and is difficult to track down.

Additionally, there doesn't seem to be any validation of state names in the top file? If I mistype a state name in the top file, it just gets dropped without error.

A possible solution might be to have two-stage validation:

1. Per-data-source validators, eg. yaml, json, etc.
2. The final parsed in-memory data-structure validator.

[thatch45]

A state name gets dropped without an error? Can you give me an example? this should generate an error.
My plan for a validator is the masterstate approach, but it has been stagnat for a while

[alecthomas]

I'll try to dig up an example, but in the interim I whipped up a quick YAML + Salt validator.

This has two stages:

1. YAML structural validation - currently primarily disallowing duplicate keys.
2. Once the YAML is parsed into a Python dictionary, validate the various Salt structures (top, state and pillar) with voluptuous. This is _very_ broad, and has no knowledge of the module-specific structures.

[thatch45]

This looks like it will easily get past many of the rendering issues and validate structure, how would you feel about us including this as a runner?

[alecthomas]

That would be fine with me, please use as much of the code as you want.

I tested this on our salt tree and it works quite well, but I have no easy means of testing it _after_ Jinja expansion, so it explodes on a few files.

And of course, it doesn't do _source validation_ on any of the other data sources: json, etc.

[thatch45]

Sounds good, at least it is a good starting point!

[jolestar]

me too !!!

[fsniper]

This should also consider jinja and python states. Maybe not python states but at least jinja states.

[rgbkrk]

I've been curious about this for a bit, so I went ahead and figured out a simple setup on travis ci with help from @whiteinge and @thatch45. If your states are open source or you use Travis CI Pro, you're :+1:.

_.travis.yml_

language: python
python:
- '2.7'

before_install:
  - sudo apt-get update
  - curl -L http://bootstrap.saltstack.org | sudo sh -s -- git develop

install:
  # Copy these states
  - sudo mkdir -p /srv/salt/states
  - sudo cp -r . /srv/salt/states
  - sudo cp .travis/minion /etc/salt/minion
  - sudo service salt-minion restart

  # Additional debug help
  - sudo cat /var/log/salt/*

  # See what kind of travis box you're on
  # to help with making your states compatible with travis
  - sudo salt-call grains.items --local

script:
  - sudo salt-call state.show_highstate --local  --retcode-passthrough

_.travis/minion_

file_client: local
file_roots:
  base:
    - /srv/salt/states

This setup was designed with the simplistic states that nbviewer uses. More about it in blog form.

You'll also want to load your pillar data onto the Travis minion and possibly run a highstate (careful with this as Travis' environment is likely a bit different from your infrastructure).

[lcosmin]

Not sure if still of use, but I've wasted ~30 minutes trying to find an error in pillar's top.sls (simplified for the sake of example):

base:
  'roles:test':
    - match:grain
    - test

Notice the missing space between match: and grain. My pillar data wasn't available to the minion and I was checking for spelling errors and such, but somehow my brain didn't "see" the missing space. Maybe some sort of warning could be added for such things.

[HontoNoRoger]

+1

[whiteinge]

Voluptuous would be great for this. It's small, simple, and crazy straight-forward. We could toss a copy of it in salt/utils/validate/ with no impact. I started work writing a schema for the Salt master config and has worked well for validating the external_auth part. I'm not going to have time to put into that for a little while so if someone else wants to take the first stab, please feel free.

See also #15836.

[v6]

I'm coming from Puppet, myself. I would have preferred to install SaltStack, but everyone and their mother was using it at my last company.

Anyway, there were a lot of things that they put together to make life easier on non-expert users. These almost made up for the severe limitations of the Puppet system.

Anyway, one of the things I liked best was puppet validate.

It saved me a lot of time.

Why no salt validate?

[thatch45]

While it is not as convenient the

salt-call state.show_highstate

Can compile the sls files and at least show you the errors. It is primarily because the sls files can be generated on the minions, so always getting the right data from Jinja can be tricky

[v6]

OK.

I just wish there was a validation checker that could point me to the
location in the files, specifically.

IDEs have this, and I'm going to be installing one for Sublime Text
and Pycharm.

As to the generation, isn't it deterministic?

On 1/14/15, Thomas S Hatch [email protected] wrote:

While it is not as convenient the

salt-call state.show_highstate

Can compile the sls files and at least show you the errors. It is primarily
because the sls files can be generated on the minions, so always getting the
right data from Jinja can be tricky

---

Reply to this email directly or view it on GitHub:
https://github.com/saltstack/salt/issues/802#issuecomment-70004550

Nate B.

[v6]

---------- Forwarded message ----------
From: Nate B [email protected]
Date: Wed, 14 Jan 2015 15:37:25 -0800
Subject: Re: [salt] Syntax checking (#802)
​​
To: saltstack/salt <
[email protected]

Cc: saltstack/salt [email protected], Nathan Dataguake Basanese <
[email protected]>, an.[email protected]

OK.

I just wish there was a validation checker that could point me to the
location in the files, specifically.

IDEs have this, and I'm going to be installing one for Sublime Text
and Pycharm.

As to the generation, isn't it deterministic?

On 1/14/15, Thomas S Hatch [email protected] wrote:

While it is not as convenient the

salt-call state.show_highstate

Can compile the sls files and at least show you the errors. It is
primarily
because the sls files can be generated on the minions, so always getting
the
right data from Jinja can be tricky

---

Reply to this email directly or view it on GitHub:
https://github.com/saltstack/salt/issues/802#issuecomment-70004550

[damon-atkins]

http://docs.saltstack.com/en/latest/ref/states/testing.html
It would be nice if you could setup a environment which defaults to testing only (dry-run).

[pcn]

I've got an alternate take on this at https://github.com/librato/salt-state-test. It's a bit rough, but feedback from someone beside me is appreciated.

[figtrap]

We really need something like this ...

[damon-atkins]

Need a tool which pulls in the pillar data, either a file or a list of files, directory and then give it a state file, and then it prints out the final state/catalog/manifest. It can work outside of salt. In fact it would be better if it did, so the checking can be perform before committing changes.

salt-call state.dry-run
salt-call state.process-state-only

[thatch45]

I recently added the mocked option to the state calls:
Salt-call state.high state mocked=True
This does the full state run just without executing the states, so everything compiles with the pillar etc.

[pcn]

@thatch45 where's the commit/branch with that in it?

[pcn]

@damon-atkins the repo I pointed to early does something like what you're asking, but the pillar and the grains can each only be one file.

[thatch45]

It is in 2015.8.7

[tampakrap]

@thatch45 Can you explain please how is that different from test=True?

[thatch45]

test=True runs in no-op mode and checks the state of the system
mocked=True runs the state runtime and resolves all requisites but it does not run any states on the target system
state.show_highstate just runs the data compiler and returns the data tht the runtime would execute with

Basically, show_highstate does syntax checks, mocked does syntax + runtime (requisites and ordering) checks, and test=True does a full state run with checks to see if anything will be changed with an actual state run

So it is basically just stopping the process at certain points to give you a clear representation of what your states are and will do

[figtrap]

Oh what I really want is some sort of IDE for salt development that would
do things like autofill pillars and variables when you're writing jinja,
linting the yaml and things like that. There is just so much wasted time
with typos. But the "mocked" feature is much needed and appreciated.

Tim Kelley

On Thu, Feb 25, 2016 at 10:46 AM, Thomas S Hatch [email protected]
wrote:

test=True runs in no-op mode and checks the state of the system
mocked=True runs the state runtime and resolves all requisites but it does
not run any states on the target system
state.show_highstate just runs the data compiler and returns the data tht
the runtime would execute with

Basically, show_highstate does syntax checks, mocked does syntax + runtime
(requisites and ordering) checks, and test=True does a full state run with
checks to see if anything will be changed with an actual state run

So it is basically just stopping the process at certain points to give you
a clear representation of what your states are and will do

—
Reply to this email directly or view it on GitHub
https://github.com/saltstack/salt/issues/802#issuecomment-188872265.

[thatch45]

Oh, yes, what you are describing would be great

[damon-atkins]

One of the issue I was hoping this would resolve is the error messages not indicating where an issue is. For example JINJA processing only says:
Rendering SLS 'base:iscdhcp' failed: Jinja variable 'list object' has no attribute 'name'
Rendering SLS 'base:iscdhcp' failed: Jinja variable 'dict object' has no attribute 'path'
The problem is 'base:iscdhcp' as a whole can contain multiple name and path, which one is it in the structure. (2015.5.8) Maybe saltstack needs to submit a change into jinja to improve errors.

Just want to be able to find issues, as much as possible, and find them before they go live (committed/pushed).

Update for Fedora running 4.4.2-301.fc23.armv7hl i.e. ARM

[salt-latest]
name=SaltStack Latest Release Channel for RHEL/Centos
#baseurl=https://repo.saltstack.com/yum/redhat///latest
baseurl=https://repo.saltstack.com/yum/redhat/latest/x86_64/latest/
failovermethod=priority
enabled=1
gpgcheck=1
#gpgkey=https://repo.saltstack.com/yum/redhat///latest/SALTSTACK-GPG--KEY.pub
gpgkey=https://repo.saltstack.com/yum/redhat/latest/x86_64/latest/SALTSTACK-GPG-KEY.pub

===============================================================
 Package              Arch         Version               Repository        Size
===============================================================
Installing:
 python-cherrypy      noarch       3.5.0-1.fc22          fedora           459 k
 python-libcloud      noarch       0.20.1-1.fc23         updates          2.0 M
 python-tornado       armv7hl      4.2.1-1.fc23          updates          642 k
 salt                 noarch       2015.8.7-1.el7        salt-latest      5.2 M
 salt-api             noarch       2015.8.7-1.el7        salt-latest       13 k
 salt-cloud           noarch       2015.8.7-1.el7        salt-latest       16 k
 salt-master          noarch       2015.8.7-1.el7        salt-latest      1.2 M
 salt-minion          noarch       2015.8.7-1.el7        salt-latest       29 k

Transaction Summary
===============================================================
Install  8 Packages

[gravyboat]

I'm just going to bump this again. People keep running into this issue of not being able to syntax check without doing a run, which doesn't provide great errors. There really needs to be a built in utility within salt to at least confirm the yaml/jinja is legit (even if it does not check against the pillar, though that would be nice). This has been a 'want' for both new and existing users for over 4 years now.

[jowolf]

bump - I wrote a custom yaml module (for another project) to trace yaml compiiles and line numbers, for example - so when it died you could see where it was last.

It was crude, and used globals for state like the line number, but it's an idea

[RobotLimeLtd]

The nearest you might get is the following, however it will only work for very simple Salt setups, and only on a server that has Salt Minion installed. Assuming your source repo contains both "salt" and "pillar" directories, each with a top.sls in them... you could run:

salt-call state.highstate --file-root=$PWD/salt --pillar-root=$PWD/pillar --local --retcode-passthrough mocked=True

However, if you have anything more exotic, such as an external pillar directory, then you're out of luck.

[gravyboat]

@RobotLimeLtd Yeah we've discussed that as an option before, and it's not a significantly satisfactory option for anyone I've talked to. Thanks for noting it though!

[chrismcmacken]

Do salt states have a documented grammar that is used to parse them?

[pcn]

@chrismcmacken have you read through https://docs.saltstack.com/en/latest/ref/renderers/ ?

[chrismcmacken]

@pcn I hadn't, salt supports a lot more renderers than I thought. Still though, if it gets parsed it must be against some set of rules. I'm sure it's a tough problem to solve, otherwise it would have been solved already, just thought I'd ask and bump the thread.

[pcn]

It doesn't seem to support a grammar so much as a list of extensible grammars that map directly to a list of python dictionaries.

[soostdijck]

Bump as well. Has anyone made progress on this?
I'm still hoping that I'll be able to check syntax via commit hooks properly.

[JensRantil]

Just wanted to share how we've created a workaround for syntax checking in our CI pipeline: In our top.sls files we've added a standardized way of adding example minion names:

# CI example: my-minion-name
'my-minion-*':
  - some.state

We then have a script that extracts all the example minion names (salt-dry-run.sh) for each minion name calls the script salt-dry-run-single.sh with the first (and only) parameter being the minion name.

You can find the code here: https://gist.github.com/JensRantil/c8757d607b379d3256299fd32a373a35

Obviously, this doesn't help with local development, but if you are for example using Github Pull Request you could add this as a CI step for this. Hope this makes the lives easier for some people.

[wyardley]

Agree that this would be very useful, something like puppet parser validate for Puppet (to do at least a basic syntax check of the code itself.
Obviously, for YAML itself, it's not hard to just add commit hooks using yamllint or similar.

Having worked with Puppet for a while, something like puppet parser validate, for example, is super handy. That is not to say that you don't also need to do other types of testing, but being able to at least catch simple typos fast (ideally on a machine that doesn't even have Salt setup on it) would be super handy. I've been playing around with this again recently, and came up with something fairly similar to a comment above, but it's not ideal.

[wyardley]

Just as a heads up, while mocked=True doesn't error, mock=True seems to be the correct arg now.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[Oloremo]

not stale

[stale[bot]]

Thank you for updating this issue. It is no longer marked as stale.