From 2d016b366f8e96a0e9955697d98d14a236606475 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Wed, 2 Nov 2022 17:04:41 -0400 Subject: [PATCH 01/24] controller side argspec read docs to create argspec (allows for handling non python modules) no_log? add property for marking options as such restrictions? handle the required_if/etc functions attributes? handles check/diff mode and more --- lib/ansible/parsing/docs_to_spec.py | 103 ++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 lib/ansible/parsing/docs_to_spec.py diff --git a/lib/ansible/parsing/docs_to_spec.py b/lib/ansible/parsing/docs_to_spec.py new file mode 100644 index 00000000000..a9fa236996d --- /dev/null +++ b/lib/ansible/parsing/docs_to_spec.py @@ -0,0 +1,103 @@ +ARGS_DOCS_KEYS = ("aliases", "choices", "default", "elements", "required", "type") + + +def option_to_spec(option): + + # use known common keys to copy data + spec = {name: option[name] for name in ARGS_DOCS_KEYS if name in option} + + # handle suboptions + if "suboptions" in option: + add_options(spec, option["suboptions"]) + + for sub in spec["options"].values(): + # check if we need to apply_defults + if "default" in sub or "fallback" in sub: + spec["apply_defaults"] = True + + #TODO: handle deprecations: + return spec + + +def restriction_to_spec(r): + + name = None + rest = None # normally a list except for 'required_by' + if 'required' in r: + if 'by' in r: + name='required_by' + rest = {r['required']: r['by']} + elif 'if' in r: + name='required_if' + rest = [r['if'], r['equals'], r['required']] + else: + for ding in ('exclusive', 'toghether', 'one_of'): + if ding in r: + if not isinstance(r[ding], Sequence): + raise AnsibleError('must be a list!') + rest = r[ding] + + if ding == 'exclusive': + name = 'mutually_exclusive' + else: + name = 'required_%s' % ding + break + else: + raise AnsibleError('unknown restriction!') + + return {name: rest} + + + # use known common keys to copy data +def add_options(argspec, options): + for n, o in sorted(options.items()): + argspec[n] = option_to_spec(o) + + +def add_restrictions(restrict_spec, restrictions): + for r in restrictions: + restrict_spec.append(restriction_to_spec(r)) + + +#argspec = {} +#add_options(argspec, doc['docs']['options']) +# +#restrictions = {} +#add_restrictions(restrictions, doc['docs']['restrictions']) +# +#attributes = {} +#add_attributes(attributes, doc['ATTRIBUTES']) + +''' +options: + ... +notes: + ... +requirements: + ... +restrictions: + # mutually_exclusive + - description: You cannot use 'a' and 'b' at the same time + exclusive: a, b + - description: You cannot use 'c' and 'x' at the same time + exclusive: c, x + + # required_toghether + - description: 'a' and 'b' required toghether + toghether: a, b + + # required_one_of + - description: at least one of a or b is required + one_of: a, b + + # required_if + - description: if x is set to y, a,b and c are required + required: [a,b,c] + if: x + equals: y + + # required_by + - required: x + description: x is required if b or c are set + by: [b,c] +''' From 0419d764f75f2275b49ebab028b4d24594e57f71 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Wed, 2 Nov 2022 17:06:49 -0400 Subject: [PATCH 02/24] nolog# --- lib/ansible/parsing/docs_to_spec.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ansible/parsing/docs_to_spec.py b/lib/ansible/parsing/docs_to_spec.py index a9fa236996d..4b1e348fb5c 100644 --- a/lib/ansible/parsing/docs_to_spec.py +++ b/lib/ansible/parsing/docs_to_spec.py @@ -1,4 +1,4 @@ -ARGS_DOCS_KEYS = ("aliases", "choices", "default", "elements", "required", "type") +ARGS_DOCS_KEYS = ("aliases", "choices", "default", "elements", "no_log", "required", "type") def option_to_spec(option): From 8802d6bd5e3c5251f5ba02737478b76c4e7cea04 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Fri, 18 Nov 2022 16:42:47 -0500 Subject: [PATCH 03/24] move so modules can use --- lib/ansible/{ => module_utils}/parsing/docs_to_spec.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename lib/ansible/{ => module_utils}/parsing/docs_to_spec.py (100%) diff --git a/lib/ansible/parsing/docs_to_spec.py b/lib/ansible/module_utils/parsing/docs_to_spec.py similarity index 100% rename from lib/ansible/parsing/docs_to_spec.py rename to lib/ansible/module_utils/parsing/docs_to_spec.py From 74aeebdf1f964ceb3964801d3b9733798b84ccec Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Wed, 23 Nov 2022 21:19:23 -0500 Subject: [PATCH 04/24] refining --- .../module_utils/parsing/docs_to_spec.py | 97 ++++++++++++++----- 1 file changed, 71 insertions(+), 26 deletions(-) diff --git a/lib/ansible/module_utils/parsing/docs_to_spec.py b/lib/ansible/module_utils/parsing/docs_to_spec.py index 4b1e348fb5c..c4278d280b5 100644 --- a/lib/ansible/module_utils/parsing/docs_to_spec.py +++ b/lib/ansible/module_utils/parsing/docs_to_spec.py @@ -1,41 +1,62 @@ +# Copyright: 2017, Ansible Project +# Simplified BSD License (see licenses/simplified_bsd.txt or https://opensource.org/licenses/BSD-2-Clause ) + +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +from collections.abc import Sequence + +from ansible.module_utils.six import string_types +# from ansible.module_utils._text import to_text + ARGS_DOCS_KEYS = ("aliases", "choices", "default", "elements", "no_log", "required", "type") -def option_to_spec(option): +def option_to_spec(option, deprecate=None): # use known common keys to copy data spec = {name: option[name] for name in ARGS_DOCS_KEYS if name in option} # handle suboptions if "suboptions" in option: - add_options(spec, option["suboptions"]) + add_options_from_doc(spec, option["suboptions"], deprecate) for sub in spec["options"].values(): # check if we need to apply_defults if "default" in sub or "fallback" in sub: spec["apply_defaults"] = True - #TODO: handle deprecations: + # Use passed-in function to handle deprecations + if deprecate is not None and 'deprecated' in option: + deprecate(option['deprecated']) + return spec def restriction_to_spec(r): - + ''' use documentation to create parameter restrictions for args_spec ''' name = None - rest = None # normally a list except for 'required_by' + rest = None # normally a list except for 'required_by' if 'required' in r: if 'by' in r: - name='required_by' + name = 'required_by' rest = {r['required']: r['by']} elif 'if' in r: - name='required_if' + name = 'required_if' rest = [r['if'], r['equals'], r['required']] else: for ding in ('exclusive', 'toghether', 'one_of'): if ding in r: - if not isinstance(r[ding], Sequence): - raise AnsibleError('must be a list!') - rest = r[ding] + + if isinstance(r[ding], string_types): + rest = r[ding].spit(',') + elif not isinstance(r[ding], Sequence): + raise TypeError('must be a list!') + else: + rest = r[ding] + + if len(rest) < 2: + raise TypeError('must have multiple elements') if ding == 'exclusive': name = 'mutually_exclusive' @@ -43,34 +64,58 @@ def restriction_to_spec(r): name = 'required_%s' % ding break else: - raise AnsibleError('unknown restriction!') - + raise Exception('unknown restriction!') return {name: rest} - # use known common keys to copy data -def add_options(argspec, options): +def add_options_from_doc(argspec, options, deprecate=None): + ''' Add option doc entries into argspec ''' for n, o in sorted(options.items()): - argspec[n] = option_to_spec(o) + argspec[n] = option_to_spec(o, deprecate) + +def get_options_from_doc(options, deprecate=None): + ''' Add option doc entries into argspec ''' + argspec = {} + add_options_from_doc(argspec, options, deprecate) + return argspec -def add_restrictions(restrict_spec, restrictions): + +def add_restrictions_from_doc(restrict_args, restrictions): + ''' add restriction doc entries into argspec ''' for r in restrictions: - restrict_spec.append(restriction_to_spec(r)) + restrict_args.append(restriction_to_spec(r)) + +def get_restrictions_from_doc(restrictions): + ''' add restriction doc entries into argspec ''' + reargs = {} + add_restrictions_from_doc(reargs, restrictions) + return reargs -#argspec = {} -#add_options(argspec, doc['docs']['options']) -# -#restrictions = {} -#add_restrictions(restrictions, doc['docs']['restrictions']) -# -#attributes = {} -#add_attributes(attributes, doc['ATTRIBUTES']) ''' +# example usage: + + argpsec = get_options_from_docs(doc.get('options', {})) + restrictions = get_restrictions_from_doc(doc.get('restrictions', {})) + validator = ModuleArgumentSpecValidator(argspec, **restrictions) + validation_result = validator.validate(params) + + final_params = validation_result.validated_parameters + + no_log_values = validation_result._no_log_values + aliases = validation_result._aliases + +''' + +# TODO: attributes = {} +# add_attributes(attributes, doc['ATTRIBUTES']) + +''' +# example of DOCUMENTATION with requirements: options: - ... + ...jkk notes: ... requirements: From 3e99025b4313158efa53bdfd31328a8be9b3149d Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Mon, 28 Nov 2022 15:06:39 -0500 Subject: [PATCH 05/24] move module finding to it's own function --- lib/ansible/plugins/action/__init__.py | 68 ++++++++++++++++++++++++-- 1 file changed, 65 insertions(+), 3 deletions(-) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index e296398622b..7df72b26acc 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -47,6 +47,7 @@ if t.TYPE_CHECKING: from ansible.playbook.play_context import PlayContext from ansible.playbook.task import Task from ansible.plugins.connection import ConnectionBase + from ansible.plugins.loader import PluginLoadContext from ansible.template import Templar @@ -253,6 +254,48 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): return True return False + def _get_module(self, module_name: str) -> PluginLoadContext: + + if module_name not in self._found: + split_module_name = module_name.split('.') + collection_name = '.'.join(split_module_name[0:2]) if len(split_module_name) > 2 else '' + leaf_module_name = resource_from_fqcr(module_name) + + # Search module path(s) for named module. + for mod_type in self._connection.module_implementation_preferences: + # Check to determine if PowerShell modules are supported, and apply + # some fixes (hacks) to module name + args. + if mod_type == '.ps1': + # FIXME: This should be temporary and moved to an exec subsystem plugin where we can define the mapping + # for each subsystem. + win_collection = 'ansible.windows' + rewrite_collection_names = ['ansible.builtin', 'ansible.legacy', ''] + # async_status, win_stat, win_file, win_copy, and win_ping are not just like their + # python counterparts but they are compatible enough for our + # internal usage + # NB: we only rewrite the module if it's not being called by the user (eg, an action calling something else) + # and if it's unqualified or FQ to a builtin + if leaf_module_name in ('stat', 'file', 'copy', 'ping') and \ + collection_name in rewrite_collection_names and self._task.action != module_name: + module_name = '%s.win_%s' % (win_collection, leaf_module_name) + elif leaf_module_name == 'async_status' and collection_name in rewrite_collection_names: + module_name = '%s.%s' % (win_collection, leaf_module_name) + + result = self._shared_loader_obj.module_loader.find_plugin_with_context(module_name, mod_type, collection_list=self._task.collections) + if result.resolved: + self._found[module_name] = result + break + else: + if result.redirect_list and len(result.redirect_list) > 1: + # take the last one in the redirect list, we may have successfully jumped through N other redirects + target_module_name = result.redirect_list[-1] + raise AnsibleError("The module {0} was redirected to {1}, which could not be loaded.".format(module_name, target_module_name)) + + else: # This is a for-else: http://bit.ly/1ElPkyg + raise AnsibleError("The module %s was not found in configured module paths" % (module_name)) + + return self._found[module_name] + def _configure_module(self, module_name, module_args, task_vars) -> tuple[_BuiltModule, str]: """ Handles the loading and templating of the module code through the @@ -263,9 +306,9 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): else: use_vars = task_vars - split_module_name = module_name.split('.') - collection_name = '.'.join(split_module_name[0:2]) if len(split_module_name) > 2 else '' - leaf_module_name = resource_from_fqcr(module_name) + # get module path and unquote args if needed + result = self._get_module(module_name, module_args) + module_path = result.plugin_resolved_path # Search module path(s) for named module. for mod_type in self._connection.module_implementation_preferences: @@ -302,6 +345,15 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): else: # This is a for-else: http://bit.ly/1ElPkyg raise AnsibleError("The module %s was not found in configured module paths" % (module_name)) + # TODO: move this tweak down to the modules, (platform: windows attribute?), not extensible here + # Remove extra quotes surrounding path parameters before sending to module. + if resource_from_fqcr(module_name) in ['win_stat', 'win_file', 'win_copy', 'slurp'] and \ + module_args and \ + hasattr(self._connection._shell, '_unquote'): + for key in ('src', 'dest', 'path'): + if key in module_args: + module_args[key] = self._connection._shell._unquote(module_args[key]) + # insert shared code and arguments into the module final_environment: dict[str, t.Any] = {} self._compute_environment_string(final_environment) @@ -347,6 +399,16 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): return module_bits, module_path + def _get_argspec_from_docs(self, module_name): + resolved = self._get_module(module_name) + argspec = '' + resolved # TODO it + return argspec + + def _validate_args(self, module_name, module_args): + argspec = self._get_argspec_from_docs(module_name) + validate(argpsec) + # call validation + def _compute_environment_string(self, raw_environment_out=None): """ Builds the environment string to be used when executing the remote task. From 9756e7c1772f893e898ac39567abaecc0bdec6b6 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Fri, 13 Jan 2023 13:45:19 -0500 Subject: [PATCH 06/24] added TODO to the meat of it --- lib/ansible/plugins/action/__init__.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 7df72b26acc..d8390c0bce0 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -401,13 +401,14 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): def _get_argspec_from_docs(self, module_name): resolved = self._get_module(module_name) - argspec = '' + resolved # TODO it + argspec = '' + resolved # TODO: actuall call build function return argspec def _validate_args(self, module_name, module_args): argspec = self._get_argspec_from_docs(module_name) - validate(argpsec) + # call validation + #TODO: validate(argpsec, module_args) def _compute_environment_string(self, raw_environment_out=None): """ From 33c997128f77251fa443b743c628a38afd7f97ef Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Thu, 20 Apr 2023 13:06:58 -0400 Subject: [PATCH 07/24] add validation parts --- .../module_utils/parsing/docs_to_spec.py | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/lib/ansible/module_utils/parsing/docs_to_spec.py b/lib/ansible/module_utils/parsing/docs_to_spec.py index c4278d280b5..c0884ffe4c2 100644 --- a/lib/ansible/module_utils/parsing/docs_to_spec.py +++ b/lib/ansible/module_utils/parsing/docs_to_spec.py @@ -6,6 +6,7 @@ __metaclass__ = type from collections.abc import Sequence +from ansible.module_utils.common.arg_spec import ArgumentSpecValidator from ansible.module_utils.six import string_types # from ansible.module_utils._text import to_text @@ -94,18 +95,35 @@ def get_restrictions_from_doc(restrictions): return reargs +def validate_spec(spec, restrictions, task_args): + + validator = ArgumentSpecValidator(spec, **restrictions) + return validator.validate(task_args) + + +def validate_spec_from_plugin(plugin): + # take plugin object (name?), get docs and process with above + pass + + ''' # example usage: + #prep argpsec = get_options_from_docs(doc.get('options', {})) restrictions = get_restrictions_from_doc(doc.get('restrictions', {})) - validator = ModuleArgumentSpecValidator(argspec, **restrictions) - validation_result = validator.validate(params) - final_params = validation_result.validated_parameters + # do + validated = validate_spec(argspec, restrictions, task_params) + + # error handle + if valided.error_messages: + raise ActionFail({'msg': 'Validation of arguments failed:\n%s' % '\n'.join(validated.error_messages), 'argument_errors': validatec.error_messages}) - no_log_values = validation_result._no_log_values - aliases = validation_result._aliases + # get info + final_params = valided.validated_parameters + no_log_values = valided._no_log_values + aliases = validated._aliases ''' From 582a97be5f90288a9814b1c7ddd2928a6c8d7253 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Mon, 24 Apr 2023 10:44:05 -0400 Subject: [PATCH 08/24] update cright --- lib/ansible/module_utils/parsing/docs_to_spec.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ansible/module_utils/parsing/docs_to_spec.py b/lib/ansible/module_utils/parsing/docs_to_spec.py index c0884ffe4c2..616f779fcba 100644 --- a/lib/ansible/module_utils/parsing/docs_to_spec.py +++ b/lib/ansible/module_utils/parsing/docs_to_spec.py @@ -1,4 +1,4 @@ -# Copyright: 2017, Ansible Project +# Copyright: 2023, Ansible Project # Simplified BSD License (see licenses/simplified_bsd.txt or https://opensource.org/licenses/BSD-2-Clause ) from __future__ import (absolute_import, division, print_function) From beb967ee3e461f102a114240915b6fc19d292eae Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Mon, 24 Apr 2023 10:45:23 -0400 Subject: [PATCH 09/24] fix deprecation passthrough --- lib/ansible/module_utils/parsing/docs_to_spec.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ansible/module_utils/parsing/docs_to_spec.py b/lib/ansible/module_utils/parsing/docs_to_spec.py index 616f779fcba..6d2dd432b1a 100644 --- a/lib/ansible/module_utils/parsing/docs_to_spec.py +++ b/lib/ansible/module_utils/parsing/docs_to_spec.py @@ -29,7 +29,7 @@ def option_to_spec(option, deprecate=None): # Use passed-in function to handle deprecations if deprecate is not None and 'deprecated' in option: - deprecate(option['deprecated']) + deprecate(**option['deprecated']) return spec From 750a3fda4b3bfe274d7456dc964a74a0885bd30b Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Mon, 24 Apr 2023 14:28:34 -0400 Subject: [PATCH 10/24] Update lib/ansible/module_utils/parsing/docs_to_spec.py Co-authored-by: Matt Clay --- lib/ansible/module_utils/parsing/docs_to_spec.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ansible/module_utils/parsing/docs_to_spec.py b/lib/ansible/module_utils/parsing/docs_to_spec.py index 6d2dd432b1a..cd718abb9d1 100644 --- a/lib/ansible/module_utils/parsing/docs_to_spec.py +++ b/lib/ansible/module_utils/parsing/docs_to_spec.py @@ -4,7 +4,7 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -from collections.abc import Sequence +from ansible.module_utils.common._collections_compat import Sequence from ansible.module_utils.common.arg_spec import ArgumentSpecValidator from ansible.module_utils.six import string_types From 2c6412b3ccac87e29cdd13298a6f9d8a78361bd8 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Mon, 24 Apr 2023 14:29:48 -0400 Subject: [PATCH 11/24] todo or not todo --- lib/ansible/plugins/action/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index d8390c0bce0..789df550ebf 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -408,7 +408,7 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): argspec = self._get_argspec_from_docs(module_name) # call validation - #TODO: validate(argpsec, module_args) + # TODO: validate(argpsec, module_args) def _compute_environment_string(self, raw_environment_out=None): """ From 71ad5c6be7c73e3ec1cfa8a1d22752b92a1f6146 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Thu, 25 Jan 2024 11:06:30 -0500 Subject: [PATCH 12/24] avoid noise --- lib/ansible/module_utils/parsing/docs_to_spec.py | 5 ++--- lib/ansible/plugins/action/__init__.py | 2 -- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/ansible/module_utils/parsing/docs_to_spec.py b/lib/ansible/module_utils/parsing/docs_to_spec.py index cd718abb9d1..ad567fec4a2 100644 --- a/lib/ansible/module_utils/parsing/docs_to_spec.py +++ b/lib/ansible/module_utils/parsing/docs_to_spec.py @@ -105,8 +105,7 @@ def validate_spec_from_plugin(plugin): # take plugin object (name?), get docs and process with above pass - -''' +e1 = ''' # example usage: #prep @@ -130,7 +129,7 @@ def validate_spec_from_plugin(plugin): # TODO: attributes = {} # add_attributes(attributes, doc['ATTRIBUTES']) -''' +e2 = ''' # example of DOCUMENTATION with requirements: options: ...jkk diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 789df550ebf..50b1238f4a7 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -406,8 +406,6 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): def _validate_args(self, module_name, module_args): argspec = self._get_argspec_from_docs(module_name) - - # call validation # TODO: validate(argpsec, module_args) def _compute_environment_string(self, raw_environment_out=None): From a53fe0389a319fa4bc7dddad7716a664bbbdece6 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Thu, 25 Jan 2024 11:34:12 -0500 Subject: [PATCH 13/24] fix imporst --- lib/ansible/module_utils/parsing/docs_to_spec.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/ansible/module_utils/parsing/docs_to_spec.py b/lib/ansible/module_utils/parsing/docs_to_spec.py index ad567fec4a2..ed0ec546696 100644 --- a/lib/ansible/module_utils/parsing/docs_to_spec.py +++ b/lib/ansible/module_utils/parsing/docs_to_spec.py @@ -1,14 +1,11 @@ # Copyright: 2023, Ansible Project # Simplified BSD License (see licenses/simplified_bsd.txt or https://opensource.org/licenses/BSD-2-Clause ) -from __future__ import (absolute_import, division, print_function) -__metaclass__ = type - -from ansible.module_utils.common._collections_compat import Sequence +from __future__ import annotations from ansible.module_utils.common.arg_spec import ArgumentSpecValidator +from ansible.module_utils.common._collections_compat import Sequence from ansible.module_utils.six import string_types -# from ansible.module_utils._text import to_text ARGS_DOCS_KEYS = ("aliases", "choices", "default", "elements", "no_log", "required", "type") From 397fb1f3c8ff18d6cde3c0f1d21107e80026dfe7 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Thu, 25 Jan 2024 11:46:00 -0500 Subject: [PATCH 14/24] clog --- changelogs/fragments/doctospec.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelogs/fragments/doctospec.yml diff --git a/changelogs/fragments/doctospec.yml b/changelogs/fragments/doctospec.yml new file mode 100644 index 00000000000..4929385484c --- /dev/null +++ b/changelogs/fragments/doctospec.yml @@ -0,0 +1,4 @@ +minor_changes: + - Expanded documentation with the ability to specify paramter relations, like required_if, mutually_exclusive, required_toghether, etc + - Added function that will take a module documentation and transform into an argspec + - Action plugins base now can use doc_to_spec functino to validate parameters on controller (no need to round trip) From 7823f80c842fb699aab3341cc963fac6ba871f2b Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Mon, 29 Jan 2024 11:52:09 -0500 Subject: [PATCH 15/24] s --- lib/ansible/module_utils/parsing/docs_to_spec.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/ansible/module_utils/parsing/docs_to_spec.py b/lib/ansible/module_utils/parsing/docs_to_spec.py index ed0ec546696..143e0cbe41c 100644 --- a/lib/ansible/module_utils/parsing/docs_to_spec.py +++ b/lib/ansible/module_utils/parsing/docs_to_spec.py @@ -102,6 +102,7 @@ def validate_spec_from_plugin(plugin): # take plugin object (name?), get docs and process with above pass + e1 = ''' # example usage: From f4688029147d5b5cae83073e83f07357a6659c14 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Tue, 26 Mar 2024 10:03:30 -0400 Subject: [PATCH 16/24] fix invocation --- lib/ansible/plugins/action/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 50b1238f4a7..0504e39c2c3 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -102,6 +102,8 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): # Backwards compat: self._display isn't really needed, just import the global display and use that. self._display = display + self._found = {} + @abstractmethod def run(self, tmp: str | None = None, task_vars: dict[str, t.Any] | None = None) -> dict[str, t.Any]: """ Action Plugins should implement this method to perform their @@ -254,7 +256,7 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): return True return False - def _get_module(self, module_name: str) -> PluginLoadContext: + def _get_module(self, module_name: str, module_args: dict | None = None) -> PluginLoadContext: if module_name not in self._found: split_module_name = module_name.split('.') From 027e8c3934d76f078fa417ef5a85f0520d65a05b Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Tue, 8 Oct 2024 10:07:38 -0400 Subject: [PATCH 17/24] Apply suggestions from code review Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com> --- lib/ansible/module_utils/parsing/docs_to_spec.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/ansible/module_utils/parsing/docs_to_spec.py b/lib/ansible/module_utils/parsing/docs_to_spec.py index 143e0cbe41c..3c7b4a19dd9 100644 --- a/lib/ansible/module_utils/parsing/docs_to_spec.py +++ b/lib/ansible/module_utils/parsing/docs_to_spec.py @@ -43,7 +43,7 @@ def restriction_to_spec(r): name = 'required_if' rest = [r['if'], r['equals'], r['required']] else: - for ding in ('exclusive', 'toghether', 'one_of'): + for ding in ('exclusive', 'together', 'one_of'): if ding in r: if isinstance(r[ding], string_types): @@ -142,9 +142,9 @@ restrictions: - description: You cannot use 'c' and 'x' at the same time exclusive: c, x - # required_toghether - - description: 'a' and 'b' required toghether - toghether: a, b + # required_together + - description: 'a' and 'b' required together + together: a, b # required_one_of - description: at least one of a or b is required From 84175af41c3e429fbf2390aa9f521ab5d5f6e403 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Tue, 8 Oct 2024 16:07:30 -0400 Subject: [PATCH 18/24] fix to standards --- .../module_utils/parsing/docs_to_spec.py | 32 +++++++++++-------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/lib/ansible/module_utils/parsing/docs_to_spec.py b/lib/ansible/module_utils/parsing/docs_to_spec.py index 3c7b4a19dd9..46636f99f5b 100644 --- a/lib/ansible/module_utils/parsing/docs_to_spec.py +++ b/lib/ansible/module_utils/parsing/docs_to_spec.py @@ -1,5 +1,5 @@ -# Copyright: 2023, Ansible Project -# Simplified BSD License (see licenses/simplified_bsd.txt or https://opensource.org/licenses/BSD-2-Clause ) +# Copyright: Contributors to the Ansible project +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) from __future__ import annotations @@ -7,10 +7,15 @@ from ansible.module_utils.common.arg_spec import ArgumentSpecValidator from ansible.module_utils.common._collections_compat import Sequence from ansible.module_utils.six import string_types +""" +Utilities to create module/plugin specs from their documentation. +""" + ARGS_DOCS_KEYS = ("aliases", "choices", "default", "elements", "no_log", "required", "type") -def option_to_spec(option, deprecate=None): +def option_to_spec(option, deprecate=None) -> dict: + """ convert from doc option entry to spec arg definition """ # use known common keys to copy data spec = {name: option[name] for name in ARGS_DOCS_KEYS if name in option} @@ -31,8 +36,9 @@ def option_to_spec(option, deprecate=None): return spec -def restriction_to_spec(r): - ''' use documentation to create parameter restrictions for args_spec ''' +def restriction_to_spec(r) -> dict: + """ read documented restriction and create spec restriction """ + name = None rest = None # normally a list except for 'required_by' if 'required' in r: @@ -67,26 +73,26 @@ def restriction_to_spec(r): def add_options_from_doc(argspec, options, deprecate=None): - ''' Add option doc entries into argspec ''' + """ Add option doc entries into argspec """ for n, o in sorted(options.items()): argspec[n] = option_to_spec(o, deprecate) def get_options_from_doc(options, deprecate=None): - ''' Add option doc entries into argspec ''' + """ Add option doc entries into argspec """ argspec = {} add_options_from_doc(argspec, options, deprecate) return argspec def add_restrictions_from_doc(restrict_args, restrictions): - ''' add restriction doc entries into argspec ''' + """ add restriction doc entries into argspec """ for r in restrictions: restrict_args.append(restriction_to_spec(r)) def get_restrictions_from_doc(restrictions): - ''' add restriction doc entries into argspec ''' + """ add restriction doc entries into argspec """ reargs = {} add_restrictions_from_doc(reargs, restrictions) return reargs @@ -103,7 +109,7 @@ def validate_spec_from_plugin(plugin): pass -e1 = ''' +e1 = """ # example usage: #prep @@ -122,12 +128,12 @@ e1 = ''' no_log_values = valided._no_log_values aliases = validated._aliases -''' +""" # TODO: attributes = {} # add_attributes(attributes, doc['ATTRIBUTES']) -e2 = ''' +e2 = """ # example of DOCUMENTATION with requirements: options: ...jkk @@ -160,4 +166,4 @@ restrictions: - required: x description: x is required if b or c are set by: [b,c] -''' +""" From 848f6f8bf2452ccfed29635c7f7c6807c8316c3b Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Thu, 10 Oct 2024 11:47:01 -0400 Subject: [PATCH 19/24] relocate docs --- .../module_utils/parsing/docs_to_spec.py | 114 +++++++++--------- 1 file changed, 54 insertions(+), 60 deletions(-) diff --git a/lib/ansible/module_utils/parsing/docs_to_spec.py b/lib/ansible/module_utils/parsing/docs_to_spec.py index 46636f99f5b..05742258a7d 100644 --- a/lib/ansible/module_utils/parsing/docs_to_spec.py +++ b/lib/ansible/module_utils/parsing/docs_to_spec.py @@ -9,6 +9,60 @@ from ansible.module_utils.six import string_types """ Utilities to create module/plugin specs from their documentation. + +# example usage: + + #prep + argpsec = get_options_from_docs(doc.get('options', {})) + restrictions = get_restrictions_from_doc(doc.get('restrictions', {})) + + # do + validated = validate_spec(argspec, restrictions, task_params) + + # error handle + if valided.error_messages: + raise ActionFail({'msg': 'Validation of arguments failed:\n%s' % '\n'.join(validated.error_messages), 'argument_errors': validatec.error_messages}) + + # get info + final_params = valided.validated_parameters + no_log_values = valided._no_log_values + aliases = validated._aliases + +''' +example of DOCUMENTATION with requirements: + + options: + ...jkk + notes: + ... + requirements: + ... + restrictions: + # mutually_exclusive + - description: You cannot use 'a' and 'b' at the same time + exclusive: a, b + - description: You cannot use 'c' and 'x' at the same time + exclusive: c, x + + # required_together + - description: 'a' and 'b' required together + together: a, b + + # required_one_of + - description: at least one of a or b is required + one_of: a, b + + # required_if + - description: if x is set to y, a,b and c are required + required: [a,b,c] + if: x + equals: y + + # required_by + - required: x + description: x is required if b or c are set + by: [b,c] +''' """ ARGS_DOCS_KEYS = ("aliases", "choices", "default", "elements", "no_log", "required", "type") @@ -107,63 +161,3 @@ def validate_spec(spec, restrictions, task_args): def validate_spec_from_plugin(plugin): # take plugin object (name?), get docs and process with above pass - - -e1 = """ -# example usage: - - #prep - argpsec = get_options_from_docs(doc.get('options', {})) - restrictions = get_restrictions_from_doc(doc.get('restrictions', {})) - - # do - validated = validate_spec(argspec, restrictions, task_params) - - # error handle - if valided.error_messages: - raise ActionFail({'msg': 'Validation of arguments failed:\n%s' % '\n'.join(validated.error_messages), 'argument_errors': validatec.error_messages}) - - # get info - final_params = valided.validated_parameters - no_log_values = valided._no_log_values - aliases = validated._aliases - -""" - -# TODO: attributes = {} -# add_attributes(attributes, doc['ATTRIBUTES']) - -e2 = """ -# example of DOCUMENTATION with requirements: -options: - ...jkk -notes: - ... -requirements: - ... -restrictions: - # mutually_exclusive - - description: You cannot use 'a' and 'b' at the same time - exclusive: a, b - - description: You cannot use 'c' and 'x' at the same time - exclusive: c, x - - # required_together - - description: 'a' and 'b' required together - together: a, b - - # required_one_of - - description: at least one of a or b is required - one_of: a, b - - # required_if - - description: if x is set to y, a,b and c are required - required: [a,b,c] - if: x - equals: y - - # required_by - - required: x - description: x is required if b or c are set - by: [b,c] -""" From a3c4d97d37fba7f978f28b7f92d68f054dff983c Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Mon, 14 Oct 2024 17:47:32 -0400 Subject: [PATCH 20/24] st it --- lib/ansible/module_utils/parsing/docs_to_spec.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ansible/module_utils/parsing/docs_to_spec.py b/lib/ansible/module_utils/parsing/docs_to_spec.py index 05742258a7d..7941fd448cf 100644 --- a/lib/ansible/module_utils/parsing/docs_to_spec.py +++ b/lib/ansible/module_utils/parsing/docs_to_spec.py @@ -90,7 +90,7 @@ def option_to_spec(option, deprecate=None) -> dict: return spec -def restriction_to_spec(r) -> dict: +def restriction_to_spec(r) -> list[list[str]] | None: """ read documented restriction and create spec restriction """ name = None @@ -123,7 +123,7 @@ def restriction_to_spec(r) -> dict: break else: raise Exception('unknown restriction!') - return {name: rest} + return rest def add_options_from_doc(argspec, options, deprecate=None): From 694d589767f4643c2db200e5fed1ad2c175ac217 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Fri, 25 Oct 2024 17:33:01 -0400 Subject: [PATCH 21/24] sldfk --- lib/ansible/module_utils/parsing/docs_to_spec.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/ansible/module_utils/parsing/docs_to_spec.py b/lib/ansible/module_utils/parsing/docs_to_spec.py index 7941fd448cf..3d0c9ab3c6a 100644 --- a/lib/ansible/module_utils/parsing/docs_to_spec.py +++ b/lib/ansible/module_utils/parsing/docs_to_spec.py @@ -1,12 +1,5 @@ # Copyright: Contributors to the Ansible project # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) - -from __future__ import annotations - -from ansible.module_utils.common.arg_spec import ArgumentSpecValidator -from ansible.module_utils.common._collections_compat import Sequence -from ansible.module_utils.six import string_types - """ Utilities to create module/plugin specs from their documentation. @@ -64,6 +57,12 @@ example of DOCUMENTATION with requirements: by: [b,c] ''' """ +from __future__ import annotations + +from ansible.module_utils.common.arg_spec import ArgumentSpecValidator +from ansible.module_utils.common._collections_compat import Sequence +from ansible.module_utils.six import string_types + ARGS_DOCS_KEYS = ("aliases", "choices", "default", "elements", "no_log", "required", "type") @@ -90,7 +89,7 @@ def option_to_spec(option, deprecate=None) -> dict: return spec -def restriction_to_spec(r) -> list[list[str]] | None: +def restriction_to_spec(r) -> t.Any: """ read documented restriction and create spec restriction """ name = None From f3028d5ccb16fd85d81615faa9053852fcb86ff8 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Tue, 3 Dec 2024 14:20:55 -0500 Subject: [PATCH 22/24] setup generic invocation todo: - needs to handle no_log, once it has argspec - update test to user friendly message - update module_utils/basic.cs to avoid creating by default and adjust msg --- lib/ansible/plugins/action/__init__.py | 25 +++++++++++++++++++--- lib/ansible/plugins/action/async_status.py | 4 ++++ lib/ansible/plugins/action/copy.py | 21 ++++++++---------- 3 files changed, 35 insertions(+), 15 deletions(-) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 0504e39c2c3..dc409407be2 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -104,6 +104,26 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): self._found = {} + def _ensure_invocation(self, result): + + need_invocation = bool(display.verbosity or C.DEFAULT_DEBUG) + if need_invocation and 'invocation' not in result: + result['invocation'] = {} + + if isinstance(result.get('invocation'), dict): + # we have invocation dict, otherwise it would be censored string already + # action plugins/modules might have popuilated + if self._task.no_log: + result['invocation'] = "CENSORED: no_log is set for this task" + else: + result["invocation"]['module_args'] = self._task.args.copy() + + # TODO: get no_log list from controller side argspec, example for copy + if result['invocation']['module_args'].get('content') is not None: + result['invocation']['module_args']['content'] = 'CENSORED: no_log is set for this parameter' + + return result + @abstractmethod def run(self, tmp: str | None = None, task_vars: dict[str, t.Any] | None = None) -> dict[str, t.Any]: """ Action Plugins should implement this method to perform their @@ -121,8 +141,7 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): * Module parameters. These are stored in self._task.args """ - # does not default to {'changed': False, 'failed': False}, as it used to break async - result: dict[str, t.Any] = {} + result: dict[str, t.Any] = {'changed': False, 'failed': False} if tmp is not None: display.warning('ActionModule.run() no longer honors the tmp parameter. Action' @@ -145,7 +164,7 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): if self._connection._shell.tmpdir is None and self._early_needs_tmp_path(): self._make_tmp_path() - return result + return self._ensure_invocation(result) def validate_argument_spec(self, argument_spec=None, mutually_exclusive=None, diff --git a/lib/ansible/plugins/action/async_status.py b/lib/ansible/plugins/action/async_status.py index 676fc9324ec..a7c02cee9ea 100644 --- a/lib/ansible/plugins/action/async_status.py +++ b/lib/ansible/plugins/action/async_status.py @@ -20,6 +20,10 @@ class ActionModule(ActionBase): results = super(ActionModule, self).run(tmp, task_vars) + # async works diff from other action plugins + for nope in ('changed', 'failed'): + del results[nope] + validation_result, new_module_args = self.validate_argument_spec( argument_spec={ 'jid': {'type': 'str', 'required': True}, diff --git a/lib/ansible/plugins/action/copy.py b/lib/ansible/plugins/action/copy.py index 89a6a8f1f95..7afb502c838 100644 --- a/lib/ansible/plugins/action/copy.py +++ b/lib/ansible/plugins/action/copy.py @@ -206,22 +206,19 @@ class ActionModule(ActionBase): # NOTE: adding invocation arguments here needs to be kept in sync with # any no_log specified in the argument_spec in the module. # This is not automatic. - # NOTE: do not add to this. This should be made a generic function for action plugins. - # This should also use the same argspec as the module instead of keeping it in sync. - if 'invocation' not in result: - if self._task.no_log: - result['invocation'] = "CENSORED: no_log is set" - else: - # NOTE: Should be removed in the future. For now keep this broken - # behaviour, have a look in the PR 51582 - result['invocation'] = self._task.args.copy() - result['invocation']['module_args'] = self._task.args.copy() + # NOTE: DO NOT ADD TO THIS. There is a generic function for action plugins. + if self._task.no_log: + result['invocation'] = "CENSORED: no_log is set" + else: + # NOTE: Should be removed in the future. For now keep this broken + # behaviour, have a look in the PR 51582, deprecate once DT is available + # TAGS: DT + result['invocation'] = self._task.args.copy() if isinstance(result['invocation'], dict): + # NOTE: also deprecate and remove once we have DT if 'content' in result['invocation']: result['invocation']['content'] = 'CENSORED: content is a no_log parameter' - if result['invocation'].get('module_args', {}).get('content') is not None: - result['invocation']['module_args']['content'] = 'VALUE_SPECIFIED_IN_NO_LOG_PARAMETER' return result From c9daa811a5ab63813aaa3fee5641725eaaffaaf9 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Sat, 28 Jun 2025 21:56:55 -0400 Subject: [PATCH 23/24] lint --- lib/ansible/module_utils/parsing/docs_to_spec.py | 4 +++- lib/ansible/plugins/action/__init__.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/ansible/module_utils/parsing/docs_to_spec.py b/lib/ansible/module_utils/parsing/docs_to_spec.py index 3d0c9ab3c6a..8b4084e0c68 100644 --- a/lib/ansible/module_utils/parsing/docs_to_spec.py +++ b/lib/ansible/module_utils/parsing/docs_to_spec.py @@ -59,6 +59,8 @@ example of DOCUMENTATION with requirements: """ from __future__ import annotations +import typing as _t + from ansible.module_utils.common.arg_spec import ArgumentSpecValidator from ansible.module_utils.common._collections_compat import Sequence from ansible.module_utils.six import string_types @@ -89,7 +91,7 @@ def option_to_spec(option, deprecate=None) -> dict: return spec -def restriction_to_spec(r) -> t.Any: +def restriction_to_spec(r): """ read documented restriction and create spec restriction """ name = None diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index dc409407be2..e79c455712b 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -141,7 +141,7 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): * Module parameters. These are stored in self._task.args """ - result: dict[str, t.Any] = {'changed': False, 'failed': False} + result: dict[str, t.Any] = {'changed': False, 'failed': False} if tmp is not None: display.warning('ActionModule.run() no longer honors the tmp parameter. Action' From 2027a465d533a913925c053b863d952ac93f081c Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Sat, 28 Jun 2025 22:07:33 -0400 Subject: [PATCH 24/24] typed --- lib/ansible/plugins/action/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index e79c455712b..ba7fd5173d2 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -102,7 +102,7 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin): # Backwards compat: self._display isn't really needed, just import the global display and use that. self._display = display - self._found = {} + self._found: dict[str, PluginLoadContext] = {} def _ensure_invocation(self, result):