From 674a57c3fa9abe1ccc6a22d2d5cff4ba8a8ba0b8 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Tue, 14 May 2019 09:59:18 -0400 Subject: [PATCH] fix module defaults (#56020) * fix module defaults - corrected precedence (specific module > group) - made into reusable function - use from gather_facts/service/package to match 'actual module used' --- .../fragments/module_default_fixes.yaml | 2 ++ lib/ansible/executor/module_common.py | 29 +++++++++++++++++++ lib/ansible/executor/task_executor.py | 17 ++--------- lib/ansible/plugins/action/gather_facts.py | 4 +++ lib/ansible/plugins/action/package.py | 4 +++ lib/ansible/plugins/action/service.py | 4 +++ 6 files changed, 45 insertions(+), 15 deletions(-) create mode 100644 changelogs/fragments/module_default_fixes.yaml diff --git a/changelogs/fragments/module_default_fixes.yaml b/changelogs/fragments/module_default_fixes.yaml new file mode 100644 index 00000000000..c49e1bdaef9 --- /dev/null +++ b/changelogs/fragments/module_default_fixes.yaml @@ -0,0 +1,2 @@ +bugfixes: + - refactored into function and use in some action plugins to match actual module used, made precedence very clear in code. diff --git a/lib/ansible/executor/module_common.py b/lib/ansible/executor/module_common.py index 1253f76f983..69b409615b0 100644 --- a/lib/ansible/executor/module_common.py +++ b/lib/ansible/executor/module_common.py @@ -1106,3 +1106,32 @@ def modify_module(module_name, module_path, module_args, templar, task_vars=None b_module_data = b"\n".join(b_lines) return (b_module_data, module_style, shebang) + + +def get_action_args_with_defaults(action, args, defaults, templar): + + tmp_args = {} + module_defaults = {} + + # Merge latest defaults into dict, since they are a list of dicts + if isinstance(defaults, list): + for default in defaults: + module_defaults.update(default) + + # if I actually have defaults, template and merge + if module_defaults: + module_defaults = templar.template(module_defaults) + + # deal with configured group defaults first + if action in C.config.module_defaults_groups: + for group in C.config.module_defaults_groups.get(action, []): + tmp_args.update((module_defaults.get('group/{0}'.format(group)) or {}).copy()) + + # handle specific action defaults + if action in module_defaults: + tmp_args.update(module_defaults[action].copy()) + + # direct args override all + tmp_args.update(args) + + return tmp_args diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py index 566030a4984..eceac95af95 100644 --- a/lib/ansible/executor/task_executor.py +++ b/lib/ansible/executor/task_executor.py @@ -17,6 +17,7 @@ import traceback from ansible import constants as C from ansible.errors import AnsibleError, AnsibleParserError, AnsibleUndefinedVariable, AnsibleConnectionFailure, AnsibleActionFail, AnsibleActionSkip from ansible.executor.task_result import TaskResult +from ansible.executor.module_common import get_action_args_with_defaults from ansible.module_utils.six import iteritems, string_types, binary_type from ansible.module_utils.six.moves import xrange from ansible.module_utils._text import to_text, to_native @@ -599,21 +600,7 @@ class TaskExecutor: self._handler = self._get_action_handler(connection=self._connection, templar=templar) # Apply default params for action/module, if present - # These are collected as a list of dicts, so we need to merge them - module_defaults = {} - for default in self._task.module_defaults: - module_defaults.update(default) - if module_defaults: - module_defaults = templar.template(module_defaults) - if self._task.action in module_defaults: - tmp_args = module_defaults[self._task.action].copy() - tmp_args.update(self._task.args) - self._task.args = tmp_args - if self._task.action in C.config.module_defaults_groups: - for group in C.config.module_defaults_groups.get(self._task.action, []): - tmp_args = (module_defaults.get('group/{0}'.format(group)) or {}).copy() - tmp_args.update(self._task.args) - self._task.args = tmp_args + self._task.args = get_action_args_with_defaults(self._task.action, self._task.args, self._task.module_defaults, templar) # And filter out any fields which were set to default(omit), and got the omit token value omit_token = variables.get('omit') diff --git a/lib/ansible/plugins/action/gather_facts.py b/lib/ansible/plugins/action/gather_facts.py index 3ab5934e04a..e461abe0069 100644 --- a/lib/ansible/plugins/action/gather_facts.py +++ b/lib/ansible/plugins/action/gather_facts.py @@ -8,6 +8,7 @@ import os import time from ansible import constants as C +from ansible.executor.module_common import get_action_args_with_defaults from ansible.plugins.action import ActionBase from ansible.utils.vars import combine_vars @@ -39,6 +40,9 @@ class ActionModule(ActionBase): # This ensures we don't pass a ``None`` value as an argument expecting a specific type mod_args = dict((k, v) for k, v in mod_args.items() if v is not None) + # handle module defaults + mod_args = get_action_args_with_defaults(fact_module, mod_args, self._task.module_defaults, self._templar) + return mod_args def run(self, tmp=None, task_vars=None): diff --git a/lib/ansible/plugins/action/package.py b/lib/ansible/plugins/action/package.py index 75a8dd56284..932acccb04b 100644 --- a/lib/ansible/plugins/action/package.py +++ b/lib/ansible/plugins/action/package.py @@ -18,6 +18,7 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type from ansible.errors import AnsibleAction, AnsibleActionFail +from ansible.executor.module_common import get_action_args_with_defaults from ansible.plugins.action import ActionBase from ansible.utils.display import Display @@ -64,6 +65,9 @@ class ActionModule(ActionBase): if 'use' in new_module_args: del new_module_args['use'] + # get defaults for specific module + new_module_args = get_action_args_with_defaults(module, new_module_args, self._task.module_defaults, self._templar) + display.vvvv("Running %s" % module) result.update(self._execute_module(module_name=module, module_args=new_module_args, task_vars=task_vars, wrap_async=self._task.async_val)) else: diff --git a/lib/ansible/plugins/action/service.py b/lib/ansible/plugins/action/service.py index 41b33d4e063..3ebd0ae17dc 100644 --- a/lib/ansible/plugins/action/service.py +++ b/lib/ansible/plugins/action/service.py @@ -19,6 +19,7 @@ __metaclass__ = type from ansible.errors import AnsibleAction, AnsibleActionFail +from ansible.executor.module_common import get_action_args_with_defaults from ansible.plugins.action import ActionBase @@ -71,6 +72,9 @@ class ActionModule(ActionBase): del new_module_args[unused] self._display.warning('Ignoring "%s" as it is not used in "%s"' % (unused, module)) + # get defaults for specific module + new_module_args = get_action_args_with_defaults(module, new_module_args, self._task.module_defaults, self._templar) + self._display.vvvv("Running %s" % module) result.update(self._execute_module(module_name=module, module_args=new_module_args, task_vars=task_vars, wrap_async=self._task.async_val)) else: