From d8a243bef0a06c20503db70db4e0c955a4e49734 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Fri, 8 Jan 2016 10:34:46 -0600 Subject: [PATCH 01/15] First pass at allowing binary modules --- lib/ansible/plugins/action/__init__.py | 24 +++++++++++++++++++----- lib/ansible/plugins/action/async.py | 9 ++++++--- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 7a8ee2b4bc8..57e4c2fa408 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -100,6 +100,12 @@ class ActionBase(with_metaclass(ABCMeta, object)): return True return False + def _is_binary(self, module_path): + textchars = bytearray({7,8,9,10,12,13,27} | set(range(0x20, 0x100)) - {0x7f}) + + with open(module_path, 'rb') as f: + start = f.read(1024) + def _configure_module(self, module_name, module_args, task_vars=None): ''' Handles the loading and templating of the module code through the @@ -147,7 +153,10 @@ class ActionBase(with_metaclass(ABCMeta, object)): # insert shared code and arguments into the module (module_data, module_style, module_shebang) = modify_module(module_name, module_path, module_args, task_vars=task_vars, module_compression=self._play_context.module_compression) - return (module_style, module_shebang, module_data) + if self._is_binary(module_path): + return ('non_native_want_json', None, module_path, True) + + return (module_style, module_shebang, module_data, False) def _compute_environment_string(self): ''' @@ -569,8 +578,9 @@ class ActionBase(with_metaclass(ABCMeta, object)): # let module know our verbosity module_args['_ansible_verbosity'] = display.verbosity - (module_style, shebang, module_data) = self._configure_module(module_name=module_name, module_args=module_args, task_vars=task_vars) - if not shebang: + (module_style, shebang, module_data, is_binary) = self._configure_module(module_name=module_name, module_args=module_args, task_vars=task_vars) + + if not shebang and not is_binary: raise AnsibleError("module (%s) is missing interpreter line" % module_name) # a remote tmp path may be necessary and not already created @@ -588,7 +598,11 @@ class ActionBase(with_metaclass(ABCMeta, object)): if remote_module_path or module_style != 'new': display.debug("transferring module to remote") - self._transfer_data(remote_module_path, module_data) + if is_binary: + # If is_binary module_data is the path to the module to transfer + self._transfer_file(module_data, remote_module_path) + else: + self._transfer_data(remote_module_path, module_data, is_path=is_binary) if module_style == 'old': # we need to dump the module args to a k=v string in a file on # the remote system, which can be read and parsed by the module @@ -604,7 +618,7 @@ class ActionBase(with_metaclass(ABCMeta, object)): # Fix permissions of the tmp path and tmp files. This should be # called after all files have been transferred. - self._fixup_perms(tmp, remote_user, recursive=True) + self._fixup_perms(tmp, remote_user, recursive=True, execute=is_binary) cmd = "" in_data = None diff --git a/lib/ansible/plugins/action/async.py b/lib/ansible/plugins/action/async.py index 4f7aa634ce2..f6d1a24c81b 100644 --- a/lib/ansible/plugins/action/async.py +++ b/lib/ansible/plugins/action/async.py @@ -54,11 +54,14 @@ class ActionModule(ActionBase): module_args['_ansible_no_log'] = True # configure, upload, and chmod the target module - (module_style, shebang, module_data) = self._configure_module(module_name=module_name, module_args=module_args, task_vars=task_vars) - self._transfer_data(remote_module_path, module_data) + (module_style, shebang, module_data, is_binary) = self._configure_module(module_name=module_name, module_args=module_args, task_vars=task_vars) + if is_binary: + self._transfer_file(module_data, remote_module_path) + else: + self._transfer_data(remote_module_path, module_data) # configure, upload, and chmod the async_wrapper module - (async_module_style, shebang, async_module_data) = self._configure_module(module_name='async_wrapper', module_args=dict(), task_vars=task_vars) + (async_module_style, shebang, async_module_data, is_binary) = self._configure_module(module_name='async_wrapper', module_args=dict(), task_vars=task_vars) self._transfer_data(async_module_path, async_module_data) argsfile = None From 0a8d016642d022c75927aeb6e1450a995cf31bc5 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 11 Jan 2016 11:07:05 -0600 Subject: [PATCH 02/15] Get binary modules working for windows, assuming .exe for windows --- lib/ansible/plugins/action/__init__.py | 14 +++++++------- lib/ansible/plugins/action/async.py | 6 +++--- lib/ansible/plugins/connection/winrm.py | 2 +- lib/ansible/plugins/shell/powershell.py | 12 +++++++++--- 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 57e4c2fa408..5fa5af9884c 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -105,6 +105,7 @@ class ActionBase(with_metaclass(ABCMeta, object)): with open(module_path, 'rb') as f: start = f.read(1024) + return bool(start.translate(None, textchars)) def _configure_module(self, module_name, module_args, task_vars=None): ''' @@ -154,9 +155,9 @@ class ActionBase(with_metaclass(ABCMeta, object)): (module_data, module_style, module_shebang) = modify_module(module_name, module_path, module_args, task_vars=task_vars, module_compression=self._play_context.module_compression) if self._is_binary(module_path): - return ('non_native_want_json', None, module_path, True) + return ('non_native_want_json', None, None, module_path, True) - return (module_style, module_shebang, module_data, False) + return (module_style, module_shebang, module_data, module_path, False) def _compute_environment_string(self): ''' @@ -578,7 +579,7 @@ class ActionBase(with_metaclass(ABCMeta, object)): # let module know our verbosity module_args['_ansible_verbosity'] = display.verbosity - (module_style, shebang, module_data, is_binary) = self._configure_module(module_name=module_name, module_args=module_args, task_vars=task_vars) + (module_style, shebang, module_data, module_path, is_binary) = self._configure_module(module_name=module_name, module_args=module_args, task_vars=task_vars) if not shebang and not is_binary: raise AnsibleError("module (%s) is missing interpreter line" % module_name) @@ -590,7 +591,7 @@ class ActionBase(with_metaclass(ABCMeta, object)): tmp = self._make_tmp_path(remote_user) if tmp: - remote_module_filename = self._connection._shell.get_remote_filename(module_name) + remote_module_filename = self._connection._shell.get_remote_filename(module_path) remote_module_path = self._connection._shell.join_path(tmp, remote_module_filename) if module_style in ['old', 'non_native_want_json']: # we'll also need a temp file to hold our module arguments @@ -599,10 +600,9 @@ class ActionBase(with_metaclass(ABCMeta, object)): if remote_module_path or module_style != 'new': display.debug("transferring module to remote") if is_binary: - # If is_binary module_data is the path to the module to transfer - self._transfer_file(module_data, remote_module_path) + self._transfer_file(module_path, remote_module_path) else: - self._transfer_data(remote_module_path, module_data, is_path=is_binary) + self._transfer_data(remote_module_path, module_data) if module_style == 'old': # we need to dump the module args to a k=v string in a file on # the remote system, which can be read and parsed by the module diff --git a/lib/ansible/plugins/action/async.py b/lib/ansible/plugins/action/async.py index f6d1a24c81b..a9406421c25 100644 --- a/lib/ansible/plugins/action/async.py +++ b/lib/ansible/plugins/action/async.py @@ -54,14 +54,14 @@ class ActionModule(ActionBase): module_args['_ansible_no_log'] = True # configure, upload, and chmod the target module - (module_style, shebang, module_data, is_binary) = self._configure_module(module_name=module_name, module_args=module_args, task_vars=task_vars) + (module_style, shebang, module_data, module_path, is_binary) = self._configure_module(module_name=module_name, module_args=module_args, task_vars=task_vars) if is_binary: - self._transfer_file(module_data, remote_module_path) + self._transfer_file(module_path, remote_module_path) else: self._transfer_data(remote_module_path, module_data) # configure, upload, and chmod the async_wrapper module - (async_module_style, shebang, async_module_data, is_binary) = self._configure_module(module_name='async_wrapper', module_args=dict(), task_vars=task_vars) + (async_module_style, shebang, async_module_data, async_module_path, is_binary) = self._configure_module(module_name='async_wrapper', module_args=dict(), task_vars=task_vars) self._transfer_data(async_module_path, async_module_data) argsfile = None diff --git a/lib/ansible/plugins/connection/winrm.py b/lib/ansible/plugins/connection/winrm.py index f5023d7efc6..c5efd481f29 100644 --- a/lib/ansible/plugins/connection/winrm.py +++ b/lib/ansible/plugins/connection/winrm.py @@ -62,7 +62,7 @@ class Connection(ConnectionBase): '''WinRM connections over HTTP/HTTPS.''' transport = 'winrm' - module_implementation_preferences = ('.ps1', '') + module_implementation_preferences = ('.ps1', '.exe', '') become_methods = [] allow_executable = False diff --git a/lib/ansible/plugins/shell/powershell.py b/lib/ansible/plugins/shell/powershell.py index aa77cb5d365..dfbae1b4284 100644 --- a/lib/ansible/plugins/shell/powershell.py +++ b/lib/ansible/plugins/shell/powershell.py @@ -55,9 +55,11 @@ class ShellModule(object): return '\'%s\'' % path # powershell requires that script files end with .ps1 - def get_remote_filename(self, base_name): - if not base_name.strip().lower().endswith('.ps1'): - return base_name.strip() + '.ps1' + def get_remote_filename(self, pathname): + base_name = os.path.basename(pathname.strip()) + name, ext = os.path.splitext(base_name.strip()) + if ext.lower() not in ['.ps1', '.exe']: + return name + '.ps1' return base_name.strip() @@ -146,6 +148,10 @@ class ShellModule(object): cmd_parts.insert(0, '&') elif shebang and shebang.startswith('#!'): cmd_parts.insert(0, shebang[2:]) + elif not shebang: + # The module is assumed to be a binary + cmd_parts[0] = self._unquote(cmd_parts[0]) + cmd_parts.append(arg_path) script = ''' Try { From 1e038e5043dd7ce63181eb25f6f7dcbc158e4581 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 11 Jan 2016 11:11:00 -0600 Subject: [PATCH 03/15] Update for py26 --- lib/ansible/plugins/action/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 5fa5af9884c..5e7113e8677 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -101,7 +101,7 @@ class ActionBase(with_metaclass(ABCMeta, object)): return False def _is_binary(self, module_path): - textchars = bytearray({7,8,9,10,12,13,27} | set(range(0x20, 0x100)) - {0x7f}) + textchars = bytearray(set([7, 8, 9, 10, 12, 13, 27]) | set(range(0x20, 0x100)) - set([0x7f])) with open(module_path, 'rb') as f: start = f.read(1024) From 35246abb2e14b2aaa323587671b719151bf2eb84 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 11 Jan 2016 11:33:33 -0600 Subject: [PATCH 04/15] Don't register new vars that aren't needed --- lib/ansible/plugins/action/async.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ansible/plugins/action/async.py b/lib/ansible/plugins/action/async.py index a9406421c25..b2d9370e9e5 100644 --- a/lib/ansible/plugins/action/async.py +++ b/lib/ansible/plugins/action/async.py @@ -61,7 +61,7 @@ class ActionModule(ActionBase): self._transfer_data(remote_module_path, module_data) # configure, upload, and chmod the async_wrapper module - (async_module_style, shebang, async_module_data, async_module_path, is_binary) = self._configure_module(module_name='async_wrapper', module_args=dict(), task_vars=task_vars) + (async_module_style, shebang, async_module_data, _, _) = self._configure_module(module_name='async_wrapper', module_args=dict(), task_vars=task_vars) self._transfer_data(async_module_path, async_module_data) argsfile = None From 6ad8ec0919e39903e6674c5f71da9577268ca87d Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Fri, 12 Feb 2016 11:56:46 -0600 Subject: [PATCH 05/15] Add integration tests for binary modules --- test/integration/Makefile | 6 ++ test/integration/library/.gitignore | 1 + test/integration/library/helloworld.go | 74 +++++++++++++++++++ .../roles/test_binary_modules/tasks/main.yml | 54 ++++++++++++++ test/integration/test_binary_modules.yml | 6 ++ 5 files changed, 141 insertions(+) create mode 100644 test/integration/library/.gitignore create mode 100644 test/integration/library/helloworld.go create mode 100644 test/integration/roles/test_binary_modules/tasks/main.yml create mode 100644 test/integration/test_binary_modules.yml diff --git a/test/integration/Makefile b/test/integration/Makefile index a812ace890d..350dd43e984 100644 --- a/test/integration/Makefile +++ b/test/integration/Makefile @@ -284,3 +284,9 @@ test_lookup_paths: setup no_log: setup # This test expects 7 loggable vars and 0 non loggable ones, if either mismatches it fails, run the ansible-playbook command to debug [ "$$(ansible-playbook no_log_local.yml -i $(INVENTORY) -e outputdir=$(TEST_DIR) -vvvvv | awk --source 'BEGIN { logme = 0; nolog = 0; } /LOG_ME/ { logme += 1;} /DO_NOT_LOG/ { nolog += 1;} END { printf "%d/%d", logme, nolog; }')" = "6/0" ] + +test_binary_modules: + cd library && GOOS=linux GOARCH=amd64 go build -o helloworld_linux helloworld.go + cd library && GOOS=windows GOARCH=amd64 go build -o helloworld_win32nt.exe helloworld.go + cd library && GOOS=darwin GOARCH=amd64 go build -o helloworld_darwin helloworld.go + ansible-playbook test_binary_modules.yml -i $(INVENTORY) -v $(TEST_FLAGS) diff --git a/test/integration/library/.gitignore b/test/integration/library/.gitignore new file mode 100644 index 00000000000..d034a06ac71 --- /dev/null +++ b/test/integration/library/.gitignore @@ -0,0 +1 @@ +helloworld_* diff --git a/test/integration/library/helloworld.go b/test/integration/library/helloworld.go new file mode 100644 index 00000000000..d4d0c82d53a --- /dev/null +++ b/test/integration/library/helloworld.go @@ -0,0 +1,74 @@ +package main + +import ( + "encoding/json" + "fmt" + "io/ioutil" + "os" +) + +type ModuleArgs struct { + Name string +} + +type Response struct { + Msg string `json:"msg"` + Changed bool `json:"changed"` + Failed bool `json:"failed"` +} + +func ExitJson(responseBody Response) { + returnResponse(responseBody) +} + +func FailJson(responseBody Response) { + responseBody.Failed = true + returnResponse(responseBody) +} + +func returnResponse(responseBody Response) { + var response []byte + var err error + response, err = json.Marshal(responseBody) + if err != nil { + response, _ = json.Marshal(Response{Msg: "Invalid response object"}) + } + fmt.Println(string(response)) + if responseBody.Failed { + os.Exit(1) + } else { + os.Exit(0) + } +} + +func main() { + var response Response + + if len(os.Args) != 2 { + response.Msg = "No argument file provided" + FailJson(response) + } + + argsFile := os.Args[1] + + text, err := ioutil.ReadFile(argsFile) + if err != nil { + response.Msg = "Could not read configuration file: " + argsFile + FailJson(response) + } + + var moduleArgs ModuleArgs + err = json.Unmarshal(text, &moduleArgs) + if err != nil { + response.Msg = "Configuration file not valid JSON: " + argsFile + FailJson(response) + } + + var name string = "World" + if moduleArgs.Name != "" { + name = moduleArgs.Name + } + + response.Msg = "Hello, " + name + "!" + ExitJson(response) +} diff --git a/test/integration/roles/test_binary_modules/tasks/main.yml b/test/integration/roles/test_binary_modules/tasks/main.yml new file mode 100644 index 00000000000..e40ea9ded5c --- /dev/null +++ b/test/integration/roles/test_binary_modules/tasks/main.yml @@ -0,0 +1,54 @@ +- debug: var=ansible_system + +- name: ping + ping: + when: ansible_system != 'Win32NT' + +- name: win_ping + win_ping: + when: ansible_system == 'Win32NT' + +- name: Hello, World! + action: "helloworld_{{ ansible_system|lower }}" + register: hello_world + +- assert: + that: + - 'hello_world.msg == "Hello, World!"' + +- name: Hello, Ansible! + action: "helloworld_{{ ansible_system|lower }}" + args: + name: Ansible + register: hello_ansible + +- assert: + that: + - 'hello_ansible.msg == "Hello, Ansible!"' + +- name: Async Hello, World! + action: "helloworld_{{ ansible_system|lower }}" + async: 1 + poll: 1 + when: ansible_system != 'Win32NT' + register: async_hello_world + +- assert: + that: + - 'async_hello_world.msg == "Hello, World!"' + when: not async_hello_world|skipped + +- name: Async Hello, Ansible! + action: "helloworld_{{ ansible_system|lower }}" + args: + name: Ansible + async: 1 + poll: 1 + when: ansible_system != 'Win32NT' + register: async_hello_ansible + +- assert: + that: + - 'async_hello_ansible.msg == "Hello, Ansible!"' + when: not async_hello_ansible|skipped + diff --git a/test/integration/test_binary_modules.yml b/test/integration/test_binary_modules.yml new file mode 100644 index 00000000000..6b1a94e6ed7 --- /dev/null +++ b/test/integration/test_binary_modules.yml @@ -0,0 +1,6 @@ +- hosts: all + roles: + - role: test_binary_modules + tags: + - test_binary_modules + From 2d18607f1e8bf9d7b91a61df5103f560d26e0e06 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Fri, 12 Feb 2016 12:06:01 -0600 Subject: [PATCH 06/15] Add GPL3 header to helloworld.go --- test/integration/library/helloworld.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/integration/library/helloworld.go b/test/integration/library/helloworld.go index d4d0c82d53a..a4c16b20e5d 100644 --- a/test/integration/library/helloworld.go +++ b/test/integration/library/helloworld.go @@ -1,3 +1,18 @@ +// This file is part of Ansible +// +// Ansible is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Ansible is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Ansible. If not, see . + package main import ( From ddf3c3838feb09b35711e8975e822814fc8b7a00 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Fri, 12 Feb 2016 16:10:13 -0600 Subject: [PATCH 07/15] Re-implement/move some code lost due to merge conflicts --- lib/ansible/plugins/shell/__init__.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/ansible/plugins/shell/__init__.py b/lib/ansible/plugins/shell/__init__.py index effbddd58ee..0e951b75915 100644 --- a/lib/ansible/plugins/shell/__init__.py +++ b/lib/ansible/plugins/shell/__init__.py @@ -50,7 +50,8 @@ class ShellBase(object): return os.path.join(*args) # some shells (eg, powershell) are snooty about filenames/extensions, this lets the shell plugin have a say - def get_remote_filename(self, base_name): + def get_remote_filename(self, pathname): + base_name = os.path.basename(pathname.strip()) return base_name.strip() def path_has_trailing_slash(self, path): @@ -164,7 +165,12 @@ class ShellBase(object): # don't quote the cmd if it's an empty string, because this will break pipelining mode if cmd.strip() != '': cmd = pipes.quote(cmd) - cmd_parts = [env_string.strip(), shebang.replace("#!", "").strip(), cmd] + cmd_parts = [] + if shebang: + shebang = shebang.replace("#!", "").strip() + else: + shebang = "" + cmd_parts.extend([env_string.strip(), shebang, cmd]) if arg_path is not None: cmd_parts.append(arg_path) new_cmd = " ".join(cmd_parts) From c22c1b47855b6a3b27f6335a6fd78f3f1e671e48 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 23 Feb 2016 12:14:52 -0600 Subject: [PATCH 08/15] Add note about reading input for binary modules --- docsite/rst/developing_modules.rst | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docsite/rst/developing_modules.rst b/docsite/rst/developing_modules.rst index d1a2cdf44ca..c50536d8e58 100644 --- a/docsite/rst/developing_modules.rst +++ b/docsite/rst/developing_modules.rst @@ -204,6 +204,24 @@ This should return something like:: {"changed": true, "time": "2012-03-14 12:23:00.000307"} +.. _binary_module_reading_input: + +Binary Modules Input +~~~~~~~~~~~~~~~~~~~~ + +Support for binary modules was added in Ansible 2.1. When Ansible detects a binary module, it will proceed to +supply the argument input as a file on ``argv[1]`` that is formatted as JSON. The JSON contents of that file +would resemble something similar to:: + + { + "name": "Ansible", + "_ansible_verbosity": 4, + "_ansible_diff": false, + "_ansible_debug": false, + "_ansible_check_mode": false, + "_ansible_no_log": false + } + .. _module_provided_facts: Module Provided 'Facts' From a4d2238e502a5d0d8b7d31f80198c21859a44212 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 12 Apr 2016 15:02:29 -0500 Subject: [PATCH 09/15] Bumping binary modules functionality to 2.2 --- docsite/rst/developing_modules.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docsite/rst/developing_modules.rst b/docsite/rst/developing_modules.rst index c50536d8e58..f1f5d4a2a23 100644 --- a/docsite/rst/developing_modules.rst +++ b/docsite/rst/developing_modules.rst @@ -209,7 +209,7 @@ This should return something like:: Binary Modules Input ~~~~~~~~~~~~~~~~~~~~ -Support for binary modules was added in Ansible 2.1. When Ansible detects a binary module, it will proceed to +Support for binary modules was added in Ansible 2.2. When Ansible detects a binary module, it will proceed to supply the argument input as a file on ``argv[1]`` that is formatted as JSON. The JSON contents of that file would resemble something similar to:: From 3466e73c504b81d472809001a4d5c5df5f7f6172 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 12 Apr 2016 15:31:43 -0500 Subject: [PATCH 10/15] Resolve test failures --- test/units/plugins/action/test_action.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py index 2a9bb55693d..62e4744cf81 100644 --- a/test/units/plugins/action/test_action.py +++ b/test/units/plugins/action/test_action.py @@ -217,7 +217,7 @@ class TestActionBase(unittest.TestCase): with patch.object(os, 'rename') as m: mock_task.args = dict(a=1, foo='fö〩') mock_connection.module_implementation_preferences = ('',) - (style, shebang, data) = action_base._configure_module(mock_task.action, mock_task.args) + (style, shebang, data, module_path, is_binary) = action_base._configure_module(mock_task.action, mock_task.args) self.assertEqual(style, "new") self.assertEqual(shebang, b"#!/usr/bin/python") @@ -229,7 +229,7 @@ class TestActionBase(unittest.TestCase): mock_task.action = 'win_copy' mock_task.args = dict(b=2) mock_connection.module_implementation_preferences = ('.ps1',) - (style, shebang, data) = action_base._configure_module('stat', mock_task.args) + (style, shebang, data, module_path, is_binary) = action_base._configure_module('stat', mock_task.args) self.assertEqual(style, "new") self.assertEqual(shebang, None) @@ -572,7 +572,7 @@ class TestActionBase(unittest.TestCase): action_base._low_level_execute_command = MagicMock() action_base._fixup_perms = MagicMock() - action_base._configure_module.return_value = ('new', '#!/usr/bin/python', 'this is the module data') + action_base._configure_module.return_value = ('new', '#!/usr/bin/python', 'this is the module data', None, False) action_base._late_needs_tmp_path.return_value = False action_base._compute_environment_string.return_value = '' action_base._connection.has_pipelining = True @@ -581,12 +581,12 @@ class TestActionBase(unittest.TestCase): self.assertEqual(action_base._execute_module(module_name='foo', module_args=dict(z=9, y=8, x=7), task_vars=dict(a=1)), dict(rc=0, stdout="ok", stdout_lines=['ok'])) # test with needing/removing a remote tmp path - action_base._configure_module.return_value = ('old', '#!/usr/bin/python', 'this is the module data') + action_base._configure_module.return_value = ('old', '#!/usr/bin/python', 'this is the module data', None, False) action_base._late_needs_tmp_path.return_value = True action_base._make_tmp_path.return_value = '/the/tmp/path' self.assertEqual(action_base._execute_module(), dict(rc=0, stdout="ok", stdout_lines=['ok'])) - action_base._configure_module.return_value = ('non_native_want_json', '#!/usr/bin/python', 'this is the module data') + action_base._configure_module.return_value = ('non_native_want_json', '#!/usr/bin/python', 'this is the module data', None, False) self.assertEqual(action_base._execute_module(), dict(rc=0, stdout="ok", stdout_lines=['ok'])) play_context.become = True @@ -594,14 +594,14 @@ class TestActionBase(unittest.TestCase): self.assertEqual(action_base._execute_module(), dict(rc=0, stdout="ok", stdout_lines=['ok'])) # test an invalid shebang return - action_base._configure_module.return_value = ('new', '', 'this is the module data') + action_base._configure_module.return_value = ('new', '', 'this is the module data', None, False) action_base._late_needs_tmp_path.return_value = False self.assertRaises(AnsibleError, action_base._execute_module) # test with check mode enabled, once with support for check # mode and once with support disabled to raise an error play_context.check_mode = True - action_base._configure_module.return_value = ('new', '#!/usr/bin/python', 'this is the module data') + action_base._configure_module.return_value = ('new', '#!/usr/bin/python', 'this is the module data', None, False) self.assertEqual(action_base._execute_module(), dict(rc=0, stdout="ok", stdout_lines=['ok'])) action_base._supports_check_mode = False self.assertRaises(AnsibleError, action_base._execute_module) From 0faddfa1680ea2c3a1d2f6e02bd3139972a9b439 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Wed, 11 May 2016 15:14:01 -0500 Subject: [PATCH 11/15] Move binary module detection into executor/module_common.py --- lib/ansible/executor/module_common.py | 19 ++++++++++++---- lib/ansible/plugins/action/__init__.py | 29 ++++++++---------------- lib/ansible/plugins/action/async.py | 8 +++---- lib/ansible/plugins/shell/__init__.py | 1 + lib/ansible/plugins/shell/powershell.py | 2 +- test/units/plugins/action/test_action.py | 14 ++++++------ 6 files changed, 37 insertions(+), 36 deletions(-) diff --git a/lib/ansible/executor/module_common.py b/lib/ansible/executor/module_common.py index 631d67a7504..ccfaab22edf 100644 --- a/lib/ansible/executor/module_common.py +++ b/lib/ansible/executor/module_common.py @@ -490,6 +490,13 @@ def recursive_finder(name, data, py_module_names, py_module_cache, zf): # Save memory; the file won't have to be read again for this ansible module. del py_module_cache[py_module_file] +def _is_binary(module_path): + textchars = bytearray(set([7, 8, 9, 10, 12, 13, 27]) | set(range(0x20, 0x100)) - set([0x7f])) + + with open(module_path, 'rb') as f: + start = f.read(1024) + return bool(start.translate(None, textchars)) + def _find_snippet_imports(module_name, module_data, module_path, module_args, task_vars, module_compression): """ Given the source of the module, convert it to a Jinja2 template to insert @@ -521,11 +528,13 @@ def _find_snippet_imports(module_name, module_data, module_path, module_args, ta module_substyle = 'jsonargs' elif b'WANT_JSON' in module_data: module_substyle = module_style = 'non_native_want_json' + elif _is_binary(module_path): + module_substyle = module_style = 'binary' shebang = None - # Neither old-style nor non_native_want_json modules should be modified + # Neither old-style, non_native_want_json nor binary modules should be modified # except for the shebang line (Done by modify_module) - if module_style in ('old', 'non_native_want_json'): + if module_style in ('old', 'non_native_want_json', 'binary'): return module_data, module_style, shebang output = BytesIO() @@ -731,7 +740,9 @@ def modify_module(module_name, module_path, module_args, task_vars=dict(), modul (module_data, module_style, shebang) = _find_snippet_imports(module_name, module_data, module_path, module_args, task_vars, module_compression) - if shebang is None: + if module_style == 'binary': + return (module_path, module_data, module_style, shebang) + elif shebang is None: lines = module_data.split(b"\n", 1) if lines[0].startswith(b"#!"): shebang = lines[0].strip() @@ -753,4 +764,4 @@ def modify_module(module_name, module_path, module_args, task_vars=dict(), modul else: shebang = to_bytes(shebang, errors='strict') - return (module_data, module_style, shebang) + return (module_path, module_data, module_style, shebang) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 5e7113e8677..6182368a402 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -100,13 +100,6 @@ class ActionBase(with_metaclass(ABCMeta, object)): return True return False - def _is_binary(self, module_path): - textchars = bytearray(set([7, 8, 9, 10, 12, 13, 27]) | set(range(0x20, 0x100)) - set([0x7f])) - - with open(module_path, 'rb') as f: - start = f.read(1024) - return bool(start.translate(None, textchars)) - def _configure_module(self, module_name, module_args, task_vars=None): ''' Handles the loading and templating of the module code through the @@ -152,12 +145,9 @@ class ActionBase(with_metaclass(ABCMeta, object)): "run 'git submodule update --init --recursive' to correct this problem." % (module_name)) # insert shared code and arguments into the module - (module_data, module_style, module_shebang) = modify_module(module_name, module_path, module_args, task_vars=task_vars, module_compression=self._play_context.module_compression) + (module_path, module_data, module_style, module_shebang) = modify_module(module_name, module_path, module_args, task_vars=task_vars, module_compression=self._play_context.module_compression) - if self._is_binary(module_path): - return ('non_native_want_json', None, None, module_path, True) - - return (module_style, module_shebang, module_data, module_path, False) + return (module_style, module_shebang, module_data, module_path) def _compute_environment_string(self): ''' @@ -301,7 +291,7 @@ class ActionBase(with_metaclass(ABCMeta, object)): return remote_path - def _fixup_perms(self, remote_path, remote_user, execute=False, recursive=True): + def _fixup_perms(self, remote_path, remote_user, execute=True, recursive=True): """ We need the files we upload to be readable (and sometimes executable) by the user being sudo'd to but we want to limit other people's access @@ -579,9 +569,8 @@ class ActionBase(with_metaclass(ABCMeta, object)): # let module know our verbosity module_args['_ansible_verbosity'] = display.verbosity - (module_style, shebang, module_data, module_path, is_binary) = self._configure_module(module_name=module_name, module_args=module_args, task_vars=task_vars) - - if not shebang and not is_binary: + (module_style, shebang, module_data, module_path) = self._configure_module(module_name=module_name, module_args=module_args, task_vars=task_vars) + if not shebang and module_style != 'binary': raise AnsibleError("module (%s) is missing interpreter line" % module_name) # a remote tmp path may be necessary and not already created @@ -593,13 +582,13 @@ class ActionBase(with_metaclass(ABCMeta, object)): if tmp: remote_module_filename = self._connection._shell.get_remote_filename(module_path) remote_module_path = self._connection._shell.join_path(tmp, remote_module_filename) - if module_style in ['old', 'non_native_want_json']: + if module_style in ('old', 'non_native_want_json', 'binary'): # we'll also need a temp file to hold our module arguments args_file_path = self._connection._shell.join_path(tmp, 'args') if remote_module_path or module_style != 'new': display.debug("transferring module to remote") - if is_binary: + if module_style == 'binary': self._transfer_file(module_path, remote_module_path) else: self._transfer_data(remote_module_path, module_data) @@ -610,7 +599,7 @@ class ActionBase(with_metaclass(ABCMeta, object)): for k,v in iteritems(module_args): args_data += '%s="%s" ' % (k, pipes.quote(text_type(v))) self._transfer_data(args_file_path, args_data) - elif module_style == 'non_native_want_json': + elif module_style in ('non_native_want_json', 'binary'): self._transfer_data(args_file_path, json.dumps(module_args)) display.debug("done transferring module to remote") @@ -618,7 +607,7 @@ class ActionBase(with_metaclass(ABCMeta, object)): # Fix permissions of the tmp path and tmp files. This should be # called after all files have been transferred. - self._fixup_perms(tmp, remote_user, recursive=True, execute=is_binary) + self._fixup_perms(tmp, remote_user, recursive=True) cmd = "" in_data = None diff --git a/lib/ansible/plugins/action/async.py b/lib/ansible/plugins/action/async.py index b2d9370e9e5..9b59f64bbaf 100644 --- a/lib/ansible/plugins/action/async.py +++ b/lib/ansible/plugins/action/async.py @@ -54,18 +54,18 @@ class ActionModule(ActionBase): module_args['_ansible_no_log'] = True # configure, upload, and chmod the target module - (module_style, shebang, module_data, module_path, is_binary) = self._configure_module(module_name=module_name, module_args=module_args, task_vars=task_vars) - if is_binary: + (module_style, shebang, module_data, module_path) = self._configure_module(module_name=module_name, module_args=module_args, task_vars=task_vars) + if module_style == 'binary': self._transfer_file(module_path, remote_module_path) else: self._transfer_data(remote_module_path, module_data) # configure, upload, and chmod the async_wrapper module - (async_module_style, shebang, async_module_data, _, _) = self._configure_module(module_name='async_wrapper', module_args=dict(), task_vars=task_vars) + (async_module_style, shebang, async_module_data, _) = self._configure_module(module_name='async_wrapper', module_args=dict(), task_vars=task_vars) self._transfer_data(async_module_path, async_module_data) argsfile = None - if module_style == 'non_native_want_json': + if module_style in ('non_native_want_json', 'binary'): argsfile = self._transfer_data(self._connection._shell.join_path(tmp, 'arguments'), json.dumps(module_args)) elif module_style == 'old': args_data = "" diff --git a/lib/ansible/plugins/shell/__init__.py b/lib/ansible/plugins/shell/__init__.py index 0e951b75915..2a3145c9f83 100644 --- a/lib/ansible/plugins/shell/__init__.py +++ b/lib/ansible/plugins/shell/__init__.py @@ -165,6 +165,7 @@ class ShellBase(object): # don't quote the cmd if it's an empty string, because this will break pipelining mode if cmd.strip() != '': cmd = pipes.quote(cmd) + cmd_parts = [] if shebang: shebang = shebang.replace("#!", "").strip() diff --git a/lib/ansible/plugins/shell/powershell.py b/lib/ansible/plugins/shell/powershell.py index dfbae1b4284..505b2e01da7 100644 --- a/lib/ansible/plugins/shell/powershell.py +++ b/lib/ansible/plugins/shell/powershell.py @@ -54,8 +54,8 @@ class ShellModule(object): return path return '\'%s\'' % path - # powershell requires that script files end with .ps1 def get_remote_filename(self, pathname): + # powershell requires that script files end with .ps1 base_name = os.path.basename(pathname.strip()) name, ext = os.path.splitext(base_name.strip()) if ext.lower() not in ['.ps1', '.exe']: diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py index 62e4744cf81..209d2a8d5b4 100644 --- a/test/units/plugins/action/test_action.py +++ b/test/units/plugins/action/test_action.py @@ -217,7 +217,7 @@ class TestActionBase(unittest.TestCase): with patch.object(os, 'rename') as m: mock_task.args = dict(a=1, foo='fö〩') mock_connection.module_implementation_preferences = ('',) - (style, shebang, data, module_path, is_binary) = action_base._configure_module(mock_task.action, mock_task.args) + (style, shebang, data, path) = action_base._configure_module(mock_task.action, mock_task.args) self.assertEqual(style, "new") self.assertEqual(shebang, b"#!/usr/bin/python") @@ -229,7 +229,7 @@ class TestActionBase(unittest.TestCase): mock_task.action = 'win_copy' mock_task.args = dict(b=2) mock_connection.module_implementation_preferences = ('.ps1',) - (style, shebang, data, module_path, is_binary) = action_base._configure_module('stat', mock_task.args) + (style, shebang, data, path) = action_base._configure_module('stat', mock_task.args) self.assertEqual(style, "new") self.assertEqual(shebang, None) @@ -572,7 +572,7 @@ class TestActionBase(unittest.TestCase): action_base._low_level_execute_command = MagicMock() action_base._fixup_perms = MagicMock() - action_base._configure_module.return_value = ('new', '#!/usr/bin/python', 'this is the module data', None, False) + action_base._configure_module.return_value = ('new', '#!/usr/bin/python', 'this is the module data', 'path') action_base._late_needs_tmp_path.return_value = False action_base._compute_environment_string.return_value = '' action_base._connection.has_pipelining = True @@ -581,12 +581,12 @@ class TestActionBase(unittest.TestCase): self.assertEqual(action_base._execute_module(module_name='foo', module_args=dict(z=9, y=8, x=7), task_vars=dict(a=1)), dict(rc=0, stdout="ok", stdout_lines=['ok'])) # test with needing/removing a remote tmp path - action_base._configure_module.return_value = ('old', '#!/usr/bin/python', 'this is the module data', None, False) + action_base._configure_module.return_value = ('old', '#!/usr/bin/python', 'this is the module data', 'path') action_base._late_needs_tmp_path.return_value = True action_base._make_tmp_path.return_value = '/the/tmp/path' self.assertEqual(action_base._execute_module(), dict(rc=0, stdout="ok", stdout_lines=['ok'])) - action_base._configure_module.return_value = ('non_native_want_json', '#!/usr/bin/python', 'this is the module data', None, False) + action_base._configure_module.return_value = ('non_native_want_json', '#!/usr/bin/python', 'this is the module data', 'path') self.assertEqual(action_base._execute_module(), dict(rc=0, stdout="ok", stdout_lines=['ok'])) play_context.become = True @@ -594,14 +594,14 @@ class TestActionBase(unittest.TestCase): self.assertEqual(action_base._execute_module(), dict(rc=0, stdout="ok", stdout_lines=['ok'])) # test an invalid shebang return - action_base._configure_module.return_value = ('new', '', 'this is the module data', None, False) + action_base._configure_module.return_value = ('new', '', 'this is the module data', 'path') action_base._late_needs_tmp_path.return_value = False self.assertRaises(AnsibleError, action_base._execute_module) # test with check mode enabled, once with support for check # mode and once with support disabled to raise an error play_context.check_mode = True - action_base._configure_module.return_value = ('new', '#!/usr/bin/python', 'this is the module data', None, False) + action_base._configure_module.return_value = ('new', '#!/usr/bin/python', 'this is the module data', 'path') self.assertEqual(action_base._execute_module(), dict(rc=0, stdout="ok", stdout_lines=['ok'])) action_base._supports_check_mode = False self.assertRaises(AnsibleError, action_base._execute_module) From 2e8146c52f911edcf83b92f7f6edb9f22d73ea13 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Wed, 11 May 2016 15:17:18 -0500 Subject: [PATCH 12/15] Improve documentation about the JSON args file --- docsite/rst/developing_modules.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docsite/rst/developing_modules.rst b/docsite/rst/developing_modules.rst index f1f5d4a2a23..1ec1b4ecdd0 100644 --- a/docsite/rst/developing_modules.rst +++ b/docsite/rst/developing_modules.rst @@ -211,10 +211,11 @@ Binary Modules Input Support for binary modules was added in Ansible 2.2. When Ansible detects a binary module, it will proceed to supply the argument input as a file on ``argv[1]`` that is formatted as JSON. The JSON contents of that file -would resemble something similar to:: +would resemble something similar to the following payload for a module accepting the same arguments as the +``ping`` module:: { - "name": "Ansible", + "data": "pong", "_ansible_verbosity": 4, "_ansible_diff": false, "_ansible_debug": false, From 651b83d8be2865e1c4f9a2c091ee7981dccb7d23 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Wed, 11 May 2016 15:36:29 -0500 Subject: [PATCH 13/15] Run test_binary_modules --- test/integration/Makefile | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/test/integration/Makefile b/test/integration/Makefile index 350dd43e984..d99271b8ede 100644 --- a/test/integration/Makefile +++ b/test/integration/Makefile @@ -23,7 +23,9 @@ VAULT_PASSWORD_FILE = vault-password CONSUL_RUNNING := $(shell python consul_running.py) EUID := $(shell id -u -r) -all: setup test_test_infra parsing test_var_precedence unicode test_templating_settings environment test_connection non_destructive destructive includes blocks pull check_mode test_hash test_handlers test_group_by test_vault test_tags test_lookup_paths no_log test_gathering_facts +UNAME := $(shell uname | tr '[:upper:]' '[:lower:]') + +all: setup test_test_infra parsing test_var_precedence unicode test_templating_settings environment test_connection non_destructive destructive includes blocks pull check_mode test_hash test_handlers test_group_by test_vault test_tags test_lookup_paths no_log test_gathering_facts test_binary_modules test_test_infra: # ensure fail/assert work locally and can stop execution with non-zero exit code @@ -286,7 +288,15 @@ no_log: setup [ "$$(ansible-playbook no_log_local.yml -i $(INVENTORY) -e outputdir=$(TEST_DIR) -vvvvv | awk --source 'BEGIN { logme = 0; nolog = 0; } /LOG_ME/ { logme += 1;} /DO_NOT_LOG/ { nolog += 1;} END { printf "%d/%d", logme, nolog; }')" = "6/0" ] test_binary_modules: - cd library && GOOS=linux GOARCH=amd64 go build -o helloworld_linux helloworld.go - cd library && GOOS=windows GOARCH=amd64 go build -o helloworld_win32nt.exe helloworld.go - cd library && GOOS=darwin GOARCH=amd64 go build -o helloworld_darwin helloworld.go - ansible-playbook test_binary_modules.yml -i $(INVENTORY) -v $(TEST_FLAGS) + mytmpdir=$(MYTMPDIR); \ + ls -al $$mytmpdir; \ + curl https://storage.googleapis.com/golang/go1.6.2.$(UNAME)-amd64.tar.gz | tar -xz -C $$mytmpdir; \ + [ $$? != 0 ] && wget -qO- https://storage.googleapis.com/golang/go1.6.2.$(UNAME)-amd64.tar.gz | tar -xz -C $$mytmpdir; \ + ls -al $$mytmpdir; \ + cd library; \ + GOROOT=$$mytmpdir/go GOOS=linux GOARCH=amd64 $$mytmpdir/go/bin/go build -o helloworld_linux helloworld.go; \ + GOROOT=$$mytmpdir/go GOOS=windows GOARCH=amd64 $$mytmpdir/go/bin/go build -o helloworld_win32nt.exe helloworld.go; \ + GOROOT=$$mytmpdir/go GOOS=darwin GOARCH=amd64 $$mytmpdir/go/bin/go build -o helloworld_darwin helloworld.go; \ + cd ..; \ + rm -rf $$mytmpdir; \ + ANSIBLE_HOST_KEY_CHECKING=false ansible-playbook test_binary_modules.yml -i $(INVENTORY) -v $(TEST_FLAGS) From 34adb54734375fe25d415a252bee0afdcabe1cd7 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Thu, 12 May 2016 12:46:07 -0500 Subject: [PATCH 14/15] Make _is_binary use already read module_data, move _is_binary check to the top of the stack --- lib/ansible/executor/module_common.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/ansible/executor/module_common.py b/lib/ansible/executor/module_common.py index ccfaab22edf..d87860ac62b 100644 --- a/lib/ansible/executor/module_common.py +++ b/lib/ansible/executor/module_common.py @@ -490,11 +490,9 @@ def recursive_finder(name, data, py_module_names, py_module_cache, zf): # Save memory; the file won't have to be read again for this ansible module. del py_module_cache[py_module_file] -def _is_binary(module_path): +def _is_binary(module_data): textchars = bytearray(set([7, 8, 9, 10, 12, 13, 27]) | set(range(0x20, 0x100)) - set([0x7f])) - - with open(module_path, 'rb') as f: - start = f.read(1024) + start = module_data[:1024] return bool(start.translate(None, textchars)) def _find_snippet_imports(module_name, module_data, module_path, module_args, task_vars, module_compression): @@ -511,7 +509,9 @@ def _find_snippet_imports(module_name, module_data, module_path, module_args, ta # module_substyle is extra information that's useful internally. It tells # us what we have to look to substitute in the module files and whether # we're using module replacer or ziploader to format the module itself. - if REPLACER in module_data: + if _is_binary(module_data): + module_substyle = module_style = 'binary' + elif REPLACER in module_data: # Do REPLACER before from ansible.module_utils because we need make sure # we substitute "from ansible.module_utils basic" for REPLACER module_style = 'new' @@ -528,8 +528,6 @@ def _find_snippet_imports(module_name, module_data, module_path, module_args, ta module_substyle = 'jsonargs' elif b'WANT_JSON' in module_data: module_substyle = module_style = 'non_native_want_json' - elif _is_binary(module_path): - module_substyle = module_style = 'binary' shebang = None # Neither old-style, non_native_want_json nor binary modules should be modified From ca22783086d40cdd0f2fe8fcde1361f8a5212cef Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Thu, 12 May 2016 12:53:30 -0500 Subject: [PATCH 15/15] modify_module does not need to return module_path, as the calling code already has access to it --- lib/ansible/executor/module_common.py | 4 ++-- lib/ansible/plugins/action/__init__.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/ansible/executor/module_common.py b/lib/ansible/executor/module_common.py index d87860ac62b..2f98e048bef 100644 --- a/lib/ansible/executor/module_common.py +++ b/lib/ansible/executor/module_common.py @@ -739,7 +739,7 @@ def modify_module(module_name, module_path, module_args, task_vars=dict(), modul (module_data, module_style, shebang) = _find_snippet_imports(module_name, module_data, module_path, module_args, task_vars, module_compression) if module_style == 'binary': - return (module_path, module_data, module_style, shebang) + return (module_data, module_style, shebang) elif shebang is None: lines = module_data.split(b"\n", 1) if lines[0].startswith(b"#!"): @@ -762,4 +762,4 @@ def modify_module(module_name, module_path, module_args, task_vars=dict(), modul else: shebang = to_bytes(shebang, errors='strict') - return (module_path, module_data, module_style, shebang) + return (module_data, module_style, shebang) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 6182368a402..9414a149fc4 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -145,7 +145,7 @@ class ActionBase(with_metaclass(ABCMeta, object)): "run 'git submodule update --init --recursive' to correct this problem." % (module_name)) # insert shared code and arguments into the module - (module_path, module_data, module_style, module_shebang) = modify_module(module_name, module_path, module_args, task_vars=task_vars, module_compression=self._play_context.module_compression) + (module_data, module_style, module_shebang) = modify_module(module_name, module_path, module_args, task_vars=task_vars, module_compression=self._play_context.module_compression) return (module_style, module_shebang, module_data, module_path)