diff --git a/changelogs/fragments/no-mutable-fieldattribute-defaults.yaml b/changelogs/fragments/no-mutable-fieldattribute-defaults.yaml new file mode 100644 index 00000000000..c5a7066d4b6 --- /dev/null +++ b/changelogs/fragments/no-mutable-fieldattribute-defaults.yaml @@ -0,0 +1,2 @@ +bugfixes: +- FieldAttribute - Do not use mutable defaults, instead allow supplying a callable for defaults of mutable types (https://github.com/ansible/ansible/issues/46824) diff --git a/lib/ansible/playbook/attribute.py b/lib/ansible/playbook/attribute.py index d688b83775b..b7dcce3e184 100644 --- a/lib/ansible/playbook/attribute.py +++ b/lib/ansible/playbook/attribute.py @@ -87,19 +87,8 @@ class Attribute: self.extend = extend self.prepend = prepend - if default is not None and self.isa in _CONTAINERS: - if default: - self.default = deepcopy(default) - else: - # Don't need to deepcopy default if the container is empty - # Note: switch to try: except once Python3 is more widespread - if hasattr(default, 'copy'): - self.default = default.copy() - else: - # list on python2 does not have .copy() - self.default = copy(default) - else: - self.default = default + if default is not None and self.isa in _CONTAINERS and not callable(default): + raise TypeError('defaults for FieldAttribute may not be mutable, please provide a callable instead') def __eq__(self, other): return other.priority == self.priority diff --git a/lib/ansible/playbook/base.py b/lib/ansible/playbook/base.py index 84c3fee1c10..48343050dd3 100644 --- a/lib/ansible/playbook/base.py +++ b/lib/ansible/playbook/base.py @@ -162,6 +162,9 @@ class FieldAttributeBase(with_metaclass(BaseMeta, object)): # need a unique object here (all members contained within are # unique already). self._attributes = self._attributes.copy() + for key, value in self._attributes.items(): + if callable(value): + self._attributes[key] = value() # and init vars, avoid using defaults in field declaration as it lives across plays self.vars = dict() diff --git a/lib/ansible/playbook/block.py b/lib/ansible/playbook/block.py index 72f091dfede..50483a9a486 100644 --- a/lib/ansible/playbook/block.py +++ b/lib/ansible/playbook/block.py @@ -32,9 +32,9 @@ from ansible.playbook.taggable import Taggable class Block(Base, Become, Conditional, Taggable): # main block fields containing the task lists - _block = FieldAttribute(isa='list', default=[], inherit=False) - _rescue = FieldAttribute(isa='list', default=[], inherit=False) - _always = FieldAttribute(isa='list', default=[], inherit=False) + _block = FieldAttribute(isa='list', default=list, inherit=False) + _rescue = FieldAttribute(isa='list', default=list, inherit=False) + _always = FieldAttribute(isa='list', default=list, inherit=False) # other fields _delegate_to = FieldAttribute(isa='string') diff --git a/lib/ansible/playbook/conditional.py b/lib/ansible/playbook/conditional.py index 90c213bde3d..874ba341855 100644 --- a/lib/ansible/playbook/conditional.py +++ b/lib/ansible/playbook/conditional.py @@ -49,7 +49,7 @@ class Conditional: to be run conditionally when a condition is met or skipped. ''' - _when = FieldAttribute(isa='list', default=[], extend=True, prepend=True) + _when = FieldAttribute(isa='list', default=list, extend=True, prepend=True) def __init__(self, loader=None): # when used directly, this class needs a loader, but we want to diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index 9d519a1b953..370a77f4df6 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -63,22 +63,22 @@ class Play(Base, Taggable, Become): _gather_timeout = FieldAttribute(isa='int', default=None, always_post_validate=True) # Variable Attributes - _vars_files = FieldAttribute(isa='list', default=[], priority=99) - _vars_prompt = FieldAttribute(isa='list', default=[], always_post_validate=False) + _vars_files = FieldAttribute(isa='list', default=list, priority=99) + _vars_prompt = FieldAttribute(isa='list', default=list, always_post_validate=False) # Role Attributes - _roles = FieldAttribute(isa='list', default=[], priority=90) + _roles = FieldAttribute(isa='list', default=list, priority=90) # Block (Task) Lists Attributes - _handlers = FieldAttribute(isa='list', default=[]) - _pre_tasks = FieldAttribute(isa='list', default=[]) - _post_tasks = FieldAttribute(isa='list', default=[]) - _tasks = FieldAttribute(isa='list', default=[]) + _handlers = FieldAttribute(isa='list', default=list) + _pre_tasks = FieldAttribute(isa='list', default=list) + _post_tasks = FieldAttribute(isa='list', default=list) + _tasks = FieldAttribute(isa='list', default=list) # Flag/Setting Attributes _force_handlers = FieldAttribute(isa='bool', always_post_validate=True) _max_fail_percentage = FieldAttribute(isa='percent', always_post_validate=True) - _serial = FieldAttribute(isa='list', default=[], 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) diff --git a/lib/ansible/playbook/play_context.py b/lib/ansible/playbook/play_context.py index a531deff22d..d1ec159bb9c 100644 --- a/lib/ansible/playbook/play_context.py +++ b/lib/ansible/playbook/play_context.py @@ -178,8 +178,8 @@ class PlayContext(Base): # general flags _verbosity = FieldAttribute(isa='int', default=0) - _only_tags = FieldAttribute(isa='set', default=set()) - _skip_tags = FieldAttribute(isa='set', default=set()) + _only_tags = FieldAttribute(isa='set', default=set) + _skip_tags = FieldAttribute(isa='set', default=set) _force_handlers = FieldAttribute(isa='bool', default=False) _start_at_task = FieldAttribute(isa='string') _step = FieldAttribute(isa='bool', default=False) diff --git a/lib/ansible/playbook/playbook_include.py b/lib/ansible/playbook/playbook_include.py index ed973a77f4c..ec3909e38e8 100644 --- a/lib/ansible/playbook/playbook_include.py +++ b/lib/ansible/playbook/playbook_include.py @@ -35,7 +35,7 @@ from ansible.template import Templar class PlaybookInclude(Base, Conditional, Taggable): _import_playbook = FieldAttribute(isa='string') - _vars = FieldAttribute(isa='dict', default=dict()) + _vars = FieldAttribute(isa='dict', default=dict) @staticmethod def load(data, basedir, variable_manager=None, loader=None): diff --git a/lib/ansible/playbook/role/metadata.py b/lib/ansible/playbook/role/metadata.py index 1a9df75634d..f4898503aad 100644 --- a/lib/ansible/playbook/role/metadata.py +++ b/lib/ansible/playbook/role/metadata.py @@ -39,7 +39,7 @@ class RoleMetadata(Base): ''' _allow_duplicates = FieldAttribute(isa='bool', default=False) - _dependencies = FieldAttribute(isa='list', default=[]) + _dependencies = FieldAttribute(isa='list', default=list) _galaxy_info = FieldAttribute(isa='GalaxyInfo') def __init__(self, owner=None): diff --git a/lib/ansible/playbook/taggable.py b/lib/ansible/playbook/taggable.py index 91b0191d2b0..38b43620a00 100644 --- a/lib/ansible/playbook/taggable.py +++ b/lib/ansible/playbook/taggable.py @@ -30,7 +30,7 @@ from ansible.template import Templar class Taggable: untagged = frozenset(['untagged']) - _tags = FieldAttribute(isa='list', default=[], listof=(string_types, int), extend=True) + _tags = FieldAttribute(isa='list', default=list, listof=(string_types, int), extend=True) def __init__(self): super(Taggable, self).__init__() diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py index 9ae95f82b57..72ec21c25f2 100644 --- a/lib/ansible/playbook/task.py +++ b/lib/ansible/playbook/task.py @@ -70,22 +70,22 @@ class Task(Base, Conditional, Taggable, Become): # inheritance is only triggered if the 'current value' is None, # default can be set at play/top level object and inheritance will take it's course. - _args = FieldAttribute(isa='dict', default=dict()) + _args = FieldAttribute(isa='dict', default=dict) _action = FieldAttribute(isa='string') _async_val = FieldAttribute(isa='int', default=0, alias='async') - _changed_when = FieldAttribute(isa='list', default=[]) + _changed_when = FieldAttribute(isa='list', default=list) _delay = FieldAttribute(isa='int', default=5) _delegate_to = FieldAttribute(isa='string') _delegate_facts = FieldAttribute(isa='bool') - _failed_when = FieldAttribute(isa='list', default=[]) + _failed_when = FieldAttribute(isa='list', default=list) _loop = FieldAttribute() _loop_control = FieldAttribute(isa='class', class_type=LoopControl, inherit=False) _notify = FieldAttribute(isa='list') _poll = FieldAttribute(isa='int', default=10) _register = FieldAttribute(isa='string') _retries = FieldAttribute(isa='int', default=3) - _until = FieldAttribute(isa='list', default=[]) + _until = FieldAttribute(isa='list', default=list) # deprecated, used to be loop and loop_args but loop has been repurposed _loop_with = FieldAttribute(isa='string', private=True, inherit=False) diff --git a/test/integration/targets/include_import/public_exposure/no_bleeding.yml b/test/integration/targets/include_import/public_exposure/no_bleeding.yml new file mode 100644 index 00000000000..b9db7132903 --- /dev/null +++ b/test/integration/targets/include_import/public_exposure/no_bleeding.yml @@ -0,0 +1,25 @@ +--- +- hosts: testhost + gather_facts: false + tasks: + - name: Static imports should expose vars at parse time, not at execution time + assert: + that: + - static_defaults_var == 'static_defaults' + - static_vars_var == 'static_vars' + - import_role: + name: static + - assert: + that: + - static_tasks_var == 'static_tasks' + - static_defaults_var == 'static_defaults' + - static_vars_var == 'static_vars' + +- hosts: testhost + gather_facts: false + tasks: + - name: Ensure vars from import_roles do not bleed between plays + assert: + that: + - static_defaults_var is undefined + - static_vars_var is undefined diff --git a/test/integration/targets/include_import/runme.sh b/test/integration/targets/include_import/runme.sh index cc3c1010b63..066c88ca28f 100755 --- a/test/integration/targets/include_import/runme.sh +++ b/test/integration/targets/include_import/runme.sh @@ -81,3 +81,4 @@ ANSIBLE_STRATEGY='free' ansible-playbook tasks/test_include_dupe_loop.yml -i ../ test "$(grep -c '"item=foo"' test_include_dupe_loop.out)" = 3 ansible-playbook public_exposure/playbook.yml -i ../../inventory "$@" +ansible-playbook public_exposure/no_bleeding.yml -i ../../inventory "$@" diff --git a/test/units/playbook/test_base.py b/test/units/playbook/test_base.py index 6562af99974..f52fd9e3f33 100644 --- a/test/units/playbook/test_base.py +++ b/test/units/playbook/test_base.py @@ -347,13 +347,13 @@ class BaseSubClass(base.Base): _test_attr_list = FieldAttribute(isa='list', listof=string_types, always_post_validate=True) _test_attr_list_no_listof = FieldAttribute(isa='list', always_post_validate=True) _test_attr_list_required = FieldAttribute(isa='list', listof=string_types, required=True, - default=[], always_post_validate=True) + default=list, always_post_validate=True) _test_attr_string = FieldAttribute(isa='string', default='the_test_attr_string_default_value') _test_attr_string_required = FieldAttribute(isa='string', required=True, default='the_test_attr_string_default_value') _test_attr_percent = FieldAttribute(isa='percent', always_post_validate=True) - _test_attr_set = FieldAttribute(isa='set', default=set(), always_post_validate=True) - _test_attr_dict = FieldAttribute(isa='dict', default={'a_key': 'a_value'}, always_post_validate=True) + _test_attr_set = FieldAttribute(isa='set', default=set, always_post_validate=True) + _test_attr_dict = FieldAttribute(isa='dict', default=lambda: {'a_key': 'a_value'}, always_post_validate=True) _test_attr_class = FieldAttribute(isa='class', class_type=ExampleSubClass) _test_attr_class_post_validate = FieldAttribute(isa='class', class_type=ExampleSubClass, always_post_validate=True)