Role argspec: allow new argument spec file (#74582)

* support separate role argspec file in ansible-doc

* support separate role argspec file in ansible-core

* support both .yml and .yaml extensions on argspec file in ansible-doc

* fix filename building bug and rename some argspec files to test variations

* use yaml extensions from constants

* add superfluous meta/main.yml files to tests

* Update lib/ansible/cli/doc.py

Co-authored-by: Sam Doran <sdoran@redhat.com>

* update docs

* ci_complete

* add changelog and allow for main.yml variations

* add collection role testing

Co-authored-by: Sam Doran <sdoran@redhat.com>

changelogs/fragments/74582-role-argspec-new-file.yml

@@ -0,0 +1,2 @@
+bugfixes:
+    - roles - allow for role arg specs in new meta file (https://github.com/ansible/ansible/issues/74525).

docs/docsite/rst/user_guide/playbooks_reuse_roles.rst

@@ -260,9 +260,16 @@ Role argument validation
 ========================
 
 Beginning with version 2.11, you may choose to enable role argument validation based on an argument
-specification defined in the role ``meta/main.yml`` file. When this argument specification is defined,
+specification. This specification is defined in the ``meta/argument_specs.yml`` file (or with the ``.yaml``
-a new task is inserted at the beginning of role execution that will validate the parameters supplied
+file extension). When this argument specification is defined, a new task is inserted at the beginning of role execution
-for the role against the specification. If the parameters fail validation, the role will fail execution.
+that will validate the parameters supplied for the role against the specification. If the parameters fail
+validation, the role will fail execution.
+
+.. note::
+
+    Ansible also supports role specifications defined in the role ``meta/main.yml`` file, as well. However,
+    any role that defines the specs within this file will not work on versions below 2.11. For this reason,
+    we recommend using the ``meta/argument_specs.yml`` file to maintain backward compatibility.
 
 .. note::
 
@@ -274,7 +281,7 @@ Specification format
 --------------------
 
 The role argument specification must be defined in a top-level ``argument_specs`` block within the
-role ``meta/main.yml`` file. All fields are lower-case.
+role ``meta/argument_specs.yml`` file. All fields are lower-case.
 
 :entry-point-name:
 
@@ -346,7 +353,7 @@ Sample specification
 
 .. code-block:: yaml
 
- # roles/myapp/meta/main.yml
+ # roles/myapp/meta/argument_specs.yml
  ---
  argument_specs:
    # roles/myapp/tasks/main.yml entry point

lib/ansible/cli/doc.py

@@ -78,16 +78,47 @@ class RoleMixin(object):
     Note: The methods for actual display of role data are not present here.
     """
 
-    ROLE_ARGSPEC_FILE = 'main.yml'
+    # Potential locations of the role arg spec file in the meta subdir, with main.yml
+    # having the lowest priority.
+    ROLE_ARGSPEC_FILES = ['argument_specs' + e for e in C.YAML_FILENAME_EXTENSIONS] + ["main" + e for e in C.YAML_FILENAME_EXTENSIONS]
 
     def _load_argspec(self, role_name, collection_path=None, role_path=None):
+        """Load the role argument spec data from the source file.
+
+        :param str role_name: The name of the role for which we want the argspec data.
+        :param str collection_path: Path to the collection containing the role. This
+            will be None for standard roles.
+        :param str role_path: Path to the standard role. This will be None for
+            collection roles.
+
+        We support two files containing the role arg spec data: either meta/main.yml
+        or meta/argument_spec.yml. The argument_spec.yml file will take precedence
+        over the meta/main.yml file, if it exists. Data is NOT combined between the
+        two files.
+
+        :returns: A dict of all data underneath the ``argument_specs`` top-level YAML
+            key in the argspec data file. Empty dict is returned if there is no data.
+        """
+
         if collection_path:
-            path = os.path.join(collection_path, 'roles', role_name, 'meta', self.ROLE_ARGSPEC_FILE)
+            meta_path = os.path.join(collection_path, 'roles', role_name, 'meta')
         elif role_path:
-            path = os.path.join(role_path, 'meta', self.ROLE_ARGSPEC_FILE)
+            meta_path = os.path.join(role_path, 'meta')
         else:
             raise AnsibleError("A path is required to load argument specs for role '%s'" % role_name)
 
+        path = None
+
+        # Check all potential spec files
+        for specfile in self.ROLE_ARGSPEC_FILES:
+            full_path = os.path.join(meta_path, specfile)
+            if os.path.exists(full_path):
+                path = full_path
+                break
+
+        if path is None:
+            return {}
+
         try:
             with open(path, 'r') as f:
                 data = from_yaml(f.read(), file_name=path)
@@ -110,18 +141,25 @@ class RoleMixin(object):
         """
         found = set()
         found_names = set()
+
         for path in role_paths:
             if not os.path.isdir(path):
                 continue
-            # Check each subdir for a meta/main.yml file
+
+            # Check each subdir for an argument spec file
             for entry in os.listdir(path):
                 role_path = os.path.join(path, entry)
-                full_path = os.path.join(role_path, 'meta', self.ROLE_ARGSPEC_FILE)
+
-                if os.path.exists(full_path):
+                # Check all potential spec files
-                    if name_filters is None or entry in name_filters:
+                for specfile in self.ROLE_ARGSPEC_FILES:
-                        if entry not in found_names:
+                    full_path = os.path.join(role_path, 'meta', specfile)
-                            found.add((entry, role_path))
+                    if os.path.exists(full_path):
-                        found_names.add(entry)
+                        if name_filters is None or entry in name_filters:
+                            if entry not in found_names:
+                                found.add((entry, role_path))
+                            found_names.add(entry)
+                        # select first-found
+                        break
         return found
 
     def _find_all_collection_roles(self, name_filters=None, collection_filter=None):
@@ -147,19 +185,23 @@ class RoleMixin(object):
             roles_dir = os.path.join(path, 'roles')
             if os.path.exists(roles_dir):
                 for entry in os.listdir(roles_dir):
-                    full_path = os.path.join(roles_dir, entry, 'meta', self.ROLE_ARGSPEC_FILE)
+
-                    if os.path.exists(full_path):
+                    # Check all potential spec files
-                        if name_filters is None:
+                    for specfile in self.ROLE_ARGSPEC_FILES:
-                            found.add((entry, collname, path))
+                        full_path = os.path.join(roles_dir, entry, 'meta', specfile)
-                        else:
+                        if os.path.exists(full_path):
-                            # Name filters might contain a collection FQCN or not.
+                            if name_filters is None:
-                            for fqcn in name_filters:
+                                found.add((entry, collname, path))
-                                if len(fqcn.split('.')) == 3:
+                            else:
-                                    (ns, col, role) = fqcn.split('.')
+                                # Name filters might contain a collection FQCN or not.
-                                    if '.'.join([ns, col]) == collname and entry == role:
+                                for fqcn in name_filters:
+                                    if len(fqcn.split('.')) == 3:
+                                        (ns, col, role) = fqcn.split('.')
+                                        if '.'.join([ns, col]) == collname and entry == role:
+                                            found.add((entry, collname, path))
+                                    elif fqcn == entry:
                                         found.add((entry, collname, path))
-                                elif fqcn == entry:
+                            break
-                                    found.add((entry, collname, path))
         return found
 
     def _build_summary(self, role, collection, argspec):

lib/ansible/playbook/role/__init__.py

@@ -21,6 +21,7 @@ __metaclass__ = type
 
 import os
 
+from ansible import constants as C
 from ansible.errors import AnsibleError, AnsibleParserError, AnsibleAssertionError
 from ansible.module_utils._text import to_text
 from ansible.module_utils.six import iteritems, binary_type, text_type
@@ -256,7 +257,8 @@ class Role(Base, Conditional, Taggable, CollectionSearch):
         task_data = self._load_role_yaml('tasks', main=self._from_files.get('tasks'))
 
         if self._should_validate:
-            task_data = self._prepend_validation_task(task_data)
+            role_argspecs = self._get_role_argspecs()
+            task_data = self._prepend_validation_task(task_data, role_argspecs)
 
         if task_data:
             try:
@@ -274,7 +276,32 @@ class Role(Base, Conditional, Taggable, CollectionSearch):
                 raise AnsibleParserError("The handlers/main.yml file for role '%s' must contain a list of tasks" % self._role_name,
                                          obj=handler_data, orig_exc=e)
 
-    def _prepend_validation_task(self, task_data):
+    def _get_role_argspecs(self):
+        """Get the role argument spec data.
+
+        Role arg specs can be in one of two files in the role meta subdir: argument_specs.yml
+        or main.yml. The former has precedence over the latter. Data is not combined
+        between the files.
+
+        :returns: A dict of all data under the top-level ``argument_specs`` YAML key
+            in the argument spec file. An empty dict is returned if there is no
+            argspec data.
+        """
+        base_argspec_path = os.path.join(self._role_path, 'meta', 'argument_specs')
+
+        for ext in C.YAML_FILENAME_EXTENSIONS:
+            full_path = base_argspec_path + ext
+            if self._loader.path_exists(full_path):
+                # Note: _load_role_yaml() takes care of rebuilding the path.
+                argument_specs = self._load_role_yaml('meta', main='argument_specs')
+                return argument_specs.get('argument_specs', {})
+
+        # We did not find the meta/argument_specs.[yml|yaml] file, so use the spec
+        # dict from the role meta data, if it exists. Ansible 2.11 and later will
+        # have the 'argument_specs' attribute, but earlier versions will not.
+        return getattr(self._metadata, 'argument_specs', {})
+
+    def _prepend_validation_task(self, task_data, argspecs):
         '''Insert a role validation task if we have a role argument spec.
 
         This method will prepend a validation task to the front of the role task
@@ -282,14 +309,15 @@ class Role(Base, Conditional, Taggable, CollectionSearch):
         exists for the entry point. Entry point defaults to `main`.
 
         :param task_data: List of tasks loaded from the role.
+        :param argspecs: The role argument spec data dict.
 
         :returns: The (possibly modified) task list.
         '''
-        if self._metadata.argument_specs:
+        if argspecs:
             # Determine the role entry point so we can retrieve the correct argument spec.
             # This comes from the `tasks_from` value to include_role or import_role.
             entrypoint = self._from_files.get('tasks', 'main')
-            entrypoint_arg_spec = self._metadata.argument_specs.get(entrypoint)
+            entrypoint_arg_spec = argspecs.get(entrypoint)
 
             if entrypoint_arg_spec:
                 validation_task = self._create_validation_task(entrypoint_arg_spec, entrypoint)

test/integration/targets/ansible-doc/roles/test_role1/meta/argument_specs.yml

@@ -0,0 +1,34 @@
+---
+argument_specs:
+    main:
+        short_description: test_role1 from roles subdir
+        description:
+            - In to am attended desirous raptures B(declared) diverted confined at. Collected instantly remaining
+              up certainly to C(necessary) as. Over walk dull into son boy door went new.
+            - At or happiness commanded daughters as. Is I(handsome) an declared at received in extended vicinity
+              subjects. Into miss on he over been late pain an. Only week bore boy what fat case left use. Match round
+              scale now style far times. Your me past an much.
+        author:
+          - John Doe (@john)
+          - Jane Doe (@jane)
+
+        options:
+            myopt1:
+                description:
+                    - First option.
+                type: "str"
+                required: true
+
+            myopt2:
+                description:
+                    - Second option
+                type: "int"
+                default: 8000
+
+            myopt3:
+                description:
+                    - Third option.
+                type: "str"
+                choices:
+                    - choice1
+                    - choice2

test/integration/targets/ansible-doc/roles/test_role1/meta/main.yml

@@ -1,37 +1,13 @@
+# This meta/main.yml exists to test that it is NOT read, with preference being
+# given to the meta/argument_specs.yml file. This spec contains additional
+# entry points (groot, foo) that the argument_specs.yml does not. If this file
+# were read, the additional entrypoints would show up in --list output, breaking
+# tests.
 ---
-dependencies:
-galaxy_info:
-
 argument_specs:
     main:
         short_description: test_role1 from roles subdir
-        description:
+    groot:
-            - In to am attended desirous raptures B(declared) diverted confined at. Collected instantly remaining
+        short_description: I am Groot
-              up certainly to C(necessary) as. Over walk dull into son boy door went new.
+    foo:
-            - At or happiness commanded daughters as. Is I(handsome) an declared at received in extended vicinity
+        short_description: I am Foo
-              subjects. Into miss on he over been late pain an. Only week bore boy what fat case left use. Match round
-              scale now style far times. Your me past an much.
-        author:
-          - John Doe (@john)
-          - Jane Doe (@jane)
-
-        options:
-            myopt1:
-                description:
-                    - First option.
-                type: "str"
-                required: true
-
-            myopt2:
-                description:
-                    - Second option
-                type: "int"
-                default: 8000
-
-            myopt3:
-                description:
-                    - Third option.
-                type: "str"
-                choices:
-                    - choice1
-                    - choice2

test/integration/targets/roles_arg_spec/collections/ansible_collections/foo/bar/roles/blah/meta/argument_specs.yml

@@ -0,0 +1,8 @@
+argument_specs:
+    main:
+        short_description: "The foo.bar.blah role"
+        options:
+            blah_str:
+                type: "str"
+                required: true
+                description: "A string value"

test/integration/targets/roles_arg_spec/collections/ansible_collections/foo/bar/roles/blah/tasks/main.yml

@@ -0,0 +1,3 @@
+- name: "First task of blah role"
+  debug:
+      msg: "The string is {{ blah_str }}"

test/integration/targets/roles_arg_spec/roles/a/meta/argument_specs.yml

@@ -0,0 +1,17 @@
+argument_specs:
+    main:
+        short_description: Main entry point for role A.
+        options:
+            a_str:
+                type: "str"
+                required: true
+
+    alternate:
+        short_description: Alternate entry point for role A.
+        options:
+            a_int:
+                type: "int"
+                required: true
+
+    no_spec_entrypoint:
+        short_description: An entry point with no spec

test/integration/targets/roles_arg_spec/roles/a/meta/main.yml

@@ -1,3 +1,6 @@
+# This meta/main.yml exists to test that it is NOT read, with preference being
+# given to the meta/argument_specs.yml file. This spec contains an extra required
+# parameter, a_something, that argument_specs.yml does not.
 argument_specs:
     main:
         short_description: Main entry point for role A.
@@ -5,13 +8,6 @@ argument_specs:
             a_str:
                 type: "str"
                 required: true
-
+            a_something:
-    alternate:
+                type: "str"
-        short_description: Alternate entry point for role A.
-        options:
-            a_int:
-                type: "int"
                 required: true
-
-    no_spec_entrypoint:
-        short_description: An entry point with no spec

test/integration/targets/roles_arg_spec/test.yml

@@ -270,3 +270,36 @@
               # and validation wasn't performed up front (thus not returning 'argument_errors').
               - "'argument_errors' not in ansible_failed_result"
               - "'The task includes an option with an undefined variable.' in ansible_failed_result.msg"
+
+- name: "New play to reset vars: Test collection-based role"
+  hosts: localhost
+  gather_facts: false
+  tasks:
+    - name: "Valid collection-based role usage"
+      import_role:
+        name: "foo.bar.blah"
+      vars:
+        blah_str: "some string"
+
+
+- name: "New play to reset vars: Test collection-based role will fail"
+  hosts: localhost
+  gather_facts: false
+  tasks:
+    - block:
+      - name: "Invalid collection-based role usage"
+        import_role:
+          name: "foo.bar.blah"
+      - fail:
+          msg: "Should not get here"
+      rescue:
+        - debug: var=ansible_failed_result
+        - name: "Validate import_role failure for collection-based role"
+          assert:
+            that:
+              - ansible_failed_result.argument_errors | length == 1
+              - "'missing required arguments: blah_str' in ansible_failed_result.argument_errors"
+              - ansible_failed_result.validate_args_context.argument_spec_name == "main"
+              - ansible_failed_result.validate_args_context.name == "blah"
+              - ansible_failed_result.validate_args_context.type == "role"
+              - "ansible_failed_result.validate_args_context.path is search('roles_arg_spec/collections/ansible_collections/foo/bar/roles/blah')"