From ff22528b076fa7ab17e096c73f5d30d370bd0448 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Fri, 14 Jul 2017 16:44:58 -0700 Subject: [PATCH] Consolidate boolean/mk_boolean conversion functions into a single location Consolidate the module_utils, constants, and config functions that convert values into booleans into a single function in module_utils. Port code to use the module_utils.validate.convert_bool.boolean function isntead of mk_boolean. --- contrib/inventory/rax.py | 5 +- lib/ansible/cli/console.py | 7 +- lib/ansible/config/data.py | 2 - lib/ansible/config/manager.py | 13 +- lib/ansible/constants.py | 13 +- lib/ansible/executor/play_iterator.py | 12 +- lib/ansible/module_utils/basic.py | 27 +- lib/ansible/module_utils/docker_common.py | 3 +- lib/ansible/module_utils/netcli.py | 5 +- lib/ansible/module_utils/parsing/__init__.py | 0 .../module_utils/parsing/convert_bool.py | 26 ++ lib/ansible/module_utils/rax.py | 1 - lib/ansible/playbook/base.py | 4 +- lib/ansible/playbook/play_context.py | 10 +- lib/ansible/plugins/action/assemble.py | 4 +- lib/ansible/plugins/action/copy.py | 12 +- lib/ansible/plugins/action/fetch.py | 10 +- lib/ansible/plugins/action/patch.py | 4 +- lib/ansible/plugins/action/set_fact.py | 4 +- lib/ansible/plugins/action/set_stats.py | 4 +- lib/ansible/plugins/action/synchronize.py | 5 +- lib/ansible/plugins/action/template.py | 5 +- lib/ansible/plugins/action/unarchive.py | 17 +- lib/ansible/plugins/callback/slack.py | 7 +- lib/ansible/plugins/connection/ssh.py | 6 +- lib/ansible/plugins/lookup/first_found.py | 4 +- lib/ansible/plugins/lookup/hashi_vault.py | 4 +- lib/ansible/plugins/lookup/subelements.py | 4 +- test/runner/injector/importer.py | 257 +++++++++++++++++- .../module_utils/parsing/test_convert_bool.py | 60 ++++ 30 files changed, 433 insertions(+), 102 deletions(-) create mode 100644 lib/ansible/module_utils/parsing/__init__.py create mode 100644 lib/ansible/module_utils/parsing/convert_bool.py mode change 120000 => 100755 test/runner/injector/importer.py create mode 100644 test/units/module_utils/parsing/test_convert_bool.py diff --git a/contrib/inventory/rax.py b/contrib/inventory/rax.py index eae1b4892f5..9704852ca33 100755 --- a/contrib/inventory/rax.py +++ b/contrib/inventory/rax.py @@ -168,7 +168,8 @@ except ImportError: from time import time -from ansible.constants import get_config, mk_boolean +from ansible.constants import get_config +from ansible.module_utils.parsing.convert_bool import boolean NON_CALLABLES = (basestring, bool, dict, int, list, type(None)) @@ -288,7 +289,7 @@ def _list_into_cache(regions): if not cbs_attachments[region]: cbs = pyrax.connect_to_cloud_blockstorage(region) for vol in cbs.list(): - if mk_boolean(vol.bootable): + if boolean(vol.bootable, strict=False): for attachment in vol.attachments: metadata = vol.volume_image_metadata server_id = attachment['server_id'] diff --git a/lib/ansible/cli/console.py b/lib/ansible/cli/console.py index 28a89a5a7be..a70f7020e8a 100644 --- a/lib/ansible/cli/console.py +++ b/lib/ansible/cli/console.py @@ -41,6 +41,7 @@ from ansible.cli import CLI from ansible.errors import AnsibleError from ansible.executor.task_queue_manager import TaskQueueManager from ansible.module_utils._text import to_native, to_text +from ansible.module_utils.parsing.convert_bool import boolean from ansible.parsing.splitter import parse_kv from ansible.playbook.play import Play from ansible.plugins import module_loader @@ -295,7 +296,7 @@ class ConsoleCLI(CLI, cmd.Cmd): def do_become(self, arg): """Toggle whether plays run with become""" if arg: - self.options.become = C.mk_boolean(arg) + self.options.become = boolean(arg, strict=False) display.v("become changed to %s" % self.options.become) self.set_prompt() else: @@ -329,7 +330,7 @@ class ConsoleCLI(CLI, cmd.Cmd): def do_check(self, arg): """Toggle whether plays run with check mode""" if arg: - self.options.check = C.mk_boolean(arg) + self.options.check = boolean(arg, strict=False) display.v("check mode changed to %s" % self.options.check) else: display.display("Please specify check mode value, e.g. `check yes`") @@ -337,7 +338,7 @@ class ConsoleCLI(CLI, cmd.Cmd): def do_diff(self, arg): """Toggle whether plays run with diff""" if arg: - self.options.diff = C.mk_boolean(arg) + self.options.diff = boolean(arg, strict=False) display.v("diff mode changed to %s" % self.options.diff) else: display.display("Please specify a diff value , e.g. `diff yes`") diff --git a/lib/ansible/config/data.py b/lib/ansible/config/data.py index 9158e2413d3..903b9cf00c6 100644 --- a/lib/ansible/config/data.py +++ b/lib/ansible/config/data.py @@ -25,8 +25,6 @@ Setting = namedtuple('Setting','name value origin') class ConfigData(object): - BOOL_TRUE = frozenset(["true", "t", "y", "1", "yes", "on"]) - def __init__(self): self._global_settings = {} self._plugins = {} diff --git a/lib/ansible/config/manager.py b/lib/ansible/config/manager.py index 88545f338fe..6bef96f7afa 100644 --- a/lib/ansible/config/manager.py +++ b/lib/ansible/config/manager.py @@ -29,6 +29,7 @@ from ansible.errors import AnsibleOptionsError, AnsibleError from ansible.module_utils.six import string_types from ansible.module_utils.six.moves import configparser from ansible.module_utils._text import to_text, to_bytes, to_native +from ansible.module_utils.parsing.convert_bool import boolean from ansible.parsing.quoting import unquote from ansible.utils.path import unfrackpath from ansible.utils.path import makedirs_safe @@ -41,6 +42,7 @@ def resolve_path(path): return unfrackpath(path, follow=False) + def get_ini_config(p, entries): ''' returns the value of last ini entry found ''' value = None @@ -54,7 +56,6 @@ def get_ini_config(p, entries): return value - class ConfigManager(object): def __init__(self, conf_file=None): @@ -179,14 +180,6 @@ class ConfigManager(object): return path - def make_boolean(self, value): - ret = value - if not isinstance(value, bool): - if value is None: - ret = False - ret = (to_text(value).lower() in self.data.BOOL_TRUE) - return ret - def ensure_type(self, value, value_type): ''' return a configuration variable with casting :arg value: The value to ensure correct typing of @@ -205,7 +198,7 @@ class ConfigManager(object): each part for environment variables and tildes. ''' if value_type == 'boolean': - value = self.make_boolean(value) + value = boolean(value, strict=False) elif value: if value_type == 'integer': diff --git a/lib/ansible/constants.py b/lib/ansible/constants.py index 62e2fbfd44f..a6a4ebd3cdb 100644 --- a/lib/ansible/constants.py +++ b/lib/ansible/constants.py @@ -22,6 +22,7 @@ __metaclass__ = type from string import ascii_letters, digits from ansible.module_utils._text import to_text +from ansible.module_utils.parsing.convert_bool import boolean, BOOLEANS_TRUE from ansible.config.manager import ConfigManager _config = ConfigManager() @@ -32,8 +33,14 @@ for setting in _config.data.get_settings(): def mk_boolean(value): - ''' moved ''' - return _config.make_boolean(value) + ''' moved to module_utils''' + try: + from __main__ import display + except: + pass + else: + display.deprecated('ansible.constants.mk_boolean() is deprecated. Use ansible.module_utils.parsing.convert_bool.boolean() instead', version='2.8') + return boolean(value, strict=False) # ### CONSTANTS ### yes, actual ones @@ -60,7 +67,7 @@ BECOME_MISSING_STRINGS = { 'ksu': 'No password given', 'pmrun': '' } # FIXME: deal with i18n -BOOL_TRUE = _config.data.BOOL_TRUE +BOOL_TRUE = BOOLEANS_TRUE DEFAULT_BECOME_PASS = None DEFAULT_PASSWORD_CHARS = to_text(ascii_letters + digits + ".,:-_", errors='strict') # characters included in auto-generated passwords DEFAULT_SUDO_PASS = None diff --git a/lib/ansible/executor/play_iterator.py b/lib/ansible/executor/play_iterator.py index a25e61368f1..47c18c5d168 100644 --- a/lib/ansible/executor/play_iterator.py +++ b/lib/ansible/executor/play_iterator.py @@ -23,14 +23,11 @@ import fnmatch from ansible import constants as C from ansible.module_utils.six import iteritems +from ansible.module_utils.parsing.convert_bool import boolean from ansible.playbook.block import Block from ansible.playbook.task import Task -boolean = C.mk_boolean - -__all__ = ['PlayIterator'] - try: from __main__ import display except ImportError: @@ -38,6 +35,9 @@ except ImportError: display = Display() +__all__ = ['PlayIterator'] + + class HostState: def __init__(self, blocks): self._blocks = blocks[:] @@ -300,10 +300,10 @@ class PlayIterator: # NOT explicitly set gather_facts to False. gathering = C.DEFAULT_GATHERING - implied = self._play.gather_facts is None or boolean(self._play.gather_facts) + implied = self._play.gather_facts is None or boolean(self._play.gather_facts, strict=False) if (gathering == 'implicit' and implied) or \ - (gathering == 'explicit' and boolean(self._play.gather_facts)) or \ + (gathering == 'explicit' and boolean(self._play.gather_facts, strict=False)) or \ (gathering == 'smart' and implied and not (self._variable_manager._fact_cache.get(host.name, {}).get('module_setup', False))): # The setup block is always self._blocks[0], as we inject it # during the play compilation in __init__ above. diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index 821f1f5d4fa..b2396d6661d 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -28,10 +28,6 @@ # USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. # -BOOLEANS_TRUE = ['y', 'yes', 'on', '1', 'true', 1, True] -BOOLEANS_FALSE = ['n', 'no', 'off', '0', 'false', 0, False] -BOOLEANS = BOOLEANS_TRUE + BOOLEANS_FALSE - SIZE_RANGES = { 'Y': 1 << 80, 'Z': 1 << 70, @@ -178,6 +174,8 @@ from ansible.module_utils.six import ( ) from ansible.module_utils.six.moves import map, reduce, shlex_quote from ansible.module_utils._text import to_native, to_bytes, to_text +from ansible.module_utils.parsing.convert_bool import BOOLEANS, BOOLEANS_FALSE, BOOLEANS_TRUE, boolean + PASSWORD_MATCH = re.compile(r'^(?:.+[-_\s])?pass(?:[-_\s]?(?:word|phrase|wrd|wd)?)(?:[-_\s].+)?$', re.I) @@ -1658,8 +1656,7 @@ class AnsibleModule(object): lowered_choices = None if param[k] == 'False': lowered_choices = _lenient_lowercase(choices) - FALSEY = frozenset(BOOLEANS_FALSE) - overlap = FALSEY.intersection(choices) + overlap = BOOLEANS_FALSE.intersection(choices) if len(overlap) == 1: # Extract from a set (param[k],) = overlap @@ -1667,8 +1664,7 @@ class AnsibleModule(object): if param[k] == 'True': if lowered_choices is None: lowered_choices = _lenient_lowercase(choices) - TRUTHY = frozenset(BOOLEANS_TRUE) - overlap = TRUTHY.intersection(choices) + overlap = BOOLEANS_TRUE.intersection(choices) if len(overlap) == 1: (param[k],) = overlap @@ -2045,16 +2041,13 @@ class AnsibleModule(object): def boolean(self, arg): ''' return a bool for the arg ''' - if arg is None or isinstance(arg, bool): + if arg is None: return arg - if isinstance(arg, string_types): - arg = arg.lower() - if arg in BOOLEANS_TRUE: - return True - elif arg in BOOLEANS_FALSE: - return False - else: - self.fail_json(msg='%s is not a valid boolean. Valid booleans include: %s' % (to_text(arg), ','.join(['%s' % x for x in BOOLEANS]))) + + try: + return boolean(arg) + except TypeError as e: + self.fail_json(msg=to_native(e)) def jsonify(self, data): for encoding in ("utf-8", "latin-1"): diff --git a/lib/ansible/module_utils/docker_common.py b/lib/ansible/module_utils/docker_common.py index fad8e5172cb..2608fcd6e33 100644 --- a/lib/ansible/module_utils/docker_common.py +++ b/lib/ansible/module_utils/docker_common.py @@ -23,8 +23,9 @@ import sys import copy from distutils.version import LooseVersion -from ansible.module_utils.basic import AnsibleModule, BOOLEANS_TRUE, BOOLEANS_FALSE +from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.six.moves.urllib.parse import urlparse +from ansible.module_utils.parsing.convert_bool import BOOLEANS_TRUE, BOOLEANS_FALSE HAS_DOCKER_PY = True HAS_DOCKER_PY_2 = False diff --git a/lib/ansible/module_utils/netcli.py b/lib/ansible/module_utils/netcli.py index daa5518fe6e..18d25a66931 100644 --- a/lib/ansible/module_utils/netcli.py +++ b/lib/ansible/module_utils/netcli.py @@ -29,7 +29,7 @@ import re import shlex import time -from ansible.module_utils.basic import BOOLEANS_TRUE, BOOLEANS_FALSE, get_exception +from ansible.module_utils.parsing.convert_bool import BOOLEANS_TRUE, BOOLEANS_FALSE from ansible.module_utils.six import string_types, text_type from ansible.module_utils.six.moves import zip @@ -163,8 +163,7 @@ class CommandRunner(object): def add_conditional(self, condition): try: self.conditionals.add(Conditional(condition)) - except AttributeError: - exc = get_exception() + except AttributeError as exc: raise AddConditionError(msg=str(exc), condition=condition) def run(self): diff --git a/lib/ansible/module_utils/parsing/__init__.py b/lib/ansible/module_utils/parsing/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/lib/ansible/module_utils/parsing/convert_bool.py b/lib/ansible/module_utils/parsing/convert_bool.py new file mode 100644 index 00000000000..415bfe1cae2 --- /dev/null +++ b/lib/ansible/module_utils/parsing/convert_bool.py @@ -0,0 +1,26 @@ +# Copyright: 2017, Ansible Project +# License: GNU General Public License v3 or later (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt ) + +from ansible.module_utils.six import binary_type, text_type +from ansible.module_utils._text import to_text + + +BOOLEANS_TRUE = frozenset(('y', 'yes', 'on', '1', 'true', 't', 1, 1.0, True)) +BOOLEANS_FALSE = frozenset(('n', 'no', 'off', '0', 'false', 'f', 0, 0.0, False)) +BOOLEANS = BOOLEANS_TRUE.union(BOOLEANS_FALSE) + + +def boolean(value, strict=True): + if isinstance(value, bool): + return value + + normalized_value = value + if isinstance(value, (text_type, binary_type)): + normalized_value = to_text(value, errors='surrogate_or_strict').lower() + + if normalized_value in BOOLEANS_TRUE: + return True + elif normalized_value in BOOLEANS_FALSE or not strict: + return False + + raise TypeError('%s is not a valid boolean. Valid booleans include: %s' % (to_text(value), ', '.join(repr(i) for i in BOOLEANS))) diff --git a/lib/ansible/module_utils/rax.py b/lib/ansible/module_utils/rax.py index af554d3223d..5c7ea669275 100644 --- a/lib/ansible/module_utils/rax.py +++ b/lib/ansible/module_utils/rax.py @@ -32,7 +32,6 @@ import os import re from uuid import UUID -from ansible.module_utils.basic import BOOLEANS from ansible.module_utils.six import text_type, binary_type FINAL_STATUSES = ('ACTIVE', 'ERROR') diff --git a/lib/ansible/playbook/base.py b/lib/ansible/playbook/base.py index 9a424c19d3d..775fd9878e6 100644 --- a/lib/ansible/playbook/base.py +++ b/lib/ansible/playbook/base.py @@ -28,8 +28,8 @@ from functools import partial from jinja2.exceptions import UndefinedError from ansible import constants as C -from ansible.constants import mk_boolean as boolean from ansible.module_utils.six import iteritems, string_types, with_metaclass +from ansible.module_utils.parsing.convert_bool import boolean from ansible.errors import AnsibleParserError, AnsibleUndefinedVariable from ansible.module_utils._text import to_text from ansible.playbook.attribute import Attribute, FieldAttribute @@ -388,7 +388,7 @@ class Base(with_metaclass(BaseMeta, object)): elif attribute.isa == 'float': value = float(value) elif attribute.isa == 'bool': - value = boolean(value) + value = boolean(value, strict=False) elif attribute.isa == 'percent': # special value, which may be an integer or float # with an optional '%' at the end diff --git a/lib/ansible/playbook/play_context.py b/lib/ansible/playbook/play_context.py index 701503890b1..8618ff3795e 100644 --- a/lib/ansible/playbook/play_context.py +++ b/lib/ansible/playbook/play_context.py @@ -33,21 +33,21 @@ from ansible.errors import AnsibleError from ansible.module_utils.six import iteritems from ansible.module_utils.six.moves import shlex_quote from ansible.module_utils._text import to_bytes +from ansible.module_utils.parsing.convert_bool import boolean from ansible.playbook.attribute import FieldAttribute from ansible.playbook.base import Base from ansible.utils.ssh_functions import check_for_controlpersist -boolean = C.mk_boolean - -__all__ = ['PlayContext'] - try: from __main__ import display except ImportError: from ansible.utils.display import Display display = Display() + +__all__ = ['PlayContext'] + # the magic variable mapping dictionary below is used to translate # host/inventory variables to fields in the PlayContext # object. The dictionary values are tuples, to account for aliases @@ -289,7 +289,7 @@ class PlayContext(Base): self.become_method = options.become_method self.become_user = options.become_user - self.check_mode = boolean(options.check) + self.check_mode = boolean(options.check, strict=False) # get ssh options FIXME: make these common to all connections for flag in ['ssh_common_args', 'docker_extra_args', 'sftp_extra_args', 'scp_extra_args', 'ssh_extra_args']: diff --git a/lib/ansible/plugins/action/assemble.py b/lib/ansible/plugins/action/assemble.py index 3fd48c5498f..398b1a3a9ff 100644 --- a/lib/ansible/plugins/action/assemble.py +++ b/lib/ansible/plugins/action/assemble.py @@ -25,9 +25,9 @@ import os.path import re import tempfile -from ansible.constants import mk_boolean as boolean from ansible.errors import AnsibleError from ansible.module_utils._text import to_native, to_text +from ansible.module_utils.parsing.convert_bool import boolean from ansible.plugins.action import ActionBase from ansible.utils.hashing import checksum_s @@ -101,7 +101,7 @@ class ActionModule(ActionBase): result['msg'] = "src and dest are required" return result - if boolean(remote_src): + if boolean(remote_src, strict=False): result.update(self._execute_module(tmp=tmp, task_vars=task_vars)) return result else: diff --git a/lib/ansible/plugins/action/copy.py b/lib/ansible/plugins/action/copy.py index f66c03a0c6a..3708e0c9d42 100644 --- a/lib/ansible/plugins/action/copy.py +++ b/lib/ansible/plugins/action/copy.py @@ -24,9 +24,9 @@ import os import stat import tempfile -from ansible.constants import mk_boolean as boolean from ansible.errors import AnsibleError, AnsibleFileNotFound from ansible.module_utils._text import to_bytes, to_native, to_text +from ansible.module_utils.parsing.convert_bool import boolean from ansible.plugins.action import ActionBase from ansible.utils.hashing import checksum @@ -43,11 +43,11 @@ class ActionModule(ActionBase): source = self._task.args.get('src', None) content = self._task.args.get('content', None) dest = self._task.args.get('dest', None) - raw = boolean(self._task.args.get('raw', 'no')) - force = boolean(self._task.args.get('force', 'yes')) - remote_src = boolean(self._task.args.get('remote_src', False)) - follow = boolean(self._task.args.get('follow', False)) - decrypt = boolean(self._task.args.get('decrypt', True)) + raw = boolean(self._task.args.get('raw', 'no'), strict=False) + force = boolean(self._task.args.get('force', 'yes'), strict=False) + remote_src = boolean(self._task.args.get('remote_src', False), strict=False) + follow = boolean(self._task.args.get('follow', False), strict=False) + decrypt = boolean(self._task.args.get('decrypt', True), strict=False) result['failed'] = True if (source is None and content is None) or dest is None: diff --git a/lib/ansible/plugins/action/fetch.py b/lib/ansible/plugins/action/fetch.py index 96191ba52a8..821a365a12c 100644 --- a/lib/ansible/plugins/action/fetch.py +++ b/lib/ansible/plugins/action/fetch.py @@ -20,10 +20,10 @@ __metaclass__ = type import os import base64 -from ansible.constants import mk_boolean as boolean from ansible.errors import AnsibleError from ansible.module_utils._text import to_bytes from ansible.module_utils.six import string_types +from ansible.module_utils.parsing.convert_bool import boolean from ansible.plugins.action import ActionBase from ansible.utils.hashing import checksum, checksum_s, md5, secure_hash from ansible.utils.path import makedirs_safe @@ -51,9 +51,11 @@ class ActionModule(ActionBase): source = self._task.args.get('src', None) dest = self._task.args.get('dest', None) - flat = boolean(self._task.args.get('flat')) - fail_on_missing = boolean(self._task.args.get('fail_on_missing')) - validate_checksum = boolean(self._task.args.get('validate_checksum', self._task.args.get('validate_md5', True))) + flat = boolean(self._task.args.get('flat'), strict=False) + fail_on_missing = boolean(self._task.args.get('fail_on_missing'), strict=False) + validate_checksum = boolean(self._task.args.get('validate_checksum', + self._task.args.get('validate_md5', True)), + strict=False) # validate source and dest are strings FIXME: use basic.py and module specs if not isinstance(source, string_types): diff --git a/lib/ansible/plugins/action/patch.py b/lib/ansible/plugins/action/patch.py index 06c96487a84..e3aea3923a7 100644 --- a/lib/ansible/plugins/action/patch.py +++ b/lib/ansible/plugins/action/patch.py @@ -20,9 +20,9 @@ __metaclass__ = type import os -from ansible.constants import mk_boolean as boolean from ansible.errors import AnsibleError from ansible.module_utils._text import to_native +from ansible.module_utils.parsing.convert_bool import boolean from ansible.plugins.action import ActionBase @@ -35,7 +35,7 @@ class ActionModule(ActionBase): result = super(ActionModule, self).run(tmp, task_vars) src = self._task.args.get('src', None) - remote_src = boolean(self._task.args.get('remote_src', 'no')) + remote_src = boolean(self._task.args.get('remote_src', 'no'), strict=False) if src is None: result['failed'] = True diff --git a/lib/ansible/plugins/action/set_fact.py b/lib/ansible/plugins/action/set_fact.py index 7532e12ac24..7c8f0346c9a 100644 --- a/lib/ansible/plugins/action/set_fact.py +++ b/lib/ansible/plugins/action/set_fact.py @@ -18,8 +18,8 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -from ansible.constants import mk_boolean as boolean from ansible.module_utils.six import iteritems, string_types +from ansible.module_utils.parsing.convert_bool import boolean from ansible.plugins.action import ActionBase from ansible.utils.vars import isidentifier @@ -46,7 +46,7 @@ class ActionModule(ActionBase): return result if isinstance(v, string_types) and v.lower() in ('true', 'false', 'yes', 'no'): - v = boolean(v) + v = boolean(v, strict=False) facts[k] = v result['changed'] = False diff --git a/lib/ansible/plugins/action/set_stats.py b/lib/ansible/plugins/action/set_stats.py index 1b85f10d38b..c0a55878be7 100644 --- a/lib/ansible/plugins/action/set_stats.py +++ b/lib/ansible/plugins/action/set_stats.py @@ -18,8 +18,8 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -from ansible.constants import mk_boolean as boolean from ansible.module_utils.six import iteritems, string_types +from ansible.module_utils.parsing.convert_bool import boolean from ansible.plugins.action import ActionBase from ansible.utils.vars import isidentifier @@ -53,7 +53,7 @@ class ActionModule(ActionBase): val = self._task.args.get(opt, None) if val is not None: if not isinstance(val, bool): - stats[opt] = boolean(self._templar.template(val)) + stats[opt] = boolean(self._templar.template(val), strict=False) else: stats[opt] = val diff --git a/lib/ansible/plugins/action/synchronize.py b/lib/ansible/plugins/action/synchronize.py index 9a0113b3c63..be142fe770f 100644 --- a/lib/ansible/plugins/action/synchronize.py +++ b/lib/ansible/plugins/action/synchronize.py @@ -23,12 +23,11 @@ from collections import MutableSequence from ansible import constants as C from ansible.module_utils.six import string_types from ansible.module_utils._text import to_text +from ansible.module_utils.parsing.convert_bool import boolean from ansible.playbook.play_context import MAGIC_VARIABLE_MAPPING from ansible.plugins.action import ActionBase from ansible.plugins import connection_loader -boolean = C.mk_boolean - class ActionModule(ActionBase): @@ -314,7 +313,7 @@ class ActionModule(ActionBase): user = None if not dest_is_local: # Src and dest rsync "path" handling - if boolean(_tmp_args.get('set_remote_user', 'yes')): + if boolean(_tmp_args.get('set_remote_user', 'yes'), strict=False): if use_delegate: user = task_vars.get('ansible_delegated_vars', dict()).get('ansible_ssh_user', None) if not user: diff --git a/lib/ansible/plugins/action/template.py b/lib/ansible/plugins/action/template.py index 5a39beb1e94..33aa580dceb 100644 --- a/lib/ansible/plugins/action/template.py +++ b/lib/ansible/plugins/action/template.py @@ -22,12 +22,11 @@ import os from ansible import constants as C from ansible.errors import AnsibleError, AnsibleFileNotFound from ansible.module_utils._text import to_text +from ansible.module_utils.parsing.convert_bool import boolean from ansible.plugins.action import ActionBase from ansible.template import generate_ansible_template_vars from ansible.utils.hashing import checksum_s -boolean = C.mk_boolean - class ActionModule(ActionBase): @@ -58,7 +57,7 @@ class ActionModule(ActionBase): source = self._task.args.get('src', None) dest = self._task.args.get('dest', None) - force = boolean(self._task.args.get('force', True)) + force = boolean(self._task.args.get('force', True), strict=False) state = self._task.args.get('state', None) newline_sequence = self._task.args.get('newline_sequence', self.DEFAULT_NEWLINE_SEQUENCE) variable_start_string = self._task.args.get('variable_start_string', None) diff --git a/lib/ansible/plugins/action/unarchive.py b/lib/ansible/plugins/action/unarchive.py index 0b4a0f4da0d..2dfd0c32cd8 100644 --- a/lib/ansible/plugins/action/unarchive.py +++ b/lib/ansible/plugins/action/unarchive.py @@ -21,10 +21,9 @@ __metaclass__ = type import os from ansible.errors import AnsibleError -from ansible.module_utils._text import to_native -from ansible.module_utils.pycompat24 import get_exception +from ansible.module_utils._text import to_text +from ansible.module_utils.parsing.convert_bool import boolean from ansible.plugins.action import ActionBase -from ansible.constants import mk_boolean as boolean class ActionModule(ActionBase): @@ -40,7 +39,7 @@ class ActionModule(ActionBase): source = self._task.args.get('src', None) dest = self._task.args.get('dest', None) - remote_src = boolean(self._task.args.get('remote_src', False)) + remote_src = boolean(self._task.args.get('remote_src', False), strict=False) creates = self._task.args.get('creates', None) decrypt = self._task.args.get('decrypt', True) @@ -53,7 +52,7 @@ class ActionModule(ActionBase): return result # We will take the information from copy and store it in # the remote_src var to use later in this file. - self._task.args['remote_src'] = remote_src = not boolean(self._task.args.pop('copy')) + self._task.args['remote_src'] = remote_src = not boolean(self._task.args.pop('copy'), strict=False) if source is None or dest is None: result['failed'] = True @@ -79,17 +78,17 @@ class ActionModule(ActionBase): if not remote_src: try: source = self._loader.get_real_file(self._find_needle('files', source), decrypt=decrypt) - except AnsibleError: + except AnsibleError as e: result['failed'] = True - result['msg'] = to_native(get_exception()) + result['msg'] = to_text(e) self._remove_tmp_path(tmp) return result try: remote_stat = self._execute_remote_stat(dest, all_vars=task_vars, follow=True) - except AnsibleError: + except AnsibleError as e: result['failed'] = True - result['msg'] = to_native(get_exception()) + result['msg'] = to_text(e) self._remove_tmp_path(tmp) return result diff --git a/lib/ansible/plugins/callback/slack.py b/lib/ansible/plugins/callback/slack.py index fbb2a48a7aa..13c5b1183f8 100644 --- a/lib/ansible/plugins/callback/slack.py +++ b/lib/ansible/plugins/callback/slack.py @@ -28,8 +28,8 @@ try: except ImportError: cli = None -from ansible.constants import mk_boolean from ansible.module_utils.urls import open_url +from ansible.module_utils.parsing.convert_bool import boolean from ansible.plugins.callback import CallbackBase try: @@ -79,8 +79,9 @@ class CallbackModule(CallbackBase): self.webhook_url = os.getenv('SLACK_WEBHOOK_URL') self.channel = os.getenv('SLACK_CHANNEL', '#ansible') self.username = os.getenv('SLACK_USERNAME', 'ansible') - self.show_invocation = mk_boolean( - os.getenv('SLACK_INVOCATION', self._display.verbosity > 1) + self.show_invocation = boolean( + os.getenv('SLACK_INVOCATION', self._display.verbosity > 1), + strict=False ) if self.webhook_url is None: diff --git a/lib/ansible/plugins/connection/ssh.py b/lib/ansible/plugins/connection/ssh.py index 400d2a26cae..500c1a1b031 100644 --- a/lib/ansible/plugins/connection/ssh.py +++ b/lib/ansible/plugins/connection/ssh.py @@ -111,16 +111,14 @@ from functools import wraps from ansible import constants as C from ansible.errors import AnsibleError, AnsibleConnectionFailure, AnsibleFileNotFound from ansible.errors import AnsibleOptionsError -from ansible.module_utils.basic import BOOLEANS from ansible.compat import selectors from ansible.module_utils.six import PY3, text_type, binary_type from ansible.module_utils.six.moves import shlex_quote from ansible.module_utils._text import to_bytes, to_native, to_text +from ansible.module_utils.parsing.convert_bool import BOOLEANS, boolean from ansible.plugins.connection import ConnectionBase, BUFSIZE from ansible.utils.path import unfrackpath, makedirs_safe -boolean = C.mk_boolean - try: from __main__ import display except ImportError: @@ -791,7 +789,7 @@ class Connection(ConnectionBase): if not isinstance(scp_if_ssh, bool): scp_if_ssh = scp_if_ssh.lower() if scp_if_ssh in BOOLEANS: - scp_if_ssh = boolean(scp_if_ssh) + scp_if_ssh = boolean(scp_if_ssh, strict=False) elif scp_if_ssh != 'smart': raise AnsibleOptionsError('scp_if_ssh needs to be one of [smart|True|False]') if scp_if_ssh == 'smart': diff --git a/lib/ansible/plugins/lookup/first_found.py b/lib/ansible/plugins/lookup/first_found.py index b41e714ca32..be4f7d145bf 100644 --- a/lib/ansible/plugins/lookup/first_found.py +++ b/lib/ansible/plugins/lookup/first_found.py @@ -122,9 +122,9 @@ import os from jinja2.exceptions import UndefinedError -from ansible.constants import mk_boolean as boolean from ansible.errors import AnsibleFileNotFound, AnsibleLookupError, AnsibleUndefinedVariable from ansible.module_utils.six import string_types +from ansible.module_utils.parsing.convert_bool import boolean from ansible.plugins.lookup import LookupBase @@ -145,7 +145,7 @@ class LookupModule(LookupBase): if isinstance(term, dict): files = term.get('files', []) paths = term.get('paths', []) - skip = boolean(term.get('skip', False)) + skip = boolean(term.get('skip', False), strict=False) filelist = files if isinstance(files, string_types): diff --git a/lib/ansible/plugins/lookup/hashi_vault.py b/lib/ansible/plugins/lookup/hashi_vault.py index 664e8cc3484..1ff3f1f1c8e 100644 --- a/lib/ansible/plugins/lookup/hashi_vault.py +++ b/lib/ansible/plugins/lookup/hashi_vault.py @@ -46,8 +46,8 @@ __metaclass__ = type import os from ansible.errors import AnsibleError +from ansible.module_utils.parsing.convert_bool import boolean from ansible.plugins.lookup import LookupBase -from ansible.constants import mk_boolean as boolean HAS_HVAC = False try: @@ -149,7 +149,7 @@ class HashiVault: self.client.auth_ldap(username, password, mount_point) def boolean_or_cacert(self, validate_certs, cacert): - validate_certs = boolean(validate_certs) + validate_certs = boolean(validate_certs, strict=False) '''' return a bool or cacert ''' if validate_certs is True: if cacert != '': diff --git a/lib/ansible/plugins/lookup/subelements.py b/lib/ansible/plugins/lookup/subelements.py index 9e3a840124f..07721994ee7 100644 --- a/lib/ansible/plugins/lookup/subelements.py +++ b/lib/ansible/plugins/lookup/subelements.py @@ -17,9 +17,9 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -from ansible.constants import mk_boolean as boolean from ansible.errors import AnsibleError from ansible.module_utils.six import string_types +from ansible.module_utils.parsing.convert_bool import boolean from ansible.plugins.lookup import LookupBase from ansible.utils.listify import listify_lookup_plugin_terms @@ -72,7 +72,7 @@ class LookupModule(LookupBase): # this particular item is to be skipped continue - skip_missing = boolean(flags.get('skip_missing', False)) + skip_missing = boolean(flags.get('skip_missing', False), strict=False) subvalue = item0 lastsubkey = False sublist = [] diff --git a/test/runner/injector/importer.py b/test/runner/injector/importer.py deleted file mode 120000 index 1f9d09cbf2a..00000000000 --- a/test/runner/injector/importer.py +++ /dev/null @@ -1 +0,0 @@ -injector.py \ No newline at end of file diff --git a/test/runner/injector/importer.py b/test/runner/injector/importer.py new file mode 100755 index 00000000000..31bc0cbc375 --- /dev/null +++ b/test/runner/injector/importer.py @@ -0,0 +1,256 @@ +#!/usr/bin/env python +"""Interpreter and code coverage injector for use with ansible-test. + +The injector serves two main purposes: + +1) Control the python interpreter used to run test tools and ansible code. +2) Provide optional code coverage analysis of ansible code. + +The injector is executed one of two ways: + +1) On the controller via a symbolic link such as ansible or pytest. + This is accomplished by prepending the injector directory to the PATH by ansible-test. + +2) As the python interpreter when running ansible modules. + This is only supported when connecting to the local host. + Otherwise set the ANSIBLE_TEST_REMOTE_INTERPRETER environment variable. + It can be empty to auto-detect the python interpreter on the remote host. + If not empty it will be used to set ansible_python_interpreter. + +NOTE: Running ansible-test with the --tox option or inside a virtual environment + may prevent the injector from working for tests which use connection + types other than local, or which use become, due to lack of permissions + to access the interpreter for the virtual environment. +""" + +from __future__ import absolute_import, print_function + +import errno +import json +import os +import sys +import pipes +import logging +import getpass + +logger = logging.getLogger('injector') # pylint: disable=locally-disabled, invalid-name +# pylint: disable=locally-disabled, invalid-name +config = None # type: InjectorConfig + + +class InjectorConfig(object): + """Mandatory configuration.""" + def __init__(self, config_path): + """Initialize config.""" + with open(config_path) as config_fd: + _config = json.load(config_fd) + + self.python_interpreter = _config['python_interpreter'] + self.coverage_file = _config['coverage_file'] + + # Read from the environment instead of config since it needs to be changed by integration test scripts. + # It also does not need to flow from the controller to the remote. It is only used on the controller. + self.remote_interpreter = os.environ.get('ANSIBLE_TEST_REMOTE_INTERPRETER', None) + + self.arguments = [to_text(c) for c in sys.argv] + + +def to_text(value): + """ + :type value: str | None + :rtype: str | None + """ + if value is None: + return None + + if isinstance(value, bytes): + return value.decode('utf-8') + + return u'%s' % value + + +def main(): + """Main entry point.""" + global config # pylint: disable=locally-disabled, global-statement + + formatter = logging.Formatter('%(asctime)s %(process)d %(levelname)s %(message)s') + log_name = 'ansible-test-coverage.%s.log' % getpass.getuser() + self_dir = os.path.dirname(os.path.abspath(__file__)) + + handler = logging.FileHandler(os.path.join('/tmp', log_name)) + handler.setFormatter(formatter) + logger.addHandler(handler) + + handler = logging.FileHandler(os.path.abspath(os.path.join(self_dir, '..', 'logs', log_name))) + handler.setFormatter(formatter) + logger.addHandler(handler) + + logger.setLevel(logging.DEBUG) + + try: + logger.debug('Self: %s', __file__) + + config_path = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'injector.json') + + try: + config = InjectorConfig(config_path) + except IOError: + logger.exception('Error reading config: %s', config_path) + exit('No injector config found. Set ANSIBLE_TEST_REMOTE_INTERPRETER if the test is not connecting to the local host.') + + logger.debug('Arguments: %s', ' '.join(pipes.quote(c) for c in config.arguments)) + logger.debug('Python interpreter: %s', config.python_interpreter) + logger.debug('Remote interpreter: %s', config.remote_interpreter) + logger.debug('Coverage file: %s', config.coverage_file) + + require_cwd = False + + if os.path.basename(__file__) == 'injector.py': + if config.coverage_file: + args, env, require_cwd = cover() + else: + args, env = runner() + else: + args, env = injector() + + logger.debug('Run command: %s', ' '.join(pipes.quote(c) for c in args)) + + altered_cwd = False + + try: + cwd = os.getcwd() + except OSError as ex: + # some platforms, such as OS X, may not allow querying the working directory when using become to drop privileges + if ex.errno != errno.EACCES: + raise + if require_cwd: + # make sure the program we execute can determine the working directory if it's required + cwd = '/' + os.chdir(cwd) + altered_cwd = True + else: + cwd = None + + logger.debug('Working directory: %s%s', cwd or '?', ' (altered)' if altered_cwd else '') + + for key in sorted(env.keys()): + logger.debug('%s=%s', key, env[key]) + + os.execvpe(args[0], args, env) + except Exception as ex: + logger.fatal(ex) + raise + + +def injector(): + """ + :rtype: list[str], dict[str, str] + """ + command = os.path.basename(__file__) + executable = find_executable(command) + + if config.coverage_file: + args, env = coverage_command() + else: + args, env = [config.python_interpreter], os.environ.copy() + + args += [executable] + + if command in ('ansible', 'ansible-playbook', 'ansible-pull'): + if config.remote_interpreter is None: + interpreter = os.path.join(os.path.dirname(__file__), 'injector.py') + elif config.remote_interpreter == '': + interpreter = None + else: + interpreter = config.remote_interpreter + + if interpreter: + args += ['--extra-vars', 'ansible_python_interpreter=' + interpreter] + + args += config.arguments[1:] + + return args, env + + +def runner(): + """ + :rtype: list[str], dict[str, str] + """ + args, env = [config.python_interpreter], os.environ.copy() + + args += config.arguments[1:] + + return args, env + + +def cover(): + """ + :rtype: list[str], dict[str, str], bool + """ + if len(config.arguments) > 1: + executable = config.arguments[1] + else: + executable = '' + + require_cwd = False + + if os.path.basename(executable).startswith('ansible_module_'): + args, env = coverage_command() + # coverage requires knowing the working directory + require_cwd = True + else: + args, env = [config.python_interpreter], os.environ.copy() + + args += config.arguments[1:] + + return args, env, require_cwd + + +def coverage_command(): + """ + :rtype: list[str], dict[str, str] + """ + self_dir = os.path.dirname(os.path.abspath(__file__)) + + args = [ + config.python_interpreter, + '-m', + 'coverage.__main__', + 'run', + '--rcfile', + os.path.join(self_dir, '.coveragerc'), + ] + + env = os.environ.copy() + env['COVERAGE_FILE'] = config.coverage_file + + return args, env + + +def find_executable(executable): + """ + :type executable: str + :rtype: str + """ + self = os.path.abspath(__file__) + path = os.environ.get('PATH', os.defpath) + seen_dirs = set() + + for path_dir in path.split(os.pathsep): + if path_dir in seen_dirs: + continue + + seen_dirs.add(path_dir) + candidate = os.path.abspath(os.path.join(path_dir, executable)) + + if candidate == self: + continue + + if os.path.exists(candidate) and os.access(candidate, os.F_OK | os.X_OK): + return candidate + + raise Exception('Executable "%s" not found in path: %s' % (executable, path)) + + +if __name__ == '__main__': + main() diff --git a/test/units/module_utils/parsing/test_convert_bool.py b/test/units/module_utils/parsing/test_convert_bool.py new file mode 100644 index 00000000000..ed8fd7f0ed5 --- /dev/null +++ b/test/units/module_utils/parsing/test_convert_bool.py @@ -0,0 +1,60 @@ +# -*- coding: utf-8 -*- +# Copyright: (c) 2017 Ansible Project +# License: GNU General Public License v3 or later (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt ) + +# Make coding more python3-ish +from __future__ import (absolute_import, division) +__metaclass__ = type + +import pytest + +from ansible.module_utils.parsing.convert_bool import boolean + + +class TestBoolean: + def test_bools(self): + assert boolean(True) is True + assert boolean(False) is False + + def test_none(self): + with pytest.raises(TypeError): + assert boolean(None, strict=True) is False + assert boolean(None, strict=False) is False + + def test_numbers(self): + assert boolean(1) is True + assert boolean(0) is False + assert boolean(0.0) is False + +# Current boolean() doesn't consider these to be true values +# def test_other_numbers(self): +# assert boolean(2) is True +# assert boolean(-1) is True +# assert boolean(0.1) is True + + def test_strings(self): + assert boolean("true") is True + assert boolean("TRUE") is True + assert boolean("t") is True + assert boolean("yes") is True + assert boolean("y") is True + assert boolean("on") is True + + def test_junk_values_nonstrict(self): + assert boolean("flibbity", strict=False) is False + assert boolean(42, strict=False) is False + assert boolean(42.0, strict=False) is False + assert boolean(object(), strict=False) is False + + def test_junk_values_strict(self): + with pytest.raises(TypeError): + assert boolean("flibbity", strict=True)is False + + with pytest.raises(TypeError): + assert boolean(42, strict=True)is False + + with pytest.raises(TypeError): + assert boolean(42.0, strict=True)is False + + with pytest.raises(TypeError): + assert boolean(object(), strict=True)is False