diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index 50bd99173c7..6f51ac832bb 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -757,11 +757,6 @@ class Play(object): # ************************************************* - def _has_vars_in(self, msg): - return "$" in msg or "{{" in msg - - # ************************************************* - def _update_vars_files_for_host(self, host, vault_password=None): def generate_filenames(host, inject, filename): @@ -782,7 +777,7 @@ class Play(object): # filename4 is the dwim'd path, but may also be mixed-scope, so we use # both play scoped vars and host scoped vars to template the filepath - if self._has_vars_in(filename3) and host is not None: + if utils.contains_vars(filename3) and host is not None: inject.update(self.vars) filename4 = template(self.basedir, filename3, inject) filename4 = utils.path_dwim(self.basedir, filename4) @@ -810,8 +805,8 @@ class Play(object): raise errors.AnsibleError("%s must be stored as a dictionary/hash" % filename4) if host is not None: target_filename = None - if self._has_vars_in(filename2): - if not self._has_vars_in(filename3): + if utils.contains_vars(filename2): + if not utils.contains_vars(filename3): target_filename = filename3 else: target_filename = filename4 @@ -860,7 +855,7 @@ class Play(object): else: # just one filename supplied, load it! filename2, filename3, filename4 = generate_filenames(host, inject, filename) - if self._has_vars_in(filename4): + if utils.contains_vars(filename4): continue if process_files(filename, filename2, filename3, filename4, host=host): processed.append(filename) diff --git a/lib/ansible/runner/__init__.py b/lib/ansible/runner/__init__.py index b7bfc7bd3c0..4c566f163e5 100644 --- a/lib/ansible/runner/__init__.py +++ b/lib/ansible/runner/__init__.py @@ -226,10 +226,6 @@ class Runner(object): # changed later via options like accelerate self.original_transport = self.transport - # enforce complex_args as a dict - if type(self.complex_args) != dict: - raise errors.AnsibleError("args must be a dictionary, received %s (%s)" % (self.complex_args, type(self.complex_args))) - # misc housekeeping if subset and self.inventory._subset is None: # don't override subset when passed from playbook @@ -677,11 +673,37 @@ class Runner(object): inject['item'] = ",".join(use_these_items) items = None - # logic to replace complex args if possible - complex_args = self.complex_args + def _safe_template_complex_args(args, inject): + # Ensure the complex args here are a dictionary, but + # first template them if they contain a variable + returned_args = args + if isinstance(args, basestring): + # If the complex_args were evaluated to a dictionary and there are + # more keys in the templated version than the evaled version, some + # param inserted additional keys (the template() call also runs + # safe_eval on the var if it looks like it's a datastructure). If the + # evaled_args are not a dict, it's most likely a whole variable (ie. + # args: {{var}}), in which case there's no way to detect the proper + # count of params in the dictionary. + + templated_args = template.template(self.basedir, args, inject, convert_bare=True) + evaled_args = utils.safe_eval(args) + + if isinstance(evaled_args, dict) and len(evaled_args) > 0 and len(evaled_args) != len(templated_args): + raise errors.AnsibleError("a variable tried to insert extra parameters into the args for this task") + + # set the returned_args to the templated_args + returned_args = templated_args + + # and a final check to make sure the complex args are a dict + if not isinstance(returned_args, dict): + raise errors.AnsibleError("args must be a dictionary, received %s" % returned_args) + + return returned_args # logic to decide how to run things depends on whether with_items is used if items is None: + complex_args = _safe_template_complex_args(self.complex_args, inject) return self._executor_internal_inner(host, self.module_name, self.module_args, inject, port, complex_args=complex_args) elif len(items) > 0: @@ -700,12 +722,8 @@ class Runner(object): this_inject = inject.copy() this_inject['item'] = x - # TODO: this idiom should be replaced with an up-conversion to a Jinja2 template evaluation - if isinstance(self.complex_args, basestring): - complex_args = template.template(self.basedir, self.complex_args, this_inject, convert_bare=True) - complex_args = utils.safe_eval(complex_args) - if type(complex_args) != dict: - raise errors.AnsibleError("args must be a dictionary, received %s" % complex_args) + complex_args = _safe_template_complex_args(self.complex_args, this_inject) + result = self._executor_internal_inner( host, self.module_name, diff --git a/lib/ansible/utils/__init__.py b/lib/ansible/utils/__init__.py index aa0a1b8ee71..979260d2759 100644 --- a/lib/ansible/utils/__init__.py +++ b/lib/ansible/utils/__init__.py @@ -1268,6 +1268,12 @@ def list_difference(a, b): result.append(x) return result +def contains_vars(data): + ''' + returns True if the data contains a variable pattern + ''' + return "$" in data or "{{" in data + def safe_eval(expr, locals={}, include_exceptions=False): ''' This is intended for allowing things like: diff --git a/test/units/TestUtils.py b/test/units/TestUtils.py index 08fb0370094..9ed0cd860df 100644 --- a/test/units/TestUtils.py +++ b/test/units/TestUtils.py @@ -518,6 +518,11 @@ class TestUtils(unittest.TestCase): self.assertEqual(ansible.utils.is_list_of_strings(['foo', 'bar', True]), False) self.assertEqual(ansible.utils.is_list_of_strings(['one', 2, 'three']), False) + def test_contains_vars(self): + self.assertTrue(ansible.utils.contains_vars('{{foo}}')) + self.assertTrue(ansible.utils.contains_vars('$foo')) + self.assertFalse(ansible.utils.contains_vars('foo')) + def test_safe_eval(self): # Not basestring self.assertEqual(ansible.utils.safe_eval(len), len)