sls_exists incorrect behavior?

[cr1st1p]

Description of Issue

sls_exists functionality seems to depend, incorrectly - according to me, to the format/corectness of the file checked for.

Setup

Files in /srv/salt - file_roots points to /srv/salt

basic.sls:

{# nothing in here #}

test2.sls:

{% set role = 'role1' %} {# can come from pillar/grains/whatever #}

include:
  - basic
{% if salt['state.sls_exists']('roles-{0}'.format(role)) %}
  - {{ 'roles-{0}'.format(role) }}
{% endif %}

Steps to Reproduce Issue

salt is installed locally, and setup is to load files from /srv/salt.
I'm going to use 'salt-call' to demonstrate, but it happens also in master/minion setup.

With the 2 files above:

 ▲ /srv/salt salt-call --local slsutil.renderer /srv/salt/test2.sls
local:
    ----------
    include:
        - basic

Correct. Let's continue:

△ /srv/salt touch roles-role1.sls

 ▲ /srv/salt salt-call --local slsutil.renderer /srv/salt/test2.sls
local:
    ----------
    include:
        - basic
        - roles-role1

Correct. Let's continue:

 ▲ /srv/salt echo 'asdas asd asd asd' > roles-role1.sls                        

 ▲ /srv/salt salt-call --local slsutil.renderer /srv/salt/test2.sls
local:
    ----------
    include:
        - basic

Incorrect!!! Syntax error made the file not be 'detected' at all!
In a production environment I really want to have the error!! My concrete case was a 'include:' with no lines in it because other jinja code did not produce line items, so the state ended in success while it should have NOT.

Ok, so... bad for the syntax error. Let's continue:

 △ /srv/salt echo -e 'include:\n  - missing' > roles-role1.sls

 ▲ /srv/salt salt-call --local slsutil.renderer /srv/salt/test2.sls
local:
    ----------
    include:
        - basic

Wait? What? C'mon!
Let's see this as well:

 △ /srv/salt echo -e 'include:\n  - basic' > roles-role1.sls                   9s 

 ▲ /srv/salt salt-call --local slsutil.renderer /srv/salt/test2.sls
local:
    ----------
    include:
        - basic
        - roles-role1

Correct.

So, sls_exists incorrectly treats what seems to be a rendering error (be it syntax issue or some other deeper thing) as state not existing.
It shouldn't.

Versions Report

Salt Version:
           Salt: 2019.2.0
 
Dependency Versions:
           cffi: 1.12.3
       cherrypy: Not Installed
       dateutil: 2.8.0
      docker-py: 3.5.0
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.10.3
        libgit2: 0.28.2
        libnacl: Not Installed
       M2Crypto: 0.35.2
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.6.2
   mysql-python: 1.2.5
      pycparser: 2.17
       pycrypto: 3.9.0
   pycryptodome: 3.9.0
         pygit2: 0.28.2
         Python: 2.7.16 (default, Mar 11 2019, 18:59:25)
   python-gnupg: Not Installed
         PyYAML: 5.1.2
          PyZMQ: 18.0.2
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 5.1.1
            ZMQ: 4.3.2
 
System Versions:
           dist:   
         locale: UTF-8
        machine: x86_64
        release: 4.19.78-1-MANJARO
         system: Linux
        version: Not Installed

[OrangeDog]

It's not checking whether a file exists, but whether an SLS (salt state) exists. If the file cannot be correctly read, then the state it attempts to describe cannot be created, and therefore doesn't exist.

[cr1st1p]

@OrangeDog :
a) Documentation mentions "SLS files" all over, and I expect people would think that way anyway
b) you also say in your comment 'if the file cannot be correctly read'. What's the definition of that?
c) If you will insist on redefining 'SLS exists' as 'SLS file that correctly renders', then I would consider that functionality of 'sls_exists' is super limited. Because if something happens with the logic in the searched SLS file I would NEVER detect that. Been there. And of course, even "correctly renders" would need a rephrase, because, as you can see in the one before last example, file is syntactically correct.
c) Btw, in the same before last example, you want to say 'salt state roles-role1.sls' does NOT exist? I would say 'missing.sls is a salt state that does not exist', but 'roles-role1.sls' ? It does even have saltstack content ('include' is saltstack syntax not jinja)

[OrangeDog]

The sate is "missing", the file that defines it is "missing.sls". They're not the same thing.

