Import role public (#81772)

revert to previous behavior to push vars to play at compile time
add `public` parameter to allow per import control of exporting (vs just the global config)

Co-authored-by: tchernomax <maxime.deroucy@gmail.com>
Co-authored-by: Sloane Hertel <19572925+s-hertel@users.noreply.github.com>

changelogs/fragments/import_role_goes_public.yml

@@ -0,0 +1,3 @@
+minor_changes:
+  - "``import_role`` action now also gets a ``public`` option that controls variable exports,  default depending on ``DEFAULT_PRIVATE_ROLE_VARS`` (if using defaults equates to ``public=True``)."
+  - "``DEFAULT_PRIVATE_ROLE_VARS`` is now overriden by explicit setting of ``public`` for ``include_roles`` and ``import_roles``."

lib/ansible/config/base.yml

@@ -937,6 +937,7 @@ DEFAULT_PRIVATE_ROLE_VARS:
     - Makes role variables inaccessible from other roles.
     - This was introduced as a way to reset role variables to default values if
       a role is used more than once in a playbook.
+    - Starting in version '2.17' M(ansible.builtin.include_roles) and M(ansible.builtin.import_roles) can override this via the C(public) parameter.
   env: [{name: ANSIBLE_PRIVATE_ROLE_VARS}]
   ini:
   - {key: private_role_vars, section: defaults}

lib/ansible/modules/import_role.py

@@ -56,6 +56,14 @@ options:
     type: bool
     default: yes
     version_added: '2.11'
+  public:
+    description:
+      - This option dictates whether the role's C(vars) and C(defaults) are exposed to the play.
+      - Variables are exposed to the play at playbook parsing time, and available to earlier roles and tasks as well unlike C(include_role).
+      - The default depends on the configuration option :ref:`default_private_role_vars`.
+    type: bool
+    default: yes
+    version_added: '2.17'
 extends_documentation_fragment:
     - action_common_attributes
     - action_common_attributes.conn

lib/ansible/playbook/role_include.py

@@ -49,10 +49,10 @@ class IncludeRole(TaskInclude):
 
     # =================================================================================
     # ATTRIBUTES
+    public = NonInheritableFieldAttribute(isa='bool', default=None, private=False, always_post_validate=True)
 
     # private as this is a 'module options' vs a task property
     allow_duplicates = NonInheritableFieldAttribute(isa='bool', default=True, private=True, always_post_validate=True)
-    public = NonInheritableFieldAttribute(isa='bool', default=False, private=True, always_post_validate=True)
     rolespec_validate = NonInheritableFieldAttribute(isa='bool', default=True, private=True, always_post_validate=True)
 
     def __init__(self, block=None, role=None, task_include=None):
@@ -136,10 +136,6 @@ class IncludeRole(TaskInclude):
         if ir._role_name is None:
             raise AnsibleParserError("'name' is a required field for %s." % ir.action, obj=data)
 
-        # public is only valid argument for includes, imports are always 'public' (after they run)
-        if 'public' in ir.args and ir.action not in C._ACTION_INCLUDE_ROLE:
-            raise AnsibleParserError('Invalid options for %s: public' % ir.action, obj=data)
-
         # validate bad args, otherwise we silently ignore
         bad_opts = my_arg_names.difference(IncludeRole.VALID_ARGS)
         if bad_opts:

lib/ansible/vars/manager.py

@@ -196,14 +196,15 @@ class VariableManager:
             basedirs = [self._loader.get_basedir()]
 
         if play:
-            if not C.DEFAULT_PRIVATE_ROLE_VARS:
-                # first we compile any vars specified in defaults/main.yml
-                # for all roles within the specified play
-                for role in play.get_roles():
-                    # role from roles or include_role+public or import_role and completed
-                    if not role.from_include or role.public or (role.static and role._completed.get(to_text(host), False)):
-                        all_vars = _combine_and_track(all_vars, role.get_default_vars(), "role '%s' defaults" % role.name)
-
+            for role in play.get_roles():
+                # role is public and
+                #    either static or dynamic and completed
+                # role is not set
+                #    use config option as default
+                role_is_static_or_completed = role.static or role._completed.get(host.name, False)
+                if role.public and role_is_static_or_completed or \
+                   role.public is None and not C.DEFAULT_PRIVATE_ROLE_VARS and role_is_static_or_completed:
+                    all_vars = _combine_and_track(all_vars, role.get_default_vars(), "role '%s' defaults" % role.name)
         if task:
             # set basedirs
             if C.PLAYBOOK_VARS_ROOT == 'all':  # should be default
@@ -386,12 +387,18 @@ class VariableManager:
                 raise AnsibleParserError("Error while reading vars files - please supply a list of file names. "
                                          "Got '%s' of type %s" % (vars_files, type(vars_files)))
 
-            # By default, we now merge in all exported vars from all roles in the play,
-            # unless the user has disabled this via a config option
-            if not C.DEFAULT_PRIVATE_ROLE_VARS:
-                for role in play.get_roles():
-                    if not role.from_include or role.public or (role.static and role._completed.get(to_text(host), False)):
-                        all_vars = _combine_and_track(all_vars, role.get_vars(include_params=False, only_exports=True), "role '%s' exported vars" % role.name)
+            # We now merge in all exported vars from all roles in the play,
+            # unless the user has disabled this
+            # role is public and
+            #    either static or dynamic and completed
+            # role is not set
+            #    use config option as default
+            for role in play.get_roles():
+                role_is_static_or_completed = role.static or role._completed.get(host.name, False)
+                if role.public and role_is_static_or_completed or \
+                   role.public is None and not C.DEFAULT_PRIVATE_ROLE_VARS and role_is_static_or_completed:
+
+                    all_vars = _combine_and_track(all_vars, role.get_vars(include_params=False, only_exports=True), "role '%s' exported vars" % role.name)
 
         # next, we merge in the vars from the role, which will specifically
         # follow the role dependency chain, and then we merge in the tasks

test/integration/targets/include_import/public_exposure/no_bleeding.yml

@@ -5,8 +5,8 @@
     - name: Static imports should expose vars at parse time, not at execution time
       assert:
         that:
-          - static_defaults_var is not defined
-          - static_vars_var is not defined
+          - static_defaults_var == 'static_defaults'
+          - static_vars_var == 'static_vars'
     - import_role:
         name: static
     - assert:

test/integration/targets/include_import/public_exposure/playbook.yml

@@ -10,16 +10,12 @@
     - name: Static imports should expose vars at parse time, not at execution time
       assert:
         that:
-          - static_defaults_var is not defined
-          - static_vars_var is not defined
-          - static_task_var is not defined
+          - static_defaults_var == 'static_defaults'
+          - static_vars_var == 'static_vars'
     - import_role:
         name: static
     - assert:
         that:
-          - static_vars_var is defined
-          - static_tasks_var is defined
-          - static_defaults_var is defined
           - static_tasks_var == 'static_tasks'
           - static_defaults_var == 'static_defaults'
           - static_vars_var == 'static_vars'

test/integration/targets/roles/vars_scope.yml

@@ -63,8 +63,7 @@
           play_only: play
           roles_and_role_vars: role_vars
           role_vars_only: role_vars
-
-- name: play and import
+- name: play baseline (no roles)
   hosts: localhost
   gather_facts: false
   vars_files:
@@ -82,6 +81,30 @@
           play_and_role_vars: play
           play_only: play
 
+- name: play and import
+  hosts: localhost
+  gather_facts: false
+  vars_files:
+    - vars/play.yml
+  tasks:
+    - include_tasks: roles/vars_scope/tasks/check_vars.yml
+      vars:
+        defined:
+          play_and_import: play
+          play_and_include: play
+          play_and_roles: play
+          play_only: play
+          default_only: default
+          import_and_role_vars: role_vars
+          include_and_role_vars: role_vars
+          play_and_import_and_role_vars: role_vars
+          play_and_role_vars: role_vars
+          play_and_role_vars_and_role_vars: role_vars
+          play_and_include_and_role_vars: role_vars
+          play_and_roles_and_role_vars: role_vars
+          roles_and_role_vars: role_vars
+          role_vars_only: role_vars
+
     - name: static import
       import_role:
         name: vars_scope