From bcddde229d63ea17ded312c8dd879540d10e84d6 Mon Sep 17 00:00:00 2001 From: "Bradley A. Thornton" Date: Tue, 20 Oct 2020 12:14:23 -0700 Subject: [PATCH] Plugin cleanup (#14) * WIP * Add argspec validation to plugins, restructure tests * Update docs * Pre PY3.8 compat changes * Run black to fix quotes * Seems the order of missing keys varies between 2.9 and 2.10 * More error ordering issues fixed during argspec validation * More black, wrong version Co-authored-by: cidrblock --- docs/ansible.utils.get_path_lookup.rst | 20 +----- docs/ansible.utils.index_of_lookup.rst | 20 +----- docs/ansible.utils.to_paths_lookup.rst | 20 +----- plugins/filter/get_path.py | 52 ++++++++++++++ plugins/filter/index_of.py | 36 ++++++++-- plugins/filter/{paths.py => to_paths.py} | 40 ++++++----- plugins/lookup/get_path.py | 33 +++++---- plugins/lookup/index_of.py | 38 ++++++++--- plugins/lookup/to_paths.py | 31 ++++++--- plugins/module_utils/common/get_path.py | 31 +++++++++ plugins/module_utils/common/index_of.py | 22 ------ .../common/{path.py => to_paths.py} | 31 +-------- .../get_path/tasks/include/argspec.yaml | 31 +++++++++ .../get_path/tasks/include/simple.yaml | 58 ++++++++++++++++ .../targets/get_path/tasks/main.yaml | 68 +++---------------- .../index_of/tasks/include/argspec.yaml | 57 ++++++++++++++++ .../tasks/{ => include}/examples.yaml | 0 .../index_of/tasks/{ => include}/simple.yaml | 0 .../targets/index_of/tasks/main.yaml | 14 +++- .../to_paths/tasks/include/argspec.yaml | 33 +++++++++ .../to_paths/tasks/include/simple.yaml | 52 ++++++++++++++ .../targets/to_paths/tasks/main.yaml | 62 +++-------------- tests/sanity/ignore-2.10.txt | 4 +- tests/sanity/ignore-2.11.txt | 4 +- tests/sanity/ignore-2.9.txt | 4 +- tests/unit/module_utils/test_get_path.py | 51 ++++++++++++++ tests/unit/module_utils/test_index_of.py | 18 +---- .../{test_path_utils.py => test_to_paths.py} | 50 ++++---------- 28 files changed, 544 insertions(+), 336 deletions(-) create mode 100644 plugins/filter/get_path.py rename plugins/filter/{paths.py => to_paths.py} (52%) create mode 100644 plugins/module_utils/common/get_path.py rename plugins/module_utils/common/{path.py => to_paths.py} (53%) create mode 100644 tests/integration/targets/get_path/tasks/include/argspec.yaml create mode 100644 tests/integration/targets/get_path/tasks/include/simple.yaml create mode 100644 tests/integration/targets/index_of/tasks/include/argspec.yaml rename tests/integration/targets/index_of/tasks/{ => include}/examples.yaml (100%) rename tests/integration/targets/index_of/tasks/{ => include}/simple.yaml (100%) create mode 100644 tests/integration/targets/to_paths/tasks/include/argspec.yaml create mode 100644 tests/integration/targets/to_paths/tasks/include/simple.yaml create mode 100644 tests/unit/module_utils/test_get_path.py rename tests/unit/module_utils/{test_path_utils.py => test_to_paths.py} (55%) diff --git a/docs/ansible.utils.get_path_lookup.rst b/docs/ansible.utils.get_path_lookup.rst index f2355e8..ddd76f3 100644 --- a/docs/ansible.utils.get_path_lookup.rst +++ b/docs/ansible.utils.get_path_lookup.rst @@ -35,24 +35,6 @@ Parameters Configuration Comments - - -
- _terms - -
- - - / required -
- - - - - - -
The values below provided in the order var, path, wantlist=.
- -
@@ -119,7 +101,7 @@ Parameters Examples -------- -.. code-block:: yaml+jinja +.. code-block:: yaml - ansible.builtin.set_fact: a: diff --git a/docs/ansible.utils.index_of_lookup.rst b/docs/ansible.utils.index_of_lookup.rst index ada5bb4..43fe80b 100644 --- a/docs/ansible.utils.index_of_lookup.rst +++ b/docs/ansible.utils.index_of_lookup.rst @@ -36,24 +36,6 @@ Parameters Configuration Comments - - -
- _terms - -
- - - / required -
- - - - - - -
The values below provided in the order test, value, key.
- -
@@ -175,7 +157,7 @@ Parameters Examples -------- -.. code-block:: yaml+jinja +.. code-block:: yaml #### Simple examples using a list of values diff --git a/docs/ansible.utils.to_paths_lookup.rst b/docs/ansible.utils.to_paths_lookup.rst index 583f3ca..43312ed 100644 --- a/docs/ansible.utils.to_paths_lookup.rst +++ b/docs/ansible.utils.to_paths_lookup.rst @@ -37,24 +37,6 @@ Parameters Configuration Comments - - -
- _terms - -
- - - / required -
- - - - - - -
The values below provided in the order var, prepend=, wantlist=.
- -
@@ -120,7 +102,7 @@ Parameters Examples -------- -.. code-block:: yaml+jinja +.. code-block:: yaml #### Simple examples diff --git a/plugins/filter/get_path.py b/plugins/filter/get_path.py new file mode 100644 index 0000000..40bd900 --- /dev/null +++ b/plugins/filter/get_path.py @@ -0,0 +1,52 @@ +# -*- coding: utf-8 -*- +# Copyright 2020 Red Hat +# GNU General Public License v3.0+ +# (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + + +""" +flatten a complex object to dot bracket notation +""" +from __future__ import absolute_import, division, print_function + +__metaclass__ = type + +from ansible.errors import AnsibleFilterError +from jinja2.filters import environmentfilter + +from ansible_collections.ansible.utils.plugins.module_utils.common.get_path import ( + get_path, +) +from ansible_collections.ansible.utils.plugins.lookup.get_path import ( + DOCUMENTATION, +) +from ansible_collections.ansible.utils.plugins.module_utils.common.argspec_validate import ( + AnsibleArgSpecValidator, +) + + +@environmentfilter +def _get_path(*args, **kwargs): + """Retrieve the value in a variable using a path. [See examples](https://github.com/ansible-collections/ansible.utils/blob/main/docs/ansible.utils.get_path_lookup.rst)""" + keys = ["environment", "var", "path"] + data = dict(zip(keys, args)) + data.update(kwargs) + environment = data.pop("environment") + aav = AnsibleArgSpecValidator( + data=data, + schema=DOCUMENTATION, + schema_format="doc", + name="get_path", + ) + valid, errors, updated_data = aav.validate() + if not valid: + raise AnsibleFilterError(errors) + updated_data["environment"] = environment + return get_path(**updated_data) + + +class FilterModule(object): + """ path filters """ + + def filters(self): + return {"get_path": _get_path} diff --git a/plugins/filter/index_of.py b/plugins/filter/index_of.py index 39f88ac..a613052 100644 --- a/plugins/filter/index_of.py +++ b/plugins/filter/index_of.py @@ -11,18 +11,46 @@ from __future__ import absolute_import, division, print_function __metaclass__ = type +from ansible.errors import AnsibleFilterError +from jinja2.filters import environmentfilter from ansible_collections.ansible.utils.plugins.module_utils.common.index_of import ( index_of, ) -from jinja2.filters import environmentfilter +from ansible_collections.ansible.utils.plugins.lookup.index_of import ( + DOCUMENTATION, +) +from ansible_collections.ansible.utils.plugins.module_utils.common.argspec_validate import ( + AnsibleArgSpecValidator, +) @environmentfilter def _index_of(*args, **kwargs): """Find the indicies of items in a list matching some criteria. [See examples](https://github.com/ansible-collections/ansible.utils/blob/main/docs/ansible.utils.index_of_lookup.rst)""" - kwargs["tests"] = args[0].tests - args = args[1:] - return index_of(*args, **kwargs) + + keys = [ + "environment", + "data", + "test", + "value", + "key", + "fail_on_missing", + "wantlist", + ] + data = dict(zip(keys, args)) + data.update(kwargs) + environment = data.pop("environment") + aav = AnsibleArgSpecValidator( + data=data, + schema=DOCUMENTATION, + schema_format="doc", + name="index_of", + ) + valid, errors, updated_data = aav.validate() + if not valid: + raise AnsibleFilterError(errors) + updated_data["tests"] = environment.tests + return index_of(**updated_data) class FilterModule(object): diff --git a/plugins/filter/paths.py b/plugins/filter/to_paths.py similarity index 52% rename from plugins/filter/paths.py rename to plugins/filter/to_paths.py index 21d5b49..b0b2cd5 100644 --- a/plugins/filter/paths.py +++ b/plugins/filter/to_paths.py @@ -11,34 +11,38 @@ from __future__ import absolute_import, division, print_function __metaclass__ = type -from ansible.errors import AnsibleFilterError -from ansible.module_utils.common._collections_compat import ( - Mapping, - MutableMapping, -) -from ansible_collections.ansible.utils.plugins.module_utils.common.path import ( +from ansible.errors import AnsibleFilterError +from ansible_collections.ansible.utils.plugins.module_utils.common.to_paths import ( to_paths, - get_path, ) -from jinja2.filters import environmentfilter +from ansible_collections.ansible.utils.plugins.lookup.to_paths import ( + DOCUMENTATION, +) +from ansible_collections.ansible.utils.plugins.module_utils.common.argspec_validate import ( + AnsibleArgSpecValidator, +) def _to_paths(*args, **kwargs): """Flatten a complex object into a dictionary of paths and values. [See examples](https://github.com/ansible-collections/ansible.utils/blob/main/docs/ansible.utils.to_paths_lookup.rst)""" - return to_paths(*args, **kwargs) - - -@environmentfilter -def _get_path(*args, **kwargs): - """Retrieve the value in a variable using a path. [See examples](https://github.com/ansible-collections/ansible.utils/blob/main/docs/ansible.utils.get_path_lookup.rst)""" - kwargs["environment"] = args[0] - args = args[1:] - return get_path(*args, **kwargs) + keys = ["var", "prepend", "wantlist"] + data = dict(zip(keys, args)) + data.update(kwargs) + aav = AnsibleArgSpecValidator( + data=data, + schema=DOCUMENTATION, + schema_format="doc", + name="to_paths", + ) + valid, errors, updated_data = aav.validate() + if not valid: + raise AnsibleFilterError(errors) + return to_paths(**updated_data) class FilterModule(object): """ path filters """ def filters(self): - return {"to_paths": _to_paths, "get_path": _get_path} + return {"to_paths": _to_paths} diff --git a/plugins/lookup/get_path.py b/plugins/lookup/get_path.py index 4d6bc9b..7b26c2d 100644 --- a/plugins/lookup/get_path.py +++ b/plugins/lookup/get_path.py @@ -21,9 +21,6 @@ DOCUMENTATION = """ - Use a C(path) to retreive a nested value from a C(var) - C(get_path) is also available as a C(filter_plugin) for convenience options: - _terms: - description: The values below provided in the order C(var), C(path), C(wantlist=). - required: True var: description: The variable from which the value should be extraced type: raw @@ -155,20 +152,32 @@ RETURN = """ - See C(wantlist) if a list is always required """ +from ansible.errors import AnsibleLookupError from ansible.plugins.lookup import LookupBase -from ansible_collections.ansible.utils.plugins.module_utils.common.path import ( +from ansible_collections.ansible.utils.plugins.module_utils.common.get_path import ( get_path, ) +from ansible_collections.ansible.utils.plugins.module_utils.common.argspec_validate import ( + AnsibleArgSpecValidator, +) class LookupModule(LookupBase): def run(self, terms, variables, **kwargs): - kwargs["environment"] = self._templar.environment - if isinstance(terms, dict): - terms.update(kwargs) - res = get_path(**terms) - else: - res = get_path(*terms, **kwargs) - if not isinstance(res, list): - return [res] + if isinstance(terms, list): + keys = ["var", "path"] + terms = dict(zip(keys, terms)) + terms.update(kwargs) + aav = AnsibleArgSpecValidator( + data=terms, + schema=DOCUMENTATION, + schema_format="doc", + name="get_path", + ) + valid, errors, updated_data = aav.validate() + if not valid: + raise AnsibleLookupError(errors) + updated_data["wantlist"] = True + updated_data["environment"] = self._templar.environment + res = get_path(**updated_data) return res diff --git a/plugins/lookup/index_of.py b/plugins/lookup/index_of.py index 486d331..40e2c15 100644 --- a/plugins/lookup/index_of.py +++ b/plugins/lookup/index_of.py @@ -22,9 +22,6 @@ DOCUMENTATION = """ - When working with a list of dictionaries, the key to evaluate can be specified - C(index_of) is also available as a C(filter_plugin) for convenience options: - _terms: - description: The values below provided in the order C(test), C(value), C(key). - required: True data: description: A list of items to enumerate and test against type: list @@ -354,20 +351,39 @@ RETURN = """ - See C(wantlist) if a list is always required """ +from ansible.errors import AnsibleLookupError from ansible.plugins.lookup import LookupBase from ansible_collections.ansible.utils.plugins.module_utils.common.index_of import ( index_of, ) +from ansible_collections.ansible.utils.plugins.module_utils.common.argspec_validate import ( + AnsibleArgSpecValidator, +) class LookupModule(LookupBase): def run(self, terms, variables, **kwargs): - kwargs["tests"] = self._templar.environment.tests - if isinstance(terms, dict): - terms.update(kwargs) - res = index_of(**terms) - else: - res = index_of(*terms, **kwargs) - if not isinstance(res, list): - return [res] + if isinstance(terms, list): + keys = [ + "data", + "test", + "value", + "key", + "fail_on_missing", + "wantlist", + ] + terms = dict(zip(keys, terms)) + terms.update(kwargs) + aav = AnsibleArgSpecValidator( + data=terms, + schema=DOCUMENTATION, + schema_format="doc", + name="index_of", + ) + valid, errors, updated_data = aav.validate() + if not valid: + raise AnsibleLookupError(errors) + updated_data["wantlist"] = True + updated_data["tests"] = self._templar.environment.tests + res = index_of(**updated_data) return res diff --git a/plugins/lookup/to_paths.py b/plugins/lookup/to_paths.py index f1a2137..767a8c2 100644 --- a/plugins/lookup/to_paths.py +++ b/plugins/lookup/to_paths.py @@ -23,9 +23,6 @@ DOCUMENTATION = """ - Brakets are used for list indicies and keys that contain special characters - C(to_paths) is also available as a filter plugin options: - _terms: - description: The values below provided in the order C(var), C(prepend=), C(wantlist=). - required: True var: description: The value of C(var) will be will be used. type: raw @@ -139,19 +136,31 @@ RETURN = """ - The value is the value """ +from ansible.errors import AnsibleLookupError from ansible.plugins.lookup import LookupBase -from ansible_collections.ansible.utils.plugins.module_utils.common.path import ( +from ansible_collections.ansible.utils.plugins.module_utils.common.to_paths import ( to_paths, ) +from ansible_collections.ansible.utils.plugins.module_utils.common.argspec_validate import ( + AnsibleArgSpecValidator, +) class LookupModule(LookupBase): def run(self, terms, variables, **kwargs): - if isinstance(terms, dict): - terms.update(kwargs) - res = to_paths(**terms) - else: - res = to_paths(*terms, **kwargs) - if not isinstance(res, list): - return [res] + if isinstance(terms, list): + keys = ["var", "prepend"] + terms = dict(zip(keys, terms)) + terms.update(kwargs) + aav = AnsibleArgSpecValidator( + data=terms, + schema=DOCUMENTATION, + schema_format="doc", + name="to_paths", + ) + valid, errors, updated_data = aav.validate() + if not valid: + raise AnsibleLookupError(errors) + updated_data["wantlist"] = True + res = to_paths(**updated_data) return res diff --git a/plugins/module_utils/common/get_path.py b/plugins/module_utils/common/get_path.py new file mode 100644 index 0000000..68bc42e --- /dev/null +++ b/plugins/module_utils/common/get_path.py @@ -0,0 +1,31 @@ +# -*- coding: utf-8 -*- +# Copyright 2020 Red Hat +# GNU General Public License v3.0+ +# (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + + +""" +flatten a complex object to dot bracket notation +""" +from __future__ import absolute_import, division, print_function + +__metaclass__ = type + + +def get_path(var, path, environment, wantlist): + """Get the value of a path within an object + + :param var: The var from which the value is retrieved + :type var: should be dict or list, but jinja can sort that out + :param path: The path to get + :type path: should be a string but jinja can sort that out + :param environment: The jinja Environment + :type environment: Environment + :return: The result of the jinja evaluation + :rtype: any + """ + string_to_variable = "{{ %s }}" % path + result = environment.from_string(string_to_variable).render(**var) + if wantlist: + return [result] + return result diff --git a/plugins/module_utils/common/index_of.py b/plugins/module_utils/common/index_of.py index 264bb45..3d2858c 100644 --- a/plugins/module_utils/common/index_of.py +++ b/plugins/module_utils/common/index_of.py @@ -59,27 +59,6 @@ def _to_well_known_type(obj): return json.loads(json.dumps(obj)) -def _check_reqs(obj, wantlist): - """Check the args passed, ensure given a list - - :param obj: The object passed to the filter plugin - :type obj: unknown - """ - errors = [] - if not is_sequence(obj): - msg = "a list is required, was passed a '{type}'.".format( - type=type(_to_well_known_type(obj)).__name__ - ) - errors.append(msg) - if not isinstance(wantlist, bool): - msg = "'wantlist' is required to be a bool, was passed a '{type}'.".format( - type=type(_to_well_known_type(wantlist)).__name__ - ) - errors.append(msg) - if errors: - _raise_error(" ".join(errors)) - - def _run_test(entry, test, right, tests): """Run a test @@ -167,7 +146,6 @@ def index_of( :param tests: The jinja tests from the current environment :type tests: ansible.template.JinjaPluginIntercept """ - _check_reqs(data, wantlist) res = list() if key is None: for idx, entry in enumerate(data): diff --git a/plugins/module_utils/common/path.py b/plugins/module_utils/common/to_paths.py similarity index 53% rename from plugins/module_utils/common/path.py rename to plugins/module_utils/common/to_paths.py index 418ccd5..e15eea4 100644 --- a/plugins/module_utils/common/path.py +++ b/plugins/module_utils/common/to_paths.py @@ -17,38 +17,9 @@ from ansible.module_utils.common._collections_compat import ( MutableMapping, ) -# Note, this file can only be used on the control node -# where ansible is installed -# limit imports to filter and lookup plugins -try: - from ansible.errors import AnsibleError -except ImportError: - pass - -def get_path(var, path, environment, wantlist=False): - """Get the value of a path within an object - - :param var: The var from which the value is retrieved - :type var: should be dict or list, but jinja can sort that out - :param path: The path to get - :type path: should be a string but jinja can sort that out - :param environment: The jinja Environment - :type environment: Environment - :return: The result of the jinja evaluation - :rtype: any - """ - string_to_variable = "{{ %s }}" % path - result = environment.from_string(string_to_variable).render(**var) - if wantlist: - return list(result) - return result - - -def to_paths(var, prepend=False, wantlist=False): +def to_paths(var, prepend, wantlist): if prepend: - if not isinstance(prepend, str): - raise AnsibleError("The value of 'prepend' must be a string.") var = {prepend: var} out = {} diff --git a/tests/integration/targets/get_path/tasks/include/argspec.yaml b/tests/integration/targets/get_path/tasks/include/argspec.yaml new file mode 100644 index 0000000..8a6609e --- /dev/null +++ b/tests/integration/targets/get_path/tasks/include/argspec.yaml @@ -0,0 +1,31 @@ +- set_fact: + a: + b: + c: + d: + - 0 + - 1 + +- name: Check argspec validation with filter + set_fact: + _result: "{{ a|ansible.utils.get_path() }}" + ignore_errors: True + register: result + +- assert: + that: "{{ result.msg == msg }}" + vars: + msg: "missing required arguments: path" + +- name: Check argspec validation with lookup + set_fact: + _result: "{{ lookup('ansible.utils.get_path') }}" + ignore_errors: True + register: result + +- assert: + that: "{{ item in result.msg }}" + loop: + - "missing required arguments:" + - path + - var diff --git a/tests/integration/targets/get_path/tasks/include/simple.yaml b/tests/integration/targets/get_path/tasks/include/simple.yaml new file mode 100644 index 0000000..41043ff --- /dev/null +++ b/tests/integration/targets/get_path/tasks/include/simple.yaml @@ -0,0 +1,58 @@ +- set_fact: + a: + b: + c: + d: + - 0 + - 1 + +- name: Simple test filter and lookup + assert: + that: "{{ item.result == item.expected }}" + loop: + - result: "{{ vars|ansible.utils.get_path('a') }}" + expected: "{{ a }}" + - result: "{{ a|ansible.utils.get_path('b') }}" + expected: "{{ a.b }}" + - result: "{{ a|ansible.utils.get_path('b.c') }}" + expected: "{{ a.b.c }}" + - result: "{{ a|ansible.utils.get_path('b.c.d') }}" + expected: "{{ a.b.c.d }}" + - result: "{{ a|ansible.utils.get_path('b.c.d[0]') }}" + expected: "{{ a.b.c.d[0] }}" + - result: "{{ a|ansible.utils.get_path('b.c.d[1]') }}" + expected: "{{ a.b.c.d[1] }}" + - result: "{{ a|ansible.utils.get_path('b[\"c\"]') }}" + expected: "{{ a.b.c }}" + - result: "{{ lookup('ansible.utils.get_path', vars, 'a') }}" + expected: "{{ a }}" + - result: "{{ lookup('ansible.utils.get_path', a, 'b') }}" + expected: "{{ a.b }}" + - result: "{{ lookup('ansible.utils.get_path', a, 'b.c') }}" + expected: "{{ a.b.c }}" + - result: "{{ lookup('ansible.utils.get_path', a, 'b.c.d') }}" + expected: "{{ a.b.c.d }}" + - result: "{{ lookup('ansible.utils.get_path', a, 'b.c.d[0]') }}" + expected: "{{ a.b.c.d[0] }}" + - result: "{{ lookup('ansible.utils.get_path', a, 'b.c.d[1]') }}" + expected: "{{ a.b.c.d[1] }}" + - result: "{{ lookup('ansible.utils.get_path', a, 'b[\"c\"]') }}" + expected: "{{ a.b.c }}" + +- set_fact: + a: + b: + c: + d: + - 0 + +- name: Simple test filter and lookup w/ wantlist + assert: + that: "{{ item.result == item.expected }}" + loop: + - result: "{{ vars|ansible.utils.get_path('a.b.c.d[0]', wantlist=True) }}" + expected: + - "{{ a.b.c.d[0] }}" + - result: "{{ lookup('ansible.utils.get_path', vars, 'a.b.c.d[0]', wantlist=True) }}" + expected: + - "{{ a.b.c.d[0] }}" diff --git a/tests/integration/targets/get_path/tasks/main.yaml b/tests/integration/targets/get_path/tasks/main.yaml index 41043ff..c9cd63e 100644 --- a/tests/integration/targets/get_path/tasks/main.yaml +++ b/tests/integration/targets/get_path/tasks/main.yaml @@ -1,58 +1,12 @@ -- set_fact: - a: - b: - c: - d: - - 0 - - 1 +- name: Recursively find all test files + find: + file_type: file + paths: "{{ role_path }}/tasks/include" + recurse: yes + use_regex: yes + patterns: + - '^(?!_).+$' + register: found -- name: Simple test filter and lookup - assert: - that: "{{ item.result == item.expected }}" - loop: - - result: "{{ vars|ansible.utils.get_path('a') }}" - expected: "{{ a }}" - - result: "{{ a|ansible.utils.get_path('b') }}" - expected: "{{ a.b }}" - - result: "{{ a|ansible.utils.get_path('b.c') }}" - expected: "{{ a.b.c }}" - - result: "{{ a|ansible.utils.get_path('b.c.d') }}" - expected: "{{ a.b.c.d }}" - - result: "{{ a|ansible.utils.get_path('b.c.d[0]') }}" - expected: "{{ a.b.c.d[0] }}" - - result: "{{ a|ansible.utils.get_path('b.c.d[1]') }}" - expected: "{{ a.b.c.d[1] }}" - - result: "{{ a|ansible.utils.get_path('b[\"c\"]') }}" - expected: "{{ a.b.c }}" - - result: "{{ lookup('ansible.utils.get_path', vars, 'a') }}" - expected: "{{ a }}" - - result: "{{ lookup('ansible.utils.get_path', a, 'b') }}" - expected: "{{ a.b }}" - - result: "{{ lookup('ansible.utils.get_path', a, 'b.c') }}" - expected: "{{ a.b.c }}" - - result: "{{ lookup('ansible.utils.get_path', a, 'b.c.d') }}" - expected: "{{ a.b.c.d }}" - - result: "{{ lookup('ansible.utils.get_path', a, 'b.c.d[0]') }}" - expected: "{{ a.b.c.d[0] }}" - - result: "{{ lookup('ansible.utils.get_path', a, 'b.c.d[1]') }}" - expected: "{{ a.b.c.d[1] }}" - - result: "{{ lookup('ansible.utils.get_path', a, 'b[\"c\"]') }}" - expected: "{{ a.b.c }}" - -- set_fact: - a: - b: - c: - d: - - 0 - -- name: Simple test filter and lookup w/ wantlist - assert: - that: "{{ item.result == item.expected }}" - loop: - - result: "{{ vars|ansible.utils.get_path('a.b.c.d[0]', wantlist=True) }}" - expected: - - "{{ a.b.c.d[0] }}" - - result: "{{ lookup('ansible.utils.get_path', vars, 'a.b.c.d[0]', wantlist=True) }}" - expected: - - "{{ a.b.c.d[0] }}" +- include: "{{ item.path }}" + loop: "{{ found.files }}" diff --git a/tests/integration/targets/index_of/tasks/include/argspec.yaml b/tests/integration/targets/index_of/tasks/include/argspec.yaml new file mode 100644 index 0000000..939a559 --- /dev/null +++ b/tests/integration/targets/index_of/tasks/include/argspec.yaml @@ -0,0 +1,57 @@ +- set_fact: + complex: + a: + b: + c: + d: + - e0: 0 + e1: ansible + e2: True + - e0: 1 + e1: redhat + +- name: Check argspec validation with filter (not a list) + set_fact: + _result: "{{ complex|ansible.utils.index_of() }}" + ignore_errors: True + register: result + +- assert: + that: "{{ msg in result.msg }}" + vars: + msg: "cannot be converted to a list" + +- name: Check argspec validation with filter (missing params) + set_fact: + _result: "{{ complex.a.b.c.d|ansible.utils.index_of() }}" + ignore_errors: True + register: result + +- assert: + that: "{{ msg in result.msg }}" + vars: + msg: "missing required arguments: test" + +- name: Check argspec validation with lookup (not a list) + set_fact: + _result: "{{ lookup('ansible.utils.index_of', complex) }}" + ignore_errors: True + register: result + +- assert: + that: "{{ msg in result.msg }}" + vars: + msg: "cannot be converted to a list" + +- name: Check argspec validation with lookup (missing params) + set_fact: + _result: "{{ lookup('ansible.utils.index_of') }}" + ignore_errors: True + register: result + +- assert: + that: "{{ item in result.msg }}" + loop: + - "missing required arguments:" + - data + - test \ No newline at end of file diff --git a/tests/integration/targets/index_of/tasks/examples.yaml b/tests/integration/targets/index_of/tasks/include/examples.yaml similarity index 100% rename from tests/integration/targets/index_of/tasks/examples.yaml rename to tests/integration/targets/index_of/tasks/include/examples.yaml diff --git a/tests/integration/targets/index_of/tasks/simple.yaml b/tests/integration/targets/index_of/tasks/include/simple.yaml similarity index 100% rename from tests/integration/targets/index_of/tasks/simple.yaml rename to tests/integration/targets/index_of/tasks/include/simple.yaml diff --git a/tests/integration/targets/index_of/tasks/main.yaml b/tests/integration/targets/index_of/tasks/main.yaml index 1c4c8db..c9cd63e 100644 --- a/tests/integration/targets/index_of/tasks/main.yaml +++ b/tests/integration/targets/index_of/tasks/main.yaml @@ -1,2 +1,12 @@ -- include: simple.yaml -- include: examples.yaml \ No newline at end of file +- name: Recursively find all test files + find: + file_type: file + paths: "{{ role_path }}/tasks/include" + recurse: yes + use_regex: yes + patterns: + - '^(?!_).+$' + register: found + +- include: "{{ item.path }}" + loop: "{{ found.files }}" diff --git a/tests/integration/targets/to_paths/tasks/include/argspec.yaml b/tests/integration/targets/to_paths/tasks/include/argspec.yaml new file mode 100644 index 0000000..cf5d1e3 --- /dev/null +++ b/tests/integration/targets/to_paths/tasks/include/argspec.yaml @@ -0,0 +1,33 @@ +- set_fact: + a: + b: + c: + d: + - 0 + +- name: Check argspec validation with lookup + set_fact: + _result: "{{ a|ansible.utils.to_paths(wantlist=5) }}" + ignore_errors: True + register: result + +- debug: + var: result +- assert: + that: "{{ msg in result.msg }}" + vars: + msg: "'5' is not a valid boolean" + + +- name: Check argspec validation with lookup + set_fact: + _result: "{{ lookup('ansible.utils.to_paths') }}" + ignore_errors: True + register: result + +- debug: + var: result +- assert: + that: "{{ msg in result.msg }}" + vars: + msg: "missing required arguments: var" diff --git a/tests/integration/targets/to_paths/tasks/include/simple.yaml b/tests/integration/targets/to_paths/tasks/include/simple.yaml new file mode 100644 index 0000000..0c81f87 --- /dev/null +++ b/tests/integration/targets/to_paths/tasks/include/simple.yaml @@ -0,0 +1,52 @@ +- set_fact: + a: + b: + c: + d: + - 0 + - 1 + +- name: Test filter and lookup plugin, simple and prepend + assert: + that: "{{ item.result == item.expected }}" + loop: + - result: "{{ a|ansible.utils.to_paths }}" + expected: + b.c.d[0]: 0 + b.c.d[1]: 1 + - result: "{{ lookup('ansible.utils.to_paths', a) }}" + expected: + b.c.d[0]: 0 + b.c.d[1]: 1 + - result: "{{ a|ansible.utils.to_paths(prepend='a') }}" + expected: + a.b.c.d[0]: 0 + a.b.c.d[1]: 1 + - result: "{{ lookup('ansible.utils.to_paths', a, prepend='a') }}" + expected: + a.b.c.d[0]: 0 + a.b.c.d[1]: 1 + +- set_fact: + a: + b: + c: + d: + - 0 + +- name: Test filter and lookup plugin, wantlist and prepend + assert: + that: "{{ item.result == item.expected }}" + loop: + - result: "{{ a|ansible.utils.to_paths(wantlist=True) }}" + expected: + - b.c.d[0]: 0 + - result: "{{ lookup('ansible.utils.to_paths', a, wantlist=True) }}" + expected: + - b.c.d[0]: 0 + - result: "{{ a|ansible.utils.to_paths(wantlist=True, prepend='a') }}" + expected: + - a.b.c.d[0]: 0 + - result: "{{ lookup('ansible.utils.to_paths', a, wantlist=True, prepend='a') }}" + expected: + - a.b.c.d[0]: 0 diff --git a/tests/integration/targets/to_paths/tasks/main.yaml b/tests/integration/targets/to_paths/tasks/main.yaml index 0c81f87..c9cd63e 100644 --- a/tests/integration/targets/to_paths/tasks/main.yaml +++ b/tests/integration/targets/to_paths/tasks/main.yaml @@ -1,52 +1,12 @@ -- set_fact: - a: - b: - c: - d: - - 0 - - 1 +- name: Recursively find all test files + find: + file_type: file + paths: "{{ role_path }}/tasks/include" + recurse: yes + use_regex: yes + patterns: + - '^(?!_).+$' + register: found -- name: Test filter and lookup plugin, simple and prepend - assert: - that: "{{ item.result == item.expected }}" - loop: - - result: "{{ a|ansible.utils.to_paths }}" - expected: - b.c.d[0]: 0 - b.c.d[1]: 1 - - result: "{{ lookup('ansible.utils.to_paths', a) }}" - expected: - b.c.d[0]: 0 - b.c.d[1]: 1 - - result: "{{ a|ansible.utils.to_paths(prepend='a') }}" - expected: - a.b.c.d[0]: 0 - a.b.c.d[1]: 1 - - result: "{{ lookup('ansible.utils.to_paths', a, prepend='a') }}" - expected: - a.b.c.d[0]: 0 - a.b.c.d[1]: 1 - -- set_fact: - a: - b: - c: - d: - - 0 - -- name: Test filter and lookup plugin, wantlist and prepend - assert: - that: "{{ item.result == item.expected }}" - loop: - - result: "{{ a|ansible.utils.to_paths(wantlist=True) }}" - expected: - - b.c.d[0]: 0 - - result: "{{ lookup('ansible.utils.to_paths', a, wantlist=True) }}" - expected: - - b.c.d[0]: 0 - - result: "{{ a|ansible.utils.to_paths(wantlist=True, prepend='a') }}" - expected: - - a.b.c.d[0]: 0 - - result: "{{ lookup('ansible.utils.to_paths', a, wantlist=True, prepend='a') }}" - expected: - - a.b.c.d[0]: 0 +- include: "{{ item.path }}" + loop: "{{ found.files }}" diff --git a/tests/sanity/ignore-2.10.txt b/tests/sanity/ignore-2.10.txt index 9fa3d47..61c150e 100644 --- a/tests/sanity/ignore-2.10.txt +++ b/tests/sanity/ignore-2.10.txt @@ -1,4 +1,4 @@ plugins/module_utils/common/index_of.py pylint:ansible-bad-module-import # file's use is limited to filter and lookups on control node -plugins/filter/paths.py pep8:E501 # ignore line length for filter doc string w/ url -plugins/module_utils/common/path.py pylint:ansible-bad-module-import # file's use is limited to filter and lookups on control node plugins/filter/index_of.py pep8:E501 # ignore line length for filter doc string w/ url +plugins/filter/get_path.py pep8:E501 # ignore line length for filter doc string w/ url +plugins/filter/to_paths.py pep8:E501 # ignore line length for filter doc string w/ url diff --git a/tests/sanity/ignore-2.11.txt b/tests/sanity/ignore-2.11.txt index 9fa3d47..61c150e 100644 --- a/tests/sanity/ignore-2.11.txt +++ b/tests/sanity/ignore-2.11.txt @@ -1,4 +1,4 @@ plugins/module_utils/common/index_of.py pylint:ansible-bad-module-import # file's use is limited to filter and lookups on control node -plugins/filter/paths.py pep8:E501 # ignore line length for filter doc string w/ url -plugins/module_utils/common/path.py pylint:ansible-bad-module-import # file's use is limited to filter and lookups on control node plugins/filter/index_of.py pep8:E501 # ignore line length for filter doc string w/ url +plugins/filter/get_path.py pep8:E501 # ignore line length for filter doc string w/ url +plugins/filter/to_paths.py pep8:E501 # ignore line length for filter doc string w/ url diff --git a/tests/sanity/ignore-2.9.txt b/tests/sanity/ignore-2.9.txt index 9fa3d47..61c150e 100644 --- a/tests/sanity/ignore-2.9.txt +++ b/tests/sanity/ignore-2.9.txt @@ -1,4 +1,4 @@ plugins/module_utils/common/index_of.py pylint:ansible-bad-module-import # file's use is limited to filter and lookups on control node -plugins/filter/paths.py pep8:E501 # ignore line length for filter doc string w/ url -plugins/module_utils/common/path.py pylint:ansible-bad-module-import # file's use is limited to filter and lookups on control node plugins/filter/index_of.py pep8:E501 # ignore line length for filter doc string w/ url +plugins/filter/get_path.py pep8:E501 # ignore line length for filter doc string w/ url +plugins/filter/to_paths.py pep8:E501 # ignore line length for filter doc string w/ url diff --git a/tests/unit/module_utils/test_get_path.py b/tests/unit/module_utils/test_get_path.py new file mode 100644 index 0000000..89964d0 --- /dev/null +++ b/tests/unit/module_utils/test_get_path.py @@ -0,0 +1,51 @@ +# -*- coding: utf-8 -*- +# Copyright 2020 Red Hat +# GNU General Public License v3.0+ +# (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + + +from __future__ import absolute_import, division, print_function + +__metaclass__ = type + +import json +import heapq +import os +import unittest +from ansible_collections.ansible.utils.plugins.module_utils.common.get_path import ( + get_path, +) + + +from ansible.template import Templar + + +class TestGetPath(unittest.TestCase): + def setUp(self): + self._environment = Templar(loader=None).environment + + def test_get_path_pass(self): + var = {"a": {"b": {"c": {"d": [0, 1]}}}} + path = "a.b.c.d[0]" + result = get_path( + var, path, environment=self._environment, wantlist=False + ) + expected = "0" + self.assertEqual(result, expected) + + def test_get_path_pass_wantlist(self): + var = {"a": {"b": {"c": {"d": [0, 1]}}}} + path = "a.b.c.d[0]" + result = get_path( + var, path, environment=self._environment, wantlist=True + ) + expected = ["0"] + self.assertEqual(result, expected) + + def test_get_path_fail(self): + var = {"a": {"b": {"c": {"d": [0, 1]}}}} + path = "a.b.e" + expected = "dict object' has no attribute 'e'" + with self.assertRaises(Exception) as exc: + get_path(var, path, environment=self._environment, wantlist=False) + self.assertIn(expected, str(exc.exception)) diff --git a/tests/unit/module_utils/test_index_of.py b/tests/unit/module_utils/test_index_of.py index 479bec5..81a5fcf 100644 --- a/tests/unit/module_utils/test_index_of.py +++ b/tests/unit/module_utils/test_index_of.py @@ -30,23 +30,6 @@ class TestIndexOfFilter(unittest.TestCase): index_of(obj, test, value, key, tests=self._tests) self.assertIn("the test '@@' was not found", str(exc.exception)) - def test_fail_not_a_list(self): - obj, test, value = True, "==", 1 - with self.assertRaises(Exception) as exc: - index_of(obj, test, value, tests=self._tests) - self.assertIn( - "a list is required, was passed a 'bool'", str(exc.exception) - ) - - def test_fail_wantlist_not_a_bool(self): - obj, test, value = [1, 2], "==", 1 - with self.assertRaises(Exception) as exc: - index_of(obj, test, value, wantlist=42, tests=self._tests) - self.assertIn( - "'wantlist' is required to be a bool, was passed a 'int'", - str(exc.exception), - ) - def test_fail_mixed_list(self): obj, test, value, key = [{"a": "b"}, True, 1, "a"], "==", "b", "a" with self.assertRaises(Exception) as exc: @@ -74,6 +57,7 @@ class TestIndexOfFilter(unittest.TestCase): # ([False], "not false", []), # ([False, 5], "boolean", 0), # ([0, False], "false", 1), + ([3, 4], "not even", 0), ([3, 4], "even", 1), ([3, 3], "even", []), ([3, 3, 3, 4], "odd", [0, 1, 2]), diff --git a/tests/unit/module_utils/test_path_utils.py b/tests/unit/module_utils/test_to_paths.py similarity index 55% rename from tests/unit/module_utils/test_path_utils.py rename to tests/unit/module_utils/test_to_paths.py index 52430e7..e600d32 100644 --- a/tests/unit/module_utils/test_path_utils.py +++ b/tests/unit/module_utils/test_to_paths.py @@ -12,57 +12,36 @@ import json import heapq import os import unittest -from ansible_collections.ansible.utils.plugins.module_utils.common.path import ( +from ansible_collections.ansible.utils.plugins.module_utils.common.get_path import ( get_path, +) +from ansible_collections.ansible.utils.plugins.module_utils.common.to_paths import ( to_paths, ) + from ansible.template import Templar -class TestPathUtils(unittest.TestCase): +class TestToPaths(unittest.TestCase): def setUp(self): self._environment = Templar(loader=None).environment - def test_get_path_pass(self): - var = {"a": {"b": {"c": {"d": [0, 1]}}}} - path = "a.b.c.d[0]" - result = get_path(var, path, environment=self._environment) - expected = "0" - self.assertEqual(result, expected) - - def test_get_path_pass_wantlist(self): - var = {"a": {"b": {"c": {"d": [0, 1]}}}} - path = "a.b.c.d[0]" - result = get_path( - var, path, environment=self._environment, wantlist=True - ) - expected = ["0"] - self.assertEqual(result, expected) - - def test_get_path_fail(self): - var = {"a": {"b": {"c": {"d": [0, 1]}}}} - path = "a.b.e" - expected = "dict object' has no attribute 'e'" - with self.assertRaises(Exception) as exc: - get_path(var, path, environment=self._environment) - self.assertIn(expected, str(exc.exception)) - def test_to_paths(self): var = {"a": {"b": {"c": {"d": [0, 1]}}}} expected = {"a.b.c.d[0]": 0, "a.b.c.d[1]": 1} - result = to_paths(var) + result = to_paths(var, prepend=None, wantlist=None) self.assertEqual(result, expected) def test_to_paths_wantlist(self): var = {"a": {"b": {"c": {"d": [0, 1]}}}} expected = [{"a.b.c.d[0]": 0, "a.b.c.d[1]": 1}] - result = to_paths(var, wantlist=True) + result = to_paths(var, prepend=None, wantlist=True) self.assertEqual(result, expected) def test_to_paths_special_char(self): var = {"a": {"b": {"c": {"Eth1/1": True}}}} expected = [{"a.b.c['Eth1/1']": True}] - result = to_paths(var, wantlist=True) + result = to_paths(var, prepend=None, wantlist=True) self.assertEqual(result, expected) def test_to_paths_prepend(self): @@ -71,13 +50,6 @@ class TestPathUtils(unittest.TestCase): result = to_paths(var, wantlist=True, prepend="var") self.assertEqual(result, expected) - def test_to_paths_prepend_fail(self): - var = {"a": {"b": {"c": {"d": [0, 1]}}}} - expected = "must be a string" - with self.assertRaises(Exception) as exc: - to_paths(var, wantlist=True, prepend=5) - self.assertIn(expected, str(exc.exception)) - def test_roundtrip_large(self): """Test the 1000 longest keys, otherwise this takes a _really_ long time""" big_json_path = os.path.join( @@ -86,8 +58,10 @@ class TestPathUtils(unittest.TestCase): with open(big_json_path) as fhand: big_json = fhand.read() var = json.loads(big_json) - paths = to_paths(var) + paths = to_paths(var, prepend=None, wantlist=None) to_tests = heapq.nlargest(1000, list(paths.keys()), key=len) for to_test in to_tests: - gotten = get_path(var, to_test, environment=self._environment) + gotten = get_path( + var, to_test, environment=self._environment, wantlist=False + ) self.assertEqual(gotten, paths[to_test])