AttributeError in _handle_iorder() (bad syntax needs better error message)

[holmboe]

When running salt minion state.show_sls myfile I get this:

[DEBUG   ] Results of YAML rendering: 
OrderedDict([('/dev/null', OrderedDict([('file.mknod', OrderedDict([('ntype', 'c'), ('major', 1), ('minor', 3), ('user', 'root'), ('group', 'root'), ('mode', 666)]))]))])
[WARNING ] The minion function caused an exception
Traceback (most recent call last):
  File "/vagrant/salt/minion.py", line 705, in _thread_return
    return_data = func(*args, **kwargs)
  File "/vagrant/salt/modules/state.py", line 341, in sls
    high_, errors = st_.render_highstate({env: mods})
  File "/vagrant/salt/state.py", line 2224, in render_highstate
    state, errors = self.render_state(sls, env, mods, matches)
  File "/vagrant/salt/state.py", line 2066, in render_state
    self._handle_iorder(state)
  File "/vagrant/salt/state.py", line 2096, in _handle_iorder
    state[name][s_dec].append(
AttributeError: 'OrderedDict' object has no attribute 'append'

The SLS-file:

/dev/null:
  file.mknod:
    ntype: c
    major: 1
    minor: 3
    user: root
    group: root
    mode: 0666

[holmboe]

Forgot to mention, this is running latest develop. I have not tested with 0.17.1.

[regilero]

+1, salt 0.17.0-1471-g55841bf, python 2.7.5+

test.sls file:

check-foobar-exists:
  file.exists:
    -name: /foo/bar

Calling it salt-call -l debug state.sls example.test

Output:

(...)
[DEBUG   ] Results of YAML rendering: 
OrderedDict([('check-foobar-exists', OrderedDict([('file.exists', OrderedDict([('-name', '/foo/bar')]))]))])
(...)
AttributeError: 'OrderedDict' object has no attribute 'append'
Traceback (most recent call last):
  File "/srv/salt/makina-states/bin/salt-call", line 28, in 
    sys.exit(salt.scripts.salt_call())
  File "/srv/salt/makina-states/src/salt/salt/scripts.py", line 77, in salt_call
    client.run()
  File "/srv/salt/makina-states/src/salt/salt/cli/__init__.py", line 308, in run
    caller.run()
  File "/srv/salt/makina-states/src/salt/salt/cli/caller.py", line 137, in run
    ret = self.call()
  File "/srv/salt/makina-states/src/salt/salt/cli/caller.py", line 78, in call
    ret['return'] = func(*args, **kwargs)
  File "/srv/salt/makina-states/src/salt/salt/modules/state.py", line 341, in sls
    high_, errors = st_.render_highstate({env: mods})
  File "/srv/salt/makina-states/src/salt/salt/state.py", line 2224, in render_highstate
    state, errors = self.render_state(sls, env, mods, matches)
  File "/srv/salt/makina-states/src/salt/salt/state.py", line 2066, in render_state
    self._handle_iorder(state)
  File "/srv/salt/makina-states/src/salt/salt/state.py", line 2096, in _handle_iorder
    state[name][s_dec].append(
AttributeError: 'OrderedDict' object has no attribute 'append'

pdb in https://github.com/saltstack/salt/blob/develop/salt/state.py#L2096 :

(Pdb) name
'check-foobar-exists'
(Pdb) s_dec
'file.exists'
(Pdb) state[name][s_dec]
OrderedDict([('-name', '/foo/bar')])

[regilero]

Note that using update instead of append fixed the bug, or at least it allows me to have a 'readable error message:

local:
    Data failed to compile:
----------
    The state "check-foobar-exists" in sls makina-states.test is not formed as a list

As this was a syntax error in the sls. Now the main problem is that we have an OrederDict instead of a list when there is a syntax error. This is related to issue #7905, and the issue is in fact that the syntax error cannot be properly detected.

Here the syntax error was a missing space before name, for @holmboe the error is maybe some missing -.

This should fix the problem:

diff --git a/salt/state.py b/salt/state.py
index caa3349..38788dc 100644
--- a/salt/state.py
+++ b/salt/state.py
@@ -2093,6 +2093,9 @@ class BaseHighState(object):
                                 if arg.keys()[0] == 'order':
                                     found = True
                     if not found:
+                        if not isinstance(state[name][s_dec], list):
+                            # quite certainly a syntax error, managed elsewhere
+                            continue
                         state[name][s_dec].append(
                                 {'order': self.iorder}
                                 )

[holmboe]

Oh I also tried with update instead of append but didn't have time to look further into it.

I can verify that adding - in my SLS solves the problem for me. Though a better error message should be the real solution I guess.

Thanks @regilero!

[basepi]

@regilero Good work tracking this down! A little while ago I think there was a change to use OrderedDict instead of dict, and it's possible that this broke an isinstance(blah, dict) check or similar somewhere else in the code.

@regilero when you say that that patch fixes the problem, do you mean that it outputs the better error message or that no error message is printed? Want to make sure that does throw an error, as that bad syntax is something we definitely don't want to fail silently on.

[regilero]

@basepi This fix allows the syntax error to be shown instead of showing a stack strace. At least in my case the syntax error implied that state[name][s_dec] was an OrderedDict instead of a list, and now the error message is The state "check-foobar-exists" in sls makina-states.test is not formed as a list, so I think the check is done later.

[s0undt3ch]

I reopened and assigned this to me. The bug is fixed but a test case should be added.

[s0undt3ch]

Test case added by @regilero in #8195. Closing.