saltstack / saltstack/salt

sls_exists incorrect behavior?

Open
#55,072 18 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

needs-more-info pending-discussion
Dominant language
Python
Stars
15.7k
Forks
5.6k
Avg merge
2d 44m
Merged PRs (30d)
80

Description

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

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the sls_exists entry point and the slsutil.renderer reproduction described in the issue. Trace how rendering failures are handled when checking roles-role1.sls, then reproduce the syntax-error and missing-include cases with salt-call --local. Done means rendering errors are surfaced instead of being treated as a nonexistent state.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
infrastructure
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.