For the same reason asking whether the file "example.py" exists is different from asking if the module "example" exists. If the former has a fatal error in it, then the latter will report false.

Yes the behaviour of sls_exists is limited to determining whether a salt state exists in the current environment. In your case that should be useful, because if the file is there but the state isn't, then there must be something wrong with the file.

[cr1st1p]

Recap: in that example, test2.sls includes state (file) 'roles-role1(.sls), which is present and has correct saltstack syntax to include also a 'missing(.sls)' state (file) that is missing (as a file).
And 'sls_exists' checks for roles-role1 "state" and NOT 'missing' state.

Quote:

The sate is "missing", the file that defines it is "missing.sls". They're not the same thing.

Can you please review the 'recap' and test example please? Code is checking for state 'roles-role1'. You are saying that 'state roles-role1.sls' is defined by file 'missing.sls' !??

What about if 'roles-role1.sls' would have 2k lines of code, and would include about 100 other files, one of which would have a syntax error or would be missing. you will say 'roles-role1.sls' is a STATE not present and that it is defined by missing state 'some-other-included-but-missing-or-errored.sls' file?!

Regarding the analogy with example.py, I don't think it helps you, because it depends on how you implement it:
a) you can do an import and you get an error and do whatever you want with it) - which is what I expect as well
b) or you can try some importlib code. I just checked, with a t.py with syntax error in it. It returned the module!

 ▲ /tmp cat t.py                                                                                                                                                                        12m 
def p:
 aa

 ▲ /tmp python3 
Python 3.7.4 (default, Oct  4 2019, 06:57:26) 
[GCC 9.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import t
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/tmp/t.py", line 1
    def p:
         ^
SyntaxError: invalid syntax

 ▲ /tmp python3 
Python 3.7.4 (default, Oct  4 2019, 06:57:26) 
[GCC 9.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import importlib
>>> importlib.util.find_spec("t")
ModuleSpec(name='t', loader=<_frozen_importlib_external.SourceFileLoader object at 0x7fd7f947efd0>, origin='/tmp/t.py')

And no, I do NOT know if SLS file exists or not, that is why I want to use 'sls_exists'.
That's what people are trying to do in #20010 as well, were you mentioned them to use 'sls_exists' !!

[OrangeDog]

Can you please review the 'recap' and test example please? Code is checking for state 'roles-role1'. You are saying that 'state roles-role1.sls' is defined by file 'missing.sls' !??

No, the salt state (abbreviated SLS) "roles-role1" is defined by the file "roles-role1.sls".

What about if 'roles-role1.sls' would have 2k lines of code, and would include about 100 other files, one of which would have a syntax error

Then the state "roles-role1" is not present. It is not a state known to the system. It cannot be applied to anything. I would also say "roles-role1.sls" has been written very poorly if it has that much code.

You can do an import and you get an error and do whatever you want with it) - which is what I expect as well

Which is also what happens if you try to load an .sls file that's missing or has syntax errors in it. The errors are logged and the operation fails. If you want to handle the situation then you can use sls_exists first to check.

[cr1st1p]

I agree with you that a 2k state file would be bad code. But that was a fictive example so it doesn't help :-)
I still do not agree with you on the semantics of 'sls_exists' :-)

Anyway, I guess that next steps would then be:

• clarify the docs of sls_exists to NOT say 'SLS files' and INCLUDE information about what happens when checking states represented by files with incorrect syntax or with missing dependencies
• let me know, please, if there is a way to detect SLS file presence. Although based on comments from Include State only if it exists #20010 it looks like there isn't.

[xeacott]

So I read this and, correct me if I am misinterpreting things, it seems as though the root of the issue is that if Jinja fails to render the contents of a state file, then the state file is determined to be MIA by Salt. More-or-less, is that about it?

And so, sls.exists should still return that this, missing.sls file, is still in fact there. But it does/doesn't?

[cr1st1p]

@xeacott: hm.. not exactly. You are only partially right, problem is worse:
a) indeed, sls says MIA if file asked for in the sls_exits() call OR dependency has Jinja problems
b) please double check the example: missing.sls is NOT the file asked for, by sls_exists(). 'missing.sls' is just some internal dependency of 'roles-role1.sls' (this is the state file asked about in sls_exists() ). So, the state file (roles-role1.sls) queried for by sls_exists() exists and is Jinja correct.

[stale]

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.

[johnnybubonic]

please keep open