From 6bbac64d3ad5582b3147088a708952df185db020 Mon Sep 17 00:00:00 2001 From: Evan Borgstrom Date: Wed, 20 Jan 2016 09:57:16 -0800 Subject: [PATCH 1/3] Add test to prove that recursive imports are currently broken Refs #30465 This also exposes some other issues with the Registry being in an inconsistent state when an exception occurs when we are rendering. --- tests/unit/pyobjects_test.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/unit/pyobjects_test.py b/tests/unit/pyobjects_test.py index 94efdc74e5dd..763fd583df33 100644 --- a/tests/unit/pyobjects_test.py +++ b/tests/unit/pyobjects_test.py @@ -86,6 +86,18 @@ class Ubuntu: Pkg.removed("samba-imported", names=[Samba.server, Samba.client]) ''' +recursive_map_template = '''#!pyobjects +from salt://map.sls import Samba + +class CustomSamba(Samba): + pass +''' + +recursive_import_template = '''#!pyobjects +from salt://recursive_map.sls import CustomSamba + +Pkg.removed("samba-imported", names=[CustomSamba.server, CustomSamba.client])''' + from_import_template = '''#!pyobjects # this spacing is like this on purpose to ensure it's stripped properly from salt://map.sls import Samba @@ -326,6 +338,9 @@ def render_and_assert(template): render_and_assert(from_import_template) render_and_assert(import_as_template) + self.write_template_file("recursive_map.sls", recursive_map_template) + render_and_assert(recursive_import_template) + def test_random_password(self): '''Test for https://github.com/saltstack/salt/issues/21796''' ret = self.render(random_password_template) From e148ea2d521313579f661373fbb93a48a5a6d40d Mon Sep 17 00:00:00 2001 From: Evan Borgstrom Date: Wed, 20 Jan 2016 11:20:50 -0800 Subject: [PATCH 2/3] Allow recursive salt:// imports Fixes #30465 --- salt/renderers/pyobjects.py | 124 ++++++++++++++++++++--------------- tests/unit/pyobjects_test.py | 21 +++++- 2 files changed, 92 insertions(+), 53 deletions(-) diff --git a/salt/renderers/pyobjects.py b/salt/renderers/pyobjects.py index 36e3f94f905c..e0b791a8a3eb 100644 --- a/salt/renderers/pyobjects.py +++ b/salt/renderers/pyobjects.py @@ -261,6 +261,7 @@ class RedHat: # Import Python Libs from __future__ import absolute_import import logging +import os import re # Import Salt Libs @@ -284,6 +285,15 @@ class RedHat: __context__ = {} +class PyobjectsModule(object): + '''This projects a wrapper for bare imports.''' + def __init__(self, name, attrs): + self.name = name + self.__dict__ = attrs + def __repr__(self): + return "" % self.name + + def load_states(): ''' This loads our states into the salt __context__ @@ -377,59 +387,69 @@ def render(template, saltenv='base', sls='', salt_data=True, **kwargs): # so that they may bring in objects from other files. while we do this we # disable the registry since all we're looking for here is python objects, # not salt state data - template_data = [] Registry.enabled = False - for line in template.readlines(): - line = line.rstrip('\r\n') - matched = False - for RE in (IMPORT_RE, FROM_RE): - matches = RE.match(line) - if not matches: - continue - - import_file = matches.group(1).strip() - try: - imports = matches.group(2).split(',') - except IndexError: - # if we don't have a third group in the matches object it means - # that we're importing everything - imports = None - - state_file = client.cache_file(import_file, saltenv) - if not state_file: - raise ImportError("Could not find the file {0!r}".format(import_file)) - - with salt.utils.fopen(state_file) as f: - state_contents = f.read() - - state_locals = {} - exec_(state_contents, _globals, state_locals) - - if imports is None: - imports = list(state_locals) - - for name in imports: - name = alias = name.strip() - - matches = FROM_AS_RE.match(name) - if matches is not None: - name = matches.group(1).strip() - alias = matches.group(2).strip() - - if name not in state_locals: - raise ImportError("{0!r} was not found in {1!r}".format( - name, - import_file - )) - _globals[alias] = state_locals[name] - - matched = True - break - - if not matched: - template_data.append(line) - - final_template = "\n".join(template_data) + + def process_template(template, template_globals): + template_data = [] + state_globals = {} + for line in template.readlines(): + line = line.rstrip('\r\n') + matched = False + for RE in (IMPORT_RE, FROM_RE): + matches = RE.match(line) + if not matches: + continue + + import_file = matches.group(1).strip() + try: + imports = matches.group(2).split(',') + except IndexError: + # if we don't have a third group in the matches object it means + # that we're importing everything + imports = None + + state_file = client.cache_file(import_file, saltenv) + if not state_file: + raise ImportError("Could not find the file {0!r}".format(import_file)) + + state_locals = {} + with salt.utils.fopen(state_file) as state_fh: + state_contents, state_locals = process_template(state_fh, template_globals) + exec_(state_contents, template_globals, state_locals) + + # if no imports have been specified then we are being imported as: import salt://foo.sls + # so we want to stick all of the locals from our state file into the template globals + # under the name of the module -> i.e. foo.MapClass + if imports is None: + import_name = os.path.splitext(os.path.basename(state_file))[0] + state_globals[import_name] = PyobjectsModule(import_name, state_locals) + else: + for name in imports: + name = alias = name.strip() + + matches = FROM_AS_RE.match(name) + if matches is not None: + name = matches.group(1).strip() + alias = matches.group(2).strip() + + if name not in state_locals: + raise ImportError("{0!r} was not found in {1!r}".format( + name, + import_file + )) + state_globals[alias] = state_locals[name] + + matched = True + break + + if not matched: + template_data.append(line) + + return "\n".join(template_data), state_globals + + # process the template that triggered the render + final_template, final_locals = process_template(template, _globals) + _globals.update(final_locals) # re-enable the registry Registry.enabled = True diff --git a/tests/unit/pyobjects_test.py b/tests/unit/pyobjects_test.py index 763fd583df33..f1c3e29ad314 100644 --- a/tests/unit/pyobjects_test.py +++ b/tests/unit/pyobjects_test.py @@ -83,7 +83,7 @@ class Ubuntu: import_template = '''#!pyobjects import salt://map.sls -Pkg.removed("samba-imported", names=[Samba.server, Samba.client]) +Pkg.removed("samba-imported", names=[map.Samba.server, map.Samba.client]) ''' recursive_map_template = '''#!pyobjects @@ -98,6 +98,12 @@ class CustomSamba(Samba): Pkg.removed("samba-imported", names=[CustomSamba.server, CustomSamba.client])''' +scope_test_import_template = '''#!pyobjects +from salt://recursive_map.sls import CustomSamba + +# since we import CustomSamba we should shouldn't be able to see Samba +Pkg.removed("samba-imported", names=[Samba.server, Samba.client])''' + from_import_template = '''#!pyobjects # this spacing is like this on purpose to ensure it's stripped properly from salt://map.sls import Samba @@ -341,6 +347,19 @@ def render_and_assert(template): self.write_template_file("recursive_map.sls", recursive_map_template) render_and_assert(recursive_import_template) + def test_import_scope(self): + self.write_template_file("map.sls", map_template) + self.write_template_file("recursive_map.sls", recursive_map_template) + + def do_render(): + ret = self.render(scope_test_import_template, + {'grains': { + 'os_family': 'Debian', + 'os': 'Debian' + }}) + + self.assertRaises(NameError, do_render) + def test_random_password(self): '''Test for https://github.com/saltstack/salt/issues/21796''' ret = self.render(random_password_template) From 788b672e3aee5df3f5738e05097eb9dfed8643a0 Mon Sep 17 00:00:00 2001 From: Evan Borgstrom Date: Wed, 20 Jan 2016 15:28:48 -0800 Subject: [PATCH 3/3] Fixup lint errors --- salt/renderers/pyobjects.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/salt/renderers/pyobjects.py b/salt/renderers/pyobjects.py index e0b791a8a3eb..f31f1f179eca 100644 --- a/salt/renderers/pyobjects.py +++ b/salt/renderers/pyobjects.py @@ -286,12 +286,14 @@ class RedHat: class PyobjectsModule(object): - '''This projects a wrapper for bare imports.''' + '''This provides a wrapper for bare imports.''' + def __init__(self, name, attrs): self.name = name self.__dict__ = attrs + def __repr__(self): - return "" % self.name + return "".format(self.name) def load_states():