fixes static 'roles:' with play collection context

changelogs/fragments/keep_correct_collection_context.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - Now plays and their roles always keep the correct collection context they are in.

lib/ansible/constants.py

@@ -187,3 +187,16 @@ for setting in config.get_configuration_definitions():
 
 for warn in config.WARNINGS:
     _warning(warn)
+
+
+class _Constants(type(sys)):
+
+    _constants = {}
+
+    def __getitem__(self, name):
+        return _constants[name]
+
+    def __setitem__(self, name, value):
+        return _constants[name] = value
+
+sys.modules[__name__] = _Constants('constants')

lib/ansible/playbook/__init__.py

@@ -22,6 +22,7 @@ __metaclass__ = type
 import os
 
 from ansible import constants as C
+from ansible.utils.collection_loader._collection_finder import _get_collection_name_from_path
 from ansible.errors import AnsibleParserError
 from ansible.module_utils.common.text.converters import to_text, to_native
 from ansible.playbook.play import Play
@@ -45,6 +46,7 @@ class Playbook:
         self._basedir = to_text(os.getcwd(), errors='surrogate_or_strict')
         self._loader = loader
         self._file_name = None
+        self._collection = None
 
     @staticmethod
     def load(file_name, variable_manager=None, loader=None):
@@ -66,6 +68,7 @@ class Playbook:
         add_all_plugin_dirs(self._basedir)
 
         self._file_name = file_name
+        self._collection = _get_collection_name_from_path(self._file_name)
 
         try:
             ds = self._loader.load_from_file(os.path.basename(file_name))
@@ -104,7 +107,7 @@ class Playbook:
                             break
                     display.display("skipping playbook '%s' due to conditional test failure" % which, color=C.COLOR_SKIP)
             else:
-                entry_obj = Play.load(entry, variable_manager=variable_manager, loader=self._loader, vars=vars)
+                entry_obj = Play.load(entry, variable_manager=variable_manager, loader=self._loader, vars=vars, collection=self._collection)
                 self._entries.append(entry_obj)
 
         # we're done, so restore the old basedir in the loader

lib/ansible/playbook/collectionsearch.py

@@ -47,6 +47,10 @@ class CollectionSearch:
         # this will only be called if someone specified a value; call the shared value
         _ensure_default_collection(collection_list=ds)
 
+        # ensure 'current's object collection is here', only playbooks and plays should have this property
+        if getattr(self, '_collection', False) and self._collection not in ds:
+            ds.insert(0, self._collection)
+
         if not ds:  # don't return an empty collection list, just return None
             return None
 
@@ -57,7 +61,6 @@ class CollectionSearch:
         env = NativeEnvironment()
         for collection_name in ds:
             if is_template(collection_name, env):
-                display.warning('"collections" is not templatable, but we found: %s, '
-                                'it will not be templated and will be used "as is".' % (collection_name))
+                display.warning(f'"collections" is not templatable, but we found: {collection_name}, it will not be templated and will be used "as is".')
 
         return ds

lib/ansible/playbook/play.py

@@ -101,6 +101,8 @@ class Play(Base, Taggable, CollectionSearch):
         self._action_groups = {}
         self._group_actions = {}
 
+        self._collection = None
+
     def __repr__(self):
         return self.get_name()
 
@@ -150,8 +152,9 @@ class Play(Base, Taggable, CollectionSearch):
         return self.name
 
     @staticmethod
-    def load(data, variable_manager=None, loader=None, vars=None):
+    def load(data, variable_manager=None, loader=None, vars=None, collection=None):
         p = Play()
+        p._collection = collection
         if vars:
             p.vars = vars.copy()
         return p.load_data(data, variable_manager=variable_manager, loader=loader)
@@ -383,19 +386,23 @@ class Play(Base, Taggable, CollectionSearch):
         roles = []
         for role in self.get_roles():
             roles.append(role.serialize())
+
         data['roles'] = roles
         data['included_path'] = self._included_path
         data['action_groups'] = self._action_groups
         data['group_actions'] = self._group_actions
+        data['collection'] = self._collection
 
         return data
 
     def deserialize(self, data):
         super(Play, self).deserialize(data)
 
+        self._collection = data.get('collection', None)
         self._included_path = data.get('included_path', None)
         self._action_groups = data.get('action_groups', {})
         self._group_actions = data.get('group_actions', {})
+
         if 'roles' in data:
             role_data = data.get('roles', [])
             roles = []
@@ -414,4 +421,5 @@ class Play(Base, Taggable, CollectionSearch):
         new_me._included_path = self._included_path
         new_me._action_groups = self._action_groups
         new_me._group_actions = self._group_actions
+        new_me._collection = self._collection
         return new_me

lib/ansible/playbook/role/definition.py

@@ -154,6 +154,13 @@ class RoleDefinition(Base, Conditional, Taggable, CollectionSearch):
 
         role_tuple = None
 
+        # ensure play collection context
+        if self._play._collection:
+            if not self._collection_list:
+                self._collection_list = [self._play._collection]
+            elif self._play._collection not in self._collection_list:
+                self._collection_list.insert(0, self._play._collection)
+
         # try to load as a collection-based role first
         if self._collection_list or AnsibleCollectionRef.is_valid_fqcr(role_name):
             role_tuple = _get_collection_role_path(role_name, self._collection_list)

test/integration/targets/collection_role_loading/aliases

@@ -0,0 +1,2 @@
+shippable/posix/group3
+context/controller

test/integration/targets/collection_role_loading/collections/ansible_collections/namespace/common/playbooks/included.yml

@@ -0,0 +1,6 @@
+---
+- hosts: '{{ target|default("localhost") }}'
+  gather_facts: false
+  tasks:
+    - name: 'Ping in common/playbooks/included.yaml'
+      ping:

test/integration/targets/collection_role_loading/collections/ansible_collections/namespace/common/roles/common_ping/tasks/main.yml

@@ -0,0 +1,3 @@
+---
+- name: 'Ping in common/roles/common_ping/tasks/main.yml'
+  ping:

test/integration/targets/collection_role_loading/collections/ansible_collections/namespace/foobar/playbooks/entrypoint.yml

@@ -0,0 +1,15 @@
+---
+- hosts: '{{ target|default("localhost") }}'
+  gather_facts: false
+  roles:
+    - role: do_stuff
+
+- import_playbook: namespace.common.included
+
+- hosts: '{{ target|default("localhost") }}'
+  gather_facts: false
+  roles:
+    - do_stuff
+  tasks:
+    - import_role: name=do_stuff
+    - include_role: name=do_stuff

test/integration/targets/collection_role_loading/collections/ansible_collections/namespace/foobar/roles/do_stuff/tasks/main.yml

@@ -0,0 +1,3 @@
+---
+- name: 'Ping in foobar/role/do_stuff/tasks/main.yml'
+  ping:

test/integration/targets/collection_role_loading/runme.sh

@@ -0,0 +1,5 @@
+#!/usr/bin/env bash
+
+set -eux
+
+ansible-playbook test.yml -i ../../inventory "$@"

test/integration/targets/collection_role_loading/test.yml

@@ -0,0 +1,2 @@
+---
+- import_playbook: namespace.foobar.entrypoint