From ab6a544e8626eb6767e9578d63b41313f287c796 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Mon, 2 Oct 2023 15:42:00 -0400 Subject: [PATCH] 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 Co-authored-by: Sloane Hertel <19572925+s-hertel@users.noreply.github.com> --- .../fragments/import_role_goes_public.yml | 3 ++ lib/ansible/config/base.yml | 1 + lib/ansible/modules/import_role.py | 8 +++++ lib/ansible/playbook/role_include.py | 6 +--- lib/ansible/vars/manager.py | 35 +++++++++++-------- .../public_exposure/no_bleeding.yml | 4 +-- .../public_exposure/playbook.yml | 8 ++--- test/integration/targets/roles/vars_scope.yml | 27 ++++++++++++-- 8 files changed, 63 insertions(+), 29 deletions(-) create mode 100644 changelogs/fragments/import_role_goes_public.yml diff --git a/changelogs/fragments/import_role_goes_public.yml b/changelogs/fragments/import_role_goes_public.yml new file mode 100644 index 00000000000..e221adbe2c1 --- /dev/null +++ b/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``." diff --git a/lib/ansible/config/base.yml b/lib/ansible/config/base.yml index 952f8857d74..a37839523cf 100644 --- a/lib/ansible/config/base.yml +++ b/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} diff --git a/lib/ansible/modules/import_role.py b/lib/ansible/modules/import_role.py index 2f118f2f57b..bc1fd4d2a98 100644 --- a/lib/ansible/modules/import_role.py +++ b/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 diff --git a/lib/ansible/playbook/role_include.py b/lib/ansible/playbook/role_include.py index bb3141fbae7..7f0948763ad 100644 --- a/lib/ansible/playbook/role_include.py +++ b/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: diff --git a/lib/ansible/vars/manager.py b/lib/ansible/vars/manager.py index b6adad34b76..54a0b87753a 100644 --- a/lib/ansible/vars/manager.py +++ b/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 diff --git a/test/integration/targets/include_import/public_exposure/no_bleeding.yml b/test/integration/targets/include_import/public_exposure/no_bleeding.yml index bce5d90e1a3..b9db7132903 100644 --- a/test/integration/targets/include_import/public_exposure/no_bleeding.yml +++ b/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: diff --git a/test/integration/targets/include_import/public_exposure/playbook.yml b/test/integration/targets/include_import/public_exposure/playbook.yml index ea9641974ce..11735e7731b 100644 --- a/test/integration/targets/include_import/public_exposure/playbook.yml +++ b/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' diff --git a/test/integration/targets/roles/vars_scope.yml b/test/integration/targets/roles/vars_scope.yml index 256723a2302..3e6b16a31c2 100644 --- a/test/integration/targets/roles/vars_scope.yml +++ b/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