Collection role relative deps (#61517)

* default collection support

* playbooks run from inside a registered collection will set that collection as the first item in the search order (as will all non-collection roles)
* this allows easy migration of runme.sh style playbook/role integration tests to collections without the playbooks/roles needing to know the name of their enclosing collection

* disable default collection test under Windows

* enable collection search for role dependencies

* unqualified role deps in collection-hosted roles will first search the containing collection
* if the calling role has specified a collections search list in metadata, it will be appended to the search order for unqualified role deps

* disable cycle detection unit test

* failing on 3.7+, needs proper cycle detection
* see #61527

changelogs/fragments/collection_dep_search.yml

@@ -0,0 +1,4 @@
+minor_changes:
+  - collection role dependencies will first search for unqualified role names in the containing collection.
+  - roles that define a collections search list in metadata will attempt to use the defined search list
+    when resolving unqualified role names.

lib/ansible/playbook/helpers.py

@@ -368,12 +368,17 @@ 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):
-    '''
-    Loads and returns a list of RoleInclude objects from the datastructure
-    list of role definitions
-    '''
-
+def load_list_of_roles(ds, play, current_role_path=None, variable_manager=None, loader=None, collection_search_list=None):
+    """
+    Loads and returns a list of RoleInclude objects from the ds list of role definitions
+    :param ds: list of roles to load
+    :param play: calling Play object
+    :param current_role_path: path of the owning role, if any
+    :param variable_manager: varmgr to use for templating
+    :param loader: loader to use for DS parsing/services
+    :param collection_search_list: list of collections to search for unqualified role names
+    :return:
+    """
     # we import here to prevent a circular dependency with imports
     from ansible.playbook.role.include import RoleInclude
 
@@ -383,7 +388,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=play.collections)
+                             loader=loader, collection_list=collection_search_list)
         roles.append(i)
 
     return roles

lib/ansible/playbook/play.py

@@ -192,7 +192,8 @@ class Play(Base, Taggable, CollectionSearch):
             ds = []
 
         try:
-            role_includes = load_list_of_roles(ds, play=self, variable_manager=self._variable_manager, loader=self._loader)
+            role_includes = load_list_of_roles(ds, play=self, variable_manager=self._variable_manager,
+                                               loader=self._loader, collection_search_list=self.collections)
         except AssertionError as e:
             raise AnsibleParserError("A malformed role declaration was encountered.", obj=self._ds, orig_exc=e)
 

lib/ansible/playbook/role/__init__.py

@@ -162,6 +162,10 @@ class Role(Base, Conditional, Taggable, CollectionSearch):
                             role_obj.add_parent(parent_role)
                         return role_obj
 
+            # TODO: need to fix cycle detection in role load (maybe use an empty dict
+            #  for the in-flight in role cache as a sentinel that we're already trying to load
+            #  that role?)
+            # see https://github.com/ansible/ansible/issues/61527
             r = Role(play=play, from_files=from_files, from_include=from_include)
             r._load_role_data(role_include, parent_role=parent_role)
 

lib/ansible/playbook/role/metadata.py

@@ -84,12 +84,27 @@ class RoleMetadata(Base, CollectionSearch):
                     raise AnsibleParserError(to_native(exc), obj=role_def, orig_exc=exc)
 
         current_role_path = None
+        collection_search_list = None
+
         if self._owner:
             current_role_path = os.path.dirname(self._owner._role_path)
 
+            # if the calling role has a collections search path defined, consult it
+            collection_search_list = self._owner.collections[:] or []
+
+            # if the calling role is a collection role, ensure that its containing collection is searched first
+            owner_collection = self._owner._role_collection
+            if owner_collection:
+                collection_search_list = [c for c in collection_search_list if c != owner_collection]
+                collection_search_list.insert(0, owner_collection)
+            # ensure fallback role search works
+            if 'ansible.legacy' not in collection_search_list:
+                collection_search_list.append('ansible.legacy')
+
         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)
+            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)
         except AssertionError as e:
             raise AnsibleParserError("A malformed list of role dependencies was encountered.", obj=self._ds, orig_exc=e)
 

lib/ansible/utils/collection_loader.py

@@ -173,7 +173,7 @@ class AnsibleCollectionLoader(with_metaclass(Singleton, object)):
             is_package = True
             location = None
             # check for implicit sub-package first
-            if os.path.isdir(candidate_child_path):
+            if os.path.isdir(to_bytes(candidate_child_path)):
                 # Py3.x implicit namespace packages don't have a file location, so they don't support get_data
                 # (which assumes the parent dir or that the loader has an internal mapping); so we have to provide
                 # a bogus leaf file on the __file__ attribute for pkgutil.get_data to strip off
@@ -181,10 +181,10 @@ class AnsibleCollectionLoader(with_metaclass(Singleton, object)):
             else:
                 for source_path in [os.path.join(candidate_child_path, '__init__.py'),
                                     candidate_child_path + '.py']:
-                    if not os.path.isfile(source_path):
+                    if not os.path.isfile(to_bytes(source_path)):
                         continue
 
-                    with open(source_path, 'rb') as fd:
+                    with open(to_bytes(source_path), 'rb') as fd:
                         source = fd.read()
 
                     code_object = compile(source=source, filename=source_path, mode='exec', flags=0, dont_inherit=True)

test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/roles/calls_intra_collection_dep_role_unqualified/meta/main.yml

@@ -0,0 +1,2 @@
+dependencies:
+  - testrole  # since testrole lives in this collection, we'll check there first

