From ddbad5842886eaff073182b5504a0b757a00b42d Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Mon, 20 Mar 2023 17:55:58 +0100 Subject: [PATCH] Use FAs with inheritance only when applicable (#80026) (#80076) ... and set default value of an attribute on an object only in NonInheritableFA. Fixes #79777 ci_complete (cherry picked from commit 6c3d2a4e51df560e350d1ca3d6a039bf0e05f2ab) --- .../79777-fix-inheritance-roles-meta.yml | 2 ++ lib/ansible/playbook/attribute.py | 1 - lib/ansible/playbook/handler.py | 4 +-- lib/ansible/playbook/loop_control.py | 14 ++++---- lib/ansible/playbook/play.py | 36 +++++++++---------- lib/ansible/playbook/playbook_include.py | 6 ++-- lib/ansible/playbook/role/definition.py | 4 +-- lib/ansible/playbook/role/metadata.py | 10 +++--- lib/ansible/playbook/role_include.py | 8 ++--- lib/ansible/playbook/task.py | 22 ++++++------ .../dep_keyword_inheritance.yml | 8 +++++ .../roles/role-meta-inheritance/meta/main.yml | 4 +++ .../targets/keyword_inheritance/runme.sh | 2 ++ 13 files changed, 68 insertions(+), 53 deletions(-) create mode 100644 changelogs/fragments/79777-fix-inheritance-roles-meta.yml create mode 100644 test/integration/targets/keyword_inheritance/dep_keyword_inheritance.yml create mode 100644 test/integration/targets/keyword_inheritance/roles/role-meta-inheritance/meta/main.yml diff --git a/changelogs/fragments/79777-fix-inheritance-roles-meta.yml b/changelogs/fragments/79777-fix-inheritance-roles-meta.yml new file mode 100644 index 00000000000..dd59e1d4efc --- /dev/null +++ b/changelogs/fragments/79777-fix-inheritance-roles-meta.yml @@ -0,0 +1,2 @@ +bugfixes: + - "Fix an issue where the value of ``become`` was ignored when used on a role used as a dependency in ``main/meta.yml`` (https://github.com/ansible/ansible/issues/79777)" diff --git a/lib/ansible/playbook/attribute.py b/lib/ansible/playbook/attribute.py index b28405d2430..692aa9a716a 100644 --- a/lib/ansible/playbook/attribute.py +++ b/lib/ansible/playbook/attribute.py @@ -178,7 +178,6 @@ class FieldAttribute(Attribute): value = self.default if callable(value): value = value() - setattr(obj, f'_{self.name}', value) return value diff --git a/lib/ansible/playbook/handler.py b/lib/ansible/playbook/handler.py index 675eecb3450..68970b4f1f0 100644 --- a/lib/ansible/playbook/handler.py +++ b/lib/ansible/playbook/handler.py @@ -19,14 +19,14 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -from ansible.playbook.attribute import FieldAttribute +from ansible.playbook.attribute import NonInheritableFieldAttribute from ansible.playbook.task import Task from ansible.module_utils.six import string_types class Handler(Task): - listen = FieldAttribute(isa='list', default=list, listof=string_types, static=True) + listen = NonInheritableFieldAttribute(isa='list', default=list, listof=string_types, static=True) def __init__(self, block=None, role=None, task_include=None): self.notified_hosts = [] diff --git a/lib/ansible/playbook/loop_control.py b/lib/ansible/playbook/loop_control.py index 2f56166575f..d69e14fb94a 100644 --- a/lib/ansible/playbook/loop_control.py +++ b/lib/ansible/playbook/loop_control.py @@ -19,18 +19,18 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -from ansible.playbook.attribute import FieldAttribute +from ansible.playbook.attribute import NonInheritableFieldAttribute from ansible.playbook.base import FieldAttributeBase class LoopControl(FieldAttributeBase): - loop_var = FieldAttribute(isa='str', default='item', always_post_validate=True) - index_var = FieldAttribute(isa='str', always_post_validate=True) - label = FieldAttribute(isa='str') - pause = FieldAttribute(isa='float', default=0, always_post_validate=True) - extended = FieldAttribute(isa='bool', always_post_validate=True) - extended_allitems = FieldAttribute(isa='bool', default=True, always_post_validate=True) + loop_var = NonInheritableFieldAttribute(isa='str', default='item', always_post_validate=True) + index_var = NonInheritableFieldAttribute(isa='str', always_post_validate=True) + label = NonInheritableFieldAttribute(isa='str') + pause = NonInheritableFieldAttribute(isa='float', default=0, always_post_validate=True) + extended = NonInheritableFieldAttribute(isa='bool', always_post_validate=True) + extended_allitems = NonInheritableFieldAttribute(isa='bool', default=True, always_post_validate=True) def __init__(self): super(LoopControl, self).__init__() diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index 23bb36b2bf6..3b763b9e491 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -25,7 +25,7 @@ from ansible.errors import AnsibleParserError, AnsibleAssertionError from ansible.module_utils._text import to_native from ansible.module_utils.common.collections import is_sequence from ansible.module_utils.six import binary_type, string_types, text_type -from ansible.playbook.attribute import FieldAttribute +from ansible.playbook.attribute import NonInheritableFieldAttribute from ansible.playbook.base import Base from ansible.playbook.block import Block from ansible.playbook.collectionsearch import CollectionSearch @@ -55,35 +55,35 @@ class Play(Base, Taggable, CollectionSearch): """ # ================================================================================= - hosts = FieldAttribute(isa='list', required=True, listof=string_types, always_post_validate=True, priority=-2) + hosts = NonInheritableFieldAttribute(isa='list', required=True, listof=string_types, always_post_validate=True, priority=-2) # Facts - gather_facts = FieldAttribute(isa='bool', default=None, always_post_validate=True) + gather_facts = NonInheritableFieldAttribute(isa='bool', default=None, always_post_validate=True) # defaults to be deprecated, should be 'None' in future - gather_subset = FieldAttribute(isa='list', default=(lambda: C.DEFAULT_GATHER_SUBSET), listof=string_types, always_post_validate=True) - gather_timeout = FieldAttribute(isa='int', default=C.DEFAULT_GATHER_TIMEOUT, always_post_validate=True) - fact_path = FieldAttribute(isa='string', default=C.DEFAULT_FACT_PATH) + gather_subset = NonInheritableFieldAttribute(isa='list', default=(lambda: C.DEFAULT_GATHER_SUBSET), listof=string_types, always_post_validate=True) + gather_timeout = NonInheritableFieldAttribute(isa='int', default=C.DEFAULT_GATHER_TIMEOUT, always_post_validate=True) + fact_path = NonInheritableFieldAttribute(isa='string', default=C.DEFAULT_FACT_PATH) # Variable Attributes - vars_files = FieldAttribute(isa='list', default=list, priority=99) - vars_prompt = FieldAttribute(isa='list', default=list, always_post_validate=False) + vars_files = NonInheritableFieldAttribute(isa='list', default=list, priority=99) + vars_prompt = NonInheritableFieldAttribute(isa='list', default=list, always_post_validate=False) # Role Attributes - roles = FieldAttribute(isa='list', default=list, priority=90) + roles = NonInheritableFieldAttribute(isa='list', default=list, priority=90) # Block (Task) Lists Attributes - handlers = FieldAttribute(isa='list', default=list, priority=-1) - pre_tasks = FieldAttribute(isa='list', default=list, priority=-1) - post_tasks = FieldAttribute(isa='list', default=list, priority=-1) - tasks = FieldAttribute(isa='list', default=list, priority=-1) + handlers = NonInheritableFieldAttribute(isa='list', default=list, priority=-1) + pre_tasks = NonInheritableFieldAttribute(isa='list', default=list, priority=-1) + post_tasks = NonInheritableFieldAttribute(isa='list', default=list, priority=-1) + tasks = NonInheritableFieldAttribute(isa='list', default=list, priority=-1) # Flag/Setting Attributes - force_handlers = FieldAttribute(isa='bool', default=context.cliargs_deferred_get('force_handlers'), always_post_validate=True) - max_fail_percentage = FieldAttribute(isa='percent', always_post_validate=True) - serial = FieldAttribute(isa='list', default=list, always_post_validate=True) - strategy = FieldAttribute(isa='string', default=C.DEFAULT_STRATEGY, always_post_validate=True) - order = FieldAttribute(isa='string', always_post_validate=True) + force_handlers = NonInheritableFieldAttribute(isa='bool', default=context.cliargs_deferred_get('force_handlers'), always_post_validate=True) + max_fail_percentage = NonInheritableFieldAttribute(isa='percent', always_post_validate=True) + serial = NonInheritableFieldAttribute(isa='list', default=list, always_post_validate=True) + strategy = NonInheritableFieldAttribute(isa='string', default=C.DEFAULT_STRATEGY, always_post_validate=True) + order = NonInheritableFieldAttribute(isa='string', always_post_validate=True) # ================================================================================= diff --git a/lib/ansible/playbook/playbook_include.py b/lib/ansible/playbook/playbook_include.py index 03210ea37dc..8e3116f4fbe 100644 --- a/lib/ansible/playbook/playbook_include.py +++ b/lib/ansible/playbook/playbook_include.py @@ -27,7 +27,7 @@ from ansible.module_utils._text import to_bytes from ansible.module_utils.six import string_types from ansible.parsing.splitter import split_args, parse_kv from ansible.parsing.yaml.objects import AnsibleBaseYAMLObject, AnsibleMapping -from ansible.playbook.attribute import FieldAttribute +from ansible.playbook.attribute import NonInheritableFieldAttribute from ansible.playbook.base import Base from ansible.playbook.conditional import Conditional from ansible.playbook.taggable import Taggable @@ -41,8 +41,8 @@ display = Display() class PlaybookInclude(Base, Conditional, Taggable): - import_playbook = FieldAttribute(isa='string') - vars_val = FieldAttribute(isa='dict', default=dict, alias='vars') + import_playbook = NonInheritableFieldAttribute(isa='string') + vars_val = NonInheritableFieldAttribute(isa='dict', default=dict, alias='vars') @staticmethod def load(data, basedir, variable_manager=None, loader=None): diff --git a/lib/ansible/playbook/role/definition.py b/lib/ansible/playbook/role/definition.py index b27a23174ba..f7ca3a85d9f 100644 --- a/lib/ansible/playbook/role/definition.py +++ b/lib/ansible/playbook/role/definition.py @@ -25,7 +25,7 @@ from ansible import constants as C from ansible.errors import AnsibleError, AnsibleAssertionError from ansible.module_utils.six import string_types from ansible.parsing.yaml.objects import AnsibleBaseYAMLObject, AnsibleMapping -from ansible.playbook.attribute import FieldAttribute +from ansible.playbook.attribute import NonInheritableFieldAttribute from ansible.playbook.base import Base from ansible.playbook.collectionsearch import CollectionSearch from ansible.playbook.conditional import Conditional @@ -43,7 +43,7 @@ display = Display() class RoleDefinition(Base, Conditional, Taggable, CollectionSearch): - role = FieldAttribute(isa='string') + role = NonInheritableFieldAttribute(isa='string') def __init__(self, play=None, role_basedir=None, variable_manager=None, loader=None, collection_list=None): diff --git a/lib/ansible/playbook/role/metadata.py b/lib/ansible/playbook/role/metadata.py index 275ee548b36..a4dbcf7e9ab 100644 --- a/lib/ansible/playbook/role/metadata.py +++ b/lib/ansible/playbook/role/metadata.py @@ -24,7 +24,7 @@ import os from ansible.errors import AnsibleParserError, AnsibleError from ansible.module_utils._text import to_native from ansible.module_utils.six import string_types -from ansible.playbook.attribute import FieldAttribute +from ansible.playbook.attribute import NonInheritableFieldAttribute from ansible.playbook.base import Base from ansible.playbook.collectionsearch import CollectionSearch from ansible.playbook.helpers import load_list_of_roles @@ -39,10 +39,10 @@ class RoleMetadata(Base, CollectionSearch): within each Role (meta/main.yml). ''' - allow_duplicates = FieldAttribute(isa='bool', default=False) - dependencies = FieldAttribute(isa='list', default=list) - galaxy_info = FieldAttribute(isa='GalaxyInfo') - argument_specs = FieldAttribute(isa='dict', default=dict) + allow_duplicates = NonInheritableFieldAttribute(isa='bool', default=False) + dependencies = NonInheritableFieldAttribute(isa='list', default=list) + galaxy_info = NonInheritableFieldAttribute(isa='GalaxyInfo') + argument_specs = NonInheritableFieldAttribute(isa='dict', default=dict) def __init__(self, owner=None): self._owner = owner diff --git a/lib/ansible/playbook/role_include.py b/lib/ansible/playbook/role_include.py index 394603729d0..fb49ec4082b 100644 --- a/lib/ansible/playbook/role_include.py +++ b/lib/ansible/playbook/role_include.py @@ -22,7 +22,7 @@ from os.path import basename import ansible.constants as C from ansible.errors import AnsibleParserError -from ansible.playbook.attribute import FieldAttribute +from ansible.playbook.attribute import NonInheritableFieldAttribute from ansible.playbook.block import Block from ansible.playbook.task_include import TaskInclude from ansible.playbook.role import Role @@ -52,9 +52,9 @@ class IncludeRole(TaskInclude): # ATTRIBUTES # private as this is a 'module options' vs a task property - allow_duplicates = FieldAttribute(isa='bool', default=True, private=True) - public = FieldAttribute(isa='bool', default=False, private=True) - rolespec_validate = FieldAttribute(isa='bool', default=True) + allow_duplicates = NonInheritableFieldAttribute(isa='bool', default=True, private=True) + public = NonInheritableFieldAttribute(isa='bool', default=False, private=True) + rolespec_validate = NonInheritableFieldAttribute(isa='bool', default=True) def __init__(self, block=None, role=None, task_include=None): diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py index 6a9136d2541..ba35fcf0e32 100644 --- a/lib/ansible/playbook/task.py +++ b/lib/ansible/playbook/task.py @@ -66,22 +66,22 @@ class Task(Base, Conditional, Taggable, CollectionSearch): # inheritance is only triggered if the 'current value' is Sentinel, # default can be set at play/top level object and inheritance will take it's course. - args = FieldAttribute(isa='dict', default=dict) - action = FieldAttribute(isa='string') + args = NonInheritableFieldAttribute(isa='dict', default=dict) + action = NonInheritableFieldAttribute(isa='string') - async_val = FieldAttribute(isa='int', default=0, alias='async') - changed_when = FieldAttribute(isa='list', default=list) - delay = FieldAttribute(isa='int', default=5) + async_val = NonInheritableFieldAttribute(isa='int', default=0, alias='async') + changed_when = NonInheritableFieldAttribute(isa='list', default=list) + delay = NonInheritableFieldAttribute(isa='int', default=5) delegate_to = FieldAttribute(isa='string') delegate_facts = FieldAttribute(isa='bool') - failed_when = FieldAttribute(isa='list', default=list) - loop = FieldAttribute() + failed_when = NonInheritableFieldAttribute(isa='list', default=list) + loop = NonInheritableFieldAttribute() loop_control = NonInheritableFieldAttribute(isa='class', class_type=LoopControl, default=LoopControl) notify = FieldAttribute(isa='list') - poll = FieldAttribute(isa='int', default=C.DEFAULT_POLL_INTERVAL) - register = FieldAttribute(isa='string', static=True) - retries = FieldAttribute(isa='int', default=3) - until = FieldAttribute(isa='list', default=list) + poll = NonInheritableFieldAttribute(isa='int', default=C.DEFAULT_POLL_INTERVAL) + register = NonInheritableFieldAttribute(isa='string', static=True) + retries = NonInheritableFieldAttribute(isa='int', default=3) + until = NonInheritableFieldAttribute(isa='list', default=list) # deprecated, used to be loop and loop_args but loop has been repurposed loop_with = NonInheritableFieldAttribute(isa='string', private=True) diff --git a/test/integration/targets/keyword_inheritance/dep_keyword_inheritance.yml b/test/integration/targets/keyword_inheritance/dep_keyword_inheritance.yml new file mode 100644 index 00000000000..3d3b684ab40 --- /dev/null +++ b/test/integration/targets/keyword_inheritance/dep_keyword_inheritance.yml @@ -0,0 +1,8 @@ +- hosts: localhost + gather_facts: false + tasks: + - include_role: + name: "{{ item }}" + loop: + - setup_test_user + - role-meta-inheritance diff --git a/test/integration/targets/keyword_inheritance/roles/role-meta-inheritance/meta/main.yml b/test/integration/targets/keyword_inheritance/roles/role-meta-inheritance/meta/main.yml new file mode 100644 index 00000000000..b0af49fb143 --- /dev/null +++ b/test/integration/targets/keyword_inheritance/roles/role-meta-inheritance/meta/main.yml @@ -0,0 +1,4 @@ +dependencies: + - role: whoami + become: true + become_user: ansibletest0 diff --git a/test/integration/targets/keyword_inheritance/runme.sh b/test/integration/targets/keyword_inheritance/runme.sh index 6b78a06dc4f..1f13ef85c78 100755 --- a/test/integration/targets/keyword_inheritance/runme.sh +++ b/test/integration/targets/keyword_inheritance/runme.sh @@ -3,3 +3,5 @@ set -eux ANSIBLE_ROLES_PATH=../ ansible-playbook -i ../../inventory test.yml "$@" + +ANSIBLE_ROLES_PATH=../ ansible-playbook -i ../../inventory dep_keyword_inheritance.yml "$@"