saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Install Salt from the Salt package repositories here:
https://docs.saltproject.io/salt/install-guide/en/latest/
Apache License 2.0
14.19k stars 5.48k forks source link

[BUG] Extending a state with an empty body triggers StopIteration in state compiler #65357

Open a-wildman opened 1 year ago

a-wildman commented 1 year ago

Description An empty state declaration (e.g. empty_declaration: {}) is legal, and is a no-op. Extending the state (via extends) with a similarly empty body triggers a StopIteration in the state compiler.

Setup (Please provide relevant configs and/or SLS files (be sure to remove sensitive info. There is no general set-up of Salt.)

Please be as specific as possible and give set-up details.

Steps to Reproduce the behavior

extend_fail.sls:

empty_declaration: {}

extend:
  empty_declaration: {}

State application:

$ salt-call state.apply extend_fail saltenv=base
[ERROR   ] An un-handled exception was caught by Salt's global exception handler:
StopIteration: 
Traceback (most recent call last):
  File "/opt/salt/salt-call", line 23, in <module>
    sys.exit(salt_call())
  File "/opt/salt/lib/python3.10/site-packages/salt/scripts.py", line 443, in salt_call
    client.run()
  File "/opt/salt/lib/python3.10/site-packages/salt/cli/call.py", line 50, in run
    caller.run()
  File "/opt/salt/lib/python3.10/site-packages/salt/cli/caller.py", line 95, in run
    ret = self.call()
  File "/opt/salt/lib/python3.10/site-packages/salt/cli/caller.py", line 202, in call
    ret["return"] = self.minion.executors[fname](
  File "/opt/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 159, in __call__
    ret = self.loader.run(run_func, *args, **kwargs)
  File "/opt/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1245, in run
    return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
  File "/opt/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1260, in _run_as
    return _func_or_method(*args, **kwargs)
  File "/opt/salt/lib/python3.10/site-packages/salt/executors/direct_call.py", line 10, in execute
    return func(*args, **kwargs)
  File "/opt/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 159, in __call__
    ret = self.loader.run(run_func, *args, **kwargs)
  File "/opt/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1245, in run
    return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
  File "/opt/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1260, in _run_as
    return _func_or_method(*args, **kwargs)
  File "/opt/salt/lib/python3.10/site-packages/salt/modules/state.py", line 833, in apply_
    return sls(mods, **kwargs)
  File "/opt/salt/lib/python3.10/site-packages/salt/modules/state.py", line 1479, in sls
    ret = st_.state.call_high(high_, orchestration_jid)
  File "/opt/salt/lib/python3.10/site-packages/salt/state.py", line 3493, in call_high
    high, ext_errors = self.reconcile_extend(high)
  File "/opt/salt/lib/python3.10/site-packages/salt/state.py", line 1731, in reconcile_extend
    state_type = next(x for x in body if not x.startswith("__"))
StopIteration
Traceback (most recent call last):
  File "/opt/salt/salt-call", line 23, in <module>
    sys.exit(salt_call())
  File "/opt/salt/lib/python3.10/site-packages/salt/scripts.py", line 443, in salt_call
    client.run()
  File "/opt/salt/lib/python3.10/site-packages/salt/cli/call.py", line 50, in run
    caller.run()
  File "/opt/salt/lib/python3.10/site-packages/salt/cli/caller.py", line 95, in run
    ret = self.call()
  File "/opt/salt/lib/python3.10/site-packages/salt/cli/caller.py", line 202, in call
    ret["return"] = self.minion.executors[fname](
  File "/opt/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 159, in __call__
    ret = self.loader.run(run_func, *args, **kwargs)
  File "/opt/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1245, in run
    return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
  File "/opt/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1260, in _run_as
    return _func_or_method(*args, **kwargs)
  File "/opt/salt/lib/python3.10/site-packages/salt/executors/direct_call.py", line 10, in execute
    return func(*args, **kwargs)
  File "/opt/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 159, in __call__
    ret = self.loader.run(run_func, *args, **kwargs)
  File "/opt/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1245, in run
    return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
  File "/opt/salt/lib/python3.10/site-packages/salt/loader/lazy.py", line 1260, in _run_as
    return _func_or_method(*args, **kwargs)
  File "/opt/salt/lib/python3.10/site-packages/salt/modules/state.py", line 833, in apply_
    return sls(mods, **kwargs)
  File "/opt/salt/lib/python3.10/site-packages/salt/modules/state.py", line 1479, in sls
    ret = st_.state.call_high(high_, orchestration_jid)
  File "/opt/salt/lib/python3.10/site-packages/salt/state.py", line 3493, in call_high
    high, ext_errors = self.reconcile_extend(high)
  File "/opt/salt/lib/python3.10/site-packages/salt/state.py", line 1731, in reconcile_extend
    state_type = next(x for x in body if not x.startswith("__"))
StopIteration

Expected behavior The above example compiles cleanly, and (expectedly) does nothing, i.e.

local:

Summary for local
-----------
Succeeded: 0
Failed:   0
-----------
Total states run:    0
Total run time:  0.000 ms

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.) ```yaml Salt Version: Salt: 3006.3 Python Version: Python: 3.10.13 (main, Sep 6 2023, 02:16:00) [Clang 14.0.0 (clang-1400.0.29.202)] Dependency Versions: cffi: 1.14.6 cherrypy: 18.6.1 dateutil: 2.8.0 docker-py: Not Installed gitdb: 4.0.5 gitpython: 3.1.32 Jinja2: 3.1.2 libgit2: Not Installed looseversion: 1.0.2 M2Crypto: Not Installed Mako: Not Installed msgpack: 1.0.2 msgpack-pure: Not Installed mysql-python: Not Installed packaging: 22.0 pycparser: 2.21 pycrypto: Not Installed pycryptodome: 3.9.8 pygit2: Not Installed python-gnupg: 0.4.8 PyYAML: 6.0.1 PyZMQ: 23.2.0 relenv: 0.13.10 smmap: 3.0.2 timelib: 0.2.4 Tornado: 4.5.3 ZMQ: 4.3.4 System Versions: dist: darwin 22.6.0 locale: utf-8 machine: x86_64 release: 22.6.0 system: Darwin version: 13.5.1 x86_64 ```

Additional context This issue does not present in Salt 3002.x

git blame suggests that the regression was introduced in the following commit: https://github.com/saltstack/salt/commit/8ec0c32514587869cad565cf4d58e6027c828dfa

OrangeDog commented 1 year ago

Is there documentation that an empty state is legal?

If you want a no-op state you are supposed to use test.nop.

a-wildman commented 1 year ago

I'm not aware of any documentation which says this is legal (or illegal, for that matter!) -- this may very well be an undocumented corner case. If it's illegal, that's fine (I understand the use case for test.nop), but in that scenario, the state compiler should definitely inform the user of that instead of raising an unhandled exception.

FWIW, I encountered this issue not in my own Salt definitions, but in a zabbix-formula state (in the context of an os_family which falls into the else-its-nothing case here)

a-wildman commented 1 year ago

For clarity, the empty state declaration itself does not trigger the issue -- the empty extends does.

Compiles cleanly:

empty_declaration: {}

Raises StopIteration:

empty_declaration: {}

extend:
  empty_declaration: {}
Ch3LL commented 1 year ago

When I'm able to dive into the code for this issue I'll know what is expected behavior but I do agree we shouldn't be stack tracing here, so we should atleast clean that up. I'll assign to myself to look at a later date.