From 3ded27fe35c5f63b51174b967c00cd4d019c51f3 Mon Sep 17 00:00:00 2001 From: Michael DeHaan Date: Fri, 30 Mar 2012 22:47:58 -0400 Subject: [PATCH] Treat module args as strings everywhere to avoid unneccessary shlex and requoting --- bin/ansible | 4 ++-- lib/ansible/playbook.py | 2 +- lib/ansible/runner.py | 39 +++++++++------------------------------ test/TestRunner.py | 6 ++++-- 4 files changed, 16 insertions(+), 35 deletions(-) diff --git a/bin/ansible b/bin/ansible index cb853eaa504..75529ec9ae2 100755 --- a/bin/ansible +++ b/bin/ansible @@ -104,7 +104,7 @@ class Cli(object): runner = ansible.runner.Runner( module_name=options.module_name, module_path=options.module_path, - module_args=shlex.split(options.module_args), + module_args=options.module_args, remote_user=options.remote_user, remote_pass=sshpass, host_list=options.inventory, timeout=options.timeout, remote_port=options.remote_port, forks=options.forks, @@ -119,7 +119,7 @@ class Cli(object): def get_polling_runner(self, old_runner, hosts, jid): return ansible.runner.Runner( module_name='async_status', module_path=old_runner.module_path, - module_args=[ "jid=%s" % jid ], remote_user=old_runner.remote_user, + module_args="jid=%s" % jid, remote_user=old_runner.remote_user, remote_pass=old_runner.remote_pass, host_list=hosts, timeout=old_runner.timeout, forks=old_runner.forks, remote_port=old_runner.remote_port, pattern='*', diff --git a/lib/ansible/playbook.py b/lib/ansible/playbook.py index 494b09cc809..4881243f154 100755 --- a/lib/ansible/playbook.py +++ b/lib/ansible/playbook.py @@ -221,7 +221,7 @@ class PlayBook(object): while (clock >= 0): # poll/loop until polling duration complete - runner.module_args = [ "jid=%s" % jid ] + runner.module_args = "jid=%s" % jid runner.module_name = 'async_status' runner.background = 0 runner.pattern = '*' diff --git a/lib/ansible/runner.py b/lib/ansible/runner.py index 2d7ea7a11ce..ef8b72423a9 100755 --- a/lib/ansible/runner.py +++ b/lib/ansible/runner.py @@ -111,6 +111,9 @@ class Runner(object): self.basedir = basedir self.sudo = sudo + if type(self.module_args) != str: + raise Exception("module_args must be a string: %s" % self.module_args) + self._tmp_paths = {} random.seed() @@ -327,24 +330,10 @@ class Runner(object): # ***************************************************** - def _coerce_args_to_string(self, args, remote_module_path): - ''' final arguments must always be made a string ''' - - if type(args) == list: - if remote_module_path.endswith('setup'): - # quote long strings so setup module gets them unscathed - args = " ".join([ "\"%s\"" % str(x) for x in args ]) - else: - args = " ".join([ str(x) for x in args ]) - return args - - # ***************************************************** - def _execute_module(self, conn, tmp, remote_module_path, args, async_jid=None, async_module=None, async_limit=None): ''' runs a module that has already been transferred ''' - args = self._coerce_args_to_string(args, remote_module_path) inject = self.setup_cache.get(conn.host,{}) conditional = utils.double_template(self.conditional, inject) if not eval(conditional): @@ -393,13 +382,9 @@ class Runner(object): ''' transfer & execute a module that is not 'copy' or 'template' ''' # shell and command are the same module - # FIXME: keep these args as strings as long as possible... if module_name == 'shell': module_name = 'command' - if type(self.module_args) == list: - self.module_args.append("#USE_SHELL") - else: - self.module_args += " #USE_SHELL" + self.module_args += " #USE_SHELL" module = self._transfer_module(conn, tmp, module_name) (result, executed) = self._execute_module(conn, tmp, module, self.module_args) @@ -419,12 +404,7 @@ class Runner(object): module_args = self.module_args if module_name == 'shell': module_name = 'command' - # FIXME: this will become cleaner once we keep args as a string - # throughout the app - if type(module_args) == list: - module_args.append("#USE_SHELL") - else: - module_args += " #USE_SHELL" + module_args += " #USE_SHELL" async = self._transfer_module(conn, tmp, 'async_wrapper') module = self._transfer_module(conn, tmp, module_name) @@ -455,7 +435,7 @@ class Runner(object): module = self._transfer_module(conn, tmp, 'copy') # run the copy module - args = [ "src=%s" % tmp_src, "dest=%s" % dest ] + args = "src=%s dest=%s" % (tmp_src, dest) (result1, executed) = self._execute_module(conn, tmp, module, args) (host, ok, data) = self._return_from_module(conn, host, result1, executed) @@ -471,7 +451,7 @@ class Runner(object): old_changed = data.get('changed', False) module = self._transfer_module(conn, tmp, 'file') - args = [ "%s=%s" % (k,v) for (k,v) in options.items() ] + args = ' '.join([ "%s=%s" % (k,v) for (k,v) in options.items() ]) (result2, executed2) = self._execute_module(conn, tmp, module, args) results2 = self._return_from_module(conn, conn.host, result2, executed) (host, ok, data2) = results2 @@ -506,7 +486,7 @@ class Runner(object): template_module = self._transfer_module(conn, tmp, 'template') # run the template module - args = [ "src=%s" % temppath, "dest=%s" % dest, "metadata=%s" % metadata ] + args = "src=%s dest=%s metadata=%s" % (temppath, dest, metadata) (result1, executed) = self._execute_module(conn, tmp, template_module, args) (host, ok, data) = self._return_from_module(conn, host, result1, executed) @@ -616,8 +596,7 @@ class Runner(object): def _match_hosts(self, pattern): ''' return all matched hosts fitting a pattern ''' - rc = [ h for h in self.host_list if self._matches(h, pattern) ] - return rc + return [ h for h in self.host_list if self._matches(h, pattern) ] # ***************************************************** diff --git a/test/TestRunner.py b/test/TestRunner.py index ca77e24f239..625fc6da79f 100644 --- a/test/TestRunner.py +++ b/test/TestRunner.py @@ -21,7 +21,7 @@ class TestRunner(unittest.TestCase): self.runner = ansible.runner.Runner( module_name='ping', module_path='library/', - module_args=[], + module_args='', remote_user=self.user, remote_pass=None, host_list='test/ansible_hosts', @@ -58,7 +58,9 @@ class TestRunner(unittest.TestCase): def _run(self, module_name, module_args, background=0): ''' run a module and get the localhost results ''' self.runner.module_name = module_name - self.runner.module_args = module_args + args = ' '.join(module_args) + print "DEBUG: using args=%s" % args + self.runner.module_args = args self.runner.background = background results = self.runner.run() # when using nosetests this will only show up on failure