Saltcheck salt-ssh support

[mchugh19]

What does this PR do?

supersedes #52434

• Optimize test copying (replace cp.cache_master with logic to cp.cache_dir on used salt states)
• Tidy "salt://" handling by using url util
• Create saltcheck wrapper for use in salt-ssh. Handles cp.cache_dir functionality by creating a tarball, copying, and extracting on ssh minion. Then calls the regular saltcheck functions.

Previous Behavior

1. saltcheck, being unable to run cp.cache_* from a salt-ssh minion, was not able to locate test files and did not function.
2. saltcheck was only able to parse an assertion_section of a single depth
3. cp.cache_master takes more time and I/O than necessary

New Behavior

1. Wrapper written to gather needed state directories, tarball them, copy tar to salt-ssh minion, extract to local cache directory, and cleanup. Then hands over saltcheck running to the local saltcheck module.
2. assertion_section can now be nested dicts or lists
3. Now uses cp.cache_dir on needed state directories

Tests written?

No

Commits signed with GPG?

No

[mchugh19]

Not sure what the best way is of detecting a salt-ssh session. Advice gladly accepted

[lomeroe]

trying out your updated version and comparing to my updates, I've found 2 things so far:

1. salt.utils.url.create is creating an invalid salt URL on windows (for me at least) it creates salt://path\to\state instead of salt://path/to/state, which then fails to cache (outside of the scope of our module, but just letting you know)

2. one benefit I had from using the sls low data in saltcheck module updates #53024 was that tst files could dynamically get included if the associated sls file was included in the state, for example:

Imagine we have:

/srv/salt/my_formula//
    init.sls
    another.sls
    finally.sls
    /saltcheck-tests/
        init.tst
        another.tst
        finally.tst

/srv/salt/my_forumla/init.sls:

include:
  - my_formula.another

some_state_definition:
  module.function:
     - name: this_name
     <other_function_stuff>

/srv/salt/my_forumla/another.sls:

include:
  - my_formula

another_state_definition:
  module.function:
     - name: another_name
     <other_function_stuff>

/srv/salt/my_formula/saltcheck-tests/init.tst

check_some_state_definition:
  <test_definition>

/srv/salt/my_formula/saltcheck-tests/another.tst

check_another_state_definition:
  <test_definition>

If I run saltcheck.run_state_tests my_formula, only the init.tst will be run (if there is some saltcheck way of doing an include and I'm just missing it, then forgive me for being daft and not understanding how to do that). Obviously, I could use the "check_all" flag (but then finally.tst would be run, which shouldnt be tested if finally.sls hasn't been run) or add every state to the command line (which if you have a state that pulls in a lot of other states, would get gross)

If I discover the states that make up "my_formula" (by generating the low data and examining it), then that data can be used to pull all the corresponding tst files in and execute them.

The alternative would be to get saltcheck to understand an "include" directive so tst files could be included in another (again, if it doesn't already exist and I'm just not seeing how to do it).

We use a lot of "meta" states that dynamically pull in other states (via map data) based on the minion OS/version/etc -- so being able to have my tst files correlate to my sls files and be discovered in a similar fashion for testing is a huge win...

The example above isn't dynamic in that it is a hardcoded include, but hopefully I'm making my point with the example. Maybe how we are doing this is not normal, but has worked very well for us in our heterogeneous environment where I may have a single state to do some "thing" on disparate systems that may have very different ways of accomplishing that "thing".

[mchugh19]

For 1 I think we'll need some saltdevs to respond. I suspect that's a bug. For now it's simple enough to not use url.create and to just manage the pathing in the module.

For 2 that does look like a massive improvement! How does your code handle the case of a .tst file itself having Jinja markup and including a map.jinja? I had an issue when trying to parse lowstate that when I pulled the .sls and .tst files, it did not also get included map.jinja files and so tests were not rendered properly. See the pillar example in the saltcheck docstring.

[lomeroe]

The slsutil.renderer in the _render_file function seems to be handling all of that for me (I didn't change anything there).

I'm doing things like the following in all my tst files and haven't run into anything not being rendered properly:

{% import_yaml 'path/to/state/defaults.yml' as state_defaults -%}
{% from 'path/to/state/map.jinja' import state with context %}
{% from 'tests/integration/saltcheck/conversion_functions.jinja import someFunction %}

{% set state_settings = salt['pillar.get']('state_pillar', default=salt['pillar.get']('common-defaults', default=state_defaults, merge=True), merge=True) -%}

The map/defaults are cached by the generation of the sls low data (since they are used by the state), but in this example, the "conversion_functions.jinja" gets cached when the tst file is rendered (as it is only used in some tst files). Were you experiencing that with salt-ssh or any use of saltcheck?

[mchugh19]

Yep @lomeroe, your cp.cache_dir logic looks sound! I just tried it on a sample state and tests, and it works as you've described. Excellent!

In the case of salt-ssh, it turns out to be more difficult. The basic problem is that an ssh minion is running salt-call --local and thus does not have the ability to run cp.cache_dir as part of the module's execution. So the logic in this PR wraps the saltcheck execution module, with a saltcheck wrapper module that bluntly evaluates what states are being checked, creates a tarfile on the master, copies that tar to the ssh minion, extracts the tar into the cache directory, then runs saltcheck as normal. Even though the saltcheck module internally then attempts to run cp.cache_dir, those calls fail, but since the cache has already been populated, the module goes ahead anyway and runs as normal.

I think the cp.cache_dir of the entire state is probably acceptable for the salt-ssh case. But otherwise, I'd go with your file caching logic for the module itself. How do you want to handle things? Should we see if this PR gets merged quickly and rebase your PR on top to rework the file caching/add_test_files_for_sls logic again, or should we merge that into this PR?