test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/roles/calls_intra_collection_dep_role_unqualified/tasks/main.yml

@@ -0,0 +1,7 @@
+- debug:
+    msg: '{{ outer_role_input | default("(undefined)") }}'
+  register: outer_role_output
+
+- assert:
+    that:
+    - outer_role_input is not defined or outer_role_input == outer_role_output.msg

test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/roles/role_subdir/subdir_testrole/tasks/main.yml

@@ -2,6 +2,9 @@
     msg: '{{ test_role_input | default("(undefined)") }}'
   register: test_role_output
 
+- set_fact:
+    testrole_source: collection
+
 - assert:
     that:
     - test_role_input is not defined or test_role_input == test_role_output.msg

test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/roles/testrole/tasks/main.yml

@@ -19,6 +19,9 @@
     msg: '{{ test_role_input | default("(undefined)") }}'
   register: test_role_output
 
+- set_fact:
+    testrole_source: collection
+
 # FIXME: add tests to ensure that block/task level stuff in a collection-hosted role properly inherit role default/meta values
 
 - assert:

test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/roles/testrole_main_yaml/tasks/main.yml

@@ -19,6 +19,9 @@
     msg: '{{ test_role_input | default("(undefined)") }}'
   register: test_role_output
 
+- set_fact:
+    testrole_source: collection
+
 # FIXME: add tests to ensure that block/task level stuff in a collection-hosted role properly inherit role default/meta values
 
 - assert:

test/integration/targets/collections/posix.yml

@@ -208,6 +208,7 @@
     assert:
       that:
       - test_role_output.msg == test_role_input
+      - testrole_source == 'collection'
 
 
 # test dynamic execution of a FQ collection-backed role
@@ -225,7 +226,7 @@
     assert:
       that:
       - test_role_output.msg == test_role_input
-
+      - testrole_source == 'collection'
 
 # test task-static execution of a FQ collection-backed role
 - name: verify collection-backed role execution (task static)
@@ -241,6 +242,7 @@
     assert:
       that:
       - test_role_output.msg == test_role_input
+      - testrole_source == 'collection'
 
 
 # test a legacy playbook-adjacent role, ensure that play collections config is not inherited
@@ -259,6 +261,8 @@
     assert:
       that:
       - test_role_output.msg == test_role_input
+      - testrole_source == 'legacy roles dir'
+
 
 # test dynamic execution of a FQ collection-backed role
 - name: verify collection-backed role execution in subdir (include)
@@ -272,6 +276,53 @@
     assert:
       that:
       - test_role_output.msg == test_role_input
+      - testrole_source == 'collection'
+
+
+# test collection-relative role deps (keyword static)
+- name: verify collection-relative role deps
+  hosts: testhost
+  vars:
+    outer_role_input: keyword static outer
+    test_role_input: keyword static inner
+  roles:
+    - testns.testcoll.calls_intra_collection_dep_role_unqualified
+  tasks:
+    - assert:
+        that:
+          - outer_role_output.msg == outer_role_input
+          - test_role_output.msg == test_role_input
+          - testrole_source == 'collection'
+
+# test collection-relative role deps (task static)
+- name: verify collection-relative role deps
+  hosts: testhost
+  vars:
+    outer_role_input: task static outer
+    test_role_input: task static inner
+  tasks:
+    - import_role:
+        name: testns.testcoll.calls_intra_collection_dep_role_unqualified
+    - assert:
+        that:
+          - outer_role_output.msg == outer_role_input
+          - test_role_output.msg == test_role_input
+          - testrole_source == 'collection'
+
+# test collection-relative role deps (task dynamic)
+- name: verify collection-relative role deps
+  hosts: testhost
+  vars:
+    outer_role_input: task dynamic outer
+    test_role_input: task dynamic inner
+  tasks:
+    - include_role:
+        name: testns.testcoll.calls_intra_collection_dep_role_unqualified
+    - assert:
+        that:
+          - outer_role_output.msg == outer_role_input
+          - test_role_output.msg == test_role_input
+          - testrole_source == 'collection'
 
 
 - name: validate static task include behavior

test/integration/targets/collections/roles/testrole/tasks/main.yml

@@ -17,6 +17,9 @@
     msg: '{{ test_role_input | default("(undefined)") }}'
   register: test_role_output
 
+- set_fact:
+    testrole_source: legacy roles dir
+
 - assert:
     that:
     - coll_module_out.source == 'user'

test/units/playbook/role/test_role.py

@@ -401,8 +401,11 @@ class TestRole(unittest.TestCase):
         i = RoleInclude.load('bad2_metadata', play=mock_play, loader=fake_loader)
         self.assertRaises(AnsibleParserError, Role.load, i, play=mock_play)
 
-        i = RoleInclude.load('recursive1_metadata', play=mock_play, loader=fake_loader)
-        self.assertRaises(AnsibleError, Role.load, i, play=mock_play)
+        # TODO: re-enable this test once Ansible has proper role dep cycle detection
+        # that doesn't rely on stack overflows being recoverable (as they aren't in Py3.7+)
+        # see https://github.com/ansible/ansible/issues/61527
+        # i = RoleInclude.load('recursive1_metadata', play=mock_play, loader=fake_loader)
+        # self.assertRaises(AnsibleError, Role.load, i, play=mock_play)
 
     @patch('ansible.playbook.role.definition.unfrackpath', mock_unfrackpath_noop)
     def test_load_role_complex(self):