Skip to content

Commit

Permalink
address circular reference in LazyLoader with ThreadLocalProxy
Browse files Browse the repository at this point in the history
when lazyLoader was getting instantiated from inside already lazyloaded
modules (ie state), pillar/grain (and probably other) globals were
already populated. In this case they weren't getting dereferenced
properly when wrapped via the NamespacedDict wrapper, causing an
infinite loop. This should fix that scenario.

Fixes #50655 (comment)
  • Loading branch information
mattp- committed Feb 5, 2019
1 parent 8af9765 commit 8b6354d
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 5 deletions.
24 changes: 19 additions & 5 deletions salt/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@
import salt.utils.lazy
import salt.utils.odict
import salt.utils.platform
import salt.utils.thread_local_proxy
import salt.utils.versions
import salt.utils.stringutils
from salt.exceptions import LoaderError
from salt.template import check_render_pipe_str
from salt.utils.decorators import Depends
from salt.utils.thread_local_proxy import ThreadLocalProxy

# Import 3rd-party libs
from salt.ext import six
Expand Down Expand Up @@ -1118,7 +1118,6 @@ def _inject_into_mod(mod, name, value, force_lock=False):
module's variable without acquiring the lock and only acquires the lock
if a new proxy has to be created and injected.
'''
from salt.utils.thread_local_proxy import ThreadLocalProxy
old_value = getattr(mod, name, None)
# We use a double-checked locking scheme in order to avoid taking the lock
# when a proxy object has already been injected.
Expand Down Expand Up @@ -1248,7 +1247,12 @@ def __init__(self,

for k, v in six.iteritems(self.pack):
if v is None: # if the value of a pack is None, lets make an empty dict
self.context_dict.setdefault(k, {})
value = self.context_dict.get(k, {})

if isinstance(value, ThreadLocalProxy):
value = ThreadLocalProxy.unproxy(value)

self.context_dict[k] = value
self.pack[k] = salt.utils.context.NamespacedDictWrapper(self.context_dict, k)

self.whitelist = whitelist
Expand Down Expand Up @@ -1526,11 +1530,21 @@ def __prep_mod_opts(self, opts):
Strip out of the opts any logger instance
'''
if '__grains__' not in self.pack:
self.context_dict['grains'] = opts.get('grains', {})
grains = opts.get('grains', {})

if isinstance(grains, ThreadLocalProxy):
grains = ThreadLocalProxy.unproxy(grains)

self.context_dict['grains'] = grains
self.pack['__grains__'] = salt.utils.context.NamespacedDictWrapper(self.context_dict, 'grains')

if '__pillar__' not in self.pack:
self.context_dict['pillar'] = opts.get('pillar', {})
pillar = opts.get('pillar', {})

if isinstance(pillar, ThreadLocalProxy):
pillar = ThreadLocalProxy.unproxy(pillar)

self.context_dict['pillar'] = pillar
self.pack['__pillar__'] = salt.utils.context.NamespacedDictWrapper(self.context_dict, 'pillar')

mod_opts = {}
Expand Down
6 changes: 6 additions & 0 deletions tests/integration/files/file/base/issue-51499.sls
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{% if 'nonexistent_module.function' in salt %}
{% do salt.log.warning("Module is available") %}
{% endif %}
always-passes:
test.succeed_without_changes:
- name: foo
13 changes: 13 additions & 0 deletions tests/integration/modules/test_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -2171,3 +2171,16 @@ def test_state_sls_integer_name(self):
self.assertEqual(state_run[state_id]['comment'],
'Success!')
self.assertTrue(state_run[state_id]['result'])

def test_state_sls_lazyloader_allows_recursion(self):
'''
This tests that referencing dunders like __salt__ work
context: https://github.com/saltstack/salt/pull/51499
'''
state_run = self.run_function('state.sls', mods='issue-51499')

state_id = 'test_|-always-passes_|-foo_|-succeed_without_changes'
self.assertIn(state_id, state_run)
self.assertEqual(state_run[state_id]['comment'],
'Success!')
self.assertTrue(state_run[state_id]['result'])

0 comments on commit 8b6354d

Please sign in to comment.