Make sure SaltCacheLoader use correct fileclient

[kaszuba]

What does this PR do?

Fix problem with jinja2.exceptions.TemplateNotFound: errors in salt-ssh

What issues does this PR fix or reference?

Fixes: #60003 #61174

Problem was caused by wrong cachedir path used by fileclient (FSClient class) in SaltCacheLoader:

• class SaltCacheLoader (in jinja) use default fsclient, with "cachedir" from salt.syspaths.CACHE_DIR
• class Single (in SSH), use fsclient with path for minion

When execute "salt-ssh ... state.apply" BaseHighState will get fsclient from SSH (with minion path), and in compile_low_chunks() files are cached in minion cache path. But when execute jinja (in render_state) fsclient will cache files in cachedir from config (different then minion) and jinja cannot find files.

[vvrein]

--- /root/original_state.py                             2022-04-01 11:45:58.041040719 +0300
+++ /usr/lib/python3/dist-packages/salt/state.py        2022-04-01 11:40:48.530320452 +0300
@@ -4022,6 +4022,8 @@
             )
         else:
             try:
+                if context is None:
+                    context = {"fileclient": self.client}
                 state = compile_template(
                     fn_,
                     self.state.rend,

Can confirm that this PR fixes the salt-ssh (3004) work on Ubuntu 20.04

[stratusjerry]

--- /root/original_state.py                             2022-04-01 11:45:58.041040719 +0300
+++ /usr/lib/python3/dist-packages/salt/state.py        2022-04-01 11:40:48.530320452 +0300
@@ -4022,6 +4022,8 @@
             )
         else:
             try:
+                if context is None:
+                    context = {"fileclient": self.client}
                 state = compile_template(
                     fn_,
                     self.state.rend,

Can confirm that this PR fixes the salt-ssh (3004) work on Ubuntu 20.04

--- /root/original_state.py                             2022-04-01 11:45:58.041040719 +0300
+++ /usr/lib/python3/dist-packages/salt/state.py        2022-04-01 11:40:48.530320452 +0300
@@ -4022,6 +4022,8 @@
             )
         else:
             try:
+                if context is None:
+                    context = {"fileclient": self.client}
                 state = compile_template(
                     fn_,
                     self.state.rend,

Can confirm that this PR fixes the salt-ssh (3004) work on Ubuntu 20.04

Can also confirm salt-ssh 3004 fix on CentOS 7

[s-at-ik]

fixed salt-ssh 3004 on debian stable for me, thanks

[aarnaud]

Nice, thanks for the fix I had the same issue

[baby-gnu]

Hello.

There are in fact 2 kinds of issues regarding import:

1. from .sls files
2. from file.managed jinja templates

This PR solves the first issue, without the need of --extra-filerefs.

The second case still has the issue.

[kaszuba]

@baby-gnu yes, this change will fix jinja generation on master side (system on which salt-ssh is executed), this was my main goal to fix, without this change salt-ssh is unusable for me (since version 3002)

About missing template files on minion side (when import is used inside "file.managed" templates). This is rather other problem, probably with salt_state.tgz generation. Checked quickly how salt_state.tgz is generated, and cannot find any detection for "include" files inside templates. Found only detection for files required directly by lowstate, looks like it will not work without --extra-filerefs. Long time ago (very old salt versions) i tried to to use multi file templates but without success, now i use only single file templates. Then sorry but i dont know whether it is a bug or additional feature is needed, it is not a problem in my environments, maybe someone other will fix it :)

[baby-gnu]

@kaszuba thanks for the explanation.

Don't worry, I mostly prefer to pass context to simple jinja templates.

Thanks.

[Rudd-O]

When is this PR gonna get merged? This bug is a huge regression for salt-ssh users. Is SaltStack becoming abandonware?

[stratusjerry]

@Ch3LL is this patch being tracked for 3005 release?

[Ch3LL]

No, it looks like it was slated for 3006 on our issue tracker. But this PR will need test coverage and a changelog before it will be accepted.

[TeddyAndrieux]

Any chance we get this PR merged ?

I'm actually stuck in Salt 3002 because of this regression

[eliasp]

This needs a test case.

When writing a test for this, here's a scenario that's easy to reproduce for me, affected by this issue:

• Create /var/tmp/some.yaml and dump some valid YAML into it
• Create /var/tmp/some.sls with the following content:

  {%- import_yaml "/var/tmp/some.yaml" as imported %}
  {{ imported }}
  

• Execute salt-call slsutil.renderer /var/tmp/some.sls default_renderer=jinja

[Rudd-O]

This needs a test case.

When writing a test for this, here's a scenario that's easy to reproduce for me, affected by this issue:

* Create `/var/tmp/some.yaml` and dump some valid YAML into it

* Create `/var/tmp/some.sls` with the following content:
  ```
  {%- import_yaml "/var/tmp/some.yaml" as imported %}
  {{ imported }}
  ```

* Execute `salt-call slsutil.renderer /var/tmp/some.sls default_renderer=jinja`

But what are the pass / fail outputs??

[Ch3LL]

Would someone be willing to try my fix here: https://github.com/saltstack/salt/pull/63184 Its still a work in progress, but it I did verify it worked with the original submitter of this issue's files: https://github.com/saltstack/salt/issues/60003. and using state.apply as the description states in this PR.

If it does not work, please post the exact files you are using and the exact command. I need to review all of the methods in salt/client/ssh/wrapper/state.py to ensure we are explicitly passing in context.

I think it would be a better fix to explicitly pass in context from salt-ssh to ensure we are using the entire context, not just setting fileclient.

[kaszuba]

Would someone be willing to try my fix here: #63184 Its still a work in progress, but it I did verify it worked with the original submitter of this issue's files: #60003. and using state.apply as the description states in this PR.

If it does not work, please post the exact files you are using and the exact command. I need to review all of the methods in salt/client/ssh/wrapper/state.py to ensure we are explicitly passing in context.

I think it would be a better fix to explicitly pass in context from salt-ssh to ensure we are using the entire context, not just setting fileclient.

Hi @Ch3LL, thanks for looking on problem, your solution looks much better then old quick fix :)

I checked your patch and not works for me, got the same error with missing template, but it is fast fixable by adding old patch in render_highstate method

    def render_highstate(self, matches, context=None):
        """
        Gather the state files and render them into a single unified salt
        high data structure.
        """
        highstate = self.building_highstate
        all_errors = []
        mods = set()
        statefiles = []
        if context is None:
            context = {"fileclient": self.client}
        for saltenv, states in matches.items():
           ....

Probably context should be also passed to "render_highstate", now in BaseHighState it is used only as "self.render_highstate(matches)", without context passing

[kaszuba]

@Ch3LL i see there is already change for render_highstate(), probably i grab wrong patch checked again and your patch works, thanks

[Ch3LL]

Yeah, I just recently pushed up a fix for that call. I'm currently adding tests (that's how I found that issue) and fixing up the rest of the functions in salt-ssh's state.py. Thanks for confirming. I'll go ahead and close in favor of https://github.com/saltstack/salt/pull/63184/files