From bda541fa0d2645dfd9efb7209c6b92e497588fe0 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Thu, 4 Apr 2019 09:59:52 -0400 Subject: [PATCH] fix missing attribs with dirct module execution (#53875) * fix missing attribs with dirct module execution * also make remote tmp handling smarter update tests * set default if attrib does not exist * add simple test --- .../fragments/always_module_attribs.yml | 2 ++ lib/ansible/module_utils/basic.py | 34 ++++++++++++------- lib/ansible/module_utils/common/parameters.py | 34 +++++++++++-------- test/integration/targets/run_modules/aliases | 1 + .../integration/targets/run_modules/args.json | 1 + .../targets/run_modules/library/test.py | 7 ++++ test/integration/targets/run_modules/runme.sh | 6 ++++ test/units/modules/conftest.py | 2 +- 8 files changed, 58 insertions(+), 29 deletions(-) create mode 100644 changelogs/fragments/always_module_attribs.yml create mode 100644 test/integration/targets/run_modules/aliases create mode 100644 test/integration/targets/run_modules/args.json create mode 100644 test/integration/targets/run_modules/library/test.py create mode 100755 test/integration/targets/run_modules/runme.sh diff --git a/changelogs/fragments/always_module_attribs.yml b/changelogs/fragments/always_module_attribs.yml new file mode 100644 index 00000000000..36111740e3e --- /dev/null +++ b/changelogs/fragments/always_module_attribs.yml @@ -0,0 +1,2 @@ +bugfixes: + - ensure we always have internal module attributes set, even if not being passed (fixes using modules as script) diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index 29a1f35c5cc..e595d326239 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -27,8 +27,6 @@ FILE_ATTRIBUTES = { 'Z': 'compresseddirty', } -PASS_BOOLS = ('no_log', 'debug', 'diff') - # Ansible modules can be written in any language. # The functions available here can be used to do many common tasks, # to simplify development of Python modules. @@ -157,6 +155,7 @@ from ansible.module_utils.common.parameters import ( list_deprecations, list_no_log_values, PASS_VARS, + PASS_BOOLS, ) from ansible.module_utils.six import ( @@ -711,8 +710,10 @@ class AnsibleModule(object): if self._tmpdir is None: basedir = None - basedir = os.path.expanduser(os.path.expandvars(self._remote_tmp)) - if not os.path.exists(basedir): + if self._remote_tmp is not None: + basedir = os.path.expanduser(os.path.expandvars(self._remote_tmp)) + + if basedir is not None and not os.path.exists(basedir): try: os.makedirs(basedir, mode=0o700) except (OSError, IOError) as e: @@ -1441,20 +1442,27 @@ class AnsibleModule(object): if legal_inputs is None: legal_inputs = self._legal_inputs - for (k, v) in list(param.items()): + for k in list(param.keys()): if check_invalid_arguments and k not in legal_inputs: unsupported_parameters.add(k) - elif k.startswith('_ansible_'): - # handle setting internal properties from internal ansible vars - key = k.replace('_ansible_', '') - if key in PASS_BOOLS: - setattr(self, PASS_VARS[key], self.boolean(v)) + + for k in PASS_VARS: + # handle setting internal properties from internal ansible vars + param_key = '_ansible_%s' % k + if param_key in param: + if k in PASS_BOOLS: + setattr(self, PASS_VARS[k][0], self.boolean(param[param_key])) else: - setattr(self, PASS_VARS[key], v) + setattr(self, PASS_VARS[k][0], param[param_key]) - # clean up internal params: - del self.params[k] + # clean up internal top level params: + if param_key in self.params: + del self.params[param_key] + else: + # use defaults if not already set + if not hasattr(self, PASS_VARS[k][0]): + setattr(self, PASS_VARS[k][0], PASS_VARS[k][1]) if unsupported_parameters: msg = "Unsupported parameters for (%s) module: %s" % (self._name, ', '.join(sorted(list(unsupported_parameters)))) diff --git a/lib/ansible/module_utils/common/parameters.py b/lib/ansible/module_utils/common/parameters.py index d0a7ea0ef69..71285fbd2d8 100644 --- a/lib/ansible/module_utils/common/parameters.py +++ b/lib/ansible/module_utils/common/parameters.py @@ -18,24 +18,28 @@ from ansible.module_utils.six import ( # Python2 & 3 way to get NoneType NoneType = type(None) +# if adding boolean attribute, also add to PASS_BOOL +# some of this dupes defaults from controller config PASS_VARS = { - 'check_mode': 'check_mode', - 'debug': '_debug', - 'diff': '_diff', - 'keep_remote_files': '_keep_remote_files', - 'module_name': '_name', - 'no_log': 'no_log', - 'remote_tmp': '_remote_tmp', - 'selinux_special_fs': '_selinux_special_fs', - 'shell_executable': '_shell', - 'socket': '_socket_path', - 'string_conversion_action': '_string_conversion_action', - 'syslog_facility': '_syslog_facility', - 'tmpdir': '_tmpdir', - 'verbosity': '_verbosity', - 'version': 'ansible_version', + 'check_mode': ('check_mode', False), + 'debug': ('_debug', False), + 'diff': ('_diff', False), + 'keep_remote_files': ('_keep_remote_files', False), + 'module_name': ('_name', None), + 'no_log': ('no_log', False), + 'remote_tmp': ('_remote_tmp', None), + 'selinux_special_fs': ('_selinux_special_fs', ['fuse', 'nfs', 'vboxsf', 'ramfs', '9p']), + 'shell_executable': ('_shell', '/bin/sh'), + 'socket': ('_socket_path', None), + 'string_conversion_action': ('_string_conversion_action', 'warn'), + 'syslog_facility': ('_syslog_facility', 'INFO'), + 'tmpdir': ('_tmpdir', None), + 'verbosity': ('_verbosity', 0), + 'version': ('ansible_version', '0.0'), } +PASS_BOOLS = ('check_mode', 'debug', 'diff', 'keep_remote_files', 'no_log') + def _return_datastructure_name(obj): """ Return native stringified values from datastructures. diff --git a/test/integration/targets/run_modules/aliases b/test/integration/targets/run_modules/aliases new file mode 100644 index 00000000000..b59832142f2 --- /dev/null +++ b/test/integration/targets/run_modules/aliases @@ -0,0 +1 @@ +shippable/posix/group3 diff --git a/test/integration/targets/run_modules/args.json b/test/integration/targets/run_modules/args.json new file mode 100644 index 00000000000..c3abc21a8fa --- /dev/null +++ b/test/integration/targets/run_modules/args.json @@ -0,0 +1 @@ +{ "ANSIBLE_MODULE_ARGS": {} } diff --git a/test/integration/targets/run_modules/library/test.py b/test/integration/targets/run_modules/library/test.py new file mode 100644 index 00000000000..bbe3182c585 --- /dev/null +++ b/test/integration/targets/run_modules/library/test.py @@ -0,0 +1,7 @@ +#!/usr/bin/python + +from ansible.module_utils.basic import AnsibleModule + +module = AnsibleModule(argument_spec=dict()) + +module.exit_json(**{'tempdir': module._remote_tmp}) diff --git a/test/integration/targets/run_modules/runme.sh b/test/integration/targets/run_modules/runme.sh new file mode 100755 index 00000000000..34c245cbf61 --- /dev/null +++ b/test/integration/targets/run_modules/runme.sh @@ -0,0 +1,6 @@ +#!/usr/bin/env bash + +set -eux + +# test running module directly +python.py library/test.py args.json diff --git a/test/units/modules/conftest.py b/test/units/modules/conftest.py index 51251fbe307..3bbfe0b7a85 100644 --- a/test/units/modules/conftest.py +++ b/test/units/modules/conftest.py @@ -20,7 +20,7 @@ def patch_ansible_module(request, mocker): if '_ansible_remote_tmp' not in request.param['ANSIBLE_MODULE_ARGS']: request.param['ANSIBLE_MODULE_ARGS']['_ansible_remote_tmp'] = '/tmp' if '_ansible_keep_remote_files' not in request.param['ANSIBLE_MODULE_ARGS']: - request.param['ANSIBLE_MODULE_ARGS']['_ansible_keep_remote_files'] = '/tmp' + request.param['ANSIBLE_MODULE_ARGS']['_ansible_keep_remote_files'] = False args = json.dumps(request.param) else: raise Exception('Malformed data to the patch_ansible_module pytest fixture')