diff --git a/lib/ansible/cli/galaxy.py b/lib/ansible/cli/galaxy.py index 07e652a2e95..a6f07eceadc 100755 --- a/lib/ansible/cli/galaxy.py +++ b/lib/ansible/cli/galaxy.py @@ -808,8 +808,6 @@ class GalaxyCLI(CLI): if "include" not in requirement: role = RoleRequirement.role_yaml_parse(requirement) display.vvv("found role %s in yaml file" % to_text(role)) - if "name" not in role and "src" not in role: - raise AnsibleError("Must specify name or src for role") return [GalaxyRole(self.galaxy, self.lazy_role_api, **role)] else: b_include_path = to_bytes(requirement["include"], errors="surrogate_or_strict") @@ -1279,9 +1277,9 @@ class GalaxyCLI(CLI): role_info.update(gr.metadata) req = RoleRequirement() - role_spec = req.role_yaml_parse({'role': role}) - if role_spec: - role_info.update(role_spec) + if ',' in role: + raise AnsibleError("Invalid old style role requirement: %s" % role) + role_info.update({'name': role, 'role': role}) data += self._display_role_info(role_info) diff --git a/lib/ansible/galaxy/role.py b/lib/ansible/galaxy/role.py index 50873c4ca82..bdc8f7d29e4 100644 --- a/lib/ansible/galaxy/role.py +++ b/lib/ansible/galaxy/role.py @@ -79,7 +79,10 @@ class GalaxyRole(object): META_REQUIREMENTS = (os.path.join('meta', 'requirements.yml'), os.path.join('meta', 'requirements.yaml')) ROLE_DIRS = ('defaults', 'files', 'handlers', 'meta', 'tasks', 'templates', 'vars', 'tests') - def __init__(self, galaxy, api, name, src=None, version=None, scm=None, path=None): + def __init__(self, galaxy, api, name=None, src=None, version=None, scm=None, path=None): + + if name is None: + AnsibleParserError("Must specify name or src for role") self._metadata = None self._metadata_dependencies = None diff --git a/lib/ansible/playbook/helpers.py b/lib/ansible/playbook/helpers.py index b908a0f9722..2ec48e4462f 100644 --- a/lib/ansible/playbook/helpers.py +++ b/lib/ansible/playbook/helpers.py @@ -303,7 +303,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h return task_list -def load_list_of_roles(ds, play, current_role_path=None, variable_manager=None, loader=None, collection_search_list=None): +def load_list_of_roles(ds, play, current_role_path=None, variable_manager=None, loader=None, collection_search_list=None, extra_spec=None): """ Loads and returns a list of RoleInclude objects from the ds list of role definitions :param ds: list of roles to load @@ -323,7 +323,7 @@ def load_list_of_roles(ds, play, current_role_path=None, variable_manager=None, roles = [] for role_def in ds: i = RoleInclude.load(role_def, play=play, current_role_path=current_role_path, variable_manager=variable_manager, - loader=loader, collection_list=collection_search_list) + loader=loader, collection_list=collection_search_list, extra_spec=extra_spec) roles.append(i) return roles diff --git a/lib/ansible/playbook/role/__init__.py b/lib/ansible/playbook/role/__init__.py index 04256898cee..0a0d7f8c07b 100644 --- a/lib/ansible/playbook/role/__init__.py +++ b/lib/ansible/playbook/role/__init__.py @@ -198,6 +198,7 @@ class Role(Base, Conditional, Taggable, CollectionSearch, Delegatable): self._role_path = role_include.get_role_path() self._role_collection = role_include._role_collection self._role_params = role_include.get_role_params() + self._internal_galaxy_spec = role_include._internal_galaxy_spec self._variable_manager = role_include.get_variable_manager() self._loader = role_include.get_loader() @@ -258,11 +259,18 @@ class Role(Base, Conditional, Taggable, CollectionSearch, Delegatable): if self._should_validate: role_argspecs = self._get_role_argspecs() - task_data = self._prepend_validation_task(task_data, role_argspecs) + validation_task_data = self._prepend_validation_task(None, role_argspecs) + validation_blocks = load_list_of_blocks( + validation_task_data, play=self._play, role=self, loader=self._loader, variable_manager=self._variable_manager + ) + for block in validation_blocks: + for task in block.block: + task.implicit = True + self._task_blocks = validation_blocks if task_data: try: - self._task_blocks = load_list_of_blocks(task_data, play=self._play, role=self, loader=self._loader, variable_manager=self._variable_manager) + self._task_blocks += load_list_of_blocks(task_data, play=self._play, role=self, loader=self._loader, variable_manager=self._variable_manager) except AssertionError as e: raise AnsibleParserError("The tasks/main.yml file for role '%s' must contain a list of tasks" % self._role_name, obj=task_data, orig_exc=e) @@ -353,6 +361,7 @@ class Role(Base, Conditional, Taggable, CollectionSearch, Delegatable): # Pass only the 'options' portion of the arg spec to the module. 'argument_spec': argument_spec.get('options', {}), 'provided_arguments': self._role_params, + 'optional_galaxy_arguments': self._internal_galaxy_spec, 'validate_args_context': { 'type': 'role', 'name': self._role_name, diff --git a/lib/ansible/playbook/role/definition.py b/lib/ansible/playbook/role/definition.py index 12d6ce182c8..d919248ceff 100644 --- a/lib/ansible/playbook/role/definition.py +++ b/lib/ansible/playbook/role/definition.py @@ -43,7 +43,7 @@ class RoleDefinition(Base, Conditional, Taggable, CollectionSearch): role = NonInheritableFieldAttribute(isa='string') - def __init__(self, play=None, role_basedir=None, variable_manager=None, loader=None, collection_list=None): + def __init__(self, play=None, role_basedir=None, variable_manager=None, loader=None, collection_list=None, extra_spec=None): super(RoleDefinition, self).__init__() @@ -57,6 +57,8 @@ class RoleDefinition(Base, Conditional, Taggable, CollectionSearch): self._role_params = dict() self._collection_list = collection_list + self._internal_galaxy_spec = extra_spec + # def __repr__(self): # return 'ROLEDEF: ' + self._attributes.get('role', '') diff --git a/lib/ansible/playbook/role/include.py b/lib/ansible/playbook/role/include.py index 934b53ce9b4..8efc9c8b70e 100644 --- a/lib/ansible/playbook/role/include.py +++ b/lib/ansible/playbook/role/include.py @@ -35,12 +35,12 @@ class RoleInclude(RoleDefinition, Delegatable): is included for execution in a play. """ - def __init__(self, play=None, role_basedir=None, variable_manager=None, loader=None, collection_list=None): + def __init__(self, play=None, role_basedir=None, variable_manager=None, loader=None, collection_list=None, extra_spec=None): super(RoleInclude, self).__init__(play=play, role_basedir=role_basedir, variable_manager=variable_manager, - loader=loader, collection_list=collection_list) + loader=loader, collection_list=collection_list, extra_spec=extra_spec) @staticmethod - def load(data, play, current_role_path=None, parent_role=None, variable_manager=None, loader=None, collection_list=None): + def load(data, play, current_role_path=None, parent_role=None, variable_manager=None, loader=None, collection_list=None, extra_spec=None): if not (isinstance(data, string_types) or isinstance(data, dict) or isinstance(data, AnsibleBaseYAMLObject)): raise AnsibleParserError("Invalid role definition: %s" % to_native(data)) @@ -48,5 +48,7 @@ class RoleInclude(RoleDefinition, Delegatable): if isinstance(data, string_types) and ',' in data: raise AnsibleError("Invalid old style role requirement: %s" % data) - ri = RoleInclude(play=play, role_basedir=current_role_path, variable_manager=variable_manager, loader=loader, collection_list=collection_list) + ri = RoleInclude( + play=play, role_basedir=current_role_path, variable_manager=variable_manager, loader=loader, collection_list=collection_list, extra_spec=extra_spec + ) return ri.load_data(data, variable_manager=variable_manager, loader=loader) diff --git a/lib/ansible/playbook/role/metadata.py b/lib/ansible/playbook/role/metadata.py index 482539a75d6..47f30b53d67 100644 --- a/lib/ansible/playbook/role/metadata.py +++ b/lib/ansible/playbook/role/metadata.py @@ -26,7 +26,7 @@ from ansible.playbook.attribute import NonInheritableFieldAttribute from ansible.playbook.base import Base from ansible.playbook.collectionsearch import CollectionSearch from ansible.playbook.helpers import load_list_of_roles -from ansible.playbook.role.requirement import RoleRequirement +from ansible.playbook.role.requirement import RoleRequirement, VALID_SPEC_KEYS __all__ = ['RoleMetadata'] @@ -101,10 +101,16 @@ class RoleMetadata(Base, CollectionSearch): if 'ansible.legacy' not in collection_search_list: collection_search_list.append('ansible.legacy') + # The dependency format is used by ansible-galaxy and when running roles. + # Since role runtime (with argument spec) requires all role params be documented, + # add a supplemental spec so this isn't a failure by default for ansible-galaxy options. + # If the role actually documents these options, they'll be validated normally. + internal_galaxy_spec = {k: {} for k in VALID_SPEC_KEYS} + try: return load_list_of_roles(roles, play=self._owner._play, current_role_path=current_role_path, variable_manager=self._variable_manager, loader=self._loader, - collection_search_list=collection_search_list) + collection_search_list=collection_search_list, extra_spec=internal_galaxy_spec) except AssertionError as e: raise AnsibleParserError("A malformed list of role dependencies was encountered.", obj=self._ds, orig_exc=e) diff --git a/lib/ansible/plugins/action/validate_argument_spec.py b/lib/ansible/plugins/action/validate_argument_spec.py index 4d68067a343..530596adfcc 100644 --- a/lib/ansible/plugins/action/validate_argument_spec.py +++ b/lib/ansible/plugins/action/validate_argument_spec.py @@ -6,6 +6,7 @@ from __future__ import annotations from ansible.errors import AnsibleError from ansible.plugins.action import ActionBase from ansible.module_utils.common.arg_spec import ArgumentSpecValidator +from ansible.playbook.role.requirement import RoleRequirement from ansible.utils.vars import combine_vars @@ -75,6 +76,21 @@ class ActionModule(ActionBase): if not isinstance(provided_arguments, dict): raise AnsibleError('Incorrect type for provided_arguments, expected dict and got %s' % type(provided_arguments)) + if self._task.implicit and (role_spec := self._task.args.get('optional_galaxy_arguments', ())): + galaxy_opts = {field: provided_arguments[field] for field in role_spec if field in provided_arguments and field not in argument_spec_data} + if galaxy_opts: + try: + # TODO: port this to use an argument spec? May need to deprecate syntax or add new argspec behavior: + # Suboptions don't support 'str' elements, defaults are determined from other options, values are mutated with custom rules, etc... + RoleRequirement.role_yaml_parse(galaxy_opts) + except AnsibleError: + # invalid for ansible-galaxy, validate normally + pass + else: + self._display.vvv(f"{self._task._role} - validated internal ansible-galaxy dependency format") + for opt in galaxy_opts: + provided_arguments.pop(opt) + args_from_vars = self.get_args_from_task_vars(argument_spec_data, task_vars) validator = ArgumentSpecValidator(argument_spec_data) validation_result = validator.validate(combine_vars(args_from_vars, provided_arguments), validate_role_argument_spec=True) diff --git a/test/integration/targets/roles_arg_spec/roles/role_with_deps/meta/main.yml b/test/integration/targets/roles_arg_spec/roles/role_with_deps/meta/main.yml new file mode 100644 index 00000000000..e567919144c --- /dev/null +++ b/test/integration/targets/roles_arg_spec/roles/role_with_deps/meta/main.yml @@ -0,0 +1,11 @@ +dependencies: + # src/scm/version/role are used by ansible-galaxy when installing dependencies + - src: a + version: unknown + a_str: required + + # only validate these fields if the argument spec includes them + - role: validate_role_req + scm: svn + src: othersource + version: "1.0.0" diff --git a/test/integration/targets/roles_arg_spec/roles/validate_role_req/meta/main.yml b/test/integration/targets/roles_arg_spec/roles/validate_role_req/meta/main.yml new file mode 100644 index 00000000000..f216e3fc317 --- /dev/null +++ b/test/integration/targets/roles_arg_spec/roles/validate_role_req/meta/main.yml @@ -0,0 +1,17 @@ +argument_specs: + main: + short_description: Main entrypoint to test validating the overloaded role params scm, src, and version. + options: + scm: + type: str + description: role variable 'scm' + choices: + - "git" + src: + type: str + description: role variable 'src' + choices: + - "src" + version: + type: int + description: role variable 'version' diff --git a/test/integration/targets/roles_arg_spec/test.yml b/test/integration/targets/roles_arg_spec/test.yml index b88d2e183d8..5b0d9f8c51f 100644 --- a/test/integration/targets/roles_arg_spec/test.yml +++ b/test/integration/targets/roles_arg_spec/test.yml @@ -478,3 +478,25 @@ - name: Import role with an empty argument_specs key import_role: name: empty_argspec + +- name: "New play to reset vars: Test not validating ambiguous role params by default" + hosts: localhost + gather_facts: false + tasks: + - block: + - name: Include role with dependencies using ansible-galaxy requirements syntax + include_role: + name: role_with_deps + + - fail: + msg: "should not get here" + + rescue: + - debug: var=ansible_failed_result + + - name: Validate failure for role params + assert: + that: + - ansible_failed_result.argument_errors | length == 3 + - ansible_failed_result.validate_args_context.name == 'validate_role_req' + - ansible_failed_result.validate_args_context.argument_spec_name == "main"