Suggestion: add saltcheck tests for formula validation

[mchugh19]

Experpt from Neon release notes and docs: https://docs.saltstack.com/en/develop/topics/releases/neon.html
https://docs.saltstack.com/en/develop/ref/modules/all/salt.modules.saltcheck.html

Saltcheck provides unittest like functionality requiring only the knowledge of salt module execution and yaml. Saltcheck uses salt modules to return data, then runs an assertion against that return. This allows for testing with all the features included in salt modules. tst files are run through the salt rendering system, enabling tests to be written in yaml (or renderer of choice), and include jinja, as well as the usual grain and pillar information.

saltcheck can be used for formula tests as part of a CI pipeline and can also be run on machines in your environment as part of a QA validation step. Personally, I see a lot of benefit to including saltcheck-tests directories in upstream formulas, as it allows for instant testing capability without other dependencies and using a state-like yaml syntax which understands pillars. For example, validating that 100 users (defined in a pillar) are properly provisioned is trivial and clear in saltcheck, while writing the same in inspec would require duplication of the salt pillar data as well as needing inspec and its dependencies which might not be appropriate in a production environment.

Example file system layout:

/srv/salt/apache/
    init.sls
    config.sls
    saltcheck-tests/
        init.tst
        config.tst
        deployment_validation.tst

Tests can be run for each state by name, for all apache/saltcheck/*.tst files, or for all states assigned to the minion in top.sls. Tests may also be created with no associated state. These tests will be run through the use of saltcheck.run_state_tests, but will not be automatically run by saltcheck.run_highstate_tests.

salt '*' saltcheck.run_state_tests apache,apache.config
salt '*' saltcheck.run_state_tests apache check_all=True
salt '*' saltcheck.run_highstate_tests
salt '*' saltcheck.run_state_tests apache.deployment_validation

Verify package installed:

verify_vim:
  module_and_function: pkg.version
  args:
    - vim
  assertion: assertNotEmpty

Ensure latest package installed:

{% for package in ["apache2", "openssh"] %}
{# or another example #}
{# for package in salt['pillar.get']("packages") #}
jinja_test_{{ package }}_latest:
  module_and_function: pkg.upgrade_available
  args:
    - {{ package }}
  assertion: assertFalse
{% endfor %}

Ensure python can load boto and boto3 libraries:
common/saltcheck-tests/init.tst

boto-installed:
  module_and_function: cmd.run
  args:
    - python -c 'import pkgutil; print(1 if pkgutil.find_loader("boto") else 0)'
  assertion: assertNotEqual
  expected-return: 0

boto3-installed:
  module_and_function: cmd.run
  args:
    - python -c 'import pkgutil; print(1 if pkgutil.find_loader("boto3") else 0)'
  assertion: assertEqual
  expected-return: 1

Test that all users exist and have the expected ssh keys. Might be appropriate for the users formula, for example.
users/saltcheck-tests/init.tst

{% for usr,data in salt['pillar.get']('users').items() %}

validate_user_{{ usr }}:
  module_and_function: user.info
  assertion_section: shell
  args:
    - {{ usr }}
  assertion: assertEqual
  expected-return: /bin/bash

{%- if 'key' in data %}
check_ssh_key_{{ usr }}:
  module_and_function: ssh.check_key
  args:
    - {{ usr }}
    - {{ data.key }}
    - ssh-rsa
    - ''
    - ''
  assertion: assertEqual
  expected-return: exists
{%- endif %}

{% endfor %}

I'd envision having basic tests like pkg.version is notNull, service.status is True, and maybe a file.contains is True live in most formulas. Since salt already has the map.jina and pkg manager abstractions, these tests can easily be upstreamed allowing for everyone's use.

[myii]

Thanks for bringing this across from Slack, @mchugh19.

Linking back to discussions where saltcheck has been mentioned previously: https://freenode.logbot.info/?ch=saltstack-formulas&q=saltcheck.

@javierbertoli @daks Not suggesting we should undo any work that's been done with inspec. However, what do you think about the idea of trying this out for a small formula that currently doesn't have any tests?

[mchugh19]

If you have a formula suggestion, I don't mind adding some tests and sample commands to kick off the whole thing.

FYI, in the linked slack discussion it was mentioned that the downside to saltcheck was the lack of documentation. That was cleaned up for the neon release (as well as adding a bit of functionality). So if anyone is curious you can look at the neon release notes as well as the saltcheck docs linked at the top of this issue for the new and improved docs. There may also be a blog post coming out in the saltstack blog.

[myii]

@mchugh19 This formula would be great, since it already has a fairly comprehensive inspec suite in place. Another one that was updated recently was the salt-formula. Or if you want something that has no testing in place, the cron-formula got a recent overhaul.

In any case, even something minor would be useful. That would give us a working example to further discussions about how we want to proceed across the whole of SaltStack Formulas. To give you an idea of what I mean, you can refer back to #21 to see how we've been progressing.

[myii]

@claudekenni has supplied PR #88, so linking here so that the discussions can be kept together. There is an issue with relative imports there that needs looking into.

[mchugh19]

I've added some saltcheck tests for the salt and cron formulas.

For cron, the existing cron module didn't make it easy to parse existing entries, so the saltcheck tests for config are dependent on an upstream PR. saltstack/salt#52441

For the salt formula, I didn't support the MacOS logic in the minion test. But if anyone wants to extend, it shouldn't be difficult as it can use the same jinja logic.

As you can see, saltcheck tests are very similar to states. So you can start a tst file by copying the equivalent state, using the existing jinja, and replacing the state logic with the equivalent tst syntax.

[myii]

@mchugh19 Great, thanks for these PRs. I've had an issue with #88, where I end up with this error (using 2019.2):

    The minion function caused an exception: Traceback (most recent call last):
      File "/usr/lib/python2.7/dist-packages/salt/minion.py", line 1660, in _thread_return
        return_data = minion_instance.executors[fname](opts, data, func, args, kwargs)
      File "/usr/lib/python2.7/dist-packages/salt/executors/direct_call.py", line 12, in execute
        return func(*args, **kwargs)
      File "/usr/lib/python2.7/dist-packages/salt/modules/saltcheck.py", line 128, in run_state_tests
        scheck = SaltCheck()
      File "/usr/lib/python2.7/dist-packages/salt/modules/saltcheck.py", line 301, in __init__
        self.salt_lc = salt.client.Caller()
      File "/usr/lib/python2.7/dist-packages/salt/client/__init__.py", line 1992, in __init__
        self.sminion = salt.minion.SMinion(self.opts)
      File "/usr/lib/python2.7/dist-packages/salt/minion.py", line 805, in __init__
        lambda: self.eval_master(self.opts, failed=True)
      File "/usr/lib/python2.7/dist-packages/tornado/ioloop.py", line 440, in run_sync
        self.start()
      File "/usr/lib/python2.7/dist-packages/zmq/eventloop/ioloop.py", line 162, in start
        super(ZMQIOLoop, self).start()
      File "/usr/lib/python2.7/dist-packages/tornado/ioloop.py", line 731, in start
        raise RuntimeError("IOLoop is already running")
    RuntimeError: IOLoop is already running

As I've mentioned on Slack, I'm expecting this to be a misconfiguration on my side rather than something specific to 2019.2.

I'll try out both PRs to see if I get any further.

[mchugh19]

@myii as mentioned on slack, but to duplicate here for history, that error was corrected in saltstack/salt@f54ada1#diff-9bd5118b900e8f9910f482d1b27af666 which will be released with Neon. There aren't any salt version dependencies in saltcheck.py itself, so you should be able to use neon's file and drop it into _module/saltcheck.py